From 5e8a39bb898bdf7f4411189da6a92356b965fa09 Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Mon, 26 Oct 2026 14:23:57 -0400 Subject: [PATCH] fix(test): pin unrouted registrations and reject repeated nests The route inventory allow-listed every `.fallback_service` inside `router::build_router`, and did not scan `.fallback` at all, so a new sub-router fallback could add an unclassified handler without changing the inventory. Allow-list entries now name the builder, call, and exact whitespace-normalized argument, and each may appear once. The existing buzz_v1 404 fallback is allow-listed explicitly. `builder_prefixes` kept one prefix per builder, so nesting a builder at a second prefix silently overwrote the first. A repeated nest is now a hard error. Five new fixtures cover a non-SPA fallback service, a repeated SPA fallback, a handler fallback inside and outside build_router, and a builder nested twice. All five fail against the previous scanner. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist Signed-off-by: Brad Seiler --- TESTING.md | 7 ++-- .../check-community-ban-route-inventory.py | 39 ++++++++++++++++--- ...heck-community-ban-route-inventory.test.py | 34 ++++++++++++++++ 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/TESTING.md b/TESTING.md index d636b9d5b1..6502cac928 100644 --- a/TESTING.md +++ b/TESTING.md @@ -41,9 +41,10 @@ named `router` or `*_router`, resolves literal or module-qualified `&str` constant paths, and applies `.nest(...)` prefixes from `router::build_router`. It fails when one of those registrations is missing from the table or a stale row remains, and on any shape it cannot classify: `.route_service`, -`.nest_service`, or `.fallback_service` (except the allow-listed SPA fallback), -`.nest` outside `build_router`, or a nest target that does not resolve to -exactly one router builder. `scripts/check-community-ban-route-inventory.test.py` +`.nest_service`, `.fallback_service`, or `.fallback` (except allow-listed +registrations, each pinned to its builder and exact argument and allowed once), +`.nest` outside `build_router`, a nest target that does not resolve to exactly +one router builder, or a builder nested more than once. `scripts/check-community-ban-route-inventory.test.py` pins each of those shapes with a fixture. Run both directly with: ```bash diff --git a/scripts/check-community-ban-route-inventory.py b/scripts/check-community-ban-route-inventory.py index af26e8b49a..b12b93732c 100644 --- a/scripts/check-community-ban-route-inventory.py +++ b/scripts/check-community-ban-route-inventory.py @@ -339,13 +339,28 @@ def builder_prefixes(builders) -> dict[str, str]: f"router::build_router: nest target {target} matches " f"{len(candidates)} router builders, expected exactly 1" ) + if candidates[0] in prefixes: + raise ValueError( + f"router::build_router: {candidates[0]} is nested more than once; " + "the inventory records one prefix per router builder" + ) prefixes[candidates[0]] = prefix return prefixes -# Service-style registrations bypass the per-route method classification, so -# each one is a hard error unless it is allow-listed here with its builder. -ALLOWED_SERVICE_REGISTRATIONS = {("router::build_router", "fallback_service")} +# Service and fallback registrations bypass the per-route method +# classification, so each one is a hard error unless this list names its +# builder, call, and exact (whitespace-normalized) argument, once. +ALLOWED_UNROUTED_REGISTRATIONS = { + # SPA static bundles; it only serves files, never a community API. + ("router::build_router", "fallback_service", "spa_fallback"), + # Unknown accessory paths answer 404 instead of SPA HTML. + ( + "api::buzz_v1::router", + "fallback", + '|| async { auth::Error::new(axum::http::StatusCode::NOT_FOUND, "not_found") }', + ), +} def collect_registered_routes() -> Counter: @@ -360,6 +375,7 @@ def collect_registered_routes() -> Counter: prefixes = builder_prefixes(builders) registrations: Counter = Counter() errors = [] + unrouted_seen = set() for path in production_sources(): source = path.read_text() mask = mask_comments_and_strings(source) @@ -373,7 +389,8 @@ def collect_registered_routes() -> Counter: return None for match in re.finditer( - r"\.\s*(route|nest|route_service|nest_service|fallback_service)\s*\(", mask + r"\.\s*(route|nest|route_service|nest_service|fallback_service|fallback)\s*\(", + mask, ): if in_ranges(match.start(), test_ranges): continue @@ -381,12 +398,22 @@ def collect_registered_routes() -> Counter: builder = enclosing_builder(match.start()) line = source.count("\n", 0, match.start()) + 1 location = f"{path.relative_to(ROOT)}:{line}" - if call.endswith("_service"): - if (builder, call) not in ALLOWED_SERVICE_REGISTRATIONS: + if call.endswith("_service") or call == "fallback": + opening = mask.find("(", match.start(), match.end()) + argument = " ".join( + source[opening + 1 : matching_delimiter(mask, opening)].split() + ) + registration = (builder, call, argument) + if registration not in ALLOWED_UNROUTED_REGISTRATIONS: errors.append( f"{location}: production .{call} registration is not classified; " "use .route with an HTTP method or allow-list it" ) + elif registration in unrouted_seen: + errors.append( + f"{location}: allow-listed .{call} registration appears more than once" + ) + unrouted_seen.add(registration) elif builder is None: errors.append( f"{location}: production .{call} registration " diff --git a/scripts/check-community-ban-route-inventory.test.py b/scripts/check-community-ban-route-inventory.test.py index 62876a1550..a554f98f22 100644 --- a/scripts/check-community-ban-route-inventory.test.py +++ b/scripts/check-community-ban-route-inventory.test.py @@ -101,6 +101,40 @@ class RouteScannerFixtures(unittest.TestCase): ADMIN.replace('get(probe))', "get(probe)).fallback_service(service)"), ) + def test_non_spa_fallback_service_in_build_router_is_rejected(self): + self.assert_rejected( + r"\.fallback_service registration is not classified", + build_router( + ".fallback_service(spa_fallback)" + '.merge(Router::new().route("/x", get(x)).fallback_service(private_service))' + ), + ) + + def test_repeated_spa_fallback_service_is_rejected(self): + self.assert_rejected( + r"\.fallback_service registration appears more than once", + build_router(".fallback_service(spa_fallback).fallback_service(spa_fallback)"), + ) + + def test_handler_fallback_is_rejected(self): + self.assert_rejected( + r"\.fallback registration is not classified", + build_router(".fallback(private_handler)"), + ) + + def test_handler_fallback_outside_build_router_is_rejected(self): + self.assert_rejected( + r"\.fallback registration is not classified", + build_router(), + ADMIN.replace('get(probe))', "get(probe)).fallback(private_handler)"), + ) + + def test_builder_nested_twice_is_rejected(self): + self.assert_rejected( + r"api::admin::router is nested more than once", + build_router('.nest("/alternate-admin", api::admin::router(state.clone()))'), + ) + def test_nest_outside_build_router_is_rejected(self): self.assert_rejected( r"\.nest registration outside router::build_router",