From cf12c86dc569e383c419e821046a8e59b4c8614a Mon Sep 17 00:00:00 2001 From: jif Date: Tue, 29 Sep 2026 15:52:28 +0000 Subject: [PATCH] Simplify configuration fingerprint canonicalization (#49295) ## What changed Replace the custom recursive JSON canonicalizer with `serde_json::Value::sort_all_objects()` before hashing configuration values, preserving existing fingerprints. ## Testing Add a regression test that checks reordered keys in nested tables and tables inside arrays produce the same existing fingerprint. GitOrigin-RevId: 890e79865a1d690adea3b35f19fcc6c30dbd17ce --- codex-rs/config/src/fingerprint.rs | 26 ++++++------------------ codex-rs/config/src/fingerprint_tests.rs | 26 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 20 deletions(-) create mode 100644 codex-rs/config/src/fingerprint_tests.rs diff --git a/codex-rs/config/src/fingerprint.rs b/codex-rs/config/src/fingerprint.rs index b4122b1da5..d6d32636ea 100644 --- a/codex-rs/config/src/fingerprint.rs +++ b/codex-rs/config/src/fingerprint.rs @@ -52,9 +52,9 @@ pub(super) fn record_origins( } pub fn version_for_toml(value: &TomlValue) -> String { - let json = serde_json::to_value(value).unwrap_or(JsonValue::Null); - let canonical = canonical_json(&json); - let serialized = serde_json::to_vec(&canonical).unwrap_or_default(); + let mut json = serde_json::to_value(value).unwrap_or(JsonValue::Null); + json.sort_all_objects(); + let serialized = serde_json::to_vec(&json).unwrap_or_default(); let mut hasher = Sha256::new(); hasher.update(serialized); let hash = hasher.finalize(); @@ -65,20 +65,6 @@ pub fn version_for_toml(value: &TomlValue) -> String { format!("sha256:{hex}") } -fn canonical_json(value: &JsonValue) -> JsonValue { - match value { - JsonValue::Object(map) => { - let mut sorted = serde_json::Map::new(); - let mut keys = map.keys().cloned().collect::>(); - keys.sort(); - for key in keys { - if let Some(val) = map.get(&key) { - sorted.insert(key, canonical_json(val)); - } - } - JsonValue::Object(sorted) - } - JsonValue::Array(items) => JsonValue::Array(items.iter().map(canonical_json).collect()), - other => other.clone(), - } -} +#[cfg(test)] +#[path = "fingerprint_tests.rs"] +mod tests; diff --git a/codex-rs/config/src/fingerprint_tests.rs b/codex-rs/config/src/fingerprint_tests.rs new file mode 100644 index 0000000000..967531283d --- /dev/null +++ b/codex-rs/config/src/fingerprint_tests.rs @@ -0,0 +1,26 @@ +//! Protects fingerprint bytes when nested configuration keys are reordered. + +use super::version_for_toml; +use pretty_assertions::assert_eq; + +#[test] +fn fingerprint_preserves_nested_key_order_independence_and_existing_hash() { + for input in [ + r#"rows = [{ z = 2, a = 1 }, { z = 4, a = 3 }] +[nested] +z = "text" +a = true +"#, + r#"rows = [{ a = 1, z = 2 }, { a = 3, z = 4 }] +[nested] +a = true +z = "text" +"#, + ] { + let value = toml::from_str(input).expect("valid config"); + assert_eq!( + version_for_toml(&value), + "sha256:a5e4c8fbdbc75d185b0f2b5291bfb2de10e8c523d6c89248876fd78a97c83176" + ); + } +}