Harden CI supply chain and add an org-wide security policy - #275
Merged
Conversation
Addresses findings from a security review of this repository. These workflows are consumed by ~45 repositories in the organization, so changes here apply org-wide. Rebased onto #274, which independently pinned actionlint by digest and added `--fail` to the two curl calls in the code quality workflow. Those parts of the review are covered by that change and are dropped here; #274's actionlint pin resolves to the linux/amd64 manifest of v1.7.11, which is correct for the GitHub-hosted runners these jobs use. Add SECURITY.md This repository provides the organization's default community health files, but had no security policy, so no wp-cli repository surfaced a "Report a vulnerability" path. Points at the WP-CLI handbook and the WordPress HackerOne program rather than restating policy that lives elsewhere. Scope `actions: write` to the job that needs it It was declared at workflow level, so it also reached `triage-new-item`, which runs on `pull_request_target` and processes pull request titles and bodies written by anyone who can open a PR. `actions: write` permits dispatching workflows and deleting caches and artifacts, a known lateral-movement path. Only `triage-unlabeled-items` needs it, and that job is `workflow_dispatch`-only. The grant has to stay in the caller, since a caller can only cap a reusable workflow's permissions and never raise them; removing it there is what broke dispatching in #271 and prompted the revert in #272. Comments in both files record this so the next attempt narrows rather than removes. Pin gherkin-lint `npx --yes gherkin-lint` resolved and executed the newest publish on every run. The package was last released in December 2023 and has two maintainers, so a single account compromise would reach CI in every repository. Correct the actions/setup-node pin comment The pinned SHA is v7.0.0, but the trailing comment read v6. The pin itself is immutable and current; the comment is what reviewers read, so a wrong one quietly defeats the point of the convention. Fail the WP-CLI download loudly `curl -O` without `-f` writes the error body to the file and still exits 0, so an outage installed an HTML page as /usr/local/bin/wp and surfaced as a confusing failure much later. Reduce blast radius of the workflow sync SKIP_DELETE stops a pattern change from deleting files across every target repository. The FILE_PATTERNS regexes now escape their dots so they match only the intended paths. Both jobs drop to `contents: read`: the sync authenticates with ACTIONS_BOT, and the action reads that token from its `with:` input and makes no API calls, so GITHUB_TOKEN is never used at all.
📝 WalkthroughWalkthroughChangesGitHub Actions hardening
Repository security policy
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the organization-wide reusable GitHub Actions workflows in wp-cli/.github and adds a default security policy so downstream wp-cli/* repositories surface a consistent vulnerability reporting path.
Changes:
- Added an org-wide
SECURITY.mdthat directs reporters to the WordPress HackerOne program and the WP-CLI handbook. - Reduced token blast radius by scoping
actions: writeto only the dispatching triage job and dropping workflow-sync jobs tocontents: read. - Hardened CI execution by pinning
gherkin-lintto a specific version and making the WP-CLI download fail loudly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
SECURITY.md |
Introduces a default, org-wide security reporting policy and scope statement. |
.github/workflows/sync-workflows.yml |
Escapes regex file patterns, prevents deletions via SKIP_DELETE, and reduces permissions to contents: read. |
.github/workflows/reusable-regenerate-readme.yml |
Makes WP-CLI download fail on HTTP errors to avoid silently installing error pages. |
.github/workflows/reusable-issue-triage.yml |
Removes workflow-level actions: write and scopes it to the triage-unlabeled-items dispatch job only. |
.github/workflows/reusable-code-quality.yml |
Corrects the setup-node pin comment and pins gherkin-lint to a fixed version. |
.github/workflows/issue-triage.yml |
Documents why actions: write must remain granted in the caller while being narrowed in the reusable workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/sync-workflows.yml (1)
110-112: 🗄️ Data Integrity & Integration | 🔵 TrivialAdd a cleanup path for
SKIP_DELETE.
SKIP_DELETE: truemakes both jobs add or update files only. If a source workflow or Dependabot file is removed or renamed, the old file remains in every target repository. This can retain an obsolete or insecure workflow.Keep the safe default, but add a drift report or an explicit cleanup allowlist.
Also applies to: 178-180
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/sync-workflows.yml around lines 110 - 112, Update the workflow synchronization jobs using SKIP_DELETE so they retain the safe no-delete default while detecting source files removed or renamed in target repositories. Add either a drift report that identifies stale files or an explicit cleanup allowlist permitting approved deletions, and apply this consistently to both workflow and Dependabot synchronization paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/reusable-regenerate-readme.yml:
- Around line 49-52: Update the WP-CLI PHAR download step in the workflow to use
an immutable commit or tag instead of the mutable gh-pages reference, then
verify the downloaded artifact with a trusted out-of-band checksum or signature
before installing it as /usr/local/bin/wp. Ensure verification failure stops the
workflow and prevents installation.
---
Nitpick comments:
In @.github/workflows/sync-workflows.yml:
- Around line 110-112: Update the workflow synchronization jobs using
SKIP_DELETE so they retain the safe no-delete default while detecting source
files removed or renamed in target repositories. Add either a drift report that
identifies stale files or an explicit cleanup allowlist permitting approved
deletions, and apply this consistently to both workflow and Dependabot
synchronization paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b9996aa0-8086-4bc6-b04d-30d2a56431cf
📒 Files selected for processing (6)
.github/workflows/issue-triage.yml.github/workflows/reusable-code-quality.yml.github/workflows/reusable-issue-triage.yml.github/workflows/reusable-regenerate-readme.yml.github/workflows/sync-workflows.ymlSECURITY.md
Comment on lines
+49
to
+52
| # `-f` matters most here: without it curl writes the error body to the | ||
| # file and still exits 0, so an outage silently installs an HTML page | ||
| # as /usr/local/bin/wp and the failure surfaces much later. | ||
| curl -fsSL -O https://raw.githubusercontent.com/wp-cli/builds/gh-pages/phar/wp-cli-nightly.phar |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file=".github/workflows/reusable-regenerate-readme.yml"
if rg -n 'raw\.githubusercontent\.com/wp-cli/builds/(gh-pages|main|master)/' "$file"; then
echo "Mutable WP-CLI branch reference remains" >&2
exit 1
fi
if ! rg -n -q '(sha256|sha512|cosign|gpg)' "$file"; then
echo "No artifact integrity verification found" >&2
exit 1
fiRepository: wp-cli/.github
Length of output: 301
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file=".github/workflows/reusable-regenerate-readme.yml"
echo "== file exists and size =="
wc -l "$file" || true
echo
echo "== relevant workflow sections =="
sed -n '1,140p' "$file" | cat -n
echo
echo "== workflow references to this workflow and inputs =="
rg -n "reusable-regenerate-readme|persist-credentials|pull_request|permissions:|wp:" .github/workflows -S || trueRepository: wp-cli/.github
Length of output: 10430
Pin and verify the WP-CLI PHAR before installation.
This workflow has write permissions, persists checkout credentials, and runs wp directly. The download path still follows mutable gh-pages and does not check a digest or signature before moving the PHAR to /usr/local/bin/wp. Use an immutable commit/tag reference and verify an out-of-band checksum or signature before installation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/reusable-regenerate-readme.yml around lines 49 - 52,
Update the WP-CLI PHAR download step in the workflow to use an immutable commit
or tag instead of the mutable gh-pages reference, then verify the downloaded
artifact with a trusted out-of-band checksum or signature before installing it
as /usr/local/bin/wp. Ensure verification failure stops the workflow and
prevents installation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses findings from a security review of this repository. These workflows
are consumed by ~45 repositories in the organization, so changes here apply
org-wide.
Rebased onto #274, which independently pinned actionlint by digest and added
--failto the two curl calls in the code quality workflow. Those parts of thereview are covered by that change and are dropped here; #274's actionlint pin
resolves to the linux/amd64 manifest of v1.7.11, which is correct for the
GitHub-hosted runners these jobs use.
Add SECURITY.md
This repository provides the organization's default community health files,
but had no security policy, so no wp-cli repository surfaced a "Report a
vulnerability" path. Points at the WP-CLI handbook and the WordPress
HackerOne program rather than restating policy that lives elsewhere.
Scope
actions: writeto the job that needs itIt was declared at workflow level, so it also reached
triage-new-item,which runs on
pull_request_targetand processes pull request titles andbodies written by anyone who can open a PR.
actions: writepermitsdispatching workflows and deleting caches and artifacts, a known
lateral-movement path. Only
triage-unlabeled-itemsneeds it, and that jobis
workflow_dispatch-only.The grant has to stay in the caller, since a caller can only cap a reusable
workflow's permissions and never raise them; removing it there is what broke
dispatching in #271 and prompted the revert in #272. Comments in both files
record this so the next attempt narrows rather than removes.
Pin gherkin-lint
npx --yes gherkin-lintresolved and executed the newest publish on everyrun. The package was last released in December 2023 and has two maintainers,
so a single account compromise would reach CI in every repository.
Correct the actions/setup-node pin comment
The pinned SHA is v7.0.0, but the trailing comment read v6. The pin itself is
immutable and current; the comment is what reviewers read, so a wrong one
quietly defeats the point of the convention.
Fail the WP-CLI download loudly
curl -Owithout-fwrites the error body to the file and still exits 0,so an outage installed an HTML page as /usr/local/bin/wp and surfaced as a
confusing failure much later.
Reduce blast radius of the workflow sync
SKIP_DELETE stops a pattern change from deleting files across every target
repository. The FILE_PATTERNS regexes now escape their dots so they match
only the intended paths. Both jobs drop to
contents: read: the syncauthenticates with ACTIONS_BOT, and the action reads that token from its
with:input and makes no API calls, so GITHUB_TOKEN is never used at all.Summary by CodeRabbit
Security
Bug Fixes
Chores