mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
fix(doctor): guard tool allowlist manifest metadata
This commit is contained in:
@@ -64,6 +64,74 @@ describe("collectPluginToolAllowlistWarnings", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("skips unreadable manifest ids before healthy tool owners", () => {
|
||||
const poisonedPlugin = {
|
||||
get id(): string {
|
||||
throw new Error("doctor tool allowlist plugin id exploded");
|
||||
},
|
||||
channels: [],
|
||||
cliBackends: [],
|
||||
hooks: [],
|
||||
manifestPath: "/virtual/poisoned/openclaw.plugin.json",
|
||||
origin: "bundled",
|
||||
providers: [],
|
||||
rootDir: "/virtual/poisoned",
|
||||
skills: [],
|
||||
source: "/virtual/poisoned/index.ts",
|
||||
contracts: {
|
||||
tools: ["poisoned_tool"],
|
||||
},
|
||||
};
|
||||
|
||||
const warnings = collectPluginToolAllowlistWarnings({
|
||||
cfg: {
|
||||
plugins: { allow: ["telegram"] },
|
||||
tools: { allow: ["firecrawl_search"] },
|
||||
},
|
||||
manifestRegistry: {
|
||||
diagnostics: [],
|
||||
plugins: [poisonedPlugin, ...manifestRegistry.plugins],
|
||||
} as unknown as PluginManifestRegistry,
|
||||
});
|
||||
|
||||
expect(warnings).toEqual([
|
||||
'- tools.allow references tool "firecrawl_search", owned by plugin "firecrawl", but plugins.allow does not include the owning plugin. Add "firecrawl" to plugins.allow or remove plugins.allow.',
|
||||
]);
|
||||
});
|
||||
|
||||
it("skips unreadable manifest tool contracts before healthy tool owners", () => {
|
||||
const poisonedPlugin = {
|
||||
id: "poisoned-plugin",
|
||||
channels: [],
|
||||
cliBackends: [],
|
||||
hooks: [],
|
||||
manifestPath: "/virtual/poisoned/openclaw.plugin.json",
|
||||
origin: "bundled",
|
||||
providers: [],
|
||||
rootDir: "/virtual/poisoned",
|
||||
skills: [],
|
||||
source: "/virtual/poisoned/index.ts",
|
||||
get contracts(): { tools: string[] } {
|
||||
throw new Error("doctor tool allowlist contracts exploded");
|
||||
},
|
||||
};
|
||||
|
||||
const warnings = collectPluginToolAllowlistWarnings({
|
||||
cfg: {
|
||||
plugins: { allow: ["telegram"] },
|
||||
tools: { allow: ["firecrawl_search"] },
|
||||
},
|
||||
manifestRegistry: {
|
||||
diagnostics: [],
|
||||
plugins: [poisonedPlugin, ...manifestRegistry.plugins],
|
||||
} as unknown as PluginManifestRegistry,
|
||||
});
|
||||
|
||||
expect(warnings).toEqual([
|
||||
'- tools.allow references tool "firecrawl_search", owned by plugin "firecrawl", but plugins.allow does not include the owning plugin. Add "firecrawl" to plugins.allow or remove plugins.allow.',
|
||||
]);
|
||||
});
|
||||
|
||||
it("warns when a tool policy references a known plugin outside plugins.allow", () => {
|
||||
const warnings = collectPluginToolAllowlistWarnings({
|
||||
cfg: {
|
||||
|
||||
@@ -125,8 +125,11 @@ function formatSourceLabelSubject(labels: Iterable<string>): { text: string; ver
|
||||
function collectToolOwners(registry: PluginManifestRegistry): Map<string, string[]> {
|
||||
const owners = new Map<string, string[]>();
|
||||
for (const plugin of registry.plugins) {
|
||||
const pluginId = normalizePluginId(plugin.id);
|
||||
for (const toolNameRaw of plugin.contracts?.tools ?? []) {
|
||||
const pluginId = readManifestPluginId(plugin);
|
||||
if (!pluginId) {
|
||||
continue;
|
||||
}
|
||||
for (const toolNameRaw of readManifestToolContractNames(plugin)) {
|
||||
const toolName = normalizeToolName(toolNameRaw);
|
||||
if (!toolName) {
|
||||
continue;
|
||||
@@ -138,7 +141,53 @@ function collectToolOwners(registry: PluginManifestRegistry): Map<string, string
|
||||
}
|
||||
|
||||
function collectKnownPluginIds(registry: PluginManifestRegistry): Set<string> {
|
||||
return new Set(registry.plugins.map((plugin) => normalizePluginId(plugin.id)));
|
||||
return new Set(
|
||||
registry.plugins
|
||||
.map((plugin) => readManifestPluginId(plugin))
|
||||
.filter((pluginId): pluginId is string => Boolean(pluginId)),
|
||||
);
|
||||
}
|
||||
|
||||
function readManifestPluginId(
|
||||
plugin: PluginManifestRegistry["plugins"][number],
|
||||
): string | undefined {
|
||||
try {
|
||||
return normalizePluginId(plugin.id);
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
function readManifestToolContractNames(
|
||||
plugin: PluginManifestRegistry["plugins"][number],
|
||||
): string[] {
|
||||
let tools: unknown;
|
||||
try {
|
||||
tools = plugin.contracts?.tools;
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
if (!Array.isArray(tools)) {
|
||||
return [];
|
||||
}
|
||||
let length: number;
|
||||
try {
|
||||
length = tools.length;
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
const entries: string[] = [];
|
||||
for (let index = 0; index < length; index += 1) {
|
||||
try {
|
||||
const toolName = tools[index];
|
||||
if (typeof toolName === "string") {
|
||||
entries.push(toolName);
|
||||
}
|
||||
} catch {
|
||||
// Doctor warnings should skip only the unreadable manifest tool row.
|
||||
}
|
||||
}
|
||||
return entries;
|
||||
}
|
||||
|
||||
function collectConfiguredMcpServerNames(cfg: OpenClawConfig): string[] {
|
||||
|
||||
Reference in New Issue
Block a user