ci: publish to npm via Trusted Publishing (OIDC) [FEPLAT-5048] - #52
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe release workflow now uses npm Trusted Publishing through OIDC, supports manual dispatch, pins setup actions, installs npm 11.5.1, and enables verbose npm logging. The package repository URL now uses ChangesRelease versioning and npm publishing
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish.yml:
- Line 21: Update the workflow_dispatch configuration and publishing flow so
manual runs require a release-tag input, use that tag for checkout rather than
the dispatcher-selected ref, and verify the corresponding GitHub Release exists
and is published before npm publish. Ensure the existing release-tag validation
also applies to manually dispatched runs.
- Around line 40-47: Update the Node.js version configured in the “Setup Node.js
20.x” actions/setup-node step to 22.14.0 or later, ensuring the publish workflow
supports the required npm Trusted Publishing CLI version. Keep the existing npm
upgrade step and setup configuration otherwise unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 004a6c2f-15ab-4a71-9e30-eda70b81ccd9
📒 Files selected for processing (3)
.github/workflows/publish.yml.github/workflows/release.ymlpackage.json
89483bb to
45c354a
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 37-45: Update the “Setup Node.js 20.x” step to use Node.js 22.14.0
or newer, while preserving the npm 11.5.1 installation required for Trusted
Publishing.
- Line 29: Restructure the release workflow so dependency installation and
release-PR handling run in a separate job without id-token permission, while npm
publication runs in an isolated protected job that alone grants id-token: write.
Update job dependencies and pass any required release outputs between the jobs,
keeping changesets/action release handling and npm publishing functional.
- Around line 14-18: Restrict manual releases to the master branch by adding an
if guard to the release job that permits execution only when github.ref is
refs/heads/master. Keep the existing push trigger and workflow_dispatch trigger
unchanged, and apply the guard to the job that runs yarn release with id-token
write.
- Around line 31-32: Update the actions/checkout step in the release workflow to
set persist-credentials to false, ensuring the GitHub token is not retained for
subsequent yarn and Changesets operations; preserve the existing checkout action
and version.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58f1f7dd-04a3-4aec-90cc-afe67efa0b39
📒 Files selected for processing (1)
.github/workflows/release.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 77-111: Remove the entire probe-publish-auth job from the
workflow, including its pull-request condition, id-token: write permission,
dependency installation, and dry-run publish steps. Do not replace it with
another OIDC probe or retain any part of this job.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8bed8f01-f9d4-47d5-b4a2-393be5ba93ce
📒 Files selected for processing (1)
.github/workflows/release.yml
The 1.2.4 publish fails with `E404 Not Found - PUT .../@phantom%2freact-native-webview`, which is npm reporting a failed auth, so two merged security fixes are still not on npm. Security has a trusted publisher registered for this package, but the job cannot use it: Node 20 ships npm 10.x, and the OIDC exchange needs npm >= 11.5.1. The job therefore falls back to `NPM_PUBLISH_TOKEN_PHANTOM_SECURITY_BOT`, created 26 Jan and never rotated. - **release.yml**: install npm 11.5.1 before the changesets step, and grant `id-token: write`. Same shape as phantom/libs, where changesets publishes over OIDC today. The npm token is gone, so the secret can be deleted. - **release.yml**: add `workflow_dispatch` to retry a publish for a version that is already merged, such as 1.2.4. Also move setup-node off v3, which actionlint reports as too old for current runners. - **package.json**: `repository.url` needs the `git+` prefix. Provenance validation under trusted publishing rejects the bare https form.
…LAT-5048] npm's OIDC exchange fails silently (lib/utils/oidc.js logs at verbose and returns undefined), so `npm publish` continues unauthenticated and reports E404. That is why the January debugging never found a cause. These two temporary jobs run only the exchange, print the claims and the real error, and publish nothing. The pair differs only by GitHub environment, the one claim no previous attempt ever set. Remove both jobs and the pull_request trigger before merge.
…048] The first probe answered the open question: the npm trusted publisher accepts this workflow file, with or without a GitHub environment (HTTP 201 in both jobs), so the missing-environment theory is wrong and no npm-side change is needed. What is still untested is the publish chain itself, where January's attempts died. npm runs the OIDC exchange before it honors --dry-run, so this job validates the npm version, PATH and workspace state without uploading anything. It fails the build if the exchange does not succeed. The real publish step now runs at verbose log level so a silent OIDC failure prints its cause. Remove this job and the pull_request trigger before merge.
The probes answered both open questions and are removed. The npm trusted publisher accepts this workflow file with or without a GitHub environment (HTTP 201 both ways), and a dry-run publish reported `npm verbose oidc Successfully retrieved and set token` with npm 11.5.1 in this workspace. They stay in the git history if the next failure needs them again.
workflow_dispatch lets a write-authorized user pick any ref, and the job then publishes the version that ref holds with id-token: write. Restrict the job to master (CodeRabbit, CWE-862).
Review feedback from CodeRabbit. - **persist-credentials: false** on both checkouts. changesets runs with commitMode: github-api, so src/git.ts routes pushTag and pushChanges through Octokit and no step needs local git auth (CWE-522). - **node 22.x**. npm documents Node >= 22.14.0 for Trusted Publishing. The earlier probe passed on 20.20.2, which satisfies npm 11.5.1's engines field, but matching the documented floor costs nothing. - The dry-run probe is back for one run to prove the exchange still works on 22.x. Remove it, and the pull_request trigger, before merge.
The dry-run probe confirmed the exchange on node v22.23.1 with npm 11.5.1: POST 201 to the exchange endpoint, then `npm verbose oidc Successfully retrieved and set token`, with nothing uploaded. Removed, and the rationale comment now names the right Node version.
0dd6170 to
3ce70cd
Compare
The 1.2.4 publish fails with
E404 Not Found - PUT .../@phantom%2freact-native-webview, so two merged Android security fixes never reached npm and the wallet app still ships 1.2.3. npm's OIDC exchange needs npm >= 11.5.1 andid-token: write. Node 20 ships npm 10.x, so the job fell back toNPM_PUBLISH_TOKEN_PHANTOM_SECURITY_BOT, created 26 Jan for 90 days and dead since April.id-token: write, drop the npm token. Verified on this PR before merge, not assumed: the token exchange returnsHTTP 201, and a dry-run publish logsnpm verbose oidc Successfully retrieved and set token. Nothing was uploaded, since npm runs the exchange before it honors--dry-run. No npm-side change is needed - the trusted publisher already accepts this workflow file, with or without a GitHub environment.NPM_CONFIG_LOGLEVEL: verbose. npm'soidc()never throws; it logs at verbose and returnsundefined, then the publish continues unauthenticated and reports only E404. That silence is why the January debugging never found a cause. Also addsworkflow_dispatchto retry a publish for an already merged version such as 1.2.4, and moves setup-node off v3, which actionlint rejects as too old for current runners.repository.urlneeds thegit+prefix, which provenance validation under trusted publishing expects.After merge, the push to master should publish 1.2.4 and tag it. The token secret and its bypass-2FA npm token can then be deleted; npm is deprecating that token class anyway.
Pre-existing and unrelated: iOS CI fails on this branch, and it failed the same way on the first push before any of these CI changes.