From 1ac6b4bec46f876eca69310d9371488e497ae5c6 Mon Sep 17 00:00:00 2001 From: Dax Raad Date: Fri, 26 Jun 2026 13:55:04 -0400 Subject: [PATCH] fix(core): authorize external read paths --- packages/core/src/tool/read.ts | 43 ++++++++++---------- packages/core/test/tool-read.test.ts | 59 +++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 22 deletions(-) diff --git a/packages/core/src/tool/read.ts b/packages/core/src/tool/read.ts index 2635e4653f..ba554cc8fb 100644 --- a/packages/core/src/tool/read.ts +++ b/packages/core/src/tool/read.ts @@ -1,12 +1,10 @@ export * as ReadTool from "./read" import { ToolFailure } from "@opencode-ai/llm" -import path from "path" import { Effect, Layer, Schema } from "effect" import { FileSystem } from "../filesystem" -import { FSUtil } from "../fs-util" import { Image } from "../image" -import { Location } from "../location" +import { LocationMutation } from "../location-mutation" import { PermissionV2 } from "../permission" import { AbsolutePath } from "../schema" import { ReadToolFileSystem } from "./read-filesystem" @@ -30,9 +28,8 @@ const Output = Schema.Union([FileSystem.Content, ReadToolFileSystem.TextPage, Re export const layer = Layer.effectDiscard( Effect.gen(function* () { const tools = yield* Tools.Service - const fs = yield* FSUtil.Service const reader = yield* ReadToolFileSystem.Service - const location = yield* Location.Service + const mutation = yield* LocationMutation.Service const image = yield* Image.Service const permission = yield* PermissionV2.Service @@ -40,7 +37,7 @@ export const layer = Layer.effectDiscard( .register({ [name]: Tool.make({ description: - "Read a text file or supported image, page through a large UTF-8 text file by line offset, or list a directory page. Relative paths resolve from the current location; absolute paths are read directly.", + "Read a text file or supported image, page through a large UTF-8 text file by line offset, or list a directory page. Relative paths resolve from the current location; absolute paths inside it are accepted, while external absolute paths require external_directory approval.", input: Input, output: Output, toModelOutput: ({ input, output }) => { @@ -53,27 +50,33 @@ export const layer = Layer.effectDiscard( }, execute: (input, context) => { return Effect.gen(function* () { - const absolute = path.resolve(location.directory, input.path) - const selected = path.isAbsolute(input.path) ? path.dirname(absolute) : location.directory - if (!path.isAbsolute(input.path) && !FSUtil.contains(location.directory, absolute)) - return yield* Effect.die(new Error("Path escapes the allowed read root")) - const real = yield* fs.realPath(absolute) - const root = yield* fs.realPath(selected) - if (!FSUtil.contains(root, real)) - return yield* Effect.die(new Error("Path escapes the allowed read root")) - const resource = path.relative(root, real).replaceAll("\\", "/") || "." - const target = AbsolutePath.make(real) - const type = yield* reader.inspect(target) + const source = { + type: "tool" as const, + messageID: context.assistantMessageID, + callID: context.toolCallID, + } + const target = yield* mutation.resolve({ path: input.path, kind: "directory" }) + const external = target.externalDirectory + if (external) + yield* permission.assert({ + ...LocationMutation.externalDirectoryPermission(external), + sessionID: context.sessionID, + agent: context.agent, + source, + }) + const resource = target.resource + const absolute = AbsolutePath.make(target.canonical) + const type = yield* reader.inspect(absolute) yield* permission.assert({ action: name, resources: [resource], save: ["*"], sessionID: context.sessionID, agent: context.agent, - source: { type: "tool", messageID: context.assistantMessageID, callID: context.toolCallID }, + source, }) - if (type === "directory") return yield* reader.list(target, { offset: input.offset, limit: input.limit }) - const content = yield* reader.read(target, resource, { + if (type === "directory") return yield* reader.list(absolute, { offset: input.offset, limit: input.limit }) + const content = yield* reader.read(absolute, resource, { offset: input.offset, limit: input.limit, }) diff --git a/packages/core/test/tool-read.test.ts b/packages/core/test/tool-read.test.ts index fcbec061b2..5b5bc2c9ff 100644 --- a/packages/core/test/tool-read.test.ts +++ b/packages/core/test/tool-read.test.ts @@ -11,6 +11,7 @@ import { PermissionV2 } from "@opencode-ai/core/permission" import { SessionV2 } from "@opencode-ai/core/session" import { AbsolutePath } from "@opencode-ai/core/schema" import { Global } from "@opencode-ai/core/global" +import { LocationMutation } from "@opencode-ai/core/location-mutation" import { location } from "./fixture/location" import { ToolRegistry } from "@opencode-ai/core/tool/registry" import { ReadTool } from "@opencode-ai/core/tool/read" @@ -97,6 +98,32 @@ const infrastructure = Layer.mergeAll( Layer.succeed(Location.Service, Location.Service.of(location({ directory: AbsolutePath.make(process.cwd()) }))), Global.layerWith({ data: Global.Path.data }), ) +const mutation = Layer.succeed( + LocationMutation.Service, + LocationMutation.Service.of({ + resolve: (input) => { + if (input.path === missingPath) + return Effect.fail(new LocationMutation.PathError({ path: input.path, reason: "non_directory_ancestor" })) + const canonical = path.resolve(process.cwd(), input.path) + const external = path.isAbsolute(input.path) && !FSUtil.contains(process.cwd(), canonical) + const resource = external ? canonical.replaceAll("\\", "/") : path.relative(process.cwd(), canonical) || "." + const directory = path.dirname(canonical) + const externalResource = path.join(directory, "*").replaceAll("\\", "/") + return Effect.succeed({ + canonical, + resource, + externalDirectory: external + ? { + action: "external_directory" as const, + directory, + resource: externalResource, + save: externalResource, + } + : undefined, + }) + }, + }), +) const unavailableImage = Layer.succeed( Image.Service, Image.Service.of({ normalize: () => Effect.fail(new Image.ResizerUnavailableError()) }), @@ -107,19 +134,21 @@ const read = ReadTool.layer.pipe( Layer.provide(permission), Layer.provide(config), Layer.provide(image), + Layer.provide(mutation), Layer.provide(infrastructure), ) -const it = testEffect(Layer.mergeAll(registry, reader, permission, config, image, infrastructure, read)) +const it = testEffect(Layer.mergeAll(registry, reader, permission, config, image, mutation, infrastructure, read)) const unavailableRead = ReadTool.layer.pipe( Layer.provide(registry), Layer.provide(reader), Layer.provide(permission), Layer.provide(config), Layer.provide(unavailableImage), + Layer.provide(mutation), Layer.provide(infrastructure), ) const itWithoutResizer = testEffect( - Layer.mergeAll(registry, reader, permission, config, unavailableImage, infrastructure, unavailableRead), + Layer.mergeAll(registry, reader, permission, config, unavailableImage, mutation, infrastructure, unavailableRead), ) const sessionID = SessionV2.ID.make("ses_read_tool_test") @@ -174,6 +203,32 @@ describe("ReadTool", () => { }), ) + it.effect("asks for external_directory approval before reading an external absolute path", () => + Effect.gen(function* () { + const registry = yield* ToolRegistry.Service + const external = path.join(path.parse(process.cwd()).root, "external-read", "notes.txt") + + expect( + yield* executeTool(registry, { + sessionID, + ...toolIdentity, + call: { type: "tool-call", id: "call-external-read", name: "read", input: { path: external } }, + }), + ).toMatchObject({ type: "json" }) + expect(assertions).toMatchObject([ + { + sessionID, + action: "external_directory", + resources: [path.join(path.dirname(external), "*").replaceAll("\\", "/")], + }, + { sessionID, action: "read", resources: [external.replaceAll("\\", "/")], save: ["*"] }, + ]) + expect(readCalls).toEqual([ + { input: AbsolutePath.make(external), page: { offset: undefined, limit: undefined } }, + ]) + }), + ) + it.effect("returns a small PNG as native media instead of durable base64 text", () => Effect.gen(function* () { const png = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII="