Skip to content

Test every MCP and Agent mode write path against every connection policy #3663

Closed

Description

@datlechin

Problem

A write from outside the editor passes several guards: the token's scopes, the connection's external access, its AI policy, and its Safe Mode level. Each guard has tests of its own. No test runs every write path against every combination, so a new write tool, or a change to one guard, can let a write through and every test stays green. This area has had real bugs: #2101 (a leading SQL comment got a write past read-only) and #3293 (MongoDB Safe Mode reads were enforced only in the classifier).

What the tests cover today:

  • MCPAuthPolicy refuses a write on a connection that is read only for external clients. The tests check that for execute_query and stop_server_session only (apple/TableProTests/Core/MCP/MCPAuthPolicyTests.swift:92 and :120). The policy's own write lists name only those two tools and confirm_destructive_operation (apple/TablePro/Core/MCP/MCPAuthPolicy.swift:9-11).
  • insert_rows, drop_database_object, create_database, run_maintenance and transaction_control are guarded only because each one passes a SQL preview to MCPStatementGate.authorize (apple/TablePro/Core/MCP/Protocol/Tools/MCPStatementGate.swift:6). Their tests check arguments and annotations (apple/TableProTests/Core/MCP/Protocol/Tools/MCPDataToolTests.swift:209), not a refusal on a read-only connection or under Safe Mode.
  • The registry test checks that a read-only scope never lists a write tool (MCPToolRegistryGuardTests.swift:133), and ExternalStatementGateTests.swift checks the statement classifier.
  • Agent mode: tests cover which tools each chat mode exposes (apple/TableProTests/Core/AI/ChatToolRegistryModeTests.swift) and the AI connection policy (AIConnectionAccessGateTests.swift). No test runs a chat write against each Safe Mode level.
  • Outside MCP servers: features/mcp-outside-servers.mdx says every call to an outside tool waits for Run or Reject, and Always Allow does not cover it. The tests check the allowlist (apple/TableProTests/Core/MCP/Outside/MCPRemoteToolScopeTests.swift) but not that rule.

Proposed solution

One table-driven suite that runs each write path against each policy and checks the outcome:

  • Paths: every tool in MCPToolRegistry.allTools whose requiredScopes contains tools:write, every Agent mode chat tool that writes, and a tool from an outside MCP server.
  • Policies: external access (blocked, read only, read and write), AI policy (Always Allow, Ask Each Time, Never), all six Safe Mode levels, and token scopes (read, write, admin).
  • One expected-outcome table: refused, asks the user, or runs. Use AlwaysAllowGate and AlwaysDenyGate for the prompt, and assert that the driver never receives the statement in a refused cell.
  • The suite fails when a write tool exists that the table does not list, so a new tool cannot skip the matrix.

Done when the suite covers every cell above and runs in the Unit plan.

Alternatives considered

Keep adding one test per tool, as today. That leaves the next new tool uncovered until someone remembers.

Platform

macOS

Related database type

N/A / General

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions