mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
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 <lyfuci11@gmail.com> Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com>
This commit is contained in:
@@ -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).`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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 = [
|
||||
|
||||
Reference in New Issue
Block a user