Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
```

Expand Down
2 changes: 2 additions & 0 deletions examples/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 30 additions & 2 deletions webhook_server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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


Expand Down
8 changes: 8 additions & 0 deletions webhook_server/config/schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
107 changes: 107 additions & 0 deletions webhook_server/tests/test_logging_separation.py
Original file line number Diff line number Diff line change
@@ -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)
38 changes: 28 additions & 10 deletions webhook_server/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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.

Expand Down