-
-
Notifications
You must be signed in to change notification settings - Fork 153
feat: mask PII data if env is set #1265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
New environment variable `P_MASK_PII` is added if set to true, server masks the PII data as per below - 1. emails are masked like `[email protected]` -> `TXXX@XXX` 2. urls are sent as None 3. strings are masked as all `X`
WalkthroughThe changes add a new command-line option to control PII masking and update the logic to mask sensitive data. In Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Options
participant to_prism_user
participant Masking
User->>CLI: Provide command with "--mask-pii" flag
CLI->>Options: Parse options (mask_pii set)
Options-->>to_prism_user: Pass configuration for processing
to_prism_user->>Masking: Call mask_pii_string(email)
Masking-->>to_prism_user: Return masked or original email
to_prism_user->>Masking: Call mask_pii_url(pictureURL)
Masking-->>to_prism_user: Return None (if masking enabled) or original URL
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/rbac/utils.rs (3)
60-66: Consider renaming function to better reflect conditional behaviorThe function name
mask_pii_stringimplies it always masks PII, but it only does so if themask_piiflag is enabled. Consider renaming tomaybe_mask_pii_stringto make this clearer.
68-74: Consider renaming function to better reflect conditional behaviorSimilar to the previous function, consider renaming to
maybe_mask_pii_urlto clarify that masking only occurs when the flag is enabled.
104-147: Consider adding tests for the conditional masking functionsThe tests currently only test the
mask_stringfunction directly. Consider adding tests formask_pii_stringandmask_pii_urlfunctions with both enabled and disabled masking to verify their conditional behavior.For example:
#[test] fn test_mask_pii_string_when_enabled() { // Mock PARSEABLE.options.mask_pii to be true // Verify that masking occurs } #[test] fn test_mask_pii_string_when_disabled() { // Mock PARSEABLE.options.mask_pii to be false // Verify that no masking occurs }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/cli.rs(1 hunks)src/rbac/utils.rs(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/rbac/utils.rs (1)
src/cli.rs (3) (3)
format(427-432)format(441-446)format(497-498)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: coverage
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
🔇 Additional comments (5)
src/cli.rs (1)
177-183: Implementation of PII masking flag looks goodThe addition of the
mask_piiflag is properly configured with command-line argument, environment variable, default value, and helpful description. This implementation matches the pattern used for other boolean options in the codebase.src/rbac/utils.rs (4)
20-22: LGTM: Required imports for PII maskingThe addition of these imports is necessary for the new PII masking functionality.
54-56: LGTM: Updated to_prism_user to use PII maskingThe function now correctly applies PII masking to sensitive fields.
76-102: LGTM: PII masking implementation matches requirementsThe implementation follows the specified requirements:
- Email addresses are masked with the first character of the username followed by X's, and @xxx
- Other strings are completely masked with X's
The function handles edge cases appropriately: None values, invalid email formats, and empty usernames.
108-146: LGTM: Comprehensive tests for masking functionalityThe tests thoroughly cover the different scenarios: valid emails, invalid emails, empty strings, generic strings, and None values.
New environment variable
P_MASK_PIIis addedif set to true, server masks the PII data as per below -
[email protected]->TXXX@XXXXSummary by CodeRabbit
New Features
Tests