From 99ff07ec6e69bd12c6456f18b75c139fa9e200d1 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 03:54:49 -0700 Subject: [PATCH 1/2] fix(mcp): scope the static MCP token to an operator-configured repo allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GITTENSORY_MCP_TOKEN is documented (README, CLI) as an ordinary end-user credential — an alternative to `gittensory-mcp login` for any miner or maintainer. But requireRepoManageAccess/requireRepoApprovalQueueAccess unconditionally trusted every non-session identity, so any holder of that one shared secret could propose, list, and decide pending merge/close/approve actions on every repo the App is installed on, not just repos they actually maintain. Scope the static `mcp` identity to a new MCP_ACTUATION_REPO_ALLOWLIST env var (comma/whitespace `owner/repo` list, or `*`/`all` to opt back into the old unscoped behavior). Unset defaults to deny-all — an operator must explicitly opt a repo in. The `api`/`internal` static identities are untouched: unlike `mcp`, those are operator-only Worker secrets never handed to end users. Advances #1936. Closes #2253. --- src/auth/security.ts | 15 +++++++ src/env.d.ts | 4 ++ src/mcp/server.ts | 17 +++++++- test/helpers/d1.ts | 1 + test/unit/auth.test.ts | 19 ++++++++- test/unit/mcp-automation-state.test.ts | 58 ++++++++++++++++++++++++++ 6 files changed, 111 insertions(+), 3 deletions(-) diff --git a/src/auth/security.ts b/src/auth/security.ts index 3ee0f3a0a3..8f2b2d14fb 100644 --- a/src/auth/security.ts +++ b/src/auth/security.ts @@ -141,6 +141,21 @@ export function parseGitHubLoginList(value: string | undefined): Set { ); } +/** Is `repoFullName` within the operator's MCP_ACTUATION_REPO_ALLOWLIST? The static `mcp` identity is minted from + * a single shared secret (GITTENSORY_MCP_TOKEN) that is documented as an ordinary end-user CLI credential — unlike + * `api`/`internal`, it is not operator-only, so unlike those it must NOT be unconditionally trusted for every + * installed repo. Unset/empty ⇒ deny (fail closed: an operator must explicitly opt a repo in). `*`/`all` ⇒ every + * repo, an explicit escape hatch for an operator who wants the old unscoped-trust behavior. (#2253) */ +export function isMcpActuationRepoAllowed(value: string | undefined, repoFullName: string): boolean { + const entries = (value ?? "") + .split(/[\s,]+/) + .map((entry) => entry.trim().toLowerCase()) + .filter(Boolean); + if (entries.length === 0) return false; + if (entries.includes("*") || entries.includes("all")) return true; + return entries.includes(repoFullName.toLowerCase()); +} + type CookieOptions = { maxAge: number; path: string; diff --git a/src/env.d.ts b/src/env.d.ts index fd2d8b551a..83409d6f9c 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -134,6 +134,10 @@ declare global { GITTENSORY_API_TOKEN: string; GITTENSORY_MCP_TOKEN: string; INTERNAL_JOB_TOKEN: string; + /** Repos the shared GITTENSORY_MCP_TOKEN may propose/decide/manage actions on (comma/whitespace `owner/repo` + * list, or `*`/`all` for every repo). Unset ⇒ none — GITTENSORY_MCP_TOKEN is a shared, end-user-obtainable + * credential, so it must not implicitly actuate on every installed repo (#2253). */ + MCP_ACTUATION_REPO_ALLOWLIST?: string; /** Shared bearer secret required by the hosted Orb ingest collector. */ ORB_INGEST_TOKEN?: string; /** AES-256-GCM master secret for maintainer BYOK provider keys (encrypt/decrypt at rest). A Worker/self-host diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 2330a13d15..3b7b71c4f9 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -4,7 +4,7 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { RequestHandlerExtra } from "@modelcontextprotocol/sdk/shared/protocol.js"; import { ElicitResultSchema, type ServerNotification, type ServerRequest } from "@modelcontextprotocol/sdk/types.js"; import { z } from "zod"; -import { authenticatePrivateToken, extractBearerToken, type AuthIdentity } from "../auth/security"; +import { authenticatePrivateToken, extractBearerToken, isMcpActuationRepoAllowed, type AuthIdentity } from "../auth/security"; import { canLoginAccessRepo, canWatchRepo, loadControlPanelAccessScope, loadControlPanelRoleSummary, type ControlPanelAccessScope } from "../services/control-panel-roles"; import { countOpenIssues, @@ -1777,8 +1777,15 @@ export class GittensoryMcp { } // Stricter than requireRepoAccess (read): a maintainer-MANAGE gate for write actions (#784 propose-action). - // A session must own/maintain the repo (or be an operator); private-token / static identities are trusted. + // A session must own/maintain the repo (or be an operator); api/internal static identities are trusted (they + // are operator-only Worker secrets, never handed to end users). The static `mcp` identity is NOT trusted here: + // GITTENSORY_MCP_TOKEN is a shared, end-user-obtainable CLI credential, so it is scoped to an explicit + // operator-configured allowlist instead (#2253). private async requireRepoManageAccess(repoFullName: string): Promise { + if (this.identity.kind === "static" && this.identity.actor === "mcp") { + if (isMcpActuationRepoAllowed(this.env.MCP_ACTUATION_REPO_ALLOWLIST, repoFullName)) return; + throw new Error("Forbidden: this repository is not in the operator's MCP_ACTUATION_REPO_ALLOWLIST."); + } if (this.identity.kind !== "session") return; const scope = await this.loadSessionAccessScope(); if (scope.operator) return; @@ -1799,7 +1806,13 @@ export class GittensoryMcp { // Approval-queue list/decide mirrors the HTTP requireRepoWriteAccess gate: // first require repo-scoped Gittensory maintainer/owner/operator authority, then verify live GitHub write. + // See requireRepoManageAccess above: api/internal static identities are trusted; the static `mcp` identity is + // scoped to MCP_ACTUATION_REPO_ALLOWLIST instead, since GITTENSORY_MCP_TOKEN is a shared end-user credential (#2253). private async requireRepoApprovalQueueAccess(repoFullName: string): Promise { + if (this.identity.kind === "static" && this.identity.actor === "mcp") { + if (isMcpActuationRepoAllowed(this.env.MCP_ACTUATION_REPO_ALLOWLIST, repoFullName)) return; + throw new Error("Forbidden: this repository is not in the operator's MCP_ACTUATION_REPO_ALLOWLIST."); + } if (this.identity.kind !== "session") return; const scope = await this.loadSessionAccessScope(); if (scope.operator) return; diff --git a/test/helpers/d1.ts b/test/helpers/d1.ts index 40ccdee9a5..324244c8c4 100644 --- a/test/helpers/d1.ts +++ b/test/helpers/d1.ts @@ -78,6 +78,7 @@ export function createTestEnv(overrides: Partial = {}): Env { GITHUB_WEBHOOK_SECRET: "test-webhook-secret", GITHUB_APP_PRIVATE_KEY: "test-private-key", ADMIN_GITHUB_LOGINS: "jsonbored", + MCP_ACTUATION_REPO_ALLOWLIST: "*", SELFHOST_TRANSIENT_CACHE: { async get(key: string) { return transientCache.get(key) ?? null; diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index fa1f946190..6df556093d 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { completeGitHubWebOAuth, createSessionFromGitHubToken, pollGitHubDeviceFlow, startGitHubDeviceFlow, startGitHubWebOAuth } from "../../src/auth/github-oauth"; import { enforceRateLimit, RateLimiter, routeClassForPath } from "../../src/auth/rate-limit"; -import { authenticatePrivateToken, buildBrowserSessionCookie, createSessionForGitHubUser, extractCookieValue, isAuthorizedGitHubSessionLogin, revokeSession, timingSafeEqual } from "../../src/auth/security"; +import { authenticatePrivateToken, buildBrowserSessionCookie, createSessionForGitHubUser, extractCookieValue, isAuthorizedGitHubSessionLogin, isMcpActuationRepoAllowed, revokeSession, timingSafeEqual } from "../../src/auth/security"; import { createTestEnv } from "../helpers/d1"; describe("private-beta auth and rate limiting", () => { @@ -33,6 +33,23 @@ describe("private-beta auth and rate limiting", () => { await expect(authenticatePrivateToken(env, malformed.token)).resolves.toBeNull(); }); + it("scopes MCP static-token actuation to an explicit repo allowlist, denying by default (#2253)", () => { + // Unset/empty ⇒ deny (fail closed — the shared GITTENSORY_MCP_TOKEN must not implicitly actuate everywhere). + expect(isMcpActuationRepoAllowed(undefined, "owner/repo")).toBe(false); + expect(isMcpActuationRepoAllowed("", "owner/repo")).toBe(false); + expect(isMcpActuationRepoAllowed(" ", "owner/repo")).toBe(false); + // An explicitly listed repo is allowed; a sibling repo NOT listed stays denied. + expect(isMcpActuationRepoAllowed("owner/repo", "owner/repo")).toBe(true); + expect(isMcpActuationRepoAllowed("owner/repo", "owner/other")).toBe(false); + // Case-insensitive, and accepts whitespace OR comma-separated lists (matches parseGitHubLoginList's parse). + expect(isMcpActuationRepoAllowed("Owner/Repo", "owner/repo")).toBe(true); + expect(isMcpActuationRepoAllowed("owner/one,owner/two", "owner/two")).toBe(true); + expect(isMcpActuationRepoAllowed("owner/one owner/two", "owner/two")).toBe(true); + // `*`/`all` is an explicit operator opt-in to the old unscoped-trust behavior — never the unset default. + expect(isMcpActuationRepoAllowed("*", "owner/anything")).toBe(true); + expect(isMcpActuationRepoAllowed("all", "owner/anything")).toBe(true); + }); + it("handles auth helper fallbacks for cookies, login lists, and token comparison", async () => { await expect(timingSafeEqual(undefined, "expected")).resolves.toBe(false); await expect(timingSafeEqual("short", "shorter")).resolves.toBe(false); diff --git a/test/unit/mcp-automation-state.test.ts b/test/unit/mcp-automation-state.test.ts index 9d1fdd6457..e5be168237 100644 --- a/test/unit/mcp-automation-state.test.ts +++ b/test/unit/mcp-automation-state.test.ts @@ -157,6 +157,43 @@ describe("MCP gittensory_propose_action (#784)", () => { expect(await listPendingAgentActions(env, { repoFullName: "owner/repo" })).toHaveLength(0); }); + it("denies a static MCP-token caller when the repo is not in MCP_ACTUATION_REPO_ALLOWLIST (#2253)", async () => { + // GITTENSORY_MCP_TOKEN is a shared, end-user-obtainable CLI credential — unlike an explicit maintainer + // session, it must not implicitly stage actions on every repo the App happens to be installed on. + const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: undefined }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + const client = await connect(env); // default identity: { kind: "static", actor: "mcp" } + 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(/MCP_ACTUATION_REPO_ALLOWLIST/); + expect(await listPendingAgentActions(env, { repoFullName: "owner/repo" })).toHaveLength(0); + }); + + it("allows a static MCP-token caller once the repo is explicitly allowlisted, but not a sibling repo (#2253)", async () => { + const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: "owner/repo" }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + await upsertRepositoryFromGitHub(env, { name: "other", full_name: "owner/other", private: false, owner: { login: "owner" } }, 5); + const client = await connect(env); + + const allowed = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge" } }); + expect(allowed.isError).toBeFalsy(); + + const denied = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "other", pullNumber: 7, actionClass: "merge" } }); + expect(denied.isError).toBe(true); + expect(await listPendingAgentActions(env, { repoFullName: "owner/other" })).toHaveLength(0); + }); + + it("leaves the api/internal static identities unconditionally trusted (unaffected by the mcp allowlist) (#2253)", async () => { + // api/internal are operator-only Worker secrets, never handed to end users — unlike the mcp actor, they are + // NOT scoped to MCP_ACTUATION_REPO_ALLOWLIST. Confirmed here with the allowlist unset, so this only passes + // because api/internal skip that check entirely (not because the repo happens to be allowlisted). + const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: undefined }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + const client = await connect(env, { kind: "static", actor: "api" } as AuthIdentity); + const result = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge" } }); + expect(result.isError).toBeFalsy(); + }); + it("does not trust cached collaborator association without live write permission", async () => { const env = createTestEnv(); await upsertInstallation(env, { @@ -306,6 +343,27 @@ describe("MCP gittensory_decide_pending_action (#784)", () => { expect((await getPendingAgentAction(env, action.id))?.status).toBe("accepted"); }); + it("denies a static MCP-token caller from deciding a pending action when the repo is not allowlisted (#2253)", async () => { + const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: undefined }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: {}, reason: "x" }); + + const client = await connect(env); + const result = await client.callTool({ name: "gittensory_decide_pending_action", arguments: { owner: "owner", repo: "repo", id: action.id, decision: "accept" } }); + expect(result.isError).toBe(true); + expect(JSON.stringify(result)).toMatch(/MCP_ACTUATION_REPO_ALLOWLIST/); + expect((await getPendingAgentAction(env, action.id))?.status).toBe("pending"); // left untouched, not silently accepted + }); + + it("leaves the api/internal static identities unconditionally trusted for the approval queue too (#2253)", async () => { + const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: undefined }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: {}, reason: "x" }); + const client = await connect(env, { kind: "static", actor: "internal" } as AuthIdentity); + const result = await client.callTool({ name: "gittensory_decide_pending_action", arguments: { owner: "owner", repo: "repo", id: action.id, decision: "reject" } }); + expect(result.isError).toBeFalsy(); + }); + it("is repo-scoped: a guessed id from another repo's queue is not_found and left untouched", async () => { const env = createTestEnv(); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); From b3e2842099fee69e38b187ebaf751dd186f71997 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 06:10:49 -0700 Subject: [PATCH 2/2] fix(ci): repair typecheck and document MCP_ACTUATION_REPO_ALLOWLIST Fix a real CI failure and the gate's documentation blocker: - typecheck: exactOptionalPropertyTypes rejects an explicit `MCP_ACTUATION_REPO_ALLOWLIST: undefined` on a Partial arg. createTestEnv's own fixture defaults this to "*" (so unrelated tests aren't broken by the new restriction), so the two "deny by default" tests need an explicit override -- use "" instead of undefined (isMcpActuationRepoAllowed treats both identically). The two "api/internal remain trusted" tests never reach that check at all, so they can omit the key. - docs: add the MCP_ACTUATION_REPO_ALLOWLIST env var to the self-host configuration docs alongside GITTENSORY_MCP_TOKEN, with deny-by-default and */all migration guidance for operators upgrading from the unscoped behavior. --- .../routes/docs.self-hosting-configuration.tsx | 18 ++++++++++++++++++ test/unit/mcp-automation-state.test.ts | 14 ++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/apps/gittensory-ui/src/routes/docs.self-hosting-configuration.tsx b/apps/gittensory-ui/src/routes/docs.self-hosting-configuration.tsx index 7f95337977..a2339e3486 100644 --- a/apps/gittensory-ui/src/routes/docs.self-hosting-configuration.tsx +++ b/apps/gittensory-ui/src/routes/docs.self-hosting-configuration.tsx @@ -76,6 +76,24 @@ INTERNAL_JOB_TOKEN=`} Any FOO_FILE is loaded into FOO at startup. Explicit{" "} FOO wins over the file variant.

+ + GITTENSORY_MCP_TOKEN is a shared, end-user-obtainable CLI credential (the + normal alternative to gittensory-mcp login), so it must not implicitly stage + actions (merges, closes, approvals) on every repo the App happens to be installed on.{" "} + MCP_ACTUATION_REPO_ALLOWLIST scopes it to an explicit, + comma/whitespace-separated owner/repo list —{" "} + unset denies all actuation for this token. Set it to * or{" "} + all to opt back into the pre-scoping, any-repo behavior. If you already rely on{" "} + GITTENSORY_MCP_TOKEN for approval-queue actuation, set this variable after + upgrading or MCP actuation stops working. + +

GitHub API cache

diff --git a/test/unit/mcp-automation-state.test.ts b/test/unit/mcp-automation-state.test.ts index 7caa01d00a..553f8bb737 100644 --- a/test/unit/mcp-automation-state.test.ts +++ b/test/unit/mcp-automation-state.test.ts @@ -192,7 +192,10 @@ describe("MCP gittensory_propose_action (#784)", () => { it("denies a static MCP-token caller when the repo is not in MCP_ACTUATION_REPO_ALLOWLIST (#2253)", async () => { // GITTENSORY_MCP_TOKEN is a shared, end-user-obtainable CLI credential — unlike an explicit maintainer // session, it must not implicitly stage actions on every repo the App happens to be installed on. - const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: undefined }); + // createTestEnv's own default is MCP_ACTUATION_REPO_ALLOWLIST: "*" (so unrelated tests aren't broken + // by this restriction); "" overrides that back to unset (isMcpActuationRepoAllowed treats "" the same + // as undefined) to exercise the real deny-by-default behavior. + const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: "" }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); const client = await connect(env); // default identity: { kind: "static", actor: "mcp" } const result = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge" } }); @@ -219,7 +222,8 @@ describe("MCP gittensory_propose_action (#784)", () => { // api/internal are operator-only Worker secrets, never handed to end users — unlike the mcp actor, they are // NOT scoped to MCP_ACTUATION_REPO_ALLOWLIST. Confirmed here with the allowlist unset, so this only passes // because api/internal skip that check entirely (not because the repo happens to be allowlisted). - const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: undefined }); + // MCP_ACTUATION_REPO_ALLOWLIST is irrelevant here: api/internal skip that check entirely (see below). + const env = createTestEnv({}); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); const client = await connect(env, { kind: "static", actor: "api" } as AuthIdentity); const result = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge" } }); @@ -376,7 +380,8 @@ describe("MCP gittensory_decide_pending_action (#784)", () => { }); it("denies a static MCP-token caller from deciding a pending action when the repo is not allowlisted (#2253)", async () => { - const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: undefined }); + // "" overrides createTestEnv's own MCP_ACTUATION_REPO_ALLOWLIST: "*" default back to unset. + const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: "" }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: {}, reason: "x" }); @@ -388,7 +393,8 @@ describe("MCP gittensory_decide_pending_action (#784)", () => { }); it("leaves the api/internal static identities unconditionally trusted for the approval queue too (#2253)", async () => { - const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: undefined }); + // MCP_ACTUATION_REPO_ALLOWLIST is irrelevant here: api/internal skip that check entirely (see below). + const env = createTestEnv({}); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: {}, reason: "x" }); const client = await connect(env, { kind: "static", actor: "internal" } as AuthIdentity);