feat(receivers): add invalidRequestSignatureHandler to HTTP/Express receivers

[singhvishalkr]

Summary

Adds an invalidRequestSignatureHandler option to HTTPReceiver and ExpressReceiver, mirroring the existing functionality in AwsLambdaReceiver from PR #2154.

When a request fails signature verification, this handler fires before the 401 response is sent. It receives the signature, timestamp, body, request, and response objects so developers can add custom logging, metrics collection, or alerting.

Changes

• Added ReceiverInvalidRequestSignatureHandlerArgs interface to HTTPModuleFunctions.ts
• Added invalidRequestSignatureHandler option to HTTPReceiverOptions and ExpressReceiverOptions
• Both receivers call the handler when signature verification fails
• Exported the new interface from src/index.ts

Test plan

• Ran existing HTTPReceiver and ExpressReceiver test suites - all tests pass
• The implementation follows the same pattern as AwsLambdaReceiver

Fixes #2156

[changeset-bot]

⚠️ No Changeset found

Latest commit: be8ad00

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[salesforce-cla]

Thanks for the contribution! Before we can merge this, we need @singhvishalkr to sign the Salesforce Inc. Contributor License Agreement.

[tsushanth]

Thanks for picking this up — extending the handler to ExpressReceiver is a real scope improvement over #2827 (which the author explicitly punted Express to a follow-up). Two pieces of context worth surfacing before the code feedback.

Context: #2827 is the in-flight PR for the same feature

#2827 by @mvanhorn covers the HTTPReceiver half of this and has been through three rounds of review with @zimeg, most recently landing at "Thank you for bringing this to a great place! I'm requesting another change for a scoped PR" with one final scoping ask. The interface name and default-handler shape there were explicitly negotiated to align with the existing AWS pattern. Worth reading that thread before iterating here — there's a path where this PR rebases on #2827 and adds only the Express delta, which would land the feature without discarding the maintainer review already invested in #2827.

Must-fix

1. Interface name collides with AwsLambdaReceiver.ts:52 and silently changes the public type.

ReceiverInvalidRequestSignatureHandlerArgs already exists on main:

// src/receivers/AwsLambdaReceiver.ts:52
export interface ReceiverInvalidRequestSignatureHandlerArgs {
  rawBody: string;
  signature: string;
  ts: number;
  awsEvent: AwsEvent;
  awsResponse: Promise<AwsResponse>;
}

This PR introduces a different interface with the same name in src/receivers/HTTPModuleFunctions.ts:

export interface ReceiverInvalidRequestSignatureHandlerArgs {
  signature: string | undefined;
  ts: number | undefined;
  body: string;          // renamed from rawBody
  request: IncomingMessage;
  response: ServerResponse;
}

The field names differ (body vs rawBody), the optionality differs (string | undefined vs non-optional string), and the AWS interface isn't currently re-exported from src/index.ts while this PR newly re-exports the HTTP variant from there (the addition at src/index.ts:66 to the HTTPModuleFunctions export block). After this PR lands, anyone who was deep-importing the AWS shape and migrates to import { ReceiverInvalidRequestSignatureHandlerArgs } from '@slack/bolt' gets a silent type change with no deprecation path.

Two clean resolutions:

• (a) Rename the new interface to HTTPReceiverInvalidRequestSignatureHandlerArgs (the approach #2827 originally took before zimeg pushed for shared naming). Export both names from src/index.ts. No collision, no migration ambiguity.
• (b) Keep the shared name but make it actually shared — match the AWS shape (rawBody, non-optional signature: string, non-optional ts: number) and add receiver-specific fields as an optional intersection (awsEvent?, awsResponse?, request?, response?, etc.). This lets users write one handler that works across receivers, which is the whole point of the parity argument.

I'd lean (b) — strictly more useful outcome — but it requires explicit agreement that awsEvent/awsResponse become optional on the shared interface, which is a coordinated change with #2827 and the AWS side.

2. HTTPReceiver.ts always passes body: ''.

// new code around HTTPReceiver.ts:461
this.invalidRequestSignatureHandler({
  signature: Array.isArray(signature) ? signature[0] : signature,
  ts: ts ? Number(Array.isArray(ts) ? ts[0] : ts) : undefined,
  body: '',          // <-- always empty
  request: req,
  response: res,
});

The ExpressReceiver path in this PR (around line 486) passes the actual body: stringBody, so a developer wiring one handler across both receivers gets '' from HTTP and the real payload from Express — silently broken, and the difference isn't covered by any test. #2827's HTTPReceiver path reads rawBody defensively in case it was set by upstream parsing:

const requestWithRawBody = req as IncomingMessage & { rawBody?: string };
const rawBody = typeof requestWithRawBody.rawBody === 'string' ? requestWithRawBody.rawBody : '';

Either adopt that pattern, or surface the buffer through parseAndVerifyHTTPRequest so it's available here without the cast. Empty-string-by-default is worse than missing-by-default because callers can't distinguish "never received a body" from "we just didn't bother passing it."

3. No tests.

src/ only — no spec changes. #2827 added 124 LOC of test/unit/receivers/HTTPReceiver.spec.ts covering: handler invocation with correct args, default-handler no-throw, missing-header fallback. The same three cases need to be added here for both HTTPReceiver and ExpressReceiver, plus an explicit assertion that the body payload is symmetric between the two — item 2 above will fall out of that assertion immediately. @mwbrooks made the same request on #2827 ("can you please add tests for the new changes? I imagine that'll be one of the first requests from the reviewers"), and the same will apply here.

4. Changeset missing.

No .changeset/*.md — the bot has flagged it. A minor entry is needed since this adds a public option to two receivers.

5. CLA.

Salesforce CLA bot is blocking; needs to be signed before merge.

Worth discussing

A. defaultInvalidRequestSignatureHandler in HTTPModuleFunctions.ts is a no-op:

export const defaultInvalidRequestSignatureHandler = (args: ReceiverInvalidRequestSignatureHandlerArgs): void => {
  const { signature, ts } = args;
  // Intentionally does nothing by default. The receiver already responds with 401.
  void signature;
  void ts;
};

AwsLambdaReceiver's default for this hook logs the signature + timestamp at warn level (its defaultInvalidRequestSignatureHandler does that). #2827's default does the same. The existing this.logger.warn('Failed to parse and verify the request data: ...') is preserved at the call site here so users don't lose visibility entirely, but a default handler that prints the actual signature/ts is strictly more useful than void signature; void ts; — that body reads like the implementation got deleted during a refactor. Either log it (parity with AWS) or remove the destructure entirely so the empty body is intentional.

B. buildVerificationBodyParserMiddleware's invalidRequestSignatureHandler parameter in ExpressReceiver.ts:460 is typed as optional:

invalidRequestSignatureHandler?: (args: httpFunc.ReceiverInvalidRequestSignatureHandlerArgs) => void,

but the only call site (around line 209) always passes a value because the constructor default fires:

invalidRequestSignatureHandler = httpFunc.defaultInvalidRequestSignatureHandler,

The if (invalidRequestSignatureHandler) guard around line 480 then becomes dead defensive code. Tighten the parameter to non-optional and drop the guard.

C. No JSDoc on HTTPReceiverOptions.invalidRequestSignatureHandler or ExpressReceiverOptions.invalidRequestSignatureHandler. #2827's version has a JSDoc block on the option calling out the non-obvious behavior ("The receiver still returns 401 Unauthorized to the client regardless of what the handler does") — that's the kind of detail that prevents a developer from thinking they can override the response by, say, calling res.status(200).send() inside the handler.

Decision

Requesting changes. The Express expansion is genuinely valuable, and a merged version of this would close out #2156 more completely than #2827 alone. But the interface name collision is a public-API foot-gun on its own, the empty body payload is a functional bug, and the test/changeset/CLA gaps are merge blockers regardless. The cleanest path forward is probably to coordinate with @mvanhorn on #2827 — either rebase the Express delta on top of that branch, or land a renamed-and-tested version here that explicitly doesn't collide with the AWS interface.

Happy to re-review once items 1–3 are addressed.

[singhvishalkr]

Thanks for the detailed review. Given #2827 already has the negotiated HTTPReceiver API shape and active maintainer review, I do not want to keep a competing public-interface change open here.

Closing this PR. The ExpressReceiver part is still useful, but it should be a small follow-up on top of #2827 once that shape lands or can be rebased cleanly.