fix(sandbox): agree on the runtime root across the Windows setup marker

internal/doctor/hardening.go

@@ -100,10 +100,31 @@ func windowsSandboxSetupCheck(goos string, backend sandbox.Backend, workspaceRoo
 	}
 	profile := sandbox.PermissionProfileFromPolicy(workspaceRoot, doctorSandboxPolicy(sandboxConfig), scope)
 	setupConfig := sandbox.WindowsSandboxSetupConfig{
-		SandboxHome:       sandboxHome,
-		CommandCWD:        workspaceRoot,
-		WorkspaceRoots:    []string{workspaceRoot},
-		PermissionProfile: profile,
+		SandboxHome:    sandboxHome,
+		CommandCWD:     workspaceRoot,
+		WorkspaceRoots: []string{workspaceRoot},
+		// The same augmentation setup and the command plan apply, so doctor
+		// fingerprints what a real command fingerprints. Checking the bare profile
+		// made doctor call a correctly prepared machine "out of date", which is the
+		// mismatch this pairing exists to close. Safe to resolve in this process:
+		// doctor runs in the operator's shell, not behind the sandbox TEMP
+		// redirection that stops the runner deriving these for itself.
+		//
+		// The RECORDED runtime root goes on the profile as well, because that is
+		// what makes the stamp check run at all. Without it profile.Runtime is nil,
+		// validateWindowsSandboxRuntimeStamp returns nil early, and doctor reported
+		// a healthy sandbox on a machine whose runtime tree had been evicted and
+		// silently recreated without the capability ACE -- precisely the state the
+		// stamp exists to catch, and the state where every sandboxed command then
+		// fails with nothing explaining why.
+		//
+		// Read from the marker rather than selected, so doctor takes no lease and
+		// creates nothing. A marker from an older schema records no root, which
+		// leaves this exactly as it was.
+		PermissionProfile: sandbox.WindowsSandboxProfileWithRuntimeRoots(
+			sandbox.PermissionProfileWithRuntimeRoot(profile, sandbox.WindowsSandboxRecordedRuntimeRoot(sandboxHome)),
+			[]string{workspaceRoot},
+		),
 	}
 	if err := sandbox.ValidateWindowsSandboxSetupMarker(setupConfig); err != nil {
 		result := check("sandbox.backend", "Sandbox backend", StatusWarn, fmt.Sprintf("Native sandbox backend %s is installed, but Windows sandbox setup is missing or out of date: %v.", backend.Name, err), map[string]any{

internal/doctor/windows_runtime_stamp_test.go

@@ -0,0 +1,138 @@
+package doctor
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/Gitlawb/zero/internal/config"
+	"github.com/Gitlawb/zero/internal/sandbox"
+)
+
+// DOCTOR HAS TO ASK THE ONE QUESTION THE MARKER CANNOT ANSWER.
+//
+// The marker hashes ACL-plan entries, which are pathnames. Whether the directory
+// those pathnames resolve to is still the tree setup provisioned is a different
+// question, and the runtime stamp is what answers it. validateWindowsSandboxRuntimeStamp
+// returns nil early when profile.Runtime is nil, which is correct for the setup
+// side and for unrestricted profiles, and was wrong here: doctor built its
+// profile with PermissionProfileFromPolicy, which never sets Runtime, so the
+// check was skipped and `zero doctor` reported a healthy sandbox on exactly the
+// state the stamp was added to detect -- an evicted runtime tree, silently
+// recreated with ordinary permissions and no capability ACE, where every
+// sandboxed command then fails with nothing explaining why.
+// doctorRuntimeCandidate returns a runtime root this workspace would really
+// select, taken from the candidate set the Windows plan folds in.
+//
+// It has to be a REAL candidate. An arbitrary path is already in the ACL plan on
+// one side and not the other, so the plan hashes diverge and validation fails
+// before the stamp is ever consulted -- which would make this test fail without
+// the fix for a reason that has nothing to do with the stamp.
+// redirectUserCache points os.UserCacheDir at test-owned storage.
+//
+// WITHOUT THIS THE TEST WRITES INTO THE DEVELOPER'S REAL CACHE. The runtime
+// candidate is derived from the process's actual user cache directory, so the
+// test was creating, recursively removing, recreating and then cleanup-removing
+// a directory under the real %LocalAppData%zerountime (or ~/.cache/zero on
+// Unix). The workspace digest made a collision with a live runtime tree
+// unlikely rather than impossible, and "unlikely" is not the standard for a
+// test that calls RemoveAll.
+//
+// All three variables, because os.UserCacheDir reads a different one per
+// platform: %LocalAppData% on Windows, $XDG_CACHE_HOME or $HOME on Unix, and
+// $HOME on macOS.
+func redirectUserCache(t *testing.T) {
+	t.Helper()
+	cache := t.TempDir()
+	t.Setenv("LOCALAPPDATA", cache)
+	t.Setenv("XDG_CACHE_HOME", cache)
+	t.Setenv("HOME", cache)
+}
+
+func doctorRuntimeCandidate(t *testing.T, workspace string) string {
+	t.Helper()
+	bare := doctorProfile(t, workspace)
+	augmented := sandbox.WindowsSandboxProfileWithRuntimeRoots(bare, []string{workspace})
+	existing := map[string]bool{}
+	for _, root := range bare.FileSystem.WriteRoots {
+		existing[root.Root] = true
+	}
+	for _, root := range augmented.FileSystem.WriteRoots {
+		if !existing[root.Root] {
+			return root.Root
+		}
+	}
+	t.Skip("no runtime candidate is derivable in this environment")
+	return ""
+}
+
+func doctorProfile(t *testing.T, workspace string) sandbox.PermissionProfile {
+	t.Helper()
+	scope, err := sandbox.NewScope(workspace, nil)
+	if err != nil {
+		t.Fatalf("NewScope: %v", err)
+	}
+	return sandbox.PermissionProfileFromPolicy(workspace, doctorSandboxPolicy(config.SandboxConfig{}), scope)
+}
+
+func writeDoctorSetupMarker(t *testing.T, home, workspace, runtimeRoot string) {
+	t.Helper()
+	profile := doctorProfile(t, workspace)
+	setup := sandbox.WindowsSandboxSetupConfig{
+		SandboxHome:    home,
+		CommandCWD:     workspace,
+		WorkspaceRoots: []string{workspace},
+		PermissionProfile: sandbox.WindowsSandboxProfileWithRuntimeRoots(
+			sandbox.PermissionProfileWithRuntimeRoot(profile, runtimeRoot),
+			[]string{workspace},
+		),
+	}
+	if _, err := sandbox.WriteWindowsSandboxSetupMarker(setup); err != nil {
+		t.Fatalf("WriteWindowsSandboxSetupMarker: %v", err)
+	}
+}
+
+func TestDoctorReportsAnEvictedRuntimeTree(t *testing.T) {
+	home := t.TempDir()
+	workspace := t.TempDir()
+	t.Setenv("ZERO_WINDOWS_SANDBOX_HOME", home)
+	redirectUserCache(t)
+
+	runtimeRoot := doctorRuntimeCandidate(t, workspace)
+	// Belt and braces: if the redirection above ever stops working, fail loudly
+	// rather than quietly operating on the developer's real cache.
+	if !strings.HasPrefix(runtimeRoot, os.TempDir()) && !strings.Contains(runtimeRoot, t.Name()) {
+		t.Fatalf("the runtime candidate %q is outside test-owned storage; this test creates and removes that path", runtimeRoot)
+	}
+	if err := os.MkdirAll(runtimeRoot, 0o700); err != nil {
+		t.Fatalf("create the runtime root: %v", err)
+	}
+	t.Cleanup(func() { _ = os.RemoveAll(runtimeRoot) })
+	writeDoctorSetupMarker(t, home, workspace, runtimeRoot)
+
+	backend := sandbox.Backend{Name: sandbox.BackendWindowsRestrictedToken}
+
+	// A healthy machine first, or the eviction assertion below would be satisfied
+	// by a check that warns unconditionally.
+	if result := windowsSandboxSetupCheck("windows", backend, workspace, config.SandboxConfig{}); result != nil {
+		t.Fatalf("a freshly set-up machine was reported unhealthy: %s", result.Message)
+	}
+
+	// cleanupSandboxRuntimeRoots evicts inactive roots on an age and count policy,
+	// and the next run recreates the pathname with inherited permissions. The plan
+	// hash never moves, so only the stamp can tell.
+	if err := os.RemoveAll(runtimeRoot); err != nil {
+		t.Fatalf("evict the runtime root: %v", err)
+	}
+	if err := os.MkdirAll(runtimeRoot, 0o700); err != nil {
+		t.Fatalf("recreate the pathname the way an ordinary run would: %v", err)
+	}
+
+	result := windowsSandboxSetupCheck("windows", backend, workspace, config.SandboxConfig{})
+	if result == nil {
+		t.Fatal("doctor reported a healthy sandbox while the provisioned runtime tree was gone; every sandboxed command on this machine would fail with nothing explaining why")
+	}
+	if !strings.Contains(strings.ToLower(result.Message), "setup") {
+		t.Errorf("the warning does not point at setup: %s", result.Message)
+	}
+}

internal/sandbox/main_test.go

@@ -0,0 +1,35 @@
+package sandbox
+
+import (
+	"os"
+	"testing"
+)
+
+// TestMain points the sandbox runtime's user-cache root at test-owned storage
+// for the whole package.
+//
+// PLAN CONSTRUCTION CREATES DIRECTORIES, which is easy to miss because it reads
+// like naming. windowsSandboxProfileWithProvisionedRuntime provisions the root
+// it selects, and the simulated-Windows tests run on every platform, so a test
+// that supplies an explicit child environment but leaves the cache alone writes
+// into the developer's real one. Not hypothetical: the machine this was found on
+// had accumulated thousands of entries under the real runtime root from exactly
+// these runs.
+//
+// Done once for the package rather than per test, because the leak is in the
+// DEFAULT: any new test that builds a Windows plan is affected unless its author
+// remembers, and remembering is what failed here. A test that needs a specific
+// cache root still overrides sandboxUserCacheDir itself.
+func TestMain(m *testing.M) {
+	root, err := os.MkdirTemp("", "zero-sandbox-testcache-")
+	if err != nil {
+		// Fail loudly rather than silently falling back to the real cache.
+		panic("sandbox tests: create the test cache root: " + err.Error())
+	}
+	sandboxUserCacheDir = func() (string, error) { return root, nil }
+
+	code := m.Run()
+
+	_ = os.RemoveAll(root)
+	os.Exit(code)
+}

internal/sandbox/runner.go

@@ -189,7 +189,18 @@ func (engine *Engine) BuildCommandPlan(spec CommandSpec) (CommandPlan, error) {
 	}
 	var runtimeCleanup func()
 	if preference != SandboxPreferenceForbid && policy.Mode != ModeDisabled {
-		runtimeState, cleanup, runtimeErr := prepareSandboxRuntime(workspaceRoot)
+		// The home THIS command asked for. Windows planning resolves
+		// ZERO_WINDOWS_SANDBOX_HOME out of spec.Env and hands it to the runner for
+		// marker validation, so selection has to read the same environment or the
+		// two disagree about which marker describes the tree. See
+		// pinnedSandboxRuntimeRoot.
+		commandSandboxHome := ""
+		if spec.Env != nil {
+			if resolved, err := ResolveWindowsSandboxHome(envListToMap(spec.Env)); err == nil {
+				commandSandboxHome = resolved
+			}
+		}
+		runtimeState, cleanup, runtimeErr := prepareSandboxRuntime(workspaceRoot, commandSandboxHome)
 		if runtimeErr != nil {
 			return CommandPlan{}, runtimeErr
 		}

internal/sandbox/runtime_bound_records_test.go

@@ -0,0 +1,88 @@
+package sandbox
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// THE TWO HALVES OF A ROLLBACK RECORD MUST DESCRIBE ONE OBJECT.
+//
+// The snapshot used to read the identity through a handle, close it, and then
+// re-resolve the pathname to read the stamp. A rename between those pairs one
+// directory's identity with another's bytes, and a rollback that correctly
+// proves it holds the first then writes the second's contents into it.
+//
+// The window itself needs an elevated installer racing an unelevated renamer and
+// is not reproducible here, so this pins the contract the binding provides: both
+// facts come back together, and they agree with the object actually at the path.
+func TestStampSnapshotPairsIdentityWithItsOwnBytes(t *testing.T) {
+	root := filepath.Join(t.TempDir(), "root")
+	if err := os.MkdirAll(root, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	const contents = "the stamp that belongs to this directory"
+	if err := os.WriteFile(windowsSandboxRuntimeStampPath(root), []byte(contents), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	identity, identified, prior, state, err := snapshotRuntimeStampBound(root)
+	if err != nil {
+		t.Fatalf("snapshot: %v", err)
+	}
+	existed := state == runtimeStampPresent
+	if !identified {
+		t.Fatal("the snapshot established no identity for a directory that exists")
+	}
+	if !existed || string(prior) != contents {
+		t.Fatalf("prior stamp = %q existed=%v, want %q", string(prior), existed, contents)
+	}
+	if direct, ok := runtimeDirIdentity(root); !ok || direct != identity {
+		t.Errorf("snapshot identity %q does not describe the directory at the path (%q)", identity, direct)
+	}
+}
+
+// An absent stamp still establishes the identity, because that came from the
+// directory handle and not from the stamp read.
+func TestStampSnapshotIdentifiesARootWithNoStamp(t *testing.T) {
+	root := filepath.Join(t.TempDir(), "root")
+	if err := os.MkdirAll(root, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	identity, identified, _, state, err := snapshotRuntimeStampBound(root)
+	if err != nil {
+		t.Fatalf("snapshot: %v", err)
+	}
+	existed := state == runtimeStampPresent
+	if !identified || identity == "" {
+		t.Error("a root with no stamp established no identity")
+	}
+	if existed {
+		t.Error("reported a prior stamp that does not exist")
+	}
+}
+
+// A created directory's identity must come from the creation, so the ledger
+// cannot record an object this run did not make.
+func TestCreatedDirectoryIdentityDescribesWhatWasCreated(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "created")
+
+	identity, identified, err := createRuntimeDirIdentified(path)
+	if err != nil {
+		t.Fatalf("create: %v", err)
+	}
+	if !identified || identity == "" {
+		t.Fatal("creation established no identity")
+	}
+	if info, statErr := os.Stat(path); statErr != nil || !info.IsDir() {
+		t.Fatalf("the directory was not created: %v", statErr)
+	}
+	if direct, ok := runtimeDirIdentity(path); !ok || direct != identity {
+		t.Errorf("creation identity %q does not describe the directory now at the path (%q)", identity, direct)
+	}
+
+	// Creating over something that exists is the caller's already-handled signal.
+	if _, _, again := createRuntimeDirIdentified(path); !os.IsExist(again) {
+		t.Errorf("creating over an existing directory returned %v, want an IsExist error", again)
+	}
+}