fix(ci): correct the pr-issue-linkage header's claims about the pinned reusable - #3209
Conversation
The header comment said the gate is safe under pull_request_target because the reusable "reads PR body metadata from the event payload only and runs no head code". Only the second half is true at the pinned SHA (7107b34, v0.14.2): the reusable's Load current PR body step live-refetches the body via `gh api` and uses the event payload only as a fallback. No token reaches that step, so the re-fetch fails auth every time and the fallback is the path that actually runs - payload-only in effect, but as a side effect of the failed re-fetch, not by design. The safety rationale the zizmor suppression points at now rests explicitly on what actually makes the trigger safe: no head-branch code ever executes (nothing checked out, read-only permissions, body passed through env/GITHUB_ENV rather than spliced into script text). Two adjacent claims in the same block are corrected in the same pass: - "runs on the reusable's hosted default runner" -> the caller pins `runner: ubuntu-24.04` explicitly in the `with:` block; the value only coincides with the reusable's default. - The no-issue marker is a case-insensitive regex matching the phrase "no linked issue" or "no related issue", not a literal string, and both body scans run with code blocks blanked, inline code spans masked, and HTML-commented text discarded. Comment text only: triggers, permissions, the pinned SHA, and the `with:` values are unchanged. actionlint and zizmor both pass on the edited file, with the dangerous-triggers suppression still honored. Closes #3184 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XtbWChCVfUWAv1Pi5Qk2hA
Independent review confirmed every claim in the rewritten header against
the pinned reusable but flagged two phrasings a maintainer could misread:
- "indented code blocks blanked" suggested the CommonMark indented-code
construct, but the masking pass blanks any line matching
/^(?: {4}|\t)/ - including a 4-space-indented nested list item that is
not code. The header now says "any 4-space- or tab-indented line".
- "Public repo:" no longer connected to the sentence that followed once
the hosted-default-runner claim was replaced with the explicit pin.
The lead-in now states why visibility matters (GitHub-hosted runners
are free for public repositories, matching ci.yml's phrasing) before
naming the pin.
Still comment text only; actionlint and zizmor both remain green with
the dangerous-triggers suppression honored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XtbWChCVfUWAv1Pi5Qk2hA
|
Claude finished @kyle-sexton's task in 1m 0s —— View job Security Review
No security issues found. This PR touches a single file, Since there is no security-relevant surface change (skip-gate criterion 2 in As a documentation-accuracy note (not a security finding): the corrected header's safety rationale — no head-branch code executes because the reusable checks out nothing, holds only |
|
Last security-reviewed head: |
|
Claude finished @kyle-sexton's task in 2m 26s —— View job Claude Code Review
|
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…ions (0.55.11) (#3217) Closes #3206 ## Summary The shared PR-body linkage validator only checked a closing keyword and `## Related`, so both local pre-checks allowed bodies the pinned `pr-issue-linkage` reusable rejects. Local gates now require the same four contract sections CI does. ## Fix `pr-linkage-validator.sh` generalizes `related_section()` into `section_content()` over `Summary`, `Fix`, `Verification`, and `Related`, using the existing heading-level semantics (a nested `###` is content). Each missing or empty section is its own `LINKAGE_PROBLEMS` entry. Both hook surfaces pick this up from the shared core; their blocked-message remedies now list all four sections so following them produces a body CI accepts. The no-issue-marker path and `update_pull_request` without `body` are unchanged. ## Verification `bash plugins/source-control/hooks/pr-body-linkage-gate.test.sh` — passed: 137 failed: 0 `bash plugins/source-control/hooks/pr-linkage-mcp-gate.test.sh` — pass=25 fail=0 New cases: the #3205 body (linkage + Summary + Verification + Related, no Fix) is blocked; a nested `###` inside each required section is content. `check-changelog-parity.sh --check-bump origin/main` passes (0.55.6). ## Related - #3205 — the PR whose `pr-issue-linkage` failure is the reproduction - #3209 — comment-only header correction on the same reusable pin; no functional overlap - `.github/workflows/pr-issue-linkage.yml` — pins the upstream reusable at v0.14.2 --------- Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
1 participant
Closes #3184
Summary
The header comment in
.github/workflows/pr-issue-linkage.ymljustified itspull_request_targettrigger — and thezizmor: ignore[dangerous-triggers]suppression that points at it — with a sentence that does not describe the pinned reusable: "the reusable reads PR body metadata from the event payload only and runs no head code." Only the second half is true. The same block also misattributed the runner to "the reusable's hosted default" and described the no-issue opt-out as a literal string. This PR rewrites the header so every claim matches the reusable's actual code at the pinned SHA (7107b34, v0.14.2).Fix
Comment text only; no functional YAML changes. Three corrections:
Load current PR bodystep live-refetches the body viagh apiand uses the event payload only as a fallback when that call fails. Re-verified at the pinned SHA: noGH_TOKEN/GITHUB_TOKENreaches that step (itsenv:carries onlyPR_NUMBERandFALLBACK_BODY, and neither the job nor the workflow wires a token), so the re-fetch fails auth every time and the payload fallback is the path that actually runs. The header now states this — payload-only in effect, as a side effect of the failed re-fetch, not by design — and rests thepull_request_targetsafety rationale on what actually makes the trigger safe: no head-branch code ever executes (the reusable checks out nothing, holdspull-requests: read/actions: readonly, and passes the body throughenv:/GITHUB_ENVrather than splicing it into script text).with:block pinsrunner: ubuntu-24.04explicitly, which coincides with the reusable's default./\bno (?:linked|related) issue\b/i), not a literal string, with both body scans running against a body whose fenced code blocks and 4-space/tab-indented lines are blanked, inline code spans masked, and HTML-commented text discarded.Per the issue's scope note, wiring a token into the reusable so the re-fetch succeeds is out of scope — that lives in
melodic-software/ci-workflowsand would be a behavior change, not a comment fix.Verification
7107b34832a7b6db5d08d3b132621c599fbe5e50: thegh apire-fetch withFALLBACK_BODYfallback, the absence of any token in the step/job/workflow environment, the absence of any checkout step, the read-onlypermissions:block, theenv:/GITHUB_ENVbody path with random heredoc delimiter, therunnerinput default, theNO_ISSUE_MARKERregex, and the code/comment masking instripRenderedHtmlComments(including the/^(?: {4}|\t)/indented-code branch).actionlintpasses on the edited file;zizmorpasses with thedangerous-triggerssuppression still honored (No findings to report. (1 ignored, 1 suppressed)).git diff -U0filtered to non-comment lines is empty:on:,permissions:, theuses:pin, and thewith:values are byte-identical tomain.Related
.github/workflows/ci.yml; it cannot collide with the concurrentdocs_onlyscope-resolution work there (ci: hoist docs_only scope resolution into one job instead of 46 references #3159).