Skip to content

Allow outside contributors to run CI with secrets when approved#1782

Merged
pirate merged 4 commits intomainfrom
allow-pr-tests
Mar 5, 2026
Merged

Allow outside contributors to run CI with secrets when approved#1782
pirate merged 4 commits intomainfrom
allow-pr-tests

Conversation

@pirate
Copy link
Member

@pirate pirate commented Mar 4, 2026

PSA potential hackers: dont get excited, we don't have any real secrets in CI worth stealing, and our CI does not autodeploy anything to prod. All important secrets and CD processes are kept in our closed-source repos.

why

what changed

test plan


Summary by cubic

Add a gating workflow that blocks CI until a maintainer approves running secrets on forked PRs. CI now triggers from that gate, resolves labels and path filters under workflow_run, removes same-repo guards so integration/e2e/evals run on approved forks, and checks out the PR commit consistently across jobs.

Written for commit c682847. Summary will update on new commits. Review in cubic

@changeset-bot
Copy link

changeset-bot bot commented Mar 4, 2026

⚠️ No Changeset found

Latest commit: c682847

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 4, 2026

Greptile Summary

This PR modifies the GitHub Actions CI workflow to allow fork contributors' PRs to trigger CI runs by switching the trigger event from pull_request to pull_request_target and removing the github.event.pull_request.head.repo.full_name == github.repository guards from the server-integration-tests and run-evals jobs.

Critical concerns:

  • No approval gate implemented: The PR title says "when approved," but the workflow contains no actual approval mechanism (e.g., a required label like safe-to-test or a review approval check). Any fork contributor can trigger jobs that consume OPENAI_API_KEY, ANTHROPIC_API_KEY, BROWSERBASE_API_KEY, and other sensitive credentials.
  • pull_request_target security implications: This trigger runs in the base repository's context and always has access to secrets, regardless of whether the PR is from a fork. While no step currently checks out fork code (mitigating the worst-case "pwn request" scenario), secrets are still injected as environment variables into jobs that will now run for fork PRs.
  • Inconsistent fork protections: The run-e2e-local-tests and run-e2e-bb-tests jobs still retain their fork checks, while server-integration-tests and run-evals have had theirs removed — creating an inconsistent policy with no clear rationale.

Confidence Score: 1/5

  • This PR introduces a security regression by exposing repository secrets to untrusted fork PRs without implementing any approval gate.
  • The switch to pull_request_target without a corresponding approval mechanism (label-based or review-based gating) means fork PRs can consume costly API credentials without maintainer approval. Additionally, the fork protections are only partially removed, leaving the CI in an inconsistent state where some jobs are protected and others are not.
  • .github/workflows/ci.yml — the entire file needs attention due to the pull_request_target change and the inconsistent removal of fork guards.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["pull_request_target event\n(opened / synchronize / labeled / unlabeled)"] --> B{Is PR from fork?}
    B -- "No (same repo)" --> C[All jobs run normally]
    B -- "Yes (fork)" --> D{Approval gate?}
    D -- "MISSING ❌" --> E["Jobs run with secrets exposed\n(OPENAI, ANTHROPIC, BROWSERBASE, etc.)"]
    E --> F["server-integration-tests\n✅ fork check removed"]
    E --> G["run-evals\n✅ fork check removed"]
    E --> H["run-e2e-local-tests\n❌ fork check still present\n(job is skipped)"]
    E --> I["run-e2e-bb-tests\n❌ fork check still present\n(job is skipped)"]
    C --> J["run-build"]
    C --> K["run-lint"]
    C --> F
    C --> G
    C --> H
    C --> I
Loading

Last reviewed commit: 4167620

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 4, 2026

Additional Comments (2)

.github/workflows/ci.yml, line 9
pull_request_target exposes secrets to fork PRs without an approval gate

Switching from pull_request to pull_request_target is a well-documented security risk ("pwn requests"). With pull_request_target, the workflow always runs in the context of the base repository, which means it has full access to all repository secrets — even when triggered by a fork PR.

While the current checkout steps don't explicitly check out fork code (so fork-submitted scripts can't run), secrets like OPENAI_API_KEY, ANTHROPIC_API_KEY, BROWSERBASE_API_KEY, etc., are still injected as environment variables into jobs that run for fork PRs (e.g., server-integration-tests and run-evals).

The PR title mentions "when approved", but there is no actual approval gate implemented in this workflow. A common safe pattern is to require a maintainer to apply a specific label (e.g., safe-to-test) before any job with secrets is triggered:

on:
  pull_request_target:
    types:
      - opened
      - synchronize
      - labeled
      - unlabeled

jobs:
  check-approval:
    if: >
      github.event.pull_request.head.repo.full_name == github.repository ||
      contains(github.event.pull_request.labels.*.name, 'safe-to-test')
    ...

Without this gate, any contributor who opens a fork PR can trigger jobs that consume your API credits.


.github/workflows/ci.yml, line 521
Inconsistent fork checks across jobs

The run-e2e-local-tests (lines 472–474) and run-e2e-bb-tests (lines 519–521) jobs retain the github.event.pull_request.head.repo.full_name == github.repository guard, while the server-integration-tests (line 377) and run-evals (lines 567–576) jobs have had theirs removed. This is inconsistent.

As a result, fork PRs will run server integration tests and evals (which consume API credits for OPENAI, ANTHROPIC, and BROWSERBASE), but not the e2e local or Browserbase e2e tests. Either all jobs should allow fork PRs (with a proper approval gate in place), or none should — the current partial state is confusing and inconsistent.

Copy link
Contributor

@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

Confidence score: 1/5

  • There are two high-confidence, high-severity CI workflow issues in .github/workflows/ci.yml, including a 9/10 finding that can invalidate PR verification by running CI against the base branch instead of the submitted PR changes.
  • The pull_request_target + default actions/checkout@v4 behavior is merge-blocking risk because regressions can slip through untested, creating a strong chance of functional breakage after merge.
  • The removed fork guards on secret-using jobs (server-integration-tests, run-evals) create a concrete security exposure path for sensitive keys, which is critical and should be fixed before merging.
  • Pay close attention to .github/workflows/ci.yml - PR validation is misconfigured and secret-handling protections were weakened.
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.yml">

<violation number="1" location=".github/workflows/ci.yml:4">
P0: Critical: `pull_request_target` with default `actions/checkout@v4` checks out the **base branch**, not the PR code. The CI will never test any changes from the pull request — it will always run against the existing base branch and appear to pass.

To actually test the PR code, every `checkout` step needs `ref: ${{ github.event.pull_request.head.sha }}`. However, doing so with `pull_request_target` would expose all repository secrets to arbitrary fork code (see next comment). The recommended safe pattern is to keep `pull_request` for building/testing untrusted code, and only use `pull_request_target` for a separate lightweight workflow that gates on an approval label before dispatching the secret-requiring jobs.</violation>

<violation number="2" location=".github/workflows/ci.yml:377">
P1: Security: Fork-guard conditions removed from secret-using jobs. The `server-integration-tests` and `run-evals` jobs expose sensitive API keys (`OPENAI_API_KEY`, `ANTHROPIC_API_KEY`, `BROWSERBASE_API_KEY`, `BRAINTRUST_API_KEY`, etc.) and previously required `github.event.pull_request.head.repo.full_name == github.repository` to prevent fork PRs from accessing them.

With `pull_request_target`, secrets are available to all triggered jobs regardless of PR origin. Removing these guards means any fork PR can trigger resource-consuming jobs with secret access. Consider implementing a label-based approval gate (e.g., require an `approved-to-run` label set by a maintainer) before running secret-using jobs.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant Ext as Outside Contributor (Fork)
    participant GHA as GitHub Actions Runner
    participant Mnt as Maintainer
    participant CI as CI Workflow (.yml)
    participant Jobs as Integration & Eval Jobs

    Note over Ext, Jobs: Flow for Forked Pull Request

    Ext->>GHA: NEW: Push code & Open PR (pull_request_target)
    
    alt Needs Approval (GitHub Policy)
        Mnt->>GHA: Approve "Run Workflow"
    end

    GHA->>CI: Trigger Workflow in context of Base Repo
    
    Note over CI: Workflow has access to Secrets/Env

    CI->>CI: Run: build-server-sea / discover-tests
    
    rect rgb(23, 37, 84)
    Note right of CI: CHANGED: Guards removed (repo check)
    CI->>Jobs: NEW: Trigger server/integration tests
    CI->>Jobs: NEW: Trigger evaluation jobs
    end

    Jobs-->>GHA: Return Test Results
    GHA-->>Ext: Display CI Status in PR UI
Loading

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

@pirate pirate changed the title allow outside contributors to run prs when approved Allow outside contributors to run CI with secrets when approved Mar 4, 2026
@pirate pirate merged commit 02c31ef into main Mar 5, 2026
7 checks passed
pirate added a commit that referenced this pull request Mar 5, 2026
pirate added a commit that referenced this pull request Mar 5, 2026
…ed" (#1786)

Reverts #1782

<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Reverts the approval-based CI for external contributors. CI now runs on
pull_request and blocks secrets for forked PRs by skipping integration,
E2E, and eval jobs.

- **Refactors**
  - Removed the “Ensure Contributor Is Trusted to Run CI” workflow.
  - Switched CI trigger to pull_request; removed workflow_run logic.
  - Read labels from github.event.pull_request; removed API calls.
  - Simplified checkouts; dropped explicit head_sha refs.
  - Updated concurrency group to use github.ref.
  - Ignored docs-only changes in CI.

<sup>Written for commit d6ace82.
Summary will update on new commits. <a
href="https://cubic.dev/pr/browserbase/stagehand/pull/1786">Review in
cubic</a></sup>

<!-- End of auto-generated description by cubic. -->
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.

2 participants