diff --git a/src/mcp/server.ts b/src/mcp/server.ts index dfb6212d7e..451c08a68a 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, type ControlPanelAccessScope } from "../services/control-panel-roles"; +import { canLoginAccessRepo, canWatchRepo, loadControlPanelAccessScope, loadControlPanelRoleSummary, type ControlPanelAccessScope } from "../services/control-panel-roles"; import { countOpenIssues, countOpenPullRequests, @@ -1241,6 +1241,15 @@ export class GittensoryMcp { throw new Error("Forbidden: session cannot access this repository."); } + // Issue-watch gate (#699 path B). Sessions may only watch repos they can SEE: any gittensory-tracked PUBLIC + // repo (the miner use case) or a PRIVATE repo they can access — never an arbitrary/private repo they cannot, + // so private-repo issues never fan out to them. Non-session (private-token) identities are trusted. + private async requireWatchableRepo(login: string, repoFullName: string): Promise { + if (this.identity.kind !== "session") return; + if (await canWatchRepo(this.env, login, repoFullName)) return; + throw new Error("Forbidden: session cannot watch 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); @@ -1398,11 +1407,7 @@ export class GittensoryMcp { private async canAccessRepo(fullName: string): Promise { if (this.identity.kind !== "session") return true; - 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())); + return canLoginAccessRepo(this.env, this.identity.actor, fullName); } private async getRepoOutcomePatterns(input: { owner: string; repo: string }): Promise { @@ -1559,6 +1564,7 @@ export class GittensoryMcp { let changed: string | undefined; if (input.action === "watch" || input.action === "unwatch") { if (!input.repoFullName) return { summary: `${input.action} requires repoFullName.`, data: {} }; + await this.requireWatchableRepo(input.login, input.repoFullName); if (input.action === "watch") { await upsertIssueWatchSubscription(this.env, { login: input.login, repoFullName: input.repoFullName, labels: input.labels }); changed = `watching ${input.repoFullName}${input.labels && input.labels.length > 0 ? ` (labels: ${input.labels.join(", ")})` : ""}`; diff --git a/src/notifications/service.ts b/src/notifications/service.ts index ba939b7a59..df649f8436 100644 --- a/src/notifications/service.ts +++ b/src/notifications/service.ts @@ -2,12 +2,14 @@ import { sanitizePublicComment } from "../github/commands"; import { countRecentNotificationDeliveries, getNotificationDeliveryById, + getRepository, insertNotificationDeliveryIfAbsent, listIssueWatchersForRepo, listNotificationSubscriptionsForLogin, markNotificationDeliveryDelivered, } from "../db/repositories"; import { isGrabbableHighMultiplierIssue } from "../signals/engine"; +import { canLoginAccessRepo } from "../services/control-panel-roles"; import type { DetectedNotificationEvent, IssueRecord, NotificationChannel, NotificationDeliveryRecord, NotificationSubscriptionRecord } from "../types"; import { nowIso } from "../utils/json"; @@ -76,21 +78,34 @@ export async function detectIssueWatchEvents(env: Env, repoFullName: string, iss const detectedAt = nowIso(); const issueLabels = new Set(issue.labels.map((label) => label.toLowerCase().trim())); const authorLogin = issue.authorLogin?.toLowerCase(); - return watchers + const matching = watchers // An empty label filter matches any issue; otherwise at least one watched label must be present. .filter((watcher) => watcher.labels.length === 0 || watcher.labels.some((label) => issueLabels.has(label))) // Don't ping the maintainer who opened the issue about their own issue. - .filter((watcher) => watcher.login.toLowerCase() !== authorLogin) - .map((watcher) => ({ - eventType: "issue_watch_match" as const, - recipientLogin: watcher.login, - repoFullName, - pullNumber: issue.number, // carries the ISSUE number for this eventType - dedupKey: `issue_watch_match:${repoFullName}#${issue.number}:${watcher.login.toLowerCase()}`, - deeplink: `https://github.com/${repoFullName}/issues/${issue.number}`, - actorLogin: issue.authorLogin ?? "unknown", - detectedAt, - })); + .filter((watcher) => watcher.login.toLowerCase() !== authorLogin); + + // Access gate: a gittensory-tracked PUBLIC repo fans out to every matching watcher (the miner use case); + // a PRIVATE — or untracked/unknown — repo only to watchers who can access it, so private-repo issues never + // reach a non-collaborator. The repo is the same for all watchers, so resolve it once and only pay the + // per-watcher access check on the private path. + const repo = await getRepository(env, repoFullName); + const authorizedWatchers = + repo && !repo.isPrivate + ? matching + : (await Promise.all(matching.map(async (watcher) => ((repo && (await canLoginAccessRepo(env, watcher.login, repoFullName))) ? watcher : null)))).filter( + (watcher) => watcher !== null, + ); + + return authorizedWatchers.map((watcher) => ({ + eventType: "issue_watch_match" as const, + recipientLogin: watcher.login, + repoFullName, + pullNumber: issue.number, // carries the ISSUE number for this eventType + dedupKey: `issue_watch_match:${repoFullName}#${issue.number}:${watcher.login.toLowerCase()}`, + deeplink: `https://github.com/${repoFullName}/issues/${issue.number}`, + actorLogin: issue.authorLogin ?? "unknown", + detectedAt, + })); } function rateLimitWindowStart(now: string): string { diff --git a/src/services/control-panel-roles.ts b/src/services/control-panel-roles.ts index 22dc95df61..e143cfe5c5 100644 --- a/src/services/control-panel-roles.ts +++ b/src/services/control-panel-roles.ts @@ -1,5 +1,5 @@ import { isAuthorizedGitHubSessionLogin } from "../auth/security"; -import { getFreshOfficialMinerDetection, listAllPullRequests, listInstallations, listRepositories } from "../db/repositories"; +import { getFreshOfficialMinerDetection, getRepository, listAllPullRequests, listInstallations, listRepositories } from "../db/repositories"; import type { ControlPanelRoleCard, ControlPanelRoleName, ControlPanelRoleSummary, InstallationRecord, PullRequestRecord, RepositoryRecord } from "../types"; import { nowIso } from "../utils/json"; @@ -33,6 +33,25 @@ export async function loadControlPanelAccessScope(env: Env, login: string): Prom }); } +export async function canLoginAccessRepo(env: Env, login: string, fullName: string): Promise { + const [scope, repo] = await Promise.all([loadControlPanelAccessScope(env, login), getRepository(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((accountLogin) => accountLogin.toLowerCase() === repo.owner.toLowerCase())); +} + +// Whether `login` may watch `fullName`'s issues. Issue-watch (#699 path B) is a MINER feature: miners watch +// PUBLIC gittensor-tracked repos they don't own or maintain, so a tracked public repo is watchable by any +// contributor. A PRIVATE repo is gated to maintainer/owner/operator scope so its issues never fan out to a +// non-collaborator. An untracked repo (unknown visibility) is treated as not watchable (fail-closed). +export async function canWatchRepo(env: Env, login: string, fullName: string): Promise { + const repo = await getRepository(env, fullName); + if (!repo) return false; + if (!repo.isPrivate) return true; + return canLoginAccessRepo(env, login, fullName); +} + export async function loadControlPanelRoleSummary(env: Env, login: string): Promise { const [miner, repositories, installations, pullRequests] = await Promise.all([ getFreshOfficialMinerDetection(env, login).catch(() => null), diff --git a/test/unit/issue-watch.test.ts b/test/unit/issue-watch.test.ts index a8dc1cc204..171faf05bb 100644 --- a/test/unit/issue-watch.test.ts +++ b/test/unit/issue-watch.test.ts @@ -8,6 +8,7 @@ import { listIssueWatchSubscriptionsForLogin, listIssueWatchersForRepo, upsertIssueWatchSubscription, + upsertRepositoryFromGitHub, } from "../../src/db/repositories"; import { isGrabbableHighMultiplierIssue } from "../../src/signals/engine"; import { buildIssueWatchNotification, buildNotificationContent, detectIssueWatchEvents } from "../../src/notifications/service"; @@ -54,6 +55,8 @@ describe("issue-watch subscriptions (CRUD)", () => { describe("detectIssueWatchEvents", () => { it("fans out one event per matching watcher, skips the author, honours the label filter", async () => { const env = createTestEnv(); + // owner/repo is a tracked PUBLIC repo, so any contributor may watch it (the miner use case). + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" }, default_branch: "main" }, 100); await upsertIssueWatchSubscription(env, { login: "alice", repoFullName: "owner/repo" }); // any label await upsertIssueWatchSubscription(env, { login: "bob", repoFullName: "owner/repo", labels: ["bug"] }); // bug only await upsertIssueWatchSubscription(env, { login: "maintainer", repoFullName: "owner/repo" }); // the issue's author @@ -78,8 +81,32 @@ describe("detectIssueWatchEvents", () => { expect(await detectIssueWatchEvents(env, "unwatched/repo", issue())).toEqual([]); // no watchers }); + + it("filters legacy watchers that no longer have repository access", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "private", full_name: "victim/private", private: true, owner: { login: "victim" }, default_branch: "main" }, 123); + await upsertIssueWatchSubscription(env, { login: "attacker", repoFullName: "victim/private" }); + + await expect(detectIssueWatchEvents(env, "victim/private", issue({ repoFullName: "victim/private", number: 77 }))).resolves.toEqual([]); + }); + + it("does not fan out for an untracked repo even if it has watchers (fail-closed on unknown visibility)", async () => { + const env = createTestEnv(); + await upsertIssueWatchSubscription(env, { login: "alice", repoFullName: "ghost/repo" }); // repo never upserted + await expect(detectIssueWatchEvents(env, "ghost/repo", issue({ repoFullName: "ghost/repo", number: 88 }))).resolves.toEqual([]); + }); + + it("still fans out a PRIVATE-repo issue to a watcher who can access it", async () => { + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "operator" }); // operator has access to any repo + await upsertRepositoryFromGitHub(env, { name: "private", full_name: "acme/private", private: true, owner: { login: "acme" }, default_branch: "main" }, 555); + await upsertIssueWatchSubscription(env, { login: "operator", repoFullName: "acme/private" }); + const events = await detectIssueWatchEvents(env, "acme/private", issue({ repoFullName: "acme/private", number: 90, authorLogin: "acme" })); + expect(events.map((event) => event.recipientLogin)).toEqual(["operator"]); + }); + it("handles an issue with no recorded author (actor falls back to 'unknown', no one is skipped)", async () => { const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" }, default_branch: "main" }, 100); await upsertIssueWatchSubscription(env, { login: "alice", repoFullName: "owner/repo" }); const events = await detectIssueWatchEvents(env, "owner/repo", issue({ number: 12, authorLogin: undefined, authorAssociation: "MEMBER" })); expect(events).toHaveLength(1); @@ -134,6 +161,32 @@ describe("MCP gittensory_watch_issues", () => { expect((unwatched.structuredContent as { watching: unknown[] }).watching).toHaveLength(0); }); + + it("lets a session watch a tracked PUBLIC repo it does not maintain (the miner use case)", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" }, default_branch: "main" }, 100); + const { session } = await createSessionForGitHubUser(env, { login: "miner", id: 1 }); + const client = await connect(env, { kind: "session", actor: "miner", session }); + + const result = await client.callTool({ name: "gittensory_watch_issues", arguments: { login: "miner", action: "watch", repoFullName: "owner/repo" } }); + + expect(result.isError).toBeFalsy(); + expect((result.structuredContent as { watching: Array<{ repoFullName: string }> }).watching).toEqual([{ repoFullName: "owner/repo", labels: [] }]); + }); + + it("blocks session actors from watching inaccessible (private) repositories", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "private", full_name: "victim/private", private: true, owner: { login: "victim" }, default_branch: "main" }, 321); + const { session } = await createSessionForGitHubUser(env, { login: "miner", id: 1 }); + const client = await connect(env, { kind: "session", actor: "miner", session }); + + const result = await client.callTool({ name: "gittensory_watch_issues", arguments: { login: "miner", action: "watch", repoFullName: "victim/private" } }); + + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toContain("session cannot watch this repository"); + await expect(listIssueWatchSubscriptionsForLogin(env, "miner")).resolves.toEqual([]); + }); + it("is self-scoped: a session cannot manage another login's watches", async () => { const env = createTestEnv(); const { session } = await createSessionForGitHubUser(env, { login: "miner", id: 1 });