From cc475d4eb991d845ccac57d23b8406c02cb5580d Mon Sep 17 00:00:00 2001 From: Yehia Ezzat <74499810+0xSemizzz@users.noreply.github.com> Date: Sun, 2 Aug 2026 14:33:39 +0300 Subject: [PATCH] fix(permission): scope "always" approval to the command that was shown `collect()` accumulated one arity-widened `always` pattern per command node into a single set, and `Permission.reply` installs an allow rule for every entry of that set when the user answers "always". For a compound command this turns one user decision into several grants: approving `git status && rm -rf ~` installs both `git status *` and `rm *`, and `rm *` matches `rm -rf /` on the next turn. Keep the widened prefix only when the request covers exactly one command. Otherwise fall back to the literal command strings, so "always" grants exactly the commands that were displayed in the prompt. --- packages/opencode/src/tool/shell.ts | 18 +++++++- packages/opencode/test/tool/shell.test.ts | 52 +++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/tool/shell.ts b/packages/opencode/src/tool/shell.ts index 1e4423e01774..6fe4760246bc 100644 --- a/packages/opencode/src/tool/shell.ts +++ b/packages/opencode/src/tool/shell.ts @@ -387,6 +387,7 @@ export const ShellTool = Tool.define( patterns: new Set(), always: new Set(), } + const found: { pattern: string; always: string }[] = [] const shellKind = ShellID.toKind(Shell.name(shell)) for (const node of commands(root)) { @@ -405,11 +406,24 @@ export const ShellTool = Tool.define( } if (tokens.length && (!cmd || !CWD.has(cmd))) { - scan.patterns.add(source(node)) - scan.always.add(BashArity.prefix(tokens).join(" ") + " *") + found.push({ + pattern: source(node), + always: BashArity.prefix(tokens).join(" ") + " *", + }) } } + for (const item of found) scan.patterns.add(item.pattern) + + // A single "always" reply approves every pattern in `scan.always`, so the widened + // arity prefix is only safe when the request covers exactly one command. For a + // compound command the user is shown one prompt and makes one decision, but each + // command would contribute its own widened rule -- approving `git status && rm -rf ~` + // would install `git status *` AND `rm *`, the latter matching `rm -rf /`. Fall back + // to the literal commands so "always" grants exactly what was displayed. + if (found.length === 1) scan.always.add(found[0].always) + else for (const item of found) scan.always.add(item.pattern) + return scan }) diff --git a/packages/opencode/test/tool/shell.test.ts b/packages/opencode/test/tool/shell.test.ts index a970f85d468f..0a516a01a87b 100644 --- a/packages/opencode/test/tool/shell.test.ts +++ b/packages/opencode/test/tool/shell.test.ts @@ -263,6 +263,58 @@ describe("tool.shell permissions", () => { }), ) + each("widens the always pattern for a single command", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped() + yield* runIn( + tmp, + Effect.gen(function* () { + const err = new Error("stop after permission") + const requests: Array> = [] + expect( + yield* fail( + { + command: "git status", + }, + capture(requests, err), + ), + ).toMatchObject({ message: err.message }) + const bashReq = requests.find((r) => r.permission === "bash") + expect(bashReq).toBeDefined() + expect(bashReq!.always).toContain("git status *") + }), + ) + }), + ) + + each("does not widen always patterns for compound commands", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped() + yield* runIn( + tmp, + Effect.gen(function* () { + const err = new Error("stop after permission") + const requests: Array> = [] + expect( + yield* fail( + { + command: "git status && rm -rf tmp", + }, + capture(requests, err), + ), + ).toMatchObject({ message: err.message }) + const bashReq = requests.find((r) => r.permission === "bash") + expect(bashReq).toBeDefined() + // Approving the prompt once must not install a rule that matches `rm -rf /`. + expect(bashReq!.always).not.toContain("rm *") + expect(bashReq!.always).not.toContain("git status *") + expect(bashReq!.always).toContain("git status") + expect(bashReq!.always).toContain("rm -rf tmp") + }), + ) + }), + ) + for (const item of ps) { it.live(`parses PowerShell conditionals for permission prompts [${item.label}]`, () => withShell(