diff --git a/changelog.d/794-api-method-guard.md b/changelog.d/794-api-method-guard.md index 530be060e..97801e091 100644 --- a/changelog.d/794-api-method-guard.md +++ b/changelog.d/794-api-method-guard.md @@ -1,7 +1,4 @@ -- The read-only query proxies state their method contract. `/api/flights`, - `/api/military` and `/api/route` answered any verb with the same body - while ignoring the request; they now answer `405 Method Not Allowed` to - anything but `GET`, the way `/api/geocode` and the other query proxies - already did, and declare `Allow: GET` with it the way `/api/radio/stations` - does. The refusal precedes rate limiting and any upstream call - (daikaginza, #794). +- Answer `405 Method Not Allowed` with `Allow: GET` to anything but `GET` on + `/api/flights`, `/api/military` and `/api/route`, as the other query proxies + do. They answered every verb with the same body. The refusal comes before + rate limiting and any upstream call (daikaginza, #794). diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index f2569f363..6a3b07c5d 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -6491,7 +6491,7 @@ apply to `public/` assets or built-preview serving. - `/api/overpass` is bounded by body/response caps, per-client/global rate limits, concurrency limits, operator-configured upstreams, in-flight dedupe, cache bounds, and static validation that every selector is spatially bounded. - `/api/military-installations` uses an independent limiter with the same 90-per-client/300-global one-minute bounds, so viewport installation refreshes never consume `/api/overpass` annotation capacity. - `/api/route` proxies bounded OSRM route requests for annotation routes, with profile allowlisting, distance caps, response caps, caching, and sanitized "no route found" errors. -- `/api/flights`, `/api/military`, and `/api/route` are read-only query proxies and answer `405 Method Not Allowed` to anything but `GET`, as `/api/geocode` and the other query proxies already did, declaring `Allow: GET` with it as `/api/radio/stations` does. The refusal precedes rate limiting and any upstream call. +- `/api/flights`, `/api/military`, and `/api/route` answer `405 Method Not Allowed` with `Allow: GET` to anything but `GET`, before rate limiting and any upstream call. - `/api/flights` negotiates gzip. The cached global snapshot and the regional adsb.lol fallback are each gzipped once per refresh (about 860 KB becomes about 250 KB). Clients that offer gzip get that frame; other clients, and bodies under 1 KiB or that gzip cannot shrink, get identity bytes. Both answers carry `Vary: Accept-Encoding`. Uncached bodies, such as auth failures, are never sent under a cached frame's encoding. - Track endpoints: `/api/vessels/track?mmsi=` (server-accumulated ring buffers; sub-route handled before the rows snapshot), `/api/flights/track?icao24=` (OAuth, 60s cache, sanitized errors, independent OpenSky credit bucket), `/api/military/track?hex=` (400 for an invalid hex, otherwise `503 history_unavailable` with `Retry-After: 1800` and `Cache-Control: no-store`, no upstream request; military tracking uses its locally accumulated trail). - Realtime debug logs redact API keys, bearer tokens, client secrets, and image data URLs before writing to disk; request bodies are size-capped. diff --git a/server/providers/aircraft/adsb-lol.js b/server/providers/aircraft/adsb-lol.js index 80aacc7c4..6bdc65e9f 100644 --- a/server/providers/aircraft/adsb-lol.js +++ b/server/providers/aircraft/adsb-lol.js @@ -83,9 +83,7 @@ export function adsbLolProxy() { // This mount also matches its /track route, served by its own handler. if (new URL(req.url || '/', 'http://localhost').pathname !== '/') return next(); - // Read-only snapshot endpoint; match the method contract its sibling - // query proxies already declare instead of serving every verb alike. - // Checked after the /track hand-off, so that route keeps its own. + // After the /track hand-off, so /track keeps its own method handling. if (req.method !== 'GET') { res.writeHead(405, { 'Content-Type': 'application/json', diff --git a/server/providers/aircraft/opensky.js b/server/providers/aircraft/opensky.js index 0a23978a5..1cc30a482 100644 --- a/server/providers/aircraft/opensky.js +++ b/server/providers/aircraft/opensky.js @@ -496,10 +496,7 @@ export function openSkyProxy() { // This mount also matches its /track route, served by its own handler. if (new URL(req.url || '/', 'http://localhost').pathname !== '/') return next(); - // Read-only snapshot endpoint: state the method contract the way - // /api/geocode and the other query proxies state it, rather than - // answering any verb with the same body and ignoring the request body. - // Checked after the /track hand-off, so that route keeps its own. + // After the /track hand-off, so /track keeps its own method handling. if (req.method !== 'GET') { res.writeHead(405, { 'Content-Type': 'application/json', diff --git a/server/providers/places/routes.js b/server/providers/places/routes.js index 2c67f1f5c..7b0a81449 100644 --- a/server/providers/places/routes.js +++ b/server/providers/places/routes.js @@ -227,8 +227,6 @@ export function installRouteMiddleware( // only shape that asks the upstream for them. A response WITHOUT steps is // byte for byte what this endpoint has always returned. middlewares.use('/api/route', async (req, res) => { - // The documented shape above is a GET with query parameters; say so, - // the way the other query proxies do, instead of answering any verb. if (req.method !== 'GET') { res.writeHead(405, { 'Content-Type': 'application/json', Allow: 'GET' }); res.end(JSON.stringify({ error: 'Method Not Allowed' })); diff --git a/src/tooling/liveProviders.test.mjs b/src/tooling/liveProviders.test.mjs index e40e4079c..df71ec9a7 100644 --- a/src/tooling/liveProviders.test.mjs +++ b/src/tooling/liveProviders.test.mjs @@ -586,10 +586,7 @@ test('track backfill proxy returns 502 on an oversized upstream body and caches }); test('the aircraft snapshot routes decline a non-GET before any upstream work', async (t) => { - // Both routes answered every verb with the full snapshot, unlike the query - // proxies beside them, so nothing said whether a body was read or ignored. - // A stubbed fetch that throws proves the refusal happens before the handler - // reaches upstream, rather than after paying for the work and discarding it. + // A fetch that throws proves the refusal comes before any upstream call. t.mock.method(globalThis, 'fetch', async (url) => { throw new Error(`upstream must not be reached: ${url}`); }); @@ -601,8 +598,7 @@ test('the aircraft snapshot routes decline a non-GET before any upstream work', for (const method of ['POST', 'PUT', 'DELETE', 'PATCH']) { const response = await request(route, '/', method); assert.equal(response.statusCode, 405, `${method} ${route}`); - // RFC 9110 section 15.5.6 requires a 405 to name the methods it does - // accept, which is what `/api/radio/stations` already does in-tree. + // RFC 9110 section 15.5.6: a 405 names the methods it accepts. assert.equal(response.headers.allow, 'GET', `${method} ${route}`); assert.deepEqual(JSON.parse(response.body), { error: 'Method Not Allowed', @@ -612,9 +608,8 @@ test('the aircraft snapshot routes decline a non-GET before any upstream work', }); test('the snapshot method check hands each /track route on to its own handler', async () => { - // Each snapshot mount is a prefix of its /track route, so the snapshot - // handler sees /track requests first and must hand them on before it - // judges the method; otherwise the 405 would answer for the other route. + // The snapshot mount also matches /track, so it must hand /track on + // before it checks the method. for (const [plugin, route] of [ [providers.openSkyProxy(), '/api/flights'], [providers.adsbLolProxy(), '/api/military'], diff --git a/src/tooling/placeProviders.test.mjs b/src/tooling/placeProviders.test.mjs index 4b728b6ac..4ae9558ff 100644 --- a/src/tooling/placeProviders.test.mjs +++ b/src/tooling/placeProviders.test.mjs @@ -502,9 +502,7 @@ test('the upstream host is pinned: a redirect is refused, not followed', async ( }); test('the routing proxy declines a non-GET before rate limiting or upstream', async (t) => { - // The documented shape is a GET with query parameters, but every verb was - // answered alike. Refuse first: a stubbed fetch that throws shows the - // rejection costs neither an upstream call nor a rate-limit slot. + // A fetch that throws proves the refusal comes before any upstream call. const request = install(installRouteMiddleware); t.mock.method(globalThis, 'fetch', async () => { throw new Error('upstream must not be reached'); @@ -516,7 +514,7 @@ test('the routing proxy declines a non-GET before rate limiting or upstream', as method, ); assert.equal(response.statusCode, 405, method); - // RFC 9110 section 15.5.6 requires a 405 to name the methods it accepts. + // RFC 9110 section 15.5.6: a 405 names the methods it accepts. assert.equal(response.headers.allow, 'GET', method); assert.deepEqual(response.body, { error: 'Method Not Allowed' }); }