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
This commit is contained in:
parent
283c7e6cb7
commit
1511886c0c
103
src/shared/file-utils.test.ts
Normal file
103
src/shared/file-utils.test.ts
Normal file
@ -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)
|
||||||
|
})
|
||||||
|
})
|
||||||
@ -1,6 +1,5 @@
|
|||||||
import { lstatSync, readlinkSync } from "fs"
|
import { lstatSync, realpathSync } from "fs"
|
||||||
import { promises as fs } from "fs"
|
import { promises as fs } from "fs"
|
||||||
import { resolve } from "path"
|
|
||||||
|
|
||||||
export function isMarkdownFile(entry: { name: string; isFile: () => boolean }): boolean {
|
export function isMarkdownFile(entry: { name: string; isFile: () => boolean }): boolean {
|
||||||
return !entry.name.startsWith(".") && entry.name.endsWith(".md") && entry.isFile()
|
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 {
|
export function resolveSymlink(filePath: string): string {
|
||||||
try {
|
try {
|
||||||
const stats = lstatSync(filePath, { throwIfNoEntry: false })
|
return realpathSync(filePath)
|
||||||
if (stats?.isSymbolicLink()) {
|
|
||||||
return resolve(filePath, "..", readlinkSync(filePath))
|
|
||||||
}
|
|
||||||
return filePath
|
|
||||||
} catch {
|
} catch {
|
||||||
return filePath
|
return filePath
|
||||||
}
|
}
|
||||||
@ -28,12 +23,7 @@ export function resolveSymlink(filePath: string): string {
|
|||||||
|
|
||||||
export async function resolveSymlinkAsync(filePath: string): Promise<string> {
|
export async function resolveSymlinkAsync(filePath: string): Promise<string> {
|
||||||
try {
|
try {
|
||||||
const stats = await fs.lstat(filePath)
|
return await fs.realpath(filePath)
|
||||||
if (stats.isSymbolicLink()) {
|
|
||||||
const linkTarget = await fs.readlink(filePath)
|
|
||||||
return resolve(filePath, "..", linkTarget)
|
|
||||||
}
|
|
||||||
return filePath
|
|
||||||
} catch {
|
} catch {
|
||||||
return filePath
|
return filePath
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user