Skip to content
21 changes: 21 additions & 0 deletions .changeset/public-gifts-lose.md
Original file line number Diff line number Diff line change
@@ -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);
},
}),
});
```
37 changes: 36 additions & 1 deletion src/receivers/HTTPReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
ts: number;
}
Comment on lines +37 to +41
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐣 note: Here are findings of the adjacent implementation:

export interface ReceiverInvalidRequestSignatureHandlerArgs {
rawBody: string;
signature: string;
ts: number;
awsEvent: AwsEvent;
awsResponse: Promise<AwsResponse>;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface HTTPReceiverInvalidRequestSignatureHandlerArgs {
rawBody: string;
signature: string | undefined;
ts: number | undefined;
}
export interface HTTPReceiverInvalidRequestSignatureHandlerArgs {
rawBody: string;
signature: string;
ts: number;
}

👁️‍🗨️ thought: I'd be curious to use default values here and perhaps matching the adjacent interface name, but I'm less confident about the second point...


// Option keys for tls.createServer() and tls.createSecureContext(), exclusive of those for http.createServer()
const httpsOptionKeys = [
'ALPNProtocols',
Expand Down Expand Up @@ -81,6 +87,16 @@ 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;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📚 suggestion: While it's not shared in other options right now we might add @jsdoc to this?

Let's also surface this in documentation here:

🔗 https://docs.slack.dev/tools/bolt-js/reference#receiver-options

clientId?: string;
clientSecret?: string;
stateSecret?: InstallProviderOptions['stateSecret']; // required when using default stateStore
Expand Down Expand Up @@ -137,6 +153,8 @@ export default class HTTPReceiver implements Receiver {

private signatureVerification: boolean;

private invalidRequestSignatureHandler: (args: HTTPReceiverInvalidRequestSignatureHandlerArgs) => void;

private app?: App;

public requestListener: RequestListener;
Expand Down Expand Up @@ -178,6 +196,7 @@ export default class HTTPReceiver implements Receiver {
logLevel = LogLevel.INFO,
processBeforeResponse = false,
signatureVerification = true,
invalidRequestSignatureHandler,
clientId = undefined,
clientSecret = undefined,
stateSecret = undefined,
Expand All @@ -195,6 +214,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 ??
(() => {
Expand Down Expand Up @@ -447,7 +468,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) ?? '',
ts: Number(req.headers['x-slack-request-timestamp']) || 0,
});
} else {
this.logger.warn(`Failed to parse the request body: ${e.message}`);
}
Expand Down Expand Up @@ -565,4 +592,12 @@ export default class HTTPReceiver implements Receiver {
installer.handleCallback(req, res, installCallbackOptions).catch(errorHandler);
}
}

private defaultInvalidRequestSignatureHandler(args: HTTPReceiverInvalidRequestSignatureHandlerArgs): void {
const { signature, ts } = args;

this.logger.warn(
`Invalid request signature detected (X-Slack-Signature: ${signature}, X-Slack-Request-Timestamp: ${ts})`,
);
}
}
124 changes: 124 additions & 0 deletions test/unit/receivers/HTTPReceiver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,130 @@ 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);
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 () => {
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 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();

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.equal(args.signature, '');
assert.equal(args.ts, 0);
assert.isUndefined((args as { logger?: unknown }).logger);
});
});

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);
Expand Down