core: extract external directory validation to shared utility to reduce code duplication across tools

10 files changed, 186 insertions, 49 deletions

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<Tool.Context, "ask"> = {
+  sessionID: "test",
+  messageID: "",
+  callID: "",
+  agent: "build",
+  abort: AbortSignal.any([]),
+  metadata: () => {},
+}
+
+describe("tool.assertExternalDirectory", () => {
+  test("no-ops for empty target", async () => {
+    const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
+    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<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
+    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<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
+    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<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
+    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<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
+    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)
+  })
+})