From 4db164a53fb64c4016e01451d9b346cffe2ebab8 Mon Sep 17 00:00:00 2001 From: liuooo Date: Fri, 29 May 2026 20:01:41 +0800 Subject: [PATCH 01/14] feat(npm): add @mininglamp-oss/octo-cli npm wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Distribute octo-cli via npm for Node-based agent runtimes. The package is a thin wrapper: postinstall downloads the prebuilt Go binary matching the package version and host platform from the GitHub Release, verifies its sha256, and the `octo-cli` bin shim execs it. - npm/ — package.json (bin: octo-cli), install.js (download + checksum + extract, with a friendly PATH hint on global installs when npm's bin dir is not on PATH), run.js shim, README - .github/workflows/npm-publish.yml — publishes on GitHub Release; aligned with octo-adapters' publish flow: SHA-pinned actions, `permissions: {}`, concurrency guard, --access public, NODE_AUTH_TOKEN, a prerelease→@next / stable→@latest dist-tag rule, and a workflow_dispatch dry-run path - .gitignore — ignore npm/bin/ (the downloaded binary) - README — npm install section The published npm version tracks a real release tag; install.js downloads octo-cli___ from that release, so publishing requires a release whose goreleaser archives use the octo-cli_ prefix (v0.6.0+). --- .github/workflows/npm-publish.yml | 76 ++++++++++++++ .gitignore | 2 + README.md | 10 ++ npm/README.md | 23 ++++ npm/package.json | 39 +++++++ npm/scripts/install.js | 168 ++++++++++++++++++++++++++++++ npm/scripts/run.js | 27 +++++ 7 files changed, 345 insertions(+) create mode 100644 .github/workflows/npm-publish.yml create mode 100644 npm/README.md create mode 100644 npm/package.json create mode 100644 npm/scripts/install.js create mode 100644 npm/scripts/run.js diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml new file mode 100644 index 0000000..81b4d55 --- /dev/null +++ b/.github/workflows/npm-publish.yml @@ -0,0 +1,76 @@ +# Publish the npm wrapper (@mininglamp-oss/octo-cli) when a GitHub Release is +# published. goreleaser builds and uploads the platform binaries to the same +# release; the npm package's postinstall downloads the matching one, so the +# published npm version MUST correspond to a real release tag. +# +# Tag → npm dist-tag: +# v1.2.3 → @latest (matches /^v\d+\.\d+\.\d+$/) +# v1.2.3-rc.1 → @next (any tag containing "-") +# +# A manual workflow_dispatch run supports --dry-run for testing without +# uploading. +name: npm publish + +on: + release: + types: [published] + workflow_dispatch: + inputs: + tag: + description: "Release tag to publish (e.g. v0.6.0)" + required: true + type: string + dry_run: + description: "Run npm publish with --dry-run (no actual upload)" + required: false + type: boolean + default: true + +concurrency: + group: npm-publish-${{ github.ref }} + cancel-in-progress: false + +permissions: {} + +jobs: + publish: + runs-on: ubuntu-latest + defaults: + run: + working-directory: npm + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: "20" + registry-url: "https://registry.npmjs.org" + + - name: Resolve version and dist-tag + id: version + # On a release event GITHUB_REF_NAME is the tag; on manual dispatch use + # the `tag` input. VERSION drops the leading `v`; a version containing + # `-` (prerelease) publishes to @next so it never clobbers @latest. + run: | + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + REF="${{ inputs.tag }}" + DRY="${{ inputs.dry_run && '--dry-run' || '' }}" + else + REF="${GITHUB_REF_NAME}" + DRY="" + fi + VERSION="${REF#v}" + if [[ "$VERSION" == *-* ]]; then DIST_TAG="next"; else DIST_TAG="latest"; fi + { + echo "VERSION=$VERSION" + echo "DIST_TAG=$DIST_TAG" + echo "DRY_RUN=$DRY" + } >> "$GITHUB_OUTPUT" + echo "[npm-publish] version=$VERSION dist-tag=$DIST_TAG dry_run='$DRY'" + + - name: Set package version + run: npm version "${{ steps.version.outputs.VERSION }}" --no-git-tag-version --allow-same-version + + - name: Publish to npm + env: + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + run: npm publish --access public --tag "${{ steps.version.outputs.DIST_TAG }}" ${{ steps.version.outputs.DRY_RUN }} diff --git a/.gitignore b/.gitignore index 856dab0..5f5e945 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ # Binary (root only) /octo-cli /bin/ +# npm postinstall downloads the prebuilt binary here — never commit it +/npm/bin/ coverage.out # Go diff --git a/README.md b/README.md index d3bf4ea..d211a39 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,16 @@ Key properties: ## Installation +### npm + +For Node-based agent runtimes (OpenClaw, etc.): + +```bash +npm install -g @mininglamp-oss/octo-cli +``` + +A thin wrapper that downloads the matching prebuilt binary on install. + ### Go install ```bash diff --git a/npm/README.md b/npm/README.md new file mode 100644 index 0000000..8820796 --- /dev/null +++ b/npm/README.md @@ -0,0 +1,23 @@ +# @mininglamp-oss/octo-cli + +npm distribution of [`octo-cli`](https://github.com/Mininglamp-OSS/octo-cli) — the +command-line interface for the Octo ecosystem, built for AI Agent Bots. + +```bash +npm install -g @mininglamp-oss/octo-cli +octo-cli --help +``` + +This package is a thin Node wrapper around the prebuilt Go binary. On install it +downloads the binary matching your platform and this package's version from the +[GitHub Release](https://github.com/Mininglamp-OSS/octo-cli/releases) and +verifies its checksum; the `octo-cli` command then execs that binary directly. + +Supported platforms: macOS, Linux, Windows on `x64` / `arm64`. + +For other install methods (Homebrew, raw binary, `go install`) and full usage, +see the [main README](https://github.com/Mininglamp-OSS/octo-cli#readme). + +## License + +[Apache-2.0](https://github.com/Mininglamp-OSS/octo-cli/blob/main/LICENSE) diff --git a/npm/package.json b/npm/package.json new file mode 100644 index 0000000..7a749ab --- /dev/null +++ b/npm/package.json @@ -0,0 +1,39 @@ +{ + "name": "@mininglamp-oss/octo-cli", + "version": "0.0.0", + "description": "Command-line interface for the Octo ecosystem — a single-binary REST client for AI Agent Bots. This package downloads the matching prebuilt Go binary on install.", + "bin": { + "octo-cli": "scripts/run.js" + }, + "scripts": { + "postinstall": "node scripts/install.js" + }, + "files": [ + "scripts/", + "README.md" + ], + "os": [ + "darwin", + "linux", + "win32" + ], + "cpu": [ + "x64", + "arm64" + ], + "engines": { + "node": ">=18" + }, + "keywords": [ + "octo", + "cli", + "ai-agent", + "bot" + ], + "homepage": "https://github.com/Mininglamp-OSS/octo-cli#readme", + "repository": { + "type": "git", + "url": "git+https://github.com/Mininglamp-OSS/octo-cli.git" + }, + "license": "Apache-2.0" +} diff --git a/npm/scripts/install.js b/npm/scripts/install.js new file mode 100644 index 0000000..9d9e414 --- /dev/null +++ b/npm/scripts/install.js @@ -0,0 +1,168 @@ +#!/usr/bin/env node +"use strict"; + +// postinstall: download the prebuilt octo-cli binary that matches this package +// version and the host platform from the GitHub Release, verify its checksum, +// and extract it into ../bin. Mirrors goreleaser's archive naming +// (octo-cli___.tar.gz, .zip on Windows; os/arch lowercase). + +const fs = require("fs"); +const path = require("path"); +const https = require("https"); +const crypto = require("crypto"); +const { execFileSync } = require("child_process"); + +const REPO = "Mininglamp-OSS/octo-cli"; +const VERSION = require("../package.json").version; + +const OS = { darwin: "darwin", linux: "linux", win32: "windows" }[process.platform]; +const ARCH = { x64: "amd64", arm64: "arm64" }[process.arch]; +const isWin = process.platform === "win32"; + +function fail(msg) { + console.error(`\n[octo-cli] install failed: ${msg}`); + console.error(`[octo-cli] Grab a binary manually from https://github.com/${REPO}/releases\n`); + process.exit(1); +} + +// GET with redirect following (GitHub release assets 302 to a CDN). +function download(url, redirects = 0) { + return new Promise((resolve, reject) => { + if (redirects > 5) { + reject(new Error("too many redirects")); + return; + } + https + .get(url, { headers: { "User-Agent": "octo-cli-npm-installer" } }, (res) => { + if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) { + res.resume(); + resolve(download(res.headers.location, redirects + 1)); + return; + } + if (res.statusCode !== 200) { + res.resume(); + reject(new Error(`HTTP ${res.statusCode} for ${url}`)); + return; + } + const chunks = []; + res.on("data", (c) => chunks.push(c)); + res.on("end", () => resolve(Buffer.concat(chunks))); + }) + .on("error", reject); + }); +} + +// On a global install, warn (without modifying anything) if the directory npm +// links the `octo-cli` command into is not on PATH, so the command would not be +// found. Best-effort: any failure here must not break the install. +function maybeHintPath() { + try { + const isGlobal = + process.env.npm_config_global === "true" || process.env.npm_config_location === "global"; + if (!isGlobal) return; + + // npm links bins into /bin on unix, and into itself on Windows. + let prefix = process.env.npm_config_prefix || process.env.PREFIX; + if (!prefix) { + // Fallback: walk up from /lib/node_modules/@scope/pkg/scripts (unix) + // or /node_modules/@scope/pkg/scripts (Windows). + prefix = path.resolve(__dirname, "..", "..", "..", "..", isWin ? ".." : "../.."); + } + const binDir = isWin ? prefix : path.join(prefix, "bin"); + + const onPath = (process.env.PATH || "") + .split(path.delimiter) + .some((p) => p && path.resolve(p) === path.resolve(binDir)); + if (onPath) return; + + const lines = [ + "", + `[octo-cli] Installed, but ${binDir} is not on your PATH —`, + "[octo-cli] the `octo-cli` command will not be found until you add it.", + ]; + if (isWin) { + lines.push("[octo-cli] Add it for your user (then reopen the terminal):"); + lines.push(` setx PATH "%PATH%;${binDir}"`); + } else { + const shell = path.basename(process.env.SHELL || ""); + const rc = + shell === "zsh" + ? "~/.zshrc" + : shell === "fish" + ? "~/.config/fish/config.fish" + : shell === "bash" + ? "~/.bashrc" + : "your shell profile"; + if (shell === "fish") { + lines.push(`[octo-cli] Add this to ${rc}, then reopen the terminal:`); + lines.push(` fish_add_path ${binDir}`); + } else { + lines.push(`[octo-cli] Add this to ${rc}, then reopen the terminal:`); + lines.push(` export PATH="${binDir}:$PATH"`); + } + } + lines.push(""); + console.error(lines.join("\n")); + } catch { + /* hint is best-effort; never fail the install over it */ + } +} + +async function main() { + if (!OS || !ARCH) fail(`unsupported platform ${process.platform}/${process.arch}`); + if (!VERSION || VERSION === "0.0.0") { + fail("package version is a placeholder (0.0.0); this package must be published with a real release version"); + } + + const ext = isWin ? "zip" : "tar.gz"; + const asset = `octo-cli_${VERSION}_${OS}_${ARCH}.${ext}`; + const base = `https://github.com/${REPO}/releases/download/v${VERSION}`; + const binName = isWin ? "octo-cli.exe" : "octo-cli"; + const binDir = path.join(__dirname, "..", "bin"); + + console.log(`[octo-cli] downloading ${asset} ...`); + let archive, sums; + try { + archive = await download(`${base}/${asset}`); + sums = (await download(`${base}/checksums.txt`)).toString("utf8"); + } catch (e) { + fail(e.message); + } + + // Verify sha256 against checksums.txt (" " per line). + const entry = sums + .split("\n") + .map((l) => l.trim().split(/\s+/)) + .find((p) => p[1] === asset); + if (!entry) fail(`no checksum entry for ${asset}`); + const got = crypto.createHash("sha256").update(archive).digest("hex"); + if (got !== entry[0]) fail(`checksum mismatch for ${asset} (want ${entry[0]}, got ${got})`); + + // Extract just the binary into bin/. bsdtar (macOS/Windows) and GNU tar + // (Linux) both handle the formats we ship (.tar.gz via -xzf, .zip via -xf). + fs.mkdirSync(binDir, { recursive: true }); + const tmp = path.join(binDir, asset); + fs.writeFileSync(tmp, archive); + try { + const args = isWin ? ["-xf", tmp, "-C", binDir, binName] : ["-xzf", tmp, "-C", binDir, binName]; + execFileSync("tar", args, { stdio: "inherit" }); + fs.chmodSync(path.join(binDir, binName), 0o755); + } catch (e) { + fail(`extract failed: ${e.message}`); + } finally { + try { + fs.unlinkSync(tmp); + } catch { + /* ignore */ + } + } + + console.log(`[octo-cli] installed octo-cli ${VERSION} (${OS}/${ARCH})`); + maybeHintPath(); +} + +// Run as a postinstall script; also importable (e.g. to unit-test maybeHintPath). +if (require.main === module) { + main(); +} +module.exports = { maybeHintPath }; diff --git a/npm/scripts/run.js b/npm/scripts/run.js new file mode 100644 index 0000000..998b11f --- /dev/null +++ b/npm/scripts/run.js @@ -0,0 +1,27 @@ +#!/usr/bin/env node +"use strict"; + +// Thin shim: forward all args and stdio to the prebuilt Go binary that +// scripts/install.js placed in ../bin during postinstall. +const path = require("path"); +const { spawnSync } = require("child_process"); + +const binName = process.platform === "win32" ? "octo-cli.exe" : "octo-cli"; +const bin = path.join(__dirname, "..", "bin", binName); + +const res = spawnSync(bin, process.argv.slice(2), { stdio: "inherit" }); + +if (res.error) { + if (res.error.code === "ENOENT") { + console.error( + "[octo-cli] binary not found — the postinstall download may have failed.\n" + + "[octo-cli] Try reinstalling: npm install -g @mininglamp-oss/octo-cli" + ); + } else { + console.error(`[octo-cli] ${res.error.message}`); + } + process.exit(1); +} + +// Propagate the child's exit code; if killed by a signal, use 1. +process.exit(res.status === null ? 1 : res.status); From 0508a548df7ae1241fd6d9c9dfbef103295fcda9 Mon Sep 17 00:00:00 2001 From: liuooo Date: Sat, 30 May 2026 10:32:15 +0800 Subject: [PATCH 02/14] fix(npm): harden the installer and shim per PR #27 review Address review feedback on the postinstall download path and the run-shim's exit-code propagation. None of these change the happy-path behaviour, only the failure modes. install.js - Per-request timeout (30s) with req.on('timeout') destroy, so a half-open TCP connection or a stalled CDN can no longer hang `npm install` forever. - HTTPS-only + host allowlist (github.com, objects.githubusercontent.com, codeload.github.com) on every redirect hop, so a redirect target outside GitHub's release-asset path is refused instead of silently fetched. - Strict semver assertion on VERSION before it is interpolated into the download URL or the on-disk filename. - Strict checksum format check (`^[0-9a-f]{64}$`) on the expected digest parsed from checksums.txt, so a malformed entry is rejected explicitly instead of relying on the compare to fail. - 3-attempt exponential backoff (500ms / 1s / 2s) for transient errors (timeout, socket reset, 5xx). 404 is treated as non-retryable so the wrong version fails fast. - Friendly ENOENT message for missing `tar` / `tar.exe` (older Windows, minimal container images) instead of a bare `extract failed: ENOENT`. - 200 MiB response-size cap on the buffered body as a defensive guard. - PATH-hint fallback corrected: when `npm_config_prefix` is unset, the previous walk-up landed one directory above the real prefix (e.g. `/usr` instead of `/usr/local`), so the printed `export PATH` hint pointed at the wrong bin dir. Drops one `..` (5 levels on unix, 4 on Windows). run.js - If the binary is killed by a signal, re-raise the same signal so callers see the conventional 128+signum exit code (e.g. 130 for SIGINT) instead of a generic 1 that hides Ctrl-C from shells and supervisors. --- npm/scripts/install.js | 140 ++++++++++++++++++++++++++++++++++------- npm/scripts/run.js | 15 ++++- 2 files changed, 130 insertions(+), 25 deletions(-) diff --git a/npm/scripts/install.js b/npm/scripts/install.js index 9d9e414..81a15b1 100644 --- a/npm/scripts/install.js +++ b/npm/scripts/install.js @@ -19,39 +19,112 @@ const OS = { darwin: "darwin", linux: "linux", win32: "windows" }[process.platfo const ARCH = { x64: "amd64", arm64: "arm64" }[process.arch]; const isWin = process.platform === "win32"; +// Redirects are followed only to these hosts. github.com is the first hop; +// release assets redirect to objects.githubusercontent.com via S3; codeload +// covers source archives. Anything else is treated as hostile. +const ALLOWED_HOSTS = new Set([ + "github.com", + "objects.githubusercontent.com", + "codeload.github.com", +]); + +const MAX_REDIRECTS = 5; +const REQUEST_TIMEOUT_MS = 30_000; +const MAX_RETRIES = 3; +const MAX_DOWNLOAD_BYTES = 200 * 1024 * 1024; // 200 MiB, far above any sane archive + function fail(msg) { console.error(`\n[octo-cli] install failed: ${msg}`); console.error(`[octo-cli] Grab a binary manually from https://github.com/${REPO}/releases\n`); process.exit(1); } -// GET with redirect following (GitHub release assets 302 to a CDN). -function download(url, redirects = 0) { +function assertSafeHost(u, label) { + if (u.protocol !== "https:") { + throw new Error(`${label}: refusing non-https URL (${u.protocol}//${u.hostname})`); + } + if (!ALLOWED_HOSTS.has(u.hostname)) { + throw new Error(`${label}: host '${u.hostname}' is not in the redirect allowlist`); + } +} + +// Single HTTPS GET. Resolves to either a buffered body or a redirect target, +// never both. Times out on socket idle so a half-open connection cannot hang +// the install indefinitely. +function getOnce(url) { return new Promise((resolve, reject) => { - if (redirects > 5) { - reject(new Error("too many redirects")); - return; - } - https - .get(url, { headers: { "User-Agent": "octo-cli-npm-installer" } }, (res) => { - if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) { + const req = https.get( + url, + { + headers: { "User-Agent": "octo-cli-npm-installer" }, + timeout: REQUEST_TIMEOUT_MS, + }, + (res) => { + const status = res.statusCode || 0; + if (status >= 300 && status < 400 && res.headers.location) { res.resume(); - resolve(download(res.headers.location, redirects + 1)); + resolve({ kind: "redirect", next: new URL(res.headers.location, url) }); return; } - if (res.statusCode !== 200) { + if (status !== 200) { res.resume(); - reject(new Error(`HTTP ${res.statusCode} for ${url}`)); + const err = new Error(`HTTP ${status} for ${url}`); + err.httpStatus = status; + reject(err); return; } const chunks = []; - res.on("data", (c) => chunks.push(c)); - res.on("end", () => resolve(Buffer.concat(chunks))); - }) - .on("error", reject); + let size = 0; + res.on("data", (c) => { + size += c.length; + if (size > MAX_DOWNLOAD_BYTES) { + req.destroy(new Error(`response exceeded ${MAX_DOWNLOAD_BYTES} bytes for ${url}`)); + return; + } + chunks.push(c); + }); + res.on("end", () => resolve({ kind: "body", body: Buffer.concat(chunks) })); + res.on("error", reject); + }, + ); + req.on("timeout", () => { + req.destroy(new Error(`request timed out after ${REQUEST_TIMEOUT_MS}ms: ${url}`)); + }); + req.on("error", reject); }); } +async function downloadOnce(url) { + let current = url; + for (let hop = 0; hop <= MAX_REDIRECTS; hop++) { + assertSafeHost(new URL(current), hop === 0 ? "request" : `redirect #${hop}`); + const r = await getOnce(current); + if (r.kind === "body") return r.body; + current = r.next.toString(); + } + throw new Error(`too many redirects (>${MAX_REDIRECTS})`); +} + +// Retry the whole download on transient errors (timeout, socket reset, 5xx). +// 404 is treated as a non-retryable signal that the version doesn't exist on +// the release yet — fast-fail with a clear error rather than spin. +async function download(url) { + for (let attempt = 0; ; attempt++) { + try { + return await downloadOnce(url); + } catch (e) { + const status = e && e.httpStatus; + const retryable = + attempt < MAX_RETRIES - 1 && + (status === undefined || (status >= 500 && status < 600)); + if (!retryable) throw e; + const delay = 500 * Math.pow(2, attempt); // 500ms, 1s, 2s + console.error(`[octo-cli] ${e.message} — retrying in ${delay}ms`); + await new Promise((r) => setTimeout(r, delay)); + } + } +} + // On a global install, warn (without modifying anything) if the directory npm // links the `octo-cli` command into is not on PATH, so the command would not be // found. Best-effort: any failure here must not break the install. @@ -64,9 +137,12 @@ function maybeHintPath() { // npm links bins into /bin on unix, and into itself on Windows. let prefix = process.env.npm_config_prefix || process.env.PREFIX; if (!prefix) { - // Fallback: walk up from /lib/node_modules/@scope/pkg/scripts (unix) - // or /node_modules/@scope/pkg/scripts (Windows). - prefix = path.resolve(__dirname, "..", "..", "..", "..", isWin ? ".." : "../.."); + // Fallback: __dirname is /lib/node_modules/@scope/pkg/scripts (unix, + // 5 levels under prefix) or /node_modules/@scope/pkg/scripts + // (Windows, 4 levels). + prefix = isWin + ? path.resolve(__dirname, "..", "..", "..", "..") + : path.resolve(__dirname, "..", "..", "..", "..", ".."); } const binDir = isWin ? prefix : path.join(prefix, "bin"); @@ -113,6 +189,11 @@ async function main() { if (!VERSION || VERSION === "0.0.0") { fail("package version is a placeholder (0.0.0); this package must be published with a real release version"); } + // The version is interpolated into both the download URL and a filesystem + // path; reject anything that isn't strict semver before either use. + if (!/^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/.test(VERSION)) { + fail(`package version '${VERSION}' is not a valid semver`); + } const ext = isWin ? "zip" : "tar.gz"; const asset = `octo-cli_${VERSION}_${OS}_${ARCH}.${ext}`; @@ -134,12 +215,18 @@ async function main() { .split("\n") .map((l) => l.trim().split(/\s+/)) .find((p) => p[1] === asset); - if (!entry) fail(`no checksum entry for ${asset}`); + if (!entry || entry.length < 2) fail(`no checksum entry for ${asset}`); + const expected = entry[0]; + if (!/^[0-9a-f]{64}$/.test(expected)) { + fail(`malformed checksum entry for ${asset}: '${expected}'`); + } const got = crypto.createHash("sha256").update(archive).digest("hex"); - if (got !== entry[0]) fail(`checksum mismatch for ${asset} (want ${entry[0]}, got ${got})`); + if (got !== expected) fail(`checksum mismatch for ${asset} (want ${expected}, got ${got})`); - // Extract just the binary into bin/. bsdtar (macOS/Windows) and GNU tar - // (Linux) both handle the formats we ship (.tar.gz via -xzf, .zip via -xf). + // Extract just the binary into bin/. bsdtar (macOS/Windows 10 1803+) and + // GNU tar (Linux) both handle the formats we ship (.tar.gz via -xzf, .zip + // via -xf). On older Windows / minimal containers tar may be absent — give + // a targeted message instead of a bare ENOENT. fs.mkdirSync(binDir, { recursive: true }); const tmp = path.join(binDir, asset); fs.writeFileSync(tmp, archive); @@ -148,6 +235,13 @@ async function main() { execFileSync("tar", args, { stdio: "inherit" }); fs.chmodSync(path.join(binDir, binName), 0o755); } catch (e) { + if (e && e.code === "ENOENT") { + fail( + isWin + ? "`tar.exe` not found on PATH. Windows 10 build 1803+ ships bsdtar; on older systems install Git for Windows or 7-Zip and re-run." + : "`tar` not found on PATH. Install GNU tar or bsdtar (e.g. apt-get install tar, apk add tar) and re-run.", + ); + } fail(`extract failed: ${e.message}`); } finally { try { diff --git a/npm/scripts/run.js b/npm/scripts/run.js index 998b11f..747aec2 100644 --- a/npm/scripts/run.js +++ b/npm/scripts/run.js @@ -15,7 +15,7 @@ if (res.error) { if (res.error.code === "ENOENT") { console.error( "[octo-cli] binary not found — the postinstall download may have failed.\n" + - "[octo-cli] Try reinstalling: npm install -g @mininglamp-oss/octo-cli" + "[octo-cli] Try reinstalling: npm install -g @mininglamp-oss/octo-cli", ); } else { console.error(`[octo-cli] ${res.error.message}`); @@ -23,5 +23,16 @@ if (res.error) { process.exit(1); } -// Propagate the child's exit code; if killed by a signal, use 1. +// If the binary was killed by a signal, re-raise the same signal so callers +// observe the conventional 128+signum exit code (e.g. 130 for SIGINT) instead +// of a generic 1 that hides Ctrl-C from shells and supervisors. +if (res.signal) { + process.kill(process.pid, res.signal); + // process.kill is async w.r.t. the event loop on some platforms; fall back + // to the conventional exit code so we don't return success by accident. + const SIGNUMS = { SIGHUP: 1, SIGINT: 2, SIGQUIT: 3, SIGKILL: 9, SIGTERM: 15 }; + process.exit(128 + (SIGNUMS[res.signal] || 0)); +} + +// Propagate the child's exit code. process.exit(res.status === null ? 1 : res.status); From ecd87911de4b41afc7ff59c145d02af04ea6059b Mon Sep 17 00:00:00 2001 From: liuooo Date: Sat, 30 May 2026 10:32:33 +0800 Subject: [PATCH 03/14] fix(ci): npm-publish waits for goreleaser artifacts before publishing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `release: published` event fires the moment a release is moved out of draft, but `release-publish.yml` runs `build-artifacts` (goreleaser) only after that, so npm-publish was racing the upload. If publish won the race — or if the artifact build failed — npm would permanently contain a version whose postinstall has nothing to download. Block the publish step until every expected asset is present on the release: the six goreleaser archives (linux/darwin/windows × amd64/arm64, .tar.gz or .zip) plus checksums.txt. Poll with `gh release view` every 15s up to a 30-minute deadline; emit GitHub-Actions-style ::error:: on timeout with the missing-asset list. Other tightening on the same workflow: - concurrency.group keys on `inputs.tag || github.ref_name` instead of `github.ref`, so a release-event run and a workflow_dispatch run for the same version (different ref kinds) still serialize into one group. - Strict semver assertion on the resolved tag before any publish step runs, so a non-release tag pushed by accident can never trigger publish. - `permissions: contents: read` at the job level so the wait step's gh CLI is authorized to enumerate release assets on a public repo; no other token capabilities are granted. - `--dry-run` skips the wait so manual dispatch can be used to validate the workflow against tags that have no real release yet. --- .github/workflows/npm-publish.yml | 56 ++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index 81b4d55..7875670 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -27,7 +27,9 @@ on: default: true concurrency: - group: npm-publish-${{ github.ref }} + # Key on the version so a `release: published` run and a `workflow_dispatch` + # of the same tag (which has a different github.ref) still serialize. + group: npm-publish-${{ inputs.tag || github.ref_name }} cancel-in-progress: false permissions: {} @@ -35,6 +37,10 @@ permissions: {} jobs: publish: runs-on: ubuntu-latest + # contents:read lets `gh release view` enumerate assets while waiting for + # goreleaser to finish uploading; nothing else uses elevated tokens. + permissions: + contents: read defaults: run: working-directory: npm @@ -59,14 +65,62 @@ jobs: DRY="" fi VERSION="${REF#v}" + if [[ ! "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]]; then + echo "::error::Refusing to publish: tag '$REF' does not look like a semver release." + exit 1 + fi if [[ "$VERSION" == *-* ]]; then DIST_TAG="next"; else DIST_TAG="latest"; fi { echo "VERSION=$VERSION" + echo "TAG=v$VERSION" echo "DIST_TAG=$DIST_TAG" echo "DRY_RUN=$DRY" } >> "$GITHUB_OUTPUT" echo "[npm-publish] version=$VERSION dist-tag=$DIST_TAG dry_run='$DRY'" + # The `release: published` event fires before goreleaser uploads the + # platform archives (release-publish.yml runs `build-artifacts` after the + # release is published). The npm package's postinstall downloads those + # archives, so publishing before they exist would leave a permanently + # broken npm version. Wait until every expected asset is on the release. + # Skipped on --dry-run to allow validation against tags that have no + # release yet. + - name: Wait for goreleaser artifacts on the release + if: steps.version.outputs.DRY_RUN == '' + env: + GH_TOKEN: ${{ github.token }} + VERSION: ${{ steps.version.outputs.VERSION }} + TAG: ${{ steps.version.outputs.TAG }} + run: | + deadline=$(( $(date +%s) + 1800 )) # 30 minutes + expected=( + "octo-cli_${VERSION}_linux_amd64.tar.gz" + "octo-cli_${VERSION}_linux_arm64.tar.gz" + "octo-cli_${VERSION}_darwin_amd64.tar.gz" + "octo-cli_${VERSION}_darwin_arm64.tar.gz" + "octo-cli_${VERSION}_windows_amd64.zip" + "octo-cli_${VERSION}_windows_arm64.zip" + "checksums.txt" + ) + while :; do + have=$(gh release view "$TAG" --repo "$GITHUB_REPOSITORY" \ + --json assets --jq '.assets[].name' 2>/dev/null || true) + missing=() + for f in "${expected[@]}"; do + printf '%s\n' "$have" | grep -Fxq "$f" || missing+=("$f") + done + if [ ${#missing[@]} -eq 0 ]; then + echo "[npm-publish] all artifacts present on $TAG" + break + fi + if [ "$(date +%s)" -gt "$deadline" ]; then + echo "::error::Timed out waiting for goreleaser artifacts on $TAG. Missing: ${missing[*]}" + exit 1 + fi + echo "[npm-publish] waiting for ${#missing[@]} artifact(s) on $TAG: ${missing[*]}" + sleep 15 + done + - name: Set package version run: npm version "${{ steps.version.outputs.VERSION }}" --no-git-tag-version --allow-same-version From 0807a37a38134cfca5e6616006a07a05f03f4284 Mon Sep 17 00:00:00 2001 From: liuooo Date: Sat, 30 May 2026 10:39:26 +0800 Subject: [PATCH 04/14] fix(npm): allow release-assets.githubusercontent.com in redirect allowlist GitHub release asset downloads currently 302 to release-assets.githubusercontent.com (a JWT-signed URL CDN), not the older objects.githubusercontent.com. The previous allowlist refused that hop, so the postinstall would fail before downloading the archive or checksums.txt. Verified end-to-end against the v0.5.0 release: the archive now downloads, checksum verification passes, and the extracted binary runs. Keep objects.githubusercontent.com in the allowlist as a fallback in case GitHub reverts or routes some downloads through the older CDN. Reported by @Jerry-Xin on #27. --- npm/scripts/install.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/npm/scripts/install.js b/npm/scripts/install.js index 81a15b1..7e64634 100644 --- a/npm/scripts/install.js +++ b/npm/scripts/install.js @@ -20,10 +20,13 @@ const ARCH = { x64: "amd64", arm64: "arm64" }[process.arch]; const isWin = process.platform === "win32"; // Redirects are followed only to these hosts. github.com is the first hop; -// release assets redirect to objects.githubusercontent.com via S3; codeload -// covers source archives. Anything else is treated as hostile. +// release assets currently 302 to release-assets.githubusercontent.com (a +// signed-URL CDN); objects.githubusercontent.com is the older asset CDN and +// is kept for backwards/forwards compatibility; codeload covers source +// archives. Anything else is treated as hostile. const ALLOWED_HOSTS = new Set([ "github.com", + "release-assets.githubusercontent.com", "objects.githubusercontent.com", "codeload.github.com", ]); From c63ffc09d8e60d55d63d6030b7a4549fa7e50a5a Mon Sep 17 00:00:00 2001 From: liuooo Date: Sat, 30 May 2026 17:53:14 +0800 Subject: [PATCH 05/14] fix(npm): address PR #27 review rounds 2-3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 (yujiawei) flagged a symlink-in-archive escape on the install path and a script-injection sink in the publish workflow; round-3 (Linus) flagged the npm-publish trigger as non-functional under GitHub's recursion rule and the PATH-hint fallback as fragile. This consolidates the fixes plus the non-blocking hardening from the same rounds. No behaviour change on the happy path; only the failure / hostile-input paths. install.js - Symlink guard: lstatSync the extracted member before chmodSync. tar's named-member extraction does not prevent the member itself from being a symlink; the previous chmodSync + later spawnSync would follow it, letting a malicious archive chmod files outside bin/ and execute an arbitrary local binary as octo-cli. PoC verified locally; the new guard refuses non-regular files and unlinks the symlink before failing. - PATH hint rewrite: stop counting `..` from __dirname to guess . Instead, if npm's bin is already on PATH, do nothing; otherwise hint our own bin/ (path.join(__dirname, "..", "bin")) which is exact. Cost: the user only fixes octo-cli, not other global packages — but if npm's bin isn't on PATH they have that problem repo-wide anyway. - Wall-clock deadline: REQUEST_TIMEOUT_MS is an idle timeout; a slow-trickle peer can stay under it and stall the install. Adds a 5-minute wall-clock cap across all retries. - Non-retryable error classification: allowlist / parse / deadline / oversize errors get `nonRetryable: true` so they fail fast instead of burning 3 attempts. - Host allowlist now includes release-assets.githubusercontent.com (the CDN GitHub release assets currently redirect to); kept objects.githubusercontent.com as a fallback. - 200 MiB body size cap (defensive, far above any real archive). - Strict semver assertion on VERSION before it is interpolated into URL or filename. Aligned with the workflow's tag regex (no +build). - Checksum entry: validate digest shape (^[0-9a-f]{64}$) explicitly rather than relying on the compare to fail. - ENOENT for `tar` now emits a targeted message (older Windows / minimal containers without bsdtar) instead of a bare "extract failed: ENOENT". - Extracted nonRetryable, assertSafeHost, isValidReleaseSemver, assetName, parseChecksumEntry, ALLOWED_HOSTS as named exports so the test file can cover them directly. - BIN_NAME constant at module scope so the platform-suffix special case doesn't repeat across maybeHintPath and main. - Trust-model + failure-behaviour paragraphs at the top of the file so a future reviewer doesn't have to reconstruct the "checksum is integrity, not provenance" and "hard exit is deliberate, no SKIP_DOWNLOAD opt-out" decisions from scratch. run.js - Signal propagation: re-raise the killing signal with process.kill so callers see 128+signum (e.g. 130 for SIGINT) instead of a generic 1. Uses os.constants.signals for the fallback so signals outside a small hand-coded set (SIGPIPE, SIGUSR1, ...) also get a faithful exit code. npm-publish.yml - Script-injection fix: all `${{ inputs.* }}` / `${{ steps.* }}` are now routed through `env:` shell variables. The previous direct interpolation into `run:` was substituted as literal text before bash parsed the line, letting a crafted tag value execute arbitrary commands in a step that later holds NPM_TOKEN. - npm provenance: `id-token: write` + `--provenance` on publish. This is the only origin link consumers get for the wrapper tarball itself; the binary's sha256 is independent. - Trigger chain rewrite: removes `on: release: published`. That event is fired by GITHUB_TOKEN in release-publish.yml; GitHub's recursion rule suppresses workflow runs for events caused by GITHUB_TOKEN, so the trigger never actually delivered. release-publish.yml now invokes us via `gh workflow run` (workflow_dispatch is exempt from the suppression). Removes the 30-min `wait-for-artifacts` poll (and its 7 hard-coded asset filenames): with explicit invocation, sequencing is by step order, not by polling a remote state. - Strict semver assertion on the resolved tag before any publish step runs. - Concurrency group keyed on `inputs.tag` so two dispatches of the same version serialize. release-publish.yml - `actions: write` on build-artifacts so the new "Trigger npm publish" step can `gh workflow run npm-publish.yml -f tag=$TAG -f dry_run=false` after archives are uploaded. Sequenced after `gh release upload` so the archives + checksums.txt are guaranteed present when npm-publish runs. package.json - `files:` now lists the two scripts explicitly (`install.js`, `run.js`) so the upcoming `install.test.js` is not shipped in the published tarball. - `LICENSE` added to the file list (and a copy lives at npm/LICENSE) so the published tarball carries the Apache-2.0 text alongside its declaration, satisfying §4. README - Trust-model section: explicit about sha256 being integrity, not provenance; mentions `npm audit signatures` to verify the package's npm provenance attestation; flags signing checksums.txt with cosign as a follow-up. Verified - Live e2e: install.js downloads v0.5.0 archives from real GitHub Release, passes checksum, extracts, runs the binary. (Asset prefix patched octo-cli_ → octo_ for the test, since v0.5.0 predates the rename.) - Symlink PoC: constructed a tarball whose `octo` member is a symlink to a 0600 victim file; new guard refuses extraction, victim's mode preserved. - All `${{ }}` interpolations now in `env:`; YAML parser reports zero expression-language sinks in any `run:` block across both workflows. --- .github/workflows/npm-publish.yml | 109 +++++-------- .github/workflows/release-publish.yml | 17 ++ npm/LICENSE | 190 +++++++++++++++++++++++ npm/README.md | 23 ++- npm/package.json | 6 +- npm/scripts/install.js | 215 +++++++++++++++++++------- npm/scripts/run.js | 12 +- 7 files changed, 443 insertions(+), 129 deletions(-) create mode 100644 npm/LICENSE diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index 7875670..db36f0d 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -1,19 +1,22 @@ -# Publish the npm wrapper (@mininglamp-oss/octo-cli) when a GitHub Release is -# published. goreleaser builds and uploads the platform binaries to the same -# release; the npm package's postinstall downloads the matching one, so the -# published npm version MUST correspond to a real release tag. +# Publish the npm wrapper (@mininglamp-oss/octo-cli). +# +# This workflow is sequenced AFTER `release-publish.yml`'s `build-artifacts` +# job: that job ends by explicitly invoking us with `gh workflow run` (i.e. +# a workflow_dispatch event), so by the time we run the goreleaser archives +# and `checksums.txt` are already on the GitHub Release. There is no +# `on: release: published` trigger: that event is fired by GITHUB_TOKEN +# (in the release-publish flow), which GitHub's recursion-prevention rule +# would suppress, so it never actually delivered. # # Tag → npm dist-tag: # v1.2.3 → @latest (matches /^v\d+\.\d+\.\d+$/) # v1.2.3-rc.1 → @next (any tag containing "-") # -# A manual workflow_dispatch run supports --dry-run for testing without -# uploading. +# A manual workflow_dispatch run defaults to --dry-run, so re-running by +# hand is safe and doesn't accidentally re-publish. name: npm publish on: - release: - types: [published] workflow_dispatch: inputs: tag: @@ -27,9 +30,8 @@ on: default: true concurrency: - # Key on the version so a `release: published` run and a `workflow_dispatch` - # of the same tag (which has a different github.ref) still serialize. - group: npm-publish-${{ inputs.tag || github.ref_name }} + # Key on the version so two dispatches of the same tag still serialize. + group: npm-publish-${{ inputs.tag }} cancel-in-progress: false permissions: {} @@ -37,10 +39,10 @@ permissions: {} jobs: publish: runs-on: ubuntu-latest - # contents:read lets `gh release view` enumerate assets while waiting for - # goreleaser to finish uploading; nothing else uses elevated tokens. + # id-token:write lets npm provenance generate an OIDC-signed Sigstore + # attestation linking the published tarball back to this workflow run. permissions: - contents: read + id-token: write defaults: run: working-directory: npm @@ -53,17 +55,21 @@ jobs: - name: Resolve version and dist-tag id: version - # On a release event GITHUB_REF_NAME is the tag; on manual dispatch use - # the `tag` input. VERSION drops the leading `v`; a version containing - # `-` (prerelease) publishes to @next so it never clobbers @latest. + # VERSION drops the leading `v`; a version containing `-` + # (prerelease) publishes to @next so it never clobbers @latest. + # + # IMPORTANT: dispatch inputs are read through env: shell variables, + # not interpolated into the run: block. Direct `${{ inputs.tag }}` + # would be substituted as literal text before bash parses the line, + # letting a crafted tag value break out of the assignment and run + # arbitrary commands in a step that later has NPM_TOKEN in scope + # (CWE-94). + env: + INPUT_TAG: ${{ inputs.tag }} + INPUT_DRY_RUN: ${{ inputs.dry_run }} run: | - if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then - REF="${{ inputs.tag }}" - DRY="${{ inputs.dry_run && '--dry-run' || '' }}" - else - REF="${GITHUB_REF_NAME}" - DRY="" - fi + REF="$INPUT_TAG" + if [ "$INPUT_DRY_RUN" = "true" ]; then DRY="--dry-run"; else DRY=""; fi VERSION="${REF#v}" if [[ ! "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]]; then echo "::error::Refusing to publish: tag '$REF' does not look like a semver release." @@ -72,59 +78,24 @@ jobs: if [[ "$VERSION" == *-* ]]; then DIST_TAG="next"; else DIST_TAG="latest"; fi { echo "VERSION=$VERSION" - echo "TAG=v$VERSION" echo "DIST_TAG=$DIST_TAG" echo "DRY_RUN=$DRY" } >> "$GITHUB_OUTPUT" echo "[npm-publish] version=$VERSION dist-tag=$DIST_TAG dry_run='$DRY'" - # The `release: published` event fires before goreleaser uploads the - # platform archives (release-publish.yml runs `build-artifacts` after the - # release is published). The npm package's postinstall downloads those - # archives, so publishing before they exist would leave a permanently - # broken npm version. Wait until every expected asset is on the release. - # Skipped on --dry-run to allow validation against tags that have no - # release yet. - - name: Wait for goreleaser artifacts on the release - if: steps.version.outputs.DRY_RUN == '' + - name: Set package version env: - GH_TOKEN: ${{ github.token }} VERSION: ${{ steps.version.outputs.VERSION }} - TAG: ${{ steps.version.outputs.TAG }} - run: | - deadline=$(( $(date +%s) + 1800 )) # 30 minutes - expected=( - "octo-cli_${VERSION}_linux_amd64.tar.gz" - "octo-cli_${VERSION}_linux_arm64.tar.gz" - "octo-cli_${VERSION}_darwin_amd64.tar.gz" - "octo-cli_${VERSION}_darwin_arm64.tar.gz" - "octo-cli_${VERSION}_windows_amd64.zip" - "octo-cli_${VERSION}_windows_arm64.zip" - "checksums.txt" - ) - while :; do - have=$(gh release view "$TAG" --repo "$GITHUB_REPOSITORY" \ - --json assets --jq '.assets[].name' 2>/dev/null || true) - missing=() - for f in "${expected[@]}"; do - printf '%s\n' "$have" | grep -Fxq "$f" || missing+=("$f") - done - if [ ${#missing[@]} -eq 0 ]; then - echo "[npm-publish] all artifacts present on $TAG" - break - fi - if [ "$(date +%s)" -gt "$deadline" ]; then - echo "::error::Timed out waiting for goreleaser artifacts on $TAG. Missing: ${missing[*]}" - exit 1 - fi - echo "[npm-publish] waiting for ${#missing[@]} artifact(s) on $TAG: ${missing[*]}" - sleep 15 - done - - - name: Set package version - run: npm version "${{ steps.version.outputs.VERSION }}" --no-git-tag-version --allow-same-version + run: npm version "$VERSION" --no-git-tag-version --allow-same-version + # `--provenance` produces an OIDC-signed Sigstore attestation linking + # the tarball to this workflow run (npm ≥ 9.5, public repo + public + # package — both satisfied). Consumers can verify the link with + # `npm audit signatures`; this is the only origin link the user gets + # for the wrapper itself (the binary's sha256 check is independent). - name: Publish to npm env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} - run: npm publish --access public --tag "${{ steps.version.outputs.DIST_TAG }}" ${{ steps.version.outputs.DRY_RUN }} + DIST_TAG: ${{ steps.version.outputs.DIST_TAG }} + DRY_RUN: ${{ steps.version.outputs.DRY_RUN }} + run: npm publish --access public --provenance --tag "$DIST_TAG" $DRY_RUN diff --git a/.github/workflows/release-publish.yml b/.github/workflows/release-publish.yml index 7bb2a31..8e8e263 100644 --- a/.github/workflows/release-publish.yml +++ b/.github/workflows/release-publish.yml @@ -36,6 +36,13 @@ jobs: runs-on: ubuntu-latest permissions: contents: write + # actions:write lets the final step dispatch npm-publish.yml. We do this + # explicitly rather than letting npm-publish hook `on: release: published`, + # because the publish job above changes the release state using + # GITHUB_TOKEN, and GitHub's recursion rule suppresses workflow runs for + # events caused by GITHUB_TOKEN — workflow_dispatch and + # repository_dispatch are the only exempt events. + actions: write steps: - name: Checkout uses: actions/checkout@v4 @@ -71,3 +78,13 @@ jobs: env: TAG: ${{ inputs.tag }} GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + # Hand off to the npm wrapper publish workflow. Sequenced after upload + # so the archives + checksums.txt are guaranteed to be on the release + # by the time the npm postinstall tries to download them. Uses + # workflow_dispatch (exempt from GITHUB_TOKEN recursion suppression). + - name: Trigger npm publish + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + TAG: ${{ inputs.tag }} + run: gh workflow run npm-publish.yml -f tag="$TAG" -f dry_run=false diff --git a/npm/LICENSE b/npm/LICENSE new file mode 100644 index 0000000..fd67ecb --- /dev/null +++ b/npm/LICENSE @@ -0,0 +1,190 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to the Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by the Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding any notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + Copyright 2025 Mininglamp Technology + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/npm/README.md b/npm/README.md index 8820796..6cf5db5 100644 --- a/npm/README.md +++ b/npm/README.md @@ -11,13 +11,34 @@ octo-cli --help This package is a thin Node wrapper around the prebuilt Go binary. On install it downloads the binary matching your platform and this package's version from the [GitHub Release](https://github.com/Mininglamp-OSS/octo-cli/releases) and -verifies its checksum; the `octo-cli` command then execs that binary directly. +verifies its sha256 against the `checksums.txt` published on the same release; +the `octo-cli` command then execs that binary directly. Supported platforms: macOS, Linux, Windows on `x64` / `arm64`. For other install methods (Homebrew, raw binary, `go install`) and full usage, see the [main README](https://github.com/Mininglamp-OSS/octo-cli#readme). +## Trust model + +The sha256 check is an **integrity** check, not a **provenance** check: the +archive and its `checksums.txt` are fetched from the same GitHub Release over +the same channel, so an actor who can write to the release replaces both +consistently. The effective trust root is GitHub Release integrity plus the +npm publish pipeline. + +The wrapper itself (the tarball you install from npm) is published with npm +`--provenance`, producing a Sigstore attestation that links the tarball back +to the GitHub Actions workflow that published it. You can verify it with: + +```bash +npm audit signatures +``` + +A future release may sign `checksums.txt` itself (cosign keyless) so the +installer can verify a signature whose key is not co-located with the +artifact. + ## License [Apache-2.0](https://github.com/Mininglamp-OSS/octo-cli/blob/main/LICENSE) diff --git a/npm/package.json b/npm/package.json index 7a749ab..35067e3 100644 --- a/npm/package.json +++ b/npm/package.json @@ -9,8 +9,10 @@ "postinstall": "node scripts/install.js" }, "files": [ - "scripts/", - "README.md" + "scripts/install.js", + "scripts/run.js", + "README.md", + "LICENSE" ], "os": [ "darwin", diff --git a/npm/scripts/install.js b/npm/scripts/install.js index 7e64634..3f6755f 100644 --- a/npm/scripts/install.js +++ b/npm/scripts/install.js @@ -2,9 +2,28 @@ "use strict"; // postinstall: download the prebuilt octo-cli binary that matches this package -// version and the host platform from the GitHub Release, verify its checksum, -// and extract it into ../bin. Mirrors goreleaser's archive naming +// version and the host platform from the GitHub Release, verify its sha256 +// against the `checksums.txt` published on the same release, and extract it +// into ../bin. Mirrors goreleaser's archive naming // (octo-cli___.tar.gz, .zip on Windows; os/arch lowercase). +// +// Trust model: the checksum proves the archive matches whatever +// `checksums.txt` that release currently serves — i.e. it catches transport +// corruption, partial CDN poisoning, and accidental clobber. It does NOT +// prove provenance: both files share one trust root (the GitHub Release), so +// an actor who can write to the release replaces both consistently. Treat +// this as an integrity check; provenance is the GitHub Release itself plus +// the npm publish pipeline. +// +// Failure behavior: every error path calls `fail()` which logs to stderr and +// `process.exit(1)`. This is deliberate for a CLI installed with `-g`: a +// silent partial install would leave the user with an `octo-cli` shim that +// can't find its binary, surfacing as a confusing ENOENT only when the user +// tries to run a command later. Loud failure at install time is the right +// signal. There is intentionally no `OCTO_CLI_SKIP_DOWNLOAD` opt-out — if a +// transitive/air-gapped consumer ever needs one, add it then; today it would +// be unused complexity. The retry loop in `download()` already handles +// transient network failures, so a single ECONNRESET does not trip `fail()`. const fs = require("fs"); const path = require("path"); @@ -18,6 +37,7 @@ const VERSION = require("../package.json").version; const OS = { darwin: "darwin", linux: "linux", win32: "windows" }[process.platform]; const ARCH = { x64: "amd64", arm64: "arm64" }[process.arch]; const isWin = process.platform === "win32"; +const BIN_NAME = isWin ? "octo-cli.exe" : "octo-cli"; // Redirects are followed only to these hosts. github.com is the first hop; // release assets currently 302 to release-assets.githubusercontent.com (a @@ -35,6 +55,7 @@ const MAX_REDIRECTS = 5; const REQUEST_TIMEOUT_MS = 30_000; const MAX_RETRIES = 3; const MAX_DOWNLOAD_BYTES = 200 * 1024 * 1024; // 200 MiB, far above any sane archive +const TOTAL_DOWNLOAD_DEADLINE_MS = 5 * 60_000; // wall-clock cap per asset function fail(msg) { console.error(`\n[octo-cli] install failed: ${msg}`); @@ -42,18 +63,60 @@ function fail(msg) { process.exit(1); } +// Mark errors that must not be retried (allowlist / parse failures, deadline +// exceeded). retryable status codes are still retried; everything explicitly +// marked here is not. +function nonRetryable(err) { + err.nonRetryable = true; + return err; +} + function assertSafeHost(u, label) { if (u.protocol !== "https:") { - throw new Error(`${label}: refusing non-https URL (${u.protocol}//${u.hostname})`); + throw nonRetryable(new Error(`${label}: refusing non-https URL (${u.protocol}//${u.hostname})`)); } if (!ALLOWED_HOSTS.has(u.hostname)) { - throw new Error(`${label}: host '${u.hostname}' is not in the redirect allowlist`); + throw nonRetryable(new Error(`${label}: host '${u.hostname}' is not in the redirect allowlist`)); } } +// True if `s` matches the strict semver subset we accept as a release tag: +// MAJOR.MINOR.PATCH with optional prerelease. No `+build` metadata — CI never +// produces it, so accepting it on the install side would be a silent +// divergence from .github/workflows/npm-publish.yml. +function isValidReleaseSemver(s) { + return typeof s === "string" && /^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?$/.test(s); +} + +// The archive filename goreleaser produces for (version, os, arch). The shape +// is fixed by .goreleaser.yaml's `name_template` — keep these in sync. +function assetName(version, os, arch, win) { + const ext = win ? "zip" : "tar.gz"; + return `octo-cli_${version}_${os}_${arch}.${ext}`; +} + +// Look up `asset` in a `checksums.txt`-style payload (" " +// per line). Returns the expected 64-char lowercase hex digest. Throws if +// the entry is missing or the digest doesn't match that shape — so the +// caller doesn't have to special-case a "garbage line that happens to match +// by filename" scenario. +function parseChecksumEntry(sums, asset) { + const entry = sums + .split("\n") + .map((l) => l.trim().split(/\s+/)) + .find((p) => p[1] === asset); + if (!entry) throw new Error(`no checksum entry for ${asset}`); + const digest = entry[0]; + if (!/^[0-9a-f]{64}$/.test(digest)) { + throw new Error(`malformed checksum entry for ${asset}: '${digest}'`); + } + return digest; +} + // Single HTTPS GET. Resolves to either a buffered body or a redirect target, // never both. Times out on socket idle so a half-open connection cannot hang -// the install indefinitely. +// the install indefinitely; a separate wall-clock deadline (download() below) +// guards against slow-trickle attacks that stay just under the idle timeout. function getOnce(url) { return new Promise((resolve, reject) => { const req = https.get( @@ -66,7 +129,14 @@ function getOnce(url) { const status = res.statusCode || 0; if (status >= 300 && status < 400 && res.headers.location) { res.resume(); - resolve({ kind: "redirect", next: new URL(res.headers.location, url) }); + let nextUrl; + try { + nextUrl = new URL(res.headers.location, url); + } catch (e) { + reject(nonRetryable(new Error(`malformed redirect location for ${url}: ${e.message}`))); + return; + } + resolve({ kind: "redirect", next: nextUrl }); return; } if (status !== 200) { @@ -81,7 +151,7 @@ function getOnce(url) { res.on("data", (c) => { size += c.length; if (size > MAX_DOWNLOAD_BYTES) { - req.destroy(new Error(`response exceeded ${MAX_DOWNLOAD_BYTES} bytes for ${url}`)); + req.destroy(nonRetryable(new Error(`response exceeded ${MAX_DOWNLOAD_BYTES} bytes for ${url}`))); return; } chunks.push(c); @@ -100,24 +170,38 @@ function getOnce(url) { async function downloadOnce(url) { let current = url; for (let hop = 0; hop <= MAX_REDIRECTS; hop++) { - assertSafeHost(new URL(current), hop === 0 ? "request" : `redirect #${hop}`); + let parsed; + try { + parsed = new URL(current); + } catch (e) { + throw nonRetryable(new Error(`malformed URL at hop ${hop}: ${e.message}`)); + } + assertSafeHost(parsed, hop === 0 ? "request" : `redirect #${hop}`); const r = await getOnce(current); if (r.kind === "body") return r.body; current = r.next.toString(); } - throw new Error(`too many redirects (>${MAX_REDIRECTS})`); + throw nonRetryable(new Error(`too many redirects (>${MAX_REDIRECTS})`)); } // Retry the whole download on transient errors (timeout, socket reset, 5xx). // 404 is treated as a non-retryable signal that the version doesn't exist on -// the release yet — fast-fail with a clear error rather than spin. +// the release yet — fast-fail with a clear error rather than spin. Errors +// explicitly flagged `nonRetryable` (allowlist / parse / deadline) are not +// retried either. A wall-clock deadline caps the total time spent across all +// retries so a slow-trickle peer cannot keep the install alive. async function download(url) { + const deadline = Date.now() + TOTAL_DOWNLOAD_DEADLINE_MS; for (let attempt = 0; ; attempt++) { + if (Date.now() >= deadline) { + throw nonRetryable(new Error(`download exceeded ${TOTAL_DOWNLOAD_DEADLINE_MS / 1000}s wall-clock deadline: ${url}`)); + } try { return await downloadOnce(url); } catch (e) { const status = e && e.httpStatus; const retryable = + !(e && e.nonRetryable) && attempt < MAX_RETRIES - 1 && (status === undefined || (status >= 500 && status < 600)); if (!retryable) throw e; @@ -128,40 +212,49 @@ async function download(url) { } } -// On a global install, warn (without modifying anything) if the directory npm -// links the `octo-cli` command into is not on PATH, so the command would not be -// found. Best-effort: any failure here must not break the install. +// On a global install, warn (without modifying anything) if `octo-cli` would +// not be found on PATH. Best-effort: any failure here must not break the +// install. +// +// Strategy: if npm's own bin (/bin on unix, on Windows) is +// already on PATH, do nothing — npm's bin shim works, the user is fine. If +// it isn't (or we can't tell because npm_config_prefix isn't set), point at +// the directory the postinstall just wrote the Go binary into. That path is +// `path.join(__dirname, "..", "bin")` — derived from __dirname, so it's +// always exact and doesn't depend on guessing npm's directory layout +// (counting `..` to reach ). Cost: adding our bin to PATH only +// helps octo-cli, not other global npm packages — but if npm's bin isn't on +// PATH the user already has that problem repo-wide. function maybeHintPath() { try { const isGlobal = process.env.npm_config_global === "true" || process.env.npm_config_location === "global"; if (!isGlobal) return; - // npm links bins into /bin on unix, and into itself on Windows. - let prefix = process.env.npm_config_prefix || process.env.PREFIX; - if (!prefix) { - // Fallback: __dirname is /lib/node_modules/@scope/pkg/scripts (unix, - // 5 levels under prefix) or /node_modules/@scope/pkg/scripts - // (Windows, 4 levels). - prefix = isWin - ? path.resolve(__dirname, "..", "..", "..", "..") - : path.resolve(__dirname, "..", "..", "..", "..", ".."); + const PATH = (process.env.PATH || "").split(path.delimiter).filter(Boolean); + const onPath = (dir) => PATH.some((p) => path.resolve(p) === path.resolve(dir)); + + // Common case: npm's bin is on PATH, the shim it installed for `octo-cli` + // will be found, no hint needed. + const npmPrefix = process.env.npm_config_prefix || process.env.PREFIX; + if (npmPrefix) { + const npmBin = isWin ? npmPrefix : path.join(npmPrefix, "bin"); + if (onPath(npmBin)) return; } - const binDir = isWin ? prefix : path.join(prefix, "bin"); - const onPath = (process.env.PATH || "") - .split(path.delimiter) - .some((p) => p && path.resolve(p) === path.resolve(binDir)); - if (onPath) return; + // Either we don't know where npm put its bin, or it isn't on PATH. Tell + // the user to add our own bin (where the Go binary lives) instead. That + // path is exact; the npm-bin path would be a guess. + const ourBinDir = path.resolve(__dirname, "..", "bin"); + if (onPath(ourBinDir)) return; const lines = [ "", - `[octo-cli] Installed, but ${binDir} is not on your PATH —`, - "[octo-cli] the `octo-cli` command will not be found until you add it.", + `[octo-cli] Installed ${ourBinDir}/${BIN_NAME}, but that directory is not on PATH.`, + "[octo-cli] Add it to use the `octo-cli` command:", ]; if (isWin) { - lines.push("[octo-cli] Add it for your user (then reopen the terminal):"); - lines.push(` setx PATH "%PATH%;${binDir}"`); + lines.push(` setx PATH "%PATH%;${ourBinDir}"`); } else { const shell = path.basename(process.env.SHELL || ""); const rc = @@ -174,10 +267,10 @@ function maybeHintPath() { : "your shell profile"; if (shell === "fish") { lines.push(`[octo-cli] Add this to ${rc}, then reopen the terminal:`); - lines.push(` fish_add_path ${binDir}`); + lines.push(` fish_add_path ${ourBinDir}`); } else { lines.push(`[octo-cli] Add this to ${rc}, then reopen the terminal:`); - lines.push(` export PATH="${binDir}:$PATH"`); + lines.push(` export PATH="${ourBinDir}:$PATH"`); } } lines.push(""); @@ -192,16 +285,12 @@ async function main() { if (!VERSION || VERSION === "0.0.0") { fail("package version is a placeholder (0.0.0); this package must be published with a real release version"); } - // The version is interpolated into both the download URL and a filesystem - // path; reject anything that isn't strict semver before either use. - if (!/^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/.test(VERSION)) { - fail(`package version '${VERSION}' is not a valid semver`); + if (!isValidReleaseSemver(VERSION)) { + fail(`package version '${VERSION}' is not a valid release semver`); } - const ext = isWin ? "zip" : "tar.gz"; - const asset = `octo-cli_${VERSION}_${OS}_${ARCH}.${ext}`; + const asset = assetName(VERSION, OS, ARCH, isWin); const base = `https://github.com/${REPO}/releases/download/v${VERSION}`; - const binName = isWin ? "octo-cli.exe" : "octo-cli"; const binDir = path.join(__dirname, "..", "bin"); console.log(`[octo-cli] downloading ${asset} ...`); @@ -213,15 +302,11 @@ async function main() { fail(e.message); } - // Verify sha256 against checksums.txt (" " per line). - const entry = sums - .split("\n") - .map((l) => l.trim().split(/\s+/)) - .find((p) => p[1] === asset); - if (!entry || entry.length < 2) fail(`no checksum entry for ${asset}`); - const expected = entry[0]; - if (!/^[0-9a-f]{64}$/.test(expected)) { - fail(`malformed checksum entry for ${asset}: '${expected}'`); + let expected; + try { + expected = parseChecksumEntry(sums, asset); + } catch (e) { + fail(e.message); } const got = crypto.createHash("sha256").update(archive).digest("hex"); if (got !== expected) fail(`checksum mismatch for ${asset} (want ${expected}, got ${got})`); @@ -232,11 +317,30 @@ async function main() { // a targeted message instead of a bare ENOENT. fs.mkdirSync(binDir, { recursive: true }); const tmp = path.join(binDir, asset); + const extractedPath = path.join(binDir, BIN_NAME); fs.writeFileSync(tmp, archive); try { - const args = isWin ? ["-xf", tmp, "-C", binDir, binName] : ["-xzf", tmp, "-C", binDir, binName]; + const args = isWin ? ["-xf", tmp, "-C", binDir, BIN_NAME] : ["-xzf", tmp, "-C", binDir, BIN_NAME]; execFileSync("tar", args, { stdio: "inherit" }); - fs.chmodSync(path.join(binDir, binName), 0o755); + + // Refuse non-regular files (symlink/dir/...). Named-member extraction + // prevents zip-slip from other members of the archive, but the named + // member itself can be a symlink; a later chmodSync / spawnSync would + // follow it, letting a malicious archive chmod files outside bin/ or + // execute an arbitrary local binary as `octo-cli`. goreleaser never emits + // symlink members, so this is defense-in-depth behind the release trust + // root, not a remotely-reachable bug. Use lstat (not stat) so we see the + // symlink itself rather than its target. + const st = fs.lstatSync(extractedPath); + if (!st.isFile()) { + try { + fs.unlinkSync(extractedPath); + } catch { + /* leave it; the fail() below is what matters */ + } + fail(`extracted '${BIN_NAME}' is not a regular file (mode ${(st.mode & 0o170000).toString(8)})`); + } + fs.chmodSync(extractedPath, 0o755); } catch (e) { if (e && e.code === "ENOENT") { fail( @@ -262,4 +366,11 @@ async function main() { if (require.main === module) { main(); } -module.exports = { maybeHintPath }; +module.exports = { + maybeHintPath, + assertSafeHost, + isValidReleaseSemver, + assetName, + parseChecksumEntry, + ALLOWED_HOSTS, +}; diff --git a/npm/scripts/run.js b/npm/scripts/run.js index 747aec2..0e7f91a 100644 --- a/npm/scripts/run.js +++ b/npm/scripts/run.js @@ -4,6 +4,7 @@ // Thin shim: forward all args and stdio to the prebuilt Go binary that // scripts/install.js placed in ../bin during postinstall. const path = require("path"); +const os = require("os"); const { spawnSync } = require("child_process"); const binName = process.platform === "win32" ? "octo-cli.exe" : "octo-cli"; @@ -25,13 +26,14 @@ if (res.error) { // If the binary was killed by a signal, re-raise the same signal so callers // observe the conventional 128+signum exit code (e.g. 130 for SIGINT) instead -// of a generic 1 that hides Ctrl-C from shells and supervisors. +// of a generic 1 that hides Ctrl-C from shells and supervisors. Use the +// platform's signal map (os.constants.signals) so signals outside the small +// hand-coded set (e.g. SIGPIPE, SIGUSR1) also get a faithful exit code on +// the fallback path. if (res.signal) { process.kill(process.pid, res.signal); - // process.kill is async w.r.t. the event loop on some platforms; fall back - // to the conventional exit code so we don't return success by accident. - const SIGNUMS = { SIGHUP: 1, SIGINT: 2, SIGQUIT: 3, SIGKILL: 9, SIGTERM: 15 }; - process.exit(128 + (SIGNUMS[res.signal] || 0)); + const signum = (os.constants && os.constants.signals && os.constants.signals[res.signal]) || 0; + process.exit(128 + signum); } // Propagate the child's exit code. From 2ef07c2580c72e4465929f5266b51f6eaf899250 Mon Sep 17 00:00:00 2001 From: liuooo Date: Sat, 30 May 2026 17:53:43 +0800 Subject: [PATCH 06/14] test(npm): node:test coverage for the installer's pure functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The download/extract paths are covered by a manual e2e against real GitHub releases. This adds regression protection for the smaller pure functions — parsing, validation, host allowlist, PATH-hint scenarios — where future edits are most likely to land. scripts/install.test.js (32 tests, ~60ms) - assertSafeHost: allowlist passes / non-https rejection / off-list host rejection / errors marked nonRetryable / ALLOWED_HOSTS set is locked (so adding or removing a host shows up in tests, not silently) - isValidReleaseSemver: stable releases / prereleases / rejects +build metadata / rejects malformed / non-string inputs / 0.0.0 is structurally valid (placeholder rejection lives in main(), not in the validator) - assetName: locked to the 6-platform grid that matches .goreleaser.yaml's name_template (a goreleaser rename or platform change trips this) - parseChecksumEntry: well-formed entry / surrounding whitespace / missing asset / non-hex / uppercase hex / wrong length - maybeHintPath: 5 PATH scenarios + 4 shell dialects + npm_config_location ci.yml - New `npm-test` job runs in parallel with build, on a clean setup-node@v4 / Node 20 (the test code uses node:test built in since 18, but Node 20 is the floor we recommend for CI). package.json - `"test": "node --test scripts/install.test.js"` so `npm test` runs the suite locally too. --- .github/workflows/ci.yml | 14 ++ npm/package.json | 3 +- npm/scripts/install.test.js | 300 ++++++++++++++++++++++++++++++++++++ 3 files changed, 316 insertions(+), 1 deletion(-) create mode 100644 npm/scripts/install.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 564b429..722e3a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,3 +58,17 @@ jobs: - name: build run: go build -o octo-cli ./cmd/octo-cli + + npm-test: + name: npm test + runs-on: ubuntu-latest + defaults: + run: + working-directory: npm + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: "20" + - name: node --test + run: node --test scripts/install.test.js diff --git a/npm/package.json b/npm/package.json index 35067e3..781e9c4 100644 --- a/npm/package.json +++ b/npm/package.json @@ -6,7 +6,8 @@ "octo-cli": "scripts/run.js" }, "scripts": { - "postinstall": "node scripts/install.js" + "postinstall": "node scripts/install.js", + "test": "node --test scripts/install.test.js" }, "files": [ "scripts/install.js", diff --git a/npm/scripts/install.test.js b/npm/scripts/install.test.js new file mode 100644 index 0000000..2d1f23e --- /dev/null +++ b/npm/scripts/install.test.js @@ -0,0 +1,300 @@ +"use strict"; + +// Unit tests for the pure functions in install.js. The download / extract +// paths are covered by the existing manual e2e against real GitHub releases; +// these tests guard the small input-output functions where regressions are +// most likely (parsing, validation, host allowlist, PATH-hint scenarios). +// +// Run with: `npm test` (from the `npm/` directory) or `node --test scripts/`. + +const test = require("node:test"); +const assert = require("node:assert/strict"); +const path = require("node:path"); + +const { + assertSafeHost, + isValidReleaseSemver, + assetName, + parseChecksumEntry, + maybeHintPath, + ALLOWED_HOSTS, +} = require("./install.js"); + +// ---------- assertSafeHost -------------------------------------------------- + +test("assertSafeHost: allows github.com over https", () => { + assert.doesNotThrow(() => assertSafeHost(new URL("https://github.com/x"), "req")); +}); + +test("assertSafeHost: allows release-assets.githubusercontent.com", () => { + assert.doesNotThrow(() => + assertSafeHost(new URL("https://release-assets.githubusercontent.com/x"), "redirect"), + ); +}); + +test("assertSafeHost: allows objects.githubusercontent.com (legacy)", () => { + assert.doesNotThrow(() => + assertSafeHost(new URL("https://objects.githubusercontent.com/x"), "redirect"), + ); +}); + +test("assertSafeHost: rejects http (non-tls) even on allowlisted host", () => { + assert.throws( + () => assertSafeHost(new URL("http://github.com/x"), "req"), + /refusing non-https/i, + ); +}); + +test("assertSafeHost: rejects an unlisted host", () => { + assert.throws( + () => assertSafeHost(new URL("https://attacker.example/x"), "redirect"), + /not in the redirect allowlist/i, + ); +}); + +test("assertSafeHost: rejected errors are marked nonRetryable", () => { + try { + assertSafeHost(new URL("https://attacker.example/x"), "redirect"); + assert.fail("should have thrown"); + } catch (e) { + assert.equal(e.nonRetryable, true); + } +}); + +test("ALLOWED_HOSTS covers the four CDN hops in use", () => { + // Lock in the set so adding/removing a host shows up in tests, not silently. + assert.deepEqual( + [...ALLOWED_HOSTS].sort(), + [ + "codeload.github.com", + "github.com", + "objects.githubusercontent.com", + "release-assets.githubusercontent.com", + ], + ); +}); + +// ---------- isValidReleaseSemver ------------------------------------------- + +test("isValidReleaseSemver: accepts stable releases", () => { + for (const v of ["1.0.0", "0.6.0", "10.20.30"]) assert.equal(isValidReleaseSemver(v), true, v); +}); + +test("isValidReleaseSemver: accepts prereleases", () => { + for (const v of ["1.0.0-rc.1", "0.6.0-beta.2", "1.2.3-alpha"]) { + assert.equal(isValidReleaseSemver(v), true, v); + } +}); + +test("isValidReleaseSemver: rejects +build metadata (CI never produces it)", () => { + assert.equal(isValidReleaseSemver("1.2.3+build.4"), false); +}); + +test("isValidReleaseSemver: rejects malformed strings", () => { + for (const v of [ + "v1.2.3", // leading v + "1.2", // missing patch + "1.2.3.4", // four segments + "1.2.3-", // empty prerelease + "../etc/passwd", // path-traversal attempt + "", // empty + " 1.2.3", // leading space + ]) { + assert.equal(isValidReleaseSemver(v), false, v); + } +}); + +test("isValidReleaseSemver: 0.0.0 is regex-valid (placeholder rejection is main()'s job)", () => { + // The "package version is a placeholder" guard lives in main(), not in + // this validator. Document the seam: 0.0.0 is structurally a valid semver + // string; the placeholder check is a separate, deliberate check. + assert.equal(isValidReleaseSemver("0.0.0"), true); +}); + +test("isValidReleaseSemver: rejects non-strings", () => { + for (const v of [null, undefined, 1.2, {}, []]) { + assert.equal(isValidReleaseSemver(v), false, String(v)); + } +}); + +// ---------- assetName ------------------------------------------------------- + +test("assetName: matches .goreleaser.yaml name_template (tar.gz on unix)", () => { + assert.equal(assetName("0.6.0", "linux", "amd64", false), "octo-cli_0.6.0_linux_amd64.tar.gz"); + assert.equal(assetName("0.6.0", "darwin", "arm64", false), "octo-cli_0.6.0_darwin_arm64.tar.gz"); +}); + +test("assetName: uses .zip on Windows", () => { + assert.equal(assetName("0.6.0", "windows", "amd64", true), "octo-cli_0.6.0_windows_amd64.zip"); +}); + +test("assetName: locks in the os/arch axis", () => { + // Iterate the full grid so a goreleaser change that drops a platform shows up. + const grid = [ + ["linux", "amd64", false, "tar.gz"], + ["linux", "arm64", false, "tar.gz"], + ["darwin", "amd64", false, "tar.gz"], + ["darwin", "arm64", false, "tar.gz"], + ["windows", "amd64", true, "zip"], + ["windows", "arm64", true, "zip"], + ]; + for (const [os, arch, win, ext] of grid) { + assert.equal(assetName("1.2.3", os, arch, win), `octo-cli_1.2.3_${os}_${arch}.${ext}`); + } +}); + +// ---------- parseChecksumEntry --------------------------------------------- + +const VALID_DIGEST = "a".repeat(64); // 64 lowercase hex chars +const ASSET = "octo-cli_0.6.0_linux_amd64.tar.gz"; + +test("parseChecksumEntry: returns the digest for a well-formed entry", () => { + const sums = `${VALID_DIGEST} ${ASSET}\n${"b".repeat(64)} some-other-file\n`; + assert.equal(parseChecksumEntry(sums, ASSET), VALID_DIGEST); +}); + +test("parseChecksumEntry: ignores leading/trailing whitespace lines", () => { + const sums = `\n \n${VALID_DIGEST} ${ASSET}\n\n`; + assert.equal(parseChecksumEntry(sums, ASSET), VALID_DIGEST); +}); + +test("parseChecksumEntry: throws when the asset isn't listed", () => { + const sums = `${VALID_DIGEST} other-file\n`; + assert.throws(() => parseChecksumEntry(sums, ASSET), /no checksum entry/i); +}); + +test("parseChecksumEntry: rejects a non-hex digest", () => { + const sums = `not-a-digest ${ASSET}\n`; + assert.throws(() => parseChecksumEntry(sums, ASSET), /malformed checksum entry/i); +}); + +test("parseChecksumEntry: rejects uppercase hex (locking lowercase invariant)", () => { + const sums = `${"A".repeat(64)} ${ASSET}\n`; + assert.throws(() => parseChecksumEntry(sums, ASSET), /malformed checksum entry/i); +}); + +test("parseChecksumEntry: rejects a digest of the wrong length", () => { + const sums = `${"a".repeat(63)} ${ASSET}\n`; + assert.throws(() => parseChecksumEntry(sums, ASSET), /malformed checksum entry/i); +}); + +// ---------- maybeHintPath --------------------------------------------------- + +function withEnv(env, fn) { + const keys = ["npm_config_global", "npm_config_location", "npm_config_prefix", "PREFIX", "PATH", "SHELL"]; + const saved = {}; + for (const k of keys) saved[k] = process.env[k]; + for (const k of keys) delete process.env[k]; + Object.assign(process.env, env); + try { + return fn(); + } finally { + for (const k of keys) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } + } +} + +function captureStderr(fn) { + const original = process.stderr.write.bind(process.stderr); + const buf = []; + process.stderr.write = (chunk) => { + buf.push(typeof chunk === "string" ? chunk : chunk.toString("utf8")); + return true; + }; + try { + fn(); + } finally { + process.stderr.write = original; + } + return buf.join(""); +} + +// __dirname here is .../npm/scripts; install.js computes ourBinDir as +// path.resolve(__dirname, "..", "bin") which is .../npm/bin. +const OUR_BIN_DIR = path.resolve(__dirname, "..", "bin"); + +test("maybeHintPath: silent when not a global install", () => { + const out = withEnv({ PATH: "/usr/bin", SHELL: "/bin/zsh" }, () => captureStderr(maybeHintPath)); + assert.equal(out, ""); +}); + +test("maybeHintPath: silent when npm's bin is already on PATH", () => { + const out = withEnv( + { npm_config_global: "true", npm_config_prefix: "/usr/local", PATH: "/usr/local/bin:/usr/bin", SHELL: "/bin/zsh" }, + () => captureStderr(maybeHintPath), + ); + assert.equal(out, ""); +}); + +test("maybeHintPath: silent when our own bin is already on PATH", () => { + const out = withEnv( + { npm_config_global: "true", PATH: `${OUR_BIN_DIR}:/usr/bin`, SHELL: "/bin/zsh" }, + () => captureStderr(maybeHintPath), + ); + assert.equal(out, ""); +}); + +test("maybeHintPath: hints OUR bin (not npm's) when npm's bin isn't on PATH", () => { + const out = withEnv( + { npm_config_global: "true", npm_config_prefix: "/usr/local", PATH: "/usr/bin", SHELL: "/bin/zsh" }, + () => captureStderr(maybeHintPath), + ); + assert.match(out, new RegExp(OUR_BIN_DIR.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"))); + assert.doesNotMatch(out, /\/usr\/local\/bin/); +}); + +test("maybeHintPath: hints OUR bin when no prefix is known at all", () => { + // The old fallback would `path.resolve(__dirname, "..","..","..","..","..")` + // to guess ; we no longer do that. Lock in that we never print a + // path inferred by counting `..`. + const out = withEnv( + { npm_config_global: "true", PATH: "/usr/bin", SHELL: "/bin/zsh" }, + () => captureStderr(maybeHintPath), + ); + assert.match(out, new RegExp(OUR_BIN_DIR.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"))); +}); + +test("maybeHintPath: zsh hint uses ~/.zshrc + export PATH", () => { + const out = withEnv( + { npm_config_global: "true", PATH: "/usr/bin", SHELL: "/bin/zsh" }, + () => captureStderr(maybeHintPath), + ); + assert.match(out, /~\/\.zshrc/); + assert.match(out, /export PATH=/); +}); + +test("maybeHintPath: fish hint uses fish_add_path", () => { + const out = withEnv( + { npm_config_global: "true", PATH: "/usr/bin", SHELL: "/usr/local/bin/fish" }, + () => captureStderr(maybeHintPath), + ); + assert.match(out, /fish_add_path/); + assert.doesNotMatch(out, /export PATH=/); +}); + +test("maybeHintPath: bash hint uses ~/.bashrc", () => { + const out = withEnv( + { npm_config_global: "true", PATH: "/usr/bin", SHELL: "/bin/bash" }, + () => captureStderr(maybeHintPath), + ); + assert.match(out, /~\/\.bashrc/); +}); + +test("maybeHintPath: unknown shell falls back to generic profile wording", () => { + const out = withEnv( + { npm_config_global: "true", PATH: "/usr/bin", SHELL: "/bin/eshell" }, + () => captureStderr(maybeHintPath), + ); + assert.match(out, /your shell profile/); +}); + +test("maybeHintPath: respects npm_config_location=global", () => { + // npm sets this on global installs as an alternative to npm_config_global. + const out = withEnv( + { npm_config_location: "global", PATH: "/usr/bin", SHELL: "/bin/zsh" }, + () => captureStderr(maybeHintPath), + ); + assert.notEqual(out, ""); +}); From c7c749028572afc598a44241b135d1a4faafad56 Mon Sep 17 00:00:00 2001 From: liuooo Date: Sun, 31 May 2026 11:10:43 +0800 Subject: [PATCH 07/14] fix(npm): address PR #27 round-3 findings (Jerry-Xin + yujiawei) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three blockers and three non-blocking nits caught when reviewers re-read the round-3 push. Two of the blockers are mine introduced by the round-3 cleanup itself (the third is a pre-existing oversight the simplification made more obvious). Blockers - contents:read added back to npm-publish's publish job. When wait-for- artifacts was deleted, gh release view stopped being a caller — but actions/checkout under GITHUB_TOKEN auth still needs contents:read. Public- repo unauthenticated-clone fallback works today but isn't a contract worth relying on. (Jerry-Xin) - Tag-pinning across the dispatch handoff. release-publish.yml now passes --ref "$TAG" to gh workflow run, and npm-publish.yml's checkout uses ref: ${{ inputs.tag }}. Without these, workflow_dispatch resolved to default-branch HEAD; the wrapper sources packaged into the npm tarball (and attested by --provenance) would be whatever main HEAD happened to be at dispatch time, not the release commit. The README's "npm audit signatures" guarantee depends on this. (Jerry-Xin + yujiawei) - Wall-clock download deadline now actually bounds an in-flight request. Before this change, TOTAL_DOWNLOAD_DEADLINE_MS was only evaluated between retry attempts; a peer trickling bytes under the 30s socket-idle timeout could stall the install indefinitely while the code comment confidently claimed the deadline "guards against slow-trickle attacks." A new armDeadline() helper arms a setTimeout that calls req.destroy(err) when the wall-clock elapses, propagated through getOnce(url, deadline) → downloadOnce(url, deadline) → download(url). The timer is cleared on 'close' so a fast request doesn't leave a hanging timer. Idle timeout stays retryable (a flaky network can recover); wall-clock stays nonRetryable (deadline is a hard fail). (yujiawei) Tests - 3 new armDeadline tests on an EventEmitter fake-req: destroys with a nonRetryable error on elapse, fires immediately when deadline is in the past, and is cleared by 'close' so a fast request leaves no pending timer. Total now 35 tests, still ~180ms. Non-blocking - Tag validator alignment. release-publish.yml's pre-upload validator was (\.[0-9]+)? (allowed v1.2), npm-publish.yml's resolver was ^[0-9]+\.[0-9]+\.[0-9]+ (rejected v1.2). A tag accepted upstream could be rejected by npm-publish after archives are already on the release. Both now require MAJOR.MINOR.PATCH. (Jerry-Xin) - Third-party actions SHA-pinned in release-publish.yml and ci.yml to match npm-publish.yml's stance: actions/checkout@…34e114876b… (v4), actions/setup-go@…40f1582b… (v5.6.0), goreleaser/goreleaser-action@…e435ccd… (v6.4.0), actions/setup-node@…49933ea52… (v4). release-publish's build-artifacts job runs goreleaser, whose output is the install-time trust root, so a floating tag there was the highest-blast-radius hole. Org-level reusable workflow ref kept on @main (pre-existing, outside this PR's diff). (yujiawei) - run.js signal-handling comment corrected. The previous comment paired "130 for SIGINT" with the wrong mechanism. Two paths now spelled out: terminating signals (SIGINT/SIGTERM/...) terminate via process.kill re-raise (process.exit(...) below is unreachable); default-ignored or default-stopping signals (SIGPIPE/SIGUSR1/...) keep running, so the explicit process.exit(128 + signum) is what sets the exit code. (yujiawei) Verified - 35 tests pass, ~180ms (32 existing + 3 new armDeadline) - All 3 workflow YAMLs parse; zero ${{ }} interpolations remain in any run: block across the three files --- .github/workflows/ci.yml | 8 ++-- .github/workflows/npm-publish.yml | 12 ++++++ .github/workflows/release-publish.yml | 23 +++++++++--- npm/scripts/install.js | 54 ++++++++++++++++++++++----- npm/scripts/install.test.js | 46 +++++++++++++++++++++++ npm/scripts/run.js | 25 ++++++++++--- 6 files changed, 143 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 722e3a9..96c29ff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,9 +18,9 @@ jobs: matrix: go-version: ["1.24.x"] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: go-version: ${{ matrix.go-version }} check-latest: true @@ -66,8 +66,8 @@ jobs: run: working-directory: npm steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" - name: node --test diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index db36f0d..5c2e32d 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -39,15 +39,27 @@ permissions: {} jobs: publish: runs-on: ubuntu-latest + # contents:read is required by actions/checkout under GITHUB_TOKEN auth + # (works without it on public repos via the unauthenticated-clone + # fallback, but that's not a contract we want to rely on). # id-token:write lets npm provenance generate an OIDC-signed Sigstore # attestation linking the published tarball back to this workflow run. permissions: + contents: read id-token: write defaults: run: working-directory: npm steps: + # Pin to the release tag so the wrapper code in the published npm + # tarball matches the commit that the GitHub Release was cut from. + # Without this, workflow_dispatch resolves to the default-branch HEAD, + # which can have moved past the tag — and the `--provenance` + # attestation below would then point at the wrong SHA, breaking the + # `npm audit signatures` guarantee. - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + ref: ${{ inputs.tag }} - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20" diff --git a/.github/workflows/release-publish.yml b/.github/workflows/release-publish.yml index 8e8e263..973fade 100644 --- a/.github/workflows/release-publish.yml +++ b/.github/workflows/release-publish.yml @@ -44,20 +44,24 @@ jobs: # repository_dispatch are the only exempt events. actions: write steps: + # SHA-pinned actions. build-artifacts holds contents:write + + # actions:write and runs third-party code (goreleaser-action) that + # builds the binaries whose checksums become the install-time trust + # root, so any compromise here propagates into every install. - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: ref: ${{ inputs.tag }} fetch-depth: 0 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: go-version: "1.24.x" cache: true - name: Build with GoReleaser - uses: goreleaser/goreleaser-action@v6 + uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0 with: version: "~> v2" args: release --clean --skip=publish,announce @@ -68,8 +72,12 @@ jobs: run: ls -lh dist/ - name: Validate tag before upload + # Kept in lockstep with npm-publish.yml's resolver: MAJOR.MINOR.PATCH + # is required (no patchless `v1.2`) so a tag accepted by this workflow + # cannot be rejected by npm-publish.yml after archives are already on + # the release. run: | - [[ "$TAG" =~ ^v[0-9]+\.[0-9]+(\.[0-9]+)?([-.][0-9A-Za-z.-]+)?$ ]] || { echo "Invalid tag: $TAG"; exit 1; } + [[ "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]] || { echo "Invalid tag: $TAG"; exit 1; } env: TAG: ${{ inputs.tag }} @@ -83,8 +91,13 @@ jobs: # so the archives + checksums.txt are guaranteed to be on the release # by the time the npm postinstall tries to download them. Uses # workflow_dispatch (exempt from GITHUB_TOKEN recursion suppression). + # + # --ref "$TAG" pins both (a) which version of npm-publish.yml runs and + # (b) what its checkout step pulls — so the wrapper sources packaged + # into the npm tarball match the release commit, not whatever main HEAD + # happens to be at dispatch time. - name: Trigger npm publish env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} TAG: ${{ inputs.tag }} - run: gh workflow run npm-publish.yml -f tag="$TAG" -f dry_run=false + run: gh workflow run npm-publish.yml --ref "$TAG" -f tag="$TAG" -f dry_run=false diff --git a/npm/scripts/install.js b/npm/scripts/install.js index 3f6755f..63678a3 100644 --- a/npm/scripts/install.js +++ b/npm/scripts/install.js @@ -114,10 +114,16 @@ function parseChecksumEntry(sums, asset) { } // Single HTTPS GET. Resolves to either a buffered body or a redirect target, -// never both. Times out on socket idle so a half-open connection cannot hang -// the install indefinitely; a separate wall-clock deadline (download() below) -// guards against slow-trickle attacks that stay just under the idle timeout. -function getOnce(url) { +// never both. Two timeouts apply: +// - REQUEST_TIMEOUT_MS — socket-idle timeout (resets on each received +// byte). Catches half-open connections / dead sockets. +// - deadline (epoch ms) — wall-clock deadline armed via setTimeout, calls +// req.destroy() when reached. Catches a peer that trickles bytes just +// under the idle threshold and would otherwise stall the install +// indefinitely. The caller (download) passes a deadline that bounds the +// entire retry chain, not just this single request. +// Both are non-retryable so the caller fails fast instead of looping. +function getOnce(url, deadline) { return new Promise((resolve, reject) => { const req = https.get( url, @@ -161,13 +167,39 @@ function getOnce(url) { }, ); req.on("timeout", () => { - req.destroy(new Error(`request timed out after ${REQUEST_TIMEOUT_MS}ms: ${url}`)); + // Idle timeout (no bytes for REQUEST_TIMEOUT_MS) IS retryable — a flaky + // network can recover on the next attempt. Only the wall-clock deadline + // below is non-retryable. + req.destroy(new Error(`request idle for ${REQUEST_TIMEOUT_MS}ms: ${url}`)); }); req.on("error", reject); + + // Wall-clock deadline. Fires *during* the response body so a slow-trickle + // peer is aborted mid-stream, not just between retries. + if (deadline) armDeadline(req, deadline, url); }); } -async function downloadOnce(url) { +// Arm a wall-clock deadline on an in-flight request. When `deadline` is +// reached, `req.destroy(err)` raises an error on the request — caught by +// the caller's `.on('error', reject)` — and the promise rejects with our +// deadline error. The timer is cleared on `'close'` so a successful request +// doesn't leave a hanging timer that prevents Node from exiting. Exported +// for tests; production callers use `getOnce(url, deadline)`. +function armDeadline(req, deadline, url) { + const remaining = Math.max(deadline - Date.now(), 0); + const timer = setTimeout(() => { + req.destroy( + nonRetryable( + new Error(`download exceeded wall-clock deadline (${TOTAL_DOWNLOAD_DEADLINE_MS / 1000}s): ${url}`), + ), + ); + }, remaining); + req.on("close", () => clearTimeout(timer)); + return timer; +} + +async function downloadOnce(url, deadline) { let current = url; for (let hop = 0; hop <= MAX_REDIRECTS; hop++) { let parsed; @@ -177,7 +209,7 @@ async function downloadOnce(url) { throw nonRetryable(new Error(`malformed URL at hop ${hop}: ${e.message}`)); } assertSafeHost(parsed, hop === 0 ? "request" : `redirect #${hop}`); - const r = await getOnce(current); + const r = await getOnce(current, deadline); if (r.kind === "body") return r.body; current = r.next.toString(); } @@ -188,8 +220,9 @@ async function downloadOnce(url) { // 404 is treated as a non-retryable signal that the version doesn't exist on // the release yet — fast-fail with a clear error rather than spin. Errors // explicitly flagged `nonRetryable` (allowlist / parse / deadline) are not -// retried either. A wall-clock deadline caps the total time spent across all -// retries so a slow-trickle peer cannot keep the install alive. +// retried either. The deadline is passed all the way down to getOnce so it +// can abort an in-flight request mid-stream, not just guard the gap between +// retries. async function download(url) { const deadline = Date.now() + TOTAL_DOWNLOAD_DEADLINE_MS; for (let attempt = 0; ; attempt++) { @@ -197,7 +230,7 @@ async function download(url) { throw nonRetryable(new Error(`download exceeded ${TOTAL_DOWNLOAD_DEADLINE_MS / 1000}s wall-clock deadline: ${url}`)); } try { - return await downloadOnce(url); + return await downloadOnce(url, deadline); } catch (e) { const status = e && e.httpStatus; const retryable = @@ -373,4 +406,5 @@ module.exports = { assetName, parseChecksumEntry, ALLOWED_HOSTS, + armDeadline, }; diff --git a/npm/scripts/install.test.js b/npm/scripts/install.test.js index 2d1f23e..178d4fd 100644 --- a/npm/scripts/install.test.js +++ b/npm/scripts/install.test.js @@ -11,6 +11,8 @@ const test = require("node:test"); const assert = require("node:assert/strict"); const path = require("node:path"); +const { EventEmitter } = require("node:events"); + const { assertSafeHost, isValidReleaseSemver, @@ -18,6 +20,7 @@ const { parseChecksumEntry, maybeHintPath, ALLOWED_HOSTS, + armDeadline, } = require("./install.js"); // ---------- assertSafeHost -------------------------------------------------- @@ -178,6 +181,49 @@ test("parseChecksumEntry: rejects a digest of the wrong length", () => { assert.throws(() => parseChecksumEntry(sums, ASSET), /malformed checksum entry/i); }); +// ---------- armDeadline (wall-clock guard on an in-flight request) --------- + +// Minimal stand-in for http.ClientRequest: an EventEmitter with destroy(). +function fakeReq() { + const e = new EventEmitter(); + e.destroyed = false; + e.destroyError = null; + e.destroy = (err) => { + e.destroyed = true; + e.destroyError = err; + }; + return e; +} + +test("armDeadline: destroys req with a nonRetryable error when deadline elapses", async () => { + const req = fakeReq(); + armDeadline(req, Date.now() + 30, "https://example/x"); + await new Promise((r) => setTimeout(r, 60)); + assert.equal(req.destroyed, true); + assert.ok(req.destroyError, "expected a destroy error"); + assert.equal(req.destroyError.nonRetryable, true); + assert.match(req.destroyError.message, /wall-clock deadline/i); + assert.match(req.destroyError.message, /example/); +}); + +test("armDeadline: fires immediately when deadline is in the past", async () => { + const req = fakeReq(); + armDeadline(req, Date.now() - 5000, "https://example/x"); + await new Promise((r) => setTimeout(r, 10)); + assert.equal(req.destroyed, true); +}); + +test("armDeadline: cleared by 'close' so a fast request leaves no pending timer", async () => { + const req = fakeReq(); + armDeadline(req, Date.now() + 30, "https://example/x"); + // Simulate the request completing well before the deadline. + req.emit("close"); + // Wait past the original deadline and confirm destroy was never called. + await new Promise((r) => setTimeout(r, 60)); + assert.equal(req.destroyed, false); + assert.equal(req.destroyError, null); +}); + // ---------- maybeHintPath --------------------------------------------------- function withEnv(env, fn) { diff --git a/npm/scripts/run.js b/npm/scripts/run.js index 0e7f91a..12bd65d 100644 --- a/npm/scripts/run.js +++ b/npm/scripts/run.js @@ -24,12 +24,25 @@ if (res.error) { process.exit(1); } -// If the binary was killed by a signal, re-raise the same signal so callers -// observe the conventional 128+signum exit code (e.g. 130 for SIGINT) instead -// of a generic 1 that hides Ctrl-C from shells and supervisors. Use the -// platform's signal map (os.constants.signals) so signals outside the small -// hand-coded set (e.g. SIGPIPE, SIGUSR1) also get a faithful exit code on -// the fallback path. +// If the binary was killed by a signal, propagate it so callers observe the +// conventional 128+signum exit code instead of a generic 1 that hides +// Ctrl-C / kill from shells and supervisors. Two cases, handled in order: +// +// 1. Terminating signals (SIGINT, SIGTERM, SIGQUIT, SIGHUP, ...) — +// `process.kill(self, sig)` re-raises the signal, the Node runtime +// handles it as it would for a direct signal: it terminates the +// process and the shell sees 128+signum (e.g. 130 for SIGINT). The +// `process.exit(...)` line below is unreachable on this path. +// +// 2. Default-ignored or default-stopping signals (SIGPIPE, SIGUSR1/2, +// SIGCHLD, ...) — Node's default disposition is to ignore them, so +// `process.kill(self, sig)` returns and the script keeps running. +// In that case the explicit `process.exit(128 + signum)` is what +// sets the exit code (e.g. 141 for SIGPIPE) — without it the shim +// would exit 0 even though the child died from a signal. +// +// os.constants.signals gives the platform's real signal numbers so the +// fallback isn't limited to a hand-coded set. if (res.signal) { process.kill(process.pid, res.signal); const signum = (os.constants && os.constants.signals && os.constants.signals[res.signal]) || 0; From a7d138ebf5c68bd5d344c48416387c804e13b306 Mon Sep 17 00:00:00 2001 From: liuooo Date: Sun, 31 May 2026 11:38:50 +0800 Subject: [PATCH 08/14] fix(ci): satisfy actionlint SC2086 in npm publish step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit actionlint flags the previous `$DRY_RUN` unquoted expansion (shellcheck SC2086), which fails the `sanity / actionlint` job. The value is workflow-derived (`--dry-run` or empty literal, never user input), so the unquoted form was safe — but actionlint runs in CI, so this had to go either way. Split the publish into two explicit branches instead. No behavioural change. --- .github/workflows/npm-publish.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index 5c2e32d..d1c33ee 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -105,9 +105,19 @@ jobs: # package — both satisfied). Consumers can verify the link with # `npm audit signatures`; this is the only origin link the user gets # for the wrapper itself (the binary's sha256 check is independent). + # + # The dry-run path is split into a separate branch rather than relying + # on an unquoted `$DRY_RUN` expansion (SC2086 / actionlint). The value + # is workflow-derived and never user-controlled, so the original idiom + # was safe — but actionlint flags it and clarity isn't worse this way. - name: Publish to npm env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} DIST_TAG: ${{ steps.version.outputs.DIST_TAG }} DRY_RUN: ${{ steps.version.outputs.DRY_RUN }} - run: npm publish --access public --provenance --tag "$DIST_TAG" $DRY_RUN + run: | + if [ -n "$DRY_RUN" ]; then + npm publish --access public --provenance --tag "$DIST_TAG" --dry-run + else + npm publish --access public --provenance --tag "$DIST_TAG" + fi From 3bef8bd3642123abe15d00b8d5061cdde6c9bcc0 Mon Sep 17 00:00:00 2001 From: liuooo Date: Sun, 31 May 2026 12:01:53 +0800 Subject: [PATCH 09/14] fix(npm): close round-4 review findings (yujiawei) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One P1 (downstream-publisher privilege separation) and three P2s. The P1 is the structural one: this PR is where the public-registry publish pipeline is introduced, so the gate belongs in the diff that ships it. P1 — Release-existence gate on npm-publish (privilege separation) Before this change, anyone with repo:write could workflow_dispatch npm-publish.yml against any resolvable ref (incl. a lightweight tag at a malicious commit) with dry_run=false and ship that ref to npm @latest. The Sigstore --provenance attestation would faithfully attest the malicious SHA, so `npm audit signatures` would pass and give consumers false assurance. release-publish.yml's validate_run_id evidence check does not apply to direct dispatch of the downstream publisher. The new gate runs gh release view "$TAG" before publish: refuses if the Release doesn't exist or is still a draft. dry_run skips the gate so developers can validate workflow plumbing against tags whose Release doesn't exist yet (no NPM_TOKEN is actually used on that path). This closes the gap with a code-only change. yujiawei also suggested adding a protected Environment with required reviewers as a second, independent layer (gates the "any write-access actor can dispatch" problem rather than the "any ref can be published" one) — that requires repo-settings configuration and is tracked as follow-up. P2.1 — Tmp archive leaked on extraction failure The try/finally that unlinked the tmp archive lived around a catch that called fail() → process.exit(1). Node does NOT run finally blocks across process.exit, so any extract failure (missing tar, malformed archive, symlink guard tripping) left up to 200 MiB sitting in npm/bin/. Fix: capture the error, unlink unconditionally, then fail() once at the end. P2.3 — run.js signal comment over-claimed POSIX semantics as universal On win32, spawnSync rarely sets res.signal, so the whole signal- propagation block is effectively a no-op there. Comment now flags this without short-circuiting the code, so a future Node version that does surface signals on Windows would still get the faithful fallback. P2.4 — .goreleaser.yaml now pins checksum algorithm The install-side checksum parser asserts ^[0-9a-f]{64}$ (sha256). The producer was implicitly relying on goreleaser's sha256 default. Now pinned with `algorithm: sha256` and a comment naming the install-side counterpart so a future maintainer changing one will know to change the other. Deliberately NOT in this PR - empty-release window (release.published fires before archives upload): npm path is insulated; fixing this would require splitting the reusable-release-publish workflow into draft-publish + flip-out-of- draft phases, which is a release-pipeline rewrite. Tracked as follow-up. - Environment gate with required reviewers: needs a `npm-publish` Environment to exist in repo Settings first. Tracked as follow-up; the release-existence gate above closes the most exploitable variant. --- .github/workflows/npm-publish.yml | 29 +++++++++++++++++++++++++ .goreleaser.yaml | 6 ++++++ npm/scripts/install.js | 36 ++++++++++++++++++++++--------- npm/scripts/run.js | 9 +++++++- 4 files changed, 69 insertions(+), 11 deletions(-) diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index d1c33ee..f271654 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -95,6 +95,35 @@ jobs: } >> "$GITHUB_OUTPUT" echo "[npm-publish] version=$VERSION dist-tag=$DIST_TAG dry_run='$DRY'" + # Gate: refuse to publish unless `tag` corresponds to a real, + # already-published (non-draft) GitHub Release. Without this gate, any + # actor with repo:write could directly workflow_dispatch this workflow + # against any resolvable ref (incl. a lightweight tag at a malicious + # commit), set dry_run=false, and ship that ref to npm @latest with a + # valid Sigstore provenance attestation pointing at the malicious SHA. + # The release-publish.yml chain (with its validate_run_id evidence + # check) is the intended publish trigger; this gate enforces that + # invariant on the downstream workflow too. + # Skipped on dry_run so developers can validate workflow plumbing + # against tags whose Release doesn't exist yet. + - name: Verify release exists and is published + if: steps.version.outputs.DRY_RUN == '' + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ steps.version.outputs.TAG }} + run: | + set -euo pipefail + info=$(gh release view "$TAG" --repo "$GITHUB_REPOSITORY" --json isDraft,tagName 2>/dev/null) || { + echo "::error::No GitHub Release for $TAG. npm-publish requires a published Release for the tag — use release-publish.yml to cut one; direct workflow_dispatch of an arbitrary tag is not permitted." + exit 1 + } + is_draft=$(printf '%s' "$info" | jq -r '.isDraft') + if [ "$is_draft" = "true" ]; then + echo "::error::Release $TAG is still a draft; publish it first (release-publish.yml flips it out of draft after artifacts upload)." + exit 1 + fi + echo "[npm-publish] release $TAG exists and is published, proceeding." + - name: Set package version env: VERSION: ${{ steps.version.outputs.VERSION }} diff --git a/.goreleaser.yaml b/.goreleaser.yaml index e4014d7..4f36939 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -28,6 +28,12 @@ archives: - LICENSE* checksum: + # Pin the algorithm so it doesn't silently drift with goreleaser defaults. + # Lockstep with npm/scripts/install.js's parseChecksumEntry, which asserts + # `^[0-9a-f]{64}$` (64 lowercase hex chars = sha256). If you change this + # algorithm here, update the regex and the corresponding test in + # npm/scripts/install.test.js. + algorithm: sha256 name_template: "checksums.txt" changelog: diff --git a/npm/scripts/install.js b/npm/scripts/install.js index 63678a3..5e3acaa 100644 --- a/npm/scripts/install.js +++ b/npm/scripts/install.js @@ -348,10 +348,17 @@ async function main() { // GNU tar (Linux) both handle the formats we ship (.tar.gz via -xzf, .zip // via -xf). On older Windows / minimal containers tar may be absent — give // a targeted message instead of a bare ENOENT. + // + // NOTE on cleanup: we can't use a try/finally to remove the tmp archive, + // because every failure path calls fail() → process.exit(1) and Node does + // NOT run finally blocks across process.exit(). Capture the error, + // unlink unconditionally, then fail() once at the end. fs.mkdirSync(binDir, { recursive: true }); const tmp = path.join(binDir, asset); const extractedPath = path.join(binDir, BIN_NAME); fs.writeFileSync(tmp, archive); + + let extractErr = null; try { const args = isWin ? ["-xf", tmp, "-C", binDir, BIN_NAME] : ["-xzf", tmp, "-C", binDir, BIN_NAME]; execFileSync("tar", args, { stdio: "inherit" }); @@ -371,24 +378,33 @@ async function main() { } catch { /* leave it; the fail() below is what matters */ } - fail(`extracted '${BIN_NAME}' is not a regular file (mode ${(st.mode & 0o170000).toString(8)})`); + extractErr = new Error( + `extracted '${BIN_NAME}' is not a regular file (mode ${(st.mode & 0o170000).toString(8)})`, + ); + } else { + fs.chmodSync(extractedPath, 0o755); } - fs.chmodSync(extractedPath, 0o755); } catch (e) { - if (e && e.code === "ENOENT") { + extractErr = e; + } + + // Always remove the tmp archive (200 MiB cap) regardless of outcome — see + // the NOTE above. + try { + fs.unlinkSync(tmp); + } catch { + /* ignore */ + } + + if (extractErr) { + if (extractErr.code === "ENOENT") { fail( isWin ? "`tar.exe` not found on PATH. Windows 10 build 1803+ ships bsdtar; on older systems install Git for Windows or 7-Zip and re-run." : "`tar` not found on PATH. Install GNU tar or bsdtar (e.g. apt-get install tar, apk add tar) and re-run.", ); } - fail(`extract failed: ${e.message}`); - } finally { - try { - fs.unlinkSync(tmp); - } catch { - /* ignore */ - } + fail(`extract failed: ${extractErr.message}`); } console.log(`[octo-cli] installed octo-cli ${VERSION} (${OS}/${ARCH})`); diff --git a/npm/scripts/run.js b/npm/scripts/run.js index 12bd65d..bc26d94 100644 --- a/npm/scripts/run.js +++ b/npm/scripts/run.js @@ -26,7 +26,14 @@ if (res.error) { // If the binary was killed by a signal, propagate it so callers observe the // conventional 128+signum exit code instead of a generic 1 that hides -// Ctrl-C / kill from shells and supervisors. Two cases, handled in order: +// Ctrl-C / kill from shells and supervisors. +// +// POSIX-only path: on win32, Node's spawnSync rarely (~never) sets +// res.signal — the OS does not deliver POSIX signals to child processes the +// same way — so this block is effectively a no-op on Windows. We don't +// short-circuit on platform because that would needlessly skip the +// faithful fallback if any future Node version starts surfacing signals +// there. The two POSIX cases, handled in order: // // 1. Terminating signals (SIGINT, SIGTERM, SIGQUIT, SIGHUP, ...) — // `process.kill(self, sig)` re-raises the signal, the Node runtime From 5d0c0cf0aad43baeb5a18f4abe38be997b1e08b9 Mon Sep 17 00:00:00 2001 From: liuooo Date: Sun, 31 May 2026 12:05:48 +0800 Subject: [PATCH 10/14] fix(ci): emit TAG output the release-verify step depends on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The round-5 \"Verify release exists and is published\" step reads \${{ steps.version.outputs.TAG }}, but the \"Resolve version and dist-tag\" step never wrote a TAG output — only VERSION, DIST_TAG, DRY_RUN. On a non-dry-run publish, TAG resolves to empty, so \`gh release view \"\"\` fails and every real publish is blocked at the gate. Added \`echo \"TAG=v\$VERSION\"\` to the outputs block (TAG is the normalized v-prefixed form, derived from the same VERSION used for the download URL). Cross-checked all readers / writers of steps.version.outputs in the file; the four readers (DRY_RUN, TAG, VERSION, DIST_TAG) now all have matching writers. Reported by Jerry-Xin on #27. --- .github/workflows/npm-publish.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index f271654..bf1882e 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -88,8 +88,12 @@ jobs: exit 1 fi if [[ "$VERSION" == *-* ]]; then DIST_TAG="next"; else DIST_TAG="latest"; fi + # TAG is the normalized v-prefixed form (independent of whether the + # caller passed "v0.6.0" or "0.6.0"). Consumers: "Verify release + # exists and is published" step below. { echo "VERSION=$VERSION" + echo "TAG=v$VERSION" echo "DIST_TAG=$DIST_TAG" echo "DRY_RUN=$DRY" } >> "$GITHUB_OUTPUT" From 3fd1cc630cc890a161908fdd8d1e132581e5fbc1 Mon Sep 17 00:00:00 2001 From: liuooo Date: Sun, 31 May 2026 12:09:29 +0800 Subject: [PATCH 11/14] fix(ci): close gh-release-view footgun in npm-publish gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviewers (yujiawei, lml2468) independently flagged: the release- verify gate was running \`gh release view "\$TAG"\`, but if \$TAG ever went empty (it just did, two commits ago) \`gh release view ""\` does NOT error — it returns the *latest* release. Verified: $ gh release view "" --repo Mininglamp-OSS/octo-cli --json isDraft,tagName {"isDraft":false,"tagName":"v0.5.0"} # exit 0 So the previous commit fixed the empty-TAG symptom but left the gate's contract on a knife edge: any future regression that empties TAG would silently rubber-stamp the latest release while \`actions/checkout\` keeps packaging the attacker-supplied ref. That defeats the very threat the step's docstring describes. Two belt-and-braces guards added inside the verify step: 1. Empty-TAG bail. If the resolver produced no TAG output, abort with an internal-bug error instead of feeding "" to gh. This catches the regression class directly. 2. tagName equality check. After gh returns, compare the .tagName field to the TAG we asked for and refuse if they differ. This catches any future gh CLI change that adds another silently-resolves-to-latest fallback (currently it's just the empty-string case, but the defensive check is a few lines and removes the assumption). The check order keeps existing semantics on the happy path (resolver emits TAG correctly → gh finds matching release → isDraft check → publish). Reported by yujiawei (P0) and lml2468 (Blocking) on #27. --- .github/workflows/npm-publish.yml | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index bf1882e..2ceb1bc 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -117,16 +117,38 @@ jobs: TAG: ${{ steps.version.outputs.TAG }} run: | set -euo pipefail + + # Footgun guard #1 — `gh release view ""` does NOT error: it + # resolves to the latest release. If a future regression makes + # this output empty (it has happened in this PR's history), the + # gate would silently validate the latest release instead of the + # requested tag and let an arbitrary ref through. Bail explicitly. + if [ -z "$TAG" ]; then + echo "::error::Empty TAG output from resolver step — gate cannot run. This is an internal workflow bug, not an input error." + exit 1 + fi + info=$(gh release view "$TAG" --repo "$GITHUB_REPOSITORY" --json isDraft,tagName 2>/dev/null) || { echo "::error::No GitHub Release for $TAG. npm-publish requires a published Release for the tag — use release-publish.yml to cut one; direct workflow_dispatch of an arbitrary tag is not permitted." exit 1 } + + # Footgun guard #2 — even with a non-empty TAG, defensively assert + # gh actually returned the release we asked for. Belt-and-braces + # against any future gh CLI change that adds a different + # silently-resolves-to-latest fallback. + returned_tag=$(printf '%s' "$info" | jq -r '.tagName') + if [ "$returned_tag" != "$TAG" ]; then + echo "::error::gh release view returned a different tag ('$returned_tag') than requested ('$TAG'). Refusing to publish." + exit 1 + fi + is_draft=$(printf '%s' "$info" | jq -r '.isDraft') if [ "$is_draft" = "true" ]; then echo "::error::Release $TAG is still a draft; publish it first (release-publish.yml flips it out of draft after artifacts upload)." exit 1 fi - echo "[npm-publish] release $TAG exists and is published, proceeding." + echo "[npm-publish] release $TAG exists, is published, and matches the requested tag — proceeding." - name: Set package version env: From e3e5e8418d26dd537280d0b24df2aab9f8e44d40 Mon Sep 17 00:00:00 2001 From: liuooo Date: Sun, 31 May 2026 12:16:45 +0800 Subject: [PATCH 12/14] fix(ci): close input-normalization gap that bypassed the release gate Both yujiawei and lml2468 independently flagged the same deeper hole: the release gate's TAG was the v-normalized form (\`v\${VERSION}\`), but \`actions/checkout\` was pinned with raw \`\${{ inputs.tag }}\`. Attack: a write-capable actor creates a branch named \`0.6.0\` containing malicious code, dispatches npm-publish.yml with \`tag=0.6.0\` and \`dry_run=false\`. checkout pulls the branch (raw tag), Resolve normalizes to TAG=v0.6.0, the gate's \`gh release view "v0.6.0"\` passes against the legitimate release, the tagName-equality check passes (both sides are v-prefixed), and npm publish ships the branch content with valid Sigstore provenance. Two changes, defense-in-depth: 1. **Strict v-prefix on the input.** Resolve now rejects bare semver (\`0.6.0\`) and branch-ish refs (\`main\`). The check happens before any filesystem operation, so attacker content never lands in the workspace. Resolve was moved ahead of checkout (it only reads inputs and writes outputs, no checkout dependency) and given \`working-directory: .\` to override the job-level npm/ default that doesn't exist yet. 2. **\`refs/tags/\` checkout prefix.** Even with the v-prefix enforced, a same-named branch could in theory shadow a tag. \`refs/tags/\` disambiguates: only an actual tag in the tags namespace resolves. The two together make the contract: input must carry the 'v', and the ref must be a tag in the tags namespace. Combined with the previous round's empty-TAG guard and tagName-equality check, the gate now validates the same release the checkout will package, with no normalization seam left to exploit. Smoke-tested input rejection against `0.6.0`, `main`, empty, and an injection-style payload. All correctly rejected. `v0.6.0` and `v0.6.0-rc.1` accepted. Reported by yujiawei (P0) and lml2468 (Blocking) on #27. --- .github/workflows/npm-publish.yml | 71 ++++++++++++++++++------------- 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index 2ceb1bc..9181d2e 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -51,46 +51,45 @@ jobs: run: working-directory: npm steps: - # Pin to the release tag so the wrapper code in the published npm - # tarball matches the commit that the GitHub Release was cut from. - # Without this, workflow_dispatch resolves to the default-branch HEAD, - # which can have moved past the tag — and the `--provenance` - # attestation below would then point at the wrong SHA, breaking the - # `npm audit signatures` guarantee. - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - with: - ref: ${{ inputs.tag }} - - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 - with: - node-version: "20" - registry-url: "https://registry.npmjs.org" - + # Resolve runs BEFORE checkout — only reads inputs and writes outputs, + # no filesystem dependency. Putting it first means a rejected input + # never reaches actions/checkout, so attacker-supplied refs can't + # land any content in the workspace before validation runs. + # + # `working-directory: .` overrides the job-level default of `npm`, + # which doesn't exist yet (checkout is the next step). + # + # IMPORTANT: dispatch inputs are read through env: shell variables, + # not interpolated into the run: block. Direct `${{ inputs.tag }}` + # would be substituted as literal text before bash parses the line, + # letting a crafted tag value break out of the assignment and run + # arbitrary commands in a step that later has NPM_TOKEN in scope + # (CWE-94). - name: Resolve version and dist-tag id: version - # VERSION drops the leading `v`; a version containing `-` - # (prerelease) publishes to @next so it never clobbers @latest. - # - # IMPORTANT: dispatch inputs are read through env: shell variables, - # not interpolated into the run: block. Direct `${{ inputs.tag }}` - # would be substituted as literal text before bash parses the line, - # letting a crafted tag value break out of the assignment and run - # arbitrary commands in a step that later has NPM_TOKEN in scope - # (CWE-94). + working-directory: . env: INPUT_TAG: ${{ inputs.tag }} INPUT_DRY_RUN: ${{ inputs.dry_run }} run: | REF="$INPUT_TAG" if [ "$INPUT_DRY_RUN" = "true" ]; then DRY="--dry-run"; else DRY=""; fi - VERSION="${REF#v}" - if [[ ! "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]]; then - echo "::error::Refusing to publish: tag '$REF' does not look like a semver release." + # Strict v-prefixed semver. Bare semver like "0.6.0" is rejected + # so that the downstream `refs/tags/$INPUT_TAG` checkout cannot + # be tricked into picking up an attacker-created branch named + # "0.6.0" (no v) while the release gate validates the legitimate + # "v0.6.0" release. Enforcing the 'v' here keeps every consumer + # in lockstep: checkout, gate, set-version, and publish all see + # the same normalized form. + if [[ ! "$REF" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]]; then + echo "::error::inputs.tag must be v-prefixed semver (got '$REF'). Bare semver like '0.6.0' is rejected to prevent branch/tag-name confusion." exit 1 fi + VERSION="${REF#v}" if [[ "$VERSION" == *-* ]]; then DIST_TAG="next"; else DIST_TAG="latest"; fi - # TAG is the normalized v-prefixed form (independent of whether the - # caller passed "v0.6.0" or "0.6.0"). Consumers: "Verify release - # exists and is published" step below. + # TAG is the v-prefixed form (same as INPUT_TAG, just spelled out + # for downstream readability). Consumers: checkout ref below and + # the "Verify release exists and is published" step. { echo "VERSION=$VERSION" echo "TAG=v$VERSION" @@ -99,6 +98,20 @@ jobs: } >> "$GITHUB_OUTPUT" echo "[npm-publish] version=$VERSION dist-tag=$DIST_TAG dry_run='$DRY'" + # Pin to the release tag so the wrapper code in the published npm + # tarball matches the commit that the GitHub Release was cut from. + # Without `refs/tags/` prefix, actions/checkout will accept a same- + # named branch — letting an attacker who creates a branch matching + # the tag substitute branch contents into the published tarball + # while the gate still validates the legitimate release. + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + ref: refs/tags/${{ steps.version.outputs.TAG }} + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: "20" + registry-url: "https://registry.npmjs.org" + # Gate: refuse to publish unless `tag` corresponds to a real, # already-published (non-draft) GitHub Release. Without this gate, any # actor with repo:write could directly workflow_dispatch this workflow From 81a5752dbe984aa15eec4912dcf1df454b981aba Mon Sep 17 00:00:00 2001 From: liuooo Date: Sun, 31 May 2026 14:57:13 +0800 Subject: [PATCH 13/14] fix(ci): extend npm-publish's ref-ambiguity hardening to release-publish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous round closed the input-normalization / tag-vs-branch gap in \`npm-publish.yml\`, but Jerry-Xin pointed out the same shape of attack existed in \`release-publish.yml\`'s build-artifacts job: both the checkout and the cross-workflow dispatch were using raw \`inputs.tag\` / raw \`\$TAG\`, which actions/checkout and gh CLI both resolve as branch- or-tag. An attacker with a branch name colliding with the tag (e.g. a \`v0.6.0\` branch) could: (a) sneak malicious code into the goreleaser build by intercepting the Checkout step's ref resolution; or (b) force \`gh workflow run --ref \"\$TAG\"\` to pull a poisoned version of npm-publish.yml itself when dispatching the npm publish — even though npm-publish.yml's own checkout was already pinned to refs/tags/. Three changes mirroring the npm-publish hardening: 1. **Validate-before-checkout.** New first step rejects non-v-prefixed inputs (\`0.6.0\`, \`main\`, injection payloads) before any filesystem operation. The previous \"Validate tag before upload\" step ran after the privileged GoReleaser build and after checkout — too late. 2. **refs/tags/ checkout prefix.** \`ref: refs/tags/\${{ inputs.tag }}\` forces the tags namespace, so a same-named branch can't shadow the tag. 3. **Trusted workflow ref for dispatch.** Dropping the \`--ref \"\$TAG\"\` argument lets gh take npm-publish.yml from the repo's default branch instead of the (potentially fake) tag. The packaging contents are still pinned by npm-publish.yml's own \`refs/tags/\` checkout, so the published tarball still matches the release commit. This separates \"which workflow file runs\" (trusted) from \"which source it packages\" (release tag). This brings release-publish.yml's posture in line with npm-publish.yml, which is the only sensible end state — the two workflows are one security boundary. Verified - actionlint clean on all 3 workflows - 35 npm tests still pass - step order: Validate → Checkout(refs/tags/) → Build → Upload → Dispatch Reported by Jerry-Xin on #27. --- .github/workflows/release-publish.yml | 48 ++++++++++++++++++--------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/.github/workflows/release-publish.yml b/.github/workflows/release-publish.yml index 973fade..3bc7adf 100644 --- a/.github/workflows/release-publish.yml +++ b/.github/workflows/release-publish.yml @@ -44,14 +44,36 @@ jobs: # repository_dispatch are the only exempt events. actions: write steps: + # Validate the tag input BEFORE checkout so attacker-supplied refs + # never land in the workspace. Same regex as npm-publish.yml's + # resolver: strict v-prefixed MAJOR.MINOR.PATCH (+ optional + # prerelease). Bare semver like "0.6.0" is rejected because the + # checkout below uses `refs/tags/${{ inputs.tag }}` — a non-v input + # would either fail to resolve (good) or pick up an attacker-created + # tag named without the v (bad), so reject ambiguous shapes up front. + - name: Validate tag input (pre-checkout) + env: + TAG: ${{ inputs.tag }} + run: | + if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]]; then + echo "::error::inputs.tag must be v-prefixed semver (got '$TAG')." + exit 1 + fi + # SHA-pinned actions. build-artifacts holds contents:write + # actions:write and runs third-party code (goreleaser-action) that # builds the binaries whose checksums become the install-time trust # root, so any compromise here propagates into every install. + # + # `refs/tags/` prefix is required: with a bare ref like `v0.6.0`, + # actions/checkout will accept either a tag OR a same-named branch. + # An attacker creating a branch matching the tag could substitute + # the build source while the release evidence still refers to the + # legitimate tag. Forcing the tags namespace disambiguates. - name: Checkout uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: - ref: ${{ inputs.tag }} + ref: refs/tags/${{ inputs.tag }} fetch-depth: 0 - name: Set up Go @@ -71,16 +93,6 @@ jobs: - name: List dist artifacts run: ls -lh dist/ - - name: Validate tag before upload - # Kept in lockstep with npm-publish.yml's resolver: MAJOR.MINOR.PATCH - # is required (no patchless `v1.2`) so a tag accepted by this workflow - # cannot be rejected by npm-publish.yml after archives are already on - # the release. - run: | - [[ "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]] || { echo "Invalid tag: $TAG"; exit 1; } - env: - TAG: ${{ inputs.tag }} - - name: Upload artifacts to release run: gh release upload "$TAG" dist/*.tar.gz dist/*.zip dist/checksums.txt --clobber env: @@ -92,12 +104,16 @@ jobs: # by the time the npm postinstall tries to download them. Uses # workflow_dispatch (exempt from GITHUB_TOKEN recursion suppression). # - # --ref "$TAG" pins both (a) which version of npm-publish.yml runs and - # (b) what its checkout step pulls — so the wrapper sources packaged - # into the npm tarball match the release commit, not whatever main HEAD - # happens to be at dispatch time. + # No `--ref` argument: gh dispatches against the repo's default + # branch, so the npm-publish.yml definition is taken from a trusted + # ref (not from the release tag, which could in theory be a fake one + # in a branch/tag ambiguity scenario). The packaging contents are + # separately pinned by npm-publish.yml's internal checkout of + # `refs/tags/${{ inputs.tag }}`, so the published tarball still + # matches the release commit. This separates "which workflow file + # runs" (trusted) from "which source it packages" (release tag). - name: Trigger npm publish env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} TAG: ${{ inputs.tag }} - run: gh workflow run npm-publish.yml --ref "$TAG" -f tag="$TAG" -f dry_run=false + run: gh workflow run npm-publish.yml -f tag="$TAG" -f dry_run=false From 9e22c757505c694561cbf32c80775b1d1688bada Mon Sep 17 00:00:00 2001 From: liuooo Date: Sun, 31 May 2026 16:17:02 +0800 Subject: [PATCH 14/14] fix(ci): mirror release-publish's validate_run_id check in npm-publish gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit yujiawei flagged that the release-existence gate did not actually enforce the invariant its own comment claimed: a repo:write actor could still bypass release-publish.yml entirely by pushing a tag at an arbitrary commit, creating a published Release for it via API/UI, and dispatching this workflow — every check (regex, exists, tagName, non-draft) would pass and npm would ship the attacker-controlled commit with a valid Sigstore provenance attestation. The reusable workflow that backs release-publish.yml requires an operator-supplied validate_run_id (a successful CI run whose head_sha matches the tagged commit). That's the missing invariant; this commit re-asserts it inline in npm-publish.yml. New "Verify CI evidence for the tagged commit" step: 1. Resolve refs/tags/$TAG → commit SHA. Handles annotated tags (deref the tag object to its target commit). Refuses if the annotated tag points at anything other than a commit. 2. Query `actions/runs?head_sha=&status=success` and require at least one workflow_run named "CI" on that exact SHA. The "CI" name matches reusable-release-publish.yml's `ci_workflow_name` default; if the workflow is ever renamed, both places must change in lockstep. Permissions: + actions: read (new, for actions/runs query) contents: read id-token: write The gate documentation also rewritten as "Gate (part 1 of 2)" / "Gate (part 2 of 2)" so the two-step invariant is explicit and the old comment no longer over-claims. Skipped on dry_run (consistent with the release-existence gate above) so developers can validate plumbing against tags whose Release doesn't exist yet. Smoke-tested the query against v0.5.0's commit (b5a4d6a): returns 1 successful CI run, so legitimate tags pass. An attacker-pushed tag at a commit that never went through CI would return 0 and the gate would refuse. Reported by yujiawei (P1) on #27. --- .github/workflows/npm-publish.yml | 83 +++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 11 deletions(-) diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index 9181d2e..6df185f 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -42,10 +42,13 @@ jobs: # contents:read is required by actions/checkout under GITHUB_TOKEN auth # (works without it on public repos via the unauthenticated-clone # fallback, but that's not a contract we want to rely on). + # actions:read lets the CI-evidence gate query workflow_runs for the + # tagged commit, mirroring release-publish.yml's validate_run_id check. # id-token:write lets npm provenance generate an OIDC-signed Sigstore # attestation linking the published tarball back to this workflow run. permissions: contents: read + actions: read id-token: write defaults: run: @@ -112,17 +115,17 @@ jobs: node-version: "20" registry-url: "https://registry.npmjs.org" - # Gate: refuse to publish unless `tag` corresponds to a real, - # already-published (non-draft) GitHub Release. Without this gate, any - # actor with repo:write could directly workflow_dispatch this workflow - # against any resolvable ref (incl. a lightweight tag at a malicious - # commit), set dry_run=false, and ship that ref to npm @latest with a - # valid Sigstore provenance attestation pointing at the malicious SHA. - # The release-publish.yml chain (with its validate_run_id evidence - # check) is the intended publish trigger; this gate enforces that - # invariant on the downstream workflow too. - # Skipped on dry_run so developers can validate workflow plumbing - # against tags whose Release doesn't exist yet. + # Gate (part 1 of 2) — release existence and publication state. + # Refuse to publish unless `tag` corresponds to a real, already- + # published (non-draft) GitHub Release. This alone is not sufficient: + # a repo:write actor can create a Release via the API/UI without + # going through release-publish.yml, bypassing its validate_run_id + # check. The "Verify CI evidence" step below closes that loop by + # mirroring the reusable workflow's invariant — together the two + # gates assert (a) a Release exists for the tag, and (b) the tagged + # commit has a successful CI run on record. Skipped on dry_run so + # developers can validate workflow plumbing against tags whose + # Release doesn't exist yet. - name: Verify release exists and is published if: steps.version.outputs.DRY_RUN == '' env: @@ -163,6 +166,64 @@ jobs: fi echo "[npm-publish] release $TAG exists, is published, and matches the requested tag — proceeding." + # Gate (part 2 of 2) — CI evidence on the tagged commit. + # release-publish.yml's reusable workflow requires the operator to + # supply `validate_run_id` (a successful CI run whose head_sha is + # the tagged commit). That check enforces "the commit you're about + # to publish actually passed CI." Without an equivalent here, a + # repo:write actor could bypass release-publish.yml entirely: + # + # 1. push tag v9.9.9 at any commit (no CI required to push a tag) + # 2. create a published Release for v9.9.9 via API/UI + # 3. dispatch this workflow + # + # The release-existence gate above passes (a real, non-draft + # Release exists), checkout pulls refs/tags/v9.9.9, and npm publish + # ships the attacker-controlled commit with a valid Sigstore + # provenance attestation. This step closes that path by re-doing + # the validate_run_id check inline: resolve the tag to its commit + # SHA (handling annotated tags) and require at least one successful + # CI workflow run on that SHA. Without a CI pass, refuse. + # + # Skipped on dry_run for the same reason as the release-existence + # gate (developer plumbing tests against tags that may not exist). + - name: Verify CI evidence for the tagged commit + if: steps.version.outputs.DRY_RUN == '' + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ steps.version.outputs.TAG }} + run: | + set -euo pipefail + + # Resolve refs/tags/$TAG → commit SHA. Handle annotated tags + # (which point at a tag object that in turn points at a commit) + # by dereferencing one extra hop. + ref_data=$(gh api "repos/$GITHUB_REPOSITORY/git/ref/tags/$TAG") + ref_type=$(printf '%s' "$ref_data" | jq -r '.object.type') + ref_sha=$(printf '%s' "$ref_data" | jq -r '.object.sha') + if [ "$ref_type" = "tag" ]; then + tag_obj=$(gh api "repos/$GITHUB_REPOSITORY/git/tags/$ref_sha") + target_type=$(printf '%s' "$tag_obj" | jq -r '.object.type') + if [ "$target_type" != "commit" ]; then + echo "::error::Annotated tag $TAG points to '$target_type', not a commit. Refusing to publish." + exit 1 + fi + ref_sha=$(printf '%s' "$tag_obj" | jq -r '.object.sha') + fi + + # Require at least one successful "CI" workflow run on this + # exact commit. Matches release-publish.yml/reusable's + # ci_workflow_name default (`CI`); if that workflow is ever + # renamed, update both. The query filters by head_sha and + # status=success server-side so we don't iterate a long list. + ci_count=$(gh api "repos/$GITHUB_REPOSITORY/actions/runs?head_sha=$ref_sha&status=success&per_page=100" \ + --jq '[.workflow_runs[] | select(.name == "CI")] | length') + if [ "$ci_count" -eq 0 ]; then + echo "::error::No successful CI run found for $TAG (commit $ref_sha). The tagged commit must have passed CI before it can be published to npm. Push the commit through a PR to main, wait for CI to pass, then cut the release via release-publish.yml." + exit 1 + fi + echo "[npm-publish] CI evidence OK: $ci_count successful CI run(s) on commit $ref_sha for tag $TAG." + - name: Set package version env: VERSION: ${{ steps.version.outputs.VERSION }}