Security: command injection & path traversal via shell-string command construction

[kevmtt]

Security review: shell-string command construction enables command injection & path traversal

A security scan of the src/ tree surfaced one root cause with several exploitable exits. All findings were confirmed by independent manual data-flow tracing and a second reviewer.

Root cause

src/utils/exec.ts wraps execSync(cmd: string) / execLive / execSafe. Passing a string to execSync runs it through the system shell (/bin/sh -c). Every call site builds the command by string-interpolating values into double quotes — but double quotes do not stop $(...) / backtick command substitution, and they don't help when a value itself contains a ". Branch names, worktree paths, the repo directory name, and compose-file contents are all attacker-influenceable and flow into these strings.

Switching exec.ts to execFileSync(file, args[]) (no shell) closes findings 1, 3, and 4 in one change.

---

Finding 1 — Command injection via git branch name → RCE (src/git/promote.ts:14)

Severity: HIGH

getWorktrees (src/git/worktree.ts:25) parses branch names from git worktree list; resolveRef then runs:

exec(`git -C "${repoRoot}" rev-parse "${ref}"`);

Git refnames legally allow $, `, (, ), ;, &, | (only space/~^:?*[\ are forbidden). So $(...) is a valid branch name and is command-substituted by the shell before git runs.

Exploit: a repo contains (or a teammate pushes) a branch named $(touch /tmp/pwned). A developer targets it with wtc promote, or an AI agent calls the MCP wtc_promote / wtc_start tool against the repo — the payload executes. The MCP path is the sharpest edge: an agent operating on an untrusted repo gets RCE with no human in the loop.

Fix: execFileSync("git", ["-C", repoRoot, "rev-parse", "--", ref]). The -- also blocks option injection for refs starting with -.

---

Finding 2 — Path traversal via attacker-controlled docker-compose.yml → host file exfiltration (src/sync/files.ts:34-58)

Severity: HIGH

getDockerfiles reads build.context / build.dockerfile from the compose file and computes path.relative(repoRoot, fullPath), which can return ../../.... syncWorktreeFiles then does copyFile(path.join(repoRoot, rel), path.join(wtPath, rel)).

Exploit: a malicious branch ships a compose file with

services:
  app:
    build:
      context: ../../
      dockerfile: etc/passwd   # or ~/.ssh/id_rsa

wtc start copies the host file into the worktree, where the attacker can read it.

Fix: after computing rel, reject it if rel.startsWith("..") || path.isAbsolute(rel). Apply the same containment check to composeRel and the extraSync entries.

---

Finding 3 — Command injection via worktree path (src/git/worktree.ts:10,55,60, src/commands/clean.ts:33)

Severity: HIGH

Worktree paths from git worktree list are interpolated into git -C "${wtPath}" … and worktree remove "${wt.path}". A path containing " or $(...) breaks out / injects. Less remotely controllable than a branch name (paths are created locally), but the same shell sink.

Fix: argument-array execFileSync, as above.

---

Finding 4 — Command injection via repo directory name in docker … ls filters (src/commands/clean.ts:42-60)

Severity: MEDIUM

repoName = path.basename(repoRoot) is interpolated unsanitized into docker network ls --filter "name=${repoName}-wt-" (and volumes). A repo cloned into a directory named repo$(id) injects. The subsequent docker rm -f ${staleContainers} also splices command output unquoted — container IDs from -aq are hex (safe), but network/volume names are not guaranteed hex.

Fix: run repoName through the existing sanitize() (as composeProjectName already does), and split('\n')-then-spread the ID/name lists into execFileSync arguments instead of interpolating.

---

What's already solid

• sanitize() ([a-z0-9-] only) is correctly applied to compose project names everywhere they reach the shell — that path is not injectable.
• YAML parsing uses the yaml package's safe default loader (no code-exec tags).
• No hardcoded secrets; env-sync / injectPortOverrides is pure fs; promoteFiles uses fs.copyFileSync.
• MCP tools accept only Zod-validated numeric indices.

No SQLi, auth, crypto, deserialization, or XSS issues — this is a local CLI with no web surface or datastore. The real attack surface is shell-out construction and file-path handling.

---

Suggested simple updates (remediation order)

1. Rewrite src/utils/exec.ts around execFileSync(file, args[]) and convert call sites to pass argument arrays. Single change; kills findings 1, 3, 4 and the option-injection variants. Example helper:

   import { execFileSync } from "node:child_process";
   
   export function run(file: string, args: string[], opts?: ExecFileSyncOptions): string {
     return (execFileSync(file, args, { encoding: "utf-8", ...opts }) as string).trim();
   }

2. Add a repo-root containment check in src/sync/files.ts (finding 2 — a filesystem bug not fixed by Is it compatible with podman #1):

   const rel = path.relative(repoRoot, fullPath);
   if (rel.startsWith("..") || path.isAbsolute(rel)) {
     throw new Error(`Path escapes repository root: ${rel}`);
   }

3. Validate refs with git check-ref-format (or an [a-zA-Z0-9/_.-] allowlist) before use, as defense-in-depth.

---

Reported via an automated + manual security review. Happy to open a PR for the exec.ts refactor and the path-containment fix if useful.