Skip to content

fix: use npm trusted publishers for release#446

Closed
juanmahidalgo wants to merge 3 commits into
masterfrom
fix/release-npm-trusted-publishing
Closed

fix: use npm trusted publishers for release#446
juanmahidalgo wants to merge 3 commits into
masterfrom
fix/release-npm-trusted-publishing

Conversation

@juanmahidalgo

Copy link
Copy Markdown
Contributor

Context

The release job failed because npm now requires authentication via Trusted Publishers (OIDC) for publishing — the legacy NODE_AUTH_TOKEN / NPM_TOKEN flow is no longer accepted. The npm Trusted Publisher for decentraland-ui2 has already been configured.

This mirrors the setup already shipped in landing-site's build-release.yml.

Changes (master.yml release job)

  • Add contents: read alongside the existing id-token: write permission.
  • Set up Node.js 24 and force npm 11 via corepack — Trusted Publishing requires npm CLI ≥ 11.5.1.
  • Pin decentraland/oddish-action to the OIDC-aware commit (0074f2d…, master @ feat/support-npm-trusted-publishing), same pin landing-site uses.
  • Add provenance: true.
  • Remove NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} — with no token present, the OIDC-aware oddish skips writing _authToken and lets npm 11 do the Trusted Publisher handshake.

S3 snapshot publishing + GitLab CDN propagation (and their AWS env vars) are unchanged.

⚠️ Requirement

The npm Trusted Publisher entry for decentraland-ui2 must reference workflow filename master.yml in this repo. Confirm at https://www.npmjs.com/package/decentraland-ui2/access — if it was set to a different filename, publishing will still fail.

Test plan

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 9, 2026

Copy link
Copy Markdown

Deploying ui2 with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4756b40
Status: ✅  Deploy successful!
Preview URL: https://1b3095d6.ui2-423.pages.dev
Branch Preview URL: https://fix-release-npm-trusted-publ.ui2-423.pages.dev

View logs

@decentraland-bot decentraland-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review — fix: use npm trusted publishers for release

The overall direction is correct — migrating from NPM_TOKEN to OIDC Trusted Publishing is the right move, and the oddish-action SHA pin (0074f2d) and provenance: true are both good. Permissions are properly scoped (id-token: write + contents: read), NODE_AUTH_TOKEN removal is correct, and no secrets are exposed.

However, the corepack approach used here has a known reliability issue in this org.

Findings

[P1 — Blocker] corepack install -g npm@11 is unreliable with OIDC provenance

From prior experience in this org (the decentraland/protocol fix — PRs #416 and #417), corepack install -g is known to be unreliable when OIDC provenance is involved. Since this PR enables provenance: true, this applies directly.

The recommended fix (already applied in decentraland/protocol) is to use actions/setup-node@v6 with Node 24, which ships npm 11.x natively — no corepack step needed. This simplifies the workflow and eliminates the reliability risk.

Suggested fix: Replace the setup-node + corepack steps with:

- uses: actions/setup-node@<v6-sha> # @v6
  with:
    node-version: 24.x

…and remove the "Enable npm 11.x via corepack" step entirely.

[P2 — Minor] actions/checkout@main is unpinned

Pre-existing issue (not introduced by this PR), but actions/checkout@main tracks a moving target and is a supply chain risk. Per org conventions (actions/checkout v4→v6), it should be pinned to a SHA.


Summary

Check Status
PR title (ADR-6) fix: use npm trusted publishers for release
Branch naming fix/release-npm-trusted-publishing
CI checks ✅ All passing
Security ✅ No secrets exposure, OIDC is more secure than token auth
Consumer impact ✅ CI-only change, no public API affected
Reliability ❌ corepack + OIDC provenance known to be unreliable

Reviewed by Jarvis 🤖 · Requested by Juan Manuel Hidalgo via Slack

s3_bucket_key: ${{ steps.publish_package.outputs.s3-bucket-key }}
steps:
- uses: actions/checkout@main
- name: Use Node.js 24.x

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] actions/setup-node@v4.0.1 is outdated for Node 24. Use actions/setup-node@v6 (SHA-pinned) — it ships npm 11.x natively with Node 24, eliminating the need for the corepack step below.

From past org learnings (decentraland/protocol PRs #416/#417): corepack install -g npm@11 is unreliable when OIDC provenance is involved — which applies here since provenance: true is enabled.

id-token: write
contents: read
outputs:
s3_bucket_key: ${{ steps.publish_package.outputs.s3-bucket-key }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Pre-existing: actions/checkout@main should be pinned to a SHA (v6 recommended per org conventions). Tracking @main is a supply chain risk.

@juanmahidalgo

Copy link
Copy Markdown
Contributor Author

Addressed both findings in 8915670:

This completes the OIDC migration (NODE_AUTH_TOKEN already removed, provenance: true set). Reminder: the npm Trusted Publisher entry for decentraland-ui2 must reference workflow filename master.yml.

@decentraland-bot decentraland-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Both findings from the previous review are addressed:

  • P1 resolved: Replaced corepack install -g npm@11 with actions/setup-node@v6.4.0 + Node 24 (ships npm 11.x natively). SHA verified ✅
  • P2 resolved: actions/checkout@main → pinned to v6.0.3 SHA (df4cb1c0). SHA verified ✅

Everything else from the first pass remains solid: oddish-action SHA-pinned to 0074f2d, provenance: true, NODE_AUTH_TOKEN removed, permissions properly scoped.

LGTM — good to merge once CI is green. 🟢


Reviewed by Jarvis 🤖 · Requested by Juan Manuel Hidalgo via Slack

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