Skip to content

fix(ci): support fork PRs in e2e-brev workflow#1860

Closed
cjagwani wants to merge 1 commit intomainfrom
fix/e2e-brev-fork-checkout
Closed

fix(ci): support fork PRs in e2e-brev workflow#1860
cjagwani wants to merge 1 commit intomainfrom
fix/e2e-brev-fork-checkout

Conversation

@cjagwani
Copy link
Copy Markdown
Contributor

@cjagwani cjagwani commented Apr 14, 2026

Summary

  • Use refs/pull/<N>/head instead of resolving the branch name for checkout when pr_number is provided
  • Fork branches don't exist in the base repo, so the previous approach failed with git fetch exit code 1
  • refs/pull/<N>/head is created by GitHub for all open PRs regardless of origin

Test plan

  • Trigger e2e-brev workflow with a fork PR number (e.g. PR feat(security): add tamper-evident audit chain logger #892) and confirm checkout succeeds
  • Trigger e2e-brev workflow with a non-fork PR number and confirm it still works
  • Trigger e2e-brev workflow with branch input (no pr_number) and confirm fallback works

Summary by CodeRabbit

  • Chores
    • Updated CI/CD workflow configuration for improved branch handling in pull request checks.

workflow_dispatch with pr_number resolved the branch name, but fork
branches don't exist in the base repo. Use refs/pull/<N>/head instead,
which GitHub creates for all open PRs regardless of origin.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 14, 2026

📝 Walkthrough

Walkthrough

A GitHub Actions workflow configuration update that introduces a new PR_REF environment variable derived from the pull request head reference and updates the checkout step to prioritize this variable over the previously computed RESOLVED_BRANCH variable.

Changes

Cohort / File(s) Summary
GitHub Actions Workflow
.github/workflows/e2e-brev.yaml
Added PR_REF environment variable from PR head reference; updated checkout ref selection to prefer env.PR_REF with fallback to inputs.branch and 'main'. The RESOLVED_BRANCH variable remains available for other workflow steps.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A workflow's leap, so clean and true,
New variables guide what we pursue,
PR heads now matter, refs all aligned,
The fallback path, so well-designed,
Main branch waits, but seldom called,
With this fix, our CI's enthralled! 🚀

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding support for fork PRs in the e2e-brev workflow by using GitHub PR refs instead of branch name resolution.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/e2e-brev-fork-checkout

Comment @coderabbitai help to get the list of available commands and usage tips.

@cjagwani cjagwani requested review from brandonpelfrey and cv April 14, 2026 02:58
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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/e2e-brev.yaml:
- Around line 118-124: The workflow is currently unconditionally setting PR_REF
to refs/pull/${{ inputs.pr_number }}/head and then checking it out
(actions/checkout@v6), which allows fork PRs to run code in jobs that have
secrets (BREV_API_TOKEN, NVIDIA_API_KEY); change the logic that sets or uses
PR_REF so fork PR refs are only used for trusted/internal PRs or explicit
maintainer opt-in: detect whether the PR head repo matches the repository owner
(e.g., compare github.event.pull_request.head.repo.full_name or
head.repo.owner.login to github.repository or github.repository_owner) and only
set PR_REF / perform the checkout of refs/pull/... when that check passes
(otherwise fall back to inputs.branch or 'main'), or gate the checkout behind an
explicit input like inputs.trust_pr_from_fork that must be true and approved by
a maintainer before exposing secrets. Ensure the checkout step
(actions/checkout@v6) uses the guarded PR_REF value so secrets are never exposed
to untrusted fork refs.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4ac8ecb9-7fc9-4bfd-8e94-b3199c832450

📥 Commits

Reviewing files that changed from the base of the PR and between 230d27e and fde92c7.

📒 Files selected for processing (1)
  • .github/workflows/e2e-brev.yaml

Comment on lines +118 to +124
# Use the PR head ref for checkout — works for both fork and non-fork PRs.
echo "PR_REF=refs/pull/${{ inputs.pr_number }}/head" >> "$GITHUB_ENV"

- name: Checkout target branch
uses: actions/checkout@v6
with:
ref: ${{ env.RESOLVED_BRANCH || inputs.branch || 'main' }}
ref: ${{ env.PR_REF || inputs.branch || 'main' }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Block untrusted fork PR refs before secret-backed checkout.

Line 119 and Line 124 now enable running fork PR code (refs/pull/<N>/head) in a job that later exposes BREV_API_TOKEN and NVIDIA_API_KEY. That is a secret-exfiltration path unless you gate fork PRs by trust level (or explicit maintainer opt-in).

🔒 Suggested guard before setting PR_REF
       - name: Resolve branch from PR number
         if: inputs.pr_number != ''
         env:
           GH_TOKEN: ${{ github.token }}
         run: |
           BRANCH=$(gh pr view ${{ inputs.pr_number }} --repo ${{ github.repository }} --json headRefName -q .headRefName)
+          IS_FORK=$(gh pr view ${{ inputs.pr_number }} --repo ${{ github.repository }} --json isCrossRepository -q .isCrossRepository)
+          AUTHOR_ASSOC=$(gh pr view ${{ inputs.pr_number }} --repo ${{ github.repository }} --json authorAssociation -q .authorAssociation)
+          if [ "$IS_FORK" = "true" ] && ! echo "$AUTHOR_ASSOC" | grep -Eq '^(OWNER|MEMBER|COLLABORATOR)$'; then
+            echo "::error::Refusing to run secret-backed e2e for untrusted fork PR #${{ inputs.pr_number }} (authorAssociation=$AUTHOR_ASSOC)."
+            exit 1
+          fi
           echo "Resolved PR #${{ inputs.pr_number }} → branch: $BRANCH"
           echo "RESOLVED_BRANCH=$BRANCH" >> "$GITHUB_ENV"
           # Use the PR head ref for checkout — works for both fork and non-fork PRs.
           echo "PR_REF=refs/pull/${{ inputs.pr_number }}/head" >> "$GITHUB_ENV"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e-brev.yaml around lines 118 - 124, The workflow is
currently unconditionally setting PR_REF to refs/pull/${{ inputs.pr_number
}}/head and then checking it out (actions/checkout@v6), which allows fork PRs to
run code in jobs that have secrets (BREV_API_TOKEN, NVIDIA_API_KEY); change the
logic that sets or uses PR_REF so fork PR refs are only used for
trusted/internal PRs or explicit maintainer opt-in: detect whether the PR head
repo matches the repository owner (e.g., compare
github.event.pull_request.head.repo.full_name or head.repo.owner.login to
github.repository or github.repository_owner) and only set PR_REF / perform the
checkout of refs/pull/... when that check passes (otherwise fall back to
inputs.branch or 'main'), or gate the checkout behind an explicit input like
inputs.trust_pr_from_fork that must be true and approved by a maintainer before
exposing secrets. Ensure the checkout step (actions/checkout@v6) uses the
guarded PR_REF value so secrets are never exposed to untrusted fork refs.

@cjagwani cjagwani closed this Apr 14, 2026
Sign up for free to 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.

1 participant