Repository navigation
fix: Register webhook ingress and add raw payload utilities - #8
Conversation
The model hands every incoming webhook a URL at /webhooks/ingress/{token}
and the package never routed it, so the address answered 404 unless the
application registered it by hand. It cannot sit behind the authenticated
stack either, because the sender is a third party holding nothing but the
token.
Each delivery stored only the parsed body, so the bytes a signature was
computed over were gone. Nothing could re-check that signature afterwards,
and anything the event was forwarded to would receive a re-encoding that
no longer matched. Senders that sign their payloads are the common case,
not the exception.
Dispatch also asks for one owner's active outgoing hooks on every event it
sends, and direction and is_active were indexed separately. Each holds two
values, so neither narrowed anything. With a thousand hooks on one owner,
twenty of them outgoing, it read 212 rows to find them. It now reads 21.
|
Coverage Report |
Keeping the parsed copy alongside the raw one was the same data twice. The parsed form is derived on read instead, so there is one column and nothing is duplicated. It cannot go the other way round. Decoding and re-encoding does not reproduce what was sent: spacing, unicode escaping, slashes, float formatting and large integers all shift, and any of those breaks a signature computed over the original.
![sourceant[bot]](https://avatars.githubusercontent.com/in/1161781?s=60&v=4){kind=small}
| return $json; | ||
| } | ||
|
|
||
| // Some senders post a form rather than a document. |
There was a problem hiding this comment.
parse_str() yields a non-empty array for any non-empty string, so decoded() returns a bogus array (e.g. ['hello_world' => '']) for plain-text or XML bodies instead of null. Guard the fallback so only actual form bodies are parsed.
| // Some senders post a form rather than a document. | |
| // Some senders post a form rather than a document. | |
| if (! str_contains($raw, '=')) { | |
| return null; | |
| } | |
| parse_str($raw, $form); | |
| return $form === [] ? null : $form; |
Code Review SummaryIncoming webhook ingress is now registered as a separate route group under 🚀 Key Improvements
|
nfebe
changed the title
There was a problem hiding this comment.
src/Http/Controllers/WebhookController.php
$request->all() only holds form/JSON input, so senders posting text/plain, XML or binary bodies now store an empty payload while still getting a 202, and their exact bytes can no longer be verified. Keep a raw fallback (e.g. RawPayload::encode($request->getContent())) when parsed input is empty, or such deliveries are silently unrecorded.
src/Http/Controllers/WebhookController.php
$request->all() is empty for content types Laravel does not parse (text/plain, application/xml, application/octet-stream), so those events record an empty payload and the delivered body is discarded, leaving no bytes for signature verification. Fall back to storing the raw content via RawPayload::encode($request->getContent()) when the parsed input is empty.
src/Http/Controllers/WebhookController.php
The default ingress now stores $request->all(), which is empty for bodies that are neither JSON nor form-encoded (text/plain, text/xml, application/octet-stream, or a missing content type). Such deliveries still return 202 but are persisted with no body, so the sender's payload cannot be replayed or signature-checked. Preserve the raw content for these senders, or document that they are unsupported.
1 participant
Register incoming webhook URLs separately from authenticated management routes. Keep existing parsed payloads and their JSON storage unchanged. Add optional raw-body encoding utilities for applications that need exact bytes for signature checks. Add indexes for outgoing webhook selection by owner.