Skip to content

fix(security): prevent shell injection in CLI tools#4

Closed
riaworks wants to merge 6 commits intothiagofinch:mainfrom
riaworks:fix/shell-injection-cli
Closed

fix(security): prevent shell injection in CLI tools#4
riaworks wants to merge 6 commits intothiagofinch:mainfrom
riaworks:fix/shell-injection-cli

Conversation

@riaworks
Copy link

@riaworks riaworks commented Mar 1, 2026

Summary

  • Replace all execSync with execFileSync + array-based arguments in bin/push.js and bin/lib/installer.js
  • Eliminates shell interpretation of user-controlled data (commit messages, file paths, tokens)
  • Use http.extraheader for git clone authentication instead of embedding token in URL
  • Add token format validation regex

Security Findings Addressed

Finding Severity CVSS File OWASP LLM Fix
M-02 Medium 6.1 bin/push.js LLM06 execSyncexecFileSync with array args
M-04 Medium 5.3 bin/lib/installer.js LLM06 execSyncexecFileSync with array args
L-04 Low 3.7 bin/lib/installer.js LLM06 Token removed from clone URL
L-09 Low 2.0 bin/lib/installer.js LLM06 Token format validation added
L-10 Low 2.0 bin/lib/installer.js LLM06 http.extraheader for auth

Changes

bin/push.js

  • Import changed from execSync to execFileSync
  • git() helper refactored to accept array arguments and use execFileSync('git', args)
  • gitSilent() refactored to use spread args
  • All 20+ call sites updated across Layer 1, 2, 3 push flows and helper functions
  • Commit message escaping (replace(/"/g, '\\"')) removed — no longer needed without shell interpretation

bin/lib/installer.js

  • Import changed from execSync to execFileSync
  • Token format validation: rejects tokens with non-alphanumeric characters
  • Git clone uses http.extraheader with Base64-encoded Authorization header
  • Token no longer embedded in clone URL (prevents leakage in ps aux and logs)

Test plan

  • mega-brain push --layer 1 (standard git flow)
  • mega-brain push --layer 2 (manifest + force-add + push --force)
  • mega-brain push --layer 3 (backup flow + secret unstaging)
  • mega-brain push --dry-run (no side effects)
  • Premium install with valid GitHub token
  • Premium install with invalid token (should reject)
  • Commit messages with special chars (", `, $, |)

Report

Full security audit: Riaworks LLM Security

🤖 Generated with Claude Code

aquilatrindade and others added 6 commits February 27, 2026 19:25
All previous history squashed for security hygiene.
Repository fully sanitized - no residual sensitive data.
…jection

- bin/push.js: Replace all execSync/shell string calls with execFileSync
  and array-based arguments across Layer 1, 2, and 3 push flows, removing
  shell interpretation of user-controlled data (commit messages, file paths)
- bin/lib/installer.js: Replace execSync git clone with execFileSync,
  use http.extraheader instead of embedding token in clone URL (prevents
  token leakage in process listings and logs), add token format validation

Findings addressed: M-02, M-04, L-04, L-09, L-10

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

3 participants