From c63b312f02c6d309285f7cd98344522e471df809 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Fri, 18 Sep 2026 12:37:30 +0000 Subject: [PATCH 1/3] Tell a delegating audit domain to block, not end its turn Run 35327271988's application-security domain fanned out to eight nested agents, issued its wait loop with run_in_background: true, said it was holding for four outstanding work streams, and ended its turn at 09:11:56. Ending the turn is how a subagent completes, so the orchestrator saw the task finish with no sentinel; all four streams returned by 09:19:29, inside a deadline that ran to 09:33:24, and every result was discarded. That is the orchestrator's own section 2 failure one level down, and a domain never reads the file carrying that rule. Put the rule in _preamble.md, which every domain does read, and widen the spec's audited version past the orchestrator. Also stop the merge placeholder and the Summary from naming a cause nobody observed: this run published "cut off at the 32-minute deadline" twenty-one minutes wrong. --- .github/audit/_preamble.md | 24 ++++++++++++++++++++++++ .github/audit/orchestrator.md | 9 ++++++--- docs/specs/security-audit.md | 4 ++-- docs/specs/security-audit.rationale.md | 2 ++ scripts/security-audit.test.mjs | 14 ++++++++++++++ scripts/spec-word-budgets.json | 2 +- 6 files changed, 49 insertions(+), 6 deletions(-) diff --git a/.github/audit/_preamble.md b/.github/audit/_preamble.md index ed643034a..b2c9c7f4b 100644 --- a/.github/audit/_preamble.md +++ b/.github/audit/_preamble.md @@ -61,6 +61,30 @@ if any `FAIL IF` in your scope is violated or any of your qualitative findings is BLOCKER. Otherwise INCONCLUSIVE if any check is `UNVERIFIABLE` or unfinished; PASS only when every check was determined. +**If you delegate, block for your delegates — never end your turn to wait.** +Subagents launch in the **background**: the Task tool returns an id, not a +report. Ending your turn ends *you*, and your caller reads that as your report +being finished — it merges the fragment as it stands and everything your +delegates write afterwards is lost. Block inside a Bash call instead, and never +with `run_in_background`, which returns an id immediately and blocks nothing. A +single Bash call is capped at ten minutes, so break the loop yourself under the +cap, issue it with `timeout: 600000`, and re-issue it for as long as your caller +is still waiting on you: + +```sh +CALL_END=$(( $(date +%s) + 540 )) +until ; do + [ "$(date +%s)" -ge "$CALL_END" ] && { echo "STILL WAITING"; break; } + sleep 10 +done +``` + +Never substitute a bare `sleep` — the harness blocks it. Run 35327271988's +`application-security` domain backgrounded exactly this loop, said it was +holding for four outstanding work streams, and ended its turn at 09:11:56. All +four finished by 09:19:29, fourteen minutes inside the deadline, and none of +their results reached the report. + Never print a secret value. `$AUDIT_PAT` is passed only as an unexpanded `GH_TOKEN=` prefix; do not echo it, do not run `printenv` or `set -x`, and do not paste the contents of any credential file into your report — report its diff --git a/.github/audit/orchestrator.md b/.github/audit/orchestrator.md index 850dbcc85..4a63691a9 100644 --- a/.github/audit/orchestrator.md +++ b/.github/audit/orchestrator.md @@ -142,7 +142,7 @@ emit() { echo "_No report — this domain produced no fragment._" else [ "$(sed -e '/^[[:space:]]*$/d' "$2" | tail -n1)" = "" ] || - echo "_Incomplete — this domain was still writing when the deadline passed. What follows is what it had recorded, not a finished report._" + echo "_Incomplete — this domain never closed its report. What follows is what it had recorded, not a finished report._" cat "$2" fi echo @@ -163,8 +163,11 @@ domain that never reported contradicts the issue carrying it, and publishes an overall `PASS` covering an unaudited domain. For a domain cut off mid-report, the Summary says that and gives its verdict -line. **Never state how much such a domain covered** — you did not watch it -work, and its fragment's own prose is about the moment it was written. Run +line. **Never state how much such a domain covered, nor why it stopped** — you +did not watch it work, and its fragment's own prose is about the moment it was +written. An unclosed fragment is not evidence of a timeout: run 35327271988's +was published as "cut off ... at the 32-minute deadline" when that domain had +in fact ended its own turn twenty-one minutes before the deadline. Run 35205193090 read "two of seven work streams had not reported" and published "completed only two of seven", turning five audited streams into five unaudited ones in the one paragraph a reader starts from. diff --git a/docs/specs/security-audit.md b/docs/specs/security-audit.md index 21d3b69c4..81a8f7ac3 100644 --- a/docs/specs/security-audit.md +++ b/docs/specs/security-audit.md @@ -50,7 +50,7 @@ Source of truth: `--agents` in `.github/workflows/security-audit.yaml`; `run_dom ## Orchestration -**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). +**Subagents launch in the background** — the Task tool returns an id, not a report — so an agent that ends its turn to await a completion notification is finished: the orchestrator ends the run, a delegating domain ships what it has (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. @@ -59,7 +59,7 @@ Source of truth: `--agents` in `.github/workflows/security-audit.yaml`; `run_dom - **FAIL IF** the orchestrator prompt stops requiring a non-turn-ending wait — a Bash `until` loop over the fragments' sentinels, **breaking on its own sub-cap under the ten-minute Bash cap** so every call ends by printing its answer, 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** a domain prompt lets findings be held for a write-up at the end, or the wait, the merge, or the verdict treats existence rather than the sentinel as a domain having reported (rationale). +- **FAIL IF** a domain prompt lets findings be held for a write-up at the end, lets a domain that delegates end its turn or background its wait loop, or the wait, the merge, or the verdict treats existence rather than the sentinel as a domain having reported (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`, `3. Merge`, and `4. The verdict` in `.github/audit/orchestrator.md`; the fragment contract in `.github/audit/_preamble.md`; the wait and merge blocks run as shipped in `scripts/security-audit.test.mjs`. diff --git a/docs/specs/security-audit.rationale.md b/docs/specs/security-audit.rationale.md index 62bd75350..6c0befd21 100644 --- a/docs/specs/security-audit.rationale.md +++ b/docs/specs/security-audit.rationale.md @@ -46,6 +46,8 @@ A missing fragment is indistinguishable, in the merged report, from a domain tha A domain that writes its fragment once, at the end, publishes nothing at all if it does not reach the end. Run 35205193090 is the case: `application-security` fanned out to fourteen nested subagents, every one of them returned (the last at 09:45:12), and the domain then produced no further output before the wait deadline at 09:53:15 — no completion notification for it ever arrived, unlike the sixteen other agents in the run. Seven work streams of finished audit were in its context and none of it was in its file, so the night's report carried `Qualitative findings: Pending.` and the ninth consecutive run held the release gate shut. Appending as findings are determined makes the same death cost only the synthesis. +A domain that delegates hits the orchestrator's own §2 failure one level down, and never reads the file that warns about it. Run 35327271988's `application-security` fanned out to eight nested agents, issued its wait loop with `run_in_background: true` — which returns an id and blocks nothing — reported "I'm holding for them before assembling the final fragment", and ended its turn at 09:11:56. Ending the turn is how a subagent completes, so the orchestrator saw the task finish with no sentinel written; the four outstanding work streams returned at 09:13:00, 09:13:04, 09:13:45 and 09:19:29, all inside a deadline that did not expire until 09:33:24, and every one of their results was discarded. The orchestrator then published the failure as "cut off ... at the 32-minute deadline" — a cause it could not have observed, twenty-one minutes wrong. + The sentinel exists because the same run showed that existence is the wrong predicate. The domain wrote a placeholder into its real fragment path at 09:40 to satisfy "write that file before you return"; `[ -s audit-application.md ]` went true, and the orchestrator — correctly unwilling to merge a placeholder — improvised `grep -q "Audit in progress"`, a predicate that worked only because it guessed wording no contract defined. With findings appended continuously the file is nonempty for most of the run, so the predicate has to be something the domain writes deliberately and last. The sentinel is checked in the reporting step too, not only in the orchestrator's wait. A domain rewrites its verdict line and then writes the sentinel, so a death between those two writes leaves `VERDICT: PASS` on line 1 of a report that stopped early — the one state where every other guard is satisfied and `PASS` closes the failure issue and opens the release gate. diff --git a/scripts/security-audit.test.mjs b/scripts/security-audit.test.mjs index 9eb805357..c37f5b9dd 100644 --- a/scripts/security-audit.test.mjs +++ b/scripts/security-audit.test.mjs @@ -189,6 +189,20 @@ test('the preamble tells domains to write the sentinel every reader waits for', assert.equal(written.length, 1, 'expected exactly one closing `printf` in the preamble'); assert.match(written[0][0], new RegExp(SENTINEL.replace(/[-[\]{}()*+?.,\\^$|#]/g, '\\$&'))); }); +// The delegation wait. The orchestrator carries this rule for itself in §2, +// but a domain is given `_preamble.md` plus its own file and never reads that +// one — so run 35327271988's `application-security` backgrounded its wait loop, +// ended its turn, and lost four work streams that finished before the deadline. +// The block is a template, not runnable, so the three parts that make it work +// are pinned as text: the rule, the backgrounding ban, and the Bash timeout +// without which every call is backgrounded anyway. +test('the preamble forbids a delegating domain from waiting by ending its turn', () => { + const preamble = readFileSync(join(repo, '.github/audit/_preamble.md'), 'utf8'); + assert.match(preamble, /never end your turn to wait/i); + assert.match(preamble, /never\s+with `run_in_background`/); + assert.match(preamble, /`timeout: 600000`/); +}); + const finishedFn = orchestrator.match(/^finished\(\) \{.*$/m)[0]; for (const [name, body, expected] of [ diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 5708c5379..5a4dd15a9 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -18,7 +18,7 @@ "docs/specs/relay.md": 10200, "docs/specs/remote-api.md": 4700, "docs/specs/remote-security-model.md": 4800, - "docs/specs/security-audit.md": 1900, + "docs/specs/security-audit.md": 1950, "docs/specs/security-ci.md": 2700, "docs/specs/security-hosted.md": 450, "docs/specs/security-local.md": 3150, From e5a15823793f33cb1d1aac28768367a911e19b72 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Fri, 18 Sep 2026 12:48:58 +0000 Subject: [PATCH 2/3] Address review: bound the delegate wait, and align the third placeholder The wait template had only a per-call break, so a domain following it literally re-issues until timeout-minutes cancels the job - the same lost synthesis from the other side of the wait. Mirror section 2: a deadline persisted to $RUNNER_TEMP, 25 minutes so it lands inside the orchestrator's 32 with room to close the fragment, and an answer line on every path so ALL FINISHED is distinguishable from a backgrounded call that printed nothing. Also name the until loop as sanctioned where the bare-sleep ban would otherwise read as forbidding it, stop claiming the transcript shows which loop body was backgrounded, and carry the placeholder fix into security-audit.yaml's own no-merged-report arm, which asserted the same unobservable cause. Both fixtures that pin its wording move with it; reverting either turns the suite red. --- .github/audit/_preamble.md | 32 ++++++++++++++++++++------- .github/workflows/security-audit.yaml | 2 +- scripts/security-audit.test.mjs | 4 ++-- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/.github/audit/_preamble.md b/.github/audit/_preamble.md index b2c9c7f4b..1dd83f4fc 100644 --- a/.github/audit/_preamble.md +++ b/.github/audit/_preamble.md @@ -68,22 +68,38 @@ being finished — it merges the fragment as it stands and everything your delegates write afterwards is lost. Block inside a Bash call instead, and never with `run_in_background`, which returns an id immediately and blocks nothing. A single Bash call is capped at ten minutes, so break the loop yourself under the -cap, issue it with `timeout: 600000`, and re-issue it for as long as your caller -is still waiting on you: +cap, issue it with `timeout: 600000`, and re-issue it under a bound of your +own: ```sh +# Persisted, because you re-issue this block in a fresh shell each time. +DEADLINE_FILE="$RUNNER_TEMP/delegate-deadline" +[ -f "$DEADLINE_FILE" ] || echo $(( $(date +%s) + 1500 )) > "$DEADLINE_FILE" +DEADLINE=$(cat "$DEADLINE_FILE") CALL_END=$(( $(date +%s) + 540 )) +ANSWER="ALL FINISHED" until ; do - [ "$(date +%s)" -ge "$CALL_END" ] && { echo "STILL WAITING"; break; } + NOW=$(date +%s) + [ "$NOW" -ge "$DEADLINE" ] && { ANSWER="DEADLINE"; break; } + [ "$NOW" -ge "$CALL_END" ] && { ANSWER="STILL WAITING"; break; } sleep 10 done +echo "$ANSWER" ``` -Never substitute a bare `sleep` — the harness blocks it. Run 35327271988's -`application-security` domain backgrounded exactly this loop, said it was -holding for four outstanding work streams, and ended its turn at 09:11:56. All -four finished by 09:19:29, fourteen minutes inside the deadline, and none of -their results reached the report. +**The call's last line is its answer.** Re-issue the block verbatim on `STILL +WAITING`; on `DEADLINE` or `ALL FINISHED` stop waiting and write up what you +have. The deadline is 25 minutes so it lands inside your caller's own bound +with room left to rewrite your verdict line and close the fragment; an empty +answer means the call was moved to the background, so re-issue it rather than +waiting on that task. + +Never substitute a bare `sleep` — the harness blocks it; the `until` loop above +is the sanctioned form. Run 35327271988's `application-security` domain +backgrounded its own wait loop, said it was holding for four outstanding work +streams, and ended its turn at 09:11:56. All four finished by 09:19:29, +fourteen minutes inside the deadline, and none of their results reached the +report. Never print a secret value. `$AUDIT_PAT` is passed only as an unexpanded `GH_TOKEN=` prefix; do not echo it, do not run `printenv` or `set -x`, and do diff --git a/.github/workflows/security-audit.yaml b/.github/workflows/security-audit.yaml index feb587b4b..6d900c4cd 100644 --- a/.github/workflows/security-audit.yaml +++ b/.github/workflows/security-audit.yaml @@ -549,7 +549,7 @@ jobs: # finished, and the no-verdict note above sends the reader # after a marker this arm never wrote. [ "$(sed -e '/^[[:space:]]*$/d' "$f" | tail -n1)" = "" ] || - echo "_Incomplete — this domain was still writing when the run ended. What follows is what it had recorded, not a finished report._" + echo "_Incomplete — this domain never closed its report. What follows is what it had recorded, not a finished report._" cat "$f" fi echo diff --git a/scripts/security-audit.test.mjs b/scripts/security-audit.test.mjs index c37f5b9dd..f8ac5b689 100644 --- a/scripts/security-audit.test.mjs +++ b/scripts/security-audit.test.mjs @@ -73,8 +73,8 @@ const cases = [ // read as a pass. Those are the inverse of the bug this arm fixes. { name: 'no merged report publishes the fragments, marking cut-off and absent domains', report: null, verdicts: ['PASS', 'PASS', null], unfinished: [1], expected: 'INCONCLUSIVE', notes: ['the merge never ran', '## audit-supply-chain.md', 'VERDICT: PASS', '## audit-application.md', '_No report — this domain produced no fragment._', - '## audit-ci-secrets.md\n\n_Incomplete — this domain was still writing'], - counts: { '_Incomplete — this domain was still writing': 1 } }, + '## audit-ci-secrets.md\n\n_Incomplete — this domain never closed its report'], + counts: { '_Incomplete — this domain never closed its report': 1 } }, ]; for (const scenario of cases) { test(`reporting: ${scenario.name}`, (t) => { From 3ad55ff4aad2cb879571a14724db368f246ace7b Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Fri, 18 Sep 2026 13:57:49 +0000 Subject: [PATCH 3/3] Address review: anchor the delegate deadline to the caller's own The template's deadline file was a fixed $RUNNER_TEMP/delegate-deadline, and all three domains are subagents of one headless session on one runner - so whichever delegates first would set the bound for the rest, and a domain starting later would break on DEADLINE in its first iteration and write up with nothing from its delegates. Name it after the fragment. Its 25 minutes was also anchored at the wrong clock: the caller's 32 runs from the orchestrator's first wait, the domain's from its own, which is after the spawn and the fan-out. In run 35327271988 that gap was ten minutes, so 25 would have landed three and a half minutes past the caller. Read the caller's deadline from the path section 2 already persists it to, less three minutes to close the fragment, and keep the 25 as the fallback for a caller that has not started waiting yet. The prose that described the old anchor moves with it. Both properties are pinned in security-audit.test.mjs. Mutation-checked: a fixed deadline name or a dropped caller read each turn the suite red (40 pass -> 39 pass / 1 fail). security-audit-local.sh now exports RUNNER_TEMP when Actions has not, since it feeds _preamble.md straight to claude -p. Without it the path is /delegate-deadline-...: unwritable for a normal user, which leaves the wait unbounded, and persistent across hand-runs for a root one, which is what section 2 rejects a repo-root fallback for. A fresh mktemp -d per run gives a hand-run what Actions gives. --- .github/audit/_preamble.md | 19 ++++++++++++++----- scripts/security-audit-local.sh | 5 +++++ scripts/security-audit.test.mjs | 12 +++++++++--- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/.github/audit/_preamble.md b/.github/audit/_preamble.md index 1dd83f4fc..5652a6eea 100644 --- a/.github/audit/_preamble.md +++ b/.github/audit/_preamble.md @@ -72,9 +72,17 @@ cap, issue it with `timeout: 600000`, and re-issue it under a bound of your own: ```sh -# Persisted, because you re-issue this block in a fresh shell each time. -DEADLINE_FILE="$RUNNER_TEMP/delegate-deadline" -[ -f "$DEADLINE_FILE" ] || echo $(( $(date +%s) + 1500 )) > "$DEADLINE_FILE" +# Persisted, because you re-issue this block in a fresh shell each time — and +# named after your own fragment, because every domain shares one $RUNNER_TEMP. +DEADLINE_FILE="$RUNNER_TEMP/delegate-deadline-" +if [ ! -f "$DEADLINE_FILE" ]; then + # Your caller's own deadline, less three minutes to close your fragment. It + # writes that file before it starts waiting; the 25 minutes here is only the + # fallback for the case where it has not. + CALLER=$(cat "$RUNNER_TEMP/audit-deadline" 2>/dev/null || true) + [ -n "$CALLER" ] || CALLER=$(( $(date +%s) + 1680 )) + echo $(( CALLER - 180 )) > "$DEADLINE_FILE" +fi DEADLINE=$(cat "$DEADLINE_FILE") CALL_END=$(( $(date +%s) + 540 )) ANSWER="ALL FINISHED" @@ -89,8 +97,9 @@ echo "$ANSWER" **The call's last line is its answer.** Re-issue the block verbatim on `STILL WAITING`; on `DEADLINE` or `ALL FINISHED` stop waiting and write up what you -have. The deadline is 25 minutes so it lands inside your caller's own bound -with room left to rewrite your verdict line and close the fragment; an empty +have. Your deadline is your caller's own, three minutes early, so you still +have room to rewrite your verdict line and close the fragment; the 25 minutes +is only the fallback for a caller that has not started waiting yet. An empty answer means the call was moved to the background, so re-issue it rather than waiting on that task. diff --git a/scripts/security-audit-local.sh b/scripts/security-audit-local.sh index 010f630ab..a86f0fdee 100755 --- a/scripts/security-audit-local.sh +++ b/scripts/security-audit-local.sh @@ -21,6 +21,11 @@ set -euo pipefail cd "$(dirname "$0")/.." AUDIT_DIR=.github/audit export GITHUB_REPOSITORY="${GITHUB_REPOSITORY:-diffplug/dormouse}" +# `_preamble.md`'s delegate wait persists its deadline under `$RUNNER_TEMP`, +# which only Actions sets. A fresh directory per run gives a hand-run the same +# property CI has — nothing from the last run survives into this one — where a +# repo-root fallback would hand the next run an expired deadline. +export RUNNER_TEMP="${RUNNER_TEMP:-$(mktemp -d)}" if ! command -v claude >/dev/null 2>&1; then echo "error: the \`claude\` CLI is not on PATH." >&2 diff --git a/scripts/security-audit.test.mjs b/scripts/security-audit.test.mjs index f8ac5b689..bc2d971c9 100644 --- a/scripts/security-audit.test.mjs +++ b/scripts/security-audit.test.mjs @@ -193,14 +193,20 @@ test('the preamble tells domains to write the sentinel every reader waits for', // but a domain is given `_preamble.md` plus its own file and never reads that // one — so run 35327271988's `application-security` backgrounded its wait loop, // ended its turn, and lost four work streams that finished before the deadline. -// The block is a template, not runnable, so the three parts that make it work -// are pinned as text: the rule, the backgrounding ban, and the Bash timeout -// without which every call is backgrounded anyway. +// The block is a template, not runnable, so the parts that make it work are +// pinned as text: the rule, the backgrounding ban, the Bash timeout without +// which every call is backgrounded anyway, and the two properties of its +// deadline. Every domain shares one `$RUNNER_TEMP`, so a fixed deadline file +// would make whichever domain delegates first set the bound for all of them; +// and the bound that has to hold is the caller's, which is already on disk at +// the path §2 persists it to. test('the preamble forbids a delegating domain from waiting by ending its turn', () => { const preamble = readFileSync(join(repo, '.github/audit/_preamble.md'), 'utf8'); assert.match(preamble, /never end your turn to wait/i); assert.match(preamble, /never\s+with `run_in_background`/); assert.match(preamble, /`timeout: 600000`/); + assert.match(preamble, /^DEADLINE_FILE="\$RUNNER_TEMP\/delegate-deadline-"$/m); + assert.match(preamble, /^\s*CALLER=\$\(cat "\$RUNNER_TEMP\/audit-deadline"/m); }); const finishedFn = orchestrator.match(/^finished\(\) \{.*$/m)[0];