fix(sandbox): use WRITE_RESTRICTED token when no DenyRead paths are configured (#658)

Removing the WRITE_RESTRICTED flag in #612 made the restricted-SID check
apply to reads as well as writes. Default Windows DACLs grant
BUILTIN\Users rather than any SID in the token's restricted list
(random capability SIDs, logon SID, Everyone), so the sandboxed process
could no longer open any executable or non-KnownDlls DLL: every spawned
command failed with exit 1 and no output, including cmd builtins'
children (gh, git, where.exe). The existing
TestWindowsRestrictedTokenNestedPipeCapture smoke test reproduces the
regression.

Restore WRITE_RESTRICTED, but only when the permission profile has no
DenyRead paths. The flag makes the kernel skip restricted-SID deny ACEs
for reads, which is exactly the DenyRead bypass #612 fixed, so profiles
that configure DenyRead keep the fully restricted token and trade spawn
capability for read-deny enforcement. DenyRead is empty by default, so
the common case regains a working sandbox while #612's guarantee holds
for the profiles that rely on it.

Author: euxaristia

internal/sandbox/windows_command_runner_windows.go

@@ -69,7 +69,13 @@ func runWindowsSandboxCommand(config WindowsSandboxCommandConfig, stderr io.Writ
 	// this has no in-token fix; preflight blocking and output hints live in
 	// internal/tools/shell_runtime.go.
 	tokenSIDs := windowsRuntimeTokenSIDs(capabilitySIDs, offlineSID, config.PermissionProfile.Network.Mode)
-	token, err := createWindowsRestrictedTokenForCapabilitySIDs(tokenSIDs)
+	// A WRITE_RESTRICTED token keeps reads unrestricted so sandboxed commands
+	// can actually launch executables; 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
+	token, err := createWindowsRestrictedTokenForCapabilitySIDs(tokenSIDs, writeRestricted)
 	if err != nil {
 		fmt.Fprintln(stderr, WindowsSandboxCommandRunnerName+": "+err.Error())
 		return 1

internal/sandbox/windows_token_windows.go

@@ -48,7 +48,7 @@ func (sid windowsLocalSID) close() {
 	}
 }
 
-func createWindowsRestrictedTokenForCapabilitySIDs(capabilitySIDStrings []string) (windows.Token, error) {
+func createWindowsRestrictedTokenForCapabilitySIDs(capabilitySIDStrings []string, writeRestricted bool) (windows.Token, error) {
 	if len(capabilitySIDStrings) == 0 {
 		return 0, errors.New("windows restricted token requires at least one capability SID")
 	}
@@ -80,10 +80,10 @@ func createWindowsRestrictedTokenForCapabilitySIDs(capabilitySIDStrings []string
 		return 0, fmt.Errorf("open process token: %w", err)
 	}
 	defer base.Close()
-	return createWindowsRestrictedTokenFromBase(base, capabilitySIDs)
+	return createWindowsRestrictedTokenFromBase(base, capabilitySIDs, writeRestricted)
 }
 
-func createWindowsRestrictedTokenFromBase(base windows.Token, capabilitySIDs []windowsLocalSID) (windows.Token, error) {
+func createWindowsRestrictedTokenFromBase(base windows.Token, capabilitySIDs []windowsLocalSID, writeRestricted bool) (windows.Token, error) {
 	logonSID, err := copyWindowsLogonSID(base)
 	if err != nil {
 		return 0, err
@@ -102,10 +102,28 @@ func createWindowsRestrictedTokenFromBase(base windows.Token, capabilitySIDs []w
 		windows.SIDAndAttributes{Sid: worldSID},
 	)
 
+	// WRITE_RESTRICTED scopes the restricted-SID check to write-type accesses:
+	// reads use only the normal token identity, so the sandboxed process can
+	// open executables, DLLs, and per-user config the user can read, while
+	// writes still require a restricted-SID match (the workspace write-jail).
+	// Without it the restricted-SID check applies to READS too, and because
+	// default Windows DACLs grant BUILTIN\Users rather than any SID in the
+	// restricted list, the token cannot open ANY executable outside the
+	// ACL-granted write roots — every spawned command dies silently.
+	//
+	// The flag also makes the kernel skip restricted-SID deny ACEs for reads,
+	// which is the DenyRead bypass fixed in #612 — so it is only safe when the
+	// profile has no DenyRead paths. The caller passes writeRestricted=false
+	// whenever DenyRead is configured, keeping #612's enforcement for exactly
+	// the profiles that need it.
+	flags := uint32(windowsDisableMaxPrivilege | windowsLUAToken)
+	if writeRestricted {
+		flags |= windowsWriteRestricted
+	}
 	var restricted windows.Token
 	result, _, callErr := procCreateRestrictedToken.Call(
 		uintptr(base),
-		uintptr(windowsDisableMaxPrivilege|windowsLUAToken),
+		uintptr(flags),
 		0,
 		0,
 		0,