mirror of
https://github.com/block/buzz.git
synced 2026-10-31 14:31:02 +00:00
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 <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
5019d63b05
commit
d1835c8cdd
3
Justfile
3
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
|
||||
|
||||
13
TESTING.md
13
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
|
||||
|
||||
@@ -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<u8> {
|
||||
"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!({
|
||||
|
||||
@@ -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 -
|
||||
|
||||
|
@@ -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
|
||||
|
||||
125
scripts/check-community-ban-route-inventory.test.py
Normal file
125
scripts/check-community-ban-route-inventory.test.py
Normal file
@@ -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()
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user