fix(repo-mode): prevent unsafe dir defaults and forced pushes (#85)

README.md

@@ -132,6 +132,7 @@ gitagent --repo https://github.com/org/repo "Add unit tests"
 | `--repo <url>` | `-r` | GitHub repo URL to clone and work on |
 | `--pat <token>` | | GitHub PAT (or set `GITHUB_TOKEN` / `GIT_TOKEN`) |
 | `--session <branch>` | | Resume an existing session branch |
+| `--read-only` | | With `--repo`: clone/checkout only, never commit or push |
 | `--model <provider:model>` | `-m` | Override model (e.g. `anthropic:claude-sonnet-4-5-20250929`) |
 | `--sandbox` | `-s` | Run in sandbox VM |
 | `--prompt <text>` | `-p` | Single-shot prompt (skip REPL) |

src/index.ts

@@ -43,7 +43,7 @@ const green = (s: string) => `\x1b[32m${s}\x1b[0m`;
 
 interface ParsedArgs {
 	model?: string;
-	dir: string;
+	dir?: string;
 	prompt?: string;
 	env?: string;
 	sandbox?: boolean;
@@ -52,13 +52,16 @@ interface ParsedArgs {
 	repo?: string;
 	pat?: string;
 	session?: string;
+	readOnly?: boolean;
 	voice?: string;
 }
 
 function parseArgs(argv: string[]): ParsedArgs {
 	const args = argv.slice(2);
 	let model: string | undefined;
-	let dir = process.cwd();
+	// Left undefined unless --dir/-d is explicitly passed, so callers can tell
+	// "user gave no --dir" apart from "user's --dir value happens to equal cwd".
+	let dir: string | undefined;
 	let prompt: string | undefined;
 	let env: string | undefined;
 	let sandbox = false;
@@ -67,6 +70,7 @@ function parseArgs(argv: string[]): ParsedArgs {
 	let repo: string | undefined;
 	let pat: string | undefined;
 	let session: string | undefined;
+	let readOnly = false;
 	let voice: string | undefined;
 
 	for (let i = 0; i < args.length; i++) {
@@ -107,6 +111,9 @@ function parseArgs(argv: string[]): ParsedArgs {
 			case "--session":
 				session = args[++i];
 				break;
+			case "--read-only":
+				readOnly = true;
+				break;
 			case "--voice":
 			case "-v":
 				// Accept optional backend name: --voice, --voice openai, --voice gemini
@@ -124,7 +131,7 @@ function parseArgs(argv: string[]): ParsedArgs {
 		}
 	}
 
-	return { model, dir, prompt, env, sandbox, sandboxRepo, sandboxToken, repo, pat, session, voice };
+	return { model, dir, prompt, env, sandbox, sandboxRepo, sandboxToken, repo, pat, session, readOnly, voice };
 }
 
 function handleEvent(
@@ -321,10 +328,10 @@ async function main(): Promise<void> {
 		return;
 	}
 
-	const { model, dir: rawDir, prompt, env, sandbox: useSandbox, sandboxRepo, sandboxToken, repo, pat, session: sessionBranch, voice } = parseArgs(process.argv);
+	const { model, dir: rawDir, prompt, env, sandbox: useSandbox, sandboxRepo, sandboxToken, repo, pat, session: sessionBranch, readOnly, voice } = parseArgs(process.argv);
 
 	// If --repo is given, derive a default dir from the repo URL (skip interactive prompt)
-	let dir = rawDir;
+	let dir = rawDir ?? process.cwd();
 	let localSession: LocalSession | undefined;
 
 	if (repo) {
@@ -340,8 +347,11 @@ async function main(): Promise<void> {
 			process.exit(1);
 		}
 
-		// Default dir: /tmp/gitagent/<repo-name> if no --dir given
-		if (dir === process.cwd()) {
+		// Default dir: /tmp/gitagent/<repo-name> if --dir wasn't explicitly passed.
+		// Checked against `rawDir` (undefined unless the user passed --dir), not
+		// against `dir === process.cwd()` — a value-equality check can't tell
+		// "no --dir given" apart from "user explicitly passed --dir matching cwd".
+		if (rawDir === undefined) {
 			const repoName = repo.split("/").pop()?.replace(/\.git$/, "") || "repo";
 			dir = resolve(`/tmp/gitagent/${repoName}`);
 		}
@@ -351,6 +361,7 @@ async function main(): Promise<void> {
 			token,
 			dir,
 			session: sessionBranch,
+			readOnly,
 		});
 		dir = localSession.dir;
 		console.log(dim(`Local session: ${localSession.branch} (${localSession.dir})`));

src/sdk-types.ts

@@ -112,6 +112,7 @@ export interface LocalRepoOptions {
 	token: string;
 	dir?: string;
 	session?: string;
+	readOnly?: boolean;
 }
 
 // ── Sandbox options ─────────────────────────────────────────────────────

src/sdk.ts

@@ -141,11 +141,18 @@ export function query(options: QueryOptions): Query {
 			if (!token) {
 				throw new Error("repo.token, GITHUB_TOKEN, or GIT_TOKEN is required with repo option");
 			}
+			const repoDir = options.repo.dir ?? options.dir;
+			if (!repoDir) {
+				throw new Error(
+					"repo mode requires an explicit `dir` (top-level `dir` or `repo.dir`) — refusing to default to the current working directory",
+				);
+			}
 			localSession = initLocalSession({
 				url: options.repo.url,
 				token,
-				dir: options.repo.dir || dir,
+				dir: repoDir,
 				session: options.repo.session,
+				readOnly: options.repo.readOnly,
 			});
 			dir = localSession.dir;
 		}

src/session.ts

@@ -1,5 +1,5 @@
-import { execSync } from "child_process";
-import { existsSync, mkdirSync, writeFileSync } from "fs";
+import { execSync, execFileSync } from "child_process";
+import { existsSync, mkdirSync, writeFileSync, realpathSync } from "fs";
 import { resolve } from "path";
 import { randomBytes } from "crypto";
 
@@ -10,6 +10,7 @@ export interface LocalRepoOptions {
 	token: string;
 	dir: string;
 	session?: string;
+	readOnly?: boolean;
 }
 
 export interface LocalSession {
@@ -29,7 +30,7 @@ function authedUrl(url: string, token: string): string {
 }
 
 function cleanUrl(url: string): string {
-	return url.replace(/^https:\/\/[^@]+@/, "https://");
+	return url.replace(/^https:\/\/[^@]+@/, "https://").replace(/\.git$/, "");
 }
 
 function git(args: string, cwd: string): string {
@@ -56,14 +57,49 @@ function getDefaultBranch(cwd: string): string {
 
 export function initLocalSession(opts: LocalRepoOptions): LocalSession {
 	const { url, token, session } = opts;
+	if (!opts.dir) {
+		throw new Error("repo.dir is required — refusing to guess a working directory for repo mode");
+	}
 	const dir = resolve(opts.dir);
 	const aUrl = authedUrl(url, token);
 
 	// Clone or update
 	if (!existsSync(dir)) {
 		execSync(`git clone --depth 1 --no-single-branch ${aUrl} ${dir}`, { stdio: "pipe" });
 	} else {
-		git(`remote set-url origin ${aUrl}`, dir);
+		// Refuse to operate unless `dir` is a git repo root in its own right.
+		// If `dir` has no local .git, git commands here would silently resolve
+		// to whatever ancestor repo does have one — e.g. a monorepo root.
+		let topLevel: string;
+		try {
+			topLevel = execSync("git rev-parse --show-toplevel", { cwd: dir, stdio: "pipe", encoding: "utf-8" }).trim();
+		} catch {
+			throw new Error(`${dir} exists but is not a git repository — refusing to operate on it`);
+		}
+		if (realpathSync(topLevel) !== realpathSync(dir)) {
+			throw new Error(
+				`${dir} has no .git of its own — git commands here would escape to the ancestor repo at ${topLevel}. Refusing to run destructive commands.`,
+			);
+		}
+
+		// Refuse to reset a repo that's already tracking a different remote —
+		// it isn't the clone this call expects, even if the folder exists.
+		let existingOrigin = "";
+		try {
+			existingOrigin = execSync("git remote get-url origin", { cwd: dir, stdio: "pipe", encoding: "utf-8" }).trim();
+		} catch {
+			// no origin configured yet — fine, we're about to set one
+		}
+		if (existingOrigin && cleanUrl(existingOrigin) !== cleanUrl(url)) {
+			throw new Error(
+				`${dir} already tracks a different remote (${cleanUrl(existingOrigin)}), not ${url}. Refusing to overwrite it.`,
+			);
+		}
+
+		// `set-url` only updates an existing remote — it errors if `origin`
+		// isn't configured yet, which happens if `dir` is a repo the caller
+		// created themselves rather than one gitagent cloned previously.
+		git(`remote ${existingOrigin ? "set-url" : "add"} origin ${aUrl}`, dir);
 		git("fetch origin", dir);
 
 		// Reset local default branch to latest remote
@@ -133,9 +169,12 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession {
 				git("diff --cached --quiet", dir);
 				// Nothing staged — skip
 			} catch {
-				// There are staged changes
+				// There are staged changes. Use execFileSync with an argv array
+				// (not the shell-interpolated `git()` helper) since commitMsg may
+				// be an arbitrary caller-supplied string containing quotes, `$()`,
+				// backticks, etc.
 				const commitMsg = msg || `gitagent: auto-commit (${branch})`;
-				git(`commit -m "${commitMsg}"`, dir);
+				execFileSync("git", ["commit", "-m", commitMsg], { cwd: dir, stdio: "pipe" });
 			}
 		},
 
@@ -144,9 +183,11 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession {
 		},
 
 		finalize() {
-			localSession.commitChanges();
-			localSession.push();
-			// Strip PAT from remote URL
+			if (!opts.readOnly) {
+				localSession.commitChanges();
+				localSession.push();
+			}
+			// Strip PAT from remote URL regardless of readOnly
 			git(`remote set-url origin ${cleanUrl(url)}`, dir);
 		},
 	};

test/mcp.test.ts

@@ -91,15 +91,15 @@ describe("buildToolsForConnection", () => {
 
 		const echo = tools.find((t) => t.name === "srv__echo")!;
 		const res = await echo.execute("call-1", { msg: "hello" });
-		assert.equal(res, "hello");
+		assert.equal(res.content[0].text, "hello");
 
 		const boom = tools.find((t) => t.name === "srv__boom")!;
 		const boomRes = await boom.execute("call-2", {});
-		assert.match(boomRes, /^Error: /);
+		assert.match(boomRes.content[0].text, /^Error: /);
 
 		const pic = tools.find((t) => t.name === "srv__pic")!;
 		const picRes = await pic.execute("call-3", {});
-		assert.match(picRes, /\[image: image\/png/);
+		assert.match(picRes.content[0].text, /\[image: image\/png/);
 
 		await conn.close();
 	});
@@ -142,7 +142,7 @@ describe("buildToolsForConnection — pagination & name sanitization", () => {
 		};
 		const tools = await buildToolsForConnection({ name: "srv", client: mockClient, close: async () => {} });
 		const res = await tools[0].execute("c1", {});
-		assert.match(res, /^Error: .*-32602/);
+		assert.match(res.content[0].text, /^Error: .*-32602/);
 	});
 });