From 25b2fdedbf8e22a9c28b07a34d712a1e9859f7c4 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Fri, 16 Oct 2026 00:12:42 -0300 Subject: [PATCH 01/10] fix(sandbox): hide host secrets from sandboxed shell commands The command child inherited the agent environment, so printenv inside sh -c returned provider API keys and bot tokens on the tool stdout channel. Replace that environment with PATH, locale, and workspace HOME before execl. Supersedes #109. --- Makefile | 10 +++-- docs/SECURITY.md | 2 +- src/sandbox/sandbox.c | 6 ++- src/sandbox/sandbox_env.c | 77 +++++++++++++++++++++++++++++++++++++++ src/sandbox/sandbox_env.h | 18 +++++++++ tests/test_sandbox.c | 24 ++++++++++++ 6 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 src/sandbox/sandbox_env.c create mode 100644 src/sandbox/sandbox_env.h diff --git a/Makefile b/Makefile index f40f391..7ac876b 100644 --- a/Makefile +++ b/Makefile @@ -127,7 +127,8 @@ ASAP_INVOKE_TEST_O := $(BINDIR)/asap_invoke_test.o # Sandbox (Phase 3 §5) SANDBOX_CORE_O := src/sandbox/sandbox.o SANDBOX_LANDLOCK_O := src/sandbox/sandbox_landlock.o -SANDBOX_O := $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) +SANDBOX_ENV_O := src/sandbox/sandbox_env.o +SANDBOX_O := $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) ALLOWLIST_SCAN_O := src/sandbox/allowlist.o ALLOWLIST_PATH_O := src/sandbox/allowlist_path.o ALLOWLIST_O := $(ALLOWLIST_SCAN_O) $(ALLOWLIST_PATH_O) @@ -385,9 +386,12 @@ $(SHELL_O): src/tools/shell.c src/tools/tool.h src/tools/shell.h src/core/config src/sandbox/sandbox.h src/sandbox/allowlist.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/tools/shell.c -$(SANDBOX_CORE_O): src/sandbox/sandbox.c src/sandbox/sandbox.h src/sandbox/sandbox_landlock.h +$(SANDBOX_CORE_O): src/sandbox/sandbox.c src/sandbox/sandbox.h src/sandbox/sandbox_landlock.h src/sandbox/sandbox_env.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/sandbox/sandbox.c +$(SANDBOX_ENV_O): src/sandbox/sandbox_env.c src/sandbox/sandbox_env.h + $(CC) $(CFLAGS) $(INC) -c -o $@ src/sandbox/sandbox_env.c + $(SANDBOX_LANDLOCK_O): src/sandbox/sandbox_landlock.c src/sandbox/sandbox_landlock.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/sandbox/sandbox_landlock.c @@ -909,7 +913,7 @@ clean-root-dsym: @rm -f shellclaw test_agent test_anthropic test_channel test_cli test_config test_file test_memory test_local_provider test_openai test_provider test_router test_shell test_skill test_telegram test_web_search test_ws clean: clean-root-dsym - rm -f $(OBJS) $(PROVIDER_COMMON_O) $(STUB_O) $(ANTHROPIC_O) $(OPENAI_COMPAT_O) $(OPENAI_O) $(LOCAL_O) $(ROUTER_O) $(CJSON_O) $(TWEETNACL_O) $(ANTHROPIC_TEST_O) $(OPENAI_TEST_O) $(LOCAL_TEST_O) $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(CHANNEL_TG_TEST_O) $(CHANNEL_COMMON_O) $(CHANNEL_STUB_O) $(CHANNEL_CLI_O) $(CHANNEL_TG_O) $(CHANNEL_DISCORD_O) $(DISCORD_HELPERS_O) $(CHANNEL_HEARTBEAT_O) $(CHANNEL_WEBCHAT_O) $(AUTH_O) $(STATIC_O) $(HTTP_O) $(HTTP_LWS_O) $(ASAP_HTTP_BODY_O) $(ROUTES_O) $(ROUTES_HARDWARE_O) $(WS_O) $(MANIFEST_O) $(MANIFEST_PROFILES_O) $(MANIFEST_BUILD_O) $(MANIFEST_SIGN_O) $(MANIFEST_KEYS_O) $(ENVELOPE_O) $(ULID_O) $(CLIENT_O) $(ASAP_REGISTRY_O) $(SERVER_O) $(ASAP_LOG_O) $(RATE_LIMIT_O) $(SHELL_O) $(WEBSEARCH_O) $(FILE_O) $(REGISTRY_O) $(CONTEXT_O) $(CONTEXT_CACHE_O) $(CONTEXT_HTTP_O) $(CONTEXT_GEO_O) $(CRYPTO_O) $(JCS_O) $(HARDWARE_STUB_O) $(HARDWARE_INIT_O) $(HARDWARE_GPIO_SNAPSHOT_O) $(HARDWARE_TEGRASTATS_O) $(HARDWARE_TOOLS_O) $(BOARD_DETECT_O) src/hardware/hardware_libgpiod.o $(HARDWARE_I2C_O) $(HARDWARE_CAMERA_O) $(CRON_O) $(ASAP_INVOKE_O) $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) $(ALLOWLIST_SCAN_O) $(ALLOWLIST_PATH_O) + rm -f $(OBJS) $(PROVIDER_COMMON_O) $(STUB_O) $(ANTHROPIC_O) $(OPENAI_COMPAT_O) $(OPENAI_O) $(LOCAL_O) $(ROUTER_O) $(CJSON_O) $(TWEETNACL_O) $(ANTHROPIC_TEST_O) $(OPENAI_TEST_O) $(LOCAL_TEST_O) $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(CHANNEL_TG_TEST_O) $(CHANNEL_COMMON_O) $(CHANNEL_STUB_O) $(CHANNEL_CLI_O) $(CHANNEL_TG_O) $(CHANNEL_DISCORD_O) $(DISCORD_HELPERS_O) $(CHANNEL_HEARTBEAT_O) $(CHANNEL_WEBCHAT_O) $(AUTH_O) $(STATIC_O) $(HTTP_O) $(HTTP_LWS_O) $(ASAP_HTTP_BODY_O) $(ROUTES_O) $(ROUTES_HARDWARE_O) $(WS_O) $(MANIFEST_O) $(MANIFEST_PROFILES_O) $(MANIFEST_BUILD_O) $(MANIFEST_SIGN_O) $(MANIFEST_KEYS_O) $(ENVELOPE_O) $(ULID_O) $(CLIENT_O) $(ASAP_REGISTRY_O) $(SERVER_O) $(ASAP_LOG_O) $(RATE_LIMIT_O) $(SHELL_O) $(WEBSEARCH_O) $(FILE_O) $(REGISTRY_O) $(CONTEXT_O) $(CONTEXT_CACHE_O) $(CONTEXT_HTTP_O) $(CONTEXT_GEO_O) $(CRYPTO_O) $(JCS_O) $(HARDWARE_STUB_O) $(HARDWARE_INIT_O) $(HARDWARE_GPIO_SNAPSHOT_O) $(HARDWARE_TEGRASTATS_O) $(HARDWARE_TOOLS_O) $(BOARD_DETECT_O) src/hardware/hardware_libgpiod.o $(HARDWARE_I2C_O) $(HARDWARE_CAMERA_O) $(CRON_O) $(ASAP_INVOKE_O) $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) $(ALLOWLIST_SCAN_O) $(ALLOWLIST_PATH_O) rm -f src/gateway/ui_assets.h find . -name '*.gcno' -o -name '*.gcda' -o -name '*.gcov' | xargs rm -f 2>/dev/null || true rm -f $(WS_TEST_O) $(BINDIR)/asap_registry_test.o $(BINDIR)/asap_invoke_test.o $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(BINDIR)/shellclaw $(BINDIR)/test_tweetnacl_smoke $(BINDIR)/test_config $(BINDIR)/test_config_patch $(BINDIR)/test_memory $(BINDIR)/test_skill $(BINDIR)/test_provider $(BINDIR)/test_anthropic $(BINDIR)/test_openai $(BINDIR)/test_local_provider $(BINDIR)/test_router $(BINDIR)/test_heartbeat $(BINDIR)/test_crypto $(BINDIR)/test_hardware_stub $(BINDIR)/test_board_detect $(BINDIR)/test_hardware_libgpiod $(BINDIR)/test_hardware_i2c $(BINDIR)/test_hardware_camera $(BINDIR)/test_pin_tables $(BINDIR)/test_hardware_init $(BINDIR)/test_hardware_tools $(BINDIR)/test_registry $(BINDIR)/test_ws $(BINDIR)/test_agent $(BINDIR)/test_channel $(BINDIR)/test_cli $(BINDIR)/test_shell $(BINDIR)/test_file $(BINDIR)/test_telegram $(BINDIR)/test_discord_helpers $(BINDIR)/test_web_search $(BINDIR)/test_cron $(BINDIR)/test_context $(BINDIR)/test_manifest_build $(BINDIR)/test_manifest_keys $(BINDIR)/test_jcs $(BINDIR)/test_asap_envelope $(BINDIR)/test_asap_ulid $(BINDIR)/test_asap_client $(BINDIR)/test_asap_registry $(BINDIR)/test_asap_server $(BINDIR)/test_asap_invoke $(BINDIR)/test_asap_log $(BINDIR)/test_auth $(BINDIR)/test_gateway_http $(BINDIR)/test_static $(BINDIR)/test_sandbox $(BINDIR)/test_allowlist $(BINDIR)/test_rate_limit diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 4a2b3fd..f3ff695 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -39,7 +39,7 @@ The primary goals are: prevent sandboxed shell commands from escaping to host de | Surface | Mechanism | Notes | |---------|-----------|-------| -| Shell (sandbox on) | `fork()` + user ns when needed + `unshare(CLONE_NEWNS \| CLONE_NEWNET \| CLONE_NEWPID)` + fork for PID 1 + Landlock workspace bound + `prctl(PR_SET_NO_NEW_PRIVS)` | Fail-closed if namespaces or Landlock cannot apply. Isolation uses a control pipe, not `sh` exit 122/123. See [Linux sandbox (Jetson)](#linux-sandbox-jetson) | +| Shell (sandbox on) | `fork()` + user ns when needed + `unshare(CLONE_NEWNS \| CLONE_NEWNET \| CLONE_NEWPID)` + fork for PID 1 + Landlock workspace bound + `prctl(PR_SET_NO_NEW_PRIVS)` + secret-free environment | Fail-closed if namespaces or Landlock cannot apply. The command child keeps `PATH`, locale, `TERM`, `TZ`, and `HOME` set to the workspace. Host API keys and bot tokens are not inherited. Isolation uses a control pipe, not `sh` exit 122/123. See [Linux sandbox (Jetson)](#linux-sandbox-jetson) | | Shell (sandbox off) | Plain `fork()` + substring fallback blocklist | **Not** a security boundary; stderr warning | | Allowlist | Substring blocklist + workspace containment (ancestor walk, quoted/embedded `/` `~`, `$HOME`/`$PWD`, `file:` URLs, relative names) | Defense-in-depth string scan; Landlock is the kernel host-FS bound | | Landlock | Ruleset on configured `workspace_path` (RW workspace + traverse-only `/` + RO `/bin` `/usr` `/lib` and `/etc/ssl/certs` only; RW `/dev/null`, `/dev/zero`, `/dev/urandom`, `/dev/tty` without `IOCTL_DEV`). Child `fchdir`s the workspace before `restrict_self`. A failed `landlock_add_rule` fails the ruleset closed. | Primary host-FS gate; `/` is `READ_DIR` only so `/etc/passwd` stays closed. `/etc/ssl/private` is not granted. Blocks symlink and `chr(47)+` host reads | diff --git a/src/sandbox/sandbox.c b/src/sandbox/sandbox.c index 4aac1a7..76a69c4 100644 --- a/src/sandbox/sandbox.c +++ b/src/sandbox/sandbox.c @@ -12,7 +12,9 @@ * procfs, applies Landlock, sets PR_SET_PDEATHSIG, and closes fds >= 3 * so a timeout SIGKILL of the isolator cannot leave the command under * host init. The isolator joins its cgroup before the command fork so - * memory.max and cpu.max apply to sh -c. Limits degrade if unavailable. + * memory.max and cpu.max apply to sh -c. The command process then + * replaces the inherited environment so provider keys and bot tokens + * are not readable via printenv. Limits degrade if unavailable. * * Non-Linux path: plain fork() + execl(); a warning is emitted to stderr. */ @@ -20,6 +22,7 @@ #define _POSIX_C_SOURCE 200809L #include "sandbox/sandbox.h" +#include "sandbox/sandbox_env.h" #include "sandbox/sandbox_landlock.h" #include #include @@ -446,6 +449,7 @@ int sandbox_exec(const char *cmd, char *out, size_t out_cap, #endif close(errpipe[1]); close_inherited_fds(); + sandbox_scrub_command_environment(workspace); execl("/bin/sh", "sh", "-c", cmd, (char *)NULL); _exit(127); } diff --git a/src/sandbox/sandbox_env.c b/src/sandbox/sandbox_env.c new file mode 100644 index 0000000..158cedf --- /dev/null +++ b/src/sandbox/sandbox_env.c @@ -0,0 +1,77 @@ +/** + * @file sandbox_env.c + * @brief Secret-free environment for the sandboxed command process. + * + * Provider keys and channel tokens live in the agent environment. The + * shell tool's stdout is returned to the model, so sh -c must not inherit + * them. A prompt-injected printenv would otherwise leak the values. + */ +#define _GNU_SOURCE +#define _POSIX_C_SOURCE 200809L + +#include "sandbox/sandbox_env.h" + +#include +#include +#if !defined(__linux__) +#include +#endif + +static const char FALLBACK_PATH[] = + "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"; + +static char *dup_env(const char *name) +{ + const char *v; + + v = getenv(name); + if (!v || !v[0]) + return NULL; + return strdup(v); +} + +static void put_env(const char *name, const char *value) +{ + if (value && value[0]) + setenv(name, value, 1); +} + +static void free_kept(char *path, char *lang, char *lc_all, char *lc_ctype, + char *term, char *tz) +{ + free(path); + free(lang); + free(lc_all); + free(lc_ctype); + free(term); + free(tz); +} + +void sandbox_scrub_command_environment(const char *workspace) +{ + char *path = dup_env("PATH"); + char *lang = dup_env("LANG"); + char *lc_all = dup_env("LC_ALL"); + char *lc_ctype = dup_env("LC_CTYPE"); + char *term = dup_env("TERM"); + char *tz = dup_env("TZ"); +#if !defined(__linux__) + static char *empty_env[] = { NULL }; + extern char **environ; +#endif + +#if defined(__linux__) + clearenv(); +#else + environ = empty_env; +#endif + put_env("PATH", path ? path : FALLBACK_PATH); + put_env("LANG", lang); + put_env("LC_ALL", lc_all); + put_env("LC_CTYPE", lc_ctype); + put_env("TERM", term); + put_env("TZ", tz); + if (workspace && workspace[0]) + setenv("HOME", workspace, 1); + free_kept(path, lang, lc_all, lc_ctype, term, tz); +} diff --git a/src/sandbox/sandbox_env.h b/src/sandbox/sandbox_env.h new file mode 100644 index 0000000..2d4e951 --- /dev/null +++ b/src/sandbox/sandbox_env.h @@ -0,0 +1,18 @@ +/** + * @file sandbox_env.h + * @brief Drop host secrets before exec of a sandboxed shell command. + */ +#ifndef SHELLCLAW_SANDBOX_ENV_H +#define SHELLCLAW_SANDBOX_ENV_H + +/** + * Replace the process environment with PATH, locale, and an optional HOME. + * + * Call only in the child that is about to execl sh -c. API keys, bot + * tokens, and LD_* variables from the agent are not passed through. + * + * Example: sandbox_scrub_command_environment("/home/me/.shellclaw/workspace"); + */ +void sandbox_scrub_command_environment(const char *workspace); + +#endif diff --git a/tests/test_sandbox.c b/tests/test_sandbox.c index 45fa2d0..41c24c1 100644 --- a/tests/test_sandbox.c +++ b/tests/test_sandbox.c @@ -147,6 +147,29 @@ static int test_stderr_captured(void) return 0; } +/** Host API keys must not be readable from sh -c. printenv is not blocklisted. */ +static int test_command_does_not_inherit_host_secrets(void) +{ + char out[4096]; + int rc; + const char *secret = "leaked_via_env"; + + ASSERT(setenv("SHELLCLAW_AUDIT_SECRET", secret, 1) == 0); + ASSERT(setenv("ANTHROPIC_API_KEY", secret, 1) == 0); + rc = sandbox_exec( + "printenv SHELLCLAW_AUDIT_SECRET ANTHROPIC_API_KEY; " + "tr '\\0' '\\n' < /proc/1/environ 2>/dev/null; echo PATH_OK", + out, sizeof out, 5000, NULL); + unsetenv("SHELLCLAW_AUDIT_SECRET"); + unsetenv("ANTHROPIC_API_KEY"); + if (skip_if_isolation_denied(rc, out, "test_command_does_not_inherit_host_secrets")) + return 0; + ASSERT(rc == 0); + ASSERT(strstr(out, secret) == NULL); + ASSERT(strstr(out, "PATH_OK") != NULL); + return 0; +} + static int test_null_cmd_returns_error(void) { char out[256]; @@ -823,6 +846,7 @@ int main(void) RUN(test_fail_closed_reports_isolation_error()); RUN(test_output_capture()); RUN(test_stderr_captured()); + RUN(test_command_does_not_inherit_host_secrets()); RUN(test_null_cmd_returns_error()); RUN(test_zero_cap_returns_error()); RUN(test_exit_nonzero_runs()); From 977cc5daa283242e75d7cf4dcb6cf1ed03b27cca Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Fri, 16 Oct 2026 00:12:42 -0300 Subject: [PATCH 02/10] fix(shell): stop trickle output from bypassing the command timeout The shell and sandbox wait loops only advanced elapsed time when poll returned idle. A command that wrote faster than the poll slice never hit shell_timeout_sec or sandbox_exec's timeout, so the agent lock stayed held until the output buffer filled. Count monotonic wall time in both loops and still report the deadline when the child exits because the pipe was closed. Supersedes #116. --- src/sandbox/sandbox.c | 84 ++++++++++++++++++++++++++++++++++++------- src/tools/shell.c | 52 +++++++++++++++++++++------ tests/test_sandbox.c | 35 ++++++++++++++++++ tests/test_shell.c | 50 ++++++++++++++++++++++++++ 4 files changed, 198 insertions(+), 23 deletions(-) diff --git a/src/sandbox/sandbox.c b/src/sandbox/sandbox.c index 76a69c4..6f5f50b 100644 --- a/src/sandbox/sandbox.c +++ b/src/sandbox/sandbox.c @@ -110,36 +110,89 @@ static void cgroup_remove(const char *base, const char *name) #endif /* __linux__ */ -static size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms) +/* Wall time since start, or -1 if the monotonic clock cannot be read. */ +static long monotonic_elapsed_ms(const struct timespec *start) { + struct timespec now; + long ms; + + if (clock_gettime(CLOCK_MONOTONIC, &now) != 0) + return -1; + ms = (now.tv_sec - start->tv_sec) * 1000L + + (now.tv_nsec - start->tv_nsec) / 1000000L; + return ms < 0 ? 0 : ms; +} + +static int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap) +{ + long remain; + + remain = (long)timeout_ms - elapsed_ms; + if (remain < 1) + return 1; + if (remain > slice_cap) + return slice_cap; + return (int)remain; +} + +/* + * Stop at timeout_ms of wall time, not idle poll time. A writer that keeps + * the pipe readable otherwise never advances the old counter, so the agent + * stays blocked until cap fills. + */ +static size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, int *hit_deadline) +{ + struct timespec start; size_t total = 0; - int elapsed_ms = 0; + int have_clock; + long elapsed_ms = 0; char chunk[512]; - if (cap == 0) return 0; - while (total < cap - 1 && elapsed_ms < timeout_ms) { + + if (hit_deadline) + *hit_deadline = 0; + if (cap == 0) + return 0; + have_clock = clock_gettime(CLOCK_MONOTONIC, &start) == 0; + while (total < cap - 1) { struct pollfd pfd; int slice; int r; ssize_t n; + + if (have_clock) { + long wall = monotonic_elapsed_ms(&start); + if (wall < 0) + have_clock = 0; + else + elapsed_ms = wall; + } + if (elapsed_ms >= (long)timeout_ms) { + if (hit_deadline) + *hit_deadline = 1; + break; + } + slice = poll_remain_ms(timeout_ms, elapsed_ms, PIPE_POLL_SLICE_MS); pfd.fd = fd; pfd.events = POLLIN; pfd.revents = 0; - slice = timeout_ms - elapsed_ms; - if (slice > PIPE_POLL_SLICE_MS) slice = PIPE_POLL_SLICE_MS; r = poll(&pfd, 1, slice); if (r < 0) { - if (errno == EINTR) continue; + if (errno == EINTR) + continue; break; } if (r == 0) { - elapsed_ms += slice; + if (!have_clock) + elapsed_ms += slice; continue; } n = read(fd, chunk, sizeof(chunk)); - if (n <= 0) break; + if (n <= 0) + break; { size_t add = (size_t)n; - if (total + add >= cap - 1) add = cap - 1 - total; + if (total + add >= cap - 1) + add = cap - 1 - total; memcpy(buf + total, chunk, add); total += add; } @@ -471,9 +524,14 @@ int sandbox_exec(const char *cmd, char *out, size_t out_cap, return report_isolation_failure(out, out_cap, iso); } } - total = drain_pipe(pipefd[0], out, out_cap, timeout_ms); - close(pipefd[0]); - timed_out = reap_child(pid, NULL); + { + int drain_deadline = 0; + total = drain_pipe(pipefd[0], out, out_cap, timeout_ms, &drain_deadline); + close(pipefd[0]); + timed_out = reap_child(pid, NULL); + if (drain_deadline) + timed_out = 1; + } if (timed_out && total < out_cap - 40) snprintf(out + total, out_cap - total, "\n[Sandbox: command timed out after %d ms]", timeout_ms); diff --git a/src/tools/shell.c b/src/tools/shell.c index 65a2d97..056d07a 100644 --- a/src/tools/shell.c +++ b/src/tools/shell.c @@ -66,6 +66,19 @@ static void kill_command_tree(pid_t pid) (void)kill(pid, SIGKILL); } +/* Wall time since start, or -1 if the monotonic clock cannot be read. */ +static long monotonic_elapsed_ms(const struct timespec *start) +{ + struct timespec now; + long ms; + + if (clock_gettime(CLOCK_MONOTONIC, &now) != 0) + return -1; + ms = (now.tv_sec - start->tv_sec) * 1000L + + (now.tv_nsec - start->tv_nsec) / 1000000L; + return ms < 0 ? 0 : ms; +} + /* Always SIGKILL the process group first so background grandchildren cannot * leak after the shell has already exited (SIGPIPE, or `cmd &`). Then reap * the tracked pid. Matching the hang fix for #69 / #96. */ @@ -115,7 +128,9 @@ static int run_unsandboxed(const char *command, int timeout_sec, pid_t pid; size_t total = 0; int timed_out = 0; - int elapsed_ms = 0; + long elapsed_ms = 0; + struct timespec start_ts; + int have_clock; char buf[256]; if (pipe(pipefd) != 0) { snprintf(result_buf, max_len, "{\"error\":\"pipe failed\"}"); @@ -140,28 +155,45 @@ static int run_unsandboxed(const char *command, int timeout_sec, (void)setpgid(pid, pid); close(pipefd[1]); result_buf[0] = '\0'; - while (total < max_len - 1 && elapsed_ms < timeout_sec * 1000) { + have_clock = clock_gettime(CLOCK_MONOTONIC, &start_ts) == 0; + /* Wall time, not idle poll time: a steady writer never used to trip + * the timeout, so the agent lock stayed held until the buffer filled. */ + while (total < max_len - 1) { struct pollfd pfd; + long remain; int rem; int r; ssize_t n; + if (have_clock) { + long wall = monotonic_elapsed_ms(&start_ts); + if (wall < 0) + have_clock = 0; + else + elapsed_ms = wall; + } + if (elapsed_ms >= (long)timeout_sec * 1000L) { + timed_out = 1; + kill_command_tree(pid); + break; + } pfd.fd = pipefd[0]; pfd.events = POLLIN; pfd.revents = 0; - rem = timeout_sec * 1000 - elapsed_ms; - if (rem > 5000) rem = 5000; + remain = (long)timeout_sec * 1000L - elapsed_ms; + if (remain > 5000) + rem = 5000; + else if (remain < 1) + rem = 1; + else + rem = (int)remain; r = poll(&pfd, 1, rem); if (r < 0) { if (errno == EINTR) continue; break; } if (r == 0) { - elapsed_ms += rem; - if (elapsed_ms >= timeout_sec * 1000) { - timed_out = 1; - kill_command_tree(pid); - break; - } + if (!have_clock) + elapsed_ms += rem; continue; } n = read(pipefd[0], buf, sizeof(buf)); diff --git a/tests/test_sandbox.c b/tests/test_sandbox.c index 41c24c1..63cc856 100644 --- a/tests/test_sandbox.c +++ b/tests/test_sandbox.c @@ -19,6 +19,7 @@ #include "sandbox/sandbox_landlock.h" #include #include +#include #include #include #include @@ -300,6 +301,39 @@ static int test_missing_workspace_fail_closed(void) return 0; } +static void trickle_timeout_watchdog(int sig) +{ + (void)sig; + fprintf(stderr, "FAIL: sandbox timeout did not fire while stdout trickled\n"); + _exit(2); +} + +static int test_timeout_kills_trickle_output(void) +{ + char out[8192]; + struct timespec t0; + struct timespec t1; + long elapsed_ms; + int rc; + + ASSERT(clock_gettime(CLOCK_MONOTONIC, &t0) == 0); + signal(SIGALRM, trickle_timeout_watchdog); + alarm(4); + rc = sandbox_exec("while true; do echo x; sleep 0.05; done", + out, sizeof(out), 400, NULL); + alarm(0); + signal(SIGALRM, SIG_DFL); + if (skip_if_isolation_denied(rc, out, "test_timeout_kills_trickle_output")) + return 0; + ASSERT(clock_gettime(CLOCK_MONOTONIC, &t1) == 0); + elapsed_ms = (t1.tv_sec - t0.tv_sec) * 1000L + + (t1.tv_nsec - t0.tv_nsec) / 1000000L; + ASSERT(rc == 0); + ASSERT(strstr(out, "timed out") != NULL); + ASSERT(elapsed_ms < 2500); + return 0; +} + static int test_timeout_kills_process(void) { char out[4096]; @@ -856,6 +890,7 @@ int main(void) RUN(test_dev_null_is_writable()); RUN(test_workspace_chdir()); RUN(test_missing_workspace_fail_closed()); + RUN(test_timeout_kills_trickle_output()); RUN(test_timeout_kills_process()); #ifdef __linux__ RUN(test_landlock_probe_without_restrict()); diff --git a/tests/test_shell.c b/tests/test_shell.c index a37341b..a9bafc9 100644 --- a/tests/test_shell.c +++ b/tests/test_shell.c @@ -281,6 +281,55 @@ static void test_shell_caps_output_without_hanging(void) "sequential sleep 9999 did not leak"); } +static void trickle_timeout_watchdog(int sig) +{ + (void)sig; + fprintf(stderr, "FAIL: shell timeout did not fire while stdout trickled\n"); + _exit(2); +} + +/* Idle-only poll accounting used to ignore shell_timeout_sec while the + * child kept the pipe readable. The agent lock stayed held until the + * output buffer filled. */ +static void test_shell_trickle_output_hits_timeout(void) +{ + const tool_t *t = tool_shell_get(); + config_t *cfg = NULL; + char errbuf[256]; + char cfg_path[64]; + char buf[8192]; + FILE *f; + struct timespec t0; + struct timespec t1; + long elapsed_ms; + int r; + + snprintf(cfg_path, sizeof(cfg_path), "/tmp/sc_shell_to_%d.toml", (int)getpid()); + f = fopen(cfg_path, "w"); + MU_ASSERT(f != NULL, "write timeout config"); + fputs("[agent]\nmodel = \"test\"\n[sandbox]\nenabled = false\nshell_timeout_sec = 1\n", f); + fclose(f); + MU_ASSERT(config_load(cfg_path, &cfg, errbuf, sizeof(errbuf)) == 0, "load timeout config"); + tool_shell_set_config(cfg); + memset(buf, 0, sizeof(buf)); + signal(SIGALRM, trickle_timeout_watchdog); + alarm(4); + MU_ASSERT(clock_gettime(CLOCK_MONOTONIC, &t0) == 0, "clock start"); + r = t->execute("{\"command\":\"while true; do echo x; sleep 0.05; done\"}", + buf, sizeof(buf)); + alarm(0); + signal(SIGALRM, SIG_DFL); + MU_ASSERT(clock_gettime(CLOCK_MONOTONIC, &t1) == 0, "clock end"); + elapsed_ms = (t1.tv_sec - t0.tv_sec) * 1000L + + (t1.tv_nsec - t0.tv_nsec) / 1000000L; + tool_shell_set_config(NULL); + config_free(cfg); + unlink(cfg_path); + MU_ASSERT(r == 0, "trickle command returns"); + MU_ASSERT(strstr(buf, "timed out") != NULL, "trickle command reports timeout"); + MU_ASSERT(elapsed_ms < 3000, "trickle command stopped near the 1s timeout"); +} + static void test_shell_caps_output_kills_background_sleep(void) { const tool_t *t = tool_shell_get(); @@ -317,6 +366,7 @@ int main(void) MU_RUN(test_shell_invalid_json); MU_RUN(test_shell_missing_command); MU_RUN(test_shell_caps_output_without_hanging); + MU_RUN(test_shell_trickle_output_hits_timeout); MU_RUN(test_shell_caps_output_kills_background_sleep); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; From def64cf2bef545448e52dc34f3f90ccbef1ee210 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Fri, 16 Oct 2026 00:34:01 -0300 Subject: [PATCH 03/10] test(sandbox): exercise drain_pipe deadlines and env scrub without namespaces GitHub-hosted runners reject unshare(CLONE_NEWUSER), so the sandbox_exec success-path tests skip there (namespace isolation failed) and the wall-clock rewrite of drain_pipe plus the secret-free child environment stayed uncovered on CI, dropping the agent-core coverage gate to 79.8%. Expose the pipe helpers under SHELLCLAW_SANDBOX_TEST (CR-21 pattern) and link them from tests/test_sandbox.c, which now covers poll_remain_ms bounds, the monotonic clamp, drain-until-EOF and the wall deadline with a trickle writer, plus the scrubbed child environment via a forked probe. --- Makefile | 10 ++- src/sandbox/sandbox.c | 16 ++++- tests/test_sandbox.c | 139 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 159 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 7ac876b..9b508e7 100644 --- a/Makefile +++ b/Makefile @@ -128,6 +128,9 @@ ASAP_INVOKE_TEST_O := $(BINDIR)/asap_invoke_test.o SANDBOX_CORE_O := src/sandbox/sandbox.o SANDBOX_LANDLOCK_O := src/sandbox/sandbox_landlock.o SANDBOX_ENV_O := src/sandbox/sandbox_env.o +# Sandbox unit test object: exposes drain_pipe helpers (SHELLCLAW_SANDBOX_TEST) +# so coverage does not depend on namespace privileges the CI runners revoke. +SANDBOX_TEST_O := $(BINDIR)/sandbox_test.o SANDBOX_O := $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) ALLOWLIST_SCAN_O := src/sandbox/allowlist.o ALLOWLIST_PATH_O := src/sandbox/allowlist_path.o @@ -392,6 +395,9 @@ $(SANDBOX_CORE_O): src/sandbox/sandbox.c src/sandbox/sandbox.h src/sandbox/sandb $(SANDBOX_ENV_O): src/sandbox/sandbox_env.c src/sandbox/sandbox_env.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/sandbox/sandbox_env.c +$(SANDBOX_TEST_O): src/sandbox/sandbox.c src/sandbox/sandbox.h src/sandbox/sandbox_landlock.h src/sandbox/sandbox_env.h + $(CC) $(CFLAGS) $(INC) -DSHELLCLAW_SANDBOX_TEST -c -o $@ src/sandbox/sandbox.c + $(SANDBOX_LANDLOCK_O): src/sandbox/sandbox_landlock.c src/sandbox/sandbox_landlock.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/sandbox/sandbox_landlock.c @@ -754,9 +760,9 @@ test_static: tests/test_static.c src/gateway/ui_assets.h src/gateway/static.o $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_static.c src/gateway/static.o $(LDLIBS) $(DSYM_SCRIPT) -test_sandbox: tests/test_sandbox.c $(SANDBOX_O) +test_sandbox: tests/test_sandbox.c $(SANDBOX_TEST_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) @mkdir -p $(BINDIR) - $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_sandbox.c $(SANDBOX_O) $(LDLIBS) + $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_sandbox.c $(SANDBOX_TEST_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) $(LDLIBS) $(DSYM_SCRIPT) test_allowlist: tests/test_allowlist.c $(ALLOWLIST_O) diff --git a/src/sandbox/sandbox.c b/src/sandbox/sandbox.c index 6f5f50b..44c859f 100644 --- a/src/sandbox/sandbox.c +++ b/src/sandbox/sandbox.c @@ -110,8 +110,18 @@ static void cgroup_remove(const char *base, const char *name) #endif /* __linux__ */ +/* Sandbox unit tests link the pipe helpers as globals under + * SHELLCLAW_SANDBOX_TEST (CR-21 provider-test pattern). CI runners revoke + * user namespaces, so the sandbox_exec success-path tests skip there and + * the wall-clock deadline rewrite would otherwise stay uncovered. */ +#ifdef SHELLCLAW_SANDBOX_TEST +#define SANDBOX_UNIT +#else +#define SANDBOX_UNIT static +#endif + /* Wall time since start, or -1 if the monotonic clock cannot be read. */ -static long monotonic_elapsed_ms(const struct timespec *start) +SANDBOX_UNIT long monotonic_elapsed_ms(const struct timespec *start) { struct timespec now; long ms; @@ -123,7 +133,7 @@ static long monotonic_elapsed_ms(const struct timespec *start) return ms < 0 ? 0 : ms; } -static int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap) +SANDBOX_UNIT int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap) { long remain; @@ -140,7 +150,7 @@ static int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap) * the pipe readable otherwise never advances the old counter, so the agent * stays blocked until cap fills. */ -static size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, int *hit_deadline) +SANDBOX_UNIT size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, int *hit_deadline) { struct timespec start; size_t total = 0; diff --git a/tests/test_sandbox.c b/tests/test_sandbox.c index 63cc856..07f9db3 100644 --- a/tests/test_sandbox.c +++ b/tests/test_sandbox.c @@ -17,6 +17,7 @@ #include "sandbox/sandbox.h" #include "sandbox/sandbox_landlock.h" +#include "sandbox/sandbox_env.h" #include #include #include @@ -25,6 +26,7 @@ #include #include #include +#include #ifdef __linux__ #include #include @@ -32,7 +34,6 @@ #include #include #include -#include #endif #define ASSERT(c) do { \ @@ -874,6 +875,137 @@ static int test_command_inherits_cgroup(void) } #endif +/* --- No-namespace units for the wall-clock deadline rewrite (#116) and the + * secret-free child environment (#109). GitHub runners reject unshare, so + * the sandbox_exec paths above skip and these units keep the helpers + * covered on every host. Linked from sandbox_test.o. --- */ + +long monotonic_elapsed_ms(const struct timespec *start); +int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap); +size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, int *hit_deadline); + +static int test_poll_remain_ms_bounds(void) +{ + ASSERT(poll_remain_ms(1000, 0, 100) == 100); + ASSERT(poll_remain_ms(1000, 950, 100) == 50); + ASSERT(poll_remain_ms(1000, 999, 100) == 1); + ASSERT(poll_remain_ms(1000, 2500, 100) == 1); + ASSERT(poll_remain_ms(1000, 0, 5000) == 1000); + return 0; +} + +static int test_monotonic_elapsed_ms_never_negative(void) +{ + struct timespec start; + struct timespec future; + + ASSERT(clock_gettime(CLOCK_MONOTONIC, &start) == 0); + usleep(30000); + ASSERT(monotonic_elapsed_ms(&start) >= 20); + ASSERT(monotonic_elapsed_ms(&start) < 60000); + ASSERT(clock_gettime(CLOCK_MONOTONIC, &future) == 0); + future.tv_sec += 10; + ASSERT(monotonic_elapsed_ms(&future) == 0); + return 0; +} + +static int test_drain_pipe_reads_until_eof(void) +{ + char buf[64]; + const char msg[] = "hi"; + size_t total; + int hit = -1; + int fds[2]; + pid_t pid; + int st; + + ASSERT(pipe(fds) == 0); + pid = fork(); + if (pid == 0) { + ssize_t w; + close(fds[0]); + w = write(fds[1], msg, sizeof(msg) - 1); + (void)w; + close(fds[1]); + exit(0); + } + close(fds[1]); + total = drain_pipe(fds[0], buf, sizeof(buf), 5000, &hit); + close(fds[0]); + waitpid(pid, &st, 0); + ASSERT(total == 2); + ASSERT(hit == 0); + ASSERT(buf[0] == 'h' && buf[1] == 'i'); + return 0; +} + +static int test_drain_pipe_stops_at_wall_deadline(void) +{ + char buf[256]; + char byte = 'x'; + size_t total; + int hit = -1; + int fds[2]; + pid_t pid; + int st; + + ASSERT(pipe(fds) == 0); + pid = fork(); + if (pid == 0) { + close(fds[0]); + /* Trickle one byte every 20 ms so the pipe stays readable and the + * cap is never reached: only the wall deadline can stop the loop. */ + for (;;) { + if (write(fds[1], &byte, 1) <= 0) + exit(0); + usleep(20000); + } + } + close(fds[1]); + total = drain_pipe(fds[0], buf, sizeof(buf), 400, &hit); + close(fds[0]); + waitpid(pid, &st, 0); + ASSERT(hit == 1); + ASSERT(total >= 1); + ASSERT(total < sizeof(buf) - 1); + return 0; +} + +static int test_scrubbed_env_child_loses_secrets(void) +{ + pid_t pid; + int st; + int status; + + pid = fork(); + if (pid == 0) { + if (setenv("SHELLCLAW_AUDIT_SECRET", "leak", 1) != 0) + exit(4); + if (setenv("PATH", "/usr/bin", 1) != 0) + exit(5); + sandbox_scrub_command_environment("/ws/home"); + if (getenv("SHELLCLAW_AUDIT_SECRET")) + exit(1); + if (!getenv("PATH") || strcmp(getenv("PATH"), "/usr/bin") != 0) + exit(2); + if (!getenv("HOME") || strcmp(getenv("HOME"), "/ws/home") != 0) + exit(3); + exit(0); + } + if (waitpid(pid, &st, 0) < 0) { + fprintf(stderr, "FAIL: waitpid scrub child: %s\n", strerror(errno)); + return 1; + } + if (!WIFEXITED(st)) + return 1; + status = WEXITSTATUS(st); + if (status != 0) { + fprintf(stderr, "FAIL: scrub env child exit=%d\n", status); + return 1; + } + return 0; +} + int main(void) { RUN(test_isolation_denied_helper()); @@ -883,6 +1015,11 @@ int main(void) RUN(test_command_does_not_inherit_host_secrets()); RUN(test_null_cmd_returns_error()); RUN(test_zero_cap_returns_error()); + RUN(test_scrubbed_env_child_loses_secrets()); + RUN(test_poll_remain_ms_bounds()); + RUN(test_monotonic_elapsed_ms_never_negative()); + RUN(test_drain_pipe_reads_until_eof()); + RUN(test_drain_pipe_stops_at_wall_deadline()); RUN(test_exit_nonzero_runs()); RUN(test_exit_122_is_not_isolation_failure()); RUN(test_exit_123_with_workspace_is_not_isolation_failure()); From 983685b4c64faa8f3ae6a6a293d485222176779b Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Fri, 16 Oct 2026 00:42:32 -0300 Subject: [PATCH 04/10] build(sandbox): keep the sandbox test object under src for the coverage capture scripts/coverage.sh captures --directory src only; a unit-test object in build/ produced a .gcda the capture never saw, so drain_pipe stayed uncovered and the agent-core gate fell to 79.1%. The ws_test.o pattern already keeps its test object under src/ for the same reason. --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9b508e7..3e01565 100644 --- a/Makefile +++ b/Makefile @@ -130,7 +130,8 @@ SANDBOX_LANDLOCK_O := src/sandbox/sandbox_landlock.o SANDBOX_ENV_O := src/sandbox/sandbox_env.o # Sandbox unit test object: exposes drain_pipe helpers (SHELLCLAW_SANDBOX_TEST) # so coverage does not depend on namespace privileges the CI runners revoke. -SANDBOX_TEST_O := $(BINDIR)/sandbox_test.o +# Lives under src/sandbox/ so the coverage.sh capture (--directory src) merges it. +SANDBOX_TEST_O := src/sandbox/sandbox_test.o SANDBOX_O := $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) ALLOWLIST_SCAN_O := src/sandbox/allowlist.o ALLOWLIST_PATH_O := src/sandbox/allowlist_path.o From 87c42daf229099aa3cd7aa80612c4b05c7b99e21 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Fri, 16 Oct 2026 00:53:05 -0300 Subject: [PATCH 05/10] build(sandbox): remove the sandbox test object in make clean The unit-test step builds src/sandbox/sandbox_test.o without --coverage in the same runner workdir; make clean did not delete it, so the coverage build reused the uninstrumented object and the capture lost the drain_pipe traces again. Precedent: WS_TEST_O is pruned the same way. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3e01565..aa14bcf 100644 --- a/Makefile +++ b/Makefile @@ -923,6 +923,6 @@ clean: clean-root-dsym rm -f $(OBJS) $(PROVIDER_COMMON_O) $(STUB_O) $(ANTHROPIC_O) $(OPENAI_COMPAT_O) $(OPENAI_O) $(LOCAL_O) $(ROUTER_O) $(CJSON_O) $(TWEETNACL_O) $(ANTHROPIC_TEST_O) $(OPENAI_TEST_O) $(LOCAL_TEST_O) $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(CHANNEL_TG_TEST_O) $(CHANNEL_COMMON_O) $(CHANNEL_STUB_O) $(CHANNEL_CLI_O) $(CHANNEL_TG_O) $(CHANNEL_DISCORD_O) $(DISCORD_HELPERS_O) $(CHANNEL_HEARTBEAT_O) $(CHANNEL_WEBCHAT_O) $(AUTH_O) $(STATIC_O) $(HTTP_O) $(HTTP_LWS_O) $(ASAP_HTTP_BODY_O) $(ROUTES_O) $(ROUTES_HARDWARE_O) $(WS_O) $(MANIFEST_O) $(MANIFEST_PROFILES_O) $(MANIFEST_BUILD_O) $(MANIFEST_SIGN_O) $(MANIFEST_KEYS_O) $(ENVELOPE_O) $(ULID_O) $(CLIENT_O) $(ASAP_REGISTRY_O) $(SERVER_O) $(ASAP_LOG_O) $(RATE_LIMIT_O) $(SHELL_O) $(WEBSEARCH_O) $(FILE_O) $(REGISTRY_O) $(CONTEXT_O) $(CONTEXT_CACHE_O) $(CONTEXT_HTTP_O) $(CONTEXT_GEO_O) $(CRYPTO_O) $(JCS_O) $(HARDWARE_STUB_O) $(HARDWARE_INIT_O) $(HARDWARE_GPIO_SNAPSHOT_O) $(HARDWARE_TEGRASTATS_O) $(HARDWARE_TOOLS_O) $(BOARD_DETECT_O) src/hardware/hardware_libgpiod.o $(HARDWARE_I2C_O) $(HARDWARE_CAMERA_O) $(CRON_O) $(ASAP_INVOKE_O) $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) $(ALLOWLIST_SCAN_O) $(ALLOWLIST_PATH_O) rm -f src/gateway/ui_assets.h find . -name '*.gcno' -o -name '*.gcda' -o -name '*.gcov' | xargs rm -f 2>/dev/null || true - rm -f $(WS_TEST_O) $(BINDIR)/asap_registry_test.o $(BINDIR)/asap_invoke_test.o $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(BINDIR)/shellclaw $(BINDIR)/test_tweetnacl_smoke $(BINDIR)/test_config $(BINDIR)/test_config_patch $(BINDIR)/test_memory $(BINDIR)/test_skill $(BINDIR)/test_provider $(BINDIR)/test_anthropic $(BINDIR)/test_openai $(BINDIR)/test_local_provider $(BINDIR)/test_router $(BINDIR)/test_heartbeat $(BINDIR)/test_crypto $(BINDIR)/test_hardware_stub $(BINDIR)/test_board_detect $(BINDIR)/test_hardware_libgpiod $(BINDIR)/test_hardware_i2c $(BINDIR)/test_hardware_camera $(BINDIR)/test_pin_tables $(BINDIR)/test_hardware_init $(BINDIR)/test_hardware_tools $(BINDIR)/test_registry $(BINDIR)/test_ws $(BINDIR)/test_agent $(BINDIR)/test_channel $(BINDIR)/test_cli $(BINDIR)/test_shell $(BINDIR)/test_file $(BINDIR)/test_telegram $(BINDIR)/test_discord_helpers $(BINDIR)/test_web_search $(BINDIR)/test_cron $(BINDIR)/test_context $(BINDIR)/test_manifest_build $(BINDIR)/test_manifest_keys $(BINDIR)/test_jcs $(BINDIR)/test_asap_envelope $(BINDIR)/test_asap_ulid $(BINDIR)/test_asap_client $(BINDIR)/test_asap_registry $(BINDIR)/test_asap_server $(BINDIR)/test_asap_invoke $(BINDIR)/test_asap_log $(BINDIR)/test_auth $(BINDIR)/test_gateway_http $(BINDIR)/test_static $(BINDIR)/test_sandbox $(BINDIR)/test_allowlist $(BINDIR)/test_rate_limit + rm -f $(WS_TEST_O) $(SANDBOX_TEST_O) $(BINDIR)/asap_registry_test.o $(BINDIR)/asap_invoke_test.o $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(BINDIR)/shellclaw $(BINDIR)/test_tweetnacl_smoke $(BINDIR)/test_config $(BINDIR)/test_config_patch $(BINDIR)/test_memory $(BINDIR)/test_skill $(BINDIR)/test_provider $(BINDIR)/test_anthropic $(BINDIR)/test_openai $(BINDIR)/test_local_provider $(BINDIR)/test_router $(BINDIR)/test_heartbeat $(BINDIR)/test_crypto $(BINDIR)/test_hardware_stub $(BINDIR)/test_board_detect $(BINDIR)/test_hardware_libgpiod $(BINDIR)/test_hardware_i2c $(BINDIR)/test_hardware_camera $(BINDIR)/test_pin_tables $(BINDIR)/test_hardware_init $(BINDIR)/test_hardware_tools $(BINDIR)/test_registry $(BINDIR)/test_ws $(BINDIR)/test_agent $(BINDIR)/test_channel $(BINDIR)/test_cli $(BINDIR)/test_shell $(BINDIR)/test_file $(BINDIR)/test_telegram $(BINDIR)/test_discord_helpers $(BINDIR)/test_web_search $(BINDIR)/test_cron $(BINDIR)/test_context $(BINDIR)/test_manifest_build $(BINDIR)/test_manifest_keys $(BINDIR)/test_jcs $(BINDIR)/test_asap_envelope $(BINDIR)/test_asap_ulid $(BINDIR)/test_asap_client $(BINDIR)/test_asap_registry $(BINDIR)/test_asap_server $(BINDIR)/test_asap_invoke $(BINDIR)/test_asap_log $(BINDIR)/test_auth $(BINDIR)/test_gateway_http $(BINDIR)/test_static $(BINDIR)/test_sandbox $(BINDIR)/test_allowlist $(BINDIR)/test_rate_limit rm -rf $(BINDIR)/*.dSYM $(DSYMDIR) rm -f $(BOOTSTRAP_DISPATCH_STUB_O) $(TOOL_RELOAD_STUB_O) $(RELOAD_CHANNEL_STUB_O) $(HTTP_RELOAD_STUB_O) From 026eca837fe161cddcbd8fcfcd732c1c37dc6288 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:25:04 -0300 Subject: [PATCH 06/10] refactor(sandbox,shell): share the wall-clock deadline helpers monotonic_elapsed_ms was duplicated verbatim between sandbox.c and shell.c (PR #125 review, Should Fix 4). Export it and poll_remain_ms through sandbox.h so both timeout loops run one implementation, and drop the two pieces of dead weight the review called out: - the have_clock fallback: CLOCK_MONOTONIC failure is not a realistic case (vDSO), and if it ever failed the deadline simply never fires while poll slices stay bounded; - the remain < 1 clamp in poll_remain_ms: unreachable, both loops break at elapsed >= timeout before calling the helper. The sandbox_exec doc now states that the timeout clock starts only after the isolation handshake (review Nice item 4). test_sandbox.c takes the two helper prototypes from the header (the drain_pipe prototype stays: it only becomes a global under SHELLCLAW_SANDBOX_TEST) and drops the assertion that covered the removed clamp branch. No behavior change: poll bounds, monotonic clamp, drain EOF, wall deadline and trickle timeout tests stay green. Refs: #125 --- src/sandbox/sandbox.c | 31 +++++++++++------------------ src/sandbox/sandbox.h | 22 +++++++++++++++++++++ src/tools/shell.c | 45 ++++++++++--------------------------------- tests/test_sandbox.c | 3 --- 4 files changed, 43 insertions(+), 58 deletions(-) diff --git a/src/sandbox/sandbox.c b/src/sandbox/sandbox.c index 44c859f..d0b7b8f 100644 --- a/src/sandbox/sandbox.c +++ b/src/sandbox/sandbox.c @@ -110,18 +110,18 @@ static void cgroup_remove(const char *base, const char *name) #endif /* __linux__ */ -/* Sandbox unit tests link the pipe helpers as globals under +/* Sandbox unit tests link drain_pipe as a global under * SHELLCLAW_SANDBOX_TEST (CR-21 provider-test pattern). CI runners revoke * user namespaces, so the sandbox_exec success-path tests skip there and - * the wall-clock deadline rewrite would otherwise stay uncovered. */ + * the wall-clock deadline loop would otherwise stay uncovered. */ #ifdef SHELLCLAW_SANDBOX_TEST #define SANDBOX_UNIT #else #define SANDBOX_UNIT static #endif -/* Wall time since start, or -1 if the monotonic clock cannot be read. */ -SANDBOX_UNIT long monotonic_elapsed_ms(const struct timespec *start) +/* Docs in sandbox.h. */ +long monotonic_elapsed_ms(const struct timespec *start) { struct timespec now; long ms; @@ -133,13 +133,11 @@ SANDBOX_UNIT long monotonic_elapsed_ms(const struct timespec *start) return ms < 0 ? 0 : ms; } -SANDBOX_UNIT int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap) +int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap) { long remain; remain = (long)timeout_ms - elapsed_ms; - if (remain < 1) - return 1; if (remain > slice_cap) return slice_cap; return (int)remain; @@ -154,7 +152,6 @@ SANDBOX_UNIT size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, in { struct timespec start; size_t total = 0; - int have_clock; long elapsed_ms = 0; char chunk[512]; @@ -162,20 +159,17 @@ SANDBOX_UNIT size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, in *hit_deadline = 0; if (cap == 0) return 0; - have_clock = clock_gettime(CLOCK_MONOTONIC, &start) == 0; + /* Unchecked on purpose: CLOCK_MONOTONIC failure is not a realistic case + * (vDSO). If it ever failed, monotonic_elapsed_ms keeps returning -1, + * the deadline never fires, and poll slices stay bounded. */ + (void)clock_gettime(CLOCK_MONOTONIC, &start); while (total < cap - 1) { struct pollfd pfd; int slice; int r; ssize_t n; - if (have_clock) { - long wall = monotonic_elapsed_ms(&start); - if (wall < 0) - have_clock = 0; - else - elapsed_ms = wall; - } + elapsed_ms = monotonic_elapsed_ms(&start); if (elapsed_ms >= (long)timeout_ms) { if (hit_deadline) *hit_deadline = 1; @@ -191,11 +185,8 @@ SANDBOX_UNIT size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, in continue; break; } - if (r == 0) { - if (!have_clock) - elapsed_ms += slice; + if (r == 0) continue; - } n = read(fd, chunk, sizeof(chunk)); if (n <= 0) break; diff --git a/src/sandbox/sandbox.h b/src/sandbox/sandbox.h index 277d513..37bc7e8 100644 --- a/src/sandbox/sandbox.h +++ b/src/sandbox/sandbox.h @@ -16,6 +16,7 @@ #define SHELLCLAW_SANDBOX_H #include +#include #ifdef __cplusplus extern "C" { @@ -45,6 +46,25 @@ typedef struct sandbox_config { const char *cgroup_base; } sandbox_config_t; +/** + * Wall-clock milliseconds elapsed since @p start, or -1 when the monotonic + * clock cannot be read. Shared by the sandbox drain loop and the unsandboxed + * shell loop so steady output cannot bypass a timeout (#116). + * + * Example: long ms = monotonic_elapsed_ms(&start); + */ +long monotonic_elapsed_ms(const struct timespec *start); + +/** + * Poll slice for the remaining wall-clock deadline: the positive remainder + * of @p timeout_ms minus @p elapsed_ms, capped at @p slice_cap. Call only + * after checking @p elapsed_ms < @p timeout_ms; an expired deadline is not + * clamped here, so a post-deadline call can return a negative value. + * + * Example: poll_remain_ms(1000, 950, 100) returns 50. + */ +int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap); + /** * Execute @p cmd inside an isolated child process and capture output. * @@ -65,6 +85,8 @@ typedef struct sandbox_config { * @param out Buffer for captured stdout+stderr. Always NUL-terminated on success. * @param out_cap Capacity of @p out (including NUL byte). Must be > 0. * @param timeout_ms Maximum wall-clock milliseconds before SIGKILL. 0 = default (10 000 ms). + * The timeout clock starts only after the isolation handshake, + * so slow namespace/Landlock setup is not counted. * @param cfg Optional sandbox configuration. NULL = use built-in defaults. * @return 0 on success (command ran; check output for exit status text), * -1 on system error (pipe/fork/clone/isolation failure). diff --git a/src/tools/shell.c b/src/tools/shell.c index 056d07a..a48683b 100644 --- a/src/tools/shell.c +++ b/src/tools/shell.c @@ -66,19 +66,6 @@ static void kill_command_tree(pid_t pid) (void)kill(pid, SIGKILL); } -/* Wall time since start, or -1 if the monotonic clock cannot be read. */ -static long monotonic_elapsed_ms(const struct timespec *start) -{ - struct timespec now; - long ms; - - if (clock_gettime(CLOCK_MONOTONIC, &now) != 0) - return -1; - ms = (now.tv_sec - start->tv_sec) * 1000L - + (now.tv_nsec - start->tv_nsec) / 1000000L; - return ms < 0 ? 0 : ms; -} - /* Always SIGKILL the process group first so background grandchildren cannot * leak after the shell has already exited (SIGPIPE, or `cmd &`). Then reap * the tracked pid. Matching the hang fix for #69 / #96. */ @@ -129,8 +116,8 @@ static int run_unsandboxed(const char *command, int timeout_sec, size_t total = 0; int timed_out = 0; long elapsed_ms = 0; + long timeout_ms; struct timespec start_ts; - int have_clock; char buf[256]; if (pipe(pipefd) != 0) { snprintf(result_buf, max_len, "{\"error\":\"pipe failed\"}"); @@ -155,23 +142,20 @@ static int run_unsandboxed(const char *command, int timeout_sec, (void)setpgid(pid, pid); close(pipefd[1]); result_buf[0] = '\0'; - have_clock = clock_gettime(CLOCK_MONOTONIC, &start_ts) == 0; + /* Unchecked on purpose: CLOCK_MONOTONIC failure is not a realistic case + * (vDSO). If it ever failed, monotonic_elapsed_ms keeps returning -1, + * the deadline never fires, and poll slices stay bounded. */ + (void)clock_gettime(CLOCK_MONOTONIC, &start_ts); /* Wall time, not idle poll time: a steady writer never used to trip * the timeout, so the agent lock stayed held until the buffer filled. */ + timeout_ms = (long)timeout_sec * 1000L; while (total < max_len - 1) { struct pollfd pfd; - long remain; int rem; int r; ssize_t n; - if (have_clock) { - long wall = monotonic_elapsed_ms(&start_ts); - if (wall < 0) - have_clock = 0; - else - elapsed_ms = wall; - } - if (elapsed_ms >= (long)timeout_sec * 1000L) { + elapsed_ms = monotonic_elapsed_ms(&start_ts); + if (elapsed_ms >= timeout_ms) { timed_out = 1; kill_command_tree(pid); break; @@ -179,23 +163,14 @@ static int run_unsandboxed(const char *command, int timeout_sec, pfd.fd = pipefd[0]; pfd.events = POLLIN; pfd.revents = 0; - remain = (long)timeout_sec * 1000L - elapsed_ms; - if (remain > 5000) - rem = 5000; - else if (remain < 1) - rem = 1; - else - rem = (int)remain; + rem = poll_remain_ms((int)timeout_ms, elapsed_ms, 5000); r = poll(&pfd, 1, rem); if (r < 0) { if (errno == EINTR) continue; break; } - if (r == 0) { - if (!have_clock) - elapsed_ms += rem; + if (r == 0) continue; - } n = read(pipefd[0], buf, sizeof(buf)); if (n <= 0) break; append_unsandboxed_output(result_buf, max_len, &total, buf, (size_t)n); diff --git a/tests/test_sandbox.c b/tests/test_sandbox.c index 07f9db3..07af9a4 100644 --- a/tests/test_sandbox.c +++ b/tests/test_sandbox.c @@ -880,8 +880,6 @@ static int test_command_inherits_cgroup(void) * the sandbox_exec paths above skip and these units keep the helpers * covered on every host. Linked from sandbox_test.o. --- */ -long monotonic_elapsed_ms(const struct timespec *start); -int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap); size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, int *hit_deadline); static int test_poll_remain_ms_bounds(void) @@ -889,7 +887,6 @@ static int test_poll_remain_ms_bounds(void) ASSERT(poll_remain_ms(1000, 0, 100) == 100); ASSERT(poll_remain_ms(1000, 950, 100) == 50); ASSERT(poll_remain_ms(1000, 999, 100) == 1); - ASSERT(poll_remain_ms(1000, 2500, 100) == 1); ASSERT(poll_remain_ms(1000, 0, 5000) == 1000); return 0; } From 4d4d3568ae913cf3b4e179388710a295682339e2 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:25:32 -0300 Subject: [PATCH 07/10] test(sandbox): strengthen the env-scrub regressions Per the PR #125 review (Should Fix 5) the regressions were weaker than they looked: the scrub-child unit never checked that LD_PRELOAD is dropped, and the sandbox_exec secret test passed a NULL workspace, so the HOME=workspace branch of the scrub never ran. test_scrubbed_env_child_loses_secrets now sets LD_PRELOAD before the scrub and asserts it absent after; the child never execs, so injecting the variable is safe on every runner. test_command_does_not_inherit_ host_secrets now runs against a real mkdtemp workspace and asserts the workspace path is visible as HOME in the command output. Refs: #125 --- tests/test_sandbox.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/test_sandbox.c b/tests/test_sandbox.c index 07af9a4..97a819f 100644 --- a/tests/test_sandbox.c +++ b/tests/test_sandbox.c @@ -153,21 +153,35 @@ static int test_stderr_captured(void) static int test_command_does_not_inherit_host_secrets(void) { char out[4096]; + char ws[] = "/tmp/sc_sb_env_XXXXXX"; + sandbox_config_t cfg; + char *dir; int rc; const char *secret = "leaked_via_env"; ASSERT(setenv("SHELLCLAW_AUDIT_SECRET", secret, 1) == 0); ASSERT(setenv("ANTHROPIC_API_KEY", secret, 1) == 0); + dir = mkdtemp(ws); + if (!dir) { + fprintf(stderr, "test_command_does_not_inherit_host_secrets: mkdtemp failed\n"); + return 1; + } + memset(&cfg, 0, sizeof cfg); + cfg.workspace_path = dir; rc = sandbox_exec( "printenv SHELLCLAW_AUDIT_SECRET ANTHROPIC_API_KEY; " - "tr '\\0' '\\n' < /proc/1/environ 2>/dev/null; echo PATH_OK", - out, sizeof out, 5000, NULL); + "tr '\\0' '\\n' < /proc/1/environ 2>/dev/null; " + "echo HOME=$(printenv HOME); echo PATH_OK", + out, sizeof out, 5000, &cfg); + rmdir(dir); unsetenv("SHELLCLAW_AUDIT_SECRET"); unsetenv("ANTHROPIC_API_KEY"); if (skip_if_isolation_denied(rc, out, "test_command_does_not_inherit_host_secrets")) return 0; ASSERT(rc == 0); ASSERT(strstr(out, secret) == NULL); + /* A real workspace exercises the HOME=workspace branch of the scrub. */ + ASSERT(strstr(out, dir) != NULL); ASSERT(strstr(out, "PATH_OK") != NULL); return 0; } @@ -980,9 +994,13 @@ static int test_scrubbed_env_child_loses_secrets(void) exit(4); if (setenv("PATH", "/usr/bin", 1) != 0) exit(5); + if (setenv("LD_PRELOAD", "/no/such/sc_fake_lib.so", 1) != 0) + exit(6); sandbox_scrub_command_environment("/ws/home"); if (getenv("SHELLCLAW_AUDIT_SECRET")) exit(1); + if (getenv("LD_PRELOAD")) + exit(7); if (!getenv("PATH") || strcmp(getenv("PATH"), "/usr/bin") != 0) exit(2); if (!getenv("HOME") || strcmp(getenv("HOME"), "/ws/home") != 0) From 9fb01d006c30aa6ef78bd03a2a57a0b489ff2ac3 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:26:25 -0300 Subject: [PATCH 08/10] test(shell,sandbox): assert trickle timeouts kill the command Per the PR #125 review (Should Fix 6) the trickle output timeout tests only asserted the elapsed time and the 'timed out' text; neither proved the command tree was actually killed, which is the whole point of the group kill and PR_SET_PDEATHSIG on the deadline path. Both trickle tests now leave a marker sleep 9999 in the command's process group. test_shell reuses the count_sleep_argv / sleep_argv_did_not_grow pattern from the output-cap tests to prove no marker sleeps survive; test_sandbox scans /proc with the same retry loop test_timeout_kills_process already uses (Linux-only, matching that precedent: the non-Linux fallback has no process-group kill). Tests only; src/ is untouched. Refs: #125 --- tests/test_sandbox.c | 20 +++++++++++++++++++- tests/test_shell.c | 9 ++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/tests/test_sandbox.c b/tests/test_sandbox.c index 97a819f..e082059 100644 --- a/tests/test_sandbox.c +++ b/tests/test_sandbox.c @@ -334,7 +334,9 @@ static int test_timeout_kills_trickle_output(void) ASSERT(clock_gettime(CLOCK_MONOTONIC, &t0) == 0); signal(SIGALRM, trickle_timeout_watchdog); alarm(4); - rc = sandbox_exec("while true; do echo x; sleep 0.05; done", + /* The background sleep is a grandchild in the command process group, + * so the deadline kill must wipe it along with the trickling shell. */ + rc = sandbox_exec("sleep 9999 & while true; do echo x; sleep 0.05; done", out, sizeof(out), 400, NULL); alarm(0); signal(SIGALRM, SIG_DFL); @@ -346,6 +348,22 @@ static int test_timeout_kills_trickle_output(void) ASSERT(rc == 0); ASSERT(strstr(out, "timed out") != NULL); ASSERT(elapsed_ms < 2500); +#ifdef __linux__ + { + int tries; + for (tries = 0; tries < 20; tries++) { + if (!proc_cmdline_has("sleep 9999")) + break; + { + struct timespec ts; + ts.tv_sec = 0; + ts.tv_nsec = 50 * 1000 * 1000; + nanosleep(&ts, NULL); + } + } + ASSERT(proc_cmdline_has("sleep 9999") == 0); + } +#endif return 0; } diff --git a/tests/test_shell.c b/tests/test_shell.c index a9bafc9..b612139 100644 --- a/tests/test_shell.c +++ b/tests/test_shell.c @@ -302,6 +302,7 @@ static void test_shell_trickle_output_hits_timeout(void) struct timespec t0; struct timespec t1; long elapsed_ms; + int before; int r; snprintf(cfg_path, sizeof(cfg_path), "/tmp/sc_shell_to_%d.toml", (int)getpid()); @@ -311,11 +312,15 @@ static void test_shell_trickle_output_hits_timeout(void) fclose(f); MU_ASSERT(config_load(cfg_path, &cfg, errbuf, sizeof(errbuf)) == 0, "load timeout config"); tool_shell_set_config(cfg); + before = count_sleep_argv("9999"); + MU_ASSERT(before >= 0, "can count sleep 9999 before trickle timeout"); memset(buf, 0, sizeof(buf)); signal(SIGALRM, trickle_timeout_watchdog); alarm(4); MU_ASSERT(clock_gettime(CLOCK_MONOTONIC, &t0) == 0, "clock start"); - r = t->execute("{\"command\":\"while true; do echo x; sleep 0.05; done\"}", + /* The background sleep sits in the command process group, so the + * timeout path must group-kill it together with the trickling shell. */ + r = t->execute("{\"command\":\"sleep 9999 & while true; do echo x; sleep 0.05; done\"}", buf, sizeof(buf)); alarm(0); signal(SIGALRM, SIG_DFL); @@ -328,6 +333,8 @@ static void test_shell_trickle_output_hits_timeout(void) MU_ASSERT(r == 0, "trickle command returns"); MU_ASSERT(strstr(buf, "timed out") != NULL, "trickle command reports timeout"); MU_ASSERT(elapsed_ms < 3000, "trickle command stopped near the 1s timeout"); + MU_ASSERT(sleep_argv_did_not_grow("9999", before), + "timeout path killed the background sleep 9999"); } static void test_shell_caps_output_kills_background_sleep(void) From c37b03151027e5a03e11dc2f9979541350f95443 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:26:55 -0300 Subject: [PATCH 09/10] docs(sandbox): record the env scrub and wall-clock timeout changes Per the PR #125 review (Should Fix 1-3) both behavior changes were user-visible but undocumented: - CHANGELOG [Unreleased]/Fixed now carries the env scrub (sandboxed commands keep only the whitelist, so LD_*, CUDA_* and proxy variables no longer reach sh -c) and the wall-clock timeout enforcement on both shell paths. - docs/SECURITY.md gains an explicit command environment model: the scrubbed whitelist for the sandboxed path, the Jetson-relevant variables that do not pass (LD_LIBRARY_PATH, CUDA_*, TMPDIR, SSL_CERT_FILE, proxies, no passthrough list in v1.0), and the intentional full-host-environment behavior of the sandbox.enabled=false fallback, where printenv works because it is reduced isolation, not a security boundary. Refs: #125 --- CHANGELOG.md | 2 ++ docs/SECURITY.md | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ce4ee7..fc0f82d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ## [Unreleased] ### Fixed +- Sandboxed shell commands no longer inherit the agent environment: the command process keeps only `PATH`, locale (`LANG`, `LC_ALL`, `LC_CTYPE`), `TERM`, `TZ`, and `HOME` (set to the workspace), so a prompt-injected `printenv` cannot read provider API keys or bot tokens. `LD_*`, `CUDA_*`, and proxy variables are dropped before `exec`; see the environment model in `docs/SECURITY.md` (#109). +- The shell command timeout is enforced against a monotonic wall clock on both the sandboxed and the unsandboxed paths, so a command that keeps stdout trickling can no longer outlive `shell_timeout_sec` and hold the agent lock until the output buffer fills (#116). - Cron re-offer waits the full timeout when a due id could not be stored in the 16-slot table, and an ack frees that slot so the next due job can be delivered and then backed off. - Landlock device grants stay `/dev/null`, `/dev/zero`, `/dev/urandom`, and `/dev/tty` without `IOCTL_DEV`. A failed `landlock_add_rule` fails the ruleset closed. - Dashboard config save and JSON patch validation use a unique temp plus `fsync`, and the live gateway config pointer is published once under `agent_lock`. diff --git a/docs/SECURITY.md b/docs/SECURITY.md index f3ff695..c6ac5e1 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -40,7 +40,7 @@ The primary goals are: prevent sandboxed shell commands from escaping to host de | Surface | Mechanism | Notes | |---------|-----------|-------| | Shell (sandbox on) | `fork()` + user ns when needed + `unshare(CLONE_NEWNS \| CLONE_NEWNET \| CLONE_NEWPID)` + fork for PID 1 + Landlock workspace bound + `prctl(PR_SET_NO_NEW_PRIVS)` + secret-free environment | Fail-closed if namespaces or Landlock cannot apply. The command child keeps `PATH`, locale, `TERM`, `TZ`, and `HOME` set to the workspace. Host API keys and bot tokens are not inherited. Isolation uses a control pipe, not `sh` exit 122/123. See [Linux sandbox (Jetson)](#linux-sandbox-jetson) | -| Shell (sandbox off) | Plain `fork()` + substring fallback blocklist | **Not** a security boundary; stderr warning | +| Shell (sandbox off) | Plain `fork()` + substring fallback blocklist | **Not** a security boundary; the full host environment is inherited — `printenv` works there (see [Command environment model](#command-environment-model)); stderr warning | | Allowlist | Substring blocklist + workspace containment (ancestor walk, quoted/embedded `/` `~`, `$HOME`/`$PWD`, `file:` URLs, relative names) | Defense-in-depth string scan; Landlock is the kernel host-FS bound | | Landlock | Ruleset on configured `workspace_path` (RW workspace + traverse-only `/` + RO `/bin` `/usr` `/lib` and `/etc/ssl/certs` only; RW `/dev/null`, `/dev/zero`, `/dev/urandom`, `/dev/tty` without `IOCTL_DEV`). Child `fchdir`s the workspace before `restrict_self`. A failed `landlock_add_rule` fails the ruleset closed. | Primary host-FS gate; `/` is `READ_DIR` only so `/etc/passwd` stays closed. `/etc/ssl/private` is not granted. Blocks symlink and `chr(47)+` host reads | | cgroups v2 | `memory.max`, `cpu.max`; isolator writes `cgroup.procs` before the command fork | Best-effort; non-fatal if cgroup write fails | @@ -48,6 +48,19 @@ The primary goals are: prevent sandboxed shell commands from escaping to host de Implementation: [`src/sandbox/sandbox.c`](../src/sandbox/sandbox.c), [`src/sandbox/allowlist.c`](../src/sandbox/allowlist.c). +### Command environment model + +The two shell paths differ in environment exposure as much as in isolation, so the model is stated explicitly: + +| Path | Environment the command sees | `printenv` inside the command | +|------|------------------------------|------------------------------| +| Sandboxed (`sandbox.enabled = true`) | Scrubbed whitelist only: `PATH`, `LANG`, `LC_ALL`, `LC_CTYPE`, `TERM`, and `TZ` keep their host values when set, and `HOME` is set to the configured workspace (when one is set). Everything else is dropped (`clearenv` before `exec`). | Only the whitelist: provider keys, bot tokens, and loader variables are gone. | +| Unsandboxed fallback (`sandbox.enabled = false`, the default when the key is omitted) | The full agent (host) environment is inherited. | Works — it prints the agent's environment, secrets included. Intentional: this path is reduced isolation, not a security boundary; a stderr warning is printed per command. | + +Jetson-relevant variables that deliberately do **not** pass into the sandbox: `LD_LIBRARY_PATH` and all other `LD_*` (including `LD_PRELOAD`), `CUDA_HOME` and other `CUDA_*`, `TMPDIR`, `SSL_CERT_FILE`, and proxy variables (`http_proxy`, `https_proxy`, `no_proxy`, …). Sandboxed workloads that rely on them must set them per command (for example `LD_LIBRARY_PATH=… cmd`); there is no `[sandbox]` passthrough list in v1.0. Tool paths that genuinely need host setup (camera capture) run argv-only `execvp` in the agent process, outside the shell sandbox. + +Implementation: [`src/sandbox/sandbox_env.c`](../src/sandbox/sandbox_env.c). + --- ## Linux sandbox (Jetson) From d708a8c1d3d72d01a61a0772aaab641c8527b7cc Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:29:23 -0300 Subject: [PATCH 10/10] refactor(sandbox,shell): loop-scope the deadline elapsed counter Fallout from sharing the wall-clock helpers: without the have_clock fallback both deadline loops recompute elapsed_ms at the top of every iteration, so the function-level '= 0' initializer became dead code and cppcheck now reports unreadVariable/variableScope in both files. Declare the counter inside the loops next to the other iteration locals instead. No behavior change; test_sandbox and test_shell stay green. Refs: #125 --- src/sandbox/sandbox.c | 2 +- src/tools/shell.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sandbox/sandbox.c b/src/sandbox/sandbox.c index d0b7b8f..fefddc5 100644 --- a/src/sandbox/sandbox.c +++ b/src/sandbox/sandbox.c @@ -152,7 +152,6 @@ SANDBOX_UNIT size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, in { struct timespec start; size_t total = 0; - long elapsed_ms = 0; char chunk[512]; if (hit_deadline) @@ -165,6 +164,7 @@ SANDBOX_UNIT size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, in (void)clock_gettime(CLOCK_MONOTONIC, &start); while (total < cap - 1) { struct pollfd pfd; + long elapsed_ms; int slice; int r; ssize_t n; diff --git a/src/tools/shell.c b/src/tools/shell.c index a48683b..bc70a2e 100644 --- a/src/tools/shell.c +++ b/src/tools/shell.c @@ -115,7 +115,6 @@ static int run_unsandboxed(const char *command, int timeout_sec, pid_t pid; size_t total = 0; int timed_out = 0; - long elapsed_ms = 0; long timeout_ms; struct timespec start_ts; char buf[256]; @@ -151,6 +150,7 @@ static int run_unsandboxed(const char *command, int timeout_sec, timeout_ms = (long)timeout_sec * 1000L; while (total < max_len - 1) { struct pollfd pfd; + long elapsed_ms; int rem; int r; ssize_t n;