Fork-PR checkouts: stop persisting the write token (follow-up to #1134) - #1143
Merged
Merged
Conversation
…flows) Follow-up to #1134, applying the reviewer-flagged mitigation: under pull_request_target the checkout otherwise persists the contents:write GITHUB_TOKEN into .git/config, where fork-controlled npm scripts can read it. Credentials are now persisted only when the head is same-repo — the same condition that already gates the git-auto-commit steps, which keep working unchanged. Fork-PR jobs become read-only. Full restructure (read-only fork jobs + separate trusted write-back job) stays with the pull_request_target review in service-operations#1083. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents fork-controlled frontend jobs from accessing persisted write credentials while preserving existing behavior for trusted runs.
Changes:
- Disables persisted checkout credentials for fork PRs.
- Retains credentials for same-repository PRs and non-PR runs.
- Applies the mitigation to all six checkout sites.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/workflows/shared-npm-publish.yaml |
Secures both checkout steps. |
.github/workflows/shared-frontend-build.yaml |
Secures all four frontend checkout steps. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Applies the mitigation Copilot's review flagged as High on #1134, which was merged with that item scoped out.
Problem: under
pull_request_target,actions/checkoutpersists thecontents: writeGITHUB_TOKENinto.git/configby default. The lint/check-types/build jobs then run fork-controlled code (npm cilifecycle hooks, npm scripts), which can read that credential and push to the base repo. Gating only the auto-commit step does not protect the token.Change (6 checkout sites across both shared workflows):
Same-repo PRs and push/tag runs keep the token — the
git-auto-commitsteps keep working unchanged (this is the exact condition that already gates them). Fork-PR jobs become read-only: build/lint still run on therefs/pull/<n>/headcheckout from #1134, but no credential is on disk.Deliberately not here: the full restructure (read-only fork jobs + a separate trusted write-back job, as in the upstream reference) — that belongs to the
pull_request_targetsecurity review in service-operations#1083, where these workflows are now in scope. This PR just removes the cheapest, highest-value part of the attack surface today.After merge: forward-merge
2.4 → 2026.1propagates this into 2026.1's copies of the same files (verified they exist there); the files don't exist above 2026.1, so the rest of the chain is ancestry-only.🤖 Generated with Claude Code