From eb1d0ce8a4dade26d89a69a3c1b5235c54b87b36 Mon Sep 17 00:00:00 2001 From: nopp Date: Mon, 12 Oct 2026 17:22:29 +0700 Subject: [PATCH 1/3] fix(testlist): validate non-empty and non-whitespace IDs across testlist commands --- src/commands/testlist.spec.ts | 65 +++++++++++++++++++++++++++++++++++ src/commands/testlist.ts | 38 ++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/src/commands/testlist.spec.ts b/src/commands/testlist.spec.ts index 35d3a39b..fa7a0fbc 100644 --- a/src/commands/testlist.spec.ts +++ b/src/commands/testlist.spec.ts @@ -1053,6 +1053,71 @@ describe('testlist run — insufficient credits → exit 12 + telemetry facts', await runTestlistRun({ ...runBase, listId: 'list-1' }, { env, fetchImpl, stdout: () => {} }); expect(takeTelemetryExtras()).toEqual({ accepted: 1, conflicts: 0, deferred: 0, skipped: 0 }); }); + + describe('testlist input validation (empty and whitespace arguments)', () => { + it('get: rejects empty and whitespace list-id', async () => { + for (const invalidId of ['', ' ', '\t\n']) { + await expect(runTestlistGet({ ...base, listId: invalidId })).rejects.toThrow(ApiError); + await expect(runTestlistGet({ ...base, listId: invalidId, dryRun: true })).rejects.toThrow(ApiError); + try { + await runTestlistGet({ ...base, listId: invalidId }); + } catch (e: any) { + expect(e.exitCode).toBe(5); + expect(e.code).toBe('VALIDATION_ERROR'); + } + } + }); + + it('delete: rejects empty and whitespace list-id', async () => { + for (const invalidId of ['', ' ', '\t\n']) { + await expect(runTestlistDelete({ ...base, listId: invalidId, confirm: true })).rejects.toThrow(ApiError); + await expect(runTestlistDelete({ ...base, listId: invalidId, dryRun: true })).rejects.toThrow(ApiError); + try { + await runTestlistDelete({ ...base, listId: invalidId, confirm: true }); + } catch (e: any) { + expect(e.exitCode).toBe(5); + expect(e.code).toBe('VALIDATION_ERROR'); + } + } + }); + + it('update: rejects empty and whitespace list-id', async () => { + for (const invalidId of ['', ' ', '\t\n']) { + await expect(runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed' })).rejects.toThrow(ApiError); + await expect(runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed', dryRun: true })).rejects.toThrow(ApiError); + try { + await runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed' }); + } catch (e: any) { + expect(e.exitCode).toBe(5); + expect(e.code).toBe('VALIDATION_ERROR'); + } + } + }); + + it('add: rejects empty and whitespace list-id and test-ids', async () => { + await expect(runTestlistAdd({ ...base, listId: ' ', testIds: ['test-1'] })).rejects.toThrow(ApiError); + await expect(runTestlistAdd({ ...base, listId: 'list-1', testIds: [' '] })).rejects.toThrow(ApiError); + await expect(runTestlistAdd({ ...base, listId: 'list-1', testIds: ['test-1', ''] })).rejects.toThrow(ApiError); + await expect(runTestlistAdd({ ...base, listId: ' ', testIds: ['test-1'], dryRun: true })).rejects.toThrow(ApiError); + await expect(runTestlistAdd({ ...base, listId: 'list-1', testIds: [' '], dryRun: true })).rejects.toThrow(ApiError); + }); + + it('remove: rejects empty and whitespace list-id and test-ids', async () => { + await expect(runTestlistRemove({ ...base, listId: ' ', testIds: ['test-1'] })).rejects.toThrow(ApiError); + await expect(runTestlistRemove({ ...base, listId: 'list-1', testIds: [' '] })).rejects.toThrow(ApiError); + await expect(runTestlistRemove({ ...base, listId: 'list-1', testIds: ['test-1', ''] })).rejects.toThrow(ApiError); + await expect(runTestlistRemove({ ...base, listId: ' ', testIds: ['test-1'], dryRun: true })).rejects.toThrow(ApiError); + await expect(runTestlistRemove({ ...base, listId: 'list-1', testIds: [' '], dryRun: true })).rejects.toThrow(ApiError); + }); + + it('run: rejects empty and whitespace list-id and cases', async () => { + await expect(runTestlistRun({ ...runBase, listId: ' ' })).rejects.toThrow(ApiError); + await expect(runTestlistRun({ ...runBase, listId: ' ', dryRun: true })).rejects.toThrow(ApiError); + await expect(runTestlistRun({ ...runBase, listId: 'list-1', cases: [' '] })).rejects.toThrow(ApiError); + await expect(runTestlistRun({ ...runBase, listId: 'list-1', cases: ['valid-case', ''] })).rejects.toThrow(ApiError); + await expect(runTestlistRun({ ...runBase, listId: 'list-1', cases: [' '], dryRun: true })).rejects.toThrow(ApiError); + }); + }); }); it.each([null, '2026-08-01T00:00:05.000Z'])( diff --git a/src/commands/testlist.ts b/src/commands/testlist.ts index 2a17c370..4e888914 100644 --- a/src/commands/testlist.ts +++ b/src/commands/testlist.ts @@ -148,6 +148,17 @@ function dryRunStderr(deps: TestListDeps): (line: string) => void { return deps.stderr ?? (line => process.stderr.write(`${line}\n`)); } +function assertValidListId(listId: string | undefined): void { + if (listId === undefined || listId.trim() === '') { + throw localValidationError( + 'list-id', + 'is required and must not be empty or whitespace-only', + undefined, + 'field', + ); + } +} + // Canned `--dry-run` samples (offline, no network) — the global `--dry-run` // contract promises a shape-accurate sample for every command. const DRY_RUN_LIST: CliTestList = { @@ -192,6 +203,7 @@ export async function runTestlistGet( opts: CommonOptions & { listId: string }, deps: TestListDeps = {}, ): Promise { + assertValidListId(opts.listId); const out = makeOutput(opts.output, deps); if (opts.dryRun) { emitDryRunBanner(dryRunStderr(deps)); @@ -250,6 +262,7 @@ export async function runTestlistUpdate( }, deps: TestListDeps = {}, ): Promise { + assertValidListId(opts.listId); const out = makeOutput(opts.output, deps); const hasProjectEnv = opts.projectEnv !== undefined && opts.projectEnv.length > 0; if (opts.clearProjectEnv && hasProjectEnv) { @@ -292,6 +305,7 @@ export async function runTestlistDelete( opts: CommonOptions & { listId: string; confirm?: boolean; idempotencyKey?: string }, deps: TestListDeps = {}, ): Promise { + assertValidListId(opts.listId); const out = makeOutput(opts.output, deps); // Destructive: deleting a list also deletes its schedules (and reaps their // EventBridge triggers). Require explicit --confirm — matches the CLI's @@ -330,10 +344,21 @@ async function mutateCases( opts: CommonOptions & { listId: string; testIds: string[]; idempotencyKey?: string }, deps: TestListDeps, ): Promise { + assertValidListId(opts.listId); const out = makeOutput(opts.output, deps); if (opts.testIds.length === 0) { throw localValidationError('test-id', 'at least one test id is required', undefined, 'field'); } + for (const testId of opts.testIds) { + if (!testId || testId.trim() === '') { + throw localValidationError( + 'test-id', + 'test id must not be empty or whitespace-only', + undefined, + 'field', + ); + } + } if (opts.idempotencyKey !== undefined) assertIdempotencyKey(opts.idempotencyKey); const idempotencyKey = opts.idempotencyKey ?? `cli-testlist-${verb}-${randomUUID()}`; emitIdempotencyKey(opts, deps, idempotencyKey, opts.idempotencyKey); @@ -487,6 +512,19 @@ function assertNoNotFound(resp: CliTestListRunResponse): void { * JUnit XML report (`--report junit`). testlist run is V3-only. */ function validateTestlistRunOptions(opts: RunTestlistRunOptions): void { + assertValidListId(opts.listId); + if (opts.cases !== undefined) { + for (const caseId of opts.cases) { + if (!caseId || caseId.trim() === '') { + throw localValidationError( + 'case', + 'case id must not be empty or whitespace-only', + undefined, + 'flag', + ); + } + } + } if (opts.idempotencyKey !== undefined) assertIdempotencyKey(opts.idempotencyKey); if ( !Number.isInteger(opts.maxConcurrency) || From 0ab0ae9b55441eb7c93f638c1cdd73a78fcfd6ff Mon Sep 17 00:00:00 2001 From: nopp Date: Mon, 12 Oct 2026 17:30:07 +0700 Subject: [PATCH 2/3] fix(testlist): use unknown type in testlist.spec.ts catch blocks --- src/commands/testlist.spec.ts | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/commands/testlist.spec.ts b/src/commands/testlist.spec.ts index fa7a0fbc..c8fa53d8 100644 --- a/src/commands/testlist.spec.ts +++ b/src/commands/testlist.spec.ts @@ -1061,9 +1061,10 @@ describe('testlist run — insufficient credits → exit 12 + telemetry facts', await expect(runTestlistGet({ ...base, listId: invalidId, dryRun: true })).rejects.toThrow(ApiError); try { await runTestlistGet({ ...base, listId: invalidId }); - } catch (e: any) { - expect(e.exitCode).toBe(5); - expect(e.code).toBe('VALIDATION_ERROR'); + } catch (e: unknown) { + const err = e as ApiError; + expect(err.exitCode).toBe(5); + expect(err.code).toBe('VALIDATION_ERROR'); } } }); @@ -1074,9 +1075,10 @@ describe('testlist run — insufficient credits → exit 12 + telemetry facts', await expect(runTestlistDelete({ ...base, listId: invalidId, dryRun: true })).rejects.toThrow(ApiError); try { await runTestlistDelete({ ...base, listId: invalidId, confirm: true }); - } catch (e: any) { - expect(e.exitCode).toBe(5); - expect(e.code).toBe('VALIDATION_ERROR'); + } catch (e: unknown) { + const err = e as ApiError; + expect(err.exitCode).toBe(5); + expect(err.code).toBe('VALIDATION_ERROR'); } } }); @@ -1087,9 +1089,10 @@ describe('testlist run — insufficient credits → exit 12 + telemetry facts', await expect(runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed', dryRun: true })).rejects.toThrow(ApiError); try { await runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed' }); - } catch (e: any) { - expect(e.exitCode).toBe(5); - expect(e.code).toBe('VALIDATION_ERROR'); + } catch (e: unknown) { + const err = e as ApiError; + expect(err.exitCode).toBe(5); + expect(err.code).toBe('VALIDATION_ERROR'); } } }); From 2aa7cd814a65890a9148e3f086bffcaa549e782e Mon Sep 17 00:00:00 2001 From: nopp Date: Mon, 12 Oct 2026 17:32:09 +0700 Subject: [PATCH 3/3] style(testlist): format testlist.spec.ts with prettier --- src/commands/testlist.spec.ts | 76 ++++++++++++++++++++++++++--------- 1 file changed, 57 insertions(+), 19 deletions(-) diff --git a/src/commands/testlist.spec.ts b/src/commands/testlist.spec.ts index c8fa53d8..878d4878 100644 --- a/src/commands/testlist.spec.ts +++ b/src/commands/testlist.spec.ts @@ -1058,7 +1058,9 @@ describe('testlist run — insufficient credits → exit 12 + telemetry facts', it('get: rejects empty and whitespace list-id', async () => { for (const invalidId of ['', ' ', '\t\n']) { await expect(runTestlistGet({ ...base, listId: invalidId })).rejects.toThrow(ApiError); - await expect(runTestlistGet({ ...base, listId: invalidId, dryRun: true })).rejects.toThrow(ApiError); + await expect(runTestlistGet({ ...base, listId: invalidId, dryRun: true })).rejects.toThrow( + ApiError, + ); try { await runTestlistGet({ ...base, listId: invalidId }); } catch (e: unknown) { @@ -1071,8 +1073,12 @@ describe('testlist run — insufficient credits → exit 12 + telemetry facts', it('delete: rejects empty and whitespace list-id', async () => { for (const invalidId of ['', ' ', '\t\n']) { - await expect(runTestlistDelete({ ...base, listId: invalidId, confirm: true })).rejects.toThrow(ApiError); - await expect(runTestlistDelete({ ...base, listId: invalidId, dryRun: true })).rejects.toThrow(ApiError); + await expect( + runTestlistDelete({ ...base, listId: invalidId, confirm: true }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistDelete({ ...base, listId: invalidId, dryRun: true }), + ).rejects.toThrow(ApiError); try { await runTestlistDelete({ ...base, listId: invalidId, confirm: true }); } catch (e: unknown) { @@ -1085,8 +1091,12 @@ describe('testlist run — insufficient credits → exit 12 + telemetry facts', it('update: rejects empty and whitespace list-id', async () => { for (const invalidId of ['', ' ', '\t\n']) { - await expect(runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed' })).rejects.toThrow(ApiError); - await expect(runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed', dryRun: true })).rejects.toThrow(ApiError); + await expect( + runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed' }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed', dryRun: true }), + ).rejects.toThrow(ApiError); try { await runTestlistUpdate({ ...base, listId: invalidId, name: 'Renamed' }); } catch (e: unknown) { @@ -1098,27 +1108,55 @@ describe('testlist run — insufficient credits → exit 12 + telemetry facts', }); it('add: rejects empty and whitespace list-id and test-ids', async () => { - await expect(runTestlistAdd({ ...base, listId: ' ', testIds: ['test-1'] })).rejects.toThrow(ApiError); - await expect(runTestlistAdd({ ...base, listId: 'list-1', testIds: [' '] })).rejects.toThrow(ApiError); - await expect(runTestlistAdd({ ...base, listId: 'list-1', testIds: ['test-1', ''] })).rejects.toThrow(ApiError); - await expect(runTestlistAdd({ ...base, listId: ' ', testIds: ['test-1'], dryRun: true })).rejects.toThrow(ApiError); - await expect(runTestlistAdd({ ...base, listId: 'list-1', testIds: [' '], dryRun: true })).rejects.toThrow(ApiError); + await expect(runTestlistAdd({ ...base, listId: ' ', testIds: ['test-1'] })).rejects.toThrow( + ApiError, + ); + await expect(runTestlistAdd({ ...base, listId: 'list-1', testIds: [' '] })).rejects.toThrow( + ApiError, + ); + await expect( + runTestlistAdd({ ...base, listId: 'list-1', testIds: ['test-1', ''] }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistAdd({ ...base, listId: ' ', testIds: ['test-1'], dryRun: true }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistAdd({ ...base, listId: 'list-1', testIds: [' '], dryRun: true }), + ).rejects.toThrow(ApiError); }); it('remove: rejects empty and whitespace list-id and test-ids', async () => { - await expect(runTestlistRemove({ ...base, listId: ' ', testIds: ['test-1'] })).rejects.toThrow(ApiError); - await expect(runTestlistRemove({ ...base, listId: 'list-1', testIds: [' '] })).rejects.toThrow(ApiError); - await expect(runTestlistRemove({ ...base, listId: 'list-1', testIds: ['test-1', ''] })).rejects.toThrow(ApiError); - await expect(runTestlistRemove({ ...base, listId: ' ', testIds: ['test-1'], dryRun: true })).rejects.toThrow(ApiError); - await expect(runTestlistRemove({ ...base, listId: 'list-1', testIds: [' '], dryRun: true })).rejects.toThrow(ApiError); + await expect( + runTestlistRemove({ ...base, listId: ' ', testIds: ['test-1'] }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistRemove({ ...base, listId: 'list-1', testIds: [' '] }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistRemove({ ...base, listId: 'list-1', testIds: ['test-1', ''] }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistRemove({ ...base, listId: ' ', testIds: ['test-1'], dryRun: true }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistRemove({ ...base, listId: 'list-1', testIds: [' '], dryRun: true }), + ).rejects.toThrow(ApiError); }); it('run: rejects empty and whitespace list-id and cases', async () => { await expect(runTestlistRun({ ...runBase, listId: ' ' })).rejects.toThrow(ApiError); - await expect(runTestlistRun({ ...runBase, listId: ' ', dryRun: true })).rejects.toThrow(ApiError); - await expect(runTestlistRun({ ...runBase, listId: 'list-1', cases: [' '] })).rejects.toThrow(ApiError); - await expect(runTestlistRun({ ...runBase, listId: 'list-1', cases: ['valid-case', ''] })).rejects.toThrow(ApiError); - await expect(runTestlistRun({ ...runBase, listId: 'list-1', cases: [' '], dryRun: true })).rejects.toThrow(ApiError); + await expect(runTestlistRun({ ...runBase, listId: ' ', dryRun: true })).rejects.toThrow( + ApiError, + ); + await expect( + runTestlistRun({ ...runBase, listId: 'list-1', cases: [' '] }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistRun({ ...runBase, listId: 'list-1', cases: ['valid-case', ''] }), + ).rejects.toThrow(ApiError); + await expect( + runTestlistRun({ ...runBase, listId: 'list-1', cases: [' '], dryRun: true }), + ).rejects.toThrow(ApiError); }); }); });