ci: skip e2e job on PRs from forks#7030
Conversation
Forks don't have access to the secrets needed for e2e tests, so the job always fails. Skip it to avoid a bad contributor experience.
serhalp
force-pushed
the
serhalp/skip-fork-e2e-ci
branch
from
9de78e2 to
7cee639
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
serhalp
force-pushed
the
serhalp/skip-fork-e2e-ci
branch
from
8a4de0a to
d936c92
Compare
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaced the prior Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0113e2291
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/workflow.yml:
- Line 51: The artifact upload step's if condition uses only
${!steps.fork-check.outputs.IS_FORK} so it won't run when prior steps fail;
change the condition to include always() so the step runs regardless of job
outcome (e.g., replace if: ${{ !steps.fork-check.outputs.IS_FORK }} with if: ${{
always() && !steps.fork-check.outputs.IS_FORK }}), keeping the same step
(artifact upload / Playwright report upload) and step id intact.
- Line 40: The e2e job's conditional uses an invalid matrix context
(matrix.node-version) even though the job has no strategy.matrix; update the e2e
job's if expression to remove the matrix check (delete "&& matrix.node-version
== '22'") so it reads like the other guards (e.g., if: ${{
!steps.fork-check.outputs.IS_FORK && !steps.release-check.outputs.IS_RELEASE
}}), or alternately add a strategy.matrix with node-version if you intended to
run a matrix—make the change on the e2e job's if line.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e1be42d3-b59b-49e1-a130-86011698702e
📒 Files selected for processing (1)
.github/workflows/workflow.yml
serhalp
force-pushed
the
serhalp/skip-fork-e2e-ci
branch
from
f0113e2 to
bcc6a61
Compare
Instead of skipping the entire job (which would block merging if e2e is a required check), run the job but skip all steps with a visible message explaining why.
serhalp
force-pushed
the
serhalp/skip-fork-e2e-ci
branch
from
bcc6a61 to
9236eaf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/workflow.yml:
- Around line 14-23: The inline bash in the step with id should-run currently
interpolates github.head_ref and github.event.pull_request.head.repo.fork
directly into the script which is unsafe; move those context values into an env:
block (e.g. GH_HEAD_REF: ${{ github.head_ref }}, PR_HEAD_REPO_FORK: ${{
github.event.pull_request.head.repo.fork }}) and then reference them inside the
run script as quoted shell variables ($GH_HEAD_REF, $PR_HEAD_REPO_FORK) to avoid
pre-shell interpolation and injection, keeping the same conditional logic but
using the env variables in the if/elif checks.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb991882-64cc-4a6e-9f5a-fe4c22069c5c
📒 Files selected for processing (1)
.github/workflows/workflow.yml
| - name: Check if tests should run | ||
| id: should-run | ||
| run: | | ||
| if [[ "${{ github.event_name }}" == "pull_request" && "${{ github.event.pull_request.head.repo.fork }}" == "true" ]]; then | ||
| echo "::notice::Skipping e2e tests — PRs from forks don't have access to required secrets." | ||
| elif [[ "${{ github.head_ref }}" == release-* ]]; then | ||
| echo "::notice::Skipping e2e tests on release PR." | ||
| else | ||
| echo "RUN=true" >> $GITHUB_OUTPUT | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect: no matches for github.head_ref directly inside run-script bodies.
rg -n -C3 '\$\{\{\s*github\.head_ref\s*\}\}' .github/workflows/workflow.ymlRepository: netlify/build
Length of output: 518
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check all direct GitHub context interpolations in inline scripts
echo "=== All github.* context in run blocks ==="
rg -n '\$\{\{\s*github\.' .github/workflows/workflow.yml | head -30
echo ""
echo "=== Full snippet of lines 14-23 ==="
sed -n '14,23p' .github/workflows/workflow.yml
echo ""
echo "=== Context around should-run step (lines 10-50) ==="
sed -n '10,50p' .github/workflows/workflow.ymlRepository: netlify/build
Length of output: 2497
Pass PR metadata through env before using it in bash.
Direct interpolation of github.head_ref into the inline bash script on line 19 creates a shell injection risk. PR branch names are user-controlled, and inline expression interpolation happens before shell parsing—a crafted ref can alter shell syntax before the script runs. Pass all context values via the env: block instead.
🛡️ Proposed fix
- name: Check if tests should run
id: should-run
+ env:
+ EVENT_NAME: ${{ github.event_name }}
+ IS_FORK: ${{ github.event.pull_request.head.repo.fork }}
+ HEAD_REF: ${{ github.head_ref }}
run: |
- if [[ "${{ github.event_name }}" == "pull_request" && "${{ github.event.pull_request.head.repo.fork }}" == "true" ]]; then
+ if [[ "${EVENT_NAME}" == "pull_request" && "${IS_FORK}" == "true" ]]; then
echo "::notice::Skipping e2e tests — PRs from forks don't have access to required secrets."
- elif [[ "${{ github.head_ref }}" == release-* ]]; then
+ elif [[ "${HEAD_REF}" == release-* ]]; then
echo "::notice::Skipping e2e tests on release PR."
else
- echo "RUN=true" >> $GITHUB_OUTPUT
+ echo "RUN=true" >> "$GITHUB_OUTPUT"
fi🧰 Tools
🪛 actionlint (1.7.12)
[error] 16-16: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/workflow.yml around lines 14 - 23, The inline bash in the
step with id should-run currently interpolates github.head_ref and
github.event.pull_request.head.repo.fork directly into the script which is
unsafe; move those context values into an env: block (e.g. GH_HEAD_REF: ${{
github.head_ref }}, PR_HEAD_REPO_FORK: ${{
github.event.pull_request.head.repo.fork }}) and then reference them inside the
run script as quoted shell variables ($GH_HEAD_REF, $PR_HEAD_REPO_FORK) to avoid
pre-shell interpolation and injection, keeping the same conditional logic but
using the env variables in the if/elif checks.
2 participants
Summary
PRs from forks don't have access to the repository secrets needed for e2e tests, so the e2e job always fails. This adds a job-level condition to skip the e2e job when the PR originates from a fork, avoiding a confusing failing-check experience for external contributors.
This still allows e2e to run on pushes to main, merge group events, and PRs from branches within the repo.