diff --git a/CHANGELOG.md b/CHANGELOG.md index 09fc50822..fb82b7527 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,14 +6,28 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Added + +- **Multi-step JSON paths on encrypted columns, everywhere**: `col -> 'a' -> 'b'` now works wherever a single field access does — in the select list, in an ordering comparison (`<`, `<=`, `>`, `>=`), and in the `->`, `->>` and `jsonb_path_query_first` spellings mixed freely, to any depth, with each step written as a literal or a placeholder. Previously only exact equality accepted a multi-step path and everything else was rejected. A chain is a single path into a single document, so it is now rewritten to one field access keyed on the whole composed path (`$.a.b`) instead of the nested accessors that would search an already-extracted entry and return NULL. Exact equality continues to fold the path and the value into one needle, which remains the stronger match. + + Two shapes are still rejected rather than answered. A path split across a subquery, CTE or view (`SELECT a -> 'foo' FROM (SELECT col -> 'bar' AS a FROM t) s`) cannot be composed at all — the extracted value does not carry the path that produced it, and the document it came from is not in scope — so write the whole path in one expression. A placeholder step in front of a *literal* final step (`col -> $1 -> 'b'`) cannot be composed either, because a literal is encrypted before any parameter is bound; parameterise the final step too (`col -> $1 -> $2`), or write the whole path as literals. + ### Fixed +- **One placeholder used as the JSON selector of two different paths**: `col -> 'a' -> $1 = $2` alongside `col -> 'b' -> $1 = $3` silently kept only one of the two paths, so one of the predicates was matched against the wrong field. The path a selector placeholder keys is recorded against the parameter it arrives in — at Bind time the parameter number is all Proxy has — so two different paths for one parameter cannot both be honoured. This is now reported as an error naming the parameter, rather than answered from whichever path was recorded last. + - **`UPDATE … SET … FROM` with same-named columns**: an `UPDATE` was rejected as ambiguous when a table in the `FROM` clause had a column with the same name as the column being assigned. The assignment now always refers to the table being updated, so these statements work and the assigned value gets the target column's type — encrypted or not. - **Encrypted values as row counts are rejected**: an encrypted column used in `LIMIT`, `OFFSET`, or `FETCH` (for example `LIMIT enc_col`) is now rejected with a type error instead of being forwarded to the database. - **Statements Proxy cannot type-check fail with a clear error**: a statement Proxy admits for type checking but has no support for is now rejected immediately with an error naming the statement, instead of surfacing later as an opaque resolution error. No currently-supported statement is affected. +### Security + +- **Chained JSON field accessors sent the intermediate field name to the database in plaintext**: `WHERE col -> 'a' -> 'b' = $1` on an encrypted JSON column emitted `eql_v3.jsonb_contains(col -> 'a', …)`, so the field name `a` appeared in the statement text PostgreSQL received (and in its logs), and native `jsonb ->` was applied to the encrypted payload — which also made the predicate match nothing. A chain is now treated as the single path it is: `$.a.b` of the whole document, folded into the one encrypted needle and matched against the bare column. Chains of any depth are supported, in the `->`, `->>` and `jsonb_path_query_first` spellings, with `=` and `<>`, and with each step written as a literal or a placeholder. + +- **A NULL JSON selector forwarded the compared value to the database in plaintext**: `WHERE col -> $1 = $2` with `$1` bound NULL builds no needle, so `$2` was never encrypted — and it was then sent to PostgreSQL exactly as the client bound it, putting the plaintext comparand on the wire and into the server log when the column's domain CHECK rejected it. An encrypted operand that produced no ciphertext is now bound NULL, which is also what the SQL means: a comparison against NULL is NULL, so the query returns no rows. + ## [3.0.0] - 2026-08-05 ### Changed diff --git a/packages/cipherstash-proxy-integration/src/select/jsonb_accessor_chain.rs b/packages/cipherstash-proxy-integration/src/select/jsonb_accessor_chain.rs new file mode 100644 index 000000000..96c4cf87a --- /dev/null +++ b/packages/cipherstash-proxy-integration/src/select/jsonb_accessor_chain.rs @@ -0,0 +1,342 @@ +//! Multi-step encrypted JSON accessor chains, everywhere — not only under an +//! exact equality. +//! +//! An encrypted JSON column is a SteVec document: an `sv` array of entries, each +//! keyed by a selector MAC. `eql_v3."->"(doc, sel)` searches that array, and what +//! it returns is one ENTRY, which has no `sv` of its own. So a chain cannot be two +//! hops — the outer call would search an entry, find nothing, and return NULL. +//! +//! The correct emission is ONE accessor carrying the composed path: +//! `j -> 'a' -> 'b'` becomes `eql_v3."->"(j, )`. These tests +//! read a nested field back through Proxy and assert the **value**, because the +//! failure being guarded against is not an error — it is a query that runs +//! perfectly and answers NULL. + +#[cfg(test)] +mod tests { + use crate::common::{clear, connect_with_tls, execute_query, random_id, trace, PROXY}; + use serde_json::Value; + + /// A document with a field two levels down, and a number there too so that + /// ordering has something to compare. + async fn insert_nested() -> i64 { + let id = random_id(); + let doc = serde_json::json!({ + "nested": { "string": "world", "number": 42 }, + "string": "hello", + }); + + execute_query( + "INSERT INTO encrypted (id, encrypted_jsonb) VALUES ($1, $2)", + &[&id, &doc], + ) + .await; + + id + } + + /// The single value `sql` projects, decrypted by Proxy. + async fn project(sql: &str, params: &[&(dyn tokio_postgres::types::ToSql + Sync)]) -> Value { + let client = connect_with_tls(*PROXY).await; + + let rows = client + .query(sql, params) + .await + .unwrap_or_else(|e| panic!("`{sql}` should execute, got: {e}")); + + assert_eq!(rows.len(), 1, "expected exactly one row from `{sql}`"); + + rows[0].get(0) + } + + /// Projecting a two-level field returns the field, not NULL. + /// + /// This is the whole point. The chain used to be refused at type check, which + /// was better than the alternative it replaced — emitting a second + /// entry-scoped accessor over an entry and answering NULL with no error at + /// all. Now it is composed into one path and actually works. + #[tokio::test] + async fn two_level_projection_returns_the_field() { + trace(); + clear().await; + insert_nested().await; + + assert_eq!( + project( + "SELECT encrypted_jsonb -> 'nested' -> 'string' FROM encrypted", + &[] + ) + .await, + Value::String("world".to_string()) + ); + } + + /// Every spelling of the same path reads the same field. + /// + /// Brackets carry no meaning, `->>` differs from `->` only in the result type, + /// and `jsonb_path_query_first` is the function spelling of a step. All four + /// decompose to the same root and the same composed path, so all four must + /// return the same value — a walker that missed one would compose a short path + /// and read a different field. + #[tokio::test] + async fn every_spelling_of_a_chain_reads_the_same_field() { + trace(); + clear().await; + insert_nested().await; + + for sql in [ + "SELECT encrypted_jsonb -> 'nested' -> 'string' FROM encrypted", + "SELECT (encrypted_jsonb -> 'nested') -> 'string' FROM encrypted", + "SELECT encrypted_jsonb -> 'nested' ->> 'string' FROM encrypted", + "SELECT jsonb_path_query_first(encrypted_jsonb, '$.nested') -> 'string' \ + FROM encrypted", + ] { + assert_eq!( + project(sql, &[]).await, + Value::String("world".to_string()), + "unexpected value for `{sql}`" + ); + } + } + + /// A chain whose outermost step is a placeholder resolves at Bind. + /// + /// The literal step is known at Parse time and the param step is not, so the + /// composed path can only be built once the param is bound — which is why this + /// needs a record carried through to the proxy rather than a rewrite alone. + /// The surviving operand is the param, so the whole path resolves together. + #[tokio::test] + async fn a_param_step_in_a_projected_chain_resolves_at_bind() { + trace(); + clear().await; + insert_nested().await; + + assert_eq!( + project( + "SELECT encrypted_jsonb -> 'nested' -> $1 FROM encrypted", + &[&"string"] + ) + .await, + Value::String("world".to_string()) + ); + + // Both steps bound: the composed path is entirely a Bind-time value. + assert_eq!( + project( + "SELECT encrypted_jsonb -> $1 -> $2 FROM encrypted", + &[&"nested", &"string"] + ) + .await, + Value::String("world".to_string()) + ); + } + + /// A placeholder step in front of a LITERAL outermost selector is refused, + /// not answered from a short path. + /// + /// The operand that survives the collapse is the outermost selector, and a + /// literal is encrypted at Parse time — before any param is bound. So the path + /// `$.<$1>.string` cannot be composed when it is needed. The mirror image + /// (`-> 'nested' -> $1`) works, because there the surviving operand is the + /// param and the whole path resolves together at Bind. + /// + /// This is the same limitation the fused equality has for `col -> $1 = 'value'` + /// and for the same reason. Composing only what is known would key `$.string` + /// and read the wrong field, silently — so refusing is the only safe answer. + #[tokio::test] + async fn a_param_step_before_a_literal_selector_is_refused() { + trace(); + clear().await; + insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + let result = client + .query( + "SELECT encrypted_jsonb -> $1 -> 'string' FROM encrypted", + &[&"nested"], + ) + .await; + + match result { + Err(_) => {} + Ok(rows) => { + // If it is not refused it must at least not have answered from a + // truncated path: `$.string` holds "hello", which is the wrong + // field and the failure this asserts against. + for row in rows { + let value: Option = row.get(0); + assert_ne!( + value, + Some(Value::String("hello".to_string())), + "a truncated path answered the wrong field" + ); + } + } + } + } + + /// A chain keys the WHOLE path, so a prefix of it selects nothing. + /// + /// `$.string` holds "hello" and `$.nested.string` holds "world". If the + /// composition dropped the inner step the chain would read `$.string` and + /// answer "hello" — a silently wrong answer rather than an error, which is + /// exactly the failure mode this guards. + #[tokio::test] + async fn a_chain_reads_the_composed_path_not_a_prefix_of_it() { + trace(); + clear().await; + insert_nested().await; + + let nested = project( + "SELECT encrypted_jsonb -> 'nested' -> 'string' FROM encrypted", + &[], + ) + .await; + let top = project("SELECT encrypted_jsonb -> 'string' FROM encrypted", &[]).await; + + assert_eq!(nested, Value::String("world".to_string())); + assert_eq!(top, Value::String("hello".to_string())); + assert_ne!( + nested, top, + "a chain must not collapse to its outermost step alone" + ); + } + + /// A path the document does not have selects nothing, and says so as NULL. + #[tokio::test] + async fn a_chain_selecting_a_missing_path_is_null() { + trace(); + clear().await; + insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + let rows = client + .query( + "SELECT encrypted_jsonb -> 'nested' -> 'absent' FROM encrypted", + &[], + ) + .await + .unwrap(); + + let value: Option = rows[0].get(0); + assert_eq!(value, None); + } + + /// Ordering over a two-level path compares the field at that path. + /// + /// The accessor SURVIVES here — the comparison wraps it in `eql_v3.ord_term` + /// rather than absorbing it the way equality does — so this exercises the + /// collapsed accessor in a predicate rather than a projection. A chain that + /// stayed two hops would compare NULL and match nothing at all, which reads as + /// "no rows" rather than as a failure. + #[tokio::test] + async fn ordering_on_a_two_level_path_compares_that_field() { + trace(); + clear().await; + let id = insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + // `$.nested.number` is 42. + for (sql, expected) in [ + ( + "SELECT id FROM encrypted WHERE encrypted_jsonb -> 'nested' -> 'number' < $1", + vec![id], + ), + ( + "SELECT id FROM encrypted WHERE encrypted_jsonb -> 'nested' -> 'number' >= $1", + vec![], + ), + ] { + let rows = client + .query(sql, &[&Value::from(100)]) + .await + .unwrap_or_else(|e| panic!("`{sql}` should execute, got: {e}")); + + let actual: Vec = rows.iter().map(|r| r.get("id")).collect(); + assert_eq!(actual, expected, "unexpected rows for `{sql}`"); + } + + // The boundary, to show the comparison is against 42 and not against + // whatever a NULL comparison would yield. + let rows = client + .query( + "SELECT id FROM encrypted WHERE encrypted_jsonb -> 'nested' -> 'number' >= $1", + &[&Value::from(42)], + ) + .await + .unwrap(); + + let actual: Vec = rows.iter().map(|r| r.get("id")).collect(); + assert_eq!(actual, vec![id], "42 >= 42 must match"); + } + + /// Equality over a chain must keep FUSING, not degrade to an accessor plus a + /// comparison. + /// + /// The fused needle MACs the path and the value together and its presence in + /// the stored `sv` is the match — strictly stronger than extracting a field and + /// then comparing it. This is the shape that already worked; it must go on + /// working unchanged now that chains collapse everywhere else. + #[tokio::test] + async fn equality_over_a_chain_still_matches() { + trace(); + clear().await; + let id = insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + let rows = client + .query( + "SELECT id FROM encrypted WHERE encrypted_jsonb -> 'nested' -> 'string' = $1", + &[&Value::String("world".to_string())], + ) + .await + .unwrap(); + + let actual: Vec = rows.iter().map(|r| r.get("id")).collect(); + assert_eq!(actual, vec![id]); + } + + /// A chain split across a subquery boundary stays refused. + /// + /// This one is impossible, not unimplemented. `EqlTerm::JsonExtracted` does not + /// carry the path that produced it, and the root column is not even in scope in + /// the outer query — so there is nothing to root a composed path at without + /// rewriting the subquery's projection. + /// + /// What the client sees depends on `mapping_errors_enabled`; what is pinned + /// here is that Proxy never rewrites it into an accessor over an entry. The + /// type-check refusal itself is pinned by + /// `eql_mapper::test::json_operation_on_an_extracted_value_is_refused`. + #[tokio::test] + async fn a_chain_split_across_a_subquery_is_not_rewritten() { + trace(); + clear().await; + insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + let result = client + .query( + "SELECT a -> 'foo' FROM \ + (SELECT encrypted_jsonb -> 'nested' AS a FROM encrypted) s", + &[], + ) + .await; + + // Refused outright, or passed through unmapped — but never answered with a + // value, which would mean a path was composed that cannot be. + if let Ok(rows) = result { + for row in rows { + let value: Option = row.get(0); + assert_eq!( + value, None, + "a chain rooted at an extracted entry must not resolve to a value" + ); + } + } + } +} diff --git a/packages/cipherstash-proxy-integration/src/select/jsonb_fusion_gaps.rs b/packages/cipherstash-proxy-integration/src/select/jsonb_fusion_gaps.rs index 3130a48ba..0c1fa5e4b 100644 --- a/packages/cipherstash-proxy-integration/src/select/jsonb_fusion_gaps.rs +++ b/packages/cipherstash-proxy-integration/src/select/jsonb_fusion_gaps.rs @@ -1,21 +1,23 @@ -//! Shapes where JSON value-selector fusion sends plaintext to the database. +//! Shapes where JSON value-selector fusion used to send plaintext to the +//! database. //! //! `col -> 'field' = value` is rewritten by fusing the field and the value into //! a single encrypted needle matched by containment, so neither half is ever -//! visible on its own. The two shapes below reach that rewrite by routes it does -//! not handle, and in each case something the client wrote in plaintext is -//! forwarded to PostgreSQL. +//! visible on its own. The two shapes below reach that rewrite by routes it did +//! not handle, and in each case something the client wrote in plaintext was +//! forwarded to PostgreSQL (CIP-3682). //! -//! # These tests are ignored, not deleted -//! -//! Each asserts the behaviour the shape must have. They fail today; un-ignoring -//! one is the acceptance test for its fix. Rejecting the shape at type-check -//! time is an equally acceptable outcome — a clear error is not a leak — in -//! which case the test should be rewritten to assert the error. +//! Both are fixed. A chained accessor is now one path (`$.nested.string`) +//! rooted at the bare column, and a fusion that cannot build a needle binds +//! NULL instead of the client's bytes. The tests assert the behaviour AND, by +//! reading the stored row on a direct connection that bypasses Proxy, that +//! nothing the client wrote in the clear is visible to the database. #[cfg(test)] mod tests { - use crate::common::{clear, connect_with_tls, execute_query, random_id, trace, PROXY}; + use crate::common::{ + clear, connect_with_tls, execute_query, get_database_port, random_id, trace, PROXY, + }; use serde_json::Value; async fn insert_nested() -> i64 { @@ -34,24 +36,37 @@ mod tests { id } - /// A chained accessor must not put the intermediate selector in the SQL, nor - /// run native `->` on the encrypted payload. - /// - /// Confirmed emitted SQL: + /// The stored payload, read on a connection straight to PostgreSQL so that + /// Proxy never gets to decrypt it. This is what the database actually holds. /// - /// ```text - /// eql_v3.jsonb_contains(encrypted_jsonb -> 'nested', '{…}') - /// ``` + /// The port comes from `get_database_port()` — the database Proxy is backed + /// by — not from `PG_PORT`. Under the TLS suite Proxy sits in front of + /// `postgres-tls` on 5617 while `PG_PORT` is 5532, so a hardcoded `PG_PORT` + /// reads a different, empty database and finds no row at all. + async fn stored_payload(id: i64) -> String { + let client = connect_with_tls(get_database_port()).await; + + let rows = client + .query( + "SELECT encrypted_jsonb::text AS payload FROM encrypted WHERE id = $1", + &[&id], + ) + .await + .unwrap(); + + rows[0].get("payload") + } + + /// A chained accessor selects one path of one document, and must not put any + /// step of that path in the SQL, nor run native `->` on the encrypted + /// payload. /// - /// The container is cloned from the *original* AST, so the inner - /// `-> 'nested'` survives untouched: the plaintext field name `'nested'` - /// ships in the statement text, and native jsonb `->` is applied to the - /// encrypted payload — which also makes the predicate match nothing. + /// It used to emit `eql_v3.jsonb_contains(encrypted_jsonb -> 'nested', …)`: + /// the container was cloned from the original AST, so the inner + /// `-> 'nested'` survived untouched. The plaintext field name shipped in the + /// statement text, and native jsonb `->` was applied to the encrypted + /// payload — which also made the predicate match nothing. #[tokio::test] - #[ignore = "Chained JSON accessor (col -> 'a' -> 'b' = value) clones the container from the \ - original AST, leaking the plaintext selector 'a' into the SQL text and running \ - native jsonb -> on the encrypted payload. Returns 0 rows as well as leaking. See \ - rewrite_json_value_selector_eq.rs."] async fn chained_accessor_does_not_leak_the_selector() { trace(); clear().await; @@ -67,23 +82,107 @@ mod tests { let actual: Vec = rows.iter().map(|r| r.get("id")).collect(); assert_eq!(vec![id], actual); + + // Nothing the client wrote is visible to the database: not the field + // names it traversed, not the value it compared. + let payload = stored_payload(id).await; + for plaintext in ["nested", "string", "world", "hello"] { + assert!( + !payload.contains(plaintext), + "the stored payload leaks `{plaintext}`: {payload}" + ); + } + } + + /// The same chain written with placeholder steps: every step is dropped from + /// the statement and folded into the needle instead. + #[tokio::test] + async fn chained_accessor_with_param_selectors_matches() { + trace(); + clear().await; + let id = insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + let sql = "SELECT id FROM encrypted WHERE encrypted_jsonb -> $1 -> $2 = $3"; + let rows = client + .query( + sql, + &[&"nested", &"string", &Value::String("world".to_string())], + ) + .await + .expect("a chained accessor with placeholder selectors should be supported"); + + let actual: Vec = rows.iter().map(|r| r.get("id")).collect(); + assert_eq!(vec![id], actual); + } + + /// A chain that selects a path the document does not have matches nothing — + /// the needle is keyed on the whole path, so a wrong step cannot match a + /// right one. + #[tokio::test] + async fn chained_accessor_with_a_wrong_path_matches_nothing() { + trace(); + clear().await; + insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + // `$.string` holds "hello" and `$.nested.string` holds "world"; neither + // is `$.nested.hello`, and the value belongs to a different path. + let sql = "SELECT id FROM encrypted WHERE encrypted_jsonb -> 'nested' -> 'string' = $1"; + let rows = client + .query(sql, &[&Value::String("hello".to_string())]) + .await + .unwrap(); + + assert!( + rows.is_empty(), + "a value stored at another path must not match this one" + ); + } + + /// `<>` on a chain is the same containment, negated. + #[tokio::test] + async fn chained_accessor_not_eq_excludes_the_match() { + trace(); + clear().await; + let id = insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + let sql = "SELECT id FROM encrypted WHERE encrypted_jsonb -> 'nested' -> 'string' <> $1"; + let rows = client + .query(sql, &[&Value::String("world".to_string())]) + .await + .unwrap(); + + assert!( + rows.is_empty(), + "the row whose `$.nested.string` IS `world` must be excluded, got {} row(s)", + rows.len() + ); + + let rows = client + .query(sql, &[&Value::String("elsewhere".to_string())]) + .await + .unwrap(); + + let actual: Vec = rows.iter().map(|r| r.get("id")).collect(); + assert_eq!(vec![id], actual); } /// A NULL selector must not forward the *value* operand in plaintext. /// /// With the path bound NULL there is no needle to build, so encryption is - /// skipped — but the rebuild path forwards the value's raw client bytes, so - /// the comparand crosses the wire and can land in the server log when the - /// domain CHECK rejects it. Confirmed: PostgreSQL received the plaintext and - /// failed with `cannot call jsonb_each on a non-object`. + /// skipped — and the rebuild path used to forward the value's raw client + /// bytes, so the comparand crossed the wire and could land in the server log + /// when the domain CHECK rejected it. Confirmed: PostgreSQL received the + /// plaintext and failed with `cannot call jsonb_each on a non-object`. /// /// `col -> NULL = x` is NULL in SQL, so the correct result is simply no /// rows. #[tokio::test] - #[ignore = "A NULL selector param forwards the VALUE operand to the database in plaintext: \ - json_value_selector_plaintext yields nothing, encryption is skipped, and \ - bind.rs's rebuild path passes the client's raw bytes through. Should bind NULL \ - and return no rows."] async fn null_selector_param_does_not_forward_plaintext() { trace(); clear().await; @@ -104,4 +203,46 @@ mod tests { "col -> NULL = x is NULL in SQL, so no rows should match" ); } + + /// A NULL step anywhere in a chain is the same: no needle, no rows, and + /// nothing of the client's forwarded. + #[tokio::test] + async fn null_step_in_a_chain_does_not_forward_plaintext() { + trace(); + clear().await; + insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + let selector: Option = None; + let sql = "SELECT id FROM encrypted WHERE encrypted_jsonb -> 'nested' -> $1 = $2"; + + let rows = client + .query(sql, &[&selector, &Value::String("world".to_string())]) + .await + .expect("a NULL step should compare as NULL, not send the value in plaintext"); + + assert!(rows.is_empty(), "a NULL step matches nothing"); + } + + /// A NULL *value* is the mirror case: nothing to encrypt, and nothing of the + /// client's to forward. + #[tokio::test] + async fn null_value_param_does_not_forward_plaintext() { + trace(); + clear().await; + insert_nested().await; + + let client = connect_with_tls(*PROXY).await; + + let value: Option = None; + let sql = "SELECT id FROM encrypted WHERE encrypted_jsonb -> $1 = $2"; + + let rows = client + .query(sql, &[&"nested", &value]) + .await + .expect("a NULL value should compare as NULL"); + + assert!(rows.is_empty(), "col -> 'nested' = NULL matches nothing"); + } } diff --git a/packages/cipherstash-proxy-integration/src/select/mod.rs b/packages/cipherstash-proxy-integration/src/select/mod.rs index 75e53782e..73ea063a1 100644 --- a/packages/cipherstash-proxy-integration/src/select/mod.rs +++ b/packages/cipherstash-proxy-integration/src/select/mod.rs @@ -1,5 +1,6 @@ mod distinct_order_by; mod group_by; +mod jsonb_accessor_chain; mod jsonb_array_elements; mod jsonb_array_length; mod jsonb_contained_by; diff --git a/packages/cipherstash-proxy/src/error.rs b/packages/cipherstash-proxy/src/error.rs index fb3982b4f..d418fa600 100644 --- a/packages/cipherstash-proxy/src/error.rs +++ b/packages/cipherstash-proxy/src/error.rs @@ -254,6 +254,18 @@ pub enum EncryptError { #[error("InvalidIndexTerm")] InvalidIndexTerm, + /// `EqlTermVariant::JsonExtracted` types the RESULT of an encrypted JSON + /// extraction, which is read back and decrypted — it is never a plaintext + /// operand on its way to the database, so it should never reach the encrypt + /// path. + /// + /// Refused rather than defaulted to `EqlOperation::Store`: storing would + /// encrypt a value with a payload shape the query does not expect and + /// silently return the wrong rows, which is the class of bug this term + /// exists to eliminate. + #[error("Internal: an extracted encrypted JSON value reached the encrypt path")] + JsonExtractedIsNotAnOperand, + /// EQL v3 orders encrypted jsonb entries by the CLLW-OPE (`op`) term and has /// no representation for CLLW-ORE (`oc`), so a column configured for /// Standard-mode ste_vec cannot be encrypted. The column has to be diff --git a/packages/cipherstash-proxy/src/postgresql/context/statement.rs b/packages/cipherstash-proxy/src/postgresql/context/statement.rs index 77053a59b..a07699651 100644 --- a/packages/cipherstash-proxy/src/postgresql/context/statement.rs +++ b/packages/cipherstash-proxy/src/postgresql/context/statement.rs @@ -1,19 +1,31 @@ use super::Column; -use eql_mapper::{JsonSelectorSource, ParamPlan}; +use eql_mapper::{JsonSelectorSegment, ParamPlan}; -/// Where the path half of a fused JSON value selector comes from. +/// Where one step of the path half of a fused JSON value selector comes from. /// -/// The proxy's copy of [`eql_mapper::JsonSelectorSource`], with param numbers +/// The proxy's copy of [`eql_mapper::JsonSelectorSegment`], with param numbers /// converted to 0-based bind indexes. #[derive(Debug, Clone, PartialEq, Eq)] -pub enum JsonSelectorPath { - /// A literal path in the SQL, known at Parse time. +pub enum JsonSelectorStep { + /// A literal selector in the SQL, known at Parse time. Literal(String), - /// A placeholder path, arriving in this (0-based) input param. + /// A placeholder selector, arriving in this (0-based) input param. Param(usize), } +/// The path half of a fused JSON value selector: the steps of the accessor +/// chain, outermost last. +/// +/// A chain (`col -> 'a' -> 'b'`) is one path into one document — the payload +/// between the steps is encrypted, so there is nothing for the database to +/// traverse. The steps are resolved and composed into a single eJSONPath at +/// encryption time, once any placeholder step is bound. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct JsonSelectorPath { + pub steps: Vec, +} + /// How the value bound to one output param is built from the input params. #[derive(Debug, Clone, PartialEq, Eq)] pub enum OutputParamSource { @@ -25,16 +37,27 @@ pub enum OutputParamSource { path: JsonSelectorPath, value: usize, }, + + /// The composed path of a collapsed multi-step accessor chain. This output's + /// whole value is that path; the param it occupies (`selector`) supplied only + /// the outermost step of it. + JsonAccessorPath { + path: JsonSelectorPath, + selector: usize, + }, } impl OutputParamSource { /// The input param this output is built *around* — the one whose wire /// format and (for a passthrough) whose bytes it inherits. For a fusion - /// that is the value operand; the path only contributes to the needle. + /// that is the value operand; the path only contributes to the needle. For a + /// collapsed chain it is the selector operand, the one step of the path that + /// was written where this output sits. pub fn primary_input(&self) -> usize { match self { OutputParamSource::Input(idx) => *idx, OutputParamSource::JsonValueSelector { value, .. } => *value, + OutputParamSource::JsonAccessorPath { selector, .. } => *selector, } } } @@ -139,17 +162,16 @@ pub fn output_params_from_plan( } eql_mapper::OutputParamSource::JsonValueSelector { path, value } => { OutputParamSource::JsonValueSelector { - path: match path { - JsonSelectorSource::Literal(path) => { - JsonSelectorPath::Literal(path.to_owned()) - } - JsonSelectorSource::Param(param) => { - JsonSelectorPath::Param(to_index(param.0)) - } - }, + path: to_selector_path(path), value: to_index(value.0), } } + eql_mapper::OutputParamSource::JsonAccessorPath { path, selector } => { + OutputParamSource::JsonAccessorPath { + path: to_selector_path(path), + selector: to_index(selector.0), + } + } }, }) .collect() @@ -159,3 +181,19 @@ pub fn output_params_from_plan( fn to_index(param: u16) -> usize { param.saturating_sub(1) as usize } + +/// Converts a mapper selector path to the proxy's 0-based form. +fn to_selector_path(path: &eql_mapper::JsonSelectorSource) -> JsonSelectorPath { + JsonSelectorPath { + steps: path + .segments() + .iter() + .map(|segment| match segment { + JsonSelectorSegment::Literal(selector) => { + JsonSelectorStep::Literal(selector.to_owned()) + } + JsonSelectorSegment::Param(param) => JsonSelectorStep::Param(to_index(param.0)), + }) + .collect(), + } +} diff --git a/packages/cipherstash-proxy/src/postgresql/data/from_sql.rs b/packages/cipherstash-proxy/src/postgresql/data/from_sql.rs index 1deff1b4d..aafd74679 100644 --- a/packages/cipherstash-proxy/src/postgresql/data/from_sql.rs +++ b/packages/cipherstash-proxy/src/postgresql/data/from_sql.rs @@ -117,28 +117,65 @@ pub fn literal_from_sql( /// `$[*].b`. Re-rooting those would produce `$.$[0]` and friends — a selector /// that matches nothing rather than erroring. pub fn json_selector_path(val: &str) -> String { - if val.starts_with('$') { - val.to_string() - } else { - format!("$.{val}") + compose_json_selector_path(std::slice::from_ref(&val)) +} + +/// Composes the steps of an accessor chain into one eJSONPath rooted at `$`. +/// +/// `col -> 'a' -> 'b'` is the single path `$.a.b` of the root document, not two +/// hops: the intermediate value is an encrypted payload the database cannot +/// traverse, so the whole chain has to be keyed into one selector. +/// +/// Every step is normalised the way [`json_selector_path`] normalises a lone +/// one, so the spellings mix freely: `jsonb_path_query_first(col, '$.a') -> 'b'` +/// composes to `$.a.b`, and a subscript step keeps its bracket +/// (`$.a[0]`) rather than gaining a spurious dot. +pub fn compose_json_selector_path(segments: &[&str]) -> String { + let mut path = String::from("$"); + + for segment in segments { + // A step written as a path of its own is already rooted, and its root + // is this path so far — drop the `$` and splice the remainder on. + let (rooted, rest) = match segment.strip_prefix('$') { + Some(rest) => (true, rest), + None => (false, *segment), + }; + + // A bare `$` selects the document itself and adds no step. + if rooted && rest.is_empty() { + continue; + } + + if !rest.starts_with('.') && !rest.starts_with('[') { + path.push('.'); + } + + path.push_str(rest); } + + path } /// Builds the composition input for a fused JSON value selector: /// `{"path": , "value": }`. /// -/// This is the one place two SQL operands become one encrypted operand. -/// `QueryOp::SteVecValueSelector` MACs the path and the canonicalised value -/// together into a single selector; its presence in the stored `sv` is the -/// equality match. The client applies the column's term filters (e.g. downcase) -/// to `value` as part of that, so case-insensitive columns work unchanged here. +/// This is the one place the operands of a JSON field equality become one +/// encrypted operand. `QueryOp::SteVecValueSelector` MACs the path and the +/// canonicalised value together into a single selector; its presence in the +/// stored `sv` is the equality match. The client applies the column's term +/// filters (e.g. downcase) to `value` as part of that, so case-insensitive +/// columns work unchanged here. +/// +/// `path` arrives as the steps of the accessor chain, already resolved to text; +/// they are composed into one eJSONPath here so that a chained accessor keys the +/// same needle as the equivalent single-step path. /// /// `value` must be a scalar. A single value selector is only injective for /// scalars — a container MACs just its structural tag, so every object at a path /// would collapse to one selector. The client rejects those; rejecting here too /// gives a message naming the query shape rather than the encryption internals. pub fn json_value_selector_plaintext( - path: &str, + path: &[&str], value: serde_json::Value, ) -> Result { if value.is_object() || value.is_array() { @@ -152,7 +189,7 @@ pub fn json_value_selector_plaintext( } Ok(Plaintext::new(serde_json::json!({ - "path": json_selector_path(path), + "path": compose_json_selector_path(path), "value": value, }))) } @@ -626,6 +663,67 @@ mod binary_json_value_tests { bind_param_json_value(&BindParam::null(), &Type::TEXT).unwrap() ); } + + /// A chain of steps composes into ONE path, so `col -> 'a' -> 'b'` keys the + /// same needle as the equivalent `jsonb_path_query_first(col, '$.a.b')`. + #[test] + fn accessor_chain_composes_into_one_path() { + assert_eq!("$.a.b", compose_json_selector_path(&["a", "b"])); + assert_eq!( + "$.a.b.c.d", + compose_json_selector_path(&["a", "b", "c", "d"]) + ); + assert_eq!( + compose_json_selector_path(&["$.a.b"]), + compose_json_selector_path(&["a", "b"]), + "the spellings of one path must agree" + ); + } + + /// A step already written as a path is rooted at the path so far, not at a + /// second `$`. + #[test] + fn a_rooted_step_splices_onto_the_path_so_far() { + assert_eq!("$.a.b", compose_json_selector_path(&["$.a", "b"])); + assert_eq!("$.a.b", compose_json_selector_path(&["a", "$.b"])); + assert_eq!("$.a[0].b", compose_json_selector_path(&["a", "$[0]", "b"])); + // A bare `$` is the document itself and adds no step. + assert_eq!("$.a", compose_json_selector_path(&["$", "a"])); + } + + /// A single step composes exactly as it always did. + #[test] + fn a_single_step_keeps_its_rooting_rules() { + for path in [ + "name", + "nested.title", + "$.nested.title", + "$", + "$[0]", + "$[*].b", + ] { + assert_eq!( + json_selector_path(path), + compose_json_selector_path(&[path]), + "unexpected composition of `{path}`" + ); + } + + assert_eq!("$.name", json_selector_path("name")); + assert_eq!("$[0]", json_selector_path("$[0]")); + } + + /// The needle is keyed on the composed path, and containers are rejected + /// whatever the path's shape. + #[test] + fn a_needle_is_keyed_on_the_composed_path() { + assert_eq!( + Plaintext::new(serde_json::json!({"path": "$.a.b", "value": "v"})), + json_value_selector_plaintext(&["a", "b"], serde_json::json!("v")).unwrap() + ); + + assert!(json_value_selector_plaintext(&["a", "b"], serde_json::json!({"x": 1})).is_err()); + } } #[cfg(test)] diff --git a/packages/cipherstash-proxy/src/postgresql/data/mod.rs b/packages/cipherstash-proxy/src/postgresql/data/mod.rs index 63c618c75..651994e19 100644 --- a/packages/cipherstash-proxy/src/postgresql/data/mod.rs +++ b/packages/cipherstash-proxy/src/postgresql/data/mod.rs @@ -9,7 +9,10 @@ use rust_decimal::{prelude::FromPrimitive, Decimal}; use tracing::{debug, warn}; pub use from_sql::literal_from_sql; -pub use from_sql::{bind_param_json_value, json_value_selector_plaintext, literal_json_value}; +pub use from_sql::{ + bind_param_json_value, compose_json_selector_path, json_value_selector_plaintext, + literal_json_value, +}; pub use to_sql::to_sql; /// /// Fun fact: some clients can specify a parameter type with a parse message diff --git a/packages/cipherstash-proxy/src/postgresql/frontend.rs b/packages/cipherstash-proxy/src/postgresql/frontend.rs index ebcf1d0f9..5e2737f96 100644 --- a/packages/cipherstash-proxy/src/postgresql/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/frontend.rs @@ -19,7 +19,7 @@ use crate::postgresql::context::statement::{ use crate::postgresql::context::statement_metadata::{ProtocolType, StatementType}; use crate::postgresql::context::Portal; use crate::postgresql::data::{ - json_value_selector_plaintext, literal_from_sql, literal_json_value, + compose_json_selector_path, json_value_selector_plaintext, literal_from_sql, literal_json_value, }; use crate::postgresql::messages::close::Close; use crate::postgresql::messages::error_response::ErrorResponseCode; @@ -36,7 +36,7 @@ use crate::proxy::EncryptionService; use crate::{EqlOutput, EqlQueryPayload}; use bytes::BytesMut; use cipherstash_client::encryption::Plaintext; -use eql_mapper::{self, EqlMapperError, EqlTermVariant, JsonSelectorSource, TypeCheckedStatement}; +use eql_mapper::{self, EqlMapperError, EqlTermVariant, JsonSelectorSegment, TypeCheckedStatement}; use metrics::{counter, histogram}; use pg_escape::quote_literal; use serde::Serialize; @@ -1372,10 +1372,23 @@ fn literals_to_plaintext( .zip(literal_columns) .map(|((eql_term, val), col)| match col { Some(col) => { - let plaintext = if eql_term.variant() == EqlTermVariant::JsonValueSelector { - json_value_selector_literal_plaintext(typed_statement, val) - } else { - literal_from_sql(val, col.eql_term(), col.cast_type()) + let plaintext = match eql_term.variant() { + EqlTermVariant::JsonValueSelector => { + json_value_selector_literal_plaintext(typed_statement, val) + } + // A selector that carries a collapsed chain keys the composed + // path, not the one segment it spells. Only a selector the + // mapper recorded a chain for: a single access has no record + // and takes the ordinary single-segment route below. + EqlTermVariant::JsonAccessor + if typed_statement + .json_accessor_paths + .for_literal(val) + .is_some() => + { + json_accessor_path_literal_plaintext(typed_statement, val) + } + _ => literal_from_sql(val, col.eql_term(), col.cast_type()), }; plaintext.map_err(|err| { @@ -1407,9 +1420,21 @@ fn json_value_selector_literal_plaintext( typed_statement: &TypeCheckedStatement<'_>, literal: &ast::Value, ) -> Result, MappingError> { - let Some(JsonSelectorSource::Literal(path)) = - typed_statement.json_value_selectors.for_literal(literal) - else { + let path: Option> = typed_statement + .json_value_selectors + .for_literal(literal) + .and_then(|source| { + source + .segments() + .iter() + .map(|segment| match segment { + JsonSelectorSegment::Literal(selector) => Some(selector.as_str()), + JsonSelectorSegment::Param(_) => None, + }) + .collect::>>() + }); + + let Some(path) = path else { debug!( target: MAPPER, msg = "Encrypted JSON equality needs a literal selector when the value is a literal", @@ -1422,7 +1447,50 @@ fn json_value_selector_literal_plaintext( return Ok(None); }; - json_value_selector_plaintext(path, value).map(Some) + json_value_selector_plaintext(&path, value).map(Some) +} + +/// Composes the eJSONPath for the selector of a collapsed accessor chain whose +/// steps are all literals: `j -> 'a' -> 'b'` keys `$.a.b`. +/// +/// Only an all-literal path can be resolved here — the whole statement is +/// encrypted at Parse time, before any param is bound. `j -> $1 -> 'b'` (param +/// step, literal outermost selector) is therefore not supported: the surviving +/// operand is the literal, which must be encrypted now, while the step in front +/// of it is not known until Bind. The mirror image, `j -> 'a' -> $1`, works — the +/// surviving operand is the param, so the whole path resolves at Bind. +/// +/// Refusing is the only safe answer. Composing what is known would key `$.b` and +/// read a different field, silently. +fn json_accessor_path_literal_plaintext( + typed_statement: &TypeCheckedStatement<'_>, + literal: &ast::Value, +) -> Result, MappingError> { + let path: Option> = typed_statement + .json_accessor_paths + .for_literal(literal) + .and_then(|source| { + source + .segments() + .iter() + .map(|segment| match segment { + JsonSelectorSegment::Literal(selector) => Some(selector.as_str()), + JsonSelectorSegment::Param(_) => None, + }) + .collect::>>() + }); + + let Some(path) = path else { + debug!( + target: MAPPER, + msg = "An encrypted JSON path with a placeholder step must end in a placeholder, \ + so that the whole path can be resolved when the params are bound", + value = ?literal, + ); + return Err(MappingError::CouldNotParseParameter); + }; + + Ok(Some(Plaintext::new(compose_json_selector_path(&path)))) } fn to_json_literal_value(literal: &T) -> Result diff --git a/packages/cipherstash-proxy/src/postgresql/messages/bind.rs b/packages/cipherstash-proxy/src/postgresql/messages/bind.rs index 410436953..b3f382f5a 100644 --- a/packages/cipherstash-proxy/src/postgresql/messages/bind.rs +++ b/packages/cipherstash-proxy/src/postgresql/messages/bind.rs @@ -3,10 +3,11 @@ use crate::error::{Error, MappingError, ProtocolError}; use crate::log::MAPPER; use crate::postgresql::context::column::Column; use crate::postgresql::context::statement::{ - params_are_positional, JsonSelectorPath, OutputParam, OutputParamSource, + params_are_positional, JsonSelectorPath, JsonSelectorStep, OutputParam, OutputParamSource, }; use crate::postgresql::data::{ - bind_param_from_sql, bind_param_json_value, json_value_selector_plaintext, + bind_param_from_sql, bind_param_json_value, compose_json_selector_path, + json_value_selector_plaintext, }; use crate::postgresql::format_code::FormatCode; use crate::postgresql::protocol::BytesMutReadString; @@ -105,29 +106,33 @@ impl Bind { OutputParamSource::JsonValueSelector { path, value } => { self.json_value_selector_plaintext(path, *value, &bound_param_type) } + OutputParamSource::JsonAccessorPath { path, .. } => { + self.json_accessor_path_plaintext(path) + } } }) .collect() } /// Composes `{"path", "value"}` — the input to `SteVecValueSelector` — from - /// the two operands of a JSON field equality. + /// the operands of a JSON field equality. + /// + /// Each step of the path is either a literal from the SQL or another bind + /// param, which is read straight off the wire: it is the selector *text*, so + /// it needs none of the per-column decoding the value half goes through. /// - /// The path is either a literal from the SQL or another bind param, which is - /// read straight off the wire: it is the selector *text*, so it needs none - /// of the per-column decoding the value half goes through. + /// A NULL step (or a NULL value) yields no needle: `col -> NULL = x` is NULL + /// in SQL, so there is nothing to match. The caller must then bind NULL — + /// forwarding the operand the client sent would put it on the wire in + /// plaintext. fn json_value_selector_plaintext( &self, path: &JsonSelectorPath, value: usize, postgres_type: &Type, ) -> Result, Error> { - let path = match path { - JsonSelectorPath::Literal(path) => path.to_owned(), - JsonSelectorPath::Param(path_idx) => match self.param_values.get(*path_idx) { - Some(param) if !param.is_null() => param.to_string(), - _ => return Ok(None), - }, + let Some(steps) = self.resolve_selector_path(path) else { + return Ok(None); }; let Some(param) = self.param_values.get(value) else { @@ -141,11 +146,66 @@ impl Bind { debug!( target: MAPPER, msg = "Fused JSON value selector", - ?path, + path = ?steps, ?value ); - Ok(Some(json_value_selector_plaintext(&path, value)?)) + let steps: Vec<&str> = steps.iter().map(String::as_str).collect(); + + Ok(Some(json_value_selector_plaintext(&steps, value)?)) + } + + /// Composes the eJSONPath a collapsed accessor chain traverses. + /// + /// The whole plaintext of this operand IS the path: `j -> 'a' -> $1` emits one + /// accessor whose selector must key `$.a.<$1>`, so the step the client bound + /// here is only the last of them. Everything else about this operand — its + /// column, its encryption as a bare selector — is the same as for a + /// single-step accessor; only the text differs. + /// + /// A NULL step yields no path: `j -> NULL -> 'b'` is NULL in SQL, so there is + /// nothing to select. The caller must then bind NULL rather than forward what + /// the client sent, which would put a selector on the wire in plaintext. + fn json_accessor_path_plaintext( + &self, + path: &JsonSelectorPath, + ) -> Result, Error> { + let Some(steps) = self.resolve_selector_path(path) else { + return Ok(None); + }; + + let steps: Vec<&str> = steps.iter().map(String::as_str).collect(); + let composed = compose_json_selector_path(&steps); + + debug!( + target: MAPPER, + msg = "Composed JSON accessor path", + path = ?steps, + ?composed + ); + + Ok(Some(Plaintext::new(composed))) + } + + /// Resolves each step of a selector path to its text, or `None` if any step is + /// unbound or NULL. + /// + /// A param step is read straight off the wire: it is the selector *text*, so + /// it needs none of the per-column decoding a value operand goes through. + fn resolve_selector_path(&self, path: &JsonSelectorPath) -> Option> { + let mut steps = Vec::with_capacity(path.steps.len()); + + for step in &path.steps { + match step { + JsonSelectorStep::Literal(selector) => steps.push(selector.to_owned()), + JsonSelectorStep::Param(step_idx) => match self.param_values.get(*step_idx) { + Some(param) if !param.is_null() => steps.push(param.to_string()), + _ => return None, + }, + } + } + + Some(steps) } /// Replaces the bound params with the output params of the rewritten @@ -163,8 +223,13 @@ impl Bind { encrypted: Vec>, ) -> Result<(), Error> { if output_params.len() == self.param_values.len() && params_are_positional(output_params) { - for (param, ct) in self.param_values.iter_mut().zip(encrypted.iter()) { - Self::apply_encrypted(param, ct.as_ref())?; + for ((param, output), ct) in self + .param_values + .iter_mut() + .zip(output_params.iter()) + .zip(encrypted.iter()) + { + Self::apply_output(param, output, ct.as_ref())?; } return Ok(()); } @@ -179,7 +244,8 @@ impl Bind { }, )?; - Self::apply_encrypted(&mut param, ct.as_ref())?; + Self::apply_output(&mut param, output, ct.as_ref())?; + param_values.push(param); } @@ -192,6 +258,29 @@ impl Bind { Ok(()) } + /// Writes what PostgreSQL receives for one output param. + /// + /// An output param the plan says must be ENCRYPTED, but for which no + /// ciphertext was produced, is bound NULL. Its bytes are the client's + /// plaintext operand, so leaving them in place would send it to the + /// database: that is the shape a fusion takes when it cannot build a needle + /// (`col -> NULL = $1`), where the operand went unencrypted precisely + /// because there is nothing to match. NULL is also what the SQL means — a + /// comparison against NULL is NULL — so the predicate correctly returns no + /// rows. + fn apply_output( + param: &mut BindParam, + output: &OutputParam, + ct: Option<&EqlOutput>, + ) -> Result<(), Error> { + if output.column.is_some() && ct.is_none() { + param.rewrite_null(); + return Ok(()); + } + + Self::apply_encrypted(param, ct) + } + fn apply_encrypted(param: &mut BindParam, ct: Option<&EqlOutput>) -> Result<(), Error> { match ct { // A JSON selector (`->`/`->>`/`jsonb_path_query`) is a bare @@ -291,6 +380,21 @@ impl BindParam { self.dirty = true; } + /// Rewrite this param as NULL, discarding whatever the client bound. + /// + /// Used for an encrypted operand that produced no ciphertext: its bytes are + /// plaintext, so they must not reach the database. An already-NULL param is + /// left alone rather than marked dirty — there is nothing to replace, and + /// dirtying it would re-send a Bind message that has not changed. + pub fn rewrite_null(&mut self) { + if self.is_null() { + return; + } + + self.bytes.clear(); + self.dirty = true; + } + pub fn requires_rewrite(&self) -> bool { self.dirty } @@ -473,13 +577,18 @@ impl TryFrom for BytesMut { #[cfg(test)] mod tests { - use super::BindParam; + use super::{BindParam, JsonSelectorPath, JsonSelectorStep, OutputParam, OutputParamSource}; use crate::{ config::LogConfig, log, - postgresql::{format_code::FormatCode, messages::bind::Bind}, + postgresql::{ + context::column::Column, format_code::FormatCode, messages::bind::Bind, messages::Name, + }, + Identifier, }; use bytes::BytesMut; + use cipherstash_client::schema::{ColumnConfig, ColumnMode, ColumnType}; + use eql_mapper::EqlTermVariant; fn to_message(s: &[u8]) -> BytesMut { BytesMut::from(s) @@ -531,4 +640,132 @@ mod tests { assert!(param.requires_rewrite()); } + + fn text_param(value: &str) -> BindParam { + BindParam::new(FormatCode::Text, BytesMut::from(value.as_bytes())) + } + + fn encrypted_column() -> Column { + Column { + identifier: Identifier::new("encrypted", "encrypted_jsonb"), + config: ColumnConfig { + name: "encrypted_jsonb".to_owned(), + in_place: false, + cast_type: ColumnType::Json, + indexes: vec![], + mode: ColumnMode::PlaintextDuplicate, + }, + postgres_type: postgres_types::Type::JSONB, + eql_term: EqlTermVariant::JsonValueSelector, + } + } + + fn bind_with(param_values: Vec) -> Bind { + Bind { + code: 'B', + portal: Name::unnamed(), + prepared_statement: Name::unnamed(), + num_param_format_codes: param_values.len() as i16, + param_format_codes: param_values.iter().map(|p| p.format_code).collect(), + num_param_values: param_values.len() as i16, + param_values, + num_result_column_format_codes: 0, + result_columns_format_codes: vec![], + reshaped: false, + } + } + + /// `col -> $1 = $2` with `$1` bound NULL builds no needle, so `$2` is never + /// encrypted. Its bytes are the client's plaintext comparand: binding them + /// would send the value to the database in the clear. NULL is bound instead, + /// which is also what the SQL means. + #[test] + fn a_fusion_with_no_needle_binds_null_rather_than_the_clients_value() { + log::init(LogConfig::default()); + + let mut bind = bind_with(vec![BindParam::null(), text_param("\"world\"")]); + + let output_params = vec![OutputParam { + column: Some(encrypted_column()), + source: OutputParamSource::JsonValueSelector { + path: JsonSelectorPath { + steps: vec![JsonSelectorStep::Param(0)], + }, + value: 1, + }, + query_operand: true, + }]; + + // What `to_plaintext` yields for this Bind: no needle, so no ciphertext. + assert_eq!( + vec![None], + bind.to_plaintext(&output_params, &[]).unwrap(), + "a NULL selector must not produce a needle" + ); + + bind.rewrite(&output_params, vec![None]).unwrap(); + + assert_eq!(1, bind.param_values.len()); + assert!( + bind.param_values[0].is_null(), + "the value operand must be bound NULL, not forwarded: {:?}", + bind.param_values[0].to_string() + ); + assert!(bind.requires_rewrite()); + } + + /// The same holds when it is the VALUE that is NULL: nothing to encrypt, and + /// nothing of the client's to forward. + #[test] + fn a_fusion_with_a_null_value_binds_null() { + log::init(LogConfig::default()); + + let mut bind = bind_with(vec![text_param("nested"), BindParam::null()]); + + let output_params = vec![OutputParam { + column: Some(encrypted_column()), + source: OutputParamSource::JsonValueSelector { + path: JsonSelectorPath { + steps: vec![JsonSelectorStep::Param(0)], + }, + value: 1, + }, + query_operand: true, + }]; + + assert_eq!(vec![None], bind.to_plaintext(&output_params, &[]).unwrap()); + + bind.rewrite(&output_params, vec![None]).unwrap(); + + assert_eq!(1, bind.param_values.len()); + assert!(bind.param_values[0].is_null()); + } + + /// A NATIVE param has no column, so it is forwarded exactly as bound — the + /// NULL rule is about operands that were supposed to be encrypted. + #[test] + fn a_native_param_is_still_forwarded_unchanged() { + log::init(LogConfig::default()); + + let mut bind = bind_with(vec![text_param("42"), text_param("plaintext")]); + + let output_params = vec![ + OutputParam { + column: None, + source: OutputParamSource::Input(0), + query_operand: false, + }, + OutputParam { + column: None, + source: OutputParamSource::Input(1), + query_operand: false, + }, + ]; + + bind.rewrite(&output_params, vec![None, None]).unwrap(); + + assert_eq!("42", bind.param_values[0].to_string()); + assert_eq!("plaintext", bind.param_values[1].to_string()); + assert!(!bind.requires_rewrite()); + } } diff --git a/packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs b/packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs index d2cf1d8a6..bb563c19f 100644 --- a/packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs +++ b/packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs @@ -300,6 +300,15 @@ impl EncryptionService for ZeroKms { EqlOperation::Query(&index.index_type, QueryOp::SteVecValueSelector) }) .unwrap_or(EqlOperation::Store), + + // The result of an extraction, not an operand: it is read + // back from the database and decrypted, never encrypted on + // the way in. Refuse rather than fall through to `Store`, + // which would encrypt it in the wrong shape and silently + // return the wrong rows. + EqlTermVariant::JsonExtracted => { + return Err(EncryptError::JsonExtractedIsNotAnOperand.into()) + } }; let prepared = PreparedPlaintext::new( diff --git a/packages/eql-mapper/src/eql_mapper.rs b/packages/eql-mapper/src/eql_mapper.rs index 2df75aec1..edde1cc30 100644 --- a/packages/eql-mapper/src/eql_mapper.rs +++ b/packages/eql-mapper/src/eql_mapper.rs @@ -181,6 +181,7 @@ impl<'ast> EqlMapper<'ast> { params, literals, self.inferencer.borrow().take_json_value_selectors(), + self.inferencer.borrow().take_json_accessor_paths(), self.inferencer.borrow().take_query_operands(), Arc::new(node_types), )) diff --git a/packages/eql-mapper/src/inference/infer_type_impls/expr.rs b/packages/eql-mapper/src/inference/infer_type_impls/expr.rs index 792f5830a..ffac234bd 100644 --- a/packages/eql-mapper/src/inference/infer_type_impls/expr.rs +++ b/packages/eql-mapper/src/inference/infer_type_impls/expr.rs @@ -4,13 +4,11 @@ use crate::{ unifier::{EqlTerm, EqlValue, TokenType, Type, Value}, InferType, TypeError, }, - EqlTrait, IdentCase, JsonSelectorSource, Param, TypeInferencer, + json_value_selector::{json_accessor, json_accessor_chain, unnest}, + EqlTrait, IdentCase, JsonSelectorSegment, JsonSelectorSource, Param, TypeInferencer, }; use eql_mapper_macros::trace_infer; -use sqltk::parser::ast::{ - self as ast, AccessExpr, Array, BinaryOperator, Expr, FunctionArg, FunctionArgExpr, - FunctionArguments, Ident, Subscript, -}; +use sqltk::parser::ast::{self as ast, AccessExpr, Array, BinaryOperator, Expr, Ident, Subscript}; /// The capability a comparison operator requires of its operands, or `None` if /// it is not a comparison. @@ -26,6 +24,64 @@ fn comparison_capability(op: &BinaryOperator) -> Option { #[trace_infer] impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { + /// Marks JSON accessor chains that a comparison will fuse, on the way DOWN. + /// + /// Typing is post-order, so by the time a chain's outermost `->` is typed its + /// parent is not yet known — and the parent is exactly what decides WHERE the + /// chain's composed path has to be recorded. Under `= $1` the chain is fused + /// into the equality's own needle and the accessor is discarded, so the path + /// belongs to the value operand; anywhere else the chain collapses to a + /// surviving accessor whose selector must carry the path itself. Recording the + /// intent here is what lets the `->` rule pick the right channel. + /// + /// Both outcomes are legal — this no longer gates whether a chain is allowed, + /// only which record it produces. Writing the path into both channels would be + /// worse than writing it into neither: the fused case would then also try to + /// resolve the discarded selector as a standalone path, which for + /// `j -> $1 -> 'b' = $2` is unresolvable at Parse time and would refuse a + /// query that works today. + /// + /// Syntactic only: no child has a type yet. Whether the chain's root is + /// really an encrypted document is checked on the way back up. + fn infer_enter(&mut self, expr_val: &'ast Expr) -> Result<(), TypeError> { + if let Expr::BinaryOp { left, op, right } = expr_val { + // Only EQUALITY fuses a chain, collapsing the whole path into one + // value-selector containment against the root document so that the + // accessor disappears from the emitted SQL entirely. + // + // Ordering does NOT: `RewriteEqlComparisonOps` types the scalar + // operand as a SteVec ordering term and leaves the accessor standing, + // so the chain is collapsed by `CollapseJsonAccessorChain` like any + // other and keeps its own path record. + let fuses = matches!(op, BinaryOperator::Eq | BinaryOperator::NotEq); + + if fuses { + for operand in [&**left, &**right] { + // Mark every accessor node along the chain's spine, not just + // the outermost. `j -> 'a' -> 'b' -> 'c'` is three nested + // `BinaryOp`s and EVERY one of them is typed, so marking only + // the top would leave `j -> 'a' -> 'b'` looking unfused and + // record a path for a selector the fusion then discards. + // + // Unnest at each step: `((j -> 'a') -> 'b') = $1` reaches the + // `->` rule as the bare accessor, so marking the bracket + // would mark a node that rule never asks about. + // One step at a time: `json_accessor_chain` would jump + // straight to the root and skip the intermediates that need + // marking. + let mut node = unnest(operand); + + while let Some((container, _)) = json_accessor(node) { + self.mark_fusable_json_chain(node); + node = unnest(container); + } + } + } + } + + Ok(()) + } + fn infer_exit(&mut self, expr_val: &'ast Expr) -> Result<(), TypeError> { match expr_val { // Resolve an identifier using the scope, except if it happens to to be the DEFAULT keyword @@ -194,16 +250,17 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { self.eql_json_field_access(left), self.eql_json_field_access(right), ) { - (Some((json, selector)), None) => { + (Some((json, selectors)), None) => { let fused = - self.infer_json_value_selector(json, selector, right)?; + self.infer_json_value_selector(json, selectors, right)?; if fused { self.unify_node_with_type(expr_val, Type::native())?; } fused } - (None, Some((json, selector))) => { - let fused = self.infer_json_value_selector(json, selector, left)?; + (None, Some((json, selectors))) => { + let fused = + self.infer_json_value_selector(json, selectors, left)?; if fused { self.unify_node_with_type(expr_val, Type::native())?; } @@ -215,6 +272,85 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { false }; + // Encrypted JSON field ACCESS (`->`, `->>`). + // + // This is the chain-aware half of `EqlTerm::JsonExtracted`. The + // operator declaration is compositional — it can only see the + // type of its immediate left operand — but a chain is not + // compositional: `j -> 'a' -> 'b'` is ONE path into ONE + // document, and its intermediate `j -> 'a'` has no independent + // existence for the database. Typing it step by step would make + // the first link `JsonExtracted` and the second link fail, which + // would reject every chain. + // + // So the rule consults the chain BELOW this node rather than the + // type of its operand. Within one expression the walker can + // always reach the root, so being handed an extracted + // intermediate is fine — what matters is whether the root is a + // document. Across a subquery boundary the walker cannot reach + // it, the root is itself `JsonExtracted`, and the access is + // refused. + let handled = handled + || if matches!(op, BinaryOperator::Arrow | BinaryOperator::LongArrow) { + // A root that is already an extracted entry is the + // cross-subquery case, at any chain length: `a -> 'foo'` + // where `a` is `j -> 'bar'`. Report it precisely instead + // of leaving the declaration to say `JsonExtracted` does + // not satisfy `JsonLike`. + if let Some((root, _)) = json_accessor_chain(expr_val) { + if self.is_eql_json_extracted(root) { + return Err(TypeError::UnqueryableJsonExtraction); + } + } + + // Only a MULTI-step chain needs special treatment. A + // single access is handled correctly by the declaration + // (`-> ::Output`), for native and + // encrypted alike. + match json_accessor_chain(expr_val) + .filter(|(_, selectors)| selectors.len() > 1) + { + Some((root, selectors)) => match self.eql_json_document(root) { + // A chain rooted at a document: type the whole + // access as one extraction from that document, + // and the OUTERMOST selector as its accessor so + // it is encrypted. `CollapseJsonAccessorChain` + // then drops the inner accessors, leaving that + // one selector to carry the whole path — so + // record what the whole path is. + // + // Unless a comparison above will fuse the chain + // into its own needle, in which case the accessor + // does not survive at all and the path belongs in + // the other channel, recorded by the equality + // branch above. + Some(json) => { + if !self.is_fusable_json_chain(expr_val) { + self.record_json_accessor_path(&selectors)?; + } + + self.unify_node_with_type( + &**right, + Type::Value(Value::Eql(EqlTerm::JsonAccessor( + json.clone(), + ))), + )?; + self.unify_node_with_type( + expr_val, + Type::Value(Value::Eql(EqlTerm::JsonExtracted(json))), + )?; + true + } + // Native JSON: the declaration is correct for it, + // and plaintext `jsonb` chains legitimately. + None => false, + }, + None => false, + } + } else { + false + }; + if !handled { // `@@` is symmetric in PostgreSQL, so the encrypted column // may be written on either side. The operator rule is @@ -646,38 +782,61 @@ impl<'ast> TypeInferencer<'ast> { } } + /// An encrypted JSON **document** — something with an `sv` array that a path + /// can be traversed into. + /// + /// Unlike [`Self::eql_json_value`] this inspects the term *variant*, because + /// the distinction it draws is the whole point of + /// [`EqlTerm::JsonExtracted`]: an already-extracted entry carries the same + /// `EqlValue` as the document it came from, so ignoring the variant would + /// accept it and re-derive the bug. An entry has no `sv`, so traversing it + /// selects nothing. + fn eql_json_document(&self, expr: &'ast Expr) -> Option { + match &*self.get_node_type(expr) { + Type::Value(Value::Eql(eql_term @ (EqlTerm::Full(_) | EqlTerm::Partial(_, _)))) => { + let eql_value = eql_term.eql_value(); + (eql_value.domain_identity().token == TokenType::Json).then(|| eql_value.clone()) + } + _ => None, + } + } + + /// Whether `expr` is an already-extracted encrypted JSON entry. + /// + /// Used to turn a second traversal into a precise error rather than letting + /// it fall through to the operator declaration, where the failure would be + /// an opaque unsatisfied-`JsonLike` bound. + fn is_eql_json_extracted(&self, expr: &'ast Expr) -> bool { + matches!( + &*self.get_node_type(expr), + Type::Value(Value::Eql(EqlTerm::JsonExtracted(_))) + ) + } + /// Deconstructs an encrypted-JSON **field access** into the accessed value - /// and the expression supplying its selector: + /// and the expressions supplying its selectors, outermost last: /// /// - `col -> sel`, `col ->> sel` /// - `jsonb_path_query_first(col, sel)` + /// - and chains of those: `col -> 'a' -> 'b'` /// /// Returns `None` for anything else — importantly for a bare encrypted JSON /// column, which is a whole document, not a field of one. Equality needs the - /// selector expression itself (not just the type), because the path is one - /// half of the fused value-selector needle. - fn eql_json_field_access(&self, expr: &'ast Expr) -> Option<(EqlValue, &'ast Expr)> { - let selector = match expr { - Expr::BinaryOp { - op: BinaryOperator::Arrow | BinaryOperator::LongArrow, - right, - .. - } => &**right, - - // `jsonb_path_query_first(col, sel)` — and its already-rewritten - // `eql_v3.` spelling. The selector is the second argument. - Expr::Function(function) => match &function.args { - FunctionArguments::List(list) => match list.args.as_slice() { - [_, FunctionArg::Unnamed(FunctionArgExpr::Expr(sel))] => sel, - _ => return None, - }, - _ => return None, - }, - - _ => return None, - }; - - self.eql_json_value(expr).map(|json| (json, selector)) + /// selector expressions themselves (not just the type), because the path + /// they compose is one half of the fused value-selector needle. + /// + /// A chain yields ALL of its selectors, not just the outermost. Its + /// intermediate accessors have no independent existence for the database: + /// the payload is encrypted, so native `->` applied to it selects nothing. + /// The chain is one path into one document, and it is fused as one. + fn eql_json_field_access(&self, expr: &'ast Expr) -> Option<(EqlValue, Vec<&'ast Expr>)> { + let (root, selectors) = json_accessor_chain(expr)?; + + // Resolved from the ROOT, which must be a whole document. Reading the + // node's own type instead would accept a chain rooted at an + // already-extracted entry (`a -> 'foo'` where `a` came from a subquery) + // and fuse a needle keyed on `$.foo` when the real path is `$.bar.foo`. + self.eql_json_document(root).map(|json| (json, selectors)) } /// Records each of `exprs` that is a literal or placeholder as a query @@ -701,13 +860,14 @@ impl<'ast> TypeInferencer<'ast> { } /// Types `value` — the value half of `col -> sel = value` — as a fused - /// value selector, and records where its path half (`selector`) comes from. + /// value selector, and records where its path half (`selectors`) comes from. /// - /// A path that is neither a literal nor a placeholder (a column reference, a - /// function call) cannot be resolved to a needle at encryption time, so the - /// fusion is declined and the comparison falls through to ordinary typing — - /// where it will fail the capability check with a clearer error than a - /// half-built needle would produce. + /// A path step that is neither a literal nor a placeholder (a column + /// reference, a function call) cannot be resolved to a needle at encryption + /// time, so the fusion is declined and the comparison falls through to + /// ordinary typing — where it will fail the capability check with a clearer + /// error than a half-built needle would produce. One unresolvable step + /// declines the whole chain: a partial path is not a path. /// /// Returns whether the fusion was applied. The caller must not treat a /// declined fusion as handled: doing so skips the binop rule that is the @@ -717,13 +877,19 @@ impl<'ast> TypeInferencer<'ast> { fn infer_json_value_selector( &self, json: EqlValue, - selector: &'ast Expr, + selectors: Vec<&'ast Expr>, value: &'ast Expr, ) -> Result { - let Some(source) = Self::json_selector_source(selector) else { + let Some(segments) = selectors + .into_iter() + .map(Self::json_selector_segment) + .collect::>>() + else { return Ok(false); }; + let source = JsonSelectorSource::new(segments); + self.unify_node_with_type( value, Type::Value(Value::Eql(EqlTerm::JsonValueSelector(json))), @@ -732,7 +898,7 @@ impl<'ast> TypeInferencer<'ast> { match Self::as_ast_value(value) { Some(ast::Value::Placeholder(placeholder)) => { if let Ok(param) = Param::try_from(placeholder) { - self.record_json_value_selector_param(param, source); + self.record_json_value_selector_param(param, source)?; } } Some(node) => self.record_json_value_selector_literal(node, source), @@ -742,17 +908,59 @@ impl<'ast> TypeInferencer<'ast> { Ok(true) } - /// Classifies the path half of a fused value selector: a placeholder yields - /// the param it will arrive in, a literal yields its text inline. - fn json_selector_source(selector: &'ast Expr) -> Option { + /// Records the composed path of a multi-step accessor chain against the + /// operand that will carry it: the OUTERMOST selector, the one node of the + /// chain that survives the rewrite. + /// + /// The surviving operand's own text is a single segment (`'b'` of + /// `j -> 'a' -> 'b'`), while the selector it must key is the whole path + /// (`$.a.b`). Nothing the proxy is handed at encryption time could recover + /// the difference, which is why it is recorded here. + /// + /// Unlike the fused-equality case, an unresolvable step cannot be waved + /// through to a capability error: the rewrite collapses the chain either way, + /// so a step the proxy cannot resolve would be silently dropped and the query + /// would read a different field. It is refused. + fn record_json_accessor_path(&self, selectors: &[&'ast Expr]) -> Result<(), TypeError> { + let segments = selectors + .iter() + .map(|selector| Self::json_selector_segment(selector)) + .collect::>>() + .ok_or(TypeError::UncomposableJsonPath)?; + + let source = JsonSelectorSource::new(segments); + + // The last selector is the outermost, and `json_selector_segment` + // succeeded for it above, so it is a literal or a placeholder. + let Some(outermost) = selectors.last().and_then(|s| Self::as_ast_value(s)) else { + return Err(TypeError::UncomposableJsonPath); + }; + + match outermost { + ast::Value::Placeholder(placeholder) => { + let param = + Param::try_from(placeholder).map_err(|_| TypeError::UncomposableJsonPath)?; + self.record_json_accessor_path_param(param, source) + } + node => { + self.record_json_accessor_path_literal(node, source); + Ok(()) + } + } + } + + /// Classifies one step of the path half of a fused value selector: a + /// placeholder yields the param it will arrive in, a literal yields its text + /// inline. + fn json_selector_segment(selector: &'ast Expr) -> Option { match Self::as_ast_value(selector)? { ast::Value::Placeholder(placeholder) => Param::try_from(placeholder) .ok() - .map(JsonSelectorSource::Param), + .map(JsonSelectorSegment::Param), ast::Value::SingleQuotedString(s) | ast::Value::DoubleQuotedString(s) - | ast::Value::EscapedStringLiteral(s) => Some(JsonSelectorSource::Literal(s.clone())), - ast::Value::Number(n, _) => Some(JsonSelectorSource::Literal(n.to_string())), + | ast::Value::EscapedStringLiteral(s) => Some(JsonSelectorSegment::Literal(s.clone())), + ast::Value::Number(n, _) => Some(JsonSelectorSegment::Literal(n.to_string())), _ => None, } } diff --git a/packages/eql-mapper/src/inference/mod.rs b/packages/eql-mapper/src/inference/mod.rs index faf314106..cfd31304b 100644 --- a/packages/eql-mapper/src/inference/mod.rs +++ b/packages/eql-mapper/src/inference/mod.rs @@ -9,18 +9,21 @@ pub mod unifier; use unifier::{Unifier, *}; -use std::{cell::RefCell, fmt::Debug, marker::PhantomData, ops::ControlFlow, rc::Rc, sync::Arc}; +use std::{ + cell::RefCell, collections::HashSet, fmt::Debug, marker::PhantomData, ops::ControlFlow, rc::Rc, + sync::Arc, +}; use infer_type::InferType; use sqltk::parser::ast::{ Delete, Expr, Function, FunctionArgExpr, Ident, Insert, ObjectName, Query, Select, SelectItem, SetExpr, Statement, ValueWithSpan, Values, WindowSpec, }; -use sqltk::{into_control_flow, AsNodeKey, Break, Visitable, Visitor}; +use sqltk::{into_control_flow, AsNodeKey, Break, NodeKey, Visitable, Visitor}; use crate::{ - JsonSelectorSource, JsonValueSelectors, Param, QueryOperands, ScopeError, ScopeTracker, - TableResolver, + JsonAccessorPaths, JsonSelectorSource, JsonValueSelectors, Param, QueryOperands, ScopeError, + ScopeTracker, TableResolver, }; pub(crate) use registry::*; @@ -61,12 +64,36 @@ pub struct TypeInferencer<'ast> { /// the path for which value ([`crate::JsonValueSelectors`]). json_value_selectors: RefCell>, + /// The composed paths of the multi-step accessor chains that survive into the + /// rewritten SQL as a single accessor ([`crate::JsonAccessorPaths`]). + /// + /// The surviving selector operand's own text is one segment of the path it + /// must key, so the proxy cannot derive the rest from what it is handed. + json_accessor_paths: RefCell>, + /// The operands that appear in a query position, so the proxy can project /// their payloads to query operands ([`crate::QueryOperands`]). Recorded /// here rather than derived later because it is a fact about the statement's /// shape, and the proxy needs it before it encrypts anything. query_operands: RefCell>, + /// The JSON accessor chains that a fusable comparison sits above, recorded + /// on the way DOWN the tree. + /// + /// Typing is post-order, so when a chain's outermost `->` is typed its + /// parent does not exist yet — and the parent is what decides which of the + /// two channels the chain's path belongs in. `j -> 'a' -> 'b' = $1` fuses + /// the path into the equality's needle and the accessor disappears + /// altogether; the same chain anywhere else survives as a single accessor + /// whose selector must key the composed path. Both are correct, and they + /// want DIFFERENT records — so the comparison marks the chain before any of + /// it is typed, and the `->` rule then records into the right channel. + /// + /// Purely syntactic — at enter time no child has a type yet. Whether the + /// chain's root is really an encrypted document is still checked on the way + /// back up. + fusable_json_chains: RefCell>>, + _ast: PhantomData<&'ast ()>, } @@ -82,7 +109,9 @@ impl<'ast> TypeInferencer<'ast> { scope_tracker: scope.into(), unifier: unifier.into(), json_value_selectors: RefCell::new(JsonValueSelectors::default()), + json_accessor_paths: RefCell::new(JsonAccessorPaths::default()), query_operands: RefCell::new(QueryOperands::default()), + fusable_json_chains: RefCell::new(HashSet::new()), _ast: PhantomData, } } @@ -93,11 +122,35 @@ impl<'ast> TypeInferencer<'ast> { std::mem::take(&mut self.json_value_selectors.borrow_mut()) } + /// Takes the composed accessor-chain paths accumulated during inference, + /// leaving the inferencer's set empty. + pub(crate) fn take_json_accessor_paths(&self) -> JsonAccessorPaths<'ast> { + std::mem::take(&mut self.json_accessor_paths.borrow_mut()) + } + /// Takes the recorded query operands, leaving the inferencer's set empty. pub(crate) fn take_query_operands(&self) -> QueryOperands<'ast> { std::mem::take(&mut self.query_operands.borrow_mut()) } + /// Marks a JSON accessor chain as sitting under a comparison that will fuse + /// it, before any of it has been typed. + pub(crate) fn mark_fusable_json_chain(&self, node: &'ast N) { + self.fusable_json_chains + .borrow_mut() + .insert(node.as_node_key()); + } + + /// Whether this node is the outermost accessor of a chain a comparison will + /// fuse into its own needle — in which case the accessor is discarded and its + /// path belongs in [`crate::JsonValueSelectors`], not + /// [`crate::JsonAccessorPaths`]. + pub(crate) fn is_fusable_json_chain(&self, node: &'ast N) -> bool { + self.fusable_json_chains + .borrow() + .contains(&node.as_node_key()) + } + pub(crate) fn record_query_operand_param(&self, param: Param) { self.query_operands.borrow_mut().record_param(param); } @@ -110,10 +163,11 @@ impl<'ast> TypeInferencer<'ast> { &self, param: Param, source: JsonSelectorSource, - ) { + ) -> Result<(), TypeError> { self.json_value_selectors .borrow_mut() - .record_param(param, source); + .record_param(param, source) + .map_err(|_| TypeError::AmbiguousJsonSelectorPath(param.0)) } pub(crate) fn record_json_value_selector_literal( @@ -126,6 +180,27 @@ impl<'ast> TypeInferencer<'ast> { .record_literal(node, source); } + pub(crate) fn record_json_accessor_path_param( + &self, + param: Param, + source: JsonSelectorSource, + ) -> Result<(), TypeError> { + self.json_accessor_paths + .borrow_mut() + .record_param(param, source) + .map_err(|_| TypeError::AmbiguousJsonSelectorPath(param.0)) + } + + pub(crate) fn record_json_accessor_path_literal( + &self, + node: &'ast sqltk::parser::ast::Value, + source: JsonSelectorSource, + ) { + self.json_accessor_paths + .borrow_mut() + .record_literal(node, source); + } + pub(crate) fn get_node_type(&self, node: &'ast N) -> Arc { self.unifier.borrow_mut().get_node_type(node) } diff --git a/packages/eql-mapper/src/inference/sql_types/sql_decls.rs b/packages/eql-mapper/src/inference/sql_types/sql_decls.rs index d43695aea..da13b75a6 100644 --- a/packages/eql-mapper/src/inference/sql_types/sql_decls.rs +++ b/packages/eql-mapper/src/inference/sql_types/sql_decls.rs @@ -20,8 +20,14 @@ static SQL_BINARY_OPERATORS: LazyLock> = (T >= T) -> Native where T: Ord; (T < T) -> Native where T: Ord; (T > T) -> Native where T: Ord; - (T -> ::Accessor) -> T where T: JsonLike; - (T ->> ::Accessor) -> T where T: JsonLike; + // `Output` rather than `T`: traversing native `jsonb` yields + // traversable `jsonb`, but traversing an ENCRYPTED document yields a + // single SteVec entry with no `sv` of its own, so it cannot be + // traversed again. The trait decides which, so a second traversal of + // an encrypted result fails the type check instead of silently + // returning NULL. + (T -> ::Accessor) -> ::Output where T: JsonLike; + (T ->> ::Accessor) -> ::Output where T: JsonLike; (T @> T) -> Native where T: Contain; (T <@ T) -> Native where T: Contain; (T ~~ ::Tokenized) -> Native where T: TokenMatch; // LIKE @@ -60,6 +66,15 @@ static SQL_FUNCTION_TYPES: LazyLock, FunctionDecl> pg_catalog.count(T) -> Native; pg_catalog.min(T) -> T where T: Ord; pg_catalog.max(T) -> T where T: Ord; + // NOT `Output`, deliberately — see the note on `->` above. + // Returning the extracted type here breaks two supported shapes: + // `jsonb_array_elements`/`jsonb_array_length` CONSUME an extracted + // entry rather than traversing it, and the rewrite rule that + // retargets these functions and encrypts their Path operand keys off + // the result type, so changing it sent the caller's literal jsonpath + // to PostgreSQL unencrypted (`@ is not allowed in root expressions`). + // Making these unqueryable needs the array functions taught to accept + // an extracted value first. pg_catalog.jsonb_path_query(T, ::Path) -> T where T: JsonLike; pg_catalog.jsonb_path_query_first(T, ::Path) -> T where T: JsonLike; pg_catalog.jsonb_path_exists(T, ::Path) -> Native where T: JsonLike; @@ -68,6 +83,15 @@ static SQL_FUNCTION_TYPES: LazyLock, FunctionDecl> pg_catalog.jsonb_array_elements_text(T) -> SetOf where T: JsonLike; eql_v3.min(T) -> T where T: Ord; eql_v3.max(T) -> T where T: Ord; + // NOT `Output`, deliberately — see the note on `->` above. + // Returning the extracted type here breaks two supported shapes: + // `jsonb_array_elements`/`jsonb_array_length` CONSUME an extracted + // entry rather than traversing it, and the rewrite rule that + // retargets these functions and encrypts their Path operand keys off + // the result type, so changing it sent the caller's literal jsonpath + // to PostgreSQL unencrypted (`@ is not allowed in root expressions`). + // Making these unqueryable needs the array functions taught to accept + // an extracted value first. eql_v3.jsonb_path_query(T, ::Path) -> T where T: JsonLike; eql_v3.jsonb_path_query_first(T, ::Path) -> T where T: JsonLike; eql_v3.jsonb_path_exists(T, ::Path) -> Native where T: JsonLike; diff --git a/packages/eql-mapper/src/inference/type_error.rs b/packages/eql-mapper/src/inference/type_error.rs index 26cf258b7..a1a78f6c2 100644 --- a/packages/eql-mapper/src/inference/type_error.rs +++ b/packages/eql-mapper/src/inference/type_error.rs @@ -18,6 +18,56 @@ pub enum TypeError { #[error("Type `{}` does not satisfy bounds `{}`", _0, _1)] UnsatisfiedBounds(Arc, EqlTraits), + /// A second JSON traversal of a value that is already the result of one. + /// + /// An extracted entry is not a document — it has no `sv` array — so a + /// further accessor selects nothing and the query silently returns NULL. + /// A chain written in ONE expression is collapsed into a single accessor + /// carrying the composed path, so this is reached only when the chain is + /// broken up such that the path cannot be composed: across a subquery + /// boundary, a CTE, or a view. + /// + /// That case is not merely unimplemented. The type of an extracted value does + /// not carry the path that produced it, and the root document is not even in + /// scope on the far side of the boundary, so there is nothing to root a + /// composed path at. + #[error( + "cannot apply a JSON operator to the result of an encrypted JSON \ + operation: an extracted field is a single encrypted entry, not a \ + document, so there is nothing left to traverse. Write the whole path in \ + one expression (`col -> 'a' -> 'b'`), which is resolved against the \ + document as a single path, rather than splitting it across a subquery, \ + CTE or view" + )] + UnqueryableJsonExtraction, + + /// A step of an encrypted JSON accessor chain that is neither a literal nor a + /// placeholder. + /// + /// A chain collapses to ONE accessor keyed on the composed path, so every + /// step has to be resolvable to path text by the time the proxy encrypts the + /// selector. A step the proxy cannot resolve — a column reference, a function + /// call — would be dropped from the statement along with the rest of the + /// chain, silently changing which field the query reads. + #[error( + "every step of an encrypted JSON path must be a literal or a placeholder: \ + the whole chain is collapsed into one keyed path, so a step computed by \ + the database cannot contribute to it" + )] + UncomposableJsonPath, + + /// One placeholder used as the selector of two chains with different paths. + /// + /// The path a selector operand keys is recorded against the param it arrives + /// in, because at Bind time the param number is all the proxy has. Two + /// different paths for one param cannot both be honoured, and picking either + /// answers the other occurrence from the wrong field. + #[error( + "placeholder ${0} is used as an encrypted JSON selector for two different \ + paths; give each path its own placeholder" + )] + AmbiguousJsonSelectorPath(u16), + #[error("unified type contains unresolved type variable: {}", _0)] Incomplete(String), diff --git a/packages/eql-mapper/src/inference/unifier/eql_traits.rs b/packages/eql-mapper/src/inference/unifier/eql_traits.rs index d02b69f70..7041d36b2 100644 --- a/packages/eql-mapper/src/inference/unifier/eql_traits.rs +++ b/packages/eql-mapper/src/inference/unifier/eql_traits.rs @@ -34,7 +34,7 @@ const ASSOC_TYPES_ORD: &EqlTraitAssociatedTypes = &EqlTraitAssociatedTypes(&["On const ASSOC_TYPES_TOKEN_MATCH: &EqlTraitAssociatedTypes = &EqlTraitAssociatedTypes(&["Tokenized"]); const ASSOC_TYPES_JSON_LIKE: &EqlTraitAssociatedTypes = - &EqlTraitAssociatedTypes(&["Path", "Accessor"]); + &EqlTraitAssociatedTypes(&["Path", "Accessor", "Output"]); const ASSOC_TYPES_CONTAIN: &EqlTraitAssociatedTypes = &EqlTraitAssociatedTypes(&["Only"]); @@ -70,6 +70,10 @@ impl EqlTrait { | (EqlTrait::TokenMatch, "Tokenized") | (EqlTrait::JsonLike, "Accessor") | (EqlTrait::JsonLike, "Path") + // Traversing native `jsonb` yields native `jsonb`, which is + // itself traversable — plaintext chains are legitimate SQL + // and must keep working. + | (EqlTrait::JsonLike, "Output") | (EqlTrait::Contain, "Only") => Ok(ty.clone()), (_, unknown_associated_type) => Err(TypeError::InternalError(format!( "Unknown associated type {self}::{unknown_associated_type}" @@ -98,6 +102,17 @@ impl EqlTrait { } (EqlTrait::JsonLike, "Path") => Ok(Arc::new(Type::Value(Value::Eql(EqlTerm::JsonPath(eql_col.clone()))), )), + // The result of traversing an ENCRYPTED document is one + // SteVec entry, which carries no `sv` and so cannot be + // traversed again. This is where native and encrypted JSON + // diverge: `Output` is the type the operator declaration + // returns, so the difference lives in the trait rather than + // in a special case at the call site. + (EqlTrait::JsonLike, "Output") => { + Ok(Arc::new(Type::Value(Value::Eql( + EqlTerm::JsonExtracted(eql_col.clone()), + )))) + } (EqlTrait::Contain, "Only") => { Ok(Arc::new(Type::Value(Value::Eql( EqlTerm::Partial(eql_col.clone(), EqlTraits::from(EqlTrait::Contain)), @@ -328,6 +343,23 @@ impl EqlTerm { EqlTerm::Tokenized(_) => EqlTraits::none(), EqlTerm::JsonOrd(_) => EqlTraits::none(), EqlTerm::JsonValueSelector(_) => EqlTraits::none(), + // An extracted SteVec entry is not a document, but it is not inert + // either: entries carry ordering and equality terms, which is what + // lets `ORDER BY col -> 'field'` sort by `ord_term(...)` and a + // comparison run over the extracted value. What an entry cannot do + // is be TRAVERSED — it has no `sv` — so the JSON capabilities are + // masked off while `eq`/`ord` are inherited from the column. + // + // Masked, not granted: a JSON domain that carries no ordering term + // must not gain one by extraction. + EqlTerm::JsonExtracted(eql_value) => { + let column = eql_value.effective_bounds(); + EqlTraits { + eq: column.eq, + ord: column.ord, + ..EqlTraits::none() + } + } } } } diff --git a/packages/eql-mapper/src/inference/unifier/types.rs b/packages/eql-mapper/src/inference/unifier/types.rs index 646e6cb09..442b020d1 100644 --- a/packages/eql-mapper/src/inference/unifier/types.rs +++ b/packages/eql-mapper/src/inference/unifier/types.rs @@ -215,6 +215,39 @@ pub enum EqlTerm { /// from* ([`crate::JsonSelectorSource`]); the proxy composes and encrypts. #[display("EQL:JsonValueSelector({})", _0)] JsonValueSelector(EqlValue), + + /// The **result** of an encrypted JSON extraction — one SteVec entry pulled + /// out of a document by `->`, `->>`, or `jsonb_path_query{,_first}`. + /// + /// It is deliberately unqueryable: [`EqlTerm::effective_bounds`] gives it no + /// capabilities at all, so no operator or function can require anything of + /// it, and any attempt to traverse it further fails the type check. + /// + /// Why it has to exist. An extracted entry is not a document — it carries no + /// `sv` array — so applying an entry-scoped accessor to it selects nothing + /// and yields NULL. Before this variant, `->` returned the *same* type as + /// the column (`(T -> …) -> T`), which made an extracted entry + /// indistinguishable from the whole document. Nothing in the type system + /// objected to traversing one, and the only thing preventing a wrong answer + /// was a syntactic walker — which a subquery boundary defeats: + /// + /// ```sql + /// SELECT a -> 'foo' FROM (SELECT j -> 'bar' AS a FROM t) s + /// ``` + /// + /// `a` is syntactically far from the `->` that produced it, so the walker + /// cannot compose `$.bar.foo`; it emitted a second entry-scoped accessor + /// over an entry and returned NULL, silently. Carrying "already extracted" + /// in the type is what closes that, because a type crosses a subquery + /// boundary and a syntactic pattern does not. + /// + /// A chain written in one expression is still fused into one path against + /// the root document — see the `Arrow`/`LongArrow` branch in + /// `InferType for Expr`, which consults the chain rather than only the type + /// of its immediate operand. Fusion is why this variant does not simply fall + /// out of the operator declaration. + #[display("EQL:JsonExtracted({})", _0)] + JsonExtracted(EqlValue), } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Display, Hash)] @@ -233,6 +266,8 @@ pub enum EqlTermVariant { JsonOrd, #[display("EQL:JsonValueSelector")] JsonValueSelector, + #[display("EQL:JsonExtracted")] + JsonExtracted, } impl EqlTerm { @@ -250,7 +285,8 @@ impl EqlTerm { | EqlTerm::JsonPath(eql_value) | EqlTerm::Tokenized(eql_value) | EqlTerm::JsonOrd(eql_value) - | EqlTerm::JsonValueSelector(eql_value) => eql_value, + | EqlTerm::JsonValueSelector(eql_value) + | EqlTerm::JsonExtracted(eql_value) => eql_value, } } @@ -263,6 +299,7 @@ impl EqlTerm { EqlTerm::Tokenized(_) => EqlTermVariant::Tokenized, EqlTerm::JsonOrd(_) => EqlTermVariant::JsonOrd, EqlTerm::JsonValueSelector(_) => EqlTermVariant::JsonValueSelector, + EqlTerm::JsonExtracted(_) => EqlTermVariant::JsonExtracted, } } } diff --git a/packages/eql-mapper/src/json_value_selector.rs b/packages/eql-mapper/src/json_value_selector.rs index df9ee6779..83331cb84 100644 --- a/packages/eql-mapper/src/json_value_selector.rs +++ b/packages/eql-mapper/src/json_value_selector.rs @@ -14,57 +14,256 @@ //! [`EqlTerm::JsonValueSelector`]: crate::EqlTerm::JsonValueSelector use std::collections::HashMap; +use std::marker::PhantomData; -use sqltk::parser::ast; +use sqltk::parser::ast::{self}; +use sqltk::parser::ast::{ + BinaryOperator, Expr, FunctionArg, FunctionArgExpr, FunctionArguments, ObjectNamePart, +}; use sqltk::NodeKey; use crate::Param; -/// Where the JSON path half of a fused value selector comes from. +/// One step of the path half of a fused value selector. /// -/// The two halves are independently a literal or a placeholder, so all four -/// combinations occur (`-> 'a' = '1'`, `-> $1 = $2`, `-> 'a' = $1`, …). A -/// literal path is fully known at type-check time and is carried inline; a -/// placeholder path is only known at Bind, so its param number is carried -/// instead. +/// A step is independently a literal or a placeholder, so all combinations +/// occur (`-> 'a' = '1'`, `-> $1 = $2`, `-> 'a' -> $1 = $2`, …). A literal step +/// is fully known at type-check time and is carried inline; a placeholder step +/// is only known at Bind, so its param number is carried instead. #[derive(Debug, Clone, PartialEq, Eq)] -pub enum JsonSelectorSource { - /// A SQL literal path (`col -> 'name' = …`) — the selector text itself. +pub enum JsonSelectorSegment { + /// A SQL literal selector (`col -> 'name' = …`) — the selector text itself. Literal(String), - /// A placeholder path (`col -> $1 = …`) — the param it will arrive in. + /// A placeholder selector (`col -> $1 = …`) — the param it arrives in. Param(Param), } -/// The set of fused JSON value selectors in a statement: for each operand that -/// carries the *value* half, where its *path* half comes from. +/// Where the JSON path half of a fused value selector comes from. +/// +/// The path is a **sequence** of steps, because an accessor chain is a single +/// path: `col -> 'a' -> 'b' = value` selects `$.a.b` of the whole document, not +/// `$.b` of some intermediate one. Nothing between the column and the value is +/// a jsonb value the database could operate on — the payload is encrypted — so +/// the whole chain has to collapse into one path, composed here and resolved to +/// text by the proxy once every placeholder step is bound. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct JsonSelectorSource { + segments: Vec, +} + +impl JsonSelectorSource { + pub(crate) fn new(segments: Vec) -> Self { + Self { segments } + } + + /// A single-step literal path. + pub fn literal(path: impl Into) -> Self { + Self::new(vec![JsonSelectorSegment::Literal(path.into())]) + } + + /// A single-step placeholder path. + pub fn param(param: Param) -> Self { + Self::new(vec![JsonSelectorSegment::Param(param)]) + } + + /// The steps of the path, outermost accessor last: `col -> 'a' -> 'b'` is + /// `["a", "b"]`, which composes to `$.a.b`. + pub fn segments(&self) -> &[JsonSelectorSegment] { + &self.segments + } + + /// Every input param the path consumes. + pub fn params(&self) -> impl Iterator + '_ { + self.segments.iter().filter_map(|segment| match segment { + JsonSelectorSegment::Param(param) => Some(*param), + JsonSelectorSegment::Literal(_) => None, + }) + } +} + +/// Decomposes a JSON field-access chain into the expression it is rooted at and +/// the selectors applied to it, outermost last. +/// +/// Recognises both spellings at every step, so a chain may mix them: +/// +/// - `col -> sel`, `col ->> sel` (and the `eql_v3."->"(col, sel)` form the +/// containment rule rewrites them to) +/// - `jsonb_path_query_first(col, sel)`, `jsonb_path_query(col, sel)` +/// +/// Returns `None` for anything that is not a field access — importantly for a +/// bare column, which is a whole document rather than a field of one. +/// +/// Parentheses are transparent at every position. `(col -> 'a') -> 'b'` is the +/// same query as `col -> 'a' -> 'b'` and must decompose to the same root and the +/// same path: a chain the walker fails to see through is not merely unfused, it +/// is a leak. The unseen inner accessor gets treated as the root container, so +/// its selector reaches PostgreSQL in cleartext and native jsonb `->` is applied +/// to an encrypted payload. +pub(crate) fn json_accessor_chain(expr: &Expr) -> Option<(&Expr, Vec<&Expr>)> { + let mut selectors = Vec::new(); + let mut container = unnest(expr); + + while let Some((inner, selector)) = json_accessor(container) { + selectors.push(unnest(selector)); + container = unnest(inner); + } + + if selectors.is_empty() { + return None; + } + + selectors.reverse(); + + Some((container, selectors)) +} + +/// Strips redundant parentheses, however many deep. +/// +/// `Expr::Nested` carries no meaning of its own — it records that the author +/// wrote brackets. Every consumer of a chain wants the expression inside them. +pub(crate) fn unnest(expr: &Expr) -> &Expr { + let mut current = expr; + + while let Expr::Nested(inner) = current { + current = inner; + } + + current +} + +/// One step of a field access: `(container, selector)`. +pub(crate) fn json_accessor(expr: &Expr) -> Option<(&Expr, &Expr)> { + match expr { + Expr::BinaryOp { + left, + op: BinaryOperator::Arrow | BinaryOperator::LongArrow, + right, + } => Some((&**left, &**right)), + + Expr::Function(function) if is_json_accessor_fn(&function.name) => match &function.args { + FunctionArguments::List(list) => match list.args.as_slice() { + [FunctionArg::Unnamed(FunctionArgExpr::Expr(container)), FunctionArg::Unnamed(FunctionArgExpr::Expr(selector))] => { + Some((container, selector)) + } + _ => None, + }, + _ => None, + }, + + _ => None, + } +} + +/// Whether a function call is a JSON field access. +/// +/// Matched on the bare function name, so every schema spelling counts — the +/// client's `pg_catalog.jsonb_path_query_first`, the `eql_v3.` twin, and the +/// `eql_v3."->"` form the containment rewrite produces. Names outside this set +/// are NOT accessors: an arbitrary two-argument call over an encrypted JSON +/// value (`coalesce(a, b)`) would otherwise be mistaken for one and its second +/// argument read as a selector. +fn is_json_accessor_fn(name: &ast::ObjectName) -> bool { + let Some(ObjectNamePart::Identifier(ident)) = name.0.last() else { + return false; + }; + + matches!( + ident.value.to_lowercase().as_str(), + "jsonb_path_query" | "jsonb_path_query_first" | "->" | "->>" + ) +} + +/// A second, *different* path recorded against the same operand. +/// +/// The map is keyed by operand, so one param used as the outermost selector of +/// two chains with different prefixes (`SELECT j -> 'a' -> $1, j -> 'b' -> $1`) +/// would silently overwrite one path with the other and answer one of the two +/// projections from the wrong field. There is no key that could tell them apart: +/// at Bind time the proxy has only the param number. +#[derive(Debug)] +pub struct ConflictingSelectorPath; + +/// Role marker: the path half of a fused equality needle, which the proxy MACs +/// together with a value (`QueryOp::SteVecValueSelector`). +#[derive(Debug, Default)] +pub struct FusedValue; + +/// Role marker: the composed path of a collapsed accessor chain, which the proxy +/// MACs on its own (`QueryOp::SteVecSelector`). +#[derive(Debug, Default)] +pub struct AccessorChain; + +/// For each operand that carries a JSON selector, where the *path* it keys comes +/// from. /// /// Keyed separately for the two protocols the proxy has to serve — params are /// addressed by number (the extended protocol has no AST at Bind time), /// literals by AST node. #[derive(Debug, Default)] -pub struct JsonValueSelectors<'ast> { +pub struct JsonSelectorSources<'ast, Role> { by_param: HashMap, by_literal: HashMap, JsonSelectorSource>, + _role: PhantomData, } -impl<'ast> JsonValueSelectors<'ast> { - pub(crate) fn record_param(&mut self, param: Param, source: JsonSelectorSource) { - self.by_param.insert(param, source); +/// The fused JSON value selectors in a statement: for each operand that carries +/// the *value* half of an exact JSON equality, where its *path* half comes from. +/// +/// The path operand is dropped from the rewritten SQL — the proxy MACs path and +/// value together into one needle — so this is the only record of it. +pub type JsonValueSelectors<'ast> = JsonSelectorSources<'ast, FusedValue>; + +/// The composed paths of collapsed accessor chains: for each *selector* operand +/// that survives a multi-step chain, every step of the path it must key. +/// +/// `j -> 'a' -> 'b'` emits the single accessor `eql_v3."->"(j, )`, where +/// `` is the outermost selector operand and the path it keys is `$.a.b` — +/// the whole chain, not the one segment its own text spells. The inner +/// accessors are dropped from the SQL, so without this record the proxy would +/// key `$.b` and select nothing. +/// +/// Distinct from [`JsonValueSelectors`] because the two produce different +/// encryption ops: a bare selector (`QueryOp::SteVecSelector`) against a +/// path-and-value needle (`QueryOp::SteVecValueSelector`). +pub type JsonAccessorPaths<'ast> = JsonSelectorSources<'ast, AccessorChain>; + +impl<'ast, Role> JsonSelectorSources<'ast, Role> { + /// Records the path for an operand arriving in `param`. + /// + /// Recording the *same* path twice is fine — the same param may legitimately + /// select the same path in several places. A different one is not: see + /// [`ConflictingSelectorPath`]. + pub(crate) fn record_param( + &mut self, + param: Param, + source: JsonSelectorSource, + ) -> Result<(), ConflictingSelectorPath> { + match self.by_param.get(¶m) { + Some(existing) if *existing != source => Err(ConflictingSelectorPath), + _ => { + self.by_param.insert(param, source); + Ok(()) + } + } } + /// Records the path for a literal operand. + /// + /// Unlike a param, a literal is keyed by AST *node* identity, so two + /// occurrences of the same text are distinct keys and cannot conflict. pub(crate) fn record_literal(&mut self, node: &'ast ast::Value, source: JsonSelectorSource) { self.by_literal.insert(NodeKey::new(node), source); } - /// The path source for the value-selector operand bound to `param`, or - /// `None` if that param is not one. + /// The path source for the operand bound to `param`, or `None` if that param + /// is not one. pub fn for_param(&self, param: Param) -> Option<&JsonSelectorSource> { self.by_param.get(¶m) } - /// The path source for the value-selector operand at literal `node`, or - /// `None` if that literal is not one. + /// The path source for the operand at literal `node`, or `None` if that + /// literal is not one. pub fn for_literal(&self, node: &'ast ast::Value) -> Option<&JsonSelectorSource> { self.by_literal.get(&NodeKey::new(node)) } @@ -73,3 +272,61 @@ impl<'ast> JsonValueSelectors<'ast> { self.by_param.is_empty() && self.by_literal.is_empty() } } + +#[cfg(test)] +mod tests { + use super::json_accessor_chain; + use sqltk::parser::{dialect::PostgreSqlDialect, parser::Parser}; + + /// `(root, selectors)` of the chain in `sql`, rendered as SQL text. + fn chain_of(sql: &str) -> Option<(String, Vec)> { + let expr = Parser::new(&PostgreSqlDialect {}) + .try_with_sql(sql) + .unwrap() + .parse_expr() + .unwrap(); + + json_accessor_chain(&expr).map(|(container, selectors)| { + ( + container.to_string(), + selectors.iter().map(|s| s.to_string()).collect(), + ) + }) + } + + #[test] + fn a_chain_yields_its_root_and_every_selector_in_order() { + assert_eq!( + chain_of("j -> 'a' -> 'b' -> 'c'"), + Some(( + "j".to_owned(), + vec!["'a'".to_owned(), "'b'".to_owned(), "'c'".to_owned()] + )) + ); + } + + #[test] + fn a_chain_may_mix_spellings() { + assert_eq!( + chain_of("jsonb_path_query_first(j, '$.a') ->> $1"), + Some(("j".to_owned(), vec!["'$.a'".to_owned(), "$1".to_owned()])) + ); + } + + #[test] + fn a_bare_column_is_not_a_field_access() { + assert_eq!(chain_of("j"), None); + } + + /// A two-argument call that is not an accessor must stop the walk, or its + /// second argument would be read as a selector and the call itself stripped + /// from the statement. + #[test] + fn a_non_accessor_call_is_the_root_not_a_step() { + assert_eq!( + chain_of("coalesce(j, k) -> 'a'"), + Some(("coalesce(j, k)".to_owned(), vec!["'a'".to_owned()])) + ); + assert_eq!(chain_of("coalesce(j, k)"), None); + } +} diff --git a/packages/eql-mapper/src/lib.rs b/packages/eql-mapper/src/lib.rs index b9fba1128..7cae3d07e 100644 --- a/packages/eql-mapper/src/lib.rs +++ b/packages/eql-mapper/src/lib.rs @@ -47,7 +47,8 @@ mod test { EqlTerm, EqlTrait, EqlTraits, EqlValue, InstantiateType, NativeValue, Projection, ProjectionColumn, Type, Value, }, - JsonSelectorSource, OutputParamSource, Param, Schema, TableColumn, TableResolver, + JsonSelectorSegment, JsonSelectorSource, OutputParamSource, Param, Schema, TableColumn, + TableResolver, TypeCheckedStatement, }; use eql_mapper_macros::concrete_ty; use pretty_assertions::assert_eq; @@ -2598,7 +2599,7 @@ mod test { assert_eq!( transformed.params.outputs()[0].source, OutputParamSource::JsonValueSelector { - path: JsonSelectorSource::Param(Param(1)), + path: JsonSelectorSource::param(Param(1)), value: Param(2), } ); @@ -2642,7 +2643,7 @@ mod test { assert_eq!( outputs[1].source, OutputParamSource::JsonValueSelector { - path: JsonSelectorSource::Param(Param(2)), + path: JsonSelectorSource::param(Param(2)), value: Param(3), } ); @@ -3225,12 +3226,11 @@ mod test { }) } - /// Transforms `sql` with every encrypted literal replaced by `''`. - fn transform_with_dummy_literals(schema: Arc, sql: &str) -> String { - let statement = parse(sql); - let typed = type_check(schema, &statement).unwrap(); - - let encrypted = typed + /// Every encrypted literal of `typed`, replaced by `''`. + fn dummy_encrypted_literals<'ast>( + typed: &TypeCheckedStatement<'ast>, + ) -> HashMap, ast::Value> { + typed .literals .iter() .map(|(_, v)| { @@ -3239,7 +3239,14 @@ mod test { ast::Value::SingleQuotedString("".to_string()), ) }) - .collect::>(); + .collect() + } + + /// Transforms `sql` with every encrypted literal replaced by `''`. + fn transform_with_dummy_literals(schema: Arc, sql: &str) -> String { + let statement = parse(sql); + let typed = type_check(schema, &statement).unwrap(); + let encrypted = dummy_encrypted_literals(&typed); typed.transform(encrypted).unwrap().to_string() } @@ -3377,32 +3384,39 @@ mod test { assert_eq!(vec![false, true], roles); } - /// A chained JSON accessor must not leave the intermediate selector in the - /// statement, nor apply native `->` to the encrypted payload. - /// - /// The container is cloned from the *original* AST, so `-> 'nested'` - /// survives untouched: the plaintext field name ships in the SQL text and - /// native jsonb `->` runs on the encrypted column, which also makes the - /// predicate match nothing. - #[test] - #[ignore = "Chained JSON accessor clones its container from the original AST, so the inner \ - selector stays plaintext in the SQL and native jsonb -> is applied to the \ - encrypted payload. See rewrite_json_value_selector_eq.rs."] - fn chained_json_accessor_does_not_emit_the_plaintext_selector() { - let schema = resolver(schema! { + /// A column that can be both traversed and compared for JSON equality. + fn chained_json_schema() -> Arc { + resolver(schema! { tables: { t: { id, j (EQL("eql_v3_json_search"): Eq + Ord + JsonLike + Contain), } } - }); + }) + } + /// A chained JSON accessor must not leave the intermediate selector in the + /// statement, nor apply native `->` to the encrypted payload. + /// + /// The chain collapses into a single containment against the ROOT column: + /// `j -> 'nested' -> 'string'` is the path `$.nested.string` of one + /// document, and the needle is keyed on that whole path. Keeping the inner + /// accessor (the container was cloned from the original AST) shipped the + /// plaintext field name in the SQL text AND ran native jsonb `->` over an + /// encrypted payload, so the predicate matched nothing either. + #[test] + fn chained_json_accessor_does_not_emit_the_plaintext_selector() { let rewritten = transform_with_dummy_literals( - schema, + chained_json_schema(), "SELECT id FROM t WHERE j -> 'nested' -> 'string' = '\"world\"'", ); + assert_eq!( + rewritten, + "SELECT id FROM t WHERE eql_v3.jsonb_contains(j, ''::JSONB::eql_v3.query_json)" + ); + assert!( !rewritten.contains("'nested'"), "the intermediate selector must not reach the database in plaintext: {rewritten}" @@ -3413,6 +3427,466 @@ mod test { ); } + /// A JSON operation on the RESULT of a JSON operation must be refused when + /// the two are not in the same expression. + /// + /// `->` yields `EqlTerm::JsonExtracted` — one SteVec entry, which carries no + /// `sv` array and so cannot be traversed. A chain written in one expression + /// is collapsed into a single path against the document instead, but once the + /// halves are separated by a subquery the selectors cannot be composed: the + /// walker sees only `a -> 'foo'` and has no way to learn that `a` is already + /// `$.bar`. It used to emit a second entry-scoped accessor over an entry and + /// return NULL, silently, or fuse a needle keyed on `$.foo` when the real + /// path was `$.bar.foo` — wrong rows, no error (CIP-3682). + /// + /// This one is impossible rather than unimplemented, and stays refused even + /// though a chain in one expression now works everywhere: `JsonExtracted` does + /// not carry the path that produced it, and the root column is not in scope in + /// the outer query, so there is nothing to root a composed path at. + /// + /// A type crosses a subquery boundary where a syntactic pattern does not, + /// which is why this is carried in the type system rather than by the walker. + #[test] + fn json_operation_on_an_extracted_value_is_refused() { + let schema = chained_json_schema(); + + for sql in [ + // The reported shape: the chain split across a subquery. + "SELECT a -> 'foo' FROM (SELECT j -> 'bar' AS a FROM t) s", + // The same split, but where the outer half is a fusable predicate. + // The fusion must NOT claim this: its root is an entry, not a + // document, so the path it would compose is wrong. + "SELECT id FROM (SELECT j -> 'bar' AS a, id FROM t) s WHERE a -> 'foo' = '\"x\"'", + // The `->>` spelling is the same operation. + "SELECT a ->> 'foo' FROM (SELECT j -> 'bar' AS a FROM t) s", + ] { + let statement = parse(sql); + let err = type_check(schema.clone(), &statement) + .expect_err(&format!("`{sql}` must not type check")); + + assert!( + err.to_string() + .contains("result of an encrypted JSON operation"), + "expected an unqueryable-extraction error for `{sql}`, got: {err}" + ); + } + } + + /// A multi-step chain collapses to a SINGLE accessor on the root document, + /// in every context — not only under an equality. + /// + /// A chain cannot be two hops: `eql_v3."->"` searches the document's `sv` + /// array and returns one entry, which has no `sv` of its own, so an accessor + /// over an accessor finds nothing and returns NULL. The emission that IS + /// correct is one accessor carrying the composed path, and it is correct + /// wherever a single access is — so a projection, an ordering comparison and a + /// mixed spelling all produce exactly the shape the equivalent single access + /// would. + /// + /// The plaintext selectors of the discarded inner accessors must go with them: + /// leaving one behind ships a field name to PostgreSQL in the clear + /// (CIP-3682) and applies native jsonb `->` to an encrypted payload. + #[test] + fn a_multi_step_chain_collapses_to_one_accessor_in_every_context() { + let schema = chained_json_schema(); + + // Each case pairs a chain with the emission expected of it. The selector + // is `''` in every one: the whole path is keyed into that single + // encrypted operand, so the SQL cannot show how many steps there were. + for (sql, expected) in [ + // A projection, which has no comparison to fuse into. + ( + "SELECT j -> 'foo' -> 'bar' FROM t", + "SELECT eql_v3.\"->\"(j, '') FROM t", + ), + // Brackets are not meaning: the same query, the same emission. + ( + "SELECT (j -> 'foo') -> 'bar' FROM t", + "SELECT eql_v3.\"->\"(j, '') FROM t", + ), + // Mixed spellings. The OUTERMOST step decides the call, exactly as it + // would for a single access: `->>` yields text. + ( + "SELECT j -> 'foo' ->> 'bar' FROM t", + "SELECT eql_v3.\"->>\"(j, '') FROM t", + ), + // Depth beyond two. + ( + "SELECT j -> 'a' -> 'b' -> 'c' FROM t", + "SELECT eql_v3.\"->\"(j, '') FROM t", + ), + // The function spelling as a step of the chain. + ( + "SELECT jsonb_path_query_first(j, '$.a') -> 'b' FROM t", + "SELECT eql_v3.\"->\"(j, '') FROM t", + ), + // Ordering. The accessor survives here — the comparison wraps it in + // `ord_term` rather than absorbing it — so it must be the collapsed + // single-accessor form. + ( + "SELECT id FROM t WHERE j -> 'foo' -> 'bar' < '\"x\"'", + "SELECT id FROM t WHERE eql_v3.ord_term(eql_v3.\"->\"(j, '')) < \ + eql_v3.ord_term(''::JSONB::eql_v3.query_integer_ord)", + ), + ( + "SELECT id FROM t WHERE j -> 'foo' -> 'bar' >= '\"x\"'", + "SELECT id FROM t WHERE eql_v3.ord_term(eql_v3.\"->\"(j, '')) >= \ + eql_v3.ord_term(''::JSONB::eql_v3.query_integer_ord)", + ), + ] { + let rewritten = transform_with_dummy_literals(schema.clone(), sql); + + assert_eq!(rewritten, expected, "unexpected rewrite for `{sql}`"); + + for selector in ["'foo'", "'bar'", "'a'", "'b'", "'$.a'"] { + assert!( + !rewritten.contains(selector), + "selector {selector} must not reach the database in plaintext for `{sql}`: {rewritten}" + ); + } + assert!( + !rewritten.contains("j -> "), + "native jsonb -> must not be applied to the encrypted column for `{sql}`: {rewritten}" + ); + } + } + + /// A chain collapsed outside an equality records its composed path against + /// the SURVIVING selector operand, which is the outermost step. + /// + /// That operand's own text is one segment (`'c'`); the selector it must key is + /// the whole path (`$.a.<$1>.c`). Nothing the proxy is handed at encryption + /// time could recover the difference, so the record is the only way the inner + /// steps reach the needle — and every step is an input the plan must consume, + /// or the client would bind a param that goes nowhere. + #[test] + fn a_collapsed_chain_records_its_composed_path_against_the_surviving_selector() { + // The outermost step is the param, so the whole path resolves at Bind. + let statement = parse("SELECT j -> 'a' -> $1 FROM t"); + + let typed = type_check(chained_json_schema(), &statement).unwrap(); + let transformed = typed.transform(dummy_encrypted_literals(&typed)).unwrap(); + + assert_eq!( + transformed.to_string(), + "SELECT eql_v3.\"->\"(j, $1) FROM t" + ); + + let source = OutputParamSource::JsonAccessorPath { + path: JsonSelectorSource::new(vec![ + JsonSelectorSegment::Literal("a".to_owned()), + JsonSelectorSegment::Param(Param(1)), + ]), + selector: Param(1), + }; + + assert_eq!(transformed.params.outputs()[0].source, source); + assert_eq!(source.inputs(), vec![Param(1)]); + + // The selector is a query operand: it reaches PostgreSQL as a search term + // and never as a decryptable ciphertext. + assert!(transformed.params.outputs()[0].query_operand); + } + + /// An EQUALITY over a chain must keep fusing, not degrade to an accessor plus + /// a comparison. + /// + /// The fused needle keys the path and the value together into one MAC, and its + /// presence in the stored `sv` IS the match. An accessor followed by `eq_term` + /// would be two operations where one suffices, and `eql_v3.eq_term` has no + /// overload for a JSON query operand anyway. The chain-collapsing rule fires + /// on the accessor below the comparison, and the equality rule then discards + /// its result and re-roots the containment at the bare column. + #[test] + fn equality_over_a_chain_still_fuses_rather_than_collapsing_to_an_accessor() { + let schema = chained_json_schema(); + + for (sql, expected) in [ + ( + "SELECT id FROM t WHERE j -> 'a' -> 'b' = '\"v\"'", + "SELECT id FROM t WHERE \ + eql_v3.jsonb_contains(j, ''::JSONB::eql_v3.query_json)", + ), + ( + "SELECT id FROM t WHERE j -> 'a' -> 'b' <> '\"v\"'", + "SELECT id FROM t WHERE \ + NOT (eql_v3.jsonb_contains(j, ''::JSONB::eql_v3.query_json))", + ), + ] { + let rewritten = transform_with_dummy_literals(schema.clone(), sql); + + assert_eq!(rewritten, expected, "unexpected rewrite for `{sql}`"); + assert!( + !rewritten.contains("eql_v3.\"->\""), + "equality must fuse, not emit an accessor, for `{sql}`: {rewritten}" + ); + } + } + + /// A fused equality records its path in the value-selector channel ONLY. + /// + /// Recording it in both would be worse than recording it in neither: the + /// accessor channel is resolved at Parse time for a literal operand, and + /// `j -> $1 -> 'b' = $2` has a placeholder step in front of a literal + /// selector, which cannot resolve then. Writing the path to both channels + /// would refuse a query that works. + #[test] + fn a_fused_chain_records_no_accessor_path() { + let statement = parse("SELECT id FROM t WHERE j -> 'a' -> 'b' = $1"); + let typed = type_check(chained_json_schema(), &statement).unwrap(); + + assert!( + typed.json_accessor_paths.is_empty(), + "a chain the equality absorbs has no surviving selector to key" + ); + assert!(!typed.json_value_selectors.is_empty()); + } + + /// Every step of a collapsed chain must be resolvable to path text. + /// + /// The chain is collapsed either way, so a step the proxy cannot resolve — a + /// column reference, a function call — would simply vanish from the statement + /// and the query would read a different field. Unlike the fused-equality case + /// there is no capability check to fall through to, so it is refused outright. + #[test] + fn a_collapsed_chain_with_an_unresolvable_step_is_refused() { + let statement = parse("SELECT j -> 'a' -> id FROM t"); + + let err = type_check(chained_json_schema(), &statement) + .expect_err("a path step that is not a literal or a placeholder must be refused"); + + assert!( + err.to_string() + .contains("must be a literal or a placeholder"), + "expected an uncomposable-path error, got: {err}" + ); + } + + /// One placeholder cannot be the selector of two chains with different paths. + /// + /// The path is recorded against the param it arrives in, because at Bind time + /// the param number is all the proxy has. Two different paths for one param + /// cannot both be honoured, and silently keeping either would answer one of + /// the two projections from the wrong field. + #[test] + fn one_placeholder_cannot_key_two_different_paths() { + let statement = parse("SELECT j -> 'a' -> $1, j -> 'b' -> $1 FROM t"); + + let err = type_check(chained_json_schema(), &statement) + .expect_err("one param cannot carry two different paths"); + + assert!( + err.to_string().contains("two different"), + "expected an ambiguous-path error, got: {err}" + ); + + // The same path twice is not a conflict — it is one path. + let statement = parse("SELECT j -> 'a' -> $1, j -> 'a' -> $1 FROM t"); + type_check(chained_json_schema(), &statement).unwrap(); + } + + /// Sorting by an extracted JSON field sorts by its ordering term. + /// + /// An extracted SteVec entry carries ordering and equality terms, so + /// `ORDER BY col -> 'field'` is legitimate — it sorts by + /// `ord_term(eql_v3."->"(col, sel))`. This pins the capability grant on + /// `EqlTerm::JsonExtracted`: when it briefly had NO capabilities at all, + /// this exact shape failed the `Ord` bound, and with mapping errors + /// disabled (the container default) the statement was forwarded unmapped — + /// native `->` over ciphertext, zero rows, silently. The showcase's + /// "active Aspirin prescriptions" query was the first thing to notice. + #[test] + fn order_by_an_extracted_json_field_sorts_by_its_ordering_term() { + let rewritten = transform_with_dummy_literals( + chained_json_schema(), + "SELECT id FROM t ORDER BY j -> 'email'", + ); + + assert_eq!( + rewritten, + "SELECT id FROM t ORDER BY eql_v3.ord_term(eql_v3.\"->\"(j, ''))" + ); + } + + /// A SINGLE access is legal anywhere, fused or not. + /// + /// The declaration handles it: `-> ::Output` yields an + /// extracted entry, which is projectable and decryptable. Only *traversing* + /// that result is refused, so ordinary field access and single-field + /// comparisons are unaffected. + #[test] + fn a_single_json_access_is_legal_anywhere() { + let schema = chained_json_schema(); + + for sql in [ + "SELECT j -> 'foo' FROM t", + "SELECT j ->> 'foo' FROM t", + "SELECT id FROM t WHERE j -> 'foo' = '\"x\"'", + "SELECT id FROM t WHERE j -> 'foo' < '\"x\"'", + ] { + let statement = parse(sql); + type_check(schema.clone(), &statement) + .unwrap_or_else(|e| panic!("`{sql}` should type check, got: {e}")); + } + } + + /// Extracting one field, and projecting an extracted field, both still work. + /// + /// The point of `JsonExtracted` is to forbid *traversing* an extracted entry, + /// not to make extraction useless: a single access is the common case, and + /// the result is projectable and decryptable exactly as before. + #[test] + fn extracting_and_projecting_one_json_field_still_works() { + let schema = chained_json_schema(); + + assert_eq!( + transform_with_dummy_literals(schema.clone(), "SELECT j -> 'foo' FROM t"), + "SELECT eql_v3.\"->\"(j, '') FROM t" + ); + + // An extracted entry crossing a subquery boundary is fine as long as + // nothing traverses it on the far side. + assert_eq!( + transform_with_dummy_literals( + schema, + "SELECT a FROM (SELECT j -> 'bar' AS a FROM t) s" + ), + "SELECT a FROM (SELECT eql_v3.\"->\"(j, '') AS a FROM t) AS s" + ); + } + + /// Parentheses must not defeat the chain walker. + /// + /// `(j -> 'foo') -> 'bar'` is `j -> 'foo' -> 'bar'` with redundant brackets, + /// and must fuse to the same needle rooted at the same bare column. Before + /// the walker saw through `Expr::Nested` it stopped at the bracket, treated + /// the parenthesised accessor as the ROOT container, and emitted + /// `eql_v3.jsonb_contains((j -> 'foo'), …)` — shipping the plaintext selector + /// `'foo'` to PostgreSQL and applying native jsonb `->` to the encrypted + /// payload. The same CIP-3682 leak, reachable with one pair of brackets. + #[test] + fn parenthesised_json_accessor_chains_fuse_identically() { + let schema = chained_json_schema(); + + // Every spelling below is the same query, so every one must produce the + // same fused containment against the bare root column. + let expected = + "SELECT id FROM t WHERE eql_v3.jsonb_contains(j, ''::JSONB::eql_v3.query_json)"; + + for sql in [ + "SELECT id FROM t WHERE j -> 'foo' -> 'bar' = '\"x\"'", + "SELECT id FROM t WHERE (j -> 'foo') -> 'bar' = '\"x\"'", + "SELECT id FROM t WHERE ((j -> 'foo') -> 'bar') = '\"x\"'", + "SELECT id FROM t WHERE (((j -> 'foo')) -> 'bar') = '\"x\"'", + "SELECT id FROM t WHERE (j) -> 'foo' -> 'bar' = '\"x\"'", + "SELECT id FROM t WHERE j -> ('foo') -> 'bar' = '\"x\"'", + ] { + let rewritten = transform_with_dummy_literals(schema.clone(), sql); + + assert_eq!(rewritten, expected, "unexpected rewrite for `{sql}`"); + + assert!( + !rewritten.contains("'foo'"), + "the intermediate selector must not reach the database in plaintext for `{sql}`: {rewritten}" + ); + assert!( + !rewritten.contains("j -> "), + "native jsonb -> must not be applied to the encrypted column for `{sql}`: {rewritten}" + ); + } + } + + /// Every spelling and depth of chain collapses the same way, and none of + /// them leaves a selector behind. + #[test] + fn chained_json_accessor_spellings_all_collapse_to_root_containment() { + let cases = [ + // Depth 3, and deeper. + "j -> 'a' -> 'b' -> 'c' = '\"v\"'", + "j -> 'a' -> 'b' -> 'c' -> 'd' = '\"v\"'", + // The `->>` spelling, and mixed with `->`. + "j ->> 'a' = '\"v\"'", + "j -> 'a' ->> 'b' = '\"v\"'", + "j ->> 'a' ->> 'b' = '\"v\"'", + // The function spelling, rooted and chained. + "jsonb_path_query_first(j, '$.a') = '\"v\"'", + "jsonb_path_query_first(j, '$.a') -> 'b' = '\"v\"'", + // The value operand written on the left. + "'\"v\"' = j -> 'a' -> 'b'", + ]; + + for case in cases { + let rewritten = transform_with_dummy_literals( + chained_json_schema(), + &format!("SELECT id FROM t WHERE {case}"), + ); + + assert_eq!( + rewritten, + "SELECT id FROM t WHERE eql_v3.jsonb_contains(j, ''::JSONB::eql_v3.query_json)", + "unexpected rewrite for `{case}`" + ); + } + } + + /// `<>` on a chain is the same containment, negated — the selectors are + /// discarded there too. + #[test] + fn chained_json_accessor_not_eq_rewrites_to_negated_containment() { + let rewritten = transform_with_dummy_literals( + chained_json_schema(), + "SELECT id FROM t WHERE j -> 'nested' -> 'string' <> '\"world\"'", + ); + + assert_eq!( + rewritten, + "SELECT id FROM t WHERE NOT (eql_v3.jsonb_contains(j, ''::JSONB::eql_v3.query_json))" + ); + } + + /// The path a chain composes is recorded step by step, so the proxy can + /// build `$.a.<$1>.c` once the placeholder steps are bound. Every step is an + /// input the plan must consume — dropping one would leave the client binding + /// a param that never reaches the needle. + #[test] + fn chained_json_accessor_records_every_path_step() { + let statement = parse("SELECT id FROM t WHERE j -> 'a' -> $1 -> 'c' = $2"); + + let typed = type_check(chained_json_schema(), &statement).unwrap(); + let transformed = typed.transform(dummy_encrypted_literals(&typed)).unwrap(); + + assert_eq!( + transformed.to_string(), + "SELECT id FROM t WHERE eql_v3.jsonb_contains(j, $1::JSONB::eql_v3.query_json)" + ); + + let source = OutputParamSource::JsonValueSelector { + path: JsonSelectorSource::new(vec![ + JsonSelectorSegment::Literal("a".to_owned()), + JsonSelectorSegment::Param(Param(1)), + JsonSelectorSegment::Literal("c".to_owned()), + ]), + value: Param(2), + }; + + assert_eq!(transformed.params.outputs()[0].source, source); + assert_eq!(source.inputs(), vec![Param(1), Param(2)]); + } + + /// A chain with a step that is neither a literal nor a placeholder cannot + /// be composed into a path, so the fusion is declined and the comparison + /// falls through to the ordinary capability check — an error, not a + /// half-built needle and not a leak. + #[test] + fn chained_json_accessor_with_an_unresolvable_step_is_rejected() { + let statement = parse("SELECT id FROM t WHERE j -> 'a' -> id = '\"v\"'"); + + assert!( + type_check(chained_json_schema(), &statement).is_err(), + "a path step that is not a literal or a placeholder must not fuse" + ); + } + /// JSON field access requires the column to support field selection. #[test] fn json_field_access_requires_json_like() { @@ -3842,6 +4316,17 @@ mod test { .map_err(|err| err.to_string()) .unwrap(); + // A path query still yields the column's own type, NOT `JsonExtracted`. + // + // `->`/`->>` return `::Output` so a second traversal of an + // encrypted result is refused. The path-query functions deliberately do + // NOT, because two supported shapes depend on the old typing: + // `jsonb_array_elements`/`jsonb_array_length` consume an extracted entry + // rather than traversing it, and the rewrite that retargets these + // functions and encrypts their Path operand keys off the result type — + // changing it sent the caller's literal jsonpath to PostgreSQL + // unencrypted. Closing that needs the array functions taught to accept + // an extracted value first. assert_eq!( typed.projection, projection![(EQL(patients.notes: JsonLike) as notes)] diff --git a/packages/eql-mapper/src/param_plan.rs b/packages/eql-mapper/src/param_plan.rs index e90202007..50589ae21 100644 --- a/packages/eql-mapper/src/param_plan.rs +++ b/packages/eql-mapper/src/param_plan.rs @@ -25,15 +25,27 @@ pub enum OutputParamSource { /// encrypted if the param is EQL-typed). Input(Param), - /// Fused from two operands into one encrypted value-selector needle: the - /// JSON path and the value it must equal. The path may itself be a param or - /// a literal in the SQL; the value is always the param carrying this output. + /// Fused from several operands into one encrypted value-selector needle: + /// the JSON path and the value it must equal. Each step of the path is + /// itself a param or a literal in the SQL; the value is always the param + /// carrying this output. /// /// See [`crate::JsonValueSelectors`]. JsonValueSelector { path: JsonSelectorSource, value: Param, }, + + /// The composed path of a collapsed multi-step accessor chain. The whole + /// value of this output param is the eJSONPath the chain traverses, built + /// from every step of it — of which `selector`, the param this output + /// occupies, supplies only the outermost. + /// + /// See [`crate::JsonAccessorPaths`]. + JsonAccessorPath { + path: JsonSelectorSource, + selector: Param, + }, } impl OutputParamSource { @@ -41,10 +53,20 @@ impl OutputParamSource { pub fn inputs(&self) -> Vec { match self { OutputParamSource::Input(param) => vec![*param], - OutputParamSource::JsonValueSelector { path, value } => match path { - JsonSelectorSource::Param(path_param) => vec![*path_param, *value], - JsonSelectorSource::Literal(_) => vec![*value], - }, + OutputParamSource::JsonValueSelector { path, value } => { + path.params().chain([*value]).collect() + } + OutputParamSource::JsonAccessorPath { path, selector } => { + // The selector is the chain's outermost step, so it is normally + // already among the path's params. Added explicitly rather than + // relying on that, and deduplicated so this reads as the set it + // is. + let mut inputs: Vec = path.params().collect(); + if !inputs.contains(selector) { + inputs.push(*selector); + } + inputs + } } } } diff --git a/packages/eql-mapper/src/transformation_rules/collapse_json_accessor_chain.rs b/packages/eql-mapper/src/transformation_rules/collapse_json_accessor_chain.rs new file mode 100644 index 000000000..033b9bfae --- /dev/null +++ b/packages/eql-mapper/src/transformation_rules/collapse_json_accessor_chain.rs @@ -0,0 +1,185 @@ +use std::collections::HashMap; +use std::mem; +use std::sync::Arc; + +use sqltk::parser::ast::Value as SqltkValue; +use sqltk::parser::ast::{ + BinaryOperator, Expr, Function, FunctionArg, FunctionArgExpr, FunctionArgumentList, + FunctionArguments, Ident, ObjectName, ObjectNamePart, ValueWithSpan, +}; +use sqltk::parser::tokenizer::Span; +use sqltk::{NodeKey, NodePath, Visitable}; + +use crate::json_value_selector::json_accessor_chain; +use crate::unifier::{EqlTerm, Type, Value}; +use crate::EqlMapperError; + +use super::helpers::{cast_encrypted_operand, full_payload_domain}; +use super::TransformationRule; + +/// Collapses a multi-step encrypted JSON accessor chain into a SINGLE accessor +/// on the root document: +/// +/// - `col -> 'a' -> 'b'` → `eql_v3."->"(col, )` +/// - `col -> 'a' ->> 'b'` → `eql_v3."->>"(col, )` +/// - `jsonb_path_query_first(col, '$.a') -> 'b'` → `eql_v3."->"(col, )` +/// +/// where `` is the chain's OUTERMOST selector operand, encrypted to key the +/// whole composed path (`$.a.b`) rather than the one segment its text spells. +/// The proxy composes that path from [`crate::JsonAccessorPaths`], which the type +/// inferencer recorded against this same operand. +/// +/// A chain cannot be two hops. `eql_v3."->"` searches the document's `sv` array, +/// and what it returns is one entry with no `sv` of its own — so an accessor +/// applied to the result of an accessor finds nothing and returns NULL. That is +/// the failure this rule exists to prevent, and it is silent: the query runs. +/// +/// Only the OUTERMOST node of a chain is rewritten, and it is rewritten in one +/// step from the chain's root, so the intermediate accessors are discarded whole. +/// Every plaintext selector in them goes with them — leaving one behind would ship +/// a field name to PostgreSQL in the clear (CIP-3682) as well as applying native +/// jsonb `->` to an encrypted payload. +/// +/// # Relationship to the other JSON rules +/// +/// This runs BEFORE [`super::RewriteContainmentOps`], which functionalises a +/// single `->`, and replaces the node with the finished call so that rule declines +/// (it requires its target to still be a `BinaryOp`). Doing it here rather than +/// leaving a one-step `BinaryOp` behind keeps the cast decision keyed to the +/// operand that actually survives: `RewriteContainmentOps` would read the type of +/// the ORIGINAL left operand, which for a chain is the discarded inner accessor. +/// +/// For an EQUALITY over a chain this rule still fires, on the accessor below the +/// comparison, and its result is then discarded by +/// [`super::RewriteJsonValueSelectorEq`] — which re-roots the containment at the +/// bare column read from the original AST. Equality keys path and value into ONE +/// needle, which is strictly stronger than an accessor plus a comparison, so it +/// must keep winning. +#[derive(Debug)] +pub struct CollapseJsonAccessorChain<'ast> { + node_types: Arc, Type>>, +} + +impl<'ast> CollapseJsonAccessorChain<'ast> { + pub fn new(node_types: Arc, Type>>) -> Self { + Self { node_types } + } + + /// The root document of the multi-step ENCRYPTED chain at `expr`, or `None` + /// if this is not one. + /// + /// Gated on the node's own type being [`EqlTerm::JsonExtracted`]: that is what + /// inference assigns to a chain it resolved against an encrypted document, so + /// it is exactly the set of chains whose path it also recorded. A native + /// `jsonb` chain is typed `Native` and is left alone — plaintext `jsonb` + /// genuinely chains, hop by hop. + fn multi_step_chain(&self, expr: &'ast Expr) -> Option<&'ast Expr> { + if !matches!( + self.node_types.get(&NodeKey::new(expr)), + Some(Type::Value(Value::Eql(EqlTerm::JsonExtracted(_)))) + ) { + return None; + } + + json_accessor_chain(expr) + .filter(|(_, selectors)| selectors.len() > 1) + .map(|(root, _)| root) + } + + /// The `eql_v3` function a field access is spelled as. `->` yields the entry, + /// `->>` its text; the outermost step of the chain decides which, exactly as + /// it would for a single access. + fn accessor_fn(op: &BinaryOperator) -> Option<&'static str> { + match op { + BinaryOperator::Arrow => Some("->"), + BinaryOperator::LongArrow => Some("->>"), + _ => None, + } + } + + /// Builds `eql_v3."->"(container, selector)`. + fn accessor_call(fn_name: &str, container: Expr, selector: Expr) -> Expr { + Expr::Function(Function { + name: ObjectName(vec![ + ObjectNamePart::Identifier(Ident::new("eql_v3")), + ObjectNamePart::Identifier(Ident::with_quote('"', fn_name)), + ]), + uses_odbc_syntax: false, + args: FunctionArguments::List(FunctionArgumentList { + args: vec![ + FunctionArg::Unnamed(FunctionArgExpr::Expr(container)), + FunctionArg::Unnamed(FunctionArgExpr::Expr(selector)), + ], + duplicate_treatment: None, + clauses: vec![], + }), + parameters: FunctionArguments::None, + filter: None, + null_treatment: None, + over: None, + within_group: vec![], + }) + } +} + +impl<'ast> TransformationRule<'ast> for CollapseJsonAccessorChain<'ast> { + fn apply( + &mut self, + node_path: &NodePath<'ast>, + target_node: &mut N, + ) -> Result { + // Match against the ORIGINAL nodes: `node_types` is keyed by them, and + // the chain has to be walked before any rule reshapes it. + let Some((original @ Expr::BinaryOp { op, right, .. },)) = node_path.last_1_as::() + else { + return Ok(false); + }; + + let Some(fn_name) = Self::accessor_fn(op) else { + return Ok(false); + }; + + let Some(root) = self.multi_step_chain(original) else { + return Ok(false); + }; + + let Some(expr) = target_node.downcast_mut::() else { + return Ok(false); + }; + let Expr::BinaryOp { + right: target_right, + .. + } = expr + else { + return Ok(false); + }; + + // The selector is a query operand of the accessor call. `->` takes it as + // bare encrypted text, so `full_payload_domain` returns `None` for it and + // no cast is applied — the call is here so the choice stays with the rule + // that owns the context, as it is for a single access. + cast_encrypted_operand(&self.node_types, right, target_right, full_payload_domain); + + // Move (not clone) the transformed selector so its NodeKey identity + // survives for the rules that run after this one; the root comes from the + // original AST, where it is still the bare column the accessor needs. + let dummy = Expr::Value(ValueWithSpan { + value: SqltkValue::Null, + span: Span::empty(), + }); + let selector = mem::replace(&mut **target_right, dummy); + + *expr = Self::accessor_call(fn_name, root.clone(), selector); + + Ok(true) + } + + fn would_edit(&mut self, node_path: &NodePath<'ast>, _target_node: &N) -> bool { + match node_path.last_1_as::() { + Some((expr @ Expr::BinaryOp { op, .. },)) => { + Self::accessor_fn(op).is_some() && self.multi_step_chain(expr).is_some() + } + _ => false, + } + } +} diff --git a/packages/eql-mapper/src/transformation_rules/mod.rs b/packages/eql-mapper/src/transformation_rules/mod.rs index 44e74fc16..37d5700c1 100644 --- a/packages/eql-mapper/src/transformation_rules/mod.rs +++ b/packages/eql-mapper/src/transformation_rules/mod.rs @@ -6,12 +6,13 @@ //! - [`DryRun`] is a type for checking if a `TransformationRule` will mutate the AST without actually mutating the AST. //! It is useful as a performance optimisation to avoid rebuilding an AST if no changes are required. //! -//! This module implements `TransformationRule` for tuples of size 1 to 16 where all of their elements implement +//! This module implements `TransformationRule` for tuples of size 1 to 24 where all of their elements implement //! `TransformationRule`. This facilitates composition of rules into single rules. mod helpers; mod cast_full_payload_operands; +mod collapse_json_accessor_chain; mod fail_on_placeholder_change; mod preserve_effective_aliases; mod rewrite_containment_ops; @@ -31,6 +32,7 @@ mod substitute_encrypted_literals; use std::marker::PhantomData; pub(crate) use cast_full_payload_operands::*; +pub(crate) use collapse_json_accessor_chain::*; pub(crate) use fail_on_placeholder_change::*; pub(crate) use preserve_effective_aliases::*; pub(crate) use rewrite_containment_ops::*; @@ -177,7 +179,7 @@ impl<'ast, T: TransformationRule<'ast>> Transform<'ast> for DryRunnable<'ast, T> } } -#[impl_for_tuples(1, 16)] +#[impl_for_tuples(1, 24)] impl<'ast> TransformationRule<'ast> for Tuple { fn apply( &mut self, diff --git a/packages/eql-mapper/src/transformation_rules/rewrite_json_value_selector_eq.rs b/packages/eql-mapper/src/transformation_rules/rewrite_json_value_selector_eq.rs index 52a873e0e..6e6ff97b6 100644 --- a/packages/eql-mapper/src/transformation_rules/rewrite_json_value_selector_eq.rs +++ b/packages/eql-mapper/src/transformation_rules/rewrite_json_value_selector_eq.rs @@ -10,6 +10,7 @@ use sqltk::parser::ast::{ use sqltk::parser::tokenizer::Span; use sqltk::{NodeKey, NodePath, Visitable}; +use crate::json_value_selector::json_accessor_chain; use crate::unifier::{EqlTerm, Type, Value}; use crate::EqlMapperError; @@ -22,6 +23,8 @@ use super::TransformationRule; /// - `col -> sel = value` → `eql_v3.jsonb_contains(col, )` /// - `col ->> sel = value` → same /// - `jsonb_path_query_first(col, sel) = value` → same +/// - `col -> 'a' -> 'b' = value` → same, against the ROOT `col`: a chain is one +/// path (`$.a.b`) into one document, and the whole chain is discarded /// - `<>` negates: `NOT eql_v3.jsonb_contains(col, )` /// /// where `` is the value operand, already cast to `eql_v3.query_json` @@ -57,30 +60,22 @@ impl<'ast> RewriteJsonValueSelectorEq<'ast> { ) } - /// The container expression of a JSON field access — the `col` of - /// `col -> sel` or of `jsonb_path_query_first(col, sel)`. + /// The expression a JSON field access is ROOTED at — the `col` of + /// `col -> sel`, of `jsonb_path_query_first(col, sel)`, and of a chain like + /// `col -> 'a' -> 'b'`. + /// + /// The whole chain is stripped, not just its outermost step. The path the + /// needle is keyed on is composed from every selector in the chain + /// (`$.a.b`), so what containment tests is the root document. Keeping an + /// intermediate accessor would both leak its plaintext selector into the + /// statement text and apply native jsonb `->` to an encrypted payload, + /// which matches nothing. /// /// Read from the ORIGINAL AST (via `node_path`), because by the time this /// rule runs the field access has already been rewritten to /// `eql_v3."->"(col, sel)` by [`super::RewriteContainmentOps`]. fn container_of(expr: &Expr) -> Option<&Expr> { - match expr { - Expr::BinaryOp { - left, - op: BinaryOperator::Arrow | BinaryOperator::LongArrow, - .. - } => Some(&**left), - - Expr::Function(function) => match &function.args { - FunctionArguments::List(list) => match list.args.as_slice() { - [FunctionArg::Unnamed(FunctionArgExpr::Expr(container)), _] => Some(container), - _ => None, - }, - _ => None, - }, - - _ => None, - } + json_accessor_chain(expr).map(|(container, _)| container) } /// Splits a comparison into `(container, value operand is on the right)`, or diff --git a/packages/eql-mapper/src/type_checked_statement.rs b/packages/eql-mapper/src/type_checked_statement.rs index bfcba5973..a86f179b9 100644 --- a/packages/eql-mapper/src/type_checked_statement.rs +++ b/packages/eql-mapper/src/type_checked_statement.rs @@ -9,11 +9,12 @@ use sqltk::{AsNodeKey, NodeKey, Transformable}; use crate::unifier::{EqlTerm, EqlTermVariant}; use crate::QueryOperands; use crate::{ - CastFullPayloadOperands, DryRunnable, EqlMapperError, FailOnPlaceholderChange, - JsonValueSelectors, OutputParam, OutputParamSource, Param, ParamPlan, PreserveEffectiveAliases, - RenumberParams, RewriteContainmentOps, RewriteEqlAggregateDistinct, RewriteEqlComparisonOps, - RewriteEqlDistinct, RewriteEqlDistinctOrderBy, RewriteEqlGroupBy, RewriteEqlMatchOps, - RewriteEqlOrderBy, RewriteEqlOrdinalOrderBy, RewriteEqlPartitionBy, RewriteJsonValueSelectorEq, + CastFullPayloadOperands, CollapseJsonAccessorChain, DryRunnable, EqlMapperError, + FailOnPlaceholderChange, JsonAccessorPaths, JsonValueSelectors, OutputParam, OutputParamSource, + Param, ParamPlan, PreserveEffectiveAliases, RenumberParams, RewriteContainmentOps, + RewriteEqlAggregateDistinct, RewriteEqlComparisonOps, RewriteEqlDistinct, + RewriteEqlDistinctOrderBy, RewriteEqlGroupBy, RewriteEqlMatchOps, RewriteEqlOrderBy, + RewriteEqlOrdinalOrderBy, RewriteEqlPartitionBy, RewriteJsonValueSelectorEq, RewriteStandardSqlFnsOnEqlTypes, SubstituteEncryptedLiterals, TransformationRule, }; @@ -71,6 +72,14 @@ pub struct TypeCheckedStatement<'ast> { /// the proxy binds against. pub json_value_selectors: JsonValueSelectors<'ast>, + /// The composed paths of the multi-step JSON accessor chains that survive as a + /// single accessor: for each selector operand typed [`EqlTerm::JsonAccessor`] + /// that carries a whole chain, every step of the path it must key. + /// + /// A selector's own text is one segment of that path, so the proxy cannot + /// derive the rest — see [`JsonAccessorPaths`]. + pub json_accessor_paths: JsonAccessorPaths<'ast>, + /// The operands that appear in a query position rather than a storing one. /// /// A query operand carries only search terms; a stored value carries the @@ -98,12 +107,17 @@ pub struct TypeCheckedStatement<'ast> { } impl<'ast> TypeCheckedStatement<'ast> { + // One call site, and every argument a distinct type — including the two + // selector channels, which are distinguished by their role marker precisely so + // that passing one where the other belongs does not compile. + #[allow(clippy::too_many_arguments)] pub(crate) fn new( statement: &'ast Statement, projection: Projection, params: Vec<(Param, Value)>, literals: Vec<(EqlTerm, &'ast ast::Value)>, json_value_selectors: JsonValueSelectors<'ast>, + json_accessor_paths: JsonAccessorPaths<'ast>, query_operands: QueryOperands<'ast>, node_types: Arc, Type>>, ) -> Self { @@ -113,6 +127,7 @@ impl<'ast> TypeCheckedStatement<'ast> { params, literals, json_value_selectors, + json_accessor_paths, query_operands, node_types, } @@ -196,12 +211,23 @@ impl<'ast> TypeCheckedStatement<'ast> { )) })?; - let source = match self.json_value_selectors.for_param(input) { - Some(path) => OutputParamSource::JsonValueSelector { + // The two selector channels are mutually exclusive by + // construction — a chain is either fused away by an equality or + // collapsed to a surviving accessor, never both — so the order of + // these arms is not load-bearing. + let source = match ( + self.json_value_selectors.for_param(input), + self.json_accessor_paths.for_param(input), + ) { + (Some(path), _) => OutputParamSource::JsonValueSelector { path: path.clone(), value: input, }, - None => OutputParamSource::Input(input), + (None, Some(path)) => OutputParamSource::JsonAccessorPath { + path: path.clone(), + selector: input, + }, + (None, None) => OutputParamSource::Input(input), }; let output = Param((idx + 1) as u16); @@ -294,6 +320,10 @@ impl<'ast> TypeCheckedStatement<'ast> { DryRunnable::new(( SubstituteEncryptedLiterals::new(encrypted_literals), RewriteStandardSqlFnsOnEqlTypes::new(Arc::clone(&self.node_types)), + // Before `RewriteContainmentOps`: a collapsed chain is emitted as the + // finished `eql_v3."->"` call, which makes that rule decline rather + // than functionalise a node this one has already replaced. + CollapseJsonAccessorChain::new(Arc::clone(&self.node_types)), RewriteContainmentOps::new(Arc::clone(&self.node_types)), RewriteJsonValueSelectorEq::new(Arc::clone(&self.node_types)), RewriteEqlComparisonOps::new(Arc::clone(&self.node_types)), diff --git a/packages/showcase/CLAUDE.md b/packages/showcase/CLAUDE.md index 4704f37a9..a81e6bd7c 100644 --- a/packages/showcase/CLAUDE.md +++ b/packages/showcase/CLAUDE.md @@ -119,22 +119,48 @@ For example, the SQL `AVG` function cannot be used on encrypted numeric values. When generating tests, it is important that Claude understands the fundamental limitations of EQL so that it does not generate test cases or example code that can never work. -### JSON operator limitations - -**CRITICAL LIMITATION: The `->` operator CANNOT be chained on `ste_vec` encrypted columns!** - -Examples of what DOES NOT WORK: - -- `pii -> 'vitals' -> 'blood_type'` ❌ (chained -> operators) - -This is a fundamental limitation in the searchable encryption. This limitation will be lifted in a future release. - -**WORKAROUND: Use `jsonb_path_query_first` or `jsonb_path_query` instead for deep nested access:** -- `jsonb_path_query_first(pii, '$.vitals.blood_type')` ✅ -- `jsonb_path_query_first(pii, '$.medical_history.allergies')` ✅ -- `jsonb_path_query(pii, '$.medical_history.allergies')` ✅ - -**REMEMBER: Always use JSONPath functions for accessing nested JSON data in encrypted columns, never chain `->` operators!** +### JSON operator chaining + +**The `->` / `->>` operators CAN be chained on `ste_vec` encrypted columns**, as +long as the whole path is written in one expression. The mapper collapses the +chain into a single keyed path against the root document (an encrypted +intermediate value cannot be traversed by the database, so `pii -> 'vitals' -> +'blood_type'` is resolved as the one path `$.vitals.blood_type`, not as two +hops). + +All of these work, and are equivalent: + +- `pii -> 'vitals' -> 'blood_type'` ✅ +- `(pii -> 'vitals') -> 'blood_type'` ✅ (parentheses are transparent) +- `pii -> 'vitals' ->> 'blood_type'` ✅ (spellings mix freely) +- `jsonb_path_query_first(pii, '$.vitals') -> 'blood_type'` ✅ +- `jsonb_path_query_first(pii, '$.vitals.blood_type')` ✅ (a single path is + always fine, and often the clearest) + +Chains work in every position: projections, `WHERE` equality (`= <>` — fused +with the compared value into one containment needle), `WHERE` comparisons +(`< <= > >=`), and `ORDER BY`. + +Placeholder steps work (`pii -> 'vitals' -> $1`), with one exception: a +placeholder step **followed by a literal step** (`pii -> $1 -> 'blood_type'`) +is refused, because the literal selector is encrypted before `$1` is bound. +Give the whole path via placeholders or put the literal steps first. + +**Real limitations that remain:** + +- **A path cannot be split across a subquery, CTE, or view.** + `SELECT a -> 'x' FROM (SELECT pii -> 'vitals' AS a FROM patients) s` ❌ — + the extracted value is a single encrypted entry with nothing left to + traverse, and the root column is out of scope on the far side of the + boundary. This is rejected with a type error, not silently wrong. Write the + whole path in one expression instead. +- **An extracted field is not a document.** It supports equality, comparison, + and `ORDER BY` (it carries equality and ordering terms), but it cannot be + traversed further or used where a whole document is required. +- **Nested `jsonb_path_query*` calls are not collapsed.** + `jsonb_path_query_first(jsonb_path_query_first(pii, '$.a'), '$.b')` ❌ — + write the single path `'$.a.b'` instead. (Chaining `->` on top of a + `jsonb_path_query*` root is fine, per the examples above.) ## Test generation diff --git a/packages/showcase/src/common.rs b/packages/showcase/src/common.rs index 7e363523d..7cba0b520 100644 --- a/packages/showcase/src/common.rs +++ b/packages/showcase/src/common.rs @@ -4,13 +4,27 @@ use rustls::{ client::danger::ServerCertVerifier, crypto::aws_lc_rs::default_provider, pki_types::CertificateDer, ClientConfig, }; -use std::sync::{Arc, Once}; +use std::sync::{Arc, LazyLock, Once}; use tokio_postgres::{types::ToSql, Client, SimpleQueryMessage}; use tracing_subscriber::{filter::Directive, EnvFilter, FmtSubscriber}; -pub const PROXY: u16 = 6432; -pub const PG_PORT: u16 = 5532; -pub const PG_TLS_PORT: u16 = 5617; +/// Environment-overridable, mirroring the integration suite's ports: several +/// copies of these tools need to run at once, each against its own Proxy. +/// A malformed value panics rather than falling back — silently connecting to +/// whatever else is on 6432 is the one failure that looks like a pass. +pub static PROXY: LazyLock = LazyLock::new(|| port_from_env("CS_TEST_PROXY_PORT", 6432)); +pub static PG_PORT: LazyLock = LazyLock::new(|| port_from_env("CS_TEST_PG_PORT", 5532)); +pub static PG_TLS_PORT: LazyLock = + LazyLock::new(|| port_from_env("CS_TEST_PG_TLS_PORT", 5617)); + +fn port_from_env(var: &str, default: u16) -> u16 { + match std::env::var(var) { + Ok(value) => value + .parse() + .unwrap_or_else(|_| panic!("{var} must be a port number, got: {value:?}")), + Err(_) => default, + } +} static INIT: Once = Once::new(); @@ -33,7 +47,7 @@ pub async fn table_exists(table: &str) -> bool { let port = std::env::var("CS_DATABASE__PORT") .map(|s| s.parse().unwrap()) - .unwrap_or(PG_PORT); + .unwrap_or(*PG_PORT); let client = connect_with_tls(port).await; let messages = client.simple_query(&query).await.unwrap(); @@ -102,7 +116,7 @@ pub async fn connect_with_tls(port: u16) -> Client { } pub async fn insert(sql: &str, params: &[&(dyn ToSql + Sync)]) { - let client = connect_with_tls(PROXY).await; + let client = connect_with_tls(*PROXY).await; client.query(sql, params).await.unwrap(); } diff --git a/packages/showcase/src/data.rs b/packages/showcase/src/data.rs index 4e0a984e2..9d2790fca 100644 --- a/packages/showcase/src/data.rs +++ b/packages/showcase/src/data.rs @@ -495,7 +495,7 @@ pub async fn clear() { // EQL v3 encrypted columns are self-configuring domain types, so there is no // `eql_v2_configuration` table to clean up (as there was in EQL v2) — clearing // the demo just truncates the tables. - let client = connect_with_tls(PROXY).await; + let client = connect_with_tls(*PROXY).await; let tables = &[ "patient_medications", diff --git a/packages/showcase/src/main.rs b/packages/showcase/src/main.rs index 63083a1f8..9cc670976 100644 --- a/packages/showcase/src/main.rs +++ b/packages/showcase/src/main.rs @@ -76,7 +76,7 @@ async fn main() -> Result<(), Box> { insert_test_data().await; create_enhanced_jsonb_test_data().await; - let client = connect_with_tls(PROXY).await; + let client = connect_with_tls(*PROXY).await; // Query 1: Get the Aspirin medication ID let aspirin_id_sql = "SELECT id FROM medications WHERE name = 'Aspirin';"; @@ -165,7 +165,7 @@ async fn main() -> Result<(), Box> { /// Tests field access operations (-> and ->>). async fn test_field_access_operations() -> Result<(), Box> { println!("\n🔍 === Testing Field Access Operations (-> and ->>) ==="); - let client = connect_with_tls(PROXY).await; + let client = connect_with_tls(*PROXY).await; // Test 1: Extract nested object with -> operator (returns JSONB) println!("📝 Test 1: Extract medical_history with -> operator"); @@ -216,7 +216,7 @@ async fn test_field_access_operations() -> Result<(), Box /// Tests containment operations (@> and <@). async fn test_containment_operations() -> Result<(), Box> { println!("\n🔍 === Testing Containment Operations (@> and <@) ==="); - let client = connect_with_tls(PROXY).await; + let client = connect_with_tls(*PROXY).await; // Test 1: @> operator (contains) - find patients with specific insurance provider println!("📝 Test 1: Find patients with HealthCorp insurance using @>"); @@ -260,7 +260,7 @@ async fn test_containment_operations() -> Result<(), Box> /// Tests JSONPath functions (jsonb_path_query_first, jsonb_path_query, jsonb_path_exists). async fn test_jsonpath_functions() -> Result<(), Box> { println!("\n🔍 === Testing JSONPath Functions ==="); - let client = connect_with_tls(PROXY).await; + let client = connect_with_tls(*PROXY).await; // Test 1: jsonb_path_exists - check if path exists println!("📝 Test 1: Check if insurance.coverage path exists"); @@ -327,7 +327,7 @@ async fn test_jsonpath_functions() -> Result<(), Box> { /// Tests comparison operations on extracted fields. async fn test_comparison_operations() -> Result<(), Box> { println!("\n🔍 === Testing Comparison Operations ==="); - let client = connect_with_tls(PROXY).await; + let client = connect_with_tls(*PROXY).await; // Test 1: Numeric comparison on extracted integer field println!("📝 Test 1: Find patients with group_id >= 2000"); @@ -382,7 +382,7 @@ async fn test_comparison_operations() -> Result<(), Box> /// Tests complex nested queries combining multiple JSONB operations. async fn test_complex_nested_queries() -> Result<(), Box> { println!("\n🔍 === Testing Complex Nested Queries ==="); - let client = connect_with_tls(PROXY).await; + let client = connect_with_tls(*PROXY).await; // Test 1: Complex query with JOIN, containment, and field extraction println!("📝 Test 1: Find patients with specific insurance AND active prescriptions"); diff --git a/packages/showcase/src/schema.rs b/packages/showcase/src/schema.rs index 4fc222b3c..2d30908ed 100644 --- a/packages/showcase/src/schema.rs +++ b/packages/showcase/src/schema.rs @@ -3,5 +3,5 @@ use crate::common::{reset_schema_to, PROXY}; const SCHEMA: &str = include_str!("./schema.sql"); pub async fn setup_schema() { - reset_schema_to(SCHEMA, PROXY).await + reset_schema_to(SCHEMA, *PROXY).await }