Skip to content

Commit e2198c7

Browse files
authored
Merge pull request #439 from cipherstash/james/cip-3700-eql-mapper-inference-hardening-unconstrained-limitfetch
fix(mapper): inference hardening — LIMIT/FETCH, UPDATE targets, unmatched statements (CIP-3700)
2 parents 7c6342f + 561e9c6 commit e2198c7

5 files changed

Lines changed: 326 additions & 18 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
66

77
## [Unreleased]
88

9+
### Fixed
10+
11+
- **`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.
12+
13+
- **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.
14+
15+
- **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.
16+
917
## [3.0.0] - 2026-08-05
1018

1119
### Changed

packages/eql-mapper/src/inference/infer_type_impls/query_statement.rs

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
use eql_mapper_macros::trace_infer;
22
use sqltk::parser::ast::{
3-
Expr, OrderBy, OrderByKind, Query, Select, SelectItem, SetExpr, Value as SqltkValue,
3+
Expr, Fetch, LimitClause, Offset, OrderBy, OrderByKind, Query, Select, SelectItem, SetExpr,
4+
Value as SqltkValue,
45
};
56

67
use crate::{
7-
inference::{InferType, TypeError},
8+
inference::{unifier::Type, InferType, TypeError},
89
EqlTrait, TypeInferencer,
910
};
1011

@@ -45,7 +46,13 @@ pub(crate) fn resolve_positional_key<'ast>(
4546
#[trace_infer]
4647
impl<'ast> InferType<'ast, Query> for TypeInferencer<'ast> {
4748
fn infer_exit(&mut self, query: &'ast Query) -> Result<(), TypeError> {
48-
let Query { body, order_by, .. } = query;
49+
let Query {
50+
body,
51+
order_by,
52+
limit_clause,
53+
fetch,
54+
..
55+
} = query;
4956

5057
self.unify_nodes(query, &**body)?;
5158

@@ -78,6 +85,51 @@ impl<'ast> InferType<'ast, Query> for TypeInferencer<'ast> {
7885
}
7986
}
8087

88+
// Row-count expressions in LIMIT/OFFSET/FETCH are evaluated by the
89+
// database as plain integers and can never be encrypted, so pin them
90+
// to `Native`. Without this a placeholder in `LIMIT $1` is left as an
91+
// unconstrained type variable, which later surfaces as an opaque
92+
// "unresolved type variable" error instead of type-checking cleanly.
93+
// `Query::locks` (FOR UPDATE/SHARE) carries no expressions, so there
94+
// is nothing to constrain there.
95+
if let Some(limit_clause) = limit_clause {
96+
match limit_clause {
97+
LimitClause::LimitOffset {
98+
limit,
99+
offset,
100+
limit_by,
101+
} => {
102+
if let Some(limit) = limit {
103+
self.unify_node_with_type(limit, Type::native())?;
104+
}
105+
if let Some(Offset { value, .. }) = offset {
106+
self.unify_node_with_type(value, Type::native())?;
107+
}
108+
// `LIMIT n BY expr, …` (ClickHouse syntax) — the BY
109+
// expressions are per-group keys, not row counts, so
110+
// `Native` would be the wrong constraint, and grouping on
111+
// an encrypted column would need its equality term.
112+
// PostgreSQL rejects the syntax anyway; rejecting it here
113+
// keeps the keys from passing through unconstrained.
114+
if !limit_by.is_empty() {
115+
return Err(TypeError::UnsupportedSqlFeature("LIMIT ... BY".into()));
116+
}
117+
}
118+
LimitClause::OffsetCommaLimit { offset, limit } => {
119+
self.unify_node_with_type(offset, Type::native())?;
120+
self.unify_node_with_type(limit, Type::native())?;
121+
}
122+
}
123+
}
124+
125+
if let Some(Fetch {
126+
quantity: Some(quantity),
127+
..
128+
}) = fetch
129+
{
130+
self.unify_node_with_type(quantity, Type::native())?;
131+
}
132+
81133
Ok(())
82134
}
83135
}

packages/eql-mapper/src/inference/infer_type_impls/statement.rs

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
1+
use std::sync::Arc;
2+
13
use eql_mapper_macros::trace_infer;
2-
use sqltk::parser::ast::{AssignmentTarget, ObjectName, ObjectNamePart, Statement};
4+
use sqltk::parser::ast::{AssignmentTarget, ObjectName, ObjectNamePart, Statement, TableFactor};
35

4-
use crate::{inference::infer_type::InferType, unifier::Type, TypeError, TypeInferencer};
6+
use crate::{
7+
inference::infer_type::InferType,
8+
unifier::{EqlTerm, EqlValue, NativeValue, Type, Value},
9+
ColumnKind, TableColumn, TypeError, TypeInferencer,
10+
};
511

612
#[trace_infer]
713
impl<'ast> InferType<'ast, Statement> for TypeInferencer<'ast> {
@@ -20,19 +26,48 @@ impl<'ast> InferType<'ast, Statement> for TypeInferencer<'ast> {
2026
}
2127

2228
Statement::Update {
23-
// FIXME: use table to resolve the assignments (instead of looking up the columns names in the scope).
24-
table: _,
29+
table,
2530
assignments,
2631
returning,
2732
..
2833
} => {
34+
// Assignment targets belong to the table being updated, so
35+
// resolve them against `table` directly. Resolving through the
36+
// lexical scope would also see every `FROM`-joined relation,
37+
// letting a same-named column there shadow the target column
38+
// (or make it spuriously ambiguous).
39+
let target_table = match &table.relation {
40+
TableFactor::Table { name, .. } if table.joins.is_empty() => name,
41+
_ => {
42+
return Err(TypeError::UnsupportedSqlFeature(
43+
"UPDATE target that is not a plain table".into(),
44+
))
45+
}
46+
};
47+
2948
for assignment in assignments.iter() {
3049
match &assignment.target {
3150
AssignmentTarget::ColumnName(ObjectName(parts)) if parts.len() == 1 => {
3251
let ObjectNamePart::Identifier(ident) = parts.last().unwrap();
52+
let stc = self
53+
.table_resolver
54+
.resolve_table_column(target_table, ident)?;
55+
56+
let tc = TableColumn {
57+
table: stc.table.clone(),
58+
column: stc.column.clone(),
59+
};
60+
61+
let value_ty = match &stc.kind {
62+
ColumnKind::Native => Value::Native(NativeValue(Some(tc))),
63+
ColumnKind::Eql(features, identity) => Value::Eql(EqlTerm::Full(
64+
EqlValue(tc, identity.clone(), *features),
65+
)),
66+
};
67+
3368
self.unify_node_with_type(
3469
&assignment.value,
35-
self.resolve_ident(ident)?,
70+
Arc::new(Type::Value(value_ty)),
3671
)?;
3772
}
3873

@@ -88,7 +123,20 @@ impl<'ast> InferType<'ast, Statement> for TypeInferencer<'ast> {
88123
// EXPLAIN itself returns metadata, not the query results - give it empty projection
89124
self.unify_node_with_type(statement, Type::empty_projection())?;
90125
}
91-
_ => {}
126+
127+
// Invariant: every statement variant admitted by
128+
// `requires_type_check` (see `eql_mapper.rs`) has an explicit arm
129+
// above that constrains the statement's top-level type. This arm
130+
// fails closed so that widening `requires_type_check` without
131+
// adding a matching arm becomes a loud error instead of a
132+
// silently-unconstrained statement.
133+
unhandled => {
134+
return Err(TypeError::InternalError(format!(
135+
"type inference has no rule for statement `{unhandled}`; \
136+
`requires_type_check` admits a statement variant that \
137+
`InferType<'_, Statement>` does not handle"
138+
)))
139+
}
92140
};
93141

94142
Ok(())

packages/eql-mapper/src/lib.rs

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,6 +1303,204 @@ mod test {
13031303
);
13041304
}
13051305

1306+
/// In `UPDATE t1 SET x = ... FROM t2` the assignment target must resolve
1307+
/// against the table being updated, not through the lexical scope. The
1308+
/// scope also contains the `FROM` relations, so a same-named column there
1309+
/// used to make the target spuriously ambiguous (and could shadow it).
1310+
/// Here both tables have an `email` column; the assignment must get
1311+
/// `users.email` — the encrypted one.
1312+
#[test]
1313+
fn update_assignment_resolves_against_target_table_not_from_relation() {
1314+
let schema = resolver(schema! {
1315+
tables: {
1316+
users: {
1317+
id,
1318+
email (EQL: Eq),
1319+
}
1320+
aux: {
1321+
id,
1322+
email,
1323+
}
1324+
}
1325+
});
1326+
1327+
let statement = parse("UPDATE users SET email = $1 FROM aux WHERE users.id = aux.id");
1328+
1329+
let typed = match type_check(schema, &statement) {
1330+
Ok(typed) => typed,
1331+
Err(err) => panic!("type check failed: {err}"),
1332+
};
1333+
1334+
let target = Value::Eql(EqlTerm::Full(EqlValue::with_canonical_identity(
1335+
TableColumn {
1336+
table: id("users"),
1337+
column: id("email"),
1338+
},
1339+
EqlTraits::from(EqlTrait::Eq),
1340+
)));
1341+
1342+
assert_eq!(typed.params, vec![(Param(1), target)]);
1343+
assert_eq!(typed.projection, Projection(vec![]));
1344+
}
1345+
1346+
/// Proxy loads its schema from the database with *quoted* column idents
1347+
/// (`Ident::with_quote('"', ..)`) behind an editable resolver, while SQL
1348+
/// usually spells the same columns unquoted. A type identity derived from
1349+
/// an assignment target must still unify with one derived from the scope,
1350+
/// so the resolver has to return the schema's canonical idents rather than
1351+
/// echo the caller's spelling. With the caller's spelling,
1352+
/// `UPDATE t SET c = $1 WHERE c = $1` pinned the same param to
1353+
/// `EQL(t."c")` and `EQL(t.c)` and failed with "cannot unify EQL terms".
1354+
#[test]
1355+
fn update_reused_param_unifies_against_quoted_schema_idents() {
1356+
let eq = EqlTraits::from(EqlTrait::Eq);
1357+
1358+
let mut schema = Schema::new("public");
1359+
let mut table = crate::model::Table::new(Ident::new("encrypted"));
1360+
table.add_column(Arc::new(crate::model::Column::native(Ident::with_quote(
1361+
'"', "id",
1362+
))));
1363+
table.add_column(Arc::new(crate::model::Column::eql(
1364+
Ident::with_quote('"', "encrypted_text"),
1365+
eq,
1366+
crate::unifier::DomainIdentity::canonical(crate::unifier::TokenType::Text, eq),
1367+
)));
1368+
schema.add_table(table);
1369+
1370+
// The editable resolver is the one Proxy uses at runtime; it resolves
1371+
// through `SchemaDelta`, not `Schema`.
1372+
let resolver = Arc::new(TableResolver::new_editable(Arc::new(schema)));
1373+
1374+
let statement = parse("UPDATE encrypted SET encrypted_text = $1 WHERE encrypted_text = $1");
1375+
1376+
let typed = match type_check(resolver, &statement) {
1377+
Ok(typed) => typed,
1378+
Err(err) => panic!("type check failed: {err}"),
1379+
};
1380+
1381+
// The param's identity is the canonical (quoted) schema spelling.
1382+
let target = Value::Eql(EqlTerm::Full(EqlValue::with_canonical_identity(
1383+
TableColumn {
1384+
table: id("encrypted"),
1385+
column: Ident::with_quote('"', "encrypted_text"),
1386+
},
1387+
eq,
1388+
)));
1389+
1390+
assert_eq!(typed.params, vec![(Param(1), target)]);
1391+
}
1392+
1393+
/// The row-count expressions in `LIMIT`/`OFFSET` can never be encrypted,
1394+
/// so placeholders there must be pinned to `Native` at inference time.
1395+
/// Previously they were left as unconstrained type variables and only
1396+
/// resolved to `Native` by the late unresolved-value fallback in
1397+
/// `Unifier::resolve_unresolved_value_nodes` — this pins the guarantee
1398+
/// where the clause is inferred instead of relying on that fallback.
1399+
#[test]
1400+
fn limit_and_offset_placeholders_infer_native() {
1401+
let schema = resolver(schema! {
1402+
tables: {
1403+
users: {
1404+
id,
1405+
email (EQL: Eq),
1406+
}
1407+
}
1408+
});
1409+
1410+
let statement = parse("SELECT id FROM users LIMIT $1 OFFSET $2");
1411+
1412+
let typed = match type_check(schema, &statement) {
1413+
Ok(typed) => typed,
1414+
Err(err) => panic!("type check failed: {err}"),
1415+
};
1416+
1417+
assert_eq!(
1418+
typed.params,
1419+
vec![
1420+
(Param(1), Value::Native(NativeValue(None))),
1421+
(Param(2), Value::Native(NativeValue(None))),
1422+
]
1423+
);
1424+
}
1425+
1426+
/// Same as `limit_and_offset_placeholders_infer_native`, but for the
1427+
/// quantity in a `FETCH FIRST n ROWS ONLY` clause.
1428+
#[test]
1429+
fn fetch_first_placeholder_infers_native() {
1430+
let schema = resolver(schema! {
1431+
tables: {
1432+
users: {
1433+
id,
1434+
email (EQL: Eq),
1435+
}
1436+
}
1437+
});
1438+
1439+
let statement = parse("SELECT id FROM users FETCH FIRST $1 ROWS ONLY");
1440+
1441+
let typed = match type_check(schema, &statement) {
1442+
Ok(typed) => typed,
1443+
Err(err) => panic!("type check failed: {err}"),
1444+
};
1445+
1446+
assert_eq!(
1447+
typed.params,
1448+
vec![(Param(1), Value::Native(NativeValue(None)))]
1449+
);
1450+
}
1451+
1452+
/// Because `LIMIT` is pinned to `Native` at inference time, an encrypted
1453+
/// value can no longer flow into it silently — the mapper refuses the
1454+
/// statement instead of forwarding SQL that the database would reject
1455+
/// (or worse, that would leak a ciphertext into a row count).
1456+
#[test]
1457+
fn encrypted_column_in_limit_is_rejected() {
1458+
let schema = resolver(schema! {
1459+
tables: {
1460+
users: {
1461+
id,
1462+
email (EQL: Eq),
1463+
}
1464+
}
1465+
});
1466+
1467+
let statement = parse("SELECT id FROM users LIMIT email");
1468+
1469+
type_check(schema, &statement)
1470+
.expect_err("an encrypted column must not type check as a LIMIT row count");
1471+
}
1472+
1473+
/// A statement variant with no inference rule must fail closed with an
1474+
/// error stating the invariant, not traverse without constraining the
1475+
/// statement's top-level type. (`requires_type_check` never admits
1476+
/// `TRUNCATE`, so this can only be reached by calling `type_check`
1477+
/// directly — but if `requires_type_check` is ever widened without a
1478+
/// matching inference rule, this is the error that makes it loud.)
1479+
#[test]
1480+
fn statement_without_inference_rule_fails_closed() {
1481+
let schema = resolver(schema! {
1482+
tables: {
1483+
users: {
1484+
id,
1485+
}
1486+
}
1487+
});
1488+
1489+
let statement = parse("TRUNCATE TABLE users");
1490+
1491+
match type_check(schema, &statement) {
1492+
Ok(_) => panic!("expected type check to fail"),
1493+
Err(err) => assert_eq!(
1494+
err.to_string(),
1495+
format!(
1496+
"type inference has no rule for statement `{statement}`; \
1497+
`requires_type_check` admits a statement variant that \
1498+
`InferType<'_, Statement>` does not handle"
1499+
)
1500+
),
1501+
}
1502+
}
1503+
13061504
#[test]
13071505
fn delete() {
13081506
// init_tracing();

0 commit comments

Comments
 (0)