From 48c03f511d76c0cbc2ca9722cb1c1754048b83aa Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 3 Sep 2026 11:30:19 +0000 Subject: [PATCH 1/3] fix(sandbox): keep agent workspace off gateway state files Default workspace_path was ~/.shellclaw, the same tree as pairing tokens, memory.db, and config.toml. With workspace_only on, inbound Discord/webchat/cron file tools could read or overwrite those files. Point the default at ~/.shellclaw/workspace and deny runtime state paths even when an operator keeps the old workspace root. Co-authored-by: esadrianno --- Makefile | 4 +- config.example.toml | 2 + src/core/bootstrap.c | 28 ++++++++++++ src/core/config.c | 4 +- src/sandbox/allowlist.c | 70 +++++++++++++++++++++++------ src/sandbox/allowlist.h | 13 ++++++ src/tools/file.c | 35 ++++++++++++++- src/tools/shell.c | 1 + tests/test_allowlist.c | 74 +++++++++++++++++++++++++++++++ tests/test_config.c | 8 ++++ tests/test_file.c | 97 +++++++++++++++++++++++++++++++++++++++++ tests/test_shell.c | 11 +++++ 12 files changed, 330 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index 2a2d541..4eaa717 100644 --- a/Makefile +++ b/Makefile @@ -434,9 +434,9 @@ test_shell: tests/test_shell.c $(SHELL_O) $(SANDBOX_O) $(ALLOWLIST_O) $(CONFIG_O $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_shell.c $(SHELL_O) $(SANDBOX_O) $(ALLOWLIST_O) $(CONFIG_O) $(TOML_O) $(CJSON_O) $(LDLIBS) $(DSYM_SCRIPT) -test_file: tests/test_file.c $(FILE_O) $(REGISTRY_O) $(CONFIG_O) $(TOML_O) $(CJSON_O) +test_file: tests/test_file.c $(FILE_O) $(REGISTRY_O) $(ALLOWLIST_O) $(CONFIG_O) $(TOML_O) $(CJSON_O) @mkdir -p $(BINDIR) - $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_file.c $(FILE_O) $(CONFIG_O) $(TOML_O) $(CJSON_O) $(LDLIBS) + $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_file.c $(FILE_O) $(ALLOWLIST_O) $(CONFIG_O) $(TOML_O) $(CJSON_O) $(LDLIBS) $(DSYM_SCRIPT) $(CHANNEL_TG_TEST_O): src/channels/telegram.c src/channels/channel.h src/core/config.h diff --git a/config.example.toml b/config.example.toml index 267c89c..142fd37 100644 --- a/config.example.toml +++ b/config.example.toml @@ -52,6 +52,8 @@ memory_limit_mb = 64 cpu_limit_percent = 50 network = false workspace_only = true +# Separate from ~/.shellclaw so file/shell tools cannot read pairing tokens or memory.db. +workspace_path = "~/.shellclaw/workspace" [heartbeat] enabled = true diff --git a/src/core/bootstrap.c b/src/core/bootstrap.c index e7fe294..cbc3a59 100644 --- a/src/core/bootstrap.c +++ b/src/core/bootstrap.c @@ -15,8 +15,12 @@ #include "gateway/http.h" #include "gateway/ws.h" #endif +#include +#include #include #include +#include +#include #define SKILLS_BUF_SIZE (256 * 1024) #define SYSTEM_PROMPT_BUF_SIZE (256 * 1024) @@ -211,8 +215,32 @@ static void channels_cleanup(void) g_cfg = NULL; } +static void ensure_workspace_directory(const char *workspace) +{ + char parent[PATH_MAX]; + const char *slash; + size_t parent_len; + + if (!workspace || !workspace[0]) return; + slash = strrchr(workspace, '/'); + if (slash && slash != workspace) { + parent_len = (size_t)(slash - workspace); + if (parent_len < sizeof(parent)) { + memcpy(parent, workspace, parent_len); + parent[parent_len] = '\0'; + if (mkdir(parent, 0700) != 0 && errno != EEXIST) + fprintf(stderr, "shellclaw: mkdir %s: %s\n", + parent, strerror(errno)); + } + } + if (mkdir(workspace, 0700) != 0 && errno != EEXIST) + fprintf(stderr, "shellclaw: mkdir workspace %s: %s\n", + workspace, strerror(errno)); +} + int tools_init(const config_t *cfg) { + ensure_workspace_directory(config_workspace_path(cfg)); tool_set_config(cfg); g_tool_count = tool_get_all(g_tools, MAX_TOOLS); return 0; diff --git a/src/core/config.c b/src/core/config.c index 440ffb5..4af729d 100644 --- a/src/core/config.c +++ b/src/core/config.c @@ -739,7 +739,9 @@ int config_load(const char *path, config_t **out, char *errbuf, size_t errbufsz) cfg->workspace_only = 1; cfg->gateway_port = DEFAULT_GATEWAY_PORT; set_string(&cfg->gateway_host, "127.0.0.1"); - set_string(&cfg->workspace_path, "~/.shellclaw"); + /* Keep tool workspace off the state dir (~/.shellclaw) so pairing tokens, + * memory.db, and config.toml are outside workspace_only by default. */ + set_string(&cfg->workspace_path, "~/.shellclaw/workspace"); set_string(&cfg->asap_agent_urn, "urn:asap:agent:shellclaw"); set_string(&cfg->asap_agent_name, "ShellClaw"); cfg->heartbeat_interval_minutes = 30; diff --git a/src/sandbox/allowlist.c b/src/sandbox/allowlist.c index c3033ce..79318c5 100644 --- a/src/sandbox/allowlist.c +++ b/src/sandbox/allowlist.c @@ -53,6 +53,9 @@ static const char *const BLOCK_SUBSTRINGS[] = { "~/.ssh/id_", "id_rsa", "id_ed25519", + "auth_tokens.json", + "shellclaw.pid", + "shellclaw.log", NULL }; @@ -113,6 +116,40 @@ int allowlist_path_is_under_workspace(const char *path, const char *workspace_ro return 0; } +int allowlist_path_is_runtime_state_file(const char *path) +{ + char resolved[PATH_MAX]; + const char *use = path; + const char *base; + const char *slash; + char parent[PATH_MAX]; + size_t parent_len; + + if (!path || !path[0]) + return 0; + if (realpath(path, resolved) != NULL) + use = resolved; + base = strrchr(use, '/'); + base = base ? base + 1 : use; + if (strcmp(base, "auth_tokens.json") == 0 || + strcmp(base, "shellclaw.pid") == 0 || + strcmp(base, "shellclaw.log") == 0) + return 1; + if (strcmp(base, "config.toml") != 0 && strcmp(base, "memory.db") != 0) + return 0; + slash = strrchr(use, '/'); + if (!slash || slash == use) + return 0; + parent_len = (size_t)(slash - use); + if (parent_len >= sizeof(parent)) + return 0; + memcpy(parent, use, parent_len); + parent[parent_len] = '\0'; + slash = strrchr(parent, '/'); + slash = slash ? slash + 1 : parent; + return strcmp(slash, ".shellclaw") == 0; +} + /* ------------------------------------------------------------------ */ /* Public: combined check */ /* ------------------------------------------------------------------ */ @@ -166,21 +203,28 @@ int allowlist_check_shell_command(const char *cmd, const allowlist_config_t *cfg if (!cmd_copy) return 0; /* fail-open on OOM */ tok = strtok_r(cmd_copy, " \t\n;|&><", &saveptr); while (tok) { + char expanded[PATH_MAX]; + const char *check = tok; + if (tok[0] == '~') { + const char *home = getenv("HOME"); + if (home) + snprintf(expanded, sizeof(expanded), "%s%s", home, tok + 1); + else + snprintf(expanded, sizeof(expanded), "%s", tok); + check = expanded; + } + if (allowlist_path_is_runtime_state_file(check)) { + set_reason(reason_buf, reason_cap, + "command blocked: runtime state file: ", check); + fprintf(stderr, "allowlist: blocked runtime state file: %s\n", check); + free(cmd_copy); + return 1; + } if (has_path_chars(tok)) { - /* Expand a leading tilde naively */ - char expanded[PATH_MAX]; - if (tok[0] == '~') { - const char *home = getenv("HOME"); - if (home) - snprintf(expanded, sizeof(expanded), "%s%s", home, tok + 1); - else - snprintf(expanded, sizeof(expanded), "%s", tok); - tok = expanded; - } - if (!allowlist_path_is_under_workspace(tok, workspace_root)) { + if (!allowlist_path_is_under_workspace(check, workspace_root)) { set_reason(reason_buf, reason_cap, - "command blocked: path escapes workspace: ", tok); - fprintf(stderr, "allowlist: blocked path outside workspace: %s\n", tok); + "command blocked: path escapes workspace: ", check); + fprintf(stderr, "allowlist: blocked path outside workspace: %s\n", check); free(cmd_copy); return 1; } diff --git a/src/sandbox/allowlist.h b/src/sandbox/allowlist.h index 76a4e4f..5f64c2e 100644 --- a/src/sandbox/allowlist.h +++ b/src/sandbox/allowlist.h @@ -66,6 +66,19 @@ int allowlist_check_shell_command(const char *cmd, const allowlist_config_t *cfg */ int allowlist_path_is_under_workspace(const char *path, const char *workspace_root); +/** + * Return 1 if @p path is a ShellClaw runtime state file that tools must not touch. + * + * Always reserved by basename: auth_tokens.json, shellclaw.pid, shellclaw.log. + * Also reserved when the parent directory is named `.shellclaw`: config.toml, memory.db. + * + * @param path Absolute, relative, or unresolved path (realpath used when the file exists). + * @return 1 if reserved, 0 otherwise. + * + * Example: allowlist_path_is_runtime_state_file("/home/me/.shellclaw/auth_tokens.json") == 1 + */ +int allowlist_path_is_runtime_state_file(const char *path); + #ifdef __cplusplus } #endif diff --git a/src/tools/file.c b/src/tools/file.c index e8170b1..c491aa6 100644 --- a/src/tools/file.c +++ b/src/tools/file.c @@ -8,6 +8,7 @@ #include "tools/tool.h" #include "tools/file.h" #include "core/config.h" +#include "sandbox/allowlist.h" #include "cJSON.h" #include #include @@ -31,11 +32,41 @@ void tool_file_set_config(const config_t *cfg) g_file_cfg = cfg; } +static int path_matches_memory_db(const char *candidate) +{ + const char *db; + char db_resolved[PATH_MAX]; + char cand_resolved[PATH_MAX]; + + if (!g_file_cfg || !candidate || !candidate[0]) return 0; + db = config_memory_db_path(g_file_cfg); + if (!db || !db[0]) return 0; + if (strcmp(candidate, db) == 0) return 1; + if (realpath(db, db_resolved) == NULL) return 0; + if (strcmp(candidate, db_resolved) == 0) return 1; + if (realpath(candidate, cand_resolved) != NULL && + strcmp(cand_resolved, db_resolved) == 0) + return 1; + return 0; +} + +static int path_is_reserved_runtime_state(const char *path, const char *resolved) +{ + if (path && allowlist_path_is_runtime_state_file(path)) return 1; + if (resolved && resolved[0] && allowlist_path_is_runtime_state_file(resolved)) + return 1; + if (path_matches_memory_db(path) || path_matches_memory_db(resolved)) + return 1; + return 0; +} + static int path_within_workspace(const char *path, char *resolved, size_t resolved_size) { if (!path || path[0] == '\0') return 0; if (!g_file_cfg || !config_workspace_only(g_file_cfg)) { - snprintf(resolved, resolved_size, "%s", path); + if (realpath(path, resolved) == NULL) + snprintf(resolved, resolved_size, "%s", path); + if (path_is_reserved_runtime_state(path, resolved)) return 0; return 1; } const char *workspace = config_workspace_path(g_file_cfg); @@ -48,6 +79,7 @@ static int path_within_workspace(const char *path, char *resolved, size_t resolv 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; + if (path_is_reserved_runtime_state(path, resolved)) return 0; return 1; } char path_copy[PATH_MAX]; @@ -59,6 +91,7 @@ static int path_within_workspace(const char *path, char *resolved, size_t resolv 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; + if (path_is_reserved_runtime_state(path, resolved)) return 0; return 1; } if (strcmp(dir, ".") == 0 || strcmp(dir, "/") == 0) break; diff --git a/src/tools/shell.c b/src/tools/shell.c index 89f125d..4197928 100644 --- a/src/tools/shell.c +++ b/src/tools/shell.c @@ -41,6 +41,7 @@ static const char *const FALLBACK_BLOCKLIST[] = { "rm -rf /", "rm -rf / ", "rm -rf /$", "rm -rf /*", "mkfs", "dd if=", "dd of=", "shutdown", "reboot", ":(){ :|:& };:", "fork()", "> /dev/sd", + "auth_tokens.json", "shellclaw.pid", "shellclaw.log", NULL }; diff --git a/tests/test_allowlist.c b/tests/test_allowlist.c index 5d2ac74..77211ed 100644 --- a/tests/test_allowlist.c +++ b/tests/test_allowlist.c @@ -89,6 +89,77 @@ static int test_null_command_blocked(void) return 0; } +static int test_block_auth_tokens_json(void) +{ + char reason[256]; + ASSERT(allowlist_check_shell_command("cat ~/.shellclaw/auth_tokens.json", + NULL, reason, sizeof(reason)) == 1); + ASSERT(allowlist_path_is_runtime_state_file("auth_tokens.json") == 1); + return 0; +} + +static int test_block_state_dir_config_and_memory(void) +{ + char dir[] = "/tmp/sc_al_state_XXXXXX"; + char state[256]; + char cfg_path[256]; + char db_path[256]; + char *tmp; + FILE *f; + allowlist_config_t acfg; + char cmd[512]; + + tmp = mkdtemp(dir); + if (!tmp) { + fprintf(stderr, "test_block_state_dir_config_and_memory: mkdtemp failed\n"); + return 1; + } + snprintf(state, sizeof(state), "%s/.shellclaw", tmp); + if (mkdir(state, 0755) != 0) { + rmdir(tmp); + return 1; + } + snprintf(cfg_path, sizeof(cfg_path), "%s/config.toml", state); + snprintf(db_path, sizeof(db_path), "%s/memory.db", state); + f = fopen(cfg_path, "w"); + if (!f) { + rmdir(state); + rmdir(tmp); + return 1; + } + fputs("x=1\n", f); + fclose(f); + f = fopen(db_path, "w"); + if (!f) { + unlink(cfg_path); + rmdir(state); + rmdir(tmp); + return 1; + } + fputs("db", f); + fclose(f); + ASSERT(allowlist_path_is_runtime_state_file(cfg_path) == 1); + ASSERT(allowlist_path_is_runtime_state_file(db_path) == 1); + acfg.workspace_path = state; + acfg.workspace_only = 1; + snprintf(cmd, sizeof(cmd), "cat %s", cfg_path); + ASSERT(allowlist_check_shell_command(cmd, &acfg, NULL, 0) == 1); + snprintf(cmd, sizeof(cmd), "cat %s", db_path); + ASSERT(allowlist_check_shell_command(cmd, &acfg, NULL, 0) == 1); + unlink(cfg_path); + unlink(db_path); + rmdir(state); + rmdir(tmp); + return 0; +} + +static int test_allow_project_config_toml(void) +{ + ASSERT(allowlist_path_is_runtime_state_file("/tmp/project/config.toml") == 0); + ASSERT(allowlist_path_is_runtime_state_file("/tmp/project/memory.db") == 0); + return 0; +} + /* ------------------------------------------------------------------ */ /* Workspace path containment */ /* ------------------------------------------------------------------ */ @@ -200,6 +271,9 @@ int main(void) RUN(test_allow_safe_command()); RUN(test_allow_echo()); RUN(test_null_command_blocked()); + RUN(test_block_auth_tokens_json()); + RUN(test_block_state_dir_config_and_memory()); + RUN(test_allow_project_config_toml()); RUN(test_path_inside_workspace()); RUN(test_path_outside_workspace()); RUN(test_path_prefix_no_slash()); diff --git a/tests/test_config.c b/tests/test_config.c index 6a27aad..a065386 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -106,6 +106,14 @@ static int test_defaults(void) ASSERT(ret == 0); ASSERT(config_agent_max_tool_iterations(cfg) == 20); ASSERT(config_agent_max_context_messages(cfg) == 40); + { + const char *ws = config_workspace_path(cfg); + size_t n; + ASSERT(ws != NULL); + n = strlen(ws); + ASSERT(n >= 10); + ASSERT(strcmp(ws + n - 10, "/workspace") == 0); + } config_free(cfg); remove(path); return 0; diff --git a/tests/test_file.c b/tests/test_file.c index b4087ac..33e6074 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -196,6 +196,102 @@ static void test_symlink_escape_rejected(void) rmdir(tmpdir); } +static int write_text_file(const char *path, const char *content) +{ + FILE *f = fopen(path, "w"); + if (!f) return -1; + if (fputs(content, f) == EOF) { + fclose(f); + return -1; + } + fclose(f); + return 0; +} + +static void test_runtime_state_files_rejected_inside_workspace(void) +{ + char tmpdir[PATH_MAX]; + char state_dir[PATH_MAX]; + char token_path[PATH_MAX]; + char config_toml[PATH_MAX]; + char memory_db[PATH_MAX]; + char ok_path[PATH_MAX]; + char config_path[PATH_MAX]; + char args[PATH_MAX + 80]; + char buf[256]; + config_t *cfg; + const tool_t *t; + int r; + + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_state_%d", (int)getpid()); + if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return; + snprintf(state_dir, sizeof(state_dir), "%s/.shellclaw", tmpdir); + if (mkdir(state_dir, 0755) != 0 && errno != EEXIST) { + rmdir(tmpdir); + return; + } + snprintf(token_path, sizeof(token_path), "%s/auth_tokens.json", state_dir); + snprintf(config_toml, sizeof(config_toml), "%s/config.toml", state_dir); + snprintf(memory_db, sizeof(memory_db), "%s/memory.db", state_dir); + snprintf(ok_path, sizeof(ok_path), "%s/notes.txt", state_dir); + MU_ASSERT(write_text_file(token_path, "[{\"token\":\"secret-pair\"}]") == 0, + "write auth_tokens.json"); + MU_ASSERT(write_text_file(config_toml, "model=\"x\"\n") == 0, "write config.toml"); + MU_ASSERT(write_text_file(memory_db, "sqlite") == 0, "write memory.db"); + MU_ASSERT(write_text_file(ok_path, "ok") == 0, "write notes.txt"); + + { + char cwd[PATH_MAX]; + FILE *f; + MU_ASSERT(getcwd(cwd, sizeof(cwd)) != NULL, "getcwd"); + snprintf(config_path, sizeof(config_path), "%s/build/test_file_state.toml", cwd); + f = fopen(config_path, "w"); + MU_ASSERT(f != NULL, "create config"); + fprintf(f, + "[agent]\nmodel=\"x\"\n[memory]\ndb_path=\"%s\"\n" + "[sandbox]\nworkspace_only=true\nworkspace_path=\"%s\"\n", + memory_db, state_dir); + fclose(f); + } + cfg = NULL; + config_load(config_path, &cfg, NULL, 0); + MU_ASSERT(cfg != NULL, "load config with state-dir workspace"); + tool_file_set_config(cfg); + t = tool_file_get(); + + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", token_path); + r = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(r == -1, "read auth_tokens.json rejected"); + MU_ASSERT(strstr(buf, "secret-pair") == NULL, "token secret not returned"); + + snprintf(args, sizeof(args), + "{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"[]\"}", token_path); + r = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(r == -1, "write auth_tokens.json rejected"); + + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", config_toml); + r = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(r == -1, "read state config.toml rejected"); + + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", memory_db); + r = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(r == -1, "read memory.db rejected"); + + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", ok_path); + r = t->execute(args, buf, sizeof(buf)); + MU_ASSERT(r == 0, "read notes.txt in workspace still allowed"); + MU_ASSERT(strcmp(buf, "ok") == 0, "notes.txt content matches"); + + config_free(cfg); + unlink(config_path); + unlink(token_path); + unlink(config_toml); + unlink(memory_db); + unlink(ok_path); + rmdir(state_dir); + rmdir(tmpdir); +} + int main(void) { MU_RUN(test_file_read_write_list); @@ -203,6 +299,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_runtime_state_files_rejected_inside_workspace); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; } diff --git a/tests/test_shell.c b/tests/test_shell.c index 8b43d94..2f52b2f 100644 --- a/tests/test_shell.c +++ b/tests/test_shell.c @@ -60,6 +60,16 @@ static void test_shell_invalid_json(void) MU_ASSERT(strstr(buf, "error") != NULL, "error in output"); } +static void test_shell_blocked_auth_tokens(void) +{ + const tool_t *t = tool_shell_get(); + char buf[256]; + buf[0] = '\0'; + tool_shell_set_config(NULL); + (void)t->execute("{\"command\":\"cat ~/.shellclaw/auth_tokens.json\"}", buf, sizeof(buf)); + MU_ASSERT(strstr(buf, "blocked") != NULL, "cat auth_tokens.json blocked"); +} + static void test_shell_missing_command(void) { const tool_t *t = tool_shell_get(); @@ -72,6 +82,7 @@ int main(void) { MU_RUN(test_shell_blocked_rm_rf); MU_RUN(test_shell_blocked_mkfs); + MU_RUN(test_shell_blocked_auth_tokens); MU_RUN(test_shell_ls_succeeds); MU_RUN(test_shell_invalid_json); MU_RUN(test_shell_missing_command); From 41fdaf85464c8a55abce994b7b4b108d3e4ef674 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Tue, 22 Sep 2026 13:53:26 -0300 Subject: [PATCH 2/3] fix(test): size allowlist state paths for format truncation GCC -Werror=format-truncation rejected snprintf of a 256-byte directory plus /config.toml into another 256-byte buffer. --- tests/test_allowlist.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_allowlist.c b/tests/test_allowlist.c index 346e8c5..e45a8fe 100644 --- a/tests/test_allowlist.c +++ b/tests/test_allowlist.c @@ -12,6 +12,7 @@ #define _POSIX_C_SOURCE 200809L #include "sandbox/allowlist.h" +#include #include #include #include @@ -122,13 +123,13 @@ static int test_block_auth_tokens_json(void) static int test_block_state_dir_config_and_memory(void) { char dir[] = "/tmp/sc_al_state_XXXXXX"; - char state[256]; - char cfg_path[256]; - char db_path[256]; + char state[PATH_MAX - 32]; + char cfg_path[PATH_MAX]; + char db_path[PATH_MAX]; char *tmp; FILE *f; allowlist_config_t acfg; - char cmd[512]; + char cmd[PATH_MAX + 16]; tmp = mkdtemp(dir); if (!tmp) { From e63a7ecae9e8e5e8c0ef34bebb7194a30d217fab Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Tue, 22 Sep 2026 14:01:55 -0300 Subject: [PATCH 3/3] fix(sandbox): block state sidecars and workspace-relative shell names WAL sidecars next to memory.db were readable, and a bare cat config.toml ran after the sandbox chdir. Also refuse a symlink workspace and apply the same state-file check on the unsandboxed shell path. --- CHANGELOG.md | 2 +- src/core/bootstrap.c | 29 +++++++++-- src/sandbox/allowlist.c | 103 ++++++++++++++++++++++++++++++++++------ src/sandbox/allowlist.h | 3 +- src/tools/shell.c | 17 +++++-- tests/test_allowlist.c | 17 +++++++ tests/test_config.c | 3 +- tests/test_file.c | 7 +-- tests/test_shell.c | 35 +++++++++++++- 9 files changed, 184 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6a3c69..cfb4f95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ## [Unreleased] ### Fixed -- Default `workspace_path` is `~/.shellclaw/workspace`, and file/shell tools still refuse `auth_tokens.json`, `shellclaw.pid`, `shellclaw.log`, and `.shellclaw` `config.toml` / `memory.db` when a custom workspace contains them. +- Default `workspace_path` is `~/.shellclaw/workspace`, and file/shell tools still refuse `auth_tokens.json`, `shellclaw.pid`, `shellclaw.log`, and `.shellclaw` `config.toml` / `memory.db` (including `memory.db-*` sidecars) when a custom workspace contains them. A bare `cat config.toml` after the sandbox `chdir` is blocked, and a symlink at the workspace path is not accepted. - Dashboard `PUT /api/config` merges JSON fields into `config.toml` and reloads live settings. Indented keys and `[section] # comment` headers are updated in place; a present field with the wrong JSON type returns 400; a saved file whose live reload fails returns 500. Gateway host and port still need a process restart to rebind. - Unsandboxed `shell` no longer blocks forever in `waitpid` after the output cap fills; leftover children (including background grandchildren) are SIGKILL'd via the command process group, and truncated capture is NUL-terminated (#69). - `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). diff --git a/src/core/bootstrap.c b/src/core/bootstrap.c index 58de247..92ac554 100644 --- a/src/core/bootstrap.c +++ b/src/core/bootstrap.c @@ -237,11 +237,30 @@ static void channels_cleanup(void) g_cfg = NULL; } -static void ensure_workspace_directory(const char *workspace) +static int workspace_is_real_dir(const char *workspace) +{ + struct stat st; + + if (lstat(workspace, &st) != 0) { + fprintf(stderr, "shellclaw: workspace %s: %s\n", workspace, strerror(errno)); + return 0; + } + if (S_ISLNK(st.st_mode)) { + fprintf(stderr, "shellclaw: workspace %s is a symlink\n", workspace); + return 0; + } + if (!S_ISDIR(st.st_mode)) { + fprintf(stderr, "shellclaw: workspace %s is not a directory\n", workspace); + return 0; + } + return 1; +} + +static int ensure_workspace_directory(const char *workspace) { const char *slash; - if (!workspace || !workspace[0]) return; + if (!workspace || !workspace[0]) return 0; slash = strrchr(workspace, '/'); if (slash && slash != workspace) { char parent[PATH_MAX]; @@ -257,11 +276,15 @@ static void ensure_workspace_directory(const char *workspace) if (mkdir(workspace, 0700) != 0 && errno != EEXIST) fprintf(stderr, "shellclaw: mkdir workspace %s: %s\n", workspace, strerror(errno)); + if (!workspace_is_real_dir(workspace)) + return -1; + return 0; } int tools_init(const config_t *cfg) { - ensure_workspace_directory(config_workspace_path(cfg)); + if (ensure_workspace_directory(config_workspace_path(cfg)) != 0) + return -1; tool_set_config(cfg); g_tool_count = tool_get_all(g_tools, SHELLCLAW_MAX_TOOLS); return 0; diff --git a/src/sandbox/allowlist.c b/src/sandbox/allowlist.c index 6f1e62e..f3a67f2 100644 --- a/src/sandbox/allowlist.c +++ b/src/sandbox/allowlist.c @@ -141,7 +141,8 @@ int allowlist_path_is_runtime_state_file(const char *path) strcmp(base, "shellclaw.pid") == 0 || strcmp(base, "shellclaw.log") == 0) return 1; - if (strcmp(base, "config.toml") != 0 && strcmp(base, "memory.db") != 0) + if (strcmp(base, "config.toml") != 0 && strcmp(base, "memory.db") != 0 && + strncmp(base, "memory.db-", 10) != 0) return 0; slash = strrchr(use, '/'); if (!slash || slash == use) @@ -156,6 +157,66 @@ int allowlist_path_is_runtime_state_file(const char *path) return strcmp(slash, ".shellclaw") == 0; } +/** Copy @p rel under @p root, collapsing "." and "..". Absolute @p rel is copied as-is. */ +static int join_under_root(const char *root, const char *rel, char *out, size_t cap) +{ + char tmp[PATH_MAX]; + char *dup; + char *save = NULL; + char *tok; + char *stack[48] = {0}; + int nstack = 0; + int i; + size_t used; + + if (!rel || !rel[0] || !out || cap == 0) + return -1; + if (rel[0] == '/') { + if (strlen(rel) + 1 > cap) + return -1; + memcpy(out, rel, strlen(rel) + 1); + return 0; + } + if (!root || !root[0]) + return -1; + if (snprintf(tmp, sizeof(tmp), "%s/%s", root, rel) >= (int)sizeof(tmp)) + return -1; + dup = strdup(tmp); + if (!dup) + return -1; + for (tok = strtok_r(dup, "/", &save); tok; tok = strtok_r(NULL, "/", &save)) { + if (strcmp(tok, ".") == 0) + continue; + if (strcmp(tok, "..") == 0) { + if (nstack > 0) + nstack--; + continue; + } + if (nstack >= (int)(sizeof(stack) / sizeof(stack[0]))) { + free(dup); + return -1; + } + stack[nstack++] = tok; + } + used = 0; + out[0] = '\0'; + for (i = 0; i < nstack; i++) { + size_t part = strlen(stack[i]); + if (used + 1 + part + 1 > cap) { + free(dup); + return -1; + } + out[used++] = '/'; + memcpy(out + used, stack[i], part); + used += part; + out[used] = '\0'; + } + free(dup); + if (used == 0) + return -1; + return 0; +} + /* ------------------------------------------------------------------ */ /* Public: combined check */ /* ------------------------------------------------------------------ */ @@ -190,20 +251,20 @@ int allowlist_check_shell_command(const char *cmd, const allowlist_config_t *cfg return 1; } } - /* Phase 2: workspace path containment */ - if (!cfg || !cfg->workspace_only || !cfg->workspace_path || !cfg->workspace_path[0]) - return 0; - workspace_only = cfg->workspace_only; - (void)workspace_only; - /* Resolve workspace root once */ - if (!realpath(cfg->workspace_path, ws_resolved)) { - /* Workspace path does not exist; use as-is. */ - size_t n = strlen(cfg->workspace_path); - if (n >= PATH_MAX) n = PATH_MAX - 1; - memcpy(ws_resolved, cfg->workspace_path, n); - ws_resolved[n] = '\0'; + /* Phase 2: runtime-state paths, then optional workspace containment. + * State files are rejected even when workspace_only is off, so an + * unsandboxed `cat ~/.shellclaw/config.toml` cannot skip the check. */ + workspace_only = cfg && cfg->workspace_only && cfg->workspace_path && + cfg->workspace_path[0]; + if (cfg && cfg->workspace_path && cfg->workspace_path[0]) { + if (!realpath(cfg->workspace_path, ws_resolved)) { + size_t n = strlen(cfg->workspace_path); + if (n >= PATH_MAX) n = PATH_MAX - 1; + memcpy(ws_resolved, cfg->workspace_path, n); + ws_resolved[n] = '\0'; + } + workspace_root = ws_resolved; } - workspace_root = ws_resolved; /* Tokenize the command and check each path-like token. */ cmd_copy = strdup(cmd); if (!cmd_copy) return 0; /* fail-open on OOM */ @@ -226,7 +287,19 @@ int allowlist_check_shell_command(const char *cmd, const allowlist_config_t *cfg free(cmd_copy); return 1; } - if (has_path_chars(tok)) { + /* sandbox_exec chdirs into the workspace, so a bare name is that file. */ + if (workspace_root && check[0] != '/') { + char joined[PATH_MAX]; + if (join_under_root(workspace_root, check, joined, sizeof(joined)) == 0 && + allowlist_path_is_runtime_state_file(joined)) { + set_reason(reason_buf, reason_cap, + "command blocked: runtime state file: ", joined); + fprintf(stderr, "allowlist: blocked runtime state file: %s\n", joined); + free(cmd_copy); + return 1; + } + } + if (workspace_only && has_path_chars(tok)) { if (!allowlist_path_is_under_workspace(check, workspace_root)) { set_reason(reason_buf, reason_cap, "command blocked: path escapes workspace: ", check); diff --git a/src/sandbox/allowlist.h b/src/sandbox/allowlist.h index 5f64c2e..b2d8ea1 100644 --- a/src/sandbox/allowlist.h +++ b/src/sandbox/allowlist.h @@ -70,7 +70,8 @@ int allowlist_path_is_under_workspace(const char *path, const char *workspace_ro * Return 1 if @p path is a ShellClaw runtime state file that tools must not touch. * * Always reserved by basename: auth_tokens.json, shellclaw.pid, shellclaw.log. - * Also reserved when the parent directory is named `.shellclaw`: config.toml, memory.db. + * Also reserved when the parent directory is named `.shellclaw`: config.toml, + * memory.db, and memory.db-* sidecars (WAL, shm, journal). * * @param path Absolute, relative, or unresolved path (realpath used when the file exists). * @return 1 if reserved, 0 otherwise. diff --git a/src/tools/shell.c b/src/tools/shell.c index fd5fe28..d2c02fa 100644 --- a/src/tools/shell.c +++ b/src/tools/shell.c @@ -233,13 +233,20 @@ static int shell_execute(const char *args_json, char *result_buf, size_t max_len free(command); return rc; } - /* Fallback path: best-effort blocklist + plain fork */ + /* Fallback path: same runtime-state predicate as the allowlist, then + * the substring blocklist. workspace_only stays off so a bare filename + * in the process cwd is not treated as ~/.shellclaw. */ fprintf(stderr, "shell: sandbox disabled — running command with reduced isolation\n"); - if (fallback_is_blocked(command)) { - free(command); - snprintf(result_buf, max_len, "{\"error\":\"command blocked for safety\"}"); - return -1; + { + char reason[256]; + reason[0] = '\0'; + if (allowlist_check_shell_command(command, NULL, reason, sizeof(reason)) || + fallback_is_blocked(command)) { + free(command); + snprintf(result_buf, max_len, "{\"error\":\"command blocked for safety\"}"); + return -1; + } } { int rc = run_unsandboxed(command, timeout_sec, result_buf, max_len); diff --git a/tests/test_allowlist.c b/tests/test_allowlist.c index e45a8fe..223b3d9 100644 --- a/tests/test_allowlist.c +++ b/tests/test_allowlist.c @@ -175,6 +175,22 @@ static int test_block_state_dir_config_and_memory(void) return 0; } +static int test_block_memory_sidecars_and_bare_names(void) +{ + allowlist_config_t acfg; + + ASSERT(allowlist_path_is_runtime_state_file("/tmp/x/.shellclaw/memory.db-wal") == 1); + ASSERT(allowlist_path_is_runtime_state_file("/tmp/x/.shellclaw/memory.db-shm") == 1); + ASSERT(allowlist_path_is_runtime_state_file("/tmp/proj/memory.db-wal") == 0); + acfg.workspace_path = "/tmp/x/.shellclaw"; + acfg.workspace_only = 1; + ASSERT(allowlist_check_shell_command("cat config.toml", &acfg, NULL, 0) == 1); + ASSERT(allowlist_check_shell_command("cat memory.db", &acfg, NULL, 0) == 1); + ASSERT(allowlist_check_shell_command("cat memory.db-wal", &acfg, NULL, 0) == 1); + ASSERT(allowlist_check_shell_command("cat notes.txt", &acfg, NULL, 0) == 0); + return 0; +} + static int test_allow_project_config_toml(void) { ASSERT(allowlist_path_is_runtime_state_file("/tmp/project/config.toml") == 0); @@ -297,6 +313,7 @@ int main(void) RUN(test_null_command_blocked()); RUN(test_block_auth_tokens_json()); RUN(test_block_state_dir_config_and_memory()); + RUN(test_block_memory_sidecars_and_bare_names()); RUN(test_allow_project_config_toml()); RUN(test_path_inside_workspace()); RUN(test_path_outside_workspace()); diff --git a/tests/test_config.c b/tests/test_config.c index 5c1e963..c32b633 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -112,7 +112,8 @@ static int test_defaults(void) ASSERT(ws != NULL); n = strlen(ws); ASSERT(n >= 10); - ASSERT(strcmp(ws + n - 10, "/workspace") == 0); + ASSERT(n >= 21); + ASSERT(strcmp(ws + n - 21, "/.shellclaw/workspace") == 0); } config_free(cfg); remove(path); diff --git a/tests/test_file.c b/tests/test_file.c index 9af7050..0907d27 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -224,12 +224,9 @@ static void test_runtime_state_files_rejected_inside_workspace(void) int r; snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_state_%d", (int)getpid()); - if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return; + MU_ASSERT(mkdir(tmpdir, 0755) == 0 || errno == EEXIST, "mkdir state tmpdir"); snprintf(state_dir, sizeof(state_dir), "%s/.shellclaw", tmpdir); - if (mkdir(state_dir, 0755) != 0 && errno != EEXIST) { - rmdir(tmpdir); - return; - } + MU_ASSERT(mkdir(state_dir, 0755) == 0 || errno == EEXIST, "mkdir .shellclaw"); snprintf(token_path, sizeof(token_path), "%s/auth_tokens.json", state_dir); snprintf(config_toml, sizeof(config_toml), "%s/config.toml", state_dir); snprintf(memory_db, sizeof(memory_db), "%s/memory.db", state_dir); diff --git a/tests/test_shell.c b/tests/test_shell.c index b310af4..a37341b 100644 --- a/tests/test_shell.c +++ b/tests/test_shell.c @@ -9,8 +9,10 @@ #include "core/config.h" #include #include +#include #include #include +#include #include #include #include @@ -71,11 +73,42 @@ static void test_shell_invalid_json(void) static void test_shell_blocked_auth_tokens(void) { const tool_t *t = tool_shell_get(); - char buf[256]; + char buf[512]; + char home[64]; + char state[128]; + char cfg_path[160]; + char cmd[256]; + char *old_home; + FILE *f; buf[0] = '\0'; tool_shell_set_config(NULL); (void)t->execute("{\"command\":\"cat ~/.shellclaw/auth_tokens.json\"}", buf, sizeof(buf)); MU_ASSERT(strstr(buf, "blocked") != NULL, "cat auth_tokens.json blocked"); + + snprintf(home, sizeof(home), "/tmp/sc_shell_home_%d", (int)getpid()); + snprintf(state, sizeof(state), "%s/.shellclaw", home); + snprintf(cfg_path, sizeof(cfg_path), "%s/config.toml", state); + MU_ASSERT(mkdir(home, 0755) == 0 || errno == EEXIST, "mkdir shell home"); + MU_ASSERT(mkdir(state, 0755) == 0 || errno == EEXIST, "mkdir shell state"); + f = fopen(cfg_path, "w"); + MU_ASSERT(f != NULL, "write state config.toml"); + fputs("secret-config\n", f); + fclose(f); + old_home = getenv("HOME"); + setenv("HOME", home, 1); + snprintf(cmd, sizeof(cmd), + "{\"command\":\"cat %s/.shellclaw/config.toml\"}", home); + buf[0] = '\0'; + (void)t->execute(cmd, buf, sizeof(buf)); + MU_ASSERT(strstr(buf, "blocked") != NULL, "unsandboxed cat state config.toml blocked"); + MU_ASSERT(strstr(buf, "secret-config") == NULL, "state config.toml not returned"); + if (old_home) + setenv("HOME", old_home, 1); + else + unsetenv("HOME"); + unlink(cfg_path); + rmdir(state); + rmdir(home); } static void test_shell_missing_command(void)