diff --git a/src/tools/file.c b/src/tools/file.c index e8170b1..75672be 100644 --- a/src/tools/file.c +++ b/src/tools/file.c @@ -10,6 +10,7 @@ #include "core/config.h" #include "cJSON.h" #include +#include #include #include #include @@ -95,6 +96,71 @@ static int file_read(const char *path, char *result_buf, size_t max_len) return 0; } +static void discard_file_tmp(int fd, char *tmp_path) +{ + if (fd >= 0) + (void)close(fd); + if (tmp_path) { + unlink(tmp_path); + free(tmp_path); + } +} + +static int write_all(int fd, const char *buf, size_t len) +{ + size_t off = 0; + + while (off < len) { + ssize_t n = write(fd, buf + off, len - off); + if (n <= 0) + return -1; + off += (size_t)n; + } + return 0; +} + +/* + * Replace the target via temp+rename so fopen("w") cannot wipe an existing + * workspace file before the new bytes are fully on disk (ENOSPC / EFBIG). + */ +static int write_file_atomic(const char *path, const char *content) +{ + size_t path_len; + char *tmp_path; + int fd; + + if (!path || !content) + return -1; + 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, 0666); + if (fd < 0) { + free(tmp_path); + return -1; + } + if (write_all(fd, content, strlen(content)) != 0) { + discard_file_tmp(fd, tmp_path); + return -1; + } + if (fsync(fd) != 0) { + discard_file_tmp(fd, tmp_path); + return -1; + } + if (close(fd) != 0) { + discard_file_tmp(-1, tmp_path); + return -1; + } + if (rename(tmp_path, path) != 0) { + discard_file_tmp(-1, tmp_path); + return -1; + } + free(tmp_path); + return 0; +} + static int file_write(const char *path, const char *content, char *result_buf, size_t max_len) { char resolved[PATH_MAX]; @@ -124,20 +190,10 @@ static int file_write(const char *path, const char *content, char *result_buf, s memcpy(safe_path + res_len + 1, base, base_len + 1); } } - FILE *f = fopen(safe_path, "w"); - if (!f) { - snprintf(result_buf, max_len, "{\"error\":\"cannot write file\"}"); + if (write_file_atomic(safe_path, content ? content : "") != 0) { + snprintf(result_buf, max_len, "{\"error\":\"write failed\"}"); return -1; } - if (content) { - size_t len = strlen(content); - if (fwrite(content, 1, len, f) != len) { - fclose(f); - snprintf(result_buf, max_len, "{\"error\":\"write failed\"}"); - return -1; - } - } - fclose(f); snprintf(result_buf, max_len, "{\"status\":\"ok\"}"); return 0; } diff --git a/tests/test_file.c b/tests/test_file.c index b4087ac..4fe15d2 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -15,6 +15,8 @@ #include #include #include +#include +#include #include #include @@ -196,6 +198,80 @@ static void test_symlink_escape_rejected(void) rmdir(tmpdir); } +/* + * file_write used fopen("w"), which truncates before the new bytes are + * durable. ENOSPC/EFBIG then destroyed the live workspace file. Atomic + * temp+rename must keep the original contents when the write fails. + */ +static void test_file_write_failure_preserves_existing(void) +{ + char tmpdir[PATH_MAX]; + char notes_path[PATH_MAX]; + char config_path[PATH_MAX]; + char args[PATH_MAX + 640]; + char buf[512]; + char oversized[512]; + struct rlimit old_lim; + struct rlimit new_lim; + config_t *cfg; + const tool_t *t; + int write_ret; + size_t i; + + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_fsize_%d", (int)getpid()); + if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return; + snprintf(notes_path, sizeof(notes_path), "%s/notes.md", tmpdir); + cfg = make_ws_config(tmpdir, config_path, sizeof(config_path)); + MU_ASSERT(cfg != NULL, "fsize: load config"); + tool_file_set_config(cfg); + t = tool_file_get(); + + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"original notes that must survive\"}", + notes_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == 0, "seed original file"); + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", notes_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == 0, "read seeded file"); + MU_ASSERT(strstr(buf, "original notes that must survive") != NULL, "seeded content present"); + + for (i = 0; i < sizeof(oversized) - 1; i++) + oversized[i] = 'A'; + oversized[sizeof(oversized) - 1] = '\0'; + + MU_ASSERT(getrlimit(RLIMIT_FSIZE, &old_lim) == 0, "getrlimit FSIZE"); + new_lim = old_lim; + new_lim.rlim_cur = 8; + (void)signal(SIGXFSZ, SIG_IGN); + MU_ASSERT(setrlimit(RLIMIT_FSIZE, &new_lim) == 0, "setrlimit FSIZE"); + + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"%s\"}", + notes_path, oversized); + write_ret = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(setrlimit(RLIMIT_FSIZE, &old_lim) == 0, "restore rlimit"); + + MU_ASSERT(write_ret != 0, "oversized write fails"); + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", notes_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == 0, "read after failed write"); + MU_ASSERT(strstr(buf, "original notes that must survive") != NULL, + "failed write must not wipe original"); + + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"recovered after limit\"}", + notes_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == 0, "write recovers after limit"); + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", notes_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == 0, "read recovered file"); + MU_ASSERT(strstr(buf, "recovered after limit") != NULL, "recovered content present"); + + config_free(cfg); + unlink(config_path); + unlink(notes_path); + snprintf(notes_path, sizeof(notes_path), "%s/notes.md.tmp", tmpdir); + unlink(notes_path); + rmdir(tmpdir); +} + int main(void) { MU_RUN(test_file_read_write_list); @@ -203,6 +279,7 @@ int main(void) MU_RUN(test_file_outside_workspace_rejected); MU_RUN(test_path_traversal_rejected); MU_RUN(test_symlink_escape_rejected); + MU_RUN(test_file_write_failure_preserves_existing); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; }