From c14d2b0c1f127e7b6c4a25150580ea5cee9972c3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 26 Apr 2026 03:17:27 +0100 Subject: [PATCH] fix: harden plugin registry contribution lookup --- src/agents/live-model-filter.test.ts | 42 ++++------ src/agents/live-model-filter.ts | 25 +++--- .../models-config.providers.implicit.ts | 6 +- ...odels-config.providers.live-filter.test.ts | 19 ++--- ....providers.plugin-allowlist-compat.test.ts | 77 ++++++++++++++----- src/commands/channel-setup/discovery.test.ts | 2 + src/commands/channel-setup/discovery.ts | 10 ++- src/commands/status-runtime-shared.test.ts | 5 +- src/plugins/manifest-registry-installed.ts | 14 +++- src/plugins/providers.ts | 12 +-- 10 files changed, 132 insertions(+), 80 deletions(-) diff --git a/src/agents/live-model-filter.test.ts b/src/agents/live-model-filter.test.ts index 4f1c449882f..090903519ec 100644 --- a/src/agents/live-model-filter.test.ts +++ b/src/agents/live-model-filter.test.ts @@ -1,32 +1,24 @@ -import { beforeEach, describe, expect, it } from "vitest"; -import { clearPluginDiscoveryCache } from "../plugins/discovery.js"; -import { clearPluginManifestRegistryCache } from "../plugins/manifest-registry.js"; +import { describe, expect, it } from "vitest"; import { shouldExcludeProviderFromDefaultHighSignalLiveSweep } from "./live-model-filter.js"; -function hermeticProviderRegistryEnv(): NodeJS.ProcessEnv { - return { - OPENCLAW_DISABLE_PERSISTED_PLUGIN_REGISTRY: "1", - OPENCLAW_DISABLE_PLUGIN_DISCOVERY_CACHE: "1", - OPENCLAW_DISABLE_PLUGIN_MANIFEST_CACHE: "1", - VITEST: "1", - } as NodeJS.ProcessEnv; +function resolveProviderOwners(provider: string): readonly string[] | undefined { + if (provider === "openai" || provider === "openai-codex") { + return ["openai"]; + } + if (provider === "codex" || provider === "codex-cli") { + return ["codex"]; + } + return undefined; } describe("shouldExcludeProviderFromDefaultHighSignalLiveSweep", () => { - beforeEach(() => { - clearPluginDiscoveryCache(); - clearPluginManifestRegistryCache(); - }); - it("excludes dedicated harness providers from the default high-signal sweep", () => { - const env = hermeticProviderRegistryEnv(); - expect( shouldExcludeProviderFromDefaultHighSignalLiveSweep({ provider: "codex", useExplicitModels: false, providerFilter: null, - env, + resolveProviderOwners, }), ).toBe(true); expect( @@ -34,7 +26,7 @@ describe("shouldExcludeProviderFromDefaultHighSignalLiveSweep", () => { provider: "openai-codex", useExplicitModels: false, providerFilter: null, - env, + resolveProviderOwners, }), ).toBe(true); expect( @@ -42,20 +34,18 @@ describe("shouldExcludeProviderFromDefaultHighSignalLiveSweep", () => { provider: "codex-cli", useExplicitModels: false, providerFilter: null, - env, + resolveProviderOwners, }), ).toBe(true); }); it("keeps dedicated harness providers when explicitly requested by provider filter", () => { - const env = hermeticProviderRegistryEnv(); - expect( shouldExcludeProviderFromDefaultHighSignalLiveSweep({ provider: "codex", useExplicitModels: false, providerFilter: new Set(["codex"]), - env, + resolveProviderOwners, }), ).toBe(false); expect( @@ -63,7 +53,7 @@ describe("shouldExcludeProviderFromDefaultHighSignalLiveSweep", () => { provider: "openai-codex", useExplicitModels: false, providerFilter: new Set(["codex-cli"]), - env, + resolveProviderOwners, }), ).toBe(false); expect( @@ -71,7 +61,7 @@ describe("shouldExcludeProviderFromDefaultHighSignalLiveSweep", () => { provider: "openai-codex", useExplicitModels: false, providerFilter: new Set(["openai"]), - env, + resolveProviderOwners, }), ).toBe(false); }); @@ -92,7 +82,7 @@ describe("shouldExcludeProviderFromDefaultHighSignalLiveSweep", () => { provider: "openai", useExplicitModels: false, providerFilter: null, - env: hermeticProviderRegistryEnv(), + resolveProviderOwners, }), ).toBe(false); }); diff --git a/src/agents/live-model-filter.ts b/src/agents/live-model-filter.ts index 7e4921bec1f..5d08e03fcd3 100644 --- a/src/agents/live-model-filter.ts +++ b/src/agents/live-model-filter.ts @@ -149,6 +149,7 @@ export function shouldExcludeProviderFromDefaultHighSignalLiveSweep(params: { config?: OpenClawConfig; workspaceDir?: string; env?: NodeJS.ProcessEnv; + resolveProviderOwners?: (provider: string) => readonly string[] | undefined; }): boolean { const provider = normalizeProviderId(params.provider ?? ""); if (!provider || params.useExplicitModels) { @@ -163,16 +164,20 @@ export function shouldExcludeProviderFromDefaultHighSignalLiveSweep(params: { if (requestedProvider === provider) { return false; } - if ( - requestedProvider && - liveProvidersShareOwningPlugin(requestedProvider, provider, { - config: params.config, - workspaceDir: params.workspaceDir, - env: params.env, - ownerCache, - }) - ) { - return false; + if (requestedProvider) { + const sharesOwner = params.resolveProviderOwners + ? (params.resolveProviderOwners(requestedProvider) ?? []).some((owner) => + (params.resolveProviderOwners?.(provider) ?? []).includes(owner), + ) + : liveProvidersShareOwningPlugin(requestedProvider, provider, { + config: params.config, + workspaceDir: params.workspaceDir, + env: params.env, + ownerCache, + }); + if (sharesOwner) { + return false; + } } if (requestedProvider && DEFAULT_HIGH_SIGNAL_LIVE_EXCLUDED_PROVIDERS.has(requestedProvider)) { return false; diff --git a/src/agents/models-config.providers.implicit.ts b/src/agents/models-config.providers.implicit.ts index 3ff12cca7fc..777ecbb7dca 100644 --- a/src/agents/models-config.providers.implicit.ts +++ b/src/agents/models-config.providers.implicit.ts @@ -70,6 +70,7 @@ function resolveProviderDiscoveryFilter(params: { config?: OpenClawConfig; workspaceDir?: string; env: NodeJS.ProcessEnv; + resolveOwners?: (provider: string) => readonly string[] | undefined; }): string[] | undefined { const { config, workspaceDir, env } = params; const testRaw = env.OPENCLAW_TEST_ONLY_PROVIDER_PLUGIN_IDS?.trim(); @@ -102,12 +103,14 @@ function resolveProviderDiscoveryFilter(params: { const pluginIds = new Set(); for (const id of ids) { const owners = + params.resolveOwners?.(id) ?? resolveOwningPluginIdsForProvider({ provider: id, config, workspaceDir, env, - }) ?? []; + }) ?? + []; if (owners.length > 0) { for (const owner of owners) { pluginIds.add(owner); @@ -125,6 +128,7 @@ export function resolveProviderDiscoveryFilterForTest(params: { config?: OpenClawConfig; workspaceDir?: string; env: NodeJS.ProcessEnv; + resolveOwners?: (provider: string) => readonly string[] | undefined; }): string[] | undefined { return resolveProviderDiscoveryFilter(params); } diff --git a/src/agents/models-config.providers.live-filter.test.ts b/src/agents/models-config.providers.live-filter.test.ts index ddd8bda6143..3cd55195b8b 100644 --- a/src/agents/models-config.providers.live-filter.test.ts +++ b/src/agents/models-config.providers.live-filter.test.ts @@ -1,24 +1,18 @@ -import { beforeEach, describe, expect, it } from "vitest"; -import { clearPluginDiscoveryCache } from "../plugins/discovery.js"; -import { clearPluginManifestRegistryCache } from "../plugins/manifest-registry.js"; +import { describe, expect, it } from "vitest"; import { resolveProviderDiscoveryFilterForTest } from "./models-config.providers.implicit.js"; function liveFilterEnv(overrides: NodeJS.ProcessEnv): NodeJS.ProcessEnv { return { - OPENCLAW_DISABLE_PERSISTED_PLUGIN_REGISTRY: "1", - OPENCLAW_DISABLE_PLUGIN_DISCOVERY_CACHE: "1", - OPENCLAW_DISABLE_PLUGIN_MANIFEST_CACHE: "1", VITEST: "1", ...overrides, } as NodeJS.ProcessEnv; } -describe("resolveProviderDiscoveryFilterForTest", () => { - beforeEach(() => { - clearPluginDiscoveryCache(); - clearPluginManifestRegistryCache(); - }); +function resolveOwners(provider: string): readonly string[] | undefined { + return provider === "claude-cli" ? ["anthropic"] : undefined; +} +describe("resolveProviderDiscoveryFilterForTest", () => { it("maps live provider backend ids to owning plugin ids", () => { expect( resolveProviderDiscoveryFilterForTest({ @@ -26,6 +20,7 @@ describe("resolveProviderDiscoveryFilterForTest", () => { OPENCLAW_LIVE_TEST: "1", OPENCLAW_LIVE_PROVIDERS: "claude-cli", }), + resolveOwners, }), ).toEqual(["anthropic"]); }); @@ -37,6 +32,7 @@ describe("resolveProviderDiscoveryFilterForTest", () => { OPENCLAW_LIVE_TEST: "1", OPENCLAW_LIVE_GATEWAY_PROVIDERS: "claude-cli", }), + resolveOwners, }), ).toEqual(["anthropic"]); }); @@ -48,6 +44,7 @@ describe("resolveProviderDiscoveryFilterForTest", () => { OPENCLAW_LIVE_TEST: "1", OPENCLAW_LIVE_PROVIDERS: "openrouter", }), + resolveOwners, }), ).toEqual(["openrouter"]); }); diff --git a/src/agents/models-config.providers.plugin-allowlist-compat.test.ts b/src/agents/models-config.providers.plugin-allowlist-compat.test.ts index 6c2e6c07757..a440b4dbe80 100644 --- a/src/agents/models-config.providers.plugin-allowlist-compat.test.ts +++ b/src/agents/models-config.providers.plugin-allowlist-compat.test.ts @@ -1,27 +1,66 @@ -import { beforeEach, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; import { withBundledPluginAllowlistCompat, withBundledPluginEnablementCompat, } from "../plugins/bundled-compat.js"; -import { clearPluginDiscoveryCache } from "../plugins/discovery.js"; -import { clearPluginManifestRegistryCache } from "../plugins/manifest-registry.js"; +import type { PluginManifestRecord, PluginManifestRegistry } from "../plugins/manifest-registry.js"; +import type { PluginRegistrySnapshot } from "../plugins/plugin-registry.js"; import { resolveEnabledProviderPluginIds } from "../plugins/providers.js"; -function providerRegistryEnv(): NodeJS.ProcessEnv { +const PROVIDER_PLUGIN_IDS = ["kilocode", "moonshot", "openrouter"] as const; + +function createProviderManifestRecord(pluginId: string): PluginManifestRecord { return { - OPENCLAW_DISABLE_PERSISTED_PLUGIN_REGISTRY: "1", - OPENCLAW_DISABLE_PLUGIN_DISCOVERY_CACHE: "1", - OPENCLAW_DISABLE_PLUGIN_MANIFEST_CACHE: "1", - VITEST: "1", - } as NodeJS.ProcessEnv; + id: pluginId, + channels: [], + providers: [pluginId], + cliBackends: [], + skills: [], + hooks: [], + origin: "bundled", + rootDir: `/virtual/${pluginId}`, + source: `/virtual/${pluginId}/index.ts`, + manifestPath: `/virtual/${pluginId}/openclaw.plugin.json`, + }; } -describe("implicit provider plugin allowlist compatibility", () => { - beforeEach(() => { - clearPluginDiscoveryCache(); - clearPluginManifestRegistryCache(); - }); +function createProviderRegistryRecord(pluginId: string): PluginRegistrySnapshot["plugins"][number] { + return { + pluginId, + manifestPath: `/virtual/${pluginId}/openclaw.plugin.json`, + manifestHash: `${pluginId}-manifest-hash`, + rootDir: `/virtual/${pluginId}`, + origin: "bundled", + enabled: true, + enabledByDefault: true, + startup: { + sidecar: false, + memory: false, + deferConfiguredChannelFullLoadUntilAfterListen: false, + agentHarnesses: [], + }, + compat: [], + }; +} +const providerRegistry: PluginRegistrySnapshot = { + version: 1, + hostContractVersion: "2026.4.25", + compatRegistryVersion: "compat-v1", + migrationVersion: 1, + policyHash: "policy-v1", + generatedAtMs: 1777118400000, + installRecords: {}, + plugins: PROVIDER_PLUGIN_IDS.map(createProviderRegistryRecord), + diagnostics: [], +}; + +const providerManifestRegistry: PluginManifestRegistry = { + plugins: PROVIDER_PLUGIN_IDS.map(createProviderManifestRecord), + diagnostics: [], +}; + +describe("implicit provider plugin allowlist compatibility", () => { it("keeps bundled implicit providers discoverable when plugins.allow is set", () => { const config = withBundledPluginEnablementCompat({ config: withBundledPluginAllowlistCompat({ @@ -38,8 +77,9 @@ describe("implicit provider plugin allowlist compatibility", () => { expect( resolveEnabledProviderPluginIds({ config, - env: providerRegistryEnv(), - onlyPluginIds: ["kilocode", "moonshot", "openrouter"], + registry: providerRegistry, + manifestRegistry: providerManifestRegistry, + onlyPluginIds: PROVIDER_PLUGIN_IDS, }), ).toEqual(["kilocode", "moonshot", "openrouter"]); }); @@ -61,8 +101,9 @@ describe("implicit provider plugin allowlist compatibility", () => { expect( resolveEnabledProviderPluginIds({ config, - env: providerRegistryEnv(), - onlyPluginIds: ["kilocode", "moonshot", "openrouter"], + registry: providerRegistry, + manifestRegistry: providerManifestRegistry, + onlyPluginIds: PROVIDER_PLUGIN_IDS, }), ).toEqual(["moonshot", "openrouter"]); }); diff --git a/src/commands/channel-setup/discovery.test.ts b/src/commands/channel-setup/discovery.test.ts index 3b69ce9139e..04aa5a357ea 100644 --- a/src/commands/channel-setup/discovery.test.ts +++ b/src/commands/channel-setup/discovery.test.ts @@ -94,6 +94,8 @@ describe("listManifestInstalledChannelIds", () => { }, contribution: "channels", config: autoEnabledConfig, + workspaceDir: "/tmp/workspace", + env: { OPENCLAW_HOME: "/tmp/home" }, }); expect(installedIds).toEqual(new Set(["slack"])); }); diff --git a/src/commands/channel-setup/discovery.ts b/src/commands/channel-setup/discovery.ts index 2fdf155501b..b991e39b70a 100644 --- a/src/commands/channel-setup/discovery.ts +++ b/src/commands/channel-setup/discovery.ts @@ -57,9 +57,13 @@ export function listManifestInstalledChannelIds(params: { env: params.env ?? process.env, }); return new Set( - listPluginContributionIds({ index, contribution: "channels", config: resolvedConfig }).map( - (channelId) => channelId as ChannelChoice, - ), + listPluginContributionIds({ + index, + contribution: "channels", + config: resolvedConfig, + workspaceDir, + env: params.env ?? process.env, + }).map((channelId) => channelId as ChannelChoice), ); } diff --git a/src/commands/status-runtime-shared.test.ts b/src/commands/status-runtime-shared.test.ts index 5415b2b72ab..a60d8eccfcd 100644 --- a/src/commands/status-runtime-shared.test.ts +++ b/src/commands/status-runtime-shared.test.ts @@ -71,7 +71,10 @@ describe("status-runtime-shared", () => { }); expect(mocks.resolveReadOnlyChannelPluginsForConfig).toHaveBeenCalledWith( { gateway: {} }, - { activationSourceConfig: { gateway: {} } }, + { + activationSourceConfig: { gateway: {} }, + includeSetupRuntimeFallback: false, + }, ); }); diff --git a/src/plugins/manifest-registry-installed.ts b/src/plugins/manifest-registry-installed.ts index 5b5c9f0c3cd..3d6241bb06a 100644 --- a/src/plugins/manifest-registry-installed.ts +++ b/src/plugins/manifest-registry-installed.ts @@ -7,28 +7,34 @@ import { extractPluginInstallRecordsFromInstalledPluginIndex } from "./installed import { loadPluginManifestRegistry, type PluginManifestRegistry } from "./manifest-registry.js"; import { DEFAULT_PLUGIN_ENTRY_CANDIDATES } from "./manifest.js"; +function resolveInstalledPluginRootDir(record: InstalledPluginIndexRecord): string { + return record.rootDir || path.dirname(record.manifestPath || process.cwd()); +} + function resolveFallbackPluginSource(record: InstalledPluginIndexRecord): string { + const rootDir = resolveInstalledPluginRootDir(record); for (const entry of DEFAULT_PLUGIN_ENTRY_CANDIDATES) { - const candidate = path.join(record.rootDir, entry); + const candidate = path.join(rootDir, entry); if (fs.existsSync(candidate)) { return candidate; } } - return path.join(record.rootDir, DEFAULT_PLUGIN_ENTRY_CANDIDATES[0]); + return path.join(rootDir, DEFAULT_PLUGIN_ENTRY_CANDIDATES[0]); } function toPluginCandidate(record: InstalledPluginIndexRecord): PluginCandidate { + const rootDir = resolveInstalledPluginRootDir(record); return { idHint: record.pluginId, source: record.source ?? resolveFallbackPluginSource(record), ...(record.setupSource ? { setupSource: record.setupSource } : {}), - rootDir: record.rootDir, + rootDir, origin: record.origin, ...(record.format ? { format: record.format } : {}), ...(record.bundleFormat ? { bundleFormat: record.bundleFormat } : {}), ...(record.packageName ? { packageName: record.packageName } : {}), ...(record.packageVersion ? { packageVersion: record.packageVersion } : {}), - packageDir: record.rootDir, + packageDir: rootDir, }; } diff --git a/src/plugins/providers.ts b/src/plugins/providers.ts index dfff44f5227..22f932ea344 100644 --- a/src/plugins/providers.ts +++ b/src/plugins/providers.ts @@ -23,6 +23,8 @@ type ProviderManifestLoadParams = { config?: PluginLoadOptions["config"]; workspaceDir?: string; env?: PluginLoadOptions["env"]; + registry?: PluginRegistrySnapshot; + manifestRegistry?: PluginManifestRegistry; }; type NormalizedPluginsConfig = ReturnType; type ProviderRegistryLoadParams = ProviderManifestLoadParams & { @@ -30,6 +32,9 @@ type ProviderRegistryLoadParams = ProviderManifestLoadParams & { }; function loadProviderRegistrySnapshot(params: ProviderManifestLoadParams): PluginRegistrySnapshot { + if (params.registry) { + return params.registry; + } return loadPluginRegistrySnapshot({ config: params.config, workspaceDir: params.workspaceDir, @@ -138,12 +143,7 @@ export function resolveBundledProviderCompatPluginIds(params: { ); } -export function resolveEnabledProviderPluginIds(params: { - config?: PluginLoadOptions["config"]; - workspaceDir?: string; - env?: PluginLoadOptions["env"]; - onlyPluginIds?: readonly string[]; -}): string[] { +export function resolveEnabledProviderPluginIds(params: ProviderRegistryLoadParams): string[] { const { registry, onlyPluginIdSet } = loadScopedProviderRegistry(params); const providerSurfacePluginIds = resolveProviderSurfacePluginIdSet({ ...params, registry }); const normalizedConfig = normalizePluginsConfigWithRegistry(params.config?.plugins, registry);