From 9dae4af7731bdc5be24904ff09eadb2063e2a46b Mon Sep 17 00:00:00 2001 From: sudacode Date: Sun, 29 Mar 2026 00:08:12 -0700 Subject: [PATCH] fix: harden coverage lane cleanup --- scripts/run-coverage-lane.test.ts | 15 ++++++- scripts/run-coverage-lane.ts | 70 ++++++++++++++++++------------- 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/scripts/run-coverage-lane.test.ts b/scripts/run-coverage-lane.test.ts index fd82e1c..7c6f0c8 100644 --- a/scripts/run-coverage-lane.test.ts +++ b/scripts/run-coverage-lane.test.ts @@ -1,7 +1,8 @@ import assert from 'node:assert/strict'; +import { resolve } from 'node:path'; import test from 'node:test'; -import { mergeLcovReports } from './run-coverage-lane'; +import { mergeLcovReports, resolveCoverageDir } from './run-coverage-lane'; test('mergeLcovReports combines duplicate source-file counters across shard outputs', () => { const merged = mergeLcovReports([ @@ -59,3 +60,15 @@ test('mergeLcovReports keeps distinct source files as separate records', () => { assert.match(merged, /SF:src\/a\.ts[\s\S]*end_of_record/); assert.match(merged, /SF:src\/b\.ts[\s\S]*end_of_record/); }); + +test('resolveCoverageDir keeps coverage output inside the repository', () => { + const repoRoot = resolve('/tmp', 'subminer-repo-root'); + + assert.equal(resolveCoverageDir(repoRoot, []), resolve(repoRoot, 'coverage')); + assert.equal( + resolveCoverageDir(repoRoot, ['--coverage-dir', 'coverage/test-src']), + resolve(repoRoot, 'coverage/test-src'), + ); + assert.throws(() => resolveCoverageDir(repoRoot, ['--coverage-dir', '../escape'])); + assert.throws(() => resolveCoverageDir(repoRoot, ['--coverage-dir', '/tmp/escape'])); +}); diff --git a/scripts/run-coverage-lane.ts b/scripts/run-coverage-lane.ts index 443e873..98b747f 100644 --- a/scripts/run-coverage-lane.ts +++ b/scripts/run-coverage-lane.ts @@ -1,6 +1,6 @@ import { existsSync, mkdirSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; import { spawnSync } from 'node:child_process'; -import { join, relative, resolve } from 'node:path'; +import { isAbsolute, join, relative, resolve } from 'node:path'; type LaneConfig = { roots: string[]; @@ -85,6 +85,15 @@ function parseCoverageDirArg(argv: string[]): string { return 'coverage'; } +export function resolveCoverageDir(repoRootDir: string, argv: string[]): string { + const candidate = resolve(repoRootDir, parseCoverageDirArg(argv)); + const rel = relative(repoRootDir, candidate); + if (isAbsolute(rel) || rel.startsWith('..')) { + throw new Error(`--coverage-dir must be within repository: ${candidate}`); + } + return candidate; +} + function parseLcovReport(report: string): LcovRecord[] { const records: LcovRecord[] = []; let current: LcovRecord | null = null; @@ -251,7 +260,7 @@ function runCoverageLane(): number { return 1; } - const coverageDir = resolve(repoRoot, parseCoverageDirArg(process.argv.slice(3))); + const coverageDir = resolveCoverageDir(repoRoot, process.argv.slice(3)); const shardRoot = join(coverageDir, '.shards'); mkdirSync(coverageDir, { recursive: true }); rmSync(shardRoot, { recursive: true, force: true }); @@ -260,37 +269,40 @@ function runCoverageLane(): number { const files = getLaneFiles(laneName); const reports: string[] = []; - for (const [index, file] of files.entries()) { - const shardDir = join(shardRoot, `${String(index + 1).padStart(3, '0')}`); - const result = spawnSync( - 'bun', - ['test', '--coverage', '--coverage-reporter=lcov', '--coverage-dir', shardDir, `./${file}`], - { - cwd: repoRoot, - stdio: 'inherit', - }, - ); + try { + for (const [index, file] of files.entries()) { + const shardDir = join(shardRoot, `${String(index + 1).padStart(3, '0')}`); + const result = spawnSync( + 'bun', + ['test', '--coverage', '--coverage-reporter=lcov', '--coverage-dir', shardDir, `./${file}`], + { + cwd: repoRoot, + stdio: 'inherit', + }, + ); - if (result.error) { - throw result.error; - } - if ((result.status ?? 1) !== 0) { - return result.status ?? 1; + if (result.error) { + throw result.error; + } + if ((result.status ?? 1) !== 0) { + return result.status ?? 1; + } + + const lcovPath = join(shardDir, 'lcov.info'); + if (!existsSync(lcovPath)) { + process.stdout.write(`Skipping empty coverage shard for ${file}\n`); + continue; + } + + reports.push(readFileSync(lcovPath, 'utf8')); } - const lcovPath = join(shardDir, 'lcov.info'); - if (!existsSync(lcovPath)) { - process.stdout.write(`Skipping empty coverage shard for ${file}\n`); - continue; - } - - reports.push(readFileSync(lcovPath, 'utf8')); + writeFileSync(join(coverageDir, 'lcov.info'), mergeLcovReports(reports), 'utf8'); + process.stdout.write(`Merged LCOV written to ${relative(repoRoot, join(coverageDir, 'lcov.info'))}\n`); + return 0; + } finally { + rmSync(shardRoot, { recursive: true, force: true }); } - - writeFileSync(join(coverageDir, 'lcov.info'), mergeLcovReports(reports), 'utf8'); - rmSync(shardRoot, { recursive: true, force: true }); - process.stdout.write(`Merged LCOV written to ${relative(repoRoot, join(coverageDir, 'lcov.info'))}\n`); - return 0; } if (require.main === module) {