Skip to content

fix(webhook): authenticate replay ledger IDs before deduplication - #93

Merged
LIghtJUNction merged 1 commit into
mainfrom
fix/issue-81-webhook-replay-identity
Sep 27, 2026
Merged

LIghtJUNction merged 1 commit into
mainfrom
fix/issue-81-webhook-replay-identity

Conversation

@LIghtJUNction

Copy link
Copy Markdown
Contributor

Scope

Refs #81 and #85. This is a small security regression fix, not closure of either umbrella issue.

The previous receiver verifier authenticated timestamp + raw body but used unsigned Msg-Delivery-Id as its replay-ledger key. An attacker holding one valid delivery could change that header and bypass seen. Event headers likewise were not bound to the signed envelope.

Changes

  • src/msg/workers/webhook.py: validate case-insensitive/non-ambiguous headers, HMAC, strict JSON and equality of envelope/header delivery ID, event ID and timestamp before reading/updating dedupe state. Malformed values produce stable failures without exposing payloads.
  • tests/test_webhook_identity.py: 13 regressions, including changed delivery/event headers, missing/non-string/non-ASCII headers, duplicate JSON keys, invalid signed envelopes, case-insensitive HTTP headers and ambiguous duplicates.
  • tests/test_webhook_delivery.py: existing receiver examples now use the actual authenticated envelope produced by EffectWorker; all prior assertions retained.

Evidence

The new regression suite failed 13/13 on the original implementation. After the fix, python -m pytest -q tests/test_webhook_identity.py tests/test_webhook_delivery.py tests/test_webhook_domain_events.py passed 29 tests using Python 3.13.5 and a real isolated PostgreSQL 16.15 instance. This local result is not Python 3.15 CI or production evidence; CI on this PR must independently pass.

Compatibility / boundaries

No database/schema migration, sender wire-format change, authorization change or deployment. The helper now intentionally rejects nonconforming unsigned/identity-incomplete receiver envelopes; the official sender already includes all required fields. Production consumers must persist the authenticated delivery ID atomically with their side effect; the optional in-memory seen set is not durable dedupe.

Remaining #81 acceptance includes full collaboration Resource/Revision primitives, comprehensive notification preferences and real external deployment matrices. #85 retains its other cross-module refactor items. No production deployment is authorized.

…lope

Refs #81, #85. Reject unsigned delivery/event identity substitutions and malformed envelope/header values before touching receiver dedupe state. Preserve timestamp plus raw-body HMAC format and sender contracts.
Copilot AI lite review requested due to automatic review settings September 27, 2026 19:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review ✅ Completed 2026-09-27T19:58:43.747245Z d0cdd4d PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@LIghtJUNction
LIghtJUNction merged commit 35689df into main Sep 27, 2026
2 checks passed

Copy link
Copy Markdown
Contributor Author

CI evidence verified from the completed run and downloaded JUnit artifact: run 36346072944 succeeded on the workflow's Python 3.15 / PostgreSQL 16 / Valkey 9.0.6 configuration. artifacts/tests.xml reports 862 tests, 0 failures, 0 errors, 0 skipped; artifacts/conformance.xml reports 8 tests, 0 failures, 0 errors, 0 skipped. The actual-tokenizer conformance step and installable artifact build both succeeded. Verification artifact: 10940324678 (rewrite-verification), archive SHA-256 5986ad8da69a5e4b78d195442d7ed55d28a082d3b751a49390f4de591c9d5267.

The final diff was reviewed and merged as 35689df. This completes the replay-identity defect only; #81 and #85 remain open for their remaining acceptance work. No production deployment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants