diff --git a/README.md b/README.md index af6d639a..b54655e4 100644 --- a/README.md +++ b/README.md @@ -743,7 +743,7 @@ thirdshift secure base main harness claude thirdshift secure base main harness codex model gpt-6.1-sol effort high ``` -The command takes the Harness, Model and Effort words and the shared Pass words (`merge`, `no-merge`, `base-fix`, `no-base-fix`, `parallel`, `email` and `no-email`, with or without dashes). It reproduces and fixes nothing, dispatches no fix, and never commits, pushes, publishes or closes an advisory; the fix-related options are reserved for later Security run work. +The command takes the Harness, Model and Effort words and the shared Pass words (`merge`, `no-merge`, `base-fix`, `no-base-fix`, `parallel`, `email` and `no-email`, with or without dashes). After recording findings, it tries to reproduce each untriaged one in a fresh throwaway worktree at the recorded audited commit, and keeps the outcome, severity when reproduced, notes and test in the private record. It preserves findings already triaged by the Day shift. It dispatches no fix and never commits, pushes, publishes or closes an advisory; the fix-related options are reserved for later Security run work. `email`, optionally followed by an address, or `email.always` in the User config asks for one **Run notification** when the Security run ends, whether the audit succeeded, failed or was interrupted. `no-email` overrides the default. The notification says how the audit ended and lists each finding it recorded or matched to an existing private record: its severity when known, title and private link. It includes no finding write-up, trace or evidence, since it passes through Resend. Detailed failure causes stay in the local logs; the notification gives the audit's status and log paths. A recording failure still lists the records reached before it failed. A skipped Security run sends none. The usual address and Resend API key checks run before the skip checks or any work; a failed send is a warning and never changes the run's outcome. diff --git a/prompts/README.md b/prompts/README.md index 02283939..07907f3a 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 00000000..b5be193e --- /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, low or informational 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 b1d95e1c..94d9a73e 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, low or informational 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 7b17c812..741a1233 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 ce999b8f..93a9a995 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, Severity}; const SECURITY_FINDING: Label = Label::new( "security-finding", @@ -41,12 +42,95 @@ impl SecurityRecords { } } - pub fn remember(&mut self, record: Value) -> &Value { - let records = match self { - Self::Advisories(records) | Self::Issues(records) => records, + pub fn remember(&mut self, record: Value) { + match self { + Self::Advisories(records) | Self::Issues(records) => records.push(record), + } + } + + pub fn finding(&self, draft: &DraftAdvisory) -> Option<&Value> { + let (records, field) = match self { + Self::Advisories(records) => (records, "description"), + Self::Issues(records) => (records, "body"), }; - records.push(record); - records.last().expect("the new record was just added") + let marker = format!("Fingerprint: `{}`", draft.fingerprint); + records.iter().find(|record| { + record[field] + .as_str() + .is_some_and(|text| text.lines().any(|line| line == marker)) + }) + } + + 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(), + untriaged: value["state"] == "draft" && value["severity"].is_null(), + }, + 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(), + 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| NEEDS_TRIAGE.is_named(name)) + }) + }), + }, + }) + } +} + +/// A finding read from the private storage selected for this repository. +pub enum SecurityRecord { + 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, + } + } + + pub fn name(&self) -> String { + match self { + Self::Advisory { id, .. } => id.clone(), + Self::Issue { number, .. } => format!("issue #{number}"), + } } } @@ -64,21 +148,6 @@ pub struct DraftAdvisory { pub package: Package, } -impl DraftAdvisory { - pub fn recorded_in<'a>(&self, records: &'a SecurityRecords) -> Option<&'a Value> { - let marker = format!("Fingerprint: `{}`", self.fingerprint); - let (records, field) = match records { - SecurityRecords::Advisories(records) => (records, "description"), - SecurityRecords::Issues(records) => (records, "body"), - }; - records.iter().find(|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. @@ -145,18 +214,85 @@ 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, - "title": draft.summary, - "html_url": url.trim() - })) + let mut record = self.issue_view(&issue, "number,body,state,labels,title")?; + record["html_url"] = json!(url.trim()); + Ok(record) + } + } + } + + /// 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<()> { + 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}"), + SecurityRecords::Advisories(Vec::new()), + ), + SecurityRecord::Issue { number, .. } => ( + format!("repos/{repo}/issues/{number}"), + 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", "-"], + 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. diff --git a/src/pass.rs b/src/pass.rs index 0d94bb58..9aaec33e 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; @@ -110,6 +111,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; } @@ -226,6 +239,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 { @@ -264,13 +299,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. @@ -308,6 +344,10 @@ mod in_memory { AuditHistory, /// 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), @@ -670,6 +710,7 @@ mod in_memory { self.calls .push(Call::CreateAdvisory(finding.fingerprint.clone())); let advisory = json!({ + "ghsa_id": finding.fingerprint, "description": finding.description, "state": "draft", "summary": finding.summary, "html_url": format!("https://github.com/acme/widgets/security/advisories/{}", finding.fingerprint), @@ -679,6 +720,31 @@ mod in_memory { 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 9d8162d4..aa306b81 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, 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/prompts_page.rs b/src/prompts_page.rs index 60935ff1..5fdc9371 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 edae2597..c97c440f 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; @@ -10,7 +10,6 @@ use crate::asks::Flags; use crate::config::UserConfig; use crate::failed_run::FailedRun; use crate::github::ListedIssue; -use crate::github::SecurityRecords; use crate::harness::Choice; use crate::issue::Repo; use crate::launch::{self, AlreadyRunning, Launch, Start}; @@ -18,6 +17,7 @@ use crate::logs::{self, Pass, Work}; use crate::pass::{LaunchAndGitHub, Outside}; pub mod audit; +pub mod reproduction; pub enum Outcome { Skipped(Skipped), @@ -164,15 +164,10 @@ fn run_through(outside: &mut impl Outside, repo: &Repo, base: &str) -> Outcome { Ok(false) => {} Err(error) => return Outcome::Audited(error.into()), } - Outcome::Audited(audit_and_record(outside, repo, base, records)) + Outcome::Audited(audit_and_record(outside, repo, base)) } -fn audit_and_record( - outside: &mut impl Outside, - repo: &Repo, - base: &str, - mut known: SecurityRecords, -) -> Ended { +fn audit_and_record(outside: &mut impl Outside, repo: &Repo, base: &str) -> Ended { let mut findings = Vec::new(); let mut log = None; let recorded = (|| -> Result { @@ -189,17 +184,54 @@ fn audit_and_record( if audited.findings.is_empty() { 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 { - let record = if let Some(record) = finding.recorded_in(&known) { + let (record, metadata) = if let Some(value) = known.finding(&finding) { recorded.existing += 1; - record + (known.record(value)?, RecordedFinding::of_record(value)?) } else { - let record = outside.create_security_record(&known, &finding)?; + let value = outside.create_security_record(&known, &finding)?; + let record = known.record(&value)?; + let metadata = RecordedFinding::of_record(&value)?; + known.remember(value); recorded.created += 1; outside.step("recorded a Security finding privately".to_string()); - known.remember(record) + (record, metadata) }; - findings.push(RecordedFinding::of_record(record)?); + let url = metadata.url.clone(); + findings.push(metadata); + if record.untriaged() && seen.insert(finding.fingerprint) { + records.push((record, url)); + } else if !record.untriaged() { + outside.step(format!( + "keeping the Day shift's grade for {}", + record.name() + )); + } + } + for (index, (record, url)) 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)?; + for finding in findings.iter_mut().filter(|finding| finding.url == *url) { + finding.severity = reproduced + .severity() + .map(|severity| severity.name().to_string()); + } + outside.step(format!( + "Security reproduction {number}: {}", + reproduced.outcome + )); } Ok(recorded) })(); @@ -454,7 +486,7 @@ mod tests { let mut outside = InMemory::default() .audited(vec![draft("old"), draft("new")]) .advisories(vec![ - json!({"state":"closed", "description":"Fingerprint: `old`", "summary":"Candidate", "html_url":"https://github.com/acme/widgets/security/advisories/GHSA-old"}), + json!({"ghsa_id":"old", "state":"closed", "description":"Fingerprint: `old`", "summary":"Candidate", "html_url":"https://github.com/acme/widgets/security/advisories/GHSA-old"}), ]); let Outcome::Audited(Ended { outcome: Ok(recorded), @@ -475,5 +507,20 @@ 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("new".into()), + &Call::UpdateSecurityRecord("new".into()), + ] + ); } } diff --git a/src/security/reproduction.rs b/src/security/reproduction.rs new file mode 100644 index 00000000..ab31ee4a --- /dev/null +++ b/src/security/reproduction.rs @@ -0,0 +1,199 @@ +//! 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, Purpose, Sessions}; +use crate::worktree::ReviewWorktree; + +#[derive(Clone, Copy, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum Severity { + Critical, + High, + Medium, + Low, + Informational, +} + +impl Severity { + pub(super) fn name(self) -> &'static str { + match self { + Self::Critical => "critical", + Self::High => "high", + Self::Medium => "medium", + Self::Low => "low", + Self::Informational => "informational", + } + } +} + +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\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\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( + 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() { + ["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, + "informational" => Severity::Informational, + _ => 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 3ddb00c7..3f0dea0f 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 c01c43e6..80032ba9 100644 --- a/tests/fakes/gh.rs +++ b/tests/fakes/gh.rs @@ -977,6 +977,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=")); @@ -1052,8 +1062,35 @@ 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 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))); + return; + } if path == &format!("repos/{}", state.at("repo").str()) { println!( "{}", @@ -1917,10 +1954,20 @@ pub fn main(args: Vec) { release_view(&state, &positional, &flags); } ["api", "graphql", rest @ ..] => graphql(&state, rest), + ["api", "--method", "PATCH", rest @ ..] + 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) } diff --git a/tests/security_run.rs b/tests/security_run.rs index 0decaf0e..2041aa56 100644 --- a/tests/security_run.rs +++ b/tests/security_run.rs @@ -23,23 +23,6 @@ fn the_one_notification(resend: &ResendStandIn) -> (String, String) { ) } -#[test] -fn a_completed_security_audit_sends_one_notification_when_asked() { - let scenario = Scenario::new(); - scenario.agent_does(&audit_script("[]")); - 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(0), "{}", result.stderr); - let (subject, text) = the_one_notification(&resend); - assert_eq!( - subject, - "[thirdshift] acme/widgets Security run: findings recorded" - ); - assert_eq!(resend.requests()[0].body["to"], "me@example.com"); - assert!(text.contains("Built with claude"), "{text}"); -} - fn finding(fingerprint: &str) -> Value { json!({ "verdict": "needs_validation", "fingerprint": fingerprint, @@ -53,312 +36,278 @@ fn finding(fingerprint: &str) -> Value { } #[test] -fn the_notification_lists_each_recorded_finding_without_its_private_write_up() { +fn a_reproduction_scores_the_finding_and_keeps_its_test_in_the_private_record() { let scenario = Scenario::new(); - let mut github = scenario.gh_state(); - github["advisories"] = json!([{ - "state": "draft", "severity": "high", "summary": "Existing finding", - "html_url": "https://github.com/acme/widgets/security/advisories/GHSA-existing", - "description": "Fingerprint: `existing`\nPrivate existing write-up." - }]); - scenario.write_gh_state(&github); - scenario.agent_does(&audit_script( - &json!([finding("existing"), finding("new")]).to_string(), - )); - let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); - let result = - scenario.run_with_env(&["secure", "email", "me@example.com"], &resend_env(&resend)); + 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 (_, text) = the_one_notification(&resend); + 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!( - text.contains("Security audit recorded 1 new finding(s), 1 already recorded"), - "{text}" + body.contains("assert_eq!(bounded_fixture(), \"overflow\");"), + "{body}" ); - assert!(text.contains("high: Existing finding"), "{text}"); + assert!(body.contains("Fix size: single"), "{body}"); assert!( - text.contains("https://github.com/acme/widgets/security/advisories/GHSA-existing"), - "{text}" + result + .stderr + .contains("reproduction 1: reproduced high single"), + "{}", + result.stderr ); - assert!(text.contains("Unchecked input size"), "{text}"); - assert!( - text.contains("https://github.com/acme/widgets/security/advisories/GHSA-test-test-0002"), - "{text}" + 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()), ); - for private in [ + 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.", - "Private existing write-up.", - "Input reaches storage without a bound.", - "Private trace.", - "Private evidence.", - "No sandbox available.", - "Exercise a bounded fixture.", - "Fingerprint:", + "Severity: medium", + "Fix size: spec", + "Local command: bounded-fixture", + "bounded_fixture();", ] { - assert!( - !resend.requests()[0].body.to_string().contains(private), - "leaked {private}: {text}" - ); + 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 a_private_security_notification_lists_existing_and_new_issue_links_without_write_ups() { - for state in ["OPEN", "CLOSED"] { - let scenario = Scenario::new(); - let mut github = scenario.gh_state(); - github["private"] = json!(true); - github["issues"]["8"] = json!(state); - github["labels"]["8"] = json!(["security-finding"]); - github["titles"]["8"] = json!("Existing finding"); - github["bodies"]["8"] = json!("Fingerprint: `existing`\nPrivate existing write-up."); - scenario.write_gh_state(&github); - scenario.agent_does(&audit_script( - &json!([finding("existing"), finding("new")]).to_string(), - )); - 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(0), "{}", result.stderr); - let (_, text) = the_one_notification(&resend); - for metadata in [ - "Security audit recorded 1 new finding(s), 1 already recorded", - "Existing finding", - "https://github.com/acme/widgets/issues/8", - "Unchecked input size", - "https://github.com/acme/widgets/issues/9", - ] { - assert!(text.contains(metadata), "missing {metadata}: {text}"); - } - for private in [ - "Private candidate write-up.", - "Private existing write-up.", - "Input reaches storage without a bound.", - "Private trace.", - "Private evidence.", - "No sandbox available.", - "Exercise a bounded fixture.", - "Fingerprint:", +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!( - !resend.requests()[0].body.to_string().contains(private), - "leaked {private}: {text}" - ); + assert!(prompt.contains(required), "missing {required}: {prompt}"); } - assert_eq!(scenario.gh_calls_of("issue", "create").len(), 1); - assert_eq!(scenario.gh_state()["issues"].as_object().unwrap().len(), 3); + 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 security_notifications_follow_email_defaults_and_command_overrides() { - for (config, args, expected_to) in [ - ("", vec!["secure"], None), - ( - "[email]\nalways = true\nto = \"default@example.com\"\n", - vec!["secure"], - Some("default@example.com"), - ), - ( - "[email]\nto = \"default@example.com\"\n", - vec!["secure", "--email"], - Some("default@example.com"), - ), - ( - "[email]\nalways = true\nto = \"default@example.com\"\n", - vec!["secure", "no-email"], - None, - ), - ( - "[email]\nto = \"default@example.com\"\n", - vec!["secure", "--email", "override@example.com"], - Some("override@example.com"), - ), - ] { - let scenario = Scenario::new(); - scenario.user_config_is(config); - scenario.agent_does(&audit_script("[]")); - let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); - let result = scenario.run_with_env(&args, &resend_env(&resend)); - assert_eq!(result.code, Some(0), "{}", result.stderr); - match expected_to { - Some(to) => { - the_one_notification(&resend); - assert_eq!(resend.requests()[0].body["to"], to); +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(); + github["private"] = json!(private); + scenario.write_gh_state(&github); + if private { + 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": null, "state": "draft", + "summary": "Existing finding", "html_url": "https://github.com/acme/widgets/security/advisories/GHSA-existing" + }]); } - None => assert!(resend.requests().is_empty()), + scenario.agent_does_in_session( + 1, + &audit_script_with_records( + &scenario, + &json!([finding("first"), finding("second")]).to_string(), + &github, + ), + ); + 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 a_recording_failure_still_notifies_each_finding_already_recorded() { +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!( + "Fingerprint: `input-size`\nAudited commit: `{}`\nExisting private evidence.\n", + commit.trim() + ); let mut github = scenario.gh_state(); - github["advisory_create_fails_after"] = json!(1); - scenario.write_gh_state(&github); - let mut first = finding("first"); - first["title"] = json!("First recorded finding"); - let mut second = finding("second"); - second["title"] = json!("Unrecorded finding"); - scenario.agent_does(&audit_script(&json!([first, second]).to_string())); - 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_eq!( - scenario.gh_state()["advisories"].as_array().unwrap().len(), - 1 + github["advisories"] = json!([{ + "ghsa_id": "GHSA-existing", "description": original, "severity": null, "state": "draft", + "summary": "Existing finding", "html_url": "https://github.com/acme/widgets/security/advisories/GHSA-existing" + }]); + scenario.agent_does_in_session( + 1, + &audit_script_with_records( + &scenario, + &json!([finding("input-size")]).to_string(), + &github, + ), ); - let (subject, text) = the_one_notification(&resend); - assert!(subject.ends_with(": audit failed"), "{subject}"); - assert!(text.contains("First recorded finding"), "{text}"); + 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!( - text.contains("https://github.com/acme/widgets/security/advisories/GHSA-test-test-0001"), - "{text}" + result.stderr.contains("reproduction 1: not reproduced"), + "{}", + result.stderr ); - assert!(!text.contains("Unrecorded finding"), "{text}"); - assert!(!text.contains("Private candidate write-up."), "{text}"); -} - -#[test] -fn a_skipped_security_run_sends_no_notification_requested_by_word_or_config() { - for args in [vec!["secure"], vec!["secure", "email", "me@example.com"]] { - let scenario = Scenario::new(); - scenario.user_config_is("[email]\nalways = true\nto = \"me@example.com\"\n"); - scenario.issue_labelled(7, &["ready-for-agent"]); - let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); - let result = scenario.run_with_env(&args, &resend_env(&resend)); - assert_eq!(result.code, Some(0), "{}", result.stderr); - assert!( - result.stderr.contains("Ready issue #7"), - "{}", - result.stderr - ); - assert!(scenario.claude_calls().is_empty()); - assert!(resend.requests().is_empty()); - } -} - -#[test] -fn failed_security_audits_send_one_notification_with_a_safe_status() { - for (script, failing_call, cause) in [ - ( - audit_script("[]").replace("Security audit: complete", "Security audit: incomplete"), - None, - "ended incomplete", - ), - ( - audit_script("[{}]"), - None, - "validator validate-findings.cjs", - ), - ( - audit_script(&json!([finding("new")]).to_string()), - Some("api --method POST"), - "creating a draft repository security advisory failed", - ), - (audit_script("[]"), Some("issue list"), "gh issue list"), - ] { - let scenario = Scenario::new(); - scenario.agent_does(&script); - if let Some(call) = failing_call { - if call == "api --method POST" { - // The API consumes the request before rejecting it, avoiding - // a race between writing stdin and an early fake gh exit. - let mut github = scenario.gh_state(); - github["advisory_create_fails_after"] = json!(0); - scenario.write_gh_state(&github); - } else { - scenario.gh_fails(call); - } - } - 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); - let (subject, text) = the_one_notification(&resend); - assert_eq!( - subject, - "[thirdshift] acme/widgets Security run: audit failed" - ); - assert!(text.contains("Audit: audit failed"), "{text}"); - assert!( - result.stderr.contains(cause), - "expected {cause}: {}", - result.stderr - ); - assert!(!text.contains("Cause:"), "{text}"); - assert!(!text.contains("Private candidate write-up."), "{text}"); - } -} - -#[test] -fn an_interrupted_security_audit_sends_one_notification() { - let scenario = Scenario::new(); - scenario.agent_does("touch \"$HOME/../started\"\nsleep 60\n"); - let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); - let result = scenario.run_and_signal_with_env( - &["secure", "email", "me@example.com"], - &resend_env(&resend), - "started", - "TERM", - ); - assert_eq!(result.code, Some(1), "{}", result.stderr); - let (subject, text) = the_one_notification(&resend); - assert_eq!( - subject, - "[thirdshift] acme/widgets Security run: interrupted" - ); - assert!(text.contains("Audit: interrupted"), "{text}"); - assert!(!text.contains("Cause:"), "{text}"); -} - -#[test] -fn a_failed_send_keeps_the_security_runs_success_or_failure() { - for script in [audit_script("[]"), "exit 3".to_string()] { - let scenario = Scenario::new(); - scenario.agent_does(&script); - let without_email = scenario.run(&["secure"]); - scenario.origin_has_commit("main", "new-work.txt", "new work", "Move Base branch"); - let resend = ResendStandIn::replying(500, "upstream exploded"); - let result = - scenario.run_with_env(&["secure", "email", "me@example.com"], &resend_env(&resend)); - assert_eq!(result.code, without_email.code, "{}", result.stderr); - assert_eq!(result.stdout, without_email.stdout); - the_one_notification(&resend); - assert!( - result - .stderr - .contains("warning: could not send the Run notification"), - "{}", - result.stderr - ); - } -} - -#[test] -fn notification_checks_precede_security_skip_checks_and_work() { - for (args, env, cause) in [ - (vec!["secure", "email"], true, "no email address"), - ( - vec!["secure", "email", "me@example.com"], - false, - "no Resend API key", - ), - ] { - let scenario = Scenario::new(); - scenario.issue_labelled(7, &["ready-for-agent"]); - let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); - let env = if env { - resend_env(&resend).to_vec() - } else { - vec![("THIRDSHIFT_RESEND_URL", resend.url())] - }; - let result = scenario.run_with_env(&args, &env); - scenario.assert_rejected_before_any_work(&result, cause); - assert!(scenario.gh_calls().is_empty()); - assert!(resend.requests().is_empty()); - } } #[test] @@ -439,7 +388,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.")); } @@ -576,6 +528,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" @@ -586,6 +544,19 @@ printf '%s\n' 'Security audit: complete' > "$FAKE_CLAUDE_FINAL_MESSAGE" ) } +/// A finding recorded during the audit is first seen after the waiting gate. +fn audit_script_with_records(scenario: &Scenario, findings: &str, records: &Value) -> String { + fs::write( + scenario.path("audit-records.json"), + serde_json::to_vec(records).unwrap(), + ) + .unwrap(); + format!( + "cp \"$HOME/../audit-records.json\" \"$FAKE_GH_STATE\"\n{}", + audit_script(findings) + ) +} + fn assert_two_quiet_skips_send_no_email(scenario: &Scenario) { let resend = support::resend::ResendStandIn::replying(200, r#"{"id":"unused"}"#); for _ in 0..2 { @@ -753,7 +724,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(), + 2 + ); assert_eq!( scenario .entries("home/.thirdshift/logs/acme/widgets/audits") @@ -823,15 +797,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!( @@ -846,12 +831,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] @@ -1042,180 +1031,664 @@ fn missing_final_line_or_invalid_reports_fail_without_recording() { audit_script("[]").replace("Security audit: complete", "Security audit: incomplete"), "ended incomplete", ), - (audit_script("[{}]"), "validator validate-findings.cjs"), + (audit_script("[{}]"), "validator validate-findings.cjs"), + ( + audit_script("[]").replace( + "'[]' > \"$output/coverage-ledger.json\"", + "'[{}]' > \"$output/coverage-ledger.json\"", + ), + "validator validate-coverage-ledger.cjs", + ), + ] { + let scenario = Scenario::new(); + scenario.agent_does(&script); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(1), "{}", result.stderr); + assert!( + result.stderr.contains(cause), + "expected {cause}: {}", + result.stderr + ); + assert!(result.stderr.contains("session log:")); + assert!(scenario.gh_state()["advisories"].is_null()); + assert_eq!(scenario.entries("work"), vec![REPO]); + let log = + fs::read_to_string(scenario.path("home/.thirdshift/logs/acme/widgets/activity.log")) + .unwrap(); + assert!(log.contains("Security run started")); + assert!(log.contains("Security run ended: failed:"), "{log}"); + let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/commands/secure"); + let command_log = fs::read_to_string(scenario.path(&format!( + "home/.thirdshift/logs/acme/widgets/commands/secure/{}", + logs[0] + ))) + .unwrap(); + assert!(command_log.contains(cause)); + } +} + +#[test] +fn rejected_candidates_stay_in_the_report_without_a_private_advisory() { + let scenario = Scenario::new(); + let mut rejected = finding("refuted-claim"); + rejected.as_object_mut().unwrap().remove("blockers"); + rejected.as_object_mut().unwrap().remove("validation_plan"); + rejected["verdict"] = json!("rejected"); + rejected["reason"] = json!("The source already checks the bound."); + scenario.agent_does(&audit_script( + &json!([finding("input-size"), rejected]).to_string(), + )); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + assert_eq!( + scenario.gh_state()["advisories"].as_array().unwrap().len(), + 1 + ); + assert_eq!( + scenario.gh_state()["advisories"][0]["vulnerabilities"][0]["package"], + json!({"ecosystem":"other", "name":null}) + ); +} + +#[test] +fn an_incomplete_run_record_cannot_be_overridden_by_a_complete_final_line() { + let scenario = Scenario::new(); + scenario.agent_does(&audit_script("[]").replace( + "\"run_status\":\"complete\"", + "\"run_status\":\"incomplete\"", + )); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(1), "{}", result.stderr); + assert!( + result.stderr.contains("run-metadata.json"), + "{}", + result.stderr + ); + assert!(scenario.gh_state()["advisories"].is_null()); +} + +#[test] +fn audits_origins_base_without_changing_the_launch_directory() { + let scenario = Scenario::new(); + scenario.origin_has_commit("main", "SECURITY.md", "Trust model", "Document trust model"); + scenario.agent_does(&format!( + "{}\ntest -z \"$(git branch --show-current)\"\ntest -f SECURITY.md\n", + audit_script("[]") + )); + let head = scenario.launch_git(&["rev-parse", "HEAD"]); + fs::write(scenario.launch_dir().join("local.txt"), "local work").unwrap(); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + assert!( + result + .stderr + .contains("Security audit recorded 0 new finding(s)"), + "{}", + result.stderr + ); + assert_eq!(scenario.launch_git(&["rev-parse", "HEAD"]), head); + assert_eq!( + fs::read_to_string(scenario.launch_dir().join("local.txt")).unwrap(), + "local work" + ); + assert_eq!(scenario.entries("work"), vec![REPO]); + assert_eq!( + scenario.origin_git(&["log", "--format=%s", "main"]), + "Document trust model\nInitial commit\n" + ); + assert!(scenario.gh_state()["prs"].as_array().unwrap().is_empty()); + let prompt = scenario.first_prompt(); + for required in [ + "thirdshift-security-audit", + "full audit mode", + "quick", + "vendored", + "third-party", + "SECURITY.md", + "earlier runs", + "incomplete", + "Security audit: complete", + "You run headless", + "commits, pushes, opens and publishes nothing", + ] { + assert!(prompt.contains(required), "missing {required}: {prompt}"); + } + scenario.assert_every_session_found_the_factory_skills(); +} + +#[test] +fn security_advisories_preserve_python_and_go_manifest_identity() { + for (manifest, contents, package) in [ + ( + "pyproject.toml", + "[project]\nname = \"widgets\"\nversion = \"1.0.0\"\n", + json!({"ecosystem":"pip", "name":"widgets"}), + ), + ( + "go.mod", + "module example.com/acme/widgets\n\ngo 1.24\n", + json!({"ecosystem":"go", "name":"example.com/acme/widgets"}), + ), + ] { + let scenario = Scenario::new(); + scenario.origin_has_commit("main", manifest, contents, "Add manifest"); + scenario.agent_does(&audit_script(&json!([finding("input-size")]).to_string())); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + assert_eq!( + scenario.gh_state()["advisories"][0]["vulnerabilities"][0]["package"], + package, + "package identity was lost for {manifest}" + ); + } +} + +#[test] +fn security_audit_names_the_threat_model_under_docs() { + let scenario = Scenario::new(); + fs::create_dir_all(scenario.launch_dir().join("docs")).unwrap(); + fs::write( + scenario.launch_dir().join("docs/THREAT-MODEL.md"), + "# Threat model\nAll callers are authenticated; tenant isolation is the boundary.\n", + ) + .unwrap(); + scenario.launch_git(&["add", "docs/THREAT-MODEL.md"]); + scenario.launch_git(&["commit", "-m", "Document threat model"]); + scenario.launch_git(&["push", "origin", "main"]); + scenario.agent_does(&audit_script("[]")); + let result = scenario.run(&["secure"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + let prompt = scenario.first_prompt(); + assert!( + prompt.contains("Read the repository's threat-model document `docs/THREAT-MODEL.md`."), + "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", + "summary": "Existing finding", "html_url": "https://github.com/acme/widgets/security/advisories/GHSA-existing" + }]); + 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") + ); +} + +#[test] +fn a_completed_security_audit_sends_one_notification_when_asked() { + let scenario = Scenario::new(); + scenario.agent_does(&audit_script("[]")); + 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(0), "{}", result.stderr); + let (subject, text) = the_one_notification(&resend); + assert_eq!( + subject, + "[thirdshift] acme/widgets Security run: findings recorded" + ); + assert_eq!(resend.requests()[0].body["to"], "me@example.com"); + assert!(text.contains("Built with claude"), "{text}"); +} + +#[test] +fn the_notification_lists_each_recorded_finding_without_its_private_write_up() { + let scenario = Scenario::new(); + let mut github = scenario.gh_state(); + github["advisories"] = json!([{ + "ghsa_id": "GHSA-existing", + "state": "draft", "severity": "high", "summary": "Existing finding", + "html_url": "https://github.com/acme/widgets/security/advisories/GHSA-existing", + "description": "Fingerprint: `existing`\nPrivate existing write-up." + }]); + scenario.write_gh_state(&github); + scenario.agent_does(&audit_script( + &json!([finding("existing"), finding("new")]).to_string(), + )); + scenario.agent_does_in_session(2, &reproduction_script("reproduced medium single")); + 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(0), "{}", result.stderr); + let (_, text) = the_one_notification(&resend); + assert!( + text.contains("Security audit recorded 1 new finding(s), 1 already recorded"), + "{text}" + ); + assert!(text.contains("high: Existing finding"), "{text}"); + assert!( + text.contains("https://github.com/acme/widgets/security/advisories/GHSA-existing"), + "{text}" + ); + assert!(text.contains("Unchecked input size"), "{text}"); + assert!(text.contains("medium: Unchecked input size"), "{text}"); + assert_eq!(scenario.claude_calls().len(), 2); + assert!( + text.contains("https://github.com/acme/widgets/security/advisories/GHSA-test-test-0002"), + "{text}" + ); + for private in [ + "Private candidate write-up.", + "Private existing write-up.", + "Input reaches storage without a bound.", + "Private trace.", + "Private evidence.", + "No sandbox available.", + "Exercise a bounded fixture.", + "Fingerprint:", + "Local command: bounded-fixture", + "bounded_fixture();", + ] { + assert!( + !resend.requests()[0].body.to_string().contains(private), + "leaked {private}: {text}" + ); + } +} + +#[test] +fn a_private_security_notification_lists_existing_and_new_issue_links_without_write_ups() { + for state in ["OPEN", "CLOSED"] { + let scenario = Scenario::new(); + let mut github = scenario.gh_state(); + github["private"] = json!(true); + github["issues"]["8"] = json!(state); + github["labels"]["8"] = json!(["security-finding"]); + github["titles"]["8"] = json!("Existing finding"); + github["bodies"]["8"] = json!("Fingerprint: `existing`\nPrivate existing write-up."); + scenario.write_gh_state(&github); + scenario.agent_does(&audit_script( + &json!([finding("existing"), finding("new")]).to_string(), + )); + scenario.agent_does_in_session(2, &reproduction_script("reproduced high single")); + 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(0), "{}", result.stderr); + let (_, text) = the_one_notification(&resend); + for metadata in [ + "Security audit recorded 1 new finding(s), 1 already recorded", + "Existing finding", + "https://github.com/acme/widgets/issues/8", + "high: Unchecked input size", + "https://github.com/acme/widgets/issues/9", + ] { + assert!(text.contains(metadata), "missing {metadata}: {text}"); + } + for private in [ + "Private candidate write-up.", + "Private existing write-up.", + "Input reaches storage without a bound.", + "Private trace.", + "Private evidence.", + "No sandbox available.", + "Exercise a bounded fixture.", + "Fingerprint:", + "Local command: bounded-fixture", + "bounded_fixture();", + ] { + assert!( + !resend.requests()[0].body.to_string().contains(private), + "leaked {private}: {text}" + ); + } + assert_eq!(scenario.gh_calls_of("issue", "create").len(), 1); + assert_eq!(scenario.gh_state()["issues"].as_object().unwrap().len(), 3); + } +} + +#[test] +fn security_notifications_follow_email_defaults_and_command_overrides() { + for (config, args, expected_to) in [ + ("", vec!["secure"], None), + ( + "[email]\nalways = true\nto = \"default@example.com\"\n", + vec!["secure"], + Some("default@example.com"), + ), + ( + "[email]\nto = \"default@example.com\"\n", + vec!["secure", "--email"], + Some("default@example.com"), + ), + ( + "[email]\nalways = true\nto = \"default@example.com\"\n", + vec!["secure", "no-email"], + None, + ), + ( + "[email]\nto = \"default@example.com\"\n", + vec!["secure", "--email", "override@example.com"], + Some("override@example.com"), + ), + ] { + let scenario = Scenario::new(); + scenario.user_config_is(config); + scenario.agent_does(&audit_script("[]")); + let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); + let result = scenario.run_with_env(&args, &resend_env(&resend)); + assert_eq!(result.code, Some(0), "{}", result.stderr); + match expected_to { + Some(to) => { + the_one_notification(&resend); + assert_eq!(resend.requests()[0].body["to"], to); + } + None => assert!(resend.requests().is_empty()), + } + } +} + +#[test] +fn a_recording_failure_still_notifies_each_finding_already_recorded() { + let scenario = Scenario::new(); + let mut github = scenario.gh_state(); + github["advisory_create_fails_after"] = json!(1); + scenario.write_gh_state(&github); + let mut first = finding("first"); + first["title"] = json!("First recorded finding"); + let mut second = finding("second"); + second["title"] = json!("Unrecorded finding"); + scenario.agent_does(&audit_script(&json!([first, second]).to_string())); + 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_eq!( + scenario.gh_state()["advisories"].as_array().unwrap().len(), + 1 + ); + let (subject, text) = the_one_notification(&resend); + assert!(subject.ends_with(": audit failed"), "{subject}"); + assert!(text.contains("First recorded finding"), "{text}"); + assert!( + text.contains("https://github.com/acme/widgets/security/advisories/GHSA-test-test-0001"), + "{text}" + ); + assert!(!text.contains("Unrecorded finding"), "{text}"); + assert!(!text.contains("Private candidate write-up."), "{text}"); +} + +#[test] +fn a_skipped_security_run_sends_no_notification_requested_by_word_or_config() { + for args in [vec!["secure"], vec!["secure", "email", "me@example.com"]] { + let scenario = Scenario::new(); + scenario.user_config_is("[email]\nalways = true\nto = \"me@example.com\"\n"); + scenario.issue_labelled(7, &["ready-for-agent"]); + let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); + let result = scenario.run_with_env(&args, &resend_env(&resend)); + assert_eq!(result.code, Some(0), "{}", result.stderr); + assert!( + result.stderr.contains("Ready issue #7"), + "{}", + result.stderr + ); + assert!(scenario.claude_calls().is_empty()); + assert!(resend.requests().is_empty()); + } +} + +#[test] +fn failed_security_audits_send_one_notification_with_a_safe_status() { + for (script, failing_call, cause) in [ + ( + audit_script("[]").replace("Security audit: complete", "Security audit: incomplete"), + None, + "ended incomplete", + ), ( - audit_script("[]").replace( - "'[]' > \"$output/coverage-ledger.json\"", - "'[{}]' > \"$output/coverage-ledger.json\"", - ), - "validator validate-coverage-ledger.cjs", + audit_script("[{}]"), + None, + "validator validate-findings.cjs", + ), + ( + audit_script(&json!([finding("new")]).to_string()), + Some("api --method POST"), + "creating a draft repository security advisory failed", ), + (audit_script("[]"), Some("issue list"), "gh issue list"), ] { let scenario = Scenario::new(); scenario.agent_does(&script); - let result = scenario.run(&["secure"]); + if let Some(call) = failing_call { + if call == "api --method POST" { + // The API consumes the request before rejecting it, avoiding + // a race between writing stdin and an early fake gh exit. + let mut github = scenario.gh_state(); + github["advisory_create_fails_after"] = json!(0); + scenario.write_gh_state(&github); + } else { + scenario.gh_fails(call); + } + } + 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); + let (subject, text) = the_one_notification(&resend); + assert_eq!( + subject, + "[thirdshift] acme/widgets Security run: audit failed" + ); + assert!(text.contains("Audit: audit failed"), "{text}"); assert!( result.stderr.contains(cause), "expected {cause}: {}", result.stderr ); - assert!(result.stderr.contains("session log:")); - assert!(scenario.gh_state()["advisories"].is_null()); - assert_eq!(scenario.entries("work"), vec![REPO]); - let log = - fs::read_to_string(scenario.path("home/.thirdshift/logs/acme/widgets/activity.log")) - .unwrap(); - assert!(log.contains("Security run started")); - assert!(log.contains("Security run ended: failed:"), "{log}"); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/commands/secure"); - let command_log = fs::read_to_string(scenario.path(&format!( - "home/.thirdshift/logs/acme/widgets/commands/secure/{}", - logs[0] - ))) - .unwrap(); - assert!(command_log.contains(cause)); + assert!(!text.contains("Cause:"), "{text}"); + assert!(!text.contains("Private candidate write-up."), "{text}"); } } #[test] -fn rejected_candidates_stay_in_the_report_without_a_private_advisory() { +fn an_interrupted_security_audit_sends_one_notification() { let scenario = Scenario::new(); - let mut rejected = finding("refuted-claim"); - rejected.as_object_mut().unwrap().remove("blockers"); - rejected.as_object_mut().unwrap().remove("validation_plan"); - rejected["verdict"] = json!("rejected"); - rejected["reason"] = json!("The source already checks the bound."); - scenario.agent_does(&audit_script( - &json!([finding("input-size"), rejected]).to_string(), - )); - let result = scenario.run(&["secure"]); - assert_eq!(result.code, Some(0), "{}", result.stderr); - assert_eq!( - scenario.gh_state()["advisories"].as_array().unwrap().len(), - 1 - ); - assert_eq!( - scenario.gh_state()["advisories"][0]["vulnerabilities"][0]["package"], - json!({"ecosystem":"other", "name":null}) + scenario.agent_does("touch \"$HOME/../started\"\nsleep 60\n"); + let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); + let result = scenario.run_and_signal_with_env( + &["secure", "email", "me@example.com"], + &resend_env(&resend), + "started", + "TERM", ); -} - -#[test] -fn an_incomplete_run_record_cannot_be_overridden_by_a_complete_final_line() { - let scenario = Scenario::new(); - scenario.agent_does(&audit_script("[]").replace( - "\"run_status\":\"complete\"", - "\"run_status\":\"incomplete\"", - )); - let result = scenario.run(&["secure"]); assert_eq!(result.code, Some(1), "{}", result.stderr); - assert!( - result.stderr.contains("run-metadata.json"), - "{}", - result.stderr + let (subject, text) = the_one_notification(&resend); + assert_eq!( + subject, + "[thirdshift] acme/widgets Security run: interrupted" ); - assert!(scenario.gh_state()["advisories"].is_null()); + assert!(text.contains("Audit: interrupted"), "{text}"); + assert!(!text.contains("Cause:"), "{text}"); } #[test] -fn audits_origins_base_without_changing_the_launch_directory() { - let scenario = Scenario::new(); - scenario.origin_has_commit("main", "SECURITY.md", "Trust model", "Document trust model"); - scenario.agent_does(&format!( - "{}\ntest -z \"$(git branch --show-current)\"\ntest -f SECURITY.md\n", - audit_script("[]") - )); - let head = scenario.launch_git(&["rev-parse", "HEAD"]); - fs::write(scenario.launch_dir().join("local.txt"), "local work").unwrap(); - let result = scenario.run(&["secure"]); - assert_eq!(result.code, Some(0), "{}", result.stderr); - assert!( - result - .stderr - .contains("Security audit recorded 0 new finding(s)"), - "{}", - result.stderr - ); - assert_eq!(scenario.launch_git(&["rev-parse", "HEAD"]), head); - assert_eq!( - fs::read_to_string(scenario.launch_dir().join("local.txt")).unwrap(), - "local work" - ); - assert_eq!(scenario.entries("work"), vec![REPO]); - assert_eq!( - scenario.origin_git(&["log", "--format=%s", "main"]), - "Document trust model\nInitial commit\n" - ); - assert!(scenario.gh_state()["prs"].as_array().unwrap().is_empty()); - let prompt = scenario.first_prompt(); - for required in [ - "thirdshift-security-audit", - "full audit mode", - "quick", - "vendored", - "third-party", - "SECURITY.md", - "earlier runs", - "incomplete", - "Security audit: complete", - "You run headless", - "commits, pushes, opens and publishes nothing", - ] { - assert!(prompt.contains(required), "missing {required}: {prompt}"); +fn a_failed_send_keeps_the_security_runs_success_or_failure() { + for script in [audit_script("[]"), "exit 3".to_string()] { + let scenario = Scenario::new(); + scenario.agent_does(&script); + let without_email = scenario.run(&["secure"]); + scenario.origin_has_commit("main", "new-work.txt", "new work", "Move Base branch"); + let resend = ResendStandIn::replying(500, "upstream exploded"); + let result = + scenario.run_with_env(&["secure", "email", "me@example.com"], &resend_env(&resend)); + assert_eq!(result.code, without_email.code, "{}", result.stderr); + assert_eq!(result.stdout, without_email.stdout); + the_one_notification(&resend); + assert!( + result + .stderr + .contains("warning: could not send the Run notification"), + "{}", + result.stderr + ); } - scenario.assert_every_session_found_the_factory_skills(); } #[test] -fn security_advisories_preserve_python_and_go_manifest_identity() { - for (manifest, contents, package) in [ - ( - "pyproject.toml", - "[project]\nname = \"widgets\"\nversion = \"1.0.0\"\n", - json!({"ecosystem":"pip", "name":"widgets"}), - ), +fn notification_checks_precede_security_skip_checks_and_work() { + for (args, env, cause) in [ + (vec!["secure", "email"], true, "no email address"), ( - "go.mod", - "module example.com/acme/widgets\n\ngo 1.24\n", - json!({"ecosystem":"go", "name":"example.com/acme/widgets"}), + vec!["secure", "email", "me@example.com"], + false, + "no Resend API key", ), ] { let scenario = Scenario::new(); - scenario.origin_has_commit("main", manifest, contents, "Add manifest"); - scenario.agent_does(&audit_script(&json!([finding("input-size")]).to_string())); - let result = scenario.run(&["secure"]); - assert_eq!(result.code, Some(0), "{}", result.stderr); - assert_eq!( - scenario.gh_state()["advisories"][0]["vulnerabilities"][0]["package"], - package, - "package identity was lost for {manifest}" - ); + scenario.issue_labelled(7, &["ready-for-agent"]); + let resend = ResendStandIn::replying(200, r#"{"id":"1"}"#); + let env = if env { + resend_env(&resend).to_vec() + } else { + vec![("THIRDSHIFT_RESEND_URL", resend.url())] + }; + let result = scenario.run_with_env(&args, &env); + scenario.assert_rejected_before_any_work(&result, cause); + assert!(scenario.gh_calls().is_empty()); + assert!(resend.requests().is_empty()); } } -#[test] -fn security_audit_names_the_threat_model_under_docs() { - let scenario = Scenario::new(); - fs::create_dir_all(scenario.launch_dir().join("docs")).unwrap(); - fs::write( - scenario.launch_dir().join("docs/THREAT-MODEL.md"), - "# Threat model\nAll callers are authenticated; tenant isolation is the boundary.\n", - ) - .unwrap(); - scenario.launch_git(&["add", "docs/THREAT-MODEL.md"]); - scenario.launch_git(&["commit", "-m", "Document threat model"]); - scenario.launch_git(&["push", "origin", "main"]); - scenario.agent_does(&audit_script("[]")); - let result = scenario.run(&["secure"]); - assert_eq!(result.code, Some(0), "{}", result.stderr); - let prompt = scenario.first_prompt(); - assert!( - prompt.contains("Read the repository's threat-model document `docs/THREAT-MODEL.md`."), - "the existing threat-model document was not named: {prompt}" - ); -} - #[test] fn unchanged_base_is_remembered_separately_for_each_branch() { let scenario = Scenario::new(); @@ -1294,3 +1767,56 @@ fn security_notification_omits_private_background_trace() { let (_, text) = the_one_notification(&resend); assert!(!text.contains("Private exploit trace"), "{text}"); } + +#[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:", + ] { + assert!( + !resend.requests()[0].body.to_string().contains(evidence), + "leaked {evidence}: {text}" + ); + } + assert_eq!(scenario.claude_calls().len(), 3); + } +}