Extract filesystem sandboxing into pathSandbox

interp/allowed_paths.go

@@ -11,113 +11,163 @@ import (
 	"strings"
 )
 
-// AllowedPaths restricts file and directory access to the specified directories.
-// Paths must be absolute directories that exist. When set, only files within
-// these directories can be opened, read, or executed.
-//
-// When not set (default), all file access is blocked.
-// An empty slice also blocks all file access.
-//
+// allowedRoot pairs an absolute directory path with its opened os.Root handle.
+type allowedRoot struct {
+	absPath string
+	root    *os.Root
+}
+
+// pathSandbox restricts filesystem access to a set of allowed directories.
 // The restriction is enforced using os.Root (Go 1.24+), which uses openat
 // syscalls for atomic path validation — immune to symlink and ".." traversal attacks.
-func AllowedPaths(paths []string) RunnerOption {
-	return func(r *Runner) error {
-		cleaned := make([]string, len(paths))
-		for i, p := range paths {
-			abs, err := filepath.Abs(p)
-			if err != nil {
-				return fmt.Errorf("AllowedPaths: cannot resolve %q: %w", p, err)
-			}
-			info, err := os.Stat(abs)
-			if err != nil {
-				return fmt.Errorf("AllowedPaths: cannot stat %q: %w", abs, err)
-			}
-			if !info.IsDir() {
-				return fmt.Errorf("AllowedPaths: %q is not a directory", abs)
-			}
-			cleaned[i] = abs
+type pathSandbox struct {
+	roots []allowedRoot
+}
+
+// newPathSandbox validates paths and creates a pathSandbox without opening
+// os.Root handles. Call [pathSandbox.openRoots] to activate the sandbox.
+func newPathSandbox(paths []string) (*pathSandbox, error) {
+	roots := make([]allowedRoot, len(paths))
+	for i, p := range paths {
+		abs, err := filepath.Abs(p)
+		if err != nil {
+			return nil, fmt.Errorf("AllowedPaths: cannot resolve %q: %w", p, err)
+		}
+		info, err := os.Stat(abs)
+		if err != nil {
+			return nil, fmt.Errorf("AllowedPaths: cannot stat %q: %w", abs, err)
 		}
-		r.allowedPaths = cleaned
+		if !info.IsDir() {
+			return nil, fmt.Errorf("AllowedPaths: %q is not a directory", abs)
+		}
+		roots[i] = allowedRoot{absPath: abs}
+	}
+	return &pathSandbox{roots: roots}, nil
+}
+
+// openRoots opens os.Root handles for every allowed path. It is a no-op if
+// the handles are already open.
+func (s *pathSandbox) openRoots() error {
+	if s == nil || len(s.roots) == 0 || s.roots[0].root != nil {
 		return nil
 	}
+	for i := range s.roots {
+		root, err := os.OpenRoot(s.roots[i].absPath)
+		if err != nil {
+			for _, prev := range s.roots[:i] {
+				prev.root.Close()
+			}
+			return fmt.Errorf("AllowedPaths: cannot open root %q: %w", s.roots[i].absPath, err)
+		}
+		s.roots[i].root = root
+	}
+	return nil
 }
 
-// findMatchingRoot returns the matching os.Root and relative path for an absolute path.
-// It returns false if no root matches.
-func findMatchingRoot(absPath string, roots []*os.Root, allowedPaths []string) (*os.Root, string, bool) {
-	for i, ap := range allowedPaths {
-		rel, err := filepath.Rel(ap, absPath)
+// resolve returns the matching os.Root and the path relative to it for the
+// given absolute path. It returns false if no root matches.
+func (s *pathSandbox) resolve(absPath string) (*os.Root, string, bool) {
+	if s == nil {
+		return nil, "", false
+	}
+	for _, ar := range s.roots {
+		rel, err := filepath.Rel(ar.absPath, absPath)
 		if err != nil {
 			continue
 		}
-		// Check for exact ".." or "..<sep>..." to detect escapes, but not
-		// filenames that happen to start with two dots (e.g. "..hidden").
 		if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
 			continue
 		}
-		return roots[i], rel, true
+		return ar.root, rel, true
 	}
 	return nil, "", false
 }
 
-// restrictedOpenHandler returns an OpenHandlerFunc that restricts file opens to allowed paths.
-// The file is opened through os.Root for atomic path validation.
-func restrictedOpenHandler(roots []*os.Root, allowedPaths []string) OpenHandlerFunc {
-	return func(ctx context.Context, path string, flag int, perm os.FileMode) (io.ReadWriteCloser, error) {
-		absPath := path
-		if !filepath.IsAbs(absPath) {
-			hc := HandlerCtx(ctx)
-			absPath = filepath.Join(hc.Dir, absPath)
-		}
+// toAbs resolves path against cwd when it is not already absolute.
+func toAbs(path, cwd string) string {
+	if filepath.IsAbs(path) {
+		return path
+	}
+	return filepath.Join(cwd, path)
+}
 
-		root, relPath, ok := findMatchingRoot(absPath, roots, allowedPaths)
-		if !ok {
-			return nil, &os.PathError{Op: "open", Path: path, Err: os.ErrPermission}
-		}
+// open implements the restricted file-open policy. The file is opened through
+// os.Root for atomic path validation.
+func (s *pathSandbox) open(ctx context.Context, path string, flag int, perm os.FileMode) (io.ReadWriteCloser, error) {
+	absPath := toAbs(path, HandlerCtx(ctx).Dir)
 
-		f, err := root.OpenFile(relPath, flag, perm)
-		if err != nil {
-			return nil, portablePathError(err)
-		}
-		return f, nil
+	root, relPath, ok := s.resolve(absPath)
+	if !ok {
+		return nil, &os.PathError{Op: "open", Path: path, Err: os.ErrPermission}
 	}
+
+	f, err := root.OpenFile(relPath, flag, perm)
+	if err != nil {
+		return nil, portablePathError(err)
+	}
+	return f, nil
 }
 
-// restrictedReadDirHandler returns a ReadDirHandlerFunc that restricts directory reads to allowed paths.
-func restrictedReadDirHandler(roots []*os.Root, allowedPaths []string) ReadDirHandlerFunc {
-	return func(ctx context.Context, path string) ([]fs.DirEntry, error) {
-		absPath := path
-		if !filepath.IsAbs(absPath) {
-			hc := HandlerCtx(ctx)
-			absPath = filepath.Join(hc.Dir, absPath)
-		}
+// readDir implements the restricted directory-read policy.
+func (s *pathSandbox) readDir(ctx context.Context, path string) ([]fs.DirEntry, error) {
+	absPath := toAbs(path, HandlerCtx(ctx).Dir)
 
-		root, relPath, ok := findMatchingRoot(absPath, roots, allowedPaths)
-		if !ok {
-			return nil, &os.PathError{Op: "readdir", Path: path, Err: os.ErrPermission}
+	root, relPath, ok := s.resolve(absPath)
+	if !ok {
+		return nil, &os.PathError{Op: "readdir", Path: path, Err: os.ErrPermission}
+	}
+
+	f, err := root.Open(relPath)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	entries, err := f.ReadDir(-1)
+	if err != nil {
+		return nil, err
+	}
+	// os.Root's ReadDir does not guarantee sorted order like os.ReadDir.
+	// Sort to match POSIX glob expansion expectations.
+	slices.SortFunc(entries, func(a, b fs.DirEntry) int {
+		if a.Name() < b.Name() {
+			return -1
+		}
+		if a.Name() > b.Name() {
+			return 1
 		}
+		return 0
+	})
+	return entries, nil
+}
 
-		f, err := root.Open(relPath)
-		if err != nil {
-			return nil, err
+// Close releases all os.Root file descriptors. It is safe to call multiple times.
+func (s *pathSandbox) Close() error {
+	if s == nil {
+		return nil
+	}
+	for i := range s.roots {
+		if s.roots[i].root != nil {
+			s.roots[i].root.Close()
+			s.roots[i].root = nil
 		}
-		defer f.Close()
-		entries, err := f.ReadDir(-1)
+	}
+	return nil
+}
+
+// AllowedPaths restricts file and directory access to the specified directories.
+// Paths must be absolute directories that exist. When set, only files within
+// these directories can be opened, read, or executed.
+//
+// When not set (default), all file access is blocked.
+// An empty slice also blocks all file access.
+func AllowedPaths(paths []string) RunnerOption {
+	return func(r *Runner) error {
+		sb, err := newPathSandbox(paths)
 		if err != nil {
-			return nil, err
+			return err
 		}
-		// os.Root's ReadDir does not guarantee sorted order like os.ReadDir.
-		// Sort to match POSIX glob expansion expectations.
-		slices.SortFunc(entries, func(a, b fs.DirEntry) int {
-			if a.Name() < b.Name() {
-				return -1
-			}
-			if a.Name() > b.Name() {
-				return 1
-			}
-			return 0
-		})
-		return entries, nil
+		r.sandbox = sb
+		return nil
 	}
 }
 

interp/api.go

@@ -83,11 +83,9 @@ type Runner struct {
 
 	lastExpandExit exitStatus // used to surface exit statuses while expanding fields
 
-	// allowedPaths restricts file/directory access to these directories.
-	// Empty (default) blocks all file access; populate via AllowedPaths option.
-	allowedPaths []string
-	// roots holds opened os.Root instances, one per allowedPaths entry.
-	roots []*os.Root
+	// sandbox restricts file/directory access to allowed directories.
+	// nil (default) blocks all file access; populate via AllowedPaths option.
+	sandbox *pathSandbox
 
 	origDir    string
 	origParams []string
@@ -268,22 +266,14 @@ func (r *Runner) Reset() {
 			r.execHandler = noExecHandler()
 		}
 		// Open os.Root handles and wrap handlers for path restriction.
-		// Default: block all file access (empty allowedPaths).
-		if r.roots == nil {
-			r.roots = make([]*os.Root, len(r.allowedPaths))
-			for i, p := range r.allowedPaths {
-				root, err := os.OpenRoot(p)
-				if err != nil {
-					for _, prev := range r.roots[:i] {
-						prev.Close()
-					}
-					r.exit.fatal(fmt.Errorf("AllowedPaths: cannot open root %q: %w", p, err))
-					return
-				}
-				r.roots[i] = root
+		// Default: block all file access (nil sandbox).
+		if r.openHandler == nil {
+			if err := r.sandbox.openRoots(); err != nil {
+				r.exit.fatal(err)
+				return
 			}
-			r.openHandler = restrictedOpenHandler(r.roots, r.allowedPaths)
-			r.readDirHandler = restrictedReadDirHandler(r.roots, r.allowedPaths)
+			r.openHandler = r.sandbox.open
+			r.readDirHandler = r.sandbox.readDir
 			// execHandler will be implementer in the future to handle host commands execution
 			// additional safeguard will be needed like Landlock sandbox
 			r.execHandler = noExecHandler()
@@ -296,8 +286,7 @@ func (r *Runner) Reset() {
 		openHandler:    r.openHandler,
 		readDirHandler: r.readDirHandler,
 
-		allowedPaths: r.allowedPaths,
-		roots:        r.roots,
+		sandbox: r.sandbox,
 
 		// These can be set by functions like [Dir] or [Params], but
 		// builtins can overwrite them; reset the fields to whatever the
@@ -376,11 +365,7 @@ func (r *Runner) Run(ctx context.Context, node syntax.Node) (retErr error) {
 // Close releases resources held by the Runner, such as os.Root file descriptors
 // opened by AllowedPaths. It is safe to call Close multiple times.
 func (r *Runner) Close() error {
-	for _, root := range r.roots {
-		root.Close()
-	}
-	r.roots = nil
-	return nil
+	return r.sandbox.Close()
 }
 
 // subshell creates a child Runner that inherits the parent's state.
@@ -399,8 +384,7 @@ func (r *Runner) subshell(background bool) *Runner {
 		openHandler:    r.openHandler,
 		readDirHandler: r.readDirHandler,
 
-		allowedPaths: r.allowedPaths,
-		roots:        r.roots, // safe: os.Root is goroutine-safe
+		sandbox: r.sandbox, // safe: os.Root is goroutine-safe
 
 		stdin:    r.stdin,
 		stdout:   r.stdout,