From 13d32695a0dad3d2d350ccf27c4a2540af888771 Mon Sep 17 00:00:00 2001 From: Jasper Date: Tue, 18 Aug 2026 13:32:03 +0000 Subject: [PATCH] fix(security): validate recipe parameter values (#11234) --- .../src/components/ParameterInputModal.tsx | 82 +++- .../__tests__/ParameterInputModal.test.tsx | 366 ++++++++++++++++++ 2 files changed, 433 insertions(+), 15 deletions(-) diff --git a/ui/desktop/src/components/ParameterInputModal.tsx b/ui/desktop/src/components/ParameterInputModal.tsx index eb5a28784..23bd753a7 100644 --- a/ui/desktop/src/components/ParameterInputModal.tsx +++ b/ui/desktop/src/components/ParameterInputModal.tsx @@ -65,6 +65,25 @@ function needsUserValue(param: Parameter): boolean { return param.requirement === 'required' || param.requirement === 'user_prompt'; } +const NUMBER_VALUE_PATTERN = /^-?(?:\d+(?:\.\d+)?|\.\d+)(?:[eE][+-]?\d+)?$/; + +function isValidParameterValue(param: Parameter, value: string): boolean { + switch (param.input_type) { + case 'select': + return param.options?.includes(value) ?? true; + case 'boolean': + return value === 'true' || value === 'false'; + case 'number': + return NUMBER_VALUE_PATTERN.test(value) && Number.isFinite(Number(value)); + default: + return true; + } +} + +function createParameterValueMap(): Record { + return Object.create(null) as Record; +} + const ParameterInputModal: React.FC = ({ parameters, onSubmit, @@ -74,38 +93,71 @@ const ParameterInputModal: React.FC = ({ const intl = useIntl(); const fieldIdPrefix = useId(); const fieldId = (key: string): string => `${fieldIdPrefix}-${key}`; - const [inputValues, setInputValues] = useState>({}); - const [validationErrors, setValidationErrors] = useState>({}); + const [inputValues, setInputValues] = useState>(createParameterValueMap); + const [validationErrors, setValidationErrors] = + useState>(createParameterValueMap); const [showCancelOptions, setShowCancelOptions] = useState(false); useEffect(() => { - const defaultValues: Record = {}; + const values = createParameterValueMap(); parameters.forEach((param) => { - if (param.requirement === 'optional' && param.default) { - defaultValues[param.key] = - param.input_type === 'boolean' ? param.default.toLowerCase() : param.default; + if (param.requirement === 'optional' && param.default != null) { + if (isValidParameterValue(param, param.default)) { + values[param.key] = param.default; + } + } + + const initialValue = + initialValues && Object.prototype.hasOwnProperty.call(initialValues, param.key) + ? initialValues[param.key] + : undefined; + if (initialValue !== undefined && isValidParameterValue(param, initialValue)) { + values[param.key] = initialValue; } }); - setInputValues({ ...defaultValues, ...initialValues }); + setInputValues(values); }, [parameters, initialValues]); const handleChange = (name: string, value: string): void => { - setInputValues((prevValues: Record) => ({ ...prevValues, [name]: value })); + setInputValues((prevValues: Record) => { + const values = Object.assign(createParameterValueMap(), prevValues); + values[name] = value; + return values; + }); }; const handleSubmit = (e: React.SyntheticEvent): void => { e.preventDefault(); - setValidationErrors({}); + setValidationErrors(createParameterValueMap()); - const requiredParams: Parameter[] = parameters.filter(needsUserValue); - const errors: Record = {}; + const errors = createParameterValueMap(); + const submittedValues = createParameterValueMap(); - requiredParams.forEach((param) => { - const value = inputValues[param.key]?.trim(); - if (!value) { + parameters.forEach((param) => { + const value = inputValues[param.key]; + if (needsUserValue(param) && !value?.trim()) { errors[param.key] = `${param.description || param.key} is required`; + return; } + + if (value === undefined) { + if ( + param.requirement === 'optional' && + param.default != null && + !isValidParameterValue(param, param.default) + ) { + errors[param.key] = `${param.description || param.key} has an invalid value`; + } + return; + } + + if (!isValidParameterValue(param, value)) { + errors[param.key] = `${param.description || param.key} has an invalid value`; + return; + } + + submittedValues[param.key] = value; }); if (Object.keys(errors).length > 0) { @@ -113,7 +165,7 @@ const ParameterInputModal: React.FC = ({ return; } - onSubmit(inputValues); + onSubmit(submittedValues); }; const handleCancel = (): void => { diff --git a/ui/desktop/src/components/__tests__/ParameterInputModal.test.tsx b/ui/desktop/src/components/__tests__/ParameterInputModal.test.tsx index 1f96b8dbd..79e2f7491 100644 --- a/ui/desktop/src/components/__tests__/ParameterInputModal.test.tsx +++ b/ui/desktop/src/components/__tests__/ParameterInputModal.test.tsx @@ -141,6 +141,306 @@ describe('ParameterInputModal', () => { }); expect(defaultProps.onSubmit).not.toHaveBeenCalled(); }); + + it.each([ + { + name: 'select value outside its options', + parameter: { + key: 'mode', + description: 'Mode', + input_type: 'select', + requirement: 'required', + options: ['safe'], + } as Parameter, + initialValue: 'hidden instruction', + visibleValue: '', + }, + { + name: 'invalid boolean', + parameter: { + key: 'enabled', + description: 'Enabled', + input_type: 'boolean', + requirement: 'required', + } as Parameter, + initialValue: 'hidden instruction', + visibleValue: '', + }, + { + name: 'nonnumeric value', + parameter: { + key: 'iterations', + description: 'Iterations', + input_type: 'number', + requirement: 'required', + } as Parameter, + initialValue: 'hidden instruction', + visibleValue: null, + }, + { + name: 'number the browser cannot represent', + parameter: { + key: 'iterations', + description: 'Iterations', + input_type: 'number', + requirement: 'required', + } as Parameter, + initialValue: '1.', + visibleValue: null, + }, + ])( + 'does not submit an invalid $name prefill', + async ({ parameter, initialValue, visibleValue }) => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + renderWithIntl( + + ); + + expect(screen.getByLabelText(new RegExp(`^${parameter.description}`))).toHaveValue( + visibleValue + ); + await user.click(screen.getByText('Start Recipe')); + + expect(onSubmit).not.toHaveBeenCalled(); + } + ); + + it('preserves free-text input for select parameters without options', async () => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + renderWithIntl( + + ); + + await user.type(screen.getByLabelText(/^Mode/), 'custom value'); + await user.click(screen.getByText('Start Recipe')); + + expect(onSubmit).toHaveBeenCalledWith({ mode: 'custom value' }); + }); + + it('submits only declared parameter keys', async () => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + renderWithIntl( + + ); + + await user.click(screen.getByText('Start Recipe')); + + expect(onSubmit).toHaveBeenCalledWith({ topic: 'declared value' }); + }); + + it.each([ + { + name: 'select', + parameter: { + key: 'mode', + description: 'Mode', + input_type: 'select', + requirement: 'optional', + options: ['safe'], + default: 'hidden instruction', + } as Parameter, + }, + { + name: 'boolean', + parameter: { + key: 'enabled', + description: 'Enabled', + input_type: 'boolean', + requirement: 'optional', + default: 'TRUE', + } as Parameter, + }, + { + name: 'number', + parameter: { + key: 'iterations', + description: 'Iterations', + input_type: 'number', + requirement: 'optional', + default: '1.', + } as Parameter, + }, + ])('blocks submission when an optional $name default is invalid', async ({ parameter }) => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + renderWithIntl( + + ); + + await user.click(screen.getByText('Start Recipe')); + + expect(screen.getByText(`${parameter.description} has an invalid value`)).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it('allows a valid user value to replace an invalid optional default', async () => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + renderWithIntl( + + ); + + await user.selectOptions(screen.getByLabelText('Mode'), 'safe'); + await user.click(screen.getByText('Start Recipe')); + + expect(onSubmit).toHaveBeenCalledWith({ mode: 'safe' }); + }); + + it.each(['string', 'date'] as const)( + 'submits an explicitly cleared optional %s value', + async (inputType) => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + renderWithIntl( + + ); + + await user.clear(screen.getByLabelText('Topic')); + await user.click(screen.getByText('Start Recipe')); + + expect(onSubmit).toHaveBeenCalledWith({ topic: '' }); + } + ); + + it('rejects an explicitly cleared optional controlled value', async () => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + renderWithIntl( + + ); + + await user.selectOptions(screen.getByLabelText('Mode'), ''); + await user.click(screen.getByText('Start Recipe')); + + expect(screen.getByText('Mode has an invalid value')).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it.each(['__proto__', 'constructor', 'toString'])( + 'submits the reserved %s prefill as an own property', + async (key) => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + const initialValues = Object.fromEntries([[key, 'safe']]); + renderWithIntl( + + ); + + expect(screen.getByLabelText(/^Reserved parameter/)).toHaveValue('safe'); + await user.click(screen.getByText('Start Recipe')); + + const submittedValues = onSubmit.mock.calls[0][0]; + expect(Object.prototype.hasOwnProperty.call(submittedValues, key)).toBe(true); + expect(submittedValues[key]).toBe('safe'); + } + ); + + it('submits an entered __proto__ value as an own property', async () => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + renderWithIntl( + + ); + + await user.type(screen.getByLabelText(/^Prototype parameter/), 'safe'); + await user.click(screen.getByText('Start Recipe')); + + const submittedValues = onSubmit.mock.calls[0][0]; + expect(Object.prototype.hasOwnProperty.call(submittedValues, '__proto__')).toBe(true); + expect(submittedValues.__proto__).toBe('safe'); + }); }); describe('Cancel Behavior', () => { @@ -206,5 +506,71 @@ describe('ParameterInputModal', () => { expect((screen.getByLabelText(/boolean parameter/i) as HTMLSelectElement).value).toBe('true'); }); + + it('keeps a valid default when an invalid prefill is supplied', () => { + renderWithIntl( + + ); + + expect(screen.getByLabelText('Mode')).toHaveValue('safe'); + }); + + it('submits valid select, boolean, and number prefills', async () => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + renderWithIntl( + + ); + + expect(screen.getByLabelText(/^Mode/)).toHaveValue('safe'); + expect(screen.getByLabelText(/^Enabled/)).toHaveValue('false'); + expect(screen.getByLabelText(/^Iterations/)).toHaveValue(150); + + await user.click(screen.getByText('Start Recipe')); + + expect(onSubmit).toHaveBeenCalledWith({ + mode: 'safe', + enabled: 'false', + iterations: '1.5e2', + }); + }); }); });