fix(web): verify review webhook deliveries

[brendan-kellam]

Fixes SOU-1463

Summary

• Verify GitHub review webhook deliveries before processing them.
• Resolve the GitHub API base URL from the verified payload.
• Add route coverage for the webhook checks.

Tests

• yarn workspace @sourcebot/web test src/app/api/(server)/webhook/route.test.ts
• yarn workspace @sourcebot/web lint

Summary by CodeRabbit

• Bug Fixes
  • GitHub review webhooks are now verified before processing, improving security and reliability.
  • Webhook handling is more robust by verifying GitHub payloads using the raw request body and safely handling malformed GitLab payload JSON.
  • GitHub API routing now resolves the API base URL from repository information in the webhook payload for improved compatibility.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a28f16a6-19f6-406a-8108-fbb4c145b144

📥 Commits

Reviewing files that changed from the base of the PR and between 7c34953 and f7fd886.

📒 Files selected for processing (2)

• CHANGELOG.md
• packages/web/src/app/api/(server)/webhook/route.test.ts

✅ Files skipped from review due to trivial changes (1)

• CHANGELOG.md

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/web/src/app/api/(server)/webhook/route.test.ts

---

Walkthrough

The webhook route now reads raw request bodies, verifies GitHub webhooks before JSON parsing, derives the GitHub API base URL from the payload, and handles invalid GitLab JSON without throwing. Tests cover the verification paths, and the changelog is updated.

Changes

GitHub Webhook Signature Verification

Layer / File(s)	Summary
Payload-based URL helpers and POST handler updates packages/web/src/app/api/(server)/webhook/route.ts	Adds EmitterWebhookEventName/WebhookEventDefinition imports; replaces header-based resolveGithubApiBaseUrl with payload-based helpers that derive the API base URL from repository.url; rewrites POST handler to read raw body text, call verifyAndReceive before parsing JSON, and wraps GitLab JSON parsing in a try/catch.
Webhook route Vitest suite packages/web/src/app/api/(server)/webhook/route.test.ts	Hoists mocks for env, logger, fs, verifyAndReceive, Octokit, and GitLab deps; adds a NextRequest helper; covers three cases: missing signature skips verification, valid signature invokes verifyAndReceive and handlers with correct args, and failed verification skips Octokit and handler.
Changelog entry CHANGELOG.md	Adds a Fixed entry noting GitHub review webhook deliveries are verified before processing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: verifying review webhook deliveries before processing them.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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 brendan/fix-SOU-1463

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/web/src/app/api/(server)/webhook/route.ts (1)

185-185: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Require X-Hub-Signature-256 here. GitHub recommends the SHA-256 webhook signature header, and supported GitHub Enterprise Server versions send it too. Accepting X-Hub-Signature keeps the SHA-1 legacy path open unnecessarily.

Proposed hardening

-        const signature = headers['x-hub-signature-256'] ?? headers['x-hub-signature'];
+        const signature = headers['x-hub-signature-256'];

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/web/src/app/api/`(server)/webhook/route.ts at line 185, The webhook
signature lookup currently falls back from X-Hub-Signature-256 to
X-Hub-Signature, which leaves the legacy SHA-1 path open. Update the webhook
validation in route.ts to require only X-Hub-Signature-256 and make the
signature handling in the webhook handler reject requests when that header is
missing, using the existing signature verification logic around the
signature/header parsing.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/web/src/app/api/`(server)/webhook/route.test.ts:
- Around line 153-169: The failure-path test in POST should assert that
verifyAndReceive was actually called before the webhook is skipped. Update the
test "skips GitHub events when verification fails" to include an expectation on
mocks.verifyAndReceive so it proves the handler reached verification and exited
because that step rejected, not due to an earlier guard. Keep the existing
checks on getInstallationOctokit and processGitHubPullRequest.

---

Nitpick comments:
In `@packages/web/src/app/api/`(server)/webhook/route.ts:
- Line 185: The webhook signature lookup currently falls back from
X-Hub-Signature-256 to X-Hub-Signature, which leaves the legacy SHA-1 path open.
Update the webhook validation in route.ts to require only X-Hub-Signature-256
and make the signature handling in the webhook handler reject requests when that
header is missing, using the existing signature verification logic around the
signature/header parsing.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 141a98f6-1cfc-4c68-a6c0-fbdeb7b557e2

📥 Commits

Reviewing files that changed from the base of the PR and between d546511 and 7c34953.

📒 Files selected for processing (3)

• CHANGELOG.md
• packages/web/src/app/api/(server)/webhook/route.test.ts
• packages/web/src/app/api/(server)/webhook/route.ts