Skip to content

fix(proxy): forward COPY FROM STDIN into plaintext tables, refuse it into encrypted columns (2.2, CIP-5101) - #483

Open
tobyhede wants to merge 1 commit into
release/2.2from
claude/project-thread-7897nb
Open

tobyhede wants to merge 1 commit into
release/2.2from
claude/project-thread-7897nb

Conversation

@tobyhede

@tobyhede tobyhede commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Toby

Fixes CIP-5101 on release/2.2: a COPY ... FROM STDIN was refused because Proxy's SQL parser rejected it.

Cause

The SQL parser expects the copy data of a COPY ... FROM STDIN to follow it in the SQL, as in a psql script.
Without a ; after STDIN it refuses the statement, so psql's \copy and driver COPY APIs could not load any table through Proxy.
With a ;, it reads the rest of the Query as copy data, so a statement after the COPY was forwarded unchecked.
For example, COPY users (id) FROM STDIN; INSERT INTO users (email) VALUES ('alice@example.com') sent the plaintext INSERT to an encrypted column.

Changes

  • SqlParser parses a Query or Parse that has a COPY ... FROM STDIN one statement at a time, with each COPY ending at its ;. Other SQL is parsed as before, from one tokenization.
  • A COPY ... FROM (STDIN, file or program) into an encrypted column is refused with MappingError::CopyIntoEncryptedColumn, which names the column. A COPY with no column list fills every column.
  • With encrypted columns configured, a COPY into a table or column Proxy has not loaded, or into a table name more than one schema has, is refused with MappingError::CopyIntoUnknownTable. This is stricter than a failed type check, which is forwarded unless mapping errors are enabled. A table created earlier on the connection, or earlier in the same Query, is known. With no encryption config, every COPY is forwarded.
  • Proxy rewrites a Query that it encrypts and leaves out statements it does not type check, so the COPY would be dropped. A COPY in such a Query is refused with MappingError::CopyWithEncryptedStatement.
  • The extended protocol behaves the same way. 2.2's batch accounting (CIP-5015) already ignores the Sync sent during a copy, and a unit test now covers it.
  • The new errors are documented in docs/errors.md, and there is a CHANGELOG entry.

Tests

  • Unit tests in postgresql::tests::copy_from_stdin cover the simple and extended protocols:
    • a plaintext COPY is forwarded with its copy data;
    • a COPY into an encrypted column, an unknown table, or from a file is refused;
    • a COPY into only the plaintext columns of an encrypted table, or into a table created in the same Query, is forwarded;
    • statements around a COPY are not forwarded as plaintext.
  • The 2.2 test harness gains Connection::encrypted() and forwarded(), ported from fix(proxy): keep CIPHERSTASH settings in step with PostgreSQL transactions (2.2 backport, CIP-5073) #481 to ease the merge. Its test encryption service now returns stub ciphertexts, so a rewrite can be tested.
  • Parser unit tests cover statements after a COPY ... FROM STDIN.
  • Integration tests, which need ZeroKMS credentials, so CI runs them:
    • copy_from_stdin.rs uses tokio-postgres copy_in, which sends the extended protocol;
    • tests/python/tests/test_copy_from_stdin.py uses psycopg cursor.copy, which sends a simple Query.

Notes for review

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

@tobyhede tobyhede self-assigned this Oct 22, 2026

@freshtonic freshtonic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed head 671548fd, against release/2.2. The parser fix and the COPY FROM check are sound, and every mutation I tried made a test fail. CI passed on PostgreSQL 14 to 17. The Rust and Python COPY integration tests ran in CI and passed.

Fix in this PR, or create a linked follow-up before 2.2 ships

B1. COPY (INSERT/UPDATE … RETURNING …) TO STDOUT sends plaintext to an encrypted column.
check_copy_from returns early unless the COPY is to: false with a CopySource::Table (frontend.rs:989-999). Also, eql_mapper::requires_type_check does not include Statement::Copy, so Proxy never type checks the inner query. A probe test through Connection::encrypted() showed that Proxy forwards COPY (INSERT INTO users (email) VALUES ('alice@example.com') RETURNING id) TO STDOUT unchanged, with no error. COPY (UPDATE users SET email = '…' RETURNING id) TO STDOUT is the same. This PR does not cause the problem. But the CHANGELOG says that Proxy now refuses a COPY into an encrypted column, and this COPY writes to one. #484 found the same problem on main. Fix: when encrypted columns are configured, refuse a CopySource::Query whose body is an INSERT, UPDATE, DELETE or MERGE, or type check the inner query.

Should fix

S1. A COPY with a schema-qualified table name loses the schema, so a table on the same connection can hide the real table.
frontend.rs:1009 resolves only the last part of the name (table_name.0.last()), but schema_delta.rs keys overlays by the full ObjectName. So Proxy forwards CREATE TEMP TABLE users (id int, email text); COPY public.users FROM STDIN. This COPY has no column list, and it loads into public.users, which has the encrypted email. A probe test confirmed this. PostgreSQL makes it hard to exploit, because the copy text must parse as eql_v2_encrypted and pass the EQL CHECK constraint. So a leak needs crafted JSON, not an accidental CSV load.

The opposite case fails closed: Proxy refuses CREATE TABLE public.staging (…); COPY staging FROM STDIN as an unknown table. That disagrees with the docs, which say that Proxy knows a table created on the same connection.

Fix: when the name has a schema, require the overlay and the base schema to agree, or refuse the COPY.

S2. A merge with #481 can bring back the bypass.
#481 removes the loop that this PR uses at frontend.rs:507-519, which runs check_copy_from on the statements after a type-check failure. #481 returns Ok(None) there.

  • If the merge keeps #481's version, Proxy forwards SELECT * FROM unknown; COPY users FROM STDIN into an encrypted column.
  • a_copy_after_a_statement_that_fails_type_checking_is_still_checked catches this. Keep that test, and keep a pass over the later statements.
  • After #481, each statement is written again with Display, so CopyWithEncryptedStatement (frontend.rs:609-615) is no longer necessary. It is safe to keep, because it fails closed. If you remove it, add a test that shows the COPY survives the rewrite.
  • I checked that Display writes a COPY with empty values without a trailing ;\n\., and that WITH (FORMAT csv, DELIMITER ';', NULL '') and WITH CSV HEADER round-trip.

S3. The CHANGELOG and docs say that with no encryption config, Proxy forwards every COPY. That is not fully true.
Proxy still parses the Query, and it refuses as invalid SQL the COPY options that sqlparser 0.56 does not support: FROM STDIN WHERE id > 1, HEADER match, (ON_ERROR ignore). This fails closed, and it is not a regression, because the base branch refused every COPY … FROM STDIN without a ;. Please add a short note about it.

Nits

  • N1. CREATE TABLE staging (LIKE users) and CREATE TABLE staging AS SELECT * FROM users give an overlay with no columns. After either, Proxy forwards COPY staging FROM STDIN, but in PostgreSQL staging has eql_v2_encrypted columns. staging is not in the encrypt config, so INSERT behaves the same. It would be stricter to treat an overlay with no columns as unknown when the COPY has no column list.
  • N2. Proxy does not check a COPY again at Bind or Execute when it was prepared while UNSAFE_DISABLE_MAPPING was on and mapping was then turned back on (frontend.rs:870-879). All prepared statements work this way.
  • N3. In tests.rs, a_simple_query_copy_into_an_encrypted_column_is_refused has the comment "The only error" above errors_sent_to_client() == 0, after try_recv has already taken the error. The assertion is correct, but the comment makes it hard to understand.
  • COPY users TO STDOUT is forwarded and returns ciphertext that is not decrypted. This fails safe, but the docs do not say it. Please add a sentence.

What I checked and found correct

  • The splitter (parser.rs:115-170) splits tokens, not text. It handles correctly:
    • ; inside '…', E'\';', $$;$$, /* ; */ and -- ;
    • DELIMITER ';'
    • lowercase keywords and odd whitespace, and a leading comment
    • more than one COPY in one Query
    • a COPY at the end with no ;
    • COPY (SELECT 1 FROM stdin) TO STDOUT
  • Forwarding after a COPY. Proxy checks every statement after a COPY, and never forwards one as plaintext. When the parse succeeds, Proxy forwards the original bytes, so the split never changes what PostgreSQL gets.
  • The encrypted-column check:
    • A COPY with no column list is refused.
    • Column names follow the case rules for quoted and unquoted identifiers.
    • An unknown table is refused, and so is a table name that is in two schemas.
    • Views and foreign tables are refused as unknown.
    • FROM PROGRAM, or from a file, into an encrypted column is refused.
    • ALTER TABLE … ADD COLUMN keeps the existing EQL kinds.
    • The simple and the extended protocols both refuse.
  • The protocol after a refusal:
    • For a refused Query, Proxy sends ErrorResponse and ReadyForQuery, and nothing goes to the server, so there is no stray CopyInResponse.
    • For a refused Parse, Proxy sets error_state and drops all messages until Sync, including pipelined CopyData.
    • The Sync sent during a copy (CIP-5015) is covered.
  • Errors and docs. The three errors are in MappingError with doc links. The anchors in docs/errors.md match. The CHANGELOG has an entry.

What I ran

  • cargo test -p cipherstash-proxy --lib on 671548fd: 188 passed, including a temporary module of about 28 probe Queries. Clippy reports no problems.
  • 5 mutations, and each one made at least one test fail:
    • removing check_copy_from after a type-check failure
    • removing CopyWithEncryptedStatement
    • making contains_copy_from_stdin always false
    • treating a COPY with no column list as having no columns
    • forwarding a COPY into an unknown table
  • CI run 39724849485: copy_into_a_plaintext_table, copy_into_an_encrypted_column_is_refused and tests/test_copy_from_stdin.py passed.

Copy link
Copy Markdown
Contributor Author

Pushing one merge commit now: it merges release/2.2 (#481) and addresses B1, S1, S2, S3 and N1 from the review. It is a fast-forward with no history rewrite. A reply to each review item will follow once it is up.


Generated by Claude Code

tobyhede pushed a commit that referenced this pull request Oct 22, 2026
Brings in #481 (per-statement settings and refused-message rejection)
and keeps the COPY check on every path the merge touches.

Review fixes (CIP-5101):
- Refuse COPY of a query that writes (INSERT/UPDATE/DELETE, or a
  writing CTE) when encryption is configured, since Proxy does not
  encrypt the query of a COPY.
- Check COPY statements that follow a statement that failed type
  checking with mapping errors off.
- A schema-qualified COPY target checks both the edited and the loaded
  table, so a TEMP table cannot hide an encrypted one.
- A COPY into a table with no known columns (such as CTAS) is refused.
- Document COPY TO returning ciphertext and unsupported COPY options.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor Author

Review of commit 671548fd80c31bc767a29a6dd0f5d1c7418befc4:

The COPY handling introduces two correctness issues involving recovery after rejected queries and resolution of newly created schema-qualified tables.

Overall impact: medium. The change affects bulk-loading workflows through both PostgreSQL protocols; the identified defects affect connections recovering from mixed encrypted/COPY queries and loads into schema-qualified tables created on the same connection.

[P2] Reject mixed COPY queries before enqueueing execution state

packages/cipherstash-proxy/src/postgresql/frontend.rs:614

This error is returned after add_portal and set_execute have queued the rejected query. The local error path sends ReadyForQuery without removing those entries, and the backend reads the oldest queued portal. For example, with a valid keyset, reject INSERT INTO users (email) VALUES ('alice@example.com') RETURNING email; COPY users (id) FROM STDIN, then run SELECT 'hello': the next result is processed using the rejected INSERT's encrypted projection, causing incorrect type mapping/decryption and loss of the result. Perform this check before registering the portal/execution, or roll those entries back on rejection.

[P2] Preserve qualified names when resolving DDL overlays

packages/cipherstash-proxy/src/postgresql/frontend.rs:1009

Dropping the schema before consulting the resolver prevents it from finding same-connection DDL overlays, which SchemaWithEdits keys by the complete ObjectName. With encryption configured elsewhere, CREATE TABLE public.staging (id int); COPY public.staging FROM STDIN; now fails with CopyIntoUnknownTable: the overlay is stored under public.staging, but this lookup asks for staging, which is absent from the loaded schema. Resolve the complete name against overlays before falling back to the unqualified loaded-schema lookup.

Validation: read-only source review of the complete pinned diff (12 files, 676 additions, 16 deletions). Tests were not run.

Copy link
Copy Markdown
Contributor Author

One more fast-forward push, no rewrite: a commit that refuses a COPY into a table created with CREATE TABLE ... (LIKE ...) (the rest of N1).


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. Everything is addressed in 9fecfee (the merge with release/2.2, plus fixes) and b727fb3.

  • B1. When encryption is configured, Proxy now refuses a COPY (query) TO whose query writes. That covers INSERT, UPDATE or DELETE as the body, and a writing CTE at any depth. The new error is CopyOfQueryThatWrites (#mapping-copy-of-query-that-writes). A read-only COPY (SELECT …) TO is still forwarded, and so is any COPY when nothing is encrypted. Tests cover all four writing forms.
  • S1. If a statement fails type checking while mapping errors are off, Proxy still checks every later COPY in the same Query. Test: a_copy_after_a_statement_that_fails_type_checking_is_still_checked.
  • S2. I merged fix(proxy): keep CIPHERSTASH settings in step with PostgreSQL transactions (2.2 backport, CIP-5073) #481. check_copy runs inside the per-statement loop next to check_for_schema_change, and in parse_handler. With per-statement settings, a statement that has mapping disabled is skipped, the same as every other statement. The guard that refuses a COPY sent in the same query as an encrypted statement is still in place.
  • S3 and COPY TO. docs/errors.md now says that COPY … TO returns ciphertext that is not decrypted. It also says that options this parser does not support (WHERE, HEADER MATCH, ON_ERROR) are refused at parse.
  • N1.
    • A COPY with no column list into a table with no known columns (CTAS) is refused as unknown.
    • CREATE TABLE staging (LIKE users) turned out to be a separate case. sqltk-parser reads (LIKE users) as a column named LIKE of type users, so the overlay had one plaintext column and the CTAS rule did not catch it. PostgreSQL reserves LIKE, so an unquoted column of that name is always the clause. Any COPY into such a table, with or without a column list, is now refused (b727fb3).
    • INSERT into these tables still has the same gap. That is part of CIP-5102.
  • N2. I agree it applies to every prepared statement, not only COPY. I left it out of this PR.
  • N3. That test was rewritten around a refused() helper, so the comment is gone.

Also, a schema-qualified COPY now checks both the table created under that name and the loaded table, so a TEMP table of the same name cannot hide an encrypted one.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Pushing ed39baf as a fast-forward with no rewrite. It moves the check for a COPY mixed with an encrypted statement ahead of portal registration, and adds a regression test.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Both findings were against 671548f. Here is where each stands now:

  • [P2] Reject mixed COPY queries before enqueueing execution state. The guard now runs before add_portal and set_execute (ed39baf, fast-forward, no rewrite). Since the merge with fix(proxy): keep CIPHERSTASH settings in step with PostgreSQL transactions (2.2 backport, CIP-5073) #481, a refused Query is replaced by a message that PostgreSQL rejects. The ReadyForQuery for that rejection drains the queued execution, so I could not reproduce the stale portal on the current head. The new test the_query_after_a_refused_copy_with_an_encrypted_statement_is_not_decrypted runs your exact sequence. It checks that SELECT 'hello' reaches the client unchanged and that nothing is decrypted.
  • [P2] Preserve qualified names when resolving DDL overlays. This was fixed in 9fecfee. A schema-qualified COPY now resolves the full name against the overlays, and also resolves the last part against the loaded schema. It is refused if either one would fill an encrypted column. Tests:
    • a_schema_qualified_copy_into_a_table_created_with_that_name_is_forwarded covers CREATE TABLE public.staging (id int); COPY public.staging FROM STDIN.
    • a_schema_qualified_copy_is_not_hidden_by_a_table_of_the_same_name covers the TEMP-table case.

Generated by Claude Code

@cipherstash-bot cipherstash-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: 🟡 merge after changes

Change before merge: two paths let a false table overlay hide an encrypted table from the COPY check (see the comments on frontend.rs lines 554 and 1086). Also add two tests for refusal branches that no test covers (lines 1122 and 1486). The other comments can wait.

Other findings not posted as comments

  • Optional: writes() sits between produces_ready_for_query and its doc comment. So the line "Sync, Query and FunctionCall are each answered with exactly one ReadyForQuery" now documents writes(). Move that line back above fn produces_ready_for_query (frontend.rs around line 1479).
  • Optional: no test refuses a COPY into a table name that two schemas have. Suppose Schema::resolve_table in eql-mapper later returns the first match. Then a plaintext table could hide an encrypted one. Use a schema with two users tables, one plaintext and one encrypted. Expect CopyIntoUnknownTable.
  • Optional: no test forwards a COPY into a table that has a quoted "like" column. So no test checks the quote_style.is_none() check (frontend.rs around line 1104).
  • A test for the check_for_schema_change call in the fallback loop is not needed if the first comment is applied, because that fix removes the call.

The test code in comments 2, 3, 5 and 6 uses helpers from tests.rs: Connection::encrypted, query, refused and forwarded_unchanged. Each comment says which helpers it uses.

How this review was made
Agent Model Review type Result
claude claude-opus-5-5 test-gap 6 found, 4 posted
claude claude-opus-5-5 rust 3 found, 2 posted
codex gpt-5.6-terra test-gap 1 found, 1 posted
codex gpt-5.6-terra rust 1 found, 1 posted

Synthesis: claude-sonnet-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 0 posted finding(s) were raised by two or more models.

Plain language: claude-sonnet-5-5 read every comment as a new reader would. 4 comment(s) had a problem that stopped the reader acting; it rewrote 4. It also rewrote the review body.

Stack: not part of a stack.

Context loaded: the description, 0 linked issue(s) and 7 discussion entries.

Comment thread packages/cipherstash-proxy/src/postgresql/frontend.rs Outdated
Comment thread packages/cipherstash-proxy/src/postgresql/frontend.rs Outdated
Comment thread packages/cipherstash-proxy/src/postgresql/frontend.rs
Comment thread packages/cipherstash-proxy/src/postgresql/frontend.rs
Comment thread packages/cipherstash-proxy/src/postgresql/frontend.rs Outdated
Comment thread packages/cipherstash-proxy/src/postgresql/frontend.rs
Comment thread packages/cipherstash-proxy/src/postgresql/frontend.rs
Comment thread packages/cipherstash-proxy/src/postgresql/parser.rs

Copy link
Copy Markdown
Contributor Author

Pushing 8a9546a as a fast-forward with no rewrite. It addresses the latest review: every COPY now checks the loaded table too, nothing after a failed statement is recorded, and it adds the missing tests.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

The review's three findings not posted as comments are also in 8a9546a:

  • The doc comment for produces_ready_for_query is back above that function.
  • a_copy_into_a_table_name_two_schemas_have_is_refused uses a schema with a plaintext and an encrypted users table.
  • a_copy_into_a_table_with_a_quoted_like_column_is_forwarded covers the quote_style check.

Generated by Claude Code

@auxesis auxesis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobyhede approved.

Thanks for addressing this bug and the review follow ups.

…into encrypted columns (CIP-5101)

The SQL parser expects the copy data of a COPY ... FROM STDIN to follow it
in the SQL, as in a psql script. It refused the statement without a `;`
after STDIN, so psql's \copy and driver COPY APIs could not load any table
through Proxy. With a `;`, it read the rest of the Query as copy data, so a
statement after the COPY was forwarded unchecked, including a plaintext
INSERT into an encrypted column.

A Query or Parse with a COPY ... FROM STDIN is now parsed one statement at a
time, with each COPY ending at its `;`. A COPY ... FROM into an encrypted
column is refused with MappingError::CopyIntoEncryptedColumn, as Proxy does
not encrypt copy data. A table or column Proxy does not know is handled as a
statement that fails type checking. A Query that Proxy rewrites to encrypt
cannot keep a COPY, so a COPY in such a Query is refused with
MappingError::CopyWithEncryptedStatement.

fix(proxy): refuse a COPY into a table Proxy cannot check (CIP-5101)

A COPY into a table or column missing from the loaded schema, or into a
table name more than one schema has, was forwarded unless mapping errors
were enabled, so it could send plaintext to an encrypted column. With
encrypted columns configured it is now refused with
MappingError::CopyIntoUnknownTable. A table created earlier on the
connection, or earlier in the same Query, is known.

A COPY after a statement that fails type checking in the same Query is
now checked too, as the Query is forwarded unchanged.

fix(proxy): name a COPY's encrypted column without quotes (CIP-5101)

Proxy loads column names from the database as quoted identifiers, so the
CopyIntoEncryptedColumn error named the column '"encrypted_text"'. The
test harness now quotes column names as the schema loader does.

fix(proxy): refuse a COPY into a table created with LIKE (CIP-5101)

The parser reads the LIKE clause of CREATE TABLE t (LIKE u) as a column
named LIKE, so Proxy does not know the columns the table copies and
forwarded a COPY into it. PostgreSQL reserves LIKE, so an unquoted column
of that name is always the clause, and such a table is now unknown.

fix(proxy): refuse a mixed COPY query before adding its portal (CIP-5101)

A Query with a COPY and an encrypted statement was refused after its
portal and execution were queued. Since the merge with #481 the forwarded
rejection drains them, so the next Query was not affected, but the check
now runs first, as the other refusals do. A test pins that the next
Query is not decrypted.

fix(proxy): check the loaded table for every COPY target (CIP-5101)

Proxy records a table created on the connection before PostgreSQL runs
the CREATE, which may then not create it (IF NOT EXISTS, a refused
Query, a rollback). A COPY trusted that record for a name without a
schema, so a plaintext record could hide an encrypted table. A COPY is
now refused if either the recorded or the loaded table has an encrypted
column it fills. After a statement that fails type checking, the CREATE
statements after it are no longer recorded, as PostgreSQL does not run
them.

Also adds tests for an unknown column of a known table, a parenthesized
writing query, a name two schemas have, a quoted "like" column, COPY with
mapping disabled, COPY of a table TO STDOUT, and two COPY FROM STDIN
statements in one Query.
@tobyhede
tobyhede force-pushed the claude/project-thread-7897nb branch from 8a9546a to cab9b97 Compare October 23, 2026 08:11

This branch has not been deployed

No deployments
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.

4 participants