diff --git a/packages/cli/src/commands/maintenance.ts b/packages/cli/src/commands/maintenance.ts index 0b04980c97..21f2d9eea6 100644 --- a/packages/cli/src/commands/maintenance.ts +++ b/packages/cli/src/commands/maintenance.ts @@ -37,6 +37,9 @@ import { batchEntityQuads } from '../batching.js'; import { runDaemon, checkForNpmVersionUpdate, + resolveLatestNpmVersion, + isValidSemver, + isPrerelease, performNpmUpdate, performNpmUpdateEdge, getCurrentCliVersion, @@ -103,6 +106,51 @@ import { runForegroundSupervisor, } from '../cli-supervisor.js'; +/** + * Decide whether an EXPLICIT `dkg update ` is allowed under the node's + * stable-only policy. Resolves a dist-tag to its concrete version first, then: + * - allowPre (config or --allow-prerelease) → always ok + * - prerelease target under stable-only → refuse + * - dist-tag that can't be resolved (registry error) → refuse (fail CLOSED, + * matching the no-arg path; otherwise a transient blip could let a + * stable-only node install an RC that `latest` happens to point at) + * Extracted + dep-injected so it is unit-testable. Returns ok, or a reason. + */ +export async function resolveExplicitUpdateTarget( + target: string, + allowPre: boolean, + deps: { + resolveLatestNpmVersion: ( + log: (m: string) => void, + allowPrerelease: boolean, + channel?: string, + ) => Promise<{ version: string | null; error?: boolean }>; + log: (m: string) => void; + }, +): Promise<{ ok: true } | { ok: false; reason: string }> { + if (allowPre) return { ok: true }; + let resolved = target; + if (!isValidSemver(resolved)) { + // Non-semver target → treat as a dist-tag and resolve it (allowPrerelease=true + // so a prerelease target is visible to be rejected). + const r = await deps.resolveLatestNpmVersion(deps.log, true, resolved); + if (r.error) { + return { + ok: false, + reason: `could not resolve dist-tag "${target}" against the npm registry (transient error). On a stable-only node a dist-tag must be confirmed non-prerelease before install — retry, pass an explicit version, or use --allow-prerelease`, + }; + } + if (r.version) resolved = r.version; + } + if (isValidSemver(resolved) && isPrerelease(resolved)) { + return { + ok: false, + reason: `target "${resolved}" is a pre-release and this node has allowPrerelease=false — re-run with --allow-prerelease to override`, + }; + } + return { ok: true }; +} + export function registerMaintenanceCommands(program: Command): void { // ─── dkg update ────────────────────────────────────────────────────── // @@ -197,6 +245,19 @@ program if (version) { version = version.replace(/^refs\/tags\/v?/, '').replace(/^v/, ''); } + // An EXPLICIT target (version or dist-tag) must still honour the + // stable-only policy: `dkg update 10.1.0-rc.1` or `dkg update latest` + // (when latest points at an RC) must NOT bypass allowPrerelease:false. + if (version) { + const gate = await resolveExplicitUpdateTarget(version, allowPre, { + resolveLatestNpmVersion, + log: logFn, + }); + if (!gate.ok) { + console.error(`[dkg update] Refusing to update: ${gate.reason}.`); + process.exit(1); + } + } if (!version) { console.log('Checking NPM registry for updates...'); const check = await checkForNpmVersionUpdate(logFn, allowPre, au.channel); diff --git a/packages/cli/src/daemon/auto-update.ts b/packages/cli/src/daemon/auto-update.ts index 9072d02f46..0b9692578e 100644 --- a/packages/cli/src/daemon/auto-update.ts +++ b/packages/cli/src/daemon/auto-update.ts @@ -372,9 +372,14 @@ export async function resolveLatestNpmVersion( return { version: null, error: false }; } - const candidates = [stable, tags.dev, tags.beta, tags.next].filter( + // Filter to VALID semver BEFORE sorting: a single malformed tag (e.g. + // latest="garbage") must not sort ahead of and mask a valid candidate + // (e.g. beta="9.0.0-beta.4") — compareSemver on garbage returns NaN, which + // makes the sort order undefined. Filtering invalid values never changes + // selection among valid ones. + const candidates = ([stable, tags.dev, tags.beta, tags.next].filter( Boolean, - ) as string[]; + ) as string[]).filter(isValidSemver); if (candidates.length === 0) return { version: null, error: false }; candidates.sort((a, b) => compareSemver(b, a)); return { version: candidates[0] }; @@ -387,19 +392,23 @@ export async function resolveLatestNpmVersion( } export function compareSemver(a: string, b: string): number { - const pa = a.replace(/^v/, "").split(/[-+]/)[0].split(".").map(Number); - const pb = b.replace(/^v/, "").split(/[-+]/)[0].split(".").map(Number); + // Build metadata (everything from `+`) is ignored for precedence (semver §10), + // so strip it FIRST. Detecting the prerelease via the raw string's `-` is + // wrong: `10.0.0+mainnet-build.1` is a STABLE version whose hyphen lives in + // build metadata — treating it as a prerelease made it sort BELOW + // `10.0.0-rc.19` and read as "not newer". + const core = (v: string) => v.replace(/^v/, "").split("+")[0]; + const ca = core(a); + const cb = core(b); + const pa = ca.split("-")[0].split(".").map(Number); + const pb = cb.split("-")[0].split(".").map(Number); for (let i = 0; i < 3; i++) { if ((pa[i] ?? 0) !== (pb[i] ?? 0)) return (pa[i] ?? 0) - (pb[i] ?? 0); } - const stripBuild = (s: string) => s.replace(/\+.*$/, ""); - const preA = a.includes("-") - ? stripBuild(a.split("-").slice(1).join("-")) - : ""; - const preB = b.includes("-") - ? stripBuild(b.split("-").slice(1).join("-")) - : ""; - if (!preA && preB) return 1; + const prerelease = (v: string) => (v.includes("-") ? v.slice(v.indexOf("-") + 1) : ""); + const preA = prerelease(ca); + const preB = prerelease(cb); + if (!preA && preB) return 1; // stable outranks its own prerelease if (preA && !preB) return -1; return preA.localeCompare(preB, undefined, { numeric: true }); } diff --git a/packages/cli/src/daemon/lifecycle.ts b/packages/cli/src/daemon/lifecycle.ts index 7b2b2b2f38..c2515b73b7 100644 --- a/packages/cli/src/daemon/lifecycle.ts +++ b/packages/cli/src/daemon/lifecycle.ts @@ -2155,6 +2155,10 @@ export async function runDaemonInner( if (!autoUpdateEnabled) return "disabled"; if (daemonState.isUpdating) return "updating"; if (daemonState.lastUpdateCheck.checkedAt === 0) return "unknown"; + // A pinned channel with no acceptable target is NOT healthy "latest" — + // surface it distinctly so central monitoring can flag an unpublished / + // rejected channel instead of seeing a falsely-current node. + if (daemonState.lastUpdateCheck.channelTargetMissing) return "channel-missing"; return daemonState.lastUpdateCheck.upToDate ? "latest" : "behind"; }, }); diff --git a/packages/cli/test/auto-update.test.ts b/packages/cli/test/auto-update.test.ts index 9c07946cbd..bd649bfd20 100644 --- a/packages/cli/test/auto-update.test.ts +++ b/packages/cli/test/auto-update.test.ts @@ -1174,6 +1174,27 @@ describe('checkForNpmVersionUpdate tag precedence', () => { const result = await checkForNpmVersionUpdate(() => {}, true); expect(result.status).toBe('up-to-date'); // not 'available' on garbage }); + + it('default path: a garbage `latest` must NOT mask a valid `beta` candidate', async () => { + const { checkForNpmVersionUpdate } = await import('../src/daemon.js'); + fetchImpl = async () => makeRegistryResponse({ latest: 'garbage', beta: '9.0.0-beta.4' }); + const result = await checkForNpmVersionUpdate(() => {}, true); + // current is 9.0.0-beta.3 → the valid beta.4 must be selected, not dropped. + expect(result.status).toBe('available'); + expect(result.version).toBe('9.0.0-beta.4'); + }); + + it('channel mainnet: a STABLE +build target is "available" over the current rc (not falsely up-to-date)', async () => { + const { checkForNpmVersionUpdate } = await import('../src/daemon.js'); + readFileImpl = async (p: any) => { + if (String(p).endsWith('.current-version')) return '10.0.0-rc.19'; + throw new Error('ENOENT'); + }; + fetchImpl = async () => makeRegistryResponse({ mainnet: '10.0.0+mainnet-build.1' }); + const result = await checkForNpmVersionUpdate(() => {}, false, 'mainnet'); + expect(result.status).toBe('available'); + expect(result.version).toBe('10.0.0+mainnet-build.1'); + }); }); describe('deriveUpdateCheckState (runCheck → /api/status mapping)', () => { @@ -1617,6 +1638,14 @@ describe('compareSemver', () => { expect(compareSemver('9.0.0+build.1', '9.0.0+build.2')).toBe(0); }); + it('treats HYPHENATED build metadata as stable, not prerelease (PR #1295 regression)', () => { + // The hyphen lives in +build metadata → 10.0.0+mainnet-build.1 is a STABLE + // 10.0.0 and must outrank 10.0.0-rc.19, not read as "not newer". + expect(compareSemver('10.0.0+mainnet-build.1', '10.0.0-rc.19')).toBeGreaterThan(0); + expect(compareSemver('10.0.0-rc.19', '10.0.0+mainnet-build.1')).toBeLessThan(0); + expect(compareSemver('10.0.0+mainnet-build.1', '10.0.0')).toBe(0); + }); + it('handles dev suffix with numeric comparison', () => { expect(compareSemver('9.0.0-beta.4-dev.200', '9.0.0-beta.4-dev.100')).toBeGreaterThan(0); }); diff --git a/packages/cli/test/maintenance-update-gate.test.ts b/packages/cli/test/maintenance-update-gate.test.ts new file mode 100644 index 0000000000..9eb315f1ac --- /dev/null +++ b/packages/cli/test/maintenance-update-gate.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect, vi } from 'vitest'; +import { resolveExplicitUpdateTarget } from '../src/commands/maintenance.js'; + +// Guards the `dkg update ` stable-only gate: an explicit prerelease / +// dist-tag-pointing-at-a-prerelease must not bypass allowPrerelease:false, and a +// transient dist-tag resolution error must fail CLOSED (PR #1295 follow-up). +describe('resolveExplicitUpdateTarget', () => { + const log = () => {}; + const resolverReturning = (ret: { version: string | null; error?: boolean }) => + vi.fn(async () => ret); + + it('allowPrerelease=true short-circuits — any target allowed, no registry hit', async () => { + const resolve = resolverReturning({ version: null }); + expect( + await resolveExplicitUpdateTarget('10.1.0-rc.1', true, { resolveLatestNpmVersion: resolve, log }), + ).toEqual({ ok: true }); + expect(resolve).not.toHaveBeenCalled(); + }); + + it('stable explicit version on a stable-only node → allowed', async () => { + const resolve = resolverReturning({ version: null }); + expect( + await resolveExplicitUpdateTarget('10.1.0', false, { resolveLatestNpmVersion: resolve, log }), + ).toEqual({ ok: true }); + }); + + it('prerelease explicit version on a stable-only node → refused', async () => { + const resolve = resolverReturning({ version: null }); + const r = await resolveExplicitUpdateTarget('10.1.0-rc.1', false, { resolveLatestNpmVersion: resolve, log }); + expect(r.ok).toBe(false); + }); + + it('dist-tag resolving to a STABLE version → allowed (resolved with allowPrerelease=true)', async () => { + const resolve = resolverReturning({ version: '10.0.0' }); + const r = await resolveExplicitUpdateTarget('latest', false, { resolveLatestNpmVersion: resolve, log }); + expect(r).toEqual({ ok: true }); + expect(resolve).toHaveBeenCalledWith(log, true, 'latest'); + }); + + it('dist-tag resolving to a PRERELEASE → refused', async () => { + const resolve = resolverReturning({ version: '10.1.0-rc.1' }); + const r = await resolveExplicitUpdateTarget('latest', false, { resolveLatestNpmVersion: resolve, log }); + expect(r.ok).toBe(false); + }); + + it('dist-tag resolution ERROR → refused (fail CLOSED, like the no-arg path)', async () => { + const resolve = resolverReturning({ version: null, error: true }); + const r = await resolveExplicitUpdateTarget('latest', false, { resolveLatestNpmVersion: resolve, log }); + expect(r.ok).toBe(false); + }); + + it('unknown dist-tag (no version, no error) → allowed to fall through (npm install fails naturally)', async () => { + const resolve = resolverReturning({ version: null }); + const r = await resolveExplicitUpdateTarget('nonexistent-tag', false, { resolveLatestNpmVersion: resolve, log }); + expect(r).toEqual({ ok: true }); + }); +});