From 6a45a8f6a26ac1755d3aeb3ea1f95b6a2d2b659e Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Thu, 1 Oct 2026 15:34:53 -0700 Subject: [PATCH] fix: check only own properties when validating tool arguments Inherited names such as constructor, toString and __proto__ passed as declared properties despite additionalProperties: false, and a schema-valued additionalProperties was accepted but never enforced. Validation now uses own properties, and definitions must give additionalProperties as a boolean. --- src/tools/catalog.test.mjs | 29 +++++++++++++++++++++++++++++ src/tools/schema.js | 13 +++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/tools/catalog.test.mjs b/src/tools/catalog.test.mjs index 4e2a5d105..cdd492d16 100644 --- a/src/tools/catalog.test.mjs +++ b/src/tools/catalog.test.mjs @@ -203,3 +203,32 @@ test('the validator checks every supported keyword', () => { 'arguments.n must be a number', ]); }); + +test('inherited names are not declared properties, and schema-valued additionalProperties is refused', () => { + const schema = { + type: 'object', + properties: { name: { type: 'string' } }, + additionalProperties: false, + }; + for (const name of ['constructor', 'toString', '__proto__']) + assert.deepEqual(validateValue(schema, JSON.parse(`{"${name}": 1}`)), [ + `arguments has unknown property ${name}`, + ]); + assert.deepEqual(validateValue({ ...schema, required: ['toString'] }, {}), [ + 'arguments is missing toString', + ]); + assert.throws( + () => + defineTool({ + name: 'loose', + title: 'Loose', + description: 'Promises checks it cannot make.', + inputSchema: { + type: 'object', + additionalProperties: { type: 'string' }, + }, + run: async () => ({ summary: '', data: {} }), + }), + /additionalProperties must be true or false/, + ); +}); diff --git a/src/tools/schema.js b/src/tools/schema.js index 942d6ff1f..7c40c3eb5 100644 --- a/src/tools/schema.js +++ b/src/tools/schema.js @@ -44,6 +44,12 @@ export function assertSupportedSchema(schema, path = 'schema') { if (!TYPES.has(schema.type)) throw new TypeError(`${path} has unsupported type ${schema.type}`); if (schema.pattern !== undefined) new RegExp(schema.pattern, 'u'); + // Only the boolean form is checked; a schema here would go unenforced. + if ( + schema.additionalProperties !== undefined && + typeof schema.additionalProperties !== 'boolean' + ) + throw new TypeError(`${path}.additionalProperties must be true or false`); for (const [name, child] of Object.entries(schema.properties || {})) assertSupportedSchema(child, `${path}.${name}`); if (schema.items) assertSupportedSchema(schema.items, `${path}[]`); @@ -63,11 +69,14 @@ export function validateValue(schema, value, path = 'arguments') { break; } for (const name of schema.required || []) { - if (value[name] === undefined) fail(`is missing ${name}`); + if (!Object.hasOwn(value, name) || value[name] === undefined) + fail(`is missing ${name}`); } + // Own properties only, so names such as constructor or __proto__ are + // not mistaken for declared ones. const properties = schema.properties || {}; for (const [name, child] of Object.entries(value)) { - if (properties[name]) + if (Object.hasOwn(properties, name)) problems.push( ...validateValue(properties[name], child, `${path}.${name}`), );