diff --git a/README.md b/README.md index df4dd325..9359b475 100644 --- a/README.md +++ b/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. diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index de96b007..ad86f83f 100644 --- a/SHELL_FEATURES.md +++ b/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//cmdline` - ❌ External commands — blocked by default; requires an ExecHandler to be configured and the binary to be within AllowedPaths diff --git a/allowedpaths/path_mode.go b/allowedpaths/path_mode.go new file mode 100644 index 00000000..872b43d2 --- /dev/null +++ b/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 +} diff --git a/allowedpaths/path_mode_nonwindows.go b/allowedpaths/path_mode_nonwindows.go new file mode 100644 index 00000000..511d1777 --- /dev/null +++ b/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 +} diff --git a/allowedpaths/path_mode_windows.go b/allowedpaths/path_mode_windows.go new file mode 100644 index 00000000..a791bab9 --- /dev/null +++ b/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 +} diff --git a/allowedpaths/path_mode_windows_test.go b/allowedpaths/path_mode_windows_test.go new file mode 100644 index 00000000..cd8a519d --- /dev/null +++ b/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) +} diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 7078968c..0bed580c 100644 --- a/allowedpaths/sandbox.go +++ b/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. diff --git a/allowedpaths/sandbox_test.go b/allowedpaths/sandbox_test.go index 5e52340c..a84af781 100644 --- a/allowedpaths/sandbox_test.go +++ b/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() diff --git a/analysis/symbols_allowedpaths.go b/analysis/symbols_allowedpaths.go index 4ba8f501..d5a195fc 100644 --- a/analysis/symbols_allowedpaths.go +++ b/analysis/symbols_allowedpaths.go @@ -35,12 +35,14 @@ var allowedpathsAllowedSymbols = []string{ "io/fs.FileMode", // 🟢 file permission bits type; pure type. "io/fs.ReadDirFile", // 🟢 read-only directory handle interface; no write capability. "os.DevNull", // 🟢 platform null device path constant; pure constant. + "os.ErrNotExist", // 🟢 sentinel error for missing literal paths; pure constant. "os.ErrPermission", // 🟢 sentinel error for permission denied; pure constant. "os.File", // 🟠 file handle returned by os.Root.Open; needed for cross-root symlink fallback. "os.FileMode", // 🟢 file permission bits type; pure type. "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.Lstat", // 🟠 checks whether a literal AllowedPaths entry exists without following symlinks, preserving legacy paths that end in :ro/:rw. "os.O_RDONLY", // 🟢 read-only file flag constant; pure constant. "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. @@ -59,6 +61,7 @@ var allowedpathsAllowedSymbols = []string{ "strings.Compare", // 🟢 compares two strings lexicographically; pure function, no I/O. "strings.EqualFold", // 🟢 case-insensitive string comparison; pure function, no I/O. "strings.HasPrefix", // 🟢 pure function for prefix matching; no I/O. + "strings.HasSuffix", // 🟢 pure function for suffix matching; no I/O. "strings.Join", // 🟢 joins string slices; pure function, no I/O. "strings.Split", // 🟢 splits a string by separator; pure function, no I/O. "syscall.ByHandleFileInformation", // 🟢 Windows file identity structure; pure type for file metadata. diff --git a/interp/api.go b/interp/api.go index 56510ea2..ea0bebc9 100644 --- a/interp/api.go +++ b/interp/api.go @@ -736,9 +736,15 @@ func (r *Runner) Warnings() []string { 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. +// AllowedPaths restricts file and directory access to the specified +// directories. Paths must be absolute directories that exist. A path may end +// with :ro or :rw; paths 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 validation. Read-write mode is parsed for future +// write-aware actions, but current rshell file opens still reject write flags. +// 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.