Repository navigation
Windows sandbox does not deny reads of cloud credential stores #662
Description
Activity
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:if runtime.GOOS == "windows" { return nil }
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:
// ... it is only unsafe when DenyRead paths are configured, because the // kernel skips restricted-SID deny ACEs for reads under that flag (#612). // Profiles with DenyRead keep the fully restricted token, trading spawn // capability for read-deny enforcement. writeRestricted := len(config.PermissionProfile.FileSystem.DenyRead) == 0
Two token shapes, and they are mutually exclusive:
- WRITE_RESTRICTED (today's default): strong write jail, but the kernel skips restricted-SID deny ACEs on reads, so deny-read does not enforce.
- Fully restricted (drop WRITE_RESTRICTED): deny-read ACEs do enforce, but the restricting-SID check now gates reads too, and stock DACLs on C:\Windows and C:\Program Files grant BUILTIN\Users rather than Everyone, so the process cannot open executables and commands fail.
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.
- added a commit that references this issue
on Jul 27, 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:// Keyring is the default only on macOS, where `security` is reliable and needs // no running daemon. Elsewhere (Linux secret-tool, headless/CI) default to the // encrypted file; keyring stays available via an explicit ZERO_CRED_STORAGE. if goos == "darwin" && kr.Available() { storage = "keyring" } else { storage = "encrypted-file" }
So on Windows, unless the user has explicitly set
ZERO_CRED_STORAGE, the backend isencrypted-file. And that backend is constructed as:path := filepath.Join(options.Dir, encryptedFileName) return &Store{backend: "encrypted-file", file: path, crypter: securefile.NewCrypter(path + ".secret")}, nil
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
- C is not a risk reduction on Windows or Linux. It only helps on macOS with the keyring available, or where a user has deliberately set
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. - D (a distinct sandbox identity) is now clearly the right shape for Windows, not merely the nicer long-term one. It is the only option that gives the sandbox a different SID, which is what makes a deny ACE meaningful without the WRITE_RESTRICTED tradeoff described above. feat(sandbox): Windows sandbox principals (foundation for #662, does not close it) #808 and feat(sandbox): give each workspace an offline and an online principal #812 build exactly that foundation.
- Deny-read remains correct on Linux and macOS, which is what fix(sandbox): deny reads of Zero credential stores #681 does, and it already covers the
.secretsiblings rather than only the token files. That distinction matters for the reason above, and it is right. - A still looks like a bad trade for the reasons in my original comment, and nothing here changes that.
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.
- C is not a risk reduction on Windows or Linux. It only helps on macOS with the keyring available, or where a user has deliberately set
- added a commit that references this issue
on Aug 1, 2026 - added a commit that references this issue
on Aug 21, 2026 - added a commit that references this issue
on Aug 23, 2026 - added a commit that references this issue
on Aug 27, 2026 - added 7 commits that reference this issue
on Aug 31, 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:DenyRead is not supported with the Windows restricted-token sandbox (elevated or unelevated): without Users/Authenticated Users in the restricting SID set, ordinary system binaries under Program Files and Windows cannot load, and adding those groups would admit their existing write grants outside WriteRoots.
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.
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:
What needs deciding
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.