diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 80e49e8e48..d019990199 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -5,7 +5,7 @@ import type { RequestHandlerExtra } from "@modelcontextprotocol/sdk/shared/proto import { ElicitResultSchema, type ServerNotification, type ServerRequest } from "@modelcontextprotocol/sdk/types.js"; import { z } from "zod"; import { authenticatePrivateToken, extractBearerToken, type AuthIdentity } from "../auth/security"; -import { loadControlPanelAccessScope, loadControlPanelRoleSummary } from "../services/control-panel-roles"; +import { loadControlPanelAccessScope, loadControlPanelRoleSummary, type ControlPanelAccessScope } from "../services/control-panel-roles"; import { countOpenIssues, countOpenPullRequests, @@ -423,6 +423,8 @@ async function describeMcpUsageRequest(request: Request, telemetryMetadata: Reco } export class GittensoryMcp { + private accessScopePromise: Promise | null = null; + constructor( private readonly env: Env, private readonly identity: AuthIdentity = { kind: "static", actor: "mcp" }, @@ -816,8 +818,20 @@ export class GittensoryMcp { } } + private async requireRepoAccess(repoFullName: string): Promise { + if (await this.canAccessRepo(repoFullName)) return; + throw new Error("Forbidden: session cannot access this repository."); + } + + private loadSessionAccessScope(): Promise { + if (this.identity.kind !== "session") throw new Error("Session access scope is only available for session identities."); + this.accessScopePromise ??= loadControlPanelAccessScope(this.env, this.identity.actor); + return this.accessScopePromise; + } + private async getRepoContext(input: { owner: string; repo: string }): Promise { const fullName = `${input.owner}/${input.repo}`; + await this.requireRepoAccess(fullName); const [repo, issues, pullRequests, recentMergedPullRequests, queueCounts, queueTrends] = await Promise.all([ getRepository(this.env, fullName), listIssueSignalSample(this.env, fullName), @@ -844,6 +858,7 @@ export class GittensoryMcp { private async getBurdenForecast(input: { owner: string; repo: string }): Promise { const fullName = `${input.owner}/${input.repo}`; + await this.requireRepoAccess(fullName); const response = await loadOrComputeBurdenForecastResponse(this.env, fullName); if (!response) { return { @@ -886,9 +901,8 @@ export class GittensoryMcp { private async canAccessRepo(fullName: string): Promise { if (this.identity.kind !== "session") return true; - const [summary, repo] = await Promise.all([loadControlPanelRoleSummary(this.env, this.identity.actor), getRepository(this.env, fullName)]); - if (summary.roles.includes("operator")) return true; - const scope = await loadControlPanelAccessScope(this.env, this.identity.actor); + const [scope, repo] = await Promise.all([this.loadSessionAccessScope(), getRepository(this.env, fullName)]); + if (scope.operator) return true; const requestedRepo = fullName.toLowerCase(); if (scope.repositoryFullNames.some((name) => name.toLowerCase() === requestedRepo)) return true; return Boolean(repo && scope.accountLogins.some((login) => login.toLowerCase() === repo.owner.toLowerCase())); @@ -896,6 +910,7 @@ export class GittensoryMcp { private async getRepoOutcomePatterns(input: { owner: string; repo: string }): Promise { const fullName = `${input.owner}/${input.repo}`; + await this.requireRepoAccess(fullName); const response = await loadOrComputeRepoOutcomePatternsResponse(this.env, fullName); if (!response) { return { @@ -967,6 +982,7 @@ export class GittensoryMcp { private async explainRepoDecision(input: { login: string; owner: string; repo: string }): Promise { this.requireContributorAccess(input.login); const fullName = `${input.owner}/${input.repo}`; + await this.requireRepoAccess(fullName); const serving = await loadContributorDecisionPackForServing(this.env, input.login); if (serving.kind === "needs_refresh") { return { @@ -1017,6 +1033,7 @@ export class GittensoryMcp { } private async preflightPr(input: z.infer>): Promise { + await this.requireRepoAccess(input.repoFullName); const [repo, issues, pullRequests, bounties, issueQuality] = await Promise.all([ getRepository(this.env, input.repoFullName), listIssues(this.env, input.repoFullName), @@ -1031,6 +1048,7 @@ export class GittensoryMcp { } private async preflightLocalDiff(input: z.infer>): Promise { + await this.requireRepoAccess(input.repoFullName); const [repo, issues, pullRequests, bounties, issueQuality] = await Promise.all([ getRepository(this.env, input.repoFullName), listIssues(this.env, input.repoFullName), @@ -1046,6 +1064,7 @@ export class GittensoryMcp { private async previewScore(input: z.infer>): Promise { if (input.contributorLogin) this.requireContributorAccess(input.contributorLogin); + await this.requireRepoAccess(input.repoFullName); const [repo, snapshot, evidence] = await Promise.all([ getRepository(this.env, input.repoFullName), getOrCreateScoringModelSnapshot(this.env), @@ -1060,6 +1079,7 @@ export class GittensoryMcp { private async explainReviewRisk(input: z.infer>): Promise { if (input.contributorLogin) this.requireContributorAccess(input.contributorLogin); + await this.requireRepoAccess(input.repoFullName); const [repo, issues, pullRequests, bounties] = await Promise.all([ getRepository(this.env, input.repoFullName), listIssues(this.env, input.repoFullName), @@ -1248,6 +1268,7 @@ export class GittensoryMcp { private async analyzeLocalBranch(input: z.infer>) { this.requireContributorAccess(input.login); + await this.requireRepoAccess(input.repoFullName); const [context, repo, issues, pullRequests, recentMergedPullRequests, bounties, snapshot, issueQuality, repoManifest] = await Promise.all([ this.loadContributorFastContext(input.login), getRepository(this.env, input.repoFullName), @@ -1296,6 +1317,7 @@ export class GittensoryMcp { private async getBountyAdvisory(id: string): Promise { const bounty = await getBounty(this.env, id); if (!bounty) throw new Error("Bounty not found."); + if (!(await this.canAccessRepo(bounty.repoFullName))) throw new Error("Bounty not found."); const [repo, issue, pullRequests] = await Promise.all([ getRepository(this.env, bounty.repoFullName), getIssue(this.env, bounty.repoFullName, bounty.issueNumber), diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index 9aef275212..d3495b248c 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -3553,6 +3553,45 @@ describe("api routes", () => { expect(body).not.toContain("SECRET roadmap PR title"); expect(body).not.toContain("SECRET-LAUNCH-CODE"); expect(body).not.toContain("confidential-roadmap"); + + const allowedMcpContext = await app.request( + "/mcp", + { + method: "POST", + headers: { ...mcpHeaders(env), authorization: `Bearer ${token}` }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: "allowed-session-repo-context", + method: "tools/call", + params: { name: "gittensory_get_repo_context", arguments: { owner: "target-org", repo: "allowed" } }, + }), + }, + env, + ); + expect(allowedMcpContext.status).toBe(200); + await expect(mcpJson(allowedMcpContext)).resolves.toMatchObject({ result: { structuredContent: { repoFullName: "target-org/allowed" } } }); + + const siblingMcpContext = await app.request( + "/mcp", + { + method: "POST", + headers: { ...mcpHeaders(env), authorization: `Bearer ${token}` }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: "forbidden-session-repo-context", + method: "tools/call", + params: { name: "gittensory_get_repo_context", arguments: { owner: "target-org", repo: "secret" } }, + }), + }, + env, + ); + expect(siblingMcpContext.status).toBe(200); + const siblingMcpBody = await siblingMcpContext.text(); + expect(siblingMcpBody).toContain("Forbidden"); + expect(siblingMcpBody).toContain("session cannot access this repository"); + expect(siblingMcpBody).not.toContain("SECRET roadmap PR title"); + expect(siblingMcpBody).not.toContain("SECRET-LAUNCH-CODE"); + expect(siblingMcpBody).not.toContain("confidential-roadmap"); }); it("returns 404 for unknown repos and serves cached snapshot with freshness for known repos", async () => { diff --git a/test/unit/mcp-upstream.test.ts b/test/unit/mcp-upstream.test.ts index f194da05d1..c3aa59ad3e 100644 --- a/test/unit/mcp-upstream.test.ts +++ b/test/unit/mcp-upstream.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import { authenticatePrivateToken, createSessionForGitHubUser } from "../../src/auth/security"; -import { persistSignalSnapshot, persistUpstreamRulesetSnapshot, upsertRepositoryFromGitHub, upsertUpstreamDriftReport } from "../../src/db/repositories"; +import { persistSignalSnapshot, persistUpstreamRulesetSnapshot, upsertBounty, upsertRepositoryFromGitHub, upsertUpstreamDriftReport } from "../../src/db/repositories"; import { GittensoryMcp } from "../../src/mcp/server"; import type { UpstreamDriftReportRecord, UpstreamRulesetSnapshotRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -43,6 +43,27 @@ describe("MCP contributor access", () => { expect(payload.data).toEqual({ status: "forbidden", repoFullName: "victim/private-repo" }); expect(JSON.stringify(payload)).not.toContain("SECRET private issue"); }); + + it("does not reveal inaccessible bounty ids through advisory errors", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "private-repo", full_name: "victim/private-repo", private: true, owner: { login: "victim" }, default_branch: "main" }); + await upsertBounty(env, { + id: "secret-bounty", + repoFullName: "victim/private-repo", + issueNumber: 7, + status: "Open", + amountText: "5.0000", + sourceUrl: "contract://issues/7", + payload: { title: "SECRET bounty" }, + }); + const { token } = await createSessionForGitHubUser(env, { login: "attacker", id: 7 }); + const identity = await authenticatePrivateToken(env, token); + if (!identity || identity.kind !== "session") throw new Error("expected session identity"); + const mcp = new GittensoryMcp(env, identity) as unknown as { getBountyAdvisory(id: string): Promise }; + + await expect(mcp.getBountyAdvisory("missing-bounty")).rejects.toThrow("Bounty not found."); + await expect(mcp.getBountyAdvisory("secret-bounty")).rejects.toThrow("Bounty not found."); + }); }); describe("MCP upstream drift tool", () => {