From 0fd9aa8e001e0643f1f2a360a5bc416082fa1581 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 12 Apr 2026 19:36:03 +0100 Subject: [PATCH] refactor(plugins): centralize manifest owner trust policy (#65459) * refactor(plugins): share manifest owner policy helpers * test(plugins): cover activated manifest owner policy * fix(plugins): honor explicit disable in setup discovery --- src/plugins/channel-plugin-ids.ts | 116 +++++--------------- src/plugins/manifest-owner-policy.test.ts | 123 ++++++++++++++++++++++ src/plugins/manifest-owner-policy.ts | 61 +++++++++++ src/plugins/providers.test.ts | 8 +- src/plugins/providers.ts | 56 +++++----- 5 files changed, 240 insertions(+), 124 deletions(-) create mode 100644 src/plugins/manifest-owner-policy.test.ts create mode 100644 src/plugins/manifest-owner-policy.ts diff --git a/src/plugins/channel-plugin-ids.ts b/src/plugins/channel-plugin-ids.ts index a549d764060..537433d72ea 100644 --- a/src/plugins/channel-plugin-ids.ts +++ b/src/plugins/channel-plugin-ids.ts @@ -13,6 +13,12 @@ import { normalizePluginsConfig, resolveEffectivePluginActivationState, } from "./config-state.js"; +import { + hasExplicitManifestOwnerTrust, + isActivatedManifestOwner, + isBundledManifestOwner, + passesManifestOwnerBasePolicy, +} from "./manifest-owner-policy.js"; import { loadPluginManifestRegistry, type PluginManifestRecord } from "./manifest-registry.js"; import { hasKind } from "./slots.js"; @@ -54,86 +60,33 @@ function normalizeChannelIds(channelIds: Iterable): string[] { ).toSorted((left, right) => left.localeCompare(right)); } -function isBundledChannelOwner(plugin: PluginManifestRecord): boolean { - return plugin.origin === "bundled"; -} - -function hasExplicitNonBundledChannelOwnerTrust(params: { +function isChannelPluginEligibleForScopedOwnership(params: { plugin: PluginManifestRecord; normalizedConfig: ReturnType; + rootConfig: OpenClawConfig; }): boolean { - return ( - params.normalizedConfig.allow.includes(params.plugin.id) || - params.normalizedConfig.entries[params.plugin.id]?.enabled === true - ); -} - -function passesExplicitChannelOwnershipPolicy(params: { - plugin: PluginManifestRecord; - normalizedConfig: ReturnType; -}): boolean { - if (!params.normalizedConfig.enabled) { - return false; - } - if (params.normalizedConfig.deny.includes(params.plugin.id)) { - return false; - } - if (params.normalizedConfig.entries[params.plugin.id]?.enabled === false) { - return false; - } if ( - params.normalizedConfig.allow.length > 0 && - !params.normalizedConfig.allow.includes(params.plugin.id) + !passesManifestOwnerBasePolicy({ + plugin: params.plugin, + normalizedConfig: params.normalizedConfig, + }) ) { return false; } - return true; -} - -function isChannelPluginEligibleForSetupDiscovery(params: { - plugin: PluginManifestRecord; - normalizedConfig: ReturnType; - rootConfig: OpenClawConfig; -}): boolean { - if (!passesExplicitChannelOwnershipPolicy(params)) { - return false; - } - if (isBundledChannelOwner(params.plugin)) { + if (isBundledManifestOwner(params.plugin)) { return true; } if (params.plugin.origin === "global" || params.plugin.origin === "config") { - return hasExplicitNonBundledChannelOwnerTrust(params); + return hasExplicitManifestOwnerTrust({ + plugin: params.plugin, + normalizedConfig: params.normalizedConfig, + }); } - return resolveEffectivePluginActivationState({ - id: params.plugin.id, - origin: params.plugin.origin, - config: params.normalizedConfig, + return isActivatedManifestOwner({ + plugin: params.plugin, + normalizedConfig: params.normalizedConfig, rootConfig: params.rootConfig, - enabledByDefault: params.plugin.enabledByDefault, - }).activated; -} - -function isChannelPluginEligibleForRuntimeOwnerActivation(params: { - plugin: PluginManifestRecord; - normalizedConfig: ReturnType; - rootConfig: OpenClawConfig; -}): boolean { - if (!passesExplicitChannelOwnershipPolicy(params)) { - return false; - } - if (isBundledChannelOwner(params.plugin)) { - return true; - } - if (params.plugin.origin === "global" || params.plugin.origin === "config") { - return hasExplicitNonBundledChannelOwnerTrust(params); - } - return resolveEffectivePluginActivationState({ - id: params.plugin.id, - origin: params.plugin.origin, - config: params.normalizedConfig, - rootConfig: params.rootConfig, - enabledByDefault: params.plugin.enabledByDefault, - }).activated; + }); } function resolveScopedChannelOwnerPluginIds(params: { @@ -142,7 +95,6 @@ function resolveScopedChannelOwnerPluginIds(params: { channelIds: readonly string[]; workspaceDir?: string; env: NodeJS.ProcessEnv; - mode: "runtime" | "setup"; cache?: boolean; }): string[] { const channelIds = normalizeChannelIds(params.channelIds); @@ -180,17 +132,11 @@ function resolveScopedChannelOwnerPluginIds(params: { if (!candidateIdSet.has(plugin.id)) { return false; } - return params.mode === "setup" - ? isChannelPluginEligibleForSetupDiscovery({ - plugin, - normalizedConfig, - rootConfig: trustConfig, - }) - : isChannelPluginEligibleForRuntimeOwnerActivation({ - plugin, - normalizedConfig, - rootConfig: trustConfig, - }); + return isChannelPluginEligibleForScopedOwnership({ + plugin, + normalizedConfig, + rootConfig: trustConfig, + }); }) .map((plugin) => plugin.id) .toSorted((left, right) => left.localeCompare(right)); @@ -204,10 +150,7 @@ export function resolveScopedChannelPluginIds(params: { env: NodeJS.ProcessEnv; cache?: boolean; }): string[] { - return resolveScopedChannelOwnerPluginIds({ - ...params, - mode: "runtime", - }); + return resolveScopedChannelOwnerPluginIds(params); } export function resolveDiscoverableScopedChannelPluginIds(params: { @@ -218,10 +161,7 @@ export function resolveDiscoverableScopedChannelPluginIds(params: { env: NodeJS.ProcessEnv; cache?: boolean; }): string[] { - return resolveScopedChannelOwnerPluginIds({ - ...params, - mode: "setup", - }); + return resolveScopedChannelOwnerPluginIds(params); } function resolveGatewayStartupDreamingPluginIds(config: OpenClawConfig): Set { diff --git a/src/plugins/manifest-owner-policy.test.ts b/src/plugins/manifest-owner-policy.test.ts new file mode 100644 index 00000000000..6546a3a0e65 --- /dev/null +++ b/src/plugins/manifest-owner-policy.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it } from "vitest"; +import { normalizePluginsConfig } from "./config-state.js"; +import { + hasExplicitManifestOwnerTrust, + isActivatedManifestOwner, + isBundledManifestOwner, + passesManifestOwnerBasePolicy, +} from "./manifest-owner-policy.js"; + +describe("manifest owner policy", () => { + it("treats bundled owners as bundled and others as non-bundled", () => { + expect(isBundledManifestOwner({ origin: "bundled" })).toBe(true); + expect(isBundledManifestOwner({ origin: "workspace" })).toBe(false); + }); + + it("respects enabled, denylist, explicit disable, and allowlist bounds", () => { + const normalizedConfig = normalizePluginsConfig({ + enabled: true, + allow: ["demo"], + deny: ["blocked"], + entries: { + disabled: { enabled: false }, + enabled: { enabled: true }, + }, + }); + + expect( + passesManifestOwnerBasePolicy({ + plugin: { id: "demo" }, + normalizedConfig, + }), + ).toBe(true); + expect( + passesManifestOwnerBasePolicy({ + plugin: { id: "blocked" }, + normalizedConfig, + }), + ).toBe(false); + expect( + passesManifestOwnerBasePolicy({ + plugin: { id: "disabled" }, + normalizedConfig, + }), + ).toBe(false); + + const explicitlyTrustedDisabledConfig = normalizePluginsConfig({ + enabled: true, + allow: ["disabled"], + entries: { + disabled: { enabled: false }, + }, + }); + expect( + passesManifestOwnerBasePolicy({ + plugin: { id: "disabled" }, + normalizedConfig: explicitlyTrustedDisabledConfig, + allowExplicitlyDisabled: true, + }), + ).toBe(true); + expect( + passesManifestOwnerBasePolicy({ + plugin: { id: "other" }, + normalizedConfig, + }), + ).toBe(false); + }); + + it("detects explicit manifest owner trust from allowlist or explicit enablement", () => { + const allowlistConfig = normalizePluginsConfig({ + allow: ["demo"], + }); + const entriesConfig = normalizePluginsConfig({ + entries: { + demo: { enabled: true }, + }, + }); + + expect( + hasExplicitManifestOwnerTrust({ + plugin: { id: "demo" }, + normalizedConfig: allowlistConfig, + }), + ).toBe(true); + expect( + hasExplicitManifestOwnerTrust({ + plugin: { id: "demo" }, + normalizedConfig: entriesConfig, + }), + ).toBe(true); + expect( + hasExplicitManifestOwnerTrust({ + plugin: { id: "demo" }, + normalizedConfig: normalizePluginsConfig({}), + }), + ).toBe(false); + }); + + it("uses effective activation state for activated manifest owners", () => { + expect( + isActivatedManifestOwner({ + plugin: { + id: "demo", + origin: "bundled", + enabledByDefault: true, + }, + normalizedConfig: normalizePluginsConfig({}), + }), + ).toBe(true); + + expect( + isActivatedManifestOwner({ + plugin: { + id: "demo", + origin: "bundled", + enabledByDefault: true, + }, + normalizedConfig: normalizePluginsConfig({ + enabled: false, + }), + }), + ).toBe(false); + }); +}); diff --git a/src/plugins/manifest-owner-policy.ts b/src/plugins/manifest-owner-policy.ts new file mode 100644 index 00000000000..a668b127df0 --- /dev/null +++ b/src/plugins/manifest-owner-policy.ts @@ -0,0 +1,61 @@ +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { normalizePluginsConfig, resolveEffectivePluginActivationState } from "./config-state.js"; +import type { PluginManifestRecord } from "./manifest-registry.js"; + +type OwnerPlugin = Pick; + +type NormalizedPluginsConfig = ReturnType; + +export function isBundledManifestOwner(plugin: Pick): boolean { + return plugin.origin === "bundled"; +} + +export function hasExplicitManifestOwnerTrust(params: { + plugin: Pick; + normalizedConfig: NormalizedPluginsConfig; +}): boolean { + return ( + params.normalizedConfig.allow.includes(params.plugin.id) || + params.normalizedConfig.entries[params.plugin.id]?.enabled === true + ); +} + +export function passesManifestOwnerBasePolicy(params: { + plugin: Pick; + normalizedConfig: NormalizedPluginsConfig; + allowExplicitlyDisabled?: boolean; +}): boolean { + if (!params.normalizedConfig.enabled) { + return false; + } + if (params.normalizedConfig.deny.includes(params.plugin.id)) { + return false; + } + if ( + params.normalizedConfig.entries[params.plugin.id]?.enabled === false && + params.allowExplicitlyDisabled !== true + ) { + return false; + } + if ( + params.normalizedConfig.allow.length > 0 && + !params.normalizedConfig.allow.includes(params.plugin.id) + ) { + return false; + } + return true; +} + +export function isActivatedManifestOwner(params: { + plugin: OwnerPlugin; + normalizedConfig: NormalizedPluginsConfig; + rootConfig?: OpenClawConfig; +}): boolean { + return resolveEffectivePluginActivationState({ + id: params.plugin.id, + origin: params.plugin.origin, + config: params.normalizedConfig, + rootConfig: params.rootConfig, + enabledByDefault: params.plugin.enabledByDefault, + }).activated; +} diff --git a/src/plugins/providers.test.ts b/src/plugins/providers.test.ts index 72f94bf5704..5776b8afdea 100644 --- a/src/plugins/providers.test.ts +++ b/src/plugins/providers.test.ts @@ -591,7 +591,7 @@ describe("resolvePluginProviders", () => { }); }); - it("keeps trusted but disabled workspace provider plugins eligible in setup discovery", () => { + it("does not keep trusted but disabled workspace provider plugins eligible in setup discovery", () => { resolvePluginProviders({ config: { plugins: { @@ -606,7 +606,7 @@ describe("resolvePluginProviders", () => { }); expectLastSetupRegistryLoad({ - onlyPluginIds: ["google", "kilocode", "moonshot", "workspace-provider"], + onlyPluginIds: ["google", "kilocode", "moonshot"], }); }); @@ -1063,7 +1063,7 @@ describe("resolvePluginProviders", () => { ); }); - it("keeps explicitly trusted disabled workspace setup owners discoverable", () => { + it("does not keep explicitly trusted disabled workspace setup owners discoverable", () => { setManifestPlugins([ createManifestProviderPlugin({ id: "workspace-activation-owner", @@ -1089,7 +1089,7 @@ describe("resolvePluginProviders", () => { }, includeUntrustedWorkspacePlugins: false, }), - ).toEqual(["workspace-activation-owner"]); + ).toEqual([]); }); it("does not auto-activate explicitly disabled trusted workspace runtime owners", () => { diff --git a/src/plugins/providers.ts b/src/plugins/providers.ts index 71b408b480e..7247be4131d 100644 --- a/src/plugins/providers.ts +++ b/src/plugins/providers.ts @@ -2,6 +2,10 @@ import { normalizeProviderId } from "../agents/provider-id.js"; import { withBundledPluginVitestCompat } from "./bundled-compat.js"; import { normalizePluginsConfig, resolveEffectivePluginActivationState } from "./config-state.js"; import type { PluginLoadOptions } from "./loader.js"; +import { + isActivatedManifestOwner, + passesManifestOwnerBasePolicy, +} from "./manifest-owner-policy.js"; import { loadPluginManifestRegistry, type PluginManifestRecord, @@ -110,22 +114,19 @@ function isProviderPluginEligibleForSetupDiscovery(params: { if (!params.shouldFilterUntrustedWorkspacePlugins || params.plugin.origin !== "workspace") { return true; } - const activation = resolveEffectivePluginActivationState({ - id: params.plugin.id, - origin: params.plugin.origin, - config: params.normalizedConfig, - rootConfig: params.rootConfig, - enabledByDefault: params.plugin.enabledByDefault, - }); - if (activation.activated) { - return true; + if ( + !passesManifestOwnerBasePolicy({ + plugin: params.plugin, + normalizedConfig: params.normalizedConfig, + }) + ) { + return false; } - const explicitlyTrustedButDisabled = - params.normalizedConfig.enabled && - !params.normalizedConfig.deny.includes(params.plugin.id) && - params.normalizedConfig.allow.includes(params.plugin.id) && - params.normalizedConfig.entries[params.plugin.id]?.enabled === false; - return explicitlyTrustedButDisabled; + return isActivatedManifestOwner({ + plugin: params.plugin, + normalizedConfig: params.normalizedConfig, + rootConfig: params.rootConfig, + }); } export function resolveDiscoverableProviderOwnerPluginIds(params: { @@ -166,31 +167,22 @@ function isProviderPluginEligibleForRuntimeOwnerActivation(params: { normalizedConfig: ReturnType; rootConfig?: PluginLoadOptions["config"]; }): boolean { - if (!params.normalizedConfig.enabled) { - return false; - } - if (params.normalizedConfig.deny.includes(params.plugin.id)) { - return false; - } - if (params.normalizedConfig.entries[params.plugin.id]?.enabled === false) { - return false; - } if ( - params.normalizedConfig.allow.length > 0 && - !params.normalizedConfig.allow.includes(params.plugin.id) + !passesManifestOwnerBasePolicy({ + plugin: params.plugin, + normalizedConfig: params.normalizedConfig, + }) ) { return false; } if (params.plugin.origin !== "workspace") { return true; } - return resolveEffectivePluginActivationState({ - id: params.plugin.id, - origin: params.plugin.origin, - config: params.normalizedConfig, + return isActivatedManifestOwner({ + plugin: params.plugin, + normalizedConfig: params.normalizedConfig, rootConfig: params.rootConfig, - enabledByDefault: params.plugin.enabledByDefault, - }).activated; + }); } export function resolveActivatableProviderOwnerPluginIds(params: {