fix(connectors): unblock read-only CTE SQL, surface upstream errors, harden Odds/WhatsApp adapters#345
Merged
Conversation
…harden Odds/WhatsApp adapters
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.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…nomial-regex ReDoS 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes uncovered while analysing 5 days of production connector/MCP usage (10 errors / 87 calls). Every root cause was verified by calling the upstream APIs directly with the real user credentials.
Changes
database.engine.ts— read-only SQL guard (real bug)The guard rejected any query not starting with
SELECT, so legitimate read-onlyWITH … SELECTCTEs — the heaviest user's BI queries — failed with "Only SELECT queries are allowed". Now:WITH,WITH x AS (INSERT … RETURNING …)),SELECT 1; DROP …),WHERE action = 'DELETE').The previous keyword loop was dead code (
&& !startsWith('SELECT')). Adds 8 tests.dynamic-mcp-tools.ts— error observabilityThe persisted
tool_invocations.errorstored only the bare axios message (Request failed with status code 400), hiding the cause. The upstream response body is now folded into the stored error, so failures are diagnosable from the audit log (e.g. weclappunknown property: customerName, OddsMissing eventId, Graphcode 190 invalid token). The full detail returned to the MCP client is unchanged.adapters/intl/odds-api.json— new catalog adapterThe previous user-built custom connectors failed because
/oddsrequires botheventIdand a valid bookmaker name (notall), and there is nosoccer_world_cupslug. Addslist_sports/list_bookmakers/list_leagues/list_events/get_event_oddswith the required params, the correct discovery workflow, and the apiKey injected from{{ODDS_API_KEY}}.adapters/intl/whatsapp-business.jsonlist_phone_numbers/list_message_templatesrequired the model to passbusinessAccountId, which it left empty (//phone_numbers→ 401). The WABA id is now injected from{{WHATSAPP_BUSINESS_ACCOUNT_ID}}and the param dropped.Not fixable in code (user-config; called out for support follow-up)
code 190) — user must regenerate a permanent System User token and setWHATSAPP_BUSINESS_ACCOUNT_ID.run-failedcause visible).customerNameNOTE is present); errors predated it.Testing
No
node_modulesin this workspace, so the build/jest suite must run in CI. The SQL guard logic was validated standalone against 16 cases (including the two real production CTE queries and thecreated_at/'DELETE'false-positive guards),catalog.tsregenerated (180 adapters), and both edited JSON files validated.