Skip to content
Merged
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ loosely while pre-1.0 (breaking changes can land on minor bumps).
unchanged. See [`docs/concepts/reconcile.md`](docs/concepts/reconcile.md)
and [#208](https://github.com/tylerreckart/arbiter/issues/208).
### Fixed
- **A2A unary HTTP errors stay bounded.** `Client::rpc` no longer concatenates
the remote response body into `err_out`. `/a2a call` copies that string into
the calling agent's tool envelope, so a verbose or hostile remote could dump
unbounded HTML/JSON (and any secrets it echoed) into conversation history.
Non-200 responses now report `HTTP <status>` plus a 200-byte, single-line
JSON-RPC `error.message` when present. JSON-RPC errors on HTTP 200 are
clipped the same way.
- **Remote `--connect` DELETE/PATCH body cap.** Conversation delete and
title-rename used a one-shot libcurl write callback that appended the
entire response with no limit. A `--connect` peer that omitted
Expand Down
9 changes: 9 additions & 0 deletions include/a2a/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -309,4 +309,13 @@ RpcResponse make_error_response(const std::shared_ptr<JsonValue>& request_id,
RpcResponse make_result_response(const std::shared_ptr<JsonValue>& request_id,
std::shared_ptr<JsonValue> result);

// Bound, single-line text for Client::rpc err_out. /a2a call copies that
// string into the calling agent's tool envelope, so a remote HTTP body or
// JSON-RPC error.message must not be forwarded wholesale (size, CR/LF, or
// secrets echoed by a verbose 4xx/5xx page).
constexpr size_t kMaxRpcErrorDetail = 200;
std::string sanitize_rpc_error_text(const std::string& text);
std::string format_rpc_http_error(long status_code, const std::string& body);
std::string format_rpc_json_error(int code, const std::string& message);

} // namespace arbiter::a2a
8 changes: 5 additions & 3 deletions src/a2a/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ std::shared_ptr<JsonValue> Client::rpc(const std::string& method,
return nullptr;
}
if (r.status_code != 200) {
err_out = "HTTP " + std::to_string(r.status_code) + ": " + r.body;
// Status plus a bounded JSON-RPC message when present — never the
// raw body. /a2a call copies err_out into the calling agent's
// tool envelope and conversation history.
err_out = format_rpc_http_error(r.status_code, r.body);
return nullptr;
}
std::shared_ptr<JsonValue> v;
Expand All @@ -111,8 +114,7 @@ std::shared_ptr<JsonValue> Client::rpc(const std::string& method,
return nullptr;
}
if (resp.error) {
err_out = "JSON-RPC error " + std::to_string(resp.error->code) +
": " + resp.error->message;
err_out = format_rpc_json_error(resp.error->code, resp.error->message);
return nullptr;
}
if (!resp.result) {
Expand Down
66 changes: 66 additions & 0 deletions src/a2a/types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,26 @@ std::shared_ptr<JsonValue> opt_passthrough(const JsonValue& v, const std::string
return p ? p : nullptr;
}

bool is_utf8_continuation(unsigned char c) {
return (c & 0xC0) == 0x80;
}

// Prefer a JSON-RPC error.message (or a top-level string "error") over
// dumping the raw body. Parse failures and non-objects yield empty —
// callers then report status alone.
std::string extract_rpc_error_message(const std::string& body) {
if (body.empty()) return {};
try {
auto v = json_parse(body);
if (!v || !v->is_object()) return {};
auto err = v->get("error");
if (!err) return {};
if (err->is_string()) return err->as_string();
if (err->is_object()) return err->get_string("message", "");
} catch (...) {}
return {};
}

void put_if(JsonObject& m, const std::string& key, const std::optional<std::string>& v) {
if (v) m[key] = jstr(*v);
}
Expand Down Expand Up @@ -627,4 +647,50 @@ RpcResponse make_result_response(const std::shared_ptr<JsonValue>& request_id,
return r;
}

// ---------------------------------------------------------------------------
// Client err_out shaping. Kept next to the JSON-RPC envelope so unit_a2a
// can pin the bound without linking the HTTP client.
// ---------------------------------------------------------------------------

std::string sanitize_rpc_error_text(const std::string& text) {
std::string out;
out.reserve(text.size());
for (unsigned char c : text) {
if (c == '\t') {
out.push_back(' ');
continue;
}
// Drop CR/LF/NUL and other ASCII controls so the detail cannot
// split a tool-result line or an SSE frame.
if (c < 0x20 || c == 0x7f) continue;
out.push_back(static_cast<char>(c));
}
if (out.size() <= kMaxRpcErrorDetail) return out;

size_t n = kMaxRpcErrorDetail;
while (n > 0 && n < out.size() &&
is_utf8_continuation(static_cast<unsigned char>(out[n]))) {
--n;
}
out.resize(n);
out += "...";
return out;
}

std::string format_rpc_http_error(long status_code, const std::string& body) {
const std::string detail =
sanitize_rpc_error_text(extract_rpc_error_message(body));
std::string out = "HTTP " + std::to_string(status_code);
if (!detail.empty()) {
out += ": ";
out += detail;
}
return out;
}

std::string format_rpc_json_error(int code, const std::string& message) {
return "JSON-RPC error " + std::to_string(code) + ": " +
sanitize_rpc_error_text(message);
}

} // namespace arbiter::a2a
40 changes: 40 additions & 0 deletions tests/test_a2a.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,46 @@ TEST_CASE("RpcResponse parses round-tripped success and error") {
CHECK(parsed_err.error->code == -32601);
}

TEST_CASE("format_rpc_http_error omits raw HTML bodies") {
const std::string html =
"<html><body>Internal error. cookie=secret token=sk-live</body></html>";
CHECK(format_rpc_http_error(500, html) == "HTTP 500");
CHECK(format_rpc_http_error(502, std::string(8000, 'x')) == "HTTP 502");
CHECK(format_rpc_http_error(401, "") == "HTTP 401");
}

TEST_CASE("format_rpc_http_error keeps a bounded JSON-RPC message") {
CHECK(format_rpc_http_error(
401, R"({"jsonrpc":"2.0","error":{"code":-32000,"message":"unauthorized"}})") ==
"HTTP 401: unauthorized");
CHECK(format_rpc_http_error(403, R"({"error":"tenant disabled"})") ==
"HTTP 403: tenant disabled");
}

TEST_CASE("sanitize_rpc_error_text drops controls and truncates") {
CHECK(sanitize_rpc_error_text("one\r\nSet-Cookie: x=1") == "oneSet-Cookie: x=1");
CHECK(sanitize_rpc_error_text("a\tb") == "a b");
const std::string exact(kMaxRpcErrorDetail, 'a');
CHECK(sanitize_rpc_error_text(exact) == exact);
CHECK(sanitize_rpc_error_text(exact + "b") == exact + "...");
// 199 ASCII + 2-byte UTF-8 (é) is 201 bytes — drop the incomplete char.
const std::string with_utf8 = std::string(199, 'a') + "\xc3\xa9";
CHECK(with_utf8.size() == kMaxRpcErrorDetail + 1);
CHECK(sanitize_rpc_error_text(with_utf8) == std::string(199, 'a') + "...");
}

TEST_CASE("format_rpc_json_error clips message the same way") {
CHECK(format_rpc_json_error(-32002, "not cancelable") ==
"JSON-RPC error -32002: not cancelable");
const std::string long_msg(400, 'z');
const std::string got = format_rpc_json_error(-32603, "oops\r\n" + long_msg);
CHECK(got.find('\n') == std::string::npos);
CHECK(got.find('\r') == std::string::npos);
CHECK(got.rfind("...") == got.size() - 3);
CHECK(got.size() == std::string("JSON-RPC error -32603: ").size() +
kMaxRpcErrorDetail + 3);
}

// ── 3. SSE parser ─────────────────────────────────────────────────────

TEST_CASE("SseReader dispatches one event per blank line") {
Expand Down
Loading