feat: webhook verifier and handler with idempotency guard - #3
Conversation
Implements HMAC-SHA256 signature verification (constant-time comparison) and a WebhookHandler that rejects duplicate event_ids before enqueuing. https://claude.ai/code/session_012bBivaZw82WSUdGnhmCbtB
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 301b50bb2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 301b50bb2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…, B017) Pin ruff/black to the versions CI currently resolves so lint stays reproducible instead of drifting with upstream releases.
is_event_processed only reflects post-commit state, so a retried delivery arriving before the processor acknowledges the first one passed the duplicate check and overwrote the queued payload via enqueue_webhook. Also check dequeue_unacknowledged() for the event_id.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac76eae092
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| DuplicateEventError: event_id already processed (logged at DEBUG). | ||
| """ | ||
| # verify_hmac_sha256 already logs at WARNING and raises WebhookSignatureError | ||
| verify_hmac_sha256(raw_bytes, signature_header, self._secret) |
There was a problem hiding this comment.
When event_id or payload comes from a header, a separately parsed object, or any other value not derived from raw_bytes, the signature authenticates only raw_bytes while the independent values are passed to the queue. A captured valid body/signature can therefore be replayed with a new event_id to bypass deduplication, or paired with a different payload, so the handler should parse and derive all queued identity/content from the verified bytes or otherwise bind them cryptographically.
Useful? React with 👍 / 👎.
| # Accept bare hex digest or "sha256=<hex>" prefix | ||
| received = signature_header.removeprefix("sha256=") | ||
|
|
||
| if not hmac.compare_digest(expected, received): |
There was a problem hiding this comment.
When a malformed signature header contains a non-ASCII character, hmac.compare_digest(expected, received) raises TypeError rather than the documented WebhookSignatureError. Such a request can consequently escape the authentication-error path and produce a 500 response; validate/decode the header as hexadecimal or compare byte strings so every malformed signature is rejected consistently.
Useful? React with 👍 / 👎.
| already_queued = any( | ||
| queued_id == event_id | ||
| for queued_id, _ in self._store.dequeue_unacknowledged() | ||
| ) |
There was a problem hiding this comment.
When two deliveries with the same event_id are handled concurrently, both can complete this scan before either reaches enqueue_webhook, so both calls report success and a backend that appends will persist duplicates; the current in-memory backend instead silently lets the later payload overwrite the earlier one. Enforce uniqueness atomically in the store operation rather than using a separate read-before-write check.
Useful? React with 👍 / 👎.
2 participants
Implements HMAC-SHA256 signature verification (constant-time comparison)
and a WebhookHandler that rejects duplicate event_ids before enqueuing.
https://claude.ai/code/session_012bBivaZw82WSUdGnhmCbtB