fix(pr-issue-linkage): ignore markers in code by kyle-sexton · Pull Request #355 · melodic-software/ci-workflows

kyle-sexton · 2026-08-05T19:40:17Z

No linked issue

Summary

Mask inline-code and fenced-code contents before validating PR linkage metadata. The v0.10.1 parser correctly prevented literal HTML-comment markers in code from hiding later live metadata, but preserved other code contents long enough for example No linked issue or ## Related text to satisfy the gate.

This keeps the comment-parser fix while restoring fail-closed behavior for linkage markers that are not rendered as metadata.

Verification

node --test .github/scripts/pr-issue-linkage.test.cjs (23/23)
actionlint .github/workflows/pr-issue-linkage.yml
Prettier
git diff --check

This PR masks fenced- and inline-code contents in the PR body before the linkage-gate check, fixing the case where example text like "No linked issue" inside a code span/fence could satisfy the gate. Scope is narrow (only the stripRenderedHtmlComments masking logic in pr-issue-linkage.yml); it doesn't touch any of the three Claude-lane workflows, so the CLAUDE.md security ground rules for those lanes don't apply here, and no uses: pins are touched.

Findings

1. Inline code's closing backtick delimiter leaks into the "masked" output — pr-issue-linkage.yml:198 (posted as an inline review comment)

Hand-traced `code` through stripRenderedHtmlComments: the opening backtick is correctly withheld (the hasClosingTickRun branch leaves inlineTicks non-null, so the guard skips the append), but the closing backtick run is appended, because the else if (inlineTicks === ticks) inlineTicks = null; branch resets inlineTicks to null just before the if (inlineTicks === null) rendered += … guard runs. Net effect: masking `code` produces ` instead of "" — the closing delimiter survives.

This is asymmetric with the (correct) opening-tick handling and doesn't fully satisfy the code's own stated goal ("Code contents are masked from the returned text"). It's likely benign against the current keyword/## Related-heading regexes when a code span sits on its own line, but it's an incomplete mask and isn't exercised by either new test (both only assert the gate's outcome, not that the masked text is delimiter-free). See the inline comment for a suggested fix direction (only append a tick-run when it neither opened nor closed a span, rather than checking only the post-update state of inlineTicks).

Verification note

I could not independently execute node --test, actionlint, or prettier in this environment — sandboxing here only permits read-only git/file operations, not arbitrary command execution. I hand-traced the masking function against several inputs instead (see above). The PR description's test/lint results are author-claimed and unverified by me.

No other correctness, error-handling, or maintainability issues found in the diff. The fenced-code masking (output.push(line) → output.push("")) is a clean full-line blank and looks correct.
· branch

claude · 2026-08-05T19:40:39Z

Claude finished @kyle-sexton's task in 3m 39s —— View job

Security Review — PR #355

Gather context (diff, changed files)
Understand the request (security review of this PR)
Review .github/scripts/pr-issue-linkage.test.cjs
Review .github/workflows/pr-issue-linkage.yml
Post findings

Scope

Reviewed only the two files changed in this PR (gh pr diff 355): the masking-logic hunks in pr-issue-linkage.yml's inline github-script step, and the two additive tests in pr-issue-linkage.test.cjs. This workflow parses github.event.pull_request.body (untrusted PR-author text) to decide whether a required merge-gate check passes — the relevant trust boundary is "can PR-author-controlled text trick the parser into treating decorative/example text as live linkage metadata (or vice versa)."

No command/script injection, credential handling, or permissions:/trigger changes are introduced by this diff — PR_BODY stays env:-passed into pure JS string logic, never spliced into run:/shell text, and the workflow-level permissions: {} and pull_request_target trigger design are untouched by this PR (out of scope for this review — see the file's own header comment, lines 13–20, for that pre-existing rationale). I deferred any pin/trigger/permission-shape concerns to zizmor's static lane per the review instructions.

Findings

SUGGESTION — Masking still misses indented (4-space) code blocks — Confidence: Medium

The new masking (.github/workflows/pr-issue-linkage.yml#L150-L216, e.g. L153 and L198) correctly blanks fenced (```/~~~) blocks and inline ` spans, closing the exact gap this PR sets out to fix (a ## Related heading or a No linked issue/closing-keyword string written only for illustration inside code now can't satisfy the gate). However, the fence detector (/^ {0,3}(\{3,}|~{3,})(.*)$/, [L151](https://github.com/melodic-software/ci-workflows/blob/a2ced9509a5d869250a463a252a99b78310bb915/.github/workflows/pr-issue-linkage.yml#L151)) only recognizes fenced code — there's no handling for CommonMark's 4-space-indented code block form. A PR body containing a 4-space-indented No linked issue/## Related/Closes #123line renders as a literal code block on GitHub (not live text/metadata) but is **not** masked here, so it still flows through toCLOSING_KEYWORD/NO_ISSUE_MARKER/extractSection` unmasked and can satisfy the gate.

This isn't a regression introduced by this diff (indented blocks were never handled, before or after), and the practical exploitability is limited — a PR author fully controls their own body and could just as easily satisfy the gate with plain unformatted text, so this isn't a privilege-escalation-style bug, just a residual bypass of the exact "fail-closed for non-live markers" property this PR's description claims to restore. Worth a follow-up if the intent is to fully close this class rather than just the fenced/inline-code instances.

Fix this →

Verified as sound

The two changed hunks correctly gate rendered += on inlineTicks === null (L198, L209) and replace fenced-block line output with "" (L153, L169) while still pushing one output line per input line — line indices stay aligned, so extractSection's heading-level scan isn't thrown off.
Line-count preservation means the unterminated-comment fail-closed behavior and the existing "stray backtick doesn't hide later metadata" test both remain intact under the new masking.
No new injection surface: PR_BODY/PR_AUTHOR/EXEMPT_AUTHORS are still env:-only inputs into JS logic, never interpolated into run: shell text.
Test additions are purely additive assertions against existing runScript harness; no security-relevant surface.

No CRITICAL or IMPORTANT issues found.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2ced9509a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-actions · 2026-08-05T19:45:11Z

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

claude · 2026-08-05T19:48:09Z

Claude finished @kyle-sexton's task in 1m 54s —— View job

Security Review — PR #355

Gather context (diff since last review, changed files)
Review latest commit against prior findings (indented-code masking, closing-tick leak)
Review .github/workflows/pr-issue-linkage.yml (full, for permissions/trigger/injection surface)
Review .github/scripts/pr-issue-linkage.test.cjs
Post findings

Scope

This is a re-review triggered by a new commit (fa0a263) pushed after my prior security review of a2ced95. Diffed a2ced95..fa0a263 (gh pr diff / git diff) and re-read the full stripRenderedHtmlComments function and its call site in pr-issue-linkage.yml end-to-end, plus the additive test in pr-issue-linkage.test.cjs. Trust boundary is unchanged from the prior review: untrusted github.event.pull_request.body text vs. the linkage-gate decision.

What changed in this commit

Two non-security correctness gaps flagged in code review comments on the prior commit are both fixed here:

Indented (4-space/tab) code blocks are now masked — L172-L175 blanks a line starting with 4 spaces or a tab (when not already inside a fence/inline span), closing the gap flagged by both the Codex review and my prior IMPORTANT/SUGGESTION finding, where an indented No linked issue / ## Related / - #123 block could satisfy the gate unmasked.
Closing inline-code backtick no longer leaks into the masked text — L195 now captures wasInline (state before this tick-run updates inlineTicks) and gates the append on !wasInline && inlineTicks === null (L203), so a tick-run that opened or closed a span is never appended. Hand-traced `code` and code again: both the opening and closing delimiters are now fully suppressed, fixing the asymmetry from my earlier finding.

Security assessment of this commit

No permissions:, trigger, or uses: pin changes — permissions: {} and the workflow_call-only trigger shape are untouched.
PR_BODY/PR_AUTHOR/EXEMPT_AUTHORS remain env:-only inputs into pure JS string logic (L86-L92); nothing is spliced into run:/shell text, so there's no new script-injection surface.
The new indent regex (/^(?: {4}|\t)/) is a simple anchored check — no ReDoS potential. hasClosingTickRun's pre-existing nested scan is unchanged by this commit and is bounded by GitHub's PR-body size limit, not attacker-adjustable beyond that.
Net effect of both fixes is a strictly tighter fail-closed mask (more content is now blanked, never less), consistent with this gate's intended fail-closed posture — no bypass was widened.
Test addition is purely additive (runScript assertions only); no security-relevant surface.

Verdict

No CRITICAL, IMPORTANT, or SUGGESTION security issues in this commit. Both non-blocking gaps identified in the prior review round (indented-code masking, closing-tick leak) are resolved, and no new attack surface was introduced.

No linked issue ## Summary - approve the exact `ci-workflows` v0.10.2 SHA in runner policy - deploy the shared PR issue linkage caller at that SHA - document the parser hardening that masks inline, fenced, and indented code while preserving real linkage metadata - retain the existing contract, permissions, secret flow, and reusable-workflow routing ## Verification - `actionlint .github/workflows/pr-issue-linkage.yml` - `python components/runner-policy/scripts/lint_runner_policy.py` - `python -m unittest discover -s components/runner-policy/tests` (242 tests) - `markdownlint README.md` - generated-distribution diff check ## Related - melodic-software/ci-workflows#355 - #327 - melodic-software/claude-code-plugins#1956 - `ci-workflows` release `v0.10.2`

No linked issue ## Summary Move the locally owned PR-linkage caller to the reviewed `ci-workflows` v0.10.2 release. This deploys the Markdown-aware parser that ignores linkage-like text inside inline, fenced, and indented code while preserving real HTML-comment metadata and the existing exact Dependabot exemption. The exact reusable-workflow SHA is approved by the synced runner policy from `standards@0d0c144`. ## Verification - `actionlint .github/workflows/pr-issue-linkage.yml` - `node .github/standards/runner-policy/runner-policy.mjs --root .` - `git diff --check` ## Related - melodic-software/ci-workflows#355 - melodic-software/standards#328 - #1957 - claude-code-plugins#1933

## Summary The scheduled `claude-lanes-repin` job has resolved v0.10.2 every day since 2026-08-05 and opened nothing: no re-pin App credential is provisioned, so it warns and directs a re-pin by hand. This is that hand re-pin, with the component rewrite performed by `repin-callers.sh apply` rather than by editing the pins directly. v0.10.2 is `e94438746c300b02385a7f8a2a2dcd19a7f4ad4a` (lightweight tag, ref points straight at the commit). ## Contract surface Both lane reusables carry an unchanged workflow-call contract against v0.9.1: identical input names, identical secret key set and required-ness, and no change to `runs-on` or the caller-permission surface. `select-runner.yml` is byte-identical between the two tags. The new `approvedReusableWorkflowContracts` entries are therefore verbatim copies of their `c136b27f` predecessors under a new SHA key, and the selector reference is owner-scoped as before. The lane deltas are internal to the reusables: the review lane moves its inline-comment tool grant out of the caller-replaceable `claude-args` default into the compose step, adds a scoped `Bash(gh pr diff:*)` grant, routes findings to line-anchored review comments, and narrows its retry gate to count real assistant turns. ## Scope `standards` is the manifest source, not a sync target, so its repo-local `.github/workflows/claude-review.yml` is re-pinned here alongside the components. The managed consumers (dotfiles, github-iac, medley, provisioning) receive the bump through the normal sync cascade; the one locally-owned caller set (claude-code-plugins) is re-pinned in its own repository. Ref: melodic-software/ci-workflows#355 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

) ## Summary Re-pins this fixture's Claude review caller from ci-workflows v0.9.1 (`c136b27f`) to v0.10.2 (`e94438746c300b02385a7f8a2a2dcd19a7f4ad4a`). The workflow's own header records why this is a hand re-pin: > LOCALLY OWNED — not a sync-manifest target and not a standards-managed file. > This fixture is deliberately outside the caller wave […] Repin it by hand > when the fleet pin moves. ## Contract surface The reusable's input names, secret key set, `runs-on`, and caller-permission surface are unchanged against v0.9.1, so the caller needs no edit beyond the pin line. ## No policy dependency Unlike the claude-code-plugins caller bump, this repository carries no `.github/standards/runner-policy/` materialization and no runner-policy gate, so this PR has no ordering dependency on the standards policy approval and can merge on its own. ## Related No linked issue. This PR closes nothing. Related, not closed: - melodic-software/standards#337 — the policy approval and component re-pin. - melodic-software/claude-code-plugins#1990 — the locally-owned caller bump (blocked on the sync). - melodic-software/ci-workflows#355 — the change released as v0.10.2. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…337) ## Summary Re-pins the Claude review and security lane callers from ci-workflows v0.9.1 (`c136b27f`) to v0.10.2 (`e94438746c300b02385a7f8a2a2dcd19a7f4ad4a`), and approves that revision in `runner-policy`. The scheduled `claude-lanes-repin` job has resolved v0.10.2 on every run since 2026-08-05 and opened nothing — no re-pin App credential is provisioned, so it warns and directs a re-pin by hand: > `components/claude-lanes/` is behind ci-workflows v0.10.2, but no re-pin App > credential is configured, so no pull request was opened. […] Until then, > re-pin by hand. This is that hand re-pin. The component rewrite was produced by `repin-callers.sh apply v0.10.2 <sha>` — the same subject the scheduled job runs — rather than by editing the pins directly, so the script's own count guard covered the rewrite. ## Contract surface Both lane reusables carry an **unchanged** workflow-call contract against v0.9.1: - identical input names on `claude-review.yml` and `claude-security-review.yml` - identical secret key set and `required:` flags - no change to `runs-on` or to any caller-permission surface - `select-runner.yml` is **byte-identical** between the two tags The new `approvedReusableWorkflowContracts` entries are therefore verbatim copies of their `c136b27f` predecessors under a new SHA key, and the selector reference stays owner-scoped to `melodic-software`. The lane deltas are internal to the reusables: the review lane moves its inline-comment tool grant out of the caller-replaceable `claude-args` default into the compose step (so replacing that default can no longer drop it), adds a scoped `Bash(gh pr diff:*)` grant in its place, routes findings to line-anchored review comments, and narrows its retry gate to count real assistant turns. The one security-relevant delta — routing every line-anchorable finding through the inline-comment tool widens the surface on which untrusted diff content reaches a rendered comment carrying a one-click-applicable suggestion block — sits inside the reusable, not on the contract, and is recorded in the runner-policy README alongside the `contents: read` precedent. ## Scope `standards` is the manifest **source**, not a sync target, so its repo-local `.github/workflows/claude-review.yml` is re-pinned here alongside the components. - Managed consumers (dotfiles, github-iac, medley, provisioning) receive the bump through the normal sync cascade — not hand-edited. - The one `locally-owned` caller set (claude-code-plugins) is re-pinned in its own repository. - `claude-lane-sandbox` is a hand-wired fixture and is re-pinned separately. ## Test plan - `components/runner-policy`: `node --test runner-policy.test.mjs` — 242/242 pass. The selector-allowlist assertion required the new revision's constant (`INLINE_COMMENT_LANE_SHA`) and list entry; that is the only test change. - `npm run lint:runner-policy` (the self-gate CI runs) — `Runner policy passed.` - `harness/shell/run-tests.sh` — 23 passed, 0 failed, 3 skipped. This includes `components/claude-lanes/claude-lanes.test.sh` (30/30), which materializes the re-pinned component into every managed consumer and actionlints the result. ## Related No linked issue. This PR closes nothing. Related, not closed: - melodic-software/ci-workflows#355 — the change released as v0.10.2. - melodic-software/claude-code-plugins#1990 — the locally-owned caller bump, which is BLOCKED until this PR merges and its sync PR delivers the updated `policy.json` there. - melodic-software/claude-lane-sandbox#3 — the hand-wired fixture bump (independent; no policy dependency). Operator note: no `CLAUDE_LANES_REPIN_APP_CLIENT_ID` / `CLAUDE_LANES_REPIN_APP_PRIVATE_KEY` is provisioned, which is why this re-pin is manual. Provisioning them would let the scheduled job open this PR itself. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…1990) > [!WARNING] > **Do not merge before melodic-software/standards#337 has merged AND its sync > PR has landed here.** CI on this branch is expected RED until then — see > "Ordering" below. The `do-not-merge` label is applied deliberately. ## Summary Re-pins both Claude lane callers from ci-workflows v0.9.1 (`c136b27f`) to v0.10.2 (`e94438746c300b02385a7f8a2a2dcd19a7f4ad4a`). These two callers are `locally-owned` in the standards sync manifest, not managed — this is the org's one PUBLIC caller target, and runner-policy forbids a public repository from referencing the governed `select-runner` indirection — so the pins are bumped here rather than arriving by sync. Neither reusable changes its input names, secret key set, `runs-on`, or caller-permission surface against v0.9.1, so no caller edit beyond the pin is required. ## Ordering — this PR is blocked `.github/standards/runner-policy/policy.json` is a MANAGED materialization and is deliberately **not** touched here. It still approves only `c136b27f`, and the gate fails closed on the new SHA. Measured locally on this branch: ``` $ node .github/standards/runner-policy/runner-policy.mjs --root . .github/workflows/claude-review.yml#review: runner-target-contract: the reusable workflow path@SHA has no reviewed runner-input contract (auto-approval declined: inputs changed since the previously reviewed ...@1d3762c2) .github/workflows/claude-security-review.yml#security-review: runner-target-contract: the reusable workflow path@SHA has no reviewed runner-input contract (auto-approval declined: inputs changed since the previously reviewed ...@66073e58) exit 1 ``` The same command on the unmodified base exits `0`, so this is caused by the pin bump and not pre-existing. melodic-software/standards#337 adds the v0.10.2 contracts to the policy source. Merging it triggers the sync cascade (`sync.yml` runs on `push` to `main`), which delivers the updated `policy.json` here. Once that sync PR merges, re-run CI on this branch and it goes green. ## Test plan - `node .github/standards/runner-policy/runner-policy.mjs --root .` — currently exits 1 by design (above); expected to exit 0 after the policy sync lands. - No other repository check is affected: the change is two `uses:` pin lines. ## Related No linked issue. This PR closes nothing. Related, not closed: - melodic-software/standards#337 — the policy approval and component re-pin this PR depends on. - melodic-software/ci-workflows#355 — the change released as v0.10.2. - melodic-software/claude-lane-sandbox#3 — the hand-wired fixture bump (independent; no policy dependency). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

fix(pr-issue-linkage): ignore markers in code

a2ced95

chatgpt-codex-connector Bot reviewed Aug 5, 2026

View reviewed changes

Comment thread .github/workflows/pr-issue-linkage.yml

claude Bot reviewed Aug 5, 2026

View reviewed changes

Comment thread .github/workflows/pr-issue-linkage.yml Outdated

fix(pr-issue-linkage): mask all markdown code

fa0a263

kyle-sexton merged commit e944387 into main Aug 5, 2026
38 checks passed

kyle-sexton deleted the codex/pr-linkage-mask-code branch August 5, 2026 19:52

kyle-sexton mentioned this pull request Aug 5, 2026

fix(runner-policy): approve PR linkage v0.10.2 melodic-software/standards#328

Merged

kyle-sexton mentioned this pull request Aug 5, 2026

fix(ci): deploy linkage parser v0.10.2 melodic-software/claude-code-plugins#1956

Merged

claude Bot mentioned this pull request Aug 6, 2026

chore(ci): repin PR linkage self-check to v0.10.2 #356

Merged

kyle-sexton mentioned this pull request Aug 8, 2026

question: v0.10.2 claude-args asymmetry — review lane lost inline-comment tool, security lane kept it #382

Closed

Navigation Menu

Uh oh!

fix(pr-issue-linkage): ignore markers in code - #355

fix(pr-issue-linkage): ignore markers in code#355
kyle-sexton merged 2 commits into
mainfrom
codex/pr-linkage-mask-code

kyle-sexton commented Aug 5, 2026

Uh oh!

cursor Bot commented Aug 5, 2026

Uh oh!

claude Bot commented Aug 5, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 5, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 5, 2026

Uh oh!

claude Bot commented Aug 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kyle-sexton commented Aug 5, 2026

Summary

Verification

Related

Uh oh!

cursor Bot commented Aug 5, 2026

Uh oh!

claude Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review

Summary

Findings

Verification note

Uh oh!

claude Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review — PR #355

Scope

Findings

Verified as sound

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 5, 2026

Uh oh!

claude Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review — PR #355

Scope

What changed in this commit

Security assessment of this commit

Verdict

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Aug 5, 2026 •

edited

Loading

claude Bot commented Aug 5, 2026 •

edited

Loading

claude Bot commented Aug 5, 2026 •

edited

Loading