mirror of
https://github.com/langflow-ai/langflow.git
synced 2026-07-25 13:56:04 +08:00
fix: prevent overwriting user-selected global variables in provider c… (#12217)
* fix: nightly now properly gets 1.9.0 branch (#12215) before it was attempting to pull release-notes as letters are alphanumerically after numbers when we sort -V then grab tail now we only look at branch names that follow the pattern '^release-[0-9]+\.[0-9]+\.[0-9]+$' * docs: add search icon (#12216) add-back-svg * fix: prevent overwriting user-selected global variables in provider config Previously, the apply_provider_variable_config_to_build_config function would automatically overwrite field values with environment variable keys whenever an env var was present, even if the user had already selected a different global variable. This fix adds a check to only auto-set the environment variable if: - The field is currently empty, OR - The field is not already configured to load from the database This preserves user selections while still providing automatic configuration for new/empty fields. Added comprehensive unit tests to verify: - Auto-setting env vars for empty fields - Preserving user-selected global variables - Overwriting hardcoded values (expected behavior) - Skipping when env var is not set - Applying component metadata correctly * [autofix.ci] apply automated fixes * style: use dictionary comprehension instead of for-loop Fixed PERF403 Ruff style warning by replacing for-loop with dictionary comprehension in update_projects_components_with_latest_component_versions * chore: retrigger CI build * test: improve test coverage and clarity for provider config - Renamed test_apply_provider_config_overwrites_hardcoded_value to test_apply_provider_config_replaces_hardcoded_with_env_var for clarity - Added test_apply_provider_config_idempotent_when_already_set to document idempotent behavior when value already matches env var key - Removed sensitive value from debug log message to prevent potential exposure of API keys or credentials These changes improve test coverage by documenting the no-op scenario and enhance security by avoiding logging of potentially sensitive data. * chore: retrigger CI build --------- Co-authored-by: Adam-Aghili <149833988+Adam-Aghili@users.noreply.github.com> Co-authored-by: Mendon Kissling <59585235+mendonk@users.noreply.github.com> Co-authored-by: Steve Haertel <shaertel@ca.ibm.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Hare <ericrhare@gmail.com>
This commit is contained in:
Steve Haertel
@ -5249,7 +5249,7 @@
|
||||
"filename": "src/lfx/src/lfx/base/models/unified_models.py",
|
||||
"hashed_secret": "665b1e3851eefefa3fb878654292f16597d25155",
|
||||
"is_verified": false,
|
||||
"line_number": 1185,
|
||||
"line_number": 1239,
|
||||
"is_secret": false
|
||||
},
|
||||
{
|
||||
@ -5257,7 +5257,7 @@
|
||||
"filename": "src/lfx/src/lfx/base/models/unified_models.py",
|
||||
"hashed_secret": "3f2df46921dd8e2c36e2ce85238705ac0774c74a",
|
||||
"is_verified": false,
|
||||
"line_number": 1196,
|
||||
"line_number": 1250,
|
||||
"is_secret": false
|
||||
},
|
||||
{
|
||||
@ -5265,7 +5265,7 @@
|
||||
"filename": "src/lfx/src/lfx/base/models/unified_models.py",
|
||||
"hashed_secret": "d4c3d66fd0c38547a3c7a4c6bdc29c36911bc030",
|
||||
"is_verified": false,
|
||||
"line_number": 1210,
|
||||
"line_number": 1264,
|
||||
"is_secret": false
|
||||
}
|
||||
],
|
||||
|
||||
5
docs/static/img/langflow-icon-black-transparent.svg
vendored
Normal file
5
docs/static/img/langflow-icon-black-transparent.svg
vendored
Normal file
@ -0,0 +1,5 @@
|
||||
<svg width="470" height="470" viewBox="0 0 470 470" fill="none" xmlns="http://www.w3.org/2000/svg">
|
||||
<path d="M342.604 243.34H389.75C398.998 243.34 406.489 250.831 406.489 260.079V287.892C406.489 297.14 398.998 304.631 389.75 304.631H348.629C344.186 304.631 339.928 306.4 336.787 309.54L266.225 380.091C263.084 383.232 258.827 385 254.383 385H220.463C211.39 385 203.956 377.765 203.724 368.691L202.991 340.297C202.747 330.886 210.308 323.115 219.73 323.115H248.927C253.371 323.115 257.629 321.347 260.769 318.206L330.739 248.237C333.879 245.097 338.137 243.328 342.58 243.328L342.604 243.34Z" fill="black"/>
|
||||
<path d="M202.619 85H249.765C259.013 85 266.504 92.4913 266.504 101.739V129.552C266.504 138.8 259.013 146.291 249.765 146.291H208.644C204.201 146.291 199.943 148.06 196.802 151.2L126.24 221.763C123.099 224.904 118.842 226.672 114.398 226.672H80.4777C71.4044 226.672 63.9712 219.436 63.7386 210.363L63.0058 181.968C62.7615 172.558 70.3226 164.799 79.7449 164.799H108.942C113.386 164.799 117.643 163.031 120.784 159.89L190.753 89.9205C193.894 86.7798 198.152 85.0116 202.595 85.0116L202.619 85Z" fill="black"/>
|
||||
<path d="M342.603 120.829H389.75C398.997 120.829 406.489 128.32 406.489 137.568V165.381C406.489 174.629 398.997 182.12 389.75 182.12H348.629C344.185 182.12 339.928 183.888 336.787 187.029L266.225 257.591C263.084 260.732 258.826 262.5 254.383 262.5H213.169C208.853 262.5 204.701 264.164 201.583 267.153L122.366 343.067C119.248 346.056 115.096 347.72 110.78 347.72H81.9083C72.6605 347.72 65.1692 340.217 65.1692 330.981V302.4C65.1692 293.152 72.6605 285.661 81.9083 285.661H110.571C115.014 285.661 119.272 283.892 122.413 280.752L197.64 205.525C200.78 202.384 205.038 200.616 209.481 200.616H248.927C253.371 200.616 257.628 198.848 260.769 195.707L330.738 125.738C333.879 122.597 338.136 120.829 342.58 120.829H342.603Z" fill="black"/>
|
||||
</svg>
|
||||
|
After Width: | Height: | Size: 1.8 KiB |
@ -1,6 +1,11 @@
|
||||
from unittest.mock import MagicMock
|
||||
import os
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from lfx.base.models.unified_models import get_unified_models_detailed, update_model_options_in_build_config
|
||||
from lfx.base.models.unified_models import (
|
||||
apply_provider_variable_config_to_build_config,
|
||||
get_unified_models_detailed,
|
||||
update_model_options_in_build_config,
|
||||
)
|
||||
|
||||
|
||||
def _flatten_models(result):
|
||||
@ -160,3 +165,227 @@ def test_update_model_options_default_field_name():
|
||||
assert "model" in result
|
||||
assert len(result["model"]["options"]) == 1
|
||||
assert result["model"]["options"][0]["name"] == "gpt-4"
|
||||
|
||||
|
||||
# Tests for apply_provider_variable_config_to_build_config
|
||||
|
||||
|
||||
def test_apply_provider_config_auto_sets_env_var_for_empty_field():
|
||||
"""Test that env var is auto-set when field is empty."""
|
||||
mock_provider_vars = [
|
||||
{
|
||||
"variable_key": "OPENAI_API_KEY",
|
||||
"component_metadata": {
|
||||
"mapping_field": "openai_api_key",
|
||||
"required": True,
|
||||
"advanced": False,
|
||||
"info": "OpenAI API Key",
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
build_config = {
|
||||
"openai_api_key": {
|
||||
"value": "",
|
||||
"show": False,
|
||||
"required": False,
|
||||
"advanced": False,
|
||||
"load_from_db": False,
|
||||
}
|
||||
}
|
||||
|
||||
with (
|
||||
patch("lfx.base.models.unified_models.get_provider_all_variables", return_value=mock_provider_vars),
|
||||
patch("lfx.base.models.unified_models._get_all_provider_specific_field_names", return_value=["openai_api_key"]),
|
||||
patch.dict(os.environ, {"OPENAI_API_KEY": "test-key-123"}), # pragma: allowlist secret
|
||||
):
|
||||
result = apply_provider_variable_config_to_build_config(build_config, "OpenAI")
|
||||
|
||||
# Should auto-set the env var key for empty field
|
||||
assert result["openai_api_key"]["value"] == "OPENAI_API_KEY"
|
||||
assert result["openai_api_key"]["load_from_db"] is True
|
||||
assert result["openai_api_key"]["show"] is True
|
||||
assert result["openai_api_key"]["required"] is True
|
||||
|
||||
|
||||
def test_apply_provider_config_preserves_user_selected_value():
|
||||
"""Test that user-selected values are NOT overwritten."""
|
||||
mock_provider_vars = [
|
||||
{
|
||||
"variable_key": "OPENAI_API_KEY",
|
||||
"component_metadata": {
|
||||
"mapping_field": "openai_api_key",
|
||||
"required": True,
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
# User has already selected a different global variable
|
||||
build_config = {
|
||||
"openai_api_key": {
|
||||
"value": "MY_CUSTOM_API_KEY",
|
||||
"load_from_db": True,
|
||||
"show": False,
|
||||
"required": False,
|
||||
}
|
||||
}
|
||||
|
||||
with (
|
||||
patch("lfx.base.models.unified_models.get_provider_all_variables", return_value=mock_provider_vars),
|
||||
patch("lfx.base.models.unified_models._get_all_provider_specific_field_names", return_value=["openai_api_key"]),
|
||||
patch.dict(os.environ, {"OPENAI_API_KEY": "test-key-123"}), # pragma: allowlist secret
|
||||
):
|
||||
result = apply_provider_variable_config_to_build_config(build_config, "OpenAI")
|
||||
|
||||
# Should preserve user's selection
|
||||
assert result["openai_api_key"]["value"] == "MY_CUSTOM_API_KEY"
|
||||
assert result["openai_api_key"]["load_from_db"] is True
|
||||
|
||||
|
||||
def test_apply_provider_config_replaces_hardcoded_with_env_var():
|
||||
"""Test that hardcoded values are replaced with env var key for global variable usage.
|
||||
|
||||
Expected behavior: When load_from_db=False (hardcoded value), the function
|
||||
replaces it with the env var key and sets load_from_db=True to enable
|
||||
global variable functionality.
|
||||
"""
|
||||
mock_provider_vars = [
|
||||
{
|
||||
"variable_key": "OPENAI_API_KEY",
|
||||
"component_metadata": {
|
||||
"mapping_field": "openai_api_key",
|
||||
"required": True,
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
# User has hardcoded a value (not loading from db)
|
||||
build_config = {
|
||||
"openai_api_key": {
|
||||
"value": "sk-hardcoded-key",
|
||||
"load_from_db": False,
|
||||
"show": False,
|
||||
"required": False,
|
||||
}
|
||||
}
|
||||
|
||||
with (
|
||||
patch("lfx.base.models.unified_models.get_provider_all_variables", return_value=mock_provider_vars),
|
||||
patch("lfx.base.models.unified_models._get_all_provider_specific_field_names", return_value=["openai_api_key"]),
|
||||
patch.dict(os.environ, {"OPENAI_API_KEY": "test-key-123"}), # pragma: allowlist secret
|
||||
):
|
||||
result = apply_provider_variable_config_to_build_config(build_config, "OpenAI")
|
||||
|
||||
# Should overwrite with env var key since load_from_db was False
|
||||
assert result["openai_api_key"]["value"] == "OPENAI_API_KEY"
|
||||
assert result["openai_api_key"]["load_from_db"] is True
|
||||
|
||||
|
||||
def test_apply_provider_config_skips_when_env_var_not_set():
|
||||
"""Test that nothing is set when env var doesn't exist."""
|
||||
mock_provider_vars = [
|
||||
{
|
||||
"variable_key": "OPENAI_API_KEY",
|
||||
"component_metadata": {
|
||||
"mapping_field": "openai_api_key",
|
||||
"required": True,
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
build_config = {
|
||||
"openai_api_key": {
|
||||
"value": "",
|
||||
"show": False,
|
||||
"required": False,
|
||||
"load_from_db": False,
|
||||
}
|
||||
}
|
||||
|
||||
with (
|
||||
patch("lfx.base.models.unified_models.get_provider_all_variables", return_value=mock_provider_vars),
|
||||
patch("lfx.base.models.unified_models._get_all_provider_specific_field_names", return_value=["openai_api_key"]),
|
||||
patch.dict(os.environ, {}, clear=True),
|
||||
):
|
||||
result = apply_provider_variable_config_to_build_config(build_config, "OpenAI")
|
||||
|
||||
# Should not set anything when env var doesn't exist
|
||||
assert result["openai_api_key"]["value"] == ""
|
||||
assert result["openai_api_key"]["load_from_db"] is False
|
||||
|
||||
|
||||
def test_apply_provider_config_applies_component_metadata():
|
||||
"""Test that component metadata (required, advanced, info) is applied."""
|
||||
mock_provider_vars = [
|
||||
{
|
||||
"variable_key": "OPENAI_API_KEY",
|
||||
"component_metadata": {
|
||||
"mapping_field": "openai_api_key",
|
||||
"required": True,
|
||||
"advanced": False,
|
||||
"info": "OpenAI API Key",
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
build_config = {
|
||||
"openai_api_key": {
|
||||
"value": "",
|
||||
"show": False,
|
||||
"required": False,
|
||||
"advanced": False,
|
||||
"load_from_db": False,
|
||||
}
|
||||
}
|
||||
|
||||
with (
|
||||
patch("lfx.base.models.unified_models.get_provider_all_variables", return_value=mock_provider_vars),
|
||||
patch("lfx.base.models.unified_models._get_all_provider_specific_field_names", return_value=["openai_api_key"]),
|
||||
patch.dict(os.environ, {"OPENAI_API_KEY": "test-key"}), # pragma: allowlist secret
|
||||
):
|
||||
result = apply_provider_variable_config_to_build_config(build_config, "OpenAI")
|
||||
|
||||
# Check metadata was applied
|
||||
assert result["openai_api_key"]["required"] is True
|
||||
assert result["openai_api_key"]["advanced"] is False
|
||||
assert result["openai_api_key"]["info"] == "OpenAI API Key"
|
||||
assert result["openai_api_key"]["show"] is True
|
||||
|
||||
|
||||
def test_apply_provider_config_idempotent_when_already_set():
|
||||
"""Test idempotent behavior when value already matches env var key.
|
||||
|
||||
When a field already has the env var key as its value and load_from_db=True,
|
||||
the function should be a no-op (no changes made). This documents the
|
||||
idempotent behavior of the function.
|
||||
"""
|
||||
mock_provider_vars = [
|
||||
{
|
||||
"variable_key": "OPENAI_API_KEY",
|
||||
"component_metadata": {
|
||||
"mapping_field": "openai_api_key",
|
||||
"required": True,
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
# Field already configured with env var key
|
||||
build_config = {
|
||||
"openai_api_key": {
|
||||
"value": "OPENAI_API_KEY",
|
||||
"load_from_db": True,
|
||||
"show": False,
|
||||
"required": False,
|
||||
}
|
||||
}
|
||||
|
||||
with (
|
||||
patch("lfx.base.models.unified_models.get_provider_all_variables", return_value=mock_provider_vars),
|
||||
patch("lfx.base.models.unified_models._get_all_provider_specific_field_names", return_value=["openai_api_key"]),
|
||||
patch.dict(os.environ, {"OPENAI_API_KEY": "test-key-123"}), # pragma: allowlist secret
|
||||
):
|
||||
result = apply_provider_variable_config_to_build_config(build_config, "OpenAI")
|
||||
|
||||
# Should remain unchanged (idempotent)
|
||||
assert result["openai_api_key"]["value"] == "OPENAI_API_KEY"
|
||||
assert result["openai_api_key"]["load_from_db"] is True
|
||||
|
||||
@ -280,14 +280,26 @@ def apply_provider_variable_config_to_build_config(
|
||||
env_var_key = var_info.get("variable_key")
|
||||
if env_var_key:
|
||||
env_value = os.environ.get(env_var_key)
|
||||
# Only set the value if the field is currently empty or not already set to load from db
|
||||
# This prevents overwriting user-selected global variables
|
||||
current_value = field_config.get("value")
|
||||
current_load_from_db = field_config.get("load_from_db", False)
|
||||
|
||||
if env_value and str(env_value).strip():
|
||||
field_config["value"] = env_var_key
|
||||
field_config["load_from_db"] = True
|
||||
logger.debug(
|
||||
"Set field %s to env var name %s (value resolved at runtime)",
|
||||
field_name,
|
||||
env_var_key,
|
||||
)
|
||||
# Only auto-set if field is empty or not already loading from db
|
||||
if not current_value or (not current_load_from_db):
|
||||
field_config["value"] = env_var_key
|
||||
field_config["load_from_db"] = True
|
||||
logger.debug(
|
||||
"Set field %s to env var name %s (value resolved at runtime)",
|
||||
field_name,
|
||||
env_var_key,
|
||||
)
|
||||
else:
|
||||
logger.debug(
|
||||
"Skipping auto-set for field %s - user has already selected a value (load_from_db=True)",
|
||||
field_name,
|
||||
)
|
||||
|
||||
return build_config
|
||||
|
||||
|
||||
Reference in New Issue
Block a user