From a4496ff1d944fbe989d1ae18f7f78eb470846731 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Fri, 10 Apr 2026 22:44:46 -0400 Subject: [PATCH] fix(update): harden install verification flow --- src/commands/update-command.ts | 230 +++++++++--------- src/utils/package-manager-detector.ts | 6 + .../update-command-current-install.test.ts | 52 ++-- 3 files changed, 149 insertions(+), 139 deletions(-) diff --git a/src/commands/update-command.ts b/src/commands/update-command.ts index 4346df7c..fd1c2ad4 100644 --- a/src/commands/update-command.ts +++ b/src/commands/update-command.ts @@ -237,16 +237,16 @@ function handleNoUpdate(reason: string | undefined): void { } /** - * Perform update via npm/yarn/pnpm/bun + * Perform update verification against the current install. */ -function verifyCurrentInstallVersion( +async function verifyCurrentInstallVersion( currentInstall: CurrentInstall, targetTag: string, expectedVersion?: string, previousState?: InstalledPackageState, isReinstall: boolean = false, deps: UpdateCommandDeps = defaultDeps -): void { +): Promise { const nextState = deps.readInstalledPackageState(currentInstall); const installedVersion = nextState.version; if (!installedVersion) { @@ -259,6 +259,7 @@ function verifyCurrentInstallVersion( console.log(color(` ${deps.formatManualUpdateCommand(targetTag, currentInstall)}`, 'command')); console.log(''); process.exit(1); + return; } const installChanged = @@ -268,12 +269,26 @@ function verifyCurrentInstallVersion( previousState.scriptMtimeMs !== nextState.scriptMtimeMs); if (expectedVersion && installedVersion !== expectedVersion) { - if (previousState?.version && installedVersion !== previousState.version) { + const postUpdateResult = await deps.checkForUpdates( + installedVersion, + true, + 'npm', + targetTag as TargetTag + ); + + if (postUpdateResult.status === 'no_update') { + return; + } + + if ( + postUpdateResult.status === 'update_available' && + postUpdateResult.latest === installedVersion + ) { return; } const comparison = deps.compareVersionsWithPrerelease(installedVersion, expectedVersion); - if (comparison < 0) { + if (comparison < 0 || installedVersion === previousState?.version) { console.log(''); console.log( fail( @@ -294,6 +309,7 @@ function verifyCurrentInstallVersion( ); console.log(''); process.exit(1); + return; } } @@ -305,18 +321,57 @@ function verifyCurrentInstallVersion( ) { console.log(''); console.log( - fail( - `Reinstall completed, but CCS could not verify that the current installation changed from ${previousState.version}.` + warn( + `Reinstall completed, but CCS could not prove that the current installation changed from ${previousState.version}. Verify the current binary manually if this reinstall was meant to repair a same-version install.` ) ); - console.log(''); - console.log('Re-run manually against the current install:'); - console.log(color(` ${deps.formatManualUpdateCommand(targetTag, currentInstall)}`, 'command')); - console.log(''); - process.exit(1); } } +function runChildProcess( + deps: UpdateCommandDeps, + command: string, + args: string[], + options: { + isWindows: boolean; + env: NodeJS.ProcessEnv; + filterCleanupWarnings?: boolean; + } +): Promise { + return new Promise((resolve, reject) => { + const { isWindows, env, filterCleanupWarnings = false } = options; + const child = isWindows + ? deps.spawn(`${command} ${args.join(' ')}`, [], { + stdio: ['inherit', 'inherit', 'pipe'], + shell: true, + env: { ...env, NODE_NO_WARNINGS: '1' }, + }) + : deps.spawn(command, args, { stdio: 'inherit', env }); + + if (isWindows && filterCleanupWarnings && child.stderr) { + let stderrBuffer = ''; + child.stderr.on('data', (data: Buffer) => { + stderrBuffer += data.toString(); + const lines = stderrBuffer.split('\n'); + stderrBuffer = lines.pop() || ''; + for (const line of lines) { + if (!/npm warn cleanup/i.test(line)) { + process.stderr.write(line + '\n'); + } + } + }); + child.stderr.on('close', () => { + if (stderrBuffer && !/npm warn cleanup/i.test(stderrBuffer)) { + process.stderr.write(stderrBuffer); + } + }); + } + + child.on('error', reject); + child.on('exit', (code) => resolve(code ?? 0)); + }); +} + async function performNpmUpdate( currentInstall: CurrentInstall, targetTag: string = 'latest', @@ -371,86 +426,6 @@ async function performNpmUpdate( const isWindows = process.platform === 'win32'; - const performUpdate = (): void => { - // On Windows, use shell with full command string to avoid deprecation warning - // Also suppress Node deprecation warnings that may come from package managers - // Pipe stderr on Windows to filter npm cleanup warnings (EPERM on native modules) - const child = isWindows - ? deps.spawn(`${updateCommand} ${updateArgs.join(' ')}`, [], { - stdio: ['inherit', 'inherit', 'pipe'], - shell: true, - env: { ...childEnv, NODE_NO_WARNINGS: '1' }, - }) - : deps.spawn(updateCommand, updateArgs, { stdio: 'inherit', env: childEnv }); - - // On Windows, filter stderr to hide npm cleanup warnings (EPERM on bcrypt.node etc.) - // These warnings are cosmetic - update succeeds despite file locking by antivirus/indexing - // Use line-buffering to handle chunk splitting (data events don't guarantee message boundaries) - if (isWindows && child.stderr) { - let stderrBuffer = ''; - child.stderr.on('data', (data: Buffer) => { - stderrBuffer += data.toString(); - const lines = stderrBuffer.split('\n'); - stderrBuffer = lines.pop() || ''; // Keep incomplete line in buffer - for (const line of lines) { - // Skip npm cleanup warnings (EPERM, ENOTEMPTY, EBUSY on native module prebuilds) - if (!/npm warn cleanup/i.test(line)) { - process.stderr.write(line + '\n'); - } - } - }); - child.stderr.on('close', () => { - // Flush remaining buffer on stream close - if (stderrBuffer && !/npm warn cleanup/i.test(stderrBuffer)) { - process.stderr.write(stderrBuffer); - } - }); - } - - child.on('exit', (code) => { - if (code === 0) { - if (expectedVersion || previousState?.version) { - verifyCurrentInstallVersion( - currentInstall, - targetTag, - expectedVersion, - previousState, - isReinstall, - deps - ); - } - console.log(''); - console.log(ok(`${isReinstall ? 'Reinstall' : 'Update'} successful!`)); - console.log(''); - console.log(`Run ${color('ccs --version', 'command')} to verify`); - console.log(info(`Tip: Use ${color('ccs config', 'command')} for web-based configuration`)); - console.log(''); - } else { - console.log(''); - console.log(fail(`${isReinstall ? 'Reinstall' : 'Update'} failed`)); - console.log(''); - console.log('Try manually:'); - console.log( - color(` ${deps.formatManualUpdateCommand(targetTag, currentInstall)}`, 'command') - ); - console.log(''); - } - process.exit(code || 0); - }); - - child.on('error', () => { - console.log(''); - console.log(fail(`Failed to run ${packageManager} ${isReinstall ? 'reinstall' : 'update'}`)); - console.log(''); - console.log('Try manually:'); - console.log( - color(` ${deps.formatManualUpdateCommand(targetTag, currentInstall)}`, 'command') - ); - console.log(''); - process.exit(1); - }); - }; - if (cacheCommand && cacheArgs) { // For bun on Windows, we pre-remove instead of cache clear const isBunPreRemove = packageManager === 'bun' && cacheArgs.includes('remove'); @@ -462,27 +437,62 @@ async function performNpmUpdate( : 'Cache clearing failed, proceeding anyway...'; console.log(info(stepMessage)); - // On Windows, use shell with full command string to avoid deprecation warning - const cacheChild = isWindows - ? deps.spawn(`${cacheCommand} ${cacheArgs.join(' ')}`, [], { - stdio: 'inherit', - shell: true, - env: { ...childEnv, NODE_NO_WARNINGS: '1' }, - }) - : deps.spawn(cacheCommand, cacheArgs, { stdio: 'inherit', env: childEnv }); - - cacheChild.on('exit', (code) => { - if (code !== 0) { + try { + const cacheCode = await runChildProcess(deps, cacheCommand, cacheArgs, { + isWindows, + env: childEnv, + }); + if (cacheCode !== 0) { console.log(warn(failMessage)); } - performUpdate(); + } catch { + console.log(warn(failMessage)); + } + } + + try { + const exitCode = await runChildProcess(deps, updateCommand, updateArgs, { + isWindows, + env: childEnv, + filterCleanupWarnings: true, }); - cacheChild.on('error', () => { - console.log(warn(failMessage)); - performUpdate(); - }); - } else { - performUpdate(); + if (exitCode === 0) { + if (expectedVersion || previousState?.version) { + await verifyCurrentInstallVersion( + currentInstall, + targetTag, + expectedVersion, + previousState, + isReinstall, + deps + ); + } + console.log(''); + console.log(ok(`${isReinstall ? 'Reinstall' : 'Update'} successful!`)); + console.log(''); + console.log(`Run ${color('ccs --version', 'command')} to verify`); + console.log(info(`Tip: Use ${color('ccs config', 'command')} for web-based configuration`)); + console.log(''); + } else { + console.log(''); + console.log(fail(`${isReinstall ? 'Reinstall' : 'Update'} failed`)); + console.log(''); + console.log('Try manually:'); + console.log( + color(` ${deps.formatManualUpdateCommand(targetTag, currentInstall)}`, 'command') + ); + console.log(''); + } + + process.exit(exitCode || 0); + } catch { + console.log(''); + console.log(fail(`Failed to run ${packageManager} ${isReinstall ? 'reinstall' : 'update'}`)); + console.log(''); + console.log('Try manually:'); + console.log(color(` ${deps.formatManualUpdateCommand(targetTag, currentInstall)}`, 'command')); + console.log(''); + process.exit(1); } } diff --git a/src/utils/package-manager-detector.ts b/src/utils/package-manager-detector.ts index 4e5cea35..606c58ed 100644 --- a/src/utils/package-manager-detector.ts +++ b/src/utils/package-manager-detector.ts @@ -123,6 +123,12 @@ function inferInstallFromPath( normalizedPath.includes('/global/') && normalizedPath.includes('/node_modules/@kaitranntt/ccs') ) { + const pnpmFlatMatch = normalizedPath.match(/\/global\/([^/]+)\/node_modules\/@kaitranntt\/ccs/); + + if (!pnpmFlatMatch || pnpmFlatMatch[1] === 'lib') { + return null; + } + return { manager: 'pnpm', prefix: getPrefixBeforeMarker(targetPath, `${path.sep}global${path.sep}`), diff --git a/tests/unit/commands/update-command-current-install.test.ts b/tests/unit/commands/update-command-current-install.test.ts index a0f44ad8..d65d6a51 100644 --- a/tests/unit/commands/update-command-current-install.test.ts +++ b/tests/unit/commands/update-command-current-install.test.ts @@ -3,6 +3,7 @@ import { handleUpdateCommand, type UpdateCommandDeps } from '../../../src/comman let logLines: string[] = []; let spawnCalls: Array<{ command: string; args: string[]; env?: NodeJS.ProcessEnv }> = []; +let exitCodes: number[] = []; let originalConsoleLog: typeof console.log; let originalProcessExit: typeof process.exit; @@ -100,6 +101,7 @@ function createDeps(): UpdateCommandDeps { beforeEach(() => { logLines = []; spawnCalls = []; + exitCodes = []; stateReads = 0; scenario = { beforeState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, @@ -120,7 +122,7 @@ beforeEach(() => { }; process.exit = ((code?: number) => { - throw new Error(`process.exit(${code ?? 0})`); + exitCodes.push(code ?? 0); }) as typeof process.exit; }); @@ -131,15 +133,14 @@ afterEach(() => { describe('update-command current install handling', () => { it('updates through the current install manager and prefix', async () => { - await expect(handleUpdateCommand({ beta: true }, createDeps())).rejects.toThrow( - 'process.exit(0)' - ); + await handleUpdateCommand({ beta: true }, createDeps()); const installCall = spawnCalls.find((call) => call.args.includes('install')); expect(installCall?.command).toBe('npm'); expect(installCall?.args).toEqual(['install', '-g', '@kaitranntt/ccs@dev']); expect(installCall?.env?.npm_config_prefix).toBe('/tmp/ccs-prefix'); + expect(exitCodes).toContain(0); }); it('fails when another manager updated elsewhere but the current binary stayed stale', async () => { @@ -148,14 +149,13 @@ describe('update-command current install handling', () => { afterState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, }; - await expect(handleUpdateCommand({ beta: true }, createDeps())).rejects.toThrow( - 'process.exit(1)' - ); + await handleUpdateCommand({ beta: true }, createDeps()); expect(logLines.join('\n')).toContain('outside the current installation'); expect(logLines.join('\n')).toContain( 'NPM_CONFIG_PREFIX=/tmp/ccs-prefix npm install -g @kaitranntt/ccs@dev' ); + expect(exitCodes).toContain(1); }); it('keeps force mode under exact target-version verification', async () => { @@ -164,39 +164,36 @@ describe('update-command current install handling', () => { afterState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, }; - await expect(handleUpdateCommand({ force: true, beta: true }, createDeps())).rejects.toThrow( - 'process.exit(1)' - ); + await handleUpdateCommand({ force: true, beta: true }, createDeps()); expect(logLines.join('\n')).toContain('outside the current installation'); + expect(exitCodes).toContain(1); }); - it('fails force mode when target resolution says no update and the current install stays unchanged', async () => { + it('warns but succeeds when target resolution says no update and the current install stays unchanged', async () => { scenario = { beforeState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, afterState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, }; updateCheckResult = { status: 'no_update' }; - await expect(handleUpdateCommand({ force: true, beta: true }, createDeps())).rejects.toThrow( - 'process.exit(1)' - ); + await handleUpdateCommand({ force: true, beta: true }, createDeps()); - expect(logLines.join('\n')).toContain('could not verify that the current installation changed'); + expect(logLines.join('\n')).toContain('could not prove that the current installation changed'); + expect(exitCodes).toContain(0); }); - it('fails force mode when target version resolution fails and the current install stays unchanged', async () => { + it('warns but succeeds when target version resolution fails and the current install stays unchanged', async () => { scenario = { beforeState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, afterState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, }; updateCheckResult = { status: 'check_failed', message: 'network' }; - await expect(handleUpdateCommand({ force: true, beta: true }, createDeps())).rejects.toThrow( - 'process.exit(1)' - ); + await handleUpdateCommand({ force: true, beta: true }, createDeps()); - expect(logLines.join('\n')).toContain('could not verify that the current installation changed'); + expect(logLines.join('\n')).toContain('could not prove that the current installation changed'); + expect(exitCodes).toContain(0); }); it('accepts a newer installed version when the dist-tag moves during update', async () => { @@ -205,11 +202,10 @@ describe('update-command current install handling', () => { afterState: { version: '7.67.1-dev.0', packageJsonMtimeMs: 200, scriptMtimeMs: 200 }, }; - await expect(handleUpdateCommand({ beta: true }, createDeps())).rejects.toThrow( - 'process.exit(0)' - ); + await handleUpdateCommand({ beta: true }, createDeps()); expect(logLines.join('\n')).not.toContain('outside the current installation'); + expect(exitCodes).toContain(0); }); it('accepts force reinstall when the version stays the same but the current install files change', async () => { @@ -219,13 +215,12 @@ describe('update-command current install handling', () => { }; updateCheckResult = { status: 'no_update' }; - await expect(handleUpdateCommand({ force: true, beta: true }, createDeps())).rejects.toThrow( - 'process.exit(0)' - ); + await handleUpdateCommand({ force: true, beta: true }, createDeps()); expect(logLines.join('\n')).not.toContain( 'could not verify that the current installation changed' ); + expect(exitCodes).toContain(0); }); it.each([ @@ -241,9 +236,7 @@ describe('update-command current install handling', () => { prefix: envValue, }; - await expect(handleUpdateCommand({ beta: true }, createDeps())).rejects.toThrow( - 'process.exit(0)' - ); + await handleUpdateCommand({ beta: true }, createDeps()); const updateCall = spawnCalls.find( (call) => @@ -252,6 +245,7 @@ describe('update-command current install handling', () => { expect(updateCall?.args).toContain(expectedArg); expect(updateCall?.env?.[envKey]).toBe(envValue); + expect(exitCodes).toContain(0); } ); });