test: allowlist dead-entry + FastAPI dependant-contract guards (PER-15249)#325
Conversation
…5249) The route-auth regression guard PER-15249 asks for already shipped with PER-15245 as horizon/tests/test_route_auth_audit.py (walks app.routes, fails any ungated APIRoute, flags un-gateable mounts, single-sources the allowlist from horizon.authentication.PUBLIC_ROUTE_PATHS). The `pytests` job that runs it is already a required status check on main, so the block-merge requirement is met. This closes the two remaining gaps the issue calls out: - test_allowlist_has_no_dead_entries: every PUBLIC_ROUTE_PATHS entry must match a mounted route, so the allowlist cannot rot into a pre-authorised hole. /scalar is the one documented exception (registered in PermitPDP.__init__ after _configure_api_routes, invisible to the audit app); the test also fails if that exemption ever goes stale. - test_router_level_dependencies_surface_in_flat_dependant: the codex-required empirical proof that include_router(dependencies=[...]) deps and nested Depends() surface in get_flat_dependant on the resolved FastAPI (0.125.0; pin >=0.115.6,<1), with diagnostics for a future bump. Verified: 8/8 in the file, 120/120 in horizon/tests/, ruff check + format clean. Red-before-green confirmed out-of-band: replaying the audit at pre-fix 02f7655 flags exactly /policy-updater/trigger, /data-updater/trigger, and /kong. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🔍 Vulnerabilities of
|
| digest | sha256:49280260f06774d9b06e599371cafc58440c8dcb516ed47bfaf4b5bfa251045f |
| vulnerabilities | |
| platform | linux/amd64 |
| size | 225 MB |
| packages | 264 |
📦 Base Image python:3.10-alpine3.22
| also known as |
|
| digest | sha256:c8f94b3bb77e6ea9015ccd091b7f8aec1b1fcbca95159675235d9a93788797cd |
| vulnerabilities |
Description
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
|
There was a problem hiding this comment.
Pull request overview
This PR strengthens the existing “fail-closed” route-auth audit by adding two contract-style tests that (1) prevent the public-route allowlist from accumulating dead entries and (2) lock in the FastAPI dependency-flattening behavior the audit relies on.
Changes:
- Add a test to ensure every
PUBLIC_ROUTE_PATHSentry corresponds to an actually-mounted route (with a single, checked exemption for/scalar). - Add a contract test proving router-level
include_router(..., dependencies=[...])dependencies and nestedDepends()dependencies surface viaget_flat_dependant. - Update imports/constants in the audit test module to support the new tests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two test-only fixes from the PR review: - The nested-flattening pin was vacuous: it reused fake_gate as both the router-level dependency and wrapper's sub-dependency, so the subset assertion passed via the directly-attached copy without ever exercising get_flat_dependant's recursion - the exact regression it names. Use a distinct inner_gate reachable only through wrapper, so the assertion fails if nested flattening ever breaks. (Verified: inner_gate is absent from the route's direct dependants and only surfaces after recursion.) - The dead-entry failure message now names ALLOWLIST_ENTRIES_NOT_IN_AUDIT_APP so a maintainer who adds a genuinely-public route registered after _configure_api_routes (the documented /scalar pattern) is pointed at the real fix instead of the misleading "route renamed" advice. Also documents the accepted trade-off of the /scalar exemption: an exempted path is not covered by the dead-entry rot check. Kept test-only by choice; the exemption-free alternative (MockPermitPDP mirroring production's post-config registration) is noted for the future. Verified: 8/8 in the file, 120/120 in horizon/tests/, ruff check + format clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gression-guard-block-merge
Closes the two remaining gaps for PER-15249 (Route auth-coverage regression guard, block-merge).
Context: most of PER-15249 already shipped
The route-auth regression guard the issue asks for landed with PER-15245 (#321) as
horizon/tests/test_route_auth_audit.py: it builds the app the production way (MockPermitPDP→_configure_api_routes), walksapp.routes, fails anyAPIRoutelacking a recognized gate (enforce_pdp_token,enforce_pdp_control_key,JWTAuthenticator,require_listener_token— nested closures flattened viaget_flat_dependant), flags un-gateable mounts, and single-sources the allowlist fromhorizon.authentication.PUBLIC_ROUTE_PATHS. ThepytestsCI job that runs it is already a required status check onmain(repo ruleset 11170517), so the block-merge requirement is met. The issue's "middleware invisible toroute.dependant" concern is moot: OPAL trigger routes are gated in place by_gate_opal_trigger_routesinhorizon/pdp.py, not via middleware.What this PR adds
Two tests, one file, no production-code changes:
test_allowlist_has_no_dead_entries— everyPUBLIC_ROUTE_PATHSentry must match a mounted route, so the allowlist can't rot into a pre-authorized hole for a future route to fall into./scalaris the one documented exception (registered inPermitPDP.__init__after_configure_api_routes, so it's invisible to the audit-built app); the test also fails if that exemption ever goes stale.test_router_level_dependencies_surface_in_flat_dependant— the issue's "codex-required" empirical proof, now a permanent contract test:include_router(dependencies=[...])deps and nestedDepends()both surface viaget_flat_dependanton the resolved FastAPI (0.125.0; pin>=0.115.6,<1), exercised through the audit's own_route_auth_gateshelper, with diagnostics for a future FastAPI bump. Uses the repo'sAnnotated[..., Depends(...)]convention to stayB008-clean.Verification
test_route_auth_audit.py: 8/8 passed (was 6)horizon/tests/: 120/120 passed (parity with thepytestsjob)ruff checkclean,ruff format --checkclean02f7655flags exactlyPOST /policy-updater/trigger,POST /data-updater/trigger,POST /kong— the three routes PER-15244/15245 gated.🤖 Generated with Claude Code