Skip to content

Revert "Allow outside contributors to run CI with secrets when approved"#1786

Merged
pirate merged 1 commit intomainfrom
revert-1782-allow-pr-tests
Mar 5, 2026
Merged

Revert "Allow outside contributors to run CI with secrets when approved"#1786
pirate merged 1 commit intomainfrom
revert-1782-allow-pr-tests

Conversation

@pirate
Copy link
Member

@pirate pirate commented Mar 5, 2026

Reverts #1782


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.

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

@changeset-bot
Copy link

changeset-bot bot commented Mar 5, 2026

⚠️ No Changeset found

Latest commit: d6ace82

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

@pirate pirate merged commit 748a673 into main Mar 5, 2026
12 checks passed
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.

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.
Architecture diagram
sequenceDiagram
    participant User as Contributor
    participant GH as GitHub Platform
    participant CI as CI Runner (Tests Workflow)
    participant Repo as Repository
    participant Secrets as GitHub Secrets
    participant API as External APIs (OpenAI/BB)

    User->>GH: Push PR / Add Label
    GH->>CI: NEW: Trigger on 'pull_request' event
    
    Note over CI: Determine Changes & Labels
    CI->>CI: CHANGED: Read labels directly from github.event
    CI->>Repo: CHANGED: Standard checkout (no head_sha ref required)

    alt PR is from internal branch (Same Repo)
        CI->>Secrets: Access OPENAI_API_KEY / ANTHROPIC_API_KEY
        Secrets-->>CI: Return Secrets
        
        rect rgb(5, 46, 22)
            Note right of CI: Run Integration / E2E / Evals
            CI->>API: Execute tests with API keys
            API-->>CI: Results
        end
    else NEW: PR is from a FORK
        rect rgb(127, 29, 29)
            Note right of CI: Security Boundary
            CI->>CI: Skip Integration, E2E, and Eval jobs
            Note over CI: Condition: head.repo.full_name != github.repository
        end
    end

    CI-->>GH: Report Status
    GH-->>User: Show Check Results
Loading

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 5, 2026

Greptile Summary

This PR reverts #1782, removing the two-step workflow_run + gating workflow approach for running CI with secrets on external contributor PRs, and returns to a direct pull_request trigger.

Key changes:

  • ci.yml trigger switches from workflow_run (watching the gate workflow) back to pull_request with opened, synchronize, labeled, unlabeled event types.
  • ensure-contributor-is-trusted-to-run-ci.yml (the no-op gate job that blocked until a maintainer approved the workflow run) is deleted entirely.
  • All jobs that consume repository secrets (server-integration-tests, run-e2e-local-tests, run-e2e-bb-tests, run-evals) are now guarded with github.event.pull_request.head.repo.full_name == github.repository to explicitly skip them for fork PRs.
  • Label detection is simplified: instead of making an API call to resolve PR labels and passing them via job outputs, the workflow now reads labels directly via github.event.pull_request.labels.*.name.
  • Concurrency group simplified from github.event.workflow_run.pull_requests[0].number to github.ref.
  • PULL_NUMBER for the coverage-status step is now github.event.pull_request.number (always set on pull_request events).

Security considerations: With a pull_request trigger (not pull_request_target), GitHub platform guarantees that repository secrets are not passed to workflows triggered by fork PRs. The added head.repo.full_name == github.repository guards provide explicit belt-and-suspenders protection. The trade-off vs PR #1782 is that external contributors can no longer run secret-dependent jobs (e2e, evals, server-integration) even after a maintainer approves their workflow run — these jobs will simply be skipped for fork PRs.

Confidence Score: 4/5

  • Safe to merge — the pull_request trigger does not expose repository secrets to fork PRs, and all secret-consuming jobs are additionally gated on head.repo.full_name == github.repository.
  • The revert is mechanically correct. GitHub's platform guarantee that repository secrets are withheld from pull_request workflows triggered by forks is the primary security control, and the explicit head.repo guards are sound belt-and-suspenders. Score is 4 rather than 5 only because the revert intentionally accepts a functional regression: external contributors can no longer run secret-dependent CI (e2e, evals, server-integration) even after a maintainer approves their workflow run, which may slow down contribution velocity for those contributors.
  • No files require special attention

Sequence Diagram

sequenceDiagram
    participant Fork as Fork/External PR
    participant Repo as Same-Repo PR
    participant GH as GitHub Actions
    participant CI as ci.yml

    note over Fork,CI: After this revert (pull_request trigger)

    Fork->>GH: Opens/updates PR
    GH->>CI: Triggers (pull_request event, NO secrets passed)
    CI->>CI: run-build, run-lint, core-unit-tests (runs, no secrets needed)
    CI->>CI: server-integration-tests / e2e / evals
    CI-->>Fork: Skipped (head.repo.full_name != github.repository guard)

    Repo->>GH: Opens/updates PR
    GH->>CI: Triggers (pull_request event, secrets available)
    CI->>CI: run-build, run-lint, core-unit-tests (runs)
    CI->>CI: server-integration-tests / e2e / evals
    CI-->>Repo: Runs with secrets (head.repo == github.repository ✓)

    note over Fork,CI: Before this revert (workflow_run two-step gate)
    Fork->>GH: Opens/updates PR
    GH->>GH: ensure-contributor-is-trusted-to-run-ci.yml (BLOCKED pending approval)
    GH-->>GH: Maintainer approves workflow run
    GH->>CI: Triggers via workflow_run (with secrets after approval)
    CI-->>Fork: Runs all jobs WITH secrets
Loading

Last reviewed commit: d6ace82

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