From e40192118ee1165133197a8f0712598c7fddb70a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 6 Sep 2026 02:35:37 -0700 Subject: [PATCH] fix(webhook): only webhook-signature commits a delivery to the Standard Webhooks path GitLab (lib/gitlab/web_hooks.rb, app/services/web_hook_service.rb) sends webhook-id and webhook-timestamp on EVERY delivery and adds webhook-signature only when a signing token is configured. Selecting the HMAC path on any webhook-* header would 401 every legacy X-Gitlab-Token install the moment it upgrades to GitLab 19, so only the signature header selects the path; id and timestamp then travel with it and the validator still fails closed when either is missing. svix-* keeps its existing any-header selection. Tests trimmed to the invariant bar: one parameterized contract (whsec_ and raw secrets accept; wrong secret, tampered body and stale timestamp reject) plus the GitLab legacy-token coexistence contract. evals/webhook_auth/standard_webhooks_ab.py drives a real aiohttp WebhookAdapter on a dedicated loopback port with real signed requests for before/after evidence. Related: #47849 (HwangJohn, cherry-picked here), #92024 (earlier salvage of #47849), #102080 and #103167 (same alias fix, same target). --- evals/webhook_auth/standard_webhooks_ab.py | 99 ++++++++++++++++++++++ gateway/platforms/webhook.py | 7 +- tests/gateway/test_webhook_adapter.py | 30 +++++++ 3 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 evals/webhook_auth/standard_webhooks_ab.py diff --git a/evals/webhook_auth/standard_webhooks_ab.py b/evals/webhook_auth/standard_webhooks_ab.py new file mode 100644 index 0000000000..aa73bf98ce --- /dev/null +++ b/evals/webhook_auth/standard_webhooks_ab.py @@ -0,0 +1,99 @@ +"""Live A/B probe: Standard Webhooks (webhook-id/-timestamp/-signature) against the REAL webhook adapter. + +Starts a real ``WebhookAdapter`` (aiohttp, loopback, dedicated port) with one secret-configured +route and one ``whsec_`` route, then sends real HTTP POSTs signed exactly as the Standard +Webhooks spec (https://github.com/standard-webhooks/standard-webhooks) and GitLab's signing-token +docs describe: HMAC-SHA256 over ``{id}.{timestamp}.{body}``, ``v1,``. Cases cover the +positive path plus negative paths (wrong secret, tampered body, stale timestamp, partial headers, +wrong scheme) and the regressions that must not move (svix-*, GitLab X-Gitlab-Token, dual-token). + + HERMES_HOME=$(mktemp -d) python evals/gateway_status_render/standard_webhooks_ab.py [--port 18644] +""" + +from __future__ import annotations + +import argparse +import asyncio +import base64 +import hashlib +import hmac +import json +import os +import subprocess +import sys +import time +from unittest.mock import AsyncMock + +import aiohttp + + +def _git_head() -> str: + return subprocess.run(["git", "rev-parse", "--short", "HEAD"], capture_output=True, text=True, + stdin=subprocess.DEVNULL).stdout.strip() + + +def _sign(secret: str, msg_id: str, ts: str, body: bytes) -> str: + key = base64.b64decode(secret.removeprefix("whsec_")) if secret.startswith("whsec_") else secret.encode() + digest = hmac.new(key, f"{msg_id}.{ts}.".encode() + body, hashlib.sha256).digest() + return "v1," + base64.b64encode(digest).decode() + + +def _std_headers(secret: str, body: bytes, *, msg_id: str = "msg_1", ts: str | None = None, sign_with: str | None = None): + ts = ts or str(int(time.time())) + return {"webhook-id": msg_id, "webhook-timestamp": ts, "webhook-signature": _sign(sign_with or secret, msg_id, ts, body)} + + +async def main(port: int) -> None: + sys.path.insert(0, os.getcwd()) + from gateway.config import PlatformConfig + from gateway.platforms.webhook import WebhookAdapter + + raw_secret = "gitlab-legacy-token" + whsec = "whsec_" + base64.b64encode(b"0123456789abcdef0123456789abcdef").decode() + routes = { + "raw": {"secret": raw_secret, "prompt": "raw route"}, + "std": {"secret": whsec, "prompt": "std route"}, + } + adapter = WebhookAdapter(PlatformConfig(enabled=True, extra={"host": "127.0.0.1", "port": port, "routes": routes})) + adapter.handle_message = AsyncMock() + assert await adapter.connect(), "adapter did not bind" + body = b'{"event_type":"invoice.paid","n":1}' + stale = str(int(time.time()) - 600) + cases = [ + ("std_whsec_valid", "std", _std_headers(whsec, body), 202), + ("std_raw_secret_valid", "raw", _std_headers(raw_secret, body, msg_id="msg_2"), 202), + ("std_wrong_secret", "std", _std_headers(whsec, body, msg_id="msg_3", sign_with="whsec_" + base64.b64encode(b"x" * 32).decode()), 401), + ("std_tampered_body", "std", _std_headers(whsec, b'{"event_type":"invoice.paid","n":2}', msg_id="msg_4"), 401), + ("std_stale_timestamp_replay", "std", _std_headers(whsec, body, msg_id="msg_5", ts=stale), 401), + ("std_partial_headers_fail_closed", "std", {"webhook-id": "msg_6", "webhook-timestamp": str(int(time.time()))}, 401), + ("std_wrong_scheme_v1a", "std", {**_std_headers(whsec, body, msg_id="msg_7"), "webhook-signature": "v1a," + base64.b64encode(b"\x00" * 64).decode()}, 401), + ("std_replay_same_id", "std", _std_headers(whsec, body, msg_id="msg_1"), 200), # dedupe => 200 duplicate + ("svix_still_valid", "std", {"svix-id": "svx_1", "svix-timestamp": str(int(time.time())), "svix-signature": _sign(whsec, "svx_1", str(int(time.time())), body)}, 202), + ("gitlab_token_only_still_valid", "raw", {"X-Gitlab-Token": raw_secret}, 202), + # GitLab >= 19 sends webhook-id/webhook-timestamp on EVERY delivery and webhook-signature only when a + # signing token is configured (lib/gitlab/web_hooks.rb, app/services/web_hook_service.rb): a legacy + # secret-token install must keep working. + ("gitlab_token_with_unsigned_webhook_id_ts", "raw", {"X-Gitlab-Token": raw_secret, "webhook-id": "gl_9", "webhook-timestamp": str(int(time.time()))}, 202), + ("gitlab_dual_token_signed_with_other_key", "raw", {**_std_headers(raw_secret, body, msg_id="msg_8", sign_with="some-other-signing-token"), "X-Gitlab-Token": raw_secret}, None), + ("no_auth", "std", {}, 401), + ] + results: dict = {"head": _git_head(), "cases": []} + async with aiohttp.ClientSession() as http: + for name, route, headers, expect in cases: + # svix case: recompute with one shared ts so header and signature agree + if name == "svix_still_valid": + ts = str(int(time.time())) + headers = {"svix-id": "svx_1", "svix-timestamp": ts, "svix-signature": _sign(whsec, "svx_1", ts, body)} + async with http.post(f"http://127.0.0.1:{port}/webhooks/{route}", data=body, headers={**headers, "Content-Type": "application/json"}) as resp: + text = await resp.text() + results["cases"].append({"case": name, "route": route, "status": resp.status, "expected": expect, + "ok": (expect is None or resp.status == expect), "body": text[:120]}) + await adapter.disconnect() + results["mismatches"] = [c["case"] for c in results["cases"] if not c["ok"]] + print(json.dumps(results, indent=2)) + + +if __name__ == "__main__": + ap = argparse.ArgumentParser() + ap.add_argument("--port", type=int, default=18644) + asyncio.run(main(ap.parse_args().port)) diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index 7eb63cd505..1e7ca78b1c 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -624,9 +624,12 @@ class WebhookAdapter(BasePlatformAdapter): return headers.get(name, "") or headers.get(name.lower(), "") or headers.get(name.upper(), "") # Svix / AgentMail: signed content is "{id}.{timestamp}.{raw_body}". Standard Webhooks - # (webhook-*; GitLab signing tokens) is the same scheme under other header names. + # (webhook-*; GitLab signing tokens) is the same scheme under other header names, but GitLab + # sends webhook-id/webhook-timestamp on EVERY delivery and webhook-signature only when a signing + # token is configured, so only the signature header commits to this path — a legacy + # X-Gitlab-Token install must keep validating below (#47451, #101837). svix = [_header(name) for name in ("svix-id", "svix-timestamp", "svix-signature")] - if not any(svix): + if not any(svix) and _header("webhook-signature"): svix = [_header(name) for name in ("webhook-id", "webhook-timestamp", "webhook-signature")] if any(svix): return _validate_svix_signature(body, secret, *svix) diff --git a/tests/gateway/test_webhook_adapter.py b/tests/gateway/test_webhook_adapter.py index 7ea8e8acde..7aee89d9b4 100644 --- a/tests/gateway/test_webhook_adapter.py +++ b/tests/gateway/test_webhook_adapter.py @@ -300,6 +300,36 @@ class TestValidateSignature: ) assert adapter._validate_signature(req, body, secret) is True + @pytest.mark.parametrize( + "secret, sign_with, body, received, stale, expected", + [ + ("whsec_" + base64.b64encode(b"0123456789abcdef").decode(), None, b'{"a":1}', b'{"a":1}', False, True), + ("raw-signing-secret", None, b'{"a":1}', b'{"a":1}', False, True), + ("real-secret", "attacker-secret", b'{"a":1}', b'{"a":1}', False, False), + ("real-secret", None, b'{"a":1}', b'{"a":2}', False, False), # tampered body + ("real-secret", None, b'{"a":1}', b'{"a":1}', True, False), # replayed stale timestamp + ], + ) + def test_standard_webhooks_headers_validate_like_svix(self, secret, sign_with, body, received, stale, expected): + """#47451/#101837: webhook-id/-timestamp/-signature is the same HMAC scheme as svix-*.""" + adapter = _make_adapter() + timestamp = str(int(time.time()) - (600 if stale else 0)) + sig = _svix_signature(body, sign_with or secret, "msg_std", timestamp) + req = _mock_request(headers={"webhook-id": "msg_std", "webhook-timestamp": timestamp, "webhook-signature": sig}) + assert adapter._validate_signature(req, received, secret) is expected + + def test_gitlab_secret_token_survives_unsigned_standard_webhooks_metadata(self): + """GitLab sends webhook-id/webhook-timestamp on every delivery and webhook-signature only when a + signing token is set; a legacy X-Gitlab-Token route must not be hijacked into the HMAC path.""" + adapter = _make_adapter() + req = _mock_request(headers={ + "X-Gitlab-Token": "legacy-token", "webhook-id": "gl_1", "webhook-timestamp": str(int(time.time())), + }) + assert adapter._validate_signature(req, b"{}", "legacy-token") is True + req_partial = _mock_request(headers={"webhook-id": "gl_2", "webhook-timestamp": str(int(time.time())), + "webhook-signature": "v1,AAAA"}) + assert adapter._validate_signature(req_partial, b"{}", "legacy-token") is False + # =================================================================== # Prompt rendering