-
Notifications
You must be signed in to change notification settings - Fork 1.8k
chore(ci): switch publish to OIDC trusted publishing #1838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
felixweinberger
wants to merge
5
commits into
main
Choose a base branch
from
fweinberger/oidc-trusted-publishing
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+6
−3
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
9aa6d77
chore(ci): switch publish to OIDC trusted publishing
felixweinberger 2a2ff06
Merge remote-tracking branch 'origin/main' into fweinberger/oidc-trus…
felixweinberger b7aad96
chore(ci): ensure npm >=11.5.1 for OIDC trusted publishing
felixweinberger 4e27fb1
chore(ci): clarify npm install comment (not a pin, a minimum version)
felixweinberger 5beda34
chore(ci): pin npm to exact 11.5.1 in publish job
felixweinberger File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟣 Pre-existing:
actions/checkout@v6andactions/setup-node@v6use floating semver tags in the publish job whilepnpm/action-setupandchangesets/actionin the same job are pinned to full commit SHAs. Consider pinning these to commit SHAs for consistency, though this PR actually improves the overall risk profile by replacing a long-lived NPM_TOKEN with a short-lived OIDC token.Extended reasoning...
In the publish job (which holds id-token: write and runs in the release environment), actions/checkout@v6 and actions/setup-node@v6 use floating semver tags, while pnpm/action-setup and changesets/action in the same job are pinned to full commit SHAs. This inconsistency is real and worth noting.
This inconsistency is entirely pre-existing — the floating @v6 tags predate this PR. The PR only adds an npm install step and removes NPM_TOKEN/NODE_AUTH_TOKEN env vars.
The refutation makes a compelling point that must be addressed: this PR actually improves the exfiltration risk profile. Before this PR, a compromised floating action in the publish job could exfiltrate the long-lived NPM_TOKEN secret (valid indefinitely, usable from anywhere). After this PR, the worst-case exfiltration is a short-lived OIDC token that expires quickly and can only be exchanged from within a trusted GitHub Actions context. Short-lived OIDC tokens are strictly less dangerous than long-lived static secrets — so the PR is a net improvement from a security standpoint.
Additionally, id-token: write was already present in the publish job before this PR (added in #1836), so the OIDC token was already exposed to these floating actions. This PR changes nothing about that existing exposure.
Mitigating factors: actions/checkout and actions/setup-node are official first-party GitHub-maintained actions. Compromising their v6 tag would require compromising GitHub's own infrastructure — a substantially higher bar than a third-party action. Many organizations deliberately exempt first-party GitHub actions from SHA-pinning requirements for this reason.
Step-by-step proof of inconsistency: (1) Attacker somehow moves the v6 tag on actions/checkout to a malicious commit. (2) The publish job runs and executes the malicious checkout action. (3) The compromised action can request a GitHub OIDC token via id-token: write. (4) However, the token is short-lived (~15 minutes) and the attacker must use it within that window. Compare this to the pre-PR state where the long-lived NPM_TOKEN was in the environment and would have been trivially exfiltrable to any external service.
Fix: Pin actions/checkout and actions/setup-node to their full commit SHAs, consistent with the other actions in the same job. Low-effort improvement for consistency.