MT-22022: Add webhook signature verification helper#66
Conversation
|
Warning Review limit reached
More reviews will be available in 29 minutes and 57 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds webhook signature verification for Mailtrap. It introduces a new ChangesWebhook Signature Verification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Rabsztok
force-pushed
the
MT-22022-webhook-signature-verification
branch
from
8194057 to
d5be921
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
examples/webhooks/verify_signature.php (1)
14-18: ⚡ Quick winReplace
assert()with explicit checks for deterministic example behaviorLines 14–18 use
assert(), which is disabled by default in production PHP (zend.assertions=0) and in various configurations, causing the example to silently pass even if verification fails. Executable examples should fail deterministically.Suggested replacement using explicit checks
-assert(WebhookSignature::verify($payload, $signature, $signingSecret) === true); +if (!WebhookSignature::verify($payload, $signature, $signingSecret)) { + fwrite(STDERR, "Expected valid signature to pass\n"); + exit(1); +} @@ -assert(WebhookSignature::verify($payload, 'not-hex', $signingSecret) === false); -assert(WebhookSignature::verify($payload, '', $signingSecret) === false); +if (WebhookSignature::verify($payload, 'not-hex', $signingSecret)) { + fwrite(STDERR, "Expected malformed signature to fail\n"); + exit(1); +} +if (WebhookSignature::verify($payload, '', $signingSecret)) { + fwrite(STDERR, "Expected empty signature to fail\n"); + exit(1); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/webhooks/verify_signature.php` around lines 14 - 18, Replace the usage of assert() around WebhookSignature::verify($payload, $signature, $signingSecret) with explicit deterministic checks: call WebhookSignature::verify for the valid case and for the bad inputs ($signature = 'not-hex' and ''), then if the results are not the expected booleans throw an exception or exit with a non-zero status (or print an error and exit) so the example fails deterministically; ensure you reference the WebhookSignature::verify call and the $payload, $signature, and $signingSecret variables when implementing the explicit pass/fail checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 292: The snippet uses a falsy-coercing operator
("file_get_contents('php://input') ?: ''") which will turn valid raw payloads
like "0" into an empty string; replace it with a non-coercing form so raw bytes
are preserved for signature checks: call file_get_contents('php://input') once
into a variable (e.g. $raw = file_get_contents('php://input')) and then use
($raw === false ? '' : $raw) (or set $raw = '' only when file_get_contents
returns === false) instead of using ?: to ensure "0" and other falsy-but-valid
payloads are not mutated.
---
Nitpick comments:
In `@examples/webhooks/verify_signature.php`:
- Around line 14-18: Replace the usage of assert() around
WebhookSignature::verify($payload, $signature, $signingSecret) with explicit
deterministic checks: call WebhookSignature::verify for the valid case and for
the bad inputs ($signature = 'not-hex' and ''), then if the results are not the
expected booleans throw an exception or exit with a non-zero status (or print an
error and exit) so the example fails deterministically; ensure you reference the
WebhookSignature::verify call and the $payload, $signature, and $signingSecret
variables when implementing the explicit pass/fail checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6bc5d157-5df0-4e94-904f-e3fe90413fa0
📒 Files selected for processing (4)
README.mdexamples/webhooks/verify_signature.phpsrc/Helper/WebhookSignature.phptests/Helper/WebhookSignatureTest.php
Add `Mailtrap\Helper\WebhookSignature::verify()` to verify Mailtrap webhook signatures using HMAC-SHA256 over the raw request body with constant-time hex comparison via `hash_equals`. Returns false (no throw) for missing/empty/malformed/wrong-length signatures so a single guard at the request handler covers every bad-input case. Includes the shared cross-SDK test fixture (payload + secret + expected signature) that all six Mailtrap SDKs use to stay byte-for-byte compatible, plus a runnable receiver example and README subsection. See https://railsware.atlassian.net/browse/MT-22022
Rabsztok
force-pushed
the
MT-22022-webhook-signature-verification
branch
from
d5be921 to
95a1218
Compare
3 participants
Motivation
Expose a helper so PHP users don't have to re-implement Mailtrap's HMAC-SHA256 webhook signature check on every receiver.
Changes
Mailtrap\Helper\WebhookSignature::verify(string $payload, string $signature, string $signingSecret): bool. HMAC-SHA256 over the raw body, constant-time compare viahash_equals. Returnsfalse(never throws) on empty / wrong-length inputs.tests/Helper/WebhookSignatureTest.phppins the cross-SDK fixture (payload + signing_secret + expected digest) shared verbatim across all six official Mailtrap SDKs to guarantee byte-for-byte parity.examples/webhooks/verify_signature.php— runnable usage snippet.How to test
CI runs the full phpunit suite. Manually:
The script should exit 0 with no output.
Companion PRs
Coordinated rollout across all six official SDKs (same algorithm, same shared fixture):
Summary by CodeRabbit
New Features
Documentation
Tests