fix(core): refuse a second condition on a key in KeyConditionExpression

crates/core/src/expression/mod.rs

@@ -26,7 +26,7 @@ pub use ast::{ArithOp, CompareOp, Expr, PathElement, UpdateAction};
 pub use evaluator::evaluate_condition;
 pub use key_condition::{
     KeyClauseOperand, KeyCondition, SortKeyCondition, key_condition_clause_operands,
-    parse_key_condition,
+    parse_key_condition, parse_key_condition_with_maps,
 };
 pub use kind::ExpressionKind;
 pub use parser::{parse_condition, parse_condition_with_depth_limit};

crates/engine/src/expression_helpers.rs

@@ -8,8 +8,9 @@ use std::collections::HashMap;
 use extenddb_core::error::DynamoDbError;
 use extenddb_core::expression::{
     Expr, ExpressionKind, ExpressionMaps, KeyCondition, PathElement, Token, UpdateAction,
-    parse_condition_with_depth_limit, parse_key_condition, parse_projection, parse_update_from,
-    tokenize_for, tokenize_with_limit, validate_no_reserved_words, validate_ordering_operand_types,
+    parse_condition_with_depth_limit, parse_key_condition_with_maps, parse_projection,
+    parse_update_from, tokenize_for, tokenize_with_limit, validate_no_reserved_words,
+    validate_ordering_operand_types,
 };
 use extenddb_core::limits::LimitsConfig;
 use extenddb_core::types::{AttributeValue, ConditionalOperator, ExpectedAttributeValue};
@@ -237,14 +238,19 @@ pub fn parse_projection_expr(
 
 /// Size-check, tokenize, reserved-word check, and parse a `KeyConditionExpression`.
 ///
+/// `maps` holds the request's `ExpressionAttributeNames` and
+/// `ExpressionAttributeValues`, which the parse reads to refuse a second
+/// condition on one key attribute.
+///
 /// Errors carry the `KeyConditionExpression` prefix, matching Amazon `DynamoDB`.
 ///
 /// # Errors
 ///
 /// Returns `DynamoDbError::ValidationException` for over-size, syntax, or
-/// reserved-word errors.
+/// reserved-word errors, and for a second condition on one key attribute.
 pub fn parse_key_condition_expr(
     expr: &str,
+    maps: &ExpressionMaps,
     limits: &LimitsConfig,
 ) -> Result<KeyCondition, DynamoDbError> {
     check_expression_size(expr, ExpressionKind::KeyCondition, limits)?;
@@ -257,7 +263,7 @@ pub fn parse_key_condition_expr(
         if limits.enforce_reserved_keywords {
             validate_no_reserved_words(&tokens)?;
         }
-        parse_key_condition(&tokens)
+        parse_key_condition_with_maps(&tokens, &maps.names, &maps.values)
     })
     .map_err(|e| prefix_expression_error(e, ExpressionKind::KeyCondition))
 }
@@ -407,4 +413,111 @@ mod tests {
             "got {err:?}"
         );
     }
+
+    /// A key condition request and the message it answers: the expression,
+    /// its `#`-prefixed names, its `:`-prefixed values (`None` for no
+    /// `ExpressionAttributeValues`) and the expected message.
+    type KceCase = (
+        &'static str,
+        Vec<(&'static str, &'static str)>,
+        Option<Vec<(&'static str, AttributeValue)>>,
+        &'static str,
+    );
+
+    /// Parse `expr` as `Query` does, with `#`-prefixed `names` and
+    /// `:`-prefixed `values` as a request carries them.
+    fn parse_kce(
+        expr: &str,
+        names: &[(&str, &str)],
+        values: Option<&[(&str, AttributeValue)]>,
+    ) -> Result<KeyCondition, DynamoDbError> {
+        let names: HashMap<String, String> = names
+            .iter()
+            .map(|(k, v)| ((*k).to_owned(), (*v).to_owned()))
+            .collect();
+        let values: Option<HashMap<String, AttributeValue>> = values.map(|vs| {
+            vs.iter()
+                .map(|(k, v)| ((*k).to_owned(), v.clone()))
+                .collect()
+        });
+        let maps = build_expression_maps(Some(&names), values.as_ref());
+        parse_key_condition_expr(expr, &maps, &LimitsConfig::default())
+    }
+
+    /// The order of the one-condition-per-key check through the engine's
+    /// entry point, on the requests measured 2026-10-31 (table `pk` S HASH,
+    /// `sk` S RANGE). The key value type check and the unused-value check
+    /// run after this parse.
+    #[test]
+    fn key_condition_duplicate_check_order_matches_the_service() {
+        const ONE_PER_KEY: &str = "Invalid KeyConditionExpression: KeyConditionExpressions \
+                                   must only contain one condition per key";
+        const UNDEFINED_U: &str = "Invalid KeyConditionExpression: An expression attribute \
+                                   name used in the document path is not defined; attribute \
+                                   name: #u";
+        const UNDEFINED_A: &str = "Invalid KeyConditionExpression: An expression attribute \
+                                   value used in expression is not defined; attribute value: :a";
+        let s = |v: &str| AttributeValue::S(v.to_owned());
+        let n = AttributeValue::N("1".to_owned());
+        let abc = [(":a", s("alice")), (":b", s("bob")), (":c", s("carol"))];
+        let cases: Vec<KceCase> = vec![
+            (
+                "pk = :a AND pk = :b",
+                vec![],
+                Some(abc[..2].to_vec()),
+                ONE_PER_KEY,
+            ),
+            (
+                "#p = :a AND pk = :n",
+                vec![("#p", "pk")],
+                Some(vec![(":a", s("alice")), (":n", n.clone())]),
+                ONE_PER_KEY,
+            ),
+            (
+                "pk = :a AND #s = :b AND sk = :n",
+                vec![("#s", "sk")],
+                Some(vec![
+                    (":a", s("alice")),
+                    (":b", s("bob")),
+                    (":n", n.clone()),
+                ]),
+                ONE_PER_KEY,
+            ),
+            (
+                "#u = :a AND sk = :b AND sk = :c",
+                vec![],
+                Some(abc.to_vec()),
+                UNDEFINED_U,
+            ),
+            (
+                "pk = :a AND pk = :b AND #u = :c",
+                vec![],
+                Some(abc.to_vec()),
+                UNDEFINED_U,
+            ),
+            ("pk = :a AND pk = :b", vec![], None, UNDEFINED_A),
+            (
+                "pk = :a AND pk = :b",
+                vec![],
+                Some(vec![(":a", s("alice")), (":b", s("bob")), (":z", s("zed"))]),
+                ONE_PER_KEY,
+            ),
+            (
+                "#u = :a AND #u > :b",
+                vec![],
+                Some(abc[..2].to_vec()),
+                UNDEFINED_U,
+            ),
+        ];
+        for (expr, names, values, want) in cases {
+            let got = parse_kce(expr, &names, values.as_deref());
+            assert!(
+                matches!(&got, Err(DynamoDbError::ValidationException(msg)) if msg == want),
+                "expr {expr}: got {got:?}"
+            );
+        }
+        // Without a duplicate the parse leaves the value to the later checks.
+        let got = parse_kce("pk = :n", &[], Some(&[(":n", n)]));
+        assert!(got.is_ok(), "pk = :n: got {got:?}");
+    }
 }

crates/engine/src/query.rs

@@ -237,7 +237,12 @@ pub async fn handle_query(
     let (mut key_condition, legacy_kc_maps) = if let Some(kce_str) =
         input.key_condition_expression.as_deref()
     {
-        let parsed = crate::expression_helpers::parse_key_condition_expr(kce_str, &ctx.limits)?;
+        // The parse refuses a second condition on one key attribute, after
+        // the expression's `#name` and `:value` references are checked and
+        // before the clauses are classified, which is where the service
+        // reports it (measured 2026-10-31).
+        let parsed =
+            crate::expression_helpers::parse_key_condition_expr(kce_str, &maps, &ctx.limits)?;
         (parsed, None)
     } else if let Some(ref kc) = input.key_conditions {
         let key_schema_pairs: Vec<(String, bool)> = query_key_info

crates/server/src/leading_keys.rs

@@ -63,7 +63,7 @@
 use std::collections::HashMap;
 
 use extenddb_core::expression::{
-    Expr, PathElement, Token, key_condition_clause_operands, parse_key_condition,
+    Expr, PathElement, Token, key_condition_clause_operands, parse_key_condition_with_maps,
     tokenize_with_limit,
 };
 use extenddb_core::types::{KeySchemaElement, KeyType, TableKeyInfo};
@@ -268,8 +268,11 @@ fn query_leading_keys(input: &Value, key_info: &TableKeyInfo, max_tokens: usize)
 
     if let Some(kce) = input.get("KeyConditionExpression").and_then(Value::as_str) {
         let tokens = tokenize_with_limit(kce, max_tokens).ok()?;
-        // The clauses that name the partition attribute, as written.
-        let pk_clauses: Vec<_> = key_condition_clause_operands(&tokens)
+        // The clauses that name the partition attribute. A key condition with
+        // a second condition on one key attribute, written with plain names
+        // or through a `#name` reference, is refused here with the check the
+        // engine runs, so it leaves the key absent whatever its operators.
+        let pk_clauses: Vec<_> = key_condition_clause_operands(&tokens, &names, &values)
             .ok()?
             .into_iter()
             .filter(|clause| resolve_attr_name(&clause.path, &names).as_deref() == Some(pk_attr))
@@ -278,21 +281,17 @@ fn query_leading_keys(input: &Value, key_info: &TableKeyInfo, max_tokens: usize)
         // engine reads it. When the engine's classification refuses the
         // condition (a missing HASH attribute on a multi-attribute key schema,
         // for one) or the value cannot be read here (an `N` that is not a
-        // number, a placeholder with no value), the key stays absent and no
-        // other clause's operand is bound in its place: the equality, not the
-        // other clause, names the partition.
+        // number, a placeholder with no value), the key stays absent.
         if pk_clauses.iter().any(|clause| clause.equality) {
             return equality_pk_value(&tokens, key_schema, pk_attr, &names, &values);
         }
-        // Exactly one clause names the partition key, under an operator the
-        // engine rejects, such as `pk > :v`. The service binds LeadingKeys to
-        // that operand and evaluates the policy before it rejects the shape,
-        // so a principal fenced away from the value gets AccessDeniedException
+        // The one clause on the partition key uses an operator the engine
+        // rejects, such as `pk > :v`. The service binds LeadingKeys to that
+        // operand and evaluates the policy before it rejects the shape, so a
+        // principal fenced away from the value gets AccessDeniedException
         // while one fenced to it gets the shape's ValidationException
-        // (measured 2026-10-21 for `pk > :v`). A condition that does not
-        // parse, one that names no partition clause, and one with two
-        // non-equality partition clauses (which value the service binds for
-        // that shape is not measured) leave the key absent.
+        // (measured 2026-10-21 for `pk > :v`). A condition that does not parse
+        // and one that names no partition clause leave the key absent.
         let [clause] = pk_clauses.as_slice() else {
             return None;
         };
@@ -348,7 +347,7 @@ fn equality_pk_value(
     names: &HashMap<String, String>,
     values: &HashMap<String, Value>,
 ) -> Option<String> {
-    let mut kc = parse_key_condition(tokens).ok()?;
+    let mut kc = parse_key_condition_with_maps(tokens, names, values).ok()?;
     // Assign every clause to its key attribute with the engine's own
     // classification: by name on a multi-attribute key schema, where the
     // first HASH attribute is the one LeadingKeys names, and by

crates/server/src/leading_keys/tests.rs

@@ -622,18 +622,18 @@ fn query_between_on_the_partition_key_is_absent() {
     assert_eq!(r, None);
 }
 
-/// An equality on the partition key decides the key, even when its value
-/// cannot be read and another clause names the partition key: the other
-/// clause's operand is never bound in its place. With `:u = {"N": "bob"}`
-/// (not a number) the key stays absent, as on main, rather than becoming
-/// `alice`.
+/// An equality on the partition key and a second clause on the partition key
+/// name the partition key twice, so the duplicate check refuses the condition
+/// and the key stays absent: the other clause's operand (`alice`) is never
+/// bound, whether the equality's value can be read or not.
 #[test]
 fn query_unreadable_partition_equality_binds_no_other_clause() {
     for (kce, u) in [
         ("pk = :u AND pk > :v", json!({"N": "bob"})),
         ("pk > :v AND pk = :u", json!({"N": "bob"})),
         ("#p = :u AND pk > :v", json!({"N": "bob"})),
         ("pk = :u AND begins_with(pk, :v)", json!({"N": "1e"})),
+        ("pk = :u AND pk > :v", json!({"S": "bob"})),
     ] {
         let input = json!({
             "TableName": "t",
@@ -644,18 +644,40 @@ fn query_unreadable_partition_equality_binds_no_other_clause() {
         let r = single_table_leading_keys(&input, "Query", Some(&main_table()), MAX_TOKENS);
         assert_eq!(r, None, "{kce}");
     }
-    // A readable equality keeps its own value with another partition clause.
-    let input = json!({
-        "TableName": "t",
-        "KeyConditionExpression": "pk = :u AND pk > :v",
-        "ExpressionAttributeValues": {":u": {"S": "bob"}, ":v": {"S": "alice"}},
-    });
-    let r = single_table_leading_keys(&input, "Query", Some(&main_table()), MAX_TOKENS);
-    assert_ne!(r, Some(vec!["alice".to_owned()]));
 }
 
-/// Two non-equality clauses on the partition key: which operand the service
-/// binds is not measured, so neither is bound, in either clause order.
+/// A second condition on one key attribute, written with plain names or
+/// through a `#name` reference, leaves the key absent, including when the
+/// only partition clause is a non-equality whose operand would otherwise be
+/// bound. The engine refuses all of these.
+#[test]
+fn query_duplicate_key_condition_leaves_the_key_absent() {
+    for kce in [
+        "pk = :a AND pk = :b",
+        "#p = :a AND pk = :b",
+        "pk = :a AND sk = :b AND sk = :c",
+        "pk = :a AND #s = :b AND sk = :c",
+        "pk > :a AND sk = :b AND sk = :c",
+        "pk > :a AND #s = :b AND sk = :c",
+    ] {
+        let input = json!({
+            "TableName": "t",
+            "KeyConditionExpression": kce,
+            "ExpressionAttributeNames": {"#p": "pk", "#s": "sk"},
+            "ExpressionAttributeValues": {
+                ":a": {"S": "alice"},
+                ":b": {"S": "bob"},
+                ":c": {"S": "carol"},
+            },
+        });
+        let r = single_table_leading_keys(&input, "Query", Some(&main_table()), MAX_TOKENS);
+        assert_eq!(r, None, "{kce}");
+    }
+}
+
+/// Two non-equality clauses on the partition key name it twice, so the
+/// duplicate check refuses the condition and neither operand is bound, in
+/// either clause order.
 #[test]
 fn query_two_non_equality_partition_clauses_bind_nothing() {
     for kce in [

tests/test_fgac_leading_keys.py

@@ -96,6 +96,12 @@ def _denied(exc: ClientError) -> bool:
     return exc.value.response["Error"]["Code"] == "AccessDeniedException"
 
 
+ONE_PER_KEY = (
+    "Invalid KeyConditionExpression: "
+    "KeyConditionExpressions must only contain one condition per key"
+)
+
+
 # The values a Query on the multi-attribute key table may name; each query
 # carries only the ones its key condition uses.
 _MK_VALUES = {":v": {"S": OWN}, ":g": {"S": "blue"}, ":x": {"S": "x1"},
@@ -371,12 +377,31 @@ def test_scan_denied_under_for_any_value(self, table):
             c.scan(TableName=table)
         assert _denied(e)
 
-    # -- An equality on the partition key decides the key ------------------
+    # -- A second clause on the partition key binds nothing ----------------
 
     def test_unreadable_partition_equality_does_not_bind_another_clause(self, table):
-        # An equality on the partition key decides the key. When its value
-        # cannot be read (`:u = {"N": "bob"}`), a second clause naming the
-        # fenced value must not be bound in its place.
+        # An equality on the partition key and a second clause on it name the
+        # partition key twice, so the duplicate check leaves
+        # dynamodb:LeadingKeys absent and the fenced value in the second
+        # clause (`:v`) is never bound. A principal allowed everything gets
+        # the one-condition-per-key error. A policy that needs a LeadingKeys
+        # value denies the request, whether the equality's value can be read
+        # (`:u = {"S": "bob"}`) or not (`:u = {"N": "bob"}`, refused either
+        # way).
+        kces = ("pk = :u AND pk > :v", "pk > :v AND pk = :u")
+        readable = {":u": {"S": OTHER}, ":v": {"S": OWN}}
+        unreadable = {":u": {"N": OTHER}, ":v": {"S": OWN}}
+        owner = self._user_client([("all", {
+            "Version": "2012-10-17",
+            "Statement": [{"Effect": "Allow", "Action": "dynamodb:*", "Resource": "*"}],
+        })])
+        for kce in kces:
+            with pytest.raises(ClientError) as e:
+                owner.query(TableName=table, KeyConditionExpression=kce,
+                            ExpressionAttributeValues=readable)
+            err = e.value.response["Error"]
+            assert err["Code"] == "ValidationException", kce
+            assert err["Message"] == ONE_PER_KEY, kce
         for form in ("ForAnyValue", "ForAllValues+Null"):
             if form == "ForAnyValue":
                 policy = self._leading_keys_policy("ForAnyValue", [OWN], table)
@@ -385,15 +410,16 @@ def test_unreadable_partition_equality_does_not_bind_another_clause(self, table)
                 stmt = policy["Statement"][0]
                 stmt["Condition"]["Null"] = {"dynamodb:LeadingKeys": "false"}
             c = self._user_client([("lk", policy)])
-            for kce in ("pk = :u AND pk > :v", "pk > :v AND pk = :u"):
-                try:
-                    got = c.query(
-                        TableName=table, KeyConditionExpression=kce,
-                        ExpressionAttributeValues={":u": {"N": OTHER}, ":v": {"S": OWN}})
-                except ClientError:
-                    continue
-                parts = {it["pk"]["S"] for it in got["Items"]}
-                assert OTHER not in parts, (form, kce, parts)
+            for kce in kces:
+                with pytest.raises(ClientError) as e:
+                    c.query(TableName=table, KeyConditionExpression=kce,
+                            ExpressionAttributeValues=readable)
+                assert _denied(e), (form, kce)
+                with pytest.raises(ClientError) as e:
+                    c.query(TableName=table, KeyConditionExpression=kce,
+                            ExpressionAttributeValues=unreadable)
+                assert e.value.response["Error"]["Code"] in (
+                    "AccessDeniedException", "ValidationException"), (form, kce)
 
     def test_non_equality_partition_with_sort_clause_is_refused(self, table):
         # The fix binds the partition operand of these shapes; the engine still