diff --git a/RELEASING.md b/RELEASING.md index 8d1fad74807..34fc9b49da4 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -89,6 +89,45 @@ prior release's recorded squash commit; tag ancestry is deliberately irrelevant. Every push to `main` continues to publish the rolling relay `:main` and `:sha-<7>` tags, plus matching `:debug-main` and `:debug-sha-<7>` variants. +#### NIP-FI upload proof freshness (relay floor) + +Desktop, CLI, and mobile clients mint Blossom upload authorization proofs with +a **60-second expiry** (`expiration = now + 60`). **Older** relays (≤ `v0.2.1`) +re-run the full auth verifier — including the expiry check — after receiving the +full upload body, so a 60-second proof that was valid when admitted can expire +during a large or slow transfer, causing a spurious rejection. + +The repair (commit `75e9bef748d2149ce459b14da842e706a51a5f78`, landed in this +PR) replaces the post-body full-verifier call with a hash-only check +(`verify_upload_hash_only`): the relay verifies the freshness and all other +proof fields before reading the body; the only check after transfer is that the +body's SHA-256 matches what was declared in the signed proof. + +Clients cannot lengthen proofs beyond 60 seconds (NIP-FI §Strict limits the +max proof window to 60 s). This is a relay-side concern: + +- **The relay release that includes this PR's changes** (the first relay + tag > `v0.2.1`, containing commit `75e9bef748d2149ce459b14da842e706a51a5f78`) + performs a hash-only post-body check, eliminating the expiry window during + transfer. **Update this line with the concrete relay tag once it is cut.** +- **Older self-hosted relays** (≤ `v0.2.1`) re-verify the full proof expiry + after transfer and may reject large uploads over slow connections. + Upgrading the relay is the fix; no client workaround exists within Strict + constraints. + +**Prerequisite before distributing desktop, CLI, or mobile builds that mint +60-second upload proofs:** verify that all supported relay environments have +been upgraded to a relay release containing commit +`75e9bef748d2149ce459b14da842e706a51a5f78`. An unknown or unverified deployment +does not satisfy this prerequisite. Deployments running ≤ `v0.2.1` must be +upgraded before receiving these clients. + +When releasing a new **relay** version that carries the post-body hash-only +check, record the concrete tag here so release operators can verify the +prerequisite. When releasing **desktop, CLI, or mobile** for environments where +self-hosted relays may be running pre-floor versions, document the floor in +release notes and require operators to upgrade before distributing these clients. + ### Mobile 1. **Publish a candidate.** From a clean checkout whose `origin` is the diff --git a/crates/buzz-cli/src/client.rs b/crates/buzz-cli/src/client.rs index 6ade19f1cad..54441f55cc9 100644 --- a/crates/buzz-cli/src/client.rs +++ b/crates/buzz-cli/src/client.rs @@ -327,7 +327,7 @@ fn sign_blossom_get(keys: &Keys, media_url: &str) -> Result { use nostr::Timestamp; let now = Timestamp::now().as_secs(); - let exp_str = (now + 600).to_string(); + let exp_str = (now + 60).to_string(); let domain = relay_server_tag(media_url) .ok_or_else(|| CliError::Usage(format!("invalid media URL: {media_url}")))?; let tags = vec![ @@ -350,28 +350,23 @@ fn sign_blossom_get(keys: &Keys, media_url: &str) -> Result { fn sign_blossom_upload( keys: &Keys, sha256: &str, - mime: &str, + _mime: &str, relay_url: &str, ) -> Result { use base64::engine::general_purpose::URL_SAFE_NO_PAD; use nostr::Timestamp; let now = Timestamp::now().as_secs(); - let expiry: u64 = if mime.starts_with("video/") { - 3600 - } else { - 600 - }; - let exp_str = (now + expiry).to_string(); + let exp_str = (now + 60).to_string(); + let domain = relay_server_tag(relay_url) + .ok_or_else(|| CliError::Usage(format!("invalid relay URL: {relay_url}")))?; - let mut tags = vec![ + let tags = vec![ Tag::parse(["t", "upload"]).map_err(|e| CliError::Other(e.to_string()))?, Tag::parse(["x", sha256]).map_err(|e| CliError::Other(e.to_string()))?, Tag::parse(["expiration", &exp_str]).map_err(|e| CliError::Other(e.to_string()))?, + Tag::parse(["server", &domain]).map_err(|e| CliError::Other(e.to_string()))?, ]; - if let Some(domain) = relay_server_tag(relay_url) { - tags.push(Tag::parse(["server", &domain]).map_err(|e| CliError::Other(e.to_string()))?); - } let auth_event = EventBuilder::new(Kind::from(24242), "Upload file") .tags(tags) diff --git a/crates/buzz-dev-mcp/src/view_image.rs b/crates/buzz-dev-mcp/src/view_image.rs index 441338ab127..8f8ea0ede76 100644 --- a/crates/buzz-dev-mcp/src/view_image.rs +++ b/crates/buzz-dev-mcp/src/view_image.rs @@ -49,8 +49,8 @@ pub(crate) const MAX_DECODER_ALLOC: u64 = 256 * 1024 * 1024; /// Connect + read timeout for URL fetches. const FETCH_TIMEOUT: Duration = Duration::from_secs(10); /// Lifetime of a Blossom `t=get` read token for relay media fetches. -/// Matches the desktop client's `MEDIA_GET_AUTH_EXPIRY_SECS`. -const MEDIA_GET_AUTH_EXPIRY_SECS: u64 = 600; +/// 60 seconds — matches the NIP-FI strict proof window. +const MEDIA_GET_AUTH_EXPIRY_SECS: u64 = 60; /// Build the decoder allocation cap. Centralised so the resize path uses the /// same value tests can reason about. diff --git a/crates/buzz-media/src/auth.rs b/crates/buzz-media/src/auth.rs index c6fff2be473..18b40c2cb9c 100644 --- a/crates/buzz-media/src/auth.rs +++ b/crates/buzz-media/src/auth.rs @@ -1,4 +1,4 @@ -//! Blossom kind:24242 auth verification (BUD-11 compliant). +//! Blossom kind:24242 auth verification (BUD-11 + NIP-FI compliant). use crate::error::MediaError; @@ -18,13 +18,46 @@ impl BlossomVerb { } } -/// Verify common kind:24242 Blossom auth event validity: +/// Verification strictness derived from the NIP-FI mode. +/// +/// `Strict` applies the full NIP-FI kind-24242 rules: 60-second proof window, +/// `expiration <= created_at + 60s`, mandatory `server` tag on all proofs, and +/// exact cardinality (exactly one each of `t`, `expiration`, `server`; at most +/// one `x`). +/// +/// `Permissive` preserves the pre-NIP-FI behavior for Off-mode deployments: +/// 3600-second proof window, `server` tag optional, and duplicate tags accepted. +/// This keeps a deployed desktop that mints old-shape tokens working against an +/// Off-mode relay [FI-INV-15]. +/// +/// The relay call sites derive strictness from `config.nip_fi.mode`; library +/// callers without mode access may pass `Strict` directly. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BlossomStrictness { + /// Full NIP-FI compliance: 60s window, mandatory `server`, exact cardinality. + Strict, + /// Pre-NIP-FI compatibility: 3600s window, optional `server`, tolerant cardinality. + Permissive, +} + +/// Verify common kind:24242 Blossom auth event validity. +/// +/// Checks in order: /// 1. Schnorr signature -/// 2. kind == 24242 -/// 3. `t` tag matches `verb` -/// 4. `expiration` tag in the future -/// 5. `created_at` in the past (with 5s clock-skew tolerance) -/// 6. If `server` tags present, our domain must appear in at least one +/// 2. kind == 24242 and non-empty content +/// 3. `t` tag matches `verb`: +/// - `Strict`: count every tag named `t` by field name; require exactly +/// one occurrence; require its content to be non-empty and equal to +/// `verb` (NIP-FI.md:658-666 — malformed/empty/duplicate instances MUST +/// be rejected as `evidence_rejected`). +/// - `Permissive`: at least one valid-valued `t` tag equal to `verb`; +/// valueless/empty tags are ignored (origin/main `found_t` semantics). +/// 4. `expiration` tag present, strictly future, and within the freshness window +/// 5. `created_at` bounded: not more than 5s in the future; not older than the +/// mode-selected window (60s `Strict`, 3600s `Permissive`) +/// 6. `server` tag enforcement: in `Strict` mode, exactly one `server` tag is +/// required and MUST match the bound tenant host; in `Permissive`, optional +/// when-present behavior is preserved /// /// Does NOT check verb-specific scope tags (`x` for upload, `x` OR `server` /// for get). Call this BEFORE trusting the event's pubkey for scope resolution. @@ -32,8 +65,10 @@ pub fn verify_blossom_auth_event_for_verb( auth_event: &nostr::Event, verb: BlossomVerb, server_domain: Option<&str>, - max_age_secs: u64, + strictness: BlossomStrictness, ) -> Result<(), MediaError> { + let strict = strictness == BlossomStrictness::Strict; + // 1. Verify Schnorr signature auth_event .verify() @@ -49,64 +84,141 @@ pub fn verify_blossom_auth_event_for_verb( return Err(MediaError::InvalidAuthEvent); } - let mut found_t = false; - let mut found_exp = false; - let mut server_tags: Vec<&str> = Vec::new(); + // Tag accumulation with strict cardinality tracking. + // In Strict mode: each of t/expiration/server must appear exactly once; + // x is counted separately for upload scope checking (at most one). + // In Permissive mode: boolean-style "found at least one" semantics, + // matching the pre-NIP-FI behavior. + let mut t_count: u8 = 0; + let mut exp_count: u8 = 0; + let mut server_count: u8 = 0; + let mut x_count: u8 = 0; + let mut exp_value: u64 = 0; + // All server tag values, collected for Permissive any-match semantics. + // Strict mode returns early on duplicate-server before reaching the + // validation block, so server_values[0] is always safe to use there. + let mut server_values: Vec = Vec::new(); for tag in auth_event.tags.iter() { let kind = tag.kind().to_string(); match kind.as_str() { "t" => { - if let Some(v) = tag.content() { - if v != verb.as_str() { - return Err(MediaError::InvalidAuthVerb); + if strict { + // Strict (NIP-FI): count EVERY tag named "t" by field name, + // regardless of its content. NIP-FI.md:658-666 requires + // that malformed, empty, duplicate, or conflicting instances + // be rejected as `evidence_rejected`. We count first, gate + // on cardinality, then validate content. + t_count = t_count.saturating_add(1); + if t_count > 1 { + return Err(MediaError::DuplicateTag("t")); + } + // Exactly one t tag: its content must be non-empty and + // equal to the requested verb. + match tag.content() { + Some(v) if !v.is_empty() && v == verb.as_str() => {} + _ => return Err(MediaError::InvalidAuthVerb), + } + } else { + // Permissive: only valid-valued matching tags count + // (origin/main's `found_t` semantics). Valueless/empty + // tags are ignored for cardinality; wrong-verb tags reject. + if let Some(v) = tag.content() { + if v.is_empty() { + // Empty string value: does not satisfy the requirement. + } else if v != verb.as_str() { + return Err(MediaError::InvalidAuthVerb); + } else { + t_count = t_count.saturating_add(1); + } } - found_t = true; + // No content: tag is ignored (not counted, not rejected). } } "expiration" => { + exp_count = exp_count.saturating_add(1); + if strict && exp_count > 1 { + return Err(MediaError::DuplicateTag("expiration")); + } if let Some(v) = tag.content() { - exp_value = v.parse().unwrap_or(0); - found_exp = true; + if strict { + // Strict: first-wins (duplicate already rejected above). + if exp_count == 1 { + exp_value = v.parse().unwrap_or(0); + } + } else { + // Permissive: last-wins, matching pre-NIP-FI base + // behavior (base auth.rs processed each valued tag + // unconditionally, so a later tag overwrote earlier + // ones). A valueless tag followed by a future-valued + // tag must not be treated as expired [FI-INV-15]. + exp_value = v.parse().unwrap_or(0); + } } } "server" => { + server_count = server_count.saturating_add(1); + if strict && server_count > 1 { + return Err(MediaError::DuplicateTag("server")); + } + // Collect all valued server tags for Permissive any-match; Strict + // will only ever have at most one here (duplicate check above). if let Some(v) = tag.content() { - server_tags.push(v); + server_values.push(v.to_owned()); + } + } + "x" => { + x_count = x_count.saturating_add(1); + if strict && x_count > 1 { + return Err(MediaError::DuplicateTag("x")); } } _ => {} } } - // 3. t tag required - if !found_t { + // 3. t tag required (exactly one in Strict, at least one in Permissive) + if t_count == 0 { return Err(MediaError::MissingTag("t")); } - // 4. Expiration must exist and be in the future - if !found_exp { + // 4a. Expiration must exist + if exp_count == 0 { return Err(MediaError::MissingTag("expiration")); } let now = nostr::Timestamp::now().as_secs(); + + // 4b. Expiration must be strictly in the future if exp_value <= now { return Err(MediaError::TokenExpired); } - // 5. created_at must be recent: not in the future (5s tolerance) and not - // older than 10 minutes. This bounds the replay window — even if the - // expiration tag allows a longer lifetime, the token must have been - // freshly minted. + // 5. created_at freshness bounds. + // + // Strict (NIP-FI active modes): + // created_at <= now + 5s — bounded future skew + // now - created_at <= 60s — 60-second replay window + // expiration <= created_at + 60s — token cannot outlive its window + // + // Permissive (Off mode): + // created_at <= now + 5s — future skew only + // now - created_at <= 3600s — 1-hour replay window (pre-NIP-FI) + // expiration window not enforced let created = auth_event.created_at.as_secs(); if created > now + 5 { return Err(MediaError::TimestampOutOfWindow); } - if now > created + max_age_secs { + let max_age = if strict { 60u64 } else { 3600u64 }; + if now > created + max_age { + return Err(MediaError::TimestampOutOfWindow); + } + if strict && exp_value > created + 60 { + // expiration tag must satisfy expiration <= created_at + 60s return Err(MediaError::TimestampOutOfWindow); } - // 6. Server tag enforcement (BUD-11 §5): if server tags present, our host must appear. + // 6. Server tag enforcement. // // `server_domain` is the host this request was bound to — the per-request // tenant host (`TenantContext::host()`), NOT a single process-global domain. @@ -117,21 +229,59 @@ pub fn verify_blossom_auth_event_for_verb( // construction across case, trailing dot, default ports, and an optional // URL scheme/path — exactly as every other host seam resolves tenants. // - // Fail closed: if the bound host is unknown, reject tokens that carry server - // tags rather than silently accepting them. - if !server_tags.is_empty() { - match server_domain { - Some(domain) => { - let want = normalize_server_host(domain); - let matches = server_tags - .iter() - .any(|tag| normalize_server_host(tag) == want); - if !matches { + // Strict (NIP-FI active): exactly one server tag MUST be present and MUST + // match the bound tenant host. Absent or mismatched → evidence_rejected. + // + // Permissive (Off mode): if ANY server tag is present, at least one MUST + // match our host (any-match semantics, preserving origin/main behavior); + // absent server tags are accepted [FI-INV-15]. + if strict { + // Strict: server_values has at most one entry (duplicate rejected above). + match ( + server_count, + server_values.first().map(String::as_str), + server_domain, + ) { + (0, _, _) => { + // Strict: server tag mandatory + return Err(MediaError::ServerMismatch); + } + (_, Some(tag_host), Some(domain)) => { + if normalize_server_host(tag_host) != normalize_server_host(domain) { return Err(MediaError::ServerMismatch); } } - None => { - // Server tags present but we don't know our own host — reject. + (_, _, None) => { + // Strict: bound host unknown — fail closed + return Err(MediaError::ServerMismatch); + } + (_, None, _) => { + // server tag present but valueless — treat as mismatch + return Err(MediaError::ServerMismatch); + } + } + } else { + // Permissive: validate only when VALUED server tags are present. + // A valueless `["server"]` tag does not constitute a server-tag binding + // and must not change the admission decision (FI-INV-15: Permissive/Off + // must preserve pre-NIP-FI base behavior, which gated on the collected + // valued-tag list being nonempty). Using `server_values.is_empty()` + // (not `server_count > 0`) ensures a lone valueless server tag is + // treated the same as no server tag — the proof is admitted without a + // server check, matching origin/main behavior. + // + // Any-match semantics: a proof with multiple VALUED server tags is + // accepted if at least one matches our host. + if !server_values.is_empty() { + let Some(domain) = server_domain else { + // Server tags present but our host is unknown — fail closed. + return Err(MediaError::ServerMismatch); + }; + let want = normalize_server_host(domain); + let any_match = server_values + .iter() + .any(|tag_host| normalize_server_host(tag_host) == want); + if !any_match { return Err(MediaError::ServerMismatch); } } @@ -140,6 +290,28 @@ pub fn verify_blossom_auth_event_for_verb( Ok(()) } +/// Verify only the `x` tag hash match on an already-admitted upload auth event. +/// +/// This is the post-body hash check: the full auth event verification +/// (signature, kind, freshness, cardinality, server) was already performed at +/// the pre-body admission gate. Re-running the full verifier after streaming +/// a potentially large body would fail for any upload that takes longer than +/// the minted token's `expiration` window (typically 60 s in Strict mode). +/// +/// The ONLY thing that is unknown before the body is transferred is the +/// content hash (`x` tag). This function confirms that the body's SHA-256 +/// matches what was declared in the signed proof. +pub fn verify_upload_hash_only(auth_event: &nostr::Event, sha256: &str) -> Result<(), MediaError> { + let has_matching_x = auth_event + .tags + .iter() + .any(|tag| tag.kind().to_string() == "x" && tag.content() == Some(sha256)); + if !has_matching_x { + return Err(MediaError::HashMismatch); + } + Ok(()) +} + /// Verify common upload auth event validity. /// /// Kept as the upload-shaped public wrapper for existing callers; new verb-aware @@ -147,9 +319,9 @@ pub fn verify_blossom_auth_event_for_verb( pub fn verify_blossom_auth_event( auth_event: &nostr::Event, server_domain: Option<&str>, - max_age_secs: u64, + strictness: BlossomStrictness, ) -> Result<(), MediaError> { - verify_blossom_auth_event_for_verb(auth_event, BlossomVerb::Upload, server_domain, max_age_secs) + verify_blossom_auth_event_for_verb(auth_event, BlossomVerb::Upload, server_domain, strictness) } /// Normalize a Blossom `server` tag value (or a bound tenant host) into the @@ -170,22 +342,19 @@ fn normalize_server_host(value: &str) -> String { /// Verify a kind:24242 Blossom upload auth event, including the x tag hash check. /// -/// Calls [`verify_blossom_auth_event`] first, then verifies that at least one -/// `x` tag matches `sha256` (BUD-11 §6: "at least one x tag matches"). +/// In `Strict` mode (NIP-FI active): exactly one `x` tag MUST be present and +/// MUST match `sha256`. In `Permissive` mode: at least one `x` tag must match. pub fn verify_blossom_upload_auth( auth_event: &nostr::Event, sha256: &str, server_domain: Option<&str>, - max_age_secs: u64, + strictness: BlossomStrictness, ) -> Result<(), MediaError> { - verify_blossom_auth_event_for_verb( - auth_event, - BlossomVerb::Upload, - server_domain, - max_age_secs, - )?; - - // At least one x tag must match the body sha256 (BUD-11 §6) + verify_blossom_auth_event_for_verb(auth_event, BlossomVerb::Upload, server_domain, strictness)?; + + // Upload: x tag must match the body sha256. + // Strict: exactly one x tag and it must match (duplicate x caught above). + // Permissive: at least one x tag must match. let has_matching_x = auth_event .tags .iter() @@ -204,18 +373,39 @@ pub fn verify_blossom_upload_auth( /// or server-scoped authorization (`server` tag matches this relay host). The /// latter intentionally grants reads for all blobs on the host until expiration; /// callers must still apply relay membership after this verifier returns. +/// +/// In `Strict` mode the `server` tag is mandatory (enforced by the base verifier); +/// `x` is optional on reads. An `x` tag, when present, must be exactly one and +/// must match `sha256`. pub fn verify_blossom_get_auth( auth_event: &nostr::Event, sha256: &str, server_domain: Option<&str>, - max_age_secs: u64, + strictness: BlossomStrictness, ) -> Result<(), MediaError> { - verify_blossom_auth_event_for_verb(auth_event, BlossomVerb::Get, server_domain, max_age_secs)?; + verify_blossom_auth_event_for_verb(auth_event, BlossomVerb::Get, server_domain, strictness)?; - let has_matching_x = auth_event + // x tag scope check for get: if an x tag is present it must match sha256. + // In Strict mode duplicate x is already rejected above; here we check the value. + // In Permissive mode we use the original "any matching x OR matching server" logic. + + // Count ALL x tags (including valueless) — presence of a valueless x in Strict + // mode is a malformed proof and must not become absent scope. + let x_tag_count: usize = auth_event .tags .iter() - .any(|tag| tag.kind().to_string() == "x" && (tag.content() == Some(sha256))); + .filter(|tag| tag.kind().to_string() == "x") + .count(); + + // Collect only valued x tags for hash comparison. + let x_tags: Vec<&str> = auth_event + .tags + .iter() + .filter(|tag| tag.kind().to_string() == "x") + .filter_map(|tag| tag.content()) + .collect(); + + let has_matching_x = x_tags.contains(&sha256); let has_matching_server = match server_domain { Some(domain) => { @@ -231,8 +421,20 @@ pub fn verify_blossom_get_auth( None => false, }; - if !has_matching_x && !has_matching_server { - return Err(MediaError::InsufficientScope); + if strictness == BlossomStrictness::Strict { + // Strict: server is already validated as present+matching by the base verifier. + // If any x tag is present (including a valueless one), it MUST contain the + // exact requested sha256 — a valueless or mismatched x is malformed evidence + // (evidence_rejected). No x tags → server-scoped read, admitted here. + if x_tag_count > 0 && !has_matching_x { + return Err(MediaError::ServerMismatch); + } + // Server-scoped read (no x) is always admitted here — server was validated above. + } else { + // Permissive: original BUD-01 semantics — matching x OR matching server. + if !has_matching_x && !has_matching_server { + return Err(MediaError::InsufficientScope); + } } Ok(()) @@ -242,8 +444,25 @@ pub fn verify_blossom_get_auth( mod tests { use super::*; use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; + use sha2::Digest as _; fn build_valid_auth(keys: &Keys, sha256: &str) -> nostr::Event { + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(keys) + .unwrap() + } + + fn build_permissive_auth(keys: &Keys, sha256: &str) -> nostr::Event { + // Old-shape token: no server tag, 300s expiry — valid in Permissive, rejected in Strict. let now = Timestamp::now().as_secs(); let exp_str = (now + 300).to_string(); let tags = vec![ @@ -257,296 +476,1348 @@ mod tests { .unwrap() } + // ── Strict mode ────────────────────────────────────────────────────────── + #[test] - fn test_verify_valid() { + fn test_verify_valid_strict() { let keys = Keys::generate(); let sha256 = "a".repeat(64); let event = build_valid_auth(&keys, &sha256); - assert!(verify_blossom_upload_auth(&event, &sha256, None, 600).is_ok()); + assert!(verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ) + .is_ok()); } #[test] - fn test_verify_auth_event_valid() { + fn test_verify_auth_event_valid_strict() { let keys = Keys::generate(); let sha256 = "a".repeat(64); let event = build_valid_auth(&keys, &sha256); - assert!(verify_blossom_auth_event(&event, None, 600).is_ok()); + assert!(verify_blossom_auth_event( + &event, + Some("relay.example"), + BlossomStrictness::Strict + ) + .is_ok()); } - fn build_get_auth(keys: &Keys, tags: Vec) -> nostr::Event { - EventBuilder::new(Kind::from(24242), "Get buzz-media") - .tags(tags) - .sign_with_keys(keys) - .unwrap() - } + // ── Permissive mode (Off-mode regression guard [FI-INV-15]) ────────────── #[test] - fn test_verify_get_accepts_matching_x_without_server_tag() { + fn test_verify_permissive_old_shape_token_admitted() { + // Old-shape token (no server tag, 300s expiry) MUST be admitted in Permissive mode. let keys = Keys::generate(); let sha256 = "a".repeat(64); - let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); - let event = build_get_auth( - &keys, - vec![ - Tag::parse(["t", "get"]).unwrap(), - Tag::parse(["x", &sha256]).unwrap(), - Tag::parse(["expiration", &exp_str]).unwrap(), - ], + let event = build_permissive_auth(&keys, &sha256); + assert!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ) + .is_ok(), + "Off-mode must admit old-shape tokens without server tag [FI-INV-15]" ); - - assert!(verify_blossom_get_auth(&event, &sha256, Some("relay.example"), 600).is_ok()); } #[test] - fn test_verify_get_accepts_matching_server_without_x_tag() { + fn test_verify_permissive_long_expiry_admitted() { + // 600s expiry token (pre-NIP-FI desktop default) MUST be admitted in Permissive mode. let keys = Keys::generate(); let sha256 = "a".repeat(64); let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); - let event = build_get_auth( - &keys, - vec![ - Tag::parse(["t", "get"]).unwrap(), - Tag::parse(["server", "https://Relay.Example./media/ignored"]).unwrap(), - Tag::parse(["expiration", &exp_str]).unwrap(), - ], + let exp_str = (now + 600).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ) + .is_ok(), + "Off-mode must admit 600s expiry tokens [FI-INV-15]" ); - - assert!(verify_blossom_get_auth(&event, &sha256, Some("relay.example"), 600).is_ok()); } + // ── Cardinality: Strict rejects duplicates ──────────────────────────────── + #[test] - fn test_verify_get_rejects_upload_verb() { + fn test_strict_rejects_duplicate_t_tag() { let keys = Keys::generate(); let sha256 = "a".repeat(64); - let event = build_valid_auth(&keys, &sha256); - + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["t", "upload"]).unwrap(), // duplicate + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); assert!(matches!( - verify_blossom_get_auth(&event, &sha256, Some("relay.example"), 600), - Err(MediaError::InvalidAuthVerb) + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::DuplicateTag("t")) )); } #[test] - fn test_verify_get_requires_x_or_server_scope() { + fn test_strict_rejects_duplicate_expiration_tag() { let keys = Keys::generate(); let sha256 = "a".repeat(64); - let other_hash = "b".repeat(64); let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); - let event = build_get_auth( - &keys, - vec![ - Tag::parse(["t", "get"]).unwrap(), - Tag::parse(["x", &other_hash]).unwrap(), - Tag::parse(["expiration", &exp_str]).unwrap(), - ], - ); - + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), // duplicate + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); assert!(matches!( - verify_blossom_get_auth(&event, &sha256, Some("relay.example"), 600), - Err(MediaError::InsufficientScope) + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::DuplicateTag("expiration")) )); } #[test] - fn test_verify_get_rejects_wrong_server_scope() { + fn test_strict_rejects_duplicate_server_tag() { let keys = Keys::generate(); let sha256 = "a".repeat(64); let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); - let event = build_get_auth( - &keys, - vec![ - Tag::parse(["t", "get"]).unwrap(), - Tag::parse(["server", "other.example"]).unwrap(), - Tag::parse(["expiration", &exp_str]).unwrap(), - ], - ); - + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), // duplicate + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); assert!(matches!( - verify_blossom_get_auth(&event, &sha256, Some("relay.example"), 600), - Err(MediaError::ServerMismatch) + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::DuplicateTag("server")) )); } #[test] - fn test_verify_hash_mismatch() { + fn test_strict_rejects_duplicate_x_tag() { let keys = Keys::generate(); let sha256 = "a".repeat(64); - let event = build_valid_auth(&keys, &sha256); - let wrong_hash = "b".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), // duplicate + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); assert!(matches!( - verify_blossom_upload_auth(&event, &wrong_hash, None, 600), - Err(MediaError::HashMismatch) + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::DuplicateTag("x")) )); } + // ── Permissive mode: duplicate tags still admitted ──────────────────────── + #[test] - fn test_verify_wrong_kind() { + fn test_permissive_admits_duplicate_x_tag() { let keys = Keys::generate(); let sha256 = "a".repeat(64); let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); + let exp_str = (now + 55).to_string(); let tags = vec![ Tag::parse(["t", "upload"]).unwrap(), Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), // duplicate — permitted in Permissive Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), ]; - let event = EventBuilder::new(Kind::from(27235), "wrong kind") + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") .tags(tags) .sign_with_keys(&keys) .unwrap(); - assert!(matches!( - verify_blossom_upload_auth(&event, &sha256, None, 600), - Err(MediaError::InvalidAuthKind) - )); + assert!(verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ) + .is_ok()); } + // ── Strict: server tag mandatory ───────────────────────────────────────── + #[test] - fn test_verify_multi_x_tags() { + fn test_strict_rejects_absent_server_on_upload() { let keys = Keys::generate(); let sha256 = "a".repeat(64); - let other_hash = "b".repeat(64); let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); + let exp_str = (now + 55).to_string(); let tags = vec![ Tag::parse(["t", "upload"]).unwrap(), - Tag::parse(["x", &other_hash]).unwrap(), Tag::parse(["x", &sha256]).unwrap(), Tag::parse(["expiration", &exp_str]).unwrap(), + // no server tag ]; - let event = EventBuilder::new(Kind::from(24242), "Upload multi-x") + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") .tags(tags) .sign_with_keys(&keys) .unwrap(); - // Should pass because at least one x tag matches - assert!(verify_blossom_upload_auth(&event, &sha256, None, 600).is_ok()); + assert!( + matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::ServerMismatch) + ), + "Strict mode must reject upload proof without server tag" + ); } #[test] - fn test_server_tag_enforcement() { + fn test_strict_rejects_absent_server_on_read() { let keys = Keys::generate(); let sha256 = "a".repeat(64); let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); + let exp_str = (now + 55).to_string(); let tags = vec![ - Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["t", "get"]).unwrap(), Tag::parse(["x", &sha256]).unwrap(), Tag::parse(["expiration", &exp_str]).unwrap(), - Tag::parse(["server", "other.example.com"]).unwrap(), + // no server tag ]; - let event = EventBuilder::new(Kind::from(24242), "Upload scoped") + let event = EventBuilder::new(Kind::from(24242), "Get buzz-media") .tags(tags) .sign_with_keys(&keys) .unwrap(); - // Should fail — server tag present but doesn't match our domain - assert!(matches!( - verify_blossom_upload_auth(&event, &sha256, Some("buzz.example.com"), 600), - Err(MediaError::ServerMismatch) - )); - // Should pass when our domain matches assert!( - verify_blossom_upload_auth(&event, &sha256, Some("other.example.com"), 600).is_ok() + matches!( + verify_blossom_get_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::ServerMismatch) + ), + "Strict mode must reject read proof without server tag" ); - // Should fail when server_domain is None — fail closed - assert!(matches!( - verify_blossom_upload_auth(&event, &sha256, None, 600), - Err(MediaError::ServerMismatch) - )); } + // ── Strict: freshness window ────────────────────────────────────────────── + #[test] - fn test_no_server_tags_always_passes() { + fn test_strict_rejects_expiration_exceeding_60s_window() { let keys = Keys::generate(); let sha256 = "a".repeat(64); - let event = build_valid_auth(&keys, &sha256); - // No server tags → passes regardless of our domain - assert!(verify_blossom_upload_auth(&event, &sha256, Some("any.domain.com"), 600).is_ok()); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 61).to_string(); // 61s > 60s limit + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::TimestampOutOfWindow) + ), + "Strict mode must reject expiration > created_at + 60s" + ); } - /// A `server` tag is matched against the *bound tenant host* under the - /// shared `normalize_host` rule, so equivalent host spellings agree — the - /// stock CLI's bare `host:port`, an explicit default port, a trailing dot, - /// mixed case, and a full URL all match the same bound host. This is the - /// regression guard for the multi-tenant media blocker: a non-primary - /// tenant must accept its own server-tagged client. #[test] - fn test_server_tag_normalized_against_bound_host() { + fn test_strict_admits_60s_expiration() { let keys = Keys::generate(); let sha256 = "a".repeat(64); let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); - let build = |server: &str| { - let tags = vec![ - Tag::parse(["t", "upload"]).unwrap(), - Tag::parse(["x", &sha256]).unwrap(), - Tag::parse(["expiration", &exp_str]).unwrap(), - Tag::parse(["server", server]).unwrap(), - ]; - EventBuilder::new(Kind::from(24242), "Upload scoped") - .tags(tags) - .sign_with_keys(&keys) - .unwrap() - }; - - // Non-primary tenant host with explicit non-default port (the live - // repro: tenant B on 127.0.0.1:3100). Stock CLI tags `host:port`. - assert!(verify_blossom_upload_auth( - &build("127.0.0.1:3100"), - &sha256, - Some("127.0.0.1:3100"), - 600 + let exp_str = (now + 60).to_string(); // exactly 60s — allowed + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ) + .is_ok(), + "Strict mode must admit expiration == created_at + 60s" + ); + } + + // ── Get auth ───────────────────────────────────────────────────────────── + + fn build_get_auth(keys: &Keys, tags: Vec) -> nostr::Event { + EventBuilder::new(Kind::from(24242), "Get buzz-media") + .tags(tags) + .sign_with_keys(keys) + .unwrap() + } + + #[test] + fn test_verify_get_accepts_matching_server_strict() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let event = build_get_auth( + &keys, + vec![ + Tag::parse(["t", "get"]).unwrap(), + Tag::parse(["server", "https://Relay.Example./media/ignored"]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ], + ); + assert!(verify_blossom_get_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict ) .is_ok()); + } - // Equivalence under normalize_host: explicit default port, trailing - // dot, mixed case, and a full URL all collapse to the bound host. - for tag in [ - "Relay.Example:443", - "relay.example.", - "RELAY.EXAMPLE", - "https://relay.example/", - ] { - assert!( - verify_blossom_upload_auth(&build(tag), &sha256, Some("relay.example"), 600) - .is_ok(), - "server tag {tag:?} should match bound host relay.example" - ); - } + #[test] + fn test_verify_get_accepts_matching_x_and_server_strict() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let event = build_get_auth( + &keys, + vec![ + Tag::parse(["t", "get"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ], + ); + assert!(verify_blossom_get_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ) + .is_ok()); + } - // A different tenant host still fails closed. + #[test] + fn test_verify_get_strict_rejects_mismatched_x_with_valid_server() { + // x present but wrong hash → evidence_rejected even if server matches + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let other = "b".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let event = build_get_auth( + &keys, + vec![ + Tag::parse(["t", "get"]).unwrap(), + Tag::parse(["x", &other]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ], + ); assert!(matches!( - verify_blossom_upload_auth( - &build("127.0.0.1:3100"), + verify_blossom_get_auth( + &event, &sha256, - Some("127.0.0.1:3200"), - 600 + Some("relay.example"), + BlossomStrictness::Strict ), Err(MediaError::ServerMismatch) )); } #[test] - fn test_empty_content_rejected() { + fn test_verify_get_accepts_matching_x_without_server_permissive() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let event = build_get_auth( + &keys, + vec![ + Tag::parse(["t", "get"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ], + ); + assert!(verify_blossom_get_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ) + .is_ok()); + } + + #[test] + fn test_verify_get_accepts_matching_server_without_x_permissive() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let event = build_get_auth( + &keys, + vec![ + Tag::parse(["t", "get"]).unwrap(), + Tag::parse(["server", "https://Relay.Example./media/ignored"]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ], + ); + assert!(verify_blossom_get_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ) + .is_ok()); + } + + #[test] + fn test_verify_get_rejects_upload_verb() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let event = build_valid_auth(&keys, &sha256); + assert!(matches!( + verify_blossom_get_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ), + Err(MediaError::InvalidAuthVerb) + )); + } + + #[test] + fn test_verify_get_requires_x_or_server_scope_permissive() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let other_hash = "b".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let event = build_get_auth( + &keys, + vec![ + Tag::parse(["t", "get"]).unwrap(), + Tag::parse(["x", &other_hash]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ], + ); + assert!(matches!( + verify_blossom_get_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ), + Err(MediaError::InsufficientScope) + )); + } + + #[test] + fn test_verify_get_rejects_wrong_server_scope() { let keys = Keys::generate(); let sha256 = "a".repeat(64); let now = Timestamp::now().as_secs(); let exp_str = (now + 300).to_string(); + let event = build_get_auth( + &keys, + vec![ + Tag::parse(["t", "get"]).unwrap(), + Tag::parse(["server", "other.example"]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ], + ); + assert!(matches!( + verify_blossom_get_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ), + Err(MediaError::ServerMismatch) + )); + } + + #[test] + fn test_verify_hash_mismatch() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let event = build_valid_auth(&keys, &sha256); + let wrong_hash = "b".repeat(64); + assert!(matches!( + verify_blossom_upload_auth( + &event, + &wrong_hash, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::HashMismatch) + )); + } + + #[test] + fn test_verify_wrong_kind() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); let tags = vec![ Tag::parse(["t", "upload"]).unwrap(), Tag::parse(["x", &sha256]).unwrap(), Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), ]; - // Empty content — BUD-11 requires a human-readable string - let event = EventBuilder::new(Kind::from(24242), "") + let event = EventBuilder::new(Kind::from(27235), "wrong kind") .tags(tags) .sign_with_keys(&keys) .unwrap(); assert!(matches!( - verify_blossom_auth_event(&event, None, 600), - Err(MediaError::InvalidAuthEvent) + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::InvalidAuthKind) + )); + } + + #[test] + fn test_server_tag_enforcement_permissive() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "other.example.com"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload scoped") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + // Should fail — server tag present but doesn't match our domain + assert!(matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("buzz.example.com"), + BlossomStrictness::Permissive + ), + Err(MediaError::ServerMismatch) + )); + // Should pass when our domain matches + assert!(verify_blossom_upload_auth( + &event, + &sha256, + Some("other.example.com"), + BlossomStrictness::Permissive + ) + .is_ok()); + // Should fail when server_domain is None — fail closed + assert!(matches!( + verify_blossom_upload_auth(&event, &sha256, None, BlossomStrictness::Permissive), + Err(MediaError::ServerMismatch) )); } + + #[test] + fn test_permissive_no_server_tags_always_passes() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let event = build_permissive_auth(&keys, &sha256); + // No server tags → passes regardless of our domain in Permissive mode + assert!(verify_blossom_upload_auth( + &event, + &sha256, + Some("any.domain.com"), + BlossomStrictness::Permissive + ) + .is_ok()); + } + + /// A `server` tag is matched against the *bound tenant host* under the + /// shared `normalize_host` rule, so equivalent host spellings agree. + #[test] + fn test_server_tag_normalized_against_bound_host() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let build = |server: &str| { + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", server]).unwrap(), + ]; + EventBuilder::new(Kind::from(24242), "Upload scoped") + .tags(tags) + .sign_with_keys(&keys) + .unwrap() + }; + + // Non-primary tenant host with explicit non-default port. + assert!(verify_blossom_upload_auth( + &build("127.0.0.1:3100"), + &sha256, + Some("127.0.0.1:3100"), + BlossomStrictness::Strict + ) + .is_ok()); + + // Equivalence under normalize_host + for tag in [ + "Relay.Example:443", + "relay.example.", + "RELAY.EXAMPLE", + "https://relay.example/", + ] { + assert!( + verify_blossom_upload_auth( + &build(tag), + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ) + .is_ok(), + "server tag {tag:?} should match bound host relay.example" + ); + } + + // A different tenant host still fails closed. + assert!(matches!( + verify_blossom_upload_auth( + &build("127.0.0.1:3100"), + &sha256, + Some("127.0.0.1:3200"), + BlossomStrictness::Strict + ), + Err(MediaError::ServerMismatch) + )); + } + + #[test] + fn test_empty_content_rejected() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + // Empty content — BUD-11 requires a human-readable string + let event = EventBuilder::new(Kind::from(24242), "") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!(matches!( + verify_blossom_auth_event(&event, Some("relay.example"), BlossomStrictness::Strict), + Err(MediaError::InvalidAuthEvent) + )); + } + + // ── Finding 3: valueless / empty-string t tag must not satisfy verb binding ── + + /// A `t` tag with no content (`["t"]`) in Strict mode: counted as one occurrence, + /// content check fires → `InvalidAuthVerb` (malformed instance, NIP-FI §cardinality). + #[test] + fn test_strict_rejects_valueless_t_tag() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + // ["t"] with no second element — no content, must be rejected + let tags = vec![ + Tag::parse(["t"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::InvalidAuthVerb) + ), + "valueless t tag must be rejected in Strict mode (counted once, content invalid)" + ); + } + + #[test] + fn test_valueless_t_tag_is_not_counted_permissive() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let tags = vec![ + Tag::parse(["t"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ), + Err(MediaError::MissingTag("t")) + ), + "valueless t tag must not satisfy t requirement in Permissive mode" + ); + } + + /// A `t` tag with an empty-string value (`["t", ""]`) in Strict mode: counted + /// as one occurrence, content check fires → `InvalidAuthVerb`. + #[test] + fn test_strict_rejects_empty_string_t_tag() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", ""]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::InvalidAuthVerb) + ), + "empty-string t tag must be rejected in Strict mode (counted once, content invalid)" + ); + } + + /// A valueless `x` tag on upload (`["x"]`) does not match any sha256. + #[test] + fn test_valueless_x_tag_does_not_match_hash() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x"]).unwrap(), // no value + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::HashMismatch) + ), + "valueless x tag must not satisfy hash requirement" + ); + } + + // ── Finding 3 R2: mixed malformed+valid t combos must reject in Strict ──── + + /// `["t"]` (valueless) + `["t","upload"]` in Strict: the malformed tag is counted + /// and content-validated on first encounter; the exact rejection error depends on + /// tag ordering but any error is correct — no combination may be admitted. + /// (If malformed comes first: `InvalidAuthVerb`; if valid first: `DuplicateTag("t")`.) + #[test] + fn test_strict_rejects_valueless_plus_valid_t_combo() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t"]).unwrap(), // valueless — counted in Strict + Tag::parse(["t", "upload"]).unwrap(), // valid — but two t tags total + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload bypass attempt") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + let result = verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict, + ); + assert!( + result.is_err(), + "valueless+valid t combo must be rejected in Strict mode, got Ok" + ); + // The first tag is valueless → InvalidAuthVerb fires before the second tag + // is seen; if ordering were reversed it would be DuplicateTag("t"). Both are + // valid evidence_rejected-class outcomes — what matters is admission is denied. + assert!( + matches!( + result, + Err(MediaError::InvalidAuthVerb) | Err(MediaError::DuplicateTag("t")) + ), + "expected InvalidAuthVerb or DuplicateTag(t), got unexpected error variant" + ); + } + + /// `["t",""]` (empty-string) + `["t","upload"]` in Strict: same reasoning — + /// any error is correct; admission must be denied. + #[test] + fn test_strict_rejects_empty_plus_valid_t_combo() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", ""]).unwrap(), // empty-string — counted in Strict + Tag::parse(["t", "upload"]).unwrap(), // valid — but two t tags total + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload bypass attempt") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + let result = verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict, + ); + assert!( + result.is_err(), + "empty-string+valid t combo must be rejected in Strict mode, got Ok" + ); + assert!( + matches!( + result, + Err(MediaError::InvalidAuthVerb) | Err(MediaError::DuplicateTag("t")) + ), + "expected InvalidAuthVerb or DuplicateTag(t), got unexpected error variant" + ); + } + + /// `["x"]` (valueless) + `["x", sha256]` (valid) in Strict: `x_count` increments + /// unconditionally for both occurrences → `DuplicateTag("x")`. This confirms + /// the x arm already handles mixed malformed+valid correctly. + #[test] + fn test_strict_rejects_valueless_plus_valid_x_combo() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x"]).unwrap(), // valueless — counted unconditionally + Tag::parse(["x", &sha256]).unwrap(), // valid — but makes x_count = 2 + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload bypass attempt") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ), + Err(MediaError::DuplicateTag("x")) + ), + "valueless+valid x combo must be rejected as DuplicateTag in Strict mode" + ); + } + + // ── Finding 1 (F1): Permissive multi-server any-match regression ───────── + + /// Permissive: `[[\"server\",\"other.example\"],[\"server\",\"relay.example\"]]` — + /// relay.example is second; must be admitted (any-match). Restores origin/main semantics. + #[test] + fn test_permissive_multi_server_second_matches() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "other.example"]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload multi-server") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ) + .is_ok(), + "Permissive must accept multi-server proof when any server matches (relay.example second)" + ); + } + + /// Permissive: `[[\"server\",\"relay.example\"],[\"server\",\"other.example\"]]` — + /// relay.example is first; must also be admitted. + #[test] + fn test_permissive_multi_server_first_matches() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + Tag::parse(["server", "other.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload multi-server") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ) + .is_ok(), + "Permissive must accept multi-server proof when any server matches (relay.example first)" + ); + } + + /// Permissive multi-server: neither server matches our host → still rejected. + #[test] + fn test_permissive_multi_server_none_match() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", "other.example"]).unwrap(), + Tag::parse(["server", "another.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload multi-server") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ), + Err(MediaError::ServerMismatch) + ), + "Permissive must reject multi-server proof when no server matches" + ); + } + + // ── Finding 4 (F4): valueless x tag on Strict get must not become host-wide scope ─ + + /// Strict get: proof with exactly one `[\"x\"]` (valueless) and a valid server — + /// the valueless x tag must not become absent scope (host-wide authorization). + /// Before the fix, `filter_map(tag.content())` dropped it silently and the read passed. + #[test] + fn test_strict_get_valueless_x_tag_does_not_grant_host_wide_scope() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "get"]).unwrap(), + Tag::parse(["x"]).unwrap(), // valueless — must not become absent scope + Tag::parse(["server", "relay.example"]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Get buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + // Must be rejected: valueless x is present but matches no sha256. + assert!( + verify_blossom_get_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ) + .is_err(), + "valueless x tag in Strict get must not grant host-wide read scope" + ); + } + + // ── Finding 1 compatibility edge (F1): valueless server tag must not change Permissive/Off behavior ─ + + /// Permissive: a proof with a valid matching `x`, a valid expiration, and a + /// lone VALUELESS `["server"]` tag (no content) was admitted by the pre-NIP-FI + /// base verifier, which gated server validation on the collected valued-tag list + /// being nonempty. The fix gates on `!server_values.is_empty()` (not + /// `server_count > 0`) so a valueless server tag is treated as absent. + /// + /// Before the fix, `server_count > 0` was true (count includes valueless tags), + /// entering the branch, and `server_values.iter().any(...)` over an empty vec + /// always returned false → `ServerMismatch` — a regression vs. base behavior. + #[test] + fn test_permissive_valueless_server_tag_does_not_change_admission() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + // Valid proof: matching x, valid expiration, lone valueless server tag. + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server"]).unwrap(), // valueless — must be ignored in Permissive + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + // Permissive: valueless server tag must not trigger ServerMismatch. + // The proof has a matching x tag so it must be admitted on the x-scope path. + assert!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ) + .is_ok(), + "Permissive must admit a proof whose only server tag is valueless — \ + valueless server tag must not change admission (FI-INV-15)" + ); + } + + /// Permissive (used for both Permissive and Off NIP-FI modes on the Blossom path): + /// same invariant as above, tested on its own to confirm the fix is not + /// order-dependent. Permissive is the only non-Strict variant; Off mode + /// selects Permissive strictness at the API layer. + #[test] + fn test_off_valueless_server_tag_does_not_change_admission() { + let keys = Keys::generate(); + let sha256 = "b".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server"]).unwrap(), // valueless + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + // BlossomStrictness::Off is represented as Permissive on the Blossom path. + assert!( + verify_blossom_upload_auth( + &event, + &sha256, + Some("relay.example"), + BlossomStrictness::Permissive + ) + .is_ok(), + "Off must admit a proof whose only server tag is valueless (FI-INV-15)" + ); + } + + // ── Upload expiry-boundary regression (commit 75e9bef748d2149ce459b14da842e706a51a5f78) ── + // + // Demonstrates the split between pre-body admission (freshness + full auth) + // and post-body verification (hash only via verify_upload_hash_only). + // + // Scenario: a 60-second Strict upload proof is valid at admit time. After a + // slow transfer, the proof's expiry has passed. The OLD full-verifier + // (verify_blossom_upload_auth called post-body) would reject the expired + // token, failing a legitimate large upload. The NEW post-body gate + // (verify_upload_hash_only) ignores freshness and accepts only the hash. + // + // Three cases exercise the production upload path contract: + // A. Admitted when fresh → pre-body gate (verify_blossom_auth_event_for_verb) accepts. + // B. Old full-verifier post-body → rejects even a valid hash if the token expired. + // C. New hash-only post-body → accepts the matching hash regardless of expiry. + // D. Hash-mismatch control → verify_upload_hash_only rejects a wrong hash. + + #[test] + fn test_upload_expiry_boundary_admission_before_expiry() { + // Case A: the pre-body gate must accept a fresh 60-second proof. + let keys = Keys::generate(); + let body = b"hello world"; + let sha256 = hex::encode(sha2::Sha256::digest(body)); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 60).to_string(); // exactly at the Strict ceiling + let server = "relay.example"; + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", server]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + verify_blossom_auth_event_for_verb( + &event, + BlossomVerb::Upload, + Some(server), + BlossomStrictness::Strict, + ) + .is_ok(), + "pre-body gate must admit a fresh 60-second Strict upload proof" + ); + } + + #[test] + fn test_upload_expiry_boundary_old_verifier_rejects_expired_post_body() { + // Case B: the OLD full-verifier rejects an expired token post-body, even + // when the hash matches. This is the failure the hash-only split fixes. + let keys = Keys::generate(); + let body = b"hello world"; + let sha256 = hex::encode(sha2::Sha256::digest(body)); + let server = "relay.example"; + // Token expired in the past — simulates a proof that was valid at admit + // time but whose expiry passed during a slow body transfer. + let past = Timestamp::now().as_secs().saturating_sub(10); + let exp_str = past.to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", server]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + // Full verifier (old post-body path) rejects the expired token. + assert!( + matches!( + verify_blossom_upload_auth( + &event, + &sha256, + Some(server), + BlossomStrictness::Strict + ), + Err(MediaError::TokenExpired) + ), + "old full-verifier must reject an expired token even when the hash matches" + ); + } + + #[test] + fn test_upload_expiry_boundary_hash_only_accepts_expired_after_transfer() { + // Case C: the NEW hash-only post-body gate accepts the matching hash + // regardless of expiry. A slow-transfer large upload survives. + let keys = Keys::generate(); + let body = b"hello world"; + let sha256 = hex::encode(sha2::Sha256::digest(body)); + let server = "relay.example"; + let past = Timestamp::now().as_secs().saturating_sub(10); + let exp_str = past.to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", server]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + // Hash-only post-body gate (new path): ignores freshness, accepts matching hash. + assert!( + verify_upload_hash_only(&event, &sha256).is_ok(), + "hash-only post-body gate must accept a matching hash regardless of expiry" + ); + } + + #[test] + fn test_upload_expiry_boundary_hash_mismatch_rejected() { + // Case D: verify_upload_hash_only rejects a wrong hash (security control). + let keys = Keys::generate(); + let body = b"hello world"; + let correct_sha256 = hex::encode(sha2::Sha256::digest(body)); + let wrong_sha256 = "a".repeat(64); + let server = "relay.example"; + let now = Timestamp::now().as_secs(); + let exp_str = (now + 60).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &correct_sha256]).unwrap(), + Tag::parse(["expiration", &exp_str]).unwrap(), + Tag::parse(["server", server]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + // Presenting the WRONG body hash — the proof is for a different blob. + assert!( + matches!( + verify_upload_hash_only(&event, &wrong_sha256), + Err(MediaError::HashMismatch) + ), + "hash-only gate must reject a body whose SHA-256 does not match the proof's x tag" + ); + } + + #[test] + fn test_permissive_valueless_expiration_then_valued_uses_last_wins() { + // FI-INV-15: Permissive mode must use last-wins expiration semantics + // (matching pre-NIP-FI base behavior). A valueless expiration tag + // followed by a future-valued expiration tag must admit, not expire. + let keys = Keys::generate(); + let sha256 = "c".repeat(64); + let now = Timestamp::now().as_secs(); + let exp_str = (now + 300).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", &sha256]).unwrap(), + Tag::parse(["expiration"]).unwrap(), // valueless first + Tag::parse(["expiration", &exp_str]).unwrap(), // valued second + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(&keys) + .unwrap(); + assert!( + verify_blossom_upload_auth( + &event, + &sha256, + None, + BlossomStrictness::Permissive, + ) + .is_ok(), + "Permissive must use last-wins expiration: valueless-first must not cause expiry rejection" + ); + } } diff --git a/crates/buzz-media/src/error.rs b/crates/buzz-media/src/error.rs index 5abbea6f580..357920e6400 100644 --- a/crates/buzz-media/src/error.rs +++ b/crates/buzz-media/src/error.rs @@ -3,6 +3,25 @@ use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; +/// Coarse Blossom denial kind for NIP-FI response shaping. +/// +/// Callers that know the active [`crate::auth::BlossomStrictness`] use this to +/// choose the correct HTTP response shape — NIP-FI fixed text/plain in Strict +/// mode, legacy JSON in Permissive mode — without requiring `buzz-media` to +/// depend on `buzz-auth`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BlossomDenialKind { + /// `Authorization` header was absent. Maps to HTTP 401 + `WWW-Authenticate: Nostr`. + MissingEvidence, + /// Authorization header or proof was present but structurally invalid, + /// malformed, expired, or otherwise rejected. Maps to HTTP 403. + EvidenceRejected, + /// A local-policy denial: relay membership required, community write fenced, + /// or similar authorization refusal that is not an auth-credential failure. + /// Maps to HTTP 403 with fixed `authorization denied\n` body [NIP-FI.md:764-775]. + AuthorizationDenied, +} + /// Errors from media operations. #[derive(Debug, thiserror::Error)] pub enum MediaError { @@ -26,6 +45,9 @@ pub enum MediaError { InvalidAuthVerb, #[error("missing required tag: {0}")] MissingTag(&'static str), + /// A tag that must appear exactly once appeared more than once. + #[error("duplicate tag: {0}")] + DuplicateTag(&'static str), #[error("hash mismatch")] HashMismatch, #[error("server mismatch")] @@ -109,6 +131,46 @@ impl From for MediaError { } } +impl MediaError { + /// Classify this error as a Blossom denial kind for response-shape selection. + /// + /// Returns `Some(BlossomDenialKind::MissingEvidence)` when the + /// `Authorization` header was absent, `Some(BlossomDenialKind::EvidenceRejected)` + /// for any structurally present but invalid/malformed/expired proof, + /// `Some(BlossomDenialKind::AuthorizationDenied)` for local-policy denials + /// (relay membership, community write fence), and `None` for non-auth errors. + /// + /// Relay call sites that know the active `BlossomStrictness` use this to + /// select the appropriate response shape: NIP-FI fixed text/plain in Strict + /// mode, legacy JSON 401 in Permissive mode. + pub fn blossom_denial_kind(&self) -> Option { + match self { + Self::MissingAuth => Some(BlossomDenialKind::MissingEvidence), + Self::InvalidAuthScheme + | Self::InvalidBase64 + | Self::InvalidAuthEvent + | Self::InvalidAuthKind + | Self::InvalidAuthVerb + | Self::DuplicateTag(_) + | Self::InvalidSignature + | Self::TokenExpired + | Self::TimestampOutOfWindow + | Self::Unauthorized + | Self::TokenRevoked + | Self::PubkeyMismatch + | Self::HashMismatch + | Self::ServerMismatch + | Self::MissingTag(_) => Some(BlossomDenialKind::EvidenceRejected), + // Local-policy denials: identity proved, but access refused by relay policy. + // Maps to AuthorizationDenied per NIP-FI.md:764-775. + Self::RelayMembershipRequired | Self::CommunityWriteFenced => { + Some(BlossomDenialKind::AuthorizationDenied) + } + _ => None, + } + } +} + impl IntoResponse for MediaError { fn into_response(self) -> Response { let (status, msg) = match &self { @@ -119,9 +181,15 @@ impl IntoResponse for MediaError { Self::FileTooLarge { .. } | Self::ImageTooLarge => { (StatusCode::PAYLOAD_TOO_LARGE, self.to_string()) } - // All authentication failures return the same generic 401 to prevent oracle enumeration. - // InsufficientScope is intentionally 403 — it's an authorization (not authentication) - // failure and is safe to distinguish since it requires a valid identity first. + // All authentication failures return the same generic 401 to prevent oracle + // enumeration in the legacy/Permissive path [FI-INV-15]. Off-mode deployments + // preserve this pre-NIP-FI behavior. InsufficientScope is intentionally 403 + // below — it is an authorization (not authentication) failure and is safe to + // distinguish because it requires a valid identity first. + // + // Strict mode routes through MediaDenial (buzz-relay) and applies the full + // NIP-FI rejection table (missing_evidence → 401, evidence_rejected → 403) + // independently of this path. Self::MissingAuth | Self::InvalidAuthScheme | Self::InvalidBase64 @@ -129,6 +197,7 @@ impl IntoResponse for MediaError { | Self::InvalidSignature | Self::InvalidAuthKind | Self::InvalidAuthVerb + | Self::DuplicateTag(_) | Self::TokenExpired | Self::TimestampOutOfWindow | Self::Unauthorized @@ -173,6 +242,136 @@ impl IntoResponse for MediaError { mod tests { use super::*; + // ── BlossomDenialKind classification ───────────────────────────────────── + + #[test] + fn missing_auth_is_missing_evidence() { + assert_eq!( + MediaError::MissingAuth.blossom_denial_kind(), + Some(BlossomDenialKind::MissingEvidence) + ); + } + + #[test] + fn structural_proof_errors_are_evidence_rejected() { + for error in [ + MediaError::InvalidAuthScheme, + MediaError::InvalidBase64, + MediaError::InvalidAuthEvent, + MediaError::InvalidAuthKind, + MediaError::InvalidAuthVerb, + MediaError::DuplicateTag("Authorization"), + MediaError::InvalidSignature, + MediaError::TokenExpired, + MediaError::TimestampOutOfWindow, + MediaError::Unauthorized, + MediaError::TokenRevoked, + MediaError::PubkeyMismatch, + MediaError::HashMismatch, + MediaError::ServerMismatch, + MediaError::MissingTag("t"), + ] { + assert_eq!( + error.blossom_denial_kind(), + Some(BlossomDenialKind::EvidenceRejected), + "expected EvidenceRejected for {error:?}" + ); + } + } + + #[test] + fn non_auth_errors_have_no_denial_kind() { + for error in [ + MediaError::NotFound, + MediaError::FileTooLarge { size: 1, max: 0 }, + MediaError::Internal, + MediaError::ServiceUnavailable, + MediaError::InsufficientScope, + ] { + assert_eq!( + error.blossom_denial_kind(), + None, + "expected None for {error:?}" + ); + } + } + + #[test] + fn local_policy_denials_are_authorization_denied() { + for error in [ + MediaError::RelayMembershipRequired, + MediaError::CommunityWriteFenced, + ] { + assert_eq!( + error.blossom_denial_kind(), + Some(BlossomDenialKind::AuthorizationDenied), + "expected AuthorizationDenied for {error:?}" + ); + } + } + + // ── IntoResponse status code pins ────────────────────────────────────── + // These pins cover the legacy/Permissive path (MediaError::into_response). + // Strict mode overrides this via MediaDenial in buzz-relay [FI-INV-15]. + + #[tokio::test] + async fn all_auth_failures_return_json_401_in_permissive_path() { + // In the legacy/Permissive path all auth failures collapse to a single + // JSON 401 to prevent oracle enumeration [FI-INV-15]. Strict mode + // (MediaDenial in buzz-relay) applies the NIP-FI 401/403 split instead. + for error in [ + MediaError::MissingAuth, + MediaError::InvalidAuthScheme, + MediaError::InvalidBase64, + MediaError::InvalidAuthEvent, + MediaError::InvalidAuthKind, + MediaError::InvalidAuthVerb, + MediaError::DuplicateTag("Authorization"), + MediaError::InvalidSignature, + MediaError::TokenExpired, + MediaError::TimestampOutOfWindow, + MediaError::Unauthorized, + MediaError::TokenRevoked, + MediaError::PubkeyMismatch, + MediaError::HashMismatch, + MediaError::ServerMismatch, + MediaError::MissingTag("t"), + ] { + let label = format!("{error:?}"); + let resp = error.into_response(); + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "Permissive path: expected 401 for {label}" + ); + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("application/json"), + "expected JSON CT for {label}, got: {ct}" + ); + // No WWW-Authenticate in the legacy JSON path. + assert!( + resp.headers().get("www-authenticate").is_none(), + "Permissive path must not include WWW-Authenticate for {label}" + ); + // Exact body: legacy path emits {"error":"authentication failed"} [FI-INV-15]. + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .expect("body collect"); + assert_eq!( + body.as_ref(), + br#"{"error":"authentication failed"}"#, + "Permissive path: wrong body for {label}" + ); + } + } + + // ── Existing non-auth response map tests ──────────────────────────────── + #[test] fn serving_backend_failures_map_to_5xx_but_fences_remain_403() { for error in [ diff --git a/crates/buzz-media/src/lib.rs b/crates/buzz-media/src/lib.rs index 3198e1f8301..81d341aa3a8 100644 --- a/crates/buzz-media/src/lib.rs +++ b/crates/buzz-media/src/lib.rs @@ -19,7 +19,7 @@ pub use bucket_index::{ TaxonomySweepOutcome, }; pub use config::{MediaConfig, S3AddressingStyle}; -pub use error::MediaError; +pub use error::{BlossomDenialKind, MediaError}; pub use storage::{ BlobHeadMeta, BlobMeta, BulkDeleteOutcome, ByteStream, MediaStorage, ObjectVersionEntry, ObjectVersionKind, ObjectVersionRef, ObjectVersionsPage, diff --git a/crates/buzz-media/src/upload.rs b/crates/buzz-media/src/upload.rs index 524b033280d..1b998bf843e 100644 --- a/crates/buzz-media/src/upload.rs +++ b/crates/buzz-media/src/upload.rs @@ -5,7 +5,7 @@ use bytes::Bytes; use sha2::{Digest, Sha256}; use tokio::io::AsyncWriteExt; -use crate::auth::verify_blossom_upload_auth; +use crate::auth::verify_upload_hash_only; use crate::config::MediaConfig; use crate::error::MediaError; use crate::storage::{BlobMeta, MediaStorage}; @@ -74,15 +74,15 @@ where let auth = auth_event.clone(); let bytes = body.clone(); let cfg = config.clone(); - // Validate the Blossom `server` tag against the host this request was bound - // to (the per-request tenant), not a process-global domain — a relay serves - // many tenant hosts. - let bound_host = ctx.host().to_string(); let (mime, sha256, ext) = tokio::task::spawn_blocking(move || -> Result<_, MediaError> { let (mime, ext) = validate(&bytes, &cfg)?; let sha256 = hex::encode(Sha256::digest(&bytes)); - // Buffered uploads (image + file): 10-minute auth window is plenty. - verify_blossom_upload_auth(&auth, &sha256, Some(bound_host.as_str()), 600)?; + // Post-body hash check only: the full auth event verification + // (signature, kind, freshness, server, cardinality) was already + // applied at the pre-body gate in the relay handler. Re-running the + // full verifier here would fail any upload that takes longer than the + // minted token's expiration window (60 s in Strict mode). + verify_upload_hash_only(&auth, &sha256)?; Ok((mime, sha256, ext)) }) .await @@ -404,12 +404,13 @@ pub async fn process_video_upload( // --- 3. Verify Blossom auth: x tag must match computed SHA-256 --- let auth = auth_event.clone(); let sha256_for_auth = sha256_hex.clone(); - // Validate the Blossom `server` tag against the bound tenant host (not a - // process-global domain) — a relay serves many tenant hosts. - let bound_host = ctx.host().to_string(); tokio::task::spawn_blocking(move || { - // Videos: 1-hour window — large uploads on slow connections need headroom. - verify_blossom_upload_auth(&auth, &sha256_for_auth, Some(bound_host.as_str()), 3600) + // Post-body hash check only: the full auth event verification + // (signature, kind, freshness, server, cardinality) was already + // applied at the pre-body gate in the relay handler. Re-running the + // full verifier here would reject any video upload that takes longer + // than the minted token's expiration window (60 s in Strict mode). + verify_upload_hash_only(&auth, &sha256_for_auth) }) .await .map_err(|_| MediaError::Internal)??; @@ -731,3 +732,357 @@ mod tests { assert!(desc.duration.is_none()); } } + +// ── Upload pipeline expiry-boundary regression ────────────────────────────── +// +// Demonstrates that the production post-body check is `verify_upload_hash_only` +// (commit 75e9bef748d2149ce459b14da842e706a51a5f78), NOT the old full verifier +// `verify_blossom_upload_auth`. +// +// The critical sequence: +// T=0: proof is minted with `created_at = now - 58`, `expiration = now + 2`. +// Strict admission passes: age 58s ≤ 60s window, lifetime 60s, expiry future. +// T=3s: body transfer completes; post-body check runs on now-expired proof. +// Old path: verify_blossom_upload_auth → TokenExpired (breaks upload). +// New path: verify_upload_hash_only → Ok if hash matches. +// +// These tests exercise both `process_upload` (buffered, upload.rs:85) and +// `process_video_upload` (streaming, upload.rs:413) so that reverting either +// post-body call to the old full verifier breaks the positive case, and +// removing the hash check breaks the negative case. +// +// Note: `nostr::Timestamp::now()` reads the OS wall clock directly; paused +// Tokio time does not advance it. The positive cases use a real 3-second +// sleep to sequence fresh admission at T=0 followed by expired completion at +// T≈3s. The negative (hash-mismatch) cases use a pre-expired proof and need +// no timing sequence. +// +// Tests that require a live MinIO instance live in the `minio_tests` module so +// the nextest profile can tag them with `#[ignore = "requires MinIO"]` and the +// unit lane skips them automatically. +#[cfg(test)] +mod minio_tests { + use super::*; + use buzz_core::tenant::{CommunityId, TenantContext}; + use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; + use uuid::Uuid; + + /// Minimal valid 1×1 RGB PNG — passes validate_content and + /// validate_image_metadata_free without any metadata chunks. + /// + /// Generated once and embedded as const bytes to avoid runtime PNG + /// encoding in every test run. The exact pixels and structure are + /// irrelevant; only the magic bytes and structural validity matter. + const MINIMAL_PNG: &[u8] = &[ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, // PNG signature + 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, // IHDR length + type + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, // width=1, height=1 + 0x08, 0x02, 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, // bit-depth=8, color=RGB + 0xde, // IHDR CRC (partial — enough for magic-byte sniff) + 0x00, 0x00, 0x00, 0x0c, 0x49, 0x44, 0x41, 0x54, // IDAT length + type + 0x78, 0x9c, 0x63, 0xf8, 0xff, 0xff, 0x3f, 0x00, // zlib-compressed scanline + 0x05, 0xfe, 0x02, 0xfe, 0x0d, 0xef, 0x46, 0xb8, // (white pixel) + 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, // IEND type + 0xae, 0x42, 0x60, 0x82, // IEND CRC + ]; + + fn minio_config() -> MediaConfig { + MediaConfig { + s3_endpoint: "http://localhost:9000".to_string(), + s3_access_key: "buzz_dev".to_string(), + s3_secret_key: "buzz_dev_secret".to_string(), + s3_bucket: "buzz-media".to_string(), + s3_region: "us-east-1".to_string(), + s3_addressing_style: crate::config::S3AddressingStyle::Path, + max_image_bytes: 50 * 1024 * 1024, + max_gif_bytes: 10 * 1024 * 1024, + max_video_bytes: 524_288_000, + max_file_bytes: 104_857_600, + public_base_url: "http://localhost:9000/buzz-media".to_string(), + upload_records_enabled: false, + upload_ip_header: None, + upload_port_header: None, + } + } + + fn test_tenant() -> TenantContext { + TenantContext::resolved(CommunityId::from_uuid(Uuid::nil()), "relay.example") + } + + /// Build a fresh Blossom upload auth event that passes Strict admission NOW. + /// + /// Strict invariants: + /// - `created_at` ≤ now + 5s (future-skew) + /// - now - `created_at` ≤ 60s (replay window) + /// - `expiration` ≤ `created_at` + 60s (token lifetime) + /// - `expiration` > now (not yet expired) + /// + /// We set `created_at = now - 58` (age = 58s, inside the 60s window), + /// `expiration = now - 58 + 60 = now + 2` (lifetime = 60s, strictly future). + /// Calling `verify_blossom_upload_auth(Strict)` at sign time must return `Ok`. + /// After a ≥3 s real sleep the proof is expired (expiration ≤ now): the old + /// full verifier would return `TokenExpired`, while `verify_upload_hash_only` + /// only checks the `x` tag and must still succeed. + fn fresh_strict_upload_auth(keys: &Keys, sha256: &str, server: &str) -> nostr::Event { + let now = Timestamp::now().as_secs(); + let created_at = now.saturating_sub(58); + let exp = created_at + 60; // now + 2s: strictly future, lifetime = 60s + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", sha256]).unwrap(), + Tag::parse(["expiration", &exp.to_string()]).unwrap(), + Tag::parse(["server", server]).unwrap(), + ]; + EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .custom_created_at(Timestamp::from_secs(created_at)) + .sign_with_keys(keys) + .expect("sign fresh upload auth") + } + + /// Build an already-expired Blossom upload auth event for the mismatch-hash + /// negative cases. No admission path is needed there: we only exercise the + /// post-body hash check, so the proof need not be fresh. + /// + /// `created_at = now - 120`, `expiration = now - 1`: definitively past for + /// the old full verifier (`TokenExpired`); the `x` tag is the only thing + /// `verify_upload_hash_only` reads. + fn expired_upload_auth(keys: &Keys, sha256: &str, server: &str) -> nostr::Event { + let now = Timestamp::now().as_secs(); + let created_at = now.saturating_sub(120); + let exp = now.saturating_sub(1); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", sha256]).unwrap(), + Tag::parse(["expiration", &exp.to_string()]).unwrap(), + Tag::parse(["server", server]).unwrap(), + ]; + EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .custom_created_at(Timestamp::from_secs(created_at)) + .sign_with_keys(keys) + .expect("sign expired upload auth") + } + + /// Case B (buffered pipeline): the NEW post-body path (`verify_upload_hash_only`) + /// accepts a proof whose expiry passes during the transfer. + /// + /// Sequence: + /// T=0: `fresh_strict_upload_auth` signs a 60s-lifetime proof that passes + /// Strict admission (`verify_blossom_upload_auth(Strict)` returns `Ok`). + /// T≈3s: real sleep lets the proof expire (expiration ≤ now). + /// T≈3s: `process_upload` runs; its post-body call is `verify_upload_hash_only`. + /// Old path: `verify_blossom_upload_auth` → `TokenExpired` (upload breaks). + /// New path: `verify_upload_hash_only` → `Ok` if hash matches. + /// + /// Note: `nostr::Timestamp::now()` reads the wall clock, not Tokio time. + /// Paused Tokio time does not advance it; a real sleep is required. + /// + /// Discriminating mutation: replacing `verify_upload_hash_only` at `upload.rs:85` + /// with `verify_blossom_upload_auth` causes this test to fail with `TokenExpired`. + /// + /// Requires a live MinIO instance (endpoint http://localhost:9000). + #[tokio::test] + #[ignore = "requires MinIO"] + async fn buffered_upload_accepts_expired_proof_when_hash_matches() { + use crate::auth::{verify_blossom_upload_auth, BlossomStrictness}; + + let keys = Keys::generate(); + let body = Bytes::from_static(MINIMAL_PNG); + let sha256 = hex::encode(sha2::Sha256::digest(&body)); + let auth = fresh_strict_upload_auth(&keys, &sha256, "relay.example"); + + // Assert that the proof passes Strict admission RIGHT NOW, before any sleep. + assert!( + verify_blossom_upload_auth( + &auth, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ) + .is_ok(), + "proof must pass Strict admission at sign time" + ); + + // Wait for the proof to expire. The expiry is `now + 2s` at sign time; + // 3s guarantees expiration ≤ current time when process_upload runs. + // `nostr::Timestamp::now()` reads the OS wall clock directly. + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + + let storage = MediaStorage::new(&minio_config()).expect("MinIO client must initialise"); + let ctx = test_tenant(); + + // process_upload calls verify_upload_hash_only post-body (the repaired path). + // The proof is now expired; a matching hash must still be accepted. + let result = process_upload( + &storage, + &minio_config(), + &ctx, + &auth, + body, + None, // no attribution + ) + .await; + + assert!( + result.is_ok(), + "buffered upload must accept an expired proof with a matching hash \ + (verify_upload_hash_only); got: {:?}", + result.err() + ); + } + + /// Case D (buffered pipeline): `verify_upload_hash_only` rejects a mismatched + /// body hash even when the proof is otherwise structurally valid. + /// + /// Discriminating mutation: removing the `verify_upload_hash_only` call + /// (or replacing it with a no-op) causes this test to accept a mismatched + /// body, which is a security regression — the relay would store whatever + /// bytes the client sent without verifying the Blossom `x` tag. + /// + /// Requires a live MinIO instance (endpoint http://localhost:9000). + #[tokio::test] + #[ignore = "requires MinIO"] + async fn buffered_upload_rejects_mismatched_hash_through_production_path() { + let keys = Keys::generate(); + // Sign the auth for a DIFFERENT hash than the actual body. + let mismatched_sha256 = "a".repeat(64); + let auth = expired_upload_auth(&keys, &mismatched_sha256, "relay.example"); + + // The body has a hash that differs from what was signed in the auth event. + let body = Bytes::from_static(MINIMAL_PNG); + + let storage = MediaStorage::new(&minio_config()).expect("MinIO client must initialise"); + let ctx = test_tenant(); + + let result = process_upload(&storage, &minio_config(), &ctx, &auth, body, None).await; + + assert!( + matches!(result, Err(MediaError::HashMismatch)), + "buffered upload must reject a body whose SHA-256 does not match \ + the signed x tag; got: {:?}", + result + ); + } + + /// Case B (streaming pipeline): `process_video_upload` accepts a proof whose + /// expiry passes during the transfer. + /// + /// Sequence mirrors the buffered case: + /// T=0: `fresh_strict_upload_auth` signs a 60s-lifetime proof that passes + /// Strict admission. + /// T≈3s: real sleep lets the proof expire. + /// T≈3s: `process_video_upload` streams to disk, computes SHA-256, then calls + /// `verify_upload_hash_only` (the repaired path). + /// Old path: `verify_blossom_upload_auth` → `TokenExpired`. + /// New path: `verify_upload_hash_only` → `Ok` if hash matches. + /// + /// Discriminating mutation: replacing `verify_upload_hash_only` at `upload.rs:413` + /// with `verify_blossom_upload_auth` causes this test to fail with `TokenExpired`. + /// + /// Requires a live MinIO instance (endpoint http://localhost:9000). + #[tokio::test] + #[ignore = "requires MinIO"] + async fn streaming_upload_accepts_expired_proof_when_hash_matches() { + use crate::auth::{verify_blossom_upload_auth, BlossomStrictness}; + use crate::validation::minimal_valid_mp4; + use futures_util::stream; + + let keys = Keys::generate(); + let body_bytes = minimal_valid_mp4(); + let sha256 = hex::encode(sha2::Sha256::digest(&body_bytes)); + let auth = fresh_strict_upload_auth(&keys, &sha256, "relay.example"); + + // Assert Strict admission passes at sign time. + assert!( + verify_blossom_upload_auth( + &auth, + &sha256, + Some("relay.example"), + BlossomStrictness::Strict + ) + .is_ok(), + "proof must pass Strict admission at sign time" + ); + + // Wait for the proof to expire (3s > 2s remaining until expiry). + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + + let body_len = body_bytes.len() as u64; + // Wrap the bytes in a single-item stream of Ok(Bytes). + // axum::Error wraps std::io::Error; we never inject an error here. + let body_stream = + stream::once(async move { Ok::<_, axum::Error>(Bytes::from(body_bytes)) }); + + let storage = MediaStorage::new(&minio_config()).expect("MinIO client must initialise"); + let ctx = test_tenant(); + + // process_video_upload streams to disk, computes SHA-256, then calls + // verify_upload_hash_only (the repaired path). The proof is now expired; + // a matching hash must still be accepted. + let result = process_video_upload( + &storage, + &minio_config(), + &ctx, + &auth, + body_stream, + Some(body_len), + None, // no attribution + ) + .await; + + assert!( + result.is_ok(), + "streaming upload must accept an expired proof with a matching hash \ + (verify_upload_hash_only); got: {:?}", + result.err() + ); + } + + /// Case D (streaming pipeline): `verify_upload_hash_only` rejects a mismatched + /// body hash through the streaming completion path. + /// + /// Discriminating mutation: removing the `verify_upload_hash_only` call at + /// `upload.rs:413` causes this test to accept a mismatched body, which is a + /// security regression — the relay would store whatever bytes the client sent + /// without verifying the Blossom `x` tag. + /// + /// Requires a live MinIO instance (endpoint http://localhost:9000). + #[tokio::test] + #[ignore = "requires MinIO"] + async fn streaming_upload_rejects_mismatched_hash_through_production_path() { + use crate::validation::minimal_valid_mp4; + use futures_util::stream; + + let keys = Keys::generate(); + // Sign auth for a DIFFERENT hash than the actual body. + let mismatched_sha256 = "a".repeat(64); + let auth = expired_upload_auth(&keys, &mismatched_sha256, "relay.example"); + + let body_bytes = minimal_valid_mp4(); + let body_len = body_bytes.len() as u64; + let body_stream = + stream::once(async move { Ok::<_, axum::Error>(Bytes::from(body_bytes)) }); + + let storage = MediaStorage::new(&minio_config()).expect("MinIO client must initialise"); + let ctx = test_tenant(); + + let result = process_video_upload( + &storage, + &minio_config(), + &ctx, + &auth, + body_stream, + Some(body_len), + None, + ) + .await; + + assert!( + matches!(result, Err(MediaError::HashMismatch)), + "streaming upload must reject a body whose SHA-256 does not match \ + the signed x tag; got: {:?}", + result + ); + } +} diff --git a/crates/buzz-media/src/validation.rs b/crates/buzz-media/src/validation.rs index 706c354d043..980e28d8cb9 100644 --- a/crates/buzz-media/src/validation.rs +++ b/crates/buzz-media/src/validation.rs @@ -954,6 +954,65 @@ pub fn mime_to_ext(mime: &str) -> &'static str { } } +/// Return a minimal structurally-valid H.264/avc1 MP4 body (618 bytes). +/// +/// Fast-start layout (ftyp + moov + mdat), single video track, 1 s, 320×240, +/// no audio. Generated from the same builder as the unit tests and verified to +/// pass `validate_video_file` with the default `MediaConfig`. +/// +/// Exposed only under `#[cfg(test)]` so it has no runtime cost; placed at +/// module scope (not inside `mod tests`) so other test modules in this crate +/// can import it via `use crate::validation::minimal_valid_mp4;`. +#[cfg(test)] +pub(crate) fn minimal_valid_mp4() -> Vec { + // ftyp(isom) + moov + mdat — pre-computed; identical to + // build_mp4_bytes(true, b"avc1", 1_000, 320, 240, false). + vec![ + 0x00, 0x00, 0x00, 0x14, 0x66, 0x74, 0x79, 0x70, 0x69, 0x73, 0x6f, 0x6d, 0x00, 0x00, 0x00, + 0x00, 0x69, 0x73, 0x6f, 0x6d, 0x00, 0x00, 0x02, 0x4e, 0x6d, 0x6f, 0x6f, 0x76, 0x00, 0x00, + 0x00, 0x6c, 0x6d, 0x76, 0x68, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0xe8, 0x00, 0x00, 0x03, 0xe8, 0x00, 0x01, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x02, 0x00, 0x00, 0x01, 0xda, 0x74, 0x72, 0x61, 0x6b, 0x00, 0x00, 0x00, 0x5c, 0x74, 0x6b, + 0x68, 0x64, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0xe8, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, + 0x00, 0x00, 0x00, 0x01, 0x40, 0x00, 0x00, 0x00, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x01, 0x76, + 0x6d, 0x64, 0x69, 0x61, 0x00, 0x00, 0x00, 0x20, 0x6d, 0x64, 0x68, 0x64, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0xe8, 0x00, 0x00, + 0x03, 0xe8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x2d, 0x68, 0x64, 0x6c, 0x72, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x76, 0x69, 0x64, 0x65, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x56, 0x69, 0x64, 0x65, 0x6f, 0x48, 0x61, + 0x6e, 0x64, 0x6c, 0x65, 0x72, 0x00, 0x00, 0x00, 0x01, 0x21, 0x6d, 0x69, 0x6e, 0x66, 0x00, + 0x00, 0x00, 0x14, 0x76, 0x6d, 0x68, 0x64, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x24, 0x64, 0x69, 0x6e, 0x66, 0x00, 0x00, 0x00, + 0x1c, 0x64, 0x72, 0x65, 0x66, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, + 0x00, 0x0c, 0x75, 0x72, 0x6c, 0x20, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xe1, 0x73, + 0x74, 0x62, 0x6c, 0x00, 0x00, 0x00, 0x79, 0x73, 0x74, 0x73, 0x64, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x69, 0x61, 0x76, 0x63, 0x31, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x40, 0x00, 0xf0, 0x00, 0x48, 0x00, 0x00, 0x00, + 0x48, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0xff, 0xff, + 0x00, 0x00, 0x00, 0x13, 0x61, 0x76, 0x63, 0x43, 0x01, 0x42, 0x00, 0x1e, 0xff, 0xe1, 0x00, + 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x73, 0x74, 0x74, 0x73, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x03, 0xe8, 0x00, 0x00, + 0x00, 0x1c, 0x73, 0x74, 0x73, 0x63, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x18, + 0x73, 0x74, 0x73, 0x7a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x14, 0x73, 0x74, 0x63, 0x6f, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x1c, 0x00, 0x00, 0x00, 0x08, 0x6d, + 0x64, 0x61, 0x74, + ] +} + #[cfg(test)] mod tests { use super::*; @@ -2324,6 +2383,21 @@ mod tests { } } + /// Smoke-test: `minimal_valid_mp4()` (the bytes embedded for streaming upload + /// tests) must round-trip through `validate_video_file`. If this fails, the + /// embedded byte sequence needs to be re-generated. + #[test] + fn test_minimal_valid_mp4_passes_validate_video_file() { + let mp4_bytes = minimal_valid_mp4(); + let tmp = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(tmp.path(), &mp4_bytes).unwrap(); + let result = validate_video_file(tmp.path(), &test_config()); + assert!( + result.is_ok(), + "minimal_valid_mp4() must pass validate_video_file; got: {result:?}" + ); + } + #[test] fn test_validate_video_accepts_proprietary_major_with_isom_compatibility() { let mut mp4_bytes = build_minimal_mp4_moov_first(); diff --git a/crates/buzz-relay/src/api/media.rs b/crates/buzz-relay/src/api/media.rs index 780532ec5d0..eb19d5a46a9 100644 --- a/crates/buzz-relay/src/api/media.rs +++ b/crates/buzz-relay/src/api/media.rs @@ -19,8 +19,12 @@ use axum::{ }; use base64::Engine; use buzz_audit::{AuditAction, NewAuditEntry}; +use buzz_auth::DenialClass; use buzz_core::tenant::TenantContext; -use buzz_media::{BlobDescriptor, MediaError, UploadAttribution, UploadNetworkInfo}; +use buzz_media::auth::BlossomStrictness; +use buzz_media::{ + BlobDescriptor, BlossomDenialKind, MediaError, UploadAttribution, UploadNetworkInfo, +}; use crate::state::AppState; @@ -38,6 +42,10 @@ pub(crate) struct AuthenticatedUpload { /// door in `bridge.rs`. Server-resolved, never client-supplied. tenant: TenantContext, route_mode: UploadRouteMode, + /// NIP-FI strictness derived at extraction time. Carried to the handler so + /// post-body auth failures (hash-binding, x-tag mismatches) produce the same + /// mode-aware denial shape as pre-body failures. + strictness: BlossomStrictness, _upload_permit: UploadPermit, } @@ -47,6 +55,63 @@ enum UploadRouteMode { LegacyMedia, } +/// Mode-aware Blossom auth rejection for the relay layer. +/// +/// In `Strict` mode, Blossom denial errors map to the NIP-FI fixed +/// text/plain responses (`DenialClass` byte contract). In `Permissive` mode +/// (Off-mode deployments), the legacy JSON 401 shape is preserved unchanged +/// [FI-INV-15]. +/// +/// Non-Blossom errors (`blossom_denial_kind()` returns `None`) always fall +/// through to `MediaError::into_response()` regardless of mode. +pub(crate) struct MediaDenial(MediaError, BlossomStrictness); + +impl IntoResponse for MediaDenial { + fn into_response(self) -> Response { + let MediaDenial(error, strictness) = self; + if strictness == BlossomStrictness::Strict { + if let Some(kind) = error.blossom_denial_kind() { + let class = match kind { + BlossomDenialKind::MissingEvidence => DenialClass::MissingEvidence, + BlossomDenialKind::EvidenceRejected => DenialClass::EvidenceRejected, + BlossomDenialKind::AuthorizationDenied => DenialClass::AuthorizationDenied, + }; + tracing::warn!( + error = %error, + denial_class = ?class, + "Blossom auth denial (strict)" + ); + let mut builder = axum::http::Response::builder() + .status(class.http_status()) + .header(header::CONTENT_TYPE, class.content_type()); + if let Some(challenge) = class.www_authenticate() { + builder = builder.header("WWW-Authenticate", challenge); + } + return builder + .body(axum::body::Body::from(class.http_body())) + .expect("NIP-FI denial response is always valid"); + } + } + error.into_response() + } +} + +/// Wrap a `MediaError` with the active strictness to produce the correct +/// response shape at the relay boundary. +fn media_denial(error: MediaError, strictness: BlossomStrictness) -> MediaDenial { + MediaDenial(error, strictness) +} + +impl From for MediaDenial { + /// Default conversion uses Permissive mode — non-auth errors always fall + /// through to `MediaError::into_response()` regardless of mode, so the + /// strictness value is irrelevant. Auth errors at the extractor boundary + /// use explicit `media_denial(e, strictness)` calls instead. + fn from(e: MediaError) -> Self { + MediaDenial(e, BlossomStrictness::Permissive) + } +} + fn should_stream_as_video(sniff: &[u8]) -> bool { infer::get(sniff).is_some_and(|kind| kind.mime_type() == "video/mp4") || buzz_media::looks_like_iso_bmff(sniff) @@ -138,7 +203,7 @@ fn acquire_upload_permit( } impl FromRequestParts> for AuthenticatedUpload { - type Rejection = MediaError; + type Rejection = MediaDenial; async fn from_request_parts( parts: &mut Parts, @@ -170,18 +235,24 @@ impl FromRequestParts> for AuthenticatedUpload { let route_mode = upload_route_mode(parts.uri.path())?; // 2. Extract and validate Blossom auth event against the bound host. - let auth_event = extract_blossom_auth(headers)?; - // Use the permissive window (3600s) here because we don't know the - // content type yet. The upload functions re-verify with the correct - // per-type window (600s for images, 3600s for video) after the body - // has been consumed and the SHA-256 computed. - buzz_media::auth::verify_blossom_auth_event(&auth_event, Some(tenant.host()), 3600)?; + // Derive strictness from NIP-FI mode: strict rules apply in active (non-Off) modes. + // Off mode preserves the pre-NIP-FI permissive behavior [FI-INV-15]. + // + // Auth errors are wrapped with `media_denial(e, strictness)` so that + // Strict mode produces NIP-FI fixed text/plain responses; Permissive + // mode falls through to the legacy JSON 401 shape [FI-INV-15]. + let strictness = blossom_strictness_from_state(state); + let auth_event = extract_blossom_auth(headers).map_err(|e| media_denial(e, strictness))?; + // Pre-body strictness check: verify freshness, cardinality, and server tag before + // the body is consumed. The x-tag hash binding is checked after body completion. + buzz_media::auth::verify_blossom_auth_event(&auth_event, Some(tenant.host()), strictness) + .map_err(|e| media_denial(e, strictness))?; // 3. Require X-SHA-256 header (BUD-11: mandatory for PUT /upload) let claimed_hash = headers .get("x-sha-256") .and_then(|v| v.to_str().ok()) - .ok_or(MediaError::MissingTag("x-sha-256"))?; + .ok_or_else(|| media_denial(MediaError::MissingTag("x-sha-256"), strictness))?; // Validate format: exactly 64 lowercase hex characters if claimed_hash.len() != 64 @@ -189,7 +260,7 @@ impl FromRequestParts> for AuthenticatedUpload { .chars() .all(|c| matches!(c, '0'..='9' | 'a'..='f')) { - return Err(MediaError::HashMismatch); + return Err(media_denial(MediaError::HashMismatch, strictness)); } // 4. Validate X-SHA-256 matches at least one x tag in the auth event @@ -198,7 +269,7 @@ impl FromRequestParts> for AuthenticatedUpload { .iter() .any(|tag| tag.kind().to_string() == "x" && (tag.content() == Some(claimed_hash))); if !has_matching_x { - return Err(MediaError::HashMismatch); + return Err(media_denial(MediaError::HashMismatch, strictness)); } // 5. Relay membership gate (NIP-43). Blossom auth proves the signer @@ -217,23 +288,25 @@ impl FromRequestParts> for AuthenticatedUpload { Some(auth_event.created_at.as_secs()), ) .await - .map_err(|_| MediaError::RelayMembershipRequired)?; + .map_err(|_| media_denial(MediaError::RelayMembershipRequired, strictness))?; if upload_rate_limited(state, tenant.community(), &auth_event.pubkey) { metrics::counter!("buzz_media_upload_rejections_total", "reason" => "rate_limit") .increment(1); - return Err(MediaError::UploadRateLimitExceeded); + return Err(MediaError::UploadRateLimitExceeded.into()); } let upload_permit = acquire_upload_permit(state, tenant.community(), &auth_event.pubkey) .inspect_err(|_| { metrics::counter!("buzz_media_upload_rejections_total", "reason" => "concurrency") .increment(1); - })?; + }) + .map_err(MediaDenial::from)?; Ok(AuthenticatedUpload { auth_event, tenant, route_mode, + strictness, _upload_permit: upload_permit, }) } @@ -317,18 +390,20 @@ fn serving_lease_lost(error: anyhow::Error) -> MediaError { /// Returns a [`BlobDescriptor`] JSON on success. // TODO(v2): Add persistent per-pubkey storage quotas. Admission limits below // bound active parser/storage work, but they do not cap durable bytes stored. -pub async fn upload_blob( +pub(crate) async fn upload_blob( State(state): State>, auth: AuthenticatedUpload, headers: HeaderMap, body: axum::body::Body, -) -> Result, MediaError> { +) -> Result, MediaDenial> { + let strictness = auth.strictness; let attribution = upload_attribution(&state, &auth, &headers).await; let serving_write = buzz_deletion::acquire_serving_write(&state.db, auth.tenant.community(), "media_upload") .await - .map_err(serving_write_error)?; + .map_err(serving_write_error) + .map_err(|e| media_denial(e, strictness))?; if auth.route_mode == UploadRouteMode::LegacyMedia { metrics::counter!("buzz_media_legacy_upload_route_total").increment(1); @@ -349,13 +424,19 @@ pub async fn upload_blob( sniff.extend_from_slice(&chunk[..chunk.len().min(needed)]); replay_chunks.push(chunk); } - Some(Err(error)) => return Err(MediaError::Io(error.to_string())), + Some(Err(error)) => { + return Err(media_denial(MediaError::Io(error.to_string()), strictness)) + } None => break, } } let replay = futures_util::stream::iter(replay_chunks.into_iter().map(Ok)).chain(source); - serving_write.verify().await.map_err(serving_lease_lost)?; + serving_write + .verify() + .await + .map_err(serving_lease_lost) + .map_err(MediaDenial::from)?; let mut descriptor = serving_write .protect(async { @@ -434,7 +515,15 @@ pub async fn upload_blob( Err(_) => MediaError::Internal, } } - })??; + }) + // The outer anyhow→MediaError map above captures lease-loss and + // protect-layer failures. Apply media_denial so Strict produces + // byte-exact NIP-FI responses for those errors. + .map_err(|e| media_denial(e, strictness))? + // The inner Result captures failures from the async body (process_*, + // hash mismatches). Wrap through media_denial so post-body auth errors + // get the same NIP-FI shape as pre-body ones. + .map_err(|e| media_denial(e, strictness))?; rewrite_descriptor_urls_for_tenant( &mut descriptor, @@ -528,12 +617,14 @@ async fn authenticate_media_read( state: &AppState, headers: &HeaderMap, sha256_ext: &str, -) -> Result { +) -> Result { let tenant = bind_media_read_tenant(state, headers).await?; - let auth_event = extract_blossom_auth(headers)?; + let strictness = blossom_strictness_from_state(state); + let auth_event = extract_blossom_auth(headers).map_err(|e| media_denial(e, strictness))?; let sha256 = sha256_ext.split('.').next().unwrap_or(sha256_ext); - buzz_media::auth::verify_blossom_get_auth(&auth_event, sha256, Some(tenant.host()), 3600)?; + buzz_media::auth::verify_blossom_get_auth(&auth_event, sha256, Some(tenant.host()), strictness) + .map_err(|e| media_denial(e, strictness))?; let auth_tag = crate::api::relay_members::extract_auth_tag_header(headers); crate::api::relay_members::enforce_relay_membership( @@ -544,7 +635,7 @@ async fn authenticate_media_read( Some(auth_event.created_at.as_secs()), ) .await - .map_err(|_| MediaError::RelayMembershipRequired)?; + .map_err(|_| media_denial(MediaError::RelayMembershipRequired, strictness))?; Ok(MediaReadAuth { tenant }) } @@ -632,14 +723,16 @@ const MAX_RANGE_CHUNK: u64 = 16 * 1024 * 1024; /// - Chunk capped at 16 MiB; clients request additional ranges for the rest /// /// All responses include `Accept-Ranges: bytes` so video players know seeking is supported. -pub async fn get_blob( +pub(crate) async fn get_blob( State(state): State>, Path(sha256_ext): Path, req_headers: HeaderMap, -) -> Result { +) -> Result { validate_media_path(&sha256_ext)?; let media_auth = authenticate_media_read(&state, &req_headers, &sha256_ext).await?; - serve_blob_for_tenant(&state, &media_auth.tenant, &sha256_ext, &req_headers).await + serve_blob_for_tenant(&state, &media_auth.tenant, &sha256_ext, &req_headers) + .await + .map_err(MediaDenial::from) } /// Serve a validated blob from an already-authorized tenant context. @@ -897,11 +990,11 @@ fn parse_byte_range(range: &str, total: u64) -> Option<(u64, u64)> { /// Content-type is derived from the validated sidecar only — never from raw S3 /// object metadata — to prevent MIME spoofing via tampered storage. If the sidecar /// is missing, we return 404 rather than fall back to untrusted metadata. -pub async fn head_blob( +pub(crate) async fn head_blob( State(state): State>, headers: HeaderMap, Path(sha256_ext): Path, -) -> Result { +) -> Result { validate_media_path(&sha256_ext)?; let media_auth = authenticate_media_read(&state, &headers, &sha256_ext).await?; let tenant = media_auth.tenant; @@ -930,7 +1023,7 @@ pub async fn head_blob( .await .map_err(|_| MediaError::NotFound)?; if requested_ext != sidecar.ext { - return Err(MediaError::NotFound); + return Err(MediaError::NotFound.into()); } } sidecar_mime @@ -985,13 +1078,29 @@ async fn resolve_s3_key( /// Extract and verify a kind:24242 Blossom auth event from the `Authorization` header. /// /// Accepts both base64url (BUD-11 spec) and standard base64 (nostr-tools compat). +/// +/// Per NIP-FI §Transport and cardinality: +/// - Missing `Authorization` → `missing_evidence` (401 via `MediaError::MissingAuth`) +/// - Repeated, comma-combined, empty, malformed, or wrong-scheme → `evidence_rejected` +/// (403 via `MediaError::InvalidAuthScheme` / `MediaError::DuplicateTag("Authorization")`) fn extract_blossom_auth(headers: &HeaderMap) -> Result { use base64::engine::general_purpose::{STANDARD, URL_SAFE_NO_PAD}; - let header = headers - .get("authorization") - .and_then(|v| v.to_str().ok()) - .ok_or(MediaError::MissingAuth)?; + // NIP-FI: repeated or comma-combined Authorization → evidence_rejected (403). + // HeaderMap::get_all returns all values for the key; more than one is malformed. + let mut auth_values = headers.get_all("authorization").iter(); + let first = auth_values.next().ok_or(MediaError::MissingAuth)?; + if auth_values.next().is_some() { + // More than one Authorization header value → evidence_rejected + return Err(MediaError::DuplicateTag("Authorization")); + } + + let header = first.to_str().map_err(|_| MediaError::InvalidAuthScheme)?; + + // Reject empty or whitespace-only Authorization values + if header.trim().is_empty() { + return Err(MediaError::InvalidAuthScheme); + } let token = header .strip_prefix("Nostr ") @@ -1008,13 +1117,34 @@ fn extract_blossom_auth(headers: &HeaderMap) -> Result Ok(event) } +/// Derive `BlossomStrictness` from the current NIP-FI mode stored in `AppState`. +/// +/// This is the bridge between the relay's mode configuration and the +/// `buzz-media` verifier. When #7264 is merged and `config.nip_fi` is +/// present, strict rules apply in any non-Off mode (Enforce, DenyProtected). +/// Off mode preserves the pre-NIP-FI permissive verifier behavior [FI-INV-15]. +/// +/// On `origin/main` (before #7264 merges), `AppState` has no `nip_fi` field — +/// the relay defaults to Permissive. The TODO below tracks the wiring to remove +/// after #7264 lands. +fn blossom_strictness_from_state(_state: &AppState) -> buzz_media::auth::BlossomStrictness { + // TODO(#7264): replace with `state.config.nip_fi.is_enforce()` once + // NipFiRelayConfig is wired into AppState by #7264. Until then, all + // deployments use Permissive (preserving existing behavior). + #[cfg(test)] + if let Some(override_strictness) = _state.config.test_blossom_strictness { + return override_strictness; + } + buzz_media::auth::BlossomStrictness::Permissive +} + #[cfg(test)] mod tests { use super::*; use std::sync::Arc; use axum::{ - body::Body, + body::{to_bytes, Body}, http::{header, Request, StatusCode}, }; use nostr::{EventBuilder, JsonUtil, Keys, Kind, Tag, Timestamp}; @@ -1023,6 +1153,251 @@ mod tests { const VALID_HASH: &str = "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"; + // ── MediaDenial response-shape tests (NIP-FI §755-773) ────────────────── + // These tests pin the byte-exact response contract for Strict mode and + // confirm Permissive mode produces the unchanged legacy JSON 401 shape. + + #[tokio::test] + async fn strict_missing_auth_produces_nip_fi_401_with_www_authenticate() { + let denial = MediaDenial(MediaError::MissingAuth, BlossomStrictness::Strict); + let resp = denial.into_response(); + + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("text/plain"), + "expected text/plain CT, got: {ct}" + ); + + let www_auth = resp + .headers() + .get("www-authenticate") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert_eq!( + www_auth, "Nostr", + "expected 'Nostr' WWW-Authenticate challenge" + ); + + let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + assert_eq!(body.as_ref(), b"authentication required\n"); + } + + #[tokio::test] + async fn strict_evidence_rejected_produces_nip_fi_403_text_plain() { + for error in [ + MediaError::InvalidSignature, + MediaError::TokenExpired, + MediaError::TimestampOutOfWindow, + MediaError::HashMismatch, + MediaError::ServerMismatch, + MediaError::MissingTag("server"), + MediaError::DuplicateTag("Authorization"), + MediaError::InvalidAuthScheme, + ] { + let label = format!("{error:?}"); + let denial = MediaDenial(error, BlossomStrictness::Strict); + let resp = denial.into_response(); + + assert_eq!( + resp.status(), + StatusCode::FORBIDDEN, + "expected 403 for Strict {label}" + ); + + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("text/plain"), + "expected text/plain CT for {label}, got: {ct}" + ); + + assert!( + resp.headers().get("www-authenticate").is_none(), + "403 must not have WWW-Authenticate for {label}" + ); + + let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + assert_eq!( + body.as_ref(), + b"evidence rejected\n", + "wrong body for {label}" + ); + } + } + + #[tokio::test] + async fn permissive_missing_auth_keeps_legacy_json_401() { + let denial = MediaDenial(MediaError::MissingAuth, BlossomStrictness::Permissive); + let resp = denial.into_response(); + + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("application/json"), + "Permissive must keep JSON CT, got: {ct}" + ); + + assert!( + resp.headers().get("www-authenticate").is_none(), + "Permissive must not add WWW-Authenticate" + ); + } + + #[tokio::test] + async fn permissive_evidence_rejected_keeps_legacy_json_401() { + for error in [ + MediaError::InvalidSignature, + MediaError::TokenExpired, + MediaError::HashMismatch, + MediaError::MissingTag("t"), + ] { + let label = format!("{error:?}"); + let denial = MediaDenial(error, BlossomStrictness::Permissive); + let resp = denial.into_response(); + + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "Permissive: expected 401 for {label}" + ); + + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("application/json"), + "Permissive must keep JSON CT for {label}, got: {ct}" + ); + } + } + + #[tokio::test] + async fn non_auth_errors_always_fall_through_regardless_of_mode() { + for strictness in [BlossomStrictness::Strict, BlossomStrictness::Permissive] { + let denial = MediaDenial(MediaError::NotFound, strictness); + assert_eq!(denial.into_response().status(), StatusCode::NOT_FOUND); + + let denial = MediaDenial(MediaError::Internal, strictness); + assert!(denial.into_response().status().is_server_error()); + } + } + + // ── Extractor hash-check denial sites (sites 1+2 per Paul's spot-check) ── + // These pins cover the missing-X-SHA-256 header (MissingTag) and the + // malformed/unmatched hash (HashMismatch) cases that previously bypassed + // the strictness split via `From for MediaDenial` (Permissive). + + #[tokio::test] + async fn strict_missing_x_sha256_header_produces_nip_fi_403() { + let denial = MediaDenial( + MediaError::MissingTag("x-sha-256"), + BlossomStrictness::Strict, + ); + let resp = denial.into_response(); + assert_eq!( + resp.status(), + StatusCode::FORBIDDEN, + "missing x-sha-256 header must be 403 in Strict mode" + ); + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("text/plain"), + "expected text/plain CT, got: {ct}" + ); + assert!( + resp.headers().get("www-authenticate").is_none(), + "403 must not have WWW-Authenticate" + ); + let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + assert_eq!(body.as_ref(), b"evidence rejected\n"); + } + + #[tokio::test] + async fn strict_hash_mismatch_produces_nip_fi_403() { + let denial = MediaDenial(MediaError::HashMismatch, BlossomStrictness::Strict); + let resp = denial.into_response(); + assert_eq!( + resp.status(), + StatusCode::FORBIDDEN, + "hash mismatch must be 403 in Strict mode" + ); + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!(ct.contains("text/plain"), "expected text/plain, got: {ct}"); + let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + assert_eq!(body.as_ref(), b"evidence rejected\n"); + } + + #[tokio::test] + async fn permissive_missing_x_sha256_header_keeps_legacy_json_401() { + let denial = MediaDenial( + MediaError::MissingTag("x-sha-256"), + BlossomStrictness::Permissive, + ); + let resp = denial.into_response(); + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "Permissive must keep legacy 401 for MissingTag" + ); + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("application/json"), + "Permissive must keep JSON CT, got: {ct}" + ); + assert!( + resp.headers().get("www-authenticate").is_none(), + "Permissive must not add WWW-Authenticate" + ); + } + + #[tokio::test] + async fn permissive_hash_mismatch_keeps_legacy_json_401() { + let denial = MediaDenial(MediaError::HashMismatch, BlossomStrictness::Permissive); + let resp = denial.into_response(); + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "Permissive must keep legacy 401 for HashMismatch" + ); + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("application/json"), + "Permissive must keep JSON CT, got: {ct}" + ); + } + #[test] fn serving_write_error_taxonomy_separates_fence_from_backend_failure() { let fenced = anyhow::Error::from(buzz_db::DbError::AccessDenied("fenced".to_string())); @@ -1037,6 +1412,398 @@ mod tests { )); } + // ── Finding 5 (F5): membership denials in Strict mode → authorization denied ─ + + #[tokio::test] + async fn strict_relay_membership_required_produces_nip_fi_403_authorization_denied() { + let denial = MediaDenial( + MediaError::RelayMembershipRequired, + BlossomStrictness::Strict, + ); + let resp = denial.into_response(); + + assert_eq!( + resp.status(), + StatusCode::FORBIDDEN, + "membership denial must be 403 in Strict mode" + ); + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("text/plain"), + "expected text/plain CT, got: {ct}" + ); + assert!( + resp.headers().get("www-authenticate").is_none(), + "403 authorization denied must not have WWW-Authenticate" + ); + let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + assert_eq!( + body.as_ref(), + b"authorization denied\n", + "NIP-FI membership denial body must be 'authorization denied\\n'" + ); + } + + #[tokio::test] + async fn permissive_relay_membership_required_keeps_legacy_json_403() { + // In Permissive mode membership denial falls through to MediaError::into_response() + // which produces the legacy JSON 403. + let denial = MediaDenial( + MediaError::RelayMembershipRequired, + BlossomStrictness::Permissive, + ); + let resp = denial.into_response(); + + assert_eq!( + resp.status(), + StatusCode::FORBIDDEN, + "membership denial must still be 403 in Permissive mode" + ); + let ct = resp + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert!( + ct.contains("application/json"), + "Permissive membership denial must keep JSON CT, got: {ct}" + ); + } + + // ── F5 production-path membership regressions ────────────────────────── + // These tests live in `mod postgres_tests` so the nextest postgres-ci + // profile discovers them via `test(/postgres_tests::/)` and attaches the + // setup and isolation scripts. All tests call `ensure_configured_community` + // which performs a real DB upsert — they require a live PostgreSQL service + // and are not safe to run in the unit lane (`just test-unit`). + + mod postgres_tests { + use super::*; + + /// Provision a membership-enabled AppState whose community host is + /// "relay.example". Returns both the state and the seeded community + /// record so callers can add members via `state.db.add_relay_member`. + async fn state_and_community( + strictness: BlossomStrictness, + ) -> (Arc, buzz_db::EnsuredCommunityRecord) { + let mut config = crate::config::Config::from_env().expect("default config loads"); + config.require_relay_membership = true; + config.redis_url = "redis://127.0.0.1:1".to_string(); + config.media_uploads_per_minute = 10; + config.media_max_concurrent_uploads = 4; + config.media_max_concurrent_uploads_per_pubkey = 2; + config.test_blossom_strictness = Some(strictness); + + let pool = sqlx::PgPool::connect_lazy(&config.database_url).expect("lazy pg pool"); + let db = buzz_db::Db::from_pool(pool.clone()); + let community = db + .ensure_configured_community("relay.example") + .await + .expect("seed relay.example community for membership tests"); + let redis_pool = deadpool_redis::Config::from_url(&config.redis_url) + .create_pool(Some(deadpool_redis::Runtime::Tokio1)) + .expect("redis pool"); + let pubsub = Arc::new( + buzz_pubsub::PubSubManager::new(&config.redis_url, redis_pool.clone()) + .await + .expect("pubsub manager"), + ); + let audit = buzz_audit::AuditService::new(pool.clone()); + let auth = buzz_auth::AuthService::new(config.auth.clone()); + let search = buzz_search::SearchService::new(pool.clone()); + let workflow_engine = Arc::new(buzz_workflow::WorkflowEngine::new( + db.clone(), + buzz_workflow::WorkflowConfig::default(), + )); + let media_storage = + buzz_media::MediaStorage::new(&config.media).expect("media storage"); + let (state, _audit_shutdown) = AppState::new( + config, + db, + redis_pool, + audit, + pubsub, + auth, + search, + workflow_engine, + nostr::Keys::generate(), + media_storage, + ); + (Arc::new(state), community) + } + + fn upload_auth_header_for(keys: &Keys, sha256: &str) -> String { + let now = Timestamp::now().as_secs(); + let exp = (now + 55).to_string(); + let tags = vec![ + Tag::parse(["t", "upload"]).unwrap(), + Tag::parse(["x", sha256]).unwrap(), + Tag::parse(["expiration", &exp]).unwrap(), + Tag::parse(["server", "relay.example"]).unwrap(), + ]; + let event = EventBuilder::new(Kind::from(24242), "Upload buzz-media") + .tags(tags) + .sign_with_keys(keys) + .expect("sign upload auth"); + format!( + "Nostr {}", + base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(event.as_json().as_bytes()) + ) + } + + /// Read membership denial in Strict mode must produce NIP-FI fixed + /// `authorization denied\n` 403 text/plain, no WWW-Authenticate. + /// + /// Red-with-reverted-wiring: reverting `media_denial(e, strictness)` at + /// the read membership call site back to `MediaDenial::from` produces + /// JSON 403 — this test fails on content-type and body assertions. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn strict_read_membership_denial_produces_authorization_denied() { + let keys = Keys::generate(); + let auth = media_get_auth_header(&keys, media_get_tags_for("relay.example", None)); + let (state, _) = state_and_community(BlossomStrictness::Strict).await; + let router = axum::Router::new() + .route( + "/media/{sha256_ext}", + axum::routing::get(get_blob).head(head_blob), + ) + .with_state(state); + + let response = router + .oneshot(media_request("GET", Some(auth))) + .await + .expect("response"); + + assert_eq!( + response.status(), + StatusCode::FORBIDDEN, + "read membership denial must be 403 in Strict mode" + ); + let ct = response + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert_eq!( + ct, "text/plain; charset=utf-8", + "Strict membership denial must be exact text/plain; charset=utf-8, got: {ct}" + ); + assert!( + response.headers().get("www-authenticate").is_none(), + "403 authorization denied must not carry WWW-Authenticate" + ); + let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!( + body.as_ref(), + b"authorization denied\n", + "Strict read membership body must be 'authorization denied\\n'" + ); + } + + /// Read membership denial in Permissive mode must preserve the exact + /// legacy JSON body (`{"error":"relay membership required"}`) and + /// application/json content-type — no WWW-Authenticate challenge. + /// + /// Confirms: (a) Permissive is unaffected by the Strict fence wiring, + /// (b) the exact legacy body text is preserved (not just the status). + #[tokio::test] + #[ignore = "requires Postgres"] + async fn permissive_read_membership_denial_keeps_exact_legacy_json_403() { + let keys = Keys::generate(); + let auth = media_get_auth_header(&keys, media_get_tags_for("relay.example", None)); + let (state, _) = state_and_community(BlossomStrictness::Permissive).await; + let router = axum::Router::new() + .route( + "/media/{sha256_ext}", + axum::routing::get(get_blob).head(head_blob), + ) + .with_state(state); + + let response = router + .oneshot(media_request("GET", Some(auth))) + .await + .expect("response"); + + assert_eq!( + response.status(), + StatusCode::FORBIDDEN, + "read membership denial must be 403 in Permissive mode" + ); + let ct = response + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert_eq!( + ct, "application/json", + "Permissive read membership denial must be exact application/json CT, got: {ct}" + ); + assert!( + response.headers().get("www-authenticate").is_none(), + "Permissive 403 must not carry WWW-Authenticate" + ); + let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!( + body.as_ref(), + br#"{"error":"relay membership required"}"#, + "Permissive read body must be exact legacy JSON bytes" + ); + } + + /// Upload membership denial in Strict mode must produce NIP-FI fixed + /// `authorization denied\n` 403 text/plain. + /// + /// Red-with-reverted-wiring: reverting `media_denial(e, strictness)` at + /// the upload membership call site back to `MediaDenial::from` produces + /// JSON 403 — this test fails on content-type and body assertions. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn strict_upload_membership_denial_produces_authorization_denied() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let auth_header = upload_auth_header_for(&keys, &sha256); + let (state, _) = state_and_community(BlossomStrictness::Strict).await; + let router = axum::Router::new() + .route("/upload", axum::routing::put(upload_blob)) + .with_state(state); + + let request = Request::builder() + .method("PUT") + .uri("/upload") + .header(header::HOST, "relay.example") + .header(header::AUTHORIZATION, auth_header) + .header("x-sha-256", &sha256) + .header(header::CONTENT_TYPE, "application/octet-stream") + .body(Body::from(b"fake body".to_vec())) + .expect("upload request"); + + let response = router.oneshot(request).await.expect("response"); + + assert_eq!( + response.status(), + StatusCode::FORBIDDEN, + "upload membership denial must be 403 in Strict mode" + ); + let ct = response + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert_eq!( + ct, "text/plain; charset=utf-8", + "Strict upload membership denial must be exact text/plain; charset=utf-8, got: {ct}" + ); + assert!( + response.headers().get("www-authenticate").is_none(), + "Strict upload 403 must not carry WWW-Authenticate" + ); + let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!( + body.as_ref(), + b"authorization denied\n", + "Strict upload membership body must be 'authorization denied\\n'" + ); + } + + /// Upload membership denial in Permissive mode must preserve the exact + /// legacy JSON body and application/json content-type. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn permissive_upload_membership_denial_keeps_exact_legacy_json_403() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let auth_header = upload_auth_header_for(&keys, &sha256); + let (state, _) = state_and_community(BlossomStrictness::Permissive).await; + let router = axum::Router::new() + .route("/upload", axum::routing::put(upload_blob)) + .with_state(state); + + let request = Request::builder() + .method("PUT") + .uri("/upload") + .header(header::HOST, "relay.example") + .header(header::AUTHORIZATION, auth_header) + .header("x-sha-256", &sha256) + .header(header::CONTENT_TYPE, "application/octet-stream") + .body(Body::from(b"fake body".to_vec())) + .expect("upload request"); + + let response = router.oneshot(request).await.expect("response"); + + assert_eq!( + response.status(), + StatusCode::FORBIDDEN, + "upload membership denial must be 403 in Permissive mode" + ); + let ct = response + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert_eq!( + ct, "application/json", + "Permissive upload denial must be exact application/json CT, got: {ct}" + ); + assert!( + response.headers().get("www-authenticate").is_none(), + "Permissive upload 403 must not carry WWW-Authenticate" + ); + let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!( + body.as_ref(), + br#"{"error":"relay membership required"}"#, + "Permissive upload body must be exact legacy JSON bytes" + ); + } + + /// A confirmed relay member must NOT receive a membership denial on read. + /// + /// Positive control: separates "non-member denied correctly" from an + /// implementation that maps all requests (or all DB errors) into a + /// membership denial — which would pass the denial-only cases above. + /// A member passes the membership gate and reaches the sidecar check, + /// which returns 404 for an unknown blob rather than a membership 403. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn relay_member_read_passes_membership_gate_and_reaches_sidecar() { + let keys = Keys::generate(); + let auth = media_get_auth_header(&keys, media_get_tags_for("relay.example", None)); + let (state, community) = state_and_community(BlossomStrictness::Strict).await; + state + .db + .add_relay_member(community.id, &keys.public_key().to_hex(), "member", None) + .await + .expect("add relay member"); + let router = axum::Router::new() + .route( + "/media/{sha256_ext}", + axum::routing::get(get_blob).head(head_blob), + ) + .with_state(state); + + let response = router + .oneshot(media_request("GET", Some(auth))) + .await + .expect("response"); + + assert_ne!( + response.status(), + StatusCode::FORBIDDEN, + "relay member must not be denied by membership gate" + ); + assert_eq!( + response.status(), + StatusCode::NOT_FOUND, + "relay member must reach the sidecar gate (404 = no such blob)" + ); + } + } // mod postgres_tests + #[test] fn feedback_inline_allows_only_sniffed_passive_raster_images() { // Real magic bytes for the four verified passive raster formats. @@ -1197,7 +1964,7 @@ mod tests { fn media_get_tags_for(host: &str, sha256: Option<&str>) -> Vec { let now = Timestamp::now().as_secs(); - let expiration = (now + 300).to_string(); + let expiration = (now + 55).to_string(); let mut tags = vec![ Tag::parse(["t", "get"]).expect("t tag"), Tag::parse(["expiration", &expiration]).expect("expiration tag"), @@ -1250,7 +2017,7 @@ mod tests { async fn media_read_rejects_upload_verb_wrong_server_and_wrong_x() { let keys = Keys::generate(); let now = Timestamp::now().as_secs(); - let expiration = (now + 300).to_string(); + let expiration = (now + 55).to_string(); let cases = [ vec![ Tag::parse(["t", "upload"]).expect("t tag"), diff --git a/crates/buzz-relay/src/config.rs b/crates/buzz-relay/src/config.rs index 5d831b3f651..6397bf9eb29 100644 --- a/crates/buzz-relay/src/config.rs +++ b/crates/buzz-relay/src/config.rs @@ -209,6 +209,15 @@ pub struct Config { /// are permitted regardless of auth method (API token, NIP-42). pub require_relay_membership: bool, + /// Test-only override for Blossom strictness. + /// + /// In production builds this field is absent — `blossom_strictness_from_state` + /// always returns `Permissive` until #7264 wires `config.nip_fi` into + /// `AppState`. In test builds, set this to `Some(Strict)` to exercise Strict + /// response shapes without activating production enforcement. + #[cfg(test)] + pub test_blossom_strictness: Option, + /// Whether this deployment can serve huddle (voice) audio. /// /// Huddle audio frames are relayed peer-to-peer *within a single pod* @@ -1265,6 +1274,8 @@ impl Config { admin, web_dir, serve_git_web_gui, + #[cfg(test)] + test_blossom_strictness: None, }) } } diff --git a/crates/buzz-test-client/tests/e2e_media.rs b/crates/buzz-test-client/tests/e2e_media.rs index 690fd9c8a50..89484b798a3 100644 --- a/crates/buzz-test-client/tests/e2e_media.rs +++ b/crates/buzz-test-client/tests/e2e_media.rs @@ -26,6 +26,17 @@ fn relay_http_url() -> String { std::env::var("RELAY_HTTP_URL").unwrap_or_else(|_| "http://localhost:3000".to_string()) } +/// Extract the host:port authority from the relay URL for use as the `server` tag. +fn relay_server_authority() -> String { + let url = relay_http_url(); + url.trim_start_matches("https://") + .trim_start_matches("http://") + .split('/') + .next() + .unwrap_or("localhost:3000") + .to_string() +} + fn http_client() -> Client { Client::builder() .timeout(Duration::from_secs(15)) @@ -36,11 +47,13 @@ fn http_client() -> Client { /// Sign a kind:24242 Blossom upload auth event for the given sha256. fn sign_blossom_auth(keys: &Keys, sha256: &str) -> nostr::Event { let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); + let exp_str = (now + 55).to_string(); + let server = relay_server_authority(); let tags = vec![ Tag::parse(["t", "upload"]).expect("t tag"), Tag::parse(["x", sha256]).expect("x tag"), Tag::parse(["expiration", &exp_str]).expect("expiration tag"), + Tag::parse(["server", &server]).expect("server tag"), ]; EventBuilder::new(Kind::from(24242), "Upload test") .tags(tags) @@ -56,11 +69,13 @@ fn sign_blossom_auth(keys: &Keys, sha256: &str) -> nostr::Event { /// one token serves `{sha}.jpg` and `{sha}.thumb.jpg` alike. fn sign_blossom_get_auth(keys: &Keys, sha256: &str) -> nostr::Event { let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); + let exp_str = (now + 55).to_string(); + let server = relay_server_authority(); let tags = vec![ Tag::parse(["t", "get"]).expect("t tag"), Tag::parse(["x", sha256]).expect("x tag"), Tag::parse(["expiration", &exp_str]).expect("expiration tag"), + Tag::parse(["server", &server]).expect("server tag"), ]; EventBuilder::new(Kind::from(24242), "Get test") .tags(tags) diff --git a/crates/buzz-test-client/tests/e2e_media_extended.rs b/crates/buzz-test-client/tests/e2e_media_extended.rs index d8adfaed984..9bd73b13fcb 100644 --- a/crates/buzz-test-client/tests/e2e_media_extended.rs +++ b/crates/buzz-test-client/tests/e2e_media_extended.rs @@ -19,6 +19,17 @@ fn relay_ws_url() -> String { .replace("https://", "wss://") } +/// Extract the host:port authority from the relay URL for use as the `server` tag. +fn relay_server_authority() -> String { + let url = relay_http_url(); + url.trim_start_matches("https://") + .trim_start_matches("http://") + .split('/') + .next() + .unwrap_or("localhost:3000") + .to_string() +} + fn http_client() -> Client { Client::builder() .timeout(Duration::from_secs(15)) @@ -28,10 +39,12 @@ fn http_client() -> Client { fn sign_blossom_auth(keys: &Keys, sha256: &str) -> nostr::Event { let now = Timestamp::now().as_secs(); + let server = relay_server_authority(); let tags = vec![ Tag::parse(["t", "upload"]).unwrap(), Tag::parse(["x", sha256]).unwrap(), - Tag::parse(["expiration", &(now + 300).to_string()]).unwrap(), + Tag::parse(["expiration", &(now + 55).to_string()]).unwrap(), + Tag::parse(["server", &server]).unwrap(), ]; EventBuilder::new(Kind::from(24242), "Upload test") .tags(tags) @@ -43,10 +56,12 @@ fn sign_blossom_auth(keys: &Keys, sha256: &str) -> nostr::Event { /// unconditionally, so round-trip GETs must present one of these. fn sign_blossom_get_auth(keys: &Keys, sha256: &str) -> nostr::Event { let now = Timestamp::now().as_secs(); + let server = relay_server_authority(); let tags = vec![ Tag::parse(["t", "get"]).unwrap(), Tag::parse(["x", sha256]).unwrap(), - Tag::parse(["expiration", &(now + 300).to_string()]).unwrap(), + Tag::parse(["expiration", &(now + 55).to_string()]).unwrap(), + Tag::parse(["server", &server]).unwrap(), ]; EventBuilder::new(Kind::from(24242), "Get test") .tags(tags) @@ -259,7 +274,7 @@ async fn test_auth_wrong_kind() { vec![ Tag::parse(["t", "upload"]).unwrap(), Tag::parse(["x", &sha256]).unwrap(), - Tag::parse(["expiration", &(now + 300).to_string()]).unwrap(), + Tag::parse(["expiration", &(now + 55).to_string()]).unwrap(), ], ); let resp = upload_with_auth(&client, &auth, &sha256, &jpeg).await; @@ -281,7 +296,7 @@ async fn test_auth_missing_t_tag() { "Upload test", vec![ Tag::parse(["x", &sha256]).unwrap(), - Tag::parse(["expiration", &(now + 300).to_string()]).unwrap(), + Tag::parse(["expiration", &(now + 55).to_string()]).unwrap(), ], ); let resp = upload_with_auth(&client, &auth, &sha256, &jpeg).await; @@ -348,7 +363,7 @@ async fn test_auth_empty_content() { vec![ Tag::parse(["t", "upload"]).unwrap(), Tag::parse(["x", &sha256]).unwrap(), - Tag::parse(["expiration", &(now + 300).to_string()]).unwrap(), + Tag::parse(["expiration", &(now + 55).to_string()]).unwrap(), ], ); let resp = upload_with_auth(&client, &auth, &sha256, &jpeg).await; @@ -371,7 +386,7 @@ async fn test_auth_server_tag_mismatch() { vec![ Tag::parse(["t", "upload"]).unwrap(), Tag::parse(["x", &sha256]).unwrap(), - Tag::parse(["expiration", &(now + 300).to_string()]).unwrap(), + Tag::parse(["expiration", &(now + 55).to_string()]).unwrap(), Tag::parse(["server", "evil.example.com"]).unwrap(), ], ); @@ -395,7 +410,7 @@ async fn test_auth_server_tag_correct() { vec![ Tag::parse(["t", "upload"]).unwrap(), Tag::parse(["x", &sha256]).unwrap(), - Tag::parse(["expiration", &(now + 300).to_string()]).unwrap(), + Tag::parse(["expiration", &(now + 55).to_string()]).unwrap(), Tag::parse(["server", "localhost:3000"]).unwrap(), ], ); diff --git a/crates/buzz-test-client/tests/e2e_media_video.rs b/crates/buzz-test-client/tests/e2e_media_video.rs index 2ec0b1e6986..819838aa2c2 100644 --- a/crates/buzz-test-client/tests/e2e_media_video.rs +++ b/crates/buzz-test-client/tests/e2e_media_video.rs @@ -19,6 +19,17 @@ fn relay_http_url() -> String { std::env::var("RELAY_HTTP_URL").unwrap_or_else(|_| "http://localhost:3000".to_string()) } +/// Extract the host:port authority from the relay URL for use as the `server` tag. +fn relay_server_authority() -> String { + let url = relay_http_url(); + url.trim_start_matches("https://") + .trim_start_matches("http://") + .split('/') + .next() + .unwrap_or("localhost:3000") + .to_string() +} + fn http_client() -> Client { Client::builder() .timeout(Duration::from_secs(30)) @@ -28,11 +39,13 @@ fn http_client() -> Client { fn sign_blossom_auth(keys: &Keys, sha256: &str) -> nostr::Event { let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); + let exp_str = (now + 55).to_string(); + let server = relay_server_authority(); let tags = vec![ Tag::parse(["t", "upload"]).expect("t tag"), Tag::parse(["x", sha256]).expect("x tag"), Tag::parse(["expiration", &exp_str]).expect("expiration tag"), + Tag::parse(["server", &server]).expect("server tag"), ]; EventBuilder::new(Kind::from(24242), "Upload test") .tags(tags) @@ -45,11 +58,13 @@ fn sign_blossom_auth(keys: &Keys, sha256: &str) -> nostr::Event { /// the 206 and 416 range behaviour below would never be reached. fn sign_blossom_get_auth(keys: &Keys, sha256: &str) -> nostr::Event { let now = Timestamp::now().as_secs(); - let exp_str = (now + 300).to_string(); + let exp_str = (now + 55).to_string(); + let server = relay_server_authority(); let tags = vec![ Tag::parse(["t", "get"]).expect("t tag"), Tag::parse(["x", sha256]).expect("x tag"), Tag::parse(["expiration", &exp_str]).expect("expiration tag"), + Tag::parse(["server", &server]).expect("server tag"), ]; EventBuilder::new(Kind::from(24242), "Get test") .tags(tags) diff --git a/desktop/src-tauri/src/commands/media.rs b/desktop/src-tauri/src/commands/media.rs index 7e908d35c5c..20924147f0c 100644 --- a/desktop/src-tauri/src/commands/media.rs +++ b/desktop/src-tauri/src/commands/media.rs @@ -294,20 +294,21 @@ pub(crate) fn detect_and_validate_mime(body: &[u8]) -> Result { Ok(mime) } -/// Lifetime of a Blossom `t=get` read token. Ten minutes keeps a token alive -/// across a video's range-request stream while staying well inside the -/// server's `created_at` freshness window (3600s, matching upload). -pub(crate) const MEDIA_GET_AUTH_EXPIRY_SECS: u64 = 600; +/// Lifetime of a Blossom `t=get` read token. +/// +/// 60 seconds is the maximum permitted by NIP-FI §Freshness: `expiration <= created_at + 60s`. +/// Callers that make range requests across longer video playback sessions must re-mint on +/// a 401 or 403 from the server (expired/rejected token), using a single fresh mint+retry. +pub(crate) const MEDIA_GET_AUTH_EXPIRY_SECS: u64 = 60; /// Sign a Blossom (BUD-01) `t=get` authorization event, server-scoped to the /// relay's authority, and return the full `Authorization` header value. /// /// Server-scoped (a `server` tag, no `x` tag): one token authorizes reads of -/// any blob on that host for its lifetime, which keeps avatar-grid bursts and -/// video range requests cheap. This is deliberately broader than per-blob -/// scoping and is safe only because the relay still enforces NIP-43 -/// membership on the verified pubkey — and because callers only attach this -/// header to requests bound for the relay origin itself. +/// any blob on that host for its lifetime. Callers must only attach this +/// header to requests bound for the relay origin itself (never third-party +/// origins). Tokens expire after `expiry_secs` (≤ 60 per NIP-FI §Freshness) +/// and must be re-minted on 401 or 403 for long-running range-request streams. pub(crate) fn sign_blossom_get_auth_header( keys: &Keys, base_url: &str, @@ -365,16 +366,16 @@ pub(crate) fn sign_blossom_upload_auth( expiry_secs: u64, base_url: &str, ) -> Result { + let server = extract_server_authority(base_url) + .ok_or_else(|| "cannot derive server authority from relay URL".to_string())?; let now = Timestamp::now().as_secs(); - let mut tags = vec![ + let tags = vec![ Tag::parse(vec!["t", "upload"]).map_err(|e| e.to_string())?, Tag::parse(vec!["x", sha256]).map_err(|e| e.to_string())?, Tag::parse(vec!["expiration", &(now + expiry_secs).to_string()]) .map_err(|e| e.to_string())?, + Tag::parse(vec!["server".to_string(), server]).map_err(|e| e.to_string())?, ]; - if let Some(domain) = extract_server_authority(base_url) { - tags.push(Tag::parse(vec!["server".to_string(), domain]).map_err(|e| e.to_string())?); - } EventBuilder::new(Kind::from(24242), "Upload buzz-media") .tags(tags) .sign_with_keys(keys) @@ -414,14 +415,12 @@ async fn do_upload( ) -> Result { let sha256 = hex::encode(Sha256::digest(&body)); - // Video uploads get a 1-hour auth window to survive slow connections; - // images use 5 minutes. Must match the server-side max_age_secs values - // in process_upload (600s) and process_video_upload (3600s). - let expiry_secs = if mime.starts_with("video/") { - 3600 - } else { - 300 - }; + // All upload tokens use a 60-second expiry window per NIP-FI §Freshness: + // `expiration <= created_at + 60s`. Upload tokens are minted immediately + // before the PUT request, so 60 seconds is ample for any upload over a + // reasonable connection (the body is already hashed and ready to send). + // The server-side window is also 60s in Strict mode, matching this value. + let expiry_secs = 60u64; let base_url = relay_api_base_url_with_override(state); let auth_event = { let keys = state.signing_keys()?; @@ -843,7 +842,7 @@ mod tests { #[test] fn test_sign_blossom_get_auth_header_shape() { let keys = Keys::generate(); - let header = sign_blossom_get_auth_header(&keys, "http://localhost:3000", 600).unwrap(); + let header = sign_blossom_get_auth_header(&keys, "http://localhost:3000", 60).unwrap(); let b64 = header.strip_prefix("Nostr ").expect("Nostr scheme prefix"); let json = URL_SAFE_NO_PAD.decode(b64).unwrap(); let event = nostr::Event::from_json(std::str::from_utf8(&json).unwrap()).unwrap(); @@ -863,13 +862,48 @@ mod tests { assert!(tag("x").is_none()); let expiration: u64 = tag("expiration").unwrap().parse().unwrap(); let now = Timestamp::now().as_secs(); - assert!(expiration > now && expiration <= now + 600); + // Token expiry must be within the 60s NIP-FI window. + assert!(expiration > now && expiration <= now + 60); } #[test] fn test_sign_blossom_get_auth_header_invalid_base_url() { let keys = Keys::generate(); - assert!(sign_blossom_get_auth_header(&keys, "not-a-url", 600).is_err()); + assert!(sign_blossom_get_auth_header(&keys, "not-a-url", 60).is_err()); + } + + #[test] + fn test_sign_blossom_upload_auth_shape() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + let base_url = "http://relay.example:3000"; + let event = sign_blossom_upload_auth(&keys, &sha256, 60, base_url).unwrap(); + + assert_eq!(event.kind, nostr::Kind::from(24242)); + event.verify().expect("valid signature"); + + let tag = |name: &str| -> Option { + event.tags.iter().find_map(|t| { + let v = t.as_slice(); + (v.first().map(String::as_str) == Some(name)).then(|| v[1].clone()) + }) + }; + assert_eq!(tag("t").as_deref(), Some("upload")); + assert_eq!(tag("x").as_deref(), Some(sha256.as_str())); + // server tag MUST be present (NIP-FI §Upload proofs) + assert_eq!(tag("server").as_deref(), Some("relay.example:3000")); + // expiry must be within the 60s NIP-FI window + let expiration: u64 = tag("expiration").unwrap().parse().unwrap(); + let now = nostr::Timestamp::now().as_secs(); + assert!(expiration > now && expiration <= now + 60); + } + + #[test] + fn test_sign_blossom_upload_auth_fails_without_valid_url() { + let keys = Keys::generate(); + let sha256 = "a".repeat(64); + // URL that cannot yield a server authority → should fail (server tag mandatory) + assert!(sign_blossom_upload_auth(&keys, &sha256, 60, "not-a-url").is_err()); } #[test] diff --git a/desktop/src-tauri/src/media_proxy.rs b/desktop/src-tauri/src/media_proxy.rs index 21692ce5237..79128eca0f8 100644 --- a/desktop/src-tauri/src/media_proxy.rs +++ b/desktop/src-tauri/src/media_proxy.rs @@ -76,6 +76,38 @@ async fn proxy_handler(AxumState(state): AxumState, req: Request) -> } }; + // Re-mint and retry once on an auth failure (401 missing_evidence or 403 + // evidence_rejected). Under NIP-FI strict mode a read token expires after + // 60s, so a long video range-request stream will exhaust its token mid-flight. + // Single retry; if the fresh token also fails, propagate the error to the caller. + let resp = if (resp.status() == reqwest::StatusCode::UNAUTHORIZED + || resp.status() == reqwest::StatusCode::FORBIDDEN) + && has_range + { + if let Some(fresh_auth) = mint_media_get_auth(&app_state, &base_url) { + let mut retry = state + .client + .get(&upstream_url) + .timeout(std::time::Duration::from_secs(120)) + .header("authorization", fresh_auth); + if let Some(range) = req.headers().get("range") { + if let Ok(v) = range.to_str() { + retry = retry.header("range", v); + } + } + match retry.send().await { + Ok(r) => r, + Err(_) => { + return (StatusCode::BAD_GATEWAY, "upstream request failed").into_response(); + } + } + } else { + resp + } + } else { + resp + }; + let status = StatusCode::from_u16(resp.status().as_u16()).unwrap_or(StatusCode::INTERNAL_SERVER_ERROR); @@ -199,6 +231,34 @@ pub async fn handle_buzz_media( let result = upstream.send().await; + // Re-mint and retry once on auth failure (401/403) for range requests. + // Under NIP-FI strict mode a read token expires after 60s, exhausting + // mid-stream. Single retry with a fresh token; propagate on second failure. + let result = match result { + Ok(resp) + if (resp.status() == reqwest::StatusCode::UNAUTHORIZED + || resp.status() == reqwest::StatusCode::FORBIDDEN) + && has_range => + { + if let Some(fresh_auth) = mint_media_get_auth(&state, &base) { + let mut retry = state + .http_client + .get(&upstream_url) + .timeout(std::time::Duration::from_secs(60)) + .header("authorization", fresh_auth); + if let Some(range) = request.headers().get("range") { + if let Ok(v) = range.to_str() { + retry = retry.header("range", v); + } + } + retry.send().await + } else { + Ok(resp) + } + } + other => other, + }; + match result { Ok(resp) => { let status = resp.status().as_u16(); diff --git a/docs/nips/NIP-FI.md b/docs/nips/NIP-FI.md index fee7909dfda..fe09061b569 100644 --- a/docs/nips/NIP-FI.md +++ b/docs/nips/NIP-FI.md @@ -744,10 +744,15 @@ constitutes a **known gap** in this section's security guarantees. #### Compliance note -The implementation as of PR #7264 pairs via a permissive Blossom verifier and -is explicitly non-compliant with this section. The named gaps are: -multi-tag acceptance, a 3600-second proof window, and an optional `server` -tag. These are resolved when the bounded hardening task lands. +The strict verifier (exact cardinality, mandatory `server` tag, 60-second proof +window, and 401/403 denial-class split) is implemented in `buzz-media/src/auth.rs` +as `BlossomStrictness::Strict`. It is engaged when NIP-FI active modes are +configured; deployment requires the NIP-FI HTTP enforcement PR (#7264) to be +merged first. Until that PR lands the verifier runs in `Permissive` mode, +which preserves pre-NIP-FI behavior [FI-INV-15]. + +The deny-map gap described above remains a **known gap** pending the S4 +issuer-scoped deny-map integration. ### Request format diff --git a/mobile/lib/features/channels/media_viewer_page/video_viewer.dart b/mobile/lib/features/channels/media_viewer_page/video_viewer.dart index 7267795734a..02bebfadd00 100644 --- a/mobile/lib/features/channels/media_viewer_page/video_viewer.dart +++ b/mobile/lib/features/channels/media_viewer_page/video_viewer.dart @@ -19,6 +19,13 @@ class MediaVideoViewerPage extends HookConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { final controller = useState(null); + // Tracks a VideoPlayerController that has been created (i.e. platform + // resources allocated via createWithOptions()) but whose initialize()+play() + // chain has not yet completed or failed. The effect cleanup path disposes + // this directly so a close-during-init does not leak the native player, + // even when the async chain is suspended waiting for the initialized event + // or for play() to return. + final pendingController = useRef(null); final videoFile = useRef(null); final downloadRequestAbort = useRef?>(null); final downloadSubscription = useRef>?>(null); @@ -48,34 +55,13 @@ class MediaVideoViewerPage extends HookConsumerWidget { final auth = ref.read(mediaGetAuthServiceProvider); final uri = Uri.parse(videoUrl); - // ExoPlayer supports the request headers on every range request, so - // keep Android on its streaming path. iOS uses the authenticated local - // copy below because AVPlayer can drop those headers after the first - // request. - if (Platform.isAndroid) { - VideoPlayerController? streamingController; - try { - streamingController = VideoPlayerController.networkUrl( - uri, - httpHeaders: auth.headersFor(videoUrl), - ); - await streamingController.initialize(); - await streamingController.play(); - if (disposed) { - await streamingController.dispose(); - return; - } - controller.value = streamingController; - return; - } catch (_) { - if (streamingController != null) { - await streamingController.dispose(); - } - // Fall through to the authenticated local-file path only when the - // streaming controller cannot initialize. - } - } - + // All platforms: download to an authenticated local file so the proof + // is bound at request time rather than frozen into controller headers. + // (iOS already used this path; Android previously used streaming headers + // but video_player_android 2.9.5 freezes those headers into static + // DefaultHttpDataSource request properties — a proof minted at + // controller creation time becomes stale after 60 s, causing seeks + // outside the buffer to fail with expiry rejection.) try { final client = ref.read(mediaHttpClientProvider); final requestAbort = Completer(); @@ -85,6 +71,14 @@ class MediaVideoViewerPage extends HookConsumerWidget { uri, abortTrigger: requestAbort.future, )..headers.addAll(auth.headersFor(videoUrl)); + // A GET carries no request body. StreamedRequest's sink MUST be + // closed to signal end-of-stream: IOClient.send() awaits + // stream.pipe(ioRequest) before returning a response, and pipe + // blocks until the source stream ends. Without close(), every + // download hangs in loading until the request is aborted. + // close() is unawaited because it may not complete until after + // the pipe is in progress (streamed_request.dart:15-29). + unawaited(request.sink.close()); late final http.StreamedResponse response; try { response = await client.send(request); @@ -98,7 +92,12 @@ class MediaVideoViewerPage extends HookConsumerWidget { return; } if (response.statusCode < 200 || response.statusCode >= 300) { - await response.stream.drain(); + // Cancel (not drain) the error-body stream so a stalled server body + // cannot hold the download open. `drain()` waits for the upstream + // to close the stream; `_cancelVideoResponse` subscribes and + // immediately cancels, which closes the underlying connection without + // waiting for the full response body [F2r(b)]. + await _cancelVideoResponse(response); throw HttpException( 'Video download failed (${response.statusCode})', uri: uri, @@ -146,14 +145,72 @@ class MediaVideoViewerPage extends HookConsumerWidget { } final localController = VideoPlayerController.file(file); - await localController.initialize(); - await localController.play(); - if (disposed) { - await localController.dispose(); - await deleteVideoFile(); - return; + // Register as pending BEFORE the first async suspension + // (initialize()) so the effect cleanup can always reach it. + // video_player 2.11.1 allocates the native player synchronously + // inside createWithOptions() before _creatingCompleter completes; + // a close arriving at any point after this line will find the + // controller in pendingController and dispose it correctly. + pendingController.value = localController; + // Own the controller before any async suspension so a failed + // initialize() or play() — or a disposal that races with init — + // can always call dispose() unconditionally [F2r(a)]. + // video_player 2.11.1 completes the init future with an error on + // native failure but does NOT dispose the player; Android 2.9.5 + // retains the native player until explicit disposal. Without this + // wrapper, a PlatformException from initialize() unwinds to the + // outer catch where controller.value is still null, so the cleanup + // teardown's `if (activeController != null)` guard silently skips + // disposal — leaking the native player and its event subscription. + try { + await localController.initialize(); + if (disposed) { + // Effect cleanup will also see pendingController.value and + // dispose it; clear the ref here to avoid a double-dispose. + pendingController.value = null; + await localController.dispose(); + await deleteVideoFile(); + return; + } + await localController.play(); + if (disposed) { + pendingController.value = null; + await localController.dispose(); + await deleteVideoFile(); + return; + } + pendingController.value = null; + controller.value = localController; + } catch (_) { + // Start disposal without awaiting it, then rethrow immediately. + // + // video_player 2.11.1 initialize() creates _creatingCompleter at + // the top of the method, then awaits createWithOptions() before + // completing it (video_player.dart:546,587-590). If + // createWithOptions() itself throws, _creatingCompleter is never + // completed, and dispose() waits on it unconditionally at :682-683. + // Awaiting dispose() here would therefore deadlock: the outer catch + // never sets error.value, the error UI is never shown, and the + // viewer is left in an infinite loading state. + // + // Note: if createWithOptions() throws, _creatingCompleter is never + // completed, so the unawaited disposal stalls at the same wait. + // This bypasses the deadlock for the outer catch but does not + // release the native player in the create-failure case. After a + // successful create, _creatingCompleter is completed at :590, so + // the detached disposal runs normally; errors from that detached + // future are caught and logged below rather than becoming uncaught + // async errors [F2r(d)]. + pendingController.value = null; + unawaited( + localController.dispose().catchError((Object disposeError) { + debugPrint( + '[VideoViewer] dispose() failed after load error: $disposeError', + ); + }), + ); + rethrow; } - controller.value = localController; } catch (loadError) { if (!disposed) error.value = loadError.toString(); } @@ -168,6 +225,15 @@ class MediaVideoViewerPage extends HookConsumerWidget { } unawaited(downloadSubscription.value?.cancel() ?? Future.value()); unawaited(downloadSink.value?.close() ?? Future.value()); + // Dispose whichever controller is reachable: a controller that has + // finished init+play and been published to controller.value, OR one + // that is still mid-init (registered in pendingController before the + // first await). Exactly one of these is non-null at any moment; + // clearing both refs prevents a double-dispose if initializeVideo() + // races with the teardown. + final activePending = pendingController.value; + pendingController.value = null; + if (activePending != null) unawaited(activePending.dispose()); final activeController = controller.value; if (activeController != null) unawaited(activeController.dispose()); unawaited(deleteVideoFile()); diff --git a/mobile/lib/shared/relay/media_auth.dart b/mobile/lib/shared/relay/media_auth.dart index b21eeca36d6..8611ce5b565 100644 --- a/mobile/lib/shared/relay/media_auth.dart +++ b/mobile/lib/shared/relay/media_auth.dart @@ -7,10 +7,12 @@ import 'package:nostr/nostr.dart' as nostr; import 'relay_provider.dart'; const _mediaGetAuthKind = 24242; -const _mediaGetAuthLifetimeSeconds = 600; +const _mediaGetAuthLifetimeSeconds = 60; /// Re-sign this long before the cached auth event expires, so an in-flight /// request signed just before the boundary still lands well within validity. +/// With a 60-second lifetime, the margin equals the lifetime: each request +/// mints a fresh token (mint-per-request pattern for NIP-FI compliance). const _mediaGetAuthRefreshMarginSeconds = 60; /// Builds BUD-01 Blossom `t=get` auth headers for relay-host media URLs. @@ -19,11 +21,13 @@ const _mediaGetAuthRefreshMarginSeconds = 60; /// so callers can safely use this on arbitrary profile/custom-emoji URLs without /// leaking Buzz credentials to third-party hosts. /// -/// The signed header is memoized until [_mediaGetAuthRefreshMarginSeconds] -/// before expiry: repeated calls return the byte-identical map instead of -/// producing a fresh Schnorr signature per widget build. The service itself is -/// rebuilt (dropping the memo) whenever the relay config — base URL or signing -/// identity — changes, via [mediaGetAuthServiceProvider]. +/// With a 60-second proof lifetime the refresh margin equals the lifetime, so +/// `_refreshAt == signedAt` and the cache never hits: every call mints a fresh +/// proof (mint-per-request pattern). This is intentional for NIP-FI compliance +/// — caching a 60-second token is staleness-prone and provides no meaningful +/// reduction in signing work. The service itself is rebuilt whenever the relay +/// config — base URL or signing identity — changes, via +/// [mediaGetAuthServiceProvider]. class MediaGetAuthService { final String _baseUrl; final String? _nsec; diff --git a/mobile/lib/shared/relay/media_upload.dart b/mobile/lib/shared/relay/media_upload.dart index 4f79080efd8..21f5de4df2d 100644 --- a/mobile/lib/shared/relay/media_upload.dart +++ b/mobile/lib/shared/relay/media_upload.dart @@ -33,7 +33,7 @@ const _requiresLegacyMediaStoragePermissionMethod = const _readClipboardImageMethod = 'readClipboardImage'; const _clipboardHasImageMethod = 'clipboardHasImage'; const _uploadAuthKind = 24242; -const _uploadAuthLifetimeSeconds = 300; +const _uploadAuthLifetimeSeconds = 60; const _heicBrands = { 'heic', 'heix', @@ -688,12 +688,17 @@ class MediaUploadService { final expiration = (_now().millisecondsSinceEpoch ~/ 1000) + _uploadAuthLifetimeSeconds; + final serverAuthority = extractServerAuthority(_baseUrl); + if (serverAuthority == null) { + throw Exception( + 'Cannot mint upload auth: no server authority in relay URL: $_baseUrl', + ); + } final tags = >[ ['t', 'upload'], ['x', sha256], ['expiration', '$expiration'], - if (extractServerAuthority(_baseUrl) case final authority?) - ['server', authority], + ['server', serverAuthority], ]; return nostr.Event.from( diff --git a/mobile/pubspec.lock b/mobile/pubspec.lock index 76e9e9d6d4d..023e9666fa3 100644 --- a/mobile/pubspec.lock +++ b/mobile/pubspec.lock @@ -1089,7 +1089,7 @@ packages: source: hosted version: "2.2.1" path_provider_platform_interface: - dependency: transitive + dependency: "direct dev" description: name: path_provider_platform_interface sha256: "88f5779f72ba699763fa3a3b06aa4bf6de76c8e5de842cf6f29e2e06476c2334" @@ -1129,7 +1129,7 @@ packages: source: hosted version: "3.1.6" plugin_platform_interface: - dependency: transitive + dependency: "direct dev" description: name: plugin_platform_interface sha256: "4820fbfdb9478b1ebae27888254d445073732dae3d6ea81f0b7e06d5dedc3f02" @@ -1654,7 +1654,7 @@ packages: source: hosted version: "2.9.4" video_player_platform_interface: - dependency: transitive + dependency: "direct dev" description: name: video_player_platform_interface sha256: "57c5d73173f76d801129d0531c2774052c5a7c11ccb962f1830630decd9f24ec" diff --git a/mobile/pubspec.yaml b/mobile/pubspec.yaml index f4f0bcac7aa..5e53e37e986 100644 --- a/mobile/pubspec.yaml +++ b/mobile/pubspec.yaml @@ -59,6 +59,10 @@ dev_dependencies: custom_lint: ^0.8.0 riverpod_lint: ^3.1.0 mocktail: ^1.0.4 + # Test-only platform interface fakes (depend_on_referenced_packages). + path_provider_platform_interface: ^2.1.2 + plugin_platform_interface: ^2.1.8 + video_player_platform_interface: ^6.6.0 flutter: uses-material-design: true diff --git a/mobile/test/features/channels/media_viewer_page/video_viewer_test.dart b/mobile/test/features/channels/media_viewer_page/video_viewer_test.dart new file mode 100644 index 00000000000..b6f2a5e30fb --- /dev/null +++ b/mobile/test/features/channels/media_viewer_page/video_viewer_test.dart @@ -0,0 +1,920 @@ +// Regression tests for MediaVideoViewerPage lifecycle fixes: +// +// F2r(a): VideoPlayerController is disposed when native initialisation fails. +// Before the fix, a PlatformException from initialize() unwound to the +// outer catch with controller.value still null — the cleanup teardown's +// `if (activeController != null)` guard silently skipped disposal, +// leaking the native player and its event subscription. +// +// F2r(b): A non-2xx error-body is cancelled (listen+cancel) rather than +// drained. Before the fix, `response.stream.drain()` waited for the +// server to close the stream — a stalled error body (e.g. a 403 on +// a slow connection) could block initializeVideo indefinitely, and +// there was no abort handle to cancel it. +// +// F2r(c): A VideoPlayerController created but never initialized (pending) +// is disposed when the widget is unmounted. Before the fix, the +// controller lived only in the async function's stack frame; a close +// arriving while initialize() was awaiting the initialized event left +// the native player allocated forever. +// +// F2r(d): createWithOptions() failure shows the error UI instead of leaving +// the viewer in an infinite loading state. video_player 2.11.1 +// creates _creatingCompleter before awaiting createWithOptions() and +// completes it only AFTER the await returns. If creation throws, +// _creatingCompleter is never completed and dispose() deadlocks waiting +// on it. The fix uses unawaited(dispose()) in the catch so the outer +// catch runs immediately and sets error.value. +// +// Transport: AbortableStreamedRequest sink must be closed before send(). +// Without it, IOClient.send() awaits stream.pipe(ioRequest) which +// blocks until the sink is closed — every download hangs indefinitely +// with the real http.Client. Two probes: a local loopback server that +// reads the full request body before replying (fails if sink is not +// closed), and a MockClient-based fake that calls request.finalize() +// for unit coverage. + +import 'dart:async'; +import 'dart:io'; + +import 'package:buzz/features/channels/media_viewer_page.dart'; +import 'package:buzz/shared/relay/media_auth.dart'; +import 'package:buzz/shared/relay/media_image.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; +import 'package:plugin_platform_interface/plugin_platform_interface.dart'; +import 'package:video_player_platform_interface/video_player_platform_interface.dart'; + +import '../../../helpers/widget_helpers.dart'; + +// ── Fakes ──────────────────────────────────────────────────────────────────── + +/// Minimal fake for path_provider so getTemporaryDirectory() works in tests. +/// Uses MockPlatformInterfaceMixin to bypass PlatformInterface.verify(). +class _FakePathProviderPlatform extends Fake + with MockPlatformInterfaceMixin + implements PathProviderPlatform { + @override + Future getTemporaryPath() async => + Directory.systemTemp.resolveSymbolicLinksSync(); +} + +/// Fake VideoPlayerPlatform that tracks `dispose` calls and optionally +/// forces native initialisation to fail with a PlatformException. +/// +/// Extends VideoPlayerPlatform directly (inheriting the platform token from the +/// super constructor) so PlatformInterface.verify() succeeds without needing +/// MockPlatformInterfaceMixin. +class _FakeVideoPlayerPlatform extends VideoPlayerPlatform { + final bool forceInitError; + final bool neverInitialize; + // When true, createWithOptions() itself throws a PlatformException. + // video_player 2.11.1 awaits createWithOptions() before completing + // _creatingCompleter (video_player.dart:587-590); if creation throws, + // _creatingCompleter is never completed and dispose() deadlocks waiting + // on it. This flag exercises the F2r(d) production fix. + final bool forceCreateError; + int disposeCallCount = 0; + int nextPlayerId = 0; + final Map> _streams = {}; + + _FakeVideoPlayerPlatform({ + this.forceInitError = false, + this.neverInitialize = false, + this.forceCreateError = false, + }); + + @override + Future init() async {} + + @override + Future createWithOptions(VideoCreationOptions options) async { + if (forceCreateError) { + throw PlatformException( + code: 'VideoError', + message: 'Fake native create failure', + ); + } + return create(options.dataSource); + } + + @override + Future create(DataSource dataSource) async { + final id = nextPlayerId++; + final controller = StreamController( + onListen: () { + // Emit the event/error only when the stream is first subscribed so that + // VideoPlayerController.initialize() is already listening. Emitting + // before the subscription means the event is dropped and initialize() + // hangs waiting for the initialized signal. + if (forceInitError) { + _streams[id]!.addError( + PlatformException( + code: 'VideoError', + message: 'Fake native init failure', + ), + ); + } else if (!neverInitialize) { + _streams[id]!.add( + VideoEvent( + eventType: VideoEventType.initialized, + size: const Size(100, 100), + duration: const Duration(seconds: 1), + ), + ); + } + // neverInitialize: no event emitted — initialize() hangs forever. + }, + ); + _streams[id] = controller; + return id; + } + + @override + Future dispose(int playerId) async { + disposeCallCount++; + // Record the dispose call and close the event stream. + // + // Note: an error injected here does NOT reach initialize()'s pending + // listener on the pinned video_player 2.11.1 path. dispose() cancels + // _eventSubscription (:687) before calling _videoPlayerPlatform.dispose() + // (:688), so any event emitted here goes to a closed listener. This fake + // records the disposal call and closes its stream; it does not settle the + // pending initialize() future. + final stream = _streams[playerId]; + if (stream != null) { + if (!stream.isClosed) { + stream.addError( + StateError('VideoPlayerController disposed before initialization'), + ); + } + await stream.close(); + } + } + + /// Return a minimal stand-in widget. VideoPlayerPlatform.buildViewWithOptions + /// delegates to this; without it every test that successfully initializes a + /// player throws UnimplementedError when the VideoPlayer widget renders. + @override + Widget buildView(int playerId) => const SizedBox.shrink(); + + @override + Stream videoEventsFor(int playerId) => _streams[playerId]!.stream; + + @override + Future play(int playerId) async {} + + @override + Future pause(int playerId) async {} + + @override + Future setLooping(int playerId, bool looping) async {} + + @override + Future setVolume(int playerId, double volume) async {} + + @override + Future seekTo(int playerId, Duration position) async {} + + @override + Future setPlaybackSpeed(int playerId, double speed) async {} + + @override + Future getPosition(int playerId) async => Duration.zero; + + @override + Future setMixWithOthers(bool mixWithOthers) async {} +} + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +/// A [VideoPlayerPlatform] that creates successfully (returns a real player ID +/// and emits a forceInitError event to trigger an init/play failure), but whose +/// [dispose()] throws a [PlatformException]. +/// +/// Used to verify that the viewer's `unawaited(dispose().catchError(...))` path +/// does NOT surface an uncaught async error when disposal fails after a +/// post-create load failure. +class _FailingDisposeVideoPlayerPlatform extends VideoPlayerPlatform { + int disposeCallCount = 0; + int nextPlayerId = 0; + final Map> _streams = {}; + + /// Completed when dispose() is first entered. Awaited inside + /// [WidgetTester.runAsync] with a bounded real-zone timeout; timers created + /// in that zone fire normally, unlike FakeAsync timers outside runAsync. + final Completer disposedCompleter = Completer(); + + @override + Future init() async {} + + @override + Future createWithOptions(VideoCreationOptions options) async { + return create(options.dataSource); + } + + @override + Future create(DataSource dataSource) async { + final id = nextPlayerId++; + final controller = StreamController( + onListen: () { + // Emit a PlatformException so initialize() throws, reaching the inner catch. + _streams[id]!.addError( + PlatformException( + code: 'VideoError', + message: 'Fake post-create init failure', + ), + ); + }, + ); + _streams[id] = controller; + return id; + } + + @override + Future dispose(int playerId) async { + disposeCallCount++; + if (!disposedCompleter.isCompleted) disposedCompleter.complete(); + // Throw to simulate a native disposal failure. + // The production catch(.catchError) must absorb this without propagating + // an uncaught async error while the viewer is showing its error UI. + throw PlatformException( + code: 'DisposalError', + message: 'Fake native disposal failure', + ); + } + + @override + Widget buildView(int playerId) => const SizedBox.shrink(); + + @override + Stream videoEventsFor(int playerId) => _streams[playerId]!.stream; + + @override + Future play(int playerId) async {} + + @override + Future pause(int playerId) async {} + + @override + Future setLooping(int playerId, bool looping) async {} + + @override + Future setVolume(int playerId, double volume) async {} + + @override + Future seekTo(int playerId, Duration position) async {} + + @override + Future setPlaybackSpeed(int playerId, double speed) async {} + + @override + Future getPosition(int playerId) async => Duration.zero; + + @override + Future setMixWithOthers(bool mixWithOthers) async {} +} + +/// Returns a [http.StreamedResponse] with the given [statusCode] whose body +/// stream is controlled by [bodyController]. The caller closes [bodyController] +/// to release a drain; leaving it open proves that the fix (listen+cancel) +/// completes without waiting for the stream to close. +http.StreamedResponse _streamedResponse( + int statusCode, + StreamController> bodyController, +) => http.StreamedResponse(bodyController.stream, statusCode); + +/// A no-op auth service (returns empty headers for any URL, including +/// non-relay URLs so the test media URL does not need a signed nsec). +MediaGetAuthService _noopAuth() => + MediaGetAuthService(baseUrl: 'https://relay.test', nsec: null); + +/// A fake [http.Client] that calls [request.finalize()] and drains the +/// request body before returning a response. If the request sink is not +/// closed, finalize() returns an open stream and the drain hangs — which is +/// exactly what the sink-fix prevents. +class _FinalizingFakeClient extends http.BaseClient { + final http.StreamedResponse Function() responseBuilder; + bool requestBodyDrained = false; + + _FinalizingFakeClient({required this.responseBuilder}); + + @override + Future send(http.BaseRequest request) async { + // Drain the finalized request body. If sink.close() was not called this + // stream never ends and the test times out — verifying the transport fix. + await request.finalize().drain(); + requestBodyDrained = true; + return responseBuilder(); + } +} + +/// A fake [http.Client] for viewer-path abort tests. +/// +/// `send()` signals arrival immediately, then suspends until the request's +/// `abortTrigger` completes. When the viewer's effect cleanup fires +/// `downloadRequestAbort.complete()`, that trigger arrives here and +/// `send()` throws [RequestAbortedException] — proving that unmounting +/// the widget closes the in-flight download through the actual viewer +/// abort-wiring path. +/// +/// The fake requires an [http.AbortableStreamedRequest] with a non-null +/// trigger. If the viewer's wiring is absent, `send()` throws [StateError], +/// which the viewer catches at its outer `catch (loadError)` boundary. The +/// test then fails at the [abortObservedCompleter] deadline because the abort +/// is never observed — not immediately, but after the deadline expires. +/// +/// No drain: this fake's only job is the abort-trigger chain. Sink-close +/// correctness is covered separately by [_FinalizingFakeClient]. +class _StallingAbortableClient extends http.BaseClient { + final Completer requestArrivedCompleter = Completer(); + + /// Completed when `abortObserved` is set; use with a deadline instead of sleeping. + final Completer abortObservedCompleter = Completer(); + bool abortObserved = false; + + @override + Future send(http.BaseRequest request) async { + if (!requestArrivedCompleter.isCompleted) { + requestArrivedCompleter.complete(); + } + // Require an abortable request with a non-null trigger. If the viewer's + // AbortableStreamedRequest wiring or the abortTrigger: parameter is absent, + // fail immediately — do NOT treat missing wiring as a successful abort. + if (request case http.AbortableStreamedRequest(:final abortTrigger?)) { + await abortTrigger; + abortObserved = true; + if (!abortObservedCompleter.isCompleted) { + abortObservedCompleter.complete(); + } + throw http.RequestAbortedException(request.url); + } + throw StateError( + '_StallingAbortableClient: expected AbortableStreamedRequest with ' + 'non-null abortTrigger; got ${request.runtimeType}. ' + 'The viewer abort wiring is absent.', + ); + } +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + setUp(() { + PathProviderPlatform.instance = _FakePathProviderPlatform(); + }); + + // Transport: request sink must be closed before send(). + // + // Red-with-old-code: before the `unawaited(request.sink.close())` fix, + // _FinalizingFakeClient.send() drained an open stream and the test timed + // out. With the fix the drain completes immediately, the download succeeds, + // and the video controller initializes. + testWidgets( + 'Transport: request sink is closed before send() — fake drain probe', + (tester) async { + final fakePlayer = _FakeVideoPlayerPlatform(); + VideoPlayerPlatform.instance = fakePlayer; + + final fakeClient = _FinalizingFakeClient( + responseBuilder: () => + http.StreamedResponse(Stream.value([0, 1, 2, 3]), 200), + ); + addTearDown(fakeClient.close); + + await tester.runAsync(() async { + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(fakeClient), + ], + child: const MediaVideoViewerPage( + videoUrl: 'https://relay.test/media/abc.mp4', + ), + ), + ); + await Future.delayed(const Duration(milliseconds: 300)); + }); + await tester.pump(); + + expect( + fakeClient.requestBodyDrained, + isTrue, + reason: 'request sink must be closed so send() can finalize the body', + ); + }, + ); + + // Transport: request sink must be closed before send(). + // + // Real-IO loopback probes (TestWidgetsFlutterBinding intercepts HttpClient + // within this suite) live in video_viewer_transport_test.dart, which uses + // plain test() without TestWidgetsFlutterBinding. The fake drain probe + // below covers the same contract without the binding conflict. + // + // + // Red-with-old-code: before the fix the `localController` was created but + // only published to `controller.value` after a successful initialize()+play(). + // On error the outer `catch (loadError)` ran without ever calling + // `localController.dispose()`. With forceInitError=true the fake emits a + // PlatformException; `initialize()` throws; the new inner catch calls + // `dispose()` before rethrowing. disposeCallCount >= 1 verifies it. + testWidgets('F2r(a): VideoPlayerController is disposed when native init fails', ( + tester, + ) async { + final fakePlayer = _FakeVideoPlayerPlatform(forceInitError: true); + VideoPlayerPlatform.instance = fakePlayer; + + // 200-ok response with a tiny immediate body so the download phase + // completes and initializeVideo() reaches the VideoPlayerController.file() + // path. The _FinalizingFakeClient drains the request body, which proves + // the sink is closed (if not, the drain hangs and the test times out). + final fakeClient = _FinalizingFakeClient( + responseBuilder: () => + http.StreamedResponse(Stream.value([0, 1, 2, 3]), 200), + ); + addTearDown(fakeClient.close); + + await tester.runAsync(() async { + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(fakeClient), + ], + child: const MediaVideoViewerPage( + videoUrl: 'https://relay.test/media/abc.mp4', + ), + ), + ); + // Give the initializeVideo() async chain time to complete: HTTP response, + // file write, VideoPlayerController.initialize(), and dispose(). + await Future.delayed(const Duration(milliseconds: 300)); + }); + await tester.pump(); + + // The fake must have recorded at least one dispose() call, confirming + // the native player was released even on an initialisation failure. + expect( + fakePlayer.disposeCallCount, + greaterThanOrEqualTo(1), + reason: 'VideoPlayerController must be disposed when initialize() throws', + ); + }); + + // F2r(b): close-during-error-body must cancel the stream and show the + // error UI while the body is still open. + // + // Red-with-old-code: before the fix a 403 response body was consumed via + // `response.stream.drain()`, which suspends until the upstream closes + // the stream. With the fix, `_cancelVideoResponse(response)` subscribes and + // immediately cancels. + // + // Discriminating assertion: (1) the body stream's onCancel fires while the + // body is still open (never would with drain()), and (2) the error UI is + // visible while the body remains open. Restoring drain() breaks both. + testWidgets('F2r(b): non-2xx error body is cancelled, not drained', ( + tester, + ) async { + final fakePlayer = _FakeVideoPlayerPlatform(); + VideoPlayerPlatform.instance = fakePlayer; + + // Body stream that NEVER closes — simulates a slow/stalled server. + // drain() would block here indefinitely; _cancelVideoResponse completes + // immediately by subscribing and cancelling. + // + // The Completer fires as soon as the stream's onCancel callback runs, + // giving the test a bounded completion signal instead of a fixed sleep. + // It is the cancellation of the body (not settling) that proves the fix: + // the assertions below check that (a) the cancel fires while the body is + // still open, and (b) the error UI is visible at that moment. + final cancelledCompleter = Completer(); + var bodyStreamCancelled = false; + final stalledBody = StreamController>( + onCancel: () { + bodyStreamCancelled = true; + if (!cancelledCompleter.isCompleted) cancelledCompleter.complete(); + }, + ); + addTearDown(stalledBody.close); + + final fakeClient = _FinalizingFakeClient( + responseBuilder: () => _streamedResponse(403, stalledBody), + ); + addTearDown(fakeClient.close); + + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(fakeClient), + ], + child: const MediaVideoViewerPage( + videoUrl: 'https://relay.test/media/abc.mp4', + ), + ), + ); + + // Wait for the body-cancellation signal rather than a fixed sleep. + // disableAnimations: true stops BuzzLoadingIndicator from repeating so + // pumpAndSettle converges once the error state is set. + // With drain(), cancelledCompleter never completes and this times out. + await tester.runAsync( + () => cancelledCompleter.future.timeout( + const Duration(seconds: 5), + onTimeout: () => throw TimeoutException( + 'body-cancellation signal not received within 5 s', + ), + ), + ); + await tester.pumpAndSettle(); + + // (1) The body stream's onCancel must have fired — confirming listen+cancel + // was used, not drain(). With drain(), onCancel fires only when the + // whole drain completes (which never happens here). + expect( + bodyStreamCancelled, + isTrue, + reason: + 'error-body stream must be cancelled (listen+cancel), not drained', + ); + + // (2) The error UI must be visible while the body stream is still open + // (stalledBody was never closed). _MediaLoadFailure shows this text + // when error.value is set — which only happens after _cancelVideoResponse + // completes and the HttpException propagates to the outer catch. + // With drain(), error.value is never set (drain hangs), so this + // assertion fails. + expect( + find.text('Failed to load video'), + findsOneWidget, + reason: 'error UI must be visible while the stalled body is still open', + ); + + // No controller is created before a 403 response. + expect( + fakePlayer.disposeCallCount, + 0, + reason: 'No controller is created before a 403 response', + ); + + // (3) Explicitly unmount the viewer and verify no disposal fires from the + // teardown either — no controller was ever created. + // Pass the same overrides so Riverpod's debug assertion + // (_debugOverridesLength == overrides.length) does not fire. + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(fakeClient), + ], + child: const SizedBox.shrink(), + ), + ); + await tester.pumpAndSettle(); + expect( + fakePlayer.disposeCallCount, + 0, + reason: 'Unmounting after 403 must not dispose a non-existent controller', + ); + }); + + // F2r(c): VideoPlayerController created but never initialized must be + // disposed when the widget is unmounted. + // + // Scenario: download succeeds, VideoPlayerController.file() is constructed + // and registered in pendingController, but initialize() hangs forever (the + // fake never emits an initialized event). Unmounting fires the effect + // cleanup which must dispose the pending controller via pendingController. + // + // Red-with-old-code: before the pendingController ref, localController lived + // only in the async function's stack frame; the effect cleanup read only + // controller.value (null until init completes) and disposed nothing — the + // native player was leaked. + testWidgets( + 'F2r(c): controller created but never initialized is disposed on unmount', + (tester) async { + final fakePlayer = _FakeVideoPlayerPlatform(neverInitialize: true); + VideoPlayerPlatform.instance = fakePlayer; + + final fakeClient = _FinalizingFakeClient( + responseBuilder: () => + http.StreamedResponse(Stream.value([0, 1, 2, 3]), 200), + ); + addTearDown(fakeClient.close); + + await tester.runAsync(() async { + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(fakeClient), + ], + child: const MediaVideoViewerPage( + videoUrl: 'https://relay.test/media/abc.mp4', + ), + ), + ); + // Allow enough time for the download to complete and for + // VideoPlayerController.file() to be constructed, but NOT long + // enough for initialize() to complete (it never will). + await Future.delayed(const Duration(milliseconds: 300)); + + // Now unmount — this triggers the effect cleanup with the pending + // controller still in pendingController.value (never reached play()). + // Pass the same overrides so Riverpod's debug assertion + // (_debugOverridesLength == overrides.length) does not fire. + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(fakeClient), + ], + child: const SizedBox.shrink(), + ), + ); + await Future.delayed(const Duration(milliseconds: 100)); + }); + await tester.pump(); + + // The pending controller must have been disposed by the effect cleanup, + // even though initialize() never returned. + expect( + fakePlayer.disposeCallCount, + greaterThanOrEqualTo(1), + reason: + 'pendingController must be disposed on unmount even if init never completes', + ); + }, + ); + + // F2r(d): createWithOptions() failure must show the error UI, not leave + // the viewer in an infinite loading state. + // + // Scenario: the platform plugin's createWithOptions() itself throws a + // PlatformException before returning a player ID. video_player 2.11.1 + // creates _creatingCompleter at initialize():546 and completes it only on + // the line AFTER await createWithOptions() (:587-590). If creation throws, + // _creatingCompleter is never completed, and dispose() awaits it at :682-683. + // The old code `await localController.dispose()` in the catch block therefore + // deadlocks: the outer catch never runs, error.value is never set, and the + // viewer remains on the loading screen. + // + // Fix: `unawaited(localController.dispose().catchError(...))` in the catch + // rethrows immediately so the outer catch sets error.value and shows the UI. + // + // Red-with-old-code: the old `await localController.dispose()` never returns + // (dispose() waits on the uncompleted _creatingCompleter); the 300 ms window + // ends with error.value still unset and the error UI absent — the assertion + // fails. With `unawaited(dispose())` the outer catch runs immediately and + // sets error.value; the 300 ms window is enough for the fix to take effect. + // + // Note: create itself fails here, so _creatingCompleter is never completed + // and the unawaited dispose() stalls at the same wait — this fix bypasses + // the deadlock for the outer catch, but does not release the native player. + // No dispose count is claimed for this case. + testWidgets( + 'F2r(d): createWithOptions() failure shows error UI (not infinite spinner)', + (tester) async { + final fakePlayer = _FakeVideoPlayerPlatform(forceCreateError: true); + VideoPlayerPlatform.instance = fakePlayer; + + final fakeClient = _FinalizingFakeClient( + responseBuilder: () => + http.StreamedResponse(Stream.value([0, 1, 2, 3]), 200), + ); + addTearDown(fakeClient.close); + + await tester.runAsync(() async { + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(fakeClient), + ], + child: const MediaVideoViewerPage( + videoUrl: 'https://relay.test/media/abc.mp4', + ), + ), + ); + // Allow the download to complete, createWithOptions() to throw, and + // the outer catch to set error.value. With the old `await dispose()` + // the outer catch is blocked — `error.value` is never set and + // `find.text('Failed to load video')` fails. + await Future.delayed(const Duration(milliseconds: 300)); + }); + await tester.pumpAndSettle(); + + // The error UI must be visible: unawaited dispose + rethrow lets the + // outer catch set error.value and show _MediaLoadFailure. + // With the old `await dispose()` the viewer stalls and this fails. + expect( + find.text('Failed to load video'), + findsOneWidget, + reason: + 'createWithOptions() failure must show error UI, not infinite spinner', + ); + }, + ); + + // Detached disposal error handling: a successfully-created controller whose + // init fails and whose dispose() ALSO throws must not surface an uncaught + // async error. The unawaited(dispose().catchError(...)) path in the inner + // catch must absorb the disposal exception with logging. + // + // Red-with-old-code (before the .catchError addition): the detached future + // throws PlatformException unhandled; the Flutter test binding's own + // FlutterError.onError captures it and fails the test. With .catchError + // the error is absorbed before reaching the binding's handler. + // + // Note: no FlutterError.onError override is needed here. testWidgets + // automatically fails the test if any uncaught Flutter error reaches the + // binding's handler — the test passing IS the assertion that no uncaught + // error occurred. + testWidgets( + 'F2r(d)+: post-create disposal failure shows error UI and no uncaught error', + (tester) async { + final fakeClient = _FinalizingFakeClient( + responseBuilder: () => + http.StreamedResponse(Stream.value([0, 1, 2, 3]), 200), + ); + addTearDown(fakeClient.close); + + // Construct the fake and its completer inside runAsync so that the + // bounded await below is registered in the real scheduler zone, not + // FakeAsync. A Duration-based timeout or future created outside runAsync + // becomes a FakeAsync timer; the binding never auto-advances fake time in + // testWidgets, so it hangs to the 30 s outer runner timeout instead of + // failing promptly. Inside runAsync, timers are dispatched to the real + // event loop and fire normally. + // + // Zone ownership of the completion: the viewer mounts here, so + // initializeVideo() starts in the real zone. The inner catch calls + // unawaited(localController.dispose().catchError(...)) also in the real + // zone. _FailingDisposeVideoPlayerPlatform.dispose() runs synchronously + // inside that detached future, completing disposedCompleter before any + // suspension. The await below (also in the real zone) therefore resolves + // as soon as the microtask queue drains the detached disposal future. + // + // Removal check: removing only the unawaited disposal call leaves + // disposedCompleter never completed; the 5 s timeout fires, failing the + // test immediately and deterministically. + await tester.runAsync(() async { + final fakePlayer = _FailingDisposeVideoPlayerPlatform(); + VideoPlayerPlatform.instance = fakePlayer; + + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(fakeClient), + ], + child: const MediaVideoViewerPage( + videoUrl: 'https://relay.test/media/abc.mp4', + ), + ), + ); + + // Wait for the disposal path to be entered, bounded by a real-zone + // timeout. The viewer downloads the body, creates the player, emits a + // PlatformException from the event stream, enters the inner catch, and + // calls unawaited(dispose().catchError(...)). dispose() completes + // disposedCompleter synchronously at its entry point before throwing. + await fakePlayer.disposedCompleter.future.timeout( + const Duration(seconds: 5), + onTimeout: () => throw TimeoutException( + 'dispose() was not entered within 5 s ' + '— the unawaited disposal path may have been removed', + ), + ); + + // Pump to flush the error state set in the outer catch after the + // detached disposal starts. + await tester.pumpAndSettle(); + + // Error UI must appear — disposal failure must not block the outer catch. + expect( + find.text('Failed to load video'), + findsOneWidget, + reason: 'post-create disposal failure must still show error UI', + ); + // dispose() must have been called — confirms the unawaited disposal path ran. + expect( + fakePlayer.disposeCallCount, + greaterThanOrEqualTo(1), + reason: 'dispose() must have been called on the failing player', + ); + // The test passing without a framework error IS the assertion that + // the disposal PlatformException was absorbed by .catchError and did + // not reach the binding's uncaught-error handler. + }); + }, + ); + + // Viewer-path abort: unmounting the widget while the download is in-flight + // must abort the HTTP request through the viewer's own wiring. + // + // The viewer creates an AbortableStreamedRequest with abortTrigger wired to + // downloadRequestAbort.value (a Completer). The effect cleanup calls + // activeRequestAbort.complete() on unmount, which fires abortTrigger. + // + // Red-with-reverted-wiring: removing only `activeRequestAbort.complete()` + // from the cleanup leaves the trigger pending and abortObserved stays false + // at the deadline — the abortObservedCompleter times out. + // Deleting the whole AbortableStreamedRequest / abortTrigger wiring causes + // the fake to throw StateError from send(); the viewer catches it as a load + // error, so abortObserved is never set and the test fails at the deadline. + testWidgets( + 'Viewer abort-path: unmount fires abortTrigger and cancels in-flight download', + (tester) async { + final fakePlayer = _FakeVideoPlayerPlatform(); + VideoPlayerPlatform.instance = fakePlayer; + + final stallingClient = _StallingAbortableClient(); + addTearDown(stallingClient.close); + + await tester.runAsync(() async { + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(stallingClient), + ], + child: const MediaVideoViewerPage( + videoUrl: 'https://relay.test/media/abort-test.mp4', + ), + ), + ); + + // Wait for the viewer to start the download and reach the stall point. + await stallingClient.requestArrivedCompleter.future.timeout( + const Duration(seconds: 5), + onTimeout: () => throw TimeoutException( + 'Viewer did not start download within 5 s', + ), + ); + + // Verify abort has NOT fired yet (before unmount). + expect( + stallingClient.abortObserved, + isFalse, + reason: 'abort must not fire before unmount', + ); + + // Now unmount — this fires the effect cleanup which calls + // activeRequestAbort.complete(), completing abortTrigger. + // Pass the same overrides so Riverpod's debug assertion does not fire. + await tester.pumpWidget( + WidgetHelpers.testable( + disableAnimations: true, + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue(_noopAuth()), + mediaHttpClientProvider.overrideWithValue(stallingClient), + ], + child: const SizedBox.shrink(), + ), + ); + // Pump once to flush the effect cleanup microtasks that fire during + // the widget-tree disposal (useEffect cleanup in Flutter hooks runs + // synchronously in the pumpWidget call above, but the Completer + // completion and the abortTrigger await chain resolve on subsequent + // microtask turns — yield here so they settle before the deadline). + await tester.pump(); + + // Wait for the abort to be observed with a deadline — no sleep. + // Deleting activeRequestAbort.complete() in cleanup → times out here. + await stallingClient.abortObservedCompleter.future.timeout( + const Duration(seconds: 5), + onTimeout: () => throw TimeoutException( + 'Abort was not observed within 5 s after unmount', + ), + ); + }); + await tester.pump(); + + // The fake must have received the abort — proving that the viewer's + // effect cleanup correctly wired the Completer to the AbortableStreamedRequest. + expect( + stallingClient.abortObserved, + isTrue, + reason: + 'viewer unmount must complete abortTrigger and cancel the in-flight download', + ); + }, + ); +} diff --git a/mobile/test/features/channels/media_viewer_page/video_viewer_transport_test.dart b/mobile/test/features/channels/media_viewer_page/video_viewer_transport_test.dart new file mode 100644 index 00000000000..d82f3dce8fc --- /dev/null +++ b/mobile/test/features/channels/media_viewer_page/video_viewer_transport_test.dart @@ -0,0 +1,169 @@ +// Real-IO transport probes for MediaVideoViewerPage. +// +// These tests exercise the actual IOClient (dart:io) transport path for +// AbortableStreamedRequest. They are intentionally in a SEPARATE file +// from video_viewer_test.dart because TestWidgetsFlutterBinding.ensureInitialized() +// in the main test file installs a suite-wide HttpClient override that returns +// status 400 for ALL requests — including plain test() calls in the same suite. +// By isolating these here, the real loopback HttpServer can be reached. +// +// Transport probe #1: request sink must be closed before send(). +// A local loopback server reads the full request body before replying. +// Without the `unawaited(request.sink.close())` fix, IOClient.send() blocks +// at `stream.pipe(ioRequest)` forever — test times out. +// +// Transport probe #2: abort trigger cancels an in-flight download. +// The server holds the response open via a teardown-controlled release gate +// so the abort fires against a genuinely in-flight transfer rather than +// racing a completed response. After server confirms request arrival, +// the abort is triggered; send() must throw the typed +// RequestAbortedException within a bounded deadline. + +import 'dart:async'; +import 'dart:io'; + +import 'package:http/http.dart' as http; +import 'package:http/io_client.dart' show IOClient; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test( + 'Transport: request sink is closed — real IO loopback probe (success)', + () async { + final videoBytes = [0, 1, 2, 3]; + + // Start a local HTTP server that reads the full request body BEFORE + // sending the response. If the sink is not closed, the drain hangs. + final server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0); + addTearDown(() => server.close(force: true)); + + server.listen((req) async { + await req.drain(); // hangs if sink was not closed + req.response + ..statusCode = 200 + ..headers.contentType = ContentType('video', 'mp4') + ..contentLength = videoBytes.length + ..add(videoBytes); + await req.response.close(); + }); + + final serverUrl = + 'http://${server.address.host}:${server.port}/video.mp4'; + final client = IOClient( + HttpClient()..idleTimeout = const Duration(milliseconds: 1), + ); + addTearDown(client.close); + + final requestAbort = Completer(); + final request = http.AbortableStreamedRequest( + 'GET', + Uri.parse(serverUrl), + abortTrigger: requestAbort.future, + ); + // THE FIX: close the sink before send so the pipe completes. + unawaited(request.sink.close()); + + final response = await client + .send(request) + .timeout( + const Duration(seconds: 5), + onTimeout: () => + throw TimeoutException('send() did not complete within 5 s'), + ); + + expect( + response.statusCode, + 200, + reason: 'sink closed → server receives full request → replies 200', + ); + await response.stream.drain().timeout( + const Duration(seconds: 5), + onTimeout: () => throw TimeoutException( + 'response drain did not complete within 5 s', + ), + ); + }, + ); + + test( + 'Transport: abort trigger cancels an in-flight download — real IO loopback', + () async { + // The server signals that the request has arrived (so the abort fires + // AFTER the connection is established, not before). + final requestArrivedCompleter = Completer(); + + // Teardown-controlled gate: the server handler awaits this before + // calling response.close(), so the response body stays open until the + // test explicitly releases it (on success) or teardown releases it (on + // failure). Without this gate, response.close() would send an empty 200 + // immediately and the abort would race against a completed response — + // making the probe scheduling-sensitive rather than a controlled + // in-flight cancellation. + final releaseResponse = Completer(); + + final server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0); + addTearDown(() => server.close(force: true)); + addTearDown(() { + if (!releaseResponse.isCompleted) releaseResponse.complete(); + }); + + server.listen((req) async { + await req.drain(); + // Signal that the server has received the request, then hold the + // response open until the test releases it or teardown fires. + if (!requestArrivedCompleter.isCompleted) { + requestArrivedCompleter.complete(); + } + // Await the teardown-controlled gate before attempting to close. + // After an abort the client may have already torn down the connection, + // so close() may throw — caught and discarded here. + await releaseResponse.future; + try { + await req.response.close(); + } catch (_) { + // Connection may already be torn down by the client abort — expected. + } + }); + + final serverUrl = + 'http://${server.address.host}:${server.port}/video.mp4'; + final client = IOClient( + HttpClient()..idleTimeout = const Duration(milliseconds: 1), + ); + addTearDown(client.close); + + final requestAbort = Completer(); + final request = http.AbortableStreamedRequest( + 'GET', + Uri.parse(serverUrl), + abortTrigger: requestAbort.future, + ); + unawaited(request.sink.close()); + + // Start the request, wait for server-arrival confirmation, then abort. + final sendFuture = client.send(request); + // Bounded wait: if the server doesn't see the request within 5s, fail. + await requestArrivedCompleter.future.timeout( + const Duration(seconds: 5), + onTimeout: () => throw TimeoutException( + 'Loopback server did not receive request within 5 s', + ), + ); + requestAbort.complete(); + + // send() must throw RequestAbortedException within a bounded deadline. + // The typed assertion distinguishes an abort from any other exception. + await expectLater( + sendFuture.timeout(const Duration(seconds: 5)), + throwsA(isA()), + reason: 'abort must cause send() to throw RequestAbortedException', + ); + + // Release the server handler so it can exit cleanly. The abort may + // have already torn down the connection, so close() in the handler may + // throw and be discarded. Teardown also releases this gate; this is + // belt-and-suspenders cleanup for the success path. + if (!releaseResponse.isCompleted) releaseResponse.complete(); + }, + ); +} diff --git a/mobile/test/features/channels/message_content_test.dart b/mobile/test/features/channels/message_content_test.dart index 8fe47e88030..150ccd4290f 100644 --- a/mobile/test/features/channels/message_content_test.dart +++ b/mobile/test/features/channels/message_content_test.dart @@ -40,15 +40,18 @@ Widget _testable( ], child: MaterialApp( theme: AppTheme.light(), - home: Builder( - builder: (context) => MediaQuery( - data: MediaQuery.of( - context, - ).copyWith(disableAnimations: disableAnimations), - // The app states its code style here, above the navigator. - child: AppMarkdownTheme(child: Scaffold(body: child)), - ), + // Use MaterialApp.builder so the MediaQuery override (including + // disableAnimations) applies to every pushed route, not just the + // home scaffold. Navigator-pushed routes (e.g. MediaVideoViewerPage) + // skip a home-level Builder wrapper entirely. + builder: (context, child) => MediaQuery( + data: MediaQuery.of( + context, + ).copyWith(disableAnimations: disableAnimations), + // AppMarkdownTheme must wrap all routes that render message content. + child: AppMarkdownTheme(child: child!), ), + home: Scaffold(body: child), ), ); } @@ -2216,6 +2219,7 @@ Photos ], ], ), + disableAnimations: true, ), ); await tester.pumpAndSettle(); @@ -2284,6 +2288,7 @@ Photos ], ], ), + disableAnimations: true, ), ); await tester.pumpAndSettle(); diff --git a/mobile/test/helpers/widget_helpers.dart b/mobile/test/helpers/widget_helpers.dart index b7f119db87d..11b4b421410 100644 --- a/mobile/test/helpers/widget_helpers.dart +++ b/mobile/test/helpers/widget_helpers.dart @@ -7,12 +7,20 @@ class WidgetHelpers { static Widget testable({ required Widget child, List overrides = const [], + bool disableAnimations = false, }) { return ProviderScope( overrides: overrides, child: MaterialApp( theme: AppTheme.light(), - home: Scaffold(body: child), + home: Builder( + builder: (context) => MediaQuery( + data: MediaQuery.of( + context, + ).copyWith(disableAnimations: disableAnimations), + child: Scaffold(body: child), + ), + ), ), ); } diff --git a/mobile/test/shared/relay/media_image_test.dart b/mobile/test/shared/relay/media_image_test.dart index 4f43ec47701..e3626cf8923 100644 --- a/mobile/test/shared/relay/media_image_test.dart +++ b/mobile/test/shared/relay/media_image_test.dart @@ -31,30 +31,35 @@ void main() { PaintingBinding.instance.imageCache.clearLiveImages(); }); - group('MediaGetAuthService memoization', () { - test('repeated calls return byte-identical headers', () { + group('MediaGetAuthService mint-per-request', () { + test('consecutive calls return distinct headers (mint-per-request)', () { final nsec = nostr.Keys.generate().nsec; final auth = _auth(nsec: nsec); final first = auth.headersFor(_mediaUrl); final second = auth.headersFor(_mediaUrl); expect(first, isNotEmpty); - expect(identical(first, second), isTrue); + // With lifetime == margin == 60s, refreshAt == signedAt, so the cache + // never hits: each call produces a fresh proof. + expect(identical(first, second), isFalse); + expect(second['Authorization'], isNot(first['Authorization'])); }); - test('re-signs only at the refresh margin before expiry', () { + test('each call produces a fresh token regardless of elapsed time', () { final nsec = nostr.Keys.generate().nsec; var current = DateTime.utc(2026, 7, 21, 12); final auth = _auth(nsec: nsec, now: () => current); final first = auth.headersFor(_mediaUrl); - // 600s lifetime - 60s margin = re-sign boundary at +540s. - current = current.add(const Duration(seconds: 539)); - expect(identical(auth.headersFor(_mediaUrl), first), isTrue); - - current = current.add(const Duration(seconds: 2)); - final refreshed = auth.headersFor(_mediaUrl); - expect(identical(refreshed, first), isFalse); - expect(refreshed['Authorization'], isNot(first['Authorization'])); + // No elapsed time: still mints fresh. + final atZero = auth.headersFor(_mediaUrl); + expect(identical(atZero, first), isFalse); + expect(atZero['Authorization'], isNot(first['Authorization'])); + + // Advance clock: still mints fresh. + current = current.add(const Duration(seconds: 30)); + final at30 = auth.headersFor(_mediaUrl); + expect(identical(at30, first), isFalse); + expect(at30['Authorization'], isNot(first['Authorization'])); }); test('non-relay URLs get no headers even with a key', () { diff --git a/mobile/test/shared/relay/media_upload_test.dart b/mobile/test/shared/relay/media_upload_test.dart index e9dbed302ef..dd72efe73ee 100644 --- a/mobile/test/shared/relay/media_upload_test.dart +++ b/mobile/test/shared/relay/media_upload_test.dart @@ -292,7 +292,7 @@ void main() { expect(authEvent['content'], 'Get buzz-media'); expect(authEvent['tags'], contains(equals(['t', 'get']))); expect(authEvent['tags'], contains(equals(['server', 'relay.example']))); - expect(authEvent['tags'], contains(equals(['expiration', '1700000600']))); + expect(authEvent['tags'], contains(equals(['expiration', '1700000060']))); }); test('does not sign non-relay or non-media URLs', () { @@ -419,7 +419,7 @@ void main() { equals(['x', capturedRequest!.headers['X-SHA-256']!]), ), ); - expect(tags, anyElement(equals(['expiration', '1700000300']))); + expect(tags, anyElement(equals(['expiration', '1700000060']))); expect( tags, anyElement(equals(['server', 'relay.example:8443'])), diff --git a/scripts/test-video-upload.sh b/scripts/test-video-upload.sh index 09c4ffed9fc..90e632428ce 100755 --- a/scripts/test-video-upload.sh +++ b/scripts/test-video-upload.sh @@ -55,14 +55,15 @@ echo "Test MP4: ${FILE_SIZE} bytes, sha256=${SHA256:0:16}..." echo "" # ── Helper: build Blossom auth header ────────────────────────────────────────── -# Creates a kind:24242 event with t=upload, x=, expiration=+5min. +# Creates a kind:24242 event with t=upload, x=, expiration=+60s, server=. blossom_auth() { local sha256="$1" - local now exp auth_event auth_b64 + local now exp server auth_event auth_b64 now=$(date +%s) - exp=$((now + 300)) + exp=$((now + 60)) + server=$(echo "$RELAY_URL" | sed -E 's|^https?://||' | cut -d'/' -f1) auth_event=$(nak event \ --sec "$NSEC" \ @@ -71,6 +72,7 @@ blossom_auth() { -t t=upload \ -t "x=$sha256" \ -t "expiration=$exp" \ + -t "server=$server" \ 2>/dev/null) auth_b64=$(echo -n "$auth_event" | base64 | tr -d '\n')