fix: clarify escaped skill path warnings

This commit is contained in:
Peter Steinberger
2026-04-12 10:53:23 -07:00
parent 2204753b62
commit e24b80b15e
2 changed files with 150 additions and 12 deletions

View File

@@ -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 () => {

View File

@@ -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}`,
});
}