From 8cc2334138a22cad54413e1a54d4915702e5f8da Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 19 Jun 2026 09:45:34 +0200 Subject: [PATCH 1/5] fix(connectors): unblock read-only CTE SQL, surface upstream errors, harden Odds/WhatsApp adapters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes found while analysing 5 days of production connector/MCP usage. database.engine: the SQL guard rejected any query not starting with SELECT, so legitimate read-only `WITH … SELECT` CTEs (the heaviest user's BI queries) failed with "Only SELECT queries are allowed". Now accept a leading WITH, block data-modifying CTEs (`WITH x AS (INSERT …)`), reject stacked statements, and strip string literals/comments first so keyword checks aren't fooled (`action = 'DELETE'`). The previous keyword loop was dead code (`&& !startsWith('SELECT')`). Adds tests. dynamic-mcp-tools: the persisted invocation error stored only the bare axios message ("Request failed with status code 400"), hiding the real cause. Fold the upstream response body into the stored error so failures are diagnosable from the audit log (e.g. weclapp "unknown property", Odds "Missing eventId", Graph "code 190"). odds-api: new catalog adapter. The previous custom connectors failed because /odds requires both eventId and a valid bookmaker (not "all") and there is no "soccer_world_cup" slug. Adds list_sports/bookmakers/leagues/events + get_event_odds with the required params and the correct discovery workflow; apiKey injected from env. whatsapp-business: list_phone_numbers/list_message_templates required the model to pass businessAccountId, which it left empty (// path → 401). Inject the WABA id from {{WHATSAPP_BUSINESS_ACCOUNT_ID}} instead, dropping the param. --- packages/backend/src/adapters/catalog.ts | 2 + .../backend/src/adapters/intl/odds-api.json | 130 ++++++++++++++++++ .../src/adapters/intl/whatsapp-business.json | 22 +-- .../engines/database.engine.spec.ts | 73 ++++++++++ .../src/connectors/engines/database.engine.ts | 58 +++++--- .../src/mcp-server/dynamic-mcp-tools.ts | 34 ++++- 6 files changed, 279 insertions(+), 40 deletions(-) create mode 100644 packages/backend/src/adapters/intl/odds-api.json diff --git a/packages/backend/src/adapters/catalog.ts b/packages/backend/src/adapters/catalog.ts index 970a05ff..4b3ba2f0 100644 --- a/packages/backend/src/adapters/catalog.ts +++ b/packages/backend/src/adapters/catalog.ts @@ -120,6 +120,7 @@ import * as newsapi from './intl/newsapi.json'; import * as nimble from './intl/nimble.json'; import * as nominatim from './intl/nominatim.json'; import * as nutshellCrm from './intl/nutshell-crm.json'; +import * as oddsApi from './intl/odds-api.json'; import * as omnisend from './intl/omnisend.json'; import * as opentable from './intl/opentable.json'; import * as openweather from './intl/openweather.json'; @@ -391,6 +392,7 @@ const RAW_ADAPTERS: AdapterDefinition[] = [ nimble as unknown as AdapterDefinition, nominatim as unknown as AdapterDefinition, nutshellCrm as unknown as AdapterDefinition, + oddsApi as unknown as AdapterDefinition, omnisend as unknown as AdapterDefinition, opentable as unknown as AdapterDefinition, openweather as unknown as AdapterDefinition, diff --git a/packages/backend/src/adapters/intl/odds-api.json b/packages/backend/src/adapters/intl/odds-api.json new file mode 100644 index 00000000..1a80206a --- /dev/null +++ b/packages/backend/src/adapters/intl/odds-api.json @@ -0,0 +1,130 @@ +{ + "slug": "odds-api", + "name": "Odds API", + "description": "Live and pre-match sports betting odds (football, basketball, tennis, baseball, American football, ice hockey, MMA, boxing and more) from dozens of bookmakers via odds-api.io. Look up sports, leagues and events, then read odds for a specific event from your selected bookmakers.", + "instructions": "This connector wraps the **odds-api.io v3** REST API. Get a free API key at https://odds-api.io and paste it as `ODDS_API_KEY` — it is injected automatically into every call, you never pass it as a parameter.\n\n**Mandatory workflow** — `odds_get_event_odds` needs a concrete `eventId` and at least one valid `bookmakers` name; you cannot call it blind:\n1. `odds_list_sports` → the valid sport **slug** (e.g. `football`, `basketball`, `tennis`). NOTE: there is no `soccer_world_cup` slug — football competitions are plain `football`, narrow them with the league slug from step 2.\n2. `odds_list_leagues` (pass the sport slug) → the league `slug` (e.g. `england-premier-league`).\n3. `odds_list_events` (pass the sport slug, optionally the league) → each event's numeric **`id`**. This is the `eventId` you pass to odds.\n4. `odds_list_bookmakers` → the exact bookmaker **names** you may query (e.g. `Bet365`, `Betano PE`). Pass them as a comma-separated `bookmakers` value. `all` is NOT accepted, and free plans are limited to a small number of selected bookmakers (the API returns a 403 listing your allowed bookmakers if you exceed it).\n5. `odds_get_event_odds` with `eventId` + `bookmakers` (+ optional `markets`).\n\n**Common errors** (the API returns a JSON `error` field):\n- `Missing eventId` / `Missing bookmakers` → required parameter omitted.\n- ` is not a valid bookmaker` → use a name from `odds_list_bookmakers`, not `all`.\n- `Access denied. You're allowed max N bookmakers` → your plan limits selected bookmakers; pick from the listed allowed set or upgrade.\n\n**Times** are ISO-8601 UTC (e.g. `2026-06-18T09:00:00Z`). Event `status` is one of `notstarted` / `inprogress` / `settled`.", + "region": "intl", + "category": "sports", + "icon": "odds-api", + "docsUrl": "https://docs.odds-api.io", + "requiredEnvVars": ["ODDS_API_KEY"], + "connector": { + "name": "Odds API", + "type": "REST", + "baseUrl": "https://api.odds-api.io/v3", + "authType": "NONE" + }, + "tools": [ + { + "name": "odds_list_sports", + "description": "List the sports supported by odds-api.io. Returns each sport's display `name` and `slug`. Use the slug for odds_list_leagues / odds_list_events (e.g. `football`, `basketball`, `tennis`, `baseball`, `american-football`, `ice-hockey`, `mixed-martial-arts`, `boxing`). There is no `soccer_world_cup` slug — use `football` plus a league slug.", + "parameters": { + "type": "object", + "properties": {} + }, + "endpointMapping": { + "method": "GET", + "path": "/sports", + "queryParams": { + "apiKey": "{{ODDS_API_KEY}}" + } + } + }, + { + "name": "odds_list_bookmakers", + "description": "List the bookmakers known to odds-api.io. Returns each bookmaker's `name` and whether it is `active`. The exact `name` value (e.g. `Bet365`, `1xbet`) is what you pass in the `bookmakers` parameter of odds_get_event_odds. Free plans only allow a limited number of selected bookmakers.", + "parameters": { + "type": "object", + "properties": {} + }, + "endpointMapping": { + "method": "GET", + "path": "/bookmakers", + "queryParams": { + "apiKey": "{{ODDS_API_KEY}}" + } + } + }, + { + "name": "odds_list_leagues", + "description": "List the leagues/competitions available for a sport. Returns each league's `name`, `slug` and `eventsCount`. Use the `slug` to narrow odds_list_events.", + "parameters": { + "type": "object", + "properties": { + "sport": { + "type": "string", + "description": "Sport slug from odds_list_sports, e.g. `football`." + } + }, + "required": ["sport"] + }, + "endpointMapping": { + "method": "GET", + "path": "/leagues", + "queryParams": { + "apiKey": "{{ODDS_API_KEY}}", + "sport": "$sport" + } + } + }, + { + "name": "odds_list_events", + "description": "List events (matches/fixtures) for a sport, optionally filtered by league. Each event has a numeric `id` (the `eventId` for odds_get_event_odds), `home`/`away` team names, `date` (ISO-8601 UTC), `league` and `status` (`notstarted` / `inprogress` / `settled`).", + "parameters": { + "type": "object", + "properties": { + "sport": { + "type": "string", + "description": "Sport slug from odds_list_sports, e.g. `football`." + }, + "league": { + "type": "string", + "description": "Optional league slug from odds_list_leagues to narrow the list." + } + }, + "required": ["sport"] + }, + "endpointMapping": { + "method": "GET", + "path": "/events", + "queryParams": { + "apiKey": "{{ODDS_API_KEY}}", + "sport": "$sport", + "league": "$league" + } + } + }, + { + "name": "odds_get_event_odds", + "description": "Get betting odds for a single event from one or more bookmakers. BOTH `eventId` and `bookmakers` are required — get `eventId` from odds_list_events and valid bookmaker names from odds_list_bookmakers (the literal `all` is rejected, and your plan caps how many you may select). Returns the event plus a `bookmakers` map of odds by market.", + "parameters": { + "type": "object", + "properties": { + "eventId": { + "type": "string", + "description": "Numeric event id from odds_list_events. Required." + }, + "bookmakers": { + "type": "string", + "description": "Comma-separated bookmaker names from odds_list_bookmakers, e.g. `Bet365` or `Bet365,Betano PE`. Required. Do NOT pass `all`." + }, + "markets": { + "type": "string", + "description": "Optional comma-separated markets to return, e.g. `h2h` (match winner), `totals`, `spreads`. Omit to return all available markets." + } + }, + "required": ["eventId", "bookmakers"] + }, + "endpointMapping": { + "method": "GET", + "path": "/odds", + "queryParams": { + "apiKey": "{{ODDS_API_KEY}}", + "eventId": "$eventId", + "bookmakers": "$bookmakers", + "markets": "$markets" + } + } + } + ] +} diff --git a/packages/backend/src/adapters/intl/whatsapp-business.json b/packages/backend/src/adapters/intl/whatsapp-business.json index 5b3ca99b..fc8ab549 100644 --- a/packages/backend/src/adapters/intl/whatsapp-business.json +++ b/packages/backend/src/adapters/intl/whatsapp-business.json @@ -20,24 +20,19 @@ "tools": [ { "name": "whatsapp_list_phone_numbers", - "description": "List all phone numbers registered on the WhatsApp Business Account. Returns each number's `id` (the phoneNumberId you pass to send tools), `display_phone_number`, `verified_name` and `quality_rating`. Call this once after setup to discover which phoneNumberId to use.", + "description": "List all phone numbers registered on the WhatsApp Business Account. Returns each number's `id` (the phoneNumberId you pass to send tools), `display_phone_number`, `verified_name` and `quality_rating`. Call this once after setup to discover which phoneNumberId to use. The WABA ID is taken automatically from the WHATSAPP_BUSINESS_ACCOUNT_ID configured on this connector — you do NOT pass it.", "parameters": { "type": "object", "properties": { - "businessAccountId": { - "type": "string", - "description": "WhatsApp Business Account ID (WABA ID). Use the value of WHATSAPP_BUSINESS_ACCOUNT_ID configured on this connector — pass it explicitly here on each call (path parameter)." - }, "limit": { "type": "integer", "description": "Max phone numbers to return per page (default 20, max 100)." } - }, - "required": ["businessAccountId"] + } }, "endpointMapping": { "method": "GET", - "path": "/{businessAccountId}/phone_numbers", + "path": "/{{WHATSAPP_BUSINESS_ACCOUNT_ID}}/phone_numbers", "queryParams": { "limit": "$limit" } @@ -45,14 +40,10 @@ }, { "name": "whatsapp_list_message_templates", - "description": "List the message templates approved on the WhatsApp Business Account. Returns each template's `id`, `name`, `language`, `status` (APPROVED / PENDING / REJECTED) and `components` (header/body/footer/buttons structure). Use this to discover which templates you can send via whatsapp_send_template_message.", + "description": "List the message templates approved on the WhatsApp Business Account. Returns each template's `id`, `name`, `language`, `status` (APPROVED / PENDING / REJECTED) and `components` (header/body/footer/buttons structure). Use this to discover which templates you can send via whatsapp_send_template_message. The WABA ID is taken automatically from the WHATSAPP_BUSINESS_ACCOUNT_ID configured on this connector — you do NOT pass it.", "parameters": { "type": "object", "properties": { - "businessAccountId": { - "type": "string", - "description": "WhatsApp Business Account ID (WABA ID)." - }, "limit": { "type": "integer", "description": "Max templates per page (default 20, max 100)." @@ -61,12 +52,11 @@ "type": "string", "description": "Comma-separated list of fields to return. Default: 'name,language,status,category,components,id'." } - }, - "required": ["businessAccountId"] + } }, "endpointMapping": { "method": "GET", - "path": "/{businessAccountId}/message_templates", + "path": "/{{WHATSAPP_BUSINESS_ACCOUNT_ID}}/message_templates", "queryParams": { "limit": "$limit", "fields": "$fields" diff --git a/packages/backend/src/connectors/engines/database.engine.spec.ts b/packages/backend/src/connectors/engines/database.engine.spec.ts index e6142d2e..ebfa85ab 100644 --- a/packages/backend/src/connectors/engines/database.engine.spec.ts +++ b/packages/backend/src/connectors/engines/database.engine.spec.ts @@ -112,6 +112,79 @@ describe('DatabaseEngine', () => { ), ).rejects.toThrow('Only SELECT queries are allowed'); }); + + it('should allow read-only WITH … SELECT CTE queries', async () => { + mockQuery.mockResolvedValue({ rows: [] }); + const cte = + 'WITH q AS (SELECT id, amount FROM sales WHERE amount > 0) ' + + 'SELECT COUNT(*) AS n, SUM(amount) AS total FROM q'; + await engine.execute( + { baseUrl: 'postgres://host/db', authType: 'NONE' }, + { method: 'query', path: cte }, + {}, + ); + expect(mockQuery).toHaveBeenCalledWith(cte); + }); + + it('should allow a multi-CTE read-only query', async () => { + mockQuery.mockResolvedValue({ rows: [] }); + const cte = + 'WITH a AS (SELECT 1 AS x), b AS (SELECT 2 AS y) ' + + 'SELECT * FROM a JOIN b ON true'; + await engine.execute( + { baseUrl: 'postgres://host/db', authType: 'NONE' }, + { method: 'query', path: cte }, + {}, + ); + expect(mockQuery).toHaveBeenCalledWith(cte); + }); + + it('should block data-modifying CTEs (WITH … AS (INSERT …))', async () => { + await expect( + engine.execute( + { baseUrl: 'postgres://host/db', authType: 'NONE' }, + { + method: 'query', + path: 'WITH x AS (INSERT INTO users VALUES (1) RETURNING id) SELECT * FROM x', + }, + {}, + ), + ).rejects.toThrow('Blocked SQL keyword in CTE: INSERT'); + }); + + it('should block data-modifying CTEs (WITH … AS (DELETE …))', async () => { + await expect( + engine.execute( + { baseUrl: 'postgres://host/db', authType: 'NONE' }, + { + method: 'query', + path: 'WITH d AS (DELETE FROM users RETURNING id) SELECT * FROM d', + }, + {}, + ), + ).rejects.toThrow('Blocked SQL keyword in CTE: DELETE'); + }); + + it('should block stacked statements after a SELECT', async () => { + await expect( + engine.execute( + { baseUrl: 'postgres://host/db', authType: 'NONE' }, + { method: 'query', path: 'SELECT 1; DROP TABLE users' }, + {}, + ), + ).rejects.toThrow('single SQL statement'); + }); + + it('should not be fooled by keyword-looking string literals', async () => { + mockQuery.mockResolvedValue({ rows: [] }); + const q = "SELECT * FROM audit WHERE action = 'DELETE' AND note = 'a;b'"; + await engine.execute( + { baseUrl: 'postgres://host/db', authType: 'NONE' }, + { method: 'query', path: q }, + {}, + ); + expect(mockQuery).toHaveBeenCalledWith(q); + }); }); describe('SQL parameter binding (prepared statements)', () => { diff --git a/packages/backend/src/connectors/engines/database.engine.ts b/packages/backend/src/connectors/engines/database.engine.ts index 6e5aa9ab..8617d557 100644 --- a/packages/backend/src/connectors/engines/database.engine.ts +++ b/packages/backend/src/connectors/engines/database.engine.ts @@ -606,33 +606,49 @@ export class DatabaseEngine { return { rows, totalRows: rows.length }; } + /** + * Remove string literals and comments so the read-only lexical checks below + * can't be fooled (e.g. `WHERE note = 'a;b'`) nor trip over harmless keyword + * substrings inside literals (e.g. `WHERE action = 'DELETE'`). + */ + private stripLiteralsAndComments(sql: string): string { + return sql + .replace(/\/\*[\s\S]*?\*\//g, ' ') // block comments + .replace(/--[^\n]*/g, ' ') // line comments + .replace(/'(?:[^']|'')*'/g, "''"); // single-quoted strings → empty literal + } + private validateQuery(sql: string): void { - const normalized = sql.trim().toUpperCase(); + const normalized = this.stripLiteralsAndComments(sql).trim().toUpperCase(); - if (!normalized.startsWith('SELECT')) { + // Read-only entry points: a plain SELECT, or a WITH (CTE) that ultimately + // selects. Common Table Expressions are a legitimate read-only construct + // (`WITH q AS (SELECT …) SELECT … FROM q`) and were previously rejected. + if (!normalized.startsWith('SELECT') && !normalized.startsWith('WITH')) { throw new Error( - 'Only SELECT queries are allowed. INSERT, UPDATE, DELETE, DROP, and other write operations are blocked.', + 'Only SELECT queries are allowed (a leading WITH … SELECT CTE is also accepted). INSERT, UPDATE, DELETE, DROP, and other write operations are blocked.', ); } - const blocked = [ - 'INSERT', - 'UPDATE', - 'DELETE', - 'DROP', - 'TRUNCATE', - 'ALTER', - 'CREATE', - 'EXEC', - 'EXECUTE', - 'GRANT', - 'REVOKE', - ]; - for (const keyword of blocked) { - const regex = new RegExp(`\\b${keyword}\\b`, 'i'); - if (regex.test(sql) && !normalized.startsWith('SELECT')) { - throw new Error(`Blocked SQL keyword: ${keyword}`); - } + // Reject stacked statements (e.g. "SELECT 1; DROP TABLE x"). A single + // trailing semicolon is tolerated; anything after it is not. + if (normalized.replace(/;\s*$/, '').includes(';')) { + throw new Error( + 'Only a single SQL statement is allowed; stacked statements are blocked.', + ); + } + + // Postgres allows data-modifying CTEs — `WITH x AS (INSERT … RETURNING …) + // SELECT …`. Those write despite the leading WITH, so block any write + // keyword that opens a CTE body. Matching `(\s*` keeps this from + // flagging ordinary identifiers (`created_at`) or read-only subqueries. + const dataModifyingCte = + /\(\s*(INSERT|UPDATE|DELETE|MERGE|DROP|TRUNCATE|ALTER|CREATE|GRANT|REVOKE)\b/; + const cteMatch = normalized.match(dataModifyingCte); + if (cteMatch) { + throw new Error( + `Blocked SQL keyword in CTE: ${cteMatch[1]}. Only read-only queries are allowed.`, + ); } } diff --git a/packages/backend/src/mcp-server/dynamic-mcp-tools.ts b/packages/backend/src/mcp-server/dynamic-mcp-tools.ts index 10a2997f..d24a6f80 100644 --- a/packages/backend/src/mcp-server/dynamic-mcp-tools.ts +++ b/packages/backend/src/mcp-server/dynamic-mcp-tools.ts @@ -278,9 +278,7 @@ export class DynamicMcpTools { input: params, status: 'ERROR', durationMs, - error: errorDetail.status - ? `${errorDetail.status} ${errorDetail.statusText || ''}: ${errorDetail.error}` - : String(errorDetail.error), + error: this.formatStoredError(errorDetail), clientInfo: context ? JSON.stringify({ authMethod: context.authMethod, apiKeyName: context.apiKeyName, @@ -347,6 +345,36 @@ export class DynamicMcpTools { return result; } + /** + * Build the string persisted in the tool_invocations.error column. Beyond the + * HTTP status line, it folds in the upstream API response body — that body + * carries the actual cause (e.g. weclapp "unknown property: customerName", + * Odds API "Missing eventId", Graph API "code 190 invalid token") which the + * bare axios message ("Request failed with status code 400") hides. Keeping it + * in the audit log makes failures diagnosable without re-running the call. + */ + private formatStoredError(detail: Record): string { + const parts: string[] = []; + const head = detail.status + ? `${detail.status} ${detail.statusText || ''}`.trim() + : ''; + if (head) parts.push(head); + if (detail.error) parts.push(String(detail.error)); + + if (detail.responseBody !== undefined && detail.responseBody !== null) { + let body = + typeof detail.responseBody === 'string' + ? detail.responseBody + : JSON.stringify(detail.responseBody); + if (body.length > 600) body = `${body.slice(0, 600)}…`; + if (body && body !== '{}' && body !== '""' && body !== '[]') { + parts.push(`body=${body}`); + } + } + + return parts.length > 0 ? parts.join(': ') : String(detail.error ?? 'Unknown error'); + } + /** * Extract rich error details from different error types so that the AI * client receives enough context to understand the failure and retry. From 618559e40514e9a7e6e7ba059cdbf626a68ec1bf Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 19 Jun 2026 09:54:08 +0200 Subject: [PATCH 2/5] test(whatsapp): update WABA-path assertions for env-injected business account id --- .../adapters/intl/whatsapp-business.live.spec.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/adapters/intl/whatsapp-business.live.spec.ts b/packages/backend/src/adapters/intl/whatsapp-business.live.spec.ts index c8f2a76e..a378f557 100644 --- a/packages/backend/src/adapters/intl/whatsapp-business.live.spec.ts +++ b/packages/backend/src/adapters/intl/whatsapp-business.live.spec.ts @@ -10,7 +10,7 @@ import { LoginTokenService } from '../../connectors/engines/login-token.service' * Graph API version, that send-tool bodies carry the mandatory * `messaging_product: "whatsapp"` discriminator, and that paths follow the * Meta Cloud API routing convention (`/{phoneNumberId}/messages`, - * `/{businessAccountId}/message_templates`, etc.). Catches the most common + * `/{{WHATSAPP_BUSINESS_ACCOUNT_ID}}/message_templates`, etc.). Catches the most common * failure modes: pinning back to a deprecated API version, omitting the * messaging_product field, or accidentally pointing at the old On-Premises * API base path. @@ -133,14 +133,22 @@ describe('whatsapp-business adapter — static spec conformance', () => { expect(audioObj['caption']).toBeUndefined(); }); - it('WABA-scoped tools target /{businessAccountId}/...', () => { + it('WABA-scoped tools inject the WABA id from {{WHATSAPP_BUSINESS_ACCOUNT_ID}}', () => { const listPhones = a.tools.find((t) => t.name === 'whatsapp_list_phone_numbers')!; - expect(listPhones.endpointMapping.path).toBe('/{businessAccountId}/phone_numbers'); + expect(listPhones.endpointMapping.path).toBe( + '/{{WHATSAPP_BUSINESS_ACCOUNT_ID}}/phone_numbers', + ); expect(listPhones.endpointMapping.method).toBe('GET'); + // The model must NOT be asked for the WABA id — it comes from env. + expect(listPhones.parameters.properties).not.toHaveProperty('businessAccountId'); + expect(listPhones.parameters.required ?? []).not.toContain('businessAccountId'); const listTemplates = a.tools.find((t) => t.name === 'whatsapp_list_message_templates')!; - expect(listTemplates.endpointMapping.path).toBe('/{businessAccountId}/message_templates'); + expect(listTemplates.endpointMapping.path).toBe( + '/{{WHATSAPP_BUSINESS_ACCOUNT_ID}}/message_templates', + ); expect(listTemplates.endpointMapping.method).toBe('GET'); + expect(listTemplates.parameters.properties).not.toHaveProperty('businessAccountId'); }); it('business profile tools target /{phoneNumberId}/whatsapp_business_profile', () => { From 059ee90adda66d7728bb2a07aa0d8a5136ab029f Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 19 Jun 2026 09:56:23 +0200 Subject: [PATCH 3/5] test(whatsapp): add parameters field to local Tool type for assertions --- .../backend/src/adapters/intl/whatsapp-business.live.spec.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/backend/src/adapters/intl/whatsapp-business.live.spec.ts b/packages/backend/src/adapters/intl/whatsapp-business.live.spec.ts index a378f557..5e9960c9 100644 --- a/packages/backend/src/adapters/intl/whatsapp-business.live.spec.ts +++ b/packages/backend/src/adapters/intl/whatsapp-business.live.spec.ts @@ -27,6 +27,10 @@ import { LoginTokenService } from '../../connectors/engines/login-token.service' interface Tool { name: string; + parameters: { + properties?: Record; + required?: string[]; + }; endpointMapping: { method: string; path: string; From bbc50babade9da7741686ba748578cf2d003dbfe Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 19 Jun 2026 10:01:23 +0200 Subject: [PATCH 4/5] fix(sql-guard): strip comments/literals via linear scan to avoid polynomial-regex ReDoS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged the block-comment regex as polynomial on untrusted query input (/*a/*a/*…). Replace the three regexes with a single O(n) character scan and add a ReDoS-resistance test. --- .../engines/database.engine.spec.ts | 24 +++++++++ .../src/connectors/engines/database.engine.ts | 54 +++++++++++++++++-- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/connectors/engines/database.engine.spec.ts b/packages/backend/src/connectors/engines/database.engine.spec.ts index ebfa85ab..f5288bb7 100644 --- a/packages/backend/src/connectors/engines/database.engine.spec.ts +++ b/packages/backend/src/connectors/engines/database.engine.spec.ts @@ -185,6 +185,30 @@ describe('DatabaseEngine', () => { ); expect(mockQuery).toHaveBeenCalledWith(q); }); + + it('should ignore keywords inside comments', async () => { + mockQuery.mockResolvedValue({ rows: [] }); + const q = 'SELECT id /* DROP TABLE x */ FROM t -- ; DELETE FROM t\n'; + await engine.execute( + { baseUrl: 'postgres://host/db', authType: 'NONE' }, + { method: 'query', path: q }, + {}, + ); + expect(mockQuery).toHaveBeenCalledWith(q); + }); + + it('rejects an unterminated block comment quickly (no ReDoS)', async () => { + const hostile = `/*${'a/*'.repeat(50000)}`; + const start = Date.now(); + await expect( + engine.execute( + { baseUrl: 'postgres://host/db', authType: 'NONE' }, + { method: 'query', path: hostile }, + {}, + ), + ).rejects.toThrow('Only SELECT queries are allowed'); + expect(Date.now() - start).toBeLessThan(1000); + }); }); describe('SQL parameter binding (prepared statements)', () => { diff --git a/packages/backend/src/connectors/engines/database.engine.ts b/packages/backend/src/connectors/engines/database.engine.ts index 8617d557..9ae2ef15 100644 --- a/packages/backend/src/connectors/engines/database.engine.ts +++ b/packages/backend/src/connectors/engines/database.engine.ts @@ -610,12 +610,58 @@ export class DatabaseEngine { * Remove string literals and comments so the read-only lexical checks below * can't be fooled (e.g. `WHERE note = 'a;b'`) nor trip over harmless keyword * substrings inside literals (e.g. `WHERE action = 'DELETE'`). + * + * Done as a single linear character scan rather than regex: a regex for + * unterminated block comments backtracks in polynomial time on hostile input + * (`/*a/*a/*…`), and the query string is fully untrusted. */ private stripLiteralsAndComments(sql: string): string { - return sql - .replace(/\/\*[\s\S]*?\*\//g, ' ') // block comments - .replace(/--[^\n]*/g, ' ') // line comments - .replace(/'(?:[^']|'')*'/g, "''"); // single-quoted strings → empty literal + let out = ''; + let i = 0; + const n = sql.length; + while (i < n) { + const c = sql[i]; + const next = sql[i + 1]; + + // Line comment: -- … end of line + if (c === '-' && next === '-') { + i += 2; + while (i < n && sql[i] !== '\n') i++; + out += ' '; + continue; + } + + // Block comment: /* … */ (an unterminated one runs to end of input) + if (c === '/' && next === '*') { + i += 2; + while (i < n && !(sql[i] === '*' && sql[i + 1] === '/')) i++; + i += 2; + out += ' '; + continue; + } + + // Single-quoted string with '' escape → collapse to an empty literal + if (c === "'") { + i++; + while (i < n) { + if (sql[i] === "'") { + if (sql[i + 1] === "'") { + i += 2; + continue; + } + i++; + break; + } + i++; + } + out += "''"; + continue; + } + + out += c; + i++; + } + return out; } private validateQuery(sql: string): void { From a444f411b3e45d83addeaa0029a8221209514acd Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 19 Jun 2026 10:05:25 +0200 Subject: [PATCH 5/5] fix(sql-guard): cap query length to bound the scan loop (CodeQL loop-bound-injection) --- .../src/connectors/engines/database.engine.spec.ts | 13 ++++++++++++- .../src/connectors/engines/database.engine.ts | 11 ++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/connectors/engines/database.engine.spec.ts b/packages/backend/src/connectors/engines/database.engine.spec.ts index f5288bb7..a24ed8c7 100644 --- a/packages/backend/src/connectors/engines/database.engine.spec.ts +++ b/packages/backend/src/connectors/engines/database.engine.spec.ts @@ -198,7 +198,7 @@ describe('DatabaseEngine', () => { }); it('rejects an unterminated block comment quickly (no ReDoS)', async () => { - const hostile = `/*${'a/*'.repeat(50000)}`; + const hostile = `/*${'a/*'.repeat(30000)}`; // ~90k chars, under MAX_QUERY_LENGTH const start = Date.now(); await expect( engine.execute( @@ -209,6 +209,17 @@ describe('DatabaseEngine', () => { ).rejects.toThrow('Only SELECT queries are allowed'); expect(Date.now() - start).toBeLessThan(1000); }); + + it('rejects an oversized query', async () => { + const huge = `SELECT '${'a'.repeat(100_001)}'`; + await expect( + engine.execute( + { baseUrl: 'postgres://host/db', authType: 'NONE' }, + { method: 'query', path: huge }, + {}, + ), + ).rejects.toThrow('Query too long'); + }); }); describe('SQL parameter binding (prepared statements)', () => { diff --git a/packages/backend/src/connectors/engines/database.engine.ts b/packages/backend/src/connectors/engines/database.engine.ts index 9ae2ef15..5e19522a 100644 --- a/packages/backend/src/connectors/engines/database.engine.ts +++ b/packages/backend/src/connectors/engines/database.engine.ts @@ -10,6 +10,10 @@ import Database from 'better-sqlite3'; export class DatabaseEngine { private readonly logger = new Logger(DatabaseEngine.name); private readonly MAX_ROWS = 1000; + // Upper bound on an incoming SQL string. The query is untrusted; capping its + // length bounds the read-only lexical scan below (no unbounded work on a + // hostile, oversized payload). 100k chars is far beyond any real analytics query. + private readonly MAX_QUERY_LENGTH = 100_000; async execute( config: { @@ -616,9 +620,14 @@ export class DatabaseEngine { * (`/*a/*a/*…`), and the query string is fully untrusted. */ private stripLiteralsAndComments(sql: string): string { + if (sql.length > this.MAX_QUERY_LENGTH) { + throw new Error( + `Query too long (${sql.length} chars; max ${this.MAX_QUERY_LENGTH}).`, + ); + } let out = ''; let i = 0; - const n = sql.length; + const n = Math.min(sql.length, this.MAX_QUERY_LENGTH); while (i < n) { const c = sql[i]; const next = sql[i + 1];