Skip to content

Commit 01fda08

Browse files
authored
fix(ai-chat)!: always require user approval for destructive operations (#1277)
* fix(ai-chat): sanitize tool schemas for Gemini function_declarations * fix(ai-chat): round-trip Gemini thoughtSignature on tool calls * fix(ai-chat)!: always require user approval for destructive operations
1 parent fce09ae commit 01fda08

13 files changed

Lines changed: 308 additions & 20 deletions

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2727
- Drop database now uses the native confirmation dialog instead of a custom sheet.
2828
- Add competitive tracking docs sourced from top TablePlus issues.
2929

30+
### Security
31+
32+
- AI Chat: destructive operations (DROP, TRUNCATE, ALTER...DROP) now always require user approval before executing. Silent safe-mode and "Always Allow" no longer bypass the confirmation for `confirm_destructive_operation`. Previously an AI could drop tables without the user seeing any prompt when the connection was in Silent mode.
33+
3034
### Fixed
3135

36+
- AI Chat: Gemini provider now sends tool schemas with `additionalProperties` stripped and optional fields rewritten from `type: [X, null]` to `type: X, nullable: true`, fixing 400 errors when sending a message with tools enabled.
37+
- AI Chat: Gemini provider now round-trips `thoughtSignature` on function calls, fixing the second-round 400 error after a tool runs.
3238
- MySQL/MariaDB: `BIT(N)` columns now display as decimal numbers (`0`, `1`, `255`) instead of raw bytes that showed up as control characters like `^A` in the data grid. (#1272)
3339
- ClickHouse, BigQuery, CloudflareD1, LibSQL, Etcd, and DynamoDB: long-running queries no longer fail at 30 seconds when Settings > Query timeout is set higher. The HTTP transport now uses the configured query timeout plus a 30-second grace, so the server's `max_execution_time` (or equivalent) fires before the client gives up. Setting "No limit" raises the transport ceiling to 1 hour. (#1267)
3440
- AI Chat: DeepSeek V4 thinking content (`reasoning_content`) is now captured during streaming and passed back in subsequent turns, fixing 400 errors when using deepseek-v4-pro or deepseek-v4-flash.

TablePro/Core/AI/Chat/ChatTransport.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ struct ChatToolSpec: Codable, Equatable, Sendable {
6969

7070
enum ChatStreamEvent: Sendable {
7171
case textDelta(String)
72-
case toolUseStart(id: String, name: String)
72+
case toolUseStart(id: String, name: String, providerMetadata: [String: String]? = nil)
7373
case toolUseDelta(id: String, inputJSONDelta: String)
7474
case toolUseEnd(id: String)
7575
case usage(AITokenUsage)

TablePro/Core/AI/Chat/ChatTurn.swift

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,12 +396,23 @@ struct ToolUseBlock: Codable, Equatable, Sendable {
396396
let name: String
397397
let input: JsonValue
398398
var approvalState: ToolApprovalState
399+
/// Opaque provider-specific data that must round-trip with the call (e.g., Gemini's
400+
/// `thoughtSignature` required when echoing a function call alongside its response).
401+
/// Keys are provider-defined; unknown providers ignore them.
402+
var providerMetadata: [String: String]?
399403

400-
init(id: String, name: String, input: JsonValue, approvalState: ToolApprovalState = .approved) {
404+
init(
405+
id: String,
406+
name: String,
407+
input: JsonValue,
408+
approvalState: ToolApprovalState = .approved,
409+
providerMetadata: [String: String]? = nil
410+
) {
401411
self.id = id
402412
self.name = name
403413
self.input = input
404414
self.approvalState = approvalState
415+
self.providerMetadata = providerMetadata
405416
}
406417

407418
init(from decoder: Decoder) throws {
@@ -410,6 +421,7 @@ struct ToolUseBlock: Codable, Equatable, Sendable {
410421
name = try container.decode(String.self, forKey: .name)
411422
input = try container.decode(JsonValue.self, forKey: .input)
412423
approvalState = try container.decodeIfPresent(ToolApprovalState.self, forKey: .approvalState) ?? .approved
424+
providerMetadata = try container.decodeIfPresent([String: String].self, forKey: .providerMetadata)
413425
}
414426

415427
func encode(to encoder: Encoder) throws {
@@ -418,10 +430,11 @@ struct ToolUseBlock: Codable, Equatable, Sendable {
418430
try container.encode(name, forKey: .name)
419431
try container.encode(input, forKey: .input)
420432
try container.encode(approvalState, forKey: .approvalState)
433+
try container.encodeIfPresent(providerMetadata, forKey: .providerMetadata)
421434
}
422435

423436
private enum CodingKeys: String, CodingKey {
424-
case id, name, input, approvalState
437+
case id, name, input, approvalState, providerMetadata
425438
}
426439
}
427440

TablePro/Core/AI/GeminiProvider.swift

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ final class GeminiProvider: ChatTransport {
184184
"name": tool.name,
185185
"description": tool.description
186186
]
187-
entry["parameters"] = try tool.inputSchema.jsonObject()
187+
let sanitized = Self.sanitizeSchemaForGemini(tool.inputSchema)
188+
entry["parameters"] = try sanitized.jsonObject()
188189
return entry
189190
}
190191
body["tools"] = [["functionDeclarations": declarations]]
@@ -219,12 +220,16 @@ final class GeminiProvider: ChatTransport {
219220
continue
220221
case .toolUse(let useBlock):
221222
let argsObject = (try? useBlock.input.jsonObject()) ?? [String: Any]()
222-
parts.append([
223+
var partEntry: [String: Any] = [
223224
"functionCall": [
224225
"name": useBlock.name,
225226
"args": argsObject
226227
]
227-
])
228+
]
229+
if let signature = useBlock.providerMetadata?["thoughtSignature"], !signature.isEmpty {
230+
partEntry["thoughtSignature"] = signature
231+
}
232+
parts.append(partEntry)
228233
case .toolResult(let resultBlock):
229234
let toolName = resolveToolName(
230235
forToolUseId: resultBlock.toolUseId,
@@ -295,7 +300,11 @@ final class GeminiProvider: ChatTransport {
295300
let id = idGenerator()
296301
let argsObject = functionCall["args"] ?? [String: Any]()
297302
let argsString = encodeArgsToJSONString(argsObject)
298-
events.append(.toolUseStart(id: id, name: name))
303+
var metadata: [String: String]?
304+
if let signature = part["thoughtSignature"] as? String, !signature.isEmpty {
305+
metadata = ["thoughtSignature": signature]
306+
}
307+
events.append(.toolUseStart(id: id, name: name, providerMetadata: metadata))
299308
events.append(.toolUseDelta(id: id, inputJSONDelta: argsString))
300309
events.append(.toolUseEnd(id: id))
301310
}
@@ -325,6 +334,57 @@ final class GeminiProvider: ChatTransport {
325334
return "{}"
326335
}
327336
}
337+
338+
// MARK: - Schema Sanitization
339+
340+
/// Strip JSON Schema features Gemini's `function_declarations` API rejects.
341+
/// Removes `additionalProperties` keys at any depth. Rewrites optional fields
342+
/// expressed as `type: ["X", "null"]` to `type: "X"` plus `nullable: true`,
343+
/// which is the form Gemini accepts.
344+
static func sanitizeSchemaForGemini(_ schema: JsonValue) -> JsonValue {
345+
switch schema {
346+
case .object(let fields):
347+
var rewritten: [String: JsonValue] = [:]
348+
var nullable = false
349+
350+
for (key, value) in fields {
351+
if key == "additionalProperties" {
352+
continue
353+
}
354+
if key == "type", case .array(let members) = value {
355+
let nonNull = members.compactMap { member -> String? in
356+
if case .string(let typeName) = member, typeName != "null" {
357+
return typeName
358+
}
359+
if case .string("null") = member {
360+
nullable = true
361+
return nil
362+
}
363+
return nil
364+
}
365+
if nonNull.count == 1, let primary = nonNull.first {
366+
rewritten["type"] = .string(primary)
367+
} else {
368+
rewritten["type"] = .array(nonNull.map(JsonValue.string))
369+
}
370+
continue
371+
}
372+
rewritten[key] = sanitizeSchemaForGemini(value)
373+
}
374+
375+
if nullable {
376+
rewritten["nullable"] = .bool(true)
377+
}
378+
379+
return .object(rewritten)
380+
381+
case .array(let items):
382+
return .array(items.map(sanitizeSchemaForGemini))
383+
384+
default:
385+
return schema
386+
}
387+
}
328388
}
329389

330390
/// Mutable state carried across `GeminiProvider.parseChunk` calls.

TablePro/Resources/Localizable.xcstrings

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16648,6 +16648,9 @@
1664816648
},
1664916649
"Drop Database…" : {
1665016650

16651+
},
16652+
"Drop Failed" : {
16653+
1665116654
},
1665216655
"Drop Foreign Table" : {
1665316656

@@ -37858,6 +37861,9 @@
3785837861
}
3785937862
}
3786037863
}
37864+
},
37865+
"Remove “%@”?" : {
37866+
3786137867
},
3786237868
"Remove %@?" : {
3786337869
"localizations" : {
@@ -47038,6 +47044,9 @@
4703847044
},
4703947045
"The previous code wasn't accepted. Wait for your authenticator to refresh, then enter the new code." : {
4704047046

47047+
},
47048+
"The provider configuration and stored API key will be deleted." : {
47049+
4704147050
},
4704247051
"The selected backup file is not readable." : {
4704347052

TablePro/ViewModels/AIChatViewModel+Streaming.swift

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ extension AIChatViewModel {
2020
let toolUseOrder: [String]
2121
let toolUseNames: [String: String]
2222
let toolUseInputs: [String: String]
23+
let toolUseMetadata: [String: [String: String]]
2324
let cancelled: Bool
2425
}
2526

@@ -144,7 +145,8 @@ extension AIChatViewModel {
144145
let assembled = Self.assembleToolUseBlocks(
145146
order: round.toolUseOrder,
146147
names: round.toolUseNames,
147-
inputs: round.toolUseInputs
148+
inputs: round.toolUseInputs,
149+
metadata: round.toolUseMetadata
148150
)
149151
let context = await MainActor.run {
150152
ChatToolContext(
@@ -244,6 +246,7 @@ extension AIChatViewModel {
244246
var toolUseOrder: [String] = []
245247
var toolUseNames: [String: String] = [:]
246248
var toolUseInputs: [String: String] = [:]
249+
var toolUseMetadata: [String: [String: String]] = [:]
247250
var reasoningIDMap: [String: UUID] = [:]
248251
let flushInterval: ContinuousClock.Duration = .milliseconds(150)
249252
var lastFlushTime: ContinuousClock.Instant = .now
@@ -255,7 +258,7 @@ extension AIChatViewModel {
255258
pendingContent += token
256259
case .usage(let usage):
257260
pendingUsage = usage
258-
case .toolUseStart(let id, let name):
261+
case .toolUseStart(let id, let name, let providerMetadata):
259262
if !pendingContent.isEmpty {
260263
await self.flushPending(content: pendingContent, usage: pendingUsage, into: assistantID)
261264
pendingContent = ""
@@ -270,6 +273,9 @@ extension AIChatViewModel {
270273
toolUseInputs[id] = ""
271274
}
272275
toolUseNames[id] = name
276+
if let providerMetadata, !providerMetadata.isEmpty {
277+
toolUseMetadata[id] = providerMetadata
278+
}
273279
case .toolUseDelta(let id, let inputJSONDelta):
274280
toolUseInputs[id, default: ""] += inputJSONDelta
275281
case .toolUseEnd:
@@ -309,6 +315,7 @@ extension AIChatViewModel {
309315
toolUseOrder: toolUseOrder,
310316
toolUseNames: toolUseNames,
311317
toolUseInputs: toolUseInputs,
318+
toolUseMetadata: toolUseMetadata,
312319
cancelled: Task.isCancelled
313320
)
314321
}
@@ -465,7 +472,8 @@ extension AIChatViewModel {
465472
nonisolated static func assembleToolUseBlocks(
466473
order: [String],
467474
names: [String: String],
468-
inputs: [String: String]
475+
inputs: [String: String],
476+
metadata: [String: [String: String]] = [:]
469477
) -> [ToolUseBlock] {
470478
order.compactMap { id -> ToolUseBlock? in
471479
guard let name = names[id] else { return nil }
@@ -479,7 +487,12 @@ extension AIChatViewModel {
479487
} else {
480488
inputValue = .object([:])
481489
}
482-
return ToolUseBlock(id: id, name: name, input: inputValue)
490+
return ToolUseBlock(
491+
id: id,
492+
name: name,
493+
input: inputValue,
494+
providerMetadata: metadata[id]
495+
)
483496
}
484497
}
485498

TablePro/ViewModels/AIChatViewModel+ToolApproval.swift

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,13 @@ extension AIChatViewModel {
3131
guard let self else { return assembledBlocks }
3232
let initial = assembledBlocks.map { block -> ToolUseBlock in
3333
let state = self.computeInitialApprovalState(for: block.name)
34-
return ToolUseBlock(id: block.id, name: block.name, input: block.input, approvalState: state)
34+
return ToolUseBlock(
35+
id: block.id,
36+
name: block.name,
37+
input: block.input,
38+
approvalState: state,
39+
providerMetadata: block.providerMetadata
40+
)
3541
}
3642
self.appendPendingToolUseBlocks(initial, assistantID: assistantID)
3743
return initial
@@ -60,17 +66,37 @@ extension AIChatViewModel {
6066
self?.updateApprovalState(blockID: block.id, newState: finalState, assistantID: assistantID)
6167
}
6268
resolved.append(ToolUseBlock(
63-
id: block.id, name: block.name, input: block.input, approvalState: finalState
69+
id: block.id,
70+
name: block.name,
71+
input: block.input,
72+
approvalState: finalState,
73+
providerMetadata: block.providerMetadata
6474
))
6575
}
6676
return resolved
6777
}
6878

6979
@MainActor
7080
func computeInitialApprovalState(for toolName: String) -> ToolApprovalState {
71-
if !ChatToolRegistry.shared.requiresApproval(toolName: toolName) {
81+
let tool = ChatToolRegistry.shared.tool(named: toolName)
82+
let toolMode = tool?.mode
83+
84+
if toolMode == .readOnly {
7285
return .approved
7386
}
87+
88+
// Destructive operations (`.agentOnly`) always require user approval.
89+
// Safe-mode level and "Always Allow" cannot bypass them — the AI must not
90+
// be able to drop tables, truncate, or alter-drop without an explicit click.
91+
if toolMode == .agentOnly {
92+
if let connection, connection.safeModeLevel.blocksAllWrites {
93+
return .denied(reason: String(
94+
localized: "Connection is read-only. Destructive operations are not permitted."
95+
))
96+
}
97+
return .pending
98+
}
99+
74100
if let connection, connection.aiAlwaysAllowedTools.contains(toolName) {
75101
return .approved
76102
}
@@ -109,6 +135,11 @@ extension AIChatViewModel {
109135

110136
@MainActor
111137
func persistAlwaysAllowed(toolName: String) {
138+
// Refuse to persist Always Allow for destructive operations.
139+
// Each DROP/TRUNCATE/ALTER...DROP must be confirmed individually.
140+
if ChatToolRegistry.shared.tool(named: toolName)?.mode == .agentOnly {
141+
return
142+
}
112143
guard var current = connection else { return }
113144
guard !current.aiAlwaysAllowedTools.contains(toolName) else { return }
114145
current.aiAlwaysAllowedTools.insert(toolName)
@@ -142,7 +173,11 @@ extension AIChatViewModel {
142173
) async {
143174
let initialState = computeInitialApprovalState(for: block.name)
144175
let pendingBlock = ToolUseBlock(
145-
id: block.id, name: block.name, input: block.input, approvalState: initialState
176+
id: block.id,
177+
name: block.name,
178+
input: block.input,
179+
approvalState: initialState,
180+
providerMetadata: block.providerMetadata
146181
)
147182
appendPendingToolUseBlocks([pendingBlock], assistantID: assistantID)
148183

TableProTests/Core/AI/AnthropicProviderParserTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ struct AnthropicProviderParserTests {
4141
]
4242
], state: &state)
4343
#expect(events.count == 1)
44-
if case .toolUseStart(let id, let name) = events.first {
44+
if case .toolUseStart(let id, let name, _) = events.first {
4545
#expect(id == "toolu_abc")
4646
#expect(name == "list_tables")
4747
} else {

0 commit comments

Comments
 (0)