Skip to content

[Windows] fix invoke build; path separator characters are different in#526

Merged
derekwbrown merged 2 commits into
masterfrom
db/fixwinbuild
Sep 5, 2017
Merged

[Windows] fix invoke build; path separator characters are different in#526
derekwbrown merged 2 commits into
masterfrom
db/fixwinbuild

Conversation

@derekwbrown

Copy link
Copy Markdown
Contributor

windows

@olivielpeau olivielpeau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙇

@derekwbrown
derekwbrown merged commit 1881110 into master Sep 5, 2017
@derekwbrown
derekwbrown deleted the db/fixwinbuild branch September 5, 2017 22:01
gh-worker-dd-mergequeue-cf854d Bot pushed a commit that referenced this pull request Jun 22, 2026
## What does this PR do?

Adds a new Private Action Runner action **`com.datadoghq.remoteaction.rshell.runRemediationCommand`**, a sibling of the existing `runCommand`. It runs the restricted shell ([rshell](https://github.com/DataDog/rshell)) in **remediation mode** (`interp.ModeRemediation`) instead of read-only mode.

Remediation mode (introduced in rshell [#526](DataDog/rshell#526) / [#532](DataDog/rshell#532)) permits:
- file-target output redirections — `>`, `>>`, `2>`, `&>`, `&>>`
- write-oriented builtins such as `truncate`

all still confined to the configured `AllowedPaths` sandbox.

### Changes
- Bump `github.com/DataDog/rshell` `v0.0.20` → `v0.0.21`.
- Refactor `RunCommandHandler` to be parameterized by `interp.Mode`; `runCommand` keeps read-only behavior, `runRemediationCommand` uses remediation mode. The two actions share all sandboxing/allowlist-intersection logic.
- Register `runRemediationCommand` in the rshell bundle.
- Unit tests + parallel E2E tests + release note.

## Motivation

Enable operator-authorized remediation workflows (writing/truncating files within the sandbox) while keeping `runCommand` strictly read-only.

## Describe how you validated your changes

- `dda inv test --targets=./pkg/privateactionrunner/bundles/remoteaction/rshell/` — 126 unit tests pass, including new coverage:
  - both actions register with the expected mode
  - remediation: in-sandbox redirect writes the file
  - read-only: same redirect is rejected, no file created
  - remediation: out-of-sandbox redirect is still rejected
- E2E `tests/privateactionrunner` package compiles; added `TestRshellRemediationWriteFile` (write + read-back) and `TestRshellRunCommandBlocksWrite`.

## Security / rollout notes

- ⚠️ **Not default-enabled.** `runRemediationCommand` is intentionally left out of `defaultCommonActionFQNs`. Being write-capable, it must be explicitly added to a runner's `actions_allowlist`.
- Operators already using a wildcard allowlist entry (e.g. `com.datadoghq.remoteaction.rshell.*` or `com.datadoghq.remoteaction.*`) would pick this action up automatically — worth a security reviewer's attention.
- Path/command allowlists (`restricted_shell.allowed_paths` / `allowed_commands`) are **shared** with `runCommand`. A follow-up could add a separate, narrower write-allowlist if desired.

## Possible drawbacks / trade-offs

- Shared allowlist means operators can't currently grant broad read paths but narrow write paths in one config.

## Additional Notes

Draft — pending review of the default-enablement decision and whether a dedicated remediation allowlist is wanted.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: jules.macret <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants