Repository navigation
Let Security runs audit while findings wait, keeping completed reproductions - #1161
Merged
Merged
Conversation
…uctions (#1043) A Security finding waiting for the Day shift no longer skips a Security run; the remaining gates keep their order. When an audit finds an untriaged record again whose reproduction already completed, it keeps that outcome and starts no reproduction session.
…er (#1043) A re-found record whose completed reproduction reproduced it is listed with its severity and, when nothing decided against fixing, offers fixing as a fresh reproduction does. Outcome::parse reads the line its Display writes, for the private record. ADR 0015 marks its amendment.
…come (#1043) An audit's write-up becomes a new record's text, so a quoted reproduction outcome in it must not stand in for a reproduction session.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A Security finding waiting for the Day shift no longer skips a Security run. When an audit finds an untriaged record again and that record's reproduction already completed, the run keeps the outcome and starts no new reproduction session.
Skipped::FindingWaitingandSecurityRecords::waiting_for_day_shiftare removed. The Inbox'ssecurity_findings_waitingis unchanged.Outcome::parse, placed next to itsDisplay, reads a record's firstOutcome:line.SecurityRecord::completed_reproductionexposes it.docs/guide.md(the gate list and "Security runs on a schedule"), the regeneratedsite/docs/index.htmland ADR 0015's consequence (marked amended) no longer say a waiting finding holds off audits.Outcome:line that no reproduction wrote (Security review, 3e78a6c).security::tests::a_new_finding_is_reproduced_even_when_its_write_up_quotes_an_outcomefailed on 8f60583, passes on the merge base and passes after the fix. After the fix,cargo test --bin thirdshiftpassed 1244 tests and every--testtarget passed 1122.Evidence
waiting_findings_skip_quietly_once_and_triage_allows_the_next_auditand two othersecurity_runtests asserted "Security run skipped: a Security finding is waiting for the Day shift". With the fix disabled,security::tests::a_refound_finding_keeps_its_completed_reproductionfailed:left: [Reproduce("refuted"), UpdateSecurityRecord("refuted"), Reproduce("unfinished"), ...] right: [Reproduce("unfinished"), UpdateSecurityRecord("unfinished")].The reviewers'
a_kept_reproduced_finding_left_unfixed_still_offers_fixingfailed on 224e527 with "a reproduced finding was left unfixed with no offer".After: all of these pass:
security::tests(12), which covers an audit while findings wait (advisory and issue storage, fixing on and off), gate order Ready → failed fix → unchanged Base, the kept reproduction and the fixing offer.tests/security_run.rs(109 passed). This includesanother_pass_holds_the_security_run_and_its_skip_is_logged(another Pass wins over a Ready issue) andwaiting_findings_hold_off_no_audit_and_keep_their_completed_reproductions(end to end: the second audit starts no reproduction session and writes nothing).cargo test --bin thirdshift: 1243 passed, run withGIT_CONFIG_GLOBALset to a plain config. The host's globaltag.gpgsign = truebreaks three unrelatedworktree::synchronization_testsotherwise.--testtarget: 1122 passed, 0 failed.npm test --prefix site: passed on 224e527; the guide is unchanged since.cargo clippy --all-targets -D warningsandcargo fmt: clean.session::execution_tests::partial_stdout_is_reported_after_log_failure_with_a_blocked_promptfailed once with a broken pipe in an untouched module. It passed in the 8f60583 run.Merge Danger
Door: two-way
Reverting restores the gate. No record format changes: only existing
Outcome:lines are read.Blast Radius: Security runs
Scheduled Security runs on repositories with untriaged findings start auditing again. That means more audit sessions, and untriaged findings can pile up, as the Day shift decided. Fixing still takes a reproduced finding before auditing.
Review coverage
Fixed point:
origin/main(3c53d52). Localmainlags origin in the primary worktree. Both reviewers read every changed file.site/docs/index.htmlwas excluded as generated output on both axes:cd site && npm run docs:checkexited 0, and its sourcedocs/guide.mdwas reviewed. Commit 8f60583, which addresses the findings, was not re-reviewed.Unaddressed findings
Standards
GLOSSARY-FORMAT.md: "one or two sentences max"). Declined. Acceptance criterion 4 requires Security run and Security finding to stop saying a waiting finding holds off audits, and the added sentences state the rule that replaces it. Both entries were far over the limit before this change.Reproduction::severityforwards toOutcome::severity. Declined.src/github/advisories.rs:607and:616call it on aReproduction, so removing it would add churn and change no behaviour.another_pass_holds_the_security_run_and_its_skip_is_logged. Declined. Acceptance criterion 2 says "The remaining gate order is tested: another Pass, Ready issue, …", and that assertion is the another-Pass-before-Ready part.Spec
update_security_recordwrites the reproduction's severity onto the draft advisory, and ADR 0015 counts giving it a severity as triage. Whether that should stay true is a Day shift call. Behaviour is right either way: no session runs and the severity is listed. Filed as Decide whether a draft advisory thirdshift reproduced still waits for the Day shift #1162.Closes #1043
Built with claude · claude-opus-5-5 · high
Security review outcome
No unaddressed introduced findings.