diff --git a/src/tools/validate-expression-tool/ValidateExpressionTool.input.schema.ts b/src/tools/validate-expression-tool/ValidateExpressionTool.input.schema.ts index 3684800..2fe04c1 100644 --- a/src/tools/validate-expression-tool/ValidateExpressionTool.input.schema.ts +++ b/src/tools/validate-expression-tool/ValidateExpressionTool.input.schema.ts @@ -3,9 +3,25 @@ import { z } from 'zod'; +// Generous for any realistic hand-written or generated expression (even a +// "match" with thousands of branches is a few KB), while bounding the size of +// the JSON.parse'd/validated structure so a pathological payload can't be used +// to exhaust memory before our own validation logic ever runs. +const MAX_EXPRESSION_JSON_LENGTH = 262_144; // 256 KB + export const ValidateExpressionInputSchema = z.object({ + // `z.any()` in the union matches everything, including oversized strings, so + // `.max()` on the string branch alone would never trigger - the length check + // has to be a refinement applied after the union instead. expression: z .union([z.string(), z.any()]) + .refine( + (value) => + typeof value !== 'string' || value.length <= MAX_EXPRESSION_JSON_LENGTH, + { + message: `Expression JSON string exceeds maximum length of ${MAX_EXPRESSION_JSON_LENGTH} characters` + } + ) .describe( 'Mapbox expression to validate (JSON string or expression array)' ), diff --git a/src/tools/validate-expression-tool/ValidateExpressionTool.ts b/src/tools/validate-expression-tool/ValidateExpressionTool.ts index af95447..82901ca 100644 --- a/src/tools/validate-expression-tool/ValidateExpressionTool.ts +++ b/src/tools/validate-expression-tool/ValidateExpressionTool.ts @@ -29,6 +29,14 @@ const INTERPOLATION_TYPE_ARG_INDEX: Record = { // This cap is far beyond any realistic hand-written or generated expression. const MAX_EXPRESSION_DEPTH = 64; +// The depth cap alone doesn't bound memory: a single huge-but-shallow array +// (e.g. a "match" with millions of branches) sails past it untouched, and +// handing that straight to createPropertyExpression can exhaust the heap - +// which, unlike a stack overflow, is not a catchable JS error and crashes the +// whole process. Checking `.length` is O(1), so oversized arrays/objects are +// rejected before we ever iterate into or copy them. +const MAX_ARRAY_LENGTH = 10_000; + // `StylePropertySpecification` only covers concrete typed properties (color, string, // number, ...), so a literal like `true` or `"red"` would fail type-checking against // any of them. Using an unrecognized `type` makes createPropertyExpression skip return @@ -209,6 +217,14 @@ export class ValidateExpressionTool extends BaseTool< if (!Array.isArray(expression)) { if (typeof expression === 'object') { + if (Object.keys(expression).length > MAX_ARRAY_LENGTH) { + errors.push({ + severity: 'error', + message: `Expression object exceeds maximum size of ${MAX_ARRAY_LENGTH} properties`, + path: path || 'root' + }); + return { depth, blocking: true }; + } return { expressionType: 'literal-object', depth, blocking: false }; } errors.push({ @@ -219,6 +235,15 @@ export class ValidateExpressionTool extends BaseTool< return { depth, blocking: true }; } + if (expression.length > MAX_ARRAY_LENGTH) { + errors.push({ + severity: 'error', + message: `Expression array exceeds maximum size of ${MAX_ARRAY_LENGTH} elements`, + path: path || 'root' + }); + return { depth, blocking: true }; + } + if (expression.length === 0) { errors.push({ severity: 'error', diff --git a/src/tools/validate-style-tool/ValidateStyleTool.input.schema.ts b/src/tools/validate-style-tool/ValidateStyleTool.input.schema.ts index 341542c..c460cb1 100644 --- a/src/tools/validate-style-tool/ValidateStyleTool.input.schema.ts +++ b/src/tools/validate-style-tool/ValidateStyleTool.input.schema.ts @@ -3,13 +3,24 @@ import { z } from 'zod'; +// Generous for any real-world style (full styles with many layers/sources are +// typically well under 1 MB), while bounding the size of the JSON.parse'd +// structure so a pathological payload can't be used to exhaust memory before +// our own validation logic ever runs. Unlike ValidateExpressionInputSchema, +// `.max()` on the string branch alone is sufficient here since the object +// branch (`z.record`) doesn't also match strings. +const MAX_STYLE_JSON_LENGTH = 10 * 1024 * 1024; // 10 MB + /** * Input schema for ValidateStyleTool * Validates Mapbox GL JS style JSON against the style specification */ export const ValidateStyleInputSchema = z.object({ style: z - .union([z.string(), z.record(z.string(), z.unknown())]) + .union([ + z.string().max(MAX_STYLE_JSON_LENGTH), + z.record(z.string(), z.unknown()) + ]) .describe( 'Mapbox style JSON object or JSON string to validate against the Mapbox Style Specification' ) diff --git a/src/tools/validate-style-tool/ValidateStyleTool.ts b/src/tools/validate-style-tool/ValidateStyleTool.ts index 6a6d829..ea32929 100644 --- a/src/tools/validate-style-tool/ValidateStyleTool.ts +++ b/src/tools/validate-style-tool/ValidateStyleTool.ts @@ -47,19 +47,49 @@ function splitPathFromMessage(message: string): { // arbitrarily deep input without risking the same problem. const MAX_STYLE_DEPTH = 64; -function exceedsMaxDepth(value: unknown, depth = 0): boolean { +// Depth alone doesn't bound memory: a single huge-but-shallow array (e.g. a +// paint expression's "match" with millions of branches) sails past the depth +// cap untouched, and handing that straight to validateMapboxStyle can exhaust +// the heap - which, unlike a stack overflow, is not a catchable JS error and +// crashes the whole process. Checking `.length`/key count is O(1), so +// oversized arrays/objects are rejected before we ever iterate into them. +const MAX_STYLE_COLLECTION_SIZE = 10_000; + +type LimitViolation = 'depth' | 'size' | null; + +function findLimitViolation(value: unknown, depth = 0): LimitViolation { if (depth > MAX_STYLE_DEPTH) { - return true; + return 'depth'; } if (Array.isArray(value)) { - return value.some((item) => exceedsMaxDepth(item, depth + 1)); + if (value.length > MAX_STYLE_COLLECTION_SIZE) { + return 'size'; + } + for (const item of value) { + const violation = findLimitViolation(item, depth + 1); + if (violation) { + return violation; + } + } + return null; } if (value && typeof value === 'object') { - return Object.values(value).some((item) => - exceedsMaxDepth(item, depth + 1) - ); + const keys = Object.keys(value); + if (keys.length > MAX_STYLE_COLLECTION_SIZE) { + return 'size'; + } + for (const key of keys) { + const violation = findLimitViolation( + (value as Record)[key], + depth + 1 + ); + if (violation) { + return violation; + } + } + return null; } - return false; + return null; } /** @@ -128,14 +158,14 @@ export class ValidateStyleTool extends BaseTool< style = input.style as MapboxStyle; } - if (exceedsMaxDepth(style)) { + const limitViolation = findLimitViolation(style); + if (limitViolation) { + const message = + limitViolation === 'depth' + ? `style exceeds maximum nesting depth of ${MAX_STYLE_DEPTH}` + : `style contains an array or object exceeding maximum size of ${MAX_STYLE_COLLECTION_SIZE} elements`; return { - content: [ - { - type: 'text', - text: `Error: style exceeds maximum nesting depth of ${MAX_STYLE_DEPTH}` - } - ], + content: [{ type: 'text', text: `Error: ${message}` }], isError: true }; } diff --git a/test/tools/validate-expression-tool/ValidateExpressionTool.test.ts b/test/tools/validate-expression-tool/ValidateExpressionTool.test.ts index ff6b922..1da6dd8 100644 --- a/test/tools/validate-expression-tool/ValidateExpressionTool.test.ts +++ b/test/tools/validate-expression-tool/ValidateExpressionTool.test.ts @@ -339,6 +339,37 @@ describe('ValidateExpressionTool', () => { ) ).toBe(true); }); + + it('should reject a wide (shallow but huge) expression instead of exhausting memory', async () => { + // Depth stays ~1 here - this is the vector the depth cap alone does not catch. + const expression: any = ['match', ['get', 'x']]; + for (let i = 0; i < 20_000; i++) { + expression.push(i, 'red'); + } + expression.push('blue'); + + const start = Date.now(); + const result = await tool.run({ expression }); + expect(Date.now() - start).toBeLessThan(1000); + + expect(result.isError).toBe(false); + const parsed = JSON.parse(result.content[0].text); + expect(parsed.valid).toBe(false); + expect( + parsed.errors.some((e: any) => + e.message.includes('exceeds maximum size') + ) + ).toBe(true); + }); + + it('should reject a JSON string expression exceeding the maximum length', async () => { + const expression = JSON.stringify(['get', 'x'.repeat(300_000)]); + + const result = await tool.run({ expression }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('exceeds maximum length'); + }); }); describe('nested expressions', () => { diff --git a/test/tools/validate-style-tool/ValidateStyleTool.test.ts b/test/tools/validate-style-tool/ValidateStyleTool.test.ts index 501aa58..3920e7c 100644 --- a/test/tools/validate-style-tool/ValidateStyleTool.test.ts +++ b/test/tools/validate-style-tool/ValidateStyleTool.test.ts @@ -357,5 +357,45 @@ describe('ValidateStyleTool', () => { expect(result.isError).toBe(true); expect(result.content[0].text).toContain('exceeds maximum nesting depth'); }); + + it('should reject a style with a wide (shallow but huge) expression instead of exhausting memory', async () => { + // Depth stays ~1 here - this is the vector the depth cap alone does not catch. + const expression: any = ['match', ['get', 'x']]; + for (let i = 0; i < 20_000; i++) { + expression.push(i, 'red'); + } + expression.push('blue'); + const style = { + version: 8, + sources: {}, + layers: [ + { + id: 'l', + type: 'background', + paint: { 'background-opacity': expression } + } + ] + }; + + const start = Date.now(); + const result = await tool.run({ style }); + expect(Date.now() - start).toBeLessThan(1000); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('exceeding maximum size'); + }); + + it('should reject a JSON string style exceeding the maximum length', async () => { + const style = JSON.stringify({ + version: 8, + sources: {}, + layers: [], + padding: 'x'.repeat(11 * 1024 * 1024) + }); + + const result = await tool.run({ style }); + + expect(result.isError).toBe(true); + }); }); });