From a6da4986af74b3ea1d8f0306fbf7469e7072682e Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Mon, 21 Sep 2026 12:33:35 -0300 Subject: [PATCH 1/6] fix(asap): free inbound response payload once on envelope OOM Transfer payload ownership to the envelope so a failed strdup cleanup does not cJSON_Delete the same object twice. --- CHANGELOG.md | 1 + src/asap/server.c | 6 ++++-- tests/test_asap_server.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc2e71b..3bcd0df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha - Gateway `/health` `version` matches `SHELLCLAW_RELEASE_VERSION`. ### Security +- Inbound ASAP response builder no longer double-frees the payload cJSON when a required envelope field cannot be allocated (unauthenticated `POST /asap` `state.query` / `task.cancel`). - Gateway shutdown joins the HTTP thread before `auth_cleanup`, so in-flight `/api/*`, `/pair`, and WebSocket upgrades cannot call `auth_validate_token` / `auth_pair` on a freed `auth_ctx`. - Gateway listen bind now uses `gateway.host` (`lws` `info.iface`). `host = "127.0.0.1"` is loopback-only. Bind-all forms (`0.0.0.0`, `*`, `::`, `[::]`, empty) require `allow_bind_all`. - Camera auto-output keeps the exclusive `mkstemp` inode (no unlink + `${tmpl}.jpg` sibling). diff --git a/src/asap/server.c b/src/asap/server.c index b62cf0d..3aea332 100644 --- a/src/asap/server.c +++ b/src/asap/server.c @@ -84,6 +84,7 @@ static int fill_response_envelope(asap_envelope_t *out, const asap_envelope_t *i const char *payload_type, cJSON *payload) { char ulid_buf[ULID_STRING_LEN + 1]; + cJSON *owned_payload; if (!out || !in || !payload_type || !payload) { if (payload) cJSON_Delete(payload); return -32603; @@ -99,13 +100,14 @@ static int fill_response_envelope(asap_envelope_t *out, const asap_envelope_t *i out->sender = in->recipient ? strdup(in->recipient) : NULL; out->recipient = in->sender ? strdup(in->sender) : NULL; out->payload_type = strdup(payload_type); - out->payload = payload; + /* Ownership of payload moves to out; asap_envelope_clear frees it once. */ + owned_payload = payload; + out->payload = owned_payload; if (in->correlation_id) out->correlation_id = strdup(in->correlation_id); if (in->trace_id) out->trace_id = strdup(in->trace_id); if (!out->id || !out->asap_version || !out->sender || !out->recipient || !out->payload_type) { - cJSON_Delete(payload); asap_envelope_clear(out); asap_envelope_init(out); return -32603; diff --git a/tests/test_asap_server.c b/tests/test_asap_server.c index 2408c58..7d10490 100644 --- a/tests/test_asap_server.c +++ b/tests/test_asap_server.c @@ -775,6 +775,38 @@ static int test_tool_execute_nonzero_reports_error(void) return 0; } +/* + * fill_response_envelope used to cJSON_Delete(payload) after assigning + * out->payload, then asap_envelope_clear(out) deleted the same object. + * HTTP parse always supplies sender/recipient, so production hits this on + * post-attach strdup OOM; dropping sender here is the same cleanup path. + */ +static int test_response_builder_missing_sender_does_not_double_free(void) +{ + asap_envelope_t in; + asap_envelope_t out; + asap_server_ctx_t ctx; + char err[128]; + cJSON *pl; + int rc; + + pl = cJSON_CreateObject(); + ASSERT(pl != NULL); + ASSERT(cJSON_AddNullToObject(pl, "task_id") != NULL); + ASSERT(wrap_build(&in, "task.cancel", pl) == 0); + free(in.sender); + in.sender = NULL; + memset(&ctx, 0, sizeof ctx); + asap_envelope_init(&out); + rc = asap_server_handle(&in, &out, &ctx, err, sizeof err); + ASSERT(rc == -32603); + ASSERT(strstr(err, "envelope") != NULL); + ASSERT(out.payload == NULL); + teardown_env(&in); + teardown_env(&out); + return 0; +} + static int submit_task_request(asap_server_ctx_t *ctx, const char *sender, const char *input) { asap_envelope_t in; @@ -1055,6 +1087,7 @@ int main(void) r |= test_mcp_omitted_arguments_defaults_to_empty_object(); r |= test_tool_call_hook_overrides_builtin_dispatch(); r |= test_tool_execute_nonzero_reports_error(); + r |= test_response_builder_missing_sender_does_not_double_free(); r |= test_trust_sender_rejects_blank_sender_when_list_nonempty(); r |= test_mcp_tool_call_holds_agent_mutex(); r |= test_mcp_tool_call_hook_holds_agent_mutex(); From 82b62cbf85e9c52cd3a0d16c05d01f2912479751 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Mon, 21 Sep 2026 12:50:58 -0300 Subject: [PATCH 2/6] fix(gateway): honor 1 MiB POST /asap body cap Skip the 64 KiB static buffer check when POST /asap uses the dynamic body so legal envelopes up to ASAP_BODY_MAX are not 413'd. --- CHANGELOG.md | 1 + src/gateway/asap_http_body.c | 7 ++++++ src/gateway/asap_http_body.h | 12 +++++++++ src/gateway/http_lws.c | 3 ++- tests/test_asap_http_body.c | 18 ++++++++++++++ tests/test_gateway_http.c | 47 +++++++++++++++++++++++++++++++++++- 6 files changed, 86 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bcd0df..3f11d89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha - `memory_init` no longer deletes an existing SQLite DB when `sqlite3_open` fails (permissions or transient I/O). - Anthropic `content` parse fails closed when growing the text buffer or `tool_use` array cannot `realloc`, instead of copying against an inflated cap. - HTTP 200 JSON-RPC results with a malformed ASAP envelope no longer double-free the duplicated request id. +- `POST /asap` honors the 1 MiB dynamic body cap instead of 413'ing envelopes above the 64 KiB static `PUT /api/config` buffer. - Inbound ASAP `mcp.tool_call` and `state.query` now hold `agent_lock()` around tool execute and SQLite `g_db` reads, matching `task.request`. - Inbound `POST /asap` now wires the process provider and tool table into `asap_ctx`, so `task.request` and `mcp.tool_call` dispatch instead of failing with `server missing cfg or provider`. - `POST /asap` rejects serialized JSON-RPC larger than the 64 KiB gateway HTTP buffer (HTTP 500 / JSON-RPC `-32603`) instead of truncating the body. diff --git a/src/gateway/asap_http_body.c b/src/gateway/asap_http_body.c index 7ca5a2d..e62f562 100644 --- a/src/gateway/asap_http_body.c +++ b/src/gateway/asap_http_body.c @@ -44,6 +44,13 @@ int asap_http_body_parse_content_length(const char *cl_buf, long *cl_out) return 0; } +int asap_http_body_exceeds_static_cap(const asap_http_body_t *body, long content_length) +{ + if (!body || body->use_dyn_body) + return 0; + return content_length > (long)BODY_BUF_SIZE; +} + int asap_http_body_init_from_request(struct lws *wsi, asap_http_body_t *body) { char cl_buf[32] = {0}; diff --git a/src/gateway/asap_http_body.h b/src/gateway/asap_http_body.h index 1eb474d..8ad59aa 100644 --- a/src/gateway/asap_http_body.h +++ b/src/gateway/asap_http_body.h @@ -30,6 +30,18 @@ typedef struct asap_http_body { */ int asap_http_body_parse_content_length(const char *cl_buf, long *cl_out); +/** + * True when Content-Length exceeds the static POST/PUT cap (BODY_BUF_SIZE). + * POST /asap uses a 1 MiB dynamic buffer (use_dyn_body); skip the static cap + * so envelopes between 64 KiB and ASAP_BODY_MAX are not 413'd. + * + * Example: asap_http_body_exceeds_static_cap(&body, 70000) is 0 when + * body.use_dyn_body is set, and 1 for the default static buffer. + * + * @return 1 if the static cap applies and is exceeded; 0 otherwise. + */ +int asap_http_body_exceeds_static_cap(const asap_http_body_t *body, long content_length); + /** * For POST /asap: validate Content-Length and allocate dynamic buffer. * @return 0 ok, -1 body too large, -2 allocation failure; non-/asap returns 0. diff --git a/src/gateway/http_lws.c b/src/gateway/http_lws.c index 468b180..86ecb79 100644 --- a/src/gateway/http_lws.c +++ b/src/gateway/http_lws.c @@ -273,7 +273,8 @@ int http_callback(struct lws *wsi, enum lws_callback_reasons reason, void *user, lws_callback_on_writable(wsi); } else { long cl = 0; - if (http_body_content_length(wsi, &cl) == 0 && cl > (long)BODY_BUF_SIZE) + if (http_body_content_length(wsi, &cl) == 0 && + asap_http_body_exceeds_static_cap(&conn->body, cl)) conn->body.body_too_large = 1; conn->body.body[0] = '\0'; conn->body.body_len = 0; diff --git a/tests/test_asap_http_body.c b/tests/test_asap_http_body.c index 5e7df11..9af8c37 100644 --- a/tests/test_asap_http_body.c +++ b/tests/test_asap_http_body.c @@ -6,6 +6,7 @@ #include "gateway/asap_http_body.h" #include +#include #include #define ASSERT(c) do { \ @@ -67,6 +68,19 @@ static int test_static_append_sets_too_large(void) return 0; } +static int test_exceeds_static_cap(void) +{ + asap_http_body_t body; + + memset(&body, 0, sizeof(body)); + ASSERT(asap_http_body_exceeds_static_cap(NULL, (long)BODY_BUF_SIZE + 1) == 0); + ASSERT(asap_http_body_exceeds_static_cap(&body, (long)BODY_BUF_SIZE) == 0); + ASSERT(asap_http_body_exceeds_static_cap(&body, (long)BODY_BUF_SIZE + 1) == 1); + body.use_dyn_body = 1; + ASSERT(asap_http_body_exceeds_static_cap(&body, (long)BODY_BUF_SIZE + 1) == 0); + return 0; +} + int main(void) { int failed = 0; @@ -86,6 +100,10 @@ int main(void) fprintf(stderr, "test_static_append_sets_too_large failed\n"); failed++; } + if (test_exceeds_static_cap() != 0) { + fprintf(stderr, "test_exceeds_static_cap failed\n"); + failed++; + } if (failed == 0) printf("test_asap_http_body: all tests passed\n"); return failed ? 1 : 0; diff --git a/tests/test_gateway_http.c b/tests/test_gateway_http.c index 2bb9456..6d087b5 100644 --- a/tests/test_gateway_http.c +++ b/tests/test_gateway_http.c @@ -874,7 +874,8 @@ static int test_asap_body_over_max(void) long code; char *body = NULL; const char payload[] = "{}"; - int r = http_post_raw(gw_url("/asap"), payload, sizeof(payload) - 1, "1000001", + /* ASAP_BODY_MAX is 1 MiB (1048576). 1000001 is still under that cap. */ + int r = http_post_raw(gw_url("/asap"), payload, sizeof(payload) - 1, "1048577", &code, &body); ASSERT(r == 0); ASSERT(code == 413); @@ -883,6 +884,46 @@ static int test_asap_body_over_max(void) return 0; } +/* + * POST /asap allocates a 1 MiB dynamic buffer. A leftover Content-Length check + * still compared against the 64 KiB static cap used by PUT /api/config, so a + * valid state.query just over 64 KiB was 413'd before parse. + */ +static int test_asap_body_over_static_cap_accepted(void) +{ + enum { PAD = 70000 }; + const char prefix[] = + "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"asap.send\",\"params\":{" + "\"id\":\"e1\",\"asap_version\":\"2.1\"," + "\"sender\":\"urn:from\",\"recipient\":\"urn:to\"," + "\"payload_type\":\"state.query\",\"payload\":{\"pad\":\""; + const char suffix[] = "\"}}}"; + size_t prefix_len = strlen(prefix); + size_t suffix_len = strlen(suffix); + size_t total = prefix_len + (size_t)PAD + suffix_len; + char *payload; + long code = 0; + char *body = NULL; + int r; + + payload = malloc(total + 1U); + ASSERT(payload != NULL); + memcpy(payload, prefix, prefix_len); + memset(payload + prefix_len, 'A', (size_t)PAD); + memcpy(payload + prefix_len + (size_t)PAD, suffix, suffix_len + 1U); + ASSERT(total > 65536U); + ASSERT(total < (1024U * 1024U)); + r = http_post(gw_url("/asap"), payload, &code, &body); + free(payload); + ASSERT(r == 0); + ASSERT(code == 200); + ASSERT(body != NULL); + ASSERT(strstr(body, "\"result\"") != NULL); + ASSERT(strstr(body, "sessions") != NULL); + free(body); + return 0; +} + static int test_health_wellknown(void) { long code; @@ -1572,6 +1613,10 @@ int main(int argc, char **argv) fprintf(stderr, "test_asap_body_over_max failed\n"); failed++; } + if (test_asap_body_over_static_cap_accepted() != 0) { + fprintf(stderr, "test_asap_body_over_static_cap_accepted failed\n"); + failed++; + } if (test_asap_invalid_body() != 0) { fprintf(stderr, "test_asap_invalid_body failed\n"); failed++; } if (test_asap_missing_fields() != 0) { fprintf(stderr, "test_asap_missing_fields failed\n"); failed++; } if (test_asap_task_request() != 0) { fprintf(stderr, "test_asap_task_request failed\n"); failed++; } From c261e4571040c2c8f63ade0477ba954a0c189366 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Mon, 21 Sep 2026 18:16:57 -0300 Subject: [PATCH 3/6] refactor(asap): drop owned_payload alias in fill_response_envelope Assign payload directly to out->payload. The extra cJSON_Delete is already gone; the local alias was only for cppcheck. --- src/asap/server.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/asap/server.c b/src/asap/server.c index 3aea332..63bebdc 100644 --- a/src/asap/server.c +++ b/src/asap/server.c @@ -84,7 +84,6 @@ static int fill_response_envelope(asap_envelope_t *out, const asap_envelope_t *i const char *payload_type, cJSON *payload) { char ulid_buf[ULID_STRING_LEN + 1]; - cJSON *owned_payload; if (!out || !in || !payload_type || !payload) { if (payload) cJSON_Delete(payload); return -32603; @@ -101,8 +100,7 @@ static int fill_response_envelope(asap_envelope_t *out, const asap_envelope_t *i out->recipient = in->sender ? strdup(in->sender) : NULL; out->payload_type = strdup(payload_type); /* Ownership of payload moves to out; asap_envelope_clear frees it once. */ - owned_payload = payload; - out->payload = owned_payload; + out->payload = payload; if (in->correlation_id) out->correlation_id = strdup(in->correlation_id); if (in->trace_id) From bfd96851e8f73a4c1387dee67e455df4dc909a94 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Mon, 21 Sep 2026 18:18:17 -0300 Subject: [PATCH 4/6] fix(gateway): treat NULL body as exceeding static cap Fail closed if a later caller forgets to pass &conn->body instead of skipping the 64 KiB check. --- src/gateway/asap_http_body.c | 4 +++- src/gateway/asap_http_body.h | 3 ++- tests/test_asap_http_body.c | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/gateway/asap_http_body.c b/src/gateway/asap_http_body.c index e62f562..46f7475 100644 --- a/src/gateway/asap_http_body.c +++ b/src/gateway/asap_http_body.c @@ -46,7 +46,9 @@ int asap_http_body_parse_content_length(const char *cl_buf, long *cl_out) int asap_http_body_exceeds_static_cap(const asap_http_body_t *body, long content_length) { - if (!body || body->use_dyn_body) + if (!body) + return 1; + if (body->use_dyn_body) return 0; return content_length > (long)BODY_BUF_SIZE; } diff --git a/src/gateway/asap_http_body.h b/src/gateway/asap_http_body.h index 8ad59aa..7728866 100644 --- a/src/gateway/asap_http_body.h +++ b/src/gateway/asap_http_body.h @@ -34,11 +34,12 @@ int asap_http_body_parse_content_length(const char *cl_buf, long *cl_out); * True when Content-Length exceeds the static POST/PUT cap (BODY_BUF_SIZE). * POST /asap uses a 1 MiB dynamic buffer (use_dyn_body); skip the static cap * so envelopes between 64 KiB and ASAP_BODY_MAX are not 413'd. + * A NULL body pointer is fail-closed (returns 1). * * Example: asap_http_body_exceeds_static_cap(&body, 70000) is 0 when * body.use_dyn_body is set, and 1 for the default static buffer. * - * @return 1 if the static cap applies and is exceeded; 0 otherwise. + * @return 1 if the static cap applies and is exceeded, or body is NULL; 0 otherwise. */ int asap_http_body_exceeds_static_cap(const asap_http_body_t *body, long content_length); diff --git a/tests/test_asap_http_body.c b/tests/test_asap_http_body.c index 9af8c37..b7bb98c 100644 --- a/tests/test_asap_http_body.c +++ b/tests/test_asap_http_body.c @@ -73,7 +73,8 @@ static int test_exceeds_static_cap(void) asap_http_body_t body; memset(&body, 0, sizeof(body)); - ASSERT(asap_http_body_exceeds_static_cap(NULL, (long)BODY_BUF_SIZE + 1) == 0); + ASSERT(asap_http_body_exceeds_static_cap(NULL, (long)BODY_BUF_SIZE + 1) == 1); + ASSERT(asap_http_body_exceeds_static_cap(NULL, 0) == 1); ASSERT(asap_http_body_exceeds_static_cap(&body, (long)BODY_BUF_SIZE) == 0); ASSERT(asap_http_body_exceeds_static_cap(&body, (long)BODY_BUF_SIZE + 1) == 1); body.use_dyn_body = 1; From 7c3d1e27237c7455127d5d2f041467fdc7519fbc Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Mon, 21 Sep 2026 18:18:43 -0300 Subject: [PATCH 5/6] docs(asap): document inbound 1 MiB POST /asap body cap Keep the leftover 64 KiB static BODY_BUF_SIZE check off inbound /asap. --- docs/ASAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ASAP.md b/docs/ASAP.md index 6a1d344..8426463 100644 --- a/docs/ASAP.md +++ b/docs/ASAP.md @@ -16,7 +16,7 @@ When the gateway is enabled and signing keys load successfully, ShellClaw serves |-------|------|----------| | `GET /.well-known/asap/manifest.json` | Public | **SignedManifest** JSON (inner manifest + Ed25519 signature + public_key) | | `GET /.well-known/asap/health` | Public | Minimal health stub (`{"status":"ok"}` in v1.0) | -| `POST /asap` | Rate-limited | JSON-RPC ASAP ingress: `task.request` / `mcp.tool_call` dispatch with the process provider and tool table. Serialized responses larger than the 64 KiB gateway HTTP buffer (`RESP_BUF_SIZE`) are rejected with HTTP 500 / JSON-RPC `-32603` rather than truncated. Compliance harness shape is still partial (see Known gaps). | +| `POST /asap` | Rate-limited | JSON-RPC ASAP ingress: `task.request` / `mcp.tool_call` dispatch with the process provider and tool table. Inbound bodies use a 1 MiB dynamic buffer (`ASAP_BODY_MAX`); do not reintroduce the 64 KiB static `BODY_BUF_SIZE` check used by `PUT /api/config`. Serialized responses larger than the 64 KiB gateway HTTP buffer (`RESP_BUF_SIZE`) are rejected with HTTP 500 / JSON-RPC `-32603` rather than truncated. Compliance harness shape is still partial (see Known gaps). | | `GET /api/asap/log` | Bearer | Inbound ASAP message log | Implementation: `src/asap/manifest.c`, `src/gateway/routes.c`. If keys cannot load, manifest route returns **500** and agent startup fails fast (`init_subsystems()`). From 5c214113970609cff96e3cf82d43e41bccbd9fc1 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Mon, 21 Sep 2026 18:19:03 -0300 Subject: [PATCH 6/6] test(gateway): record POST /asap rate-limit headroom Seven counted POSTs in the 60s window leave 3 of 10 slots; the 1 MiB 413 path is LWS init and is not counted. --- tests/test_gateway_http.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_gateway_http.c b/tests/test_gateway_http.c index 6d087b5..a471390 100644 --- a/tests/test_gateway_http.c +++ b/tests/test_gateway_http.c @@ -1609,6 +1609,13 @@ int main(int argc, char **argv) failed++; } if (test_health_wellknown() != 0) { fprintf(stderr, "test_health_wellknown failed\n"); failed++; } + /* + * Per-IP /asap is ASAP_RATE_LIMIT_RPM (10) per 60s, counted in + * handle_asap after dyn malloc. CL > 1 MiB 413s at LWS init and is + * not counted. Seven POSTs reach handle_asap (invalid_body, + * missing_fields, task_request, mcp_tool_call, mcp_unknown_tool, + * oversized_response, body_over_static_cap_accepted). Headroom is 3. + */ if (test_asap_body_over_max() != 0) { fprintf(stderr, "test_asap_body_over_max failed\n"); failed++;