From 2967d17974900314c9b9aeb0a5f8c1e0a90382fb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 10 Sep 2026 11:10:01 +0000 Subject: [PATCH 1/2] fix(sandbox): block $HOME/$PWD shell expansions in workspace allowlist has_path_chars only flagged tokens starting with / ~ ., so workspace_only skipped cat $HOME/.shellclaw/auth_tokens.json and ANSI-C $'\x2f...' paths. Expand HOME/PWD forms and fail-closed on other $ expansions before the workspace realpath check. Namespaces alone do not chroot the host FS. Co-authored-by: esadrianno --- src/sandbox/allowlist.c | 100 ++++++++++++++++++++++++++++++++++------ src/sandbox/allowlist.h | 4 +- tests/test_allowlist.c | 46 ++++++++++++++++++ 3 files changed, 135 insertions(+), 15 deletions(-) diff --git a/src/sandbox/allowlist.c b/src/sandbox/allowlist.c index c3033ce..7dd7256 100644 --- a/src/sandbox/allowlist.c +++ b/src/sandbox/allowlist.c @@ -70,11 +70,83 @@ static void set_reason(char *buf, size_t cap, const char *prefix, const char *de buf[cap - 1] = '\0'; } -/** Return 1 if @p s begins with prefix after any leading whitespace. */ +/** + * Return 1 if @p tok looks like a filesystem path (or expands to one). + * Includes shell parameter expansions such as `$HOME/...` and `${PWD}/...` + * that never start with `/` `~` `.` but become absolute after /bin/sh expands them. + */ static int has_path_chars(const char *tok) { - if (!tok) return 0; - return tok[0] == '/' || tok[0] == '~' || tok[0] == '.'; + if (!tok || !tok[0]) return 0; + if (tok[0] == '/' || tok[0] == '~' || tok[0] == '.') return 1; + /* `$HOME`, `${HOME}/x`, `$PWD/../out`, `$(pwd)/x`, ANSI-C `$'\x2f...'`. */ + if (tok[0] == '$') return 1; + return 0; +} + +/** + * Expand a leading `~`, `$HOME` / `${HOME}`, or `$PWD` / `${PWD}` for workspace checks. + * Unknown `$...` forms fail closed (return -1) so workspace_only cannot be bypassed + * via arbitrary parameter / command substitutions. + * @return 0 and writes into @p expanded on success; -1 if the token must be blocked. + */ +static int expand_shell_path_token(const char *tok, char *expanded, size_t expanded_cap) +{ + const char *home; + const char *cwd; + const char *suffix; + int n; + + if (!tok || !expanded || expanded_cap == 0) return -1; + + if (tok[0] == '~') { + home = getenv("HOME"); + if (home) + n = snprintf(expanded, expanded_cap, "%s%s", home, tok + 1); + else + n = snprintf(expanded, expanded_cap, "%s", tok); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + + if (tok[0] != '$') { + if (strlen(tok) >= expanded_cap) return -1; + memcpy(expanded, tok, strlen(tok) + 1); + return 0; + } + + /* Fail closed on ANSI-C quoting and command substitution (encoded / dynamic paths). */ + if (tok[1] == '\'' || tok[1] == '"' || tok[1] == '(') + return -1; + + home = getenv("HOME"); + cwd = getenv("PWD"); + if (strncmp(tok, "${HOME}", 7) == 0) { + suffix = tok + 7; + if (!home) return -1; + n = snprintf(expanded, expanded_cap, "%s%s", home, suffix); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + if (strncmp(tok, "$HOME", 5) == 0 && (tok[5] == '\0' || tok[5] == '/')) { + suffix = tok + 5; + if (!home) return -1; + n = snprintf(expanded, expanded_cap, "%s%s", home, suffix); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + if (strncmp(tok, "${PWD}", 6) == 0) { + suffix = tok + 6; + if (!cwd) return -1; + n = snprintf(expanded, expanded_cap, "%s%s", cwd, suffix); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + if (strncmp(tok, "$PWD", 4) == 0 && (tok[4] == '\0' || tok[4] == '/')) { + suffix = tok + 4; + if (!cwd) return -1; + n = snprintf(expanded, expanded_cap, "%s%s", cwd, suffix); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + + /* Other `$VAR` / `${VAR}` forms: fail closed under workspace_only. */ + return -1; } /* ------------------------------------------------------------------ */ @@ -167,20 +239,20 @@ int allowlist_check_shell_command(const char *cmd, const allowlist_config_t *cfg tok = strtok_r(cmd_copy, " \t\n;|&><", &saveptr); while (tok) { 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; + const char *check_path = tok; + if (expand_shell_path_token(tok, expanded, sizeof(expanded)) != 0) { + set_reason(reason_buf, reason_cap, + "command blocked: unresolved shell path expansion: ", tok); + fprintf(stderr, "allowlist: blocked unresolved shell path: %s\n", tok); + free(cmd_copy); + return 1; } - if (!allowlist_path_is_under_workspace(tok, workspace_root)) { + check_path = expanded; + if (!allowlist_path_is_under_workspace(check_path, 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_path); + fprintf(stderr, "allowlist: blocked path outside workspace: %s\n", check_path); free(cmd_copy); return 1; } diff --git a/src/sandbox/allowlist.h b/src/sandbox/allowlist.h index 76a4e4f..6d81fbf 100644 --- a/src/sandbox/allowlist.h +++ b/src/sandbox/allowlist.h @@ -9,7 +9,9 @@ * "mkfs", "dd of=/dev/", fork bombs, etc.). * 2. An optional workspace-containment check: if enabled via allowlist_config_t, * path-like tokens in the command are resolved with realpath(3) and rejected - * when they escape the declared workspace root. + * when they escape the declared workspace root. Tokens that become absolute only + * after shell expansion (`$HOME/...`, `${PWD}/...`, ANSI-C `$'\x2f...'`) are + * expanded or fail-closed before that check. * * Both checks are intentionally conservative and may produce false positives. * They are a best-effort defence-in-depth layer; real isolation is provided by diff --git a/tests/test_allowlist.c b/tests/test_allowlist.c index 5d2ac74..b006559 100644 --- a/tests/test_allowlist.c +++ b/tests/test_allowlist.c @@ -144,6 +144,51 @@ static int test_workspace_only_allows_inside_path(void) return 0; } +/** + * Shell expands `$HOME` / `${HOME}` / `$PWD` before open(2). Tokens never start + * with `/` `~` `.`, so the old has_path_chars gate skipped them and workspace_only + * could not stop `cat $HOME/.shellclaw/auth_tokens.json`. + */ +static int test_workspace_only_blocks_home_env_expansion(void) +{ + allowlist_config_t cfg; + char reason[256]; + char ws[] = "/tmp/sc_al_home_XXXXXX"; + char *dir; + const char *home = getenv("HOME"); + + dir = mkdtemp(ws); + if (!dir) { + fprintf(stderr, "test_workspace_only_blocks_home_env_expansion: mkdtemp failed\n"); + return 1; + } + cfg.workspace_path = dir; + cfg.workspace_only = 1; + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat $HOME/.shellclaw/auth_tokens.json", + &cfg, reason, sizeof(reason)) == 1); + ASSERT(strstr(reason, "escapes workspace") != NULL || + strstr(reason, "unresolved") != NULL); + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat ${HOME}/.shellclaw/auth_tokens.json", + &cfg, reason, sizeof(reason)) == 1); + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat $PWD/../outside.txt", + &cfg, reason, sizeof(reason)) == 1); + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat $'\\x2fetc\\x2fpasswd'", + &cfg, reason, sizeof(reason)) == 1); + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat notes.txt", &cfg, reason, sizeof(reason)) == 0); + if (home && strcmp(home, dir) == 0) { + /* Degenerate: HOME equals workspace — $HOME alone is in-bounds. */ + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("ls $HOME", &cfg, reason, sizeof(reason)) == 0); + } + rmdir(dir); + return 0; +} + /* ------------------------------------------------------------------ */ /* Symlink escape test (5.4) */ /* ------------------------------------------------------------------ */ @@ -205,6 +250,7 @@ int main(void) RUN(test_path_prefix_no_slash()); RUN(test_workspace_only_blocks_outside_path()); RUN(test_workspace_only_allows_inside_path()); + RUN(test_workspace_only_blocks_home_env_expansion()); RUN(test_symlink_escape()); printf("test_allowlist: all tests passed\n"); return 0; From fbed4b59e94f640601de399095b3a29a6cb67857 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 10 Sep 2026 11:16:25 +0000 Subject: [PATCH 2/2] fix(sandbox): treat quoted $HOME tokens as path expansions Double-quoted cat "$HOME/.shellclaw/auth_tokens.json" kept the quotes, so has_path_chars skipped the token. Strip one surrounding quote pair before the expand/workspace check. /bin/sh still expands the inner form. Co-authored-by: esadrianno --- src/sandbox/allowlist.c | 18 ++++++++++++++++++ src/sandbox/allowlist.h | 4 ++-- tests/test_allowlist.c | 3 +++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/sandbox/allowlist.c b/src/sandbox/allowlist.c index 7dd7256..9664767 100644 --- a/src/sandbox/allowlist.c +++ b/src/sandbox/allowlist.c @@ -70,6 +70,23 @@ static void set_reason(char *buf, size_t cap, const char *prefix, const char *de buf[cap - 1] = '\0'; } +/** + * Strip one matching pair of surrounding quotes so `"$HOME/x"` is checked + * the same way as `$HOME/x`. /bin/sh still expands the inner form. + */ +static char *strip_surrounding_quotes(char *tok) +{ + size_t n; + if (!tok || !tok[0]) return tok; + n = strlen(tok); + if (n >= 2 && ((tok[0] == '\'' && tok[n - 1] == '\'') || + (tok[0] == '"' && tok[n - 1] == '"'))) { + tok[n - 1] = '\0'; + return tok + 1; + } + return tok; +} + /** * Return 1 if @p tok looks like a filesystem path (or expands to one). * Includes shell parameter expansions such as `$HOME/...` and `${PWD}/...` @@ -238,6 +255,7 @@ 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) { + tok = strip_surrounding_quotes(tok); if (has_path_chars(tok)) { char expanded[PATH_MAX]; const char *check_path = tok; diff --git a/src/sandbox/allowlist.h b/src/sandbox/allowlist.h index 6d81fbf..5bc7da0 100644 --- a/src/sandbox/allowlist.h +++ b/src/sandbox/allowlist.h @@ -10,8 +10,8 @@ * 2. An optional workspace-containment check: if enabled via allowlist_config_t, * path-like tokens in the command are resolved with realpath(3) and rejected * when they escape the declared workspace root. Tokens that become absolute only - * after shell expansion (`$HOME/...`, `${PWD}/...`, ANSI-C `$'\x2f...'`) are - * expanded or fail-closed before that check. + * after shell expansion (`$HOME/...`, `"$HOME/..."`, `${PWD}/...`, ANSI-C + * `$'\x2f...'`) are unquoted, expanded, or fail-closed before that check. * * Both checks are intentionally conservative and may produce false positives. * They are a best-effort defence-in-depth layer; real isolation is provided by diff --git a/tests/test_allowlist.c b/tests/test_allowlist.c index b006559..6ea9131 100644 --- a/tests/test_allowlist.c +++ b/tests/test_allowlist.c @@ -179,6 +179,9 @@ static int test_workspace_only_blocks_home_env_expansion(void) ASSERT(allowlist_check_shell_command("cat $'\\x2fetc\\x2fpasswd'", &cfg, reason, sizeof(reason)) == 1); reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat \"$HOME/.shellclaw/auth_tokens.json\"", + &cfg, reason, sizeof(reason)) == 1); + reason[0] = '\0'; ASSERT(allowlist_check_shell_command("cat notes.txt", &cfg, reason, sizeof(reason)) == 0); if (home && strcmp(home, dir) == 0) { /* Degenerate: HOME equals workspace — $HOME alone is in-bounds. */