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
46 changes: 39 additions & 7 deletions src/auth/security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,21 +141,53 @@ 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 ?? "")
/** Shared CSV/whitespace allowlist parse for the MCP repo-allowlist env vars — both the actuation (write) and
* read allowlists use the identical fail-closed/wildcard parsing, just gate a different security boundary at
* their respective call sites. */
function parseMcpRepoAllowlistEntries(value: string | undefined): string[] {
return (value ?? "")
.split(/[\s,]+/)
.map((entry) => entry.trim().toLowerCase())
.filter(Boolean);
}

/** Does an allowlist value grant `repoFullName`? Unset/empty ⇒ deny (fail closed). `*`/`all` ⇒ every repo, an
* explicit escape hatch for an operator who wants unscoped trust. */
function matchesMcpRepoAllowlist(value: string | undefined, repoFullName: string): boolean {
const entries = parseMcpRepoAllowlistEntries(value);
if (entries.length === 0) return false;
if (entries.includes("*") || entries.includes("all")) return true;
return entries.includes(repoFullName.toLowerCase());
}

/** 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 {
return matchesMcpRepoAllowlist(value, repoFullName);
}

/** Is `repoFullName` within the operator's MCP_READ_REPO_ALLOWLIST? Same fail-closed/wildcard model as
* isMcpActuationRepoAllowed, kept as a SEPARATE allowlist so an operator can grant broad read access without
* also granting actuation (merge/close/approve) trust, or the reverse. Gates the static `mcp` identity's
* read-only MCP tools: repo context, issue quality, watch subscriptions, and (via isMcpReadUnscoped below) the
* non-repo-scoped contributor/operator tools. (#2455) */
export function isMcpReadRepoAllowed(value: string | undefined, repoFullName: string): boolean {
return matchesMcpRepoAllowlist(value, repoFullName);
}

/** Is MCP_READ_REPO_ALLOWLIST set to the full `*`/`all` wildcard? Contributor-login-scoped tools (another
* contributor's decision pack/profile/notifications) and operator-scoped tools (fleet analytics) have no single
* repo to check a scoped allowlist entry against, so — unlike the repo-scoped read tools above — they only
* unlock for the static `mcp` identity via the full wildcard opt-in: a repo-scoped allowlist does not imply a
* right to read an ARBITRARY other contributor's private data or cross-instance operator-only analytics. (#2455) */
export function isMcpReadUnscoped(value: string | undefined): boolean {
const entries = parseMcpRepoAllowlistEntries(value);
return entries.includes("*") || entries.includes("all");
}

type CookieOptions = {
maxAge: number;
path: string;
Expand Down
5 changes: 5 additions & 0 deletions src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ declare global {
* 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;
/** Repos the shared GITTENSORY_MCP_TOKEN may READ via MCP tools (repo context, issue quality, watch
* subscriptions) — comma/whitespace `owner/repo` list, or `*`/`all` for every repo AND for the
* non-repo-scoped contributor/operator tools (another contributor's private data, fleet analytics). Unset
* ⇒ none. A separate allowlist from MCP_ACTUATION_REPO_ALLOWLIST so read and write trust can differ (#2455). */
MCP_READ_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
40 changes: 32 additions & 8 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, isMcpActuationRepoAllowed, type AuthIdentity } from "../auth/security";
import { authenticatePrivateToken, extractBearerToken, isMcpActuationRepoAllowed, isMcpReadRepoAllowed, isMcpReadUnscoped, type AuthIdentity } from "../auth/security";
import { canLoginAccessRepo, canWatchRepo, loadControlPanelAccessScope, loadControlPanelRoleSummary, type ControlPanelAccessScope } from "../services/control-panel-roles";
import {
countOpenIssues,
Expand Down Expand Up @@ -1770,6 +1770,13 @@ export class GittensoryMcp {
if (this.identity.kind === "session" && this.identity.actor.toLowerCase() !== login.toLowerCase()) {
throw new Error("Forbidden: session can only access the authenticated GitHub login.");
}
// The static `mcp` identity must not read an ARBITRARY other contributor's private decision pack, profile,
// or notifications by default — GITTENSORY_MCP_TOKEN is a shared, end-user-obtainable CLI credential, not an
// operator-only secret (see requireRepoManageAccess). There is no per-login allowlist, so only the full
// MCP_READ_REPO_ALLOWLIST wildcard opt-in unlocks this, matching requireOperatorAccess below. (#2455)
if (this.identity.kind === "static" && this.identity.actor === "mcp" && !isMcpReadUnscoped(this.env.MCP_READ_REPO_ALLOWLIST)) {
throw new Error("Forbidden: this MCP token is not authorized to read another contributor's data.");
}
}

private async requireRepoAccess(repoFullName: string): Promise<void> {
Expand Down Expand Up @@ -1842,6 +1849,9 @@ export class GittensoryMcp {
// 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.
// Its only caller (watchIssues) already gates the static `mcp` identity via requireContributorAccess's
// unscoped-MCP_READ_REPO_ALLOWLIST-wildcard-only check first, which is strictly stronger than any repo-scoped
// check this function could add — a static mcp caller can only ever reach here already fully trusted. (#2455)
private async requireWatchableRepo(login: string, repoFullName: string): Promise<void> {
if (this.identity.kind !== "session") return;
if (await canWatchRepo(this.env, login, repoFullName)) return;
Expand Down Expand Up @@ -2031,8 +2041,15 @@ export class GittensoryMcp {
}

private async canAccessRepo(fullName: string): Promise<boolean> {
if (this.identity.kind !== "session") return true;
return canLoginAccessRepo(this.env, this.identity.actor, fullName);
if (this.identity.kind === "session") return canLoginAccessRepo(this.env, this.identity.actor, fullName);
// The static `mcp` identity is a shared, end-user-obtainable CLI credential — scope it to the operator's
// MCP_READ_REPO_ALLOWLIST instead of trusting it for every installed repo, mirroring requireRepoManageAccess's
// MCP_ACTUATION_REPO_ALLOWLIST scoping for writes. api/internal static identities remain trusted (operator-only
// Worker secrets, never handed to end users). (#2455)
if (this.identity.kind === "static" && this.identity.actor === "mcp") {
return isMcpReadRepoAllowed(this.env.MCP_READ_REPO_ALLOWLIST, fullName);
}
return true;
}

private async getRepoOutcomePatterns(input: { owner: string; repo: string }): Promise<ToolPayload> {
Expand Down Expand Up @@ -2065,12 +2082,19 @@ export class GittensoryMcp {
}

// Operator-only gate: the fleet view aggregates ALL self-hosters' calibration, so a session must be an
// operator; private-token / static identities are trusted (the same model as the other measurement tools).
// operator. api/internal static identities are trusted (operator-only Worker secrets). The static `mcp`
// identity is NOT trusted by default — it is a shared, end-user-obtainable CLI credential, and fleet analytics
// has no single repo to scope a MCP_READ_REPO_ALLOWLIST entry against, so only the full wildcard opt-in
// (mirroring requireContributorAccess) unlocks it. (#2455)
private async requireOperatorAccess(): Promise<void> {
if (this.identity.kind !== "session") return;
const scope = await this.loadSessionAccessScope();
if (scope.operator) return;
throw new Error("Forbidden: operator authority is required for fleet analytics.");
if (this.identity.kind === "session") {
const scope = await this.loadSessionAccessScope();
if (scope.operator) return;
throw new Error("Forbidden: operator authority is required for fleet analytics.");
}
if (this.identity.kind === "static" && this.identity.actor === "mcp" && !isMcpReadUnscoped(this.env.MCP_READ_REPO_ALLOWLIST)) {
throw new Error("Forbidden: this MCP token is not authorized for operator-only fleet analytics.");
}
}

private async getFleetAnalytics(input: { windowDays?: number | undefined }): Promise<ToolPayload> {
Expand Down
1 change: 1 addition & 0 deletions test/helpers/d1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export function createTestEnv(overrides: Partial<Env> = {}): Env {
GITHUB_APP_PRIVATE_KEY: "test-private-key",
ADMIN_GITHUB_LOGINS: "jsonbored",
MCP_ACTUATION_REPO_ALLOWLIST: "*",
MCP_READ_REPO_ALLOWLIST: "*",
SELFHOST_TRANSIENT_CACHE: {
async get(key: string) {
return transientCache.get(key) ?? null;
Expand Down
10 changes: 10 additions & 0 deletions test/unit/issue-watch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,4 +216,14 @@ describe("MCP gittensory_watch_issues", () => {
expect(result.isError).toBe(true);
expect(JSON.stringify(result.content)).toContain("authenticated GitHub login");
});

// Regression test for #2455: the shared, end-user-obtainable GITTENSORY_MCP_TOKEN must not manage an
// ARBITRARY login's watch subscriptions by default. "" overrides createTestEnv's own
// MCP_READ_REPO_ALLOWLIST: "*" default back to unset, exercising the real deny-by-default behavior.
it("forbids the static mcp identity without an MCP_READ_REPO_ALLOWLIST wildcard opt-in (#2455)", async () => {
const client = await connect(createTestEnv({ MCP_READ_REPO_ALLOWLIST: "" }));
const result = await client.callTool({ name: "gittensory_watch_issues", arguments: { login: "miner", action: "list" } });
expect(result.isError).toBe(true);
expect(JSON.stringify(result.content)).toMatch(/not authorized to read another contributor's data/i);
});
});
15 changes: 15 additions & 0 deletions test/unit/mcp-fleet-analytics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,19 @@ describe("gittensory_get_fleet_analytics MCP tool", () => {
expect(result.isError).toBeTruthy();
expect(JSON.stringify(result.content)).toMatch(/operator authority/i);
});

// Regression test for #2455: the shared, end-user-obtainable GITTENSORY_MCP_TOKEN must not read
// cross-instance operator-only fleet analytics by default. "" overrides createTestEnv's own
// MCP_READ_REPO_ALLOWLIST: "*" default back to unset, exercising the real deny-by-default behavior.
it("forbids the static mcp identity without an MCP_READ_REPO_ALLOWLIST wildcard opt-in (#2455)", async () => {
const result = await (await connect(createTestEnv({ MCP_READ_REPO_ALLOWLIST: "" }))).callTool({ name: "gittensory_get_fleet_analytics", arguments: {} });
expect(result.isError).toBeTruthy();
expect(JSON.stringify(result.content)).toMatch(/not authorized for operator-only fleet analytics/i);
});

it("forbids the static mcp identity when MCP_READ_REPO_ALLOWLIST is scoped to specific repos, not the wildcard (#2455)", async () => {
const result = await (await connect(createTestEnv({ MCP_READ_REPO_ALLOWLIST: "acme/widgets" }))).callTool({ name: "gittensory_get_fleet_analytics", arguments: {} });
expect(result.isError).toBeTruthy();
expect(JSON.stringify(result.content)).toMatch(/not authorized for operator-only fleet analytics/i);
});
});
27 changes: 27 additions & 0 deletions test/unit/mcp-output-schemas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,33 @@ describe("MCP tool calls return schema-valid structured content", () => {
expect(data.repoFullName).toBe("octo/demo");
});

// Regression test for #2455: api/internal static identities are operator-only Worker secrets (never handed to
// end users, unlike the shared GITTENSORY_MCP_TOKEN), so canAccessRepo must remain unconditionally trusted for
// them even with MCP_READ_REPO_ALLOWLIST unset — mirroring the existing api/internal-trusted tests for the
// write-side MCP_ACTUATION_REPO_ALLOWLIST guards.
it("gittensory_get_repo_context trusts the api static identity unconditionally, regardless of MCP_READ_REPO_ALLOWLIST (#2455)", async () => {
const { client } = await connectTestClient(createTestEnv({ MCP_READ_REPO_ALLOWLIST: "" }), { kind: "static", actor: "api" });
const result = await client.callTool({ name: "gittensory_get_repo_context", arguments: { owner: "octo", repo: "demo" } });
expect(result.isError).toBeFalsy();
const data = result.structuredContent as Record<string, unknown>;
expect(data.repoFullName).toBe("octo/demo");
});

// Regression test for #2455: the shared, end-user-obtainable GITTENSORY_MCP_TOKEN must not read an arbitrary
// repo's context by default.
it("gittensory_get_repo_context forbids the static mcp identity without an MCP_READ_REPO_ALLOWLIST wildcard/scoped opt-in (#2455)", async () => {
const { client } = await connectTestClient(createTestEnv({ MCP_READ_REPO_ALLOWLIST: "" }));
const result = await client.callTool({ name: "gittensory_get_repo_context", arguments: { owner: "octo", repo: "demo" } });
expect(result.isError).toBe(true);
expect(JSON.stringify(result.content)).toMatch(/cannot access this repository/i);
});

it("gittensory_get_repo_context allows the static mcp identity once the repo is explicitly allowlisted (#2455)", async () => {
const { client } = await connectTestClient(createTestEnv({ MCP_READ_REPO_ALLOWLIST: "octo/demo" }));
const result = await client.callTool({ name: "gittensory_get_repo_context", arguments: { owner: "octo", repo: "demo" } });
expect(result.isError).toBeFalsy();
});

it("gittensory_get_maintainer_noise returns a structured noise triage report for a repo", async () => {
const env = createTestEnv();
await upsertRepositoryFromGitHub(env, { name: "demo", full_name: "octo/demo", private: false, owner: { login: "octo" }, default_branch: "main" });
Expand Down
Loading