Skip to content

docs(security): name .github/audit/ in the audit-visibility window - #451

Closed
dormouse-bot wants to merge 2 commits into
mainfrom
docs/security-audit-window-names-audit-prompts
Closed

docs(security): name .github/audit/ in the audit-visibility window#451
dormouse-bot wants to merge 2 commits into
mainfrom
docs/security-audit-window-names-audit-prompts

Conversation

@dormouse-bot

@dormouse-bot dormouse-bot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

SECURITY.md's Audit visibility paragraph — the prose spec of workflow-audit.yaml — says the job "walks every commit touching .github/workflows/ or .config/tend.yaml", naming two of the three paths actually in the window. A FAIL IF in the same document then asserts the third: "FAIL IF .github/audit/ is outside workflow-audit.yaml's diff window." A reader checking that FAIL IF against the paragraph it belongs to gets a contradiction, and the paragraph is the one a reader reaches first.

The workflow is right, the prose is stale. .github/audit/ was added to the window by #446 (59b0cedf) without the paragraph following. This names it, and adds the one-clause reason parallel to the sentence already there for .config/tend.yaml, restated from the workflow's own comment at the pathspec: the prompts decide what gets audited and by whom, so a bot that edits them changes the audit's coverage without touching a single workflow file.

Every claim added here is true of main as it stands and stays true after #450, so the two can merge in either order and neither conflicts with #449. Docs only — no behavior change. Found by the review on #450, which fixes the corresponding gap in the workflow's runtime output; kept separate because either change reverts without affecting the other.

Verification

The Audit visibility paragraph enumerated two of the three paths
workflow-audit.yaml actually walks, while a FAIL IF further down asserts
that .github/audit/ is in that window. The workflow is right; #446 added
the path without the prose following.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 26, 2026

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 320ae38
Status: ✅  Deploy successful!
Preview URL: https://163c3f42.mouseterm.pages.dev
Branch Preview URL: https://docs-security-audit-window-n.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.

The enumeration fix itself checks out: the window's pathspec is three paths, the paragraph named two, and the FAIL IF at the bottom asserted the third — the contradiction is real and this closes it. Two things about the new prose landing ahead of #450, though.

The "must be enumerated together" sentence is false of main today. The added clause reads "All three paths must be enumerated together wherever the window is described, here and in the workflow." On main at 59b0cedf, workflow-audit.yaml describes the window in three more places and each names only two paths: the single-parent branch of own_changes() (git show --name-only --pretty='' "$sha" -- .github/workflows/ .config/tend.yaml), the merge branch just below it (git diff --name-only "$p" "$sha" -- ...), and the issue-body header (echo "$COUNT unexplained commit(s) touching \.github/workflows/` or `.config/tend.yaml` since ..."`). #450 fixes all three, but until it merges this PR replaces one prose/code contradiction with another — a reader who follows the sentence into the workflow to check it lands in exactly the position this PR exists to spare them. Simplest resolution is merge order: land #450 first, then this. If they land the other way round, the sentence should be scoped to the pathspec it's actually true of, not asserted over the whole file.

The regen-arm sentence is the next stale enumeration, and nothing owns it. Later in this same paragraph: "A tend regeneration must ... must not touch .config/tend.yaml in the same commit." After #450 both classifier arms also refuse .github/audit/ ([ -z "$(git show --name-only --pretty='' "$sha" -- .config/tend.yaml .github/audit/)" ] || return 1 in is_renovate_pin_bump and is_tend_regen). The PR body defers that prose to #450 — "#450 changes those arms to refuse .github/audit/ and is where that prose belongs" — but #450's only changed file is .github/workflows/workflow-audit.yaml, so no PR currently updates it. That leaves the same shape of drift one sentence further along, in the paragraph this PR just rewrote for exactly that reason. Either add the clause here (it's true the moment #450 lands, and this is the PR that owns this paragraph), or add a SECURITY.md hunk to #450.

No conflict with #449 — it touches SECURITY.md at the disclosure-roots paragraph and the audit-scope bullets, not this one.

…ependent

The review on this PR flagged two claims that depend on #450 landing first.

"All three paths must be enumerated together wherever the window is
described, here and in the workflow" is false of main: workflow-audit.yaml
describes the window in three more places and each names only two paths.
Scope the claim to the selection pathspec the paragraph is actually the
prose spec of, which names the same three paths today and after #450.

The regen-arm sentence ("must not touch .config/tend.yaml") was the next
stale enumeration and no PR owned it, since #450 changes only the workflow.
State the invariant non-enumeratively instead — both arms inspect only
.github/workflows/, so every other path the window selects has to be
refused outright — which holds regardless of merge order and does not need
re-editing when a path is added.
@dormouse-bot

Copy link
Copy Markdown
Collaborator Author

Both points taken; pushed 320ae386. I took neither of the offered resolutions literally, because both of them make one of these two PRs depend on the other's merge order, and I'd rather neither did — a SECURITY.md hunk in #450 would also collide with this one, since the whole paragraph is a single source line.

The enumeration claim is now scoped to the pathspec it's true of. "All three paths must be enumerated together wherever the window is described, here and in the workflow""This paragraph is the prose spec of that job's selection pathspec, so the two enumerations name the same paths: a path added to the window without landing here leaves a reader checking that FAIL IF against a paragraph that contradicts it." That's the git log pathspec, which names three paths on main today, so the sentence is true before #450 and unchanged by it. own_changes() and the issue-body header are out of its scope — they're #450's to fix, and the reader is no longer sent to them.

The regen-arm sentence keeps its enumeration but no longer needs one. Rather than pre-document #450's widening, I added the invariant behind it in a form that doesn't enumerate: both arms inspect only .github/workflows/, so every other path the window selects has to be refused outright rather than examined, and widening the window without widening those refusals is what would let a commit hide inside an already-classified diff shape. True as both arms stand, still true once #450 widens them, and it needs no edit the next time a path is added — which is what "nothing owns it" was really about. Verified the premise: the Renovate content test diffs -- .github/workflows/, and the regen arm's file list and git status --porcelain check are both scoped to it.

Net effect is that #450 and #451 can land in either order, and #449 still doesn't overlap.

One thing I found while verifying, not fixed here

The regen arm's comment on main opens "The generator's output is only as trustworthy as its input, and .config/tend.yaml is outside this audit's window" (workflow-audit.yaml#L196-L198). The config is inside the window — it's in the git log pathspec at #L108, and SECURITY.md spends a clause on why. The sentence reads as if it means "outside the reproducibility check's scope", which is true and is the point the rest of the comment makes, but "this audit's window" is the phrase the file uses for the git log selection everywhere else. #450 rewrites the lines directly under it; I left it alone rather than edit a security workflow from a session dispatched at this PR.

@nedtwigg

Copy link
Copy Markdown
Member

Superseded by #452, which makes the same correction — I had missed it too.

The Audit visibility paragraph is the prose spec of that pathspec, and it named two paths where the window had more. #452 states all four it now watches (.github/workflows/, .config/tend.yaml, .github/audit/, .vscode/) and adds the sentence your PR argues for: that a path added to one enumeration and not the other leaves a reader checking the FAIL IF against a paragraph contradicting it.

One difference from your version, from #450's review rather than yours: SECURITY.md itself is not in the window. I had briefly added it; #450 argued it out, and #452 agrees — that job watches code that executes from an unreviewed branch, and a FAIL IF is inert until it reaches main.

Closing as superseded, not as wrong.

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.

2 participants