fix: enforce external directory checks for shell paths

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 = /(?<![\w:>/])\/(?:[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<string>()
+        // 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)
           }
         }

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/hosts",
+          "printf secret 2>>/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<Omit<PermissionV1.Request, "id" | "sessionID" | "tool">> = []
+          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<Omit<PermissionV1.Request, "id" | "sessionID" | "tool">> = []
+        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<Omit<PermissionV1.Request, "id" | "sessionID" | "tool">> = []
+      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<Omit<PermissionV1.Request, "id" | "sessionID" | "tool">> = []
+                  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(