From 7d3b03865d4d87e28253337e0c9cc5d9f97b2bc1 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Fri, 9 Oct 2026 16:38:10 -0400 Subject: [PATCH] Report failed Security reproductions after completed audits (#603) --- src/notification.rs | 19 ++++ src/run_ending.rs | 19 +++- src/security.rs | 13 ++- tests/security_run.rs | 242 +++++++++++++++++++++++++++++------------- 4 files changed, 218 insertions(+), 75 deletions(-) diff --git a/src/notification.rs b/src/notification.rs index 385979c..9f05d99 100644 --- a/src/notification.rs +++ b/src/notification.rs @@ -212,10 +212,27 @@ fn body( let mut text = format!("Result: {}\n", account.outcome); if let Some(findings) = account.security_findings { let audit = match &account.ended { + Err(_) if account.failed_reproduction.is_some() => "audit complete", Ok(line) => line.as_str(), Err(_) => account.outcome, }; text += &format!("Audit: {audit}\n"); + if let Some(number) = account.failed_reproduction { + let status = if account.interrupted { + "interrupted" + } else if account + .ended + .as_ref() + .err() + .and_then(|cause| cause.safeguard_refusal()) + .is_some() + { + "refused" + } else { + "failed" + }; + text += &format!("Reproduction: {number} {status}\n"); + } if !findings.is_empty() { text += "\nSecurity findings:\n"; for finding in findings { @@ -331,6 +348,7 @@ mod tests { ticket_lines: &[], review: None, security_findings: None, + failed_reproduction: None, urls: vec![PR], } } @@ -349,6 +367,7 @@ mod tests { ticket_lines: &[], review: None, security_findings: None, + failed_reproduction: None, urls: Vec::new(), } } diff --git a/src/run_ending.rs b/src/run_ending.rs index 6222f36..62edeab 100644 --- a/src/run_ending.rs +++ b/src/run_ending.rs @@ -53,11 +53,20 @@ pub fn read(ending: &Ending) -> Result, &Skip> { ticket_lines: &[], review: None, security_findings: None, + failed_reproduction: None, urls: Vec::new(), }, - Err(failed) => Account::of_failure(failed, "audit failed"), + Err(failed) => Account::of_failure( + failed, + if audited.failed_reproduction.is_some() { + "reproduction failed" + } else { + "audit failed" + }, + ), }; Ok(Account { + failed_reproduction: audited.failed_reproduction, security_findings: Some(&audited.findings), advice: &audited.advice, ..account @@ -102,6 +111,8 @@ pub struct Account<'a> { pub ticket_lines: &'a [String], /// In an Architect run, how its Architecture review ended. pub review: Option>, + /// The failed reproduction, after a completed Security audit and private recording. + pub failed_reproduction: Option, /// Safe metadata from a Security run's private records, even if the audit failed. pub security_findings: Option<&'a [crate::security::RecordedFinding]>, /// The URLs on stdout, a line each: the pull request's, or that of the @@ -172,6 +183,7 @@ impl<'a> Account<'a> { ticket_lines: &reached.ticket_lines, review: None, security_findings: None, + failed_reproduction: None, urls: vec![&reached.pr_url], }, Err(failed) => Account { @@ -196,6 +208,7 @@ impl<'a> Account<'a> { ticket_lines: &failed.ticket_lines, review: None, security_findings: None, + failed_reproduction: None, urls: failed.pr_url.as_deref().into_iter().collect(), } } @@ -224,6 +237,7 @@ impl<'a> Account<'a> { ticket_lines: &[], review: None, security_findings: None, + failed_reproduction: None, urls: vec![reviewed.url()], }, (Err(failed), None) => Account::of_failure(failed, "review failed"), @@ -476,6 +490,7 @@ mod tests { ticket_lines: &[], review: None, security_findings: None, + failed_reproduction: None, urls: vec![PR], } } @@ -494,6 +509,7 @@ mod tests { ticket_lines: &[], review: None, security_findings: None, + failed_reproduction: None, urls: Vec::new(), } } @@ -670,6 +686,7 @@ mod tests { dispatched: None, }), security_findings: None, + failed_reproduction: None, urls: vec![PLAN], } ); diff --git a/src/security.rs b/src/security.rs index 73673f2..24f2440 100644 --- a/src/security.rs +++ b/src/security.rs @@ -44,9 +44,11 @@ pub enum Outcome { }, } -/// The audit's outcome and the private records it reached, including before a failure. +/// The Security run's outcome and the private records it reached, including before a failure. pub struct Ended { pub outcome: Result, + /// A failed reproduction's number; its audit and private recording completed. + pub failed_reproduction: Option, pub findings: Vec, pub advice: Vec, } @@ -55,6 +57,7 @@ impl From for Ended { fn from(error: anyhow::Error) -> Self { Self { outcome: Err(error.into()), + failed_reproduction: None, findings: Vec::new(), advice: Vec::new(), } @@ -329,6 +332,7 @@ fn audit_and_record( let mut advice = Vec::new(); let mut findings = Vec::new(); let mut log = None; + let mut reproduction = None; let recorded = (|| -> Result { outside.check_harness()?; outside.check_node()?; @@ -376,6 +380,7 @@ fn audit_and_record( "starting Security reproduction {number} of {}", record.name() )); + reproduction = Some(number); let (reproduced, session_log) = outside.reproduce(record, number); if session_log.is_some() { log = session_log; @@ -409,11 +414,17 @@ fn audit_and_record( } Ok(recorded) })(); + let failed_reproduction = if recorded.is_err() { + reproduction + } else { + None + }; Ended { outcome: recorded.map_err(|error| FailedRun { log, ..error.into() }), + failed_reproduction, findings, advice, } diff --git a/tests/security_run.rs b/tests/security_run.rs index 4473b7a..3fb07ef 100644 --- a/tests/security_run.rs +++ b/tests/security_run.rs @@ -99,6 +99,13 @@ fn safeguard_refusals_notify_the_cause_without_private_diagnostics() { subject, "[thirdshift] acme/widgets Security run: audit failed" ); + assert!(text.contains("Audit: audit failed"), "{text}"); + assert!(!text.contains("audit complete"), "{text}"); + assert!(!text.contains("Reproduction:"), "{text}"); + assert_eq!( + scenario.claude_calls().len() + scenario.codex_calls().len(), + 1 + ); assert!(text.contains(&format!("Cause: {cause}")), "{text}"); assert!(!text.contains("Private refusal evidence."), "{text}"); } @@ -197,38 +204,85 @@ fn codex_logs_the_requested_model_for_each_security_session_and_resume() { #[test] fn refused_security_reproductions_keep_the_record_and_stop_before_the_next_finding() { for (harness, script, cause) in safeguard_refusals() { - let scenario = Scenario::new(); - scenario.agent_does_in_session( - 1, - &audit_script(&json!([finding("first"), finding("second")]).to_string()), - ); - scenario.agent_does_in_session(2, script); - scenario.agent_does_in_session(3, "exit 99\n"); - let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); - let result = scenario.run_with_env( - &["secure", "harness", harness, "email", "me@example.com"], - &resend_env(&resend), - ); - assert_eq!(result.code, Some(1), "{}", result.stderr); - assert!(result.stderr.contains(cause), "{}", result.stderr); - let (_, text) = the_one_notification(&resend); - assert!(text.contains(cause), "{text}"); - assert!(!text.contains("Private refusal evidence."), "{text}"); - let state = scenario.gh_state(); - for record in state["advisories"].as_array().unwrap() { - assert!(record["severity"].is_null()); - assert!( - !record["description"] - .as_str() + for private in [false, true] { + let scenario = Scenario::new(); + let mut github = scenario.gh_state(); + github["private"] = json!(private); + scenario.write_gh_state(&github); + scenario.agent_does_in_session( + 1, + &audit_script(&json!([finding("first"), finding("second")]).to_string()), + ); + scenario.agent_does_in_session(2, script); + scenario.agent_does_in_session(3, "exit 99\n"); + let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); + let result = scenario.run_with_env( + &[ + "secure", + "security-fix", + "harness", + harness, + "email", + "me@example.com", + ], + &resend_env(&resend), + ); + assert_eq!(result.code, Some(1), "{}", result.stderr); + assert!(result.stderr.contains(cause), "{}", result.stderr); + let (subject, text) = the_one_notification(&resend); + assert!(subject.ends_with(": reproduction failed"), "{subject}"); + for line in [ + "Result: reproduction failed", + "Audit: audit complete", + "Reproduction: 1 refused", + "Session log:", + "security-reproduction-1.jsonl", + "Command log:", + ] { + assert!(text.contains(line), "missing {line}: {text}"); + } + assert!(text.contains(cause), "{text}"); + assert!(!text.contains("Private refusal evidence."), "{text}"); + let state = scenario.gh_state(); + let records = if private { + state["bodies"] + .as_object() .unwrap() - .contains("## Reproduction") + .values() + .cloned() + .collect::>() + } else { + let advisories = state["advisories"].as_array().unwrap(); + assert_eq!(advisories.len(), 2); + assert!(advisories.iter().all(|record| record["severity"].is_null())); + advisories + .iter() + .map(|record| record["description"].clone()) + .collect() + }; + assert_eq!(records.len(), 2); + for record in records { + assert!(!record.as_str().unwrap().contains("## Reproduction")); + } + assert_eq!( + scenario.claude_calls().len() + scenario.codex_calls().len(), + 2 ); + assert_eq!(scenario.entries("work"), vec![REPO]); + assert!(state["prs"].as_array().unwrap().is_empty()); + for private_detail in [ + "Private candidate write-up.", + "Private trace.", + "Private evidence.", + ] { + assert!( + !resend.requests()[0] + .body + .to_string() + .contains(private_detail) + ); + } } - assert_eq!( - scenario.claude_calls().len() + scenario.codex_calls().len(), - 2 - ); - assert_eq!(scenario.entries("work"), vec![REPO]); } } @@ -2625,6 +2679,8 @@ fn failed_security_audits_send_one_notification_with_a_safe_status() { "[thirdshift] acme/widgets Security run: audit failed" ); assert!(text.contains("Audit: audit failed"), "{text}"); + assert!(!text.contains("audit complete"), "{text}"); + assert!(!text.contains("Reproduction:"), "{text}"); assert!( result.stderr.contains(cause), "expected {cause}: {}", @@ -2786,53 +2842,93 @@ fn security_notification_omits_private_background_trace() { #[test] fn a_reproduction_failure_notifies_recorded_metadata_without_private_evidence() { for private in [false, true] { - let scenario = Scenario::new(); - let mut github = scenario.gh_state(); - github["private"] = json!(private); - scenario.write_gh_state(&github); - let mut first = finding("first"); - first["title"] = json!("First recorded finding"); - let mut second = finding("second"); - second["title"] = json!("Second recorded finding"); - scenario.agent_does_in_session(1, &audit_script(&json!([first, second]).to_string())); - scenario.agent_does_in_session(2, &reproduction_script("reproduced high single")); - scenario.agent_does_in_session(3, &reproduction_script("incomplete")); - let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); - let result = - scenario.run_with_env(&["secure", "email", "me@example.com"], &resend_env(&resend)); - assert_eq!(result.code, Some(1), "{}", result.stderr); - assert!( - result - .stderr - .contains("Security reproduction ended without the final line"), - "{}", - result.stderr - ); - let (subject, text) = the_one_notification(&resend); - assert!(subject.ends_with(": audit failed"), "{subject}"); - assert!(text.contains("high: First recorded finding"), "{text}"); - assert!(text.contains("Second recorded finding"), "{text}"); - assert!(!text.contains("high: Second recorded finding"), "{text}"); - let url = if private { - "https://github.com/acme/widgets/issues/" - } else { - "https://github.com/acme/widgets/security/advisories/" - }; - assert_eq!(text.matches(url).count(), 2, "{text}"); - for evidence in [ - "Private candidate write-up.", - "Private trace.", - "Private evidence.", - "Local command: bounded-fixture", - "bounded_fixture();", - "Cause:", + for failed_script in [ + reproduction_script("incomplete"), + "printf '%s\\n' 'Private provider diagnostic.' >&2\nexit 3\n".to_string(), ] { - assert!( - !resend.requests()[0].body.to_string().contains(evidence), - "leaked {evidence}: {text}" + let scenario = Scenario::new(); + let mut github = scenario.gh_state(); + github["private"] = json!(private); + scenario.write_gh_state(&github); + let mut first = finding("first"); + first["title"] = json!("First recorded finding"); + let mut second = finding("second"); + second["title"] = json!("Second recorded finding"); + scenario.agent_does_in_session( + 1, + &audit_script(&json!([first, second, finding("third")]).to_string()), ); + scenario.agent_does_in_session(2, &reproduction_script("reproduced high single")); + scenario.agent_does_in_session(3, &failed_script); + let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); + let result = scenario.run_with_env( + &["secure", "security-fix", "email", "me@example.com"], + &resend_env(&resend), + ); + assert_eq!(result.code, Some(1), "{}", result.stderr); + if failed_script.contains("incomplete") { + assert!( + result + .stderr + .contains("Security reproduction ended without the final line"), + "{}", + result.stderr + ); + } + let (subject, text) = the_one_notification(&resend); + assert!(subject.ends_with(": reproduction failed"), "{subject}"); + for line in [ + "Result: reproduction failed", + "Audit: audit complete", + "Reproduction: 2 failed", + "Session log:", + "security-reproduction-2.jsonl", + "Command log:", + ] { + assert!(text.contains(line), "missing {line}: {text}"); + } + assert!(text.contains("high: First recorded finding"), "{text}"); + assert!(text.contains("Second recorded finding"), "{text}"); + assert!(!text.contains("high: Second recorded finding"), "{text}"); + let url = if private { + "https://github.com/acme/widgets/issues/" + } else { + "https://github.com/acme/widgets/security/advisories/" + }; + assert_eq!(text.matches(url).count(), 3, "{text}"); + for evidence in [ + "Private candidate write-up.", + "Private trace.", + "Private evidence.", + "Local command: bounded-fixture", + "bounded_fixture();", + "Cause:", + "Private provider diagnostic.", + ] { + assert!( + !resend.requests()[0].body.to_string().contains(evidence), + "leaked {evidence}: {text}" + ); + } + assert_eq!(scenario.claude_calls().len(), 3); + let state = scenario.gh_state(); + assert!(state["prs"].as_array().unwrap().is_empty()); + if private { + assert_eq!(state["bodies"].as_object().unwrap().len(), 3); + assert!( + state["bodies"]["8"] + .as_str() + .unwrap() + .contains("Severity: high") + ); + } else { + let records = state["advisories"].as_array().unwrap(); + assert_eq!(records.len(), 3); + assert_eq!(records[0]["severity"], "high"); + assert!(records[1]["severity"].is_null()); + assert!(records[2]["severity"].is_null()); + } } - assert_eq!(scenario.claude_calls().len(), 3); } }