diff --git a/CHANGELOG.md b/CHANGELOG.md index c55475c..831d9ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,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. @@ -41,6 +42,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`). - `auth_pair` persists tokens via unique temp (`mkstemp`)+fsync+rename so a failed write cannot wipe `auth_tokens.json` (#71). - `auth_pair` fails closed when bearer RNG fails (no uninitialized token, no tokens-file write, pairing code kept) (#92). - 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`. 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()`). diff --git a/src/asap/server.c b/src/asap/server.c index b62cf0d..63bebdc 100644 --- a/src/asap/server.c +++ b/src/asap/server.c @@ -99,13 +99,13 @@ 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); + /* Ownership of payload moves to out; asap_envelope_clear frees it once. */ out->payload = 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/src/gateway/asap_http_body.c b/src/gateway/asap_http_body.c index 7ca5a2d..46f7475 100644 --- a/src/gateway/asap_http_body.c +++ b/src/gateway/asap_http_body.c @@ -44,6 +44,15 @@ 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) + return 1; + if (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..7728866 100644 --- a/src/gateway/asap_http_body.h +++ b/src/gateway/asap_http_body.h @@ -30,6 +30,19 @@ 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. + * 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, or body is NULL; 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..b7bb98c 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,20 @@ 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) == 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; + ASSERT(asap_http_body_exceeds_static_cap(&body, (long)BODY_BUF_SIZE + 1) == 0); + return 0; +} + int main(void) { int failed = 0; @@ -86,6 +101,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_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(); diff --git a/tests/test_gateway_http.c b/tests/test_gateway_http.c index 2bb9456..a471390 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; @@ -1568,10 +1609,21 @@ 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++; } + 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++; }