From 18ffd15b5e6c38a099569e7e7eefbb3db0d81286 Mon Sep 17 00:00:00 2001 From: Brad Seiler Date: Thu, 29 Oct 2026 20:22:01 -0400 Subject: [PATCH] test(auth): bind route controls to exact outcomes and merged routers Co-authored-by: Sarnow <1894dcbafaa4df1ac607a9bf879ac0b7d77fbccb0e0f4619ecc252c8adbad8a2@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler --- TESTING.md | 47 +++---- .../tests/community_ban_routes.rs | 131 ++++++++++++++---- .../fixtures/community-ban-regressions.tsv | 4 + .../community-ban-route-inventory.tsv | 4 +- .../check-community-ban-route-inventory.py | 40 ++++-- ...heck-community-ban-route-inventory.test.py | 35 +++++ 6 files changed, 194 insertions(+), 67 deletions(-) diff --git a/TESTING.md b/TESTING.md index 6502cac928..936de3aaac 100644 --- a/TESTING.md +++ b/TESTING.md @@ -44,7 +44,9 @@ row remains, and on any shape it cannot classify: `.route_service`, `.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` +one router builder, or a builder nested more than once. New or repeated +`.merge(...)` registrations also fail; existing root aliases and the Git +settings merge are pinned to their exact builder and argument. `scripts/check-community-ban-route-inventory.test.py` pins each of those shapes with a fixture. Run both directly with: ```bash @@ -63,8 +65,12 @@ Use a fresh disposable local relay, PostgreSQL, Redis, and MinIO stack for that command, and set `DATABASE_URL` or `BUZZ_TEST_DATABASE_URL` to the disposable PostgreSQL database. Start the relay with `BUZZ_V1_ENABLED=true` so the accessory-route cases exercise their production handlers. The matrix creates -unique tenant hosts and identities; GIF -requests stop at malformed local input, and Git/workflow requests use absent resources. +unique tenant hosts and identities. Unset `BUZZ_KLIPY_API_KEY`: GIF controls +assert the exact provider-disabled response without contacting a provider. +The malformed JPEG control asserts its validation response; Git, workflow and +media-read controls assert exact missing-resource responses. Successful reads +and writes must return their expected JSON shape, not merely avoid a ban. +Ordinary-member routes also run a clear delegated-agent control. Do not point these tests at a shared, development, or hosted database or relay. The inventory keeps membership separate from restriction policy. Public NIP-11, @@ -72,26 +78,15 @@ NIP-05, policy documents, and health probes have no authenticated community principal. Anonymous policy-receipt acceptance is pre-membership and has no principal to restrict. Invite claim is also pre-membership, but it carries a verified NIP-98 principal and is classified as restriction-enforced in the -inventory. Enforcement will land with BUZZ-268; coverage remains pending until -that sibling lands and qualifies the updated v1/v2 lifecycle. After BUZZ-268 -lands, the intended contract is to retain valid -unexpired v1 codes and mint only v2, with a 72-hour default and 30-day maximum -lifetime. Both formats will reject banned claimants and owners transactionally, -and issuer bans will permanently invalidate v2. For v1, an unset operator -`invalid_after` will use natural expiry; when set, the code will reject at or -after that absolute boundary, while an earlier code expiry still takes effect. -That operator boundary will not affect v2. Legacy v1 codes have no issuer -identity, so issuer-specific revocation remains limited until a verified fleet -drain or security approval. This sibling contract, including the operator -cutoff, is not implemented by BUZZ-269. Relay-operator APIs, deployment-admin APIs, +inventory. The integrated invitation regressions cover restricted nonmembers, +verified owners and v1/v2 lifecycle behavior; see +[Invitation compatibility and revocation](#invitation-compatibility-and-revocation) +for the authoritative contract. Relay-operator APIs, deployment-admin APIs, NIP-FI commands, the localhost Git hook, and the gated mesh test endpoint use -separate auth planes; their exemption reason is recorded on each route row. -Workflow webhooks use a secret-authenticated caller with no user principal. -After BUZZ-272 lands, run admission will also check the saved workflow owner's -community ban. That behavior is not implemented by BUZZ-269, and its coverage -remains explicitly pending BUZZ-272; the pending row is not an exemption for -banned owners. BUZZ-269 currently has two pending sibling cases: invite claim and -workflow-owner webhook admission; final integration requires zero pending rows. +separate auth planes; each exemption is recorded in its route row. +Secret-authenticated workflow webhooks recheck the saved workflow owner's +community ban through the workflow engine. Their PostgreSQL regression covers +owner bans, tenant scope and failed lookups. No sibling coverage remains pending. Root and huddle admission, final-admission races, fail-closed reads, owner-to- agent restriction, timeout behavior, and tenant-scoped eviction point to their @@ -108,9 +103,11 @@ direct `.route(...)` calls inside production functions named `router` or ending in `_router`; a `.route(...)` elsewhere is a scanner error. It resolves a `.nest(...)` prefix only when the relay's `build_router` passes a literal or a resolved string constant and a direct call to another recognized router function. It does not -expand macros, discover `.route_service(...)` or `.nest_service(...)`, resolve -dynamic path/prefix expressions, or interpret inline nested routers and -conditional/opaque router composition. It also does not derive auth or +expand macros or interpret dynamic paths, prefixes or arbitrary router +composition. Service-style calls and unsupported nesting or merging are errors, +rather than silently omitted registrations. The existing root merge aliases and +Git settings merge are explicit, single-use exceptions; their builders' direct +routes remain inventoried. It also does not derive auth or restriction policy from handler code, or prove that the named regression function contains the expected assertions or is selected by CI. If production routes use an unrecognized registration shape, extend the scanner and bind diff --git a/crates/buzz-test-client/tests/community_ban_routes.rs b/crates/buzz-test-client/tests/community_ban_routes.rs index 762a6df0ac..3e9424ab07 100644 --- a/crates/buzz-test-client/tests/community_ban_routes.rs +++ b/crates/buzz-test-client/tests/community_ban_routes.rs @@ -2,8 +2,8 @@ //! //! The route inventory is shared with `scripts/check-community-ban-route-inventory.py`. //! This ignored test uses a disposable local relay/PostgreSQL/MinIO stack and -//! fresh tenant hosts and identities. GIF requests stop at malformed local -//! input before any provider request; Git and workflow requests use absent +//! fresh tenant hosts and identities. Run without BUZZ_KLIPY_API_KEY so GIF +//! controls assert the disabled-provider response; Git and workflow requests use absent //! resources and must stop before subprocess or durable work. //! //! ```text @@ -252,7 +252,7 @@ fn route_path(row: &RouteRow, case: &str, actor: &Principal) -> String { fn request_body(case: &str, actor: &Principal) -> Vec { match case { - "events" => EventBuilder::new(Kind::TextNote, "community-ban route matrix") + "events" => EventBuilder::new(Kind::Metadata, "{}") .sign_with_keys(&actor.keys) .expect("sign test message") .as_json() @@ -274,7 +274,9 @@ fn request_body(case: &str, actor: &Principal) -> Vec { "gif_search" | "gif_share" | "profile_search" | "invite_mint" | "buzz_read_state" => { b"{".to_vec() } - "media_upload" => b"not an image file".to_vec(), + "media_upload" => vec![ + 0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10, b'J', b'F', b'I', b'F', 0, + ], "git_upload_pack" | "git_receive_pack" => Vec::new(), "git_default_post" => serde_json::to_vec(&serde_json::json!({ "branch": "main", @@ -444,28 +446,78 @@ fn assert_ban_denial( } fn assert_allowed_control(response_status: StatusCode, body: &str, route: &RouteRow, case: &str) { - assert!( - matches!(response_status.as_u16(), 200..=299 | 400 | 404 | 409 | 415 | 422), - "{} {} case {case}: unbanned control must pass authentication and authorization, then reach the fixture's expected success or input/resource result; got {}: {body}", - route.method, - route.path, - response_status - ); - let denial_message = if route.id.starts_with("git_default_branch") { - BANNED_MESSAGE - } else if route.id.starts_with("git_") { - "blocked: banned from this community" - } else if route.id.starts_with("media_") { - "relay membership required" - } else { - BANNED_MESSAGE + let expected_status = match case { + "events" + | "moderation_command" + | "query" + | "count" + | "buzz_sidebar" + | "moderation_reports" + | "moderation_owner_ban" + | "moderation_audit" + | "moderation_restricted" => StatusCode::OK, + "profile_search" | "invite_mint" | "buzz_read_state" => StatusCode::BAD_REQUEST, + // This isolated fixture runs without a GIF provider key. + "gif_search" | "gif_share" | "workflow_runs" | "workflow_approvals" | "media_get" + | "media_head" | "git_info_refs" | "git_upload_pack" | "git_receive_pack" + | "git_default_get" | "git_default_post" => StatusCode::NOT_FOUND, + "media_upload" => StatusCode::UNPROCESSABLE_ENTITY, + _ => panic!("no positive-control contract for {case}"), }; - assert!( - !body.contains(denial_message), - "{} {} case {case}: allowed control must not receive a ban denial; body: {body}", - route.method, - route.path + assert_eq!( + response_status, expected_status, + "{} {} case {case}: positive control reached an unexpected result: {body}", + route.method, route.path ); + + if case == "media_head" { + assert!(body.is_empty(), "HEAD must suppress the missing-blob body"); + return; + } + if matches!( + case, + "git_info_refs" | "git_upload_pack" | "git_receive_pack" + ) { + assert_eq!(body, "repository not found"); + return; + } + let value: serde_json::Value = serde_json::from_str(body) + .unwrap_or_else(|error| panic!("case {case}: expected JSON, got {body}: {error}")); + match case { + "events" | "moderation_command" => { + assert_eq!(value["accepted"], true, "case {case}: {body}"); + assert!(value["event_id"].as_str().is_some_and(|id| id.len() == 64)); + } + "query" + | "moderation_reports" + | "moderation_owner_ban" + | "moderation_audit" + | "moderation_restricted" => assert!(value.is_array(), "case {case}: {body}"), + "count" => assert!(value["count"].as_u64().is_some(), "count response: {body}"), + "buzz_sidebar" => { + assert!(value["channels"].is_array(), "sidebar response: {body}"); + assert!(value.get("next_cursor").is_some()); + } + "buzz_read_state" => { + assert_eq!(value["error"]["code"], "invalid_request"); + assert!(value["error"]["request_id"].as_str().is_some()); + } + _ => { + let expected_error = match case { + "profile_search" => "invalid profile search JSON", + "invite_mint" => { + "invalid invite JSON: EOF while parsing an object at line 1 column 1" + } + "gif_search" | "gif_share" => "GIF search is not configured", + "workflow_runs" | "workflow_approvals" => "workflow not found", + "media_get" => "not found", + "media_upload" => "invalid image data", + "git_default_get" | "git_default_post" => "repository not found", + _ => unreachable!(), + }; + assert_eq!(value["error"], expected_error, "case {case}: {body}"); + } + } } fn principal<'a>(name: &str, principals: &[&'a Principal]) -> &'a Principal { @@ -538,6 +590,19 @@ async fn community_http_ban_matrix() { keys: Keys::generate(), nip_oa_tag: None, }; + let clear_member_agent_keys = Keys::generate(); + let clear_member_agent = Principal { + name: "clear-member-agent", + nip_oa_tag: Some( + buzz_sdk::nip_oa::compute_auth_tag( + &clear_member.keys, + &clear_member_agent_keys.public_key(), + "", + ) + .expect("sign clear ordinary-owner agent proof"), + ), + keys: clear_member_agent_keys, + }; let clear_admin = Principal { name: "admin", keys: Keys::generate(), @@ -574,6 +639,7 @@ async fn community_http_ban_matrix() { let banned = [&banned_member, &banned_admin, &banned_agent]; let controls = [ &clear_member, + &clear_member_agent, &clear_admin, &clear_owner, &clear_moderator_agent, @@ -600,11 +666,18 @@ async fn community_http_ban_matrix() { "moderation_owner_ban" => "clear-moderator-agent", _ => route.control.as_str(), }; - let control = principal(control_name, &controls); - let response = send_route_request(&client, &route, case, control, &host).await; - let status = response.status(); - let body = response.text().await.unwrap_or_default(); - assert_allowed_control(status, &body, &route, case); + let control_names = if control_name == "member" { + vec!["member", "clear-member-agent"] + } else { + vec![control_name] + }; + for name in control_names { + let control = principal(name, &controls); + let response = send_route_request(&client, &route, case, control, &host).await; + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + assert_allowed_control(status, &body, &route, case); + } } } } diff --git a/crates/buzz-test-client/tests/fixtures/community-ban-regressions.tsv b/crates/buzz-test-client/tests/fixtures/community-ban-regressions.tsv index 8422f392bf..5385e8ade7 100644 --- a/crates/buzz-test-client/tests/fixtures/community-ban-regressions.tsv +++ b/crates/buzz-test-client/tests/fixtures/community-ban-regressions.tsv @@ -18,3 +18,7 @@ ingest_owner_ban crates/buzz-relay/src/handlers/ingest.rs owner_ban_blocks_every moderation_owner_ban crates/buzz-relay/src/api/bridge.rs moderation_read_honors_fresh_and_stored_banned_agent_owners postgres-ci moderation_owner_ban_auth_time_and_stored_link timeout_admin_denial crates/buzz-test-client/tests/regression_relay_admin_ban_gate.rs banned_and_timed_out_admins_are_refused_but_unrestricted_admin_can_administer relay-e2e timed_out_admin_relay_admin_denial http_tenant_isolation crates/buzz-test-client/tests/community_ban_routes.rs ban_is_scoped_to_host_resolved_community relay-e2e http_ban_tenant_isolation +invite_restricted_claim crates/buzz-relay/src/api/invites/postgres_tests/restriction_tests.rs signed_nonmember_claims_reject_restrictions_without_consuming_invite postgres-ci restricted_nonmember_claim_never_consumes_invite +workflow_owner_ban crates/buzz-relay/src/api/bridge/workflow_webhook_postgres_tests.rs webhook_admission_enforces_owner_ban_tenant_scope_and_lookup_failure postgres-ci webhook_owner_ban_tenant_scope_and_fail_closed_lookup +durable_live_revalidation crates/buzz-relay/src/state.rs missed_member_owner_ban_and_first_owner_link_revoke_root_audio_sessions postgres-ci missed_fanout_member_and_owner_revocation +token_owner_revalidation crates/buzz-relay/src/state.rs token_owner_admission_survives_scan_with_nip_oa_disabled_until_owner_removal postgres-ci nip_fi_owner_admission_matches_live_scan 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 75f7ae6bf4..93764a228e 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 @@ -29,12 +29,12 @@ join_policy relay GET /api/join-policy public_metadata not_applicable not_applic join_policy_terms relay GET /api/join-policy/terms public_metadata not_applicable not_applicable:public_metadata classified - join_policy_privacy relay GET /api/join-policy/privacy public_metadata not_applicable not_applicable:public_metadata classified - invite_accept_policy relay POST /api/invites/accept-policy pre_membership_anonymous exempt not_applicable:no_authenticated_principal classified - -invite_claim relay POST /api/invites/claim pre_membership_nip98 exempt enforce pending:BUZZ-268 BUZZ-268 +invite_claim relay POST /api/invites/claim pre_membership_nip98 exempt enforce pg:signed_nonmember_claims_reject_restrictions_without_consuming_invite - nip_fi_disconnect relay POST /api/nip-fi/disconnect internal_nip_fi not_applicable not_applicable:internal_auth_plane classified - moderation_reports relay GET /moderation/reports community_nip98 community enforce matrix:moderation_reports+moderation_owner_ban admin moderation_audit relay GET /moderation/audit community_nip98 community enforce matrix:moderation_audit+moderation_owner_ban admin moderation_restricted relay GET /moderation/restricted community_nip98 community enforce matrix:moderation_restricted+moderation_owner_ban admin -workflow_webhook relay POST /hooks/{id} workflow_webhook_secret not_applicable enforce pending:BUZZ-272 BUZZ-272 +workflow_webhook relay POST /hooks/{id} workflow_webhook_secret not_applicable enforce pg:webhook_admission_enforces_owner_ban_tenant_scope_and_lookup_failure - mesh_demo_echo relay POST /_mesh/demo/echo testbed_internal not_applicable not_applicable:testbed_only classified - huddle_audio relay GET /huddle/{channel_id}/audio community_nip42 community enforce pg:audio_join_rejects_community_banned_member+audio_join_rejects_banned_admin+audio_join_rejects_banned_owner_agent - media_upload relay PUT /upload community_blossom community enforce matrix:media_upload member diff --git a/scripts/check-community-ban-route-inventory.py b/scripts/check-community-ban-route-inventory.py index b12b93732c..ff1f280841 100644 --- a/scripts/check-community-ban-route-inventory.py +++ b/scripts/check-community-ban-route-inventory.py @@ -348,10 +348,17 @@ def builder_prefixes(builders) -> dict[str, str]: return prefixes -# 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. +# Service, fallback, and merge registrations bypass per-route classification. +# Pin the existing composition seams by builder and exact normalized argument; +# reject new or repeated compositions until their routes can be classified. ALLOWED_UNROUTED_REGISTRATIONS = { + # These root aliases are constructed in build_router at the root prefix. + ("router::build_router", "merge", "media_router"), + ("router::build_router", "merge", "git_router"), + ("router::build_router", "merge", "git_policy_router"), + ("router::build_router", "merge", "optional"), + # Git settings already carry their complete /git paths; no nest prefix. + ("api::git::transport::git_router", "merge", "super::settings::router()"), # 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. @@ -381,6 +388,11 @@ def collect_registered_routes() -> Counter: mask = mask_comments_and_strings(source) test_ranges = test_module_ranges(mask) path_builders = [builder for builder in builders if builder[2] == path] + # Root aliases have root paths only when merged outside a nest target. + nest_arguments = [] + for nest in re.finditer(r"\.\s*nest\s*\(", mask): + opening = mask.find("(", nest.start(), nest.end()) + nest_arguments.append((opening, matching_delimiter(mask, opening))) def enclosing_builder(position: int) -> str | None: for builder in path_builders: @@ -389,7 +401,7 @@ def collect_registered_routes() -> Counter: return None for match in re.finditer( - r"\.\s*(route|nest|route_service|nest_service|fallback_service|fallback)\s*\(", + r"\.\s*(route|nest|route_service|nest_service|fallback_service|fallback|merge)\s*\(", mask, ): if in_ranges(match.start(), test_ranges): @@ -398,13 +410,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") or call == "fallback": + if call.endswith("_service") or call in {"fallback", "merge"}: 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: + if ( + call == "merge" + and builder == "router::build_router" + and in_ranges(match.start(), nest_arguments) + ): + errors.append( + f"{location}: root .merge registration inside a .nest target " + "is not classified" + ) + elif 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" @@ -551,7 +572,6 @@ def check_route_inventory() -> tuple[int, int, int]: found = any( test_function_exists(path, test_name) for path in RELAY_SRC.rglob("*.rs") - if "tests" not in path.parts and not path.name.endswith("_tests.rs") ) if not found: raise ValueError(f"{route_id}: missing PostgreSQL regression {test_name!r}") @@ -573,10 +593,8 @@ def check_route_inventory() -> tuple[int, int, int]: details.append("inventory rows without registered routes: " + ", ".join(map(str, stale))) raise ValueError("; ".join(details)) - if set(pending) != {("invite_claim", "pending:BUZZ-268"), ("workflow_webhook", "pending:BUZZ-272")}: - raise ValueError( - "pending coverage must explicitly track BUZZ-268 invitations and BUZZ-272 webhook owner bans" - ) + if pending: + raise ValueError(f"integrated ban coverage cannot retain pending sibling cases: {pending}") claim = next((row for row in routes if row["id"] == "invite_claim"), None) if claim is None or claim["membership"] != "exempt" or claim["restriction"] != "enforce": raise ValueError("invite claim must remain membership-exempt and restriction-enforced") diff --git a/scripts/check-community-ban-route-inventory.test.py b/scripts/check-community-ban-route-inventory.test.py index a554f98f22..d0e4bcc1d4 100644 --- a/scripts/check-community-ban-route-inventory.test.py +++ b/scripts/check-community-ban-route-inventory.test.py @@ -68,6 +68,41 @@ class RouteScannerFixtures(unittest.TestCase): with self.assertRaisesRegex(ValueError, message): self.scan(router, admin) + def test_nested_builder_merge_is_rejected(self): + self.assert_rejected( + r"\.merge registration is not classified", + build_router(), + ADMIN.replace('get(probe))', 'get(probe)).merge(crate::api::profiles::router(state))'), + ) + + def test_unclassified_root_merge_is_rejected(self): + self.assert_rejected( + r"\.merge registration is not classified", + build_router(".merge(api::admin::router(state.clone()))"), + ) + + def test_existing_root_merge_alias_is_allowed_once(self): + self.scan(build_router(".merge(media_router)")) + self.assert_rejected( + r"allow-listed \.merge registration appears more than once", + build_router(".merge(media_router).merge(media_router)"), + ) + + def test_root_merge_alias_moved_inside_nest_is_rejected(self): + self.assert_rejected( + r"root \.merge registration inside a \.nest target is not classified", + build_router().replace( + 'api::admin::router(state.clone()))', + 'api::admin::router(state.clone()).merge(git_policy_router))', + ), + ) + + def test_merge_outside_router_builder_is_rejected(self): + self.assert_rejected( + r"\.merge registration is not classified", + build_router() + "\nfn hidden() { opaque.merge(private_router); }", + ) + def test_same_named_constants_resolve_per_module(self): routes = self.scan(build_router()) self.assertEqual(