From 9d937accc22ac07d1a6098892c6469fde8c1f679 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Fri, 29 May 2026 09:32:45 +0200 Subject: [PATCH 01/11] initial logic --- relay-server/src/endpoints/common.rs | 1 + relay-server/src/endpoints/upload.rs | 45 +++++++-- relay-server/src/services/upload.rs | 66 ++++++++----- relay-server/src/utils/tus.rs | 143 +++++++++++++++++++++++++-- tests/integration/test_minidump.py | 43 ++++++++ tests/integration/test_upload.py | 65 ++++++++++++ 6 files changed, 321 insertions(+), 42 deletions(-) diff --git a/relay-server/src/endpoints/common.rs b/relay-server/src/endpoints/common.rs index 291a8e7818e..bdb9cd0f83c 100644 --- a/relay-server/src/endpoints/common.rs +++ b/relay-server/src/endpoints/common.rs @@ -600,6 +600,7 @@ where .send(Create { scoping, length: None, + attachment_type: item.attachment_type(), }) .await .ok()? diff --git a/relay-server/src/endpoints/upload.rs b/relay-server/src/endpoints/upload.rs index dbb377bd013..de91c590d58 100644 --- a/relay-server/src/endpoints/upload.rs +++ b/relay-server/src/endpoints/upload.rs @@ -154,10 +154,13 @@ async fn handle_post( check_kill_switch(&state)?; relay_log::trace!("Validating headers"); - let upload_length = tus::validate_post_headers(&headers).map_err(Error::from)?; + let headers = tus::validate_post_headers(&headers).map_err(Error::from)?; let config = state.config(); - if upload_length.is_some_and(|len| len > config.max_upload_size()) { + if headers + .upload_length + .is_some_and(|len| len > config.max_upload_size()) + { return Err(StatusCode::PAYLOAD_TOO_LARGE.into()); } @@ -174,11 +177,11 @@ async fn handle_post( })?; relay_log::trace!("Checking request"); - let scoping = check_request(&state, meta, upload_length, project).await?; + let scoping = check_request(&state, meta, &headers, project).await?; // Unconditionally create the upload location: relay_log::trace!("Creating upload location"); - let result = create(&state, scoping, upload_length).await; + let result = create(&state, scoping, &headers).await; let location = result.inspect_err(|e| { relay_log::warn!(error = e as &dyn std::error::Error, "create failed"); })?; @@ -221,7 +224,7 @@ async fn handle_patch( })?; relay_log::trace!("Checking request"); - let scoping = check_request(&state, meta, length.value(), project).await?; + let scoping = scoping(&state, meta, project).await?; let stream = body .into_data_stream() @@ -282,13 +285,14 @@ fn check_kill_switch(state: &ServiceState) -> Result<(), StatusCode> { async fn create( state: &ServiceState, scoping: Scoping, - upload_length: Option, + headers: &tus::Headers, ) -> Result, Error> { let location = state .upload() .send(upload::Create { scoping, - length: upload_length, + length: headers.upload_length, + attachment_type: headers.metadata.map(|m| m.attachment_type), }) .await??; @@ -321,14 +325,17 @@ async fn upload( async fn check_request( state: &ServiceState, meta: RequestMeta, - upload_length: Option, + headers: &tus::Headers, project: Project<'_>, ) -> Result { let mut envelope = Envelope::from_request(None, meta); envelope.require_feature(Feature::UploadEndpoint); let mut item = Item::new(ItemType::Attachment); item.set_payload(ContentType::AttachmentRef, vec![]); - item.set_attachment_length(upload_length.unwrap_or(1)); + item.set_attachment_length(headers.upload_length.unwrap_or(1)); + if let Some(ref metadata) = headers.metadata { + item.set_attachment_type(metadata.attachment_type); + } envelope.add_item(item); let mut envelope = Managed::from_envelope(envelope, state.outcome_aggregator().clone()); let rate_limits = project @@ -347,6 +354,26 @@ async fn check_request( Ok(scoping) } +async fn scoping( + state: &ServiceState, + meta: RequestMeta, + project: Project<'_>, +) -> Result { + let mut envelope = Envelope::from_request(None, meta); + envelope.require_feature(Feature::UploadEndpoint); + let mut envelope = Managed::from_envelope(envelope, state.outcome_aggregator().clone()); + + let _ = project + .check_envelope(&mut envelope) + .await + .map_err(|err| err.map(BadStoreRequest::EventRejected).into_inner())?; + + // We are not really processing an envelope here, only keep the updated scoping: + let scoping = envelope.scoping(); + envelope.accept(|x| x); + Ok(scoping) +} + fn is_hyper_user_error(error: &(dyn std::error::Error + 'static)) -> bool { error .downcast_ref::() diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index 6cba1fbc74e..3209fbea6fa 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -30,6 +30,7 @@ use tokio_util::io::{ReaderStream, StreamReader}; #[cfg(feature = "processing")] use uuid::Uuid; +use crate::envelope::AttachmentType; use crate::http::{HttpError, RequestBuilder, Response}; #[cfg(feature = "processing")] @@ -114,6 +115,8 @@ pub struct Create { /// /// Trusted clients (i.e. PoP Relays) are allowed to omit the length (see `Upload-Defer-Length: 1`). pub length: Option, + /// The attachment type of the upload. + pub attachment_type: Option, } /// The type used to stream a request body. @@ -241,11 +244,15 @@ enum Backend { impl Service { async fn create( &self, - Create { scoping, length }: Create, + Create { + scoping, + length, + attachment_type, + }: Create, ) -> Result, Error> { match &self.backend { Backend::Upstream { addr } => { - let (request, rx) = UploadRequest::create(scoping, length); + let (request, rx) = UploadRequest::create(scoping, length, attachment_type); addr.send(SendRequest(request)); let response = rx.await??; SignedLocation::try_from_response(response) @@ -354,7 +361,7 @@ pub trait UploadLength: for<'de> Deserialize<'de> { /// A provisional upload length which may or may not yet be known. /// -/// /// See also [`Final`]. +/// See also [`Final`]. #[derive(Debug, Clone, Copy, Deserialize)] #[serde(transparent)] pub struct Provisional(Option); @@ -561,6 +568,7 @@ where enum RequestKind { Create { length: Option, + attachment_type: Option, }, Upload { location: SignedLocation, @@ -581,6 +589,7 @@ impl UploadRequest { fn create( scoping: Scoping, length: Option, + attachment_type: Option, ) -> ( Self, oneshot::Receiver>, @@ -590,7 +599,10 @@ impl UploadRequest { ( Self { scoping, - kind: RequestKind::Create { length }, + kind: RequestKind::Create { + length, + attachment_type, + }, sender, }, rx, @@ -627,10 +639,11 @@ impl UploadRequest { ) } + #[expect(dead_code)] /// Returns the length of the upload, if known. fn length(&self) -> Option { match &self.kind { - RequestKind::Create { length } | RequestKind::Upload { length, .. } => *length, + RequestKind::Create { length, .. } | RequestKind::Upload { length, .. } => *length, } } } @@ -690,24 +703,31 @@ impl UpstreamRequest for UploadRequest { } fn build(&mut self, builder: &mut RequestBuilder) -> Result<(), HttpError> { - let upload_length = self.length(); - if let RequestKind::Upload { - stream, encoding, .. - } = &mut self.kind - { - let Some(body) = RetryableStream::new(stream.clone()) else { - relay_log::error!("upload request stream was already consumed"); - return Err(HttpError::Misconfigured); - }; - tus::add_upload_headers(builder); - - let body = encode_body(body, *encoding); - builder.content_encoding(*encoding); - - builder.body(reqwest::Body::wrap_stream(body)); - } else { - tus::add_creation_headers(upload_length, builder); - } + match &mut self.kind { + RequestKind::Create { + length, + attachment_type, + } => { + tus::add_creation_headers(*length, *attachment_type, builder); + } + RequestKind::Upload { + location: _, + stream, + length: _, + encoding, + } => { + let Some(body) = RetryableStream::new(stream.clone()) else { + relay_log::error!("upload request stream was already consumed"); + return Err(HttpError::Misconfigured); + }; + tus::add_upload_headers(builder); + + let body = encode_body(body, *encoding); + builder.content_encoding(*encoding); + + builder.body(reqwest::Body::wrap_stream(body)); + } + }; let project_key = self.scoping.project_key; builder.header("X-Sentry-Auth", format!("Sentry sentry_key={project_key}")); diff --git a/relay-server/src/utils/tus.rs b/relay-server/src/utils/tus.rs index 333592ac1f4..9dbbffe1418 100644 --- a/relay-server/src/utils/tus.rs +++ b/relay-server/src/utils/tus.rs @@ -8,9 +8,11 @@ use std::str::FromStr; use axum::http::HeaderMap; +use data_encoding::BASE64; use http::HeaderValue; use http::header::AsHeaderName; +use crate::envelope::AttachmentType; use crate::http::RequestBuilder; #[derive(Debug, thiserror::Error)] @@ -35,6 +37,12 @@ pub enum Error { expected: &'static str, received: String, }, + /// The `sentry` entry in `Upload-Metadata` is not valid base64. + #[error("invalid base64 in `sentry` Upload-Metadata value: {0}")] + UploadMetadataBase64(#[from] data_encoding::DecodeError), + /// The decoded `sentry` entry in `Upload-Metadata` is not valid JSON or does not match the schema. + #[error("invalid `sentry` Upload-Metadata payload: {0}")] + UploadMetadataJson(#[from] serde_json::Error), } /// TUS protocol header for the protocol version. @@ -67,15 +75,36 @@ pub const UPLOAD_DEFER_LENGTH: &str = "Upload-Defer-Length"; /// See . pub const UPLOAD_OFFSET: &str = "Upload-Offset"; +/// TUS protocol header for metadata. +/// +/// See . +pub const UPLOAD_METADATA: &str = "Upload-Metadata"; + /// Expected value of the content-type header. pub const EXPECTED_CONTENT_TYPE: HeaderValue = HeaderValue::from_static(EXPECTED_CONTENT_TYPE_STR); const EXPECTED_CONTENT_TYPE_STR: &str = "application/offset+octet-stream"; +/// Sentry-specific metadata extracted from the TUS `Upload-Metadata` header. +#[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)] +pub struct Metadata { + /// The [`AttachmentType`] of the upload. + /// + /// Used for preliminary rate-limiting checks. + pub attachment_type: AttachmentType, +} + +/// Subset of TUS request headers. +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct Headers { + /// The declared `Upload-Length`, or `None` if `Upload-Defer-Length: 1` was used. + pub upload_length: Option, + /// Sentry-specific `Upload-Metadata` entry. + pub metadata: Option, +} + /// Validates TUS protocol headers and returns a subset of parsed values. -/// -/// Returns the declared `Upload-Length`. -pub fn validate_post_headers(headers: &HeaderMap) -> Result, Error> { +pub fn validate_post_headers(headers: &HeaderMap) -> Result { let tus_version = headers.get(TUS_RESUMABLE); if tus_version != Some(&TUS_VERSION) { return Err(Error::Version( @@ -94,8 +123,7 @@ pub fn validate_post_headers(headers: &HeaderMap) -> Result, Error let upload_defer_length: Option = parse_header(headers, UPLOAD_DEFER_LENGTH); // Exactly one of Upload-Length and Upload-Defer-Length must be present. - // Upload-Defer-Length is only accepted if its value is 1 (as demanded by the TUS protocol) - // and `allow_defer_length` is true (i.e. the sender is trusted/internal). + // Upload-Defer-Length is only accepted if its value is 1 (as demanded by the TUS protocol). let upload_length = match (upload_length, upload_defer_length) { (Some(u), None) => Ok(Some(u)), (None, Some(1)) => Ok(None), @@ -105,7 +133,12 @@ pub fn validate_post_headers(headers: &HeaderMap) -> Result, Error }), }?; - Ok(upload_length) + let metadata = upload_metadata(headers)?; + + Ok(Headers { + upload_length, + metadata, + }) } /// Validates TUS protocol headers and returns the expected upload length. @@ -139,7 +172,18 @@ pub fn validate_patch_headers(headers: &HeaderMap) -> Result<(), Error> { } /// Prepares the required TUS request headers for upstream requests. -pub fn add_creation_headers(upload_length: Option, builder: &mut RequestBuilder) { +pub fn add_creation_headers( + upload_length: Option, + attachment_type: Option, + builder: &mut RequestBuilder, +) { + if let Some(attachment_type) = attachment_type + && let Ok(json) = serde_json::to_vec(&Metadata { attachment_type }) + && let Ok(header) = HeaderValue::from_str(&format!("sentry {}", BASE64.encode(&json))) + { + builder.header(UPLOAD_METADATA, header); + } + builder.header(TUS_RESUMABLE, TUS_VERSION); if let Some(upload_length) = upload_length { builder.header(UPLOAD_LENGTH, HeaderValue::from(upload_length)); @@ -163,6 +207,24 @@ pub fn response_headers() -> HeaderMap { headers } +/// Extracts the sentry metadata payload from the `Upload-Metadata` header. +fn upload_metadata(headers: &HeaderMap) -> Result, Error> { + let Some(metadata): Option = parse_header(headers, UPLOAD_METADATA) else { + return Ok(None); + }; + + let Some(sentry_payload) = metadata.split(',').find_map(|kv| match kv.split_once(' ') { + Some(("sentry", value)) => Some(value), + _ => None, + }) else { + return Ok(None); + }; + + let decoded_sentry_payload = BASE64.decode(sentry_payload.as_bytes())?; + let metadata = serde_json::from_slice(&decoded_sentry_payload)?; + Ok(Some(metadata)) +} + fn parse_header(headers: &HeaderMap, header_name: K) -> Option { headers.get(header_name)?.to_str().ok()?.parse().ok() } @@ -205,7 +267,13 @@ mod tests { headers.insert(TUS_RESUMABLE, HeaderValue::from_static("1.0.0")); headers.insert(UPLOAD_LENGTH, HeaderValue::from_static("1024")); let result = validate_post_headers(&headers); - assert_eq!(result.unwrap(), Some(1024)); + assert_eq!( + result.unwrap(), + Headers { + upload_length: Some(1024), + metadata: None + } + ); } #[test] @@ -218,7 +286,13 @@ mod tests { ); headers.insert(UPLOAD_LENGTH, HeaderValue::from_static("1024")); let result = validate_post_headers(&headers); - assert_eq!(result.unwrap(), Some(1024)); + assert!(matches!( + result, + Ok(Headers { + upload_length: Some(1024), + metadata: None + }) + )); } #[test] @@ -240,7 +314,13 @@ mod tests { ); headers.insert(UPLOAD_DEFER_LENGTH, HeaderValue::from_static("1")); let result = validate_post_headers(&headers); - assert!(matches!(result, Ok(None))); + assert!(matches!( + result, + Ok(Headers { + upload_length: None, + metadata: None + }) + )); } #[test] @@ -252,6 +332,49 @@ mod tests { assert!(matches!(result, Err(Error::UploadLength { .. }))); } + #[test] + fn test_validate_tus_headers_upload_meta() { + let mut headers = HeaderMap::new(); + headers.insert(TUS_RESUMABLE, HeaderValue::from_static("1.0.0")); + headers.insert(UPLOAD_DEFER_LENGTH, HeaderValue::from_static("1")); + headers.insert( + UPLOAD_METADATA, + HeaderValue::from_static( + "filename d29ybGRfZG9taW5hdGlvbl9wbGFuLnBkZg==,is_confidential,sentry eyJhdHRhY2htZW50X3R5cGUiOiAiZXZlbnQubWluaWR1bXAiLCAgInBhcmVudF90eXBlIjogImVycm9yIn0=", + ), + ); + let result = validate_post_headers(&headers); + assert!(matches!( + result, + Ok(Headers { + upload_length: None, + metadata: Some(Metadata { + attachment_type: AttachmentType::Minidump + }) + }) + )); + } + + #[test] + fn test_validate_tus_headers_upload_meta_bad_base64() { + let mut headers = HeaderMap::new(); + headers.insert(TUS_RESUMABLE, HeaderValue::from_static("1.0.0")); + headers.insert(UPLOAD_DEFER_LENGTH, HeaderValue::from_static("1")); + headers.insert(UPLOAD_METADATA, HeaderValue::from_static("sentry e=")); + let result = validate_post_headers(&headers); + assert!(matches!(result, Err(Error::UploadMetadataBase64(_)))); + } + + #[test] + fn test_validate_tus_headers_upload_meta_bad_json() { + let mut headers = HeaderMap::new(); + headers.insert(TUS_RESUMABLE, HeaderValue::from_static("1.0.0")); + headers.insert(UPLOAD_DEFER_LENGTH, HeaderValue::from_static("1")); + headers.insert(UPLOAD_METADATA, HeaderValue::from_static("sentry e30=")); + let result = validate_post_headers(&headers); + assert!(matches!(result, Err(Error::UploadMetadataJson(_)))); + } + #[test] fn test_validate_patch_headers_missing_version() { let headers = HeaderMap::new(); diff --git a/tests/integration/test_minidump.py b/tests/integration/test_minidump.py index 401c3661254..559f6880811 100644 --- a/tests/integration/test_minidump.py +++ b/tests/integration/test_minidump.py @@ -1076,6 +1076,49 @@ def test_minidump_objectstore_uploads_external_chain( } +def test_minidump_objectstore_uploads_external_chain_attachment_limited( + mini_sentry, + relay, + relay_with_processing, + attachments_consumer, +): + project_id = 42 + project_config = mini_sentry.add_full_project_config(project_id) + project_config["config"].setdefault("features", []).extend( + [ + "projects:relay-minidump-attachment-uploads", + "projects:relay-upload-endpoint", + "projects:relay-minidump-uploads", + ] + ) + project_config["config"]["quotas"] = [ + { + "categories": ["attachment"], + "limit": 0, + "reasonCode": "attachments_exceeded", + }, + ] + + relay = relay(relay_with_processing(), external=True) + project_config["config"]["trustedRelays"] = list(relay.iter_public_keys()) + + attachments_consumer = attachments_consumer() + + response = relay.send_minidump( + project_id=project_id, + files=[ + (MINIDUMP_ATTACHMENT_NAME, "minidump.dmp", b"MDMP content"), + ("logs", "log.txt", b"Some log file content"), + ], + ) + assert response.ok + + _, unpacked = attachments_consumer.get_message() + assert {att["name"] for att in unpacked["attachments"]} == { + "minidump.dmp", + } + + def test_minidump_objectstore_errors( mini_sentry, relay, diff --git a/tests/integration/test_upload.py b/tests/integration/test_upload.py index c466ddf3e5c..fa97947a8db 100644 --- a/tests/integration/test_upload.py +++ b/tests/integration/test_upload.py @@ -182,6 +182,71 @@ def test_upload_unsupported_tus_version( } +@pytest.mark.parametrize( + "metadata,expected_status_code,expected_detail", + [ + pytest.param( + "sentry eyJhdHRhY2htZW50X3R5cGUiOiAiZXZlbnQubWluaWR1bXAifQ==", + 201, + None, + id="valid_metadata", + ), + pytest.param( + None, + 201, + None, + id="no_metadata", + ), + pytest.param( + "filename d29ybGRfZG9taW5hdGlvbl9wbGFuLnBkZg==", + 201, + None, + id="no_sentry_entry", + ), + pytest.param( + "sentry e=", + 400, + "TUS protocol error: invalid base64 in `sentry` Upload-Metadata value: invalid length at 0", + id="invalid_base64", + ), + pytest.param( + "sentry e30=", + 400, + "TUS protocol error: invalid `sentry` Upload-Metadata payload: missing field `attachment_type` at line 1 column 2", + id="invalid_json", + ), + ], +) +def test_upload_with_metadata( + mini_sentry, + relay, + dummy_upload, + project_config, + metadata, + expected_status_code, + expected_detail, +): + project_id = 42 + relay = relay(mini_sentry) + + headers = { + "Tus-Resumable": "1.0.0", + "Upload-Defer-Length": "1", + } + if metadata is not None: + headers["Upload-Metadata"] = metadata + + response = relay.post( + "/api/%s/upload/?sentry_key=%s" + % (project_id, mini_sentry.get_dsn_public_key(project_id)), + headers=headers, + ) + + assert response.status_code == expected_status_code + if expected_detail is not None: + assert response.json()["detail"] == expected_detail + + def test_upload_missing_upload_length(mini_sentry, relay, dummy_upload, project_config): project_id = 42 From 2736e6c7612fd454e149e6d4fcb3e2d672e3e660 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 2 Jun 2026 11:20:55 -0400 Subject: [PATCH 02/11] remove unused length field from --- relay-server/src/services/upload.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index 3209fbea6fa..63ea1c672ef 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -573,7 +573,6 @@ enum RequestKind { Upload { location: SignedLocation, stream: TakeOnce>>, - length: Option, encoding: HttpEncoding, }, } @@ -622,7 +621,6 @@ impl UploadRequest { location, stream, } = stream; - let length = stream.length(); ( Self { @@ -630,7 +628,6 @@ impl UploadRequest { kind: RequestKind::Upload { location, stream: TakeOnce::new(stream), - length, encoding: HttpEncoding::Zstd, // just a default, will be overwritten by .configure() }, sender, @@ -638,14 +635,6 @@ impl UploadRequest { rx, ) } - - #[expect(dead_code)] - /// Returns the length of the upload, if known. - fn length(&self) -> Option { - match &self.kind { - RequestKind::Create { length, .. } | RequestKind::Upload { length, .. } => *length, - } - } } impl fmt::Debug for UploadRequest { @@ -713,7 +702,6 @@ impl UpstreamRequest for UploadRequest { RequestKind::Upload { location: _, stream, - length: _, encoding, } => { let Some(body) = RetryableStream::new(stream.clone()) else { From 4d80608392ebb9d83123a8c54c453a59536632b7 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 2 Jun 2026 11:22:05 -0400 Subject: [PATCH 03/11] rename to --- relay-server/src/endpoints/upload.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/relay-server/src/endpoints/upload.rs b/relay-server/src/endpoints/upload.rs index de91c590d58..cea922b9223 100644 --- a/relay-server/src/endpoints/upload.rs +++ b/relay-server/src/endpoints/upload.rs @@ -224,7 +224,7 @@ async fn handle_patch( })?; relay_log::trace!("Checking request"); - let scoping = scoping(&state, meta, project).await?; + let scoping = validate_request(&state, meta, project).await?; let stream = body .into_data_stream() @@ -354,7 +354,7 @@ async fn check_request( Ok(scoping) } -async fn scoping( +async fn validate_request( state: &ServiceState, meta: RequestMeta, project: Project<'_>, From 64a345758c87645036b3c75e5691f5e1451a959a Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 2 Jun 2026 11:29:28 -0400 Subject: [PATCH 04/11] rename Error variants --- relay-server/src/utils/tus.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/relay-server/src/utils/tus.rs b/relay-server/src/utils/tus.rs index 9dbbffe1418..d25c570ccdf 100644 --- a/relay-server/src/utils/tus.rs +++ b/relay-server/src/utils/tus.rs @@ -39,10 +39,10 @@ pub enum Error { }, /// The `sentry` entry in `Upload-Metadata` is not valid base64. #[error("invalid base64 in `sentry` Upload-Metadata value: {0}")] - UploadMetadataBase64(#[from] data_encoding::DecodeError), + InvalidMetadataBase64(#[from] data_encoding::DecodeError), /// The decoded `sentry` entry in `Upload-Metadata` is not valid JSON or does not match the schema. #[error("invalid `sentry` Upload-Metadata payload: {0}")] - UploadMetadataJson(#[from] serde_json::Error), + InvalidMetadata(#[from] serde_json::Error), } /// TUS protocol header for the protocol version. @@ -362,7 +362,7 @@ mod tests { headers.insert(UPLOAD_DEFER_LENGTH, HeaderValue::from_static("1")); headers.insert(UPLOAD_METADATA, HeaderValue::from_static("sentry e=")); let result = validate_post_headers(&headers); - assert!(matches!(result, Err(Error::UploadMetadataBase64(_)))); + assert!(matches!(result, Err(Error::InvalidMetadataBase64(_)))); } #[test] @@ -372,7 +372,7 @@ mod tests { headers.insert(UPLOAD_DEFER_LENGTH, HeaderValue::from_static("1")); headers.insert(UPLOAD_METADATA, HeaderValue::from_static("sentry e30=")); let result = validate_post_headers(&headers); - assert!(matches!(result, Err(Error::UploadMetadataJson(_)))); + assert!(matches!(result, Err(Error::InvalidMetadata(_)))); } #[test] From 5a3d9dd5803ce6d5486350e395b944d49ce09f63 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 2 Jun 2026 11:30:57 -0400 Subject: [PATCH 05/11] import serde --- relay-server/src/utils/tus.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/relay-server/src/utils/tus.rs b/relay-server/src/utils/tus.rs index d25c570ccdf..c517f578ff2 100644 --- a/relay-server/src/utils/tus.rs +++ b/relay-server/src/utils/tus.rs @@ -11,6 +11,7 @@ use axum::http::HeaderMap; use data_encoding::BASE64; use http::HeaderValue; use http::header::AsHeaderName; +use serde::{Deserialize, Serialize}; use crate::envelope::AttachmentType; use crate::http::RequestBuilder; @@ -86,7 +87,7 @@ pub const EXPECTED_CONTENT_TYPE: HeaderValue = HeaderValue::from_static(EXPECTED const EXPECTED_CONTENT_TYPE_STR: &str = "application/offset+octet-stream"; /// Sentry-specific metadata extracted from the TUS `Upload-Metadata` header. -#[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Deserialize, Serialize)] pub struct Metadata { /// The [`AttachmentType`] of the upload. /// From 874c1a05cbcc1373e488d1696dafdef6997f71cc Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Wed, 3 Jun 2026 07:01:59 -0400 Subject: [PATCH 06/11] bubble up tus header errors --- relay-server/src/http.rs | 4 ++++ relay-server/src/services/upload.rs | 2 +- relay-server/src/services/upstream.rs | 2 ++ relay-server/src/utils/tus.rs | 15 ++++++++------- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/relay-server/src/http.rs b/relay-server/src/http.rs index 626ed644bc0..eb05e515450 100644 --- a/relay-server/src/http.rs +++ b/relay-server/src/http.rs @@ -11,6 +11,7 @@ use std::io; use std::time::Duration; +use http::header::InvalidHeaderValue; use relay_config::HttpEncoding; pub use reqwest::StatusCode; use reqwest::header::{HeaderMap, HeaderValue}; @@ -28,6 +29,8 @@ pub enum HttpError { Json(#[from] serde_json::Error), #[error("request was retried or not initialized")] Misconfigured, + #[error("failed to construct header")] + Header(#[from] InvalidHeaderValue), } impl HttpError { @@ -41,6 +44,7 @@ impl HttpError { Self::Json(_) => false, Self::Overflow => false, Self::Misconfigured => false, + Self::Header(_) => false, } } } diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index 63ea1c672ef..0b39761cdaf 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -697,7 +697,7 @@ impl UpstreamRequest for UploadRequest { length, attachment_type, } => { - tus::add_creation_headers(*length, *attachment_type, builder); + tus::add_creation_headers(*length, *attachment_type, builder)?; } RequestKind::Upload { location: _, diff --git a/relay-server/src/services/upstream.rs b/relay-server/src/services/upstream.rs index 791e750c80c..7aa828984c3 100644 --- a/relay-server/src/services/upstream.rs +++ b/relay-server/src/services/upstream.rs @@ -198,6 +198,7 @@ impl UpstreamRequestError { UpstreamRequestError::Http(HttpError::Reqwest(_)) => "reqwest_error", UpstreamRequestError::Http(HttpError::Overflow) => "overflow", UpstreamRequestError::Http(HttpError::Misconfigured) => "misconfigured", + UpstreamRequestError::Http(HttpError::Header(_)) => "header", UpstreamRequestError::RateLimited(_) => "rate_limited", UpstreamRequestError::ResponseError(_, _) => "response_error", UpstreamRequestError::ChannelClosed => "channel_closed", @@ -218,6 +219,7 @@ impl IntoResponse for UpstreamRequestError { HttpError::Io(_) => StatusCode::BAD_GATEWAY.into_response(), HttpError::Json(_) => StatusCode::BAD_REQUEST.into_response(), HttpError::Misconfigured => StatusCode::INTERNAL_SERVER_ERROR.into_response(), + HttpError::Header(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(), }, Self::SendFailed(e) => { if find_error_source(&e, is_length_limit_error).is_some() { diff --git a/relay-server/src/utils/tus.rs b/relay-server/src/utils/tus.rs index c517f578ff2..51936ece898 100644 --- a/relay-server/src/utils/tus.rs +++ b/relay-server/src/utils/tus.rs @@ -14,7 +14,7 @@ use http::header::AsHeaderName; use serde::{Deserialize, Serialize}; use crate::envelope::AttachmentType; -use crate::http::RequestBuilder; +use crate::http::{HttpError, RequestBuilder}; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -177,11 +177,10 @@ pub fn add_creation_headers( upload_length: Option, attachment_type: Option, builder: &mut RequestBuilder, -) { - if let Some(attachment_type) = attachment_type - && let Ok(json) = serde_json::to_vec(&Metadata { attachment_type }) - && let Ok(header) = HeaderValue::from_str(&format!("sentry {}", BASE64.encode(&json))) - { +) -> Result<(), HttpError> { + if let Some(attachment_type) = attachment_type { + let json = serde_json::to_vec(&Metadata { attachment_type })?; + let header = HeaderValue::from_str(&format!("sentry {}", BASE64.encode(&json)))?; builder.header(UPLOAD_METADATA, header); } @@ -190,7 +189,9 @@ pub fn add_creation_headers( builder.header(UPLOAD_LENGTH, HeaderValue::from(upload_length)); } else { builder.header(UPLOAD_DEFER_LENGTH, "1"); - } + }; + + Ok(()) } /// Prepares the required TUS request headers for upstream requests. From 5e25d0648916e60f8df763c9349cf3f31a27fdca Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Wed, 3 Jun 2026 07:03:03 -0400 Subject: [PATCH 07/11] split up upload_metadata --- relay-server/src/utils/tus.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/relay-server/src/utils/tus.rs b/relay-server/src/utils/tus.rs index 51936ece898..dc44c496e9f 100644 --- a/relay-server/src/utils/tus.rs +++ b/relay-server/src/utils/tus.rs @@ -211,20 +211,25 @@ pub fn response_headers() -> HeaderMap { /// Extracts the sentry metadata payload from the `Upload-Metadata` header. fn upload_metadata(headers: &HeaderMap) -> Result, Error> { - let Some(metadata): Option = parse_header(headers, UPLOAD_METADATA) else { + let Some(payload) = parse_upload_metadata(headers) else { return Ok(None); }; + Some(parse_sentry_metadata(&payload)).transpose() +} - let Some(sentry_payload) = metadata.split(',').find_map(|kv| match kv.split_once(' ') { - Some(("sentry", value)) => Some(value), +/// Extracts the raw (base64) sentry payload from the `Upload-Metadata` header. +fn parse_upload_metadata(headers: &HeaderMap) -> Option { + let metadata: String = parse_header(headers, UPLOAD_METADATA)?; + metadata.split(',').find_map(|kv| match kv.split_once(' ') { + Some(("sentry", value)) => Some(value.to_owned()), _ => None, - }) else { - return Ok(None); - }; + }) +} - let decoded_sentry_payload = BASE64.decode(sentry_payload.as_bytes())?; - let metadata = serde_json::from_slice(&decoded_sentry_payload)?; - Ok(Some(metadata)) +/// Decodes and deserializes the sentry metadata payload. +fn parse_sentry_metadata(payload: &str) -> Result { + let decoded = BASE64.decode(payload.as_bytes())?; + Ok(serde_json::from_slice(&decoded)?) } fn parse_header(headers: &HeaderMap, header_name: K) -> Option { From 4bf2efde571005e7596975f0ab006a037253ce85 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Wed, 3 Jun 2026 08:37:55 -0400 Subject: [PATCH 08/11] avoid relay_with_processing in test --- tests/integration/test_minidump.py | 31 ++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/tests/integration/test_minidump.py b/tests/integration/test_minidump.py index 559f6880811..eb4249ce3a0 100644 --- a/tests/integration/test_minidump.py +++ b/tests/integration/test_minidump.py @@ -1079,9 +1079,13 @@ def test_minidump_objectstore_uploads_external_chain( def test_minidump_objectstore_uploads_external_chain_attachment_limited( mini_sentry, relay, - relay_with_processing, - attachments_consumer, + dummy_upload, ): + mini_sentry.global_config["options"][ + "relay.objectstore-attachments.sample-rate" + ] = 1.0 + mini_sentry.global_config["options"]["relay.endpoint-fetch-config.enabled"] = True + project_id = 42 project_config = mini_sentry.add_full_project_config(project_id) project_config["config"].setdefault("features", []).extend( @@ -1099,25 +1103,36 @@ def test_minidump_objectstore_uploads_external_chain_attachment_limited( }, ] - relay = relay(relay_with_processing(), external=True) + inner = relay(mini_sentry, options={"outcomes": {"emit_outcomes": True}}) + relay = relay(inner, external=True, options={"outcomes": {"emit_outcomes": True}}) project_config["config"]["trustedRelays"] = list(relay.iter_public_keys()) - attachments_consumer = attachments_consumer() + # Do some busy work + relay.send_event(project_id) + mini_sentry.get_captured_envelope() response = relay.send_minidump( project_id=project_id, files=[ - (MINIDUMP_ATTACHMENT_NAME, "minidump.dmp", b"MDMP content"), + (MINIDUMP_ATTACHMENT_NAME, "minidump.dmp", b"MDMPcontent"), ("logs", "log.txt", b"Some log file content"), ], ) assert response.ok - _, unpacked = attachments_consumer.get_message() - assert {att["name"] for att in unpacked["attachments"]} == { - "minidump.dmp", + envelope = mini_sentry.get_captured_envelope() + by_name = { + i.headers.get("filename"): i + for i in envelope.items + if i.headers.get("type") == "attachment" } + assert "log.txt" not in by_name + assert ( + by_name["minidump.dmp"].headers["content_type"] + == "application/vnd.sentry.attachment-ref+json" + ) + def test_minidump_objectstore_errors( mini_sentry, From 677231e700f0648dd61fd6eade39b72551db3ce0 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Wed, 3 Jun 2026 09:23:30 -0400 Subject: [PATCH 09/11] move logic from integration to unit tests --- relay-server/src/utils/tus.rs | 65 ++++++++++++++++++++++++++------ tests/integration/test_upload.py | 55 +++------------------------ 2 files changed, 59 insertions(+), 61 deletions(-) diff --git a/relay-server/src/utils/tus.rs b/relay-server/src/utils/tus.rs index dc44c496e9f..9969ab6efb5 100644 --- a/relay-server/src/utils/tus.rs +++ b/relay-server/src/utils/tus.rs @@ -363,23 +363,64 @@ mod tests { } #[test] - fn test_validate_tus_headers_upload_meta_bad_base64() { + fn test_parse_sentry_metadata_valid() { + let payload = "eyJhdHRhY2htZW50X3R5cGUiOiAiZXZlbnQubWluaWR1bXAifQ=="; + assert_eq!( + parse_sentry_metadata(payload).unwrap(), + Metadata { + attachment_type: AttachmentType::Minidump + } + ); + } + + #[test] + fn test_parse_sentry_metadata_invalid_base64() { + assert!(matches!( + parse_sentry_metadata("e="), + Err(Error::InvalidMetadataBase64(_)) + )); + } + + #[test] + fn test_parse_sentry_metadata_invalid_json() { + assert!(matches!( + parse_sentry_metadata("e30="), + Err(Error::InvalidMetadata(_)) + )); + } + + #[test] + fn test_upload_metadata_absent() { + assert_eq!(upload_metadata(&HeaderMap::new()).unwrap(), None); + } + + #[test] + fn test_upload_metadata_no_sentry_entry() { let mut headers = HeaderMap::new(); - headers.insert(TUS_RESUMABLE, HeaderValue::from_static("1.0.0")); - headers.insert(UPLOAD_DEFER_LENGTH, HeaderValue::from_static("1")); - headers.insert(UPLOAD_METADATA, HeaderValue::from_static("sentry e=")); - let result = validate_post_headers(&headers); - assert!(matches!(result, Err(Error::InvalidMetadataBase64(_)))); + headers.insert( + UPLOAD_METADATA, + HeaderValue::from_static( + "filename d29ybGRfZG9taW5hdGlvbl9wbGFuLnBkZg==,is_confidential", + ), + ); + assert_eq!(upload_metadata(&headers).unwrap(), None); } #[test] - fn test_validate_tus_headers_upload_meta_bad_json() { + fn test_upload_metadata_with_sentry_entry() { let mut headers = HeaderMap::new(); - headers.insert(TUS_RESUMABLE, HeaderValue::from_static("1.0.0")); - headers.insert(UPLOAD_DEFER_LENGTH, HeaderValue::from_static("1")); - headers.insert(UPLOAD_METADATA, HeaderValue::from_static("sentry e30=")); - let result = validate_post_headers(&headers); - assert!(matches!(result, Err(Error::InvalidMetadata(_)))); + headers.insert( + UPLOAD_METADATA, + HeaderValue::from_static( + "filename d29ybGRfZG9taW5hdGlvbl9wbGFuLnBkZg==,sentry eyJhdHRhY2htZW50X3R5cGUiOiAiZXZlbnQubWluaWR1bXAifQ==", + ), + ); + assert_eq!( + upload_metadata(&headers).unwrap(), + Some(Metadata { + attachment_type: AttachmentType::Minidump + }) + ); } #[test] diff --git a/tests/integration/test_upload.py b/tests/integration/test_upload.py index fa97947a8db..9f80d3d6b27 100644 --- a/tests/integration/test_upload.py +++ b/tests/integration/test_upload.py @@ -182,69 +182,26 @@ def test_upload_unsupported_tus_version( } -@pytest.mark.parametrize( - "metadata,expected_status_code,expected_detail", - [ - pytest.param( - "sentry eyJhdHRhY2htZW50X3R5cGUiOiAiZXZlbnQubWluaWR1bXAifQ==", - 201, - None, - id="valid_metadata", - ), - pytest.param( - None, - 201, - None, - id="no_metadata", - ), - pytest.param( - "filename d29ybGRfZG9taW5hdGlvbl9wbGFuLnBkZg==", - 201, - None, - id="no_sentry_entry", - ), - pytest.param( - "sentry e=", - 400, - "TUS protocol error: invalid base64 in `sentry` Upload-Metadata value: invalid length at 0", - id="invalid_base64", - ), - pytest.param( - "sentry e30=", - 400, - "TUS protocol error: invalid `sentry` Upload-Metadata payload: missing field `attachment_type` at line 1 column 2", - id="invalid_json", - ), - ], -) def test_upload_with_metadata( mini_sentry, relay, dummy_upload, project_config, - metadata, - expected_status_code, - expected_detail, ): project_id = 42 relay = relay(mini_sentry) - headers = { - "Tus-Resumable": "1.0.0", - "Upload-Defer-Length": "1", - } - if metadata is not None: - headers["Upload-Metadata"] = metadata - response = relay.post( "/api/%s/upload/?sentry_key=%s" % (project_id, mini_sentry.get_dsn_public_key(project_id)), - headers=headers, + headers={ + "Tus-Resumable": "1.0.0", + "Upload-Defer-Length": "1", + "Upload-Metadata": "sentry eyJhdHRhY2htZW50X3R5cGUiOiAiZXZlbnQubWluaWR1bXAifQ==", + }, ) - assert response.status_code == expected_status_code - if expected_detail is not None: - assert response.json()["detail"] == expected_detail + assert response.status_code == 201 def test_upload_missing_upload_length(mini_sentry, relay, dummy_upload, project_config): From ae8beac2426d6db91891d1293976c98cad3af27c Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Wed, 3 Jun 2026 09:34:11 -0400 Subject: [PATCH 10/11] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06719975e2f..5a15d9fda00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Features**: - Convert measurements to attributes based on information from `sentry-conventions`. This is gated behind a project feature flag. ([#6007](https://github.com/getsentry/relay/pull/6007)) +- Add metadata support for the `/upload` endpoint. ([#6028](https://github.com/getsentry/relay/pull/6028)) **Bug Fixes**: From 55213a0ce36d8275977a18eed4c6ca9200d51911 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Wed, 3 Jun 2026 10:29:17 -0400 Subject: [PATCH 11/11] rename functions --- relay-server/src/endpoints/upload.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/relay-server/src/endpoints/upload.rs b/relay-server/src/endpoints/upload.rs index 044b4bd74af..239dab27cf8 100644 --- a/relay-server/src/endpoints/upload.rs +++ b/relay-server/src/endpoints/upload.rs @@ -177,7 +177,7 @@ async fn handle_post( })?; relay_log::trace!("Checking request"); - let scoping = check_request(&state, meta, &headers, project).await?; + let scoping = validate_and_limit(&state, meta, &headers, project).await?; // Unconditionally create the upload location: relay_log::trace!("Creating upload location"); @@ -224,7 +224,7 @@ async fn handle_patch( })?; relay_log::trace!("Checking request"); - let scoping = validate_request(&state, meta, project).await?; + let scoping = validate(&state, meta, project).await?; let stream = body .into_data_stream() @@ -328,7 +328,7 @@ async fn upload( /// /// This is currently the easiest way to guarantee that the upload gets checked the same way as /// the envelope. -async fn check_request( +async fn validate_and_limit( state: &ServiceState, meta: RequestMeta, headers: &tus::Headers, @@ -360,7 +360,7 @@ async fn check_request( Ok(scoping) } -async fn validate_request( +async fn validate( state: &ServiceState, meta: RequestMeta, project: Project<'_>,