feat(find): implement -exec command {} ;#133
Conversation
Add -exec predicate to find, running commands in find's working
directory with the full relative path as the {} replacement. Unlike
-execdir (which uses ./basename in the file's parent directory),
-exec passes the complete path (e.g. dir/sub/file.txt) matching
GNU find behavior.
Key design decisions:
- \; mode only — + (batch) mode rejected with clear error
- AllowedCommands enforced at parse-time AND eval-time (defense-in-depth)
- Same sandbox — sub-commands run through RunCommand with AllowedPaths
- No shell re-parsing — {} replacement via strings.ReplaceAll
- No ./ prefix (unlike -execdir) — matches GNU find; -execdir
recommended for TOCTOU-sensitive and dash-injection-safe use cases
- Parser refactored into shared parseExecLikePredicate helper
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex conduct a comprehensive security and code review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90b46529f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
GNU find applies {} substitution across the entire command vector
including the command name. Previously only execArgs were substituted
while execCmd was passed through unchanged. Now both evalExec and
evalExecDir substitute {} in the command name and validate the
resolved name against CommandAllowed. The post-parse eager check
skips commands containing {} since the substituted value depends
on each file.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex conduct a comprehensive security and code review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
- P1: error messages now use resolved cmd (post-{} substitution)
instead of the raw template e.execCmd
- P2: extract shared evalExecLike helper — evalExec and evalExecDir
are now thin wrappers that compute replacement and dir
- P3: TestFindExecFullPathReplacement reuses testExecFilename helper
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex conduct a comprehensive security and code review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
### What does this PR do? See changes: - https://github.com/DataDog/rshell/releases/tag/v0.0.7 - https://github.com/DataDog/rshell/releases/tag/v0.0.8 Besides the minor fixes to improve security and memory usage, we have those changes to rshell commands: - feat(find): implement -execdir command {} ; by @matt-dz in DataDog/rshell#125 - feat(find): implement -exec command {} ; by @matt-dz in DataDog/rshell#133 - feat(ip): add ip route show/get builtin (Linux only) by @AlexandreYang in DataDog/rshell#135 ### Testing ✅ Tested e2e with local agent ### Describe how you validated your changes Test rshell works e2e ### Additional Notes Co-authored-by: alexandre.yang <alexandre.yang@datadoghq.com>
See changes: - https://github.com/DataDog/rshell/releases/tag/v0.0.7 - https://github.com/DataDog/rshell/releases/tag/v0.0.8 Besides the minor fixes to improve security and memory usage, we have those changes to rshell commands: - feat(find): implement -execdir command {} ; by @matt-dz in DataDog/rshell#125 - feat(find): implement -exec command {} ; by @matt-dz in DataDog/rshell#133 - feat(ip): add ip route show/get builtin (Linux only) by @AlexandreYang in DataDog/rshell#135 ✅ Tested e2e with local agent Test rshell works e2e Co-authored-by: alexandre.yang <alexandre.yang@datadoghq.com>
Summary
Implements
find -exec CMD [ARG]... \;— runs a command once per matched file, replacing{}with the full relative path. This complements the existing-execdirpredicate (which runs in the file's parent directory with./basename).\;mode only —+(batch) mode is deferred and rejected with a clear error messageRunCommandwith the sameAllowedPathsrestrictions{}replacement viastrings.ReplaceAllproduces discrete Go string arguments-execand-execdirnow share aparseExecLikePredicatehelper, eliminating code duplication-execvs-execdir-exec(new)-execdir(existing){}replacementdir/sub/file.txt)./basename(./file.txt)./prefixSecurity model
The sandbox enforces
AllowedPathson all sub-command file operations, so even if a path is replaced with a symlink between stat and exec, the sub-command cannot escape the sandbox. The help text documents that-execdiris recommended over-execfor TOCTOU-sensitive and dash-injection-safe use cases.Changes
builtins/find/expr.goexprExeckind, sharedparseExecLikePredicatehelper, removed-execfrom blocked listbuiltins/find/eval.goevalExec()— usesprintPathfor{},execWorkDirfor RunCommand dirbuiltins/find/find.goexecWorkDirinto evalContext, renamedcollectExecCmdsto cover both, updated help/docsbuiltins/find/expr_test.go-execbuiltins/find/builtin_find_pentest_test.gotests/scenarios/cmd/find/exec/SHELL_FEATURES.mdTest plan
gofmt -l .— cleango build ./...— buildsGOOS=windows go build ./...— cross-compilesgo test ./builtins/find/ -v -count=1— all pass (including 15 new pentest + 8 parser tests)go test ./tests/ -run TestShellScenarios$ -count=1 -timeout 120s— all pass (23 new exec scenarios)go test ./... -count=1 -timeout 120s— all passRSHELL_BASH_TEST=1 go test ./tests/ -run TestShellScenariosAgainstBash -timeout 120s— requires Docker🤖 Generated with Claude Code