diff --git a/extensions/voice-call/src/providers/twilio.test.ts b/extensions/voice-call/src/providers/twilio.test.ts index 4ef8d6d0e79..4c85eb63eb6 100644 --- a/extensions/voice-call/src/providers/twilio.test.ts +++ b/extensions/voice-call/src/providers/twilio.test.ts @@ -12,6 +12,7 @@ vi.mock("./shared/guarded-json-api.js", () => ({ import type { WebhookContext } from "../types.js"; import { TwilioProvider } from "./twilio.js"; import { TwilioApiError } from "./twilio/api.js"; +import { verifyTwilioProviderWebhook } from "./twilio/webhook.js"; const STREAM_URL = "wss://example.ngrok.app/voice/stream"; @@ -123,6 +124,37 @@ function configureTelephonyTwiMlFallback(params: { providerCallId: string; strea } describe("TwilioProvider", () => { + it("redacts turnToken query params from failed verification warnings", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + + try { + const result = verifyTwilioProviderWebhook({ + ctx: { + headers: { + host: "example.com", + "x-twilio-signature": "invalid", + }, + rawBody: "CallSid=CS123&CallStatus=completed&From=%2B15550000000", + url: "https://example.com/voice/twilio?callId=call-1&turnToken=secret-turn-token", + method: "POST", + query: { callId: "call-1", turnToken: "secret-turn-token" }, + }, + authToken: "test-auth-token", + currentPublicUrl: null, + options: {}, + }); + + const messages = warn.mock.calls.map((call) => call.join(" ")).join("\n"); + expect(result.ok).toBe(false); + expect(messages).toContain("turnToken=***"); + expect(messages).toContain("callId=***"); + expect(messages).not.toContain("secret-turn-token"); + expect(warn).toHaveBeenCalledOnce(); + } finally { + warn.mockRestore(); + } + }); + it("uses the derived regional hostname for call status", async () => { guardedJsonApiRequestMock.mockResolvedValue({ status: "completed" }); const provider = new TwilioProvider({ diff --git a/extensions/voice-call/src/providers/twilio/webhook.ts b/extensions/voice-call/src/providers/twilio/webhook.ts index 8306d85f83f..d8dccab1305 100644 --- a/extensions/voice-call/src/providers/twilio/webhook.ts +++ b/extensions/voice-call/src/providers/twilio/webhook.ts @@ -24,9 +24,6 @@ export function verifyTwilioProviderWebhook(params: { if (!result.ok) { console.warn(`[twilio] Webhook verification failed: ${result.reason}`); - if (result.verificationUrl) { - console.warn(`[twilio] Verification URL: ${result.verificationUrl}`); - } } return { diff --git a/extensions/voice-call/src/webhook-security.test.ts b/extensions/voice-call/src/webhook-security.test.ts index 3d1a3151f58..eba694dc9c8 100644 --- a/extensions/voice-call/src/webhook-security.test.ts +++ b/extensions/voice-call/src/webhook-security.test.ts @@ -583,7 +583,7 @@ describe("verifyTwilioWebhook", () => { it("uses request query when publicUrl omits it", () => { const authToken = "test-auth-token"; const publicUrl = "https://example.com/voice/webhook"; - const urlWithQuery = `${publicUrl}?callId=abc`; + const urlWithQuery = `${publicUrl}?callId=abc&turnToken=secret-turn-token`; const postBody = "CallSid=CS123&CallStatus=completed&From=%2B15550000000"; const signature = twilioSignature({ @@ -600,15 +600,60 @@ describe("verifyTwilioWebhook", () => { "x-twilio-signature": signature, }, rawBody: postBody, - url: "http://local/voice/webhook?callId=abc", + url: "http://local/voice/webhook?callId=abc&turnToken=secret-turn-token", method: "POST", - query: { callId: "abc" }, + query: { callId: "abc", turnToken: "secret-turn-token" }, }, authToken, { publicUrl }, ); expect(result.ok).toBe(true); + expect(result.verificationUrl).toBe(urlWithQuery); + }); + + it("redacts query params from invalid Twilio signature diagnostics", () => { + const result = verifyTwilioWebhook( + { + headers: { + host: "example.com", + "x-twilio-signature": "invalid", + }, + rawBody: "CallSid=CS123&CallStatus=completed&From=%2B15550000000", + url: "https://example.com/voice/webhook?callId=call-1&turnToken=secret-turn-token", + method: "POST", + query: { callId: "call-1", turnToken: "secret-turn-token" }, + }, + "test-auth-token", + { publicUrl: "https://user:pass@example.com/callback#fragment-secret" }, + ); + + expect(result.ok).toBe(false); + expect(result.reason).toContain("Invalid signature for URL:"); + expect(result.reason).toContain("turnToken=***"); + expect(result.reason).toContain("callId=***"); + expect(result.reason).not.toContain("secret-turn-token"); + expect(result.reason).not.toContain("user:pass"); + expect(result.reason).not.toContain("fragment-secret"); + }); + + it("does not echo malformed verification URLs in diagnostics", () => { + const result = verifyTwilioWebhook( + { + headers: { "x-twilio-signature": "invalid" }, + rawBody: "CallSid=CS123", + url: "https://local/voice/webhook", + method: "POST", + }, + "test-auth-token", + { publicUrl: "not a url?turnToken=secret-turn-token" }, + ); + + expect(result).toMatchObject({ + ok: false, + reason: "Invalid signature for URL: ", + }); + expect(result.reason).not.toContain("secret-turn-token"); }); it("treats changed idempotency header as replay for identical signed requests", () => { @@ -713,7 +758,8 @@ describe("verifyTwilioWebhook", () => { expect(result.ok).toBe(false); // Attacker's host is ignored - uses Host header instead - expect(result.verificationUrl).toBe("https://legitimate.example.com/voice/webhook"); + expect(result.reason).toContain("https://legitimate.example.com/voice/webhook"); + expect(result.verificationUrl).toBeUndefined(); }); it("uses X-Forwarded-Host when allowedHosts whitelist is provided", () => { @@ -795,7 +841,8 @@ describe("verifyTwilioWebhook", () => { expect(result.ok).toBe(false); // Attacker's host not in whitelist, falls back to Host header - expect(result.verificationUrl).toBe("https://localhost/voice/webhook"); + expect(result.reason).toContain("https://localhost/voice/webhook"); + expect(result.verificationUrl).toBeUndefined(); }); it("trusts forwarding headers only from trusted proxy IPs", () => { @@ -875,7 +922,8 @@ describe("verifyTwilioWebhook", () => { ); expect(result.ok).toBe(false); - expect(result.verificationUrl).toBe("https://legitimate.example.com/voice/webhook"); + expect(result.reason).toContain("https://legitimate.example.com/voice/webhook"); + expect(result.verificationUrl).toBeUndefined(); }); it("succeeds when Twilio signs URL without port but server URL has port", () => { const authToken = "test-auth-token"; diff --git a/extensions/voice-call/src/webhook-security.ts b/extensions/voice-call/src/webhook-security.ts index b02bdbe5ff5..1203c4eee16 100644 --- a/extensions/voice-call/src/webhook-security.ts +++ b/extensions/voice-call/src/webhook-security.ts @@ -310,6 +310,21 @@ function buildTwilioVerificationUrl( } } +function redactTwilioVerificationUrlForDiagnostics(url: string): string { + try { + const parsed = new URL(url); + parsed.username = parsed.username ? "***" : ""; + parsed.password = parsed.password ? "***" : ""; + parsed.hash = parsed.hash ? "#***" : ""; + for (const key of Array.from(parsed.searchParams.keys())) { + parsed.searchParams.set(key, "***"); + } + return parsed.toString(); + } catch { + return ""; + } +} + function stripPortFromUrl(url: string): string { try { const parsed = new URL(url); @@ -351,7 +366,7 @@ function extractPortFromHostHeader(hostHeader?: string): string | undefined { interface TwilioVerificationResult { ok: boolean; reason?: string; - /** The URL that was used for verification (for debugging) */ + /** The original URL that passed signature verification; never set on failures. */ verificationUrl?: string; /** Whether we're running behind ngrok free tier */ isNgrokFreeTier?: boolean; @@ -619,11 +634,11 @@ export function verifyTwilioWebhook( // Check if this is ngrok free tier - the URL might have different format const isNgrokFreeTier = verificationUrl.includes(".ngrok-free.app") || verificationUrl.includes(".ngrok.io"); + const diagnosticVerificationUrl = redactTwilioVerificationUrlForDiagnostics(verificationUrl); return { ok: false, - reason: `Invalid signature for URL: ${verificationUrl}`, - verificationUrl, + reason: `Invalid signature for URL: ${diagnosticVerificationUrl}`, isNgrokFreeTier, }; }