fix(plugins): default bundled discovery to allowlist

This commit is contained in:
Peter Steinberger
2026-05-04 22:46:39 +01:00
parent 55df2d4598
commit b2096d19ec
19 changed files with 185 additions and 77 deletions

View File

@@ -61,7 +61,31 @@ const providerManifestRegistry: PluginManifestRegistry = {
};
describe("implicit provider plugin allowlist compatibility", () => {
it("keeps bundled implicit providers discoverable when plugins.allow is set", () => {
it("keeps bundled implicit providers discoverable in explicit compat mode", () => {
const config = withBundledPluginEnablementCompat({
config: withBundledPluginAllowlistCompat({
config: {
plugins: {
allow: ["openrouter"],
bundledDiscovery: "compat",
},
},
pluginIds: ["kilocode", "moonshot"],
}),
pluginIds: ["kilocode", "moonshot"],
});
expect(
resolveEnabledProviderPluginIds({
config,
registry: providerRegistry,
manifestRegistry: providerManifestRegistry,
onlyPluginIds: PROVIDER_PLUGIN_IDS,
}),
).toEqual(["kilocode", "moonshot", "openrouter"]);
});
it("respects allowlist for bundled plugins by default", () => {
const config = withBundledPluginEnablementCompat({
config: withBundledPluginAllowlistCompat({
config: {
@@ -81,7 +105,7 @@ describe("implicit provider plugin allowlist compatibility", () => {
manifestRegistry: providerManifestRegistry,
onlyPluginIds: PROVIDER_PLUGIN_IDS,
}),
).toEqual(["kilocode", "moonshot", "openrouter"]);
).toEqual(["openrouter"]);
});
it("respects allowlist for bundled plugins when bundledDiscovery is allowlist", () => {
@@ -114,6 +138,7 @@ describe("implicit provider plugin allowlist compatibility", () => {
config: {
plugins: {
allow: ["openrouter"],
bundledDiscovery: "compat",
deny: ["kilocode"],
},
},

View File

@@ -267,6 +267,33 @@ describe("legacy migrate mention routing", () => {
});
});
describe("legacy bundled provider discovery migrate", () => {
it("sets compat mode for existing restrictive plugin allowlists", () => {
const res = migrateLegacyConfigForTest({
plugins: {
allow: ["telegram"],
},
});
expect(res.config?.plugins?.bundledDiscovery).toBe("compat");
expect(res.changes).toContain(
'Set plugins.bundledDiscovery="compat" to preserve legacy bundled provider discovery for this restrictive plugins.allow config.',
);
});
it("does not override explicit bundled discovery mode", () => {
const res = migrateLegacyConfigForTest({
plugins: {
allow: ["telegram"],
bundledDiscovery: "allowlist",
},
});
expect(res.config).toBeNull();
expect(res.changes).toEqual([]);
});
});
describe("legacy migrate sandbox scope aliases", () => {
it("removes legacy agents.defaults.llm timeout config", () => {
const res = migrateLegacyConfigForTest({

View File

@@ -3,6 +3,7 @@ import {
type LegacyConfigMigrationSpec,
type LegacyConfigRule,
} from "../../../config/legacy.shared.js";
import { isRecord } from "./legacy-config-record-shared.js";
import { migrateLegacyXSearchConfig } from "./legacy-x-search-migrate.js";
const X_SEARCH_RULE: LegacyConfigRule = {
@@ -11,7 +12,40 @@ const X_SEARCH_RULE: LegacyConfigRule = {
'tools.web.x_search.apiKey moved to the xAI plugin; use plugins.entries.xai.config.webSearch.apiKey instead. Run "openclaw doctor --fix".',
};
const BUNDLED_DISCOVERY_COMPAT_RULE: LegacyConfigRule = {
path: ["plugins", "allow"],
message:
'plugins.allow now gates bundled provider discovery by default; run "openclaw doctor --fix" to preserve legacy bundled provider compatibility as plugins.bundledDiscovery="compat", or set plugins.bundledDiscovery="allowlist" to keep the stricter behavior.',
requireSourceLiteral: true,
match: (value, root) => {
if (!Array.isArray(value) || value.length === 0) {
return false;
}
const plugins = isRecord(root.plugins) ? root.plugins : undefined;
return plugins?.bundledDiscovery === undefined;
},
};
export const LEGACY_CONFIG_MIGRATIONS_RUNTIME_PROVIDERS: LegacyConfigMigrationSpec[] = [
defineLegacyConfigMigration({
id: "plugins.allow->plugins.bundledDiscovery.compat",
describe: "Preserve legacy bundled provider discovery for existing restrictive allowlists",
legacyRules: [BUNDLED_DISCOVERY_COMPAT_RULE],
apply: (raw, changes) => {
const plugins = isRecord(raw.plugins) ? raw.plugins : undefined;
if (!plugins || plugins.bundledDiscovery !== undefined) {
return;
}
const allow = plugins.allow;
if (!Array.isArray(allow) || allow.length === 0) {
return;
}
plugins.bundledDiscovery = "compat";
changes.push(
'Set plugins.bundledDiscovery="compat" to preserve legacy bundled provider discovery for this restrictive plugins.allow config.',
);
},
}),
defineLegacyConfigMigration({
id: "tools.web.x_search.apiKey->plugins.entries.xai.config.webSearch.apiKey",
describe: "Move legacy x_search auth into the xAI plugin webSearch config",

View File

@@ -113,10 +113,13 @@ describe("collectPluginToolAllowlistWarnings", () => {
expect(warnings).toEqual([]);
});
it("warns when restrictive plugins.allow leaves bundled provider discovery in compat mode", () => {
it("warns when restrictive plugins.allow leaves bundled provider discovery in explicit compat mode", () => {
const warnings = collectBundledProviderAllowlistPolicyWarnings({
cfg: {
plugins: { allow: ["telegram"] },
plugins: {
allow: ["telegram"],
bundledDiscovery: "compat",
},
},
});
@@ -125,16 +128,18 @@ describe("collectPluginToolAllowlistWarnings", () => {
]);
});
it("does not warn when bundled provider discovery follows the allowlist", () => {
const warnings = collectBundledProviderAllowlistPolicyWarnings({
cfg: {
plugins: {
allow: ["telegram"],
bundledDiscovery: "allowlist",
},
},
});
it.each([
{ name: "default", plugins: { allow: ["telegram"] } },
{
name: "explicit allowlist",
plugins: { allow: ["telegram"], bundledDiscovery: "allowlist" },
},
])(
"does not warn when bundled provider discovery follows the allowlist ($name)",
({ plugins }) => {
const warnings = collectBundledProviderAllowlistPolicyWarnings({ cfg: { plugins } });
expect(warnings).toEqual([]);
});
expect(warnings).toEqual([]);
},
);
});

View File

@@ -204,7 +204,7 @@ export function collectBundledProviderAllowlistPolicyWarnings(params: {
if (!Array.isArray(allow) || allow.length === 0) {
return [];
}
if (params.cfg.plugins?.bundledDiscovery === "allowlist") {
if (params.cfg.plugins?.bundledDiscovery !== "compat") {
return [];
}
return [

View File

@@ -24191,7 +24191,7 @@ export const GENERATED_BASE_CONFIG_SCHEMA: BaseConfigSchemaResponse = {
enum: ["compat", "allowlist"],
title: "Bundled Plugin Discovery",
description:
'Controls bundled plugin runtime discovery when plugins.allow is configured. "compat" (default) preserves legacy behavior where bundled provider plugins can be force-loaded on every chat turn. "allowlist" gates bundled provider plugins by plugins.allow like third-party plugins.',
'Controls bundled plugin runtime discovery when plugins.allow is configured. "allowlist" (default) gates bundled provider plugins by plugins.allow like third-party plugins. "compat" preserves legacy behavior where bundled provider plugins can be force-loaded on every chat turn.',
},
},
additionalProperties: false,
@@ -28874,7 +28874,7 @@ export const GENERATED_BASE_CONFIG_SCHEMA: BaseConfigSchemaResponse = {
},
"plugins.bundledDiscovery": {
label: "Bundled Plugin Discovery",
help: 'Controls bundled plugin runtime discovery when plugins.allow is configured. "compat" (default) preserves legacy behavior where bundled provider plugins can be force-loaded on every chat turn. "allowlist" gates bundled provider plugins by plugins.allow like third-party plugins.',
help: 'Controls bundled plugin runtime discovery when plugins.allow is configured. "allowlist" (default) gates bundled provider plugins by plugins.allow like third-party plugins. "compat" preserves legacy behavior where bundled provider plugins can be force-loaded on every chat turn.',
tags: ["advanced"],
},
"plugins.deny": {

View File

@@ -1213,7 +1213,7 @@ export const FIELD_HELP: Record<string, string> = {
"plugins.slots.contextEngine":
"Selects the active context engine plugin by id so one plugin provides context orchestration behavior.",
"plugins.bundledDiscovery":
'Controls bundled plugin runtime discovery when plugins.allow is configured. "compat" (default) preserves legacy behavior where bundled provider plugins can be force-loaded on every chat turn. "allowlist" gates bundled provider plugins by plugins.allow like third-party plugins.',
'Controls bundled plugin runtime discovery when plugins.allow is configured. "allowlist" (default) gates bundled provider plugins by plugins.allow like third-party plugins. "compat" preserves legacy behavior where bundled provider plugins can be force-loaded on every chat turn.',
"plugins.entries":
"Per-plugin settings keyed by plugin ID including enablement and plugin-specific runtime configuration payloads. Use this for scoped plugin tuning without changing global loader policy.",
"plugins.entries.*.enabled":

View File

@@ -55,10 +55,10 @@ export type PluginsConfig = {
* Controls how bundled plugins participate in runtime provider discovery when
* `allow` is configured.
*
* - `"compat"` (default): bundled provider plugins are force-loaded on
* every chat turn regardless of the allowlist (legacy behavior).
* - `"allowlist"`: bundled provider plugins are gated by `allow` and
* `entries.<id>.enabled` like third-party plugins.
* - `"allowlist"` (default): bundled provider plugins are gated by `allow`
* and `entries.<id>.enabled` like third-party plugins.
* - `"compat"`: legacy mode for migrated configs; bundled provider plugins
* can be force-loaded regardless of the allowlist.
*/
bundledDiscovery?: "compat" | "allowlist";
load?: PluginsLoadConfig;

View File

@@ -79,9 +79,9 @@ export function withActivatedPluginIds(params: {
return params.config;
}
const originalAllow = params.config?.plugins?.allow ?? [];
// Empty allowlists are still open; allowlist mode only stops compat from widening configured allowlists.
// Empty allowlists are still open; only explicit compat widens configured allowlists.
const useAllowlistDiscovery =
params.config?.plugins?.bundledDiscovery === "allowlist" && originalAllow.length > 0;
params.config?.plugins?.bundledDiscovery !== "compat" && originalAllow.length > 0;
const originalAllowSet = useAllowlistDiscovery ? new Set(originalAllow) : undefined;
const allow = new Set(originalAllow);
const entries = {

View File

@@ -6,7 +6,7 @@ export function withBundledPluginAllowlistCompat(params: {
config: OpenClawConfig | undefined;
pluginIds: readonly string[];
}): OpenClawConfig | undefined {
if (params.config?.plugins?.bundledDiscovery === "allowlist") {
if (params.config?.plugins?.bundledDiscovery !== "compat") {
return params.config;
}
const allow = params.config?.plugins?.allow;
@@ -42,8 +42,10 @@ export function withBundledPluginEnablementCompat(params: {
}): OpenClawConfig | undefined {
const existingEntries = params.config?.plugins?.entries ?? {};
const forcePluginsEnabled = params.config?.plugins?.enabled === false;
const useAllowlistDiscovery = params.config?.plugins?.bundledDiscovery === "allowlist";
const allowSet = useAllowlistDiscovery ? new Set(params.config?.plugins?.allow ?? []) : undefined;
const useCompatDiscovery = params.config?.plugins?.bundledDiscovery === "compat";
const allow = params.config?.plugins?.allow;
const allowSet =
!useCompatDiscovery && Array.isArray(allow) && allow.length > 0 ? new Set(allow) : undefined;
let changed = false;
const nextEntries: Record<string, PluginEntryConfig> = { ...existingEntries };

View File

@@ -295,6 +295,7 @@ function createBundledProviderCompatOptions(params?: { onlyPluginIds?: readonly
config: {
plugins: {
allow: ["openrouter"],
bundledDiscovery: "compat" as const,
},
},
bundledProviderAllowlistCompat: true,
@@ -593,7 +594,7 @@ describe("resolvePluginProviders", () => {
).toEqual(["legacy-auth-owner"]);
});
it("filters bundled provider plugins by allowlist when bundledDiscovery is allowlist", () => {
it("filters bundled provider plugins by allowlist by default", () => {
setManifestPlugins([
createManifestProviderPlugin({
id: "kilocode",
@@ -619,7 +620,6 @@ describe("resolvePluginProviders", () => {
config: {
plugins: {
allow: ["openrouter"],
bundledDiscovery: "allowlist",
},
},
env: {} as NodeJS.ProcessEnv,
@@ -628,7 +628,7 @@ describe("resolvePluginProviders", () => {
expect(discovered).toEqual(["openrouter"]);
});
it("returns all bundled provider plugins in compat mode (default)", () => {
it("returns all bundled provider plugins in explicit compat mode", () => {
setManifestPlugins([
createManifestProviderPlugin({
id: "kilocode",
@@ -654,6 +654,7 @@ describe("resolvePluginProviders", () => {
config: {
plugins: {
allow: ["openrouter"],
bundledDiscovery: "compat",
},
},
env: {} as NodeJS.ProcessEnv,
@@ -829,6 +830,7 @@ describe("resolvePluginProviders", () => {
config: {
plugins: {
allow: ["openrouter"],
bundledDiscovery: "compat",
},
},
bundledProviderAllowlistCompat: true,
@@ -842,11 +844,38 @@ describe("resolvePluginProviders", () => {
});
});
it("loads all discovered provider plugins in setup mode", () => {
it("scopes setup provider plugin discovery to the allowlist by default", () => {
resolvePluginProviders({
config: {
plugins: {
allow: ["google"],
},
},
mode: "setup",
includeUntrustedWorkspacePlugins: false,
});
expectLastSetupRegistryLoad({
onlyPluginIds: ["google"],
});
expect(getLastSetupLoadedPluginConfig()).toEqual(
expect.objectContaining({
plugins: expect.objectContaining({
allow: ["google"],
entries: expect.objectContaining({
google: { enabled: true },
}),
}),
}),
);
});
it("loads all discovered provider plugins in setup mode for explicit compat configs", () => {
resolvePluginProviders({
config: {
plugins: {
allow: ["openrouter"],
bundledDiscovery: "compat",
entries: {
google: { enabled: false },
},
@@ -884,6 +913,7 @@ describe("resolvePluginProviders", () => {
config: {
plugins: {
allow: ["openrouter"],
bundledDiscovery: "compat",
},
},
mode: "setup",
@@ -900,6 +930,7 @@ describe("resolvePluginProviders", () => {
config: {
plugins: {
allow: ["openrouter", "workspace-provider"],
bundledDiscovery: "compat",
entries: {
"workspace-provider": { enabled: false },
},
@@ -919,6 +950,7 @@ describe("resolvePluginProviders", () => {
config: {
plugins: {
allow: ["openrouter", "workspace-provider"],
bundledDiscovery: "compat",
deny: ["workspace-provider"],
entries: {
"workspace-provider": { enabled: false },
@@ -948,9 +980,7 @@ describe("resolvePluginProviders", () => {
includeUntrustedWorkspacePlugins: false,
});
expectLastSetupRegistryLoad({
onlyPluginIds: ["google", "kilocode", "moonshot"],
});
expect(loadOpenClawPluginsMock).not.toHaveBeenCalled();
});
it("loads provider plugins from the auto-enabled config snapshot", () => {
@@ -1242,16 +1272,7 @@ describe("resolvePluginProviders", () => {
mode: "setup",
});
expect(loadOpenClawPluginsMock).toHaveBeenCalledWith(
expect.objectContaining({
config: expect.objectContaining({
plugins: expect.objectContaining({
enabled: false,
allow: ["setup-owned-provider"],
}),
}),
}),
);
expect(loadOpenClawPluginsMock).not.toHaveBeenCalled();
});
it("does not override explicitly disabled setup owners", () => {
@@ -1278,18 +1299,7 @@ describe("resolvePluginProviders", () => {
mode: "setup",
});
expect(loadOpenClawPluginsMock).toHaveBeenCalledWith(
expect.objectContaining({
config: expect.objectContaining({
plugins: expect.objectContaining({
allow: ["setup-owned-provider"],
entries: {
"setup-owned-provider": { enabled: false },
},
}),
}),
}),
);
expect(loadOpenClawPluginsMock).not.toHaveBeenCalled();
});
it("filters explicit setup owners through the untrusted workspace discovery gate", () => {

View File

@@ -255,7 +255,7 @@ export function resolveDiscoveredProviderPluginIds(params: {
const { registry, onlyPluginIdSet } = loadScopedProviderRegistry(params);
const providerSurfacePluginIds = resolveProviderSurfacePluginIdSet({ ...params, registry });
const shouldFilterUntrustedWorkspacePlugins = params.includeUntrustedWorkspacePlugins === false;
const shouldFilterBundledByAllowlist = params.config?.plugins?.bundledDiscovery === "allowlist";
const shouldFilterBundledByAllowlist = params.config?.plugins?.bundledDiscovery !== "compat";
const normalizedConfig = normalizePluginsConfigWithRegistry(params.config?.plugins, registry);
return listRegistryPluginIds(registry, (plugin) => {
if (
@@ -298,6 +298,9 @@ function isProviderPluginEligibleForSetupDiscovery(params: {
) {
return false;
}
if (params.plugin.origin === "bundled") {
return true;
}
return isActivatedManifestOwner({
plugin: toManifestOwnerRecord(params.plugin),
normalizedConfig: params.normalizedConfig,
@@ -313,7 +316,7 @@ export function resolveDiscoverableProviderOwnerPluginIds(params: {
includeUntrustedWorkspacePlugins?: boolean;
}): string[] {
const shouldFilterUntrustedWorkspacePlugins = params.includeUntrustedWorkspacePlugins === false;
const shouldFilterBundledByAllowlist = params.config?.plugins?.bundledDiscovery === "allowlist";
const shouldFilterBundledByAllowlist = params.config?.plugins?.bundledDiscovery !== "compat";
return resolveProviderOwnerPluginIds({
...params,
isEligible: (plugin, normalizedConfig) =>

View File

@@ -32,7 +32,7 @@ function filterAllowlistedBundledPluginIds(
) {
const allow = config?.plugins?.allow;
if (
config?.plugins?.bundledDiscovery !== "allowlist" ||
config?.plugins?.bundledDiscovery === "compat" ||
!Array.isArray(allow) ||
allow.length === 0
) {

View File

@@ -445,7 +445,7 @@ describe("resolvePluginWebSearchProviders", () => {
const providers = resolvePluginWebSearchProviders({
config: {
plugins: {
allow: ["perplexity"],
allow: ["brave"],
},
},
mode: "setup",