From fa79736b875ba0a8437f42ff01626b35fab1d8f4 Mon Sep 17 00:00:00 2001 From: Dillon Mulroy Date: Sun, 11 Jan 2026 15:17:36 -0500 Subject: [PATCH] fix: check worktree for external_directory permission in subdirs (#7811) --- packages/opencode/src/file/index.ts | 4 +- packages/opencode/src/project/instance.ts | 13 +++ packages/opencode/src/tool/bash.ts | 4 +- .../opencode/src/tool/external-directory.ts | 3 +- .../opencode/test/file/path-traversal.test.ts | 83 +++++++++++++++++++ 5 files changed, 101 insertions(+), 6 deletions(-) diff --git a/packages/opencode/src/file/index.ts b/packages/opencode/src/file/index.ts index d2ff1d0b1..76b7be4b7 100644 --- a/packages/opencode/src/file/index.ts +++ b/packages/opencode/src/file/index.ts @@ -279,7 +279,7 @@ export namespace 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. - if (!Filesystem.contains(Instance.directory, full)) { + if (!Instance.containsPath(full)) { throw new Error(`Access denied: path escapes project directory`) } @@ -339,7 +339,7 @@ export namespace 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. - if (!Filesystem.contains(Instance.directory, resolved)) { + if (!Instance.containsPath(resolved)) { throw new Error(`Access denied: path escapes project directory`) } diff --git a/packages/opencode/src/project/instance.ts b/packages/opencode/src/project/instance.ts index d668037c0..ddaa90f1e 100644 --- a/packages/opencode/src/project/instance.ts +++ b/packages/opencode/src/project/instance.ts @@ -4,6 +4,7 @@ import { Project } from "./project" import { State } from "./state" import { iife } from "@/util/iife" import { GlobalBus } from "@/bus/global" +import { Filesystem } from "@/util/filesystem" interface Context { directory: string @@ -46,6 +47,18 @@ export const Instance = { get project() { return context.use().project }, + /** + * Check if a path is within the project boundary. + * Returns true if path is inside Instance.directory OR Instance.worktree. + * Paths within the worktree but outside the working directory should not trigger external_directory permission. + */ + containsPath(filepath: string) { + if (Filesystem.contains(Instance.directory, filepath)) return true + // Non-git projects set worktree to "/" which would match ANY absolute path. + // Skip worktree check in this case to preserve external_directory permissions. + if (Instance.worktree === "/") return false + return Filesystem.contains(Instance.worktree, filepath) + }, state(init: () => S, dispose?: (state: Awaited) => Promise): () => S { return State.create(() => Instance.directory, init, dispose) }, diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index e06a3f157..f3a1b04d4 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -85,7 +85,7 @@ export const BashTool = Tool.define("bash", async () => { throw new Error("Failed to parse command") } const directories = new Set() - if (!Filesystem.contains(Instance.directory, cwd)) directories.add(cwd) + if (!Instance.containsPath(cwd)) directories.add(cwd) const patterns = new Set() const always = new Set() @@ -124,7 +124,7 @@ export const BashTool = Tool.define("bash", async () => { process.platform === "win32" && resolved.match(/^\/[a-z]\//) ? resolved.replace(/^\/([a-z])\//, (_, drive) => `${drive.toUpperCase()}:\\`).replace(/\//g, "\\") : resolved - if (!Filesystem.contains(Instance.directory, normalized)) directories.add(normalized) + if (!Instance.containsPath(normalized)) directories.add(normalized) } } } diff --git a/packages/opencode/src/tool/external-directory.ts b/packages/opencode/src/tool/external-directory.ts index 05f83c00d..1d3958fc4 100644 --- a/packages/opencode/src/tool/external-directory.ts +++ b/packages/opencode/src/tool/external-directory.ts @@ -1,6 +1,5 @@ import path from "path" import type { Tool } from "./tool" -import { Filesystem } from "../util/filesystem" import { Instance } from "../project/instance" type Kind = "file" | "directory" @@ -15,7 +14,7 @@ export async function assertExternalDirectory(ctx: Tool.Context, target?: string if (options?.bypass) return - if (Filesystem.contains(Instance.directory, target)) return + if (Instance.containsPath(target)) return const kind = options?.kind ?? "file" const parentDir = kind === "directory" ? target : path.dirname(target) diff --git a/packages/opencode/test/file/path-traversal.test.ts b/packages/opencode/test/file/path-traversal.test.ts index c20c76a2e..44ae8f154 100644 --- a/packages/opencode/test/file/path-traversal.test.ts +++ b/packages/opencode/test/file/path-traversal.test.ts @@ -1,5 +1,6 @@ import { test, expect, describe } from "bun:test" 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" @@ -113,3 +114,85 @@ describe("File.list path traversal protection", () => { }) }) }) + +describe("Instance.containsPath", () => { + test("returns true for path inside directory", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: () => { + expect(Instance.containsPath(path.join(tmp.path, "foo.txt"))).toBe(true) + expect(Instance.containsPath(path.join(tmp.path, "src", "file.ts"))).toBe(true) + }, + }) + }) + + test("returns true for path inside worktree but outside directory (monorepo subdirectory scenario)", async () => { + await using tmp = await tmpdir({ git: true }) + const subdir = path.join(tmp.path, "packages", "lib") + await fs.mkdir(subdir, { recursive: true }) + + await Instance.provide({ + directory: subdir, + fn: () => { + // .opencode at worktree root, but we're running from packages/lib + expect(Instance.containsPath(path.join(tmp.path, ".opencode", "state"))).toBe(true) + // sibling package should also be accessible + expect(Instance.containsPath(path.join(tmp.path, "packages", "other", "file.ts"))).toBe(true) + // worktree root itself + expect(Instance.containsPath(tmp.path)).toBe(true) + }, + }) + }) + + test("returns false for path outside both directory and worktree", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: () => { + expect(Instance.containsPath("/etc/passwd")).toBe(false) + expect(Instance.containsPath("/tmp/other-project")).toBe(false) + }, + }) + }) + + test("returns false for path with .. escaping worktree", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: () => { + expect(Instance.containsPath(path.join(tmp.path, "..", "escape.txt"))).toBe(false) + }, + }) + }) + + test("handles directory === worktree (running from repo root)", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: () => { + expect(Instance.directory).toBe(Instance.worktree) + expect(Instance.containsPath(path.join(tmp.path, "file.txt"))).toBe(true) + expect(Instance.containsPath("/etc/passwd")).toBe(false) + }, + }) + }) + + test("non-git project does not allow arbitrary paths via worktree='/'", async () => { + await using tmp = await tmpdir() // no git: true + + await Instance.provide({ + directory: tmp.path, + fn: () => { + // worktree is "/" for non-git projects, but containsPath should NOT allow all paths + expect(Instance.containsPath(path.join(tmp.path, "file.txt"))).toBe(true) + expect(Instance.containsPath("/etc/passwd")).toBe(false) + expect(Instance.containsPath("/tmp/other")).toBe(false) + }, + }) + }) +})