diff --git a/packages/backend/src/connectors/engines/rest.engine.spec.ts b/packages/backend/src/connectors/engines/rest.engine.spec.ts index a02dbcd2..5a72f609 100644 --- a/packages/backend/src/connectors/engines/rest.engine.spec.ts +++ b/packages/backend/src/connectors/engines/rest.engine.spec.ts @@ -190,6 +190,22 @@ describe('RestEngine', () => { expect(sent.params).not.toHaveProperty('__rawquery'); }); + it('drops __rawquery keys that would reach the prototype chain', async () => { + // The fragment comes from a tool argument, so from the model. + mockedAxios.mockResolvedValue({ data: {} }); + + await engine.execute( + { baseUrl: 'https://api.example.com', authType: 'NONE' }, + { method: 'GET', path: '/article', queryParams: { __rawquery: '$filter' } }, + { filter: '__proto__=x&constructor=y&prototype=z&name-eq=ok' }, + ); + + const sent = mockedAxios.mock.calls[0][0] as unknown as { params: Record }; + expect(Object.keys(sent.params)).toEqual(['name-eq']); + expect(Object.getPrototypeOf(sent.params)).toBe(Object.prototype); + expect(({} as Record).x).toBeUndefined(); + }); + it('omits __rawquery entirely when the source param is absent', async () => { mockedAxios.mockResolvedValue({ data: {} }); diff --git a/packages/backend/src/connectors/engines/rest.engine.ts b/packages/backend/src/connectors/engines/rest.engine.ts index 0e6bc3c0..195d464c 100644 --- a/packages/backend/src/connectors/engines/rest.engine.ts +++ b/packages/backend/src/connectors/engines/rest.engine.ts @@ -182,6 +182,7 @@ export class RestEngine { const raw = String(mappedQuery['__rawquery']); delete mappedQuery['__rawquery']; for (const [k, v] of new URLSearchParams(raw)) { + if (isUnsafeKey(k)) continue; mappedQuery[k] = v; } } @@ -658,6 +659,7 @@ export class RestEngine { ) { bodyParams = {}; for (const [k, v] of new URLSearchParams(axiosConfig.data)) { + if (isUnsafeKey(k)) continue; bodyParams[k] = v; } } @@ -759,6 +761,15 @@ export class RestEngine { } } +/** + * Keys that would reach an object's prototype chain instead of naming a + * parameter. The query and form strings parsed here come from tool arguments, + * so from the model; no API names a parameter like this. + */ +function isUnsafeKey(key: string): boolean { + return key === '__proto__' || key === 'constructor' || key === 'prototype'; +} + /** * axios's own percent-encoding for a query component, reproduced so that * swapping in a custom serializer changes ONLY the array shape and nothing diff --git a/packages/backend/src/connectors/graphql-builtins.spec.ts b/packages/backend/src/connectors/graphql-builtins.spec.ts index bdfa748a..844003a2 100644 --- a/packages/backend/src/connectors/graphql-builtins.spec.ts +++ b/packages/backend/src/connectors/graphql-builtins.spec.ts @@ -98,3 +98,36 @@ describe('buildGraphqlBuiltinTools', () => { }, ); }); + +describe('slugifyForPrefix without a backtracking regex', () => { + // The previous implementation, kept here as the reference the new one must + // match. Its trim regex was polynomial on long runs of underscores. + const reference = (name: string) => + name.toLowerCase().replace(/[^a-z0-9]+/g, '_').replace(/^_+|_+$/g, '') || 'graphql'; + + it('matches the previous output on awkward and random inputs', () => { + const fixed = ['', '___', '--a--', 'A__B', ' x ', 'Ärger Öl', 'İstanbul API', '9lives', '..', 'a/b\\c']; + const alphabet = 'aZ9_-. /\\Äéİߣ\t\n@'; + const random = Array.from({ length: 2000 }, (_, i) => { + let str = ''; + let seed = i * 2654435761; + const len = (i % 17) + 1; + for (let j = 0; j < len; j++) { + seed = (seed * 1103515245 + 12345) >>> 0; + str += alphabet[seed % alphabet.length]; + } + return str; + }); + for (const input of [...fixed, ...random]) { + expect(slugifyForPrefix(input)).toBe(reference(input)); + } + }); + + it('stays linear on the input that made the old regex crawl', () => { + const hostile = '_'.repeat(50_000) + '!'; + const started = Date.now(); + expect(slugifyForPrefix(hostile)).toBe('graphql'); + expect(Date.now() - started).toBeLessThan(200); + }); +}); + diff --git a/packages/backend/src/connectors/graphql-builtins.ts b/packages/backend/src/connectors/graphql-builtins.ts index 17f869be..73a80ad2 100644 --- a/packages/backend/src/connectors/graphql-builtins.ts +++ b/packages/backend/src/connectors/graphql-builtins.ts @@ -49,18 +49,34 @@ export interface GraphqlBuiltinOptions { * slugifyForPrefix("") → "graphql" (safe fallback) */ export function slugifyForPrefix(name: string): string { - const slug = name - .toLowerCase() - .replace(/[^a-z0-9]+/g, '_') - .replace(/^_+|_+$/g, ''); + // One pass instead of `.replace(/^_+|_+$/g, '')`, which backtracks + // polynomially on a long run of underscores (CodeQL js/polynomial-redos). + let slug = ''; + let pendingUnderscore = false; + for (const ch of name.toLowerCase()) { + if ((ch >= 'a' && ch <= 'z') || (ch >= '0' && ch <= '9')) { + if (pendingUnderscore && slug) slug += '_'; + slug += ch; + pendingUnderscore = false; + } else { + pendingUnderscore = true; + } + } return slug || 'graphql'; } +/** `url` without trailing slashes, without the backtracking `/\/+$/`. */ +function trimTrailingSlashes(url: string): string { + let end = url.length; + while (end > 0 && url[end - 1] === '/') end--; + return url.slice(0, end); +} + export function buildGraphqlBuiltinTools( opts: GraphqlBuiltinOptions, ): GraphqlBuiltinTool[] { const { prefix, displayName, baseUrl } = opts; - const schemaUrl = opts.schemaUrl || `${baseUrl.replace(/\/+$/, '')}/schema`; + const schemaUrl = opts.schemaUrl || `${trimTrailingSlashes(baseUrl)}/schema`; const variablesSchema = { type: 'object', diff --git a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts index 2d93423c..725ace4f 100644 --- a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts +++ b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts @@ -432,6 +432,9 @@ export class McpEndpointController { res.status(200); res.setHeader('Content-Type', 'text/event-stream'); res.setHeader('Cache-Control', 'no-cache'); + // The body echoes the request id; nosniff keeps any browser from + // reading this stream as anything but the event stream it is. + res.setHeader('X-Content-Type-Options', 'nosniff'); res.end(`event: message\ndata: ${JSON.stringify(payload)}\n\n`); } return true;