ci: restore fork builds with checkout v6.1#7474
Conversation
|
The CI failures on this PR are expected and do not exercise this branch's workflow changes.
The branch is intentionally minimal: 11 fork-head checkout steps each add Please do not rerun CI or add a no-op commit to change this status; the updated |
Explicitly allow the 11 pull_request_target checkout steps that intentionally check out fork head SHAs under checkout v6.1. Disable credential persistence for the same untrusted working trees while preserving the existing runner, cache, container, permission, and Guix behavior. Co-Authored-By: Claude <noreply@anthropic.com>
bd84229 to
dd53780
Compare
|
🕓 Ready for review — 9 ahead in queue (commit dd53780) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd537806bf
ℹ️ 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".
| uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| allow-unsafe-pr-checkout: true |
There was a problem hiding this comment.
Keep fork code out of the package-write workflow
For fork PRs, build.yml invokes this reusable workflow from pull_request_target while granting packages: write; this opt-in now permits the fork-controlled commit to modify the CI Dockerfile and have docker/build-push-action publish the resulting arbitrary image to the repository's GHCR package. persist-credentials: false only removes Git's persisted credential and does not prevent the later docker/login-action from supplying GITHUB_TOKEN for the push. This turns any opened fork PR into a package-publication path; run untrusted heads in a read-only pull_request workflow, or keep the privileged workflow checked out at the trusted base revision.
Useful? React with 👍 / 👎.
WalkthroughThe pull-request checkout configuration is standardized across seven GitHub Actions workflows. Each updated Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/build-container.yml:
- Around line 43-45: Prevent fork-controlled code from running in privileged
pull_request_target jobs. In .github/workflows/build-container.yml at 43-45,
89-91, and 134-136, separate GHCR login/push from the untrusted Docker build.
Move the fork-controlled make/script/Guix steps to an unprivileged pull_request
workflow in .github/workflows/build-depends.yml at 54-56 and 138-140,
build-src.yml at 59-61, cache-depends-sources.yml at 25-27, lint.yml at 26-28,
test-src.yml at 38-40, and guix-build.yml at 30-32 and 91-93.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c34453d3-7aed-4a31-8f66-93c24562c369
📒 Files selected for processing (7)
.github/workflows/build-container.yml.github/workflows/build-depends.yml.github/workflows/build-src.yml.github/workflows/cache-depends-sources.yml.github/workflows/guix-build.yml.github/workflows/lint.yml.github/workflows/test-src.yml
| ref: ${{ github.event.pull_request.head.sha }} | ||
| allow-unsafe-pr-checkout: true | ||
| persist-credentials: false |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in .github/workflows/build-container.yml .github/workflows/build-depends.yml .github/workflows/build-src.yml .github/workflows/cache-depends-sources.yml .github/workflows/guix-build.yml .github/workflows/lint.yml .github/workflows/test-src.yml; do
echo "===== $f ====="
echo "--- triggers ---"
sed -n '1,120p' "$f" | sed -n '/^on:/,/^[^[:space:]-]/p'
echo "--- cited regions ---"
case "$f" in
.github/workflows/build-container.yml)
sed -n '1,180p' "$f" | nl -ba | sed -n '35,50p;80,95p;128,142p'
;;
.github/workflows/build-depends.yml)
sed -n '1,220p' "$f" | nl -ba | sed -n '48,62p;132,146p'
;;
.github/workflows/build-src.yml)
sed -n '1,140p' "$f" | nl -ba | sed -n '52,68p'
;;
.github/workflows/cache-depends-sources.yml)
sed -n '1,90p' "$f" | nl -ba | sed -n '18,32p'
;;
.github/workflows/guix-build.yml)
sed -n '1,170p' "$f" | nl -ba | sed -n '24,38p;85,99p'
;;
.github/workflows/lint.yml)
sed -n '1,90p' "$f" | nl -ba | sed -n '20,34p'
;;
.github/workflows/test-src.yml)
sed -n '1,110p' "$f" | nl -ba | sed -n '32,44p'
;;
esac
echo
doneRepository: dashpay/dash
Length of output: 1057
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show each workflow trigger and the job steps around the referenced checkout blocks.
for f in .github/workflows/build-container.yml .github/workflows/build-depends.yml .github/workflows/build-src.yml .github/workflows/cache-depends-sources.yml .github/workflows/guix-build.yml .github/workflows/lint.yml .github/workflows/test-src.yml; do
echo "===== $f ====="
echo "--- trigger and job names ---"
python3 - <<'PY' "$f"
import sys, yaml, pathlib
path = pathlib.Path(sys.argv[1])
data = yaml.safe_load(path.read_text())
print(data.get("on"))
print("jobs:", list(data.get("jobs", {}).keys()))
PY
echo "--- cited regions ---"
case "$f" in
.github/workflows/build-container.yml)
nl -ba "$f" | sed -n '35,50p;80,95p;128,142p'
;;
.github/workflows/build-depends.yml)
nl -ba "$f" | sed -n '48,62p;132,146p'
;;
.github/workflows/build-src.yml)
nl -ba "$f" | sed -n '52,68p'
;;
.github/workflows/cache-depends-sources.yml)
nl -ba "$f" | sed -n '18,32p'
;;
.github/workflows/guix-build.yml)
nl -ba "$f" | sed -n '24,38p;85,99p'
;;
.github/workflows/lint.yml)
nl -ba "$f" | sed -n '20,34p'
;;
.github/workflows/test-src.yml)
nl -ba "$f" | sed -n '32,44p'
;;
esac
echo
doneRepository: dashpay/dash
Length of output: 354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the workflow sections directly in case Python YAML isn't available.
for f in .github/workflows/build-container.yml .github/workflows/build-depends.yml .github/workflows/build-src.yml .github/workflows/cache-depends-sources.yml .github/workflows/guix-build.yml .github/workflows/lint.yml .github/workflows/test-src.yml; do
echo "===== $f ====="
echo "--- first 80 lines ---"
nl -ba "$f" | sed -n '1,80p'
echo "--- around cited lines ---"
case "$f" in
.github/workflows/build-container.yml)
nl -ba "$f" | sed -n '35,50p;80,95p;128,142p'
;;
.github/workflows/build-depends.yml)
nl -ba "$f" | sed -n '48,62p;132,146p'
;;
.github/workflows/build-src.yml)
nl -ba "$f" | sed -n '52,68p'
;;
.github/workflows/cache-depends-sources.yml)
nl -ba "$f" | sed -n '18,32p'
;;
.github/workflows/guix-build.yml)
nl -ba "$f" | sed -n '24,38p;85,99p'
;;
.github/workflows/lint.yml)
nl -ba "$f" | sed -n '20,34p'
;;
.github/workflows/test-src.yml)
nl -ba "$f" | sed -n '32,44p'
;;
esac
echo
doneRepository: dashpay/dash
Length of output: 263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in .github/workflows/build-container.yml .github/workflows/build-depends.yml .github/workflows/build-src.yml .github/workflows/cache-depends-sources.yml .github/workflows/guix-build.yml .github/workflows/lint.yml .github/workflows/test-src.yml; do
echo "FILE: $f"
grep -nE '^(on:| pull_request_target:| pull_request:| workflow_dispatch:|jobs:| - uses: actions/checkout@| ref:| allow-unsafe-pr-checkout:| persist-credentials:| - run:| - name: )' "$f" | sed -n '1,120p'
echo
doneRepository: dashpay/dash
Length of output: 3401
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "allow-unsafe-pr-checkout|pull_request_target|actions/checkout@" .github/workflowsRepository: dashpay/dash
Length of output: 3577
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== callers of workflow_call files ==="
rg -n "uses:\s*\./\.github/workflows/(build-container|build-depends|build-src|cache-depends-sources|guix-build|lint|test-src)\.yml" .github/workflows
echo
echo "=== build.yml trigger + relevant jobs ==="
sed -n '1,220p' .github/workflows/build.yml | awk 'NR>=1 && NR<=220 {print NR ":" $0}'
echo
echo "=== workflow_call declarations for the reviewed files ==="
for f in .github/workflows/build-container.yml .github/workflows/build-depends.yml .github/workflows/build-src.yml .github/workflows/cache-depends-sources.yml .github/workflows/guix-build.yml .github/workflows/lint.yml .github/workflows/test-src.yml; do
echo "--- $f ---"
rg -n "^(on:| workflow_call:| pull_request_target:| pull_request:|permissions:|jobs:)" "$f"
doneRepository: dashpay/dash
Length of output: 12381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in .github/workflows/build-container.yml .github/workflows/build-depends.yml .github/workflows/build-src.yml .github/workflows/cache-depends-sources.yml .github/workflows/guix-build.yml .github/workflows/lint.yml .github/workflows/test-src.yml; do
echo "===== $f ====="
case "$f" in
.github/workflows/build-container.yml)
sed -n '36,50p;84,95p;128,141p' "$f" | awk '{print NR ":" $0}'
;;
.github/workflows/build-depends.yml)
sed -n '48,60p;134,183p' "$f" | awk '{print NR ":" $0}'
;;
.github/workflows/build-src.yml)
sed -n '54,66p;120,180p' "$f" | awk '{print NR ":" $0}'
;;
.github/workflows/cache-depends-sources.yml)
sed -n '20,40p' "$f" | awk '{print NR ":" $0}'
;;
.github/workflows/guix-build.yml)
sed -n '22,40p;86,125p' "$f" | awk '{print NR ":" $0}'
;;
.github/workflows/lint.yml)
sed -n '22,46p' "$f" | awk '{print NR ":" $0}'
;;
.github/workflows/test-src.yml)
sed -n '34,60p' "$f" | awk '{print NR ":" $0}'
;;
esac
echo
doneRepository: dashpay/dash
Length of output: 11992
Do not run fork PR heads in privileged CI jobs. These reusable workflows are called from .github/workflows/build.yml, which runs on pull_request_target, so github.event.pull_request.head.sha checks out attacker-controlled code under the workflow token. persist-credentials: false does not make that safe.
.github/workflows/build-container.yml#L43-L45,#L89-L91,#L134-L136: keep GHCR login/push separate from the untrusted Docker build..github/workflows/build-depends.yml#L54-L56,#L138-L140;.github/workflows/build-src.yml#L59-L61;.github/workflows/cache-depends-sources.yml#L25-L27;.github/workflows/lint.yml#L26-L28;.github/workflows/test-src.yml#L38-L40;.github/workflows/guix-build.yml#L30-L32,#L91-L93: move the fork-controlledmake/script/Guix steps into an unprivilegedpull_requestworkflow.
🧰 Tools
🪛 actionlint (1.7.12)
[error] 44-44: input "allow-unsafe-pr-checkout" is not defined in action "actions/checkout@v6". available inputs are "clean", "fetch-depth", "fetch-tags", "filter", "github-server-url", "lfs", "path", "persist-credentials", "ref", "repository", "set-safe-directory", "show-progress", "sparse-checkout", "sparse-checkout-cone-mode", "ssh-key", "ssh-known-hosts", "ssh-strict", "ssh-user", "submodules", "token"
(action)
📍 Affects 7 files
.github/workflows/build-container.yml#L43-L45(this comment).github/workflows/build-container.yml#L89-L91.github/workflows/build-container.yml#L134-L136.github/workflows/build-depends.yml#L54-L56.github/workflows/build-depends.yml#L138-L140.github/workflows/build-src.yml#L59-L61.github/workflows/cache-depends-sources.yml#L25-L27.github/workflows/guix-build.yml#L30-L32.github/workflows/guix-build.yml#L91-L93.github/workflows/lint.yml#L26-L28.github/workflows/test-src.yml#L38-L40
🤖 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/build-container.yml around lines 43 - 45, Prevent
fork-controlled code from running in privileged pull_request_target jobs. In
.github/workflows/build-container.yml at 43-45, 89-91, and 134-136, separate
GHCR login/push from the untrusted Docker build. Move the fork-controlled
make/script/Guix steps to an unprivileged pull_request workflow in
.github/workflows/build-depends.yml at 54-56 and 138-140, build-src.yml at
59-61, cache-depends-sources.yml at 25-27, lint.yml at 26-28, test-src.yml at
38-40, and guix-build.yml at 30-32 and 91-93.
There was a problem hiding this comment.
Show an actual exploit / problem, and we can try to make it happen. But we currently need this for our builds.
keep GHCR login/push separate from the untrusted Docker build.
this is only used for ci artifacts. If we can make improvements without breakage, I'm happy to minimize permissions we give tokens etc.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== reusable workflow callers and permissions ==='
rg -n -C 8 \
'pull_request_target:|permissions:|uses:\s*\./\.github/workflows/build-container\.yml|packages:\s*(write|read)|contents:\s*(write|read)' \
.github/workflows
echo
echo '=== build-container call sites (full job context) ==='
rg -n -C 16 'uses:\s*\./\.github/workflows/build-container\.yml' .github/workflows
echo
echo '=== GHCR image/tag references and build-container inputs ==='
rg -n -C 4 \
'ghcr\.io/|build-container\.yml|container_name:|name:.*container|context:|file:' \
.github/workflows .github Dockerfile* 2>/dev/null || true
echo
echo '=== workflow file inventory ==='
fd -t f -e yml -e yaml .github/workflows | sortLength of output: 34527
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== CI workflow: container outputs and downstream consumers ==='
sed -n '1,260p' .github/workflows/build.yml | nl -ba
echo
echo '=== container-path consumers and container image declarations ==='
rg -n -C 10 \
'container-path|container:\s|image:\s*\$\{\{|ghcr\.io/\$\{\{.*dashcore-ci|dashcore-ci-(runner|slim)' \
.github/workflows
echo
echo '=== relevant reusable-workflow execution and checkout sections ==='
for f in \
.github/workflows/build-depends.yml \
.github/workflows/build-src.yml \
.github/workflows/test-src.yml \
.github/workflows/lint.yml; do
echo "--- $f ---"
rg -n -C 8 'container-path|container:|image:|actions/checkout@v6|run:' "$f"
doneLength of output: 1120
🤖 web_search executed:
GitHub Actions documentation: for a pull_request_target workflow, what are GITHUB_REF and GITHUB_SHA on a pull request from a fork? Also document the security warning about checking out and running pull request head code in pull_request_target.
Length of output: 18827
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== CI workflow (downstream jobs after reusable container calls) ==='
sed -n '90,260p' .github/workflows/build.yml
echo
echo '=== container-path/image consumers ==='
rg -n -C 12 \
'container-path|container:\s|image:\s*\$\{\{|dashcore-ci-(runner|slim)|ghcr\.io/' \
.github/workflows
echo
echo '=== reusable workflow container declarations ==='
for f in \
.github/workflows/build-depends.yml \
.github/workflows/build-src.yml \
.github/workflows/test-src.yml \
.github/workflows/lint.yml; do
echo "--- $f ---"
rg -n -C 8 'container-path|container:|image:|actions/checkout@v6|run:' "$f" || true
doneLength of output: 68343
@PastaPastaPasta, there is a concrete exploit path here; I agree my earlier “separate GHCR login/push” wording was incomplete. The core problem is executing the fork’s image/code under pull_request_target, not merely the location of the login step.
A fork PR can:
- Modify
contrib/containers/ci/ci.Dockerfileorci-slim.Dockerfileto replace a command used by CI (for example the shell or a build tool) with a wrapper that captures its environment or alters artifacts. - Trigger
.github/workflows/build.yml, which ispull_request_targetwithpackages: write. - Have
.github/workflows/build-container.ymlcheck out that fork SHA, build it, and push it to the upstreamghcr.io/dashpay/...namespace. - Cause the same run’s downstream jobs to execute that attacker-built image as root:
build-depends.ymlusescontainer: image: ${{ inputs.container-path }}build-src.yml,lint.yml, andtest-src.ymldo the same.
- Those jobs then run the fork’s scripts (
make,build_src.sh, lint/test scripts) in the attacker-controlled container while retaining the caller workflow’s privileges.
This is not limited to an isolated artifact: the image output is passed directly to the subsequent CI jobs. Additionally, GITHUB_REF on pull_request_target is the base branch, so the manifest is tagged with the base-branch tag and latest; a fork PR can overwrite those shared CI-image tags until a trusted build replaces them.
I would not run a live token-exfiltration proof of concept against the project. GitHub explicitly documents this checkout-and-execute pattern as insecure: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target. Their actions/checkout v6 documentation added allow-unsafe-pr-checkout specifically because this pattern enables “pwn request” vulnerabilities.
A minimally disruptive direction is:
- Keep
pull_request_targetonly for non-executing trusted automation. - Run fork build/test jobs under
pull_requestwith read-only permissions and no GHCR push. - Reserve the current image-publish / privileged CI path for pushes and collaborator-approved PRs (for example, a maintainer-controlled label or a separately dispatched trusted workflow).
That preserves fork CI, but removes the fork → upstream package → privileged execution chain. The existing persist-credentials: false remains good defense-in-depth, but it only removes checkout’s Git credential persistence; it does not change the trust level of the job container or the packages: write token.
Issue being fixed or feature implemented
actions/checkout@v6moved to v6.1.0 on 2026-07-20. The new release rejects fork pull-request head checkouts frompull_request_targetunless the workflow explicitly acknowledges them, which broke Dash Core CI in run 29761916351.What was done?
allow-unsafe-pr-checkout: trueto the 11 checkout steps that intentionally check outgithub.event.pull_request.head.sha.persist-credentials: falseto the same steps so checkout does not leave its GitHub token in Git configuration for fork-controlled code.GHCR is used only for CI artifacts, not production. This PR intentionally restores the existing CI design rather than introducing broader workflow hardening.
How has this been tested?
git diff --check origin/develop...HEAD.dd537806bfe63ab76ccfe7c4bbb46dad4dec190c; its tree exactly matches the previously reviewedbd84229ee25e7918e17230b0fbfc748f575cd5c9tree (ship, no findings).actionlintwas unavailable locally, so no local actionlint result is claimed.Breaking changes
None. This restores fork pull-request CI under checkout v6.1 while retaining the existing workflow behavior.
Checklist