fix(security): validate recipe parameter values (#11234)

2026-08-18 13:32:03 +00:00
parent 403fcc84c7
commit 13d32695a0
2 changed files with 433 additions and 15 deletions
@@ -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<string, string> {
+  return Object.create(null) as Record<string, string>;
+}
+
 const ParameterInputModal: React.FC<ParameterInputModalProps> = ({
  parameters,
  onSubmit,
@@ -74,38 +93,71 @@ const ParameterInputModal: React.FC<ParameterInputModalProps> = ({
  const intl = useIntl();
  const fieldIdPrefix = useId();
  const fieldId = (key: string): string => `${fieldIdPrefix}-${key}`;
-  const [inputValues, setInputValues] = useState<Record<string, string>>({});
-  const [validationErrors, setValidationErrors] = useState<Record<string, string>>({});
+  const [inputValues, setInputValues] = useState<Record<string, string>>(createParameterValueMap);
+  const [validationErrors, setValidationErrors] =
+    useState<Record<string, string>>(createParameterValueMap);
  const [showCancelOptions, setShowCancelOptions] = useState(false);

  useEffect(() => {
-    const defaultValues: Record<string, string> = {};
+    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<string, string>) => ({ ...prevValues, [name]: value }));
+    setInputValues((prevValues: Record<string, string>) => {
+      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<string, string> = {};
+    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<ParameterInputModalProps> = ({
      return;
    }

-    onSubmit(inputValues);
+    onSubmit(submittedValues);
  };

  const handleCancel = (): void => {
@@ -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(
+          <ParameterInputModal
+            {...defaultProps}
+            onSubmit={onSubmit}
+            parameters={[parameter]}
+            initialValues={{ [parameter.key]: initialValue }}
+          />
+        );
+
+        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(
+        <ParameterInputModal
+          {...defaultProps}
+          onSubmit={onSubmit}
+          parameters={[
+            {
+              key: 'mode',
+              description: 'Mode',
+              input_type: 'select',
+              requirement: 'required',
+            },
+          ]}
+        />
+      );
+
+      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(
+        <ParameterInputModal
+          {...defaultProps}
+          onSubmit={onSubmit}
+          parameters={[
+            {
+              key: 'topic',
+              description: 'Topic',
+              input_type: 'string',
+              requirement: 'required',
+            },
+          ]}
+          initialValues={{ topic: 'declared value', undeclared: 'hidden instruction' }}
+        />
+      );
+
+      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(
+        <ParameterInputModal {...defaultProps} onSubmit={onSubmit} parameters={[parameter]} />
+      );
+
+      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(
+        <ParameterInputModal
+          {...defaultProps}
+          onSubmit={onSubmit}
+          parameters={[
+            {
+              key: 'mode',
+              description: 'Mode',
+              input_type: 'select',
+              requirement: 'optional',
+              options: ['safe'],
+              default: 'hidden instruction',
+            },
+          ]}
+        />
+      );
+
+      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(
+          <ParameterInputModal
+            {...defaultProps}
+            onSubmit={onSubmit}
+            parameters={[
+              {
+                key: 'topic',
+                description: 'Topic',
+                input_type: inputType,
+                requirement: 'optional',
+                default: 'default topic',
+              },
+            ]}
+          />
+        );
+
+        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(
+        <ParameterInputModal
+          {...defaultProps}
+          onSubmit={onSubmit}
+          parameters={[
+            {
+              key: 'mode',
+              description: 'Mode',
+              input_type: 'select',
+              requirement: 'optional',
+              options: ['safe'],
+              default: 'safe',
+            },
+          ]}
+        />
+      );
+
+      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(
+          <ParameterInputModal
+            {...defaultProps}
+            onSubmit={onSubmit}
+            parameters={[
+              {
+                key,
+                description: 'Reserved parameter',
+                input_type: 'string',
+                requirement: 'required',
+              },
+            ]}
+            initialValues={initialValues}
+          />
+        );
+
+        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(
+        <ParameterInputModal
+          {...defaultProps}
+          onSubmit={onSubmit}
+          parameters={[
+            {
+              key: '__proto__',
+              description: 'Prototype parameter',
+              input_type: 'string',
+              requirement: 'required',
+            },
+          ]}
+        />
+      );
+
+      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(
+        <ParameterInputModal
+          {...defaultProps}
+          parameters={[
+            {
+              key: 'mode',
+              description: 'Mode',
+              input_type: 'select',
+              requirement: 'optional',
+              options: ['safe'],
+              default: 'safe',
+            },
+          ]}
+          initialValues={{ mode: 'hidden instruction' }}
+        />
+      );
+
+      expect(screen.getByLabelText('Mode')).toHaveValue('safe');
+    });
+
+    it('submits valid select, boolean, and number prefills', async () => {
+      const user = userEvent.setup();
+      const onSubmit = vi.fn();
+      renderWithIntl(
+        <ParameterInputModal
+          {...defaultProps}
+          onSubmit={onSubmit}
+          parameters={[
+            {
+              key: 'mode',
+              description: 'Mode',
+              input_type: 'select',
+              requirement: 'required',
+              options: ['safe'],
+            },
+            {
+              key: 'enabled',
+              description: 'Enabled',
+              input_type: 'boolean',
+              requirement: 'required',
+            },
+            {
+              key: 'iterations',
+              description: 'Iterations',
+              input_type: 'number',
+              requirement: 'required',
+            },
+          ]}
+          initialValues={{ mode: 'safe', enabled: 'false', iterations: '1.5e2' }}
+        />
+      );
+
+      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',
+      });
+    });
  });
 });