[VasiliyRad]

The bug (security — higher impact than PR #37)

skills/phone-conversation/scripts/conversation-server.ts binds to 0.0.0.0 to accommodate ngrok's local-tunnel client + the Twilio webhook path. That makes every endpoint LAN-reachable. /twilio/* paths validate Twilio's signature; the rest are unauthenticated control endpoints:

Endpoint	What an unauth'd LAN caller can do
POST /call	Originate an outbound Twilio call from the owner's account (costs money, reveals caller-ID, harassment vector)
POST /concurrent-call	Spam-dial multiple targets in parallel using the owner's account
POST /hangup	Terminate any active owner call
POST /play-audio	Stream /tmp/sutando-* audio during owner's active calls (PR #37 gated the path but the endpoint itself was still unauth'd)
POST /stop-audio	Stop owner's audio playback mid-call
POST /meeting, /meeting-approve	Start meeting-mode / upgrade to task-delegation

Concrete repro:

```bash
curl -X POST http://:3100/call
-d '{"to":"+1xxxxxxxxxx","message":"go"}'
```

Triggers an outbound call from the owner's Twilio account. No auth check, no rate limit, no logging of the caller.

Why this is the right fix

New loopback_guard.ts exports isLoopback(req) — a one-line predicate against req.socket.remoteAddress covering IPv4 (127.0.0.1), IPv6 (::1), and IPv4-mapped IPv6 loopback (::ffff:127.0.0.1).

In the request handler, BEFORE the endpoint dispatch:

```ts
if (path !== '/health'
&& !path.startsWith('/twilio/')
&& !isLoopback(req)) {
json(res, 403, { error: 'control endpoints are loopback-only' });
return;
}
```

• /twilio/* exempt — validated downstream by TWILIO_AUTH_TOKEN HMAC signature check.
• /health exempt — read-only liveness probe.
• Everything else is loopback-only.

Logs the rejected remote address to stderr for operator triage.

Alternative considered: switch bind from 0.0.0.0 to 127.0.0.1 and route ngrok separately. Rejected because it changes the server's deployment shape — would break installs where ngrok isn't the tunneling solution. The endpoint-level gate is cleaner and preserves the deployment surface.

Why this is a good-quality change

• Maintainability: one predicate, one gate, before the dispatch table. Easy to audit. Matches the existing in-codebase pattern (_is_path_sendable for files; this is the network-layer equivalent).
• Testability: isLoopback extracted to its own tiny module so unit tests don't have to load conversation-server.ts (which has heavy module-load side effects).
• Robustness: closes a real unauth'd-RCE-shape attack on the owner's Twilio account. Combined with PR fix(phone-conversation): allowlist body.path on /play-audio (LAN-reachable RCE-shape) #37 (path allowlist), the phone server is now properly hardened.
• Low risk: doesn't change the bind address. ngrok still works. Twilio webhooks still reach /twilio/* over the public tunnel. /health LAN-side probes still work. Legit owner-side callers always hit from loopback (the agent's inline-tools.ts does fetch('http://localhost:3100/...') — verified loopback).

Tests

tests/phone-conversation-loopback-guard.test.ts — 14 cases:

10 unit tests of isLoopback:

Input	Loopback?
127.0.0.1	✅
::1	✅
::ffff:127.0.0.1 (IPv4-mapped IPv6)	✅
192.168.1.42 (LAN v4)	❌
10.0.0.5 (private v4)	❌
1.1.1.1 (public v4)	❌
fe80::1 (link-local v6)	❌
fc00::1 (ULA v6)	❌
::ffff:192.168.1.42 (mapped non-loopback v4)	❌
"" / undefined	❌

4 source-grep architectural assertions on conversation-server.ts:

Assertion	Pins
imports isLoopback from the guard module	Module split intact
gates non-/twilio non-/health endpoints	The actual security check
returns 403 (not 404 or silent)	Operator-visible signal
logs the rejected remote address	Incident triage

All 14 pass locally. npm run typecheck clean.

Test plan

• npx tsx --test tests/phone-conversation-loopback-guard.test.ts — 14/14 pass
• Typecheck clean
• Manual: from another machine on the LAN, curl -X POST http://<owner-host>:3100/call -d '{"to":"+1xxx","message":"x"}' — verify 403 + log. Then from localhost — verify normal handling.

🤖 Generated with Claude Code

[bassilkhilo-ag2]

Cold review (proactive-loop pass)

Vulnerability description is accurate. POST /call with an arbitrary to triggering Twilio calls on the owner's account is a real harassment + cost vector. The other control endpoints (hangup, concurrent-call, meeting-mode) compound the surface.

isLoopback implementation — correct and minimal:

addr === '127.0.0.1' || addr === '::1' || addr === '::ffff:127.0.0.1'

Covers all three loopback forms that req.socket.remoteAddress can return in practice. The 127.0.0.0/8 range isn't needed here — Node.js's HTTP server always reports exactly 127.0.0.1 for loopback connections. Module extraction for testability is the right call given conversation-server.ts's heavy module-load side effects.

Gate placement (before dispatch table, after path parse) is correct — short-circuits before any business logic runs. The /twilio/* and /health exemptions are both justified: Twilio webhooks have their own HMAC validation; health probes are legitimately LAN-visible.

Logging on rejection (REJECTED non-loopback ${method} ${path} from ${remote}) is exactly what an operator needs for incident triage — remote address, method, and path in one line.

10 unit tests of the predicate + 4 source-grep architectural assertions — good split. The ::ffff:192.168.1.42 case (mapped non-loopback) is the one that could have been a false-positive if the implementation used IP prefix matching instead of explicit values.

One minor note (non-blocking): inline-tools.ts's control-endpoint callers use http://localhost:3100/ — which resolves to 127.0.0.1 on most systems, so they'll pass. Worth verifying on IPv6-only stacks where localhost may resolve to ::1; both forms are covered by the predicate, so this is already fine.

LGTM.

[rickchen007]

Approved (admin) — security fix, isLoopback predicate correct (covers IPv4/IPv6/v4-mapped loopback), gate placed before dispatch, /twilio + /health correctly exempt. 14 tests.