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",