Skip to content

fix: validate connection parameters to prevent command injection#1392

Merged
louisgv merged 1 commit intomainfrom
security-scan
Feb 17, 2026
Merged

fix: validate connection parameters to prevent command injection#1392
louisgv merged 1 commit intomainfrom
security-scan

Conversation

@la14-1
Copy link
Member

@la14-1 la14-1 commented Feb 17, 2026

Why: Prevents HIGH severity command injection vulnerability where malicious values in ~/.spawn/history.json could execute arbitrary commands when connecting to or deleting servers.

Summary

Security Impact

Attack Vector (Before Fix):

  1. User's ~/.spawn/history.json gets corrupted or manually edited
  2. Malicious values inserted: {"ip": "8.8.8.8; rm -rf /tmp/pwned", ...}
  3. User runs spawn list and selects the record
  4. Shell command constructed with unvalidated input → code execution

After Fix:

  • All connection parameters validated before use (IP, username, server_id)
  • Strict allowlist patterns reject shell metacharacters
  • IPv4 octets validated to be 0-255
  • Unix usernames validated against standard pattern
  • Server IDs validated to prevent path traversal and injection

Changes

New validation functions in cli/src/security.ts:

  • validateConnectionIP() - validates IPv4/IPv6 or sentinel values ("sprite-console", "fly-ssh", "daytona-sandbox")
  • validateUsername() - validates Unix username pattern (lowercase, starts with letter/underscore)
  • validateServerIdentifier() - validates server names/IDs (alphanumeric, hyphens, dots, underscores only)

Updated functions:

  • cmdConnect() - validates all connection params before constructing SSH/sprite commands
  • buildDeleteScript() - validates server IDs before shell interpolation
  • mergeLastConnection() - validates data from bash scripts before saving to history

Test coverage:

  • Added security-connection-validation.test.ts with 23 test cases
  • Covers valid inputs (IPv4, IPv6, sentinels, usernames, server IDs)
  • Covers invalid inputs (shell metacharacters, path traversal, injection attempts)

Version Bump

  • Bumped CLI version from 0.3.2 → 0.3.3 (security patch)
  • Users will auto-update on next spawn run

Testing

cd cli
bun test security-connection-validation.test.ts  # All 23 tests pass
bun run build                                     # Builds successfully

Verified no regressions in existing test suite (same failures as main branch).


-- refactor/security-auditor

…, #1380)

Add input validation for SSH connection parameters (IP, username, server_name)
and server identifiers used in delete operations. This prevents command injection
attacks if ~/.spawn/history.json is corrupted or tampered with.

Changes:
- Add validateConnectionIP() - validates IPv4/IPv6 addresses and sentinels
- Add validateUsername() - validates Unix username format
- Add validateServerIdentifier() - validates server names/IDs
- Update cmdConnect() to validate all connection params before use
- Update buildDeleteScript() to validate server IDs before interpolation
- Update mergeLastConnection() to validate data from bash scripts
- Add comprehensive test coverage for all validation functions
- Bump CLI version to 0.3.3 (security patch)

Security impact:
- Prevents HIGH severity command injection via history.ip/user (issue #1381)
- Prevents MEDIUM severity command injection via server_id (issue #1380)

Agent: security-auditor

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Member

@louisgv louisgv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED

Summary

This PR adds critical input validation for connection parameters to prevent command injection via corrupted/tampered history files. Implementation is sound with defense-in-depth approach.

Findings

  • LOW (cosmetic) cli/src/security.ts:169-183 — IPv4 validation accepts leading zeros (e.g., "01.1.1.1"). No security risk: parseInt(octet, 10) forces decimal interpretation, and IP is only used in SSH string interpolation already protected by validation.

Security Strengths

  1. Defense in depth — Validation at 3 layers: data merge, SSH connect, server delete
  2. Comprehensive allowlists — All validators block shell metacharacters, path traversal, command substitution
  3. Proper error handling — Graceful degradation on merge, hard fail before use with clear guidance
  4. Excellent test coverage — 23 tests covering edge cases and injection attempts

Tests

  • bash -n: N/A (no shell scripts)
  • bun test (security): PASS (23/23)
  • bun test (all): Pre-existing failures unrelated to PR
  • curl|bash: N/A (TypeScript only)
  • macOS compat: N/A (TypeScript only)
  • Edge cases: PASS (all injection attempts blocked)

Test Results

✓ IPv4 octet range validation (256.x.x.x rejected)
✓ Shell metacharacters blocked (;, &, |, $, backticks)
✓ Command substitution blocked ($(...), `...`)
✓ Path traversal blocked (.., /, \)
✓ Newlines/tabs/spaces blocked in usernames and IDs
✓ Empty/whitespace-only values rejected
✓ Length limits enforced (username: 32, server ID: 128)
✓ Special sentinels accepted (sprite-console, fly-ssh, daytona-sandbox)


-- security/pr-reviewer-1392

@louisgv louisgv merged commit 06351d6 into main Feb 17, 2026
2 of 3 checks passed
@louisgv louisgv deleted the security-scan branch February 17, 2026 11:32
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.

[HIGH] Command injection risk in SSH connection parameter handling [MEDIUM] Command injection risk in buildDeleteScript server ID handling

2 participants