[tobyhede]

Requested by Toby

With mapping errors off (the default), a statement that fails type checking falls back: Proxy forwards the client's Query unchanged.
That also forwarded every other statement in the Query as the client wrote it.
So SELECT * FROM no_such_table; INSERT INTO users (email) VALUES ('a@example.com') reached PostgreSQL with the plaintext email, and so did the reverse order (where Proxy encrypted the INSERT and then discarded the result).
In production the failing statement can be one PostgreSQL runs but Proxy cannot type check, such as a read of a table Proxy has not loaded, so the INSERT runs with plaintext in the SQL.
This was already on the 2.2 base before #481, so it is not a regression from the recent backports.

This mirrors main's batch rule ("a fallback forwards the whole Query as the client wrote it, so it must not carry a statement that needs mapping") and main's write policy.

Changes

• Simple Query: the fallback is refused, with the type-check error, when another statement in the Query needs mapping (encryption or decryption), before or after the failing statement. Later statements are type checked to find out. A statement after SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = true is still forwarded as written, as before.
• Writes: a statement that fails type checking and writes data is refused while encrypted columns are configured, whatever the mapping-errors setting, in both the simple and extended protocol. This covers INSERT/UPDATE/DELETE/MERGE, including ones in PREPARE, EXPLAIN and data-modifying CTEs. The exception is a write where Proxy knows every target table has no encrypted columns (2.2's type checker misses common plaintext writes, such as id = ANY(ARRAY[...])). A target that is encrypted, unknown, has no known columns, or is not a plain table name (MERGE) is refused. eql_mapper::write_targets names the targets, and the COPY check's table resolution is shared as Frontend::known_tables.
• eql_mapper::writes_data is ported from main, with its unit test.
• A read that fails type checking is still forwarded unchanged when nothing beside it needs mapping. With no encrypted columns configured, nothing changes.
• The integration test encrypted_column_not_defined_in_schema now expects Proxy's type-check error instead of PostgreSQL's "column does not exist", as main does.
• docs/errors.md and the CHANGELOG.md [Unreleased] section describe the new behaviour.

Tests

New postgresql::tests::type_check_fallback module. The two reported repros (encrypted INSERT after and before a failing statement), a read of an encrypted column beside a failing statement, and the write cases (simple and extended) failed before the fix, each forwarding the plaintext or the Query unchanged. They pass now, along with cases that must still fall back: a lone failing read, a failing read beside plaintext statements or SET/DISCARD, a write after mapping is disabled, and a write with no encrypted columns.

cargo test -p cipherstash-proxy -p eql-mapper --lib and clippy with -D warnings pass locally. Integration tests run in CI.

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

[freshtonic]

Reviewed head b8a5990f against release/2.2 (4938a420). Approved. This fixes the fail-open from my review of #491 (CIP-5168): both repros are now refused, and a fallback with nothing to map is still forwarded unchanged. One test is missing (S1). I will add it in a follow-up PR on top of this branch. The other items are notes and nits, and they can go in this PR or later.

Test gap (I will add the test in a follow-up PR)

S1. No test covers a target table with no known columns. writes_encrypted refuses a write to a table that Proxy knows but whose columns it does not know (table.columns.is_empty(), for example a table created with AS on this connection). When I removed that condition, all 11 type_check_fallback tests still passed. The follow-up adds CREATE TABLE staging AS SELECT * FROM users, and then an INSERT and an UPDATE on staging that fail the type check. Both must be refused.

Notes (not blocking)

• A plaintext predicate on an encrypted column still reaches PostgreSQL in a fallback. I confirmed these with a scratch test. Both are forwarded unchanged, and no error goes to the client:

  • SELECT id FROM users WHERE email = 'a@example.com' AND id = ANY(ARRAY[1::int8])
  • UPDATE notes SET body = 'x' FROM users WHERE users.email = 'a@example.com' AND notes.id = ANY(ARRAY[1::int8])

  The first is the read fallback, which main also keeps (may_fall_back allows a read that hits a coverage gap). The PR description says this is intended. The second is a result of the exception for plaintext targets, which main does not have: on main, a write never falls back while encrypted columns are configured. The UPDATE leaks no more than a SELECT with the same predicate, so I accept the exception, because 2.2's type checker misses common plaintext writes. But the CHANGELOG and docs/errors.md must not let a reader think that a fallback never sends plaintext. Maybe add one sentence: "A statement that falls back is sent as written, so a plaintext value that it compares with an encrypted column reaches the database."

• main also refuses a nested write in a statement that it does not type check (requires_type_check is false and writes_data is true). On 2.2, check_copy covers the usual case, COPY (INSERT …) TO STDOUT. Statements such as CREATE RULE … DO INSERT … still go through without a check. This is rare, so I note it only for parity with main.

Nits

• writes_encrypted also returns true for a write to an unknown table, or to a target that it cannot name. A name such as may_write_encrypted agrees better with the doc comment.
• The comment "PostgreSQL stops at the failed statement, so a CREATE after it is not run" is older than this PR, but it is now on the new loop. It is true only when PostgreSQL also rejects the statement. When the failure is only in Proxy's type check (the production case in the PR description), PostgreSQL runs the statement and the CREATE after it, and Proxy does not record the schema change. The new checks are safe here, because a write to a table that Proxy does not know is refused. But the comment is wrong for this case.

What I checked and found correct

• Mutations. I removed each item below, and for each one at least one type_check_fallback test failed:

  • || mapped in the first branch.
  • The refusal when a later statement needs mapping.
  • The refusal of a later write that fails the type check.
  • The writes_encrypted check in the first branch.
  • The writes_encrypted check in the Parse handler.
  • known_tables returning None → refuse.
  • write_targets returning None → refuse.

  The one condition that did not cause a failure is in S1.

• mapped covers decryption. to_encryptable_statement returns Some for an encrypted projection, so a SELECT email FROM users beside a failing statement is refused in either order. The test a_read_of_an_encrypted_column_beside… checks this.

• known_tables. It returns both the table created on this connection and the loaded table, so a name that also has an encrypted table in another schema is refused. The COPY check uses the same resolution, without a change in behaviour.

• writes_data and write_targets. They find writes in PREPARE, EXPLAIN and data-modifying CTEs. MERGE, a multi-table DELETE, an UPDATE with joins and an INSERT into a table function return None, so they are refused.

• Mapping disabled. A statement after SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = true is skipped in the new loop, and is_passthrough() also checks the global setting. The two ways of disabling mapping behave as before.

• The Python test. encrypted_column_not_defined_in_schema writes to a known encrypted table, so Proxy's refusal now comes before PostgreSQL's error. The new expectation is correct.

What I ran

• cargo test -p cipherstash-proxy --lib: 281 of 282 passed. The one failure, config::tandem::tests::prometheus_config, is the environment race. The config:: tests with --test-threads=1: 42 of 42 passed.
• cargo test -p eql-mapper --lib: 79 passed, 6 ignored.
• The mutations and the scratch test above. I reverted all changes after the runs.
• CI: all 4 checks pass.

[freshtonic]

Follow-up: #495 adds the missing test from S1 in my review (a fallback write to a table with no known columns is refused). It is based on this branch, and GitHub changes its base to release/2.2 when this PR merges. Ticket: CIP-5204.

[tobyhede]

Thanks for the review. I'm about to push one fast-forward commit on top of b8a5990 (no rewrite), so a follow-up branch based on it still applies:

• Rename writes_encrypted to may_write_encrypted.
• Correct the "PostgreSQL stops at the failed statement" comment on the new loop, as you described.
• Add your sentence to docs/errors.md and the CHANGELOG: a statement that is passed through is sent as written, so a plaintext value it compares with an encrypted column reaches the database.

S1 is left to your follow-up as you proposed. The CREATE RULE … DO INSERT parity note stays out of this PR.

---

Generated by Claude Code