Windows sandbox does not deny reads of cloud credential stores

[euxaristia]

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:

1. 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.
2. 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(windows): add Users and Authenticated Users SIDs to restricted token SIDs #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.

[Vasanthdev2004]

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 WindowsACLDenyRead action and planWindowsDenyReadPaths builds deny ACEs from PermissionProfile.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 writeRestricted to 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.

[Vasanthdev2004]

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 is encrypted-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.go describes "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) #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 .secret siblings 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.