fix: harden SQL classification for the tool CLI and MCP server

HK
7h ago
+13270-384

Issue for this PR

Stacked on #1609 (base tool-cli-daemon). It holds the classifier hardening from later review rounds of #1609, kept out of that PR after it was approved. Most of these gaps also exist on main.

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

#1609 exposes the SQL classifier to outside agents (MCP server, altimate tool run): reads run by default, writes need --allow-write, and DROP DATABASE / DROP SCHEMA / TRUNCATE are refused even then. This PR fixes cases where the classifier read the SQL differently from the warehouse.

sql_explain: newly reachable through #1609; its tool code is unchanged from main.

  • It sent the caller's SQL to the warehouse unchecked. On Redshift and DuckDB the driver runs every statement in the text, so SELECT 1; DELETE FROM t ran the DELETE without --allow-write.
  • Input starting with ANALYZE became EXPLAIN ANALYZE, which executes the statement.
  • It now explains a single statement only, refuses EXPLAIN options in its input, and refuses blocked SQL.

Writes that were read as reads (on main too):

  • Snowflake ->> statement chains.
  • dbt Jinja anywhere in the SQL on the dbt route. {% do run_query(...) %} runs at compile time, even inside a comment.
  • Spark/Hive TRANSFORM ... USING.
  • Functions that write or call outside systems:
    • ClickHouse jdbc
    • Spark reflect and java_method
    • Databricks http_request and remote_query
    • DuckDB bigquery_execute, ducklake_*, postgres_attach and sqlite_attach
    • Snowflake GETNEXTVAL
  • Sequence advances (.NEXTVAL), including quoted or commented forms, in Oracle, Snowflake and MariaDB.
  • A quote inside a Spark or MySQL /*+ hint */, Oracle q'...' quotes, a Spark -- comment ending in a backslash, and a /* inside a comment where nesting is unknown. These now make the SQL uncertain, so it is refused rather than read.
  • ClickHouse $tag$...$tag$ heredocs are lexed as strings.

DROP DATABASE / DROP SCHEMA / TRUNCATE block (on main too). The block reads the SQL text, so it is best effort. docs/docs/usage/tool.md now says so, and recommends a database user without DROP/TRUNCATE for a hard guarantee. Within that:

  • A write's literals are read as SQL, with escapes decoded as each engine decodes them. This covers EXEC(...), sp_executesql, EXECUTE IMMEDIATE, PREPARE ... FROM, DO $$...$$, dblink_exec, and routine bodies created and called in one call.
  • TRUNCATE is found:
    • at the start of a statement, a block (BEGIN [NOT ATOMIC], REPEAT, ...) or a handler;
    • after EXPLAIN (...) and PREPARE p AS;
    • after THEN/ELSE, with quoted, dotted, db..t or IF EXISTS targets and NOWAIT/WAIT n.
  • DROP CATALOG, DROP NAMESPACE, DROP EXTERNAL SCHEMA and DROP PLUGGABLE DATABASE are blocked.

Regressions from #1609 fixed here. Each of these refused a valid read:

  • A column or alias named truncate, which was hard-blocked.
  • SELECT 1 AS throw on SQL Server.
  • A column named nextval.

Two consistency fixes:

  • data_diff reads an empty target_warehouse as the source warehouse in both layers.
  • The omitted-warehouse dbt dialect check follows the dbt adapter the daemon has already created.

Known limits (each refuses rather than lets through):

  • A write whose literal mentions DROP followed by DATABASE/SCHEMA/CATALOG/NAMESPACE, or the word TRUNCATE, is refused even with --allow-write. Example: INSERT INTO audit VALUES ('TRUNCATE').
  • Privilege and trigger names (GRANT TRUNCATE, BEFORE TRUNCATE) are refused.
  • PostgreSQL literals ending in a backslash are refused, since standard_conforming_strings may be off.
  • Oracle q'...' quotes are refused.
  • ClickHouse jdbc(...) is a write, so reads through it need --allow-write.

Out of scope, a follow-up: enforcing the DROP/TRUNCATE block in the database, through a restricted role or a read-only transaction where the driver supports one. That would cover statements built at run time, which no text check can see.

How did you verify your code works?

  • bun run typecheck is clean.
  • bun test test/altimate/tools test/altimate/tool-cli test/altimate/native test/altimate/sql-query-deadline.test.ts test/altimate/dbt-first-execution.test.ts: 1741 pass, 0 fail.
  • Each case above has a test:
    • classifier cases per dialect in sql-classify.test.ts;
    • runner tests for sql_explain with a fake connector, which receives nothing;
    • sql_execute tests for dbt Jinja (including a MongoDB default) in sql-comment-safety.test.ts.
  • Large crafted inputs stay linear, with timing tests.
  • The changes went through repeated independent reviews (codex and claude).
  • Manual end to end against a local DuckDB warehouse, on a copy of the database:
    • #1609's head: tool run sql_explain with SELECT 1; DELETE FROM orders, without --allow-write, returned a plan and deleted all 100 rows.
    • This branch (13c911cb29):
      • that input, and ANALYZE DELETE FROM orders, are refused with "Only a single statement can be explained";
      • a single-statement plan still works;
      • the table keeps its 100 rows.

Cost side effects

None. No model calls, queries or cloud resources are added; the changes only refuse more SQL before it runs.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR
  • I checked for cost side effects (model calls, queries or cloud resources that grow with user data) and described them, or wrote "none"

🤖 Generated with Claude Code

Summary by cubic

Hardens SQL classification for the tool CLI and MCP server so destructive statements are refused before they run, and valid reads stop getting blocked. The hard block on DROP DATABASE / DROP SCHEMA / TRUNCATE now covers more forms but is documented as best effort, since it reads SQL text: for a hard guarantee, connect with a database user lacking those privileges.

Bug Fixes

  • sql_explain now explains a single statement per call and refuses EXPLAIN options in its input, since some drivers run every statement after the prefix and EXPLAIN ANALYZE executes the statement.
  • Refuses more writes that ran as reads: Snowflake ->> chains, dbt Jinja anywhere in the SQL on the dbt route, Spark/Hive TRANSFORM ... USING, and functions that write or call external systems (ClickHouse jdbc, Spark reflect/java_method, Databricks http_request/remote_query, DuckDB bigquery_execute/ducklake_*/postgres_attach/sqlite_attach, Snowflake GETNEXTVAL). Reads through ClickHouse jdbc(...) now need --allow-write.
  • Sequence advances through quoted or commented NEXTVAL in Oracle, Snowflake, and MariaDB count as writes.
  • The hard block reads a write's literals as SQL with escapes decoded per engine, finds TRUNCATE at statement, block, and handler starts and after THEN/ELSE with quoted, dotted, and IF EXISTS targets, and also blocks DROP CATALOG, DROP NAMESPACE, DROP EXTERNAL SCHEMA, and DROP PLUGGABLE DATABASE.
  • Columns or aliases named truncate, throw, and nextval are reads again.
  • data_diff reads an empty target_warehouse as the source warehouse; the omitted-warehouse dbt dialect check follows the adapter the daemon already created.

Summary by CodeRabbit

  • New Features

    • Added a standalone tool CLI for discovering and running data tools without a model, plus a stdio MCP server for compatible clients.
    • Added optional workspace tools for context, skills, and approved read-only operations, along with workspace client commands.
    • Schema inspection can now list schemas and tables, and SQL execution supports batches of up to eight queries.
    • Added workspace skill syncing for Claude Code and optional routing guidance in CLAUDE.md.
    • Analyst responses now receive stronger checks for requested items, number sources, and ambiguous readings.

  • Improvements

    • Expanded warehouse catalog discovery and MongoDB collection listings, with bounded results and counts.
    • Strengthened read-only SQL protections across tools and supported databases.