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/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/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")); + } } 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();