diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 787282ecd..b68078f14 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -15,6 +15,7 @@ import { FileTime } from "../file/time" import { Filesystem } from "../util/filesystem" import { Instance } from "../project/instance" import { Snapshot } from "@/snapshot" +import { assertExternalDirectory } from "./external-directory" const MAX_DIAGNOSTICS_PER_FILE = 20 @@ -40,18 +41,7 @@ export const EditTool = Tool.define("edit", { } const filePath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) - if (!Filesystem.contains(Instance.directory, filePath)) { - const parentDir = path.dirname(filePath) - await ctx.ask({ - permission: "external_directory", - patterns: [parentDir, path.join(parentDir, "*")], - always: [parentDir + "/*"], - metadata: { - filepath: filePath, - parentDir, - }, - }) - } + await assertExternalDirectory(ctx, filePath) let diff = "" let contentOld = "" diff --git a/packages/opencode/src/tool/external-directory.ts b/packages/opencode/src/tool/external-directory.ts new file mode 100644 index 000000000..05f83c00d --- /dev/null +++ b/packages/opencode/src/tool/external-directory.ts @@ -0,0 +1,33 @@ +import path from "path" +import type { Tool } from "./tool" +import { Filesystem } from "../util/filesystem" +import { Instance } from "../project/instance" + +type Kind = "file" | "directory" + +type Options = { + bypass?: boolean + kind?: Kind +} + +export async function assertExternalDirectory(ctx: Tool.Context, target?: string, options?: Options) { + if (!target) return + + if (options?.bypass) return + + if (Filesystem.contains(Instance.directory, target)) return + + const kind = options?.kind ?? "file" + const parentDir = kind === "directory" ? target : path.dirname(target) + const glob = path.join(parentDir, "*") + + await ctx.ask({ + permission: "external_directory", + patterns: [glob], + always: [glob], + metadata: { + filepath: target, + parentDir, + }, + }) +} diff --git a/packages/opencode/src/tool/glob.ts b/packages/opencode/src/tool/glob.ts index 0c643796d..dda57f6ee 100644 --- a/packages/opencode/src/tool/glob.ts +++ b/packages/opencode/src/tool/glob.ts @@ -4,6 +4,7 @@ import { Tool } from "./tool" import DESCRIPTION from "./glob.txt" import { Ripgrep } from "../file/ripgrep" import { Instance } from "../project/instance" +import { assertExternalDirectory } from "./external-directory" export const GlobTool = Tool.define("glob", { description: DESCRIPTION, @@ -29,6 +30,7 @@ export const GlobTool = Tool.define("glob", { let search = params.path ?? Instance.directory search = path.isAbsolute(search) ? search : path.resolve(Instance.directory, search) + await assertExternalDirectory(ctx, search, { kind: "directory" }) const limit = 100 const files = [] diff --git a/packages/opencode/src/tool/grep.ts b/packages/opencode/src/tool/grep.ts index b7cba1674..ad62621e0 100644 --- a/packages/opencode/src/tool/grep.ts +++ b/packages/opencode/src/tool/grep.ts @@ -4,6 +4,8 @@ import { Ripgrep } from "../file/ripgrep" import DESCRIPTION from "./grep.txt" import { Instance } from "../project/instance" +import path from "path" +import { assertExternalDirectory } from "./external-directory" const MAX_LINE_LENGTH = 2000 @@ -30,7 +32,9 @@ export const GrepTool = Tool.define("grep", { }, }) - const searchPath = params.path || Instance.directory + let searchPath = params.path ?? Instance.directory + searchPath = path.isAbsolute(searchPath) ? searchPath : path.resolve(Instance.directory, searchPath) + await assertExternalDirectory(ctx, searchPath, { kind: "directory" }) const rgPath = await Ripgrep.filepath() const args = ["-nH", "--hidden", "--follow", "--field-match-separator=|", "--regexp", params.pattern] diff --git a/packages/opencode/src/tool/ls.ts b/packages/opencode/src/tool/ls.ts index b8638b3e9..cc3d75007 100644 --- a/packages/opencode/src/tool/ls.ts +++ b/packages/opencode/src/tool/ls.ts @@ -4,6 +4,7 @@ import * as path from "path" import DESCRIPTION from "./ls.txt" import { Instance } from "../project/instance" import { Ripgrep } from "../file/ripgrep" +import { assertExternalDirectory } from "./external-directory" export const IGNORE_PATTERNS = [ "node_modules/", @@ -42,6 +43,7 @@ export const ListTool = Tool.define("list", { }), async execute(params, ctx) { const searchPath = path.resolve(Instance.directory, params.path || ".") + await assertExternalDirectory(ctx, searchPath, { kind: "directory" }) await ctx.ask({ permission: "list", diff --git a/packages/opencode/src/tool/lsp.ts b/packages/opencode/src/tool/lsp.ts index df4692bf6..ca352280b 100644 --- a/packages/opencode/src/tool/lsp.ts +++ b/packages/opencode/src/tool/lsp.ts @@ -5,6 +5,7 @@ import { LSP } from "../lsp" import DESCRIPTION from "./lsp.txt" import { Instance } from "../project/instance" import { pathToFileURL } from "url" +import { assertExternalDirectory } from "./external-directory" const operations = [ "goToDefinition", @@ -27,14 +28,15 @@ export const LspTool = Tool.define("lsp", { character: z.number().int().min(1).describe("The character offset (1-based, as shown in editors)"), }), execute: async (args, ctx) => { + const file = path.isAbsolute(args.filePath) ? args.filePath : path.join(Instance.directory, args.filePath) + await assertExternalDirectory(ctx, file) + await ctx.ask({ permission: "lsp", patterns: ["*"], always: ["*"], metadata: {}, }) - - const file = path.isAbsolute(args.filePath) ? args.filePath : path.join(Instance.directory, args.filePath) const uri = pathToFileURL(file).href const position = { file, diff --git a/packages/opencode/src/tool/patch.ts b/packages/opencode/src/tool/patch.ts index 62d9f70f2..08a58bfea 100644 --- a/packages/opencode/src/tool/patch.ts +++ b/packages/opencode/src/tool/patch.ts @@ -7,8 +7,8 @@ import { Bus } from "../bus" import { FileWatcher } from "../file/watcher" import { Instance } from "../project/instance" import { Patch } from "../patch" -import { Filesystem } from "../util/filesystem" import { createTwoFilesPatch } from "diff" +import { assertExternalDirectory } from "./external-directory" const PatchParams = z.object({ patchText: z.string().describe("The full patch text that describes all changes to be made"), @@ -49,19 +49,7 @@ export const PatchTool = Tool.define("patch", { for (const hunk of hunks) { const filePath = path.resolve(Instance.directory, hunk.path) - - if (!Filesystem.contains(Instance.directory, filePath)) { - const parentDir = path.dirname(filePath) - await ctx.ask({ - permission: "external_directory", - patterns: [parentDir, path.join(parentDir, "*")], - always: [parentDir + "/*"], - metadata: { - filepath: filePath, - parentDir, - }, - }) - } + await assertExternalDirectory(ctx, filePath) switch (hunk.type) { case "add": @@ -103,12 +91,15 @@ export const PatchTool = Tool.define("patch", { const diff = createTwoFilesPatch(filePath, filePath, oldContent, newContent) + const movePath = hunk.move_path ? path.resolve(Instance.directory, hunk.move_path) : undefined + await assertExternalDirectory(ctx, movePath) + fileChanges.push({ filePath, oldContent, newContent, type: hunk.move_path ? "move" : "update", - movePath: hunk.move_path ? path.resolve(Instance.directory, hunk.move_path) : undefined, + movePath, }) totalDiff += diff + "\n" diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index 567425174..ce4ab2861 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -5,9 +5,9 @@ import { Tool } from "./tool" import { LSP } from "../lsp" import { FileTime } from "../file/time" import DESCRIPTION from "./read.txt" -import { Filesystem } from "../util/filesystem" import { Instance } from "../project/instance" import { Identifier } from "../id/id" +import { assertExternalDirectory } from "./external-directory" const DEFAULT_READ_LIMIT = 2000 const MAX_LINE_LENGTH = 2000 @@ -27,18 +27,9 @@ export const ReadTool = Tool.define("read", { } const title = path.relative(Instance.worktree, filepath) - if (!ctx.extra?.["bypassCwdCheck"] && !Filesystem.contains(Instance.directory, filepath)) { - const parentDir = path.dirname(filepath) - await ctx.ask({ - permission: "external_directory", - patterns: [parentDir], - always: [parentDir + "/*"], - metadata: { - filepath, - parentDir, - }, - }) - } + await assertExternalDirectory(ctx, filepath, { + bypass: Boolean(ctx.extra?.["bypassCwdCheck"]), + }) await ctx.ask({ permission: "read", diff --git a/packages/opencode/src/tool/write.ts b/packages/opencode/src/tool/write.ts index a0ca6b14f..222bac3f8 100644 --- a/packages/opencode/src/tool/write.ts +++ b/packages/opencode/src/tool/write.ts @@ -10,6 +10,7 @@ import { FileTime } from "../file/time" import { Filesystem } from "../util/filesystem" import { Instance } from "../project/instance" import { trimDiff } from "./edit" +import { assertExternalDirectory } from "./external-directory" const MAX_DIAGNOSTICS_PER_FILE = 20 const MAX_PROJECT_DIAGNOSTICS_FILES = 5 @@ -22,12 +23,7 @@ export const WriteTool = Tool.define("write", { }), async execute(params, ctx) { const filepath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) - /* TODO - if (!Filesystem.contains(Instance.directory, filepath)) { - const parentDir = path.dirname(filepath) - ... - } - */ + await assertExternalDirectory(ctx, filepath) const file = Bun.file(filepath) const exists = await file.exists() diff --git a/packages/opencode/test/tool/external-directory.test.ts b/packages/opencode/test/tool/external-directory.test.ts new file mode 100644 index 000000000..b21f6a971 --- /dev/null +++ b/packages/opencode/test/tool/external-directory.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, test } from "bun:test" +import path from "path" +import type { Tool } from "../../src/tool/tool" +import { Instance } from "../../src/project/instance" +import { assertExternalDirectory } from "../../src/tool/external-directory" +import type { PermissionNext } from "../../src/permission/next" + +const baseCtx: Omit = { + sessionID: "test", + messageID: "", + callID: "", + agent: "build", + abort: AbortSignal.any([]), + metadata: () => {}, +} + +describe("tool.assertExternalDirectory", () => { + test("no-ops for empty target", async () => { + const requests: Array> = [] + const ctx: Tool.Context = { + ...baseCtx, + ask: async (req) => { + requests.push(req) + }, + } + + await Instance.provide({ + directory: "/tmp", + fn: async () => { + await assertExternalDirectory(ctx) + }, + }) + + expect(requests.length).toBe(0) + }) + + test("no-ops for paths inside Instance.directory", async () => { + const requests: Array> = [] + const ctx: Tool.Context = { + ...baseCtx, + ask: async (req) => { + requests.push(req) + }, + } + + await Instance.provide({ + directory: "/tmp/project", + fn: async () => { + await assertExternalDirectory(ctx, path.join("/tmp/project", "file.txt")) + }, + }) + + expect(requests.length).toBe(0) + }) + + test("asks with a single canonical glob", async () => { + const requests: Array> = [] + const ctx: Tool.Context = { + ...baseCtx, + ask: async (req) => { + requests.push(req) + }, + } + + const directory = "/tmp/project" + const target = "/tmp/outside/file.txt" + const expected = path.join(path.dirname(target), "*") + + await Instance.provide({ + directory, + fn: async () => { + await assertExternalDirectory(ctx, target) + }, + }) + + const req = requests.find((r) => r.permission === "external_directory") + expect(req).toBeDefined() + expect(req!.patterns).toEqual([expected]) + expect(req!.always).toEqual([expected]) + }) + + test("uses target directory when kind=directory", async () => { + const requests: Array> = [] + const ctx: Tool.Context = { + ...baseCtx, + ask: async (req) => { + requests.push(req) + }, + } + + const directory = "/tmp/project" + const target = "/tmp/outside" + const expected = path.join(target, "*") + + await Instance.provide({ + directory, + fn: async () => { + await assertExternalDirectory(ctx, target, { kind: "directory" }) + }, + }) + + const req = requests.find((r) => r.permission === "external_directory") + expect(req).toBeDefined() + expect(req!.patterns).toEqual([expected]) + expect(req!.always).toEqual([expected]) + }) + + test("skips prompting when bypass=true", async () => { + const requests: Array> = [] + const ctx: Tool.Context = { + ...baseCtx, + ask: async (req) => { + requests.push(req) + }, + } + + await Instance.provide({ + directory: "/tmp/project", + fn: async () => { + await assertExternalDirectory(ctx, "/tmp/outside/file.txt", { bypass: true }) + }, + }) + + expect(requests.length).toBe(0) + }) +})