From 9d61eef14107a667b1a7cce4c69a76d9017f75a0 Mon Sep 17 00:00:00 2001 From: "opencode-agent[bot]" Date: Sun, 7 Jun 2026 13:09:27 +0000 Subject: [PATCH] fix: keep read path validation model-visible --- packages/core/src/filesystem.ts | 19 ++++++++++++++----- packages/core/src/tool/read.ts | 17 ++++++++++++----- .../core/test/location-filesystem.test.ts | 5 +++++ packages/core/test/tool-read.test.ts | 17 +++++++++++++++++ 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/packages/core/src/filesystem.ts b/packages/core/src/filesystem.ts index 3c08c8135f..671aaa0a08 100644 --- a/packages/core/src/filesystem.ts +++ b/packages/core/src/filesystem.ts @@ -35,6 +35,13 @@ export class BinaryFileError extends Error { } } +export class PathValidationError extends Error { + constructor(message: string) { + super(message) + this.name = "PathValidationError" + } +} + const BINARY_EXTENSIONS = new Set([ ".zip", ".tar", @@ -265,21 +272,23 @@ export const layer = Layer.effect( ToolOutputStore.MANAGED_DIRECTORY, ) if (input && path.isAbsolute(input)) { - if (reference) return yield* Effect.die(new Error("Absolute paths cannot use a project reference")) + if (reference) + return yield* Effect.die(new PathValidationError("Absolute paths cannot use a project reference")) if (path.dirname(input) !== managed || !path.basename(input).startsWith("tool_")) - return yield* Effect.die(new Error("Absolute path is not managed tool output")) + return yield* Effect.die(new PathValidationError("Absolute path is not managed tool output")) const real = yield* fs.realPath(input).pipe(Effect.orDie) const managedRoot = yield* fs.realPath(managed).pipe(Effect.orDie) if (path.dirname(real) !== managedRoot || !path.basename(real).startsWith("tool_")) - return yield* Effect.die(new Error("Path escapes managed tool output")) + return yield* Effect.die(new PathValidationError("Path escapes managed tool output")) return { absolute: input, real, directory: managed, root: managedRoot } } const selected = yield* select(reference) const absolute = path.resolve(selected.directory, input ?? ".") if (!FSUtil.contains(selected.directory, absolute)) - return yield* Effect.die(new Error("Path escapes the location")) + return yield* Effect.die(new PathValidationError("Path escapes the location")) const real = yield* fs.realPath(absolute).pipe(Effect.orDie) - if (!FSUtil.contains(selected.root, real)) return yield* Effect.die(new Error("Path escapes the location")) + if (!FSUtil.contains(selected.root, real)) + return yield* Effect.die(new PathValidationError("Path escapes the location")) return { absolute, real, ...selected } }) const entry = Effect.fnUntraced(function* (absolute: string, selected = { directory: location.directory, root }) { diff --git a/packages/core/src/tool/read.ts b/packages/core/src/tool/read.ts index 45045ad8a1..8d62388092 100644 --- a/packages/core/src/tool/read.ts +++ b/packages/core/src/tool/read.ts @@ -68,14 +68,21 @@ export const layer = Layer.effectDiscard( return yield* Effect.fail(new FileSystem.BinaryFileError(resolved.resource)) return content }).pipe( + Effect.catchDefect((defect) => + defect instanceof FileSystem.PathValidationError + ? Effect.fail(new ToolFailure({ message: defect.message })) + : Effect.die(defect), + ), Effect.mapError((error) => { const message = - error instanceof FileSystem.BinaryFileError || - error instanceof FileSystem.MediaIngestLimitError || - error instanceof Image.DecodeError || - error instanceof Image.SizeError + error instanceof ToolFailure ? error.message - : `Unable to read ${input.path}` + : error instanceof FileSystem.BinaryFileError || + error instanceof FileSystem.MediaIngestLimitError || + error instanceof Image.DecodeError || + error instanceof Image.SizeError + ? error.message + : `Unable to read ${input.path}` return new ToolFailure({ message }) }), ) diff --git a/packages/core/test/location-filesystem.test.ts b/packages/core/test/location-filesystem.test.ts index 56d2fa3dfb..bc8ae959f8 100644 --- a/packages/core/test/location-filesystem.test.ts +++ b/packages/core/test/location-filesystem.test.ts @@ -67,6 +67,11 @@ describe("FileSystem", () => { expect(yield* service.read({ path: output })).toMatchObject({ type: "text", content: "failure here" }) expect((yield* service.resolveRoot({ path: output })).real).toBe(output) + expect( + yield* service + .resolveReadPath({ path: unrelated }) + .pipe(Effect.catchDefect((defect) => Effect.succeed(defect))), + ).toBeInstanceOf(FileSystem.PathValidationError) expect(yield* Effect.exit(service.read({ path: unrelated }))).toMatchObject({ _tag: "Failure" }) expect(yield* Effect.exit(service.read({ path: managed }))).toMatchObject({ _tag: "Failure" }) }).pipe(provide(worktree, inertReferences, FSUtil.defaultLayer, data)) diff --git a/packages/core/test/tool-read.test.ts b/packages/core/test/tool-read.test.ts index fa317a00e5..18f156085b 100644 --- a/packages/core/test/tool-read.test.ts +++ b/packages/core/test/tool-read.test.ts @@ -490,6 +490,23 @@ describe("ReadTool", () => { }), ) + it.effect("returns path validation failures as model-visible tool output", () => + Effect.gen(function* () { + const registry = yield* ToolRegistry.Service + + resolveFailure = new FileSystem.PathValidationError("Absolute path is not managed tool output") + expect( + yield* executeTool(registry, { + sessionID, + ...toolIdentity, + call: { type: "tool-call", id: "call-unmanaged", name: "read", input: { path: "/tmp/private.txt" } }, + }), + ).toEqual({ type: "error", value: "Absolute path is not managed tool output" }) + + expect(readCalls).toEqual([]) + }), + ) + it.effect("forwards pagination and returns bounded text pages with continuation", () => Effect.gen(function* () { readResult = new FileSystem.TextPage({