Add npm trusted-publishing support + ci.yaml publish workflow#34
Conversation
Distribute the volcano CLI via npm as a thin wrapper (@kong/volcano-cli): - package.json with bin shim, files allowlist, postinstall, public access - bin/volcano.js launcher execs the platform binary, forwarding args/exit code - scripts/npm/download.js downloads + SHA256-verifies the binary from the matching GitHub Release; self-heals on first run if postinstall is skipped - .github/workflows/ci.yaml publishes to npm via OIDC trusted publishing (id-token: write, no NPM_TOKEN) on stable release publish - README + .gitignore updates
There was a problem hiding this comment.
Pull request overview
This PR adds an npm-distributed wrapper package (@kong/volcano-cli) that downloads a matching GitHub Release binary (verified via SHA256SUMS) and introduces a GitHub Actions workflow to publish that package to npm using trusted publishing (OIDC).
Changes:
- Introduces an npm package (
package.json) + launcher shim (bin/volcano.js) to run the platform-specific Release binary. - Adds Node scripts to download and SHA256-verify Release assets (
scripts/npm/*) with a non-fatal postinstall hook. - Adds an npm publish workflow triggered by stable GitHub Releases (
.github/workflows/ci.yaml) and updates docs/ignore rules.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/npm/install.js |
Adds non-fatal postinstall hook to download the CLI binary (or defer to first run). |
scripts/npm/download.js |
Implements platform resolution, release URL construction, download, and SHA256 verification. |
bin/volcano.js |
Adds launcher shim that downloads on demand and execs the native binary with argument/stdio passthrough. |
package.json |
Defines @kong/volcano-cli package metadata, published files allowlist, scripts, and publish config. |
.github/workflows/ci.yaml |
Adds OIDC-based npm publish workflow triggered by stable release: published. |
README.md |
Documents npm installation flow and environment variable to skip postinstall download. |
.gitignore |
Ignores downloaded platform binaries and node_modules/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…write, pin npm - download.js: select http/https client by URL scheme (supports http mirrors), reject unsupported schemes with a clear error - download.js: remove partial temp file on download failure; clear destination before rename so force re-download / Windows overwrite works - ci.yaml: pin npm to the 11 line instead of npm@latest for reproducibility
- Rename .github/workflows/ci.yaml -> publish-npm.yml (parallels publish-cli.yml; avoids confusion with the existing ci.yml Go checks) - bin/volcano.js: handle unsupported platforms (e.g. win32-arm64, which passes the package.json os/cpu gate) with a clean error instead of an unhandled promise rejection; add a last-resort .catch() on main() - publish-npm.yml: scope id-token: write to the publish job (least privilege); widen release-asset verification retries (~90s) to ride out the window where the release event can fire before all assets finish uploading
- download.js: hash the fully-written temp file instead of a mid-stream data listener; verifies exactly what landed on disk and avoids flowing-mode subtleties (#5) - publish-npm.yml: verify assets with a ranged GET (first byte) instead of a HEAD request, confirming they are actually downloadable like postinstall does; GitHub honors Range so only 1 byte is fetched (#6)
swkeever
left a comment
swkeever
left a comment
There was a problem hiding this comment.
I found one issue in the automatic npm publish path.
|
|
||
| on: | ||
| # Fires when publish-cli.yml publishes the GitHub Release for a version tag. | ||
| release: |
There was a problem hiding this comment.
The normal stable release path creates the GitHub Release from publish-cli.yml using GH_TOKEN: ${{ github.token }}. GitHub does not enqueue new workflow runs for events caused by GITHUB_TOKEN other than documented exceptions, and release is not one of them: https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/trigger-a-workflow#triggering-a-workflow-from-a-workflow. That means this release: published trigger will not run after the automated CLI release, so the npm package only publishes via manual workflow_dispatch. Please either publish npm from the existing release workflow after the assets are uploaded, dispatch this workflow explicitly (workflow_dispatch/repository_dispatch with the needed permissions), or create the release with a GitHub App/PAT if the release event should fan out.
marckong
left a comment
marckong
left a comment
There was a problem hiding this comment.
Review
Careful, well-documented change making the repo npm publish-compatible via trusted publishing (OIDC). I traced asset naming and SHA256SUMS format against publish-cli.yml / install-volcano.sh and they line up (parseChecksum, TARGETS, and the v<version> tag mapping all match), and node --check passes on all three JS files.
Edge cases I'd normally hunt for are already handled: win32-arm64 caught by resolveTarget() throwing, non-fatal postinstall with first-run self-heal, signal re-raising, partial-download cleanup, and the 0.0.0 publish guard. No blocking bugs found.
Overall correctness: correct. Existing code/tests are unaffected and the patch is free of blocking issues. Just the one already-documented operational caveat: the one-time npm Trusted Publisher setup (Workflow filename: publish-npm.yml) must be configured or publishing will fail — expected, not a code defect.
Two optional non-blocking notes inline below.
| return; | ||
| } | ||
|
|
||
| const target = resolveTarget(); |
There was a problem hiding this comment.
[P3] When run in this repo (or before npm version sets a real version), pkg.version is 0.0.0, so ensureBinary() tries to fetch from the nonexistent v0.0.0 release and prints a download warning. Harmless since postinstall is non-fatal, but you could short-circuit when the version is 0.0.0 to avoid the noise for maintainers running npm install locally.
| // against the release's SHA256SUMS manifest. Idempotent: no-op if present. | ||
| async function ensureBinary({ force = false } = {}) { | ||
| const dest = binaryPath(); | ||
| if (!force && fs.existsSync(dest)) { |
There was a problem hiding this comment.
[P3] ensureBinary (and the existsSync check in bin/volcano.js) skip checksum re-verification when the binary already exists, so a previously corrupt or truncated download won't self-heal on subsequent runs. Acceptable given the best-effort self-heal design — just noting the assumption.
Summary
Makes the repo
npm publish-compatible via npm trusted publishing (OIDC) and adds thepublish-npm.ymlworkflow that runsnpm publish.The CLI is distributed as a thin npm wrapper (
@kong/volcano-cli) around the existing cosign-signed GitHub Release binaries — no long-livedNPM_TOKENrequired.Changes
package.json—@kong/volcano-cliwithbin: volcano → bin/volcano.js,filesallowlist (published tarball is ~9 kB — no Go source or 10 MB binary),postinstall,publishConfig.access=public, andrepository.urlmatching the GitHub repo (required for trusted publishing).bin/volcano.js— launcher that execs the platform binary, forwarding args, stdio, and exit code; downloads on first run if the binary is missing. Unsupported platforms (e.g. Windows arm64) fail with a clean error instead of an unhandled rejection.scripts/npm/download.js— maps platform → release target, downloads from the matchingv<version>release, and verifies against that release'sSHA256SUMS(pure Node, no cosign at install time).scripts/npm/install.js— postinstall hook (non-fatal sonpm installstill succeeds offline / with--ignore-scripts)..github/workflows/publish-npm.yml— publishes to npm via OIDC (id-token: writescoped to the publish job, GitHub-hosted runner,npm@11for trusted-publishing support ≥ 11.5.1), version derived from the release tag, asset-existence guard, thennpm publish. Triggers on stablerelease: published(+workflow_dispatch). Named to parallelpublish-cli.yml(CLI → GitHub releases, npm → registry) and avoid confusion with the existingci.ymlGo checks.README.md/.gitignore— npm install docs; ignore downloaded binaries +node_modules.Verification
node --checkon all JS,npm pack --dry-runcontents reviewednpm install → runcycle against the realv0.0.4release: downloaded, checksum-verified, ranvolcano --version, confirmed exit-code passthroughFollow-ups / notes
Kong, Repository:volcano-cli, Workflow filename:publish-npm.yml(must match exactly, including extension). See https://docs.npmjs.com/trusted-publishers#troubleshooting.publish-cli.ymlcreates the GitHub Release (the npm package downloads those assets). Only stablevX.Y.Zreleases publish to npm; nightlies are skipped (different asset naming). Can add anightlydist-tag if wanted.SHA256SUMSover TLS rather than running cosign (which the shell installer does). This keeps the package dependency-free at install; npm provenance still attests the wrapper. Cosign verification in postinstall could be added later if desired.