From ce5a3fd860b7530556576d4a10e0b41f42e1914b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pau=20Rul=C2=B7lan=20Ferragut?= <86043499+prullanferragut@users.noreply.github.com> Date: Wed, 20 May 2026 15:51:27 +0200 Subject: [PATCH] fix: strip inline env var prefixes from bash permission patterns Commands like 'GITHUB_TOKEN=x git push --force' were matching '*': allow instead of the intended deny rule because variable_assignment nodes were included in the pattern string used for permission matching. Build the permission pattern from parts() tokens (which already exclude variable_assignment children) rather than from the raw node text, then re-attach any redirection suffix from the parent redirected_statement so 'echo test > output.txt' still produces the full pattern as before. Also adds integration tests verifying the corrected behavior. Based on test groundwork from PR #16086. Fixes issue #16075 --- packages/opencode/src/tool/shell.ts | 19 +++++-- packages/opencode/test/tool/shell.test.ts | 63 +++++++++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/packages/opencode/src/tool/shell.ts b/packages/opencode/src/tool/shell.ts index 506d98466e76..6238c693be9b 100644 --- a/packages/opencode/src/tool/shell.ts +++ b/packages/opencode/src/tool/shell.ts @@ -119,10 +119,6 @@ function parts(node: Node) { return out } -function source(node: Node) { - return (node.parent?.type === "redirected_statement" ? node.parent.text : node.text).trim() -} - function commands(node: Node) { return node.descendantsOfType("command").filter((child): child is Node => Boolean(child)) } @@ -401,7 +397,20 @@ export const ShellTool = Tool.define( } if (tokens.length && (!cmd || !CWD.has(cmd))) { - scan.patterns.add(source(node)) + // Build the pattern from parts() tokens, which already exclude variable_assignment + // nodes (e.g. GITHUB_TOKEN=x), so the pattern only contains the actual command. + let pattern = tokens.join(" ") + if (node.parent?.type === "redirected_statement") { + // Re-attach any redirection suffix (e.g. "2>&1", "> file") that lives on + // the parent but outside the command node itself. + // Guard with startsWith: tree-sitter guarantees child ranges are within + // parent ranges, but .text is a string slice so we verify before slicing. + if (node.parent.text.startsWith(node.text)) { + const redirectSuffix = node.parent.text.slice(node.text.length).trim() + if (redirectSuffix) pattern = pattern + " " + redirectSuffix + } + } + scan.patterns.add(pattern) scan.always.add(BashArity.prefix(tokens).join(" ") + " *") } } diff --git a/packages/opencode/test/tool/shell.test.ts b/packages/opencode/test/tool/shell.test.ts index fe4f5a483468..274e480a3cd2 100644 --- a/packages/opencode/test/tool/shell.test.ts +++ b/packages/opencode/test/tool/shell.test.ts @@ -1023,6 +1023,69 @@ describe("tool.shell permissions", () => { ) }), ) + + each("strips inline env var prefix from permission pattern", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped() + yield* runIn( + tmp, + Effect.gen(function* () { + const err = new Error("stop after permission") + const requests: Array> = [] + yield* fail( + { command: 'CI=true git commit -m "test"', description: "Commit with env prefix" }, + capture(requests, err), + ) + const bashReq = requests.find((r) => r.permission === "bash") + expect(bashReq).toBeDefined() + expect(bashReq!.patterns).toContain('git commit -m "test"') + expect(bashReq!.patterns).not.toContain('CI=true git commit -m "test"') + }), + ) + }), + ) + + each("strips multiple inline env var prefixes from permission pattern", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped() + yield* runIn( + tmp, + Effect.gen(function* () { + const err = new Error("stop after permission") + const requests: Array> = [] + yield* fail( + { command: "FOO=1 BAR=2 echo hello", description: "Echo with multiple env prefixes" }, + capture(requests, err), + ) + const bashReq = requests.find((r) => r.permission === "bash") + expect(bashReq).toBeDefined() + expect(bashReq!.patterns).toContain("echo hello") + expect(bashReq!.patterns).not.toContain("FOO=1") + }), + ) + }), + ) + + each("strips env var prefix and preserves redirection in permission pattern", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped() + yield* runIn( + tmp, + Effect.gen(function* () { + const err = new Error("stop after permission") + const requests: Array> = [] + yield* fail( + { command: "CI=true echo hello > output.txt", description: "Redirect with env prefix" }, + capture(requests, err), + ) + const bashReq = requests.find((r) => r.permission === "bash") + expect(bashReq).toBeDefined() + expect(bashReq!.patterns).toContain("echo hello > output.txt") + expect(bashReq!.patterns).not.toContain("CI=true") + }), + ) + }), + ) }) describe("tool.shell abort", () => {