From e24b80b15e468267ee5049f88aaebbbd29196ab6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 12 Apr 2026 10:53:23 -0700 Subject: [PATCH] fix: clarify escaped skill path warnings --- .../skills.loadworkspaceskillentries.test.ts | 84 ++++++++++++++++++- src/agents/skills/workspace.ts | 78 +++++++++++++++-- 2 files changed, 150 insertions(+), 12 deletions(-) diff --git a/src/agents/skills.loadworkspaceskillentries.test.ts b/src/agents/skills.loadworkspaceskillentries.test.ts index e3452fb46c2..1447eb26e0b 100644 --- a/src/agents/skills.loadworkspaceskillentries.test.ts +++ b/src/agents/skills.loadworkspaceskillentries.test.ts @@ -323,9 +323,8 @@ describe("loadWorkspaceSkillEntries", () => { expect(entries.map((entry) => entry.skill.name)).not.toContain("outside-skill"); const [line] = warn.mock.calls[0] ?? []; const warningLine = String(line); - expect(warningLine).toContain( - "Skipping skill path that resolves outside its configured root:", - ); + expect(warningLine).toContain("Skipping escaped skill path outside its configured root:"); + expect(warningLine).toContain("reason=symlink-escape"); expect(warningLine).toContain("source=openclaw-workspace"); expect(warningLine).toContain(`root=${path.join(workspaceDir, "skills")}`); expect(warningLine).toContain(`requested=${requestedPath}`); @@ -333,6 +332,85 @@ describe("loadWorkspaceSkillEntries", () => { }, ); + it.runIf(process.platform !== "win32")( + "calls out bundled symlink escapes as likely local checkout mutations", + async () => { + const workspaceDir = await createTempWorkspaceDir(); + const bundledDir = path.join(workspaceDir, ".bundled"); + const outsideDir = await createTempWorkspaceDir(); + const escapedSkillDir = path.join(outsideDir, "outside-bundled-skill"); + await writeSkill({ + dir: escapedSkillDir, + name: "outside-bundled-skill", + description: "Outside bundled", + }); + await fs.mkdir(bundledDir, { recursive: true }); + const requestedPath = path.join(bundledDir, "escaped-bundled-skill"); + await fs.symlink(escapedSkillDir, requestedPath, "dir"); + setLoggerOverride({ level: "silent", consoleLevel: "warn" }); + const warn = vi.fn(); + loggingState.rawConsole = { + log: vi.fn(), + info: vi.fn(), + warn, + error: vi.fn(), + }; + + const entries = loadWorkspaceSkillEntries(workspaceDir, { + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: bundledDir, + }); + + expect(entries.map((entry) => entry.skill.name)).not.toContain("outside-bundled-skill"); + const [line] = warn.mock.calls[0] ?? []; + const warningLine = String(line); + expect(warningLine).toContain("Skipping escaped skill path outside its configured root:"); + expect(warningLine).toContain("source=openclaw-bundled"); + expect(warningLine).toContain("reason=bundled-symlink-escape"); + expect(warningLine).toContain("hint=likely-stray-local-symlink-or-checkout-mutation"); + expect(warningLine).toContain(`requested=${requestedPath}`); + expect(warningLine).toContain("resolved="); + }, + ); + + it.runIf(process.platform !== "win32")( + "uses compact home-relative paths in escaped skill console warnings", + async () => { + const workspaceDir = path.join(fakeHome, "workspace"); + const outsideDir = path.join(fakeHome, "outside"); + tempDirs.push(workspaceDir, outsideDir); + const bundledDir = path.join(workspaceDir, ".bundled"); + const escapedSkillDir = path.join(outsideDir, "outside-bundled-skill"); + await writeSkill({ + dir: escapedSkillDir, + name: "outside-bundled-skill", + description: "Outside bundled", + }); + await fs.mkdir(bundledDir, { recursive: true }); + const requestedPath = path.join(bundledDir, "escaped-bundled-skill"); + await fs.symlink(escapedSkillDir, requestedPath, "dir"); + setLoggerOverride({ level: "silent", consoleLevel: "warn" }); + const warn = vi.fn(); + loggingState.rawConsole = { + log: vi.fn(), + info: vi.fn(), + warn, + error: vi.fn(), + }; + + loadWorkspaceSkillEntries(workspaceDir, { + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: bundledDir, + }); + + const [line] = warn.mock.calls[0] ?? []; + const warningLine = String(line); + expect(warningLine).toContain("root=~/workspace/.bundled"); + expect(warningLine).toContain("requested=~/workspace/.bundled/escaped-bundled-skill"); + expect(warningLine).toContain("resolved=~/outside/outside-bundled-skill"); + }, + ); + it.runIf(process.platform !== "win32")( "skips workspace skill files that resolve outside the workspace root", async () => { diff --git a/src/agents/skills/workspace.ts b/src/agents/skills/workspace.ts index 4cdd76e406e..e3dcb903005 100644 --- a/src/agents/skills/workspace.ts +++ b/src/agents/skills/workspace.ts @@ -44,12 +44,19 @@ function resolveUserHomeDir(): string | undefined { } } -function compactSkillPaths(skills: Skill[]): Skill[] { - const homes = [resolveHomeDir(), resolveUserHomeDir()] - .filter((home): home is string => !!home) - .map((home) => path.resolve(home)) +function resolveCompactHomePrefixes(): string[] { + const homes = [resolveHomeDir(), resolveUserHomeDir()].filter((home): home is string => !!home); + const resolvedHomes = homes.map((home) => path.resolve(home)); + const realHomes = resolvedHomes + .map((home) => tryRealpath(home)) + .filter((home): home is string => !!home); + return [...resolvedHomes, ...realHomes] .filter((home, index, all) => all.indexOf(home) === index) .sort((a, b) => b.length - a.length); +} + +function compactSkillPaths(skills: Skill[]): Skill[] { + const homes = resolveCompactHomePrefixes(); if (homes.length === 0) return skills; return skills.map((s) => ({ ...s, @@ -67,6 +74,10 @@ function compactHomePath(filePath: string, homes: readonly string[]): string { return filePath; } +function compactPathForConsoleMessage(filePath: string): string { + return compactHomePath(filePath, resolveCompactHomePrefixes()); +} + function isSkillVisibleInAvailableSkillsPrompt(entry: SkillEntry): boolean { if (entry.exposure) { return entry.exposure.includeInAvailableSkillsPrompt !== false; @@ -162,6 +173,45 @@ function tryRealpath(filePath: string): string | null { } } +function isSymlinkPath(filePath: string): boolean { + try { + return fs.lstatSync(filePath).isSymbolicLink(); + } catch { + return false; + } +} + +function buildEscapedSkillPathReason(params: { source: string; candidatePath: string }): { + reason: string; + consoleHint: string; +} { + const candidateIsSymlink = isSymlinkPath(params.candidatePath); + if (params.source === "openclaw-bundled" && candidateIsSymlink) { + return { + reason: "bundled-symlink-escape", + consoleHint: + "reason=bundled-symlink-escape hint=likely-stray-local-symlink-or-checkout-mutation", + }; + } + if (candidateIsSymlink) { + return { + reason: "symlink-escape", + consoleHint: "reason=symlink-escape", + }; + } + if (params.source === "openclaw-bundled") { + return { + reason: "bundled-root-escape", + consoleHint: + "reason=bundled-root-escape hint=likely-stray-local-symlink-or-checkout-mutation", + }; + } + return { + reason: "path-escape", + consoleHint: "reason=path-escape", + }; +} + function warnEscapedSkillPath(params: { source: string; rootDir: string; @@ -169,20 +219,30 @@ function warnEscapedSkillPath(params: { candidatePath: string; candidateRealPath: string; }) { + const compactRootDir = compactPathForConsoleMessage(params.rootDir); + const compactRootRealPath = compactPathForConsoleMessage(params.rootRealPath); + const compactCandidatePath = compactPathForConsoleMessage(params.candidatePath); + const compactCandidateRealPath = compactPathForConsoleMessage(params.candidateRealPath); const rootResolved = path.resolve(params.rootDir) === params.rootRealPath ? "" - : ` rootResolved=${params.rootRealPath}`; - skillsLogger.warn("Skipping skill path that resolves outside its configured root.", { + : ` rootResolved=${compactRootRealPath}`; + const escapeReason = buildEscapedSkillPathReason({ + source: params.source, + candidatePath: params.candidatePath, + }); + skillsLogger.warn("Skipping escaped skill path outside its configured root.", { source: params.source, rootDir: params.rootDir, rootRealPath: params.rootRealPath, path: params.candidatePath, realPath: params.candidateRealPath, + reason: escapeReason.reason, consoleMessage: - `Skipping skill path that resolves outside its configured root: ` + - `source=${params.source} root=${params.rootDir}${rootResolved} ` + - `requested=${params.candidatePath} resolved=${params.candidateRealPath}`, + `Skipping escaped skill path outside its configured root: ` + + `source=${params.source} root=${compactRootDir}${rootResolved} ` + + `${escapeReason.consoleHint} requested=${compactCandidatePath} ` + + `resolved=${compactCandidateRealPath}`, }); }