Skip to content
Merged
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
2 changes: 1 addition & 1 deletion src/api/routes.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Hono, type Context } from "hono";

Check notice on line 1 in src/api/routes.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 1 in src/api/routes.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 1 in src/api/routes.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
import { z } from "zod";
import { analyzePRQueue, type AuthorRole, type ChecksStatus } from "../queue-intelligence";
import { completeGitHubWebOAuth, createSessionFromGitHubToken, pollGitHubDeviceFlow, startGitHubDeviceFlow, startGitHubWebOAuth } from "../auth/github-oauth";
Expand Down Expand Up @@ -2030,7 +2030,7 @@
const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`;
const decision = c.req.param("decision");
if (decision !== "accept" && decision !== "reject") return c.json({ error: "invalid_decision", detail: "decision must be 'accept' or 'reject'" }, 400);
const gate = await requireRepoMaintainer(c, fullName);
const gate = await requireRepoWriteAccess(c, fullName);
/* v8 ignore next -- unauthorized requests are rejected by the auth middleware before reaching the handler. */
if (gate instanceof Response) return gate;
const pending = await getPendingAgentAction(c.env, c.req.param("id"));
Expand Down
22 changes: 20 additions & 2 deletions src/mcp/server.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createMcpHandler } from "agents/mcp";

Check notice on line 1 in src/mcp/server.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 1 in src/mcp/server.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 1 in src/mcp/server.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
import type { Context } from "hono";
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import type { RequestHandlerExtra } from "@modelcontextprotocol/sdk/shared/protocol.js";
Expand Down Expand Up @@ -43,6 +43,7 @@
} from "../db/repositories";
import { buildNotificationFeed } from "../notifications/service";
import { contributorRepoStatsFromGittensor, fetchGittensorContributorSnapshot } from "../gittensor/api";
import { getRepositoryCollaboratorPermission } from "../github/app";
import { fetchPublicContributorProfile } from "../github/public";
import { listLatestRegistrySnapshots } from "../registry/sync";
import { getOrCreateScoringModelSnapshot, isTimeDecayEnabled } from "../scoring/model";
Expand Down Expand Up @@ -336,6 +337,11 @@
mergeMethod: z.enum(["merge", "squash", "rebase"]).optional(),
closeComment: z.string().max(60000).optional(),
};

// GitHub permissions that imply real write access to a repo. Cached PR author_association can report
// MEMBER/COLLABORATOR for users without push permission, so write-capable MCP surfaces must verify live.
const REPO_WRITE_PERMISSIONS = new Set(["admin", "maintain", "write"]);

const proposeActionOutputSchema = {
created: z.boolean().optional(),
action: z
Expand Down Expand Up @@ -1520,8 +1526,20 @@
private async requireRepoManageAccess(repoFullName: string): Promise<void> {
if (this.identity.kind !== "session") return;
const scope = await this.loadSessionAccessScope();
if (scope.operator || scope.repositoryFullNames.includes(repoFullName)) return;
throw new Error("Forbidden: maintainer access is required to propose an action on this repository.");
if (scope.operator) return;

const repo = await getRepository(this.env, repoFullName);
const installationId = repo?.installationId ?? null;
let permission: string | null = null;
if (installationId !== null) {
try {
permission = await getRepositoryCollaboratorPermission(this.env, installationId, repoFullName, this.identity.actor);
} catch {
permission = null;
}
}
if (permission && REPO_WRITE_PERMISSIONS.has(permission)) return;
throw new Error("Forbidden: write access is required to propose an action on this repository.");
}

// Issue-watch gate (#699 path B). Sessions may only watch repos they can SEE: any gittensory-tracked PUBLIC
Expand Down
40 changes: 36 additions & 4 deletions test/unit/mcp-automation-state.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
import { Client } from "@modelcontextprotocol/sdk/client/index.js";

Check notice on line 1 in test/unit/mcp-automation-state.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 1 in test/unit/mcp-automation-state.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 1 in test/unit/mcp-automation-state.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
import { describe, expect, it } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { GittensoryMcp } from "../../src/mcp/server";
import { createPendingAgentActionIfAbsent, listPendingAgentActions, upsertInstallation, upsertRepositoryFromGitHub, upsertRepositorySettings } from "../../src/db/repositories";
import { getRepositoryCollaboratorPermission } from "../../src/github/app";
import { createPendingAgentActionIfAbsent, listPendingAgentActions, upsertInstallation, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub, upsertRepositorySettings } from "../../src/db/repositories";
import type { AuthIdentity } from "../../src/auth/security";
import { createTestEnv } from "../helpers/d1";

vi.mock("../../src/github/app", async (importOriginal) => ({
...(await importOriginal<typeof import("../../src/github/app")>()),
getRepositoryCollaboratorPermission: vi.fn(),
}));
const mockedPermission = vi.mocked(getRepositoryCollaboratorPermission);

beforeEach(() => {
mockedPermission.mockReset();
mockedPermission.mockResolvedValue("write");
});

async function connect(env: Env, identity?: AuthIdentity) {
const server = (identity ? new GittensoryMcp(env, identity) : new GittensoryMcp(env)).createServer();
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
Expand Down Expand Up @@ -118,13 +130,33 @@
expect(JSON.stringify(result)).toMatch(/not installed/i);
});

it("forbids a session without maintainer access to the repo", async () => {
it("forbids a session without live GitHub write access to the repo", async () => {
const env = createTestEnv();
await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5);
mockedPermission.mockResolvedValue("read");
const client = await connect(env, { kind: "session", actor: "rando" } as AuthIdentity);
const result = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge" } });
expect(result.isError).toBe(true);
expect(JSON.stringify(result)).toMatch(/maintainer access/i);
expect(JSON.stringify(result)).toMatch(/write access/i);
expect(await listPendingAgentActions(env, { repoFullName: "owner/repo" })).toHaveLength(0);
});

it("does not trust cached collaborator association without live write permission", async () => {
const env = createTestEnv();
await upsertInstallation(env, {
installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write" }, events: ["pull_request"] },
repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }],
});
await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5);
await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "x", state: "open", user: { login: "reader" }, author_association: "COLLABORATOR", head: { sha: "sha" } });
mockedPermission.mockResolvedValue("read");

const client = await connect(env, { kind: "session", actor: "reader" } as AuthIdentity);
const result = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge" } });

expect(result.isError).toBe(true);
expect(JSON.stringify(result)).toMatch(/write access/i);
expect(mockedPermission).toHaveBeenCalledWith(env, 5, "owner/repo", "reader");
expect(await listPendingAgentActions(env, { repoFullName: "owner/repo" })).toHaveLength(0);
});
});
Loading