From d218061c64506b8d758cae7754a3d453268590d2 Mon Sep 17 00:00:00 2001 From: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Date: Mon, 4 May 2026 15:12:17 +0000 Subject: [PATCH] fix(clawsweeper): address review for automerge-openclaw-openclaw-77328 (1) --- src/agents/skills/plugin-skills.test.ts | 48 +++++++++++++++++++++++++ src/agents/skills/plugin-skills.ts | 26 ++++++++++++-- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/agents/skills/plugin-skills.test.ts b/src/agents/skills/plugin-skills.test.ts index f5058110c89..d68d4f39bbe 100644 --- a/src/agents/skills/plugin-skills.test.ts +++ b/src/agents/skills/plugin-skills.test.ts @@ -281,6 +281,31 @@ describe("resolvePluginSkillDirs", () => { expect(dirs).toEqual([]); }); + it("cleans up generated plugin skill links when the plugin registry is empty", async () => { + const workspaceDir = await tempDirs.make("openclaw-"); + const pluginSkillsDir = await tempDirs.make("managed-plugin-skills-"); + const staleRoot = await tempDirs.make("stale-plugin-skills-"); + const staleSkill = path.join(staleRoot, "stale-skill"); + await fs.mkdir(staleSkill, { recursive: true }); + fsSync.symlinkSync(staleSkill, path.join(pluginSkillsDir, "stale-skill"), "dir"); + + hoisted.loadPluginManifestRegistryForInstalledIndex.mockReturnValue({ + diagnostics: [], + plugins: [], + }); + + const dirs = resolvePluginSkillDirs({ + workspaceDir, + config: {} as OpenClawConfig, + pluginSkillsDir, + }); + + expect(dirs).toEqual([]); + await expect(fs.lstat(path.join(pluginSkillsDir, "stale-skill"))).rejects.toMatchObject({ + code: "ENOENT", + }); + }); + it("resolves Claude bundle command roots through the normal plugin skill path", async () => { const workspaceDir = await tempDirs.make("openclaw-"); const pluginRoot = await tempDirs.make("openclaw-claude-bundle-"); @@ -438,6 +463,29 @@ describe("publishPluginSkills", () => { expect(fsSync.existsSync(path.join(managedDir, "broken-skill"))).toBe(false); }); + it.runIf(process.platform !== "win32")( + "skips child skill directories whose SKILL.md symlinks outside the declared root", + async () => { + const skillParent = await tempDirs.make("plugin-skills-"); + const managedDir = await tempDirs.make("managed-skills-"); + const outsideDir = await tempDirs.make("outside-skill-file-"); + const parentDir = path.join(skillParent, "skills"); + const leakDir = path.join(parentDir, "leak"); + await fs.mkdir(leakDir, { recursive: true }); + await fs.writeFile( + path.join(outsideDir, "SKILL.md"), + "---\nname: leak\ndescription: Outside\n---\n", + ); + await fs.symlink(path.join(outsideDir, "SKILL.md"), path.join(leakDir, "SKILL.md")); + const validDir = await writeSkillDir(parentDir, "valid"); + + publishPluginSkills([parentDir], { pluginSkillsDir: managedDir }); + + expect(fsSync.existsSync(path.join(managedDir, "leak"))).toBe(false); + expect(fsSync.readlinkSync(path.join(managedDir, "valid"))).toBe(validDir); + }, + ); + it("does not create managed skills dir when skill dirs list is empty", async () => { const parent = await tempDirs.make("parent-"); const managedDir = path.join(parent, "does-not-exist"); diff --git a/src/agents/skills/plugin-skills.ts b/src/agents/skills/plugin-skills.ts index 9ef5cdfcf40..7c2ed971db0 100644 --- a/src/agents/skills/plugin-skills.ts +++ b/src/agents/skills/plugin-skills.ts @@ -32,6 +32,9 @@ export function resolvePluginSkillDirs(params: { }); const registry = metadataSnapshot.manifestRegistry; if (registry.plugins.length === 0) { + publishPluginSkills([], { + pluginSkillsDir: params.pluginSkillsDir, + }); return []; } const normalizedPlugins = normalizePluginsConfigWithResolver( @@ -113,7 +116,7 @@ function resolveDefaultPluginSkillsDir(): string { * Otherwise child subdirectories that contain SKILL.md are expanded. */ function collectSkillTargets(dir: string, targets: Map): void { - if (fs.existsSync(path.join(dir, "SKILL.md"))) { + if (hasPublishableSkillFile({ skillDir: dir, rootDir: dir })) { const basename = path.basename(dir); const existing = targets.get(basename); if (existing) { @@ -136,7 +139,7 @@ function collectSkillTargets(dir: string, targets: Map): void { for (const entry of entries) { if (!entry.isDirectory()) continue; const childPath = path.join(dir, entry.name); - if (!fs.existsSync(path.join(childPath, "SKILL.md"))) continue; + if (!hasPublishableSkillFile({ skillDir: childPath, rootDir: dir })) continue; const basename = entry.name; const existing = targets.get(basename); if (existing) { @@ -150,6 +153,25 @@ function collectSkillTargets(dir: string, targets: Map): void { } } +function hasPublishableSkillFile(params: { skillDir: string; rootDir: string }): boolean { + const skillMd = path.join(params.skillDir, "SKILL.md"); + let skillMdStat: fs.Stats; + try { + skillMdStat = fs.lstatSync(skillMd); + } catch { + return false; + } + if (!skillMdStat.isFile() || skillMdStat.isSymbolicLink()) { + log.warn(`plugin skill SKILL.md is not a regular file: ${skillMd}`); + return false; + } + if (!isPathInsideWithRealpath(params.rootDir, skillMd, { requireRealpath: true })) { + log.warn(`plugin skill SKILL.md escapes declared skill root: ${skillMd}`); + return false; + } + return true; +} + /** * Creates symlinks from each resolved plugin skill directory into the * plugin skills directory (~/.openclaw/plugin-skills/) so the agent SDK can