fix(sandbox): protect daemon token file#685
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe sandbox now denies reads to the file specified by ChangesDaemon token sandbox protection
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR closes a sandbox escape where ZERO_DAEMON_REMOTE_TOKEN_FILE could be inherited by sandboxed commands (allowing them to locate and read the daemon bearer token file under the read-all posture). It scrubs the pointer env var across platforms and extends the existing “credential deny-read” profile logic to also deny reads of the referenced token file where deny-read enforcement is supported.
Changes:
- Scrub
ZERO_DAEMON_REMOTE_TOKEN_FILEfrom sandbox command environments (in addition to the inline token env var). - Extend
credentialDenyReadPathsto include the path named byZERO_DAEMON_REMOTE_TOKEN_FILE(alongsideGOOGLE_APPLICATION_CREDENTIALS) and plumb this through the pure helper. - Add/extend regression tests covering env scrubbing and permission-profile deny-read construction (skipping the deny-read assertion on Windows per existing platform limitations).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/sandbox/runner.go | Adds ZERO_DAEMON_REMOTE_TOKEN_FILE to the sandbox env scrub list. |
| internal/sandbox/runner_test.go | Extends env scrubbing regression test to ensure the pointer env var is removed. |
| internal/sandbox/profile.go | Adds ZERO_DAEMON_REMOTE_TOKEN_FILE to default credential deny-read path construction and updates helper signature/docs. |
| internal/sandbox/manager_test.go | Updates credential deny-read tests for the new parameter and adds a profile-level regression test for daemon token file denial (non-Windows). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
- [P1] Deny writes to the daemon token file on macOS as well
internal/sandbox/profile.go:176
The new target entersDenyRead, but the Seatbelt backend translates that only intofile-read*and unlink denials. Its broadfile-write*allowance still covers every workspace root and the default temporary roots. Therefore, whenZERO_DAEMON_REMOTE_TOKEN_FILEnames a file under/tmpor another writable root, a sandboxed command can discover the filename from its parent directory and overwrite or truncate the bearer-token file. This makes the remote bridge unavailable and can replace its credential on a restart/reload. Add a write denial for credentialDenyReadfiles in the Seatbelt profile (and a macOS regression case for a token under a writable temporary root).
Address code review on PR Gitlawb#685: the Seatbelt profile only translated DenyRead entries into file-read* and file-write-unlink denials. The broad file-write* allowance for workspace/temp write roots still covered a DenyRead file (e.g. the file ZERO_DAEMON_REMOTE_TOKEN_FILE names) if it happened to sit under one of them, so a sandboxed command could discover and overwrite/truncate the daemon bearer-token file even though it couldn't read or delete it. A file a sandboxed command must not read has no legitimate reason to be written either, so seatbeltProfileFromPermissionProfile now also emits a full file-write* deny for every DenyRead path, placed after the broad write allow (deny rules that follow an allow win, matching the existing DenyWrite/metadata-carveout ordering). Adds a regression test with a DenyRead file under a writable /tmp root, and extends the existing deny-ordering test to assert the new file-write* rule. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
anandh8x
left a comment
anandh8x
left a comment
There was a problem hiding this comment.
Re-reviewing against commit 2248aca8 (head). The macOS Seatbelt fix (patch 2/2) is the right primitive: a DenyRead file that's also under a writable root was overwritable/truncatable because the prior profile only emitted file-read* and file-write-unlink, not file-write*. Denying the full write direction for every DenyRead path is correct, the ordering (deny after the broad allow) is correct, and TestSeatbeltProfileDeniesWritesToDenyReadUnderWritableRoot covers both the rule presence and the ordering. The TestSeatbeltProfileProtectsMetadataAndDenyOrdering extension covers the general case.
LGTM.
Cross-PR note: #685 depends on the credentialDenyReadPathsIn signature change from #681 (daemon token file as a parameter) and the scrubSensitiveEnv plumbed sensitiveEnvKeys from #682. Recommend rebasing #685 onto #681 + #682 in that order.
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
Local review: built and ran go test ./internal/sandbox on darwin/arm64; all pass. The deny-write-for-DenyRead fix is a genuine security improvement (closes the truncate/overwrite bypass under a writable root). One integration note.
| // credentialDenyReadPathsIn is the pure core of credentialDenyReadPaths, | ||
| // separated so tests can exercise it against a synthetic home directory. | ||
| func credentialDenyReadPathsIn(home string, googleCredentials string, allowRead []string) []string { | ||
| func credentialDenyReadPathsIn(home string, googleCredentials string, daemonTokenFile string, allowRead []string) []string { |
There was a problem hiding this comment.
[P2] Sibling-PR merge conflict on credentialDenyReadPathsIn signature
This PR keeps the flat (home, googleCredentials, daemonTokenFile, allowRead) signature, while #681 rewrites the same function to take a credentialPathOptions struct. Both PRs also edit manager_test.go and runner_test.go against the same test. Each merges into main cleanly on its own, but only one lands as-is; the other must rebase. Also note #682 changes scrubSensitiveEnv in runner.go (this PR adds ZERO_DAEMON_REMOTE_TOKEN_FILE to the same function's literal list) -> sequence the three sandbox credential PRs.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
- [P1] Protect the configured symlink pathname as well as its target
internal/sandbox/profile.go:200
normalizeProfilePathsresolvesZERO_DAEMON_REMOTE_TOKEN_FILEthrough symlinks before it is added toDenyRead. If the configured pathname is a symlink under a writable root such as/tmp, the new deny rules protect only its current referent; a sandboxed command can unlink the writable symlink and recreate a regular file at the configured pathname. On the next remote-daemon start,TokenFromEnvreads that replacement pathname and accepts the attacker-chosen bearer token (or fails, causing a denial of service). Preserve and deny the lexical configured path in addition to its resolved target, and add a symlink-replacement regression test.
There was a problem hiding this comment.
Approving clean security hardening. Scrubbing ZERO_DAEMON_REMOTE_TOKEN_FILE from child envs and adding its target to the credential deny-read set closes a real hole (a sandboxed command could otherwise resolve the pointer and read the daemon bearer-token file under the read-all posture), and extending the macOS seatbelt profile to file-write*-deny every DenyRead path is the right fix: denyReadRules only blocked read and unlink, leaving a credential file under a writable root overwritable/truncatable. I checked the Linux bubblewrap path and it already bind-mounts DenyRead targets read-only, so this just brings macOS to parity. One thing to be aware of: the write-deny now covers all DenyRead paths (~/.aws, ~/.azure, etc.), so no sandboxed command can update cloud creds consistent with the existing unlink-deny and fine under the current threat model, just calling it out.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sandbox/profile.go`:
- Around line 320-328: Keep normalizeProfilePath purely lexical by removing its
filepath.EvalSymlinks resolution and returning the result of
normalizeProfilePathLexical unchanged. Resolve symlinks only within
normalizeProfilePathVariants while retaining both the configured lexical path
and resolved target for deny-policy expansion, and add a regression test
covering a writable denied symlink.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 974aa02c-d6a1-45e8-ae0b-c2df72771e98
📒 Files selected for processing (4)
internal/sandbox/manager_test.gointernal/sandbox/profile.gointernal/sandbox/runner.gointernal/sandbox/runner_test.go
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Do not pass a lexical symlink to Bubblewrap's deny mount
internal/sandbox/profile.go:200
For an existingZERO_DAEMON_REMOTE_TOKEN_FILEsymlink, the new variant list includes the symlink pathname as well as its target. The Linux backend then emits--ro-bind /dev/null <symlink>for that pathname; Bubblewrap rejects a symlink mount destination before the command starts (Can't create file at .../daemon-token: No such file or directory). Thus configuring the supported token-file option through a symlink makes every Linux sandboxed command fail to launch. Materialize/protect that pathname with a Bubblewrap-safe mechanism (or avoid adding it to the Linux deny-mount list) and add a Linux regression test. -
[P1] Resolve the token-file path in the daemon's context, not each worker's
internal/sandbox/profile.go:195
TokenFromEnvaccepts relative token paths, andserve-remotereads one before it starts workers. The daemon then preservesZERO_DAEMON_REMOTE_TOKEN_FILEfor workers whosecmd.Diris the per-sessionspec.Cwd;normalizeProfilePathLexicalconsequently turnstokeninto a path beneath that session instead of the daemon startup directory that contains the actual bearer-token file. The real file is left outsideDenyReadunder the read-all posture, so a sandboxed command that can infer its location can read it. Normalize the value at the daemon boundary (or pass an already-absolute protected path) and cover a remote worker whose session CWD differs from the daemon CWD.
|
Following up on my earlier approve, which I am pulling back from for now. jatmn's latest P1 is a real one: the symlink-protection commit adds the ZERO_DAEMON_REMOTE_TOKEN_FILE symlink pathname itself, not just its resolved target, to the Linux deny-mount list, and Bubblewrap rejects a symlink as a mount destination, so every sandboxed command on Linux fails to launch when that option points at a symlink. I am on Windows and cannot reproduce the bwrap behavior here, but jatmn tested it on Linux with the exact "Can't create file ... daemon-token" error and the mechanism is sound. The target protection and the macOS write-deny are still the right hardening. This just needs the Linux side to protect that pathname without ro-binding the symlink itself (materialize it, or keep the symlink pathname off the Linux deny-mount list). Not re-approving until that is closed. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sandbox/linux_helper.go`:
- Around line 319-324: Add the same lexical-symlink guard used in the DenyRead
path to appendReadOnlyLinuxPathArgs, checking the mount path with os.Lstat and
returning the existing args unchanged when it is a symlink. Keep the current
handling for non-symlink paths unchanged.
In `@internal/sandbox/profile.go`:
- Line 325: The FileSystemPolicy initializers in PermissionProfileFromPolicy and
seatbeltCompatibilityPermissionProfile must preserve both lexical and resolved
paths for user deny policies. Replace single-path normalization for
policy.DenyRead and policy.DenyWrite with normalizeProfilePathVariants, while
leaving normalizeProfilePath unchanged for other uses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cbb0b9b3-3559-4c77-bb5a-2c1692650e7a
📒 Files selected for processing (7)
internal/cli/daemon.gointernal/cli/daemon_test.gointernal/sandbox/linux_helper.gointernal/sandbox/linux_helper_test.gointernal/sandbox/manager_test.gointernal/sandbox/profile.gointernal/sandbox/runner_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/sandbox/runner_test.go
- internal/sandbox/manager_test.go
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Preserve the resolved target for user-configured
DenyReadsymlinks
internal/sandbox/profile.go:104
normalizeProfilePathis now lexical-only, while this initializer still usesnormalizeProfilePathsfor policy entries. On Linux,appendUnreadableLinuxPathArgsthen skips that symlink mount destination and no resolved target is present (unlike the credential-path branch). Thus a policy such asdenyRead: [link], wherelinkpoints to a secret, produces no deny mount under the read-all profile and the sandboxed command can read the target. Keep both variants for deny paths (and update the macOS compatibility initializer) so the Bubblewrap-safe target is actually denied. -
[P1] Do not use lexical paths for ordinary sandbox roots
internal/sandbox/profile.go:324
This changed the shared normalizer used forworkspaceRoot,AllowWrite, andDenyWrite, not just the new credential deny variant. A workspace opened through a symlink now reaches Linux Bubblewrap as--bind <link> <link>; Bubblewrap rejects a symlink mount destination, so every sandboxed command fails before it starts. I reproduced the failure with a symlinked workspace. Restore resolved normalization for ordinary roots and keep lexical-plus-resolved handling scoped to deny-path expansion. -
[P1] Do not leave a writable token-file symlink unprotected on Linux
internal/sandbox/linux_helper.go:319
Skipping the lexical symlink avoids Bubblewrap's invalid mount destination, but only its original target is masked. IfZERO_DAEMON_REMOTE_TOKEN_FILEis a symlink under a writable root such as/tmp, a sandboxed command can replace it with a link to another host-readable file and read through the replacement; it can also corrupt the daemon's token path. The test currently asserts the unsafe omission. Protect or materialize the lexical pathname with a Bubblewrap-safe mechanism rather than simply dropping its deny rule. -
[P1] Handle symlinked parent directories before emitting a deny mount
internal/sandbox/linux_helper.go:319
TheLstatcheck catches only a final-component symlink. For a supported token path such as/tmp/linkdir/token, wherelinkdiris a symlink,Lstat(token)reports a regular file and the helper emits a deny mount through the symlinked parent. Bubblewrap rejects that destination and every Linux sandbox launch fails. Detect path traversal through a symlink (or omit the lexical variant after retaining the resolved target) and add a regression case for this layout.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sandbox/linux_helper.go`:
- Around line 323-349: The Linux path argument helpers currently abort on
lexical symlinks instead of skipping them when their resolved target is also
protected. Update the profile-processing flow around appendReadOnlyLinuxPathArgs
and appendUnreadableLinuxPathArgs to recognize lexical symlink entries whose
resolved targets exist in the same deny set, skip those entries, and continue
enforcing the target; retain the existing error behavior when no enforceable
target is present. Update the related test to assert successful sandbox startup
and target enforcement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: baa5d0ce-25e0-42a5-8752-15ae141e7d1d
📒 Files selected for processing (7)
internal/cli/daemon.gointernal/cli/daemon_test.gointernal/sandbox/linux_helper.gointernal/sandbox/linux_helper_test.gointernal/sandbox/manager_test.gointernal/sandbox/profile.gointernal/sandbox/runner.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/cli/daemon.go
- internal/sandbox/runner.go
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Keep the remote token excluded from in-process file tools
internal/sandbox/profile.go:104
The new daemon-token path is added only toPermissionProfile.FileSystem.DenyRead, which protects wrapped shell commands. Built-in tools do not consume that profile:read_filereads scoped files directly, and grep/glob exclusions are built fromPolicy.DenyRead. If the token file is inside a remote session workspace (for example, a daemon started with a relative token-file path from that workspace), a remote-controlled agent can useread_fileto exfiltrate the bridge bearer token. Apply the automatic credential exclusion to the in-process read/search tool boundary as well, and cover this with an end-to-end tool test. -
[P1] Preserve inline-token precedence when a token-file variable is stale
internal/cli/daemon.go:480
TokenFromEnvintentionally returns a nonemptyZERO_DAEMON_REMOTE_TOKENbefore consultingZERO_DAEMON_REMOTE_TOKEN_FILE, but this new preflight resolves the file first. Consequently, a valid inline token plus an inherited missing or dangling token-file variable now makesdaemon serve-remoteexit instead of starting. Only canonicalize the file when it is the selected source (or otherwise leave an ignored file pointer from changing the result), and add the both-variables regression case. -
[P1] Do not make symlink-backed credential paths disable every Linux sandbox command
internal/sandbox/linux_helper.go:344
The profile now deliberately retains both lexical and resolved forms of every credential/deny path, but the Linux argument builder aborts whenever either form has a symlink component. This makes common configurations such asGOOGLE_APPLICATION_CREDENTIALS=/var/run/...(where/var/runis commonly a symlink to/run) fail plan construction for every sandboxed command; the pre-PR profile kept only the resolved target. Preserve the denial of the resolved target while using a Bubblewrap-safe treatment for the lexical path instead of turning a valid credential configuration into a global sandbox-startup failure.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sandbox/engine.go`:
- Around line 57-75: Update withAutomaticDenyRead to recompute automaticDenyRead
from the current effective policy before merging it with policy.DenyRead, rather
than reusing the constructor-time list. Ensure credential paths allowed through
session or turn permission profiles are removed from the automatic deny set
while preserving deduplication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 059619b5-dbbc-4812-a361-6fad61cca69c
📒 Files selected for processing (6)
internal/cli/daemon.gointernal/cli/daemon_test.gointernal/sandbox/engine.gointernal/sandbox/linux_helper.gointernal/sandbox/linux_helper_test.gointernal/tools/read_exclusions_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/cli/daemon.go
- internal/sandbox/linux_helper.go
Address code review on PR Gitlawb#685: the Seatbelt profile only translated DenyRead entries into file-read* and file-write-unlink denials. The broad file-write* allowance for workspace/temp write roots still covered a DenyRead file (e.g. the file ZERO_DAEMON_REMOTE_TOKEN_FILE names) if it happened to sit under one of them, so a sandboxed command could discover and overwrite/truncate the daemon bearer-token file even though it couldn't read or delete it. A file a sandboxed command must not read has no legitimate reason to be written either, so seatbeltProfileFromPermissionProfile now also emits a full file-write* deny for every DenyRead path, placed after the broad write allow (deny rules that follow an allow win, matching the existing DenyWrite/metadata-carveout ordering). Adds a regression test with a DenyRead file under a writable /tmp root, and extends the existing deny-ordering test to assert the new file-write* rule. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
38c150e to
8228de9
Compare
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Keep the daemon token excluded from Windows in-process tools.
internal/sandbox/profile.go:167
credentialDenyReadPathsreturns no paths on Windows, sowithAutomaticDenyReadadds nothing to the policy used byEvaluateandReadExclusions. A Windows remote-daemon session whose token file is in its workspace can therefore useread_file,grep, orglobto retrieve the bearer token; scrubbing the environment does not protect the file. The new end-to-end test skips Windows. Apply a non-bypassable in-process exclusion on Windows as well (and feed the Windows ACL path where appropriate), or reject this token-file setup there. -
[P1] Do not let
AllowReadre-include the automatic daemon-token deny.
internal/sandbox/engine.go:76
The helper deliberately retains the daemon token despiteAllowRead, but this method appends it to the ordinaryDenyReadlist.readDeniedResolvedthen re-includes a more-specificAllowReadpath, including one granted by a session or turn permission profile. Consequentlyread_file, grep, and glob can expose the bridge token despite the stated non-opt-out guarantee. Keep this automatic token exclusion separate from user-configured deny roots, or ensure its read check cannot be re-included. -
[P1] Do not skip the lexical symlink deny path in a writable bind.
internal/sandbox/linux_helper.go:249
When a deny entry has both its lexical symlink and resolved target, the new helper skips the lexical path and masks only the target. If the link is under a writable workspace or temp bind, sandboxed code can unlink and recreate that directory entry, then read or write through the replacement link. This defeats the replacement protection for whichnormalizeProfilePathVariantswas added, affecting bothDenyReadandDenyWrite. Preserve a deny mechanism for the writable lexical name or fail closed rather than accepting the unprotected replacement path. -
[P1] Ignore an unused dangling token-file pointer when an inline token wins.
internal/sandbox/profile.go:195
runDaemonServeRemotenow correctly leavesZERO_DAEMON_REMOTE_TOKEN_FILEalone whenZERO_DAEMON_REMOTE_TOKENis set, but automatic profile construction still processes that file variable. If it names a dangling symlink,Lstatadds the lexical link and Linux sandbox construction subsequently fails on its symlink component. Thus a valid inline-token remote daemon cannot run any sandboxed command merely because it inherited a stale pointer thatTokenFromEnvwould never use. Skip the file candidate whenever the inline token is selected, and cover the dangling-symlink variant of the precedence test. -
[P2] Preserve
DenyRead's read-only contract on macOS.
internal/sandbox/runner.go:643
This adds Seatbeltfile-write*denials for every configuredDenyReadpath, although the policy and the in-process gate reserve write restrictions forDenyWrite. A configuration that intentionally hides a generated file from reads while allowing a sandboxed command to refresh it now succeeds throughEngine.Evaluatebut fails only under the macOS backend. Restrict the added write deny to the non-overrideable credential paths that require it, or make the policy semantics consistent across every enforcement path. -
[P2] Apply the token's write protection to in-process write tools too.
internal/sandbox/engine.go:321
The automatic path is merged only intoDenyRead;validatePathWithPolicychecks that list only forSideEffectRead. A remote-controlled session can therefore usewrite_file,edit_file, orapply_patchon a workspace-local token, even though this patch correctly makes the shell-side credential deny non-writable. That permits token replacement or corruption (for example, a bridge restart denial of service). Add the automatic token path to the corresponding in-process write denial, with regression coverage for the direct write tools.
1c98788 to
f6e0191
Compare
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Apply the daemon-token exclusion to in-process read tools
internal/sandbox/profile.go:104
This only places the file inPermissionProfile.DenyRead, which protects wrapped commands; the currentEngine.Evaluateand grep/glob exclusions instead readPolicy.DenyRead. Consequently, when the remote daemon's token file is in the session workspace, the remote-controlled agent can invokeread_file, grep, or glob on it and exfiltrate the bridge bearer token. Add a non-overrideable automatic exclusion at the in-process read/search boundary and cover the end-to-end tool path. -
[P1] Make the automatic token path non-writable as well
internal/sandbox/profile.go:104
The token is added only toDenyRead, so direct write tools do not reject it, and macOS translates the entry intofile-read*and unlink denials while still broadly allowingfile-write*below the workspace and temporary roots. A remote agent can therefore overwrite or truncate a workspace-//tmp-local token withwrite_file/edit_file/apply_patch(or a macOS shell command), causing bridge denial of service or replacing the token on restart. Enforce an automatic write exclusion for this credential at both the direct-tool and Seatbelt boundaries. -
[P1] Canonicalize the selected token filename in the daemon's context
internal/sandbox/profile.go:195
TokenFromEnvreads a relative token filename relative to the daemon's startup directory, but workers inherit the same string and this helper resolves it relative to each session'scmd.Dir; the actual bearer file is therefore outside the generated deny list. For example, a daemon started in/daemonwithZERO_DAEMON_REMOTE_TOKEN_FILE=tokenprotects/session/tokenfor a worker launched in/session, while its live bearer file is/daemon/token. It also expands a leading~/even thoughos.ReadFiletreats that environment value literally, creating the same mismatch for a literal~directory. Resolve and persist the selected file path whenserve-remotestarts (using the same filename semantics asTokenFromEnv) before workers inherit it, and test a worker with a different CWD. -
[P1] Protect the configured token-file pathname, not just its symlink target
internal/sandbox/profile.go:200
normalizeProfilePathsresolves an existing symlink, so the profile denies only its current referent. If the configured pathname is a symlink under a writable workspace or temporary root, sandboxed code can replace that directory entry and the next daemon start will read the attacker-controlled replacement (or fail authentication). Preserve the lexical path with a Bubblewrap-safe protection/materialization strategy, or reject this unsafe configuration, and add a symlink-replacement regression test.
Summary
ZERO_DAEMON_REMOTE_TOKEN_FILEfrom inherited sandbox command environmentsAllowReadopt-out behaviorRoot cause
The sandbox scrubbed the inline
ZERO_DAEMON_REMOTE_TOKENvalue but left its file-pointer alternative in the child environment. Under the read-all workspace posture, a sandboxed command could read that pointer and then the bearer-token file outside the workspace.The pointer is now scrubbed on every platform. Its existing normalized target is protected through the same credential deny-read mechanism used for
GOOGLE_APPLICATION_CREDENTIALSon platforms where that mechanism is enabled. Windows filesystem deny-read remains subject to the existing ACL-model limitation tracked by #662.Fixes #677
Validation
go test ./internal/sandbox -count=1go vet ./...git diff --checkThe repository-pinned
golangci-lint@v2.12.2andgovulncheck@v1.3.0currently build with Go 1.25.12 and cannot load this repository's Go 1.26.5 packages.Summary by CodeRabbit