From 48ba3a4198b88e56d5c35857540f5dbd0a545f76 Mon Sep 17 00:00:00 2001 From: Shakker Date: Sun, 26 Apr 2026 04:14:27 +0100 Subject: [PATCH] fix: clean migrated plugin install config --- CHANGELOG.md | 1 + docs/cli/plugins.md | 3 + src/config/io.ts | 163 +++++++++++++++++++++++++---- src/config/io.write-config.test.ts | 8 ++ 4 files changed, 153 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f134b0f7b8a..c0e7c7855f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -173,6 +173,7 @@ Docs: https://docs.openclaw.ai - Plugins/install: anchor bundled runtime-dependency npm installs with an OpenClaw-owned package manifest so Linux updates cannot accidentally write to a parent `$HOME/node_modules` tree. Fixes #71730. - Plugins/install: pass onboarding plugin config into plugin index writes so local plugin installs outside default discovery roots keep their install records. Thanks @shakkernerd. - Plugins/install: migrate shipped `plugins.installs` config records into the plugin index while stripping them from runtime config and future writes. Thanks @shakkernerd. +- Plugins/install: durably remove shipped `plugins.installs` from `openclaw.json` after its records are copied into the plugin index, while rolling back the index write if config cleanup fails. Thanks @shakkernerd. - Plugins/install: keep migrated plugin install records in the plugin index even when the plugin manifest is missing or invalid, so update, uninstall, inspect, and audit can still recover broken installs. Thanks @shakkernerd. - Plugins/security: keep plugin audit JSON check ids stable while reporting plugin index install-record findings with updated wording. Thanks @shakkernerd. - CLI/config: reject direct `plugins.installs` edits with guidance to use `openclaw plugins install`, `openclaw plugins update`, or `openclaw plugins uninstall` instead. Thanks @shakkernerd. diff --git a/docs/cli/plugins.md b/docs/cli/plugins.md index fa67e1f8638..a15947b032c 100644 --- a/docs/cli/plugins.md +++ b/docs/cli/plugins.md @@ -247,6 +247,9 @@ metadata, including records for broken or missing plugin manifests. The `plugins` array is the manifest-derived cold registry cache. The file includes a do-not-edit warning and is used by `openclaw plugins update`, uninstall, diagnostics, and the cold plugin registry. +When OpenClaw sees shipped legacy `plugins.installs` records in config, it moves +them into the plugin index and removes the config key; if either write fails, +the config records are kept so the install metadata is not lost. ### Uninstall diff --git a/src/config/io.ts b/src/config/io.ts index 26e1f6aab9d..3e9835d2175 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -136,6 +136,12 @@ type ShippedPluginInstallConfigWriteMigration = }; }; +type ShippedPluginInstallConfigReadMigration = { + config: unknown; + persistedRootParsed?: unknown; + persistedRootRaw?: string; +}; + const CONFIG_HEALTH_STATE_FILENAME = "config-health.json"; const loggedInvalidConfigs = new Set(); @@ -1228,21 +1234,104 @@ export function createConfigIO( return applyConfigOverrides(cfgWithOwnerDisplaySecret); } + function captureFileSnapshotSync(filePath: string): + | { + existed: false; + } + | { + existed: true; + raw: string; + } { + return deps.fs.existsSync(filePath) + ? ({ + existed: true, + raw: deps.fs.readFileSync(filePath, "utf-8"), + } as const) + : ({ existed: false } as const); + } + + function restoreFileSnapshotSync( + filePath: string, + previousFile: + | { + existed: false; + } + | { + existed: true; + raw: string; + }, + ): void { + if (previousFile.existed) { + deps.fs.writeFileSync(filePath, previousFile.raw, { + encoding: "utf-8", + mode: 0o600, + }); + return; + } + try { + deps.fs.unlinkSync(filePath); + } catch (err) { + if ((err as NodeJS.ErrnoException)?.code !== "ENOENT") { + throw err; + } + } + } + + function replaceConfigFileSync(raw: string): void { + const dir = path.dirname(configPath); + deps.fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + const tmp = path.join( + dir, + `${path.basename(configPath)}.${process.pid}.${crypto.randomUUID()}.tmp`, + ); + try { + deps.fs.writeFileSync(tmp, raw, { + encoding: "utf-8", + mode: 0o600, + }); + try { + deps.fs.renameSync(tmp, configPath); + } catch (err) { + const code = (err as NodeJS.ErrnoException)?.code; + if (code !== "EPERM" && code !== "EEXIST") { + throw err; + } + deps.fs.copyFileSync(tmp, configPath); + deps.fs.chmodSync(configPath, 0o600); + deps.fs.unlinkSync(tmp); + } + } catch (err) { + try { + deps.fs.unlinkSync(tmp); + } catch (cleanupErr) { + if ((cleanupErr as NodeJS.ErrnoException)?.code !== "ENOENT") { + deps.logger.warn(`Failed to clean temporary config file ${tmp}: ${String(cleanupErr)}`); + } + } + throw err; + } + } + function migrateAndStripShippedPluginInstallConfigRecords( configRaw: unknown, - options: { persist?: boolean } = {}, - ): unknown { + options: { persist?: boolean; rootConfigRaw?: unknown } = {}, + ): ShippedPluginInstallConfigReadMigration { const installRecords = extractShippedPluginInstallConfigRecords(configRaw); const stripped = stripShippedPluginInstallConfigRecords(configRaw); if (Object.keys(installRecords).length === 0) { - return stripped; + return { config: stripped }; } if (options.persist === false) { - return stripped; + return { config: stripped }; } try { const stateDir = resolveStateDir(deps.env, deps.homedir); + const filePath = resolveInstalledPluginIndexRecordsStorePath({ + env: deps.env, + stateDir, + }); + const previousFile = captureFileSnapshotSync(filePath); const existingRecords = loadInstalledPluginIndexInstallRecordsSync({ env: deps.env, stateDir, @@ -1258,16 +1347,33 @@ export function createConfigIO( stateDir, }); } + const rootConfigRaw = options.rootConfigRaw; + if ( + rootConfigRaw !== undefined && + Object.keys(extractShippedPluginInstallConfigRecords(rootConfigRaw)).length > 0 + ) { + const persistedRootParsed = stripShippedPluginInstallConfigRecords(rootConfigRaw); + const persistedRootRaw = JSON.stringify(persistedRootParsed, null, 2) + .trimEnd() + .concat("\n"); + try { + replaceConfigFileSync(persistedRootRaw); + } catch (err) { + restoreFileSnapshotSync(filePath, previousFile); + throw err; + } + return { config: stripped, persistedRootParsed, persistedRootRaw }; + } } catch (err) { deps.logger.warn( `Config (${configPath}): could not migrate shipped plugins.installs records into the plugin index: ${formatErrorMessage( err, )}`, ); - return configRaw; + return { config: configRaw }; } - return stripped; + return { config: stripped }; } function ensureShippedPluginInstallConfigRecordsMigratedForWrite( @@ -1374,16 +1480,20 @@ export function createConfigIO( }); const effectiveRaw = recovered.raw; const effectiveParsed = recovered.parsed; - const hash = hashConfigRaw(effectiveRaw); const readResolution = resolveConfigForRead( resolveConfigIncludesForRead(effectiveParsed, configPath, deps), deps.env, ); const resolvedConfig = readResolution.resolvedConfigRaw; const legacyResolution = resolveLegacyConfigForRead(resolvedConfig, effectiveParsed); - const effectiveConfigRaw = migrateAndStripShippedPluginInstallConfigRecords( + const installMigration = migrateAndStripShippedPluginInstallConfigRecords( legacyResolution.effectiveConfigRaw, + { rootConfigRaw: effectiveParsed }, ); + const effectiveConfigRaw = installMigration.config; + const snapshotRaw = installMigration.persistedRootRaw ?? effectiveRaw; + const snapshotParsed = installMigration.persistedRootParsed ?? effectiveParsed; + const hash = hashConfigRaw(snapshotRaw); for (const w of readResolution.envWarnings) { deps.logger.warn( `Config (${configPath}): missing env var "${w.varName}" at ${w.configPath} - feature using this value will be unavailable`, @@ -1395,8 +1505,8 @@ export function createConfigIO( ...createConfigFileSnapshot({ path: configPath, exists: true, - raw: effectiveRaw, - parsed: effectiveParsed, + raw: snapshotRaw, + parsed: snapshotParsed, sourceConfig: {}, valid: true, runtimeConfig: {}, @@ -1424,8 +1534,8 @@ export function createConfigIO( ...createConfigFileSnapshot({ path: configPath, exists: true, - raw: effectiveRaw, - parsed: effectiveParsed, + raw: snapshotRaw, + parsed: snapshotParsed, sourceConfig: coerceConfig(effectiveConfigRaw), valid: false, runtimeConfig: coerceConfig(effectiveConfigRaw), @@ -1457,8 +1567,8 @@ export function createConfigIO( ...createConfigFileSnapshot({ path: configPath, exists: true, - raw: effectiveRaw, - parsed: effectiveParsed, + raw: snapshotRaw, + parsed: snapshotParsed, sourceConfig: coerceConfig(effectiveConfigRaw), valid: true, runtimeConfig: cfg, @@ -1594,10 +1704,19 @@ export function createConfigIO( const resolvedConfigRaw = readResolution.resolvedConfigRaw; const legacyResolution = resolveLegacyConfigForRead(resolvedConfigRaw, effectiveParsed); - const effectiveConfigRaw = migrateAndStripShippedPluginInstallConfigRecords( + const installMigration = migrateAndStripShippedPluginInstallConfigRecords( legacyResolution.effectiveConfigRaw, - { persist: options.persistShippedPluginInstallMigration !== false }, + { + persist: options.persistShippedPluginInstallMigration !== false, + rootConfigRaw: effectiveParsed, + }, ); + const effectiveConfigRaw = installMigration.config; + const snapshotRaw = installMigration.persistedRootRaw ?? effectiveRaw; + const snapshotParsed = installMigration.persistedRootParsed ?? effectiveParsed; + const snapshotHash = installMigration.persistedRootRaw + ? hashConfigRaw(installMigration.persistedRootRaw) + : hash; fallbackSourceConfig = coerceConfig(effectiveConfigRaw); const validated = validateConfigObjectWithPlugins(effectiveConfigRaw, { env: deps.env, @@ -1608,12 +1727,12 @@ export function createConfigIO( snapshot: createConfigFileSnapshot({ path: configPath, exists: true, - raw: effectiveRaw, - parsed: effectiveParsed, + raw: snapshotRaw, + parsed: snapshotParsed, sourceConfig: coerceConfig(effectiveConfigRaw), valid: false, runtimeConfig: coerceConfig(effectiveConfigRaw), - hash, + hash: snapshotHash, issues: validated.issues, warnings: [...validated.warnings, ...envVarWarnings], legacyIssues: legacyResolution.sourceLegacyIssues, @@ -1627,14 +1746,14 @@ export function createConfigIO( snapshot: createConfigFileSnapshot({ path: configPath, exists: true, - raw: effectiveRaw, - parsed: effectiveParsed, + raw: snapshotRaw, + parsed: snapshotParsed, // Use resolvedConfigRaw (after $include and ${ENV} substitution but BEFORE runtime defaults) // for config set/unset operations (issue #6070) sourceConfig: coerceConfig(effectiveConfigRaw), valid: true, runtimeConfig: snapshotConfig, - hash, + hash: snapshotHash, issues: [], warnings: [...validated.warnings, ...envVarWarnings], legacyIssues: legacyResolution.sourceLegacyIssues, diff --git a/src/config/io.write-config.test.ts b/src/config/io.write-config.test.ts index 36db1e1a041..cd04d5b6e50 100644 --- a/src/config/io.write-config.test.ts +++ b/src/config/io.write-config.test.ts @@ -181,6 +181,10 @@ describe("config io write", () => { }), ], }); + const persistedConfig = JSON.parse(await fs.readFile(configPath, "utf-8")) as { + plugins?: { installs?: unknown }; + }; + expect(persistedConfig.plugins?.installs).toBeUndefined(); } finally { mockLoadPluginManifestRegistry.mockReturnValue({ diagnostics: [], @@ -234,6 +238,10 @@ describe("config io write", () => { }, plugins: [], }); + const persistedConfig = JSON.parse(await fs.readFile(configPath, "utf-8")) as { + plugins?: { installs?: unknown }; + }; + expect(persistedConfig.plugins?.installs).toBeUndefined(); }); });