Skip to content

fix(security): expand $HOME before path validation in downloadFile (#3080)#3081

Merged
louisgv merged 1 commit intomainfrom
fix/issue-3080
Mar 30, 2026
Merged

fix(security): expand $HOME before path validation in downloadFile (#3080)#3081
louisgv merged 1 commit intomainfrom
fix/issue-3080

Conversation

@la14-1
Copy link
Copy Markdown
Member

@la14-1 la14-1 commented Mar 28, 2026

Why: downloadFile (and GCP uploadFile) allowed any $VAR in remote paths — this is a path traversal vector via $OLDPWD, $PWD, etc. The fix removes $ from the allowed charset regex by normalizing $HOME to ~ before validation.

Fixes #3080

Changes

  • digitalocean.ts: downloadFile — expand $HOME~ before regex validation, remove $ from charset
  • aws.ts: downloadFile — same fix
  • sprite.ts: downloadFileSprite — same fix, update variable reference from expandedPath to normalizedRemote
  • gcp.ts: both uploadFile and downloadFile — same fix (GCP had $ in upload too)
  • hetzner.ts: downloadFile — same fix
  • package.json: Bump CLI version 0.27.6 → 0.27.7

Test plan

  • bunx @biomejs/biome check src/ passes with 0 errors
  • bun test — all 1951 tests pass
  • Paths like $HOME/.config still work (normalized to ~/.config)
  • Paths like $OLDPWD/../etc/passwd are now rejected

-- refactor/security-auditor

@la14-1 la14-1 marked this pull request as ready for review March 28, 2026 06:32
@la14-1
Copy link
Copy Markdown
Member Author

la14-1 commented Mar 30, 2026

Rebased onto main and resolved version conflict in packages/cli/package.json (bumped to 0.28.2). Lint and all 1964 tests pass.

-- refactor/pr-maintainer

@la14-1
Copy link
Copy Markdown
Member Author

la14-1 commented Mar 30, 2026

Rebased onto main and resolved version conflict in packages/cli/package.json (bumped to 0.28.3). Lint passes (172 files, 0 errors) and all 1968 tests pass.

-- refactor/pr-maintainer

@la14-1
Copy link
Copy Markdown
Member Author

la14-1 commented Mar 30, 2026

Rebased onto main and resolved version conflict in packages/cli/package.json (bumped to 0.28.3). Lint passes (172 files, 0 errors) and all 2029 tests pass.

-- refactor/pr-maintainer

Fixes #3080

Prevents path traversal via other $VAR expansions by normalizing
$HOME to ~ before the strict path regex check, removing the need
to allow $ in the charset.

Applied to all 5 cloud providers:
- digitalocean: downloadFile
- aws: downloadFile
- sprite: downloadFileSprite
- gcp: uploadFile + downloadFile
- hetzner: downloadFile

Also bumps CLI version to 0.27.7.

Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@la14-1
Copy link
Copy Markdown
Member Author

la14-1 commented Mar 30, 2026

Rebased onto main and resolved version conflict in packages/cli/package.json (bumped to 0.29.2). Lint passes (172 files, 0 errors) and all 1972 tests pass.

-- refactor/pr-maintainer

Copy link
Copy Markdown
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 - Security improvement
Commit: 8e832c1

Findings

No security issues found. This PR improves security by:

  1. Correct path validation order: Expands $HOME BEFORE validation instead of after
  2. Stricter regex: Removes $ from allowed characters (/^[a-zA-Z0-9/_.~-]+$/ instead of /^[a-zA-Z0-9/_.~$-]+$/), preventing shell variable injection
  3. More precise expansion: Uses /^\$HOME\// to require trailing slash, avoiding false positives
  4. Consistent across providers: Applied identically to all 5 cloud providers (AWS, DigitalOcean, GCP, Hetzner, Sprite)

Tests

  • bun test: PASS (1972/1972)
  • biome lint: PASS (172 files, 0 errors)
  • bash -n: N/A (no shell scripts modified)
  • macOS compat: N/A (TypeScript only)

Impact

This fixes issue #3080 where $HOME/... paths could bypass character validation. The fix ensures shell variables are expanded and normalized before security checks, strengthening path traversal protection.


-- security/pr-reviewer

@louisgv louisgv added the security-approved Security review approved label Mar 30, 2026
@louisgv louisgv merged commit 9624141 into main Mar 30, 2026
5 checks passed
@louisgv louisgv deleted the fix/issue-3080 branch March 30, 2026 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approved Security review approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: Path traversal risk via $VAR expansion in remote paths

2 participants