diff --git a/api/routes/workflow.py b/api/routes/workflow.py index acc0391..a155949 100644 --- a/api/routes/workflow.py +++ b/api/routes/workflow.py @@ -21,10 +21,15 @@ from api.sdk_expose import sdk_expose from api.services.auth.depends import get_user from api.services.configuration.check_validity import UserConfigurationValidator from api.services.configuration.masking import ( + mask_workflow_configurations, mask_workflow_definition, merge_workflow_api_keys, ) -from api.services.configuration.resolve import resolve_effective_config +from api.services.configuration.merge import merge_workflow_configuration_secrets +from api.services.configuration.resolve import ( + enrich_overrides_with_api_keys, + resolve_effective_config, +) from api.services.mps_service_key_client import mps_service_key_client from api.services.posthog_client import capture_event from api.services.reports import generate_workflow_report_csv @@ -410,7 +415,9 @@ async def create_workflow( "current_definition_id": workflow.current_definition_id, "template_context_variables": workflow.template_context_variables, "call_disposition_codes": workflow.call_disposition_codes, - "workflow_configurations": workflow.workflow_configurations, + "workflow_configurations": mask_workflow_configurations( + workflow.workflow_configurations + ), } @@ -508,7 +515,9 @@ async def create_workflow_from_template( "current_definition_id": workflow.current_definition_id, "template_context_variables": workflow.template_context_variables, "call_disposition_codes": workflow.call_disposition_codes, - "workflow_configurations": workflow.workflow_configurations, + "workflow_configurations": mask_workflow_configurations( + workflow.workflow_configurations + ), } except HTTPException: @@ -653,7 +662,7 @@ async def get_workflow( "current_definition_id": workflow.current_definition_id, "template_context_variables": template_vars, "call_disposition_codes": workflow.call_disposition_codes, - "workflow_configurations": workflow_configs, + "workflow_configurations": mask_workflow_configurations(workflow_configs), "version_number": active_def.version_number if active_def else None, "version_status": active_def.status if active_def else None, "workflow_uuid": workflow.workflow_uuid, @@ -691,7 +700,9 @@ async def get_workflow_versions( created_at=v.created_at, published_at=v.published_at, workflow_json=mask_workflow_definition(v.workflow_json), - workflow_configurations=v.workflow_configurations, + workflow_configurations=mask_workflow_configurations( + v.workflow_configurations + ), template_context_variables=v.template_context_variables, ) for v in versions @@ -776,7 +787,9 @@ async def create_workflow_draft( created_at=draft.created_at, published_at=draft.published_at, workflow_json=mask_workflow_definition(draft.workflow_json), - workflow_configurations=draft.workflow_configurations, + workflow_configurations=mask_workflow_configurations( + draft.workflow_configurations + ), template_context_variables=draft.template_context_variables, ) @@ -834,7 +847,9 @@ async def update_workflow_status( "current_definition_id": workflow.current_definition_id, "template_context_variables": workflow.template_context_variables, "call_disposition_codes": workflow.call_disposition_codes, - "workflow_configurations": workflow.workflow_configurations, + "workflow_configurations": mask_workflow_configurations( + workflow.workflow_configurations + ), "total_runs": run_count, } except ValueError as e: @@ -941,15 +956,34 @@ async def update_workflow( # Validate model_overrides: resolve onto global config, then # run the same validator used by the user-configurations endpoint. - if request.workflow_configurations and request.workflow_configurations.get( - "model_overrides" - ): + # Also stamp the current global API key into the override so the override + # remains functional if the global config later switches to a different provider. + workflow_configurations = request.workflow_configurations + if workflow_configurations and workflow_configurations.get("model_overrides"): + existing_workflow = await db_client.get_workflow( + workflow_id, organization_id=user.selected_organization_id + ) + if existing_workflow is None: + raise HTTPException( + status_code=404, detail=f"Workflow with id {workflow_id} not found" + ) + existing_draft = await db_client.get_draft_version(workflow_id) + existing_configs = ( + existing_draft.workflow_configurations + if existing_draft + else existing_workflow.released_definition.workflow_configurations + ) + workflow_configurations = merge_workflow_configuration_secrets( + workflow_configurations, + existing_configs, + ) user_config = await db_client.get_user_configurations(user.id) try: - effective = resolve_effective_config( + enriched_overrides = enrich_overrides_with_api_keys( + workflow_configurations["model_overrides"], user_config, - request.workflow_configurations["model_overrides"], ) + effective = resolve_effective_config(user_config, enriched_overrides) await UserConfigurationValidator().validate( effective, organization_id=user.selected_organization_id, @@ -957,6 +991,10 @@ async def update_workflow( ) except ValueError as e: raise HTTPException(status_code=422, detail=str(e)) + workflow_configurations = { + **workflow_configurations, + "model_overrides": enriched_overrides, + } # Reject upfront if any new trigger path collides with another # workflow's trigger — keeps the workflow record from @@ -979,7 +1017,7 @@ async def update_workflow( name=request.name, workflow_definition=workflow_definition, template_context_variables=request.template_context_variables, - workflow_configurations=request.workflow_configurations, + workflow_configurations=workflow_configurations, organization_id=user.selected_organization_id, ) @@ -1015,7 +1053,7 @@ async def update_workflow( "current_definition_id": workflow.current_definition_id, "template_context_variables": template_vars, "call_disposition_codes": workflow.call_disposition_codes, - "workflow_configurations": workflow_configs, + "workflow_configurations": mask_workflow_configurations(workflow_configs), "version_number": active_def.version_number if active_def else None, "version_status": active_def.status if active_def else None, } @@ -1062,7 +1100,9 @@ async def duplicate_workflow_endpoint( "current_definition_id": workflow.current_definition_id, "template_context_variables": workflow.template_context_variables, "call_disposition_codes": workflow.call_disposition_codes, - "workflow_configurations": workflow.workflow_configurations, + "workflow_configurations": mask_workflow_configurations( + workflow.workflow_configurations + ), } except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) @@ -1345,7 +1385,9 @@ async def duplicate_workflow_template( "current_definition_id": workflow.current_definition_id, "template_context_variables": workflow.template_context_variables, "call_disposition_codes": workflow.call_disposition_codes, - "workflow_configurations": workflow.workflow_configurations, + "workflow_configurations": mask_workflow_configurations( + workflow.workflow_configurations + ), } diff --git a/api/services/configuration/masking.py b/api/services/configuration/masking.py index f1ed1f6..877cad9 100644 --- a/api/services/configuration/masking.py +++ b/api/services/configuration/masking.py @@ -9,6 +9,7 @@ The rules are simple: in storage. """ +import copy from typing import Any, Dict, Optional from api.schemas.user_configuration import UserConfiguration @@ -19,6 +20,7 @@ VISIBLE_CHARS = 4 # number of trailing characters to reveal MASK_CHAR = "*" MASK_MARKER = "***" # substring that indicates a masked key SERVICE_SECRET_FIELDS = ("api_key", "credentials", "aws_access_key", "aws_secret_key") +MODEL_OVERRIDE_FIELDS = ("llm", "tts", "stt", "realtime") def contains_masked_key(value: str | list[str] | None) -> bool: @@ -67,6 +69,12 @@ def mask_key(real_key: str, visible: int = VISIBLE_CHARS) -> str: return f"{masked_part}{real_key[-visible:]}" +def _mask_secret_value(value: str | list[str]) -> str | list[str]: + if isinstance(value, list): + return [mask_key(k) for k in value] + return mask_key(value) + + def is_mask_of(masked: str, real_key: str) -> bool: """Return *True* if *masked* equals the mask of *real_key* under the current rules.""" return mask_key(real_key) == masked @@ -117,10 +125,7 @@ def _mask_service(service_cfg: Optional[ServiceConfig]) -> Optional[Dict[str, An if secret_field not in data or not data[secret_field]: continue raw = data[secret_field] - if isinstance(raw, list): - data[secret_field] = [mask_key(k) for k in raw] - else: - data[secret_field] = mask_key(raw) + data[secret_field] = _mask_secret_value(raw) return data @@ -139,6 +144,28 @@ def mask_user_config(config: UserConfiguration) -> Dict[str, Any]: } +def mask_workflow_configurations(config: Optional[Dict]) -> Optional[Dict]: + """Mask secret fields inside workflow-level model overrides for API responses.""" + if not config: + return config + + masked = copy.deepcopy(config) + model_overrides = masked.get("model_overrides") + if not isinstance(model_overrides, dict): + return masked + + for section in MODEL_OVERRIDE_FIELDS: + override = model_overrides.get(section) + if not isinstance(override, dict): + continue + for secret_field in SERVICE_SECRET_FIELDS: + raw = override.get(secret_field) + if raw: + override[secret_field] = _mask_secret_value(raw) + + return masked + + # --------------------------------------------------------------------------- # Workflow definition helpers – mask / merge node API keys # --------------------------------------------------------------------------- diff --git a/api/services/configuration/merge.py b/api/services/configuration/merge.py index 937060d..f421648 100644 --- a/api/services/configuration/merge.py +++ b/api/services/configuration/merge.py @@ -4,17 +4,67 @@ from __future__ import annotations stored, while honouring masked API keys. """ +import copy from typing import Dict from api.schemas.user_configuration import UserConfiguration from api.services.configuration.masking import ( + MODEL_OVERRIDE_FIELDS, SERVICE_SECRET_FIELDS, + contains_masked_key, resolve_masked_api_keys, ) SERVICE_FIELDS = ("llm", "tts", "stt", "embeddings", "realtime") +def _same_provider(incoming_cfg: dict, existing_cfg: dict) -> bool: + return not ( + existing_cfg.get("provider") is not None + and incoming_cfg.get("provider") is not None + and incoming_cfg.get("provider") != existing_cfg.get("provider") + ) + + +def _merge_service_secret_fields( + incoming_cfg: dict, + existing_cfg: dict, + *, + preserve_missing: bool, + masked_value_preserves_full_secret: bool = False, +) -> dict: + """Restore existing real secrets when incoming values are masked. + + If ``preserve_missing`` is true, missing incoming secret fields are also + copied from the existing config. User config updates need that behavior; + workflow model overrides leave missing secrets blank so later enrichment can + copy from the current global config. + """ + if not _same_provider(incoming_cfg, existing_cfg): + return incoming_cfg + + for secret_field in SERVICE_SECRET_FIELDS: + if secret_field not in existing_cfg: + continue + + incoming_secret = incoming_cfg.get(secret_field) + existing_secret = existing_cfg[secret_field] + if incoming_secret is not None: + if contains_masked_key(incoming_secret): + incoming_cfg[secret_field] = ( + existing_secret + if masked_value_preserves_full_secret + else resolve_masked_api_keys( + incoming_secret, + existing_secret, + ) + ) + elif preserve_missing: + incoming_cfg[secret_field] = existing_secret + + return incoming_cfg + + def merge_user_configurations( existing: UserConfiguration, incoming_partial: Dict[str, dict] ) -> UserConfiguration: @@ -41,23 +91,12 @@ def merge_user_configurations( return # nothing to do old_cfg = merged.get(service_name, {}) - - provider_changed = ( - old_cfg.get("provider") is not None - and incoming_cfg.get("provider") is not None - and incoming_cfg.get("provider") != old_cfg.get("provider") - ) - - if not provider_changed: - for secret_field in SERVICE_SECRET_FIELDS: - incoming_secret = incoming_cfg.get(secret_field) - if incoming_secret is not None: - if old_cfg and secret_field in old_cfg: - incoming_cfg[secret_field] = resolve_masked_api_keys( - incoming_secret, old_cfg[secret_field] - ) - elif secret_field in old_cfg: - incoming_cfg[secret_field] = old_cfg[secret_field] + if old_cfg: + incoming_cfg = _merge_service_secret_fields( + incoming_cfg, + old_cfg, + preserve_missing=True, + ) merged[service_name] = incoming_cfg @@ -75,3 +114,46 @@ def merge_user_configurations( merged["timezone"] = incoming_partial["timezone"] return UserConfiguration.model_validate(merged) + + +def merge_workflow_configuration_secrets( + incoming_config: dict | None, + existing_config: dict | None, +) -> dict | None: + """Restore persisted workflow override secrets when the client sends masks. + + Workflow model overrides intentionally persist real keys so a workflow keeps + running after the global provider changes. API responses mask those keys, so + save requests must merge masked placeholders back to the stored real values. + + Unlike user config updates, a missing workflow override secret is not copied + from the existing workflow config. Missing means "copy from current global" + during the later enrichment step. + """ + if not incoming_config or not existing_config: + return incoming_config + + merged = copy.deepcopy(incoming_config) + incoming_overrides = merged.get("model_overrides") + existing_overrides = existing_config.get("model_overrides") + if not isinstance(incoming_overrides, dict) or not isinstance( + existing_overrides, dict + ): + return merged + + for section in MODEL_OVERRIDE_FIELDS: + incoming_section = incoming_overrides.get(section) + existing_section = existing_overrides.get(section) + if not isinstance(incoming_section, dict) or not isinstance( + existing_section, dict + ): + continue + + incoming_overrides[section] = _merge_service_secret_fields( + incoming_section, + existing_section, + preserve_missing=False, + masked_value_preserves_full_secret=True, + ) + + return merged diff --git a/api/services/configuration/resolve.py b/api/services/configuration/resolve.py index a55e61a..742e46b 100644 --- a/api/services/configuration/resolve.py +++ b/api/services/configuration/resolve.py @@ -2,6 +2,8 @@ from __future__ import annotations +import copy + from api.schemas.user_configuration import UserConfiguration from api.services.configuration.registry import ( REGISTRY, @@ -29,6 +31,48 @@ def _build_section_from_override(service_type: ServiceType, override: dict): return config_cls(**override) +_SECRET_FIELDS = ("api_key", "credentials", "aws_access_key", "aws_secret_key") + + +def enrich_overrides_with_api_keys( + model_overrides: dict, + user_config: UserConfiguration, +) -> dict: + """Copy API keys from the global config into model_overrides where missing. + + When a workflow override selects the same provider as the current global + config but omits the API key, the override becomes broken if the global + config later switches to a different provider. This function stamps the + global provider's API key (and other secret fields) into the override at + save time so the override is self-contained. + """ + result = copy.deepcopy(model_overrides) + for section_key in _SECTION_MAP: + if section_key not in result: + continue + override = result[section_key] + override_provider = override.get("provider") + if not override_provider: + continue + global_section = getattr(user_config, section_key, None) + if global_section is None: + continue + if getattr(global_section, "provider", None) != override_provider: + continue + for field in _SECRET_FIELDS: + if override.get(field): + continue + if field == "api_key" and hasattr(global_section, "get_all_api_keys"): + all_keys = global_section.get_all_api_keys() + if all_keys: + override[field] = all_keys[0] if len(all_keys) == 1 else all_keys + else: + global_value = getattr(global_section, field, None) + if global_value is not None: + override[field] = global_value + return result + + def resolve_effective_config( user_config: UserConfiguration, model_overrides: dict | None, diff --git a/api/tests/test_resolve_effective_config.py b/api/tests/test_resolve_effective_config.py index c747387..c539c7c 100644 --- a/api/tests/test_resolve_effective_config.py +++ b/api/tests/test_resolve_effective_config.py @@ -10,6 +10,11 @@ Module under test: api.services.configuration.resolve import pytest from api.schemas.user_configuration import UserConfiguration +from api.services.configuration.masking import ( + contains_masked_key, + mask_workflow_configurations, +) +from api.services.configuration.merge import merge_workflow_configuration_secrets from api.services.configuration.registry import ( DeepgramSTTConfiguration, ElevenlabsTTSConfiguration, @@ -19,7 +24,10 @@ from api.services.configuration.registry import ( OpenAILLMService, UltravoxRealtimeLLMConfiguration, ) -from api.services.configuration.resolve import resolve_effective_config +from api.services.configuration.resolve import ( + enrich_overrides_with_api_keys, + resolve_effective_config, +) # --------------------------------------------------------------------------- # Fixtures @@ -403,3 +411,209 @@ class TestUnknownKeys: {"embeddings": {"provider": "openai", "model": "text-embedding-3-small"}}, ) assert result.embeddings is None # was None in global, stays None + + +# --------------------------------------------------------------------------- +# enrich_overrides_with_api_keys +# --------------------------------------------------------------------------- + + +class TestEnrichOverridesWithApiKeys: + def test_injects_api_key_when_same_provider(self, global_config): + """Override matching the global provider gets the global API key stamped in.""" + overrides = { + "tts": { + "provider": "elevenlabs", + "voice": "Bella", + "model": "eleven_flash_v2_5", + } + } + enriched = enrich_overrides_with_api_keys(overrides, global_config) + assert enriched["tts"]["api_key"] == "el-global-tts" + + def test_injects_all_api_keys_when_global_has_multiple(self, global_config): + """Override matching a multi-key global provider gets every global key.""" + global_config.llm.api_key = ["sk-global-1", "sk-global-2"] + overrides = {"llm": {"provider": "openai", "model": "gpt-4.1-mini"}} + + enriched = enrich_overrides_with_api_keys(overrides, global_config) + + assert enriched["llm"]["api_key"] == ["sk-global-1", "sk-global-2"] + + def test_does_not_overwrite_existing_api_key(self, global_config): + """Override that already has an api_key keeps its own key.""" + overrides = { + "tts": { + "provider": "elevenlabs", + "api_key": "my-own-key", + "voice": "Bella", + "model": "eleven_flash_v2_5", + } + } + enriched = enrich_overrides_with_api_keys(overrides, global_config) + assert enriched["tts"]["api_key"] == "my-own-key" + + def test_skips_when_provider_differs(self, global_config): + """Override for a different provider is not enriched with the global key.""" + overrides = { + "tts": {"provider": "cartesia", "voice": "some-voice", "model": "sonic-3"} + } + enriched = enrich_overrides_with_api_keys(overrides, global_config) + assert "api_key" not in enriched["tts"] + + def test_does_not_mutate_original(self, global_config): + """The input overrides dict must not be modified.""" + overrides = { + "tts": { + "provider": "elevenlabs", + "voice": "Bella", + "model": "eleven_flash_v2_5", + } + } + original_copy = { + "tts": { + "provider": "elevenlabs", + "voice": "Bella", + "model": "eleven_flash_v2_5", + } + } + enrich_overrides_with_api_keys(overrides, global_config) + assert overrides == original_copy + + def test_regression_override_survives_global_provider_change(self, global_config): + """Core bug: override for provider A still works after global switches to B. + + Steps: + 1. Global TTS = ElevenLabs, Override TTS = ElevenLabs (different voice) + 2. enrich_overrides_with_api_keys stamps ElevenLabs API key into override + 3. Global TTS changes to Deepgram (simulate by building a new config) + 4. resolve_effective_config must still return a valid ElevenLabs config + """ + override_at_save_time = { + "tts": { + "provider": "elevenlabs", + "voice": "Bella", + "model": "eleven_flash_v2_5", + } + } + enriched = enrich_overrides_with_api_keys(override_at_save_time, global_config) + assert enriched["tts"]["api_key"] == "el-global-tts" + + # Simulate global config switching to Deepgram + from api.services.configuration.registry import DeepgramTTSConfiguration + + new_global = global_config.model_copy( + update={ + "tts": DeepgramTTSConfiguration( + provider="deepgram", api_key="dg-new", voice="aura-2-helena-en" + ) + } + ) + + # The enriched override should resolve correctly against the new global + result = resolve_effective_config(new_global, enriched) + assert result.tts.provider == "elevenlabs" + assert result.tts.voice == "Bella" + assert result.tts.api_key == "el-global-tts" + + +class TestWorkflowConfigurationSecrets: + def test_masks_model_override_secrets(self): + configs = { + "model_overrides": { + "llm": { + "provider": "openai", + "api_key": "sk-real-llm-key", + "model": "gpt-4.1-mini", + }, + "tts": { + "provider": "elevenlabs", + "api_key": "el-real-tts-key", + "voice": "Bella", + }, + }, + "ambient_noise_configuration": {"enabled": True}, + } + + masked = mask_workflow_configurations(configs) + + assert masked["model_overrides"]["llm"]["api_key"] != "sk-real-llm-key" + assert contains_masked_key(masked["model_overrides"]["llm"]["api_key"]) + assert masked["model_overrides"]["llm"]["api_key"].endswith("-key") + assert masked["model_overrides"]["tts"]["api_key"] != "el-real-tts-key" + assert masked["ambient_noise_configuration"] == {"enabled": True} + assert configs["model_overrides"]["llm"]["api_key"] == "sk-real-llm-key" + + def test_restores_masked_model_override_secrets_from_existing_config(self): + existing = { + "model_overrides": { + "tts": { + "provider": "elevenlabs", + "api_key": "el-real-tts-key", + "voice": "Rachel", + } + } + } + incoming = mask_workflow_configurations(existing) + incoming["model_overrides"]["tts"]["voice"] = "Bella" + + merged = merge_workflow_configuration_secrets(incoming, existing) + + assert merged["model_overrides"]["tts"]["api_key"] == "el-real-tts-key" + assert merged["model_overrides"]["tts"]["voice"] == "Bella" + assert incoming["model_overrides"]["tts"]["api_key"] != "el-real-tts-key" + + def test_single_masked_key_preserves_existing_multi_key_override(self): + existing = { + "model_overrides": { + "llm": { + "provider": "openai", + "api_key": ["sk-workflow-1", "sk-workflow-2"], + "model": "gpt-4.1-mini", + } + } + } + incoming = mask_workflow_configurations(existing) + incoming["model_overrides"]["llm"]["api_key"] = incoming["model_overrides"][ + "llm" + ]["api_key"][0] + + merged = merge_workflow_configuration_secrets(incoming, existing) + + assert merged["model_overrides"]["llm"]["api_key"] == [ + "sk-workflow-1", + "sk-workflow-2", + ] + + def test_missing_secret_copies_current_global_key_instead_of_existing_workflow_key( + self, global_config + ): + global_config.stt.api_key = ["dg-global-1", "dg-global-2"] + existing = { + "model_overrides": { + "stt": { + "provider": "deepgram", + "api_key": "dg-workflow-key", + "model": "nova-3-general", + "language": "multi", + } + } + } + incoming = { + "model_overrides": { + "stt": { + "provider": "deepgram", + "model": "nova-3-general", + "language": "en", + } + } + } + + merged = merge_workflow_configuration_secrets(incoming, existing) + enriched = enrich_overrides_with_api_keys( + merged["model_overrides"], + global_config, + ) + + assert enriched["stt"]["api_key"] == ["dg-global-1", "dg-global-2"] + assert enriched["stt"]["language"] == "en" diff --git a/ui/src/app/workflow/[workflowId]/hooks/useWorkflowState.ts b/ui/src/app/workflow/[workflowId]/hooks/useWorkflowState.ts index f190d6d..0b795d0 100644 --- a/ui/src/app/workflow/[workflowId]/hooks/useWorkflowState.ts +++ b/ui/src/app/workflow/[workflowId]/hooks/useWorkflowState.ts @@ -495,7 +495,10 @@ export const useWorkflowState = ({ throw new Error(msg); } - setWorkflowConfigurations(configurationsWithDictionary); + const savedConfigurations = response.data?.workflow_configurations + ? (response.data.workflow_configurations as WorkflowConfigurations) + : configurationsWithDictionary; + setWorkflowConfigurations(savedConfigurations); // Set name directly in the store to avoid setWorkflowName which marks isDirty: true useWorkflowStore.setState({ workflowName: newWorkflowName }); logger.info('Workflow configurations saved successfully');