[bradseiler]

Summary

Classify all 79 registered routes (75 relay, 4 health) and bind community-ban checks to signed requests with exact allowed controls. New or opaque router composition fails the inventory guard, including unclassified .merge(...) calls, nested Git settings merges, and partially classified method chains such as get(...).on(...) or get(...).post_service(...). Known method chains and .with_state, .layer, and .route_layer modifiers remain supported.

Replace broad positive-status allowances with case-specific statuses and response bodies or shapes. Ordinary-member routes also exercise a clear owner-delegated agent. GIF controls require no provider key. Media reads, Git reads and workflows use absent resources; malformed JPEG upload and an invalid Git-push repository name assert their exact validation errors.

The moderation-read gate now uses the signed NIP-98 creation time for owner-proof validation. Main now reads relay-signed discovery metadata and admin inputs behind the replacement lock and retains it through storage (#9035). This layer preserves that implementation and its stale-publisher regression, replacing the older optimistic retry scheme. Integrated invitation, workflow and live-session regressions replace stale pending-sibling rows. The inventory requires zero pending cases.

The moderation-read fix ships with the relay without a feature flag. The discovery lock implementation already ships on main. Verify current discovery metadata and rosters under concurrent edits and owner-linked ban denials; rollback of the moderation-read fix is a revert of this layer.

Related issue

BUZZ-269. Stack: #8529 → #8241 → #8239 → #8235. #8237 and #8242 are merged. Shared preparation fixes already on main were dropped from this layer.

Testing

Current head 47e15f7a465976c2cd3fea3caf9199c2ebbb9527 in native stack 8725, rebased by Brad through GitHub onto main 44b3c4823. Brad owns further ref updates.

POST /operator/communities/invites is classified as operator control with conditional membership and enforced actor restrictions. The bound PostgreSQL journey permits relay owners/admins and denies a banned admin, members and outsiders as appropriate.

The inventory passes with 80 routes, 24 HTTP behavior cases, 24 supporting regressions and zero pending cases. All 23 scanner fixtures pass. Independent review clears the classification and preserves all 21 carried patches in range-diff. Removing the route row or renaming its bound test made the guard fail on the unchanged earlier candidate.

The earlier video range failure was reproduced as a shared-blob fixture race on both the earlier stack and pinned main. Fix that separately with unique MP4 blobs and review from the enforce-mode owner. That historical diagnosis is not a current-head runtime result.

Normal pre-push hooks passed on integrated head 7ee3cf92f59a96349099d48fc115e5b8bb6a9716, including the branch-skew guard, unit tests, desktop checks/tests, Tauri checks, mobile checks/tests and file-size checks. These hooks validate the checked-out top tree; hosted CI remains the gate for each individual PR head.

Current-head hosted CI is terminal and green, including runtime unit and PostgreSQL jobs. McKeon independently cleared all four current heads: carried patches range-diff unchanged (8/20/21/6), with no overlap between the newer main delta and stack files. The separate main-only security-review authorization check remains failed for this stacked base; no bypass or retarget was performed.

Historical local validation at integrated head 7ee3cf92f5: buzz-db/buzz-relay clippy passed with all targets and warnings denied; package tests passed 2,035/2,035; desktop build, Tauri check/tests, web build and mobile tests passed. The full PostgreSQL lane finished 1,503/1,505, with two failures still unresolved in that run: cluster_global_sample_writer_fails_closed_when_activity_is_masked and hanging_redis_peer_times_out_before_the_health_listener_binds. The runner constructed a postgresql:// URI; the fence test requires literal postgres://, explaining its prefix panic. The Redis test exceeded its scheduling bound at 8.03 seconds; that local failure remains unexplained. No full local just ci pass is claimed. These receipts remain attributed to the historical head.

The separate security-review workflow only authorizes PRs targeting main. Do not retarget or retry this stacked PR to bypass that restriction.

Human review and verification remain required. Existing review decisions are preserved. No merge, deployment, production repair or human acceptance was performed.

Generated with Codex

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is fae3f0346c44bbdf9629a7b5967965391c2bd22f...69edf3220e5a60853355d39608d75321e29025f9.
A new review must complete for this exact range. When manual authorization
is required, a user with write access must comment exactly
@buzz-security-review 69edf3220e5a60853355d39608d75321e29025f9 to authorize a new review.
Any previous review applies only to its recorded range.

[TheSentinel454]

authorize_moderation_read (api/bridge.rs:~2736) now passes signed_created_at where it used to pass None. Previously, a moderator agent whose owner was freshly banned via NIP-OA could still read the moderation queue. That's a security fix, and moderation_read_honors_fresh_and_stored_banned_agent_owners pins it well. Could it be named in the title and summary, or split into its own PR so it can be reverted or backported on its own?

[wpfleger96]

🤖 Thanks for this. Two blocking problems, both in what the new coverage proves rather than in production behavior: the inventory guard misses a supported router shape, and the positive controls pass when the allowed operation is broken. Both were reproduced by mutation at this head.

Blocking

1. A second direct mount of the same router escapes the inventory (correctness). check-community-ban-route-inventory.py L286–304 keeps one prefix per router builder (prefixes[candidates[0]] = prefix), and L340–363 emits that builder's routes only under that one prefix. Adding .nest("/extra", api::buzz_v1::router(state.clone())) before the existing /buzz/v1 mount exposes /extra/me/sidebar and /extra/me/read-state on the relay, but the guard still exits 0 and reports 74 routes. An ordinary added .route(...) does fail it, as intended. This is the direct, literal .nest shape TESTING.md says is supported, not one of the listed gaps (route_service, macros, opaque composition), so new endpoints can skip both classification and the signed route matrix.

Fix: collect every mount prefix per builder (or fail closed when a builder is mounted more than once), and add a guard regression with two direct mounts of the same builder.

2. Positive controls accept unrelated failures (correctness). assert_allowed_control in community_ban_routes.rs L443–465 accepts any 2xx, 400, 404, 409, 415, or 422 for every control case and only checks that the body isn't a ban denial. Keeping the /count membership/ban gate but returning 404 immediately after it leaves all three new tests passing (community_http_ban_matrix, banned_root_websocket_is_refused, ban_is_scoped_to_host_resolved_community). Removing the gate instead does fail the matrix (/count returns 200 {"count":1} to the banned member), so the ban assertions are sound. The problem is that the unbanned side doesn't prove the operation still works, and moderation reads with valid inputs could likewise return 400/404 and pass.

Fix: give each control case its expected status and response: 200 plus response shape for valid reads and submissions (e.g. /count returns a numeric count), and the exact validation or missing-resource response for fixtures that are deliberately malformed or absent. Seed an accessible resource where resource authorization is part of what the case claims.

Scores

• Minimalism 8/10: the PR also carries the mesh receive, scraper, and push-lease fixture fixes that #8237 carries. They look correct; landing them once would keep this diff to the coverage change.
• Elegance 9/10: the route table, guard script, and signed matrix are a clean split, and the public/internal classifications and the two pending invitation/webhook rows are explicit rather than silently skipped.
• Correctness 7/10: the two items above. What holds up: ban denials require 403 plus the route's specific denial, not any 401/404; an empty matrix fails the case-set comparison; the ignored tests run by name in CI (Relay E2E and PostgreSQL jobs) and pass locally. The bridge.rs moderation-read change is a real fix: fresh owner proofs are now evaluated at the verified NIP-98 time, like the other NIP-98 routes, with a PostgreSQL regression. The mesh_boot.rs echo loop keeps one in-flight receive across drain ticks and drops it only on shutdown. The ingest.rs / relay_admin.rs edits are comment-only, and the 200 → 403 timed-out-admin expectation matches the shared gate on main.

Nonblocking

• The banned delegated agent has no matching unbanned-agent control outside the moderation reads. On /buzz/v1/* the denial check only looks for code == "forbidden", on media only for relay membership required, and HEAD is bodyless, so those cases can't tell an owner-ban denial from some other agent-specific denial. A clear owner/agent pair per case would fix that.
• The guard ignores fallback / fallback_service, including the SPA fallback in router.rs. TESTING.md lists route_service / nest_service as known gaps but not this one.
• TESTING.md restates the unmerged invitation contract (72h / 30-day lifetimes, v1 invalid_after), which will drift if #8237 changes.
• The description doesn't mention the moderation-read fix in bridge.rs, and still says "Draft only" while the PR is ready for review.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/source-visible flows) lanes.

Two existing P2 coverage defects remain at this head, independently confirmed from source. They are already described in review 5511682177, so I am linking that feedback rather than duplicating inline comments:

• Preserve every direct router mount. In scripts/check-community-ban-route-inventory.py:286–304, prefixes[candidates[0]] = prefix overwrites an earlier mount; lines 342–363 emit only the retained prefix. A second direct mount of the same builder can therefore expose endpoints absent from the inventory without changing the guard’s result. Collect all prefixes, or fail explicitly on multiple mounts, and cover that supported composition shape with a guard regression.
• Make allowed controls assert the expected outcome. crates/buzz-test-client/tests/community_ban_routes.rs:443–465 accepts any 2xx, 400, 404, 409, 415, or 422 for every control. A valid /count or moderation read returning an unrelated 404 after admission still passes. Assert each case’s expected status and response shape; use exact validation/missing-resource responses for deliberately invalid fixtures, and accessible resources where resource authorization is part of the claim.

The mesh echo retains its in-flight receive across housekeeping ticks, and the moderation-read change supplies the verified NIP-98 timestamp to owner-ban evaluation. No additional actionable defect was established in this review. The explicitly pending invitation/webhook siblings and final integrated zero-pending requirement remain unchanged.

Coverage limitation: the HTTP matrix’s CI relay has membership enforcement off; closed-relay admission is covered by separate targeted regressions, not by that full route matrix.

Minimalism 9/10; elegance 9/10; correctness 8/10 because of the two coverage defects above.

Reviewed head dda7ef8815cf7675b399bcee4512da3fd6ec7945 against base/merge-base 326e2301cb4b1edcb8a72d01ac4b19a83545365f. Source-only: no tests, guard scripts, mutation checks, builds, applications, or live services were executed. A read-only CI snapshot showed 74 successful and 33 skipped checks; GitHub also reports merge conflicts. These checks do not establish runtime, cross-pod, or deployment readiness. Public PR description and discussion were inspected; no attached images or actionable publication disclosure found. This is a non-blocking COMMENT review; humans decide what must change.

[wpfleger96]

🤖 Thanks for the scanner hardening. The repeated-nest gap from my last review is fixed, and the duplicate .nest and .route_service registrations now fail the guard, which I confirmed by adding each one to a copy of this head. Two blockers remain: the positive controls are unchanged, and the guard misses routers composed with .merge.

Blocking

1. Positive controls still accept unrelated failures (correctness). This is unchanged since my last review. assert_allowed_control in community_ban_routes.rs L446–469 still accepts any 2xx, 400, 404, 409, 415, or 422 for every control and only checks that the body isn't a ban denial. A handler that admits the unbanned member and then returns 404 (the /count mutation from the last review) still passes the matrix. The new profile_search case inherits the same check, and its control body is a deliberately malformed {, so it proves only that the request got past auth.

Fix: give each control case its expected status and response: 200 plus the response shape for valid reads and writes (e.g. /count returns a numeric count), and the exact validation or missing-resource response for fixtures that are deliberately malformed or absent.

2. The guard misses router builders composed with .merge (correctness). The registration check at L392 looks at route, nest, the *_service calls, and fallback, but not merge. builder_prefixes (L317–347) only reads .nest in build_router, and L430 emits each builder's routes once, under that builder's nest prefix or "". So a builder merged into a nested router, or merged in more than one place, exposes routes the inventory never sees. To check, I added .merge(crate::api::git::git_policy_router(state.clone())) inside api::buzz_v1::router in a copy of this head. The guard still exits 0 and reports the same 77 routes, even though /buzz/v1/internal/git/policy now has no row. This is the same class of gap as the repeated .nest and the shapes in the earlier inline thread, through the one composition call the scanner doesn't read. The relay already uses it (build_router merges four routers, git_router merges settings::router).

Fix: track .merge(<builder>(...)) the way .nest is tracked, so a merged builder's routes are emitted under the merging router's effective prefix, and reject a builder that is merged or nested more than once. Alternatively, reject .merge of a builder call outside build_router. Add a fixture for each case.

Scores

• Minimalism 8/10: the PR still carries the mesh-receive, boot, scraper, and push-lease fixture fixes shared with #8237, #8241, and #8242. They look right, but landing them once would keep this diff to the coverage change.
• Elegance 9/10: keying constants by module and failing every unclassified shape through one error path is a clear improvement. The allow-listed fallbacks are pinned to their builder and exact argument, which keeps the exemptions honest.
• Correctness 7/10: the two items above. What holds up: the duplicate-nest and route_service mutations both fail the guard (exit 1), all 13 scanner fixtures pass, and the baseline guard reports 77 routes, 24 HTTP cases, 19 supporting regressions, and 2 pending. The two seed commits (6eb83c0e0, 9a2c8704a) only adapt the test seeds to the new add_relay_member signature and MembershipWrite return type.

Nonblocking

• TESTING.md L106–114 still says the scanner doesn't discover .route_service(...) or .nest_service(...), which contradicts L43–47 now that both are hard errors. It also lists "conditional/opaque router composition" as a gap. Once item 2 is fixed, that paragraph should say which composition the guard handles.
• Still open from the last review: the banned delegated agent has no matching unbanned non-moderator agent control, so on /buzz/v1/*, media, and HEAD cases the denial check can't tell an owner-ban denial from some other agent-specific denial.
• Still open from the last review: TESTING.md L78–81 restates the unmerged invitation contract from #8237 (72-hour default, 30-day maximum, invalid_after), which will drift if #8237 changes.
• The description still doesn't mention the moderation-read fix in bridge.rs (fresh owner proofs now evaluated at the verified NIP-98 time). Luke asked about this too. It's a real security fix and should be named, or split out so it can be reverted or backported on its own.

[wpfleger96]

🤖 This head adds three commits on top of the one I reviewed last time (070b77b0). They change only the route-inventory guard, its fixture tests and TESTING.md. The blocker from my last review is fixed, and I found nothing new that blocks.

What changed and what I checked

• The mixed method-chain gap is closed. The new route_methods (L481–512) reads a route's whole method expression one call at a time instead of searching it for helper names. The first call must be a known HTTP helper, optionally written as axum::routing::<helper>. Each later call must be another known helper or .with_state, .layer or .route_layer. Anything else fails the guard, even after a known helper: on, on_service, *_service, fallback, merge, a variable, a wrapper call, or trailing tokens. Handler and middleware arguments are skipped whole, so a post(...) inside a closure doesn't count as a method.
• Masking holds. The expression comes from source with comments, strings, raw strings and char literals already blanked, so a ) inside a raw-string handler argument doesn't break the delimiter match.
• Axum coverage. HTTP_METHODS covers every named method helper in Axum 0.8.9 (the version in the lockfile) plus any. Every route expression the relay registers today still classifies. Other valid spellings such as routing::get(...) or ::axum::routing::get(...) are rejected rather than read, which is the right direction for this guard.
• The new fixtures cover rejection of each mixed shape, preserved supported chains, every HTTP_METHODS entry, and helper-named calls inside arguments.
• The TESTING.md lines from my last review are now wrapped. The description now matches the 79-route inventory and covers the discovery fix.

Runtime verification

At this exact head:

• The guard passes against the real relay source (79 routes, 24 behavior cases, 23 supporting regressions, 0 pending), and all 23 scanner fixtures pass.
• With the production /info registration in router.rs replaced, the guard fails with a classification error for get(h).on(...), get(h).post_service(svc) and routing::get(h).
• get(|| async { r#")"# }) correctly stays GET /info. Adding .post(h) after it fails for the missing POST /info row, and adding .on(...) or .post_service(...) fails classification, so a raw-string ) can't hide a chained method.

CI passes at this head. The Codex security review job fails only because this PR targets its stack branch rather than main.

Scores

• Minimalism 9/10: one small chain walker replaces the permissive search, with no new configuration.
• Elegance 9/10: the narrow fail-closed grammar matches the rest of the guard and is documented in the docstring and TESTING.md.
• Correctness 9/10: both reported shapes and the related unknown APIs fail, and current production routes still pass.

Nonblocking

• TESTING.md says method expressions "must start with a known HTTP helper" but doesn't say the axum::routing:: path is also accepted, or that other path spellings are rejected. One clause would cover it.
• Still open from my last review: delayed_discovery_cannot_restore_stale_metadata signs the stale candidate and the intervening edit at the same timestamp (L1535), so the NIP-16 event-ID tiebreak alone can reject it without exercising the fence. Make the stale candidate strictly newer, and cover a 39001 conflict after 39000 commits.
• Still open from my last review: expected_revision: Option<Option<&[u8]>> in replace_addressable_event_checked (replaceable.rs L437) would read better as a small enum.
• As the description notes, the CI Rust path filter doesn't include the scanner script paths, so a future scanner-only change wouldn't run the guard in CI.