Skip to content

Use pull_request_target for e2e tests to support fork PRs#796

Open
kelos-bot[bot] wants to merge 1 commit intomainfrom
kelos-task-793
Open

Use pull_request_target for e2e tests to support fork PRs#796
kelos-bot[bot] wants to merge 1 commit intomainfrom
kelos-task-793

Conversation

@kelos-bot
Copy link

@kelos-bot kelos-bot bot commented Mar 26, 2026

/kind bug

What type of PR is this?

Bug fix — e2e tests cannot access secrets for PRs from forked repositories.

What this PR does / why we need it:

GitHub Actions does not expose repository secrets to pull_request events from forks. This PR switches the e2e test job to use pull_request_target instead, which runs in the base repository's context and has access to secrets.

Changes:

  • Add pull_request_target trigger to the CI workflow
  • Non-e2e jobs (build, verify, test, test-integration) skip pull_request_target events to avoid double runs
  • E2e job now triggers on pull_request_target (with ok-to-test label gate) instead of pull_request
  • Checkout step in e2e job explicitly uses github.event.pull_request.head.sha to test the PR's actual code

The existing ok-to-test label gate ensures a maintainer has reviewed the code before secrets are exposed to fork PRs.

Which issue(s) this PR is related to:

Fixes #793

Special notes for your reviewer:

The pull_request_target event runs the workflow definition from the base branch (main), not the PR branch. This is the standard secure pattern for running CI with secrets on fork PRs — the workflow itself is trusted (from main), and the ok-to-test label ensures maintainer approval before secrets are used.

Does this PR introduce a user-facing change?

NONE

Summary by cubic

Enable e2e tests on forked PRs by switching to pull_request_target, gated by the ok-to-test label, so secrets are available safely. Fixes #793.

  • Bug Fixes
    • Add pull_request_target trigger; e2e runs only when PR has ok-to-test.
    • Guard build/verify/unit/integration jobs to ignore pull_request_target and avoid duplicate runs.
    • In e2e, checkout uses ${{ github.event.pull_request.head.sha || github.sha }} to test the PR’s code.

Written for commit daa21c4. Summary will update on new commits.

GitHub Actions does not expose repository secrets to pull_request events
from forks. Switch the e2e job to trigger on pull_request_target instead,
which runs in the base repository context and has access to secrets. The
existing ok-to-test label gate ensures maintainer review before secrets
are exposed. Non-e2e jobs continue to use pull_request to avoid double
runs. The checkout step now explicitly uses the PR head SHA to test the
correct code.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/ci.yaml">

<violation number="1" location=".github/workflows/ci.yaml:9">
P1: Adding `pull_request_target` without separating concurrency groups can cause `pull_request` and `pull_request_target` runs to cancel each other, potentially skipping required CI jobs.</violation>

<violation number="2" location=".github/workflows/ci.yaml:11">
P0: `pull_request_target` on `synchronize` allows newly pushed fork commits to run e2e with secrets as long as a prior `ok-to-test` label remains. Require fresh maintainer re-approval per commit (for example, trigger only on `labeled`).</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

types: [opened, synchronize, reopened, labeled]
pull_request_target:
branches: [main]
types: [opened, synchronize, reopened, labeled]
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 26, 2026

Choose a reason for hiding this comment

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

P0: pull_request_target on synchronize allows newly pushed fork commits to run e2e with secrets as long as a prior ok-to-test label remains. Require fresh maintainer re-approval per commit (for example, trigger only on labeled).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yaml, line 11:

<comment>`pull_request_target` on `synchronize` allows newly pushed fork commits to run e2e with secrets as long as a prior `ok-to-test` label remains. Require fresh maintainer re-approval per commit (for example, trigger only on `labeled`).</comment>

<file context>
@@ -6,6 +6,9 @@ on:
     types: [opened, synchronize, reopened, labeled]
+  pull_request_target:
+    branches: [main]
+    types: [opened, synchronize, reopened, labeled]
   merge_group:
   workflow_dispatch:
</file context>
Suggested change
types: [opened, synchronize, reopened, labeled]
types: [labeled]
Fix with Cubic

pull_request:
branches: [main]
types: [opened, synchronize, reopened, labeled]
pull_request_target:
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 26, 2026

Choose a reason for hiding this comment

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

P1: Adding pull_request_target without separating concurrency groups can cause pull_request and pull_request_target runs to cancel each other, potentially skipping required CI jobs.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yaml, line 9:

<comment>Adding `pull_request_target` without separating concurrency groups can cause `pull_request` and `pull_request_target` runs to cancel each other, potentially skipping required CI jobs.</comment>

<file context>
@@ -6,6 +6,9 @@ on:
   pull_request:
     branches: [main]
     types: [opened, synchronize, reopened, labeled]
+  pull_request_target:
+    branches: [main]
+    types: [opened, synchronize, reopened, labeled]
</file context>
Fix with Cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

e2e tests cannot access secrets for PRs from forked repositories

1 participant