diff --git a/packages/core/src/location-mutation.ts b/packages/core/src/location-mutation.ts index 3c15d9a2a4..0de177aafe 100644 --- a/packages/core/src/location-mutation.ts +++ b/packages/core/src/location-mutation.ts @@ -25,7 +25,7 @@ export type ResolveInput = typeof ResolveInput.Type export class PathError extends Schema.TaggedErrorClass()("LocationMutation.PathError", { path: Schema.String, - reason: Schema.Literals(["location_escape", "non_directory_ancestor"]), + reason: Schema.Literal("non_directory_ancestor"), }) {} export interface ExternalDirectoryAuthorization { @@ -83,7 +83,6 @@ const layer = Layer.effect( Effect.gen(function* () { const fs = yield* FSUtil.Service const location = yield* Location.Service - const locationRoot = yield* fs.realPath(location.directory) function notFound(effect: Effect.Effect) { return effect.pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined))) @@ -124,14 +123,8 @@ const layer = Layer.effect( const lexicallyInternal = FSUtil.contains(location.directory, absolute) const resolved = yield* resolvePath(absolute) - if (lexicallyInternal && !FSUtil.contains(locationRoot, resolved.canonical)) { - return yield* new PathError({ path: input.path, reason: "location_escape" }) - } - const external = !lexicallyInternal - const resource = external - ? slash(resolved.canonical) - : slash(path.relative(locationRoot, resolved.canonical) || ".") + const resource = external ? slash(resolved.canonical) : slash(path.relative(location.directory, absolute) || ".") const externalDirectory = input.kind === "directory" && resolved.type === "Directory" ? resolved.canonical : resolved.directory const externalResource = slash(path.join(externalDirectory, "*")) diff --git a/packages/core/test/location-mutation.test.ts b/packages/core/test/location-mutation.test.ts index 35e21905d2..ba52af866e 100644 --- a/packages/core/test/location-mutation.test.ts +++ b/packages/core/test/location-mutation.test.ts @@ -77,7 +77,7 @@ describe("LocationMutation", () => { ), ) - it.live("rejects a prospective target below an escaping symlink ancestor", () => + it.live("authorizes a prospective target below an external symlink by its in-location path", () => withTmp((directory) => { const outside = `${directory}-outside` return Effect.gen(function* () { @@ -86,10 +86,12 @@ describe("LocationMutation", () => { await fs.mkdir(outside) await fs.symlink(outside, path.join(directory, "escape")) }) - const error = yield* Effect.flip( - (yield* LocationMutation.Service).resolve({ path: path.join("escape", "new.txt") }), - ) - expect(error).toMatchObject({ _tag: "LocationMutation.PathError", reason: "location_escape" }) + const target = yield* (yield* LocationMutation.Service).resolve({ path: path.join("escape", "new.txt") }) + expect(target).toMatchObject({ + canonical: path.join(yield* Effect.promise(() => fs.realpath(outside)), "new.txt"), + resource: "escape/new.txt", + }) + expect(target.externalDirectory).toBeUndefined() yield* Effect.promise(() => fs.rm(outside, { recursive: true, force: true })) }).pipe(provide(directory)) }), @@ -106,7 +108,7 @@ describe("LocationMutation", () => { expect(yield* (yield* LocationMutation.Service).resolve({ path: "linked/new.txt" })).toMatchObject({ canonical: path.join(yield* Effect.promise(() => fs.realpath(directory)), "actual", "new.txt"), - resource: "actual/new.txt", + resource: "linked/new.txt", }) }).pipe(provide(directory)), ), diff --git a/packages/core/test/tool-edit.test.ts b/packages/core/test/tool-edit.test.ts index 4beb47f849..1e1d806d93 100644 --- a/packages/core/test/tool-edit.test.ts +++ b/packages/core/test/tool-edit.test.ts @@ -199,6 +199,41 @@ describe("EditTool", () => { ), ) + it.live("edits an external symlink target with only its in-location permission", () => + Effect.acquireUseRelease( + Effect.promise(() => Promise.all([tmpdir(), tmpdir()])), + ([active, outside]) => { + reset() + if (process.platform === "win32") return Effect.void + const target = path.join(outside.path, "external.txt") + const link = path.join(active.path, "link.txt") + return Effect.promise(async () => { + await fs.writeFile(target, "before") + await fs.symlink(target, link) + }).pipe( + Effect.andThen( + withTool(active.path, (registry) => + executeTool(registry, call({ path: "link.txt", oldString: "before", newString: "after" })), + ), + ), + Effect.andThen((result) => + Effect.sync(() => { + expect(result.type).toBe("text") + expect(assertions.map((input) => input.action)).toEqual(["edit"]) + expect(assertions[0]?.resources).toEqual(["link.txt"]) + }), + ), + Effect.andThen(Effect.promise(() => fs.readFile(target, "utf8"))), + Effect.tap((content) => Effect.sync(() => expect(content).toBe("after"))), + ) + }, + ([active, outside]) => + Effect.promise(() => + Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined), + ), + ), + ) + it.live("approves an explicit external absolute path before edit", () => Effect.acquireUseRelease( Effect.promise(() => Promise.all([tmpdir(), tmpdir()])), diff --git a/packages/core/test/tool-write.test.ts b/packages/core/test/tool-write.test.ts index 11698f9224..6e4d61c57c 100644 --- a/packages/core/test/tool-write.test.ts +++ b/packages/core/test/tool-write.test.ts @@ -220,6 +220,39 @@ describe("WriteTool", () => { ), ) + it.live("writes an external symlink target with only its in-location permission", () => + Effect.acquireUseRelease( + Effect.promise(() => Promise.all([tmpdir(), tmpdir()])), + ([active, outside]) => { + reset() + if (process.platform === "win32") return Effect.void + const target = path.join(outside.path, "external.txt") + const link = path.join(active.path, "link.txt") + return Effect.promise(async () => { + await fs.writeFile(target, "before") + await fs.symlink(target, link) + }).pipe( + Effect.andThen( + withTool(active.path, (registry) => executeTool(registry, call({ path: "link.txt", content: "after" }))), + ), + Effect.andThen((result) => + Effect.sync(() => { + expect(result.type).toBe("text") + expect(assertions.map((input) => input.action)).toEqual(["edit"]) + expect(assertions[0]?.resources).toEqual(["link.txt"]) + }), + ), + Effect.andThen(Effect.promise(() => fs.readFile(target, "utf8"))), + Effect.tap((content) => Effect.sync(() => expect(content).toBe("after"))), + ) + }, + ([active, outside]) => + Effect.promise(() => + Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined), + ), + ), + ) + it.live("approves an explicit external absolute path before edit", () => Effect.acquireUseRelease( Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),