Skip to content

[POC] PAR: wire AllowedCommandPatterns + DeniedCommandPatterns from runCommand task input#50384

Draft
matt-dz wants to merge 1 commit intomainfrom
matthew.deguzman/rshell-command-patterns
Draft

[POC] PAR: wire AllowedCommandPatterns + DeniedCommandPatterns from runCommand task input#50384
matt-dz wants to merge 1 commit intomainfrom
matthew.deguzman/rshell-command-patterns

Conversation

@matt-dz
Copy link
Copy Markdown
Contributor

@matt-dz matt-dz commented May 5, 2026

What does this PR do?

Plumbs two new rshell pattern axes from the runCommand task input through the Datadog Agent's Private Action Runner (PAR) to the rshell interpreter:

  • AllowedCommandPatterns [][]string — argv-prefix patterns shaped (command [, subcommand_path...]) that admit invocations whose leading structural tokens match (ignoring flags per a registered CommandSpec).
  • DeniedCommandPatterns [][]string — same shape, deny-first precedence: a deny match short-circuits the gate to a refusal even when an allow rule would otherwise admit.

Run() passes both fields straight to interp.AllowedCommandPatterns(...) / interp.DeniedCommandPatterns(...) without any agent-side parsing. The JSON wire format preserves the structured token-list shape natively, so there's nothing to split.

Motivation

POC for the rshell permission-model RFC. The architectural claim is that fine-grained scope enforcement on shell commands has to happen post-expansion on the runner — substitution-defeat attacks like $(printf ip) route show are invisible to any static caller. rshell already enforces this; this PR makes the surface area available end-to-end so the wf-actions-server can inject patterns into task payloads and have the agent honour them.

The classic carve-out use case for the deny axis: an operator can express "allow ip wholesale, but block ip route specifically" with AllowedCommands={rshell:ip} plus DeniedCommandPatterns=[["ip","route"]]. ip addr and ip link still admit; ip route is refused at the gate.

Wire format

{
  "command": "ip route show",
  "allowedCommands": ["rshell:ip"],
  "deniedCommandPatterns": [["ip", "route"]],
  "allowedPaths": {"default": ["/tmp"]}
}

Out of scope (intentionally)

  • Operator-side intersection. The existing RunCommandHandler intersects backend AllowedCommands and AllowedPaths with operator config. For patterns, the backend is authoritative for now — adding operator-side pattern config means extending the agent's YAML schema and intersection logic, neither of which adds to the POC's argument.
  • CommandSpecs registration. rshell ships a built-in spec for ip (the only builtin with multi-token subcommand structure). External-command pattern enforcement (kubectl/git/docker) needs an ExecHandler first, which is a much bigger lift.
  • Production-ready dependency wiring. Includes a replace github.com/DataDog/rshell => /Users/matthew.deguzman/dd/rshell directive in go.mod. Strip before this graduates out of POC — it's local-dev scaffolding that lets the pattern work be validated before a tagged rshell release lands.

Describe how you validated your changes

  • 5 new unit tests in `run_command_test.go` covering: allow-pattern admit/block, deny-pattern block-even-when-name-allows, deny-doesn't-false-positive-on-siblings, missing-pattern-fields-don't-break-runner.
  • 4 existing assertions updated for rshell's new `"not permitted by policy"` error format (was `"command not allowed"`).
  • All tests in `pkg/privateactionrunner/bundles/remoteaction/rshell/` pass: `go test ./pkg/privateactionrunner/bundles/remoteaction/rshell/...` is green.
  • Architectural cases (substitution-defeat, structural matching, deny-first precedence) live in rshell's own test suite (DataDog/rshell#222); here we only verify field plumbing.

End-to-end smoke test inside a Lima Linux VM with the local rshell built in:

```bash
RSH=./rshell
$RSH --allowed-commands rshell:ip --denied-command-patterns "ip route" -p /tmp -c 'ip route show'

rshell: ip route show: blocked by deny pattern "ip route"

exit 127 ✓

$RSH --allowed-commands rshell:ip --denied-command-patterns "ip route" -p /tmp -c 'ip addr show'

(real interface listings)

exit 0 ✓

```

Additional Notes

  • The companion rshell PR is DataDog/rshell#222 (also draft).
  • The dd-source side (proto extension to `ScriptScope`, EG/EP-backed Provider, server-side injectors) is the next step. Until those land, the new fields are dead code on the wire — the agent reads them correctly but no enqueue path writes them.
  • Pre-existing flake in `pkg/privateactionrunner/adapters/config/transform_test.go` is unrelated to this change (verified by stash-testing against main).

🤖 Generated with Claude Code

Plumbs the two new rshell pattern axes from the runCommand task input
to the rshell interpreter:

  RunCommandInputs gains:
    AllowedCommandPatterns [][]string
    DeniedCommandPatterns  [][]string

  Run() passes them straight to interp.AllowedCommandPatterns(...) and
  interp.DeniedCommandPatterns(...) without any agent-side parsing
  (the JSON wire format preserves the structured token-list shape
  natively, so there's nothing to split or unflatten).

Operator-side intersection is deliberately skipped — for the POC the
backend is authoritative for patterns. CommandSpecs registration is
also skipped; rshell's built-in spec for `ip` covers the demo target,
and external-command pattern enforcement (kubectl/git/docker) needs
an ExecHandler anyway, which is a much bigger lift.

Includes a local replace directive in go.mod pointing at
~/dd/rshell. STRIP THIS BEFORE THIS PATCH GRADUATES OUT OF POC — it's
dev scaffolding that lets the pattern work be tested before a tagged
rshell release lands.

Tests:
  - 5 new tests covering allow-pattern admit/block, deny-pattern
    block-even-when-name-allows, deny doesn't false-positive on
    siblings, missing pattern fields don't break the runner.
  - 4 existing assertions updated for rshell's new
    "not permitted by policy" error format (was "command not allowed").

Architectural cases (substitution-defeat, structural matching,
deny-first precedence) live in rshell's own test suite; here we only
verify field plumbing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dd-octo-sts dd-octo-sts Bot added the internal Identify a non-fork PR label May 5, 2026
@github-actions github-actions Bot added the medium review PR review might take time label May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Identify a non-fork PR medium review PR review might take time team/action-platform

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant