From 18e36fbb3dad4394fd72b27bfc817675fdd644fe Mon Sep 17 00:00:00 2001 From: wqymi Date: Tue, 28 Jul 2026 19:14:03 +0800 Subject: [PATCH] fix(tool): enforce isolated-child git safety in the bash tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All worktrees of a repo share ONE .git/ ref store, so an isolated child session running `git rebase main` or `git checkout main` inside its own worktree writes refs the MAIN checkout is sitting on and can move its HEAD — corrupting the tree the user and other agents are working in. This has happened for real. Until now the rule existed only as prompt text in orchestrator.txt with zero enforcement. Add a cross-branch git guard on the bash tool's existing parsed-command choke point. It fires only for isolated children (Instance.directory under `/worktree/…`), so the orchestrator and ordinary sessions still integrate branches freely. Blocks rebase/merge, branch switches, branch delete/force-move, foreign force-push/delete-push, worktree mutations and direct ref writes; leaves the child's own add/commit/push/status/diff/ stash/file-restore/new-branch work untouched. The error names what was blocked, why, and what to do instead. Also honor an explicit `session grant-approval` on the parent-grant inheritance path: decideAskRouting routes an ordinary background subagent to `inherit`, never to `forward`, so the DB-backed grant was consulted nowhere on a subagent's path — the documented command silently did nothing and the child's external_directory ask failed closed mid-task. --- packages/opencode/src/permission/index.ts | 16 + packages/opencode/src/tool/bash.ts | 21 +- .../opencode/src/tool/isolated-git-guard.ts | 313 ++++++++++++++++++ .../opencode/test/permission/inherit.test.ts | 84 +++++ .../test/tool/bash-isolated-git-guard.test.ts | 112 +++++++ .../test/tool/isolated-git-guard.test.ts | 237 +++++++++++++ 6 files changed, 782 insertions(+), 1 deletion(-) create mode 100644 packages/opencode/src/tool/isolated-git-guard.ts create mode 100644 packages/opencode/test/tool/bash-isolated-git-guard.test.ts create mode 100644 packages/opencode/test/tool/isolated-git-guard.test.ts diff --git a/packages/opencode/src/permission/index.ts b/packages/opencode/src/permission/index.ts index 8534e2b1d..826a05b9b 100644 --- a/packages/opencode/src/permission/index.ts +++ b/packages/opencode/src/permission/index.ts @@ -280,6 +280,22 @@ export const layer = Layer.effect( // hold isn't matched → we do NOT return here → it fails closed at the // non-interactive gate. No human wait, no hang. if (needsAsk && input.inherit && !forced) { + // An EXPLICIT `session grant-approval ` pre-authorizes this + // child. That grant is DB-backed (write-through in forwardRef.setGrant), + // so unlike the in-memory parentGrants snapshot it survives a restart and + // is visible to a child running in its own Instance/process. Checked here + // because `decideAskRouting` routes an ordinary background subagent to + // `inherit`, never to `forward` — the only other place grantAllowed is + // consulted — so without this the documented command silently does + // nothing for subagents and their asks fail closed below. + if (forwardRef.grantAllowed(input.inherit.parentSessionID, request.sessionID)) { + log.info("parent holds an explicit approval grant, auto-allowing", { + permission: request.permission, + patterns: request.patterns, + parentSessionID: input.inherit.parentSessionID, + }) + return + } const parentSnapshot = forwardRef.getParentGrants(input.inherit.parentSessionID) if (parentSnapshot) { // Mirror the parent's own two-phase evaluation (see the deny loop diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index c1d6b9f11..b93fe586c 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -1,6 +1,6 @@ import z from "zod" import os from "os" -import { createWriteStream, readFileSync } from "node:fs" +import { createWriteStream, existsSync, readFileSync } from "node:fs" import * as Tool from "./tool" import path from "path" import DESCRIPTION from "./bash.txt" @@ -16,6 +16,7 @@ import { Flag } from "@/flag/flag" import { Shell } from "@/shell/shell" import { SessionCwd } from "./session-cwd" +import * as IsolatedGit from "./isolated-git-guard" import { BashArity } from "@/permission/arity" import * as Truncate from "./truncate" import { Plugin } from "@/plugin" @@ -764,6 +765,24 @@ export const BashTool = Tool.define( const timeout = params.timeout ?? DEFAULT_TIMEOUT const ps = PS.has(name) const root = yield* parse(params.command, ps) + // Cross-branch git guard for isolated children. Sits on the SAME + // parsed AST the permission scan uses, so every command node in a + // pipeline / `&&` chain / subshell is checked, and it runs BEFORE + // ask() so a rejected command never prompts and never spawns. + // Keyed on Instance.directory (the session's own worktree), not on + // `cwd` — a child that `cd`s into the main checkout and rebases + // there is exactly the accident this exists to stop. + const own = Instance.directory + if (IsolatedGit.isIsolatedWorktree(own)) { + IsolatedGit.assertIsolatedGitAllowed({ + commands: commands(root).map((node) => parts(node).map((item) => item.text)), + sources: commands(root).map((node) => source(node)), + directory: own, + isolated: true, + branch: IsolatedGit.ownBranch(own), + isPath: (arg) => existsSync(path.resolve(cwd, arg)), + }) + } const scan = yield* collect(root, cwd, ps, shell) if (!Instance.containsPath(cwd)) scan.dirs.add(cwd) // Delete-containing commands are authorized by askDelete alone — diff --git a/packages/opencode/src/tool/isolated-git-guard.ts b/packages/opencode/src/tool/isolated-git-guard.ts new file mode 100644 index 000000000..a6aa8d6f0 --- /dev/null +++ b/packages/opencode/src/tool/isolated-git-guard.ts @@ -0,0 +1,313 @@ +import * as path from "path" +import { readFileSync, realpathSync } from "node:fs" +import { AppFileSystem } from "@mimo-ai/shared/filesystem" +import { Global } from "../global" + +/** + * Cross-branch git guard for ISOLATED CHILD sessions. + * + * An isolated child (`session create --isolate`) runs in an app-managed git + * worktree under `/worktree//`. Every worktree of a + * repository shares ONE ref store (`--git-common-dir`), so a git command run + * from inside the child's worktree can mutate refs the MAIN checkout is sitting + * on — `git rebase main`, `git checkout main`, `git branch -f main …` all reach + * across and can move the main checkout's HEAD, corrupting the tree the user + * and other agents are actively working in. This has happened for real. + * + * The orchestrator prompt already tells children not to do this; this module + * makes it a mechanism instead of a suggestion. It only fires for isolated + * children — the orchestrator and ordinary sessions legitimately integrate + * branches and are never gated here. + * + * SCOPE / HONEST LIMITS + * - This is a string/argv-level check over the already-parsed command AST. A + * determined command can evade it (shell aliases, `sh -c "$(printf …)"`, + * `eval`, a wrapper script, a git alias configured in the repo). It is not a + * sandbox and does not try to be one. The target is the ACCIDENTAL + * HEAD-moving mistake that actually occurs in practice. + * - `git reset` is deliberately NOT gated: it can only move the CURRENT + * worktree's own branch, so it cannot corrupt another checkout's HEAD. Its + * destructive `--hard` form is already behind the forced-ask `bash_delete` + * prompt in bash.ts. + * - `git pull` is deliberately NOT gated for the same reason: it only + * fast-forwards/merges INTO the child's own branch and never writes another + * branch's ref. + */ + +/** git global options that consume the FOLLOWING token as their value. */ +const GLOBAL_WITH_VALUE = new Set([ + "-C", + "-c", + "--git-dir", + "--work-tree", + "--namespace", + "--exec-path", + "--config-env", + "--super-prefix", +]) + +/** Flags that make `rebase`/`merge` operate on an ALREADY IN-PROGRESS operation + * in this worktree only. Those are recovery actions, always allowed. */ +const IN_PROGRESS = new Set([ + "--abort", + "--continue", + "--skip", + "--quit", + "--edit-todo", + "--show-current-patch", +]) + +/** `git branch` flags that delete, force-move or rename a ref. */ +const BRANCH_MUTATE = new Set(["-d", "-D", "--delete", "-f", "--force", "-m", "-M", "--move"]) + +/** `git worktree` subcommands that write the shared worktree registry. */ +const WORKTREE_MUTATE = new Set(["add", "remove", "move", "prune", "repair", "lock", "unlock"]) + +export type Violation = { + /** The single command that was rejected, as written. */ + command: string + /** Short human reason, e.g. "git rebase rewrites shared refs". */ + reason: string +} + +function unquote(text: string) { + if (text.length < 2) return text + const first = text[0] + if ((first === '"' || first === "'") && first === text[text.length - 1]) return text.slice(1, -1) + return text +} + +/** + * Splits a git argv into `{ sub, args }`, skipping git's global options so + * `git --no-pager -C /elsewhere rebase main` is still seen as `rebase`. + * Returns undefined when the command is not git or has no subcommand. + */ +export function gitInvocation(tokens: string[]): { sub: string; args: string[] } | undefined { + const argv = tokens.map(unquote).filter((tok) => tok.length > 0) + if (argv.length < 2) return + const head = path.basename(argv[0]).toLowerCase() + if (head !== "git" && head !== "git.exe") return + let i = 1 + while (i < argv.length) { + const tok = argv[i] + if (!tok.startsWith("-")) break + if (GLOBAL_WITH_VALUE.has(tok)) { + i += 2 + continue + } + i += 1 + } + if (i >= argv.length) return + return { sub: argv[i], args: argv.slice(i + 1) } +} + +/** Positional (non-flag) arguments. A bare `-` is positional: `git checkout -` + * switches to the previous branch. */ +function positionals(args: string[]) { + return args.filter((arg) => arg === "-" || !arg.startsWith("-")) +} + +function valueOf(args: string[], flags: Set) { + for (let i = 0; i < args.length; i++) { + const arg = args[i] + for (const flag of flags) { + if (arg === flag) return args[i + 1] + if (arg.startsWith(flag + "=")) return arg.slice(flag.length + 1) + } + } +} + +/** Normalizes a push refspec to the branch name it WRITES on the remote. */ +function pushTarget(refspec: string) { + const spec = refspec.startsWith("+") ? refspec.slice(1) : refspec + const colon = spec.lastIndexOf(":") + const dst = colon === -1 ? spec : spec.slice(colon + 1) + return dst.replace(/^refs\/heads\//, "") +} + +/** + * Decides whether a single git invocation crosses the isolated child's branch + * boundary. Pure: `isPath` is injected so the caller owns filesystem access. + * `branch` is the child's own branch (undefined when it could not be read — + * ownership-sensitive rules then fail closed, which is safe because those forms + * are destructive and a child never needs them). + */ +export function violates(input: { + tokens: string[] + branch?: string + isPath?: (arg: string) => boolean +}): string | undefined { + const git = gitInvocation(input.tokens) + if (!git) return + const { sub, args } = git + const branch = input.branch + const isPath = input.isPath ?? (() => false) + const owns = (name: string) => Boolean(branch) && (name === branch || name === "HEAD") + + switch (sub) { + case "rebase": { + if (args.some((arg) => IN_PROGRESS.has(arg))) return + return "`git rebase` rewrites history against another branch and writes the shared ref store" + } + case "merge": { + if (args.some((arg) => IN_PROGRESS.has(arg))) return + return "`git merge` integrates another branch — integration is the orchestrator's job" + } + case "checkout": + case "switch": { + // With `--`, or with 2+ positionals, git restores FILES from a tree-ish + // and never moves HEAD. Always allowed. + if (args.includes("--")) return + const pos = positionals(args) + if (pos.length !== 1) { + if (args.includes("--detach")) return "`--detach` moves this worktree's HEAD off its own branch" + return + } + // Creating a brand-new branch is the child's own ref: allowed. + const created = valueOf(args, new Set(["-b", "-c", "--orphan"])) + if (created !== undefined) return + // `-B`/`-C` force-create, which RESETS an existing branch — only safe on its own. + const forced = valueOf(args, new Set(["-B", "-C"])) + if (forced !== undefined) { + if (owns(forced)) return + return `\`git ${sub}\` force-creating '${forced}' resets a branch this session does not own` + } + const target = pos[0] + if (isPath(target)) return + if (owns(target)) return + return `switching this worktree to '${target}' moves a ref/HEAD in the shared ref store` + } + case "branch": { + if (!args.some((arg) => BRANCH_MUTATE.has(arg))) return + const pos = positionals(args) + if (pos.length > 0 && pos.every((name) => owns(name))) return + return "deleting, force-moving or renaming a branch mutates the shared ref store" + } + case "push": { + const pos = positionals(args) + const specs = pos.slice(1) + const forced = + args.includes("--force") || args.includes("-f") || specs.some((spec) => spec.startsWith("+")) + const deleting = args.includes("--delete") || specs.some((spec) => spec.startsWith(":")) + if (!forced && !deleting) return + // No refspec ⇒ the current (own) branch. + if (specs.length === 0) return + const foreign = specs.map(pushTarget).filter((name) => !owns(name)) + if (foreign.length === 0) return + return `force-pushing or deleting '${foreign[0]}' rewrites a branch this session does not own` + } + case "worktree": { + const pos = positionals(args) + if (pos.length === 0 || !WORKTREE_MUTATE.has(pos[0])) return + return `\`git worktree ${pos[0]}\` mutates the shared worktree registry` + } + case "update-ref": { + const pos = positionals(args) + const ref = pos[0] ?? "" + if (owns(ref.replace(/^refs\/heads\//, ""))) return + return "`git update-ref` writes the shared ref store directly" + } + case "symbolic-ref": { + // One positional is a READ (`git symbolic-ref HEAD`); two is a write. + const pos = positionals(args) + if (pos.length < 2 && !args.includes("--delete") && !args.includes("-d")) return + return "`git symbolic-ref` repoints HEAD in the shared ref store" + } + default: + return + } +} + +/** True when `directory` is one of the app-managed worktrees that back an + * isolated child session (`/worktree//`). The + * orchestrator and ordinary sessions run in the user's project directory and + * are therefore never gated. Mirrors the trusted-root test already used by + * `src/tool/external-directory.ts`, but additionally compares realpaths: + * `Instance.provide` stores a realpath-resolved directory, so on a symlinked + * prefix (macOS `/var` → `/private/var`) a raw string-prefix test misses. */ +export function isIsolatedWorktree(directory: string | undefined, root?: string) { + if (!directory) return false + const base = root ?? path.join(Global.Path.data, "worktree") + if (AppFileSystem.contains(base, directory)) return true + return AppFileSystem.contains(real(base), real(directory)) +} + +function real(target: string) { + try { + return realpathSync(target) + } catch { + return path.resolve(target) + } +} + +/** Best-effort read of the branch a worktree is on, without spawning git. + * `/.git` is a file containing `gitdir: /.git/worktrees/`, + * whose `HEAD` holds `ref: refs/heads/`. Returns undefined on any + * surprise (detached HEAD, unreadable, plain clone layout is handled too). */ +export function ownBranch(directory: string): string | undefined { + try { + const dotGit = path.join(directory, ".git") + let gitDir = dotGit + try { + const pointer = readFileSync(dotGit, "utf-8") + const match = pointer.match(/^gitdir:\s*(.+)$/m) + if (match) gitDir = path.resolve(directory, match[1].trim()) + } catch { + // `.git` is a directory (ordinary clone) — read HEAD from it directly. + } + const head = readFileSync(path.join(gitDir, "HEAD"), "utf-8").trim() + const ref = head.match(/^ref:\s*refs\/heads\/(.+)$/) + return ref ? ref[1] : undefined + } catch { + return undefined + } +} + +function message(input: { violation: Violation; directory: string; branch?: string }) { + const branch = input.branch ?? "(could not be determined)" + return ( + `Blocked in an isolated child session: ${input.violation.command}\n` + + `Reason: ${input.violation.reason}.\n\n` + + `Every worktree of this repository shares ONE .git/ ref store, so a cross-branch git\n` + + `operation run from inside your worktree writes refs the MAIN checkout is using and can\n` + + `move its HEAD — corrupting the working tree the user and other agents are in right now.\n\n` + + ` your worktree: ${input.directory}\n` + + ` your branch: ${branch}\n\n` + + `Do this instead:\n` + + ` - work only in your worktree, on your own branch\n` + + ` - \`git add\` + \`git commit\` + \`git push\` YOUR branch, and nothing else\n` + + ` - ask the orchestrator to rebase/merge/land it — cross-branch integration is its job\n` + ) +} + +/** + * The gate. Throws (loudly, with remediation) when any command in a bash + * invocation crosses the isolated child's branch boundary. No-ops when the + * session is not an isolated child. + */ +export function assertIsolatedGitAllowed(input: { + /** argv of every command node in the parsed invocation (pipelines, `&&` + * chains and subshells each contribute one entry). */ + commands: string[][] + /** Rendered source of each command, index-aligned with `commands`; used only + * for the error text. Falls back to the joined argv. */ + sources?: string[] + directory: string + isolated: boolean + branch?: string + isPath?: (arg: string) => boolean +}): void { + if (!input.isolated) return + for (let i = 0; i < input.commands.length; i++) { + const tokens = input.commands[i] + const reason = violates({ tokens, branch: input.branch, isPath: input.isPath }) + if (!reason) continue + throw new Error( + message({ + violation: { command: input.sources?.[i] ?? tokens.join(" "), reason }, + directory: input.directory, + branch: input.branch, + }), + ) + } +} diff --git a/packages/opencode/test/permission/inherit.test.ts b/packages/opencode/test/permission/inherit.test.ts index 3e5d0df42..5ca574cd6 100644 --- a/packages/opencode/test/permission/inherit.test.ts +++ b/packages/opencode/test/permission/inherit.test.ts @@ -17,6 +17,7 @@ afterEach(async () => { beforeEach(() => { forwardRef.parentGrants.clear() + forwardRef.clearGrantsForParent("ses_parent") }) const bus = Bus.layer @@ -140,3 +141,86 @@ describe("Permission.ask parent-grant inheritance", () => { ), ) }) + +// An EXPLICIT `session grant-approval ` must reach a background +// SUBAGENT too. decideAskRouting routes such a child to `inherit` (never to +// `forward`), so before this the DB-backed grant was consulted nowhere on the +// subagent's path: `grant-approval` silently did nothing and the child's +// external_directory ask failed closed. Unlike the in-memory parentGrants +// snapshot, this grant survives a restart and a separate-process child, so it +// cannot lapse mid-task. +describe("Permission.ask explicit grant-approval reaches a background subagent", () => { + it.live( + "grant-approval for this child auto-approves with NO parent snapshot at all", + provideTmpdirInstance(() => + Effect.gen(function* () { + const perm = yield* Permission.Service + forwardRef.setGrant("ses_parent", "ses_child") + let asked = 0 + const unsub = Bus.subscribe(Permission.Event.Asked, () => { + asked += 1 + }) + // No setParentGrants: this is exactly the "snapshot missing / lapsed" + // case (separate-process child, or a parent that never asked). + const result = yield* perm + .ask(childAsk(["/Users/me/projects/app/*"], { permission: "external_directory" as never })) + .pipe(Effect.exit) + unsub() + expect(result._tag).toBe("Success") + expect(asked).toBe(0) + expect((yield* perm.list()).length).toBe(0) + }), + ), + ) + + it.live( + "an 'all' grant covers any child of that parent", + provideTmpdirInstance(() => + Effect.gen(function* () { + const perm = yield* Permission.Service + forwardRef.setGrant("ses_parent", "*") + const result = yield* perm + .ask(childAsk(["/anywhere/x.ts"], { sessionID: "ses_other_child" as never })) + .pipe(Effect.exit) + expect(result._tag).toBe("Success") + }), + ), + ) + + it.live( + "WITHOUT a grant and without a snapshot the subagent still fails closed", + provideTmpdirInstance(() => + Effect.gen(function* () { + const perm = yield* Permission.Service + const result = yield* perm.ask(childAsk(["/ungranted/x.ts"])).pipe(Effect.exit) + expect(result._tag).toBe("Failure") + }), + ), + ) + + it.live( + "a grant for a DIFFERENT child does not leak to this one", + provideTmpdirInstance(() => + Effect.gen(function* () { + const perm = yield* Permission.Service + forwardRef.setGrant("ses_parent", "ses_someone_else") + const result = yield* perm.ask(childAsk(["/ungranted/x.ts"])).pipe(Effect.exit) + expect(result._tag).toBe("Failure") + }), + ), + ) + + it.live( + "forced-ask (bash_delete) is NOT auto-approved by a grant", + provideTmpdirInstance(() => + Effect.gen(function* () { + const perm = yield* Permission.Service + forwardRef.setGrant("ses_parent", "*") + const result = yield* perm + .ask(childAsk(["rm -rf /"], { permission: "bash_delete" as never })) + .pipe(Effect.exit) + expect(result._tag).toBe("Failure") + }), + ), + ) +}) diff --git a/packages/opencode/test/tool/bash-isolated-git-guard.test.ts b/packages/opencode/test/tool/bash-isolated-git-guard.test.ts new file mode 100644 index 000000000..37b2ef86c --- /dev/null +++ b/packages/opencode/test/tool/bash-isolated-git-guard.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, test } from "bun:test" +import { Effect, Layer, ManagedRuntime } from "effect" +import fs from "fs/promises" +import path from "path" +import { BashTool } from "../../src/tool/bash" +import { Instance } from "../../src/project/instance" +import { Global } from "../../src/global" +import { Agent } from "../../src/agent/agent" +import { Truncate } from "../../src/tool" +import { SessionID, MessageID } from "../../src/session/schema" +import * as CrossSpawnSpawner from "../../src/effect/cross-spawn-spawner" +import { AppFileSystem } from "@mimo-ai/shared/filesystem" +import { Plugin } from "../../src/plugin" +import { tmpdir } from "../fixture/fixture" + +// End-to-end proof that the cross-branch git guard is actually WIRED into the +// bash tool (not just a unit-tested module): an isolated child's Instance +// directory lives under `/worktree/...`, and a cross-branch git command +// executed there must be rejected before the process is spawned. + +const runtime = ManagedRuntime.make( + Layer.mergeAll( + CrossSpawnSpawner.defaultLayer, + AppFileSystem.defaultLayer, + Plugin.defaultLayer, + Truncate.defaultLayer, + Agent.defaultLayer, + ), +) + +const ctx = { + sessionID: SessionID.make("ses_guard_test"), + messageID: MessageID.make(""), + callID: "", + agent: "build", + abort: AbortSignal.any([]), + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, +} + +const init = () => runtime.runPromise(BashTool.pipe(Effect.flatMap((info) => info.init()))) + +async function withIsolatedWorktree(fn: (dir: string) => Promise) { + const dir = path.join(Global.Path.data, "worktree", `guardtest-${Math.random().toString(36).slice(2)}`) + await fs.mkdir(dir, { recursive: true }) + try { + await Instance.provide({ directory: dir, fn: () => fn(dir) }) + } finally { + await fs.rm(dir, { recursive: true, force: true }).catch(() => {}) + } +} + +const run = async (command: string) => { + const bash = await init() + return Effect.runPromise(bash.execute({ command, description: "guard probe" }, ctx)) +} + +describe("tool.bash isolated-child git guard", () => { + test("rejects `git rebase main` from an isolated child's worktree", async () => { + await withIsolatedWorktree(async (dir) => { + let text = "" + await run("git rebase main").then( + () => {}, + (err) => { + text = String(err?.message ?? err) + }, + ) + expect(text).toContain("Blocked in an isolated child session") + expect(text).toContain("git rebase main") + expect(text).toContain("shares ONE .git/ ref store") + expect(text).toContain(dir) + expect(text).toContain("ask the orchestrator") + }) + }) + + test("rejects `git checkout main` even when chained after legitimate work", async () => { + await withIsolatedWorktree(async () => { + let text = "" + await run("git add -A && git commit -m wip && git checkout main").then( + () => {}, + (err) => { + text = String(err?.message ?? err) + }, + ) + expect(text).toContain("Blocked in an isolated child session") + expect(text).toContain("git checkout main") + }) + }) + + test("allows the child's own work: `git status` runs normally", async () => { + await withIsolatedWorktree(async () => { + const result = await run("git status --porcelain") + // Not a repo, so git exits non-zero — the point is that it RAN rather + // than being rejected by the guard. + expect(typeof result.metadata.exit).toBe("number") + expect(result.output).not.toContain("Blocked in an isolated child session") + }) + }) + + test("a NON-isolated session is unaffected — `git rebase` is not gated", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const result = await run("git rebase definitely-no-such-branch") + expect(result.output).not.toContain("Blocked in an isolated child session") + expect(result.metadata.exit).not.toBe(0) + }, + }) + }) +}) diff --git a/packages/opencode/test/tool/isolated-git-guard.test.ts b/packages/opencode/test/tool/isolated-git-guard.test.ts new file mode 100644 index 000000000..00b31fe8c --- /dev/null +++ b/packages/opencode/test/tool/isolated-git-guard.test.ts @@ -0,0 +1,237 @@ +import { describe, expect, test } from "bun:test" +import * as path from "path" +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs" +import { tmpdir } from "node:os" +import { + assertIsolatedGitAllowed, + gitInvocation, + isIsolatedWorktree, + ownBranch, + violates, +} from "../../src/tool/isolated-git-guard" + +const BRANCH = "feat/child-work" +const DIR = "/data/worktree/p_abc/child-1" + +// Every real filename the allowed-list tests lean on. `git checkout ` is +// only distinguishable from `git checkout ` by what exists on disk. +const FILES = new Set(["src/tool/bash.ts", "README.md", ".", "packages"]) +const isPath = (arg: string) => FILES.has(arg) + +function reject(command: string) { + const tokens = command.split(/\s+/) + return violates({ tokens, branch: BRANCH, isPath }) +} + +describe("isolated-git-guard / blocked cross-branch operations", () => { + const blocked = [ + "git rebase main", + "git rebase origin/main", + "git rebase -i HEAD~3", + "git merge main", + "git merge --no-ff origin/main", + "git checkout main", + "git checkout dev", + "git checkout origin/main", + "git checkout -", + "git switch main", + "git switch --detach", + "git checkout -B main", + "git branch -f main HEAD", + "git branch -D other/branch", + "git branch --delete dev", + "git branch -m dev renamed", + "git push --force origin main", + "git push -f origin HEAD:main", + "git push origin +main", + "git push origin :dev", + "git push --delete origin dev", + "git worktree add ../other -b x", + "git worktree remove ../other", + "git update-ref refs/heads/main HEAD", + "git symbolic-ref HEAD refs/heads/main", + // git global options must not smuggle the subcommand past the guard + "git --no-pager rebase main", + "git -C /elsewhere/repo checkout main", + "git -c core.pager=cat merge main", + ] + + for (const command of blocked) { + test(`blocks: ${command}`, () => { + expect(reject(command)).toBeTruthy() + }) + } +}) + +describe("isolated-git-guard / allowed child work", () => { + const allowed = [ + "git add -A", + "git add src/tool/bash.ts", + "git commit -m wip", + "git commit --amend --no-edit", + "git status", + "git status --porcelain", + "git diff", + "git diff --cached", + "git log --oneline -5", + "git show HEAD", + "git fetch origin", + "git push", + "git push origin HEAD", + `git push -u origin ${BRANCH}`, + `git push --force origin ${BRANCH}`, + `git push --force-with-lease origin ${BRANCH}`, + "git stash", + "git stash pop", + "git checkout -- src/tool/bash.ts", + "git checkout -- .", + "git checkout HEAD -- src/tool/bash.ts", + "git checkout src/tool/bash.ts", + "git checkout .", + "git restore src/tool/bash.ts", + "git checkout -b feat/child-work-2", + "git switch -c feat/child-work-2", + `git checkout ${BRANCH}`, + `git checkout -B ${BRANCH}`, + "git rebase --abort", + "git rebase --continue", + "git merge --abort", + "git worktree list", + "git branch", + "git branch --show-current", + "git symbolic-ref HEAD", + // git reset / git pull are deliberately out of scope: neither can write a + // ref other than the current worktree's own branch. + "git reset --hard HEAD", + "git reset --hard origin/main", + "git pull", + "git pull origin main", + // non-git commands are never inspected + "bun test", + "rebase main", + ] + + for (const command of allowed) { + test(`allows: ${command}`, () => { + expect(reject(command)).toBeUndefined() + }) + } +}) + +describe("isolated-git-guard / unknown own branch fails closed on destructive forms", () => { + const check = (command: string) => violates({ tokens: command.split(/\s+/), isPath }) + + test("branch delete is blocked when the branch cannot be read", () => { + expect(check("git branch -D whatever")).toBeTruthy() + }) + + test("force push with an explicit refspec is blocked when the branch cannot be read", () => { + expect(check("git push --force origin whatever")).toBeTruthy() + }) + + test("plain commit/push still work when the branch cannot be read", () => { + expect(check("git commit -m wip")).toBeUndefined() + expect(check("git push")).toBeUndefined() + }) +}) + +describe("isolated-git-guard / gitInvocation", () => { + test("skips global options that consume a value", () => { + expect(gitInvocation(["git", "-C", "/repo", "--no-pager", "merge", "main"])).toEqual({ + sub: "merge", + args: ["main"], + }) + }) + + test("ignores non-git commands", () => { + expect(gitInvocation(["bun", "rebase"])).toBeUndefined() + expect(gitInvocation(["git"])).toBeUndefined() + }) + + test("unquotes tokens", () => { + expect(gitInvocation(["git", "checkout", '"main"'])).toEqual({ sub: "checkout", args: ["main"] }) + }) +}) + +describe("isolated-git-guard / isolation signal", () => { + const root = path.join("/data", "worktree") + + test("app-managed worktree directory is an isolated child", () => { + expect(isIsolatedWorktree(path.join(root, "p_abc", "child-1"), root)).toBe(true) + }) + + test("the user's project directory is NOT an isolated child", () => { + expect(isIsolatedWorktree("/Users/me/projects/app", root)).toBe(false) + }) + + test("undefined directory is not an isolated child", () => { + expect(isIsolatedWorktree(undefined, root)).toBe(false) + }) +}) + +describe("isolated-git-guard / ownBranch", () => { + test("reads the branch through a linked worktree's .git pointer file", () => { + const base = mkdtempSync(path.join(tmpdir(), "guard-wt-")) + const gitDir = path.join(base, "repo", ".git", "worktrees", "child") + mkdirSync(gitDir, { recursive: true }) + writeFileSync(path.join(gitDir, "HEAD"), `ref: refs/heads/${BRANCH}\n`) + const wt = path.join(base, "child") + mkdirSync(wt) + writeFileSync(path.join(wt, ".git"), `gitdir: ${gitDir}\n`) + expect(ownBranch(wt)).toBe(BRANCH) + }) + + test("returns undefined for a detached HEAD", () => { + const base = mkdtempSync(path.join(tmpdir(), "guard-detached-")) + mkdirSync(path.join(base, ".git"), { recursive: true }) + writeFileSync(path.join(base, ".git", "HEAD"), "1234567890abcdef\n") + expect(ownBranch(base)).toBeUndefined() + }) + + test("returns undefined when nothing is readable", () => { + expect(ownBranch(path.join(tmpdir(), "definitely-not-a-repo-" + Date.now()))).toBeUndefined() + }) +}) + +describe("isolated-git-guard / assertIsolatedGitAllowed", () => { + const call = (commands: string[][], isolated: boolean) => + assertIsolatedGitAllowed({ commands, directory: DIR, isolated, branch: BRANCH, isPath }) + + test("a NON-isolated session is completely unaffected", () => { + expect(() => call([["git", "rebase", "main"]], false)).not.toThrow() + expect(() => call([["git", "checkout", "main"]], false)).not.toThrow() + expect(() => call([["git", "branch", "-D", "dev"]], false)).not.toThrow() + }) + + test("an isolated child is blocked", () => { + expect(() => call([["git", "rebase", "main"]], true)).toThrow() + }) + + test("every command node in a chain is checked, not just the first", () => { + expect(() => + call( + [ + ["git", "add", "-A"], + ["git", "commit", "-m", "wip"], + ["git", "checkout", "main"], + ], + true, + ), + ).toThrow(/git checkout main/) + }) + + test("the error names what was blocked, why, and what to do instead", () => { + let text = "" + try { + call([["git", "rebase", "main"]], true) + } catch (err) { + text = (err as Error).message + } + expect(text).toContain("git rebase main") + expect(text).toContain("shares ONE .git/ ref store") + expect(text).toContain(DIR) + expect(text).toContain(BRANCH) + expect(text).toContain("git push") + expect(text).toContain("ask the orchestrator") + }) +})