Closed (fixed)
Project:
Postmark Webhooks
Version:
1.0.0-alpha1
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
6 Sep 2026 at 02:15 UTC
Updated:
20 Sep 2026 at 06:15 UTC
Jump to comment: Most recent
Problem/Motivation
JSON decoding only checks is_array. An empty JSON list is acknowledged and stored as a blank event. Description supplied as an array throws TypeError. Other fields are unchecked against database lengths, and request content is read without an application size bound.
Evidence and scope
Reviewed 1.0.0-alpha1, source commit 02fd9d36af5237e712cecb7155d79725f7824880. Location: src/Controller/PostmarkWebhookController.php:61.
Two isolated kernel reproductions confirmed blank-row acceptance and Description TypeError. Requests used the valid test secret; no authentication bypass is claimed.
Proposed resolution
Define a small validated event input contract with event-specific required fields, scalar and length checks, recipient normalization, and a documented body limit. Allow harmless unknown fields for forward compatibility. Choose and document responses based on Postmark retry behavior; avoid leaking body or secret.
Acceptance criteria
Test objects versus lists, missing fields, nulls, nested values, oversized strings/body, Unicode and valid provider fixtures. Invalid input must not persist partial or blank rows. Add matching config validation constraints for imported suppression-window values.
Issue fork postmark_webhooks-3621122
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3621122-validate-webhook-payload
changes, plain diff MR !2
Comments
Comment #3
jmcerdaReady for review in https://git.drupalcode.org/project/postmark_webhooks/-/merge_requests/2 . Depends on the event-identity foundation in merge request !1. Validation covers object shape, extracted field types and lengths, Unicode, large provider IDs, conflicting recipients, bounded reads and configuration import constraints. Both Drupal 10.3/PHP 8.3 and Drupal 11/PHP 8.4 passed 28 tests (114 assertions) plus the added import-subscriber test (1 assertion). Drupal/DrupalPractice coding standards pass; manual findings review is recorded in the merge request.
Comment #4
jmcerdaFollow-up review found an event-specific validation gap: a Bounce with a provider ID but no Type is accepted without suppression; a corrected delivery with the same ID is then acknowledged as a retry. Reproducing this against the current integration candidate and adding a regression before tightening the Bounce input contract. This belongs to the existing validation scope and intersects #3621119 and #3621120.
Comment #5
jmcerdaThe incomplete-Bounce regression failed against the prior integration candidate (HTTP 200 instead of 400). The fix now requires a nonempty Type without surrounding whitespace before an event identity is stored. The regression proves malformed input leaves no event or suppression, and a corrected HardBounce using the same provider ID establishes permanent suppression exactly once. Unknown well-formed bounce types remain log-only; other record types do not require Type.
Drupal 10.3/PHP 8.3 and Drupal 11/PHP 8.4 each pass 39 kernel tests and 226 assertions on PostgreSQL, locally and in integration CI. Drupal/DrupalPractice coding standards pass. The combined suite includes simultaneous independent receivers, unrelated database failure propagation, transaction rollback, retained alpha upgrades and suppression surviving event purge.
Fallback findings review is complete with no outstanding findings; the automated review service failed to run and is not counted as a passed review. Earlier review findings remain fixed. The validation and durable-state prerequisite remains under integration review, together with #3621121, #3621124 and #3621126. Nothing was merged or released in this follow-up. Previously stored incomplete bounces are not automatically repaired, and missing classifications cannot be reconstructed from local history.
Comment #13
jmcerdaIntegrated into 1.x at d3c5d94 after integration review and passing Drupal 10.3/PHP 8.3 and Drupal 11/PHP 8.4 PostgreSQL CI (39 tests, 226 assertions each). The merged branch was mirrored additively to the public source repository.
This includes input validation, the incomplete-Bounce retry fix, durable suppression (#3621121), occurrence-time handling (#3621124), and the shared policy boundary (#3621126). The changes are on the development branch only: no release tag or deployment has been made. Existing alpha installations must run database updates when upgrading; previously discarded history cannot be reconstructed.