fix: check Content-Length before downloading HTTP response body (#74)

* fix: check Content-Length before downloading HTTP response body

The HTTP tool previously downloaded the entire response body into memory
before checking the size limit, allowing a malicious server to cause OOM.
Now the Content-Length header is checked first to reject obviously
oversized responses, and the body is streamed with a hard size cap so
reading stops as soon as the limit is exceeded.

* fix: check chunk size before allocation and fix Content-Length parsing

Address review feedback:
- Check body.len() + chunk.len() before extend_from_slice to prevent
  OOM from a single oversized chunk
- Use let-chain for Content-Length parsing instead of unwrap_or to
  gracefully handle invalid headers

* docs: document MAX_RESPONSE_SIZE rationale and add tracing on rejection

Address review feedback: explain why 5 MB was chosen for the response
size limit and log a warning when Content-Length causes early rejection.

---------

Co-authored-by: Yi LIU <[email protected]>

src/tools/builtin/http.rs

@@ -5,13 +5,18 @@ use std::net::{IpAddr, ToSocketAddrs};
 use std::time::Duration;
 
 use async_trait::async_trait;
+use futures::StreamExt;
 use reqwest::Client;
 
 use crate::context::JobContext;
 use crate::safety::LeakDetector;
 use crate::tools::tool::{Tool, ToolError, ToolOutput, require_str};
 
-/// Maximum response body size (5 MB). Prevents OOM from unbounded responses.
+/// Maximum response body size (5 MB).
+///
+/// 5 MB is large enough for typical JSON API responses and moderate HTML pages,
+/// but small enough to prevent OOM from malicious or runaway servers.  The WASM
+/// HTTP wrapper uses the same limit for consistency.
 const MAX_RESPONSE_SIZE: usize = 5 * 1024 * 1024;
 
 /// Tool for making HTTP requests.
@@ -230,19 +235,43 @@ impl Tool for HttpTool {
             .filter_map(|(k, v)| v.to_str().ok().map(|v| (k.to_string(), v.to_string())))
             .collect();
 
-        // Get response body with size cap to prevent OOM
-        let body_bytes = response.bytes().await.map_err(|e| {
-            ToolError::ExternalService(format!("failed to read response body: {}", e))
-        })?;
-
-        if body_bytes.len() > MAX_RESPONSE_SIZE {
+        // Pre-check Content-Length header to reject obviously oversized responses
+        // before downloading anything, preventing OOM from malicious servers.
+        if let Some(content_length) = response.headers().get(reqwest::header::CONTENT_LENGTH)
+            && let Ok(s) = content_length.to_str()
+            && let Ok(len) = s.parse::<usize>()
+            && len > MAX_RESPONSE_SIZE
+        {
+            tracing::warn!(
+                url = %parsed_url,
+                content_length = len,
+                max = MAX_RESPONSE_SIZE,
+                "Rejected HTTP response: Content-Length exceeds limit"
+            );
             return Err(ToolError::ExecutionFailed(format!(
-                "Response body too large ({} bytes, max {})",
-                body_bytes.len(),
-                MAX_RESPONSE_SIZE
+                "Response Content-Length ({} bytes) exceeds maximum allowed size ({} bytes)",
+                len, MAX_RESPONSE_SIZE
             )));
         }
 
+        // Stream the response body with a hard size cap. Even if Content-Length was
+        // absent or lied about the size, we stop reading once we exceed the limit.
+        let mut body = Vec::new();
+        let mut stream = response.bytes_stream();
+        while let Some(chunk) = StreamExt::next(&mut stream).await {
+            let chunk = chunk.map_err(|e| {
+                ToolError::ExternalService(format!("failed to read response body: {}", e))
+            })?;
+            if body.len() + chunk.len() > MAX_RESPONSE_SIZE {
+                return Err(ToolError::ExecutionFailed(format!(
+                    "Response body exceeds maximum allowed size ({} bytes)",
+                    MAX_RESPONSE_SIZE
+                )));
+            }
+            body.extend_from_slice(&chunk);
+        }
+        let body_bytes = bytes::Bytes::from(body);
+
         let body_text = String::from_utf8_lossy(&body_bytes).into_owned();
 
         // Try to parse as JSON, fall back to string
@@ -328,4 +357,10 @@ mod tests {
         // Public
         assert!(!is_disallowed_ip(&IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8))));
     }
+
+    #[test]
+    fn test_max_response_size_is_reasonable() {
+        // MAX_RESPONSE_SIZE should be 5 MB to prevent OOM while allowing typical API responses.
+        assert_eq!(MAX_RESPONSE_SIZE, 5 * 1024 * 1024);
+    }
 }