Skip to content

fix(ssh): quote remote workdir and path operands in one-shot execution - #5036

Closed
WAXLYY wants to merge 1 commit into
NousResearch:mainfrom
WAXLYY:fix/ssh-quote-workdir
Closed

fix(ssh): quote remote workdir and path operands in one-shot execution#5036
WAXLYY wants to merge 1 commit into
NousResearch:mainfrom
WAXLYY:fix/ssh-quote-workdir

Conversation

@WAXLYY

@WAXLYY WAXLYY commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

What changed

Quoted remote path operands embedded in SSH shell commands with shlex.quote() in tools/environments/ssh.py.

This includes:

  • one-shot execution workdir handling
  • mkdir -p path operands
  • temporary file reads via cat
  • temp cleanup via rm -f

Also added regression tests covering:

  • injection-like workdir input
  • legitimate workdirs containing spaces
  • quoted temp-path helper behavior

Why

The one-shot SSH execution path embedded workdir directly into the remote shell command without quoting. This could lead to command injection and also broke valid directory paths containing spaces.

Quoting these operands ensures they are treated as data by the remote shell and makes behavior consistent with the persistent shell path.

How to test

  • Run with TERMINAL_ENV=ssh and TERMINAL_SSH_PERSISTENT=false using:
    • workdir="/tmp/a; touch /tmp/pwned #"
    • before the fix, an unintended extra command may run
    • after the fix, it must not
  • Run with:
    • workdir="/tmp/release artifacts"
    • confirm the command executes in the correct directory
  • Verify sudo-based SSH commands also run in the intended directory

Validation

  • pytest tests/tools/test_ssh_environment.py -q14 passed, 11 skipped
  • python -m compileall tools/environments/ssh.py tests/tools/test_ssh_environment.py → successful

@teknium1

teknium1 commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

This is now fixed via PR #5629 (merged), which sanitizes workdir with tilde-aware shlex.quote + an allowlist validator. Thanks for flagging the vulnerability!

@teknium1 teknium1 closed this Apr 6, 2026
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.

2 participants