From eb76c819095712f583fad3c2c07c81cdb7c6a190 Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Sat, 27 Jun 2026 11:52:18 -0400 Subject: [PATCH 1/2] feat(relay): route git pre-receive policy callback over UDS when configured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-receive hook curls the relay's own /internal/git/policy callback. By default it targets the TCP loopback 127.0.0.1:{bind_port}. In a mesh deployment (e.g. Istio) that loopback port can be iptables-captured by the sidecar, so the relay's call to itself fails with 'network error reaching policy service' and every push is rejected. The relay already serves the same router over its UDS listener (main.rs) when BUZZ_UDS_PATH is set, so the policy endpoint is already reachable over the socket — only the hook's curl target needs to change. - transport.rs: when config.uds_path is set, point BUZZ_HOOK_URL at a placeholder host (curl ignores host:port under --unix-socket) and pass the socket path via BUZZ_HOOK_SOCKET. Extract policy_hook_url() as a pure, unit-testable helper. When uds_path is None, behavior is byte-identical to before (TCP loopback) — local/dev/CI are untouched. - hook.rs: add --unix-socket "$BUZZ_HOOK_SOCKET" to the policy curl iff BUZZ_HOOK_SOCKET is non-empty. --unix-socket is stock libcurl. - Unit tests for both URL branches. Pairs with a tf-stacks chart change that sets BUZZ_UDS_PATH + a shared emptyDir mount; that removes the relay's dependency on the mesh loopback entirely. Refs RESEARCH/SPROUT_GIT_POLICY_DEPLOY_MISMATCH.md. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- crates/buzz-relay/src/api/git/hook.rs | 10 ++++ crates/buzz-relay/src/api/git/transport.rs | 65 ++++++++++++++++++++-- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/crates/buzz-relay/src/api/git/hook.rs b/crates/buzz-relay/src/api/git/hook.rs index 8b0136661..9424e4464 100644 --- a/crates/buzz-relay/src/api/git/hook.rs +++ b/crates/buzz-relay/src/api/git/hook.rs @@ -122,7 +122,17 @@ fi SAFE_REPO_ID=$(printf '%s' "$BUZZ_REPO_ID" | sed 's/\\/\\\\/g; s/"/\\"/g') BODY="{\"repo_id\":\"${SAFE_REPO_ID}\",\"repo_owner\":\"${BUZZ_REPO_OWNER}\",\"pusher_pubkey\":\"${BUZZ_PUSHER_PUBKEY}\",\"ref_updates\":[${REFS}],\"timestamp\":${TIMESTAMP},\"signature\":\"${SIGNATURE}\"}" +# Route the policy callback over a unix socket when the relay configured one +# (BUZZ_HOOK_SOCKET). This lets the hook reach the relay's own endpoint even +# when the loopback TCP port is intercepted in-pod (e.g. by an Istio sidecar). +# When unset/empty, fall through to the TCP URL exactly as before. +CURL_TRANSPORT=() +if [ -n "${BUZZ_HOOK_SOCKET:-}" ]; then + CURL_TRANSPORT=(--unix-socket "$BUZZ_HOOK_SOCKET") +fi + HTTP_CODE=$(curl --silent --max-time 10 \ + "${CURL_TRANSPORT[@]}" \ -o "$RESP_FILE" \ -w "%{http_code}" \ -X POST \ diff --git a/crates/buzz-relay/src/api/git/transport.rs b/crates/buzz-relay/src/api/git/transport.rs index 02cd71ca6..3abdbf2cd 100644 --- a/crates/buzz-relay/src/api/git/transport.rs +++ b/crates/buzz-relay/src/api/git/transport.rs @@ -627,6 +627,21 @@ pub async fn upload_pack( ) } +/// Build the URL the pre-receive hook curls for the `/internal/git/policy` +/// callback. +/// +/// When a UDS path is configured the hook routes over the unix socket (the +/// caller adds `--unix-socket`), and curl ignores the URL host:port — so the +/// host here is a placeholder. Otherwise the hook curls the relay's own TCP +/// loopback on its bind port. Kept as a small pure fn so the host/port +/// selection is unit-testable without standing up a relay. +fn policy_hook_url(uds_path: Option<&str>, bind_port: u16) -> String { + match uds_path { + Some(_) => "http://localhost/internal/git/policy".to_string(), + None => format!("http://127.0.0.1:{bind_port}/internal/git/policy"), + } +} + /// `POST /git/{owner}/{repo}/git-receive-pack` /// /// Handles push — client sends ref updates + pack data. @@ -695,12 +710,23 @@ pub async fn receive_pack( })?; // Build hook env vars for the pre-receive hook. - let hook_url = format!( - "http://127.0.0.1:{}/internal/git/policy", - state.config.bind_addr.port() + // + // The hook curls the relay's own /internal/git/policy callback. By default + // it targets the TCP loopback `127.0.0.1:{bind_port}`. When a UDS is + // configured (BUZZ_UDS_PATH), the same router is also served over the unix + // socket (see main.rs), so we point the hook at the socket via curl's + // --unix-socket. This dodges any in-pod L4 interception of the loopback + // TCP port (e.g. an Istio sidecar's iptables capture of the app port), + // which otherwise breaks the relay's call to itself. + // + // Under --unix-socket curl ignores the URL host:port and uses only the + // path, so the URL host is a placeholder when a socket is set. + let hook_url = policy_hook_url( + state.config.uds_path.as_deref(), + state.config.bind_addr.port(), ); let hooks_dir = repo.path().join("hooks").display().to_string(); - let hook_env = vec![ + let mut hook_env = vec![ ("BUZZ_HOOK_URL", hook_url), ( "BUZZ_HOOK_SECRET", @@ -716,6 +742,12 @@ pub async fn receive_pack( ("GIT_CONFIG_KEY_0", "core.hooksPath".to_string()), ("GIT_CONFIG_VALUE_0", hooks_dir), ]; + // When a UDS is configured, route the policy callback over it. The hook + // only honors BUZZ_HOOK_SOCKET when it is non-empty, so the TCP path is + // untouched for local/dev/CI where BUZZ_UDS_PATH is unset. + if let Some(ref uds_path) = state.config.uds_path { + hook_env.push(("BUZZ_HOOK_SOCKET", uds_path.clone())); + } // Run receive-pack against the tempdir. Returns the *owned* subprocess // output (PackOutput) — crucially NOT a Response, so the post-push @@ -1329,4 +1361,29 @@ mod track_c_tests { let caps = String::from_utf8_lossy(&body); assert!(caps.contains("object-format=sha256")); } + + #[test] + fn policy_hook_url_uses_tcp_loopback_without_uds() { + // No UDS configured → hook curls the relay's own TCP loopback on its + // bind port. Unchanged behavior for local/dev/CI. + assert_eq!( + policy_hook_url(None, 3000), + "http://127.0.0.1:3000/internal/git/policy" + ); + assert_eq!( + policy_hook_url(None, 8080), + "http://127.0.0.1:8080/internal/git/policy" + ); + } + + #[test] + fn policy_hook_url_uses_placeholder_host_with_uds() { + // UDS configured → curl routes over --unix-socket and ignores the URL + // host:port, so the bind port must not leak into the URL. The path is + // what matters; the host is a placeholder. + let url = policy_hook_url(Some("/run/buzz/relay.sock"), 3000); + assert_eq!(url, "http://localhost/internal/git/policy"); + assert!(!url.contains("3000")); + assert!(!url.contains("127.0.0.1")); + } } From cd5da7e3e85098c17840baf01af1a75dbc4a66fc Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Sat, 27 Jun 2026 12:02:09 -0400 Subject: [PATCH 2/2] fix(relay): let require_localhost accept the UDS policy callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Max's review caught that routing the policy callback over the UDS isn't enough: the UDS listener served `into_make_service()` (no connect-info), so over the socket there is no `ConnectInfo` in request extensions. The `/internal/git/policy` route's `require_localhost` guard reads exactly that and `unwrap_or(false)`s — so the UDS callback would fail closed with HTTP 403, turning the original 'network error' into 'push denied by policy (HTTP 403)'. Fix: - Add a `UdsConnectInfo` marker with `Connected>`, and serve the UDS listener with `into_make_service_with_connect_info::()`. - Teach `require_localhost` to accept either a loopback TCP `SocketAddr` OR the presence of `ConnectInfo`. The UDS is an in-pod filesystem path, so its presence is itself proof the caller is on-host. TCP behavior is unchanged and still fail-closed without a loopback `SocketAddr`. - Unit tests for the guard: no connect-info -> 403, loopback TCP -> 200, non-loopback TCP -> 403, UDS marker -> 200. Reviewed-by: Max Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- crates/buzz-relay/src/api/git/mod.rs | 105 ++++++++++++++++++++++++++- crates/buzz-relay/src/main.rs | 21 ++++-- 2 files changed, 118 insertions(+), 8 deletions(-) diff --git a/crates/buzz-relay/src/api/git/mod.rs b/crates/buzz-relay/src/api/git/mod.rs index 730ca225f..57e2d051d 100644 --- a/crates/buzz-relay/src/api/git/mod.rs +++ b/crates/buzz-relay/src/api/git/mod.rs @@ -33,18 +33,54 @@ pub mod transport; pub use transport::git_router; +/// Connect-info marker for requests that arrived over the relay's Unix-domain +/// socket listener (`BUZZ_UDS_PATH`). +/// +/// The UDS listener is bound to a filesystem path inside the pod and is only +/// reachable by processes in the same pod (the pre-receive hook is one). There +/// is no peer IP for a unix socket, so axum can't synthesize a loopback +/// `ConnectInfo` — instead we attach this marker and treat its +/// presence as equivalent to "came from localhost" in `require_localhost`. +#[derive(Clone, Debug)] +pub struct UdsConnectInfo; + +#[cfg(unix)] +impl + axum::extract::connect_info::Connected< + axum::serve::IncomingStream<'_, tokio::net::UnixListener>, + > for UdsConnectInfo +{ + fn connect_info(_stream: axum::serve::IncomingStream<'_, tokio::net::UnixListener>) -> Self { + Self + } +} + /// Middleware that rejects requests from non-loopback addresses. /// /// Defense-in-depth: the internal policy endpoint should only be reachable /// from localhost (the pre-receive hook runs on the same host as the relay). +/// +/// Two trusted transports satisfy "localhost": +/// - a loopback TCP peer (`ConnectInfo` with a loopback IP), or +/// - the relay's own Unix-domain socket (`ConnectInfo`), which +/// is bound to an in-pod path and not reachable off-host. +/// +/// Fail-closed: if neither connect-info is present, reject. In particular the +/// TCP listener still requires a loopback `SocketAddr`, so this does not weaken +/// the existing TCP guard. async fn require_localhost(req: Request, next: Next) -> Response { - let is_loopback = req + let from_loopback_tcp = req .extensions() .get::>() .map(|ci| ci.0.ip().is_loopback()) .unwrap_or(false); - if !is_loopback { + let from_uds = req + .extensions() + .get::>() + .is_some(); + + if !from_loopback_tcp && !from_uds { return (StatusCode::FORBIDDEN, "internal endpoint: localhost only").into_response(); } @@ -63,3 +99,68 @@ pub fn git_policy_router(state: Arc) -> Router { .layer(middleware::from_fn(require_localhost)) .with_state(state) } + +#[cfg(test)] +mod require_localhost_tests { + use super::*; + use axum::{body::Body, http::Request, routing::get}; + use std::net::{Ipv4Addr, SocketAddr}; + use tower::ServiceExt; // for `oneshot` + + /// A trivial router guarded by `require_localhost`, returning 200 when the + /// guard lets the request through. The handler itself never rejects, so any + /// 403 we observe is the guard's doing. + fn guarded_router() -> Router { + Router::new() + .route("/x", get(|| async { StatusCode::OK })) + .layer(middleware::from_fn(require_localhost)) + } + + async fn status_with(install_connect_info: F) -> StatusCode + where + F: FnOnce(Request) -> Request, + { + let req = install_connect_info(Request::builder().uri("/x").body(Body::empty()).unwrap()); + guarded_router().oneshot(req).await.unwrap().status() + } + + #[tokio::test] + async fn rejects_when_no_connect_info() { + // Fail-closed: neither TCP nor UDS connect-info present. + assert_eq!(status_with(|req| req).await, StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn accepts_loopback_tcp() { + let status = status_with(|mut req| { + req.extensions_mut() + .insert(ConnectInfo(SocketAddr::from((Ipv4Addr::LOCALHOST, 12345)))); + req + }) + .await; + assert_eq!(status, StatusCode::OK); + } + + #[tokio::test] + async fn rejects_non_loopback_tcp() { + let status = status_with(|mut req| { + req.extensions_mut().insert(ConnectInfo(SocketAddr::from(( + Ipv4Addr::new(10, 0, 0, 5), + 12345, + )))); + req + }) + .await; + assert_eq!(status, StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn accepts_uds_marker() { + let status = status_with(|mut req| { + req.extensions_mut().insert(ConnectInfo(UdsConnectInfo)); + req + }) + .await; + assert_eq!(status, StatusCode::OK); + } +} diff --git a/crates/buzz-relay/src/main.rs b/crates/buzz-relay/src/main.rs index b63c19758..625d98703 100644 --- a/crates/buzz-relay/src/main.rs +++ b/crates/buzz-relay/src/main.rs @@ -631,12 +631,21 @@ async fn serve( let router_uds = router.clone(); let mut uds_rx = shutdown_tx.subscribe(); let uds_handle = tokio::spawn(async move { - axum::serve(uds_listener, router_uds.into_make_service()) - .with_graceful_shutdown(async move { - uds_rx.changed().await.ok(); - }) - .await - .ok(); + // Serve UDS connections with a connect-info marker so the internal + // `/internal/git/policy` route's `require_localhost` guard accepts + // them: a unix socket has no peer IP, so without this the guard's + // loopback-`SocketAddr` check fails closed (HTTP 403). The socket + // is an in-pod path, so its presence is itself the localhost proof. + axum::serve( + uds_listener, + router_uds + .into_make_service_with_connect_info::(), + ) + .with_graceful_shutdown(async move { + uds_rx.changed().await.ok(); + }) + .await + .ok(); }); let mut tcp_rx = shutdown_tx.subscribe();