Skip to content

ci: pr-triage sees every closing keyword form; codeql checkout keeps no credentials - #90

Merged
askalf merged 3 commits into
mainfrom
ci/actions-audit
Sep 26, 2026
Merged

askalf merged 3 commits into
mainfrom
ci/actions-audit

Conversation

@askalf

@askalf askalf commented Sep 26, 2026

Copy link
Copy Markdown
Owner

From the 2026-09-25 audit of every action.

  • pr-triage: the closing-keyword check only matched Closes #N. A body reading Fixes askalf/repo#12 on a branch named issue-13-fix looked like it closed nothing, so Closes #13 was appended and merging closed both, one an issue the author never named. The regex now covers bare #N, owner/repo#N and the full issue URL, the same one plumbline already runs (pr-triage.yml:107-113 there).
  • codeql: the checkout keeps persist-credentials: false like every other checkout in the repo.

No behaviour change for a PR whose body already closes an issue in any form, or whose branch names none.

@github-actions github-actions Bot added github_actions Pull requests that update GitHub Actions code size/S 10-49 hand-written lines labels Sep 26, 2026

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the gating lane (gating review).

Verdict: request changes. The regex change is correct: traced Fixes askalf/repo#12, a bare Closes #12, and a full issue URL through the new pattern and all three match, so the branch-derived Closes #N is no longer appended on top of an existing cross-repo or URL reference; the trailing \b only rejects forms like #12abc that GitHub would not honour either. The codeql persist-credentials: false addition matches every other checkout in the repo and CodeQL init/analyze do not need the git credential. The blocking finding is the new code comment, which is patch narration with an em dash rather than a description of what the code requires.

1. Blocking: .github/workflows/pr-triage.yml:106

        // Every closing form GitHub honours, not just the bare one. A body
        // reading "Fixes askalf/repo#12" on a branch named issue-13-fix used
        // to look like it closed nothing, so `Closes #13` was appended and
        // MERGING CLOSED BOTH — one an issue the author never named.
        // Covers bare #N, cross-repo owner/repo#N, and the full issue URL.

This comment narrates the bug the patch fixes ("used to look like it closed nothing, so Closes #13 was appended and MERGING CLOSED BOTH — one an issue the author never named") instead of stating what the code needs to be true. That history belongs in the commit message and PR body, where it already is; in the file it reads as generated patch narration, including the em dash and the shouted clause, and goes stale the moment it merges.

Suggested fix:

Keep only the constraint the reader needs, e.g. `// Any reference form GitHub honours after a closing keyword: #N, owner/repo#N, or the full issue URL. Missing one here means a second Closes line gets appended below.`

rule:reads-as-generated

@askalf
askalf enabled auto-merge (squash) September 26, 2026 01:31
@github-actions github-actions Bot added size/XS Under 10 hand-written lines and removed size/S 10-49 hand-written lines labels Sep 26, 2026

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the gating lane (gating review).

Verdict: approve. Approving. The new closing-keyword regex in pr-triage.yml is static (no untrusted interpolation), and I traced the three reference forms GitHub honours after a closing keyword through it: Closes #13, Fixes askalf/repo#12 and Resolves https://github.com/askalf/cordon/issues/12 all match, so the branch-derived Closes #N is no longer appended on top of an existing cross-repo or URL reference; the added trailing \b only rejects things like #12abc, which GitHub would not treat as a reference either. The rest of the block (branch parsing, issue lookup, body update) is unchanged. The codeql.yml checkout now sets persist-credentials: false, which is consistent with the other checkouts and safe since codeql-action init/analyze do not need git credentials; the workflow's permissions and pinned action SHAs are untouched. Comments and commit messages read as hand-written with no attribution or secrets. No tests exist for workflow scripts in this repo, so there is nothing for this diff to add.

@askalf
askalf merged commit 2ec3c07 into main Sep 26, 2026
14 checks passed
@askalf
askalf deleted the ci/actions-audit branch September 26, 2026 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update GitHub Actions code size/XS Under 10 hand-written lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants