mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
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.
This commit is contained in:
committed by
GitHub
parent
6d47044e73
commit
680001bec4
@@ -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<string, PluginInstallRecord>;
|
||||
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: {} },
|
||||
|
||||
@@ -265,6 +265,11 @@ export function startGatewayConfigReloader(opts: {
|
||||
const activeReloads = new Set<Promise<void>>();
|
||||
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<void>; commit?: () => void } | undefined;
|
||||
let publishedSourceRollback: (() => Promise<void>) | 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;
|
||||
|
||||
@@ -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<string[]>>(),
|
||||
);
|
||||
|
||||
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<string[]>((_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);
|
||||
});
|
||||
});
|
||||
@@ -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<ReadonlySet<string>>
|
||||
>();
|
||||
// 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<OpenClawConfig, Promise<ReadonlySet<string>>>();
|
||||
|
||||
registerPluginMetadataProcessMemoLifecycleClear(() => {
|
||||
pluginGatewayAuthBypassPathsCache = new WeakMap();
|
||||
});
|
||||
|
||||
async function resolvePluginGatewayAuthBypassPaths(
|
||||
configSnapshot: OpenClawConfig,
|
||||
@@ -45,15 +51,18 @@ async function resolvePluginGatewayAuthBypassPaths(
|
||||
export function getCachedPluginGatewayAuthBypassPaths(
|
||||
configSnapshot: OpenClawConfig,
|
||||
): Promise<ReadonlySet<string>> {
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user