From 49cd286730dd59151227eeda908c81dad756e58c Mon Sep 17 00:00:00 2001 From: ArjunDeshwal Date: Mon, 31 Aug 2026 04:58:01 +0530 Subject: [PATCH] fix(release): verify downloaded binary identity --- .github/workflows/ci.yml | 14 ++ cli/release-core/launcher.js | 140 ++++++++++- cli/scripts/build-binary.ts | 1 + cli/src/__tests__/build-identity.test.ts | 61 +++++ .../__tests__/launcher-avx2-fallback.test.ts | 36 ++- .../release/artifact-identity.test.ts | 220 ++++++++++++++++++ .../__tests__/release/wrapper-safety.test.ts | 20 +- cli/src/build-identity.ts | 25 ++ cli/src/entry.ts | 32 ++- 9 files changed, 529 insertions(+), 20 deletions(-) create mode 100644 cli/src/__tests__/build-identity.test.ts create mode 100644 cli/src/__tests__/release/artifact-identity.test.ts create mode 100644 cli/src/build-identity.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2f2e22ae1..4d65cf19d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,4 +51,18 @@ jobs: run: | chmod +x cli/bin/freebuff cli/bin/freebuff --version + BUILD_INFO="$(cli/bin/freebuff --print-build-info)" bun -e ' + const actual = JSON.parse(process.env.BUILD_INFO); + const expected = { + schemaVersion: 1, + product: "freebuff", + version: "0.0.0-ci", + target: "linux-x64", + }; + if (JSON.stringify(actual) !== JSON.stringify(expected)) { + throw new Error( + `Unexpected binary identity: ${JSON.stringify(actual)}`, + ); + } + ' bun cli/scripts/smoke-binary.ts cli/bin/freebuff diff --git a/cli/release-core/launcher.js b/cli/release-core/launcher.js index 867c1d5d6e..0ffea93263 100644 --- a/cli/release-core/launcher.js +++ b/cli/release-core/launcher.js @@ -25,6 +25,8 @@ function createLauncher(productConfig) { // Tests only. os.homedir() ignores $HOME under `bun test`, so pointing HOME // at a temp dir is not enough to keep a test off the real ~/.config. configDir: configDirOverride = null, + // Tests only. Production keeps the 10s bound for slow Windows startup. + buildIdentityTimeoutMs = 10000, } = productConfig /** @@ -82,6 +84,10 @@ function createLauncher(productConfig) { /** Bytes of the binary's stderr kept for the crash report. */ const STDERR_TAIL_BYTES = 8192 + /** Versioned contract emitted by compiled binaries via --print-build-info. */ + const BUILD_IDENTITY_SCHEMA_VERSION = 1 + const BUILD_IDENTITY_OUTPUT_BYTES = 4096 + function getUnsignedExitCode(code) { return code != null && code < 0 ? code >>> 0 : code } @@ -741,6 +747,120 @@ function createLauncher(productConfig) { } } + function readStagedBinaryIdentity(binaryPath) { + return new Promise((resolve, reject) => { + const child = spawn(binaryPath, ['--print-build-info'], { + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true, + env: { ...process.env, NO_COLOR: '1', TERM: 'dumb' }, + }) + + let stdout = '' + let stderr = '' + let outputBytes = 0 + let settled = false + + const settle = (error, identity = null) => { + if (settled) return + settled = true + clearTimeout(timer) + if (error) reject(error) + else resolve(identity) + } + + const append = (stream) => (chunk) => { + outputBytes += chunk.length + if (outputBytes > BUILD_IDENTITY_OUTPUT_BYTES) { + const error = new Error( + `Downloaded artifact produced more than ${BUILD_IDENTITY_OUTPUT_BYTES} bytes while reporting its identity`, + ) + error.code = 'ARTIFACT_IDENTITY_INVALID' + child.kill('SIGKILL') + settle(error) + return + } + if (stream === 'stdout') stdout += chunk.toString('utf8') + else stderr += chunk.toString('utf8') + } + + child.stdout?.on('data', append('stdout')) + child.stderr?.on('data', append('stderr')) + + const timer = setTimeout(() => { + const error = new Error( + `Downloaded artifact did not report its identity within ${buildIdentityTimeoutMs}ms`, + ) + error.code = 'ARTIFACT_IDENTITY_TIMEOUT' + child.kill('SIGKILL') + settle(error) + }, buildIdentityTimeoutMs) + + child.once('error', (cause) => { + const error = new Error( + `Could not execute downloaded artifact: ${cause.message}`, + { cause }, + ) + error.code = 'ARTIFACT_IDENTITY_EXEC_FAILED' + settle(error) + }) + child.once('close', (code, signal) => { + if (code !== 0) { + const detail = stderr.trim().slice(0, 512) + const error = new Error( + `Downloaded artifact identity command exited with ${ + signal ? `signal ${signal}` : `code ${code}` + }${detail ? `: ${detail}` : ''}`, + ) + error.code = 'ARTIFACT_IDENTITY_EXEC_FAILED' + settle(error) + return + } + + let identity + try { + identity = JSON.parse(stdout.trim()) + } catch { + const error = new Error( + 'Downloaded artifact returned malformed build identity JSON', + ) + error.code = 'ARTIFACT_IDENTITY_INVALID' + settle(error) + return + } + settle(null, identity) + }) + }) + } + + async function validateStagedBinary({ tempBinaryPath, version, targetKey }) { + const identity = await readStagedBinaryIdentity(tempBinaryPath) + const expected = { + schemaVersion: BUILD_IDENTITY_SCHEMA_VERSION, + product: packageName, + version, + target: targetKey, + } + + const matches = + identity && + !Array.isArray(identity) && + identity.schemaVersion === expected.schemaVersion && + identity.product === expected.product && + identity.version === expected.version && + identity.target === expected.target + + if (!matches) { + const error = new Error( + `Downloaded artifact identity mismatch: expected ${JSON.stringify( + expected, + )}, received ${JSON.stringify(identity)}`, + ) + error.code = 'ARTIFACT_IDENTITY_MISMATCH' + error.retryable = false + throw error + } + } + async function stageBinary( version, targetKey = getDownloadTargetKey(), @@ -775,11 +895,22 @@ function createLauncher(productConfig) { if (process.platform !== 'win32') { fs.chmodSync(tempBinaryPath, 0o755) } + await validateStagedBinary({ tempBinaryPath, version, targetKey }) } catch (error) { try { fs.rmSync(CONFIG.tempDownloadDir, { recursive: true, force: true }) } catch { - // Preserve the original chmod error. + // Preserve the original staging error. + } + if ( + typeof error?.code === 'string' && + error.code.startsWith('ARTIFACT_IDENTITY_') + ) { + trackUpdateFailed(error.message, version, { + stage: 'artifact_validation', + errorCode: error.code, + target: targetKey, + }) } throw error } @@ -977,7 +1108,10 @@ function createLauncher(productConfig) { // relaunch's download for the shared temp directory (prepareTempDownloadDir // rmSyncs it) and then spend six seconds SIGKILLing a process that has // already exited. - if (runningProcess.exitCode !== null || runningProcess.signalCode !== null) { + if ( + runningProcess.exitCode !== null || + runningProcess.signalCode !== null + ) { return } @@ -1459,6 +1593,8 @@ function createLauncher(productConfig) { getCurrentVersion, getMetadataVersion, getRequiredWrapperVersion, + readStagedBinaryIdentity, + validateStagedBinary, ensureBinaryReady, isTargetAllowedForThisMachine, CONFIG, diff --git a/cli/scripts/build-binary.ts b/cli/scripts/build-binary.ts index d20fa22a79..361dbef2f2 100644 --- a/cli/scripts/build-binary.ts +++ b/cli/scripts/build-binary.ts @@ -165,6 +165,7 @@ async function main() { const defineFlags = [ ['process.env.NODE_ENV', '"production"'], ['process.env.CODEBUFF_IS_BINARY', '"true"'], + ['process.env.CODEBUFF_CLI_BINARY_NAME', JSON.stringify(binaryName)], ['process.env.CODEBUFF_CLI_VERSION', `"${version}"`], ['process.env.CODEBUFF_CLI_TARGET', `"${getCliTargetLabel(targetInfo)}"`], ['process.env.FREEBUFF_MODE', `"${process.env.FREEBUFF_MODE ?? 'false'}"`], diff --git a/cli/src/__tests__/build-identity.test.ts b/cli/src/__tests__/build-identity.test.ts new file mode 100644 index 0000000000..03f95370b7 --- /dev/null +++ b/cli/src/__tests__/build-identity.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, test } from 'bun:test' +import { spawnSync } from 'node:child_process' +import { fileURLToPath } from 'node:url' + +import { + BUILD_IDENTITY_SCHEMA_VERSION, + createBuildIdentity, +} from '../build-identity' + +describe('compiled binary identity', () => { + test('uses a versioned, machine-readable contract', () => { + expect( + createBuildIdentity({ + product: 'freebuff', + version: '1.2.3', + target: 'win32-x64', + }), + ).toEqual({ + schemaVersion: BUILD_IDENTITY_SCHEMA_VERSION, + product: 'freebuff', + version: '1.2.3', + target: 'win32-x64', + }) + }) + + test('preserves baseline target labels', () => { + expect( + createBuildIdentity({ + product: 'freebuff', + version: '1.2.3', + target: 'win32-x64-baseline', + }).target, + ).toBe('win32-x64-baseline') + }) + + test('entrypoint reports identity without loading the UI', () => { + const entryPath = fileURLToPath(new URL('../entry.ts', import.meta.url)) + const result = spawnSync( + process.execPath, + [entryPath, '--print-build-info'], + { + encoding: 'utf8', + env: { + ...process.env, + CODEBUFF_CLI_BINARY_NAME: 'freebuff', + CODEBUFF_CLI_VERSION: '9.8.7', + CODEBUFF_CLI_TARGET: 'win32-x64', + }, + }, + ) + + expect(result.status).toBe(0) + expect(result.stderr).toBe('') + expect(JSON.parse(result.stdout)).toEqual({ + schemaVersion: BUILD_IDENTITY_SCHEMA_VERSION, + product: 'freebuff', + version: '9.8.7', + target: 'win32-x64', + }) + }) +}) diff --git a/cli/src/__tests__/launcher-avx2-fallback.test.ts b/cli/src/__tests__/launcher-avx2-fallback.test.ts index ff32c0a905..0718368d4c 100644 --- a/cli/src/__tests__/launcher-avx2-fallback.test.ts +++ b/cli/src/__tests__/launcher-avx2-fallback.test.ts @@ -83,7 +83,10 @@ async function waitFor(done: () => boolean, timeoutMs = 5000) { * matters beyond the assertions: a relaunched child's handler firing after a * test would otherwise call the real process.exit and take the runner down. */ -const launcher = { lines: [] as string[], exitCodes: [] as (number | undefined)[] } +const launcher = { + lines: [] as string[], + exitCodes: [] as (number | undefined)[], +} let restoreLauncherCapture = () => {} function captureLauncherOutput() { @@ -109,9 +112,23 @@ function captureLauncherOutput() { /** A tar.gz holding a single `freebuff.exe` that runs `script`. */ function baselineTarball(script: string) { const stageDir = mkdtempSync(join(tmpdir(), 'launcher-baseline-')) - writeFileSync(join(stageDir, 'freebuff.exe'), `#!/bin/sh\n${script}\n`, { - mode: 0o755, - }) + const identity = JSON.stringify({ + schemaVersion: 1, + product: 'freebuff', + version: '1.2.3', + target: 'win32-x64-baseline', + }) + writeFileSync( + join(stageDir, 'freebuff.exe'), + `#!/bin/sh +if [ "$1" = "--print-build-info" ]; then + printf '%s\\n' '${identity}' + exit 0 +fi +${script} +`, + { mode: 0o755 }, + ) const archive = join(stageDir, 'out.tar.gz') execFileSync('tar', ['-czf', archive, '-C', stageDir, 'freebuff.exe']) return readFileSync(archive) @@ -201,9 +218,9 @@ describe('windows AVX2 detection', () => { expect(t.detectMachineHasAvx2()).toBe(false) expect(t.readCachedAvx2()).toBe(false) - expect(JSON.parse(readFileSync(t.getCpuFeatureCachePath(), 'utf8'))).toEqual( - { avx2: false }, - ) + expect( + JSON.parse(readFileSync(t.getCpuFeatureCachePath(), 'utf8')), + ).toEqual({ avx2: false }) }) test('a recorded failure selects baseline up front on the NEXT launch', () => { @@ -265,7 +282,10 @@ describe('windows AVX2 detection', () => { describe('recovery after a recorded failure', () => { /** Simulate a completed install of `target` at `version`. */ function installBinary(t: ReturnType, target: string) { - writeFileSync(t.CONFIG.metadataPath, JSON.stringify({ version: '1.2.3', target })) + writeFileSync( + t.CONFIG.metadataPath, + JSON.stringify({ version: '1.2.3', target }), + ) writeFileSync(t.CONFIG.binaryPath, 'pretend binary') } diff --git a/cli/src/__tests__/release/artifact-identity.test.ts b/cli/src/__tests__/release/artifact-identity.test.ts new file mode 100644 index 0000000000..2c92426070 --- /dev/null +++ b/cli/src/__tests__/release/artifact-identity.test.ts @@ -0,0 +1,220 @@ +import { afterEach, describe, expect, test } from 'bun:test' +import { + chmodSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs' +import { createServer } from 'node:http' +import type { AddressInfo } from 'node:net' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +const { createLauncher } = require('../../../release-core/launcher.js') + +const fixtureDirs: string[] = [] + +afterEach(() => { + for (const directory of fixtureDirs.splice(0)) { + rmSync(directory, { recursive: true, force: true }) + } +}) + +function identityBinary({ + identity, + body, +}: { + identity?: unknown + body?: string +}): string { + const directory = mkdtempSync(join(tmpdir(), 'artifact-identity-')) + fixtureDirs.push(directory) + const binary = join(directory, 'candidate') + const serializedIdentity = JSON.stringify(identity) ?? 'null' + const script = body ?? `printf '%s\\n' '${serializedIdentity}'` + writeFileSync(binary, `#!/bin/sh\n${script}\n`, { mode: 0o755 }) + chmodSync(binary, 0o755) + return binary +} + +function validator(timeoutMs = 10000) { + const configDir = mkdtempSync(join(tmpdir(), 'artifact-config-')) + fixtureDirs.push(configDir) + return createLauncher({ + packageName: 'freebuff', + displayName: 'Freebuff', + configDir, + buildIdentityTimeoutMs: timeoutMs, + }).__testing.validateStagedBinary +} + +const expected = { + schemaVersion: 1, + product: 'freebuff', + version: '1.2.3', + target: 'win32-x64', +} + +describe('downloaded artifact identity validation', () => { + test('accepts the requested product, version, and target', async () => { + await expect( + validator()({ + tempBinaryPath: identityBinary({ identity: expected }), + version: expected.version, + targetKey: expected.target, + }), + ).resolves.toBeUndefined() + }) + + for (const [field, value] of [ + ['product', 'codebuff'], + ['version', '1.2.2'], + ['target', 'linux-arm64'], + ['schemaVersion', 2], + ] as const) { + test(`rejects a mismatched ${field}`, async () => { + const identity = { ...expected, [field]: value } + await expect( + validator()({ + tempBinaryPath: identityBinary({ identity }), + version: expected.version, + targetKey: expected.target, + }), + ).rejects.toMatchObject({ code: 'ARTIFACT_IDENTITY_MISMATCH' }) + }) + } + + test('rejects an unrelated bundled script', async () => { + await expect( + validator()({ + tempBinaryPath: identityBinary({ + body: "echo 'Usage: node test-bootstrap-caching.mjs PLUGIN_PATH present|missing' >&2; exit 1", + }), + version: expected.version, + targetKey: expected.target, + }), + ).rejects.toMatchObject({ code: 'ARTIFACT_IDENTITY_EXEC_FAILED' }) + }) + + test('reports an artifact that cannot be executed', async () => { + const directory = mkdtempSync(join(tmpdir(), 'missing-artifact-')) + fixtureDirs.push(directory) + await expect( + validator()({ + tempBinaryPath: join(directory, 'freebuff'), + version: expected.version, + targetKey: expected.target, + }), + ).rejects.toMatchObject({ code: 'ARTIFACT_IDENTITY_EXEC_FAILED' }) + }) + + test('rejects malformed output', async () => { + await expect( + validator()({ + tempBinaryPath: identityBinary({ body: "echo 'not json'" }), + version: expected.version, + targetKey: expected.target, + }), + ).rejects.toMatchObject({ code: 'ARTIFACT_IDENTITY_INVALID' }) + }) + + test('bounds identity command output', async () => { + await expect( + validator()({ + tempBinaryPath: identityBinary({ + body: 'i=0; while [ $i -lt 5000 ]; do printf x; i=$((i+1)); done', + }), + version: expected.version, + targetKey: expected.target, + }), + ).rejects.toMatchObject({ code: 'ARTIFACT_IDENTITY_INVALID' }) + }) + + test('bounds a hung identity check', async () => { + await expect( + validator(50)({ + tempBinaryPath: identityBinary({ body: 'sleep 5' }), + version: expected.version, + targetKey: expected.target, + }), + ).rejects.toMatchObject({ code: 'ARTIFACT_IDENTITY_TIMEOUT' }) + }) + + test('does not replace a cached binary with a mislabeled release artifact', async () => { + const configDir = mkdtempSync(join(tmpdir(), 'artifact-cache-')) + const archiveDir = mkdtempSync(join(tmpdir(), 'artifact-archive-')) + fixtureDirs.push(configDir, archiveDir) + + const launcher = createLauncher({ + packageName: 'freebuff', + displayName: 'Freebuff', + wrapperVersion: '2.0.0', + includeTreeSitterWasm: false, + configDir, + }) + const { CONFIG } = launcher.__testing + const target = `${process.platform}-${process.arch}` + const cachedContents = '#!/bin/sh\necho cached\n' + writeFileSync(CONFIG.binaryPath, cachedContents, { mode: 0o755 }) + writeFileSync( + CONFIG.metadataPath, + JSON.stringify({ version: '1.0.0', target }), + ) + + const wrongBinary = identityBinary({ + identity: { + schemaVersion: 1, + product: 'codebuff', + version: '2.0.0', + target, + }, + }) + writeFileSync( + join(archiveDir, CONFIG.binaryName), + readFileSync(wrongBinary), + { mode: 0o755 }, + ) + const archivePath = join(archiveDir, 'release.tar.gz') + const tar = require('tar') as typeof import('tar') + await tar.c({ cwd: archiveDir, file: archivePath, gzip: true }, [ + CONFIG.binaryName, + ]) + const archive = readFileSync(archivePath) + + const server = createServer((_request, response) => { + response.writeHead(200, { + 'content-length': archive.byteLength, + 'content-type': 'application/gzip', + }) + response.end(archive) + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + + const previousAppUrl = process.env.NEXT_PUBLIC_CODEBUFF_APP_URL + const previousNoProxy = process.env.NO_PROXY + const { port } = server.address() as AddressInfo + process.env.NEXT_PUBLIC_CODEBUFF_APP_URL = `http://127.0.0.1:${port}` + process.env.NO_PROXY = '127.0.0.1' + + const originalConsoleError = console.error + console.error = () => {} + try { + await launcher.__testing.ensureBinaryReady() + expect(readFileSync(CONFIG.binaryPath, 'utf8')).toBe(cachedContents) + expect( + JSON.parse(readFileSync(CONFIG.metadataPath, 'utf8')), + ).toMatchObject({ version: '1.0.0', target }) + } finally { + console.error = originalConsoleError + if (previousAppUrl === undefined) { + delete process.env.NEXT_PUBLIC_CODEBUFF_APP_URL + } else { + process.env.NEXT_PUBLIC_CODEBUFF_APP_URL = previousAppUrl + } + if (previousNoProxy === undefined) delete process.env.NO_PROXY + else process.env.NO_PROXY = previousNoProxy + await new Promise((resolve) => server.close(() => resolve())) + } + }) +}) diff --git a/cli/src/__tests__/release/wrapper-safety.test.ts b/cli/src/__tests__/release/wrapper-safety.test.ts index aeb0d60c04..e930d3e140 100644 --- a/cli/src/__tests__/release/wrapper-safety.test.ts +++ b/cli/src/__tests__/release/wrapper-safety.test.ts @@ -252,15 +252,29 @@ describe('shared release launcher safety', () => { configDir, }) const { CONFIG } = launcher.__testing + const target = launcher.__testing.getDefaultTargetKey() + const replacementIdentity = JSON.stringify({ + schemaVersion: 1, + product: 'repair-test', + version: '2.0.0', + target, + }) + const replacementBinary = [ + '#!/bin/sh', + `printf '%s\\n' '${replacementIdentity}'`, + '', + ].join('\n') writeFileSync(CONFIG.binaryPath, 'stale binary') writeFileSync( CONFIG.metadataPath, JSON.stringify({ version: '1.0.0', - target: process.platform + '-' + process.arch, + target, }), ) - writeFileSync(join(archiveDir, CONFIG.binaryName), 'replacement binary') + writeFileSync(join(archiveDir, CONFIG.binaryName), replacementBinary, { + mode: 0o755, + }) const tar = require('tar') as typeof import('tar') await tar.c({ cwd: archiveDir, file: archivePath, gzip: true }, [ @@ -281,7 +295,7 @@ describe('shared release launcher safety', () => { await launcher.__testing.ensureBinaryReady() expect(readFileSync(CONFIG.binaryPath, 'utf8')).toBe( - 'replacement binary', + replacementBinary, ) expect( JSON.parse(readFileSync(CONFIG.metadataPath, 'utf8')), diff --git a/cli/src/build-identity.ts b/cli/src/build-identity.ts new file mode 100644 index 0000000000..05cddb3801 --- /dev/null +++ b/cli/src/build-identity.ts @@ -0,0 +1,25 @@ +export const BUILD_IDENTITY_SCHEMA_VERSION = 1 as const + +export type BuildIdentity = { + schemaVersion: typeof BUILD_IDENTITY_SCHEMA_VERSION + product: string + version: string + target: string +} + +/** + * Machine-readable identity embedded into every compiled CLI artifact. + * The release launcher verifies this before replacing an installed binary. + */ +export function createBuildIdentity({ + product, + version, + target, +}: Omit): BuildIdentity { + return { + schemaVersion: BUILD_IDENTITY_SCHEMA_VERSION, + product, + version, + target, + } +} diff --git a/cli/src/entry.ts b/cli/src/entry.ts index a403f3d2c4..bbb725904e 100644 --- a/cli/src/entry.ts +++ b/cli/src/entry.ts @@ -1,12 +1,30 @@ #!/usr/bin/env bun -import { - isTerminalCommandBrokerInvocation, - serveTerminalCommandBroker, -} from './utils/terminal-command-broker' +import { createBuildIdentity } from './build-identity' -if (isTerminalCommandBrokerInvocation(process.argv)) { - await serveTerminalCommandBroker() +// The release launcher asks a staged binary to identify itself before install. +// Handle this before importing the UI so verification cannot depend on +// tree-sitter, terminal setup, authentication, or network availability. +if (process.argv.includes('--print-build-info')) { + console.log( + JSON.stringify( + createBuildIdentity({ + product: + process.env.CODEBUFF_CLI_BINARY_NAME ?? + (process.env.FREEBUFF_MODE === 'true' ? 'freebuff' : 'codebuff'), + version: process.env.CODEBUFF_CLI_VERSION ?? 'dev', + target: + process.env.CODEBUFF_CLI_TARGET ?? + `${process.platform}-${process.arch}`, + }), + ), + ) } else { - await import('./index') + const { isTerminalCommandBrokerInvocation, serveTerminalCommandBroker } = + await import('./utils/terminal-command-broker') + if (isTerminalCommandBrokerInvocation(process.argv)) { + await serveTerminalCommandBroker() + } else { + await import('./index') + } }