From 7fac6580367c38503f165c757a82d291144da3ae Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 11 Sep 2026 18:18:12 +0000 Subject: [PATCH 1/2] Cap A2A unary HTTP bodies against unbounded remotes rpc_call and http_get appended every write-callback byte with no size limit. SSE was capped in #317, but a hostile agent-card or JSON-RPC peer that omits Content-Length could still grow the process until OOM. Refuse bodies over 16 MiB (inbound API / kSseMaxEventBytes), abort via MAXFILESIZE and the write callback, and clear a partial buffer. Co-authored-by: Tyler Reckart --- CHANGELOG.md | 7 ++ CMakeLists.txt | 18 +++++ include/a2a/http.h | 7 ++ src/a2a/http.cpp | 52 +++++++++++--- tests/test_a2a_http.cpp | 146 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 219 insertions(+), 11 deletions(-) create mode 100644 tests/test_a2a_http.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index dae12b43..588b4446 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). ## [Unreleased] +### Fixed +- **A2A unary HTTP body cap.** `rpc_call` / `http_get` now refuse + responses over 16 MiB (same ceiling as the inbound HTTP API and + `kSseMaxEventBytes`). A remote that omitted `Content-Length` could + previously grow the client buffer without bound; SSE was already + capped (#317). + ## [0.13.4] — 2026-09-10 ### Fixed diff --git a/CMakeLists.txt b/CMakeLists.txt index ee7a9662..2b6b7e39 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1491,6 +1491,24 @@ if(INDEX_BUILD_TESTS) ) add_test(NAME unit_a2a COMMAND unit_a2a) + # Unary A2A HTTP body cap. Separate binary so the in-process unit_a2a + # suite stays network-free; this one stands up a loopback server. + add_executable(unit_a2a_http + tests/test_a2a_http.cpp + src/a2a/http.cpp + src/a2a/sse_reader.cpp + src/ssrf_guard.cpp + ) + target_include_directories(unit_a2a_http PRIVATE + ${CMAKE_SOURCE_DIR}/include + ${CMAKE_SOURCE_DIR}/third_party/doctest + ) + target_link_libraries(unit_a2a_http PRIVATE + CURL::libcurl + Threads::Threads + ) + add_test(NAME unit_a2a_http COMMAND unit_a2a_http) + # Constitution / prompt composer. Pins the capability-bundle resolution # and verifies that empty-capabilities still emits the legacy surface # (back-compat for the master orchestrator). diff --git a/include/a2a/http.h b/include/a2a/http.h index ec87cc67..df493e1a 100644 --- a/include/a2a/http.h +++ b/include/a2a/http.h @@ -20,12 +20,19 @@ #include "a2a/sse_reader.h" #include +#include #include #include #include namespace arbiter::a2a { +// Unary rpc_call / http_get body cap. Matches kSseMaxEventBytes and the +// inbound HTTP API body limit so a remote cannot OOM the process by +// streaming without a Content-Length. SSE already caps per-event and +// diagnostic raw buffers; these calls previously appended unbounded. +inline constexpr size_t kHttpMaxBodyBytes = 16 * 1024 * 1024; + struct HttpHeader { std::string name; std::string value; diff --git a/src/a2a/http.cpp b/src/a2a/http.cpp index df585101..834e8994 100644 --- a/src/a2a/http.cpp +++ b/src/a2a/http.cpp @@ -21,10 +21,40 @@ namespace { // libcurl write callback for buffering a complete response body. // CURL signals EOF by closing; our caller reads `body` after perform(). +// Returning 0 aborts with CURLE_WRITE_ERROR once the unary size cap +// is hit so a remote without Content-Length cannot grow `body` without +// bound (the SSE path already caps via SseReader + kRawCap). +struct BodyBuf { + std::string* body = nullptr; + bool overflow = false; +}; + size_t write_to_string(char* ptr, size_t size, size_t nmemb, void* userdata) { - auto* dst = static_cast(userdata); - dst->append(ptr, size * nmemb); - return size * nmemb; + auto* buf = static_cast(userdata); + const size_t n = size * nmemb; + if (!buf || !buf->body || buf->overflow) return 0; + if (n > kHttpMaxBodyBytes || buf->body->size() > kHttpMaxBodyBytes - n) { + buf->overflow = true; + return 0; + } + buf->body->append(ptr, n); + return n; +} + +void apply_unary_size_cap(CURL* curl) { + curl_easy_setopt(curl, CURLOPT_MAXFILESIZE_LARGE, + static_cast(kHttpMaxBodyBytes)); +} + +void finish_unary(HttpResponse& out, CURLcode rc, const BodyBuf& buf) { + if (buf.overflow || rc == CURLE_FILESIZE_EXCEEDED) { + out.error = "response exceeded size limit"; + out.body.clear(); + return; + } + if (rc != CURLE_OK) { + out.error = curl_easy_strerror(rc); + } } // Build a curl_slist from our HttpHeader vector. Caller frees with @@ -132,18 +162,18 @@ HttpResponse rpc_call(const std::string& url, curl_easy_setopt(curl, CURLOPT_POST, 1L); curl_easy_setopt(curl, CURLOPT_POSTFIELDS, body.c_str()); curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, static_cast(body.size())); + BodyBuf buf{&out.body, false}; curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_to_string); - curl_easy_setopt(curl, CURLOPT_WRITEDATA, &out.body); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &buf); apply_common_opts(curl, timeout_secs, ssrf_guard); + apply_unary_size_cap(curl); CURLcode rc = curl_easy_perform(curl); curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &out.status_code); curl_slist_free_all(headers); curl_easy_cleanup(curl); - if (rc != CURLE_OK) { - out.error = curl_easy_strerror(rc); - } + finish_unary(out, rc, buf); return out; } @@ -165,21 +195,21 @@ HttpResponse http_get(const std::string& url, /*accept_sse=*/false); headers = curl_slist_append(headers, "Accept: application/json"); + BodyBuf buf{&out.body, false}; curl_easy_setopt(curl, CURLOPT_URL, url.c_str()); curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers); curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L); curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_to_string); - curl_easy_setopt(curl, CURLOPT_WRITEDATA, &out.body); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &buf); apply_common_opts(curl, timeout_secs, ssrf_guard); + apply_unary_size_cap(curl); CURLcode rc = curl_easy_perform(curl); curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &out.status_code); curl_slist_free_all(headers); curl_easy_cleanup(curl); - if (rc != CURLE_OK) { - out.error = curl_easy_strerror(rc); - } + finish_unary(out, rc, buf); return out; } diff --git a/tests/test_a2a_http.cpp b/tests/test_a2a_http.cpp new file mode 100644 index 00000000..5f84e22c --- /dev/null +++ b/tests/test_a2a_http.cpp @@ -0,0 +1,146 @@ +// tests/test_a2a_http.cpp — Unary A2A HTTP body cap (#317's SSE caps +// did not cover rpc_call / http_get). A loopback server streams past +// kHttpMaxBodyBytes; the client must abort and surface a size-limit +// error instead of buffering the whole payload. +#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN +#include "doctest.h" + +#include "a2a/http.h" + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +using namespace std::chrono_literals; +using arbiter::a2a::http_get; +using arbiter::a2a::rpc_call; +using arbiter::a2a::kHttpMaxBodyBytes; + +namespace { + +struct LoopbackServer { + int listen_fd = -1; + int port = 0; + std::thread acceptor; + + // When true, send Content-Length larger than the cap and a tiny body + // so CURLOPT_MAXFILESIZE_LARGE can refuse before the write callback. + bool advertise_oversize_length = false; + // When true, omit Content-Length and stream just past the cap so the + // write callback is the one that trips. + bool stream_oversize_body = false; + + std::atomic client_gone{false}; + + void start() { + listen_fd = ::socket(AF_INET, SOCK_STREAM, 0); + REQUIRE(listen_fd >= 0); + int yes = 1; + ::setsockopt(listen_fd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes)); + + sockaddr_in addr{}; + addr.sin_family = AF_INET; + addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + addr.sin_port = 0; + REQUIRE(::bind(listen_fd, reinterpret_cast(&addr), + sizeof(addr)) == 0); + REQUIRE(::listen(listen_fd, 1) == 0); + + socklen_t len = sizeof(addr); + REQUIRE(::getsockname(listen_fd, reinterpret_cast(&addr), + &len) == 0); + port = ntohs(addr.sin_port); + + acceptor = std::thread([this] { handle_one(); }); + } + + void handle_one() { + int cs = ::accept(listen_fd, nullptr, nullptr); + if (cs < 0) return; + char scratch[4096]; + ::recv(cs, scratch, sizeof(scratch), 0); + + if (advertise_oversize_length) { + const std::string hdr = + "HTTP/1.1 200 OK\r\n" + "Content-Type: application/json\r\n" + "Content-Length: " + + std::to_string(kHttpMaxBodyBytes + 1) + + "\r\n\r\n{}"; + ::send(cs, hdr.data(), hdr.size(), 0); + } else if (stream_oversize_body) { + const char hdr[] = + "HTTP/1.1 200 OK\r\n" + "Content-Type: application/json\r\n" + "\r\n"; + ::send(cs, hdr, sizeof(hdr) - 1, 0); + std::string chunk(64 * 1024, 'x'); + size_t sent = 0; + const size_t want = kHttpMaxBodyBytes + chunk.size(); + while (sent < want) { + const ssize_t n = ::send(cs, chunk.data(), chunk.size(), 0); + if (n <= 0) { + client_gone.store(true, std::memory_order_release); + break; + } + sent += static_cast(n); + } + } else { + const char reply[] = + "HTTP/1.1 200 OK\r\n" + "Content-Type: application/json\r\n" + "Content-Length: 2\r\n" + "\r\n" + "{}"; + ::send(cs, reply, sizeof(reply) - 1, 0); + } + ::close(cs); + } + + ~LoopbackServer() { + if (listen_fd >= 0) { + ::shutdown(listen_fd, SHUT_RDWR); + ::close(listen_fd); + } + if (acceptor.joinable()) acceptor.join(); + } +}; + +} // namespace + +TEST_CASE("http_get accepts a small JSON body") { + LoopbackServer srv; + srv.start(); + const std::string url = "http://127.0.0.1:" + std::to_string(srv.port) + "/"; + auto r = http_get(url, {}, /*timeout=*/5, /*ssrf_guard=*/false); + CHECK(r.error.empty()); + CHECK(r.status_code == 200); + CHECK(r.body == "{}"); +} + +TEST_CASE("http_get refuses Content-Length over the unary cap") { + LoopbackServer srv; + srv.advertise_oversize_length = true; + srv.start(); + const std::string url = "http://127.0.0.1:" + std::to_string(srv.port) + "/"; + auto r = http_get(url, {}, /*timeout=*/5, /*ssrf_guard=*/false); + CHECK(r.error.find("size limit") != std::string::npos); + CHECK(r.body.empty()); +} + +TEST_CASE("rpc_call refuses a streamed body past the unary cap") { + LoopbackServer srv; + srv.stream_oversize_body = true; + srv.start(); + const std::string url = "http://127.0.0.1:" + std::to_string(srv.port) + "/"; + auto r = rpc_call(url, {}, "{}", /*timeout=*/10, /*ssrf_guard=*/false); + CHECK(r.error.find("size limit") != std::string::npos); + CHECK(r.body.empty()); +} From fab5e81f604ce6d9e669f95f139fae8107762514 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 11 Sep 2026 18:34:34 +0000 Subject: [PATCH 2/2] Make sandbox /read TOCTOU test deterministic macos-arm64 CI raced the 80 ms pause plus helper thread: resolve finished after the planter, so O_NOFOLLOW never saw the swap. Plant the leaf symlink from a post-resolve hook instead. Co-authored-by: Tyler Reckart --- CHANGELOG.md | 3 +++ include/sandbox.h | 7 +++++++ src/sandbox.cpp | 4 +++- tests/test_sandbox_ssrf.cpp | 27 +++++++++++---------------- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 588b4446..cfb06cf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). `kSseMaxEventBytes`). A remote that omitted `Content-Length` could previously grow the client buffer without bound; SSE was already capped (#317). +- **Sandbox `/read` TOCTOU test is deterministic.** The leaf-symlink-swap + case plants the swap via a post-resolve hook instead of a timed helper + thread (macos-arm64 CI was racing the 80 ms pause). ## [0.13.4] — 2026-09-10 diff --git a/include/sandbox.h b/include/sandbox.h index 8470ad2a..1b368a09 100644 --- a/include/sandbox.h +++ b/include/sandbox.h @@ -50,6 +50,7 @@ #include #include #include +#include #include #include #include @@ -137,6 +138,12 @@ struct SandboxConfig { // plant a leaf symlink swap in the TOCTOU window. 0 in production. int read_check_pause_ms = 0; + // Test-only: runs after path resolve and before the O_NOFOLLOW open, + // when set. Takes precedence over read_check_pause_ms so a TOCTOU + // test can plant a leaf symlink without a thread-scheduling race. + // Empty in production. + std::function read_check_hook; + // Idle-reaping threshold, seconds. A background reaper stops // tenant containers whose last sandbox operation (/exec, /write, // /read, /list) was longer ago than this. Workspace files are diff --git a/src/sandbox.cpp b/src/sandbox.cpp index b67194dd..fe28cb05 100644 --- a/src/sandbox.cpp +++ b/src/sandbox.cpp @@ -1663,7 +1663,9 @@ bool SandboxManager::read_from_workspace(int64_t tenant_id, err_out = "no such file in workspace: " + clean; return false; } - if (cfg_.read_check_pause_ms > 0) { + if (cfg_.read_check_hook) { + cfg_.read_check_hook(); + } else if (cfg_.read_check_pause_ms > 0) { std::this_thread::sleep_for( std::chrono::milliseconds(cfg_.read_check_pause_ms)); } diff --git a/tests/test_sandbox_ssrf.cpp b/tests/test_sandbox_ssrf.cpp index 9495eeae..41532fb0 100644 --- a/tests/test_sandbox_ssrf.cpp +++ b/tests/test_sandbox_ssrf.cpp @@ -6,14 +6,11 @@ #include "ssrf_guard.h" #include -#include -#include #include #include #include #include #include -#include #include #include @@ -165,7 +162,16 @@ TEST_CASE("sandbox read: O_NOFOLLOW refuses leaf symlink swap after resolve") { cfg.workspaces_root = root + "/workspaces"; cfg.runtime = "docker"; cfg.idle_seconds = 0; - cfg.read_check_pause_ms = 80; + + const int64_t tid = 8; + const std::string decoy = cfg.workspaces_root + "/t" + + std::to_string(tid) + "/decoy.txt"; + bool swapped = false; + cfg.read_check_hook = [&]() { + ::unlink(decoy.c_str()); + REQUIRE(::symlink(outside.c_str(), decoy.c_str()) == 0); + swapped = true; + }; const std::string bin = root + "/bin"; fs::create_directories(bin); @@ -182,26 +188,15 @@ TEST_CASE("sandbox read: O_NOFOLLOW refuses leaf symlink swap after resolve") { SandboxManager mgr(cfg); REQUIRE(mgr.usable()); - const int64_t tid = 8; std::string ws = mgr.ensure_workspace(tid); REQUIRE_FALSE(ws.empty()); std::string werr; REQUIRE(mgr.write_to_workspace(tid, "decoy.txt", "workspace-bytes", werr)); - const std::string decoy = ws + "/decoy.txt"; - - std::atomic swapped{false}; - std::thread planter([&]() { - std::this_thread::sleep_for(std::chrono::milliseconds(20)); - ::unlink(decoy.c_str()); - REQUIRE(::symlink(outside.c_str(), decoy.c_str()) == 0); - swapped.store(true); - }); std::string content, mime, err; const bool ok = mgr.read_from_workspace(tid, "decoy.txt", content, mime, err); - planter.join(); - REQUIRE(swapped.load()); + REQUIRE(swapped); CHECK_FALSE(ok); CHECK(content.find("secret-host-bytes") == std::string::npos); CHECK((err.find("symlink") != std::string::npos ||