Skip to content

Keep provisioning obligations out of the FAIL IF list so the audit can decide - #748

Merged
nedtwigg merged 9 commits into
mainfrom
audit-external-obligations
Sep 23, 2026
Merged

nedtwigg merged 9 commits into
mainfrom
audit-external-obligations

Conversation

@dormouse-bot

@dormouse-bot dormouse-bot commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Makes the fix from #757 general. #757 resolved #747 for Hosted: it moved the two provisioning sentences in docs/specs/security-hosted.md under ## Future, and kept the deploy's preflight gate as an audited FAIL IF. This PR adds the rule so the next spec author doesn't recreate the problem, tells every audit domain how to treat external state, and clears the one live counterexample left in docs/specs/security-ci.md.

  • docs/specs/security-audit.md: new rule. Never write a FAIL IF condition no audit run can read: audit the readable half. The rest goes under ## Future only while it is unbuilt; otherwise it is stated beside the rule. GitHub state that AUDIT_PAT can read stays audited. Budget ratcheted from 1950 to 2000.
  • .github/audit/_preamble.md: tells every domain that a condition no run can read is not a check — record it as INFO, never UNVERIFIABLE, since nothing a later run can read would settle it. Three bullets say what to verdict depending on how the obligation is written: inside a FAIL IF, beside a rule or staged under ## Future, or promoted above the fold. AUDIT_PAT-readable GitHub state still gets checked, and a failed call there is still UNVERIFIABLE.
  • .github/audit/hosted.md: drops the promotion sentence the preamble now states for every domain; the sentence naming which item is staged stays.
  • docs/specs/security-ci.md: the Hosted credential FAIL IF had the same problem, asking the auditor to inspect Cloudflare and Neon token scope. It now covers GitHub secret placement alone. The provider-token isolation obligation and the marketing-separated deployment identity in hosted/README.md are dropped rather than restated — nothing reads them and no one asked for them.
  • docs/specs/security-audit.rationale.md: the evidence. Runs 35586089654 and 35709640946 read the same unchanged tree to opposite verdicts; Stage Hosted production activation under Future so the audit can pass #757 was the fix.
History

dormouse-bot opened this before #757 with its own edit to security-hosted.md. After #757 merged, that edit conflicted, so the merge from main takes main's security-hosted.md unchanged. The general rule was reworded to point at ## Future, matching #757. A later round replaced the provider-token clause with a Must; nedtwigg then removed it outright along with the repo's TTR references, so security-ci.md ends below the word count it started at and its budget is unchanged.

Verified: pnpm lint:specs, node --test scripts/security-audit.test.mjs (45 pass), node scripts/public-docs-lint.mjs.

Refs #747

…n decide

security-hosted.md carried two conditions no run can determine from the
repository: the Cloudflare script-injection exclusion, a zone setting, and
the closing activation sentence, which states its own answer. Two
consecutive nightly runs read them opposite ways — 2026-09-21 resolved both
to PASS, 2026-09-22 left both UNVERIFIABLE and returned INCONCLUSIVE on a
375-PASS, 0-FAIL pass, holding the release gate shut (#747).

Both now sit in the activation paragraph as obligations rather than inside a
FAIL IF. security-audit.md states the rule for future authors, and the shared
preamble tells a domain what verdict an external obligation gets, so neither
becomes an undetermined check again.

Refs #747
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 22, 2026

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: cd3965f
Status: ✅  Deploy successful!
Preview URL: https://b898ec2b.mouseterm.pages.dev
Branch Preview URL: https://audit-external-obligations.mouseterm.pages.dev

View logs

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.

The rule is keyed on outside the repository, but the property that makes a condition undecidable is unreadable by any run. The rationale states the right test ("nothing a later run can read settles it"); the two normative lines don't, and ci-and-secrets is almost entirely conditions on state outside the repository that AUDIT_PAT can read — rulesets, environments, secret placement, actions/permissions/workflow, the bot's collaborator role. As written, _preamble.md tells that domain those are "not a check at all", to verdict only "the repository's own state", and to record the rest as INFO. Applied to docs/specs/security-ci.md → "FAIL IF either admin-gating ruleset is missing or weakened", there is no in-repo half to verdict at all, so the strongest gate on main becomes an INFO note and the domain still returns PASS. It also contradicts this spec's own "Must execute every FAIL IF as a mechanical check (gh api, grep, file read, or a script run)" and .github/audit/ci-and-secrets.md's "Report an inaccessible check as UNVERIFIABLE". Given the failure being fixed is two runs reading one sentence differently, the replacement is worth making unreadable-two-ways. Suggestions inline — the security-audit.md one lands at 1950/1950, so it still needs no ratchet.

Second, the shape survives in docs/specs/security-ci.md under "Hosted Deployments", in the bullet reading "preview credentials can reach production/TTR/marketing resources. Inspect GitHub secret placement and Cloudflare/Neon token scope; names alone do not isolate resources." Cloudflare and Neon token scope is the same external-console state this PR removes from security-hosted.md, and it sits in ci-and-secrets — a domain #746 doesn't touch either, so the split there wouldn't contain it. Splitting that clause the same way (GitHub-side secret placement audited, provider token scope an activation obligation) before the new rule lands would keep the rule from shipping with a live counterexample in its sibling spec.

Comment thread .github/audit/_preamble.md Outdated
Comment thread docs/specs/security-audit.md Outdated
nedtwigg and others added 2 commits September 22, 2026 21:34
…tions

# Conflicts:
#	docs/specs/security-hosted.md
#757 already moved the two Hosted provisioning sentences under
security-hosted.md's ## Future and kept the in-repo preflight gate as a
FAIL IF, so the merge takes main's security-hosted.md. The general rule
and the preamble instruction still apply to every other spec; they now
name ## Future as where an external obligation goes, the rationale
records how #747 was resolved, and the budget is ratcheted for the rule.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.

The new remedy sends external obligations to ## Future, but AGENTS.md → "Spec lifecycle" reserves that section for unbuilt design: "Everything above ## Future describes the code as it is — present tense"; "a spec with no unbuilt design has none"; and "Promotion is part of done. A staged item is finished only when its text moves above the fold." An obligation on infrastructure that already exists never becomes in-repo code, so it can never be promoted, and rests below the fold permanently — the one resting state that rule forbids. #757's destination fit because Hosted genuinely is not provisioned yet; generalized, the rule inherits a home that only suits an unbuilt subject.

Four of the specs it governs — security-ci.md, security-local.md, security-supply-chain.md, security.md — have no ## Future section at all, and security-ci.md is where the external clause on the open thread above lives. Following the rule there means adding a ## Future to a spec AGENTS.md says should not have one, to hold a fact about how the project is deployed today.

Separately, .github/audit/_preamble.md's "as for anything a spec stages under ## Future" alludes to a convention stated in exactly one place — .github/audit/hosted.md, scoped to that domain — and leaves behind the clause that bounds it: "Once it is promoted above the fold, audit it as a FAIL IF like any other." Generalizing to every domain should carry that sentence into the preamble rather than gesture at it.

The inline suggestion keeps ## Future for genuinely staged obligations and leaves a standing one beside its rule; it lands at 1987 words, inside the ratcheted 2000.

Comment thread docs/specs/security-audit.md Outdated
…lt work

Applies dormouse-bot's three threads together. The rule and preamble now
say "no audit run can read" instead of "outside the repository", so the
GitHub state AUDIT_PAT reaches stays a check. ## Future holds an
obligation only while its subject is unbuilt; a standing one is stated
beside its rule. The preamble carries the promotion clause from hosted.md.
security-ci.md's Hosted credential rule is split the same way: GitHub
secret placement stays audited, provider token scope becomes a Must.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.

Splitting security-ci.md's Hosted credential bullet creates a third disposition for an unreadable obligation — stated beside its rule as a Must — and _preamble.md covers only two. Its instruction reads "Verdict the FAIL IF's readable condition, and record the external obligation as INFO", which presumes the obligation is still inside a FAIL IF. ci-and-secrets is told to run a mechanical check "for each FAIL IF in your scope", and its qualitative pass owns .github/, .config/, .claude/, .vscode/, scripts/, website/public/, and code that touches a secret — Cloudflare and Neon token scope is in none of those. So the obligation this PR deliberately preserves is readable two ways, an INFO every run or nothing at all, which is the failure this PR exists to close, on its own first example. The suggestion names all three dispositions, and moves the promotion clause to the end where it bounds the staged case alone.

Comment thread .github/audit/_preamble.md Outdated
Takes dormouse-bot's preamble wording, drops hosted.md's now-duplicate
promotion clause, and moves security-ci.md's token-scope Must below the
rule list so the section keeps one shape.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.

UNVERIFIABLE lost the scoping word that bounded it. The previous wording read "UNVERIFIABLE there would make every later run inconclusive too", where "there" tied the claim to the unreadable-condition case. Without it the sentence asserts that UNVERIFIABLE makes every later run inconclusive, unqualified — which the paragraph immediately above contradicts ("Use UNVERIFIABLE only for a check you could not determine — a transient network error…"), and which .github/audit/hosted.md contradicts directly: it tells that domain to report the two pgstencil provenance checks as UNVERIFIABLE if the public GitHub API is unreachable. The sentence that follows rescues the AUDIT_PAT case by name, so a domain hitting a transient failure outside GitHub state is the one left reading a general prohibition. Suggestion inline restores the word and rewraps.

Separately, worth weighing rather than acting on: this one paragraph has now drawn a distinct finding on each of the last four pushes, and it currently carries three dispositions, two rescues, and a promotion rule in eleven lines. If another edge turns up, the shape is likelier to be the problem than the wording.

Comment thread .github/audit/_preamble.md Outdated
The paragraph drew a new finding on four consecutive pushes. As a short
list it states each disposition once, keeps UNVERIFIABLE scoped to the
unreadable case, and defers failed calls on AUDIT_PAT state to the
paragraph above instead of restating it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.

The restructure carried the promotion clause over unchanged, so the third bullet now sits under a stem reading "A condition no audit run can read" and says that condition, once promoted, "is a FAIL IF like any other" — the one disposition docs/specs/security-audit.md's new rule says never exists. The live example is the item this PR's sibling staged: security-hosted.md's Production activation bundles the preflight values with "Cloudflare script injection excluded for the Hosted hostname", so promoting it tells the hosted domain to FAIL on a zone setting no run can read — the verdict the PR exists to prevent. Naming the promoted rule's form rather than the timing fixes it in the same two lines.

Second, the trailing paragraph replaced "UNVERIFIABLE stays right for a call that fails" with "the paragraph above governs a call that fails", and the nearest paragraph above it says "Record it as INFO, never as UNVERIFIABLE". That resolves backwards for the domain it is written for: .github/audit/ci-and-secrets.md instructs "Report an inaccessible check as UNVERIFIABLE", and supply-chain.md says the same, so a failed gh api on the admin-gating ruleset now reads as INFO — which does not hold the verdict open, so an undetermined check on the strongest gate on main passes silently. Restoring the named outcome costs one word and leaves nothing to resolve.

Comment thread .github/audit/_preamble.md Outdated
Comment thread .github/audit/_preamble.md Outdated
…led calls

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@nedtwigg
nedtwigg marked this pull request as ready for review September 23, 2026 05:32
TTR is an unrelated project; nothing in Dormouse should name it. The
token-scope Must and the marketing-separated deployment identity were
obligations no audit can read and no one asked for; GitHub-side secret
placement stays audited.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The carve-out lands beside an unqualified absolute it contradicts. _preamble.md L11-13 still reads "A FAIL IF bullet may assert several properties in one sentence; each clause gets its own verdict and its own evidence. Never satisfy a bullet in bulk." The new first bullet says the opposite for an unreadable clause: verdict only that rule's readable condition.

That pair is what the two runs in the rationale split on. Run 35586089654 resolved the unreadable clause to PASS "on the ground that the audited condition was the in-repo half" — the new bullet's reading; run 35709640946 gave the clause its own verdict and left it UNVERIFIABLE — L12-13's reading. Both instructions are still in the file, ten lines apart, and the one that reads as absolute comes first. security-audit.md L82 keeps them from meeting in a spec written from today on, but the first bullet exists for the drifted FAIL IF that L82 forbids, and that is the only case where the conflict fires — the #747 case exactly. Suggestion inline disapplies the each-clause rule where the carve-out is introduced, so it costs no new sentence.

Separately, on the shape rather than this round's wording: the fourth review noted the paragraph had drawn a distinct finding on each of four pushes and said that if another edge turned up, the shape was likelier to be the problem than the wording. This is that edge, and it is the first one landing outside the added lines, in text the paragraph now has to override at a distance. The paragraph carries three dispositions, a promotion rule, and a rescue; the alternative worth weighing is whether the preamble needs more than the stem and the AUDIT_PAT sentence, with the dispositions living in docs/specs/security-audit.md beside the authoring rule. That is a call about the approach, not a defect, so it stays here rather than becoming a sixth revision pushed onto the branch.

15c11da reviewed and clean: dropping the token-scope Must leaves no readable condition unaudited — GitHub secret placement stays in the FAIL IF above it, and preview configuration copying production routes or bindings is still security-hosted.md's. The hosted/README.md renumbering has no cross-references into it, and docs/specs/hosted.md -> "Must keep production, test, and preview databases and credentials separate" still resolves against what the runbook says.

Verification on this head
  • node scripts/spec-lint.mjs — clean apart from two standalone/sidecar/node_modules path misses in specs this PR does not touch, an artifact of a checkout without pnpm install.
  • node --test scripts/security-audit.test.mjs — 45/45.
  • node scripts/public-docs-lint.mjs — 15 checks.
  • Budgets: countWords reports security-audit.md at 1985 against 2000 and security-ci.md at 2679 against 2700, so the revert of the security-ci.md ratchet is right — it now sits below where it started (2695 at the merge base).

Comment thread .github/audit/_preamble.md Outdated
Resolves the spec-word-budgets.json conflict with #761 and applies the
review suggestion that disapplies the preamble's each-clause rule where
the unreadable-condition carve-out is introduced.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@nedtwigg
nedtwigg merged commit 340b980 into main Sep 23, 2026
10 of 11 checks passed
nedtwigg added a commit that referenced this pull request Sep 23, 2026
Both PRs add a rule to security-audit.md, so its budget is re-ratcheted
to 2100. Also corrects the rationale: only the findings carry the 40-line
cap; the 500-character cut applies to every lifted line.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch is waiting to be deployed

1 waiting deployment
hosted-preview cd3965ff Waiting Sep 23, 2026 by nedtwigg via deploy #303
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.

[security-audit] INCONCLUSIVE on 2026-09-22

2 participants