fix(slack): timing-safe socket-token comparison (port 9824d33 slice) (#126)

Port the Slack slice of upstream 9824d33 (PR #441 adapter-hardening
pass): the forwarded Socket Mode events receiver validates the
`x-slack-socket-token` bearer against `socket_forwarding_secret`.

Encode both operands to UTF-8 bytes before `hmac.compare_digest`,
mirroring upstream `timingSafeStringEqual`'s `Buffer.from(x, "utf8")`
comparison. This keeps the constant-time guarantee, returns False on
length mismatch (the secret length is fixed at config time, so not a
leak), and avoids the `TypeError` that `str` `compare_digest` raises on
non-ASCII tokens (which would have surfaced as a crash rather than a
clean 401). Per CLAUDE.md Port Rule: `==` for signatures -> hmac.compare_digest.

Tests: spy on `hmac.compare_digest` to prove the timing-safe path is
exercised with byte operands (load-bearing against a regression to
`==`), plus a non-ASCII token is rejected with 401 rather than crashing.
Existing valid/invalid/missing-secret cases retained.

1 of 4 slices of the 9824d33 security pass (gchat/github/linear slices
ship as separate PRs). Refs #98.

https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj

Co-authored-by: Claude <noreply@anthropic.com>

Author: patrick-chinchill

src/chat_sdk/adapters/slack/adapter.py

@@ -1235,8 +1235,14 @@ async def handle_webhook(self, request: Any, options: WebhookOptions | None = No
         # success.
         socket_token = headers.get("x-slack-socket-token") or headers.get("X-Slack-Socket-Token")
         if socket_token:
+            # Constant-time bearer comparison (upstream timingSafeStringEqual,
+            # 9824d33 / PR #441). Encode both operands to UTF-8 bytes so
+            # ``compare_digest`` mirrors upstream's ``Buffer.from(x, "utf8")``
+            # comparison: it returns False on length mismatch (the secret's
+            # length is fixed at config time, so that is not a leak) and never
+            # raises on non-ASCII tokens the way str comparison would.
             if not self._socket_forwarding_secret or not hmac.compare_digest(
-                socket_token, self._socket_forwarding_secret
+                socket_token.encode(), self._socket_forwarding_secret.encode()
             ):
                 self._logger.warn("Invalid socket forwarding token")
                 return {"body": "Invalid socket token", "status": 401}

tests/test_slack_socket_mode.py

@@ -343,6 +343,85 @@ async def test_webhook_socket_token_rejected_when_no_secret_configured(self):
         result = await adapter.handle_webhook(request)
         assert result["status"] == 401
 
+    async def test_webhook_socket_token_uses_timing_safe_compare(self, monkeypatch):
+        """The socket-token check MUST route through ``hmac.compare_digest``.
+
+        Upstream switched this comparison to a constant-time check
+        (timingSafeStringEqual, 9824d33 / PR #441). This is the load-bearing
+        guard against a regression to a plain ``==`` / ``!=`` comparison:
+        we spy on ``hmac.compare_digest`` as referenced by the adapter module
+        and assert it is invoked for the socket token, with both operands
+        encoded to bytes (mirroring upstream's ``Buffer.from(x, "utf8")``).
+        """
+        import time as _time
+
+        from chat_sdk.adapters.slack import adapter as adapter_mod
+
+        calls: list[tuple[Any, Any]] = []
+        real_compare = adapter_mod.hmac.compare_digest
+
+        def _spy_compare(a, b):
+            calls.append((a, b))
+            return real_compare(a, b)
+
+        monkeypatch.setattr(adapter_mod.hmac, "compare_digest", _spy_compare)
+
+        adapter = _make_socket_adapter(socket_forwarding_secret="fwd-secret")
+        chat = _make_mock_chat()
+        adapter._chat = chat
+        forwarded = {
+            "type": "socket_event",
+            "eventType": "events_api",
+            "body": {
+                "type": "event_callback",
+                "event": {
+                    "type": "message",
+                    "channel": "C123",
+                    "ts": "1.0",
+                    "user": "U1",
+                    "text": "hi",
+                    "team": "T1",
+                },
+                "team_id": "T1",
+            },
+            "timestamp": int(_time.time()),
+        }
+        request = _FakeRequest(
+            json.dumps(forwarded),
+            headers={
+                "content-type": "application/json",
+                "x-slack-socket-token": "fwd-secret",
+            },
+        )
+        result = await adapter.handle_webhook(request)
+
+        # Valid token => accepted, AND the timing-safe path was exercised
+        # with byte-encoded operands.
+        assert result["status"] == 200
+        assert (b"fwd-secret", b"fwd-secret") in calls, (
+            f"socket token must be compared via hmac.compare_digest on bytes, not == ; recorded calls: {calls!r}"
+        )
+
+    async def test_webhook_socket_token_non_ascii_rejected_not_crash(self):
+        """A non-ASCII socket token must be rejected (401), never crash.
+
+        ``hmac.compare_digest`` raises ``TypeError`` on non-ASCII ``str``
+        operands; encoding to bytes (the upstream behavior) avoids that. This
+        pins the ``.encode()`` so a regression to str comparison surfaces as a
+        crash here rather than silently in production.
+        """
+        adapter = _make_socket_adapter(socket_forwarding_secret="fwd-secret")
+        adapter._chat = _make_mock_chat()
+        request = _FakeRequest(
+            json.dumps({"type": "socket_event", "eventType": "events_api", "body": {}}),
+            headers={
+                "content-type": "application/json",
+                "x-slack-socket-token": "café-not-the-secret",
+            },
+        )
+        result = await adapter.handle_webhook(request)
+        assert result["status"] == 401
+
 
 # ---------------------------------------------------------------------------
 # _route_socket_event dispatch