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(