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. diff --git a/crates/buzz-relay/src/api/git/transport.rs b/crates/buzz-relay/src/api/git/transport.rs index 704bbf1c1d6..942e0ca64b9 100644 --- a/crates/buzz-relay/src/api/git/transport.rs +++ b/crates/buzz-relay/src/api/git/transport.rs @@ -2108,6 +2108,117 @@ 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. +/// +/// 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, + body_timeout: Duration, + 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, request_mime) + && has_single_exact_header(headers, &header::CONTENT_LENGTH, b"4"); + + if !is_candidate { + return next.run(request).await; + } + + let (parts, body) = request.into_parts(); + // 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 + // 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(result_mime), + ); + response.headers_mut().insert( + header::CONTENT_LENGTH, + axum::http::HeaderValue::from_static("0"), + ); + response + } + Ok(Ok(bytes)) => { + next.run(axum::http::Request::from_parts(parts, Body::from(bytes))) + .await + } + 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 + } + } +} + +/// 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. @@ -2116,8 +2227,32 @@ 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-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, + 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, + PROBE_BODY_TIMEOUT, + request, + next, + ) + })), + ) .merge(super::settings::router()) .layer(RequestBodyLimitLayer::new(body_limit)) .with_state(state) @@ -2134,6 +2269,294 @@ mod track_c_tests { use std::io::Write; use std::process::Output; use tempfile::TempDir; + use tower::ServiceExt; + + /// 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`. 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, + post(|| async { + Response::builder() + .status(StatusCode::IM_A_TEAPOT) + .header("x-probe-next", "reached") + .body(Body::empty()) + .unwrap() + }) + .route_layer(axum::middleware::from_fn(move |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)], + 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 git_compatibility_probe_is_exact_and_path_independent() { + 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 (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 [ + ( + "/git/{owner}/{repo}/git-receive-pack", + RECEIVE_PACK_PROBE_MIMES, + UPLOAD_PACK_PROBE_MIMES.0, + ), + ( + "/git/{owner}/{repo}/git-upload-pack", + UPLOAD_PACK_PROBE_MIMES, + RECEIVE_PACK_PROBE_MIMES.0, + ), + ] { + 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, + "{service_suffix} must not admit {wrong_mime_str}" + ); + assert_eq!(response.headers().get("x-probe-next").unwrap(), "reached"); + } + } + + /// 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() 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() {