fix(core): authorize symlinks by lexical path (#38180)

Co-authored-by: Aiden Cline <rekram1-node@users.noreply.github.com>

packages/core/src/location-mutation.ts

@@ -25,7 +25,7 @@ export type ResolveInput = typeof ResolveInput.Type
 
 export class PathError extends Schema.TaggedErrorClass<PathError>()("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<A>(effect: Effect.Effect<A, FSUtil.Error>) {
       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
+      const resource = external ? slash(resolved.canonical) : slash(path.relative(location.directory, absolute) || ".")
-        ? slash(resolved.canonical)
-        : slash(path.relative(locationRoot, resolved.canonical) || ".")
       const externalDirectory =
         input.kind === "directory" && resolved.type === "Directory" ? resolved.canonical : resolved.directory
       const externalResource = slash(path.join(externalDirectory, "*"))

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(
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: path.join("escape", "new.txt") })
-          (yield* LocationMutation.Service).resolve({ path: path.join("escape", "new.txt") }),
+        expect(target).toMatchObject({
-        )
+          canonical: path.join(yield* Effect.promise(() => fs.realpath(outside)), "new.txt"),
-        expect(error).toMatchObject({ _tag: "LocationMutation.PathError", reason: "location_escape" })
+          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)),
     ),

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()])),

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()])),