diff --git a/backend/onyx/server/manage/image_generation/api.py b/backend/onyx/server/manage/image_generation/api.py index 9a25d1f6ed1..0cfa08af935 100644 --- a/backend/onyx/server/manage/image_generation/api.py +++ b/backend/onyx/server/manage/image_generation/api.py @@ -374,6 +374,22 @@ def update_config( old_provider.name = f"{old_provider.name}-old-{old_llm_provider_id}" db_session.flush() + # Determine actual API key to use: + # - Clone mode (source_llm_provider_id): API key comes from source provider + # - New credentials mode: Use provided api_key, or preserve existing if not changed + actual_api_key = config_update.api_key + if config_update.source_llm_provider_id is None and old_provider: + # Check if we should preserve existing API key: + # - api_key_changed=False AND (key is None/empty OR looks masked) + provided_key_is_masked = ( + config_update.api_key and "****" in config_update.api_key + ) + if not config_update.api_key_changed and ( + not config_update.api_key or provided_key_is_masked + ): + # Preserve existing API key when user didn't change it + actual_api_key = old_provider.api_key + # 3. Build and create new LLM provider provider_request = _build_llm_provider_request( db_session=db_session, @@ -381,7 +397,7 @@ def update_config( model_name=config_update.model_name, source_llm_provider_id=config_update.source_llm_provider_id, provider=config_update.provider, - api_key=config_update.api_key, + api_key=actual_api_key, api_base=config_update.api_base, api_version=config_update.api_version, deployment_name=config_update.deployment_name, diff --git a/backend/onyx/server/manage/image_generation/models.py b/backend/onyx/server/manage/image_generation/models.py index 7a93803e5d7..a45a732b3b8 100644 --- a/backend/onyx/server/manage/image_generation/models.py +++ b/backend/onyx/server/manage/image_generation/models.py @@ -6,6 +6,15 @@ from onyx.db.models import ImageGenerationConfig as ImageGenerationConfigModel +def _mask_api_key(api_key: str | None) -> str | None: + """Mask API key, showing first 4 and last 4 characters.""" + if not api_key: + return None + if len(api_key) <= 8: + return "****" + return api_key[:4] + "****" + api_key[-4:] + + class TestImageGenerationRequest(BaseModel): """Request model for testing image generation API key. @@ -79,6 +88,9 @@ class ImageGenerationConfigUpdate(BaseModel): api_version: str | None = None deployment_name: str | None = None + # If False and using new credentials mode, preserve existing API key from DB + api_key_changed: bool = False + class ImageGenerationConfigView(BaseModel): """Response model for image generation config with related data.""" @@ -117,10 +129,13 @@ class ImageGenerationCredentials(BaseModel): def from_model( cls, config: "ImageGenerationConfigModel" ) -> "ImageGenerationCredentials": - """Convert database model to credentials model.""" + """Convert database model to credentials model. + + Note: API key is masked for security - only first 4 and last 4 chars shown. + """ llm_provider = config.model_configuration.llm_provider return cls( - api_key=llm_provider.api_key, + api_key=_mask_api_key(llm_provider.api_key), api_base=llm_provider.api_base, api_version=llm_provider.api_version, deployment_name=llm_provider.deployment_name, diff --git a/backend/tests/integration/tests/image_generation/test_image_generation_config.py b/backend/tests/integration/tests/image_generation/test_image_generation_config.py index acd86cd0a70..3b7c5116328 100644 --- a/backend/tests/integration/tests/image_generation/test_image_generation_config.py +++ b/backend/tests/integration/tests/image_generation/test_image_generation_config.py @@ -172,8 +172,8 @@ def test_get_config_credentials( user_performing_action=admin_user, ) - # Credentials should contain the unmasked API key - assert credentials["api_key"] == test_api_key + # Credentials should contain the masked API key (first 4 + **** + last 4) + assert credentials["api_key"] == "sk-t****2345" assert "api_base" in credentials assert "api_version" in credentials assert "deployment_name" in credentials @@ -223,12 +223,12 @@ def test_update_config_direct_key_entry( assert updated_config.image_provider_id == config.image_provider_id assert updated_config.model_name == "dall-e-3" - # Verify credentials were updated + # Verify credentials were updated (masked: first 4 + **** + last 4) credentials = ImageGenerationConfigManager.get_credentials( image_provider_id=config.image_provider_id, user_performing_action=admin_user, ) - assert credentials["api_key"] == new_api_key + assert credentials["api_key"] == "sk-u****2345" def test_update_config_clone_mode( diff --git a/web/src/app/admin/configuration/image-generation/forms/ImageGenFormWrapper.tsx b/web/src/app/admin/configuration/image-generation/forms/ImageGenFormWrapper.tsx index ca4299f44a8..34da51f5a4a 100644 --- a/web/src/app/admin/configuration/image-generation/forms/ImageGenFormWrapper.tsx +++ b/web/src/app/admin/configuration/image-generation/forms/ImageGenFormWrapper.tsx @@ -51,12 +51,13 @@ export function ImageGenFormWrapper({ const isEditMode = !!existingConfig; // Compute API key options from existing providers matching this image provider + // API keys from LLM providers are already masked by backend (first 4 + **** + last 4) const apiKeyOptions = useMemo(() => { return existingProviders .filter((p) => p.provider === imageProvider.provider_name) .map((provider) => ({ value: `existing:${provider.id}:${provider.name}`, - label: `${provider.api_key || "****"}`, + label: provider.api_key || "****", })); }, [existingProviders, imageProvider.provider_name]); @@ -181,20 +182,26 @@ export function ImageGenFormWrapper({ }); } } else { - // New credentials mode - test the new API key first - const result = await testImageGenerationApiKey(payload.modelName, { - provider: payload.provider, - apiKey: payload.apiKey, - apiBase: payload.apiBase, - apiVersion: payload.apiVersion, - deploymentName: payload.deploymentName, - }); + // New credentials mode - check if API key was changed from masked value + // A masked key contains "****", so if present, user hasn't entered a new key + const apiKeyChanged = !apiKeyValue?.includes("****"); - if (!result.ok) { - setApiStatus("error"); - setErrorMessage(result.errorMessage || "API key validation failed"); - setIsSubmitting(false); - return; + // Test the API key first (only if changed or creating new config) + if (apiKeyChanged) { + const result = await testImageGenerationApiKey(payload.modelName, { + provider: payload.provider, + apiKey: payload.apiKey, + apiBase: payload.apiBase, + apiVersion: payload.apiVersion, + deploymentName: payload.deploymentName, + }); + + if (!result.ok) { + setApiStatus("error"); + setErrorMessage(result.errorMessage || "API key validation failed"); + setIsSubmitting(false); + return; + } } // Create or update config @@ -206,6 +213,7 @@ export function ImageGenFormWrapper({ apiBase: payload.apiBase, apiVersion: payload.apiVersion, deploymentName: payload.deploymentName, + apiKeyChanged, }); } else { await createImageGenerationConfig({ diff --git a/web/src/lib/configuration/imageConfigurationService.ts b/web/src/lib/configuration/imageConfigurationService.ts index 8b5023af8a4..febfb365966 100644 --- a/web/src/lib/configuration/imageConfigurationService.ts +++ b/web/src/lib/configuration/imageConfigurationService.ts @@ -174,6 +174,9 @@ export interface ImageGenerationConfigUpdateOptions { apiBase?: string; apiVersion?: string; deploymentName?: string; + + // If true, apiKey was changed by user; if false, backend preserves existing key + apiKeyChanged?: boolean; } /** @@ -197,6 +200,8 @@ export async function updateImageGenerationConfig( api_base: options.apiBase, api_version: options.apiVersion, deployment_name: options.deploymentName, + // If false, backend preserves existing API key + api_key_changed: options.apiKeyChanged ?? false, }), }); diff --git a/web/src/refresh-components/inputs/InputComboBox/components/OptionsList.tsx b/web/src/refresh-components/inputs/InputComboBox/components/OptionsList.tsx index 903b0482bad..4110fd73f8a 100644 --- a/web/src/refresh-components/inputs/InputComboBox/components/OptionsList.tsx +++ b/web/src/refresh-components/inputs/InputComboBox/components/OptionsList.tsx @@ -97,7 +97,8 @@ export const OptionsList: React.FC = ({ {/* Matched/Filtered Options */} {matchedOptions.map((option, idx) => { const globalIndex = idx + indexOffset; - const isExact = isExactMatch(option); + // Only highlight first exact match, not all matches + const isExact = idx === 0 && isExactMatch(option); return (