Skip to content

move to npm trusted publishing#129

Closed
wpalmeri wants to merge 1 commit intomasterfrom
npm-trusted-publishing
Closed

move to npm trusted publishing#129
wpalmeri wants to merge 1 commit intomasterfrom
npm-trusted-publishing

Conversation

@wpalmeri
Copy link

Trust relationship already established on npmjs. Add provenance flag.

Trust relationship already established on npmjs. Add provenance flag.
Copilot AI review requested due to automatic review settings January 16, 2026 00:27
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the npm publishing workflow from token-based authentication to npm's trusted publishing using OIDC authentication. The trust relationship has already been established on npmjs.org, and this change adds the necessary GitHub Actions permissions and provenance flags to enable secure, token-less publishing.

Changes:

  • Added OIDC permissions (id-token: write and contents: read) to the workflow
  • Added --provenance flag to npm publish commands for transparency and attestation
  • Removed NODE_AUTH_TOKEN environment variable from publish steps

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +32 to +34
yarn publish --access public --provenance
sed -i -e 's#"@stellar/js-xdr"#"js-xdr"#' package.json
yarn publish
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
yarn publish --provenance
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

Yarn Classic (v1.x) does not support the --provenance flag. Based on the yarn.lock file showing 'yarn lockfile v1', this project uses Yarn Classic. The --provenance flag is only supported by npm v9.5.0+ and Yarn Berry (v2+). Consider using npm publish instead of yarn publish, or migrate to Yarn Berry if you want to continue using Yarn with provenance support.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Big if true… we’re overdue for a package manager port but yeah sounds like we can’t do it this way until that’s done

Copy link
Author

Choose a reason for hiding this comment

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

oof. we need to do this soon. npm has killed classic tokens and we can only provision 90 day tokens

Copy link
Author

Choose a reason for hiding this comment

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

current token is already expired i believe

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @Ryang-21, we should prioritize this 🥲

run: |
V=$(cat package.json | jq '.version' | sed -e 's/\"//g')
echo "Deprecating js-xdr@$V"
npm deprecate js-xdr@"<= $V" "⚠️ This package has moved to @stellar/js-xdr! 🚚"
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The npm deprecate command requires authentication but no longer has access to NODE_AUTH_TOKEN. With trusted publishing via OIDC, the authentication token should be automatically provided by the setup-node action when registry-url is configured. However, ensure that the npm deprecate command can authenticate using the OIDC token set up by setup-node. If this step fails, you may need to add an environment variable that references the token created by setup-node (typically available via NODE_AUTH_TOKEN environment variable that setup-node creates automatically).

Copilot uses AI. Check for mistakes.
@Ryang-21
Copy link
Contributor

Included with the pnpm migration in #130

@Ryang-21 Ryang-21 closed this Jan 20, 2026
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.

3 participants