fix(media): enforce channel ACL on bound blobs (follow-up to 769ac70)

crates/buzz-media/src/error.rs

@@ -18,6 +18,8 @@ pub enum MediaError {
     InvalidImage,
     #[error("media contains metadata or a non-canonical metadata channel")]
     MetadataForbidden,
+    #[error("invalid tag value for {0}")]
+    InvalidTag(&'static str),
     #[error("invalid signature")]
     InvalidSignature,
     #[error("invalid auth event kind")]
@@ -151,6 +153,7 @@ impl IntoResponse for MediaError {
             | Self::InvalidVideo
             | Self::InvalidImage
             | Self::MetadataForbidden => (StatusCode::UNPROCESSABLE_ENTITY, self.to_string()),
+            Self::InvalidTag(_) => (StatusCode::BAD_REQUEST, self.to_string()),
             Self::Io(_) | Self::StorageError(_) | Self::Internal => {
                 tracing::error!(error = %self, "media storage error");
                 (StatusCode::INTERNAL_SERVER_ERROR, "internal error".into())
@@ -179,6 +182,16 @@ mod tests {
         }
     }
 
+    #[test]
+    fn invalid_tag_maps_to_400() {
+        // A malformed `h` (channel) tag on the Blossom auth event is a caller
+        // mistake, not an auth failure — it must surface as 400, not 401/500.
+        assert_eq!(
+            MediaError::InvalidTag("h").into_response().status(),
+            StatusCode::BAD_REQUEST
+        );
+    }
+
     #[test]
     fn invalid_or_noncanonical_media_maps_to_422() {
         for error in [

crates/buzz-media/src/storage.rs

@@ -330,6 +330,67 @@ mod tests {
         );
     }
 
+    /// Sidecars written before channel binding existed have no `channel_id`
+    /// key. They must still parse, as unbound. This is what makes rolling the
+    /// channel ACL out migration-free — every already-stored sidecar in S3
+    /// predates the field.
+    #[test]
+    fn blob_meta_legacy_sidecar_without_channel_id_parses_as_unbound() {
+        let legacy = r#"{
+            "dim": "800x600",
+            "blurhash": "LEHV6nWB2yk8pyo0adR*",
+            "thumb_url": "https://media.example.com/abc.thumb.jpg",
+            "ext": "jpg",
+            "mime_type": "image/jpeg",
+            "size": 1234,
+            "uploaded_at": 1700000000
+        }"#;
+
+        let meta: BlobMeta = serde_json::from_str(legacy).expect("legacy sidecar must still parse");
+        assert_eq!(meta.channel_id, None, "legacy sidecars are unbound");
+        // The rest of the sidecar must survive untouched.
+        assert_eq!(meta.ext, "jpg");
+        assert_eq!(meta.mime_type, "image/jpeg");
+        assert_eq!(meta.size, 1234);
+        assert_eq!(meta.uploaded_at, 1700000000);
+    }
+
+    /// The other half of the back-compat contract: an unbound sidecar must
+    /// serialize *without* the key, so newly written sidecars stay readable by
+    /// any older relay still running the previous binary.
+    #[test]
+    fn blob_meta_unbound_omits_channel_id_key() {
+        let meta = BlobMeta {
+            ext: "jpg".to_string(),
+            mime_type: "image/jpeg".to_string(),
+            size: 1234,
+            channel_id: None,
+            ..Default::default()
+        };
+
+        let json = serde_json::to_value(&meta).expect("serialize unbound sidecar");
+        assert!(
+            json.get("channel_id").is_none(),
+            "unbound sidecars must omit channel_id entirely, got {json}"
+        );
+    }
+
+    #[test]
+    fn blob_meta_bound_round_trips_channel_id() {
+        let channel_id = uuid::Uuid::from_u128(0x0bad_c0de);
+        let meta = BlobMeta {
+            ext: "jpg".to_string(),
+            mime_type: "image/jpeg".to_string(),
+            size: 1234,
+            channel_id: Some(channel_id),
+            ..Default::default()
+        };
+
+        let json = serde_json::to_string(&meta).expect("serialize bound sidecar");
+        let parsed: BlobMeta = serde_json::from_str(&json).expect("round-trip bound sidecar");
+        assert_eq!(parsed.channel_id, Some(channel_id));
+    }
+
     #[test]
     fn partial_static_keys_are_rejected() {
         let err = match MediaStorage::new(&storage_config("buzz_dev", "")) {
@@ -422,4 +483,10 @@ pub struct BlobMeta {
     /// Video duration in seconds. `None` for non-video blobs.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub duration_secs: Option<f64>,
+    /// Optional originating channel — when `Some`, reads require channel membership
+    /// for private channels (relay membership alone is insufficient). `None` means
+    /// the blob is not bound to a channel (e.g. avatar, open-channel, legacy).
+    /// Added as follow-up to `769ac70` which noted channel binding was missing.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub channel_id: Option<uuid::Uuid>,
 }

crates/buzz-media/src/upload.rs

@@ -17,6 +17,44 @@ use crate::validation::{
     validate_video_file,
 };
 
+/// Extract an optional `h` (channel) tag from a Blossom auth event.
+///
+/// When the uploader includes `["h", "<uuid>"]` in the kind:24242 auth
+/// event, the sidecar is bound to that channel and subsequent `GET`/`HEAD`
+/// requests require channel membership for private channels. `None` means
+/// unbound (legacy / avatar / open-channel) — relay membership alone suffices.
+/// If an `h` tag is present but malformed (missing value, not a UUID, nil
+/// UUID, or multiple conflicting values), the upload fails with
+/// `InvalidTag("h")` (400) rather than silently downgrading to unbound.
+fn extract_channel_id(
+    auth_event: &nostr::Event,
+) -> Result<Option<uuid::Uuid>, crate::error::MediaError> {
+    let h_tag = nostr::SingleLetterTag::lowercase(nostr::Alphabet::H);
+    let mut found: Option<uuid::Uuid> = None;
+    for tag in auth_event.tags.iter() {
+        if tag.kind() == nostr::TagKind::SingleLetter(h_tag) {
+            let val = tag
+                .content()
+                .ok_or(crate::error::MediaError::InvalidTag("h"))?;
+            let id = uuid::Uuid::parse_str(val)
+                .map_err(|_| crate::error::MediaError::InvalidTag("h"))?;
+            if id.is_nil() {
+                return Err(crate::error::MediaError::InvalidTag("h"));
+            }
+            if let Some(existing) = found {
+                if existing != id {
+                    // Multiple h tags with different UUIDs — ambiguous binding.
+                    return Err(crate::error::MediaError::InvalidTag("h"));
+                }
+                // Duplicate same UUID — idempotent, keep first.
+            } else {
+                found = Some(id);
+            }
+        }
+    }
+    Ok(found)
+}
+
 /// Shared buffered-upload pipeline for the image and generic-file paths.
 ///
 /// Both paths are identical except for two steps, which are injected:
@@ -212,6 +250,7 @@ pub async fn process_upload(
     body: Bytes,
     attribution: Option<UploadAttribution>,
 ) -> Result<BlobDescriptor, MediaError> {
+    let channel_id = extract_channel_id(auth_event)?;
     process_buffered_upload(
         BufferedUploadInput {
             storage,
@@ -226,7 +265,11 @@ pub async fn process_upload(
             let ext = mime_to_ext(&mime).to_string();
             Ok((mime, ext))
         },
-        |input| async move { prepare_image_metadata(storage, config, input).await },
+        |input| async move {
+            let mut meta = prepare_image_metadata(storage, config, input).await?;
+            meta.channel_id = channel_id;
+            Ok(meta)
+        },
     )
     .await
 }
@@ -250,6 +293,7 @@ pub async fn process_file_upload(
     body: Bytes,
     attribution: Option<UploadAttribution>,
 ) -> Result<BlobDescriptor, MediaError> {
+    let channel_id = extract_channel_id(auth_event)?;
     process_buffered_upload(
         BufferedUploadInput {
             storage,
@@ -271,6 +315,7 @@ pub async fn process_file_upload(
                 mime_type: input.mime,
                 uploaded_at: input.uploaded_at,
                 duration_secs: None,
+                channel_id,
             };
             Ok(meta)
         },
@@ -298,6 +343,7 @@ pub async fn process_video_upload(
     content_length: Option<u64>,
     attribution: Option<UploadAttribution>,
 ) -> Result<BlobDescriptor, MediaError> {
+    let channel_id = extract_channel_id(auth_event)?;
     // --- 1. Stream body to temp file, compute SHA-256 incrementally ---
     let tmp = tempfile::NamedTempFile::new().map_err(|e| MediaError::Io(e.to_string()))?;
     let tmp_path = tmp.path().to_path_buf();
@@ -476,6 +522,7 @@ pub async fn process_video_upload(
         size: file_size,
         uploaded_at,
         duration_secs: Some(video_meta.duration_secs),
+        channel_id,
     };
 
     // Record before publishing the sidecar serve gate. See the buffered path.
@@ -596,6 +643,7 @@ mod tests {
             size: 5_000_000,
             uploaded_at: 1700000000,
             duration_secs: Some(29.5),
+            channel_id: None,
         };
 
         let desc = build_descriptor(
@@ -654,6 +702,7 @@ mod tests {
             size: 100_000,
             uploaded_at: 1700000000,
             duration_secs: None,
+            channel_id: None,
         };
 
         let desc = build_descriptor(
@@ -711,6 +760,101 @@ mod tests {
         assert_eq!(detect("connection reset"), std::io::ErrorKind::Other);
     }
 
+    /// Sign a kind:24242 Blossom auth event carrying `tags`.
+    ///
+    /// `extract_channel_id` only reads tags, but it takes a real `nostr::Event`,
+    /// so the tests build and sign real events rather than a stub.
+    fn auth_event_with_tags(tags: Vec<nostr::Tag>) -> nostr::Event {
+        let keys = nostr::Keys::generate();
+        nostr::EventBuilder::new(nostr::Kind::from(24242), "Upload buzz-media")
+            .tags(tags)
+            .sign_with_keys(&keys)
+            .expect("sign auth event")
+    }
+
+    fn h_tag(value: &str) -> nostr::Tag {
+        nostr::Tag::parse(["h", value]).expect("h tag")
+    }
+
+    #[track_caller]
+    fn assert_invalid_h_tag(result: Result<Option<uuid::Uuid>, MediaError>) {
+        match result {
+            Err(MediaError::InvalidTag(tag)) => assert_eq!(tag, "h"),
+            other => panic!("expected InvalidTag(\"h\"), got {other:?}"),
+        }
+    }
+
+    #[test]
+    fn extract_channel_id_absent_h_tag_is_unbound() {
+        // No h tag at all: the blob is unbound (avatar / legacy / open channel).
+        let event = auth_event_with_tags(vec![
+            nostr::Tag::parse(["t", "upload"]).expect("t tag"),
+            nostr::Tag::parse(["x", &"a".repeat(64)]).expect("x tag"),
+        ]);
+        assert_eq!(extract_channel_id(&event).expect("no h tag is valid"), None);
+    }
+
+    #[test]
+    fn extract_channel_id_single_valid_h_tag_binds_channel() {
+        let channel_id = uuid::Uuid::from_u128(0x1234_5678_9abc_def0);
+        let event = auth_event_with_tags(vec![
+            nostr::Tag::parse(["t", "upload"]).expect("t tag"),
+            h_tag(&channel_id.to_string()),
+        ]);
+        assert_eq!(
+            extract_channel_id(&event).expect("valid h tag"),
+            Some(channel_id)
+        );
+    }
+
+    #[test]
+    fn extract_channel_id_rejects_non_uuid_value() {
+        let event = auth_event_with_tags(vec![h_tag("general")]);
+        assert_invalid_h_tag(extract_channel_id(&event));
+    }
+
+    #[test]
+    fn extract_channel_id_rejects_nil_uuid() {
+        // The nil UUID is a parseable-but-meaningless binding — fail closed
+        // rather than silently downgrading the blob to unbound.
+        let event = auth_event_with_tags(vec![h_tag("00000000-0000-0000-0000-000000000000")]);
+        assert_invalid_h_tag(extract_channel_id(&event));
+    }
+
+    #[test]
+    fn extract_channel_id_rejects_empty_value() {
+        let event = auth_event_with_tags(vec![h_tag("")]);
+        assert_invalid_h_tag(extract_channel_id(&event));
+    }
+
+    #[test]
+    fn extract_channel_id_rejects_valueless_h_tag() {
+        let event = auth_event_with_tags(vec![nostr::Tag::parse(["h"]).expect("valueless h tag")]);
+        assert_invalid_h_tag(extract_channel_id(&event));
+    }
+
+    #[test]
+    fn extract_channel_id_duplicate_same_uuid_is_idempotent() {
+        let channel_id = uuid::Uuid::from_u128(0xfeed_face);
+        let value = channel_id.to_string();
+        let event = auth_event_with_tags(vec![h_tag(&value), h_tag(&value)]);
+        assert_eq!(
+            extract_channel_id(&event).expect("duplicate identical h tags are idempotent"),
+            Some(channel_id)
+        );
+    }
+
+    #[test]
+    fn extract_channel_id_rejects_conflicting_h_tags() {
+        // Two different channels is an ambiguous binding — reject rather than
+        // letting tag order decide which channel's ACL guards the blob.
+        let event = auth_event_with_tags(vec![
+            h_tag(&uuid::Uuid::from_u128(1).to_string()),
+            h_tag(&uuid::Uuid::from_u128(2).to_string()),
+        ]);
+        assert_invalid_h_tag(extract_channel_id(&event));
+    }
+
     #[test]
     fn test_build_descriptor_no_meta() {
         // When meta is None, all optional fields should be None.

crates/buzz-relay/src/api/admin/mod.rs

@@ -223,12 +223,22 @@ async fn feedback_attachment(
         return Err(ApiError::not_found());
     }
 
-    let response = crate::api::media::serve_blob_for_tenant(&state, &tenant, &sha256, &headers)
-        .await
-        .map_err(|error| match error {
-            buzz_media::MediaError::NotFound => ApiError::not_found(),
-            _ => ApiError::internal(),
-        })?;
+    // `Operator`: this read is already admin-authorized above and pinned to the
+    // feedback row's own provenance, so the sidecar channel ACL is deliberately
+    // not applied — an admin must be able to read an attachment from a private
+    // channel they are not a member of.
+    let response = crate::api::media::serve_blob_for_tenant_with_authority(
+        &state,
+        &tenant,
+        &sha256,
+        &headers,
+        crate::api::media::BlobReadAuthority::Operator,
+    )
+    .await
+    .map_err(|error| match error {
+        buzz_media::MediaError::NotFound => ApiError::not_found(),
+        _ => ApiError::internal(),
+    })?;
     tracing::info!(
         feedback_id = %feedback.id,
         community_id = %feedback.community_id,