Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions skills/phone-conversation/scripts/conversation-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1163,6 +1163,20 @@ async function waitForWebhook(): Promise<void> {
// 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;
Expand All @@ -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 });
Expand Down
31 changes: 31 additions & 0 deletions skills/phone-conversation/scripts/loopback_guard.ts
Original file line number Diff line number Diff line change
@@ -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';
}
92 changes: 92 additions & 0 deletions tests/phone-conversation-loopback-guard.test.ts
Original file line number Diff line number Diff line change
@@ -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/);
});
});
Loading