fix(security): clear custom provider transition secrets (#11467)

2026-08-23 23:17:35 +00:00
parent 1c4a6912f5
commit c0c5353782
2 changed files with 214 additions and 7 deletions
@@ -0,0 +1,180 @@
+import { act, render, screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { describe, expect, it, vi } from 'vitest';
+import { IntlTestWrapper } from '../../../../../../i18n/test-utils';
+import CustomProviderForm from './CustomProviderForm';
+
+const templates = vi.hoisted(() => ({
+  a: {
+    providerId: 'template-a',
+    name: 'Template A',
+    format: 'openai',
+    apiUrl: 'https://a.example.com',
+    models: [
+      {
+        id: 'model-a',
+        name: 'Model A',
+        contextLimit: 8192,
+        capabilities: {
+          toolCall: false,
+          reasoning: false,
+          attachment: false,
+          temperature: true,
+        },
+        deprecated: false,
+      },
+    ],
+    supportsStreaming: true,
+    envVar: 'TEMPLATE_A_API_KEY',
+    docUrl: '',
+  },
+  b: {
+    providerId: 'template-b',
+    name: 'Template B',
+    format: 'anthropic',
+    apiUrl: 'https://b.example.com',
+    models: [
+      {
+        id: 'model-b',
+        name: 'Model B',
+        contextLimit: 8192,
+        capabilities: {
+          toolCall: false,
+          reasoning: false,
+          attachment: false,
+          temperature: true,
+        },
+        deprecated: false,
+      },
+    ],
+    supportsStreaming: true,
+    envVar: 'TEMPLATE_B_API_KEY',
+    docUrl: '',
+  },
+}));
+
+vi.mock('../ProviderCatalogPicker', () => ({
+  default: ({ onSelect }: { onSelect: (template: typeof templates.a) => void }) => (
+    <div>
+      <button onClick={() => onSelect(templates.a)}>Use Template A</button>
+      <button onClick={() => onSelect(templates.b)}>Use Template B</button>
+    </div>
+  ),
+}));
+
+const renderForm = (onSubmit = vi.fn()) => {
+  render(
+    <CustomProviderForm initialData={null} isEditable onSubmit={onSubmit} onCancel={vi.fn()} />,
+    { wrapper: IntlTestWrapper }
+  );
+  return onSubmit;
+};
+
+const openTemplateCatalog = async (user: ReturnType<typeof userEvent.setup>) => {
+  await user.click(screen.getByText('Start from a provider template'));
+};
+
+const addHeader = async (user: ReturnType<typeof userEvent.setup>, name: string, value: string) => {
+  const nameInputs = screen.getAllByPlaceholderText('Header name');
+  const valueInputs = screen.getAllByPlaceholderText('Value');
+  await user.type(nameInputs[nameInputs.length - 1], name);
+  await user.type(valueInputs[valueInputs.length - 1], value);
+  await user.click(screen.getByRole('button', { name: 'Add' }));
+};
+
+describe('CustomProviderForm transitions', () => {
+  it('does not carry credentials from a cleared template into the next template', async () => {
+    const user = userEvent.setup();
+    const onSubmit = renderForm();
+    await openTemplateCatalog(user);
+    await user.click(screen.getByRole('button', { name: 'Use Template A' }));
+
+    await user.type(screen.getByLabelText(/API Key/), 'template-a-secret');
+    await addHeader(user, 'Authorization', 'Bearer template-a');
+
+    const pendingNames = screen.getAllByPlaceholderText('Header name');
+    const pendingValues = screen.getAllByPlaceholderText('Value');
+    await user.type(pendingNames[pendingNames.length - 1], 'Authorization');
+    await user.type(pendingValues[pendingValues.length - 1], 'Bearer pending-template-a');
+    await user.click(screen.getByRole('button', { name: 'Add' }));
+    expect(screen.getByText('A header with this name already exists')).toBeInTheDocument();
+
+    await user.click(screen.getByRole('button', { name: 'Clear' }));
+    await openTemplateCatalog(user);
+    await user.click(screen.getByRole('button', { name: 'Use Template B' }));
+
+    expect(screen.getByLabelText(/API Key/)).toHaveValue('');
+    expect(screen.queryByDisplayValue('Bearer template-a')).not.toBeInTheDocument();
+    expect(screen.queryByDisplayValue('Bearer pending-template-a')).not.toBeInTheDocument();
+    expect(screen.queryByText('A header with this name already exists')).not.toBeInTheDocument();
+
+    await user.type(screen.getByLabelText(/API Key/), 'template-b-secret');
+    await addHeader(user, 'X-Template-B', 'template-b-header');
+    await user.click(screen.getByRole('button', { name: 'Create Provider' }));
+
+    await waitFor(() => expect(onSubmit).toHaveBeenCalledOnce());
+    expect(onSubmit).toHaveBeenCalledWith(
+      expect.objectContaining({
+        api_key: 'template-b-secret',
+        catalog_provider_id: 'template-b',
+        headers: { 'X-Template-B': 'template-b-header' },
+      })
+    );
+  });
+
+  it('clears secrets and submit state when returning to the setup choice', async () => {
+    vi.spyOn(console, 'error').mockImplementation(() => {});
+    const user = userEvent.setup();
+    let rejectSubmit: ((reason: Error) => void) | undefined;
+    const onSubmit = vi.fn(
+      () =>
+        new Promise<void>((_resolve, reject) => {
+          rejectSubmit = reject;
+        })
+    );
+    renderForm(onSubmit);
+    await user.click(screen.getByText('Configure manually'));
+
+    await user.type(screen.getByLabelText(/Display Name/), 'Manual Provider');
+    await user.type(screen.getByLabelText(/API URL/), 'https://manual.example.com');
+    await user.type(screen.getByLabelText(/Available Models/), 'model-a');
+    await user.click(screen.getByLabelText('This provider requires an API key'));
+    await user.type(screen.getByLabelText(/API Key/), 'manual-secret');
+    await addHeader(user, 'Authorization', 'Bearer manual-secret');
+
+    const pendingNames = screen.getAllByPlaceholderText('Header name');
+    const pendingValues = screen.getAllByPlaceholderText('Value');
+    await user.type(pendingNames[pendingNames.length - 1], 'Authorization');
+    await user.type(pendingValues[pendingValues.length - 1], 'Bearer pending-secret');
+    await user.click(screen.getByRole('button', { name: 'Add' }));
+    await user.click(screen.getByRole('button', { name: 'Create Provider' }));
+    await waitFor(() => expect(onSubmit).toHaveBeenCalledOnce());
+
+    await user.click(screen.getByRole('button', { name: '← Back' }));
+    await user.click(screen.getByText('Configure manually'));
+
+    await act(async () => {
+      rejectSubmit?.(new Error('save failed'));
+      await Promise.resolve();
+    });
+
+    expect(screen.getByLabelText(/API Key/)).toHaveValue('');
+    expect(screen.queryByDisplayValue('Bearer manual-secret')).not.toBeInTheDocument();
+    expect(screen.queryByDisplayValue('Bearer pending-secret')).not.toBeInTheDocument();
+    expect(screen.queryByText('A header with this name already exists')).not.toBeInTheDocument();
+    expect(screen.queryByText(/Failed to save provider/)).not.toBeInTheDocument();
+  });
+
+  it('clears form validation when returning to the setup choice', async () => {
+    const user = userEvent.setup();
+    renderForm();
+    await user.click(screen.getByText('Configure manually'));
+    await user.click(screen.getByRole('button', { name: 'Create Provider' }));
+    expect(screen.getByText('Display name is required')).toBeInTheDocument();
+
+    await user.click(screen.getByRole('button', { name: '← Back' }));
+    await user.click(screen.getByText('Configure manually'));
+
+    expect(screen.queryByText('Display name is required')).not.toBeInTheDocument();
+  });
+});
@@ -1,4 +1,4 @@
-import React, { useState, useEffect } from 'react';
+import React, { useState, useEffect, useRef } from 'react';
 import { Input } from '../../../../../ui/input';
 import { Select } from '../../../../../ui/Select';
 import { Button } from '../../../../../ui/button';
@@ -264,11 +264,24 @@ export default function CustomProviderForm({
  const [validationErrors, setValidationErrors] = useState<Record<string, string>>({});
  const [submitError, setSubmitError] = useState<string | null>(null);
  const [showDeleteConfirmation, setShowDeleteConfirmation] = useState(false);
+  const contextVersionRef = useRef(0);

  // Template + step state
  const [selectedTemplate, setSelectedTemplate] = useState<ProviderTemplateDto | null>(null);
  const [step, setStep] = useState<Step>(initialData ? 'form' : 'choice');

+  const clearSensitiveState = () => {
+    contextVersionRef.current += 1;
+    setApiKey('');
+    setHeaders([]);
+    setNewHeaderKey('');
+    setNewHeaderValue('');
+    setHeaderValidationError(null);
+    setInvalidHeaderFields({ key: false, value: false });
+    setValidationErrors({});
+    setSubmitError(null);
+  };
+
  useEffect(() => {
    if (initialData) {
      const engineMap: Record<string, string> = {
@@ -297,6 +310,7 @@ export default function CustomProviderForm({
  }, [initialData]);

  const handleTemplateSelect = (template: ProviderTemplateDto) => {
+    clearSensitiveState();
    setSelectedTemplate(template);

    // Prefill fields from template
@@ -320,6 +334,7 @@ export default function CustomProviderForm({
  };

  const handleClearTemplate = () => {
+    clearSensitiveState();
    setSelectedTemplate(null);
    setDisplayName('');
    setApiUrl('');
@@ -331,6 +346,16 @@ export default function CustomProviderForm({
    setStep('choice');
  };

+  const handleBackToChoice = () => {
+    clearSensitiveState();
+    setStep('choice');
+  };
+
+  const handleCancel = () => {
+    clearSensitiveState();
+    onCancel();
+  };
+
  const handleRequiresAuthChange = (checked: boolean) => {
    setRequiresAuth(checked);
    if (!checked) {
@@ -406,6 +431,7 @@ export default function CustomProviderForm({

  const handleSubmit = async (e: React.FormEvent) => {
    e.preventDefault();
+    const contextVersion = contextVersionRef.current;
    setSubmitError(null);
    setValidationErrors({});

@@ -464,6 +490,7 @@ export default function CustomProviderForm({
        base_path: basePath || undefined,
      });
    } catch (error) {
+      if (contextVersionRef.current !== contextVersion) return;
      console.error('Failed to save custom provider:', error);
      setSubmitError(intl.formatMessage(i18n.submitError));
    }
@@ -522,7 +549,7 @@ export default function CustomProviderForm({
          </div>
        </button>
        <div className="flex justify-end pt-2">
-          <Button type="button" variant="outline" onClick={onCancel}>
+          <Button type="button" variant="outline" onClick={handleCancel}>
            {intl.formatMessage(i18n.cancel)}
          </Button>
        </div>
@@ -534,12 +561,12 @@ export default function CustomProviderForm({
  if (step === 'catalog') {
    return (
      <div className="mt-4">
-        <ProviderCatalogPicker onSelect={handleTemplateSelect} onCancel={onCancel} embedded />
+        <ProviderCatalogPicker onSelect={handleTemplateSelect} onCancel={handleCancel} embedded />
        <div className="flex justify-between pt-4">
-          <Button type="button" variant="ghost" onClick={() => setStep('choice')}>
+          <Button type="button" variant="ghost" onClick={handleBackToChoice}>
            {intl.formatMessage(i18n.back)}
          </Button>
-          <Button type="button" variant="outline" onClick={onCancel}>
+          <Button type="button" variant="outline" onClick={handleCancel}>
            {intl.formatMessage(i18n.cancel)}
          </Button>
        </div>
@@ -587,7 +614,7 @@ export default function CustomProviderForm({

      {/* Back to choice (create without template only) */}
      {!initialData && !selectedTemplate && (
-        <Button type="button" variant="ghost" size="sm" onClick={() => setStep('choice')}>
+        <Button type="button" variant="ghost" size="sm" onClick={handleBackToChoice}>
          {intl.formatMessage(i18n.back)}
        </Button>
      )}
@@ -952,7 +979,7 @@ export default function CustomProviderForm({
              {intl.formatMessage(i18n.deleteProvider)}
            </Button>
          )}
-          <Button type="button" variant="outline" onClick={onCancel}>
+          <Button type="button" variant="outline" onClick={handleCancel}>
            {intl.formatMessage(i18n.cancel)}
          </Button>
          <Button type="submit">