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 = [