diff --git a/CHANGELOG.md b/CHANGELOG.md index 331ff2d..cc2e71b 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/src/hardware/hardware_camera.c b/src/hardware/hardware_camera.c index 5fbf95d..f7e57f9 100644 --- a/src/hardware/hardware_camera.c +++ b/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)); diff --git a/src/hardware/hardware_camera.h b/src/hardware/hardware_camera.h index 48bb014..67156f6 100644 --- a/src/hardware/hardware_camera.h +++ b/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); diff --git a/src/tools/file.c b/src/tools/file.c index e8170b1..2fd08dd 100644 --- a/src/tools/file.c +++ b/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 +#include #include #include #include @@ -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,46 +232,24 @@ 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; } @@ -145,10 +257,16 @@ static int file_write(const char *path, const char *content, char *result_buf, s 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\"}"); diff --git a/src/tools/hardware_tools.c b/src/tools/hardware_tools.c index 1667e15..9289be9 100644 --- a/src/tools/hardware_tools.c +++ b/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) diff --git a/tests/test_file.c b/tests/test_file.c index b4087ac..a21bb2e 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -196,6 +196,280 @@ static void test_symlink_escape_rejected(void) rmdir(tmpdir); } +static int slurp_file(const char *path, char *buf, size_t cap) +{ + FILE *f = fopen(path, "rb"); + size_t n; + if (!f) return -1; + n = fread(buf, 1, cap - 1, f); + fclose(f); + buf[n] = '\0'; + return 0; +} + +static void test_write_does_not_truncate_file_used_as_directory(void) +{ + char tmpdir[PATH_MAX]; + char victim[PATH_MAX]; + char nested[PATH_MAX]; + char config_path[PATH_MAX]; + char args[PATH_MAX + 128]; + char buf[256]; + char kept[64]; + config_t *cfg; + const tool_t *t; + FILE *f; + int r; + + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_filedir_%d", (int)getpid()); + if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return; + snprintf(victim, sizeof(victim), "%s/important.md", tmpdir); + f = fopen(victim, "w"); + MU_ASSERT(f != NULL, "create victim file"); + fputs("KEEP", f); + fclose(f); + cfg = make_ws_config(tmpdir, config_path, sizeof(config_path)); + MU_ASSERT(cfg != NULL, "file-as-dir: load config"); + tool_file_set_config(cfg); + t = tool_file_get(); + snprintf(nested, sizeof(nested), "%s/important.md/nested.txt", tmpdir); + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"PWNED\"}", nested); + r = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(r == -1, "write through file-as-directory is rejected"); + MU_ASSERT(slurp_file(victim, kept, sizeof(kept)) == 0, "victim still readable"); + MU_ASSERT(strcmp(kept, "KEEP") == 0, "victim content preserved"); + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", nested); + r = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(r == -1, "read through file-as-directory is rejected"); + MU_ASSERT(strstr(buf, "KEEP") == NULL, "read does not leak victim contents"); + config_free(cfg); + unlink(config_path); + unlink(victim); + rmdir(tmpdir); +} + +static void test_write_does_not_collapse_missing_parent_onto_basename(void) +{ + char tmpdir[PATH_MAX]; + char victim[PATH_MAX]; + char nested[PATH_MAX]; + char config_path[PATH_MAX]; + char args[PATH_MAX + 128]; + char buf[256]; + char kept[64]; + config_t *cfg; + const tool_t *t; + FILE *f; + int r; + + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_missdir_%d", (int)getpid()); + if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return; + snprintf(victim, sizeof(victim), "%s/notes.md", tmpdir); + f = fopen(victim, "w"); + MU_ASSERT(f != NULL, "create same-basename victim"); + fputs("KEEP", f); + fclose(f); + cfg = make_ws_config(tmpdir, config_path, sizeof(config_path)); + MU_ASSERT(cfg != NULL, "missing-parent: load config"); + tool_file_set_config(cfg); + t = tool_file_get(); + snprintf(nested, sizeof(nested), "%s/missing_dir/notes.md", tmpdir); + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"PWNED\"}", nested); + r = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(r == -1, "write with missing parent is rejected"); + MU_ASSERT(slurp_file(victim, kept, sizeof(kept)) == 0, "workspace notes.md still readable"); + MU_ASSERT(strcmp(kept, "KEEP") == 0, "missing parent does not overwrite same basename"); + config_free(cfg); + unlink(config_path); + unlink(victim); + rmdir(tmpdir); +} + +static void test_file_write_dangling_symlink_rejected(void) +{ + char tmpdir[PATH_MAX]; + char outside[PATH_MAX]; + char link_path[PATH_MAX]; + char config_path[PATH_MAX]; + char args[PATH_MAX + 128]; + char buf[256]; + config_t *cfg; + const tool_t *t; + struct stat st; + int r; + + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_dangle_%d", (int)getpid()); + if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return; + snprintf(outside, sizeof(outside), "/tmp/sc_file_pwned_%d", (int)getpid()); + unlink(outside); + snprintf(link_path, sizeof(link_path), "%s/leak", tmpdir); + unlink(link_path); + if (symlink(outside, link_path) != 0) { + rmdir(tmpdir); + return; + } + cfg = make_ws_config(tmpdir, config_path, sizeof(config_path)); + MU_ASSERT(cfg != NULL, "dangling symlink: load config"); + tool_file_set_config(cfg); + t = tool_file_get(); + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"pwned\"}", + link_path); + r = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(r == -1, "write through dangling symlink rejected"); + MU_ASSERT(stat(outside, &st) != 0, "host path outside workspace not created"); + config_free(cfg); + unlink(config_path); + unlink(link_path); + unlink(outside); + rmdir(tmpdir); +} + +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 + 128]; + char buf[256]; + config_t *cfg; + const tool_t *t; + int write_ret; + + /* + * Directory without write permission: creating the mkstemp sidecar + * fails, but fopen/open O_TRUNC on the existing file still succeeds. + * Atomic replace must leave the original body intact. + */ + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_nowrite_%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, "nowrite: 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"); + + MU_ASSERT(chmod(tmpdir, 0555) == 0, "make workspace dir non-writable"); + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"should not land\"}", + notes_path); + write_ret = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(chmod(tmpdir, 0755) == 0, "restore workspace dir mode"); + + MU_ASSERT(write_ret != 0, "write into non-writable dir 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 chmod\"}", + notes_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == 0, "write recovers after chmod"); + 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 chmod") != 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); +} + +static void test_write_does_not_clobber_sibling_tmp(void) +{ + char tmpdir[PATH_MAX]; + char notes_path[PATH_MAX]; + char sibling_tmp[PATH_MAX]; + char config_path[PATH_MAX]; + char args[PATH_MAX + 128]; + char buf[256]; + char kept[64]; + config_t *cfg; + const tool_t *t; + FILE *f; + + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_sibtmp_%d", (int)getpid()); + if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return; + snprintf(notes_path, sizeof(notes_path), "%s/notes.md", tmpdir); + snprintf(sibling_tmp, sizeof(sibling_tmp), "%s/notes.md.tmp", tmpdir); + f = fopen(sibling_tmp, "w"); + MU_ASSERT(f != NULL, "create sibling notes.md.tmp"); + fputs("SIBLING KEEP", f); + fclose(f); + cfg = make_ws_config(tmpdir, config_path, sizeof(config_path)); + MU_ASSERT(cfg != NULL, "sibling tmp: load config"); + tool_file_set_config(cfg); + t = tool_file_get(); + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"NEW NOTES\"}", + notes_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == 0, "write notes.md succeeds"); + MU_ASSERT(slurp_file(notes_path, kept, sizeof(kept)) == 0, "notes.md readable"); + MU_ASSERT(strcmp(kept, "NEW NOTES") == 0, "notes.md has new content"); + MU_ASSERT(slurp_file(sibling_tmp, kept, sizeof(kept)) == 0, "sibling tmp still readable"); + MU_ASSERT(strcmp(kept, "SIBLING KEEP") == 0, "write must not O_TRUNC notes.md.tmp"); + config_free(cfg); + unlink(config_path); + unlink(notes_path); + unlink(sibling_tmp); + rmdir(tmpdir); +} + +static void test_write_in_workspace_symlink_alias_rejected(void) +{ + char tmpdir[PATH_MAX]; + char notes_path[PATH_MAX]; + char alias_path[PATH_MAX]; + char config_path[PATH_MAX]; + char args[PATH_MAX + 128]; + char buf[256]; + char kept[64]; + config_t *cfg; + const tool_t *t; + FILE *f; + + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_alias_%d", (int)getpid()); + if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return; + snprintf(notes_path, sizeof(notes_path), "%s/notes.md", tmpdir); + snprintf(alias_path, sizeof(alias_path), "%s/alias.md", tmpdir); + f = fopen(notes_path, "w"); + MU_ASSERT(f != NULL, "create notes.md"); + fputs("KEEP", f); + fclose(f); + unlink(alias_path); + if (symlink(notes_path, alias_path) != 0 && symlink("notes.md", alias_path) != 0) { + unlink(notes_path); + rmdir(tmpdir); + return; + } + cfg = make_ws_config(tmpdir, config_path, sizeof(config_path)); + MU_ASSERT(cfg != NULL, "alias: load config"); + tool_file_set_config(cfg); + t = tool_file_get(); + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"PWNED\"}", + alias_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == -1, + "write through in-workspace symlink alias is rejected"); + MU_ASSERT(slurp_file(notes_path, kept, sizeof(kept)) == 0, "notes.md still readable"); + MU_ASSERT(strcmp(kept, "KEEP") == 0, "alias write must not change notes.md"); + config_free(cfg); + unlink(config_path); + unlink(alias_path); + unlink(notes_path); + rmdir(tmpdir); +} + int main(void) { MU_RUN(test_file_read_write_list); @@ -203,6 +477,12 @@ 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_write_does_not_truncate_file_used_as_directory); + MU_RUN(test_write_does_not_collapse_missing_parent_onto_basename); + MU_RUN(test_file_write_dangling_symlink_rejected); + MU_RUN(test_file_write_failure_preserves_existing); + MU_RUN(test_write_does_not_clobber_sibling_tmp); + MU_RUN(test_write_in_workspace_symlink_alias_rejected); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; } diff --git a/tests/test_hardware_camera.c b/tests/test_hardware_camera.c index 3e27175..46fb2c1 100644 --- a/tests/test_hardware_camera.c +++ b/tests/test_hardware_camera.c @@ -7,6 +7,7 @@ #include "hardware/hardware_camera.h" #include #include +#include #include static char s_spawn_out_path[256]; @@ -214,6 +215,31 @@ static int test_output_path_outside_workspace_rejected(void) return 0; } +static int test_empty_workspace_enforced_denies_outside(void) +{ + char result[256]; + char err[128]; + const char *outside = "/tmp/shellclaw_cam_empty_ws_escape.jpg"; + + unlink(outside); + /* Non-NULL empty string: workspace_only on, path missing/empty. */ + hardware_camera_set_workspace(""); + ASSERT(hardware_camera_output_allowed(outside) == 0); + RUN(setup_mock()); + ASSERT(hardware_camera_capture(BOARD_JETSON_ORIN_NANO, "csi", "640x480", 75, 0, 0, + outside, result, sizeof(result), err, + sizeof(err)) == -1); + ASSERT(strstr(err, "workspace") != NULL); + ASSERT(s_spawn_called == 0); + ASSERT(access(outside, F_OK) != 0); + teardown(); + hardware_camera_init(); + hardware_camera_set_workspace(NULL); + ASSERT(hardware_camera_output_allowed(outside) == 1); + hardware_camera_shutdown(); + return 0; +} + static int test_output_path_inside_workspace_allowed(void) { char result[256]; @@ -234,6 +260,35 @@ static int test_output_path_inside_workspace_allowed(void) return 0; } +static int test_output_dangling_symlink_rejected(void) +{ + char result[256]; + char err[128]; + char ws[128]; + char link_path[256]; + char outside[256]; + struct stat st; + + ASSERT(test_runner_mkdtemp_path("shellclaw_cam_dangle", ws, sizeof(ws)) == 0); + snprintf(outside, sizeof(outside), "/tmp/sc_cam_pwned_%d.jpg", (int)getpid()); + unlink(outside); + snprintf(link_path, sizeof(link_path), "%s/shot.jpg", ws); + ASSERT(symlink(outside, link_path) == 0); + hardware_camera_set_workspace(ws); + RUN(setup_mock()); + ASSERT(hardware_camera_capture(BOARD_JETSON_ORIN_NANO, "csi", "640x480", 75, 0, 0, + link_path, result, sizeof(result), err, + sizeof(err)) == -1); + ASSERT(strstr(err, "workspace") != NULL); + ASSERT(s_spawn_called == 0); + ASSERT(stat(outside, &st) != 0); + teardown(); + unlink(link_path); + unlink(outside); + rmdir(ws); + return 0; +} + static int test_output_path_traversal_rejected(void) { char result[256]; @@ -605,7 +660,9 @@ int main(void) RUN(test_capture_argument_validation()); RUN(test_unsafe_output_path_rejected()); RUN(test_output_path_outside_workspace_rejected()); + RUN(test_empty_workspace_enforced_denies_outside()); RUN(test_output_path_inside_workspace_allowed()); + RUN(test_output_dangling_symlink_rejected()); RUN(test_output_path_traversal_rejected()); RUN(test_resolution_injection_rejected()); RUN(test_camera_type_injection_rejected());