diff --git a/README.md b/README.md index c2733037..0c18064d 100644 --- a/README.md +++ b/README.md @@ -1310,8 +1310,11 @@ Configure comprehensive logging: ```yaml log-level: INFO # DEBUG, INFO, WARNING, ERROR log-file: /path/to/webhook-server.log +mask-sensitive-data: true # Mask sensitive data (tokens, passwords) in logs (default: true) ``` +**Security Note**: Set `mask-sensitive-data: false` only for debugging purposes in development. In production environments, always keep it `true` to prevent exposure of sensitive credentials in logs. + ### Metrics and Observability - **Request/Response logging** with delivery IDs @@ -1394,8 +1397,11 @@ Enable detailed logging: ```yaml log-level: DEBUG +mask-sensitive-data: false # Only for debugging - NOT recommended in production ``` +**⚠️ Warning**: Disabling sensitive data masking will expose tokens, passwords, and API keys in logs. Use only in development environments. + ### Log Analysis Key log patterns to monitor: diff --git a/examples/.github-webhook-server.yaml b/examples/.github-webhook-server.yaml index 9940acb3..44b1023a 100644 --- a/examples/.github-webhook-server.yaml +++ b/examples/.github-webhook-server.yaml @@ -5,6 +5,7 @@ # Logging configuration (overrides global settings) log-level: DEBUG # Options: INFO, DEBUG log-file: /path/to/repository-specific.log +mask-sensitive-data: false # Disable masking for debugging (NOT recommended in production) # Slack integration slack-webhook-url: https://hooks.slack.com/services/YOUR/SLACK/WEBHOOK diff --git a/examples/config.yaml b/examples/config.yaml index f5bdd216..2966d59b 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -2,6 +2,7 @@ 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 +mask-sensitive-data: true # Mask sensitive data in logs (default: true). Set to false for debugging (NOT recommended in production) # Server configuration disable-ssl-warnings: true # Disable SSL warnings (useful in production to reduce log noise from SSL certificate issues) @@ -59,6 +60,7 @@ repositories: name: my-org/my-repository log-level: DEBUG # Override global log-level for repository log-file: my-repository.log # Override global log-file for repository + mask-sensitive-data: false # Override global setting - disable masking for debugging this specific repo (NOT recommended in production) slack-webhook-url: # Send notification to slack on several operations verified-job: true pypi: diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index dbf40a47..444a1ea6 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -10,6 +10,10 @@ properties: log-file: type: string description: File path for the log file + mask-sensitive-data: + type: boolean + description: Mask sensitive data in logs (tokens, passwords, secrets, etc.). Default is true for security. + default: true github-app-id: type: integer description: The `webhook server` GitHub app ID @@ -116,6 +120,10 @@ properties: log-file: type: string description: Override global log-file for repository + mask-sensitive-data: + type: boolean + description: Override global mask-sensitive-data setting for this repository + default: true slack-webhook-url: type: string description: Slack webhook URL diff --git a/webhook_server/tests/manifests/config.yaml b/webhook_server/tests/manifests/config.yaml index 0837396f..e00f459b 100644 --- a/webhook_server/tests/manifests/config.yaml +++ b/webhook_server/tests/manifests/config.yaml @@ -1,5 +1,6 @@ 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 +mask-sensitive-data: true # Mask sensitive data in logs (default: true) github-app-id: 123456 # GitHub app id github-tokens: diff --git a/webhook_server/tests/test_helpers.py b/webhook_server/tests/test_helpers.py index 7b4fe7b1..01909088 100644 --- a/webhook_server/tests/test_helpers.py +++ b/webhook_server/tests/test_helpers.py @@ -232,6 +232,75 @@ def test_get_logger_with_params_log_file_path(self, tmp_path, monkeypatch): assert log_dir.exists() assert (log_dir / "test.log").exists() or True # File may not be created until logging + def test_get_logger_with_params_mask_sensitive_default(self, tmp_path): + """Test get_logger_with_params masks sensitive data by default.""" + with patch("webhook_server.utils.helpers.Config") as mock_config: + # Set up config to return default values (mask_sensitive not set) + def get_value_side_effect(value, **kwargs): + if value == "log-file": + return "test.log" + if value == "log-level": + return "INFO" + if value == "mask-sensitive-data": + return kwargs.get("return_on_none", True) + return kwargs.get("return_on_none") + + mock_config.return_value.get_value.side_effect = get_value_side_effect + mock_config.return_value.data_dir = str(tmp_path) + + with patch("webhook_server.utils.helpers.get_logger") as mock_get_logger: + get_logger_with_params() + # Verify mask_sensitive=True was passed + mock_get_logger.assert_called_once() + call_kwargs = mock_get_logger.call_args[1] + assert call_kwargs["mask_sensitive"] is True + + def test_get_logger_with_params_mask_sensitive_disabled(self, tmp_path): + """Test get_logger_with_params respects mask-sensitive-data=false config.""" + with patch("webhook_server.utils.helpers.Config") as mock_config: + # Set up config to explicitly disable masking + def get_value_side_effect(value, **kwargs): + if value == "log-file": + return "test.log" + if value == "log-level": + return "INFO" + if value == "mask-sensitive-data": + return False # Explicitly disabled + return kwargs.get("return_on_none") + + mock_config.return_value.get_value.side_effect = get_value_side_effect + mock_config.return_value.data_dir = str(tmp_path) + + with patch("webhook_server.utils.helpers.get_logger") as mock_get_logger: + get_logger_with_params() + # Verify mask_sensitive=False was passed + mock_get_logger.assert_called_once() + call_kwargs = mock_get_logger.call_args[1] + assert call_kwargs["mask_sensitive"] is False + + def test_get_logger_with_params_mask_sensitive_enabled_explicit(self, tmp_path): + """Test get_logger_with_params respects mask-sensitive-data=true config.""" + with patch("webhook_server.utils.helpers.Config") as mock_config: + # Set up config to explicitly enable masking + def get_value_side_effect(value, **kwargs): + if value == "log-file": + return "test.log" + if value == "log-level": + return "INFO" + if value == "mask-sensitive-data": + return True # Explicitly enabled + return kwargs.get("return_on_none") + + mock_config.return_value.get_value.side_effect = get_value_side_effect + mock_config.return_value.data_dir = str(tmp_path) + + with patch("webhook_server.utils.helpers.get_logger") as mock_get_logger: + get_logger_with_params() + # Verify mask_sensitive=True was passed + mock_get_logger.assert_called_once() + call_kwargs = mock_get_logger.call_args[1] + assert call_kwargs["mask_sensitive"] is True + @pytest.mark.asyncio async def test_run_command_success(self): """Test run_command with a successful command.""" diff --git a/webhook_server/tests/test_schema_validator.py b/webhook_server/tests/test_schema_validator.py index 750c536f..be6c2778 100644 --- a/webhook_server/tests/test_schema_validator.py +++ b/webhook_server/tests/test_schema_validator.py @@ -67,7 +67,7 @@ def _validate_root_fields(self, config: dict[str, Any]) -> None: self.errors.append(f"Field '{field}' must be an integer") # Boolean fields - boolean_fields = ["verify-github-ips", "verify-cloudflare-ips", "disable-ssl-warnings"] + boolean_fields = ["verify-github-ips", "verify-cloudflare-ips", "disable-ssl-warnings", "mask-sensitive-data"] for field in boolean_fields: if field in config and not isinstance(config[field], bool): self.errors.append(f"Field '{field}' must be a boolean") @@ -160,7 +160,7 @@ def _validate_single_repository(self, repo_name: str, repo_config: Any) -> None: self.errors.append(f"Repository '{repo_name}' field '{field}' must be a string") # Boolean fields - boolean_fields = ["verified-job", "pre-commit"] + boolean_fields = ["verified-job", "pre-commit", "mask-sensitive-data"] for field in boolean_fields: if field in repo_config and not isinstance(repo_config[field], bool): self.errors.append(f"Repository '{repo_name}' field '{field}' must be a boolean") diff --git a/webhook_server/utils/helpers.py b/webhook_server/utils/helpers.py index 91552bc4..434634e8 100644 --- a/webhook_server/utils/helpers.py +++ b/webhook_server/utils/helpers.py @@ -24,7 +24,6 @@ def get_logger_with_params( repository_name: str = "", - mask_sensitive: bool = True, ) -> Logger: mask_sensitive_patterns: list[str] = [ # Passwords and secrets @@ -63,6 +62,8 @@ def get_logger_with_params( log_level: str = _config.get_value(value="log-level", return_on_none="INFO") log_file: str = _config.get_value(value="log-file") + # 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")