Sql driver: implement the §8.1 catalog identifier-validation gate (FF-1) #496

New Issue

2026-07-24T21:00:09-04:00

dohertj2 commented

2026-07-24 21:00:09 -04:00

The Sql driver's ISqlDialect/SqlServerDialect docs describe a design-§8.1 gate that validates an authored table/column against the live catalog before it is ever quoted into SQL text, rejecting unknown identifiers. That gate is NOT implemented and no task on feat/sql-poll-driver scheduled it; QuoteIdentifier (bracket-quoting, Cc+Cf rejection) is currently the sole defence. Risk is bounded — a hostile identifier is quoted into one nonexistent object, the query fails post-open, and the tag Bad-codes (BadCommunicationError) rather than executing anything — but the defence-in-depth and doc-honesty gap should be closed. Also fixed the docs on-branch to stop over-claiming. Reference: docs/plans/2026-07-15-sql-poll-driver-design.md §8.1.

dohertj2 commented

2026-07-25 16:08:36 -04:00

Implemented on fix/sql-driver-followups (4dc2ad80).

SqlCatalogGate + SqlCatalogLoader resolve every authored identifier against the live catalog at Initialize and replace it with the catalog’s own spelling, so the identifier text in an emitted poll query is a string the driver read back out of ListSchemas/ListTables/ListColumns — not operator input. Quoting becomes the backstop it was documented to be. The over-claiming docs on ISqlDialect/SqlServerDialect are corrected.

Decisions worth knowing:

Substitution, not just validation. Exact-ordinal match first, then a unique case-insensitive hit — SQL Server’s default collation is CI, so case variants have always worked and rejecting them would break valid deployments. An ambiguous CI match under a case-sensitive collation is refused rather than guessed: picking one would publish another column’s data under the operator’s node.
Charset check before catalog lookup. A name with a control or Unicode format character is rejected without being echoed into a log line (Trojan-Source); a name that passes is safe to render, which is why catalog-miss messages do name it — an operator hunting a typo has to see what they wrote. Both halves are pinned.
A rejected tag keeps its node. It leaves the polled table but stays in the authored table, so it still materializes and reads BadNodeIdUnknown — §8.1’s stated outcome. My first wiring dropped it from both, deleting the node instead; an existing test caught it. A status code can only be published by a node that exists.
An unreadable catalog faults Initialize rather than rejecting every tag. That is the absence of evidence about the tags, not evidence against them — rejecting all of them would serve a confidently-empty address space and send the operator hunting typos that do not exist. Zero visible schemas is treated the same way, since that is what a missing GRANT looks like.
ISqlDialect gains DefaultSchemaSql (SELECT SCHEMA_NAME()). An unqualified TagValues must resolve in whatever schema the server reports; hardcoding dbo would be a silent lie on estates that map service accounts to their own default schema.
Bounded load: one schema list, one default-schema scalar, then one ListTables per distinct authored schema and one ListColumns per distinct authored relation — never a full enumeration, nothing after Initialize, every authored name bound as a parameter.
Accepted v1 limitation: a 3-part db.schema.table cannot be validated from this connection’s catalog, so it is rejected with a message pointing at the fix (expose the data through a view in the connected database).

Tests: 24 pure gate tests, 9 end-to-end driver tests against the real SQLite catalog, and 3 new live tests against the real SQL Server on 10.100.0.35 — that suite now runs 21 green, exercising the real SELECT SCHEMA_NAME() + INFORMATION_SCHEMA path. Verified falsifiable: bypassing the gate turns exactly the three rejection assertions red and leaves the rest green.

SqlInjectionRegressionTests was deliberately not rewritten to expect BadNodeIdUnknown. It drives SqlPollReader directly, below the gate, and pins the quoting backstop on its own — defence in depth is only worth the name if each layer holds independently. Its scope note (which said the gate does not exist) now explains why it stays put.

Implemented on `fix/sql-driver-followups` (`4dc2ad80`). `SqlCatalogGate` + `SqlCatalogLoader` resolve every authored identifier against the live catalog at Initialize and **replace it with the catalog’s own spelling**, so the identifier text in an emitted poll query is a string the driver read back out of `ListSchemas`/`ListTables`/`ListColumns` — not operator input. Quoting becomes the backstop it was documented to be. The over-claiming docs on `ISqlDialect`/`SqlServerDialect` are corrected. Decisions worth knowing: - **Substitution, not just validation.** Exact-ordinal match first, then a *unique* case-insensitive hit — SQL Server’s default collation is CI, so case variants have always worked and rejecting them would break valid deployments. An ambiguous CI match under a case-sensitive collation is **refused rather than guessed**: picking one would publish another column’s data under the operator’s node. - **Charset check before catalog lookup.** A name with a control or Unicode format character is rejected *without being echoed* into a log line (Trojan-Source); a name that passes is safe to render, which is why catalog-miss messages **do** name it — an operator hunting a typo has to see what they wrote. Both halves are pinned. - **A rejected tag keeps its node.** It leaves the *polled* table but stays in the *authored* table, so it still materializes and reads `BadNodeIdUnknown` — §8.1’s stated outcome. My first wiring dropped it from both, deleting the node instead; an existing test caught it. A status code can only be published by a node that exists. - **An unreadable catalog faults Initialize rather than rejecting every tag.** That is the *absence* of evidence about the tags, not evidence against them — rejecting all of them would serve a confidently-empty address space and send the operator hunting typos that do not exist. Zero visible schemas is treated the same way, since that is what a missing GRANT looks like. - **`ISqlDialect` gains `DefaultSchemaSql`** (`SELECT SCHEMA_NAME()`). An unqualified `TagValues` must resolve in whatever schema the *server* reports; hardcoding `dbo` would be a silent lie on estates that map service accounts to their own default schema. - **Bounded load:** one schema list, one default-schema scalar, then one `ListTables` per distinct authored schema and one `ListColumns` per distinct authored relation — never a full enumeration, nothing after Initialize, every authored name bound as a parameter. - **Accepted v1 limitation:** a 3-part `db.schema.table` cannot be validated from this connection’s catalog, so it is rejected with a message pointing at the fix (expose the data through a view in the connected database). **Tests:** 24 pure gate tests, 9 end-to-end driver tests against the real SQLite catalog, and 3 new live tests against the real SQL Server on `10.100.0.35` — that suite now runs 21 green, exercising the real `SELECT SCHEMA_NAME()` + `INFORMATION_SCHEMA` path. **Verified falsifiable:** bypassing the gate turns exactly the three rejection assertions red and leaves the rest green. `SqlInjectionRegressionTests` was deliberately **not** rewritten to expect `BadNodeIdUnknown`. It drives `SqlPollReader` directly, below the gate, and pins the quoting backstop on its own — defence in depth is only worth the name if each layer holds independently. Its scope note (which said the gate does not exist) now explains why it stays put.

dohertj2 referenced this issue from a commit

2026-07-25 20:24:23 -04:00

merge: SQL poll driver (feat/sql-poll-driver)

dohertj2 referenced this issue from a commit

2026-07-25 20:24:23 -04:00

feat(sql): implement the design §8.1 catalog identifier-validation gate (#496)

dohertj2 referenced this issue from a commit

2026-07-25 22:44:40 -04:00

merge: Sql driver follow-ups #496/#497/#498 + the Runtime.Tests flake #500 (fix/sql-driver-followups)

dohertj2 closed this issue

2026-07-26 08:34:51 -04:00

Sign in to join this conversation.