fix(sandbox): post-merge audit — exit codes, reserved dsts, root-uid, egress tests#103
Merged
dangtony98 merged 2 commits intomainfrom Apr 23, 2026
Merged
fix(sandbox): post-merge audit — exit codes, reserved dsts, root-uid, egress tests#103dangtony98 merged 2 commits intomainfrom
dangtony98 merged 2 commits intomainfrom
Conversation
… egress tests
- Propagate the container's real exit status through an ExitCodeError
sentinel. Previously every non-zero container exit collapsed to the
parent returning 1, breaking CI use cases like `vault run -- pytest`.
Defers still run because the error returns up through Cobra before
Execute() unwraps and os.Exit(Code)s.
- Reject `uid == 0` on Linux with --share-agent-dir. The usermod/groupmod
remap would hand the in-container claude user uid 0 alongside the
NET_ADMIN/NET_RAW/SETUID/SETGID/KILL caps, and no-new-privileges
doesn't disarm ambient caps on root.
- Expand reservedContainerDsts: /, /etc (subtree), ContainerClaudeConfig
(added by --share-agent-dir but never reserved), and both
/usr/local/sbin/{init-firewall,entrypoint}.sh. Without
ContainerClaudeConfig on the list, a user --mount could override the
bind-mounted ~/.claude.json.
- Add TestIntegration_EgressBlocked_Bypasses covering the channels a
compromised agent would actually try: IPv6 literal, UDP, ICMP,
curl --noproxy '*', and an env-stripped HTTPS_PROXY bypass. Shared
runInFirewalledContainer helper collapses the old end-to-end
test's duplicated argv too.
- Bake iputils-ping into the image so the ICMP probe doesn't apt-get
install it per test run.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- validateContainerDst was asymmetric: it caught `dst == reserved` and `dst inside reserved`, but not `reserved inside dst`. A user could `--mount /tmp/evil:/usr/local/sbin` to silently shadow both entrypoint.sh and init-firewall.sh, so ENTRYPOINT would execute attacker code as PID 1 / UID 0 with NET_ADMIN/NET_RAW/SETUID/SETGID/ KILL before any iptables rule got installed. Same gap covered the new ContainerClaudeConfig reservation (bypassable via /home/claude). Add the symmetric prefix check; extend the test table with the parent paths (/usr/local/sbin, /usr/local, /usr, /home/claude, /home). - cobra's SilenceErrors and SilenceUsage are independent gates — setting only the former still dumps the full usage block on every non-zero container exit, turning `vault run -- pytest` into a wall of help text on test failure. Set both on the exit-code path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Post-merge audit of the container-mode sandbox (PR #99 + follow-up
--share-agent-dirwork). Four must-fix items from the audit:ExitCodeErrorsentinel unwrapped inExecute(). Previously every non-zero exit collapsed to1, which broke CI use cases likevault run -- pytest. Defers still run — the error returns normally through Cobra beforeExecute()unwraps andos.Exit(Code)s.--share-agent-dir: rejectuid == 0on Linux. Theusermod/groupmodremap would hand the in-containerclaudeuser uid 0 alongsideNET_ADMIN/NET_RAW/SETUID/SETGID/KILLcaps, andno-new-privilegesdoesn't disarm ambient caps on root.reservedContainerDsts:/,/etc(subtree),ContainerClaudeConfig(added by--share-agent-dirbut never reserved), and both/usr/local/sbin/{init-firewall,entrypoint}.sh. WithoutContainerClaudeConfigon the list, a user--mountcould override the bind-mounted~/.claude.json.TestIntegration_EgressBlocked_Bypassescovers the channels a compromised agent would actually try: IPv6 literal, UDP, ICMP,curl --noproxy '*', and an env-strippedHTTPS_PROXYbypass. SharedrunInFirewalledContainerhelper also collapses the existing end-to-end test's duplicated docker argv.iputils-pingbaked into the image so the ICMP probe doesn'tapt-get installper test run (asset hash bumped).Test plan
make testgreen (all unit tests)go vet -tags docker_integration ./...cleango test -tags docker_integration ./internal/sandbox/ -run Integration -von Linux + macOS (reviewer, please run — requires Docker)./agent-vault run --sandbox=container -- sh -c 'exit 42'returns 42 (not 1)./agent-vault run --sandbox=container --mount /tmp/x:/home/claude/.claude.json -- truerejectssudo ./agent-vault run --sandbox=container --share-agent-dir -- truerejects with the new error🤖 Generated with Claude Code