feat(nip-fi): harden Blossom kind-24242 verifier to NIP-FI spec

[wpfleger96]

Brings buzz-media, buzz-relay, all first-party kind-24242 minters, and desktop token minting into compliance with NIP-FI §kind-24242. Resolves the compliance note added in #7278.

What changed

buzz-media/src/auth.rs

• Add BlossomStrictness enum (Strict | Permissive). Strict applies full NIP-FI rules (active modes); Permissive preserves pre-NIP-FI Off-mode behavior byte-identical [FI-INV-15].
• Rewrite verify_blossom_auth_event_for_verb with count-based cardinality: exactly one t/expiration/server in Strict; duplicate x also rejected. Permissive keeps main's boolean semantics exactly: any valued t other than the verb (including "") rejects, valueless t is ignored.
• Strict: mandatory server tag on all proofs (upload + read); absent or mismatched → evidence_rejected (ServerMismatch).
• Strict: 60-second proof window (now - created_at <= 60s, expiration <= created_at + 60s). Permissive: 3600s / optional server.
• [F1] Permissive server validation uses any-match semantics: a proof with multiple server tags is accepted if any tag matches the bound host. This restores origin/main base behavior. Strict duplicate-rejection unchanged.
• [F1 compat] Permissive server gate uses server_values.is_empty() (valued tags only) rather than server_count > 0 (includes valueless tags). A lone valueless ["server"] tag is treated as absent in Permissive/Off mode, matching pre-NIP-FI base behavior [FI-INV-15].
• [F4] Valueless x tags in Strict get reads are tracked before filter_map so they cannot become absent scope (host-wide authorization). Any x tag present in Strict must contain the exact requested sha256.
• [F3 unit] Cases A–D in auth.rs are helper-level unit tests: they call verify_blossom_upload_auth and verify_upload_hash_only directly. They do not enter the upload pipeline (process_upload/process_video_upload). Retained as helper evidence.

buzz-media/src/error.rs

• Add DuplicateTag(&'static str) variant.
• Add blossom_denial_kind() method: classifies each auth error as MissingEvidence, EvidenceRejected, or AuthorizationDenied. Non-auth errors return None.
• IntoResponse retains the legacy JSON 401 shape [FI-INV-15].

buzz-relay/src/api/media.rs

• Add MediaDenial(MediaError, BlossomStrictness) wrapper mapping to byte-exact NIP-FI fixed responses in Strict; falls through to legacy JSON in Permissive.
• blossom_strictness_from_state() selects Strict when state.config.nip_fi.is_enforce(), otherwise Permissive. Strictness is threaded through the admit_nip_fi_http_on_state NIP-98 closures on upload and read, and through the post-admission X-SHA-256, hash-binding, and membership denials.
• extract_blossom_auth keeps first-value Authorization extraction; Enforce rejects repeated fields at the admit_nip_fi_http cardinality gate, and Off keeps legacy behavior [FI-INV-15].
• Both membership-denial call sites use media_denial(RelayMembershipRequired, strictness) so Strict mode produces the correct fixed-text response.
• upload_blob serving-write fence conversion (:406) uses map_err(|e| media_denial(e, strictness)) — a community fence committed between auth admission and lease acquisition no longer resets Strict to Permissive.
• Mode-driven regressions in mod postgres_tests (selected by the nextest postgres-ci profile via test(/postgres_tests::/), #[ignore = "requires Postgres"]), run through the full relay router with a real Enforce verifier and assertion: Enforce rejects Permissive-only proofs (300s lifetime, missing server tag) on both upload routes and reads with evidence rejected; Off accepts the same proofs with pre-NIP-FI responses; DenyProtected answers 503 before any proof check; membership denials are authorization denied text/plain in Enforce and exact legacy JSON 403 in Off, with a member positive control. GET and HEAD are each discriminated (HEAD with a suppressed body). Both upload routes pin the handler's X-SHA-256 missing/malformed/unsigned and post-body hash-mismatch denials in both modes. Off rejects an empty-valued t beside a valid one with legacy 401 JSON in either order, and ignores a valueless one. Exact status, Content-Type, challenge, and body bytes are pinned.

buzz-media/src/upload.rs (mod minio_tests)

• [F3 pipeline] Four tests exercising the actual post-body call sites (process_upload :85, process_video_upload :413) with real MinIO I/O: buffered/streaming × expired-proof-accept + mismatched-hash-reject. The positive cases use fresh_strict_upload_auth (created_at = now-58, expiration = now+2, lifetime = 60s) — Strict-admissible at sign time — followed by a real 3s sleep so the proof is expired when the upload completes. nostr::Timestamp::now() reads the OS wall clock directly; paused Tokio time does not advance it. The negative cases use a pre-expired proof with a mismatched x tag. All four are #[ignore = "requires MinIO"]; no CI lane runs them automatically.

buzz-media/src/validation.rs

• minimal_valid_mp4() promoted to #[cfg(test)] pub(crate) — a 618-byte pre-computed H.264/avc1 fast-start MP4 available to all buzz-media test modules; smoke-tested by test_minimal_valid_mp4_passes_validate_video_file.

desktop/src-tauri/src/commands/media.rs

• sign_blossom_upload_auth: server tag mandatory; upload expiry 60s (was 3600s/300s).
• MEDIA_GET_AUTH_EXPIRY_SECS: 60s (was 600s).

desktop/src-tauri/src/media_proxy.rs

• Single re-mint+retry on 401/403 for range requests; non-auth failures do not retry.

crates/buzz-cli/src/client.rs

• Read expiry: 60s (was 600s). Upload expiry: 60s. server tag mandatory on upload.

crates/buzz-dev-mcp/src/view_image.rs

• MEDIA_GET_AUTH_EXPIRY_SECS: 60s (was 600s). Test updated.

mobile/lib/shared/relay/media_auth.dart

• _mediaGetAuthLifetimeSeconds: 60 (was 600). Refresh margin updated.

mobile/lib/shared/relay/media_upload.dart

• _uploadAuthLifetimeSeconds: 60 (was 300). server tag mandatory.

mobile/lib/features/channels/media_viewer_page/video_viewer.dart

• [F2] Remove Android-specific VideoPlayerController.networkUrl streaming path. video_player_android 2.9.5 freezes headers into static DefaultHttpDataSource request properties — a 60s proof minted at controller creation time becomes stale on seeks. All platforms use the authenticated local-file download path.
• [F2 transport] unawaited(request.sink.close()) before client.send(). AbortableStreamedRequest carries no request body; without closing the sink, IOClient.send() awaits stream.pipe(ioRequest) which blocks until the sink ends — every video download hung in loading indefinitely on every platform.
• [F2 pending-ctrl] pendingController ref set before the first await (initialize()), cleared in all exit paths. Effect cleanup disposes pendingController.value so a close-during-init releases the native player even if the initialized event never arrives.
• [F2r(a)] On native init failure (post-create, pre-initialized-event), the inner catch starts disposal detached via unawaited(localController.dispose().catchError(...)) — the .catchError handler absorbs any disposal exception so it does not reach the test error zone — then rethrows the original load error. video_player 2.11.1 completes the init future with an error without disposing the native player.
• [F2r(b)] Non-2xx error body cancelled via _cancelVideoResponse() (subscribe+cancel) instead of drain(). drain() waits for the upstream to close; a stalled server body blocked initializeVideo indefinitely.
• [F2r(d)] unawaited(localController.dispose().catchError(...)) in the catch — not await. Two failure mechanisms exist: (1) when createWithOptions() throws, _creatingCompleter is never completed and await dispose() deadlocks at :682-683, blocking the outer catch from setting error state. (2) when createWithOptions() succeeds but initialization fails, dispose() runs but its thrown exception becomes an unhandled async error reaching the test zone. Both are fixed: unawaited lets the outer catch run immediately and set error.value; .catchError(...) on the detached disposal future absorbs any disposal exception.

mobile/test/features/channels/media_viewer_page/video_viewer_test.dart

• _FinalizingFakeClient: calls request.finalize().drain() — a test without the sink fix times out because the drain never completes, validating the transport contract.
• F2r(a): forceInitError=true fake emits PlatformException on subscribe; disposeCallCount >= 1 verifies disposal even on init failure.
• F2r(b): stalledBody StreamController with onCancel Completer (bounded completion signal, not a sleep). Asserts: (1) bodyStreamCancelled=true while body stream is still open (impossible with drain()); (2) error UI visible while body remains open. Restoring drain() breaks both.
• F2r(c): neverInitialize=true fake never sends initialized event; unmount asserts disposeCallCount >= 1 via pendingController. Distinct from the event-error case.
• F2r(d): forceCreateError=true fake throws PlatformException from createWithOptions() itself (before any player ID is returned). Asserts error UI visible within a bounded deadline. With the old await dispose() the outer catch is blocked — error.value is never set and the error UI assertion fails at the deadline; with unawaited(dispose()) the error text appears immediately.

mobile/test/features/channels/media_viewer_page/video_viewer_transport_test.dart (separate file)

• Real-IO loopback probes using flutter_test (not package:test) to avoid depend_on_referenced_packages lint. No TestWidgetsFlutterBinding — no HttpClient override interfering with dart:io.
• Probe 1: HttpServer.bind(loopbackIPv4, 0) reads the full request body before replying. Without sink.close(), req.drain() hangs and the test times out.
• Probe 2: server signals request arrival via a Completer (not a sleep); abort fires after confirmed arrival; client.send() must throw RequestAbortedException (typed assertion) within a bounded 5s deadline.

scripts/test-video-upload.sh

• Proof minted with +60 expiry and mandatory server tag.

buzz-test-client/tests/e2e_media*.rs + buzz-relay/src/api/media.rs test fixtures

• All fixtures updated to 55s expiry + server tag.

docs/nips/NIP-FI.md

• Compliance note: strict verifier engaged in enforce, Permissive in off, 503 in deny_protected; deny-map (S4) named as the remaining gap.

RELEASING.md

• New "NIP-FI upload proof freshness (relay floor)" section documenting that 60s proofs require the first relay release carrying this PR's post-body hash-only check (the first tag > v0.2.1 — update once cut) for reliable large-file uploads. Operators on older self-hosted relays (≤ v0.2.1) must upgrade before receiving these clients.

Finding 3: 60s upload proofs vs old-relay compatibility

Desktop/CLI/mobile upload proofs are now 60s. Older relays re-run full expiry verification after the body is transferred. A valid upload whose body transfer takes >60s will fail on an old relay.

Resolution (relay floor prerequisite): RELEASING.md requires that verified supported deployments (relay releases confirmed to carry the post-body hash-only check, commit 75e9bef748) be in place before distributing 60s-proof client builds. Unknown or unverified self-hosted relay deployments are excluded from distribution until they upgrade. This makes the verified-deployment status a prerequisite for release distribution, not a post-distribution advisory. The concrete tag (> v0.2.1) is to be recorded once cut.

Sequencing note

Builds on #7264 (merged): Blossom strictness is derived from its config.nip_fi mode in this PR, so enabling BUZZ_NIP_FI_MODE=enforce activates the strict verifier with no further wiring.

GIF compatibility exception

An unauthenticated Off-mode request on a configured tenant with no GIF provider changes from 404 to 401 (base gifs.rs:269-275 checked an unconfigured provider first; the current code authenticates first). All other Off-mode Blossom responses are byte-identical to pre-NIP-FI behavior.

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is 02c6309f534050e54c8d36f5ce90e48e4dd152b1...651ab57d21f55128d9a1758e5058c1d6b3184539.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 651ab57d21f55128d9a1758e5058c1d6b3184539 to authorize a new review.
Any previous review applies only to its recorded range.

[wesbillman]

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

Changes requested: the same remaining P2

Reviewed ad6ac0612de3137320028cacb4db24c044a35a20 against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, concentrating on the two-file corrective delta since 08c4152609ad93733fc4c45211c2f1a9d0ce16cb and the exit from review 5279715271. Relay-first rollout remains a valid solution. No client negotiation system or automated deployment gate is requested.

[P2] Require verification of the repaired relay before distributing 60-second-proof clients

Partial fix credited: RELEASING.md:103–116 removes the invented relay-v0.5.0 and unsupported already-deployed assertion. However, the replacement only asks release operators to document a floor and advise self-hosted upgrades. It never requires verifying that the relevant hosted/target relays actually run the repaired implementation before desktop/CLI/mobile distribution. Removing the deployment assertion is not the deployment check requested in the previous review.

The impact is unchanged: these clients issue 60-second upload proofs, while the older video upload pipeline reruns full verification after transfer. Source-derived reproduction, not executed: upload a valid video over 90 seconds to that older implementation. Admission can succeed, then completion fails on proof expiry; re-signing and repeating another 90-second transfer does not fix it. The head pre-body admission and post-body hash-only check remain correctly separated.

Smallest exit: make the release prerequisite explicit, for example:

Before distributing desktop, CLI, or mobile builds that mint 60-second upload proofs, verify that the relevant hosted and target self-hosted relays run a build containing repair commit 75e9bef748d2149ce459b14da842e706a51a5f78 (or a verified containing release tag). Do not distribute to those environments until this check passes. Record the concrete containing tag here when released.

That commit is an ancestor of the reviewed head and contains the post-body repair. A new release tag or assertion that deployment already happened is not required to fix these instructions; carrying out the rollout remains an operational prerequisite. Also change lines 95–98 from “The relay in Strict mode” to “the older full-verifier relay implementation”: head Strict admission does not have this defect.

Coverage and preserved scope

• Preserve the previously requested regression bound to the production upload caller: admission before proof expiry, body completion after expiry, matching hash succeeds, mismatched hash fails. No media/relay upload source or tests changed in this corrective delta. In the inspected exact-head buzz-media auth/upload tests, relay media-handler tests, and buzz-test-client/tests/e2e_media*.rs, that timing witness is still absent. Existing expired-token coverage rejects at admission, not after an admitted transfer; a helper-only test would not catch the production caller reverting to full verification.
• The new viewer test exercises actual widget teardown and would catch removal of activeRequestAbort.complete(). Nonblocking witness weakness: _StallingAbortableClient.send sets abortObserved even when the request is not abortable or its trigger is null. Removing that wiring therefore skips the wait and can still satisfy the assertion. Fail explicitly on missing type/trigger and assert no abort before unmount; this is not a new production blocker.
• Previously credited F1/F2/F4, transport/init cleanup, membership response-byte tests, and Strict serving-write denial handling remain credited. Deferred #7264 activation, assertion pairing, and deny-map integration remain out of scope; permissive expiration semantics and acquisition-fence handler coverage remain nonblocking. This is not a whole-feature PASS.

Validation limits: source-only review on pinned Blox, with independent source/release/metadata lanes and coordinator verification. No checkout, installation, build, tests, PR/dependency execution, native playback, or live slow-transfer probe. One existing exact-head CI snapshot at 2026-09-22 16:06:15 UTC contained 87 contexts: 58 success, 28 skipped, one cancelled (Run Codex Security Review); rollup FAILURE, with no failing test/build context in that snapshot. No reruns or monitoring. Deployed relay state was not verified.

[wesbillman]

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

Review clear: prior release blocker resolved

Reviewed 19ea00ba0378af520f0f8738f5ff7ad8ea1c986e against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, concentrating on the seven-file corrective delta since ad6ac0612de3137320028cacb4db24c044a35a20 and the agreed exit from review 5280683315. No remaining blocking finding in this bounded re-review. This is a comment, not an approval or certification of deployment/CI.

Resolved and credited

• Relay-first compatibility: RELEASING.md:94–129 now requires verifying that supported relay environments run a release containing repair commit 75e9bef748d2149ce459b14da842e706a51a5f78 before desktop/CLI/mobile distribution. Unknown or unverified deployments do not satisfy the prerequisite. The wording correctly attributes the defect to older relays and asks for the concrete containing tag once released, without inventing one or claiming deployment. Repair ancestry is verified. Actual rollout verification remains an operator prerequisite, not something this review established.
• Upload and legacy semantics: relay admission still verifies freshness before body consumption; buffered and streaming production callers retain hash-only completion checks (upload.rs:85,413). The new pipeline tests reach those real callers and add matching/mismatched-hash controls. Permissive expiration parsing now restores the base implementation’s last-valued-wins behavior without changing Strict duplicate rejection.
• Mobile corrective delta: detached disposal now handles errors without delaying the original load-error UI (video_viewer.dart:184–216). The strengthened viewer test requires an actual abortable request/trigger and observes no abort before unmount. The separate IOClient transport probe holds the response open until cancellation. Previously accepted verifier, playback, membership-denial and serving-write repairs remain credited.

Nonblocking coverage qualification

The new MinIO pipeline tests are useful production-bound post-body checks, not yet a same-proof admission-before-expiry/completion-after-expiry witness. expired_upload_auth sets created_at = now - 120 and expiration = now - 1: that 119-second lifetime cannot pass Strict admission. The fresh-admission unit test uses a different proof. All four pipeline cases are ignored by default; the inspected _ci-relay.yml ignored-test selectors do not select them. Follow up with a Strict-admissible proof carried across the boundary and an explicit infrastructure-backed invocation. This is a coverage limitation, not evidence that the repaired production path is wrong, and does not reopen the resolved release blocker.

Scope and validation

Independent upload, mobile and release/history lanes returned and were integrated with coordinator source verification. Unchanged minters and previously credited GET/HEAD/denial paths were not independently recertified. Deferred #7264 activation, assertion pairing and deny-map integration remain excluded; this is not a whole-feature PASS.

Source-only on pinned Blox: no checkout, install, build, tests, PR/dependency execution, native playback or live slow-transfer/deployment probe. Static PNG chunk/CRC and MP4 box-bound inspection is not a runtime validator pass. One existing exact-head CI snapshot contained 87 contexts: 54 success, 28 skipped, four failures (Desktop Smoke E2E shards 1 and 4 plus two Desktop aggregates), and one cancelled Codex Security Review. No cause inferred, reruns or monitoring; CI is not green.

[wesbillman]

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

Review clear: bounded corrective re-review

Reviewed HEAD 4afefe42639907f15dbb124ce7b0ebf96088b921 against BASE 26ede6dfa2496993aa62ce5781d2112df4c2d009, continuing the prior clear review. No actionable blocker found in the corrective delta. This is a comment, not an approval or whole-feature certification.

• Scope: after subtracting inherited base movement, the branch-specific changes are confined to crates/buzz-media/src/upload.rs tests and the two mobile video-viewer test files. Previously credited production fixes, relay-first release policy, and the NIP-FI branch amendment remain unchanged. Supported relays still must be verified to contain repair 75e9bef748d2149ce459b14da842e706a51a5f78 before distributing 60-second-proof clients; this review does not establish deployment.
• Upload witness: buffered and streaming positives now pass the same Strict-admissible proof through explicit verification, a real three-second wait, and the production completion pipeline. This addresses the prior same-proof coverage note. It is still a verifier-to-pipeline test, not a real HTTP slow-transfer test; the four MinIO cases remain ignored by default, and this review did not execute them. The short admission margin and wall-clock dependency remain test reliability limitations, not evidence of a production regression.
• Mobile witness: F2r(d)+ now waits for disposal entry with a real-zone five-second timeout inside runAsync, instead of a fixed delay followed by a FakeAsync-zone wait. It still requires the error UI and an observed disposal call from a fake that throws during disposal. The transport-file delta only corrects comments about the held response and possible socket-close exception.

Validation and limits

Independent upload and mobile source-review lanes were integrated with coordinator checks on the pinned Blox host. No checkout, installation, build, test, PR/dependency execution, native playback, live slow-transfer probe, CI rerun or monitoring was performed. Existing exact-head check snapshot: 59 successful, 28 skipped, one cancelled security-review job, no failed checks; the Mobile job succeeded. This does not establish an executed MinIO witness.

Unchanged client minters and earlier GET/HEAD/denial repairs were not independently recertified. Deferred #7264 activation, assertion pairing, and deny-map integration remain excluded. No new merge condition is introduced by this re-review.

addressed

[bradseiler]

Integration TODO before merge:

• Merge feat(relay): enforce NIP-FI assertion+NIP-98 pairing on HTTP ingress #7264 first.
• Then rebase this PR onto the resulting latest main, resolve the api/media.rs / config.rs overlap while preserving both assertion-pairing and strict verifier/denial behavior, and rerun exact-head CI/security review.
• Blossom "strictness" mode swap

[wesbillman]

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

Review clear for this delta; no blocking findings. Reviewed head 651ab57d21f55128d9a1758e5058c1d6b3184539 against base 6410e685a80d42db0645fadc9bbe710559ef911e, following the previous clear review.

• Verified the new integration with #7264: Enforce selects Strict consistently for upload, GET and HEAD; assertion-key pairing remains intact; post-admission hash, membership and write-fence denials retain mode-specific responses. Off restores main’s empty-valued/valueless t behavior. DenyProtected rejects before proof verification.
• Source-only review on pinned Blox, with independent auth/privacy and contract lanes. Existing exact-head Unit, PostgreSQL and Relay E2E checks passed. Run Codex Security Review was cancelled, not a security-review pass. No PR code was executed during this review; the previously documented MinIO expiry-test coverage limitation remains.
• The relay-first distribution prerequisite remains: verify every supported relay contains repair 75e9bef748d2149ce459b14da842e706a51a5f78 before shipping 60-second-proof clients. Deployment verification is not established here. The inherited S4 deny-map gap and unchanged, previously reviewed client paths are not newly certified.

This is a bounded re-review, not whole-feature or rollout approval.