From 9adf57a1db9b8e1246b17d9b48b65b715c97cc9a Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Mon, 30 Mar 2026 09:37:10 -0400 Subject: [PATCH 1/9] feat(allowedpaths): skip nonexistent paths instead of failing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AllowedPaths is an allowlist — paths that don't exist at construction time are silently skipped. The sandbox operates with whatever paths are available. Tests: - TestNewSkipsNonexistentPaths: existing dir + nonexistent path - TestNewAllPathsNonexistent: all paths missing, empty sandbox - TestNewEmptyPaths: empty list, blocks all access - TestNewMixedExistingAndNonexistent: 3 paths, middle one missing - YAML scenario: nonexistent path skipped, existing dir works Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 27 ++++------ allowedpaths/sandbox_test.go | 54 +++++++++++++++++++ interp/allowed_paths_test.go | 16 +++--- .../nonexistent_path_skipped.yaml | 14 +++++ 4 files changed, 85 insertions(+), 26 deletions(-) create mode 100644 tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 904716ee..ad577f8f 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -36,31 +36,22 @@ type Sandbox struct { roots []root } -// New validates paths and eagerly opens os.Root handles so the -// allowed directories are pinned before the caller can modify them between -// construction and the first run. +// New creates a sandbox from an allowlist of directory paths. Paths that do +// not exist or cannot be opened are silently skipped — the sandbox operates +// with whatever paths are available at construction time. func New(paths []string) (*Sandbox, error) { - roots := make([]root, len(paths)) - for i, p := range paths { + roots := make([]root, 0, len(paths)) + for _, p := range paths { abs, err := filepath.Abs(p) if err != nil { - return nil, fmt.Errorf("AllowedPaths: cannot resolve %q: %w", p, err) + continue } r, err := os.OpenRoot(abs) if err != nil { - for _, prev := range roots[:i] { - if prev.root != nil { - prev.root.Close() - } - } - - info, statErr := os.Stat(abs) - if statErr == nil && !info.IsDir() { - return nil, fmt.Errorf("AllowedPaths: %q is not a directory", abs) - } - return nil, fmt.Errorf("AllowedPaths: cannot open root %q: %w", abs, err) + // Path does not exist or is not a directory — skip. + continue } - roots[i] = root{absPath: abs, root: r} + roots = append(roots, root{absPath: abs, root: r}) } return &Sandbox{roots: roots}, nil } diff --git a/allowedpaths/sandbox_test.go b/allowedpaths/sandbox_test.go index e36ccc14..34902ddc 100644 --- a/allowedpaths/sandbox_test.go +++ b/allowedpaths/sandbox_test.go @@ -306,3 +306,57 @@ func TestCollectDirEntries(t *testing.T) { assert.Equal(t, "cherry", entries[2].Name()) }) } + +func TestNewSkipsNonexistentPaths(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644)) + + sb, err := New([]string{"/nonexistent/path", dir}) + require.NoError(t, err, "nonexistent paths should be skipped") + defer sb.Close() + + // The existing directory should still be accessible. + f, err := sb.Open(filepath.Join(dir, "test.txt"), dir, os.O_RDONLY, 0) + require.NoError(t, err) + f.Close() +} + +func TestNewAllPathsNonexistent(t *testing.T) { + sb, err := New([]string{"/does/not/exist", "/also/missing"}) + require.NoError(t, err, "all-nonexistent paths should succeed with empty sandbox") + defer sb.Close() + + // Sandbox should block all access. + _, err = sb.Stat("/tmp", "/tmp") + assert.ErrorIs(t, err, os.ErrPermission) +} + +func TestNewEmptyPaths(t *testing.T) { + sb, err := New([]string{}) + require.NoError(t, err, "empty path list should succeed") + defer sb.Close() + + // Sandbox should block all access. + _, err = sb.Stat("/tmp", "/tmp") + assert.ErrorIs(t, err, os.ErrPermission) +} + +func TestNewMixedExistingAndNonexistent(t *testing.T) { + dirA := t.TempDir() + dirB := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dirA, "a.txt"), []byte("aaa"), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(dirB, "b.txt"), []byte("bbb"), 0644)) + + sb, err := New([]string{dirA, "/nonexistent", dirB}) + require.NoError(t, err, "nonexistent path between valid dirs should be skipped") + defer sb.Close() + + // Both existing directories should be accessible. + f, err := sb.Open(filepath.Join(dirA, "a.txt"), dirA, os.O_RDONLY, 0) + require.NoError(t, err, "first existing dir should work") + f.Close() + + f, err = sb.Open(filepath.Join(dirB, "b.txt"), dirB, os.O_RDONLY, 0) + require.NoError(t, err, "second existing dir should work") + f.Close() +} diff --git a/interp/allowed_paths_test.go b/interp/allowed_paths_test.go index 7a34d606..8106977b 100644 --- a/interp/allowed_paths_test.go +++ b/interp/allowed_paths_test.go @@ -57,23 +57,23 @@ func runScript(t *testing.T, script, dir string, opts ...interp.RunnerOption) (s } func TestAllowedPathsOption(t *testing.T) { - t.Run("invalid path rejected", func(t *testing.T) { - _, err := interp.New( + t.Run("nonexistent path skipped", func(t *testing.T) { + runner, err := interp.New( interp.AllowedPaths([]string{"/nonexistent/path/that/does/not/exist"}), ) - require.Error(t, err) - assert.Contains(t, err.Error(), "AllowedPaths") + require.NoError(t, err, "nonexistent paths should be skipped, not rejected") + runner.Close() }) - t.Run("file not directory rejected", func(t *testing.T) { + t.Run("file path skipped", func(t *testing.T) { tmpFile := filepath.Join(t.TempDir(), "file.txt") require.NoError(t, os.WriteFile(tmpFile, []byte("test"), 0644)) - _, err := interp.New( + runner, err := interp.New( interp.AllowedPaths([]string{tmpFile}), ) - require.Error(t, err) - assert.Contains(t, err.Error(), "not a directory") + require.NoError(t, err, "file paths should be skipped (not directories)") + runner.Close() }) t.Run("valid directory accepted", func(t *testing.T) { diff --git a/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml b/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml new file mode 100644 index 00000000..73f5206e --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml @@ -0,0 +1,14 @@ +description: Nonexistent paths in AllowedPaths are skipped, existing paths still work +skip_assert_against_bash: true +setup: + files: + - path: allowed/data.txt + content: "hello\n" +input: + allowed_paths: ["/nonexistent/path", "allowed"] + script: |+ + cat allowed/data.txt +expect: + stdout: "hello\n" + stderr: "" + exit_code: 0 From 3bf632ee187f3f6c1675afa7e91cfb0f1fa12fb2 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Mon, 30 Mar 2026 09:42:42 -0400 Subject: [PATCH 2/9] fix(test): remove unnecessary skip_assert_against_bash from scenario MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Success-path scenario (cat on existing file) produces identical output in both rshell and bash — no skip needed. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml b/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml index 73f5206e..1aa381dc 100644 --- a/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml +++ b/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml @@ -1,5 +1,4 @@ description: Nonexistent paths in AllowedPaths are skipped, existing paths still work -skip_assert_against_bash: true setup: files: - path: allowed/data.txt From d4f1e73cab58c809fc0567a8ed18c2727ae668ea Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Mon, 30 Mar 2026 09:46:46 -0400 Subject: [PATCH 3/9] fix: treat AllowedPaths as suggestions, use |+ block scalars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AllowedPaths are suggestions — if we can't open a path for any reason (missing, not a directory, no permission, etc.), skip it and work with whatever paths are available. Also use |+ block scalars in scenario YAML per AGENTS.md convention. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 4 +++- .../shell/allowed_paths/nonexistent_path_skipped.yaml | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index ad577f8f..8a17936d 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -48,7 +48,9 @@ func New(paths []string) (*Sandbox, error) { } r, err := os.OpenRoot(abs) if err != nil { - // Path does not exist or is not a directory — skip. + // AllowedPaths is a suggestion, not a requirement. If we can't + // open a path (missing, not a directory, no permission, etc.), + // skip it and work with whatever paths are available. continue } roots = append(roots, root{absPath: abs, root: r}) diff --git a/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml b/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml index 1aa381dc..31b1684e 100644 --- a/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml +++ b/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml @@ -8,6 +8,7 @@ input: script: |+ cat allowed/data.txt expect: - stdout: "hello\n" - stderr: "" + stdout: |+ + hello + stderr: |+ exit_code: 0 From ddb465b6194fadc1af46bf3fbe9addb342190935 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Mon, 30 Mar 2026 10:03:14 -0400 Subject: [PATCH 4/9] feat(allowedpaths): warn on stderr when skipping unavailable paths Write a warning to os.Stderr when a path cannot be opened, so operators can see which paths were dropped. Temporary approach until a proper logging mechanism is added. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 8a17936d..8f321eae 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -44,6 +44,7 @@ func New(paths []string) (*Sandbox, error) { for _, p := range paths { abs, err := filepath.Abs(p) if err != nil { + fmt.Fprintf(os.Stderr, "AllowedPaths: skipping %q: %v\n", p, err) continue } r, err := os.OpenRoot(abs) @@ -51,6 +52,7 @@ func New(paths []string) (*Sandbox, error) { // AllowedPaths is a suggestion, not a requirement. If we can't // open a path (missing, not a directory, no permission, etc.), // skip it and work with whatever paths are available. + fmt.Fprintf(os.Stderr, "AllowedPaths: skipping %q: %v\n", abs, err) continue } roots = append(roots, root{absPath: abs, root: r}) From 9d9386c937c091774c25accb7c7845864ef947ba Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Mon, 30 Mar 2026 10:07:07 -0400 Subject: [PATCH 5/9] fix(allowedsymbols): add fmt.Fprintf and os.Stderr to allowedpaths list Needed for the stderr warning when skipping unavailable paths in New(). Co-Authored-By: Claude Opus 4.6 (1M context) --- analysis/symbols_allowedpaths.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/analysis/symbols_allowedpaths.go b/analysis/symbols_allowedpaths.go index aac1dd5a..9ac60142 100644 --- a/analysis/symbols_allowedpaths.go +++ b/analysis/symbols_allowedpaths.go @@ -22,6 +22,7 @@ var allowedpathsAllowedSymbols = []string{ "errors.Is", // 🟢 error comparison; pure function, no I/O. "errors.New", // 🟢 creates a simple error value; pure function, no I/O. "fmt.Errorf", // 🟢 formatted error creation; pure function, no I/O. + "fmt.Fprintf", // 🟠 writes warning to os.Stderr when skipping unavailable paths. "io.EOF", // 🟢 sentinel error value; pure constant. "io.ReadWriteCloser", // 🟢 combined interface type; no side effects. "io/fs.DirEntry", // 🟢 interface type for directory entries; no side effects. @@ -33,6 +34,7 @@ var allowedpathsAllowedSymbols = []string{ "io/fs.ReadDirFile", // 🟢 read-only directory handle interface; no write capability. "os.DevNull", // 🟢 platform null device path constant; pure constant. "os.ErrPermission", // 🟢 sentinel error for permission denied; pure constant. + "os.Stderr", // 🟠 process stderr; used for warnings when skipping unavailable paths. "os.FileMode", // 🟢 file permission bits type; pure type. "os.Getgid", // 🟠 returns the numeric group id of the caller; read-only syscall. "os.Getgroups", // 🟠 returns supplementary group ids; read-only syscall. From ad6a97070dd2d77607cf4b35ae21cd447a4a19cd Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Mon, 30 Mar 2026 10:23:29 -0400 Subject: [PATCH 6/9] refactor(allowedpaths): route warnings through configured stderr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New() now accepts an io.Writer for warnings instead of writing to os.Stderr directly. The AllowedPaths RunnerOption passes r.stderr when available, falling back to os.Stderr if not yet configured. This respects the runner's I/O isolation — warnings go to the configured stderr stream, not the process-global one. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 12 +++++-- allowedpaths/sandbox_test.go | 14 ++++---- allowedpaths/sandbox_unix_test.go | 49 ++++++++++++++-------------- allowedpaths/sandbox_windows_test.go | 17 +++++----- analysis/symbols_allowedpaths.go | 1 + analysis/symbols_interp.go | 1 + interp/api.go | 8 ++++- 7 files changed, 59 insertions(+), 43 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 8f321eae..be14ccd2 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -39,12 +39,18 @@ type Sandbox struct { // New creates a sandbox from an allowlist of directory paths. Paths that do // not exist or cannot be opened are silently skipped — the sandbox operates // with whatever paths are available at construction time. -func New(paths []string) (*Sandbox, error) { +// +// warn receives diagnostic messages about skipped paths. If nil, warnings +// are written to os.Stderr. +func New(paths []string, warn io.Writer) (*Sandbox, error) { + if warn == nil { + warn = os.Stderr + } roots := make([]root, 0, len(paths)) for _, p := range paths { abs, err := filepath.Abs(p) if err != nil { - fmt.Fprintf(os.Stderr, "AllowedPaths: skipping %q: %v\n", p, err) + fmt.Fprintf(warn, "AllowedPaths: skipping %q: %v\n", p, err) continue } r, err := os.OpenRoot(abs) @@ -52,7 +58,7 @@ func New(paths []string) (*Sandbox, error) { // AllowedPaths is a suggestion, not a requirement. If we can't // open a path (missing, not a directory, no permission, etc.), // skip it and work with whatever paths are available. - fmt.Fprintf(os.Stderr, "AllowedPaths: skipping %q: %v\n", abs, err) + fmt.Fprintf(warn, "AllowedPaths: skipping %q: %v\n", abs, err) continue } roots = append(roots, root{absPath: abs, root: r}) diff --git a/allowedpaths/sandbox_test.go b/allowedpaths/sandbox_test.go index 34902ddc..8d606b6e 100644 --- a/allowedpaths/sandbox_test.go +++ b/allowedpaths/sandbox_test.go @@ -42,7 +42,7 @@ func TestSandboxOpenRejectsWriteFlags(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -74,7 +74,7 @@ func TestReadDirLimited(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, fmt.Sprintf("f%02d", i)), nil, 0644)) } - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -193,7 +193,7 @@ func TestReadDirNCapExceeded(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, fmt.Sprintf("f%02d", i)), nil, 0644)) } - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -311,7 +311,7 @@ func TestNewSkipsNonexistentPaths(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644)) - sb, err := New([]string{"/nonexistent/path", dir}) + sb, err := New([]string{"/nonexistent/path", dir}, io.Discard) require.NoError(t, err, "nonexistent paths should be skipped") defer sb.Close() @@ -322,7 +322,7 @@ func TestNewSkipsNonexistentPaths(t *testing.T) { } func TestNewAllPathsNonexistent(t *testing.T) { - sb, err := New([]string{"/does/not/exist", "/also/missing"}) + sb, err := New([]string{"/does/not/exist", "/also/missing"}, io.Discard) require.NoError(t, err, "all-nonexistent paths should succeed with empty sandbox") defer sb.Close() @@ -332,7 +332,7 @@ func TestNewAllPathsNonexistent(t *testing.T) { } func TestNewEmptyPaths(t *testing.T) { - sb, err := New([]string{}) + sb, err := New([]string{}, io.Discard) require.NoError(t, err, "empty path list should succeed") defer sb.Close() @@ -347,7 +347,7 @@ func TestNewMixedExistingAndNonexistent(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dirA, "a.txt"), []byte("aaa"), 0644)) require.NoError(t, os.WriteFile(filepath.Join(dirB, "b.txt"), []byte("bbb"), 0644)) - sb, err := New([]string{dirA, "/nonexistent", dirB}) + sb, err := New([]string{dirA, "/nonexistent", dirB}, io.Discard) require.NoError(t, err, "nonexistent path between valid dirs should be skipped") defer sb.Close() diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index 2d45e74c..4210503d 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -8,6 +8,7 @@ package allowedpaths import ( + "io" "os" "path/filepath" "syscall" @@ -26,7 +27,7 @@ func TestAccessFIFODoesNotBlock(t *testing.T) { fifoPath := filepath.Join(dir, "pipe") require.NoError(t, syscall.Mkfifo(fifoPath, 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -54,7 +55,7 @@ func TestAccessReadPermissionDenied(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "noread.txt"), []byte("secret"), 0200)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -72,7 +73,7 @@ func TestAccessWriteDenied(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readonly.txt"), []byte("data"), 0444)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -90,7 +91,7 @@ func TestAccessExecDenied(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "noexec.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -103,7 +104,7 @@ func TestAccessReadAllowed(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readable.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -115,7 +116,7 @@ func TestAccessWriteAllowed(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "writable.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -127,7 +128,7 @@ func TestAccessExecAllowed(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "script.sh"), []byte("#!/bin/sh"), 0755)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -138,7 +139,7 @@ func TestAccessExecAllowed(t *testing.T) { func TestAccessNonexistent(t *testing.T) { dir := t.TempDir() - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -153,7 +154,7 @@ func TestAccessOutsideSandbox(t *testing.T) { outside := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -166,7 +167,7 @@ func TestAccessDirectory(t *testing.T) { dir := t.TempDir() require.NoError(t, os.Mkdir(filepath.Join(dir, "subdir"), 0755)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -180,7 +181,7 @@ func TestAccessSymlinkWithinSandbox(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, "target.txt"), []byte("data"), 0644)) require.NoError(t, os.Symlink("target.txt", filepath.Join(dir, "link.txt"))) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -195,7 +196,7 @@ func TestAccessSymlinkEscapeBlocked(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) require.NoError(t, os.Symlink(filepath.Join(outside, "secret.txt"), filepath.Join(dir, "escape.txt"))) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -213,7 +214,7 @@ func TestAccessCombinedModes(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "rx.sh"), []byte("#!/bin/sh"), 0555)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -236,7 +237,7 @@ func TestAccessReadRegularFileOpenFile(t *testing.T) { } dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "writeonly.txt"), []byte("data"), 0200)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() assert.ErrorIs(t, sb.Access("writeonly.txt", dir, 0x04), os.ErrPermission) @@ -247,7 +248,7 @@ func TestAccessReadRegularFileOpenFile(t *testing.T) { func TestAccessReadRegularFileAllowed(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readable.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() assert.NoError(t, sb.Access("readable.txt", dir, 0x04)) @@ -259,7 +260,7 @@ func TestAccessReadRegularFileAllowed(t *testing.T) { func TestAccessFIFOReadFallsBackToModeBits(t *testing.T) { dir := t.TempDir() require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "pipe"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -281,7 +282,7 @@ func TestAccessFIFOReadDeniedModeBits(t *testing.T) { } dir := t.TempDir() require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "pipe"), 0200)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -300,7 +301,7 @@ func TestAccessFIFOReadDeniedModeBits(t *testing.T) { func TestAccessDirectoryReadUsesModeBits(t *testing.T) { dir := t.TempDir() require.NoError(t, os.Mkdir(filepath.Join(dir, "subdir"), 0755)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() assert.NoError(t, sb.Access("subdir", dir, 0x04)) @@ -314,7 +315,7 @@ func TestAccessDirectoryReadDenied(t *testing.T) { } dir := t.TempDir() require.NoError(t, os.Mkdir(filepath.Join(dir, "noread"), 0300)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() assert.ErrorIs(t, sb.Access("noread", dir, 0x04), os.ErrPermission) @@ -329,7 +330,7 @@ func TestAccessReadWriteCombined(t *testing.T) { dir := t.TempDir() // 0444 = readable but not writable require.NoError(t, os.WriteFile(filepath.Join(dir, "readonly.txt"), []byte("data"), 0444)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() // Read-only succeeds @@ -347,7 +348,7 @@ func TestAccessFdRelativeSymlink(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "target.txt"), []byte("data"), 0644)) require.NoError(t, os.Symlink("target.txt", filepath.Join(dir, "link.txt"))) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() assert.NoError(t, sb.Access("link.txt", dir, 0x04)) @@ -360,7 +361,7 @@ func TestAccessFdRelativeEscapeBlocked(t *testing.T) { outside := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) require.NoError(t, os.Symlink(filepath.Join(outside, "secret.txt"), filepath.Join(dir, "escape.txt"))) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() assert.Error(t, sb.Access("escape.txt", dir, 0x04)) @@ -381,7 +382,7 @@ func TestAccessSocketFallsBackToStat(t *testing.T) { defer syscall.Close(fd) require.NoError(t, syscall.Bind(fd, &syscall.SockaddrUnix{Name: sockPath})) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -397,7 +398,7 @@ func TestAccessFIFONonBlocking(t *testing.T) { dir := t.TempDir() require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "fifo"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() diff --git a/allowedpaths/sandbox_windows_test.go b/allowedpaths/sandbox_windows_test.go index 1b221b0b..b0ae8115 100644 --- a/allowedpaths/sandbox_windows_test.go +++ b/allowedpaths/sandbox_windows_test.go @@ -8,6 +8,7 @@ package allowedpaths import ( + "io" "os" "path/filepath" "testing" @@ -22,7 +23,7 @@ func TestAccessReadAllowedWindows(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readable.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -34,7 +35,7 @@ func TestAccessReadAllowedWindows(t *testing.T) { func TestAccessNonexistentWindows(t *testing.T) { dir := t.TempDir() - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -49,7 +50,7 @@ func TestAccessOutsideSandboxWindows(t *testing.T) { outside := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -63,7 +64,7 @@ func TestAccessDirectoryReadWindows(t *testing.T) { dir := t.TempDir() require.NoError(t, os.Mkdir(filepath.Join(dir, "subdir"), 0755)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -78,7 +79,7 @@ func TestAccessSymlinkEscapeBlockedWindows(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) require.NoError(t, os.Symlink(filepath.Join(outside, "secret.txt"), filepath.Join(dir, "escape.txt"))) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -93,7 +94,7 @@ func TestAccessSymlinkWithinSandboxWindows(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, "target.txt"), []byte("data"), 0644)) require.NoError(t, os.Symlink("target.txt", filepath.Join(dir, "link.txt"))) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -106,7 +107,7 @@ func TestAccessWriteDeniedWindows(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readonly.txt"), []byte("data"), 0444)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() @@ -119,7 +120,7 @@ func TestAccessExecAlwaysDeniedWindows(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}) + sb, err := New([]string{dir}, io.Discard) require.NoError(t, err) defer sb.Close() diff --git a/analysis/symbols_allowedpaths.go b/analysis/symbols_allowedpaths.go index 9ac60142..bf68a52d 100644 --- a/analysis/symbols_allowedpaths.go +++ b/analysis/symbols_allowedpaths.go @@ -25,6 +25,7 @@ var allowedpathsAllowedSymbols = []string{ "fmt.Fprintf", // 🟠 writes warning to os.Stderr when skipping unavailable paths. "io.EOF", // 🟢 sentinel error value; pure constant. "io.ReadWriteCloser", // 🟢 combined interface type; no side effects. + "io.Writer", // 🟢 interface type for warning output; no side effects. "io/fs.DirEntry", // 🟢 interface type for directory entries; no side effects. "io/fs.ErrExist", // 🟢 sentinel error for "already exists"; pure constant. "io/fs.ErrNotExist", // 🟢 sentinel error for "does not exist"; pure constant. diff --git a/analysis/symbols_interp.go b/analysis/symbols_interp.go index ffd2ff8d..55b480f2 100644 --- a/analysis/symbols_interp.go +++ b/analysis/symbols_interp.go @@ -47,6 +47,7 @@ var interpAllowedSymbols = []string{ "os.O_RDONLY", // 🟢 read-only file flag constant; pure constant. "os.PathError", // 🟢 error type wrapping path and operation; pure type. "os.Pipe", // 🟠 creates an OS pipe pair; needed for shell pipelines. + "os.Stderr", // 🟠 process stderr; fallback for AllowedPaths warnings when runner stderr is not yet configured. "path/filepath.IsAbs", // 🟢 checks if path is absolute; pure function, no I/O. "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. "runtime.GOOS", // 🟢 current OS name constant; pure constant, no I/O. diff --git a/interp/api.go b/interp/api.go index 94e10489..2ea1f350 100644 --- a/interp/api.go +++ b/interp/api.go @@ -549,7 +549,13 @@ func (r *Runner) Close() error { // An empty slice also blocks all file access. func AllowedPaths(paths []string) RunnerOption { return func(r *Runner) error { - sb, err := allowedpaths.New(paths) + // Use the runner's configured stderr for warnings if available, + // otherwise fall back to os.Stderr. + warn := r.stderr + if warn == nil { + warn = os.Stderr + } + sb, err := allowedpaths.New(paths, warn) if err != nil { return err } From 85e090213aa22d2c9fe6b754bcb4637898ff1ca1 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Mon, 30 Mar 2026 10:34:07 -0400 Subject: [PATCH 7/9] fix(test): use relative path so scenario exercises AllowedPaths skip logic The test harness filters out non-existent absolute paths via os.Stat before passing them to interp.AllowedPaths. Switch to a relative path ("nonexistent_dir") which the harness resolves to a temp-dir child that genuinely doesn't exist, ensuring the skip logic in allowedpaths.New is actually exercised. Also expect the stderr warning and mark skip_assert_against_bash since this is rshell-specific. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../shell/allowed_paths/nonexistent_path_skipped.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml b/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml index 31b1684e..4bbbec88 100644 --- a/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml +++ b/tests/scenarios/shell/allowed_paths/nonexistent_path_skipped.yaml @@ -1,14 +1,17 @@ description: Nonexistent paths in AllowedPaths are skipped, existing paths still work +# skip: AllowedPaths skip logic is an rshell-specific feature not present in bash +skip_assert_against_bash: true setup: files: - path: allowed/data.txt content: "hello\n" input: - allowed_paths: ["/nonexistent/path", "allowed"] + allowed_paths: ["nonexistent_dir", "allowed"] script: |+ cat allowed/data.txt expect: stdout: |+ hello - stderr: |+ + stderr_contains: + - "AllowedPaths: skipping" exit_code: 0 From 6f03767c63c7b74b2e3050cbc4176fb17f55ae7d Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Mon, 30 Mar 2026 10:42:51 -0400 Subject: [PATCH 8/9] refactor(allowedpaths): defer warning output until stderr is configured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit allowedpaths.New no longer accepts an io.Writer for warnings. Instead it collects diagnostics into a byte buffer and returns them alongside the sandbox. The AllowedPaths RunnerOption stores the warnings on the runner, and interp.New flushes them to r.stderr after all options have been applied and defaults set. This makes warning output independent of option ordering — callers can pass AllowedPaths before or after StdIO and warnings always reach the configured stderr. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 17 +++++----- allowedpaths/sandbox_test.go | 14 ++++---- allowedpaths/sandbox_unix_test.go | 49 ++++++++++++++-------------- allowedpaths/sandbox_windows_test.go | 17 +++++----- analysis/symbols_allowedpaths.go | 5 ++- analysis/symbols_interp.go | 1 - interp/api.go | 19 +++++++---- 7 files changed, 61 insertions(+), 61 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index be14ccd2..80e410e6 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -8,6 +8,7 @@ package allowedpaths import ( + "bytes" "errors" "fmt" "io" @@ -40,17 +41,15 @@ type Sandbox struct { // not exist or cannot be opened are silently skipped — the sandbox operates // with whatever paths are available at construction time. // -// warn receives diagnostic messages about skipped paths. If nil, warnings -// are written to os.Stderr. -func New(paths []string, warn io.Writer) (*Sandbox, error) { - if warn == nil { - warn = os.Stderr - } +// Diagnostic messages about skipped paths are collected into warnings. The +// caller is responsible for writing them to the appropriate output stream. +func New(paths []string) (*Sandbox, []byte, error) { + var warnings bytes.Buffer roots := make([]root, 0, len(paths)) for _, p := range paths { abs, err := filepath.Abs(p) if err != nil { - fmt.Fprintf(warn, "AllowedPaths: skipping %q: %v\n", p, err) + fmt.Fprintf(&warnings, "AllowedPaths: skipping %q: %v\n", p, err) continue } r, err := os.OpenRoot(abs) @@ -58,12 +57,12 @@ func New(paths []string, warn io.Writer) (*Sandbox, error) { // AllowedPaths is a suggestion, not a requirement. If we can't // open a path (missing, not a directory, no permission, etc.), // skip it and work with whatever paths are available. - fmt.Fprintf(warn, "AllowedPaths: skipping %q: %v\n", abs, err) + fmt.Fprintf(&warnings, "AllowedPaths: skipping %q: %v\n", abs, err) continue } roots = append(roots, root{absPath: abs, root: r}) } - return &Sandbox{roots: roots}, nil + return &Sandbox{roots: roots}, warnings.Bytes(), nil } // resolve returns the matching os.Root and the path relative to it for the diff --git a/allowedpaths/sandbox_test.go b/allowedpaths/sandbox_test.go index 8d606b6e..5e52340c 100644 --- a/allowedpaths/sandbox_test.go +++ b/allowedpaths/sandbox_test.go @@ -42,7 +42,7 @@ func TestSandboxOpenRejectsWriteFlags(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -74,7 +74,7 @@ func TestReadDirLimited(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, fmt.Sprintf("f%02d", i)), nil, 0644)) } - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -193,7 +193,7 @@ func TestReadDirNCapExceeded(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, fmt.Sprintf("f%02d", i)), nil, 0644)) } - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -311,7 +311,7 @@ func TestNewSkipsNonexistentPaths(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644)) - sb, err := New([]string{"/nonexistent/path", dir}, io.Discard) + sb, _, err := New([]string{"/nonexistent/path", dir}) require.NoError(t, err, "nonexistent paths should be skipped") defer sb.Close() @@ -322,7 +322,7 @@ func TestNewSkipsNonexistentPaths(t *testing.T) { } func TestNewAllPathsNonexistent(t *testing.T) { - sb, err := New([]string{"/does/not/exist", "/also/missing"}, io.Discard) + sb, _, err := New([]string{"/does/not/exist", "/also/missing"}) require.NoError(t, err, "all-nonexistent paths should succeed with empty sandbox") defer sb.Close() @@ -332,7 +332,7 @@ func TestNewAllPathsNonexistent(t *testing.T) { } func TestNewEmptyPaths(t *testing.T) { - sb, err := New([]string{}, io.Discard) + sb, _, err := New([]string{}) require.NoError(t, err, "empty path list should succeed") defer sb.Close() @@ -347,7 +347,7 @@ func TestNewMixedExistingAndNonexistent(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dirA, "a.txt"), []byte("aaa"), 0644)) require.NoError(t, os.WriteFile(filepath.Join(dirB, "b.txt"), []byte("bbb"), 0644)) - sb, err := New([]string{dirA, "/nonexistent", dirB}, io.Discard) + sb, _, err := New([]string{dirA, "/nonexistent", dirB}) require.NoError(t, err, "nonexistent path between valid dirs should be skipped") defer sb.Close() diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index 4210503d..fdd1a6b3 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -8,7 +8,6 @@ package allowedpaths import ( - "io" "os" "path/filepath" "syscall" @@ -27,7 +26,7 @@ func TestAccessFIFODoesNotBlock(t *testing.T) { fifoPath := filepath.Join(dir, "pipe") require.NoError(t, syscall.Mkfifo(fifoPath, 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -55,7 +54,7 @@ func TestAccessReadPermissionDenied(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "noread.txt"), []byte("secret"), 0200)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -73,7 +72,7 @@ func TestAccessWriteDenied(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readonly.txt"), []byte("data"), 0444)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -91,7 +90,7 @@ func TestAccessExecDenied(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "noexec.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -104,7 +103,7 @@ func TestAccessReadAllowed(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readable.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -116,7 +115,7 @@ func TestAccessWriteAllowed(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "writable.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -128,7 +127,7 @@ func TestAccessExecAllowed(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "script.sh"), []byte("#!/bin/sh"), 0755)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -139,7 +138,7 @@ func TestAccessExecAllowed(t *testing.T) { func TestAccessNonexistent(t *testing.T) { dir := t.TempDir() - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -154,7 +153,7 @@ func TestAccessOutsideSandbox(t *testing.T) { outside := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -167,7 +166,7 @@ func TestAccessDirectory(t *testing.T) { dir := t.TempDir() require.NoError(t, os.Mkdir(filepath.Join(dir, "subdir"), 0755)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -181,7 +180,7 @@ func TestAccessSymlinkWithinSandbox(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, "target.txt"), []byte("data"), 0644)) require.NoError(t, os.Symlink("target.txt", filepath.Join(dir, "link.txt"))) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -196,7 +195,7 @@ func TestAccessSymlinkEscapeBlocked(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) require.NoError(t, os.Symlink(filepath.Join(outside, "secret.txt"), filepath.Join(dir, "escape.txt"))) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -214,7 +213,7 @@ func TestAccessCombinedModes(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "rx.sh"), []byte("#!/bin/sh"), 0555)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -237,7 +236,7 @@ func TestAccessReadRegularFileOpenFile(t *testing.T) { } dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "writeonly.txt"), []byte("data"), 0200)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() assert.ErrorIs(t, sb.Access("writeonly.txt", dir, 0x04), os.ErrPermission) @@ -248,7 +247,7 @@ func TestAccessReadRegularFileOpenFile(t *testing.T) { func TestAccessReadRegularFileAllowed(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readable.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() assert.NoError(t, sb.Access("readable.txt", dir, 0x04)) @@ -260,7 +259,7 @@ func TestAccessReadRegularFileAllowed(t *testing.T) { func TestAccessFIFOReadFallsBackToModeBits(t *testing.T) { dir := t.TempDir() require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "pipe"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -282,7 +281,7 @@ func TestAccessFIFOReadDeniedModeBits(t *testing.T) { } dir := t.TempDir() require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "pipe"), 0200)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -301,7 +300,7 @@ func TestAccessFIFOReadDeniedModeBits(t *testing.T) { func TestAccessDirectoryReadUsesModeBits(t *testing.T) { dir := t.TempDir() require.NoError(t, os.Mkdir(filepath.Join(dir, "subdir"), 0755)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() assert.NoError(t, sb.Access("subdir", dir, 0x04)) @@ -315,7 +314,7 @@ func TestAccessDirectoryReadDenied(t *testing.T) { } dir := t.TempDir() require.NoError(t, os.Mkdir(filepath.Join(dir, "noread"), 0300)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() assert.ErrorIs(t, sb.Access("noread", dir, 0x04), os.ErrPermission) @@ -330,7 +329,7 @@ func TestAccessReadWriteCombined(t *testing.T) { dir := t.TempDir() // 0444 = readable but not writable require.NoError(t, os.WriteFile(filepath.Join(dir, "readonly.txt"), []byte("data"), 0444)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() // Read-only succeeds @@ -348,7 +347,7 @@ func TestAccessFdRelativeSymlink(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "target.txt"), []byte("data"), 0644)) require.NoError(t, os.Symlink("target.txt", filepath.Join(dir, "link.txt"))) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() assert.NoError(t, sb.Access("link.txt", dir, 0x04)) @@ -361,7 +360,7 @@ func TestAccessFdRelativeEscapeBlocked(t *testing.T) { outside := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) require.NoError(t, os.Symlink(filepath.Join(outside, "secret.txt"), filepath.Join(dir, "escape.txt"))) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() assert.Error(t, sb.Access("escape.txt", dir, 0x04)) @@ -382,7 +381,7 @@ func TestAccessSocketFallsBackToStat(t *testing.T) { defer syscall.Close(fd) require.NoError(t, syscall.Bind(fd, &syscall.SockaddrUnix{Name: sockPath})) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -398,7 +397,7 @@ func TestAccessFIFONonBlocking(t *testing.T) { dir := t.TempDir() require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "fifo"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() diff --git a/allowedpaths/sandbox_windows_test.go b/allowedpaths/sandbox_windows_test.go index b0ae8115..317281b9 100644 --- a/allowedpaths/sandbox_windows_test.go +++ b/allowedpaths/sandbox_windows_test.go @@ -8,7 +8,6 @@ package allowedpaths import ( - "io" "os" "path/filepath" "testing" @@ -23,7 +22,7 @@ func TestAccessReadAllowedWindows(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readable.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -35,7 +34,7 @@ func TestAccessReadAllowedWindows(t *testing.T) { func TestAccessNonexistentWindows(t *testing.T) { dir := t.TempDir() - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -50,7 +49,7 @@ func TestAccessOutsideSandboxWindows(t *testing.T) { outside := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -64,7 +63,7 @@ func TestAccessDirectoryReadWindows(t *testing.T) { dir := t.TempDir() require.NoError(t, os.Mkdir(filepath.Join(dir, "subdir"), 0755)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -79,7 +78,7 @@ func TestAccessSymlinkEscapeBlockedWindows(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) require.NoError(t, os.Symlink(filepath.Join(outside, "secret.txt"), filepath.Join(dir, "escape.txt"))) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -94,7 +93,7 @@ func TestAccessSymlinkWithinSandboxWindows(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, "target.txt"), []byte("data"), 0644)) require.NoError(t, os.Symlink("target.txt", filepath.Join(dir, "link.txt"))) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -107,7 +106,7 @@ func TestAccessWriteDeniedWindows(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "readonly.txt"), []byte("data"), 0444)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() @@ -120,7 +119,7 @@ func TestAccessExecAlwaysDeniedWindows(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("data"), 0644)) - sb, err := New([]string{dir}, io.Discard) + sb, _, err := New([]string{dir}) require.NoError(t, err) defer sb.Close() diff --git a/analysis/symbols_allowedpaths.go b/analysis/symbols_allowedpaths.go index bf68a52d..c129e39d 100644 --- a/analysis/symbols_allowedpaths.go +++ b/analysis/symbols_allowedpaths.go @@ -17,15 +17,15 @@ package analysis // // The permanently banned packages (reflect, unsafe) apply here too. var allowedpathsAllowedSymbols = []string{ + "bytes.Buffer", // 🟢 in-memory byte buffer; collects sandbox warnings for deferred output. "context.Context", // 🟢 context type used to signal cancellation; no I/O or side effects. "errors.As", // 🟢 error type assertion; pure function, no I/O. "errors.Is", // 🟢 error comparison; pure function, no I/O. "errors.New", // 🟢 creates a simple error value; pure function, no I/O. "fmt.Errorf", // 🟢 formatted error creation; pure function, no I/O. - "fmt.Fprintf", // 🟠 writes warning to os.Stderr when skipping unavailable paths. + "fmt.Fprintf", // 🟠 writes warning messages to in-memory buffer during sandbox construction. "io.EOF", // 🟢 sentinel error value; pure constant. "io.ReadWriteCloser", // 🟢 combined interface type; no side effects. - "io.Writer", // 🟢 interface type for warning output; no side effects. "io/fs.DirEntry", // 🟢 interface type for directory entries; no side effects. "io/fs.ErrExist", // 🟢 sentinel error for "already exists"; pure constant. "io/fs.ErrNotExist", // 🟢 sentinel error for "does not exist"; pure constant. @@ -35,7 +35,6 @@ var allowedpathsAllowedSymbols = []string{ "io/fs.ReadDirFile", // 🟢 read-only directory handle interface; no write capability. "os.DevNull", // 🟢 platform null device path constant; pure constant. "os.ErrPermission", // 🟢 sentinel error for permission denied; pure constant. - "os.Stderr", // 🟠 process stderr; used for warnings when skipping unavailable paths. "os.FileMode", // 🟢 file permission bits type; pure type. "os.Getgid", // 🟠 returns the numeric group id of the caller; read-only syscall. "os.Getgroups", // 🟠 returns supplementary group ids; read-only syscall. diff --git a/analysis/symbols_interp.go b/analysis/symbols_interp.go index 55b480f2..ffd2ff8d 100644 --- a/analysis/symbols_interp.go +++ b/analysis/symbols_interp.go @@ -47,7 +47,6 @@ var interpAllowedSymbols = []string{ "os.O_RDONLY", // 🟢 read-only file flag constant; pure constant. "os.PathError", // 🟢 error type wrapping path and operation; pure type. "os.Pipe", // 🟠 creates an OS pipe pair; needed for shell pipelines. - "os.Stderr", // 🟠 process stderr; fallback for AllowedPaths warnings when runner stderr is not yet configured. "path/filepath.IsAbs", // 🟢 checks if path is absolute; pure function, no I/O. "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. "runtime.GOOS", // 🟢 current OS name constant; pure constant, no I/O. diff --git a/interp/api.go b/interp/api.go index 2ea1f350..0f2bc50d 100644 --- a/interp/api.go +++ b/interp/api.go @@ -51,6 +51,11 @@ type runnerConfig struct { // nil (default) blocks all file access; populate via AllowedPaths option. sandbox *allowedpaths.Sandbox + // sandboxWarnings holds diagnostic messages about skipped AllowedPaths + // entries. Written to stderr after all options are applied and defaults + // are set, so the output target is independent of option ordering. + sandboxWarnings []byte + // allowedCommands is the set of command names (builtins or external) that // the interpreter is permitted to execute. If nil and allowAllCommands is // false, no commands are allowed. @@ -235,6 +240,11 @@ func New(opts ...RunnerOption) (*Runner, error) { if r.stdout == nil || r.stderr == nil { StdIO(r.stdin, r.stdout, r.stderr)(r) } + // Flush any sandbox warnings now that stderr is guaranteed to be set. + if len(r.sandboxWarnings) > 0 { + r.stderr.Write(r.sandboxWarnings) + r.sandboxWarnings = nil + } r.proc = builtins.NewProcProvider(r.procPath) return r, nil } @@ -549,17 +559,12 @@ func (r *Runner) Close() error { // An empty slice also blocks all file access. func AllowedPaths(paths []string) RunnerOption { return func(r *Runner) error { - // Use the runner's configured stderr for warnings if available, - // otherwise fall back to os.Stderr. - warn := r.stderr - if warn == nil { - warn = os.Stderr - } - sb, err := allowedpaths.New(paths, warn) + sb, warnings, err := allowedpaths.New(paths) if err != nil { return err } r.sandbox = sb + r.sandboxWarnings = warnings return nil } } From 217735d34ea2696919440567b2a940bc3fb0d0b7 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Mon, 30 Mar 2026 10:44:51 -0400 Subject: [PATCH 9/9] style(allowedpaths): name return values in New for clarity Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 80e410e6..f83c2b8e 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -43,13 +43,13 @@ type Sandbox struct { // // Diagnostic messages about skipped paths are collected into warnings. The // caller is responsible for writing them to the appropriate output stream. -func New(paths []string) (*Sandbox, []byte, error) { - var warnings bytes.Buffer +func New(paths []string) (sb *Sandbox, warnings []byte, err error) { + var buf bytes.Buffer roots := make([]root, 0, len(paths)) for _, p := range paths { abs, err := filepath.Abs(p) if err != nil { - fmt.Fprintf(&warnings, "AllowedPaths: skipping %q: %v\n", p, err) + fmt.Fprintf(&buf, "AllowedPaths: skipping %q: %v\n", p, err) continue } r, err := os.OpenRoot(abs) @@ -57,12 +57,12 @@ func New(paths []string) (*Sandbox, []byte, error) { // AllowedPaths is a suggestion, not a requirement. If we can't // open a path (missing, not a directory, no permission, etc.), // skip it and work with whatever paths are available. - fmt.Fprintf(&warnings, "AllowedPaths: skipping %q: %v\n", abs, err) + fmt.Fprintf(&buf, "AllowedPaths: skipping %q: %v\n", abs, err) continue } roots = append(roots, root{absPath: abs, root: r}) } - return &Sandbox{roots: roots}, warnings.Bytes(), nil + return &Sandbox{roots: roots}, buf.Bytes(), nil } // resolve returns the matching os.Root and the path relative to it for the