Repository navigation
Windows sandbox does not deny reads of cloud credential stores #662
Description
Context
PR #660 added default deny-read rules so sandboxed commands cannot read well-known credential stores (~/.aws, ~/.config/gcloud, ~/.azure, and the file GOOGLE_APPLICATION_CREDENTIALS points to). The rules are injected at permission profile construction (credentialDenyReadPaths in internal/sandbox/profile.go) and enforced by the seatbelt backend on macOS (file-read* deny rules) and the Linux helper (unreadable binds).
Windows is deliberately exempted, so on Windows a sandboxed command can still read those credential files under the read-all workspace posture.
Why Windows was skipped
Windows deny-read is implemented via ACL deny entries applied against capability SIDs (planWindowsDenyReadPaths / windowsReadDenyCapabilitySIDs in internal/sandbox/windows_acl.go). Making the profile DenyRead list non-empty by default has two side effects:
- windowsReadDenyCapabilitySIDs stops returning nil, which moves the runner off the WRITE_RESTRICTED token path that fix(sandbox): use WRITE_RESTRICTED token when no DenyRead paths are configured #658 introduces for the no-deny-read case. The unelevated tier depends on that path; the full restricted-token path has a history of commands silently exiting 1 when unelevated.
- The ACL deny entries mutate filesystem ACLs on the user's real credential directories, which is a heavier and more persistent intervention than the process-scoped seatbelt/bwrap rules on the other platforms, and needs elevation in some configurations.
What needs deciding
- Whether default credential deny-read on Windows should wait for the token/ACL model to settle (fix(sandbox): keep Windows restricted-token SIDs narrow (no Users broaden) #640, fix(sandbox): use WRITE_RESTRICTED token when no DenyRead paths are configured #658 are in flight), or whether a scoped mechanism exists that does not flip the token selection.
- Whether ACL-based deny on real user directories is acceptable at all as a default, given it outlives the sandboxed process.
- Whether the interim mitigation (env scrubbing of provider keys and GOOGLE_APPLICATION_CREDENTIALS, which does apply on Windows) is enough until then.
Pointer
The exemption and rationale are documented on credentialDenyReadPaths in internal/sandbox/profile.go ("Windows is skipped" comment). Remove the runtime.GOOS gate there once a Windows mechanism lands.
Activity
Vasanthdev2004 commentedon Jul 26, 2026
Writing up what I found while reviewing #681 and #640, because this issue is blocking both of them and the reason it has been hard to close is not obvious from the outside.
Where it stands today
credentialDenyReadPaths(internal/sandbox/profile.go) opens with:So on Windows the credential deny-read list is empty and no credential path is protected. On Linux and macOS the same function returns ~/.aws, ~/.config/gcloud, ~/.azure and $GOOGLE_APPLICATION_CREDENTIALS, and #681 extends that set with Zero's own config dir, both OAuth token stores and their .secret siblings, while explicitly deferring Windows here.
The machinery to enforce it on Windows already exists: windows_acl.go has a
WindowsACLDenyReadaction andplanWindowsDenyReadPathsbuilds deny ACEs fromPermissionProfile.FileSystem.DenyRead. Nothing is missing at the ACL layer.Why it is not a one-line fix
This is the part worth knowing before deciding. From windows_command_runner_windows.go:
Two token shapes, and they are mutually exclusive:
So simply deleting the Windows early return would flip
writeRestrictedto false for every sandboxed command and break command execution. That is exactly the wall #640 hit, and why it proposes adding Users and Authenticated Users to the restricting SIDs. In other words #662 and #640 are the same decision, and picking an option here decides #640 too.Options
A. Turn on the existing mechanism and accept the fully restricted token (this is #640's path).
Remove the early return, then broaden the restricting SIDs so executables still open. Gets real kernel-enforced deny-read for credentials. The cost is what I found reviewing #640: adding Users/AuthUsers widens the write jail for every securable object type, not just files, since restricting SIDs are intersected on registry keys, services and named pipes too, while the compensating DACL rewrite only covers the filesystem. #640 also currently fails setup on a normal machine and rewrites system DACLs permanently. Highest fidelity, highest blast radius.
B. Keep WRITE_RESTRICTED and deny reads by a different mechanism.
A plain DENY ACE naming the user account is honoured regardless of restricted-SID logic, but the sandboxed child runs as the same user as Zero itself, so denying the child also denies the parent. This only works with a distinct sandbox identity, which is option D. Not viable on its own.
C. Reduce what is worth reading (storage hardening, cross-platform).
Zero already supports a keyring backend and an encrypted-file backend for provider keys. If secrets are not sitting in readable plaintext, a sandboxed read of the config dir yields nothing useful, and this helps Linux and macOS too rather than being a Windows-only patch. Worth confirming what the default backend actually is on Windows and whether the OAuth token stores are covered, since those are files today. Cheapest real risk reduction, does not need any token surgery.
D. Give the sandbox a distinct identity (AppContainer or a dedicated user).
The proper Windows answer. A separate SID makes deny ACEs target the sandbox without touching Zero, and it fixes reads structurally rather than by carve-out. It is also the modern platform recommendation and would let the write jail keep WRITE_RESTRICTED. Much larger change, and it touches the whole Windows execution path.
E. Accept and document.
Leave the posture as is, keep scrubbing credential environment variables so tools cannot discover paths as easily, and state plainly in the docs that on Windows the sandbox is a write jail and network gate, not a confidentiality boundary. Costs nothing, closes nothing, but stops the gap being invisible.
What I would suggest
C first, then D, and hold A.
C is the only option that reduces real risk without a token redesign, and it helps every platform. D is the right long-term shape. I would not take A as currently implemented: it buys credential read-deny at the price of widening the write jail across the whole object namespace, which is a worse trade than the problem it solves, and #640 needs rework regardless.
Whatever is chosen, I would also keep this issue open when #681 merges. #681 fixes the Linux and macOS half and correctly defers Windows here, so closing #675 on that merge would otherwise make the Windows gap invisible.
Happy to prototype C (confirm the default credential backend on Windows and cover the OAuth token stores) if that is the direction.
feat(sandbox): add Windows sandbox principals
Vasanthdev2004 commentedon Jul 28, 2026
Correcting my own comment above. I recommended "C first, then D, and hold A", where C was storage hardening on the theory that if secrets are not sitting in readable plaintext then a sandboxed read of the config directory yields nothing useful. I flagged at the time that this needed confirming. I have now confirmed it, and it does not hold on the platform this issue is about.
What the credential backend actually does
credstore.New(internal/credstore/credstore.go) picks the backend like this:So on Windows, unless the user has explicitly set
ZERO_CRED_STORAGE, the backend isencrypted-file. And that backend is constructed as:The AES key lives at
<file>.secret, in the same directory, with the same 0600 mode. The OAuth token store has the same shape:internal/oauth/encrypt.godescribes "a per-user random secret persisted (0600) beside the token file".Why that breaks the argument
Encryption at rest with the key stored next to the ciphertext defends against someone who obtains the file on its own: a backup, a stray copy, a stolen disk. It does not defend against a process that can read the directory, because that process reads both halves and decrypts. A sandboxed command under the read-all posture is exactly such a process.
Store.Encrypted()returning true is therefore accurate for its intended meaning ("protected at rest") and misleading if read as "protected from a sandboxed read". Those are different threat models and only the first one holds here.Revised recommendation
ZERO_CRED_STORAGE=keyring. It is still worth doing for those cases, but it cannot be the answer to this issue, and I was wrong to put it first..secretsiblings rather than only the token files. That distinction matters for the reason above, and it is right.Net effect on the ordering: D for Windows, #681 for the other platforms, and C demoted from "cheapest real risk reduction" to "worth having for macOS keyring users but not a fix for this issue".
Apologies for the detour. The original comment reads as though the cheap option was available and it is not.
feat(sandbox): add Windows sandbox principals
feat(sandbox): add Windows sandbox principals
feat(sandbox): add Windows sandbox principals
feat(sandbox): add Windows sandbox principals
feat(sandbox): add Windows sandbox principals
fix(sandbox): address CodeRabbit review findings
feat(sandbox): add Windows sandbox principals
fix(sandbox): keep principal provisioning closed until a launch path …
feat(sandbox): add Windows sandbox principals
fix(sandbox): keep principal provisioning closed until a launch path …
feat(sandbox): add Windows sandbox principals
fix(sandbox): keep principal provisioning closed until a launch path …
feat(sandbox): add Windows sandbox principals
fix(sandbox): keep principal provisioning closed until a launch path …
Vasanthdev2004 commentedon Sep 24, 2026
Status on
mainat 99721c7, since two things have changed here and neither closes this.An explicitly configured DenyRead on Windows now fails closed rather than being quietly ineffective.
#1006(2026-09-11) addedwindowsDenyReadRestrictedTokenUnsupportedProfile, checked by the manager, the command-plan builder, setup and the runner, so a profile with DenyRead is rejected before any restricted-token path can provision or launch, with an error that names the mechanism:So the shape of the gap has narrowed. It is no longer "a user can configure a deny that silently does nothing"; it is "there is no default credential protection on Windows, and asking for one is refused with a reason".
credentialDenyReadPathsstill returns an empty set immediately whenruntime.GOOS == "windows", so the default skip this issue describes is unchanged.#808 does not lift the blocker, despite its title. It says "foundation for #662, does not close it", and I want that to be unmissable from here rather than only from there. It is opt-in behind an environment variable, no principal runs a command,
windowsPrincipalLaunchAvailableanswers unavailable on every machine, and both setup entry points therefore refuse to provision a principal. What it changes today is whatzero doctorreports.The precondition the code names is the same one it named a year of commits ago: access-time confinement. Until something provides that, the two side effects in the issue body still hold, and in particular the ACL objection is unchanged, since a deny ACE mutates the user's real credential directory whichever SID it names.
One correction carried forward so nobody picks it up from this thread: the storage-hardening option I ranked first in an earlier comment here was wrong for this platform and I retracted it. Encryption at rest does not help against a process that can read the containing directory, because
credstoreselectsencrypted-fileeverywhere except macOS with a working keyring, and that backend keeps the AES key beside the ciphertext at the same mode.