fix(core): authorize patch symlink entries

packages/core/src/tool/plugin/patch.ts

@@ -10,7 +10,7 @@ import path from "path"
 import { Bom } from "@opencode-ai/util/bom"
 import { FSUtil } from "@opencode-ai/util/fs-util"
 import { Formatter } from "../../formatter"
-import { Location } from "../../location"
+import { LocationMutation } from "../../location-mutation"
 import { Patch } from "@opencode-ai/util/patch"
 import { Permission } from "../../permission"
 import DESCRIPTION from "../patch.txt"
@@ -54,16 +54,11 @@ type Prepared =
       readonly content: string
       readonly before: string
       readonly after: string
-      readonly moveTarget?: Target
+      readonly moveTarget?: LocationMutation.Target
     })
 
-interface Target {
-  readonly canonical: string
-  readonly resource: string
-  readonly externalDirectory?: {
-    readonly directory: string
-    readonly resource: string
-  }
+interface Target extends LocationMutation.Target {
+  readonly entry: string
 }
 
 export const Plugin = {
@@ -71,7 +66,7 @@ export const Plugin = {
   effect: Effect.fn("PatchTool.Plugin")(function* (ctx: PluginContext) {
     const fs = yield* FSUtil.Service
     const formatter = yield* Formatter.Service
-    const location = yield* Location.Service
+    const mutation = yield* LocationMutation.Service
     const permission = yield* Permission.Service
 
     yield* ctx.tool
@@ -107,26 +102,46 @@ export const Plugin = {
                     return yield* new ToolFailure({ message: "patch rejected: empty patch" })
                   }
                   const prepared: Prepared[] = []
-                  const targets: Target[] = []
+                  const resources: string[] = []
                   const updates = new Map<string, string>()
                   for (const hunk of hunks) {
                     yield* Effect.gen(function* () {
-                      const target = resolveTarget(location, hunk.path)
-                      targets.push(target)
+                      const resolved = yield* mutation.resolve({ path: hunk.path, kind: "file" })
+                      const removesSource =
+                        hunk.type === "delete" || (hunk.type === "update" && hunk.movePath !== undefined)
+                      const entryDirectory = removesSource
+                        ? yield* mutation.resolve({ path: path.dirname(hunk.path), kind: "directory" })
+                        : undefined
+                      const target = {
+                        ...resolved,
+                        entry: entryDirectory
+                          ? path.join(entryDirectory.canonical, path.basename(hunk.path))
+                          : resolved.canonical,
+                      } satisfies Target
+                      resources.push(target.resource)
                       if (target.externalDirectory) {
                         yield* permission.assert({
-                          action: "external_directory",
-                          resources: [target.externalDirectory.resource],
-                          save: [target.externalDirectory.resource],
-                          metadata: {
-                            filepath: target.canonical,
-                            parentDir: target.externalDirectory.directory,
-                          },
+                          ...LocationMutation.externalDirectoryPermission(target.externalDirectory),
                           sessionID: context.sessionID,
                           agent: context.agent,
                           source,
                         })
                       }
+                      if (
+                        entryDirectory?.externalDirectory &&
+                        entryDirectory.externalDirectory.resource !== target.externalDirectory?.resource
+                      ) {
+                        yield* permission.assert({
+                          ...LocationMutation.externalDirectoryPermission(entryDirectory.externalDirectory),
+                          sessionID: context.sessionID,
+                          agent: context.agent,
+                          source,
+                        })
+                      }
+                      if (entryDirectory?.externalDirectory) {
+                        const entryResource = target.entry.replaceAll("\\", "/")
+                        if (entryResource !== target.resource) resources.push(entryResource)
+                      }
                       if (hunk.type === "add") {
                         prepared.push({
                           ...hunk,
@@ -185,17 +200,13 @@ export const Plugin = {
                         catch: (error) =>
                           new ToolFailure({ message: `patch verification failed: ${errorMessage(error)}` }),
                       })
-                      const moveTarget = hunk.movePath ? resolveTarget(location, hunk.movePath) : undefined
-                      if (moveTarget) targets.push(moveTarget)
+                      const moveTarget = hunk.movePath
+                        ? yield* mutation.resolve({ path: hunk.movePath, kind: "file" })
+                        : undefined
+                      if (moveTarget) resources.push(moveTarget.resource)
                       if (moveTarget?.externalDirectory) {
                         yield* permission.assert({
-                          action: "external_directory",
-                          resources: [moveTarget.externalDirectory.resource],
-                          save: [moveTarget.externalDirectory.resource],
-                          metadata: {
-                            filepath: moveTarget.canonical,
-                            parentDir: moveTarget.externalDirectory.directory,
-                          },
+                          ...LocationMutation.externalDirectoryPermission(moveTarget.externalDirectory),
                           sessionID: context.sessionID,
                           agent: context.agent,
                           source,
@@ -222,10 +233,10 @@ export const Plugin = {
                   const patchFiles = prepared.map((change) => patchFile(change))
                   yield* permission.assert({
                     action: "edit",
-                    resources: [...new Set(targets.map((target) => target.resource))],
+                    resources: [...new Set(resources)],
                     save: ["*"],
                     metadata: {
-                      filepath: targets.map((target) => target.resource).join(", "),
+                      filepath: resources.join(", "),
                       diff: patchFiles.map((file) => `${file.patch}\n`).join(""),
                       files: patchFiles,
                     },
@@ -258,14 +269,14 @@ export const Plugin = {
                         }
                         if (change.type === "delete") {
                           yield* fs
-                            .remove(change.target.canonical)
+                            .remove(change.target.entry)
                             .pipe(
                               Effect.mapError((error) => fail(`Failed to delete ${change.target.resource}`, error)),
                             )
                           applied.push({
                             type: change.type,
                             resource: change.target.resource,
-                            target: change.target.canonical,
+                            target: change.target.entry,
                           })
                           return
                         }
@@ -274,7 +285,7 @@ export const Plugin = {
                           yield* fs
                             .writeWithDirs(moveTarget.canonical, change.content)
                             .pipe(Effect.mapError((error) => fail(`Failed to write ${moveTarget.resource}`, error)))
-                          yield* fs.remove(change.target.canonical).pipe(
+                          yield* fs.remove(change.target.entry).pipe(
                             Effect.mapError((error) =>
                               fail(`Wrote ${moveTarget.resource} but failed to remove ${change.target.resource}`, error),
                             ),
@@ -414,24 +425,3 @@ function trimDiff(diff: string) {
     })
     .join("\n")
 }
-
-function resolveTarget(location: Location.Interface, value: string): Target {
-  const canonical =
-    process.platform === "win32"
-      ? FSUtil.normalizePath(path.resolve(location.directory, value))
-      : path.resolve(location.directory, value)
-  const projectRoot = path.parse(location.project.directory).root
-  const external =
-    !FSUtil.contains(location.directory, canonical) &&
-    (location.project.directory === projectRoot || !FSUtil.contains(location.project.directory, canonical))
-  const directory = path.dirname(canonical)
-  const resource =
-    process.platform === "win32"
-      ? FSUtil.normalizePathPattern(path.join(directory, "*"))
-      : path.join(directory, "*").replaceAll("\\", "/")
-  return {
-    canonical,
-    resource: path.relative(location.project.directory, canonical).replaceAll("\\", "/") || ".",
-    externalDirectory: external ? { directory, resource } : undefined,
-  }
-}

packages/core/test/tool-patch.test.ts

@@ -8,6 +8,7 @@ import { LayerNode } from "@opencode-ai/util/effect/layer-node"
 import { FSUtil } from "@opencode-ai/util/fs-util"
 import { Formatter } from "@opencode-ai/core/formatter"
 import { Location } from "@opencode-ai/core/location"
+import { LocationMutation } from "@opencode-ai/core/location-mutation"
 import { Permission } from "@opencode-ai/core/permission"
 import { AbsolutePath } from "@opencode-ai/core/schema"
 import { Session } from "@opencode-ai/core/session"
@@ -22,7 +23,7 @@ import { toolIdentity, executeTool, registerToolPlugin, toolDefinitions } from "
 const patchToolNode = makeLocationNode({
   name: "test/patch-tool-plugin",
   layer: Layer.effectDiscard(registerToolPlugin(PatchTool.Plugin)),
-  deps: [Tool.node, Formatter.node, FSUtil.node, Location.node, Permission.node],
+  deps: [Tool.node, Formatter.node, FSUtil.node, LocationMutation.node, Permission.node],
 })
 
 const sessionID = Session.ID.make("ses_patch_tool_test")
@@ -403,6 +404,72 @@ describe("PatchTool", () => {
     ),
   )
 
+  it.live("authorizes external symlink entries when deleting and moving", () =>
+    Effect.acquireUseRelease(
+      Effect.promise(() => tmpdir()),
+      (tmp) => {
+        reset()
+        if (process.platform === "win32") return Effect.void
+        const active = path.join(tmp.path, "active")
+        const outside = path.join(tmp.path, "outside")
+        const other = path.join(tmp.path, "other")
+        const target = path.join(other, "target.txt")
+        const link = path.join(outside, "link.txt")
+        const moved = path.join(active, "moved.txt")
+        return Effect.promise(async () => {
+          await Promise.all([fs.mkdir(active), fs.mkdir(outside), fs.mkdir(other)])
+          await fs.writeFile(target, "before\n")
+          await fs.symlink(target, link)
+        }).pipe(
+          Effect.andThen(
+            withTool(active, (registry) =>
+              Effect.gen(function* () {
+                expect(
+                  yield* executeTool(registry, call(`*** Begin Patch\n*** Delete File: ${link}\n*** End Patch`)),
+                ).toMatchObject({ status: "completed" })
+                const targetRoot = yield* Effect.promise(() => fs.realpath(other))
+                const entryRoot = yield* Effect.promise(() => fs.realpath(outside))
+                expect(assertions).toMatchObject([
+                  { action: "external_directory", resources: [path.join(targetRoot, "*")] },
+                  { action: "external_directory", resources: [path.join(entryRoot, "*")] },
+                  {
+                    action: "edit",
+                    resources: [path.join(targetRoot, "target.txt"), path.join(entryRoot, "link.txt")],
+                  },
+                ])
+                expect(yield* exists(link)).toBe(false)
+                expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("before\n")
+
+                reset()
+                yield* Effect.promise(() => fs.symlink(target, link))
+                expect(
+                  yield* executeTool(
+                    registry,
+                    call(
+                      `*** Begin Patch\n*** Update File: ${link}\n*** Move to: moved.txt\n@@\n-before\n+after\n*** End Patch`,
+                    ),
+                  ),
+                ).toMatchObject({ status: "completed" })
+                expect(assertions).toMatchObject([
+                  { action: "external_directory", resources: [path.join(targetRoot, "*")] },
+                  { action: "external_directory", resources: [path.join(entryRoot, "*")] },
+                  {
+                    action: "edit",
+                    resources: [path.join(targetRoot, "target.txt"), path.join(entryRoot, "link.txt"), "moved.txt"],
+                  },
+                ])
+                expect(yield* exists(link)).toBe(false)
+                expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("before\n")
+                expect(yield* Effect.promise(() => fs.readFile(moved, "utf8"))).toBe("after\n")
+              }),
+            ),
+          ),
+        )
+      },
+      (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
+    ),
+  )
+
   it.live("includes move file info in output and metadata", () =>
     withTempTool((directory, registry) =>
       Effect.gen(function* () {
@@ -828,7 +895,7 @@ describe("PatchTool", () => {
     ),
   )
 
-  it.live("treats a sibling path inside the project worktree as internal", () =>
+  it.live("requires external permission for a sibling path inside the project worktree", () =>
     Effect.acquireUseRelease(
       Effect.promise(() => tmpdir()),
       (tmp) => {
@@ -847,7 +914,17 @@ describe("PatchTool", () => {
                       call("*** Begin Patch\n*** Update File: ../sibling.txt\n@@\n-before\n+after\n*** End Patch"),
                     ),
                   ).toMatchObject({ status: "completed" })
-                  expect(assertions.map((input) => input.action)).toEqual(["edit"])
+                  const root = yield* Effect.promise(() => fs.realpath(tmp.path))
+                  expect(assertions).toMatchObject([
+                    {
+                      action: "external_directory",
+                      resources: [path.join(root, "*").replaceAll("\\", "/")],
+                    },
+                    {
+                      action: "edit",
+                      resources: [path.join(root, "sibling.txt").replaceAll("\\", "/")],
+                    },
+                  ])
                   expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
                 }),
               tmp.path,