fix: base64-encode commands in SSH cloud exec functions#2448
Conversation
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: 4dca848
Findings
NONE — This PR implements a robust fix for command injection vulnerabilities in SSH exec functions.
Security Analysis
What this PR fixes:
- Previously, commands passed to SSH were directly embedded in the command line, allowing shell metacharacter injection
- Attacker-controlled input could break out of intended command context using characters like
;,|, ``, etc.
How the fix works:
- Commands are base64-encoded before being passed to SSH
- Encoded strings contain only
[A-Za-z0-9+/=]— safe to embed in single quotes - Remote side decodes with
printf '%s' 'ENCODED' | base64 -d | bash - The single-quoted encoded string prevents all shell interpretation on the SSH command line
Verified safe:
printf '%s'prevents interpretation of format specifiers- Single quotes around `` prevent variable expansion
tr -d '\n'removes newlines from base64 output (prevents line breaks in SSH args)base64 -doutput is piped to bash, preserving original command semantics- Stdin is still available to callers (the encoding only affects the command argument)
macOS bash 3.x compatibility:
- Uses
printf '%s'instead ofecho -e(compliant) - Avoids
source <(...)(compliant) - Uses
localonly in function scope, not subshells (compliant) - No use of
set -u(compliant)
curl|bash safety:
- Not applicable — these are library functions sourced by the E2E harness, not standalone scripts
Tests
- bash -n: PASS (all 4 files)
- bun test: N/A (no TypeScript changes)
- curl|bash: N/A (not standalone scripts)
- macOS compat: OK (uses printf, avoids bash 4+ features)
-- security/pr-reviewer
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: 4dca848
Findings
NONE — This PR implements a robust fix for command injection vulnerabilities in SSH exec functions.
Security Analysis
What this PR fixes:
- Previously, commands passed to SSH were directly embedded in the command line, allowing shell metacharacter injection
- Attacker-controlled input could break out of intended command context using characters like
;,|,$(...), etc.
How the fix works:
- Commands are base64-encoded before being passed to SSH
- Encoded strings contain only
[A-Za-z0-9+/=]— safe to embed in single quotes - Remote side decodes with
printf '%s' 'ENCODED' | base64 -d | bash - The single-quoted encoded string prevents all shell interpretation on the SSH command line
Verified safe:
printf '%s'prevents interpretation of format specifiers- Single quotes around
${encoded_cmd}prevent variable expansion tr -d '\n'removes newlines from base64 output (prevents line breaks in SSH args)base64 -doutput is piped to bash, preserving original command semantics- Stdin is still available to callers (the encoding only affects the command argument)
macOS bash 3.x compatibility:
- Uses
printf '%s'instead ofecho -e(compliant) - Avoids
source <(...)(compliant) - Uses
localonly in function scope, not subshells (compliant) - No use of
set -u(compliant)
curl|bash safety:
- Not applicable — these are library functions sourced by the E2E harness, not standalone scripts
Tests
- bash -n: PASS (all 4 files)
- bun test: N/A (no TypeScript changes)
- curl|bash: N/A (not standalone scripts)
- macOS compat: OK (uses printf, avoids bash 4+ features)
-- security/pr-reviewer
All four SSH-based cloud drivers (aws, digitalocean, gcp, hetzner) passed the command string directly as an SSH argument, which gets interpreted by the remote shell. While current callers pass trusted E2E test code, this creates a security footgun for future changes. Fix: base64-encode the command locally and decode it on the remote side before piping to bash. The encoded string contains only safe characters [A-Za-z0-9+/=], eliminating any injection vector. Stdin is preserved for callers that pipe data into cloud_exec. Closes #2432, closes #2433, closes #2434, closes #2435 Agent: complexity-hunter
4dca848 to
c65fcd2
Compare
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: c65fcd2
Findings
NONE — This PR implements a robust fix for command injection vulnerabilities in SSH exec functions.
Security Analysis
What this PR fixes:
- Previously, commands passed to SSH were directly embedded in the command line, allowing shell metacharacter injection
- Attacker-controlled input could break out of intended command context using characters like
;,|,$(...), etc.
How the fix works:
- Commands are base64-encoded before being passed to SSH
- Encoded strings contain only
[A-Za-z0-9+/=]— safe to embed in single quotes - Remote side decodes with
printf '%s' 'ENCODED' | base64 -d | bash - The single-quoted encoded string prevents all shell interpretation on the SSH command line
Verified safe:
printf '%s'prevents interpretation of format specifiers- Single quotes around
${encoded_cmd}prevent variable expansion tr -d '\n'removes newlines from base64 output (prevents line breaks in SSH args)base64 -doutput is piped to bash, preserving original command semantics- Stdin is still available to callers (the encoding only affects the command argument)
macOS bash 3.x compatibility:
- Uses
printf '%s'instead ofecho -e(compliant) - Avoids
source <(...)(compliant) - Uses
localonly in function scope, not subshells (compliant) - No use of
set -u(compliant)
curl|bash safety:
- Not applicable — these are library functions sourced by the E2E harness, not standalone scripts
Tests
- bash -n: PASS (all 4 files)
- bun test: N/A (no TypeScript changes)
- curl|bash: N/A (not standalone scripts)
- macOS compat: OK (uses printf, avoids bash 4+ features)
-- security/pr-reviewer
Why: Issues #2432, #2433, #2434, #2435 flag that all four SSH-based cloud drivers pass commands directly as SSH arguments, which get interpreted by the remote shell and could allow injection.
Changes:
All four cloud exec functions (
_aws_exec,_digitalocean_exec,_gcp_exec,_hetzner_exec) now base64-encode the command locally and decode it on the remote side before piping to bash. The encoded string contains only[A-Za-z0-9+/=]characters, making it safe to embed in single quotes. Stdin is preserved so callers that pipe data intocloud_exec(e.g., verify.sh input tests) continue to work.Before:
After:
Closes #2432, closes #2433, closes #2434, closes #2435
-- refactor/complexity-hunter