Skip to content

Conversation

@patrickarlt
Copy link
Contributor

This PR sets up trusted publishing for @esri/arcgis-rest-* packages. Long lived personal tokens are on the way out after continued supply chain attacks.

This PR switches to use NPM Trusted Publishing according to this documentation:

Then we switch to using the GitHub actions provided GITHUB_TOKEN instead of the token in the repo secrets for the Git operations that make up the rest of the publishing process according to these docs:

This PR will also need to updated to remove the --legacy-peer-deps flags if #1272 merges first.

Copy link
Contributor

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 modernizes the release process by implementing NPM's trusted publishing mechanism, eliminating the need for long-lived personal access tokens in favor of GitHub's OIDC-based authentication. This change enhances security by removing stored credentials and aligns with NPM's strengthened authentication requirements.

Key changes:

  • Configured trusted publishing with OIDC for NPM package releases
  • Replaced custom tokens with GitHub's built-in GITHUB_TOKEN for Git operations
  • Updated Node.js version from 18 to 24 for the release workflow

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
.github/workflows/release.yml Updated to use trusted publishing with OIDC, added necessary permissions, upgraded Node.js to v24, and switched from custom tokens to GITHUB_TOKEN
.github/workflows/manual-release.yml Removed manual release workflow (no longer needed with trusted publishing setup)
RELEASE.md Updated documentation to reflect trusted publishing configuration and removed references to deprecated token management

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

Copy link
Contributor

@gavinr-maps gavinr-maps left a comment

Choose a reason for hiding this comment

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

Looks good! one question:

@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
arcgis-rest-basemap-sessions.src 100% 100% 0
arcgis-rest-basemap-sessions.src.utils 100% 100% 0
arcgis-rest-demographics.src 100% 100% 0
arcgis-rest-developer-credentials.src 100% 100% 0
arcgis-rest-developer-credentials.src.shared 100% 100% 0
arcgis-rest-elevation.src 100% 100% 0
arcgis-rest-feature-service.src 100% 100% 0
arcgis-rest-geocoding.src 100% 100% 0
arcgis-rest-places.src 100% 100% 0
arcgis-rest-portal.src.groups 100% 100% 0
arcgis-rest-portal.src.items 100% 100% 0
arcgis-rest-portal.src.orgs 100% 100% 0
arcgis-rest-portal.src.services 100% 100% 0
arcgis-rest-portal.src.sharing 100% 100% 0
arcgis-rest-portal.src.users 100% 100% 0
arcgis-rest-portal.src.util 100% 100% 0
arcgis-rest-request.src 100% 100% 0
arcgis-rest-request.src.types 100% 100% 0
arcgis-rest-request.src.utils 100% 100% 0
arcgis-rest-routing.src 100% 100% 0
Summary 100% (2186 / 2186) 100% (1217 / 1217) 0

Minimum allowed line rate is 100%

Copy link
Contributor

@dixonyant dixonyant left a comment

Choose a reason for hiding this comment

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

LGTM! I left only one comment.

matrix:
os: [ubuntu-latest]
node: [18]
node: [24]
Copy link
Contributor

Choose a reason for hiding this comment

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

will we need to retroactively go through the other yml files and update them do remove legacy-peer-deps and update the node versions to 24 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dixonyant yes eventually we want to only be on LTS versions of Node in these tests but I wanted to make sure we were up to date here since have to use NPM 11.5.1

@patrickarlt
Copy link
Contributor Author

@dixonyant @gavinr-maps going to merge this and see if we get a release of the request package!

@patrickarlt patrickarlt merged commit d041cd3 into main Nov 24, 2025
2 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.

4 participants