fix: validate secondary-index keys on UpdateItem

crates/core/src/validation/mod.rs

@@ -997,9 +997,28 @@ pub fn validate_index_keys(
     item: &Item,
     indexes: &[IndexKeyRef<'_>],
     attr_defs: &[AttributeDefinition],
+) -> Result<(), DynamoDbError> {
+    validate_index_keys_in_context(item, indexes, attr_defs, SecondaryIndexEmptyContext::Item)
+}
+
+/// [`validate_index_keys`] with the caller choosing the empty-value message
+/// variant.
+///
+/// DynamoDB words the rejection differently depending on how the offending value
+/// arrived. An item supplied directly reports the attribute and index by name; a
+/// value produced by an update expression reports that the update expression
+/// attempted the change and names neither. A path that evaluates an update
+/// expression and then validates the resulting item must therefore pass
+/// [`SecondaryIndexEmptyContext::UpdateExpression`], or it rejects for the right
+/// reason with the wrong message.
+pub fn validate_index_keys_in_context(
+    item: &Item,
+    indexes: &[IndexKeyRef<'_>],
+    attr_defs: &[AttributeDefinition],
+    context: SecondaryIndexEmptyContext,
 ) -> Result<(), DynamoDbError> {
     validate_index_key_types(item, indexes, attr_defs)?;
-    validate_index_key_not_empty(item, indexes, SecondaryIndexEmptyContext::Item)
+    validate_index_key_not_empty(item, indexes, context)
 }
 
 /// Indexes sorted by name, so the alphabetically-first index that uses a

crates/storage-mongodb/src/data_engine.rs

@@ -252,7 +252,12 @@ impl MongoEngine {
         // work so the caller sees a top-level ValidationException on
         // wrong-type or empty index-key attributes (D-M10, RFC-0003
         // §2.3).
-        self.validate_index_keys_for_item(key_info, &item).await?;
+        self.validate_index_keys_for_item(
+            key_info,
+            &item,
+            extenddb_core::validation::SecondaryIndexEmptyContext::Item,
+        )
+        .await?;
 
         let coll_name = data_collection_name(&key_info.table_id);
         let coll = self.data_db.collection::<Document>(&coll_name);
@@ -816,9 +821,13 @@ impl MongoEngine {
                 // the resulting item — D-M10, RFC-0003 §2.3. Same
                 // shape as put_item's up-front check, but the
                 // post-update item is what actually gets written.
-                self.validate_index_keys_for_item(key_info, &new_item)
-                    .await
-                    .map_err(TxErr::Fatal)?;
+                self.validate_index_keys_for_item(
+                    key_info,
+                    &new_item,
+                    extenddb_core::validation::SecondaryIndexEmptyContext::UpdateExpression,
+                )
+                .await
+                .map_err(TxErr::Fatal)?;
 
                 let mut new_doc = item_to_document(
                     &new_item,
@@ -1702,6 +1711,7 @@ impl MongoEngine {
         &self,
         key_info: &TableKeyInfo,
         item: &Item,
+        context: extenddb_core::validation::SecondaryIndexEmptyContext,
     ) -> Result<(), StorageError> {
         let idx_pairs = self.fetch_index_key_schemas(&key_info.table_id).await?;
         if idx_pairs.is_empty() {
@@ -1714,8 +1724,13 @@ impl MongoEngine {
                 key_schema: ks.as_slice(),
             })
             .collect();
-        extenddb_core::validation::validate_index_keys(item, &refs, &key_info.attribute_definitions)
-            .map_err(|e| StorageError::Validation(e.to_string()))
+        extenddb_core::validation::validate_index_keys_in_context(
+            item,
+            &refs,
+            &key_info.attribute_definitions,
+            context,
+        )
+        .map_err(|e| StorageError::Validation(e.to_string()))
     }
 
     async fn sync_indexes_in_session(
@@ -2542,10 +2557,11 @@ impl MongoEngine {
                             key_schema: ks.as_slice(),
                         })
                         .collect();
-                    extenddb_core::validation::validate_index_keys(
+                    extenddb_core::validation::validate_index_keys_in_context(
                         &item,
                         &idx_refs,
                         &key_info.attribute_definitions,
+                        extenddb_core::validation::SecondaryIndexEmptyContext::UpdateExpression,
                     )
                     .map_err(|e| {
                         TransactOpError::Cancel(CancellationReason::validation_error(e.to_string()))

crates/storage-postgres/src/data/transactions.rs

@@ -255,7 +255,7 @@ fn transact_op_table_id<'a>(op: &'a TransactWriteOp<'_>) -> &'a str {
 /// cancellation reasons (BLOCKER #3 fix).
 /// Build [`validation::IndexKeyRef`] views over the table's indexes for
 /// secondary-index key validation.
-fn index_key_refs(indexes: &[IndexMeta]) -> Vec<validation::IndexKeyRef<'_>> {
+pub(crate) fn index_key_refs(indexes: &[IndexMeta]) -> Vec<validation::IndexKeyRef<'_>> {
     indexes
         .iter()
         .map(|idx| validation::IndexKeyRef {

crates/storage-postgres/src/data/update_item.rs

@@ -158,6 +158,22 @@ impl PostgresEngine {
             validation::validate_item_size(&item, self.max_item_size_bytes)
                 .map_err(|e| StorageError::Validation(e.to_string()))?;
 
+            // Secondary-index key validation on the post-update item, matching
+            // the transactional Update path: an update expression must not set
+            // an index key to a mismatched type, nor to an empty string or
+            // binary value. Validating the evaluated image rather than the
+            // expression is what covers if_not_exists and list_append, whose
+            // result is not knowable from the expression alone.
+            let idx_refs = super::transactions::index_key_refs(&indexes);
+            validation::validate_index_key_types(&item, &idx_refs, &key_info.attribute_definitions)
+                .map_err(|e| StorageError::Validation(e.to_string()))?;
+            validation::validate_index_key_not_empty(
+                &item,
+                &idx_refs,
+                validation::SecondaryIndexEmptyContext::UpdateExpression,
+            )
+            .map_err(|e| StorageError::Validation(e.to_string()))?;
+
             let new_item = if return_new { Some(item.clone()) } else { None };
 
             let item_json =

tests/rust/src/index_key_validation.rs

@@ -161,3 +161,77 @@ async fn put_item_rejects_empty_binary_index_key_reports_binary() {
         "expected type-correct 'empty binary value' message, got: {m}"
     );
 }
+
+/// Control for the rejection above: a normal update that leaves the index key
+/// valid must still succeed.
+#[tokio::test]
+async fn update_item_accepts_setting_an_index_key_to_a_valid_value() {
+    let c = client();
+    let t = tables().await;
+    let key_value = format!("idxupdok_{}", ts());
+
+    let mut item: HashMap<String, _> = HashMap::new();
+    item.insert(HASH_KEY_S.into(), s(&key_value));
+    item.insert(GSI_HASH_KEY.into(), s("h"));
+    item.insert(GSI_RANGE_KEY.into(), s("r"));
+    c.put_item()
+        .table_name(&t.simple_key_string_gsi)
+        .set_item(Some(item))
+        .send()
+        .await
+        .expect("seed item must be accepted");
+
+    c.update_item()
+        .table_name(&t.simple_key_string_gsi)
+        .key(HASH_KEY_S, s(&key_value))
+        .update_expression("SET #g = :v")
+        .expression_attribute_names("#g", GSI_HASH_KEY)
+        .expression_attribute_values(":v", s("h2"))
+        .send()
+        .await
+        .expect("a valid index-key update must be accepted");
+}
+
+/// An update expression that sets a secondary-index key to an empty value must
+/// be rejected, the same as writing that value with PutItem. Before this was
+/// enforced, UpdateItem accepted the write and stored an unindexable key.
+#[tokio::test]
+async fn update_item_rejects_setting_an_index_key_to_an_empty_value() {
+    let c = client();
+    let t = tables().await;
+    let key_value = format!("idxupde_{}", ts());
+
+    // Seed a row with a valid index key so the update targets an existing item.
+    let mut item: HashMap<String, _> = HashMap::new();
+    item.insert(HASH_KEY_S.into(), s(&key_value));
+    item.insert(GSI_HASH_KEY.into(), s("h"));
+    item.insert(GSI_RANGE_KEY.into(), s("r"));
+    c.put_item()
+        .table_name(&t.simple_key_string_gsi)
+        .set_item(Some(item))
+        .send()
+        .await
+        .expect("seed item must be accepted");
+
+    let err = c
+        .update_item()
+        .table_name(&t.simple_key_string_gsi)
+        .key(HASH_KEY_S, s(&key_value))
+        .update_expression("SET #g = :empty")
+        .expression_attribute_names("#g", GSI_HASH_KEY)
+        .expression_attribute_values(":empty", s(""))
+        .send()
+        .await
+        .expect_err("setting an index key to an empty value must be rejected");
+    assert_eq!(
+        err_code(&err),
+        Some("ValidationException"),
+        "{}",
+        err_msg(&err)
+    );
+    let m = err_msg(&err);
+    assert!(
+        m.contains("update expression attempted to update a secondary index key"),
+        "expected the update-expression index-key message, got: {m}"
+    );
+}