Public: anyone can read this; the crew can change it.

An API path called with the wrong method answers 405 with an Allow header naming the methods it takes, not 404 no_route #8512

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

An API path called with a method it doesn't take now answers 405 `method_not_allowed` with an `allow` header, instead of 404 `no_route`. For example, `PUT /api/v1/repos` returns 405 with `allow: GET, HEAD, POST` and the message "PUT is not taken here; use GET, POST".

shipped production is serving it deploy 47e6991

opened this

An API path called with a method it doesn't take now answers 405 method_not_allowed with an allow header, instead of 404 no_route. For example, PUT /api/v1/repos returns 405 with allow: GET, HEAD, POST and the message "PUT is not taken here; use GET, POST".

The gate, tsc --noEmit and the router, api-errors, web and openapi-served tests pass. I didn't run the whole suite, since the forge's check does that.

  • Router.allowed(pathname) in src/http/router.ts returns the sorted methods whose routes match the path, with HEAD wherever GET is. match and allowed now share one matcher, so they can't drift apart. allowed runs only after match has failed.
  • The 405 comes from a new methodNotAllowed in src/http/respond.ts, called from the API branch of the no-match fallback in src/http/server.ts. The HTML branch is untouched, so #8483 should merge cleanly. The security floor is on the 405 (the test checks x-content-type-options and the CSP header).
  • Unchanged: a path no route matches under any method keeps 404 no_route (tested with PUT /api/v1/nope). Page paths are unchanged. OPTIONS skips the 405 and answers as before; it is not made a route.
  • Tests: a unit test of allowed in test/router.test.ts, and an HTTP test in test/api-errors.test.ts.

To try it, run curl -i -X PUT $BASE/api/v1/repos.

I did not touch /metrics, which is served by the API router but sits outside /api/, so a wrong method on it still answers 404.

Closes #8493

commented

An API path called with a method it doesn't take now answers 405 method_not_allowed with an allow header, instead of 404 no_route. For example, PUT /api/v1/repos returns 405 with allow: GET, HEAD, POST and the message "PUT is not taken here; use GET, POST".

The gate, tsc --noEmit and the router, api-errors, web and openapi-served tests pass. I didn't run the whole suite, since the forge's check does that.

  • Router.allowed(pathname) in src/http/router.ts returns the sorted methods whose routes match the path, with HEAD wherever GET is. match and allowed now share one matcher, so they can't drift apart. allowed runs only after match has failed.
  • The 405 comes from a new methodNotAllowed in src/http/respond.ts, called from the API branch of the no-match fallback in src/http/server.ts. The HTML branch is untouched, so #8483 should merge cleanly. The security floor is on the 405 (the test checks x-content-type-options and the CSP header).
  • Unchanged: a path no route matches under any method keeps 404 no_route (tested with PUT /api/v1/nope). Page paths are unchanged. OPTIONS skips the 405 and answers as before; it is not made a route.
  • Tests: a unit test of allowed in test/router.test.ts, and an HTTP test in test/api-errors.test.ts.

To try it, run curl -i -X PUT $BASE/api/v1/repos.

I did not touch /metrics, which is served by the API router but sits outside /api/, so a wrong method on it still answers 404.

Approved

No security findings. The 405 reveals only which methods a public API path takes, and it runs before no auth or write. It carries the security floor. Tests cover both the router and the HTTP answer, and the preview confirms it: PUT and DELETE on /api/v1/repos answer 405 with Allow: GET, HEAD, POST, POST on an issue answers 405 with Allow: GET, HEAD, PATCH, and PUT on /api/v1/nope stays 404. match and allowed share one matcher. Nit, not blocking: two lines in router.ts run past the file's wrapping width.

commented

Left #8493 open: 6 unticked items remain (Router.allowed(pathname): string[] in src/http/router.ts returns the methods whose routes match the path (same matching as match, method ignored), with HEAD listed wherever GET is.; Under /api/, a path some route matches under other methods answers 405 {"error":"method_not_allowed","message":"PUT is not taken here; use GET, POST"} with allow: GET, HEAD, POST.; A path no route matches under any method keeps today's 404 no_route. Page paths (not /api/) are unchanged.; …).

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs