From 71db28c2849dd7637cffd52b6a7e949dcaeedeed Mon Sep 17 00:00:00 2001 From: "Knut Martin Torn knutmt@gmail.com" Date: Sun, 20 Sep 2026 23:39:16 +0200 Subject: [PATCH] fix(svix): accept space-separated multi-signature headers Svix sends every signature it holds during a signing-secret rotation, separated by spaces: `v1, v1,`. Only one matches the secret the receiver has, and the message must be accepted on that one. The parser split the combined string on commas alone, so `v1,AAA v1,BBB,t=..,id=..` became ['v1', 'AAA v1', 'BBB', ...]: the first signature was captured as the literal `AAA v1` and the second was dropped, because nothing preceding it equalled 'v1'. Result: every signature fails for the whole duration of a rotation, on svix and on clerk, which delegates to it. The provider's own doc comment already documented the correct format ("multiple sigs v1, v1,"), so the intent was right and only the parse was wrong. Split on commas AND whitespace; base64 contains no whitespace, so this cannot split a signature. Every existing svix and clerk test used a single signature, which is why this branch was never exercised. Added six: the matching signature first, last, and among three; none matching (so a parser that blindly accepted multi-signature headers would not pass); tolerance still applying; and one through the headers object, which is the path Clerk actually takes. Verified by mutation: restoring the comma-only split fails exactly the four acceptance tests and leaves the two rejection tests passing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mkd9oGgdMEFUftzCeiCfZw --- src/providers/svix.ts | 9 +++++- test/verify.test.ts | 75 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/src/providers/svix.ts b/src/providers/svix.ts index 15c9fb7..445407b 100644 --- a/src/providers/svix.ts +++ b/src/providers/svix.ts @@ -29,7 +29,14 @@ export const svix: ProviderVerifier = { let timestamp: string | undefined; let msgId: string | undefined; - const parts = signature.split(','); + // Split on commas AND whitespace: svix separates the scheme from the + // signature with a comma but separates SIGNATURES from each other with a + // space (`v1, v1,`), which it does during a signing-secret + // rotation. Splitting on commas alone yields ['v1', ' v1', ''] + // — the first signature is captured with a trailing ' v1' and the second + // is dropped, so every signature fails for the duration of the rotation. + // Base64 contains no whitespace, so this cannot split a signature. + const parts = signature.split(/[,\s]+/); for (let i = 0; i < parts.length; i++) { const part = parts[i].trim(); if (part === 'v1' && i + 1 < parts.length) { diff --git a/test/verify.test.ts b/test/verify.test.ts index 61d1744..6b36f11 100644 --- a/test/verify.test.ts +++ b/test/verify.test.ts @@ -456,6 +456,81 @@ describe('webhook-verify', () => { false ); }); + + // Svix sends every signature it holds during a signing-secret rotation, + // space-separated: `v1, v1,`. Only one matches the secret the + // receiver has, and the message must be accepted on that one. Every test + // above uses a single signature, so this branch — which the provider's + // own doc comment describes — was never exercised. + function multiSig(sigs: string[], ts: number, id: string): string { + return `${sigs.map((s) => `v1,${s}`).join(' ')},t=${ts},id=${id}`; + } + + function rawSig(body: string, key: string, id: string, ts: number): string { + const secretKey = key.startsWith('whsec_') + ? Buffer.from(key.slice(6), 'base64') + : Buffer.from(key, 'base64'); + return createHmac('sha256', secretKey).update(`${id}.${ts}.${body}`).digest('base64'); + } + + describe('multiple signatures (secret rotation)', () => { + const ts = Math.floor(Date.now() / 1000); + const otherSecret = `whsec_${Buffer.from('a-different-secret-key').toString('base64')}`; + const thirdSecret = `whsec_${Buffer.from('third-secret-here-ok').toString('base64')}`; + + it('should accept when the matching signature is FIRST', () => { + const good = rawSig(payload, secret, msgId, ts); + const stale = rawSig(payload, otherSecret, msgId, ts); + assert.strictEqual(verify('svix', payload, multiSig([good, stale], ts, msgId), secret), true); + }); + + it('should accept when the matching signature is LAST', () => { + // Position matters: splitting on commas alone corrupts the first + // signature and drops every later one, so a naive parser fails this + // case and the one above for different reasons. + const good = rawSig(payload, secret, msgId, ts); + const stale = rawSig(payload, otherSecret, msgId, ts); + assert.strictEqual(verify('svix', payload, multiSig([stale, good], ts, msgId), secret), true); + }); + + it('should accept the matching signature among three', () => { + const good = rawSig(payload, secret, msgId, ts); + const a = rawSig(payload, otherSecret, msgId, ts); + const b = rawSig(payload, thirdSecret, msgId, ts); + assert.strictEqual(verify('svix', payload, multiSig([a, good, b], ts, msgId), secret), true); + }); + + it('should still reject when NONE of the signatures match', () => { + // Without this, a parser that accepted any multi-signature header + // outright would pass the three tests above. + const a = rawSig(payload, otherSecret, msgId, ts); + const b = rawSig(payload, thirdSecret, msgId, ts); + assert.strictEqual(verify('svix', payload, multiSig([a, b], ts, msgId), secret), false); + }); + + it('should apply the timestamp tolerance to multi-signature headers too', () => { + const old = ts - 3600; + const good = rawSig(payload, secret, msgId, old); + const stale = rawSig(payload, otherSecret, msgId, old); + assert.strictEqual(verify('svix', payload, multiSig([good, stale], old, msgId), secret), false); + }); + + it('should verify through the headers object, as Clerk sends it', () => { + // getSignature() joins the raw svix-signature header with t= and id=, + // so in production a multi-signature header reaches the parser via + // this path. Passing a pre-joined string bypasses it. + const good = rawSig(payload, secret, msgId, ts); + const stale = rawSig(payload, otherSecret, msgId, ts); + assert.strictEqual( + verify('svix', payload, { + 'svix-id': msgId, + 'svix-timestamp': String(ts), + 'svix-signature': `v1,${stale} v1,${good}`, + }, secret), + true + ); + }); + }); }); describe('Clerk', () => {