From b0774934e8d85e1a87649295bf639efa7ca4ae72 Mon Sep 17 00:00:00 2001 From: naps62-yolo Date: Sun, 31 May 2026 21:12:17 +0000 Subject: [PATCH] fix(maestro): bound web proxy request bodies + stream responses (NAP-257) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `proxy_http` buffered request bodies with `to_bytes(.., usize::MAX)` and materialized full upstream responses with `.bytes()`. Anyone reachable on the bind address could POST a multi-GB body (the cookie middleware admits any signed cookie) and OOM the process; a large upstream reply did the same. Cap buffered request bodies at 8 MiB and return 413 on overflow; stream the upstream response straight through via `bytes_stream()` → `Body::from_stream` so it's never fully held in RAM. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/maestro-web/src/proxy.rs | 67 +++++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/crates/maestro-web/src/proxy.rs b/crates/maestro-web/src/proxy.rs index 508eca3..be435ee 100644 --- a/crates/maestro-web/src/proxy.rs +++ b/crates/maestro-web/src/proxy.rs @@ -66,14 +66,22 @@ impl ProxyState { // HTTP proxy // --------------------------------------------------------------------------- +/// Cap on the request body we'll buffer before forwarding upstream. The proxied +/// daemon endpoints (`/api/sessions` CRUD, etc.) all carry small JSON; 8 MiB is +/// far above anything legitimate. Without a cap, anyone reachable on the bind +/// address could POST a multi-GB body and OOM the process (NAP-257). +pub(crate) const MAX_REQUEST_BODY: usize = 8 * 1024 * 1024; + /// Forward an HTTP request to the upstream daemon and stream the response back. pub async fn proxy_http(State(state): State, req: Request) -> Response { let (parts, body) = req.into_parts(); let upstream_uri = build_uri(&state.upstream.daemon_http_base, &parts.uri); - let body_bytes = match axum::body::to_bytes(body, usize::MAX).await { + // Bound the buffered request body. `to_bytes` returns `Err` once the body + // exceeds the limit; treat that as 413 rather than buffering unboundedly. + let body_bytes = match axum::body::to_bytes(body, MAX_REQUEST_BODY).await { Ok(b) => b, - Err(e) => return bad_gateway(&format!("read body: {e}")), + Err(_) => return payload_too_large(), }; let mut upstream_req = state @@ -92,16 +100,14 @@ pub async fn proxy_http(State(state): State, req: Request) -> Respon let status = upstream_resp.status(); let headers = filter_response_headers(upstream_resp.headers()); - let body = match upstream_resp.bytes().await { - Ok(b) => b, - Err(e) => return bad_gateway(&format!("upstream body: {e}")), - }; + // Stream the response body straight through instead of materializing it — + // a large upstream reply never gets fully buffered in RAM (NAP-257). let mut resp = Response::builder().status(status); for (name, value) in headers.iter() { resp = resp.header(name, value); } - resp.body(Body::from(body)) + resp.body(Body::from_stream(upstream_resp.bytes_stream())) .unwrap_or_else(|e| bad_gateway(&format!("build response: {e}"))) } @@ -176,6 +182,19 @@ fn bad_gateway(msg: &str) -> Response { .into_response() } +fn payload_too_large() -> Response { + tracing::warn!(limit = MAX_REQUEST_BODY, "proxy: request body exceeds cap"); + let body = format!( + r#"{{"error":{{"code":"payload_too_large","message":"request body exceeds {MAX_REQUEST_BODY} bytes"}}}}"# + ); + ( + StatusCode::PAYLOAD_TOO_LARGE, + [(header::CONTENT_TYPE, "application/json")], + body, + ) + .into_response() +} + // --------------------------------------------------------------------------- // WebSocket proxy // --------------------------------------------------------------------------- @@ -283,4 +302,38 @@ mod tests { let u = UpstreamConfig::from_daemon_url("https://daemon.example/"); assert_eq!(&*u.daemon_ws_base, "wss://daemon.example"); } + + /// NAP-257: a request body over the cap is rejected with 413 before we ever + /// dial upstream — so an oversized POST can't OOM the proxy. The upstream is + /// a bogus unreachable address; a 413 (not a 502) proves we bailed early. + #[tokio::test] + async fn oversized_request_body_is_rejected_with_413() { + let state = ProxyState::new(UpstreamConfig::from_daemon_url("http://127.0.0.1:9")); + let big = vec![0u8; MAX_REQUEST_BODY + 1]; + let req = Request::builder() + .method("POST") + .uri("/api/sessions") + .body(Body::from(big)) + .unwrap(); + + let resp = proxy_http(State(state), req).await; + assert_eq!(resp.status(), StatusCode::PAYLOAD_TOO_LARGE); + } + + /// A body at the cap still proceeds to the upstream send (which fails here + /// with 502 since nothing is listening) — i.e. the cap is exclusive of the + /// limit, not off-by-one against legitimate traffic. + #[tokio::test] + async fn body_at_cap_is_not_rejected_by_the_size_check() { + let state = ProxyState::new(UpstreamConfig::from_daemon_url("http://127.0.0.1:9")); + let body = vec![0u8; MAX_REQUEST_BODY]; + let req = Request::builder() + .method("POST") + .uri("/api/sessions") + .body(Body::from(body)) + .unwrap(); + + let resp = proxy_http(State(state), req).await; + assert_eq!(resp.status(), StatusCode::BAD_GATEWAY); + } }