From bce48655f4c3b8a6a044724c8a65a5b36dccb617 Mon Sep 17 00:00:00 2001 From: goldenapples Date: Wed, 29 Jul 2026 16:20:46 -0400 Subject: [PATCH] Don't reprompt if there's already been a review on the current dependencies This action shouldn't be prompting multiple times for the same plugin changes. If a review has been dismissed or is already open, then it's not necessary to notify the user again on every commit. --- .../actions/plugin-security-review/README.md | 5 +++- .../actions/plugin-security-review/action.yml | 25 +++++++++++++------ .../plugin-security-review/changed-plugins.sh | 23 +++++++++++------ 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/.github/actions/plugin-security-review/README.md b/.github/actions/plugin-security-review/README.md index 8a89481..a6ce1b1 100644 --- a/.github/actions/plugin-security-review/README.md +++ b/.github/actions/plugin-security-review/README.md @@ -6,7 +6,10 @@ Third-party plugins and themes are typically gitignored and installed by Compose If the standard reports findings, the action posts a **Request changes** review (via `GITHUB_TOKEN`) naming each plugin and the exact command to re-run the scan locally. The job itself always exits successfully, but this review creates intentional friction by requiring a human to dismiss the review in order to unblock merge. This avoids hard-blocking CI on the false positives that third-party plugins routinely trigger, while still surfacing findings that may be real. -The review is keyed to the head commit SHA: re-running the workflow will not stack duplicate reviews for the same commit, but a subsequent commit that changes `composer.lock` again earns a fresh review. +The review is keyed to the set of changed packages, not to the head commit. The action fingerprints the changed `name@version@reference` identities and records that fingerprint in the review body as an HTML comment. If a review carrying the same fingerprint is already on the pull request, no second review is posted — so ordinary pushes to the branch will not re-request changes a human has already dismissed. Changing any package version or reference produces a different fingerprint, which is new code and earns a fresh review. + +> [!NOTE] +> Because the fingerprint covers the packages rather than the scan output, editing the security ruleset itself mid-PR will not re-post a review for packages already reported. Re-run the scan locally if you change the standard. Packages are located by checking the installer-paths roots commonly used across WordPress projects, `client-mu-plugins/`, `mu-plugins/`, `plugins/`, and `themes/`. diff --git a/.github/actions/plugin-security-review/action.yml b/.github/actions/plugin-security-review/action.yml index 30c8254..66c448d 100644 --- a/.github/actions/plugin-security-review/action.yml +++ b/.github/actions/plugin-security-review/action.yml @@ -40,10 +40,13 @@ runs: id: detect shell: bash run: | - bash "${{ github.action_path }}/changed-plugins.sh" "${{ github.event.pull_request.base.sha }}" > changed-plugins.txt + bash "${{ github.action_path }}/changed-plugins.sh" "${{ github.event.pull_request.base.sha }}" changed-identities.txt > changed-plugins.txt if [ -s changed-plugins.txt ]; then echo "Detected plugin changes:" cat changed-plugins.txt + # Hash of the exact package versions under review, used below to avoid re-reviewing + # a set of package changes this PR has already been told about. + echo "fingerprint=$( sha256sum changed-identities.txt | cut -d ' ' -f 1 )" >> "$GITHUB_OUTPUT" echo "found=true" >> "$GITHUB_OUTPUT" else echo "No third-party plugin additions or updates detected." @@ -89,25 +92,31 @@ runs: - name: Request manual security review if: steps.scan.outputs.clean == 'false' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + FINGERPRINT: ${{ steps.detect.outputs.fingerprint }} with: script: | const fs = require('fs'); const findings = fs.readFileSync('review-body.md', 'utf8').trim(); const { owner, repo } = context.repo; const pull_number = context.payload.pull_request.number; - const headSha = context.payload.pull_request.head.sha; - // Idempotency: one review per head commit. New commits re-trigger a - // review (new code must be re-verified); a human's dismissal of the - // review for the current commit is preserved across workflow re-runs. + // Idempotency: one review per set of package changes, identified by a marker + // carrying the fingerprint of the changed name@version@reference identities. + // Keying on the packages rather than on the head commit means an unrelated + // push to the branch does not re-request changes a human has already + // dismissed, while any further package change is new code that earns a + // fresh review. + const marker = ``; + const reviews = await github.paginate(github.rest.pulls.listReviews, { owner, repo, pull_number, }); const alreadyReviewed = reviews.some( - (r) => r.user?.login === 'github-actions[bot]' && r.commit_id === headSha + (r) => r.user?.login === 'github-actions[bot]' && r.body?.includes(marker) ); if (alreadyReviewed) { - core.info('This commit already has a bot review; not posting another.'); + core.info('These package changes have already been reviewed on this PR; not posting another review.'); return; } @@ -127,6 +136,8 @@ runs: '2. Inspect flagged areas and confirm the code is fit for purpose — distinguishing real vulnerabilities from false positives.', '3. **Dismiss this review** to record that you have verified it and to unblock merge.', docsUrl ? `\nFull process: ${docsUrl}` : '', + '', + marker, ].join('\n'); await github.rest.pulls.createReview({ diff --git a/.github/actions/plugin-security-review/changed-plugins.sh b/.github/actions/plugin-security-review/changed-plugins.sh index e66d8fa..d416b2a 100755 --- a/.github/actions/plugin-security-review/changed-plugins.sh +++ b/.github/actions/plugin-security-review/changed-plugins.sh @@ -15,12 +15,15 @@ # commit reference differs (i.e., we check whether branch-level package references like # `dev-main` without any specific version string resolve to a different commit SHA). # -# Usage: changed-plugins.sh -# Output: one plugin/theme directory path per line; empty if nothing changed. +# Usage: changed-plugins.sh [changed-identities-file] +# Output: one plugin/theme directory path per line; empty if nothing changed. If a second +# argument is given, the changed name@version@reference identities are also written +# to that path, so a caller can fingerprint exactly which package versions changed. set -euo pipefail -BASE_REF="${1:?Usage: changed-plugins.sh }" +BASE_REF="${1:?Usage: changed-plugins.sh [changed-identities-file]}" +IDENTITIES_FILE="${2:-}" # Identity = name@version@reference for each WordPress plugin/muplugin/theme package. wp_package_identities() { @@ -33,14 +36,20 @@ base_identities="$( git show "${BASE_REF}:composer.lock" 2>/dev/null | wp_packag head_identities="$( wp_package_identities < composer.lock | sort )" # Identities in HEAD but not in BASE = added, version-changed, or ref-changed. -changed_packages="$( +changed_identities="$( comm -13 \ <( printf '%s\n' "${base_identities}" ) \ - <( printf '%s\n' "${head_identities}" ) \ - | sed 's/@.*//' \ - | sort -u + <( printf '%s\n' "${head_identities}" ) )" +# The identity list is a fingerprint of the exact package versions under review, which the +# caller can use to recognize a set of changes it has already reported on. +if [ -n "${IDENTITIES_FILE}" ]; then + printf '%s\n' "${changed_identities}" > "${IDENTITIES_FILE}" +fi + +changed_packages="$( printf '%s\n' "${changed_identities}" | sed 's/@.*//' | sort -u )" + [ -z "${changed_packages}" ] && exit 0 # Map each "vendor/name" package to its installed directory.