fix(file,camera): workspace write integrity

CHANGELOG.md

@@ -5,6 +5,9 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha
 ## [Unreleased]
 
 ### Fixed
+- `write_file` maps to the intended path instead of the first existing ancestor, so a nested path cannot truncate a workspace file treated as a directory or overwrite a same-named file in a parent (#67). Leaf workspace symlinks (dangling or an in-workspace alias) are rejected (`lstat` + `O_NOFOLLOW`) instead of creating host files outside the workspace (#90).
+- `write_file` persists via unique temp (`mkstemp`)+fsync+rename so a failed write cannot wipe an existing workspace file and a sibling `path.tmp` is not truncated (#78).
+- Camera capture fails closed when `workspace_only` is on with an empty `workspace_path`, and rejects leaf symlink outputs (#91, #90).
 - Cron job `schedule` and `message` are delivered as full SQLite TEXT instead of truncating to 127/511 bytes (#73).
 - Cron jobs are committed (delete/advance) only after successful agent delivery, so a failed `agent_run` cannot drop a reminder (#57).
 - Recurring cron jobs search the next run from the following minute with a 366-day window; ack fail-closes unparseable schedules to now+365d so they cannot re-fire every poll (#65).

src/hardware/hardware_camera.c

@@ -32,6 +32,8 @@ typedef enum camera_cli_kind {
 
 static int s_camera_ready;
 static char s_workspace[PATH_MAX];
+/** Non-zero when workspace_only is on (set_workspace with non-NULL). */
+static int s_workspace_enforced;
 static hardware_camera_spawn_fn s_test_spawn;
 static char *s_last_argv[HARDWARE_CAMERA_ARGV_MAX];
 static char s_last_argv_storage[HARDWARE_CAMERA_ARGV_MAX][ARG_BUF_SZ];
@@ -108,13 +110,20 @@ static int path_inside_workspace(const char *path)
 	char ws_resolved[PATH_MAX];
 	char resolved[PATH_MAX];
 	char path_copy[PATH_MAX];
+	struct stat lst;
 
-	if (s_workspace[0] == '\0' || !path || path[0] == '\0')
+	/* workspace_only off (set_workspace(NULL)): no containment. */
+	if (!s_workspace_enforced)
 		return 1;
+	/* Enforced but missing/empty root or empty path: deny (file.c parity). */
+	if (s_workspace[0] == '\0' || !path || path[0] == '\0')
+		return 0;
 	if (realpath(s_workspace, ws_resolved) == NULL)
 		return 0;
 	if (realpath(path, resolved) != NULL)
 		return resolved_under_workspace(resolved, ws_resolved);
+	if (lstat(path, &lst) == 0 && S_ISLNK(lst.st_mode))
+		return 0;
 	snprintf(path_copy, sizeof(path_copy), "%s", path);
 	for (;;) {
 		char *dir = dirname(path_copy);
@@ -417,7 +426,13 @@ int hardware_camera_init(void)
 
 void hardware_camera_set_workspace(const char *workspace)
 {
-	if (!workspace || workspace[0] == '\0') {
+	if (!workspace) {
+		s_workspace[0] = '\0';
+		s_workspace_enforced = 0;
+		return;
+	}
+	s_workspace_enforced = 1;
+	if (workspace[0] == '\0') {
 		s_workspace[0] = '\0';
 		return;
 	}
@@ -436,6 +451,7 @@ void hardware_camera_shutdown(void)
 	s_test_spawn = NULL;
 	s_camera_ready = 0;
 	s_workspace[0] = '\0';
+	s_workspace_enforced = 0;
 	s_spawn_timeout_ms = HARDWARE_CAMERA_SPAWN_TIMEOUT_MS;
 	s_last_argv_count = 0;
 	memset(s_last_argv, 0, sizeof(s_last_argv));

src/hardware/hardware_camera.h

@@ -51,14 +51,18 @@ int hardware_camera_capture(board_id_t board, const char *camera_type,
 
 /**
  * Bind the file-tool workspace root used for caller-supplied capture paths.
- * NULL or empty disables the check (auto temp files and unit tests).
+ * NULL disables containment (workspace_only off). Non-NULL enables enforcement;
+ * an empty string fails closed (deny caller paths) — same as write_file when
+ * workspace_path is missing/empty under workspace_only.
+ *
+ * Example: hardware_camera_set_workspace("") denies /tmp/out.jpg; NULL allows it.
  */
 void hardware_camera_set_workspace(const char *workspace);
 
 /**
  * Return 1 if @p path may be used as a caller-supplied capture output.
- * Auto temp (NULL/empty) is always allowed. When a workspace is bound,
- * the path must resolve under that root (same policy as write_file).
+ * Auto temp (NULL/empty) is always allowed. When enforcement is on, the path
+ * must resolve under the bound root (same policy as write_file).
  */
 int hardware_camera_output_allowed(const char *path);
 

src/tools/file.c

@@ -2,6 +2,9 @@
  * @file file.c
  * @brief File tool: read_file, write_file, list_dir with workspace boundary check.
  */
+#if defined(__APPLE__)
+#define _DARWIN_C_SOURCE
+#endif
 #define _POSIX_C_SOURCE 200809L
 #define _GNU_SOURCE
 
@@ -10,6 +13,7 @@
 #include "core/config.h"
 #include "cJSON.h"
 #include <dirent.h>
+#include <fcntl.h>
 #include <libgen.h>
 #include <limits.h>
 #include <stdio.h>
@@ -31,49 +35,179 @@ void tool_file_set_config(const config_t *cfg)
 	g_file_cfg = cfg;
 }
 
+static int resolved_is_under_workspace(const char *resolved)
+{
+	char ws_resolved[PATH_MAX];
+	const char *workspace;
+	size_t ws_len;
+
+	if (!resolved || !g_file_cfg) return 0;
+	workspace = config_workspace_path(g_file_cfg);
+	if (!workspace || workspace[0] == '\0') return 0;
+	if (realpath(workspace, ws_resolved) == NULL) return 0;
+	ws_len = strlen(ws_resolved);
+	if (strncmp(resolved, ws_resolved, ws_len) != 0) return 0;
+	if (resolved[ws_len] != '\0' && resolved[ws_len] != '/') return 0;
+	return 1;
+}
+
+static int path_is_symlink(const char *path)
+{
+	struct stat lst;
+
+	if (!path) return 0;
+	if (lstat(path, &lst) != 0) return 0;
+	return S_ISLNK(lst.st_mode) ? 1 : 0;
+}
+
 static int path_within_workspace(const char *path, char *resolved, size_t resolved_size)
 {
+	const char *workspace;
+	char path_copy[PATH_MAX];
+
 	if (!path || path[0] == '\0') return 0;
 	if (!g_file_cfg || !config_workspace_only(g_file_cfg)) {
 		snprintf(resolved, resolved_size, "%s", path);
 		return 1;
 	}
-	const char *workspace = config_workspace_path(g_file_cfg);
-	if (!workspace || workspace[0] == '\0') {
-		return 0;  /* Deny: cannot validate without workspace path */
-	}
-	char ws_resolved[PATH_MAX];
-	if (realpath(workspace, ws_resolved) == NULL) return 0;
-	if (realpath(path, resolved) != NULL) {
-		size_t ws_len = strlen(ws_resolved);
-		if (strncmp(resolved, ws_resolved, ws_len) != 0) return 0;
-		if (resolved[ws_len] != '\0' && resolved[ws_len] != '/') return 0;
-		return 1;
-	}
-	char path_copy[PATH_MAX];
+	workspace = config_workspace_path(g_file_cfg);
+	if (!workspace || workspace[0] == '\0')
+		return 0;
+	if (realpath(path, resolved) != NULL)
+		return resolved_is_under_workspace(resolved);
+	/* Leaf symlink: ancestor prefix is not enough — open() would follow it. */
+	if (path_is_symlink(path))
+		return 0;
 	snprintf(path_copy, sizeof(path_copy), "%s", path);
 	for (;;) {
 		char *dir = dirname(path_copy);
 		if (!dir || dir[0] == '\0') break;
-		if (realpath(dir, resolved) != NULL) {
-			size_t ws_len = strlen(ws_resolved);
-			if (strncmp(resolved, ws_resolved, ws_len) != 0) return 0;
-			if (resolved[ws_len] != '\0' && resolved[ws_len] != '/') return 0;
-			return 1;
-		}
+		if (realpath(dir, resolved) != NULL)
+			return resolved_is_under_workspace(resolved);
 		if (strcmp(dir, ".") == 0 || strcmp(dir, "/") == 0) break;
 		snprintf(path_copy, sizeof(path_copy), "%s", dir);
 	}
 	return 0;
 }
 
+/*
+ * Membership via ancestor is not the write target. Using that ancestor as
+ * fopen() would truncate a file treated as a directory (#67).
+ */
+static int resolve_workspace_write_path(const char *path, char *safe_path, size_t cap)
+{
+	char parent[PATH_MAX];
+	char path_copy[PATH_MAX];
+	struct stat st;
+	const char *base;
+	char *dir;
+	int n;
+
+	if (path_is_symlink(path))
+		return 0;
+	if (realpath(path, safe_path) != NULL) {
+		if (stat(safe_path, &st) != 0 || !S_ISREG(st.st_mode)) return 0;
+		return resolved_is_under_workspace(safe_path);
+	}
+	if (snprintf(path_copy, sizeof(path_copy), "%s", path) >= (int)sizeof(path_copy))
+		return 0;
+	dir = dirname(path_copy);
+	if (!dir || realpath(dir, parent) == NULL) return 0;
+	if (stat(parent, &st) != 0 || !S_ISDIR(st.st_mode)) return 0;
+	if (!resolved_is_under_workspace(parent)) return 0;
+	base = strrchr(path, '/');
+	base = base ? base + 1 : path;
+	if (base[0] == '\0' || strcmp(base, ".") == 0 || strcmp(base, "..") == 0)
+		return 0;
+	n = snprintf(safe_path, cap, "%s/%s", parent, base);
+	return n > 0 && (size_t)n < cap;
+}
+
+static void discard_file_tmp(int fd, const char *tmp_path)
+{
+	if (fd >= 0)
+		(void)close(fd);
+	if (tmp_path && tmp_path[0] != '\0')
+		(void)unlink(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;
+}
+
+/*
+ * Unique temp+rename so O_TRUNC cannot wipe the live file, and a sibling
+ * named path.tmp is not used as the sidecar (ENOSPC, EFBIG, or a
+ * non-writable parent dir). mkstemp uses O_EXCL, so a planted symlink at
+ * the random name cannot be followed (the path.tmp #90 shape).
+ */
+static int write_file_atomic(const char *path, const char *content)
+{
+	char path_copy[PATH_MAX];
+	char tmp_path[PATH_MAX];
+	char *dir;
+	int fd;
+	int n;
+
+	if (!path || !content)
+		return -1;
+	if (snprintf(path_copy, sizeof(path_copy), "%s", path) >= (int)sizeof(path_copy))
+		return -1;
+	dir = dirname(path_copy);
+	if (!dir || dir[0] == '\0')
+		return -1;
+	n = snprintf(tmp_path, sizeof(tmp_path), "%s/.sc-write-XXXXXX", dir);
+	if (n < 0 || (size_t)n >= sizeof(tmp_path))
+		return -1;
+	fd = mkstemp(tmp_path);
+	if (fd < 0)
+		return -1;
+	(void)fcntl(fd, F_SETFD, FD_CLOEXEC);
+	if (fchmod(fd, 0644) != 0) {
+		discard_file_tmp(fd, 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;
+	}
+	return 0;
+}
+
 static int file_read(const char *path, char *result_buf, size_t max_len)
 {
 	char resolved[PATH_MAX];
+	int ws_only;
 	if (!path_within_workspace(path, resolved, sizeof(resolved))) {
 		snprintf(result_buf, max_len, "{\"error\":\"path outside workspace\"}");
 		return -1;
 	}
+	ws_only = g_file_cfg ? config_workspace_only(g_file_cfg) : 0;
+	if (ws_only && realpath(path, resolved) == NULL) {
+		snprintf(result_buf, max_len, "{\"error\":\"cannot open file\"}");
+		return -1;
+	}
 	FILE *f = fopen(resolved, "rb");
 	if (!f) {
 		snprintf(result_buf, max_len, "{\"error\":\"cannot open file\"}");
@@ -98,57 +232,41 @@ static int file_read(const char *path, char *result_buf, size_t max_len)
 static int file_write(const char *path, const char *content, char *result_buf, size_t max_len)
 {
 	char resolved[PATH_MAX];
+	int ws_only;
+	char safe_path[PATH_MAX];
+
 	if (!path_within_workspace(path, resolved, sizeof(resolved))) {
 		snprintf(result_buf, max_len, "{\"error\":\"path outside workspace\"}");
 		return -1;
 	}
-	int ws_only = g_file_cfg ? config_workspace_only(g_file_cfg) : 0;
-	char safe_path[PATH_MAX];
+	ws_only = g_file_cfg ? config_workspace_only(g_file_cfg) : 0;
 	if (!ws_only) {
 		snprintf(safe_path, sizeof(safe_path), "%s", path);
-	} else {
-		struct stat st;
-		if (stat(resolved, &st) == 0 && S_ISREG(st.st_mode)) {
-			snprintf(safe_path, sizeof(safe_path), "%s", resolved);
-		} else {
-			const char *base = strrchr(path, '/');
-			base = base ? base + 1 : path;
-			size_t res_len = strlen(resolved);
-			size_t base_len = strlen(base);
-			if (res_len + 1 + base_len >= sizeof(safe_path)) {
-				snprintf(result_buf, max_len, "{\"error\":\"path too long\"}");
-				return -1;
-			}
-			memcpy(safe_path, resolved, res_len);
-			safe_path[res_len] = '/';
-			memcpy(safe_path + res_len + 1, base, base_len + 1);
-		}
-	}
-	FILE *f = fopen(safe_path, "w");
-	if (!f) {
+	} else if (!resolve_workspace_write_path(path, safe_path, sizeof(safe_path))) {
 		snprintf(result_buf, max_len, "{\"error\":\"cannot write file\"}");
 		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;
-		}
+	if (write_file_atomic(safe_path, content ? content : "") != 0) {
+		snprintf(result_buf, max_len, "{\"error\":\"write failed\"}");
+		return -1;
 	}
-	fclose(f);
 	snprintf(result_buf, max_len, "{\"status\":\"ok\"}");
 	return 0;
 }
 
 static int file_list(const char *path, char *result_buf, size_t max_len)
 {
 	char resolved[PATH_MAX];
+	int ws_only;
 	if (!path_within_workspace(path, resolved, sizeof(resolved))) {
 		snprintf(result_buf, max_len, "{\"error\":\"path outside workspace\"}");
 		return -1;
 	}
+	ws_only = g_file_cfg ? config_workspace_only(g_file_cfg) : 0;
+	if (ws_only && realpath(path, resolved) == NULL) {
+		snprintf(result_buf, max_len, "{\"error\":\"cannot list directory\"}");
+		return -1;
+	}
 	DIR *d = opendir(resolved);
 	if (!d) {
 		snprintf(result_buf, max_len, "{\"error\":\"cannot list directory\"}");

src/tools/hardware_tools.c

@@ -100,10 +100,14 @@ static const size_t HARDWARE_TOOL_COUNT =
 void tool_hardware_set_config(const config_t *cfg)
 {
 	g_hw_cfg = cfg;
-	if (cfg && config_workspace_only(cfg))
-		hardware_camera_set_workspace(config_workspace_path(cfg));
-	else
+	if (cfg && config_workspace_only(cfg)) {
+		const char *ws = config_workspace_path(cfg);
+
+		/* Pass "" when path is missing so camera fails closed like write_file. */
+		hardware_camera_set_workspace(ws ? ws : "");
+	} else {
 		hardware_camera_set_workspace(NULL);
+	}
 }
 
 size_t tool_hardware_get_all(const tool_t **out, size_t max_count)