fix: publish npm binaries as per-platform optionalDependencies#447
Merged
Conversation
The @ory/cli npm package used binwrap to download the CLI binary at install time. binwrap is unmaintained and depends on the deprecated request package, whose transitive dependencies trigger unfixable critical npm audit findings in every consuming project. The npm package now follows the esbuild pattern: the release pipeline publishes one package per platform (e.g. @ory/cli-linux-x64) containing just the prebuilt binary, and @ory/cli itself ships a dependency-free launcher plus exact-version optionalDependencies on those packages. npm's os/cpu fields ensure only the binary matching the consumer's platform is downloaded. Install scripts and all runtime npm dependencies are gone, and npm audit reports zero vulnerabilities. Prereleases are now published under the "next" dist-tag instead of "latest", and Windows arm64 binaries are now published to npm. Closes #410 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NaL51TwfZAAEEWU2v5CNWT
📝 WalkthroughWalkthroughThe package now uses a platform-specific optional dependency layout. A new launcher resolves and executes the matching binary, while ChangesPlatform-specific npm distribution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)CLI launch flowsequenceDiagram
participant npm
participant npm_run_js
participant PlatformPackage
npm->>npm_run_js: invoke CLI
npm_run_js->>PlatformPackage: resolve matching binary
npm_run_js->>PlatformPackage: spawn binary
PlatformPackage-->>npm_run_js: return exit status or signal
Release publishing flowsequenceDiagram
participant CI
participant publish_js
participant GitHub_Releases
participant npm_registry
CI->>publish_js: pass release ref
publish_js->>GitHub_Releases: download platform archive
publish_js->>publish_js: build and smoke-test packages
publish_js->>npm_registry: publish platform packages
publish_js->>npm_registry: publish root package
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/ci.yaml:
- Around line 95-99: Update the publish step around npm/publish.js so
github.ref_name is passed via a dedicated environment variable rather than
interpolated directly into the shell script. Invoke node npm/publish.js using
that environment variable, preserving the existing NPM_TOKEN handling and
publish behavior.
🪄 Autofix (Beta)
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 Plus
Run ID: ed4e7db3-a4e0-4bab-9bf1-6dbb3c61ae22
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.github/workflows/ci.yaml.npmignorenpm/index.jsnpm/publish.jsnpm/run.jspackage.json
💤 Files with no reviewable changes (2)
- .npmignore
- npm/index.js
Pass github.ref_name to npm/publish.js via an environment variable instead of interpolating it into the run script, so a crafted tag name cannot inject shell commands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NaL51TwfZAAEEWU2v5CNWT
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yaml (1)
84-100: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDeclare least-privilege permissions for the publish job.
This job publishes through
NPM_TOKENand does not appear to need GitHub write access, but it inherits the repository’s defaultGITHUB_TOKENpermissions. Add an explicit job-level permission block, such ascontents: read, and grant anything additional only ifory/ci/checkout@masterrequires it.🛡️ Proposed fix
npm-publish: name: Publish to npm runs-on: ubuntu-latest + permissions: + contents: read if: ${{ github.ref_type == 'tag' }}🤖 Prompt for 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. In @.github/workflows/ci.yaml around lines 84 - 100, Update the npm-publish job to declare an explicit least-privilege permissions block with contents: read for checkout, and add no broader GitHub permissions unless required by ory/ci/checkout@master. Keep the existing NPM_TOKEN publishing flow unchanged.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 84-100: Update the npm-publish job to declare an explicit
least-privilege permissions block with contents: read for checkout, and add no
broader GitHub permissions unless required by ory/ci/checkout@master. Keep the
existing NPM_TOKEN publishing flow unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e35d0d59-0483-4d0f-bb3b-8cb6d16a2b6e
📒 Files selected for processing (1)
.github/workflows/ci.yaml
jonas-jonas
approved these changes
Jul 22, 2026
jonas-jonas
left a comment
jonas-jonas
left a comment
Member
There was a problem hiding this comment.
Pretty cool, much neater than binwrap. I like, let's try it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
npm auditflags every project that installs@ory/cli(#410). The package used binwrap to download the CLI binary at install time; binwrap is unmaintained and hard-depends on the deprecatedrequestpackage, pulling in vulnerableform-data,tough-cookie,qs,uuid, andtarversions with no fix available. As of today that's 8 findings (3 critical) in a fresh install. Theoverridesadded in #409 didn't help consumers, since npm only honorsoverridesfrom the root package of an install.Solution
Adopt the esbuild/rollup pattern of per-platform binary packages:
npm/run.js— dependency-free launcher thatbin.orypoints to. Mapsprocess.platform/process.archto the matching platform package, resolves the binary, and spawns it with inherited stdio, propagating exit codes and signals.npm/publish.js— release-time script run by thenpm-publishCI job. Downloads the goreleaser archives from the GitHub release, extracts each binary into a minimal@ory/cli-<platform>-<arch>package (os/cpufields make npm install only the matching one), smoke-tests the host binary, publishes the six platform packages first, then injects exact-versionoptionalDependenciesintopackage.jsonand publishes@ory/cliitself.binwrap, theinstall/preparescripts, and all runtime npm dependencies are removed.@ory/cliis now a 5.6 kB package with four files and zero install scripts.v1.3.1-pre.0) now publish under thenextdist-tag instead oflatest.The
optionalDependenciesmap is deliberately not checked in: it is injected at publish time, because checked-in entries pointing at not-yet-published versions would 404 on every devnpm install.Verification
--dry-runagainst the real v1.3.1 and v1.3.1-pre.0 GitHub releases: all six platform packages build, the host smoke test (ory version) passes, and dist-tags resolve tolatest/nextrespectively.npx ory versionruns the real binary, stdio and exit codes propagate, andnpm auditreports 0 vulnerabilities (vs. 8 with the published 1.2.0).Notes for reviewers
@orynpm scope —NPM_TOKEN_AENEASRmust be allowed to create packages there (--access publicis passed).node npm/publish.js v1.3.1and an npm token.Closes #410
🤖 Generated with Claude Code
https://claude.ai/code/session_01NaL51TwfZAAEEWU2v5CNWT
Summary by CodeRabbit
New Features
Improvements