fix(query): enforce read-only SQL contract in query_asset_tool

[vjymisal0]

Summary

Fixes #226. query_asset_tool is documented as read-only ("CRITICAL: Use READ-ONLY queries to retrieve data. Write and modify queries are not supported by this tool."), but tools/query.py::query_asset() only checked that sql and connection_qualified_name were non-empty before building a QueryRequest and calling client.queries.stream(...). Any SQL — INSERT/UPDATE/DELETE/DDL, or multiple statements — was passed straight through to the Atlan query service. An operator restricting write tools via RESTRICTED_TOOLS (per the README's "Read-Only Access" example) would still have unrestricted SQL execution available, since query_asset_tool wasn't even in that list.

Changes

• modelcontextprotocol/utils/sql_validator.py (new): a small, dependency-free validate_read_only_sql(sql) that:
  • strips string/identifier literals and comments so keyword matching isn't fooled by SQL that merely mentions a keyword inside a literal,
  • rejects multi-statement SQL (;-separated),
  • only allows statements whose leading keyword is SELECT/WITH/SHOW/DESCRIBE/DESC/EXPLAIN/VALUES,
  • rejects write/DDL/session keywords (INSERT, UPDATE, DELETE, DROP, ALTER, CREATE, GRANT, INTO, ...) anywhere in the statement, so a write hidden inside a CTE (e.g. WITH t AS (INSERT INTO ... RETURNING *) SELECT * FROM t) is also caught.
• modelcontextprotocol/tools/query.py: call the validator before constructing QueryRequest; on rejection, return the existing {success, data, error, query_info} shape (fails closed, never reaches the Atlan client) rather than raising.
• modelcontextprotocol/server.py: note the enforcement in the query_asset_tool docstring.
• modelcontextprotocol/README.md: add query_asset_tool to the documented tool-restriction list (it was already restrictable via RESTRICTED_TOOLS, just undocumented), and note that the tool now rejects non-read-only SQL on its own.
• modelcontextprotocol/tests/ (new): this repo has no test suite yet (tracked separately in [FEATURE] Add tests #36), so I added a minimal pytest setup (wired via [tool.pytest.ini_options] in pyproject.toml, pytest added as a dev extra) with:
  • test_sql_validator.py: parametrized allow/reject cases for the validator.
  • test_query.py: confirms query_asset() rejects write/multi-statement SQL without calling get_atlan_client(), and that a valid SELECT still reaches QueryRequest/client.queries.stream() (mocked, no network/credentials needed).

This is intentionally a conservative keyword-based check rather than a full SQL parser/AST — it's meant to fail closed on anything ambiguous rather than to validate SQL correctness, per the issue's suggested approach.

Test plan

• pytest tests/ — 27 passed
• ruff check / ruff format clean on all new/touched files
• Manually verified against the issue's repro shape (INSERT/UPDATE/DELETE/DDL/multi-statement SQL rejected before QueryRequest is built; existing SELECT examples from the tool's own docstring still work)

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[vjymisal0]

The only red check I can see is with no public details. Endor Labs is green. I don't have access to the Snyk org output, so this likely needs a maintainer-side rerun or details from the Snyk dashboard.