[codex] Parse path access modes in allowed paths

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. Path entries may end with `:ro` or `:rw`; entries without a suffix default to read-only. On platforms that permit literal paths with colons, an existing path ending in `:ro` or `:rw` is kept as a literal read-only path for backward compatibility; otherwise, the suffix is stripped before path validation. `:rw` is parsed and preserved for future write-aware actions, but current rshell commands still cannot write files. When overlapping entries match a path, the most specific configured path controls its mode. 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`, `ip route`, and `df` builtins bypass `AllowedPaths` for their kernel-state reads. `ss` and `ip route` open `/proc/net/*` paths directly; `df` reads `/proc/self/mountinfo` (Linux) or calls `getfsstat(2)` (macOS), then issues `unix.Statfs(2)` against every kernel-reported mount point. These paths are hardcoded — never derived from user input — and `Statfs` returns metadata only (block / inode counts, filesystem type, block size). There is no sandbox-escape risk, but operators cannot use `AllowedPaths` to block `ss` from enumerating local sockets, `ip route` from reading the routing table, or `df` from reporting mount-table capacity — these reads succeed regardless of the configured path policy.
 

SHELL_FEATURES.md

@@ -108,7 +108,7 @@ The in-shell `help` command mirrors these feature categories: run `help` for a c
 ## 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 to specified directories; entries may end with `:ro` or `:rw`, entries without a suffix default to read-only, existing literal paths ending in `:ro` or `:rw` are preserved as read-only on platforms that permit them, and `:rw` is parsed for future write-aware actions without enabling file writes today
 - ✅ 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); `ps` does not read `/proc/<pid>/cmdline`
 - ❌ External commands — blocked by default; requires an ExecHandler to be configured and the binary to be within AllowedPaths

allowedpaths/path_mode.go

@@ -0,0 +1,35 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2026-present Datadog, Inc.
+
+package allowedpaths
+
+import "strings"
+
+type pathMode uint8
+
+const (
+	pathModeReadOnly pathMode = iota
+	pathModeReadWrite
+)
+
+func parseAllowedPathMode(path string) (string, pathMode) {
+	path, mode, _ := splitAllowedPathMode(path)
+	return path, mode
+}
+
+func splitAllowedPathMode(path string) (string, pathMode, bool) {
+	for _, suffix := range []struct {
+		text string
+		mode pathMode
+	}{
+		{text: ":ro", mode: pathModeReadOnly},
+		{text: ":rw", mode: pathModeReadWrite},
+	} {
+		if strings.HasSuffix(path, suffix.text) && len(path) > len(suffix.text) {
+			return path[:len(path)-len(suffix.text)], suffix.mode, true
+		}
+	}
+	return path, pathModeReadOnly, false
+}

allowedpaths/path_mode_nonwindows.go

@@ -0,0 +1,24 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2026-present Datadog, Inc.
+
+//go:build !windows
+
+package allowedpaths
+
+import (
+	"errors"
+	"os"
+)
+
+func resolveAllowedPathMode(path string) (string, pathMode) {
+	stripped, mode, ok := splitAllowedPathMode(path)
+	if !ok {
+		return path, pathModeReadOnly
+	}
+	if _, err := os.Lstat(path); err == nil || !errors.Is(err, os.ErrNotExist) {
+		return path, pathModeReadOnly
+	}
+	return stripped, mode
+}

allowedpaths/path_mode_windows.go

@@ -0,0 +1,16 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2026-present Datadog, Inc.
+
+//go:build windows
+
+package allowedpaths
+
+func resolveAllowedPathMode(path string) (string, pathMode) {
+	stripped, mode, ok := splitAllowedPathMode(path)
+	if !ok {
+		return path, pathModeReadOnly
+	}
+	return stripped, mode
+}

allowedpaths/path_mode_windows_test.go

@@ -0,0 +1,27 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2026-present Datadog, Inc.
+
+//go:build windows
+
+package allowedpaths
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestResolveAllowedPathModeStripsWindowsSuffix(t *testing.T) {
+	base := filepath.Join(t.TempDir(), "policy")
+
+	path, mode := resolveAllowedPathMode(base + ":rw")
+	assert.Equal(t, base, path)
+	assert.Equal(t, pathModeReadWrite, mode)
+
+	path, mode = resolveAllowedPathMode(base + ":ro")
+	assert.Equal(t, base, path)
+	assert.Equal(t, pathModeReadOnly, mode)
+}

allowedpaths/sandbox.go

@@ -42,6 +42,7 @@ const MaxGlobEntries = 10_000
 type root struct {
 	absPath          string
 	canonicalAbsPath string
+	mode             pathMode
 	root             *os.Root
 }
 
@@ -63,6 +64,7 @@ func New(paths []string) (sb *Sandbox, warnings []byte, err error) {
 	var buf bytes.Buffer
 	roots := make([]root, 0, len(paths))
 	for _, p := range paths {
+		p, mode := resolveAllowedPathMode(p)
 		abs, err := filepath.Abs(p)
 		if err != nil {
 			fmt.Fprintf(&buf, "AllowedPaths: skipping %q: %v\n", p, err)
@@ -87,7 +89,7 @@ func New(paths []string) (sb *Sandbox, warnings []byte, err error) {
 		if evalErr != nil {
 			canonical = abs
 		}
-		roots = append(roots, root{absPath: abs, canonicalAbsPath: canonical, root: r})
+		roots = append(roots, root{absPath: abs, canonicalAbsPath: canonical, mode: mode, root: r})
 	}
 	return &Sandbox{roots: roots}, buf.Bytes(), nil
 }
@@ -113,6 +115,8 @@ func (s *Sandbox) resolve(absPath string) (*root, string, bool) {
 	if s == nil {
 		return nil, "", false
 	}
+	var best *root
+	var bestRel string
 	for i := range s.roots {
 		rel, err := filepath.Rel(s.roots[i].absPath, absPath)
 		if err != nil {
@@ -121,9 +125,12 @@ func (s *Sandbox) resolve(absPath string) (*root, string, bool) {
 		if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
 			continue
 		}
-		return &s.roots[i], rel, true
+		if best == nil || len(s.roots[i].absPath) > len(best.absPath) {
+			best = &s.roots[i]
+			bestRel = rel
+		}
 	}
-	return nil, "", false
+	return best, bestRel, best != nil
 }
 
 // isAncestorOfRoot reports whether absPath is a directory prefix of any
@@ -276,42 +283,35 @@ func (s *Sandbox) Access(path string, cwd string, mode uint32) error {
 	if s == nil {
 		return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
 	}
-	for _, ar := range s.roots {
-		rel, err := filepath.Rel(ar.absPath, absPath)
-		if err != nil {
-			continue
-		}
-		if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
-			continue
-		}
+	ar, rel, ok := s.resolve(absPath)
+	if !ok {
+		return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
+	}
 
-		// accessCheck opens or stats the path through os.Root and
-		// performs the permission check (fd-relative OpenFile with
-		// O_NONBLOCK for reads on Unix, mode-bit inspection for
-		// everything else).
-		checkRead := mode&modeRead != 0
-		checkWrite := mode&modeWrite != 0
-		checkExec := mode&modeExecute != 0
-
-		_, err = ar.accessCheck(rel, checkRead, checkWrite, checkExec)
-		if err == nil {
-			return nil
-		}
-		if !isPathEscapeError(err) {
-			return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
-		}
-		// Symlink escapes this root — resolve across all roots.
-		resolved, resolvedRel, ok := s.resolveRootFollowingSymlinks(absPath, false)
-		if !ok {
-			return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
-		}
-		_, err = resolved.accessCheck(resolvedRel, checkRead, checkWrite, checkExec)
-		if err != nil {
-			return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
-		}
+	// accessCheck opens or stats the path through os.Root and performs
+	// the permission check (fd-relative OpenFile with O_NONBLOCK for
+	// reads on Unix, mode-bit inspection for everything else).
+	checkRead := mode&modeRead != 0
+	checkWrite := mode&modeWrite != 0
+	checkExec := mode&modeExecute != 0
+
+	_, err := ar.accessCheck(rel, checkRead, checkWrite, checkExec)
+	if err == nil {
 		return nil
 	}
-	return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
+	if !isPathEscapeError(err) {
+		return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
+	}
+	// Symlink escapes this root — resolve across all roots.
+	resolved, resolvedRel, ok := s.resolveRootFollowingSymlinks(absPath, false)
+	if !ok {
+		return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
+	}
+	_, err = resolved.accessCheck(resolvedRel, checkRead, checkWrite, checkExec)
+	if err != nil {
+		return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
+	}
+	return nil
 }
 
 // toAbs resolves path against cwd when it is not already absolute.

allowedpaths/sandbox_test.go

@@ -12,6 +12,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 	"time"
 
@@ -66,6 +67,137 @@ func TestSandboxOpenRejectsWriteFlags(t *testing.T) {
 	f.Close()
 }
 
+func TestParseAllowedPathMode(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		path string
+		mode pathMode
+	}{
+		{name: "default read-only", in: "/var/log", path: "/var/log", mode: pathModeReadOnly},
+		{name: "explicit read-only", in: "/var/log:ro", path: "/var/log", mode: pathModeReadOnly},
+		{name: "explicit read-write", in: "/var/log:rw", path: "/var/log", mode: pathModeReadWrite},
+		{name: "last terminal suffix wins", in: "/var/log:rw:ro", path: "/var/log:rw", mode: pathModeReadOnly},
+		{name: "middle suffix is path text", in: "/var/log:rw/datadog", path: "/var/log:rw/datadog", mode: pathModeReadOnly},
+		{name: "unknown suffix is path text", in: "/var/log:rx", path: "/var/log:rx", mode: pathModeReadOnly},
+		{name: "bare ro suffix is path text", in: ":ro", path: ":ro", mode: pathModeReadOnly},
+		{name: "bare rw suffix is path text", in: ":rw", path: ":rw", mode: pathModeReadOnly},
+		{name: "empty path", in: "", path: "", mode: pathModeReadOnly},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			path, mode := parseAllowedPathMode(tt.in)
+			assert.Equal(t, tt.path, path)
+			assert.Equal(t, tt.mode, mode)
+		})
+	}
+}
+
+func TestResolveAllowedPathModePreservesExistingLiteralPath(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("literal paths ending in :rw/:ro are POSIX-only")
+	}
+
+	dir := t.TempDir()
+	literal := filepath.Join(dir, "tenant:rw")
+	require.NoError(t, os.Mkdir(literal, 0755))
+
+	path, mode := resolveAllowedPathMode(literal)
+	assert.Equal(t, literal, path)
+	assert.Equal(t, pathModeReadOnly, mode)
+}
+
+func TestAllowedPathModesAreStoredAfterSuffixStripping(t *testing.T) {
+	dir := t.TempDir()
+
+	sb, _, err := New([]string{dir + ":rw"})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	assert.Equal(t, []string{dir}, sb.Paths())
+
+	root, _, ok := sb.resolve(dir)
+	require.True(t, ok)
+	assert.Equal(t, pathModeReadWrite, root.mode)
+}
+
+func TestAllowedPathModeMostSpecificRootWins(t *testing.T) {
+	dir := t.TempDir()
+	child := filepath.Join(dir, "datadog")
+	require.NoError(t, os.Mkdir(child, 0755))
+	require.NoError(t, os.WriteFile(filepath.Join(child, "agent.log"), []byte("data"), 0644))
+
+	sb, _, err := New([]string{dir + ":rw", child + ":ro"})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	root, _, ok := sb.resolve(filepath.Join(child, "agent.log"))
+	require.True(t, ok)
+	assert.Equal(t, child, root.absPath)
+	assert.Equal(t, pathModeReadOnly, root.mode)
+}
+
+func TestAllowedPathModeMostSpecificReadWriteWins(t *testing.T) {
+	dir := t.TempDir()
+	child := filepath.Join(dir, "datadog")
+	require.NoError(t, os.Mkdir(child, 0755))
+	require.NoError(t, os.WriteFile(filepath.Join(child, "agent.log"), []byte("data"), 0644))
+
+	sb, _, err := New([]string{dir + ":ro", child + ":rw"})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	root, _, ok := sb.resolve(filepath.Join(child, "agent.log"))
+	require.True(t, ok)
+	assert.Equal(t, child, root.absPath)
+	assert.Equal(t, pathModeReadWrite, root.mode)
+}
+
+func TestAllowedPathReadWriteModeDoesNotEnableWriteOpen(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644))
+
+	sb, _, err := New([]string{dir + ":rw"})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	f, err := sb.Open("test.txt", dir, os.O_RDWR, 0)
+	assert.Nil(t, f)
+	assert.ErrorIs(t, err, os.ErrPermission)
+}
+
+func TestAllowedPathModeDoesNotWidenExistingLiteralSuffixPath(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("literal paths ending in :rw/:ro are POSIX-only")
+	}
+
+	parent := t.TempDir()
+	base := filepath.Join(parent, "tenant")
+	literal := base + ":rw"
+	require.NoError(t, os.Mkdir(base, 0755))
+	require.NoError(t, os.Mkdir(literal, 0755))
+	require.NoError(t, os.WriteFile(filepath.Join(base, "base.txt"), []byte("base"), 0644))
+	require.NoError(t, os.WriteFile(filepath.Join(literal, "literal.txt"), []byte("literal"), 0644))
+
+	sb, _, err := New([]string{literal})
+	require.NoError(t, err)
+	defer sb.Close()
+
+	assert.Equal(t, []string{literal}, sb.Paths())
+
+	root, _, ok := sb.resolve(filepath.Join(literal, "literal.txt"))
+	require.True(t, ok)
+	assert.Equal(t, literal, root.absPath)
+	assert.Equal(t, pathModeReadOnly, root.mode)
+
+	f, err := sb.Open(filepath.Join(literal, "literal.txt"), "/", os.O_RDONLY, 0)
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+
+	_, err = sb.Open(filepath.Join(base, "base.txt"), "/", os.O_RDONLY, 0)
+	assert.ErrorIs(t, err, os.ErrPermission)
+}
+
 func TestReadDirLimited(t *testing.T) {
 	dir := t.TempDir()