-
Notifications
You must be signed in to change notification settings - Fork 153
[ES-2777] Always show configuration through actuator/env with some key/values sanitized #1608
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
…anitized Signed-off-by: Sachin Rana <sacrana324@gmail.com>
WalkthroughAdds a new Spring configuration class that implements SanitizingFunction to mask actuator environment values based on configurable suffix and substring key patterns; includes unit tests and enables actuator env values in bootstrap properties. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2026-01-16T08:59:50.574ZApplied to files:
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting 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. Comment |
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: 2
🤖 Fix all issues with AI agents
In
`@esignet-core/src/main/java/io/mosip/esignet/core/config/ActuatorSanitizationConfig.java`:
- Around line 24-30: In apply(SanitizableData data) add a null-safety check for
data.getKey() before calling toLowerCase: obtain the key, return data
immediately (or treat as non-matching) if key is null, otherwise call
key.toLowerCase(Locale.ROOT) and proceed with the existing checks against
keysToBeSanitizedEndsWith and keysToBeSanitizedContains; keep using
data.withSanitizedValue() when a match is found so behavior is unchanged for
non-null keys.
In `@esignet-service/src/main/resources/bootstrap.properties`:
- Line 44: The bootstrap property management.endpoint.env.show-values=ALWAYS is
safe only because actuator access is restricted by
SecurityConfig.requests.anyRequest().authenticated(); verify and update
ActuatorSanitizationConfig to include any deployment-specific sensitive
environment variable names (add entries under
mosip.esignet.actuator.sanitize.key.*) so keys beyond the defaults (password,
secret, key, token, vcap_services, sun.java.command, credentials) are sanitized
before the env endpoint reveals values; locate ActuatorSanitizationConfig and
add the custom sensitive keys your deployment uses.
🧹 Nitpick comments (2)
esignet-core/src/main/java/io/mosip/esignet/core/config/ActuatorSanitizationConfig.java (1)
18-22: Consider reviewing the default sanitization patterns.The default patterns provide good baseline coverage, but note:
"key"may over-sanitize non-sensitive properties ending with "key" (e.g.,cache.entry.key,partition.key)- Consider adding common sensitive patterns like
"auth","bearer","private"tokeysToBeSanitizedContainsThis is a minor consideration since patterns are configurable via properties.
esignet-core/src/test/java/io/mosip/esignet/core/config/ActuatorSanitizationConfigTest.java (1)
24-65: Tests cover the core scenarios well.The three tests adequately verify endsWith, contains, and no-match paths. The
API_CREDENTIALS_IDtest nicely validates case-insensitive matching.Consider adding edge case tests for robustness (optional):
- Null key handling
- Empty pattern lists
- Key matching both endsWith and contains patterns
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
esignet-core/src/main/java/io/mosip/esignet/core/config/ActuatorSanitizationConfig.javaesignet-core/src/test/java/io/mosip/esignet/core/config/ActuatorSanitizationConfigTest.javaesignet-service/src/main/resources/bootstrap.properties
🔇 Additional comments (1)
esignet-core/src/test/java/io/mosip/esignet/core/config/ActuatorSanitizationConfigTest.java (1)
17-22: LGTM! Test setup is clean.Good use of
ReflectionTestUtilsto inject test-specific values, allowing focused testing of the sanitization logic.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
esignet-core/src/main/java/io/mosip/esignet/core/config/ActuatorSanitizationConfig.java
Show resolved
Hide resolved
Signed-off-by: Sachin Rana <sacrana324@gmail.com>
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.