feat: Add Mistral API key validation with format checking#1
feat: Add Mistral API key validation with format checking#1jjshanks wants to merge 1 commit intonewtro:mainfrom
Conversation
- Add format validation for Mistral API keys (32 chars, uppercase, lowercase, number) - Show warning on blur when key format doesn't match expected pattern - Extract validation logic to shared utility for testability - Add Vitest test framework with 13 test cases for validation - Remove duplicate validation code from backend services
📝 WalkthroughWalkthroughThis pull request introduces Vitest testing infrastructure, adds Mistral API key provider support to the settings, and implements client-side API key format validation with real-time feedback in the UI. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/renderer/components/settings/Settings.tsx (1)
432-432: Use the sharedValidationResulttype instead of an inline duplicate.The state type
{ valid: boolean; warning?: string } | nullis structurally identical toValidationResult | nullalready exported from../../../shared/validation.♻️ Proposed refactor
+import { validateKeyFormat, type ValidationResult } from '../../../shared/validation'; ... -const [validationResult, setValidationResult] = useState<{ valid: boolean; warning?: string } | null>(null); +const [validationResult, setValidationResult] = useState<ValidationResult | null>(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/settings/Settings.tsx` at line 432, The state declaration uses an inline type duplicate instead of the shared ValidationResult type; replace the inline type in the useState call for validationResult/setValidationResult with the imported ValidationResult (i.e., change useState<{ valid: boolean; warning?: string } | null> to useState<ValidationResult | null>) and ensure ValidationResult is imported from '../../../shared/validation' at the top of Settings.tsx.src/renderer/components/settings/ApiKeys.tsx (1)
48-54:docsUrldoesn't link directly to the API keys page.Every other provider links to their specific key management page (e.g.,
console.groq.com/keys,platform.openai.com/api-keys). The Mistral entry links to the console root.✏️ Proposed fix
- docsUrl: 'https://console.mistral.ai', + docsUrl: 'https://console.mistral.ai/api-keys',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/settings/ApiKeys.tsx` around lines 48 - 54, Update the Mistral provider entry (the object with id: 'mistral' in ApiKeys.tsx) so its docsUrl points directly to the Mistral API key management page instead of the console root; replace the current 'https://console.mistral.ai' value with the correct key-management URL used by Mistral (e.g., the console API keys path) so users land directly on the page to create/view API keys.src/shared/validation.test.ts (1)
4-64: Good coverage overall; consider one additional edge-case.The suite covers length, character-class variants, trimming, and empty inputs. One gap: a 32-character string that satisfies all three character-class checks but contains special/non-alphanumeric characters (e.g.,
"aA1!@#$%aA1!@#$%aA1!@#$%aA1!@#$") would pass the current validation. If the intent is alphanumeric-only keys, add a regex guard and a corresponding test. If special characters are intentionally permitted, a passing test makes that contract explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/validation.test.ts` around lines 4 - 64, The current validateMistralApiKey allows special characters — update the validator (validateMistralApiKey) to enforce an alphanumeric-only check (e.g. /^[A-Za-z0-9]+$/) and return a warning like "API key must be alphanumeric" when it fails; then add a unit test that passes a 32-character string containing special characters (e.g. "aA1!@#$%aA1!@#$%aA1!@#$%aA1!@#$") and asserts result.valid is false and the warning mentions alphanumeric to make the contract explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/settings/Settings.tsx`:
- Around line 435-439: handleBlur currently writes any validateKeyFormat output
(including { valid: true }) into validationResult which hides the "API key
saved" indicator; change handleBlur to only call setValidationResult when
validateKeyFormat returns an actual warning/invalid state (e.g., result &&
result.valid === false or result.message/warning exists), and otherwise clear
validationResult (set to null) so valid-but-unrecognized providers don't
permanently suppress the saved indicator; update references to
validateKeyFormat, handleBlur, and setValidationResult accordingly.
In `@src/shared/validation.ts`:
- Around line 11-15: The validation currently trims the input inside
validateApiKey (trimmedKey = key.trim()) so the length check can pass while
callers still store the untrimmed key and later get a 401; update validateApiKey
in src/shared/validation.ts to avoid this mismatch by returning the trimmed
value (or an explicit flag) to callers so they store/send the trimmed API key,
or alternatively change the length check to use the raw key (key) so the warning
reflects what will be stored; reference the trimmedKey variable and the function
validateApiKey (or the surrounding validation function) when making the change.
- Around line 6-27: The validateMistralApiKey function currently enforces
hardcoded length and character rules; remove those character-level checks and
just validate presence (trim and ensure non-empty) or replace them with an
optional live-validation call; specifically, update validateMistralApiKey to
trim the input and return { valid: false, warning: 'API key is required' } only
when empty, otherwise return { valid: true } (or implement an actual API-based
verification instead of the length/regex checks).
---
Nitpick comments:
In `@src/renderer/components/settings/ApiKeys.tsx`:
- Around line 48-54: Update the Mistral provider entry (the object with id:
'mistral' in ApiKeys.tsx) so its docsUrl points directly to the Mistral API key
management page instead of the console root; replace the current
'https://console.mistral.ai' value with the correct key-management URL used by
Mistral (e.g., the console API keys path) so users land directly on the page to
create/view API keys.
In `@src/renderer/components/settings/Settings.tsx`:
- Line 432: The state declaration uses an inline type duplicate instead of the
shared ValidationResult type; replace the inline type in the useState call for
validationResult/setValidationResult with the imported ValidationResult (i.e.,
change useState<{ valid: boolean; warning?: string } | null> to
useState<ValidationResult | null>) and ensure ValidationResult is imported from
'../../../shared/validation' at the top of Settings.tsx.
In `@src/shared/validation.test.ts`:
- Around line 4-64: The current validateMistralApiKey allows special characters
— update the validator (validateMistralApiKey) to enforce an alphanumeric-only
check (e.g. /^[A-Za-z0-9]+$/) and return a warning like "API key must be
alphanumeric" when it fails; then add a unit test that passes a 32-character
string containing special characters (e.g. "aA1!@#$%aA1!@#$%aA1!@#$%aA1!@#$")
and asserts result.valid is false and the warning mentions alphanumeric to make
the contract explicit.
| const handleBlur = () => { | ||
| if (!value || !providerId) return; | ||
| const result = validateKeyFormat(value, providerId); | ||
| setValidationResult(result); | ||
| }; |
There was a problem hiding this comment.
handleBlur sets validationResult to { valid: true } for non-Mistral providers, breaking the "API key saved" indicator.
validateKeyFormat returns { valid: true } for all unrecognized providers (openai, groq, anthropic, gemini). After the first blur on any of those inputs, validationResult is non-null, which makes !validationResult && hasValue (Line 508) evaluate to false — permanently hiding "API key saved" for the rest of the session.
The simplest fix is to only update state when there's actually something to surface (i.e., a warning):
🛡️ Proposed fix
const handleBlur = () => {
if (!value || !providerId) return;
const result = validateKeyFormat(value, providerId);
- setValidationResult(result);
+ if (result?.valid === false) {
+ setValidationResult(result);
+ }
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/settings/Settings.tsx` around lines 435 - 439,
handleBlur currently writes any validateKeyFormat output (including { valid:
true }) into validationResult which hides the "API key saved" indicator; change
handleBlur to only call setValidationResult when validateKeyFormat returns an
actual warning/invalid state (e.g., result && result.valid === false or
result.message/warning exists), and otherwise clear validationResult (set to
null) so valid-but-unrecognized providers don't permanently suppress the saved
indicator; update references to validateKeyFormat, handleBlur, and
setValidationResult accordingly.
| export function validateMistralApiKey(key: string): ValidationResult { | ||
| if (!key) { | ||
| return { valid: false, warning: 'API key is required' }; | ||
| } | ||
|
|
||
| const trimmedKey = key.trim(); | ||
|
|
||
| if (trimmedKey.length !== 32) { | ||
| return { valid: false, warning: 'API key should be 32 characters long. This may not be a valid Mistral API key.' }; | ||
| } | ||
| if (!/[A-Z]/.test(trimmedKey)) { | ||
| return { valid: false, warning: 'API key should contain at least 1 uppercase letter. This may not be a valid Mistral API key.' }; | ||
| } | ||
| if (!/[a-z]/.test(trimmedKey)) { | ||
| return { valid: false, warning: 'API key should contain at least 1 lowercase letter. This may not be a valid Mistral API key.' }; | ||
| } | ||
| if (!/[0-9]/.test(trimmedKey)) { | ||
| return { valid: false, warning: 'API key should contain at least 1 number. This may not be a valid Mistral API key.' }; | ||
| } | ||
|
|
||
| return { valid: true }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Mistral AI API key format length prefix
💡 Result:
- Prefix / format: Mistral API keys are not guaranteed to have any fixed prefix (i.e., they’re essentially an opaque token). GitGuardian’s detector notes they are “Prefixed: False”. [1]
- Length: Mistral does not publicly document a fixed key length (so you shouldn’t validate keys by length/prefix). [1]
- How they’re used in requests: Send the raw key as a Bearer token in the
Authorizationheader:Authorization: Bearer <your_api_key>. [2]
Sources:
[1] GitGuardian – Mistral AI API Key (detector details incl. “Prefixed: False”)
[2] TypingMind docs – Mistral AI (shows Authorization: Bearer your_api_key)
Remove or relax the hardcoded validation constraints for Mistral API keys.
Mistral does not publicly document a fixed key length or required character composition. API keys are opaque tokens with no guaranteed prefix (not "sk-" or otherwise) and no documented format restrictions. The current validation requiring exactly 32 characters with uppercase, lowercase, and numeric characters will reject valid Mistral keys that don't match this pattern, generating false warnings for legitimate users.
Either validate keys against actual API responses or remove these character-level checks entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/validation.ts` around lines 6 - 27, The validateMistralApiKey
function currently enforces hardcoded length and character rules; remove those
character-level checks and just validate presence (trim and ensure non-empty) or
replace them with an optional live-validation call; specifically, update
validateMistralApiKey to trim the input and return { valid: false, warning: 'API
key is required' } only when empty, otherwise return { valid: true } (or
implement an actual API-based verification instead of the length/regex checks).
| const trimmedKey = key.trim(); | ||
|
|
||
| if (trimmedKey.length !== 32) { | ||
| return { valid: false, warning: 'API key should be 32 characters long. This may not be a valid Mistral API key.' }; | ||
| } |
There was a problem hiding this comment.
Trimming during validation but not before storage creates a false-positive.
trimmedKey is used for all validation checks, but the raw (potentially whitespace-padded) key is what callers store and later send to the API. A user who pastes validKey32chars (with surrounding spaces) will see the format check pass, but the stored value will fail the Mistral API with a 401.
Consider either (a) validating the untrimmed input so the warning is accurate, or (b) also returning/indicating to callers that the key should be trimmed:
🛡️ Option A — validate the raw input (no silent trimming)
export function validateMistralApiKey(key: string): ValidationResult {
if (!key) {
return { valid: false, warning: 'API key is required' };
}
- const trimmedKey = key.trim();
+ const trimmedKey = key.trim();
+ if (trimmedKey !== key) {
+ return { valid: false, warning: 'API key must not contain leading or trailing whitespace.' };
+ }
if (trimmedKey.length !== 32) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const trimmedKey = key.trim(); | |
| if (trimmedKey.length !== 32) { | |
| return { valid: false, warning: 'API key should be 32 characters long. This may not be a valid Mistral API key.' }; | |
| } | |
| const trimmedKey = key.trim(); | |
| if (trimmedKey !== key) { | |
| return { valid: false, warning: 'API key must not contain leading or trailing whitespace.' }; | |
| } | |
| if (trimmedKey.length !== 32) { | |
| return { valid: false, warning: 'API key should be 32 characters long. This may not be a valid Mistral API key.' }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/validation.ts` around lines 11 - 15, The validation currently
trims the input inside validateApiKey (trimmedKey = key.trim()) so the length
check can pass while callers still store the untrimmed key and later get a 401;
update validateApiKey in src/shared/validation.ts to avoid this mismatch by
returning the trimmed value (or an explicit flag) to callers so they store/send
the trimmed API key, or alternatively change the length check to use the raw key
(key) so the warning reflects what will be stored; reference the trimmedKey
variable and the function validateApiKey (or the surrounding validation
function) when making the change.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores