From 1d1cc49cf1744c086caa7e864d7782f6994fb124 Mon Sep 17 00:00:00 2001 From: Beon de Nood Date: Fri, 27 Mar 2026 23:53:11 -0400 Subject: [PATCH 1/2] fix: add CAPISCIO_REQUIRE_CHECKSUM fail-closed mode When CAPISCIO_REQUIRE_CHECKSUM=true, binary download fails if checksums.txt is unavailable or the asset is missing from it. Default behavior remains fail-open (warn and continue) for backward compatibility. --- src/utils/binary-manager.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/utils/binary-manager.ts b/src/utils/binary-manager.ts index a1c6a8a..8b61472 100644 --- a/src/utils/binary-manager.ts +++ b/src/utils/binary-manager.ts @@ -161,6 +161,9 @@ export class BinaryManager { } private async verifyChecksum(filePath: string, assetName: string): Promise { + const requireChecksum = ['1', 'true', 'yes'].includes( + (process.env.CAPISCIO_REQUIRE_CHECKSUM ?? '').toLowerCase() + ); const checksumsUrl = `https://github.com/${REPO_OWNER}/${REPO_NAME}/releases/download/${VERSION}/checksums.txt`; let expectedHash: string | null = null; @@ -175,11 +178,25 @@ export class BinaryManager { } } } catch { + if (requireChecksum) { + fs.unlinkSync(filePath); + throw new Error( + 'Checksum verification required (CAPISCIO_REQUIRE_CHECKSUM=true) ' + + 'but checksums.txt is not available. Cannot verify binary integrity.' + ); + } console.warn('Warning: Could not fetch checksums.txt. Skipping integrity verification.'); return; } if (!expectedHash) { + if (requireChecksum) { + fs.unlinkSync(filePath); + throw new Error( + `Checksum verification required (CAPISCIO_REQUIRE_CHECKSUM=true) ` + + `but asset ${assetName} not found in checksums.txt.` + ); + } console.warn(`Warning: Asset ${assetName} not found in checksums.txt. Skipping verification.`); return; } From df7cc4ae0047ed72359ffd800679dd347161838a Mon Sep 17 00:00:00 2001 From: Beon de Nood Date: Sat, 28 Mar 2026 00:10:59 -0400 Subject: [PATCH 2/2] fix: address PR #40 review comments 1. Replace unlinkSync with rmSync({ force: true }) so file-removal errors never mask the checksum error 2. Switch to stream-based hashing (createReadStream + crypto pipe) instead of readFileSync to avoid loading entire binary into memory 3. Add checksum.test.ts with 7 tests covering: match, mismatch, fetch-fail (optional), fetch-fail (required), entry-missing (optional), entry-missing (required), env-var variants --- src/utils/binary-manager.ts | 15 +- tests/unit/checksum.test.ts | 263 ++++++++++++++++++++++++++++++++++++ 2 files changed, 273 insertions(+), 5 deletions(-) create mode 100644 tests/unit/checksum.test.ts diff --git a/src/utils/binary-manager.ts b/src/utils/binary-manager.ts index 8b61472..5236639 100644 --- a/src/utils/binary-manager.ts +++ b/src/utils/binary-manager.ts @@ -179,7 +179,7 @@ export class BinaryManager { } } catch { if (requireChecksum) { - fs.unlinkSync(filePath); + fs.rmSync(filePath, { force: true }); throw new Error( 'Checksum verification required (CAPISCIO_REQUIRE_CHECKSUM=true) ' + 'but checksums.txt is not available. Cannot verify binary integrity.' @@ -191,7 +191,7 @@ export class BinaryManager { if (!expectedHash) { if (requireChecksum) { - fs.unlinkSync(filePath); + fs.rmSync(filePath, { force: true }); throw new Error( `Checksum verification required (CAPISCIO_REQUIRE_CHECKSUM=true) ` + `but asset ${assetName} not found in checksums.txt.` @@ -201,12 +201,17 @@ export class BinaryManager { return; } - const fileBuffer = fs.readFileSync(filePath); - const actualHash = crypto.createHash('sha256').update(fileBuffer).digest('hex'); + const actualHash = await new Promise((resolve, reject) => { + const hash = crypto.createHash('sha256'); + const fileStream = fs.createReadStream(filePath); + fileStream.on('data', (chunk) => hash.update(chunk)); + fileStream.on('end', () => resolve(hash.digest('hex'))); + fileStream.on('error', reject); + }); if (actualHash !== expectedHash) { // Remove the tampered file - fs.unlinkSync(filePath); + fs.rmSync(filePath, { force: true }); throw new Error( `Binary integrity check failed for ${assetName}. ` + `Expected SHA-256: ${expectedHash}, got: ${actualHash}. ` + diff --git a/tests/unit/checksum.test.ts b/tests/unit/checksum.test.ts new file mode 100644 index 0000000..1092b69 --- /dev/null +++ b/tests/unit/checksum.test.ts @@ -0,0 +1,263 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import fs from 'fs'; +import os from 'os'; +import { Readable } from 'stream'; + +// Mock stream pipeline +vi.mock('stream', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + default: { + ...actual.default, + pipeline: (source: any, dest: any, cb: any) => { + if (cb) cb(null); + return { on: vi.fn() }; + }, + }, + pipeline: (source: any, dest: any, cb: any) => { + if (cb) cb(null); + return { on: vi.fn() }; + }, + }; +}); + +vi.mock('fs'); +vi.mock('axios'); +vi.mock('ora', () => ({ + default: vi.fn(() => ({ + start: vi.fn().mockReturnThis(), + succeed: vi.fn().mockReturnThis(), + fail: vi.fn().mockReturnThis(), + })), +})); + +function resetBinaryManager() { + vi.resetModules(); +} + +/** + * Helper: create a mock readable stream that emits the given data buffer + * and connects to crypto.createHash correctly via the 'data'/'end' event pattern. + */ +function mockReadStream(data: Buffer): fs.ReadStream { + const readable = new Readable({ + read() { + this.push(data); + this.push(null); + }, + }); + return readable as unknown as fs.ReadStream; +} + +describe('Checksum verification', () => { + const KNOWN_CONTENT = Buffer.from('test binary content'); + // Pre-computed SHA-256 of KNOWN_CONTENT + let KNOWN_HASH: string; + + beforeEach(async () => { + vi.clearAllMocks(); + resetBinaryManager(); + + // Compute expected hash dynamically so we don't hardcode + const crypto = await import('crypto'); + KNOWN_HASH = crypto.createHash('sha256').update(KNOWN_CONTENT).digest('hex'); + + // Default fs mocks + vi.mocked(fs.existsSync).mockImplementation((p) => { + const s = String(p); + if (s.includes('capiscio-core') && !s.includes('package.json')) return false; + return s.includes('package.json'); + }); + vi.mocked(fs.mkdirSync).mockReturnValue(undefined); + vi.mocked(fs.mkdtempSync).mockReturnValue('/tmp/capiscio-test'); + vi.mocked(fs.createWriteStream).mockReturnValue({ + on: vi.fn(), + once: vi.fn(), + emit: vi.fn(), + write: vi.fn(), + end: vi.fn(), + } as any); + vi.mocked(fs.copyFileSync).mockReturnValue(undefined); + vi.mocked(fs.chmodSync).mockReturnValue(undefined); + vi.mocked(fs.rmSync).mockReturnValue(undefined); + + vi.spyOn(os, 'platform').mockReturnValue('linux'); + vi.spyOn(os, 'arch').mockReturnValue('x64'); + + delete process.env.CAPISCIO_REQUIRE_CHECKSUM; + }); + + afterEach(() => { + vi.restoreAllMocks(); + delete process.env.CAPISCIO_REQUIRE_CHECKSUM; + }); + + /** + * Helper to set up axios mocks for download + checksum flow. + * @param checksumResponse - resolved value for checksums.txt GET, or Error to reject + * @param fileContent - buffer to serve as the downloaded binary content + */ + async function setupMocks( + checksumResponse: { data: string } | Error, + fileContent: Buffer = KNOWN_CONTENT, + ) { + const axios = await import('axios'); + + const downloadStream: any = { + pipe: vi.fn().mockReturnThis(), + on: vi.fn((event, cb) => { + if (event === 'end') cb(); + return downloadStream; + }), + once: vi.fn(), + emit: vi.fn(), + }; + + // First call = binary download, second call = checksums.txt + vi.mocked(axios.default.get).mockImplementation((url: string) => { + if (url.includes('checksums.txt')) { + if (checksumResponse instanceof Error) { + return Promise.reject(checksumResponse); + } + return Promise.resolve(checksumResponse); + } + // Binary download + return Promise.resolve({ data: downloadStream }); + }); + + // Mock createReadStream to return a stream of the file content + vi.mocked(fs.createReadStream).mockReturnValue(mockReadStream(fileContent)); + + return axios; + } + + it('should pass when checksum matches', async () => { + const assetName = 'capiscio-linux-amd64'; + await setupMocks({ + data: `${KNOWN_HASH} ${assetName}\n`, + }); + + const { BinaryManager } = await import('../../src/utils/binary-manager'); + const instance = BinaryManager.getInstance(); + + // Should not throw — checksum matches + await expect(instance.getBinaryPath()).resolves.toBeDefined(); + // rmSync is called once for temp dir cleanup (recursive), but never for the + // downloaded file alone (non-recursive force-only), which is the mismatch path. + const rmCalls = vi.mocked(fs.rmSync).mock.calls; + const fileDeleteCalls = rmCalls.filter( + ([, opts]) => opts && typeof opts === 'object' && !('recursive' in opts), + ); + expect(fileDeleteCalls).toHaveLength(0); + }); + + it('should throw and delete file when checksum mismatches', async () => { + const assetName = 'capiscio-linux-amd64'; + await setupMocks({ + data: `deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef ${assetName}\n`, + }); + + const { BinaryManager } = await import('../../src/utils/binary-manager'); + const instance = BinaryManager.getInstance(); + + await expect(instance.getBinaryPath()).rejects.toThrow('Binary integrity check failed'); + // Tampered file should be removed with force + expect(fs.rmSync).toHaveBeenCalledWith( + expect.stringContaining('capiscio'), + { force: true }, + ); + }); + + it('should skip verification when checksums.txt fetch fails and CAPISCIO_REQUIRE_CHECKSUM is not set', async () => { + await setupMocks(new Error('Network error')); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const { BinaryManager } = await import('../../src/utils/binary-manager'); + const instance = BinaryManager.getInstance(); + + await expect(instance.getBinaryPath()).resolves.toBeDefined(); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Could not fetch checksums.txt')); + warnSpy.mockRestore(); + }); + + it('should throw when checksums.txt fetch fails and CAPISCIO_REQUIRE_CHECKSUM=true', async () => { + process.env.CAPISCIO_REQUIRE_CHECKSUM = 'true'; + await setupMocks(new Error('Network error')); + + const { BinaryManager } = await import('../../src/utils/binary-manager'); + const instance = BinaryManager.getInstance(); + + await expect(instance.getBinaryPath()).rejects.toThrow( + 'Checksum verification required', + ); + expect(fs.rmSync).toHaveBeenCalledWith( + expect.stringContaining('capiscio'), + { force: true }, + ); + }); + + it('should throw when asset not found in checksums.txt and CAPISCIO_REQUIRE_CHECKSUM=true', async () => { + process.env.CAPISCIO_REQUIRE_CHECKSUM = 'true'; + // checksums.txt exists but does not contain our asset + await setupMocks({ + data: 'abc123 some-other-asset\n', + }); + + const { BinaryManager } = await import('../../src/utils/binary-manager'); + const instance = BinaryManager.getInstance(); + + await expect(instance.getBinaryPath()).rejects.toThrow( + 'not found in checksums.txt', + ); + expect(fs.rmSync).toHaveBeenCalledWith( + expect.stringContaining('capiscio'), + { force: true }, + ); + }); + + it('should skip verification when asset not found in checksums.txt and require is off', async () => { + await setupMocks({ + data: 'abc123 some-other-asset\n', + }); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const { BinaryManager } = await import('../../src/utils/binary-manager'); + const instance = BinaryManager.getInstance(); + + await expect(instance.getBinaryPath()).resolves.toBeDefined(); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('not found in checksums.txt')); + warnSpy.mockRestore(); + }); + + it('should accept CAPISCIO_REQUIRE_CHECKSUM values: 1, yes, TRUE', async () => { + for (const val of ['1', 'yes', 'TRUE']) { + resetBinaryManager(); + vi.clearAllMocks(); + + vi.mocked(fs.existsSync).mockImplementation((p) => { + const s = String(p); + if (s.includes('capiscio-core') && !s.includes('package.json')) return false; + return s.includes('package.json'); + }); + vi.mocked(fs.mkdirSync).mockReturnValue(undefined); + vi.mocked(fs.mkdtempSync).mockReturnValue('/tmp/capiscio-test'); + vi.mocked(fs.createWriteStream).mockReturnValue({ + on: vi.fn(), once: vi.fn(), emit: vi.fn(), write: vi.fn(), end: vi.fn(), + } as any); + vi.mocked(fs.copyFileSync).mockReturnValue(undefined); + vi.mocked(fs.chmodSync).mockReturnValue(undefined); + vi.mocked(fs.rmSync).mockReturnValue(undefined); + vi.spyOn(os, 'platform').mockReturnValue('linux'); + vi.spyOn(os, 'arch').mockReturnValue('x64'); + + process.env.CAPISCIO_REQUIRE_CHECKSUM = val; + await setupMocks(new Error('fetch failed')); + + const { BinaryManager } = await import('../../src/utils/binary-manager'); + const instance = BinaryManager.getInstance(); + + await expect(instance.getBinaryPath()).rejects.toThrow('Checksum verification required'); + } + }); +});