2
0
Эх сурвалжийг харах

fix: resolve symlinks in Instance cache to prevent duplicate contexts (#16651)

Co-authored-by: LukeParkerDev <[email protected]>
John Mylchreest 1 сар өмнө
parent
commit
f1c3a44190

+ 9 - 1
packages/opencode/src/util/filesystem.ts

@@ -114,8 +114,16 @@ export namespace Filesystem {
   }
 
   // We cannot rely on path.resolve() here because git.exe may come from Git Bash, Cygwin, or MSYS2, so we need to translate these paths at the boundary.
+  // Also resolves symlinks so that callers using the result as a cache key
+  // always get the same canonical path for a given physical directory.
   export function resolve(p: string): string {
-    return normalizePath(pathResolve(windowsPath(p)))
+    const resolved = pathResolve(windowsPath(p))
+    try {
+      return normalizePath(realpathSync(resolved))
+    } catch (e) {
+      if (isEnoent(e)) return normalizePath(resolved)
+      throw e
+    }
   }
 
   export function windowsPath(p: string): string {

+ 52 - 0
packages/opencode/test/util/filesystem.test.ts

@@ -502,5 +502,57 @@ describe("filesystem", () => {
       const drive = tmp.path[0].toLowerCase()
       expect(Filesystem.resolve(`/mnt/${drive}`)).toBe(Filesystem.resolve(`${drive.toUpperCase()}:/`))
     })
+
+    test("resolves symlinked directory to canonical path", async () => {
+      await using tmp = await tmpdir()
+      const target = path.join(tmp.path, "real")
+      await fs.mkdir(target)
+      const link = path.join(tmp.path, "link")
+      await fs.symlink(target, link)
+      expect(Filesystem.resolve(link)).toBe(Filesystem.resolve(target))
+    })
+
+    test("returns unresolved path when target does not exist", async () => {
+      await using tmp = await tmpdir()
+      const missing = path.join(tmp.path, "does-not-exist-" + Date.now())
+      const result = Filesystem.resolve(missing)
+      expect(result).toBe(Filesystem.normalizePath(path.resolve(missing)))
+    })
+
+    test("throws ELOOP on symlink cycle", async () => {
+      await using tmp = await tmpdir()
+      const a = path.join(tmp.path, "a")
+      const b = path.join(tmp.path, "b")
+      await fs.symlink(b, a)
+      await fs.symlink(a, b)
+      expect(() => Filesystem.resolve(a)).toThrow()
+    })
+
+    // Windows: chmod(0o000) is a no-op, so EACCES cannot be triggered
+    test("throws EACCES on permission-denied symlink target", async () => {
+      if (process.platform === "win32") return
+      if (process.getuid?.() === 0) return // skip when running as root
+      await using tmp = await tmpdir()
+      const dir = path.join(tmp.path, "restricted")
+      await fs.mkdir(dir)
+      const link = path.join(tmp.path, "link")
+      await fs.symlink(dir, link)
+      await fs.chmod(dir, 0o000)
+      try {
+        expect(() => Filesystem.resolve(path.join(link, "child"))).toThrow()
+      } finally {
+        await fs.chmod(dir, 0o755)
+      }
+    })
+
+    // Windows: traversing through a file throws ENOENT (not ENOTDIR),
+    // which resolve() catches as a fallback instead of rethrowing
+    test("rethrows non-ENOENT errors", async () => {
+      if (process.platform === "win32") return
+      await using tmp = await tmpdir()
+      const file = path.join(tmp.path, "not-a-directory")
+      await fs.writeFile(file, "x")
+      expect(() => Filesystem.resolve(path.join(file, "child"))).toThrow()
+    })
   })
 })