Skip to content

fix(isolation): make the shipped default posture actually work and actually enforce - #153

Merged
las7 merged 1 commit into
mainfrom
harden/sandbox-defaults
Aug 11, 2026
Merged

fix(isolation): make the shipped default posture actually work and actually enforce#153
las7 merged 1 commit into
mainfrom
harden/sandbox-defaults

Conversation

@las7

@las7 las7 commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

The shipped default configuration (enable_seccomp + enable_cap_restrictions, both true) could not start a container, and both controls were switched off in CI to work around it. That made the default posture — the one a new user gets, the one SECURITY.md describes — the single configuration nothing exercised. Four defects lived in that blind spot.

Found while pre-auditing against the SandboxGym benchmark methodology. The headline risk was never a clever escape: it was that the advertised boundary and the enforced boundary had drifted apart.

The four defects

1. The seccomp profile blocked container init. The default-deny allowlist permitted prctl only for PR_SET_NAME(15) and PR_SET_PDEATHSIG(1). runc also needs PR_CAPBSET_DROP(24), PR_SET_KEEPCAPS(8) and PR_CAP_AMBIENT(47):

error during container init: unable to apply bounding set: operation not permitted

Multiple args entries in one seccomp rule are AND-ed, not OR-ed, so each option needs its own rule.

2. The entrypoint aborted before user code ran. chown -R sandbox:sandbox /tmp/.cache needs CAP_CHOWN, which --cap-drop=ALL strips; under set -e the EPERM killed the container (exit_code: 1, empty stdout). Dirs are now created as the sandbox user — no capability required. Same fix for the dependency-install target, whose chown was likewise unguarded.

3. The in-container timeout never fired. entrypoint.sh runs timeout ... gosu sandbox python, so a uid-0 supervisor signals a uid-1000 child — that needs CAP_KILL. Measured on a 2s budget:

capabilities result elapsed
--cap-drop=ALL (shipped default) SIGTERM refused, exit=137 5.0s
unrestricted exit=124 2.0s
--cap-drop=ALL --cap-add=KILL exit=124 2.0s

The SIGTERM was silently dropped; only the --kill-after SIGKILL ended the job, 10s late. Since the host backstop is timeout + 5s, the in-container timeout never won — every timeout fell through to the coarser host kill, losing the phase attribution we advertise. gosu clears all capabilities on the uid switch, so CAP_KILL never reaches user code, and its scope is the container's own PID namespace.

4. The library path enforced a weaker posture than the server. Diffing the assembled argv from both paths under one identical config:

ONLY in CodeExecutor:  --security-opt=seccomp=.../seccomp_profile.json
                       --tmpfs=/tmp:rw,noexec,nosuid,size=100m
ONLY in Sandbox:       --tmpfs=/tmp:rw,exec,nosuid,size=100m

Sandbox never passed the seccomp profile at allenable_seccomp was inert there and Docker's permissive default applied. Verified in-container:

before:  ptrace -> ALLOWED     tmp exec -> EXECUTED
after:   ptrace -> BLOCKED     tmp exec -> BLOCKED (PermissionError)

SECURITY.md claimed writable space was limited to /output/ and a noexec /tmp/, and described seccomp as always-on. Both were false for every library-mode run.

Why CI could not see any of this

test_isolation_invariants.py asserted only on base_isolation_args — the shared base. Every per-path flag appended on top was unguarded, so the drift was structurally invisible.

The CI overrides blamed the runner ("GitHub Actions Docker can't modify capability bounding sets", "Custom seccomp profiles block container init in GitHub Actions"). Neither is runner-specific — both reproduce on any native-Linux Docker, and the cap failure was a downstream symptom of the seccomp denial.

Tests

  • Full-argv parity between both execution paths (the structural fix).
  • The profile allows every prctl option init needs, and no multi-value prctl rule (which can never match).
  • No unguarded chown under set -e.
  • A live container test asserting the shipped defaults run code with ptrace and /tmp-exec blocked.

All five new tests fail on the parent commit, one per defect. CI stops overriding the two controls and runs the real defaults.

Full suite: 770 passed, lint and format clean.

Caveats

  • Verified under runc only — no gVisor on the audit box, so the runsc path is unverified.
  • This PR flips CI to the real default posture; that flip is what this CI run is proving.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HCi216irm5J9XxAW14WGPh

…tually enforce

The default configuration (enable_seccomp + enable_cap_restrictions, both
true) could not start a container, and the two controls were switched off in
CI to work around it. That made the shipped default the one configuration
nothing exercised, and it hid three further defects.

1. seccomp profile blocked container init. The default-deny profile allowed
   prctl only for PR_SET_NAME and PR_SET_PDEATHSIG, but the OCI runtime needs
   PR_CAPBSET_DROP, PR_SET_KEEPCAPS and PR_CAP_AMBIENT during init. Every
   `docker run` failed with "unable to apply bounding set". Not a CI quirk:
   it reproduces on any native-Linux Docker. Each option needs its own rule,
   because args within one rule are AND-ed, not OR-ed.

2. entrypoint aborted before user code. `chown -R sandbox:sandbox` on the
   cache dirs needs CAP_CHOWN, which --cap-drop=ALL strips; under `set -e`
   the EPERM killed the container. The dirs are now created as the sandbox
   user, which needs no capability. Same fix for the dependency-install
   target, whose chown was likewise unguarded.

3. in-container timeout never fired. entrypoint.sh runs
   `timeout ... gosu sandbox python`, so a uid-0 supervisor signals a uid-1000
   child, which needs CAP_KILL. Without it the SIGTERM was refused and
   silently dropped, so the limit was only enforced by the --kill-after
   SIGKILL 10s later, past the host-side backstop. Re-add CAP_KILL: gosu
   clears all capabilities on the uid switch, so it never reaches user code,
   and its scope is the container's own PID namespace.

4. library path enforced a weaker posture than the server. Sandbox never
   passed --security-opt=seccomp at all (so enable_seccomp did nothing there
   and Docker's permissive default profile applied, allowing ptrace) and
   always mounted /tmp exec. Verified in-container: ptrace ALLOWED -> BLOCKED,
   /tmp exec EXECUTED -> BLOCKED.

The invariant tests only covered base_isolation_args, the shared base, so
per-path flags were unguarded and this drift was invisible. Tests now assert
the FULL assembled argv from both paths is identical, that the profile allows
the prctl options init needs, and that the shipped defaults really do run code
with ptrace and /tmp-exec blocked. All five new tests fail on the parent
commit. CI stops overriding the two controls and runs the real defaults.

SECURITY.md updated to match what is now enforced.
@las7
las7 requested a review from ethanbailie as a code owner August 11, 2026 07:36
@las7
las7 merged commit 842c6e8 into main Aug 11, 2026
4 checks passed
las7 added a commit that referenced this pull request Aug 11, 2026
Follow-up to #153. Those were controls that did not work; these are defaults
and boundaries that worked exactly as written, but where what was written was
the permissive choice. A scanner reads the default, not the warning next to it.

1. security_mode now defaults to 'strict'. It was 'permissive', so on any host
   without gVisor every job silently ran on runc while the docs called gVisor
   "the sole isolation boundary". Strict refuses to run instead. Set
   'permissive' explicitly for local dev and CI on hosts without gVisor.

2. server_host now defaults to 127.0.0.1, and binding a non-loopback interface
   while api_auth_enabled is false is REFUSED at startup rather than warned
   about. That combination is an unauthenticated remote code-execution
   endpoint; a log line is the wrong control for it, because the operator who
   most needs it is the one not reading logs.
   allow_unauthenticated_network_access=true is the explicit opt-out for
   deployments that authenticate in front of Tako VM.

3. The shared uv dependency cache is now scoped per job type instead of one
   host-wide volume. It is mounted read-write at a path the sandbox user owns
   and stays mounted for the container's whole life, not just the install
   phase, so one job can write a cache entry a later job's `uv pip install`
   resolves and executes. This narrows the blast radius to a group the
   operator defines; it does NOT eliminate the channel, since jobs sharing a
   job type still share a cache. Tako VM has no tenant identity to key on.
   Documented as such in uv_cache_volume().

4. build_session_run_command now validates workspace_dir before it becomes a
   read-write host bind mount: absolute, normalized, no '..', not a sensitive
   system directory, and symlink-resolved so a symlinked workspace cannot
   redirect the mount. Nothing calls this yet (Phase 1a scaffolding) -- the
   guard is added while the path is still unreachable, rather than after.

Tests cover each: the secure defaults, that the unauthenticated network bind
is refused and the loopback/authenticated/opted-out cases still work, that
cache scopes are distinct and shell-safe, and a table of unsafe workspace
paths including a symlink redirect. Existing tests that asserted the old
defaults are updated to assert the new intent rather than the new strings, and
the default-posture test relaxes only security_mode when the host has no
gVisor, since seccomp and capabilities are runtime-independent.

Docs, SECURITY.md and tako_vm.yaml.example updated to match.

Verified on a host with gVisor installed: 802 passed under strict + runsc, and
798 passed with runsc removed to simulate a CI runner (remaining failures were
this box's known load-related container stalls, which reproduce on origin/main
and did not recur after cleanup).
@las7 las7 mentioned this pull request Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant