alkhttp/tasks/server/review-001-extra-routes-auth.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
review-001-extra-routes-auth	Mount extra_routes under the bearer-auth middleware (SRV-01)	completed		narrow	medium	component	implementation	server review-001 security

Description

Review 001 finding SRV-01 (docs/reviews/001-initial-implementation-review.md, empirically verified): build_router (src/server/adapter.rs:170-183) applies bearer_auth_middleware via route_layer before merging extra_routes, so assembly-layer custom routes are mounted without auth — contradicting ADR-046 §4 ("custom routes carry the same auth middleware by default"). Amplified by ResolvedIdentity extraction being infallible: a custom handler silently receives None on every request.

Fix: apply the auth layer after merging extra routes so the documented default holds. Per-route opt-out remains the deployment's explicit choice (extras may carry their own inner layers). Also decide and document the SRV-06 interaction: a same-method collision on a reserved path panics in axum's merge (sanctioned), but a different-method merge (e.g. custom POST /search) silently serves on a reserved path — enforce RESERVED_PATHS or document the behavior; the exported constant currently has no reader.

Acceptance Criteria

• A test mounts an extra route and asserts ResolvedIdentity is resolved from the bearer token (auth applies)
• A test shows an extra route carrying its own layer can still opt out (documented escape hatch)
• SRV-06 decision landed: per-method reserved-path merges rejected/documented; RESERVED_PATHS enforced or un-exported
• ADR-046 §4 language matches the implemented default after the fix
• cargo test and cargo clippy --all-targets -- -D warnings pass

References

• docs/reviews/001-initial-implementation-review.md (Part A, SRV-01, SRV-06)
• docs/architecture/decisions/046-assembly-layer-custom-http-routes.md

Notes

SRV-06 decision: enforce RESERVED_PATHS (kept exported, now with a reader). build_router only merges extras into the default surface; it cannot silently reject, and returning a Result would ripple the error through with_decoy's rebuild for no real gain — a wrong path is a programming error, so panic-at-construction matches axum's own same-method behavior and ADR-046 §3's "panics/warns" clause. Mechanism: a probe MethodRouter occupied on all nine routable methods is merged against the extras before the real merge, so a same-method overlap panics in axum's merge (as before) and a different-method registration (e.g. custom POST /search next to the default GET /search) now panics too. any()-mounted extras are NOT caught (axum's fallback-first merge shape does not conflict per-method) — the standard, method-router style of registering extras is covered. Outside the reserved set, same-path-different-method merges remain legal. Enforcement lives in build_router (not with_extra_routes) so with_decoy's rebuild path is covered as well. Module docs in src/server/adapter.rs document the rule; ADR-046 §3 gained one sentence restating the per-method rule (§4 needed no change — the implemented default now matches its text).

Auth-ordering note: the MCP nest (/mcp) is merged before the route_layer and keeps its own explicit bearer_auth_middleware layer, so reordering does not un-auth it (existing mcp_endpoint_serves_four_gateway_tools_bearer_gated test stays green). The WS upgrade route's stricter ws_bearer_auth inner layer is unaffected (route_layer on the route itself).

Summary

src/server/adapter.rs: moved the shared bearer_auth_middleware route_layer from before to after the extra_routes merge (SRV-01), so assembly-layer custom routes now resolve the bearer token by default — extra_routes_resolve_bearer_identity_through_the_default_auth asserts ResolvedIdentity is Some("worker-a") for a valid token and None without one; extra_route_with_own_layer_can_opt_out_of_default_auth shows a route's own inner layer winning (documented escape hatch). Landed the SRV-06 enforcement (enforce_reserved_paths: pre-merge probe; RESERVED_PATHS stays exported and now has a reader) — extra_route_on_reserved_path_panics_at_construction asserts a custom POST /search panics with axum's overlapping-route message; extra_route_on_non_reserved_path_merges_cleanly covers the legal different-method merge outside the reserved set. Kept green: mcp_endpoint_serves_four_gateway_tools_bearer_gated (MCP nest carries its own auth layer), all gateway tests, and the full suite — cargo test (215) + cargo test --all-features (260 + 29 integration), cargo clippy --all-targets/--all-features -- -D warnings, and cargo fmt --check all pass. ADR-046: §3 one-sentence per-method clarification; §4 unchanged. The /mcp body-limit fix (SRV-03) and the with_decoy .take() fix (SRV-05) are different tasks and untouched.