fix/audit-findings #5
2 changed files with 58 additions and 1 deletions
fix(check): gate on recomputed metrics, not the stored copy
`brick check` read `metrics` from the model JSON and gated on them, so
setting `cycle_count` to 0 and `score` to 100 in the file turned a
failing `--max-cycles 0` gate into "passed" without changing the graph.
check now recomputes the metrics from the loaded nodes and edges and
gates on those; if the stored copy differs, that is itself a failure
("stored metrics do not match the graph"). Honest models recompute to
identical values, so existing gates are unchanged.
Test: diff_cli::check_recomputes_metrics_instead_of_trusting_the_file
fails before this change (the tampered model passes).
commit
4c630ffeb1
13
src/main.rs
13
src/main.rs
|
|
@ -227,8 +227,19 @@ fn run(cli: Cli) -> Result<()> {
|
||||||
bail!("--min-score must be a finite number");
|
bail!("--min-score must be a finite number");
|
||||||
}
|
}
|
||||||
let building = read_json(&input)?;
|
let building = read_json(&input)?;
|
||||||
let metrics = &building.metrics;
|
// Gate on metrics recomputed from the graph, never on the stored copy:
|
||||||
|
// editing `metrics` in the JSON must not be able to pass a gate.
|
||||||
|
let mut recomputed = building.clone();
|
||||||
|
brick::metrics::calculate(&mut recomputed);
|
||||||
let mut failures = Vec::new();
|
let mut failures = Vec::new();
|
||||||
|
if serde_json::to_value(&recomputed.metrics)?
|
||||||
|
!= serde_json::to_value(&building.metrics)?
|
||||||
|
{
|
||||||
|
failures.push(
|
||||||
|
"stored metrics do not match the graph; gating on recomputed metrics".into(),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
let metrics = &recomputed.metrics;
|
||||||
if metrics.structural_health.score < min_score {
|
if metrics.structural_health.score < min_score {
|
||||||
failures.push(format!(
|
failures.push(format!(
|
||||||
"score {:.1} is below {:.1}",
|
"score {:.1} is below {:.1}",
|
||||||
|
|
|
||||||
|
|
@ -259,3 +259,49 @@ fn relocated_artifacts_compare_after_checkouts_are_deleted() {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn check_recomputes_metrics_instead_of_trusting_the_file() {
|
||||||
|
// `check` gated on the metrics stored in the JSON, so editing `cycle_count` and
|
||||||
|
// `score` turned a failing gate into a pass without changing the graph.
|
||||||
|
let directory = tempfile::tempdir().unwrap();
|
||||||
|
let project = directory.path().join("project");
|
||||||
|
fs::create_dir(&project).unwrap();
|
||||||
|
fs::write(
|
||||||
|
project.join("lib.rs"),
|
||||||
|
"pub fn alpha() { beta(); }\nfn beta() { alpha(); }\n",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
let model = scan::scan(&project).unwrap();
|
||||||
|
assert!(model.metrics.cycle_count > 0);
|
||||||
|
let honest = directory.path().join("honest.json");
|
||||||
|
write_model(&honest, &model);
|
||||||
|
let gate = |path: &Path| {
|
||||||
|
Command::new(env!("CARGO_BIN_EXE_brick"))
|
||||||
|
.args([
|
||||||
|
"check",
|
||||||
|
path.to_str().unwrap(),
|
||||||
|
"--max-cycles",
|
||||||
|
"0",
|
||||||
|
"--min-score",
|
||||||
|
"0",
|
||||||
|
])
|
||||||
|
.output()
|
||||||
|
.unwrap()
|
||||||
|
};
|
||||||
|
assert!(!gate(&honest).status.success());
|
||||||
|
|
||||||
|
let mut value: Value = serde_json::to_value(&model).unwrap();
|
||||||
|
value["metrics"]["cycle_count"] = 0.into();
|
||||||
|
value["metrics"]["structural_health"]["score"] = 100.0.into();
|
||||||
|
let tampered = directory.path().join("tampered.json");
|
||||||
|
fs::write(&tampered, serde_json::to_vec_pretty(&value).unwrap()).unwrap();
|
||||||
|
let output = gate(&tampered);
|
||||||
|
assert!(!output.status.success(), "tampered metrics passed the gate");
|
||||||
|
let text = format!(
|
||||||
|
"{}{}",
|
||||||
|
String::from_utf8_lossy(&output.stdout),
|
||||||
|
String::from_utf8_lossy(&output.stderr)
|
||||||
|
);
|
||||||
|
assert!(text.contains("stored metrics"), "{text}");
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue