fix(openclaw): remove symlink skill in openclaw file (#935)
This commit is contained in:
Haze
@@ -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
|
||||
|
||||
178
electron/gateway/skills-symlink-cleanup.ts
Normal file
178
electron/gateway/skills-symlink-cleanup.ts
Normal file
@@ -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/<name> pointing at ~/.agents/skills/<name>. 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;
|
||||
}
|
||||
286
tests/unit/skills-symlink-cleanup.test.ts
Normal file
286
tests/unit/skills-symlink-cleanup.test.ts
Normal file
@@ -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<typeof import('node:fs')>('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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user