From f7bc6521ad54acb8f7b85ee9aa44ce5309bb30f7 Mon Sep 17 00:00:00 2001 From: Tal Weiss Date: Sun, 2 Aug 2026 22:17:07 +0200 Subject: [PATCH 1/4] fix(git): allow large smart HTTP pushes Signed-off-by: Tal Weiss --- crates/buzz-relay/src/api/git/transport.rs | 188 ++++++++++++++++++++- crates/buzz-test-client/tests/e2e_git.rs | 135 ++++++++++++++- 2 files changed, 318 insertions(+), 5 deletions(-) diff --git a/crates/buzz-relay/src/api/git/transport.rs b/crates/buzz-relay/src/api/git/transport.rs index 704bbf1c1d6..356d6724f2d 100644 --- a/crates/buzz-relay/src/api/git/transport.rs +++ b/crates/buzz-relay/src/api/git/transport.rs @@ -2108,6 +2108,74 @@ async fn finalize_push_inner( response } +/// Admit Git's unauthenticated four-byte receive-pack compatibility probe. +/// +/// Large smart-HTTP pushes first send a flush packet before Git retries with +/// the real, authenticated chunked pack. This middleware is mounted only on +/// the receive-pack POST route and deliberately matches the probe's complete +/// wire shape before returning a path-independent empty result. +async fn receive_pack_compatibility_probe( + request: axum::http::Request, + next: axum::middleware::Next, +) -> Response { + fn has_single_exact_header( + headers: &axum::http::HeaderMap, + name: &axum::http::HeaderName, + expected: &[u8], + ) -> bool { + let mut values = headers.get_all(name).iter(); + matches!(values.next(), Some(value) if value.as_bytes() == expected) + && values.next().is_none() + } + + let headers = request.headers(); + let is_candidate = request.method() == axum::http::Method::POST + && !headers.contains_key(header::AUTHORIZATION) + && !headers.contains_key(header::CONTENT_ENCODING) + && !headers.contains_key(header::TRANSFER_ENCODING) + && has_single_exact_header( + headers, + &header::CONTENT_TYPE, + b"application/x-git-receive-pack-request", + ) + && has_single_exact_header(headers, &header::CONTENT_LENGTH, b"4"); + + if !is_candidate { + return next.run(request).await; + } + + let (parts, body) = request.into_parts(); + match axum::body::to_bytes(body, 4).await { + Ok(bytes) if bytes.as_ref() == b"0000" => { + // Git sends this unauthenticated flush packet before a large, + // authenticated chunked receive-pack request. The response is + // deliberately path-independent: do not resolve a tenant, inspect + // repository state, hydrate, run Git, or mutate anything here. + let mut response = Response::new(Body::empty()); + response.headers_mut().insert( + header::CONTENT_TYPE, + axum::http::HeaderValue::from_static("application/x-git-receive-pack-result"), + ); + response.headers_mut().insert( + header::CONTENT_LENGTH, + axum::http::HeaderValue::from_static("0"), + ); + response + } + Ok(bytes) => { + next.run(axum::http::Request::from_parts(parts, Body::from(bytes))) + .await + } + Err(_) => { + // The declared four-byte body exceeded the hard collection limit + // or failed to stream. Preserve the normal authentication path; + // an unauthenticated request is rejected before its body is read. + next.run(axum::http::Request::from_parts(parts, Body::empty())) + .await + } + } +} + /// Build the git sub-router with its own body limit. /// /// Mounted at `/git/{owner}/{repo}/...` with a configurable max pack size. @@ -2117,7 +2185,11 @@ pub fn git_router(state: Arc) -> Router { Router::new() .route("/git/{owner}/{repo}/info/refs", get(info_refs)) .route("/git/{owner}/{repo}/git-upload-pack", post(upload_pack)) - .route("/git/{owner}/{repo}/git-receive-pack", post(receive_pack)) + .route( + "/git/{owner}/{repo}/git-receive-pack", + post(receive_pack) + .route_layer(axum::middleware::from_fn(receive_pack_compatibility_probe)), + ) .merge(super::settings::router()) .layer(RequestBodyLimitLayer::new(body_limit)) .with_state(state) @@ -2134,6 +2206,120 @@ mod track_c_tests { use std::io::Write; use std::process::Output; use tempfile::TempDir; + use tower::ServiceExt; + + fn receive_pack_probe_test_router() -> Router { + Router::new().route( + "/git/{owner}/{repo}/git-receive-pack", + post(|| async { + Response::builder() + .status(StatusCode::IM_A_TEAPOT) + .header("x-probe-next", "reached") + .body(Body::empty()) + .unwrap() + }) + .route_layer(axum::middleware::from_fn(receive_pack_compatibility_probe)), + ) + } + + fn receive_pack_probe_request( + uri: &str, + headers: &[(&str, &str)], + body: &'static [u8], + ) -> axum::http::Request { + let mut request = axum::http::Request::builder().method("POST").uri(uri); + for (name, value) in headers { + request = request.header(*name, *value); + } + request.body(Body::from(body)).unwrap() + } + + #[tokio::test] + async fn receive_pack_compatibility_probe_is_exact_and_path_independent() { + const CONTENT_TYPE: (&str, &str) = + ("content-type", "application/x-git-receive-pack-request"); + const CONTENT_LENGTH: (&str, &str) = ("content-length", "4"); + let exact_headers = [CONTENT_TYPE, CONTENT_LENGTH]; + + for uri in [ + "/git/alice/repo/git-receive-pack", + "/git/not-a-real-owner/not-a-real-repo/git-receive-pack", + ] { + let response = receive_pack_probe_test_router() + .oneshot(receive_pack_probe_request(uri, &exact_headers, b"0000")) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.headers().get(header::CONTENT_TYPE).unwrap(), + "application/x-git-receive-pack-result" + ); + assert_eq!(response.headers().get(header::CONTENT_LENGTH).unwrap(), "0"); + assert!(response.headers().get("x-probe-next").is_none()); + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + assert!(body.is_empty()); + } + + type ProbeCase = (Vec<(&'static str, &'static str)>, &'static [u8]); + let near_misses: Vec = vec![ + (exact_headers.to_vec(), b"0001"), + (exact_headers.to_vec(), b"00000"), + (vec![CONTENT_LENGTH], b"0000"), + (vec![CONTENT_TYPE], b"0000"), + ( + vec![ + ( + "content-type", + "application/x-git-rece-pack-request; charset=utf-8", + ), + CONTENT_LENGTH, + ], + b"0000", + ), + (vec![CONTENT_TYPE, ("content-length", "5")], b"0000"), + ( + vec![ + CONTENT_TYPE, + CONTENT_LENGTH, + ("authorization", "Nostr invalid-but-present"), + ], + b"0000", + ), + ( + vec![ + CONTENT_TYPE, + CONTENT_LENGTH, + ("content-encoding", "identity"), + ], + b"0000", + ), + ( + vec![ + CONTENT_TYPE, + CONTENT_LENGTH, + ("transfer-encoding", "chunked"), + ], + b"0000", + ), + (vec![CONTENT_TYPE, CONTENT_TYPE, CONTENT_LENGTH], b"0000"), + (vec![CONTENT_TYPE, CONTENT_LENGTH, CONTENT_LENGTH], b"0000"), + ]; + + for (headers, body) in near_misses { + let response = receive_pack_probe_test_router() + .oneshot(receive_pack_probe_request( + "/git/alice/repo/git-receive-pack", + &headers, + body, + )) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::IM_A_TEAPOT); + assert_eq!(response.headers().get("x-probe-next").unwrap(), "reached"); + } + } fn oid_sha1() -> String { "cb09a769da1c01f458fa6959d4e8eded38fac8d3".to_string() diff --git a/crates/buzz-test-client/tests/e2e_git.rs b/crates/buzz-test-client/tests/e2e_git.rs index 3c82e317649..2f59371f372 100644 --- a/crates/buzz-test-client/tests/e2e_git.rs +++ b/crates/buzz-test-client/tests/e2e_git.rs @@ -85,8 +85,19 @@ async fn create_test_channel(keys: &Keys) -> String { /// Run `git` with the Buzz credential helper and isolated config. fn git_status(args: &[&str], cwd: &Path, owner_nsec: &str) -> std::process::Output { + git_status_with_env(args, cwd, owner_nsec, &[]) +} + +/// Run isolated `git` with additional process-scoped environment variables. +fn git_status_with_env( + args: &[&str], + cwd: &Path, + owner_nsec: &str, + extra_env: &[(&str, &str)], +) -> std::process::Output { let helper = credential_helper(); - Command::new("git") + let mut command = Command::new("git"); + command .args([ "-c", "credential.useHttpPath=true", @@ -107,9 +118,11 @@ fn git_status(args: &[&str], cwd: &Path, owner_nsec: &str) -> std::process::Outp .env("GIT_CONFIG_GLOBAL", "/dev/null") .env("GIT_CONFIG_NOSYSTEM", "1") .env_remove("GIT_CONFIG_COUNT") - .env("NOSTR_PRIVATE_KEY", owner_nsec) - .output() - .expect("spawn git") + .env("NOSTR_PRIVATE_KEY", owner_nsec); + for (key, value) in extra_env { + command.env(key, value); + } + command.output().expect("spawn git") } /// Run `git` with the Buzz credential helper and isolated config. Asserts the @@ -408,6 +421,120 @@ async fn git_clone_push_fetch_force_roundtrip() { assert!(tags.contains("v1.0"), "tag v1.0 cloned back: {tags}"); } +/// Git switches to its large-request smart-HTTP sequence only when the pack +/// exceeds `http.postBuffer`. Compressible text fixtures do not exercise that +/// path, so generate deterministic pseudo-random bytes and verify Git stores +/// more than one MiB of compressed loose objects before pushing. +#[tokio::test] +#[ignore = "requires live relay + MinIO + git"] +async fn git_large_push_crosses_post_buffer_and_roundtrips() { + use nostr::ToBech32; + + let owner = Keys::generate(); + let owner_hex = owner.public_key().to_hex(); + let owner_nsec = owner.secret_key().to_bech32().unwrap(); + let repo = format!("e2e-git-large-{}", uuid::Uuid::new_v4().simple()); + let s3 = GitS3Probe::from_env(); + + let channel = create_test_channel(&owner).await; + let announce = EventBuilder::new(Kind::from(30617), "") + .tags(vec![ + Tag::parse(["d", &repo]).unwrap(), + Tag::parse(["name", "e2e large-push git repo"]).unwrap(), + Tag::parse(["buzz-channel", &channel]).unwrap(), + ]) + .sign_with_keys(&owner) + .unwrap(); + post_event(&announce).await; + tokio::time::sleep(Duration::from_secs(2)).await; + + let tmp = tempdir_named("buzz-e2e-git-large"); + let url = format!("{}/git/{}/{}", relay_http_url(), owner_hex, repo); + git( + &["clone", "--quiet", &url, "source"], + tmp.path(), + &owner_nsec, + ); + let source = tmp.path().join("source"); + let empty_pointer = s3.require_pointer(&owner_hex, &repo).await; + + let mut state = 0x4d59_5df4_d0f3_3173u64; + let mut large_blob = vec![0u8; 2 * 1024 * 1024]; + for byte in &mut large_blob { + // xorshift64* is deterministic but sufficiently noise-like that Git's + // zlib compression cannot shrink this fixture below the one-MiB gate. + state ^= state >> 12; + state ^= state << 25; + state ^= state >> 27; + *byte = state.wrapping_mul(0x2545_f491_4f6c_dd1d).to_le_bytes()[0]; + } + std::fs::write(source.join("large.bin"), &large_blob).unwrap(); + git(&["add", "large.bin"], &source, &owner_nsec); + git( + &["commit", "--quiet", "-m", "large incompressible fixture"], + &source, + &owner_nsec, + ); + git(&["branch", "-M", "main"], &source, &owner_nsec); + + let count = git(&["count-objects", "-v"], &source, &owner_nsec); + let loose_kib = count + .lines() + .find_map(|line| line.strip_prefix("size: ")) + .and_then(|value| value.parse::().ok()) + .expect("git count-objects reports loose-object size"); + assert!( + loose_kib > 1024, + "fixture must remain above the 1 MiB postBuffer after compression; got {loose_kib} KiB" + ); + + // Pin the threshold rather than relying on a machine's Git defaults. This + // forces the unauthenticated four-byte probe followed by the authenticated + // chunked receive-pack request that regressed in issue #2880. + let push = git_status_with_env( + &[ + "-c", + "http.postBuffer=1048576", + "push", + "--quiet", + "origin", + "main", + ], + &source, + &owner_nsec, + &[("GIT_TRACE_CURL", "1"), ("GIT_TRACE_CURL_NO_DATA", "1")], + ); + let trace = String::from_utf8_lossy(&push.stderr).to_lowercase(); + assert!( + push.status.success(), + "large push failed (authorization-bearing trace omitted)" + ); + assert!( + trace.contains("content-length: 4"), + "push did not exercise Git's four-byte compatibility probe" + ); + assert!( + trace.contains("transfer-encoding: chunked"), + "push did not follow the probe with a chunked pack request" + ); + let published = s3.require_pointer(&owner_hex, &repo).await; + assert_ne!( + published, empty_pointer, + "large push must advance the S3 manifest pointer" + ); + + git( + &["clone", "--quiet", &url, "verify"], + tmp.path(), + &owner_nsec, + ); + let roundtripped = std::fs::read(tmp.path().join("verify/large.bin")).unwrap(); + assert_eq!( + roundtripped, large_blob, + "large blob roundtrips byte-for-byte" + ); +} + #[tokio::test] #[ignore = "requires live relay + MinIO + git"] async fn git_concurrent_push_one_wins_and_repo_recovers() { From d7b7ab9169e4f365b9e333ca582176092ec04163 Mon Sep 17 00:00:00 2001 From: Tal Weiss Date: Sun, 2 Aug 2026 20:22:28 -0400 Subject: [PATCH 2/4] fix(git): admit the compatibility probe on upload-pack too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Git sends the same unauthenticated four-byte flush-packet probe before any smart HTTP POST whose body exceeds http.postBuffer — not just large pushes. Fetch negotiations cross the 1 MiB decoded threshold when the client's divergent local history contributes enough have lines (an in-limit repo plus ~21k unpushed divergent branches suffices; measured live at MAX_MANIFEST_REFS = 10_000 with divergent client history: 1.46 MiB decoded negotiation, probe fired, fetch got 401). Parameterize the receive-pack probe middleware over the service's (request MIME, result MIME) pair — the predicate was already otherwise service-agnostic — and mount it on the upload-pack POST route as well. All other predicate legs are unchanged: no Authorization / Content-Encoding / Transfer-Encoding headers, a single exact Content-Length of 4, and an exact 0000 body; near misses fall through to the normal authenticated path. Tests: the existing exactness/path-independence matrix now runs against both services (2 path shapes + 11 near-misses each), plus a cross-service case proving each route rejects the other service's request MIME. Fixes #4423 Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: Tal Weiss --- crates/buzz-relay/src/api/git/transport.rs | 269 ++++++++++++++------- 1 file changed, 175 insertions(+), 94 deletions(-) diff --git a/crates/buzz-relay/src/api/git/transport.rs b/crates/buzz-relay/src/api/git/transport.rs index 356d6724f2d..245cafd9b29 100644 --- a/crates/buzz-relay/src/api/git/transport.rs +++ b/crates/buzz-relay/src/api/git/transport.rs @@ -2108,13 +2108,20 @@ async fn finalize_push_inner( response } -/// Admit Git's unauthenticated four-byte receive-pack compatibility probe. +/// Admit Git's unauthenticated four-byte compatibility probe for a smart +/// HTTP service. /// -/// Large smart-HTTP pushes first send a flush packet before Git retries with -/// the real, authenticated chunked pack. This middleware is mounted only on -/// the receive-pack POST route and deliberately matches the probe's complete -/// wire shape before returning a path-independent empty result. -async fn receive_pack_compatibility_probe( +/// Before a request whose body exceeds `http.postBuffer` (large chunked +/// pushes, and fetch negotiations whose decoded `want`/`have` list crosses +/// 1 MiB), Git first sends an unauthenticated flush packet and only retries +/// with the real, authenticated request after the probe succeeds. This +/// middleware is shared by the receive-pack and upload-pack POST routes — +/// parameterized over the service's request/result MIME pair, with every +/// other predicate leg identical — and deliberately matches the probe's +/// complete wire shape before returning a path-independent empty result. +async fn git_compatibility_probe( + request_mime: &'static [u8], + result_mime: &'static str, request: axum::http::Request, next: axum::middleware::Next, ) -> Response { @@ -2133,11 +2140,7 @@ async fn receive_pack_compatibility_probe( && !headers.contains_key(header::AUTHORIZATION) && !headers.contains_key(header::CONTENT_ENCODING) && !headers.contains_key(header::TRANSFER_ENCODING) - && has_single_exact_header( - headers, - &header::CONTENT_TYPE, - b"application/x-git-receive-pack-request", - ) + && has_single_exact_header(headers, &header::CONTENT_TYPE, request_mime) && has_single_exact_header(headers, &header::CONTENT_LENGTH, b"4"); if !is_candidate { @@ -2148,13 +2151,13 @@ async fn receive_pack_compatibility_probe( match axum::body::to_bytes(body, 4).await { Ok(bytes) if bytes.as_ref() == b"0000" => { // Git sends this unauthenticated flush packet before a large, - // authenticated chunked receive-pack request. The response is - // deliberately path-independent: do not resolve a tenant, inspect - // repository state, hydrate, run Git, or mutate anything here. + // authenticated chunked request. The response is deliberately + // path-independent: do not resolve a tenant, inspect repository + // state, hydrate, run Git, or mutate anything here. let mut response = Response::new(Body::empty()); response.headers_mut().insert( header::CONTENT_TYPE, - axum::http::HeaderValue::from_static("application/x-git-receive-pack-result"), + axum::http::HeaderValue::from_static(result_mime), ); response.headers_mut().insert( header::CONTENT_LENGTH, @@ -2176,6 +2179,18 @@ async fn receive_pack_compatibility_probe( } } +/// Exact `Content-Type` Git sends for each smart HTTP service's POST body, +/// and the matching result MIME the probe response must carry. Paired per +/// service so the two probe mounts below cannot mix request/result MIMEs. +const RECEIVE_PACK_PROBE_MIMES: (&[u8], &str) = ( + b"application/x-git-receive-pack-request", + "application/x-git-receive-pack-result", +); +const UPLOAD_PACK_PROBE_MIMES: (&[u8], &str) = ( + b"application/x-git-upload-pack-request", + "application/x-git-upload-pack-result", +); + /// Build the git sub-router with its own body limit. /// /// Mounted at `/git/{owner}/{repo}/...` with a configurable max pack size. @@ -2184,11 +2199,19 @@ pub fn git_router(state: Arc) -> Router { Router::new() .route("/git/{owner}/{repo}/info/refs", get(info_refs)) - .route("/git/{owner}/{repo}/git-upload-pack", post(upload_pack)) + .route( + "/git/{owner}/{repo}/git-upload-pack", + post(upload_pack).route_layer(axum::middleware::from_fn(|request, next| { + let (request_mime, result_mime) = UPLOAD_PACK_PROBE_MIMES; + git_compatibility_probe(request_mime, result_mime, request, next) + })), + ) .route( "/git/{owner}/{repo}/git-receive-pack", - post(receive_pack) - .route_layer(axum::middleware::from_fn(receive_pack_compatibility_probe)), + post(receive_pack).route_layer(axum::middleware::from_fn(|request, next| { + let (request_mime, result_mime) = RECEIVE_PACK_PROBE_MIMES; + git_compatibility_probe(request_mime, result_mime, request, next) + })), ) .merge(super::settings::router()) .layer(RequestBodyLimitLayer::new(body_limit)) @@ -2208,9 +2231,15 @@ mod track_c_tests { use tempfile::TempDir; use tower::ServiceExt; - fn receive_pack_probe_test_router() -> Router { + /// A probe test router mounting `git_compatibility_probe` for one service + /// on its real route path, exactly as `git_router()` mounts it. The inner + /// handler marks near-miss fallthrough with `x-probe-next: reached`. + fn git_probe_test_router( + route: &str, + (request_mime, result_mime): (&'static [u8], &'static str), + ) -> Router { Router::new().route( - "/git/{owner}/{repo}/git-receive-pack", + route, post(|| async { Response::builder() .status(StatusCode::IM_A_TEAPOT) @@ -2218,11 +2247,13 @@ mod track_c_tests { .body(Body::empty()) .unwrap() }) - .route_layer(axum::middleware::from_fn(receive_pack_compatibility_probe)), + .route_layer(axum::middleware::from_fn(move |request, next| { + git_compatibility_probe(request_mime, result_mime, request, next) + })), ) } - fn receive_pack_probe_request( + fn git_probe_request( uri: &str, headers: &[(&str, &str)], body: &'static [u8], @@ -2235,88 +2266,138 @@ mod track_c_tests { } #[tokio::test] - async fn receive_pack_compatibility_probe_is_exact_and_path_independent() { - const CONTENT_TYPE: (&str, &str) = - ("content-type", "application/x-git-receive-pack-request"); - const CONTENT_LENGTH: (&str, &str) = ("content-length", "4"); - let exact_headers = [CONTENT_TYPE, CONTENT_LENGTH]; - - for uri in [ - "/git/alice/repo/git-receive-pack", - "/git/not-a-real-owner/not-a-real-repo/git-receive-pack", - ] { - let response = receive_pack_probe_test_router() - .oneshot(receive_pack_probe_request(uri, &exact_headers, b"0000")) - .await - .unwrap(); - assert_eq!(response.status(), StatusCode::OK); - assert_eq!( - response.headers().get(header::CONTENT_TYPE).unwrap(), - "application/x-git-receive-pack-result" - ); - assert_eq!(response.headers().get(header::CONTENT_LENGTH).unwrap(), "0"); - assert!(response.headers().get("x-probe-next").is_none()); - let body = axum::body::to_bytes(response.into_body(), usize::MAX) - .await - .unwrap(); - assert!(body.is_empty()); - } - - type ProbeCase = (Vec<(&'static str, &'static str)>, &'static [u8]); - let near_misses: Vec = vec![ - (exact_headers.to_vec(), b"0001"), - (exact_headers.to_vec(), b"00000"), - (vec![CONTENT_LENGTH], b"0000"), - (vec![CONTENT_TYPE], b"0000"), + async fn git_compatibility_probe_is_exact_and_path_independent() { + for (route_template, mimes) in [ ( - vec![ - ( - "content-type", - "application/x-git-rece-pack-request; charset=utf-8", - ), - CONTENT_LENGTH, - ], - b"0000", + "/git/{owner}/{repo}/git-receive-pack", + RECEIVE_PACK_PROBE_MIMES, ), - (vec![CONTENT_TYPE, ("content-length", "5")], b"0000"), ( - vec![ - CONTENT_TYPE, - CONTENT_LENGTH, - ("authorization", "Nostr invalid-but-present"), - ], - b"0000", + "/git/{owner}/{repo}/git-upload-pack", + UPLOAD_PACK_PROBE_MIMES, ), + ] { + let (request_mime, result_mime) = mimes; + let request_mime_str = std::str::from_utf8(request_mime).unwrap(); + let content_type: (&str, &str) = ("content-type", request_mime_str); + let content_length: (&str, &str) = ("content-length", "4"); + let exact_headers = [content_type, content_length]; + let service_suffix = route_template.rsplit('/').next().unwrap(); + + for uri in [ + format!("/git/alice/repo/{service_suffix}"), + format!("/git/not-a-real-owner/not-a-real-repo/{service_suffix}"), + ] { + let response = git_probe_test_router(route_template, mimes) + .oneshot(git_probe_request(&uri, &exact_headers, b"0000")) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK, "{service_suffix} {uri}"); + assert_eq!( + response.headers().get(header::CONTENT_TYPE).unwrap(), + result_mime, + "{service_suffix}" + ); + assert_eq!(response.headers().get(header::CONTENT_LENGTH).unwrap(), "0"); + assert!(response.headers().get("x-probe-next").is_none()); + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + assert!(body.is_empty()); + } + + // A truncated MIME near-miss for this service: drop the last + // character of the request MIME and append a charset parameter. + let truncated_mime = format!( + "{}; charset=utf-8", + &request_mime_str[..request_mime_str.len() - 1] + ); + + type ProbeCase<'a> = (Vec<(&'a str, &'a str)>, &'static [u8]); + let near_misses: Vec = vec![ + (exact_headers.to_vec(), b"0001"), + (exact_headers.to_vec(), b"00000"), + (vec![content_length], b"0000"), + (vec![content_type], b"0000"), + ( + vec![("content-type", truncated_mime.as_str()), content_length], + b"0000", + ), + (vec![content_type, ("content-length", "5")], b"0000"), + ( + vec![ + content_type, + content_length, + ("authorization", "Nostr invalid-but-present"), + ], + b"0000", + ), + ( + vec![ + content_type, + content_length, + ("content-encoding", "identity"), + ], + b"0000", + ), + ( + vec![ + content_type, + content_length, + ("transfer-encoding", "chunked"), + ], + b"0000", + ), + (vec![content_type, content_type, content_length], b"0000"), + (vec![content_type, content_length, content_length], b"0000"), + ]; + + for (headers, body) in near_misses { + let uri = format!("/git/alice/repo/{service_suffix}"); + let response = git_probe_test_router(route_template, mimes) + .oneshot(git_probe_request(&uri, &headers, body)) + .await + .unwrap(); + assert_eq!( + response.status(), + StatusCode::IM_A_TEAPOT, + "{service_suffix} near-miss {headers:?}" + ); + assert_eq!(response.headers().get("x-probe-next").unwrap(), "reached"); + } + } + } + + #[tokio::test] + async fn git_compatibility_probe_rejects_cross_service_mime() { + // The other service's request MIME is a well-formed Git MIME but the + // wrong one for this route: it must fall through to the handler, not + // be admitted as a probe. + for (route_template, mimes, wrong_mime) in [ ( - vec![ - CONTENT_TYPE, - CONTENT_LENGTH, - ("content-encoding", "identity"), - ], - b"0000", + "/git/{owner}/{repo}/git-receive-pack", + RECEIVE_PACK_PROBE_MIMES, + UPLOAD_PACK_PROBE_MIMES.0, ), ( - vec![ - CONTENT_TYPE, - CONTENT_LENGTH, - ("transfer-encoding", "chunked"), - ], - b"0000", + "/git/{owner}/{repo}/git-upload-pack", + UPLOAD_PACK_PROBE_MIMES, + RECEIVE_PACK_PROBE_MIMES.0, ), - (vec![CONTENT_TYPE, CONTENT_TYPE, CONTENT_LENGTH], b"0000"), - (vec![CONTENT_TYPE, CONTENT_LENGTH, CONTENT_LENGTH], b"0000"), - ]; - - for (headers, body) in near_misses { - let response = receive_pack_probe_test_router() - .oneshot(receive_pack_probe_request( - "/git/alice/repo/git-receive-pack", - &headers, - body, - )) + ] { + let service_suffix = route_template.rsplit('/').next().unwrap(); + let uri = format!("/git/alice/repo/{service_suffix}"); + let wrong_mime_str = std::str::from_utf8(wrong_mime).unwrap(); + let headers = [("content-type", wrong_mime_str), ("content-length", "4")]; + let response = git_probe_test_router(route_template, mimes) + .oneshot(git_probe_request(&uri, &headers, b"0000")) .await .unwrap(); - assert_eq!(response.status(), StatusCode::IM_A_TEAPOT); + assert_eq!( + response.status(), + StatusCode::IM_A_TEAPOT, + "{service_suffix} must not admit {wrong_mime_str}" + ); assert_eq!(response.headers().get("x-probe-next").unwrap(), "reached"); } } From 9fcf114e0f79e8a08cc420205090876579dd21af Mon Sep 17 00:00:00 2001 From: Tal Weiss Date: Sun, 2 Aug 2026 20:22:28 -0400 Subject: [PATCH 3/4] fix(git): bound the compatibility probe's body read Admitting the 0000 probe means reading four unauthenticated body bytes. Give that read its own 60 s deadline that fails closed into the normal authenticated path, so a client that sends the probe headers and then withholds the body cannot park a task. The wider request-time policy for the API and media routers is a separate change (#4424). Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Co-Authored-By: Claude Fable 5.1 Signed-off-by: Tal Weiss --- crates/buzz-relay/src/api/git/transport.rs | 180 +++++++++++++++++++-- 1 file changed, 168 insertions(+), 12 deletions(-) diff --git a/crates/buzz-relay/src/api/git/transport.rs b/crates/buzz-relay/src/api/git/transport.rs index 245cafd9b29..942e0ca64b9 100644 --- a/crates/buzz-relay/src/api/git/transport.rs +++ b/crates/buzz-relay/src/api/git/transport.rs @@ -2108,6 +2108,21 @@ async fn finalize_push_inner( response } +/// Deadline for collecting the four-byte compatibility-probe body. +/// +/// The probe middleware reads the request body *before* authentication, so a +/// client that presents the exact probe headers and then withholds the body +/// would otherwise park an unauthenticated task indefinitely. On expiry the +/// middleware fails closed into the normal authentication path, which rejects +/// the unauthenticated request without reading a body. +/// +/// Git and curl write the probe headers and the `0000` body in the same +/// segment, so a real probe never waits on this; 10s only bounds how long a +/// forged probe can hold a task. It is tighter than its surroundings: the +/// relay's `axum::serve` installs no hyper timer, so the header phase of an +/// unauthenticated connection has no deadline at all today (#4424). +const PROBE_BODY_TIMEOUT: Duration = Duration::from_secs(10); + /// Admit Git's unauthenticated four-byte compatibility probe for a smart /// HTTP service. /// @@ -2122,6 +2137,7 @@ async fn finalize_push_inner( async fn git_compatibility_probe( request_mime: &'static [u8], result_mime: &'static str, + body_timeout: Duration, request: axum::http::Request, next: axum::middleware::Next, ) -> Response { @@ -2148,8 +2164,13 @@ async fn git_compatibility_probe( } let (parts, body) = request.into_parts(); - match axum::body::to_bytes(body, 4).await { - Ok(bytes) if bytes.as_ref() == b"0000" => { + // Bound the pre-auth body collection: a candidate that withholds its + // declared four bytes must not park an unauthenticated task. On expiry, + // fall through to the normal authentication path exactly like a stream + // failure below — the unauthenticated request is rejected without its + // body ever being read. + match tokio::time::timeout(body_timeout, axum::body::to_bytes(body, 4)).await { + Ok(Ok(bytes)) if bytes.as_ref() == b"0000" => { // Git sends this unauthenticated flush packet before a large, // authenticated chunked request. The response is deliberately // path-independent: do not resolve a tenant, inspect repository @@ -2165,14 +2186,21 @@ async fn git_compatibility_probe( ); response } - Ok(bytes) => { + Ok(Ok(bytes)) => { next.run(axum::http::Request::from_parts(parts, Body::from(bytes))) .await } - Err(_) => { - // The declared four-byte body exceeded the hard collection limit - // or failed to stream. Preserve the normal authentication path; - // an unauthenticated request is rejected before its body is read. + Ok(Err(_)) | Err(_) => { + // The declared four-byte body exceeded the hard collection limit, + // failed to stream, or was withheld past the collection deadline. + // Preserve the normal authentication path; an unauthenticated + // request is rejected before its body is read. That is what makes + // the empty substitute body safe: `upload_pack` and `receive_pack` + // both extract `GitAuth` from the request parts before touching + // the body, and a candidate by definition carries no + // `Authorization`, so it is answered 401 without decoding. If + // either service ever admits anonymous requests, this branch must + // answer 401 itself instead of falling through. next.run(axum::http::Request::from_parts(parts, Body::empty())) .await } @@ -2203,14 +2231,26 @@ pub fn git_router(state: Arc) -> Router { "/git/{owner}/{repo}/git-upload-pack", post(upload_pack).route_layer(axum::middleware::from_fn(|request, next| { let (request_mime, result_mime) = UPLOAD_PACK_PROBE_MIMES; - git_compatibility_probe(request_mime, result_mime, request, next) + git_compatibility_probe( + request_mime, + result_mime, + PROBE_BODY_TIMEOUT, + request, + next, + ) })), ) .route( "/git/{owner}/{repo}/git-receive-pack", post(receive_pack).route_layer(axum::middleware::from_fn(|request, next| { let (request_mime, result_mime) = RECEIVE_PACK_PROBE_MIMES; - git_compatibility_probe(request_mime, result_mime, request, next) + git_compatibility_probe( + request_mime, + result_mime, + PROBE_BODY_TIMEOUT, + request, + next, + ) })), ) .merge(super::settings::router()) @@ -2233,10 +2273,13 @@ mod track_c_tests { /// A probe test router mounting `git_compatibility_probe` for one service /// on its real route path, exactly as `git_router()` mounts it. The inner - /// handler marks near-miss fallthrough with `x-probe-next: reached`. - fn git_probe_test_router( + /// handler marks near-miss fallthrough with `x-probe-next: reached`. The + /// body-collection timeout is injectable so stall tests do not sleep for + /// the production constant. + fn git_probe_test_router_with_timeout( route: &str, (request_mime, result_mime): (&'static [u8], &'static str), + body_timeout: Duration, ) -> Router { Router::new().route( route, @@ -2248,11 +2291,15 @@ mod track_c_tests { .unwrap() }) .route_layer(axum::middleware::from_fn(move |request, next| { - git_compatibility_probe(request_mime, result_mime, request, next) + git_compatibility_probe(request_mime, result_mime, body_timeout, request, next) })), ) } + fn git_probe_test_router(route: &str, mimes: (&'static [u8], &'static str)) -> Router { + git_probe_test_router_with_timeout(route, mimes, PROBE_BODY_TIMEOUT) + } + fn git_probe_request( uri: &str, headers: &[(&str, &str)], @@ -2402,6 +2449,115 @@ mod track_c_tests { } } + /// A request body that never yields its declared bytes — the wire shape + /// of a client that sends the exact probe headers and then withholds the + /// four-byte body forever. + fn stalled_probe_body() -> Body { + Body::from_stream(futures_util::stream::pending::< + Result, + >()) + } + + #[tokio::test] + async fn git_compatibility_probe_candidate_with_withheld_body_fails_closed_in_time() { + // An exact probe candidate that never sends its body must be bounded + // by the collection deadline and fail closed into the normal path + // (the teapot handler here; authentication in production) instead of + // parking an unauthenticated task forever. + for (route_template, mimes) in [ + ( + "/git/{owner}/{repo}/git-receive-pack", + RECEIVE_PACK_PROBE_MIMES, + ), + ( + "/git/{owner}/{repo}/git-upload-pack", + UPLOAD_PACK_PROBE_MIMES, + ), + ] { + let service_suffix = route_template.rsplit('/').next().unwrap(); + let request_mime_str = std::str::from_utf8(mimes.0).unwrap(); + let request = axum::http::Request::builder() + .method("POST") + .uri(format!("/git/alice/repo/{service_suffix}")) + .header("content-type", request_mime_str) + .header("content-length", "4") + .body(stalled_probe_body()) + .unwrap(); + + let started = std::time::Instant::now(); + let response = tokio::time::timeout( + Duration::from_secs(5), + git_probe_test_router_with_timeout( + route_template, + mimes, + Duration::from_millis(50), + ) + .oneshot(request), + ) + .await + .expect("stalled probe candidate must be bounded by the collection deadline") + .unwrap(); + + assert!( + started.elapsed() < Duration::from_secs(5), + "{service_suffix}: deadline must fire at the configured bound" + ); + assert_eq!( + response.status(), + StatusCode::IM_A_TEAPOT, + "{service_suffix}: expiry must fail closed into the normal path" + ); + assert_eq!(response.headers().get("x-probe-next").unwrap(), "reached"); + } + } + + #[tokio::test] + async fn git_compatibility_probe_dropped_stalled_future_never_reaches_handler() { + let reached = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let reached_flag = reached.clone(); + let (request_mime, result_mime) = RECEIVE_PACK_PROBE_MIMES; + let router = Router::new().route( + "/git/{owner}/{repo}/git-receive-pack", + post(move || { + let reached = reached_flag.clone(); + async move { + reached.store(true, std::sync::atomic::Ordering::SeqCst); + StatusCode::OK + } + }) + .route_layer(axum::middleware::from_fn(move |request, next| { + git_compatibility_probe( + request_mime, + result_mime, + Duration::from_secs(60), + request, + next, + ) + })), + ); + + let request = axum::http::Request::builder() + .method("POST") + .uri("/git/alice/repo/git-receive-pack") + .header("content-type", std::str::from_utf8(request_mime).unwrap()) + .header("content-length", "4") + .body(stalled_probe_body()) + .unwrap(); + + let response_future = router.oneshot(request); + tokio::select! { + _ = response_future => panic!("stalled probe candidate must not produce a response yet"), + _ = tokio::time::sleep(Duration::from_millis(50)) => {} + } + // The future was dropped by the select while parked on body + // collection; the inner handler must never run afterwards. + tokio::time::sleep(Duration::from_millis(100)).await; + assert!( + !reached.load(std::sync::atomic::Ordering::SeqCst), + "dropped stalled probe future must never invoke the handler" + ); + } + fn oid_sha1() -> String { "cb09a769da1c01f458fa6959d4e8eded38fac8d3".to_string() } From 8e518101250fc06e800b52fb60b17018e6e5c047 Mon Sep 17 00:00:00 2001 From: Tal Weiss Date: Sat, 19 Sep 2026 14:18:13 +0200 Subject: [PATCH 4/4] ci(unit): run the git probe regression tests in the unit lane The unit lane selects buzz-relay tests by explicit nextest expression, and none of its terms matched api::git::transport::track_c_tests, so the four compatibility-probe tests were compiled by clippy but executed by no hosted job. They need no database or storage, so they belong in the infra-free lane. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Tal Weiss --- Justfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Justfile b/Justfile index 3f5bec95a64..2d841b4336d 100644 --- a/Justfile +++ b/Justfile @@ -457,7 +457,7 @@ test-unit: # the ~30s sqlx acquire timeout, so they do not belong in the infra-free # unit job either. cargo nextest run -p buzz-relay --lib \ - -E '(test(/^api::admin::/) - test(=api::admin::tests::disabled_mode_allows_unauthenticated_requests_on_the_admin_host) - test(=api::admin::tests::nip98_mode_unrostered_signer_does_not_consume_a_replay_slot)) + test(/^handlers::channel_authz::/) + test(/^handlers::moderation_authz::/) + test(/^handlers::side_effects::tests::/) + test(/^storage_sweep::tests::/)' + -E '(test(/^api::admin::/) - test(=api::admin::tests::disabled_mode_allows_unauthenticated_requests_on_the_admin_host) - test(=api::admin::tests::nip98_mode_unrostered_signer_does_not_consume_a_replay_slot)) + test(/^handlers::channel_authz::/) + test(/^handlers::moderation_authz::/) + test(/^handlers::side_effects::tests::/) + test(/^storage_sweep::tests::/) + test(/^api::git::transport::track_c_tests::git_compatibility_probe_/)' # ACP author-gate and queue tests protect the trust boundary between # relay events and agent prompts. They are infra-free; ignored lifecycle # tests remain excluded and run in their dedicated integration lanes.