feat(interp): add WarningsWriter option and Warnings accessor for sandbox diagnostics

README.md

@@ -66,7 +66,7 @@ Every access path is default-deny:
 
 **AllowedCommands** restricts which commands (builtins or external) the interpreter may execute. Commands must be specified with the `rshell:` namespace prefix (e.g. `rshell:cat`, `rshell:echo`). If not set, no commands are allowed.
 
-**AllowedPaths** restricts all file operations to specified directories using Go's `os.Root` API (`openat` syscalls), making it immune to symlink traversal, TOCTOU races, and `..` escape attacks.
+**AllowedPaths** restricts all file operations to specified directories using Go's `os.Root` API (`openat` syscalls), making it immune to symlink traversal, TOCTOU races, and `..` escape attacks. Configured directories that cannot be opened (missing, not a directory, no permission) are skipped with a diagnostic message; by default these messages are flushed once to the runner's stderr at construction time. Callers that need to keep stderr clean of sandbox diagnostics can route them to a dedicated sink with `WarningsWriter(io.Writer)` or retrieve them programmatically via `Runner.Warnings()`.
 
 > **Note:** The `ss` and `ip route` builtins bypass `AllowedPaths` for their `/proc/net/*` reads. Both builtins open kernel pseudo-filesystem paths (e.g. `/proc/net/tcp`, `/proc/net/route`) directly with `os.Open` rather than going through the sandboxed opener. These paths are hardcoded in the implementation and are never derived from user input, so there is no sandbox-escape risk. However, operators cannot use `AllowedPaths` to block `ss` from enumerating local sockets or `ip route` from reading the routing table — these reads succeed regardless of the configured path policy.
 

interp/allowed_paths_test.go

@@ -9,6 +9,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"io"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -289,3 +290,85 @@ func TestAllowedPathsClose(t *testing.T) {
 	require.NoError(t, runner.Close())
 	require.NoError(t, runner.Close())
 }
+
+func TestSandboxWarningsDefaultGoToStderr(t *testing.T) {
+	var outBuf, errBuf bytes.Buffer
+	runner, err := interp.New(
+		interp.StdIO(nil, &outBuf, &errBuf),
+		interp.AllowedPaths([]string{"/nonexistent/path/that/does/not/exist"}),
+	)
+	require.NoError(t, err)
+	defer runner.Close()
+
+	assert.Contains(t, errBuf.String(), "AllowedPaths: skipping",
+		"with no WarningsWriter option, warnings should fall back to stderr")
+	assert.Empty(t, outBuf.String(), "warnings must never land on stdout")
+}
+
+func TestSandboxWarningsRoutedToDedicatedWriter(t *testing.T) {
+	var outBuf, errBuf, warnBuf bytes.Buffer
+	runner, err := interp.New(
+		interp.StdIO(nil, &outBuf, &errBuf),
+		interp.WarningsWriter(&warnBuf),
+		interp.AllowedPaths([]string{"/nonexistent/path/that/does/not/exist"}),
+	)
+	require.NoError(t, err)
+	defer runner.Close()
+
+	assert.Contains(t, warnBuf.String(), "AllowedPaths: skipping",
+		"warnings should be routed to the dedicated writer")
+	assert.Empty(t, errBuf.String(),
+		"with WarningsWriter set, stderr must remain clean of sandbox diagnostics")
+	assert.Empty(t, outBuf.String(), "warnings must never land on stdout")
+}
+
+func TestSandboxWarningsAccessor(t *testing.T) {
+	t.Run("returns warnings as slice", func(t *testing.T) {
+		runner, err := interp.New(
+			interp.WarningsWriter(io.Discard),
+			interp.AllowedPaths([]string{
+				"/nonexistent/path/one",
+				"/nonexistent/path/two",
+			}),
+		)
+		require.NoError(t, err)
+		defer runner.Close()
+
+		warnings := runner.Warnings()
+		require.Len(t, warnings, 2, "one entry per skipped path")
+		assert.Contains(t, warnings[0], "AllowedPaths: skipping")
+		assert.Contains(t, warnings[1], "AllowedPaths: skipping")
+	})
+
+	t.Run("returns nil when no warnings emitted", func(t *testing.T) {
+		dir := t.TempDir()
+		runner, err := interp.New(
+			interp.AllowedPaths([]string{dir}),
+		)
+		require.NoError(t, err)
+		defer runner.Close()
+
+		assert.Nil(t, runner.Warnings(),
+			"a clean configuration should yield no warnings")
+	})
+
+	t.Run("accessor still works when streaming flush is suppressed", func(t *testing.T) {
+		runner, err := interp.New(
+			interp.WarningsWriter(io.Discard),
+			interp.AllowedPaths([]string{"/nonexistent/path"}),
+		)
+		require.NoError(t, err)
+		defer runner.Close()
+
+		warnings := runner.Warnings()
+		require.Len(t, warnings, 1,
+			"io.Discard should suppress streaming output but not the accessor")
+		assert.Contains(t, warnings[0], "AllowedPaths: skipping")
+	})
+}
+
+func TestWarningsWriterRejectsNil(t *testing.T) {
+	_, err := interp.New(interp.WarningsWriter(nil))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "writer must not be nil")
+}

interp/api.go

@@ -55,10 +55,19 @@ type runnerConfig struct {
 	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.
+	// entries. Flushed to warningsWriter after all options are applied and
+	// defaults are set, so the output target is independent of option
+	// ordering. Retained on the runner after flush so callers can also
+	// retrieve them programmatically via [Runner.Warnings].
 	sandboxWarnings []byte
 
+	// warningsWriter is the destination for sandbox diagnostic messages
+	// (currently just AllowedPaths skip warnings). Defaults to r.stderr
+	// when unset; callers can route warnings to a separate sink (e.g. a
+	// dedicated buffer or logger) via [WarningsWriter] so they are not
+	// conflated with command stderr.
+	warningsWriter io.Writer
+
 	// hostPrefix is stored here so HostPrefix can be applied before or
 	// after AllowedPaths. Applied to the sandbox in New() after all
 	// options are processed.
@@ -280,15 +289,21 @@ func New(opts ...RunnerOption) (*Runner, error) {
 	if r.stdout == nil || r.stderr == nil {
 		StdIO(r.stdin, r.stdout, r.stderr)(r)
 	}
+	// Default sandbox warnings to r.stderr so today's behaviour is
+	// preserved for callers who do not opt in to a dedicated sink.
+	if r.warningsWriter == nil {
+		r.warningsWriter = r.stderr
+	}
 	// Apply host prefix if set, now that both HostPrefix and AllowedPaths
 	// have been processed regardless of option ordering.
 	if r.hostPrefix != "" && r.sandbox != nil {
 		r.sandbox.SetHostPrefix(r.hostPrefix)
 	}
-	// Flush any sandbox warnings now that stderr is guaranteed to be set.
+	// Flush any sandbox warnings now that the warnings sink is guaranteed
+	// to be set. The buffer is retained on the runner so callers can also
+	// retrieve warnings via [Runner.Warnings].
 	if len(r.sandboxWarnings) > 0 {
-		r.stderr.Write(r.sandboxWarnings)
-		r.sandboxWarnings = nil
+		r.warningsWriter.Write(r.sandboxWarnings)
 	}
 	r.proc = builtins.NewProcProvider(r.procPath)
 	return r, nil
@@ -631,6 +646,53 @@ func (r *Runner) Close() error {
 	return r.sandbox.Close()
 }
 
+// WarningsWriter routes sandbox diagnostic messages (currently produced by
+// [AllowedPaths] when a configured directory cannot be opened) to the given
+// writer instead of the runner's stderr.
+//
+// Sandbox diagnostics are buffered during option processing and flushed once
+// during [New], after all other options have been applied. They are not
+// written again on subsequent runs.
+//
+// When unset, warnings fall back to the runner's stderr writer (whatever was
+// supplied via [StdIO]), matching the historical behaviour. Callers that
+// inspect stderr to detect command failure should pass a dedicated writer
+// here so sandbox diagnostics are not conflated with command output.
+//
+// Passing [io.Discard] suppresses the streaming flush entirely; the messages
+// remain accessible via [Runner.Warnings] regardless.
+func WarningsWriter(w io.Writer) RunnerOption {
+	return func(r *Runner) error {
+		if w == nil {
+			return fmt.Errorf("WarningsWriter: writer must not be nil")
+		}
+		r.warningsWriter = w
+		return nil
+	}
+}
+
+// Warnings returns the sandbox diagnostic messages collected during runner
+// construction (currently produced by [AllowedPaths] when a configured
+// directory cannot be opened), one entry per warning. The slice is empty when
+// no warnings were emitted.
+//
+// Callers that integrate rshell as a library can use this to surface
+// configuration problems in their own structured output (e.g. a "warnings"
+// field in an API response) without parsing them out of stderr.
+func (r *Runner) Warnings() []string {
+	if len(r.sandboxWarnings) == 0 {
+		return nil
+	}
+	s := string(r.sandboxWarnings)
+	// allowedpaths.New emits one warning per line, each terminated with
+	// "\n". Strip a single trailing newline before splitting so the result
+	// is one entry per warning rather than ending in a stray empty string.
+	if strings.HasSuffix(s, "\n") {
+		s = s[:len(s)-1]
+	}
+	return strings.Split(s, "\n")
+}
+
 // 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.