diff --git a/Makefile b/Makefile index 762bfec..7280bcb 100644 --- a/Makefile +++ b/Makefile @@ -377,7 +377,7 @@ $(HTTP_O): src/gateway/http.c src/gateway/http.h src/gateway/http_lws.h src/gate $(HTTP_LWS_O): src/gateway/http_lws.c src/gateway/http_lws.h src/gateway/asap_http_body.h src/gateway/routes.h src/gateway/auth.h src/gateway/static.h src/gateway/ws.h $(CC) $(CFLAGS) $(INC) $(GATEWAY_CFLAGS) -pthread -c -o $@ src/gateway/http_lws.c -$(ROUTES_O): src/gateway/routes.c src/gateway/routes.h src/gateway/routes_hardware.h src/gateway/http.h src/gateway/http_lws.h src/gateway/auth.h src/gateway/rate_limit.h src/tools/context.h src/asap/manifest.h src/asap/envelope.h src/asap/server.h src/asap/log.h src/core/bootstrap.h src/core/agent.h src/core/config.h src/core/config_patch.h src/core/reload.h src/core/memory.h src/core/skill.h src/providers/provider.h src/channels/channel.h src/tools/cron.h src/tools/tool.h +$(ROUTES_O): src/gateway/routes.c src/gateway/routes.h src/gateway/routes_hardware.h src/gateway/http.h src/gateway/http_lws.h src/gateway/auth.h src/gateway/rate_limit.h src/tools/context.h src/asap/manifest.h src/asap/envelope.h src/asap/server.h src/asap/log.h src/core/bootstrap.h src/core/agent.h src/core/config.h src/core/config_patch.h src/core/dispatch.h src/core/reload.h src/core/memory.h src/core/skill.h src/providers/provider.h src/channels/channel.h src/tools/cron.h src/tools/tool.h $(CC) $(CFLAGS) $(INC) $(GATEWAY_CFLAGS) -pthread -c -o $@ src/gateway/routes.c $(ROUTES_HARDWARE_O): src/gateway/routes_hardware.c src/gateway/routes_hardware.h src/gateway/routes.h src/gateway/http_lws.h src/gateway/uri_match.h src/hardware/hardware.h src/hardware/hardware_gpio_snapshot.h src/hardware/hardware_tegrastats.h src/hardware/board_detect.h src/core/config.h diff --git a/src/core/agent.h b/src/core/agent.h index 27e8c32..a1dcd6e 100644 --- a/src/core/agent.h +++ b/src/core/agent.h @@ -54,6 +54,7 @@ int agent_run(const config_t *cfg, const char *session_id, const char *user_mess * Every agent_run() caller (main-loop handle_message, inbound ASAP HTTP, * WebSocket dispatcher) must hold this mutex for the duration of agent_run(). * /reset in handle_message must also hold it around session_delete(). + * Dashboard DELETE /api/sessions/:id must use dispatch_delete_session(). * After a successful cron ch->send, handle_message re-takes it around * cron_ack_delivery() (SQLite amalgamation is SQLITE_THREADSAFE=0). * Inbound mcp.tool_call must hold it around tool execute (see #60). diff --git a/src/core/dispatch.c b/src/core/dispatch.c index 44b1479..68d2427 100644 --- a/src/core/dispatch.c +++ b/src/core/dispatch.c @@ -43,6 +43,18 @@ static int send_then_maybe_ack_cron(const channel_t *ch, const channel_incoming_ return maybe_ack_cron(ch, msg); } +int dispatch_delete_session(const char *session_id) +{ + int rc; + if (!session_id || session_id[0] == '\0') + return -1; + /* Serialize with agent_run so session_save cannot recreate the row. */ + agent_lock(); + rc = session_delete(session_id); + agent_unlock(); + return rc; +} + int handle_message(const channel_t *ch, const channel_incoming_msg_t *msg) { const char *text = msg->text ? msg->text : ""; diff --git a/src/core/dispatch.h b/src/core/dispatch.h index 36ac069..e94bb48 100644 --- a/src/core/dispatch.h +++ b/src/core/dispatch.h @@ -18,6 +18,20 @@ extern "C" { */ int handle_message(const channel_t *ch, const channel_incoming_msg_t *msg); +/** + * Delete one session while holding the agent mutex. + * + * Dashboard `DELETE /api/sessions/:id` runs on the gateway thread. agent_run() + * loads history, talks to the model, then session_save()s. Without this lock + * the delete lands in that window and the save puts the old transcript back. + * + * Example: `if (dispatch_delete_session(id) != 0) respond 404;` + * + * @param session_id Session row id. Empty or NULL is rejected. + * @return 0 when a row was removed, non-zero otherwise. + */ +int dispatch_delete_session(const char *session_id); + #ifdef __cplusplus } #endif diff --git a/src/core/skill.c b/src/core/skill.c index 7401b9a..180654f 100644 --- a/src/core/skill.c +++ b/src/core/skill.c @@ -325,15 +325,39 @@ int skill_list_names(const config_t *cfg, char **names_out, int max_count) int skill_get_content(const config_t *cfg, const char *name, char *out_buf, size_t out_size) { - if (!cfg || !name || !out_buf || out_size == 0) return -1; char path[MAX_PATH_LEN]; - if (build_skill_path(cfg, name, path, sizeof(path)) != 0) return -1; - FILE *f = fopen(path, "r"); - if (!f) return -1; + FILE *f; + long sz; + size_t used; + + if (!cfg || !name || !out_buf || out_size == 0) + return -1; + if (build_skill_path(cfg, name, path, sizeof(path)) != 0) + return -1; + f = fopen(path, "r"); + if (!f) + return -1; + if (fseek(f, 0, SEEK_END) != 0) { + fclose(f); + return -1; + } + sz = ftell(f); + /* Dashboard GET copies this buffer into the editor and PUT writes it + * back. A short read must not look like the whole file. */ + if (sz < 0 || (size_t)sz >= out_size) { + fclose(f); + return (sz < 0) ? -1 : -2; + } + if (fseek(f, 0, SEEK_SET) != 0) { + fclose(f); + return -1; + } out_buf[0] = '\0'; - size_t used = 0; + used = 0; append_file_content(f, out_buf, out_size, &used); fclose(f); + if (used != (size_t)sz) + return -2; return 0; } diff --git a/src/core/skill.h b/src/core/skill.h index 49cdae3..42d1dd9 100644 --- a/src/core/skill.h +++ b/src/core/skill.h @@ -63,8 +63,11 @@ int skill_list_names(const config_t *cfg, char **names_out, int max_count); * @param cfg Configuration (uses config_skills_dir). * @param name Skill name (file base name). * @param out_buf Buffer to receive content. - * @param out_size Size of out_buf. - * @return 0 on success, -1 if not found or error. + * @param out_size Size of out_buf. Must hold the whole file plus a NUL. + * @return 0 on success, -2 if the file does not fit in out_buf, -1 if not found or error. + * + * Example: skill_get_content(cfg, "deploy", buf, sizeof(buf)) == -2 means the + * caller must not treat buf as the skill body. */ int skill_get_content(const config_t *cfg, const char *name, char *out_buf, size_t out_size); diff --git a/src/gateway/routes.c b/src/gateway/routes.c index 760a05a..da4e1db 100644 --- a/src/gateway/routes.c +++ b/src/gateway/routes.c @@ -17,6 +17,7 @@ #include "asap/log.h" #include "core/agent.h" #include "core/bootstrap.h" +#include "core/dispatch.h" #include "core/config.h" #include "core/config_patch.h" #include "core/memory.h" @@ -377,28 +378,75 @@ static void handle_skills_list(const config_t *cfg, char *buf, size_t size, int #define SKILL_CONTENT_MAX 32768 #define MEMORY_RESULTS_MAX 16384 -static void handle_skill_get(const config_t *cfg, const char *name, char *buf, size_t size, int *status) +static void skill_too_large(char *buf, size_t size, int *status, const char *name, + const char *limit_desc) { - char *content = malloc(SKILL_CONTENT_MAX); - if (!content) { json_error(buf, size, status, 500, "Out of memory"); return; } - if (skill_get_content(cfg, name, content, SKILL_CONTENT_MAX) != 0) { - free(content); - json_error(buf, size, status, 404, "Skill not found"); + char msg[256]; + int n; + + n = snprintf(msg, sizeof(msg), "skill \"%s\" exceeds %s", + name ? name : "", limit_desc ? limit_desc : "edit buffer"); + if (n < 0 || (size_t)n >= sizeof(msg)) + json_error(buf, size, status, 413, "skill exceeds edit buffer"); + else + json_error(buf, size, status, 413, msg); +} + +static void skill_get_send(const char *name, const char *content, char *buf, size_t size, + int *status) +{ + cJSON *obj; + char *printed; + obj = cJSON_CreateObject(); + if (!obj) { + json_error(buf, size, status, 500, "Internal error"); return; } - cJSON *obj = cJSON_CreateObject(); - if (!obj) { free(content); json_error(buf, size, status, 500, "Internal error"); return; } cJSON_AddItemToObject(obj, "name", cJSON_CreateString(name)); cJSON_AddItemToObject(obj, "content", cJSON_CreateString(content)); - free(content); - char *s = cJSON_PrintUnformatted(obj); + printed = cJSON_PrintUnformatted(obj); cJSON_Delete(obj); - if (s) { - json_response(buf, size, status, s); - free(s); - } else { + if (!printed) { json_error(buf, size, status, 500, "Internal error"); + return; } + /* A cut-off 200 body parses as empty in the dashboard and Save wipes the file. */ + if (strlen(printed) >= size) { + char limit_desc[64]; + free(printed); + snprintf(limit_desc, sizeof(limit_desc), "%zu byte response buffer", size); + skill_too_large(buf, size, status, name, limit_desc); + return; + } + json_response(buf, size, status, printed); + free(printed); +} + +static void handle_skill_get(const config_t *cfg, const char *name, char *buf, size_t size, int *status) +{ + char *content; + int rc; + + content = malloc(SKILL_CONTENT_MAX); + if (!content) { + json_error(buf, size, status, 500, "Out of memory"); + return; + } + rc = skill_get_content(cfg, name, content, SKILL_CONTENT_MAX); + if (rc != 0) { + free(content); + if (rc == -2) { + char limit_desc[64]; + snprintf(limit_desc, sizeof(limit_desc), "%d byte edit buffer", + SKILL_CONTENT_MAX - 1); + skill_too_large(buf, size, status, name, limit_desc); + } else { + json_error(buf, size, status, 404, "Skill not found"); + } + return; + } + skill_get_send(name, content, buf, size, status); + free(content); } static void handle_skill_create(const config_t *cfg, const char *body, size_t body_len, @@ -508,11 +556,7 @@ static void handle_sessions_list(char *buf, size_t size, int *status) static void handle_session_delete(const char *id, char *buf, size_t size, int *status) { - int rc; - agent_lock(); - rc = session_delete(id); - agent_unlock(); - if (rc != 0) { + if (dispatch_delete_session(id) != 0) { json_error(buf, size, status, 404, "Session not found"); return; } diff --git a/tests/test_dispatch.c b/tests/test_dispatch.c index 2478f42..4286bac 100644 --- a/tests/test_dispatch.c +++ b/tests/test_dispatch.c @@ -468,8 +468,32 @@ static int test_handle_message_holds_agent_mutex(void) return 0; } +static int test_api_session_delete_holds_agent_mutex(void) +{ + const char *db_path = "build/test_dispatch_api_delete.db"; + char history[128]; + + memory_cleanup(); + remove(db_path); + ASSERT(memory_init(db_path) == 0); + ASSERT(session_save("webchat:9", "[{\"role\":\"user\",\"content\":\"secret\"}]") == 0); + g_agent_mutex_held_during_reset = 0; + session_delete_set_hook_for_test(reset_lock_probe); + ASSERT(dispatch_delete_session("webchat:9") == 0); + session_delete_set_hook_for_test(NULL); + ASSERT(g_agent_mutex_held_during_reset == 1); + ASSERT(agent_mutex_is_locked_for_test() == 0); + ASSERT(session_load("webchat:9", history, sizeof(history)) != 0); + ASSERT(dispatch_delete_session("") != 0); + ASSERT(dispatch_delete_session(NULL) != 0); + memory_cleanup(); + remove(db_path); + return 0; +} + int main(void) { + RUN(test_api_session_delete_holds_agent_mutex()); RUN(test_reset_clears_session()); RUN(test_status_returns_version()); RUN(test_agent_failure_fallback_message()); diff --git a/tests/test_gateway_http.c b/tests/test_gateway_http.c index 58b3bfb..f2b6aab 100644 --- a/tests/test_gateway_http.c +++ b/tests/test_gateway_http.c @@ -1179,6 +1179,13 @@ static int test_api_skill_create_delete(const char *token) ASSERT(code == 200 || code == 201); free(body); body = NULL; + r = http_get_auth(gw_url("/api/skills/test_integration_skill"), token, &code, &body); + ASSERT(r == 0); + ASSERT(code == 200); + ASSERT(body != NULL); + ASSERT(strstr(body, "Test skill for integration") != NULL); + free(body); + body = NULL; r = http_delete_auth(gw_url("/api/skills/test_integration_skill"), token, &code, &body); ASSERT(r == 0); ASSERT(code == 200); @@ -1186,6 +1193,65 @@ static int test_api_skill_create_delete(const char *token) return 0; } +/* A skill larger than the edit buffer must not come back as HTTP 200. + * The dashboard saves whatever GET returned, which used to drop the tail. */ +static int test_api_skill_get_refuses_oversize(const char *token) +{ + enum { BODY_N = 40000 }; + const char *tail = "SKILL_TAIL_KEEP"; + char *content; + char *json; + char path[256]; + char disk[32]; + long code; + char *body = NULL; + FILE *f; + size_t tail_len; + size_t json_cap; + int n; + int r; + + tail_len = strlen(tail); + content = malloc((size_t)BODY_N + 1); + ASSERT(content != NULL); + memset(content, 'B', (size_t)BODY_N); + memcpy(content + (size_t)BODY_N - tail_len, tail, tail_len); + content[BODY_N] = '\0'; + json_cap = (size_t)BODY_N + 64; + json = malloc(json_cap); + ASSERT(json != NULL); + n = snprintf(json, json_cap, "{\"name\":\"big_skill\",\"content\":\"%s\"}", content); + ASSERT(n > 0 && (size_t)n < json_cap); + r = http_post_auth(gw_url("/api/skills"), token, json, &code, &body); + ASSERT(r == 0); + ASSERT(code == 200 || code == 201); + free(body); + body = NULL; + r = http_get_auth(gw_url("/api/skills/big_skill"), token, &code, &body); + ASSERT(r == 0); + ASSERT(code == 413); + ASSERT(body != NULL); + ASSERT(strstr(body, "big_skill") != NULL); + ASSERT(strstr(body, "exceeds") != NULL); + free(body); + snprintf(path, sizeof(path), "%s/.shellclaw/skills/big_skill.md", g_test_home); + f = fopen(path, "r"); + ASSERT(f != NULL); + ASSERT(fseek(f, -((long)tail_len), SEEK_END) == 0); + ASSERT(fread(disk, 1, tail_len, f) == tail_len); + disk[tail_len] = '\0'; + fclose(f); + ASSERT(strcmp(disk, tail) == 0); + free(content); + free(json); + body = NULL; + r = http_delete_auth(gw_url("/api/skills/big_skill"), token, &code, &body); + ASSERT(r == 0); + ASSERT(code == 200); + free(body); + return 0; +} + static int test_api_memory(const char *token) { long code; @@ -1728,6 +1794,10 @@ int main(int argc, char **argv) if (test_api_context_snapshot_get(token) != 0) { fprintf(stderr, "test_api_context_snapshot_get failed\n"); failed++; } if (test_api_skills_list(token) != 0) { fprintf(stderr, "test_api_skills_list failed\n"); failed++; } if (test_api_skill_create_delete(token) != 0) { fprintf(stderr, "test_api_skill_create_delete failed\n"); failed++; } + if (test_api_skill_get_refuses_oversize(token) != 0) { + fprintf(stderr, "test_api_skill_get_refuses_oversize failed\n"); + failed++; + } if (test_api_memory(token) != 0) { fprintf(stderr, "test_api_memory failed\n"); failed++; } if (test_api_cron_list(token) != 0) { fprintf(stderr, "test_api_cron_list failed\n"); failed++; } if (test_api_cron_create_delete(token) != 0) { fprintf(stderr, "test_api_cron_create_delete failed\n"); failed++; } diff --git a/tests/test_skill.c b/tests/test_skill.c index dd49bf3..f00120e 100644 --- a/tests/test_skill.c +++ b/tests/test_skill.c @@ -249,6 +249,45 @@ static int test_skill_update_write_failure_preserves_existing(void) return 0; } +/* + * Dashboard GET /api/skills/:name loads this buffer into a textarea and PUT + * saves it back. A short buffer must not succeed with a prefix, or Save + * deletes the rest of the skill file. + */ +static int test_skill_get_content_refuses_short_buffer(void) +{ + char cmd[256]; + char small[32]; + char exact[64]; + char full[128]; + const char *body = "HEAD_MARKER_skill_body_TAIL_MARKER_must_survive"; + size_t body_len; + config_t *cfg = NULL; + char errbuf[256]; + + snprintf(cmd, sizeof(cmd), "rm -rf \"%s\" && mkdir -p \"%s\"", TMP_DIR, TMP_DIR); + ASSERT(system(cmd) == 0); + ASSERT(write_minimal_config(TMP_DIR) == 0); + ASSERT(config_load(TMP_CONFIG, &cfg, errbuf, sizeof(errbuf)) == 0); + ASSERT(skill_create(cfg, "longskill", body) == 0); + body_len = strlen(body); + ASSERT(body_len + 1 <= sizeof(exact)); + + memset(small, 'X', sizeof(small)); + ASSERT(skill_get_content(cfg, "longskill", small, sizeof(small)) == -2); + ASSERT(skill_get_content(cfg, "longskill", exact, body_len) == -2); + ASSERT(skill_get_content(cfg, "longskill", exact, body_len + 1) == 0); + ASSERT(strcmp(exact, body) == 0); + ASSERT(skill_get_content(cfg, "longskill", full, sizeof(full)) == 0); + ASSERT(strstr(full, "TAIL_MARKER_must_survive") != NULL); + + config_free(cfg); + remove(TMP_CONFIG); + snprintf(cmd, sizeof(cmd), "rm -rf \"%s\"", TMP_DIR); + (void)system(cmd); + return 0; +} + static int test_hot_reload_watch(void) { char cmd[256]; @@ -287,6 +326,7 @@ int main(void) RUN(test_system_prompt_base_order()); RUN(test_skill_crud()); RUN(test_skill_update_write_failure_preserves_existing()); + RUN(test_skill_get_content_refuses_short_buffer()); RUN(test_hot_reload_watch()); printf("test_skill: all tests passed\n"); return 0;