Prove route auth coverage at build time with a security manifest (Part of #1604)#1850
Conversation
Add an explicit #[public] route marker, derive a build-time auth classification for every route (gated / public / framework / unclassified), and add `autumn routes audit` — a CI gate that emits a schema-versioned JSON security manifest and exits non-zero on any unclassified route, naming the offending route and handler. Part of #1604.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c17bd0db4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn classify( | ||
| source: &RouteSource, | ||
| api_doc: &crate::openapi::ApiDoc, | ||
| ) -> (RouteClassification, Vec<String>, Vec<String>, bool) { |
There was a problem hiding this comment.
When an app uses generated repository APIs with policy = ..., the generated Route carries that runtime guard in route.repository.has_policy (see RepositoryApiMeta::has_policy and the repository macro's generated policy checks), but this classifier only receives ApiDoc and therefore marks those protected CRUD routes as unclassified because their ApiDoc::has_policy stays at the default. In a CI run of autumn routes audit, policy-protected repository endpoints will fail the new gate even though the handlers enforce authorization; pass the Route or repository metadata into classification and treat repository.has_policy as gated.
Useful? React with 👍 / 👎.
| public: #is_public, | ||
| module_path: ::core::module_path!(), |
There was a problem hiding this comment.
This wires #[public] into the standard HTTP route macro only, but the audit gate reads ApiDoc::public for every Route. Routes emitted by #[static_get] and #[ws] still build ApiDoc with ..Default::default() and never call api_doc::is_public, so even adding #[public] to those handlers leaves them unclassified and makes autumn routes audit fail with no annotation that can turn those routes green. I checked the route emitters in autumn-macros/src/static_route.rs and autumn-macros/src/ws.rs.
Useful? React with 👍 / 👎.
…ute macros Two false-failure fixes in the `autumn routes audit` classification gate. P1: repository auto-API routes (`#[repository(api = ..., policy = ...)]`) carry their record-level authorization guard on `RepositoryApiMeta::has_policy`, not on the handler `ApiDoc` (the generated CRUD handlers build their ApiDoc with `..Default::default()` and never set `secured`/`has_policy`). `classify()` only saw the ApiDoc, so a policy-protected CRUD endpoint classified `unclassified` and failed the gate despite enforcing authorization. Thread the route's repository metadata into `classify()` and treat `repository.has_policy` as `gated` (with `policy = true`). A bare `#[repository(api = ...)]` with no policy still classifies `unclassified`, which the gate should keep flagging. P2: `#[public]` detection (`api_doc::is_public`) was only wired into the standard HTTP route macro. `#[static_get]` and `#[ws]` built their ApiDoc without calling `is_public`, so `#[public]` on those handlers was ignored and they stayed `unclassified` with no way to turn green. Wire `is_public` (and `module_path` for consistent audit diagnostics) into both emitters, mirroring the HTTP route macro. Part of #1604.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45784c4d6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let output = Command::new(&binary) | ||
| .env("AUTUMN_DUMP_ROUTES", "1") | ||
| .stdout(std::process::Stdio::piped()) | ||
| .stderr(std::process::Stdio::inherit()) |
There was a problem hiding this comment.
If an app mounts a raw Axum router with AppBuilder::merge() or nest(), run_dump_routes_mode warns on stderr and omits those routes (autumn/src/app.rs:4300-4308), but the audit command only parses stdout and still exits based on the listed routes. In that context CI can pass and emit a manifest that is missing unaudited endpoints, so the command no longer proves route auth coverage; make audit mode treat the hidden-router warning/count as a failure or an explicit unclassified entry.
Useful? React with 👍 / 👎.
| api_version: None, | ||
| status: None, | ||
| sunset_opt_out: None, | ||
| ..Default::default() |
There was a problem hiding this comment.
When the default AdminPlugin is installed, admin_router wraps these routes in the role-check middleware, but this metadata now inherits RouteInfo::default() for the new audit fields, whose classification is Unclassified. run_dump_routes_mode appends declared_routes directly, so autumn routes audit will fail every protected admin route in apps using the default admin plugin; populate the classification/roles from the plugin's require_role setting (or mark public when auth is disabled) instead of leaving the default.
Useful? React with 👍 / 👎.
| package: package.as_deref(), | ||
| bin: bin.as_deref(), |
There was a problem hiding this comment.
With the existing autumn routes -p blog audit ordering, clap stores -p blog on the parent Commands::Routes, but this branch shadows it with the audit subcommand's absent package and passes None to compile_binary/find_binary. In workspaces that audits the wrong target or errors with multiple binaries unless users move the same flag after audit; fall back to the parent package/bin when the subcommand values are unset.
Useful? React with 👍 / 👎.
…in routes, honor parent target flags Three soundness gaps in the `autumn routes audit` coverage gate: - Omitted routes: raw routers added via AppBuilder::merge()/nest() are unenumerable and were silently dropped from the dumped listing, letting the gate pass while missing endpoints. The dump now emits a stable machine-readable stderr marker (OMITTED_ROUTES_MARKER) with the omitted count; audit captures stderr, forwards it, and hard-fails (non-zero) with guidance when any routers were omitted. - Default admin routes: AdminPlugin's declared route metadata inherited RouteInfo::default() (Unclassified), false-failing every guarded admin route out of the box. admin_route_infos now classifies from require_role: Gated (carrying the role) when a role is required, Public when auth is explicitly disabled. - Parent target flags: `autumn routes -p blog audit` dropped the parent -p/--bin because dispatch read only the audit subcommand's own values. Dispatch now falls back to the parent flags (audit.package.or(parent)), so both flag orderings resolve the same target. Part of #1604.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4afd70ce6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| // Fail the gate on either unclassified routes or omitted (unprovable) ones. | ||
| let failed = omitted > 0 || audit_exit_code(&routes) != 0; |
There was a problem hiding this comment.
In apps using a plugin that both nests a raw router and declares its route metadata—e.g. AdminPlugin::build still does app.nest(&prefix, router).declare_plugin_routes(declared) at autumn-admin-plugin/src/lib.rs:270-275—the child dump includes the classified declared routes in stdout but also emits the omitted marker because nest_routers.len() > 0. This branch then exits 1 solely due to omitted > 0, so autumn routes audit false-fails the first-party admin plugin even after its routes are marked gated/public; distinguish undeclared raw routers from declared plugin mounts before failing.
Useful? React with 👍 / 👎.
A prior commit in this PR made `autumn routes audit` hard-fail whenever `run_dump_routes_mode` reports any raw router mounted via `.merge()`/`.nest()` (the `OMITTED_ROUTES_MARKER`), because such routers are opaque to route enumeration and their auth posture can't be proven. But the first-party `AdminPlugin::build` nests a raw router AND declares that router's route metadata via `declare_plugin_routes`. Its endpoints therefore DO appear in the listing (correctly classified gated/public), yet the nested raw router still tripped the omitted marker, so the audit false-failed the first-party admin plugin. Distinguish a genuinely-undeclared raw `nest()`/`merge()` (unauditable, must fail) from a nested router whose routes were declared alongside the mount (auditable, must not count as omitted) via an explicit builder-level association: a new `AppBuilder::nest_declared` records the mount's prefix in `declared_nest_prefixes`, and the omitted-router count subtracts those declared-covered nests. `AdminPlugin` now mounts through `nest_declared`. Bare `nest()`/`merge()` mounts stay opaque and still hard-fail the gate, preserving the soundness guarantee. Only the omitted-count computation changed; the serving path and route-listing JSON shape are unchanged. Part of #1604.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c1f5fd10e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| false, | ||
| ); | ||
| } | ||
| let repo_has_policy = repository.is_some_and(|r| r.has_policy); |
There was a problem hiding this comment.
When a repository API uses scope = ... without policy = ..., the generated list route carries RepositoryApiMeta { has_policy: false, scope_check: Some(...) } and enforces the registered scope before returning records. Because this classification only considers repository.has_policy, those scope-protected GET /api/... routes are emitted as unclassified, so autumn routes audit fails even though the endpoint has an authorization guard; include the repository scope metadata (for example scope_check.is_some()) in the gated classification.
Useful? React with 👍 / 👎.
| let hidden = omitted_router_count( | ||
| merge_routers.len(), | ||
| nest_routers.len(), | ||
| declared_nest_prefixes.len(), |
There was a problem hiding this comment.
For plugins already following the documented app.nest(prefix, router).declare_plugin_routes(routes) pattern, declared_nest_prefixes remains empty because only the new nest_declared() API records a covered mount. This omitted count therefore still reports those declared nested routers as hidden and makes autumn routes audit exit 1 even when their RouteInfo entries are present and classified; fresh evidence is that the gate subtracts only declared_nest_prefixes.len(), not declarations registered through the existing API.
Useful? React with 👍 / 👎.
…red nests in audit Fix two route-auth coverage regressions in the #1604 audit gate. route_listing::classify now treats a repository auto-API declared with `scope = ...` (but no `policy = ...`) as Gated: its generated list handler enforces the registered scope at runtime while leaving both the handler ApiDoc and RepositoryApiMeta::has_policy at defaults, so it was previously false-failed as Unclassified. A repository route with neither policy nor scope stays Unclassified (fail-closed). No scope name is recorded on the meta, so scopes stays empty and policy stays false. Omitted-router accounting now recognises the documented `app.nest(prefix, router).declare_plugin_routes(routes)` coverage pattern: a nested mount is covered (not omitted) when at least one declared route's path falls under its prefix (segment-boundary aware). This subsumes the just-added nest_declared helper, which is removed along with the declared_nest_prefixes bookkeeping; AdminPlugin reverts to the plain documented pattern. A bare nest with no covering declaration and every merge() (rootless) still count as omitted, so the gate stays fail-closed. Part of #1604.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk-dev #1850 +/- ##
=============================================
+ Coverage 86.82% 87.07% +0.24%
=============================================
Files 273 278 +5
Lines 202424 208509 +6085
=============================================
+ Hits 175761 181556 +5795
- Misses 26663 26953 +290 🚀 New features to boost your workflow:
|
…nsions (Part of #1627) (#1879) * feat(routes-audit): security posture manifest v2 with provenance envelope (Part of #1627) Extend the merged #1850 route-audit manifest into a provenance-tagged, multi-dimension security posture manifest (first #1627 slice). Schema v2: - Each dimension is wrapped in a `{ provenance, source, entries }` envelope, and a top-level `excluded` list names dimensions deferred to later slices with the provenance class they will eventually carry. - `routes` (provable) keeps its v1 entry shape, only wrapped in the envelope. - New `csrf` dimension (declared): one entry per mutating route with `csrf_enforced`/`exempt`, mirroring the runtime CsrfLayer predicate; the configured exemptions are recorded once at the dimension level. - New `security_headers` dimension (declared): one entry per header key with the effective declared value (including the resolved CSP template) and an `emitted` flag. Plumbing (macro-free): the `AUTUMN_DUMP_ROUTES` dump additionally emits the resolved CSRF + headers config on a stderr marker line (`[autumn:security-config] `), gated on `AUTUMN_DUMP_SECURITY=1` so only `autumn routes audit` sees it and the routes-only JSON parse path stays byte-compatible. Emitted lists are sorted for byte-determinism. Tests: red->green falsifiability fixtures for each dimension driving the real `build_manifest` path (disabling CSRF / adding an exempt prefix flips exactly the affected csrf entries; weakening x_frame_options / emptying the CSP changes exactly that header entry; byte-identical rebuild), plus a consolidated `manifest_posture` integration module covering the autumn-web dump contract. Docs: new guide page defining the provable / declared / runtime-only provenance classes with one example each, cross-linked from routes-cli. CHANGELOG intentionally not edited. * Use cleaned exempt_paths in CSRF check and dedup security dump lists - routes_audit: per-route path_is_exempt now uses the sorted+deduped exempt_paths local, matching the dimension-level exempt_paths record. - route_listing: dedup safe_methods and exempt_paths after sorting so the serialized wire payload is duplicate-free. * fix(routes-audit): align security manifest declared dimensions with runtime Four honesty fixes so the security-posture manifest never claims more than the runtime actually enforces: - CSP nonce (A): dump the nonce-aware CSP template with the stable AUTUMN_CSP_NONCE placeholder when nonce injection is active + default CSP, mirroring ComputedHeaders::from_config. Extracted a shared resolved_content_security_policy() as the single source of truth. - Webhook CSRF exemptions (B): fold every configured webhook endpoint path into csrf.exempt_paths (sorted + deduped), matching apply_csrf_middleware's with_exempt_path calls, so webhook POST routes report exempt/unenforced. - WebSocket routes (C): treat WS-listed routes as CSRF-safe. #[ws] routes are mounted as GET, so CsrfLayer sees GET and never validates a token. - HeaderValue parse (D): emitted = non-empty AND HeaderValue::from_str ok, mirroring the runtime layer that skips invalid header values. Adds a red->green fixture per finding. * fix(routes-audit): drop csp_nonce from security_headers dimension csp_nonce is a config toggle that shapes the CSP value, not a response header. Runtime stores the nonce in request extensions and substitutes it into Content-Security-Policy; it never emits a csp_nonce header. Reporting it in security_headers overstated the actual response headers. The nonce toggle's effect stays visible via the nonce-aware CSP value. * fix(routes-audit): fold mounted unsubscribe path into CSRF exempt dump Runtime's apply_csrf_middleware exempts the framework-owned RFC 8058 one-click unsubscribe endpoint from CSRF under the mail feature when should_mount_unsubscribe_endpoint() holds, but SecurityDump::from_config only folded webhook endpoints, under-reporting that exemption in the v2 manifest. Fold UNSUBSCRIBE_PATH in under the identical cfg+predicate. * fix(routes-audit): decide WS CSRF safety on normalized GET method WS routes are listed with the synthetic method WS but mount via axum::routing::get, so runtime CsrfService checks the real GET against the configured safe_methods. The audit previously treated WS as unconditionally safe, so an app that removes GET from safe_methods would have its WebSocket upgrade CSRF-validated at runtime while the manifest omitted it entirely. Normalize WS to GET before the safe-method check and drop the unconditional WS branch, so WS routes are excluded under default config (GET safe) but surface in csrf.entries with the exempt-path predicate applied when GET is not a safe method. The entry keeps its listed WS method for display, consistent with the routes dimension. * fix(routes-audit): enumerate mutating actuator routes so csrf posture sees them The route listing derived framework actuator routes from actuator_endpoint_paths, a GET-only, paths-only list, and wrapped every entry as a framework GET. Two consequences fed the csrf posture dimension a wrong picture of the mutating attack surface: - PUT {prefix}/loggers/{name} (mounted when actuator.sensitive) was absent from the listing entirely, so csrf never saw it. - POST {prefix}/webhooks/replay (mounted under http-client) was emitted as a phantom GET, since the listing forced GET on every actuator path even though runtime mounts only POST there. Add actuator_mutating_routes(prefix, sensitive) returning (method, path) pairs gated to match the mounts, a method-aware RouteInfo::framework_route constructor, and push those routes after the GET loop in append_framework_routes. Drop /webhooks/replay from the GET path list so it is emitted once, as POST. /webhooks/dlq stays GET (mounted as GET). Adds listing-level and csrf-dimension fixtures asserting the PUT loggers route surfaces with csrf enforced and the phantom GET replay is gone. * fix(routes-audit): list mounted one-click unsubscribe routes so csrf posture sees the exempt POST When the mail feature is enabled and the default RFC 8058 one-click unsubscribe endpoint is mounted, runtime merges unsubscribe_router() (GET + POST at UNSUBSCRIBE_PATH) and exempts that path from CSRF. SecurityDump::from_config already folds UNSUBSCRIBE_PATH into csrf.exempt_paths, but no framework RouteInfo was appended for it, so build_csrf_dimension emitted no entry for the runtime POST and the manifest under-reported CSRF posture. append_framework_routes now lists GET + POST framework routes at UNSUBSCRIBE_PATH under the same should_mount_unsubscribe_endpoint() predicate the exempt fold uses, so the POST surfaces as { csrf_enforced: false, exempt: true } matching runtime. * fix(routes): restore webhook-replay barrier bypass; enumerate job-status route; disclose serve-path routers Change 1 (regression fix): a prior commit dropped `/webhooks/replay` from `actuator_endpoint_paths` to kill a phantom GET in the route listing, but that helper is dual-use — `StartupBarrierState::from_config` seeds the startup barrier's actuator allow-list from it. Removing the path meant the mounted `POST {prefix}/webhooks/replay` no longer bypassed the startup barrier. Restore the path in `actuator_endpoint_paths` (runtime allow-list complete again) and instead exclude any path also produced by `actuator_mutating_routes` from the GET-wrap in `append_framework_routes`, so the listing carries only the real POST and no phantom GET. Change 2: enumerate the tracked-job status route `GET /_autumn/jobs/{token}` in `append_framework_routes`, gated on `jobs.tracking.route_enabled` (default true), matching the router mount. Change 3: add one honest `excluded` manifest entry (`serve_path_routers`) disclosing that opt-in serve-path HTTP surfaces (MCP, inbound-mail, storage, SEO) are injected after the route-dump early-exit and so are not enumerated at build time, with their CSRF-exempt/self-protected mutating endpoints noted; enumeration deferred to a dump-plumbing follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjQWjvP71LMRPsQexcYPvi --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Part of #1604. First slice: route/auth as a build-time, diffable security artifact. No change to runtime authorization semantics or to
autumn-web's default behavior — existing apps are unaffected until they run the new command.Before / After
#[secured]/#[authorize]). Nothing distinguishes a deliberately public route from one where someone forgot the guard, and no build step fails when a route has no auth posture.autumn routesmerely displays routes.autumn routes auditcommand lists each route's classification, emits a stable-ordered JSON security manifest, and exits non-zero when any route is unclassified, naming the exact route + handler — ready to wire into CI as a gate. A new#[public]marker lets a handler declare itself deliberately open, so unaudited routes fail rather than pass silently.What's in this slice
#[public]marker (autumn-macros) — mirrors the#[secured]marker mechanism; threads apublicflag throughApiDoc.autumn/src/route_listing.rs) — each route →gated(roles/scopes/policy) ·public·framework(pre-classified) ·unclassified. Previously the listing dropped auth posture; it now propagates the real roles/scopes/policy.autumn routes audit(autumn-cli) — nested subcommand (existingautumn routeslisting unchanged). Flags:--manifest <path>,--json,-p/--package,--bin,--strict. Emits a schema-versioned manifest and fails the build on unclassified routes.{ "schema_version": 1, "dimensions": { "routes": [ { path, method, name, classification, roles, scopes, policy, source, provenance } ] } }, stable-ordered by(path, method). Every entry isprovenance: "provable"(derived from macro-expanded code) so Expand the build-time manifest to cover the full security posture #1627 can add dimensions without breaking the schema.How it works
Auth posture already lands on
Route.api_docat macro-expansion time (ApiDoc.secured/required_roles/required_scopes/has_policy, populated byautumn-macros/src/route.rsfrom#[secured]/#[authorize]). Routes are exhaustively enumerable via the ownedroutes!→__autumn_route_info_*→AUTUMN_DUMP_ROUTESpipeline. This slice adds the#[public]bit, classifies incollect_route_infos, and adds the CLI audit + manifest + exit-code gate (following thedoctor.rsexit-code pattern).Falsifiability (the keystone AC), proven end-to-end against the
helloexampleRED — three unannotated handlers:
GREEN — after adding
#[public]/#[secured]:Backed by library-, CLI-, and compile-based tests (
route_listing::unclassified_route_turns_green_when_guarded_or_public,routes_audit::audit_gate_red_then_green, macroroute_macro_marks_public_*).Verification
cargo fmt --all -- --checkcleancargo clippy -p autumn-web -p autumn-macros -p autumn-cli -p autumn-admin-plugin --all-targets -- -D warningscleanautumn-macros498,autumn-web --lib route_listing::40,autumn-cliaudit/parse 129)Deferred (follow-on slices)
autumn newscaffold CI — flag is threaded but currently advisory; fail-on-unclassified is already the gate default. (AC: strict prod-profile posture check.)declare_plugin_routes) don't carryApiDocposture yet, so they classifyunclassified— threading posture through declared routes fits Expand the build-time manifest to cover the full security posture #1627.module_path!()(real, cheap);file!()deferred.Generated by Claude Code