diff --git a/webhook_server/app.py b/webhook_server/app.py index 55759a6f..d1e0461a 100644 --- a/webhook_server/app.py +++ b/webhook_server/app.py @@ -43,7 +43,7 @@ # Global variables ALLOWED_IPS: tuple[ipaddress._BaseNetwork, ...] = () -LOGGER = get_logger_with_params(name="main") +LOGGER = get_logger_with_params() _lifespan_http_client: httpx.AsyncClient | None = None @@ -210,7 +210,7 @@ async def process_webhook(request: Request, background_tasks: BackgroundTasks) - # Create repository-specific logger repository_name = hook_data["repository"]["name"] - logger = get_logger_with_params(name="main", repository_name=repository_name) + logger = get_logger_with_params(repository_name=repository_name) logger.info(f"{log_context} Processing webhook for repository: {repository_name}") async def process_with_error_handling(_api: GithubWebhook, _logger: logging.Logger) -> None: diff --git a/webhook_server/tests/test_helpers.py b/webhook_server/tests/test_helpers.py index 4070b587..7b4fe7b1 100644 --- a/webhook_server/tests/test_helpers.py +++ b/webhook_server/tests/test_helpers.py @@ -63,14 +63,14 @@ def test_extract_key_from_dict_complex_nested(self) -> None: def test_get_logger_with_params_default(self) -> None: """Test logger creation with default parameters.""" - unique_name = "test_helpers_logger" - logger = get_logger_with_params(name=unique_name) + logger = get_logger_with_params() assert isinstance(logger, logging.Logger) - assert logger.name == unique_name + # Logger name is now based on log file path to ensure single handler instance + assert logger.name.startswith("webhook-server-") def test_get_logger_with_params_with_repository(self) -> None: """Test logger creation with repository name.""" - logger = get_logger_with_params(name="test", repository_name="test-repo") + logger = get_logger_with_params(repository_name="test-repo") assert isinstance(logger, logging.Logger) # The logger should have repository-specific formatting @@ -226,7 +226,7 @@ def test_get_logger_with_params_log_file_path(self, tmp_path, monkeypatch): mock_config = MockConfig.return_value mock_config.get_value.side_effect = lambda value, **kwargs: "test.log" if value == "log-file" else "INFO" mock_config.data_dir = str(tmp_path) - logger = get_logger_with_params(name="test_logger", repository_name="repo") + logger = get_logger_with_params(repository_name="repo") assert isinstance(logger, logging.Logger) log_dir = tmp_path / "logs" assert log_dir.exists() diff --git a/webhook_server/utils/app_utils.py b/webhook_server/utils/app_utils.py index 7ef7240e..5786d025 100644 --- a/webhook_server/utils/app_utils.py +++ b/webhook_server/utils/app_utils.py @@ -16,7 +16,7 @@ CLOUDFLARE_IPS_URL: str = "https://api.cloudflare.com/client/v4/ips" # Logger for utilities -LOGGER = get_logger_with_params(name="app_utils") +LOGGER = get_logger_with_params() async def get_github_allowlist(http_client: httpx.AsyncClient) -> list[str]: diff --git a/webhook_server/utils/github_repository_and_webhook_settings.py b/webhook_server/utils/github_repository_and_webhook_settings.py index 8d6fd2c8..d5caa3f8 100644 --- a/webhook_server/utils/github_repository_and_webhook_settings.py +++ b/webhook_server/utils/github_repository_and_webhook_settings.py @@ -11,7 +11,7 @@ from webhook_server.utils.helpers import get_api_with_highest_rate_limit, get_logger_with_params from webhook_server.utils.webhook import create_webhook -LOGGER = get_logger_with_params(name="repository-and-webhook-settings") +LOGGER = get_logger_with_params() def get_repository_api(repository: str) -> tuple[str, github.Github | None, str]: diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index d213baca..6457dfd0 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -42,7 +42,7 @@ "required_conversation_resolution": True, } -LOGGER = get_logger_with_params(name="github-repository-settings") +LOGGER = get_logger_with_params() def _get_github_repo_api(github_api: github.Github, repository: int | str) -> Repository | None: diff --git a/webhook_server/utils/helpers.py b/webhook_server/utils/helpers.py index f7e38266..91552bc4 100644 --- a/webhook_server/utils/helpers.py +++ b/webhook_server/utils/helpers.py @@ -23,17 +23,40 @@ def get_logger_with_params( - name: str, repository_name: str = "", mask_sensitive: bool = True, ) -> Logger: mask_sensitive_patterns: list[str] = [ + # Passwords and secrets "container_repository_password", - "-p", "password", + "secret", + # Tokens and API keys "token", "apikey", - "secret", + "api_key", + "github_token", + "GITHUB_TOKEN", + "pypi", + # Authentication credentials + "username", + "login", + "-u", + "-p", + "--username", + "--password", + "--creds", + # Private keys and sensitive IDs + "private_key", + "private-key", + "webhook_secret", + "webhook-secret", + "github-app-id", + # Slack webhooks (contain sensitive URLs) + "slack-webhook-url", + "slack_webhook_url", + "webhook-url", + "webhook_url", ] _config = Config(repository=repository_name) @@ -49,8 +72,14 @@ def get_logger_with_params( log_file = os.path.join(log_file_path, 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 = f"webhook-server-{log_file or 'console'}" + return get_logger( - name=name, + name=logger_cache_key, filename=log_file, level=log_level, file_max_bytes=1024 * 1024 * 10, @@ -74,7 +103,7 @@ def extract_key_from_dict(key: Any, _dict: dict[Any, Any]) -> Any: def get_github_repo_api(github_app_api: github.Github, repository: int | str) -> Repository: - logger = get_logger_with_params(name="helpers") + logger = get_logger_with_params() logger.debug(f"Get GitHub API for repository {repository}") return github_app_api.get_repo(repository) @@ -97,7 +126,7 @@ async def run_command( Returns: tuple: True, out if command succeeded, False, err otherwise. """ - logger = get_logger_with_params(name="helpers") + logger = get_logger_with_params() out_decoded: str = "" err_decoded: str = "" kwargs["stdout"] = subprocess.PIPE @@ -158,7 +187,7 @@ def get_api_with_highest_rate_limit(config: Config, repository_name: str = "") - Returns: tuple: API, token, api_user """ - logger = get_logger_with_params(name="helpers") + logger = get_logger_with_params() api: github.Github | None = None token: str | None = None @@ -204,7 +233,7 @@ def get_api_with_highest_rate_limit(config: Config, repository_name: str = "") - def log_rate_limit(rate_limit: RateLimitOverview, api_user: str) -> None: - logger = get_logger_with_params(name="helpers") + logger = get_logger_with_params() rate_limit_str: str time_for_limit_reset: int = (rate_limit.rate.reset - datetime.datetime.now(tz=datetime.timezone.utc)).seconds diff --git a/webhook_server/utils/webhook.py b/webhook_server/utils/webhook.py index 8cb79c91..30214106 100644 --- a/webhook_server/utils/webhook.py +++ b/webhook_server/utils/webhook.py @@ -10,7 +10,7 @@ get_logger_with_params, ) -LOGGER = get_logger_with_params(name="webhook") +LOGGER = get_logger_with_params() def process_github_webhook(