Enable e2e mock test on release branches#60
Conversation
📝 WalkthroughWalkthroughThe e2e-mock-test workflow configuration was updated to expand the Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Security concerns
Using Actionable: Verify that all 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/e2e-mock-test.yml:
- Around line 4-6: The workflow currently uses pull_request_target with branches
['main','release-*'] and later checks out github.event.pull_request.head.sha
(and runs repo code with secrets), allowing untrusted PR code to access secrets;
either restrict execution to same-repo PRs by gating the job with a trust check
(e.g., guard using github.event.pull_request.head.repo.full_name ==
github.repository) before the checkout/run steps that use secrets, or change the
trigger from pull_request_target to pull_request so the PR head is checked out
without exposing secrets; update the job that invokes actions/checkout (ref:
github.event.pull_request.head.sha) and any steps using secrets to only run
after the same-repo guard or after switching to a safe trigger.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 3c9e8b1e-9366-4788-852b-2e914f5b6737
📒 Files selected for processing (1)
.github/workflows/e2e-mock-test.yml
| pull_request_target: | ||
| types: [labeled, synchronize] | ||
| branches: [main] | ||
| branches: [main, 'release-*'] |
There was a problem hiding this comment.
Block untrusted PR code execution in pull_request_target before widening to release-*.
Line 6 expands a privileged pull_request_target workflow to more branches, while Line 50 checks out github.event.pull_request.head.sha and later runs repo code with secrets (Lines 88 and 99). This is a direct untrusted-code + secret exposure path (CWE-829, CWE-200). Add a trust gate (same-repo only) or move this job to a safer trigger pattern.
Suggested hardening patch
e2e-test:
name: E2E Mock vLLM Test
permissions:
contents: read
# Only runs when a maintainer adds the run-e2e-test label specifically.
# Does not trigger on other labels or on synchronize (new pushes).
- if: github.event.action == 'labeled' && github.event.label.name == 'run-e2e-test'
+ if: >-
+ github.event.action == 'labeled' &&
+ github.event.label.name == 'run-e2e-test' &&
+ github.event.pull_request.head.repo.full_name == github.repository
runs-on: ubuntu-latestAs per coding guidelines, "Avoid pull_request_target with checkout of PR head (token exfiltration)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e-mock-test.yml around lines 4 - 6, The workflow
currently uses pull_request_target with branches ['main','release-*'] and later
checks out github.event.pull_request.head.sha (and runs repo code with secrets),
allowing untrusted PR code to access secrets; either restrict execution to
same-repo PRs by gating the job with a trust check (e.g., guard using
github.event.pull_request.head.repo.full_name == github.repository) before the
checkout/run steps that use secrets, or change the trigger from
pull_request_target to pull_request so the PR head is checked out without
exposing secrets; update the job that invokes actions/checkout (ref:
github.event.pull_request.head.sha) and any steps using secrets to only run
after the same-repo guard or after switching to a safe trigger.
There was a problem hiding this comment.
run-e2e-test label gate is used
There was a problem hiding this comment.
@aneeshkp, the label gate is acknowledged — and the remove-label-on-push job that strips the label on every new push (forcing re-review before re-labeling) makes this a well-layered mitigation. That pattern is widely used and accepted for pull_request_target workflows that need to run on fork PRs.
The residual risk I flagged is that a maintainer could be socially engineered into adding the label to a PR containing malicious code, which would then run with RAHII_ON_XKS_PULL_SECRET. Adding the same-repo guard would eliminate that vector entirely but would also prevent external contributors from ever getting the e2e test run — so there's a real trade-off to weigh.
If you're comfortable with the current label-gate approach (maintainer reviews code before labeling), that's a reasonable decision for this repo. I'll leave the choice to you.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aneeshkp, zdtsw The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2 participants
The e2e mock test workflow only triggers on PRs to
main.Add
release-*to the branch filter so PRs targeting releasebranches (e.g., release-3.4-ea.2) also run the e2e test.
Summary by CodeRabbit