Skip to content

fix: use raw request body for webhook signature verification#852

Open
zacharyblasczyk wants to merge 3 commits intomainfrom
fix/webhook-raw-body-signature
Open

fix: use raw request body for webhook signature verification#852
zacharyblasczyk wants to merge 3 commits intomainfrom
fix/webhook-raw-body-signature

Conversation

@zacharyblasczyk
Copy link
Copy Markdown
Member

@zacharyblasczyk zacharyblasczyk commented Mar 20, 2026

Summary

  • Webhook HMAC signatures were computed over JSON.stringify(req.body) (re-serialized) instead of the original bytes from the request
  • If GitHub or TFC ever changes JSON serialization (whitespace, key order, number formatting), signature verification would silently break
  • Switch webhook routes to express.raw({ type: "application/json" }) so handlers receive the original Buffer and verify against actual bytes
  • Skip global express.json() for webhook paths to prevent it from consuming the stream first

Test plan

  • Verify GitHub webhook signature verification still works (send a test event)
  • Verify TFE webhook signature verification still works after tfc-workspace-job-agent-v2 merges
  • Confirm non-webhook API routes still parse JSON normally

Summary by CodeRabbit

  • Improvements
    • Webhook handling made more reliable: incoming deliveries are now verified and requests lacking a valid signature return 401 Unauthorized.
    • Webhook payload parsing is more robust to prevent malformed delivery handling.
    • Webhook endpoints now bypass generic JSON parsing to avoid interference with raw delivery payloads and improve consistency.

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.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 20, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f98cd757-3da8-40ad-a282-30addec2cc00

📥 Commits

Reviewing files that changed from the base of the PR and between 0747356 and 9cab6ae.

📒 Files selected for processing (1)
  • apps/api/src/server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/api/src/server.ts

📝 Walkthrough

Walkthrough

GitHub 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

Cohort / File(s) Summary
GitHub Webhook Route
apps/api/src/routes/github/index.ts
Replaced asyncHandler-based JSON handling with express.raw({ type: "application/json" }); signature header is extracted and verification now accepts the raw Buffer (uses body.toString("utf8")); payload is JSON.parsed after verification and passed to handleWorkflowRunEvent.
Server Middleware
apps/api/src/server.ts
Reworked global JSON parser into conditional middleware that bypasses JSON parsing for paths starting with /api/github/webhook or /api/tfe/webhook, preserving express.json({ limit: "100mb" }) for other routes.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant GitHub as GitHub
participant Server as API_Server
participant Verifier as Signature_Verifier
participant Handler as Webhook_Handler

GitHub->>Server: POST /api/github/webhook (raw body + x-hub-signature-256)
Note over Server: Conditional middleware skips express.json for webhook paths
Server->>Verifier: verify(body: Buffer, signatureHeader)
Verifier-->>Server: valid / invalid
alt valid
Server->>Handler: parse JSON from raw body -> handleWorkflowRunEvent(payload)
Handler-->>Server: 200 OK
Server-->>GitHub: 200 OK
else invalid or missing signature
Server-->>GitHub: 401 Unauthorized
end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I hop through bytes with nimble cheer,
I check each signature sincere,
Raw buffers first, then parse with care,
Workflow events handled fair—🐇✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main technical change: fixing webhook signature verification by using raw request bodies instead of parsed/reserialized JSON.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/webhook-raw-body-signature

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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