From a74db571d66fa6fdf0a4d59a5530b5be1020ef93 Mon Sep 17 00:00:00 2001 From: Haze <709547807@qq.com> Date: Wed, 29 Apr 2026 11:38:21 +0800 Subject: [PATCH] fix(openclaw): remove symlink skill in openclaw file (#935) --- electron/gateway/config-sync.ts | 12 + electron/gateway/skills-symlink-cleanup.ts | 178 +++++++++++++ tests/unit/skills-symlink-cleanup.test.ts | 286 +++++++++++++++++++++ 3 files changed, 476 insertions(+) create mode 100644 electron/gateway/skills-symlink-cleanup.ts create mode 100644 tests/unit/skills-symlink-cleanup.test.ts diff --git a/electron/gateway/config-sync.ts b/electron/gateway/config-sync.ts index 679b3dd..ccf622a 100644 --- a/electron/gateway/config-sync.ts +++ b/electron/gateway/config-sync.ts @@ -28,6 +28,7 @@ import { logger } from '../utils/logger'; import { prependPathEntry } from '../utils/env-path'; import { copyPluginFromNodeModules, fixupPluginManifest, cpSyncSafe } from '../utils/plugin-install'; import { stripSystemdSupervisorEnv } from './config-sync-env'; +import { cleanupAgentsSymlinkedSkills } from './skills-symlink-cleanup'; export interface GatewayLaunchContext { @@ -344,6 +345,17 @@ export async function syncGatewayConfigBeforeLaunch( logger.warn('Failed to clean stale built-in extensions:', err); } + // Remove stray symlinks under ~/.openclaw/skills whose realpath resolves + // inside ~/.agents/skills. OpenClaw's hardened skill loader rejects these + // on every launch (reason=symlink-escape) and the underlying skills are + // still discovered via the agents-skills-personal source, so the symlinks + // are pure log noise. Transitional workaround for openclaw/openclaw#59219. + try { + cleanupAgentsSymlinkedSkills(); + } catch (err) { + logger.warn('Failed to clean .agents/skills-targeted skill symlinks:', err); + } + // Auto-upgrade installed plugins before Gateway starts so that // the plugin manifest ID matches what sanitize wrote to the config. // Only install/upgrade plugins for channels that are actually configured diff --git a/electron/gateway/skills-symlink-cleanup.ts b/electron/gateway/skills-symlink-cleanup.ts new file mode 100644 index 0000000..ed4d39d --- /dev/null +++ b/electron/gateway/skills-symlink-cleanup.ts @@ -0,0 +1,178 @@ +/** + * Pre-launch cleanup for stray skill symlinks under ~/.openclaw/skills. + * + * Background: since openclaw commit 253e159700 ("fix: harden workspace skill + * path containment"), the Gateway rejects any candidate under a skills root + * whose realpath escapes that root, logging a noisy + * `Skipping escaped skill path outside its configured root. + * reason=symlink-escape source=openclaw-managed ...` + * warning per offending entry on every start. + * + * A common offender is one-shot install scripts that drop symlinks into + * ~/.openclaw/skills/ pointing at ~/.agents/skills/. The skills + * still load via the separate `agents-skills-personal` source (which scans + * ~/.agents/skills directly), so the symlinks under ~/.openclaw/skills are + * pure log noise — and a duplicate entry that the loader can never accept. + * + * This helper is invoked before each Gateway launch to remove those + * specific symlinks. Scope is intentionally narrow: + * - source dir: ~/.openclaw/skills (resolved via getOpenClawSkillsDir()) + * - target dir: ~/.agents/skills only (NOT the broader ~/.agents tree) + * Symlinks whose realpath resolves anywhere else under ~/.agents (e.g. + * ~/.agents/tools/foo) or to unrelated locations are left untouched. + * + * Removal uses fs.rmSync({ force: true }) rather than fs.unlinkSync so that + * Windows directory symlinks and junctions (the form that non-admin Windows + * installs end up creating) are deleted correctly. unlinkSync raises EPERM + * on those on Windows. + * + * This is a transitional workaround. Once openclaw/openclaw#59219 lands and + * the loader stops rejecting managed-source symlinks whose realpath escapes + * the managed root, this helper can be removed entirely. + */ +import { + existsSync, + lstatSync, + readdirSync, + realpathSync, + rmSync, + type Dirent, +} from 'node:fs'; +import { homedir } from 'node:os'; +import path from 'node:path'; +import { getOpenClawSkillsDir } from '../utils/paths'; +import { logger } from '../utils/logger'; + +export interface CleanupOptions { + /** Override for ~/.openclaw/skills (mainly for tests). */ + skillsDir?: string; + /** Override for ~/.agents/skills (mainly for tests). */ + agentsDir?: string; +} + +export interface CleanupResult { + /** Symlink names that were unlinked from the skills dir. */ + removed: string[]; + /** Total number of symlink entries that were inspected. */ + examined: number; +} + +function defaultSkillsDir(): string { + return getOpenClawSkillsDir(); +} + +function defaultAgentsDir(): string { + return path.join(homedir(), '.agents', 'skills'); +} + +/** + * Resolve the agents skills directory to its real path. When the directory + * itself does not exist yet (fresh install), fall back to realpath'ing its + * parent and re-appending the basename so a `~/.agents -> /opt/agents` + * indirection is still honored. As a final fallback returns the lexical + * resolved path. + */ +function resolveAgentsRealRoot(agentsDir: string): string { + if (existsSync(agentsDir)) { + try { + return realpathSync(agentsDir); + } catch { + // fall through + } + } + const parent = path.dirname(agentsDir); + const tail = path.basename(agentsDir); + if (parent && parent !== agentsDir && existsSync(parent)) { + try { + return path.join(realpathSync(parent), tail); + } catch { + // fall through + } + } + return path.resolve(agentsDir); +} + +/** + * Lower-case path strings on Win32 only so the `path.relative` byte-wise + * comparison aligns with NTFS case-insensitive semantics. No-op elsewhere. + */ +function normalizeForCompare(p: string): string { + return process.platform === 'win32' ? p.toLowerCase() : p; +} + +function isInside(parent: string, child: string): boolean { + const rel = path.relative(normalizeForCompare(parent), normalizeForCompare(child)); + if (rel === '') return true; + return !rel.startsWith('..') && !path.isAbsolute(rel); +} + +export function cleanupAgentsSymlinkedSkills(opts: CleanupOptions = {}): CleanupResult { + const skillsDir = opts.skillsDir ?? defaultSkillsDir(); + const agentsDir = opts.agentsDir ?? defaultAgentsDir(); + const result: CleanupResult = { removed: [], examined: 0 }; + + if (!existsSync(skillsDir)) { + return result; + } + + let entries: Dirent[]; + try { + entries = readdirSync(skillsDir, { withFileTypes: true, encoding: 'utf8' }); + } catch (err) { + logger.warn(`[skills-cleanup] Failed to list ${skillsDir}:`, err); + return result; + } + + const agentsRealRoot = resolveAgentsRealRoot(agentsDir); + + for (const entry of entries) { + const entryPath = path.join(skillsDir, entry.name); + + let isSymlink = entry.isSymbolicLink(); + if (!isSymlink) { + try { + isSymlink = lstatSync(entryPath).isSymbolicLink(); + } catch { + continue; + } + } + if (!isSymlink) continue; + + result.examined++; + + let realTarget: string; + try { + realTarget = realpathSync(entryPath); + } catch { + continue; + } + + if (!isInside(agentsRealRoot, realTarget)) continue; + + try { + // rmSync({ force: true }) handles file symlinks, directory symlinks, + // and Windows junctions uniformly. unlinkSync would raise EPERM on + // directory symlinks/junctions on Windows. + rmSync(entryPath, { force: true }); + result.removed.push(entry.name); + } catch (err) { + logger.warn(`[skills-cleanup] Failed to remove ${entryPath}:`, err); + } + } + + if (result.removed.length > 0) { + logger.info( + `[skills-cleanup] Removed ${result.removed.length} stray skill symlink(s) ` + + `under ${skillsDir} that resolved into ${agentsRealRoot} ` + + `(workaround for openclaw/openclaw#59219): ` + + result.removed.join(', '), + ); + } else if (result.examined > 0) { + logger.debug( + `[skills-cleanup] Examined ${result.examined} symlink(s) under ${skillsDir}; ` + + `none resolved into ${agentsRealRoot}`, + ); + } + + return result; +} diff --git a/tests/unit/skills-symlink-cleanup.test.ts b/tests/unit/skills-symlink-cleanup.test.ts new file mode 100644 index 0000000..20e228e --- /dev/null +++ b/tests/unit/skills-symlink-cleanup.test.ts @@ -0,0 +1,286 @@ +import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +const { rmSyncMock } = vi.hoisted(() => ({ rmSyncMock: vi.fn() })); + +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs'); + const wrappedRmSync: typeof actual.rmSync = (...args) => { + rmSyncMock(...args); + return actual.rmSync(...args); + }; + return { + ...actual, + default: { ...actual, rmSync: wrappedRmSync }, + rmSync: wrappedRmSync, + }; +}); + +vi.mock('@electron/utils/logger', () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, +})); + +import { + existsSync, + lstatSync, + mkdirSync, + mkdtempSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; +import { cleanupAgentsSymlinkedSkills } from '@electron/gateway/skills-symlink-cleanup'; +import { logger } from '@electron/utils/logger'; + +const SYMLINK_TYPE: 'dir' | 'junction' = process.platform === 'win32' ? 'junction' : 'dir'; + +describe('cleanupAgentsSymlinkedSkills', () => { + let root: string; + let skillsDir: string; + let agentsRootDir: string; + let agentsSkillsDir: string; + + beforeEach(() => { + root = mkdtempSync(path.join(tmpdir(), 'clawx-skills-cleanup-')); + skillsDir = path.join(root, 'openclaw', 'skills'); + agentsRootDir = path.join(root, 'agents'); + agentsSkillsDir = path.join(agentsRootDir, 'skills'); + mkdirSync(skillsDir, { recursive: true }); + mkdirSync(agentsSkillsDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + function makeAgentSkill(name: string): string { + const dir = path.join(agentsSkillsDir, name); + mkdirSync(dir, { recursive: true }); + writeFileSync(path.join(dir, 'SKILL.md'), '# test\n'); + return dir; + } + + it('removes symlinks whose realpath resolves into the agents/skills dir', () => { + const target = makeAgentSkill('lark-foo'); + const link = path.join(skillsDir, 'lark-foo'); + symlinkSync(target, link, SYMLINK_TYPE); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed).toEqual(['lark-foo']); + expect(res.examined).toBe(1); + expect(existsSync(link)).toBe(false); + expect(existsSync(target)).toBe(true); + }); + + it('removes multiple .agents/skills-targeted symlinks in one pass', () => { + for (const name of ['lark-base', 'lark-im', 'lark-doc']) { + const target = makeAgentSkill(name); + symlinkSync(target, path.join(skillsDir, name), SYMLINK_TYPE); + } + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed.sort()).toEqual(['lark-base', 'lark-doc', 'lark-im']); + expect(res.examined).toBe(3); + }); + + it('keeps in-tree symlinks and regular directories', () => { + const realSkillDir = path.join(skillsDir, 'real-skill'); + mkdirSync(realSkillDir); + writeFileSync(path.join(realSkillDir, 'SKILL.md'), ''); + const insideLink = path.join(skillsDir, 'alias'); + symlinkSync(realSkillDir, insideLink, SYMLINK_TYPE); + + const plainDir = path.join(skillsDir, 'plain'); + mkdirSync(plainDir); + writeFileSync(path.join(plainDir, 'SKILL.md'), ''); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed).toEqual([]); + expect(res.examined).toBe(1); + expect(lstatSync(insideLink).isSymbolicLink()).toBe(true); + expect(lstatSync(plainDir).isDirectory()).toBe(true); + }); + + it('keeps symlinks pointing at unrelated locations', () => { + const elsewhere = path.join(root, 'elsewhere', 'foo'); + mkdirSync(elsewhere, { recursive: true }); + const link = path.join(skillsDir, 'foo'); + symlinkSync(elsewhere, link, SYMLINK_TYPE); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed).toEqual([]); + expect(res.examined).toBe(1); + expect(lstatSync(link).isSymbolicLink()).toBe(true); + }); + + it('keeps symlinks pointing under .agents but outside .agents/skills', () => { + const tools = path.join(agentsRootDir, 'tools', 'foo'); + mkdirSync(tools, { recursive: true }); + writeFileSync(path.join(tools, 'README.md'), ''); + const link = path.join(skillsDir, 'foo'); + symlinkSync(tools, link, SYMLINK_TYPE); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed).toEqual([]); + expect(res.examined).toBe(1); + expect(lstatSync(link).isSymbolicLink()).toBe(true); + }); + + it('removes file-type symlinks targeting a file inside .agents/skills', () => { + const skillDir = makeAgentSkill('lark-meta'); + const targetFile = path.join(skillDir, 'SKILL.md'); + const link = path.join(skillsDir, 'lark-meta.md'); + symlinkSync(targetFile, link, 'file'); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed).toEqual(['lark-meta.md']); + expect(existsSync(link)).toBe(false); + expect(existsSync(targetFile)).toBe(true); + }); + + it('skips broken symlinks without throwing', () => { + const dangling = path.join(root, 'gone'); + const link = path.join(skillsDir, 'broken'); + symlinkSync(dangling, link, SYMLINK_TYPE); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed).toEqual([]); + expect(res.examined).toBe(1); + expect(lstatSync(link).isSymbolicLink()).toBe(true); + }); + + it('handles a missing skills dir as a no-op', () => { + rmSync(skillsDir, { recursive: true, force: true }); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res).toEqual({ removed: [], examined: 0 }); + }); + + it('handles a missing agents dir without removing anything', () => { + rmSync(agentsRootDir, { recursive: true, force: true }); + const target = path.join(agentsSkillsDir, 'lark-foo'); + mkdirSync(target, { recursive: true }); + const link = path.join(skillsDir, 'lark-foo'); + symlinkSync(target, link, SYMLINK_TYPE); + + rmSync(target, { recursive: true, force: true }); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed).toEqual([]); + expect(lstatSync(link).isSymbolicLink()).toBe(true); + }); + + it('follows realpath through an indirected agents dir symlink', () => { + const realAgentsRoot = path.join(root, 'real-agents'); + const realAgentsSkills = path.join(realAgentsRoot, 'skills'); + mkdirSync(path.join(realAgentsSkills, 'lark-foo'), { recursive: true }); + rmSync(agentsRootDir, { recursive: true, force: true }); + symlinkSync(realAgentsRoot, agentsRootDir, SYMLINK_TYPE); + + const link = path.join(skillsDir, 'lark-foo'); + symlinkSync(path.join(realAgentsSkills, 'lark-foo'), link, SYMLINK_TYPE); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed).toEqual(['lark-foo']); + expect(existsSync(link)).toBe(false); + }); + + it('falls back to parent realpath when agents/skills does not exist yet', () => { + rmSync(agentsSkillsDir, { recursive: true, force: true }); + + const realAgentsSkills = path.join(root, 'real', 'agents', 'skills'); + mkdirSync(realAgentsSkills, { recursive: true }); + const target = path.join(realAgentsSkills, 'lark-foo'); + mkdirSync(target); + writeFileSync(path.join(target, 'SKILL.md'), ''); + const link = path.join(skillsDir, 'lark-foo'); + symlinkSync(target, link, SYMLINK_TYPE); + + // agentsRootDir exists, agentsSkillsDir does not: parent fallback should + // not falsely report containment for this unrelated target. + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + + expect(res.removed).toEqual([]); + expect(lstatSync(link).isSymbolicLink()).toBe(true); + }); + + it('uses fs.rmSync({ force: true }) so directory symlinks/junctions delete on Windows', () => { + const target = makeAgentSkill('lark-rm'); + const link = path.join(skillsDir, 'lark-rm'); + symlinkSync(target, link, SYMLINK_TYPE); + + rmSyncMock.mockClear(); + + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + expect(res.removed).toEqual(['lark-rm']); + + const linkRmCall = rmSyncMock.mock.calls.find((args) => args[0] === link); + expect(linkRmCall).toBeDefined(); + expect(linkRmCall?.[1]).toEqual({ force: true }); + }); + + it('matches paths case-insensitively when running on Win32', () => { + // Create the agents tree with all-uppercase basenames so a lowercase + // override differs lexically. On case-insensitive filesystems (macOS + // APFS) this still passes because realpathSync canonicalises both sides; + // on case-sensitive filesystems (Linux ext4) the test only succeeds + // because of the Win32 lowercase normalisation in isInside(). + const upperAgentsRoot = path.join(root, 'UPPER_AGENTS'); + const upperAgentsSkills = path.join(upperAgentsRoot, 'UPPER_SKILLS'); + const target = path.join(upperAgentsSkills, 'lark-foo'); + mkdirSync(target, { recursive: true }); + writeFileSync(path.join(target, 'SKILL.md'), ''); + + const link = path.join(skillsDir, 'lark-foo'); + symlinkSync(target, link, SYMLINK_TYPE); + + const lowered = path.join(root, 'upper_agents', 'upper_skills'); + + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { + value: 'win32', + configurable: true, + }); + try { + const res = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: lowered }); + expect(res.removed).toEqual(['lark-foo']); + } finally { + Object.defineProperty(process, 'platform', { + value: originalPlatform, + configurable: true, + }); + } + }); + + it('is idempotent: a second invocation is a no-op and emits no info log', () => { + const target = makeAgentSkill('lark-once'); + const link = path.join(skillsDir, 'lark-once'); + symlinkSync(target, link, SYMLINK_TYPE); + + const first = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + expect(first.removed).toEqual(['lark-once']); + + const infoMock = vi.mocked(logger.info); + infoMock.mockClear(); + + const second = cleanupAgentsSymlinkedSkills({ skillsDir, agentsDir: agentsSkillsDir }); + expect(second).toEqual({ removed: [], examined: 0 }); + expect(infoMock).not.toHaveBeenCalled(); + }); +});