Skip to content

fix: Remove pull_request_target code checkout vulnerability #694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
fix-it-felix-sentry wants to merge 1 commit into from
+21 −46
Closed

fix: Remove pull_request_target code checkout vulnerability #694

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 21 additions & 46 deletions .github/workflows/eval-skill-fork.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,51 +24,29 @@ jobs:
gh api "repos/$REPO/issues/$PR/labels/eval-skill" -X DELETE 2>/dev/null || true
gh api "repos/$REPO/issues/$PR/labels/eval-skill-passed" -X DELETE 2>/dev/null || true

eval:
name: Run skill eval
notify-manual-review:
name: Require manual review for fork PRs
if: >-
github.event.action == 'labeled'
&& github.event.label.name == 'eval-skill'
&& github.event.pull_request.head.repo.fork == true
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
with:
ref: ${{ github.event.pull_request.head.sha }}

- uses: oven-sh/setup-bun@v2

- uses: actions/cache@v5
id: cache
with:
path: node_modules
key: node-modules-${{ hashFiles('bun.lock', 'patches/**') }}
- if: steps.cache.outputs.cache-hit != 'true'
run: bun install --frozen-lockfile

- name: Eval SKILL.md
id: eval
run: bun run eval:skill
env:
ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
continue-on-error: true
# SECURITY: Do not checkout PR code in pull_request_target context.
# pull_request_target runs with write permissions and access to secrets,
# but checking out PR code would allow malicious PRs to exfiltrate secrets
# via modified dependencies or build scripts.
# See: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

- name: Post commit status
env:
GH_TOKEN: ${{ github.token }}
run: |
SHA="${{ github.event.pull_request.head.sha }}"
if [[ "${{ steps.eval.outcome }}" == "success" ]]; then
STATE="success"
DESC="Skill eval passed"
else
STATE="failure"
DESC="Skill eval failed"
fi
gh api "repos/${{ github.repository }}/statuses/$SHA" \
-f state="$STATE" \
-f state="pending" \
Comment on lines 43 to +47
Copy link
Copy Markdown

@sentry sentry bot Apr 9, 2026

Choose a reason for hiding this comment

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

Bug: The workflow sets the eval-skill/fork commit status to pending, but the main CI requires success, permanently blocking fork PRs that modify skill files from passing checks.
Severity: HIGH

Suggested Fix

Update the workflow to allow a maintainer's action, such as adding the eval-skill-passed label, to trigger an update that sets the eval-skill/fork commit status to success. This will unblock the CI check in ci.yml after a maintainer has manually approved the skill evaluation.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: .github/workflows/eval-skill-fork.yml#L43-L47

Potential issue: The `ci.yml` workflow requires the `eval-skill/fork` commit status to
be `success` for fork PRs that modify skill files. However, the new
`eval-skill-fork.yml` workflow hardcodes this status to `pending`. The instructions for
maintainers mention adding a label (`eval-skill-passed`) after manual review, but the CI
check does not look for this label. Because there is no mechanism to update the commit
status to `success`, any fork PR modifying skill files will be permanently blocked from
passing CI checks and cannot be merged.

Did we get this right? 👍 / 👎 to inform future reviews.

-f context="eval-skill/fork" \
-f description="$DESC"
-f description="Manual review required for fork PRs (security restriction)"
Copy link
Copy Markdown
Contributor

@cursor cursor bot Apr 9, 2026

Choose a reason for hiding this comment

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

Fork PR commit status permanently stuck at pending

High Severity

The workflow sets the eval-skill/fork commit status to "pending" but never provides a path to set it to "success". The ci.yml eval-skill job checks this status and fails if it's not "success". The comment instructs maintainers to add the eval-skill-passed label, but no workflow reacts to that label by updating the commit status. This means fork PRs that modify skill files can never pass CI — the status is permanently stuck at "pending".

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7259a84. Configure here.


- name: Remove eval-skill label
if: always()
Expand All @@ -78,20 +56,17 @@ jobs:
gh api "repos/${{ github.repository }}/issues/${{ github.event.number }}/labels/eval-skill" \
-X DELETE 2>/dev/null || true

# Use the SENTRY_RELEASE_BOT app token to add the label — app tokens
# can trigger workflow runs, unlike GITHUB_TOKEN (recursion protection).
- name: Get app token
id: token
if: steps.eval.outcome == 'success'
uses: actions/create-github-app-token@v3
with:
app-id: ${{ vars.SENTRY_RELEASE_BOT_CLIENT_ID }}
private-key: ${{ secrets.SENTRY_RELEASE_BOT_PRIVATE_KEY }}

- name: Add eval-skill-passed label (triggers main CI re-run)
if: steps.eval.outcome == 'success'
- name: Add comment with instructions
env:
GH_TOKEN: ${{ steps.token.outputs.token }}
GH_TOKEN: ${{ github.token }}
run: |
gh api "repos/${{ github.repository }}/issues/${{ github.event.number }}/labels" \
--input - <<< '{"labels":["eval-skill-passed"]}'
gh api "repos/${{ github.repository }}/issues/${{ github.event.number }}/comments" \
-f body="⚠️ **Security Notice**: Automated skill evaluation is disabled for fork PRs to prevent potential secret exfiltration.

For security reasons, this workflow was updated to not execute untrusted code from fork PRs with access to repository secrets. A maintainer with write access can manually trigger the evaluation by:

1. Checking out the PR branch locally
2. Running \`bun run eval:skill\` with appropriate API credentials
3. Reviewing the results and adding the \`eval-skill-passed\` label if successful

See [GitHub Security Lab: Preventing pwn requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/) for more information about this security issue."
Loading