From e6337a421e3978c9eade831938dafd270d52d732 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Thu, 8 Oct 2026 06:35:19 -0400 Subject: [PATCH 1/4] Reproduce recorded Security findings in isolated sessions (#542) --- prompts/README.md | 1 + prompts/security-reproduction.md | 21 +++ site/prompts/index.html | 19 +++ src/github.rs | 2 +- src/github/advisories.rs | 115 +++++++++++-- src/pass.rs | 71 +++++++- src/prompt.rs | 15 ++ src/prompts_page.rs | 11 ++ src/security.rs | 52 +++++- src/security/reproduction.rs | 193 +++++++++++++++++++++ src/worktree.rs | 17 ++ tests/fakes/gh.rs | 24 +++ tests/security_run.rs | 278 ++++++++++++++++++++++++++++++- 13 files changed, 792 insertions(+), 27 deletions(-) create mode 100644 prompts/security-reproduction.md create mode 100644 src/security/reproduction.rs diff --git a/prompts/README.md b/prompts/README.md index 0228393..07907f3 100644 --- a/prompts/README.md +++ b/prompts/README.md @@ -26,6 +26,7 @@ With `harness codex`, each session runs `codex exec --json --dangerously-bypass- | Spec review | In a Spec run, starts the Spec review once every Ticket has landed on the Spec branch, before the Spec PR, a draft until then, is marked ready. | [spec-review.md](spec-review.md) | | Architecture review | Starts the Architecture review, the session an Architect run opens with, in a worktree at the head of the Base branch. The line naming the focus is left out when the command gives none. | [architecture-review.md](architecture-review.md) | | Security audit | Starts the report-only Security audit in a throwaway worktree at origin's Base branch head. The threat-model line is included when a conventional document exists; artifacts stay under the repository's audit root. | [security-audit.md](security-audit.md) | +| Security reproduction | After a Security audit, tries to reproduce one recorded finding in a fresh throwaway worktree at its audited commit. The test is copied into the private record only after a complete outcome. | [security-reproduction.md](security-reproduction.md) | | Conflict Repair | Starts a Repair session when merging the Base branch into the Issue branch leaves conflicts. | [conflict-repair.md](conflict-repair.md) | | Conflict Repair, on Foreign commits | In a Merge run, starts a Repair session when merging Foreign commits from the Issue branch on origin into the local one leaves conflicts. | [foreign-conflict-repair.md](foreign-conflict-repair.md) | | Review Repair | In a Merge run, starts a Repair session once Foreign commits are merged into the Issue branch, to review them from the head the Run last knew as its own before they can be merged. | [review-repair.md](review-repair.md) | diff --git a/prompts/security-reproduction.md b/prompts/security-reproduction.md new file mode 100644 index 0000000..eb65a95 --- /dev/null +++ b/prompts/security-reproduction.md @@ -0,0 +1,21 @@ + + +# Security reproduction + +After a Security audit, tries to reproduce one recorded finding in a fresh throwaway worktree at its audited commit. The test is copied into the private record only after a complete outcome. + +``` +Reproduce this recorded Security finding at audited commit ``. +Read the `thirdshift-security-audit` skill's likelihood-and-impact severity rubric. +Write a proof-of-concept test from the finding's validation plan, then run it against the untouched code in this throwaway worktree. Use harmless payloads only, never a deployed site or a real third-party service. Do not fix or change repository source. +If the test reproduces the finding, score its severity with the skill's rubric and judge whether one session can hold the fix (single) or it needs a Spec (spec). +Test file: ``. +Write the test's full text to this file, even when not reproduced. In your final message, give reproduction notes: the exact command, its result, and, when reproduced, likelihood, impact and the fix-size reasoning. +End your final message with exactly `Security reproduction: reproduced `, where severity is critical, high, medium or low and size is single or spec, or `Security reproduction: not reproduced`. +This session commits, pushes, opens and publishes nothing. + +Recorded finding: + + +You run headless: nobody is watching, and ending your turn ends the session. Run tests and other long commands in the foreground, raising the command's timeout if needed. If a command is moved to the background, wait for that task by its own task id or output file, never by process names or patterns (`pgrep`, `ps | grep`, and the like): other sessions on this machine run the same commands. Never end your turn while a background task you depend on is still running: ending the turn kills it. Before ending your turn, stop every background task you no longer need, by its task id (with the `TaskStop` tool, if you have it): a task still running when your turn ends is taken as work you were waiting on. +``` diff --git a/site/prompts/index.html b/site/prompts/index.html index b1d95e1..e094f25 100644 --- a/site/prompts/index.html +++ b/site/prompts/index.html @@ -174,6 +174,25 @@

Security audit

Write the skill's report artifacts and run-metadata.json; mark run_status complete only when all required artifacts are written. Run both skill validators before finishing. End your final message with exactly `Security audit: complete` after writing valid artifacts, or `Security audit: incomplete` when incomplete. +You run headless: nobody is watching, and ending your turn ends the session. Run tests and other long commands in the foreground, raising the command's timeout if needed. If a command is moved to the background, wait for that task by its own task id or output file, never by process names or patterns (`pgrep`, `ps | grep`, and the like): other sessions on this machine run the same commands. Never end your turn while a background task you depend on is still running: ending the turn kills it. Before ending your turn, stop every background task you no longer need, by its task id (with the `TaskStop` tool, if you have it): a task still running when your turn ends is taken as work you were waiting on. + + +
+

Security reproduction

+

After a Security audit, tries to reproduce one recorded finding in a fresh throwaway worktree at its audited commit. The test is copied into the private record only after a complete outcome.

+

Sent by a Security run, before any unit

+ 
Reproduce this recorded Security finding at audited commit `<audited commit>`.
+Read the `thirdshift-security-audit` skill's likelihood-and-impact severity rubric.
+Write a proof-of-concept test from the finding's validation plan, then run it against the untouched code in this throwaway worktree. Use harmless payloads only, never a deployed site or a real third-party service. Do not fix or change repository source.
+If the test reproduces the finding, score its severity with the skill's rubric and judge whether one session can hold the fix (single) or it needs a Spec (spec).
+Test file: `<test file>`.
+Write the test's full text to this file, even when not reproduced. In your final message, give reproduction notes: the exact command, its result, and, when reproduced, likelihood, impact and the fix-size reasoning.
+End your final message with exactly `Security reproduction: reproduced <severity> <size>`, where severity is critical, high, medium or low and size is single or spec, or `Security reproduction: not reproduced`.
+This session commits, pushes, opens and publishes nothing.
+
+Recorded finding:
+<recorded Security finding>
+
 You run headless: nobody is watching, and ending your turn ends the session. Run tests and other long commands in the foreground, raising the command's timeout if needed. If a command is moved to the background, wait for that task by its own task id or output file, never by process names or patterns (`pgrep`, `ps | grep`, and the like): other sessions on this machine run the same commands. Never end your turn while a background task you depend on is still running: ending the turn kills it. Before ending your turn, stop every background task you no longer need, by its task id (with the `TaskStop` tool, if you have it): a task still running when your turn ends is taken as work you were waiting on.
diff --git a/src/github.rs b/src/github.rs index 7b17c81..741a123 100644 --- a/src/github.rs +++ b/src/github.rs @@ -13,7 +13,7 @@ use crate::labels::{Label, Labels}; use crate::process::{self, Control, Interruption}; mod advisories; -pub use advisories::{DraftAdvisory, Package, SecurityRecords}; +pub use advisories::{DraftAdvisory, Package, SecurityRecord, SecurityRecords}; #[cfg(test)] mod execution_tests; diff --git a/src/github/advisories.rs b/src/github/advisories.rs index 009bb23..07626af 100644 --- a/src/github/advisories.rs +++ b/src/github/advisories.rs @@ -8,6 +8,7 @@ use serde_json::{Value, json}; use super::GitHub; use crate::issue::IssueUrl; use crate::labels::{Label, NEEDS_TRIAGE}; +use crate::security::reproduction::Reproduction; const SECURITY_FINDING: Label = Label::new( "security-finding", @@ -25,6 +26,68 @@ impl SecurityRecords { Self::Advisories(records) | Self::Issues(records) => records.push(record), } } + + pub fn finding(&self, draft: &DraftAdvisory) -> Result> { + let (records, field) = match self { + Self::Advisories(records) => (records, "description"), + Self::Issues(records) => (records, "body"), + }; + let marker = format!("Fingerprint: `{}`", draft.fingerprint); + records + .iter() + .find(|record| { + record[field] + .as_str() + .is_some_and(|text| text.lines().any(|line| line == marker)) + }) + .map(|record| self.record(record)) + .transpose() + } + + pub fn record(&self, value: &Value) -> Result { + Ok(match self { + Self::Advisories(_) => SecurityRecord::Advisory { + id: value["ghsa_id"] + .as_str() + .context("Security finding record has no advisory ID")? + .to_string(), + description: value["description"] + .as_str() + .context("Security finding record has no description")? + .to_string(), + }, + Self::Issues(_) => SecurityRecord::Issue { + number: value["number"] + .as_u64() + .context("Security finding record has no issue number")?, + description: value["body"] + .as_str() + .context("Security finding record has no body")? + .to_string(), + }, + }) + } +} + +/// A finding read from the private storage selected for this repository. +pub enum SecurityRecord { + Advisory { id: String, description: String }, + Issue { number: u64, description: String }, +} + +impl SecurityRecord { + pub fn description(&self) -> &str { + match self { + Self::Advisory { description, .. } | Self::Issue { description, .. } => description, + } + } + + pub fn name(&self) -> String { + match self { + Self::Advisory { id, .. } => id.clone(), + Self::Issue { number, .. } => format!("issue #{number}"), + } + } } #[derive(Clone, Debug, Serialize)] @@ -41,21 +104,6 @@ pub struct DraftAdvisory { pub package: Package, } -impl DraftAdvisory { - pub fn already_recorded(&self, records: &SecurityRecords) -> bool { - let marker = format!("Fingerprint: `{}`", self.fingerprint); - let (records, field) = match records { - SecurityRecords::Advisories(records) => (records, "description"), - SecurityRecords::Issues(records) => (records, "body"), - }; - records.iter().any(|record| { - record[field] - .as_str() - .is_some_and(|description| description.lines().any(|line| line == marker)) - }) - } -} - impl GitHub { /// Every state and every page. A 404 selects issues only when repository /// metadata confirms they will be private. @@ -122,16 +170,49 @@ impl GitHub { } let url = std::str::from_utf8(&output.stdout) .context("creating a private Security finding issue returned invalid UTF-8")?; - IssueUrl::parse(url.trim()).map_err(|_| { + let issue = IssueUrl::parse(url.trim()).map_err(|_| { anyhow::anyhow!( "creating a private Security finding issue returned no issue URL" ) })?; - Ok(json!({"body": draft.description})) + self.issue_view(&issue, "number,body") } } } + /// Only the reproduction fields change; state, labels and disclosure stay + /// with the Day shift. API diagnostics may contain private test evidence. + pub fn update_security_record( + &self, + repo: &str, + record: &SecurityRecord, + reproduction: &Reproduction, + ) -> Result<()> { + let description = reproduction.description(record.description()); + let (path, body) = match record { + SecurityRecord::Advisory { id, .. } => ( + format!("repos/{repo}/security-advisories/{id}"), + json!({"description": description, "severity": reproduction.severity()}), + ), + SecurityRecord::Issue { number, .. } => ( + format!("repos/{repo}/issues/{number}"), + json!({"body": description}), + ), + }; + let body = serde_json::to_vec(&body)?; + let output = self.output_with_input( + &["api", "--method", "PATCH", &path, "--input", "-"], + Some(&body), + )?; + if !output.status.success() { + bail!( + "updating a private Security finding record failed ({})", + output.status + ); + } + Ok(()) + } + /// The create endpoint creates a draft. No severity or version claim. fn create_security_advisory(&self, repo: &str, draft: &DraftAdvisory) -> Result { let body = serde_json::to_vec(&json!({ diff --git a/src/pass.rs b/src/pass.rs index bb49eba..46c82c9 100644 --- a/src/pass.rs +++ b/src/pass.rs @@ -14,7 +14,8 @@ use std::fmt::Display; use std::path::PathBuf; -use crate::github::{DraftAdvisory, SecurityRecords}; +use crate::github::{DraftAdvisory, SecurityRecord, SecurityRecords}; +use crate::security::reproduction::Reproduction; use anyhow::Result; use serde_json::Value; @@ -108,6 +109,18 @@ pub trait Outside { records: &SecurityRecords, finding: &DraftAdvisory, ) -> Result; + /// Reproduce the finding read from this record, in a fresh checkout. + fn reproduce( + &mut self, + record: &SecurityRecord, + number: usize, + ) -> (Result, Option); + /// Write only a completed reproduction to its private record. + fn update_security_record( + &mut self, + record: &SecurityRecord, + reproduction: &Reproduction, + ) -> Result<()>; /// Run `dispatch` to its end, on the Harness the pass checked. fn dispatch(&mut self, dispatch: Dispatch) -> Ended; } @@ -217,6 +230,28 @@ impl Outside for LaunchAndGitHub<'_> { GitHub::new().create_security_record(&self.repo.slug(), records, finding) } + fn reproduce( + &mut self, + record: &SecurityRecord, + number: usize, + ) -> (Result, Option) { + crate::security::reproduction::run( + self.launch.git(), + self.repo, + record, + number, + self.harness, + ) + } + + fn update_security_record( + &mut self, + record: &SecurityRecord, + reproduction: &Reproduction, + ) -> Result<()> { + GitHub::new().update_security_record(&self.repo.slug(), record, reproduction) + } + /// The run's asks are the Architect plan's or the Ready issue's, from /// the command's flags and the User config, on the checked Harness. fn dispatch(&mut self, dispatch: Dispatch) -> Ended { @@ -255,13 +290,14 @@ mod in_memory { use anyhow::{Result, anyhow, bail}; use super::{Dispatch, Outside}; - use crate::github::{DraftAdvisory, SecurityRecords}; + use crate::github::{DraftAdvisory, SecurityRecord, SecurityRecords}; use crate::github::{Issue, ListedIssue}; use crate::issue::{IssueUrl, Repo}; use crate::labels::{Edit, Label, Labels}; use crate::logs::{Pass, Work}; use crate::ready::ReadyIssue; use crate::run::{Ended, Goal, Reached}; + use crate::security::reproduction::{Outcome, Reproduction}; use serde_json::{Value, json}; /// A call a pass made outside itself, in the order it made it. @@ -297,6 +333,10 @@ mod in_memory { AdvisoryList, /// It recorded this fingerprint privately. CreateAdvisory(String), + /// It tried to reproduce this private record, in sequence. + Reproduce(String), + /// It wrote a completed reproduction to this private record. + UpdateSecurityRecord(String), /// It recorded that it started work: on this issue, for a Pickup /// run, or on its repository, for an Architect run. Started(Option), @@ -636,11 +676,36 @@ mod in_memory { ) -> Result { self.calls .push(Call::CreateAdvisory(finding.fingerprint.clone())); - let advisory = json!({"description": finding.description, "state": "draft"}); + let advisory = json!({"ghsa_id": finding.fingerprint, "description": finding.description, "state": "draft"}); self.advisories.push(advisory.clone()); Ok(advisory) } + fn reproduce( + &mut self, + record: &SecurityRecord, + _number: usize, + ) -> (Result, Option) { + self.calls.push(Call::Reproduce(record.name())); + ( + Ok(Reproduction { + outcome: Outcome::NotReproduced, + notes: "Not reproduced with a local fixture".into(), + test: "bounded_fixture()".into(), + }), + self.session_log.clone(), + ) + } + + fn update_security_record( + &mut self, + record: &SecurityRecord, + _reproduction: &Reproduction, + ) -> Result<()> { + self.calls.push(Call::UpdateSecurityRecord(record.name())); + Ok(()) + } + fn dispatch(&mut self, dispatch: Dispatch) -> Ended { self.calls.push(match dispatch { Dispatch::ArchitectPlan { plan, base } => Call::DispatchPlan { diff --git a/src/prompt.rs b/src/prompt.rs index 9d8162d..2fe3f96 100644 --- a/src/prompt.rs +++ b/src/prompt.rs @@ -313,3 +313,18 @@ pub fn security_audit( root = root.display() ) } + +pub fn security_reproduction(commit: &str, finding: &str, test: &std::path::Path) -> String { + format!( + "Reproduce this recorded Security finding at audited commit `{commit}`.\n\ + Read the `thirdshift-security-audit` skill's likelihood-and-impact severity rubric.\n\ + Write a proof-of-concept test from the finding's validation plan, then run it against the untouched code in this throwaway worktree. Use harmless payloads only, never a deployed site or a real third-party service. Do not fix or change repository source.\n\ + If the test reproduces the finding, score its severity with the skill's rubric and judge whether one session can hold the fix (single) or it needs a Spec (spec).\n\ + Test file: `{test}`.\n\ + Write the test's full text to this file, even when not reproduced. In your final message, give reproduction notes: the exact command, its result, and, when reproduced, likelihood, impact and the fix-size reasoning.\n\ + End your final message with exactly `Security reproduction: reproduced `, where severity is critical, high, medium or low and size is single or spec, or `Security reproduction: not reproduced`.\n\ + This session commits, pushes, opens and publishes nothing.\n\n\ + Recorded finding:\n{finding}\n\n{HEADLESS}", + test = test.display(), + ) +} diff --git a/src/prompts_page.rs b/src/prompts_page.rs index 60935ff..5fdc937 100644 --- a/src/prompts_page.rs +++ b/src/prompts_page.rs @@ -177,6 +177,17 @@ fn prompts() -> Vec { Some("SECURITY.md"), ), }, + Prompt { + id: "prompt-security-reproduction", + title: "Security reproduction", + when: "After a Security audit, tries to reproduce one recorded finding in a fresh throwaway worktree at its audited commit. The test is copied into the private record only after a complete outcome.", + sender: SecurityRun, + text: prompt::security_reproduction( + "", + "", + std::path::Path::new(""), + ), + }, Prompt { id: "prompt-conflict-repair", title: "Conflict Repair", diff --git a/src/security.rs b/src/security.rs index 7555223..d0cecb8 100644 --- a/src/security.rs +++ b/src/security.rs @@ -1,5 +1,5 @@ -//! Report-only Security runs: gate before work, audit origin's Base branch, -//! then record the validated findings privately for the Day shift. +//! Security runs: gate before work, audit origin's Base branch, then record +//! and reproduce the findings privately for the Day shift. use std::fmt; @@ -16,6 +16,7 @@ use crate::logs::{self, Pass, Work}; use crate::pass::{LaunchAndGitHub, Outside}; pub mod audit; +pub mod reproduction; pub enum Outcome { Skipped(Skipped), @@ -109,6 +110,7 @@ fn audit_and_record( outside.check_node()?; outside.started(Work::SecurityRun(repo)); let (audited, log) = outside.audit(base); + let mut log = log; let recorded = (|| -> Result { let audited = audited?; let mut recorded = Recorded { @@ -119,16 +121,41 @@ fn audit_and_record( return Ok(recorded); } let mut known = outside.security_records()?; + let mut records = Vec::new(); + let mut seen = std::collections::HashSet::new(); for finding in audited.findings { - if finding.already_recorded(&known) { + let record = if let Some(record) = known.finding(&finding)? { recorded.existing += 1; + record } else { let record = outside.create_security_record(&known, &finding)?; + let read = known.record(&record)?; known.remember(record); recorded.created += 1; outside.step("recorded a Security finding privately".to_string()); + read + }; + if seen.insert(finding.fingerprint) { + records.push(record); } } + for (index, record) in records.iter().enumerate() { + let number = index + 1; + outside.step(format!( + "starting Security reproduction {number} of {}", + record.name() + )); + let (reproduced, session_log) = outside.reproduce(record, number); + if session_log.is_some() { + log = session_log; + } + let reproduced = reproduced?; + outside.update_security_record(record, &reproduced)?; + outside.step(format!( + "Security reproduction {number}: {}", + reproduced.outcome + )); + } Ok(recorded) })(); recorded.map_err(|error| FailedRun { @@ -222,7 +249,7 @@ mod tests { let mut outside = InMemory::default() .audited(vec![draft("old"), draft("new")]) .advisories(vec![ - json!({"state":"closed", "description":"Fingerprint: `old`"}), + json!({"ghsa_id":"old", "state":"closed", "description":"Fingerprint: `old`"}), ]); let Outcome::Audited(Ok(recorded)) = run_through(&mut outside, &widgets(), "main") else { panic!("audit should succeed"); @@ -239,5 +266,22 @@ mod tests { .collect::>(), vec!["new"] ); + assert_eq!( + outside + .calls + .iter() + .filter(|call| matches!( + call, + Call::CreateAdvisory(_) | Call::Reproduce(_) | Call::UpdateSecurityRecord(_) + )) + .collect::>(), + vec![ + &Call::CreateAdvisory("new".into()), + &Call::Reproduce("old".into()), + &Call::UpdateSecurityRecord("old".into()), + &Call::Reproduce("new".into()), + &Call::UpdateSecurityRecord("new".into()), + ] + ); } } diff --git a/src/security/reproduction.rs b/src/security/reproduction.rs new file mode 100644 index 0000000..352ffd4 --- /dev/null +++ b/src/security/reproduction.rs @@ -0,0 +1,193 @@ +//! One proof-of-concept session in an owned disposable checkout. Its test +//! and final message are collected before the checkout is removed. + +use std::fmt; +use std::fs; +use std::path::PathBuf; + +use anyhow::{Context, Result, bail}; +use serde::Serialize; + +use crate::git::Git; +use crate::github::SecurityRecord; +use crate::harness::Choice; +use crate::issue::Repo; +use crate::logs; +use crate::prompt; +use crate::session::{Logs, Sessions}; +use crate::worktree::ReviewWorktree; + +#[derive(Clone, Copy, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum Severity { + Critical, + High, + Medium, + Low, +} + +impl Severity { + fn name(self) -> &'static str { + match self { + Self::Critical => "critical", + Self::High => "high", + Self::Medium => "medium", + Self::Low => "low", + } + } +} + +pub enum FixSize { + Single, + Spec, +} + +impl FixSize { + fn name(&self) -> &'static str { + match self { + Self::Single => "single", + Self::Spec => "spec", + } + } +} + +pub enum Outcome { + Reproduced { severity: Severity, size: FixSize }, + NotReproduced, +} + +impl fmt::Display for Outcome { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Reproduced { severity, size } => { + write!(f, "reproduced {} {}", severity.name(), size.name()) + } + Self::NotReproduced => write!(f, "not reproduced"), + } + } +} + +pub struct Reproduction { + pub outcome: Outcome, + pub notes: String, + pub test: String, +} + +impl Reproduction { + pub fn severity(&self) -> Option { + match self.outcome { + Outcome::Reproduced { severity, .. } => Some(severity), + Outcome::NotReproduced => None, + } + } + + pub fn description(&self, original: &str) -> String { + let original = original + .split_once("\n## Reproduction\n") + .map_or(original, |(original, _)| original); + let severity = self + .severity() + .map(|severity| format!("Severity: {}\n", severity.name())) + .unwrap_or_default(); + let size = match &self.outcome { + Outcome::Reproduced { size, .. } => format!("Fix size: {}\n", size.name()), + Outcome::NotReproduced => String::new(), + }; + // A test may itself contain Markdown fences. Preserve its text without + // allowing one of those fences to close the record's code block. + let fence = "`".repeat( + self.test + .lines() + .map(|line| line.chars().take_while(|c| *c == '`').count()) + .max() + .unwrap_or(0) + .max(2) + + 1, + ); + format!( + "{}\n\n## Reproduction\n\nOutcome: {}\n{severity}{size}\n{}\n\n### Proof-of-concept test\n\n{fence}\n{}{fence}\n", + original.trim_end(), + self.outcome, + self.notes, + if self.test.ends_with('\n') { + self.test.clone() + } else { + format!("{}\n", self.test) + } + ) + } +} + +pub fn run( + launch: &Git, + repo: &Repo, + record: &SecurityRecord, + number: usize, + harness: &Choice, +) -> (Result, Option) { + let prepared = (|| -> Result<_> { + let commit = record + .description() + .lines() + .find_map(|line| { + line.strip_prefix("Audited commit: `") + .and_then(|commit| commit.strip_suffix('`')) + }) + .context("Security finding record has no audited commit")?; + if ![40, 64].contains(&commit.len()) || !commit.bytes().all(|byte| byte.is_ascii_hexdigit()) + { + bail!("Security finding record has an invalid audited commit"); + } + let worktree = ReviewWorktree::at_commit(launch, &repo.name, commit)?; + let root = logs::root(repo).join("reproductions"); + fs::create_dir_all(&root)?; + let output = tempfile::Builder::new() + .prefix("reproduction-") + .tempdir_in(root)?; + let test = output.path().join("test.txt"); + let prompt = prompt::security_reproduction(commit, record.description(), &test); + Ok((worktree, output, test, prompt)) + })(); + let (worktree, _output, test, prompt) = match prepared { + Ok(prepared) => prepared, + Err(error) => return (Err(error), None), + }; + let logs = Logs::of_security_run(repo); + Sessions::within(&logs, worktree.path(), harness, |sessions| { + let message = + sessions.run_to_final_message(&format!("security-reproduction-{number}"), &prompt)?; + let message = message.as_deref().unwrap_or_default().trim_end(); + let (notes, line) = message.rsplit_once('\n').unwrap_or(("", message)); + let outcome = match line.split_whitespace().collect::>().as_slice() { + ["Security", "reproduction:", "not", "reproduced"] => Outcome::NotReproduced, + ["Security", "reproduction:", "reproduced", severity, size] => { + let severity = match *severity { + "critical" => Severity::Critical, + "high" => Severity::High, + "medium" => Severity::Medium, + "low" => Severity::Low, + _ => bail!("the Security reproduction ended with an invalid severity"), + }; + let size = match *size { + "single" => FixSize::Single, + "spec" => FixSize::Spec, + _ => bail!("the Security reproduction ended with an invalid fix size"), + }; + Outcome::Reproduced { severity, size } + } + _ => { + bail!("the Security reproduction ended without the final line its prompt asks for") + } + }; + let test = fs::read_to_string(test) + .context("could not read the Security reproduction's test file")?; + if notes.trim().is_empty() || test.trim().is_empty() { + bail!("the Security reproduction ended without reproduction notes or test text"); + } + Ok(Reproduction { + outcome, + notes: notes.trim().to_string(), + test, + }) + }) +} diff --git a/src/worktree.rs b/src/worktree.rs index 3ddb00c..3f0dea0 100644 --- a/src/worktree.rs +++ b/src/worktree.rs @@ -356,6 +356,23 @@ pub struct ReviewWorktree { } impl ReviewWorktree { + /// Reproduce one finding at its audited commit, without fetching a newer + /// Base branch. Uses the review checkout's acquisition and disposal rules. + pub fn at_commit(launch: &Git, repo: &str, commit: &str) -> Result { + let _lock = lock_launch(launch)?; + let (root, path) = sibling(launch, &format!("{repo}-security-reproduce"))?; + ownership::recover_review(launch, &path)?; + progress::step(format_args!( + "creating worktree {} detached at {commit}", + path.display() + )); + let checkout = acquisition::add(launch, None, &path, commit)?; + Ok(Self { + launch: Git::new(root), + checkout, + }) + } + /// Check out `origin/`, detached, in a new worktree next to the /// launch repository's root, named `-architect`. A worktree left /// there by a process that ended before cleanup is removed only with diff --git a/tests/fakes/gh.rs b/tests/fakes/gh.rs index 05acf81..d6e755d 100644 --- a/tests/fakes/gh.rs +++ b/tests/fakes/gh.rs @@ -1038,6 +1038,25 @@ fn advisory_api(state: &mut Json, positional: &[String], flags: &Flags) { println!("{body}"); } +/// Update a finding issue's body through the private-record operation. +fn finding_issue_patch(state: &mut Json, rest: &[&str]) { + use std::io::Read; + let (positional, flags) = parse(rest); + let (repo, path) = repo_prefix(&positional[0]).unwrap(); + check_repo_is(state, Some(repo)); + let n = path.strip_prefix("issues/").unwrap(); + assert_eq!(flag(&flags, "input"), Some("-")); + let mut input = String::new(); + std::io::stdin().read_to_string(&mut input).unwrap(); + let body = parse_json(&input); + let issue_state = state.at("issues").at(n).clone(); + state + .entry("bodies", object([])) + .set(n, body.at("body").clone()); + save(state); + println!("{}", issue_fields(state, n, &issue_state)); +} + fn api(state: &mut Json, positional: &[String], flags: &Flags) { let path = &positional[0]; if path == &format!("repos/{}", state.at("repo").str()) { @@ -1901,6 +1920,11 @@ pub fn main(args: Vec) { release_view(&state, &positional, &flags); } ["api", "graphql", rest @ ..] => graphql(&state, rest), + ["api", "--method", "PATCH", rest @ ..] + if rest.iter().any(|word| word.contains("/issues/")) => + { + finding_issue_patch(&mut state, rest) + } ["api", "--method", "PATCH", rest @ ..] if !rest .iter() diff --git a/tests/security_run.rs b/tests/security_run.rs index 7a10764..f3dcfa8 100644 --- a/tests/security_run.rs +++ b/tests/security_run.rs @@ -17,6 +17,268 @@ fn finding(fingerprint: &str) -> Value { }) } +#[test] +fn a_reproduction_scores_the_finding_and_keeps_its_test_in_the_private_record() { + let scenario = Scenario::new(); + scenario.agent_does_in_session( + 1, + &audit_script(&json!([finding("input-size")]).to_string()), + ); + scenario.agent_does_in_session(2, r#" +printf '%s' "$FAKE_CLAUDE_PROMPT" > prompt.txt +test_file=$(sed -n 's/^Test file: `\(.*\)`\.$/\1/p' prompt.txt) +test -n "$test_file" +printf '%s\n' 'assert_eq!(bounded_fixture(), "overflow");' > "$test_file" +printf '%s\n' 'A harmless local fixture crossed the storage bound. Likelihood high, impact high; one session can hold the fix.' 'Security reproduction: reproduced high single' > "$FAKE_CLAUDE_FINAL_MESSAGE" +"#); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + let state = scenario.gh_state(); + let record = &state["advisories"][0]; + assert_eq!(record["severity"], "high"); + let body = record["description"].as_str().unwrap(); + assert!(body.contains("Likelihood high, impact high"), "{body}"); + assert!( + body.contains("assert_eq!(bounded_fixture(), \"overflow\");"), + "{body}" + ); + assert!(body.contains("Fix size: single"), "{body}"); + assert!( + result + .stderr + .contains("reproduction 1: reproduced high single"), + "{}", + result.stderr + ); + assert_eq!(scenario.claude_calls().len(), 2); + scenario.assert_every_session_found_the_factory_skills(); +} + +#[test] +fn a_private_reproduction_writes_severity_size_notes_and_test_into_the_finding_issue() { + let scenario = Scenario::new(); + let mut github = scenario.gh_state(); + github["private"] = json!(true); + scenario.write_gh_state(&github); + scenario.agent_does_in_session( + 1, + &audit_script(&json!([finding("input-size")]).to_string()), + ); + scenario.agent_does_in_session(2, &reproduction_script("reproduced medium spec")); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + let state = scenario.gh_state(); + let body = state["bodies"]["8"].as_str().unwrap(); + for required in [ + "Fingerprint: `input-size`", + "Private candidate write-up.", + "Severity: medium", + "Fix size: spec", + "Local command: bounded-fixture", + "bounded_fixture();", + ] { + assert!(body.contains(required), "missing {required}: {body}"); + } + assert_eq!( + state["labels"]["8"], + json!(["security-finding", "needs-triage"]) + ); + assert_eq!(state["issues"]["8"], "OPEN"); + assert!(result.stderr.contains("issue #8"), "{}", result.stderr); + assert!(!result.stderr.contains("Local command: bounded-fixture")); +} + +fn reproduction_script(outcome: &str) -> String { + format!( + r#" +printf '%s' "$FAKE_CLAUDE_PROMPT" > prompt.txt +test_file=$(sed -n 's/^Test file: `\(.*\)`\.$/\1/p' prompt.txt) +test -n "$test_file" +printf '%s\n' 'bounded_fixture();' > "$test_file" +printf '%s\n' 'Local command: bounded-fixture; harmless local evidence and likelihood/impact reasoning.' 'Security reproduction: {outcome}' > "$FAKE_CLAUDE_FINAL_MESSAGE" +"# + ) +} + +#[test] +fn reproductions_are_sequential_fresh_and_pinned_to_the_audited_commit() { + let scenario = Scenario::new(); + let audited = scenario.origin_git(&["rev-parse", "main"]); + scenario.agent_does_in_session( + 1, + &format!( + r#"{} +touch audit-only +new=$(git commit-tree "$(git rev-parse 'HEAD^{{tree}}')" -p HEAD -m 'Base advanced') +git push origin "$new:refs/heads/main" +"#, + audit_script(&json!([finding("first"), finding("second")]).to_string()) + ), + ); + scenario.agent_does_in_session( + 2, + &format!( + r#" +test -z "$(git branch --show-current)" +test ! -e audit-only +git rev-parse HEAD >> "$HOME/../reproductions" +printf '%s\n' first >> "$HOME/../sequence" +touch previous-poc +{} +"#, + reproduction_script("reproduced low single") + ), + ); + scenario.agent_does_in_session( + 3, + &format!( + r#" +test ! -e previous-poc +test ! -e audit-only +test "$(cat "$HOME/../sequence")" = first +git rev-parse HEAD >> "$HOME/../reproductions" +printf '%s\n' second >> "$HOME/../sequence" +{} +"#, + reproduction_script("not reproduced") + ), + ); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + assert_ne!(scenario.origin_git(&["rev-parse", "main"]), audited); + assert_eq!( + fs::read_to_string(scenario.path("reproductions")).unwrap(), + format!("{audited}{audited}") + ); + assert_eq!( + fs::read_to_string(scenario.path("sequence")).unwrap(), + "first\nsecond\n" + ); + let calls = scenario.claude_calls(); + assert_eq!(calls.len(), 3); + for (index, fingerprint) in [(1, "first"), (2, "second")] { + let prompt = calls[index]["prompt"].as_str().unwrap(); + for required in [ + "validation plan", + "untouched code", + "harmless payloads only", + "never a deployed site or a real third-party service", + "likelihood-and-impact", + "one session", + "Test file:", + "Security reproduction:", + "You run headless", + ] { + assert!(prompt.contains(required), "missing {required}: {prompt}"); + } + assert!( + prompt.contains(&format!("Fingerprint: `{fingerprint}`")), + "{prompt}" + ); + assert_eq!(calls[index]["branch"], ""); + } + assert_eq!(scenario.entries("work"), vec![REPO]); + scenario.assert_every_session_found_the_factory_skills(); +} + +#[test] +fn incomplete_reproductions_preserve_the_record_and_stop_before_the_next_session() { + for private in [false, true] { + for (outcome, cause) in [ + ("No final line", "without the final line"), + ("reproduced invalid single", "invalid severity"), + ("reproduced high invalid", "invalid fix size"), + ] { + let scenario = Scenario::new(); + let commit = scenario.origin_git(&["rev-parse", "main"]); + let original = format!( + "Fingerprint: `first`\nAudited commit: `{}`\nExisting private evidence.\n", + commit.trim() + ); + let mut github = scenario.gh_state(); + if private { + github["private"] = json!(true); + github["issues"]["8"] = json!("OPEN"); + github["bodies"] = json!({"8": original}); + github["labels"] = json!({"8": ["security-finding", "needs-triage"]}); + } else { + github["advisories"] = json!([{"ghsa_id": "GHSA-existing", "description": original, "severity": "high", "state": "draft"}]); + } + 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, &reproduction_script(outcome)); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(1), "{}", result.stderr); + assert!(result.stderr.contains(cause), "{}", result.stderr); + assert!( + result.stderr.contains("security-reproduction-1.jsonl"), + "{}", + result.stderr + ); + let state = scenario.gh_state(); + if private { + assert_eq!(state["bodies"]["8"], original); + assert_eq!(state["labels"]["8"], github["labels"]["8"]); + assert!( + state["bodies"]["9"] + .as_str() + .unwrap() + .contains("Fingerprint: `second`") + ); + } else { + assert_eq!(state["advisories"][0], github["advisories"][0]); + assert_eq!(state["advisories"].as_array().unwrap().len(), 2); + } + assert_eq!(scenario.claude_calls().len(), 2); + assert!( + scenario + .gh_calls_of("api", "--method") + .iter() + .all(|call| call[2] != "PATCH") + ); + assert_eq!(scenario.entries("work"), vec![REPO]); + } + } +} + +#[test] +fn an_unreproduced_finding_clears_its_previous_severity_and_keeps_the_test_and_notes() { + let scenario = Scenario::new(); + let commit = scenario.origin_git(&["rev-parse", "main"]); + let original = format!( + "Fingerprint: `input-size`\nAudited commit: `{}`\nExisting private evidence.\n", + commit.trim() + ); + let mut github = scenario.gh_state(); + github["advisories"] = json!([{"ghsa_id": "GHSA-existing", "description": original, "severity": "high", "state": "draft"}]); + scenario.write_gh_state(&github); + scenario.agent_does_in_session( + 1, + &audit_script(&json!([finding("input-size")]).to_string()), + ); + scenario.agent_does_in_session(2, &reproduction_script("not reproduced")); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + let state = scenario.gh_state(); + let record = &state["advisories"][0]; + assert_eq!(record["severity"], Value::Null); + let body = record["description"].as_str().unwrap(); + assert!(body.starts_with(original.trim_end()), "{body}"); + assert!(body.contains("Outcome: not reproduced"), "{body}"); + assert!(body.contains("Local command: bounded-fixture"), "{body}"); + assert!(body.contains("bounded_fixture();"), "{body}"); + assert!(!body.contains("Severity:"), "{body}"); + assert!(!body.contains("Fix size:"), "{body}"); + assert!( + result.stderr.contains("reproduction 1: not reproduced"), + "{}", + result.stderr + ); +} + #[test] fn records_a_finding_privately_as_a_draft_without_severity_or_versions() { let scenario = Scenario::new(); @@ -95,7 +357,10 @@ fn a_private_repository_records_each_finding_as_a_labelled_issue_with_the_adviso commit.trim(), serde_json::to_string_pretty(&finding(fingerprint)).unwrap() ); - assert_eq!(state["bodies"][number], expected); + let body = state["bodies"][number].as_str().unwrap(); + assert!(body.starts_with(&expected), "{body}"); + assert!(body.contains("Outcome: not reproduced"), "{body}"); + assert!(!body.contains("Severity:"), "{body}"); } assert!(!result.stderr.contains("Private candidate write-up.")); } @@ -231,6 +496,12 @@ fn audit_script(findings: &str) -> String { format!( r#" printf '%s' "$FAKE_CLAUDE_PROMPT" > prompt.txt +test_file=$(sed -n 's/^Test file: `\(.*\)`\.$/\1/p' prompt.txt) +if [ -n "$test_file" ]; then + printf '%s\n' 'bounded_fixture();' > "$test_file" + printf '%s\n' 'The harmless bounded fixture did not reproduce the claim.' 'Security reproduction: not reproduced' > "$FAKE_CLAUDE_FINAL_MESSAGE" + exit 0 +fi output=$(sed -n 's/^Output directory: `\(.*\)`\.$/\1/p' prompt.txt) test -n "$output" printf '%s\n' '{findings}' > "$output/findings.json" @@ -268,7 +539,10 @@ fn keeps_fingerprints_in_every_advisory_state_and_creates_each_new_finding_once( assert_eq!(scenario.gh_state()["advisories"], github["advisories"]); let api_calls = scenario.gh_calls_of("api", "--method"); assert_eq!(api_calls.iter().filter(|call| call[2] == "POST").count(), 2); - assert!(api_calls.iter().all(|call| call[2] == "POST")); + assert_eq!( + api_calls.iter().filter(|call| call[2] == "PATCH").count(), + 4 + ); assert_eq!( scenario .entries("home/.thirdshift/logs/acme/widgets/audits") From e6f0e5b3afa120a78babe9a44197f90fe18eb96d Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Thu, 8 Oct 2026 06:51:38 -0400 Subject: [PATCH 2/4] Preserve finding evidence and Day shift grades after Spec review (#542) --- prompts/security-reproduction.md | 2 +- site/prompts/index.html | 2 +- src/github/advisories.rs | 79 ++++++++++++++-- src/prompt.rs | 2 +- src/security.rs | 9 +- src/security/reproduction.rs | 7 +- tests/fakes/gh.rs | 17 ++++ tests/security_run.rs | 156 ++++++++++++++++++++++++++++++- 8 files changed, 254 insertions(+), 20 deletions(-) diff --git a/prompts/security-reproduction.md b/prompts/security-reproduction.md index eb65a95..b5be193 100644 --- a/prompts/security-reproduction.md +++ b/prompts/security-reproduction.md @@ -11,7 +11,7 @@ Write a proof-of-concept test from the finding's validation plan, then run it ag If the test reproduces the finding, score its severity with the skill's rubric and judge whether one session can hold the fix (single) or it needs a Spec (spec). Test file: ``. Write the test's full text to this file, even when not reproduced. In your final message, give reproduction notes: the exact command, its result, and, when reproduced, likelihood, impact and the fix-size reasoning. -End your final message with exactly `Security reproduction: reproduced `, where severity is critical, high, medium or low and size is single or spec, or `Security reproduction: not reproduced`. +End your final message with exactly `Security reproduction: reproduced `, where severity is critical, high, medium, low or informational and size is single or spec, or `Security reproduction: not reproduced`. This session commits, pushes, opens and publishes nothing. Recorded finding: diff --git a/site/prompts/index.html b/site/prompts/index.html index e094f25..94d9a73 100644 --- a/site/prompts/index.html +++ b/site/prompts/index.html @@ -187,7 +187,7 @@

Security reproduction

If the test reproduces the finding, score its severity with the skill's rubric and judge whether one session can hold the fix (single) or it needs a Spec (spec). Test file: `<test file>`. Write the test's full text to this file, even when not reproduced. In your final message, give reproduction notes: the exact command, its result, and, when reproduced, likelihood, impact and the fix-size reasoning. -End your final message with exactly `Security reproduction: reproduced <severity> <size>`, where severity is critical, high, medium or low and size is single or spec, or `Security reproduction: not reproduced`. +End your final message with exactly `Security reproduction: reproduced <severity> <size>`, where severity is critical, high, medium, low or informational and size is single or spec, or `Security reproduction: not reproduced`. This session commits, pushes, opens and publishes nothing. Recorded finding: diff --git a/src/github/advisories.rs b/src/github/advisories.rs index 07626af..844a054 100644 --- a/src/github/advisories.rs +++ b/src/github/advisories.rs @@ -8,7 +8,7 @@ use serde_json::{Value, json}; use super::GitHub; use crate::issue::IssueUrl; use crate::labels::{Label, NEEDS_TRIAGE}; -use crate::security::reproduction::Reproduction; +use crate::security::reproduction::{Reproduction, Severity}; const SECURITY_FINDING: Label = Label::new( "security-finding", @@ -55,6 +55,7 @@ impl SecurityRecords { .as_str() .context("Security finding record has no description")? .to_string(), + untriaged: value["state"] == "draft" && value["severity"].is_null(), }, Self::Issues(_) => SecurityRecord::Issue { number: value["number"] @@ -64,6 +65,16 @@ impl SecurityRecords { .as_str() .context("Security finding record has no body")? .to_string(), + untriaged: value["state"] + .as_str() + .is_some_and(|state| state.eq_ignore_ascii_case("open")) + && value["labels"].as_array().is_some_and(|labels| { + labels.iter().any(|label| { + label["name"] + .as_str() + .is_some_and(|name| name.eq_ignore_ascii_case(NEEDS_TRIAGE.name())) + }) + }), }, }) } @@ -71,11 +82,27 @@ impl SecurityRecords { /// A finding read from the private storage selected for this repository. pub enum SecurityRecord { - Advisory { id: String, description: String }, - Issue { number: u64, description: String }, + Advisory { + id: String, + description: String, + untriaged: bool, + }, + Issue { + number: u64, + description: String, + untriaged: bool, + }, } impl SecurityRecord { + /// A Day-shift decision is the finding's grade; a repeated fingerprint + /// must not publish new proof-of-concept evidence or replace that grade. + pub fn untriaged(&self) -> bool { + match self { + Self::Advisory { untriaged, .. } | Self::Issue { untriaged, .. } => *untriaged, + } + } + pub fn description(&self) -> &str { match self { Self::Advisory { description, .. } | Self::Issue { description, .. } => description, @@ -175,7 +202,7 @@ impl GitHub { "creating a private Security finding issue returned no issue URL" ) })?; - self.issue_view(&issue, "number,body") + self.issue_view(&issue, "number,body,state,labels") } } } @@ -188,17 +215,53 @@ impl GitHub { record: &SecurityRecord, reproduction: &Reproduction, ) -> Result<()> { - let description = reproduction.description(record.description()); - let (path, body) = match record { + if !record.untriaged() { + bail!("refusing to replace a triaged Security finding record"); + } + let (path, storage) = match record { SecurityRecord::Advisory { id, .. } => ( format!("repos/{repo}/security-advisories/{id}"), - json!({"description": description, "severity": reproduction.severity()}), + SecurityRecords::Advisories(Vec::new()), ), SecurityRecord::Issue { number, .. } => ( format!("repos/{repo}/issues/{number}"), - json!({"body": description}), + SecurityRecords::Issues(Vec::new()), ), }; + let observed = self.output_with_input(&["api", &path], None)?; + if !observed.status.success() { + bail!( + "reading a private Security finding record before its update failed ({})", + observed.status + ); + } + let observed: Value = serde_json::from_slice(&observed.stdout) + .context("reading a private Security finding record returned invalid JSON")?; + let current = storage.record(&observed)?; + if !current.untriaged() { + bail!( + "the Security finding was triaged during its reproduction; leaving the record unchanged" + ); + } + if current.description() != record.description() { + bail!( + "the Security finding changed during its reproduction; leaving the record unchanged" + ); + } + if matches!(record, SecurityRecord::Advisory { .. }) + && matches!(reproduction.severity(), Some(Severity::Informational)) + { + bail!( + "informational severity has no GitHub advisory field; the Day shift must decide its representation; leaving the record unchanged" + ); + } + let description = reproduction.description(record.description()); + let body = match record { + SecurityRecord::Advisory { .. } => { + json!({"description": description, "severity": reproduction.severity()}) + } + SecurityRecord::Issue { .. } => json!({"body": description}), + }; let body = serde_json::to_vec(&body)?; let output = self.output_with_input( &["api", "--method", "PATCH", &path, "--input", "-"], diff --git a/src/prompt.rs b/src/prompt.rs index 2fe3f96..aa306b8 100644 --- a/src/prompt.rs +++ b/src/prompt.rs @@ -322,7 +322,7 @@ pub fn security_reproduction(commit: &str, finding: &str, test: &std::path::Path If the test reproduces the finding, score its severity with the skill's rubric and judge whether one session can hold the fix (single) or it needs a Spec (spec).\n\ Test file: `{test}`.\n\ Write the test's full text to this file, even when not reproduced. In your final message, give reproduction notes: the exact command, its result, and, when reproduced, likelihood, impact and the fix-size reasoning.\n\ - End your final message with exactly `Security reproduction: reproduced `, where severity is critical, high, medium or low and size is single or spec, or `Security reproduction: not reproduced`.\n\ + End your final message with exactly `Security reproduction: reproduced `, where severity is critical, high, medium, low or informational and size is single or spec, or `Security reproduction: not reproduced`.\n\ This session commits, pushes, opens and publishes nothing.\n\n\ Recorded finding:\n{finding}\n\n{HEADLESS}", test = test.display(), diff --git a/src/security.rs b/src/security.rs index d0cecb8..6411d21 100644 --- a/src/security.rs +++ b/src/security.rs @@ -135,8 +135,13 @@ fn audit_and_record( outside.step("recorded a Security finding privately".to_string()); read }; - if seen.insert(finding.fingerprint) { + if record.untriaged() && seen.insert(finding.fingerprint) { records.push(record); + } else if !record.untriaged() { + outside.step(format!( + "keeping the Day shift's grade for {}", + record.name() + )); } } for (index, record) in records.iter().enumerate() { @@ -277,8 +282,6 @@ mod tests { .collect::>(), vec![ &Call::CreateAdvisory("new".into()), - &Call::Reproduce("old".into()), - &Call::UpdateSecurityRecord("old".into()), &Call::Reproduce("new".into()), &Call::UpdateSecurityRecord("new".into()), ] diff --git a/src/security/reproduction.rs b/src/security/reproduction.rs index 352ffd4..da23b1d 100644 --- a/src/security/reproduction.rs +++ b/src/security/reproduction.rs @@ -24,6 +24,7 @@ pub enum Severity { High, Medium, Low, + Informational, } impl Severity { @@ -33,6 +34,7 @@ impl Severity { Self::High => "high", Self::Medium => "medium", Self::Low => "low", + Self::Informational => "informational", } } } @@ -83,7 +85,7 @@ impl Reproduction { pub fn description(&self, original: &str) -> String { let original = original - .split_once("\n## Reproduction\n") + .split_once("\n\n") .map_or(original, |(original, _)| original); let severity = self .severity() @@ -105,7 +107,7 @@ impl Reproduction { + 1, ); format!( - "{}\n\n## Reproduction\n\nOutcome: {}\n{severity}{size}\n{}\n\n### Proof-of-concept test\n\n{fence}\n{}{fence}\n", + "{}\n\n\n## Reproduction\n\nOutcome: {}\n{severity}{size}\n{}\n\n### Proof-of-concept test\n\n{fence}\n{}{fence}\n", original.trim_end(), self.outcome, self.notes, @@ -166,6 +168,7 @@ pub fn run( "high" => Severity::High, "medium" => Severity::Medium, "low" => Severity::Low, + "informational" => Severity::Informational, _ => bail!("the Security reproduction ended with an invalid severity"), }; let size = match *size { diff --git a/tests/fakes/gh.rs b/tests/fakes/gh.rs index d6e755d..cd82962 100644 --- a/tests/fakes/gh.rs +++ b/tests/fakes/gh.rs @@ -976,6 +976,16 @@ fn advisory_api(state: &mut Json, positional: &[String], flags: &Flags) { } let method = flag(flags, "method").unwrap_or("GET"); if method == "GET" { + if let Some(id) = rest.strip_prefix("security-advisories/") { + let advisory = state + .at("advisories") + .items() + .iter() + .find(|advisory| advisory.at("ghsa_id").as_str() == Some(id)) + .unwrap(); + println!("{advisory}"); + return; + } let wanted = query .split('&') .find_map(|pair| pair.strip_prefix("state=")); @@ -1059,6 +1069,13 @@ fn finding_issue_patch(state: &mut Json, rest: &[&str]) { fn api(state: &mut Json, positional: &[String], flags: &Flags) { let path = &positional[0]; + if let Some((repo, rest)) = repo_prefix(path) + && let Some(n) = rest.strip_prefix("issues/") + { + check_repo_is(state, Some(repo)); + println!("{}", issue_fields(state, n, state.at("issues").at(n))); + return; + } if path == &format!("repos/{}", state.at("repo").str()) { println!( "{}", diff --git a/tests/security_run.rs b/tests/security_run.rs index f3dcfa8..07e5a1a 100644 --- a/tests/security_run.rs +++ b/tests/security_run.rs @@ -202,7 +202,7 @@ fn incomplete_reproductions_preserve_the_record_and_stop_before_the_next_session github["bodies"] = json!({"8": original}); github["labels"] = json!({"8": ["security-finding", "needs-triage"]}); } else { - github["advisories"] = json!([{"ghsa_id": "GHSA-existing", "description": original, "severity": "high", "state": "draft"}]); + github["advisories"] = json!([{"ghsa_id": "GHSA-existing", "description": original, "severity": null, "state": "draft"}]); } scenario.write_gh_state(&github); scenario.agent_does_in_session( @@ -245,7 +245,7 @@ fn incomplete_reproductions_preserve_the_record_and_stop_before_the_next_session } #[test] -fn an_unreproduced_finding_clears_its_previous_severity_and_keeps_the_test_and_notes() { +fn an_unreproduced_finding_keeps_no_severity_and_keeps_the_test_and_notes() { let scenario = Scenario::new(); let commit = scenario.origin_git(&["rev-parse", "main"]); let original = format!( @@ -253,7 +253,7 @@ fn an_unreproduced_finding_clears_its_previous_severity_and_keeps_the_test_and_n commit.trim() ); let mut github = scenario.gh_state(); - github["advisories"] = json!([{"ghsa_id": "GHSA-existing", "description": original, "severity": "high", "state": "draft"}]); + github["advisories"] = json!([{"ghsa_id": "GHSA-existing", "description": original, "severity": null, "state": "draft"}]); scenario.write_gh_state(&github); scenario.agent_does_in_session( 1, @@ -541,7 +541,7 @@ fn keeps_fingerprints_in_every_advisory_state_and_creates_each_new_finding_once( assert_eq!(api_calls.iter().filter(|call| call[2] == "POST").count(), 2); assert_eq!( api_calls.iter().filter(|call| call[2] == "PATCH").count(), - 4 + if state == "draft" { 4 } else { 2 } ); assert_eq!( scenario @@ -846,3 +846,151 @@ fn security_audit_names_the_threat_model_under_docs() { "the existing threat-model document was not named: {prompt}" ); } + +#[test] +fn reproduction_preserves_an_original_finding_reproduction_heading() { + 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 candidate = finding("markdown-evidence"); + let original_writeup = "Candidate notes.\n\n## Reproduction\n\nOriginal local validation evidence must survive."; + candidate["description"] = json!(original_writeup); + scenario.agent_does_in_session(1, &audit_script(&json!([candidate]).to_string())); + scenario.agent_does_in_session(2, &reproduction_script("reproduced medium single")); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + let state = scenario.gh_state(); + let body = if private { + state["bodies"]["8"].as_str().unwrap() + } else { + state["advisories"][0]["description"].as_str().unwrap() + }; + assert!( + body.contains(original_writeup), + "original finding was deleted: {body}" + ); + assert!( + body.contains("\"validation_plan\""), + "validation plan was deleted: {body}" + ); + assert!(body.contains("Severity: medium"), "{body}"); + } +} + +#[test] +fn a_published_finding_keeps_new_reproduction_evidence_private_and_its_grade() { + let scenario = Scenario::new(); + let commit = scenario.origin_git(&["rev-parse", "main"]); + let original = format!( + "Fingerprint: `published-finding`\nAudited commit: `{}`\nDay-shift-approved published description.\n", + commit.trim() + ); + let mut github = scenario.gh_state(); + github["advisories"] = json!([{ + "ghsa_id": "GHSA-existing", "description": original, + "severity": "high", "state": "published" + }]); + scenario.write_gh_state(&github); + scenario.agent_does_in_session( + 1, + &audit_script(&json!([finding("published-finding")]).to_string()), + ); + scenario.agent_does_in_session(2, &reproduction_script("reproduced critical single")); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + let state = scenario.gh_state(); + assert_eq!( + state["advisories"][0]["description"], original, + "new reproduction evidence became part of a published advisory" + ); + assert_eq!( + state["advisories"][0]["severity"], "high", + "the Day shift grade changed" + ); +} + +#[test] +fn a_private_reproduction_accepts_the_rubrics_informational_severity() { + let scenario = Scenario::new(); + let mut github = scenario.gh_state(); + github["private"] = json!(true); + scenario.write_gh_state(&github); + scenario.agent_does_in_session( + 1, + &audit_script(&json!([finding("minimal-impact")]).to_string()), + ); + scenario.agent_does_in_session(2, &reproduction_script("reproduced informational single")); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + let state = scenario.gh_state(); + let body = state["bodies"]["8"].as_str().unwrap(); + assert!(body.contains("Severity: informational"), "{body}"); + assert!(body.contains("bounded_fixture();"), "{body}"); + assert!(body.contains("Local command: bounded-fixture"), "{body}"); +} + +#[test] +fn a_finding_triaged_during_reproduction_is_left_unchanged() { + let scenario = Scenario::new(); + scenario.agent_does_in_session( + 1, + &audit_script(&json!([finding("input-size")]).to_string()), + ); + scenario.agent_does_in_session(2, &format!(r#" +printf '%s\n' '{{"state":"published","severity":"high"}}' | gh api --method PATCH repos/acme/widgets/security-advisories/GHSA-test-test-0001 --input - >/dev/null +{} +"#, reproduction_script("reproduced critical single"))); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(1), "{}", result.stderr); + assert!( + result.stderr.contains("triaged during its reproduction"), + "{}", + result.stderr + ); + let state = scenario.gh_state(); + let record = &state["advisories"][0]; + assert_eq!(record["state"], "published"); + assert_eq!(record["severity"], "high"); + assert!( + !record["description"] + .as_str() + .unwrap() + .contains("bounded_fixture();") + ); +} + +#[test] +fn an_informational_advisory_requires_a_day_shift_decision_without_an_invented_grade() { + let scenario = Scenario::new(); + scenario.agent_does_in_session( + 1, + &audit_script(&json!([finding("minimal-impact")]).to_string()), + ); + scenario.agent_does_in_session(2, &reproduction_script("reproduced informational single")); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(1), "{}", result.stderr); + assert!( + result + .stderr + .contains("the Day shift must decide its representation"), + "{}", + result.stderr + ); + let state = scenario.gh_state(); + let record = &state["advisories"][0]; + assert_eq!(record["severity"], Value::Null); + assert!( + !record["description"] + .as_str() + .unwrap() + .contains("") + ); + assert!( + scenario + .gh_calls_of("api", "--method") + .iter() + .all(|call| call[2] != "PATCH") + ); +} From b495ebcac82a8edb72c3d91c4a2c7f009279e46f Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Thu, 8 Oct 2026 07:03:35 -0400 Subject: [PATCH 3/4] Route fake GitHub updates by endpoint rather than body URLs (#542) --- tests/fakes/gh.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/fakes/gh.rs b/tests/fakes/gh.rs index cd82962..2d296fb 100644 --- a/tests/fakes/gh.rs +++ b/tests/fakes/gh.rs @@ -1071,6 +1071,7 @@ fn api(state: &mut Json, positional: &[String], flags: &Flags) { let path = &positional[0]; if let Some((repo, rest)) = repo_prefix(path) && let Some(n) = rest.strip_prefix("issues/") + && n.parse::().is_ok() { check_repo_is(state, Some(repo)); println!("{}", issue_fields(state, n, state.at("issues").at(n))); @@ -1938,14 +1939,19 @@ pub fn main(args: Vec) { } ["api", "graphql", rest @ ..] => graphql(&state, rest), ["api", "--method", "PATCH", rest @ ..] - if rest.iter().any(|word| word.contains("/issues/")) => + if rest.first().is_some_and(|path| { + repo_prefix(path).is_some_and(|(_, path)| { + path.strip_prefix("issues/") + .is_some_and(|number| number.parse::().is_ok()) + }) + }) => { finding_issue_patch(&mut state, rest) } ["api", "--method", "PATCH", rest @ ..] if !rest - .iter() - .any(|word| word.contains("/security-advisories")) => + .first() + .is_some_and(|path| path.contains("/security-advisories")) => { pr_patch(&mut state, rest) } From b9b14b29d06d9092aba81a663d392dc2137b1c5a Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Thu, 8 Oct 2026 08:14:17 -0400 Subject: [PATCH 4/4] Fix Security reproduction session purpose after base merge The base branch added a required session Purpose, but reproduction still called the old two-argument API. This caused E0061 in both CI test jobs and both release builds. Pass Purpose::Security and extend the Codex integration scenario to cover reproduction and its Resume with the Security launch policy. Validated with cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test, cargo nextest run (1541 passed), and the Linux musl dist artifact build plus its version command. --- src/security/reproduction.rs | 9 ++++++--- tests/security_run.rs | 31 +++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/security/reproduction.rs b/src/security/reproduction.rs index da23b1d..ed181a8 100644 --- a/src/security/reproduction.rs +++ b/src/security/reproduction.rs @@ -14,7 +14,7 @@ use crate::harness::Choice; use crate::issue::Repo; use crate::logs; use crate::prompt; -use crate::session::{Logs, Sessions}; +use crate::session::{Logs, Purpose, Sessions}; use crate::worktree::ReviewWorktree; #[derive(Clone, Copy, Serialize)] @@ -156,8 +156,11 @@ pub fn run( }; let logs = Logs::of_security_run(repo); Sessions::within(&logs, worktree.path(), harness, |sessions| { - let message = - sessions.run_to_final_message(&format!("security-reproduction-{number}"), &prompt)?; + let message = sessions.run_to_final_message( + Purpose::Security, + &format!("security-reproduction-{number}"), + &prompt, + )?; let message = message.as_deref().unwrap_or_default().trim_end(); let (notes, line) = message.rsplit_once('\n').unwrap_or(("", message)); let outcome = match line.split_whitespace().collect::>().as_slice() { diff --git a/tests/security_run.rs b/tests/security_run.rs index 55494ee..8514d72 100644 --- a/tests/security_run.rs +++ b/tests/security_run.rs @@ -612,15 +612,26 @@ fn codex_security_sessions_and_their_resumes_raise_the_thread_cap_and_request_fr 1, &format!( "{}\necho '{{\"type\":\"item.started\",\"item\":{{\"id\":\"pending\",\"type\":\"collab_tool_call\",\"tool\":\"spawn_agent\",\"prompt\":\"Verify finding\",\"status\":\"in_progress\"}}}}'\n", - audit_script("[]") + audit_script(&json!([finding("input-size")]).to_string()) ), ); scenario .agent_does("printf '%s\\n' 'Security audit: complete' > \"$FAKE_CLAUDE_FINAL_MESSAGE\"\n"); + scenario.agent_does_in_session( + 3, + &format!( + "{}\necho '{{\"type\":\"item.started\",\"item\":{{\"id\":\"pending\",\"type\":\"command_execution\",\"command\":\"bounded-fixture\",\"status\":\"in_progress\"}}}}'\n", + reproduction_script("reproduced high single") + ), + ); + scenario.agent_does_in_session( + 4, + "printf '%s\\n' 'The resumed local fixture confirmed the finding.' 'Security reproduction: reproduced high single' > \"$FAKE_CLAUDE_FINAL_MESSAGE\"\n", + ); let result = scenario.run(&["secure", "harness", "codex"]); assert_eq!(result.code, Some(0), "{}", result.stderr); let calls = scenario.codex_calls(); - assert_eq!(calls.len(), 2); + assert_eq!(calls.len(), 4); for call in &calls { let args = call["argv"].as_array().unwrap(); assert!( @@ -635,12 +646,16 @@ fn codex_security_sessions_and_their_resumes_raise_the_thread_cap_and_request_fr assert!(prompt.contains("fork_turns: \"none\""), "{prompt}"); assert!(prompt.contains("fresh sub-agents"), "{prompt}"); } - let resumed = calls[1]["argv"].as_array().unwrap(); - assert!( - resumed - .windows(2) - .any(|pair| pair == [json!("resume"), json!("fake-thread-1")]) - ); + for (index, thread) in [(1, "fake-thread-1"), (3, "fake-thread-3")] { + let resumed = calls[index]["argv"].as_array().unwrap(); + assert!( + resumed + .windows(2) + .any(|pair| pair == [json!("resume"), json!(thread)]), + "{resumed:?}" + ); + } + assert_eq!(scenario.gh_state()["advisories"][0]["severity"], "high"); } #[test]