From 1511886c0cedbdcad64ab815fe242543ec855ee0 Mon Sep 17 00:00:00 2001 From: Bram van der Horn Date: Thu, 12 Feb 2026 12:12:40 +0100 Subject: [PATCH] fix: use fs.realpath instead of manual path.resolve for symlink resolution resolveSymlink and resolveSymlinkAsync incorrectly resolved relative symlinks by using path.resolve(filePath, '..', linkTarget). This fails when symlinks use multi-level relative paths (e.g. ../../skills/...) or when symlinks are chained (symlink pointing to a directory containing more symlinks). Replace with fs.realpathSync/fs.realpath which delegates to the OS for correct resolution of all symlink types: relative, absolute, chained, and nested. Fixes #1738 AI-assisted-by: claude-opus-4.6 via opencode AI-contribution: partial AI-session: 20260212-120629-4gTXvDGV --- src/shared/file-utils.test.ts | 103 ++++++++++++++++++++++++++++++++++ src/shared/file-utils.ts | 16 +----- 2 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 src/shared/file-utils.test.ts diff --git a/src/shared/file-utils.test.ts b/src/shared/file-utils.test.ts new file mode 100644 index 00000000..82d16dab --- /dev/null +++ b/src/shared/file-utils.test.ts @@ -0,0 +1,103 @@ +import { describe, it, expect, beforeAll, afterAll } from "bun:test" +import { mkdirSync, writeFileSync, symlinkSync, rmSync } from "fs" +import { join } from "path" +import { tmpdir } from "os" +import { resolveSymlink, resolveSymlinkAsync, isSymbolicLink } from "./file-utils" + +const testDir = join(tmpdir(), "file-utils-test-" + Date.now()) + +// Create a directory structure that mimics the real-world scenario: +// +// testDir/ +// ├── repo/ +// │ ├── skills/ +// │ │ └── category/ +// │ │ └── my-skill/ +// │ │ └── SKILL.md +// │ └── .opencode/ +// │ └── skills/ +// │ └── my-skill -> ../../skills/category/my-skill (relative symlink) +// └── config/ +// └── skills -> ../repo/.opencode/skills (absolute symlink) + +const realSkillDir = join(testDir, "repo", "skills", "category", "my-skill") +const repoOpencodeSkills = join(testDir, "repo", ".opencode", "skills") +const configSkills = join(testDir, "config", "skills") + +beforeAll(() => { + // Create real skill directory with a file + mkdirSync(realSkillDir, { recursive: true }) + writeFileSync(join(realSkillDir, "SKILL.md"), "# My Skill") + + // Create .opencode/skills/ with a relative symlink to the real skill + mkdirSync(repoOpencodeSkills, { recursive: true }) + symlinkSync("../../skills/category/my-skill", join(repoOpencodeSkills, "my-skill")) + + // Create config/skills as an absolute symlink to .opencode/skills + mkdirSync(join(testDir, "config"), { recursive: true }) + symlinkSync(repoOpencodeSkills, configSkills) +}) + +afterAll(() => { + rmSync(testDir, { recursive: true, force: true }) +}) + +describe("resolveSymlink", () => { + it("resolves a regular file path to itself", () => { + const filePath = join(realSkillDir, "SKILL.md") + expect(resolveSymlink(filePath)).toBe(filePath) + }) + + it("resolves a relative symlink to its real path", () => { + const symlinkPath = join(repoOpencodeSkills, "my-skill") + expect(resolveSymlink(symlinkPath)).toBe(realSkillDir) + }) + + it("resolves a chained symlink (symlink-to-dir-containing-symlinks) to the real path", () => { + // This is the real-world scenario: + // config/skills/my-skill -> (follows config/skills) -> repo/.opencode/skills/my-skill -> repo/skills/category/my-skill + const chainedPath = join(configSkills, "my-skill") + expect(resolveSymlink(chainedPath)).toBe(realSkillDir) + }) + + it("returns the original path for non-existent paths", () => { + const fakePath = join(testDir, "does-not-exist") + expect(resolveSymlink(fakePath)).toBe(fakePath) + }) +}) + +describe("resolveSymlinkAsync", () => { + it("resolves a regular file path to itself", async () => { + const filePath = join(realSkillDir, "SKILL.md") + expect(await resolveSymlinkAsync(filePath)).toBe(filePath) + }) + + it("resolves a relative symlink to its real path", async () => { + const symlinkPath = join(repoOpencodeSkills, "my-skill") + expect(await resolveSymlinkAsync(symlinkPath)).toBe(realSkillDir) + }) + + it("resolves a chained symlink (symlink-to-dir-containing-symlinks) to the real path", async () => { + const chainedPath = join(configSkills, "my-skill") + expect(await resolveSymlinkAsync(chainedPath)).toBe(realSkillDir) + }) + + it("returns the original path for non-existent paths", async () => { + const fakePath = join(testDir, "does-not-exist") + expect(await resolveSymlinkAsync(fakePath)).toBe(fakePath) + }) +}) + +describe("isSymbolicLink", () => { + it("returns true for a symlink", () => { + expect(isSymbolicLink(join(repoOpencodeSkills, "my-skill"))).toBe(true) + }) + + it("returns false for a regular directory", () => { + expect(isSymbolicLink(realSkillDir)).toBe(false) + }) + + it("returns false for a non-existent path", () => { + expect(isSymbolicLink(join(testDir, "does-not-exist"))).toBe(false) + }) +}) diff --git a/src/shared/file-utils.ts b/src/shared/file-utils.ts index cfeda816..47d6c9f6 100644 --- a/src/shared/file-utils.ts +++ b/src/shared/file-utils.ts @@ -1,6 +1,5 @@ -import { lstatSync, readlinkSync } from "fs" +import { lstatSync, realpathSync } from "fs" import { promises as fs } from "fs" -import { resolve } from "path" export function isMarkdownFile(entry: { name: string; isFile: () => boolean }): boolean { return !entry.name.startsWith(".") && entry.name.endsWith(".md") && entry.isFile() @@ -16,11 +15,7 @@ export function isSymbolicLink(filePath: string): boolean { export function resolveSymlink(filePath: string): string { try { - const stats = lstatSync(filePath, { throwIfNoEntry: false }) - if (stats?.isSymbolicLink()) { - return resolve(filePath, "..", readlinkSync(filePath)) - } - return filePath + return realpathSync(filePath) } catch { return filePath } @@ -28,12 +23,7 @@ export function resolveSymlink(filePath: string): string { export async function resolveSymlinkAsync(filePath: string): Promise { try { - const stats = await fs.lstat(filePath) - if (stats.isSymbolicLink()) { - const linkTarget = await fs.readlink(filePath) - return resolve(filePath, "..", linkTarget) - } - return filePath + return await fs.realpath(filePath) } catch { return filePath }