From 680001bec4d44d671eae408b18b8b89b905bd15e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 21 Aug 2026 15:35:31 -0700 Subject: [PATCH] fix(gateway): fix plugin metadata lifecycle staleness in auth-bypass cache and config reload (#127664) The gateway HTTP auth-bypass path cache was keyed only by config object identity, so replacing a channel plugin's declared bypass contract while config identity stayed stable preserved obsolete unauthenticated HTTP routes. It now clears on plugin metadata lifecycle resets alongside every other plugin-derived process memo. notifyPluginMetadataChanged cleared the process plugin metadata snapshot slot, but when config bytes were unchanged the config-reload diff took its early no-op return and skipped both plugin reload and republishing, leaving the snapshot slot empty so configless readers cold-scanned repeatedly. An unchanged-bytes metadata signal now forces a plugin-reload plan so the runtime generation republishes. --- src/gateway/config-reload.test.ts | 48 ++++++++++++ src/gateway/config-reload.ts | 36 ++++++++- src/gateway/server-http-plugin-auth.test.ts | 86 +++++++++++++++++++++ src/gateway/server-http-plugin-auth.ts | 23 ++++-- 4 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 src/gateway/server-http-plugin-auth.test.ts diff --git a/src/gateway/config-reload.test.ts b/src/gateway/config-reload.test.ts index 362c8c42b6ad..cbf549334333 100644 --- a/src/gateway/config-reload.test.ts +++ b/src/gateway/config-reload.test.ts @@ -4507,6 +4507,54 @@ describe("startGatewayConfigReloader", () => { await harness.reloader.stop(); }); + it("forces a plugin reload when signaled metadata leaves config and install records identical", async () => { + const activeConfig: OpenClawConfig = { + gateway: { reload: {} }, + }; + const installRecords = { + brave: { + source: "npm", + spec: "@openclaw/brave", + installPath: "/tmp/openclaw/plugins/brave", + }, + } satisfies Record; + const readSnapshot = vi.fn(async () => + makeSnapshot({ + sourceConfig: activeConfig, + runtimeConfig: activeConfig, + config: activeConfig, + hash: "unchanged-config", + }), + ); + const readPluginInstallRecords = vi.fn(async () => ({ ...installRecords })); + const harness = createReloaderHarness(readSnapshot, { + initialConfig: activeConfig, + initialCompareConfig: activeConfig, + initialPluginInstallRecords: installRecords, + readPluginInstallRecords, + }); + + harness.reloader.notifyPluginMetadataChanged(); + await vi.runOnlyPendingTimersAsync(); + + expect(harness.onRestart).not.toHaveBeenCalled(); + const [plan, nextConfig] = getOnlyHotReloadCall(harness); + expect(plan.changedPaths).toEqual([]); + expect(plan.restartGateway).toBe(false); + expect(plan.reloadPlugins).toBe(true); + expect(plan.disposeMcpRuntimes).toBe(true); + expect(nextConfig).toBe(activeConfig); + + // The refresh is consumed by the committed reload: a later watcher echo of + // identical bytes must not replace the plugin runtime generation again. + harness.watcher.emit("change"); + await vi.runOnlyPendingTimersAsync(); + expect(harness.onHotReload).toHaveBeenCalledTimes(1); + expect(harness.onRestart).not.toHaveBeenCalled(); + + await harness.reloader.stop(); + }); + it("keeps external plugin policy-only writes on the hot reload path", async () => { const previousConfig: OpenClawConfig = { gateway: { reload: {} }, diff --git a/src/gateway/config-reload.ts b/src/gateway/config-reload.ts index 0ea1c6939b4f..223b2df250e6 100644 --- a/src/gateway/config-reload.ts +++ b/src/gateway/config-reload.ts @@ -265,6 +265,11 @@ export function startGatewayConfigReloader(opts: { const activeReloads = new Set>(); let missingConfigRetries = 0; let configWriteEpoch = 0; + // Signaled metadata changes clear the process snapshot slot before the diff + // pass runs; the counters keep that pass honest when config bytes are + // unchanged, and stay pending until a plugin reload or restart commits. + let pluginMetadataRefreshRequests = 0; + let pluginMetadataRefreshApplied = 0; let pendingInProcessConfig: InProcessConfigCandidate | null = null; let activeInProcessConfig: InProcessConfigCandidate | null = null; let watcherIntentCandidate: InProcessConfigCandidate | null = null; @@ -644,7 +649,16 @@ export function startGatewayConfigReloader(opts: { } notifyCommitted(); }; - if (changedPaths.length === 0) { + // A signaled metadata change emptied the process snapshot slot. An + // unchanged config diff must still replace the plugin runtime generation so + // the slot republishes instead of leaving configless readers cold-scanning + // against a registry that diverged from the live runtime owners. + const pluginMetadataRefreshToken = pluginMetadataRefreshRequests; + const forcePluginMetadataReload = pluginMetadataRefreshToken !== pluginMetadataRefreshApplied; + const markPluginMetadataRefreshApplied = () => { + pluginMetadataRefreshApplied = pluginMetadataRefreshToken; + }; + if (changedPaths.length === 0 && !forcePluginMetadataReload) { let publishedSource: { rollback: () => Promise; commit?: () => void } | undefined; let publishedSourceRollback: (() => Promise) | undefined; let publishedSourceRolledBack = false; @@ -681,7 +695,11 @@ export function startGatewayConfigReloader(opts: { } const followUp = resolveConfigWriteFollowUp(afterWrite); - opts.log.info(`config change detected; evaluating reload (${changedPaths.join(", ")})`); + opts.log.info( + changedPaths.length > 0 + ? `config change detected; evaluating reload (${changedPaths.join(", ")})` + : "plugin metadata changed with identical config; replacing plugin runtime generation", + ); if (followUp.mode === "none") { opts.log.info(`config reload skipped by writer intent (${followUp.reason})`); await commitReloadBaseline({ runtimeApplied: false }); @@ -692,6 +710,12 @@ export function startGatewayConfigReloader(opts: { forceChangedPaths: pluginInstallWholeRecordPaths, candidateConfig: nextConfig, }); + if (forcePluginMetadataReload && !plan.restartGateway && !plan.reloadPlugins) { + // Mirror the `plugins.*` hot rule pairing: a replaced plugin registry + // also invalidates MCP runtimes assembled from the previous generation. + plan.reloadPlugins = true; + plan.disposeMcpRuntimes = true; + } if (nextSettings.mode === "off") { opts.log.info("config reload disabled (gateway.reload.mode=off)"); await commitReloadBaseline({ runtimeApplied: false }); @@ -716,12 +740,15 @@ export function startGatewayConfigReloader(opts: { await opts.onConfigChange?.(restartPlan, nextConfig); await prepareRestart(restartPlan, nextConfig, ownership, nextSourceConfig); await commitReloadBaseline(); + // The accepted restart owns snapshot republication at next startup. + markPluginMetadataRefreshApplied(); return; } if (plan.restartGateway) { await opts.onConfigChange?.(plan, nextConfig); await prepareRestart(plan, nextConfig, ownership, nextSourceConfig); await commitReloadBaseline(); + markPluginMetadataRefreshApplied(); return; } @@ -735,6 +762,10 @@ export function startGatewayConfigReloader(opts: { assertCurrent(); await appliedRevision.apply(plan, nextConfig, nextConfigRevisionHash); await commitReloadBaseline(); + if (plan.reloadPlugins) { + // The committed reload republished the metadata snapshot generation. + markPluginMetadataRefreshApplied(); + } }; const promoteAcceptedSnapshot = async (snapshot: ConfigFileSnapshot, reason: string) => { @@ -1271,6 +1302,7 @@ export function startGatewayConfigReloader(opts: { notifyPluginMetadataChanged: () => { // The signal carries a metadata change while config bytes stay identical. // Clear both metadata and config-echo caches before scheduling the shared diff path. + pluginMetadataRefreshRequests += 1; clearLoadInstalledPluginIndexInstallRecordsCache(); clearPluginMetadataLifecycleCaches(); startupInternalWriteHash = null; diff --git a/src/gateway/server-http-plugin-auth.test.ts b/src/gateway/server-http-plugin-auth.test.ts new file mode 100644 index 000000000000..adc67f3ef0e4 --- /dev/null +++ b/src/gateway/server-http-plugin-auth.test.ts @@ -0,0 +1,86 @@ +// Covers plugin gateway auth bypass caching across metadata lifecycle resets. +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { clearPluginMetadataLifecycleCaches } from "../plugins/plugin-metadata-lifecycle.js"; +import { getCachedPluginGatewayAuthBypassPaths } from "./server-http-plugin-auth.js"; + +const resolveBypassPaths = vi.hoisted(() => + vi.fn<(params: { channelId: string; cfg: OpenClawConfig }) => Promise>(), +); + +vi.mock("../channels/plugins/gateway-auth-bypass.js", () => ({ + resolveBundledChannelGatewayAuthBypassPaths: resolveBypassPaths, +})); + +describe("getCachedPluginGatewayAuthBypassPaths", () => { + beforeEach(() => { + resolveBypassPaths.mockReset(); + clearPluginMetadataLifecycleCaches(); + }); + + it("caches resolved bypass paths per config identity", async () => { + const config: OpenClawConfig = { channels: { telegram: {} } }; + resolveBypassPaths.mockResolvedValue(["/telegram/webhook"]); + + await expect(getCachedPluginGatewayAuthBypassPaths(config)).resolves.toEqual( + new Set(["/telegram/webhook"]), + ); + await getCachedPluginGatewayAuthBypassPaths(config); + expect(resolveBypassPaths).toHaveBeenCalledTimes(1); + }); + + it("drops cached bypass paths on a metadata lifecycle reset despite stable config identity", async () => { + const config: OpenClawConfig = { channels: { telegram: {} } }; + resolveBypassPaths.mockResolvedValueOnce(["/telegram/old-bypass"]); + await expect(getCachedPluginGatewayAuthBypassPaths(config)).resolves.toEqual( + new Set(["/telegram/old-bypass"]), + ); + + // Same config object, replaced plugin contract: the reset must invalidate, + // or the predecessor's unauthenticated paths keep bypassing gateway auth. + clearPluginMetadataLifecycleCaches(); + resolveBypassPaths.mockResolvedValueOnce(["/telegram/new-bypass"]); + + await expect(getCachedPluginGatewayAuthBypassPaths(config)).resolves.toEqual( + new Set(["/telegram/new-bypass"]), + ); + }); + + it("retries after a failed resolution instead of caching the rejection", async () => { + const config: OpenClawConfig = { channels: { telegram: {} } }; + resolveBypassPaths.mockRejectedValueOnce(new Error("resolution failed")); + await expect(getCachedPluginGatewayAuthBypassPaths(config)).rejects.toThrow( + "resolution failed", + ); + + resolveBypassPaths.mockResolvedValueOnce(["/telegram/webhook"]); + await expect(getCachedPluginGatewayAuthBypassPaths(config)).resolves.toEqual( + new Set(["/telegram/webhook"]), + ); + }); + + it("keeps the fresh generation cached when a stale failed resolution settles late", async () => { + const config: OpenClawConfig = { channels: { telegram: {} } }; + let rejectStale: (error: Error) => void = () => {}; + resolveBypassPaths.mockReturnValueOnce( + new Promise((_resolve, reject) => { + rejectStale = reject; + }), + ); + const stale = getCachedPluginGatewayAuthBypassPaths(config); + + clearPluginMetadataLifecycleCaches(); + resolveBypassPaths.mockResolvedValue(["/telegram/webhook"]); + await expect(getCachedPluginGatewayAuthBypassPaths(config)).resolves.toEqual( + new Set(["/telegram/webhook"]), + ); + + rejectStale(new Error("stale plugin generation failed")); + await expect(stale).rejects.toThrow("stale plugin generation failed"); + + // The stale rejection evicts from its own generation only; the fresh entry + // must stay cached instead of being re-resolved. + await getCachedPluginGatewayAuthBypassPaths(config); + expect(resolveBypassPaths).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/gateway/server-http-plugin-auth.ts b/src/gateway/server-http-plugin-auth.ts index a9b5a677d03f..e9bed61a915e 100644 --- a/src/gateway/server-http-plugin-auth.ts +++ b/src/gateway/server-http-plugin-auth.ts @@ -1,5 +1,6 @@ import { resolveBundledChannelGatewayAuthBypassPaths } from "../channels/plugins/gateway-auth-bypass.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { registerPluginMetadataProcessMemoLifecycleClear } from "../plugins/plugin-metadata-lifecycle.js"; import type { AuthorizedGatewayHttpRequest } from "./http-auth-utils.js"; import type { PluginNodeCapabilitySurface } from "./plugin-node-capability.js"; import { @@ -18,10 +19,15 @@ export type ResolvePluginNodeCapabilityRoute = ( pathContext: PluginRoutePathContext, ) => PluginNodeCapabilitySurface | undefined; -const pluginGatewayAuthBypassPathsCache = new WeakMap< - OpenClawConfig, - Promise> ->(); +// Bypass paths come from plugin-declared artifacts, not config bytes alone. A +// metadata lifecycle reset can replace that contract while the config object +// identity stays stable, so the cache must roll to a fresh generation on reset +// or a replaced channel plugin keeps its predecessor's HTTP auth exceptions. +let pluginGatewayAuthBypassPathsCache = new WeakMap>>(); + +registerPluginMetadataProcessMemoLifecycleClear(() => { + pluginGatewayAuthBypassPathsCache = new WeakMap(); +}); async function resolvePluginGatewayAuthBypassPaths( configSnapshot: OpenClawConfig, @@ -45,15 +51,18 @@ async function resolvePluginGatewayAuthBypassPaths( export function getCachedPluginGatewayAuthBypassPaths( configSnapshot: OpenClawConfig, ): Promise> { - const cached = pluginGatewayAuthBypassPathsCache.get(configSnapshot); + const cache = pluginGatewayAuthBypassPathsCache; + const cached = cache.get(configSnapshot); if (cached) { return cached; } const resolved = resolvePluginGatewayAuthBypassPaths(configSnapshot).catch((error: unknown) => { - pluginGatewayAuthBypassPathsCache.delete(configSnapshot); + // Evict from the owning generation only; a stale failure settling after a + // lifecycle reset must not drop a freshly cached entry. + cache.delete(configSnapshot); throw error; }); - pluginGatewayAuthBypassPathsCache.set(configSnapshot, resolved); + cache.set(configSnapshot, resolved); return resolved; }