diff --git a/packages/opencode/src/permission/evaluate.ts b/packages/opencode/src/permission/evaluate.ts index a7fc5e1709..ce8280a936 100644 --- a/packages/opencode/src/permission/evaluate.ts +++ b/packages/opencode/src/permission/evaluate.ts @@ -1 +1,66 @@ -export { evaluate } from "." +// altimate_change start — upstream_fix: match equivalent path spellings with one rule precedence +import type { PermissionV1 } from "@opencode-ai/core/v1/permission" +import { Wildcard } from "@opencode-ai/core/util/wildcard" + +// `pattern` is the spelling the caller authored; `aliases` are equivalent normalized +// spellings. An alias only ever tightens a decision: the result is the strictest of the +// authored spelling's rule and the last rule matching it or, for deny/ask rules, an alias. +// An alias match can never relax a stricter authored match. +const STRICTNESS = { allow: 0, ask: 1, deny: 2 } as const + +function evaluatePatterns( + permission: string, + pattern: string, + aliases: string[], + ...rulesets: PermissionV1.Ruleset[] +): PermissionV1.Rule { + const rules = rulesets.flat().filter((rule) => Wildcard.match(permission, rule.permission)) + const authored = rules.findLast((rule) => Wildcard.match(pattern, rule.pattern)) ?? { + action: "ask" as const, + permission, + pattern: "*", + } + // Rule order still decides: an allow rule only applies to the authored spelling. + const aliased = rules.findLast( + (rule) => + Wildcard.match(pattern, rule.pattern) || + (rule.action !== "allow" && aliases.some((alias) => Wildcard.match(alias, rule.pattern))), + ) + return aliased && STRICTNESS[aliased.action] > STRICTNESS[authored.action] ? aliased : authored +} + +export function evaluate(permission: string, pattern: string, ...rulesets: PermissionV1.Ruleset[]): PermissionV1.Rule { + return evaluatePatterns(permission, pattern, [], ...rulesets) +} + +export function evaluateRequest( + request: Pick, + pattern: string, + ...rulesets: PermissionV1.Ruleset[] +): PermissionV1.Rule { + const aliases = request.metadata.permissionAliases + const values = aliases && typeof aliases === "object" ? (aliases as Record)[pattern] : undefined + const groups = Array.isArray(values) + ? values.filter( + (group): group is string[] => Array.isArray(group) && group.every((value) => typeof value === "string"), + ) + : [] + if (!groups.length) return evaluate(request.permission, pattern, ...rulesets) + // Preserve rule order within each access. Distinct invocations must all be allowed, + // even when they normalize to the same command. + const rules = groups.map((group) => evaluatePatterns(request.permission, pattern, group, ...rulesets)) + return rules.find((rule) => rule.action === "deny") ?? rules.find((rule) => rule.action === "ask") ?? rules[0]! +} + +export function evaluateRequestPatterns( + request: Pick, + ...rulesets: PermissionV1.Ruleset[] +) { + // Legacy callers return at the first prompt, so reject all denied accesses first. + return request.patterns + .map((pattern) => ({ pattern, rule: evaluateRequest(request, pattern, ...rulesets) })) + .sort((a, b) => Number(b.rule.action === "deny") - Number(a.rule.action === "deny")) +} +// altimate_change end + +export * as PermissionEvaluate from "./evaluate" diff --git a/packages/opencode/src/permission/index.ts b/packages/opencode/src/permission/index.ts index 2e06d7be0e..786d4f013a 100644 --- a/packages/opencode/src/permission/index.ts +++ b/packages/opencode/src/permission/index.ts @@ -7,6 +7,9 @@ import os from "os" import { PermissionV1 } from "@opencode-ai/core/v1/permission" import { EventV2Bridge } from "@/event-v2-bridge" import { EventV2 } from "@opencode-ai/core/event" +// altimate_change start — upstream_fix: evaluate normalized aliases without changing rule order +import { PermissionEvaluate } from "./evaluate" +// altimate_change end // altimate_change start — upstream_fix: load persisted permission approvals import { Database, eq } from "@/storage/db" import { PermissionTable } from "./permission.sql" @@ -40,17 +43,9 @@ interface State { approved: PermissionV1.Rule[] } -export function evaluate(permission: string, pattern: string, ...rulesets: PermissionV1.Ruleset[]): PermissionV1.Rule { - return ( - rulesets - .flat() - .findLast((rule) => Wildcard.match(permission, rule.permission) && Wildcard.match(pattern, rule.pattern)) ?? { - action: "ask", - permission, - pattern: "*", - } - ) -} +// altimate_change start — upstream_fix: share path-alias evaluation with legacy permissions +export { evaluate } from "./evaluate" +// altimate_change end export class Service extends Context.Service()("@opencode/Permission") {} @@ -91,7 +86,9 @@ export const layer = Layer.effect( let needsAsk = false for (const pattern of request.patterns) { - const rule = evaluate(request.permission, pattern, ruleset, approved) + // altimate_change start — upstream_fix: include normalized aliases in permission matching + const rule = PermissionEvaluate.evaluateRequest(request, pattern, ruleset, approved) + // altimate_change end yield* Effect.logInfo("evaluated", { permission: request.permission, pattern, action: rule }) if (rule.action === "deny") { return yield* new PermissionV1.DeniedError({ @@ -174,7 +171,9 @@ export const layer = Layer.effect( for (const [id, item] of pending.entries()) { if (item.info.sessionID !== existing.info.sessionID) continue const ok = item.info.patterns.every( - (pattern) => evaluate(item.info.permission, pattern, approved).action === "allow", + // altimate_change start — upstream_fix: apply approvals to equivalent path spellings + (pattern) => PermissionEvaluate.evaluateRequest(item.info, pattern, approved).action === "allow", + // altimate_change end ) if (!ok) continue pending.delete(id) diff --git a/packages/opencode/src/permission/next.ts b/packages/opencode/src/permission/next.ts index edc2ca1d03..d28e5edb91 100644 --- a/packages/opencode/src/permission/next.ts +++ b/packages/opencode/src/permission/next.ts @@ -13,6 +13,9 @@ import { ProjectID } from "@/project/schema" import { Wildcard } from "@/util/wildcard" import os from "os" import z from "zod" +// altimate_change start — upstream_fix: share normalized path matching with Effect permissions +import { PermissionEvaluate } from "./evaluate" +// altimate_change end export namespace PermissionNext { const log = Log.create({ service: "permission" }) @@ -135,8 +138,9 @@ export namespace PermissionNext { async (input) => { const s = await state() const { ruleset, ...request } = input - for (const pattern of request.patterns ?? []) { - const rule = evaluate(request.permission, pattern, ruleset, s.approved) + // altimate_change start — upstream_fix: check normalized denies before any approval prompt + for (const { pattern, rule } of PermissionEvaluate.evaluateRequestPatterns(request, ruleset, s.approved)) { + // altimate_change end log.info("evaluated", { permission: request.permission, pattern, action: rule }) if (rule.action === "deny") { Telemetry.track({ @@ -229,7 +233,9 @@ export namespace PermissionNext { for (const [id, pending] of s.pending) { if (pending.info.sessionID !== sessionID) continue const ok = pending.info.patterns.every( - (pattern) => evaluate(pending.info.permission, pattern, s.approved).action === "allow", + // altimate_change start — upstream_fix: apply approvals to equivalent path spellings + (pattern) => PermissionEvaluate.evaluateRequest(pending.info, pattern, s.approved).action === "allow", + // altimate_change end ) if (!ok) continue s.pending.delete(id) diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 52bea98df4..0413e2f2c5 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -8,6 +8,7 @@ import { Instance } from "../project/instance" import { lazy } from "@/util/lazy" import { Language } from "web-tree-sitter" import fs from "fs/promises" +import { homedir } from "os" import { Filesystem } from "@/util/filesystem" import { fileURLToPath } from "url" @@ -18,6 +19,152 @@ import { BashArity } from "@/permission/arity" import { Truncate } from "./truncation" 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" + +function shellWords(node: Node, includeNumbers = false) { + const types = ["command_name", "word", "string", "raw_string", "concatenation"] + // tree-sitter parses a bare numeric argument (`mkdir -m 700`) as `number`. + if (includeNumbers) types.push("number") + return node.namedChildren.filter((child): child is Node => child !== null && types.includes(child.type)) +} + +function literalWord(node: Node | null): string | undefined { + if (!node) return + if (node.type === "raw_string") return node.text.slice(1, -1) + if (node.type === "word" || node.type === "number") { + let text = node.text + if (text === "~" || text.startsWith("~/")) text = homedir() + text.slice(1) + // Backslash-escaped metacharacters are literal; only unescaped ones mean expansion. + if (/[~$*?{\[]/.test(text.replace(/\\./gs, ""))) return + return text.replace(/\\(.)/gs, "$1") + } + if (node.type === "string" && node.namedChildren.every((child) => child?.type === "string_content")) { + return node.text + .slice(1, -1) + .replace(/\\\n/g, "") + .replace(/\\([$`"\\])/g, "$1") + } + if (node.type === "command_name" || node.type === "concatenation") { + const parts = node.namedChildren.map(literalWord) + if (parts.every((part) => part !== undefined)) return parts.join("") + } +} + +// Cap on tracked directories so pathological scripts cannot blow up alias generation. +const MAX_TRACKED_DIRECTORIES = 32 +// Operand-spelling combinations are enumerated in full up to this many operands. +const MAX_MIXED_OPERANDS = 4 +const MAX_ALIAS_VARIANTS = 4096 + +// Options that take a separate non-path value; the value must not be read as an operand. +const VALUE_OPTIONS: Record = { + mkdir: ["-m", "--mode"], + cp: ["-S", "--suffix"], + mv: ["-S", "--suffix"], + touch: ["-d", "--date", "-t"], + ls: ["-I", "--ignore", "-w", "--width", "-T", "--tabsize"], +} +// Long options whose `=value` is a path operand. +const PATH_OPTIONS = ["--reference", "--target-directory"] +const CD_FLAGS = ["-L", "-P", "-e", "-@"] + +// cwds that a following command may run in after `cd`. `cd` can fail (`cd missing; cat x`) +// 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[] { + 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() + 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. + const fresh = cwds.map((cwd) => path.resolve(cwd, target)) + return [...new Set([...fresh, ...cwds])].slice(0, MAX_TRACKED_DIRECTORIES) +} + +// Walk commands in source order, tracking every directory a command could run in. +// Subshells and substitutions keep their directory changes local. +function* shellCommands(node: Node | null, cwds: string[]): Generator<{ node: Node; cwds: string[] }, string[]> { + if (!node) return cwds + if (node.type === "command") { + yield { node, cwds } + for (const child of node.namedChildren) yield* shellCommands(child, cwds) + const words = shellWords(node, true) + return words[0] && literalWord(words[0]) === "cd" ? afterCd(cwds, words) : cwds + } + const local = ["subshell", "command_substitution", "pipeline"].includes(node.type) + let next = cwds + for (const child of node.namedChildren) next = yield* shellCommands(child, next) + return local ? cwds : next +} + +interface PathOperand { + word: Node + arg: string | undefined + prefix: string +} + +// Collect path operands, honoring `--`, quoted flags, option values and `--reference=`. +function pathOperands(name: string, words: Node[]): PathOperand[] { + const operands: PathOperand[] = [] + const valueOptions = VALUE_OPTIONS[name] ?? [] + let terminated = false + let skipNext = false + let referenced = false + for (const word of words.slice(1)) { + const arg = literalWord(word) + if (skipNext) { + skipNext = false + continue + } + // chmod symbolic modes such as `-w` or `-rwx` are the mode operand, not options. + const symbolicMode = name === "chmod" && arg !== undefined && /^-[ugoa]*[rwxXst]+[ugoa,rwxXst=+-]*$/.test(arg) + if (!terminated && !symbolicMode && arg?.startsWith("-")) { + if (arg === "--") terminated = true + else { + const option = PATH_OPTIONS.find((candidate) => arg.startsWith(candidate + "=")) + if (option) { + if (option === "--reference") referenced = true + operands.push({ word, arg: arg.slice(option.length + 1), prefix: option + "=" }) + } else if (valueOptions.includes(arg)) skipNext = true + } + continue + } + operands.push({ word, arg, prefix: "" }) + } + // chmod/chown take a mode/owner first unless --reference supplies it. + if (["chmod", "chown"].includes(name) && !referenced) { + const first = operands.findIndex((operand) => !operand.prefix) + if (first >= 0) operands.splice(first, 1) + } + return operands +} + +// Resolve symlinks in the deepest existing parent so a not-yet-created destination +// behind a symlinked directory still maps to where it will really be written. +async function realpathThroughParents(target: string): Promise { + const missing: string[] = [] + let current = target + while (true) { + const resolved = await fs.realpath(current).catch(() => "") + if (resolved) return path.join(resolved, ...missing.reverse()) + const parent = path.dirname(current) + if (parent === current) return "" + missing.push(path.basename(current)) + current = parent + } +} + +function quoteLike(word: Node, text: string) { + if (word.type === "string") return `"${text}"` + if (word.type === "raw_string") return `'${text}'` + return text +} +// 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 @@ -64,7 +211,10 @@ const HOST_MARKERS = new Set([ "ALTIMATE_PINNED_WORKSPACE_NAME", "ALTIMATE_PINNED_WORKSPACE_ROOT", ]) -export function stripHostMarkers(env: Record, platform: NodeJS.Platform = process.platform) { +export function stripHostMarkers( + env: Record, + platform: NodeJS.Platform = process.platform, +) { // Windows environment names are case-insensitive: a child reads `altimate_code_serve` // as the marker, so every spelling goes there. POSIX keeps exact matching. for (const key of Object.keys(env)) { @@ -135,7 +285,9 @@ export const BashTool = Tool.define("bash", async () => { ), }), async execute(params, ctx) { - const cwd = params.workdir || Instance.directory + // altimate_change start — upstream_fix: resolve workdir relative to the active instance + const cwd = path.resolve(Instance.directory, params.workdir || ".") + // altimate_change end if (params.timeout !== undefined && params.timeout < 0) { throw new Error(`Invalid timeout value: ${params.timeout}. Timeout must be a positive number.`) } @@ -148,36 +300,38 @@ export const BashTool = Tool.define("bash", async () => { if (!Instance.containsPath(cwd)) directories.add(cwd) const patterns = new Set() const always = new Set() + // altimate_change start — upstream_fix: retain raw command rules alongside absolute path rules + const permissionAliases = new Map>() - for (const node of tree.rootNode.descendantsOfType("command")) { - if (!node) continue - + for (const { node, cwds } of shellCommands(tree.rootNode, [cwd])) { + // altimate_change end // Get full command text including redirects if present - let commandText = node.parent?.type === "redirected_statement" ? node.parent.text : node.text - - const command = [] - for (let i = 0; i < node.childCount; i++) { - const child = node.child(i) - if (!child) continue - if ( - child.type !== "command_name" && - child.type !== "word" && - child.type !== "string" && - child.type !== "raw_string" && - child.type !== "concatenation" - ) { - continue - } - command.push(child.text) - } + const original = node.parent?.type === "redirected_statement" ? node.parent.text : node.text + + // altimate_change start — upstream_fix: normalize static path operands, including directory listings + const words = shellWords(node) + const operandWords = shellWords(node, true) + const command = words.map((word) => word.text) + const name = words[0] && literalWord(words[0]) + const source = node.parent?.type === "redirected_statement" ? node.parent : node + const variants = new Set([original]) // not an exhaustive list, but covers most common cases - if (["cd", "rm", "cp", "mv", "mkdir", "touch", "chmod", "chown", "cat"].includes(command[0])) { - for (const arg of command.slice(1)) { - if (arg.startsWith("-") || (command[0] === "chmod" && arg.startsWith("+"))) continue - const resolved = await fs.realpath(path.resolve(cwd, arg)).catch(() => "") - log.info("resolved path", { arg, resolved }) - if (resolved) { + if (name && ["cd", "rm", "cp", "mv", "mkdir", "touch", "chmod", "chown", "cat", "ls"].includes(name)) { + const operands = name === "cd" ? pathOperands("cat", operandWords) : pathOperands(name, operandWords) + for (const commandCwd of cwds) { + const targets = operands.flatMap((operand) => { + if (operand.arg === undefined || (name === "chmod" && operand.arg.startsWith("+"))) return [] + return [{ operand, target: path.resolve(commandCwd, operand.arg) }] + }) + const listing = name === "ls" && operands.length === 0 + if (listing) targets.push({ operand: undefined as never, target: commandCwd }) + const reals = await Promise.all(targets.map((item) => realpathThroughParents(item.target))) + reals.forEach((resolved, index) => { + log.info("resolved path", { arg: targets[index].target, resolved }) + }) + for (const resolved of reals) { + if (!resolved) continue const normalized = process.platform === "win32" ? Filesystem.windowsPath(resolved).replace(/\//g, "\\") : resolved if (!Instance.containsPath(normalized)) { @@ -185,12 +339,80 @@ export const BashTool = Tool.define("bash", async () => { directories.add(dir) } } + // Each operand can be written as authored, absolute (keeping its quoting), bare + // absolute or symlink-resolved, and the rule may mix them (`cp a /abs/b`). + const written = targets.flatMap((item, index) => (item.operand ? [{ item, real: reals[index] }] : [])) + const choices = written.map(({ item, real }) => + item.operand + ? [ + item.operand.word.text, + item.operand.prefix + quoteLike(item.operand.word, item.target), + item.operand.prefix + item.target, + item.operand.prefix + (real || item.target), + ].filter((value, i, all) => all.indexOf(value) === i) + : [], + ) + const combos: number[][] = [] + if (written.length <= MAX_MIXED_OPERANDS) { + const expand = (prefix: number[]) => { + if (prefix.length === choices.length) return void combos.push(prefix) + choices[prefix.length].forEach((_, i) => expand([...prefix, i])) + } + expand([]) + } else { + // Too many operands to enumerate: uniform spellings plus each operand spelled + // differently from the rest. + for (let i = 0; i < 4; i++) combos.push(choices.map((options) => Math.min(i, options.length - 1))) + choices.forEach((options, index) => { + options.forEach((_, i) => combos.push(choices.map((_, j) => (j === index ? i : 0)))) + }) + } + // `cat -- x` and `cat x` read the same file, so also alias the form without `--`. + const terminator = + name === "cd" ? undefined : operandWords.slice(1).find((word) => literalWord(word) === "--") + for (const combo of combos) { + for (const dropTerminator of terminator ? [false, true] : [false]) { + if (variants.size >= MAX_ALIAS_VARIANTS) break + const replacements = written + .map(({ item }, index) => ({ + start: item.operand?.word.startIndex ?? 0, + end: item.operand?.word.endIndex ?? 0, + text: choices[index][combo[index]], + skip: !item.operand, + })) + .filter((replacement) => !replacement.skip) + if (listing) + replacements.push({ start: node.endIndex, end: node.endIndex, text: ` ${commandCwd}`, skip: false }) + if (terminator && dropTerminator) + replacements.push({ + start: terminator.startIndex - 1, + end: terminator.endIndex, + text: "", + skip: false, + }) + let text = original + for (const replacement of replacements.sort((a, b) => b.start - a.start)) { + const start = replacement.start - source.startIndex + const end = replacement.end - source.startIndex + text = text.slice(0, start) + replacement.text + text.slice(end) + } + variants.add(text) + } + } } } + // Insignificant shell whitespace must not let an exact rule miss the command. + for (const variant of [...variants]) if (!/["'\\]/.test(variant)) variants.add(variant.replace(/[ \t]+/g, " ")) + const aliases = permissionAliases.get(original) ?? new Set() + for (const variant of variants) aliases.add(variant) + permissionAliases.set(original, aliases) + // altimate_change end // cd covered by above check if (command.length && command[0] !== "cd") { - patterns.add(commandText) + // altimate_change start — upstream_fix: rules match the authored command text + patterns.add(original) + // altimate_change end always.add(BashArity.prefix(command).join(" ") + " *") } } @@ -214,7 +436,13 @@ export const BashTool = Tool.define("bash", async () => { permission: "bash", patterns: Array.from(patterns), always: Array.from(always), - metadata: {}, + // altimate_change start — upstream_fix: evaluate raw and normalized commands as equivalent spellings + metadata: { + permissionAliases: Object.fromEntries( + Array.from(permissionAliases, ([key, values]) => [key, [...values].map((value) => [value])]), + ), + }, + // altimate_change end }) } diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index b17b6925b8..d55c548c12 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -12,6 +12,7 @@ import { isPdfAttachment, sniffAttachmentMime } from "@/util/media" // altimate_change start — upstream_fix: restore stale-file guard (dropped in v1.17.9 merge) import { FileTime } from "../file/time" import { Instance } from "../project/instance" +import { realpath } from "fs/promises" // altimate_change end const DEFAULT_READ_LIMIT = 2000 @@ -235,10 +236,9 @@ export const ReadTool = Tool.define< ctx: Tool.Context, ) { const instance = yield* InstanceState.context - let filepath = params.filePath - if (!path.isAbsolute(filepath)) { - filepath = path.resolve(instance.directory, filepath) - } + // altimate_change start — upstream_fix: normalize read paths before permission evaluation + let filepath = path.resolve(instance.directory, params.filePath) + // altimate_change end if (process.platform === "win32") { filepath = FSUtil.normalizePath(filepath) } @@ -256,11 +256,19 @@ export const ReadTool = Tool.define< kind: stat?.type === "Directory" ? "directory" : "file", }) + // altimate_change start — upstream_fix: match absolute and symlink-resolved deny rules; allow rules keep the authored spelling + const resolved = yield* Effect.promise(() => realpath(filepath).catch(() => filepath)) + const relative = path.relative(instance.worktree, filepath) yield* ctx.ask({ permission: "read", - patterns: [path.relative(instance.worktree, filepath)], + patterns: [relative], always: ["*"], - metadata: {}, + metadata: { + permissionAliases: { + [relative]: [Array.from(new Set([filepath, resolved, path.relative(instance.worktree, resolved)]))], + }, + }, + // altimate_change end }) if (!stat) return yield* miss(filepath) diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index b046399909..1a90ae9383 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -12,6 +12,9 @@ import { InstanceStore } from "../../src/project/instance-store" import { requireInstance, TestInstance, tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" import { MessageID, SessionID } from "../../src/session/schema" +import { PermissionNext as LegacyPermission } from "../../src/permission/next" +import { Instance } from "../../src/project/instance" +import { PermissionEvaluate } from "../../src/permission/evaluate" const events = EventV2Bridge.defaultLayer const noopBootstrap = Layer.succeed(InstanceBootstrap.Service, InstanceBootstrap.Service.of({ run: Effect.void })) @@ -558,6 +561,172 @@ test("disabled - specific allow overrides wildcard deny", () => { // ask tests +it.instance( + "ask - one allowed spelling cannot mask another denied command invocation", + () => + Effect.gen(function* () { + const err = yield* fail( + ask({ + sessionID: SessionID.make("session_test"), + permission: "bash", + patterns: ["cat /project/note.md"], + metadata: { permissionAliases: { "cat /project/note.md": [["cat note.md"], ["cat ./note.md"]] } }, + always: [], + ruleset: Permission.fromConfig({ + bash: { "*": "allow", "cat note.md": "deny", "cat ./note.md": "allow" }, + }), + }), + ) + expect(err).toBeInstanceOf(PermissionV1.DeniedError) + }), + { git: true }, +) + +it.instance( + "legacy ask - checks normalized denies before prompting for an earlier command", + () => + Effect.gen(function* () { + const instance = yield* requireInstance + const result = yield* Effect.promise(() => + Instance.restore(instance, async () => { + const request = LegacyPermission.ask({ + sessionID: SessionID.make("ses_alias_legacy"), + permission: "bash", + patterns: ["ls", "ls /project"], + metadata: { permissionAliases: { "ls /project": [["ls .."]] } }, + always: [], + ruleset: [{ permission: "bash", pattern: "ls /project", action: "deny" }], + }).then(() => "allowed", (err: unknown) => err) + const result = await Promise.race([request, new Promise((resolve) => setTimeout(() => resolve("pending"), 100))]) + for (const item of await LegacyPermission.list()) { + await LegacyPermission.reply({ requestID: item.id, reply: "reject" }) + } + await request + return result + }), + ) + expect(result).toBeInstanceOf(LegacyPermission.DeniedError) + }), + { git: true }, +) + +it.instance( + "ask - denies a relative path through its absolute alias", + () => + Effect.gen(function* () { + const err = yield* fail( + ask({ + sessionID: SessionID.make("session_test"), + permission: "read", + patterns: ["../sibling/output.md"], + metadata: { permissionAliases: { "../sibling/output.md": [["/project/sibling/output.md"]] } }, + always: [], + ruleset: Permission.fromConfig({ read: { "*": "allow", "/project/sibling/*": "deny" } }), + }), + ) + expect(err).toBeInstanceOf(PermissionV1.DeniedError) + }), + { git: true }, +) + +it.instance( + "ask - preserves a restrictive relative allowlist with absolute aliases", + () => + Effect.gen(function* () { + yield* ask({ + sessionID: SessionID.make("session_test"), + permission: "read", + patterns: ["src/index.ts"], + metadata: { permissionAliases: { "src/index.ts": [["/project/src/index.ts"]] } }, + always: [], + ruleset: Permission.fromConfig({ read: { "*": "deny", "src/*": "allow" } }), + }) + expect(yield* list()).toHaveLength(0) + }), + { git: true }, +) + +it.instance( + "ask - an absolute allow does not override a relative deny for a relative request", + () => + Effect.gen(function* () { + const err = yield* fail( + ask({ + sessionID: SessionID.make("session_test"), + permission: "read", + patterns: ["src/index.ts"], + metadata: { permissionAliases: { "src/index.ts": [["/project/src/index.ts"]] } }, + always: [], + ruleset: Permission.fromConfig({ read: { "src/*": "deny", "/project/src/*": "allow" } }), + }), + ) + expect(err).toBeInstanceOf(PermissionV1.DeniedError) + }), + { git: true }, +) + +it.instance( + "ask - allow rules never match alias spellings", + () => + Effect.gen(function* () { + const fiber = yield* ask({ + id: PermissionV1.ID.make("per_alias_allow_only"), + sessionID: SessionID.make("session_test"), + permission: "read", + patterns: ["private.txt"], + metadata: { permissionAliases: { "private.txt": [["/repo/public/project/private.txt"]] } }, + always: [], + ruleset: Permission.fromConfig({ read: { "*": "deny", "*/public/*": "allow" } }), + }).pipe(Effect.forkScoped) + const err = yield* Fiber.await(fiber) + expect(String(err)).toContain("DeniedError") + }), + { git: true }, +) + +it.instance( + "ask - later relative deny overrides an earlier absolute allow", + () => + Effect.gen(function* () { + const err = yield* fail( + ask({ + sessionID: SessionID.make("session_test"), + permission: "read", + patterns: ["src/index.ts"], + metadata: { permissionAliases: { "src/index.ts": [["/project/src/index.ts"]] } }, + always: [], + ruleset: Permission.fromConfig({ read: { "/project/src/*": "allow", "src/*": "deny" } }), + }), + ) + expect(err).toBeInstanceOf(PermissionV1.DeniedError) + }), + { git: true }, +) + +it.instance( + "ask - keeps aliases scoped to their own command", + () => + Effect.gen(function* () { + const err = yield* fail( + ask({ + sessionID: SessionID.make("session_test"), + permission: "bash", + patterns: ["cat public.md", "cat private.md"], + metadata: { + permissionAliases: { + "cat public.md": [["cat /project/public.md"]], + "cat private.md": [["cat /project/private.md"]], + }, + }, + always: [], + ruleset: Permission.fromConfig({ bash: { "*": "deny", "cat /project/public.md": "allow" } }), + }), + ) + expect(err).toBeInstanceOf(PermissionV1.DeniedError) + }), + { git: true }, +) + it.instance( "ask - resolves immediately when action is allow", () => @@ -727,6 +896,39 @@ it.instance( // reply tests +it.instance( + "reply - always approval does not resolve pending requests that only match through an alias", + () => + Effect.gen(function* () { + const first = yield* ask({ + id: PermissionV1.ID.make("per_alias_first"), + sessionID: SessionID.make("session_alias"), + permission: "bash", + patterns: ["cat ./notes.md"], + metadata: { permissionAliases: { "cat ./notes.md": [["cat /project/notes.md"]] } }, + always: ["cat ./notes.md"], + ruleset: [], + }).pipe(Effect.forkScoped) + const second = yield* ask({ + id: PermissionV1.ID.make("per_alias_second"), + sessionID: SessionID.make("session_alias"), + permission: "bash", + patterns: ["cat notes.md"], + metadata: { permissionAliases: { "cat notes.md": [["cat /project/notes.md"]] } }, + always: [], + ruleset: [], + }).pipe(Effect.forkScoped) + + yield* waitForPending(2) + yield* reply({ requestID: PermissionV1.ID.make("per_alias_first"), reply: "always" }) + yield* Fiber.join(first) + expect(yield* list()).toHaveLength(1) + yield* reply({ requestID: PermissionV1.ID.make("per_alias_second"), reply: "reject" }) + yield* Fiber.await(second) + }), + { git: true }, +) + it.instance( "reply - once resolves the pending ask", () => @@ -1203,3 +1405,12 @@ it.instance( }), { git: true }, ) + +test("evaluateRequest - an ask rule matching only an alias never relaxes an authored deny", () => { + const request = { permission: "read", metadata: { permissionAliases: { "a.txt": [["/abs/a.txt"]] } } } + const rule = PermissionEvaluate.evaluateRequest(request, "a.txt", [ + { permission: "read", pattern: "a.txt", action: "deny" }, + { permission: "read", pattern: "/abs/*", action: "ask" }, + ]) + expect(rule.action).toBe("deny") +}) diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index 6034d4125b..4a3a5d33ad 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -14,9 +14,11 @@ import { Effect, Layer, ManagedRuntime } from "effect" import { Agent } from "../../src/agent/agent" import { Truncate as TruncateService } from "../../src/tool/truncate" import { InstanceRef } from "../../src/effect/instance-ref" +import { PermissionEvaluate } from "../../src/permission/evaluate" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { testEffect } from "../lib/effect" +const evaluateRequest = PermissionEvaluate.evaluateRequest const it = testEffect(CrossSpawnSpawner.defaultLayer) // The shared tool fixture stubs Truncate with a no-op so most tool tests don't @@ -128,116 +130,410 @@ describe("tool.bash", () => { test("basic", async () => { await provideInstance(projectRoot, async () => { - const bash = await initTool(BashTool) - const result = await bash.execute( - { - command: "echo 'test'", - description: "Echo test message", - }, - ctx, - ) - expect(result.metadata.exit).toBe(0) - expect(result.metadata.output).toContain("test") - }) + const bash = await initTool(BashTool) + const result = await bash.execute( + { + command: "echo 'test'", + description: "Echo test message", + }, + ctx, + ) + expect(result.metadata.exit).toBe(0) + expect(result.metadata.output).toContain("test") + }) }) }) describe("tool.bash permissions", () => { - test("asks for bash permission with correct pattern", async () => { + test.each([ + ["cat ../sibling/output.md", "cat", "sibling/output.md"], + ['cat "../sibling/output.md"', "cat", "sibling/output.md"], + ["cd ../sibling && cat output.md", "cat", "sibling/output.md"], + ["cd ../sibling; cat output.md", "cat", "sibling/output.md"], + ["ls ..", "ls", ""], + ["ls; ls ..", "ls", ""], + ["cd .. && ls", "ls", ""], + ["cd ../sibling || exit 1; cat output.md", "cat", "sibling/output.md"], + ["cd -- ../sibling && cat output.md", "cat", "sibling/output.md"], + ["cd -P ../sibling && cat output.md", "cat", "sibling/output.md"], + ["cd ../sibling && cat output.md &", "cat", "sibling/output.md"], + ["cat -- ../sibling/output.md", "cat", "sibling/output.md"], + ["chmod -w ../sibling/output.md", "chmod -w", "sibling/output.md"], + ["cd ../sibling; cd a; cd b; cd c; cd d; cd e; cd f; cat output.md", "cat", "sibling/a/b/c/d/e/f/output.md"], + ["cd missing; cd ../sibling; cat output.md", "cat", "sibling/output.md"], + ])("denies normalized paths in %s", async (command, executable, target) => { + await using tmp = await tmpdir({ git: true }) + await fs.mkdir(path.join(tmp.path, "workspace")) + await fs.mkdir(path.join(tmp.path, "sibling")) + await Bun.write(path.join(tmp.path, "sibling", "output.md"), "private content") + await provideInstance(path.join(tmp.path, "workspace"), async () => { + const bash = await initTool(BashTool) + const absolute = `${executable} ${path.join(tmp.path, target)}` + const ruleset: PermissionNext.Ruleset = [ + { permission: "*", pattern: "*", action: "allow" }, + { permission: "bash", pattern: absolute, action: "deny" }, + ] + const testCtx = { + ...ctx, + ask: async (request: Omit) => { + for (const pattern of request.patterns) { + if (evaluateRequest(request, pattern, ruleset).action === "deny") throw new Error("Path denied") + } + }, + } + for (const spelling of [absolute, command]) { + await expect(bash.execute({ command: spelling, description: "Inspect path" }, testCtx)).rejects.toThrow( + "Path denied", + ) + } + }) + }) + + test.each([ + ["cd nested & cat local.txt", "cat nested/local.txt"], + ["cat 'one two'", "cat one two"], + ])("does not let an allow rule for a normalized spelling cover %s", async (command, allowedSpelling) => { await using tmp = await tmpdir({ git: true }) + await fs.mkdir(path.join(tmp.path, "nested")) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const requests: Array> = [] + const bash = await initTool(BashTool) + const absolute = (value: string) => value.replace(/(\S+)$/, (_, last) => path.join(tmp.path, last)) + const ruleset: PermissionNext.Ruleset = [ + { permission: "*", pattern: "*", action: "deny" }, + { permission: "bash", pattern: absolute(allowedSpelling), action: "allow" }, + { permission: "bash", pattern: "cd *", action: "allow" }, + ] + const testCtx = { + ...ctx, + ask: async (request: Omit) => { + for (const pattern of request.patterns) { + if (evaluateRequest(request, pattern, ruleset).action !== "allow") throw new Error("Path denied") + } + }, + } + await expect(bash.execute({ command, description: "Read file" }, testCtx)).rejects.toThrow("Path denied") + }) + }) + + test("denies the operand of a command when an option consumes a separate value", async () => { + await using tmp = await tmpdir({ git: true }) + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + const exact = `mkdir -m 700 ${path.join(tmp.path, "private")}` + const ruleset: PermissionNext.Ruleset = [ + { permission: "*", pattern: "*", action: "allow" }, + { permission: "bash", pattern: exact, action: "deny" }, + ] + const testCtx = { + ...ctx, + ask: async (request: Omit) => { + for (const pattern of request.patterns) { + if (evaluateRequest(request, pattern, ruleset).action === "deny") throw new Error("Path denied") + } + }, + } + await expect(bash.execute({ command: "mkdir -m 700 private", description: "Make" }, testCtx)).rejects.toThrow( + "Path denied", + ) + }) + }) + + test("denies mixed authored and absolute operand spellings and numeric cd targets", async () => { + await using tmp = await tmpdir({ git: true }) + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + const deny = async (rule: string, command: string) => { + const ruleset: PermissionNext.Ruleset = [ + { permission: "*", pattern: "*", action: "allow" }, + { permission: "bash", pattern: rule, action: "deny" }, + ] const testCtx = { ...ctx, - ask: async (req: Omit) => { - requests.push(req) + ask: async (request: Omit) => { + for (const pattern of request.patterns) { + if (evaluateRequest(request, pattern, ruleset).action === "deny") throw new Error("Path denied") + } }, } - await bash.execute( - { - command: "echo hello", - description: "Echo hello", - }, - testCtx, - ) - expect(requests.length).toBe(1) - expect(requests[0].permission).toBe("bash") - expect(requests[0].patterns).toContain("echo hello") - }) + await expect(bash.execute({ command, description: "Probe" }, testCtx)).rejects.toThrow("Path denied") + } + await deny(`cp public.txt ${path.join(tmp.path, "private.txt")}`, "cp public.txt private.txt") + await deny(`cat ${path.join(tmp.path, "123", "secret")}`, "cd 123 && cat secret") + await deny(`cp a b c ${path.join(tmp.path, "dest")}`, "cp a b c dest") + await deny(`cp a ${path.join(tmp.path, "b")} c d e`, "cp a b c d e") + await deny(`cat ${path.join(tmp.path, "secret")}`, "cat secret") + await deny(`cat ${path.join(tmp.path, "secret$")}`, "cat secret\\$") + await fs.symlink(os.tmpdir(), path.join(tmp.path, "elsewhere")) + await deny(`cp * ${path.join(await fs.realpath(os.tmpdir()), "fresh-target")}`, "cp local elsewhere/fresh-target") + }) }) - test("asks for bash permission with multiple commands", async () => { + test("denies a symlinked operand by its resolved destination", async () => { await using tmp = await tmpdir({ git: true }) + await fs.mkdir(path.join(tmp.path, "real")) + await Bun.write(path.join(tmp.path, "real", "key"), "secret") + await fs.symlink(path.join(tmp.path, "real"), path.join(tmp.path, "link")) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) + const bash = await initTool(BashTool) + const ruleset: PermissionNext.Ruleset = [ + { permission: "*", pattern: "*", action: "allow" }, + { permission: "bash", pattern: `cat ${await fs.realpath(path.join(tmp.path, "real"))}/key`, action: "deny" }, + ] + const testCtx = { + ...ctx, + ask: async (request: Omit) => { + for (const pattern of request.patterns) { + if (evaluateRequest(request, pattern, ruleset).action === "deny") throw new Error("Path denied") + } + }, + } + await expect(bash.execute({ command: "cat link/key", description: "Read" }, testCtx)).rejects.toThrow( + "Path denied", + ) + }) + }) + + test("keeps directory changes inside subshells and substitutions", async () => { + await using tmp = await tmpdir({ git: true }) + await fs.mkdir(path.join(tmp.path, "nested")) + await Bun.write(path.join(tmp.path, "local.txt"), "local content") + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + for (const command of ["(cd nested); cat local.txt", 'echo "$(cd nested)"; cat local.txt']) { const requests: Array> = [] - const testCtx = { - ...ctx, - ask: async (req: Omit) => { - requests.push(req) - }, - } await bash.execute( + { command, description: "Read local file" }, { - command: "echo foo && echo bar", - description: "Echo twice", + ...ctx, + ask: async (req: (typeof requests)[number]) => { + requests.push(req) + }, }, - testCtx, ) - expect(requests.length).toBe(1) - expect(requests[0].permission).toBe("bash") - expect(requests[0].patterns).toContain("echo foo") - expect(requests[0].patterns).toContain("echo bar") - }) + const request = requests.find((req) => req.permission === "bash")! + const ruleset: PermissionNext.Ruleset = [ + { permission: "bash", pattern: "*", action: "allow" }, + { permission: "bash", pattern: `cat ${path.join(tmp.path, "local.txt")}`, action: "deny" }, + ] + expect(request.patterns.some((pattern) => evaluateRequest(request, pattern, ruleset).action === "deny")).toBe( + true, + ) + } + }) }) - test("asks for external_directory permission when cd to parent", async () => { + test("retains exact relative command allow rules", async () => { await using tmp = await tmpdir({ git: true }) + await Bun.write(path.join(tmp.path, "local.txt"), "local content") await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const requests: Array> = [] - const testCtx = { + const bash = await initTool(BashTool) + const ruleset: PermissionNext.Ruleset = [ + { permission: "bash", pattern: "*", action: "deny" }, + { permission: "bash", pattern: "cat local.txt", action: "allow" }, + ] + const result = await bash.execute( + { command: "cat local.txt", description: "Read local file" }, + { ...ctx, - ask: async (req: Omit) => { - requests.push(req) + ask: async (request: Omit) => { + for (const pattern of request.patterns) + expect(evaluateRequest(request, pattern, ruleset).action).toBe("allow") }, - } - await bash.execute( + }, + ) + expect(result.output).toContain("local content") + }) + }) + + test("keeps repeated commands in different directories as separate accesses", async () => { + await using tmp = await tmpdir({ git: true }) + await fs.mkdir(path.join(tmp.path, "nested")) + await Bun.write(path.join(tmp.path, "local.txt"), "private content") + await Bun.write(path.join(tmp.path, "nested", "local.txt"), "public content") + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + const ruleset: PermissionNext.Ruleset = [ + { permission: "bash", pattern: "*", action: "allow" }, + { permission: "bash", pattern: `cat ${path.join(tmp.path, "local.txt")}`, action: "deny" }, + { permission: "bash", pattern: `cat ${path.join(tmp.path, "nested", "local.txt")}`, action: "allow" }, + ] + await expect( + bash.execute( { - command: "cd ../", - description: "Change to parent directory", + command: "cat local.txt; cd nested && cat local.txt", + description: "Read files in two directories", }, - testCtx, - ) - const extDirReq = requests.find((r) => r.permission === "external_directory") - expect(extDirReq).toBeDefined() - }) + { + ...ctx, + ask: async (request: Omit) => { + for (const pattern of request.patterns) { + if (evaluateRequest(request, pattern, ruleset).action === "deny") throw new Error("Path denied") + } + }, + }, + ), + ).rejects.toThrow("Path denied") + }) }) - test("asks for external_directory permission when workdir is outside project", async () => { + test("checks external_directory for parent listings", async () => { await using tmp = await tmpdir({ git: true }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const requests: Array> = [] - const testCtx = { - ...ctx, - ask: async (req: Omit) => { - requests.push(req) + const bash = await initTool(BashTool) + await expect( + bash.execute( + { command: "ls ..", description: "List parent directory" }, + { + ...ctx, + ask: async (request: Omit) => { + if (request.permission === "external_directory") throw new Error("External directory denied") + }, }, - } - await bash.execute( + ), + ).rejects.toThrow("External directory denied") + }) + }) + + test("does not merge distinct raw rules for the same absolute command", async () => { + await using tmp = await tmpdir({ git: true }) + await Bun.write(path.join(tmp.path, "local.txt"), "private content") + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + const ruleset: PermissionNext.Ruleset = [ + { permission: "bash", pattern: "*", action: "allow" }, + { permission: "bash", pattern: "cat local.txt", action: "deny" }, + { permission: "bash", pattern: "cat ./local.txt", action: "allow" }, + ] + await expect( + bash.execute( + { command: "cat local.txt; cat ./local.txt", description: "Read local file twice" }, { - command: "ls", - workdir: os.tmpdir(), - description: "List temp dir", + ...ctx, + ask: async (request: Omit) => { + for (const pattern of request.patterns) { + if (evaluateRequest(request, pattern, ruleset).action === "deny") throw new Error("Path denied") + } + }, }, - testCtx, - ) - const extDirReq = requests.find((r) => r.permission === "external_directory") - expect(extDirReq).toBeDefined() - expect(extDirReq!.patterns).toContain(path.join(os.tmpdir(), "*")) - }) + ), + ).rejects.toThrow("Path denied") + }) + }) + + test("resolves relative workdir from the instance directory", async () => { + await using tmp = await tmpdir({ git: true }) + await fs.mkdir(path.join(tmp.path, "nested")) + await Bun.write(path.join(tmp.path, "nested", "local.txt"), "nested content") + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute( + { + command: "cat local.txt", + workdir: "nested", + description: "Read nested file", + }, + ctx, + ) + expect(result.metadata.exit).toBe(0) + expect(result.output).toContain("nested content") + }) + }) + + test("asks for bash permission with correct pattern", async () => { + await using tmp = await tmpdir({ git: true }) + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "echo hello", + description: "Echo hello", + }, + testCtx, + ) + expect(requests.length).toBe(1) + expect(requests[0].permission).toBe("bash") + expect(requests[0].patterns).toContain("echo hello") + }) + }) + + test("asks for bash permission with multiple commands", async () => { + await using tmp = await tmpdir({ git: true }) + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "echo foo && echo bar", + description: "Echo twice", + }, + testCtx, + ) + expect(requests.length).toBe(1) + expect(requests[0].permission).toBe("bash") + expect(requests[0].patterns).toContain("echo foo") + expect(requests[0].patterns).toContain("echo bar") + }) + }) + + test("asks for external_directory permission when cd to parent", async () => { + await using tmp = await tmpdir({ git: true }) + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "cd ../", + description: "Change to parent directory", + }, + testCtx, + ) + const extDirReq = requests.find((r) => r.permission === "external_directory") + expect(extDirReq).toBeDefined() + }) + }) + + test("asks for external_directory permission when workdir is outside project", async () => { + await using tmp = await tmpdir({ git: true }) + await provideInstance(tmp.path, async () => { + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "ls", + workdir: os.tmpdir(), + description: "List temp dir", + }, + testCtx, + ) + const extDirReq = requests.find((r) => r.permission === "external_directory") + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(path.join(os.tmpdir(), "*")) + }) }) test("asks for external_directory permission when file arg is outside project", async () => { @@ -248,215 +544,215 @@ describe("tool.bash permissions", () => { }) await using tmp = await tmpdir({ git: true }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const requests: Array> = [] - const testCtx = { - ...ctx, - ask: async (req: Omit) => { - requests.push(req) - }, - } - const filepath = path.join(outerTmp.path, "outside.txt") - await bash.execute( - { - command: `cat ${filepath}`, - description: "Read external file", - }, - testCtx, - ) - const extDirReq = requests.find((r) => r.permission === "external_directory") - const expected = path.join(outerTmp.path, "*") - expect(extDirReq).toBeDefined() - expect(extDirReq!.patterns).toContain(expected) - expect(extDirReq!.always).toContain(expected) - }) + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + const filepath = path.join(outerTmp.path, "outside.txt") + await bash.execute( + { + command: `cat ${filepath}`, + description: "Read external file", + }, + testCtx, + ) + const extDirReq = requests.find((r) => r.permission === "external_directory") + const expected = path.join(outerTmp.path, "*") + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(expected) + expect(extDirReq!.always).toContain(expected) + }) }) test("does not ask for external_directory permission when rm inside project", async () => { await using tmp = await tmpdir({ git: true }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const requests: Array> = [] - const testCtx = { - ...ctx, - ask: async (req: Omit) => { - requests.push(req) - }, - } + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } - await Bun.write(path.join(tmp.path, "tmpfile"), "x") + await Bun.write(path.join(tmp.path, "tmpfile"), "x") - await bash.execute( - { - command: `rm -rf ${path.join(tmp.path, "nested")}`, - description: "remove nested dir", - }, - testCtx, - ) + await bash.execute( + { + command: `rm -rf ${path.join(tmp.path, "nested")}`, + description: "remove nested dir", + }, + testCtx, + ) - const extDirReq = requests.find((r) => r.permission === "external_directory") - expect(extDirReq).toBeUndefined() - }) + const extDirReq = requests.find((r) => r.permission === "external_directory") + expect(extDirReq).toBeUndefined() + }) }) test("includes always patterns for auto-approval", async () => { await using tmp = await tmpdir({ git: true }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const requests: Array> = [] - const testCtx = { - ...ctx, - ask: async (req: Omit) => { - requests.push(req) - }, - } - await bash.execute( - { - command: "git log --oneline -5", - description: "Git log", - }, - testCtx, - ) - expect(requests.length).toBe(1) - expect(requests[0].always.length).toBeGreaterThan(0) - expect(requests[0].always.some((p) => p.endsWith("*"))).toBe(true) - }) + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "git log --oneline -5", + description: "Git log", + }, + testCtx, + ) + expect(requests.length).toBe(1) + expect(requests[0].always.length).toBeGreaterThan(0) + expect(requests[0].always.some((p) => p.endsWith("*"))).toBe(true) + }) }) test("does not ask for bash permission when command is cd only", async () => { await using tmp = await tmpdir({ git: true }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const requests: Array> = [] - const testCtx = { - ...ctx, - ask: async (req: Omit) => { - requests.push(req) - }, - } - await bash.execute( - { - command: "cd .", - description: "Stay in current directory", - }, - testCtx, - ) - const bashReq = requests.find((r) => r.permission === "bash") - expect(bashReq).toBeUndefined() - }) + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "cd .", + description: "Stay in current directory", + }, + testCtx, + ) + const bashReq = requests.find((r) => r.permission === "bash") + expect(bashReq).toBeUndefined() + }) }) test("matches redirects in permission pattern", async () => { await using tmp = await tmpdir({ git: true }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const requests: Array> = [] - const testCtx = { - ...ctx, - ask: async (req: Omit) => { - requests.push(req) - }, - } - await bash.execute({ command: "cat > /tmp/output.txt", description: "Redirect ls output" }, testCtx) - const bashReq = requests.find((r) => r.permission === "bash") - expect(bashReq).toBeDefined() - expect(bashReq!.patterns).toContain("cat > /tmp/output.txt") - }) + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute({ command: "cat > /tmp/output.txt", description: "Redirect ls output" }, testCtx) + const bashReq = requests.find((r) => r.permission === "bash") + expect(bashReq).toBeDefined() + expect(bashReq!.patterns).toContain("cat > /tmp/output.txt") + }) }) test("always pattern has space before wildcard to not include different commands", async () => { await using tmp = await tmpdir({ git: true }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const requests: Array> = [] - const testCtx = { - ...ctx, - ask: async (req: Omit) => { - requests.push(req) - }, - } - await bash.execute({ command: "ls -la", description: "List" }, testCtx) - const bashReq = requests.find((r) => r.permission === "bash") - expect(bashReq).toBeDefined() - const pattern = bashReq!.always[0] - expect(pattern).toBe("ls *") - }) + const bash = await initTool(BashTool) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute({ command: "ls -la", description: "List" }, testCtx) + const bashReq = requests.find((r) => r.permission === "bash") + expect(bashReq).toBeDefined() + const pattern = bashReq!.always[0] + expect(pattern).toBe("ls *") + }) }) }) describe("tool.bash truncation", () => { test("truncates output exceeding line limit", async () => { await provideInstance(projectRoot, async () => { - const bash = await initToolWithTruncate(BashTool) - const lineCount = Truncate.MAX_LINES + 500 - const result = await bash.execute( - { - command: `seq 1 ${lineCount}`, - description: "Generate lines exceeding limit", - }, - ctx, - ) - expect((result.metadata as any).truncated).toBe(true) - expect(result.output).toContain("truncated") - expect(result.output).toContain("The tool call succeeded but the output was truncated") - }) + const bash = await initToolWithTruncate(BashTool) + const lineCount = Truncate.MAX_LINES + 500 + const result = await bash.execute( + { + command: `seq 1 ${lineCount}`, + description: "Generate lines exceeding limit", + }, + ctx, + ) + expect((result.metadata as any).truncated).toBe(true) + expect(result.output).toContain("truncated") + expect(result.output).toContain("The tool call succeeded but the output was truncated") + }) }) test("truncates output exceeding byte limit", async () => { await provideInstance(projectRoot, async () => { - const bash = await initToolWithTruncate(BashTool) - const byteCount = Truncate.MAX_BYTES + 10000 - const result = await bash.execute( - { - command: `head -c ${byteCount} /dev/zero | tr '\\0' 'a'`, - description: "Generate bytes exceeding limit", - }, - ctx, - ) - expect((result.metadata as any).truncated).toBe(true) - expect(result.output).toContain("truncated") - expect(result.output).toContain("The tool call succeeded but the output was truncated") - }) + const bash = await initToolWithTruncate(BashTool) + const byteCount = Truncate.MAX_BYTES + 10000 + const result = await bash.execute( + { + command: `head -c ${byteCount} /dev/zero | tr '\\0' 'a'`, + description: "Generate bytes exceeding limit", + }, + ctx, + ) + expect((result.metadata as any).truncated).toBe(true) + expect(result.output).toContain("truncated") + expect(result.output).toContain("The tool call succeeded but the output was truncated") + }) }) test("does not truncate small output", async () => { await provideInstance(projectRoot, async () => { - const bash = await initTool(BashTool) - const result = await bash.execute( - { - command: "echo hello", - description: "Echo hello", - }, - ctx, - ) - expect((result.metadata as any).truncated).toBe(false) - const eol = process.platform === "win32" ? "\r\n" : "\n" - expect(result.output).toBe(`hello${eol}`) - }) + const bash = await initTool(BashTool) + const result = await bash.execute( + { + command: "echo hello", + description: "Echo hello", + }, + ctx, + ) + expect((result.metadata as any).truncated).toBe(false) + const eol = process.platform === "win32" ? "\r\n" : "\n" + expect(result.output).toBe(`hello${eol}`) + }) }) test("full output is saved to file when truncated", async () => { await provideInstance(projectRoot, async () => { - const bash = await initToolWithTruncate(BashTool) - const lineCount = Truncate.MAX_LINES + 100 - const result = await bash.execute( - { - command: `seq 1 ${lineCount}`, - description: "Generate lines for file check", - }, - ctx, - ) - expect((result.metadata as any).truncated).toBe(true) + const bash = await initToolWithTruncate(BashTool) + const lineCount = Truncate.MAX_LINES + 100 + const result = await bash.execute( + { + command: `seq 1 ${lineCount}`, + description: "Generate lines for file check", + }, + ctx, + ) + expect((result.metadata as any).truncated).toBe(true) - const filepath = (result.metadata as any).outputPath - expect(filepath).toBeTruthy() + const filepath = (result.metadata as any).outputPath + expect(filepath).toBeTruthy() - const saved = await Filesystem.readText(filepath) - const lines = saved.trim().split("\n") - expect(lines.length).toBe(lineCount) - expect(lines[0]).toBe("1") - expect(lines[lineCount - 1]).toBe(String(lineCount)) - }) + const saved = await Filesystem.readText(filepath) + const lines = saved.trim().split("\n") + expect(lines.length).toBe(lineCount) + expect(lines[0]).toBe("1") + expect(lines[lineCount - 1]).toBe(String(lineCount)) + }) }) }) @@ -466,48 +762,44 @@ describe("tool.bash PATH injection", () => { await using tmp = await tmpdir({ git: true }) const toolsDir = path.join(tmp.path, ".opencode", "tools") await fs.mkdir(toolsDir, { recursive: true }) - await fs.writeFile( - path.join(toolsDir, "my-custom-tool"), - '#!/usr/bin/env bash\necho "custom-tool-output"', - { mode: 0o755 }, - ) + await fs.writeFile(path.join(toolsDir, "my-custom-tool"), '#!/usr/bin/env bash\necho "custom-tool-output"', { + mode: 0o755, + }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const result = await bash.execute( - { - command: "my-custom-tool", - description: "Run custom tool from .opencode/tools/", - }, - ctx, - ) - expect(result.metadata.exit).toBe(0) - expect(result.metadata.output).toContain("custom-tool-output") - }) + const bash = await initTool(BashTool) + const result = await bash.execute( + { + command: "my-custom-tool", + description: "Run custom tool from .opencode/tools/", + }, + ctx, + ) + expect(result.metadata.exit).toBe(0) + expect(result.metadata.output).toContain("custom-tool-output") + }) }) test(".altimate-code/tools/ is prepended to PATH so user tools are executable", async () => { await using tmp = await tmpdir({ git: true }) const toolsDir = path.join(tmp.path, ".altimate-code", "tools") await fs.mkdir(toolsDir, { recursive: true }) - await fs.writeFile( - path.join(toolsDir, "my-altimate-tool"), - '#!/usr/bin/env bash\necho "altimate-tool-output"', - { mode: 0o755 }, - ) + await fs.writeFile(path.join(toolsDir, "my-altimate-tool"), '#!/usr/bin/env bash\necho "altimate-tool-output"', { + mode: 0o755, + }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const result = await bash.execute( - { - command: "my-altimate-tool", - description: "Run custom tool from .altimate-code/tools/", - }, - ctx, - ) - expect(result.metadata.exit).toBe(0) - expect(result.metadata.output).toContain("altimate-tool-output") - }) + const bash = await initTool(BashTool) + const result = await bash.execute( + { + command: "my-altimate-tool", + description: "Run custom tool from .altimate-code/tools/", + }, + ctx, + ) + expect(result.metadata.exit).toBe(0) + expect(result.metadata.output).toContain("altimate-tool-output") + }) }) test("PATH does not contain duplicate .opencode/tools/ entries", async () => { @@ -516,44 +808,44 @@ describe("tool.bash PATH injection", () => { await fs.mkdir(toolsDir, { recursive: true }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const result = await bash.execute( - { - command: 'echo "$PATH"', - description: "Print PATH", - }, - ctx, - ) - const pathValue = result.metadata.output.trim() - const sep = process.platform === "win32" ? ";" : ":" - const entries = pathValue.split(sep) - const count = entries.filter((e: string) => e === toolsDir).length - expect(count).toBeLessThanOrEqual(1) - }) + const bash = await initTool(BashTool) + const result = await bash.execute( + { + command: 'echo "$PATH"', + description: "Print PATH", + }, + ctx, + ) + const pathValue = result.metadata.output.trim() + const sep = process.platform === "win32" ? ";" : ":" + const entries = pathValue.split(sep) + const count = entries.filter((e: string) => e === toolsDir).length + expect(count).toBeLessThanOrEqual(1) + }) }) test("ALTIMATE_BIN_DIR is on PATH with higher priority than system dirs", async () => { await using tmp = await tmpdir({ git: true }) await provideInstance(tmp.path, async () => { - const bash = await initTool(BashTool) - const result = await bash.execute( - { - command: 'echo "$PATH"', - description: "Print PATH to check ALTIMATE_BIN_DIR", - }, - ctx, - ) - const pathValue = result.metadata.output.trim() - const binDir = process.env.ALTIMATE_BIN_DIR - if (binDir) { - const sep = process.platform === "win32" ? ";" : ":" - const entries = pathValue.split(sep) - const binIdx = entries.indexOf(binDir) - // ALTIMATE_BIN_DIR should appear early in PATH (prepended, not appended) - expect(binIdx).toBeGreaterThanOrEqual(0) - expect(binIdx).toBeLessThan(5) - } - }) + const bash = await initTool(BashTool) + const result = await bash.execute( + { + command: 'echo "$PATH"', + description: "Print PATH to check ALTIMATE_BIN_DIR", + }, + ctx, + ) + const pathValue = result.metadata.output.trim() + const binDir = process.env.ALTIMATE_BIN_DIR + if (binDir) { + const sep = process.platform === "win32" ? ";" : ":" + const entries = pathValue.split(sep) + const binIdx = entries.indexOf(binDir) + // ALTIMATE_BIN_DIR should appear early in PATH (prepended, not appended) + expect(binIdx).toBeGreaterThanOrEqual(0) + expect(binIdx).toBeLessThan(5) + } + }) }) }) // altimate_change end diff --git a/packages/opencode/test/tool/read.test.ts b/packages/opencode/test/tool/read.test.ts index e2be87a31e..deba1f0955 100644 --- a/packages/opencode/test/tool/read.test.ts +++ b/packages/opencode/test/tool/read.test.ts @@ -53,6 +53,7 @@ const readLayer = (flags: Partial = {}) => CrossSpawnSpawner.defaultLayer, Instruction.defaultLayer, LSP.defaultLayer, + Permission.defaultLayer, Ripgrep.defaultLayer, Truncate.defaultLayer, ) @@ -279,6 +280,67 @@ describe("tool.read external_directory permission", () => { ) }) +describe("tool.read absolute path permissions", () => { + const guarded = Effect.fn("ReadToolTest.guarded")(function* (ruleset: PermissionV1.Ruleset) { + const permission = yield* Permission.Service + return { + ...ctx, + ask: (request: Omit) => + permission.ask({ ...request, sessionID: ctx.sessionID, ruleset }).pipe(Effect.orDie), + } + }) + + it.live("denies relative and absolute sibling paths with the same absolute rule", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + const current = path.join(dir, "current") + const sibling = path.join(dir, "sibling", "output.md") + yield* put(path.join(current, "local.md"), "local content") + yield* put(sibling, "private content") + const next = yield* guarded([ + { permission: "read", pattern: "*", action: "allow" }, + { permission: "read", pattern: full(sibling), action: "deny" }, + ]) + + for (const filePath of ["../sibling/output.md", sibling, `${current}/../sibling/output.md`]) { + expect(yield* fail(current, { filePath }, next)).toBeInstanceOf(PermissionV1.DeniedError) + } + }), + ) + + it.live("denies directory aliases and relative filenames with absolute rules", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + const filepath = path.join(dir, "private.md") + yield* put(filepath, "private content") + const next = yield* guarded([ + { permission: "read", pattern: "*", action: "allow" }, + { permission: "read", pattern: full(dir), action: "deny" }, + { permission: "read", pattern: "*/private.md", action: "deny" }, + ]) + + for (const filePath of ["", ".", dir, "private.md", filepath]) { + expect(yield* fail(dir, { filePath }, next)).toBeInstanceOf(PermissionV1.DeniedError) + } + }), + ) + + it.live("preserves worktree-relative allowlist rules", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + yield* put(path.join(dir, "public.md"), "public content") + yield* put(path.join(dir, "private.md"), "private content") + const next = yield* guarded([ + { permission: "read", pattern: "*", action: "deny" }, + { permission: "read", pattern: "public.md", action: "allow" }, + ]) + + expect((yield* exec(dir, { filePath: "public.md" }, next)).output).toContain("public content") + expect(yield* fail(dir, { filePath: "private.md" }, next)).toBeInstanceOf(PermissionV1.DeniedError) + }), + ) +}) + describe("tool.read env file permissions", () => { const cases: [string, boolean][] = [ [".env", true],