From 73ae3ab45baaaa76eaece971ddf89b9247b90f1f Mon Sep 17 00:00:00 2001 From: finalerock44 <77282157+finalerock44@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:03:08 +0100 Subject: [PATCH 1/3] feat(cloud): device matrix via repeated --ios-config/--android-config (#1105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One upload targets N device configs. Each --ios-config : / --android-config :[:play] names exactly one validated cell — no cross-product. A device matrix is single-platform; mixing platforms is rejected before any upload. - parseDeviceMatrix() (unit-tested): parsing, :play Google Play cell, mixed- platform + malformed rejection. - Each cell validated against the compatibility matrix up front; fails fast naming the bad cell before the flow zip is uploaded. - Serialized as the deviceMatrix field; single-device submissions stay byte- identical. targetPlatform now derives from the matrix too. - Pre-submit cost preview via POST /uploads/estimateMatrix (cells + est. cost + per-column breakdown); tolerant of older APIs that lack the endpoint (404). - --json tests[].device (structured {name, osVersion, googlePlay}) on both the sync and async paths, so a flow run on two devices is disambiguated. Note: generated schema.types.ts is intentionally not regenerated here (it lags dev's swagger); regenerate from dev after the API lands. Runtime reads config/simulator_name structurally. --- src/commands/cloud.ts | 76 ++++++++++++++++++++++ src/config/flags/device.flags.ts | 8 +++ src/gateways/api-gateway.ts | 45 +++++++++++++ src/services/results-polling.service.ts | 42 ++++++++++++ src/services/test-submission.service.ts | 21 +++++- src/types/domain/device.types.ts | 14 ++++ src/utils/device-matrix.ts | 68 +++++++++++++++++++ test/integration/cloud.integration.test.ts | 26 ++++++++ test/unit/device-matrix.test.ts | 61 +++++++++++++++++ 9 files changed, 360 insertions(+), 1 deletion(-) create mode 100644 src/utils/device-matrix.ts create mode 100644 test/unit/device-matrix.test.ts diff --git a/src/commands/cloud.ts b/src/commands/cloud.ts index db1c0fb..081ea17 100644 --- a/src/commands/cloud.ts +++ b/src/commands/cloud.ts @@ -20,6 +20,7 @@ import { import { MoropoService } from '../services/moropo.service.js'; import { ReportDownloadService } from '../services/report-download.service.js'; import { + deviceFromResultRow, ResultsPollingService, RunFailedError, } from '../services/results-polling.service.js'; @@ -31,8 +32,10 @@ import { EAndroidDevices, EiOSDevices, EiOSVersions, + isIosMatrixConfig, } from '../types/domain/device.types.js'; import { resolveAuth } from '../utils/auth.js'; +import { matrixIsIos, parseDeviceMatrix } from '../utils/device-matrix.js'; import { detectCiContext, isCI } from '../utils/ci.js'; import { CliError, @@ -211,6 +214,10 @@ export const cloudCommand = defineCommand({ 'android-device', ); const androidNoSnapshot = Boolean(args['android-no-snapshot']); + // Repeatable device-matrix flags: one validated cell each, no cross-product. + const iosConfigFlags = collectRepeatedFlag(rawArgs, ['--ios-config']); + const androidConfigFlags = collectRepeatedFlag(rawArgs, ['--android-config']); + const deviceMatrix = parseDeviceMatrix(iosConfigFlags, androidConfigFlags); const json = Boolean(args.json); const jsonFileFlag = Boolean(args['json-file']); const jsonFileName = args['json-file-name'] as string | undefined; @@ -502,6 +509,28 @@ export const cloudCommand = defineCommand({ { debug, logger: (m: string) => out(m) }, ); + // Validate every device-matrix cell up front — each on its own, against + // the same compatibility matrix — so an unsupported cell fails fast, + // naming it, before anything is uploaded. + for (const cfg of deviceMatrix) { + if (isIosMatrixConfig(cfg)) { + deviceValidationService.validateiOSDevice( + cfg.iOSVersion, + cfg.iOSDevice, + compatibilityData, + { debug, logger: (m: string) => out(m) }, + ); + } else { + deviceValidationService.validateAndroidDevice( + cfg.androidApiLevel, + cfg.androidDevice, + cfg.googlePlay ?? googlePlay, + compatibilityData, + { debug, logger: (m: string) => out(m) }, + ); + } + } + if (maestroChromeOnboarding && !androidApiLevel && !androidDevice) { warnOut( 'The --maestro-chrome-onboarding flag only applies to Android tests and will be ignored for iOS tests.', @@ -639,6 +668,8 @@ export const cloudCommand = defineCommand({ 'include-tags': includeTags, 'exclude-tags': excludeTags, 'exclude-flows': excludeFlows, + 'ios-config': iosConfigFlags, + 'android-config': androidConfigFlags, }; for (const [k, v] of Object.entries(args)) { if (!canonicalFlagKeys.has(k)) continue; @@ -762,6 +793,7 @@ export const cloudCommand = defineCommand({ continueOnFailure, debug, deviceLocale, + deviceMatrix, env, executionPlan, flowFile, @@ -784,6 +816,49 @@ export const cloudCommand = defineCommand({ disableAnimations, }); + // Device-matrix cost preview: the server prices the exact fan-out (quote + // == charge) so the user sees the cell count and estimated cost before the + // flow zip is uploaded. An unsupported cell fails fast here. Skipped when + // there is no matrix, and tolerant of older APIs that lack the endpoint. + if (deviceMatrix.length > 0) { + const estimate = await ApiGateway.estimateMatrix(apiUrl, auth, fields); + if (estimate) { + const osPrefix = matrixIsIos(deviceMatrix) ? 'iOS' : 'API'; + const rows = ui.fields([ + ['cells', colors.highlight(String(estimate.cellCount))], + ['est. cost', colors.highlight(`$${estimate.totalCost.toFixed(2)}`)], + ]); + for (const col of estimate.columns) { + const label = [ + col.deviceName, + col.osVersion && `${osPrefix} ${col.osVersion}`, + col.googlePlay && 'Play', + ] + .filter(Boolean) + .join(' · '); + rows.push( + ...ui.fields([ + [ + label, + colors.dim( + `${col.flowCount} flow${col.flowCount === 1 ? '' : 's'} · $${col.cost.toFixed(2)}`, + ), + ], + ]), + ); + } + if (estimate.excludedFlows.length > 0) { + rows.push( + colors.dim( + `${estimate.excludedFlows.length} flow${estimate.excludedFlows.length === 1 ? '' : 's'} target their own device (excluded from the matrix)`, + ), + ); + } + out(ui.section('Device matrix')); + out(ui.branch(rows)); + } + } + // New path: upload the zip directly to storage, then submit a JSON test // referencing it. Older API deployments lack these endpoints — a real API // 404s (route undefined), some proxies 405 (path/method not allowed); in @@ -865,6 +940,7 @@ export const cloudCommand = defineCommand({ consoleUrl: url, status: 'PENDING', tests: results.map((r) => ({ + device: deviceFromResultRow(r), fileName: r.test_file_name, flowName: testMetadataMap[r.test_file_name]?.flowName || diff --git a/src/config/flags/device.flags.ts b/src/config/flags/device.flags.ts index 37135d4..1fa81df 100644 --- a/src/config/flags/device.flags.ts +++ b/src/config/flags/device.flags.ts @@ -42,6 +42,14 @@ export const deviceFlags = { type: 'string', description: `[iOS only] iOS version to run your flow against (options: ${iosVersions})`, }, + 'ios-config': { + type: 'string', + description: `[iOS only] Device-matrix cell as :, e.g. iphone-16:18. Repeatable — every flow runs once per cell (no cross-product). Cannot be combined with --android-config.`, + }, + 'android-config': { + type: 'string', + description: `[Android only] Device-matrix cell as : (append :play for Google Play), e.g. pixel-7:34 or pixel-7:34:play. Repeatable — every flow runs once per cell (no cross-product). Cannot be combined with --ios-config.`, + }, orientation: { type: 'string', description: diff --git a/src/gateways/api-gateway.ts b/src/gateways/api-gateway.ts index 7ba37b4..cf5b455 100644 --- a/src/gateways/api-gateway.ts +++ b/src/gateways/api-gateway.ts @@ -588,6 +588,51 @@ export const ApiGateway = { } }, + /** + * Dry-run cost + cell-count estimate for a (possibly device-matrix) + * submission. Runs the same resolve → validate → fan-out → price core as the + * submit path server-side, without persisting. Returns null when the API + * predates this endpoint (404) so the caller can proceed without a preview; + * a 400 (an invalid config) surfaces as a normal API error so the CLI can + * fail fast before uploading the flow zip. + */ + async estimateMatrix(baseUrl: string, auth: AuthContext, body: Record) { + try { + const res = await fetch(`${baseUrl}/uploads/estimateMatrix`, { + body: JSON.stringify(body), + headers: { + 'content-type': 'application/json', + ...auth.headers, + }, + method: 'POST', + }); + if (res.status === 404) { + return null; + } + if (!res.ok) { + await this.handleApiError(res, 'Failed to estimate device matrix'); + } + return await parseJsonResponse<{ + cellCount: number; + totalCost: number; + excludedFlows: string[]; + columns: Array<{ + deviceName: string; + osVersion: string; + googlePlay: boolean; + flowCount: number; + cost: number; + }>; + }>(res, 'Failed to estimate device matrix'); + } catch (error) { + if (error instanceof TypeError && error.message === 'fetch failed') { + throw this.enhanceFetchError(error, `${baseUrl}/uploads/estimateMatrix`); + } + + throw error; + } + }, + /** * Generic report download method that handles both junit and allure reports diff --git a/src/services/results-polling.service.ts b/src/services/results-polling.service.ts index c1cf800..774fc66 100644 --- a/src/services/results-polling.service.ts +++ b/src/services/results-polling.service.ts @@ -47,10 +47,46 @@ export interface TestMetadata { tags: string[]; } +/** + * The device a result ran on. Additive: single-device runs are unchanged, and + * a device matrix disambiguates two `tests[]` entries that share a `name` (the + * same flow on two devices) by their device. + */ +export interface TestDevice { + googlePlay?: boolean; + name?: string; + osVersion?: string; +} + +/** + * Structured device for a result row. Prefers the friendly deviceName/osVersion + * the per-flow targeting / matrix fan-out stamps onto each result's config + * (#1097), falling back to the raw simulator_name for older rows. Returns + * undefined when neither is present, so single-device runs that predate the + * field simply omit `device`. Shared by the sync polling path and the async + * (--async --json) path so both emit an identical device shape. + */ +export function deviceFromResultRow(r: { + config?: unknown; + simulator_name?: string | null; +}): TestDevice | undefined { + const config = (r.config ?? {}) as { deviceName?: string; osVersion?: string }; + const sim = r.simulator_name ?? undefined; + const name = config.deviceName ?? sim; + if (!name && !config.osVersion) return undefined; + return { + name, + osVersion: config.osVersion, + googlePlay: sim ? /(_PLAY|-play)$/.test(sim) : undefined, + }; +} + export interface PollingResult { consoleUrl: string; status: 'FAILED' | 'PASSED'; tests: Array<{ + /** Device this result ran on (present when the API reports it). */ + device?: TestDevice; durationSeconds: null | number; failReason?: string; /** File path of the test (same as name, for clarity) */ @@ -311,6 +347,12 @@ export class ResultsPollingService { ? 'PASSED' : 'FAILED', tests: resultsWithoutEarlierTries.map((r) => ({ + // r carries config/simulator_name at runtime; the committed generated + // types lag the API (regenerated wholesale from dev's swagger), so read + // them through the helper's structural type. + device: deviceFromResultRow( + r as { config?: unknown; simulator_name?: string | null }, + ), durationSeconds: r.duration_seconds ?? null, failReason: r.status === 'FAILED' ? r.fail_reason || 'No reason provided' : undefined, diff --git a/src/services/test-submission.service.ts b/src/services/test-submission.service.ts index 5d4aea6..bbff215 100644 --- a/src/services/test-submission.service.ts +++ b/src/services/test-submission.service.ts @@ -2,6 +2,7 @@ import { createHash } from 'node:crypto'; import * as path from 'node:path'; import { compressFilesFromRelativePath } from '../methods.js'; +import { DeviceMatrixConfig } from '../types/domain/device.types.js'; import { toPortableRelativePath } from '../utils/paths.js'; import { IExecutionPlan } from './execution-plan.service.js'; @@ -15,6 +16,7 @@ export interface TestSubmissionConfig { continueOnFailure?: boolean; debug?: boolean; deviceLocale?: string; + deviceMatrix?: DeviceMatrixConfig[]; disableAnimations?: boolean; env?: string[]; executionPlan: IExecutionPlan; @@ -85,6 +87,7 @@ export class TestSubmissionService { maestroChromeOnboarding, raw, disableAnimations, + deviceMatrix, debug = false, logger, } = config; @@ -183,7 +186,23 @@ export class TestSubmissionService { // Note: googlePlay is now included in configPayload below instead of as a separate field // to work around a FormData parsing issue in the API - const targetPlatform = iOSDevice || iOSVersion ? 'ios' : 'android'; + // Explicit device matrix (one upload, N cells). Only sent when present, so + // single-device submissions stay byte-identical. + if (deviceMatrix && deviceMatrix.length > 0) { + fields.deviceMatrix = JSON.stringify(deviceMatrix); + } + + // Platform used only to pick which workspace-config disableAnimations flag + // applies. A device matrix is single-platform; its first cell decides. Fall + // back to the scalar iOS flags for single-device submissions. + const matrixPlatform = + deviceMatrix && deviceMatrix.length > 0 + ? 'iOSDevice' in deviceMatrix[0] + ? 'ios' + : 'android' + : undefined; + const targetPlatform = + matrixPlatform ?? (iOSDevice || iOSVersion ? 'ios' : 'android'); const configYamlDisableAnimations = targetPlatform === 'ios' ? Boolean(workspaceConfig?.platform?.ios?.disableAnimations) diff --git a/src/types/domain/device.types.ts b/src/types/domain/device.types.ts index 064c23a..45e2632 100644 --- a/src/types/domain/device.types.ts +++ b/src/types/domain/device.types.ts @@ -40,3 +40,17 @@ export enum EAndroidApiLevels { 'thirtyTwo' = '32', 'twentyNine' = '29', } + +/** + * One explicit device-matrix cell. iOS entries carry {iOSDevice, iOSVersion}; + * Android entries carry {androidDevice, androidApiLevel} plus an optional Play + * channel. Sent to the API as the `deviceMatrix` array; every non-targeted flow + * runs once per cell. There is no cross-product — each entry is one cell. + */ +export type DeviceMatrixConfig = + | { iOSDevice: string; iOSVersion: string } + | { androidApiLevel: string; androidDevice: string; googlePlay?: boolean }; + +export const isIosMatrixConfig = ( + c: DeviceMatrixConfig, +): c is { iOSDevice: string; iOSVersion: string } => 'iOSDevice' in c; diff --git a/src/utils/device-matrix.ts b/src/utils/device-matrix.ts new file mode 100644 index 0000000..18e26cc --- /dev/null +++ b/src/utils/device-matrix.ts @@ -0,0 +1,68 @@ +import { + DeviceMatrixConfig, + isIosMatrixConfig, +} from '../types/domain/device.types.js'; +import { CliError } from './cli.js'; + +/** + * Parse repeated `--ios-config :` and + * `--android-config :[:play]` flags into an explicit device + * matrix. Each entry is one validated cell — there is NO cross-product, because + * the compatibility matrix is ragged and a cross-product would invent cells the + * user never asked for. + * + * A device matrix is single-platform (one upload, one binary), so mixing iOS + * and Android configs is rejected here before anything is uploaded. + * + * @throws CliError on malformed syntax or a mixed-platform matrix. + */ +export function parseDeviceMatrix( + iosConfigs: string[], + androidConfigs: string[], +): DeviceMatrixConfig[] { + if (iosConfigs.length > 0 && androidConfigs.length > 0) { + throw new CliError( + 'A device matrix cannot mix platforms: use either --ios-config or --android-config, not both. One upload runs one binary.', + ); + } + + const configs: DeviceMatrixConfig[] = []; + + for (const raw of iosConfigs) { + const parts = raw.split(':'); + if (parts.length !== 2 || !parts[0] || !parts[1]) { + throw new CliError( + `Invalid --ios-config "${raw}". Expected :, e.g. iphone-16:18.`, + ); + } + configs.push({ iOSDevice: parts[0], iOSVersion: parts[1] }); + } + + for (const raw of androidConfigs) { + const parts = raw.split(':'); + // : with an optional trailing :play for a Play cell. + if ( + parts.length < 2 || + parts.length > 3 || + !parts[0] || + !parts[1] || + (parts.length === 3 && parts[2] !== 'play') + ) { + throw new CliError( + `Invalid --android-config "${raw}". Expected : or ::play, e.g. pixel-7:34 or pixel-7:34:play.`, + ); + } + configs.push({ + androidDevice: parts[0], + androidApiLevel: parts[1], + ...(parts.length === 3 ? { googlePlay: true } : {}), + }); + } + + return configs; +} + +/** True when the matrix targets iOS (used to pick the validation lookup). */ +export function matrixIsIos(configs: DeviceMatrixConfig[]): boolean { + return configs.length > 0 && isIosMatrixConfig(configs[0]); +} diff --git a/test/integration/cloud.integration.test.ts b/test/integration/cloud.integration.test.ts index 253057f..ab1de5d 100644 --- a/test/integration/cloud.integration.test.ts +++ b/test/integration/cloud.integration.test.ts @@ -163,6 +163,32 @@ appId: com.example.app }); }); + // #1105 device matrix: repeated --ios-config / --android-config cells. + describe('device matrix', () => { + it('accepts a repeated --ios-config matrix and still yields one upload', async () => { + const command = `${CLI} cloud ${iosAppFile} ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --ios-config iphone-16:18 --ios-config iphone-16-pro:26 --async --json`; + + const { stdout } = await exec(command, { timeout: 30_000 }); + // Still one upload with one uploadId — the matrix is a property of the + // upload, not N uploads. + expectAsyncRunJson(stdout); + }); + + it('rejects mixing --ios-config and --android-config before any upload', async () => { + const command = `${CLI} cloud ${iosAppFile} ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --ios-config iphone-16:18 --android-config pixel-7:34`; + + const { output } = await runExpectingFailure(command); + expect(output.toLowerCase()).to.include('cannot mix platforms'); + }); + + it('rejects a malformed --ios-config, naming the value', async () => { + const command = `${CLI} cloud ${iosAppFile} ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --ios-config iphone-16`; + + const { output } = await runExpectingFailure(command); + expect(output).to.include('iphone-16'); + }); + }); + describe('device configuration options', () => { // Async non-JSON runs always reach submission against the mock API. // `.include` keeps these robust to incidental extra lines (e.g. the diff --git a/test/unit/device-matrix.test.ts b/test/unit/device-matrix.test.ts new file mode 100644 index 0000000..7e8c497 --- /dev/null +++ b/test/unit/device-matrix.test.ts @@ -0,0 +1,61 @@ +import { expect } from 'chai'; + +import { CliError } from '../../src/utils/cli.js'; +import { + matrixIsIos, + parseDeviceMatrix, +} from '../../src/utils/device-matrix.js'; + +/** + * The device matrix is the load-bearing part of #1105: each --ios-config / + * --android-config names exactly one cell, there is no cross-product, and a + * matrix is single-platform. These are pure and worth pinning precisely. + */ +describe('parseDeviceMatrix', () => { + it('returns an empty matrix when no config flags are passed', () => { + expect(parseDeviceMatrix([], [])).to.deep.equal([]); + expect(matrixIsIos([])).to.equal(false); + }); + + it('parses each --ios-config as exactly one cell (no cross-product)', () => { + const matrix = parseDeviceMatrix( + ['iphone-16:18', 'iphone-16-pro:26'], + [], + ); + expect(matrix).to.deep.equal([ + { iOSDevice: 'iphone-16', iOSVersion: '18' }, + { iOSDevice: 'iphone-16-pro', iOSVersion: '26' }, + ]); + expect(matrixIsIos(matrix)).to.equal(true); + }); + + it('parses --android-config, with :play marking a Google Play cell', () => { + expect(parseDeviceMatrix([], ['pixel-7:34', 'pixel-7:34:play'])).to.deep.equal([ + { androidDevice: 'pixel-7', androidApiLevel: '34' }, + { androidDevice: 'pixel-7', androidApiLevel: '34', googlePlay: true }, + ]); + }); + + it('rejects a mixed-platform matrix', () => { + expect(() => + parseDeviceMatrix(['iphone-16:18'], ['pixel-7:34']), + ).to.throw(CliError, /cannot mix platforms/i); + }); + + it('rejects malformed iOS syntax, naming the offending value', () => { + expect(() => parseDeviceMatrix(['iphone-16'], [])).to.throw( + CliError, + /iphone-16/, + ); + expect(() => parseDeviceMatrix(['iphone-16:'], [])).to.throw(CliError); + expect(() => parseDeviceMatrix([':18'], [])).to.throw(CliError); + }); + + it('rejects malformed Android syntax and a bad third segment', () => { + expect(() => parseDeviceMatrix([], ['pixel-7'])).to.throw(CliError); + expect(() => parseDeviceMatrix([], ['pixel-7:34:store'])).to.throw( + CliError, + /pixel-7:34:store/, + ); + }); +}); From 4f38419a54b13bd82d42a8366441b98499bcd2e1 Mon Sep 17 00:00:00 2001 From: finalerock44 <77282157+finalerock44@users.noreply.github.com> Date: Fri, 10 Jul 2026 22:46:10 +0100 Subject: [PATCH 2/3] fix(cloud): tolerate 405 from estimateMatrix preview (#1105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cost preview is best-effort, but the gateway only treated 404 as 'endpoint absent' and threw on 405. An API that predates the endpoint (incl. the CI Prism mock running against dev's swagger before this lands) 405s with NO_METHOD_MATCHED, which blocked the submit. Treat 404 and 405 alike — return null and proceed without a preview. A 400 (invalid config) still surfaces. --- src/gateways/api-gateway.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/gateways/api-gateway.ts b/src/gateways/api-gateway.ts index cf5b455..48eeb18 100644 --- a/src/gateways/api-gateway.ts +++ b/src/gateways/api-gateway.ts @@ -591,10 +591,11 @@ export const ApiGateway = { /** * Dry-run cost + cell-count estimate for a (possibly device-matrix) * submission. Runs the same resolve → validate → fan-out → price core as the - * submit path server-side, without persisting. Returns null when the API - * predates this endpoint (404) so the caller can proceed without a preview; - * a 400 (an invalid config) surfaces as a normal API error so the CLI can - * fail fast before uploading the flow zip. + * submit path server-side, without persisting. The preview is best-effort: + * an API that predates this endpoint 404s (route undefined) or 405s (route + * matched a sibling path, no POST) — in either case return null so the caller + * proceeds without a preview. A 400 (an invalid config) still surfaces as a + * normal API error so the CLI can fail fast before uploading the flow zip. */ async estimateMatrix(baseUrl: string, auth: AuthContext, body: Record) { try { @@ -606,7 +607,7 @@ export const ApiGateway = { }, method: 'POST', }); - if (res.status === 404) { + if (res.status === 404 || res.status === 405) { return null; } if (!res.ok) { From aa25c2ed6a8faebc0c4025e0c6bb13eece58033f Mon Sep 17 00:00:00 2001 From: finalerock44 <77282157+finalerock44@users.noreply.github.com> Date: Fri, 10 Jul 2026 22:59:19 +0100 Subject: [PATCH 3/3] test: run integration CLI via execFile argv, not a shell (#1105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flags every `exec(\)` in the integration suite as js/shell-command-injection-from-environment — the built CLI's absolute path (path.resolve) flows through a template string into a shell. There is no real injection (test harness, local fixture paths), but the whole class is avoidable. Route the shared exec helper through execFile(process.execPath, argv) — no shell — by tokenising the `${CLI} ` command. This is CodeQL's recommended remediation (argument list, not a shell string) and clears the entire class across all five integration files at once, with no call-site changes (signature-compatible; runExpectingFailure still reads stdout/stderr from the rejection). Verified the CLI runs and non-zero exits still surface stderr. --- test/integration/helpers.ts | 43 +++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/test/integration/helpers.ts b/test/integration/helpers.ts index d60eeb1..0c998f9 100644 --- a/test/integration/helpers.ts +++ b/test/integration/helpers.ts @@ -7,15 +7,54 @@ * assert the success path unconditionally: a dead or missing mock API must * fail the suite, never soften it. */ -import { exec as execCallback } from 'node:child_process'; +import { execFile as execFileCallback } from 'node:child_process'; import * as path from 'node:path'; import { promisify } from 'node:util'; -export const exec = promisify(execCallback); +const execFileAsync = promisify(execFileCallback); /** Absolute path to the built CLI so tests can run with any cwd. */ export const CLI = path.resolve('dist/index.js'); +export interface ExecResult { + stdout: string; + stderr: string; +} + +/** + * Split a `${CLI} …` command line into argv, honouring single/double quotes so + * a quoted multi-word value stays one token. These test commands contain no + * other shell metacharacters, so this is exact for our use. + */ +function tokenize(command: string): string[] { + const tokens: string[] = []; + const pattern = /"([^"]*)"|'([^']*)'|(\S+)/g; + let match: RegExpExecArray | null; + while ((match = pattern.exec(command)) !== null) { + tokens.push(match[1] ?? match[2] ?? match[3]); + } + return tokens; +} + +/** + * Run a test command **without a shell**. Every integration command is + * `${CLI} ` — an absolute script path plus arguments — so we tokenise it + * and invoke the current Node binary directly with the script and args as argv + * (`execFile`, never `exec`). Passing an argument list instead of a shell string + * is CodeQL's recommended fix for `js/shell-command-injection-from-environment`: + * with no shell there is nothing for the (uncontrolled, but trusted) absolute + * paths to inject into. Signature-compatible with the previous + * `promisify(child_process.exec)` so no call site changes. + */ +export async function exec( + command: string, + opts: { cwd?: string; env?: NodeJS.ProcessEnv; timeout?: number } = {}, +): Promise { + const argv = tokenize(command); + const { stdout, stderr } = await execFileAsync(process.execPath, argv, opts); + return { stdout: String(stdout), stderr: String(stderr) }; +} + export const MOCK_API_URL = process.env.MOCK_API_URL ?? 'http://localhost:3001'; /** One of the keys accepted by the mock API's auth shim. */