feat(api): add GitHub webhook routes for push and pull_request - #213
Open
bbornino wants to merge 1 commit into
Open
feat(api): add GitHub webhook routes for push and pull_request#213bbornino wants to merge 1 commit into
bbornino wants to merge 1 commit into
Conversation
Add POST /webhooks/github/push and POST /webhooks/github/pull_request, each verifying the X-Hub-Signature-256 header (HMAC-SHA256 over the raw request body, constant-time compared) before enqueuing the raw payload as a BullMQ job. The push route additionally filters to refs/heads/main, logging and returning early for other branches without enqueuing. Request bodies are validated loosely (just that they're JSON objects, plus the one `ref` field the push route itself needs) since interpreting the payload contents is playfulprogramming#206's job, not this one's.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds two new webhook receiver routes:
POST /webhooks/github/push— receives GitHub'spusheventPOST /webhooks/github/pull_request— receives GitHub'spull_requesteventBoth verify the request's authenticity, then enqueue the raw payload as a BullMQ job for later processing. Interpreting the payload contents is explicitly out of scope for this PR — that's #206's job.
apps/worker/src/tasks/is untouched; the newWEBHOOK_PUSH/WEBHOOK_PULL_REQUESTjob types have no processor yet.Signature verification
Per GitHub's docs on validating webhook deliveries, each request's
X-Hub-Signature-256header is checked against an HMAC-SHA256 of the raw request body (not a re-serialized version of the parsed JSON — GitHub signs the exact bytes it sent, so a route-scopedaddContentTypeParsercaptures the raw string beforeJSON.parse). The comparison usescrypto.timingSafeEqual, following the same constant-time pattern already used inshouldBypassRateLimit(apps/api/src/plugins/rate-limit/index.ts). A missing or invalid signature returns401.This required a new required env var,
GITHUB_WEBHOOK_SECRET, added topackages/common/src/env.ts'sEnvSchemaand documented in.env.example. It gets generated when configuring the webhook in the repo's GitHub settings.Branch filtering
The push route additionally checks
ref === "refs/heads/main"per the issue's note — a push to any other branch is logged and returns200without enqueuing a job.Request body schemas
Kept intentionally loose (
additionalProperties: true, just validating it's a JSON object) rather than fully typing GitHub's large payload shapes — again, deferring to #206. The push route's schema does type the one field it actually reads (ref).Job types
WEBHOOK_PUSH/WEBHOOK_PULL_REQUESTadded toTasksinpackages/bullmq/src/tasks/types.ts, with minimalTaskInputs(unknown— raw payload, untyped on purpose) andTaskOutputs(object) entries. No processor implementation.Testing
apps/api/src/routes/webhooks/verify-signature.test.ts— unit tests for the signature-verification helper itself (valid/invalid/tampered/missing/array-header cases)apps/api/src/routes/webhooks/push.test.ts/pull-request.test.ts— route-level tests viaapp.inject(), mockingcreateJob, covering: valid signature → job enqueued, non-main branch → skipped, invalid/missing signature →401pnpm test:unit,pnpm run build:all, andpnpm run prettierall pass clean.Open question for @fennifith
This route's scope overlaps conceptually with the older #7 ("Webhook to import/sync content from GitHub with the db"), which has no cross-reference to #205. Does #205 supersede #7, or are they meant to coexist (e.g. #7 being the eventual sync-on-webhook logic that #206 will implement on top of these routes)? Want to make sure we're not duplicating effort or leaving #7 stale if this closes it out in spirit.