From 39ad4539bfe8ed155dc56a4f9f09008f2d3c4d50 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:17:10 +0200 Subject: [PATCH 1/5] fix(workflows): prevent script injection via the dirtyLabel input The "Create label if it doesn't exist" step interpolated the caller-controlled `dirtyLabel` input directly into the `actions/github-script` body. Because this workflow is reusable and is typically called from a `pull_request_target` trigger, the step runs with a write-scoped token, so a label name crafted to break out of the surrounding string literal could execute arbitrary code in the context of the calling repository. Pass the input through the environment and read it via `process.env` instead, so the value is always treated as inert data rather than as part of the script. Reported by zizmor as template-injection (high severity). Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/reusable-merge-conflict-check.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/reusable-merge-conflict-check.yml b/.github/workflows/reusable-merge-conflict-check.yml index 538c8de..f4c78b5 100644 --- a/.github/workflows/reusable-merge-conflict-check.yml +++ b/.github/workflows/reusable-merge-conflict-check.yml @@ -40,12 +40,16 @@ jobs: steps: - name: "Create label if it doesn't exist" uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + # Passed via the environment, never interpolated into the script, as this is a + # caller-controlled input and interpolation would allow script injection. + DIRTY_LABEL: ${{ inputs.dirtyLabel }} with: script: | try { await github.rest.issues.createLabel({ ...context.repo, - name: "${{ inputs.dirtyLabel }}" + name: process.env.DIRTY_LABEL }); } catch(e) { // Ignore if labels exist already. From a244059b4e2098fced0ea5366ae1419119429561 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:17:48 +0200 Subject: [PATCH 2/5] feat(workflows): mark merge conflict comments as resolved once fixed The merge conflict check already removes the dirty label when a conflict is resolved, but the comment explaining the conflict stayed visible, leaving stale noise on the PR. Minimize it as RESOLVED instead, which collapses it the same way the "Hide comment" UI does. The check step's existing `prDirtyStatuses` output is reused to determine which PRs are no longer conflicting, so no extra merge state queries are needed. Only comments which were authored by this workflow, are not already minimized, and whose body matches `commentOnDirty` exactly are minimized, so human comments and unrelated bot comments are never hidden. Pagination stops when the API claims a further page without advancing the cursor, which would otherwise loop forever, and the job now has an explicit timeout as a second line of defense. Permissions are declared explicitly and default to none at the workflow level, so the job only gets the scopes it actually needs. The behavior is opt-out via the new `minimizeCommentOnClean` input. Co-Authored-By: Claude Opus 5 (1M context) --- .../reusable-merge-conflict-check.yml | 125 ++++++++++++++++++ README.md | 3 + 2 files changed, 128 insertions(+) diff --git a/.github/workflows/reusable-merge-conflict-check.yml b/.github/workflows/reusable-merge-conflict-check.yml index f4c78b5..de6d653 100644 --- a/.github/workflows/reusable-merge-conflict-check.yml +++ b/.github/workflows/reusable-merge-conflict-check.yml @@ -31,12 +31,29 @@ on: type: string required: false default: '' + minimizeCommentOnClean: + description: "Whether to mark the `commentOnDirty` comment as resolved once the merge conflict is resolved." + type: boolean + required: false + default: true + +# Default to no permissions at all; the job below opts in to only what it needs. +permissions: {} jobs: check-prs: name: Merge conflict check runs-on: ubuntu-latest + + # Safety net: the merge conflict check retries for at most ~10 minutes + # (retryAfter 120s * retryMax 5), so anything beyond this is a runaway job. + timeout-minutes: 20 + + permissions: + issues: write # Needed to create the dirty label and to add/remove labels on PRs. + pull-requests: write # Needed to comment and to mark those comments as resolved. + steps: - name: "Create label if it doesn't exist" uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 @@ -56,6 +73,7 @@ jobs: } - name: Check PRs for merge conflicts + id: check uses: eps1lon/actions-label-merge-conflict@0273be72a0bbd58fcd71d0d6c02c209b50d1e5e1 # v3.1.0 with: repoToken: ${{ secrets.GITHUB_TOKEN }} @@ -63,3 +81,110 @@ jobs: removeOnDirtyLabel: ${{ inputs.removeOnDirtyLabel }} commentOnDirty: ${{ inputs.commentOnDirty }} commentOnClean: ${{ inputs.commentOnClean }} + + - name: "Mark merge conflict comments as resolved for PRs which are no longer conflicting" + if: ${{ inputs.minimizeCommentOnClean && inputs.commentOnDirty != '' }} + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + # Passed via the environment, never interpolated into the script, as these are + # caller-controlled inputs and interpolation would allow script injection. + DIRTY_STATUSES: ${{ steps.check.outputs.prDirtyStatuses }} + COMMENT_ON_DIRTY: ${{ inputs.commentOnDirty }} + with: + script: | + const dirtyStatuses = JSON.parse(process.env.DIRTY_STATUSES || '{}'); + const commentOnDirty = process.env.COMMENT_ON_DIRTY; + + // Only PRs which the check found to be conflict-free need cleaning up. + const cleanPrNumbers = Object.entries(dirtyStatuses) + .filter(([, isDirty]) => isDirty === false) + .map(([number]) => Number(number)); + + if (cleanPrNumbers.length === 0) { + core.info('No conflict-free PRs to clean up.'); + return; + } + + const query = ` + query prComments($owner: String!, $repo: String!, $number: Int!, $after: String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + comments(first: 100, after: $after) { + nodes { + id + body + isMinimized + viewerDidAuthor + } + pageInfo { + endCursor + hasNextPage + } + } + } + } + } + `; + + const mutation = ` + mutation minimize($id: ID!) { + minimizeComment(input: { subjectId: $id, classifier: RESOLVED }) { + minimizedComment { + isMinimized + } + } + } + `; + + for (const number of cleanPrNumbers) { + let after = null; + let hasNextPage = true; + + while (hasNextPage) { + const response = await github.graphql(query, { + ...context.repo, + number, + after, + }); + + const { nodes, pageInfo } = response.repository.pullRequest.comments; + + // Guard against a malformed response which claims another page but hands + // back no cursor, or the same cursor again. Either would have us re-request + // the same page forever. Checked before processing so a stuck cursor cannot + // cause the same page to be handled twice. + const cursorStuck = + pageInfo.hasNextPage && + (!pageInfo.endCursor || pageInfo.endCursor === after); + + if (cursorStuck) { + core.warning( + `Pagination for PR #${number} did not advance; stopping after this page to avoid an endless loop.` + ); + } + + // Only minimize this workflow's own, not yet minimized, conflict comments. + // Matching on the exact comment body prevents unrelated comments from + // being hidden if the bot posted other comments on the same PR. + const staleComments = nodes.filter( + (comment) => + comment.viewerDidAuthor && + !comment.isMinimized && + comment.body.trim() === commentOnDirty.trim() + ); + + for (const comment of staleComments) { + try { + await github.graphql(mutation, { id: comment.id }); + core.info(`Marked comment ${comment.id} on PR #${number} as resolved.`); + } catch (e) { + core.warning( + `Could not minimize comment ${comment.id} on PR #${number}: ${e.message}` + ); + } + } + + hasNextPage = pageInfo.hasNextPage && !cursorStuck; + after = pageInfo.endCursor; + } + } diff --git a/README.md b/README.md index e1d6ac2..d0e7f44 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,9 @@ The following re-usable workflows are available: Defaults to: _"A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch."_. - `commentOnClean`: Optional. Comment to add when the pull request is not conflicting anymore. Supports markdown. Defaults to _no comment_. + - `minimizeCommentOnClean`: Optional. Whether to mark the `commentOnDirty` comment as resolved (collapsed) once the + merge conflict has been resolved. Only comments posted by this workflow which exactly match the `commentOnDirty` + text are minimized. Has no effect when `commentOnDirty` is empty. Defaults to 'true'. ## A .github repository with versioning ? From f20213590f95888d35ecae19ce5126efd7c3ea4e Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:03:26 +0200 Subject: [PATCH 3/5] docs(workflows): document the comment matching limitation and token scope Code review raised two assumptions worth recording. Minimizing relies on an exact match against `commentOnDirty`, so editing that input leaves comments which were posted with the previous wording visible instead of collapsing them. Mention this in the README so the behavior is not surprising. Also note that `viewerDidAuthor` filters on the identity behind the token, which is the default GITHUB_TOKEN here, so a future change passing a PAT or app token to that step would widen which comments get minimized. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/reusable-merge-conflict-check.yml | 4 ++++ README.md | 2 ++ 2 files changed, 6 insertions(+) diff --git a/.github/workflows/reusable-merge-conflict-check.yml b/.github/workflows/reusable-merge-conflict-check.yml index de6d653..76bb119 100644 --- a/.github/workflows/reusable-merge-conflict-check.yml +++ b/.github/workflows/reusable-merge-conflict-check.yml @@ -166,6 +166,10 @@ jobs: // Only minimize this workflow's own, not yet minimized, conflict comments. // Matching on the exact comment body prevents unrelated comments from // being hidden if the bot posted other comments on the same PR. + // Note that `viewerDidAuthor` is scoped to the identity behind the token, + // which is the default GITHUB_TOKEN (github-actions[bot]) here. Passing a + // PAT or app token to this step would widen this to that identity's + // comments, so don't set `github-token` without revisiting this filter. const staleComments = nodes.filter( (comment) => comment.viewerDidAuthor && diff --git a/README.md b/README.md index d0e7f44..298f8fb 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,8 @@ The following re-usable workflows are available: - `minimizeCommentOnClean`: Optional. Whether to mark the `commentOnDirty` comment as resolved (collapsed) once the merge conflict has been resolved. Only comments posted by this workflow which exactly match the `commentOnDirty` text are minimized. Has no effect when `commentOnDirty` is empty. Defaults to 'true'. + Note that changing `commentOnDirty` will leave already posted comments unmatched, so those will stay visible + instead of being collapsed. ## A .github repository with versioning ? From 5a8eeb338652a39753457bc55ff0a0f625761b21 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:11:07 +0200 Subject: [PATCH 4/5] perf(workflows): scan PR comments newest first when minimizing The conflict comment is usually one of the most recent comments on a PR, but the scan walked the comment history from the oldest comment forward, so PRs with a long history needed several requests to reach it. Walk the comments backwards instead, which normally finds the comment in the first page. The endless loop guard now tracks `startCursor`/`hasPreviousPage` accordingly. Co-Authored-By: Claude Opus 5 (1M context) --- .../reusable-merge-conflict-check.yml | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/.github/workflows/reusable-merge-conflict-check.yml b/.github/workflows/reusable-merge-conflict-check.yml index 76bb119..a8584aa 100644 --- a/.github/workflows/reusable-merge-conflict-check.yml +++ b/.github/workflows/reusable-merge-conflict-check.yml @@ -105,11 +105,14 @@ jobs: return; } + // Walk the comments newest first, as the conflict comment is usually one of the + // most recent ones. On PRs with a long history this normally finds it in the + // first page instead of paging through the entire comment history. const query = ` - query prComments($owner: String!, $repo: String!, $number: Int!, $after: String) { + query prComments($owner: String!, $repo: String!, $number: Int!, $before: String) { repository(owner: $owner, name: $repo) { pullRequest(number: $number) { - comments(first: 100, after: $after) { + comments(last: 100, before: $before) { nodes { id body @@ -117,8 +120,8 @@ jobs: viewerDidAuthor } pageInfo { - endCursor - hasNextPage + startCursor + hasPreviousPage } } } @@ -137,14 +140,14 @@ jobs: `; for (const number of cleanPrNumbers) { - let after = null; - let hasNextPage = true; + let before = null; + let hasPreviousPage = true; - while (hasNextPage) { + while (hasPreviousPage) { const response = await github.graphql(query, { ...context.repo, number, - after, + before, }); const { nodes, pageInfo } = response.repository.pullRequest.comments; @@ -154,8 +157,8 @@ jobs: // the same page forever. Checked before processing so a stuck cursor cannot // cause the same page to be handled twice. const cursorStuck = - pageInfo.hasNextPage && - (!pageInfo.endCursor || pageInfo.endCursor === after); + pageInfo.hasPreviousPage && + (!pageInfo.startCursor || pageInfo.startCursor === before); if (cursorStuck) { core.warning( @@ -188,7 +191,7 @@ jobs: } } - hasNextPage = pageInfo.hasNextPage && !cursorStuck; - after = pageInfo.endCursor; + hasPreviousPage = pageInfo.hasPreviousPage && !cursorStuck; + before = pageInfo.startCursor; } } From c1d50858e78b0cfa4a72e572d159455f1d20b28a Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:14:42 +0200 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .github/workflows/reusable-merge-conflict-check.yml | 4 ++-- README.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/reusable-merge-conflict-check.yml b/.github/workflows/reusable-merge-conflict-check.yml index a8584aa..1215713 100644 --- a/.github/workflows/reusable-merge-conflict-check.yml +++ b/.github/workflows/reusable-merge-conflict-check.yml @@ -167,8 +167,8 @@ jobs: } // Only minimize this workflow's own, not yet minimized, conflict comments. - // Matching on the exact comment body prevents unrelated comments from - // being hidden if the bot posted other comments on the same PR. + // Matching on the comment body (after trimming leading/trailing whitespace) + // prevents unrelated comments from being hidden if the bot posted other comments on the same PR. // Note that `viewerDidAuthor` is scoped to the identity behind the token, // which is the default GITHUB_TOKEN (github-actions[bot]) here. Passing a // PAT or app token to this step would widen this to that identity's diff --git a/README.md b/README.md index 298f8fb..5a0d893 100644 --- a/README.md +++ b/README.md @@ -33,8 +33,8 @@ The following re-usable workflows are available: - `commentOnClean`: Optional. Comment to add when the pull request is not conflicting anymore. Supports markdown. Defaults to _no comment_. - `minimizeCommentOnClean`: Optional. Whether to mark the `commentOnDirty` comment as resolved (collapsed) once the - merge conflict has been resolved. Only comments posted by this workflow which exactly match the `commentOnDirty` - text are minimized. Has no effect when `commentOnDirty` is empty. Defaults to 'true'. + merge conflict has been resolved. Only comments posted by this workflow whose body matches `commentOnDirty` + (ignoring leading/trailing whitespace) are minimized. Has no effect when `commentOnDirty` is empty. Defaults to 'true'. Note that changing `commentOnDirty` will leave already posted comments unmatched, so those will stay visible instead of being collapsed.