From d1835c8cdd461e46042db2f506490e68129d8f30 Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Mon, 26 Oct 2026 14:12:16 -0400 Subject: [PATCH] fix(test): fail the route inventory on every shape it cannot classify The scanner keyed `&str` route constants by bare name, so main's `api::profiles::SEARCH_PATH` collided with `api::gifs::SEARCH_PATH` and reported a false duplicate. Key constants by module and resolve qualified references against that module. Unclassified registrations are now hard errors instead of silent passes: `.route_service`, `.nest_service`, and `.fallback_service` (except the allow-listed SPA fallback in `build_router`), `.nest` outside `router::build_router`, and nest targets that do not resolve to exactly one router builder. A fixture self-test pins each shape and runs in `just test-unit` and `scripts/run-tests.sh unit`. Classify the three routes main added: `POST /profiles/search` (member gate, now in the HTTP ban matrix) and the operator-plane `GET /operator/communities/joined` and `GET /operator/communities/nip-fi-binding`. 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 --- Justfile | 3 +- TESTING.md | 13 +- .../tests/community_ban_routes.rs | 5 +- .../community-ban-route-inventory.tsv | 3 + .../check-community-ban-route-inventory.py | 131 +++++++++++++----- ...heck-community-ban-route-inventory.test.py | 125 +++++++++++++++++ scripts/run-tests.sh | 3 + 7 files changed, 242 insertions(+), 41 deletions(-) create mode 100644 scripts/check-community-ban-route-inventory.test.py diff --git a/Justfile b/Justfile index d0143768f0..1369299ced 100644 --- a/Justfile +++ b/Justfile @@ -397,9 +397,10 @@ test-unit: #!/usr/bin/env bash set -euo pipefail ./scripts/test-ensure-local-relay-key.sh + python3 scripts/check-community-ban-route-inventory.py + python3 scripts/check-community-ban-route-inventory.test.py # Shared Huddle transport, codec, and loopback authentication regressions. cargo test -p buzz-audio-client - python3 scripts/check-community-ban-route-inventory.py if command -v cargo-nextest &>/dev/null; then cargo nextest run -p buzz-core -p buzz-auth --lib # Mesh JWKS proxy-bypass regression: infra-free loopback I/O with diff --git a/TESTING.md b/TESTING.md index e19bd55acc..d636b9d5b1 100644 --- a/TESTING.md +++ b/TESTING.md @@ -37,13 +37,18 @@ combination space (PR #6807). Scope Playwright locators — unscoped classifies the relay's production Axum routes by listener, method, path, authentication plane, membership behavior, and community-restriction behavior. The inventory guard finds direct `.route(...)` calls in production functions -named `router` or `*_router`, resolves literal or declared `&str` paths, and -applies direct literal `.nest(...)` prefixes from the relay router. It fails -when one of those registrations is missing from the table or a stale row -remains. Run it directly with: +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` +pins each of those shapes with a fixture. Run both directly with: ```bash python3 scripts/check-community-ban-route-inventory.py +python3 scripts/check-community-ban-route-inventory.test.py ``` `just test-unit` and `just test` run the guard; CI also runs the ignored HTTP diff --git a/crates/buzz-test-client/tests/community_ban_routes.rs b/crates/buzz-test-client/tests/community_ban_routes.rs index de9b3d3eea..762a6df0ac 100644 --- a/crates/buzz-test-client/tests/community_ban_routes.rs +++ b/crates/buzz-test-client/tests/community_ban_routes.rs @@ -35,6 +35,7 @@ const COMMUNITY_BAN_MATRIX_CASES: &[&str] = &[ "count", "gif_search", "gif_share", + "profile_search", "workflow_runs", "workflow_approvals", "invite_mint", @@ -270,7 +271,9 @@ fn request_body(case: &str, actor: &Principal) -> Vec { "limit": 1 }])) .expect("serialize Nostr filter"), - "gif_search" | "gif_share" | "invite_mint" | "buzz_read_state" => b"{".to_vec(), + "gif_search" | "gif_share" | "profile_search" | "invite_mint" | "buzz_read_state" => { + b"{".to_vec() + } "media_upload" => b"not an image file".to_vec(), "git_upload_pack" | "git_receive_pack" => Vec::new(), "git_default_post" => serde_json::to_vec(&serde_json::json!({ diff --git a/crates/buzz-test-client/tests/fixtures/community-ban-route-inventory.tsv b/crates/buzz-test-client/tests/fixtures/community-ban-route-inventory.tsv index b1431793fc..75f7ae6bf4 100644 --- a/crates/buzz-test-client/tests/fixtures/community-ban-route-inventory.tsv +++ b/crates/buzz-test-client/tests/fixtures/community-ban-route-inventory.tsv @@ -10,16 +10,19 @@ http_query relay POST /query community_nip98 community enforce matrix:query memb http_count relay POST /count community_nip98 community enforce matrix:count member gif_search relay POST /gifs/search community_nip98 community enforce matrix:gif_search member gif_share relay POST /gifs/share community_nip98 community enforce matrix:gif_share member +profile_search relay POST /profiles/search community_nip98 community enforce matrix:profile_search member workflow_runs relay GET /workflows/{workflow_id}/runs community_nip98 community enforce matrix:workflow_runs member workflow_approvals relay GET /workflows/{workflow_id}/runs/{run_id}/approvals community_nip98 community enforce matrix:workflow_approvals member operator_list_communities relay GET /operator/communities operator_control not_applicable not_applicable:relay_operator_control classified - operator_provision_community relay POST /operator/communities operator_control not_applicable not_applicable:relay_operator_control classified - +operator_list_joined_communities relay GET /operator/communities/joined operator_control not_applicable not_applicable:relay_operator_control classified - operator_register_listener_keys relay POST /operator/listener/pubkeys operator_control not_applicable not_applicable:relay_operator_control classified - operator_remove_listener_keys relay DELETE /operator/listener/pubkeys operator_control not_applicable not_applicable:relay_operator_control classified - operator_archive_community relay POST /operator/communities/archive operator_control not_applicable not_applicable:relay_operator_control classified - operator_unarchive_community relay POST /operator/communities/unarchive operator_control not_applicable not_applicable:relay_operator_control classified - operator_delete_community relay POST /operator/communities/delete operator_control not_applicable not_applicable:relay_operator_control classified - operator_community_availability relay GET /operator/communities/availability operator_control not_applicable not_applicable:relay_operator_control classified - +operator_community_nip_fi_binding relay GET /operator/communities/nip-fi-binding operator_control not_applicable not_applicable:relay_operator_control classified - operator_transfer_community relay POST /operator/communities/transfer operator_control not_applicable not_applicable:relay_operator_control classified - invite_mint relay POST /api/invites community_nip98 community enforce matrix:invite_mint admin join_policy relay GET /api/join-policy public_metadata not_applicable not_applicable:public_metadata classified - diff --git a/scripts/check-community-ban-route-inventory.py b/scripts/check-community-ban-route-inventory.py index 6fbc511f1f..af26e8b49a 100644 --- a/scripts/check-community-ban-route-inventory.py +++ b/scripts/check-community-ban-route-inventory.py @@ -191,14 +191,24 @@ def in_ranges(position: int, ranges: list[tuple[int, int]]) -> bool: return any(start <= position < end for start, end in ranges) +def module_path(path: Path) -> str: + module_parts = list(path.relative_to(RELAY_SRC).with_suffix("").parts) + if module_parts[-1] in {"mod", "lib", "main"}: + module_parts.pop() + return "::".join(module_parts) + + +def production_sources(): + for path in sorted(RELAY_SRC.rglob("*.rs")): + if "tests" in path.parts or path.name.endswith("_tests.rs"): + continue + yield path + + def router_functions(path: Path, source: str, mask: str): pattern = re.compile(r"\bfn\s+([A-Za-z_]\w*)\s*\(") excluded = test_module_ranges(mask) - relative = path.relative_to(RELAY_SRC).with_suffix("") - module_parts = list(relative.parts) - if module_parts[-1] == "mod": - module_parts.pop() - module = "::".join(module_parts) + module = module_path(path) for match in pattern.finditer(mask): name = match.group(1) @@ -241,31 +251,52 @@ def string_value(expression: str) -> str | None: return raw.group(2) if raw else None -def route_constants() -> dict[str, str]: +def route_constants() -> dict[tuple[str, str], str]: + """Map `(module, NAME)` to each production `&str` constant's value. + + Keyed by module so same-named constants in different modules (for example + `api::gifs::SEARCH_PATH` and `api::profiles::SEARCH_PATH`) stay distinct. + """ constants = {} pattern = re.compile( r"(?:pub(?:\([^)]*\))?\s+)?const\s+([A-Za-z_]\w*)\s*:\s*&str\s*=\s*" r'("(?:\\.|[^"\\])*"|r(?:#+)?".*?"(?:#+)?)\s*;', re.DOTALL, ) - for path in RELAY_SRC.rglob("*.rs"): - if "tests" in path.parts or path.name.endswith("_tests.rs"): - continue + for path in production_sources(): + module = module_path(path) for match in pattern.finditer(path.read_text()): value = string_value(match.group(2)) if value is not None: - constants[match.group(1)] = value + constants[(module, match.group(1))] = value return constants -def route_path(expression: str, constants: dict[str, str]) -> str: +def route_path(expression: str, constants: dict[tuple[str, str], str], module: str) -> str: literal = string_value(expression) if literal is not None: return literal - constant = re.search(r"([A-Za-z_]\w*)\s*$", expression.strip()) - if constant and constant.group(1) in constants: - return constants[constant.group(1)] - raise ValueError(f"route path is not a literal or known string constant: {expression.strip()}") + reference = re.fullmatch( + r"(?:(?:crate|self)::)?((?:[A-Za-z_]\w*::)*)([A-Za-z_]\w*)", expression.strip() + ) + if reference is None: + raise ValueError( + f"route path is not a literal or known string constant: {expression.strip()}" + ) + qualifier = reference.group(1).rstrip(":") + name = reference.group(2) + candidates = [key for key in constants if key[1] == name] + if qualifier: + candidates = [ + key for key in candidates if key[0] == qualifier or key[0].endswith(f"::{qualifier}") + ] + elif len(candidates) > 1: + candidates = [key for key in candidates if key[0] == module] + if len(candidates) != 1: + raise ValueError( + f"route path is not a literal or known string constant: {expression.strip()}" + ) + return constants[candidates[0]] def registered_calls(mask: str, source: str, function_start: int, function_end: int, call: str): @@ -291,25 +322,36 @@ def builder_prefixes(builders) -> dict[str, str]: if main is None: raise ValueError("could not find relay router::build_router") _, _, path, source, mask, start, _, end = main + constants = route_constants() prefixes = {} for first, second_mask, second_source in registered_calls(mask, source, start, end, "nest"): - prefix = route_path(first, route_constants()) + prefix = route_path(first, constants, module_path(path)) target_match = re.search(r"((?:[A-Za-z_]\w*::)*[A-Za-z_]\w*)\s*\(", second_mask) - if prefix is None or target_match is None: - continue - target = target_match.group(1) + if target_match is None: + raise ValueError( + f"router::build_router: nest target is not a router builder call: " + f"{second_source.strip()}" + ) + target = re.sub(r"^(?:crate|self)::", "", target_match.group(1)) candidates = [b[0] for b in builders if b[0] == target or b[0].endswith(f"::{target}")] - if len(candidates) == 1: - prefixes[candidates[0]] = prefix + if len(candidates) != 1: + raise ValueError( + f"router::build_router: nest target {target} matches " + f"{len(candidates)} router builders, expected exactly 1" + ) + 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")} + + def collect_registered_routes() -> Counter: constants = route_constants() builders = [] - for path in RELAY_SRC.rglob("*.rs"): - if "tests" in path.parts or path.name.endswith("_tests.rs"): - continue + for path in production_sources(): source = path.read_text() mask = mask_comments_and_strings(source) for qualified_name, name, start, brace, end in router_functions(path, source, mask): @@ -318,31 +360,50 @@ def collect_registered_routes() -> Counter: prefixes = builder_prefixes(builders) registrations: Counter = Counter() errors = [] - for path in RELAY_SRC.rglob("*.rs"): - if "tests" in path.parts or path.name.endswith("_tests.rs"): - continue + for path in production_sources(): source = path.read_text() mask = mask_comments_and_strings(source) test_ranges = test_module_ranges(mask) - router_ranges = [ - (builder[6], builder[7]) for builder in builders if builder[2] == path - ] - for match in re.finditer(r"\.\s*route\s*\(", mask): + path_builders = [builder for builder in builders if builder[2] == path] + + def enclosing_builder(position: int) -> str | None: + for builder in path_builders: + if builder[6] <= position < builder[7]: + return builder[0] + return None + + for match in re.finditer( + r"\.\s*(route|nest|route_service|nest_service|fallback_service)\s*\(", mask + ): if in_ranges(match.start(), test_ranges): continue - if not any(start <= match.start() < end for start, end in router_ranges): - line = source.count("\n", 0, match.start()) + 1 + call = match.group(1) + 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: + errors.append( + f"{location}: production .{call} registration is not classified; " + "use .route with an HTTP method or allow-list it" + ) + elif builder is None: errors.append( - f"{path.relative_to(ROOT)}:{line}: production .route registration " + f"{location}: production .{call} registration " "is outside a recognized router builder" ) + elif call == "nest" and builder != "router::build_router": + errors.append( + f"{location}: production .nest registration outside " + "router::build_router is not classified" + ) for qualified_name, name, path, source, mask, _, start, end in builders: listener = "health" if qualified_name == "router::build_health_router" else "relay" prefix = prefixes.get(qualified_name, "") for path_expression, method_mask, _ in registered_calls(mask, source, start, end, "route"): try: - route = route_path(path_expression, constants) + route = route_path(path_expression, constants, module_path(path)) except ValueError as error: errors.append(f"{path.relative_to(ROOT)}::{name}: {error}") continue diff --git a/scripts/check-community-ban-route-inventory.test.py b/scripts/check-community-ban-route-inventory.test.py new file mode 100644 index 0000000000..62876a1550 --- /dev/null +++ b/scripts/check-community-ban-route-inventory.test.py @@ -0,0 +1,125 @@ +#!/usr/bin/env python3 +"""Fixture self-tests: every route shape the scanner can't classify must fail.""" + +from __future__ import annotations + +import importlib.util +import tempfile +import textwrap +import unittest +from pathlib import Path + +SCRIPT = Path(__file__).resolve().parent / "check-community-ban-route-inventory.py" +SPEC = importlib.util.spec_from_file_location("route_inventory", SCRIPT) +scanner = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(scanner) + +API_MOD = """ +pub mod admin; +pub mod gifs; +pub mod profiles; +""" +GIFS = """ +pub(crate) const SEARCH_PATH: &str = "/gifs/search"; +""" +PROFILES = """ +pub(crate) const SEARCH_PATH: &str = "/profiles/search"; +""" +ADMIN = """ +pub fn router(state: State) -> Router { + Router::new().route("/probe", get(probe)) +} +""" + + +def build_router(extra: str = "") -> str: + return textwrap.dedent( + f""" + pub fn build_router(state: State) -> Router {{ + Router::new() + .route(api::gifs::SEARCH_PATH, post(api::gifs::search)) + .route(api::profiles::SEARCH_PATH, post(api::profiles::search)) + .nest("/api/admin/v1", api::admin::router(state.clone())) + {extra} + }} + """ + ) + + +class RouteScannerFixtures(unittest.TestCase): + def scan(self, router: str, admin: str = ADMIN): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + src = root / "crates/buzz-relay/src" + (src / "api").mkdir(parents=True) + (src / "router.rs").write_text(router) + (src / "api/mod.rs").write_text(API_MOD) + (src / "api/gifs.rs").write_text(GIFS) + (src / "api/profiles.rs").write_text(PROFILES) + (src / "api/admin.rs").write_text(admin) + saved = scanner.ROOT, scanner.RELAY_SRC + scanner.ROOT, scanner.RELAY_SRC = root, src + try: + return scanner.collect_registered_routes() + finally: + scanner.ROOT, scanner.RELAY_SRC = saved + + def assert_rejected(self, message: str, router: str, admin: str = ADMIN): + with self.assertRaisesRegex(ValueError, message): + self.scan(router, admin) + + def test_same_named_constants_resolve_per_module(self): + routes = self.scan(build_router()) + self.assertEqual( + set(routes), + { + ("relay", "POST", "/gifs/search"), + ("relay", "POST", "/profiles/search"), + ("relay", "GET", "/api/admin/v1/probe"), + }, + ) + + def test_spa_fallback_service_is_allowed(self): + self.scan(build_router(".fallback_service(spa_fallback)")) + + def test_route_service_is_rejected(self): + self.assert_rejected( + r"\.route_service registration is not classified", + build_router('.route_service("/probe", service)'), + ) + + def test_nest_service_is_rejected(self): + self.assert_rejected( + r"\.nest_service registration is not classified", + build_router('.nest_service("/probe", service)'), + ) + + def test_fallback_service_outside_build_router_is_rejected(self): + self.assert_rejected( + r"\.fallback_service registration is not classified", + build_router(), + ADMIN.replace('get(probe))', "get(probe)).fallback_service(service)"), + ) + + def test_nest_outside_build_router_is_rejected(self): + self.assert_rejected( + r"\.nest registration outside router::build_router", + build_router(), + ADMIN.replace('get(probe))', 'get(probe)).nest("/zz", other_router(state))'), + ) + + def test_unresolved_nest_target_is_rejected(self): + self.assert_rejected( + r"nest target missing::router matches 0 router builders", + build_router('.nest("/zz", missing::router(state.clone()))'), + ) + + def test_unknown_route_constant_is_rejected(self): + self.assert_rejected( + r"not a literal or known string constant: api::gifs::MISSING", + build_router(".route(api::gifs::MISSING, get(missing))"), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 24cb2205c8..73eb997571 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -81,6 +81,9 @@ run_unit_tests() { run_test_step "community-ban route inventory" \ python3 scripts/check-community-ban-route-inventory.py + run_test_step "community-ban route scanner fixtures" \ + python3 scripts/check-community-ban-route-inventory.test.py + run_test_step "buzz-core tests" \ cargo test -p buzz-core --lib -- --nocapture