deploy: device/host interpolated unquoted into shell script (command injection)

[lee-reinhardt]

Summary

avocado deploy's device/host string is interpolated, unquoted and unvalidated, into a generated shell script that runs in the SDK build container. A host value containing shell command substitution ($(...) or backticks) executes as code. DeviceSpec::parse performs no character validation, so any byte sequence except an embedded @/: reaches the interpolation.

Where

• src/commands/runtime/deploy.rs:30 — DeviceSpec::parse splits on @ and the last : only; it never validates the host character set.
• src/commands/runtime/deploy.rs:720 — create_deploy_script builds the deploy script with format! and interpolates the parsed host:
  • line 730: SSH_DEST="{ssh_dest}" (ssh_dest = spec.ssh_destination() → root@<host>)
  • line 732: DEVICE_HOST="{device_host}" (device_host = spec.host)

Double quotes in shell do not suppress command substitution — $(...), backticks, and ${...} still expand inside "...". So the generated script contains an attacker-influenced expansion.

Reproduction

avocado deploy <runtime> -d '$(id>&2)'

DeviceSpec::parse("$(id>&2)") yields host = "$(id>&2)" (no @, no :, so no splitting). The script then contains:

SSH_DEST="root@$(id>&2)"
DEVICE_HOST="$(id>&2)"

When the script runs in the SDK container (with a valid built runtime present, before the SSH step), $(id>&2) executes. A backtick form (-d '`id`') and a ${IFS}-based form behave the same. This is command execution inside the build container, which has the project mounts available.

Impact

Code execution in the deploy/build container from the value of -d. In isolation -d is user-supplied, but the same code path is reachable from non-interactive front-ends that pass a caller-influenced device string, so the host string should be treated as untrusted at this boundary regardless of the immediate caller.

Fix

Two layers, both worthwhile:

1. Don't interpolate untrusted values into shell source. Prefer passing SSH_DEST / DEVICE_HOST (and the port) into the script via the child process's environment (Command::env) or argv, rather than string-substituting them into the script body. Environment values are not re-parsed as shell.
2. Validate the host in DeviceSpec::parse. Accept only an IPv4 literal or a DNS/mDNS hostname — an allowlist of [A-Za-z0-9.-] for the host part, plus the existing optional numeric port. That rejects every shell metacharacter ($, `, (, ), ;, &, |, <, >, whitespace) before it can reach the script.

Add regression tests asserting parse rejects $(...), backtick, and ;/|/& host values, alongside the existing create_deploy_script tests at lines 1217/1240.

Note

The consuming desktop app already hardened its own pre-spawn validation to an [A-Za-z0-9.-] + optional-port allowlist, which closes the exposure through that front-end. This issue tracks fixing it at the CLI's own shell boundary so the guarantee holds for every caller.

[lee-reinhardt]

Additional reachability worth flagging for prioritization: this sink is reachable from an unauthenticated caller, not only from a user typing -d at the terminal.

The avocado-desktop app runs a loopback MCP server (127.0.0.1, no auth, also reachable from the dev VM via user-mode NAT) that exposes a free-form run_avocado_cli tool. A caller can invoke avocado deploy <rt> -d 'root@$(...)' (or the non-shortcut runtime deploy form) through it, and the host string flows straight into the interpolated deploy script with no intermediate validation. The desktop's structured deploy path now validates the host to an IPv4/DNS grammar, but the free-form path can't be meaningfully allow-listed without reimplementing device parsing there — the correct fix is here, at the shell boundary, which closes every caller (structured, free-form, and direct CLI) at once.

No change to the proposed fix; this is a note that the practical exposure is broader than interactive use, so it merits prioritizing.