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 39c24a8..3136269 100644 --- a/src/gateway/routes.c +++ b/src/gateway/routes.c @@ -377,28 +377,76 @@ 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; + char limit_desc[64]; + + 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) { + 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; + char limit_desc[64]; + 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) { + 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, 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;