[ACTP] PAR: redesign rshell allow-list contract (sentinel defaults)#49949
Open
julesmcrt wants to merge 1 commit intojules.macret/Q/rshell/02-warningsfrom
Open
[ACTP] PAR: redesign rshell allow-list contract (sentinel defaults)#49949julesmcrt wants to merge 1 commit intojules.macret/Q/rshell/02-warningsfrom
julesmcrt wants to merge 1 commit intojules.macret/Q/rshell/02-warningsfrom
Conversation
Replaces the three-way nil/[]/[X] slice contract with a uniform "always
intersect" contract using sentinel defaults:
- allowed_paths default is ["/"]; pathContains("/", X) is true for any
absolute path, so the intersection passes the backend list through
when the operator hasn't narrowed.
- allowed_commands default is ["rshell:*"]; the wildcard token is a
special case in the operator-side intersection that admits every
backend entry in the "rshell:" namespace (scoped via
onlyRshellPrefixedCommands).
The IsConfigured gate and the handler's nil-pass-through bypass are
gone. End-user behavior is preserved: unset operator config gets the
backend list as-is, explicit empty list is the kill-switch, explicit
non-empty narrows.
Path utilities extracted to a new helper.go: cleanPathList,
reducePathListToBroadest, intersectPathLists, commonPath, plus
onlyRshellPrefixedCommands. Comprehensive unit tests in helper_test.go.
julesmcrt
requested review from
s-alad and
samrcarr
and removed request for
a team
julesmcrt
added
changelog/no-changelog
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4ef4305be
ℹ️ 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".
| // - allowed_commands defaults to ["rshell:*"]; the wildcard is a | ||
| // special case in the intersection that admits every backend | ||
| // command in the "rshell:" namespace. | ||
| config.BindEnvAndSetDefault(PARRestrictedShellAllowedPaths, []string{RShellPathAllowAll}) |
There was a problem hiding this comment.
This changes the user-visible default for private_action_runner.restricted_shell.allowed_paths (and the command default just below), but the generated config documentation/schema was not updated: pkg/config/schema/core_schema.yaml still advertises restricted_shell_allowed_paths with default /var/log and there is no documented allowed_commands sentinel. Operators relying on the generated docs will configure or audit the rshell allow-list against the old contract, so the docs/schema should be updated in the same change.
Useful? React with 👍 / 👎.
Contributor
|
🎯 Code Coverage (details) 🔗 Commit SHA: b4ef430 | Docs | Datadog PR Page | Give us feedback! |
Contributor
Files inventory check summaryFile checks results against ancestor c1888d89: Results for datadog-agent_7.80.0~devel.git.265.b4ef430.pipeline.109973441-1_amd64.deb:No change detected |
Contributor
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
21 successful checks with minimal change (< 2 KiB)
On-wire sizes (compressed)
|
s-alad
approved these changes
Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Replaces the three-way
nil/[]/[X]slice contract for the operator-side allow-lists with a uniform "always intersect" contract using sentinel defaults:allowed_pathsdefault is["/"].pathContains("/", X)is true for any absolute path, so the intersection passes the backend list through when the operator hasn't narrowed.allowed_commandsdefault is["rshell:*"]. The wildcard token is a special case in the operator-side intersection: when present, every backend entry in thershell:namespace is admitted (scoped viaonlyRshellPrefixedCommands).The
IsConfiguredgate and the handler's nil-pass-through bypass are gone. End-user behavior is preserved on every axis:[]Helper extraction
Pure path utilities moved to a new
helper.go:cleanPathList,reducePathListToBroadest,intersectPathLists,commonPath— containment-aware path matching with normalized canonical forms.onlyRshellPrefixedCommands— namespace scoping for the wildcard branch.Comprehensive unit tests in
helper_test.gocover every helper plus property tests (idempotence, order-independence) forreducePathListToBroadest.Stacked on top of #49948 (PR2 in the split)
This is PR3 of 4 splitting #49825. Base: PR2 branch. Will retarget after PR1+PR2 land.
Describe how you validated your changes
pkg/privateactionrunner/adapters/config,pkg/privateactionrunner/bundles/remoteaction/rshell, andpkg/config/setup. Linter clean.helper_test.gocovers:commonPath(every shape),cleanPathList(incl. dot-segment edge cases),reducePathListToBroadest(incl. multi-domination + idempotence + order-independence),intersectPathLists(incl. multi-pair containment regression cases),onlyRshellPrefixedCommands(incl. wildcard token / barershell/rshell:alone).run_command_test.gouse the sentinels for the pass-through case and pin the kill-switch / containment / namespace-required scenarios.🤖 Generated with Claude Code