From 1a2279e4ca17b382c16c829cff61cc3b7a4b933a Mon Sep 17 00:00:00 2001 From: Pugar Huda Mantoro Date: Sun, 18 Oct 2026 08:52:18 +0700 Subject: [PATCH 1/4] fix(test): reject --out / --report-file paths that end in a separator openOutputFile (test scaffold --out, test code get --out) and the JUnit --report-file guard checked the resolved path for a trailing separator. path.resolve() drops that separator for relative paths, and the --out guard only knew '/', so `--out plans/` or `--out C:\work\plans\` was written as a file named `plans` and reported as success. Check the raw argument for a trailing '/' or '\' instead. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011dsM5h73r1JTRLmWxK2ZZv --- src/commands/test.test.ts | 30 +++++++++++++++++++++++++++++- src/commands/test.ts | 4 +++- src/lib/junit-report.test.ts | 15 +++++++++++++-- src/lib/junit-report.ts | 3 ++- 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/commands/test.test.ts b/src/commands/test.test.ts index 11ec49e8..2a312ae1 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,34 @@ 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 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 8bf5ce5b..8627fd9c 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -16469,7 +16469,9 @@ 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('/')) { + // Check the RAW path: `resolve()` drops a trailing separator, and on + // Windows the separator is usually `\`. + if (/[\\/]$/.test(rawPath)) { throw localValidationError('out', 'must point to a file, not a directory'); } // Validate the parent dir synchronously so a missing or non-directory diff --git a/src/lib/junit-report.test.ts b/src/lib/junit-report.test.ts index 6ccdce69..7f1eda5d 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,17 @@ describe('writeJUnitReportFile', () => { }); }); + it('rejects a relative directory path ending in a separator and writes nothing', async () => { + const dir = mkdtempSync(join(tmpdir(), 'junit-report-file-sep-')); + const target = `${relative(process.cwd(), join(dir, 'reports'))}/`; + 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 f523cd36..b90c862e 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'); } From 6c53cad7ad4debb27896227a6bdd5527d899e2ef Mon Sep 17 00:00:00 2001 From: Pugar Huda Mantoro Date: Tue, 20 Oct 2026 10:28:15 +0700 Subject: [PATCH 2/4] test(report): assert the backslash case for --report-file too Co-Authored-By: Claude Opus 5.5 (1M context) --- src/lib/junit-report.test.ts | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/lib/junit-report.test.ts b/src/lib/junit-report.test.ts index 7f1eda5d..6e30e024 100644 --- a/src/lib/junit-report.test.ts +++ b/src/lib/junit-report.test.ts @@ -402,16 +402,19 @@ describe('writeJUnitReportFile', () => { }); }); - it('rejects a relative directory path ending in a separator and writes nothing', async () => { - const dir = mkdtempSync(join(tmpdir(), 'junit-report-file-sep-')); - const target = `${relative(process.cwd(), join(dir, 'reports'))}/`; - await expect(writeJUnitReportFile(target, '')).rejects.toMatchObject({ - code: 'VALIDATION_ERROR', - exitCode: 5, - }); - expect(readdirSync(dir)).toEqual([]); - rmSync(dir, { recursive: true, force: true }); - }); + 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-')); From 490051c5e52e1ccee5359dfa78372692895e067e Mon Sep 17 00:00:00 2001 From: Pugar Huda Mantoro Date: Tue, 20 Oct 2026 16:36:12 +0700 Subject: [PATCH 3/4] fix(test): check --out for a trailing separator before the overwrite check `--out existing-dir/` without --force said "already exists"; it now says the path must point to a file. Addresses review on #566. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/commands/test.test.ts | 11 +++++++++++ src/commands/test.ts | 18 +++++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/commands/test.test.ts b/src/commands/test.test.ts index 2a312ae1..32ec76f9 100644 --- a/src/commands/test.test.ts +++ b/src/commands/test.test.ts @@ -3437,6 +3437,17 @@ describe('runScaffold', () => { 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 8627fd9c..77bddcce 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,11 +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). - // Check the RAW path: `resolve()` drops a trailing separator, and on - // Windows the separator is usually `\`. - if (/[\\/]$/.test(rawPath)) { - 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 From 0b7f6a14b87b01b4c9077707f8c0bf2c47fa030e Mon Sep 17 00:00:00 2001 From: Pugar Huda Mantoro Date: Tue, 20 Oct 2026 16:37:45 +0700 Subject: [PATCH 4/4] style: prettier Co-Authored-By: Claude Opus 5.5 (1M context) --- src/commands/test.test.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/commands/test.test.ts b/src/commands/test.test.ts index 32ec76f9..79342ebf 100644 --- a/src/commands/test.test.ts +++ b/src/commands/test.test.ts @@ -3441,10 +3441,20 @@ describe('runScaffold', () => { const dir = mkdtempSync(join(tmpdir(), 'cli-scaffold-sepdir-')); await expect( runScaffold( - { profile: 'default', output: 'text', debug: false, scaffoldType: 'frontend', out: `${dir}/`, force: false }, + { + 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') }); + ).rejects.toMatchObject({ + exitCode: 5, + nextAction: expect.stringContaining('must point to a file, not a directory'), + }); rmSync(dir, { recursive: true, force: true }); });