From 48c03f511d76c0cbc2ca9722cb1c1754048b83aa Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 3 Sep 2026 11:30:19 +0000 Subject: [PATCH] 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);