Skip to content

fix(mcp): let SQL Server scripts past the execution gate and keep GO lines out of single statements - #3124

Merged
datlechin merged 1 commit into
mainfrom
fix/mcp-sql-server-script-gate
Sep 24, 2026
Merged

datlechin merged 1 commit into
mainfrom
fix/mcp-sql-server-script-gate

Conversation

@datlechin

Copy link
Copy Markdown
Member

Follow-up to #3123, which was merged before review finished. Two defects a reviewer found in it, both confirmed.

1. SQL Server scripts were still refused on MCP, AppleScript and the assistant

#3123 let a SQL Server script past the first gate, ExternalStatementGate.classify, by passing allowsMultiStatement. Every caller then runs the second gate, DefaultExecutionGate. It counts statements with QueryClassifier.isMultiStatement and denies the text with "Multiple statements are not permitted for this client" when the caller lacks .mayRunMultiStatement. That check runs before any Safe Mode level, so it applied at Silent too. No caller passed the capability:

  • MCP execute_query: MCPStatementGate built [.mayWrite] plus the destructive and consent flags.
  • AppleScript run query: ScriptQueryRunner passed [.mayWrite, .mayRunDestructive].
  • Assistant execute_query: context.writeCapabilities.

So only a script that scans as one statement ran. The #3078 script (DECLARE ...; and four SELECT ...;) and SELECT 1 AS a / GO / SELECT 2 AS b were refused. The #3123 tests called the bridge and the executor directly and never reached the second gate.

Change. The fact that lets a script past classify now also grants .mayRunMultiStatement, so the two gates cannot disagree:

  • MCPStatementGate.authorize inserts it when allowsMultiStatement is true, next to the existing allowsDestructive insert.
  • ExternalStatementGate.capabilities(_:takingScriptsOn:) adds it where acceptsScripts(on:) holds.
  • ScriptQueryRunner.capabilities(on:) builds the script's set from it, and run feeds that one set to both classify and authorizeExecution.
  • ExecuteQueryChatTool does the same with context.writeCapabilities.

Nothing else loosens. DefaultExecutionGate still tiers the whole text by its worst statement, and a caller that does not take scripts still gets no capability.

2. A leading GO line still reached the server, and GO n still ran once

runStatement sends SQLStatementScanner.executableText, which kept everything from the start of the text. #3123 routed execute_query around it and left the other tools that send one statement: confirm_destructive_operation (MCP and assistant), explain_query and export_data. A GO line alone is a segment with no statement, so both gates counted GO\nDROP TABLE t as one statement and passed it.

Measured against Azure SQL Edge 15 with a db-lib probe, one dbcmd per text as the driver sends it:

Text sent Server answer Table afterwards
GO\nDROP TABLE dbo.stale Msg 2812 "Could not find stored procedure 'GO'.", dbsqlexec FAIL dropped (OBJECT_ID NULL)
DROP TABLE dbo.stale SUCCEED dropped

So confirm_destructive_operation dropped the table and then reported a failure.

Change.

  • executableText now starts where the first statement starts, as it already ended where the last one ends. A leading GO line, a leading Oracle / line or a stray leading ; no longer reaches the driver. Comments before the first statement stay: a probe over 12 inputs on SQL Server, PostgreSQL, MySQL and Oracle changed only those leading separators.
  • QueryClassifier.isMultiStatement also counts a statement that a GO n line runs more than once. Every gate for a caller that does not take scripts now refuses TRUNCATE TABLE t\nGO 2 before anyone is prompted, with "Send one statement at a time." Before, the count was dropped and the statement ran once. execute_query, run query and the assistant's execute_query on SQL Server still honour GO n. The editor carries .mayRunMultiStatement and is unaffected.

CHANGELOG: defect 1 completes the unreleased #3123 entry, so it gets no line of its own. Defect 2 was in 0.75.0 and gets a Fixed line. docs/external-api/mcp-tools.mdx now says a GO n statement is refused outside execute_query.

Tests

Each of these fails on main:

  • MCPStatementGateRefusalTests.sqlServerScriptClearsBothGates: the SQL Server batch variables are lost between statements #3078 script, a GO script and GO 3 through MCPStatementGate.authorize and the real shared DefaultExecutionGate at Silent. On main the second gate throws forbidden.
  • MCPScriptResultTests.executeQueryToolRunsTheScript and chatToolRunsTheScript: ExecuteQueryTool.perform and ExecuteQueryChatTool.execute end to end, on an injected SQL Server session with a fake batch driver, for a ; script and a GO script. Both gates run, and the payload has both result sets.
  • ScriptingPolicyTests.sqlServerScriptClearsTheExecutionGate: ScriptQueryRunner.capabilities(on:) through a real DefaultExecutionGate. SQL Server scripts pass, and SELECT 1; SELECT 2 on PostgreSQL is still denied. scriptsMayNotRunMultipleStatements is now scoped to engines that do not take scripts.
  • MCPScriptResultTests.leadingSeparatorIsNotSent: GO\nDROP TABLE dbo.stale on the statement path reaches the driver as DROP TABLE dbo.stale.
  • SQLBatchSeparatorTests.leadingSeparatorIsNotSent (package) and SQLStatementPLSQLSplittingTests.executableTextDropsALeadingSlash.
  • QueryClassifierMultiStatementTests.repeatedBatchIsMultiStatement and ExternalStatementGateTests.repeatedStatementIsRefused. A bare GO, GO 1 and a GO 3 that ends an empty batch still count as one statement.
  • ExternalStatementGateTests.scriptCapabilityFollowsTheEngine.

What each red case rests on: removing the .mayRunMultiStatement insert turns the MCP and AppleScript cases red, passing context.writeCapabilities again turns the assistant case red, starting executableText at offset 0 turns the separator cases red, and dropping the repeatCount clause turns the GO n cases red. I did not run that mutation pass locally. The session's permission check blocked it as weakening a security gate. The first gate test fails on main from source reading: the shared gate returns .denied and MCPAuthPolicy throws forbidden.

Verification

  • verify.sh build: PASS (after generate, which picked up a file from fix(plugins): run SQL Server imports as GO batches and end each statement of a SQL Server dump with GO #3122).
  • verify.sh test on the 6 edited suites: 97/97 PASS. Every argument of the parameterised cases ran, checked in the xcresult.
  • verify.sh test on 33 neighbouring suites that own the changed types (QueryClassifier*, ExecutionGateTests, ExternalStatementGateLexicalTests, DatabaseAccessBridge*, ExportDataTool*, ConfirmDestructiveOperationToolTests, MCPStatementConsentGuardTests, ExecuteQueryToolTests, the chat tool suites and others): 495/495 PASS.
  • swift test --filter TableProSQLGrammarTests: 86/86 PASS.
  • verify.sh lint on all 13 Swift files: 0 violations. verify.sh docs: PASS. check-ios-shared-isolation.py: OK.

Not covered: no live run of the MCP server or AppleScript against SQL Server. The sandboxed Debug app does not start the MCP server without a Settings change in the UI, and AppleScript needs the Automation consent prompt. The server's side of the leading-GO defect is covered by the probe in the table. No driver code changed, so scripts/check-mssql-batch-results.sh does not apply. No UI automation either: the flow needs a SQL Server connection, which the bundled Chinook SQLite sample cannot stand in for.

Found while fixing #3078 (#3105).

@mintlify

mintlify Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 24, 2026, 5:56 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

This branch was successfully deployed

1 active deployment
staging - docs — 7da3b3b8 Deployed Sep 24, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant