From f7b8d5553e595b54bb58685ef023c8b100fd69b8 Mon Sep 17 00:00:00 2001 From: SahilRakhaiya05 Date: Thu, 25 Jun 2026 14:48:40 +0530 Subject: [PATCH] fix(test): reject empty code get --out and strip code-file BOM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When test code get --out receives an empty inline body, closeOutputFile left a zero-byte file with exit 0 — scripts and agents treated that as a successful download. Abort the sink, unlink any artifact, and surface VALIDATION_ERROR instead. Plan/steps JSON already strip a leading UTF-8 BOM from PowerShell 5.1 files; apply the same strip to --code-file reads so uploaded test source is not corrupted by an invisible U+FEFF prefix. --- src/commands/test.test.ts | 62 +++++++++++++++++++++++++++++++++++++++ src/commands/test.ts | 44 ++++++++++++++++++++++++--- 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/src/commands/test.test.ts b/src/commands/test.test.ts index 6921d57..2c12438 100644 --- a/src/commands/test.test.ts +++ b/src/commands/test.test.ts @@ -1556,6 +1556,44 @@ describe('runCodeGet', () => { ), ).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 }); }); + + it('--out (text mode) rejects empty inline code with VALIDATION_ERROR and leaves no artifact', async () => { + const { credentialsPath } = makeCreds(); + const emptyCode: CliTestCode = { ...TEST_CODE_INLINE, code: '' }; + const fetchImpl = makeFetch(() => ({ body: emptyCode })); + const dir = mkdtempSync(join(tmpdir(), 'cli-test-code-empty-out-')); + const target = join(dir, 'empty.ts'); + await expect( + runCodeGet( + { + profile: 'default', + output: 'text', + debug: false, + testId: 'test_fe', + out: target, + }, + { credentialsPath, fetchImpl }, + ), + ).rejects.toMatchObject({ + code: 'VALIDATION_ERROR', + exitCode: 5, + details: expect.objectContaining({ field: 'out' }), + }); + expect(existsSync(target)).toBe(false); + }); + + it('text mode without --out still hints on stderr when inline code is empty', async () => { + const { credentialsPath } = makeCreds(); + const emptyCode: CliTestCode = { ...TEST_CODE_INLINE, code: '' }; + const fetchImpl = makeFetch(() => ({ body: emptyCode })); + const stderr: string[] = []; + const got = await runCodeGet( + { profile: 'default', output: 'text', debug: false, testId: 'test_fe' }, + { credentialsPath, fetchImpl, stderr: line => stderr.push(line) }, + ); + expect(got.code).toBe(''); + expect(stderr.join('\n')).toContain('no code generated yet'); + }); }); describe('runCodePut', () => { @@ -1610,6 +1648,30 @@ describe('runCodePut', () => { expect(sent.headers.get('content-type')).toBe('application/json'); }); + it('strips a UTF-8 BOM from --code-file before uploading (Windows PowerShell 5.1 default)', async () => { + const { credentialsPath } = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'cli-p4-bom-')); + const codeFile = join(dir, 'updated.spec.ts'); + writeFileSync(codeFile, '\uFEFF' + 'updated body', 'utf8'); + let seenBody: unknown; + const fetchImpl = makeFetch((_url, init) => { + seenBody = init.body ? JSON.parse(init.body as string) : undefined; + return { body: SAMPLE_RESPONSE }; + }); + await runCodePut( + { + profile: 'default', + output: 'json', + debug: false, + testId: 'test_alpha', + codeFile, + expectedVersion: 'v3', + }, + { credentialsPath, fetchImpl, stdout: () => undefined, stderr: () => undefined }, + ); + expect(seenBody).toEqual({ code: 'updated body' }); + }); + it('forwards --language in the body when set', async () => { const { credentialsPath } = makeCreds(); const codeFile = writeCodeFile('print("hi")'); diff --git a/src/commands/test.ts b/src/commands/test.ts index e254958..142fc49 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -1,4 +1,12 @@ -import { createWriteStream, readFileSync, readdirSync, statSync, type WriteStream } from 'node:fs'; +import { + createWriteStream, + existsSync, + readFileSync, + readdirSync, + statSync, + unlinkSync, + type WriteStream, +} from 'node:fs'; import { stat } from 'node:fs/promises'; import { dirname, extname, isAbsolute, join, resolve } from 'node:path'; import { randomUUID } from 'node:crypto'; @@ -879,7 +887,7 @@ function readCodeFileGuarded(path: string): string { function readCodeFile(path: string): string { try { - return readFileSync(resolveAbsolute(path), 'utf8'); + return stripBom(readFileSync(resolveAbsolute(path), 'utf8')); } catch (err) { const code = (err as NodeJS.ErrnoException).code; if (code === 'ENOENT') { @@ -3211,7 +3219,7 @@ export async function runCodeGet(opts: CodeGetOptions, deps: TestDeps = {}): Pro return code; } - const fileSink = opts.out !== undefined ? openOutputFile(opts.out) : null; + let fileSink = opts.out !== undefined ? openOutputFile(opts.out) : null; const out = fileSink ? makeFileOutput(opts.output, fileSink) : makeOutput(opts.output, deps); const client = makeClient(opts, deps); @@ -3234,13 +3242,28 @@ export async function runCodeGet(opts: CodeGetOptions, deps: TestDeps = {}): Pro // P2-10: draft test with no code yet — empty body would produce // silent empty stdout. Print a friendly hint to stderr instead so // the operator knows what happened, and keep exit 0. + // + // With `--out`, refuse to leave a zero-byte artifact behind: agents + // and scripts that check file size would otherwise treat exit 0 as + // a successful download. + if (fileSink) { + await abortOutputFile(fileSink); + fileSink = null; + throw localValidationError( + 'out', + 'test has no generated code yet — run the test first (refusing to write an empty --out file)', + ); + } const stderrFn = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); stderrFn('(no code generated yet — run the test first)'); } else { await out.writeChunk(code.code); } - if (fileSink) await closeOutputFile(fileSink); + if (fileSink) { + await closeOutputFile(fileSink); + fileSink = null; + } return code; } catch (err) { if (fileSink) await closeOutputFile(fileSink).catch(() => undefined); @@ -7842,6 +7865,19 @@ function closeOutputFile(sink: FileSink): Promise { }); } +/** Tear down an opened `--out` sink without leaving a zero-byte artifact. */ +function abortOutputFile(sink: FileSink): Promise { + return new Promise(resolve => { + sink.stream.destroy(); + try { + if (existsSync(sink.path)) unlinkSync(sink.path); + } catch { + // Best-effort cleanup — the validation error is the primary signal. + } + resolve(); + }); +} + /** A presigned `code` body is any `https://` URL — never anything else. */ export function isPresignedCodeUrl(code: string): boolean { return code.startsWith('https://');