Skip to content

Comments

feat: add configurable sensitive data masking in logs#873

Merged
myakove merged 1 commit intomainfrom
feature/configurable-log-masking
Oct 19, 2025
Merged

feat: add configurable sensitive data masking in logs#873
myakove merged 1 commit intomainfrom
feature/configurable-log-masking

Conversation

@myakove
Copy link
Collaborator

@myakove myakove commented Oct 19, 2025

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

Summary by CodeRabbit

  • New Features

    • Added mask-sensitive-data configuration option to control sensitive data masking in logs (enabled by default).
    • Option configurable at both global and repository levels for granular control.
    • Can be disabled for debugging purposes (not recommended in production).
  • Documentation

    • Updated documentation with guidance on the new masking configuration option.

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
@myakove-bot
Copy link
Collaborator

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: A tracking issue is created for this PR and will be closed when the PR is merged or closed
  • Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest build-container - Rebuild and test container image
  • /retest python-module-install - Test Python package installation
  • /retest pre-commit - Run pre-commit hooks and checks
  • /retest conventional-title - Validate commit message format
  • /retest all - Run all available tests

Container Operations

  • /build-and-push-container - Build and push container image (tagged with PR number)
    • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 1 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No WIP, hold, or conflict labels
  5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

  • myakove
  • rnetser

Reviewers:

  • myakove
  • rnetser
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is automatically removed on each new commit
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Container Builds: Container images are automatically tagged with the PR number
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@coderabbitai
Copy link

coderabbitai bot commented Oct 19, 2025

Walkthrough

This PR introduces a new configuration option mask-sensitive-data to control whether sensitive data is masked in logs. The option is added to the configuration schema at global and repository levels with a default value of true, integrated into example configurations, and the logger initialization logic is updated to retrieve the masking preference from configuration instead of a function parameter.

Changes

Cohort / File(s) Summary
Configuration Schema & Examples
webhook_server/config/schema.yaml, examples/config.yaml, examples/.github-webhook-server.yaml
Added mask-sensitive-data boolean property (default: true) at both root and repository levels with descriptions documenting the security implications of disabling masking
Test Configuration
webhook_server/tests/manifests/config.yaml
Added global mask-sensitive-data: true configuration option to test manifest
Logger Implementation
webhook_server/utils/helpers.py
Modified get_logger_with_params() to retrieve mask-sensitive-data from config via _config.get_value() instead of accepting it as a function parameter
Schema Validation Tests
webhook_server/tests/test_schema_validator.py
Added mask-sensitive-data as a validated boolean field at root and repository configuration levels
Logger Helper Tests
webhook_server/tests/test_helpers.py
Added three unit tests verifying correct mask_sensitive argument passed to get_logger() for default, disabled, and explicitly enabled scenarios
Documentation
README.md
Updated documentation to describe the new mask-sensitive-data configuration option and security considerations

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

The changes follow a consistent homogeneous pattern (adding the same config option across files), but the logic modification in helpers.py requires verification that config retrieval works correctly and that removing the function parameter does not break existing callers. Schema validation changes and new tests are straightforward confirmations of the feature.

Possibly related issues

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "feat: add configurable sensitive data masking in logs" directly and accurately describes the main change introduced by this pull request. The changeset adds a new boolean configuration option mask-sensitive-data that allows users to control whether sensitive data is masked in logs, available both globally and per-repository. The title is specific and clear—it conveys that a configurable feature is being added for log masking behavior—and avoids vague terminology. This title would effectively communicate the primary change to teammates scanning the project history.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/configurable-log-masking

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf0a7fd and 1301c68.

📒 Files selected for processing (8)
  • README.md (2 hunks)
  • examples/.github-webhook-server.yaml (1 hunks)
  • examples/config.yaml (2 hunks)
  • webhook_server/config/schema.yaml (2 hunks)
  • webhook_server/tests/manifests/config.yaml (1 hunks)
  • webhook_server/tests/test_helpers.py (1 hunks)
  • webhook_server/tests/test_schema_validator.py (2 hunks)
  • webhook_server/utils/helpers.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
webhook_server/tests/test_helpers.py (2)
webhook_server/libs/config.py (1)
  • get_value (68-88)
webhook_server/utils/helpers.py (1)
  • get_logger_with_params (25-89)
webhook_server/utils/helpers.py (1)
webhook_server/libs/config.py (1)
  • get_value (68-88)
🔇 Additional comments (13)
examples/config.yaml (3)

5-5: LGTM! Secure default for global masking configuration.

The global mask-sensitive-data: true setting provides a secure default that masks sensitive data in logs. The inline comment clearly documents the purpose and includes an appropriate production warning.


63-63: LGTM! Clear repository-level override example.

The repository-specific override demonstrates how to disable masking for debugging while maintaining the appropriate production warning. This example will help users understand the configuration hierarchy.


8-8: LGTM! Appropriate example for repository-specific configuration.

This example clearly demonstrates how individual repositories can override the global masking setting for debugging purposes, with proper security warnings included.

webhook_server/config/schema.yaml (2)

13-16: LGTM! Well-defined root-level schema property.

The schema correctly defines mask-sensitive-data as a boolean with a secure default value of true and includes a clear security-focused description.


123-126: LGTM! Consistent repository-level schema definition.

The repository-level schema definition mirrors the root-level property correctly, enabling per-repository overrides while maintaining the secure default.

README.md (2)

1313-1317: LGTM! Clear documentation with prominent security guidance.

The logging configuration documentation clearly explains the mask-sensitive-data option with its default value and includes an appropriately emphasized security note about production vs. development usage.


1400-1404: LGTM! Appropriate debug mode documentation with warnings.

The debug mode section correctly documents disabling sensitive data masking for debugging purposes and includes a prominent warning (with emoji) about the security implications of exposing credentials in logs.

webhook_server/tests/manifests/config.yaml (1)

3-3: LGTM! Test manifest correctly includes the new configuration option.

The test manifest appropriately includes mask-sensitive-data: true with the secure default value, enabling proper testing of the masking behavior.

webhook_server/tests/test_schema_validator.py (2)

70-70: LGTM! Root-level validation correctly updated.

The validator properly includes mask-sensitive-data in the root-level boolean fields list, ensuring type safety for the global configuration option.


163-163: LGTM! Repository-level validation correctly updated.

The validator properly includes mask-sensitive-data in the repository-level boolean fields list, ensuring consistent type validation for repository-specific overrides.

webhook_server/tests/test_helpers.py (1)

235-303: LGTM! Comprehensive test coverage for masking behavior.

The three new tests comprehensively verify the mask-sensitive-data configuration integration:

  1. Default behavior (masking enabled when config not set)
  2. Explicit disable (respects false config value)
  3. Explicit enable (respects true config value)

The mocking strategy correctly isolates the config retrieval logic and verifies that the mask_sensitive parameter is passed correctly to the underlying logger.

webhook_server/utils/helpers.py (1)

65-66: LGTM! Clean config-driven implementation with secure defaults.

The implementation correctly retrieves mask-sensitive-data from configuration with a secure default of True (via return_on_none=True). The comment clearly documents the purpose, and the Config.get_value method properly handles the configuration hierarchy (repository-level overrides global level).

examples/.github-webhook-server.yaml (1)

8-8: Config option placement and documentation look good.

The new mask-sensitive-data option is correctly added under the Logging configuration section with consistent naming conventions (kebab-case) and includes a clear security warning. The false value appropriately demonstrates how to disable masking for debugging purposes in a repository-specific override.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@myakove
Copy link
Collaborator Author

myakove commented Oct 19, 2025

/approve
/verified

@myakove myakove merged commit 154931c into main Oct 19, 2025
8 of 9 checks passed
@myakove myakove deleted the feature/configurable-log-masking branch October 19, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants