Skip to content

Commit 6254c57

Browse files
fix(maestro): validate resolved numeric strings before coercion
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 17a2be7 commit 6254c57

4 files changed

Lines changed: 65 additions & 4 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,31 @@
11
import { expect, test } from 'vitest';
22
import { resolveMaestroTimingPolicy } from '../compatibility-policy.ts';
3+
import { resolveNumeric } from '../engine-flow.ts';
34

45
test('uses the Maestro-compatible extended wait default', () => {
56
expect(resolveMaestroTimingPolicy().extendedWaitUntilTimeoutMs).toBe(17_000);
67
});
8+
9+
test('resolveNumeric coerces valid resolved strings and numbers', () => {
10+
expect(resolveNumeric('42', 'x', {})).toBe(42);
11+
expect(resolveNumeric('3.5', 'x', {})).toBe(3.5);
12+
expect(resolveNumeric(3.5, 'x', {})).toBe(3.5);
13+
expect(resolveNumeric('0', 'x', { integer: true, nonNegative: true })).toBe(0);
14+
expect(resolveNumeric('5', 'x', { integer: true, positive: true })).toBe(5);
15+
expect(resolveNumeric(5, 'x', { integer: true, positive: true })).toBe(5);
16+
expect(resolveNumeric(undefined, 'x', {})).toBeUndefined();
17+
});
18+
19+
test('resolveNumeric rejects blank, whitespace, and malformed resolved strings', () => {
20+
expect(() => resolveNumeric('', 'x', {})).toThrow(/must be a finite number/);
21+
expect(() => resolveNumeric(' ', 'x', {})).toThrow(/must be a finite number/);
22+
expect(() => resolveNumeric('', 'x', { integer: true, nonNegative: true })).toThrow(
23+
/must be a non-negative integer/,
24+
);
25+
expect(() => resolveNumeric(' ', 'x', { integer: true, nonNegative: true })).toThrow(
26+
/must be a non-negative integer/,
27+
);
28+
expect(() => resolveNumeric('abc', 'x', {})).toThrow(/must be a finite number/);
29+
expect(() => resolveNumeric('1.2.3', 'x', {})).toThrow(/must be a finite number/);
30+
expect(() => resolveNumeric('1.5', 'x', { integer: true })).toThrow(/must be an integer/);
31+
});

src/compat/maestro/__tests__/runtime-port.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,4 +421,23 @@ describe('MaestroRuntimePort', () => {
421421
input: { durationMs: 250 },
422422
});
423423
});
424+
425+
test('rejects blank or whitespace resolved numeric option values with a clear error', async () => {
426+
const operations = makeOperations();
427+
const program = parseMaestroProgram(
428+
'---\n- extendedWaitUntil:\n visible: Ready\n timeout: ${TIMEOUT}\n',
429+
);
430+
431+
await expect(
432+
executeMaestroProgram(program, createMaestroRuntimePort(operations), {
433+
env: { TIMEOUT: '' },
434+
}),
435+
).rejects.toThrow(/extendedWaitUntil\.timeout must be a finite number/);
436+
437+
await expect(
438+
executeMaestroProgram(program, createMaestroRuntimePort(operations), {
439+
env: { TIMEOUT: ' ' },
440+
}),
441+
).rejects.toThrow(/extendedWaitUntil\.timeout must be a finite number/);
442+
});
424443
});

src/compat/maestro/engine-flow.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import path from 'node:path';
22
import { AppError } from '../../kernel/errors.ts';
33
import { createRequestCanceledError } from '../../request/cancel.ts';
4+
import { INTEGER_STRING_PATTERN, NUMERIC_STRING_PATTERN } from './program-ir-values.ts';
45
import type {
56
MaestroCommand,
67
MaestroProgram,
@@ -40,7 +41,10 @@ export function resolveNumeric(
4041
constraints: ResolveNumericConstraints = {},
4142
): number | undefined {
4243
if (value === undefined) return undefined;
43-
const num = typeof value === 'number' ? value : Number(value);
44+
const num =
45+
typeof value === 'number'
46+
? value
47+
: Number(parseResolvedNumericString(value, name, constraints));
4448
const description = resolveNumericDescription(constraints);
4549
if (!Number.isFinite(num)) {
4650
throw new AppError('INVALID_ARGS', `Maestro ${name} must be ${description}.`);
@@ -57,13 +61,26 @@ export function resolveNumeric(
5761
return num;
5862
}
5963

64+
function parseResolvedNumericString(
65+
value: string,
66+
name: string,
67+
constraints: ResolveNumericConstraints,
68+
): string {
69+
const description = resolveNumericDescription(constraints);
70+
const pattern = constraints.integer ? INTEGER_STRING_PATTERN : NUMERIC_STRING_PATTERN;
71+
if (!pattern.test(value)) {
72+
throw new AppError('INVALID_ARGS', `Maestro ${name} must be ${description}.`);
73+
}
74+
return value;
75+
}
76+
6077
export function readIterationCount(
6178
value: number | string | undefined,
6279
fallback: number,
6380
context: MaestroExecutionContext,
6481
name: string,
6582
): number {
66-
const resolved = value === undefined ? fallback : Number(context.resolve(String(value)));
83+
const resolved = value === undefined ? fallback : context.resolve(String(value));
6784
return resolveNumeric(resolved, name, { integer: true, nonNegative: true }) ?? fallback;
6885
}
6986

src/compat/maestro/program-ir-values.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,8 @@ export function readOptionalBoolean(
200200
}
201201

202202
const VARIABLE_PATTERN = /^\$\{[A-Za-z_][A-Za-z0-9_.]*\}$/;
203-
const NUMERIC_STRING_PATTERN = /^-?\d+(\.\d+)?$/;
204-
const INTEGER_STRING_PATTERN = /^-?\d+$/;
203+
export const NUMERIC_STRING_PATTERN = /^-?\d+(\.\d+)?$/;
204+
export const INTEGER_STRING_PATTERN = /^-?\d+$/;
205205

206206
type NumericScalarConstraints = {
207207
integer?: boolean;

0 commit comments

Comments
 (0)