From 4f173b667d17d12f3e3eafd678430048b5b77996 Mon Sep 17 00:00:00 2001 From: shanglei Date: Tue, 28 Apr 2026 14:59:24 +0800 Subject: [PATCH 1/5] feat(install): enhance binary URL resolution with environment variable support --- scripts/install.js | 97 ++++++++++++++++++++++-- scripts/install.test.js | 163 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 252 insertions(+), 8 deletions(-) diff --git a/scripts/install.js b/scripts/install.js index 70763ee87..6ebecc725 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -10,15 +10,16 @@ const crypto = require("crypto"); const VERSION = require("../package.json").version.replace(/-.*$/, ""); const REPO = "larksuite/cli"; const NAME = "lark-cli"; +const DEFAULT_MIRROR_HOST = "https://registry.npmmirror.com"; // Allowlist gates the *initial* request URL only. curl --location follows // redirects (capped by --max-redirs 3) without re-checking the target host. // This is acceptable because checksum verification is the primary integrity // control; the allowlist is defense-in-depth to reject obviously wrong URLs. -const ALLOWED_HOSTS = [ +const ALLOWED_HOSTS = new Set([ "github.com", "objects.githubusercontent.com", "registry.npmmirror.com", -]; +]); const PLATFORM_MAP = { darwin: "darwin", @@ -38,14 +39,91 @@ const isWindows = process.platform === "win32"; const ext = isWindows ? ".zip" : ".tar.gz"; const archiveName = `${NAME}-${VERSION}-${platform}-${arch}${ext}`; const GITHUB_URL = `https://github.com/${REPO}/releases/download/v${VERSION}/${archiveName}`; -const MIRROR_URL = `https://registry.npmmirror.com/-/binary/lark-cli/v${VERSION}/${archiveName}`; +const MIRROR_URL = resolveMirrorUrl(process.env, archiveName, VERSION); +// Derived mirror host (from npm_config_registry / LARK_CLI_DOWNLOAD_HOST) is +// allowed at runtime — checksum verification still protects content integrity. +try { + ALLOWED_HOSTS.add(new URL(MIRROR_URL).hostname); +} catch (_) { + // resolveMirrorUrl always returns a valid URL; guard is defensive. +} const binDir = path.join(__dirname, "..", "bin"); const dest = path.join(binDir, NAME + (isWindows ? ".exe" : "")); +// Build the binary mirror URL. Resolution order: +// 1. LARK_CLI_DOWNLOAD_HOST — explicit override for any environment. +// 2. npm_config_registry — honors `npm install --registry=` when +// the corporate mirror proxies npmmirror's +// /-/binary//v/ path. +// 3. registry.npmmirror.com — public China mirror fallback. +// The default public npmjs registry is skipped in step 2 because it does not +// host binaries under /-/binary/... +// +// Both override sources are constrained to HTTPS URLs with a real hostname. +// This prevents `file://` (would let curl read local paths and pass the empty +// hostname through assertAllowedHost), `ftp://`, or `http://` — which would +// silently downgrade transport security for the binary download. +function resolveMirrorUrl(env, archive, version) { + const binaryPath = `/-/binary/lark-cli/v${version}/${archive}`; + + const override = (env.LARK_CLI_DOWNLOAD_HOST || "").trim(); + if (override) { + // User explicitly opted in — fail loudly on bad input rather than fall + // through to a different host than they asked for. + const base = parseDownloadBase(override, "LARK_CLI_DOWNLOAD_HOST"); + return joinUrl(base.origin + base.pathname, binaryPath); + } + + const registry = (env.npm_config_registry || "").trim(); + if (registry && !isDefaultNpmjsRegistry(registry) && isValidDownloadBase(registry)) { + const base = new URL(registry); + return joinUrl(base.origin + base.pathname, binaryPath); + } + + return joinUrl(DEFAULT_MIRROR_HOST, binaryPath); +} + +function joinUrl(base, suffix) { + return base.replace(/\/+$/, "") + suffix; +} + +function parseDownloadBase(raw, source) { + let parsed; + try { + parsed = new URL(raw); + } catch (_) { + throw new Error(`${source} is not a valid URL: ${raw}`); + } + if (parsed.protocol !== "https:" || !parsed.hostname) { + throw new Error( + `${source} must be an https:// URL with a hostname (got ${raw})` + ); + } + return parsed; +} + +function isValidDownloadBase(raw) { + try { + const parsed = new URL(raw); + return parsed.protocol === "https:" && !!parsed.hostname; + } catch (_) { + return false; + } +} + +function isDefaultNpmjsRegistry(url) { + try { + const { hostname } = new URL(url); + return hostname === "registry.npmjs.org"; + } catch (_) { + return false; + } +} + function assertAllowedHost(url) { const { hostname } = new URL(url); - if (!ALLOWED_HOSTS.includes(hostname)) { + if (!ALLOWED_HOSTS.has(hostname)) { throw new Error(`Download host not allowed: ${hostname}`); } } @@ -176,12 +254,17 @@ if (require.main === module) { } catch (err) { console.error(`Failed to install ${NAME}:`, err.message); console.error( - `\nIf you are behind a firewall or in a restricted network, try setting a proxy:\n` + + `\nIf you are behind a firewall or in a restricted network, try one of:\n` + + ` # 1. Use a proxy:\n` + ` export https_proxy=http://your-proxy:port\n` + - ` npm install -g @larksuite/cli` + ` npm install -g @larksuite/cli\n\n` + + ` # 2. Point to a corporate npm mirror that proxies /-/binary/lark-cli/...:\n` + + ` npm install -g @larksuite/cli --registry=https://your-corp-mirror/\n\n` + + ` # 3. Override the binary download host directly:\n` + + ` LARK_CLI_DOWNLOAD_HOST=https://your-host npm install -g @larksuite/cli` ); process.exit(1); } } -module.exports = { getExpectedChecksum, verifyChecksum, assertAllowedHost }; +module.exports = { getExpectedChecksum, verifyChecksum, assertAllowedHost, resolveMirrorUrl }; diff --git a/scripts/install.test.js b/scripts/install.test.js index a0aea9ba2..bb6bb2600 100644 --- a/scripts/install.test.js +++ b/scripts/install.test.js @@ -9,7 +9,7 @@ const os = require("os"); const crypto = require("crypto"); -const { getExpectedChecksum, verifyChecksum, assertAllowedHost } = require("./install.js"); +const { getExpectedChecksum, verifyChecksum, assertAllowedHost, resolveMirrorUrl } = require("./install.js"); describe("getExpectedChecksum", () => { function makeTmpChecksums(content) { @@ -164,3 +164,164 @@ describe("assertAllowedHost", () => { ); }); }); + +describe("resolveMirrorUrl", () => { + const ARCHIVE = "lark-cli-1.0.0-linux-amd64.tar.gz"; + const VERSION = "1.0.0"; + + it("falls back to npmmirror when no env vars are set", () => { + const url = resolveMirrorUrl({}, ARCHIVE, VERSION); + assert.equal( + url, + "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + ); + }); + + it("does not derive from the default npmjs registry", () => { + // The public npmjs registry doesn't host /-/binary//..., so we must + // not point downloads at it. + const url = resolveMirrorUrl( + { npm_config_registry: "https://registry.npmjs.org/" }, + ARCHIVE, + VERSION + ); + assert.equal( + url, + "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + ); + }); + + it("derives the binary URL from a non-default npm_config_registry", () => { + const url = resolveMirrorUrl( + { npm_config_registry: "https://corp.example.com/repository/npm-public/" }, + ARCHIVE, + VERSION + ); + assert.equal( + url, + "https://corp.example.com/repository/npm-public/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + ); + }); + + it("strips trailing slashes from the registry URL", () => { + const url = resolveMirrorUrl( + { npm_config_registry: "https://corp.example.com///" }, + ARCHIVE, + VERSION + ); + assert.equal( + url, + "https://corp.example.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + ); + }); + + it("LARK_CLI_DOWNLOAD_HOST takes precedence over npm_config_registry", () => { + const url = resolveMirrorUrl( + { + LARK_CLI_DOWNLOAD_HOST: "https://override.example.com", + npm_config_registry: "https://corp.example.com/", + }, + ARCHIVE, + VERSION + ); + assert.equal( + url, + "https://override.example.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + ); + }); + + it("ignores empty/whitespace env values", () => { + const url = resolveMirrorUrl( + { LARK_CLI_DOWNLOAD_HOST: " ", npm_config_registry: "" }, + ARCHIVE, + VERSION + ); + assert.equal( + url, + "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + ); + }); + + it("rejects file:// in LARK_CLI_DOWNLOAD_HOST", () => { + assert.throws( + () => resolveMirrorUrl( + { LARK_CLI_DOWNLOAD_HOST: "file:///tmp" }, + ARCHIVE, + VERSION + ), + { message: /LARK_CLI_DOWNLOAD_HOST must be an https:\/\/ URL/ } + ); + }); + + it("rejects ftp:// in LARK_CLI_DOWNLOAD_HOST", () => { + assert.throws( + () => resolveMirrorUrl( + { LARK_CLI_DOWNLOAD_HOST: "ftp://example.com" }, + ARCHIVE, + VERSION + ), + { message: /LARK_CLI_DOWNLOAD_HOST must be an https:\/\/ URL/ } + ); + }); + + it("rejects http:// in LARK_CLI_DOWNLOAD_HOST", () => { + assert.throws( + () => resolveMirrorUrl( + { LARK_CLI_DOWNLOAD_HOST: "http://example.com" }, + ARCHIVE, + VERSION + ), + { message: /LARK_CLI_DOWNLOAD_HOST must be an https:\/\/ URL/ } + ); + }); + + it("rejects https:// with no hostname in LARK_CLI_DOWNLOAD_HOST", () => { + // `https://` without a host is rejected by the URL parser; surface a + // meaningful error instead of a raw TypeError. + assert.throws( + () => resolveMirrorUrl( + { LARK_CLI_DOWNLOAD_HOST: "https://" }, + ARCHIVE, + VERSION + ), + { message: /LARK_CLI_DOWNLOAD_HOST is not a valid URL/ } + ); + }); + + it("rejects garbage in LARK_CLI_DOWNLOAD_HOST", () => { + assert.throws( + () => resolveMirrorUrl( + { LARK_CLI_DOWNLOAD_HOST: "not-a-url" }, + ARCHIVE, + VERSION + ), + { message: /LARK_CLI_DOWNLOAD_HOST is not a valid URL/ } + ); + }); + + it("silently falls back when npm_config_registry is non-https", () => { + // Implicit feature: don't break installs whose npm registry is plain http. + // The user didn't opt into binary-mirror behavior, so just use the default. + const url = resolveMirrorUrl( + { npm_config_registry: "http://internal.example.com/" }, + ARCHIVE, + VERSION + ); + assert.equal( + url, + "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + ); + }); + + it("silently falls back when npm_config_registry is file://", () => { + const url = resolveMirrorUrl( + { npm_config_registry: "file:///tmp" }, + ARCHIVE, + VERSION + ); + assert.equal( + url, + "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + ); + }); +}); From 8fbaacee90464c3523b4b47d02e4bf7d7b8e422f Mon Sep 17 00:00:00 2001 From: shanglei Date: Tue, 28 Apr 2026 19:38:59 +0800 Subject: [PATCH 2/5] fix(install): defer mirror resolution into install() to surface friendly errors resolveMirrorUrl was called at module scope, so an invalid LARK_CLI_DOWNLOAD_HOST (e.g. file://) threw before the try/catch in the postinstall entrypoint, dumping a raw stack trace instead of the recovery guidance with proxy/registry/host-override options. Move resolution into install() via getMirrorUrl() so the throw is caught and the user sees the actionable help text. --- scripts/install.js | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/scripts/install.js b/scripts/install.js index 6ebecc725..02f9b909b 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -39,14 +39,6 @@ const isWindows = process.platform === "win32"; const ext = isWindows ? ".zip" : ".tar.gz"; const archiveName = `${NAME}-${VERSION}-${platform}-${arch}${ext}`; const GITHUB_URL = `https://github.com/${REPO}/releases/download/v${VERSION}/${archiveName}`; -const MIRROR_URL = resolveMirrorUrl(process.env, archiveName, VERSION); -// Derived mirror host (from npm_config_registry / LARK_CLI_DOWNLOAD_HOST) is -// allowed at runtime — checksum verification still protects content integrity. -try { - ALLOWED_HOSTS.add(new URL(MIRROR_URL).hostname); -} catch (_) { - // resolveMirrorUrl always returns a valid URL; guard is defensive. -} const binDir = path.join(__dirname, "..", "bin"); const dest = path.join(binDir, NAME + (isWindows ? ".exe" : "")); @@ -128,6 +120,16 @@ function assertAllowedHost(url) { } } +// Resolve the mirror URL and admit its host. Called from install() so that +// resolution errors (e.g. invalid LARK_CLI_DOWNLOAD_HOST) propagate into the +// guarded try/catch and surface the recovery guidance instead of a raw +// module-load stack trace. +function getMirrorUrl(env) { + const mirrorUrl = resolveMirrorUrl(env, archiveName, VERSION); + ALLOWED_HOSTS.add(new URL(mirrorUrl).hostname); + return mirrorUrl; +} + function download(url, destPath) { assertAllowedHost(url); const args = [ @@ -144,6 +146,11 @@ function download(url, destPath) { } function install() { + // Resolve the mirror URL up front so a bad LARK_CLI_DOWNLOAD_HOST throws + // here (inside the guarded path) and gets the friendly error help below, + // not a raw module-load stack trace. + const mirrorUrl = getMirrorUrl(process.env); + fs.mkdirSync(binDir, { recursive: true }); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "lark-cli-")); @@ -153,7 +160,7 @@ function install() { try { download(GITHUB_URL, archivePath); } catch (err) { - download(MIRROR_URL, archivePath); + download(mirrorUrl, archivePath); } const expectedHash = getExpectedChecksum(archiveName); From 93e6081f7397861ddba9199be17705e405322ee0 Mon Sep 17 00:00:00 2001 From: shanglei Date: Tue, 28 Apr 2026 20:06:42 +0800 Subject: [PATCH 3/5] fix(install): keep npmmirror fallback when npm_config_registry is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveMirrorUrl returned a single URL, so any non-default npm_config_registry replaced the npmmirror fallback entirely. Corporate npm proxies (Verdaccio, Artifactory, Nexus) often only serve npm package metadata and don't host /-/binary//..., turning previously-working installs into 404s when GitHub is unreachable. Switch to resolveMirrorUrls returning an ordered chain: - LARK_CLI_DOWNLOAD_HOST set → [override] only (explicit user choice; no silent leak to npmmirror). - Otherwise → [derived_from_registry?, npmmirror_default]; npmmirror is always the final entry, restoring the pre-PR safety net. install() now walks [GITHUB_URL, ...mirrorUrls] and stops at the first success. --- scripts/install.js | 77 ++++++++++++------- scripts/install.test.js | 165 +++++++++++++++++++++++----------------- 2 files changed, 145 insertions(+), 97 deletions(-) diff --git a/scripts/install.js b/scripts/install.js index 02f9b909b..91030ac94 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -43,37 +43,49 @@ const GITHUB_URL = `https://github.com/${REPO}/releases/download/v${VERSION}/${a const binDir = path.join(__dirname, "..", "bin"); const dest = path.join(binDir, NAME + (isWindows ? ".exe" : "")); -// Build the binary mirror URL. Resolution order: -// 1. LARK_CLI_DOWNLOAD_HOST — explicit override for any environment. -// 2. npm_config_registry — honors `npm install --registry=` when -// the corporate mirror proxies npmmirror's -// /-/binary//v/ path. -// 3. registry.npmmirror.com — public China mirror fallback. +// Build the ordered list of binary mirror URLs to try. Resolution rules: +// 1. LARK_CLI_DOWNLOAD_HOST — explicit override; returned as the SOLE +// URL so we never silently fall through to +// a host the user did not authorize. +// 2. npm_config_registry — when the user has set a non-default +// registry (npmmirror clone, corp Verdaccio, +// Artifactory, …), include the derived path +// first. Many of these proxies don't actually +// host /-/binary//..., so we ALWAYS +// append the public npmmirror as a final +// fallback so the install does not regress +// from the previous behavior of "GitHub → +// npmmirror". +// 3. registry.npmmirror.com — public China mirror, always tried last +// unless an explicit override was set. // The default public npmjs registry is skipped in step 2 because it does not // host binaries under /-/binary/... // -// Both override sources are constrained to HTTPS URLs with a real hostname. +// LARK_CLI_DOWNLOAD_HOST is constrained to HTTPS URLs with a real hostname. // This prevents `file://` (would let curl read local paths and pass the empty // hostname through assertAllowedHost), `ftp://`, or `http://` — which would // silently downgrade transport security for the binary download. -function resolveMirrorUrl(env, archive, version) { +function resolveMirrorUrls(env, archive, version) { const binaryPath = `/-/binary/lark-cli/v${version}/${archive}`; + const defaultUrl = joinUrl(DEFAULT_MIRROR_HOST, binaryPath); const override = (env.LARK_CLI_DOWNLOAD_HOST || "").trim(); if (override) { // User explicitly opted in — fail loudly on bad input rather than fall - // through to a different host than they asked for. + // through to a different host than they asked for. No additional fallback + // either; the user picked this host on purpose. const base = parseDownloadBase(override, "LARK_CLI_DOWNLOAD_HOST"); - return joinUrl(base.origin + base.pathname, binaryPath); + return [joinUrl(base.origin + base.pathname, binaryPath)]; } + const urls = []; const registry = (env.npm_config_registry || "").trim(); if (registry && !isDefaultNpmjsRegistry(registry) && isValidDownloadBase(registry)) { const base = new URL(registry); - return joinUrl(base.origin + base.pathname, binaryPath); + urls.push(joinUrl(base.origin + base.pathname, binaryPath)); } - - return joinUrl(DEFAULT_MIRROR_HOST, binaryPath); + if (!urls.includes(defaultUrl)) urls.push(defaultUrl); + return urls; } function joinUrl(base, suffix) { @@ -120,14 +132,14 @@ function assertAllowedHost(url) { } } -// Resolve the mirror URL and admit its host. Called from install() so that +// Resolve the mirror URL chain and admit each host. Called from install() so // resolution errors (e.g. invalid LARK_CLI_DOWNLOAD_HOST) propagate into the // guarded try/catch and surface the recovery guidance instead of a raw // module-load stack trace. -function getMirrorUrl(env) { - const mirrorUrl = resolveMirrorUrl(env, archiveName, VERSION); - ALLOWED_HOSTS.add(new URL(mirrorUrl).hostname); - return mirrorUrl; +function getMirrorUrls(env) { + const urls = resolveMirrorUrls(env, archiveName, VERSION); + for (const u of urls) ALLOWED_HOSTS.add(new URL(u).hostname); + return urls; } function download(url, destPath) { @@ -146,10 +158,10 @@ function download(url, destPath) { } function install() { - // Resolve the mirror URL up front so a bad LARK_CLI_DOWNLOAD_HOST throws - // here (inside the guarded path) and gets the friendly error help below, - // not a raw module-load stack trace. - const mirrorUrl = getMirrorUrl(process.env); + // Resolve the mirror URL chain up front so a bad LARK_CLI_DOWNLOAD_HOST + // throws here (inside the guarded path) and gets the friendly error help + // below, not a raw module-load stack trace. + const mirrorUrls = getMirrorUrls(process.env); fs.mkdirSync(binDir, { recursive: true }); @@ -157,11 +169,22 @@ function install() { const archivePath = path.join(tmpDir, archiveName); try { - try { - download(GITHUB_URL, archivePath); - } catch (err) { - download(mirrorUrl, archivePath); + // Try GitHub first, then walk the mirror chain in order. Stop at the + // first success. This preserves the "GitHub → npmmirror" safety net + // even when an unrelated npm_config_registry was set globally and its + // /-/binary/ path doesn't actually serve our archive. + let lastErr; + let downloaded = false; + for (const url of [GITHUB_URL, ...mirrorUrls]) { + try { + download(url, archivePath); + downloaded = true; + break; + } catch (e) { + lastErr = e; + } } + if (!downloaded) throw lastErr; const expectedHash = getExpectedChecksum(archiveName); verifyChecksum(archivePath, expectedHash); @@ -274,4 +297,4 @@ if (require.main === module) { } } -module.exports = { getExpectedChecksum, verifyChecksum, assertAllowedHost, resolveMirrorUrl }; +module.exports = { getExpectedChecksum, verifyChecksum, assertAllowedHost, resolveMirrorUrls }; diff --git a/scripts/install.test.js b/scripts/install.test.js index bb6bb2600..62479133e 100644 --- a/scripts/install.test.js +++ b/scripts/install.test.js @@ -9,7 +9,7 @@ const os = require("os"); const crypto = require("crypto"); -const { getExpectedChecksum, verifyChecksum, assertAllowedHost, resolveMirrorUrl } = require("./install.js"); +const { getExpectedChecksum, verifyChecksum, assertAllowedHost, resolveMirrorUrls } = require("./install.js"); describe("getExpectedChecksum", () => { function makeTmpChecksums(content) { @@ -165,86 +165,113 @@ describe("assertAllowedHost", () => { }); }); -describe("resolveMirrorUrl", () => { +describe("resolveMirrorUrls", () => { const ARCHIVE = "lark-cli-1.0.0-linux-amd64.tar.gz"; const VERSION = "1.0.0"; + const DEFAULT = "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz"; - it("falls back to npmmirror when no env vars are set", () => { - const url = resolveMirrorUrl({}, ARCHIVE, VERSION); - assert.equal( - url, - "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" - ); + it("returns only the default mirror when no env vars are set", () => { + assert.deepEqual(resolveMirrorUrls({}, ARCHIVE, VERSION), [DEFAULT]); }); it("does not derive from the default npmjs registry", () => { // The public npmjs registry doesn't host /-/binary//..., so we must // not point downloads at it. - const url = resolveMirrorUrl( - { npm_config_registry: "https://registry.npmjs.org/" }, - ARCHIVE, - VERSION + assert.deepEqual( + resolveMirrorUrls( + { npm_config_registry: "https://registry.npmjs.org/" }, + ARCHIVE, + VERSION + ), + [DEFAULT] ); - assert.equal( - url, - "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + }); + + it("derives from non-default npm_config_registry AND keeps default as fallback", () => { + // Critical: a corporate npm proxy (Verdaccio/Artifactory/Nexus) often + // doesn't actually serve /-/binary//..., so we must keep the + // public npmmirror as a final fallback or installs regress vs. the + // pre-PR "GitHub → npmmirror" behavior. + assert.deepEqual( + resolveMirrorUrls( + { npm_config_registry: "https://corp.example.com/repository/npm-public/" }, + ARCHIVE, + VERSION + ), + [ + "https://corp.example.com/repository/npm-public/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz", + DEFAULT, + ] ); }); - it("derives the binary URL from a non-default npm_config_registry", () => { - const url = resolveMirrorUrl( - { npm_config_registry: "https://corp.example.com/repository/npm-public/" }, + it("derived URL appears before the default in the chain", () => { + const urls = resolveMirrorUrls( + { npm_config_registry: "https://corp.example.com/" }, ARCHIVE, VERSION ); - assert.equal( - url, - "https://corp.example.com/repository/npm-public/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + assert.equal(urls.length, 2); + assert.match(urls[0], /^https:\/\/corp\.example\.com\//); + assert.equal(urls[1], DEFAULT); + }); + + it("does not duplicate the default if the registry already points at it", () => { + // If npm_config_registry happens to be the public npmmirror, we still + // want a single entry, not two identical ones. + assert.deepEqual( + resolveMirrorUrls( + { npm_config_registry: "https://registry.npmmirror.com/" }, + ARCHIVE, + VERSION + ), + [DEFAULT] ); }); it("strips trailing slashes from the registry URL", () => { - const url = resolveMirrorUrl( - { npm_config_registry: "https://corp.example.com///" }, - ARCHIVE, - VERSION - ); - assert.equal( - url, - "https://corp.example.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + assert.deepEqual( + resolveMirrorUrls( + { npm_config_registry: "https://corp.example.com///" }, + ARCHIVE, + VERSION + ), + [ + "https://corp.example.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz", + DEFAULT, + ] ); }); - it("LARK_CLI_DOWNLOAD_HOST takes precedence over npm_config_registry", () => { - const url = resolveMirrorUrl( - { - LARK_CLI_DOWNLOAD_HOST: "https://override.example.com", - npm_config_registry: "https://corp.example.com/", - }, - ARCHIVE, - VERSION - ); - assert.equal( - url, - "https://override.example.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + it("LARK_CLI_DOWNLOAD_HOST is the SOLE entry — no fallback", () => { + // Explicit user choice: do not silently leak to npmmirror. + assert.deepEqual( + resolveMirrorUrls( + { + LARK_CLI_DOWNLOAD_HOST: "https://override.example.com", + npm_config_registry: "https://corp.example.com/", + }, + ARCHIVE, + VERSION + ), + ["https://override.example.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz"] ); }); it("ignores empty/whitespace env values", () => { - const url = resolveMirrorUrl( - { LARK_CLI_DOWNLOAD_HOST: " ", npm_config_registry: "" }, - ARCHIVE, - VERSION - ); - assert.equal( - url, - "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + assert.deepEqual( + resolveMirrorUrls( + { LARK_CLI_DOWNLOAD_HOST: " ", npm_config_registry: "" }, + ARCHIVE, + VERSION + ), + [DEFAULT] ); }); it("rejects file:// in LARK_CLI_DOWNLOAD_HOST", () => { assert.throws( - () => resolveMirrorUrl( + () => resolveMirrorUrls( { LARK_CLI_DOWNLOAD_HOST: "file:///tmp" }, ARCHIVE, VERSION @@ -255,7 +282,7 @@ describe("resolveMirrorUrl", () => { it("rejects ftp:// in LARK_CLI_DOWNLOAD_HOST", () => { assert.throws( - () => resolveMirrorUrl( + () => resolveMirrorUrls( { LARK_CLI_DOWNLOAD_HOST: "ftp://example.com" }, ARCHIVE, VERSION @@ -266,7 +293,7 @@ describe("resolveMirrorUrl", () => { it("rejects http:// in LARK_CLI_DOWNLOAD_HOST", () => { assert.throws( - () => resolveMirrorUrl( + () => resolveMirrorUrls( { LARK_CLI_DOWNLOAD_HOST: "http://example.com" }, ARCHIVE, VERSION @@ -279,7 +306,7 @@ describe("resolveMirrorUrl", () => { // `https://` without a host is rejected by the URL parser; surface a // meaningful error instead of a raw TypeError. assert.throws( - () => resolveMirrorUrl( + () => resolveMirrorUrls( { LARK_CLI_DOWNLOAD_HOST: "https://" }, ARCHIVE, VERSION @@ -290,7 +317,7 @@ describe("resolveMirrorUrl", () => { it("rejects garbage in LARK_CLI_DOWNLOAD_HOST", () => { assert.throws( - () => resolveMirrorUrl( + () => resolveMirrorUrls( { LARK_CLI_DOWNLOAD_HOST: "not-a-url" }, ARCHIVE, VERSION @@ -302,26 +329,24 @@ describe("resolveMirrorUrl", () => { it("silently falls back when npm_config_registry is non-https", () => { // Implicit feature: don't break installs whose npm registry is plain http. // The user didn't opt into binary-mirror behavior, so just use the default. - const url = resolveMirrorUrl( - { npm_config_registry: "http://internal.example.com/" }, - ARCHIVE, - VERSION - ); - assert.equal( - url, - "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + assert.deepEqual( + resolveMirrorUrls( + { npm_config_registry: "http://internal.example.com/" }, + ARCHIVE, + VERSION + ), + [DEFAULT] ); }); it("silently falls back when npm_config_registry is file://", () => { - const url = resolveMirrorUrl( - { npm_config_registry: "file:///tmp" }, - ARCHIVE, - VERSION - ); - assert.equal( - url, - "https://registry.npmmirror.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz" + assert.deepEqual( + resolveMirrorUrls( + { npm_config_registry: "file:///tmp" }, + ARCHIVE, + VERSION + ), + [DEFAULT] ); }); }); From ebbcef8e7172b3c3c1478fc87e797e47a651853a Mon Sep 17 00:00:00 2001 From: shanglei Date: Tue, 28 Apr 2026 20:22:25 +0800 Subject: [PATCH 4/5] fix(install): skip GitHub when LARK_CLI_DOWNLOAD_HOST is set The download loop unconditionally tried GITHUB_URL first, even when the user explicitly named a download host. In locked-down networks, probing github.com can trigger DLP / firewall alerts and contradicts the explicit-override semantics ("use only this host, nothing else"). When LARK_CLI_DOWNLOAD_HOST is set, the chain is now just [override]. When it isn't, behavior is unchanged: [GITHUB_URL, derived?, npmmirror]. --- scripts/install.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/scripts/install.js b/scripts/install.js index 91030ac94..ee0037624 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -163,19 +163,26 @@ function install() { // below, not a raw module-load stack trace. const mirrorUrls = getMirrorUrls(process.env); + // When the user explicitly sets LARK_CLI_DOWNLOAD_HOST, skip the GitHub + // attempt entirely. The user has named the only host they want hit; + // probing github.com first can trigger DLP / firewall alerts and violates + // the explicit-override semantics documented above. + const hasExplicitHost = !!(process.env.LARK_CLI_DOWNLOAD_HOST || "").trim(); + const downloadUrls = hasExplicitHost ? mirrorUrls : [GITHUB_URL, ...mirrorUrls]; + fs.mkdirSync(binDir, { recursive: true }); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "lark-cli-")); const archivePath = path.join(tmpDir, archiveName); try { - // Try GitHub first, then walk the mirror chain in order. Stop at the - // first success. This preserves the "GitHub → npmmirror" safety net - // even when an unrelated npm_config_registry was set globally and its - // /-/binary/ path doesn't actually serve our archive. + // Walk the chain in order; stop at the first success. The default chain + // is GitHub → derived(npm_config_registry)? → npmmirror, preserving the + // pre-PR safety net when a corporate proxy doesn't actually host + // /-/binary//... let lastErr; let downloaded = false; - for (const url of [GITHUB_URL, ...mirrorUrls]) { + for (const url of downloadUrls) { try { download(url, archivePath); downloaded = true; From 6267400d2b046d7d0dc299ae5ce4d17c848cad29 Mon Sep 17 00:00:00 2001 From: shanglei Date: Wed, 29 Apr 2026 15:07:38 +0800 Subject: [PATCH 5/5] refactor(install): drop LARK_CLI_DOWNLOAD_HOST env override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #640 only asked for --registry to influence the binary download. The LARK_CLI_DOWNLOAD_HOST escape hatch was added speculatively for locked-down networks but is YAGNI — users in those environments already have npm-level mirrors (--registry) or proxy controls (https_proxy). Removing it shrinks the surface area: - delete parseDownloadBase() and its strict https-only validation - drop the install() branch that skipped GitHub on explicit override - simplify failure-help message to two recovery options Resolution chain becomes [GITHUB, derived_from_npm_config_registry?, npmmirror_default]. The npmmirror tail still preserves the pre-PR safety net when a corp registry doesn't actually serve /-/binary//... End-to-end verified on Linux + Windows via real `npm install -g `: all four user scenarios pass, with the issue #640 path (--registry= npmmirror + GitHub blocked) finishing in 2s on Linux / 6s on Windows. --- scripts/install.js | 67 +++++++----------------------------- scripts/install.test.js | 76 ++--------------------------------------- 2 files changed, 14 insertions(+), 129 deletions(-) diff --git a/scripts/install.js b/scripts/install.js index ee0037624..334804170 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -44,10 +44,7 @@ const binDir = path.join(__dirname, "..", "bin"); const dest = path.join(binDir, NAME + (isWindows ? ".exe" : "")); // Build the ordered list of binary mirror URLs to try. Resolution rules: -// 1. LARK_CLI_DOWNLOAD_HOST — explicit override; returned as the SOLE -// URL so we never silently fall through to -// a host the user did not authorize. -// 2. npm_config_registry — when the user has set a non-default +// 1. npm_config_registry — when the user has set a non-default // registry (npmmirror clone, corp Verdaccio, // Artifactory, …), include the derived path // first. Many of these proxies don't actually @@ -56,28 +53,16 @@ const dest = path.join(binDir, NAME + (isWindows ? ".exe" : "")); // fallback so the install does not regress // from the previous behavior of "GitHub → // npmmirror". -// 3. registry.npmmirror.com — public China mirror, always tried last -// unless an explicit override was set. -// The default public npmjs registry is skipped in step 2 because it does not +// 2. registry.npmmirror.com — public China mirror, always tried last. +// The default public npmjs registry is skipped in step 1 because it does not // host binaries under /-/binary/... // -// LARK_CLI_DOWNLOAD_HOST is constrained to HTTPS URLs with a real hostname. -// This prevents `file://` (would let curl read local paths and pass the empty -// hostname through assertAllowedHost), `ftp://`, or `http://` — which would -// silently downgrade transport security for the binary download. +// Non-https / malformed npm_config_registry is silently ignored so npm users +// with http-only internal registries don't have their installs broken. function resolveMirrorUrls(env, archive, version) { const binaryPath = `/-/binary/lark-cli/v${version}/${archive}`; const defaultUrl = joinUrl(DEFAULT_MIRROR_HOST, binaryPath); - const override = (env.LARK_CLI_DOWNLOAD_HOST || "").trim(); - if (override) { - // User explicitly opted in — fail loudly on bad input rather than fall - // through to a different host than they asked for. No additional fallback - // either; the user picked this host on purpose. - const base = parseDownloadBase(override, "LARK_CLI_DOWNLOAD_HOST"); - return [joinUrl(base.origin + base.pathname, binaryPath)]; - } - const urls = []; const registry = (env.npm_config_registry || "").trim(); if (registry && !isDefaultNpmjsRegistry(registry) && isValidDownloadBase(registry)) { @@ -92,21 +77,6 @@ function joinUrl(base, suffix) { return base.replace(/\/+$/, "") + suffix; } -function parseDownloadBase(raw, source) { - let parsed; - try { - parsed = new URL(raw); - } catch (_) { - throw new Error(`${source} is not a valid URL: ${raw}`); - } - if (parsed.protocol !== "https:" || !parsed.hostname) { - throw new Error( - `${source} must be an https:// URL with a hostname (got ${raw})` - ); - } - return parsed; -} - function isValidDownloadBase(raw) { try { const parsed = new URL(raw); @@ -133,9 +103,7 @@ function assertAllowedHost(url) { } // Resolve the mirror URL chain and admit each host. Called from install() so -// resolution errors (e.g. invalid LARK_CLI_DOWNLOAD_HOST) propagate into the -// guarded try/catch and surface the recovery guidance instead of a raw -// module-load stack trace. +// derived hosts only become trusted when actually needed. function getMirrorUrls(env) { const urls = resolveMirrorUrls(env, archiveName, VERSION); for (const u of urls) ALLOWED_HOSTS.add(new URL(u).hostname); @@ -158,17 +126,8 @@ function download(url, destPath) { } function install() { - // Resolve the mirror URL chain up front so a bad LARK_CLI_DOWNLOAD_HOST - // throws here (inside the guarded path) and gets the friendly error help - // below, not a raw module-load stack trace. const mirrorUrls = getMirrorUrls(process.env); - - // When the user explicitly sets LARK_CLI_DOWNLOAD_HOST, skip the GitHub - // attempt entirely. The user has named the only host they want hit; - // probing github.com first can trigger DLP / firewall alerts and violates - // the explicit-override semantics documented above. - const hasExplicitHost = !!(process.env.LARK_CLI_DOWNLOAD_HOST || "").trim(); - const downloadUrls = hasExplicitHost ? mirrorUrls : [GITHUB_URL, ...mirrorUrls]; + const downloadUrls = [GITHUB_URL, ...mirrorUrls]; fs.mkdirSync(binDir, { recursive: true }); @@ -176,10 +135,10 @@ function install() { const archivePath = path.join(tmpDir, archiveName); try { - // Walk the chain in order; stop at the first success. The default chain - // is GitHub → derived(npm_config_registry)? → npmmirror, preserving the - // pre-PR safety net when a corporate proxy doesn't actually host - // /-/binary//... + // Walk the chain in order; stop at the first success. Default chain: + // GitHub → derived(npm_config_registry)? → npmmirror. The npmmirror + // tail preserves the pre-PR safety net when a corporate proxy doesn't + // actually host /-/binary//... let lastErr; let downloaded = false; for (const url of downloadUrls) { @@ -296,9 +255,7 @@ if (require.main === module) { ` export https_proxy=http://your-proxy:port\n` + ` npm install -g @larksuite/cli\n\n` + ` # 2. Point to a corporate npm mirror that proxies /-/binary/lark-cli/...:\n` + - ` npm install -g @larksuite/cli --registry=https://your-corp-mirror/\n\n` + - ` # 3. Override the binary download host directly:\n` + - ` LARK_CLI_DOWNLOAD_HOST=https://your-host npm install -g @larksuite/cli` + ` npm install -g @larksuite/cli --registry=https://your-corp-mirror/` ); process.exit(1); } diff --git a/scripts/install.test.js b/scripts/install.test.js index 62479133e..664cd2337 100644 --- a/scripts/install.test.js +++ b/scripts/install.test.js @@ -243,25 +243,10 @@ describe("resolveMirrorUrls", () => { ); }); - it("LARK_CLI_DOWNLOAD_HOST is the SOLE entry — no fallback", () => { - // Explicit user choice: do not silently leak to npmmirror. + it("ignores empty/whitespace npm_config_registry", () => { assert.deepEqual( resolveMirrorUrls( - { - LARK_CLI_DOWNLOAD_HOST: "https://override.example.com", - npm_config_registry: "https://corp.example.com/", - }, - ARCHIVE, - VERSION - ), - ["https://override.example.com/-/binary/lark-cli/v1.0.0/lark-cli-1.0.0-linux-amd64.tar.gz"] - ); - }); - - it("ignores empty/whitespace env values", () => { - assert.deepEqual( - resolveMirrorUrls( - { LARK_CLI_DOWNLOAD_HOST: " ", npm_config_registry: "" }, + { npm_config_registry: "" }, ARCHIVE, VERSION ), @@ -269,63 +254,6 @@ describe("resolveMirrorUrls", () => { ); }); - it("rejects file:// in LARK_CLI_DOWNLOAD_HOST", () => { - assert.throws( - () => resolveMirrorUrls( - { LARK_CLI_DOWNLOAD_HOST: "file:///tmp" }, - ARCHIVE, - VERSION - ), - { message: /LARK_CLI_DOWNLOAD_HOST must be an https:\/\/ URL/ } - ); - }); - - it("rejects ftp:// in LARK_CLI_DOWNLOAD_HOST", () => { - assert.throws( - () => resolveMirrorUrls( - { LARK_CLI_DOWNLOAD_HOST: "ftp://example.com" }, - ARCHIVE, - VERSION - ), - { message: /LARK_CLI_DOWNLOAD_HOST must be an https:\/\/ URL/ } - ); - }); - - it("rejects http:// in LARK_CLI_DOWNLOAD_HOST", () => { - assert.throws( - () => resolveMirrorUrls( - { LARK_CLI_DOWNLOAD_HOST: "http://example.com" }, - ARCHIVE, - VERSION - ), - { message: /LARK_CLI_DOWNLOAD_HOST must be an https:\/\/ URL/ } - ); - }); - - it("rejects https:// with no hostname in LARK_CLI_DOWNLOAD_HOST", () => { - // `https://` without a host is rejected by the URL parser; surface a - // meaningful error instead of a raw TypeError. - assert.throws( - () => resolveMirrorUrls( - { LARK_CLI_DOWNLOAD_HOST: "https://" }, - ARCHIVE, - VERSION - ), - { message: /LARK_CLI_DOWNLOAD_HOST is not a valid URL/ } - ); - }); - - it("rejects garbage in LARK_CLI_DOWNLOAD_HOST", () => { - assert.throws( - () => resolveMirrorUrls( - { LARK_CLI_DOWNLOAD_HOST: "not-a-url" }, - ARCHIVE, - VERSION - ), - { message: /LARK_CLI_DOWNLOAD_HOST is not a valid URL/ } - ); - }); - it("silently falls back when npm_config_registry is non-https", () => { // Implicit feature: don't break installs whose npm registry is plain http. // The user didn't opt into binary-mirror behavior, so just use the default.