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
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,24 @@ INTERNAL_JOB_TOKEN=<random-32-byte-token>`}
Any <code>FOO_FILE</code> is loaded into <code>FOO</code> at startup. Explicit{" "}
<code>FOO</code> wins over the file variant.
</p>
<Callout variant="warn" title="MCP_ACTUATION_REPO_ALLOWLIST">
<code>GITTENSORY_MCP_TOKEN</code> is a shared, end-user-obtainable CLI credential (the
normal alternative to <code>gittensory-mcp login</code>), so it must not implicitly stage
actions (merges, closes, approvals) on every repo the App happens to be installed on.{" "}
<code>MCP_ACTUATION_REPO_ALLOWLIST</code> scopes it to an explicit,
comma/whitespace-separated <code>owner/repo</code> list —{" "}
<strong>unset denies all actuation</strong> for this token. Set it to <code>*</code> or{" "}
<code>all</code> to opt back into the pre-scoping, any-repo behavior. If you already rely on{" "}
<code>GITTENSORY_MCP_TOKEN</code> for approval-queue actuation, set this variable after
upgrading or MCP actuation stops working.
</Callout>
<CodeBlock
filename=".env"
code={`# Deny-by-default: unset means the static MCP token cannot stage or decide any action.
MCP_ACTUATION_REPO_ALLOWLIST=owner/repo-one, owner/repo-two
# Restore pre-upgrade any-repo behavior:
# MCP_ACTUATION_REPO_ALLOWLIST=*`}
/>

<h2>GitHub API cache</h2>
<p>
Expand Down
15 changes: 15 additions & 0 deletions src/auth/security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ export function parseGitHubLoginList(value: string | undefined): Set<string> {
);
}

/** 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;
Expand Down
4 changes: 4 additions & 0 deletions src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 15 additions & 2 deletions src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1778,8 +1778,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<void> {
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;
Expand All @@ -1800,7 +1807,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<void> {
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;
Expand Down
1 change: 1 addition & 0 deletions test/helpers/d1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export function createTestEnv(overrides: Partial<Env> = {}): 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;
Expand Down
19 changes: 18 additions & 1 deletion test/unit/auth.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down Expand Up @@ -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);
Expand Down
64 changes: 64 additions & 0 deletions test/unit/mcp-automation-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,47 @@ 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.
// 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" } });
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).
// 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" } });
expect(result.isError).toBeFalsy();
});

it("does not trust cached collaborator association without live write permission", async () => {
const env = createTestEnv();
await upsertInstallation(env, {
Expand Down Expand Up @@ -338,6 +379,29 @@ 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 () => {
// "" 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" });

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 () => {
// 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);
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);
Expand Down
Loading