Skip to content

Enable e2e mock test on release branches #60

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

Merged
zdtsw merged 1 commit into from
Apr 7, 2026
Merged

Enable e2e mock test on release branches #60

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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-mock-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: E2E Mock vLLM Test
on:
pull_request_target:
types: [labeled, synchronize]
branches: [main]
branches: [main, 'release-*']
Comment on lines 4 to +6
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 6, 2026 •
edited
Loading

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 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-latest

As 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.

Copy link
Copy Markdown
Contributor Author

@aneeshkp aneeshkp Apr 6, 2026

Choose a reason for hiding this comment

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

run-e2e-test label gate is used

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 6, 2026

Choose a reason for hiding this comment

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

@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.


permissions: {}

Expand Down
Loading