Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
9b0577f
feat(find): add --help flag and -type b,c support
matt-dz Mar 16, 2026
ec29704
test(find): add tests for -type b,c block/char device support
matt-dz Mar 16, 2026
0a5be0b
feat(find): add 21 GNU find predicates for comprehensive file search
matt-dz Mar 16, 2026
328427b
fix: add new find predicate symbols to the allowlist
matt-dz Mar 16, 2026
fb4eb74
[iter 1] Fix review findings: samefile cache, user/group error dedup,…
matt-dz Mar 16, 2026
bdfb9fa
[iter 1] Fix Windows CI: add stdout_windows for perm/executable tests
matt-dz Mar 16, 2026
9b4e203
[iter 3] Fix codex review: -perm special bits, -quit as action, -user…
matt-dz Mar 16, 2026
24bff8a
Address codex review: -xdev evaluate before skip, -samefile errors, s…
matt-dz Mar 16, 2026
75da86c
refactor(find): gut low-value predicates, keep only agent-useful flags
matt-dz Mar 16, 2026
33ebf3f
refactor(find): remove -printf — -print and -print0 suffice
matt-dz Mar 16, 2026
a37fb20
fix(find): -perm special bits (setuid/setgid/sticky) and symbolic s/t
matt-dz Mar 16, 2026
33b6a22
Address codex review: enable bash parity, remove -H from usage
matt-dz Mar 16, 2026
f6b7675
Address codex review: fix symbolic = clearing special bits, add tests
matt-dz Mar 16, 2026
44012c0
fix(find): remove incorrect stdout_windows from perm_any_write test
matt-dz Mar 16, 2026
e676125
Address codex review: --help after paths, re-enable bash parity
matt-dz Mar 16, 2026
41e4bdb
docs(find): note unsupported GNU symbolic mode extensions
matt-dz Mar 16, 2026
7fc02fb
Address codex review: revert --help expression scan, fix quit test order
matt-dz Mar 16, 2026
c8a8fd1
fix(find): recognize --help after path operands via parsePrimary
matt-dz Mar 16, 2026
084c9f8
fix(find): use non-blocking stat-based readability checks for -readable
matt-dz Mar 16, 2026
caa6792
fix(access): preserve root read/write bypass in permission checks
matt-dz Mar 17, 2026
77b0ce3
feat(find): support GNU symbolic mode copy-bits and conditional execu…
matt-dz Mar 17, 2026
45dc6bc
fix(find): add --help to help output, fix Windows perm test failures
matt-dz Mar 17, 2026
79b6055
fix(find): respect who mask when applying symbolic sticky bit
matt-dz Mar 17, 2026
08016dd
Merge branch 'main' of github.com:DataDog/rshell into find-help-and-t…
matt-dz Mar 17, 2026
132af05
fix(access): use kernel access(2) for permission checks instead of mo…
matt-dz Mar 17, 2026
7e0a406
fix(access): guard accessCheck by accepting root + rel separately
matt-dz Mar 17, 2026
9e7299d
fix(access): use OpenFile for Windows read checks, bind accessCheck t…
matt-dz Mar 17, 2026
15b78e6
fix(access): eliminate TOCTOU by keeping all permission checks fd-rel…
matt-dz Mar 17, 2026
dd4ec3d
fix(access): open-first with O_NONBLOCK to eliminate Stat→OpenFile TO…
matt-dz Mar 17, 2026
834c08d
fix(find): stop treating -quit as implicit-print suppressor
matt-dz Mar 17, 2026
1ac97c4
fix(ci): deny execute checks on Windows, add golang.org/x/sys to lice…
matt-dz Mar 17, 2026
b8cbd93
test: add skip_if_root to scenario framework; guard readable_not_read…
matt-dz Mar 17, 2026
ff8acd7
test: guard test_write_denied scenario against root execution
matt-dz Mar 17, 2026
de40aaa
feat(find): remove -readable predicate
matt-dz Mar 18, 2026
0df4aea
test: remove root-sensitive test_write_denied scenario
matt-dz Mar 18, 2026
bfbe097
test: remove unused SkipIfRoot scenario field
matt-dz Mar 18, 2026
f4ae091
test: remove redundant test_exec_check scenario
matt-dz Mar 18, 2026
be06e57
fix(test): align Windows execute test with always-deny implementation
matt-dz Mar 18, 2026
24188ba
Merge branch 'main' into find-help-and-type-bc
matt-dz Mar 18, 2026
0402635
fix: check f.Close() errors in accessCheck on Unix and Windows
matt-dz Mar 18, 2026
5ed2876
Merge branch 'main' of github.com:DataDog/rshell into find-help-and-t…
matt-dz Mar 18, 2026
d7d90ca
fix(access): restore write checks in Windows accessCheck
matt-dz Mar 18, 2026
79a963a
fix(ci): add missing find symbols to builtins allowlist
matt-dz Mar 18, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion SHELL_FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Blocked features are rejected before execution with exit code 2.
- ✅ `echo [-neE] [ARG]...` — write arguments to stdout; `-n` suppresses trailing newline, `-e` enables backslash escapes, `-E` disables them (default)
- ✅ `exit [N]` — exit the shell with status N (default 0)
- ✅ `false` — return exit code 1
- ✅ `find [-L] [PATH...] [EXPRESSION]` — search for files in a directory hierarchy; supports `-name`, `-iname`, `-path`, `-ipath`, `-type`, `-size`, `-empty`, `-newer`, `-mtime`, `-mmin`, `-maxdepth`, `-mindepth`, `-print`, `-print0`, `-prune`, logical operators (`!`, `-a`, `-o`, `()`); blocks `-exec`, `-delete`, `-regex` for sandbox safety
- ✅ `find [-L] [-P] [PATH...] [EXPRESSION]` — search for files in a directory hierarchy; supports `--help`, `-name`, `-iname`, `-path`, `-ipath`, `-type` (b,c,d,f,l,p,s), `-size`, `-empty`, `-newer`, `-mtime`, `-mmin`, `-perm`, `-maxdepth`, `-mindepth`, `-print`, `-print0`, `-prune`, `-quit`, logical operators (`!`, `-a`, `-o`, `()`); blocks `-exec`, `-delete`, `-regex` for sandbox safety
- ✅ `grep [-EFGivclLnHhoqsxw] [-e PATTERN] [-m NUM] [-A NUM] [-B NUM] [-C NUM] PATTERN [FILE]...` — print lines that match patterns; uses RE2 regex engine (linear-time, no backtracking)
- ✅ `head [-n N|-c N] [-q|-v] [FILE]...` — output the first part of files (default: first 10 lines); `-z`/`--zero-terminated` and `--follow` are rejected
- ✅ `help` — display all available builtin commands with brief descriptions; for detailed flag info, use `<command> --help`
Expand Down
103 changes: 93 additions & 10 deletions allowedpaths/portable_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,107 @@ func FileIdentity(_ string, info fs.FileInfo, _ *Sandbox) (uint64, uint64, bool)
return uint64(st.Dev), uint64(st.Ino), true
}

func (r *root) accessCheck(rel string, checkRead, checkWrite, checkExec bool) (fs.FileInfo, error) {
// Write-only or exec-only checks (no read): single Stat + mode-bit
// inspection. No TOCTOU because there is only one resolution.
if !checkRead {
info, err := r.root.Stat(rel)
if err != nil {
return nil, err
}
if !effectiveHasPerm(info, false, checkWrite, checkExec) {
return info, os.ErrPermission
}
return info, nil
}

// Read checks: open-first to get an fd, then fstat the fd.
// O_NONBLOCK prevents blocking on FIFOs (open returns immediately
// even without a writer). It is harmless on regular files and dirs.
f, openErr := r.root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0)
if openErr != nil {
// OpenFile failed. Possible reasons:
// - Permission denied on a regular file (kernel/ACL)
// - Unopenable type (Unix socket → ENXIO/EOPNOTSUPP)
// - Path does not exist or symlink escape blocked
//
// Fall back to Stat for metadata. This is NOT a TOCTOU risk:
// the open already failed, so there is no fd pointing to a
// wrong inode.
info, err := r.root.Stat(rel)
if err != nil {
return nil, err
}
// For regular files, the open failure is the kernel's
// authoritative answer (may reflect ACLs that mode bits
// miss). Trust it.
if info.Mode().IsRegular() {
return info, os.ErrPermission
}
// Non-regular files that can't be opened (e.g. sockets):
// fall back to mode-bit inspection.
if !effectiveHasPerm(info, checkRead, checkWrite, checkExec) {
return info, os.ErrPermission
}
return info, nil
}

// OpenFile succeeded — fstat the fd for metadata from this exact inode.
info, err := f.Stat()
closeErr := f.Close()
if err != nil {
return nil, err
}
if closeErr != nil {
return nil, closeErr
}

// For regular files, the successful open proves read permission
// (kernel-level, ACL-accurate). For FIFOs and directories,
// O_NONBLOCK open succeeds regardless of read permission, so
// mode-bit check is still needed.
if !info.Mode().IsRegular() {
if !effectiveHasPerm(info, checkRead, checkWrite, checkExec) {
return info, os.ErrPermission
}
return info, nil
}

// Regular file: read proven. Check write/exec if needed.
if checkWrite || checkExec {
if !effectiveHasPerm(info, false, checkWrite, checkExec) {
return info, os.ErrPermission
}
}
return info, nil
}

// effectiveHasPerm checks whether the current process has the requested
// permission (writeMask or execMask, each a 3-bit pattern like 0222 or 0111)
// by inspecting the file's owner/group/other permission class that applies to
// the effective UID and GID of the running process.
// permission by inspecting the file's owner/group/other permission class
// that applies to the effective UID and GID of the running process.
//
// On Unix this uses the Stat_t from info.Sys() to determine the owning
// UID/GID and then selects the owner, group, or other permission bits
// accordingly. If the type assertion fails (should not happen in practice),
// it falls back to checking any-class bits.
func effectiveHasPerm(info fs.FileInfo, writeMask, execMask fs.FileMode, checkWrite, checkExec bool) bool {
func effectiveHasPerm(info fs.FileInfo, checkRead, checkWrite, checkExec bool) bool {
perm := info.Mode().Perm()

// Determine which permission class applies to the current process.
// Default to "other" bits and narrow down if we have Stat_t.
ownerBits := fs.FileMode(0007) // other bits by default
if st, ok := info.Sys().(*syscall.Stat_t); ok {
uid := os.Getuid()
if uid == 0 {
// Root bypasses read/write permission checks (CAP_DAC_OVERRIDE).
// Execute still requires at least one x bit to be set.
if checkExec && perm&0111 == 0 {
return false
}
return true
}
gid := os.Getgid()
switch {
case uid == 0:
// root can read/write anything; for execute, any x bit suffices.
ownerBits = 0777
case int(st.Uid) == uid:
ownerBits = 0700
case int(st.Gid) == gid:
Expand All @@ -70,14 +149,18 @@ func effectiveHasPerm(info fs.FileInfo, writeMask, execMask fs.FileMode, checkWr
}
}

if checkRead {
if perm&0444&ownerBits == 0 {
return false
Comment thread
matt-dz marked this conversation as resolved.
}
}
if checkWrite {
// Intersect the write mask with the applicable owner bits.
if perm&writeMask&ownerBits == 0 {
if perm&0222&ownerBits == 0 {
return false
}
}
if checkExec {
if perm&execMask&ownerBits == 0 {
if perm&0111&ownerBits == 0 {
return false
}
}
Expand Down
49 changes: 40 additions & 9 deletions allowedpaths/portable_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,44 @@ func FileIdentity(absPath string, _ fs.FileInfo, sandbox *Sandbox) (uint64, uint
return uint64(d.VolumeSerialNumber), uint64(d.FileIndexHigh)<<32 | uint64(d.FileIndexLow), true
}

// effectiveHasPerm checks whether the current process has the requested
// permission on Windows. Windows does not use Unix UID/GID permission classes,
// so we fall back to checking any-class bits (0222 / 0111) as before.
func effectiveHasPerm(info fs.FileInfo, writeMask, execMask fs.FileMode, checkWrite, checkExec bool) bool {
perm := info.Mode().Perm()
if checkWrite && perm&writeMask == 0 {
return false
}
return !(checkExec && perm&execMask == 0)
// accessCheck verifies the path is inside the sandbox via os.Root.Stat,
// then checks read permission by attempting to open the file through
// os.Root. This respects NTFS ACLs — the kernel denies the open if
// the current user lacks read permission. Named pipes cannot appear in
// regular directories on Windows, so this cannot block.
//
// - Read: verified by opening through os.Root (respects NTFS ACLs).
// - Write: checked via mode bits from Stat. On Windows,
// FILE_ATTRIBUTE_READONLY clears the write permission bits in
// Mode().Perm(), so mode-bit inspection is reliable.
// - Execute: Windows has no POSIX execute bits. The check always
// returns ErrPermission so that test -x behaves like a POSIX shell.
func (r *root) accessCheck(rel string, checkRead, checkWrite, checkExec bool) (fs.FileInfo, error) {
info, err := r.root.Stat(rel)
if err != nil {
return nil, err
}

// Windows has no POSIX execute bits — always deny execute checks.
if checkExec {
return info, os.ErrPermission
}

// On Windows, FILE_ATTRIBUTE_READONLY clears the write permission
// bits in Mode().Perm(). Check them for write access.
if checkWrite && info.Mode().Perm()&0200 == 0 {
return info, os.ErrPermission
}

if checkRead && !info.IsDir() {
f, err := r.root.OpenFile(rel, os.O_RDONLY, 0)
if err != nil {
return info, os.ErrPermission
}
if err := f.Close(); err != nil {
return info, err
}
}

return info, nil
}
52 changes: 18 additions & 34 deletions allowedpaths/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,18 @@ func (s *Sandbox) resolve(absPath string) (*os.Root, string, bool) {
// Access checks whether the resolved path is accessible with the given mode.
// All operations go through os.Root to stay within the sandbox.
// Mode: 0x04 = read, 0x02 = write, 0x01 = execute.
//
// On Unix, read permission for regular files is verified by attempting
// to open through os.Root with O_NONBLOCK (fd-relative openat, respects
// POSIX ACLs, never blocks on FIFOs). Metadata is obtained from the
// opened fd via fstat to eliminate TOCTOU between open and stat.
// For special files where open fails (e.g. sockets), and for write and
// execute checks, mode-bit inspection is used on the fd-relative Stat
// result. On Windows, the same OpenFile approach is used for read
// checks; write and execute checks are not performed.
//
// All operations are fd-relative through os.Root — no filesystem path is
// re-resolved through the mutable namespace after initial validation.
func (s *Sandbox) Access(path string, cwd string, mode uint32) error {
absPath := toAbs(path, cwd)

Expand All @@ -102,42 +114,14 @@ func (s *Sandbox) Access(path string, cwd string, mode uint32) error {
continue
}

// Open through os.Root once. This checks read access and gives
// us a file descriptor for an atomic Stat (no TOCTOU window).
f, err := ar.root.Open(rel)
// 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).
_, err = ar.accessCheck(rel, mode&0x04 != 0, mode&0x02 != 0, mode&0x01 != 0)
if err != nil {
if mode&0x04 != 0 && !IsErrIsDirectory(err) {
return PortablePathError(err)
}
// Read not requested, or target is a directory; fall back to Stat.
info, serr := ar.root.Stat(rel)
if serr != nil {
return PortablePathError(serr)
}
if !effectiveHasPerm(info, 0222, 0111, mode&0x02 != 0, mode&0x01 != 0) {
return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
}
return nil
}

// For write and execute, use mode bits from f.Stat() on the
// open fd — atomic, no TOCTOU window.
// The sandbox is read-only so -w is informational only.
// effectiveHasPerm checks the permission class (owner/group/other)
// that applies to the current process's effective UID/GID on Unix,
// rather than the union of all classes.
if mode&0x03 != 0 {
info, err := f.Stat()
if err != nil {
f.Close()
return PortablePathError(err)
}
if !effectiveHasPerm(info, 0222, 0111, mode&0x02 != 0, mode&0x01 != 0) {
f.Close()
return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
}
return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
}
f.Close()
return nil
}
return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission}
Expand Down
Loading
Loading