Compare the webhook signature in constant time

[ckarnell]

construct_event compares the webhook signature with !=, which returns as soon as two bytes differ. That leaks how long a shared prefix is, so an attacker who can send requests and time the response can recover a valid signature a character at a time without ever knowing the secret. Both the sync and async copies do it.

This swaps in hmac.compare_digest at both sites. Two lines.

No behaviour changes, and that is worth saying plainly, because it means no test can show you the original problem. Accepting and rejecting the same signatures before and after is the whole point. So the test I added guards the other thing, which is the obvious wrong way to make this change.

hmac.compare_digest raises TypeError on str arguments that contain non-ASCII. The signature arrives in an attacker-controlled header, so compare_digest(signature, digest) on the raw strings turns a header of v0=café into an unhandled TypeError instead of a clean rejection. Encoding both sides to bytes first avoids it. test_construct_event_non_ascii_signature_is_rejected_not_crash fails against that naive version and passes here. It is a real control.

The second test covers AsyncElevenLabs.webhooks.construct_event, which carries its own copy of this logic and had no coverage at all.

Suite is 9 passed, up from 7.

Severity is low and I would not want it overstated. It needs an attacker who can send many webhook deliveries and time the responses across a network, which is noisy enough to make the attack awkward in practice. It is still the standard fix and it costs nothing.

I have not measured the timing difference against your service, so this rests on the usual argument for constant-time comparison.

---

Note

Medium Risk
Touches webhook signature verification on attacker-controlled headers; changes are standard hardening but affect security-sensitive request handling.

Overview
Hardens construct_event on both sync and async webhook clients so untrusted signature headers fail predictably and signatures are compared in constant time.

Signature verification now uses hmac.compare_digest on UTF-8 bytes instead of != on strings, closing a timing side channel and avoiding TypeError when the v0= value contains non-ASCII. Malformed t= values are caught and returned as BadRequestError (“Invalid timestamp…”) instead of an unhandled ValueError.

Tests cover non-ASCII signatures, non-numeric timestamps (sync and async), and basic async construct_event parity with sync.

^{Reviewed by Cursor Bugbot for commit ea069b3. Bugbot is set up for automated code reviews on this repo. Configure here.}

[maksym-t-didww]

Not a blocker — this is right, and encoding both sides before compare_digest is a detail I would have got wrong.

One adjacent case in the same function, if you would rather have it here than in a follow-up: the timestamp a few lines above is parsed unguarded, so an attacker-controlled t= that is not an integer raises ValueError instead of the BadRequestError the docstring promises:

>>> client.webhooks.construct_event(body, f"t=abc,v0={'0' * 64}", secret)
ValueError: invalid literal for int() with base 10: 'abc'

In a typical webhook handler that surfaces as a 500 rather than a 400. The TS SDK does not behave this way: Number("abc") is NaN, NaN < tolerance is false, so it falls through to the signature comparison and returns the normal error — this is a Python-only divergence.

         # Validate timestamp
-        req_timestamp = int(timestamp) * 1000
+        try:
+            req_timestamp = int(timestamp) * 1000
+        except ValueError:
+            raise BadRequestError(body=BadRequestErrorBody(error="Invalid timestamp in signature header"))
         tolerance = int(time.time() * 1000) - 30 * 60 * 1000

(twice, once per client, like your change)

and a test in the shape of the ones you added:

def test_construct_event_non_numeric_timestamp():
    """A non-numeric t= must be rejected as a bad request, not raise ValueError.

    The signature header is attacker-controlled, so a malformed timestamp has to
    surface as the documented BadRequestError rather than an unhandled exception.
    """
    client = ElevenLabs()
    webhook_secret = "test_secret"
    body = json.dumps({"event_type": "speech.completed", "id": "123456"})
    sig_header = f"t=not-a-number,v0={'0' * 64}"

    with pytest.raises(BadRequestError) as excinfo:
        client.webhooks.construct_event(body, sig_header, webhook_secret)

    assert "Invalid timestamp" in str(excinfo.value)

With the patch applied the webhook suite passes with the two new cases (sync and async) and no changes to the existing seven. Happy to open it separately once this lands if you would rather keep this PR to one thing.

[ckarnell]

Good catch, thanks. Folded it into this PR since it is the same function and a second one would just mean reviewing construct_event twice.

Confirmed your read both ways: t=abc raises ValueError where every other failure path raises BadRequestError, and the JS SDK does not diverge (Number('abc') is NaN, NaN < tolerance is false, so it falls through to the signature compare). The parse now rejects a non-numeric timestamp as a bad request, guarded in the sync and async copies with a test each. Credited you in the commit.