From b7ad444ea7bf018497c3d5dcbb2bf4d21e69940c Mon Sep 17 00:00:00 2001 From: Justin Stayton Date: Thu, 27 Aug 2026 06:58:25 -0400 Subject: [PATCH] Test the remaining branches & guard the secret Branch coverage sat at 87.88%. The empty comma-separated header parts, the `indexOf('=') === -1` fallbacks in both halves of the header, and the default leeway of 5 minutes all went unexercised. Mutation testing turned up more: changing the default leeway to 60, or dropping it outright, left the whole suite green. A missing `secret` also threw a raw `TypeError` out of `createHmac` rather than the `TruepicWebhookVerifierError` that the JSDoc and the type declarations promise, and a non-string `header` died on `header.split`. Both now throw the documented error, which means an empty secret reports "Secret is missing or empty" where it used to report "Signature is not valid". No existing message changed. Coverage is now 100% line and branch across both files. Co-Authored-By: Claude Opus 5 --- src/error.test.js | 40 +++++++++ src/main.js | 9 +- src/main.test.js | 205 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 src/error.test.js diff --git a/src/error.test.js b/src/error.test.js new file mode 100644 index 0000000..b69c36a --- /dev/null +++ b/src/error.test.js @@ -0,0 +1,40 @@ +const assert = require('node:assert/strict') +const { describe, it } = require('node:test') +const TruepicWebhookVerifierError = require('./error') +const verifyTruepicWebhook = require('./main') + +describe('TruepicWebhookVerifierError', () => { + const message = 'Signature is not valid' + + it('sets the `message`', () => { + assert.strictEqual( + new TruepicWebhookVerifierError(message).message, + message, + ) + }) + + it('sets the `name` to the class name', () => { + assert.strictEqual( + new TruepicWebhookVerifierError(message).name, + 'TruepicWebhookVerifierError', + ) + }) + + it('is an instance of `Error`', () => { + assert.ok(new TruepicWebhookVerifierError(message) instanceof Error) + }) + + it('captures a stack trace pointing at the caller', () => { + const error = new TruepicWebhookVerifierError(message) + + assert.ok(error.stack.startsWith(`TruepicWebhookVerifierError: ${message}`)) + assert.match(error.stack, /error\.test\.js/) + }) + + it('is exported from the main module', () => { + assert.strictEqual( + verifyTruepicWebhook.TruepicWebhookVerifierError, + TruepicWebhookVerifierError, + ) + }) +}) diff --git a/src/main.js b/src/main.js index 144c0db..67510a1 100644 --- a/src/main.js +++ b/src/main.js @@ -17,7 +17,7 @@ const TruepicWebhookVerifierError = require('./error') * @returns {Object} The parsed `timestamp` and `signature` values. */ function parseHeader(header) { - if (!header?.length) { + if (typeof header !== 'string' || !header.length) { throw new TruepicWebhookVerifierError('Header is missing or empty') } @@ -108,6 +108,13 @@ function verifyTimestamp({ timestamp, leewayMinutes }) { * @returns {true} If verification succeeds. */ function verifySignature({ url, secret, body, timestamp, signature }) { + // Guard the secret before handing it to `createHmac`, which would otherwise + // throw a raw `TypeError` and break the documented contract that every + // failure is a `TruepicWebhookVerifierError`. + if (typeof secret !== 'string' || !secret.length) { + throw new TruepicWebhookVerifierError('Secret is missing or empty') + } + // Rebuild the signature (SHA-256 HMAC digest) with a secret that only Truepic // and the intended receiver are privy to. const comparisonSignature = createHmac('sha256', secret) diff --git a/src/main.test.js b/src/main.test.js index 3095c15..4fc6984 100644 --- a/src/main.test.js +++ b/src/main.test.js @@ -67,9 +67,60 @@ describe('verifyTruepicWebhook', () => { true, ) }) + + it('defaults to a 5 minute leeway if none is given', (t) => { + t.mock.timers.enable({ + apis: ['Date'], + now: sentAtMs + 1000 * 60 * 5, + }) + + assert.strictEqual( + verifyTruepicWebhook({ + url, + secret, + header, + body, + }), + true, + ) + }) + + it('accepts a webhook that arrives instantly with no leeway', (t) => { + t.mock.timers.enable({ apis: ['Date'], now: sentAtMs }) + + assert.strictEqual( + verifyTruepicWebhook({ + url, + secret, + header, + body, + leewayMinutes: 0, + }), + true, + ) + }) }) describe('throws a `TruepicWebhookVerifierError`', () => { + it('that is an instance of the exported error class', () => { + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret, + header: '', + body, + leewayMinutes, + }), + (error) => { + assert.ok(error instanceof TruepicWebhookVerifierError) + assert.ok(error instanceof Error) + + return true + }, + ) + }) + it('if the `header` is missing', () => { assert.throws( () => @@ -98,6 +149,20 @@ describe('verifyTruepicWebhook', () => { ) }) + it('if the `header` is not a string', () => { + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret, + header: ['t=1698259719,s=abc'], + body, + leewayMinutes, + }), + new TruepicWebhookVerifierError('Header is missing or empty'), + ) + }) + it('if the `header` cannot be parsed into timestamp and signature', () => { assert.throws( () => @@ -130,6 +195,38 @@ describe('verifyTruepicWebhook', () => { ) }) + it('if the `header` is missing the timestamp part entirely', () => { + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret, + header: ',s=test', + body, + leewayMinutes, + }), + new TruepicWebhookVerifierError( + 'Header cannot be parsed into timestamp and signature', + ), + ) + }) + + it('if the `header` is missing the signature part entirely', () => { + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret, + header: 't=1698259719,', + body, + leewayMinutes, + }), + new TruepicWebhookVerifierError( + 'Header cannot be parsed into timestamp and signature', + ), + ) + }) + it('if the `header` is missing the timestamp (`t`)', () => { assert.throws( () => @@ -158,6 +255,20 @@ describe('verifyTruepicWebhook', () => { ) }) + it('if the `header` timestamp part has no `=`', () => { + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret, + header: 't,s=test', + body, + leewayMinutes, + }), + new TruepicWebhookVerifierError('Timestamp is missing or empty'), + ) + }) + it('if the `header` timestamp (`t`) is not a number', () => { assert.throws( () => @@ -200,6 +311,20 @@ describe('verifyTruepicWebhook', () => { ) }) + it('if the `header` signature part has no `=`', () => { + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret, + header: 't=1698259719,s', + body, + leewayMinutes, + }), + new TruepicWebhookVerifierError('Signature is missing or empty'), + ) + }) + it('if the webhook arrives more than 5 minutes late', (t) => { t.mock.timers.enable({ apis: ['Date'], @@ -242,6 +367,44 @@ describe('verifyTruepicWebhook', () => { ) }) + it('if the webhook arrives more than 5 minutes late and no leeway is given', (t) => { + t.mock.timers.enable({ + apis: ['Date'], + now: sentAtMs + 1000 * 60 * 5 + 1, + }) + + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret, + header, + body, + }), + new TruepicWebhookVerifierError( + 'Timestamp is not within allowed window', + ), + ) + }) + + it('if the webhook is late at all and no leeway is allowed', (t) => { + t.mock.timers.enable({ apis: ['Date'], now: sentAtMs + 1 }) + + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret, + header, + body, + leewayMinutes: 0, + }), + new TruepicWebhookVerifierError( + 'Timestamp is not within allowed window', + ), + ) + }) + it('if the `url` is not where the request was sent', () => { assert.throws( () => @@ -298,6 +461,48 @@ describe('verifyTruepicWebhook', () => { ) }) + it('if the `secret` is missing', () => { + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret: undefined, + header, + body, + leewayMinutes, + }), + new TruepicWebhookVerifierError('Secret is missing or empty'), + ) + }) + + it('if the `secret` is empty', () => { + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret: '', + header, + body, + leewayMinutes, + }), + new TruepicWebhookVerifierError('Secret is missing or empty'), + ) + }) + + it('if the `secret` is not a string', () => { + assert.throws( + () => + verifyTruepicWebhook({ + url, + secret: 1698259719, + header, + body, + leewayMinutes, + }), + new TruepicWebhookVerifierError('Secret is missing or empty'), + ) + }) + it('if the `secret` is not what was used to sign', () => { assert.throws( () =>