From f9c19c74f9bd70bf6f152131019e330d6ecd198d Mon Sep 17 00:00:00 2001 From: RaghwendraSingh Date: Wed, 4 Nov 2026 00:59:42 +0530 Subject: [PATCH 1/3] fix(tool): apply `external_directory: deny` to every path a bash command names The bash tool only checked a fixed list of file commands (`cat`, `ls`, `cd`, `rm`, ...) against `external_directory`, so under a configured deny a command could still read, write or execute outside the project: `head /abs/file`, `echo x > /tmp/x`, `/host/.venv/bin/python`, `cd /outside && ...`. - When the effective `external_directory` rule for `*` is `deny`, also check absolute, `~` and `..` arguments, a command run by path, redirection targets and every directory a command runs in, using the existing tree-sitter walk - A denied path refuses the call with an error naming the path; a path the config allows (or sets to `ask`) goes through the normal permission request - Permitted out-of-project paths are recorded as `metadata.external_paths` - Unset, the default `ask`, and `allow` keep today's behaviour unchanged Co-Authored-By: Claude Opus 5.5 --- packages/opencode/src/tool/bash.ts | 143 ++++++++- .../bash-external-directory-gate.test.ts | 71 +++++ .../test/tool/bash-external-directory.test.ts | 275 ++++++++++++++++++ 3 files changed, 488 insertions(+), 1 deletion(-) create mode 100644 packages/opencode/test/agent/bash-external-directory-gate.test.ts create mode 100644 packages/opencode/test/tool/bash-external-directory.test.ts diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 0ef418bdae..b127f47097 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -21,6 +21,9 @@ import { Plugin } from "@/plugin" import { Global } from "@/global" // altimate_change start — upstream_fix: resolve literal shell paths for permission matching import type { Node } from "web-tree-sitter" +import type { PermissionV1 } from "@opencode-ai/core/v1/permission" +import { Wildcard } from "@opencode-ai/core/util/wildcard" +import { PermissionEvaluate } from "@/permission/evaluate" function shellWords(node: Node, includeNumbers = false) { const types = ["command_name", "word", "string", "raw_string", "concatenation"] @@ -166,6 +169,106 @@ function quoteLike(word: Node, text: string) { } // altimate_change end +// altimate_change start — apply a configured `external_directory: deny` to every path a command names +// The operand check above only covers a fixed list of file commands. When the effective +// external_directory rule for "*" is deny, every literal path the command names is checked +// as well: absolute, `~` and `..` arguments, a command run by path, redirection targets and +// the directories commands run in. Under the default ("ask") and "allow" nothing changes. +// This is a guard rail over literal command text, not an OS sandbox. + +// Character devices that are not files in any directory. +const DEVICE_PATHS = new Set([ + "/dev/null", + "/dev/stdin", + "/dev/stdout", + "/dev/stderr", + "/dev/tty", + "/dev/zero", + "/dev/random", + "/dev/urandom", +]) +// Running a system binary by its full path (`/usr/bin/env python`) is not a project escape. +const SYSTEM_BIN_DIRS = new Set([ + "/bin", + "/sbin", + "/usr/bin", + "/usr/sbin", + "/usr/local/bin", + "/usr/local/sbin", + "/opt/homebrew/bin", +]) + +export function externalDirectoryDenied(rules: PermissionV1.Ruleset | undefined) { + const rule = rules?.findLast((item) => Wildcard.match("external_directory", item.permission) && item.pattern === "*") + return rule?.action === "deny" +} + +function namesPath(arg: string) { + return path.isAbsolute(arg) || /(^|[\\/])\.\.([\\/]|$)/.test(arg) +} + +function isDevice(target: string) { + return DEVICE_PATHS.has(target) || /^\/dev\/fd\/\d+$/.test(target) +} + +// Literal path candidates a command names, resolved against every directory it may run in. +function commandPathCandidates(root: Node, cwd: string) { + const candidates = new Set() + for (const { node, cwds } of shellCommands(root, [cwd])) { + for (const commandCwd of cwds) candidates.add(commandCwd) + const words = shellWords(node, true) + const name = words[0] ? literalWord(words[0]) : undefined + if (name?.includes("/")) { + for (const commandCwd of cwds) { + const target = path.resolve(commandCwd, name) + if (!SYSTEM_BIN_DIRS.has(path.dirname(target))) candidates.add(target) + } + } + for (const word of words.slice(1)) { + const arg = literalWord(word) + // Option values (`--out=/x`) and URLs are not read as paths. + if (!arg || arg.startsWith("-") || !namesPath(arg)) continue + for (const commandCwd of cwds) candidates.add(path.resolve(commandCwd, arg)) + } + } + // Redirection targets anywhere in the script; heredoc bodies hold no file_redirect nodes. + const stack: Node[] = [root] + while (stack.length) { + const node = stack.pop()! + if (node.type === "file_redirect") { + const destination = node.childForFieldName("destination") ?? node.namedChildren.at(-1) ?? null + const target = literalWord(destination) + if (target && namesPath(target)) candidates.add(path.resolve(cwd, target)) + } + for (const child of node.namedChildren) if (child) stack.push(child) + } + return [...candidates].filter((target) => !isDevice(target)) +} + +// Out-of-project locations a command names, as the directory an external_directory rule matches. +async function externalCommandPaths(root: Node, cwd: string) { + const found: { path: string; dir: string }[] = [] + for (const target of commandPathCandidates(root, cwd)) { + const real = await realpathThroughParents(target) + if (!real || isDevice(real)) continue + // Text whose first segment does not exist (an awk or sed program such as `/x/ {print}`) + // names no real location. + const first = real.split(path.sep).filter(Boolean)[0] + if (first && !(await fs.stat(path.join(path.parse(real).root, first)).catch(() => undefined))) continue + if (Instance.containsPath(real)) continue + const dir = (await Filesystem.isDir(real)) ? real : path.dirname(real) + found.push({ path: real, dir }) + } + return found +} + +function directoryGlob(dir: string) { + // Preserve POSIX-looking paths with /s, even on Windows + if (dir.startsWith("/")) return `${dir.replace(/[\\/]+$/, "")}/*` + return path.join(dir, "*") +} +// altimate_change end + // altimate_change start — run-mode markers must not reach bash child processes. // `run` sets ALTIMATE_RUN_MODE on its own process to arm run-mode-only // mechanisms (DONE-termination gate, starvation directives, doom-loop @@ -417,6 +520,39 @@ export const BashTool = Tool.define("bash", async () => { } } + // altimate_change start — apply a configured `external_directory: deny` to every path a command names + const guarded = externalDirectoryDenied(ctx.permissions) + const externalPaths = new Set() + if (guarded) { + const named = await externalCommandPaths(tree.rootNode, cwd) + // A directory the operand check found is listed only when no named path inside it is. + const found = [ + ...Array.from(directories) + .filter((dir) => !named.some((item) => item.path === dir || item.path.startsWith(dir + path.sep))) + .map((dir) => ({ path: dir, dir })), + ...named, + ] + const denied = new Set() + for (const item of found) { + externalPaths.add(item.path) + // Paths here are symlink-resolved, so rules are matched with their resolved spelling too. + const rule = PermissionEvaluate.evaluateResolved( + "external_directory", + directoryGlob(item.dir), + ctx.permissions!, + ) + if (rule.action === "deny") denied.add(item.path) + else directories.add(item.dir) + } + if (denied.size > 0) { + throw new Error( + `external_directory is denied: this command touches ${denied.size === 1 ? "a path" : "paths"} outside the project (${Instance.worktree === "/" ? Instance.directory : Instance.worktree}): ${[...denied].join(", ")}. ` + + "Use paths inside the project instead.", + ) + } + } + // altimate_change end + if (directories.size > 0) { const globs = Array.from(directories).map((dir) => { // Preserve POSIX-looking paths with /s, even on Windows @@ -427,7 +563,9 @@ export const BashTool = Tool.define("bash", async () => { permission: "external_directory", patterns: globs, always: globs, - metadata: {}, + // altimate_change start — under `external_directory: deny` the globs are symlink-resolved paths + metadata: guarded ? { resolvedPatterns: globs } : {}, + // altimate_change end }) } @@ -663,6 +801,9 @@ export const BashTool = Tool.define("bash", async () => { // exit status without declaring an execution failure to the wrapper. ...(failureKind && { failure_kind: failureKind }), // altimate_change end + // altimate_change start — out-of-project paths permitted under `external_directory: deny` + ...(externalPaths.size > 0 && { external_paths: [...externalPaths] }), + // altimate_change end }, output, } diff --git a/packages/opencode/test/agent/bash-external-directory-gate.test.ts b/packages/opencode/test/agent/bash-external-directory-gate.test.ts new file mode 100644 index 0000000000..7999df2146 --- /dev/null +++ b/packages/opencode/test/agent/bash-external-directory-gate.test.ts @@ -0,0 +1,71 @@ +// altimate_change start — which agent rulesets turn on the bash external_directory path guard +import { afterEach, expect } from "bun:test" +import { Effect, Layer } from "effect" +import { disposeAllInstances } from "../fixture/fixture" +import { testEffect } from "../lib/effect" +import { Agent } from "../../src/agent/agent" +import { Auth } from "../../src/auth" +import { Config } from "../../src/config/config" +import { RuntimeFlags } from "../../src/effect/runtime-flags" +import { Plugin } from "../../src/plugin" +import { Provider } from "../../src/provider/provider" +import { Skill } from "../../src/skill" +import { LocationServiceMap } from "@opencode-ai/core/location-layer" +import { externalDirectoryDenied } from "../../src/tool/bash" +import { Permission } from "../../src/permission" + +const it = testEffect( + Agent.layer.pipe( + Layer.provide(Plugin.defaultLayer), + Layer.provide(Provider.defaultLayer), + Layer.provide(Auth.defaultLayer), + Layer.provide(Config.defaultLayer), + Layer.provide(Skill.defaultLayer), + Layer.provide(LocationServiceMap.layer), + Layer.provide(RuntimeFlags.layer({})), + ), +) + +afterEach(async () => { + await disposeAllInstances() +}) + +it.instance("no built-in agent that can run bash turns the guard on without configuration", () => + Effect.gen(function* () { + const agents = yield* Agent.Service.use((svc) => svc.list()) + expect(agents.length).toBeGreaterThan(0) + // The guard only engages where external_directory is already denied for "*". Among the + // built-in agents those are the ones whose ruleset denies everything, bash included, so no + // built-in agent that can run bash changes behaviour. + for (const agent of agents) { + if (!externalDirectoryDenied(agent.permission)) continue + expect([agent.name, Permission.evaluate("bash", "*", agent.permission).action]).toEqual([agent.name, "deny"]) + } + for (const name of ["builder", "general", "plan", "reviewer", "explore"]) + expect([name, externalDirectoryDenied(agents.find((agent) => agent.name === name)!.permission)]).toEqual([ + name, + false, + ]) + }), +) + +it.instance( + "configured external_directory: deny turns the guard on", + () => + Effect.gen(function* () { + const builder = yield* Agent.Service.use((svc) => svc.get("builder")) + expect(externalDirectoryDenied(builder!.permission)).toBe(true) + }), + { config: { permission: { external_directory: "deny" } } }, +) + +it.instance( + "configured external_directory: allow or ask leaves the guard off", + () => + Effect.gen(function* () { + const builder = yield* Agent.Service.use((svc) => svc.get("builder")) + expect(externalDirectoryDenied(builder!.permission)).toBe(false) + }), + { config: { permission: { external_directory: { "*": "ask", "/srv/*": "deny" } } } }, +) +// altimate_change end diff --git a/packages/opencode/test/tool/bash-external-directory.test.ts b/packages/opencode/test/tool/bash-external-directory.test.ts new file mode 100644 index 0000000000..9e94c783a0 --- /dev/null +++ b/packages/opencode/test/tool/bash-external-directory.test.ts @@ -0,0 +1,275 @@ +// altimate_change start — the bash tool applies a configured `external_directory: deny` to every path a command names +import { describe, expect, test } from "bun:test" +import fs from "fs/promises" +import os from "os" +import path from "path" +import { BashTool } from "../../src/tool/bash" +import { Instance } from "../../src/project/instance" +import { tmpdir, provideTestInstance } from "../fixture/fixture" +import type { PermissionNext } from "../../src/permission/next" +import { PermissionEvaluate } from "../../src/permission/evaluate" +import { Truncate } from "../../src/tool/truncation" +import { SessionID, MessageID } from "../../src/session/schema" +import { initTool } from "../altimate/tool-fixture" + +type Request = Omit + +const baseCtx = { + sessionID: SessionID.make("ses_test"), + messageID: MessageID.make("msg_test"), + callID: "", + agent: "build", + abort: AbortSignal.any([]), + messages: [], + metadata: () => {}, + ask: async () => {}, +} + +async function provideInstance(directory: string, fn: () => Promise): Promise { + return provideTestInstance({ directory, fn: (instanceCtx) => Instance.restore(instanceCtx, fn) }) +} + +// The defaults an agent starts with, followed by the user's `permission` config, as the agent builds them. +function rules(external: Record | undefined): PermissionNext.Ruleset { + return [ + { permission: "*", pattern: "*", action: "allow" }, + { permission: "external_directory", pattern: "*", action: "ask" }, + ...Object.entries(external ?? {}).map(([pattern, action]) => ({ + permission: "external_directory", + pattern, + action, + })), + { permission: "external_directory", pattern: Truncate.GLOB, action: "allow" }, + ] +} + +// Mirrors Permission.ask: a deny rule rejects, an ask rule prompts (recorded), allow passes. +function context(ruleset: PermissionNext.Ruleset | undefined) { + const requests: Request[] = [] + const prompts: Request[] = [] + return { + requests, + prompts, + ctx: { + ...baseCtx, + ...(ruleset && { permissions: ruleset }), + ask: async (request: Request) => { + requests.push(request) + if (!ruleset) return + for (const { rule } of PermissionEvaluate.evaluateRequestPatterns(request, ruleset)) { + if (rule.action === "deny") throw new Error("PermissionDeniedError") + if (rule.action === "ask") prompts.push(request) + } + }, + }, + } +} + +async function fixture() { + const outside = await tmpdir({ + init: async (dir) => { + await fs.mkdir(path.join(dir, "sibling")) + await Bun.write(path.join(dir, "sibling", "output.md"), "private content") + await fs.mkdir(path.join(dir, "host", ".venv", "bin"), { recursive: true }) + const python = path.join(dir, "host", ".venv", "bin", "python") + await Bun.write(python, `#!/bin/sh\necho ran > "${path.join(dir, "host", "ran.txt")}"\necho host-python\n`) + await fs.chmod(python, 0o755) + }, + }) + const project = await tmpdir({ + git: true, + init: (dir) => Bun.write(path.join(dir, "inside.txt"), "inside content"), + }) + return { outside, project } +} + +describe("tool.bash external_directory: deny", () => { + test.each([ + ["absolute read", (o: string) => `cat /etc/hosts`, /etc\/hosts/], + [ + "read by a command outside the operand list", + (o: string) => `head -n 1 ${o}/sibling/output.md`, + /sibling\/output\.md/, + ], + ["home directory listing", () => `ls ~`, undefined], + ["home-relative read", () => `cat ~/.profile-that-may-not-exist`, undefined], + ["cd outside the project", (o: string) => `cd ${o}/sibling && echo hi`, /sibling/], + ["redirection outside the project", (o: string) => `echo x > ${o}/written.txt`, /written\.txt/], + ["append redirection outside the project", (o: string) => `echo x >> ${o}/written.txt`, /written\.txt/], + [ + "host checkout virtualenv by path", + (o: string) => `${o}/host/.venv/bin/python -c 'print(1)'`, + /host\/\.venv\/bin\/python/, + ], + ["host checkout virtualenv after cd", (o: string) => `cd ${o}/host && .venv/bin/python -V`, /host/], + ])("refuses %s and names the path", async (_name, command, pathPattern) => { + const { outside, project } = await fixture() + await using _outside = outside + await using _project = project + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const { ctx } = context(rules({ "*": "deny" })) + const run = bash.execute({ command: command(outside.path), description: "Touch an outside path" }, ctx) + await expect(run).rejects.toThrow(/external_directory is denied/) + if (pathPattern) await expect(run).rejects.toThrow(pathPattern) + else await expect(run).rejects.toThrow(os.homedir().split(path.sep).filter(Boolean).at(-1)!) + }) + // Nothing ran: no write landed outside and the host interpreter never executed. + expect(await Bun.file(path.join(outside.path, "written.txt")).exists()).toBe(false) + expect(await Bun.file(path.join(outside.path, "host", "ran.txt")).exists()).toBe(false) + }) + + test("refuses an outside workdir and names it", async () => { + const { outside, project } = await fixture() + await using _outside = outside + await using _project = project + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const { ctx } = context(rules({ "*": "deny" })) + await expect( + bash.execute({ command: "echo hi", workdir: outside.path, description: "Run outside" }, ctx), + ).rejects.toThrow(new RegExp(`external_directory is denied.*${path.basename(outside.path)}`)) + }) + }) + + test("allows paths inside the project, URLs, option values, programs, devices and heredoc bodies", async () => { + const { outside, project } = await fixture() + await using _outside = outside + await using _project = project + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const { ctx, requests } = context(rules({ "*": "deny" })) + for (const command of [ + "cat inside.txt", + `cat ${project.path}/inside.txt`, + "echo x > ./out.txt && cat out.txt", + "mkdir -p nested/deeper && ls nested", + "echo https://example.com/a/b", + "echo --out=/etc/hosts", + "awk '/inside/ {print}' inside.txt", + "sed -e 's/inside/x/' inside.txt", + "echo hi 2>/dev/null", + "echo hi > /dev/null", + "/usr/bin/env echo hi", + `cat <<'EOF'\n/etc/hosts\n${outside.path}/sibling/output.md\nEOF`, + ]) { + const result = await bash.execute({ command, description: "Stay inside" }, ctx) + expect(result.metadata.exit).toBe(0) + expect(result.metadata.external_paths).toBeUndefined() + } + expect(requests.filter((request) => request.permission === "external_directory")).toEqual([]) + }) + }) + + test("allows a directory the config allows and records it in metadata", async () => { + const { outside, project } = await fixture() + await using _outside = outside + await using _project = project + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const allowed = path.join(outside.path, "sibling") + const { ctx } = context(rules({ "*": "deny", [path.join(allowed, "*")]: "allow" })) + const result = await bash.execute( + { command: `echo x > ${allowed}/scratch.txt && cat ${allowed}/output.md`, description: "Use scratch dir" }, + ctx, + ) + expect(result.metadata.exit).toBe(0) + expect(result.output).toBe("private content") + expect(result.metadata.external_paths).toEqual( + expect.arrayContaining([path.join(allowed, "scratch.txt"), path.join(allowed, "output.md")]), + ) + expect(await Bun.file(path.join(allowed, "scratch.txt")).text()).toBe("x\n") + }) + }) + + test("allows the tool-output directory the file tools allow", async () => { + await using project = await tmpdir({ git: true }) + await fs.mkdir(Truncate.DIR, { recursive: true }) + const target = path.join(Truncate.DIR, `bash-guard-${Math.random().toString(36).slice(2)}.txt`) + await Bun.write(target, "saved output") + try { + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const { ctx } = context(rules({ "*": "deny" })) + const result = await bash.execute({ command: `cat ${target}`, description: "Read saved output" }, ctx) + expect(result.output).toBe("saved output") + expect(result.metadata.external_paths).toEqual([await fs.realpath(target)]) + }) + } finally { + await fs.rm(target, { force: true }) + } + }) + + test("a directory configured as ask goes through the normal permission prompt", async () => { + const { outside, project } = await fixture() + await using _outside = outside + await using _project = project + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const askDir = path.join(outside.path, "sibling") + const { ctx, prompts } = context(rules({ "*": "deny", [path.join(askDir, "*")]: "ask" })) + const result = await bash.execute({ command: `head -n 1 ${askDir}/output.md`, description: "Read" }, ctx) + expect(result.output).toBe("private content") + expect(prompts.map((request) => request.permission)).toEqual(["external_directory"]) + expect(prompts[0].patterns).toContain(path.join(askDir, "*")) + }) + }) +}) + +// Without `external_directory: deny` (unset, the default "ask", or "allow") the tool behaves +// exactly as before: the same permission requests, the same outputs and the same metadata keys. +describe("tool.bash without external_directory: deny", () => { + for (const [label, ruleset] of [ + ["no ruleset", undefined], + ["default ask", rules(undefined)], + ["allow", rules({ "*": "allow" })], + ] as const) { + test(`keeps today's behaviour (${label})`, async () => { + const { outside, project } = await fixture() + await using _outside = outside + await using _project = project + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const sibling = path.join(outside.path, "sibling") + const observed: Array<{ command: string; output: string; keys: string[]; external: string[][] }> = [] + for (const command of [ + `head -n 1 ${sibling}/output.md`, + `echo x > ${outside.path}/written.txt && cat ${outside.path}/written.txt`, + `${outside.path}/host/.venv/bin/python`, + `cat ${sibling}/output.md`, + "cat inside.txt", + ]) { + const { ctx, requests } = context(ruleset) + const result = await bash.execute({ command, description: "Fixed command" }, ctx) + observed.push({ + command: command.replaceAll(outside.path, ""), + output: result.output.trim(), + keys: Object.keys(result.metadata).sort(), + external: requests + .filter((request) => request.permission === "external_directory") + .map((request) => request.patterns.map((pattern) => pattern.replaceAll(outside.path, ""))), + }) + } + const keys = ["description", "exit", "exit_code", "output", "truncated"] + expect(observed).toEqual([ + { command: "head -n 1 /sibling/output.md", output: "private content", keys, external: [] }, + { + command: "echo x > /written.txt && cat /written.txt", + output: "x", + keys, + external: [["/*"]], + }, + { command: "/host/.venv/bin/python", output: "host-python", keys, external: [] }, + { + command: "cat /sibling/output.md", + output: "private content", + keys, + external: [["/sibling/*"]], + }, + { command: "cat inside.txt", output: "inside content", keys, external: [] }, + ]) + }) + }) + } +}) +// altimate_change end From 1dd20cd7204ab8878e14603cbc3fb1790ae575c8 Mon Sep 17 00:00:00 2001 From: RaghwendraSingh Date: Thu, 5 Nov 2026 15:39:35 +0530 Subject: [PATCH 2/3] =?UTF-8?q?fix(tool):=20bash=20`external=5Fdirectory:?= =?UTF-8?q?=20deny`=20guard=20v2=20=E2=80=94=20per-command=20cwd,=20every?= =?UTF-8?q?=20literal=20form?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review on the bash path guard. Under `external_directory: deny` the guard now walks the script itself: - redirections resolve in the directory of the command that owns them (`a && b > f` applies to `b`), and every word of a `file_redirect` destination is checked (`cat > out /outside/x`) - `cd x && y` runs `y` only in `x`, so in-project `..` after `cd` is no longer refused; `cd x; y` / `||` keep the earlier directory possible; `pushd`/`popd` move the tracked directory like `cd` - assignment values (prefix, standalone, `export`/`declare`), split on `:` for `PATH`-style values - option values: `--x=v`, `key=v` (`dd of=`), attached `-oPATH` - literal `sh|bash|zsh|dash|ksh -c` strings and `eval` are parsed with the same parser (depth cap 3; deeper is refused) - relative words are resolved through symlinks, so an in-project link to outside is judged by where it points - Windows: the first-segment check takes the segment after the drive root (`firstSegmentPath`, unit-tested with `path.win32`) - `for` values, `[[ ]]` words, `case` words, `$'...'` without escapes; `~user` for another user is refused - awk/sed/grep program or pattern operands are not paths; devices are left out of the request; a wrapper's system-binary argument (`nohup /usr/bin/true`) is not an escape - a not-yet-created allowed directory is matched as itself - at most 2000 candidates per command, and `ctx.abort` is honoured - the external_directory request reuses `directoryGlob` Unset, `ask` and `allow` keep the main-branch behaviour: all new code stays behind `guarded`. Co-Authored-By: Claude Opus 5.5 --- packages/opencode/src/tool/bash.ts | 446 +++++++++++++++--- .../bash-external-directory-gate.test.ts | 12 +- .../test/tool/bash-external-directory.test.ts | 176 ++++++- 3 files changed, 567 insertions(+), 67 deletions(-) diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index b127f47097..03225b75f8 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -24,6 +24,7 @@ import type { Node } from "web-tree-sitter" import type { PermissionV1 } from "@opencode-ai/core/v1/permission" import { Wildcard } from "@opencode-ai/core/util/wildcard" import { PermissionEvaluate } from "@/permission/evaluate" +import { userInfo } from "os" function shellWords(node: Node, includeNumbers = false) { const types = ["command_name", "word", "string", "raw_string", "concatenation"] @@ -76,12 +77,17 @@ const CD_FLAGS = ["-L", "-P", "-e", "-@"] // or sit behind `||`, a conditional or a background marker, so the previous directories // stay possible. An unresolvable target (`cd $VAR`, `cd -`) adds nothing: the extra // directories only ever add aliases, which can tighten a decision but never relax it. -function afterCd(cwds: string[], words: Node[]): string[] { +// The literal directory `cd` moves to: home without an operand, undefined when not literal. +function cdTarget(words: Node[]): string | undefined { const args = words.slice(1).map(literalWord) let i = 0 while (i < args.length && args[i] !== undefined && CD_FLAGS.includes(args[i]!)) i++ if (args[i] === "--") i++ - const target = i < args.length ? args[i] : homedir() + return i < args.length ? args[i] : homedir() +} + +function afterCd(cwds: string[], words: Node[]): string[] { + const target = cdTarget(words) if (target === undefined || target === "-") return cwds // Newly reachable directories come first so the cap drops the oldest speculative // branches, never the directory a straight-line script actually enters. @@ -171,9 +177,11 @@ function quoteLike(word: Node, text: string) { // altimate_change start — apply a configured `external_directory: deny` to every path a command names // The operand check above only covers a fixed list of file commands. When the effective -// external_directory rule for "*" is deny, every literal path the command names is checked -// as well: absolute, `~` and `..` arguments, a command run by path, redirection targets and -// the directories commands run in. Under the default ("ask") and "allow" nothing changes. +// external_directory rule for "*" is deny, every literal path the command text names is checked +// as well: arguments (including `--opt=value`, `key=value` and `-oVALUE` values), a command run by +// path, redirection targets, assignment values, `for`/`[[ ]]`/`case` words, the directories commands +// run in, and literal `sh -c`/`eval` strings. Each is resolved against the directory its command +// runs in and through symlinks. Under the default ("ask") and "allow" nothing changes. // This is a guard rail over literal command text, not an OS sandbox. // Character devices that are not files in any directory. @@ -197,67 +205,376 @@ const SYSTEM_BIN_DIRS = new Set([ "/usr/local/sbin", "/opt/homebrew/bin", ]) +// Programs that run the program named in their arguments. +const WRAPPERS = new Set(["env", "nohup", "command", "time", "xargs", "exec", "nice", "stdbuf", "timeout", "sudo"]) +// Shells whose `-c` string is parsed with the same parser. +const SHELLS = new Set(["sh", "bash", "zsh", "dash", "ksh"]) +// Programs whose first operand (or `-e` value) is a program or pattern, not a path. +const PATTERN_PROGRAMS = new Set(["awk", "gawk", "mawk", "nawk", "sed", "gsed", "grep", "egrep", "fgrep", "rg"]) +const PATTERN_OPTIONS = ["-e", "--expression", "--regexp", "-f", "--file"] +const PATTERN_VALUE_OPTIONS = ["-F", "-v", "-A", "-B", "-C", "-m", "--max-count", "-l", "-s"] +const MAX_SHELL_DEPTH = 3 +// Bounds the realpath/stat work one command can cause. +const MAX_GUARD_CANDIDATES = 2000 export function externalDirectoryDenied(rules: PermissionV1.Ruleset | undefined) { const rule = rules?.findLast((item) => Wildcard.match("external_directory", item.permission) && item.pattern === "*") return rule?.action === "deny" } -function namesPath(arg: string) { - return path.isAbsolute(arg) || /(^|[\\/])\.\.([\\/]|$)/.test(arg) -} - function isDevice(target: string) { return DEVICE_PATHS.has(target) || /^\/dev\/fd\/\d+$/.test(target) } -// Literal path candidates a command names, resolved against every directory it may run in. -function commandPathCandidates(root: Node, cwd: string) { - const candidates = new Set() - for (const { node, cwds } of shellCommands(root, [cwd])) { - for (const commandCwd of cwds) candidates.add(commandCwd) - const words = shellWords(node, true) - const name = words[0] ? literalWord(words[0]) : undefined - if (name?.includes("/")) { - for (const commandCwd of cwds) { - const target = path.resolve(commandCwd, name) - if (!SYSTEM_BIN_DIRS.has(path.dirname(target))) candidates.add(target) +// The top-level entry a path starts with (`/etc` for `/etc/hosts`, `C:\Users` for `C:\Users\x`). +export function firstSegmentPath(real: string, p: path.PlatformPath = path) { + const root = p.parse(real).root + const first = real.slice(root.length).split(p.sep).filter(Boolean)[0] + return first ? p.join(root, first) : undefined +} + +const GUARD_WORD_TYPES = ["command_name", "word", "string", "raw_string", "concatenation", "number", "ansi_c_string"] + +function guardWords(node: Node) { + return node.namedChildren.filter((child): child is Node => child !== null && GUARD_WORD_TYPES.includes(child.type)) +} + +type Literal = { text: string } | { refuse: string } | undefined + +// A word's literal value, also covering `$'…'` without escapes and `~user`. +function guardLiteral(node: Node | null): Literal { + if (!node) return + if (node.type === "ansi_c_string") return node.text.includes("\\") ? undefined : { text: node.text.slice(2, -1) } + if (node.type === "concatenation" || node.type === "command_name") { + const parts = node.namedChildren.map(guardLiteral) + if (parts.some((part) => part && "refuse" in part)) return { refuse: node.text } + if (parts.every((part) => part !== undefined)) return { text: parts.map((part) => (part as { text: string }).text).join("") } + return + } + const user = node.type === "word" ? /^~([A-Za-z_][\w.-]*)(?=\/|$)/.exec(node.text) : null + if (user) { + // `~name` is another user's home unless it names the current user. + if (user[1] !== currentUser()) return { refuse: node.text } + const rest = node.text.slice(user[0].length) + if (/[~$*?{\[]/.test(rest.replace(/\\./gs, ""))) return + return { text: homedir() + rest.replace(/\\(.)/gs, "$1") } + } + const text = literalWord(node) + return text === undefined ? undefined : { text } +} + +function currentUser() { + try { + return userInfo().username + } catch { + return undefined + } +} + +function expandHome(text: string) { + return text === "~" || text.startsWith("~/") ? homedir() + text.slice(1) : text +} + +// The path-like values one argument carries: the word itself, the value of `--x=v` / `key=v`, +// or the attached value of a short option (`-o/path`). +function argumentValues(arg: string) { + if (arg.startsWith("--")) { + const eq = arg.indexOf("=") + return eq > 0 ? [arg.slice(eq + 1)] : [] + } + if (arg.startsWith("-")) { + const attached = arg.slice(2) + return /^-[A-Za-z]/.test(arg) && /^(~|\.\.|.*\/)/.test(attached) ? [attached] : [] + } + const assignment = /^[A-Za-z_][\w.-]*=(.*)$/s.exec(arg) + return assignment ? [arg, assignment[1]] : [arg] +} + +// Literal pieces of an assignment value; `PATH`-style values are split on `:`. +function assignmentValues(value: Node | null): string[] { + if (!value) return [] + const literal = guardLiteral(value) + let text = literal && "text" in literal ? literal.text : value.text + if (!literal && value.type === "string") text = text.slice(1, -1) + return text + .split(":") + .filter((piece) => piece && !/[$`*?[]/.test(piece.replace(/\\./gs, ""))) + .map((piece) => expandHome(piece.replace(/\\(.)/gs, "$1"))) +} + +interface GuardScan { + candidates: Set + refusals: Set + stack: string[][] + depth: number + parse: (text: string) => Node | undefined +} +interface Cwds { + // directories after the node when it succeeded, and in any outcome + ok: string[] + any: string[] +} + +const union = (...lists: string[][]) => [...new Set(lists.flat())].slice(0, MAX_TRACKED_DIRECTORIES) +const same = (cwds: string[]): Cwds => ({ ok: cwds, any: cwds }) + +function addCandidate(scan: GuardScan, cwds: string[], value: string) { + const target = expandHome(value) + for (const cwd of cwds) scan.candidates.add(path.resolve(cwd, target)) +} + +// Walk the script in execution order, tracking the directories each command can run in. +// `cd x && y` runs y only in x; `cd x; y` and `cd x || y` keep the earlier directory possible. +function scanNode(node: Node | null, cwds: string[], scan: GuardScan): Cwds { + if (!node) return same(cwds) + switch (node.type) { + case "command": + return scanCommand(node, cwds, scan) + case "variable_assignment": + for (const value of assignmentValues(node.childForFieldName("value"))) addCandidate(scan, cwds, value) + scanChildren(node, cwds, scan) + return same(cwds) + case "file_redirect": + // `destination` holds several words when arguments follow the target (`cat > out /x`). + for (const child of node.namedChildren) { + if (!child || child.type === "file_descriptor") continue + const literal = guardLiteral(child) + if (literal && "refuse" in literal) scan.refusals.add(literal.refuse) + else if (literal) addCandidate(scan, cwds, literal.text) } + scanChildren(node, cwds, scan) + return same(cwds) + case "redirected_statement": { + const body = node.childForFieldName("body") + const redirects = node.namedChildren.filter((child): child is Node => !!child && child.id !== body?.id) + return scanRedirected(body, redirects, cwds, scan) } - for (const word of words.slice(1)) { - const arg = literalWord(word) - // Option values (`--out=/x`) and URLs are not read as paths. - if (!arg || arg.startsWith("-") || !namesPath(arg)) continue - for (const commandCwd of cwds) candidates.add(path.resolve(commandCwd, arg)) + case "list": { + const [left, right] = node.namedChildren + const or = node.children.some((child) => child?.type === "||") + const l = scanNode(left, cwds, scan) + const r = scanNode(right, or ? l.any : l.ok, scan) + return { ok: or ? union(l.ok, r.ok) : r.ok, any: union(l.any, r.any) } } + case "pipeline": + case "subshell": + case "command_substitution": + case "process_substitution": + case "function_definition": { + // Pipeline stages, subshells and function bodies keep their directory changes local. + if (node.type === "pipeline") scanChildren(node, cwds, scan) + else scanSequence(node, cwds, scan) + return same(cwds) + } + case "for_statement": + for (const value of node.childrenForFieldName("value")) addWord(scan, cwds, value) + return same(scanSequence(node, cwds, scan)) + case "case_statement": + addWord(scan, cwds, node.childForFieldName("value")) + return same(scanSequence(node, cwds, scan)) + case "test_command": { + const stack = [...node.namedChildren] + while (stack.length) { + const child = stack.pop() + if (!child) continue + if (GUARD_WORD_TYPES.includes(child.type)) addWord(scan, cwds, child) + else stack.push(...child.namedChildren) + } + scanChildren(node, cwds, scan) + return same(cwds) + } + default: + return same(scanSequence(node, cwds, scan)) } - // Redirection targets anywhere in the script; heredoc bodies hold no file_redirect nodes. - const stack: Node[] = [root] - while (stack.length) { - const node = stack.pop()! - if (node.type === "file_redirect") { - const destination = node.childForFieldName("destination") ?? node.namedChildren.at(-1) ?? null - const target = literalWord(destination) - if (target && namesPath(target)) candidates.add(path.resolve(cwd, target)) +} + +// Children in order, each starting where the previous may have left off. +function scanSequence(node: Node, cwds: string[], scan: GuardScan) { + let next = cwds + for (const child of node.namedChildren) next = scanNode(child, next, scan).any + return next +} + +// Children that cannot change the directory (substitutions inside words, pipeline stages). +function scanChildren(node: Node, cwds: string[], scan: GuardScan) { + for (const child of node.namedChildren) scanNode(child, cwds, scan) +} + +// The parser attaches `a && b > f` and `a | b > f` to the whole list; the shell applies the +// redirection to the last command only, in that command's directory. +function scanRedirected(body: Node | null, redirects: Node[], cwds: string[], scan: GuardScan): Cwds { + if (body?.type === "list") { + const [left, right] = body.namedChildren + const or = body.children.some((child) => child?.type === "||") + const l = scanNode(left, cwds, scan) + const r = scanRedirected(right, redirects, or ? l.any : l.ok, scan) + return { ok: or ? union(l.ok, r.ok) : r.ok, any: union(l.any, r.any) } + } + if (body?.type === "pipeline") { + const stages = body.namedChildren.filter((child): child is Node => !!child) + for (const stage of stages.slice(0, -1)) scanNode(stage, cwds, scan) + scanRedirected(stages.at(-1) ?? null, redirects, cwds, scan) + return same(cwds) + } + for (const redirect of redirects) scanNode(redirect, cwds, scan) + return scanNode(body, cwds, scan) +} + +function addWord(scan: GuardScan, cwds: string[], word: Node | null) { + const literal = guardLiteral(word) + if (!literal) return + if ("refuse" in literal) return void scan.refusals.add(literal.refuse) + for (const value of argumentValues(literal.text)) addCandidate(scan, cwds, value) +} + +function scanNested(script: string, cwds: string[], scan: GuardScan): Cwds { + if (scan.depth >= MAX_SHELL_DEPTH) { + scan.refusals.add(`shell commands nested more than ${MAX_SHELL_DEPTH} levels deep`) + return same(cwds) + } + const root = scan.parse(script) + if (!root) return same(cwds) + scan.depth++ + try { + return scanNode(root, cwds, scan) + } finally { + scan.depth-- + } +} + +function scanCommand(node: Node, cwds: string[], scan: GuardScan): Cwds { + // Every directory a command may run in; substitutions and assignments inside it. + for (const cwd of cwds) scan.candidates.add(cwd) + scanChildren(node, cwds, scan) + const words = guardWords(node) + const literals = words.map(guardLiteral) + const text = (i: number) => { + const literal = literals[i] + return literal && "text" in literal ? literal.text : undefined + } + // Wrappers (`env FOO=1 nohup python …`) are followed, through their options, to the program they run. + const programs: number[] = [] + for (let i = 0; i < words.length && text(i) !== undefined; ) { + programs.push(i) + if (!WRAPPERS.has(path.basename(text(i)!))) break + i++ + while (i < words.length && /^(-|[A-Za-z_]\w*=|\d+[smhd]?$)/.test(text(i) ?? "")) i++ + } + const program = programs.at(-1) ?? 0 + for (const index of programs) { + const name = text(index)! + if (!name.includes("/")) continue + for (const cwd of cwds) { + const target = path.resolve(cwd, expandHome(name)) + if (!SYSTEM_BIN_DIRS.has(path.dirname(target))) scan.candidates.add(target) + } + } + const name = text(program) + const base = name === undefined ? undefined : path.basename(name) + const start = program + 1 + + if (base === "cd" || base === "pushd") { + const target = base === "pushd" && start >= words.length ? undefined : cdTarget(words.slice(program)) + if (target === undefined || target === "-" || /^[+-]\d+$/.test(target)) return same(cwds) + const fresh = [...new Set(cwds.map((cwd) => path.resolve(cwd, target)))] + if (base === "pushd") scan.stack.push(cwds) + for (const dir of fresh) scan.candidates.add(dir) + return { ok: fresh, any: union(fresh, cwds) } + } + if (base === "popd") { + const previous = scan.stack.pop() + return previous ? { ok: previous, any: union(previous, cwds) } : same(cwds) + } + const literalArgs = words.slice(start).map((_, i) => text(start + i)) + if (base && SHELLS.has(base)) { + // `bash -c 'script'` / `bash -lc 'script'`: the first operand after the options is the script. + let i = 0 + let command = false + while (i < literalArgs.length && literalArgs[i]?.startsWith("-")) { + if (/^-[A-Za-z]*c[A-Za-z]*$/.test(literalArgs[i]!)) command = true + i++ + } + const script = literalArgs[i] + if (command && script !== undefined) scanNested(script, cwds, scan) + } + if (base === "eval" && literalArgs.every((arg) => arg !== undefined)) { + // `eval` runs in this shell, so its directory changes carry on. + return scanNested(literalArgs.join(" "), cwds, scan) + } + + // The program or pattern operand of awk/sed/grep is not a path. + const skip = new Set(programs) + if (base && PATTERN_PROGRAMS.has(base)) { + let explicit = false + for (let i = start; i < words.length; i++) { + const arg = text(i) + if (arg === undefined) break + if (arg === "--") { + if (!explicit) skip.add(i + 1) + break + } + if (PATTERN_OPTIONS.includes(arg)) { + explicit = true + // `-e PROGRAM` holds a program; `-f FILE` names a file and stays checked. + if (arg !== "-f" && arg !== "--file") skip.add(i + 1) + i++ + continue + } + if (PATTERN_VALUE_OPTIONS.includes(arg)) { + i++ + continue + } + if (arg.startsWith("-")) continue + if (!explicit) skip.add(i) + break + } + } + const wrapped = programs.length > 1 || (base !== undefined && WRAPPERS.has(base)) + for (let i = 1; i < words.length; i++) { + if (skip.has(i)) continue + const literal = literals[i] + if (!literal) continue + if ("refuse" in literal) { + scan.refusals.add(literal.refuse) + continue + } + for (const value of argumentValues(literal.text)) { + // A wrapper's system-binary argument (`nohup /usr/bin/true`) is not an escape. + if (wrapped && path.isAbsolute(value) && SYSTEM_BIN_DIRS.has(path.dirname(value))) continue + addCandidate(scan, cwds, value) } - for (const child of node.namedChildren) if (child) stack.push(child) } - return [...candidates].filter((target) => !isDevice(target)) + return same(cwds) +} + +// Literal path candidates a command names, resolved against the directory each part runs in. +export function commandPathCandidates(root: Node, cwd: string, parse: (text: string) => Node | undefined) { + const scan: GuardScan = { candidates: new Set(), refusals: new Set(), stack: [], depth: 0, parse } + scanNode(root, [cwd], scan) + return { + candidates: [...scan.candidates].filter((target) => !isDevice(target)), + refusals: [...scan.refusals], + } } -// Out-of-project locations a command names, as the directory an external_directory rule matches. -async function externalCommandPaths(root: Node, cwd: string) { - const found: { path: string; dir: string }[] = [] - for (const target of commandPathCandidates(root, cwd)) { +// Out-of-project locations a command names, with whether each exists yet. +async function externalCommandPaths(candidates: string[], abort: AbortSignal) { + if (candidates.length > MAX_GUARD_CANDIDATES) + throw new Error( + `external_directory is denied: this command names more than ${MAX_GUARD_CANDIDATES} paths to check. Split it into smaller commands.`, + ) + const found: { path: string; dir: string; exists: boolean }[] = [] + for (const target of candidates) { + abort.throwIfAborted() const real = await realpathThroughParents(target) if (!real || isDevice(real)) continue // Text whose first segment does not exist (an awk or sed program such as `/x/ {print}`) // names no real location. - const first = real.split(path.sep).filter(Boolean)[0] - if (first && !(await fs.stat(path.join(path.parse(real).root, first)).catch(() => undefined))) continue + const first = firstSegmentPath(real) + if (first && !(await fs.stat(first).catch(() => undefined))) continue if (Instance.containsPath(real)) continue - const dir = (await Filesystem.isDir(real)) ? real : path.dirname(real) - found.push({ path: real, dir }) + const stat = await fs.stat(real).catch(() => undefined) + found.push({ path: real, dir: stat?.isDirectory() ? real : path.dirname(real), exists: !!stat }) } return found } @@ -524,25 +841,28 @@ export const BashTool = Tool.define("bash", async () => { const guarded = externalDirectoryDenied(ctx.permissions) const externalPaths = new Set() if (guarded) { - const named = await externalCommandPaths(tree.rootNode, cwd) - // A directory the operand check found is listed only when no named path inside it is. - const found = [ - ...Array.from(directories) - .filter((dir) => !named.some((item) => item.path === dir || item.path.startsWith(dir + path.sep))) - .map((dir) => ({ path: dir, dir })), - ...named, - ] - const denied = new Set() - for (const item of found) { + // The guard judges every path itself, each in the directory its command runs in, so the + // operand check's directories (which keep pre-`cd` directories as aliases) are replaced. + directories.clear() + const p = await parser() + const { candidates, refusals } = commandPathCandidates( + tree.rootNode, + cwd, + (text) => p.parse(text)?.rootNode ?? undefined, + ) + const denied = new Set(refusals) + for (const item of await externalCommandPaths(candidates, ctx.abort)) { externalPaths.add(item.path) // Paths here are symlink-resolved, so rules are matched with their resolved spelling too. - const rule = PermissionEvaluate.evaluateResolved( - "external_directory", - directoryGlob(item.dir), - ctx.permissions!, + // A directory that does not exist yet is also matched as itself (`ls `). + const dirs = item.exists ? [item.dir] : [item.dir, item.path] + const allowed = dirs.find( + (dir) => + PermissionEvaluate.evaluateResolved("external_directory", directoryGlob(dir), ctx.permissions!).action !== + "deny", ) - if (rule.action === "deny") denied.add(item.path) - else directories.add(item.dir) + if (allowed === undefined) denied.add(item.path) + else directories.add(allowed) } if (denied.size > 0) { throw new Error( @@ -554,11 +874,9 @@ export const BashTool = Tool.define("bash", async () => { // altimate_change end if (directories.size > 0) { - const globs = Array.from(directories).map((dir) => { - // Preserve POSIX-looking paths with /s, even on Windows - if (dir.startsWith("/")) return `${dir.replace(/[\\/]+$/, "")}/*` - return path.join(dir, "*") - }) + // altimate_change start — one directory-to-glob rule for the guard and the request + const globs = Array.from(directories).map(directoryGlob) + // altimate_change end await ctx.ask({ permission: "external_directory", patterns: globs, diff --git a/packages/opencode/test/agent/bash-external-directory-gate.test.ts b/packages/opencode/test/agent/bash-external-directory-gate.test.ts index 7999df2146..13775a7d3f 100644 --- a/packages/opencode/test/agent/bash-external-directory-gate.test.ts +++ b/packages/opencode/test/agent/bash-external-directory-gate.test.ts @@ -60,7 +60,17 @@ it.instance( ) it.instance( - "configured external_directory: allow or ask leaves the guard off", + "configured external_directory: allow leaves the guard off", + () => + Effect.gen(function* () { + const builder = yield* Agent.Service.use((svc) => svc.get("builder")) + expect(externalDirectoryDenied(builder!.permission)).toBe(false) + }), + { config: { permission: { external_directory: "allow" } } }, +) + +it.instance( + "configured external_directory: ask with a narrower deny leaves the guard off", () => Effect.gen(function* () { const builder = yield* Agent.Service.use((svc) => svc.get("builder")) diff --git a/packages/opencode/test/tool/bash-external-directory.test.ts b/packages/opencode/test/tool/bash-external-directory.test.ts index 9e94c783a0..176d74cd28 100644 --- a/packages/opencode/test/tool/bash-external-directory.test.ts +++ b/packages/opencode/test/tool/bash-external-directory.test.ts @@ -3,7 +3,8 @@ import { describe, expect, test } from "bun:test" import fs from "fs/promises" import os from "os" import path from "path" -import { BashTool } from "../../src/tool/bash" +import * as BashModule from "../../src/tool/bash" +const { BashTool } = BashModule import { Instance } from "../../src/project/instance" import { tmpdir, provideTestInstance } from "../fixture/fixture" import type { PermissionNext } from "../../src/permission/next" @@ -145,7 +146,6 @@ describe("tool.bash external_directory: deny", () => { "echo x > ./out.txt && cat out.txt", "mkdir -p nested/deeper && ls nested", "echo https://example.com/a/b", - "echo --out=/etc/hosts", "awk '/inside/ {print}' inside.txt", "sed -e 's/inside/x/' inside.txt", "echo hi 2>/dev/null", @@ -216,6 +216,169 @@ describe("tool.bash external_directory: deny", () => { }) }) +// Review cases: each form ran (or was wrongly refused) under `external_directory: deny` before. +async function v2fixture() { + const outside = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "s.txt"), "outside secret") + await fs.mkdir(path.join(dir, "dir with space")) + await Bun.write(path.join(dir, "dir with space", "s.txt"), "outside secret") + const py = path.join(dir, "py") + await Bun.write(py, `#!/bin/sh\necho ran > "${path.join(dir, "ran.txt")}"\necho host-python\n`) + await fs.chmod(py, 0o755) + }, + }) + const project = await tmpdir({ + git: true, + init: async (dir) => { + await Bun.write(path.join(dir, "inside.txt"), "inside content") + await Bun.write(path.join(dir, "run.sh"), "#!/bin/sh\necho in-project-script\n") + await fs.chmod(path.join(dir, "run.sh"), 0o755) + await fs.mkdir(path.join(dir, "a", "b"), { recursive: true }) + }, + }) + await fs.symlink(outside.path, path.join(project.path, "lnk")) + return { outside, project } +} + +// Files the refused commands would have created outside the project. +async function leaked(outside: string, project: string) { + const parent = path.dirname(project) + const base = path.basename(project) + const files = [ + path.join(outside, "w"), + path.join(outside, "ran.txt"), + path.join(parent, `${base}-marker`), + path.join(parent, `${base}-m`), + ] + const present = await Promise.all(files.map((file) => Bun.file(file).exists())) + return files.filter((_, i) => present[i]) +} + +describe("tool.bash external_directory: deny (review cases)", () => { + test.each([ + // redirection targets resolve in the directory of the command that owns them + ["redirect after cd", (o: string, p: string) => `cd ../.. && echo pwn > ../${path.basename(p)}-marker`, "a/b"], + // every word of a redirection destination is an argument at run time + ["word after a redirection", (o: string) => `cat > stolen.txt ${o}/s.txt`, undefined], + ["word after a /dev/null redirection", (o: string) => `head > /dev/null ${o}/s.txt`, undefined], + // assignment values + ["program from an assignment", (o: string) => `PY=${o}/py; $PY`, undefined], + ["PATH entry from an export", (o: string) => `export PATH=${o}:$PATH; py`, undefined], + ["file from an export", (o: string) => `export FOO=${o}/s.txt; head "$FOO"`, undefined], + ["prefix assignment", (o: string) => `OUT=${o}/w sh -c 'echo x > "$OUT"'`, undefined], + // option values + ["--opt=value", (o: string) => `sort --output=${o}/w inside.txt`, undefined], + ["attached short option", (o: string) => `sort -o${o}/w inside.txt`, undefined], + ["key=value write", (o: string) => `dd if=inside.txt of=${o}/w`, undefined], + ["key=value read", (o: string) => `dd if=${o}/s.txt`, undefined], + ["--flag=value on echo", () => `echo --out=/etc/hosts`, undefined], + // literal nested shell strings + ["bash -c", (o: string) => `bash -c 'cat ${o}/s.txt'`, undefined], + ["sh -c", (o: string) => `sh -c "cat ${o}/s.txt"`, undefined], + ["eval", (o: string) => `eval "cat ${o}/s.txt"`, undefined], + ["wrapped shell", (o: string) => `env bash -lc 'cat ${o}/s.txt'`, undefined], + // symlinks inside the project that point outside + ["read through an in-project symlink", () => `head lnk/s.txt`, undefined], + ["read through ./symlink", () => `head ./lnk/s.txt`, undefined], + ["write through an in-project symlink", () => `echo x > lnk/w`, undefined], + // pushd moves the directory like cd + ["pushd then a relative write", (o: string, p: string) => `pushd ../.. && cp inside.txt ../${path.basename(p)}-m`, "a/b"], + // words outside simple commands + ["for loop values", (o: string) => `for f in ${o}/s.txt; do head "$f"; done`, undefined], + ["[[ test ]]", (o: string) => `[[ -f ${o}/s.txt ]]`, undefined], + ["[ test ]", (o: string) => `[ -f ${o}/s.txt ]`, undefined], + ["case word", (o: string) => `case ${o}/s.txt in *) echo y ;; esac`, undefined], + // other literal spellings + ["ANSI-C quoted argument", (o: string) => `head $'${o}/s.txt'`, undefined], + ["ANSI-C quoted program", (o: string) => `$'${o}/py'`, undefined], + ["another user's home", () => `head ~root/x`, undefined], + // redirection operators and quoting + ["stderr redirection", (o: string) => `cat inside.txt 2> ${o}/w`, undefined], + ["&> redirection", (o: string) => `echo x &> ${o}/w`, undefined], + ["input redirection", (o: string) => `cat < ${o}/s.txt`, undefined], + ["read-write redirection", (o: string) => `cat <> ${o}/s.txt`, undefined], + ["quoted path with spaces", (o: string) => `cat "${o}/dir with space/s.txt"`, undefined], + ])("refuses %s", async (_name, command, workdir) => { + const { outside, project } = await v2fixture() + await using _outside = outside + await using _project = project + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const { ctx } = context(rules({ "*": "deny" })) + await expect( + bash.execute( + { + command: command(outside.path, project.path), + ...(workdir && { workdir: path.join(project.path, workdir) }), + description: "Escape attempt", + }, + ctx, + ), + ).rejects.toThrow(/external_directory is denied/) + }) + expect(await leaked(outside.path, project.path)).toEqual([]) + }) + + test("allows in-project commands that reach parents after cd, regex operands, devices and system binaries", async () => { + const { outside, project } = await v2fixture() + await using _outside = outside + await using _project = project + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const { ctx } = context(rules({ "*": "deny" })) + for (const [command, expected] of [ + ["cd a && head -c 3 ../inside.txt", "ins"], + ["cd a && sh ../run.sh", "in-project-script"], + ["(cd a && head -c 3 ../inside.txt)", "ins"], + ["cd a && ../run.sh", "in-project-script"], + ["cd a && echo ok > ../r.txt && cat ../r.txt", "ok"], + ["cd a/b && cd .. && head -c 3 ../inside.txt", "ins"], + ["pushd a > /dev/null && head -c 3 ../inside.txt", "ins"], + ["awk '/var/ {print}' inside.txt", ""], + ["sed -n '/usr/p' inside.txt", ""], + ["grep -c /etc/ inside.txt || true", "0"], + ["grep -e /usr/ inside.txt || true", ""], + ["cat /dev/null", ""], + ["cat /dev/null > f.txt && cat f.txt", ""], + ["ls /dev/null", "/dev/null"], + ["nohup /usr/bin/true 2>/dev/null", ""], + ["/usr/bin/env /usr/bin/true", ""], + ["command -v /usr/bin/true", "/usr/bin/true"], + ["bash -c 'head -c 3 inside.txt'", "ins"], + [`head -c 3 $'inside.txt'`, "ins"], + ["for f in inside.txt; do head -c 3 $f; done", "ins"], + ["git log --format=%H/%s -n 1 || true", undefined], + ["echo https://example.com/a/b", "https://example.com/a/b"], + ] as const) { + const result = await bash.execute({ command, description: "Stay inside" }, ctx) + if (expected !== undefined) expect([command, result.output.trim()]).toEqual([command, expected]) + expect([command, result.metadata.external_paths]).toEqual([command, undefined]) + } + }) + }) + + test("a not-yet-created directory that the config allows is not judged by its parent", async () => { + const { outside, project } = await v2fixture() + await using _outside = outside + await using _project = project + await provideInstance(project.path, async () => { + const bash = await initTool(BashTool) + const allowed = path.join(outside.path, "later") + const { ctx } = context(rules({ "*": "deny", [path.join(allowed, "*")]: "allow" })) + const result = await bash.execute({ command: `ls ${allowed} || true`, description: "List" }, ctx) + expect(result.metadata.external_paths).toEqual([path.join(await fs.realpath(outside.path), "later")]) + }) + }) + + test("the first segment of a Windows path is taken after the drive root", () => { + expect(BashModule.firstSegmentPath("C:\\Users\\x\\f", path.win32)).toBe("C:\\Users") + expect(BashModule.firstSegmentPath("\\\\server\\share\\dir\\f", path.win32)).toBe("\\\\server\\share\\dir") + expect(BashModule.firstSegmentPath("/etc/hosts", path.posix)).toBe("/etc") + expect(BashModule.firstSegmentPath("/", path.posix)).toBeUndefined() + }) +}) + // Without `external_directory: deny` (unset, the default "ask", or "allow") the tool behaves // exactly as before: the same permission requests, the same outputs and the same metadata keys. describe("tool.bash without external_directory: deny", () => { @@ -238,6 +401,8 @@ describe("tool.bash without external_directory: deny", () => { `${outside.path}/host/.venv/bin/python`, `cat ${sibling}/output.md`, "cat inside.txt", + "cat /dev/null", + `sort -o${outside.path}/w.txt inside.txt && cat ${outside.path}/w.txt`, ]) { const { ctx, requests } = context(ruleset) const result = await bash.execute({ command, description: "Fixed command" }, ctx) @@ -267,6 +432,13 @@ describe("tool.bash without external_directory: deny", () => { external: [["/sibling/*"]], }, { command: "cat inside.txt", output: "inside content", keys, external: [] }, + { command: "cat /dev/null", output: "", keys, external: [["/dev/*"]] }, + { + command: "sort -o/w.txt inside.txt && cat /w.txt", + output: "inside content", + keys, + external: [["/*"]], + }, ]) }) }) From 1cd6c8435f9a9853f4defaf57aaa33bb2be24e4d Mon Sep 17 00:00:00 2001 From: RaghwendraSingh Date: Thu, 5 Nov 2026 15:39:35 +0530 Subject: [PATCH 3/3] docs: describe the bash `external_directory: deny` guard and its limits - `security-faq.md`: what the deny check covers, that skill/temp directories lose their default allow under a `*` deny, and the limits (literal text only; guard rail, not a sandbox) - `permissions.md`: scope and limits next to the strict example Co-Authored-By: Claude Opus 5.5 --- docs/docs/configure/permissions.md | 4 ++++ docs/docs/reference/security-faq.md | 1 + 2 files changed, 5 insertions(+) diff --git a/docs/docs/configure/permissions.md b/docs/docs/configure/permissions.md index 65bbc9393e..b59705b9e6 100644 --- a/docs/docs/configure/permissions.md +++ b/docs/docs/configure/permissions.md @@ -204,6 +204,10 @@ When working near production systems. Blocks destructive operations entirely and } ``` +With `external_directory: "deny"`, the bash tool refuses any command whose literal text names a path outside the project (arguments and option values, programs run by path, redirections, assignment values, `cd`/`pushd` targets and literal `sh -c`/`eval` strings), resolved through symlinks and in the directory each command runs in. Directories you allow under `external_directory` keep working. A `"*": "deny"` also removes the default allow for skill directories and the CLI temp directory, so bundled skill scripts stored there are refused unless you allow those directories again. + +This is a guard rail, not a sandbox. It sees only literal command text: paths built at run time (`$HOME/x`, `$(pwd)/..`, globs) and files a program opens on its own (`python -c "open('/etc/hosts')"`, a script that writes to `/tmp`) are not checked. Use a container or VM when you need isolation. + ### Per-Agent Lockdown Give each agent only the permissions it needs: diff --git a/docs/docs/reference/security-faq.md b/docs/docs/reference/security-faq.md index ab524c7ac4..c6640b62e4 100644 --- a/docs/docs/reference/security-faq.md +++ b/docs/docs/reference/security-faq.md @@ -305,6 +305,7 @@ Altimate Code includes several layers of protection to keep the agent within you - **Path traversal blocking.** Paths containing `../` sequences that would escape the project are rejected with an "Access denied" error. - **Sensitive file protection.** Writing to credential files (`.env`, `.ssh/`, `.aws/`, private keys) triggers a confirmation prompt, even inside the project. See [below](#why-am-i-being-prompted-to-edit-env-files) for details. - **Bash command analysis.** The bash tool parses commands with tree-sitter to detect file operations (`rm`, `cp`, `mv`, etc.) targeting paths outside your project, and prompts for permission. +- **Bash path guard under `external_directory: "deny"`.** With the default (`"ask"`) or `"allow"`, the bash tool checks only the file operations above. When `external_directory` is `"deny"` for `*`, it also refuses any command whose text names a path outside the project: arguments (including `--opt=/x`, `-o/x` and `key=/x` values), a program run by path, redirection targets, assignment values (`PY=/x`, `export PATH=/x:$PATH`), `for`/`[[ ]]`/`case` words, `cd`/`pushd` targets, and literal `sh -c '…'`/`eval "…"` strings. Each path is resolved in the directory its command runs in and through symlinks, so a symlink inside the project that points outside is caught. Directories allowed in `external_directory` (and the tool-output directory) stay usable. Under a `"*": "deny"` rule, skill and reference directories and the CLI temp directory lose their default allow unless you allow them again, so bundled skill scripts that live there are refused too. This is a guard rail over literal command text, not a sandbox: paths built at run time (`$HOME/x`, `$(pwd)/..`, globs, variables set outside the command), paths a program opens itself (`python -c "open('/etc/hosts')"`, a script that writes to `/tmp`), heredoc bodies and shells nested more than three levels deep are not seen. - **Non-git project safety.** For projects outside a git repository, the boundary is strictly the working directory (not the entire filesystem). These protections operate at the application level. For additional isolation, you can run Altimate Code inside a Docker container or VM.