From 1d013c219bb0983cfc5481431974f88ee1c8d7c5 Mon Sep 17 00:00:00 2001 From: Sean Sun <1194458432@qq.com> Date: Tue, 23 Jun 2026 10:20:17 +0800 Subject: [PATCH] plugins: clarify allowlist warning when entries don't match discovered ids (#68389) * plugins: clarify allowlist warning when entries don't match discovered ids When plugins.allow contains entries that do not match any discovered plugin id (for example a channel id like feishu instead of the real plugin id openclaw-lark), stop emitting the misleading "plugins.allow is empty" warning. Emit a specific mismatch warning that lists the unknown allow entries alongside the discovered plugin ids and points users at the plugin id rather than a channel id or npm package name. Refs #68352 * plugins: treat bundled plugin ids as valid allow entries Codex P2 on #68389: warnWhenAllowlistIsOpen computed allowHasMatch against the auto-discoverable (workspace + global) subset only, so a legitimate bundled-only allowlist like plugins.allow=['telegram'] would trip the new mismatch warning whenever any non-bundled plugin happened to be discoverable alongside it. Compare allow entries to every discovered plugin id (bundled + workspace + global) for both the short-circuit and the unmatched- entries computation. The warning text stays scoped to non-bundled auto-discoverable plugins; we just stop flagging bundled ids as 'does not match any discovered plugin ids'. Add a regression test that covers the bundled-only allowlist + non-bundled workspace plugin combination. Refs #68352 * chore: drop release-owned CHANGELOG entry (AGENTS.md: changelog is release-generated) * plugins: clarify allowlist warning when entries do not match plugin ids --------- Co-authored-by: Sean Sun Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com> --- src/plugins/loader-provenance.ts | 26 +++++++-- src/plugins/loader.test.ts | 98 ++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 4 deletions(-) diff --git a/src/plugins/loader-provenance.ts b/src/plugins/loader-provenance.ts index 7c0b91ef646c..9bf73a04a98d 100644 --- a/src/plugins/loader-provenance.ts +++ b/src/plugins/loader-provenance.ts @@ -223,15 +223,20 @@ export function warnWhenAllowlistIsOpen(params: { if (!params.pluginsEnabled) { return; } - if (params.allow.length > 0) { - return; - } const autoDiscoverable = params.discoverablePlugins.filter( (entry) => entry.origin === "workspace" || entry.origin === "global", ); if (autoDiscoverable.length === 0) { return; } + // Match allow entries against every discovered plugin id, including bundled ids. Otherwise a + // valid bundled-only allowlist would look mismatched whenever workspace/global plugins exist. + const allDiscoveredIds = new Set(params.discoverablePlugins.map((entry) => entry.id)); + const hasConfiguredAllowlist = params.allow.length > 0; + const allowHasDiscoveredMatch = params.allow.some((id) => allDiscoveredIds.has(id)); + if (hasConfiguredAllowlist && allowHasDiscoveredMatch) { + return; + } if (params.warningCache.hasOpenAllowlistWarning(params.warningCacheKey)) { return; } @@ -241,8 +246,21 @@ export function warnWhenAllowlistIsOpen(params: { .join(", "); const extra = autoDiscoverable.length > 6 ? ` (+${autoDiscoverable.length - 6} more)` : ""; params.warningCache.recordOpenAllowlistWarning(params.warningCacheKey); + if (!hasConfiguredAllowlist) { + params.logger.warn( + `[plugins] plugins.allow is empty; discovered non-bundled plugins may auto-load: ${preview}${extra}. Set plugins.allow to explicit trusted ids.`, + ); + return; + } + const unmatchedEntries = params.allow.filter((id) => !allDiscoveredIds.has(id)); + const unmatchedPreview = unmatchedEntries + .slice(0, 6) + .map((id) => `"${id}"`) + .join(", "); + const unmatchedExtra = + unmatchedEntries.length > 6 ? ` (+${unmatchedEntries.length - 6} more)` : ""; params.logger.warn( - `[plugins] plugins.allow is empty; discovered non-bundled plugins may auto-load: ${preview}${extra}. Set plugins.allow to explicit trusted ids.`, + `[plugins] plugins.allow entries ${unmatchedPreview}${unmatchedExtra} do not match any discovered plugin ids; discovered non-bundled plugins: ${preview}${extra}. Use the plugin id (not a channel id or npm package name).`, ); } diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index 4e3f5061d03e..61088283bc6f 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -8626,6 +8626,104 @@ module.exports = { }); }); + it("warns when plugins.allow entries do not match any discovered plugin ids", () => { + useNoBundledPlugins(); + clearPluginLoaderCache(); + const { workspaceDir } = writeWorkspacePlugin({ + id: "warn-mismatch-allow-plugin", + }); + const warnings: string[] = []; + loadOpenClawPlugins({ + cache: false, + workspaceDir, + logger: createWarningLogger(warnings), + config: { + plugins: { + enabled: true, + // User configured a channel-style id that does not match the real plugin id. + allow: ["warn-mismatch-allow-channel"], + }, + }, + }); + const emptyWarnings = warnings.filter((msg) => msg.includes("plugins.allow is empty")); + const mismatchWarnings = warnings.filter((msg) => + msg.includes("do not match any discovered plugin ids"), + ); + expect(emptyWarnings, "should not emit empty-allowlist warning").toHaveLength(0); + expect(mismatchWarnings, "should emit mismatch warning once").toHaveLength(1); + expect(mismatchWarnings[0]).toContain(`"warn-mismatch-allow-channel"`); + expect(mismatchWarnings[0]).toContain("warn-mismatch-allow-plugin"); + expect(mismatchWarnings[0]).toContain("Use the plugin id"); + }); + + it("stays quiet when plugins.allow contains at least one matching plugin id", () => { + useNoBundledPlugins(); + clearPluginLoaderCache(); + const { workspaceDir } = writeWorkspacePlugin({ + id: "warn-partial-allow-plugin", + }); + const warnings: string[] = []; + loadOpenClawPlugins({ + cache: false, + workspaceDir, + logger: createWarningLogger(warnings), + config: { + plugins: { + enabled: true, + // Allow contains one real plugin id plus a stray channel-style entry. + allow: ["warn-partial-allow-plugin", "warn-partial-allow-channel"], + }, + }, + }); + const openAllowWarnings = warnings.filter( + (msg) => + msg.includes("plugins.allow is empty") || + msg.includes("do not match any discovered plugin ids"), + ); + expect(openAllowWarnings, "should not emit allowlist warning when one id matches").toHaveLength( + 0, + ); + }); + + it("stays quiet when plugins.allow matches only bundled plugin ids, even while workspace/global plugins are present", () => { + // Regression for Codex P2 feedback on #68389: the mismatch warning should be computed + // against the full discovered plugin set (bundled + workspace + global), not only the + // workspace/global subset that the warning talks about. An allowlist that intentionally + // trusts bundled ids like ["telegram"] is valid and must not trip the mismatch path just + // because some unrelated non-bundled plugin happens to be auto-discoverable. + useNoBundledPlugins(); + clearPluginLoaderCache(); + writeBundledPlugin({ + id: "warn-bundled-allow-only-plugin", + body: simplePluginBody("warn-bundled-allow-only-plugin"), + }); + const { workspaceDir } = writeWorkspacePlugin({ + id: "warn-noise-workspace-plugin", + }); + const warnings: string[] = []; + loadOpenClawPlugins({ + cache: false, + workspaceDir, + logger: createWarningLogger(warnings), + config: { + plugins: { + enabled: true, + // Allowlist intentionally only trusts a bundled plugin id. + allow: ["warn-bundled-allow-only-plugin"], + }, + }, + }); + const openAllowWarnings = warnings.filter( + (msg) => + msg.includes("plugins.allow is empty") || + msg.includes("do not match any discovered plugin ids"), + ); + expect( + openAllowWarnings, + "bundled-only allowlists should not trip the mismatch warning", + ).toHaveLength(0); + }); + it("handles workspace-discovered plugins according to trust and precedence", () => { useNoBundledPlugins(); const scenarios = [