From 02aa1a3060d66728bdd703ec3e2f4106d52bfc9c Mon Sep 17 00:00:00 2001 From: Amin Chirazi Date: Sat, 1 Aug 2026 13:35:28 +0000 Subject: [PATCH] fix(cli): the report predicted a tier the run had already decided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #341 made the run's containment tier win over the probe's, and `check_egress` has honoured that since. The REPORT did not. It called `containment(spec)` before the run started and printed that, then reused it for the `--json` payload. On Linux the two agree by construction, so this was invisible. On Windows the run is the authority — seven steps can fail after the probe passes — so a run that WAS contained printed "not contained" beside a trace lane saying the opposite. That is worse than either being wrong on its own. An auditor holding two artifacts that describe one run differently has no way to tell which is lying, and the artifact that is right is the one nobody printed. So `record` and `replay` return the tier they achieved alongside their outcome, and the report prefers it. The tier leaves by out-parameter because it becomes known part way through: every `?` before that point would otherwise have to name a tier it does not have yet. A run that failed before it started reports the prediction, which is the honest answer for a run that achieved nothing. `achieved_tier` is one function used by both paths. The certification path and the reporting path computing this separately is how they came to disagree in the first place. The corrected line prints only when the run's answer DIFFERS from the prediction, so nothing changes on Linux and no output grows a line for a distinction that platform does not have. Not fixed here, and it is the same defect: the run record at `build_control_record` still stores the predicted tier. It is built where the run is not in scope, which is a larger change than this one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XdXrbksFKirm7yW6EDunur --- crates/flowproof-cli/src/agent_flow.rs | 81 ++++++++++++++++++++++++-- crates/flowproof-cli/src/lib.rs | 26 +++++++-- 2 files changed, 96 insertions(+), 11 deletions(-) diff --git a/crates/flowproof-cli/src/agent_flow.rs b/crates/flowproof-cli/src/agent_flow.rs index 68fabea..1b4573c 100644 --- a/crates/flowproof-cli/src/agent_flow.rs +++ b/crates/flowproof-cli/src/agent_flow.rs @@ -832,6 +832,18 @@ pub fn containment(spec: &FlowSpec) -> Containment { /// egress lane to store (record) or discard (replay). Fails - so record mints /// no trace and replay fails the flow - when `assert_no_egress` cannot be /// certified or was violated. +/// The tier to REPORT: what the run ACHIEVED where it decided one, and the +/// pre-run prediction only where it did not. +/// +/// One definition, used by the certification path and the reporting path +/// alike. Them disagreeing is the failure mode worth designing out: a report +/// that says "not contained" over a run `assert_no_egress` certified would +/// leave an auditor with two artifacts describing the same run differently, +/// and no way to tell which one is lying. +pub fn achieved_tier(run: &AgentRun, spec_tier: &Containment) -> Containment { + run.containment.clone().unwrap_or_else(|| spec_tier.clone()) +} + fn check_egress( plan: &Plan, run: &AgentRun, @@ -843,7 +855,7 @@ fn check_egress( // fail to become contained after the probe said yes, and certifying on // the probe's optimism would be the false green of #300 and #301 arriving // by prediction rather than by silence. - let containment = run.containment.as_ref().unwrap_or(spec_tier); + let containment = &achieved_tier(run, spec_tier); // `assert_no_egress` is a CAPABILITY claim: it can only certify where // containment is actually enforced. There is no bypass flag. if plan.assert_no_egress && !containment.is_enforced() { @@ -1297,7 +1309,22 @@ fn warn_unprotected_tools(spec: &FlowSpec, plan: &Plan, phase: &str) { } } -pub fn record(spec: &FlowSpec, out: &Path) -> Result<(), String> { +pub fn record(spec: &FlowSpec, out: &Path) -> (Containment, Result<(), String>) { + let mut achieved = None; + let outcome = record_inner(spec, out, &mut achieved); + (achieved.unwrap_or_else(|| containment(spec)), outcome) +} + +/// The body. Takes `achieved` as an out-parameter rather than returning the +/// tier, because the tier becomes known PART WAY THROUGH and every `?` before +/// that point would otherwise have to name a tier it does not have yet. A run +/// that failed before it started reports the prediction, which is the honest +/// answer for a run that achieved nothing. +fn record_inner( + spec: &FlowSpec, + out: &Path, + achieved: &mut Option, +) -> Result<(), String> { let mut plan = plan(spec)?; warn_unprotected_tools(spec, &plan, "record"); // Set up the MCP boundary BEFORE the agent starts: write the plans and @@ -1339,6 +1366,7 @@ pub fn record(spec: &FlowSpec, out: &Path) -> Result<(), String> { if let Some(warning) = egress_warning(&plan, &tier) { eprintln!("{warning}"); } + *achieved = Some(achieved_tier(&run, &tier)); let egress = check_egress(&plan, &run, &tier)?; // The secret-leak scan runs BEFORE the trace is minted: a leak fails the // run so NO trace is written. That doubles as a store-guard - a secret @@ -1360,7 +1388,18 @@ pub fn record(spec: &FlowSpec, out: &Path) -> Result<(), String> { /// Replay an `app: agent` flow: serve the recorded cassette, run the /// agent, and check that the trajectory reproduced and the assertions /// still hold. -pub fn replay(spec: &FlowSpec, trace_path: &Path) -> Result<(), String> { +pub fn replay(spec: &FlowSpec, trace_path: &Path) -> (Containment, Result<(), String>) { + let mut achieved = None; + let outcome = replay_inner(spec, trace_path, &mut achieved); + (achieved.unwrap_or_else(|| containment(spec)), outcome) +} + +/// The body; see [`record_inner`] for why the tier leaves by out-parameter. +fn replay_inner( + spec: &FlowSpec, + trace_path: &Path, + achieved: &mut Option, +) -> Result<(), String> { let mut plan = plan(spec)?; warn_unprotected_tools(spec, &plan, "replay"); let raw = std::fs::read_to_string(trace_path) @@ -1406,6 +1445,7 @@ pub fn replay(spec: &FlowSpec, trace_path: &Path) -> Result<(), String> { if let Some(warning) = egress_warning(&plan, &tier) { eprintln!("{warning}"); } + *achieved = Some(achieved_tier(&run, &tier)); check_egress(&plan, &run, &tier)?; // Re-scan the recorded corpus for declared secrets by the SAME mechanism // as record, so an unchanged system replays the same verdict. The corpus @@ -1800,6 +1840,33 @@ mod tests { } } + /// The REPORT follows the run, not the probe. + /// + /// `check_egress` already certified on this rule. The report line did not, + /// so a Windows run that WAS contained printed "not contained" on stdout + /// and in `--json`, beside a trace lane that said the opposite. Two + /// artifacts describing one run differently is worse than either being + /// wrong alone: an auditor has no way to tell which one is lying. + #[test] + fn the_reported_tier_is_the_one_the_run_achieved() { + let mut run = egress_run(vec![]); + run.containment = Some(Containment::Enforced); + let predicted = Containment::NotContained("this host cannot enforce".into()); + assert_eq!(achieved_tier(&run, &predicted), Containment::Enforced); + } + + /// And the other direction, which is NOT implied by the one above: where + /// the run decided no tier - a `url:` service, a flow engaging no egress, + /// a platform with no mechanism - the prediction stands. A run that + /// determined nothing must not be read as one that achieved nothing + /// either; those are different sentences. + #[test] + fn a_run_that_decided_no_tier_reports_the_prediction() { + let run = egress_run(vec![]); + let predicted = Containment::NotContained("a service flowproof did not start".into()); + assert_eq!(achieved_tier(&run, &predicted), predicted); + } + /// `assert_no_egress` is a CAPABILITY claim: on any tier that is not /// enforced it fails outright, with no bypass, rather than passing /// vacuously. @@ -2275,7 +2342,9 @@ mod tests { )) .expect("spec parses"); - replay(&spec, &trace).expect("driver-blind replay via url passes"); + replay(&spec, &trace) + .1 + .expect("driver-blind replay via url passes"); handle.join().ok(); } @@ -2291,7 +2360,9 @@ mod tests { )) .expect("spec parses"); - let why = replay(&spec, &trace).expect_err("mispointed service must fail"); + let why = replay(&spec, &trace) + .1 + .expect_err("mispointed service must fail"); handle.join().ok(); assert!(why.contains("made 0 model calls"), "{why}"); assert!( diff --git a/crates/flowproof-cli/src/lib.rs b/crates/flowproof-cli/src/lib.rs index e01bbf4..15d4036 100644 --- a/crates/flowproof-cli/src/lib.rs +++ b/crates/flowproof-cli/src/lib.rs @@ -298,11 +298,18 @@ fn cmd_record( // The containment tier prints on EVERY agent run, on every platform, // pass or fail - computed before the run so it shows even when // recording errors out. - let tier = agent_flow::containment(&spec); + let predicted = agent_flow::containment(&spec); if !json { + println!("{}", predicted.report_line()); + } + let (tier, outcome) = agent_flow::record(&spec, &out); + // Reprinted only when the RUN decided a different tier than the probe + // predicted. On Linux they agree by construction, so this is silent; + // on Windows the run is the authority and the line above was a guess. + if !json && tier != predicted { println!("{}", tier.report_line()); } - agent_flow::record(&spec, &out)?; + outcome?; if json { println!( "{}", @@ -706,10 +713,14 @@ fn run_agent_flow_in_suite( } // The containment tier prints on every agent run, pass or fail - the // single-spec path does the same, and a suite must not hide it. + let predicted = agent_flow::containment(spec); if !json { - println!("{}", agent_flow::containment(spec).report_line()); + println!("{}", predicted.report_line()); + } + let (tier, outcome) = agent_flow::replay(spec, trace_path); + if !json && tier != predicted { + println!("{}", tier.report_line()); } - let outcome = agent_flow::replay(spec, trace_path); if let Some(cmd) = &manifest.after_each { run_hook(cmd, spec_path, "after_each")?; } @@ -1284,11 +1295,14 @@ fn cmd_run( run_hook(cmd, spec_path, "before_each")?; } // The containment tier prints on EVERY agent run, pass or fail. - let tier = agent_flow::containment(&spec); + let predicted = agent_flow::containment(&spec); if !json { + println!("{}", predicted.report_line()); + } + let (tier, outcome) = agent_flow::replay(&spec, &trace_path); + if !json && tier != predicted { println!("{}", tier.report_line()); } - let outcome = agent_flow::replay(&spec, &trace_path); if let Some(cmd) = manifest.as_ref().and_then(|m| m.after_each.as_ref()) { run_hook(cmd, spec_path, "after_each")?; }