# Security Audit Report — sm121-launcher

**Scope:** whole repository (manual deep review)
**Date:** 2026-07-31
**Method:** manual source + data-flow audit (the `audit` skill's automated phases —
CodeQL, Semgrep Pro, sub-agent Review Chambers, real-VM PoCs — were not runnable in
this environment; no such tooling is installed). Findings below are based on direct
code reading and remote-shell data-flow tracing in `src/`.

## 1. Executive Summary

`sm121` is an operator-run CLI that drives SSH (`russh`) to launch vLLM/SGLang
inference containers on a small fleet of trusted DGX Spark nodes. The overall
architecture is defensible: host keys are pinned and verified per connection,
script uploads are base64-encoded, and the highest-touch data-flow paths
(`resolve_path_ref_on_node`, composed-model dirs) already apply careful shell
quoting. The tool is **not** designed to face untrusted input directly.

The material risks are all on the **trust boundary assumption**: every configured
recipe, `config.toml` value, and mount path is executed as privileged shell on the
DGX nodes (`docker run`, `before_serve`, `mods`, `sync`). Two concrete gaps were
found where attacker-influenced recipe/config content reaches a remote shell
**without** the quoting discipline used elsewhere:

1. **MEDIUM — image sync pipes the operator's private SSH key to an unverified host** (`src/docker/sync.rs:111`)
2. **MEDIUM — unquoted `docker run` interpolation lets a crafted recipe inject shell** (`src/docker/command.rs`)

Two lower-severity / hygiene items (HA token over cleartext HTTP; TOFU host-key
bootstrap) are documented for awareness. One notable **positive**: host-key
verification is mandatory — the client refuses to connect when no key is pinned.

## 2. Threat Model

- **Trusted actors:** the operator and the DGX cluster hosts (a closed deployment).
- **Trust anchors:** local `config.toml` (gitignored), the `recipes/` tree, and the
  operator's SSH private key.
- **Attackers considered:** an external adversary who can (a) get a malicious
  recipe/config committed or supplied to the operator, (b) impersonate/MITM a node
  IP during image sync, or (c) observe traffic on the control LAN. A fully
  compromised node is out of scope (it already has the operator's trust).
- **Boundary:** SSH commands constructed from config/recipe values and executed on
  nodes with `channel.exec` (→ remote shell). Any metacharacter in those values is
  a candidate injection primitive.

## 3. Findings

### F1 — MEDIUM (FIXED): Image sync disables host-key verification and ships the operator key
**File:** `src/docker/sync.rs` (`sync_image_to_nodes`). **Status: remediated** — the inner `ssh docker load` now uses `-o StrictHostKeyChecking=yes -o UserKnownHostsFile=<per-target known_hosts>`; the target's public key is fetched over a verified russh session and written to a temporary `known_hosts` on the source node (removed after the pipe). An impersonator at `target.ssh_ip` is rejected before auth.

```rust
let pipe_cmd = format!(
    "docker save {} | ssh -o StrictHostKeyChecking=no -i {} {}@{} docker load",
    shq(image), shq(key), shq(user), shq(target.ssh_ip));
```

**Impact:** If an attacker can impersonate `target.ssh_ip` (ARP spoofing on a shared
LAN, or a stale/mistyped IP in config) during a multi-node sync, the inner `ssh`
presents the operator's **private key** (`-i <key>`) and accepts any host key. The
attacker receives the key material and/or injects a malicious `docker load` onto a
DGX node. The outer russh hop *is* verified, but the inner one is not.

**Condition:** `cluster.nodes.len() > 1` + `sync_image` + an image to copy. Not hit
by single-node deployments.

**Repro:** configure a second node whose `ssh_ip` is a host under attacker control
that answers TCP/22; run a multi-node `serve` with `sync_image`. The attacker gets
the `-i` key.

**Note:** The exact `StrictHostKeyChecking=no` behavior with `-o` prefixed options
ordering should also be re-checked (OpenSSH may still prompt=hang for unknown keys;
the bypass intent is what matters here).

**Recommendation:** Do not reuse the operator key with `StrictHostKeyChecking=no`.
Reuse the verified russh session to stream the image (pipe over the authenticated
channel) instead of an inner `ssh`, or pin the peering host keys and use
`-o StrictHostKeyChecking=yes HostKeyAlgorithms=...`.

---

### F2 — MEDIUM (FIXED): Unquoted `docker run` interpolation → remote shell injection
**File:** `src/docker/command.rs`, `src/docker/tail.rs`. **Status: remediated** — every interpolated value in the assembled `docker run` command is now shell-quoted via `shq`: `-e 'K=V'`, `-v 'host':'container'[:ro]`, `--name`/`--label` values, the cache-dir and mods mounts, and every tail word (entrypoint, model, args, flags). Two regression tests (`malicious_env_value_cannot_inject`, `malicious_mount_container_cannot_inject`) assert the quoting holds.

```rust
.map(|(k, v)| format!("-e {k}={v}"))            // recipe.[env], mount env names
format!("-v {}:/root/.cache/huggingface", ctx.cache_dir)
format!("-v {}:{}", m.host, m.container)         // extra_mounts resolved host/container
```

The whole `docker run ...` string is joined and executed via
`run_ssh` → `channel.exec(true, ...)` (remote shell). Unlike the careful `shq()`
usage elsewhere (cleanup, path refs, `stop_cluster`), these fields are **not**
shell-quoted.

**Impact:** A recipe `[env]` entry or `extra_mounts` container/host path containing
shell metacharacters — e.g. `FOO = "$(id > /tmp/pwned)"`, `container = "/x; touch /y"`
— executes arbitrary commands as root on the GPU node at serve time.

**Condition:** Attacker must get attacker-controlled content into a recipe/config
(the operator-authored trust boundary). This is **not** remotely reachable on its
own, but it is the concrete way a malicious recipe in the shared repo escalates to
full node execution, and it is inconsistent with the `shq`-everywhere convention in
the same tree.

**Recommendation:** Shell-quote each `-e` value and each `-v` host/container path
(the flag names and `:` separators stay literal) before joining. Validate or quote
the `--name sm121_{recipe_name}_{rank}` slot too.

---

### F3 — LOW: Home Assistant token is transmitted in cleartext
**File:** `src/home_assistant.rs` (`new`), and `config.toml`
`[home_assistant] url = "http://ha.lan:8123"`.

**Impact:** The long-lived HA bearer token travels as `Authorization: Bearer <tok>`
over plaintext HTTP on the control LAN. A passive observer (or MITM) can recover a
token that grants broad HA API access (entity control), and can spoof power-on/off.

**Condition:** HA endpoint served over `http://` (as currently configured) and an
observer on the LAN path.

**Recommendation:** Terminate HA behind TLS and set `url = "https://...".`

---

### F4 — INFO: TOFU host-key bootstrap & long-lived token in config
**Files:** `src/host_keys.rs`, `src/main.rs`.

- `init-host-keys` pins whatever key the server presents on first contact
  (trust-on-first-use). A MITM during the *first* run would be pinned. Standard for
  this tool class; acceptable, but worth documenting and verifying the pin
  out-of-band once.
- The HA token lives in plaintext in `config.toml`. It is **gitignored** (verified,
  `config.toml` is in `.gitignore`), so it is not committed. It is, however, present
  in the local working tree. Restrict file mode (e.g. `chmod 600 config.toml`) and
  treat the token as a long-lived credential with broad HA API access.

---

### Confirmed-strong controls (no action)
- **Host key verification is enforced:** `ssh_session_with_timeout` bails if
  `host_key` is unset; `VerifyingHandler` compares a SHA-256 of the presented key
  against the pinned fingerprint (`src/ssh.rs`). Good.
- **Script uploads / dir uploads are base64-encoded** with shell-quoted local paths
  (`upload_script`, `upload_dir` in `src/ssh.rs`). Good.
- **Model path-ref resolution** (`resolve_path_ref_on_node`) and **composed-model
  dir** (`docker/compose.rs`) carefully `shq()` every path component, and validate
  glob/exclude to an allow-list of `[A-Za-z0-9*?._-/]` to prevent injection. Good.
- Config/recipe/TOML are parsed with standard `toml`/`serde`; no unsafe
  deserialization (no `serde_json::Value` from untrusted input entering code paths).

## 4. Methodology Summary

Adapted the `audit` skill (10-phase) to a manual, single-agent run: repository
inventory → dependency/SBOM review (`Cargo.toml`) → threat-model construction →
data-flow tracing of every place a config/recipe value reaches a remote shell →
inline enrichment (is input attacker-controlled? does it cross a trust boundary?)
→ severity calibration with the trusted-operator threat model. Automated SAST
phases (CodeQL, Semgrep Pro, sub-agent chambers) were **not** run — the tools are
not installed in this environment, so results here are qualitative.

## 5. Findings Summary

| ID | Severity | Title | Status |
|----|----------|-------|--------|
| F1 | MEDIUM | Image sync uses `StrictHostKeyChecking=no` and ships the operator key | FIXED |
| F2 | MEDIUM | Unquoted `docker run` env/mount interpolation → remote shell injection | FIXED |
| F3 | LOW | HA bearer token sent over cleartext HTTP | open |
| F4 | INFO  | TOFU host-key bootstrap; long-lived HA token in config file | open |

## 6. Conclusion

The codebase is generally well-structured for its trust model, with notably strong
host-key pinning and careful quoting in the trickiest path-resolution code. The two
Medium findings both sit on the "recipe/config content is privileged" boundary and
are the highest-value hardening targets: (F1) never hand the operator key to an
unverified inner `ssh`, and (F2) apply the same `shq` discipline already used
elsewhere to every `docker run` interpolation. Recommend fixing both before any
deployment where recipes may be sourced from a less-trusted location. No
remotely-exploitable, unauthenticated vulnerability affecting the intended
(single-operator, pinned-cluster) deployment was found.