Skip to content

fix: Separate an unreadable webhook payload from a forged one - #469

Merged
razor-x merged 1 commit into
betafrom
claude/php-audit-uoa7nb-m5-webhook-payload-error
Aug 19, 2026
Merged

fix: Separate an unreadable webhook payload from a forged one#469
razor-x merged 1 commit into
betafrom
claude/php-audit-uoa7nb-m5-webhook-payload-error

Conversation

@razor-x

@razor-x razor-x commented Aug 17, 2026

Copy link
Copy Markdown
Member

Fixes finding M5 from the cross-SDK audit.

Renamed to InvalidWebhookPayloadError per review, and rebased onto beta at 4ee0b79 (4.0.0-beta.16).

The problem

SeamWebhook::verify raised Svix's WebhookVerificationException for a payload whose signature had already matched:

$this->webhook->verify($payload, $normalized_headers);   // signature is good

$event = Event::from_json(json_decode($payload));

if ($event === null) {
    throw new WebhookVerificationException(
        "The verified webhook payload did not contain an event",
    );
}

The message admits verification succeeded, but the exception type says otherwise, and the README maps that type to HTTP 400. So an unreadable delivery was answered with an error and Svix redelivered it across its full backoff schedule — a payload that will never parse, retried for hours — while whoever read the logs saw a verification failure and went looking for a forgery.

A correctly signed payload that parsed but wasn't an event was worse: Event::from_json returned an all-null Event and verify handed it back with no signal at all.

Neither path had any test coverage.

The fix

A new Seam\InvalidWebhookPayloadError implements SeamException for "verified, but the body is not a readable event". Signature failures keep raising WebhookVerificationException.

The two cases now warrant different responses, and the README shows both:

} catch (Svix\Exception\WebhookVerificationException $error) {
    http_response_code(401);
} catch (Seam\InvalidWebhookPayloadError $error) {
    http_response_code(204);
}

Also in this PR:

  • Catch the JSON parse failure explicitly via json_last_error() rather than inferring it from a null event, so malformed JSON is distinguishable from a well-formed non-event.
  • Treat a signed payload with no event_id as unreadable instead of returning an all-null Event.
  • Cast header names before lowercasing: an all-digit header name arrives as an int key and would TypeError in strtolower.

This also closes a hole in SeamException's stated promise that one catch block covers everything the SDK raises — until now the webhook path could only throw an external Svix type.

Tests

The suite had 4 tests covering the happy path and forgery. Now 13. Every new unreadable-payload case fails against beta in one of the two ways the audit described — three raise WebhookVerificationException (parse failure read as forgery), two return an all-null Event with no exception:

{not json, null, `` (empty), [1, 2], `{"hello":"world"}`

Plus the expired-timestamp and missing-header cases the suite never had.

Full suite: 237 tests green, psalm clean, npm run lint clean.

Not in this PR

The audit also flags the svix constraint (^1.40, locked at v1.99.1) as wide for a security-critical dependency, and the hard-coded 5-minute tolerance as unconfigurable. Both are dependency-policy calls rather than defects, so I left them alone — happy to tighten the constraint if you want it.


Generated by Claude Code

SeamWebhook::verify raised Svix's WebhookVerificationException for a
payload whose signature had already matched but whose body would not
parse. The README maps that exception to a 400, so an unreadable
delivery was answered with an error and Svix redelivered it on its full
backoff schedule, while whoever read the logs saw a verification failure
and went looking for a forgery.

Raise InvalidWebhookPayloadError instead, a new SeamException, so the
two cases can be answered differently: a signature that does not match
may be forged, and a body that does not parse is genuinely from Seam and
will not become readable however many times it arrives.

Catch the parse failure explicitly rather than inferring it from a null
event, and treat a correctly signed payload that is not an event the
same way. It used to be returned as an Event with every field null, with
nothing to tell the caller.

Cast header names before lowercasing them, since an all-digit name
arrives as an int key and would raise a TypeError.

The parse path had no coverage at all. Add cases for malformed JSON, a
non-object body, an empty body, and a signed non-event, along with the
expired-timestamp and missing-header cases the suite was missing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HH3wdHh4Y6Wjyc5uHwk5iG
@razor-x
razor-x force-pushed the claude/php-audit-uoa7nb-m5-webhook-payload-error branch from d5f1cdf to 43a0c6a Compare August 19, 2026 22:20
@razor-x
razor-x merged commit b24d042 into beta Aug 19, 2026
15 checks passed
@razor-x
razor-x deleted the claude/php-audit-uoa7nb-m5-webhook-payload-error branch August 19, 2026 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants