fix(relay): let require_localhost accept the UDS policy callback

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<SocketAddr>` 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<IncomingStream<UnixListener>>`, and serve the UDS listener with `into_make_service_with_connect_info::<UdsConnectInfo>()`. - Teach `require_localhost` to accept either a loopback TCP `SocketAddr` OR the presence of `ConnectInfo<UdsConnectInfo>`. 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 <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
2026-08-18 06:50:31 +02:00 · 2026-06-27 12:02:09 -04:00
parent eb76c81909
commit cd5da7e3e8
2 changed files with 118 additions and 8 deletions
@@ -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<SocketAddr>` — 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<SocketAddr>` with a loopback IP), or
+/// - the relay's own Unix-domain socket (`ConnectInfo<UdsConnectInfo>`), 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<Body>, next: Next) -> Response {
-    let is_loopback = req
+    let from_loopback_tcp = req
        .extensions()
        .get::<ConnectInfo<SocketAddr>>()
        .map(|ci| ci.0.ip().is_loopback())
        .unwrap_or(false);

-    if !is_loopback {
+    let from_uds = req
+        .extensions()
+        .get::<ConnectInfo<UdsConnectInfo>>()
+        .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<AppState>) -> 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<F>(install_connect_info: F) -> StatusCode
+    where
+        F: FnOnce(Request<Body>) -> Request<Body>,
+    {
+        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);
+    }
+}
@@ -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::<buzz_relay::api::git::UdsConnectInfo>(),
+            )
+            .with_graceful_shutdown(async move {
+                uds_rx.changed().await.ok();
+            })
+            .await
+            .ok();
        });

        let mut tcp_rx = shutdown_tx.subscribe();