fix(sandbox,shell): hide host secrets and enforce the command timeout

[adriannoes]

Summary

Consolidates two cursor high-severity sandbox/shell fixes as operator-authored commits:

• fix(sandbox): hide host secrets from sandboxed shell commands (supersedes fix(sandbox): hide host secrets from sandboxed shell commands #109) — sandboxed commands previously inherited host secrets from the environment; the sandbox now builds a filtered environment (new src/sandbox/sandbox_env.c/.h) so credentials do not leak into sandboxed processes.
• fix(shell): stop trickle output from bypassing the command timeout (supersedes fix(shell): stop trickle output from bypassing the command timeout #116) — a command slowly trickling output kept resetting the timeout, so the agent lock stayed held indefinitely; the deadline is now a true wall clock bound in both the output cap and the sandbox pipe loop.

Consolidation note: two cppcheck variableScope findings introduced by #116 were resolved in-place (loop-local wall timers moved into their use site).

Testing

• make test_sandbox — passed (namespace tests skipped on macOS, as designed)
• make test_shell — 30 tests, 0 failed
• CI=true make test — passed
• make static — passed

Supersedes #109 and #116.

[cursor]

Context. ShellClaw is a C99 edge agent (Jetson / Raspberry Pi) built with Make, one test binary per module. The shell tool runs commands either through sandbox_exec (namespaces, Landlock, cgroups) or a plain fork fallback. This PR is in src/sandbox and src/tools/shell.c.

Summary. The PR does two things. The sandboxed command no longer inherits the agent's environment, so a prompt-injected printenv cannot read API keys or bot tokens. Both shell paths (sandboxed and unsandboxed) now enforce the timeout against a monotonic wall clock instead of an idle-poll counter. Before, a command that kept writing never advanced the counter, so the agent lock stayed held until the output buffer filled. I built and ran test_sandbox and test_shell here and both pass. The two new regression tests ran and were not skipped. I found no blockers.

Must Fix

• None.

Should Fix

1. CHANGELOG.md has no [Unreleased] Fixed entry for either change. The env scrub is user-visible (LD_*, proxy and CUDA_* variables no longer reach sh -c), so it needs one.
2. The unsandboxed path still leaks every secret. run_unsandboxed in src/tools/shell.c still forks with the full agent environment. sandbox.enabled defaults to off when the TOML key is omitted, and docs/SECURITY.md line 43 only says "not a security boundary". Please state explicitly that printenv works there, or scrub there too (reusing sandbox_scrub_command_environment is cheap).
3. The scrubbed environment is a behavior change and is undocumented. sandbox_env.c keeps only PATH, LANG, LC_ALL, LC_CTYPE, TERM, TZ and HOME. On Jetson, anything that relies on LD_LIBRARY_PATH, CUDA_HOME, TMPDIR, SSL_CERT_FILE or proxy variables will break inside the sandbox. If that is intended, list it in docs/SECURITY.md and the CHANGELOG. An optional passthrough list in [sandbox] would help later.
4. The timeout logic is duplicated. monotonic_elapsed_ms is copied verbatim into sandbox.c:114 and shell.c:70. The have_clock fallback and the remain < 1 branch are also duplicated. The remain < 1 branch is unreachable because the loop already breaks at elapsed >= timeout. CLOCK_MONOTONIC failing is not a realistic case, so I would drop have_clock. One small shared deadline helper would cut this, and would bring drain_pipe (about 60 lines) and run_unsandboxed (about 85 lines) closer to the 40-line guideline in AGENTS.md.
5. The env test is weaker than it looks (tests/test_sandbox.c, test_command_does_not_inherit_host_secrets):
   • /proc/1/environ inside the new PID namespace is the command itself after clearenv, so that line checks the same thing as printenv.
   • The test passes NULL as the workspace, so the HOME=workspace branch is never exercised.
   • It does not check that LD_PRELOAD is dropped or that PATH is kept. The PATH_OK marker only proves the shell ran.
   • Add a case with a workspace that asserts HOME, and one that asserts LD_PRELOAD is absent.
6. The trickle tests only assert the elapsed time and the "timed out" text. Neither checks that the command was actually killed. Add a /proc scan for the leftover sleep, as test_shell_caps_output_kills_background_sleep does. That is the property the group kill and PDEATHSIG exist to provide.

Nice to Have

• sandbox_scrub_command_environment runs strdup, clearenv and setenv in a forked child of a multithreaded process (the gateway has an lws thread). glibc's env lock is not reset in the child, so this could deadlock if another thread is inside setenv/putenv at fork time. A safer shape is to build an envp[] in the parent before fork() and call execle. That would also remove the #if !defined(__linux__) environ branch and the strdup/free helper.
• drain_pipe was reformatted in places unrelated to the fix (for example if (errno == EINTR) continue; split across lines). It makes the diff noisier than it needs to be.
• Pre-existing, but now more visible: shell.c:265 computes timeout_sec * 1000 as an int, and the new (long)timeout_sec * 1000L overflows on 32-bit long for large configured values. Clamp shell_timeout_sec at config load.
• The sandbox timeout clock starts only after the isolation handshake, so a slow setup is not counted. That is fine, but worth one sentence in the header comment.

Positive Highlights

• The root cause is fixed at the right place. Switching from "sum of idle poll slices" to a monotonic deadline covers both the sandbox and unsandboxed loops, and the new comment in drain_pipe explains why.
• The scrub runs in the command process after cgroup join, namespaces, Landlock and close_inherited_fds, and just before execl, so setup code still has what it needs and nothing leaks into the command. LD_* is dropped as a side benefit.
• The two regression tests use a SIGALRM watchdog, so a regression fails fast instead of hanging CI. Each test cleans up before asserting. The Makefile wiring for the new object (SANDBOX_O, clean) is complete.

  

_{Sent by Cursor Automation: Code review}

[adriannoes]

Review response — done in 026eca8 (deadline helpers now live once in sandbox.h and shell.c drops its copy; have_clock fallback and the unreachable remain<1 clamp removed), 4d4d356 (env test asserts LD_PRELOAD is dropped and PATH kept; the Linux test now passes a workspace so HOME is covered), 9fb01d0 (trickle timeout tests assert the sleep is actually killed), c37b031 (SECURITY.md env model and changelog), d708a8c (loop-scoping fix cppcheck required after the have_clock drop).

Deferred: building envp[] before fork with execle (real but rare fork hazard — architectural follow-up) and clamping shell_timeout_sec at config load.