Uh oh!
There was an error while loading. Please reload this page.
security: use shellQuote() in agent-setup.ts for consistent null-byte defense - #2546
Conversation
… defense Agent: code-health Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: c0e1f8f
Findings
No security issues found. This PR strengthens the codebase's security posture.
Security Improvements
This PR replaces manual shell escaping with the centralized shellQuote() function in agent-setup.ts:
Changes:
- Line 243 - GitHub token: Replaced manual
replace(/'/g, "'\\''")withshellQuote(githubToken) - Line 257 - Git user.name: Replaced manual escaping with
shellQuote(hostGitName) - Line 260 - Git user.email: Replaced manual escaping with
shellQuote(hostGitEmail)
Security Benefits:
- Null-byte defense:
shellQuote()rejects null bytes (\0) which could truncate strings at the C/OS level - Consistency: Uses the same battle-tested escaping logic as other cloud modules (GCP, AWS, Hetzner, DigitalOcean)
- Defense-in-depth: Even though callers validate tokens/email addresses,
shellQuote()provides an additional security layer - Test coverage: The
gcp-shellquote.test.tsfile contains comprehensive tests for shell injection vectors (command substitution, pipes, semicolons, backticks, variable expansion)
Original code pattern:
constescaped=githubToken.replace(/'/g,"'\\''");ghCmd=`export GITHUB_TOKEN='${escaped}' && ${ghCmd}`;New pattern:
ghCmd=`export GITHUB_TOKEN=${shellQuote(githubToken)} && ${ghCmd}`;Both patterns use POSIX single-quote escaping ('\'' technique), but shellQuote() adds null-byte validation.
Tests
- bash -n: N/A (no shell scripts modified)
- bun test: PASS (1380 tests, 0 failures)
- curl|bash: N/A (only TypeScript changes)
- macOS compat: N/A (only TypeScript changes)
Verification
All existing tests pass. The change maintains identical shell escaping semantics while adding null-byte protection. The PR also bumps the CLI version from 0.17.0 to 0.17.1 (appropriate patch version increment for a security improvement).
-- security/pr-reviewer
Uh oh!
There was an error while loading. Please reload this page.
Dead backwards-compat re-export left over from the shellQuote consolidation (PRs #2533, #2535, #2546). Zero consumers import shellQuote from gcp/gcp.ts — all correctly import from shared/ui.ts. Per CLAUDE.md: avoid backwards-compatibility hacks; delete unused code. Agent: code-health Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
… defense (OpenRouterLabs#2546) Agent: code-health Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Dead backwards-compat re-export left over from the shellQuote consolidation (PRs #2533, #2535, #2546). Zero consumers import shellQuote from gcp/gcp.ts — all correctly import from shared/ui.ts. Per CLAUDE.md: avoid backwards-compatibility hacks; delete unused code. Agent: code-health Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Dead backwards-compat re-export left over from the shellQuote consolidation (PRs #2533, #2535, #2546). Zero consumers import shellQuote from gcp/gcp.ts — all correctly import from shared/ui.ts. Per CLAUDE.md: avoid backwards-compatibility hacks; delete unused code. Agent: code-health Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
… defense (OpenRouterLabs#2546) Agent: code-health Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Why: PR #2535 consolidated shellQuote() across all cloud modules to add null-byte validation, but missed 3 inline .replace() calls in shared/agent-setup.ts. This closes that gap for GitHub tokens and git identity values passed over SSH.
Changes
packages/cli/src/shared/agent-setup.ts: ImportshellQuotefrom./uiand replace 3 inline.replace(/'/g, "'\\''")patterns withshellQuote()calls — adds null-byte rejection as defense-in-depthpackages/cli/package.json: Patch version bump (0.17.0 → 0.17.1)Verification
bunx @biomejs/biome check src/— 0 errorsbun test— 1380 pass, 0 fail-- refactor/code-health