fix: use raw request body for webhook signature verification#852
fix: use raw request body for webhook signature verification#852zacharyblasczyk wants to merge 3 commits intomainfrom
Conversation
Both GitHub and TFE webhook handlers computed HMACs over JSON.stringify(req.body) — the re-serialized parsed body rather than the original bytes. If the upstream ever changes its JSON serialization (whitespace, key order, number formatting), signature verification would silently break. Switch webhook routes to express.raw() so the handler receives the original Buffer. The global express.json() middleware now skips webhook paths to avoid consuming the stream first.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughGitHub webhook handling was changed to use raw body parsing and explicit signature verification on the raw payload; the server JSON middleware was adjusted to skip parsing for webhook routes so those routes can receive raw bodies. Changes
Sequence Diagram(s)mermaid GitHub->>Server: POST /api/github/webhook (raw body + x-hub-signature-256) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Summary
JSON.stringify(req.body)(re-serialized) instead of the original bytes from the requestexpress.raw({ type: "application/json" })so handlers receive the originalBufferand verify against actual bytesexpress.json()for webhook paths to prevent it from consuming the stream firstTest plan
tfc-workspace-job-agent-v2mergesSummary by CodeRabbit