Skip to content

Commit db82c21

Browse files
Mateuszfactory-droid[bot]
andcommitted
fix: Suppress false positive security warnings for API keys loaded from env
Refactored _discover_api_keys_from_config_backends in src/core/common/logging_utils.py to check if the discovered API key matches the corresponding environment variable. If it matches, the security warning is suppressed, as this is the intended and secure configuration pattern. Also added a new test case est_api_key_discovery_suppresses_env_vars to ests/unit/core/test_logging_utils.py to verify this behavior and prevent regression. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 parent 73b2b95 commit db82c21

File tree

2 files changed

+71
-3
lines changed

2 files changed

+71
-3
lines changed

src/core/common/logging_utils.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,6 @@ def __init__(
125125
"credentials",
126126
}
127127

128-
# Keep track of security warnings already logged to avoid spamming logs
129-
_logged_security_warnings: set[str] = set()
130128

131129
# Regular expressions for redacting sensitive information
132130
# Match common API key prefixes with more specific patterns to reduce false positives:
@@ -509,7 +507,7 @@ def configure_logging_with_environment_tagging(
509507
handlers.append(file_handler)
510508

511509
# Configure structlog
512-
structlog_processors = [
510+
structlog_processors: list[Any] = [
513511
structlog.stdlib.PositionalArgumentsFormatter(),
514512
structlog.processors.StackInfoRenderer(),
515513
structlog.processors.format_exc_info,
@@ -634,6 +632,11 @@ def _discover_api_keys_from_config_backends(
634632
if k:
635633
found.add(str(k))
636634
# SECURITY WARNING: Log when API keys are found in config
635+
# Check if the key matches the environment variable (false positive check)
636+
env_var = f"{b.upper()}_API_KEY"
637+
if str(k) == os.getenv(env_var):
638+
continue
639+
637640
warn_key = f"backends.{b}.api_key"
638641
if warn_key not in _logged_security_warnings:
639642
logger = get_logger(__name__)
@@ -645,6 +648,11 @@ def _discover_api_keys_from_config_backends(
645648
else:
646649
found.add(str(ak))
647650
# SECURITY WARNING: Log when API keys are found in config
651+
# Check if the key matches the environment variable (false positive check)
652+
env_var = f"{b.upper()}_API_KEY"
653+
if str(ak) == os.getenv(env_var):
654+
continue
655+
648656
warn_key = f"backends.{b}.api_key"
649657
if warn_key not in _logged_security_warnings:
650658
logger = get_logger(__name__)

tests/unit/core/test_logging_utils.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44

55
import logging
6+
import sys
67
from unittest.mock import MagicMock, patch
78

89
import pytest
@@ -256,3 +257,62 @@ def test_log_context(self) -> None:
256257

257258
# Verify bind was called with the correct context
258259
mock_logger.bind.assert_called_once_with(request_id="123", user_id="456")
260+
261+
def test_api_key_discovery_suppresses_env_vars(self) -> None:
262+
"""Test that API key discovery suppresses warnings for keys found in env vars."""
263+
from src.core.common.logging_utils import (
264+
_logged_security_warnings,
265+
discover_api_keys_from_config_and_env,
266+
)
267+
from src.core.config.app_config import AppConfig
268+
269+
# Clear previous warnings
270+
_logged_security_warnings.clear()
271+
272+
# Setup mocks
273+
config = MagicMock(spec=AppConfig)
274+
backends = MagicMock()
275+
config.backends = backends
276+
277+
# Mock Minimax backend config
278+
minimax_config = MagicMock()
279+
minimax_config.api_key = "test-minimax-key"
280+
backends.minimax = minimax_config
281+
282+
# Mock backend registry
283+
with patch.dict(
284+
"sys.modules", {"src.core.services.backend_registry": MagicMock()}
285+
):
286+
sys.modules[
287+
"src.core.services.backend_registry"
288+
].backend_registry.get_registered_backends.return_value = ["minimax"]
289+
290+
# Case 1: Key matches env var -> No warning
291+
with patch.dict("os.environ", {"MINIMAX_API_KEY": "test-minimax-key"}):
292+
with patch(
293+
"src.core.common.logging_utils.get_logger"
294+
) as mock_get_logger:
295+
mock_logger = MagicMock()
296+
mock_get_logger.return_value = mock_logger
297+
298+
discover_api_keys_from_config_and_env(config)
299+
300+
# Verify no warning logged
301+
mock_logger.warning.assert_not_called()
302+
303+
# Reset warnings for next case
304+
_logged_security_warnings.clear()
305+
306+
# Case 2: Key does NOT match env var -> Warning logged
307+
with patch.dict("os.environ", {"MINIMAX_API_KEY": "different-key"}):
308+
with patch(
309+
"src.core.common.logging_utils.get_logger"
310+
) as mock_get_logger:
311+
mock_logger = MagicMock()
312+
mock_get_logger.return_value = mock_logger
313+
314+
discover_api_keys_from_config_and_env(config)
315+
316+
# Verify warning logged
317+
mock_logger.warning.assert_called_once()
318+
assert "SECURITY WARNING" in mock_logger.warning.call_args[0][0]

0 commit comments

Comments
 (0)