← Back to PRs

#20177: fix(security): block command substitution in unquoted heredoc bodies

by orlyjamie open 2026-02-18 16:22 View on GitHub →
maintainer size: S
## Summary Fixes an allowlist bypass in the shell command analyzer where unquoted heredoc bodies were not scanned for command substitution tokens. **Advisory:** https://github.com/openclaw/openclaw/security/advisories/GHSA-65rx-fvh6-r4h2 ## Problem `splitShellPipeline` in `src/infra/exec-approvals-analysis.ts` parses shell commands to enforce exec allowlists. When it encounters a heredoc (`<<EOF`), it correctly identifies the delimiter but then skips the entire heredoc body without checking for disallowed tokens. In bash, when the heredoc delimiter is **unquoted**, the shell performs command substitution on the body. This means: ```bash cat <<EOF $(id) EOF ``` The analyzer sees `cat <<EOF`, checks that `cat` is allowlisted, and approves the command. But `bash -c` then executes `$(id)` inside the heredoc body - bypassing the allowlist entirely. This is exploitable in unattended gateway deployments where the allowlist is the primary security boundary and there is no human to review non-allowlisted commands. ## Fix - `parseHeredocDelimiter` now returns whether the delimiter was `quoted` (single or double quoted) or unquoted - `HeredocSpec` stores the `quoted` state - When scanning an unquoted heredoc body, the parser now rejects commands containing `` ` ``, `$(`, or `${` tokens - Quoted heredocs (`<<'EOF'` / `<<"EOF"`) are unaffected - the shell treats their body as literal text, so no expansion occurs ## Test plan - [x] Rejects `$(cmd)` in unquoted heredoc body - [x] Rejects backtick substitution in unquoted heredoc body - [x] Rejects `${VAR}` expansion in unquoted heredoc body - [x] Rejects nested command substitution in unquoted heredoc - [x] Allows `$(cmd)` in single-quoted heredoc body (safe - shell treats as literal) - [x] Allows `$(cmd)` in double-quoted heredoc body (safe - shell treats as literal) - [x] Allows plain text in unquoted heredoc body - [x] Existing heredoc tests still pass <!-- greptile_comment --> <h3>Greptile Summary</h3> Closes a real security hole in the shell command analyzer's heredoc handling. When a heredoc uses an **unquoted** delimiter (`<<EOF`), bash performs command substitution on the body — but the parser was skipping the body entirely, allowing `$(cmd)`, backticks, and `${...}` to sneak past the allowlist. The fix adds a `quoted` flag to `HeredocSpec` and `parseHeredocDelimiter`, then rejects unquoted heredoc bodies containing expansion tokens (`$(`/`${`/backtick). Quoted heredocs (`<<'EOF'`/`<<"EOF"`) are correctly left alone since the shell treats their bodies as literal. - **Security**: Blocks the allowlist bypass described in GHSA-65rx-fvh6-r4h2 - **Test coverage**: 7 new tests covering `$(cmd)`, backticks, `${VAR}`, nested substitution, quoted heredocs, and plain text — all well-targeted - **Minor test gap**: No test for `<<-` (strip-tabs) combined with an unquoted delimiter containing command substitution; the code handles it correctly, but explicit coverage would strengthen confidence <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — it correctly closes a real allowlist bypass with a well-scoped, minimal fix. - The fix is correct, minimal, and well-tested. The `quoted` flag accurately distinguishes safe (quoted) from unsafe (unquoted) heredocs, and the token scanning in the body loop catches the dangerous patterns. Score is 4 instead of 5 only because there's a minor test gap (no `<<-` + unquoted + substitution test) and no coverage for escaped expansion (`\$(cmd)`) which would be a false positive rather than a security gap. - No files require special attention — changes are minimal and well-contained. <sub>Last reviewed commit: a54bba6</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs