Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

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.

- **`ANY`/`ALL` over an array of encrypted values**: `col = ANY(ARRAY['a', 'b'])` now works on an encrypted column whose domain supports the operator, rewritten elementwise to the same term form as the scalar comparison — each array element is encrypted like any other comparison operand. `ARRAY[…]` written in the statement is the supported spelling; an encrypted subquery projection (`ANY(SELECT enc FROM …)`) or a bare array parameter (`ANY($1)`) is rejected with an explanatory error, where previously the subquery form was forwarded unrewritten and silently matched nothing.

### 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.

- **Literals and params that escape type checking now fail closed**: a literal or parameter whose type was never worked out during type checking used to be silently assumed to be plaintext — so a value in a clause the type checker did not cover could skip encryption without any error. Proxy now only makes that assumption where it is provably safe: a value that only flows to the client through a `SELECT` projection (`SELECT 'lit'`, `SELECT $1`), or comparison operands that relate to nothing but each other (`WHERE 1 = 1`, `1 IN (1, 2)`, and friends — anything encrypted arriving in such a comparison types it concretely first). Anywhere else the statement is rejected with an error naming the value. As part of this, `WHERE`/`HAVING`/join `ON` conditions and `ORDER BY`/`GROUP BY` ordinals are now explicitly typed as plaintext where they appear, and an encrypted column used bare as a boolean condition (for example `WHERE enc_col`) is rejected instead of being forwarded to the database.

- **`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.
Expand Down
5 changes: 4 additions & 1 deletion packages/eql-mapper/src/eql_mapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ impl<'ast> EqlMapper<'ast> {
.borrow_mut()
.resolve_unresolved_associated_types();

let _ = self.unifier.borrow_mut().resolve_unresolved_value_nodes();
// A failure here is a genuine type-checking failure: it means a value node escaped
// inference entirely, and assuming it is native would be fail-open (the value could
// relate to an encrypted column and silently skip encryption).
self.unifier.borrow_mut().resolve_unresolved_value_nodes()?;

let projection = self.projection_type(statement);
let params = self.param_types(&self.unifier.borrow());
Expand Down
96 changes: 92 additions & 4 deletions packages/eql-mapper/src/inference/infer_type_impls/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> {
self.unify_node_with_type(&**b, ty.clone())?;
self.unify_node_with_type(expr_val, Type::native())?;
self.unify_nodes(&**a, &**b)?;

// The result is native regardless of the operand type, so a
// literal-only operand pair (`1 IS DISTINCT FROM 2`) may stay
// unresolved — mark it groundable, as for `=` (see BinaryOp).
let a_ty = self.get_node_type(&**a);
self.unifier.borrow_mut().mark_natively_groundable(a_ty);
}

Expr::InList {
Expand All @@ -158,6 +164,12 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> {
// inconsistent with `=` on the same column, which is caught
// here.
self.unify_node_with_bound(&**expr, EqlTrait::Eq)?;

// The result is native regardless of the operand type, so a
// literal-only operand group (`1 IN (1, 2)`) may stay
// unresolved — mark it groundable, as for `=` (see BinaryOp).
let expr_ty = self.get_node_type(&**expr);
self.unifier.borrow_mut().mark_natively_groundable(expr_ty);
}

Expr::InSubquery {
Expand Down Expand Up @@ -191,6 +203,11 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> {
self.unify_node_with_type(&**expr, ty.clone())?;
self.unify_node_with_type(&**low, ty.clone())?;
self.unify_node_with_type(&**high, ty.clone())?;

// The result is native regardless of the operand type, so a
// literal-only operand group (`1 BETWEEN 0 AND 2`) may stay
// unresolved — mark it groundable, as for `=` (see BinaryOp).
self.unifier.borrow_mut().mark_natively_groundable(ty);
}

Expr::BinaryOp { left, op, right } => {
Expand Down Expand Up @@ -371,6 +388,20 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> {
};

get_sql_binop_rule(op).apply_constraints(self, lhs, rhs, expr_val)?;

// A comparison's result is native regardless of its operand
// type, so the operands (now unified with each other) may
// stay unresolved when both are literals — `WHERE 1 = 1`.
// Mark the pair as safe to default to native at the end of
// inference: anything encrypted meeting it before then
// grounds it concretely and the mark becomes irrelevant.
// Eager grounding here would be wrong — a shared param
// (`WHERE $1 = 1 AND enc = $1`) can still be made EQL by a
// later occurrence.
if comparison_capability(op).is_some() {
let lhs_ty = self.get_node_type(lhs);
self.unifier.borrow_mut().mark_natively_groundable(lhs_ty);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// The operands of a predicate reach PostgreSQL as query
Expand Down Expand Up @@ -460,14 +491,62 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> {
right,
} => {
self.unify_node_with_type(expr_val, Type::native())?;
self.unify_nodes(&**left, &**right)?;

// `x <op> ANY/ALL (…)` applies `<op>` to every element, so the
// capability is the operator's. Discarding `compare_op` left
// both `= ANY` and `> ANY` unconstrained.
// `x <op> ANY/ALL (rhs)` applies `<op>` between `x` and every
// ELEMENT of `rhs`, so when `rhs` resolved to an array its
// element type is what `x` unifies with — unifying with the
// array itself made `1 = ANY(ARRAY[1, 2])` type `1` as an
// array (and made `enc = ANY(ARRAY[…])` a conflict). Any other
// right-hand shape (a cast like `$1::int[]`, which is opaquely
// native; a subquery, whose single-column projection unifies
// with a scalar) keeps the direct unification.
let right_ty = self.get_node_type(&**right);
let rhs_is_array = matches!(&*right_ty, Type::Value(Value::Array(_)));
if let Type::Value(Value::Array(crate::unifier::Array(elem_ty))) = &*right_ty {
self.unify_node_with_type(&**left, elem_ty.clone())?;
} else {
self.unify_nodes(&**left, &**right)?;
}

// As for a binary comparison, the operator decides the
// capability the operands must carry: `= ANY` needs `Eq`,
// `< ALL` needs `Ord`.
if let Some(eql_trait) = comparison_capability(compare_op) {
self.unify_node_with_bound(&**left, eql_trait)?;
}

// Encrypted operands are supported only in the ARRAY-literal
// shape, which `RewriteEqlAnyAllOps` rewrites to the term form
// elementwise. A subquery projection or a bare array param has
// no rewrite: emitted as-is it would compare the raw jsonb
// payloads — whose ciphertext is randomised per row — and
// silently match nothing, so refuse loudly instead. (Checked on
// both sides: after the scalar-with-projection special case the
// encrypted type can be recorded against either node.)
if !rhs_is_array
&& (self.get_node_type(&**left).contains_eql()
|| self.get_node_type(&**right).contains_eql())
{
return Err(TypeError::UnsupportedSqlFeature(
"ANY/ALL over an encrypted subquery or array parameter (use ARRAY[...])"
.into(),
));
}

// The operands of the predicate reach PostgreSQL as query
// operands — terms only, never a ciphertext — exactly as for a
// binary comparison.
if let Expr::Array(ast::Array { elem, .. }) = &**right {
self.record_query_operands(std::iter::once(&**left).chain(elem.iter()));
} else {
self.record_query_operands([&**left]);
}

// The result is native regardless of the operand type, so a
// literal-only operand group (`1 = ANY(ARRAY[1])`) may stay
// unresolved — mark it groundable, as for `=` (see BinaryOp).
let left_ty = self.get_node_type(&**left);
self.unifier.borrow_mut().mark_natively_groundable(left_ty);
}

Expr::Ceil { expr, .. }
Expand Down Expand Up @@ -612,6 +691,15 @@ impl<'ast> InferType<'ast, Expr> for TypeInferencer<'ast> {
// The comparison is equality, so the operand's domain
// has to carry an equality term.
self.unify_node_with_bound(&**operand, EqlTrait::Eq)?;

// The CASE's own type is the results', independent of
// the operand's, so a literal-only operand group
// (`CASE 1 WHEN 1 …`) may stay unresolved — mark it
// groundable, as for `=` (see BinaryOp).
let operand_ty = self.get_node_type(&**operand);
self.unifier
.borrow_mut()
.mark_natively_groundable(operand_ty);
}
None => {
for cond_when in conditions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ impl<'ast> InferType<'ast, Query> for TypeInferencer<'ast> {
for order_by_expr in exprs {
let key = resolve_positional_key(select, &order_by_expr.expr);
self.unify_node_with_bound(key, EqlTrait::Ord)?;

// A key written as a literal (`ORDER BY 1`) reaches the
// database as a plain constant — PostgreSQL only accepts
// integer ordinals here — so the literal itself is always
// native, independently of the projected column it selects.
// This also covers ordinals that cannot be resolved against a
// projection, e.g. `ORDER BY 1` after a set operation.
if matches!(&order_by_expr.expr, Expr::Value(_)) {
self.unify_node_with_type(&order_by_expr.expr, Type::native())?;
}
}
}

Expand Down
64 changes: 63 additions & 1 deletion packages/eql-mapper/src/inference/infer_type_impls/select.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use eql_mapper_macros::trace_infer;
use sqltk::parser::ast::{Distinct, Expr, GroupByExpr, Select, SelectItem};
use sqltk::parser::ast::{
Distinct, Expr, GroupByExpr, JoinConstraint, JoinOperator, Select, SelectItem,
};

use super::query_statement::resolve_positional_key;
use crate::unifier::{Projection, Type, Value};
Expand Down Expand Up @@ -31,6 +33,58 @@ impl<'ast> InferType<'ast, Select> for TypeInferencer<'ast> {
}
}

// `WHERE`, `HAVING` and join `ON` conditions are boolean expressions,
// and booleans are always native — every EQL comparison produces a
// native result. Pin the condition to `Native` so that a bare literal
// or placeholder condition (`WHERE true`, `ON true`, `WHERE $1`) is
// typed where the clause is inferred instead of relying on the late
// unresolved-value fallback, and so that an encrypted value can never
// itself be the condition.
if let Some(selection) = &select.selection {
self.unify_node_with_type(selection, Type::native())?;
}

if let Some(having) = &select.having {
self.unify_node_with_type(having, Type::native())?;
}

for table_with_joins in &select.from {
for join in &table_with_joins.joins {
let constraint = match &join.join_operator {
JoinOperator::Join(constraint)
| JoinOperator::Inner(constraint)
| JoinOperator::Left(constraint)
| JoinOperator::LeftOuter(constraint)
| JoinOperator::Right(constraint)
| JoinOperator::RightOuter(constraint)
| JoinOperator::FullOuter(constraint)
| JoinOperator::Semi(constraint)
| JoinOperator::LeftSemi(constraint)
| JoinOperator::RightSemi(constraint)
| JoinOperator::Anti(constraint)
| JoinOperator::LeftAnti(constraint)
| JoinOperator::RightAnti(constraint)
| JoinOperator::StraightJoin(constraint) => Some(constraint),

JoinOperator::AsOf {
match_condition,
constraint,
} => {
self.unify_node_with_type(match_condition, Type::native())?;
Some(constraint)
}

JoinOperator::CrossJoin
| JoinOperator::CrossApply
| JoinOperator::OuterApply => None,
};

if let Some(JoinConstraint::On(condition)) = constraint {
self.unify_node_with_type(condition, Type::native())?;
}
}
}

// Deduplication is equality, so every expression `DISTINCT` dedupes on
// must support it. For an encrypted column that means its domain has to
// carry an equality term — `eql_v3_boolean`, for instance, is
Expand Down Expand Up @@ -75,6 +129,14 @@ impl<'ast> InferType<'ast, Select> for TypeInferencer<'ast> {
for expr in exprs {
let key = resolve_positional_key(Some(select), expr);
self.unify_node_with_bound(key, EqlTrait::Eq)?;

// A key written as a literal (`GROUP BY 1`) reaches the
// database as a plain constant — PostgreSQL only accepts
// integer ordinals here — so the literal itself is always
// native, independently of the projected column it selects.
if matches!(expr, Expr::Value(_)) {
self.unify_node_with_type(expr, Type::native())?;
}
}
}

Expand Down
7 changes: 7 additions & 0 deletions packages/eql-mapper/src/inference/type_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ pub enum TypeError {
#[error("unified type contains unresolved type variable: {}", _0)]
Incomplete(String),

#[error(
"the type of value `{}` was never constrained during type inference; \
refusing to assume it is native",
_0
)]
UnresolvedValue(String),

#[error("{}", _0)]
Expected(String),

Expand Down
Loading
Loading