From 49518300dcd3db1cf5d01108408bd48f76470570 Mon Sep 17 00:00:00 2001
From: Lingxuan Zuo <skyzlxuan@gmail.com>
Date: Fri, 10 Apr 2026 15:15:29 +0800
Subject: [PATCH] Fix provider API key validation trimming (#810)

---
 src/components/settings/ProvidersSettings.tsx | 18 +++--
 src/lib/providers.ts                          |  6 +-
 src/pages/Setup/index.tsx                     | 16 +++-
 src/stores/providers.ts                       |  4 +-
 tests/e2e/provider-lifecycle.spec.ts          | 79 +++++++++++++++++++
 tests/unit/provider-store-validation.test.ts  | 42 ++++++++++
 tests/unit/providers.test.ts                  |  2 +
 7 files changed, 155 insertions(+), 12 deletions(-)
 create mode 100644 tests/unit/provider-store-validation.test.ts

diff --git a/src/components/settings/ProvidersSettings.tsx b/src/components/settings/ProvidersSettings.tsx
index a9199e03d..78055ad33 100644
--- a/src/components/settings/ProvidersSettings.tsx
+++ b/src/components/settings/ProvidersSettings.tsx
@@ -34,6 +34,7 @@ import {
   getProviderDocsUrl,
   type ProviderType,
   getProviderIconUrl,
+  normalizeProviderApiKeyInput,
   resolveProviderApiKeyForSave,
   resolveProviderModelForSave,
   shouldShowProviderModelId,
@@ -424,10 +425,11 @@ function ProviderCard({
     try {
       const payload: { newApiKey?: string; updates?: Partial<ProviderConfig> } = {};
       const normalizedFallbackModels = normalizeFallbackModels(fallbackModelsText.split('\n'));
+      const normalizedNewKey = normalizeProviderApiKeyInput(newKey);
 
-      if (newKey.trim()) {
+      if (normalizedNewKey) {
         setValidating(true);
-        const result = await onValidateKey(newKey, {
+        const result = await onValidateKey(normalizedNewKey, {
           baseUrl: baseUrl.trim() || undefined,
           apiProtocol: (account.vendorId === 'custom' || account.vendorId === 'ollama') ? apiProtocol : undefined,
         });
@@ -437,7 +439,7 @@ function ProviderCard({
           setSaving(false);
           return;
         }
-        payload.newApiKey = newKey.trim();
+        payload.newApiKey = normalizedNewKey;
       }
 
       {
@@ -1164,13 +1166,14 @@ function AddProviderDialog({
     try {
       // Validate key first if the provider requires one and a key was entered
       const requiresKey = typeInfo?.requiresApiKey ?? false;
-      if (requiresKey && !apiKey.trim()) {
+      const normalizedApiKey = normalizeProviderApiKeyInput(apiKey);
+      if (requiresKey && !normalizedApiKey) {
         setValidationError(t('aiProviders.toast.invalidKey')); // reusing invalid key msg or should add 'required' msg? null checks
         setSaving(false);
         return;
       }
-      if (requiresKey && apiKey) {
-        const result = await onValidateKey(selectedType, apiKey, {
+      if (requiresKey && normalizedApiKey) {
+        const result = await onValidateKey(selectedType, normalizedApiKey, {
           baseUrl: baseUrl.trim() || undefined,
           apiProtocol: (selectedType === 'custom' || selectedType === 'ollama') ? apiProtocol : undefined,
         });
@@ -1191,7 +1194,7 @@ function AddProviderDialog({
       await onAdd(
         selectedType,
         name || (typeInfo?.id === 'custom' ? t('aiProviders.custom') : typeInfo?.name) || selectedType,
-        apiKey.trim(),
+        normalizedApiKey,
         {
           baseUrl: baseUrl.trim() || undefined,
           apiProtocol: (selectedType === 'custom' || selectedType === 'ollama') ? apiProtocol : undefined,
@@ -1655,6 +1658,7 @@ function AddProviderDialog({
 
               <div className="flex justify-end gap-3">
                 <Button
+                  data-testid="add-provider-submit-button"
                   onClick={handleAdd}
                   className={cn("rounded-full px-8 h-[42px] text-[13px] font-semibold bg-[#0a84ff] hover:bg-[#007aff] text-white shadow-sm", useOAuthFlow && "hidden")}
                   disabled={!selectedType || saving || (showModelIdField && modelId.trim().length === 0)}
diff --git a/src/lib/providers.ts b/src/lib/providers.ts
index 4cb1af123..74a570727 100644
--- a/src/lib/providers.ts
+++ b/src/lib/providers.ts
@@ -246,9 +246,13 @@ export function resolveProviderModelForSave(
   return trimmedModelId || provider?.defaultModelId || undefined;
 }
 
+export function normalizeProviderApiKeyInput(apiKey: string): string {
+  return apiKey.trim();
+}
+
 /** Normalize provider API key before saving; Ollama uses a local placeholder when blank. */
 export function resolveProviderApiKeyForSave(type: ProviderType | string, apiKey: string): string | undefined {
-  const trimmed = apiKey.trim();
+  const trimmed = normalizeProviderApiKeyInput(apiKey);
   if (type === 'ollama') {
     return trimmed || OLLAMA_PLACEHOLDER_API_KEY;
   }
diff --git a/src/pages/Setup/index.tsx b/src/pages/Setup/index.tsx
index fa80a417f..33c8e29f6 100644
--- a/src/pages/Setup/index.tsx
+++ b/src/pages/Setup/index.tsx
@@ -99,6 +99,7 @@ import {
   type ProviderTypeInfo,
   getProviderDocsUrl,
   getProviderIconUrl,
+  normalizeProviderApiKeyInput,
   resolveProviderApiKeyForSave,
   resolveProviderModelForSave,
   shouldInvertInDark,
@@ -1009,6 +1010,7 @@ function ProviderContent({
   const isOAuth = selectedProviderData?.isOAuth ?? false;
   const supportsApiKey = selectedProviderData?.supportsApiKey ?? false;
   const useOAuthFlow = isOAuth && (!supportsApiKey || authMode === 'oauth');
+  const normalizedApiKey = normalizeProviderApiKeyInput(apiKey);
 
   const handleValidateAndSave = async () => {
     if (!selectedProvider) return;
@@ -1034,11 +1036,19 @@ function ProviderContent({
     try {
       // Validate key if the provider requires one and a key was entered
       const isApiKeyRequired = requiresKey || (supportsApiKey && authMode === 'apikey');
-      if (isApiKeyRequired && apiKey) {
+      if (isApiKeyRequired && !normalizedApiKey) {
+        setKeyValid(false);
+        onConfiguredChange(false);
+        toast.error(t('provider.invalid'));
+        setValidating(false);
+        return;
+      }
+
+      if (isApiKeyRequired) {
         const result = await invokeIpc(
           'provider:validateKey',
           selectedAccountId || selectedProvider,
-          apiKey,
+          normalizedApiKey,
           {
             baseUrl: baseUrl.trim() || undefined,
             apiProtocol: (selectedProvider === 'custom' || selectedProvider === 'ollama')
@@ -1146,7 +1156,7 @@ function ProviderContent({
   const isApiKeyRequired = requiresKey || (supportsApiKey && authMode === 'apikey');
   const canSubmit =
     selectedProvider
-    && (isApiKeyRequired ? apiKey.length > 0 : true)
+    && (isApiKeyRequired ? normalizedApiKey.length > 0 : true)
     && (showModelIdField ? modelId.trim().length > 0 : true)
     && !useOAuthFlow;
 
diff --git a/src/stores/providers.ts b/src/stores/providers.ts
index 1863bbe7f..fb74d4349 100644
--- a/src/stores/providers.ts
+++ b/src/stores/providers.ts
@@ -9,6 +9,7 @@ import type {
   ProviderVendorInfo,
   ProviderWithKeyInfo,
 } from '@/lib/providers';
+import { normalizeProviderApiKeyInput } from '@/lib/providers';
 import { hostApiFetch } from '@/lib/host-api';
 import {
   fetchProviderSnapshot,
@@ -326,9 +327,10 @@ export const useProviderStore = create<ProviderState>((set, get) => ({
   
   validateAccountApiKey: async (providerId, apiKey, options) => {
     try {
+      const normalizedApiKey = normalizeProviderApiKeyInput(apiKey);
       const result = await hostApiFetch<{ valid: boolean; error?: string }>('/api/providers/validate', {
         method: 'POST',
-        body: JSON.stringify({ providerId, apiKey, options }),
+        body: JSON.stringify({ providerId, apiKey: normalizedApiKey, options }),
       });
       return result;
     } catch (error) {
diff --git a/tests/e2e/provider-lifecycle.spec.ts b/tests/e2e/provider-lifecycle.spec.ts
index 4dfd0a043..de30420ad 100644
--- a/tests/e2e/provider-lifecycle.spec.ts
+++ b/tests/e2e/provider-lifecycle.spec.ts
@@ -62,4 +62,83 @@ test.describe('ClawX provider lifecycle', () => {
       await relaunchedApp.close();
     }
   });
+
+  test('trims whitespace before validating and saving a custom provider key', async ({ electronApp, page }) => {
+    await completeSetup(page);
+
+    await electronApp.evaluate(async ({ app: _app }) => {
+      const { ipcMain } = process.mainModule!.require('electron') as typeof import('electron');
+
+      let accounts: Array<Record<string, unknown>> = [];
+      let statuses: Array<Record<string, unknown>> = [];
+      let defaultAccountId: string | null = null;
+
+      const respond = (json: unknown, status = 200) => ({
+        ok: true,
+        data: {
+          status,
+          ok: status >= 200 && status < 300,
+          json,
+        },
+      });
+
+      ipcMain.removeHandler('hostapi:fetch');
+      ipcMain.handle('hostapi:fetch', async (_event: unknown, request: { path?: string; method?: string; body?: string | null }) => {
+        const path = request?.path ?? '';
+        const method = request?.method ?? 'GET';
+        const body = request?.body ? JSON.parse(request.body) : null;
+
+        if (path === '/api/provider-accounts' && method === 'GET') return respond(accounts);
+        if (path === '/api/providers' && method === 'GET') return respond(statuses);
+        if (path === '/api/provider-vendors' && method === 'GET') return respond([]);
+        if (path === '/api/provider-accounts/default' && method === 'GET') return respond({ accountId: defaultAccountId });
+
+        if (path === '/api/providers/validate' && method === 'POST') {
+          if (body?.apiKey !== 'sk-lm-test') {
+            return respond({ valid: false, error: `unexpected key: ${String(body?.apiKey)}` }, 400);
+          }
+          return respond({ valid: true });
+        }
+
+        if (path === '/api/provider-accounts' && method === 'POST') {
+          accounts = [body.account];
+          statuses = [{
+            id: body.account.id,
+            name: body.account.label,
+            type: body.account.vendorId,
+            baseUrl: body.account.baseUrl,
+            model: body.account.model,
+            enabled: body.account.enabled,
+            createdAt: body.account.createdAt,
+            updatedAt: body.account.updatedAt,
+            hasKey: Boolean(body.apiKey),
+            keyMasked: body.apiKey ? 'sk-***' : null,
+          }];
+          return respond({ success: true });
+        }
+
+        if (path === '/api/provider-accounts/default' && method === 'PUT') {
+          defaultAccountId = body?.accountId ?? null;
+          return respond({ success: true });
+        }
+
+        return respond({});
+      });
+    });
+
+    await page.getByTestId('sidebar-nav-models').click();
+    await expect(page.getByTestId('providers-settings')).toBeVisible();
+
+    await page.getByTestId('providers-add-button').click();
+    await expect(page.getByTestId('add-provider-dialog')).toBeVisible();
+
+    await page.getByTestId('add-provider-type-custom').click();
+    await page.getByTestId('add-provider-name-input').fill('LM Studio Local');
+    await page.getByTestId('add-provider-api-key-input').fill('  sk-lm-test \n');
+    await page.getByTestId('add-provider-base-url-input').fill('http://127.0.0.1:1234/v1');
+    await page.getByTestId('add-provider-model-id-input').fill('local-model');
+    await page.getByTestId('add-provider-submit-button').click();
+
+    await expect(page.getByTestId('provider-card-custom')).toContainText('LM Studio Local');
+  });
 });
diff --git a/tests/unit/provider-store-validation.test.ts b/tests/unit/provider-store-validation.test.ts
new file mode 100644
index 000000000..68ceeb49d
--- /dev/null
+++ b/tests/unit/provider-store-validation.test.ts
@@ -0,0 +1,42 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const mockFetchProviderSnapshot = vi.fn();
+const mockHostApiFetch = vi.fn();
+
+vi.mock('@/lib/provider-accounts', () => ({
+  fetchProviderSnapshot: (...args: unknown[]) => mockFetchProviderSnapshot(...args),
+}));
+
+vi.mock('@/lib/host-api', () => ({
+  hostApiFetch: (...args: unknown[]) => mockHostApiFetch(...args),
+}));
+
+import { useProviderStore } from '@/stores/providers';
+
+describe('useProviderStore – validateAccountApiKey()', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('trims API keys before sending provider validation requests', async () => {
+    mockHostApiFetch.mockResolvedValueOnce({ valid: true });
+
+    const result = await useProviderStore.getState().validateAccountApiKey('custom', '  sk-lm-test \n', {
+      baseUrl: 'http://127.0.0.1:1234/v1',
+      apiProtocol: 'openai-completions',
+    });
+
+    expect(result).toEqual({ valid: true });
+    expect(mockHostApiFetch).toHaveBeenCalledWith('/api/providers/validate', {
+      method: 'POST',
+      body: JSON.stringify({
+        providerId: 'custom',
+        apiKey: 'sk-lm-test',
+        options: {
+          baseUrl: 'http://127.0.0.1:1234/v1',
+          apiProtocol: 'openai-completions',
+        },
+      }),
+    });
+  });
+});
diff --git a/tests/unit/providers.test.ts b/tests/unit/providers.test.ts
index bec958a1a..bb0b836c1 100644
--- a/tests/unit/providers.test.ts
+++ b/tests/unit/providers.test.ts
@@ -1,5 +1,6 @@
 import { describe, expect, it } from 'vitest';
 import {
+  normalizeProviderApiKeyInput,
   PROVIDER_TYPES,
   PROVIDER_TYPE_INFO,
   getProviderDocsUrl,
@@ -171,6 +172,7 @@ describe('provider metadata', () => {
   });
 
   it('normalizes provider API keys for save flow', () => {
+    expect(normalizeProviderApiKeyInput('  sk-test \n')).toBe('sk-test');
     expect(resolveProviderApiKeyForSave('ollama', '')).toBe('ollama-local');
     expect(resolveProviderApiKeyForSave('ollama', '   ')).toBe('ollama-local');
     expect(resolveProviderApiKeyForSave('ollama', 'real-key')).toBe('real-key');