Compare commits

...

5 Commits

Author SHA1 Message Date
Peter Steinberger
ee2cfa5ca1 fix: harden voice-call webhooks (#1213) (thanks @andrew-kurin) 2026-01-19 06:00:01 +00:00
Ghost
0e1ebc7da1 Voice-call: avoid streaming on notify callbacks 2026-01-19 05:48:06 +00:00
Ghost
0e3e6f6b70 Voice-call: fix Twilio status callbacks 2026-01-19 05:48:06 +00:00
Ghost
337dd41ce9 Voice-call: fix Twilio signature ordering 2026-01-19 05:48:06 +00:00
Ghost
88336ef4ea Voice-call: fix tailscale tunnel path 2026-01-19 05:48:06 +00:00
8 changed files with 126 additions and 18 deletions

View File

@ -20,6 +20,7 @@ Docs: https://docs.clawd.bot
- TUI: show generic empty-state text for searchable pickers. (#1201) — thanks @vignesh07. - TUI: show generic empty-state text for searchable pickers. (#1201) — thanks @vignesh07.
- Doctor: canonicalize legacy session keys in session stores to prevent stale metadata. (#1169) - Doctor: canonicalize legacy session keys in session stores to prevent stale metadata. (#1169)
- CLI: centralize CLI command registration to keep fast-path routing and program wiring in sync. (#1207) — thanks @gumadeiras. - CLI: centralize CLI command registration to keep fast-path routing and program wiring in sync. (#1207) — thanks @gumadeiras.
- Voice call: keep Twilio notify callbacks out of streaming paths, normalize Tailscale serve paths, and honor ASCII signature ordering. (#1213) — thanks @andrew-kurin.
## 2026.1.18-5 ## 2026.1.18-5

View File

@ -64,17 +64,24 @@ export class TwilioProvider implements VoiceCallProvider {
/** Storage for TwiML content (for notify mode with URL-based TwiML) */ /** Storage for TwiML content (for notify mode with URL-based TwiML) */
private readonly twimlStorage = new Map<string, string>(); private readonly twimlStorage = new Map<string, string>();
/** Track notify-mode calls to avoid streaming on follow-up callbacks. */
private readonly notifyCalls = new Set<string>();
/** /**
* Delete stored TwiML for a given `callId`. * Delete stored TwiML for a given `callId`.
* *
* We keep TwiML in-memory only long enough to satisfy the initial Twilio * We keep TwiML in-memory only long enough to satisfy the initial Twilio
* webhook request (notify mode). Subsequent webhooks should not reuse it. * webhook request (notify mode). The notify guard is cleared separately.
*/ */
private deleteStoredTwiml(callId: string): void { private deleteStoredTwiml(callId: string): void {
this.twimlStorage.delete(callId); this.twimlStorage.delete(callId);
} }
private clearNotifyState(callId: string): void {
this.twimlStorage.delete(callId);
this.notifyCalls.delete(callId);
}
/** /**
* Delete stored TwiML for a call, addressed by Twilio's provider call SID. * Delete stored TwiML for a call, addressed by Twilio's provider call SID.
* *
@ -87,7 +94,7 @@ export class TwilioProvider implements VoiceCallProvider {
const callIdMatch = webhookUrl.match(/callId=([^&]+)/); const callIdMatch = webhookUrl.match(/callId=([^&]+)/);
if (!callIdMatch) return; if (!callIdMatch) return;
this.deleteStoredTwiml(callIdMatch[1]); this.clearNotifyState(callIdMatch[1]);
} }
constructor(config: TwilioConfig, options: TwilioProviderOptions = {}) { constructor(config: TwilioConfig, options: TwilioProviderOptions = {}) {
@ -137,7 +144,7 @@ export class TwilioProvider implements VoiceCallProvider {
*/ */
private async apiRequest<T = unknown>( private async apiRequest<T = unknown>(
endpoint: string, endpoint: string,
params: Record<string, string>, params: Record<string, string | string[]>,
options?: { allowNotFound?: boolean }, options?: { allowNotFound?: boolean },
): Promise<T> { ): Promise<T> {
return await twilioApiRequest<T>({ return await twilioApiRequest<T>({
@ -257,12 +264,12 @@ export class TwilioProvider implements VoiceCallProvider {
case "no-answer": case "no-answer":
case "failed": case "failed":
if (callIdOverride) { if (callIdOverride) {
this.deleteStoredTwiml(callIdOverride); this.clearNotifyState(callIdOverride);
} }
return { ...baseEvent, type: "call.ended", reason: callStatus }; return { ...baseEvent, type: "call.ended", reason: callStatus };
case "canceled": case "canceled":
if (callIdOverride) { if (callIdOverride) {
this.deleteStoredTwiml(callIdOverride); this.clearNotifyState(callIdOverride);
} }
return { ...baseEvent, type: "call.ended", reason: "hangup-bot" }; return { ...baseEvent, type: "call.ended", reason: "hangup-bot" };
default: default:
@ -286,6 +293,9 @@ export class TwilioProvider implements VoiceCallProvider {
if (!ctx) return TwilioProvider.EMPTY_TWIML; if (!ctx) return TwilioProvider.EMPTY_TWIML;
const params = new URLSearchParams(ctx.rawBody); const params = new URLSearchParams(ctx.rawBody);
const type =
typeof ctx.query?.type === "string" ? ctx.query.type.trim() : undefined;
const isStatusCallback = type === "status";
const callStatus = params.get("CallStatus"); const callStatus = params.get("CallStatus");
const direction = params.get("Direction"); const direction = params.get("Direction");
const callIdFromQuery = const callIdFromQuery =
@ -297,13 +307,21 @@ export class TwilioProvider implements VoiceCallProvider {
// Handle initial TwiML request (when Twilio first initiates the call) // Handle initial TwiML request (when Twilio first initiates the call)
// Check if we have stored TwiML for this call (notify mode) // Check if we have stored TwiML for this call (notify mode)
if (callIdFromQuery) { if (callIdFromQuery && !isStatusCallback) {
const storedTwiml = this.twimlStorage.get(callIdFromQuery); const storedTwiml = this.twimlStorage.get(callIdFromQuery);
if (storedTwiml) { if (storedTwiml) {
// Clean up after serving (one-time use) // Clean up after serving (one-time use)
this.deleteStoredTwiml(callIdFromQuery); this.deleteStoredTwiml(callIdFromQuery);
return storedTwiml; return storedTwiml;
} }
if (this.notifyCalls.has(callIdFromQuery)) {
return TwilioProvider.EMPTY_TWIML;
}
}
// Status callbacks should not receive TwiML.
if (isStatusCallback) {
return TwilioProvider.EMPTY_TWIML;
} }
// Handle subsequent webhook requests (status callbacks, etc.) // Handle subsequent webhook requests (status callbacks, etc.)
@ -385,16 +403,18 @@ export class TwilioProvider implements VoiceCallProvider {
// We now serve it from the webhook endpoint instead of sending inline // We now serve it from the webhook endpoint instead of sending inline
if (input.inlineTwiml) { if (input.inlineTwiml) {
this.twimlStorage.set(input.callId, input.inlineTwiml); this.twimlStorage.set(input.callId, input.inlineTwiml);
// Keep notify-mode calls out of streaming TwiML for any follow-up callbacks.
this.notifyCalls.add(input.callId);
} }
// Build request params - always use URL-based TwiML. // Build request params - always use URL-based TwiML.
// Twilio silently ignores `StatusCallback` when using the inline `Twiml` parameter. // Twilio silently ignores `StatusCallback` when using the inline `Twiml` parameter.
const params: Record<string, string> = { const params: Record<string, string | string[]> = {
To: input.to, To: input.to,
From: input.from, From: input.from,
Url: url.toString(), // TwiML serving endpoint Url: url.toString(), // TwiML serving endpoint
StatusCallback: statusUrl.toString(), // Separate status callback endpoint StatusCallback: statusUrl.toString(), // Separate status callback endpoint
StatusCallbackEvent: "initiated ringing answered completed", StatusCallbackEvent: ["initiated", "ringing", "answered", "completed"],
Timeout: "30", Timeout: "30",
}; };

View File

@ -0,0 +1,39 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { twilioApiRequest } from "./api.js";
describe("twilioApiRequest", () => {
afterEach(() => {
vi.unstubAllGlobals();
vi.restoreAllMocks();
});
it("encodes array params as repeated form fields", async () => {
const fetchMock = vi.fn().mockResolvedValue({
ok: true,
text: async () => "",
});
vi.stubGlobal("fetch", fetchMock as unknown as typeof fetch);
await twilioApiRequest({
baseUrl: "https://api.example.com",
accountSid: "AC123",
authToken: "token",
endpoint: "/Calls.json",
body: {
To: "+15555550123",
StatusCallbackEvent: ["initiated", "completed"],
},
});
const [, init] = fetchMock.mock.calls[0] ?? [];
const body = init?.body as URLSearchParams | undefined;
expect(body?.getAll("StatusCallbackEvent")).toEqual([
"initiated",
"completed",
]);
expect(body?.get("To")).toBe("+15555550123");
});
});

View File

@ -3,16 +3,34 @@ export async function twilioApiRequest<T = unknown>(params: {
accountSid: string; accountSid: string;
authToken: string; authToken: string;
endpoint: string; endpoint: string;
body: Record<string, string>; body: URLSearchParams | Record<string, string | string[]>;
allowNotFound?: boolean; allowNotFound?: boolean;
}): Promise<T> { }): Promise<T> {
const bodyParams =
params.body instanceof URLSearchParams
? params.body
: Object.entries(params.body).reduce<URLSearchParams>(
(acc, [key, value]) => {
if (Array.isArray(value)) {
for (const entry of value) {
acc.append(key, entry);
}
} else if (typeof value === "string") {
acc.append(key, value);
}
return acc;
},
new URLSearchParams(),
);
const response = await fetch(`${params.baseUrl}${params.endpoint}`, { const response = await fetch(`${params.baseUrl}${params.endpoint}`, {
method: "POST", method: "POST",
headers: { headers: {
Authorization: `Basic ${Buffer.from(`${params.accountSid}:${params.authToken}`).toString("base64")}`, Authorization: `Basic ${Buffer.from(`${params.accountSid}:${params.authToken}`).toString("base64")}`,
"Content-Type": "application/x-www-form-urlencoded", "Content-Type": "application/x-www-form-urlencoded",
}, },
body: new URLSearchParams(params.body), // URL-encoded body required by Twilio, with repeated keys for arrays.
body: bodyParams,
}); });
if (!response.ok) { if (!response.ok) {
@ -26,4 +44,3 @@ export async function twilioApiRequest<T = unknown>(params: {
const text = await response.text(); const text = await response.text();
return text ? (JSON.parse(text) as T) : (undefined as T); return text ? (JSON.parse(text) as T) : (undefined as T);
} }

View File

@ -230,12 +230,14 @@ export async function startTailscaleTunnel(config: {
throw new Error("Could not get Tailscale DNS name. Is Tailscale running?"); throw new Error("Could not get Tailscale DNS name. Is Tailscale running?");
} }
const path = config.path.startsWith("/") ? config.path : `/${config.path}`;
// --set-path already mounts the path; keep the target base URL to avoid /path/path.
const localUrl = `http://127.0.0.1:${config.port}`; const localUrl = `http://127.0.0.1:${config.port}`;
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
const proc = spawn( const proc = spawn(
"tailscale", "tailscale",
[config.mode, "--bg", "--yes", "--set-path", config.path, localUrl], [config.mode, "--bg", "--yes", "--set-path", path, localUrl],
{ stdio: ["ignore", "pipe", "pipe"] }, { stdio: ["ignore", "pipe", "pipe"] },
); );
@ -247,7 +249,7 @@ export async function startTailscaleTunnel(config: {
proc.on("close", (code) => { proc.on("close", (code) => {
clearTimeout(timeout); clearTimeout(timeout);
if (code === 0) { if (code === 0) {
const publicUrl = `https://${dnsName}${config.path}`; const publicUrl = `https://${dnsName}${path}`;
console.log( console.log(
`[voice-call] Tailscale ${config.mode} active: ${publicUrl}`, `[voice-call] Tailscale ${config.mode} active: ${publicUrl}`,
); );
@ -256,7 +258,7 @@ export async function startTailscaleTunnel(config: {
publicUrl, publicUrl,
provider: `tailscale-${config.mode}`, provider: `tailscale-${config.mode}`,
stop: async () => { stop: async () => {
await stopTailscaleTunnel(config.mode, config.path); await stopTailscaleTunnel(config.mode, path);
}, },
}); });
} else { } else {

View File

@ -79,7 +79,7 @@ function twilioSignature(params: {
let dataToSign = params.url; let dataToSign = params.url;
const sortedParams = Array.from( const sortedParams = Array.from(
new URLSearchParams(params.postBody).entries(), new URLSearchParams(params.postBody).entries(),
).sort((a, b) => a[0].localeCompare(b[0])); ).sort((a, b) => (a[0] < b[0] ? -1 : a[0] > b[0] ? 1 : 0));
for (const [key, value] of sortedParams) { for (const [key, value] of sortedParams) {
dataToSign += key + value; dataToSign += key + value;
@ -205,4 +205,33 @@ describe("verifyTwilioWebhook", () => {
expect(result.ok).toBe(true); expect(result.ok).toBe(true);
}); });
it("sorts params using ASCII order", () => {
const authToken = "test-auth-token";
const publicUrl = "https://example.com/voice/webhook";
const postBody = "Z=first&a=second";
const signature = twilioSignature({
authToken,
url: publicUrl,
postBody,
});
const result = verifyTwilioWebhook(
{
headers: {
host: "example.com",
"x-forwarded-proto": "https",
"x-twilio-signature": signature,
},
rawBody: postBody,
url: "http://local/voice/webhook",
method: "POST",
},
authToken,
{ publicUrl },
);
expect(result.ok).toBe(true);
});
}); });

View File

@ -23,9 +23,9 @@ export function validateTwilioSignature(
// Build the string to sign: URL + sorted params (key+value pairs) // Build the string to sign: URL + sorted params (key+value pairs)
let dataToSign = url; let dataToSign = url;
// Sort params alphabetically and append key+value // Sort params by ASCII order (Twilio spec) and append key+value.
const sortedParams = Array.from(params.entries()).sort((a, b) => const sortedParams = Array.from(params.entries()).sort((a, b) =>
a[0].localeCompare(b[0]), a[0] < b[0] ? -1 : a[0] > b[0] ? 1 : 0,
); );
for (const [key, value] of sortedParams) { for (const [key, value] of sortedParams) {

View File

@ -553,7 +553,7 @@ export function buildAgentSystemPrompt(params: {
} }
export function buildRuntimeLine( export function buildRuntimeLine(
runtimeInfo: { runtimeInfo?: {
agentId?: string; agentId?: string;
host?: string; host?: string;
os?: string; os?: string;