diff --git a/README.md b/README.md index 3774aed7..07fabc35 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,8 @@ port: 5000 max-workers: 20 log-level: INFO log-file: webhook-server.log +mcp-log-file: mcp_server.log +logs-server-log-file: logs_server.log # Security Configuration webhook-secret: "your-webhook-secret" # pragma: allowlist secret @@ -1364,6 +1366,8 @@ Configure comprehensive logging: ```yaml log-level: INFO # DEBUG, INFO, WARNING, ERROR log-file: /path/to/webhook-server.log +mcp-log-file: /path/to/mcp_server.log +logs-server-log-file: /path/to/logs_server.log mask-sensitive-data: true # Mask sensitive data (tokens, passwords) in logs (default: true) ``` diff --git a/examples/config.yaml b/examples/config.yaml index d134708d..41fc4084 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -2,6 +2,8 @@ log-level: INFO # Set global log level, change take effect immediately without server restart log-file: webhook-server.log # Set global log file, change take effect immediately without server restart +mcp-log-file: mcp_server.log # Set global MCP log file, change take effect immediately without server restart +logs-server-log-file: logs_server.log # Set global Logs Server log file, change take effect immediately without server restart mask-sensitive-data: true # Mask sensitive data in logs (default: true). Set to false for debugging (NOT recommended in production) # Server configuration diff --git a/webhook_server/app.py b/webhook_server/app.py index 7340dc47..28a79ae0 100644 --- a/webhook_server/app.py +++ b/webhook_server/app.py @@ -40,7 +40,10 @@ parse_datetime_string, verify_signature, ) -from webhook_server.utils.helpers import get_logger_with_params, prepare_log_prefix +from webhook_server.utils.helpers import ( + get_logger_with_params, + prepare_log_prefix, +) from webhook_server.web.log_viewer import LogViewerController # Constants @@ -120,6 +123,25 @@ async def lifespan(_app: FastAPI) -> AsyncGenerator[None, None]: config = Config(logger=LOGGER) root_config = config.root_data + + # Configure MCP logging separation + if MCP_SERVER_ENABLED: + mcp_log_file = root_config.get("mcp-log-file", "mcp_server.log") + + # Use get_logger_with_params to reuse existing logging configuration logic + # (rotation, sensitive data masking, formatting) + # This returns a logger configured for the specific file + mcp_file_logger = get_logger_with_params(log_file_name=mcp_log_file) + + # Add the configured handler to the actual MCP logger and stop propagation + # This ensures MCP logs go ONLY to mcp_server.log and not webhook_server.log + if mcp_file_logger.handlers: + for handler in mcp_file_logger.handlers: + mcp_logger.addHandler(handler) + + mcp_logger.propagate = False + LOGGER.info(f"MCP logging configured to: {mcp_log_file} via handlers from {mcp_file_logger.name}") + verify_github_ips = root_config.get("verify-github-ips", False) verify_cloudflare_ips = root_config.get("verify-cloudflare-ips", False) disable_ssl_warnings = root_config.get("disable-ssl-warnings", False) @@ -428,7 +450,13 @@ def get_log_viewer_controller() -> LogViewerController: """ global _log_viewer_controller_singleton if _log_viewer_controller_singleton is None: - _log_viewer_controller_singleton = LogViewerController(logger=LOGGER) + # Use global LOGGER for config operations + config = Config(logger=LOGGER) + logs_server_log_file = config.get_value("logs-server-log-file", return_on_none="logs_server.log") + + # Create dedicated logger for log viewer + log_viewer_logger = get_logger_with_params(log_file_name=logs_server_log_file) + _log_viewer_controller_singleton = LogViewerController(logger=log_viewer_logger) return _log_viewer_controller_singleton diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 77324641..92d67b26 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -10,6 +10,14 @@ properties: log-file: type: string description: File path for the log file + mcp-log-file: + type: string + description: File path for the MCP log file + default: mcp_server.log + logs-server-log-file: + type: string + description: File path for the Logs Server log file + default: logs_server.log mask-sensitive-data: type: boolean description: Mask sensitive data in logs (tokens, passwords, secrets, etc.). Default is true for security. diff --git a/webhook_server/tests/test_logging_separation.py b/webhook_server/tests/test_logging_separation.py new file mode 100644 index 00000000..91060d73 --- /dev/null +++ b/webhook_server/tests/test_logging_separation.py @@ -0,0 +1,107 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +import webhook_server.app +from webhook_server.app import get_log_viewer_controller, lifespan +from webhook_server.libs.config import Config +from webhook_server.utils.helpers import get_log_file_path + + +def test_get_log_file_path_absolute(): + config = MagicMock(spec=Config) + config.data_dir = "/tmp/data" + path = get_log_file_path(config, "/absolute/path.log") + assert path == "/absolute/path.log" + + +def test_get_log_file_path_relative(): + config = MagicMock(spec=Config) + config.data_dir = "/tmp/data" + # Mock os.makedirs to avoid filesystem side effects + with patch("os.makedirs") as mock_makedirs: + path = get_log_file_path(config, "server.log") + assert path == "/tmp/data/logs/server.log" + mock_makedirs.assert_called_once_with("/tmp/data/logs", exist_ok=True) + + +def test_get_log_file_path_none(): + config = MagicMock(spec=Config) + path = get_log_file_path(config, None) + assert path is None + + +@pytest.mark.asyncio +async def test_mcp_logging_configuration(): + # Mock dependencies + mock_app = MagicMock() + + with ( + patch("webhook_server.app.Config") as MockConfig, + patch("webhook_server.app.MCP_SERVER_ENABLED", True), + patch("webhook_server.app.logging.getLogger") as mock_get_logger, + patch("webhook_server.app.get_logger_with_params") as mock_get_logger_params, + patch("webhook_server.app.get_github_allowlist", new_callable=AsyncMock), + patch("webhook_server.app.get_cloudflare_allowlist", new_callable=AsyncMock), + patch("webhook_server.app.httpx.AsyncClient") as MockClient, + patch("webhook_server.app.LOGGER"), + ): + # Setup mocks + mock_config_instance = MockConfig.return_value + mock_config_instance.root_data = {"mcp-log-file": "mcp_server.log", "verify-github-ips": False} + + mock_client_instance = MockClient.return_value + mock_client_instance.aclose = AsyncMock() + + mcp_logger = MagicMock() + mcp_logger.filters = [] + mock_get_logger.return_value = mcp_logger + + # Mock the logger returned by get_logger_with_params + mcp_file_logger = MagicMock() + mcp_handler = MagicMock() + mcp_file_logger.handlers = [mcp_handler] + mcp_file_logger.name = "mcp_logger" + mock_get_logger_params.return_value = mcp_file_logger + + # Run lifespan + async with lifespan(mock_app): + pass + + # Verify configuration + # Check if get_logger_with_params was called with correct log file + mock_get_logger_params.assert_any_call(log_file_name="mcp_server.log") + + # Check if handler was added to the main MCP logger + mcp_logger.addHandler.assert_called_with(mcp_handler) + + # Check if propagation was disabled + assert mcp_logger.propagate is False + + +def test_log_viewer_controller_logging_separation(): + with ( + patch("webhook_server.app.Config") as MockConfig, + patch("webhook_server.app.get_logger_with_params") as mock_get_logger_params, + patch("webhook_server.app.LogViewerController") as MockController, + patch("webhook_server.app.LOGGER"), + ): + # Reset singleton + webhook_server.app._log_viewer_controller_singleton = None + + # Setup config + mock_config_instance = MockConfig.return_value + mock_config_instance.get_value.side_effect = ( + lambda value, return_on_none=None: "logs_server.log" if value == "logs-server-log-file" else return_on_none + ) + + # Setup logger + mock_logger = MagicMock() + mock_get_logger_params.return_value = mock_logger + + # Call function + get_log_viewer_controller() + + # Verify + mock_get_logger_params.assert_called_with(log_file_name="logs_server.log") + MockController.assert_called_with(logger=mock_logger) diff --git a/webhook_server/utils/helpers.py b/webhook_server/utils/helpers.py index 03e0d854..46062fb6 100644 --- a/webhook_server/utils/helpers.py +++ b/webhook_server/utils/helpers.py @@ -30,6 +30,7 @@ def get_logger_with_params( repository_name: str = "", + log_file_name: str | None = None, ) -> Logger: mask_sensitive_patterns: list[str] = [ # Passwords and secrets @@ -67,27 +68,22 @@ def get_logger_with_params( _config = Config(repository=repository_name) log_level: str = _config.get_value(value="log-level", return_on_none="INFO") - log_file: str = _config.get_value(value="log-file") + log_file_config: str = _config.get_value(value="log-file") + log_file: str | None = log_file_name or log_file_config # Get mask-sensitive-data config (default: True to hide sensitive data) mask_sensitive: bool = _config.get_value(value="mask-sensitive-data", return_on_none=True) - if log_file and not log_file.startswith("/"): - log_file_path = os.path.join(_config.data_dir, "logs") - - if not os.path.isdir(log_file_path): - os.makedirs(log_file_path, exist_ok=True) - - log_file = os.path.join(log_file_path, log_file) + log_file_path_resolved = get_log_file_path(config=_config, log_file_name=log_file) # CRITICAL FIX: Use a fixed logger name for the same log file to ensure # only ONE RotatingFileHandler instance manages the file rotation. # Multiple handlers writing to the same file causes rotation to fail. # The original 'name' parameter is preserved in log records via the logger name. - logger_cache_key = os.path.basename(log_file) if log_file else "console" + logger_cache_key = os.path.basename(log_file_path_resolved) if log_file_path_resolved else "console" return get_logger( name=logger_cache_key, - filename=log_file, + filename=log_file_path_resolved, level=log_level, file_max_bytes=1024 * 1024 * 10, mask_sensitive=mask_sensitive, @@ -96,6 +92,28 @@ def get_logger_with_params( ) +def get_log_file_path(config: Config, log_file_name: str | None) -> str | None: + """ + Resolve the full path for a log file using the configuration data directory. + + Args: + config: Config object containing data_dir + log_file_name: Name of the log file (e.g., "server.log") + + Returns: + Full path to the log file, or None if log_file_name is None + """ + if log_file_name and not log_file_name.startswith("/"): + log_file_path = os.path.join(config.data_dir, "logs") + + if not os.path.isdir(log_file_path): + os.makedirs(log_file_path, exist_ok=True) + + return os.path.join(log_file_path, log_file_name) + + return log_file_name + + def _sanitize_log_value(value: str) -> str: """Sanitize value for safe inclusion in structured log messages.