Skip to content

Pin actions/checkout and allow unsafe PR checkout in docs validation - #1100

Open
sjishnu-moveworks wants to merge 1 commit into
mainfrom
pin-checkout-allow-unsafe-pr-checkout
Open

Pin actions/checkout and allow unsafe PR checkout in docs validation#1100
sjishnu-moveworks wants to merge 1 commit into
mainfrom
pin-checkout-allow-unsafe-pr-checkout

Conversation

@sjishnu-moveworks

Copy link
Copy Markdown
Collaborator

The Developer Docs Validation workflow referenced actions/checkout@v4, but GitHub auto-upgrades deprecated major-tag references of official actions at runtime, so the workflow actually runs checkout v7 — which introduces a safety gate that blocks checking out fork PR code in pull_request_target workflows. That is why contributor fork PRs currently fail this check.

Applies the same fix as moveworks-emu/k8s-manifests#17064:

  • Pin actions/checkout to the v7.0.1 commit SHA (3d3c42e5...), removing the runtime auto-upgrade variable.
  • Set allow-unsafe-pr-checkout: true on the checkout step. It intentionally fetches fork PR code under pull_request_target so the ci/ validation scripts can inspect the contributed docs; the job runs with permissions: read-all and no extra secrets.

The create-github-app-token portion of the upstream PR does not apply — this repo has no such step.

The Developer Docs Validation workflow referenced actions/checkout@v4, but
GitHub auto-upgrades deprecated major-tag references of official actions at
runtime, so the workflow actually runs checkout v7 — which introduces a
safety gate that blocks checking out fork PR code in pull_request_target
workflows. That is why contributor fork PRs currently fail this check.
Applies the same fix as moveworks-emu/k8s-manifests#17064:
- Pin actions/checkout to the v7.0.1 commit SHA (3d3c42e5...), removing the
runtime auto-upgrade variable.
- Set allow-unsafe-pr-checkout: true on the checkout step. It intentionally
fetches fork PR code under pull_request_target so the ci/ validation
scripts can inspect the contributed docs; the job runs with
permissions: read-all and no extra secrets.
The create-github-app-token portion of the upstream PR does not apply — this
repo has no such step.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# pull_request_target so the ci/ validation scripts can inspect the
# contributed docs. The job runs with permissions: read-all and no
# secrets beyond the default token.
allow-unsafe-pr-checkout: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Severity: HIGH

Pwn-request / RCE via pull_request_target + untrusted code execution. This PR bypasses the actions/checkout v7 safety gate and then immediately runs code from the fork: pip install -r ci/requirements.txt, python ci/check_readme_changes.py, python -m ci.validate_v3, python ci/copy_code_blocks.py, python ci/generate_cleanup_report.py. An attacker only needs to open a fork PR with a malicious ci/requirements.txt or modified Python script to execute arbitrary code in the pull_request_target context, which carries the repo GITHUB_TOKEN (read-all + pull-requests: write in the comment job). GitHub's own guidance states: "You must ensure the checked-out code is only ever inspected as data and never executed" before setting allow-unsafe-pr-checkout: true. See GitHub docs.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: This is a pwn-request vulnerability that requires an architectural redesign of the workflow, not a simple flag removal. Simply deleting allow-unsafe-pr-checkout: true would just break the workflow without eliminating the risk. The recommended fix is to split this into two separate workflows using the safe pull_request + workflow_run pattern:

  1. Workflow 1 (pr-validate.yml) — triggered by pull_request (not pull_request_target): Checks out the PR HEAD, runs pip install -r ci/requirements.txt, and executes all CI scripts (check_readme_changes.py, validate_v3, copy_code_blocks.py, generate_cleanup_report.py). Because this trigger runs in the fork's restricted sandbox with no write permissions and no access to repo secrets, executing fork-controlled code is safe. Save the outputs (validation results, messages) as workflow artifacts.

  2. Workflow 2 (pr-comment.yml) — triggered by workflow_run on completion of Workflow 1: This runs in the base repo context (with pull-requests: write). It downloads the artifacts produced by Workflow 1 and posts the comment(s) to the PR. It must never check out or execute PR code — only read the artifact data.

This pattern completely separates untrusted code execution (sandboxed pull_request context) from privileged operations (base-repo workflow_run context), eliminating the pwn-request attack surface. See GitHub's security hardening guidance at https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/ for a detailed implementation example.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@sjishnu-moveworks@JishnuMoveworks