From 1301c68e7bdbb82be6943dadd1b5cd96538a56b7 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sun, 19 Oct 2025 14:03:21 +0300 Subject: [PATCH] feat: add configurable sensitive data masking in logs Added configuration option to control sensitive data masking in logs, allowing users to disable masking for debugging purposes while keeping it enabled by default for security. Configuration: - New `mask-sensitive-data` boolean config (default: true) - Available as global config and per-repository override - Reads from config via get_logger_with_params() Changes: - Updated helpers.py to read mask-sensitive-data from config - Added schema definition for new config option - Updated example configs (config.yaml, .github-webhook-server.yaml) - Added 3 comprehensive tests for masking behavior - Updated README.md with usage documentation and security warnings - Updated schema validator to include new boolean field Security: - Default remains true (masking enabled) for production safety - When disabled, exposes tokens, passwords, API keys, etc. in logs - Recommended only for development/debugging Testing: - Verified default masking works (sensitive data hidden) - Verified explicit disable works (sensitive data visible) - Verified explicit enable works (sensitive data hidden) - All 53 tests pass --- README.md | 6 ++ examples/.github-webhook-server.yaml | 1 + examples/config.yaml | 2 + webhook_server/config/schema.yaml | 8 +++ webhook_server/tests/manifests/config.yaml | 1 + webhook_server/tests/test_helpers.py | 69 +++++++++++++++++++ webhook_server/tests/test_schema_validator.py | 4 +- webhook_server/utils/helpers.py | 3 +- 8 files changed, 91 insertions(+), 3 deletions(-) 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")