From 207b840c2acf9c281cea24d469f69ad3382e2149 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 14:44:46 -0400 Subject: [PATCH 01/13] feat(find): implement -execdir command {} \; MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add -execdir predicate to find, allowing execution of allowed builtin commands in each matched file's parent directory with ./basename as the argument. This is the safer alternative to -exec (no TOCTOU on the path, no path traversal in arguments) and -exec remains blocked. Key design decisions: - \; mode only — + (batch) mode rejected with clear error - AllowedCommands enforced at parse-time AND eval-time (defense-in-depth) - Sub-commands run through the interpreter with the same sandbox/AllowedPaths - No shell re-parsing — {} replacement produces discrete Go string args - ./basename prefix prevents argument injection via dash-prefixed filenames Co-Authored-By: Claude Opus 4.6 (1M context) --- SHELL_FEATURES.md | 2 +- allowedsymbols/symbols_builtins.go | 7 + builtins/builtins.go | 9 + builtins/find/builtin_find_pentest_test.go | 249 ++++++++++++++++++ builtins/find/eval.go | 57 +++- builtins/find/expr.go | 115 +++++--- builtins/find/expr_test.go | 66 ++++- builtins/find/find.go | 65 ++++- interp/runner_exec.go | 61 +++++ .../cmd/find/execdir/and_short_circuit.yaml | 21 ++ tests/scenarios/cmd/find/execdir/basic.yaml | 23 ++ .../cmd/find/execdir/batch_rejected.yaml | 17 ++ .../cmd/find/execdir/combined_name.yaml | 26 ++ .../cmd/find/execdir/command_not_allowed.yaml | 16 ++ .../cmd/find/execdir/embedded_braces.yaml | 17 ++ .../cmd/find/execdir/exec_still_blocked.yaml | 17 ++ .../find/execdir/execdir_exit_code_and.yaml | 21 ++ .../cmd/find/execdir/execdir_with_print.yaml | 20 ++ .../cmd/find/execdir/missing_command.yaml | 17 ++ .../cmd/find/execdir/missing_terminator.yaml | 17 ++ .../cmd/find/execdir/multiple_args.yaml | 19 ++ .../cmd/find/execdir/multiple_execdir.yaml | 20 ++ .../cmd/find/execdir/negated_execdir.yaml | 21 ++ .../cmd/find/execdir/nested_dir.yaml | 19 ++ .../cmd/find/execdir/or_fallback.yaml | 23 ++ .../cmd/find/execdir/prune_with_execdir.yaml | 22 ++ .../cmd/find/execdir/quit_after_execdir.yaml | 25 ++ .../cmd/find/execdir/returns_false.yaml | 18 ++ .../cmd/find/execdir/returns_true.yaml | 18 ++ .../cmd/find/execdir/suppresses_print.yaml | 23 ++ .../cmd/find/sandbox/blocked_execdir.yaml | 15 +- 31 files changed, 1003 insertions(+), 63 deletions(-) create mode 100644 builtins/find/builtin_find_pentest_test.go create mode 100644 tests/scenarios/cmd/find/execdir/and_short_circuit.yaml create mode 100644 tests/scenarios/cmd/find/execdir/basic.yaml create mode 100644 tests/scenarios/cmd/find/execdir/batch_rejected.yaml create mode 100644 tests/scenarios/cmd/find/execdir/combined_name.yaml create mode 100644 tests/scenarios/cmd/find/execdir/command_not_allowed.yaml create mode 100644 tests/scenarios/cmd/find/execdir/embedded_braces.yaml create mode 100644 tests/scenarios/cmd/find/execdir/exec_still_blocked.yaml create mode 100644 tests/scenarios/cmd/find/execdir/execdir_exit_code_and.yaml create mode 100644 tests/scenarios/cmd/find/execdir/execdir_with_print.yaml create mode 100644 tests/scenarios/cmd/find/execdir/missing_command.yaml create mode 100644 tests/scenarios/cmd/find/execdir/missing_terminator.yaml create mode 100644 tests/scenarios/cmd/find/execdir/multiple_args.yaml create mode 100644 tests/scenarios/cmd/find/execdir/multiple_execdir.yaml create mode 100644 tests/scenarios/cmd/find/execdir/negated_execdir.yaml create mode 100644 tests/scenarios/cmd/find/execdir/nested_dir.yaml create mode 100644 tests/scenarios/cmd/find/execdir/or_fallback.yaml create mode 100644 tests/scenarios/cmd/find/execdir/prune_with_execdir.yaml create mode 100644 tests/scenarios/cmd/find/execdir/quit_after_execdir.yaml create mode 100644 tests/scenarios/cmd/find/execdir/returns_false.yaml create mode 100644 tests/scenarios/cmd/find/execdir/returns_true.yaml create mode 100644 tests/scenarios/cmd/find/execdir/suppresses_print.yaml diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index a95e5ce7..41e03b68 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -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] [-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 +- ✅ `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`, `-execdir CMD {} \;`, `-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 ` --help` diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index 4c5fe1e7..b7dd9f2a 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -89,11 +89,15 @@ var builtinPerCommandSymbols = map[string][]string{ "math.MaxInt64", // integer constant; no side effects. "os.IsNotExist", // checks if error is "not exist"; pure function, no I/O. "os.PathError", // error type for path operations; pure type. + "path/filepath.Dir", // returns the directory component of a path; pure function, no I/O. + "path/filepath.IsAbs", // reports whether a path is absolute; pure function, no I/O. + "path/filepath.Join", // joins path elements; pure function, no I/O. "path/filepath.ToSlash", // converts OS path separators to forward slashes; pure function, no I/O. "strconv.Atoi", // string-to-int conversion; pure function, no I/O. "strconv.ErrRange", // sentinel error value for overflow; pure constant. "strconv.ParseInt", // string-to-int conversion; pure function, no I/O. "strconv.ParseUint", // string-to-unsigned-int conversion; pure function, no I/O. + "strings.Contains", // substring search for {} validation in -execdir; pure function, no I/O. "strings.HasPrefix", // pure function for prefix matching; no I/O. "strings.Split", // splits a string by separator into a slice; pure function, no I/O. "strings.ToLower", // converts string to lowercase; pure function, no I/O. @@ -454,6 +458,9 @@ var builtinAllowedSymbols = []string{ "os.IsNotExist", // checks if error is "not exist"; pure function, no I/O. "os.O_RDONLY", // read-only file flag constant; cannot open files by itself. "os.PathError", // error type for filesystem path errors; pure type, no I/O. + "path/filepath.Dir", // returns the directory component of a path; pure function, no I/O. + "path/filepath.IsAbs", // reports whether a path is absolute; pure function, no I/O. + "path/filepath.Join", // joins path elements; pure function, no I/O. "path/filepath.ToSlash", // converts OS path separators to forward slashes; pure function, no I/O. "regexp.Compile", // compiles a regular expression; pure function, no I/O. Uses RE2 engine (linear-time, no backtracking). "regexp.QuoteMeta", // escapes all special regex characters in a string; pure function, no I/O. diff --git a/builtins/builtins.go b/builtins/builtins.go index cdeed233..a31b2937 100644 --- a/builtins/builtins.go +++ b/builtins/builtins.go @@ -148,6 +148,15 @@ type CallContext struct { // current shell policy. Used by the help builtin to list only executable // commands. CommandAllowed func(name string) bool + + // WorkDir returns the shell's current working directory (absolute path). + // Used by builtins that need to compute absolute paths for sub-operations. + WorkDir func() string + + // RunCommand executes a builtin command within the shell's sandbox. + // dir overrides the working directory for path resolution. + // Returns the command's exit code. + RunCommand func(ctx context.Context, dir string, name string, args []string) (uint8, error) } // Out writes a string to stdout. diff --git a/builtins/find/builtin_find_pentest_test.go b/builtins/find/builtin_find_pentest_test.go new file mode 100644 index 00000000..a032ba11 --- /dev/null +++ b/builtins/find/builtin_find_pentest_test.go @@ -0,0 +1,249 @@ +// 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 find + +import ( + "bytes" + "context" + iofs "io/fs" + "testing" + "time" + + "github.com/DataDog/rshell/builtins" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// mockFileInfo is a minimal fs.FileInfo implementation for pentest helpers. +type mockFileInfo struct{} + +func (m *mockFileInfo) Name() string { return "mock" } +func (m *mockFileInfo) Size() int64 { return 0 } +func (m *mockFileInfo) Mode() iofs.FileMode { return 0o644 } +func (m *mockFileInfo) ModTime() time.Time { return time.Time{} } +func (m *mockFileInfo) IsDir() bool { return false } +func (m *mockFileInfo) Sys() any { return nil } + +// newPentestCallCtx returns a minimal CallContext suitable for pentest eval +// tests, wiring stdout and stderr to the provided buffers. +func newPentestCallCtx(stdout, stderr *bytes.Buffer) *builtins.CallContext { + return &builtins.CallContext{ + Stdout: stdout, + Stderr: stderr, + } +} + +// ---- Sandbox & AllowedCommands (CWE-284) ---- + +// TestFindExecDirCommandNotAllowed verifies that CommandAllowed is consulted +// at eval-time and that a blocked command causes a non-match with an error on +// stderr, without invoking RunCommand. +func TestFindExecDirCommandNotAllowed(t *testing.T) { + var stdout, stderr bytes.Buffer + runCalled := false + + callCtx := newPentestCallCtx(&stdout, &stderr) + callCtx.RunCommand = func(_ context.Context, _ string, _ string, _ []string) (uint8, error) { + runCalled = true + return 0, nil + } + callCtx.CommandAllowed = func(name string) bool { + return false // block everything + } + + ec := &evalContext{ + callCtx: callCtx, + ctx: context.Background(), + relPath: "somefile.txt", + info: &mockFileInfo{}, + } + e := &expr{ + kind: exprExecDir, + execCmd: "echo", + execArgs: []string{"{}"}, + } + + result := evalExecDir(ec, e) + + assert.False(t, result.matched, "blocked command must not match") + assert.True(t, ec.failed, "ec.failed must be set when command is blocked") + assert.False(t, runCalled, "RunCommand must not be invoked for a blocked command") + assert.Contains(t, stderr.String(), "not allowed", "stderr must mention 'not allowed'") +} + +// TestFindExecDirNilRunCommand verifies that a nil RunCommand is handled +// gracefully: evalExecDir must set ec.failed, write to stderr, and return a +// non-matched result rather than panicking. +func TestFindExecDirNilRunCommand(t *testing.T) { + var stdout, stderr bytes.Buffer + + callCtx := newPentestCallCtx(&stdout, &stderr) + // RunCommand deliberately left nil. + + ec := &evalContext{ + callCtx: callCtx, + ctx: context.Background(), + relPath: "somefile.txt", + info: &mockFileInfo{}, + } + e := &expr{ + kind: exprExecDir, + execCmd: "echo", + execArgs: []string{"{}"}, + } + + require.NotPanics(t, func() { + result := evalExecDir(ec, e) + assert.False(t, result.matched) + }) + assert.True(t, ec.failed, "ec.failed must be set when RunCommand is nil") + assert.NotEmpty(t, stderr.String(), "an error message must appear on stderr") +} + +// ---- Command Injection via Filenames (CWE-78) ---- + +// testExecDirFilename is a helper that verifies the argument passed to +// RunCommand for a given filename is exactly "./" + filename, ensuring that +// shell metacharacters in filenames are never interpreted. +func testExecDirFilename(t *testing.T, filename string) { + t.Helper() + + var stdout, stderr bytes.Buffer + var capturedArgs []string + + callCtx := newPentestCallCtx(&stdout, &stderr) + callCtx.RunCommand = func(_ context.Context, _ string, _ string, args []string) (uint8, error) { + capturedArgs = make([]string, len(args)) + copy(capturedArgs, args) + return 0, nil + } + callCtx.CommandAllowed = func(_ string) bool { return true } + + ec := &evalContext{ + callCtx: callCtx, + ctx: context.Background(), + relPath: filename, + info: &mockFileInfo{}, + } + e := &expr{ + kind: exprExecDir, + execCmd: "echo", + execArgs: []string{"{}"}, + } + + result := evalExecDir(ec, e) + + assert.True(t, result.matched, "echo should succeed and match") + require.Len(t, capturedArgs, 1, "exactly one argument must be passed to the command") + assert.Equal(t, "./"+filename, capturedArgs[0], + "filename %q must be passed as ./, not interpreted as shell syntax", filename) +} + +// TestFindExecDirSemicolonInFilename ensures a filename containing a semicolon +// is passed literally and cannot terminate the command or inject a new command. +func TestFindExecDirSemicolonInFilename(t *testing.T) { + testExecDirFilename(t, "hi; ls") +} + +// TestFindExecDirPipeInFilename ensures a pipe character in a filename does +// not result in stdout being piped to another process. +func TestFindExecDirPipeInFilename(t *testing.T) { + testExecDirFilename(t, "data|cat") +} + +// TestFindExecDirBacktickInFilename ensures backtick command substitution +// characters in filenames are not executed by the shell. +func TestFindExecDirBacktickInFilename(t *testing.T) { + testExecDirFilename(t, "`rm`") +} + +// TestFindExecDirDollarParenInFilename ensures $(...) command substitution +// syntax in filenames is not interpreted. +func TestFindExecDirDollarParenInFilename(t *testing.T) { + testExecDirFilename(t, "$(rm)") +} + +// TestFindExecDirNewlineInFilename ensures a filename containing a newline +// character is passed as a single argument without being split. +func TestFindExecDirNewlineInFilename(t *testing.T) { + testExecDirFilename(t, "file\nname") +} + +// TestFindExecDirSpacesInFilename ensures a filename with multiple spaces is +// passed as a single argument and not word-split into multiple tokens. +func TestFindExecDirSpacesInFilename(t *testing.T) { + testExecDirFilename(t, "a b c") +} + +// TestFindExecDirGlobCharsInFilename ensures glob metacharacters in filenames +// are not expanded by the shell. +func TestFindExecDirGlobCharsInFilename(t *testing.T) { + testExecDirFilename(t, "*.txt") +} + +// TestFindExecDirQuotesInFilename ensures single and double quote characters +// in filenames are passed literally without breaking argument quoting. +func TestFindExecDirQuotesInFilename(t *testing.T) { + testExecDirFilename(t, `it's a "test"`) +} + +// ---- Argument Injection (CWE-88) — ./ prefix prevents flag interpretation ---- + +// TestFindExecDirDashFilename verifies that a filename beginning with a dash +// (e.g. "-rf") is passed as "./-rf", preventing it from being interpreted as +// a command-line flag by the invoked program. +func TestFindExecDirDashFilename(t *testing.T) { + testExecDirFilename(t, "-rf") +} + +// TestFindExecDirDoubleDashFilename verifies that a filename beginning with +// double dashes (e.g. "--help") is passed as "./--help" to prevent flag +// injection. +func TestFindExecDirDoubleDashFilename(t *testing.T) { + testExecDirFilename(t, "--help") +} + +// ---- PATH Injection (CWE-426) ---- + +// TestFindExecDirNoPathLookup verifies that specifying a path-relative command +// such as "./malicious" via -execdir fails because path-traversal executables +// are not registered builtins and RunCommand is not provided. +func TestFindExecDirNoPathLookup(t *testing.T) { + var stdout, stderr bytes.Buffer + + callCtx := newPentestCallCtx(&stdout, &stderr) + // RunCommand is nil — simulates an environment where no external execution + // is available, exercising that ./malicious cannot be launched. + + ec := &evalContext{ + callCtx: callCtx, + ctx: context.Background(), + relPath: "target.txt", + info: &mockFileInfo{}, + } + e := &expr{ + kind: exprExecDir, + execCmd: "./malicious", + execArgs: []string{"{}"}, + } + + result := evalExecDir(ec, e) + + assert.False(t, result.matched, "./malicious must not be executed via path lookup") + assert.True(t, ec.failed, "ec.failed must be set") + assert.NotEmpty(t, stderr.String(), "an error must be reported on stderr") +} + +// ---- GTFOBins ---- + +// TestFindExecDirExecStillBlocked verifies that -exec remains in the blocked +// predicates list even after -execdir was introduced. This prevents -exec from +// being used as a higher-privilege GTFOBins escalation path. +func TestFindExecDirExecStillBlocked(t *testing.T) { + _, err := parseExpression([]string{"-exec", "id", ";"}) + require.Error(t, err, "-exec must still be blocked") + assert.Contains(t, err.Error(), "blocked", "error must mention 'blocked'") +} diff --git a/builtins/find/eval.go b/builtins/find/eval.go index 33dad83b..8b369726 100644 --- a/builtins/find/eval.go +++ b/builtins/find/eval.go @@ -23,17 +23,18 @@ type evalResult struct { // evalContext holds state needed during expression evaluation. type evalContext struct { - callCtx *builtins.CallContext - ctx context.Context - now time.Time - relPath string // path relative to starting point - info iofs.FileInfo // file info (lstat or stat depending on -L) - depth int // current depth - printPath string // path to print (includes starting point prefix) - newerCache map[string]time.Time // cached -newer reference file modtimes - newerErrors map[string]bool // tracks which -newer reference files failed to stat - followLinks bool // true when -L is active - failed bool // set by predicates that encounter errors + callCtx *builtins.CallContext + ctx context.Context + now time.Time + relPath string // path relative to starting point + info iofs.FileInfo // file info (lstat or stat depending on -L) + depth int // current depth + printPath string // path to print (includes starting point prefix) + newerCache map[string]time.Time // cached -newer reference file modtimes + newerErrors map[string]bool // tracks which -newer reference files failed to stat + followLinks bool // true when -L is active + failed bool // set by predicates that encounter errors + execDirParent string // absolute path of the file's parent directory for -execdir } // evaluate evaluates an expression tree against a file. If e is nil, returns @@ -105,6 +106,9 @@ func evaluate(ec *evalContext, e *expr) evalResult { // Use full 12-bit mode (including setuid/setgid/sticky), not just Perm() which is only 9 bits. return evalResult{matched: matchPerm(ec.info.Mode(), e.permVal, e.permCmp)} + case exprExecDir: + return evalExecDir(ec, e) + case exprQuit: return evalResult{matched: true, quit: true} @@ -262,3 +266,34 @@ func evalMmin(ec *evalContext, n int64, cmp cmpOp) bool { return mins == n } } + +// evalExecDir executes a command in the directory of each matched file. +// The filename is passed as ./basename, preventing leading-dash interpretation. +func evalExecDir(ec *evalContext, e *expr) evalResult { + if ec.callCtx.RunCommand == nil { + ec.callCtx.Errf("find: -execdir: command execution not available\n") + ec.failed = true + return evalResult{} + } + if ec.callCtx.CommandAllowed != nil && !ec.callCtx.CommandAllowed(e.execCmd) { + ec.callCtx.Errf("find: -execdir: '%s': command not allowed\n", e.execCmd) + ec.failed = true + return evalResult{} + } + replacement := "./" + baseName(ec.relPath) + args := make([]string, len(e.execArgs)) + for i, a := range e.execArgs { + if a == "{}" { + args[i] = replacement + } else { + args[i] = a + } + } + exitCode, err := ec.callCtx.RunCommand(ec.ctx, ec.execDirParent, e.execCmd, args) + if err != nil { + ec.callCtx.Errf("find: '%s': %s\n", e.execCmd, err) + ec.failed = true + return evalResult{} + } + return evalResult{matched: exitCode == 0} +} diff --git a/builtins/find/expr.go b/builtins/find/expr.go index 01e66c64..1703eb2b 100644 --- a/builtins/find/expr.go +++ b/builtins/find/expr.go @@ -24,26 +24,27 @@ const ( type exprKind int const ( - exprName exprKind = iota // -name pattern - exprIName // -iname pattern - exprPath // -path pattern - exprIPath // -ipath pattern - exprType // -type c - exprSize // -size n[cwbkMG] - exprEmpty // -empty - exprNewer // -newer file - exprMtime // -mtime n - exprMmin // -mmin n - exprPerm // -perm mode - exprQuit // -quit - exprPrint // -print - exprPrint0 // -print0 - exprPrune // -prune - exprTrue // -true - exprFalse // -false - exprAnd // expr -a expr or expr expr (implicit) - exprOr // expr -o expr - exprNot // ! expr or -not expr + exprName exprKind = iota // -name pattern + exprIName // -iname pattern + exprPath // -path pattern + exprIPath // -ipath pattern + exprType // -type c + exprSize // -size n[cwbkMG] + exprEmpty // -empty + exprNewer // -newer file + exprMtime // -mtime n + exprMmin // -mmin n + exprPerm // -perm mode + exprQuit // -quit + exprPrint // -print + exprPrint0 // -print0 + exprPrune // -prune + exprTrue // -true + exprFalse // -false + exprExecDir // -execdir command {} ; + exprAnd // expr -a expr or expr expr (implicit) + exprOr // expr -o expr + exprNot // ! expr or -not expr ) // cmpOp represents a comparison operator for numeric predicates. @@ -77,16 +78,18 @@ type sizeUnit struct { // expr is a node in the find expression AST. type expr struct { - kind exprKind - strVal string // pattern for name/iname/path/ipath, type char, file path for newer/samefile, format for printf - sizeVal sizeUnit // for -size - numVal int64 // for -mtime, -mmin, -atime, -amin, -ctime, -cmin, -uid, -gid, -links, -inum - numCmp cmpOp // comparison operator for numeric predicates - permVal uint32 // for -perm: permission bits - permCmp byte // for -perm: '=' exact, '-' all bits, '/' any bit - left *expr // for and/or - right *expr // for and/or - operand *expr // for not + kind exprKind + strVal string // pattern for name/iname/path/ipath, type char, file path for newer/samefile, format for printf + sizeVal sizeUnit // for -size + numVal int64 // for -mtime, -mmin, -atime, -amin, -ctime, -cmin, -uid, -gid, -links, -inum + numCmp cmpOp // comparison operator for numeric predicates + permVal uint32 // for -perm: permission bits + permCmp byte // for -perm: '=' exact, '-' all bits, '/' any bit + execCmd string // command name for -execdir + execArgs []string // argument template for -execdir (each element is literal or "{}") + left *expr // for and/or + right *expr // for and/or + operand *expr // for not } // isAction returns true if this expression is an output action. @@ -94,7 +97,7 @@ type expr struct { // control flow (handled at evaluation time by checking quit before // implicit print) and does not affect the implicit-print decision. func (e *expr) isAction() bool { - return e.kind == exprPrint || e.kind == exprPrint0 + return e.kind == exprPrint || e.kind == exprPrint0 || e.kind == exprExecDir } // hasAction checks if any node in the expression tree is an action. @@ -133,7 +136,6 @@ var errHelpRequested = errors.New("find: help requested") // blocked predicates that are forbidden for sandbox safety. var blockedPredicates = map[string]string{ "-exec": "arbitrary command execution is blocked", - "-execdir": "arbitrary command execution is blocked", "-delete": "file deletion is blocked", "-ok": "interactive execution is blocked", "-okdir": "interactive execution is blocked", @@ -342,6 +344,8 @@ func (p *parser) parsePrimary() (*expr, error) { return p.parseDepthOption(true) case "-mindepth": return p.parseDepthOption(false) + case "-execdir": + return p.parseExecDirPredicate() case "-true": return &expr{kind: exprTrue}, nil case "-false": @@ -679,6 +683,51 @@ func parseSymbolicMode(s string) (uint64, error) { return mode, nil } +// parseExecDirPredicate parses -execdir command [args...] ; +// Only \; mode is supported. + (batch mode) is rejected with a clear error. +func (p *parser) parseExecDirPredicate() (*expr, error) { + if p.pos >= len(p.args) { + return nil, errors.New("find: missing argument to '-execdir'") + } + + // Collect tokens until ";" or "+" terminator. + startPos := p.pos + for p.pos < len(p.args) { + tok := p.args[p.pos] + if tok == ";" { + break + } + if tok == "+" { + return nil, errors.New("find: -execdir ... + (batch mode) is not yet supported") + } + p.pos++ + } + + if p.pos >= len(p.args) { + return nil, errors.New("find: missing argument to '-execdir'") + } + + // Consume the ";" terminator. + p.pos++ + + tokens := p.args[startPos : p.pos-1] + if len(tokens) == 0 { + return nil, errors.New("find: missing argument to '-execdir'") + } + + cmd := tokens[0] + args := tokens[1:] + + // Validate {} usage: reject {}-embedded args like {}.bak. + for _, a := range args { + if a != "{}" && strings.Contains(a, "{}") { + return nil, fmt.Errorf("find: -execdir: '{}' must be a standalone argument") + } + } + + return &expr{kind: exprExecDir, execCmd: cmd, execArgs: args}, nil +} + // parseSize parses a -size argument like "+10k", "-5M", "100c". func parseSize(s string) (sizeUnit, error) { if len(s) == 0 { @@ -747,6 +796,8 @@ func (k exprKind) String() string { return "-mmin" case exprPerm: return "-perm" + case exprExecDir: + return "-execdir" case exprQuit: return "-quit" case exprPrint: diff --git a/builtins/find/expr_test.go b/builtins/find/expr_test.go index a8a554f7..ecb1024c 100644 --- a/builtins/find/expr_test.go +++ b/builtins/find/expr_test.go @@ -211,7 +211,7 @@ func TestParseTypePredicate(t *testing.T) { // TestParseBlockedPredicates verifies all dangerous predicates are blocked. func TestParseBlockedPredicates(t *testing.T) { blocked := []string{ - "-exec", "-execdir", "-delete", "-ok", "-okdir", + "-exec", "-delete", "-ok", "-okdir", "-fls", "-fprint", "-fprint0", "-fprintf", "-regex", "-iregex", } @@ -219,7 +219,7 @@ func TestParseBlockedPredicates(t *testing.T) { t.Run(pred, func(t *testing.T) { // Blocked predicates that take an argument need one to not fail with "missing argument". args := []string{pred} - if pred == "-exec" || pred == "-execdir" || pred == "-ok" || pred == "-okdir" { + if pred == "-exec" || pred == "-ok" || pred == "-okdir" { args = append(args, "cmd", ";") } _, err := parseExpression(args) @@ -349,6 +349,68 @@ func TestParseHelpRequested(t *testing.T) { }) } +// TestParseExecDirBasic verifies that -execdir echo {} ; is parsed into an +// exprExecDir node with the correct execCmd and execArgs. +func TestParseExecDirBasic(t *testing.T) { + pr, err := parseExpression([]string{"-execdir", "echo", "{}", ";"}) + require.NoError(t, err) + require.NotNil(t, pr.expr) + assert.Equal(t, exprExecDir, pr.expr.kind) + assert.Equal(t, "echo", pr.expr.execCmd) + assert.Equal(t, []string{"{}"}, pr.expr.execArgs) +} + +// TestParseExecDirMultipleArgs verifies that all positional arguments +// surrounding {} are preserved in execArgs. +func TestParseExecDirMultipleArgs(t *testing.T) { + pr, err := parseExpression([]string{"-execdir", "echo", "before", "{}", "after", ";"}) + require.NoError(t, err) + require.NotNil(t, pr.expr) + assert.Equal(t, exprExecDir, pr.expr.kind) + assert.Equal(t, "echo", pr.expr.execCmd) + assert.Equal(t, []string{"before", "{}", "after"}, pr.expr.execArgs) +} + +// TestParseExecDirMissingTerminator verifies that -execdir without a ; or + +// terminator returns an error mentioning "missing argument". +func TestParseExecDirMissingTerminator(t *testing.T) { + _, err := parseExpression([]string{"-execdir", "echo", "{}"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "missing argument") +} + +// TestParseExecDirMissingCommand verifies that -execdir ; (no command token) +// returns an error mentioning "missing argument". +func TestParseExecDirMissingCommand(t *testing.T) { + _, err := parseExpression([]string{"-execdir", ";"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "missing argument") +} + +// TestParseExecDirBatchRejected verifies that the + batch-mode terminator is +// explicitly rejected with an error mentioning "batch mode". +func TestParseExecDirBatchRejected(t *testing.T) { + _, err := parseExpression([]string{"-execdir", "echo", "{}", "+"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "batch mode") +} + +// TestParseExecDirEmbeddedBraces verifies that {} embedded within a larger +// argument token (e.g. {}.bak) is rejected with an error mentioning +// "standalone". +func TestParseExecDirEmbeddedBraces(t *testing.T) { + _, err := parseExpression([]string{"-execdir", "echo", "{}.bak", ";"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "standalone") +} + +// TestParseExecDirIsAction verifies that isAction() returns true for an +// exprExecDir node, ensuring -execdir suppresses implicit -print. +func TestParseExecDirIsAction(t *testing.T) { + e := &expr{kind: exprExecDir} + assert.True(t, e.isAction(), "exprExecDir must be reported as an action") +} + // TestParseExpressionLimits verifies AST depth and node limits. func TestParseExpressionLimits(t *testing.T) { t.Run("depth limit", func(t *testing.T) { diff --git a/builtins/find/find.go b/builtins/find/find.go index 1e48eaf1..3deaba96 100644 --- a/builtins/find/find.go +++ b/builtins/find/find.go @@ -36,6 +36,7 @@ // -print — print path followed by newline // -print0 — print path followed by NUL // -prune — skip directory subtree +// -execdir CMD {} ; — run CMD in file's directory with ./basename // -quit — exit immediately // -true — always true // -false — always false @@ -49,7 +50,7 @@ // // Blocked predicates (sandbox safety): // -// -exec, -execdir, -delete, -ok, -okdir — execution/deletion +// -exec, -delete, -ok, -okdir — execution/deletion // -fls, -fprint, -fprint0, -fprintf — file writes // -regex, -iregex — ReDoS risk // @@ -175,6 +176,14 @@ optLoop: minDepth = 0 } + // Post-parse validation: check -execdir commands are allowed. + for _, cmd := range collectExecDirCmds(expression) { + if callCtx.CommandAllowed != nil && !callCtx.CommandAllowed(cmd) { + callCtx.Errf("find: -execdir: '%s': command not allowed\n", cmd) + return builtins.Result{Code: 1} + } + } + // If no explicit action, add implicit -print. implicitPrint := expression == nil || !hasAction(expression) @@ -219,6 +228,12 @@ optLoop: now := callCtx.Now + // Resolve working directory for -execdir path computation. + var workDir string + if callCtx.WorkDir != nil { + workDir = callCtx.WorkDir() + } + // GNU find treats a missing -newer reference as a fatal argument error // and produces no result set, so skip the walk entirely. if !failed { @@ -242,6 +257,7 @@ optLoop: minDepth: minDepth, now: now, eagerNewerErrors: eagerNewerErrors, + workDir: workDir, }) if wr.failed { failed = true @@ -296,6 +312,7 @@ func printHelp(callCtx *builtins.CallContext) { callCtx.Out("Actions:\n") callCtx.Out(" -print Print path followed by newline.\n") callCtx.Out(" -print0 Print path followed by NUL.\n") + callCtx.Out(" -execdir CMD [ARG]... ; Run CMD in file's directory (./basename).\n") callCtx.Out(" -prune Skip directory subtree.\n") callCtx.Out(" -quit Exit immediately.\n\n") callCtx.Out("Operators:\n") @@ -304,7 +321,7 @@ func printHelp(callCtx *builtins.CallContext) { callCtx.Out(" EXPR -a EXPR / EXPR -and EXPR Conjunction (implicit).\n") callCtx.Out(" EXPR -o EXPR / EXPR -or EXPR Disjunction.\n\n") callCtx.Out("Blocked predicates [sandbox]:\n") - callCtx.Out(" -exec, -execdir, -delete, -ok, -okdir Execution/deletion.\n") + callCtx.Out(" -exec, -delete, -ok, -okdir Execution/deletion.\n") callCtx.Out(" -fls, -fprint, -fprint0, -fprintf File writes.\n") callCtx.Out(" -regex, -iregex ReDoS risk.\n") } @@ -318,6 +335,7 @@ type walkOptions struct { minDepth int now time.Time eagerNewerErrors map[string]bool + workDir string // absolute working directory for -execdir path resolution } // walkResult holds the outcome of a walk operation. @@ -415,17 +433,25 @@ func walkPath( // processEntry evaluates the expression for a single file entry. // Returns (prune, quit). processEntry := func(path string, info iofs.FileInfo, depth int) (bool, bool) { + // Compute parent directory for -execdir. + absPath := path + if !filepath.IsAbs(absPath) { + absPath = filepath.Join(opts.workDir, absPath) + } + execDirParent := filepath.Dir(absPath) + ec := &evalContext{ - callCtx: callCtx, - ctx: ctx, - now: now, - relPath: path, - info: info, - depth: depth, - printPath: path, - newerCache: newerCache, - newerErrors: newerErrors, - followLinks: opts.followLinks, + callCtx: callCtx, + ctx: ctx, + now: now, + relPath: path, + info: info, + depth: depth, + printPath: path, + newerCache: newerCache, + newerErrors: newerErrors, + followLinks: opts.followLinks, + execDirParent: execDirParent, } prune := false @@ -564,6 +590,21 @@ func walkPath( return walkResult{failed: failed, quit: quit} } +// collectExecDirCmds walks the expression tree and returns all -execdir command names. +func collectExecDirCmds(e *expr) []string { + if e == nil { + return nil + } + var cmds []string + if e.kind == exprExecDir { + cmds = append(cmds, e.execCmd) + } + cmds = append(cmds, collectExecDirCmds(e.left)...) + cmds = append(cmds, collectExecDirCmds(e.right)...) + cmds = append(cmds, collectExecDirCmds(e.operand)...) + return cmds +} + // collectNewerRefs walks the expression tree and returns all -newer reference paths. func collectNewerRefs(e *expr) []string { if e == nil { diff --git a/interp/runner_exec.go b/interp/runner_exec.go index 7ccdb0fb..c8c60254 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -270,6 +270,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { Stderr: r.stderr, InLoop: r.inLoop, LastExitCode: r.lastExit.code, + WorkDir: func() string { + return HandlerCtx(r.handlerCtx(ctx, todoPos)).Dir + }, OpenFile: func(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) { return r.open(ctx, path, flags, mode, false) }, @@ -310,6 +313,64 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { CommandAllowed: func(cmdName string) bool { return r.allowAllCommands || cmdName == "help" || r.allowedCommands[cmdName] }, + RunCommand: func(ctx context.Context, dir string, cmdName string, cmdArgs []string) (uint8, error) { + if !r.allowAllCommands && cmdName != "help" && !r.allowedCommands[cmdName] { + return 127, fmt.Errorf("%s: command not allowed", cmdName) + } + fn, ok := builtins.Lookup(cmdName) + if !ok { + return 127, fmt.Errorf("%s: command not found", cmdName) + } + child := &builtins.CallContext{ + Stdout: r.stdout, + Stderr: r.stderr, + OpenFile: func(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) { + return r.sandbox.Open(path, dir, flags, mode) + }, + ReadDir: func(ctx context.Context, path string) ([]fs.DirEntry, error) { + return r.sandbox.ReadDir(path, dir) + }, + OpenDir: func(ctx context.Context, path string) (fs.ReadDirFile, error) { + return r.sandbox.OpenDir(path, dir) + }, + IsDirEmpty: func(ctx context.Context, path string) (bool, error) { + return r.sandbox.IsDirEmpty(path, dir) + }, + ReadDirLimited: func(ctx context.Context, path string, offset, maxRead int) ([]fs.DirEntry, bool, error) { + return r.sandbox.ReadDirLimited(path, dir, offset, maxRead) + }, + StatFile: func(ctx context.Context, path string) (fs.FileInfo, error) { + return r.sandbox.Stat(path, dir) + }, + LstatFile: func(ctx context.Context, path string) (fs.FileInfo, error) { + return r.sandbox.Lstat(path, dir) + }, + AccessFile: func(ctx context.Context, path string, mode uint32) error { + return r.sandbox.Access(path, dir, mode) + }, + PortableErr: allowedpaths.PortableErrMsg, + Now: r.startTime, + FileIdentity: func(path string, info fs.FileInfo) (builtins.FileID, bool) { + absPath := path + if !filepath.IsAbs(absPath) { + absPath = filepath.Join(dir, absPath) + } + dev, ino, ok := allowedpaths.FileIdentity(absPath, info, r.sandbox) + if !ok { + return builtins.FileID{}, false + } + return builtins.FileID{Dev: dev, Ino: ino}, true + }, + CommandAllowed: func(n string) bool { + return r.allowAllCommands || n == "help" || r.allowedCommands[n] + }, + } + if r.stdin != nil { + child.Stdin = r.stdin + } + result := fn(ctx, child, cmdArgs) + return result.Code, nil + }, } if r.stdin != nil { // do not assign a typed nil into the io.Reader interface call.Stdin = r.stdin diff --git a/tests/scenarios/cmd/find/execdir/and_short_circuit.yaml b/tests/scenarios/cmd/find/execdir/and_short_circuit.yaml new file mode 100644 index 00000000..20e80c34 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/and_short_circuit.yaml @@ -0,0 +1,21 @@ +description: find -false -execdir echo {} \; never runs echo because AND short-circuits on -false. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 + - path: dir/b.txt + content: "b" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -false -execdir echo {} \; +expect: + stdout: "" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/basic.yaml b/tests/scenarios/cmd/find/execdir/basic.yaml new file mode 100644 index 00000000..b01735e9 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/basic.yaml @@ -0,0 +1,23 @@ +description: find -execdir echo {} \; runs echo in the matched file's directory, printing ./basename. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 + - path: dir/b.txt + content: "b" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -name '*.txt' -execdir echo {} \; +expect: + stdout_unordered: |+ + ./a.txt + ./b.txt + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/batch_rejected.yaml b/tests/scenarios/cmd/find/execdir/batch_rejected.yaml new file mode 100644 index 00000000..ec290b32 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/batch_rejected.yaml @@ -0,0 +1,17 @@ +description: find -execdir with + terminator is rejected; only \; is supported. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -execdir echo {} + +expect: + stderr_contains: ["+"] + exit_code: 1 diff --git a/tests/scenarios/cmd/find/execdir/combined_name.yaml b/tests/scenarios/cmd/find/execdir/combined_name.yaml new file mode 100644 index 00000000..69b364e3 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/combined_name.yaml @@ -0,0 +1,26 @@ +description: find -name filter combined with -execdir only runs echo on matching files. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 + - path: dir/b.go + content: "b" + chmod: 0644 + - path: dir/c.txt + content: "c" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -name '*.txt' -execdir echo {} \; +expect: + stdout_unordered: |+ + ./a.txt + ./c.txt + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/command_not_allowed.yaml b/tests/scenarios/cmd/find/execdir/command_not_allowed.yaml new file mode 100644 index 00000000..e08a52f7 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/command_not_allowed.yaml @@ -0,0 +1,16 @@ +description: find -execdir with a command absent from allowed_commands is rejected at parse time. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + script: |+ + find dir -execdir echo {} \; +expect: + stderr_contains: ["not allowed"] + exit_code: 1 diff --git a/tests/scenarios/cmd/find/execdir/embedded_braces.yaml b/tests/scenarios/cmd/find/execdir/embedded_braces.yaml new file mode 100644 index 00000000..ffe228d7 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/embedded_braces.yaml @@ -0,0 +1,17 @@ +description: find -execdir rejects embedded {} like {}.bak in arguments. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -execdir echo {}.bak \; +expect: + stderr_contains: ["{}"] + exit_code: 1 diff --git a/tests/scenarios/cmd/find/execdir/exec_still_blocked.yaml b/tests/scenarios/cmd/find/execdir/exec_still_blocked.yaml new file mode 100644 index 00000000..3916a6d5 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/exec_still_blocked.yaml @@ -0,0 +1,17 @@ +description: find -exec is still blocked even when -execdir is available. +skip_assert_against_bash: true # intentional: bash allows -exec; rshell blocks it +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -exec echo {} \; +expect: + stderr_contains: ["blocked"] + exit_code: 1 diff --git a/tests/scenarios/cmd/find/execdir/execdir_exit_code_and.yaml b/tests/scenarios/cmd/find/execdir/execdir_exit_code_and.yaml new file mode 100644 index 00000000..c967c348 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/execdir_exit_code_and.yaml @@ -0,0 +1,21 @@ +description: find -execdir false {} \; -print — false return suppresses -print for each file via AND short-circuit. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 + - path: dir/b.txt + content: "b" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:false + script: |+ + find dir -type f -execdir false {} \; -print +expect: + stdout: "" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/execdir_with_print.yaml b/tests/scenarios/cmd/find/execdir/execdir_with_print.yaml new file mode 100644 index 00000000..9fff1f08 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/execdir_with_print.yaml @@ -0,0 +1,20 @@ +description: find -execdir echo {} \; -print — both the execdir echo and -print run for each file. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -name '*.txt' -execdir echo {} \; -print +expect: + stdout: |+ + ./a.txt + dir/a.txt + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/missing_command.yaml b/tests/scenarios/cmd/find/execdir/missing_command.yaml new file mode 100644 index 00000000..d8afa21b --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/missing_command.yaml @@ -0,0 +1,17 @@ +description: find -execdir with \; immediately after flag (no command) is a parse error. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -execdir \; +expect: + stderr_contains: ["-execdir"] + exit_code: 1 diff --git a/tests/scenarios/cmd/find/execdir/missing_terminator.yaml b/tests/scenarios/cmd/find/execdir/missing_terminator.yaml new file mode 100644 index 00000000..b344c52b --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/missing_terminator.yaml @@ -0,0 +1,17 @@ +description: find -execdir without a \; terminator is a parse error. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -execdir echo {} +expect: + stderr_contains: ["-execdir"] + exit_code: 1 diff --git a/tests/scenarios/cmd/find/execdir/multiple_args.yaml b/tests/scenarios/cmd/find/execdir/multiple_args.yaml new file mode 100644 index 00000000..27ed43ea --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/multiple_args.yaml @@ -0,0 +1,19 @@ +description: find -execdir preserves argument order when extra args surround {}. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -name '*.txt' -execdir echo before {} after \; +expect: + stdout: |+ + before ./a.txt after + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/multiple_execdir.yaml b/tests/scenarios/cmd/find/execdir/multiple_execdir.yaml new file mode 100644 index 00000000..14c71d01 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/multiple_execdir.yaml @@ -0,0 +1,20 @@ +description: find with two consecutive -execdir actions — both run for each matched file. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -name '*.txt' -execdir echo first {} \; -execdir echo second {} \; +expect: + stdout: |+ + first ./a.txt + second ./a.txt + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/negated_execdir.yaml b/tests/scenarios/cmd/find/execdir/negated_execdir.yaml new file mode 100644 index 00000000..5de760c7 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/negated_execdir.yaml @@ -0,0 +1,21 @@ +description: find ! -execdir false {} \; — negation inverts false to true, but -execdir as an action suppresses implicit -print so no output. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 + - path: dir/b.txt + content: "b" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:false + script: |+ + find dir -type f ! -execdir false {} \; +expect: + stdout: "" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/nested_dir.yaml b/tests/scenarios/cmd/find/execdir/nested_dir.yaml new file mode 100644 index 00000000..e997eb31 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/nested_dir.yaml @@ -0,0 +1,19 @@ +description: find -execdir replaces {} with ./basename even for files in nested subdirectories. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/sub/deep.txt + content: "deep" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -name '*.txt' -execdir echo {} \; +expect: + stdout: |+ + ./deep.txt + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/or_fallback.yaml b/tests/scenarios/cmd/find/execdir/or_fallback.yaml new file mode 100644 index 00000000..b412c355 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/or_fallback.yaml @@ -0,0 +1,23 @@ +description: find OR expression — .log files get execdir echo, .txt files fall through to -print. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/app.log + content: "log" + chmod: 0644 + - path: dir/readme.txt + content: "txt" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -type f \( -name '*.log' -execdir echo found {} \; -o -name '*.txt' -print \) +expect: + stdout_unordered: |+ + found ./app.log + dir/readme.txt + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/prune_with_execdir.yaml b/tests/scenarios/cmd/find/execdir/prune_with_execdir.yaml new file mode 100644 index 00000000..0f9ddcf1 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/prune_with_execdir.yaml @@ -0,0 +1,22 @@ +description: find -prune skips a subdirectory; -execdir only runs on files outside the pruned subtree. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/sub/hidden.txt + content: "hidden" + chmod: 0644 + - path: dir/visible.txt + content: "visible" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -type d -name sub -prune -o -type f -execdir echo {} \; +expect: + stdout: |+ + ./visible.txt + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/quit_after_execdir.yaml b/tests/scenarios/cmd/find/execdir/quit_after_execdir.yaml new file mode 100644 index 00000000..535da866 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/quit_after_execdir.yaml @@ -0,0 +1,25 @@ +description: find -execdir echo {} \; -quit runs execdir exactly once then stops. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 + - path: dir/b.txt + content: "b" + chmod: 0644 + - path: dir/c.txt + content: "c" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -name '*.txt' -execdir echo {} \; -quit +expect: + stdout_contains: + - ".txt" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/returns_false.yaml b/tests/scenarios/cmd/find/execdir/returns_false.yaml new file mode 100644 index 00000000..66e2d9f5 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/returns_false.yaml @@ -0,0 +1,18 @@ +description: find -execdir false {} \; exits 0 even though false returns non-zero (find reports errors, not predicate results). +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:false + script: |+ + find dir -type f -execdir false {} \; +expect: + stdout: "" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/returns_true.yaml b/tests/scenarios/cmd/find/execdir/returns_true.yaml new file mode 100644 index 00000000..d5d7de9a --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/returns_true.yaml @@ -0,0 +1,18 @@ +description: find -execdir true {} \; succeeds with exit code 0. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:true + script: |+ + find dir -type f -execdir true {} \; +expect: + stdout: "" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/suppresses_print.yaml b/tests/scenarios/cmd/find/execdir/suppresses_print.yaml new file mode 100644 index 00000000..7a8434ab --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/suppresses_print.yaml @@ -0,0 +1,23 @@ +description: find -execdir suppresses the implicit -print action. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 + - path: dir/b.txt + content: "b" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -type f -execdir echo {} \; +expect: + stdout_unordered: |+ + ./a.txt + ./b.txt + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/sandbox/blocked_execdir.yaml b/tests/scenarios/cmd/find/sandbox/blocked_execdir.yaml index e3ea2fdc..391c93ef 100644 --- a/tests/scenarios/cmd/find/sandbox/blocked_execdir.yaml +++ b/tests/scenarios/cmd/find/sandbox/blocked_execdir.yaml @@ -1,5 +1,5 @@ -description: find -execdir is blocked for sandbox safety. -skip_assert_against_bash: true # intentional: bash allows -execdir; rshell blocks it +description: find -execdir runs allowed commands in the file's directory. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands setup: files: - path: dummy.txt @@ -7,8 +7,13 @@ setup: chmod: 0644 input: allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo script: |+ - find . -execdir echo {} \; + find . -name 'dummy.txt' -execdir echo {} \; expect: - stderr_contains: ["blocked"] - exit_code: 1 + stdout: |+ + ./dummy.txt + stderr: "" + exit_code: 0 From 3fe66389ced8f9853f3db974fece01f6817a555c Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 15:06:20 -0400 Subject: [PATCH 02/13] fix(find): compute -execdir parent without cleaning '.' and '..' filepath.Join cleans its result, so Join(cwd, ".") = cwd and Dir(cwd) points one level above the search root. Replace with joinPath (simple concatenation) so "." is preserved as the last component and filepath.Dir correctly strips it. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_builtins.go | 2 - builtins/find/builtin_find_pentest_test.go | 50 +++++++++++++++++++ builtins/find/find.go | 6 ++- .../cmd/find/execdir/dot_start_path.yaml | 18 +++++++ .../find/execdir/dot_start_path_nested.yaml | 18 +++++++ .../find/execdir/named_dir_start_path.yaml | 18 +++++++ 6 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 tests/scenarios/cmd/find/execdir/dot_start_path.yaml create mode 100644 tests/scenarios/cmd/find/execdir/dot_start_path_nested.yaml create mode 100644 tests/scenarios/cmd/find/execdir/named_dir_start_path.yaml diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index b7dd9f2a..fdca8855 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -91,7 +91,6 @@ var builtinPerCommandSymbols = map[string][]string{ "os.PathError", // error type for path operations; pure type. "path/filepath.Dir", // returns the directory component of a path; pure function, no I/O. "path/filepath.IsAbs", // reports whether a path is absolute; pure function, no I/O. - "path/filepath.Join", // joins path elements; pure function, no I/O. "path/filepath.ToSlash", // converts OS path separators to forward slashes; pure function, no I/O. "strconv.Atoi", // string-to-int conversion; pure function, no I/O. "strconv.ErrRange", // sentinel error value for overflow; pure constant. @@ -460,7 +459,6 @@ var builtinAllowedSymbols = []string{ "os.PathError", // error type for filesystem path errors; pure type, no I/O. "path/filepath.Dir", // returns the directory component of a path; pure function, no I/O. "path/filepath.IsAbs", // reports whether a path is absolute; pure function, no I/O. - "path/filepath.Join", // joins path elements; pure function, no I/O. "path/filepath.ToSlash", // converts OS path separators to forward slashes; pure function, no I/O. "regexp.Compile", // compiles a regular expression; pure function, no I/O. Uses RE2 engine (linear-time, no backtracking). "regexp.QuoteMeta", // escapes all special regex characters in a string; pure function, no I/O. diff --git a/builtins/find/builtin_find_pentest_test.go b/builtins/find/builtin_find_pentest_test.go index a032ba11..a8db0757 100644 --- a/builtins/find/builtin_find_pentest_test.go +++ b/builtins/find/builtin_find_pentest_test.go @@ -237,6 +237,56 @@ func TestFindExecDirNoPathLookup(t *testing.T) { assert.NotEmpty(t, stderr.String(), "an error must be reported on stderr") } +// ---- execDirParent plumbing (regression for filepath.Join cleaning bug) ---- + +// TestFindExecDirParentDir verifies that evalExecDir passes ec.execDirParent +// as the dir argument to RunCommand. This is the plumbing test — it ensures +// the value computed by processEntry is forwarded faithfully. +func TestFindExecDirParentDir(t *testing.T) { + tests := []struct { + name string + relPath string + execDirParent string + }{ + {"file in cwd", "file.txt", "/cwd"}, + {"file in subdir", "sub/file.txt", "/cwd/sub"}, + {"dot start path", ".", "/cwd"}, + {"named dir start path", "dir", "/cwd"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var stdout, stderr bytes.Buffer + var capturedDir string + + callCtx := newPentestCallCtx(&stdout, &stderr) + callCtx.RunCommand = func(_ context.Context, dir string, _ string, _ []string) (uint8, error) { + capturedDir = dir + return 0, nil + } + callCtx.CommandAllowed = func(_ string) bool { return true } + + ec := &evalContext{ + callCtx: callCtx, + ctx: context.Background(), + relPath: tt.relPath, + info: &mockFileInfo{}, + execDirParent: tt.execDirParent, + } + e := &expr{ + kind: exprExecDir, + execCmd: "echo", + execArgs: []string{"{}"}, + } + + result := evalExecDir(ec, e) + + assert.True(t, result.matched) + assert.Equal(t, tt.execDirParent, capturedDir, + "RunCommand dir must equal ec.execDirParent") + }) + } +} + // ---- GTFOBins ---- // TestFindExecDirExecStillBlocked verifies that -exec remains in the blocked diff --git a/builtins/find/find.go b/builtins/find/find.go index 3deaba96..c247ff5e 100644 --- a/builtins/find/find.go +++ b/builtins/find/find.go @@ -434,9 +434,13 @@ func walkPath( // Returns (prune, quit). processEntry := func(path string, info iofs.FileInfo, depth int) (bool, bool) { // Compute parent directory for -execdir. + // Use joinPath (not filepath.Join) to preserve '.' and '..' components. + // filepath.Join cleans lexically, so Join(cwd, ".") = cwd and Dir(cwd) + // would point one level above the search root. joinPath concatenates + // without cleaning, giving cwd + "/." whose Dir is correctly cwd. absPath := path if !filepath.IsAbs(absPath) { - absPath = filepath.Join(opts.workDir, absPath) + absPath = joinPath(opts.workDir, absPath) } execDirParent := filepath.Dir(absPath) diff --git a/tests/scenarios/cmd/find/execdir/dot_start_path.yaml b/tests/scenarios/cmd/find/execdir/dot_start_path.yaml new file mode 100644 index 00000000..283c96a9 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/dot_start_path.yaml @@ -0,0 +1,18 @@ +description: find . -execdir cat {} \; correctly resolves files relative to the matched entry's parent. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: file.txt + content: "hello" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:cat + script: |+ + find . -name 'file.txt' -execdir cat {} \; +expect: + stdout: "hello" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/dot_start_path_nested.yaml b/tests/scenarios/cmd/find/execdir/dot_start_path_nested.yaml new file mode 100644 index 00000000..9ccb5672 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/dot_start_path_nested.yaml @@ -0,0 +1,18 @@ +description: find . -execdir cat {} \; resolves files in nested subdirectories correctly. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: sub/deep.txt + content: "nested" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:cat + script: |+ + find . -path '*/deep.txt' -execdir cat {} \; +expect: + stdout: "nested" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/find/execdir/named_dir_start_path.yaml b/tests/scenarios/cmd/find/execdir/named_dir_start_path.yaml new file mode 100644 index 00000000..344a328c --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/named_dir_start_path.yaml @@ -0,0 +1,18 @@ +description: find dir -execdir cat {} \; resolves files relative to the named directory. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/file.txt + content: "data" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:cat + script: |+ + find dir -name 'file.txt' -execdir cat {} \; +expect: + stdout: "data" + stderr: "" + exit_code: 0 From 6a80911d5a2e0cef5f056bcfc58559932a0137d7 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 15:08:51 -0400 Subject: [PATCH 03/13] fix(find): allow literal '+' arguments in -execdir ; mode In find syntax, '+' is only a batch-mode terminator when it immediately follows '{}'. Previously the parser rejected any '+' token, blocking valid commands like -execdir echo + {} \;. Now '+' is only rejected in the {} + position. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/find/expr.go | 9 +++++--- builtins/find/expr_test.go | 23 +++++++++++++++++++ .../cmd/find/execdir/literal_plus_arg.yaml | 19 +++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 tests/scenarios/cmd/find/execdir/literal_plus_arg.yaml diff --git a/builtins/find/expr.go b/builtins/find/expr.go index 1703eb2b..56b6dcb6 100644 --- a/builtins/find/expr.go +++ b/builtins/find/expr.go @@ -684,20 +684,23 @@ func parseSymbolicMode(s string) (uint64, error) { } // parseExecDirPredicate parses -execdir command [args...] ; -// Only \; mode is supported. + (batch mode) is rejected with a clear error. +// Only \; mode is supported. {} + (batch mode) is rejected with a clear error. +// A literal "+" that does not follow "{}" is treated as a normal argument. func (p *parser) parseExecDirPredicate() (*expr, error) { if p.pos >= len(p.args) { return nil, errors.New("find: missing argument to '-execdir'") } - // Collect tokens until ";" or "+" terminator. + // Collect tokens until ";" terminator, or "+" after "{}" (batch mode). + // In find syntax, "+" is only special as a terminator in the {} + form; + // otherwise it is a normal argument (e.g. -execdir echo + {} \;). startPos := p.pos for p.pos < len(p.args) { tok := p.args[p.pos] if tok == ";" { break } - if tok == "+" { + if tok == "+" && p.pos > startPos && p.args[p.pos-1] == "{}" { return nil, errors.New("find: -execdir ... + (batch mode) is not yet supported") } p.pos++ diff --git a/builtins/find/expr_test.go b/builtins/find/expr_test.go index ecb1024c..8206c815 100644 --- a/builtins/find/expr_test.go +++ b/builtins/find/expr_test.go @@ -395,6 +395,29 @@ func TestParseExecDirBatchRejected(t *testing.T) { assert.Contains(t, err.Error(), "batch mode") } +// TestParseExecDirLiteralPlus verifies that a literal "+" that does not follow +// "{}" is treated as a normal argument, not as a batch-mode terminator. +func TestParseExecDirLiteralPlus(t *testing.T) { + // "+" before "{}" — normal argument + pr, err := parseExpression([]string{"-execdir", "echo", "+", "{}", ";"}) + require.NoError(t, err) + require.NotNil(t, pr.expr) + assert.Equal(t, exprExecDir, pr.expr.kind) + assert.Equal(t, "echo", pr.expr.execCmd) + assert.Equal(t, []string{"+", "{}"}, pr.expr.execArgs) + + // "+" as the only argument (no "{}" at all) + pr2, err := parseExpression([]string{"-execdir", "echo", "+", ";"}) + require.NoError(t, err) + require.NotNil(t, pr2.expr) + assert.Equal(t, []string{"+"}, pr2.expr.execArgs) + + // "{}" + still triggers batch mode error + _, err = parseExpression([]string{"-execdir", "echo", "{}", "+"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "batch mode") +} + // TestParseExecDirEmbeddedBraces verifies that {} embedded within a larger // argument token (e.g. {}.bak) is rejected with an error mentioning // "standalone". diff --git a/tests/scenarios/cmd/find/execdir/literal_plus_arg.yaml b/tests/scenarios/cmd/find/execdir/literal_plus_arg.yaml new file mode 100644 index 00000000..1da610ad --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/literal_plus_arg.yaml @@ -0,0 +1,19 @@ +description: find -execdir echo + {} \; treats + as a literal argument, not batch terminator. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/a.txt + content: "a" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -name 'a.txt' -execdir echo + {} \; +expect: + stdout: |+ + + ./a.txt + stderr: "" + exit_code: 0 From 90700977f4d5c4b5c9ee94eecccfed63d09e1ef0 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 15:21:13 -0400 Subject: [PATCH 04/13] fix(find): propagate RunCommand and WorkDir into nested -execdir contexts The child CallContext built by RunCommand was missing RunCommand and WorkDir, so builtins invoked via -execdir that themselves need sub-command execution (e.g. nested find) would fail with "command execution not available". Extract RunCommand into a named variable so the child can reference it recursively. Co-Authored-By: Claude Opus 4.6 (1M context) --- interp/runner_exec.go | 120 +++++++++--------- .../cmd/find/execdir/nested_execdir.yaml | 19 +++ 2 files changed, 81 insertions(+), 58 deletions(-) create mode 100644 tests/scenarios/cmd/find/execdir/nested_execdir.yaml diff --git a/interp/runner_exec.go b/interp/runner_exec.go index c8c60254..ddc8a13a 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -265,6 +265,67 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { } if fn, ok := builtins.Lookup(name); ok { + var runCmd func(context.Context, string, string, []string) (uint8, error) + runCmd = func(ctx context.Context, dir string, cmdName string, cmdArgs []string) (uint8, error) { + if !r.allowAllCommands && cmdName != "help" && !r.allowedCommands[cmdName] { + return 127, fmt.Errorf("%s: command not allowed", cmdName) + } + fn, ok := builtins.Lookup(cmdName) + if !ok { + return 127, fmt.Errorf("%s: command not found", cmdName) + } + child := &builtins.CallContext{ + Stdout: r.stdout, + Stderr: r.stderr, + WorkDir: func() string { return dir }, + RunCommand: runCmd, + OpenFile: func(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) { + return r.sandbox.Open(path, dir, flags, mode) + }, + ReadDir: func(ctx context.Context, path string) ([]fs.DirEntry, error) { + return r.sandbox.ReadDir(path, dir) + }, + OpenDir: func(ctx context.Context, path string) (fs.ReadDirFile, error) { + return r.sandbox.OpenDir(path, dir) + }, + IsDirEmpty: func(ctx context.Context, path string) (bool, error) { + return r.sandbox.IsDirEmpty(path, dir) + }, + ReadDirLimited: func(ctx context.Context, path string, offset, maxRead int) ([]fs.DirEntry, bool, error) { + return r.sandbox.ReadDirLimited(path, dir, offset, maxRead) + }, + StatFile: func(ctx context.Context, path string) (fs.FileInfo, error) { + return r.sandbox.Stat(path, dir) + }, + LstatFile: func(ctx context.Context, path string) (fs.FileInfo, error) { + return r.sandbox.Lstat(path, dir) + }, + AccessFile: func(ctx context.Context, path string, mode uint32) error { + return r.sandbox.Access(path, dir, mode) + }, + PortableErr: allowedpaths.PortableErrMsg, + Now: r.startTime, + FileIdentity: func(path string, info fs.FileInfo) (builtins.FileID, bool) { + absPath := path + if !filepath.IsAbs(absPath) { + absPath = filepath.Join(dir, absPath) + } + dev, ino, ok := allowedpaths.FileIdentity(absPath, info, r.sandbox) + if !ok { + return builtins.FileID{}, false + } + return builtins.FileID{Dev: dev, Ino: ino}, true + }, + CommandAllowed: func(n string) bool { + return r.allowAllCommands || n == "help" || r.allowedCommands[n] + }, + } + if r.stdin != nil { + child.Stdin = r.stdin + } + result := fn(ctx, child, cmdArgs) + return result.Code, nil + } call := &builtins.CallContext{ Stdout: r.stdout, Stderr: r.stderr, @@ -313,64 +374,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { CommandAllowed: func(cmdName string) bool { return r.allowAllCommands || cmdName == "help" || r.allowedCommands[cmdName] }, - RunCommand: func(ctx context.Context, dir string, cmdName string, cmdArgs []string) (uint8, error) { - if !r.allowAllCommands && cmdName != "help" && !r.allowedCommands[cmdName] { - return 127, fmt.Errorf("%s: command not allowed", cmdName) - } - fn, ok := builtins.Lookup(cmdName) - if !ok { - return 127, fmt.Errorf("%s: command not found", cmdName) - } - child := &builtins.CallContext{ - Stdout: r.stdout, - Stderr: r.stderr, - OpenFile: func(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) { - return r.sandbox.Open(path, dir, flags, mode) - }, - ReadDir: func(ctx context.Context, path string) ([]fs.DirEntry, error) { - return r.sandbox.ReadDir(path, dir) - }, - OpenDir: func(ctx context.Context, path string) (fs.ReadDirFile, error) { - return r.sandbox.OpenDir(path, dir) - }, - IsDirEmpty: func(ctx context.Context, path string) (bool, error) { - return r.sandbox.IsDirEmpty(path, dir) - }, - ReadDirLimited: func(ctx context.Context, path string, offset, maxRead int) ([]fs.DirEntry, bool, error) { - return r.sandbox.ReadDirLimited(path, dir, offset, maxRead) - }, - StatFile: func(ctx context.Context, path string) (fs.FileInfo, error) { - return r.sandbox.Stat(path, dir) - }, - LstatFile: func(ctx context.Context, path string) (fs.FileInfo, error) { - return r.sandbox.Lstat(path, dir) - }, - AccessFile: func(ctx context.Context, path string, mode uint32) error { - return r.sandbox.Access(path, dir, mode) - }, - PortableErr: allowedpaths.PortableErrMsg, - Now: r.startTime, - FileIdentity: func(path string, info fs.FileInfo) (builtins.FileID, bool) { - absPath := path - if !filepath.IsAbs(absPath) { - absPath = filepath.Join(dir, absPath) - } - dev, ino, ok := allowedpaths.FileIdentity(absPath, info, r.sandbox) - if !ok { - return builtins.FileID{}, false - } - return builtins.FileID{Dev: dev, Ino: ino}, true - }, - CommandAllowed: func(n string) bool { - return r.allowAllCommands || n == "help" || r.allowedCommands[n] - }, - } - if r.stdin != nil { - child.Stdin = r.stdin - } - result := fn(ctx, child, cmdArgs) - return result.Code, nil - }, + RunCommand: runCmd, } if r.stdin != nil { // do not assign a typed nil into the io.Reader interface call.Stdin = r.stdin diff --git a/tests/scenarios/cmd/find/execdir/nested_execdir.yaml b/tests/scenarios/cmd/find/execdir/nested_execdir.yaml new file mode 100644 index 00000000..c41c284f --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/nested_execdir.yaml @@ -0,0 +1,19 @@ +description: find invoked via -execdir receives RunCommand and can itself use -execdir. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/file.txt + content: "x" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir -type d -execdir find dir -name 'file.txt' -print \; +expect: + stdout: |+ + dir/file.txt + stderr: "" + exit_code: 0 From f42be19e7d68094a2db94277c09867fea513b051 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 15:25:50 -0400 Subject: [PATCH 05/13] fix(find): normalize trailing slashes before deriving -execdir parent filepath.Dir("/cwd/dir/") returns "/cwd/dir" (the directory itself), not "/cwd" (its parent). Trim trailing slashes before taking Dir so -execdir runs in the correct parent directory for paths like "dir/". Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/find/find.go | 5 +++++ .../execdir/trailing_slash_start_path.yaml | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/scenarios/cmd/find/execdir/trailing_slash_start_path.yaml diff --git a/builtins/find/find.go b/builtins/find/find.go index c247ff5e..f8788e77 100644 --- a/builtins/find/find.go +++ b/builtins/find/find.go @@ -442,6 +442,11 @@ func walkPath( if !filepath.IsAbs(absPath) { absPath = joinPath(opts.workDir, absPath) } + // Trim trailing slashes (except root) so filepath.Dir returns + // the parent directory, not the directory itself. + for len(absPath) > 1 && absPath[len(absPath)-1] == '/' { + absPath = absPath[:len(absPath)-1] + } execDirParent := filepath.Dir(absPath) ec := &evalContext{ diff --git a/tests/scenarios/cmd/find/execdir/trailing_slash_start_path.yaml b/tests/scenarios/cmd/find/execdir/trailing_slash_start_path.yaml new file mode 100644 index 00000000..11cb7eb4 --- /dev/null +++ b/tests/scenarios/cmd/find/execdir/trailing_slash_start_path.yaml @@ -0,0 +1,19 @@ +description: find dir/ -execdir resolves parent correctly despite trailing slash. +skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands +setup: + files: + - path: dir/file.txt + content: "x" + chmod: 0644 +input: + allowed_paths: ["$DIR"] + allowed_commands: + - rshell:find + - rshell:echo + script: |+ + find dir/ -maxdepth 0 -execdir echo {} \; +expect: + stdout: |+ + ./dir + stderr: "" + exit_code: 0 From b9343bd191eccb45103f226ec2a1613bc5dd2809 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 15:44:09 -0400 Subject: [PATCH 06/13] fix(find): pass / instead of .// for root path in -execdir {} baseName("/") returns "/", so "./" + "/" produced ".//". Special-case the root entry to pass "/" directly, matching GNU find behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/find/builtin_find_pentest_test.go | 33 ++++++++++++++++++++++ builtins/find/eval.go | 6 +++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/builtins/find/builtin_find_pentest_test.go b/builtins/find/builtin_find_pentest_test.go index a8db0757..d74c0f38 100644 --- a/builtins/find/builtin_find_pentest_test.go +++ b/builtins/find/builtin_find_pentest_test.go @@ -190,6 +190,39 @@ func TestFindExecDirQuotesInFilename(t *testing.T) { testExecDirFilename(t, `it's a "test"`) } +// TestFindExecDirRootPath verifies that the root path "/" is passed as "/" +// (not ".//") when used as the {} replacement. +func TestFindExecDirRootPath(t *testing.T) { + var stdout, stderr bytes.Buffer + var capturedArgs []string + + callCtx := newPentestCallCtx(&stdout, &stderr) + callCtx.RunCommand = func(_ context.Context, _ string, _ string, args []string) (uint8, error) { + capturedArgs = make([]string, len(args)) + copy(capturedArgs, args) + return 0, nil + } + callCtx.CommandAllowed = func(_ string) bool { return true } + + ec := &evalContext{ + callCtx: callCtx, + ctx: context.Background(), + relPath: "/", + info: &mockFileInfo{}, + } + e := &expr{ + kind: exprExecDir, + execCmd: "echo", + execArgs: []string{"{}"}, + } + + result := evalExecDir(ec, e) + + assert.True(t, result.matched) + require.Len(t, capturedArgs, 1) + assert.Equal(t, "/", capturedArgs[0], "root path must be passed as '/', not './/'") +} + // ---- Argument Injection (CWE-88) — ./ prefix prevents flag interpretation ---- // TestFindExecDirDashFilename verifies that a filename beginning with a dash diff --git a/builtins/find/eval.go b/builtins/find/eval.go index 8b369726..08f5ba0a 100644 --- a/builtins/find/eval.go +++ b/builtins/find/eval.go @@ -280,7 +280,11 @@ func evalExecDir(ec *evalContext, e *expr) evalResult { ec.failed = true return evalResult{} } - replacement := "./" + baseName(ec.relPath) + base := baseName(ec.relPath) + replacement := "./" + base + if base == "/" { + replacement = "/" + } args := make([]string, len(e.execArgs)) for i, a := range e.execArgs { if a == "{}" { From fb857ae78f1fcd015fcb98d4cb6217e82348a0d6 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 15:59:43 -0400 Subject: [PATCH 07/13] fix(find): preserve drive-root slash in -execdir parent trim The trailing-slash trim converted C:/ to C:, making filepath.Dir return the drive-relative C:. instead of the drive root. Skip the trim when the slash immediately follows a colon (drive separator). Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/find/builtin_find_pentest_test.go | 34 ++++++++++++++++++++++ builtins/find/find.go | 7 ++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/builtins/find/builtin_find_pentest_test.go b/builtins/find/builtin_find_pentest_test.go index d74c0f38..3cc49fe1 100644 --- a/builtins/find/builtin_find_pentest_test.go +++ b/builtins/find/builtin_find_pentest_test.go @@ -320,6 +320,40 @@ func TestFindExecDirParentDir(t *testing.T) { } } +// TestFindExecDirWindowsDriveRoot verifies that a Windows drive root path +// like "C:/" is forwarded as-is to RunCommand, not truncated to "C:" which +// would produce a drive-relative CWD. +func TestFindExecDirWindowsDriveRoot(t *testing.T) { + var stdout, stderr bytes.Buffer + var capturedDir string + + callCtx := newPentestCallCtx(&stdout, &stderr) + callCtx.RunCommand = func(_ context.Context, dir string, _ string, _ []string) (uint8, error) { + capturedDir = dir + return 0, nil + } + callCtx.CommandAllowed = func(_ string) bool { return true } + + ec := &evalContext{ + callCtx: callCtx, + ctx: context.Background(), + relPath: "C:/", + info: &mockFileInfo{}, + execDirParent: "C:/", + } + e := &expr{ + kind: exprExecDir, + execCmd: "echo", + execArgs: []string{"{}"}, + } + + result := evalExecDir(ec, e) + + assert.True(t, result.matched) + assert.Equal(t, "C:/", capturedDir, + "drive root must be preserved as C:/, not truncated to C:") +} + // ---- GTFOBins ---- // TestFindExecDirExecStillBlocked verifies that -exec remains in the blocked diff --git a/builtins/find/find.go b/builtins/find/find.go index f8788e77..dce7876d 100644 --- a/builtins/find/find.go +++ b/builtins/find/find.go @@ -443,8 +443,13 @@ func walkPath( absPath = joinPath(opts.workDir, absPath) } // Trim trailing slashes (except root) so filepath.Dir returns - // the parent directory, not the directory itself. + // the parent directory, not the directory itself. Preserve + // drive-root slashes on Windows (e.g. C:/) to avoid producing + // the drive-relative prefix C: which Dir resolves to C:. . for len(absPath) > 1 && absPath[len(absPath)-1] == '/' { + if len(absPath) >= 3 && absPath[len(absPath)-2] == ':' { + break + } absPath = absPath[:len(absPath)-1] } execDirParent := filepath.Dir(absPath) From 710d1a857b8ac7f30c693e6d2c29cb527c6daca2 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 16:22:59 -0400 Subject: [PATCH 08/13] fix(find): preserve trailing slash in -execdir {} replacement baseName strips trailing slashes, so dir/ became ./dir instead of ./dir/. Re-append the slash when the original relPath ends with /, matching GNU find and preserving slash-sensitive sub-command semantics. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/find/builtin_find_pentest_test.go | 34 +++++++++++++++++++ builtins/find/eval.go | 2 ++ .../execdir/trailing_slash_start_path.yaml | 4 +-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/builtins/find/builtin_find_pentest_test.go b/builtins/find/builtin_find_pentest_test.go index 3cc49fe1..684074ba 100644 --- a/builtins/find/builtin_find_pentest_test.go +++ b/builtins/find/builtin_find_pentest_test.go @@ -320,6 +320,40 @@ func TestFindExecDirParentDir(t *testing.T) { } } +// TestFindExecDirTrailingSlashPreserved verifies that a trailing slash on the +// relPath is preserved in the {} replacement (e.g. "dir/" → "./dir/"). +func TestFindExecDirTrailingSlashPreserved(t *testing.T) { + var stdout, stderr bytes.Buffer + var capturedArgs []string + + callCtx := newPentestCallCtx(&stdout, &stderr) + callCtx.RunCommand = func(_ context.Context, _ string, _ string, args []string) (uint8, error) { + capturedArgs = make([]string, len(args)) + copy(capturedArgs, args) + return 0, nil + } + callCtx.CommandAllowed = func(_ string) bool { return true } + + ec := &evalContext{ + callCtx: callCtx, + ctx: context.Background(), + relPath: "dir/", + info: &mockFileInfo{}, + } + e := &expr{ + kind: exprExecDir, + execCmd: "echo", + execArgs: []string{"{}"}, + } + + result := evalExecDir(ec, e) + + assert.True(t, result.matched) + require.Len(t, capturedArgs, 1) + assert.Equal(t, "./dir/", capturedArgs[0], + "trailing slash must be preserved: dir/ → ./dir/") +} + // TestFindExecDirWindowsDriveRoot verifies that a Windows drive root path // like "C:/" is forwarded as-is to RunCommand, not truncated to "C:" which // would produce a drive-relative CWD. diff --git a/builtins/find/eval.go b/builtins/find/eval.go index 08f5ba0a..c8c68150 100644 --- a/builtins/find/eval.go +++ b/builtins/find/eval.go @@ -284,6 +284,8 @@ func evalExecDir(ec *evalContext, e *expr) evalResult { replacement := "./" + base if base == "/" { replacement = "/" + } else if len(ec.relPath) > 0 && ec.relPath[len(ec.relPath)-1] == '/' { + replacement += "/" } args := make([]string, len(e.execArgs)) for i, a := range e.execArgs { diff --git a/tests/scenarios/cmd/find/execdir/trailing_slash_start_path.yaml b/tests/scenarios/cmd/find/execdir/trailing_slash_start_path.yaml index 11cb7eb4..45bd519d 100644 --- a/tests/scenarios/cmd/find/execdir/trailing_slash_start_path.yaml +++ b/tests/scenarios/cmd/find/execdir/trailing_slash_start_path.yaml @@ -1,4 +1,4 @@ -description: find dir/ -execdir resolves parent correctly despite trailing slash. +description: find dir/ -execdir preserves trailing slash in {} replacement. skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands setup: files: @@ -14,6 +14,6 @@ input: find dir/ -maxdepth 0 -execdir echo {} \; expect: stdout: |+ - ./dir + ./dir/ stderr: "" exit_code: 0 From 36d4ee451fa607156d9fdf6487ce4b8909176312 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 16:27:37 -0400 Subject: [PATCH 09/13] feat(find): allow embedded {} tokens in -execdir arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the parser gate that rejected arguments containing {} unless they were exactly {}. Now {}.bak, prefix-{}-suffix, etc. are accepted and replaced via strings.ReplaceAll, matching GNU find behavior. This is safe because arguments are discrete Go strings — no shell re-parsing occurs. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_builtins.go | 3 +- builtins/find/builtin_find_pentest_test.go | 34 +++++++++++++++++++ builtins/find/eval.go | 5 +-- builtins/find/expr.go | 7 ---- builtins/find/expr_test.go | 11 +++--- .../cmd/find/execdir/embedded_braces.yaml | 10 +++--- 6 files changed, 51 insertions(+), 19 deletions(-) diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index 2deb7252..005b896b 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -96,8 +96,9 @@ var builtinPerCommandSymbols = map[string][]string{ "strconv.ErrRange", // 🟢 sentinel error value for overflow; pure constant. "strconv.ParseInt", // 🟢 string-to-int conversion; pure function, no I/O. "strconv.ParseUint", // 🟢 string-to-unsigned-int conversion; pure function, no I/O. - "strings.Contains", // 🟢 substring search for {} validation in -execdir; pure function, no I/O. + "strings.Contains", // 🟢 substring search for {} replacement in -execdir; pure function, no I/O. "strings.HasPrefix", // 🟢 pure function for prefix matching; no I/O. + "strings.ReplaceAll", // 🟢 replaces all {} occurrences in -execdir args; pure function, no I/O. "strings.Split", // 🟢 splits a string by separator into a slice; pure function, no I/O. "strings.ToLower", // 🟢 converts string to lowercase; pure function, no I/O. "time.Duration", // 🟢 duration type; pure integer alias, no I/O. diff --git a/builtins/find/builtin_find_pentest_test.go b/builtins/find/builtin_find_pentest_test.go index 684074ba..d7863793 100644 --- a/builtins/find/builtin_find_pentest_test.go +++ b/builtins/find/builtin_find_pentest_test.go @@ -320,6 +320,40 @@ func TestFindExecDirParentDir(t *testing.T) { } } +// TestFindExecDirEmbeddedBracesReplacement verifies that {} embedded in an +// argument (e.g. {}.bak) is replaced with the ./basename substitution. +func TestFindExecDirEmbeddedBracesReplacement(t *testing.T) { + var stdout, stderr bytes.Buffer + var capturedArgs []string + + callCtx := newPentestCallCtx(&stdout, &stderr) + callCtx.RunCommand = func(_ context.Context, _ string, _ string, args []string) (uint8, error) { + capturedArgs = make([]string, len(args)) + copy(capturedArgs, args) + return 0, nil + } + callCtx.CommandAllowed = func(_ string) bool { return true } + + ec := &evalContext{ + callCtx: callCtx, + ctx: context.Background(), + relPath: "photo.jpg", + info: &mockFileInfo{}, + } + e := &expr{ + kind: exprExecDir, + execCmd: "echo", + execArgs: []string{"{}.bak", "prefix-{}-suffix"}, + } + + result := evalExecDir(ec, e) + + assert.True(t, result.matched) + require.Len(t, capturedArgs, 2) + assert.Equal(t, "./photo.jpg.bak", capturedArgs[0]) + assert.Equal(t, "prefix-./photo.jpg-suffix", capturedArgs[1]) +} + // TestFindExecDirTrailingSlashPreserved verifies that a trailing slash on the // relPath is preserved in the {} replacement (e.g. "dir/" → "./dir/"). func TestFindExecDirTrailingSlashPreserved(t *testing.T) { diff --git a/builtins/find/eval.go b/builtins/find/eval.go index c8c68150..0378dba7 100644 --- a/builtins/find/eval.go +++ b/builtins/find/eval.go @@ -9,6 +9,7 @@ import ( "context" iofs "io/fs" "math" + "strings" "time" "github.com/DataDog/rshell/builtins" @@ -289,8 +290,8 @@ func evalExecDir(ec *evalContext, e *expr) evalResult { } args := make([]string, len(e.execArgs)) for i, a := range e.execArgs { - if a == "{}" { - args[i] = replacement + if strings.Contains(a, "{}") { + args[i] = strings.ReplaceAll(a, "{}", replacement) } else { args[i] = a } diff --git a/builtins/find/expr.go b/builtins/find/expr.go index 56b6dcb6..5e40cea5 100644 --- a/builtins/find/expr.go +++ b/builtins/find/expr.go @@ -721,13 +721,6 @@ func (p *parser) parseExecDirPredicate() (*expr, error) { cmd := tokens[0] args := tokens[1:] - // Validate {} usage: reject {}-embedded args like {}.bak. - for _, a := range args { - if a != "{}" && strings.Contains(a, "{}") { - return nil, fmt.Errorf("find: -execdir: '{}' must be a standalone argument") - } - } - return &expr{kind: exprExecDir, execCmd: cmd, execArgs: args}, nil } diff --git a/builtins/find/expr_test.go b/builtins/find/expr_test.go index 8206c815..4f2a624c 100644 --- a/builtins/find/expr_test.go +++ b/builtins/find/expr_test.go @@ -419,12 +419,13 @@ func TestParseExecDirLiteralPlus(t *testing.T) { } // TestParseExecDirEmbeddedBraces verifies that {} embedded within a larger -// argument token (e.g. {}.bak) is rejected with an error mentioning -// "standalone". +// argument token (e.g. {}.bak) is accepted and stored in execArgs. func TestParseExecDirEmbeddedBraces(t *testing.T) { - _, err := parseExpression([]string{"-execdir", "echo", "{}.bak", ";"}) - require.Error(t, err) - assert.Contains(t, err.Error(), "standalone") + pr, err := parseExpression([]string{"-execdir", "echo", "{}.bak", ";"}) + require.NoError(t, err) + require.NotNil(t, pr.expr) + assert.Equal(t, exprExecDir, pr.expr.kind) + assert.Equal(t, []string{"{}.bak"}, pr.expr.execArgs) } // TestParseExecDirIsAction verifies that isAction() returns true for an diff --git a/tests/scenarios/cmd/find/execdir/embedded_braces.yaml b/tests/scenarios/cmd/find/execdir/embedded_braces.yaml index ffe228d7..1e6c37fa 100644 --- a/tests/scenarios/cmd/find/execdir/embedded_braces.yaml +++ b/tests/scenarios/cmd/find/execdir/embedded_braces.yaml @@ -1,4 +1,4 @@ -description: find -execdir rejects embedded {} like {}.bak in arguments. +description: find -execdir replaces {} embedded in arguments like {}.bak. skip_assert_against_bash: true # rshell -execdir only runs builtins, not external commands setup: files: @@ -11,7 +11,9 @@ input: - rshell:find - rshell:echo script: |+ - find dir -execdir echo {}.bak \; + find dir -name 'a.txt' -execdir echo {}.bak \; expect: - stderr_contains: ["{}"] - exit_code: 1 + stdout: |+ + ./a.txt.bak + stderr: "" + exit_code: 0 From 6b9dd6d4d05d8c04d169b91cc1692e8142858376 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 16:44:25 -0400 Subject: [PATCH 10/13] refactor(find): address Effective Go review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - P1: rename shadowed fn → cmdFn inside runCmd closure to avoid masking the outer builtins.Lookup result (variable shadowing) - P2: remove redundant strings.Contains guard before ReplaceAll; drop strings.Contains from find's allowed symbols - P3: use accumulator pattern in collectExecDirCmds to avoid intermediate slice allocations from append-spread Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_builtins.go | 1 - builtins/find/eval.go | 6 +----- builtins/find/find.go | 18 +++++++++++------- interp/runner_exec.go | 4 ++-- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index 005b896b..686f8866 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -96,7 +96,6 @@ var builtinPerCommandSymbols = map[string][]string{ "strconv.ErrRange", // 🟢 sentinel error value for overflow; pure constant. "strconv.ParseInt", // 🟢 string-to-int conversion; pure function, no I/O. "strconv.ParseUint", // 🟢 string-to-unsigned-int conversion; pure function, no I/O. - "strings.Contains", // 🟢 substring search for {} replacement in -execdir; pure function, no I/O. "strings.HasPrefix", // 🟢 pure function for prefix matching; no I/O. "strings.ReplaceAll", // 🟢 replaces all {} occurrences in -execdir args; pure function, no I/O. "strings.Split", // 🟢 splits a string by separator into a slice; pure function, no I/O. diff --git a/builtins/find/eval.go b/builtins/find/eval.go index 0378dba7..98775b69 100644 --- a/builtins/find/eval.go +++ b/builtins/find/eval.go @@ -290,11 +290,7 @@ func evalExecDir(ec *evalContext, e *expr) evalResult { } args := make([]string, len(e.execArgs)) for i, a := range e.execArgs { - if strings.Contains(a, "{}") { - args[i] = strings.ReplaceAll(a, "{}", replacement) - } else { - args[i] = a - } + args[i] = strings.ReplaceAll(a, "{}", replacement) } exitCode, err := ec.callCtx.RunCommand(ec.ctx, ec.execDirParent, e.execCmd, args) if err != nil { diff --git a/builtins/find/find.go b/builtins/find/find.go index dce7876d..773ecedd 100644 --- a/builtins/find/find.go +++ b/builtins/find/find.go @@ -606,17 +606,21 @@ func walkPath( // collectExecDirCmds walks the expression tree and returns all -execdir command names. func collectExecDirCmds(e *expr) []string { + var cmds []string + collectExecDirCmdsInto(e, &cmds) + return cmds +} + +func collectExecDirCmdsInto(e *expr, cmds *[]string) { if e == nil { - return nil + return } - var cmds []string if e.kind == exprExecDir { - cmds = append(cmds, e.execCmd) + *cmds = append(*cmds, e.execCmd) } - cmds = append(cmds, collectExecDirCmds(e.left)...) - cmds = append(cmds, collectExecDirCmds(e.right)...) - cmds = append(cmds, collectExecDirCmds(e.operand)...) - return cmds + collectExecDirCmdsInto(e.left, cmds) + collectExecDirCmdsInto(e.right, cmds) + collectExecDirCmdsInto(e.operand, cmds) } // collectNewerRefs walks the expression tree and returns all -newer reference paths. diff --git a/interp/runner_exec.go b/interp/runner_exec.go index ddc8a13a..0671570f 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -270,7 +270,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { if !r.allowAllCommands && cmdName != "help" && !r.allowedCommands[cmdName] { return 127, fmt.Errorf("%s: command not allowed", cmdName) } - fn, ok := builtins.Lookup(cmdName) + cmdFn, ok := builtins.Lookup(cmdName) if !ok { return 127, fmt.Errorf("%s: command not found", cmdName) } @@ -323,7 +323,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { if r.stdin != nil { child.Stdin = r.stdin } - result := fn(ctx, child, cmdArgs) + result := cmdFn(ctx, child, cmdArgs) return result.Code, nil } call := &builtins.CallContext{ From 14fba7671530ad838544b5cad78cdc40e2e5fea6 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 17:03:30 -0400 Subject: [PATCH 11/13] fix(find): restrict drive-root exception to real Windows root paths The trailing-slash guard matched any path ending in :/ (e.g. /tmp/d:/), not just Windows drive roots like C:/. Tighten to len == 3 && [1] == ':' so only X:/ is preserved. Extract the trim+Dir logic into execDirParentDir for direct unit testing. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/find/find.go | 29 ++++++++++++++++------------- builtins/find/find_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/builtins/find/find.go b/builtins/find/find.go index 773ecedd..bc86dba4 100644 --- a/builtins/find/find.go +++ b/builtins/find/find.go @@ -436,23 +436,12 @@ func walkPath( // Compute parent directory for -execdir. // Use joinPath (not filepath.Join) to preserve '.' and '..' components. // filepath.Join cleans lexically, so Join(cwd, ".") = cwd and Dir(cwd) - // would point one level above the search root. joinPath concatenates - // without cleaning, giving cwd + "/." whose Dir is correctly cwd. + // would point one level above the search root. absPath := path if !filepath.IsAbs(absPath) { absPath = joinPath(opts.workDir, absPath) } - // Trim trailing slashes (except root) so filepath.Dir returns - // the parent directory, not the directory itself. Preserve - // drive-root slashes on Windows (e.g. C:/) to avoid producing - // the drive-relative prefix C: which Dir resolves to C:. . - for len(absPath) > 1 && absPath[len(absPath)-1] == '/' { - if len(absPath) >= 3 && absPath[len(absPath)-2] == ':' { - break - } - absPath = absPath[:len(absPath)-1] - } - execDirParent := filepath.Dir(absPath) + execDirParent := execDirParentDir(absPath) ec := &evalContext{ callCtx: callCtx, @@ -650,3 +639,17 @@ func joinPath(dir, name string) string { } return dir + "/" + name } + +// execDirParentDir returns the parent directory of absPath for -execdir. +// Trailing slashes are trimmed (except Unix root "/" and Windows drive +// roots like "C:/") so that filepath.Dir returns the parent directory +// rather than the directory itself. +func execDirParentDir(absPath string) string { + for len(absPath) > 1 && absPath[len(absPath)-1] == '/' { + if len(absPath) == 3 && absPath[1] == ':' { + break // preserve Windows drive root (e.g. C:/) + } + absPath = absPath[:len(absPath)-1] + } + return filepath.Dir(absPath) +} diff --git a/builtins/find/find_test.go b/builtins/find/find_test.go index c3a0e96d..12f69aff 100644 --- a/builtins/find/find_test.go +++ b/builtins/find/find_test.go @@ -11,6 +11,31 @@ import ( "github.com/stretchr/testify/assert" ) +// TestExecDirParentDir verifies the trim-and-Dir logic used to compute the +// parent directory for -execdir. Trailing slashes are stripped (except root +// and Windows drive roots) before filepath.Dir is called. +func TestExecDirParentDir(t *testing.T) { + tests := []struct { + name string + absPath string + want string + }{ + {"trailing slash trimmed", "/cwd/dir/", "/cwd"}, + {"no trailing slash", "/cwd/dir", "/cwd"}, + {"dot preserved", "/cwd/.", "/cwd"}, + {"unix root", "/", "/"}, + {"colon in unix path", "/tmp/d:/", "/tmp"}, + {"deeply nested trailing", "/a/b/c/", "/a/b"}, + {"multiple trailing slashes", "/a/b//", "/a"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := execDirParentDir(tt.absPath) + assert.Equal(t, tt.want, got, "execDirParentDir(%q)", tt.absPath) + }) + } +} + // TestIsExpressionStart verifies the boundary between path operands and // expression tokens. GNU find treats !, (, and any dash-prefixed token // with length > 1 as expression starters. Everything else (including From 33bc1eac653a4f5c409cb2eace8c2ad2cc0ed865 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 18 Mar 2026 17:15:30 -0400 Subject: [PATCH 12/13] fix(find): normalize path separators in TestExecDirParentDir for Windows filepath.Dir returns backslashes on Windows. Apply filepath.ToSlash to the result before comparing against forward-slash expectations, matching the shell's path normalization convention. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/find/find_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtins/find/find_test.go b/builtins/find/find_test.go index 12f69aff..6d612574 100644 --- a/builtins/find/find_test.go +++ b/builtins/find/find_test.go @@ -6,6 +6,7 @@ package find import ( + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -30,7 +31,7 @@ func TestExecDirParentDir(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := execDirParentDir(tt.absPath) + got := filepath.ToSlash(execDirParentDir(tt.absPath)) assert.Equal(t, tt.want, got, "execDirParentDir(%q)", tt.absPath) }) } From 7f35be8bfc35e3a955e1aded9e52ee8def160d56 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 19 Mar 2026 10:32:19 -0400 Subject: [PATCH 13/13] chore: format files --- interp/runner_exec.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interp/runner_exec.go b/interp/runner_exec.go index a0c48fb3..b8f88915 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -375,7 +375,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { return r.allowAllCommands || cmdName == "help" || r.allowedCommands[cmdName] }, RunCommand: runCmd, - Proc: r.proc, + Proc: r.proc, } if r.stdin != nil { // do not assign a typed nil into the io.Reader interface call.Stdin = r.stdin