Skip to content

Let Security runs audit while findings wait, keeping completed reproductions - #1161

Merged
JacobStephens2 merged 4 commits into
mainfrom
issue-1043
Oct 23, 2026
Merged

JacobStephens2 merged 4 commits into
mainfrom
issue-1043

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

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.

 run_through
   Ready issue?            -> skip
   failed Security fix?    -> skip
   fixing && next_fix?     -> fix before auditing
-  finding waiting for the Day shift? -> skip
   Base branch unchanged?  -> skip
   audit_and_record
     for each finding
       graded by the Day shift      -> keep its grade
+      untriaged, reproduction done -> keep outcome, list its severity,
+                                      offer fixing if reproduced and undecided
       untriaged, no outcome        -> reproduce (never run, failed, refused, interrupted)
  • Skipped::FindingWaiting and SecurityRecords::waiting_for_day_shift are removed. The Inbox's security_findings_waiting is unchanged.
  • Outcome::parse, placed next to its Display, reads a record's first Outcome: line. SecurityRecord::completed_reproduction exposes it.
  • GLOSSARY (Security run, Security finding), docs/guide.md (the gate list and "Security runs on a schedule"), the regenerated site/docs/index.html and ADR 0015's consequence (marked amended) no longer say a waiting finding holds off audits.
  • Only a record found again keeps its completed reproduction. A new record is always reproduced, because its text is the audit's own write-up and might quote an Outcome: line that no reproduction wrote (Security review, 3e78a6c). security::tests::a_new_finding_is_reproduced_even_when_its_write_up_quotes_an_outcome failed on 8f60583, passes on the merge base and passes after the fix. After the fix, cargo test --bin thirdshift passed 1244 tests and every --test target passed 1122.

Evidence

  • Before: waiting_findings_skip_quietly_once_and_triage_allows_the_next_audit and two other security_run tests 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_reproduction failed:
    left: [Reproduce("refuted"), UpdateSecurityRecord("refuted"), Reproduce("unfinished"), ...] right: [Reproduce("unfinished"), UpdateSecurityRecord("unfinished")].
    The reviewers' a_kept_reproduced_finding_left_unfixed_still_offers_fixing failed 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 includes another_pass_holds_the_security_run_and_its_skip_is_logged (another Pass wins over a Ready issue) and waiting_findings_hold_off_no_audit_and_keep_their_completed_reproductions (end to end: the second audit starts no reproduction session and writes nothing).
  • Local validation on 8f60583:
    • cargo test --bin thirdshift: 1243 passed, run with GIT_CONFIG_GLOBAL set to a plain config. The host's global tag.gpgsign = true breaks three unrelated worktree::synchronization_tests otherwise.
    • Every --test target: 1122 passed, 0 failed.
    • npm test --prefix site: passed on 224e527; the guide is unchanged since.
    • cargo clippy --all-targets -D warnings and cargo fmt: clean.
    • On 224e527, session::execution_tests::partial_stdout_is_reported_after_log_failure_with_a_blocked_prompt failed 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). Local main lags origin in the primary worktree. Both reviewers read every changed file. site/docs/index.html was excluded as generated output on both axes: cd site && npm run docs:check exited 0, and its source docs/guide.md was reviewed. Commit 8f60583, which addresses the findings, was not re-reviewed.

Unaddressed findings

Standards

  • GLOSSARY entries get longer (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.
  • Middle Man: Reproduction::severity forwards to Outcome::severity. Declined. src/github/advisories.rs:607 and :616 call it on a Reproduction, so removing it would add churn and change no behaviour.
  • Divergent Change: the new Ready-issue assertion in 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

  • c2: a re-found advisory reproduced by thirdshift is logged as "keeping the Day shift's grade". Declined; the Day shift must decide. update_security_record writes 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.

…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.
@JacobStephens2
JacobStephens2 merged commit 2354215 into main Oct 23, 2026
18 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-1043 branch October 23, 2026 17:05
@JacobStephens2 JacobStephens2 mentioned this pull request Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A waiting Security finding no longer holds off Security audits

1 participant