Skip to content

fix: trusted publishing#158

Merged
varonix0 merged 1 commit intomainfrom
daniel/fix-trusted-publishing
Mar 25, 2026
Merged

fix: trusted publishing#158
varonix0 merged 1 commit intomainfrom
daniel/fix-trusted-publishing

Conversation

@varonix0
Copy link
Copy Markdown
Member

@varonix0 varonix0 commented Mar 25, 2026

Description 📣

Fixed trusted publishing misconfiguration. There are a few issues:

  1. Trusted publishing is not supported in Node 20. We had to upgrade to Node V22 which comes with a newer version of npm.
  2. The package.json were misconfigured to point to the incorrect repo. (Unrelated to trusted publishing)

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

Tests 🛠️



Open with Devin

@varonix0 varonix0 self-assigned this Mar 25, 2026
@varonix0 varonix0 requested a review from 0xArshdeep March 25, 2026 20:38
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@varonix0 varonix0 merged commit a6eb233 into main Mar 25, 2026
7 checks passed
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 25, 2026

Greptile Summary

This PR fixes misconfigured npm trusted publishing by upgrading Node.js from 20 to 22 (shipping with npm 10.x, which supports OIDC-based trusted publishing), adding registry-url to the setup-node action so that npm authentication is properly configured via .npmrc, removing the now-redundant --registry flag from the npm publish command, and correcting the repository URL in npm/package.json from the main monorepo to the correct CLI repository.

Key changes:

  • Node upgraded from 20'22' — required for npm trusted publishing support
  • registry-url: "https://registry.npmjs.org" added to setup-node — correctly sets up npm auth via OIDC when combined with id-token: write (already present at the job level)
  • --registry=https://registry.npmjs.org/ removed from the npm publish command — now handled by the .npmrc generated by setup-node, which is the correct approach
  • npm/package.json repository URL corrected from Infisical/infisical.gitInfisical/cli.git

Pre-existing issue surfaced by this PR: The npm publish --tarball=./infisical-sdk-${{github.ref_name}} command uses a non-existent --tarball flag and a filename that does not match what npm pack produces. In practice npm likely falls back to publishing from the working directory, but this is worth correcting explicitly (see inline comment).

Confidence Score: 4/5

  • Safe to merge — the core trusted publishing fixes are correct, with one pre-existing publish command issue worth addressing
  • The two changed files are small and focused. The Node upgrade, registry-url addition, and removal of the redundant --registry flag are all correct steps for npm trusted publishing. The package.json URL fix is straightforward. The only concern (flagged inline) is a pre-existing bug with the --tarball flag and filename mismatch in the publish command, which is not introduced by this PR but may prevent the intended tarball-based publish path from working correctly.
  • .github/workflows/release_build_infisical_cli.yml — the npm publish command's tarball argument should be revisited

Important Files Changed

Filename Overview
.github/workflows/release_build_infisical_cli.yml Upgrades Node.js to 22, adds registry-url for OIDC trusted publishing, and removes the now-redundant --registry flag from npm publish. Changes are correct for enabling trusted publishing; however, the pre-existing --tarball flag (not a valid npm publish option) and mismatched tarball filename pattern may cause publish to fall back to publishing from the current directory rather than the packed tarball.
npm/package.json Corrects the repository URL from the main infisical monorepo to the correct CLI-specific repository. A straightforward and correct fix.

Reviews (1): Last reviewed commit: "fix: trusted publishing" | Re-trigger Greptile

- name: Publish NPM
working-directory: ${{ env.working-directory }}
run: npm publish --tarball=./infisical-sdk-${{github.ref_name}} --access public --registry=https://registry.npmjs.org/
run: npm publish --tarball=./infisical-sdk-${{github.ref_name}} --access public
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 --tarball is not a valid npm publish flag

npm publish does not accept a --tarball flag — the tarball path is a positional argument, not a named option. Additionally, the filename pattern ./infisical-sdk-${{github.ref_name}} (e.g. ./infisical-sdk-v1.2.3) does not match what npm pack actually produces for the @infisical/cli package (which would be infisical-cli-<version>.tgz).

In practice, npm likely ignores the unrecognised --tarball option and publishes from the current ./npm working directory instead — so the workflow may still function, but not in the intended way (re-packing from source on every publish rather than using the explicitly packed tarball).

The correct form is to pass the tarball as a positional argument:

Suggested change
run: npm publish --tarball=./infisical-sdk-${{github.ref_name}} --access public
run: npm publish ./infisical-cli-${{ env.CLI_VERSION }}.tgz --access public

Or, to reliably capture the exact tarball path produced by npm pack, capture its output in the Pack step:

- name: Pack NPM
  working-directory: ${{ env.working-directory }}
  run: echo "TARBALL=$(npm pack --json | jq -r '.[0].filename')" >> $GITHUB_ENV

- name: Publish NPM
  working-directory: ${{ env.working-directory }}
  run: npm publish "${{ env.TARBALL }}" --access public

Note: this bug is pre-existing and not introduced by this PR, but worth fixing while the publish step is being touched.

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