diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f5c8cdf4..4d8ece3b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- MCP query safety: three-tier classification with server-side confirmation for write and destructive queries + ## [0.34.0] - 2026-04-22 ### Added diff --git a/TablePro/Core/MCP/MCPAuthGuard.swift b/TablePro/Core/MCP/MCPAuthGuard.swift index dd1cea7f4..049c9d2b1 100644 --- a/TablePro/Core/MCP/MCPAuthGuard.swift +++ b/TablePro/Core/MCP/MCPAuthGuard.swift @@ -65,14 +65,22 @@ actor MCPAuthGuard { safeModeLevel: SafeModeLevel ) async throws { let isWrite = QueryClassifier.isWriteQuery(sql, databaseType: databaseType) + let needsDialog = safeModeLevel != .silent && (isWrite || safeModeLevel == .alertFull || safeModeLevel == .safeModeFull) + + var window: NSWindow? + if needsDialog { + window = await MainActor.run { + NSApp.activate(ignoringOtherApps: true) + return NSApp.keyWindow ?? NSApp.mainWindow + } + } - // SafeModeGuard.checkPermission is @MainActor async; Swift hops automatically let permission = await SafeModeGuard.checkPermission( level: safeModeLevel, isWriteOperation: isWrite, sql: sql, operationDescription: String(localized: "MCP query execution"), - window: nil, + window: window, databaseType: databaseType ) diff --git a/TablePro/Core/MCP/MCPConnectionBridge.swift b/TablePro/Core/MCP/MCPConnectionBridge.swift index a5903d5b9..d0dfbfb69 100644 --- a/TablePro/Core/MCP/MCPConnectionBridge.swift +++ b/TablePro/Core/MCP/MCPConnectionBridge.swift @@ -169,19 +169,24 @@ actor MCPConnectionBridge { maxRows: Int, timeoutSeconds: Int ) async throws -> JSONValue { - let (driver, _) = try await resolveDriver(connectionId) + let (driver, databaseType) = try await resolveDriver(connectionId) + let isWrite = QueryClassifier.isWriteQuery(query, databaseType: databaseType) + let hasReturning = query.range(of: #"\bRETURNING\b"#, options: [.regularExpression, .caseInsensitive]) != nil + let shouldUseFetchRows = !isWrite || hasReturning let effectiveLimit = maxRows + 1 let startTime = CFAbsoluteTimeGetCurrent() - // trackOperation is @MainActor; Swift hops automatically. - // The driver.fetchRows call inside runs on the cooperative pool. let result: QueryResult = try await DatabaseManager.shared.trackOperation( sessionId: connectionId ) { try await withThrowingTaskGroup(of: QueryResult.self) { group in group.addTask { - try await driver.fetchRows(query: query, offset: 0, limit: effectiveLimit) + if shouldUseFetchRows { + try await driver.fetchRows(query: query, offset: 0, limit: effectiveLimit) + } else { + try await driver.execute(query: query) + } } group.addTask { try await Task.sleep(for: .seconds(timeoutSeconds)) diff --git a/TablePro/Core/MCP/MCPRouter.swift b/TablePro/Core/MCP/MCPRouter.swift index 5cdbd3978..2b692031d 100644 --- a/TablePro/Core/MCP/MCPRouter.swift +++ b/TablePro/Core/MCP/MCPRouter.swift @@ -646,7 +646,8 @@ extension MCPRouter { [ MCPToolDefinition( name: "execute_query", - description: "Execute a SQL or NoSQL query on a connected database", + description: "Execute a SQL query. All queries are subject to the connection's safe mode policy. " + + "DROP/TRUNCATE/ALTER...DROP must use the confirm_destructive_operation tool.", inputSchema: .object([ "type": "object", "properties": .object([ @@ -680,7 +681,7 @@ extension MCPRouter { ), MCPToolDefinition( name: "export_data", - description: "Export query results or table data to CSV, JSON, SQL, or XLSX", + description: "Export query results or table data to CSV, JSON, or SQL", inputSchema: .object([ "type": "object", "properties": .object([ @@ -713,6 +714,32 @@ extension MCPRouter { ]), "required": .array([.string("connection_id"), .string("format")]) ]) + ), + MCPToolDefinition( + name: "confirm_destructive_operation", + description: "Execute a destructive DDL query (DROP, TRUNCATE, ALTER...DROP) after explicit confirmation.", + inputSchema: .object([ + "type": "object", + "properties": .object([ + "connection_id": .object([ + "type": "string", + "description": "UUID of the active connection" + ]), + "query": .object([ + "type": "string", + "description": "The destructive query to execute" + ]), + "confirmation_phrase": .object([ + "type": "string", + "description": "Must be exactly: I understand this is irreversible" + ]) + ]), + "required": .array([ + .string("connection_id"), + .string("query"), + .string("confirmation_phrase") + ]) + ]) ) ] } diff --git a/TablePro/Core/MCP/MCPToolHandler.swift b/TablePro/Core/MCP/MCPToolHandler.swift index 9f2972eb1..1c2972222 100644 --- a/TablePro/Core/MCP/MCPToolHandler.swift +++ b/TablePro/Core/MCP/MCPToolHandler.swift @@ -43,6 +43,8 @@ final class MCPToolHandler: Sendable { return try await handleGetTableDDL(arguments, sessionId: sessionId) case "export_data": return try await handleExportData(arguments, sessionId: sessionId) + case "confirm_destructive_operation": + return try await handleConfirmDestructiveOperation(arguments, sessionId: sessionId) case "switch_database": return try await handleSwitchDatabase(arguments, sessionId: sessionId) case "switch_schema": @@ -94,6 +96,10 @@ final class MCPToolHandler: Sendable { throw MCPError.invalidParams("Query exceeds 100KB limit") } + guard !QueryClassifier.isMultiStatement(query) else { + throw MCPError.invalidParams("Multi-statement queries are not supported. Send one statement at a time.") + } + try await authGuard.checkConnectionAccess(connectionId: connectionId, sessionId: sessionId) let (databaseType, safeModeLevel, databaseName) = try await resolveConnectionMeta(connectionId) @@ -105,46 +111,87 @@ final class MCPToolHandler: Sendable { _ = try await bridge.switchSchema(connectionId: connectionId, schema: schema) } - try await authGuard.checkQueryPermission( - sql: query, - connectionId: connectionId, - databaseType: databaseType, - safeModeLevel: safeModeLevel - ) + let tier = QueryClassifier.classifyTier(query, databaseType: databaseType) - let startTime = Date() - let result: JSONValue - do { - result = try await bridge.executeQuery( - connectionId: connectionId, - query: query, - maxRows: maxRows, - timeoutSeconds: timeoutSeconds + switch tier { + case .destructive: + throw MCPError.forbidden( + "Destructive queries (DROP, TRUNCATE, ALTER...DROP) cannot be executed via execute_query. " + + "Use the confirm_destructive_operation tool instead." ) - let elapsed = Date().timeIntervalSince(startTime) - await authGuard.logQuery( + + case .write, .safe: + try await authGuard.checkQueryPermission( sql: query, connectionId: connectionId, - databaseName: databaseName, - executionTime: elapsed, - rowCount: result["row_count"]?.intValue ?? 0, - wasSuccessful: true, - errorMessage: nil + databaseType: databaseType, + safeModeLevel: safeModeLevel ) - } catch { - let elapsed = Date().timeIntervalSince(startTime) - await authGuard.logQuery( - sql: query, - connectionId: connectionId, - databaseName: databaseName, - executionTime: elapsed, - rowCount: 0, - wasSuccessful: false, - errorMessage: error.localizedDescription + } + + let result = try await executeAndLog( + query: query, + connectionId: connectionId, + databaseName: databaseName, + maxRows: maxRows, + timeoutSeconds: timeoutSeconds + ) + + return MCPToolResult(content: [.text(encodeJSON(result))], isError: nil) + } + + // MARK: - Destructive Confirmation + + private func handleConfirmDestructiveOperation( + _ args: JSONValue?, + sessionId: String + ) async throws -> MCPToolResult { + let connectionId = try requireUUID(args, key: "connection_id") + let query = try requireString(args, key: "query") + let confirmationPhrase = try requireString(args, key: "confirmation_phrase") + + guard confirmationPhrase == "I understand this is irreversible" else { + throw MCPError.invalidParams( + "confirmation_phrase must be exactly: I understand this is irreversible" + ) + } + + guard !QueryClassifier.isMultiStatement(query) else { + throw MCPError.invalidParams( + "Multi-statement queries are not supported. Send one statement at a time." + ) + } + + try await authGuard.checkConnectionAccess(connectionId: connectionId, sessionId: sessionId) + + let (databaseType, safeModeLevel, databaseName) = try await resolveConnectionMeta(connectionId) + + let tier = QueryClassifier.classifyTier(query, databaseType: databaseType) + guard tier == .destructive else { + throw MCPError.invalidParams( + "This tool only accepts destructive queries (DROP, TRUNCATE, ALTER...DROP). " + + "Use execute_query for other queries." ) - throw error } + try await authGuard.checkQueryPermission( + sql: query, + connectionId: connectionId, + databaseType: databaseType, + safeModeLevel: safeModeLevel + ) + + let mcpSettings = await MainActor.run { AppSettingsManager.shared.mcp } + let timeoutSeconds = mcpSettings.queryTimeoutSeconds + + let result = try await executeAndLog( + query: query, + connectionId: connectionId, + databaseName: databaseName, + maxRows: 0, + timeoutSeconds: timeoutSeconds + ) + return MCPToolResult(content: [.text(encodeJSON(result))], isError: nil) } @@ -344,6 +391,49 @@ final class MCPToolHandler: Sendable { return MCPToolResult(content: [.text(encodeJSON(result))], isError: nil) } + // MARK: - Execute and Log + + private func executeAndLog( + query: String, + connectionId: UUID, + databaseName: String, + maxRows: Int, + timeoutSeconds: Int + ) async throws -> JSONValue { + let startTime = Date() + do { + let result = try await bridge.executeQuery( + connectionId: connectionId, + query: query, + maxRows: maxRows, + timeoutSeconds: timeoutSeconds + ) + let elapsed = Date().timeIntervalSince(startTime) + await authGuard.logQuery( + sql: query, + connectionId: connectionId, + databaseName: databaseName, + executionTime: elapsed, + rowCount: result["row_count"]?.intValue ?? 0, + wasSuccessful: true, + errorMessage: nil + ) + return result + } catch { + let elapsed = Date().timeIntervalSince(startTime) + await authGuard.logQuery( + sql: query, + connectionId: connectionId, + databaseName: databaseName, + executionTime: elapsed, + rowCount: 0, + wasSuccessful: false, + errorMessage: error.localizedDescription + ) + throw error + } + } + // MARK: - Parameter Helpers private func requireUUID(_ args: JSONValue?, key: String) throws -> UUID { diff --git a/TablePro/Core/Utilities/SQL/QueryClassifier.swift b/TablePro/Core/Utilities/SQL/QueryClassifier.swift index e93428cea..78a4f1425 100644 --- a/TablePro/Core/Utilities/SQL/QueryClassifier.swift +++ b/TablePro/Core/Utilities/SQL/QueryClassifier.swift @@ -5,6 +5,12 @@ import Foundation +enum QueryTier { + case safe + case write + case destructive +} + enum QueryClassifier { private static let writeQueryPrefixes: [String] = [ "INSERT ", "UPDATE ", "DELETE ", "REPLACE ", @@ -40,7 +46,18 @@ enum QueryClassifier { } let uppercased = trimmed.uppercased() - return writeQueryPrefixes.contains { uppercased.hasPrefix($0) } + if writeQueryPrefixes.contains(where: { uppercased.hasPrefix($0) }) { + return true + } + + if uppercased.hasPrefix("WITH ") { + let dmlKeywords = ["INSERT ", "UPDATE ", "DELETE ", "MERGE "] + for keyword in dmlKeywords where uppercased.contains(keyword) { + return true + } + } + + return false } static func isDangerousQuery(_ sql: String, databaseType: DatabaseType) -> Bool { @@ -73,4 +90,44 @@ enum QueryClassifier { return false } + + static func classifyTier(_ sql: String, databaseType: DatabaseType) -> QueryTier { + let trimmed = sql.trimmingCharacters(in: .whitespacesAndNewlines) + let uppercased = trimmed.uppercased() + + if databaseType == .redis { + let firstToken = trimmed.prefix(while: { !$0.isWhitespace }).uppercased() + if firstToken == "FLUSHDB" || firstToken == "FLUSHALL" { + return .destructive + } + } else { + if uppercased.hasPrefix("DROP ") || uppercased.hasPrefix("TRUNCATE ") { + return .destructive + } + if uppercased.hasPrefix("ALTER ") && uppercased.range(of: " DROP ", options: .literal) != nil { + return .destructive + } + + if uppercased.hasPrefix("WITH ") { + let destructiveKeywords = ["DROP ", "TRUNCATE "] + for keyword in destructiveKeywords where uppercased.contains(keyword) { + return .destructive + } + let writeKeywords = ["INSERT ", "UPDATE ", "DELETE ", "MERGE "] + for keyword in writeKeywords where uppercased.contains(keyword) { + return .write + } + } + } + + if isWriteQuery(sql, databaseType: databaseType) { + return .write + } + + return .safe + } + + static func isMultiStatement(_ sql: String) -> Bool { + SQLStatementScanner.allStatements(in: sql).count > 1 + } } diff --git a/docs/features/mcp.mdx b/docs/features/mcp.mdx index 9ab5b4507..c9842da81 100644 --- a/docs/features/mcp.mdx +++ b/docs/features/mcp.mdx @@ -9,7 +9,7 @@ TablePro includes a built-in [Model Context Protocol](https://modelcontextprotoc ## Enabling the Server -Open **Settings > Terminal** and toggle **Enable MCP Server**. The server starts on port `23508` by default. A green status indicator confirms it is running. +Open **Settings > MCP** and toggle **Enable MCP Server**. The server starts on port `23508` by default. A green status indicator confirms it is running. +All safety controls run server-side in TablePro. AI clients cannot bypass them. + + +### Network + +- Binds to `127.0.0.1` only. Never exposed to the network. +- Origin header validated to prevent DNS rebinding attacks. Only `localhost`, `127.0.0.1`, `[::1]` allowed. + +### Connection access + +Each connection has an AI Policy: -- `tablepro://connections` - all saved connections with metadata -- `tablepro://connections/{id}/schema` - full schema for a connected database -- `tablepro://connections/{id}/history` - recent query history (supports `?limit=`, `?search=`, `?date_filter=`) +| Policy | Behavior | +|--------|----------| +| Always Allow | No prompt | +| Ask Each Time (default) | Approval dialog on first access per session (30s timeout) | +| Never | Blocked | -## Security +Set per connection or change the default in **Settings > AI**. Approvals are cleared when the session ends, times out, or the app restarts. -### AI Connection Policy +### Query tiers -Each connection has an **AI Policy** that controls MCP access: +| Tier | Examples | What happens | +|------|----------|-------------| +| Safe | SELECT, SHOW, EXPLAIN | Runs immediately | +| Write | UPDATE, DELETE, INSERT | Uses the connection's [Safe Mode](/features/safe-mode) | +| Destructive | DROP, TRUNCATE, ALTER...DROP | Blocked in `execute_query`. Must use `confirm_destructive_operation` | -- **Always Allow** - MCP tools can use this connection without prompts -- **Ask Each Time** - TablePro shows an approval dialog when an MCP client first accesses the connection in a session -- **Never** - MCP access is blocked entirely +CTE-prefixed writes like `WITH ... DELETE FROM` are detected and classified correctly. -Set the policy in the connection's settings. The default policy for new connections is configurable in **Settings > AI**. +#### Destructive queries + +`execute_query` rejects destructive queries with an error. Use `confirm_destructive_operation` instead: + +```json +{ + "connection_id": "uuid", + "query": "DROP TABLE temp_data", + "confirmation_phrase": "I understand this is irreversible" +} +``` + +The confirmation phrase must match exactly. The connection's Safe Mode still applies on top of this. ### Safe Mode -[Safe Mode](/features/safe-mode) applies to MCP queries the same way it applies to manual queries. If a connection is set to Alert or higher, write queries from MCP clients require the same confirmation or Touch ID authentication. +MCP queries follow the same [Safe Mode](/features/safe-mode) as manual queries: + +- **Silent** - no confirmation +- **Read-Only** - all writes blocked +- **Alert** - confirmation dialog for writes +- **Alert (Full)** - confirmation dialog for all queries including reads +- **Safe Mode** - confirmation + Touch ID for writes +- **Safe Mode (Full)** - confirmation + Touch ID for all queries + +If denied, the query returns an error to the MCP client. Nothing is executed. + +### Limits + +- Row limit: 500 default, 10,000 max (per query via `max_rows`) +- Export row limit: 50,000 default, 100,000 max +- Query timeout: 30s default, 300s max (per query via `timeout_seconds`) +- Single statements only (multi-statement blocked) +- Query size: 100 KB max +- HTTP body: 10 MB max +- Max concurrent sessions: 10 +- Session idle timeout: 5 minutes + +### Logging + +MCP queries are logged to [Query History](/features/query-history) (can be disabled in Settings). + +### Data access + +MCP clients **can** access: connection metadata, schemas, query results, query history. -Read-Only connections block all write queries from MCP clients. +MCP clients **cannot** access: passwords, SSH keys, license data, app settings, files on your Mac. ## Troubleshooting -**Port conflict**: If the server fails to start, another process may be using port 23508. Change the port in **Settings > Terminal**. +**Port conflict**: If the server fails to start, another process may be using port 23508. Change the port in **Settings > MCP**. -**"Server not fully initialized"**: Restart the MCP server from **Settings > Terminal**. If the issue persists, quit and relaunch TablePro. +**"Server not fully initialized"**: Restart the MCP server from **Settings > MCP**. If the issue persists, quit and relaunch TablePro. **App must be running**: The MCP server only runs while TablePro is open. AI tools cannot connect if the app is quit or the server is disabled. **Connection refused**: Verify the server is running (green indicator in Settings). Check that your AI tool's MCP config URL matches the port shown in Settings. -**Query blocked by Safe Mode**: The MCP server respects the same Safe Mode levels as manual queries. Check the connection's safe mode setting if queries are being rejected. +**Query blocked by Safe Mode**: Check the connection's safe mode setting if queries are being rejected.