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
41 changes: 40 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 GitHub issue URL that the PR must explicitly close/fix/resolve. */
expectedIssueUrl?: string;
/** 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 Down Expand Up @@ -62,7 +65,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 +91,37 @@ export async function verifyPrOwnership(
return { ok: false, reason: "repo_mismatch" };
}

if (input.expectedIssueUrl && !prBodyClosesIssue(body.body ?? "", input.expectedIssueUrl)) {
return { ok: false, reason: "issue_mismatch" };
}

return { ok: true };
}

const ISSUE_URL_RE =
/^https:\/\/github\.com\/([^/]+)\/([^/]+)\/issues\/(\d+)\/?$/i;
const CLOSING_KEYWORDS_RE = /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+/i;

function prBodyClosesIssue(body: string, issueUrl: string): boolean {
const match = issueUrl.match(ISSUE_URL_RE);
if (!match) return false;

const owner = match[1]!;
const repo = match[2]!;
const issueNumber = match[3]!;
const escapedOwner = escapeRegExp(owner);
const escapedRepo = escapeRegExp(repo);
const escapedIssueNumber = escapeRegExp(issueNumber);

const references = [
`#${escapedIssueNumber}`,
`${escapedOwner}/${escapedRepo}#${escapedIssueNumber}`,
`https:\\/\\/github\\.com\\/${escapedOwner}\\/${escapedRepo}\\/issues\\/${escapedIssueNumber}\\/?`,
].join("|");

return new RegExp(`${CLOSING_KEYWORDS_RE.source}(?:${references})`, "i").test(body);
}

function escapeRegExp(value: string): string {
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
}
46 changes: 46 additions & 0 deletions packages/shared/tests/github/verify-pr-ownership.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe("verifyPrOwnership", () => {
JSON.stringify({
user: { login: "alice" },
base: { repo: { html_url: "https://github.com/acme/proj" } },
body: "Fixes #42",
}),
{ status: 200 }
)
Expand All @@ -28,6 +29,51 @@ describe("verifyPrOwnership", () => {
prUrl: "https://github.com/acme/proj/pull/42",
expectedGithubHandle: "alice",
expectedRepoUrl: "https://github.com/acme/proj",
expectedIssueUrl: "https://github.com/acme/proj/issues/42",
});

expect(result).toEqual({ ok: true });
});

it("returns issue_mismatch when PR closes a different issue", async () => {
fetchMock.mockResolvedValueOnce(
new Response(
JSON.stringify({
user: { login: "alice" },
base: { repo: { html_url: "https://github.com/acme/proj" } },
body: "Closes #70",
}),
{ status: 200 }
)
);

const result = await verifyPrOwnership({
prUrl: "https://github.com/acme/proj/pull/99",
expectedGithubHandle: "alice",
expectedRepoUrl: "https://github.com/acme/proj",
expectedIssueUrl: "https://github.com/acme/proj/issues/67",
});

expect(result).toEqual({ ok: false, reason: "issue_mismatch" });
});

it("accepts full issue URLs in closing references", async () => {
fetchMock.mockResolvedValueOnce(
new Response(
JSON.stringify({
user: { login: "alice" },
base: { repo: { html_url: "https://github.com/acme/proj" } },
body: "Resolves https://github.com/acme/proj/issues/67",
}),
{ status: 200 }
)
);

const result = await verifyPrOwnership({
prUrl: "https://github.com/acme/proj/pull/99",
expectedGithubHandle: "alice",
expectedRepoUrl: "https://github.com/acme/proj",
expectedIssueUrl: "https://github.com/acme/proj/issues/67",
});

expect(result).toEqual({ ok: true });
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