From 57729bee8f311853577b2dcbee247d431a038989 Mon Sep 17 00:00:00 2001 From: jr551 <2920328+jr551@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:44:30 +0100 Subject: [PATCH 1/2] feat(gateway): route-configurable webhook signature schemes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Providers that bind a timestamp into the signed message — ElevenLabs, Stripe, Slack — all use the same construction the adapter already implements for its generic V2 scheme, and differ only in how the pieces are packaged into headers. None of them validate today: the built-in probing looks for a fixed set of header names, so a correct secret still yields "Invalid signature". Rather than adding a branch per vendor, a route can describe the packaging in config and reuse the existing, already-tested primitive: routes: my-provider: secret: "..." signature: header: "ElevenLabs-Signature" # t=,v0= signature_part: "v0" timestamp_part: "t" template: "{timestamp}.{body}" tolerance_seconds: 1800 Stripe and Slack are the same block with different labels; a provider that signs the body alone under its own header name is `template: "{body}"`. No provider names appear in the code. Setting `signature` is exclusive and fail-closed: built-in probing is skipped for that route, so a sender cannot downgrade it to a weaker scheme it happens to also carry headers for — including the legacy, replay-vulnerable V1 fallback. Restating the built-in V2 scheme as a `signature` block is how a route opts out of V1. Routes without the block are untouched. --- gateway/platforms/webhook.py | 393 ++++++++++++- .../test_webhook_configured_signature.py | 555 ++++++++++++++++++ website/docs/user-guide/messaging/webhooks.md | 79 +++ 3 files changed, 1023 insertions(+), 4 deletions(-) create mode 100644 tests/gateway/test_webhook_configured_signature.py diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index 99eefcd634af..3bc62c75cfff 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -27,6 +27,10 @@ binds a timestamp into the signed data for replay protection; the legacy body-only V1 (X-Webhook-Signature) is deprecated but still accepted with a warning, since it has no replay protection + - A route may instead declare a `signature` block describing the header + layout its provider uses (see "Route-configurable signature schemes" + below). That is exclusive: built-in header probing, including the + legacy V1 fallback, is skipped for that route - Set secret to "INSECURE_NO_AUTH" to skip validation (testing only) """ @@ -169,6 +173,192 @@ def _hmac_str_equal(provided: str, expected: str) -> bool: return hmac.compare_digest(provided.encode(), expected.encode()) +# --------------------------------------------------------------------------- +# Route-configurable signature schemes +# --------------------------------------------------------------------------- +# +# A large family of providers authenticates webhooks in exactly the same way — +# an HMAC over a timestamp-bound message — and differs only in how the pieces +# are packaged into headers: +# +# ElevenLabs ElevenLabs-Signature: t=,v0= over "." +# Stripe Stripe-Signature: t=,v1= over "." +# Slack X-Slack-Signature: v0= over "v0::" +# X-Slack-Request-Timestamp: +# +# The cryptography there is byte-identical to the generic V2 scheme this +# adapter already implements; only the packaging differs. Growing one +# hard-coded branch per vendor does not scale — each new provider needs a code +# change and a release — so a route can instead describe the packaging in +# config and reuse the same verified primitive: +# +# routes: +# my-provider: +# secret: "..." +# signature: +# header: "ElevenLabs-Signature" +# signature_part: "v0" # label inside a "k=v,k=v" header +# timestamp_part: "t" # or timestamp_header: "X-Some-Timestamp" +# template: "{timestamp}.{body}" +# algorithm: "sha256" # sha1 | sha256 | sha512 +# encoding: "hex" # hex | base64 +# tolerance_seconds: 1800 +# +# Configuring `signature` is EXCLUSIVE: the built-in GitHub/GitLab/Svix/ +# Linear/generic probing is skipped entirely for that route. A route pinned to +# one provider therefore cannot be authenticated through a different — possibly +# weaker — scheme, which also closes the legacy V1 downgrade path for it. + +_SIGNATURE_ALGORITHMS = { + # HMAC remains a sound authenticator over SHA-1, and some providers still + # offer nothing stronger; the docs steer new integrations to sha256. + "sha1": hashlib.sha1, + "sha256": hashlib.sha256, + "sha512": hashlib.sha512, +} +_SIGNATURE_ENCODINGS = ("hex", "base64") +_SIGNATURE_TEMPLATE_TOKENS = ("body", "timestamp") +_DEFAULT_SIGNATURE_TEMPLATE = "{timestamp}.{body}" +_DEFAULT_SIGNATURE_TOLERANCE_SECONDS = 300 +_TEMPLATE_TOKEN_RE = re.compile(r"\{([^{}]*)\}") + + +def _parse_signature_spec(route_name: str, raw: Any) -> Dict[str, Any]: + """Normalise a route's ``signature`` block, or raise ``ValueError``. + + Every rejection here is a configuration bug the operator can fix, so the + message names the route and the offending key. ``connect()`` lets the error + propagate (a typo should stop the gateway, not silently 401 every delivery + until someone reads the logs); the request path catches it and fails + closed, because dynamically-registered routes never pass through + ``connect()``. + """ + prefix = f"[webhook] Route '{route_name}' signature config" + if not isinstance(raw, dict): + raise ValueError(f"{prefix} must be a mapping, got {type(raw).__name__}") + + def _opt_str(key: str) -> str: + value = raw.get(key, "") + if value in (None, ""): + return "" + if not isinstance(value, str): + raise ValueError(f"{prefix} key '{key}' must be a string") + return value.strip() + + header = _opt_str("header") + if not header: + raise ValueError(f"{prefix} requires a non-empty 'header'") + + algorithm_name = (_opt_str("algorithm") or "sha256").lower() + if algorithm_name not in _SIGNATURE_ALGORITHMS: + raise ValueError( + f"{prefix} has unknown algorithm '{algorithm_name}' " + f"(expected one of {', '.join(sorted(_SIGNATURE_ALGORITHMS))})" + ) + + encoding = (_opt_str("encoding") or "hex").lower() + if encoding not in _SIGNATURE_ENCODINGS: + raise ValueError( + f"{prefix} has unknown encoding '{encoding}' " + f"(expected one of {', '.join(_SIGNATURE_ENCODINGS)})" + ) + + template = raw.get("template", _DEFAULT_SIGNATURE_TEMPLATE) + if not isinstance(template, str) or not template: + raise ValueError(f"{prefix} key 'template' must be a non-empty string") + tokens = set(_TEMPLATE_TOKEN_RE.findall(template)) + unknown = sorted(tokens - set(_SIGNATURE_TEMPLATE_TOKENS)) + if unknown: + raise ValueError( + f"{prefix} template uses unknown placeholder(s) " + f"{', '.join('{%s}' % t for t in unknown)} " + f"(expected only {', '.join('{%s}' % t for t in _SIGNATURE_TEMPLATE_TOKENS)})" + ) + if "body" not in tokens: + raise ValueError( + f"{prefix} template must contain '{{body}}' — a signature that " + f"does not cover the payload authenticates nothing about it" + ) + uses_timestamp = "timestamp" in tokens + + timestamp_part = _opt_str("timestamp_part") + timestamp_header = _opt_str("timestamp_header") + if timestamp_part and timestamp_header: + raise ValueError( + f"{prefix} sets both 'timestamp_part' and 'timestamp_header'; " + f"the timestamp comes from exactly one place" + ) + if uses_timestamp and not (timestamp_part or timestamp_header): + raise ValueError( + f"{prefix} template uses '{{timestamp}}' but neither " + f"'timestamp_part' nor 'timestamp_header' says where to read it" + ) + if (timestamp_part or timestamp_header) and not uses_timestamp: + raise ValueError( + f"{prefix} reads a timestamp but the template never uses " + f"'{{timestamp}}', so it would not be authenticated" + ) + + tolerance = raw.get("tolerance_seconds", _DEFAULT_SIGNATURE_TOLERANCE_SECONDS) + if isinstance(tolerance, bool) or not isinstance(tolerance, int): + raise ValueError(f"{prefix} key 'tolerance_seconds' must be an integer") + if tolerance <= 0: + raise ValueError(f"{prefix} key 'tolerance_seconds' must be positive") + + return { + "header": header, + "signature_part": _opt_str("signature_part"), + "signature_prefix": raw.get("signature_prefix") or "", + "timestamp_part": timestamp_part, + "timestamp_header": timestamp_header, + "template": template, + "uses_timestamp": uses_timestamp, + "algorithm": _SIGNATURE_ALGORITHMS[algorithm_name], + "algorithm_name": algorithm_name, + "encoding": encoding, + "tolerance_seconds": tolerance, + } + + +def _split_signature_header(value: str) -> Dict[str, List[str]]: + """Parse a ``k=v,k=v`` signature header into label → list of values. + + Values collect into a list because providers emit a label more than once + during secret rotation (Stripe sends several ``v1=`` entries while the old + and new secrets are both live), and accepting any of them is what makes + rotation non-breaking. Chunks without ``=`` are ignored rather than fatal: + a hostile sender can pad the header with junk, and the parts that matter + are looked up by name, never by position. + """ + parsed: Dict[str, List[str]] = {} + for chunk in value.split(","): + label, sep, part = chunk.partition("=") + if not sep: + continue + parsed.setdefault(label.strip(), []).append(part.strip()) + return parsed + + +def _render_signed_message(template: str, timestamp: str, body: bytes) -> bytes: + """Build the exact byte string the sender signed. + + The body is spliced in as raw bytes rather than decoded to ``str``: a + signature covers the bytes on the wire, and round-tripping a payload that + is not valid UTF-8 (or that re-encodes differently) would turn a genuine + delivery into a mismatch. + + ``{body}`` is located in the *template* before the timestamp is + substituted, so an attacker-supplied timestamp cannot introduce a second + ``{body}`` marker and move where the payload is spliced. + """ + head, _, tail = template.partition("{body}") + return ( + head.replace("{timestamp}", timestamp).encode() + + body + + tail.replace("{timestamp}", timestamp).encode() + ) + + def check_webhook_requirements() -> bool: """Check if webhook adapter dependencies are available.""" return AIOHTTP_AVAILABLE @@ -200,6 +390,8 @@ def __init__(self, config: PlatformConfig): # Routes already warned about legacy V1 body-only signatures # (once-per-route so a busy sender doesn't spam the log). self._v1_signature_warned: set[str] = set() + # Routes whose configured signature template binds no timestamp + self._unbound_signature_warned: set[str] = set() # Delivery info keyed by session chat_id. # @@ -270,6 +462,11 @@ async def connect(self, *, is_reconnect: bool = False) -> bool: f"INSECURE_NO_AUTH is for local testing only. " f"Refusing to start to prevent accidental exposure." ) + + # Surface a malformed `signature` block at startup rather than as + # a 401 on every delivery that the operator has to guess at. + if route.get("signature") is not None: + _parse_signature_spec(name, route["signature"]) # deliver_only routes bypass the agent — the POST body becomes a # direct push notification via the configured delivery target. # Validate up-front so misconfiguration surfaces at startup rather @@ -715,7 +912,9 @@ async def _handle_webhook(self, request: "web.Request") -> "web.Response": status=403, ) if secret != _INSECURE_NO_AUTH: - if not self._validate_signature(request, raw_body, secret): + if not self._validate_signature( + request, raw_body, secret, route_config=route_config + ): logger.warning( "[webhook] Invalid signature for route %s", route_name ) @@ -1076,9 +1275,18 @@ async def _end_webhook_session( # ------------------------------------------------------------------ def _validate_signature( - self, request: "web.Request", body: bytes, secret: str + self, + request: "web.Request", + body: bytes, + secret: str, + route_config: Optional[dict] = None, ) -> bool: - """Validate webhook signature (GitHub, GitLab, Svix, Linear, generic HMAC-SHA256).""" + """Validate a webhook signature. + + A route carrying a ``signature`` block is validated against exactly + that scheme; otherwise the built-in providers are probed in turn + (GitHub, GitLab, Svix, Linear, generic HMAC-SHA256). + """ def _header(name: str) -> str: return ( request.headers.get(name, "") @@ -1086,6 +1294,38 @@ def _header(name: str) -> str: or request.headers.get(name.upper(), "") ) + route_name = request.match_info.get("route_name", "") + if route_config is None: + # Direct callers (and the test suite) may not pass the route in. + # Resolving it here keeps a configured scheme authoritative on + # every path instead of silently reverting to header probing. + route_config = self._routes.get(route_name) or {} + + # Route-configured scheme: exclusive and fail-closed. Built-in probing + # below is unreachable for such a route, so a sender cannot downgrade + # it to a weaker scheme it happens to also have headers for. + configured = route_config.get("signature") + if configured is not None: + try: + spec = _parse_signature_spec(route_name, configured) + except ValueError as exc: + # Reachable for dynamically-registered routes, which never go + # through connect()'s startup validation. + logger.error("%s", exc) + return False + return self._validate_configured_signature( + route_name=route_name, + spec=spec, + header_value=_header(spec["header"]), + timestamp_value=( + _header(spec["timestamp_header"]) + if spec["timestamp_header"] + else "" + ), + body=body, + secret=secret, + ) + # Svix / AgentMail: # svix-id: msg_... # svix-timestamp: unix seconds @@ -1183,7 +1423,6 @@ def _header(name: str) -> str: expected = hmac.new( secret.encode(), body, hashlib.sha256 ).hexdigest() - route_name = request.match_info.get("route_name", "") if route_name not in self._v1_signature_warned: self._v1_signature_warned.add(route_name) logger.warning( @@ -1202,6 +1441,152 @@ def _header(name: str) -> str: ) return False + def _validate_configured_signature( + self, + *, + route_name: str, + spec: Dict[str, Any], + header_value: str, + timestamp_value: str, + body: bytes, + secret: str, + ) -> bool: + """Validate a signature described by a route's ``signature`` block. + + Every path out of here is either an explicit ``True`` on a verified + HMAC or a logged ``False`` — there is no fall-through to another + scheme, because the block exists precisely to pin a route to one. + """ + if not header_value: + logger.warning( + "[webhook] Route '%s' expects signature header '%s' but the " + "request did not send it", + route_name, + spec["header"], + ) + return False + + needs_parts = bool(spec["signature_part"] or spec["timestamp_part"]) + parts = _split_signature_header(header_value) if needs_parts else {} + + if spec["signature_part"]: + candidates = parts.get(spec["signature_part"], []) + if not candidates: + logger.warning( + "[webhook] Route '%s' header '%s' has no '%s=' part", + route_name, + spec["header"], + spec["signature_part"], + ) + return False + else: + candidates = [header_value.strip()] + + signature_prefix = spec["signature_prefix"] + if signature_prefix: + candidates = [ + candidate[len(signature_prefix):] + for candidate in candidates + if candidate.startswith(signature_prefix) + ] + if not candidates: + logger.warning( + "[webhook] Route '%s' header '%s' is missing the required " + "'%s' prefix", + route_name, + spec["header"], + signature_prefix, + ) + return False + + candidates = [candidate for candidate in candidates if candidate] + if not candidates: + logger.warning( + "[webhook] Route '%s' header '%s' carried an empty signature", + route_name, + spec["header"], + ) + return False + + timestamp = "" + if spec["uses_timestamp"]: + if spec["timestamp_part"]: + # A hostile sender can repeat "t=" to pair one timestamp with + # another's signature. Only the first occurrence is honoured, + # and a conflicting second one is refused outright rather than + # letting the caller pick whichever value validates. + found = parts.get(spec["timestamp_part"], []) + if len(set(found)) > 1: + logger.warning( + "[webhook] Route '%s' header '%s' carried conflicting " + "'%s=' values", + route_name, + spec["header"], + spec["timestamp_part"], + ) + return False + timestamp = found[0] if found else "" + else: + timestamp = timestamp_value.strip() + + if not timestamp: + logger.warning( + "[webhook] Route '%s' signature requires a timestamp but " + "none was sent", + route_name, + ) + return False + try: + ts = int(timestamp) + except (TypeError, ValueError): + logger.warning( + "[webhook] Route '%s' sent a non-integer signature " + "timestamp", + route_name, + ) + return False + # Symmetric window: a timestamp far in the future is as much a + # sign of a forged or misconfigured sender as a stale one, and + # accepting it would hand an attacker an unbounded replay ticket. + if abs(int(time.time()) - ts) > spec["tolerance_seconds"]: + logger.warning( + "[webhook] Route '%s' signature timestamp outside the " + "%ds replay window", + route_name, + spec["tolerance_seconds"], + ) + return False + elif route_name not in self._unbound_signature_warned: + self._unbound_signature_warned.add(route_name) + logger.warning( + "[webhook] Route '%s' signs the body only (no '{timestamp}' " + "in the template), so a captured request replays " + "indefinitely. Bind a timestamp if the provider offers one.", + route_name, + ) + + message = _render_signed_message(spec["template"], timestamp, body) + digest = hmac.new(secret.encode(), message, spec["algorithm"]).digest() + if spec["encoding"] == "hex": + expected = digest.hex() + # Hex case carries no information, so normalising it before the + # compare keeps the timing guarantee while accepting providers + # that emit uppercase. + candidates = [candidate.lower() for candidate in candidates] + else: + expected = base64.b64encode(digest).decode() + + for candidate in candidates: + if _hmac_str_equal(candidate, expected): + return True + + logger.warning( + "[webhook] Route '%s' signature mismatch on header '%s'", + route_name, + spec["header"], + ) + return False + def _validate_svix_signature( self, body: bytes, diff --git a/tests/gateway/test_webhook_configured_signature.py b/tests/gateway/test_webhook_configured_signature.py new file mode 100644 index 000000000000..45f9773afcb3 --- /dev/null +++ b/tests/gateway/test_webhook_configured_signature.py @@ -0,0 +1,555 @@ +"""Route-configurable webhook signature schemes. + +The adapter can be told, per route, how a provider packages its HMAC into +headers instead of that packaging being hard-coded one vendor at a time. The +cases below pin the three shapes that cover most of the ecosystem — + +* ElevenLabs ``ElevenLabs-Signature: t=,v0=`` over ``.`` +* Stripe ``Stripe-Signature: t=,v1=`` over ``.`` +* Slack ``X-Slack-Signature: v0=`` plus a separate timestamp header, + over ``v0::`` + +— plus the failure modes that matter when the endpoint is reachable from the +public internet and the signature is the only thing in front of an agent: a +missing or empty header, a right-shaped wrong value, a tampered body, the wrong +secret, timestamps that are stale, in the future, or not numbers at all, +conflicting repeated header parts, and malformed configuration. Every one of +those must fail closed; none may fall through to a different scheme. +""" + +import hashlib +import hmac +import json +import time +from unittest.mock import MagicMock + +import pytest +from aiohttp import web +from aiohttp.test_utils import TestClient, TestServer + +from gateway.config import PlatformConfig +from gateway.platforms.webhook import ( + WebhookAdapter, + _parse_signature_spec, + _render_signed_message, + _split_signature_header, +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +ELEVENLABS_SIGNATURE = { + "header": "ElevenLabs-Signature", + "signature_part": "v0", + "timestamp_part": "t", + "template": "{timestamp}.{body}", + "tolerance_seconds": 1800, +} + +STRIPE_SIGNATURE = { + "header": "Stripe-Signature", + "signature_part": "v1", + "timestamp_part": "t", + "template": "{timestamp}.{body}", +} + +SLACK_SIGNATURE = { + "header": "X-Slack-Signature", + "signature_prefix": "v0=", + "timestamp_header": "X-Slack-Request-Timestamp", + "template": "v0:{timestamp}:{body}", +} + +BODY = json.dumps({"type": "post_call_transcription", "data": {"id": "c1"}}).encode() +SECRET = "wsec_0123456789abcdef0123456789abcdef" + + +def _make_adapter(routes, host="127.0.0.1", **extra_kw): + extra = {"host": host, "port": 0, "routes": routes} + extra.update(extra_kw) + return WebhookAdapter(PlatformConfig(enabled=True, extra=extra)) + + +def _create_app(adapter: WebhookAdapter) -> web.Application: + app = web.Application(client_max_size=adapter._max_body_bytes) + app.router.add_post("/webhooks/{route_name}", adapter._handle_webhook) + return app + + +def _mock_request(headers=None, route_name="r"): + req = MagicMock() + req.headers = headers or {} + req.match_info = {"route_name": route_name} + req.method = "POST" + return req + + +def _hex_hmac(secret: str, message: bytes, algorithm=hashlib.sha256) -> str: + return hmac.new(secret.encode(), message, algorithm).hexdigest() + + +def _elevenlabs_header(body: bytes, secret: str, timestamp=None) -> str: + """Reproduce elevenlabs-python's construct_event() header exactly.""" + ts = str(int(time.time())) if timestamp is None else str(timestamp) + return f"t={ts},v0={_hex_hmac(secret, f'{ts}.'.encode() + body)}" + + +def _stripe_header(body: bytes, secrets, timestamp=None) -> str: + ts = str(int(time.time())) if timestamp is None else str(timestamp) + sigs = ",".join( + f"v1={_hex_hmac(s, f'{ts}.'.encode() + body)}" for s in secrets + ) + return f"t={ts},{sigs}" + + +def _slack_headers(body: bytes, secret: str, timestamp=None) -> dict: + ts = str(int(time.time())) if timestamp is None else str(timestamp) + signed = f"v0:{ts}:".encode() + body + return { + "X-Slack-Request-Timestamp": ts, + "X-Slack-Signature": "v0=" + _hex_hmac(secret, signed), + } + + +def _route(signature, secret=SECRET, **extra): + route = {"secret": secret, "prompt": "{__raw__}", "deliver": "log"} + if signature is not None: + route["signature"] = signature + route.update(extra) + return route + + +def _validate(signature, body=BODY, headers=None, secret=SECRET, route_secret=None): + """Run one validation against a route configured with *signature*.""" + routes = {"r": _route(signature, secret=route_secret or secret)} + adapter = _make_adapter(routes) + return adapter._validate_signature( + _mock_request(headers), body, route_secret or secret + ) + + +# =================================================================== +# ElevenLabs — the combined "t=,v0=" header +# =================================================================== + + +class TestElevenLabsShape: + def test_valid_signature_accepted(self): + headers = {"ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET)} + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is True + + def test_secret_used_raw_including_wsec_prefix(self): + """ElevenLabs keys the HMAC with the literal secret, prefix included. + + Svix-style ``whsec_`` secrets are base64 payloads that must be decoded + first; ``wsec_`` ones are not. Stripping or decoding here would break + every real delivery, so pin the raw-key behaviour. + """ + headers = {"ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET)} + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is True + assert ( + _validate( + ELEVENLABS_SIGNATURE, + headers={ + "ElevenLabs-Signature": _elevenlabs_header( + BODY, SECRET.removeprefix("wsec_") + ) + }, + ) + is False + ) + + def test_tampered_body_rejected(self): + headers = {"ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET)} + assert _validate(ELEVENLABS_SIGNATURE, body=BODY + b" ", headers=headers) is False + + def test_wrong_secret_rejected(self): + headers = {"ElevenLabs-Signature": _elevenlabs_header(BODY, "wsec_other")} + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is False + + def test_expired_timestamp_rejected(self): + stale = int(time.time()) - 1801 + headers = {"ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET, stale)} + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is False + + def test_future_timestamp_rejected(self): + """A clock far ahead is as suspect as a stale one — and accepting it + would hand an attacker a replay ticket valid for as long as they like.""" + ahead = int(time.time()) + 1801 + headers = {"ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET, ahead)} + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is False + + def test_timestamp_inside_window_accepted(self): + recent = int(time.time()) - 1500 + headers = {"ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET, recent)} + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is True + + def test_default_tolerance_is_tighter_than_elevenlabs(self): + """Tolerance defaults to the adapter's existing 300s, not the + provider's 30 minutes; a route opts into a looser window explicitly.""" + spec = dict(ELEVENLABS_SIGNATURE) + spec.pop("tolerance_seconds") + old = int(time.time()) - 400 + headers = {"ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET, old)} + assert _validate(spec, headers=headers) is False + + @pytest.mark.parametrize( + "value", + [ + "", + "garbage", + "t=,v0=", + "v0=" + "a" * 64, # no timestamp part + f"t={int(time.time())}", # no signature part + f"t={int(time.time())},v0=", # empty signature + f"t=not-a-number,v0={'a' * 64}", + f"t={int(time.time())},v0={'a' * 64}", # right shape, wrong value + f"t={int(time.time())},v0=zz", + ], + ) + def test_malformed_or_forged_headers_rejected(self, value): + assert _validate(ELEVENLABS_SIGNATURE, headers={"ElevenLabs-Signature": value}) is False + + def test_missing_header_rejected(self): + assert _validate(ELEVENLABS_SIGNATURE, headers={}) is False + + def test_non_ascii_signature_rejected_not_raised(self): + """Hostile non-ASCII must fail closed, not raise out of the handler.""" + value = f"t={int(time.time())},v0=ské-not-hex" + assert _validate(ELEVENLABS_SIGNATURE, headers={"ElevenLabs-Signature": value}) is False + + def test_conflicting_timestamp_parts_rejected(self): + """Repeating ``t=`` must not let a caller pair a fresh timestamp with + a signature computed over a different one.""" + valid = _elevenlabs_header(BODY, SECRET) + stale = int(time.time()) - 100000 + assert ( + _validate( + ELEVENLABS_SIGNATURE, + headers={"ElevenLabs-Signature": f"t={stale},{valid}"}, + ) + is False + ) + + def test_repeated_identical_timestamp_tolerated(self): + valid = _elevenlabs_header(BODY, SECRET) + ts = valid.split(",")[0] + assert ( + _validate( + ELEVENLABS_SIGNATURE, + headers={"ElevenLabs-Signature": f"{valid},{ts}"}, + ) + is True + ) + + def test_padding_junk_in_header_ignored(self): + valid = _elevenlabs_header(BODY, SECRET) + assert ( + _validate( + ELEVENLABS_SIGNATURE, + headers={"ElevenLabs-Signature": f"junk,{valid},x=1,"}, + ) + is True + ) + + def test_uppercase_hex_accepted(self): + valid = _elevenlabs_header(BODY, SECRET) + ts, sig = valid.split(",v0=") + assert ( + _validate( + ELEVENLABS_SIGNATURE, + headers={"ElevenLabs-Signature": f"{ts},v0={sig.upper()}"}, + ) + is True + ) + + +# =================================================================== +# Other provider shapes — the point of making this configurable +# =================================================================== + + +class TestOtherProviderShapes: + def test_stripe_signature_accepted(self): + headers = {"Stripe-Signature": _stripe_header(BODY, [SECRET])} + assert _validate(STRIPE_SIGNATURE, headers=headers) is True + + def test_stripe_secret_rotation_accepts_either_signature(self): + """Stripe sends one ``v1=`` per live secret while a roll is in flight.""" + headers = {"Stripe-Signature": _stripe_header(BODY, ["old-secret", SECRET])} + assert _validate(STRIPE_SIGNATURE, headers=headers) is True + + def test_stripe_rotation_all_wrong_rejected(self): + headers = {"Stripe-Signature": _stripe_header(BODY, ["old-secret", "other"])} + assert _validate(STRIPE_SIGNATURE, headers=headers) is False + + def test_stripe_tampered_body_rejected(self): + headers = {"Stripe-Signature": _stripe_header(BODY, [SECRET])} + assert _validate(STRIPE_SIGNATURE, body=b"{}", headers=headers) is False + + def test_slack_signature_accepted(self): + """Slack keeps the timestamp in its own header and signs a + colon-delimited message — a different layout, same primitive.""" + assert _validate(SLACK_SIGNATURE, headers=_slack_headers(BODY, SECRET)) is True + + def test_slack_missing_timestamp_header_rejected(self): + headers = _slack_headers(BODY, SECRET) + headers.pop("X-Slack-Request-Timestamp") + assert _validate(SLACK_SIGNATURE, headers=headers) is False + + def test_slack_stale_timestamp_rejected(self): + headers = _slack_headers(BODY, SECRET, timestamp=int(time.time()) - 400) + assert _validate(SLACK_SIGNATURE, headers=headers) is False + + def test_slack_missing_v0_prefix_rejected(self): + headers = _slack_headers(BODY, SECRET) + headers["X-Slack-Signature"] = headers["X-Slack-Signature"].removeprefix("v0=") + assert _validate(SLACK_SIGNATURE, headers=headers) is False + + def test_slack_body_swapped_rejected(self): + assert _validate(SLACK_SIGNATURE, body=b"{}", headers=_slack_headers(BODY, SECRET)) is False + + def test_base64_encoding_supported(self): + import base64 as b64 + + spec = { + "header": "X-Provider-Signature", + "template": "{body}", + "encoding": "base64", + } + digest = hmac.new(SECRET.encode(), BODY, hashlib.sha256).digest() + headers = {"X-Provider-Signature": b64.b64encode(digest).decode()} + assert _validate(spec, headers=headers) is True + assert _validate(spec, headers={"X-Provider-Signature": "AAAA"}) is False + + def test_sha1_and_sha512_supported(self): + for name, algorithm in (("sha1", hashlib.sha1), ("sha512", hashlib.sha512)): + spec = { + "header": "X-Legacy-Signature", + "template": "{body}", + "algorithm": name, + } + good = {"X-Legacy-Signature": _hex_hmac(SECRET, BODY, algorithm)} + assert _validate(spec, headers=good) is True, name + bad = {"X-Legacy-Signature": _hex_hmac("nope", BODY, algorithm)} + assert _validate(spec, headers=bad) is False, name + + def test_existing_generic_v2_scheme_is_expressible(self): + """The built-in V2 scheme restated as config — which is how a route + pins itself to V2 and drops the legacy V1 fallback.""" + spec = { + "header": "X-Webhook-Signature-V2", + "timestamp_header": "X-Webhook-Timestamp", + "template": "{timestamp}.{body}", + } + ts = str(int(time.time())) + headers = { + "X-Webhook-Signature-V2": _hex_hmac(SECRET, f"{ts}.".encode() + BODY), + "X-Webhook-Timestamp": ts, + } + assert _validate(spec, headers=headers) is True + + +# =================================================================== +# Exclusivity — no downgrade to another scheme +# =================================================================== + + +class TestExclusivity: + def test_legacy_v1_signature_rejected_on_configured_route(self): + """The V1 body-only header has no replay protection. A route that has + declared its provider must not be authenticated through it.""" + headers = {"X-Webhook-Signature": _hex_hmac(SECRET, BODY)} + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is False + + def test_github_signature_rejected_on_configured_route(self): + headers = {"X-Hub-Signature-256": "sha256=" + _hex_hmac(SECRET, BODY)} + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is False + + def test_gitlab_token_rejected_on_configured_route(self): + assert _validate(ELEVENLABS_SIGNATURE, headers={"X-Gitlab-Token": SECRET}) is False + + def test_v2_headers_rejected_on_configured_route(self): + ts = str(int(time.time())) + headers = { + "X-Webhook-Signature-V2": _hex_hmac(SECRET, f"{ts}.".encode() + BODY), + "X-Webhook-Timestamp": ts, + } + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is False + + def test_valid_provider_header_still_wins_alongside_junk(self): + headers = { + "ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET), + "X-Hub-Signature-256": "sha256=deadbeef", + } + assert _validate(ELEVENLABS_SIGNATURE, headers=headers) is True + + def test_unconfigured_routes_keep_builtin_probing(self): + """Backwards compatibility: without a `signature` block nothing moves.""" + headers = {"X-Hub-Signature-256": "sha256=" + _hex_hmac(SECRET, BODY)} + assert _validate(None, headers=headers) is True + + +# =================================================================== +# Configuration validation +# =================================================================== + + +class TestSignatureConfigValidation: + @pytest.mark.parametrize( + "block", + [ + "not-a-mapping", + {}, # no header + {"header": " "}, # blank header + {"header": "H", "algorithm": "md5"}, # unsupported + {"header": "H", "encoding": "base32"}, # unsupported + {"header": "H", "template": "{timestamp}"}, # body not signed + {"header": "H", "template": "{nope}.{body}"}, # unknown token + {"header": "H", "template": "{body}", "timestamp_part": "t"}, + {"header": "H", "timestamp_part": "t", "timestamp_header": "X-T"}, + {"header": "H", "template": "{timestamp}.{body}"}, # no ts source + {"header": "H", "template": "{body}", "tolerance_seconds": 0}, + {"header": "H", "template": "{body}", "tolerance_seconds": "300"}, + {"header": "H", "template": "{body}", "tolerance_seconds": True}, + {"header": 5, "template": "{body}"}, + ], + ) + def test_invalid_blocks_rejected(self, block): + with pytest.raises(ValueError): + _parse_signature_spec("r", block) + + def test_defaults_applied(self): + spec = _parse_signature_spec("r", {"header": "H", "template": "{body}"}) + assert spec["algorithm_name"] == "sha256" + assert spec["encoding"] == "hex" + assert spec["tolerance_seconds"] == 300 + assert spec["uses_timestamp"] is False + + def test_elevenlabs_block_parses(self): + spec = _parse_signature_spec("r", ELEVENLABS_SIGNATURE) + assert spec["uses_timestamp"] is True + assert spec["tolerance_seconds"] == 1800 + assert spec["signature_part"] == "v0" + + @pytest.mark.asyncio + async def test_connect_refuses_malformed_block(self): + adapter = _make_adapter({"r": _route({"header": "H", "algorithm": "rot13"})}) + with pytest.raises(ValueError, match="unknown algorithm"): + await adapter.connect() + + @pytest.mark.asyncio + async def test_connect_accepts_valid_block(self): + adapter = _make_adapter({"r": _route(ELEVENLABS_SIGNATURE)}) + assert await adapter.connect() is True + await adapter.disconnect() + + def test_malformed_block_on_a_live_route_fails_closed(self): + """Dynamically-registered routes never see connect(); a bad block + there must reject requests, not raise or accept them.""" + routes = {"r": _route({"header": "H", "encoding": "base32"})} + adapter = _make_adapter(routes) + req = _mock_request({"H": "anything"}) + assert adapter._validate_signature(req, BODY, SECRET) is False + + +# =================================================================== +# Message construction +# =================================================================== + + +class TestSignedMessage: + def test_raw_body_bytes_are_spliced_not_decoded(self): + body = b"\xff\xfe not utf-8" + assert _render_signed_message("{timestamp}.{body}", "123", body) == b"123." + body + + def test_non_utf8_body_validates_end_to_end(self): + body = b'{"blob":"\xc3\x28"}' + ts = str(int(time.time())) + headers = { + "ElevenLabs-Signature": f"t={ts},v0={_hex_hmac(SECRET, f'{ts}.'.encode() + body)}" + } + assert _validate(ELEVENLABS_SIGNATURE, body=body, headers=headers) is True + + def test_timestamp_cannot_forge_a_body_marker(self): + """The body slot is located before the timestamp is substituted, so a + crafted timestamp cannot move where the payload is spliced.""" + assert ( + _render_signed_message("{timestamp}.{body}", "{body}", b"X") + == b"{body}.X" + ) + + def test_split_collects_repeated_labels(self): + assert _split_signature_header("t=1,v1=a,v1=b") == {"t": ["1"], "v1": ["a", "b"]} + + def test_split_ignores_valueless_chunks(self): + assert _split_signature_header("junk,t=1") == {"t": ["1"]} + + +# =================================================================== +# End to end over HTTP +# =================================================================== + + +class TestOverHttp: + @pytest.mark.asyncio + async def test_valid_delivery_accepted_tampered_rejected(self): + routes = {"elevenlabs-calls": _route(ELEVENLABS_SIGNATURE, deliver_only=False)} + adapter = _make_adapter(routes) + seen = [] + adapter.handle_message = lambda event: seen.append(event) + + async def _capture(event): + seen.append(event) + + adapter.handle_message = _capture + + async with TestClient(TestServer(_create_app(adapter))) as cli: + good = await cli.post( + "/webhooks/elevenlabs-calls", + data=BODY, + headers={ + "Content-Type": "application/json", + "ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET), + }, + ) + assert good.status == 202 + + tampered = BODY.replace(b"c1", b"c2") + bad = await cli.post( + "/webhooks/elevenlabs-calls", + data=tampered, + headers={ + "Content-Type": "application/json", + "ElevenLabs-Signature": _elevenlabs_header(BODY, SECRET), + }, + ) + assert bad.status == 401 + + missing = await cli.post( + "/webhooks/elevenlabs-calls", + data=BODY, + headers={"Content-Type": "application/json"}, + ) + assert missing.status == 401 + + assert len(seen) == 1 + + @pytest.mark.asyncio + async def test_oversized_body_rejected_before_validation(self): + routes = {"r": _route(ELEVENLABS_SIGNATURE)} + adapter = _make_adapter(routes, max_body_bytes=512) + big = b"x" * 4096 + + async with TestClient(TestServer(_create_app(adapter))) as cli: + resp = await cli.post( + "/webhooks/r", + data=big, + headers={ + "Content-Type": "application/json", + "ElevenLabs-Signature": _elevenlabs_header(big, SECRET), + }, + ) + assert resp.status == 413 diff --git a/website/docs/user-guide/messaging/webhooks.md b/website/docs/user-guide/messaging/webhooks.md index dd1148b0fac1..5390bf80c10e 100644 --- a/website/docs/user-guide/messaging/webhooks.md +++ b/website/docs/user-guide/messaging/webhooks.md @@ -80,6 +80,7 @@ Routes define how different webhook sources are handled. Each route is a named e |----------|----------|-------------| | `events` | No | List of event types to accept (e.g. `["pull_request"]`). If empty, all events are accepted. Event type is read from `X-GitHub-Event`, `X-GitLab-Event`, or `event_type` in the payload. | | `secret` | **Yes** | HMAC secret for signature validation. Falls back to the global `secret` if not set on the route. Set to `"INSECURE_NO_AUTH"` for testing only (skips validation). | +| `signature` | No | Describes a provider's signature header layout so it can be validated without provider-specific code — see [Custom signature schemes](#custom-signature-schemes). Setting it pins the route to that scheme exclusively. | | `profile` | No | Profile authorized to execute this route when `gateway.multiplex_profiles` is enabled. Omit it for a default-profile-only route; set a profile name (for example `coder`) to bind the route and its secret to `/p/coder/webhooks/`. | | `prompt` | No | Template string with dot-notation payload access (e.g. `{pull_request.title}`). If omitted, the full JSON payload is dumped into the prompt. Payload fields are untrusted — see [Authenticated does not mean trusted](#authenticated-does-not-mean-trusted). | | `filters` | No | Declarative payload filters evaluated after auth/body/event filtering and before agent or direct delivery work. Non-matches return `{"status":"ignored","reason":"filter"}` with HTTP 200. | @@ -503,6 +504,83 @@ The adapter validates incoming webhook signatures using the appropriate method f If a secret is configured but no recognized signature header is present, the request is rejected. +### Custom signature schemes {#custom-signature-schemes} + +Many providers use the same construction as the generic V2 scheme — an HMAC over a +timestamp-bound message — and differ only in how they package it into headers. +Rather than requiring a code change per provider, a route can describe that +packaging with a `signature` block: + +```yaml +routes: + elevenlabs-calls: + secret: "wsec_your_elevenlabs_webhook_secret" + signature: + header: "ElevenLabs-Signature" # ElevenLabs-Signature: t=,v0= + signature_part: "v0" + timestamp_part: "t" + template: "{timestamp}.{body}" + tolerance_seconds: 1800 # ElevenLabs allows 30 minutes + prompt: "Post-call webhook: {__raw__}" +``` + +| Key | Default | Description | +|-----|---------|-------------| +| `header` | — (required) | Header carrying the signature. | +| `signature_part` | _(none)_ | When the header is a `k=v,k=v` list, the label holding the digest (e.g. `v0`, `v1`). Omit when the whole header value is the signature. | +| `signature_prefix` | `""` | Literal prefix that must be present on the signature value and is stripped before comparison (e.g. `sha256=`, `v0=`). | +| `timestamp_part` | _(none)_ | Label inside the same header holding the Unix-seconds timestamp. | +| `timestamp_header` | _(none)_ | Separate header holding the timestamp. Mutually exclusive with `timestamp_part`. | +| `template` | `{timestamp}.{body}` | The exact message the provider signs. Must contain `{body}`; may contain `{timestamp}`. | +| `algorithm` | `sha256` | `sha1`, `sha256`, or `sha512`. | +| `encoding` | `hex` | `hex` or `base64`. | +| `tolerance_seconds` | `300` | Maximum clock skew, in either direction, between the signed timestamp and the server. | + +Other common providers: + +```yaml +# Stripe — Stripe-Signature: t=,v1= +signature: + header: "Stripe-Signature" + signature_part: "v1" + timestamp_part: "t" + +# Slack — X-Slack-Signature: v0=, timestamp in its own header +signature: + header: "X-Slack-Signature" + signature_prefix: "v0=" + timestamp_header: "X-Slack-Request-Timestamp" + template: "v0:{timestamp}:{body}" + +# A provider that signs the body only, under its own header name +signature: + header: "X-Gitea-Signature" + template: "{body}" +``` + +Notes: + +- **It is exclusive.** When `signature` is set, the built-in GitHub / GitLab / + Svix / Linear / generic probing is skipped for that route, and a request that + does not carry a valid signature in the configured header is rejected. A route + pinned to one provider therefore cannot be authenticated through a different, + possibly weaker, scheme — including the legacy V1 fallback. Restating the + built-in V2 scheme as a `signature` block (`header: X-Webhook-Signature-V2`, + `timestamp_header: X-Webhook-Timestamp`) is how you opt a route out of V1. +- **It fails closed.** A missing header, a missing or empty signature part, a + missing or non-numeric timestamp, a timestamp outside the tolerance in either + direction, conflicting repeated timestamp parts, or a malformed `signature` + block all reject the request. Nothing falls through to another scheme. +- **Rotation works.** A repeated signature label (Stripe emits one `v1=` per + live secret during a roll) is accepted if any of the values verifies. +- **Timestamps are required for replay protection.** A `template` without + `{timestamp}` signs the body alone, so a captured request replays + indefinitely; the gateway logs a warning once per route. Bind a timestamp + whenever the provider offers one. +- A malformed `signature` block on a route in `config.yaml` stops the gateway at + startup with an error naming the route and key, rather than silently + rejecting every delivery. + ### Secret is required Every route must have a secret — either set directly on the route or inherited from the global `secret`. Routes without a secret cause the adapter to fail at startup with an error. For development/testing only, you can set the secret to `"INSECURE_NO_AUTH"` to skip validation entirely. @@ -571,6 +649,7 @@ This is the same trust model that applies to everything the agent reads: web pag - Ensure the secret in your route config exactly matches the secret configured in the webhook source - For GitHub, the secret is HMAC-based — check `X-Hub-Signature-256` - For GitLab, the secret is a plain token match — check `X-Gitlab-Token` +- If the provider is not one of the built-ins, describe its header layout with a [`signature` block](#custom-signature-schemes) rather than disabling auth - Check gateway logs for `Invalid signature` warnings ### Event being ignored From 81d4cbecc189cf181ece453d6d6d90dfbf077dee Mon Sep 17 00:00:00 2001 From: jr551 <2920328+jr551@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:29:42 +0100 Subject: [PATCH 2/2] fix(gateway): normalise every signature option and pin template shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of 57729bee8f found two parser-contract defects. 1. `signature_prefix` was the one string option that bypassed `_opt_str()` (`raw.get("signature_prefix") or ""`). A truthy non-string passed `connect()` as valid, then reached `candidate.startswith(prefix)` in the verifier and raised `TypeError` — HTTP 500 on the request path, on the one boundary that is supposed to fail closed, and no better for a dynamically-registered route whose reparse also let it through. Every string option now goes through `_opt_str()`. Auditing the rest of the parser found no sibling: `template` and `tolerance_seconds` were already type-checked explicitly. 2. The template check only asserted the token set *contained* `body`, so `"{body}:{body}"` was accepted while `_render_signed_message()` substitutes a single partition point — rendering a literal `{body}` into the signed message. Exactly one `{body}` is now required. Rejecting rather than substituting every occurrence is the fail-closed reading, and it is the compatible direction: a later release can widen the accepted set without breaking anyone, whereas tightening this later would break configs that had silently "worked". `{timestamp}` is a plain string replacement, so it may repeat and every occurrence is substituted; that is now pinned too. Regressions added for both, over the whole class rather than the two reported keys: non-string values for every string option are rejected at parse time, rejected by `connect()` for a static route, and rejected with a logged `False` / HTTP 401 — never an exception — for a live route. All 13 fail against the previous head. --- gateway/platforms/webhook.py | 20 +- .../test_webhook_configured_signature.py | 173 ++++++++++++++++++ website/docs/user-guide/messaging/webhooks.md | 8 +- 3 files changed, 196 insertions(+), 5 deletions(-) diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index 3bc62c75cfff..c208f199940e 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -274,11 +274,23 @@ def _opt_str(key: str) -> str: f"{', '.join('{%s}' % t for t in unknown)} " f"(expected only {', '.join('{%s}' % t for t in _SIGNATURE_TEMPLATE_TOKENS)})" ) - if "body" not in tokens: + # Exactly one, not merely at least one: the renderer splices the raw body + # at a single point, so a repeated marker would leave a literal "{body}" in + # the signed message. Rejecting is the fail-closed reading, and it is the + # direction that stays compatible — a later release can accept more + # templates without breaking anyone, whereas tightening this later would + # break configs that had silently "worked". + body_markers = template.count("{body}") + if body_markers == 0: raise ValueError( f"{prefix} template must contain '{{body}}' — a signature that " f"does not cover the payload authenticates nothing about it" ) + if body_markers > 1: + raise ValueError( + f"{prefix} template contains '{{body}}' {body_markers} times; " + f"exactly one is required so the signed message is unambiguous" + ) uses_timestamp = "timestamp" in tokens timestamp_part = _opt_str("timestamp_part") @@ -308,7 +320,7 @@ def _opt_str(key: str) -> str: return { "header": header, "signature_part": _opt_str("signature_part"), - "signature_prefix": raw.get("signature_prefix") or "", + "signature_prefix": _opt_str("signature_prefix"), "timestamp_part": timestamp_part, "timestamp_header": timestamp_header, "template": template, @@ -349,7 +361,9 @@ def _render_signed_message(template: str, timestamp: str, body: bytes) -> bytes: ``{body}`` is located in the *template* before the timestamp is substituted, so an attacker-supplied timestamp cannot introduce a second - ``{body}`` marker and move where the payload is spliced. + ``{body}`` marker and move where the payload is spliced. The template is + validated to hold exactly one ``{body}``; ``{timestamp}`` may repeat, and + every occurrence is substituted. """ head, _, tail = template.partition("{body}") return ( diff --git a/tests/gateway/test_webhook_configured_signature.py b/tests/gateway/test_webhook_configured_signature.py index 45f9773afcb3..21f9ff50b66e 100644 --- a/tests/gateway/test_webhook_configured_signature.py +++ b/tests/gateway/test_webhook_configured_signature.py @@ -415,6 +415,16 @@ class TestSignatureConfigValidation: {"header": "H", "template": "{body}", "tolerance_seconds": "300"}, {"header": "H", "template": "{body}", "tolerance_seconds": True}, {"header": 5, "template": "{body}"}, + {"header": "H", "template": "{body}", "signature_prefix": 1}, + {"header": "H", "template": "{body}", "signature_prefix": ["v0="]}, + {"header": "H", "template": "{body}", "signature_part": 1}, + {"header": "H", "template": "{body}", "algorithm": 256}, + {"header": "H", "template": "{body}", "encoding": 16}, + {"header": "H", "template": "{timestamp}.{body}", "timestamp_part": 1}, + {"header": "H", "template": "{timestamp}.{body}", "timestamp_header": 1}, + {"header": "H", "template": 5}, + {"header": "H", "template": "{body}:{body}"}, + {"header": "H", "template": "{timestamp}.{body}{body}", "timestamp_part": "t"}, ], ) def test_invalid_blocks_rejected(self, block): @@ -553,3 +563,166 @@ async def test_oversized_body_rejected_before_validation(self): }, ) assert resp.status == 413 + + +# =================================================================== +# Review regressions — a malformed block must never raise +# +# Every string-valued key must be normalised at parse time. Before this was +# enforced, a truthy non-string `signature_prefix` passed startup validation +# and then reached `candidate.startswith(prefix)` in the verifier, raising +# TypeError — a 500 on the request path, on the one boundary that is supposed +# to fail closed. These pin the whole class, not just the reported key. +# =================================================================== + + +STRING_KEYS = [ + "header", + "signature_part", + "signature_prefix", + "timestamp_part", + "timestamp_header", + "template", +] + + +def _block_with(key, value): + """A block that is valid except for *key*.""" + block = {"header": "X-Sig", "template": "{timestamp}.{body}", "timestamp_part": "t"} + block[key] = value + return block + + +class TestMalformedBlockNeverRaises: + @pytest.mark.parametrize("key", STRING_KEYS) + @pytest.mark.parametrize("value", [1, 0.5, ["x"], {"x": 1}, object()]) + def test_non_string_values_rejected_at_parse(self, key, value): + with pytest.raises(ValueError): + _parse_signature_spec("r", _block_with(key, value)) + + @pytest.mark.asyncio + @pytest.mark.parametrize("key", STRING_KEYS) + async def test_connect_rejects_non_string(self, key): + """Static routes must fail at startup, not at request time.""" + adapter = _make_adapter({"r": _route(_block_with(key, 1))}) + with pytest.raises(ValueError): + await adapter.connect() + + @pytest.mark.parametrize("key", STRING_KEYS) + def test_live_route_returns_false_without_raising(self, key): + """A dynamically-registered route never reaches connect()'s check, so + the request path must reject it rather than raise out of the handler.""" + adapter = _make_adapter({}) + adapter._routes["dyn"] = _route(_block_with(key, 1)) + req = _mock_request({"X-Sig": "v0=" + "a" * 64}, route_name="dyn") + assert adapter._validate_signature(req, BODY, SECRET) is False + + def test_non_string_prefix_reaches_the_startswith_call_site(self): + """The reported defect: a truthy non-string prefix used to survive + parsing and blow up inside the verifier on `candidate.startswith()`.""" + block = _block_with("signature_prefix", 1) + with pytest.raises(ValueError): + _parse_signature_spec("r", block) + + adapter = _make_adapter({}) + adapter._routes["dyn"] = _route(block) + ts = str(int(time.time())) + header = f"t={ts},v0={_hex_hmac(SECRET, f'{ts}.'.encode() + BODY)}" + req = _mock_request({"X-Sig": header}, route_name="dyn") + assert adapter._validate_signature(req, BODY, SECRET) is False + + @pytest.mark.asyncio + @pytest.mark.parametrize("key", STRING_KEYS) + async def test_over_http_is_401_never_500(self, key): + """End to end: a malformed live route rejects with 401, and the + request never reaches agent dispatch.""" + adapter = _make_adapter({}) + adapter._routes["dyn"] = _route(_block_with(key, 1)) + seen = [] + + async def _capture(event): + seen.append(event) + + adapter.handle_message = _capture + + ts = str(int(time.time())) + async with TestClient(TestServer(_create_app(adapter))) as cli: + resp = await cli.post( + "/webhooks/dyn", + data=BODY, + headers={ + "Content-Type": "application/json", + "X-Sig": f"t={ts},v0={_hex_hmac(SECRET, f'{ts}.'.encode() + BODY)}", + }, + ) + assert resp.status == 401, f"{key}: got {resp.status}" + assert seen == [] + + +# =================================================================== +# Review regressions — accepted templates have deterministic semantics +# +# The renderer splices the raw body at a single point, so a repeated {body} +# used to leave a literal "{body}" in the signed message: "{body}:{body}" +# rendered `:{body}`. Rejecting at startup is the fail-closed +# reading and keeps the accepted set unambiguous. +# =================================================================== + + +class TestTemplateShape: + @pytest.mark.parametrize( + "template", + ["{body}{body}", "{body}:{body}", "{body}.{body}.{body}"], + ) + def test_repeated_body_marker_rejected(self, template): + with pytest.raises(ValueError, match="exactly one"): + _parse_signature_spec("r", {"header": "H", "template": template}) + + def test_missing_body_marker_rejected(self): + with pytest.raises(ValueError, match=r"must contain"): + _parse_signature_spec("r", {"header": "H", "template": "{timestamp}", + "timestamp_part": "t"}) + + def test_single_body_marker_accepted(self): + spec = _parse_signature_spec("r", {"header": "H", "template": "{body}"}) + assert spec["template"] == "{body}" + + @pytest.mark.asyncio + async def test_connect_rejects_repeated_body_marker(self): + adapter = _make_adapter({"r": _route({"header": "H", "template": "{body}{body}"})}) + with pytest.raises(ValueError, match="exactly one"): + await adapter.connect() + + def test_repeated_timestamp_marker_is_substituted_everywhere(self): + """{timestamp} may repeat — unlike {body} it is a plain string + replacement, so every occurrence is substituted and the result is + deterministic. Pinned so the two markers can't silently diverge.""" + assert ( + _render_signed_message("{timestamp}:{timestamp}.{body}", "42", b"X") + == b"42:42.X" + ) + + def test_repeated_timestamp_validates_end_to_end(self): + spec = { + "header": "X-Sig", + "signature_part": "v0", + "timestamp_part": "t", + "template": "{timestamp}:{timestamp}.{body}", + } + ts = str(int(time.time())) + signed = f"{ts}:{ts}.".encode() + BODY + headers = {"X-Sig": f"t={ts},v0={_hex_hmac(SECRET, signed)}"} + assert _validate(spec, headers=headers) is True + + def test_no_literal_marker_survives_into_the_signed_message(self): + """The bug this closes: any accepted template must render without a + leftover placeholder.""" + for template in ("{body}", "{timestamp}.{body}", "v0:{timestamp}:{body}"): + spec = _parse_signature_spec("r", dict( + {"header": "H", "template": template}, + **({"timestamp_part": "t"} if "{timestamp}" in template else {}), + )) + rendered = _render_signed_message(spec["template"], "123", b"PAYLOAD") + assert b"{body}" not in rendered + assert b"{timestamp}" not in rendered + assert b"PAYLOAD" in rendered diff --git a/website/docs/user-guide/messaging/webhooks.md b/website/docs/user-guide/messaging/webhooks.md index 5390bf80c10e..542afd6f5acd 100644 --- a/website/docs/user-guide/messaging/webhooks.md +++ b/website/docs/user-guide/messaging/webhooks.md @@ -531,7 +531,7 @@ routes: | `signature_prefix` | `""` | Literal prefix that must be present on the signature value and is stripped before comparison (e.g. `sha256=`, `v0=`). | | `timestamp_part` | _(none)_ | Label inside the same header holding the Unix-seconds timestamp. | | `timestamp_header` | _(none)_ | Separate header holding the timestamp. Mutually exclusive with `timestamp_part`. | -| `template` | `{timestamp}.{body}` | The exact message the provider signs. Must contain `{body}`; may contain `{timestamp}`. | +| `template` | `{timestamp}.{body}` | The exact message the provider signs. Must contain `{body}` **exactly once**; `{timestamp}` is optional and may repeat. | | `algorithm` | `sha256` | `sha1`, `sha256`, or `sha512`. | | `encoding` | `hex` | `hex` or `base64`. | | `tolerance_seconds` | `300` | Maximum clock skew, in either direction, between the signed timestamp and the server. | @@ -579,7 +579,11 @@ Notes: whenever the provider offers one. - A malformed `signature` block on a route in `config.yaml` stops the gateway at startup with an error naming the route and key, rather than silently - rejecting every delivery. + rejecting every delivery. A route registered dynamically never reaches that + startup check, so the same validation runs on the request path and rejects + with `401` — it never raises. +- Every string-valued key is type-checked. `template` must contain `{body}` + exactly once, so the signed message is never ambiguous. ### Secret is required