fix: address external_directory gaps and improve symlink checks

packages/opencode/src/file/index.ts

@@ -277,18 +277,21 @@ export namespace File {
     const project = Instance.project
     const full = path.join(Instance.directory, file)
 
-    // TODO: Filesystem.contains is lexical only - symlinks inside the project can escape.
-    // TODO: On Windows, cross-drive paths bypass this check. Consider realpath canonicalization.
+    // First do lexical check to catch obvious path traversal attempts
     if (!Instance.containsPath(full)) {
       throw new Error(`Access denied: path escapes project directory`)
     }
 
     const bunFile = Bun.file(full)
-
     if (!(await bunFile.exists())) {
       return { type: "text", content: "" }
     }
 
+    // For existing files, also check resolved path to catch symlinks pointing outside
+    if (!Filesystem.containsResolved(Instance.directory, full)) {
+      throw new Error(`Access denied: path escapes project directory`)
+    }
+
     const encode = await shouldEncode(bunFile)
 
     if (encode) {
@@ -337,9 +340,8 @@ export namespace File {
     }
     const resolved = dir ? path.join(Instance.directory, dir) : Instance.directory
 
-    // TODO: Filesystem.contains is lexical only - symlinks inside the project can escape.
-    // TODO: On Windows, cross-drive paths bypass this check. Consider realpath canonicalization.
-    if (!Instance.containsPath(resolved)) {
+    // Use containsResolved to handle symlinks that point outside the project
+    if (!Instance.containsPath(resolved) || !Filesystem.containsResolved(Instance.directory, resolved)) {
       throw new Error(`Access denied: path escapes project directory`)
     }
 

packages/opencode/src/tool/edit.ts

@@ -43,6 +43,20 @@ export const EditTool = Tool.define("edit", {
     const filePath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath)
     await assertExternalDirectory(ctx, filePath)
 
+    // Check if path is a symlink pointing outside (even if target doesn't exist)
+    if (!Filesystem.containsResolved(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,
+        },
+      })
+    }
+
     let diff = ""
     let contentOld = ""
     let contentNew = ""

packages/opencode/src/tool/patch.ts

@@ -8,6 +8,7 @@ import { FileWatcher } from "../file/watcher"
 import { Instance } from "../project/instance"
 import { Patch } from "../patch"
 import { createTwoFilesPatch } from "diff"
+import { Filesystem } from "../util/filesystem"
 import { assertExternalDirectory } from "./external-directory"
 
 const PatchParams = z.object({
@@ -51,6 +52,20 @@ export const PatchTool = Tool.define("patch", {
       const filePath = path.resolve(Instance.directory, hunk.path)
       await assertExternalDirectory(ctx, filePath)
 
+      // Check if path is a symlink pointing outside (even if target doesn't exist)
+      if (!Filesystem.containsResolved(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,
+          },
+        })
+      }
+
       switch (hunk.type) {
         case "add":
           if (hunk.type === "add") {

packages/opencode/src/tool/read.ts

@@ -7,6 +7,7 @@ import { FileTime } from "../file/time"
 import DESCRIPTION from "./read.txt"
 import { Instance } from "../project/instance"
 import { Identifier } from "../id/id"
+import { Filesystem } from "../util/filesystem"
 import { assertExternalDirectory } from "./external-directory"
 
 const DEFAULT_READ_LIMIT = 2000
@@ -31,6 +32,21 @@ export const ReadTool = Tool.define("read", {
       bypass: Boolean(ctx.extra?.["bypassCwdCheck"]),
     })
 
+    // Check if path is a symlink pointing outside (even if target doesn't exist)
+    // This must happen BEFORE the exists check to catch broken symlinks
+    if (!ctx.extra?.["bypassCwdCheck"] && !Filesystem.containsResolved(Instance.directory, filepath)) {
+      const parentDir = path.dirname(filepath)
+      await ctx.ask({
+        permission: "external_directory",
+        patterns: [parentDir],
+        always: [parentDir + "/*"],
+        metadata: {
+          filepath,
+          parentDir,
+        },
+      })
+    }
+
     await ctx.ask({
       permission: "read",
       patterns: [filepath],
@@ -39,7 +55,9 @@ export const ReadTool = Tool.define("read", {
     })
 
     const file = Bun.file(filepath)
-    if (!(await file.exists())) {
+    const exists = await file.exists()
+
+    if (!exists) {
       const dir = path.dirname(filepath)
       const base = path.basename(filepath)
 

packages/opencode/src/tool/write.ts

@@ -25,6 +25,21 @@ export const WriteTool = Tool.define("write", {
     const filepath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath)
     await assertExternalDirectory(ctx, filepath)
 
+    // Check if path is a symlink pointing outside (even if target doesn't exist)
+    // This must happen BEFORE the exists check to catch broken symlinks
+    if (!Filesystem.containsResolved(Instance.directory, filepath)) {
+      const parentDir = path.dirname(filepath)
+      await ctx.ask({
+        permission: "external_directory",
+        patterns: [parentDir],
+        always: [parentDir + "/*"],
+        metadata: {
+          filepath,
+          parentDir,
+        },
+      })
+    }
+
     const file = Bun.file(filepath)
     const exists = await file.exists()
     const contentOld = exists ? await file.text() : ""

packages/opencode/src/util/filesystem.ts

@@ -1,6 +1,6 @@
-import { realpathSync } from "fs"
+import { lstatSync, readlinkSync, realpathSync } from "fs"
 import { exists } from "fs/promises"
-import { dirname, join, relative } from "path"
+import { dirname, isAbsolute, join, relative, resolve } from "path"
 
 export namespace Filesystem {
   /**
@@ -22,10 +22,75 @@ export namespace Filesystem {
     return !relA || !relA.startsWith("..") || !relB || !relB.startsWith("..")
   }
 
+  /**
+   * Lexical containment check - does NOT resolve symlinks.
+   * Use containsResolved() when the child path may be a symlink pointing outside.
+   */
   export function contains(parent: string, child: string) {
     return !relative(parent, child).startsWith("..")
   }
 
+  /**
+   * Containment check with symlink resolution.
+   * Returns true only if the resolved child path is within the resolved parent.
+   * Returns false if the path is a symlink pointing outside, even if broken.
+   *
+   * Note: There is an inherent TOCTOU (time-of-check-time-of-use) race condition
+   * between this check and actual file operations. This is acceptable for the
+   * threat model of protecting against malicious symlinks in user-controlled
+   * directories, but not against active attackers with concurrent write access.
+   */
+  export function containsResolved(parent: string, child: string): boolean {
+    try {
+      const resolvedParent = realpathSync(parent)
+
+      // First, check if the child path is a symlink
+      try {
+        const stats = lstatSync(child)
+        if (stats.isSymbolicLink()) {
+          // It's a symlink - check where it points
+          const linkTarget = readlinkSync(child)
+          const absoluteTarget = isAbsolute(linkTarget) ? linkTarget : resolve(dirname(child), linkTarget)
+
+          // Try to resolve the full path (handles symlink chains)
+          try {
+            const resolvedChild = realpathSync(child)
+            return !relative(resolvedParent, resolvedChild).startsWith("..")
+          } catch {
+            // Broken symlink - check if target would be inside parent.
+            // relative() normalizes paths, so traversal attempts like
+            // /project/../../../etc are correctly detected as escaping.
+            return !relative(resolvedParent, absoluteTarget).startsWith("..")
+          }
+        }
+      } catch {
+        // lstatSync failed - path doesn't exist at all (not even as broken symlink)
+      }
+
+      // Not a symlink - try to resolve normally
+      try {
+        const resolvedChild = realpathSync(child)
+        return !relative(resolvedParent, resolvedChild).startsWith("..")
+      } catch {
+        // Path doesn't exist - check parent directory
+        const childDir = dirname(child)
+        if (childDir === child) return false // root directory
+
+        try {
+          const resolvedChildDir = realpathSync(childDir)
+          return !relative(resolvedParent, resolvedChildDir).startsWith("..")
+        } catch {
+          // Parent directory also doesn't exist - fall back to lexical check.
+          // This is safe because symlinks can't exist if the parent doesn't.
+          return contains(parent, child)
+        }
+      }
+    } catch {
+      // Parent doesn't exist or can't be resolved - deny access
+      return false
+    }
+  }
+
   export async function findUp(target: string, start: string, stop?: string) {
     let current = start
     const result = []

packages/opencode/test/file/path-traversal.test.ts

@@ -1,35 +1,15 @@
 import { test, expect, describe } from "bun:test"
+import { $ } from "bun"
 import path from "path"
 import fs from "fs/promises"
-import { Filesystem } from "../../src/util/filesystem"
 import { File } from "../../src/file"
 import { Instance } from "../../src/project/instance"
 import { tmpdir } from "../fixture/fixture"
 
-describe("Filesystem.contains", () => {
-  test("allows paths within project", () => {
-    expect(Filesystem.contains("/project", "/project/src")).toBe(true)
-    expect(Filesystem.contains("/project", "/project/src/file.ts")).toBe(true)
-    expect(Filesystem.contains("/project", "/project")).toBe(true)
-  })
-
-  test("blocks ../ traversal", () => {
-    expect(Filesystem.contains("/project", "/project/../etc")).toBe(false)
-    expect(Filesystem.contains("/project", "/project/src/../../etc")).toBe(false)
-    expect(Filesystem.contains("/project", "/etc/passwd")).toBe(false)
-  })
-
-  test("blocks absolute paths outside project", () => {
-    expect(Filesystem.contains("/project", "/etc/passwd")).toBe(false)
-    expect(Filesystem.contains("/project", "/tmp/file")).toBe(false)
-    expect(Filesystem.contains("/home/user/project", "/home/user/other")).toBe(false)
-  })
-
-  test("handles prefix collision edge cases", () => {
-    expect(Filesystem.contains("/project", "/project-other/file")).toBe(false)
-    expect(Filesystem.contains("/project", "/projectfile")).toBe(false)
-  })
-})
+/*
+ * NOTE: Filesystem.contains() and Filesystem.containsResolved() unit tests
+ * are in test/util/filesystem.test.ts - not duplicated here.
+ */
 
 /*
  * Integration tests for File.read() and File.list() path traversal protection.
@@ -84,6 +64,43 @@ describe("File.read path traversal protection", () => {
       },
     })
   })
+
+  test("rejects symlink pointing outside project", async () => {
+    await using tmp = await tmpdir()
+
+    // Create symlink inside project that points to /etc
+    const symlinkPath = path.join(tmp.path, "escape-link")
+    await $`ln -s /etc ${symlinkPath}`.quiet()
+
+    await Instance.provide({
+      directory: tmp.path,
+      fn: async () => {
+        // The symlink "escape-link" exists inside the project lexically,
+        // but points to /etc which is outside - should be rejected
+        await expect(File.read("escape-link/passwd")).rejects.toThrow("Access denied: path escapes project directory")
+      },
+    })
+  })
+
+  test("allows symlink pointing within project", async () => {
+    await using tmp = await tmpdir({
+      init: async (dir) => {
+        await Bun.write(path.join(dir, "target.txt"), "symlink target content")
+      },
+    })
+
+    // Create symlink pointing to file within same project
+    const symlinkPath = path.join(tmp.path, "internal-link.txt")
+    await $`ln -s ${path.join(tmp.path, "target.txt")} ${symlinkPath}`.quiet()
+
+    await Instance.provide({
+      directory: tmp.path,
+      fn: async () => {
+        const result = await File.read("internal-link.txt")
+        expect(result.content).toBe("symlink target content")
+      },
+    })
+  })
 })
 
 describe("File.list path traversal protection", () => {