From 88810417406ff097650f08f51a3bbc3a6537b868 Mon Sep 17 00:00:00 2001 From: Priec Date: Sat, 8 Aug 2026 18:01:57 +0200 Subject: [PATCH] alias is surface, real column name is internal --- search/src/lib.rs | 128 ++++++++++++++++++++++++++++++++++------------ server | 2 +- 2 files changed, 95 insertions(+), 35 deletions(-) diff --git a/search/src/lib.rs b/search/src/lib.rs index 294f8949..ec62032b 100644 --- a/search/src/lib.rs +++ b/search/src/lib.rs @@ -438,11 +438,13 @@ async fn resolve_constraint_targets( .map_err(|e| Status::internal(format!("Column mapping lookup failed: {}", e)))?; if rows.is_empty() { - let target = SearchConstraintTarget { - table_name: table_filter.map(str::to_string), - column: column.to_string(), - }; - return Ok(vec![target]); + return Err(Status::invalid_argument(format!( + "Column alias '{}' was not found{}", + column, + table_filter + .map(|table_name| format!(" in table '{}'", table_name)) + .unwrap_or_default() + ))); } let mut seen = HashSet::new(); @@ -527,20 +529,35 @@ async fn table_row_display_columns( fn remap_json_to_display_names( value: serde_json::Value, physical_to_display: &HashMap, -) -> serde_json::Value { +) -> Result { match value { serde_json::Value::Object(object) => { let mut remapped = serde_json::Map::with_capacity(object.len()); for (key, value) in object { - let final_key = physical_to_display.get(&key).cloned().unwrap_or(key); + let final_key = match physical_to_display.get(&key) { + Some(display_name) => display_name.clone(), + None if is_public_system_column(&key) => key, + None => { + return Err(Status::failed_precondition( + "A table column has no public alias mapping", + )); + } + }; remapped.insert(final_key, value); } - serde_json::Value::Object(remapped) + Ok(serde_json::Value::Object(remapped)) } - other => other, + other => Ok(other), } } +fn is_public_system_column(name: &str) -> bool { + matches!( + name, + "id" | "deleted" | "created_at" | "row_revision" | "account_id" + ) +} + /// One value per display column, positionally aligned with them, so a column /// that is NULL for this row stays visible as an empty slot. fn row_display_values(value: &serde_json::Value, columns: &[String]) -> Vec { @@ -593,12 +610,17 @@ async fn resolve_order_column( table_physical_to_display_map(pool, profile_name, table_name).await?; let physical_column = physical_to_display .iter() - .find(|(physical, display)| { - physical.eq_ignore_ascii_case(&requested_column) - || display.eq_ignore_ascii_case(&requested_column) - }) + .find(|(_, display)| display.eq_ignore_ascii_case(&requested_column)) .map(|(physical, _)| physical.clone()) - .unwrap_or_else(|| requested_column.clone()); + .or_else(|| { + is_public_system_column(&requested_column).then(|| requested_column.clone()) + }) + .ok_or_else(|| { + Status::invalid_argument(format!( + "Column alias '{}' was not found in table '{}.{}'", + requested_column, profile_name, table_name + )) + })?; let physical_column = sqlx::query_scalar::<_, String>( r#" @@ -698,15 +720,21 @@ async fn fetch_ordered_rows( .await .map_err(|e| Status::internal(format!("DB query for default results failed: {}", e)))?; - Ok(rows + rows .into_iter() - .map(|row| { - let id: i64 = row.try_get("id").unwrap_or_default(); - let json_data: serde_json::Value = row.try_get("data").unwrap_or_default(); - let position: i64 = row.try_get("picker_position").unwrap_or_default(); - let json_data = remap_json_to_display_names(json_data, &physical_to_display); + .map(|row| -> Result { + let id: i64 = row + .try_get("id") + .map_err(|error| Status::internal(format!("Search id read failed: {}", error)))?; + let json_data: serde_json::Value = row.try_get("data").map_err(|error| { + Status::internal(format!("Search row read failed: {}", error)) + })?; + let position: i64 = row.try_get("picker_position").map_err(|error| { + Status::internal(format!("Search position read failed: {}", error)) + })?; + let json_data = remap_json_to_display_names(json_data, &physical_to_display)?; let row_display_values = row_display_values(&json_data, &display_columns); - Hit { + Ok(Hit { id, score: 0.0, content_json: json_data.to_string(), @@ -714,9 +742,9 @@ async fn fetch_ordered_rows( row_display_values, row_display_columns: display_columns.clone(), position: u64::try_from(position).ok(), - } + }) }) - .collect()) + .collect() } async fn run_search( @@ -841,7 +869,7 @@ async fn run_search( for row in rows { let id: i64 = row.try_get("id").unwrap_or_default(); let json_data: serde_json::Value = row.try_get("data").unwrap_or_default(); - let json_data = remap_json_to_display_names(json_data, &physical_to_display); + let json_data = remap_json_to_display_names(json_data, &physical_to_display)?; let display_values = row_display_values(&json_data, &display_columns); content_map.insert( (table_name.clone(), id), @@ -923,16 +951,24 @@ async fn fetch_ordered_candidate_rows( .await .map_err(|e| Status::internal(format!("Ordered search query failed: {}", e)))?; - Ok(rows + rows .into_iter() - .map(|row| { - let id: i64 = row.try_get("id").unwrap_or_default(); - let json_data: serde_json::Value = row.try_get("data").unwrap_or_default(); - let position: i64 = row.try_get("picker_position").unwrap_or_default(); - let score: f32 = row.try_get("candidate_score").unwrap_or_default(); - let json_data = remap_json_to_display_names(json_data, &physical_to_display); + .map(|row| -> Result { + let id: i64 = row + .try_get("id") + .map_err(|error| Status::internal(format!("Search id read failed: {}", error)))?; + let json_data: serde_json::Value = row.try_get("data").map_err(|error| { + Status::internal(format!("Search row read failed: {}", error)) + })?; + let position: i64 = row.try_get("picker_position").map_err(|error| { + Status::internal(format!("Search position read failed: {}", error)) + })?; + let score: f32 = row.try_get("candidate_score").map_err(|error| { + Status::internal(format!("Search score read failed: {}", error)) + })?; + let json_data = remap_json_to_display_names(json_data, &physical_to_display)?; let display_values = row_display_values(&json_data, &display_columns); - Hit { + Ok(Hit { id, score, content_json: json_data.to_string(), @@ -940,9 +976,9 @@ async fn fetch_ordered_candidate_rows( row_display_values: display_values, row_display_columns: display_columns.clone(), position: u64::try_from(position).ok(), - } + }) }) - .collect()) + .collect() } #[tonic::async_trait] @@ -960,6 +996,30 @@ mod tests { use super::*; use common::proto::komp_ac::search::{SearchOrder, SearchOrderDirection}; + #[test] + fn search_response_mapping_exposes_aliases_only() { + let mappings = HashMap::from([("1".to_string(), "customer".to_string())]); + let mapped = remap_json_to_display_names( + serde_json::json!({"1": "Acme", "id": 4}), + &mappings, + ) + .unwrap(); + + assert_eq!(mapped, serde_json::json!({"customer": "Acme", "id": 4})); + } + + #[test] + fn search_response_mapping_fails_instead_of_leaking_a_real_name() { + let error = remap_json_to_display_names( + serde_json::json!({"1": "Acme", "2": "hidden"}), + &HashMap::from([("1".to_string(), "customer".to_string())]), + ) + .unwrap_err(); + + assert_eq!(error.code(), tonic::Code::FailedPrecondition); + assert!(!error.message().contains("'2'")); + } + fn request(order: Option, table_name: Option<&str>) -> SearchRequest { SearchRequest { profile_name: "finance".to_string(), diff --git a/server b/server index 598521cc..585ae28e 160000 --- a/server +++ b/server @@ -1 +1 @@ -Subproject commit 598521ccd446987d8472b2b1b387cfa0b99b0efb +Subproject commit 585ae28e4f26eb10e5f27d3809a9a5e8e71d2ca1