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] 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();