mirror of
https://github.com/block/buzz.git
synced 2026-10-31 22:41:12 +00:00
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 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com> Signed-off-by: Brad Seiler <seiler@squareup.com>
This commit is contained in:
committed by
Brad Seiler
parent
d1835c8cdd
commit
5e8a39bb89
@@ -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
|
||||
|
||||
@@ -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 "
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user