Close the audit's findings on itself, and run application-security on Opus

[nedtwigg]

Follow-up to #445, driven by #447 and by running scripts/security-audit-local.sh repeatedly until all three domains came back clean. Supersedes #449, #450, and #451 — see the last section.

The finding that started it

default_workflow_permissions was write, so tend-mention.yaml's verify job — the only job in the repo declaring no permissions: block — got a token with nine write scopes, in a job triggerable by any issue comment. Flipped to read, with can_approve_pull_request_reviews to false. It costs tend nothing: that job's steps authenticate with TEND_BOT_TOKEN via a step-level env:, which is upstream's own design, so the ambient token was never used.

Also enabled secret scanning (history backfill: 0 alerts), push protection, and Dependabot alerts — which immediately surfaced two moderate Rust advisories. Renovate gets a vulnerabilityAlerts block so a CVE doesn't wait for the Monday window.

tend 0.1.19 closes the AGENTS.md gap

Upstream released the #1005 fix on 2026-08-26, hours before this branch. pin-instruction-paths.sh now globs **/AGENTS.md at any depth where 0.1.18's flat list named it nowhere — which mattered here specifically, because this repo keeps its instructions in AGENTS.md behind a one-line CLAUDE.md pointer, so the old revert restored a pointer and left the body attacker-controlled on a fork PR. Regenerated onto 0.1.19; the FAIL IF now pins a version floor, since the control lives in upstream code and a pin moving backwards reopens it invisibly.

Product fixes the audit found

Session snapshots were world-readable. write_session_to created sessions/ and its files under the bare umask — 0644 in a 0755 directory. Those blobs carry terminal transcripts: whatever your shells printed, which is a superset of every other secret this document tracks. Now 0700/0600, applied to the temp file before any bytes land, since the atomic rename preserves the mode.

Remote resize was clamped only at the bottom. terminal.resize carried a peer-supplied number to term.resize in the owning webview, and xterm allocates rows × cols. One frame asking for a million by a million wedges every terminal in that window. Bounded in the shared helper.

Pending pairings were unbounded in two places the ceremony's cap didn't cover. Each pair frame also allocates in RemoteHost.#clients and the service's mirrored queue, keyed by a relay-chosen clientId, removed only by a client-gone a hostile relay never sends. Measured: 5000 frames retained ~16 MB, with quadratic bridge traffic. What makes it worth fixing is who reaches it — a synced or stolen passkey is documented as buying "the ability to ask", and asking was a denial of service.

requireUserVerification was unreachable on both sides while two comments asserted the halves must mirror or they "silently disagree on what a valid assertion is". Wired through rather than deleted, since the server-side enforcement exists and is tested: DORMOUSE_REQUIRE_USER_VERIFICATION=true, mirrored into HostEnrollResponse and the Host's ConnectionPolicy.

The auditor auditing itself

application-security runs on Opus, the other two on Sonnet. The mechanical domains do fine on Sonnet — they caught the window BLOCKERs and the stale hono disclosure — but the findings needing real reasoning all came from appsec: tracing a relay-minted clientId to keystroke injection, working out that an 8-character fingerprint carried ~40 bits because a P-256 point's leading byte is constant. Verified the override takes effect by pinning a session to Sonnet and watching the subagent return as Opus.

A domain's verdict is now binding. The reporting step checked fragments existed but never read them, so a subagent could return FAIL and a mis-merged PASS would sail through — worse than a missing fragment, since PASS closes the failure issue and opens the release gate. Line 1 of each fragment is the literal verdict; the step downgrades on dissent.

workflow-audit.yaml's window reaches every consumer. I widened the git log pathspec without widening own_changes or the classifier refusals, which was worse than not widening at all: commits touching only the new paths were selected, produced no attributable files, and were silently dropped while SECURITY.md claimed coverage. One WINDOW array now, with the classifiers' half derived from it.

Things this branch got wrong first

Recording these because they are the argument for the audit existing:

• The Renovate cooldown did the opposite of what I documented. Omitting minimumReleaseAge from vulnerabilityAlerts doesn't inherit it — Renovate's default for that block is null, force-applied. Now explicit.
• "Both signing secrets are env-only" was wrong twice: there are three, and APPLE_SIGN_PASS sits on notarytool's command line for up to 30 minutes.
• "Server compromise — no Host access" overstated the bound. It can't create an authorized Client, but #onMsg gates on state.established for a relay-minted clientId with no per-frame auth, so it can act through one.
• My claim that the PAT never enters the appsec context was false — it's a step-level env: every subagent inherits.
• The local runner pinned only one side of the model split, so on a machine defaulting to opus[1m] the relation was inverted. Every local run before that fix was more capable than the nightly.

Supersedes #449, #450, #451

All three are dormouse-bot PRs on the same problems. Two found things I hadn't, and their substance is folded in here — verified present file by file:

• docs(security): name every dotfile directory in the audit scopes, and dormouse-lib as a root #449 — dormouse-lib is a disclosure root for a specific reason: vscode-ext declares only node-pty and ws and reaches the lib by relative import from 15 files, so its dependency walk never arrives there. My prose called it "a workspace edge", which was wrong. Also its website/-as-subtraction split and the explicit dotfile-directory ownership.
• fix(workflow-audit): report commits that only touch .github/audit/ #450 — found the own_changes BLOCKER independently, with an equivalent fix. Its argument that SECURITY.md does not belong in the window was right and I reversed my own addition: that job watches code executing from an unreviewed branch, and a FAIL IF is inert until merged to main, which is admin-gated — so it would add nothing over PR review while reporting on nearly every security PR.
• docs(security): name .github/audit/ in the audit-visibility window #451 — the Audit visibility paragraph named two paths where the window had four. I'd missed that too.

Not done, deliberately

• hostToken travels in the /ws/host query string. The "browsers can't set WS headers" rationale is true of /ws/client, not of /ws/host. Closing it means an Authorization header, a createWebSocket signature that carries one, and relay support across both hosts — untestable here, and rated INFO since TLS covers it in transit. Documented with the remedy named.
• jsign --storepass and notarytool --password stay on argv; neither tool offers an alternative. Documented as accepted with the reasoning.
• The two workflow-audit window evasions (committer-date --since, branch deleted before the fetch) remain open and documented.

Full suite green; cargo check clean against the pinned self-contained Node.

[cloudflare-workers-and-pages]

Deploying mouseterm with    Cloudflare Pages

Latest commit:	e040281
Status:	✅  Deploy successful!
Preview URL:	https://793dbc7e.mouseterm.pages.dev
Branch Preview URL:	https://audit-scope-catchall.mouseterm.pages.dev

View logs

[dormouse-bot]

Read the diff and the merged tree, and ran a second structured pass over the code portion. Four findings, one of which narrows a claim SECURITY.md now makes.

The headline: the pairing bound closes the payload, not the map. #evictOldestPairingIfFull denies the oldest pending, but #denyPairing only does state.pending = undefined — it never removes the #clients entry. So #clients still grows by one entry per distinct pair frame, keyed by a clientId that #onFrame type-checks (typeof (frame as { clientId?: unknown }).clientId !== 'string') and never length-bounds. PairingRequest fields are capped at PAIRING_FIELD_LIMIT = 1024, so the thing that just got bounded is the bounded half; the residual key string is the unbounded one. The new test asserts denial count and savedRecords, neither of which observes map size, so it passes either way. Inline suggestion below.

That matters for the prose too: the new FAIL IF reads "the only thing that removes one is a client-gone a hostile relay simply never sends. Both are capped at MAX_PENDING_PAIRINGS" — which is true of the pending queue and not of the client map the same sentence describes.

The rest is smaller: a vacuous branch in the new verdict-dissent guard, a spec sentence that overstates what two of the three evictions do, and some mangled indentation in app.ts.

Everything else I traced held up. clampTerminalDimension is genuinely on the peer-reachable path (peer-surfaces.ts nextCols/nextRows feeding term.resize, and both remote-api.ts sites), MAX_PENDING_PAIRINGS re-exports through export * from './security/pairing.js', the requireUserVerification wiring reaches passkey.ts's if (expected.requireUserVerification && !userVerified) on both sides, restrict_to_owner on the temp file before write_all is the right ordering for a rename that preserves mode, and the new wire.test.mjs is picked up by node --test test/*.test.mjs.

[dormouse-bot]

The review I left above stopped at the code portion. Two things in workflow-audit.yaml it didn't reach — the first introduced by this PR.

The issue-body header is the last hand-maintained window enumeration, and the literal it carries is wrong. #L366 now names five paths — `.github/workflows/`, `.config/tend.yaml`, `.github/audit/`, `.vscode/`, `SECURITY.md` — where WINDOW at #L136 is four. The fifth is the one this same PR spends ten lines excluding on purpose: "SECURITY.md is deliberately NOT in the window" (#L103-L112) — the same call you described on #451 as #450's, which #452 agrees with. That string is the audit's outward face, the first line of the issue a human opens, so it would claim coverage the job does not have. That is the failure mode #L123-L128 is already watching for ("the FAIL IF then claims coverage that does not exist"), one level further out than that comment reaches — it governs the consumers that select commits, and this one only describes them.

Every other consumer is derived: WINDOW_NON_WORKFLOW is "${WINDOW[@]:1}", and git log, own_changes, and both classifier refusals all expand the array. Deriving the header too closes the drift rather than re-fixing the enumeration:

WINDOW_MD=$(printf '`%s`, ' "${WINDOW[@]}"); WINDOW_MD=${WINDOW_MD%, }
echo "$COUNT unexplained commit(s) in the audit window ($WINDOW_MD) since \`$SINCE\`."

Everything from WINDOW= to the gh issue create is one run: block (the step opens at #L76 and nothing else starts before EOF), so WINDOW is in scope there. Ran it against the current array — output is today's line minus SECURITY.md.

Second, smaller, and not yours: the regen arm still opens ".config/tend.yaml is outside this audit's window" (#L241) while #L114 says it is in. It means "outside the reproducibility check's scope" — true, and the rest of the comment makes exactly that point — but "this audit's window" is now the name of a variable twenty lines up. Unchanged by this PR; raising it here only because this is the PR that gave the window one definition.

Verification

• The header is this PR's change, not pre-existing. git diff origin/main...pr452 -- .github/workflows/workflow-audit.yaml:

  -            echo "$COUNT unexplained commit(s) touching \`.github/workflows/\` or \`.config/tend.yaml\` since \`$SINCE\`."
  +            echo "$COUNT unexplained commit(s) in the audit window (\`.github/workflows/\`, \`.config/tend.yaml\`, \`.github/audit/\`, \`.vscode/\`, \`SECURITY.md\`) since \`$SINCE\`."

  Two paths → five; four of the five match WINDOW, and SECURITY.md is new to both.

• SECURITY.md appears at six lines in the file at f6530bcc. Five are prose or a cross-reference; #L366 is the only one that puts it in a path list, and no assignment adds it to WINDOW. So the string is the whole of the discrepancy — selection behavior is correct as written.

• The regen-arm sentence is verbatim from main at 59b0cedf#L198; it is outside this PR's diff for that file.

• Snippet output, run with the array as it stands: 7 unexplained commit(s) in the audit window (`.github/workflows/`, `.config/tend.yaml`, `.github/audit/`, `.vscode/`) since `2026-08-20`.

[dormouse-bot]

All four findings from the last round are genuinely closed — I re-traced each one rather than taking the replies at face value. The eviction now removes the record under the right guard, the case handles all three verdict states, the spec sentence attributes the denial where it happens, and app.ts is re-indented. MAX_CLIENT_ID_LENGTH = 256 is well-chosen: relay.ts mints toBase64Url(randomBytes(16)) (22 chars), so there is an order of magnitude of headroom, and the check sits ahead of the switch so it covers every frame type, not just pair. trackedClientCount is a real improvement over asserting on denial counts.

Three things on the new commit, one of which is mine to correct.

The verdictless branch lands on issue prose that describes a different failure. My suggestion last round said plain MISSING "lands on the generic Audit produced no verdict branch, which fits." That was wrong, and the mistake is worth naming precisely because the code took it: the issue-body branch is selected on MISSING_FRAGMENTS being non-empty, not on the cause of the MISSING. The new arm deliberately doesn't populate MISSING_FRAGMENTS — correctly, for the reason you gave — so a verdictless fragment falls through to the else, whose first sentence is "The audit step exited without writing audit-status.txt." In this path audit-status.txt said PASS; that is the precondition of the whole block. Details and a reproduction inline.

A doc comment got orphaned. trackedClientCount was inserted between #evictOldestPairingIfFull's JSDoc and the method, so the eviction block now documents the getter and the method is undocumented. Purely mechanical; inline.

The map is bounded against pair, not bounded. #evictOldestPairingIfFull runs only from #onPair and only counts entries whose pending is set. #onConnect and #onConnect2 both call #resetAuthorization, which calls #clientState — creating a #clients entry unconditionally, with no cap and nothing that evicts it. So a relay sending connect frames under distinct clientIds still grows the map one entry per frame until the socket drops (#dropTransientState clears it). Each entry is now much cheaper — MAX_CLIENT_ID_LENGTH caps the key and there is no PairingRequest attached — and this is not a regression; the path was there before. What's new is the claim: the test comment says "The map itself is bounded" and the spec lists the client map among "all three are capped". I'd narrow both rather than widen the cap here — bounding the connect path is a separate design question (evicting an entry that may be established is a different act from denying a pending pairing), and it doesn't belong in a PR scoped to the pairing queue.

Also, MAX_CLIENT_ID_LENGTH is a new acceptance rule on the hostile-relay frame path, and SECURITY.md now leans on it ("which is why clientId is length-bounded at the frame boundary too"), but docs/specs/remote-security-model.md doesn't record it — neither in the "Pending requests are bounded" paragraph nor in that section's Source of truth: list. Per AGENTS.md ("When updating code covered by a spec, update the spec to match") that's a one-sentence addition.

Happy to push a commit with all of this — the workflow fix is the only one that isn't a one-liner, and it needs the third branch in the issue body, which is outside the diff so I can't suggest it inline.

Verification

• Verdictless prose. Ran the new case loop verbatim against three fragments where frag-a.md opens with a heading and the other two open with VERDICT: PASS, then the STATUS = MISSING branch selection:

  ::warning::frag-a.md has no VERDICT line; the dissent check cannot read it.
  STATUS=MISSING  MISSING_FRAGMENTS=''  DISSENTING=''
  --- issue body ---
  Audit produced no verdict at DATE.
  The audit step exited without writing `audit-status.txt`.
  It did leave a partial audit-report.md ... A report without a verdict is the expected
  shape of an audit that ran short ... Start at the markers: UNVERIFIABLE / _No report ..._
  

  Three claims, all false here: the status file was written and said PASS; the report did reach a verdict; and the UNVERIFIABLE / _No report …_ markers the reader is sent to won't be there, because every domain reported. frag-a.md appears only in the ::warning:: — the issue never names it.

• The comment block above that branch anticipates exactly this. "A downgraded PASS means the named domains are the whole story — the rest reported, and the 'ran short, no verdict' prose below would be wrong about a run that did reach one." The new case is a downgraded PASS that names no domains, so it reaches the branch that sentence rules out.

• Orphaned JSDoc. remote-host.ts reads /** Drop the oldest pending pairing … */ at the block opening Drop the oldest pending pairing when the queue is full, then immediately /** How many clients this Host is tracking … */, then get trackedClientCount(). Two stacked blocks: the second binds to the getter, the first binds to nothing, and #evictOldestPairingIfFull follows with no doc.

• connect allocation path. #onConnect → #resetAuthorization(clientId) → #clientState(clientId), whose body is let state = this.#clients.get(clientId); if (!state) { state = { established: false, authGeneration: 0 }; this.#clients.set(clientId, state); }. #evictOldestPairingIfFull's counter is for (const state of this.#clients.values()) if (state.pending) pendingCount++, so entries created this way are invisible to it and the while (pendingCount >= MAX_PENDING_PAIRINGS) loop never runs on a pure-connect workload.

• The new assertion is tight, not vacuous. Traced the steady state: at frame 8 the evict fires, deletes c0, and the add restores the count, so trackedClientCount settles at exactly MAX_PENDING_PAIRINGS (8) and denials at 200 - 8 = 192 — both assertions sit on the boundary. I could not execute the suite (pnpm install hasn't run in this checkout; vitest fails to load lib/vite.config.ts), so this is traced, not run — CI's Build & Test is green on c0af617.

• Everything else re-checked and clean. The case arms are equivalent to the old grep -q '^VERDICT: FAIL' for the FAIL path ('VERDICT: FAIL'* matches the same prefix, and tolerates a trailing \r); [ "$STATUS" = FAIL ] || STATUS=MISSING correctly refuses to downgrade an earlier real FAIL, and a later FAIL still overrides an earlier MISSING; the eviction's !evicted.established && !evicted.session guard cannot drop a live client, and pendingCount-- is right on both branches since #denyPairing clears pending either way; app.ts re-indents cleanly with the comment and }; back in column position.

[dormouse-bot]

All three findings from the last round are closed at 62848c7a — re-traced each rather than taking the replies at face value.

The UNREADABLE_VERDICTS arm is correct: the variable is written in the dissent loop and read in the issue-body block, both inside the same run: step, so it is in scope; the arm sits between the missing-fragment arm and the generic else; and each of the three prose blocks is now true of the cause that selects it. The JSDoc is re-attached to #evictOldestPairingIfFull with trackedClientCount keeping its own. And the narrowing holds against the code — #evictOldestPairingIfFull counts only state.pending, #onConnect allocates through #resetAuthorization → #clientState unconditionally, and #dropTransientState does this.#clients.clear(), so "cleared wholesale when the socket drops" is accurate rather than hopeful. MAX_CLIENT_ID_LENGTH is checked ahead of the switch, which is what makes the spec's "before any map is touched" literally true; adding it to the Source of truth: list was the right call given SECURITY.md leans on it.

One new finding, and it is an interaction rather than a line. The dissent loop this PR adds is gated on [ "$STATUS" = "PASS" ], and the pre-existing missing-fragment loop twenty lines up writes that same variable — so one empty fragment turns the dissent check off for every other fragment. A domain that returned VERDICT: FAIL beside a missing sibling is then never noticed, and the issue the maintainer reads points them at "the rest" precisely when a domain in the rest failed. Details, the reachable path, and the gate that matches the intent are inline.

Also still open from my comment above: workflow-audit.yaml #L366 still enumerates five paths where WINDOW is four. The head commit doesn't touch that file, so I read it as queued behind this round rather than declined — flagging only so it doesn't fall off.

[dormouse-bot]

Two findings from this round that the review above didn't reach — both outside the incremental's five files, neither one a restatement of the dissent-gate finding.

#evictOldestPairingIfFull charges a stranger for a re-send. #onPair calls it unconditionally, but the line right after — this.#clientState(clientId).pending = pending — replaces the pending for a client that already has one. So when the queue is full and an already-queued client re-sends pair (a reconnect retry, or a client that never saw its own pair-result), the count is still MAX_PENDING_PAIRINGS afterward and an unrelated client has eaten the superseded denial for it. A hostile relay gains nothing here — it can produce the same evictions with fresh clientIds — so what this costs is the legitimate re-send. One line, and it keeps the counter honest about what actually grows:

    // Skip when this client already holds a slot: the assignment below
    // replaces its own pending rather than adding one, so evicting here would
    // cost an unrelated client its place without bounding anything.
    if (!this.#clients.get(clientId)?.pending) this.#evictOldestPairingIfFull();

requireUserVerification is mirrored at enrollment and never again. performEnrollment is its only writer, RemoteHostService.#start() loads the persisted blob and hands it straight to #startHost, and RemoteHost's constructor builds #policy from it once. Nothing re-fetches HostEnrollResponse. So setting DORMOUSE_REQUIRE_USER_VERIFICATION=true on a running deployment leaves every already-enrolled Host at false indefinitely, and the operator has to re-enroll each one with nothing telling them so. SECURITY.md's new FAIL IF — "reachable on one side without being mirrored to the other" — therefore holds for Hosts enrolled after the flag, not for the deployment. The Server still refuses a UV-absent connect2 itself, so what's lost is the Host-side half specifically, which is the half that survives Server compromise and the reason the mirroring exists. I'd put the caveat on the docs/specs/server.md table row this PR adds; a refresh-on-reconnect is a bigger design call than this PR is scoped for.

Happy to push either, or both alongside the workflow fix.

Verification

• Re-send eviction. The counter is for (const state of this.#clients.values()) if (state.pending) pendingCount++, which includes the re-sending client's own entry, so while (pendingCount >= MAX_PENDING_PAIRINGS) fires; #denyPairing + this.#clients.delete(oldestId) drops the oldest other client, and #clientState(clientId).pending = pending then overwrites in place. Net size unchanged, one stranger denied.
• The two sides stay consistent, which is why this shows up only as a fairness bug and not a divergence: #denyPairing ends in #dismissApproval(clientId) → the service's #resolvePairing, which removes the evicted entry from #pairings before #requestApproval enqueues the new one. That is also why #enqueuePairing's own while (this.#pairings.size >= MAX_PENDING_PAIRINGS) is a backstop that doesn't fire on this path — worth knowing before anyone "fixes" it there instead.
• No enrollment refresh. performEnrollment has exactly one non-test call site (service.ts #enroll). #start() reads this.#store.loadEnrollment() and goes to #startHost(enrollment) with no server round-trip. adopt carries a blob rather than re-fetching. Server-side enforcement is unaffected — handshake.ts passes its own #requireUserVerification into the policy, covered by server/test/handshake.test.mjs.
• Also checked and clean, for the record: MAX_CLIENT_ID_LENGTH / PAIRING_FIELD_LIMIT / MAX_PENDING_PAIRINGS all resolve where the spec's Source of truth: now points; _preamble.md's new line-1 rule has no competing instruction in the three domain files; restrict_to_owner(&tmp, 0o600) runs before write_all with the dir already 0700; model_args="--model opus" behind [ … ] && does not trip set -e (an AND-list's own status is exempt — ran it to confirm, since that one is easy to get backwards); clampTerminalDimension's new ceiling doesn't reach a local geometry-derived path that could exceed 2000; and vulnerabilityAlerts.schedule: [] is the documented "any time" value.
• I could not execute the suites in this checkout (pnpm install hasn't run), so both findings are traced rather than run.

[dormouse-bot]

The gate change is right, and the ordering it encodes (FAIL outranks MISSING outranks PASS) is the correct reading — I re-simulated the loop and a VERDICT: FAIL beside an empty sibling now reaches the FAIL branch instead of being masked. The workflow-audit.yaml header is derived from WINDOW now, which closes the drift for good rather than re-fixing the literal (WINDOW is in scope there — the step opens at - name: Audit workflow file changes and nothing else starts before EOF).

Two things the widened gate reaches that the prose downstream still assumes it can't. Both are the same class as the finding this commit fixes: a message that was true of the only state that could select it, and now isn't. Inline, with a simulation of each in the fold.

1. MISSING alongside an unreadable verdict says nothing about the unreadable one. MISSING_FRAGMENTS and UNREADABLE_VERDICTS were mutually exclusive before this commit — the dissent loop was gated on PASS, so one empty fragment kept it from ever running. They can now both be set, and the issue-body chain is still an elif, so the first arm wins and the unreadable fragment is dropped from the issue entirely. The reader is told "any report reproduced below covers only the rest" while a domain in "the rest" is exactly the one whose verdict couldn't be read. You handled this combination in the FAIL branch — the new Incomplete besides / Unreadable verdict notes — and the MISSING branch is the same combination one arm over.

2. The dissent path can now be entered with no merged verdict at all, and still reports one. STATUS is MISSING before either loop when audit-status.txt is absent or unreadable; the missing-fragment loop is still gated on PASS, so it's skipped, but the dissent loop now runs. A fragment opening VERDICT: FAIL correctly escalates — that's the ordering working — but the annotation says Audit reported PASS but … and the report addendum says "The merged verdict was PASS", when the audit reported nothing. Worse on that path: MISSING_FRAGMENTS is still empty (its loop never ran), so the Incomplete besides note can't fire, and a run that never wrote a verdict and lost a fragment files as a plain FAIL with no sign either happened.

Also worth a clause: the new FAIL IF requires "A FAIL arriving alongside missing or unreadable fragments must say so in the issue" — which covers the branch you fixed but not (1), the MISSING arrival the same gate change created.

Verification

I extracted the reporting step verbatim (top-level case, both loops, the addendum, and the full issue-body chain) and ran it under set -euo pipefail with real fragment files. Six scenarios; the two that matter:

• (1) status PASS, frag-a.md absent, frag-b.md opening with a heading, frag-c.md VERDICT: PASS:

  ::warning::Audit reported PASS but frag-a.md is missing or empty; downgrading to INCONCLUSIVE.
  ::warning::frag-b.md has no VERDICT line; the dissent check cannot read it.
  STATUS=MISSING  MISSING_FRAGMENTS='frag-a.md'  UNREADABLE_VERDICTS='frag-b.md'
  --- issue body ---
  The audit reported `PASS`, but it was downgraded: no report fragment was left for:
  frag-a.md. … The named domains are unaudited; any report reproduced below covers only the rest.
  

  frag-b.md appears only in the ::warning::. Re-ran the identical input against the pre-commit gate ([ "$STATUS" = "PASS" ], the one line changed): UNREADABLE_VERDICTS='', so the elif was sound before and is not now.

• (2) no audit-status.txt, frag-a.md absent, frag-b.md VERDICT: FAIL, frag-c.md VERDICT: PASS:

  ::error::Audit reported PASS but frag-b.md opens with VERDICT: FAIL; downgrading to FAIL.
  STATUS=FAIL  MISSING_FRAGMENTS=''  UNREADABLE_VERDICTS=''  DISSENTING='frag-b.md'
  --- issue body ---
  Audit failed at DATE.
  # merged report
  ---
  **Downgraded to FAIL by the reporting step.** The merged verdict was
  `PASS`, but these domain reports open with `VERDICT: FAIL`: frag-b.md.
  

  Three claims, none true of this run: the audit reported nothing, no verdict was merged, and frag-a.md — unaudited — is unmentioned. Same input on the pre-commit gate never enters the loop at all.

• Also checked and clean: the two [ -n … ] && lines in the FAIL branch don't trip set -e (a failing command in an && list is exempt unless it's the final one, and cat audit-report.md follows the group) — ran the both-empty-then-one-set combination to confirm rather than reasoning about it; [ "$STATUS" = FAIL ] || STATUS=MISSING refuses the downgrade in both loop orders; 'VERDICT: FAIL'* still tolerates a trailing \r; ${WINDOW[*]} renders the four paths in one code span (a shape change from the per-path backticks I'd sketched, not a defect — the point was deriving it); and scripts/security-audit-local.sh needs no mirror of this gate, since it exits at "merge and verdict are the orchestrator's job in CI; read the fragments directly here."

• I could not execute the workflow itself, so this is the step's logic re-run in isolation, not a live audit run. Build & Test, both Standalone Platform Check legs, Standalone Smoketest, and the Chromatic checks are green on 47d01c98.

[dormouse-bot]

The rewrite closes both of last round's findings, and closes them by removing the class rather than patching the two states. I re-ran the extracted step across the combinations rather than reading the arms: MISSING alongside an unreadable verdict now emits both notes, and STATUS_FILE_VERDICT survives the escalation, so a dissent-derived FAIL on a run that never wrote a status file says so in the body. Widening the missing-fragment gate to != "FAIL" was the necessary other half — the two guards agree about when they run now — and the belt-and-braces arm really is unreachable, since every path that sets MISSING also records a condition.

One condition the issue still cannot report, and it is the one the FAIL IF names. Both fragment guards are gated on [ "$STATUS" != "FAIL" ], so when the orchestrator itself writes FAIL, neither loop runs. A domain that left no fragment beside a real finding then produces no ::warning::, no note, and nothing in the issue — while SECURITY.md requires that "A FAIL arriving alongside missing or unreadable fragments must say so in the issue, or the report reads as the whole story." That holds only for a dissent-derived FAIL, which is the path where STATUS was still PASS or MISSING when the guards ran. Details and the run are inline; the fix has to touch the fi two lines past the hunk, so I can't suggest the whole shape — happy to push it.

The last piece of per-combination prose is the one this commit's new FAIL IF names. The addendum appended to audit-report.md still opens "Downgraded to FAIL by the reporting step. The merged verdict was PASS, but…" — the sentence this thread flagged, unchanged here. I'm not re-arguing that thread; what's new is its standing. "FAIL IF the reporting step writes issue prose per combination of conditions rather than one note per condition that holds" is repo policy as of this commit, and that addendum is prose about a combination (DISSENTING set and the merge said PASS) sitting one cat below a note that contradicts it. The notes block already carries the condition-shaped version of the same fact.

Verification

I extracted the Surface result, file or close issue step verbatim from 60ebf67f (245 lines, comments and all), stubbed gh, and ran it under set -eo pipefail against real fragment files in a scratch dir. The five that matter:

• Orchestrator FAIL + one empty fragment (audit-status.txt = FAIL, audit-application.md present but empty):

  --- annotations --- (none)
  Audit failed at DATE. [Run](…)
  
  # merged report
  real finding here
  

  audit-application.md is unaudited and appears nowhere — not in the body, not as a ::warning:: in the job log. The artifact step's if-no-files-found: warn doesn't cover it either: the file exists, it is just empty.

• Dissent-derived FAIL + the same empty fragment (audit-status.txt = PASS) — the contrast, and it is correct: both the A domain returned FAIL and A domain left no report notes fire, because the guards ran while STATUS was still PASS.

• No status file + a dissenting fragment — round 4's second finding, fixed at the note level, and where the addendum shows:

  - **A domain returned `FAIL`.** audit-supply-chain.md opened with `VERDICT: FAIL` …
  - **The audit wrote no verdict.** `audit-status.txt` was absent, empty, or not `PASS`/`FAIL` …
  
  # partial report
  ---
  **Downgraded to FAIL by the reporting step.** The merged verdict was
  `PASS`, but these domain reports open with `VERDICT: FAIL`: …
  

  Two bullets and the addendum, in one issue, disagreeing about whether a verdict was merged.

• PASS + one missing fragment + one heading-first fragment — both notes, both ::warning::s, title INCONCLUSIVE. Round 4's first finding is closed.

• Clean PASS — exit 0, Audit passed., no audit-comment.md. Dropping the [ ! -s audit-report.md ] placeholder write costs nothing: the body's No audit-report.md was produced. covers it, and the placeholder was written after the upload step anyway, so nothing observed it.

Also checked and clean: STATUS_FILE_VERDICT is assigned before either loop, so it records the file rather than the escalation; [ -n "$TRANSCRIPT_URL" ] && LINKS=… does not trip set -e (every run above had an empty TRANSCRIPT_URL and still produced a full body); NOTES=$(mktemp) is removed after the redirect closes; the elif [ "$STATUS" = "MISSING" ] arm is unreachable as its comment claims; scripts/security-audit-local.sh needs no mirror, since it stops at "merge and verdict are the orchestrator's job in CI"; and the SECURITY.md bullet's rename to "either fragment guard" matches both gates as written.

I could not execute the workflow itself, so this is the real step re-run in isolation, not a live audit run. All eight non-review checks are green on 60ebf67f.

[dormouse-bot]

I traced the separation independently before seeing the reply already on the gate thread, so I'll skip re-publishing the same scenario table — my run agrees with it, including that the escalation elif's != "FAIL" is what holds the ceiling on an orchestrator FAIL and that dropping the addendum costs the artifact nothing (the upload step runs before it).

One thing that reply didn't reach, in the other file, and it is the same shape as the defect this commit closes. The rewritten FAIL IF bullet keeps the sentence "The missing-fragment loop writes the same variable, so a PASS gate lets one empty fragment switch the dissent check off for every other one" — present tense, and it contradicts its own third sentence two clauses earlier ("Both loops run unconditionally and only record"). After this commit that loop writes only MISSING_FRAGMENTS; STATUS is assigned in exactly one place, the escalation block. It is also a restatement of the bullet's own first historical clause ("gated on PASS, one empty fragment silenced the dissent check"), so it's redundant as well as stale.

That matters more here than it would in a comment: SECURITY.md is what the audit reads as the control, so an auditor checking this bullet goes looking for a loop that writes STATUS and finds a rule whose rationale describes code that isn't there — which is exactly "prose that was true of the shape it was written against, made false by the next change", the class this PR exists to end. The suggestion folds the sentence's one non-redundant clause (the consequence) into the historical clause that already carries that shape.