diff --git a/src/core/skill.c b/src/core/skill.c index 6cb0490..eb5c8c7 100644 --- a/src/core/skill.c +++ b/src/core/skill.c @@ -333,6 +333,64 @@ int skill_get_content(const config_t *cfg, const char *name, char *out_buf, size return 0; } +static void discard_skill_tmp(int fd, char *tmp_path) +{ + if (fd >= 0) + (void)close(fd); + if (tmp_path) { + unlink(tmp_path); + free(tmp_path); + } +} + +/* + * Replace skill markdown via temp+rename so fopen("w") cannot wipe an existing + * skill before the new content is fully on disk (ENOSPC / EFBIG / crash). + */ +static int write_skill_atomic(const char *path, const char *content) +{ + size_t content_len; + size_t off; + size_t path_len; + char *tmp_path; + int fd; + + if (!path || !content) return -1; + content_len = strlen(content); + path_len = strlen(path); + tmp_path = malloc(path_len + 8); + if (!tmp_path) return -1; + snprintf(tmp_path, path_len + 8, "%s.tmp", path); + fd = open(tmp_path, O_WRONLY | O_CREAT | O_TRUNC, 0600); + if (fd < 0) { + free(tmp_path); + return -1; + } + off = 0; + while (off < content_len) { + ssize_t n = write(fd, content + off, content_len - off); + if (n <= 0) { + discard_skill_tmp(fd, tmp_path); + return -1; + } + off += (size_t)n; + } + if (fsync(fd) != 0) { + discard_skill_tmp(fd, tmp_path); + return -1; + } + if (close(fd) != 0) { + discard_skill_tmp(-1, tmp_path); + return -1; + } + if (rename(tmp_path, path) != 0) { + discard_skill_tmp(-1, tmp_path); + return -1; + } + free(tmp_path); + return 0; +} + int skill_create(const config_t *cfg, const char *name, const char *content) { if (!cfg || !name || !content) return -1; @@ -343,12 +401,7 @@ int skill_create(const config_t *cfg, const char *name, const char *content) fclose(f); return -1; } - f = fopen(path, "w"); - if (!f) return -1; - size_t len = strlen(content); - size_t written = fwrite(content, 1, len, f); - fclose(f); - return (written == len) ? 0 : -1; + return write_skill_atomic(path, content); } int skill_update(const config_t *cfg, const char *name, const char *content) @@ -356,12 +409,7 @@ int skill_update(const config_t *cfg, const char *name, const char *content) if (!cfg || !name || !content) return -1; char path[MAX_PATH_LEN]; if (build_skill_path(cfg, name, path, sizeof(path)) != 0) return -1; - FILE *f = fopen(path, "w"); - if (!f) return -1; - size_t len = strlen(content); - size_t written = fwrite(content, 1, len, f); - fclose(f); - return (written == len) ? 0 : -1; + return write_skill_atomic(path, content); } int skill_delete(const config_t *cfg, const char *name) diff --git a/tests/test_skill.c b/tests/test_skill.c index 7fde121..dd49bf3 100644 --- a/tests/test_skill.c +++ b/tests/test_skill.c @@ -5,10 +5,13 @@ #include "core/config.h" #include "core/skill.h" +#include +#include #include #include #include -#include +#include +#include #define ASSERT(c) do { if (!(c)) { fprintf(stderr, "FAIL: %s:%d %s\n", __FILE__, __LINE__, #c); return 1; } } while (0) #define RUN(t) do { int r = (t); if (r) return r; } while (0) @@ -194,6 +197,58 @@ static int test_skill_crud(void) return 0; } +/* + * skill_update used fopen("w") which truncates before write. A failed write + * (ENOSPC/EFBIG) wiped the live skill. Atomic temp+rename must preserve it. + */ +static int test_skill_update_write_failure_preserves_existing(void) +{ + char cmd[256]; + char content[256]; + char oversized[512]; + struct rlimit old_lim; + struct rlimit new_lim; + int update_ret; + size_t i; + 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, "keepme", "# Keep\nOriginal skill body that must survive") == 0); + ASSERT(skill_get_content(cfg, "keepme", content, sizeof(content)) == 0); + ASSERT(strstr(content, "Original skill body that must survive") != NULL); + + for (i = 0; i < sizeof(oversized) - 1; i++) + oversized[i] = 'A'; + oversized[sizeof(oversized) - 1] = '\0'; + + ASSERT(getrlimit(RLIMIT_FSIZE, &old_lim) == 0); + new_lim = old_lim; + new_lim.rlim_cur = 8; + (void)signal(SIGXFSZ, SIG_IGN); + ASSERT(setrlimit(RLIMIT_FSIZE, &new_lim) == 0); + + update_ret = skill_update(cfg, "keepme", oversized); + ASSERT(setrlimit(RLIMIT_FSIZE, &old_lim) == 0); + + ASSERT(update_ret != 0); + ASSERT(skill_get_content(cfg, "keepme", content, sizeof(content)) == 0); + ASSERT(strstr(content, "Original skill body that must survive") != NULL); + + ASSERT(skill_update(cfg, "keepme", "# Keep\nRecovered after limit") == 0); + ASSERT(skill_get_content(cfg, "keepme", content, sizeof(content)) == 0); + ASSERT(strstr(content, "Recovered after limit") != 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]; @@ -231,6 +286,7 @@ int main(void) RUN(test_missing_dir_no_crash()); RUN(test_system_prompt_base_order()); RUN(test_skill_crud()); + RUN(test_skill_update_write_failure_preserves_existing()); RUN(test_hot_reload_watch()); printf("test_skill: all tests passed\n"); return 0;