diff --git a/CHANGELOG.md b/CHANGELOG.md index 19358afd3..676584906 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Send anonymous server-side PostHog events as personless so unauthenticated requests don't inflate person counts. [#1367](https://github.com/sourcebot-dev/sourcebot/pull/1367) - [EE] Fixed Ask Sourcebot mermaid diagrams overflowing their container by contain-fitting them to both width and height, and made revealing a diagram from the answer jump it into view instantly to avoid over/undershooting. [#1373](https://github.com/sourcebot-dev/sourcebot/pull/1373) +- Verified GitHub review webhook deliveries before processing them. [#1378](https://github.com/sourcebot-dev/sourcebot/pull/1378) - Passed Zoekt index parameters via argv to preserve revision names with punctuation. [#1376](https://github.com/sourcebot-dev/sourcebot/pull/1376) ## [5.0.4] - 2026-06-18 diff --git a/packages/web/src/app/api/(server)/webhook/route.test.ts b/packages/web/src/app/api/(server)/webhook/route.test.ts new file mode 100644 index 000000000..0816b4364 --- /dev/null +++ b/packages/web/src/app/api/(server)/webhook/route.test.ts @@ -0,0 +1,178 @@ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, test, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + env: { + GITHUB_REVIEW_AGENT_APP_ID: "app-id", + GITHUB_REVIEW_AGENT_APP_WEBHOOK_SECRET: "webhook-secret", + GITHUB_REVIEW_AGENT_APP_PRIVATE_KEY_PATH: "/tmp/github-app.pem", + REVIEW_AGENT_AUTO_REVIEW_ENABLED: "true", + REVIEW_AGENT_REVIEW_COMMAND: "review", + }, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + readFileSync: vi.fn(() => "private-key"), + verifyAndReceive: vi.fn(), + getInstallationOctokit: vi.fn(), + octokit: { + rest: { + pulls: { + get: vi.fn(), + }, + }, + }, + octokitDefaults: vi.fn((defaults: unknown) => ({ defaults })), + processGitHubPullRequest: vi.fn(), + processGitLabMergeRequest: vi.fn(), +})); + +vi.mock("@sourcebot/shared", () => ({ + env: mocks.env, + createLogger: () => mocks.logger, +})); + +vi.mock("fs", () => ({ + default: { + readFileSync: mocks.readFileSync, + }, +})); + +vi.mock("octokit", () => ({ + App: vi.fn(function () { + return { + webhooks: { + verifyAndReceive: mocks.verifyAndReceive, + }, + getInstallationOctokit: mocks.getInstallationOctokit, + }; + }), + Octokit: { + plugin: vi.fn(() => ({ + defaults: mocks.octokitDefaults, + })), + }, +})); + +vi.mock("@octokit/plugin-throttling", () => ({ + throttling: {}, +})); + +vi.mock("@gitbeaker/rest", () => ({ + Gitlab: vi.fn(), +})); + +vi.mock("@/features/agents/review-agent/app", () => ({ + processGitHubPullRequest: mocks.processGitHubPullRequest, + processGitLabMergeRequest: mocks.processGitLabMergeRequest, +})); + +vi.mock("@/features/agents/review-agent/types", () => ({ + gitLabMergeRequestPayloadSchema: { + safeParse: vi.fn(), + }, + gitLabNotePayloadSchema: { + safeParse: vi.fn(), + }, +})); + +const importRoute = async () => { + vi.resetModules(); + return import("./route"); +}; + +const createGitHubRequest = (body: unknown, headers: Record = {}) => ( + new NextRequest("https://sourcebot.example.com/api/webhook", { + method: "POST", + headers: { + "content-type": "application/json", + "x-github-event": "pull_request", + "x-github-delivery": "delivery-id", + ...headers, + }, + body: JSON.stringify(body), + }) +); + +describe("POST /api/webhook", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.verifyAndReceive.mockResolvedValue(undefined); + mocks.getInstallationOctokit.mockResolvedValue(mocks.octokit); + }); + + test("skips GitHub events without a signature", async () => { + const { POST } = await importRoute(); + + await POST(createGitHubRequest({ + action: "opened", + installation: { id: 123 }, + repository: { + url: "https://api.github.com/repos/sourcebot-dev/sourcebot", + }, + pull_request: { number: 1 }, + })); + + expect(mocks.verifyAndReceive).not.toHaveBeenCalled(); + expect(mocks.getInstallationOctokit).not.toHaveBeenCalled(); + expect(mocks.processGitHubPullRequest).not.toHaveBeenCalled(); + }); + + test("verifies GitHub events and ignores host headers when choosing the API base URL", async () => { + const { POST } = await importRoute(); + const payload = { + action: "opened", + installation: { id: 123 }, + repository: { + url: "https://ghe.example.com/api/v3/repos/sourcebot-dev/sourcebot", + }, + pull_request: { number: 1 }, + }; + + await POST(createGitHubRequest(payload, { + "x-hub-signature-256": "sha256=signature", + "x-github-enterprise-host": "other.example.com", + })); + + expect(mocks.verifyAndReceive).toHaveBeenCalledWith({ + id: "delivery-id", + name: "pull_request", + payload: JSON.stringify(payload), + signature: "sha256=signature", + }); + expect(mocks.octokitDefaults).toHaveBeenCalledWith({ baseUrl: "https://api.github.com" }); + expect(mocks.octokitDefaults).toHaveBeenCalledWith({ baseUrl: "https://ghe.example.com/api/v3" }); + expect(mocks.octokitDefaults).not.toHaveBeenCalledWith({ baseUrl: "https://other.example.com/api/v3" }); + expect(mocks.getInstallationOctokit).toHaveBeenCalledWith(123); + expect(mocks.processGitHubPullRequest).toHaveBeenCalledWith(mocks.octokit, payload.pull_request); + }); + + test("skips GitHub events when verification fails", async () => { + mocks.verifyAndReceive.mockRejectedValue(new Error("invalid signature")); + const { POST } = await importRoute(); + const payload = { + action: "opened", + installation: { id: 123 }, + repository: { + url: "https://api.github.com/repos/sourcebot-dev/sourcebot", + }, + pull_request: { number: 1 }, + }; + + await POST(createGitHubRequest(payload, { + "x-hub-signature-256": "sha256=signature", + })); + + expect(mocks.verifyAndReceive).toHaveBeenCalledWith({ + id: "delivery-id", + name: "pull_request", + payload: JSON.stringify(payload), + signature: "sha256=signature", + }); + expect(mocks.getInstallationOctokit).not.toHaveBeenCalled(); + expect(mocks.processGitHubPullRequest).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/web/src/app/api/(server)/webhook/route.ts b/packages/web/src/app/api/(server)/webhook/route.ts index 4ed57bf5f..a108548fa 100644 --- a/packages/web/src/app/api/(server)/webhook/route.ts +++ b/packages/web/src/app/api/(server)/webhook/route.ts @@ -2,7 +2,8 @@ import { NextRequest } from "next/server"; import { App, Octokit } from "octokit"; -import { WebhookEventDefinition} from "@octokit/webhooks/types"; +import type { EmitterWebhookEventName } from "@octokit/webhooks"; +import type { WebhookEventDefinition } from "@octokit/webhooks/types"; import { Gitlab } from "@gitbeaker/rest"; import { env } from "@sourcebot/shared"; import { processGitHubPullRequest, processGitLabMergeRequest } from "@/features/agents/review-agent/app"; @@ -58,13 +59,43 @@ const normalizeGithubApiBaseUrl = (baseUrl?: string) => { return baseUrl.replace(/\/+$/, ""); }; -const resolveGithubApiBaseUrl = (headers: Record) => { - const enterpriseHost = headers["x-github-enterprise-host"]; - if (enterpriseHost) { - return normalizeGithubApiBaseUrl(`https://${enterpriseHost}/api/v3`); +const getRepositoryApiUrl = (payload: unknown) => { + if ( + typeof payload === "object" && + payload !== null && + "repository" in payload && + typeof payload.repository === "object" && + payload.repository !== null && + "url" in payload.repository && + typeof payload.repository.url === "string" + ) { + return payload.repository.url; + } + + return undefined; +}; + +const resolveGithubApiBaseUrl = (payload: unknown) => { + const repositoryApiUrl = getRepositoryApiUrl(payload); + if (!repositoryApiUrl) { + return DEFAULT_GITHUB_API_BASE_URL; } - return DEFAULT_GITHUB_API_BASE_URL; + try { + const url = new URL(repositoryApiUrl); + if (url.protocol !== "https:") { + return DEFAULT_GITHUB_API_BASE_URL; + } + + const reposPathIndex = url.pathname.indexOf("/repos/"); + if (reposPathIndex === -1) { + return DEFAULT_GITHUB_API_BASE_URL; + } + + return normalizeGithubApiBaseUrl(`${url.origin}${url.pathname.slice(0, reposPathIndex)}`); + } catch { + return DEFAULT_GITHUB_API_BASE_URL; + } }; const getGithubAppForBaseUrl = (baseUrl: string) => { @@ -132,14 +163,46 @@ if (env.GITLAB_REVIEW_AGENT_TOKEN) { // eslint-disable-next-line authz/require-auth-wrapper -- authenticated via GitHub App / GitLab webhook secrets, not user session export const POST = async (request: NextRequest) => { - const body = await request.json(); + const bodyText = await request.text(); const headers = Object.fromEntries(Array.from(request.headers.entries(), ([key, value]) => [key.toLowerCase(), value])); + let parsedBody: unknown; + + const getParsedBody = () => { + parsedBody ??= JSON.parse(bodyText); + return parsedBody; + }; const githubEvent = headers['x-github-event']; if (githubEvent) { logger.info('GitHub event received:', githubEvent); - const githubApiBaseUrl = resolveGithubApiBaseUrl(headers); + const githubWebhookApp = getGithubAppForBaseUrl(DEFAULT_GITHUB_API_BASE_URL); + if (!githubWebhookApp) { + logger.warn('Received GitHub webhook event but GitHub app env vars are not set'); + return Response.json({ status: 'ok' }); + } + + const signature = headers['x-hub-signature-256'] ?? headers['x-hub-signature']; + if (!signature) { + logger.warn('GitHub webhook signature is missing'); + return Response.json({ status: 'ok' }); + } + + let body: unknown; + try { + await githubWebhookApp.webhooks.verifyAndReceive({ + id: headers['x-github-delivery'] ?? '', + name: githubEvent as EmitterWebhookEventName, + payload: bodyText, + signature, + }); + body = getParsedBody(); + } catch (error) { + logger.warn('GitHub webhook signature or payload is invalid', error); + return Response.json({ status: 'ok' }); + } + + const githubApiBaseUrl = resolveGithubApiBaseUrl(body); logger.debug('Using GitHub API base URL for event', { githubApiBaseUrl }); const githubApp = getGithubAppForBaseUrl(githubApiBaseUrl); @@ -212,6 +275,14 @@ export const POST = async (request: NextRequest) => { return Response.json({ status: 'ok' }); } + let body: unknown; + try { + body = getParsedBody(); + } catch (error) { + logger.warn('GitLab webhook payload is invalid', error); + return Response.json({ status: 'ok' }); + } + if (isGitLabMergeRequestEvent(gitlabEvent, body)) { if (env.REVIEW_AGENT_AUTO_REVIEW_ENABLED === "false") { logger.info('Review agent auto review (REVIEW_AGENT_AUTO_REVIEW_ENABLED) is disabled, skipping');