From 0ed8c09eaecbdac005146fb4250cb37bc9a88018 Mon Sep 17 00:00:00 2001 From: Fabio Leitao Date: Wed, 14 Oct 2026 15:29:39 -0300 Subject: [PATCH 1/2] test(cli): pin sandbox policy golden temp roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields compared the policy snapshot to a fixed golden after stripping only the temp roots NewScope keeps. NewScope drops a candidate that sits inside an earlier one, while the JSON (built with a nil scope) still lists it, so a TMPDIR inside /tmp — or TMP inside TEMP on Windows — survived normalization and failed the golden. Pin the variables that path reads (TMPDIR on Unix, TEMP and TMP on Windows) and cover a nested temp dir that is not inside the earlier candidate, so the normalizer removes it instead of leaving it in the JSON. Refs #1121 --- internal/cli/sandbox_test.go | 105 ++++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 2 deletions(-) diff --git a/internal/cli/sandbox_test.go b/internal/cli/sandbox_test.go index 53ffd7b4d..adfb1438e 100644 --- a/internal/cli/sandbox_test.go +++ b/internal/cli/sandbox_test.go @@ -485,6 +485,103 @@ func TestTUISandboxSetupCommandGatedToWindowsNativeBackend(t *testing.T) { } func TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields(t *testing.T) { + // Pin before any t.TempDir so an ambient temp dir cannot reach the snapshot. + // Unix reads TMPDIR (and always considers /tmp). Windows reads TEMP and TMP. + // XDG_RUNTIME_DIR is not a temp write-root candidate. + pinSandboxPolicyGoldenTempEnv(t) + assertSandboxPolicyJSONGolden(t, "") +} + +// TestRunSandboxPolicyJSONGoldenNestedTempDir runs the same golden with a temp +// directory that sits inside another one. +// +// NewScope drops a root covered by an earlier one. The policy JSON is built +// with a nil scope, so it still lists every candidate, and the golden +// normalizer only removes what NewScope reports. A TMPDIR inside /tmp (or a +// TMP inside TEMP) therefore survives normalization. This test pins the +// variables the snapshot reads: on Unix TMPDIR is the inner directory, and +// only when that directory is not inside the hard-coded /tmp, so NewScope +// keeps both roots and the normalizer strips both; on Windows TEMP and TMP +// name the same outer directory, because pointing TMP at the inner one is +// the leak. The inner path must not remain in the normalized JSON. +func TestRunSandboxPolicyJSONGoldenNestedTempDir(t *testing.T) { + outer := sandboxGoldenTempDirOutside(t, ".") + inner := filepath.Join(outer, "inner") + if err := os.MkdirAll(inner, 0o755); err != nil { + t.Fatalf("MkdirAll(%q): %v", inner, err) + } + pinned := outer + switch runtime.GOOS { + case "windows": + t.Setenv("TEMP", outer) + t.Setenv("TMP", outer) + default: + if sandboxPathInside("/tmp", inner) { + t.Fatalf("nested TMPDIR %q sits inside /tmp; NewScope would drop it and the golden would mismatch", inner) + } + t.Setenv("TMPDIR", inner) + pinned = inner + } + normalized := assertSandboxPolicyJSONGolden(t, pinned) + for _, leaked := range []string{outer, inner} { + if strings.Contains(normalized, leaked) || strings.Contains(normalized, filepath.ToSlash(leaked)) { + t.Fatalf("normalized policy leaked temp root %q:\n%s", leaked, normalized) + } + } +} + +// pinSandboxPolicyGoldenTempEnv fixes the temp variables +// defaultTempWriteRootCandidatesForGOOS reads so the golden does not depend on +// the process environment. An empty TMPDIR leaves the Unix hard-coded /tmp as +// the only candidate. TEMP and TMP name one directory so neither is nested in +// the other. +func pinSandboxPolicyGoldenTempEnv(t *testing.T) { + t.Helper() + if runtime.GOOS == "windows" { + dir := t.TempDir() + t.Setenv("TEMP", dir) + t.Setenv("TMP", dir) + return + } + t.Setenv("TMPDIR", "") +} + +func sandboxGoldenTempDirOutside(t *testing.T, parent string) string { + t.Helper() + dir, err := os.MkdirTemp(parent, ".zero-sandbox-golden-") + if err != nil { + t.Fatalf("MkdirTemp(%q): %v", parent, err) + } + t.Cleanup(func() { _ = os.RemoveAll(dir) }) + abs, err := filepath.Abs(dir) + if err != nil { + t.Fatalf("Abs(%q): %v", dir, err) + } + resolved, err := filepath.EvalSymlinks(abs) + if err != nil { + t.Fatalf("EvalSymlinks(%q): %v", abs, err) + } + return resolved +} + +func sandboxPathInside(parent string, child string) bool { + resolvedParent := parent + if resolved, err := filepath.EvalSymlinks(parent); err == nil { + resolvedParent = resolved + } + resolvedChild := child + if resolved, err := filepath.EvalSymlinks(child); err == nil { + resolvedChild = resolved + } + relative, err := filepath.Rel(resolvedParent, resolvedChild) + if err != nil { + return false + } + return relative == "." || (relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) && !filepath.IsAbs(relative)) +} + +func assertSandboxPolicyJSONGolden(t *testing.T, mustList string) string { + t.Helper() // Point all user-config roots at an empty directory so the platform-specific // credential deny baseline can be asserted without leaking host paths into // the platform-neutral golden comparison. @@ -526,8 +623,8 @@ func TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields(t *testing.T) { t.Fatalf("policy exit = %d, stderr %q", exitCode, stderr.String()) } - got := stdout.String() - got = replacePathToken(got, workspace, "$WORKSPACE") + raw := stdout.String() + got := replacePathToken(raw, workspace, "$WORKSPACE") got = replacePathToken(got, store.FilePath(), "$GRANTS") gotBytes := normalizeSandboxPolicyGoldenTempRoots(t, []byte(got), workspace, emptyHome) wantBytes, err := os.ReadFile(filepath.Join("testdata", "sandbox_policy_windows_unavailable.golden.json")) @@ -537,6 +634,10 @@ func TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields(t *testing.T) { if !jsonValuesEqual(t, wantBytes, gotBytes) { t.Fatalf("policy JSON golden mismatch\nwant:\n%s\ngot:\n%s", string(wantBytes), string(gotBytes)) } + if mustList != "" && !strings.Contains(raw, mustList) { + t.Fatalf("policy JSON did not list pinned temp root %q\n%s", mustList, raw) + } + return string(gotBytes) } func normalizeSandboxPolicyGoldenTempRoots(t *testing.T, gotBytes []byte, workspace string, emptyHome string) []byte { From 790f3bbddf58428fb7d7597a95a57f232d5f91a3 Mon Sep 17 00:00:00 2001 From: Fabio Leitao Date: Wed, 14 Oct 2026 15:50:26 -0300 Subject: [PATCH 2/2] test(cli): run the golden against an ambient nested temp dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nested golden created its temp dir in the package working directory and failed when that path sat inside /tmp, so a checkout under /tmp could not run it and the test wrote into the source tree. Stage the issue's shape first — TMPDIR inside /tmp, or TMP inside TEMP — then let the pin override those variables. Directories come from t.TempDir. If the nested shape cannot be built, skip. Refs #1121 --- internal/cli/sandbox_test.go | 117 +++++++++++++++-------------------- 1 file changed, 51 insertions(+), 66 deletions(-) diff --git a/internal/cli/sandbox_test.go b/internal/cli/sandbox_test.go index adfb1438e..e82da4563 100644 --- a/internal/cli/sandbox_test.go +++ b/internal/cli/sandbox_test.go @@ -492,42 +492,61 @@ func TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields(t *testing.T) { assertSandboxPolicyJSONGolden(t, "") } -// TestRunSandboxPolicyJSONGoldenNestedTempDir runs the same golden with a temp -// directory that sits inside another one. -// -// NewScope drops a root covered by an earlier one. The policy JSON is built -// with a nil scope, so it still lists every candidate, and the golden -// normalizer only removes what NewScope reports. A TMPDIR inside /tmp (or a -// TMP inside TEMP) therefore survives normalization. This test pins the -// variables the snapshot reads: on Unix TMPDIR is the inner directory, and -// only when that directory is not inside the hard-coded /tmp, so NewScope -// keeps both roots and the normalizer strips both; on Windows TEMP and TMP -// name the same outer directory, because pointing TMP at the inner one is -// the leak. The inner path must not remain in the normalized JSON. +// TestRunSandboxPolicyJSONGoldenNestedTempDir stages the temp-dir shape from +// #1121 and then pins over it. On Unix that shape is a TMPDIR inside /tmp; on +// Windows it is TMP inside TEMP. NewScope drops the inner root, the policy +// JSON still lists it, and the golden normalizer leaves it behind. The pin +// replaces those variables before the snapshot, so the golden matches and the +// nested path does not survive. The directories come from t.TempDir, never +// from the package working directory, so a checkout under /tmp is not special. func TestRunSandboxPolicyJSONGoldenNestedTempDir(t *testing.T) { - outer := sandboxGoldenTempDirOutside(t, ".") - inner := filepath.Join(outer, "inner") - if err := os.MkdirAll(inner, 0o755); err != nil { - t.Fatalf("MkdirAll(%q): %v", inner, err) - } - pinned := outer - switch runtime.GOOS { - case "windows": - t.Setenv("TEMP", outer) - t.Setenv("TMP", outer) - default: - if sandboxPathInside("/tmp", inner) { - t.Fatalf("nested TMPDIR %q sits inside /tmp; NewScope would drop it and the golden would mismatch", inner) - } - t.Setenv("TMPDIR", inner) - pinned = inner + nested := stageNestedSandboxTempEnv(t) + pinSandboxPolicyGoldenTempEnv(t) + normalized := assertSandboxPolicyJSONGolden(t, "") + if strings.Contains(normalized, nested) || strings.Contains(normalized, filepath.ToSlash(nested)) { + t.Fatalf("normalized policy leaked nested temp root %q:\n%s", nested, normalized) } - normalized := assertSandboxPolicyJSONGolden(t, pinned) - for _, leaked := range []string{outer, inner} { - if strings.Contains(normalized, leaked) || strings.Contains(normalized, filepath.ToSlash(leaked)) { - t.Fatalf("normalized policy leaked temp root %q:\n%s", leaked, normalized) +} + +// stageNestedSandboxTempEnv installs the leaking temp-dir shape and returns +// the inner path. The caller pins afterward. Skip, rather than fail, when +// that shape cannot be built. +func stageNestedSandboxTempEnv(t *testing.T) string { + t.Helper() + if runtime.GOOS == "windows" { + outer := t.TempDir() + inner := filepath.Join(outer, "inner") + if err := os.MkdirAll(inner, 0o755); err != nil { + t.Skipf("create nested TEMP/TMP: %v", err) } + t.Setenv("TEMP", outer) + t.Setenv("TMP", inner) + return inner + } + resolvedTmp, err := filepath.EvalSymlinks("/tmp") + if err != nil { + t.Skipf("resolve /tmp: %v", err) + } + info, err := os.Stat(resolvedTmp) + if err != nil || !info.IsDir() { + t.Skip("/tmp is not a directory; cannot stage a nested TMPDIR") + } + // t.TempDir is a child of os.TempDir(). A further child is the ambient + // case: TMPDIR inside the hard-coded /tmp candidate. + nested := filepath.Join(t.TempDir(), "nested") + if err := os.MkdirAll(nested, 0o755); err != nil { + t.Skipf("create nested TMPDIR: %v", err) + } + resolvedNested, err := filepath.EvalSymlinks(nested) + if err != nil { + t.Skipf("resolve nested TMPDIR: %v", err) + } + rel, err := filepath.Rel(resolvedTmp, resolvedNested) + if err != nil || rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { + t.Skipf("temp dir %q is not inside /tmp; cannot stage the nested TMPDIR case", resolvedNested) } + t.Setenv("TMPDIR", resolvedNested) + return resolvedNested } // pinSandboxPolicyGoldenTempEnv fixes the temp variables @@ -546,40 +565,6 @@ func pinSandboxPolicyGoldenTempEnv(t *testing.T) { t.Setenv("TMPDIR", "") } -func sandboxGoldenTempDirOutside(t *testing.T, parent string) string { - t.Helper() - dir, err := os.MkdirTemp(parent, ".zero-sandbox-golden-") - if err != nil { - t.Fatalf("MkdirTemp(%q): %v", parent, err) - } - t.Cleanup(func() { _ = os.RemoveAll(dir) }) - abs, err := filepath.Abs(dir) - if err != nil { - t.Fatalf("Abs(%q): %v", dir, err) - } - resolved, err := filepath.EvalSymlinks(abs) - if err != nil { - t.Fatalf("EvalSymlinks(%q): %v", abs, err) - } - return resolved -} - -func sandboxPathInside(parent string, child string) bool { - resolvedParent := parent - if resolved, err := filepath.EvalSymlinks(parent); err == nil { - resolvedParent = resolved - } - resolvedChild := child - if resolved, err := filepath.EvalSymlinks(child); err == nil { - resolvedChild = resolved - } - relative, err := filepath.Rel(resolvedParent, resolvedChild) - if err != nil { - return false - } - return relative == "." || (relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) && !filepath.IsAbs(relative)) -} - func assertSandboxPolicyJSONGolden(t *testing.T, mustList string) string { t.Helper() // Point all user-config roots at an empty directory so the platform-specific