feat(nip-fi): admin disconnect/deny API with in-memory deny-until-TTL map (S4)

[wpfleger96]

Implements S4 of the buzz-enterprise-identity program: the NIP-FI admin disconnect API with deny-until-TTL semantics.

Merge order: This branch carries a pre-#7224 S3 snapshot (merge-base 88687876f) and merges after #7224; base reconciliation follows.

What this adds

buzz-auth

crates/buzz-auth/src/nip_fi/deny_map.rs — NipFiDenyMap

In-memory deny set, no persistence (Option B, relay restart amnesia is accepted and documented as issuer-re-push).

• Per-issuer DashMap shards — cross-issuer capacity starvation is impossible
• Merge rule: max(existing_until, incoming_until) on same-key collision — a delayed shorter-until command never shortens an active deny [FI-TRACE-DENY-SET]
• Past-until commands: never create or shorten entries; atomically calls the session-close path but skips the deny-entry write [FI-TRACE-DENY-SET]
• Per-issuer hard capacity cap, fail-closed (DenySetFull) — spec requires 503 and the jti is not consumed on full [VerifyCommandJwt step 7]
• Self-evicting TTL (lazy eviction on read and write)
• Atomic jti reservation + deny-entry insertion in one shard lock (both-or-neither) [VerifyCommandJwt step 7]
• is_denied(issuer, pubkey, now) — the clean interface S5 (HTTP enforcement) consumes

crates/buzz-auth/src/nip_fi/command.rs — CommandVerifier<S>

Verifies typ=nip-fi-command+jwt tokens against the same issuer JWKS as assertions.

• Validates method, path, target_pubkey, aud, iat, exp, jti, and body hash claims
• CommandIssuerPolicy carries maximum_command_age_seconds (normative ≤ 60), authorized_principals (sub allowlist), and per-issuer deny_set_capacity
• jti reserved at the final admission step — after authz check + body-match — closing the DoS seam where early reservation would let a crafted rejected command consume jti slots [VerifyCommandJwt step 7]
• Reuses the same parsing/validation primitives as the assertion verifier via pub(super) helper promotion — no crypto duplication

buzz-relay

crates/buzz-relay/src/api/nip_fi.rs — POST /api/nip-fi/disconnect

• Extracts command JWT from Nostr-Federated-Identity: Bearer <token> header
• Validates JSON body pubkey (lowercase hex, exactly 32 bytes)
• Delegates to CommandVerifier::verify — jti + deny entry written atomically on success
• Closes all live sessions for the target pubkey via ConnectionManager::disconnect_nip_fi
• Response contract: 200 {"disconnected": true} on success; 400/401/403/503 per the spec rejection table
• Endpoint is not a protected HTTP surface — no NIP-98, no NIP-FI assertion; auth is entirely inside CommandVerifier::verify
• Cross-pod propagation failures increment buzz_nip_fi_disconnect_propagation_failures_total (no iss/pubkey in labels [FI-TRACE-PRIVACY-NONPUBLIC])
• build_nip_fi_command_components — startup builder callable from main.rs

crates/buzz-relay/src/state.rs

• ConnectionManager::disconnect_nip_fi — issuer-global (unfenced) cross-community session close, sends NOTICE before cancel; sets AuthorizationDenied via first-writer-wins publish_disconnect_reason (writes only when slot is None) so the send loop's cancel branch emits a 1008 POLICY close frame with reason "authorization denied" [spec: deny applies across all communities under the issuer]
• CommunityConnectionControl::publish_disconnect_reason — atomic first-writer-wins helper; all writers (disconnect_community, disconnect_nip_fi, the expiry task, key-pairing) route through it to prevent concurrent CommunityDeleted / AuthorizationDenied causes from misattributing the close frame
• AppState::nip_fi_deny_map and nip_fi_command_verifier fields — None-initialized; endpoint returns 503 until startup wires them in

crates/buzz-relay/src/router.rs + src/api/mod.rs

• Route wired: POST /api/nip-fi/disconnect sits outside all NIP-98 middleware layers

crates/buzz-relay/src/handlers/auth.rs + audio/handler.rs

• is_denied checked at WS admission after pubkey registration (spec steps 5+6 ordering): root WS (handlers/auth.rs:346), audio (audio/handler.rs:349), and the pre-upgrade early-bounce path — authorization_denied frame + explicit 1008 POLICY close frame + close on match; NIP-FI expiry sends the same policy close on all paths including the audio pre-send-loop window (check_cancel!() arms and JoinCommitError::Expired exit)

crates/buzz-relay/src/main.rs

• install_nip_fi_command_components wired at startup (main.rs:543) with shared-Arc JWKS source

Test coverage

20 new tests in nip_fi::deny_map and 26 new tests in nip_fi::command:

• Both command delivery orders → max(until) [FI-TRACE-DENY-SET oracle]
• Merge rule both directions
• Past-until commands: absent entry inserts expired; active entry left unchanged
• Capacity exhaustion: fail-closed, nothing inserted, jti not consumed
• jti replay rejection
• Cross-issuer capacity isolation
• CommandIssuerPolicy construction validation (zero age, >60 age, empty principals, zero capacity, empty issuer)

22 unit tests in api::nip_fi: header extraction contract (401/403), pubkey parsing (uppercase rejected, wrong length). 4 additional ConnectionManager/CommunityConnectionControl tests: conn_manager_disconnect_nip_fi_sets_authorization_denied_reason, conn_manager_disconnect_nip_fi_ignores_unproven_connection, community_disconnect_then_nip_fi_keeps_community_deleted_reason (first-writer-wins: CommunityDeleted not clobbered), and nip_fi_disconnect_then_community_keeps_authorization_denied_reason (first-writer-wins: AuthorizationDenied not clobbered). W_FIX1: pre-send-loop drain emits restricted JSON then 1008 POLICY close.

All spec oracle cases are falsifiable: a mutation that violates the merge rule, the past-until invariant, or the capacity semantics will break the corresponding test.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head 2a42ddff049aaaf4bcfdf994680c0b1b4e2785d7 against base ac5a18697c8294e9237f505a2e01ec6fc374849a, source-only. No PR code was checked out, built, tested, or executed.

The review preserves the accepted RAM-only/restart-amnesia design. Startup wiring, WS admission deny checks, and S5 HTTP enforcement are explicitly deferred, not blockers by themselves. The issues below are in the delivered callable components; main.rs currently leaves the endpoint unconfigured.

P2: Close existing huddle sockets as well as Nostr sockets

Handler close call / new close scan.

The success path scans only ConnectionManager. Huddle audio sockets instead register with community_connections, have their own cancellation token, and prove their key through a separate NIP-42 handshake (audio handler, proof). They never enter this scan.

With a configured verifier, a target already connected to chat and a huddle receives a successful disconnect, loses the chat socket, but retains the independently authenticated audio socket and its audio access. NIP-FI's delivered disconnect contract closes all live WebSockets whose proven key matches (spec). This is not the deferred new-admission check. Include every existing key-proven socket type in targeted cancellation, and cover a target with simultaneous Nostr/audio sockets plus an unaffected other key.

P2: Preserve fractional NumericDate deadlines

command.rs:448–466.

The new parser floors every fractional NumericDate and reconstructs it with zero nanoseconds. For an otherwise valid signed command with until = T + 0.9, the stored deadline becomes T, so the deny map reports the key allowed at T + 0.1, before the issuer's signed deadline. Flooring iat can also admit evidence beyond the allowed future-skew boundary. The existing assertion parser already preserves nanoseconds (verifier.rs:916–952). Reuse that semantics and add signed-command tests through verify_at, asserting denial just before fractional until and expiry at equality, plus future-iat and ceiling boundaries.

P2: Keep issuer identities out of logs

api/nip_fi.rs:144–149 and configuration warnings.

A successful close emits the exact signed iss as caller_iss when debug logging is enabled; invalid configuration emits the issuer URI at warning level. This exports deployment-private identity coordinates to logs, contrary to the explicit NIP-FI privacy contract. Remove the issuer fields, retain fixed reason/count diagnostics, and bind a privacy regression to the actual success/configuration logging paths.

P2: Bound the replay map independently of deny-entry capacity

deny_map.rs:98–126.

Even with issuer deny capacity 1, repeated valid commands updating one active key bypass the entry-capacity guard and retain a new jti string each time. Every mutation then scans that growing replay map under the shard lock. There is no replay-count/byte budget or command rate bound in this path. A buggy or abusive authorized issuer can exceed its intended memory budget and increase lock-held work; this is not an anonymous attack or a measured outage. Large JTIs are bounded only by the total 64-KiB token limit, and accepted clock skew can extend retention to 360 seconds.

Add an explicit per-issuer replay-resource bound checked in the same atomic admission step. Exhaustion must leave both the new deny mutation and JTI reservation unapplied, without evicting unexpired replay identities. Test repeated same-key updates at the budget, concurrent reservations, and reuse of a rejected still-valid JTI after capacity frees.

Validation and smaller compatibility note

The changed tests exercise policy constructors, error helpers, header/pubkey parsing, and sequential deny-map operations. They do not exercise the new command verification or disconnect handler end to end; the referenced command/tests.rs is absent from the complete pinned head tree. The regression cases above need the production seams, not copies of predicates or response constants.

Non-blocking: missing-header 401 is missing WWW-Authenticate: Nostr required by the rejection table; plain_response only adds Content-Type. Existing DenialClass already provides the challenge value.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head f03efb3f472600ba9ab2061a5136d22277a9fbe3 against base ac5a18697c8294e9237f505a2e01ec6fc374849a, source-only. No checkout, build, tests, imports, or PR-code execution. All three independent review lanes returned; findings below were independently reconciled against the source.

The original audio-scan omission, fractional NumericDate parser, explicit issuer-log sites, and unbounded replay-map implementation are corrected. The current source also delivers startup and cross-pod wiring despite the stale PR description. Three defects remain in that delivered integration.

P2: Do not turn remote capacity pressure into permanent issuer-wide denial

deny_map.rs:277–290,341–367, called by main.rs:1215–1258.

Source-derived witness: configure two maps/pods with issuer capacity 1. Before propagation, each accepts a command for a different key. Deliver the remote messages: each new-key merge hits capacity and inserts the issuer into blocked_issuers. Every subsequent is_denied call for that issuer returns true, including unrelated keys and times after both signed TTLs have expired. No expiry or recovery clears this flag; restart is required. A delayed, already-expired remote command against a full shard takes the same path.

This breaks the delivered shared deny interface’s targeted, self-expiring contract. It is not a claim of an already-wired admission outage: WS/S5 admission consumers remain deferred. The spec explicitly permits asynchronous propagation loss with issuer re-push; it does not require permanent denial of unrelated users. Remove the sticky issuer-wide transition on ordinary capacity exhaustion, retain existing live entries and observable recovery, and preserve target session closure for past-until delivery without creating future denial. Do not add persistence. Replace the test that codifies deny-all (deny_map.rs:831–879) with the two-map/capacity, unrelated-key, and post-TTL cases. Poisoned-lock handling is separate from normal capacity pressure.

P2: Sanitize configuration parse errors before they enter logs

nip_fi_config.rs:159–162 forwards the raw serde error into ConfigError; main.rs:163–165 logs that error.

An otherwise complete issuer entry containing "authorized_principals": "admin@private.example" instead of an array produces a type-error diagnostic containing the supplied principal string. This happens before the new index-only policy validation. Startup fails closed, but private principal/email data is exported to logs, violating NIP-FI’s explicit privacy contract. Keep a fixed error category and safe location information rather than {e}. Add a malformed-sensitive-value regression through the actual configuration/error-reporting path. The old explicit issuer log sites are fixed; this is a newly introduced path.

P2: Isolate the new route fixtures from process-environment mutation

api/nip_fi.rs:634–640 and the absent-verifier fixture at line 742 call Config::from_env().expect(...). That now reads NIP-FI environment configuration. In the same test binary, nip_fi_config.rs:399–462 sets the mode to permissive, or to enforce without issuers, under a mutex private to that module.

If a route fixture reads during either interval, configuration returns an error and the fixture panics before reaching the route under test. The private mutex does not protect those readers. Construct fixture configuration without process-env reads, or coordinate every relevant reader/writer using shared synchronization. Do not rely on serial execution of this test module. This is a source-derived parallel-test failure, not a claimed local reproduction.

Regression coverage and stable exit criteria

Keep the real signed-verifier and real-router tests; they are a substantial improvement. Finish the already-requested corrective witnesses: signed verify_at fractional until just-before/equality, future-iat and ceiling boundaries; an actual audio-handler registration plus target chat/audio and unaffected-peer disconnect; and privacy-path capture. Existing registry tests manually populate the key, so removing audio/handler.rs:252 survives them. Integer-only command fixtures do not catch reintroducing timestamp flooring. The “production assembly” test calls only the component builder; its comment at api/nip_fi.rs:990–995 incorrectly claims deleting main’s state assignments makes it fail. Correct that claim or bind the actual assembly seam. These are source-inspected coverage limits, not test-run results.

Exit criteria are the three bounded defects above and regression witnesses for the corrective paths. Preserve RAM-only/restart amnesia, asynchronous propagation/re-push, and the separately deferred WS admission/S5 HTTP work. No broader session-lifecycle rewrite, distributed completion guarantee, or persistence requirement is added. The missing-header WWW-Authenticate: Nostr note is also resolved.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head b6f5ba30de70c06f581fc9201c090726210d9679 against base ac5a18697c8294e9237f505a2e01ec6fc374849a, focused on the two corrective commits since f03efb3f472600ba9ab2061a5136d22277a9fbe3 and the prior exit criteria. Source/metadata review only: no checkout, build, tests, imports, or execution of PR code.

The same three P2 findings remain. Moving the blocked flag under the shard mutex improves serialization, but preserves the incorrect permanent issuer-wide denial. The new publisher/consumer and startup seams improve testability; they do not close the policy/privacy/fixture findings below.

1. P2: Ordinary remote capacity exhaustion still permanently denies unrelated keys

deny_map.rs:401–409, is_denied:316–327, consumer:317–343

Source-derived witness: two pods each have issuer capacity 1 and independently accept different target keys before propagation. Deliver the remote entries: the new-key capacity miss sets IssuerShard.blocked = true. is_denied_or_blocked returns that bit for every key (deny_map.rs:115–119), and TTL eviction never clears it (101–105). Both unrelated keys and the missed target remain denied even after all signed until values expire. A delayed, already-expired remote entry against a full shard causes the same transition.

This is the same targeted/self-expiring shared-interface defect as before, not a claimed live outage in the deferred WS/S5 admission consumers. The contract permits asynchronous loss and issuer re-push; it does not authorize permanent denial of unrelated users. Holding the flag under a mutex makes the wrong state transition atomic, not correct.

On ordinary CapacityExceeded, retain existing live entries, return the capacity outcome, close only the delivered target’s sessions through the consumer, and leave recovery to the accepted re-push policy. Remove the sticky issuer-wide transition; poisoned-lock handling can remain separately fail-closed. Replace the tests that require unrelated-key denial (deny_map.rs:889–938,1099–1141; api/nip_fi.rs:1241–1354) with two-map capacity, unrelated-key, and post-TTL cases. No persistence or distributed completion guarantee is requested.

2. P2: Configuration parsing still exports private principal values to startup logs

nip_fi_config.rs:159–162, main.rs:163–165

An otherwise complete issuer entry with "authorized_principals": "admin@private.example" instead of an array still produces a serde type error containing the supplied string. The parser interpolates that raw error into ConfigError; startup logs it verbatim. This happens before the new index-only missing-command-field validation. Startup rejects the configuration, but leaks the principal/email in doing so, contrary to the explicit privacy contract.

Keep a fixed error category and safe location information instead of raw {e}. Add a malformed-sensitive-value regression through the actual configuration/error-reporting path. The new missing-command-fields test does not cover this failure mode.

3. P2: Existing and new route fixtures still race process-environment writers

api/nip_fi.rs:895–902, new consumer fixture:1261, nip_fi_config.rs:459–477

The route fixtures still call Config::from_env().expect(...); the corrective commits add more copies at api/nip_fi.rs:1261,1378,1513 alongside 899,1003. That loader reads NIP-FI config (config.rs:1266). In the same test binary, NIP-FI tests set BUZZ_NIP_FI_MODE=permissive, or Enforce with issuers absent, under a mutex private to that module (nip_fi_config.rs:413). The route readers do not take it. A concurrent read returns a configuration error and panics before the intended route/consumer/startup assertion.

Construct fixture configuration without process-environment reads, or coordinate all relevant readers/writers with shared synchronization. Overriding DB/Redis fields after loading does not fix the fallible ambient read. This is a source-derived parallel-test failure, not a claimed local reproduction.

Corrective coverage and scope

The new fractional_deadline_survives_publisher_wire_and_consumer_equality_boundary test (api/nip_fi.rs:1371–1490) exercises the real publisher mapping, encode/decode, apply helper and map equality comparison. Credit that wire-path regression. It starts from a synthetic CommandResult, so it does not close the previously requested signed fractional-NumericDate verify_at boundary witness. Integer future-iat and until ceiling cases already exist; the remaining gap is fractional parsing and boundary preservation on the signed path. command.rs is unchanged from the previous reviewed head.

The replacement installer test (api/nip_fi.rs:1508–1613) now invokes the production helper that owns both AppState assignments and verifies that command verification writes to the same map. The former builder-only assembly criticism is resolved. It seeds the key snapshot and checks the warmup result; the final shutdown store is not an observed refresh-task exit, so do not claim that lifecycle was tested.

The actual audio-handler registration plus targeted chat/audio and unaffected-peer witness remains uncovered in the inspected tests. The registry tests manually set the proven key (state.rs:2650–2742), so they bypass the production registration at audio/handler.rs:250–252; the new consumer tests assert map state without live sessions on both transports. Finish the previously requested registration-to-dual-transport-close witness. This is a carried-forward coverage gap, not a new production regression. All three independent lanes have returned and been reconciled. No runtime or mutation-test results are claimed here.

Stable exit criteria remain these three bounded defects and the previously requested corrective witnesses. Preserve RAM-only restart amnesia, asynchronous propagation/issuer re-push, and separately deferred WS admission/S5 HTTP. No broad session-lifecycle rewrite or additional persistence requirement is added. Existing HTTP status/body classes, pubkey-targeted close behavior, and command/JWKS verification contracts were traced through the changed wiring; unchanged cryptographic internals were not reopened as a fresh audit.

addressed

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: one unresolved defect and one corrective-test regression

Reviewed 5dfc5fdf0e2ba0b7bfcff605461cd22acc16882a against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, continuing review 5273544749. This is the bounded corrective review of the three intervening commits, not a renewed S3/mesh/HTTP audit.

Credited: the root writer now drains its reserved denial receiver first under one shared deadline on every cancellation exit (connection.rs:745–934); the URL/pool fixtures now use the hermetic constructor while retaining explicit overrides (state.rs:2226–2263). Neither needs redesign.

1. P2: JOIN still publishes the wrong generation wire key (existing F3)

audio/handler.rs:2332–2337 emits "lifecycle_generation", but Desktop's parser reads content.generation only. The revised Rust assertion still checks the incompatible key; changing its value to the Off-mode UUID and moving the wrapper to postgres_tests does not fix the wire contract.

Source-derived reproduction: an already-hydrated observer receives START then JOIN for a new live room. Desktop records "pending"; the next authoritative liveness response supplies the real UUID (Off-mode producer). The refresh passes the old active generations to reconciliation, which clears admissions on this mismatch. The in-huddle indicator disappears while audio remains connected.

Repair: emit "generation": lifecycle_generation, correct the Rust oracle, and bind coverage to the actual Desktop JOIN parser/liveness reconciliation contract, including Off mode.

2. P2: the rewritten race witnesses circularly wait until their hooks panic

Representative: state.rs:3761–3777 and 3805–3822. After hook_ready, the coordinator synchronously calls disconnect_community() before sending hook_proceed. That call acquires the same mutex the deny worker holds while its hook waits for hook_proceed.

The resulting schedule is deterministic: worker holds mutex → waits for proceed; coordinator waits for mutex → cannot send proceed. After five seconds, recv_timeout(...).expect(...) panics in the worker. The poisoned lock is recovered by the competing transition, but joining the worker then fails. These witnesses cannot reach their intended passing terminal-frame assertions. This is a test-only validation regression, not a production deadlock, and was established from source, not a test run.

The same ordering occurs in eight rewritten witnesses: w_cancel_race, w_expiry_cancel_race, w_pairing_cancel_race, w_auth_cancel_race, w_manager_cancel_race, w_lifecycle_cancel_race, w_root_manager_drain_race, and w_audio_registry_lifecycle_cancel_race (their blocking calls/proceed sends are at state.rs:3815–3818, 4000–4003, 4189–4192, 4348–4351, 4523–4526, 4721–4724, 4866–4869, and 5002–5005). lifecycle_cancel and drain_all enter the same transition mutex.

Repair: put the competing lock-taking operation on a separate thread, keep the coordinator free to release the hook, establish the blocked/uncancelled state while the hook is held, then release and join both operations before checking the terminal oracle. Preserve the per-control UUID isolation; the production serialization itself remains credited.

F4: production ordering remains accepted; existing coverage gap remains open

Both real rejection branches still enqueue denial before lifecycle cancellation (add-peer, commit). But the rewritten witnesses construct separate past-deadline and future-deadline gates and call expiry_deny_terminal directly. Neither enters the corresponding handle_active_audio_connection rejection branch. Removing either production call leaves these test bodies unchanged, contrary to their comments.

This is the existing acceptance gap, not another current authorization/delivery defect. Finish the independently delayed-expiry-task witnesses through both actual rejection seams, asserting restricted JSON before 1008 Close and failing when the corresponding production call is removed.

Scope, evidence, and stable exit

F1 observer permits, F2 terminal serialization, F5 quiescence, F7 connection-scoped hook design, and F4 production ordering stay credited. RAM-only restart amnesia/issuer re-push, asynchronous propagation, and separate S5 HTTP enforcement stay accepted. The new finding concerns the changed test rendezvous, not a reopening of those contracts.

Source-only review on pinned Blox, with independent metadata/history reconciliation and coordinator verification of producer/consumer and lock/hook ordering. No checkout, installation, build, tests, PR-code execution, or runtime reproduction. The one existing exact-head CI snapshot had 34 successful, 19 skipped, 3 in-progress checks, zero failures (56 total), including a successful Rust unit lane. That snapshot is not evidence that these particular witnesses ran; no CI failure is attributed to them, and no rerun or monitoring was performed.

Stable exit: fix the JOIN key/consumer regression, repair the eight changed race witnesses, and finish F4's already-required production-bound coverage. Previously accepted production fixes do not need redesign.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: two inherited items remain; F4 coverage is closed

Reviewed e828136739f6360ae0407ee6b636f19c2bb83ed5 against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, continuing review 5273883265. This is the bounded corrective review of the two intervening commits, not a renewed S3/mesh/HTTP audit. The contract remains coherent huddle presence across JOIN/liveness and denial-before-close delivery with production-bound regression witnesses.

Credited: both new F4 witnesses now enter the actual audio rejection branches and assert restricted JSON followed by 1008 Close (add-peer, commit). They park the handler at its gate seam and cancel its token while the independent deadline task has not fired. This closes the previously missing caller binding; the older primitive tests are now honestly labelled. The restart-arm denial drain, added pre-send-loop rejection drains, and shared test-environment mutex are also credited from source. No test execution is claimed.

1. P2: JOIN still uses the wrong generation wire key (existing F3)

audio/handler.rs:2405–2410 still emits "lifecycle_generation", while Desktop’s parser reads only content.generation. The Rust oracle still asserts the incompatible key.

Source-derived reproduction: an already-hydrated observer receives START then JOIN for a new live room. Desktop records "pending"; the next authoritative liveness response supplies the UUID. Refresh passes the old generation map to reconciliation, which clears admissions on the mismatch. The in-huddle indicator disappears while audio remains connected, including Off mode.

The newly added liveness witness does not close this contract. It seeds KIND_HUDDLE_LIVENESS - 1 and discards insertion errors: that is 48103, not the 48100 required by huddle_started_links. Its fresh fixture therefore supplies no matching START link; absence of a liveness EVENT is explicitly allowed to pass. It also never consumes an actual JOIN or calls Desktop’s parser, so changing JOIN’s key/value cannot affect this witness. This is a defect in the attempted F3 coverage, not a separate product requirement.

Repair: emit "generation": lifecycle_generation, correct the Rust oracle, and bind coverage to the actual JOIN → Desktop parser → liveness reconciliation contract. Seed the proper START kind, fail on fixture errors/missing EVENT, and compare against a real JOIN rather than only the in-memory expected UUID. Include Off mode.

2. P2: five race witnesses still circularly wait until their hooks panic

Three witnesses now put the competing call on its own thread, which removes their circular wait. Five remain unchanged: w_cancel_race, w_expiry_cancel_race, w_pairing_cancel_race, w_auth_cancel_race, and w_manager_cancel_race.

Representative: the worker’s hook waits for hook_proceed while holding the transition mutex. After hook_ready, the coordinator synchronously calls disconnect_community before sending proceed; that call takes the same mutex. Worker waits for coordinator; coordinator waits for worker. After five seconds the hook times out and panics, and joining the worker fails. The intended terminal-frame assertions cannot complete. This is a test-only validation defect, not a production deadlock, established from source rather than execution.

The remaining blocking-call/proceed pairs are state.rs:3815/3818, 4000/4003, 4189/4192, 4348/4351, and 4523/4526.

Repair: move those five competing calls onto independent threads, keep the coordinator free to release the hook, then join both operations before the terminal oracle. Preserve connection-scoped hook isolation. Non-blocking refinement for the three repaired witnesses: their immediate spawn-then-release no longer forces contention, so their documented mutation detection is scheduler-dependent; establish the blocked/uncancelled state before release. This does not reopen the accepted production serialization.

Scope, validation, and stable exit

F1 observer permits, F2 terminal serialization, F5 quiescence, F7 connection-scoped hook design, the root-writer shared-deadline drain, and hermetic fixtures remain credited. RAM-only restart amnesia/issuer re-push, asynchronous propagation, and separate S5 HTTP enforcement remain accepted. F4 production ordering and its new caller-bound coverage are closed. No unrelated S3/mesh/HTTP changes or generic writer hardening are requested.

Source-only review on pinned Blox, with independent metadata/history reconciliation and coordinator verification of decisive producer/consumer, fixture/query, and lock/hook paths. No checkout, installation, build, tests, PR-code execution, or runtime reproduction. The one existing exact-head CI snapshot had 17 successful, 18 skipped, 12 in-progress, and 2 failed checks (49 total). Failures were Rust / Rust Lint and Rust / Windows Rust; Unit Tests was in progress and the relay PostgreSQL lane skipped. No failure cause is attributed here, and no rerun or monitoring was performed.

Stable exit: repair F3’s wire contract and meaningful consumer coverage, and remove the circular wait from the five remaining race witnesses. Do not redesign the credited fixes.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: the same two exit items remain

Reviewed 497ac9e3f982315d0e29412e67ef35130dcf5c84 against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, continuing review 5274381697. The sole descendant commit changes only crates/buzz-relay/src/nip_fi_session.rs (+234/-9). The F3 producer/consumer and five race-witness files are byte-identical to the prior reviewed head. This is a bounded corrective review, not a reopened S3/mesh/HTTP audit.

Credited: F5 replaces the 50 ms sleep with a connection-keyed, bounded cancel-arm rendezvous and adds a subscription-registry cleanup witness. The new witness holds an effect permit, enters the real registry-disconnect/expiry path, models an in-flight subscription registration, then explicitly removes it after task completion and asserts zero remaining entries. That is useful source-level coverage; it is not an executed mutation result or a full production-teardown integration test. Existing F5 production quiescence remains credited, with no new blocker in this delta.

1. P2: JOIN still emits a generation key Desktop does not consume (F3)

JOIN serialization writes lifecycle_generation; Desktop’s parser reads only generation. The Rust oracle still asserts the incompatible key.

Source-derived reproduction/impact: an already-hydrated observer receives START then JOIN for a new live room. Desktop records "pending"; the next liveness response supplies the real UUID. Refresh passes the previous generation map to reconciliation, which clears admissions on that mismatch. The in-huddle indicator disappears while audio remains connected, including Off mode.

The attempted F3 liveness coverage remains vacuous: its fixture seeds KIND_HUDDLE_LIVENESS - 1 (48103 ENDED, not 48100 STARTED) and discards insertion errors. The database lookup requires STARTED, while the oracle explicitly passes without any liveness EVENT. It never consumes an actual JOIN or invokes Desktop’s parser.

Repair: emit "generation": lifecycle_generation, fix the Rust oracle, and cover the actual JOIN → Desktop parser → liveness reconciliation contract, including Off mode. Use a valid START fixture, fail on fixture errors/missing EVENT, and compare with the real JOIN rather than only the in-memory UUID.

2. P2: five race witnesses still circularly wait until the hook times out

The unchanged witnesses are w_cancel_race, w_expiry_cancel_race, w_pairing_cancel_race, w_auth_cancel_race, and w_manager_cancel_race.

Source-derived schedule/impact: the producer fires its test hook while holding the transition mutex; the hook waits for the coordinator’s proceed signal. After readiness, the coordinator synchronously calls disconnect_community before releasing the hook. That call takes the same mutex. Neither side can advance until the five-second hook timeout panics, so the worker join/terminal oracle cannot complete successfully. This is a test-only validation defect, not a production deadlock.

All five blocking-call/proceed pairs remain state.rs:3815/3818, 4000/4003, 4189/4192, 4348/4351, and 4523/4526.

Repair: run the competing calls on independent threads, leave the coordinator free to release the hook, then join both operations before asserting the terminal result. Preserve connection-scoped hook isolation. The previously noted contention refinement for the three already-repaired witnesses remains non-blocking.

Scope and validation

The product contract remains coherent huddle presence across JOIN/liveness and denial-before-close delivery with meaningful regression witnesses. F1 observer permits, F2 terminal serialization, F4 production ordering and caller-bound coverage, F5 quiescence, F7 hook isolation, the root-writer shared-deadline drain, and hermetic fixtures remain credited. RAM-only restart amnesia/issuer re-push, asynchronous propagation, and separate S5 HTTP enforcement remain accepted. No unrelated hardening is requested.

Source-only review on pinned Blox, with independent metadata/history reconciliation and coordinator verification of the decisive contracts. No checkout, installation, build, tests, PR-code execution, or runtime reproduction. One existing exact-head CI snapshot: 47 successful, 27 skipped, 6 failed, 1 cancelled displayed entries (81 total; aggregate duplicates included). Red entries concern Rust Lint, Windows Rust, and PostgreSQL Tests; Run Codex Security Review was cancelled. No logs were inspected, causes attributed, reruns requested, or monitoring performed.

Stable exit: repair F3’s wire contract and meaningful consumer coverage; remove the circular waits from the five remaining race witnesses. Preserve the credited fixes.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: finish the new audio drain and the agreed F3 witness

Reviewed a311d5c1369b2fee0b4c8e017efa031144938840 against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, continuing review 5274898572. The corrective delta from 497ac9e3f982315d0e29412e67ef35130dcf5c84 changes nine relay files (+627/-124). This is not a renewed inherited S3/mesh/HTTP audit. The contract remains coherent huddle presence across JOIN/liveness and denial-before-close delivery with meaningful production-bound regression witnesses.

Credited: JOIN now emits generation, and its Rust serialization assertion checks that key. The liveness fixture now uses the proper START kind, schema-compatible rows, required insert success, and a required liveness EVENT. All five remaining race witnesses move the competing disconnect to a separate thread, release the producer hook, and join both before the oracle, removing the test-only circular waits. Connection-scoped hook isolation is preserved. These items should not be redesigned or reopened.

1. P2: serialize the new audio error-exit drain before reading its terminal queue

The new drain_terminal! drains with try_recv() and then reads the disconnect reason, without first entering the terminal transition lock. But disconnect_nip_fi publishes AuthorizationDenied before enqueueing its payload under that lock. An empty queue is therefore not evidence that no denial has won.

Source-derived schedule: the audio connection has registered its terminal sender and proven key. An admin disconnect wins the reason slot on another worker, then is descheduled before try_send. A membership failure enters the new macro: try_recv() sees empty, the reason read sees denial, and the handler sends a 1008 authorization-denied close and returns. The producer's later payload has no live receiver. The outer registration wrapper only cancels after the handler returns; it cannot recover the dropped frame.

Impact: a rejected audio client misses the canonical denial payload even though the terminal close says authorization denied. This is incomplete enforcement in the newly added drain, not an authorization bypass or a claim that these returns worked at the prior head.

Smallest repair: call the existing control.lifecycle_cancel() before this drain, as the later pre-writer exits already do. It waits for a winning producer's enqueue or claims the lifecycle-close slot before a later producer can win. Add a caller-bound witness for one of these newly covered error exits with the producer paused between reason publication and enqueue. Preserve the shared flush deadline; no new synchronization owner is needed. When lifecycle close wins, this also makes the error exit send a bare close, consistent with the later exits.

2. P2: finish the existing F3 producer-to-consumer witness

The liveness test still compares the response only to state.huddle_liveness_generation, then asserts that same independent expectation. Its complete body never creates or reads a JOIN or invokes Desktop. The separate JOIN witness supplies the expected generation directly to commit_participant_join, bypassing the production caller’s generation selection.

Falsifying source counterexample: change the Off-mode fallback at audio/handler.rs:756–759 to a different UUID. Neither F3 witness observes that caller, so both retain the same inputs/assertions. Real JOIN and liveness then disagree and Desktop’s reconciliation clears admissions. This is a source-derived test-coverage counterexample, not an executed mutation or a claim that the corrected wire currently has this bug.

Existing Desktop tests do not supply the missing binding: their event helper synthesizes generation; the live START/JOIN case stops before a liveness refresh. The same-generation case starts from hydrated history rather than the new-live-room sequence. The previously requested cross-boundary regression remains unprotected.

Smallest repair: finish the agreed Off-mode witness: obtain the real JOIN using production generation selection, feed its actual wire content through Desktop’s parser/runtime after hydration, then reconcile the corresponding real liveness response and assert that the admission remains present. Keep the corrected fixture and fail-on-missing-EVENT behavior. No client behavior change or new general-purpose harness is requested.

Scope and validation

F1 observer permits, the existing F2 serialized transitions, F4 ordering/caller-bound coverage, F5 quiescence, F7 hook isolation, and hermetic fixture isolation remain credited. The changed root restart writer has no material finding. Command-verifier and deny-map implementations did not change in this corrective delta. RAM-only restart amnesia/issuer re-push, asynchronous propagation, and separate S5 HTTP enforcement remain accepted. The previously non-blocking contention refinement remains non-blocking: sleeps do not prove arrival, but this does not reopen the repaired circular waits or production serialization.

Stable exit: serialize the newly added early-error drain and bind its race witness; finish the already-requested F3 producer-to-consumer witness. The corrected JOIN wire and five circular-wait repairs stay closed.

Source-only on pinned Blox, with independent corrective/delta review, history reconciliation, and coordinator verification. No checkout, installation, build, tests, PR-code execution, or runtime reproduction. One exact-head CI snapshot at about 15:03Z contained 47 runs: 12 successful, 17 skipped, 18 in progress, and none failed/cancelled. No monitoring, reruns, or failure attribution.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: finish the two agreed regression witnesses

Reviewed 00647e415bdf5996c79ed8b6940c973bfee8fc51 against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, continuing review 5280105346. Scope is the seven-file corrective delta from a311d5c1369b2fee0b4c8e017efa031144938840, not another inherited S3/mesh/HTTP audit.

Credited: the early drain_terminal! now calls the existing serialized lifecycle_cancel() before draining, and the terminal payload/close share a one-second flush deadline. F3 now drives the real admission handler and reads its committed JOIN, so changing the production Off-mode fallback UUID is finally observable. The corrected JOIN key, schema/liveness fixture, circular-wait repairs, and existing F1/F2/F4/F5/F7 production contracts remain closed.

1. P2: bind the early-drain serialization repair to its actual caller

The production fix at audio/handler.rs:611–635 is correct by inspection, but its previously requested race witness is still missing. The RoomEnded witness completes disconnect_nip_fi before releasing the handler and exercises the separate finalize_drain! path. The registry race witness calls control.lifecycle_cancel() directly with a synthetic queue consumer. Neither observes the early macro’s call at line 619.

Falsifying source schedule: remove only $control.lifecycle_cancel() from that macro. On an audio connection with no assertion/expiry task, pause the real registry deny producer after publishing AuthorizationDenied and before enqueueing its payload; let an early membership-error exit reach the macro. It can drain an empty queue and send the authorization-denied close before the producer enqueues. The RoomEnded and direct-control tests retain their behavior because neither uses the removed call. The new expiry-task await is not a substitute on this no-expiry path.

Required finish: drive one actual early-error caller with that reason-before-enqueue interleaving and assert the canonical denial payload before the policy close. The test must fail when the macro’s serialization call is removed. Reuse the existing connection-scoped producer hook and bounded coordination; no new production synchronization owner. This is missing protection for the repaired denial-delivery defect, not a claim that the current fix still drops the frame.

2. P2: finish the agreed F3 Desktop retention sequence

The new real-admission witness checks committed JOIN generation against state.huddle_liveness_generation, then disconnects. It does not query liveness or exercise Desktop. The liveness witness still creates a separate room fixture. Their shared-source equality is useful and closes the old fallback-UUID counterexample; it does not supply the explicitly requested post-hydration client sequence.

Desktop’s existing synthetic event builders manufacture their own wire content. Its new-live-room START/JOIN test stops before a liveness refresh. Consequently the newly produced JOIN is still not fed through the actual parser/runtime and then reconciled with its corresponding real liveness response. The relevant user impact remains admission disappearance during reconciliation; no current production mismatch is alleged.

Required finish, unchanged from the previous review: use the real production-selected JOIN content through Desktop’s parser/runtime after hydration, reconcile the corresponding liveness response, and assert the admission remains present. Retain both newly credited Rust assertions. A small producer-derived contract fixture plus the existing Desktop harness is sufficient; no client behavior change or general-purpose cross-language framework is requested.

Non-blocking corrective note

At audio/handler.rs:245–250, timeout(send).await.is_err() handles only the timeout, not Ok(Err(send_error)). The previous code returned on send failure; the new code enters the auth receive loop. Accept only Ok(Ok(())) to preserve that behavior. The receive path remains bounded by the existing five-second auth timeout and may observe the transport failure immediately, so I am not elevating this to a security/resource-exhaustion blocker without stronger evidence.

Stable exit and validation

Complete the two caller/consumer-bound witnesses above; preserve the credited production repairs. The contention-entry refinement stays non-blocking, as previously agreed: entry notification is not proof of lock contention, and adding sleeps would not establish deterministic ordering either. RAM-only restart amnesia/issuer re-push, asynchronous propagation, and separate S5 HTTP enforcement remain accepted. No new audit scope is requested.

Source-only on the pinned Blox, with complementary reviews and coordinator verification. No checkout, installation, builds, tests, mutation execution, or runtime reproduction. The sole exact-head CI snapshot at 18:19:06Z showed 51 successful, 2 failed, 26 skipped and 1 in-progress checks. The failures were PostgreSQL Domain / PostgreSQL Tests and its aggregate; their cause was not inspected or attributed to these findings. No CI monitoring or reruns.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: finish the remaining Desktop consumer witness

Reviewed 6e90134ec2296d9efaa804a0efff3d259e5bea57 against base c213d90d5c450e2e579505da4a4145f528ecbcf2, continuing review 5282041142. This is the bounded corrective review of the two agreed regression witnesses, not a new audit of inherited base changes or S3/mesh/HTTP behavior.

P2: F3 still stops before the actual Desktop consumer

The new same-admission liveness chain is real progress: it reads the production admission’s committed 48101 content, calls the real relay liveness handler for that room, and compares the returned generation with the JOIN generation. That closes the prior separation between the two Rust fixtures.

However, handler.rs:6051–6059 asserts string equality and then cleans up. Despite the “hydrated Desktop-semantics oracle” comment, no Desktop parser/runtime runs and no post-refresh Desktop admission is asserted. BASE→HEAD adds no Desktop witness or producer-derived client fixture. The existing runtime builders still construct their own content, and the post-hydration START/JOIN test still ends before refreshing liveness.

Why this remains actionable: the requested producer→consumer regression protection is still absent. Changing Desktop’s interpretation or reconciliation of these payloads cannot falsify the new Rust assertion; the user-visible failure being protected is an admitted participant disappearing during liveness reconciliation (actual clearing boundary). This is not an allegation of a current production generation mismatch.

Stable exit, unchanged: pass producer-derived serialized JOIN content and its corresponding liveness response through the actual hydrated Desktop parser/runtime, refresh authoritative liveness, and assert the participant remains present. Keep the mismatched-generation retirement control and the credited Rust assertions. A small contract fixture and the existing Desktop harness suffice; no client behavior change or cross-language framework is requested.

Closed / nonblocking

• The early-drain witness now reaches the actual membership-error caller with no assertion/expiry task, uses the real registry producer paused after reason publication and before enqueue, and asserts the exact restricted payload before 1008 (test). The prior caller-binding gap is closed by source inspection. Its 20 ms producer-release sleep is not proof of deterministic contention or an executed mutation result; that refinement remains nonblocking as previously agreed.
• The auth challenge send now accepts only Ok(Ok(())) (lines 247–256), resolving the prior minor note. Previously credited production serialization/bounded flush, generation key/schema, circular-wait repairs, and F1/F2/F4/F5/F7 contracts remain closed. Accepted RAM-only restart/re-push, asynchronous propagation and separate S5 HTTP enforcement are unchanged.

Validation limits

Source-only on the pinned Blox, with complementary reviews and coordinator verification. No checkout, builds, tests, mutation execution or runtime reproduction. New Rust witnesses are structurally selected by the ignored PostgreSQL CI lane. The sole exact-head metadata snapshot at 2026-09-22 22:49:49Z showed 51 successful, 27 skipped, 2 failed and 1 cancelled checks; PostgreSQL Domain / PostgreSQL Tests and its aggregate failed. Their cause was not inspected or attributed to these changes, and this snapshot does not establish that the new witnesses passed. No CI reruns or monitoring.

[github-actions]

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

• Exact PR diff: 312cf674fad5563c24a66ec5350d306d192ed64a...cea34b1ca94584244d06701e8f24d839630f3cbb
• Model: gpt-5.6-sol

💡 Click "edited" above to see earlier reviews for this PR.

---

Review Summary

Overall Risk: HIGH

The NIP-FI admission path does not bind assertions to the host-resolved community, and issuer-scoped disconnects close sessions belonging to other issuers.

Findings

[HIGH] Assertions are verified without the host-derived community boundary

• Category: Isolation
• Location: crates/buzz-relay/src/router.rs:380 (source)
• Description: The upgrade handler calls the global assertion verifier before bind_community. The verifier accepts any audience configured for the issuer, and neither receives nor later checks the resolved community, its canonical audience, or its issuer allowlist. Consequently, an assertion valid for one community is accepted when presented to any other configured community host; subsequent NIP-42 pairing only proves possession of the asserted key and does not restore the missing tenant binding.
• Impact: A user who controls a key accepted by community B can authenticate there using an assertion issued for community A, bypassing community B's federated-identity issuer and audience policy and gaining access to its protected relay and audio surfaces.
• Recommendation: Resolve the host before assertion verification and pass an immutable community authorization context containing the exact expected audience and authorized issuers. Require an exact audience match and issuer membership for that community before accessing JWKS. Apply the same correction to the audio upgrade path.

[MEDIUM] Issuer-scoped disconnect closes same-key sessions from every issuer

• Category: Isolation
• Location: crates/buzz-relay/src/api/nip_fi.rs:134 (source)
• Description: After verifying a command for caller_iss, the handler invokes both disconnect_nip_fi scans with only the target pubkey. ConnectionManager and CommunityConnectionRegistry store and compare only the proven pubkey, not the admitting assertion issuer. Thus a command from issuer A closes matching sessions admitted under issuer B, although the deny entry itself is correctly scoped to issuer A.
• Impact: An authorized administrator for one issuer can disrupt live relay and audio sessions belonging to another issuer whenever the same Nostr key is used in both identity domains.
• Recommendation: Record the admitting issuer alongside the proven pubkey in both connection registries, pass cmd.caller_iss or the propagated message issuer into the close scans, and match on the complete issuer-and-pubkey tuple.

Notes

• No additional limitations were reported.

---

_{Generated by Codex Security Review |
Requested by: @wpfleger96 |
Workflow run}

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: F3 still substitutes a model for the Desktop consumer

Reviewed cea34b1ca94584244d06701e8f24d839630f3cbb against 312cf674fad5563c24a66ec5350d306d192ed64a, continuing the sole remaining P2 from review 5284842512. This is a bounded corrective review, not a fresh audit of inherited S3/mesh/HTTP behavior or upstream base movement.

P2: Exercise the actual hydrated Desktop runtime, not the new Rust imitation

The added HuddlePresenceOracle owns a separate Rust HashMap and manually implements JOIN application and generation reconciliation. The new assertions only inspect that model. They do not call Desktop’s parser, hydration, liveness-response handling, or presence callback.

The real runtime refresh parses the liveness events, reconciles the tracker, then publishes a filtered snapshot. BASE→HEAD changes no Desktop files; its existing harness still constructs independent payloads. The post-hydration START/JOIN test still disposes before refreshing liveness.

Impact / falsifiability: a change to Desktop’s interpretation or reconciliation of the producer payload cannot affect this Rust-only witness. The agreed regression protection against an admitted participant disappearing at authoritative refresh remains missing. This is not a claim that the current production generations mismatch, nor that Desktop has no other liveness tests.

Unchanged exit: feed producer-derived serialized JOIN and corresponding liveness events through the actual hydrated Desktop runtime, trigger authoritative refresh, and assert that the admitted participant remains in the published presence set. Keep the existing mismatched-generation retirement control and credited Rust producer assertions. A small contract fixture plus the existing Desktop harness is sufficient; no production client change or cross-language framework is requested. Adding more assertions to the Rust imitation does not close this seam.

Credited / nonblocking

• The same real admission → committed JOIN → real relay liveness chain remains credited. The new signer/p-tag checks strengthen producer coverage; prior early-drain and F1/F2/F4/F5/F7 fixes remain closed. RAM-only restart/re-push, asynchronous propagation and separate S5 HTTP enforcement remain accepted.
• Minor documentation fix: restore literal \n escapes in NIP-FI.md:838–843. Actual line breaks now split three normative denial-table rows. This is not an additional merge blocker.

Validation

Source-only on the pinned Blox, with complementary reviews and coordinator verification. No checkout, builds, tests, mutation execution or runtime reproduction. The single exact-head CI snapshot at 2026-09-23 01:47:21Z showed 51 successful, 26 skipped and two running checks; PostgreSQL tests and Rust lint passed, while two Desktop smoke jobs were running. No reruns or monitoring. Passing relay checks do not establish the absent Desktop contract witness.