Repository navigation
refactor(proxy): route each statement through one refuse-by-default decision - #484
Conversation
|
Supported pooler compatibility (PostgreSQL 17) is failing on the Go pgx suite's direct baseline ( The likely cause comes from the base branch (CIP-5102), not from this PR's commit. This PR doesn't change that outcome; the same write would be refused by the base's guard. The fix belongs with CIP-5102. One option is to have the schema middleware remember the native temporary tables created on the connection, so writes to them count as native rather than unresolvable. I've raised it there, and I'm not re-running the job because the failure is deterministic. Generated by Claude Code |
9b0dc9a to
d8691ec
Compare
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation — automated review by Claude Code (/code-review medium, findings cross-checked against the PR head 8e15737 and existing PR feedback via company-skills:cross-check-review-findings).
Result: no critical or blocking issues; no new findings.
Checked at head:
Frontend::routeis the only decision point; the Query and Parse paths no longer carry their own check ladders.is_coverage_gapis an allowlist (StatementCouldNotBeTypeChecked,Internal), so other errors fail closed.InvalidInboundEqlPayloadand the legacy EQL v2 refusal are not on it.- A fallback cannot be forwarded alongside a
Route::Mapstatement. All statements are routed before any is acted on, so statements after a fallback are now checked. - With encrypted columns configured, a write falls back only when every target is a known-native table (
write_targetsreturnsNonefor non-plain targets, so those are refused). An unfalsified fallback still returns the original bytes (Ok(None)), not a re-serialised statement. - Tests cover the refusal, the extended-protocol path, the no-encrypted-columns case, and native-table-created-earlier edge cases. CHANGELOG,
docs/errors.mdandCONTEXT.mdmatch the behaviour.
Observation, not a finding: the PR's own comment reports the Go pgx pooler job failing on the base branch (CIP-5102). It is not caused by this change.
Generated by Claude Code
Proxy's view of a connection's tables lagged PostgreSQL's inside a simple Query and for native temporary tables, and #484 patched the gap at the fallback layer with two side channels. Fix the schema instead: - The schema middleware keeps each connection's native temporary tables as a pg_temp layer the type checker resolves first. A table is added when PostgreSQL confirms its CREATE, and removed by DROP, DISCARD, ON COMMIT DROP at commit, and rollback of its transaction or savepoint. - Each statement in a simple Query is type checked against the schema as the encryption-neutral DDL before it in the Query leaves it. - Writes therefore type check normally, so a write never falls back while encrypted columns are configured. The earlier-statements slice, the temp table side set, writes_only_native_tables, creates_native_table, object_names_equal and eql_mapper::write_targets are gone. A same-batch ALTER TABLE ADD COLUMN or RENAME followed by a write now type checks instead of being refused, and a write before its CREATE TEMP TABLE, or after its DROP, is refused. Closes CIP-5109 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A design-level model of how Proxy maps, forwards or refuses each statement, checked with TLC: plaintext must never reach an encrypted column except with UNSAFE_DISABLE_MAPPING. Covers the refuse-by-default route from #484, simple Query batches, Parse and Bind, transactions and concurrent DDL. TLC confirms the safety properties for clients that never disable mapping, and records three findings as expected counterexamples: stale routes under concurrent DDL, statement kinds Proxy never type checks, and a Parse under UNSAFE_DISABLE_MAPPING bound after mapping is enabled again. Three injected faults are each caught. Adds a non-required CI workflow. No production code changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
freshtonic
left a comment
There was a problem hiding this comment.
Reviewed head e06e125a, against the base claude/project-thread-2yjn9s. I first reviewed 88f73d70, and then the two new commits. The core refactor is sound: Frontend::route is the one place that decides, and the allowlist makes refusal the default. The tests guard the refactor well. a113b8fb (AmbiguousTable) is correct. But e06e125a adds a new fail-open path.
Blocking
B1. CREATE TABLE IF NOT EXISTS for a table that PostgreSQL has and Proxy has not loaded forwards plaintext.
batch_models (proxy/schema/middleware.rs:900-920) treats a name that the loaded schema cannot resolve as a new plain table. Its doc comment says the table "has no encrypted columns, or the batch view would not get here". That is true only for tables that are in Proxy's encrypt config. A table that Proxy has not loaded is not in the schema and not in the encrypt config, so neither guard sees it.
Scenario: late already exists in PostgreSQL with an EQL column, but Proxy has not loaded it yet. The client sends one simple Query, CREATE TABLE IF NOT EXISTS late (secret text); INSERT INTO late (secret) VALUES ('plaintext').
- Proxy models a new plain
late, so it forwards theINSERTwith the plaintext. - PostgreSQL skips the CREATE with a notice and does not abort the batch. Then it runs the
INSERTagainst the reallate. - Only the EQL domain CHECK stops the value, and the PostgreSQL error and logs then contain the plaintext.
CREATE TABLE IF NOT EXISTS public.late …has the same problem.
This is a regression: on 88f73d70, Proxy refused the same Query as an unresolvable table. A plain CREATE TABLE late … is safe, because PostgreSQL fails on the existing table and aborts the batch. Only IF NOT EXISTS lets the batch continue.
This can happen after a migration that runs directly against PostgreSQL, until the next schema reload (config_reload_interval, 60 s by default). It can also happen on a connection in an open transaction whose snapshot is older than a change from another connection. docs/errors.md:438 still says that a table "created on another connection since Proxy last loaded the schema cannot be resolved", so the docs now disagree with the code.
A probe test on e06e125a showed that the Query is forwarded as written, with nothing encrypted.
Fix: revert e06e125a, or model IF NOT EXISTS only for a name that an earlier statement in the same batch created. Please add a test for this case. The new test write_after_create_if_not_exists_of_an_encrypted_table_is_not_forwarded_as_plaintext covers only a table that Proxy already knows.
Checked and safe in the new commits:
- Ambiguous names:
batch_modelsuses the scratch catalog of permanent tables, so a temporary table cannot make a lookup fail there. A write is refused withAmbiguousTable. CREATE TABLE IF NOT EXISTSon an encrypted name, with or withoutpublic.orpg_temp., andCREATE TEMP TABLE IF NOT EXISTS encrypted: theddl_may_change_encryptionguard refuses them.AmbiguousTablereaches every path that usedas_table_not_found. A read on an ambiguous name is still forwarded.
Should fix (not blocking)
1. Proxy now refuses every write that fails type checking, also writes to native tables. The CHANGELOG does not say this.
may_fall_back (middleware/frontend.rs:1278-1280): with encrypted columns configured anywhere in the database, Proxy now refuses these statements:
UPDATE encrypted SET id = v.x FROM (VALUES (1)) AS v(x) WHERE encrypted.id = v.x("no matches for identifier v.x")CREATE TEMP TABLE n (id int); INSERT INTO n SELECT x FROM unnest(ARRAY[1,2]) AS x("Unsupported table source: UNNEST")INSERT INTO encrypted (id) SELECT x FROM unnest(...)INSERT INTO <temp> SELECT * FROM pg_catalog.pg_class
The base branch forwarded the first three. Bulk UPDATE … FROM (VALUES …) is a common pattern in ORMs and in hand-written SQL. The compatibility note in the CHANGELOG names only MERGE and SQL PREPARE, so it does not tell customers the full effect. You can do one of these:
- Bring back the fallback for "all write targets are known native tables" (
write_targets, removed in 1fb2a48). - Keep the refusal, and add these examples to the CHANGELOG compatibility note and to docs/errors.md.
A probe test confirmed this on head and on base.
2. COPY (INSERT … RETURNING …) TO STDOUT sends plaintext to an encrypted column.
The Forward::NotTypeChecked arm (frontend.rs:1249-1251) returns before the write check. So Proxy forwards COPY (INSERT INTO encrypted (secret) VALUES ('plaintext') RETURNING id) TO STDOUT unchanged, and PostgreSQL accepts DML with RETURNING in COPY. Only the EQL domain CHECK constraint stops it. This existed before this PR, but it disagrees with "a write never falls back". eql_mapper::writes_data already returns true for this statement. So the fix is small: with encrypted columns configured, route refuses a statement that is not type checked and writes data. A probe test confirmed this.
3. CREATE TABLE pg_temp.n (…); INSERT INTO n … is refused. Proxy treats a CREATE with the pg_temp. prefix and no TEMP keyword as a permanent table. Please document this or model it. (a113b8fb fixes the misleading message for a name that is both a temporary and a permanent table.)
Nits
- The temp-table layer can forget a table that PostgreSQL keeps.
Temporary::failedcovers the time between two ReadyForQuery messages, not one transaction (proxy/schema/middleware.rs:195-211). So inBEGIN; CREATE TEMP TABLE t …; COMMIT; SELECT 1/0(one simple Query), Proxy forgetst, but PostgreSQL keeps it. This causes over-refusal only.BEGIN; DROP TABLE t; COMMIT; <error>has the same cause and leaves a stalet; the shadowing rule makes that case safe.ROLLBACK AND CHAINis not handled. Found by reading the code. - The batch view ignores ROLLBACK and COMMIT. Proxy forwards
BEGIN; CREATE TEMP TABLE t …; ROLLBACK; INSERT INTO t …. PostgreSQL then fails with "relation does not exist", so this is safe. - No unit test for the allowlist. If
is_coverage_gapmatches everyMappingError, all lib tests still pass. Only the integration testlegacy_v2_column::select_is_refusedcatches it. A unit test that sends a SELECT on a legacy v2 column with mapping errors off would guard it. - No test exercises the encryption guard in
batch_resolvers. If you skipddl_may_change_encryptionthere, all tests still pass, becausesimple_query_requires_fail_closedrefuses the case first. This is good defence in depth, but the inner guard has no test. - The PR body is wrong about DO. sqlparser cannot parse
DO, so Proxy refuses it. CALL and SQLEXECUTEare forwarded, as before. - A read that falls back still sends plaintext query operands to PostgreSQL, for example
SELECT * FROM encrypted WHERE secret = 'plaintext' AND nosuch(id). This is how the read fallback is designed to work, and it existed before this PR. ButStatementCouldNotBeTypeCheckedalso covers bound (capability) failures, so the allowlist is wider than its name suggests.
What I checked and found correct
- One decision point.
routedecides for a simple Query with one or more statements, for Parse, for SQL PREPARE and for MERGE. When the config disables mapping,interceptreturns early for the whole connection. WhenSETdisables mapping,routereturnsMappingDisabled. No path forwards a statement without a decision. Internalcan safely fall back, becausemay_fall_backrequires!writes_encrypted.- Write detection.
writes_datadetects CTE writes,EXPLAIN ANALYZEof a write,PREPAREof a write and MERGE. CTE and EXPLAIN writes are encrypted correctly when they type check.SELECT INTOandCREATE TABLE AScopy only ciphertext. - Multi-statement Query. Proxy routes every statement before it acts on any. If the batch has a fallback and a mapped statement, Proxy refuses the whole batch.
- Temp-table lifecycle. Proxy adds a temp table only after PostgreSQL confirms the CREATE. It handles a failed CREATE, DROP, DISCARD TEMP and DISCARD ALL, ON COMMIT DROP, rollback, rollback to a savepoint, and a pipelined CREATE then Parse.
- Name resolution. A shadowed name does not resolve.
pg_temp.tresolves only to the temp table, andpublic.tnever resolves to it. - Batch view. DDL that changes encryption is refused. After
ADD COLUMNorRENAME COLUMNon the encrypted table, the write to the encrypted column is still encrypted. Quoted names stay distinct. - CI. The pooler failure with Go pgx is fixed on head. All 8 checks pass.
What I ran
- On
e06e125a:cargo test -p cipherstash-proxy --lib: 458 passed, except the known random failure. On88f73d70: 457 passed (one known test that fails at random,config::tandem::missing_database_config, failed in one run).cargo test -p eql-mapper: 225 passed. - About 60 probe SQL cases on
88f73d70, 5 on base and 16 one06e125a. - 12 mutations: 10 make tests fail. The two that no test catches are nits 3 and 4.
|
Thanks for the thorough review. Here is how each point is handled. The commits are about to be pushed. B1. Agreed, that is a regression. Should fix 1. Kept the refusal, since refusing every write that fails type checking is the policy chosen for this PR. Should fix 2. Should fix 3. Nits
Generated by Claude Code |
A design-level model of how Proxy maps, forwards or refuses each statement, checked with TLC: plaintext must never reach an encrypted column except with UNSAFE_DISABLE_MAPPING. Covers the refuse-by-default route from #484, simple Query batches, Parse and Bind, transactions and concurrent DDL. TLC confirms the safety properties for clients that never disable mapping, and records three findings as expected counterexamples: stale routes under concurrent DDL, statement kinds Proxy never type checks, and a Parse under UNSAFE_DISABLE_MAPPING bound after mapping is enabled again. Three injected faults are each caught. Adds a non-required CI workflow. No production code changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
freshtonic
left a comment
There was a problem hiding this comment.
Re-reviewed head 4d14630b, against the base claude/project-thread-2yjn9s (now at 025dba5b). Thank you for the quick fixes. B1, the COPY write and pg_temp. are fixed, and the two missing guard tests are added. But the merge of 025dba5b from the base creates a new fail-open path.
Blocking
B2. If every encrypted column has a domain conflict, writes to those columns are forwarded as plaintext.
025dba5b removes a column's encrypt config when tables with the same name in two schemas declare different domains. If those are the only encrypted columns, the encrypt config becomes empty. Then has_encrypted_columns() (context/mod.rs:1748, !encrypt_config().is_empty()) is false, so the write refusal (frontend.rs:1273) and the refusal of a write in a statement that is not type checked (frontend.rs:1253) both turn off. But the schema still marks the columns as EQL.
Scenario: app.users.email is eql_v3_text_search, public.users.email is eql_v3_text, and no other column is encrypted. Then Proxy forwards these as written, with the plaintext:
UPDATE app.users SET email = v.x FROM (VALUES ('plaintext')) AS v(x) …MERGE INTO app.users … INSERT (email) VALUES ('plaintext')COPY (INSERT INTO app.users (email) VALUES ('plaintext') RETURNING id) TO STDOUTINSERT INTO users (email) VALUES ('plaintext'). The bare name matches two schemas, so the statement falls back.
Only the EQL domain CHECK in PostgreSQL stops these. Proxy refuses INSERT INTO app.users … and SELECT email FROM app.users correctly. The 025dba5b test adds another encrypted column "so encrypted columns are configured", and that hides this case. A probe test on 4d14630b confirmed it.
Fix: make has_encrypted_columns true when the effective schema has any EQL column, or when the encrypt config is not empty or the schema has an EQL column. Add a test with no other encrypted column.
Fixed since my last review (checked by probe tests)
- B1: Proxy refuses
CREATE TABLE IF NOT EXISTS late …; INSERT INTO late …and thepublic.latevariant again. The exception for a table that an earlier statement in the same Query created is safe. If the first CREATE fails because the table exists, PostgreSQL skips the rest of the Query. - The COPY write (
d1b558a7): Proxy refusesCOPY (INSERT/UPDATE/DELETE … RETURNING) TO STDOUTin both protocols, also after a SELECT in the same Query. It still forwardsCOPY (SELECT …) TO,COPY t TO,EXECUTEandCALL. pg_temp.(7fda1d92):- Proxy treats
CREATE TABLE pg_temp.nas a temporary table, also with other letter case and with quotes. - Proxy removes it after DROP, ON COMMIT DROP, rollback to a savepoint, a failed transaction and DISCARD TEMP.
- When a permanent table has the same name, a write to that name is refused as ambiguous.
- Proxy treats
- Guard tests (
f6fdea38): M1 and M12 now make tests fail, and so does removing thed1b558a7check. - The merge: none of
21f623dc'swrite_targetsfallback came back. Other statements cover the cases that its tests covered.proxy/schema/manager.rsmatches025dba5b. When another column is encrypted, a write toapp.usersnever uses the config ofpublic.users. - Over-refusal: you chose to keep the refusal and document it. I accept that.
Nits
- The CHANGELOG entry now has the examples, but its "Compatibility note" line (
CHANGELOG.md:82) still names onlyMERGEand SQLPREPARE. Add the native-table writes andCOPY (…) TO. - The list of refusals at
docs/errors.md:436does not haveCOPY (…) TO, butdocs/errors.md:250now lists it. - The temporary-table layer and the batch view always resolve a bare name to the temporary table first, and Proxy does not track
SET search_path. Withsearch_path = public, pg_temp,CREATE TEMP TABLE x …; CREATE TABLE IF NOT EXISTS x …; INSERT INTO x …writes topublic.x, which Proxy may not have loaded. The default search path is safe. Please at least document this. - Proxy still models
IF NOT EXISTSfor a temporary table (CREATE TEMP TABLE IF NOT EXISTS late …, orpg_temp.late). This is unsafe only if a temporary table with an EQL column exists that Proxy did not see, for example one created inside a function. That is unlikely, because Proxy does not support encrypted temporary tables. - Your reply says that COPY FROM STDIN is "not affected because
writes_datais false". On this head, it never reachesroute. The parser refuses it in both protocols with "Expected: ;, found: EOF". #483 fixes this on 2.2. - Earlier nits 1 and 6 are still open, as you said:
failedcovers the time between two ReadyForQuery messages, not one transaction, and Proxy does not handleROLLBACK AND CHAIN. A read that falls back can send plaintext operands. These can wait.
What I ran
- On
4d14630b:cargo test -p cipherstash-proxy --lib464 passed,cargo test -p eql-mapper225 passed, andcargo fmt --checkandcargo clippy --workspace --all-targets -D warningsreport no problems.
|
Thanks for the re-review. These commits are about to be pushed. B2. Agreed. Nits
Generated by Claude Code |
|
Pushed Generated by Claude Code |
Proxy's view of a connection's tables lagged PostgreSQL's inside a simple Query and for native temporary tables, and #484 patched the gap at the fallback layer with two side channels. Fix the schema instead: - The schema middleware keeps each connection's native temporary tables as a pg_temp layer the type checker resolves first. A table is added when PostgreSQL confirms its CREATE, and removed by DROP, DISCARD, ON COMMIT DROP at commit, and rollback of its transaction or savepoint. - Each statement in a simple Query is type checked against the schema as the encryption-neutral DDL before it in the Query leaves it. - Writes therefore type check normally, so a write never falls back while encrypted columns are configured. The earlier-statements slice, the temp table side set, writes_only_native_tables, creates_native_table, object_names_equal and eql_mapper::write_targets are gone. A same-batch ALTER TABLE ADD COLUMN or RENAME followed by a write now type checks instead of being refused, and a write before its CREATE TEMP TABLE, or after its DROP, is refused. Closes CIP-5109 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
d8d1d78 to
1190a91
Compare
freshtonic
left a comment
There was a problem hiding this comment.
Re-reviewed head 1190a910. Its file tree is byte-for-byte the same as the tree of d8d1d784, which is the commit I reviewed. I reviewed af2dcb05 and cb5c9212 first; the merge a6dd4f91 changes no file. Then I reviewed d8d1d784. B2 is fixed, so I approve.
B2 (af2dcb05)
has_encrypted_columns(context/mod.rs:1749) is now also true when the loaded schema has a column that is not native, even when the encrypt config is empty. This includes EQL columns and legacy v2 columns.- Both write refusals use it:
frontend.rs:1253and:1273. These are its only callers. Only the backend usesis_passthrough, which is still based on an empty config, so it does not change the frontend decision. write_is_refused_when_only_columns_with_no_config_are_encryptedcovers my scenario with an empty encrypt config: the bare-name INSERT,UPDATE … FROM (VALUES …), MERGE and the COPY write. I removed the new check locally, and the test failed. With the check, all 25unresolvable_table_testspass.- The test harness now adds the
legacytable only whenencryptedis set. This is necessary, because a legacy v2 column now counts as an encrypted column, so the tests without encryption would refuse writes.
Docs (cb5c9212)
- The compatibility note in the CHANGELOG now lists the native-table writes and
COPY (…) TO. - The "Unresolvable table" section now lists
COPY (…) TO. CONTEXT.mdand the "Ambiguous table" section say that Proxy assumespg_tempis searched first and does not trackSET search_path.
Unsupported operations (d8d1d784)
This fixes my earlier nit 6 for operations that a column does not support. mapping_error (frontend.rs:1329) now refuses an UnsatisfiedBounds error as UnsupportedEncryptedOperation, and does not treat it as a coverage gap that falls back. It does this before the unresolvable-table check and in both protocols, because both protocols use mapping_error. UnsatisfiedBounds comes only from bound checks on EQL types: native types meet every bound. So catalog queries and reads of plaintext columns still fall back. The error text names the type and the missing bound, not the operand. read_using_an_operation_an_encrypted_column_does_not_support_is_refused fails if the check is removed (I tried this).
Nits
- From reading the type checker (I did not probe these),
UnsatisfiedBoundsalso comes from projections, for exampleSELECT DISTINCTorUNIONover an encrypted column that has noEq. It also comes fromORDER BYorGROUP BYon a column with no ordering or equality index. These reads send no plaintext operand. Before this commit they fell back, and PostgreSQL ordered or compared the ciphertext asjsonb, which gives wrong results with no error. So refusing them is better. But the CHANGELOG names only<. Please add the projection andORDER BYcases to the compatibility note, because applications that send them will now get an error. - The new test covers only the simple protocol. The extended protocol uses the same
mapping_error, so a short Parse test would guard it. - A database that has only legacy v2 columns, and no encrypt config, now refuses writes that Proxy cannot type check. This is correct, because Proxy cannot map those columns. But the CHANGELOG says "while encrypted columns are configured", so a user with only v2 columns might not see that this applies to them. A few words, such as "including legacy EQL v2 columns", would make it clear.
What I ran
cargo test -p cipherstash-proxy --libond8d1d784: 466 passed.cargo fmt --checkandcargo clippy -D warningsreport no problems.cargo test -p eql-mapper: 225 passed.
Conditions
- This approval is conditional on CI passing. GitHub runs no CI on
1190a910, because the PR now conflicts with its base branch, so the last CI result for this tree comes fromd8d1d784. That run was still in progress when I approved. - Please resolve the conflict, and make sure CI passes on the new head before merge.
Approved, on the condition that CI passes.
|
Thanks for approving. I pushed
I'll confirm here when CI on the new head is green. Generated by Claude Code |
e0fb5da to
6143e4e
Compare
fb28477 to
089c427
Compare
6143e4e to
e9d509a
Compare
35ca8e8 to
7aea8ff
Compare
Frontend::route is now the only place that decides what happens to a statement: map it, forward it for a named reason, or refuse it. Only a coverage gap (a statement EQL Mapper could not type check, or an internal error) may fall back to forwarding the statement as written, and only for a read while mapping errors are off. Any other error, including one added later, refuses. While any column is encrypted, a write never falls back. So that writes to native tables still type check instead of being refused, Proxy's schema now covers them: - A connection's native temporary tables live in a temporary layer that resolves before permanent tables. A table is added once PostgreSQL confirms its CREATE, including CREATE TABLE pg_temp.name, and removed by DROP, DISCARD, ON COMMIT DROP, rollback and rollback to a savepoint. A bare name that also names a permanent table is ambiguous and refused. - In a simple Query, each statement is type checked against the schema plus the encryption-neutral DDL before it in the same Query. CREATE TABLE IF NOT EXISTS is not modelled, since the table may already exist with encrypted columns Proxy has not loaded. Also refuses a write nested in a statement Proxy does not type check, such as COPY (INSERT ... RETURNING ...) TO STDOUT; an operation an encrypted column's EQL type does not support, which would otherwise reach PostgreSQL with a plaintext operand or compare ciphertext; and counts encrypted columns in the loaded schema even when the encrypt config has none for them.
7aea8ff to
109eca9
Compare
Requested by Toby
Stacked on the CIP-5102 branch (
claude/project-thread-2yjn9s). Also closes CIP-5109. Retarget tomainonce that merges.Why
The recent chain of fail-open fixes (CIP-5101, CIP-5102, #482) shares a cause: forwarding a statement as written was the default, and refusing was opt-in.
Query and Parse each copied the same ladder of checks, and anything that fell through was forwarded.
Error::must_fail_closedwas a deny-list, so every new refusal had to be added to it or it silently became a passthrough.One new rule (CIP-5102) touched 8 to 10 call sites.
What changes
Frontend::routeis the only place that decides what happens to a statement:Route::Map,Route::Forward(Forward)with a named reason (MappingDisabled,NotTypeChecked,NothingEncrypted,Fallback), orErrto refuse.Error::must_fail_closed(deny-list) is replaced byError::is_coverage_gap(allowlist): onlyStatementCouldNotBeTypeCheckedandInternalmay fall back. Any error added later fails closed without being listed.eql_mapper::writes_data) that fails type checking is refused. Reads keep the fallback so catalog queries from drivers, ORMs and psql keep working (Toby's call).any_requires_mappingand themappedflag.Behaviour changes
With encrypted columns configured and mapping errors off (the production default), these were forwarded with plaintext values and are now refused:
MERGEinto a resolvable table.SQL
PREPAREof anINSERT/UPDATE/DELETE, as a simple Query or a Parse.Such a write after a statement that falls back in the same simple Query.
INSERT INTO encrypted (id, encrypted_unconfigured) ...(an unknown column) is now refused by Proxy instead of reaching PostgreSQL; the integration test is updated.Still forwarded: reads that can't be type checked.
A write never falls back (CIP-5109).
Instead, Proxy's schema now covers the tables the old exceptions were for:
pg_templayer that resolves before the catalog.They are added once PostgreSQL confirms the
CREATE, and removed byDROP,DISCARD TEMP/ALL,ON COMMIT DROPat commit, rollback, and rollback to a savepoint.A bare name that is both a temporary and a permanent table is ambiguous, so a write to it is refused with a new Ambiguous table error that says to qualify it.
CREATE TABLE(includingCREATE TABLE pg_temp.name),ALTER TABLE ... ADD COLUMNandRENAMEfollowed by a write in the same batch are now forwarded; a write placed before itsCREATE TEMPORARY TABLEis refused.CREATE TABLE IF NOT EXISTSis not modelled, because the table may already exist with encrypted columns Proxy has not loaded, so a write after it is refused unless an earlier statement in the batch created the table.INSERT INTO encrypted ... SELECT ... FROM <table created earlier in the query>is now encrypted rather than refused.Tests
New unit tests in
frontend.rs(unresolvable_table_tests), written first and confirmed failing on the base branch:simple_write_that_fails_type_checking_is_refusedextended_write_that_fails_type_checking_is_refusedstatements_after_a_passthrough_fallback_are_checkedwrite_that_fails_type_checking_is_forwarded_without_encrypted_columnsextended_read_on_an_unresolvable_table_is_forwardedCIP-5109 adds resolver tests in
eql-mapperand temporary-table lifecycle and batch-view tests in the schema middleware.All 457
cipherstash-proxylib tests and 225eql-mappertests pass, and clippy is clean with-D warnings.Not in this PR
Option<Statement>from the prepared statement. Since writes that fail type checking are now refused at Parse, aNonethere can only come from a read.EXECUTEand CALL are stillForward::NotTypeChecked, except that a write nested in one, such asCOPY (INSERT ... RETURNING ...) TO STDOUT, is now refused while any column is encrypted.DOis refused because it can't be parsed.Acknowledgment
By submitting this pull request, I confirm that CipherStash can use, modify, copy, and redistribute this contribution, under the terms of CipherStash's choice.
Generated by Claude Code