diff --git a/src/commands/test.test.ts b/src/commands/test.test.ts index 11ec49e..79342eb 100644 --- a/src/commands/test.test.ts +++ b/src/commands/test.test.ts @@ -10,7 +10,7 @@ import { } from 'node:fs'; import { execFileSync, spawn } from 'node:child_process'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { join, relative } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { Command } from 'commander'; import type { RunResponse } from '../lib/runs.types.js'; @@ -3409,6 +3409,55 @@ describe('runScaffold', () => { await expect(runScaffold({ ...opts, force: true }, deps)).resolves.toBeDefined(); }); + // A trailing separator means "a directory", whether the path is relative + // (path.resolve drops the separator) or a native Windows path ending in `\`. + // Neither may be written as a FILE named after the intended directory. + it.each([ + [ + 'relative path ending in /', + (dir: string) => `${relative(process.cwd(), join(dir, 'plans'))}/`, + ], + ['absolute path ending in \\', (dir: string) => `${join(dir, 'plans')}\\`], + ])('--out given a %s rejects and writes nothing', async (_label, outFor) => { + const dir = mkdtempSync(join(tmpdir(), 'cli-scaffold-sep-')); + await expect( + runScaffold( + { + profile: 'default', + output: 'text', + debug: false, + scaffoldType: 'frontend', + out: outFor(dir), + force: false, + }, + { stdout: () => undefined, stderr: () => undefined, env: {} }, + ), + ).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 }); + expect(readdirSync(dir)).toEqual([]); + rmSync(dir, { recursive: true, force: true }); + }); + + it('--out naming an existing directory with a trailing separator says it is a directory, not that it exists', async () => { + const dir = mkdtempSync(join(tmpdir(), 'cli-scaffold-sepdir-')); + await expect( + runScaffold( + { + profile: 'default', + output: 'text', + debug: false, + scaffoldType: 'frontend', + out: `${dir}/`, + force: false, + }, + { stdout: () => undefined, stderr: () => undefined, env: {} }, + ), + ).rejects.toMatchObject({ + exitCode: 5, + nextAction: expect.stringContaining('must point to a file, not a directory'), + }); + rmSync(dir, { recursive: true, force: true }); + }); + it('--out pointing at an existing path (here a directory) rejects without --force', async () => { const dir = mkdtempSync(join(tmpdir(), 'cli-scaffold-dir-')); await expect( diff --git a/src/commands/test.ts b/src/commands/test.ts index 8bf5ce5..77bddcc 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -6237,6 +6237,8 @@ export async function runScaffold( if (opts.out !== undefined) { const resolved = isAbsolute(opts.out) ? opts.out : resolve(process.cwd(), opts.out); + // Before the overwrite check: `--out existing-dir/` is a directory, not a file that already exists. + assertOutIsFilePath(opts.out); // Never clobber silently: scaffolds are starting points the user edits, so // an accidental re-run must not erase their work. --force opts in. if (!opts.force && existsSync(resolved)) { @@ -16461,6 +16463,16 @@ interface FileSink { * `resolved` directly means the real `--out` file is never truncated * up front, see `closeOutputFile` for the commit step. */ +/** + * Check the RAW path: `resolve()` drops a trailing separator, and on + * Windows the separator is usually `\`. + */ +function assertOutIsFilePath(rawPath: string): void { + if (/[\\/]$/.test(rawPath)) { + throw localValidationError('out', 'must point to a file, not a directory'); + } +} + function openOutputFile(rawPath: string): FileSink { if (typeof rawPath !== 'string' || rawPath.length === 0) { throw localValidationError('out', 'must be a non-empty file path'); @@ -16469,9 +16481,7 @@ function openOutputFile(rawPath: string): FileSink { // Defensive: reject obviously-bad paths up front (a directory string // would fail later with EISDIR; that's a clearer 5/VALIDATION_ERROR // surface than letting it crash mid-write with TransportError). - if (resolved.endsWith('/')) { - throw localValidationError('out', 'must point to a file, not a directory'); - } + assertOutIsFilePath(rawPath); // Validate the parent dir synchronously so a missing or non-directory // parent surfaces as exit 5 / VALIDATION_ERROR rather than exit 1 / // TRANSPORT_ERROR. Without this, an ENOENT/ENOTDIR fires asynchronously diff --git a/src/lib/junit-report.test.ts b/src/lib/junit-report.test.ts index 6ccdce6..6e30e02 100644 --- a/src/lib/junit-report.test.ts +++ b/src/lib/junit-report.test.ts @@ -1,6 +1,6 @@ -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { join, relative } from 'node:path'; import { describe, expect, it } from 'vitest'; import { ApiError } from './errors.js'; import { @@ -402,6 +402,20 @@ describe('writeJUnitReportFile', () => { }); }); + it.each(['/', '\\'])( + 'rejects a relative directory path ending in %s and writes nothing', + async sep => { + const dir = mkdtempSync(join(tmpdir(), 'junit-report-file-sep-')); + const target = `${relative(process.cwd(), join(dir, 'reports'))}${sep}`; + await expect(writeJUnitReportFile(target, '')).rejects.toMatchObject({ + code: 'VALIDATION_ERROR', + exitCode: 5, + }); + expect(readdirSync(dir)).toEqual([]); + rmSync(dir, { recursive: true, force: true }); + }, + ); + it('rejects parent that is a file', async () => { const dir = mkdtempSync(join(tmpdir(), 'junit-report-file-parent-')); const parentFile = join(dir, 'not-a-dir'); diff --git a/src/lib/junit-report.ts b/src/lib/junit-report.ts index f523cd3..b90c862 100644 --- a/src/lib/junit-report.ts +++ b/src/lib/junit-report.ts @@ -272,7 +272,8 @@ async function assertReportFileParent(rawPath: string): Promise { throw localValidationError('report-file', 'must be a non-empty file path'); } const resolved = isAbsolute(rawPath) ? rawPath : resolve(process.cwd(), rawPath); - if (resolved.endsWith('/') || resolved.endsWith('\\')) { + // Check the RAW path: `resolve()` drops a trailing separator. + if (/[\\/]$/.test(rawPath)) { throw localValidationError('report-file', 'must point to a file, not a directory'); }