fix(plugins): guard plugin tool descriptor reads

This commit is contained in:
Vincent Koc
2026-06-04 07:44:43 +02:00
parent 18ecb82034
commit 4c0d955808
3 changed files with 571 additions and 44 deletions

View File

@@ -144,23 +144,40 @@ function asJsonObject(value: unknown): JsonObject {
return value as JsonObject;
}
function readToolField(tool: AnyAgentTool, key: string): unknown {
try {
return Reflect.get(tool, key);
} catch {
return undefined;
}
}
function readToolStringField(tool: AnyAgentTool, key: string): string | undefined {
const value = readToolField(tool, key);
return typeof value === "string" ? value : undefined;
}
export function capturePluginToolDescriptor(params: {
pluginId: string;
tool: AnyAgentTool;
toolName: string;
description: string;
parameters: unknown;
optional: boolean;
}): CachedPluginToolDescriptor {
const label = (params.tool as { label?: unknown }).label;
const label = readToolStringField(params.tool, "label");
const title = typeof label === "string" && label.trim() ? label.trim() : undefined;
const displaySummary = readToolStringField(params.tool, "displaySummary");
return {
...(params.tool.displaySummary ? { displaySummary: params.tool.displaySummary } : {}),
...(displaySummary ? { displaySummary } : {}),
optional: params.optional,
descriptor: {
name: params.tool.name,
name: params.toolName,
...(title ? { title } : {}),
description: params.tool.description,
inputSchema: asJsonObject(params.tool.parameters),
description: params.description,
inputSchema: asJsonObject(params.parameters),
owner: { kind: "plugin", pluginId: params.pluginId },
executor: { kind: "plugin", pluginId: params.pluginId, toolName: params.tool.name },
executor: { kind: "plugin", pluginId: params.pluginId, toolName: params.toolName },
},
};
}

View File

@@ -164,6 +164,146 @@ function createMalformedTool(name: string) {
};
}
function createToolWithUnreadableParameters(name: string) {
return {
name,
description: `${name} tool`,
get parameters() {
throw new Error(`${name} parameters exploded`);
},
async execute() {
return { content: [{ type: "text", text: "bad" }] };
},
};
}
function createToolWithUnstableParameters(name: string) {
let readCount = 0;
return {
name,
description: `${name} tool`,
get parameters() {
readCount += 1;
if (readCount > 1) {
throw new Error(`${name} parameters exploded`);
}
return { type: "object", properties: {} };
},
async execute() {
return { content: [{ type: "text", text: "bad" }] };
},
};
}
function createToolWithUnreadableDescription(name: string) {
return {
name,
get description() {
throw new Error(`${name} description exploded`);
},
parameters: { type: "object", properties: {} },
async execute() {
return { content: [{ type: "text", text: "bad" }] };
},
};
}
function createToolWithUnreadableOptionalMetadata(name: string) {
return {
name,
description: `${name} tool`,
get label() {
throw new Error(`${name} label exploded`);
},
get displaySummary() {
throw new Error(`${name} display summary exploded`);
},
parameters: { type: "object", properties: {} },
async execute() {
return { content: [{ type: "text", text: "ok" }] };
},
};
}
function createToolWithInitiallyUnreadableName(name: string) {
let readCount = 0;
return {
get name() {
readCount += 1;
if (readCount === 1) {
throw new Error(`${name} name exploded`);
}
return name;
},
description: `${name} tool`,
parameters: { type: "object", properties: {} },
async execute() {
return { content: [{ type: "text", text: "bad" }] };
},
};
}
function createToolWithChangingName(firstName: string, nextName: string) {
let readCount = 0;
return {
get name() {
readCount += 1;
return readCount === 1 ? firstName : nextName;
},
description: `${firstName} tool`,
parameters: { type: "object", properties: {} },
async execute() {
return { content: [{ type: "text", text: "ok" }] };
},
};
}
function createToolWithNonConfigurableChangingName(firstName: string, nextName: string) {
let readCount = 0;
const tool = {
description: `${firstName} tool`,
parameters: { type: "object", properties: {} },
async execute() {
return { content: [{ type: "text", text: "ok" }] };
},
};
Object.defineProperty(tool, "name", {
configurable: false,
enumerable: true,
get() {
readCount += 1;
return readCount <= 2 ? firstName : nextName;
},
});
return tool;
}
function createToolWithThrowingNameDescriptor(name: string) {
return new Proxy(
{
description: `${name} tool`,
parameters: { type: "object", properties: {} },
async execute() {
return { content: [{ type: "text", text: "ok" }] };
},
},
{
get(target, property, receiver) {
if (property === "name") {
return name;
}
return Reflect.get(target, property, receiver);
},
getOwnPropertyDescriptor(target, property) {
if (property === "name") {
throw new Error(`${name} descriptor exploded`);
}
return Reflect.getOwnPropertyDescriptor(target, property);
},
},
);
}
function installConsoleMethodSpy(method: "log" | "warn") {
const spy = vi.fn();
loggingState.rawConsole = {
@@ -1939,6 +2079,236 @@ describe("resolvePluginTools optional tools", () => {
);
});
it("skips tools with unreadable parameters while keeping valid sibling tools", () => {
const registry = setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["broken_tool", "valid_tool"],
factory: () => [createToolWithUnreadableParameters("broken_tool"), makeTool("valid_tool")],
},
]);
const tools = resolvePluginTools(createResolveToolsParams());
expectResolvedToolNames(tools, ["valid_tool"]);
expectSingleDiagnosticMessage(
registry.diagnostics,
"plugin tool is malformed (schema-bug): broken_tool missing parameters object",
);
});
it("skips tools whose parameters become unreadable before descriptor caching", () => {
const registry = setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["broken_tool", "valid_tool"],
factory: () => [createToolWithUnstableParameters("broken_tool"), makeTool("valid_tool")],
},
]);
const tools = resolvePluginTools(createResolveToolsParams());
expectResolvedToolNames(tools, ["valid_tool"]);
expectSingleDiagnosticMessage(
registry.diagnostics,
"plugin tool is malformed (schema-bug): broken_tool missing parameters object",
);
});
it("skips tools with unreadable descriptions while keeping valid sibling tools", () => {
const registry = setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["broken_tool", "valid_tool"],
factory: () => [createToolWithUnreadableDescription("broken_tool"), makeTool("valid_tool")],
},
]);
const tools = resolvePluginTools(createResolveToolsParams());
expectResolvedToolNames(tools, ["valid_tool"]);
expectSingleDiagnosticMessage(
registry.diagnostics,
"plugin tool is malformed (schema-bug): broken_tool missing description string",
);
});
it("caches tool descriptors without forcing optional metadata getters", () => {
const factory = vi.fn(() => createToolWithUnreadableOptionalMetadata("noisy_tool"));
setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["noisy_tool"],
factory,
},
]);
const first = resolvePluginTools(createResolveToolsParams());
const second = resolvePluginTools(createResolveToolsParams());
expectResolvedToolNames(first, ["noisy_tool"]);
expectResolvedToolNames(second, ["noisy_tool"]);
expect(second[0]?.description).toBe("noisy_tool tool");
expect(second[0]?.label).toBe("noisy_tool");
expect(second[0]?.displaySummary).toBeUndefined();
expect(factory).toHaveBeenCalledTimes(1);
});
it("does not let unreadable names bypass tool deny policy", () => {
const registry = setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["blocked_tool", "valid_tool"],
factory: () => [
createToolWithInitiallyUnreadableName("blocked_tool"),
makeTool("valid_tool"),
],
},
]);
const tools = resolvePluginTools(createResolveToolsParams({ toolDenylist: ["blocked_tool"] }));
expectResolvedToolNames(tools, ["valid_tool"]);
expectSingleDiagnosticMessage(registry.diagnostics, "plugin tool is malformed (schema-bug):");
});
it("skips tools whose names change between policy and registration", () => {
const registry = setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["safe_tool", "valid_tool"],
factory: () => [
createToolWithChangingName("safe_tool", "blocked_tool"),
makeTool("valid_tool"),
],
},
]);
const tools = resolvePluginTools(createResolveToolsParams({ toolDenylist: ["blocked_tool"] }));
expectResolvedToolNames(tools, ["valid_tool"]);
expectSingleDiagnosticMessage(
registry.diagnostics,
"plugin tool is malformed (schema-bug): safe_tool unstable name",
);
});
it("rejects non-configurable accessor tool names that cannot be pinned", () => {
const registry = setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["safe_tool", "valid_tool"],
factory: () => [
createToolWithNonConfigurableChangingName("safe_tool", "blocked_tool"),
makeTool("valid_tool"),
],
},
]);
const tools = resolvePluginTools(createResolveToolsParams({ toolDenylist: ["blocked_tool"] }));
expectResolvedToolNames(tools, ["valid_tool"]);
expectSingleDiagnosticMessage(
registry.diagnostics,
"plugin tool is malformed (schema-bug): safe_tool unstable name",
);
});
it("skips tools whose name descriptor cannot be inspected", () => {
const registry = setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["broken_tool", "valid_tool"],
factory: () => [
createToolWithThrowingNameDescriptor("broken_tool"),
makeTool("valid_tool"),
],
},
]);
const tools = resolvePluginTools(createResolveToolsParams());
expectResolvedToolNames(tools, ["valid_tool"]);
expectSingleDiagnosticMessage(registry.diagnostics, "plugin tool is malformed (schema-bug):");
});
it("keeps tools with non-configurable stable names usable", async () => {
const nonConfigurableNameTool = {
...makeTool("frozen_tool"),
async execute() {
return { content: [{ type: "text", text: "frozen-ok" }] };
},
};
Object.defineProperty(nonConfigurableNameTool, "name", {
configurable: false,
enumerable: true,
value: "frozen_tool",
writable: false,
});
setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["frozen_tool"],
factory: () => nonConfigurableNameTool,
},
]);
const tools = resolvePluginTools(createResolveToolsParams());
expectResolvedToolNames(tools, ["frozen_tool"]);
expect(() => ({ ...tools[0] })).not.toThrow();
await expect(tools[0]?.execute("call", {}, undefined)).resolves.toEqual({
content: [{ type: "text", text: "frozen-ok" }],
});
});
it("rejects non-configurable writable tool names that cannot be pinned", () => {
const registry = setRegistry([
{
pluginId: "schema-bug",
optional: false,
source: "/tmp/schema-bug.js",
names: ["safe_tool", "valid_tool"],
factory: () => {
const tool = makeTool("safe_tool");
Object.defineProperty(tool, "name", {
configurable: false,
enumerable: true,
value: "safe_tool",
writable: true,
});
return [tool, makeTool("valid_tool")];
},
},
]);
const tools = resolvePluginTools(createResolveToolsParams());
expectResolvedToolNames(tools, ["valid_tool"]);
expectSingleDiagnosticMessage(
registry.diagnostics,
"plugin tool is malformed (schema-bug): safe_tool unstable name",
);
});
it("warns with plugin factory timing details when a factory is slow", () => {
vi.useFakeTimers({ now: 0 });
const warnSpy = installConsoleMethodSpy("warn");

View File

@@ -71,6 +71,10 @@ type PluginToolFactoryTiming = {
};
type PluginToolFactoryResult = AnyAgentTool | AnyAgentTool[] | null | undefined;
type PluginToolCandidate = {
tool: unknown;
name: string;
};
const log = createSubsystemLogger("plugins/tools");
const PLUGIN_TOOL_FACTORY_WARN_TOTAL_MS = 5_000;
@@ -79,6 +83,7 @@ const PLUGIN_TOOL_FACTORY_SUMMARY_LIMIT = 20;
const pluginToolMeta = new WeakMap<AnyAgentTool, PluginToolMeta>();
const scopedPluginTools = new WeakMap<AnyAgentTool, Map<string, AnyAgentTool>>();
const unreadablePluginToolProperties = new WeakMap<object, Set<PropertyKey>>();
/** Attaches plugin ownership metadata to a concrete agent tool instance. */
export function setPluginToolMeta(tool: AnyAgentTool, meta: PluginToolMeta): void {
@@ -317,12 +322,90 @@ function isOptionalToolEntryPotentiallyAllowed(params: {
return params.names.some((name) => params.allowlist.has(normalizeToolName(name)));
}
function readPluginToolName(tool: unknown): string {
function markPluginToolPropertyUnreadable(tool: object, property: PropertyKey): void {
const existing = unreadablePluginToolProperties.get(tool);
if (existing) {
existing.add(property);
return;
}
unreadablePluginToolProperties.set(tool, new Set([property]));
}
function wasPluginToolPropertyUnreadable(tool: unknown, property: PropertyKey): boolean {
return (
typeof tool === "object" &&
tool !== null &&
unreadablePluginToolProperties.get(tool)?.has(property) === true
);
}
function readPluginToolProperty(tool: unknown, property: keyof AnyAgentTool): unknown {
if (!isRecord(tool)) {
return undefined;
}
try {
return Reflect.get(tool, property, tool);
} catch {
markPluginToolPropertyUnreadable(tool, property);
return undefined;
}
}
function readPluginToolName(tool: unknown): string {
const name = readPluginToolProperty(tool, "name");
if (typeof name !== "string") {
return "";
}
// Optional-tool allowlists need a best-effort name before full shape validation.
return typeof tool.name === "string" ? tool.name.trim() : "";
return name.trim();
}
function preparePluginToolWithStableName(
tool: AnyAgentTool,
name: string,
): AnyAgentTool | undefined {
const currentName = readPluginToolName(tool);
if (currentName !== name || wasPluginToolPropertyUnreadable(tool, "name")) {
return undefined;
}
let nameDescriptor: PropertyDescriptor | undefined;
let extensible: boolean;
try {
nameDescriptor = Reflect.getOwnPropertyDescriptor(tool, "name");
extensible = Reflect.isExtensible(tool);
} catch {
return undefined;
}
if (nameDescriptor && !nameDescriptor.configurable) {
return "value" in nameDescriptor && nameDescriptor.value === name && !nameDescriptor.writable
? tool
: undefined;
}
if (!extensible && !nameDescriptor) {
return undefined;
}
return new Proxy<AnyAgentTool>(tool, {
get(target, prop, receiver) {
if (prop === "name") {
return name;
}
return Reflect.get(target, prop, receiver);
},
getOwnPropertyDescriptor(target, prop) {
if (prop === "name") {
if (!extensible && !nameDescriptor) {
return undefined;
}
return {
configurable: true,
enumerable: nameDescriptor?.enumerable ?? true,
value: name,
writable: false,
};
}
return Reflect.getOwnPropertyDescriptor(target, prop);
},
});
}
function toElapsedMs(value: number): number {
@@ -450,18 +533,32 @@ function shouldWarnPluginToolFactoryTimings(params: {
);
}
function describeMalformedPluginTool(tool: unknown): string | undefined {
function describeMalformedPluginTool(
tool: unknown,
name = readPluginToolName(tool),
): string | undefined {
if (!isRecord(tool)) {
return "tool must be an object";
}
const name = readPluginToolName(tool);
if (!name) {
if (!name || wasPluginToolPropertyUnreadable(tool, "name")) {
return "missing non-empty name";
}
if (typeof tool.execute !== "function") {
if (
typeof readPluginToolProperty(tool, "description") !== "string" ||
wasPluginToolPropertyUnreadable(tool, "description")
) {
return `${name} missing description string`;
}
if (
typeof readPluginToolProperty(tool, "execute") !== "function" ||
wasPluginToolPropertyUnreadable(tool, "execute")
) {
return `${name} missing execute function`;
}
if (!isRecord(tool.parameters)) {
if (
!isRecord(readPluginToolProperty(tool, "parameters")) ||
wasPluginToolPropertyUnreadable(tool, "parameters")
) {
return `${name} missing parameters object`;
}
return undefined;
@@ -696,12 +793,22 @@ function createCachedDescriptorPluginTool(params: {
const resolved = resolvePluginToolFactory(candidate, params.ctx);
const listRaw: unknown[] = Array.isArray(resolved) ? resolved : resolved ? [resolved] : [];
for (const toolRaw of listRaw) {
const malformedReason = describeMalformedPluginTool(toolRaw);
if (!isRecord(toolRaw)) {
continue;
}
const candidateName = readPluginToolName(toolRaw);
if (!candidateName || wasPluginToolPropertyUnreadable(toolRaw, "name")) {
continue;
}
const malformedReason = describeMalformedPluginTool(toolRaw, candidateName);
if (malformedReason) {
continue;
}
const runtimeTool = toolRaw as AnyAgentTool;
if (normalizeToolName(readPluginToolName(runtimeTool)) === requestedToolName) {
const runtimeTool = preparePluginToolWithStableName(
toolRaw as AnyAgentTool,
candidateName,
);
if (runtimeTool && normalizeToolName(candidateName) === requestedToolName) {
return runtimeTool;
}
}
@@ -1215,6 +1322,29 @@ export function resolvePluginTools(params: {
continue;
}
const listRaw: unknown[] = Array.isArray(resolved) ? resolved : [resolved];
const recordMalformedTool = (reason: string) => {
const message = `plugin tool is malformed (${entry.pluginId}): ${reason}`;
context.logger.error(message);
registry.diagnostics.push({
level: "error",
pluginId: entry.pluginId,
source: entry.source,
message,
});
};
const namedListRaw: PluginToolCandidate[] = [];
for (const tool of listRaw) {
if (!isRecord(tool)) {
namedListRaw.push({ tool, name: "" });
continue;
}
const name = readPluginToolName(tool);
if (!name || wasPluginToolPropertyUnreadable(tool, "name")) {
recordMalformedTool("missing non-empty name");
continue;
}
namedListRaw.push({ tool, name });
}
const selectedManifestToolNames =
manifestPlugin && availabilityNames.length > 0
? new Set(allowlistNames.map((name) => normalizeToolName(name)))
@@ -1224,13 +1354,12 @@ export function resolvePluginTools(params: {
? new Set(availabilityNames.map((name) => normalizeToolName(name)))
: undefined;
const availableList = manifestPlugin
? listRaw.filter((tool) => {
const toolName = readPluginToolName(tool);
const normalizedToolName = normalizeToolName(toolName);
? namedListRaw.filter((candidate) => {
const normalizedToolName = normalizeToolName(candidate.name);
if (
isManifestToolOptional(manifestPlugin, toolName) &&
isManifestToolOptional(manifestPlugin, candidate.name) &&
!isOptionalToolAllowed({
toolName,
toolName: candidate.name,
pluginId: entry.pluginId,
allowlist,
})
@@ -1246,25 +1375,25 @@ export function resolvePluginTools(params: {
}
return isManifestToolNameAvailable({
plugin: manifestPlugin,
toolName,
toolName: candidate.name,
config: params.context.runtimeConfig ?? context.config,
env,
hasAuthForProvider: params.hasAuthForProvider,
});
})
: listRaw;
: namedListRaw;
const policyAvailableList = availableList.filter(
(tool) =>
(candidate) =>
!denylistBlocksPluginTool({
pluginId: entry.pluginId,
toolName: readPluginToolName(tool),
toolName: candidate.name,
denylist,
}),
);
const list = entry.optional
? policyAvailableList.filter((tool) =>
? policyAvailableList.filter((candidate) =>
isOptionalToolAllowed({
toolName: readPluginToolName(tool),
toolName: candidate.name,
pluginId: entry.pluginId,
allowlist,
}),
@@ -1274,26 +1403,34 @@ export function resolvePluginTools(params: {
continue;
}
const normalizedNameSet = new Set<string>();
for (const toolRaw of list) {
for (const candidate of list) {
// Plugin factories run at request time and can return arbitrary values; isolate
// malformed tools here so one bad plugin tool cannot poison every provider.
const malformedReason = describeMalformedPluginTool(toolRaw);
const malformedReason = describeMalformedPluginTool(candidate.tool, candidate.name);
if (malformedReason) {
const message = `plugin tool is malformed (${entry.pluginId}): ${malformedReason}`;
context.logger.error(message);
registry.diagnostics.push({
level: "error",
pluginId: entry.pluginId,
source: entry.source,
message,
});
recordMalformedTool(malformedReason);
continue;
}
const tool = preparePluginToolWithStableName(candidate.tool as AnyAgentTool, candidate.name);
if (!tool) {
recordMalformedTool(`${candidate.name} unstable name`);
continue;
}
const toolName = candidate.name;
const description = readPluginToolProperty(tool, "description");
if (typeof description !== "string" || wasPluginToolPropertyUnreadable(tool, "description")) {
recordMalformedTool(`${toolName} missing description string`);
continue;
}
const parameters = readPluginToolProperty(tool, "parameters");
if (!isRecord(parameters) || wasPluginToolPropertyUnreadable(tool, "parameters")) {
recordMalformedTool(`${toolName} missing parameters object`);
continue;
}
const tool = toolRaw as AnyAgentTool;
const undeclared = entry.declaredNames
? findUndeclaredPluginToolNames({
declaredNames: entry.declaredNames,
toolNames: [tool.name],
toolNames: [toolName],
})
: [];
if (undeclared.length > 0) {
@@ -1307,9 +1444,9 @@ export function resolvePluginTools(params: {
});
continue;
}
const normalizedToolName = normalizeToolName(tool.name);
const normalizedToolName = normalizeToolName(toolName);
if (normalizedNameSet.has(normalizedToolName) || existingNormalized.has(normalizedToolName)) {
const message = `plugin tool name conflict (${entry.pluginId}): ${tool.name}`;
const message = `plugin tool name conflict (${entry.pluginId}): ${toolName}`;
if (!params.suppressNameConflicts) {
context.logger.error(message);
registry.diagnostics.push({
@@ -1322,19 +1459,19 @@ export function resolvePluginTools(params: {
continue;
}
normalizedNameSet.add(normalizedToolName);
existing.add(tool.name);
existing.add(toolName);
existingNormalized.add(normalizedToolName);
const optional = isPluginToolOptional({
entry,
manifestPlugin,
toolName: tool.name,
toolName,
});
pluginToolMeta.set(tool, {
pluginId: entry.pluginId,
optional,
trustedLocalMedia: isTrustedManifestLocalMediaTool({
manifestPlugin,
toolName: tool.name,
toolName,
}),
});
if (manifestPlugin) {
@@ -1343,6 +1480,9 @@ export function resolvePluginTools(params: {
capturePluginToolDescriptor({
pluginId: entry.pluginId,
tool,
toolName,
description,
parameters,
optional,
}),
);