Give everything chopin connect starts only Chopin-issued GitHub and MCP credentials, and stop a launch Chopin refuses

apps/connector/src/acp.ts

@@ -7,6 +7,7 @@ import type {
 } from "@agentclientprotocol/sdk";
 import { PROCESSES, stopBackend } from "./planner";
 import { type Sandbox, spawnIn } from "./sandbox";
+import { withoutGitHub } from "./github-token";
 import { VERSION } from "./version";
 
 /** How long a stopped agent's process group has to exit before it is killed. */
@@ -240,7 +241,8 @@ export async function runAgent(options: AgentOptions): Promise<string> {
 		app,
 		options.command,
 		options.cwd,
-		options.env ? { ...process.env, ...options.env } : undefined,
+		// Nothing of the machine's GitHub setup reaches a run's agent; its own comes in `env`.
+		{ ...withoutGitHub(process.env), ...options.env },
 		options.stderr,
 		{
 			...(options.sandbox ? { sandbox: options.sandbox } : {}),

apps/connector/src/binary.test.ts

@@ -7,6 +7,7 @@ import { Server } from "@modelcontextprotocol/sdk/server/index.js";
 import { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js";
 import { CallToolRequestSchema, ListToolsRequestSchema } from "@modelcontextprotocol/sdk/types.js";
 import { compiledChopin } from "./testing/binary";
+import { answerTokens } from "./testing/github-grant";
 import { git } from "./workspace";
 
 let scratch = await mkdtemp(join(tmpdir(), "chopin-binary-"));
@@ -181,6 +182,9 @@ test.skipIf(!binary)(
 				if (path === "/api/connector/connections") {
 					return Response.json({ token: "connection-token" });
 				}
+				// The run's GitHub token, which the binary asks for before anything starts.
+				let issued = await answerTokens(request);
+				if (issued) return issued;
 				let token = request.headers.get("authorization")?.replace(/^Bearer /, "") ?? "";
 				if (path !== "/connector/mcp" || !["connection-token", "run-token"].includes(token)) {
 					return new Response(null, { status: 401 });

apps/connector/src/checkouts.test.ts

@@ -5,7 +5,14 @@ import { join } from "node:path";
 import { spawnSync } from "node:child_process";
 import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
 import { checkouts, clonePath, clones, noCheckout, registerCheckouts, sshCause } from "./checkouts";
-import { gitCredentials, githubEnvironment, type GitHubToken } from "./github-token";
+import {
+	gitCredentials,
+	githubEnvironment,
+	githubRefusal,
+	GitHubRefused,
+	type GitHubToken,
+} from "./github-token";
+import { BOT } from "./testing/github-grant";
 import { git } from "./workspace";
 
 const TOKEN = "ghu_MachineToken0123456789";
@@ -78,6 +85,9 @@ async function localGitHub(global = "") {
 		token: TOKEN,
 		expiresAt: Date.now() + 3_600_000,
 		file,
+		config: join(scratch, "gh"),
+		identity: BOT,
+		expired: new AbortController().signal,
 		secrets: () => [TOKEN],
 		stop: async () => {},
 	};
@@ -195,13 +205,14 @@ test.skipIf(process.platform === "win32")(
 	},
 );
 
-test("work for a repository with no checkout and no GitHub token fails naming --checkout", async () => {
+test("work for a repository with no checkout fails as Chopin refused its GitHub token, or naming --checkout without a way to ask", async () => {
 	let base = await mkdtemp(join(tmpdir(), "chopin-checkouts-"));
 	try {
 		let machine = checkouts({ directory: base, registered: {} });
-		await expect(machine.resolve("org/repo", async () => undefined)).rejects.toThrow(
-			noCheckout("org/repo"),
-		);
+		await expect(machine.resolve("org/repo", async () => {
+			throw new GitHubRefused("repository-forbidden", "org/repo");
+		})).rejects.toThrow(githubRefusal("repository-forbidden", "org/repo"));
+		await expect(machine.resolve("org/repo")).rejects.toThrow(noCheckout("org/repo"));
 		expect(noCheckout("org/repo")).toContain("chopin connect --checkout <path>");
 		await expect(machine.resolve("../repo")).rejects.toThrow("invalid repository");
 	} finally {
@@ -295,10 +306,21 @@ test.skipIf(process.platform === "win32")(
 			git(root, "init", "--quiet");
 			git(root, "remote", "add", "origin", "https://github.com/Org/Repo.git");
 			let env = githubEnvironment(
-				{ token: TOKEN, expiresAt: 0, file: "/nowhere", secrets: () => [], stop: async () => {} },
+				{
+					token: TOKEN,
+					expiresAt: 0,
+					file: "/nowhere",
+					config: "/nowhere-gh",
+					identity: BOT,
+					expired: new AbortController().signal,
+					secrets: () => [],
+					stop: async () => {},
+				},
 				"Org/Repo",
 			);
-			expect(env.GH_TOKEN).toBe(TOKEN);
+			// gh reads the launch's own configuration; no token variable would win over it.
+			expect(env.GH_CONFIG_DIR).toBe("/nowhere-gh");
+			expect(env.GH_TOKEN).toBeUndefined();
 			let url = (environment: Record<string, string>, ...args: string[]) =>
 				effective(SSH_REWRITE, environment, "-C", root, "remote", "get-url", ...args);
 			expect(url({}, "--push", "origin")).toBe("git@github.com:Org/Repo.git");

apps/connector/src/checkouts.ts

@@ -114,12 +114,12 @@ export function checkouts(options: { directory: string; registered: Registered }
 	return {
 		registered: options.registered,
 		/**
-		 * The checkout for work in `repository`; `token` gives the machine's GitHub token for it,
-		 * asked for only when there is no registered checkout.
+		 * The checkout for work in `repository`; `token` gives Chopin's GitHub token for it, asked
+		 * for here only when there is no registered checkout, and rejecting when Chopin gave none.
 		 */
 		async resolve(
 			repository: string,
-			token?: () => Promise<GitHubToken | undefined>,
+			token?: () => Promise<GitHubToken>,
 		): Promise<Resolved> {
 			let key = Object.keys(options.registered).find(name =>
 				name.toLowerCase() === repository.toLowerCase()

apps/connector/src/config.ts

@@ -34,7 +34,7 @@ export type ConnectSettings = {
 	env?: Record<string, string>;
 	/** `--keep-runs`: how many finished runs' worktrees to keep, 0 for all; absent keeps 15. */
 	keepRuns?: number;
-	/** `--require-sandbox`, `--no-sandbox`, `--sandbox-host` and `--sandbox-write`. */
+	/** `--require-sandbox`, `--no-sandbox`, `--sandbox-host`, `--sandbox-write` and `--sandbox-hide`. */
 	sandbox?: SandboxSettings;
 };
 

apps/connector/src/git-host.test.ts

@@ -375,7 +375,7 @@ test("a commondir made after the connector first saw the checkout sends its git
 	expect(made.fired()).toEqual([]);
 });
 
-test("the ssh command a person chose is kept over the one a repository names, and a person's GIT_SSH_COMMAND over both", async () => {
+test("the connector's git never runs an ssh command: not the person's, not one in its environment, not ssh on PATH, and not the one a repository names", async () => {
 	let made = repository("ssh");
 	let home = join(made.root, "home");
 	mkdirSync(home);
@@ -384,22 +384,30 @@ test("the ssh command a person chose is kept over the one a repository names, an
 		join(home, ".gitconfig"),
 		`[core]\n\tsshCommand = ${join(made.root, "person-ssh")}\n`,
 	);
+	let bin = join(made.root, "bin");
+	mkdirSync(bin);
+	program(join(bin, "ssh"), `touch '${made.ran}/path'; exit 1`);
+	plain(made.repo, "config", "core.sshCommand", join(made.evil, "ssh"));
+	plain(made.repo, "config", "remote.origin.url", "git@github.com:org/repo.git");
 	let env: NodeJS.ProcessEnv = {
 		...process.env,
 		HOME: home,
 		XDG_CONFIG_HOME: join(home, ".config"),
+		PATH: `${bin}:${process.env.PATH}`,
+		SSH_AUTH_SOCK: join(made.root, "agent.sock"),
 	};
 	delete env.GIT_SSH_COMMAND;
 	// The fixture home is the person's whole configuration, whatever the runner has set.
 	delete env.GIT_CONFIG_GLOBAL;
 	delete env.GIT_CONFIG_SYSTEM;
 	await gitAsync(["-C", made.repo, "fetch", "origin"], { env }).catch(() => undefined);
-	expect(made.fired()).toEqual(["person"]);
 	program(join(made.root, "environment-ssh"), `touch '${made.ran}/environment'; exit 1`);
 	await gitAsync(["-C", made.repo, "fetch", "origin"], {
-		env: { ...env, GIT_SSH_COMMAND: join(made.root, "environment-ssh") },
+		env: { ...env, GIT_SSH_COMMAND: join(made.root, "environment-ssh"), GIT_SSH: "ssh" },
 	}).catch(() => undefined);
-	expect(made.fired()).toEqual(["environment", "person"]);
+	expect(made.fired()).toEqual([]);
+	expect(hostGitEnvironment(env)).toMatchObject({ GIT_SSH_COMMAND: "false" });
+	expect(hostGitEnvironment(env).SSH_AUTH_SOCK).toBeUndefined();
 });
 
 /** The `GIT_CONFIG_*` entries an environment holds, in order. */
@@ -437,7 +445,8 @@ test("every setting the connector's git is given: helpers reset and none of the
 	expect(hardened).toMatchObject({
 		GIT_CONFIG_NOSYSTEM: "1",
 		GIT_ATTR_NOSYSTEM: "1",
-		GIT_SSH_COMMAND: "ssh -i key",
+		// Never the person's SSH, whatever they set: GitHub is reached over https with Chopin's token.
+		GIT_SSH_COMMAND: "false",
 		GIT_DIR: join(made.repo, ".git"),
 		GIT_COMMON_DIR: join(made.repo, ".git"),
 		GIT_WORK_TREE: made.repo,

apps/connector/src/git-host.ts

@@ -21,12 +21,13 @@ import { promisify } from "node:util";
  *   own: `GIT_CONFIG_NOSYSTEM` and `GIT_CONFIG_GLOBAL` set aside the system's and the person's.
  * - **What a repository's configuration may set** for the programs git runs is replaced in the
  *   environment (`GIT_CONFIG_COUNT`, after any entries the caller gave, where they win over every
- *   configuration file as `-c` does): the hooks, the file-system monitor, the ssh command (also
- *   pinned in `GIT_SSH_COMMAND` to the person's own, which wins over any configuration), the
+ *   configuration file as `-c` does): the hooks, the file-system monitor, the ssh command, the
  *   global attributes file, remotes that are commands (`protocol.ext.allow`) and the local
  *   transport (`protocol.file.allow`), which is never allowed. Every credential helper any
  *   configuration named is reset and none is named again: what a command needs to authenticate,
  *   its caller gives in the environment (`GIT_CONFIG_*`, as `gitCredentials` does).
+ * - **No SSH.** `GIT_SSH_COMMAND` is `false`, which wins over any configuration: the connector
+ *   reaches GitHub only over https with Chopin's token, never with the person's SSH keys or agent.
  *
  * What this does not do: a repository's configuration is still read, so a filter, a proxy or
  * another setting a repository carries applies to the connector's commands as it would to anyone's.
@@ -112,21 +113,18 @@ export function repositoryAt(path: string): Repository | undefined {
 type Person = {
 	/** A file in the connector's own temporary directory: what a fetch and a checkout need. */
 	file: string;
-	/** The ssh command git would use without a repository's say. */
-	ssh: string;
 };
 
 /**
  * What a checkout and a fetch need of a person's configuration: where to fetch from and how to
- * reach it (`url.*`, `http.*`, `ssh.*`, `transfer.*`, `fetch.*`, `safe.*`), how files are written (line endings, symlinks and names,
+ * reach it (`url.*`, `http.*`, `transfer.*`, `fetch.*`, `safe.*`), how files are written (line endings, symlinks and names,
  * `core.*` of those kinds) and their own filters (`filter.*`, `lfs.*`), which is what a Git LFS
  * checkout needs. The rest, a person's aliases, editor, pager, hooks and monitor among it, is not.
  */
 const SECTIONS = new Set([
 	"url",
 	"http",
 	"https",
-	"ssh",
 	"safe",
 	"transfer",
 	"fetch",
@@ -196,7 +194,6 @@ function personConfig(env: NodeJS.ProcessEnv, repository?: Repository): Person {
 		env.XDG_CONFIG_HOME,
 		env.GIT_CONFIG_GLOBAL,
 		env.GIT_CONFIG_SYSTEM,
-		env.GIT_SSH,
 		repository?.gitDir,
 	].join("\0");
 	let known = people.get(key);
@@ -208,20 +205,18 @@ function personConfig(env: NodeJS.ProcessEnv, repository?: Repository): Person {
 	let entries = [...entriesOf("--system", reading), ...entriesOf("--global", reading)];
 	let file = join(privateDirectory(), `gitconfig-${people.size}`);
 	let kept: string[] = [];
-	let ssh = "";
 	for (let [name, value] of entries) {
 		let first = name.indexOf(".");
 		let last = name.lastIndexOf(".");
 		let section = name.slice(0, Math.max(first, 0));
-		if (name === "core.sshcommand") ssh = value;
 		if (first > 0 && (SECTIONS.has(section) || CORE.has(name))) {
 			// A section with its subsection, as git writes them, and the value quoted.
 			let subsection = first === last ? "" : ` "${quote(name.slice(first + 1, last))}"`;
 			kept.push(`[${section}${subsection}]\n\t${name.slice(last + 1)} = "${quote(value)}"\n`);
 		}
 	}
 	writeFileSync(file, kept.join(""), { mode: 0o600 });
-	let person = { file, ssh: ssh || env.GIT_SSH || "ssh" };
+	let person = { file };
 	people.set(key, person);
 	return person;
 }
@@ -257,7 +252,12 @@ export function hostGitEnvironment(
 	hardened.GIT_CONFIG_NOSYSTEM = "1";
 	hardened.GIT_CONFIG_GLOBAL = person.file;
 	hardened.GIT_ATTR_NOSYSTEM = "1";
-	if (!env.GIT_SSH_COMMAND) hardened.GIT_SSH_COMMAND = person.ssh;
+	// Never the person's SSH keys or agent: GitHub is reached over https with Chopin's token.
+	hardened.GIT_SSH_COMMAND = "false";
+	delete hardened.GIT_SSH;
+	delete hardened.SSH_AUTH_SOCK;
+	delete hardened.GIT_ASKPASS;
+	delete hardened.SSH_ASKPASS;
 	if (repository) {
 		hardened.GIT_DIR = repository.gitDir;
 		hardened.GIT_COMMON_DIR = repository.commonDir;