From 36775231387f621711951a50a4437b4320f215dd Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 00:18:15 +1000 Subject: [PATCH 01/12] fix(mapper): fuse a chained JSON accessor into one value-selector path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `WHERE col -> 'a' -> 'b' = $1` on an encrypted JSON column emitted `eql_v3.jsonb_contains(col -> 'a', )`. The rewrite took its container from the original AST, which for a single accessor is the bare column — but for a chain is another accessor. So the plaintext field name `a` shipped in the statement text PostgreSQL received, and native jsonb `->` was applied to an encrypted payload, which matched nothing. Swapping the container alone would not fix it: the needle is keyed on the path, so containment against the root column has to be keyed on the WHOLE path (`$.a.b`), not on the last step. A chain is one path into one document — the intermediate values have no independent existence, because the payload between the steps is encrypted and cannot be traversed. So the path becomes a sequence. `JsonSelectorSource` now holds the steps of the chain, each independently a literal or a placeholder, and the inference walks the accessor chain to collect them; the rewrite strips the whole chain back to its root. One unresolvable step declines the fusion exactly as one unresolvable selector did before — a partial path is not a path — and every placeholder step is an input the param plan consumes, so none is silently dropped. The chain walk only recognises the accessor spellings by name (`->`, `->>`, `jsonb_path_query`, `jsonb_path_query_first`). Accepting any two-argument call, as the previous single-step match did, would read the second argument of `coalesce(a, b)` as a selector and strip the call. Refs CIP-3682 --- .../src/inference/infer_type_impls/expr.rs | 91 ++++---- .../eql-mapper/src/json_value_selector.rs | 194 +++++++++++++++++- packages/eql-mapper/src/lib.rs | 155 +++++++++++--- packages/eql-mapper/src/param_plan.rs | 14 +- .../rewrite_json_value_selector_eq.rs | 33 ++- 5 files changed, 378 insertions(+), 109 deletions(-) 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..244eae3ad 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_chain, + 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. @@ -194,16 +192,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())?; } @@ -647,37 +646,25 @@ impl<'ast> TypeInferencer<'ast> { } /// 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 (_, selectors) = json_accessor_chain(expr)?; + + self.eql_json_value(expr).map(|json| (json, selectors)) } /// Records each of `exprs` that is a literal or placeholder as a query @@ -701,13 +688,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 +705,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))), @@ -742,17 +736,18 @@ 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 { + /// 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/json_value_selector.rs b/packages/eql-mapper/src/json_value_selector.rs index df9ee6779..cd6162924 100644 --- a/packages/eql-mapper/src/json_value_selector.rs +++ b/packages/eql-mapper/src/json_value_selector.rs @@ -15,27 +15,143 @@ use std::collections::HashMap; -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), } +/// 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. +pub(crate) fn json_accessor_chain(expr: &Expr) -> Option<(&Expr, Vec<&Expr>)> { + let mut selectors = Vec::new(); + let mut container = expr; + + while let Some((inner, selector)) = json_accessor(container) { + selectors.push(selector); + container = inner; + } + + if selectors.is_empty() { + return None; + } + + selectors.reverse(); + + Some((container, selectors)) +} + +/// One step of a field access: `(container, selector)`. +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" | "->" | "->>" + ) +} + /// The set of fused JSON value selectors in a statement: for each operand that /// carries the *value* half, where its *path* half comes from. /// @@ -73,3 +189,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..ed9b3427d 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,97 @@ mod test { ); } + /// 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() { diff --git a/packages/eql-mapper/src/param_plan.rs b/packages/eql-mapper/src/param_plan.rs index e90202007..af02b10d5 100644 --- a/packages/eql-mapper/src/param_plan.rs +++ b/packages/eql-mapper/src/param_plan.rs @@ -25,9 +25,10 @@ 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 { @@ -41,10 +42,9 @@ 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() + } } } } 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 From 52bf8c73c3b8d75bd293a729f8df58d44fa172a5 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 00:18:30 +1000 Subject: [PATCH 02/12] fix(proxy): compose the fused JSON selector path from every accessor step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mapper now hands the proxy the STEPS of a JSON accessor chain rather than a single selector, because a chain is one path into one document. Resolve each step — a literal from the SQL or a bind param read straight off the wire — and compose them into one eJSONPath before keying the value-selector needle on it. Composition is the same normalisation a lone selector always got, applied per step and spliced onto the path so far: a step already written as a path (`jsonb_path_query_first(col, '$.a') -> 'b'`) is rooted at the path so far rather than at a second `$`, and a subscript step keeps its bracket. `col -> 'a' -> 'b'` and `jsonb_path_query_first(col, '$.a.b')` therefore key the same needle, which is what makes the two spellings of one path interchangeable. The chained-accessor integration tests are un-ignored, and assert on what the DATABASE holds: they read the stored row on a connection that bypasses Proxy, so a payload carrying any field name or value the client wrote in the clear fails the test. Refs CIP-3682 --- CHANGELOG.md | 4 + .../src/select/jsonb_fusion_gaps.rs | 162 ++++++++++++++---- .../src/postgresql/context/statement.rs | 44 +++-- .../src/postgresql/data/from_sql.rs | 120 +++++++++++-- .../src/postgresql/frontend.rs | 22 ++- .../src/postgresql/messages/bind.rs | 40 +++-- 6 files changed, 319 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09fc50822..6ce8aea8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **Equality on encrypted JSON fields**: `WHERE col -> 'field' = 'value'` now works on encrypted JSON columns, in both the simple and extended query protocols, and in the `->>` and `jsonb_path_query_first(col, path) = value` spellings. `<>` is supported as the negation. The field and the value are combined into a single encrypted value-selector needle and matched by containment, so a query never reveals the field and value separately. Matching is exact and case-sensitive; the value must be a JSON scalar (comparing a whole object or array to a field is rejected — use containment with `@>` instead). +### 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. + ### Fixed - **Statement errors no longer desync the connection**: when a statement failed inside the proxy (an unsupported operation on an encrypted column, for instance), the error was written straight to the client and could overtake responses still in flight from the server — with connection pools and prepared-statement caching, the client then saw a protocol error (`unexpected message from server` in tokio_postgres) instead of the proxy's message, typically right after an encrypted statement had run on the same connection. The proxy now delivers statement errors through the server, so clients always receive the proxy's actual error message, in order, and the connection remains usable. 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..eb3e1762a 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,27 @@ -//! 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 +//! A chained accessor is now one path (`$.nested.string`) rooted at the bare +//! column. Its 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. //! -//! 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. +//! # The NULL-selector test is ignored, not deleted +//! +//! It asserts the behaviour that shape must have. It fails today; un-ignoring it +//! is the acceptance test for its fix. #[cfg(test)] mod tests { - use crate::common::{clear, connect_with_tls, execute_query, random_id, trace, PROXY}; + use crate::common::{ + clear, connect, connect_with_tls, execute_query, random_id, trace, PG_PORT, PROXY, + }; use serde_json::Value; async fn insert_nested() -> i64 { @@ -34,24 +40,32 @@ 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: - /// - /// ```text - /// eql_v3.jsonb_contains(encrypted_jsonb -> 'nested', '{…}') - /// ``` + /// 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. + async fn stored_payload(id: i64) -> String { + let client = connect(*PG_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,15 +81,103 @@ 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. diff --git a/packages/cipherstash-proxy/src/postgresql/context/statement.rs b/packages/cipherstash-proxy/src/postgresql/context/statement.rs index 77053a59b..338a09264 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 { @@ -139,13 +151,19 @@ 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: 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(), }, value: to_index(value.0), } 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/frontend.rs b/packages/cipherstash-proxy/src/postgresql/frontend.rs index ebcf1d0f9..17974f9e4 100644 --- a/packages/cipherstash-proxy/src/postgresql/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/frontend.rs @@ -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; @@ -1407,9 +1407,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 +1434,7 @@ 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) } 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..eee802d1b 100644 --- a/packages/cipherstash-proxy/src/postgresql/messages/bind.rs +++ b/packages/cipherstash-proxy/src/postgresql/messages/bind.rs @@ -3,7 +3,7 @@ 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, @@ -111,24 +111,33 @@ impl Bind { } /// Composes `{"path", "value"}` — the input to `SteVecValueSelector` — from - /// the two operands of a JSON field equality. + /// the operands of a JSON field equality. /// - /// 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. + /// 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. + /// + /// 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 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 Ok(None), + }, + } + } let Some(param) = self.param_values.get(value) else { return Ok(None); @@ -141,11 +150,13 @@ 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)?)) } /// Replaces the bound params with the output params of the rewritten @@ -180,6 +191,7 @@ impl Bind { )?; Self::apply_encrypted(&mut param, ct.as_ref())?; + param_values.push(param); } From d0305ffa2574eec8b85576af9ac23eedd22f05e9 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 00:19:07 +1000 Subject: [PATCH 03/12] fix(proxy): bind NULL when an encrypted operand produced no ciphertext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `WHERE col -> $1 = $2` with `$1` bound NULL builds no needle: there is no path to key the value selector on, so `$2` is never encrypted. The Bind rebuild then forwarded the param it was built around unchanged — which for this shape is the client's PLAINTEXT comparand. The value crossed the wire in the clear, and landed in the server log when the column's domain CHECK rejected it. An output param the plan says must be encrypted, but for which no ciphertext was produced, is now bound NULL instead of inheriting bytes. That is exactly what the SQL means — `col -> NULL = x` is NULL, so the predicate matches nothing — and it holds the invariant in one place rather than enumerating the ways a needle can fail to build (NULL selector, NULL step of a chain, NULL value). An already-NULL param is left alone rather than dirtied, so a Bind message that has not changed is not re-sent. Refs CIP-3682 --- CHANGELOG.md | 2 + .../src/select/jsonb_fusion_gaps.rs | 60 ++++-- .../src/postgresql/messages/bind.rs | 186 +++++++++++++++++- 3 files changed, 230 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ce8aea8d..fef6b72b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **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. + ### Fixed - **Statement errors no longer desync the connection**: when a statement failed inside the proxy (an unsupported operation on an encrypted column, for instance), the error was written straight to the client and could overtake responses still in flight from the server — with connection pools and prepared-statement caching, the client then saw a protocol error (`unexpected message from server` in tokio_postgres) instead of the proxy's message, typically right after an encrypted statement had run on the same connection. The proxy now delivers statement errors through the server, so clients always receive the proxy's actual error message, in order, and the connection remains usable. 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 eb3e1762a..050e0741a 100644 --- a/packages/cipherstash-proxy-integration/src/select/jsonb_fusion_gaps.rs +++ b/packages/cipherstash-proxy-integration/src/select/jsonb_fusion_gaps.rs @@ -7,15 +7,11 @@ //! not handle, and in each case something the client wrote in plaintext was //! forwarded to PostgreSQL (CIP-3682). //! -//! A chained accessor is now one path (`$.nested.string`) rooted at the bare -//! column. Its 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. -//! -//! # The NULL-selector test is ignored, not deleted -//! -//! It asserts the behaviour that shape must have. It fails today; un-ignoring it -//! is the acceptance test for its fix. +//! 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 { @@ -182,10 +178,6 @@ mod tests { /// `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; @@ -206,4 +198,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/src/postgresql/messages/bind.rs b/packages/cipherstash-proxy/src/postgresql/messages/bind.rs index eee802d1b..8bba7a715 100644 --- a/packages/cipherstash-proxy/src/postgresql/messages/bind.rs +++ b/packages/cipherstash-proxy/src/postgresql/messages/bind.rs @@ -174,8 +174,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(()); } @@ -190,7 +195,7 @@ impl Bind { }, )?; - Self::apply_encrypted(&mut param, ct.as_ref())?; + Self::apply_output(&mut param, output, ct.as_ref())?; param_values.push(param); } @@ -204,6 +209,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 @@ -303,6 +331,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 } @@ -485,13 +528,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) @@ -543,4 +591,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()); + } } From ed612c5a5032bffd3dc8c4e535e38a08ffc6b187 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 11:23:39 +1000 Subject: [PATCH 04/12] test(integration): read the stored payload from the database Proxy uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stored_payload` connected to `PG_PORT` to read the row back on a connection that bypasses Proxy. `PG_PORT` is the non-TLS PostgreSQL on 5532, but the TLS suite CI runs puts Proxy in front of `postgres-tls` on 5617 — so the read went to a different database, found no row, and the test panicked indexing an empty result rather than asserting anything. It passed locally only because a single-database setup makes the two ports the same, which is exactly the condition that hid the bug. Now uses `get_database_port()`, which prefers `CS_DATABASE__PORT` and falls back to `PG_PORT` — the same helper `query_direct_by`, `reset_schema` and the resilience tests already use for this. This test was the only place in the suite reaching for `PG_PORT` directly. The connection is TLS now too, matching every other direct-to-PostgreSQL path. --- .../src/select/jsonb_fusion_gaps.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 050e0741a..0c1fa5e4b 100644 --- a/packages/cipherstash-proxy-integration/src/select/jsonb_fusion_gaps.rs +++ b/packages/cipherstash-proxy-integration/src/select/jsonb_fusion_gaps.rs @@ -16,7 +16,7 @@ #[cfg(test)] mod tests { use crate::common::{ - clear, connect, connect_with_tls, execute_query, random_id, trace, PG_PORT, PROXY, + clear, connect_with_tls, execute_query, get_database_port, random_id, trace, PROXY, }; use serde_json::Value; @@ -38,8 +38,13 @@ mod tests { /// 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. + /// + /// 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(*PG_PORT).await; + let client = connect_with_tls(get_database_port()).await; let rows = client .query( From 334f31a619f0680e35cf6f7b6060954310e99dc9 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 12:31:24 +1000 Subject: [PATCH 05/12] fix(mapper): see through parentheses when walking a JSON accessor chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `json_accessor_chain` matched `Expr::BinaryOp` and the accessor functions but never `Expr::Nested`, so a single pair of brackets stopped the walk. That is not just a missed fusion, it is the CIP-3682 leak again by another route. On `(j -> 'foo') -> 'bar' = $1` the walker stopped at the bracket, took the parenthesised accessor to be the ROOT container, and emitted: eql_v3.jsonb_contains((j -> 'foo'), $1::JSONB::eql_v3.query_json) The plaintext selector 'foo' ships to PostgreSQL, native jsonb -> is applied to the encrypted payload, and the needle is keyed on `$.bar` when the real path is `$.foo.bar` — so it matches nothing either. Outer brackets instead declined the fusion altogether and fell through to whole-document equality over doubly-nested `->` calls. Parentheses now unwrap at the root, at every step, and in the selector position, so every bracketing of the same query decomposes to the same root and the same path. `Expr::Nested` carries no meaning beyond recording that the author typed brackets; every consumer of a chain wants what is inside them. Both the inference hook and the rewrite rule reach chains through this one function, so both are fixed by it. --- .../eql-mapper/src/json_value_selector.rs | 27 ++++++++++-- packages/eql-mapper/src/lib.rs | 41 +++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/packages/eql-mapper/src/json_value_selector.rs b/packages/eql-mapper/src/json_value_selector.rs index cd6162924..42204a3a5 100644 --- a/packages/eql-mapper/src/json_value_selector.rs +++ b/packages/eql-mapper/src/json_value_selector.rs @@ -92,13 +92,20 @@ impl JsonSelectorSource { /// /// 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 = expr; + let mut container = unnest(expr); while let Some((inner, selector)) = json_accessor(container) { - selectors.push(selector); - container = inner; + selectors.push(unnest(selector)); + container = unnest(inner); } if selectors.is_empty() { @@ -110,6 +117,20 @@ pub(crate) fn json_accessor_chain(expr: &Expr) -> Option<(&Expr, Vec<&Expr>)> { 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. +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)`. fn json_accessor(expr: &Expr) -> Option<(&Expr, &Expr)> { match expr { diff --git a/packages/eql-mapper/src/lib.rs b/packages/eql-mapper/src/lib.rs index ed9b3427d..70916d4f8 100644 --- a/packages/eql-mapper/src/lib.rs +++ b/packages/eql-mapper/src/lib.rs @@ -3427,6 +3427,47 @@ mod test { ); } + /// 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] From acb2511a12e1694f91c4fb288456e4a7dbe97e2f Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 12:53:19 +1000 Subject: [PATCH 06/12] fix(mapper): make an extracted encrypted JSON value unqueryable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `->` was declared `(T -> ::Accessor) -> T`, so the result of an extraction had the SAME type as the document it came from. An extracted SteVec entry carries no `sv` array, so traversing one selects nothing — but nothing in the type system said so, and the only thing standing between a user and a wrong answer was a syntactic walker. A walker can always be defeated by moving the halves apart: SELECT a -> 'foo' FROM (SELECT j -> 'bar' AS a FROM t) s `a` is syntactically far from the `->` that produced it, so the path `$.bar.foo` cannot be composed. It emitted a second entry-scoped accessor over an entry and returned NULL, silently. Written as a predicate it was worse: the fusion claimed it, rooted a needle at `a`, and keyed it on `$.foo` when the real path was `$.bar.foo` — wrong rows, no error. `EqlTerm::JsonExtracted` now types that result, with no capabilities at all, so no operator or function can require anything of it. A type crosses a subquery boundary where a syntactic pattern does not, which is the whole reason to carry this in the type system. Inference has to be fusion-aware for that to work. The operator declaration is compositional — it sees only its immediate left operand — but a chain is not: `j -> 'a' -> 'b'` is ONE path into ONE document. Typing it step by step would make the first link `JsonExtracted` and the second link fail, rejecting every chain including the ones that fuse correctly. So the `Arrow`/`LongArrow` rule consults the chain BELOW the node and asks whether its ROOT is a document. Within one expression the walker always reaches the root, so an extracted intermediate is fine; across a subquery it cannot, the root is itself extracted, and the access is refused with a message naming the fix. `eql_json_field_access` likewise resolves the root as a document rather than reading the node's own type, which is what stops the fusion claiming a chain rooted at an entry. The declaration is left alone: it is correct for native `jsonb`, which legitimately chains, and every encrypted case is now handled before it. Proxy side: `JsonExtracted` reaching the encrypt path is refused rather than defaulted to `EqlOperation::Store`. It is the result of a read, not an operand, and storing it would encrypt the wrong payload shape and silently return the wrong rows — the failure this term exists to prevent. --- packages/cipherstash-proxy/src/error.rs | 12 +++ .../src/proxy/zerokms/zerokms.rs | 9 ++ .../src/inference/infer_type_impls/expr.rs | 94 ++++++++++++++++++- .../eql-mapper/src/inference/type_error.rs | 16 ++++ .../src/inference/unifier/eql_traits.rs | 3 + .../eql-mapper/src/inference/unifier/types.rs | 39 +++++++- packages/eql-mapper/src/lib.rs | 65 +++++++++++++ 7 files changed, 235 insertions(+), 3 deletions(-) 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/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/inference/infer_type_impls/expr.rs b/packages/eql-mapper/src/inference/infer_type_impls/expr.rs index 244eae3ad..e385f111f 100644 --- a/packages/eql-mapper/src/inference/infer_type_impls/expr.rs +++ b/packages/eql-mapper/src/inference/infer_type_impls/expr.rs @@ -214,6 +214,61 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { false }; + // Encrypted JSON field ACCESS (`->`, `->>`). + // + // This is the fusion-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 including the ones that fuse + // correctly. + // + // 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) { + match json_accessor_chain(expr_val) { + Some((root, _)) => match self.eql_json_document(root) { + // A chain rooted at a document: type the whole + // access as one extraction from that document, + // and the selector as its accessor so it is + // encrypted. + Some(json) => { + 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 + } + // Rooted at an entry someone already extracted: + // there is no `sv` left to traverse. + None if self.is_eql_json_extracted(root) => { + return Err(TypeError::UnqueryableJsonExtraction); + } + // 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 @@ -645,6 +700,37 @@ 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 expressions supplying its selectors, outermost last: /// @@ -662,9 +748,13 @@ impl<'ast> TypeInferencer<'ast> { /// 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 (_, selectors) = json_accessor_chain(expr)?; + let (root, selectors) = json_accessor_chain(expr)?; - self.eql_json_value(expr).map(|json| (json, selectors)) + // 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 diff --git a/packages/eql-mapper/src/inference/type_error.rs b/packages/eql-mapper/src/inference/type_error.rs index 26cf258b7..4bcbd22db 100644 --- a/packages/eql-mapper/src/inference/type_error.rs +++ b/packages/eql-mapper/src/inference/type_error.rs @@ -18,6 +18,22 @@ 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 fused into a single path instead, + /// so this is reached only when the chain is broken up such that the + /// selectors cannot be composed: across a subquery boundary, a CTE, or a + /// view. + #[error( + "cannot apply a JSON operator to the result of an encrypted JSON \ + operation. Write the whole path in one expression (`col -> 'a' -> 'b'`) \ + so it can be resolved against the document, rather than extracting a \ + field and traversing the result" + )] + UnqueryableJsonExtraction, + #[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..21be52984 100644 --- a/packages/eql-mapper/src/inference/unifier/eql_traits.rs +++ b/packages/eql-mapper/src/inference/unifier/eql_traits.rs @@ -328,6 +328,9 @@ impl EqlTerm { EqlTerm::Tokenized(_) => EqlTraits::none(), EqlTerm::JsonOrd(_) => EqlTraits::none(), EqlTerm::JsonValueSelector(_) => EqlTraits::none(), + // Unqueryable by construction: no operator or function can require + // any capability of an already-extracted JSON entry. + EqlTerm::JsonExtracted(_) => 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/lib.rs b/packages/eql-mapper/src/lib.rs index 70916d4f8..19400fd1c 100644 --- a/packages/eql-mapper/src/lib.rs +++ b/packages/eql-mapper/src/lib.rs @@ -3427,6 +3427,71 @@ 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 fused 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). + /// + /// 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}" + ); + } + } + + /// 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, From 0cabd73d850707b6c5bb8f3ed0c37c260ddfe308 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 14:09:12 +1000 Subject: [PATCH 07/12] refactor(mapper): put the extracted-JSON type behind JsonLike::Output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `->` and `->>` were declared to return `T`, so the result of traversing native jsonb and the result of traversing an ENCRYPTED document had the same type as their input. The two are not the same thing: native jsonb yields traversable jsonb, while an encrypted document yields one SteVec entry with no `sv` of its own, which cannot be traversed again. `JsonLike` gains an `Output` associated type so the trait carries that difference instead of a special case at the call site: Native resolves it to Native, and an encrypted document resolves it to `EqlTerm::JsonExtracted`. Plaintext chains keep working; encrypted ones are refused wherever the chain cannot be composed into a single path. Deliberately NOT applied to `jsonb_path_query`/`jsonb_path_query_first`. Doing so broke two supported shapes, so it needs prior work: - `jsonb_array_elements` and `jsonb_array_length` CONSUME an extracted entry rather than traversing it, which is a legitimate operation the canonical array recipe depends on. They require `JsonLike`, which an extracted value deliberately does not satisfy. - the rewrite that retargets these functions to their `eql_v3` twins and encrypts their Path operand keys off the result type. Changing it sent the caller's literal jsonpath to PostgreSQL unencrypted, which PostgreSQL then rejected: `@ is not allowed in root expressions`. Both point at the same missing distinction — traversing an extraction versus consuming one — which is worth settling before the path-query functions join this. --- .../src/inference/sql_types/sql_decls.rs | 28 +++++++++++++++++-- .../src/inference/unifier/eql_traits.rs | 17 ++++++++++- packages/eql-mapper/src/lib.rs | 11 ++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) 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/unifier/eql_traits.rs b/packages/eql-mapper/src/inference/unifier/eql_traits.rs index 21be52984..086f65830 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)), diff --git a/packages/eql-mapper/src/lib.rs b/packages/eql-mapper/src/lib.rs index 19400fd1c..e27a86ffa 100644 --- a/packages/eql-mapper/src/lib.rs +++ b/packages/eql-mapper/src/lib.rs @@ -4053,6 +4053,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)] From 07d6b0f6b4ecf5783885ae9bc33c07067ba10901 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 15:11:43 +1000 Subject: [PATCH 08/12] fix(mapper): use infer_enter to tell a fusable chain from a doomed one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `->` now returns `::Output`, so an encrypted extraction is unqueryable — but that left the projection chain still passing, because the fusion-aware branch intercepted before the declaration could object. It had no way to tell `j -> 'a' -> 'b' = $1`, which fuses, from `SELECT j -> 'a' -> 'b'`, which has no rewrite at all. The reason is ordering. Typing is post-order, so when a chain's outermost `->` is typed its parent does not exist yet — and the parent is precisely what decides whether the chain is legal. `infer_enter` runs on the way DOWN, before any child is typed, so the comparison can mark the chain first and the `->` rule can then ask whether anything above it will collapse the chain. What follows from that: - A multi-step chain is permitted only where it is marked. Unmarked, it is refused with a message naming the fix, rather than emitting nested entry-scoped accessors over an entry and returning NULL. - Only EQUALITY marks. Ordering types the scalar operand but leaves the chain standing in the emitted SQL, so a multi-step ordering comparison has no correct rewrite either and is now refused. Single-field ordering is unaffected. - The whole chain SPINE is marked, one step at a time. `j->'a'->'b'->'c'` is three nested `BinaryOp`s and every one of them is typed, so marking only the outermost left the intermediates looking doomed and refused the query. `json_accessor_chain` jumps to the root, so this walks `json_accessor` instead. - Marks are recorded against the UNNESTED node, since that is what the `->` rule is handed for `((j -> 'a') -> 'b') = $1`. A SINGLE access needs none of this: the declaration types it, so it stays legal anywhere, projectable and decryptable as before. Only traversing an extraction is refused. --- .../src/inference/infer_type_impls/expr.rs | 88 ++++++++++++++++++- packages/eql-mapper/src/inference/mod.rs | 41 ++++++++- .../eql-mapper/src/inference/type_error.rs | 7 +- .../eql-mapper/src/json_value_selector.rs | 4 +- packages/eql-mapper/src/lib.rs | 61 +++++++++++++ 5 files changed, 192 insertions(+), 9 deletions(-) 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 e385f111f..b3bd6700c 100644 --- a/packages/eql-mapper/src/inference/infer_type_impls/expr.rs +++ b/packages/eql-mapper/src/inference/infer_type_impls/expr.rs @@ -4,7 +4,7 @@ use crate::{ unifier::{EqlTerm, EqlValue, TokenType, Type, Value}, InferType, TypeError, }, - json_value_selector::json_accessor_chain, + json_value_selector::{json_accessor, json_accessor_chain, unnest}, EqlTrait, IdentCase, JsonSelectorSegment, JsonSelectorSource, Param, TypeInferencer, }; use eql_mapper_macros::trace_infer; @@ -24,6 +24,59 @@ 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 whether + /// the chain is legal. `j -> 'a' -> 'b' = $1` is one path into one document + /// and fuses into a single containment; the same chain in a projection has + /// nothing to collapse it and would apply an entry-scoped accessor to an + /// entry, silently returning NULL. Recording the intent here is what lets the + /// `->` rule tell them apart. + /// + /// 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. It collapses the whole path into + // one value-selector containment against the root document, so the + // intermediate accessors disappear from the emitted SQL. + // + // Ordering does NOT: `RewriteEqlComparisonOps` types the scalar + // operand as a SteVec ordering term but leaves the accessor chain + // standing, so a multi-step ordering chain would emit nested + // accessors over an entry and compare NULL. There is no correct + // rewrite for it, so it must not be marked — being refused is the + // right outcome until ordering learns to compose a path too. + 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 like an + // unfused chain and refuse the whole query. + // + // 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 @@ -235,7 +288,38 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { // refused. let handled = handled || if matches!(op, BinaryOperator::Arrow | BinaryOperator::LongArrow) { - match json_accessor_chain(expr_val) { + // Only a MULTI-step chain needs special treatment. A + // single access is handled correctly by the declaration + // (`-> ::Output`), for native and + // encrypted alike, and is legal anywhere. + // 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); + } + } + + match json_accessor_chain(expr_val) + .filter(|(_, selectors)| selectors.len() > 1) + { + Some((root, _)) if !self.is_fusable_json_chain(expr_val) => { + // Nothing above will collapse this chain, so + // there is no rewrite that would be correct: it + // would apply an entry-scoped accessor to an + // entry and return NULL. Refuse with a message + // that names the fix, rather than letting the + // declaration report an unsatisfied `JsonLike`. + if self.eql_json_document(root).is_some() + || self.is_eql_json_extracted(root) + { + return Err(TypeError::UnqueryableJsonExtraction); + } + false + } Some((root, _)) => match self.eql_json_document(root) { // A chain rooted at a document: type the whole // access as one extraction from that document, diff --git a/packages/eql-mapper/src/inference/mod.rs b/packages/eql-mapper/src/inference/mod.rs index faf314106..7856a71be 100644 --- a/packages/eql-mapper/src/inference/mod.rs +++ b/packages/eql-mapper/src/inference/mod.rs @@ -9,14 +9,17 @@ 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, @@ -67,6 +70,22 @@ pub struct TypeInferencer<'ast> { /// 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 whether the chain is legal depends + /// entirely on that parent. `j -> 'a' -> 'b' = $1` is one path into one + /// document and fuses; the same chain in a projection has no fusion to + /// collapse it and would emit an entry-scoped accessor over an entry, + /// returning NULL. `infer_enter` on the comparison marks the chain before + /// any of it is typed, so the `->` rule can tell the two apart. + /// + /// 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 ()>, } @@ -83,6 +102,7 @@ impl<'ast> TypeInferencer<'ast> { unifier: unifier.into(), json_value_selectors: RefCell::new(JsonValueSelectors::default()), query_operands: RefCell::new(QueryOperands::default()), + fusable_json_chains: RefCell::new(HashSet::new()), _ast: PhantomData, } } @@ -98,6 +118,23 @@ impl<'ast> TypeInferencer<'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. A chain nothing fuses has no correct rewrite, so it must not be + /// permitted past the type check. + 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); } diff --git a/packages/eql-mapper/src/inference/type_error.rs b/packages/eql-mapper/src/inference/type_error.rs index 4bcbd22db..83efb0139 100644 --- a/packages/eql-mapper/src/inference/type_error.rs +++ b/packages/eql-mapper/src/inference/type_error.rs @@ -28,9 +28,10 @@ pub enum TypeError { /// view. #[error( "cannot apply a JSON operator to the result of an encrypted JSON \ - operation. Write the whole path in one expression (`col -> 'a' -> 'b'`) \ - so it can be resolved against the document, rather than extracting a \ - field and traversing the result" + operation: an extracted field is a single encrypted entry, not a \ + document, so there is nothing left to traverse. A multi-step path is \ + resolved against the whole document only by exact equality (`col -> 'a' \ + -> 'b' = $1`, or `<>`); anywhere else, select the one field you need" )] UnqueryableJsonExtraction, diff --git a/packages/eql-mapper/src/json_value_selector.rs b/packages/eql-mapper/src/json_value_selector.rs index 42204a3a5..efc360342 100644 --- a/packages/eql-mapper/src/json_value_selector.rs +++ b/packages/eql-mapper/src/json_value_selector.rs @@ -121,7 +121,7 @@ pub(crate) fn json_accessor_chain(expr: &Expr) -> Option<(&Expr, Vec<&Expr>)> { /// /// `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. -fn unnest(expr: &Expr) -> &Expr { +pub(crate) fn unnest(expr: &Expr) -> &Expr { let mut current = expr; while let Expr::Nested(inner) = current { @@ -132,7 +132,7 @@ fn unnest(expr: &Expr) -> &Expr { } /// One step of a field access: `(container, selector)`. -fn json_accessor(expr: &Expr) -> Option<(&Expr, &Expr)> { +pub(crate) fn json_accessor(expr: &Expr) -> Option<(&Expr, &Expr)> { match expr { Expr::BinaryOp { left, diff --git a/packages/eql-mapper/src/lib.rs b/packages/eql-mapper/src/lib.rs index e27a86ffa..810073795 100644 --- a/packages/eql-mapper/src/lib.rs +++ b/packages/eql-mapper/src/lib.rs @@ -3467,6 +3467,67 @@ mod test { } } + /// A multi-step chain is only legal where something will fuse it into a + /// single path. Nothing else has a correct rewrite. + /// + /// Only exact equality collapses a chain, into one value-selector containment + /// against the root document. Everywhere else the chain would survive into + /// the emitted SQL as nested entry-scoped accessors applied to an entry, + /// which selects nothing and yields NULL — the silent-wrong-answer failure + /// this whole family of types exists to prevent. + /// + /// Typing is post-order, so when the outermost `->` of a chain is typed its + /// parent is not yet known. `infer_enter` on the comparison marks the chain's + /// spine on the way down, which is what lets the `->` rule distinguish + /// "about to be fused" from "nothing will collapse this". + #[test] + fn a_multi_step_chain_is_refused_where_nothing_fuses_it() { + let schema = chained_json_schema(); + + for sql in [ + // A projection has no comparison to fuse into. + "SELECT j -> 'foo' -> 'bar' FROM t", + "SELECT (j -> 'foo') -> 'bar' FROM t", + "SELECT j -> 'foo' ->> 'bar' FROM t", + // Ordering types the scalar operand but leaves the chain standing, + // so a multi-step ordering comparison has no correct rewrite either. + "SELECT id FROM t WHERE j -> 'foo' -> 'bar' < '\"x\"'", + "SELECT id FROM t WHERE j -> 'foo' -> 'bar' >= '\"x\"'", + ] { + 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 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, From 96b00b4077dddbd8ebf0577a965dbd654da10730 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 16:13:12 +1000 Subject: [PATCH 09/12] feat: resolve a multi-step encrypted JSON path in every context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A multi-step chain was refused everywhere except exact equality. Refusing was the right call against the alternative it replaced — nested accessors over an entry, answering NULL with no error — but it was a limitation, not a necessity. The correct emission exists for every context, and it is the same one equality already builds a path out of. `eql_v3."->"(doc, sel)` searches the document's `sv` array and returns one ENTRY, which has no `sv` of its own. So `j -> 'a' -> 'b'` cannot be two hops; it is ONE accessor keyed on the composed path: eql_v3."->"(j, ) which is exactly the shape a single access already emits. Since a chain and a single access are both typed `JsonExtracted`, they can reach exactly the same contexts — so once a chain collapses to that shape, legality is uniform and there is nothing left for the refusal to protect. What that took: - `CollapseJsonAccessorChain` rewrites the outermost node of a chain in one step from its ROOT, discarding the intermediate accessors whole so no plaintext selector is left behind (CIP-3682). It runs BEFORE `RewriteContainmentOps` and emits the finished call, which makes that rule decline: it would otherwise key its cast decision to the type of the original left operand, which for a chain is the accessor being discarded rather than the operand that survives. - A parallel path-only channel, `JsonAccessorPaths`. The operand that survives is the outermost selector, and its own text is ONE segment of the path it has to key — nothing the proxy is handed at encryption time could recover the rest. It is a separate channel from the fused equality one because the two mean different encryption ops (`SteVecSelector` versus `SteVecValueSelector`), and a separate TYPE, so passing one where the other belongs does not compile. - The proxy composes. Where a `JsonAccessor` operand became `json_selector_path(val)` it now consults the record and composes every segment, falling back to the single-segment behaviour when there is no record — which is what keeps a single access on exactly its old path. Param steps resolve at Bind; literal steps at Parse. The `infer_enter` mark is REPURPOSED, not removed. It no longer decides whether a chain is legal — it decides which channel the path goes in. Under `= $1` the accessor is absorbed into the needle and never appears, so the path belongs to the value operand; anywhere else the accessor survives and carries the path itself. Recording into both would be worse than neither: the accessor channel resolves a literal operand at Parse time, so `j -> $1 -> 'b' = $2` would start failing a query that works. Equality keeps fusing. Its needle MACs path and value together and its presence in the stored `sv` IS the match, which is strictly stronger than an accessor plus a comparison — and `eql_v3.eq_term` has no overload for a JSON query operand anyway. The collapse rule still fires on the accessor below the comparison; the equality rule then discards that result and re-roots the containment at the bare column, as before. Two shapes stay refused, both because they are impossible rather than unimplemented: - Split across a subquery, CTE or view. `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. `UnqueryableJsonExtraction`, whose message now describes only this. - A placeholder step in front of a LITERAL final step (`j -> $1 -> 'b'`). The surviving operand is the literal, encrypted at Parse time, before `$1` is bound. Same limitation the fused equality has for `col -> $1 = 'value'`, and refused the same way — composing only what is known would key `$.b` and read a different field. Also fixed, because the new channel made it reachable and the old one already had it: one placeholder used as the selector of two chains with DIFFERENT paths silently kept whichever was recorded last, answering one occurrence from the wrong field. The path is keyed by param number because that is all Bind has, so there is no key that could tell the two apart — it is now an error naming the param. The same path twice is not a conflict. An unresolvable step (a column reference, a function call) is refused outright rather than declined. Unlike the fused case there is no capability check to fall through to: the chain collapses either way, so the step would simply vanish from the statement. --- CHANGELOG.md | 20 +- .../src/postgresql/context/statement.rs | 50 ++-- .../src/postgresql/data/mod.rs | 5 +- .../src/postgresql/frontend.rs | 66 ++++- .../src/postgresql/messages/bind.rs | 73 +++++- packages/eql-mapper/src/eql_mapper.rs | 1 + .../src/inference/infer_type_impls/expr.rs | 131 ++++++---- packages/eql-mapper/src/inference/mod.rs | 62 ++++- .../eql-mapper/src/inference/type_error.rs | 47 +++- .../eql-mapper/src/json_value_selector.rs | 82 +++++- packages/eql-mapper/src/lib.rs | 235 +++++++++++++++--- packages/eql-mapper/src/param_plan.rs | 22 ++ .../collapse_json_accessor_chain.rs | 185 ++++++++++++++ .../src/transformation_rules/mod.rs | 6 +- .../eql-mapper/src/type_checked_statement.rs | 46 +++- 15 files changed, 879 insertions(+), 152 deletions(-) create mode 100644 packages/eql-mapper/src/transformation_rules/collapse_json_accessor_chain.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index fef6b72b2..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 @@ -30,12 +44,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **Equality on encrypted JSON fields**: `WHERE col -> 'field' = 'value'` now works on encrypted JSON columns, in both the simple and extended query protocols, and in the `->>` and `jsonb_path_query_first(col, path) = value` spellings. `<>` is supported as the negation. The field and the value are combined into a single encrypted value-selector needle and matched by containment, so a query never reveals the field and value separately. Matching is exact and case-sensitive; the value must be a JSON scalar (comparing a whole object or array to a field is rejected — use containment with `@>` instead). -### 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. - ### Fixed - **Statement errors no longer desync the connection**: when a statement failed inside the proxy (an unsupported operation on an encrypted column, for instance), the error was written straight to the client and could overtake responses still in flight from the server — with connection pools and prepared-statement caching, the client then saw a protocol error (`unexpected message from server` in tokio_postgres) instead of the proxy's message, typically right after an encrypted statement had run on the same connection. The proxy now delivers statement errors through the server, so clients always receive the proxy's actual error message, in order, and the connection remains usable. diff --git a/packages/cipherstash-proxy/src/postgresql/context/statement.rs b/packages/cipherstash-proxy/src/postgresql/context/statement.rs index 338a09264..a07699651 100644 --- a/packages/cipherstash-proxy/src/postgresql/context/statement.rs +++ b/packages/cipherstash-proxy/src/postgresql/context/statement.rs @@ -37,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, } } } @@ -151,23 +162,16 @@ pub fn output_params_from_plan( } eql_mapper::OutputParamSource::JsonValueSelector { path, value } => { OutputParamSource::JsonValueSelector { - path: 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(), - }, + 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() @@ -177,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/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 17974f9e4..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; @@ -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| { @@ -1437,6 +1450,49 @@ fn json_value_selector_literal_plaintext( 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 where T: ?Sized + Serialize, diff --git a/packages/cipherstash-proxy/src/postgresql/messages/bind.rs b/packages/cipherstash-proxy/src/postgresql/messages/bind.rs index 8bba7a715..b3f382f5a 100644 --- a/packages/cipherstash-proxy/src/postgresql/messages/bind.rs +++ b/packages/cipherstash-proxy/src/postgresql/messages/bind.rs @@ -6,7 +6,8 @@ use crate::postgresql::context::statement::{ 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,6 +106,9 @@ 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() @@ -127,17 +131,9 @@ impl Bind { value: usize, postgres_type: &Type, ) -> Result, Error> { - 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 Ok(None), - }, - } - } + let Some(steps) = self.resolve_selector_path(path) else { + return Ok(None); + }; let Some(param) = self.param_values.get(value) else { return Ok(None); @@ -159,6 +155,59 @@ impl Bind { 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 /// statement. /// 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 b3bd6700c..ffac234bd 100644 --- a/packages/eql-mapper/src/inference/infer_type_impls/expr.rs +++ b/packages/eql-mapper/src/inference/infer_type_impls/expr.rs @@ -27,27 +27,32 @@ 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 whether - /// the chain is legal. `j -> 'a' -> 'b' = $1` is one path into one document - /// and fuses into a single containment; the same chain in a projection has - /// nothing to collapse it and would apply an entry-scoped accessor to an - /// entry, silently returning NULL. Recording the intent here is what lets the - /// `->` rule tell them apart. + /// 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. It collapses the whole path into - // one value-selector containment against the root document, so the - // intermediate accessors disappear from the emitted SQL. + // 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 but leaves the accessor chain - // standing, so a multi-step ordering chain would emit nested - // accessors over an entry and compare NULL. There is no correct - // rewrite for it, so it must not be marked — being refused is the - // right outcome until ordering learns to compose a path too. + // 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 { @@ -55,8 +60,8 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { // 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 like an - // unfused chain and refuse the whole query. + // 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 @@ -269,15 +274,14 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { // Encrypted JSON field ACCESS (`->`, `->>`). // - // This is the fusion-aware half of `EqlTerm::JsonExtracted`. The + // 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 including the ones that fuse - // correctly. + // 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 @@ -288,10 +292,6 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { // refused. let handled = handled || if matches!(op, BinaryOperator::Arrow | BinaryOperator::LongArrow) { - // Only a MULTI-step chain needs special treatment. A - // single access is handled correctly by the declaration - // (`-> ::Output`), for native and - // encrypted alike, and is legal anywhere. // 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 @@ -303,29 +303,32 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { } } + // 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, _)) if !self.is_fusable_json_chain(expr_val) => { - // Nothing above will collapse this chain, so - // there is no rewrite that would be correct: it - // would apply an entry-scoped accessor to an - // entry and return NULL. Refuse with a message - // that names the fix, rather than letting the - // declaration report an unsatisfied `JsonLike`. - if self.eql_json_document(root).is_some() - || self.is_eql_json_extracted(root) - { - return Err(TypeError::UnqueryableJsonExtraction); - } - false - } - Some((root, _)) => match self.eql_json_document(root) { + 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 selector as its accessor so it is - // encrypted. + // 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( @@ -338,11 +341,6 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> { )?; true } - // Rooted at an entry someone already extracted: - // there is no `sv` left to traverse. - None if self.is_eql_json_extracted(root) => { - return Err(TypeError::UnqueryableJsonExtraction); - } // Native JSON: the declaration is correct for it, // and plaintext `jsonb` chains legitimately. None => false, @@ -900,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), @@ -910,6 +908,47 @@ impl<'ast> TypeInferencer<'ast> { Ok(true) } + /// 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. diff --git a/packages/eql-mapper/src/inference/mod.rs b/packages/eql-mapper/src/inference/mod.rs index 7856a71be..cfd31304b 100644 --- a/packages/eql-mapper/src/inference/mod.rs +++ b/packages/eql-mapper/src/inference/mod.rs @@ -22,8 +22,8 @@ use sqltk::parser::ast::{ 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::*; @@ -64,6 +64,13 @@ 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 @@ -74,12 +81,13 @@ pub struct TypeInferencer<'ast> { /// 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 whether the chain is legal depends - /// entirely on that parent. `j -> 'a' -> 'b' = $1` is one path into one - /// document and fuses; the same chain in a projection has no fusion to - /// collapse it and would emit an entry-scoped accessor over an entry, - /// returning NULL. `infer_enter` on the comparison marks the chain before - /// any of it is typed, so the `->` rule can tell the two apart. + /// 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 @@ -101,6 +109,7 @@ 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, @@ -113,6 +122,12 @@ 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()) @@ -127,8 +142,9 @@ impl<'ast> TypeInferencer<'ast> { } /// Whether this node is the outermost accessor of a chain a comparison will - /// fuse. A chain nothing fuses has no correct rewrite, so it must not be - /// permitted past the type check. + /// 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() @@ -147,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( @@ -163,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/type_error.rs b/packages/eql-mapper/src/inference/type_error.rs index 83efb0139..a1a78f6c2 100644 --- a/packages/eql-mapper/src/inference/type_error.rs +++ b/packages/eql-mapper/src/inference/type_error.rs @@ -22,19 +22,52 @@ pub enum TypeError { /// /// 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 fused into a single path instead, - /// so this is reached only when the chain is broken up such that the - /// selectors cannot be composed: across a subquery boundary, a CTE, or a - /// view. + /// 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. A multi-step path is \ - resolved against the whole document only by exact equality (`col -> 'a' \ - -> 'b' = $1`, or `<>`); anywhere else, select the one field you need" + 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/json_value_selector.rs b/packages/eql-mapper/src/json_value_selector.rs index efc360342..83331cb84 100644 --- a/packages/eql-mapper/src/json_value_selector.rs +++ b/packages/eql-mapper/src/json_value_selector.rs @@ -14,6 +14,7 @@ //! [`EqlTerm::JsonValueSelector`]: crate::EqlTerm::JsonValueSelector use std::collections::HashMap; +use std::marker::PhantomData; use sqltk::parser::ast::{self}; use sqltk::parser::ast::{ @@ -173,35 +174,96 @@ fn is_json_accessor_fn(name: &ast::ObjectName) -> bool { ) } -/// The set of fused JSON value selectors in a statement: for each operand that -/// carries the *value* half, where its *path* half comes from. +/// 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)) } diff --git a/packages/eql-mapper/src/lib.rs b/packages/eql-mapper/src/lib.rs index 810073795..6324fc960 100644 --- a/packages/eql-mapper/src/lib.rs +++ b/packages/eql-mapper/src/lib.rs @@ -3432,13 +3432,18 @@ mod test { /// /// `->` yields `EqlTerm::JsonExtracted` — one SteVec entry, which carries no /// `sv` array and so cannot be traversed. A chain written in one expression - /// is fused into a single path against the document instead, but once the + /// 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] @@ -3467,45 +3472,219 @@ mod test { } } - /// A multi-step chain is only legal where something will fuse it into a - /// single path. Nothing else has a correct rewrite. + /// A multi-step chain collapses to a SINGLE accessor on the root document, + /// in every context — not only under an equality. /// - /// Only exact equality collapses a chain, into one value-selector containment - /// against the root document. Everywhere else the chain would survive into - /// the emitted SQL as nested entry-scoped accessors applied to an entry, - /// which selects nothing and yields NULL — the silent-wrong-answer failure - /// this whole family of types exists to prevent. + /// 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. /// - /// Typing is post-order, so when the outermost `->` of a chain is typed its - /// parent is not yet known. `infer_enter` on the comparison marks the chain's - /// spine on the way down, which is what lets the `->` rule distinguish - /// "about to be fused" from "nothing will collapse this". + /// 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_is_refused_where_nothing_fuses_it() { + fn a_multi_step_chain_collapses_to_one_accessor_in_every_context() { let schema = chained_json_schema(); - for sql in [ - // A projection has no comparison to fuse into. - "SELECT j -> 'foo' -> 'bar' FROM t", - "SELECT (j -> 'foo') -> 'bar' FROM t", - "SELECT j -> 'foo' ->> 'bar' FROM t", - // Ordering types the scalar operand but leaves the chain standing, - // so a multi-step ordering comparison has no correct rewrite either. - "SELECT id FROM t WHERE j -> 'foo' -> 'bar' < '\"x\"'", - "SELECT id FROM t WHERE j -> 'foo' -> 'bar' >= '\"x\"'", + // 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 statement = parse(sql); - let err = type_check(schema.clone(), &statement) - .expect_err(&format!("`{sql}` must not type check")); + 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!( - err.to_string() - .contains("result of an encrypted JSON operation"), - "expected an unqueryable-extraction error for `{sql}`, got: {err}" + !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(); + } + /// A SINGLE access is legal anywhere, fused or not. /// /// The declaration handles it: `-> ::Output` yields an diff --git a/packages/eql-mapper/src/param_plan.rs b/packages/eql-mapper/src/param_plan.rs index af02b10d5..50589ae21 100644 --- a/packages/eql-mapper/src/param_plan.rs +++ b/packages/eql-mapper/src/param_plan.rs @@ -35,6 +35,17 @@ pub enum OutputParamSource { 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 { @@ -45,6 +56,17 @@ impl OutputParamSource { 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/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)), From 7b656cce07d52492afd8ab9caeba152235bb6955 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 16:13:29 +1000 Subject: [PATCH 10/12] test(integration): read a two-level encrypted JSON path back through Proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Type-checking is not evidence here. The failure this change fixes is not an error — it is a query that runs perfectly, decrypts nothing, and answers NULL. So these assert the VALUE that comes back through a live Proxy, not merely that nothing blew up. What each one pins that the mapper tests cannot: - a two-level projection returns "world", the field at `$.nested.string` - all four spellings of that path — bare, parenthesised, `->>`, and the `jsonb_path_query_first` step — return the SAME value, so a walker that missed one cannot pass by composing a short path - the composed path is not a prefix of itself: `$.string` holds "hello" and `$.nested.string` holds "world", so dropping the inner step would read the wrong field and still look like a success - a param step resolves at Bind, with one step bound and with both - ordering compares the field at the path, including at the boundary (42 >= 42), because a chain left as two hops would compare NULL and read as "no rows" rather than as a failure - equality still matches, unchanged - `j -> $1 -> 'b'` never answers from a truncated path - a chain split across a subquery is never rewritten into a value The document is inserted through Proxy and read back through Proxy, so the selector each query composes has to key the same path the stored `sv` entries were keyed on — which is the part no unit test can check. --- .../src/select/jsonb_accessor_chain.rs | 342 ++++++++++++++++++ .../src/select/mod.rs | 1 + 2 files changed, 343 insertions(+) create mode 100644 packages/cipherstash-proxy-integration/src/select/jsonb_accessor_chain.rs 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/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; From 462706eccbf90a1d737bd8e24b7586d88fffb434 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 17:22:18 +1000 Subject: [PATCH 11/12] fix(mapper): an extracted JSON field is orderable and equatable, not inert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `EqlTerm::JsonExtracted` was given no capabilities at all, on the theory that nothing further can be done with an extracted value. That over-reached: a SteVec entry carries ordering and equality terms, so `ORDER BY col -> 'field'` is a legitimate, previously-working query that sorts by `ord_term(eql_v3."->"(col, sel))` — and it started failing the `Ord` bound. CI caught it in the showcase, not the test suites, and the failure mode is worth recording: with mapping errors disabled (the proxy container's default) the refused statement was forwarded to PostgreSQL unmapped, so native jsonb `->` ran over ciphertext and the "active Aspirin prescriptions" query returned 0 rows with no error anywhere. The type error existed; the flag threw it away. (Exactly the failure CIP-3680 removes.) The entry's capabilities are now `eq` and `ord`, MASKED from the column's own traits rather than granted outright — a JSON domain that carries no ordering term must not gain one by extraction. The JSON capabilities stay off: an entry has no `sv`, so traversal remains refused, which is the distinction this type exists to draw. Also: the showcase's ports are now environment-overridable, mirroring the integration suite. Diagnosing this required running the showcase against an isolated Proxy, and its hardcoded 6432 was the last thing standing in the way — the same problem bf72e2fb solved for the integration crate. --- .../src/inference/unifier/eql_traits.rs | 20 +++++++++++--- packages/eql-mapper/src/lib.rs | 23 ++++++++++++++++ packages/showcase/src/common.rs | 26 ++++++++++++++----- packages/showcase/src/data.rs | 2 +- packages/showcase/src/main.rs | 12 ++++----- packages/showcase/src/schema.rs | 2 +- 6 files changed, 68 insertions(+), 17 deletions(-) diff --git a/packages/eql-mapper/src/inference/unifier/eql_traits.rs b/packages/eql-mapper/src/inference/unifier/eql_traits.rs index 086f65830..7041d36b2 100644 --- a/packages/eql-mapper/src/inference/unifier/eql_traits.rs +++ b/packages/eql-mapper/src/inference/unifier/eql_traits.rs @@ -343,9 +343,23 @@ impl EqlTerm { EqlTerm::Tokenized(_) => EqlTraits::none(), EqlTerm::JsonOrd(_) => EqlTraits::none(), EqlTerm::JsonValueSelector(_) => EqlTraits::none(), - // Unqueryable by construction: no operator or function can require - // any capability of an already-extracted JSON entry. - EqlTerm::JsonExtracted(_) => 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/lib.rs b/packages/eql-mapper/src/lib.rs index 6324fc960..7cae3d07e 100644 --- a/packages/eql-mapper/src/lib.rs +++ b/packages/eql-mapper/src/lib.rs @@ -3685,6 +3685,29 @@ mod test { 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 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 } From ea2212b9e5df2d62e7ece6cf655ad83453f308c2 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Thu, 30 Jul 2026 22:28:43 +1000 Subject: [PATCH 12/12] =?UTF-8?q?docs(showcase):=20chained=20JSON=20access?= =?UTF-8?q?ors=20are=20supported=20now=20=E2=80=94=20document=20the=20real?= =?UTF-8?q?=20limits?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The showcase's CLAUDE.md declared chained `->` a critical limitation of searchable encryption, to be lifted in a future release, and steered every nested access through the jsonb_path_query* workaround. That release is this branch: a chain written in one expression collapses into a single keyed path against the root document, in every position, so the guidance was steering examples away from syntax that now works — and would have made Claude generate needlessly contorted showcase examples. The section now documents what is actually true, including the limits that remain and why each one exists rather than a bare "does not work": - a path split across a subquery/CTE/view is impossible, not pending — the extracted entry has nothing left to traverse and the root column is out of scope; it is rejected with a type error - an extracted field is orderable and equatable but not a document - a placeholder step followed by a literal step is refused, because the literal is encrypted before the placeholder is bound - nested jsonb_path_query* calls are not collapsed; write one path Every claim was verified against the mapper this session, including the multi-step ORDER BY shape, which was probed rather than assumed. --- packages/showcase/CLAUDE.md | 58 +++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 16 deletions(-) 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