Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/mcp/lib/tools/submissions/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
51 changes: 50 additions & 1 deletion packages/shared/src/github/verify-pr-ownership.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand All @@ -24,6 +26,7 @@ export type VerifyPrOwnershipResult =
| "pr_not_found"
| "author_mismatch"
| "repo_mismatch"
| "issue_mismatch"
| "rate_limited"
| "invalid_url"
| "upstream_error";
Expand All @@ -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<VerifyPrOwnershipResult> {
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}
44 changes: 44 additions & 0 deletions packages/shared/tests/github/verify-pr-ownership.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions relayer/src/submission-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,7 @@ async function checkPrOwnership(
prUrl: sub.prUrl,
expectedGithubHandle: ownershipCtx.githubHandle,
expectedRepoUrl: bountyRepoUrl,
expectedIssueUrl: ownershipCtx.githubIssueUrl,
token: deps.githubToken ?? undefined,
});
} catch (err) {
Expand Down