[VasiliyRad]

The bug (security — RCE)

Two-stage chain that lets a Discord user achieve arbitrary code execution on the owner's machine.

Stage 1: discord-bridge saves attachment filenames verbatim

src/discord-bridge.py (two sites, lines 2304 + 2357):

```python
local_path = INBOX_DIR / f"{int(time.time()*1000)}_{att.filename}"
```

Discord allows arbitrary filenames: spaces, quotes, semicolons, backticks, $, ... An attacker uploads x\"; touch /tmp/pwn; #.jpg. The bridge saves it as /tmp/discord-inbox/1779392000000_x\"; touch /tmp/pwn; #.jpg.

Stage 2: conversation-server fast path shell-splices the path

skills/phone-conversation/scripts/conversation-server.ts:310:

```ts
const image = execSync('ls -t /tmp/discord-inbox/*.jpg ...').toString().trim();
const result = execSync(`bash .../prepend-image.sh "${image}" "${video}" 3`, ...);
```

When the owner is on a phone call and says "prepend image to video," the image glob picks up the malicious filename. execSync (/bin/sh -c) expansion:

```bash
bash .../prepend-image.sh "/tmp/discord-inbox/.._x"; touch /tmp/pwn; #.jpg" "..." 3
```

The first arg to bash is \"/tmp/discord-inbox/..._x\"; the closing \" ends the argument. Then ; touch /tmp/pwn runs as a shell command, then ; #.jpg\" ... is a comment.

Concrete RCE from a Discord attachment + a single voice trigger from the owner.

Fix — two layers

Layer A: sanitize at the boundary

src/discord-bridge.py: new _safe_attachment_basename() helper. Mirrors _safe_id from src/agent-api.py. Applied at both attachment save sites.

Layer B: defense-in-depth at the use

skills/phone-conversation/scripts/conversation-server.ts: switch the fast path from
```ts
execSync(`bash .../prepend-image.sh "${image}" "${video}" 3`)
```
to
```ts
const scriptPath = `${process.env.HOME}/.claude/skills/video-concat/scripts/prepend-image.sh`;
execFileSync('bash', [scriptPath, image, video, '3'], ...);
```

execFileSync bypasses /bin/sh -c. Closes the injection class regardless of filename content.

Tests

8 cases on the sanitizer, 3 cases on the execFileSync wiring. All 11 pass locally; npm run typecheck clean; Python syntax clean.

Test plan

• python3 tests/discord-bridge-attachment-filename-sanitize.test.py — 8/8 pass
• npx tsx --test tests/conversation-server-fastpath-execfile.test.ts — 3/3 pass
• Typecheck clean
• Manual: upload Discord attachment named pwn\"; touch /tmp/pwn-RCE; #.jpg; verify saved path has shell-meta chars stripped and /tmp/pwn-RCE does NOT appear after triggering the fast path

🤖 Generated with Claude Code

[liususan091219]

Reviewed the Discord-attachment RCE fix. The two-stage chain is correctly diagnosed and the two-layer fix closes it at both ends — ready to merge.

Things I like:

1. Defense at the boundary AND at the use. Either layer alone would close the RCE, but doing both is the right call: _safe_attachment_basename() (discord-bridge.py) means no dangerous filename ever lands in /tmp/discord-inbox/ for any consumer, and the execFileSync switch (conversation-server.ts:~310) protects the fast path even if a future write site forgets to sanitize.
2. _safe_attachment_basename is carefully built — allowlist [a-zA-Z0-9_\-.], .strip("._") to kill leading .. traversal, length cap, and extension-preservation so the downstream *.jpg/*.png globs keep matching legitimate uploads. I traced ../../../etc/passwd through it: /→_, leading dotted run stripped → etc_passwd, no slash, no .. — matches the test's assertions.
3. Mirroring _safe_id from agent-api.py keeps the sanitizer shape consistent across the codebase's two attacker-facing ingest points — one mental model, not two.
4. The fast-path fix correctly handles the no-shell consequence: ~ doesn't expand under execFileSync, so the script path is rebuilt as ${process.env.HOME}/.claude/.... Easy detail to miss; the PR got it right.
5. Both attachment save sites in discord-bridge.py (the embed-snapshot loop ~2335 and the main-message loop ~2391) are patched — not just the one in the headline example.

Non-blocking observations:

• The .py test actually executes _safe_attachment_basename against real adversarial inputs (quotes, backticks, $IFS, traversal, unicode, 10KB name) — genuine behavioral coverage, stronger than the source-grep guards. The .ts side is source-grep (consistent with the repo convention).
• _safe_attachment_basename collapses every disallowed char to _, so two distinct uploads (a b.jpg, a-b.jpg) can sanitize to the same basename — but the int(time.time()*1000)_ timestamp prefix already disambiguates, so no collision in practice. Just noting it isn't an injective transform.

Cross-PR: conversation-server.ts is also touched by #1019 (the /meeting handler) and #1018 (config path) in different regions — no textual conflict expected with this PR's import line + fast-path change, but second-to-merge should re-run typecheck.

Tests: 8 behavioral py assertions + 3 ts guards; the PR body reports all passing.

Correctly diagnosed RCE, fixed at both the boundary and the use, with real behavioral tests on the sanitizer. LGTM.
— Lucy (Mac Studio bot)