← Back to PRs

#16525: fix(shell): stop rejecting newlines in double-quoted args (#16470)

by yinghaosang open 2026-02-14 20:47 View on GitHub →
stale size: S trusted-contributor
## Summary `splitShellPipeline` rejects literal newlines inside double-quoted strings, which blocks auto-approval for commands like `gh issue create --body "## Heading\n\ntext"`. The analysis bails with `ok: false` before safeBin or allowlist checks run, so there's no workaround short of `--body-file`. Closes #16470 lobster-biscuit ## Root Cause `iterateQuoteAware` in `exec-approvals-analysis.ts` has a `\n`/`\r` check in the `inDouble` block that treats literal newlines as unsupported tokens. But POSIX shells allow newlines inside double-quoted strings — they're just part of the string value. The `$()` and backtick checks already guard against command substitution, so the newline check isn't doing any security work. ## Changes - Before: any command with a newline inside double quotes gets `ok: false`, skipping all approval checks - After: newlines inside double quotes are treated as string content (same as single quotes already do) Unquoted newlines are still rejected via `DISALLOWED_PIPELINE_TOKENS`. ## Tests - `exec-approvals.test.ts` — 2 new tests (literal `\n` and `\r\n` inside double quotes), both fail before fix, pass after - All 56 existing tests pass - `pnpm build && pnpm check` pass (pre-existing format issue in unrelated `.agents/skills/submit-pr/SKILL.md`) <!-- greptile_comment --> <h3>Greptile Summary</h3> Removes overly restrictive newline check from double-quoted string parsing, allowing commands like `gh issue create --body "## Heading\n\ntext"` to pass approval checks. Also fixes security audit to recognize dangerous node commands as valid when used in `denyCommands`. **Main changes:** - Removed newline/carriage-return rejection inside double-quoted strings in `splitShellPipeline` - Added `DEFAULT_DANGEROUS_NODE_COMMANDS` to known commands list in security audit - Added test coverage for literal newlines and `\r\n` inside double quotes - Added test to verify dangerous commands aren't flagged as unknown in audit **Security validation:** - Command substitution (`$()` and backticks) still blocked inside double quotes - Unquoted newlines still rejected via `DISALLOWED_PIPELINE_TOKENS` - Aligns behavior with POSIX shell semantics and existing single-quote handling <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with no security risks - The fix is minimal, well-tested, and addresses a legitimate bug. Security checks for command substitution remain intact, unquoted newlines are still blocked, and the change aligns with POSIX shell behavior. The audit fix correctly adds dangerous commands to the known commands list without compromising security. - No files require special attention <sub>Last reviewed commit: 7510c6b</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs