Skip to content

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

Open
riaworks wants to merge 1 commit intothiagofinch:mainfrom
riaworks:fix/shell-injection-cli
Open

fix(security): prevent shell injection in CLI tools#4
riaworks wants to merge 1 commit 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

…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>
@riaworks riaworks requested a review from thiagofinch as a code owner March 1, 2026 22:03
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.

1 participant