diff --git a/skills/phone-conversation/scripts/conversation-server.ts b/skills/phone-conversation/scripts/conversation-server.ts index 1a144eac8e..ebd23ca938 100644 --- a/skills/phone-conversation/scripts/conversation-server.ts +++ b/skills/phone-conversation/scripts/conversation-server.ts @@ -1163,6 +1163,20 @@ async function waitForWebhook(): Promise { // and a WebSocket endpoint for real-time audio streaming (Media Streams). // Claude also calls these endpoints to trigger actions (/call, /concurrent-call, /hangup). +// Control endpoints (anything not under /twilio/*) drive owner-account +// actions: originate outbound calls (/call), hang up active calls +// (/hangup), play audio through Twilio (/play-audio), etc. The server +// binds to 0.0.0.0 because ngrok's local-tunnel client + the Twilio +// webhook path need it (see server.listen at the bottom). That means +// every endpoint is LAN-reachable. /twilio/* paths validate Twilio's +// signature; the rest had no auth at all — any caller on the LAN could +// POST /call with an arbitrary `to` and trigger Twilio calls on the +// owner's account (cost + caller-ID exposure + harassment vector). +// `isLoopback` (see loopback_guard.ts) gates non-/twilio control +// endpoints to loopback only; LAN callers get 403. /health is exempt +// — it's read-only and useful for LAN-side liveness checks. +import { isLoopback } from './loopback_guard.js'; + const server = createServer(async (req, res) => { const url = new URL(req.url ?? '', `http://localhost:${PORT}`); const path = url.pathname; @@ -1172,6 +1186,15 @@ const server = createServer(async (req, res) => { res.end(); return; } + // Reject non-Twilio endpoints from non-loopback callers (see comment + // on `isLoopback` above). + if (path !== '/health' && !path.startsWith('/twilio/') && !isLoopback(req)) { + const remote = req.socket.remoteAddress ?? '?'; + console.log(`${ts()} [PhoneServer] REJECTED non-loopback ${req.method} ${path} from ${remote}`); + json(res, 403, { error: 'control endpoints are loopback-only' }); + return; + } + try { if (path === '/health' && req.method === 'GET') { json(res, 200, { status: 'ok', activeCalls: activeCalls.size, webhookUrl: WEBHOOK_BASE_URL }); diff --git a/skills/phone-conversation/scripts/loopback_guard.ts b/skills/phone-conversation/scripts/loopback_guard.ts new file mode 100644 index 0000000000..dff21e0b28 --- /dev/null +++ b/skills/phone-conversation/scripts/loopback_guard.ts @@ -0,0 +1,31 @@ +// Loopback-only access guard for the phone-conversation HTTP server. +// +// The server 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 +// internal control endpoints (originate calls, hang up, play audio, +// etc.) that should never be exposed to the LAN. +// +// `isLoopback(req)` is the one-line predicate used by the request +// handler to gate non-/twilio control endpoints. Extracted to its own +// module so we can unit-test the predicate without loading +// conversation-server.ts (which has heavy module-load side effects: +// Gemini SDK init, Twilio client, env validation that `exit(1)`s when +// credentials are missing). + +import type { IncomingMessage } from 'node:http'; + +/** + * True iff the request originated from a loopback interface — IPv4 + * `127.0.0.1`, IPv6 `::1`, or the IPv4-mapped-IPv6 form + * `::ffff:127.0.0.1` that some Node builds report when listening on + * IPv4 via a dual-stack socket. + * + * Defensive against the same bypass class as `_is_safe_callback_url` + * in `src/agent-api.py`: a hostname/address that *resolves* to + * loopback via IPv4-mapping must be treated as loopback. + */ +export function isLoopback(req: IncomingMessage): boolean { + const addr = req.socket.remoteAddress ?? ''; + return addr === '127.0.0.1' || addr === '::1' || addr === '::ffff:127.0.0.1'; +} diff --git a/tests/phone-conversation-loopback-guard.test.ts b/tests/phone-conversation-loopback-guard.test.ts new file mode 100644 index 0000000000..716d6b30d7 --- /dev/null +++ b/tests/phone-conversation-loopback-guard.test.ts @@ -0,0 +1,92 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import type { IncomingMessage } from 'node:http'; +import { isLoopback } from '../skills/phone-conversation/scripts/loopback_guard.js'; + +// Security-relevant regression guard for the loopback-only gate on the +// phone-conversation server's control endpoints. +// +// Pre-fix: every endpoint (incl. `/call`, `/hangup`, `/concurrent-call`, +// `/play-audio`, etc.) was LAN-reachable because the server binds to +// `0.0.0.0` for ngrok. /twilio/* paths validate Twilio's signature; +// everything else was unauthenticated. Worst case: anyone on the LAN +// could POST `/call` with an arbitrary phone number and originate a +// Twilio call on the owner's account. + +function fakeReq(remoteAddress: string | undefined): IncomingMessage { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return { socket: { remoteAddress } } as any; +} + +describe('loopback_guard.isLoopback', () => { + it('accepts IPv4 loopback (127.0.0.1)', () => { + assert.equal(isLoopback(fakeReq('127.0.0.1')), true); + }); + it('accepts IPv6 loopback (::1)', () => { + assert.equal(isLoopback(fakeReq('::1')), true); + }); + it('accepts IPv4-mapped IPv6 loopback (::ffff:127.0.0.1)', () => { + assert.equal(isLoopback(fakeReq('::ffff:127.0.0.1')), true); + }); + it('rejects LAN IPv4 (192.168.x.x)', () => { + assert.equal(isLoopback(fakeReq('192.168.1.42')), false); + }); + it('rejects another LAN IPv4 (10.x.x.x)', () => { + assert.equal(isLoopback(fakeReq('10.0.0.5')), false); + }); + it('rejects public IPv4', () => { + assert.equal(isLoopback(fakeReq('1.1.1.1')), false); + }); + it('rejects link-local IPv6 (fe80::)', () => { + assert.equal(isLoopback(fakeReq('fe80::1')), false); + }); + it('rejects ULA IPv6 (fc00::)', () => { + assert.equal(isLoopback(fakeReq('fc00::1')), false); + }); + it('rejects IPv4-mapped IPv6 to a NON-loopback address', () => { + // Defensive: the SSRF-bypass shape (private IPv4 mapped into + // IPv6) must NOT be treated as loopback unless it actually maps + // to 127.0.0.1. + assert.equal(isLoopback(fakeReq('::ffff:192.168.1.42')), false); + assert.equal(isLoopback(fakeReq('::ffff:10.0.0.5')), false); + }); + it('rejects empty / undefined remoteAddress', () => { + assert.equal(isLoopback(fakeReq('')), false); + assert.equal(isLoopback(fakeReq(undefined)), false); + }); +}); + +describe('conversation-server.ts wiring', () => { + const SRC = readFileSync( + join(import.meta.dirname ?? '.', '..', 'skills/phone-conversation/scripts/conversation-server.ts'), + 'utf-8', + ); + + it('imports isLoopback from the guard module', () => { + assert.match(SRC, /from\s+['"]\.\/loopback_guard(?:\.js)?['"]/, ( + 'conversation-server.ts must import isLoopback from ./loopback_guard' + )); + }); + + it('gates non-/twilio non-/health endpoints with isLoopback', () => { + assert.match( + SRC, + /path\s*!==\s*['"]\/health['"][\s\S]{0,80}!\s*path\.startsWith\(['"]\/twilio\/['"]\)[\s\S]{0,80}!isLoopback\(req\)/, + 'conversation-server.ts must check `path !== "/health" && !path.startsWith("/twilio/") && !isLoopback(req)` before dispatching to control endpoints', + ); + }); + + it('returns 403 (not 200) on the rejection path', () => { + assert.match( + SRC, + /json\(res,\s*403,\s*\{\s*error:\s*['"]control endpoints are loopback-only['"]/, + 'conversation-server.ts must respond 403 with the documented error string', + ); + }); + + it('logs the rejected remote address for incident triage', () => { + assert.match(SRC, /REJECTED non-loopback/); + }); +});