Skip to content

feat(allowedpaths): skip nonexistent paths instead of failing #164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matt-dz merged 9 commits into from
Mar 30, 2026
Merged

feat(allowedpaths): skip nonexistent paths instead of failing #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9adf57a
feat(allowedpaths): skip nonexistent paths instead of failing
matt-dz Mar 30, 2026
3bf632e
fix(test): remove unnecessary skip_assert_against_bash from scenario
matt-dz Mar 30, 2026
d4f1e73
fix: treat AllowedPaths as suggestions, use |+ block scalars
matt-dz Mar 30, 2026
ddb465b
feat(allowedpaths): warn on stderr when skipping unavailable paths
matt-dz Mar 30, 2026
9d9386c
fix(allowedsymbols): add fmt.Fprintf and os.Stderr to allowedpaths list
matt-dz Mar 30, 2026
ad6a970
refactor(allowedpaths): route warnings through configured stderr
matt-dz Mar 30, 2026
85e0902
fix(test): use relative path so scenario exercises AllowedPaths skip …
matt-dz Mar 30, 2026
6f03767
refactor(allowedpaths): defer warning output until stderr is configured
matt-dz Mar 30, 2026
217735d
style(allowedpaths): name return values in New for clarity
matt-dz Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions allowedpaths/sandbox.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package allowedpaths

import (
"bytes"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -36,33 +37,32 @@ 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.
func New(paths []string) (*Sandbox, error) {
roots := make([]root, len(paths))
for i, p := range paths {
// 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.
//
// 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) (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 {
return nil, fmt.Errorf("AllowedPaths: cannot resolve %q: %w", p, err)
fmt.Fprintf(&buf, "AllowedPaths: skipping %q: %v\n", p, err)
continue
Comment thread
matt-dz marked this conversation as resolved.
Comment thread
matt-dz marked this conversation as resolved.
}
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)
}
Comment thread
matt-dz marked this conversation as resolved.
return nil, fmt.Errorf("AllowedPaths: cannot open root %q: %w", abs, err)
// 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(&buf, "AllowedPaths: skipping %q: %v\n", abs, err)
continue
Comment thread
matt-dz marked this conversation as resolved.
Comment thread
matt-dz marked this conversation as resolved.
Comment thread
matt-dz marked this conversation as resolved.
Comment thread
matt-dz marked this conversation as resolved.
}
roots[i] = root{absPath: abs, root: r}
roots = append(roots, root{absPath: abs, root: r})
}
return &Sandbox{roots: roots}, nil
return &Sandbox{roots: roots}, buf.Bytes(), nil
}

// resolve returns the matching os.Root and the path relative to it for the
Expand Down
60 changes: 57 additions & 3 deletions allowedpaths/sandbox_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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})
require.NoError(t, err)
defer sb.Close()

Expand Down Expand Up @@ -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})
require.NoError(t, err)
defer sb.Close()

Expand Down Expand Up @@ -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})
require.NoError(t, err)
defer sb.Close()

Expand Down Expand Up @@ -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()
}
48 changes: 24 additions & 24 deletions allowedpaths/sandbox_unix_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand Down Expand Up @@ -54,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -72,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -90,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -103,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -115,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -127,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -138,7 +138,7 @@ func TestAccessExecAllowed(t *testing.T) {
func TestAccessNonexistent(t *testing.T) {
dir := t.TempDir()

sb, err := New([]string{dir})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -153,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -166,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -180,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -195,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -213,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -236,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()
assert.ErrorIs(t, sb.Access("writeonly.txt", dir, 0x04), os.ErrPermission)
Expand All @@ -247,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()
assert.NoError(t, sb.Access("readable.txt", dir, 0x04))
Expand All @@ -259,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -281,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -300,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()
assert.NoError(t, sb.Access("subdir", dir, 0x04))
Expand All @@ -314,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()
assert.ErrorIs(t, sb.Access("noread", dir, 0x04), os.ErrPermission)
Expand All @@ -329,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()
// Read-only succeeds
Expand All @@ -347,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()
assert.NoError(t, sb.Access("link.txt", dir, 0x04))
Expand All @@ -360,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()
assert.Error(t, sb.Access("escape.txt", dir, 0x04))
Expand All @@ -381,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand All @@ -397,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})
sb, _, err := New([]string{dir})
require.NoError(t, err)
defer sb.Close()

Expand Down
Loading
Loading