fix(cli): harden official plugin recovery (#93325)

* fix(cli): harden official plugin recovery * fix(config): preserve include write context * fix(config): reject external include mutations * fix(config): bind snapshots to config paths * fix(config): preserve write ownership * fix(cli): preflight plugin config mutations * chore(plugin-sdk): refresh api baseline * test(config): prove install env policy mutations * fix(cli): preflight plugin updates * fix(cli): preflight non-npm id migrations * chore(plugin-sdk): refresh api baseline * fix(cli): satisfy plugin recovery checks
2026-06-29 03:43:36 +00:00 · 2026-06-15 23:07:29 +08:00
parent c1219d161d
commit 767e8280ac
39 changed files with 9380 additions and 898 deletions
--- a/src/cli/plugins-update-command.ts
+++ b/src/cli/plugins-update-command.ts
@@ -3,17 +3,32 @@ import { theme } from "../../packages/terminal-core/src/theme.js";
 import {
  assertConfigWriteAllowedInCurrentMode,
  getRuntimeConfig,
-  readConfigFileSnapshot,
+  readConfigFileSnapshotForWrite,
  replaceConfigFile,
 } from "../config/config.js";
+import { createMergePatch } from "../config/io.write-prepare.js";
+import { applyMergePatch } from "../config/merge-patch.js";
+import { extractShippedPluginInstallConfigRecords } from "../config/plugin-install-config-migration.js";
+import type { OpenClawConfig } from "../config/types.openclaw.js";
+import type { PluginInstallRecord } from "../config/types.plugins.js";
 import { updateNpmInstalledHookPacks } from "../hooks/update.js";
 import {
  loadInstalledPluginIndexInstallRecords,
  withoutPluginInstallRecords,
  withPluginInstallRecords,
 } from "../plugins/installed-plugin-index-records.js";
-import { updateNpmInstalledPlugins } from "../plugins/update.js";
+import {
+  isPluginInstallRecordUpdateSource,
+  pluginInstallRecordMayMigrateConfigId,
+  updateNpmInstalledPlugins,
+} from "../plugins/update.js";
 import { defaultRuntime } from "../runtime.js";
+import {
+  containsConfigIncludeDirective,
+  resolveCombinedPluginAndHookConfigMutationPreflight,
+  resolveInstallConfigMutationPreflights,
+  selectInstallMutationWriteOptions,
+} from "./plugins-install-persist.js";
 import { commitPluginInstallRecordsWithConfig } from "./plugins-install-record-commit.js";
 import { refreshPluginRegistryAfterConfigMutation } from "./plugins-registry-refresh.js";
 import { logPluginUpdateOutcomes } from "./plugins-update-outcomes.js";
@@ -26,6 +41,62 @@ import { promptYesNo } from "./prompt.js";
 const DEPRECATED_DANGEROUS_FORCE_UNSAFE_UPDATE_WARNING =
  "--dangerously-force-unsafe-install is deprecated and no longer affects plugin updates because built-in install-time dangerous-code scanning has been removed. Configure security.installPolicy for operator-owned install decisions.";

+function mayMutatePluginInstallRecord(
+  record: PluginInstallRecord | undefined,
+  specOverride: string | undefined,
+): boolean {
+  if (!isPluginInstallRecordUpdateSource(record)) {
+    return false;
+  }
+  if (record?.source === "npm") {
+    return Boolean(specOverride ?? record.spec);
+  }
+  if (record?.source === "git") {
+    return Boolean(record.spec);
+  }
+  if (record?.source === "clawhub") {
+    return Boolean(record.clawhubPackage);
+  }
+  return Boolean(record?.marketplaceSource && record.marketplacePlugin);
+}
+
+function pluginConfigReferencesId(config: ReturnType<typeof getRuntimeConfig>, pluginId: string) {
+  const plugins = config.plugins;
+  return (
+    plugins?.allow?.includes(pluginId) ||
+    plugins?.deny?.includes(pluginId) ||
+    Object.hasOwn(plugins?.entries ?? {}, pluginId) ||
+    plugins?.slots?.memory === pluginId ||
+    plugins?.slots?.contextEngine === pluginId
+  );
+}
+
+function shouldPreserveEmptyPlugins(params: {
+  parsed: unknown;
+  sourceConfig: ReturnType<typeof getRuntimeConfig>;
+}): boolean {
+  const plugins = params.sourceConfig.plugins;
+  const parsedPlugins =
+    params.parsed && typeof params.parsed === "object" && !Array.isArray(params.parsed)
+      ? (params.parsed as Record<string, unknown>).plugins
+      : undefined;
+  return Boolean(
+    plugins &&
+    (!Object.hasOwn(plugins, "installs") ||
+      Object.keys(plugins).some((key) => key !== "installs") ||
+      containsConfigIncludeDirective(parsedPlugins)),
+  );
+}
+
+function projectUpdaterResultOntoSourceConfig(params: {
+  runtimeBase: OpenClawConfig;
+  sourceBase: OpenClawConfig;
+  updatedConfig: OpenClawConfig;
+}): OpenClawConfig {
+  const updatePatch = createMergePatch(params.runtimeBase, params.updatedConfig);
+  return applyMergePatch(params.sourceBase, updatePatch) as OpenClawConfig;
+}
+
 /** Run plugin/hook-pack updates, persist changed install records, and refresh runtime registry. */
 export async function runPluginUpdateCommand(params: {
  id?: string;
@@ -33,10 +104,42 @@ export async function runPluginUpdateCommand(params: {
 }) {
  assertConfigWriteAllowedInCurrentMode();

-  const sourceSnapshotPromise = readConfigFileSnapshot().catch(() => null);
-  const cfg = getRuntimeConfig();
-  const pluginInstallRecords = await loadInstalledPluginIndexInstallRecords();
+  const sourceSnapshotPromise = readConfigFileSnapshotForWrite()
+    .then((prepared) => ({
+      ...prepared,
+      writeOptions: selectInstallMutationWriteOptions(prepared.writeOptions),
+    }))
+    .catch(() => null);
+  const mutationSnapshot = params.opts.dryRun ? null : await sourceSnapshotPromise;
+  if (!params.opts.dryRun && !mutationSnapshot) {
+    defaultRuntime.error("Could not inspect config ownership before updating plugins or hooks.");
+    return defaultRuntime.exit(1);
+  }
+  if (mutationSnapshot && !mutationSnapshot.snapshot.valid) {
+    defaultRuntime.error("Cannot update plugins or hooks while the config is invalid.");
+    return defaultRuntime.exit(1);
+  }
+  // Bind selection, updater input, ownership checks, and persistence to one
+  // mutation-start snapshot so concurrent config changes cannot be resurrected.
+  const cfg = mutationSnapshot?.snapshot.runtimeConfig ?? getRuntimeConfig();
+  const sourceCfg = mutationSnapshot?.snapshot.sourceConfig ?? cfg;
+  const shippedPluginInstallRecords = mutationSnapshot
+    ? {
+        ...extractShippedPluginInstallConfigRecords(mutationSnapshot.snapshot.parsed),
+        ...extractShippedPluginInstallConfigRecords(mutationSnapshot.snapshot.sourceConfig),
+      }
+    : extractShippedPluginInstallConfigRecords(cfg);
+  const persistedPluginInstallRecords = await loadInstalledPluginIndexInstallRecords();
+  // Persisted index records win over shipped legacy config during migration.
+  const pluginInstallRecords = {
+    ...shippedPluginInstallRecords,
+    ...persistedPluginInstallRecords,
+  };
  const cfgWithPluginInstallRecords = withPluginInstallRecords(cfg, pluginInstallRecords);
+  const sourceCfgWithPluginInstallRecords = withPluginInstallRecords(
+    sourceCfg,
+    pluginInstallRecords,
+  );
  const logger = {
    info: (msg: string) => defaultRuntime.log(msg),
    warn: (msg: string) => defaultRuntime.log(theme.warn(msg)),
@@ -64,49 +167,143 @@ export async function runPluginUpdateCommand(params: {
    return defaultRuntime.exit(1);
  }

-  const pluginResult = await updateNpmInstalledPlugins({
-    config: cfgWithPluginInstallRecords,
-    pluginIds: pluginSelection.pluginIds,
-    specOverrides: pluginSelection.specOverrides,
-    dryRun: params.opts.dryRun,
-    dangerouslyForceUnsafeInstall: params.opts.dangerouslyForceUnsafeInstall,
-    logger,
-    onIntegrityDrift: async (drift) => {
-      const specLabel = drift.resolvedSpec ?? drift.spec;
-      defaultRuntime.log(
-        theme.warn(
-          `Integrity drift detected for "${drift.pluginId}" (${specLabel})` +
-            `\nExpected: ${drift.expectedIntegrity}` +
-            `\nActual:   ${drift.actualIntegrity}`,
-        ),
+  const selectedHooks = cfg.hooks?.internal?.installs ?? {};
+  const pluginUpdateMayMutate =
+    !params.opts.dryRun &&
+    pluginSelection.pluginIds.some((pluginId) => {
+      return mayMutatePluginInstallRecord(
+        pluginInstallRecords[pluginId],
+        pluginSelection.specOverrides?.[pluginId],
      );
-      if (drift.dryRun) {
-        return true;
-      }
-      return await promptYesNo(`Continue updating "${drift.pluginId}" with this artifact?`);
-    },
-  });
-  const hookResult = await updateNpmInstalledHookPacks({
-    config: pluginResult.config,
-    hookIds: hookSelection.hookIds,
-    specOverrides: hookSelection.specOverrides,
-    dryRun: params.opts.dryRun,
-    logger,
-    onIntegrityDrift: async (drift) => {
-      const specLabel = drift.resolvedSpec ?? drift.spec;
-      defaultRuntime.log(
-        theme.warn(
-          `Integrity drift detected for hook pack "${drift.hookId}" (${specLabel})` +
-            `\nExpected: ${drift.expectedIntegrity}` +
-            `\nActual:   ${drift.actualIntegrity}`,
-        ),
+    });
+  const hookUpdateMayMutate =
+    !params.opts.dryRun &&
+    hookSelection.hookIds.some((hookId) => {
+      const record = selectedHooks[hookId];
+      return (
+        record?.source === "npm" && Boolean(hookSelection.specOverrides?.[hookId] ?? record.spec)
      );
-      if (drift.dryRun) {
-        return true;
+    });
+  if (pluginUpdateMayMutate || hookUpdateMayMutate) {
+    if (!mutationSnapshot) {
+      defaultRuntime.error("Could not inspect config ownership before updating plugins or hooks.");
+      return defaultRuntime.exit(1);
+    }
+    const { hookMutation, pluginMutation } = resolveInstallConfigMutationPreflights({
+      parsed: (mutationSnapshot.snapshot.parsed ?? {}) as Record<string, unknown>,
+      snapshotPath: mutationSnapshot.snapshot.path,
+      writeOptions: mutationSnapshot.writeOptions,
+    });
+    // Write snapshots retain valid shipped install records in sourceConfig after
+    // include resolution; parsed also catches root-authored legacy records.
+    const pluginRecordCleanupMayMutate =
+      Object.keys(extractShippedPluginInstallConfigRecords(mutationSnapshot.snapshot.sourceConfig))
+        .length > 0 ||
+      Object.keys(extractShippedPluginInstallConfigRecords(mutationSnapshot.snapshot.parsed))
+        .length > 0;
+    const parsedConfig =
+      mutationSnapshot.snapshot.parsed &&
+      typeof mutationSnapshot.snapshot.parsed === "object" &&
+      !Array.isArray(mutationSnapshot.snapshot.parsed)
+        ? (mutationSnapshot.snapshot.parsed as Record<string, unknown>)
+        : {};
+    const pluginReferencesMayBeUnresolved =
+      Object.hasOwn(parsedConfig, "$include") ||
+      containsConfigIncludeDirective(mutationSnapshot.snapshot.sourceConfig.plugins);
+    const pluginIdMigrationMayMutate = pluginSelection.pluginIds.some((pluginId) => {
+      return (
+        pluginInstallRecordMayMigrateConfigId({
+          pluginId,
+          record: pluginInstallRecords[pluginId],
+          specOverride: pluginSelection.specOverrides?.[pluginId],
+        }) &&
+        (pluginReferencesMayBeUnresolved ||
+          pluginConfigReferencesId(mutationSnapshot.snapshot.sourceConfig, pluginId))
+      );
+    });
+    // Manual update records stay in the index unless shipped-record cleanup or
+    // scoped-package compatibility migrates authored references from a legacy id.
+    const pluginConfigMayMutate = pluginRecordCleanupMayMutate || pluginIdMigrationMayMutate;
+    const blockedReasons = new Set<string>();
+    if (pluginConfigMayMutate && pluginMutation.mode === "blocked") {
+      blockedReasons.add(pluginMutation.reason);
+    }
+    if (hookUpdateMayMutate && hookMutation.mode === "blocked") {
+      blockedReasons.add(hookMutation.reason);
+    }
+    if (
+      pluginConfigMayMutate &&
+      hookUpdateMayMutate &&
+      pluginMutation.mode === "allowed" &&
+      hookMutation.mode === "allowed"
+    ) {
+      // Config persistence can commit one include-owned top-level section, not
+      // a mixed plugin-and-hook mutation spanning root and include ownership.
+      const combinedMutation = resolveCombinedPluginAndHookConfigMutationPreflight({
+        parsed: (mutationSnapshot.snapshot.parsed ?? {}) as Record<string, unknown>,
+        snapshotPath: mutationSnapshot.snapshot.path,
+      });
+      if (combinedMutation.mode === "blocked") {
+        blockedReasons.add(combinedMutation.reason);
      }
-      return await promptYesNo(`Continue updating hook pack "${drift.hookId}" with this artifact?`);
-    },
-  });
+    }
+    if (blockedReasons.size > 0) {
+      defaultRuntime.error(Array.from(blockedReasons).join(" "));
+      return defaultRuntime.exit(1);
+    }
+  }
+
+  const pluginResult =
+    pluginSelection.pluginIds.length > 0
+      ? await updateNpmInstalledPlugins({
+          config: cfgWithPluginInstallRecords,
+          pluginIds: pluginSelection.pluginIds,
+          specOverrides: pluginSelection.specOverrides,
+          dryRun: params.opts.dryRun,
+          dangerouslyForceUnsafeInstall: params.opts.dangerouslyForceUnsafeInstall,
+          logger,
+          onIntegrityDrift: async (drift) => {
+            const specLabel = drift.resolvedSpec ?? drift.spec;
+            defaultRuntime.log(
+              theme.warn(
+                `Integrity drift detected for "${drift.pluginId}" (${specLabel})` +
+                  `\nExpected: ${drift.expectedIntegrity}` +
+                  `\nActual:   ${drift.actualIntegrity}`,
+              ),
+            );
+            if (drift.dryRun) {
+              return true;
+            }
+            return await promptYesNo(`Continue updating "${drift.pluginId}" with this artifact?`);
+          },
+        })
+      : { config: cfgWithPluginInstallRecords, changed: false, outcomes: [] };
+  const hookResult =
+    hookSelection.hookIds.length > 0
+      ? await updateNpmInstalledHookPacks({
+          config: pluginResult.config,
+          hookIds: hookSelection.hookIds,
+          specOverrides: hookSelection.specOverrides,
+          dryRun: params.opts.dryRun,
+          logger,
+          onIntegrityDrift: async (drift) => {
+            const specLabel = drift.resolvedSpec ?? drift.spec;
+            defaultRuntime.log(
+              theme.warn(
+                `Integrity drift detected for hook pack "${drift.hookId}" (${specLabel})` +
+                  `\nExpected: ${drift.expectedIntegrity}` +
+                  `\nActual:   ${drift.actualIntegrity}`,
+              ),
+            );
+            if (drift.dryRun) {
+              return true;
+            }
+            return await promptYesNo(
+              `Continue updating hook pack "${drift.hookId}" with this artifact?`,
+            );
+          },
+        })
+      : { config: pluginResult.config, changed: false, outcomes: [] };

  const outcomeSummary = logPluginUpdateOutcomes({
    outcomes: [...pluginResult.outcomes, ...hookResult.outcomes],
@@ -114,27 +311,39 @@ export async function runPluginUpdateCommand(params: {
  });

  if (!params.opts.dryRun && (pluginResult.changed || hookResult.changed)) {
+    const sourceSnapshot = mutationSnapshot ?? (await sourceSnapshotPromise);
    const nextPluginInstallRecords = pluginResult.config.plugins?.installs ?? {};
    const shouldPersistPluginInstallIndex =
      pluginResult.changed || Object.keys(pluginInstallRecords).length > 0;
-    // Plugin install records live in the persisted index; config only carries hook-pack changes.
-    const nextConfig = shouldPersistPluginInstallIndex
-      ? withoutPluginInstallRecords(hookResult.config)
-      : hookResult.config;
+    const sourceShapedUpdateConfig = projectUpdaterResultOntoSourceConfig({
+      runtimeBase: cfgWithPluginInstallRecords,
+      sourceBase: sourceCfgWithPluginInstallRecords,
+      updatedConfig: hookResult.config,
+    });
+    // Plugin install records live in the persisted index. Preserve an authored
+    // empty plugins section so include ownership does not become a false mutation.
+    const nextConfig = withoutPluginInstallRecords(sourceShapedUpdateConfig, {
+      preserveEmptyPlugins: shouldPreserveEmptyPlugins({
+        parsed: sourceSnapshot?.snapshot.parsed,
+        sourceConfig: sourceSnapshot?.snapshot.sourceConfig ?? {},
+      }),
+    });
    if (shouldPersistPluginInstallIndex) {
      await commitPluginInstallRecordsWithConfig({
-        previousInstallRecords: pluginInstallRecords,
+        previousInstallRecords: persistedPluginInstallRecords,
        nextInstallRecords: nextPluginInstallRecords,
        nextConfig,
-        baseHash: (await sourceSnapshotPromise)?.hash,
+        baseHash: sourceSnapshot?.snapshot.hash,
        writeOptions: {
+          ...sourceSnapshot?.writeOptions,
          afterWrite: { mode: "restart", reason: "plugin source changed" },
        },
      });
    } else {
      await replaceConfigFile({
        nextConfig,
-        baseHash: (await sourceSnapshotPromise)?.hash,
+        baseHash: sourceSnapshot?.snapshot.hash,
+        writeOptions: sourceSnapshot?.writeOptions,
      });
    }
    if (pluginResult.changed) {