Skip to content

Commit 9fecb88

Browse files
committed
fix(slack): prefer webhook_verifier over signing_secret (vercel/chat#468)
BREAKING: flips precedence to match upstream's reversal in vercel/chat#468 (commit 0f0c203, chat@4.29.0). When both webhook_verifier and signing_secret are configured, webhook_verifier now wins; SLACK_SIGNING_SECRET env var is also ignored when a verifier is set, so an env-configured deployment can't silently shadow a verifier the caller wired up. This reverses the precedence the Python port shipped in 0.4.27 (PR #87), which preferred signing_secret to track upstream's intent at that time — upstream has since reversed itself and this port follows. The stale comment at adapter.py:385 ("matches upstream") is updated accordingly. Part of the 0.4.29 sync wave; tracking issue #98. https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj
1 parent c43ea05 commit 9fecb88

5 files changed

Lines changed: 84 additions & 25 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
Part of the 0.4.29 sync wave (tracking issue #98).
6+
7+
### Behavior changes (Slack)
8+
9+
- **`webhook_verifier` now takes precedence over `signing_secret`** (#108, vercel/chat#468, commit `0f0c203`). When a Slack adapter is constructed with both `webhook_verifier` and `signing_secret` (or with `webhook_verifier` while `SLACK_SIGNING_SECRET` is set in the env), the verifier wins and the signing-secret path is dropped entirely. This **reverses** the precedence the Python port shipped in 0.4.27 (PR #87), which preferred `signing_secret` to match upstream's intent at that time. Upstream reversed itself in vercel/chat#468 (`chat@4.29.0`) so an env-configured `SLACK_SIGNING_SECRET` could not silently shadow a verifier the caller wired up; this port now follows. **Migration:** if you relied on a configured `signing_secret` overriding `webhook_verifier`, drop the `webhook_verifier` from your `SlackAdapterConfig` (or, if you wired the verifier in deliberately, your signing-secret path is now correctly inert and you can remove it). The built-in HMAC + 5-minute timestamp tolerance only applies on the signing-secret path; verifier implementers remain responsible for replay protection (`slack/types.py` SECURITY contract).
10+
311
## 0.4.27 (2026-05-28)
412

513
Synced to upstream `vercel/chat@4.27.0` (release commit `f55378a`, Apr 30 2026). Highlights: Slack Socket Mode + dynamic bot-token resolver, Teams native DM streaming, `chat.get_user()` across all 8 adapters, Telegram MarkdownV2 rendering, and a sweep of adapter bug fixes. Sets `UPSTREAM_PARITY = "4.27.0"`.

docs/UPSTREAM_SYNC.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ stay explicit instead of being rediscovered in code review.
624624
| `RawMessage.text` override field — **transitional** | New optional `text: str \| None = None` field on `RawMessage` (`src/chat_sdk/types.py`). When set by an adapter, `Thread._handle_stream` MUST prefer it over its own local accumulator when constructing the recorded `SentMessage` body / message-history entry. `None` falls back to the local accumulator (backward-compatible default for adapters that don't need the override). Set by `_stream_via_emit` so the recorded message matches what Teams actually shipped, even when chunks were buffered into the throttle window and cancellation skipped the flush emit. | Upstream's `RawMessage` (`packages/chat/src/types.ts`) is `{ id; raw; threadId }` only. Cancellation-text reconciliation lives inside `@microsoft/teams.apps`'s `IStreamer.emit` (the npm SDK owns the buffer and never surfaces a buffered-but-unsent suffix to `chat`). | Direct consequence of the hand-rolled Teams native streaming row above. Without the override, the SDK's local accumulator (which captures every chunk yielded to the adapter, including chunks coalesced into the throttle window) would diverge from what Teams actually accepted whenever a session is canceled with buffered text pending — recording text the user never saw. Disappears alongside the hand-rolled wire format once we migrate to `microsoft-teams-apps` (Python). Regression coverage: `tests/test_thread_faithful.py::test_should_prefer_raw_message_text_override_over_local_accumulator` (would fail if someone "fixes" Thread.stream back to upstream's local-accumulator-only behavior) and `tests/test_teams_native_streaming.py::test_canceled_stream_sets_raw_message_text_override`. |
625625
| Teams divider rendering | `card_to_adaptive_card` hoists `separator: True` onto the next sibling (or emits a non-empty Container for a trailing divider) | `convertDividerToElement` emits an empty `Container` with `separator: True` | Upstream shares the same bug: Microsoft Teams renders an empty Container at zero height, so the separator line is effectively invisible. Python port fixes locally (issue #45) rather than blocking on upstream. |
626626
| `SlackAdapter.current_token` / `current_token_async` / `current_client` | Public accessors that return the request-context-bound token and a preconfigured `AsyncWebClient`. `current_token` (sync `@property`) reads the cache; `current_token_async` (async method) invokes the resolver on demand for callable `bot_token` configs used outside `handle_webhook`. | Not exposed (`getToken()` is private on the TS `SlackAdapter`) | Python-only addition (issue #47). Downstream code that calls Slack Web APIs from inside a handler — email resolution, user profile fetches, reaction bookkeeping — otherwise depends on underscore-prefixed helpers. The async variant is required because the sync `current_token` cannot drive an async resolver (see `bot_token` resolver invocation site row). |
627-
| `SlackAdapterConfig.webhook_verifier` | Optional `Callable[[request, body], bool \| str \| None \| Awaitable[...]]` that fully replaces signing-secret HMAC verification. Lets callers integrate platform-managed verification (e.g. Slack Enterprise Grid edge proxies, KMS-signed payloads, test harness escape hatches). Mutually exclusive with `signing_secret` — explicit `signing_secret` wins when both are passed. | Not exposed — TS only honors `SLACK_SIGNING_SECRET` HMAC verification (`signRequest` / `verifyRequestSignature`) | Python-only extension. The contract is documented as a SECURITY surface in `slack/types.py` (`SlackWebhookVerifier`): returning truthy passes the request, falsy/None rejects 401, and a `str` substitutes the request body before dispatch. Closes the same gap that drives `bot_token` resolvers (custom secret-manager integration) for the signing-secret half of the auth model. |
627+
| `SlackAdapterConfig.webhook_verifier` | Optional `Callable[[request, body], bool \| str \| None \| Awaitable[...]]` that fully replaces signing-secret HMAC verification. Lets callers integrate platform-managed verification (e.g. Slack Enterprise Grid edge proxies, KMS-signed payloads, test harness escape hatches). `webhook_verifier` takes precedence over both `signing_secret` (config) and the `SLACK_SIGNING_SECRET` env var — when set, both are ignored. | Upstream has its own `webhookVerifier` field on `SlackAdapterConfig` and matches this precedence direction after vercel/chat#468 (commit `0f0c203`, chat@4.29.0). | Behavior parity restored in 0.4.29 sync wave. The original Python port (PR #87, 0.4.27) preferred `signing_secret` to match upstream's intent at that time; upstream reversed itself in #468 so an env-configured `SLACK_SIGNING_SECRET` could not silently shadow a verifier the caller wired up. This port follows. The contract is documented as a SECURITY surface in `slack/types.py` (`SlackWebhookVerifier`): returning truthy passes the request, falsy/None rejects 401, and a `str` substitutes the request body before dispatch. |
628628
| Slack `bot_token` resolver invocation site | Resolved once at `handle_webhook` entry into a per-request `ContextVar`; sync `_get_token` reads it for the rest of the request. Public adapter methods (`post_message`, `add_reaction`, `upload_files`, etc.) DON'T re-resolve — calling them outside `handle_webhook` (cron jobs, background tasks) with a callable `bot_token` raises `AuthenticationError` until the caller awaits `current_token_async()` first | TS `getToken` is async and resolves on EVERY API call site, so cron/background usage just works | Python keeps `_get_token` sync to preserve the existing pre-resolver public API and to avoid threading `await` through every adapter call site. The trade-off is that callable-`bot_token` usage outside the webhook flow needs an explicit `await adapter.current_token_async()` (or `await adapter._resolve_default_token()`) before the first sync-token-consuming call. Static-string `bot_token` is unaffected (cache primed at construction). |
629629
| Slack `bot_token` resolver caching scope | Single resolution per request, cached in `_resolved_default_token` `ContextVar` for the rest of that request | Provider invoked on every API call within a single request | Within-request caching enables the sync `_get_token` path. Functionally equivalent for rotation (TTL >> request lifetime); diverges only if the resolver is itself sensitive to per-call freshness (rare). |
630630
| `ConcurrencyConfig.max_concurrent` | Enforced via `asyncio.Semaphore` in the `"concurrent"` strategy path; rejects non-integer or `<= 0` values, and rejects any non-`None` `max_concurrent` paired with a non-`"concurrent"` strategy | Accepted into the config type with docstring "Default: Infinity" but never read (3 writes, 0 reads) | Silent correctness bug upstream — consumers setting `max_concurrent=N` with `strategy="concurrent"` reasonably expect an N-way bound on in-flight handlers. We honor the documented contract via a semaphore and fail-fast on misconfiguration so it's never silent. `max_concurrent=None` stays compatible with every strategy (unbounded default). |

src/chat_sdk/adapters/slack/adapter.py

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -276,9 +276,11 @@ def __init__(self, config: SlackAdapterConfig | None = None) -> None:
276276

277277
mode = config.mode or "webhook"
278278

279-
# An explicit ``webhook_verifier`` opts out of the SLACK_SIGNING_SECRET
280-
# env fallback — otherwise an env-configured deployment would silently
281-
# shadow the verifier the caller intended to use.
279+
# ``webhook_verifier`` takes precedence over ``signing_secret`` (config)
280+
# and the ``SLACK_SIGNING_SECRET`` env var. When the caller wires up a
281+
# verifier we ignore both so an env-configured deployment can't silently
282+
# shadow it (mirrors upstream vercel/chat#468, which reversed the
283+
# original direction the Python port shipped in PR #87).
282284
#
283285
# Use explicit ``is not None`` checks rather than truthiness fallbacks
284286
# (per docs/UPSTREAM_SYNC.md hazard #1): an explicit empty string for
@@ -317,13 +319,17 @@ def __init__(self, config: SlackAdapterConfig | None = None) -> None:
317319
"slack",
318320
"webhook_verifier must be callable.",
319321
)
320-
signing_secret = (
321-
config.signing_secret
322-
if config.signing_secret is not None
323-
else (None if webhook_verifier is not None else os.environ.get("SLACK_SIGNING_SECRET"))
324-
)
325-
if signing_secret == "":
326-
signing_secret = None
322+
if webhook_verifier is not None:
323+
# Verifier wins: drop both the config ``signing_secret`` and the
324+
# ``SLACK_SIGNING_SECRET`` env fallback. Mirrors upstream
325+
# vercel/chat#468 (``webhookVerifier`` ?? undefined : (...)).
326+
signing_secret: str | None = None
327+
else:
328+
signing_secret = (
329+
config.signing_secret if config.signing_secret is not None else os.environ.get("SLACK_SIGNING_SECRET")
330+
)
331+
if signing_secret == "":
332+
signing_secret = None
327333
# ``signing_secret`` is required in webhook mode (unless a
328334
# ``webhook_verifier`` replaces the built-in HMAC check). Socket mode
329335
# legitimately runs without a signing secret because Slack does not
@@ -381,9 +387,9 @@ def __init__(self, config: SlackAdapterConfig | None = None) -> None:
381387

382388
self._name = "slack"
383389
self._signing_secret: str | None = signing_secret
384-
# ``webhook_verifier`` is honored only when ``signing_secret`` is not set —
385-
# signing_secret takes precedence when both are provided (matches upstream).
386-
self._webhook_verifier: SlackWebhookVerifier | None = None if signing_secret else webhook_verifier
390+
# ``webhook_verifier`` takes precedence; ``signing_secret`` is only used
391+
# when no verifier is configured (matches upstream vercel/chat#468).
392+
self._webhook_verifier: SlackWebhookVerifier | None = webhook_verifier
387393
# Resolver returning the default (single-workspace) bot token. ``None`` in
388394
# multi-workspace mode where the token is resolved per-team from the
389395
# InstallationStore. Single-workspace mode with a static string still
@@ -1276,9 +1282,11 @@ async def handle_webhook(self, request: Any, options: WebhookOptions | None = No
12761282
if self._mode == "socket":
12771283
return {"body": "Webhooks are disabled in socket mode", "status": 405}
12781284

1279-
# Verify the request — custom verifier takes priority when configured
1280-
# without a signing_secret. The verifier may also return a string that
1281-
# replaces the body for downstream parsing (e.g. canonicalization).
1285+
# Verify the request — when a custom ``webhook_verifier`` is configured
1286+
# it takes precedence over ``signing_secret`` / ``SLACK_SIGNING_SECRET``
1287+
# (matches upstream vercel/chat#468). The verifier may also return a
1288+
# string that replaces the body for downstream parsing (e.g.
1289+
# canonicalization).
12821290
if self._webhook_verifier is not None:
12831291
try:
12841292
verifier_result = self._webhook_verifier(request, body)

src/chat_sdk/adapters/slack/types.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,16 @@ class SlackAdapterConfig:
8585
# not required in socket mode (Slack does not sign socket events).
8686
mode: SlackAdapterMode = "webhook"
8787
# Signing secret for webhook verification. Defaults to SLACK_SIGNING_SECRET env var,
88-
# *unless* ``webhook_verifier`` is provided — passing an explicit verifier opts
89-
# out of the env fallback so a deployment-set ``SLACK_SIGNING_SECRET`` can't
90-
# silently shadow the verifier. Required in webhook mode; optional in socket mode.
88+
# *unless* ``webhook_verifier`` is provided — an explicit verifier takes
89+
# precedence over both this field and the ``SLACK_SIGNING_SECRET`` env var,
90+
# so an env-configured deployment can't silently shadow the verifier the
91+
# caller wired up. Required in webhook mode; optional in socket mode.
9192
signing_secret: str | None = None
9293
# Custom webhook verifier. When provided, replaces the built-in HMAC + timestamp
9394
# check. See :data:`SlackWebhookVerifier` for the SECURITY contract — the
9495
# implementer is responsible for constant-time comparison and replay protection.
95-
# When both ``signing_secret`` and ``webhook_verifier`` are set, ``signing_secret``
96-
# takes precedence.
96+
# ``webhook_verifier`` takes precedence over ``signing_secret`` and the
97+
# ``SLACK_SIGNING_SECRET`` env var; when it is set, those are ignored.
9798
webhook_verifier: SlackWebhookVerifier | None = None
9899
# Shared secret for authenticating events forwarded from a separate
99100
# socket-mode listener via HTTP POST. Auto-detected from

tests/test_slack_dynamic_token_and_verifier.py

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,16 @@ def test_webhook_verifier_alone_is_sufficient(self):
385385
if old is not None:
386386
os.environ["SLACK_SIGNING_SECRET"] = old
387387

388-
def test_signing_secret_takes_precedence_over_verifier(self):
388+
def test_webhook_verifier_takes_precedence_over_signing_secret(self):
389+
"""When both are set, ``webhook_verifier`` wins.
390+
391+
Port of upstream vercel/chat#468 (commit ``0f0c203``): the original
392+
Python port (PR #87) and upstream both used to prefer
393+
``signing_secret``; upstream has since reversed itself and this port
394+
follows. Callers wiring up a verifier no longer have it silently
395+
shadowed by a present ``signing_secret`` (or ``SLACK_SIGNING_SECRET``
396+
env var; see :meth:`test_verifier_opts_out_of_env_signing_secret`).
397+
"""
389398
verifier_called: list[int] = []
390399

391400
def verifier(req: Any, body: str) -> bool:
@@ -399,8 +408,10 @@ def verifier(req: Any, body: str) -> bool:
399408
webhook_verifier=verifier,
400409
)
401410
)
402-
# Internal: webhook_verifier was suppressed because signing_secret won.
403-
assert adapter._webhook_verifier is None
411+
# The verifier wins; the explicit signing_secret is dropped so the
412+
# built-in HMAC path is never taken.
413+
assert adapter._webhook_verifier is verifier
414+
assert adapter._signing_secret is None
404415

405416
def test_verifier_opts_out_of_env_signing_secret(self):
406417
"""An explicit verifier suppresses the SLACK_SIGNING_SECRET env fallback.
@@ -791,6 +802,37 @@ def verifier(req: Any, body: str) -> str:
791802
assert response["status"] == 200
792803
assert json.loads(response["body"]) == {"challenge": "canonical"}
793804

805+
@pytest.mark.asyncio
806+
async def test_verifier_runs_even_when_signing_secret_is_configured(self):
807+
"""``webhook_verifier`` wins over a configured ``signing_secret``.
808+
809+
Port of upstream vercel/chat#468 — the upstream test was previously
810+
titled "prefers signingSecret over webhookVerifier" and asserted
811+
``verifier`` was *not* called when both were set. Upstream reversed
812+
that behavior; this Python port mirrors the new direction: with no
813+
signing headers (which would fail the built-in HMAC check), the
814+
verifier still runs and the request succeeds.
815+
"""
816+
verifier_calls: list[tuple[Any, str]] = []
817+
818+
def verifier(req: Any, body: str) -> bool:
819+
verifier_calls.append((req, body))
820+
return True
821+
822+
adapter = SlackAdapter(
823+
SlackAdapterConfig(
824+
signing_secret="test-signing-secret",
825+
bot_token="xoxb-x",
826+
webhook_verifier=verifier,
827+
)
828+
)
829+
body = json.dumps({"type": "url_verification", "challenge": "test-challenge"})
830+
# No signing headers — only the verifier should run.
831+
response = await adapter.handle_webhook(_FakeRequest(body, {"content-type": "application/json"}))
832+
assert response["status"] == 200
833+
assert json.loads(response["body"]) == {"challenge": "test-challenge"}
834+
assert len(verifier_calls) == 1
835+
794836
@pytest.mark.asyncio
795837
async def test_verifier_path_does_not_invoke_default_signature_check(self):
796838
"""When verifier is configured, the built-in HMAC + timestamp check is skipped.

0 commit comments

Comments
 (0)