diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index decc104b713b..5bbbb3ff43e9 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -125,6 +125,20 @@ def _is_loopback_host(host: Optional[str]) -> bool: return host.strip().lower() in _LOOPBACK_HOSTS +def _hmac_str_equal(provided: str, expected: str) -> bool: + """Timing-safe equality for two ``str`` values, tolerant of non-ASCII input. + + ``hmac.compare_digest`` raises ``TypeError`` when given a ``str`` that + contains non-ASCII characters. The ``provided`` value here is an + attacker-controlled signature/token header on a public, unauthenticated + webhook endpoint, so a single non-ASCII byte would otherwise raise out of + the request handler and return a 500 instead of rejecting the request. + Comparing as UTF-8 bytes keeps the constant-time guarantee while making a + hostile header fail closed with a clean rejection. + """ + return hmac.compare_digest(provided.encode(), expected.encode()) + + def check_webhook_requirements() -> bool: """Check if webhook adapter dependencies are available.""" return AIOHTTP_AVAILABLE @@ -969,12 +983,12 @@ class WebhookAdapter(BasePlatformAdapter): expected = "sha256=" + hmac.new( secret.encode(), body, hashlib.sha256 ).hexdigest() - return hmac.compare_digest(gh_sig, expected) + return _hmac_str_equal(gh_sig, expected) # GitLab: X-Gitlab-Token = gl_token = request.headers.get("X-Gitlab-Token", "") if gl_token: - return hmac.compare_digest(gl_token, secret) + return _hmac_str_equal(gl_token, secret) # Generic V2: X-Webhook-Signature-V2 = ."> # X-Webhook-Timestamp = (required for V2) @@ -1017,7 +1031,7 @@ class WebhookAdapter(BasePlatformAdapter): expected_v2 = hmac.new( secret.encode(), signed_content, hashlib.sha256 ).hexdigest() - return hmac.compare_digest(v2_sig, expected_v2) + return _hmac_str_equal(v2_sig, expected_v2) # Generic V1 (legacy): X-Webhook-Signature = # (deprecated — no replay protection, since the signature only @@ -1041,7 +1055,7 @@ class WebhookAdapter(BasePlatformAdapter): "'.').", route_name, ) - return hmac.compare_digest(generic_sig, expected) + return _hmac_str_equal(generic_sig, expected) # No recognised signature header but secret is configured → reject logger.debug( @@ -1095,7 +1109,7 @@ class WebhookAdapter(BasePlatformAdapter): version, signature = part.split(",", 1) except ValueError: continue - if version == "v1" and hmac.compare_digest(signature, expected): + if version == "v1" and _hmac_str_equal(signature, expected): return True return False diff --git a/tests/gateway/test_webhook_adapter.py b/tests/gateway/test_webhook_adapter.py index 10165d73c825..eb53b95ad4bc 100644 --- a/tests/gateway/test_webhook_adapter.py +++ b/tests/gateway/test_webhook_adapter.py @@ -166,6 +166,40 @@ class TestValidateSignature: req = _mock_request(headers={}) # no sig headers at all assert adapter._validate_signature(req, b"{}", "my-secret") is False + def test_non_ascii_signature_headers_reject_without_raising(self): + """The signature headers are attacker-controlled on a public, unauth + endpoint. A non-ASCII byte in one must be rejected (False), not crash + the handler: hmac.compare_digest raises TypeError on a non-ASCII str.""" + adapter = _make_adapter() + body = b'{"action": "opened"}' + secret = "webhook-secret-42" + hostile = "ské-not-a-valid-signature" + for header in ( + "X-Hub-Signature-256", + "X-Gitlab-Token", + "X-Webhook-Signature", + ): + req = _mock_request(headers={header: hostile}) + # Must return False, never raise. + assert adapter._validate_signature(req, body, secret) is False + + def test_non_ascii_generic_v2_signature_rejected(self): + """V2 branch (timestamp-bound) also rejects a non-ASCII signature.""" + adapter = _make_adapter() + req = _mock_request(headers={ + "X-Webhook-Signature-V2": "ské-bad", + "X-Webhook-Timestamp": str(int(time.time())), + }) + assert adapter._validate_signature(req, b"{}", "secret") is False + + def test_non_ascii_secret_still_validates_a_matching_token(self): + """A non-ASCII configured secret must still match its exact GitLab + token value byte for byte (bytes comparison keeps this working).""" + adapter = _make_adapter() + secret = "gl-tökén-välue" + req = _mock_request(headers={"X-Gitlab-Token": secret}) + assert adapter._validate_signature(req, b"{}", secret) is True + def test_validate_no_secret_allows_all(self): """When the secret is empty/falsy, the validator is never even called by the handler (secret check is 'if secret and secret != _INSECURE...').