From 2968bcd576e640ef686c5973e67ca94c96aa50ee Mon Sep 17 00:00:00 2001 From: Alix-007 Date: Thu, 9 Jul 2026 18:19:40 +0800 Subject: [PATCH] fix(twilio): redact webhook turnToken diagnostics (#102089) * fix(twilio): redact webhook turnToken diagnostics * chore(twilio): rerun ci * fix(twilio): contain webhook URL diagnostics Co-authored-by: Alix-007 --------- Co-authored-by: Peter Steinberger (cherry picked from commit b37476ab0b6416ab0ca98b9a894256ec3ad27e0e) --- .../voice-call/src/providers/twilio.test.ts | 32 ++++++++++ .../src/providers/twilio/webhook.ts | 3 - .../voice-call/src/webhook-security.test.ts | 60 +++++++++++++++++-- extensions/voice-call/src/webhook-security.ts | 21 ++++++- 4 files changed, 104 insertions(+), 12 deletions(-) diff --git a/extensions/voice-call/src/providers/twilio.test.ts b/extensions/voice-call/src/providers/twilio.test.ts index db998b1732ff..af3df8ab3db4 100644 --- a/extensions/voice-call/src/providers/twilio.test.ts +++ b/extensions/voice-call/src/providers/twilio.test.ts @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; 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"; @@ -113,6 +114,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("sends direct initial TwiML for notify-mode outbound calls", async () => { const provider = createProvider(); const apiRequest = createApiRequestMock(async () => ({ sid: "CA123", status: "queued" })); diff --git a/extensions/voice-call/src/providers/twilio/webhook.ts b/extensions/voice-call/src/providers/twilio/webhook.ts index 8306d85f83f0..d8dccab13050 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 85f4091ece78..7e4af245d77f 100644 --- a/extensions/voice-call/src/webhook-security.test.ts +++ b/extensions/voice-call/src/webhook-security.test.ts @@ -547,7 +547,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({ @@ -564,15 +564,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", () => { @@ -677,7 +722,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", () => { @@ -759,7 +805,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", () => { @@ -811,7 +858,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 07ff69d15edc..ccc695b65706 100644 --- a/extensions/voice-call/src/webhook-security.ts +++ b/extensions/voice-call/src/webhook-security.ts @@ -372,6 +372,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); @@ -413,7 +428,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; @@ -681,11 +696,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, }; }