Skip to content

ci: publish to npm via Trusted Publishing (OIDC) [FEPLAT-5048] - #52

Merged
dan-phantom merged 7 commits into
masterfrom
dan/feplat-5048-trusted-publishing
Aug 12, 2026
Merged

ci: publish to npm via Trusted Publishing (OIDC) [FEPLAT-5048]#52
dan-phantom merged 7 commits into
masterfrom
dan/feplat-5048-trusted-publishing

Conversation

@dan-phantom

@dan-phantom dan-phantom commented Aug 10, 2026

Copy link
Copy Markdown

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 and id-token: write. Node 20 ships npm 10.x, so the job fell back to NPM_PUBLISH_TOKEN_PHANTOM_SECURITY_BOT, created 26 Jan for 90 days and dead since April.

  • release.yml: install npm 11.5.1, grant id-token: write, drop the npm token. Verified on this PR before merge, not assumed: the token exchange returns HTTP 201, and a dry-run publish logs npm 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.
  • release.yml: NPM_CONFIG_LOGLEVEL: verbose. npm's oidc() never throws; it logs at verbose and returns undefined, then the publish continues unauthenticated and reports only E404. That silence is why the January debugging never found a cause. Also adds workflow_dispatch to 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.
  • package.json: repository.url needs the git+ 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.

@dan-phantom
dan-phantom marked this pull request as draft August 10, 2026 16:28
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f30bbbeb-2b07-4089-9bb5-9d13eecf32a9

📥 Commits

Reviewing files that changed from the base of the PR and between be1b79e and a259a2a.

📒 Files selected for processing (1)
  • .github/workflows/release.yml
💤 Files with no reviewable changes (1)
  • .github/workflows/release.yml

📝 Walkthrough

Walkthrough

The 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 git+https.

Changes

Release versioning and npm publishing

Layer / File(s) Summary
Release-triggered npm publishing
.github/workflows/release.yml
The workflow adds manual dispatch support, enables OIDC permissions, pins action revisions, installs npm 11.5.1, replaces token authentication with Trusted Publishing, and enables verbose npm logging.
Package repository metadata
package.json
The repository URL uses the git+https protocol.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the main change: publishing to npm through Trusted Publishing with OIDC.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dan/feplat-5048-trusted-publishing

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4b99048 and 89483bb.

📒 Files selected for processing (3)
  • .github/workflows/publish.yml
  • .github/workflows/release.yml
  • package.json

Comment thread .github/workflows/publish.yml Outdated
Comment thread .github/workflows/publish.yml Outdated
@dan-phantom
dan-phantom force-pushed the dan/feplat-5048-trusted-publishing branch from 89483bb to 45c354a Compare August 11, 2026 07:35

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 89483bb and 45c354a.

📒 Files selected for processing (1)
  • .github/workflows/release.yml

Comment thread .github/workflows/release.yml
Comment thread .github/workflows/release.yml
Comment thread .github/workflows/release.yml
Comment thread .github/workflows/release.yml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between e79af25 and be1b79e.

📒 Files selected for processing (1)
  • .github/workflows/release.yml

Comment thread .github/workflows/release.yml Outdated
@dan-phantom
dan-phantom marked this pull request as ready for review August 12, 2026 09:23
@dan-phantom
dan-phantom requested review from a team and tian000 August 12, 2026 09:25
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.
@dan-phantom
dan-phantom force-pushed the dan/feplat-5048-trusted-publishing branch from 0dd6170 to 3ce70cd Compare August 12, 2026 13:54
@dan-phantom
dan-phantom merged commit 17d8ca6 into master Aug 12, 2026
12 checks passed
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