Skip to content

fix(desktop-electron): sanitize CLI command args to prevent OS command injection#17346

Closed
kvenux wants to merge 1 commit intoanomalyco:devfrom
kvenux:fix/cwe-78-desktop-electron-cli-cmd
Closed

fix(desktop-electron): sanitize CLI command args to prevent OS command injection#17346
kvenux wants to merge 1 commit intoanomalyco:devfrom
kvenux:fix/cwe-78-desktop-electron-cli-cmd

Conversation

@kvenux
Copy link
Copy Markdown

@kvenux kvenux commented Mar 13, 2026

Issue for this PR

Closes #17345

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

spawnCommand and buildCommand in packages/desktop-electron/src/main/cli.ts accepted a raw string for CLI arguments and interpolated it directly into shell commands (bash -c, WSL script). If any parameter like hostname contained shell metacharacters (;, $(), backticks), they would be executed by the shell.

Changed both functions to accept string[] instead of string. Each argument is now individually shellEscape()d before interpolation into shell commands. Windows direct mode passes the array directly without shell involvement.

How did you verify your code works?

Added cli.security.test.ts with 13 test cases covering:

  • Shell metacharacter injection via ;, |, &&
  • Command substitution via $() and backticks
  • Normal args regression (no breakage)

All 13 tests pass.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

… (CWE-78)

Change spawnCommand/buildCommand to accept string[] instead of string,
and shellEscape each argument individually before interpolating into
shell commands. This prevents injection via hostname, port, or other
user-controllable parameters.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added needs:compliance This means the issue will auto-close after 2 hours. and removed needs:compliance This means the issue will auto-close after 2 hours. labels Mar 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

@kvenux
Copy link
Copy Markdown
Author

kvenux commented Mar 13, 2026

Closing: after deeper analysis, hostname is hardcoded as "127.0.0.1", port comes from getSidecarPort(), and password is randomUUID(). None of these values are attacker-controllable. The shell injection vector cannot be triggered in practice.

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.

fix(desktop-electron): OS command injection in CLI command spawning (CWE-78)

2 participants