From f04fb954dcd5d4149b60c79a2afedae01ff3eb75 Mon Sep 17 00:00:00 2001 From: Ankit <96786190+PiKa919@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:27:23 +0530 Subject: [PATCH 1/2] fix: enforce external directory checks for shell paths --- packages/opencode/src/tool/shell.ts | 98 +++++++++++++++++--- packages/opencode/test/tool/shell.test.ts | 106 ++++++++++++++++++++++ 2 files changed, 193 insertions(+), 11 deletions(-) diff --git a/packages/opencode/src/tool/shell.ts b/packages/opencode/src/tool/shell.ts index 1e4423e01774..2e07c0c5b790 100644 --- a/packages/opencode/src/tool/shell.ts +++ b/packages/opencode/src/tool/shell.ts @@ -1,6 +1,6 @@ import { Effect, Stream } from "effect" import os from "os" -import { createWriteStream } from "node:fs" +import { createWriteStream, realpathSync } from "node:fs" import * as Tool from "./tool" import path from "path" import { containsPath, type InstanceContext } from "../project/instance-context" @@ -125,11 +125,10 @@ function commands(node: Node) { } function unquote(text: string) { - if (text.length < 2) return text - const first = text[0] - const last = text[text.length - 1] - if ((first === '"' || first === "'") && first === last) return text.slice(1, -1) - return text + // Shell words may concatenate quoted and unquoted segments, e.g. + // `"$HOME"/.ssh/config`. Remove only quote delimiters while preserving + // characters inside each quoted segment. + return text.replace(/("[^"]*"|'[^']*')/g, (segment) => segment.slice(1, -1)) } function home(text: string) { @@ -151,11 +150,16 @@ function auto(key: string, cwd: string, shell: string) { if (name === "PSHOME") return path.dirname(shell) } +function variableValue(key: string, cwd: string, shell: string) { + return auto(key, cwd, shell) ?? envValue(key) ?? "" +} + function expand(text: string, cwd: string, shell: string) { const out = unquote(text) .replace(/\$\{env:([^}]+)\}/gi, (_, key: string) => envValue(key) || "") .replace(/\$env:([A-Za-z_][A-Za-z0-9_]*)/gi, (_, key: string) => envValue(key) || "") - .replace(/\$(HOME|PWD|PSHOME)(?=$|[\\/])/gi, (_, key: string) => auto(key, cwd, shell) || "") + .replace(/\$\{([A-Za-z_][A-Za-z0-9_]*)\}/g, (_, key: string) => variableValue(key, cwd, shell)) + .replace(/\$([A-Za-z_][A-Za-z0-9_]*)(?=$|[\\/])/g, (_, key: string) => variableValue(key, cwd, shell)) return home(out) } @@ -217,6 +221,58 @@ function pathArgs(list: Part[], ps: boolean, cmd = false) { return out } +function redirectionArgs(node: Node, ps: boolean) { + const out: string[] = [] + const owner = node.parent?.type === "redirected_statement" ? node.parent : node + if (ps) { + for (const item of owner.descendantsOfType("redirection").filter((item): item is Node => Boolean(item))) { + const value = item.text.replace(/^\s*\d*>{1,2}\s*/, "").trim() + if (value && !value.startsWith("&")) out.push(value) + } + return out + } + + // The bash grammar exposes the destination separately, so quoted strings + // containing `>` are not mistaken for redirects. + for (const item of owner.descendantsOfType("file_redirect").filter((item): item is Node => Boolean(item))) { + const value = item.childForFieldName("destination")?.text + if (value && !value.startsWith("&")) out.push(value) + } + return out +} + +function embeddedPathArgs(text: string) { + const out: string[] = [] + // A shell command can hand an entire script to another interpreter (for + // example `python -c 'open("/etc/hosts")'`). Such paths are not separate + // shell arguments, so also inspect path literals embedded in arguments. + const absolute = /(?/])\/(?:[A-Za-z0-9._~@+%-]+\/)*[A-Za-z0-9._~@+%-]+/g + for (const match of text.matchAll(absolute)) { + // The AST-based redirect scan handles this case; avoid rediscovering its + // destination when the redirect operator is separated by whitespace. + if (text.slice(0, match.index).trimEnd().endsWith(">")) continue + out.push(match[0]) + } + return out +} + +function resolveThroughSymlinks(target: string) { + let current = target + const missing: string[] = [] + while (true) { + try { + const resolved = realpathSync.native(current) + return path.join(resolved, ...missing.reverse()) + } catch (error) { + if (!(error instanceof Error) || !("code" in error) || error.code !== "ENOENT") return target + const parent = path.dirname(current) + if (parent === current) return target + missing.unshift(path.basename(current)) + current = parent + } + } +} + function preview(text: string) { if (text.length <= MAX_METADATA_LENGTH) return text return "...\n\n" + text.slice(-MAX_METADATA_LENGTH) @@ -367,7 +423,7 @@ export const ShellTool = Tool.define( }) const argPath = Effect.fn("ShellTool.argPath")(function* (arg: string, cwd: string, ps: boolean, shell: string) { - const text = ps ? expand(arg, cwd, shell) : home(unquote(arg)) + const text = expand(arg, cwd, shell) const file = text && prefix(text) if (!file || dynamic(file, ps)) return const next = ps ? provider(file) : file @@ -394,12 +450,32 @@ export const ShellTool = Tool.define( const tokens = command.map((item) => item.text) const cmd = ps || shellKind === "cmd" ? tokens[0]?.toLowerCase() : tokens[0] + const pathCandidates = new Set() + // Keep the command-specific parsing for options such as PowerShell's + // -Path/-LiteralPath, but also inspect every non-option argument. A + // filesystem path can be consumed by any executable (for example + // `head`, `ls`, `tee`, or a user script), so a command allowlist cannot + // be a security boundary. if (cmd && (FILES.has(cmd) || (shellKind === "cmd" && CMD_FILES.has(cmd)))) { - for (const arg of pathArgs(command, ps, shellKind === "cmd")) { + for (const arg of pathArgs(command, ps, shellKind === "cmd")) pathCandidates.add(arg) + } + for (const arg of command.slice(1)) { + if (!arg.text.startsWith("-")) pathCandidates.add(arg.text) + } + for (const arg of redirectionArgs(node, ps)) pathCandidates.add(arg) + for (const arg of embeddedPathArgs(source(node))) pathCandidates.add(arg) + + if (pathCandidates.size > 0) { + for (const arg of pathCandidates) { const resolved = yield* argPath(arg, cwd, ps, shell) yield* Effect.logInfo("resolved path", { arg, resolved }) - if (!resolved || containsPath(resolved, instance)) continue - const dir = (yield* fs.isDir(resolved)) ? resolved : path.dirname(resolved) + const boundary = resolved && resolveThroughSymlinks(resolved) + if (!resolved || !boundary || containsPath(boundary, instance)) continue + // Keep the user-facing path for ordinary external inputs, but do + // not scope a permission pattern to an in-project symlink that + // points outside the workspace. + const reportPath = containsPath(resolved, instance) ? boundary : resolved + const dir = (yield* fs.isDir(reportPath)) ? reportPath : path.dirname(reportPath) scan.dirs.add(dir) } } diff --git a/packages/opencode/test/tool/shell.test.ts b/packages/opencode/test/tool/shell.test.ts index a970f85d468f..bb9a93aee9f1 100644 --- a/packages/opencode/test/tool/shell.test.ts +++ b/packages/opencode/test/tool/shell.test.ts @@ -1,5 +1,6 @@ import { PermissionV1 } from "@opencode-ai/core/v1/permission" import { describe, expect } from "bun:test" +import { symlink } from "node:fs/promises" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { Cause, Effect, Exit, Layer } from "effect" import type * as Scope from "effect/Scope" @@ -341,6 +342,79 @@ describe("tool.shell permissions", () => { ), ) + each("does not let unlisted commands bypass external_directory", () => + runIn( + projectRoot, + Effect.gen(function* () { + const commands = [ + "head -1 /etc/hosts", + "ls /etc", + "printf secret >/etc/opencode-permission-test", + "cat >/etc/opencode-permission-test", + "command ls /etc", + "head -1 $HOME/.ssh/config", + "head -1 ${HOME}/.ssh/config", + 'head -1 "$HOME"/.ssh/config', + "printf secret >|/etc/opencode-permission-test", + "cat <(head -1 /etc/hosts)", + "cat {/etc,/var}/hosts", + `python3 -c 'open("/etc/hosts").read()'`, + `sh -c 'cat /etc/hosts'`, + ] + + for (const command of commands) { + const err = new Error(`stop after permission: ${command}`) + const requests: Array> = [] + expect(yield* fail({ command }, capture(requests, err))).toMatchObject({ message: err.message }) + if (!requests.find((request) => request.permission === "external_directory")) { + throw new Error(`external_directory was not requested for: ${command}`) + } + } + }), + ), + ) + + each("does not treat a quoted redirection-looking argument as a path", () => + runIn( + projectRoot, + Effect.gen(function* () { + const requests: Array> = [] + yield* run({ command: 'echo "> /etc/hosts"' }, capture(requests)) + expect(requests.find((request) => request.permission === "external_directory")).toBeUndefined() + }), + ), + ) + + it.live("does not treat a workspace symlink to an external directory as internal", () => + Effect.gen(function* () { + const project = yield* tmpdirScoped() + const outside = yield* tmpdirScoped() + const linked = path.join(project, "linked") + yield* Effect.promise(() => symlink(outside, linked, process.platform === "win32" ? "junction" : "dir")) + + const err = new Error("stop after permission") + const requests: Array> = [] + yield* runIn( + project, + Effect.gen(function* () { + expect( + yield* fail({ command: `cat ${path.join(linked, "secret.txt")}` }, capture(requests, err)), + ).toMatchObject({ message: err.message }) + }), + ) + const extDirReq = requests.find((request) => request.permission === "external_directory") + expect(extDirReq).toBeDefined() + if (extDirReq?.permission !== "external_directory") return + const expected = glob(path.join(outside, "*")) + expect(extDirReq.patterns).toContain(expected) + expect(extDirReq.metadata).toMatchObject({ + directories: [outside], + patterns: [expected], + }) + }), + ) + if (process.platform === "win32") { if (bash) { it.live("asks for nested bash command permissions [bash]", () => @@ -481,6 +555,38 @@ describe("tool.shell permissions", () => { ) } + for (const item of ps) { + it.live(`uses the shell-independent HOME path for PowerShell expansion [${item.label}]`, () => + withShell( + item, + Effect.acquireUseRelease( + Effect.sync(() => { + const previous = process.env.HOME + process.env.HOME = path.join(projectRoot, "fake-home") + return previous + }), + (previous) => + runIn( + projectRoot, + Effect.gen(function* () { + const requests: Array> = [] + yield* run({ command: 'Get-Content "$HOME/.ssh/config"' }, capture(requests)) + const extDirReq = requests.find((request) => request.permission === "external_directory") + expect(extDirReq).toBeDefined() + if (extDirReq?.permission !== "external_directory") return + expect(extDirReq.patterns).toContain(glob(path.join(os.homedir(), ".ssh", "*"))) + }), + ), + (previous) => + Effect.sync(() => { + if (previous === undefined) delete process.env.HOME + else process.env.HOME = previous + }), + ), + ), + ) + } + for (const item of ps) { it.live(`asks for external_directory permission for $PWD PowerShell paths [${item.label}]`, () => withShell( From db1ddf49d87093ce0e8b11ae6cbcc8a19368dbc7 Mon Sep 17 00:00:00 2001 From: Ankit <96786190+PiKa919@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:39:46 +0530 Subject: [PATCH 2/2] fix: scan embedded Windows paths --- packages/opencode/src/tool/shell.ts | 9 +++++++++ packages/opencode/test/tool/shell.test.ts | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/packages/opencode/src/tool/shell.ts b/packages/opencode/src/tool/shell.ts index 2e07c0c5b790..c642c0aea48b 100644 --- a/packages/opencode/src/tool/shell.ts +++ b/packages/opencode/src/tool/shell.ts @@ -253,6 +253,15 @@ function embeddedPathArgs(text: string) { if (text.slice(0, match.index).trimEnd().endsWith(">")) continue out.push(match[0]) } + + // Apply the same protection to Windows drive and UNC paths embedded in a + // nested command, including quoted paths with spaces. + const quotedWindows = /(["'])(?:(?:[A-Za-z]:[\\/])|(?:\\\\[^\\/\s]+[\\/][^\\/\s]+[\\/]))[^"']*\1/g + for (const match of text.matchAll(quotedWindows)) out.push(match[0].slice(1, -1)) + + const windows = /(?()]+/g + for (const match of text.matchAll(windows)) out.push(match[0]) + return out } diff --git a/packages/opencode/test/tool/shell.test.ts b/packages/opencode/test/tool/shell.test.ts index bb9a93aee9f1..a041abd80a87 100644 --- a/packages/opencode/test/tool/shell.test.ts +++ b/packages/opencode/test/tool/shell.test.ts @@ -362,6 +362,10 @@ describe("tool.shell permissions", () => { `python3 -c 'open("/etc/hosts").read()'`, `sh -c 'cat /etc/hosts'`, ] + if (process.platform === "win32") { + const windowsFile = path.join(process.env.WINDIR!, "win.ini").replaceAll("\\", "/") + commands.push(`python -c 'open("${windowsFile}").read()'`) + } for (const command of commands) { const err = new Error(`stop after permission: ${command}`)