diff --git a/apps/mcp/lib/tools/submissions/create.ts b/apps/mcp/lib/tools/submissions/create.ts index c55ec5a..92b9e40 100644 --- a/apps/mcp/lib/tools/submissions/create.ts +++ b/apps/mcp/lib/tools/submissions/create.ts @@ -142,6 +142,7 @@ export async function handleSubmissionsCreate(raw: unknown) { prUrl: parsed.data.pr_url, expectedGithubHandle: auth.profile.github_handle, expectedRepoUrl: repoUrl, + expectedIssueUrl: b.github_issue_url, token: process.env.GITHUB_TOKEN, }); if (!verify.ok) { diff --git a/packages/shared/src/github/verify-pr-ownership.ts b/packages/shared/src/github/verify-pr-ownership.ts index f61d0ac..f1dec8d 100644 --- a/packages/shared/src/github/verify-pr-ownership.ts +++ b/packages/shared/src/github/verify-pr-ownership.ts @@ -12,6 +12,8 @@ export type VerifyPrOwnershipInput = { expectedGithubHandle: string; /** Full URL like `https://github.com/owner/repo` (no trailing slash). */ expectedRepoUrl: string; + /** Optional full URL like `https://github.com/owner/repo/issues/123`. */ + expectedIssueUrl?: string | null; /** Optional GitHub token for higher rate limit. */ token?: string; }; @@ -24,6 +26,7 @@ export type VerifyPrOwnershipResult = | "pr_not_found" | "author_mismatch" | "repo_mismatch" + | "issue_mismatch" | "rate_limited" | "invalid_url" | "upstream_error"; @@ -32,6 +35,12 @@ export type VerifyPrOwnershipResult = const PR_URL_RE = /^https:\/\/github\.com\/([^/]+)\/([^/]+)\/pull\/(\d+)\/?$/; +const ISSUE_URL_RE = + /^https:\/\/github\.com\/([^/]+)\/([^/]+)\/issues\/(\d+)\/?$/; + +const CLOSING_KEYWORD_RE = + /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+(?:(?:https:\/\/github\.com\/([^/]+)\/([^/]+)\/issues\/)?#?(\d+))/gi; + export async function verifyPrOwnership( input: VerifyPrOwnershipInput ): Promise { @@ -62,7 +71,11 @@ export async function verifyPrOwnership( if (!res.ok) return { ok: false, reason: "upstream_error" }; - let body: { user: { login: string } | null; base: { repo: { html_url: string } | null } | null }; + let body: { + user: { login: string } | null; + base: { repo: { html_url: string } | null } | null; + body?: string | null; + }; try { body = (await res.json()) as typeof body; } catch { @@ -84,5 +97,41 @@ export async function verifyPrOwnership( return { ok: false, reason: "repo_mismatch" }; } + if (input.expectedIssueUrl) { + const expectedIssue = parseIssueUrl(input.expectedIssueUrl); + if (!expectedIssue) return { ok: false, reason: "issue_mismatch" }; + + if (!prBodyClosesIssue(body.body ?? "", expectedIssue)) { + return { ok: false, reason: "issue_mismatch" }; + } + } + return { ok: true }; } + +function parseIssueUrl(url: string): { owner: string; repo: string; number: string } | null { + const m = url.match(ISSUE_URL_RE); + if (!m) return null; + return { owner: m[1]!.toLowerCase(), repo: m[2]!.toLowerCase(), number: m[3]! }; +} + +function prBodyClosesIssue( + body: string, + expectedIssue: { owner: string; repo: string; number: string } +): boolean { + for (const m of body.matchAll(CLOSING_KEYWORD_RE)) { + const [, owner, repo, number] = m; + if (number !== expectedIssue.number) continue; + + // `Fixes #123` is scoped to the already-verified base repo. A fully + // qualified URL must point to the same repo as the bounty issue. + if (!owner && !repo) return true; + if ( + owner?.toLowerCase() === expectedIssue.owner && + repo?.toLowerCase() === expectedIssue.repo + ) { + return true; + } + } + return false; +} diff --git a/packages/shared/tests/github/verify-pr-ownership.test.ts b/packages/shared/tests/github/verify-pr-ownership.test.ts index e36f87a..b01d08b 100644 --- a/packages/shared/tests/github/verify-pr-ownership.test.ts +++ b/packages/shared/tests/github/verify-pr-ownership.test.ts @@ -33,6 +33,50 @@ describe("verifyPrOwnership", () => { expect(result).toEqual({ ok: true }); }); + it("returns ok when the PR body closes the expected issue", async () => { + fetchMock.mockResolvedValueOnce( + new Response( + JSON.stringify({ + user: { login: "alice" }, + base: { repo: { html_url: "https://github.com/acme/proj" } }, + body: "Implements the fix.\n\nCloses #67", + }), + { status: 200 } + ) + ); + + const result = await verifyPrOwnership({ + prUrl: "https://github.com/acme/proj/pull/42", + expectedGithubHandle: "alice", + expectedRepoUrl: "https://github.com/acme/proj", + expectedIssueUrl: "https://github.com/acme/proj/issues/67", + }); + + expect(result).toEqual({ ok: true }); + }); + + it("returns issue_mismatch when the PR body closes a different issue", async () => { + fetchMock.mockResolvedValueOnce( + new Response( + JSON.stringify({ + user: { login: "alice" }, + base: { repo: { html_url: "https://github.com/acme/proj" } }, + body: "This solves the diff-filter bug.\n\nCloses #67", + }), + { status: 200 } + ) + ); + + const result = await verifyPrOwnership({ + prUrl: "https://github.com/acme/proj/pull/42", + expectedGithubHandle: "alice", + expectedRepoUrl: "https://github.com/acme/proj", + expectedIssueUrl: "https://github.com/acme/proj/issues/70", + }); + + expect(result).toEqual({ ok: false, reason: "issue_mismatch" }); + }); + it("returns author_mismatch when login differs", async () => { fetchMock.mockResolvedValueOnce( new Response( diff --git a/relayer/src/submission-handler.ts b/relayer/src/submission-handler.ts index f9a96b8..296b6ec 100644 --- a/relayer/src/submission-handler.ts +++ b/relayer/src/submission-handler.ts @@ -727,6 +727,7 @@ async function checkPrOwnership( prUrl: sub.prUrl, expectedGithubHandle: ownershipCtx.githubHandle, expectedRepoUrl: bountyRepoUrl, + expectedIssueUrl: ownershipCtx.githubIssueUrl, token: deps.githubToken ?? undefined, }); } catch (err) {