feat: proxy feedback-scoped admin attachments (#2059)

Signed-off-by: npub1dccv64krpcpse5cmkzfeh998cftungyatw3djt8jwdw6g43f7fyqzzmrf7 <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: npub1dccv64krpcpse5cmkzfeh998cftungyatw3djt8jwdw6g43f7fyqzzmrf7 <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: Wes <wesbillman@users.noreply.github.com>
This commit is contained in:
Kalvin C
2026-07-18 08:15:57 -07:00
committed by GitHub
parent 87dc4dccba
commit d7f918e3cb
5 changed files with 338 additions and 19 deletions

View File

@@ -401,6 +401,7 @@ function FeedbackDetailView({ id }: { id: string }) {
<StateView resource={resource}>
{(feedback) => {
const attachments = feedbackAttachments(
feedback.id,
feedback.tags,
feedback.communityHost,
);
@@ -457,6 +458,7 @@ type FeedbackStatuses = Record<string, boolean>;
interface FeedbackAttachment {
url: string;
sourceUrl: string;
mimeType: string;
hash: string;
size?: number;
@@ -494,6 +496,7 @@ function timeRangeStart(range: string) {
}
function feedbackAttachments(
feedbackId: string,
tags: string[][],
communityHost: string,
): FeedbackAttachment[] {
@@ -514,7 +517,8 @@ function feedbackAttachments(
const parsedSize = Number(values.get("size"));
return [
{
url: safeUrl,
url: `/api/admin/v1/feedback/${encodeURIComponent(feedbackId)}/attachments/${hash}`,
sourceUrl: safeUrl,
mimeType,
hash,
size:
@@ -546,7 +550,7 @@ function stripAttachmentMarkdown(
attachments?: FeedbackAttachment[],
) {
const knownUrls = attachments
? new Set(attachments.map((attachment) => attachment.url))
? new Set(attachments.map((attachment) => attachment.sourceUrl))
: undefined;
return body
.replace(/!?\[[^\]]*\]\(([^)\s]+)(?:\s+"[^"]*")?\)/g, (match, url) => {

View File

@@ -235,10 +235,16 @@ test("feedback attachments render from imeta without raw markdown", async ({
await expect(page.getByText("![image]", { exact: false })).toHaveCount(0);
await expect(
page.getByRole("img", { name: "screenshot.png" }),
).toHaveAttribute("src", imageUrl);
).toHaveAttribute(
"src",
`/api/admin/v1/feedback/${id}/attachments/${"a".repeat(64)}`,
);
await expect(
page.getByRole("link", { name: /diagnostics.txt/ }),
).toHaveAttribute("href", fileUrl);
).toHaveAttribute(
"href",
`/api/admin/v1/feedback/${id}/attachments/${"b".repeat(64)}`,
);
const fileHeight = await page
.locator(".file-attachment")
.evaluate((element) => element.getBoundingClientRect().height);

View File

@@ -31,6 +31,10 @@ pub fn router(state: Arc<crate::state::AppState>) -> Router {
.route("/reports/{id}", get(report_detail))
.route("/feedback", get(feedback))
.route("/feedback/{id}", get(feedback_detail))
.route(
"/feedback/{id}/attachments/{sha256}",
get(feedback_attachment),
)
.layer(middleware::from_fn(security_headers))
.layer(RequestBodyLimitLayer::new(1024))
.with_state(state)
@@ -184,6 +188,110 @@ async fn feedback_detail(
.ok_or_else(ApiError::not_found)
}
async fn feedback_attachment(
State(state): State<Arc<crate::state::AppState>>,
headers: HeaderMap,
Path((id, sha256)): Path<(Uuid, String)>,
) -> Result<Response, ApiError> {
authorize(&state, &headers)?;
if !is_sha256(&sha256) {
return Err(ApiError::not_found());
}
let feedback = state
.db
.admin_get_feedback(id)
.await?
.ok_or_else(ApiError::not_found)?;
if !feedback_references_hash(&feedback.tags, &feedback.community_host, &sha256) {
return Err(ApiError::not_found());
}
// Resolve the tenant from server-owned feedback provenance, then assert the
// resolved row still agrees with the feedback FK. Client input never names
// a community, host, object key, extension, or upstream URL.
let tenant = crate::tenant::bind_community(&state.db, &feedback.community_host)
.await
.map_err(|_| ApiError::not_found())?;
if tenant.community().as_uuid() != &feedback.community_id {
tracing::warn!(
feedback_id = %feedback.id,
feedback_community_id = %feedback.community_id,
resolved_community_id = %tenant.community(),
"admin feedback attachment tenant provenance mismatch"
);
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(),
})?;
tracing::info!(
feedback_id = %feedback.id,
community_id = %feedback.community_id,
attachment_sha256 = %sha256,
"admin feedback attachment read"
);
Ok(response)
}
fn feedback_references_hash(tags: &serde_json::Value, community_host: &str, sha256: &str) -> bool {
tags.as_array()
.into_iter()
.flatten()
.filter_map(|tag| tag.as_array())
.filter(|tag| tag.first().and_then(|value| value.as_str()) == Some("imeta"))
.any(|tag| {
let fields = tag
.iter()
.skip(1)
.filter_map(|value| value.as_str()?.split_once(' '))
.collect::<std::collections::HashMap<_, _>>();
fields.get("x") == Some(&sha256)
&& fields
.get("url")
.is_some_and(|url| attachment_url_matches(url, community_host, sha256))
})
}
fn attachment_url_matches(url: &str, community_host: &str, sha256: &str) -> bool {
let parsed = if url.starts_with('/') {
url::Url::parse(&format!("https://{community_host}{url}"))
} else {
url::Url::parse(url)
};
let Ok(url) = parsed else {
return false;
};
let authority = url.port().map_or_else(
|| url.host_str().unwrap_or_default().to_string(),
|port| format!("{}:{port}", url.host_str().unwrap_or_default()),
);
let Some(media_name) = url.path().strip_prefix("/media/") else {
return false;
};
let Some((url_hash, extension)) = media_name.split_once('.') else {
return false;
};
matches!(url.scheme(), "http" | "https")
&& buzz_core::tenant::normalize_host(&authority)
== buzz_core::tenant::normalize_host(community_host)
&& url_hash == sha256
&& crate::api::media::is_safe_ext(extension)
&& url.query().is_none()
&& url.fragment().is_none()
}
fn is_sha256(value: &str) -> bool {
value.len() == 64
&& value
.chars()
.all(|character| matches!(character, '0'..='9' | 'a'..='f'))
}
fn summarize_body(body: &str, tags: &serde_json::Value) -> String {
const MAX_CHARS: usize = 240;
let attachment_urls = tags
@@ -220,6 +328,104 @@ fn summarize_body(body: &str, tags: &serde_json::Value) -> String {
#[cfg(test)]
mod tests {
use super::*;
use axum::{body::Body, http::Request};
use tower::ServiceExt;
async fn test_state() -> Arc<crate::state::AppState> {
let mut config = crate::config::Config::from_env().expect("default config loads");
config.require_relay_membership = false;
config.redis_url = "redis://127.0.0.1:1".to_string();
config.admin = Some(crate::config::AdminConfig {
host: "admin.example".to_string(),
web_dir: None,
});
let pool = sqlx::PgPool::connect_lazy(&config.database_url).expect("lazy pg pool");
let db = buzz_db::Db::from_pool(pool.clone());
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) = crate::state::AppState::new(
config,
db,
redis_pool,
audit,
pubsub,
auth,
search,
workflow_engine,
nostr::Keys::generate(),
media_storage,
);
Arc::new(state)
}
const HASH: &str = "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789";
#[tokio::test]
async fn feedback_attachment_requires_admin_host_before_database_access() {
let response = router(test_state().await)
.oneshot(
Request::builder()
.uri(format!("/feedback/{}/attachments/{HASH}", Uuid::nil()))
.header(header::HOST, "community.example")
.body(Body::empty())
.expect("request"),
)
.await
.expect("response");
assert_eq!(response.status(), axum::http::StatusCode::FORBIDDEN);
}
#[tokio::test]
async fn feedback_attachment_rejects_unknown_feedback() {
let response = router(test_state().await)
.oneshot(
Request::builder()
.uri(format!("/feedback/{}/attachments/{HASH}", Uuid::nil()))
.header(header::HOST, "admin.example")
.body(Body::empty())
.expect("request"),
)
.await
.expect("response");
assert_eq!(response.status(), axum::http::StatusCode::NOT_FOUND);
}
#[tokio::test]
async fn feedback_attachment_rejects_write_methods() {
let state = test_state().await;
for method in ["POST", "PUT", "PATCH", "DELETE"] {
let response = router(state.clone())
.oneshot(
Request::builder()
.method(method)
.uri(format!("/feedback/{}/attachments/{HASH}", Uuid::nil()))
.header(header::HOST, "admin.example")
.body(Body::empty())
.expect("request"),
)
.await
.expect("response");
assert_eq!(
response.status(),
axum::http::StatusCode::METHOD_NOT_ALLOWED,
"{method}"
);
}
}
#[test]
fn report_filters_reject_unknown_values() {
@@ -244,4 +450,68 @@ mod tests {
"Useful context."
);
}
fn attachment_tags(host: &str, x: &str, url_hash: &str) -> serde_json::Value {
serde_json::json!([[
"imeta",
format!("url https://{host}/media/{url_hash}.png"),
"m image/png",
format!("x {x}"),
"size 100"
]])
}
#[test]
fn feedback_attachment_requires_matching_imeta_hash_and_source_host() {
let tags = attachment_tags("community.example", HASH, HASH);
assert!(feedback_references_hash(&tags, "community.example", HASH));
let unreferenced = "f".repeat(64);
assert!(!feedback_references_hash(
&tags,
"community.example",
&unreferenced
));
assert!(!feedback_references_hash(
&tags,
"other-community.example",
HASH
));
}
#[test]
fn feedback_attachment_rejects_cross_field_and_path_substitution() {
let other_hash = "f".repeat(64);
assert!(!feedback_references_hash(
&attachment_tags("community.example", HASH, &other_hash),
"community.example",
HASH
));
for url in [
format!("https://community.example/media/{HASH}.png?token=leak"),
format!("https://community.example/media/{HASH}.thumb.jpg"),
format!("https://community.example/media/{HASH}.png/extra"),
format!("https://evil.example/media/{HASH}.png"),
] {
assert!(!attachment_url_matches(&url, "community.example", HASH));
}
}
#[test]
fn feedback_attachment_accepts_valid_relative_source_url() {
assert!(attachment_url_matches(
&format!("/media/{HASH}.png"),
"community.example",
HASH
));
}
#[test]
fn feedback_attachment_hash_is_exact_lowercase_sha256() {
assert!(is_sha256(HASH));
assert!(!is_sha256(&HASH.to_uppercase()));
assert!(!is_sha256(&HASH[..63]));
assert!(!is_sha256(&format!("{HASH}.png")));
}
}

View File

@@ -606,17 +606,30 @@ pub async fn get_blob(
req_headers: HeaderMap,
) -> Result<Response, MediaError> {
validate_media_path(&sha256_ext)?;
let require_media_get_auth = state.config.require_media_get_auth;
let media_auth = authenticate_media_read(&state, &req_headers, &sha256_ext).await?;
let tenant = media_auth.tenant;
let cache_control = blob_cache_control(require_media_get_auth);
serve_blob_for_tenant(&state, &media_auth.tenant, &sha256_ext, &req_headers).await
}
/// Serve a validated blob from an already-authorized tenant context.
///
/// This is the common byte-serving mechanism for Blossom reads and narrowly
/// scoped internal readers. Callers must establish their own authorization
/// before entering this function; the tenant is never derived from client input.
pub(crate) async fn serve_blob_for_tenant(
state: &AppState,
tenant: &TenantContext,
sha256_ext: &str,
req_headers: &HeaderMap,
) -> Result<Response, MediaError> {
validate_media_path(sha256_ext)?;
let cache_control = blob_cache_control(state.config.require_media_get_auth);
// Sidecar gate FIRST — reject before any blob I/O. Storage is not authoritative.
let content_type = if sha256_ext.ends_with(".thumb.jpg") {
let parent_hash = sha256_ext.strip_suffix(".thumb.jpg").unwrap_or(&sha256_ext);
let parent_hash = sha256_ext.strip_suffix(".thumb.jpg").unwrap_or(sha256_ext);
let _ = state
.media_storage
.read_sidecar_mime(&tenant, parent_hash)
.read_sidecar_mime(tenant, parent_hash)
.await
.ok_or(MediaError::NotFound)?;
"image/jpeg".to_string()
@@ -625,14 +638,14 @@ pub async fn get_blob(
// the sidecar's canonical extension — sidecar is authoritative.
let sidecar_mime = state
.media_storage
.read_sidecar_mime(&tenant, &sha256_ext)
.read_sidecar_mime(tenant, sha256_ext)
.await
.ok_or(MediaError::NotFound)?;
if sha256_ext.contains('.') {
let requested_ext = sha256_ext.rsplit('.').next().unwrap_or("");
let sidecar = state
.media_storage
.get_sidecar(&tenant, sha256_ext.split('.').next().unwrap_or(&sha256_ext))
.get_sidecar(tenant, sha256_ext.split('.').next().unwrap_or(sha256_ext))
.await
.map_err(|_| MediaError::NotFound)?;
if requested_ext != sidecar.ext {
@@ -652,7 +665,7 @@ pub async fn get_blob(
"attachment"
};
let key = resolve_s3_key(&state.media_storage, &tenant, &sha256_ext).await?;
let key = resolve_s3_key(&state.media_storage, tenant, sha256_ext).await?;
// Parse optional Range header.
let range_header = req_headers
@@ -661,8 +674,7 @@ pub async fn get_blob(
.map(|s| s.to_owned());
// Extract single-range value, if present. Multi-range (comma-separated) is
// unsupported — we ignore it and serve the full body per RFC 9110 §14.2:
// "A server MAY ignore the Range header field."
// unsupported — we ignore it and serve the full body per RFC 9110 §14.2.
let single_range = range_header.filter(|r| !r.contains(','));
match single_range {
@@ -689,7 +701,7 @@ pub async fn get_blob(
Ok(resp)
}
Some(range_str) => {
// Single-range request — HEAD first to get total size without loading the blob.
// S3-native single-range response, capped to bound request memory.
let total = state
.media_storage
.head_with_metadata(&key)
@@ -697,7 +709,6 @@ pub async fn get_blob(
.ok_or(MediaError::NotFound)?
.size;
// Parse range: "bytes=START-END", "bytes=START-", or "bytes=-N" (suffix).
let parsed = parse_byte_range(&range_str, total);
match parsed {
Some((start, end)) => {
@@ -709,13 +720,10 @@ pub async fn get_blob(
.map_err(|_| MediaError::Internal);
}
// Clamp end to total-1, then cap chunk size.
let end = end.min(total.saturating_sub(1));
let end = end
.min(start.saturating_add(MAX_RANGE_CHUNK - 1))
.min(total.saturating_sub(1));
// S3-native range GET — never loads the full blob into RAM.
let chunk = state.media_storage.get_range(&key, start, end).await?;
let content_range = format!("bytes {start}-{end}/{total}");

View File

@@ -36,3 +36,34 @@ For local review, run `just admin-seed` before `just admin`. The seed command
also uploads real image and diagnostic fixtures to local MinIO. Feedback search
and filters run over the bounded browser result set; the **Acted on** checkbox is
stored in that browser's local storage.
## Feedback attachment boundary
Feedback attachment bytes are available only through the feedback-scoped read
route:
- `GET /api/admin/v1/feedback/:id/attachments/:sha256`
The route uses the same private-ingress, exact admin `Host`, and same-origin
boundary as the JSON API. It is not a generic media endpoint. The relay loads
the feedback row, derives its community from server-owned provenance, verifies
that host resolution still maps to the row's `community_id`, and requires the
requested SHA-256 to match both the `x` field and source-community `/media/` URL
in that row's persisted `imeta` tag. It then reads the tenant-scoped media
sidecar before accessing the shared content-addressed blob. Unknown feedback,
unreferenced hashes, malformed paths, and cross-community substitutions all
collapse to `404`.
Only `GET` and `HEAD` are routed. Existing community `/media/*` authorization is
unchanged, including `BUZZ_REQUIRE_MEDIA_GET_AUTH`; the browser receives no
Blossom credential or reusable signed URL. Responses are uncached, `nosniff`,
governed by a restrictive CSP, streamed from object storage, and non-previewable
content retains attachment disposition. Successful reads produce a structured
trace containing feedback ID, community ID, and attachment hash, but no feedback
body or attachment URL.
The human trust boundary remains the private admin ingress. WARP/source-IP
admission is not per-operator identity. Anyone admitted to the dashboard can
read attachments for feedback records they can access. Per-person attribution
or revocation requires authenticated operator identity at ingress/application
level; this endpoint deliberately does not claim to provide it.