diff --git a/.changeset/slack-prefer-webhook-verifier.md b/.changeset/slack-prefer-webhook-verifier.md new file mode 100644 index 000000000..e31bcdf73 --- /dev/null +++ b/.changeset/slack-prefer-webhook-verifier.md @@ -0,0 +1,9 @@ +--- +"@chat-adapter/slack": patch +--- + +fix(slack): prefer `webhookVerifier` over `signingSecret` and `SLACK_SIGNING_SECRET` + +When a `webhookVerifier` is configured, it now takes precedence over both the +`signingSecret` config field and the `SLACK_SIGNING_SECRET` env var. Previously, +a configured `signingSecret` (or env var) would shadow the verifier. diff --git a/packages/adapter-slack/README.md b/packages/adapter-slack/README.md index 9579f7e06..979244c67 100644 --- a/packages/adapter-slack/README.md +++ b/packages/adapter-slack/README.md @@ -58,7 +58,7 @@ createSlackAdapter({ }); ``` -If both `signingSecret` and `webhookVerifier` are set, `signingSecret` wins. When using `webhookVerifier`, you are responsible for replay/timestamp protection — the built-in 5-minute timestamp tolerance only applies to the `signingSecret` path. +If both `signingSecret` and `webhookVerifier` are set, `webhookVerifier` wins — it also takes precedence over the `SLACK_SIGNING_SECRET` env var, so an env-configured deployment can't silently shadow a verifier you wired up. When using `webhookVerifier`, you are responsible for replay/timestamp protection — the built-in 5-minute timestamp tolerance only applies to the `signingSecret` path. ## Multi-workspace mode diff --git a/packages/adapter-slack/src/index.test.ts b/packages/adapter-slack/src/index.test.ts index 200aeec6f..99f59c4d0 100644 --- a/packages/adapter-slack/src/index.test.ts +++ b/packages/adapter-slack/src/index.test.ts @@ -450,7 +450,7 @@ describe("handleWebhook - webhookVerifier", () => { expect(verifier.mock.calls[0]?.[1]).toBe(body); }); - it("prefers signingSecret over webhookVerifier when both are set", async () => { + it("prefers webhookVerifier over signingSecret when both are set", async () => { const secret = "test-signing-secret"; const verifier = vi.fn(() => true); const adapter = createSlackAdapter({ @@ -464,11 +464,16 @@ describe("handleWebhook - webhookVerifier", () => { type: "url_verification", challenge: "test-challenge", }); - const request = createWebhookRequest(body, secret); + // No signing headers — only the verifier should run. + const request = new Request("https://example.com/webhook", { + method: "POST", + headers: { "content-type": "application/json" }, + body, + }); const response = await adapter.handleWebhook(request); expect(response.status).toBe(200); - expect(verifier).not.toHaveBeenCalled(); + expect(verifier).toHaveBeenCalledTimes(1); }); it("ignores SLACK_SIGNING_SECRET env var when webhookVerifier is configured", async () => { diff --git a/packages/adapter-slack/src/index.ts b/packages/adapter-slack/src/index.ts index dd6171415..114f5642c 100644 --- a/packages/adapter-slack/src/index.ts +++ b/packages/adapter-slack/src/index.ts @@ -208,8 +208,8 @@ export interface SlackAdapterConfig { * raw body for downstream parsing — useful when the verifier needs to * canonicalize or substitute the verified payload. * - * When both `signingSecret` and `webhookVerifier` are specified, - * `signingSecret` takes precedence. + * `webhookVerifier` takes precedence over `signingSecret` and the + * `SLACK_SIGNING_SECRET` env var; when it is set, those are ignored. * * SECURITY: When this is used in place of `signingSecret`, the built-in * Slack timestamp tolerance check is NOT performed. Implementations are @@ -566,12 +566,12 @@ export class SlackAdapter implements Adapter { constructor(config: SlackAdapterConfig = {}) { const webhookVerifier = config.webhookVerifier; - // An explicit webhookVerifier in config opts out of the SLACK_SIGNING_SECRET - // env fallback — otherwise an env-configured deployment would silently - // shadow the verifier the caller intended to use. - const signingSecret = - config.signingSecret ?? - (webhookVerifier ? undefined : process.env.SLACK_SIGNING_SECRET); + // webhookVerifier takes precedence over signingSecret (config) and the + // SLACK_SIGNING_SECRET env var. When the caller wires up a verifier we + // ignore both so an env-configured deployment can't silently shadow it. + const signingSecret = webhookVerifier + ? undefined + : (config.signingSecret ?? process.env.SLACK_SIGNING_SECRET); if ( !(signingSecret || webhookVerifier) && (config.mode ?? "webhook") === "webhook" @@ -603,10 +603,10 @@ export class SlackAdapter implements Adapter { this.client = new WebClient(undefined, { ...(slackApiUrl ? { slackApiUrl } : {}), }); - // webhookVerifier takes precedence when signingSecret is not configured; - // if both are provided, signingSecret wins. + // webhookVerifier takes precedence; signingSecret is only used when no + // verifier is configured. this.signingSecret = signingSecret; - this.webhookVerifier = signingSecret ? undefined : webhookVerifier; + this.webhookVerifier = webhookVerifier; this.defaultBotTokenProvider = botTokenProvider; this.logger = config.logger ?? new ConsoleLogger("info").child("slack"); this.userName = config.userName || "bot"; @@ -4902,12 +4902,12 @@ export function createSlackAdapter( } const webhookVerifier = config?.webhookVerifier; - // An explicit webhookVerifier in config opts out of the SLACK_SIGNING_SECRET - // env fallback — otherwise an env-configured deployment would silently - // shadow the verifier the caller intended to use. - const signingSecret = - config?.signingSecret ?? - (webhookVerifier ? undefined : process.env.SLACK_SIGNING_SECRET); + // webhookVerifier takes precedence over signingSecret (config) and the + // SLACK_SIGNING_SECRET env var. When the caller wires up a verifier we + // ignore both so an env-configured deployment can't silently shadow it. + const signingSecret = webhookVerifier + ? undefined + : (config?.signingSecret ?? process.env.SLACK_SIGNING_SECRET); if (mode === "webhook" && !(signingSecret || webhookVerifier)) { throw new ValidationError( "slack",