Skip to content

feat(sandbox): implement real strict mode for macOS and Linux#155

Open
zjshen14 wants to merge 1 commit into
mainfrom
feat/issue-149
Open

feat(sandbox): implement real strict mode for macOS and Linux#155
zjshen14 wants to merge 1 commit into
mainfrom
feat/issue-149

Conversation

@zjshen14
Copy link
Copy Markdown
Owner

Summary

  • Linux (bwrap): Replaces the blanket --ro-bind / / with enumerated read-only binds of minimum system paths (/usr, /bin, /sbin, /lib, /lib64, /etc); $HOME is not mounted at all; --unshare-net blocks external network. Paths are bound conditionally on existence to handle distros with merged-usr or no /lib64.
  • macOS (sandbox-exec): New buildStrictProfile restricts file-read* to cwd + system binary paths; file-write* to cwd + /private/tmp; network to localhost only (no external outbound).
  • Both BwrapRunner and SandboxExecRunner constructors no longer downgrade strict→auto or emit the "not yet implemented" stderr warning.
  • Strict-mode test blocks added to bwrap.test.ts and sandbox-exec.test.ts covering: network deny, HOME write deny, ~/.ssh read deny, CWD write allow, localhost allow.
  • README.md, docs/architecture.md, and the A7 design doc status line updated.

Closes #149

Test plan

  • npm run typecheck && npm run lint && npm run format:check && npm test — all pass (553 tests, 28 skipped; the 13 new bwrap strict tests all pass; sandbox-exec strict tests correctly skip on Linux)
  • On a Linux host with bwrap: verify OPENCLI_SANDBOX=strict opencli run "curl https://example.com" fails; verify opencli run "touch ./test.txt" succeeds
  • On macOS: verify OPENCLI_SANDBOX=strict opencli run "curl https://example.com" fails; verify cat ~/.ssh/id_rsa fails; verify writes to CWD succeed

Generated by Claude Code

strict mode was stubbed and fell back to auto with a stderr warning.
This implements it properly on both platforms:

- Linux (bwrap): `buildStrictArgs` drops the blanket `--ro-bind / /`
  and enumerates minimum system paths (/usr, /bin, /sbin, /lib, /lib64,
  /etc) as read-only; $HOME is not mounted at all; --unshare-net blocks
  external network. Paths are only bound if they exist on the host to
  avoid bwrap failure on distros with merged-usr or no /lib64.

- macOS (sandbox-exec): `buildStrictProfile` restricts file-read* to
  cwd + system binary paths; file-write* to cwd + /private/tmp;
  network to localhost only (no external outbound).

Both constructors no longer downgrade strict→auto or emit the
"not yet implemented" stderr warning.

Strict-mode test blocks added to bwrap.test.ts and
sandbox-exec.test.ts covering: network deny, HOME write deny,
~/.ssh read deny, CWD write allow, localhost allow.

Docs updated: README.md, docs/architecture.md, and the A7 design doc
status line.

Closes #149
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zjshen14
Copy link
Copy Markdown
Owner Author

Nice work on the strict-mode implementation. The approach is well thought-out:

  • buildStrictArgs gating each system path on existsSync(p) elegantly handles distro variance (Alpine's missing /lib64, Debian's merged-usr /sbin, etc.).
  • Removing the strict → auto downgrade and the "not yet implemented" warning is clean.
  • Tests on both platforms cover the key invariants (network blocked, HOME writes blocked, CWD writes allowed, localhost allowed).

A couple of things worth tightening before merge:

sandbox-exec.test.ts"blocks reads from ~/.ssh" test assertion is weak

expect(result.stderr + result.stdout).toMatch(/permitted|denied/i);

This doesn't assert exitCode !== 0. If mkdir -p ~/.ssh succeeds silently (dir already exists) and the touch exits non-zero without printing to stderr, the test passes vacuously. A stronger assertion would combine expect(result.exitCode).not.toBe(0) with the message check (consistent with how the bwrap tests are written).

sandbox-exec.tsbuildStrictProfile allows (allow file-read* (subpath "/private/var/db"))
This grants reads to system database files (TCC privacy database, CoreData stores, etc.). This is broader than what shell operation requires. If this is needed for a specific tool (e.g. system_profiler, security), a comment explaining why would help. If it's not strictly necessary, narrowing or removing it would tighten the security posture.

Cannot merge directly (touches src/tools/exec/sandbox/). Recommend maintainer review and merge.


Generated by Claude Code

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.

feat(sandbox): implement real strict mode (Phase 2 of #127)

2 participants