mirror of
https://github.com/ValueCell-ai/ClawX.git
synced 2026-08-14 00:48:10 +00:00
fix: prevent plugin cleanup from deleting bundled OpenClaw runtime (#1197)
This commit is contained in:
@@ -118,7 +118,7 @@ jobs:
|
||||
run: pnpm install --frozen-lockfile
|
||||
|
||||
- name: Test Windows attachment open-with bridge
|
||||
run: pnpm exec vitest run tests/unit/attachment-open-with.test.ts tests/unit/attachment-open-with-native.test.ts
|
||||
run: pnpm exec vitest run tests/unit/attachment-open-with.test.ts tests/unit/attachment-open-with-native.test.ts tests/unit/safe-fs.test.ts
|
||||
|
||||
- name: Generate extension bridge
|
||||
run: pnpm run ext:bridge
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { app } from 'electron';
|
||||
import path from 'path';
|
||||
import { existsSync, readFileSync, mkdirSync, readdirSync, rmSync, symlinkSync } from 'fs';
|
||||
import { existsSync, readFileSync, mkdirSync, readdirSync, symlinkSync } from 'fs';
|
||||
import { homedir } from 'os';
|
||||
import { join } from 'path';
|
||||
|
||||
@@ -34,6 +34,7 @@ import { syncProxyConfigToOpenClaw } from '../utils/openclaw-proxy';
|
||||
import { logger } from '../utils/logger';
|
||||
import { prependPathEntry } from '../utils/env-path';
|
||||
import { copyPluginFromNodeModules, fixupPluginManifest, cpSyncSafe, buildCandidateSources, repairTrustedOfficialPluginInstallRecords, removeTrustedOfficialPluginInstallRecord, syncTrustedOfficialPluginInstallRecord, resolvePluginNpmPackagePath } from '../utils/plugin-install';
|
||||
import { safeRmSync } from '../utils/safe-fs';
|
||||
import { CLAWX_OPENAI_IMAGE_PROVIDER_KEY } from '../utils/openclaw-image-relay-constants';
|
||||
import { ensureOpenClaw2026_7_1UpgradeSnapshot } from '../utils/openclaw-upgrade-snapshot';
|
||||
import { stripSystemdSupervisorEnv } from './config-sync-env';
|
||||
@@ -120,7 +121,7 @@ function cleanupStaleBuiltInExtensions(): void {
|
||||
if (existsSync(fsPath(extDir))) {
|
||||
logger.info(`[plugin] Removing stale built-in extension copy: ${ext}`);
|
||||
try {
|
||||
rmSync(fsPath(extDir), { recursive: true, force: true });
|
||||
safeRmSync(fsPath(extDir));
|
||||
} catch (err) {
|
||||
logger.warn(`[plugin] Failed to remove stale extension ${ext}:`, err);
|
||||
}
|
||||
@@ -192,7 +193,7 @@ function ensureConfiguredPluginsUpgraded(configuredChannels: string[]): boolean
|
||||
logger.info(`[plugin] ${isInstalled ? 'Auto-upgrading' : 'Installing'} ${channelType} plugin${isInstalled ? `: ${installedVersion} → ${sourceVersion}` : `: ${sourceVersion}`} (bundled)`);
|
||||
try {
|
||||
mkdirSync(fsPath(join(homedir(), '.openclaw', 'extensions')), { recursive: true });
|
||||
rmSync(fsPath(targetDir), { recursive: true, force: true });
|
||||
safeRmSync(fsPath(targetDir));
|
||||
cpSyncSafe(bundledDir, targetDir);
|
||||
fixupPluginManifest(targetDir);
|
||||
syncTrustedOfficialPluginInstallRecord(dirName, targetDir);
|
||||
@@ -258,7 +259,7 @@ function cleanupUnconfiguredChannelPlugins(configuredChannels: string[]): boolea
|
||||
|
||||
logger.info(`[plugin] Removing unconfigured channel plugin: ${channelType} (${dirName})`);
|
||||
try {
|
||||
rmSync(fsPath(targetDir), { recursive: true, force: true });
|
||||
safeRmSync(fsPath(targetDir));
|
||||
} catch (err) {
|
||||
logger.warn(`[plugin] Failed to remove unconfigured channel plugin ${channelType}:`, err);
|
||||
succeeded = false;
|
||||
|
||||
@@ -7,12 +7,13 @@
|
||||
*/
|
||||
import { app } from 'electron';
|
||||
import path from 'node:path';
|
||||
import { existsSync, cpSync, copyFileSync, statSync, lstatSync, mkdirSync, rmSync, readFileSync, writeFileSync, readdirSync, realpathSync, symlinkSync, unlinkSync } from 'node:fs';
|
||||
import { existsSync, cpSync, copyFileSync, statSync, lstatSync, mkdirSync, readFileSync, writeFileSync, readdirSync, realpathSync, symlinkSync, unlinkSync } from 'node:fs';
|
||||
import { readdir, stat, copyFile, mkdir } from 'node:fs/promises';
|
||||
import { homedir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { logger } from './logger';
|
||||
import { getOpenClawResolvedDir } from './paths';
|
||||
import { safeRmSync } from './safe-fs';
|
||||
import {
|
||||
upsertPluginInstallRecordsIntoSqlite,
|
||||
removePluginInstallRecordsFromSqlite,
|
||||
@@ -491,7 +492,7 @@ export function repairPluginOpenClawPeerLink(
|
||||
logger.warn(`[plugin] Cannot replace non-OpenClaw peer directory at ${linkPath}`);
|
||||
return false;
|
||||
}
|
||||
rmSync(fsPath(linkPath), { recursive: true, force: true });
|
||||
safeRmSync(fsPath(linkPath));
|
||||
} else {
|
||||
logger.warn(`[plugin] Cannot replace non-directory OpenClaw peer at ${linkPath}`);
|
||||
return false;
|
||||
@@ -664,7 +665,7 @@ export function copyPluginFromNodeModules(npmPkgPath: string, targetDir: string,
|
||||
}
|
||||
|
||||
// 1. Copy plugin package itself
|
||||
rmSync(fsPath(targetDir), { recursive: true, force: true });
|
||||
safeRmSync(fsPath(targetDir));
|
||||
mkdirSync(fsPath(targetDir), { recursive: true });
|
||||
cpSyncSafe(realPath, targetDir);
|
||||
|
||||
@@ -764,7 +765,7 @@ export function ensurePluginInstalled(
|
||||
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
|
||||
try {
|
||||
mkdirSync(fsPath(extensionsRoot), { recursive: true });
|
||||
rmSync(fsPath(targetDir), { recursive: true, force: true });
|
||||
safeRmSync(fsPath(targetDir));
|
||||
cpSyncSafe(sourceDir, targetDir);
|
||||
if (!existsSync(fsPath(join(targetDir, 'openclaw.plugin.json')))) {
|
||||
return { installed: false, warning: `Failed to install ${pluginLabel} plugin mirror (manifest missing).` };
|
||||
@@ -778,7 +779,7 @@ export function ensurePluginInstalled(
|
||||
attempts.push({ attempt, ...diagnostic });
|
||||
if (attempt < maxAttempts) {
|
||||
try {
|
||||
rmSync(fsPath(targetDir), { recursive: true, force: true });
|
||||
safeRmSync(fsPath(targetDir));
|
||||
} catch {
|
||||
// Ignore cleanup failures before retry.
|
||||
}
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
import { dirname, join } from 'node:path';
|
||||
import { lstatSync, readdirSync, realpathSync, rmdirSync, unlinkSync } from 'node:fs';
|
||||
|
||||
function normalizeComparablePath(input: string): string {
|
||||
if (process.platform === 'win32') {
|
||||
return input.replace(/\\/g, '/').toLowerCase();
|
||||
}
|
||||
return input;
|
||||
}
|
||||
|
||||
function isPathInside(root: string, candidate: string): boolean {
|
||||
const normalizedRoot = normalizeComparablePath(root);
|
||||
const normalizedCandidate = normalizeComparablePath(candidate);
|
||||
const rootWithSep = normalizedRoot.endsWith('/') ? normalizedRoot : `${normalizedRoot}/`;
|
||||
return normalizedCandidate === normalizedRoot || normalizedCandidate.startsWith(rootWithSep);
|
||||
}
|
||||
|
||||
function errnoCode(error: unknown): string | undefined {
|
||||
return error && typeof error === 'object'
|
||||
? (error as NodeJS.ErrnoException).code
|
||||
: undefined;
|
||||
}
|
||||
|
||||
function resolveRealPath(input: string): string {
|
||||
// Node's JavaScript realpath implementation can split a Windows namespaced
|
||||
// path (\\?\C:\...) at the drive colon and try to lstat "C:". The native
|
||||
// implementation accepts the same long-path form without reparsing it.
|
||||
return realpathSync.native(input);
|
||||
}
|
||||
|
||||
function removeLinkEntry(entryPath: string): void {
|
||||
// Never recursively remove a link. In particular, an NTFS junction may point
|
||||
// at the bundled OpenClaw runtime outside the plugin tree.
|
||||
try {
|
||||
unlinkSync(entryPath);
|
||||
} catch (error) {
|
||||
const code = errnoCode(error);
|
||||
if (code === 'ENOENT') return;
|
||||
// libuv normally unlinks Windows junctions directly. Some Windows filesystems
|
||||
// report directory links as EPERM/EISDIR, where a non-recursive rmdir removes
|
||||
// the junction node without traversing its target.
|
||||
if (process.platform === 'win32' && (code === 'EPERM' || code === 'EISDIR')) {
|
||||
rmdirSync(entryPath);
|
||||
return;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
function removeFileEntry(entryPath: string): void {
|
||||
try {
|
||||
unlinkSync(entryPath);
|
||||
} catch (error) {
|
||||
if (errnoCode(error) !== 'ENOENT') throw error;
|
||||
}
|
||||
}
|
||||
|
||||
function removeDirectoryEntry(entryPath: string, deletionRootRealPath: string): void {
|
||||
let stat;
|
||||
try {
|
||||
stat = lstatSync(entryPath);
|
||||
} catch (error) {
|
||||
if (errnoCode(error) === 'ENOENT') return;
|
||||
throw error;
|
||||
}
|
||||
|
||||
if (stat.isSymbolicLink()) {
|
||||
removeLinkEntry(entryPath);
|
||||
return;
|
||||
}
|
||||
|
||||
if (stat.isDirectory()) {
|
||||
// Resolve before descending. If resolution fails, propagate the error rather
|
||||
// than falling back to fs.rmSync(), which could follow an outbound junction.
|
||||
const entryRealPath = resolveRealPath(entryPath);
|
||||
if (!isPathInside(deletionRootRealPath, entryRealPath)) {
|
||||
throw new Error(`Refusing to recursively delete directory outside root: ${entryPath} -> ${entryRealPath}`);
|
||||
}
|
||||
|
||||
for (const child of readdirSync(entryPath)) {
|
||||
removeDirectoryEntry(join(entryPath, child), deletionRootRealPath);
|
||||
}
|
||||
rmdirSync(entryPath);
|
||||
return;
|
||||
}
|
||||
|
||||
removeFileEntry(entryPath);
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a file or directory tree without following outbound directory
|
||||
* junctions/symlinks on Windows. Plain fs.rmSync({ recursive: true }) can
|
||||
* traverse NTFS junctions (for example plugin node_modules/openclaw peers)
|
||||
* and delete link targets outside the requested tree.
|
||||
*/
|
||||
export function safeRmSync(targetPath: string): void {
|
||||
let stat;
|
||||
try {
|
||||
stat = lstatSync(targetPath);
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return;
|
||||
throw error;
|
||||
}
|
||||
|
||||
if (stat.isSymbolicLink()) {
|
||||
removeLinkEntry(targetPath);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!stat.isDirectory()) {
|
||||
removeFileEntry(targetPath);
|
||||
return;
|
||||
}
|
||||
|
||||
// Fail closed when either path cannot be resolved. Falling back to recursive
|
||||
// rm here would reintroduce the junction traversal this helper prevents.
|
||||
const parentRealPath = resolveRealPath(dirname(targetPath));
|
||||
const deletionRootRealPath = resolveRealPath(targetPath);
|
||||
if (!isPathInside(parentRealPath, deletionRootRealPath)) {
|
||||
throw new Error(`Refusing to recursively delete directory outside parent: ${targetPath} -> ${deletionRootRealPath}`);
|
||||
}
|
||||
|
||||
for (const child of readdirSync(targetPath)) {
|
||||
removeDirectoryEntry(join(targetPath, child), deletionRootRealPath);
|
||||
}
|
||||
|
||||
rmdirSync(targetPath);
|
||||
}
|
||||
@@ -0,0 +1,35 @@
|
||||
---
|
||||
id: fix-windows-plugin-cleanup
|
||||
title: Fix Windows channel plugin cleanup
|
||||
scenario: plugin-lifecycle-management
|
||||
taskType: plugin-lifecycle
|
||||
intent: Remove unconfigured channel plugins safely when their Windows paths use the namespaced path prefix.
|
||||
touchedAreas:
|
||||
- .github/workflows/check.yml
|
||||
- electron/gateway/config-sync.ts
|
||||
- electron/utils/plugin-install.ts
|
||||
- electron/utils/safe-fs.ts
|
||||
- tests/unit/safe-fs.test.ts
|
||||
- harness/specs/tasks/fix-windows-plugin-cleanup.md
|
||||
expectedUserBehavior:
|
||||
- Removing a configured channel also removes its stale plugin directory on Windows.
|
||||
- Plugin cleanup never follows outbound directory links into the bundled OpenClaw runtime.
|
||||
requiredProfiles:
|
||||
- fast
|
||||
requiredTests:
|
||||
- tests/unit/safe-fs.test.ts
|
||||
acceptance:
|
||||
- Safe recursive removal accepts Windows namespaced paths such as `\\?\C:\Users\...\extensions\wecom`.
|
||||
- Real-path validation does not reduce a namespaced drive path to `C:`.
|
||||
- Outbound symlink and junction targets remain untouched.
|
||||
references:
|
||||
- harness/specs/scenarios/gateway-backend-communication.md
|
||||
- harness/specs/scenarios/plugin-lifecycle-management.md
|
||||
docs:
|
||||
required: false
|
||||
---
|
||||
|
||||
This task covers the Windows cleanup path used during Gateway configuration
|
||||
synchronization after a channel is removed. The deletion guard must retain its
|
||||
junction-safety checks while resolving namespaced paths through the native
|
||||
Windows real-path implementation.
|
||||
@@ -0,0 +1,82 @@
|
||||
import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join, toNamespacedPath } from 'node:path';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { existsSync } from 'node:fs';
|
||||
import { safeRmSync } from '@electron/utils/safe-fs';
|
||||
|
||||
const SYMLINK_TYPE: 'dir' | 'junction' = process.platform === 'win32' ? 'junction' : 'dir';
|
||||
|
||||
describe('safeRmSync', () => {
|
||||
let root: string;
|
||||
|
||||
afterEach(() => {
|
||||
if (root && existsSync(root)) {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('removes a directory tree without deleting outbound symlink/junction targets', () => {
|
||||
root = mkdtempSync(join(tmpdir(), 'clawx-safe-rm-'));
|
||||
const bundledRuntime = join(root, 'bundled-openclaw');
|
||||
const pluginDir = join(root, 'extensions', 'openclaw-weixin');
|
||||
const peerLink = join(pluginDir, 'node_modules', 'openclaw');
|
||||
|
||||
mkdirSync(bundledRuntime, { recursive: true });
|
||||
writeFileSync(join(bundledRuntime, 'openclaw.mjs'), 'export {}');
|
||||
writeFileSync(join(bundledRuntime, 'package.json'), '{"name":"openclaw"}');
|
||||
|
||||
mkdirSync(join(pluginDir, 'node_modules'), { recursive: true });
|
||||
writeFileSync(join(pluginDir, 'openclaw.plugin.json'), '{"id":"openclaw-weixin"}');
|
||||
symlinkSync(bundledRuntime, peerLink, SYMLINK_TYPE);
|
||||
|
||||
safeRmSync(pluginDir);
|
||||
|
||||
expect(existsSync(pluginDir)).toBe(false);
|
||||
expect(existsSync(join(bundledRuntime, 'openclaw.mjs'))).toBe(true);
|
||||
expect(existsSync(join(bundledRuntime, 'package.json'))).toBe(true);
|
||||
});
|
||||
|
||||
it('removes a top-level outbound directory link without deleting its target', () => {
|
||||
root = mkdtempSync(join(tmpdir(), 'clawx-safe-rm-link-'));
|
||||
const target = join(root, 'runtime');
|
||||
const link = join(root, 'plugin-link');
|
||||
|
||||
mkdirSync(target, { recursive: true });
|
||||
writeFileSync(join(target, 'marker.txt'), 'keep');
|
||||
symlinkSync(target, link, SYMLINK_TYPE);
|
||||
|
||||
safeRmSync(link);
|
||||
|
||||
expect(existsSync(link)).toBe(false);
|
||||
expect(existsSync(join(target, 'marker.txt'))).toBe(true);
|
||||
});
|
||||
|
||||
it('is a no-op when the path is already missing', () => {
|
||||
root = mkdtempSync(join(tmpdir(), 'clawx-safe-rm-missing-'));
|
||||
const missing = join(root, 'does-not-exist');
|
||||
|
||||
expect(() => safeRmSync(missing)).not.toThrow();
|
||||
});
|
||||
|
||||
it.runIf(process.platform === 'win32')('removes a directory tree through a Windows namespaced path', () => {
|
||||
root = mkdtempSync(join(tmpdir(), 'clawx-safe-rm-namespaced-'));
|
||||
const pluginDir = join(root, 'extensions', 'wecom');
|
||||
const namespacedPluginDir = toNamespacedPath(pluginDir);
|
||||
|
||||
mkdirSync(pluginDir, { recursive: true });
|
||||
writeFileSync(join(pluginDir, 'openclaw.plugin.json'), '{"id":"wecom"}');
|
||||
|
||||
expect(namespacedPluginDir).toMatch(/^\\\\\?\\/);
|
||||
expect(() => safeRmSync(namespacedPluginDir)).not.toThrow();
|
||||
expect(existsSync(pluginDir)).toBe(false);
|
||||
});
|
||||
|
||||
it('runs the junction regression test in the Windows CI job', () => {
|
||||
const projectRoot = join(import.meta.dirname, '..', '..');
|
||||
const workflow = readFileSync(join(projectRoot, '.github', 'workflows', 'check.yml'), 'utf8');
|
||||
const windowsJob = workflow.slice(workflow.indexOf(' build:'));
|
||||
|
||||
expect(windowsJob).toContain('tests/unit/safe-fs.test.ts');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user