Second-round review follow-up. Security notes was written as the
canonical list of residual risks but never mentioned the new
SANDBOX_SSH_KEY mechanism, which has the same "signs for any repo it can
reach" shape as the forwarded agent (already documented) - add a bullet
covering it and the /root-only mount.
Also nothing enforced that SANDBOX_SSH_KEY actually be a dedicated
low-privilege key rather than a real personal identity, which is the
whole basis for accepting it as passphrase-less and left loaded for an
entire unattended session. bin/claude-sandbox now warns (non-fatal) if
it looks like a default personal keyfile (id_rsa, id_ed25519, etc).
Git commits inside the sandbox need a user.name/user.email. Carries over
a throwaway copy of the host's ~/.gitconfig, generated by bin/claude-sandbox
and mounted as its own directory (not the real file, and not a single-file
bind mount at ~/.gitconfig) with GIT_CONFIG_GLOBAL pointing git at the file
inside it.
That took two iterations to get right, both caught by actually testing gh
auth setup-git (the thing that makes GitHub read-only from in here) rather
than just checking the identity resolved:
- Mounting the real file read-only broke gh auth setup-git outright, which
needs to *write* a credential-helper entry into the global gitconfig
("could not write config file: Device or resource busy").
- Mounting a read-write *copy* of the file directly at ~/.gitconfig hit the
exact same error - a single-file bind mount can't have its backing inode
replaced, and git config writes via the usual write-temp-then-rename
pattern. Only a directory mount, with git pointed at a file inside it,
actually supports that.
Verified end to end this time: gh auth setup-git completes cleanly, the
credential-helper entries land in the sandbox's copy, user.name/email
resolve correctly, and the real host ~/.gitconfig is untouched throughout.
Also has bin/claude-sandbox warn (non-fatally) at launch if ssh-agent has
zero identities loaded - the forwarded socket still connects fine either
way, but git-over-ssh silently fails until a key's been ssh-add'ed, which
otherwise only surfaces confusingly deep into a session as a failed push.
Confirmed the forwarding mechanism itself works correctly by testing with
a throwaway key - the reported failure was the host agent having no
identities loaded, not a bug in the forwarding.
Test suite grew to 26 with a check for the gitconfig mount shape.
Claude Code's own per-command sandboxing (~/.claude/settings.json's
"sandbox" block) is redundant on top of this container - which is already
a real, working sandbox on its own - and it couldn't even function here
anyway (bwrap can't create a namespace on this host; even CAP_SYS_ADMIN
only got it to a mount-propagation error, not further). Rather than
leaving it half-broken, disable it properly: settings.json is mounted
read-only specifically so a session can't tamper with it, so it can't be
patched from inside - bin/claude-sandbox now generates a sandbox-specific
copy (real settings.json with sandbox.enabled forced to false) on the
host and mounts that instead. bubblewrap/socat (added for this last
commit, never actually working) are removed - nothing needs them now.
Also fixes a real bug found in the same session: sandbox-context.sh told
agents to worktree via `git worktree add ../<branch>`, a sibling of the
mounted project directory. That path isn't backed by real host storage -
only the exact bind-mounted directory is - so a worktree created there
(observed landing under /home/node/<branch> in practice) silently
disappears, along with whatever work went into it, when the container
exits. Now instructs `.worktrees/<branch>` under the project root instead,
plus a .git/info/exclude entry so it doesn't clutter the user's real
checkout's git status.
Test suite extended to 25: sandbox.enabled=false generation, no
bubblewrap/socat, and no worktree instruction pointing outside the mounted
project. All pass.
~/.claude/CLAUDE.md (already carried into the container) tells agents to
write specs to ~/specs/ instead of a project-local path - that instruction
was already in effect inside the sandbox, but nothing mounted the
directory it points at, so an agent following it would write into a path
that vanishes with the container. Bind-mounted read-write at the fixed
host path ~/specs (auto-created if missing), same pattern as the shared
scratch space but not configurable - it's tied to the CLAUDE.md
convention, not a per-run choice.
Also adds bubblewrap/socat, which Claude Code checks for before offering
its own internal per-command sandboxing and /remote-control. This clears
the "Sandbox disabled: dependencies are missing" warning, but doesn't
actually fix /remote-control: bwrap still can't create a namespace on this
host ("kernel does not allow non-privileged user namespaces"), and even
adding CAP_SYS_ADMIN to the container only got further to a mount-
propagation permission error, not further progress. Making it actually
work would need privilege escalation approaching full --privileged mode,
which would undo most of this session's hardening - not pursued. Kept the
packages anyway since they at least make the startup warning accurate
rather than noise, in case a future decision revisits this tradeoff.
All 22 existing tests still pass.
cowsay/cowthink/fortune install to /usr/games on Debian, not on this image's
PATH by default - added the packages and extended PATH via ENV so nvim's
start-screen header (plugins/ui.lua) can actually find them as non-login
subprocesses. That surfaced a real, pre-existing bug in ui.lua itself
(unrelated to the sandbox): `cowsay -l` wraps cow names across multiple
space-separated lines after a header line, but the code treated each whole
line as a single cow name, so a wrapped line got passed to cowthink -f as
one bogus multi-word argument. Fixed to parse every name from every line.
Verified directly against the real cowsay/fortune installed in the image.
The sandbox now opens claudecode.nvim's terminal on startup (CLAUDE_SANDBOX-
gated, so this doesn't affect normal host nvim use) instead of requiring
<leader>ac - starting a Claude session is the point of being in there.
:ClaudeCode is only defined once the plugin loads (it's lazy-loaded on that
keymap, no `cmd` in its spec), so the autocmd force-loads it first.
Also adds an optional read-write scratch volume (~/shared in the container,
$SANDBOX_SHARE_DIR on the host, auto-created) separate from the project
mount, for passing files back and forth with a sandboxed session without
touching the actual project checkout. Mentioned in sandbox-context.sh so a
sandboxed agent knows it's there.
DNS resolution was previously unrestricted - only the resulting IP
connections were firewalled - so a compromised session could still
exfiltrate data by encoding it into query names to an attacker-controlled
domain. dns-proxy.sh now runs a local dnsmasq with no fallback resolver;
only allowlisted names (shared with init-firewall.sh via fixed-domains.sh)
get forwarded, everything else is refused outright.
IPv6 is blocked outright via ip6tables DROP-all, since there was no IPv6
equivalent of the IPv4 allowlist.
If LOKI_URL is set, every DNS query and every new outbound connection
attempt (allowed or rejected, via NFLOG + ulogd2) ships to Loki as JSON,
for visibility in the existing Grafana/Prometheus setup.
Also fixes two bugs found along the way:
- init-firewall.sh's script-wide IFS (set for stricter splitting elsewhere)
silently broke EXTRA_ALLOWED_DOMAINS whenever it held more than one
space-separated entry, collapsing it into a single bogus token.
- bin/claude-sandbox-allow interpolated the domain argument directly into a
bash -c string executed via docker exec - a command injection path,
since sandbox-context.sh tells a sandboxed agent to hand this exact
command to the user to copy-paste. Fixed with upfront domain validation
and positional-argument passing instead of string interpolation.
Nvim isn't a single process attached to the terminal: the process that
actually spawns jobs (clipboard-copy included) is a separate `nvim --embed`
core process with no tty of its own (ps shows `?`); the real pty belongs to
a distinct parent UI process. Checking only the immediate parent's tty (as
the previous fix did) found the embed process's `?` and fell through to the
broken /dev/tty path every time in real interactive use, even though it
looked fine in headless testing. Walk up the process tree past any tty-less
ancestors instead.
Verified against the real docker-compose path in actual (non-headless) TUI
mode, with real keystrokes fed through a pty: the OSC 52 sequence now
reaches the terminal with no error, where it previously still failed.
- clipboard-copy: nvim's job-spawned clipboard provider process doesn't get
its own controlling terminal, so /dev/tty (ENXIO) fails. Resolve the real
tty from the parent (nvim)'s recorded tty via ps instead.
- entrypoint.sh: chown the allocated pty to node before dropping privileges.
A normal login/ssh session always does this; this entrypoint skipped it,
leaving the tty root-owned so node couldn't write to it directly even
after resolving the right path.
- init-firewall.sh/README: allow release-assets.githubusercontent.com.
GitHub release assets (e.g. mason-registry's own index) redirect here,
a separate CDN from github.com's published IP ranges - without it mason
can't even look up a package's install spec, regardless of PyPI access.
All three verified against a rebuilt image: OSC 52 write reaches the real
pty with no error, and mason's registry refresh succeeds through the
firewall.
The lookup was matching on head.ref alone, so running create with a
different --base for the same branch would silently return an
unrelated PR's URL instead of opening a new one. Resolve base before
the check and filter on both head.ref and base.ref.
gitea-pr create now checks for an existing open PR on the current
branch first and returns its URL instead of creating a new one. Only
matches open PRs by design -- once merged, new commits on the same
branch correctly get a new PR.
branderson
Brad Anderson