From 7cb9bb99b5bcdd2cd503a13cb4233ad06ecbf223 Mon Sep 17 00:00:00 2001 From: Yehia Ezzat <74499810+0xSemizzz@users.noreply.github.com> Date: Sun, 2 Aug 2026 14:48:38 +0300 Subject: [PATCH] fix(tool): resolve symlinks before the project containment check `containsPath` is `path.relative` string math, but every caller hands the same unresolved string to an fs call, and fs follows symlinks. A link inside the project pointing outside it passed the check while the read or write landed outside the project, so `external_directory` never fired. The fallback prompt was wrong too, not just absent: `read` and `edit` render `path.relative(worktree, filepath)`, so the user saw the in-project link path while a different file was being touched. Windows already resolved here, via `normalizePath` -> `realpathSync.native`; POSIX passed the target through untouched. Resolve on both so the check, the glob, and the prompt metadata all name the path the filesystem will use. Plain realpath is not sufficient because `write` and `apply_patch` target files that do not exist yet, so walk up to the nearest existing ancestor and re-attach the tail. Errors other than ENOENT keep the lexical path, preserving today's behaviour rather than failing the tool call. --- .../opencode/src/tool/external-directory.ts | 31 +++++++++- .../test/tool/external-directory.test.ts | 60 +++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/packages/opencode/src/tool/external-directory.ts b/packages/opencode/src/tool/external-directory.ts index 4372db82d89e..0f88dbd37601 100644 --- a/packages/opencode/src/tool/external-directory.ts +++ b/packages/opencode/src/tool/external-directory.ts @@ -1,4 +1,5 @@ import path from "path" +import { realpathSync } from "fs" import { Effect } from "effect" import { InstanceState } from "@/effect/instance-state" import type * as Tool from "./tool" @@ -7,6 +8,34 @@ import { FSUtil } from "@opencode-ai/core/fs-util" type Kind = "file" | "directory" +// `containsPath` is pure path math (`path.relative`), but every caller then hands the +// same string to an fs call, and fs follows symlinks. A link inside the project that +// points outside it therefore passed the check while the read/write landed outside -- +// and the permission prompt rendered the in-project path, so it named the wrong file. +// +// Windows already resolved here via `normalizePath` (which calls `realpathSync.native`). +// Do the same on POSIX so both platforms check, and display, the path the filesystem +// will actually touch. +function resolveTarget(target: string) { + return follow(process.platform === "win32" ? FSUtil.normalizePath(target) : path.resolve(target)) +} + +// A plain realpath is not enough: `write` and `apply_patch` legitimately target files +// that do not exist yet, and realpath fails with ENOENT on those. Walk up to the nearest +// existing ancestor and re-attach the tail, so a symlinked parent directory is still +// followed. Any other error (ELOOP, EACCES) keeps the lexical path, which is the +// behaviour that shipped before -- resolution is a guard here, not a hard requirement. +function follow(input: string): string { + try { + return realpathSync(input) + } catch (err) { + if ((err as NodeJS.ErrnoException)?.code !== "ENOENT") return input + const parent = path.dirname(input) + if (parent === input) return input + return path.join(follow(parent), path.basename(input)) + } +} + type Options = { bypass?: boolean kind?: Kind @@ -22,7 +51,7 @@ export const assertExternalDirectoryEffect = Effect.fn("Tool.assertExternalDirec if (options?.bypass) return false const ins = yield* InstanceState.context - const full = process.platform === "win32" ? FSUtil.normalizePath(target) : target + const full = resolveTarget(target) if (containsPath(full, ins)) return false const kind = options?.kind ?? "file" diff --git a/packages/opencode/test/tool/external-directory.test.ts b/packages/opencode/test/tool/external-directory.test.ts index d43accfb70dc..c86bce74901a 100644 --- a/packages/opencode/test/tool/external-directory.test.ts +++ b/packages/opencode/test/tool/external-directory.test.ts @@ -2,6 +2,8 @@ import { PermissionV1 } from "@opencode-ai/core/v1/permission" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { describe, expect } from "bun:test" import path from "path" +import { realpathSync } from "fs" +import { symlink } from "fs/promises" import { Effect } from "effect" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import type { Tool } from "@/tool/tool" @@ -105,6 +107,64 @@ describe("tool.assertExternalDirectory", () => { }), ) + if (process.platform !== "win32") { + it.instance("follows symlinks that escape the project before checking containment", () => + Effect.gen(function* () { + const test = yield* TestInstance + const { requests, ctx } = makeCtx() + + const outside = yield* tmpdirScoped() + const link = path.join(test.directory, "vendor") + yield* Effect.promise(() => symlink(outside, link)) + + // Lexically inside the project, but resolves outside it. + const target = path.join(link, "existing.txt") + yield* Effect.promise(() => Bun.write(target, "x")) + const expected = glob(path.join(realpathSync(outside), "*")) + + yield* assertExternalDirectoryEffect(ctx, target) + + const req = requests.find((r) => r.permission === "external_directory") + expect(req).toBeDefined() + expect(req!.patterns).toEqual([expected]) + expect(req!.metadata.filepath).toBe(path.join(realpathSync(outside), "existing.txt")) + }), + ) + + it.instance("follows symlinks for targets that do not exist yet", () => + Effect.gen(function* () { + const test = yield* TestInstance + const { requests, ctx } = makeCtx() + + const outside = yield* tmpdirScoped() + const link = path.join(test.directory, "vendor") + yield* Effect.promise(() => symlink(outside, link)) + + // `write` and `apply_patch` target files that do not exist yet; realpath + // fails on those, so the nearest existing ancestor has to be resolved. + const target = path.join(link, "brand-new.txt") + const expected = glob(path.join(realpathSync(outside), "*")) + + yield* assertExternalDirectoryEffect(ctx, target) + + const req = requests.find((r) => r.permission === "external_directory") + expect(req).toBeDefined() + expect(req!.patterns).toEqual([expected]) + }), + ) + + it.instance("does not prompt for ordinary paths inside the project", () => + Effect.gen(function* () { + const test = yield* TestInstance + const { requests, ctx } = makeCtx() + + yield* assertExternalDirectoryEffect(ctx, path.join(test.directory, "src", "new-file.ts")) + + expect(requests.length).toBe(0) + }), + ) + } + if (process.platform === "win32") { it.instance( "normalizes Windows path variants to one glob",