diff --git a/.github/audit/orchestrator.md b/.github/audit/orchestrator.md index 7d1ad1bd0..812f67295 100644 --- a/.github/audit/orchestrator.md +++ b/.github/audit/orchestrator.md @@ -42,7 +42,7 @@ So do not end your turn. Block inside a Bash call instead, waiting for the files the subagents write: ```sh -# 25 minutes, counted from the first time this loop runs — i.e. after +# 32 minutes, counted from the first time this loop runs — i.e. after # checkout, setup-node, and the install have already spent runner time. # Persisted to a file because the prose below tells you to re-issue this # block past the ten-minute Bash cap: a fresh shell would otherwise @@ -58,26 +58,44 @@ files the subagents write: # deadline without raising that one puts the runner's cancellation first # again, and this graceful path stops being reachable at all. DEADLINE_FILE="$RUNNER_TEMP/audit-deadline" -[ -f "$DEADLINE_FILE" ] || echo $(( $(date +%s) + 1500 )) > "$DEADLINE_FILE" +[ -f "$DEADLINE_FILE" ] || echo $(( $(date +%s) + 1920 )) > "$DEADLINE_FILE" DEADLINE=$(cat "$DEADLINE_FILE") +# Every call ends itself while it can still print. A Bash call that reaches +# the harness's ten-minute cap is moved to the background instead of +# returning, handing back no fragment list and no `DEADLINE` — nothing to +# decide the next step on, which is how run 34581574869 ended its turn four +# minutes short of the deadline and published no report at all. 540 leaves a +# minute of margin under the cap. +CALL_END=$(( $(date +%s) + 540 )) until [ -s audit-supply-chain.md ] && [ -s audit-ci-secrets.md ] && [ -s audit-application.md ]; do - [ "$(date +%s)" -ge "$DEADLINE" ] && { echo "DEADLINE"; break; } + NOW=$(date +%s) + [ "$NOW" -ge "$DEADLINE" ] && { echo "DEADLINE"; break; } + [ "$NOW" -ge "$CALL_END" ] && { echo "STILL WAITING"; break; } sleep 10 done -ls -la audit-*.md +ls -la audit-*.md 2>/dev/null || echo "no fragments yet" ``` A single Bash call is capped at ten minutes, and a domain can legitimately take longer than that. Issue this call with the maximum Bash timeout -(`timeout: 600000`) — the harness default is two minutes, and at that length -the 25 minutes take a dozen re-issues instead of three. A timed-out wait is -**not** a failure — re-issue the same loop until either every fragment exists -or the 25-minute deadline passes. Re-issuing the wait is the whole technique; -treating the first Bash timeout as "the subagents died" throws away work that -was still running. Re-issue the block **verbatim**, including the -`DEADLINE_FILE` lines: they read back the deadline the first call wrote, so the -25 minutes accumulate across re-issues and the `DEADLINE` branch fires on the -third call instead of never. +(`timeout: 600000`) — at the harness default of two minutes every call is +backgrounded before the loop's own 540-second break can print, so no answer +comes back at all. **What the call printed is what you act on, and there are +exactly two decisions:** + +- `STILL WAITING` — the nine minutes elapsed and a fragment is still missing. + Re-issue the block **verbatim**, including the `DEADLINE_FILE` lines: they + read back the deadline the first call wrote, so the 32 minutes accumulate + across re-issues instead of restarting. This is not a failure, and it is the + whole technique — treating it as "the subagents died" throws away work that + was still running. +- `DEADLINE`, or an `ls` listing all three fragments — stop waiting and go to + §3. Nothing more will arrive. + +If a call ever comes back saying it was moved to the background, it printed +neither answer: the block was edited or its `timeout` was short. Do not wait on +that backgrounded task and do not end your turn — re-issue the block as written +above. Never poll by ending your turn, and never substitute a bare `sleep` — the harness blocks it. The `until` loop above is the sanctioned form. @@ -133,3 +151,9 @@ running short: a partial report reaches a human through the INCONCLUSIVE issue, while a status file with no report behind it reaches nobody. Write `audit-status.txt` only once the rules above establish a verdict. Do not call `exit` — the workflow inspects the status file. + +**Never end your turn while `audit-report.md` does not exist.** Ending it is +what ends the run, so a run that stops there publishes nothing at all — not +even the domains that did report, whose fragments then reach a human only +through a 14-day artifact. If you have nothing left to wait on, merge §3 with +whatever fragments exist and let the placeholders say the rest. diff --git a/.github/workflows/security-audit.yaml b/.github/workflows/security-audit.yaml index 19c5f6592..28e420383 100644 --- a/.github/workflows/security-audit.yaml +++ b/.github/workflows/security-audit.yaml @@ -32,7 +32,7 @@ jobs: # and carries a pointer back here. AUDIT_FRAGMENTS: audit-supply-chain.md audit-ci-secrets.md audit-application.md # Must stay well above the orchestrator's own wait deadline in - # `.github/audit/orchestrator.md` (25 minutes, counted from the first + # `.github/audit/orchestrator.md` (32 minutes, counted from the first # time its wait loop runs — i.e. after checkout, setup-node, the # install, and the spawn — and persisted across the loop's # re-issues so it accumulates rather than restarting). @@ -85,6 +85,20 @@ jobs: # and have the prompt instruct Claude to prefix `gh api` # calls with `GH_TOKEN=$AUDIT_PAT`. AUDIT_PAT: ${{ secrets.AUDIT_PAT }} + # Process-wide: this raises the cap on every Bash call in the + # session, the three domain subagents' as well as the + # orchestrator's. What it is for is the orchestrator's wait loop, + # which breaks itself at 540s so a capped call still prints + # `STILL WAITING`; at the two-minute default that call is + # backgrounded before the break can print, leaving no line to act + # on — the run 34581574869 failure. The prompt asks for + # `timeout: 600000` per call, and this makes forgetting it + # harmless rather than fatal. The wider scope is accepted, not + # incidental: a domain's own long commands (`application-security` + # re-runs vitest and the repo lints) stop being backgrounded + # mid-run, and the cost is that a subagent that blocks with no + # self-break loses ten minutes rather than two. + BASH_DEFAULT_TIMEOUT_MS: "600000" with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} # release.yml dispatches this workflow with the default diff --git a/docs/specs/security-audit.md b/docs/specs/security-audit.md index 290ca4f8d..ea7e1513d 100644 --- a/docs/specs/security-audit.md +++ b/docs/specs/security-audit.md @@ -52,11 +52,12 @@ Source of truth: `--agents` in `.github/workflows/security-audit.yaml`; `run_dom **Subagents launch in the background** — the Task tool returns an id, not a report — so an orchestrator that ends its turn to await a completion notification ends the whole run: one headless turn, nothing resumes it (rationale). -- **The job's `timeout-minutes: 40` stays above the orchestrator's 25-minute wait deadline** (rationale). +- **The job's `timeout-minutes: 40` stays above the orchestrator's 32-minute wait deadline** (rationale). - **`--allowed-tools` enforces none of this**: it only auto-approves and removes nothing. `Task`/`Agent` are allowed on purpose; only `Workflow` is denied. - **Each subagent writes its own report fragment before returning its verdict** — `audit-supply-chain.md`, `audit-ci-secrets.md`, `audit-application.md` — and the orchestrator concatenates them rather than retyping. Fragments upload with the transcript, so an orchestrator that dies mid-merge still ships what the domains found. -- **FAIL IF** the orchestrator prompt stops requiring a non-turn-ending wait — a Bash `until` loop over the fragment files, re-issued past the ten-minute Bash cap, under a bounded 25-minute deadline **persisted to a file** (`$RUNNER_TEMP/audit-deadline`) rather than recomputed from `now` (rationale). +- **FAIL IF** the orchestrator prompt stops requiring a non-turn-ending wait — a Bash `until` loop over the fragment files, **breaking on its own sub-cap under the ten-minute Bash cap** so a capped call still prints `STILL WAITING`, re-issued under a bounded 32-minute deadline **persisted to a file** (`$RUNNER_TEMP/audit-deadline`) rather than recomputed from `now` (rationale). +- **FAIL IF** the prompt permits ending the turn without `audit-report.md` (rationale). - **FAIL IF** the orchestrator can report `PASS` while a subagent left no report fragment — nor `FAIL`, unless some domain actually returned one: the prompt writes no status file when a fragment is missing and no domain failed, routing an audit that ran out of time to INCONCLUSIVE. Both exit non-zero and hold the release gate shut (rationale). Source of truth: `2. Wait without ending your turn` and `4. The verdict` in `.github/audit/orchestrator.md`. diff --git a/docs/specs/security-audit.rationale.md b/docs/specs/security-audit.rationale.md index 1b797975f..cf65e6528 100644 --- a/docs/specs/security-audit.rationale.md +++ b/docs/specs/security-audit.rationale.md @@ -34,6 +34,10 @@ The fix is not to stop delegating. `--allowed-tools` only auto-approves and remo The deadline is persisted because one longer than the ten-minute Bash cap cannot fire inside a single call: a re-issued loop that recomputes it from `now` never reaches it, so the bound is written down but never binds, and only the runner's cancellation ends the wait. `RUNNER_TEMP` carries no fallback on purpose — a repo-root fallback would survive between hand-runs and hand an already-expired deadline to the next one. +A call that reaches the cap is *moved to the background*, not returned: it prints nothing back, so re-issuing becomes a judgement call rather than a step. Run 34457954349 happened to re-issue a third time and its deadline fell inside that call, so it merged and published two PASS domains; run 34581574869 spent one extra call checking the fragments, which shifted the phase enough that a third wait would have been needed, ended its turn instead, and published no report at all — the same two domains' PASS fragments survived only in the artifact. A loop that ends itself under the cap turns both nights into the same printed answer. + +The 25-minute deadline was raised to 32 after `application-security` failed to report inside it two nights running — the deadline expired on it on 2026-09-10, and on 2026-09-11 it was still sweeping when the run ended at 21 minutes — while roughly 13 of the job's 40 minutes went unused on both nights. + At `timeout-minutes: 20` the runner cancelled the job before the 25-minute deadline could fire, so the graceful "give up and report what the domains found" path was unreachable and every overrun landed as INCONCLUSIVE. The 40-minute slack also covers the merge, verdict, redact, upload, and reporting steps after the wait. A missing fragment is indistinguishable, in the merged report, from a domain that found nothing, and only one of those is safe to publish a release on.