From 542d4635c49e79888d2b5f184682a238f24bb738 Mon Sep 17 00:00:00 2001 From: YoVinchen Date: Mon, 26 Jan 2026 22:46:24 +0800 Subject: [PATCH] fix(config): improve Gemini common config parsing robustness - Add ENV format quote stripping (KEY="value" -> value) - Add toast warnings for filtered forbidden keys - Unify error codes with GEMINI_CONFIG_ERROR_CODES constants - Remove duplicate isPlainObject, use shared implementation - Fix type guard for filter entries --- src-tauri/src/config_merge.rs | 110 +++++++---- src-tauri/src/services/provider/live.rs | 11 +- .../providers/EditProviderDialog.tsx | 66 ++++--- .../providers/forms/ProviderForm.tsx | 51 ++--- .../forms/hooks/useGeminiCommonConfig.ts | 66 +++---- src/utils/providerConfigUtils.ts | 187 ++++++++++++++++-- 6 files changed, 343 insertions(+), 148 deletions(-) diff --git a/src-tauri/src/config_merge.rs b/src-tauri/src/config_merge.rs index 89967efae..91ad107d1 100644 --- a/src-tauri/src/config_merge.rs +++ b/src-tauri/src/config_merge.rs @@ -620,7 +620,7 @@ fn merge_claude_config(custom_config: &JsonValue, common_snippet: &str) -> Merge // Return warning without logging - caller will log if needed return MergeResult { config: custom_config.clone(), - warning: Some(format!("Claude common config parse error: {e}")), + warning: Some(format!("COMMON_CONFIG_PARSE_ERROR: {e}")), }; } }; @@ -642,7 +642,7 @@ fn merge_codex_config(custom_config: &JsonValue, common_snippet: &str) -> MergeR compute_final_toml_config_str(config_str, common_snippet, true); if let Some(e) = error { // Return warning without logging - caller will log if needed - warning = Some(format!("Codex TOML merge warning: {e}")); + warning = Some(format!("CODEX_TOML_MERGE_ERROR: {e}")); } obj.insert("config".to_string(), JsonValue::String(merged_toml)); } @@ -656,44 +656,25 @@ fn merge_codex_config(custom_config: &JsonValue, common_snippet: &str) -> MergeR /// Merge Gemini config (JSON format for env field). /// -/// Gemini common config can be stored in two formats: -/// - Wrapped: `{"env": {"KEY": "VALUE", ...}}` (matches provider settings_config structure) -/// - Flat: `{"KEY": "VALUE", ...}` (simpler format used by frontend) +/// Gemini common config can be stored in three formats: +/// - ENV format: KEY=VALUE lines (one per line) +/// - Flat JSON: `{"KEY": "VALUE", ...}` +/// - Wrapped JSON: `{"env": {"KEY": "VALUE", ...}}` /// -/// This function supports both formats for backward compatibility. +/// This function supports all formats for backward compatibility. fn merge_gemini_config(custom_config: &JsonValue, common_snippet: &str) -> MergeResult { - let common_value: JsonValue = match serde_json::from_str(common_snippet) { - Ok(v) => v, - Err(e) => { - // Return warning without logging - caller will log if needed - return MergeResult { - config: custom_config.clone(), - warning: Some(format!("Gemini common config parse error: {e}")), - }; - } - }; + // Parse common config (try JSON first, then ENV format) + let common_env = parse_gemini_common_snippet(common_snippet); + + if common_env.is_empty() { + return MergeResult { + config: custom_config.clone(), + warning: None, + }; + } let mut merged_config = custom_config.clone(); - // Get the common env object (support both wrapped and flat formats) - let common_env = match common_value.as_object() { - Some(obj) => { - // Check if it's wrapped format {"env": {...}} - if let Some(env_value) = obj.get("env").and_then(|v| v.as_object()) { - env_value.clone() - } else { - // Flat format {"KEY": "VALUE", ...} - obj.clone() - } - } - None => { - return MergeResult { - config: custom_config.clone(), - warning: Some("COMMON_CONFIG_NOT_OBJECT".to_string()), - }; - } - }; - // Merge only the env field // If custom config has env, merge common into it; otherwise initialize with common env if let Some(merged_obj) = merged_config.as_object_mut() { @@ -718,6 +699,65 @@ fn merge_gemini_config(custom_config: &JsonValue, common_snippet: &str) -> Merge } } +/// Parse Gemini common config snippet supporting multiple formats. +/// +/// Formats supported: +/// - ENV format: KEY=VALUE lines +/// - Flat JSON: {"KEY": "VALUE", ...} +/// - Wrapped JSON: {"env": {"KEY": "VALUE", ...}} +fn parse_gemini_common_snippet(snippet: &str) -> serde_json::Map { + let trimmed = snippet.trim(); + if trimmed.is_empty() { + return serde_json::Map::new(); + } + + // Try JSON first + if let Ok(parsed) = serde_json::from_str::(trimmed) { + if let Some(obj) = parsed.as_object() { + // Check if it's wrapped format {"env": {...}} + if let Some(env_value) = obj.get("env").and_then(|v| v.as_object()) { + return env_value.clone(); + } + // Flat format + return obj.clone(); + } + } + + // Parse as ENV format (KEY=VALUE lines) + let mut result = serde_json::Map::new(); + for line in trimmed.lines() { + let line_trimmed = line.trim(); + if line_trimmed.is_empty() || line_trimmed.starts_with('#') { + continue; + } + if let Some(equal_index) = line_trimmed.find('=') { + let key = line_trimmed[..equal_index].trim(); + let raw_value = line_trimmed[equal_index + 1..].trim(); + // Strip surrounding quotes (single or double) from value + // e.g., KEY="value" or KEY='value' -> value + let value = strip_env_quotes(raw_value); + if !key.is_empty() { + result.insert(key.to_string(), JsonValue::String(value.to_string())); + } + } + } + result +} + +/// Strip surrounding quotes from ENV value. +/// Supports both single and double quotes: "value" -> value, 'value' -> value +fn strip_env_quotes(s: &str) -> &str { + let bytes = s.as_bytes(); + if bytes.len() >= 2 { + let first = bytes[0]; + let last = bytes[bytes.len() - 1]; + if (first == b'"' && last == b'"') || (first == b'\'' && last == b'\'') { + return &s[1..s.len() - 1]; + } + } + s +} + // ============================================================================ // Unit Tests // ============================================================================ diff --git a/src-tauri/src/services/provider/live.rs b/src-tauri/src/services/provider/live.rs index a6dc111ad..69747c336 100644 --- a/src-tauri/src/services/provider/live.rs +++ b/src-tauri/src/services/provider/live.rs @@ -167,15 +167,18 @@ fn write_live_snapshot_internal( } AppType::Codex => { let obj = config_to_write.as_object().ok_or_else(|| { - AppError::Config("Codex provider settings_config must be a JSON object".to_string()) + AppError::Config( + "CODEX_CONFIG_NOT_OBJECT: settings_config must be a JSON object".to_string(), + ) })?; let auth = obj.get("auth").ok_or_else(|| { - AppError::Config("Codex provider settings_config missing 'auth' field".to_string()) + AppError::Config( + "CODEX_CONFIG_MISSING_AUTH: settings_config missing 'auth' field".to_string(), + ) })?; let config_str = obj.get("config").and_then(|v| v.as_str()).ok_or_else(|| { AppError::Config( - "Codex provider settings_config missing 'config' field or not a string" - .to_string(), + "CODEX_CONFIG_MISSING_CONFIG: settings_config missing 'config' field or not a string".to_string(), ) })?; diff --git a/src/components/providers/EditProviderDialog.tsx b/src/components/providers/EditProviderDialog.tsx index 45baadb36..3ad53a5d8 100644 --- a/src/components/providers/EditProviderDialog.tsx +++ b/src/components/providers/EditProviderDialog.tsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; +import { toast } from "sonner"; import { Save } from "lucide-react"; import { Button } from "@/components/ui/button"; import { FullScreenPanel } from "@/components/common/FullScreenPanel"; @@ -11,6 +12,7 @@ import { import { providersApi, vscodeApi, configApi, type AppId } from "@/lib/api"; import { extractDifference, isPlainObject } from "@/utils/configMerge"; import { extractTomlDifference } from "@/utils/tomlConfigMerge"; +import { parseGeminiCommonConfigSnippet } from "@/utils/providerConfigUtils"; interface EditProviderDialogProps { open: boolean; @@ -114,42 +116,46 @@ export function EditProviderDialog({ setLiveSettings(live); } } else if (appId === "gemini") { - // Gemini: common config supports two formats: + // Gemini: common config supports three formats: + // - ENV format: KEY=VALUE lines // - Flat JSON: {"KEY": "VALUE", ...} // - Wrapped JSON: {"env": {"KEY": "VALUE", ...}} const liveEnv = (live as { env?: Record }).env ?? {}; - // Parse common config as JSON with format detection - const parsedSnippet = JSON.parse( - commonSnippet.trim(), - ) as Record; - // Check if wrapped format {"env": {...}} - const commonEnvRaw = - parsedSnippet.env && - typeof parsedSnippet.env === "object" - ? (parsedSnippet.env as Record) - : parsedSnippet; - // Convert to string record, filtering out non-string values - const commonEnvStrObj: Record = {}; - for (const [key, value] of Object.entries(commonEnvRaw)) { - if (typeof value === "string") { - commonEnvStrObj[key] = value; - } - } - if ( - isPlainObject(liveEnv) && - isPlainObject(commonEnvStrObj) - ) { - const { customConfig } = extractDifference( - liveEnv, - commonEnvStrObj, + + // Use shared parser with validation + const parseResult = parseGeminiCommonConfigSnippet( + commonSnippet, + { strictForbiddenKeys: false }, + ); + + if (parseResult.error) { + console.warn( + "[EditProviderDialog] Gemini common config parse error:", + parseResult.error, ); - setLiveSettings({ - ...live, - env: customConfig, - }); - } else { setLiveSettings(live); + } else { + // Show warning toast if keys were filtered + if (parseResult.warning) { + toast.warning(parseResult.warning); + } + + if ( + isPlainObject(liveEnv) && + Object.keys(parseResult.env).length > 0 + ) { + const { customConfig } = extractDifference( + liveEnv, + parseResult.env, + ); + setLiveSettings({ + ...live, + env: customConfig, + }); + } else { + setLiveSettings(live); + } } } else { // Claude: 处理 JSON 格式 diff --git a/src/components/providers/forms/ProviderForm.tsx b/src/components/providers/forms/ProviderForm.tsx index 60db93800..79093cc87 100644 --- a/src/components/providers/forms/ProviderForm.tsx +++ b/src/components/providers/forms/ProviderForm.tsx @@ -38,6 +38,7 @@ import { applyTemplateValues } from "@/utils/providerConfigUtils"; import { mergeProviderMeta } from "@/utils/providerMetaUtils"; import { extractDifference, isPlainObject } from "@/utils/configMerge"; import { extractTomlDifference } from "@/utils/tomlConfigMerge"; +import { parseGeminiCommonConfigSnippet } from "@/utils/providerConfigUtils"; import { getCodexCustomTemplate } from "@/config/codexTemplates"; import CodexConfigEditor from "./CodexConfigEditor"; import { CommonConfigEditor } from "./CommonConfigEditor"; @@ -68,33 +69,33 @@ import { import { useProvidersQuery } from "@/lib/query/queries"; /** - * Parse Gemini common config snippet. - * Supports two formats: + * Parse Gemini common config snippet for difference extraction. + * Uses shared parser with non-strict forbidden keys (filter instead of reject). + * + * Supports three formats: + * - ENV format: KEY=VALUE lines (one per line) * - Flat JSON: {"KEY": "VALUE", ...} * - Wrapped JSON: {"env": {"KEY": "VALUE", ...}} * - * Returns empty object if parsing fails. + * Returns { env, warning } - caller should display warning via toast if present. */ -function parseGeminiCommonConfig(snippet: string): Record { - try { - const parsed = JSON.parse(snippet); - if (!parsed || typeof parsed !== "object") { - return {}; - } - // Check if it's wrapped format {"env": {...}} - const envObj = - parsed.env && typeof parsed.env === "object" ? parsed.env : parsed; - // Convert to string record - const result: Record = {}; - for (const [key, value] of Object.entries(envObj)) { - if (typeof value === "string") { - result[key] = value; - } - } - return result; - } catch { - return {}; +function parseGeminiCommonConfig(snippet: string): { + env: Record; + warning?: string; +} { + const result = parseGeminiCommonConfigSnippet(snippet, { + strictForbiddenKeys: false, // Don't fail, just filter + }); + + if (result.error) { + console.warn( + "[ProviderForm] Gemini common config parse error:", + result.error, + ); + return { env: {} }; } + + return { env: result.env, warning: result.warning }; } const CLAUDE_DEFAULT_CONFIG = JSON.stringify({ env: {} }, null, 2); @@ -973,9 +974,13 @@ export function ProviderForm({ if (useGeminiCommonConfigFlag && geminiCommonConfigSnippet.trim()) { // Parse common config as JSON (flat {"KEY": "VALUE"} format) // Note: geminiCommonConfigSnippet is stored as JSON by useGeminiCommonConfig hook - const commonEnvObj = parseGeminiCommonConfig( + const { env: commonEnvObj, warning } = parseGeminiCommonConfig( geminiCommonConfigSnippet.trim(), ); + // Show warning toast if keys were filtered + if (warning) { + toast.warning(warning); + } if (isPlainObject(envObj) && isPlainObject(commonEnvObj)) { const { customConfig } = extractDifference(envObj, commonEnvObj); // Convert to string record with type guard to avoid type assertion issues diff --git a/src/components/providers/forms/hooks/useGeminiCommonConfig.ts b/src/components/providers/forms/hooks/useGeminiCommonConfig.ts index 494e76892..8b2e1ffde 100644 --- a/src/components/providers/forms/hooks/useGeminiCommonConfig.ts +++ b/src/components/providers/forms/hooks/useGeminiCommonConfig.ts @@ -3,14 +3,10 @@ import { useTranslation } from "react-i18next"; import { toast } from "sonner"; import { configApi } from "@/lib/api"; import { - GEMINI_COMMON_ENV_FORBIDDEN_KEYS, - type GeminiForbiddenEnvKey, + parseGeminiCommonConfigSnippet, + GEMINI_CONFIG_ERROR_CODES, } from "@/utils/providerConfigUtils"; -import { - computeFinalConfig, - extractDifference, - isPlainObject, -} from "@/utils/configMerge"; +import { computeFinalConfig, extractDifference } from "@/utils/configMerge"; import type { ProviderMeta } from "@/types"; const LEGACY_STORAGE_KEY = "cc-switch:gemini-common-config-snippet"; @@ -131,54 +127,38 @@ export function useGeminiCommonConfig({ hasInitializedEditMode.current = false; }, [selectedPresetId]); - // 解析通用配置片段 + // 解析通用配置片段 - 使用共享解析器 + // 支持三种格式: ENV (KEY=VALUE), 扁平 JSON, 包裹 JSON {"env":{...}} const parseSnippetEnv = useCallback( ( snippetString: string, ): { env: Record; error?: string } => { - const trimmed = snippetString.trim(); - if (!trimmed) { - return { env: {} }; - } + const result = parseGeminiCommonConfigSnippet(snippetString, { + strictForbiddenKeys: true, + }); - let parsed: unknown; - try { - parsed = JSON.parse(trimmed); - } catch { - return { env: {}, error: t("geminiConfig.invalidJsonFormat") }; - } - - if (!isPlainObject(parsed)) { - return { env: {}, error: t("geminiConfig.invalidJsonFormat") }; - } - - const keys = Object.keys(parsed); - const forbiddenKeys = keys.filter((key) => - GEMINI_COMMON_ENV_FORBIDDEN_KEYS.includes(key as GeminiForbiddenEnvKey), - ); - if (forbiddenKeys.length > 0) { - return { - env: {}, - error: t("geminiConfig.commonConfigInvalidKeys", { - keys: forbiddenKeys.join(", "), - }), - }; - } - - const env: Record = {}; - for (const [key, value] of Object.entries(parsed)) { - if (typeof value !== "string") { + if (result.error) { + // Map error codes to i18n keys + if (result.error.startsWith(GEMINI_CONFIG_ERROR_CODES.FORBIDDEN_KEYS)) { + const keys = result.error.split(": ")[1] ?? result.error; + return { + env: {}, + error: t("geminiConfig.commonConfigInvalidKeys", { keys }), + }; + } + if ( + result.error.startsWith(GEMINI_CONFIG_ERROR_CODES.VALUE_NOT_STRING) + ) { return { env: {}, error: t("geminiConfig.commonConfigInvalidValues"), }; } - const normalized = value.trim(); - if (!normalized) continue; - env[key] = normalized; + // Generic format error (NOT_OBJECT, ENV_NOT_OBJECT, or parse failure) + return { env: {}, error: t("geminiConfig.invalidJsonFormat") }; } - return { env }; + return { env: result.env }; }, [t], ); diff --git a/src/utils/providerConfigUtils.ts b/src/utils/providerConfigUtils.ts index 37bb364d8..14103b5ca 100644 --- a/src/utils/providerConfigUtils.ts +++ b/src/utils/providerConfigUtils.ts @@ -2,6 +2,7 @@ import type { TemplateValueConfig } from "../config/claudeProviderPresets"; import { normalizeQuotes } from "@/utils/textNormalization"; +import { isPlainObject } from "@/utils/configMerge"; // Gemini 通用配置禁止的键（共享常量，供 hook 和同步逻辑复用） export const GEMINI_COMMON_ENV_FORBIDDEN_KEYS = [ @@ -11,10 +12,6 @@ export const GEMINI_COMMON_ENV_FORBIDDEN_KEYS = [ export type GeminiForbiddenEnvKey = (typeof GEMINI_COMMON_ENV_FORBIDDEN_KEYS)[number]; -const isPlainObject = (value: unknown): value is Record => { - return Object.prototype.toString.call(value) === "[object Object]"; -}; - const deepMerge = ( target: Record, source: Record, @@ -192,15 +189,20 @@ export const hasGeminiCommonConfigSnippet = ( const parsed = JSON.parse(snippetString); if (!isPlainObject(parsed)) return false; - const entries = Object.entries(parsed).filter(([key, value]) => { - if ( - GEMINI_COMMON_ENV_FORBIDDEN_KEYS.includes(key as GeminiForbiddenEnvKey) - ) { - return false; - } - if (typeof value !== "string") return false; - return value.trim().length > 0; - }); + const entries = Object.entries(parsed).filter( + (entry): entry is [string, string] => { + const [key, value] = entry; + if ( + GEMINI_COMMON_ENV_FORBIDDEN_KEYS.includes( + key as GeminiForbiddenEnvKey, + ) + ) { + return false; + } + if (typeof value !== "string") return false; + return value.trim().length > 0; + }, + ); if (entries.length === 0) return false; @@ -1040,3 +1042,162 @@ export const setCodexModelName = ( const lines = normalizedText.split("\n"); return `${replacementLine}\n${lines.join("\n")}`; }; + +// ============================================================================ +// Gemini Common Config Parsing Utilities +// ============================================================================ + +/** + * Error codes for Gemini common config parsing. + * These codes are used for consistent error handling and i18n mapping. + */ +export const GEMINI_CONFIG_ERROR_CODES = { + NOT_OBJECT: "GEMINI_CONFIG_NOT_OBJECT", + ENV_NOT_OBJECT: "GEMINI_CONFIG_ENV_NOT_OBJECT", + VALUE_NOT_STRING: "GEMINI_CONFIG_VALUE_NOT_STRING", + FORBIDDEN_KEYS: "GEMINI_CONFIG_FORBIDDEN_KEYS", +} as const; + +/** + * Result of parsing Gemini common config snippet + */ +export interface GeminiCommonConfigParseResult { + /** Parsed env key-value pairs (empty if invalid) */ + env: Record; + /** Error message if parsing/validation failed (starts with error code) */ + error?: string; + /** Warning message (non-fatal, config still usable) */ + warning?: string; +} + +/** + * Parse Gemini common config snippet with full validation. + * + * Supports three formats: + * - ENV format: KEY=VALUE lines (one per line, # for comments) + * - Flat JSON: {"KEY": "VALUE", ...} + * - Wrapped JSON: {"env": {"KEY": "VALUE", ...}} + * + * Validation rules: + * - Forbidden keys (GOOGLE_GEMINI_BASE_URL, GEMINI_API_KEY) are rejected + * - Non-string values are rejected + * - Empty string values are filtered out + * - Arrays and non-plain objects are rejected + * + * @param snippet - The common config snippet string + * @param options - Optional configuration + * @returns Parse result with env, error, and warning + */ +export function parseGeminiCommonConfigSnippet( + snippet: string, + options?: { + /** If true, reject forbidden keys with error; otherwise filter them with warning */ + strictForbiddenKeys?: boolean; + }, +): GeminiCommonConfigParseResult { + const trimmed = snippet.trim(); + if (!trimmed) { + return { env: {} }; + } + + const strictForbiddenKeys = options?.strictForbiddenKeys ?? true; + let rawEnv: Record = {}; + let isJson = false; + + // Try JSON first + try { + const parsed = JSON.parse(trimmed); + + // Must be a plain object (not array, null, etc.) + if (!isPlainObject(parsed)) { + return { + env: {}, + error: `${GEMINI_CONFIG_ERROR_CODES.NOT_OBJECT}: must be a JSON object, not array or primitive`, + }; + } + + isJson = true; + + // Check if wrapped format {"env": {...}} + if ("env" in parsed) { + const envField = parsed.env; + if (!isPlainObject(envField)) { + return { + env: {}, + error: `${GEMINI_CONFIG_ERROR_CODES.ENV_NOT_OBJECT}: 'env' field must be a plain object`, + }; + } + rawEnv = envField as Record; + } else { + // Flat format + rawEnv = parsed as Record; + } + } catch { + // Not JSON, parse as ENV format (KEY=VALUE lines) + isJson = false; + for (const line of trimmed.split("\n")) { + const lineTrimmed = line.trim(); + if (!lineTrimmed || lineTrimmed.startsWith("#")) continue; + const equalIndex = lineTrimmed.indexOf("="); + if (equalIndex > 0) { + const key = lineTrimmed.substring(0, equalIndex).trim(); + // Strip surrounding quotes (single or double) from value + // e.g., KEY="value" or KEY='value' -> value + const rawValue = lineTrimmed.substring(equalIndex + 1).trim(); + const value = rawValue.replace(/^["'](.*)["']$/, "$1"); + if (key) { + rawEnv[key] = value; + } + } + } + } + + // Validate and filter entries + const env: Record = {}; + const warnings: string[] = []; + const forbiddenKeysFound: string[] = []; + + for (const [key, value] of Object.entries(rawEnv)) { + // Check forbidden keys + if ( + GEMINI_COMMON_ENV_FORBIDDEN_KEYS.includes(key as GeminiForbiddenEnvKey) + ) { + forbiddenKeysFound.push(key); + continue; + } + + // Must be string + if (typeof value !== "string") { + if (isJson) { + return { + env: {}, + error: `${GEMINI_CONFIG_ERROR_CODES.VALUE_NOT_STRING}: value for '${key}' must be a string, got ${typeof value}`, + }; + } + // For ENV format, skip non-strings silently (shouldn't happen) + continue; + } + + // Filter empty strings + const trimmedValue = value.trim(); + if (!trimmedValue) { + continue; + } + + env[key] = trimmedValue; + } + + // Handle forbidden keys + if (forbiddenKeysFound.length > 0) { + const msg = `${GEMINI_CONFIG_ERROR_CODES.FORBIDDEN_KEYS}: ${forbiddenKeysFound.join(", ")}`; + if (strictForbiddenKeys) { + return { env: {}, error: msg }; + } + warnings.push(msg); + } + + return { + env, + warning: warnings.length > 0 ? warnings.join("; ") : undefined, + }; +}