[VasiliyRad]

The bug (security)

skills/phone-conversation/scripts/conversation-server.ts exposes a POST /play-audio endpoint that currently does:

```ts
if (!body.path) { json(res, 400, ...); return; }
if (!existsSync(body.path)) { json(res, 404, ...); return; }
// ... spawn ffmpeg with body.path as -i
const ffmpegProc = spawn('ffmpeg', [...seekArgs, '-re', '-i', body.path, ...]);
ffmpegProc.stdout.on('data', (pcmBuf) => {
// mu-law encode + stream over Twilio WebSocket to active phone call
});
```

Two structural problems compound:

1. No authentication on the endpoint. The phone server binds to 0.0.0.0 to accommodate ngrok's local-tunnel client → any caller on the LAN can POST /play-audio.
2. No allowlist on body.path — only existsSync(). The attacker chooses any local file the server's user can read; ffmpeg opens it; its audio (if any decodable track) streams over Twilio to whoever is on the active phone call.

Concretely: if the owner is on a phone call, anyone on the LAN can POST {"path": "/path/to/owner-private-recording.mov"} and the unrelated call participant suddenly hears the recording.

Fix

Adopt the established _is_path_sendable pattern used in discord-bridge.py and telegram-bridge.py for [file: /path] marker delivery: explicit allowlist of path prefixes + realpath collapse to defeat symlink / .. escapes.

New file skills/phone-conversation/scripts/audio_path_guard.ts owns the allowlist + helper:

```ts
export const AUDIO_ALLOWED_PREFIXES = ['/tmp/sutando-', '/private/tmp/sutando-'];

export function isAllowedAudioPath(fpath: string): boolean {
// existsSync -> realpathSync -> prefix check
}
```

Matches the recording skill's /tmp/sutando-recording-*.mov convention used by the legitimate callers (src/recording-tools.ts).

The /play-audio handler replaces its existsSync() precheck with isAllowedAudioPath() and returns 403 (not 404) on rejected paths so operator logs distinguish "missing file" from "policy violation."

Scope note: the second ffmpeg spawn site in conversation-server.ts (QuickTime-resume re-stream, around line ~1645) uses a path derived from an internal ls -t /tmp/sutando-recording-*.mov shell glob — NOT user-controlled — so no additional gate is needed there.

Tests

tests/phone-conversation-audio-path-guard.test.ts — 8 cases:

Case	Pins
allows files directly under /tmp/sutando-	Common-case happy path
rejects /etc/hosts	Canonical disallowed file
rejects /tmp/other-not-sutando-	Different prefix
rejects non-existent file	existsSync precheck
rejects null/empty/undefined/non-string	Defensive typing
rejects symlink → outside allowlist	Precise reason for realpathSync
rejects .. traversal	Same guard via textual traversal
AUDIO_ALLOWED_PREFIXES documented-set	Architectural — silent widening fails

All 8 pass locally. npm run typecheck clean.

Note on the 2 pre-existing failing tests observed locally (task-bridge-format, task-bridge-isvoice): both fail with ERR_UNKNOWN_BUILTIN_MODULE: No such built-in module: node:sqlite. Verified these fail on untouched main too — local Node version doesn't expose node:sqlite. Unrelated to this PR.

Test plan

• npx tsx --test tests/phone-conversation-audio-path-guard.test.ts — 8/8 pass
• npm run typecheck — clean
• Manual: with phone server running, POST /play-audio with {"path": "/etc/hosts"} — verify 403 + no ffmpeg spawn. Then POST with a real /tmp/sutando-recording-*.mov during an active call — verify normal playback.

Found while reviewing phone-conversation server. Happy to adjust scope, naming, or test surface if the maintainers prefer a different shape.

🤖 Generated with Claude Code

[CLAassistant]

All committers have signed the CLA.

[sonichi]

LGTM. The bug is genuinely bad — LAN-reachable file exfil during an active phone call. 0.0.0.0 bind + only existsSync(body.path) validation means any LAN device can POST /play-audio with {"path": "/path/to/private-recording.mov"} and ffmpeg's decoded audio streams over Twilio to whoever happens to be on the call.

Fix mirrors the _is_path_sendable pattern from discord-bridge.py + telegram-bridge.py. Allowlist /tmp/sutando-* + /private/tmp/sutando-* (macOS realpath expansion). realpathSync defeats symlink + .. escapes. Allowlist is tight by design with a comment about adding new sources explicitly.

Both ffmpeg-spawn sites covered (the /play-audio handler + the QuickTime-resume re-stream). Easy to miss the second one; PR body explicitly calls it out.

[sonichi]

Hi @VasiliyRad — really nice batch, all 14 reviewed LGTM (the 2 already merged + 12 still open).

Update: thanks for signing the CLA — all 12 open PRs now show license/cla = pass. We'll handle the rebases on our side (maintainerCanModify: true on all of them), starting with #990 first so #985 cleanly rebases onto the consolidated _split_file_markers helper, then the rest in any order.

You won't need to touch them again unless rebase produces conflicts.

Thanks for the careful security/UX hardening pass — composability across PRs (e.g. #985 building on #990's refactor) was thoughtful.

[VasiliyRad]

Thanks @sonichi — and thanks for handling the rebases on your side. Acknowledged on the merge order (#990 first so #985 picks up the consolidated _split_file_markers helper cleanly). I'll keep watching for rebase-conflict pings and respond same-day if any come up; otherwise I'll stay out of the way.

The composability note is appreciated — I deliberately staged #985 after #990 so dm-result.py could import from the shared chunker/marker layer rather than re-defining drift-prone copies. Same reason #988 (proactive routing) depends on the state/last-owner-activity.json shape that's already established — leaning on existing primitives rather than inventing new coordination.

Happy to send follow-ups if any of these surface gaps once they're on main. Specifically open to:

• A REST multipart upload path for dm-result.py so file markers in fix(dm-result): honor tierMap + strip file markers from REST DM body #985 actually deliver attachments (called out as follow-up in the PR body).
• The corresponding port of fix(phone-conversation): gate non-/twilio control endpoints to loopback only #992's loopback gate to src/web-server.ts and src/dashboard.py-adjacent servers (this PR was scoped to phone-conversation; the same shape applies to other 0.0.0.0-bound endpoints).
• An IPv4-mapped IPv6 audit pass on any other private-IP checks in the codebase (mirror of fix(agent-api): close SSRF bypass via IPv4-mapped IPv6 hostnames #981's projection).

Each is a contained PR. Just point me at the priority order if/when you want them.

[qingyun-wu]

Nice one! LGTM!