Skip to content

Harden query execution and inline write safety - #43

Open
fcoury wants to merge 2 commits into
masterfrom
agent/harden-query-execution
Open

Harden query execution and inline write safety#43
fcoury wants to merge 2 commits into
masterfrom
agent/harden-query-execution

Conversation

@fcoury

@fcoury fcoury commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Why

After #44 landed, tsql streams unpageable simple-protocol results and preserves SQL NULL identity, but several safety gaps remain around server-side result bounds and writes against production-like connections. Large eligible queries can be constrained earlier, metadata lookups should not interpolate table names, and destructive or generated writes should be reviewable before execution.

This change adds layered accident prevention without treating client-side checks as an authorization boundary: saved read-only PostgreSQL connections also enable the server's read-only transaction default, while the TUI fails closed when it cannot confidently classify a statement.

What Changed

  • Route eligible single PostgreSQL SELECT, WITH, VALUES, and TABLE statements—including joins, subqueries, and CTEs—through a capped cursor query, and cap direct DML RETURNING results before they reach the simple-protocol result path.
  • Classify PostgreSQL and MongoDB operations conservatively, including parenthesis-aware detection of UPDATE or DELETE statements without a matching top-level WHERE; require confirmation for destructive Classic queries and direct destructive Notebook users to Classic for review.
  • Add top-level read_only_connections configuration for saved connections, block writes and unknown statement shapes locally, enable default_transaction_read_only for PostgreSQL sessions, disable inline cell editing, and mark the active connection as [read-only] in the status line.
  • Show the exact generated SQL before an inline PostgreSQL cell update runs, quote reserved identifiers, parameterize metadata queries, and use column types plus SQL-null identity when generating edited values and row-match predicates.
  • Raise the workspace MSRV to Rust 1.88, enforce warning-free workspace linting and startup coverage in CI, make database-backed integration tests fail when their required URL is absent, and align the local format/clippy hooks with the current workspace layout.

Saved connections can be protected in ~/.tsql/connections.toml:

read_only_connections = ["production", "analytics-replica"]

This setting is an accident-prevention guard. Database roles without write privileges remain the security boundary.

How to Test

  1. Add a saved PostgreSQL connection name to read_only_connections, connect through that saved entry, and confirm the status line includes [read-only]. Run a SELECT and verify it succeeds, then attempt an UPDATE, CALL, or malformed statement and verify tsql blocks it.
  2. On a writable test connection, run an UPDATE or DELETE without a top-level WHERE. Confirm the destructive-write dialog shows the statement, cancel it, and verify no rows changed. Then run a scoped UPDATE ... WHERE ... and verify it follows the normal execution path.
  3. Edit a PostgreSQL grid cell and confirm the dialog displays the exact generated UPDATE before execution. Exercise a text column containing NULL, 123, or true and verify the value remains text rather than being coerced.
  4. Set a small connection.max_rows, run a larger join or CTE in Classic mode, and verify the result stays within the configured cap while normal paging and result transforms remain usable.
  5. Describe a table whose name needs quoting and verify metadata loads successfully without interpolating the identifier into the metadata query.

Targeted tests run for this change:

  • cargo test -p tsql query_safety --lib
  • cargo test -p tsql destructive_query_requests_confirmation --lib
  • cargo test -p tsql read_only_saved_connection_fails_closed_for_writes_and_unknown_sql --lib
  • cargo test -p tsql cell_update_confirmation_includes_generated_sql --lib
  • cargo test -p tsql test_build_update_where_clause_falls_back_to_ctid_match --lib
  • cargo test -p tsql --test startup_nonblocking
  • TEST_DATABASE_URL=postgres://fcoury@localhost/postgres cargo test -p tsql --test integration_tests

@fcoury
fcoury marked this pull request as ready for review July 10, 2026 01:28
@fcoury
fcoury force-pushed the agent/harden-query-execution branch from 213dad5 to 52bfe22 Compare July 10, 2026 15:50
fcoury added 2 commits July 13, 2026 22:40
Bound eligible Classic results before cursor materialization and add
fail-closed write classification, destructive confirmations, and saved
read-only connections with PostgreSQL session enforcement.

Parameterize metadata lookups and preserve typed cell values while
confirming generated update SQL before execution.
Run strict workspace linting, the Rust 1.88 compatibility check, and
startup coverage in CI. Make database-backed tests fail when their
required connection is missing.

Update local hooks for the workspace layout and document saved
read-only connection safeguards.
@fcoury
fcoury force-pushed the agent/harden-query-execution branch from 52bfe22 to 9705e8c Compare July 14, 2026 06:05
@fcoury fcoury changed the title Harden query execution and result handling Harden query execution and inline write safety Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant