[rehearsal] validate secretless external-fork E2E path - #2382
Closed
pratikspatil024 wants to merge 6 commits into
Closed
pratikspatil024 wants to merge 6 commits into
pratikspatil024 wants to merge 6 commits into
Conversation
Fork PRs never receive secrets or an OIDC token on `pull_request`, so all three kurtosis e2e legs were structurally unable to run on external contributions. Reviewers had no e2e signal at merge time. Add a `pull_request_target` trigger so fork PRs run these legs with base-repo credentials against the fork's head commit, gated on the repo's outside-collaborator approval setting. The workflow definition always comes from the base branch, so a fork cannot rewrite the steps; each new push needs approving again. The two PR triggers are made mutually exclusive per job, since both firing would report the same check name twice. `Checkout bor` now pins the ref explicitly: `pull_request_target` defaults to the base branch, which would silently test develop instead of the PR. The build jobs gain an explicit read-only token, which fork `pull_request` runs previously got for free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both `pull_request` and `pull_request_target` resolve `github.event.pull_request.number` to the same value, so a PR created one run per trigger and both landed in the same concurrency group. With `cancel-in-progress: true`, whichever was queued second cancelled the other — so the all-skipped run could cancel the real one, nondeterministically, on any PR rather than only fork PRs. Add `github.event_name` to the group. Cancellation within one event is unchanged, so successive pushes to a PR still supersede the previous run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Under `pull_request_target` these jobs check out fork-authored code with base-repo credentials. `actions/checkout` leaves the token in the local repo config, `COPY . .` ships that directory into the build context, and the built image is saved into a publicly downloadable artifact — so a PR-modified Dockerfile could read the token out of the build context. The version stamping in the build reads local tags, so the directory has to stay in the context; dropping the persisted credential is the narrower fix. Nothing in these jobs needs authenticated access after checkout, and neither repo fetches submodules here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new job-level permission restrictions omit required actions scopes for artifact upload/download, which can cause the workflows to fail when transferring the built Docker image artifacts between jobs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the Kurtosis E2E GitHub Actions workflows to support running safely on external fork pull_request events without relying on repository secrets or OIDC, enabling a “secretless” dependency setup path for fork-authored code.
Changes:
- Add
reopenedto thepull_requesttrigger types for all Kurtosis E2E workflows. - Make checkouts safer for fork PRs by disabling persisted credentials (
persist-credentials: false) and scoping job permissions. - Add a fork-specific setup path that skips authenticated registry setup and installs required tools (Kurtosis CLI,
yq, Foundry) using public artifacts.
File summaries
| File | Description |
|---|---|
| .github/workflows/kurtosis-e2e.yml | Adds fork-safe setup path, restricts permissions, and disables persisted checkout credentials for the main E2E leg. |
| .github/workflows/kurtosis-stateless-e2e.yml | Mirrors the fork-safe setup path and permissions hardening for the stateless E2E workflow. |
| .github/workflows/kurtosis-pipeline-e2e.yml | Mirrors the fork-safe setup path and permissions hardening for the pipeline E2E workflow. |
Review details
Suppressed comments (3)
.github/workflows/kurtosis-e2e.yml:46
actions/upload-artifactneeds the workflow token to haveactions: write; with job permissions restricted tocontents: readthe heimdall image artifact upload can fail.
build-heimdall-v2:
permissions:
contents: read
.github/workflows/kurtosis-stateless-e2e.yml:46
actions/upload-artifactneeds the workflow token to haveactions: write; with job permissions restricted tocontents: readthe heimdall image artifact upload can fail.
build-heimdall-v2:
permissions:
contents: read
.github/workflows/kurtosis-pipeline-e2e.yml:52
actions/upload-artifactneeds the workflow token to haveactions: write; with job permissions restricted tocontents: readthe heimdall image artifact upload can fail.
build-heimdall-v2:
permissions:
contents: read
- Files reviewed: 3/3 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
20
to
+22
| build-bor: | ||
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
.github/workflows/kurtosis-e2e.yml
Comment on lines
78
to
80
| permissions: | ||
| contents: read | ||
| actions: write | ||
| id-token: write |
There was a problem hiding this comment.
.github/workflows/kurtosis-e2e.yml
Comment on lines
26
to
+28
| build-bor: | ||
| permissions: | ||
| contents: read |
Comment on lines
84
to
86
| permissions: | ||
| contents: read | ||
| actions: write | ||
| id-token: write |
Comment on lines
20
to
+22
| build-bor: | ||
| permissions: | ||
| contents: read |
Comment on lines
78
to
80
| permissions: | ||
| contents: read | ||
| actions: write | ||
| id-token: write |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The fork path’s pip install approach is likely to be unreliable on ubuntu-latest due to externally-managed Python environments unless installed to user site and added to PATH.
Review details
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
.github/workflows/kurtosis-e2e.yml:123
python3 -m pip install ...on ubuntu-latest can fail due to the system Python environment being externally managed (PEP 668), and even when it succeeds theyqentrypoint may land in~/.local/binwhich is not guaranteed to be onPATH. Installing with--userand explicitly adding~/.local/bintoGITHUB_PATHmakes the fork path more reliable.
.github/workflows/kurtosis-pipeline-e2e.yml:138python3 -m pip install ...on ubuntu-latest can fail due to the system Python environment being externally managed (PEP 668), and even when it succeeds theyqentrypoint may land in~/.local/binwhich is not guaranteed to be onPATH. Installing with--userand explicitly adding~/.local/bintoGITHUB_PATHmakes the fork path more reliable.
.github/workflows/kurtosis-stateless-e2e.yml:123python3 -m pip install ...on ubuntu-latest can fail due to the system Python environment being externally managed (PEP 668), and even when it succeeds theyqentrypoint may land in~/.local/binwhich is not guaranteed to be onPATH. Installing with--userand explicitly adding~/.local/bintoGITHUB_PATHmakes the fork path more reliable.
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Temporary rehearsal — DO NOT MERGE.
This PR reproduces the changes from #2380 through a genuine fork-origin pull_request event. It validates that the trusted setup is skipped, the secretless fork setup runs without OIDC, registry credentials, or repository secrets, and all E2E legs can execute using public dependencies.
This PR will be closed after the rehearsal completes.