mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-20 01:21:41 -06:00
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 <li.long15@xydigit.com>
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
(cherry picked from commit b37476ab0b)
This commit is contained in:
@@ -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" }));
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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: <invalid verification 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";
|
||||
|
||||
@@ -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 "<invalid verification URL>";
|
||||
}
|
||||
}
|
||||
|
||||
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,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user