MT-22022: Add webhook signature verification helper#110
Open
Rabsztok wants to merge 3 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR introduces webhook signature verification for the Mailtrap Ruby gem via HMAC-SHA256 validation, adding the ChangesWebhook Signature Verification
Sequence DiagramsequenceDiagram
participant Receiver as Webhook Receiver
participant Verify as Mailtrap::Webhooks
participant OpenSSL as OpenSSL::HMAC
participant Compare as OpenSSL.fixed_length_secure_compare
Receiver->>Verify: verify_signature(payload, signature, signing_secret)
alt Invalid inputs
Verify-->>Receiver: false
else Valid inputs
Verify->>OpenSSL: hexdigest(signing_secret, payload)
OpenSSL-->>Verify: expected_hex_digest
Verify->>Compare: fixed_length_secure_compare(expected_hex_digest, signature)
Compare-->>Verify: true or false
Verify-->>Receiver: boolean result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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
🤖 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 `@examples/webhooks_signature_verification.rb`:
- Around line 1-2: Add explicit requires for Rack and JSON at the top of the
example: include require 'rack' and require 'json' alongside the existing
require 'mailtrap' so that Rack::Request and JSON.parse are defined when running
the standalone script (this affects the usage sites of Rack::Request and
JSON.parse in the file).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f503201f-688f-4346-89a4-536e35b0c132
📒 Files selected for processing (5)
README.mdexamples/webhooks_signature_verification.rblib/mailtrap.rblib/mailtrap/webhooks.rbspec/mailtrap/webhooks_spec.rb
Rabsztok
force-pushed
the
MT-22022-webhook-signature-verification
branch
from
2c0513e to
f43cb34
Compare
This was referenced May 20, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/mailtrap/webhooks_spec.rb (1)
24-153: ⚡ Quick winAdd explicit non-string input cases to pin the public contract.
The suite verifies many malformed values, but it does not currently assert the documented
falsebehavior for non-string inputs (nil, numeric, etc.).✅ Suggested test addition
describe '.verify_signature' do @@ context 'with an empty payload but a non-empty signature' do it 'returns false' do @@ end end + + context 'with non-string inputs' do + it 'returns false without raising' do + expect( + described_class.verify_signature( + payload: 123, + signature: fixture_expected_signature, + signing_secret: fixture_signing_secret + ) + ).to be false + + expect( + described_class.verify_signature( + payload: fixture_payload, + signature: nil, + signing_secret: fixture_signing_secret + ) + ).to be false + end + end🤖 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 `@spec/mailtrap/webhooks_spec.rb` around lines 24 - 153, Add tests exercising non-string inputs for the public contract of verify_signature: create a new context(s) that calls described_class.verify_signature with payload and/or signing_secret set to nil, numeric (e.g. 12345), and a non-string type like an Array, using fixture_expected_signature (and also an empty or nil signature) and assert the method returns false (and does not raise). Reference the method verify_signature and the constant SIGNATURE_HEX_LENGTH only to keep behavior consistent with existing malformed-signature tests; ensure each case mirrors the pattern used in other contexts (call verify_signature then expect(result).to be false).
🤖 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 `@examples/webhooks_signature_verification.rb`:
- Around line 5-6: The hard-coded hex-like secret assigned to signing_secret
looks like a real credential and can trigger secret scanners; replace that
literal with an explicit non-secret placeholder (e.g.,
'your_signing_secret_here' or 'REPLACE_WITH_SIGNING_SECRET') and update any
accompanying comment to indicate it must be set from a safe source (env var or
secret manager) before computing signature with OpenSSL::HMAC.hexdigest using
signing_secret and payload (refer to signing_secret, signature, payload).
---
Nitpick comments:
In `@spec/mailtrap/webhooks_spec.rb`:
- Around line 24-153: Add tests exercising non-string inputs for the public
contract of verify_signature: create a new context(s) that calls
described_class.verify_signature with payload and/or signing_secret set to nil,
numeric (e.g. 12345), and a non-string type like an Array, using
fixture_expected_signature (and also an empty or nil signature) and assert the
method returns false (and does not raise). Reference the method verify_signature
and the constant SIGNATURE_HEX_LENGTH only to keep behavior consistent with
existing malformed-signature tests; ensure each case mirrors the pattern used in
other contexts (call verify_signature then expect(result).to be false).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fa6cb163-7069-448c-a84c-5202f9998a3e
📒 Files selected for processing (5)
README.mdexamples/webhooks_signature_verification.rblib/mailtrap.rblib/mailtrap/webhooks.rbspec/mailtrap/webhooks_spec.rb
✅ Files skipped from review due to trivial changes (1)
- lib/mailtrap.rb
mklocek
reviewed
May 21, 2026
| → _Sending Domains_ → _Your domain_ → _SMTP/API Settings_ to find the SMTP | ||
| configuration example. | ||
|
|
||
| ### Verifying webhook signatures |
Contributor
There was a problem hiding this comment.
Same as I commented in .NET SDK PR - I don't think it's such a core functionality that it must be mentioned in general README
IgorDobryn
approved these changes
May 22, 2026
Contributor
IgorDobryn
left a comment
IgorDobryn
left a comment
There was a problem hiding this comment.
Comments above makes sense
mklocek
approved these changes
May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Follow-up to mailtrap-ruby#108 review comment: expose a helper so users don't have to re-implement Mailtrap's HMAC-SHA256 webhook signature check on every receiver.
Changes
Mailtrap::Webhooks.verify_signature(payload:, signature:, signing_secret:)→true/false. HMAC-SHA256 over the raw body, constant-time compare viaOpenSSL.fixed_length_secure_compare. Returnsfalse(never raises) on empty / wrong-length / non-hex / non-string inputs.spec/mailtrap/webhooks_spec.rbpins a cross-SDK fixture (payload + signing_secret + expected digest) shared verbatim across all six official Mailtrap SDKs to guarantee byte-for-byte parity.examples/webhooks_signature_verification.rb— runnable usage snippet.How to test
CI runs the full spec suite and lint. Manually:
bundle exec ruby examples/webhooks_signature_verification.rbThe 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