From 3ba7a2b986e93a96338a4a887b4881ad1420d470 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Tue, 24 Mar 2026 01:24:17 -0700 Subject: [PATCH 1/5] feat(HTTPReceiver): add invalidRequestSignatureHandler callback Adds an optional invalidRequestSignatureHandler to HTTPReceiver, matching the callback added to AwsLambdaReceiver in PR #2154. When signature verification fails, the handler fires with the raw body, signature header, and timestamp. Defaults to a noop. Refs #2156 --- src/receivers/HTTPReceiver.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/receivers/HTTPReceiver.ts b/src/receivers/HTTPReceiver.ts index bd08b19c2..b7207e63d 100644 --- a/src/receivers/HTTPReceiver.ts +++ b/src/receivers/HTTPReceiver.ts @@ -34,6 +34,12 @@ import type { ParamsIncomingMessage } from './ParamsIncomingMessage'; import { type CustomRoute, type ReceiverRoutes, buildReceiverRoutes } from './custom-routes'; import { verifyRedirectOpts } from './verify-redirect-opts'; +export interface HTTPReceiverInvalidRequestSignatureHandlerArgs { + rawBody: string; + signature: string | undefined; + ts: number | undefined; +} + // Option keys for tls.createServer() and tls.createSecureContext(), exclusive of those for http.createServer() const httpsOptionKeys = [ 'ALPNProtocols', @@ -81,6 +87,7 @@ export interface HTTPReceiverOptions { logLevel?: LogLevel; processBeforeResponse?: boolean; signatureVerification?: boolean; + invalidRequestSignatureHandler?: (args: HTTPReceiverInvalidRequestSignatureHandlerArgs) => void; clientId?: string; clientSecret?: string; stateSecret?: InstallProviderOptions['stateSecret']; // required when using default stateStore @@ -137,6 +144,8 @@ export default class HTTPReceiver implements Receiver { private signatureVerification: boolean; + private invalidRequestSignatureHandler: (args: HTTPReceiverInvalidRequestSignatureHandlerArgs) => void; + private app?: App; public requestListener: RequestListener; @@ -178,6 +187,7 @@ export default class HTTPReceiver implements Receiver { logLevel = LogLevel.INFO, processBeforeResponse = false, signatureVerification = true, + invalidRequestSignatureHandler, clientId = undefined, clientSecret = undefined, stateSecret = undefined, @@ -195,6 +205,8 @@ export default class HTTPReceiver implements Receiver { this.signingSecret = signingSecret; this.processBeforeResponse = processBeforeResponse; this.signatureVerification = signatureVerification; + this.invalidRequestSignatureHandler = + invalidRequestSignatureHandler ?? this.defaultInvalidRequestSignatureHandler.bind(this); this.logger = logger ?? (() => { @@ -448,6 +460,13 @@ export default class HTTPReceiver implements Receiver { const e = err as Error; if (this.signatureVerification) { this.logger.warn(`Failed to parse and verify the request data: ${e.message}`); + const requestWithRawBody = req as IncomingMessage & { rawBody?: string }; + const rawBody = typeof requestWithRawBody.rawBody === 'string' ? requestWithRawBody.rawBody : ''; + this.invalidRequestSignatureHandler({ + rawBody, + signature: req.headers['x-slack-signature'] as string | undefined, + ts: req.headers['x-slack-request-timestamp'] ? Number(req.headers['x-slack-request-timestamp']) : undefined, + }); } else { this.logger.warn(`Failed to parse the request body: ${e.message}`); } @@ -565,4 +584,8 @@ export default class HTTPReceiver implements Receiver { installer.handleCallback(req, res, installCallbackOptions).catch(errorHandler); } } + + private defaultInvalidRequestSignatureHandler(_args: HTTPReceiverInvalidRequestSignatureHandlerArgs): void { + // noop - signature verification failure is already logged and a 401 is returned + } } From 4aa2f019c6116fc685b9992d988203eaa90dc47c Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Mon, 30 Mar 2026 08:59:50 -0700 Subject: [PATCH 2/5] test(HTTPReceiver): add tests for invalidRequestSignatureHandler Cover three scenarios: custom handler called with correct args on signature failure, default noop handler doesn't throw, and missing headers pass undefined for signature/ts. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/unit/receivers/HTTPReceiver.spec.ts | 119 +++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/test/unit/receivers/HTTPReceiver.spec.ts b/test/unit/receivers/HTTPReceiver.spec.ts index 3d9ab1b43..d97715e10 100644 --- a/test/unit/receivers/HTTPReceiver.spec.ts +++ b/test/unit/receivers/HTTPReceiver.spec.ts @@ -571,6 +571,125 @@ describe('HTTPReceiver', () => { }); }); + describe('invalidRequestSignatureHandler', () => { + it('should call the custom handler when signature verification fails', async () => { + const spy = sinon.spy(); + const fakeParseAndVerify = sinon.fake.rejects(new Error('Signature mismatch')); + const fakeBuildNoBodyResponse = sinon.fake(); + + const overridesWithFakeVerify = mergeOverrides(overrides, { + './HTTPModuleFunctions': { + parseAndVerifyHTTPRequest: fakeParseAndVerify, + parseHTTPRequestBody: sinon.fake(), + buildNoBodyResponse: fakeBuildNoBodyResponse, + '@noCallThru': true, + }, + }); + + const HTTPReceiver = importHTTPReceiver(overridesWithFakeVerify); + const receiver = new HTTPReceiver({ + signingSecret: 'secret', + logger: noopLogger, + invalidRequestSignatureHandler: spy, + }); + assert.isNotNull(receiver); + + const fakeReq = sinon.createStubInstance(IncomingMessage) as unknown as IncomingMessage; + fakeReq.url = '/slack/events'; + fakeReq.method = 'POST'; + fakeReq.headers = { + 'x-slack-signature': 'v0=bad', + 'x-slack-request-timestamp': '1234567890', + }; + (fakeReq as IncomingMessage & { rawBody?: string }).rawBody = '{"token":"test"}'; + + const fakeRes = sinon.createStubInstance(ServerResponse) as unknown as ServerResponse; + + receiver.requestListener(fakeReq, fakeRes); + + // Wait for the async closure inside handleIncomingEvent to settle + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert(spy.calledOnce, 'invalidRequestSignatureHandler should be called once'); + const args = spy.firstCall.args[0]; + assert.equal(args.rawBody, '{"token":"test"}'); + assert.equal(args.signature, 'v0=bad'); + assert.equal(args.ts, 1234567890); + }); + + it('should use the default noop handler when no custom handler is provided', async () => { + const fakeParseAndVerify = sinon.fake.rejects(new Error('Signature mismatch')); + const fakeBuildNoBodyResponse = sinon.fake(); + + const overridesWithFakeVerify = mergeOverrides(overrides, { + './HTTPModuleFunctions': { + parseAndVerifyHTTPRequest: fakeParseAndVerify, + parseHTTPRequestBody: sinon.fake(), + buildNoBodyResponse: fakeBuildNoBodyResponse, + '@noCallThru': true, + }, + }); + + const HTTPReceiver = importHTTPReceiver(overridesWithFakeVerify); + const receiver = new HTTPReceiver({ + signingSecret: 'secret', + logger: noopLogger, + }); + + const fakeReq = sinon.createStubInstance(IncomingMessage) as unknown as IncomingMessage; + fakeReq.url = '/slack/events'; + fakeReq.method = 'POST'; + fakeReq.headers = {}; + + const fakeRes = sinon.createStubInstance(ServerResponse) as unknown as ServerResponse; + + // Should not throw even without a custom handler + receiver.requestListener(fakeReq, fakeRes); + await new Promise((resolve) => setTimeout(resolve, 50)); + + sinon.assert.calledOnce(fakeBuildNoBodyResponse); + sinon.assert.calledWith(fakeBuildNoBodyResponse, fakeRes, 401); + }); + + it('should pass undefined for signature and ts when headers are missing', async () => { + const spy = sinon.spy(); + const fakeParseAndVerify = sinon.fake.rejects(new Error('Signature mismatch')); + const fakeBuildNoBodyResponse = sinon.fake(); + + const overridesWithFakeVerify = mergeOverrides(overrides, { + './HTTPModuleFunctions': { + parseAndVerifyHTTPRequest: fakeParseAndVerify, + parseHTTPRequestBody: sinon.fake(), + buildNoBodyResponse: fakeBuildNoBodyResponse, + '@noCallThru': true, + }, + }); + + const HTTPReceiver = importHTTPReceiver(overridesWithFakeVerify); + const receiver = new HTTPReceiver({ + signingSecret: 'secret', + logger: noopLogger, + invalidRequestSignatureHandler: spy, + }); + + const fakeReq = sinon.createStubInstance(IncomingMessage) as unknown as IncomingMessage; + fakeReq.url = '/slack/events'; + fakeReq.method = 'POST'; + fakeReq.headers = {}; + + const fakeRes = sinon.createStubInstance(ServerResponse) as unknown as ServerResponse; + + receiver.requestListener(fakeReq, fakeRes); + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert(spy.calledOnce); + const args = spy.firstCall.args[0]; + assert.equal(args.rawBody, ''); + assert.isUndefined(args.signature); + assert.isUndefined(args.ts); + }); + }); + it("should throw if request doesn't match install path, redirect URI path, or custom routes", async () => { const installProviderStub = sinon.createStubInstance(InstallProvider); const HTTPReceiver = importHTTPReceiver(overrides); From 6ceb6961982c7643761da6c1214917228ac700cd Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Sun, 12 Apr 2026 15:36:34 -0700 Subject: [PATCH 3/5] chore: changeset --- .changeset/public-gifts-lose.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .changeset/public-gifts-lose.md diff --git a/.changeset/public-gifts-lose.md b/.changeset/public-gifts-lose.md new file mode 100644 index 000000000..0fae756c5 --- /dev/null +++ b/.changeset/public-gifts-lose.md @@ -0,0 +1,21 @@ +--- +"@slack/bolt": minor +--- + +feat(HTTPReceiver): add invalidRequestSignatureHandler callback + +Details of a failed request can be parsed and logged with the customized `invalidRequestSignatureHandler` callback for the `HTTPReceiver` receiver: + +```javascript +import { App, HTTPReceiver } from "@slack/bolt"; + +const app = new App({ + token: process.env.SLACK_BOT_TOKEN, + receiver: new HTTPReceiver({ + signingSecret: "unexpectedvalue", + invalidRequestSignatureHandler: (args) => { + app.logger.warn(args); + }, + }), +}); +``` From 3c1f24c8e8d7e0295c4640c1964b02ae77255c83 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Tue, 14 Apr 2026 22:01:37 -0400 Subject: [PATCH 4/5] refactor(HTTPReceiver): address review feedback on invalid-signature handler Per @zimeg: - Tighten HTTPReceiverInvalidRequestSignatureHandlerArgs: signature is now string (default '') and ts is number (default 0) when headers are missing, matching AwsLambdaReceiver's ReceiverInvalidRequestSignatureHandlerArgs shape and removing the 'string | undefined' / 'number | undefined' awkwardness in override handlers. - Add logger to the args so override handlers can emit their own structured output using the receiver's configured logger. - Move the default warn log into defaultInvalidRequestSignatureHandler so overrides can fully suppress or replace the warning instead of having it always emitted before the handler runs. Format mirrors AwsLambdaReceiver's default: 'Invalid request signature detected (X-Slack-Signature: ..., X-Slack-Request-Timestamp: ...)'. - Add jsdoc on the invalidRequestSignatureHandler option describing the default behavior, the 401 response, and the override contract. - Update the two HTTPReceiver.spec.ts test cases that previously asserted 'undefined' signature/ts to check the new defaults ('', 0) and to verify a logger is passed to the handler. --- src/receivers/HTTPReceiver.ts | 28 ++++++++++++++++++------ test/unit/receivers/HTTPReceiver.spec.ts | 8 ++++--- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/receivers/HTTPReceiver.ts b/src/receivers/HTTPReceiver.ts index b7207e63d..6c394971e 100644 --- a/src/receivers/HTTPReceiver.ts +++ b/src/receivers/HTTPReceiver.ts @@ -36,8 +36,9 @@ import { verifyRedirectOpts } from './verify-redirect-opts'; export interface HTTPReceiverInvalidRequestSignatureHandlerArgs { rawBody: string; - signature: string | undefined; - ts: number | undefined; + signature: string; + ts: number; + logger: Logger; } // Option keys for tls.createServer() and tls.createSecureContext(), exclusive of those for http.createServer() @@ -87,6 +88,15 @@ export interface HTTPReceiverOptions { logLevel?: LogLevel; processBeforeResponse?: boolean; signatureVerification?: boolean; + /** + * Called when an incoming request fails signature verification. Override to + * emit custom telemetry, return a specific response body, or suppress the + * default warn log. The receiver still returns `401 Unauthorized` to the + * client regardless of what the handler does. + * + * Defaults to a handler that logs a warning with the received + * `x-slack-signature` and `x-slack-request-timestamp` values. + */ invalidRequestSignatureHandler?: (args: HTTPReceiverInvalidRequestSignatureHandlerArgs) => void; clientId?: string; clientSecret?: string; @@ -459,13 +469,13 @@ export default class HTTPReceiver implements Receiver { } catch (err) { const e = err as Error; if (this.signatureVerification) { - this.logger.warn(`Failed to parse and verify the request data: ${e.message}`); const requestWithRawBody = req as IncomingMessage & { rawBody?: string }; const rawBody = typeof requestWithRawBody.rawBody === 'string' ? requestWithRawBody.rawBody : ''; this.invalidRequestSignatureHandler({ rawBody, - signature: req.headers['x-slack-signature'] as string | undefined, - ts: req.headers['x-slack-request-timestamp'] ? Number(req.headers['x-slack-request-timestamp']) : undefined, + signature: (req.headers['x-slack-signature'] as string) ?? '', + ts: Number(req.headers['x-slack-request-timestamp']) || 0, + logger: this.logger, }); } else { this.logger.warn(`Failed to parse the request body: ${e.message}`); @@ -585,7 +595,11 @@ export default class HTTPReceiver implements Receiver { } } - private defaultInvalidRequestSignatureHandler(_args: HTTPReceiverInvalidRequestSignatureHandlerArgs): void { - // noop - signature verification failure is already logged and a 401 is returned + private defaultInvalidRequestSignatureHandler(args: HTTPReceiverInvalidRequestSignatureHandlerArgs): void { + const { signature, ts, logger } = args; + + logger.warn( + `Invalid request signature detected (X-Slack-Signature: ${signature}, X-Slack-Request-Timestamp: ${ts})`, + ); } } diff --git a/test/unit/receivers/HTTPReceiver.spec.ts b/test/unit/receivers/HTTPReceiver.spec.ts index d97715e10..7f96afbbc 100644 --- a/test/unit/receivers/HTTPReceiver.spec.ts +++ b/test/unit/receivers/HTTPReceiver.spec.ts @@ -615,6 +615,7 @@ describe('HTTPReceiver', () => { assert.equal(args.rawBody, '{"token":"test"}'); assert.equal(args.signature, 'v0=bad'); assert.equal(args.ts, 1234567890); + assert.isDefined(args.logger, 'logger should be passed to the handler'); }); it('should use the default noop handler when no custom handler is provided', async () => { @@ -651,7 +652,7 @@ describe('HTTPReceiver', () => { sinon.assert.calledWith(fakeBuildNoBodyResponse, fakeRes, 401); }); - it('should pass undefined for signature and ts when headers are missing', async () => { + it('should pass empty signature and zero ts when headers are missing', async () => { const spy = sinon.spy(); const fakeParseAndVerify = sinon.fake.rejects(new Error('Signature mismatch')); const fakeBuildNoBodyResponse = sinon.fake(); @@ -685,8 +686,9 @@ describe('HTTPReceiver', () => { assert(spy.calledOnce); const args = spy.firstCall.args[0]; assert.equal(args.rawBody, ''); - assert.isUndefined(args.signature); - assert.isUndefined(args.ts); + assert.equal(args.signature, ''); + assert.equal(args.ts, 0); + assert.isDefined(args.logger); }); }); From 468f0bd1a0930c0f4fcc4a0cd2a4d1d500e058ed Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:21:36 -0700 Subject: [PATCH 5/5] refactor(HTTPReceiver): drop logger from invalidRequestSignatureHandler args Mirror the AwsLambdaReceiver pattern: the default handler reaches for `this.logger` directly instead of plumbing a `logger` argument through the interface. Removes `logger: Logger` from `HTTPReceiverInvalidRequestSignatureHandlerArgs`, the call site in `requestListener`, and the destructure in `defaultInvalidRequestSignatureHandler`. Per @zimeg on PR #2827, keeping the interface symmetrical with `ReceiverInvalidRequestSignatureHandlerArgs` on `AwsLambdaReceiver` leaves room for a shared interface in a follow-up PR. Tests updated to assert `logger` is no longer present on args; full HTTPReceiver suite (20 tests) passes with build, lint, and type tests green. --- src/receivers/HTTPReceiver.ts | 6 ++---- test/unit/receivers/HTTPReceiver.spec.ts | 7 +++++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/receivers/HTTPReceiver.ts b/src/receivers/HTTPReceiver.ts index 6c394971e..f7970cee7 100644 --- a/src/receivers/HTTPReceiver.ts +++ b/src/receivers/HTTPReceiver.ts @@ -38,7 +38,6 @@ export interface HTTPReceiverInvalidRequestSignatureHandlerArgs { rawBody: string; signature: string; ts: number; - logger: Logger; } // Option keys for tls.createServer() and tls.createSecureContext(), exclusive of those for http.createServer() @@ -475,7 +474,6 @@ export default class HTTPReceiver implements Receiver { rawBody, signature: (req.headers['x-slack-signature'] as string) ?? '', ts: Number(req.headers['x-slack-request-timestamp']) || 0, - logger: this.logger, }); } else { this.logger.warn(`Failed to parse the request body: ${e.message}`); @@ -596,9 +594,9 @@ export default class HTTPReceiver implements Receiver { } private defaultInvalidRequestSignatureHandler(args: HTTPReceiverInvalidRequestSignatureHandlerArgs): void { - const { signature, ts, logger } = args; + const { signature, ts } = args; - logger.warn( + this.logger.warn( `Invalid request signature detected (X-Slack-Signature: ${signature}, X-Slack-Request-Timestamp: ${ts})`, ); } diff --git a/test/unit/receivers/HTTPReceiver.spec.ts b/test/unit/receivers/HTTPReceiver.spec.ts index 7f96afbbc..42b024965 100644 --- a/test/unit/receivers/HTTPReceiver.spec.ts +++ b/test/unit/receivers/HTTPReceiver.spec.ts @@ -615,7 +615,10 @@ describe('HTTPReceiver', () => { assert.equal(args.rawBody, '{"token":"test"}'); assert.equal(args.signature, 'v0=bad'); assert.equal(args.ts, 1234567890); - assert.isDefined(args.logger, 'logger should be passed to the handler'); + assert.isUndefined( + (args as { logger?: unknown }).logger, + 'logger should not be passed to the handler (parity with AwsLambdaReceiver)', + ); }); it('should use the default noop handler when no custom handler is provided', async () => { @@ -688,7 +691,7 @@ describe('HTTPReceiver', () => { assert.equal(args.rawBody, ''); assert.equal(args.signature, ''); assert.equal(args.ts, 0); - assert.isDefined(args.logger); + assert.isUndefined((args as { logger?: unknown }).logger); }); });