MT-22022: Add webhook signature verification helper#67
Conversation
📝 WalkthroughWalkthroughAdds HMAC-SHA256 webhook signature verification (mailtrap.webhooks.verify_signature), re-exports it at package top-level, provides a WSGI example that verifies incoming requests, updates README examples, and adds comprehensive unit tests. ChangesWebhook signature verification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 docstrings
🧪 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
35b2788 to
129bc81
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
249-274: 💤 Low valueClear and accurate webhook signature documentation.
The explanation of HMAC-SHA256 verification is well-written, and the important warning about using raw request body bytes (not re-serialized JSON) is prominently placed.
Consider adding a brief note about framework-specific approaches for obtaining the raw request body, e.g.:
**Framework examples:** - Flask: `raw_body = request.get_data()` - FastAPI: `raw_body = await request.body()` - Django: `raw_body = request.body`This would help users who are unfamiliar with accessing raw request bodies in their chosen framework, though the current framework-agnostic approach is also valid.
🤖 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 `@README.md` around lines 249 - 274, Add a short framework-specific note under the "Verifying webhook signatures" section to show how to obtain the raw request body for common Python frameworks (e.g., Flask, FastAPI, Django) so users can correctly pass raw_body to mt.verify_signature; reference the Mailtrap-Signature header and the signing_secret usage, and keep the note concise and clearly labeled (e.g., "Framework examples") to avoid changing the existing verification guidance.
🤖 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.
Nitpick comments:
In `@README.md`:
- Around line 249-274: Add a short framework-specific note under the "Verifying
webhook signatures" section to show how to obtain the raw request body for
common Python frameworks (e.g., Flask, FastAPI, Django) so users can correctly
pass raw_body to mt.verify_signature; reference the Mailtrap-Signature header
and the signing_secret usage, and keep the note concise and clearly labeled
(e.g., "Framework examples") to avoid changing the existing verification
guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99d05986-8255-4a01-bbc5-c16791711838
📒 Files selected for processing (5)
README.mdexamples/webhooks/verify_signature.pymailtrap/__init__.pymailtrap/webhooks.pytests/unit/test_webhook_signature.py
Add `mailtrap.webhooks.verify_signature` (re-exported as `mt.verify_signature`) to verify Mailtrap webhook signatures using HMAC-SHA256 over the raw request body with constant-time hex comparison via `hmac.compare_digest`. Returns False (no raise) for missing/empty/malformed/wrong-length signatures so a single guard at the request handler covers every bad-input case. Accepts both `str` and `bytes` payloads; signed input must be the raw body bytes (the helper docstring and example warn against re-serializing parsed JSON). 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 Flask receiver example and README subsection. See https://railsware.atlassian.net/browse/MT-22022
Rabsztok
force-pushed
the
MT-22022-webhook-signature-verification
branch
from
129bc81 to
f2f05dc
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/unit/test_webhook_signature.py (2)
24-24: ⚡ Quick winSuppress secret-scanner false positives for the shared fixture.
Line 24 is an intentional cross-SDK test fixture, but it will keep tripping leak/password detectors. Add an inline suppression note so CI/security scans stay actionable.
Suggested patch
-FIXTURE_SIGNING_SECRET = "8d9a3c0e7f5b2d4a6c1e9f8b3a7d5c2e" +FIXTURE_SIGNING_SECRET = "8d9a3c0e7f5b2d4a6c1e9f8b3a7d5c2e" # noqa: S105 - shared non-production test fixture🤖 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 `@tests/unit/test_webhook_signature.py` at line 24, The test constant FIXTURE_SIGNING_SECRET in tests/unit/test_webhook_signature.py is an intentional cross-SDK fixture that triggers secret scanners; update the declaration to include an inline suppression comment explaining it's a non-sensitive test fixture and add tool-specific suppressions (for example append a short note plus common suppressors like "# noqa: S105" and/or "# pragma: nosec") so CI/security scanners ignore this known test value while keeping the explanatory comment next to FIXTURE_SIGNING_SECRET.
30-126: ⚡ Quick winAdd explicit wrong-type input tests for contract lock-in.
verify_signatureguaranteesFalse(not raise) for wrong types; add direct tests for invalid payload/signature/secret types to prevent regressions.Suggested test additions
+ def test_returns_false_for_wrong_input_types(self) -> None: + assert verify_signature(123, FIXTURE_EXPECTED_SIGNATURE, FIXTURE_SIGNING_SECRET) is False + assert verify_signature(FIXTURE_PAYLOAD, 123, FIXTURE_SIGNING_SECRET) is False + assert verify_signature(FIXTURE_PAYLOAD, FIXTURE_EXPECTED_SIGNATURE, 123) is False🤖 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 `@tests/unit/test_webhook_signature.py` around lines 30 - 126, Add tests to TestVerifySignature that pass invalid types to verify_signature and assert it returns False (not raise) to lock the contract: include cases such as payload as an int and as None, signature as bytes, int, and a list, and signing_secret as None and as a dict; use the existing FIXTURE_EXPECTED_SIGNATURE, FIXTURE_SIGNING_SECRET, FIXTURE_PAYLOAD and SIGNATURE_HEX_LENGTH symbols to construct inputs and assertions so failures point to verify_signature.
🤖 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/verify_signature.py`:
- Around line 13-16: The code currently reads and decodes the request body into
a UTF-8 string before verification which can raise UnicodeDecodeError and
violates verify_signature's expectation of raw bytes; change the logic that
reads environ["wsgi.input"] into the variable payload so it remains bytes (do
not call .decode("utf-8")) and pass those bytes directly to
mt.verify_signature(payload, signature, SIGNING_SECRET) so the signature
function receives the exact raw request body.
---
Nitpick comments:
In `@tests/unit/test_webhook_signature.py`:
- Line 24: The test constant FIXTURE_SIGNING_SECRET in
tests/unit/test_webhook_signature.py is an intentional cross-SDK fixture that
triggers secret scanners; update the declaration to include an inline
suppression comment explaining it's a non-sensitive test fixture and add
tool-specific suppressions (for example append a short note plus common
suppressors like "# noqa: S105" and/or "# pragma: nosec") so CI/security
scanners ignore this known test value while keeping the explanatory comment next
to FIXTURE_SIGNING_SECRET.
- Around line 30-126: Add tests to TestVerifySignature that pass invalid types
to verify_signature and assert it returns False (not raise) to lock the
contract: include cases such as payload as an int and as None, signature as
bytes, int, and a list, and signing_secret as None and as a dict; use the
existing FIXTURE_EXPECTED_SIGNATURE, FIXTURE_SIGNING_SECRET, FIXTURE_PAYLOAD and
SIGNATURE_HEX_LENGTH symbols to construct inputs and assertions so failures
point to verify_signature.
🪄 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: aad07a05-0968-4832-bf3c-d240b48385db
📒 Files selected for processing (5)
README.mdexamples/webhooks/verify_signature.pymailtrap/__init__.pymailtrap/webhooks.pytests/unit/test_webhook_signature.py
✅ Files skipped from review due to trivial changes (1)
- README.md
| payload = environ["wsgi.input"].read(length).decode("utf-8") | ||
| signature = environ.get("HTTP_MAILTRAP_SIGNATURE", "") | ||
|
|
||
| if not mt.verify_signature(payload, signature, SIGNING_SECRET): |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Python 3, does bytes.decode("utf-8") raise UnicodeDecodeError on invalid UTF-8 by default?
💡 Result:
Yes, in Python 3, bytes.decode("utf-8") raises a UnicodeDecodeError by default when it encounters invalid UTF-8 sequences [1][2]. This occurs because the default value for the errors argument in the bytes.decode method is 'strict' [1][2]. The 'strict' error handler is defined to raise a UnicodeError (specifically a UnicodeDecodeError during decoding) whenever an encoding or decoding error is encountered [2][3][4]. If you wish to change this behavior, you can provide a different error handler for the errors argument, such as: - 'ignore': Discards the malformed bytes [3]. - 'replace': Inserts the official Unicode replacement character (U+FFFD) for malformed bytes [3][5]. Example: # This raises UnicodeDecodeError b'\xff'.decode('utf-8') # This returns '\ufffd' b'\xff'.decode('utf-8', errors='replace')
Citations:
- 1: https://docs.python.org/3/library/stdtypes.html?highlight=decode
- 2: https://docs.python.org/3/library/codecs.html?highlight=codecs
- 3: https://johnlekberg.com/blog/2020-04-03-codec-errors.html
- 4: https://docs.python.org/3/library/codecs.html
- 5: https://stackoverflow.com/questions/40029017/python2-using-decode-with-errors-replace-still-returns-errors
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant snippet in the example file with surrounding context
sed -n '1,120p' examples/webhooks/verify_signature.py | nl -ba
# Find where verify_signature is defined/used in the repo
rg -n "def verify_signature|verify_signature\(" -S . | head -n 50Repository: mailtrap/mailtrap-python
Length of output: 111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the example file with line numbers (no nl)
cat -n examples/webhooks/verify_signature.py | sed -n '1,160p'
# Find verify_signature definitions/usages
rg -n "def verify_signature|verify_signature\(" -S . | head -n 80Repository: mailtrap/mailtrap-python
Length of output: 2293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect verify_signature implementation
cat -n mailtrap/webhooks.py | sed -n '1,200p'
# Inspect unit tests around webhook signature verification
cat -n tests/unit/test_webhook_signature.py | sed -n '1,220p'Repository: mailtrap/mailtrap-python
Length of output: 8903
Use raw request body bytes for signature verification (remove UTF-8 decoding).
examples/webhooks/verify_signature.py decodes environ["wsgi.input"] with .decode("utf-8") before calling mt.verify_signature. With Python’s default errors="strict", this can raise UnicodeDecodeError on non-UTF-8 payloads, whereas verify_signature is designed to handle inputs safely; also verify_signature expects the raw request body “exactly as received” and accepts bytes directly.
Suggested patch
- payload = environ["wsgi.input"].read(length).decode("utf-8")
+ payload = environ["wsgi.input"].read(length)
signature = environ.get("HTTP_MAILTRAP_SIGNATURE", "")
if not mt.verify_signature(payload, signature, SIGNING_SECRET):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| payload = environ["wsgi.input"].read(length).decode("utf-8") | |
| signature = environ.get("HTTP_MAILTRAP_SIGNATURE", "") | |
| if not mt.verify_signature(payload, signature, SIGNING_SECRET): | |
| payload = environ["wsgi.input"].read(length) | |
| signature = environ.get("HTTP_MAILTRAP_SIGNATURE", "") | |
| if not mt.verify_signature(payload, signature, SIGNING_SECRET): |
🤖 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.py` around lines 13 - 16, The code
currently reads and decodes the request body into a UTF-8 string before
verification which can raise UnicodeDecodeError and violates verify_signature's
expectation of raw bytes; change the logic that reads environ["wsgi.input"] into
the variable payload so it remains bytes (do not call .decode("utf-8")) and pass
those bytes directly to mt.verify_signature(payload, signature, SIGNING_SECRET)
so the signature function receives the exact raw request body.
3 participants
Motivation
Expose a helper so Python 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. Re-exported asmailtrap.verify_signatureformt.verify_signature(...)usage. HMAC-SHA256 over the raw body, constant-time compare viahmac.compare_digest. Acceptspayloadasstr(UTF-8) orbytes. ReturnsFalse(never raises) on empty / wrong-length / non-hex / wrong-type inputs.tests/unit/test_webhook_signature.pypins 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.py— runnable usage snippet.How to test
CI runs pytest, mypy, and pre-commit. 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