From 70b496f94c367160cf5b1b85e281a2b8c0e5fe87 Mon Sep 17 00:00:00 2001 From: Nivesh353 Date: Mon, 27 Jul 2026 00:41:58 +0530 Subject: [PATCH 1/3] fix(repo-mode): prevent unsafe dir defaults and forced pushes (#85) Repo mode previously defaulted `dir` to process.cwd() when omitted, letting git commands silently escape to an ancestor repo if the target folder had no .git of its own. It also always committed and pushed on finalize() with no way to opt out, even for read-only use. - Require an explicit `dir` in repo mode instead of defaulting to cwd - Refuse to operate unless `dir` is a git repo root in its own right, and unless it already tracks the expected remote - Add `repo.readOnly` (SDK) and `--read-only` (CLI) to skip commit/push in finalize() while still stripping the embedded token --- README.md | 1 + package-lock.json | 4 ++-- src/index.ts | 10 ++++++++-- src/sdk-types.ts | 1 + src/sdk.ts | 9 ++++++++- src/session.ts | 43 +++++++++++++++++++++++++++++++++++++++---- 6 files changed, 59 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 7d0257f..1f6faf5 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,7 @@ gitagent --repo https://github.com/org/repo "Add unit tests" | `--repo ` | `-r` | GitHub repo URL to clone and work on | | `--pat ` | | GitHub PAT (or set `GITHUB_TOKEN` / `GIT_TOKEN`) | | `--session ` | | Resume an existing session branch | +| `--read-only` | | With `--repo`: clone/checkout only, never commit or push | | `--model ` | `-m` | Override model (e.g. `anthropic:claude-sonnet-4-5-20250929`) | | `--sandbox` | `-s` | Run in sandbox VM | | `--prompt ` | `-p` | Single-shot prompt (skip REPL) | diff --git a/package-lock.json b/package-lock.json index 0901d27..54f09a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@open-gitagent/gitagent", - "version": "2.0.1", + "version": "2.0.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@open-gitagent/gitagent", - "version": "2.0.1", + "version": "2.0.2", "license": "MIT", "dependencies": { "@mariozechner/pi-agent-core": "^0.70.2", diff --git a/src/index.ts b/src/index.ts index ba4eeb2..e420fec 100644 --- a/src/index.ts +++ b/src/index.ts @@ -52,6 +52,7 @@ interface ParsedArgs { repo?: string; pat?: string; session?: string; + readOnly?: boolean; voice?: string; } @@ -67,6 +68,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 +109,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 +129,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,7 +326,7 @@ async function main(): Promise { 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; @@ -351,6 +356,7 @@ async function main(): Promise { token, dir, session: sessionBranch, + readOnly, }); dir = localSession.dir; console.log(dim(`Local session: ${localSession.branch} (${localSession.dir})`)); diff --git a/src/sdk-types.ts b/src/sdk-types.ts index 5ab79ca..d2528f8 100644 --- a/src/sdk-types.ts +++ b/src/sdk-types.ts @@ -112,6 +112,7 @@ export interface LocalRepoOptions { token: string; dir?: string; session?: string; + readOnly?: boolean; } // ── Sandbox options ───────────────────────────────────────────────────── diff --git a/src/sdk.ts b/src/sdk.ts index 55b941c..ce1b475 100644 --- a/src/sdk.ts +++ b/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; } diff --git a/src/session.ts b/src/session.ts index cdc6a45..4053f80 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1,5 +1,5 @@ import { execSync } from "child_process"; -import { existsSync, mkdirSync, writeFileSync } from "fs"; +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 { @@ -56,6 +57,9 @@ 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); @@ -63,6 +67,35 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession { if (!existsSync(dir)) { execSync(`git clone --depth 1 --no-single-branch ${aUrl} ${dir}`, { stdio: "pipe" }); } else { + // 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.`, + ); + } + git(`remote set-url origin ${aUrl}`, dir); git("fetch origin", dir); @@ -144,9 +177,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); }, }; From 19df132209496a465128d7dfb52d766627d0dfdd Mon Sep 17 00:00:00 2001 From: Nivesh353 Date: Mon, 27 Jul 2026 00:45:13 +0530 Subject: [PATCH 2/3] fixed mcp test file --- test/mcp.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/mcp.test.ts b/test/mcp.test.ts index c9efb17..d2b890a 100644 --- a/test/mcp.test.ts +++ b/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/); }); }); From c5c40d1c1ef369e550b6f0286a005802f950a2c9 Mon Sep 17 00:00:00 2001 From: Nivesh353 Date: Mon, 27 Jul 2026 01:10:10 +0530 Subject: [PATCH 3/3] fix(repo-mode): fix PR review findings + missing-origin crash - Normalize .git suffix in cleanUrl so remote-mismatch comparison doesn't false-positive on url vs url.git - Use execFileSync for commit -m to avoid shell injection via arbitrary commit messages - Track --dir via an explicit undefined sentinel instead of comparing against process.cwd(), which couldn't tell "no --dir" apart from "--dir explicitly matching cwd" - Use `git remote add` instead of `set-url` when origin isn't configured yet, fixing a crash found while verifying the above --- src/index.ts | 15 ++++++++++----- src/session.ts | 16 +++++++++++----- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/index.ts b/src/index.ts index e420fec..ae6c0c4 100644 --- a/src/index.ts +++ b/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; @@ -59,7 +59,9 @@ interface ParsedArgs { 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; @@ -329,7 +331,7 @@ async function main(): Promise { 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) { @@ -345,8 +347,11 @@ async function main(): Promise { process.exit(1); } - // Default dir: /tmp/gitagent/ if no --dir given - if (dir === process.cwd()) { + // Default dir: /tmp/gitagent/ 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}`); } diff --git a/src/session.ts b/src/session.ts index 4053f80..f418b27 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1,4 +1,4 @@ -import { execSync } from "child_process"; +import { execSync, execFileSync } from "child_process"; import { existsSync, mkdirSync, writeFileSync, realpathSync } from "fs"; import { resolve } from "path"; import { randomBytes } from "crypto"; @@ -30,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 { @@ -96,7 +96,10 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession { ); } - git(`remote set-url origin ${aUrl}`, dir); + // `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 @@ -166,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" }); } },