test(sandbox): make a denied write distinguishable from a runner failure

The denial assertions expected exit 1, which the runner also returns for
its own errors -- a failed marker validation, a bad argument, a token it
could not build. Written that way the test passes both when the sandbox
denies the write and when nothing ever ran, and the second case proves
nothing while being indistinguishable from success.

That is a poor property for any test and a bad one for this test, whose
entire job is to notice a confinement regression. The leading
"granted root is writable" assertion covered part of it, but said nothing
about whether the two later commands launched.

The denial commands now report a distinctive exit code the runner cannot
produce for itself, so the code proves cmd.exe actually ran and its
redirect was refused.

Re-checked in both directions rather than assumed: the test passes against
the fix, and restoring the World SID unconditionally fails it with
"exit code = 0, want 77" -- still catching the bug it was written for.

Raised by CodeRabbit on #865.

internal/sandbox/runner_windows_integration_test.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -386,6 +387,127 @@ func runWindowsRealSmokeCommand(t *testing.T, runnerExe string, base WindowsSand
 	}
 }
 
+// The write jail must hold on a path whose DACL grants Everyone write access.
+//
+// A WRITE_RESTRICTED token runs TWO checks for a write and needs both to pass:
+// the normal one against its enabled SIDs, and a second against its RESTRICTED
+// SID list. The jail is built on the second check only succeeding where Zero has
+// explicitly ACL'd a capability SID. Putting the World SID (S-1-1-0) in the
+// restricted list breaks that globally — every principal is a member of
+// Everyone, so on a DACL that grants Everyone write, the restricted half passes
+// for free and confinement collapses back to the ordinary user's own
+// permissions, which is precisely the boundary the sandbox exists to be
+// stricter than.
+//
+// The runner already states this rule for the SIDs it refuses to add ("None of
+// the granted SIDs can be added to the restricted list without collapsing the
+// write jail"); Everyone was in the list anyway, since the original sandbox
+// baseline.
+//
+// Realistic rather than theoretical: administrators open share roots with
+// Everyone:F, and third-party installers ship loose ACLs. It needs no privilege,
+// no symlink and no race. Checked and ruled out: C:\Users\Public\Documents
+// grants BATCH, not Everyone, so the common "public folder" case is not this.
+func TestWindowsRestrictedTokenDeniesWritesToEveryoneWritablePaths(t *testing.T) {
+	if os.Getenv("ZERO_SANDBOX_REAL_SMOKE") != "1" {
+		t.Skip("set ZERO_SANDBOX_REAL_SMOKE=1 to run real Windows sandbox smoke tests")
+	}
+	runnerExe := realSmokeExecutable(t, "ZERO_WINDOWS_COMMAND_RUNNER_EXE", WindowsSandboxCommandRunnerName)
+
+	root := t.TempDir()
+	outside := t.TempDir()
+	everyoneDir := filepath.Join(outside, "everyone-writable")
+	if err := os.MkdirAll(everyoneDir, 0o700); err != nil {
+		t.Fatalf("MkdirAll everyone-writable: %v", err)
+	}
+	// Granted through the production ACL applier, so the hostile DACL is built
+	// the same way a real one is rather than by a test-only shortcut.
+	snapshot, applied, err := applyWindowsACLPathGroup(windowsACLPathGroup{
+		Path: everyoneDir,
+		Entries: []WindowsACLEntry{{
+			Action:     WindowsACLAllowWrite,
+			Path:       everyoneDir,
+			Capability: "S-1-1-0",
+		}},
+	})
+	if err != nil {
+		t.Fatalf("grant Everyone write: %v", err)
+	}
+	if !applied {
+		t.Fatal("precondition: the Everyone grant did not apply, so there is no bypass to test for")
+	}
+	t.Cleanup(func() { _ = rollbackWindowsACLSnapshots([]windowsACLSnapshot{snapshot}) })
+
+	sandboxHome := filepath.Join(root, ".zero-sandbox")
+	// No DenyRead on purpose: that is what makes the runner choose the
+	// WRITE_RESTRICTED token, which is the default posture and the one whose
+	// restricted-SID list this test is about.
+	profile := PermissionProfile{
+		FileSystem: FileSystemPolicy{
+			Kind:                 FileSystemRestricted,
+			ReadRoots:            []string{root},
+			WriteRoots:           []WritableRoot{{Root: root, ProtectedMetadataNames: []string{".git", ".zero", ".agents"}}},
+			IncludePlatformRoots: true,
+			AllowTemp:            true,
+		},
+		Network: NetworkPolicy{Mode: NetworkDeny},
+	}
+	config := WindowsSandboxCommandArgsOptions{
+		SandboxHome:       sandboxHome,
+		CommandCWD:        root,
+		WorkspaceRoots:    []string{root},
+		PermissionProfile: profile,
+		SandboxLevel:      WindowsSandboxLevelUnelevated,
+	}
+
+	// Granted root first. A jail that denies everything would satisfy the two
+	// assertions below while being useless, and this separates the two outcomes.
+	insideMarker := filepath.Join(root, "write-ok.txt")
+	runWindowsRealSmokeCommand(t, runnerExe, config, []string{
+		"cmd.exe", "/d", "/s", "/c", "echo ok>" + insideMarker,
+	}, 0)
+	if contents, err := os.ReadFile(insideMarker); err != nil || strings.TrimSpace(string(contents)) != "ok" {
+		t.Fatalf("sandboxed write to a granted root = %q, %v; want ok", contents, err)
+	}
+
+	// Control: an ordinary directory outside every granted root. This is the
+	// behaviour the Everyone case must match.
+	controlMarker := filepath.Join(outside, "control-denied.txt")
+	runWindowsRealSmokeCommand(t, runnerExe, config, deniedWriteCommand(controlMarker), deniedWriteExitCode)
+	if _, err := os.Stat(controlMarker); err == nil {
+		t.Fatal("precondition: the sandbox allowed a write to an ordinary path outside every granted root, so this test cannot measure the Everyone case")
+	}
+
+	// The assertion. Same as the control in every respect except the DACL.
+	everyoneMarker := filepath.Join(everyoneDir, "everyone-denied.txt")
+	runWindowsRealSmokeCommand(t, runnerExe, config, deniedWriteCommand(everyoneMarker), deniedWriteExitCode)
+	if _, err := os.Stat(everyoneMarker); err == nil {
+		t.Error("the sandbox wrote outside every granted root because the path grants Everyone write; the restricted-SID list is satisfied by a SID every principal carries")
+	} else if !os.IsNotExist(err) {
+		t.Errorf("stat the Everyone-writable marker: %v", err)
+	}
+}
+
+// deniedWriteExitCode is chosen so a denial cannot be confused with the runner
+// failing to start the command at all.
+//
+// The obvious spelling of these assertions is `echo leaked>path` expecting exit
+// 1, but the runner itself exits 1 for its own errors — a failed marker
+// validation, a bad argument, a token it could not build. A test written that
+// way passes both when the sandbox denies the write and when nothing ever ran,
+// and the second case proves nothing while looking identical to success. On a
+// test whose whole job is catching a confinement regression, that is a failure
+// mode to design out rather than hope about.
+//
+// 77 is arbitrary beyond being outside the range the runner produces for itself.
+const deniedWriteExitCode = 77
+
+// deniedWriteCommand attempts a write and reports deniedWriteExitCode when the
+// redirect is refused, so the exit code also proves cmd.exe actually ran.
+func deniedWriteCommand(marker string) []string {
+	return []string{"cmd.exe", "/d", "/s", "/c", "echo leaked>" + marker + " || exit " + strconv.Itoa(deniedWriteExitCode)}
+}
+
 func powershellSingleQuote(value string) string {
 	out := "'"
 	for _, r := range value {

internal/sandbox/windows_token_windows.go

@@ -88,19 +88,55 @@ func createWindowsRestrictedTokenFromBase(base windows.Token, capabilitySIDs []w
 	if err != nil {
 		return 0, err
 	}
-	worldSID, err := windows.CreateWellKnownSid(windows.WinWorldSid)
-	if err != nil {
-		return 0, fmt.Errorf("create world SID: %w", err)
-	}
-
 	entries := make([]windows.SIDAndAttributes, 0, len(capabilitySIDs)+2)
 	for _, sid := range capabilitySIDs {
 		entries = append(entries, windows.SIDAndAttributes{Sid: sid.sid})
 	}
-	entries = append(entries,
-		windows.SIDAndAttributes{Sid: sidFromBytes(logonSID)},
-		windows.SIDAndAttributes{Sid: worldSID},
-	)
+	entries = append(entries, windows.SIDAndAttributes{Sid: sidFromBytes(logonSID)})
+
+	// The World SID (S-1-1-0, Everyone) is added ONLY to the token that does not
+	// carry WRITE_RESTRICTED, and putting it back unconditionally would reopen a
+	// write-jail bypass.
+	//
+	// A restricted SID is a key to every object whose DACL names it. That is why
+	// the runner refuses to add the user SID, Administrators or SYSTEM — "each
+	// has write access nearly everywhere". Everyone is the broadest of the lot:
+	// every principal carries it, so under WRITE_RESTRICTED the second
+	// (restricted-SID) check passes for free on any path whose DACL grants
+	// Everyone write, and confinement falls back to the ordinary user's own
+	// permissions — the exact boundary this token exists to be stricter than. It
+	// needs no privilege, no symlink and no race: an Everyone-writable directory
+	// is enough, and share roots opened with Everyone:F and loose installer ACLs
+	// supply them. It was present from the original sandbox baseline,
+	// uncommented, and under WRITE_RESTRICTED nothing depends on it, because that
+	// flag already exempts reads from the restricted-SID check.
+	//
+	// Without the flag it IS load-bearing and cannot simply be dropped. The
+	// restricted-SID check then applies to READS too, and default Windows DACLs
+	// grant BUILTIN\Users rather than anything in this list, so a token without
+	// Everyone cannot open cmd.exe — the process dies at launch with
+	// STATUS_ACCESS_DENIED (0xC0000022) before it runs anything. That path is
+	// only taken when the profile configures DenyRead, which is already the
+	// posture that trades capability for read-deny enforcement (#612).
+	//
+	// So the bypass survives for DenyRead profiles, deliberately and narrowly,
+	// rather than being traded for a sandbox that cannot start a command. Closing
+	// it there needs a different mechanism (a read-side grant that is not a
+	// universal group), tracked separately.
+	//
+	// The logon SID above stays in both modes: it is this token's own rather than
+	// a broad group, and broadenWindowsRestrictedTokenDefaultDacl depends on it so
+	// the process can use pipes and events it creates for itself.
+	//
+	// Anything added here needs the same scrutiny — Authenticated Users, Users,
+	// INTERACTIVE and BATCH would each produce this bypass on a DACL naming them.
+	if !writeRestricted {
+		worldSID, err := windows.CreateWellKnownSid(windows.WinWorldSid)
+		if err != nil {
+			return 0, fmt.Errorf("create world SID: %w", err)
+		}
+		entries = append(entries, 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