1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
//! Unrecognised-key reporting for the HOST-level config files (#5207).
//!
//! Why: the owner ruling's complaint is that a misspelled config key currently
//! no-ops in silence. The obvious fix — `#[serde(deny_unknown_fields)]` on
//! [`crate::core::config::MpmConfig`] and
//! [`crate::core::trusty_tools_config::TrustyToolsConfig`] — is a REGRESSION on
//! those two, because both loaders answer a parse failure by returning
//! `Default`. Denying unknown fields there would upgrade "one key is ignored"
//! into "the entire file is ignored": a single typo would silently drop every
//! model override, every section, the whole config, behind one `warn` line. The
//! failure it introduces is strictly worse than the one it fixes.
//!
//! So strictness is applied where it is safe and total — the NEW committed
//! surface, [`crate::core::project_config::ProjectLevelConfig`], which has no
//! legacy corpus and is reviewed in a PR — and the host files get this instead:
//! the lenient parse they already had, plus a loud, precise report of every key
//! that parse threw away. The typo stops being silent; the other keys keep
//! working.
//!
//! What: [`unknown_key_paths`] diffs the raw document against a round-trip of
//! the parsed struct. Anything present in the input but absent from the
//! round-trip was dropped by serde, i.e. the schema does not define it. This
//! needs no hand-maintained key list, so it cannot drift as sections are added,
//! and it descends into nested tables. [`report_unknown_keys`] logs the result.
//! Test: `unknown_top_level_key_is_reported`, `unknown_nested_key_is_reported`,
//! `known_keys_are_not_reported`, `map_valued_sections_are_not_reported`,
//! `explicit_null_is_not_reported` in `config_keys_tests.rs`.
//!
//! [`unknown_key_paths`]: crate::core::config_keys::unknown_key_paths
//! [`report_unknown_keys`]: crate::core::config_keys::report_unknown_keys
use serde::Serialize;
use serde_json::Value;
/// Convert a TOML document to the common `serde_json::Value` tree.
///
/// Why: the two host config files are TOML and YAML; normalising both into one
/// tree type lets a single diff serve them. Returns `None` for input that does
/// not parse — the caller has already reported that as a malformed file.
/// Test: `unknown_top_level_key_is_reported`.
pub fn toml_document(raw: &str) -> Option<Value> {
toml::from_str::<Value>(raw).ok()
}
/// Convert a YAML document to the common `serde_json::Value` tree.
///
/// Why: see [`toml_document`]. A YAML file whose root is not a mapping (empty
/// file, bare scalar) yields `None`, since there are no keys to check.
/// Test: `explicit_null_is_not_reported`.
pub fn yaml_document(raw: &str) -> Option<Value> {
serde_yaml::from_str::<Value>(raw)
.ok()
.filter(Value::is_object)
}
/// Dotted paths of every key in `raw` that `parsed`'s schema does not define.
///
/// Why: this is the whole detection mechanism, and it is deliberately derived
/// rather than declared — round-tripping the PARSED value re-emits exactly the
/// keys serde understood, so the difference is exactly what serde discarded. A
/// hand-written list of valid keys would have to be updated every time a section
/// is added, and would silently stop catching typos the day someone forgot.
/// What: returns dotted paths (`"models.tiers.hiaku"`) in document order.
/// Descends into any table present on both sides. Two deliberate exclusions:
/// a key whose input value is `null` is skipped, because a field with
/// `skip_serializing_if = "Option::is_none"` legitimately round-trips to
/// nothing; and a table that round-trips as a table is compared key-by-key,
/// which means a `HashMap`-valued section (`[models.agents]`, whose keys are
/// arbitrary agent names) reports nothing, since every key survives.
/// Test: `unknown_top_level_key_is_reported`, `unknown_nested_key_is_reported`,
/// `known_keys_are_not_reported`, `map_valued_sections_are_not_reported`.
pub fn unknown_key_paths<T: Serialize>(raw: &Value, parsed: &T) -> Vec<String> {
let Ok(known) = serde_json::to_value(parsed) else {
return Vec::new();
};
let mut out = Vec::new();
diff_into(raw, &known, "", &mut out);
out
}
/// Recursive worker for [`unknown_key_paths`].
///
/// What: walks `raw`'s object keys; a key missing from `known` is recorded, a
/// key present on both recurses. Non-object nodes terminate the walk.
/// Test: via [`unknown_key_paths`]'s tests.
fn diff_into(raw: &Value, known: &Value, prefix: &str, out: &mut Vec<String>) {
let (Value::Object(raw_map), Value::Object(known_map)) = (raw, known) else {
return;
};
for (key, raw_value) in raw_map {
let path = if prefix.is_empty() {
key.clone()
} else {
format!("{prefix}.{key}")
};
match known_map.get(key) {
Some(known_value) => diff_into(raw_value, known_value, &path, out),
// An explicit `null` legitimately round-trips to nothing via
// `skip_serializing_if = "Option::is_none"`, so it is not a typo.
None if raw_value.is_null() => {}
None => out.push(path),
}
}
}
/// Log every unrecognised key in a host config file.
///
/// Why: the whole point of the module — an ignored key must produce a signal an
/// operator can act on, naming the file and the exact path so the fix is
/// mechanical. `warn` rather than `error`: the file still applied, and every key
/// the schema does define took effect.
/// What: one warning listing all unrecognised paths; silent when there are none
/// (the overwhelmingly common case).
/// Test: `report_is_silent_for_a_clean_document`.
pub fn report_unknown_keys<T: Serialize>(file: &str, raw: &Value, parsed: &T) {
let unknown = unknown_key_paths(raw, parsed);
if unknown.is_empty() {
return;
}
tracing::warn!(
"{file}: ignoring {} unrecognised key(s): {}. \
These had no effect — check for a typo against the documented schema.",
unknown.len(),
unknown.join(", ")
);
}
#[cfg(test)]
#[path = "config_keys_tests.rs"]
mod tests;