feat(allowedpaths): permit writes through sandbox open

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. 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()`.
+**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. Both reads and writes are sandboxed by the same mechanism — files outside the allowlist cannot be opened, created, truncated, or appended to. The cross-root symlink fallback is read-only: a symlink that points outside its `os.Root` is followed for reads but never for writes (avoids a TOCTOU window where a malicious link target could be swapped between resolution and open). Whether a particular shell feature actually performs writes is layered on top — output redirections (`>`, `>>`) to file targets are still rejected at parse time, so the user-visible surface remains read-only until builtins or redirection layers opt in. 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.
 

SHELL_FEATURES.md

@@ -99,7 +99,7 @@ Blocked features are rejected before execution with exit code 2.
 ## Execution
 
 - ✅ AllowedCommands — restricts which commands (builtins or external) may be executed; commands require the `rshell:` namespace prefix (e.g. `rshell:cat`); if not set, no commands are allowed
-- ✅ AllowedPaths filesystem sandboxing — restricts all file access to specified directories
+- ✅ AllowedPaths filesystem sandboxing — restricts all file access (read and write) to specified directories; cross-root symlink fallback is read-only to avoid TOCTOU on writes
 - ✅ Whole-run execution timeout — callers can bound a `Run()` call via `context.Context`, `interp.MaxExecutionTime`, or the CLI `--timeout` flag; the deadline applies to the entire script, not each individual command
 - ✅ ProcPath — overrides the proc filesystem path used by `ps` (default `/proc`; Linux-only; useful for testing/container environments)
 - ❌ External commands — blocked by default; requires an ExecHandler to be configured and the binary to be within AllowedPaths

allowedpaths/sandbox.go

@@ -291,11 +291,29 @@ func IsDevNull(path string) bool {
 	return false
 }
 
+// allowedOpenFlags is the set of os.OpenFile flag bits permitted by the
+// sandbox. Anything outside this mask (O_DIRECTORY, O_NOFOLLOW, O_SYNC,
+// O_NONBLOCK, etc.) is rejected as a defense-in-depth measure so unknown
+// or platform-specific flags cannot slip through.
+const allowedOpenFlags = os.O_RDONLY | os.O_WRONLY | os.O_RDWR |
+	os.O_APPEND | os.O_CREATE | os.O_EXCL | os.O_TRUNC
+
 // Open implements the restricted file-open policy. The file is opened through
-// os.Root for atomic path validation. Only read-only access is permitted;
-// any write flags are rejected as a defense-in-depth measure.
+// os.Root for atomic path validation, which uses openat under the hood and is
+// immune to symlink and ".." traversal between path validation and open.
+//
+// Read and write opens are both permitted. Only the flag bits in
+// allowedOpenFlags are accepted; anything else returns ErrPermission.
+//
+// The cross-root symlink fallback (resolveFollowingSymlinks) is read-only.
+// Following a symlink that escapes its os.Root and then performing a write
+// (O_CREATE/O_TRUNC/O_APPEND/O_WRONLY/O_RDWR) is the classic TOCTOU footgun:
+// a malicious symlink could redirect a create or truncate to a target that
+// has changed between resolution and open, defeating the sandbox. Writes
+// must therefore stay within a single os.Root and never traverse the
+// cross-root fallback.
 func (s *Sandbox) Open(path string, cwd string, flag int, perm os.FileMode) (io.ReadWriteCloser, error) {
-	if flag != os.O_RDONLY {
+	if flag&^allowedOpenFlags != 0 {
 		return nil, &os.PathError{Op: "open", Path: path, Err: os.ErrPermission}
 	}
 
@@ -313,7 +331,12 @@ func (s *Sandbox) Open(path string, cwd string, flag int, perm os.FileMode) (io.
 	if !isPathEscapeError(err) {
 		return nil, PortablePathError(err)
 	}
-	// Symlink escapes this root — resolve across all roots.
+	// Symlink escapes this root. Only fall back across roots for
+	// read-only opens — cross-root writes are TOCTOU-unsafe (see the
+	// function comment above).
+	if flag != os.O_RDONLY {
+		return nil, PortablePathError(err)
+	}
 	r, rel, ok := s.resolveFollowingSymlinks(absPath, false)
 	if !ok {
 		return nil, PortablePathError(err)

allowedpaths/sandbox_test.go

@@ -38,29 +38,136 @@ func (f fakeFileInfo) ModTime() time.Time { return time.Time{} }
 func (f fakeFileInfo) IsDir() bool        { return false }
 func (f fakeFileInfo) Sys() any           { return nil }
 
-func TestSandboxOpenRejectsWriteFlags(t *testing.T) {
+func TestSandboxWriteAllowedPath(t *testing.T) {
 	dir := t.TempDir()
-	require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644))
 
 	sb, _, err := New([]string{dir})
 	require.NoError(t, err)
 	defer sb.Close()
 
-	writeFlags := []int{
-		os.O_WRONLY,
-		os.O_RDWR,
-		os.O_APPEND,
-		os.O_CREATE,
-		os.O_TRUNC,
-		os.O_WRONLY | os.O_CREATE | os.O_TRUNC,
-	}
-	for _, flag := range writeFlags {
-		f, err := sb.Open("test.txt", dir, flag, 0644)
-		assert.Nil(t, f, "open with flag %d should return nil", flag)
-		assert.ErrorIs(t, err, os.ErrPermission, "open with flag %d should be denied", flag)
-	}
+	// O_CREATE|O_WRONLY for a new file inside the allowlist should succeed
+	// and the file's contents should reflect the write.
+	f, err := sb.Open("created.txt", dir, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
+	require.NoError(t, err)
+	n, err := f.Write([]byte("hello"))
+	require.NoError(t, err)
+	assert.Equal(t, 5, n)
+	require.NoError(t, f.Close())
+
+	got, err := os.ReadFile(filepath.Join(dir, "created.txt"))
+	require.NoError(t, err)
+	assert.Equal(t, "hello", string(got))
+}
+
+func TestSandboxWriteOutsideAllowedPath(t *testing.T) {
+	allowed := t.TempDir()
+	outside := t.TempDir()
+
+	sb, _, err := New([]string{allowed})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	// Absolute path outside the allowlist must be rejected for writes.
+	target := filepath.Join(outside, "should-not-exist.txt")
+	f, err := sb.Open(target, allowed, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
+	assert.Nil(t, f)
+	assert.ErrorIs(t, err, os.ErrPermission)
+
+	_, statErr := os.Stat(target)
+	assert.True(t, os.IsNotExist(statErr), "file outside allowlist must not be created")
+}
+
+func TestSandboxAppend(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "log.txt")
+	require.NoError(t, os.WriteFile(path, []byte("first\n"), 0644))
+
+	sb, _, err := New([]string{dir})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	f, err := sb.Open("log.txt", dir, os.O_WRONLY|os.O_APPEND, 0)
+	require.NoError(t, err)
+	_, err = f.Write([]byte("second\n"))
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+
+	got, err := os.ReadFile(path)
+	require.NoError(t, err)
+	assert.Equal(t, "first\nsecond\n", string(got))
+}
+
+func TestSandboxTruncate(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "data.txt")
+	require.NoError(t, os.WriteFile(path, []byte("original-content"), 0644))
+
+	sb, _, err := New([]string{dir})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	f, err := sb.Open("data.txt", dir, os.O_WRONLY|os.O_TRUNC, 0)
+	require.NoError(t, err)
+	_, err = f.Write([]byte("short"))
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+
+	got, err := os.ReadFile(path)
+	require.NoError(t, err)
+	assert.Equal(t, "short", string(got), "O_TRUNC must replace, not append to, the original content")
+}
+
+// TestSandboxWriteThroughSymlinkEscapeRejected ensures the cross-root
+// symlink fallback is not used for write opens. Following a symlink that
+// escapes its os.Root and then performing a create or truncate is the
+// classic TOCTOU footgun: the link target can be swapped between
+// resolution and open. Writes must stay inside a single os.Root.
+func TestSandboxWriteThroughSymlinkEscapeRejected(t *testing.T) {
+	allowed := t.TempDir()
+	outside := t.TempDir()
+
+	// Symlink inside the allowlist that points to a path *outside* the
+	// allowlist. os.Root will reject this with a path-escape error;
+	// the sandbox must then refuse to fall back for writes.
+	linkPath := filepath.Join(allowed, "escape")
+	require.NoError(t, os.Symlink(filepath.Join(outside, "target.txt"), linkPath))
+
+	sb, _, err := New([]string{allowed})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	f, err := sb.Open("escape", allowed, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
+	assert.Nil(t, f)
+	assert.Error(t, err)
+
+	_, statErr := os.Stat(filepath.Join(outside, "target.txt"))
+	assert.True(t, os.IsNotExist(statErr), "symlink target must not be created outside the sandbox")
+}
+
+func TestSandboxWriteRejectsUnknownFlag(t *testing.T) {
+	dir := t.TempDir()
+
+	sb, _, err := New([]string{dir})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	// Pick a high bit that is not in allowedOpenFlags. We OR with
+	// O_WRONLY so the access mode itself is valid; only the unknown bit
+	// should trigger rejection.
+	const unknownFlag = 1 << 30
+	f, err := sb.Open("x.txt", dir, os.O_WRONLY|os.O_CREATE|unknownFlag, 0644)
+	assert.Nil(t, f)
+	assert.ErrorIs(t, err, os.ErrPermission)
+}
+
+func TestSandboxOpenReadStillWorks(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644))
+
+	sb, _, err := New([]string{dir})
+	require.NoError(t, err)
+	defer sb.Close()
 
-	// Read-only should still work.
 	f, err := sb.Open("test.txt", dir, os.O_RDONLY, 0)
 	require.NoError(t, err)
 	f.Close()

analysis/symbols_allowedpaths.go

@@ -41,7 +41,13 @@ var allowedpathsAllowedSymbols = []string{
 	"os.Getgid",                          // 🟠 returns the numeric group id of the caller; read-only syscall.
 	"os.Getgroups",                       // 🟠 returns supplementary group ids; read-only syscall.
 	"os.Getuid",                          // 🟠 returns the numeric user id of the caller; read-only syscall.
+	"os.O_APPEND",                        // 🟢 append-on-write file flag constant; pure constant. Part of the sandbox open-flag allowlist.
+	"os.O_CREATE",                        // 🟢 create-if-missing file flag constant; pure constant. Part of the sandbox open-flag allowlist.
+	"os.O_EXCL",                          // 🟢 exclusive-create file flag constant; pure constant. Part of the sandbox open-flag allowlist.
 	"os.O_RDONLY",                        // 🟢 read-only file flag constant; pure constant.
+	"os.O_RDWR",                          // 🟢 read-write file flag constant; pure constant. Part of the sandbox open-flag allowlist.
+	"os.O_TRUNC",                         // 🟢 truncate-on-open file flag constant; pure constant. Part of the sandbox open-flag allowlist.
+	"os.O_WRONLY",                        // 🟢 write-only file flag constant; pure constant. Part of the sandbox open-flag allowlist.
 	"os.OpenRoot",                        // 🟠 opens a directory as a root for sandboxed file access; needed for sandbox.
 	"os.PathError",                       // 🟢 error type wrapping path and operation; pure type.
 	"os.Root",                            // 🟠 sandboxed directory root type; core of the filesystem sandbox.

interp/api.go

@@ -695,7 +695,14 @@ func (r *Runner) Warnings() []string {
 
 // 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.
+// these directories can be opened (for reading or writing), read, or executed.
+//
+// The sandbox itself permits both read and write opens through os.Root;
+// whether a particular shell feature (a builtin, a redirection, etc.)
+// actually performs writes is a separate, layered decision. The validate
+// pass currently blocks file-target output redirections (>, >>) at parse
+// time, so the user-visible surface remains read-only until those layers
+// opt in.
 //
 // When not set (default), all file access is blocked.
 // An empty slice also blocks all file access.