mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 15:59:30 +00: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>
This commit is contained in:
parent
2f66c3c0c8
commit
b37476ab0b
4 changed files with 104 additions and 12 deletions
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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: <invalid verification 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";
|
||||
|
|
|
|||
|
|
@ -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 "<invalid verification URL>";
|
||||
}
|
||||
}
|
||||
|
||||
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,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue