Fix provider API key validation trimming#810
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c269b16c30
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const isApiKeyRequired = requiresKey || (supportsApiKey && authMode === 'apikey'); | ||
| if (isApiKeyRequired && apiKey) { | ||
| const normalizedApiKey = normalizeProviderApiKeyInput(apiKey); | ||
| if (isApiKeyRequired && normalizedApiKey) { |
There was a problem hiding this comment.
Require non-empty normalized key in setup flow
When isApiKeyRequired is true, this condition only validates if normalizedApiKey is truthy; a whitespace-only input now skips validation and falls into the success path. In that case, resolveProviderApiKeyForSave(selectedProvider, apiKey) later trims to an empty value, so setup can save a required-key provider without a key and still mark configuration successful. This is a regression from the prior behavior where whitespace triggered validation and was rejected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR fixes false 401 validation failures by normalizing (trimming) provider API key input before validation across onboarding (Setup) and Settings add/edit flows, and adds regression coverage to prevent the issue from returning.
Changes:
- Introduces
normalizeProviderApiKeyInput()and applies it consistently in save/validate flows. - Updates
useProviderStore.validateAccountApiKey()to trim the key before sending validation requests. - Adds unit + Electron E2E coverage for whitespace-trimmed validation and saving.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/providers.ts |
Adds normalizeProviderApiKeyInput() and reuses it in resolveProviderApiKeyForSave(). |
src/stores/providers.ts |
Trims API key before calling /api/providers/validate. |
src/pages/Setup/index.tsx |
Trims API key before IPC validation in onboarding flow. |
src/components/settings/ProvidersSettings.tsx |
Trims API key before validation/save in add/edit provider UI; adds a missing test id for submit button. |
tests/unit/providers.test.ts |
Adds unit coverage for API key normalization helper. |
tests/unit/provider-store-validation.test.ts |
Adds regression test ensuring validation requests receive trimmed keys. |
tests/e2e/provider-lifecycle.spec.ts |
Adds E2E regression test ensuring whitespace is trimmed before validate+save for custom provider key. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Validate key if the provider requires one and a key was entered | ||
| const isApiKeyRequired = requiresKey || (supportsApiKey && authMode === 'apikey'); | ||
| if (isApiKeyRequired && apiKey) { | ||
| const normalizedApiKey = normalizeProviderApiKeyInput(apiKey); | ||
| if (isApiKeyRequired && normalizedApiKey) { | ||
| const result = await invokeIpc( |
There was a problem hiding this comment.
In the Setup flow, whitespace-only API keys now bypass validation: normalizedApiKey becomes '', so the if (isApiKeyRequired && normalizedApiKey) block is skipped and the else marks the key as valid, allowing the save to proceed with effectiveApiKey resolving to undefined. If the API key is required, treat an empty normalized key as an error (and also ensure the submit enablement uses the normalized value) so required-key providers can’t be configured with only spaces/newlines.
| toast.error(result.error || t('aiProviders.toast.invalidKey')); | ||
| setSaving(false); | ||
| return; | ||
| } | ||
| payload.newApiKey = newKey.trim(); | ||
| payload.newApiKey = resolveProviderApiKeyForSave(account.vendorId, normalizedNewKey) as string; | ||
| } |
There was a problem hiding this comment.
Avoid casting the result of resolveProviderApiKeyForSave(... ) as string. This hides potential undefined values at compile time and is unnecessary here because normalizedNewKey is already non-empty in this branch. Prefer assigning normalizedNewKey directly, or use a non-null assertion with a short comment explaining why it can’t be undefined.
…ueCell-ai#810 ValueCell-ai#839 ValueCell-ai#897 ValueCell-ai#907) Add DeepSeek provider, Moonshot Global endpoint, update Kimi to k2.6, fix API key validation trimming.
Summary
Root Cause
Provider configuration paths were validating the raw API key input but saving the trimmed value. If a user pasted an LM Studio token with leading or trailing whitespace, validation failed before the normalized key could ever be saved.
User Impact
Custom and compatible provider setup is now more resilient to pasted API keys, especially LM Studio-style local gateway configurations.
Validation
pnpm test tests/unit/providers.test.ts tests/unit/provider-store-validation.test.tspnpm run typecheckpnpm run build:vitepnpm exec playwright test tests/e2e/provider-lifecycle.spec.ts -g "trims whitespace before validating and saving a custom provider key"