fix(file): persist workspace writes with atomic replace

src/tools/file.c

@@ -10,6 +10,7 @@
 #include "core/config.h"
 #include "cJSON.h"
 #include <dirent.h>
+#include <fcntl.h>
 #include <libgen.h>
 #include <limits.h>
 #include <stdio.h>
@@ -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;
 }

tests/test_file.c

@@ -15,6 +15,8 @@
 #include <string.h>
 #include <limits.h>
 #include <errno.h>
+#include <signal.h>
+#include <sys/resource.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
@@ -196,13 +198,88 @@ 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);
 	MU_RUN(test_file_empty_workspace_denies_all);
 	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;
 }