From bbf02c322443b7e833d730bbfa1dbd5d246e71e2 Mon Sep 17 00:00:00 2001 From: BaxBit Date: Sun, 24 May 2026 04:45:13 -0700 Subject: [PATCH] fix(gateway): validate Svix webhook signatures (#30200) --- gateway/platforms/webhook.py | 85 +++++++++++- tests/gateway/test_webhook_adapter.py | 181 ++++++++++++++++++++++++++ 2 files changed, 264 insertions(+), 2 deletions(-) diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index 93017de4e2b..354cc797c39 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -27,6 +27,8 @@ Security: """ import asyncio +import base64 +import binascii import hashlib import hmac import json @@ -419,6 +421,7 @@ class WebhookAdapter(BasePlatformAdapter): request.headers.get("X-GitHub-Event", "") or request.headers.get("X-GitLab-Event", "") or payload.get("event_type", "") + or payload.get("type", "") or "unknown" ) allowed_events = route_config.get("events", []) @@ -471,7 +474,10 @@ class WebhookAdapter(BasePlatformAdapter): # Build a unique delivery ID delivery_id = request.headers.get( "X-GitHub-Delivery", - request.headers.get("X-Request-ID", str(int(time.time() * 1000))), + request.headers.get( + "svix-id", + request.headers.get("X-Request-ID", str(int(time.time() * 1000))), + ), ) # ── Idempotency ───────────────────────────────────────── @@ -616,7 +622,32 @@ class WebhookAdapter(BasePlatformAdapter): def _validate_signature( self, request: "web.Request", body: bytes, secret: str ) -> bool: - """Validate webhook signature (GitHub, GitLab, generic HMAC-SHA256).""" + """Validate webhook signature (GitHub, GitLab, Svix, generic HMAC-SHA256).""" + def _header(name: str) -> str: + return ( + request.headers.get(name, "") + or request.headers.get(name.lower(), "") + or request.headers.get(name.upper(), "") + ) + + # Svix / AgentMail: + # svix-id: msg_... + # svix-timestamp: unix seconds + # svix-signature: v1, [v1, ...] + # Signed content is: "{id}.{timestamp}.{raw_body}". Svix secrets + # usually start with "whsec_" and the remainder is base64-encoded. + svix_id = _header("svix-id") + svix_timestamp = _header("svix-timestamp") + svix_signature = _header("svix-signature") + if svix_id or svix_timestamp or svix_signature: + return self._validate_svix_signature( + body=body, + secret=secret, + msg_id=svix_id, + timestamp=svix_timestamp, + signature_header=svix_signature, + ) + # GitHub: X-Hub-Signature-256 = sha256= gh_sig = request.headers.get("X-Hub-Signature-256", "") if gh_sig: @@ -644,6 +675,56 @@ class WebhookAdapter(BasePlatformAdapter): ) return False + def _validate_svix_signature( + self, + body: bytes, + secret: str, + msg_id: str, + timestamp: str, + signature_header: str, + tolerance_seconds: int = 300, + ) -> bool: + """Validate Svix-compatible signatures used by AgentMail webhooks.""" + if not (msg_id and timestamp and signature_header and secret): + return False + + try: + ts = int(timestamp) + except (TypeError, ValueError): + return False + if abs(int(time.time()) - ts) > tolerance_seconds: + logger.warning("[webhook] Svix signature timestamp outside replay window") + return False + + if secret.startswith("whsec_"): + encoded_secret = secret.removeprefix("whsec_") + try: + key = base64.b64decode(encoded_secret, validate=True) + except (binascii.Error, ValueError): + logger.debug("[webhook] Invalid whsec_ Svix signing secret") + return False + else: + # Be permissive for providers that document Svix-style headers but + # hand out raw shared secrets rather than whsec_ base64 secrets. + logger.debug("[webhook] Validating Svix-style signature with raw secret") + key = secret.encode() + + signed_content = msg_id.encode() + b"." + timestamp.encode() + b"." + body + expected = base64.b64encode( + hmac.new(key, signed_content, hashlib.sha256).digest() + ).decode() + + # Svix can send multiple signatures separated by spaces during secret + # rotation. Each entry is formatted as "vN,". + for part in signature_header.split(): + try: + version, signature = part.split(",", 1) + except ValueError: + continue + if version == "v1" and hmac.compare_digest(signature, expected): + return True + return False + # ------------------------------------------------------------------ # Prompt rendering # ------------------------------------------------------------------ diff --git a/tests/gateway/test_webhook_adapter.py b/tests/gateway/test_webhook_adapter.py index 8ca98cfb2bf..153bdfd6830 100644 --- a/tests/gateway/test_webhook_adapter.py +++ b/tests/gateway/test_webhook_adapter.py @@ -15,6 +15,7 @@ Covers: """ import asyncio +import base64 import hashlib import hmac import json @@ -100,6 +101,18 @@ def _generic_signature(body: bytes, secret: str) -> str: return hmac.new(secret.encode(), body, hashlib.sha256).hexdigest() +def _svix_signature(body: bytes, secret: str, msg_id: str, timestamp: str) -> str: + """Compute a Svix v1 signature header for *body* using *secret*.""" + key = ( + base64.b64decode(secret.removeprefix("whsec_")) + if secret.startswith("whsec_") + else secret.encode() + ) + signed = msg_id.encode() + b"." + timestamp.encode() + b"." + body + digest = hmac.new(key, signed, hashlib.sha256).digest() + return "v1," + base64.b64encode(digest).decode() + + # =================================================================== # Signature validation # =================================================================== @@ -170,6 +183,134 @@ class TestValidateSignature: req = _mock_request(headers={"X-Webhook-Signature": sig}) assert adapter._validate_signature(req, body, secret) is True + def test_validate_svix_signature_valid(self): + """Valid Svix/AgentMail v1 signature headers are accepted.""" + adapter = _make_adapter() + body = b'{"event_type":"message.received"}' + secret = "whsec_" + base64.b64encode(b"agentmail-signing-secret").decode() + msg_id = "msg_123" + timestamp = str(int(time.time())) + sig = _svix_signature(body, secret, msg_id, timestamp) + req = _mock_request( + headers={ + "svix-id": msg_id, + "svix-timestamp": timestamp, + "svix-signature": sig, + } + ) + assert adapter._validate_signature(req, body, secret) is True + + def test_validate_svix_signature_wrong_body_rejects(self): + """Svix/AgentMail signatures are bound to the exact raw request body.""" + adapter = _make_adapter() + signed_body = b'{"event_type":"message.received"}' + received_body = b'{"event_type":"message.sent"}' + secret = "whsec_" + base64.b64encode(b"agentmail-signing-secret").decode() + msg_id = "msg_123" + timestamp = str(int(time.time())) + sig = _svix_signature(signed_body, secret, msg_id, timestamp) + req = _mock_request( + headers={ + "svix-id": msg_id, + "svix-timestamp": timestamp, + "svix-signature": sig, + } + ) + assert adapter._validate_signature(req, received_body, secret) is False + + def test_validate_svix_signature_old_timestamp_rejects(self): + """Svix/AgentMail signatures outside the replay window are rejected.""" + adapter = _make_adapter() + body = b'{"event_type":"message.received"}' + secret = "whsec_" + base64.b64encode(b"agentmail-signing-secret").decode() + msg_id = "msg_123" + timestamp = str(int(time.time()) - 301) + sig = _svix_signature(body, secret, msg_id, timestamp) + req = _mock_request( + headers={ + "svix-id": msg_id, + "svix-timestamp": timestamp, + "svix-signature": sig, + } + ) + assert adapter._validate_signature(req, body, secret) is False + + def test_validate_svix_signature_multiple_entries_accepts_matching_v1(self): + """Svix rotation headers may contain multiple space-separated signatures.""" + adapter = _make_adapter() + body = b'{"event_type":"message.received"}' + secret = "whsec_" + base64.b64encode(b"agentmail-signing-secret").decode() + msg_id = "msg_123" + timestamp = str(int(time.time())) + sig = _svix_signature(body, secret, msg_id, timestamp) + req = _mock_request( + headers={ + "svix-id": msg_id, + "svix-timestamp": timestamp, + "svix-signature": "v1,wrong " + sig, + } + ) + assert adapter._validate_signature(req, body, secret) is True + + def test_validate_svix_signature_missing_signature_rejects(self): + """Partial Svix headers reject instead of falling through to another scheme.""" + adapter = _make_adapter() + req = _mock_request(headers={"svix-id": "msg_123"}) + assert adapter._validate_signature(req, b"{}", "secret") is False + + def test_validate_svix_signature_unsupported_version_rejects(self): + """Only Svix v1 signatures are accepted.""" + adapter = _make_adapter() + body = b'{"event_type":"message.received"}' + secret = "whsec_" + base64.b64encode(b"agentmail-signing-secret").decode() + msg_id = "msg_123" + timestamp = str(int(time.time())) + sig = _svix_signature(body, secret, msg_id, timestamp).replace("v1,", "v2,") + req = _mock_request( + headers={ + "svix-id": msg_id, + "svix-timestamp": timestamp, + "svix-signature": sig, + } + ) + assert adapter._validate_signature(req, body, secret) is False + + def test_validate_svix_signature_invalid_whsec_rejects(self): + """Malformed whsec_ secrets are rejected, not silently treated as raw secrets.""" + adapter = _make_adapter() + body = b'{"event_type":"message.received"}' + malformed_secret = "whsec_not-valid-base64!" + msg_id = "msg_123" + timestamp = str(int(time.time())) + raw_sig = _svix_signature( + body, malformed_secret.removeprefix("whsec_"), msg_id, timestamp + ) + req = _mock_request( + headers={ + "svix-id": msg_id, + "svix-timestamp": timestamp, + "svix-signature": raw_sig, + } + ) + assert adapter._validate_signature(req, body, malformed_secret) is False + + def test_validate_svix_signature_raw_secret_valid(self): + """Raw shared secrets are accepted for Svix-style senders without whsec_ secrets.""" + adapter = _make_adapter() + body = b'{"event_type":"message.received"}' + secret = "raw-agentmail-secret" + msg_id = "msg_123" + timestamp = str(int(time.time())) + sig = _svix_signature(body, secret, msg_id, timestamp) + req = _mock_request( + headers={ + "svix-id": msg_id, + "svix-timestamp": timestamp, + "svix-signature": sig, + } + ) + assert adapter._validate_signature(req, body, secret) is True + # =================================================================== # Prompt rendering @@ -304,6 +445,27 @@ class TestEventFilter: ) assert resp.status == 202 + @pytest.mark.asyncio + async def test_event_filter_accepts_payload_type_field(self): + """Svix-style payloads often use a top-level `type` event field.""" + routes = { + "svix": { + "secret": _INSECURE_NO_AUTH, + "events": ["message.received"], + "prompt": "got it", + } + } + adapter = _make_adapter(routes=routes) + adapter.handle_message = AsyncMock() + + app = _create_app(adapter) + async with TestClient(TestServer(app)) as cli: + resp = await cli.post( + "/webhooks/svix", + json={"type": "message.received"}, + ) + assert resp.status == 202 + # =================================================================== # HTTP handling @@ -432,6 +594,25 @@ class TestIdempotency: resp2 = await cli.post("/webhooks/idem", json={"x": 1}, headers=headers) assert resp2.status == 202 # re-accepted + @pytest.mark.asyncio + async def test_svix_id_used_as_delivery_id_for_deduplication(self): + """Svix retries reuse svix-id, so use it as the delivery ID when present.""" + routes = {"idem": {"secret": _INSECURE_NO_AUTH, "prompt": "test"}} + adapter = _make_adapter(routes=routes) + adapter.handle_message = AsyncMock() + + app = _create_app(adapter) + async with TestClient(TestServer(app)) as cli: + headers = {"svix-id": "msg_duplicate"} + resp1 = await cli.post("/webhooks/idem", json={"a": 1}, headers=headers) + assert resp1.status == 202 + + resp2 = await cli.post("/webhooks/idem", json={"a": 1}, headers=headers) + assert resp2.status == 200 + data = await resp2.json() + assert data["status"] == "duplicate" + assert data["delivery_id"] == "msg_duplicate" + # =================================================================== # Rate limiting