From 743309efb6ebffffc485538956db05e699bd253f Mon Sep 17 00:00:00 2001 From: ronkenx9 <140641518+ronkenx9@users.noreply.github.com> Date: Tue, 13 Oct 2026 23:31:30 +0100 Subject: [PATCH 1/3] fix(test): check --report-file and --summary-file paths before dispatching a run test run (single and --all) and testlist run now refuse an empty path, a missing or non-directory parent, or a directory target with exit 5 / VALIDATION_ERROR before anything is triggered, instead of failing only after a billed batch has finished. The checks move out of assertReportFileParent into a shared assertArtifactPath helper; write-time handling and the best-effort end-of-run --summary-file write are unchanged. Closes #455 --- src/commands/test.run.spec.ts | 57 +++++++++++++++++++++++++++++++++++ src/commands/test.ts | 18 +++++++++++ src/commands/testlist.spec.ts | 24 ++++++++++++++- src/commands/testlist.ts | 5 +++ src/lib/junit-report.test.ts | 35 ++++++++++++++++++++- src/lib/junit-report.ts | 32 +++++++++++++------- 6 files changed, 158 insertions(+), 13 deletions(-) diff --git a/src/commands/test.run.spec.ts b/src/commands/test.run.spec.ts index f41e5e93..90e029b8 100644 --- a/src/commands/test.run.spec.ts +++ b/src/commands/test.run.spec.ts @@ -710,6 +710,34 @@ describe('runTestRun — with --wait', () => { expect(printed.status).toBe('passed'); }); + it('a --summary-file in a missing directory is refused before the run is triggered', async () => { + const { credentialsPath } = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'cli-single-preflight-')); + const calls: Array<{ url: string; method: string }> = []; + const fetchImpl = makeFetch((url, init) => { + calls.push({ url, method: init?.method ?? 'GET' }); + return { body: TRIGGER_RESP }; + }); + await expect( + runTestRun( + { + profile: 'default', + output: 'json', + debug: false, + dryRun: false, + testId: 'test_xyz', + wait: true, + timeoutSeconds: 60, + summaryFile: join(dir, 'no-such-subdir', 'summary.json'), + }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {}, sleep: instantSleep }, + ), + ).rejects.toMatchObject({ exitCode: 5, details: { field: 'summary-file' } }); + // Read-only target resolution may run first; the run itself is never triggered. + expect(calls.filter(c => c.method !== 'GET')).toEqual([]); + rmSync(dir, { recursive: true, force: true }); + }); + it('wait path — failed status → throws CLIError with exitCode 1', async () => { const { credentialsPath } = makeCreds(); const fetchImpl = makeFetch(url => { @@ -5627,6 +5655,35 @@ describe('runTestRunAll — zero-dispatch fails the CI gate', () => { expect(JSON.parse(readFileSync(summaryFile, 'utf8')).passed).toBe(0); }); + it.each([ + ['--report-file', ['--report', 'junit', '--report-file'], 'junit.xml', 'report-file'], + ['--summary-file', ['--summary-file'], 'summary.json', 'summary-file'], + ])( + 'test run --all: a %s in a missing directory is refused before anything is dispatched', + async (_flag, flags, file, field) => { + vi.resetModules(); + const { createTestCommand } = await import('./test.js'); + const { credentialsPath } = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'cli-preflight-')); + const fetchImpl = vi.fn(makeFetch(() => ({ body: EMPTY_BATCH }))); + const command = createTestCommand({ + credentialsPath, + fetchImpl, + stdout: () => {}, + stderr: () => {}, + sleep: () => Promise.resolve(), + }); + await expect( + command.parseAsync( + ['run', '--all', '--project', 'P', '--wait', ...flags, join(dir, 'no-such-subdir', file)], + { from: 'user' }, + ), + ).rejects.toMatchObject({ exitCode: 5, details: { field } }); + expect(fetchImpl).not.toHaveBeenCalled(); + rmSync(dir, { recursive: true, force: true }); + }, + ); + it('--allow-empty does NOT green a dispatched-but-failed run', async () => { const { credentialsPath } = makeCreds(); const acceptedBatch: BatchRunFreshResponse = { diff --git a/src/commands/test.ts b/src/commands/test.ts index ffab7a82..1a484d1d 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -57,6 +57,7 @@ import { resolveBatchReportProjectId, skippedJUnitResultsFromSummary, writeJUnitReportFile, + assertArtifactPath, type JUnitReportFormat, parseJUnitReportFormat, type JUnitTestResult, @@ -8094,6 +8095,17 @@ function assertFreshRunWaitOptions( ); } +/** Pre-dispatch: refuse unwritable --report-file / --summary-file before anything is triggered. */ +function assertRunArtifactPaths(opts: { + report?: JUnitReportFormat; + reportFile?: string; + summaryFile?: string; +}): void { + if (opts.report === 'junit' && opts.reportFile !== undefined) + assertArtifactPath('report-file', opts.reportFile); + if (opts.summaryFile !== undefined) assertArtifactPath('summary-file', opts.summaryFile); +} + function isSameLoopbackPort(url: string, port: number): boolean { try { return parseLoopbackTargetUrl(url)?.port === port; @@ -8330,6 +8342,7 @@ export async function runTestRun( client = makeClient(clientOpts, deps); } assertFreshRunWaitOptions(opts, false); + assertRunArtifactPaths(opts); const tunnelRequestTimeoutMs = isTunnelRun ? resolveRequestTimeoutMs(clientOpts, deps.env ?? process.env) : undefined; @@ -15710,6 +15723,11 @@ export function createTestCommand(deps: TestDeps = {}): Command { '--summary-file requires --wait (it reduces the terminal run result). Add --wait.', ); } + assertRunArtifactPaths({ + report, + reportFile: cmdOpts.reportFile, + summaryFile: cmdOpts.summaryFile, + }); if (isAll && !usingLocal && cmdOpts.tunnelClient === undefined) { // --all path: wave-ordered fresh batch run. diff --git a/src/commands/testlist.spec.ts b/src/commands/testlist.spec.ts index fc1b698c..7e861ba9 100644 --- a/src/commands/testlist.spec.ts +++ b/src/commands/testlist.spec.ts @@ -1,7 +1,7 @@ import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { runTestlistAdd, runTestlistCreate, @@ -668,6 +668,28 @@ describe('testlist run', () => { expect(err.exitCode).toBe(6); }); + it.each([ + ['--report-file', { report: 'junit', reportFile: 'missing/report.xml' }, 'report-file'], + ['--summary-file', { summaryFile: 'missing/summary.json' }, 'summary-file'], + ])( + 'a %s in a missing directory is refused before the list is dispatched', + async (_flag, over, field) => { + const { env } = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'cli-testlist-preflight-')); + const fetchImpl = vi.fn(makeFetch(() => ({ body: { accepted: [] } }))); + const paths = Object.fromEntries( + Object.entries(over).map(([k, v]) => [k, k === 'report' ? v : join(dir, v as string)]), + ); + const err = (await runTestlistRun( + { ...runBase, listId: 'list-1', wait: true, ...paths } as never, + { env, fetchImpl, stdout: () => {}, stderr: () => {}, sleep: instantSleep }, + ).catch(e => e)) as { exitCode: number; details?: { field?: string } }; + expect(err.exitCode).toBe(5); + expect(err.details?.field).toBe(field); + expect(fetchImpl).not.toHaveBeenCalled(); + }, + ); + it('gh-output: an all-conflict --wait run emits annotations + summary before exiting 6', async () => { const { env } = makeCreds(); const dir = mkdtempSync(join(tmpdir(), 'cli-testlist-conflict-')); diff --git a/src/commands/testlist.ts b/src/commands/testlist.ts index fe65ac7f..ec0160ad 100644 --- a/src/commands/testlist.ts +++ b/src/commands/testlist.ts @@ -55,6 +55,7 @@ import { parseJUnitReportFormat, skippedJUnitResultsFromSummary, writeJUnitReportFile, + assertArtifactPath, type JUnitReportFormat, type JUnitTestResult, } from '../lib/junit-report.js'; @@ -536,6 +537,10 @@ function validateTestlistRunOptions(opts: RunTestlistRunOptions): void { '--summary-file requires --wait (it reduces the terminal run result). Add --wait.', ); } + if (opts.report === 'junit' && opts.reportFile !== undefined) { + assertArtifactPath('report-file', opts.reportFile); + } + if (opts.summaryFile !== undefined) assertArtifactPath('summary-file', opts.summaryFile); } export async function runTestlistRun( diff --git a/src/lib/junit-report.test.ts b/src/lib/junit-report.test.ts index 5e88eaad..3e2ff827 100644 --- a/src/lib/junit-report.test.ts +++ b/src/lib/junit-report.test.ts @@ -1,9 +1,10 @@ -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; import { ApiError } from './errors.js'; import { + assertArtifactPath, assertJUnitReportOptions, buildJUnitReport, durationSecondsBetween, @@ -690,3 +691,35 @@ describe('per-row classname for batches spanning projects', () => { ); }); }); + +describe('assertArtifactPath', () => { + it('returns the resolved path when the parent directory exists', () => { + const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); + expect(assertArtifactPath('summary-file', join(dir, 'summary.json'))).toBe( + join(dir, 'summary.json'), + ); + rmSync(dir, { recursive: true, force: true }); + }); + + it('refuses a missing parent directory with the flag name', () => { + const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); + expect(() => assertArtifactPath('summary-file', join(dir, 'nope', 's.json'))).toThrow(ApiError); + try { + assertArtifactPath('report-file', join(dir, 'nope', 'r.xml')); + } catch (err) { + expect((err as ApiError).details).toMatchObject({ field: 'report-file' }); + } + rmSync(dir, { recursive: true, force: true }); + }); + + it('refuses an existing directory as the target', () => { + const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); + mkdirSync(join(dir, 'sub')); + expect(() => assertArtifactPath('report-file', join(dir, 'sub'))).toThrow(ApiError); + rmSync(dir, { recursive: true, force: true }); + }); + + it('refuses an empty path', () => { + expect(() => assertArtifactPath('summary-file', '')).toThrow(ApiError); + }); +}); diff --git a/src/lib/junit-report.ts b/src/lib/junit-report.ts index 08f61211..8a75bc35 100644 --- a/src/lib/junit-report.ts +++ b/src/lib/junit-report.ts @@ -1,5 +1,5 @@ -import { createWriteStream } from 'node:fs'; -import { rename, stat, unlink } from 'node:fs/promises'; +import { createWriteStream, statSync } from 'node:fs'; +import { rename, unlink } from 'node:fs/promises'; import { basename, dirname, isAbsolute, join, resolve } from 'node:path'; import { randomUUID } from 'node:crypto'; import { localValidationError, TransportError } from './errors.js'; @@ -350,34 +350,44 @@ export function skippedJUnitResultsFromSummary( .map(row => ({ testId: row.testId, status: 'skipped', skipReason: row.error ?? row.status })); } -async function assertReportFileParent(rawPath: string): Promise { +/** + * Check that an output artifact path (`--report-file`, `--summary-file`) can be + * written: non-empty, not a directory, and inside an existing directory. + * Returns the resolved absolute path. + * + * Called in each command's pre-flight, before anything is dispatched or billed, + * so a mistyped directory fails fast with exit 5 instead of surfacing only after + * a full batch has run. The write itself re-checks (the directory can vanish in + * between). + */ +export function assertArtifactPath(field: 'report-file' | 'summary-file', rawPath: string): string { if (typeof rawPath !== 'string' || rawPath.length === 0) { - throw localValidationError('report-file', 'must be a non-empty file path'); + throw localValidationError(field, 'must be a non-empty file path'); } const resolved = isAbsolute(rawPath) ? rawPath : resolve(process.cwd(), rawPath); if (resolved.endsWith('/') || resolved.endsWith('\\')) { - throw localValidationError('report-file', 'must point to a file, not a directory'); + throw localValidationError(field, 'must point to a file, not a directory'); } const parent = dirname(resolved); let parentStat; try { - parentStat = await stat(parent); + parentStat = statSync(parent); } catch { - throw localValidationError('report-file', `parent directory does not exist: ${parent}`); + throw localValidationError(field, `parent directory does not exist: ${parent}`); } if (!parentStat.isDirectory()) { - throw localValidationError('report-file', `parent path is not a directory: ${parent}`); + throw localValidationError(field, `parent path is not a directory: ${parent}`); } let targetStat; try { - targetStat = await stat(resolved); + targetStat = statSync(resolved); } catch { return resolved; } if (targetStat.isDirectory()) { - throw localValidationError('report-file', `must point to a file, not a directory: ${resolved}`); + throw localValidationError(field, `must point to a file, not a directory: ${resolved}`); } return resolved; } @@ -386,7 +396,7 @@ async function assertReportFileParent(rawPath: string): Promise { * Atomically write JUnit XML to `--report-file` (temp sibling + rename). */ export async function writeJUnitReportFile(rawPath: string, xml: string): Promise { - const resolved = await assertReportFileParent(rawPath); + const resolved = assertArtifactPath('report-file', rawPath); const parent = dirname(resolved); const tmpPath = join(parent, `.${basename(resolved)}.tmp-${randomUUID()}`); From 1a8554368825eb3be2fcb4d1cc4fd06a0b144da2 Mon Sep 17 00:00:00 2001 From: ronkenx9 Date: Thu, 22 Oct 2026 01:19:10 +0100 Subject: [PATCH 2/3] fix(test): only treat ENOENT/ENOTDIR as a missing artifact path The pre-flight treated every stat error as "does not exist": EACCES on the parent was reported as a missing directory, and EACCES/ELOOP on the target passed as "not created yet", so the failure resurfaced at write time after the run was dispatched. Report other codes as a validation error naming the cause. Tests cover a symlink loop and an unreadable parent, and the missing-parent test now fails if nothing is thrown. --- src/lib/junit-report.test.ts | 58 ++++++++++++++++++++++++++++++++---- src/lib/junit-report.ts | 25 +++++++++++++--- 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/lib/junit-report.test.ts b/src/lib/junit-report.test.ts index 3e2ff827..5117d215 100644 --- a/src/lib/junit-report.test.ts +++ b/src/lib/junit-report.test.ts @@ -1,4 +1,12 @@ -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { + chmodSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; @@ -704,14 +712,52 @@ describe('assertArtifactPath', () => { it('refuses a missing parent directory with the flag name', () => { const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); expect(() => assertArtifactPath('summary-file', join(dir, 'nope', 's.json'))).toThrow(ApiError); - try { - assertArtifactPath('report-file', join(dir, 'nope', 'r.xml')); - } catch (err) { - expect((err as ApiError).details).toMatchObject({ field: 'report-file' }); - } + expect(() => assertArtifactPath('report-file', join(dir, 'nope', 'r.xml'))).toThrow( + expect.objectContaining({ details: expect.objectContaining({ field: 'report-file' }) }), + ); rmSync(dir, { recursive: true, force: true }); }); + it('reports an unreadable target instead of treating it as absent', () => { + const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); + // a <-> b symlink loop: stat fails with ELOOP, which must not pass as "file not created yet" + symlinkSync(join(dir, 'b'), join(dir, 'a')); + symlinkSync(join(dir, 'a'), join(dir, 'b')); + expect(() => assertArtifactPath('report-file', join(dir, 'a'))).toThrow( + expect.objectContaining({ + details: expect.objectContaining({ + field: 'report-file', + reason: expect.stringMatching(/^cannot access .*ELOOP$/), + }), + }), + ); + rmSync(dir, { recursive: true, force: true }); + }); + + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'reports a permission error on the parent instead of "does not exist"', + () => { + const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); + mkdirSync(join(dir, 'locked', 'out'), { recursive: true }); + chmodSync(join(dir, 'locked'), 0o000); + try { + expect(() => + assertArtifactPath('summary-file', join(dir, 'locked', 'out', 's.json')), + ).toThrow( + expect.objectContaining({ + details: expect.objectContaining({ + field: 'summary-file', + reason: expect.stringMatching(/^cannot access parent directory .*EACCES$/), + }), + }), + ); + } finally { + chmodSync(join(dir, 'locked'), 0o755); + rmSync(dir, { recursive: true, force: true }); + } + }, + ); + it('refuses an existing directory as the target', () => { const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); mkdirSync(join(dir, 'sub')); diff --git a/src/lib/junit-report.ts b/src/lib/junit-report.ts index 8a75bc35..6b0ca417 100644 --- a/src/lib/junit-report.ts +++ b/src/lib/junit-report.ts @@ -360,6 +360,15 @@ export function skippedJUnitResultsFromSummary( * a full batch has run. The write itself re-checks (the directory can vanish in * between). */ +function errorCode(err: unknown): string { + return (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown error'; +} + +function isAbsentPathError(err: unknown): boolean { + const code = errorCode(err); + return code === 'ENOENT' || code === 'ENOTDIR'; +} + export function assertArtifactPath(field: 'report-file' | 'summary-file', rawPath: string): string { if (typeof rawPath !== 'string' || rawPath.length === 0) { throw localValidationError(field, 'must be a non-empty file path'); @@ -373,8 +382,13 @@ export function assertArtifactPath(field: 'report-file' | 'summary-file', rawPat let parentStat; try { parentStat = statSync(parent); - } catch { - throw localValidationError(field, `parent directory does not exist: ${parent}`); + } catch (err) { + if (isAbsentPathError(err)) + throw localValidationError(field, `parent directory does not exist: ${parent}`); + throw localValidationError( + field, + `cannot access parent directory ${parent}: ${errorCode(err)}`, + ); } if (!parentStat.isDirectory()) { throw localValidationError(field, `parent path is not a directory: ${parent}`); @@ -383,8 +397,11 @@ export function assertArtifactPath(field: 'report-file' | 'summary-file', rawPat let targetStat; try { targetStat = statSync(resolved); - } catch { - return resolved; + } catch (err) { + // Only a missing target is fine (it will be created); anything else (EACCES, ELOOP...) would fail the + // write after the run was dispatched, which is exactly what this pre-flight exists to prevent. + if (isAbsentPathError(err)) return resolved; + throw localValidationError(field, `cannot access ${resolved}: ${errorCode(err)}`); } if (targetStat.isDirectory()) { throw localValidationError(field, `must point to a file, not a directory: ${resolved}`); From 4554de3ec0a6699bbbf305a8b96906accec1cd50 Mon Sep 17 00:00:00 2001 From: ronkenx9 <140641518+ronkenx9@users.noreply.github.com> Date: Tue, 10 Nov 2026 09:56:33 +0100 Subject: [PATCH 3/3] fix(test): retain raw artifact path guard after preflight rebase --- src/commands/testlist.spec.ts | 15 +++++++++++++++ src/lib/junit-report.test.ts | 20 ++++++++++++++++++++ src/lib/junit-report.ts | 24 +++++++++++++----------- 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/commands/testlist.spec.ts b/src/commands/testlist.spec.ts index 7e861ba9..0f9a124c 100644 --- a/src/commands/testlist.spec.ts +++ b/src/commands/testlist.spec.ts @@ -690,6 +690,21 @@ describe('testlist run', () => { }, ); + it('refuses --report-file reports/ before dispatch', async () => { + const { env } = makeCreds(); + const fetchImpl = vi.fn(makeFetch(() => ({ body: { accepted: [] } }))); + const err = (await runTestlistRun( + { ...runBase, listId: 'list-1', wait: true, report: 'junit', reportFile: 'reports/' }, + { env, fetchImpl, stdout: () => {}, stderr: () => {}, sleep: instantSleep }, + ).catch(e => e)) as ApiError; + expect(err.exitCode).toBe(5); + expect(err.details).toMatchObject({ + field: 'report-file', + reason: 'must point to a file, not a directory', + }); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + it('gh-output: an all-conflict --wait run emits annotations + summary before exiting 6', async () => { const { env } = makeCreds(); const dir = mkdtempSync(join(tmpdir(), 'cli-testlist-conflict-')); diff --git a/src/lib/junit-report.test.ts b/src/lib/junit-report.test.ts index 5117d215..2c4c7029 100644 --- a/src/lib/junit-report.test.ts +++ b/src/lib/junit-report.test.ts @@ -721,7 +721,9 @@ describe('assertArtifactPath', () => { it('reports an unreadable target instead of treating it as absent', () => { const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); // a <-> b symlink loop: stat fails with ELOOP, which must not pass as "file not created yet" + // eslint-disable-next-line security/detect-non-literal-fs-filename -- create a symlink only inside this test's fresh temporary directory. symlinkSync(join(dir, 'b'), join(dir, 'a')); + // eslint-disable-next-line security/detect-non-literal-fs-filename -- complete the loop inside the same isolated temporary directory. symlinkSync(join(dir, 'a'), join(dir, 'b')); expect(() => assertArtifactPath('report-file', join(dir, 'a'))).toThrow( expect.objectContaining({ @@ -738,7 +740,9 @@ describe('assertArtifactPath', () => { 'reports a permission error on the parent instead of "does not exist"', () => { const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); + // eslint-disable-next-line security/detect-non-literal-fs-filename -- create the permission fixture inside this test's fresh temporary directory. mkdirSync(join(dir, 'locked', 'out'), { recursive: true }); + // eslint-disable-next-line security/detect-non-literal-fs-filename -- restrict only the temporary fixture directory and restore it in finally. chmodSync(join(dir, 'locked'), 0o000); try { expect(() => @@ -752,6 +756,7 @@ describe('assertArtifactPath', () => { }), ); } finally { + // eslint-disable-next-line security/detect-non-literal-fs-filename -- restore permissions on the temporary fixture before cleanup. chmodSync(join(dir, 'locked'), 0o755); rmSync(dir, { recursive: true, force: true }); } @@ -760,6 +765,7 @@ describe('assertArtifactPath', () => { it('refuses an existing directory as the target', () => { const dir = mkdtempSync(join(tmpdir(), 'cli-artifact-')); + // eslint-disable-next-line security/detect-non-literal-fs-filename -- make a target-directory fixture inside this test's fresh temporary directory. mkdirSync(join(dir, 'sub')); expect(() => assertArtifactPath('report-file', join(dir, 'sub'))).toThrow(ApiError); rmSync(dir, { recursive: true, force: true }); @@ -768,4 +774,18 @@ describe('assertArtifactPath', () => { it('refuses an empty path', () => { expect(() => assertArtifactPath('summary-file', '')).toThrow(ApiError); }); + + it.each(['reports/', 'reports\\'])( + 'refuses a raw trailing separator before resolving %s', + path => { + expect(() => assertArtifactPath('report-file', path)).toThrow( + expect.objectContaining({ + details: expect.objectContaining({ + field: 'report-file', + reason: 'must point to a file, not a directory', + }), + }), + ); + }, + ); }); diff --git a/src/lib/junit-report.ts b/src/lib/junit-report.ts index 6b0ca417..dc89636a 100644 --- a/src/lib/junit-report.ts +++ b/src/lib/junit-report.ts @@ -350,6 +350,15 @@ export function skippedJUnitResultsFromSummary( .map(row => ({ testId: row.testId, status: 'skipped', skipReason: row.error ?? row.status })); } +function errorCode(err: unknown): string { + return (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown error'; +} + +function isAbsentPathError(err: unknown): boolean { + const code = errorCode(err); + return code === 'ENOENT' || code === 'ENOTDIR'; +} + /** * Check that an output artifact path (`--report-file`, `--summary-file`) can be * written: non-empty, not a directory, and inside an existing directory. @@ -360,27 +369,19 @@ export function skippedJUnitResultsFromSummary( * a full batch has run. The write itself re-checks (the directory can vanish in * between). */ -function errorCode(err: unknown): string { - return (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown error'; -} - -function isAbsentPathError(err: unknown): boolean { - const code = errorCode(err); - return code === 'ENOENT' || code === 'ENOTDIR'; -} - export function assertArtifactPath(field: 'report-file' | 'summary-file', rawPath: string): string { if (typeof rawPath !== 'string' || rawPath.length === 0) { throw localValidationError(field, 'must be a non-empty file path'); } - const resolved = isAbsolute(rawPath) ? rawPath : resolve(process.cwd(), rawPath); - if (resolved.endsWith('/') || resolved.endsWith('\\')) { + if (/[\\/]$/.test(rawPath)) { throw localValidationError(field, 'must point to a file, not a directory'); } + const resolved = isAbsolute(rawPath) ? rawPath : resolve(process.cwd(), rawPath); const parent = dirname(resolved); let parentStat; try { + // eslint-disable-next-line security/detect-non-literal-fs-filename -- inspect the user-selected artifact parent before dispatch; no file content is read. parentStat = statSync(parent); } catch (err) { if (isAbsentPathError(err)) @@ -396,6 +397,7 @@ export function assertArtifactPath(field: 'report-file' | 'summary-file', rawPat let targetStat; try { + // eslint-disable-next-line security/detect-non-literal-fs-filename -- inspect the user-selected artifact target to reject directories and inaccessible paths before dispatch. targetStat = statSync(resolved); } catch (err) { // Only a missing target is fine (it will be created); anything else (EACCES, ELOOP...) would fail the