[feature] Desensitize sensitive information(new) #11
Conversation
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant Cache
participant Database
Client->>Controller: GET /receivers
Controller->>Service: getReceivers(name)
Service->>Database: Query NoticeReceiver entities
Database-->>Service: List<NoticeReceiver>
Service->>Controller: List<NoticeReceiver>
Controller->>Controller: Map to List<NoticeReceiverVO>
Controller-->>Client: List<NoticeReceiverVO>
Client->>Controller: POST /editReceiver (with desensitized email/phone)
Controller->>Service: editReceiver(NoticeReceiver)
Service->>Cache: Lookup original email/phone by (id+masked value)
Cache-->>Service: Original email/phone (if exists)
Service->>Database: Save NoticeReceiver with restored values
Database-->>Service: Success
Service-->>Controller: Success
Controller-->>Client: Success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @arvi18, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a robust mechanism for sensitive data desensitization within the application, focusing on NoticeReceiver entities. It ensures that personal information like phone numbers and emails are masked when presented to users via the API, while maintaining the ability to access the original data internally for necessary operations. This enhances data privacy and security.
Highlights
- Data Desensitization Implementation: Implemented a new feature to desensitize sensitive information, specifically phone numbers and email addresses, for
NoticeReceiverentities when they are exposed via the API. - Custom Serialization Logic: Introduced custom Jackson
JsonSerializerimplementations (EmailDesensitizationSerializerandPhoneDesensitizationSerializer) to automatically apply desensitization rules during JSON serialization ofNoticeReceiverVOobjects. - Original Data Caching: Established a
DESENSITIZATION_MAP_CACHEwithinCacheFactoryto temporarily store the original, un-desensitized sensitive values. This allows the system to retrieve the full data when needed for internal operations, such as updating records or sending test notifications. - API and Service Layer Integration: Modified the
NoticeConfigControllerto returnNoticeReceiverVOobjects, ensuring that sensitive data is desensitized when retrieved through the API. Correspondingly,NoticeConfigServiceImplwas updated to fetch original sensitive data from the cache before performingeditReceiverorsendTestMsgoperations. - New Dependency: Added the
hutool-alllibrary as a new dependency to thehertzbeat-commonand rootpom.xmlfiles, leveraging its utility functions, particularly for phone number desensitization.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
🤖 Refacto.AI: PR Summary & Reviewer Guide🎯 1. The Goal (Why was this PR raised?)This PR implements data desensitization for sensitive information (specifically email and phone numbers) in the notification receiver functionality. The goal is to protect user privacy by masking sensitive contact information when it's displayed, while maintaining the ability to use the original data when needed. 🗺️ 2. The Changes (What was implemented?)
🤔 3. Key Areas for Human ReviewArea of Concern: Data Security in Desensitization Implementation
Area of Concern: Cache Management for Sensitive Data
Area of Concern: API Response Transformation
Area of Concern: Original Value Restoration Logic
|
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
There was a problem hiding this comment.
Code Review
This pull request introduces a feature to desensitize sensitive information. The implementation is generally good, but there are areas for improvement in dependency management, code duplication, testing, and code style. Please address the comments provided.
| SimpleModule simpleModule = new SimpleModule(); | ||
| simpleModule.addSerializer(String.class,emailDesensitizationSerializer); | ||
|
|
||
| objectMapper.registerModule(simpleModule); |
There was a problem hiding this comment.
The EmailDesensitizationSerializer is already registered via @JsonSerialize in NoticeReceiverVO. Registering it again for all strings is incorrect and unnecessary. Remove the simpleModule registration.
| SimpleModule simpleModule = new SimpleModule(); | |
| simpleModule.addSerializer(String.class,emailDesensitizationSerializer); | |
| objectMapper.registerModule(simpleModule); | |
| objectMapper.registerModule(simpleModule); |
| SimpleModule simpleModule = new SimpleModule(); | ||
| simpleModule.addSerializer(String.class,phoneDesensitizationSerializer); | ||
|
|
||
| objectMapper.registerModule(simpleModule); |
There was a problem hiding this comment.
The PhoneDesensitizationSerializer is already registered via @JsonSerialize in NoticeReceiverVO. Registering it again for all strings is incorrect and unnecessary. Remove the simpleModule registration.
| SimpleModule simpleModule = new SimpleModule(); | |
| simpleModule.addSerializer(String.class,phoneDesensitizationSerializer); | |
| objectMapper.registerModule(simpleModule); | |
| objectMapper.registerModule(simpleModule); |
| <dependency> | ||
| <groupId>cn.hutool</groupId> | ||
| <artifactId>hutool-all</artifactId> | ||
| <version>${hutool-all.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
Consider using specific hutool modules instead of hutool-all to reduce the application size. For the functionality used in this PR (StrUtil, DesensitizedUtil), hutool-core might be sufficient.
| * get desensitizationMap cache | ||
| * @return desensitizationMap cache | ||
| */ | ||
| public static CommonCacheService<String, Object> getDesensitizationMapCache(){return DESENSITIZATION_MAP_CACHE;}; |
There was a problem hiding this comment.
For better readability, format this method across multiple lines.
public static CommonCacheService<String, Object> getDesensitizationMapCache() {
return DESENSITIZATION_MAP_CACHE;
}| import jakarta.persistence.Column; | ||
| import jakarta.persistence.GeneratedValue; | ||
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; |
There was a problem hiding this comment.
The NoticeReceiverVO is a DTO, and should not have JPA annotations. Remove @Id and @GeneratedValue.
| desensitizationMapCache.put(currentValue.getId() + "_" + emailDesensitization, email); | ||
| } | ||
| jsonGenerator.writeString(emailDesensitization); | ||
| } |
There was a problem hiding this comment.
Add a newline character at the end of the file.
}| } | ||
|
|
||
| jsonGenerator.writeString(phoneDesensitization); | ||
| } |
There was a problem hiding this comment.
Add a newline character at the end of the file.
}| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | ||
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){ | ||
| noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail())); | ||
| } | ||
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){ | ||
| noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone())); | ||
| } |
There was a problem hiding this comment.
Optimize the cache retrieval logic by performing a single lookup per field. Extract this logic into a private helper method to avoid code duplication with the sendTestMsg method.
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){ | |
| noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail())); | |
| } | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){ | |
| noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone())); | |
| } | |
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail())) { | |
| Object originalEmail = desensitizationMapCache.get(noticeReceiver.getId() + "_" + noticeReceiver.getEmail()); | |
| if (ObjectUtil.isNotEmpty(originalEmail)) { | |
| noticeReceiver.setEmail((String) originalEmail); | |
| } | |
| } | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone())) { | |
| Object originalPhone = desensitizationMapCache.get(noticeReceiver.getId() + "_" + noticeReceiver.getPhone()); | |
| if (ObjectUtil.isNotEmpty(originalPhone)) { | |
| noticeReceiver.setPhone((String) originalPhone); | |
| } | |
| } |
| if (null != noticeReceiver.getId()){ | ||
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | ||
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){ | ||
| noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail())); | ||
| } | ||
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){ | ||
| noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone())); | ||
| } | ||
| } |
There was a problem hiding this comment.
This cache retrieval logic is duplicated from the editReceiver method. Extract it into a private helper method to avoid code duplication.
| if (null != noticeReceiver.getId()){ | |
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){ | |
| noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail())); | |
| } | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){ | |
| noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone())); | |
| } | |
| } | |
| if (null != noticeReceiver.getId()){ | |
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail())) { | |
| Object originalEmail = desensitizationMapCache.get(noticeReceiver.getId() + "_" + noticeReceiver.getEmail()); | |
| if (ObjectUtil.isNotEmpty(originalEmail)) { | |
| noticeReceiver.setEmail((String) originalEmail); | |
| } | |
| } | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone())) { | |
| Object originalPhone = desensitizationMapCache.get(noticeReceiver.getId() + "_" + noticeReceiver.getPhone()); | |
| if (ObjectUtil.isNotEmpty(originalPhone)) { | |
| noticeReceiver.setPhone((String) originalPhone); | |
| } | |
| } | |
| } |
| <groupId>cn.hutool</groupId> | ||
| <artifactId>hutool-all</artifactId> | ||
| <version>${hutool-all.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
Consider using specific hutool modules instead of hutool-all to reduce the application size. For the functionality used, hutool-core should be sufficient.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/serialize/EmailDesensitizationSerializer.java (1)
44-45: Add null safety checks (same issue as phone serializer).The same null safety issues exist here as in the phone serializer. Apply the same fixes for null checking and error handling.
🧹 Nitpick comments (5)
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/cache/CacheFactory.java (1)
64-68: Improve method formatting for consistency.The getter method formatting is inconsistent with other methods in the class.
- /** - * get desensitizationMap cache - * @return desensitizationMap cache - */ - public static CommonCacheService<String, Object> getDesensitizationMapCache(){return DESENSITIZATION_MAP_CACHE;}; + /** + * get desensitizationMap cache + * @return desensitizationMap cache + */ + public static CommonCacheService<String, Object> getDesensitizationMapCache() { + return DESENSITIZATION_MAP_CACHE; + }hertzbeat-common/src/test/java/org/apache/hertzbeat/common/serialize/EmailDesensitizationSerializerTest.java (1)
23-24: Improve test method naming for clarity.The test method name "test" is not descriptive and doesn't follow JUnit naming conventions.
- @Test - public void test() throws JsonProcessingException { + @Test + public void testEmailDesensitizationAndCaching() throws JsonProcessingException {hertzbeat-common/src/test/java/org/apache/hertzbeat/common/serialize/PhoneDesensitizationSerializerTest.java (2)
24-24: Use a more descriptive test method name.The method name
testis not descriptive and doesn't convey what functionality is being tested.-public void test() throws JsonProcessingException { +public void testPhoneDesensitizationAndCaching() throws JsonProcessingException {
24-41: Add edge case testing.The test only covers the happy path. Consider adding tests for edge cases such as null phone numbers, empty strings, and phones without sufficient digits for desensitization.
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/serialize/PhoneDesensitizationSerializer.java (1)
35-49: Consider making the serializer more generic.The serializer is tightly coupled to
NoticeReceiverVO. Consider using reflection or a generic approach to make it reusable with other entities that have anidfield.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
hertzbeat-common/pom.xml(1 hunks)hertzbeat-common/src/main/java/org/apache/hertzbeat/common/cache/CacheFactory.java(2 hunks)hertzbeat-common/src/main/java/org/apache/hertzbeat/common/entity/dto/vo/NoticeReceiverVO.java(1 hunks)hertzbeat-common/src/main/java/org/apache/hertzbeat/common/serialize/EmailDesensitizationSerializer.java(1 hunks)hertzbeat-common/src/main/java/org/apache/hertzbeat/common/serialize/PhoneDesensitizationSerializer.java(1 hunks)hertzbeat-common/src/test/java/org/apache/hertzbeat/common/serialize/EmailDesensitizationSerializerTest.java(1 hunks)hertzbeat-common/src/test/java/org/apache/hertzbeat/common/serialize/PhoneDesensitizationSerializerTest.java(1 hunks)hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/controller/NoticeConfigController.java(2 hunks)hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/NoticeConfigServiceImpl.java(3 hunks)pom.xml(2 hunks)
🔇 Additional comments (8)
hertzbeat-common/pom.xml (1)
146-151: LGTM! Hutool dependency addition supports desensitization features.The addition of the Hutool utility library aligns well with the new desensitization serializers and caching functionality introduced in this PR. The dependency uses property-based versioning which is consistent with other dependencies in the project.
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/cache/CacheFactory.java (1)
37-38: LGTM! Cache configuration is consistent.The new desensitization cache follows the same configuration pattern as existing caches, which ensures consistent behavior across the application.
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/controller/NoticeConfigController.java (3)
23-23: LGTM! Import addition supports VO conversion.The Hutool BeanUtil import is necessary for the entity-to-VO conversion logic.
31-31: LGTM! VO import enables desensitization.The NoticeReceiverVO import supports returning desensitized data instead of raw entities.
88-90: Double-check API clients & documentation for response schema changeWe’ve searched the codebase and found no internal Java clients referencing the old NoticeReceiver type—only the controller’s own unit test and the E2E suite hit
/api/notice/receiverswithout asserting on response fields. Because switching from NoticeReceiver to NoticeReceiverVO could alter the JSON payload, please:
- Review any external API consumers (mobile apps, third-party integrations, etc.) to ensure they tolerate the new VO schema.
- Update your OpenAPI/Swagger spec or other API documentation to reflect the new properties on NoticeReceiverVO.
- If backward compatibility is required, consider versioning the endpoint or adding compatibility fields.
Relevant locations:
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/controller/NoticeConfigController.java(lines ~88–90)hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/controller/NoticeConfigControllerTest.javapom.xml (1)
479-484: LGTM! Dependency structure is correct.The Hutool dependency declaration correctly references the version property and follows the project's dependency management patterns.
hertzbeat-common/src/test/java/org/apache/hertzbeat/common/serialize/EmailDesensitizationSerializerTest.java (1)
29-33: Consider the scope of serializer registration.Registering the EmailDesensitizationSerializer for all String.class fields may cause unintended desensitization of non-email string fields. Consider if this is the intended behavior or if the serializer should be more targeted.
Please verify that registering the serializer for all String fields aligns with the actual usage in the application. The NoticeReceiverVO likely uses field-specific annotations to control which fields get desensitized.
hertzbeat-common/src/test/java/org/apache/hertzbeat/common/serialize/PhoneDesensitizationSerializerTest.java (1)
40-40: Verify the cache key construction logic.The cache key uses
noticeReceiverVO.getPhone()which should contain the desensitized phone number after deserialization, but the test expects to find the original phone number in the cache. This suggests the cache key construction might be correct, but it would be clearer to explicitly verify both the desensitized value and the cache lookup.Add assertions to verify the desensitization worked correctly:
String jsonString = objectMapper.writeValueAsString(noticeReceiver); NoticeReceiverVO noticeReceiverVO = objectMapper.readValue(jsonString, NoticeReceiverVO.class); +// Verify phone was desensitized +assertNotEquals(noticeReceiver.getPhone(), noticeReceiverVO.getPhone()); +assertTrue(noticeReceiverVO.getPhone().contains("*")); CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); assertEquals(noticeReceiver.getPhone(),desensitizationMapCache.get(noticeReceiverVO.getId()+"_"+noticeReceiverVO.getPhone()));
| import jakarta.persistence.Column; | ||
| import jakarta.persistence.GeneratedValue; | ||
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; |
There was a problem hiding this comment.
Remove JPA entity annotations from VO class.
A VO (Value Object/View Object) should not contain JPA entity annotations like @Id, @GeneratedValue, @Column. These annotations are for entity classes that map to database tables, not for DTOs used for data transfer.
-import jakarta.persistence.Column;
-import jakarta.persistence.GeneratedValue;
-import jakarta.persistence.GenerationType;
-import jakarta.persistence.Id;
-@Id
-@GeneratedValue(strategy = GenerationType.IDENTITY)
@Schema(title = "Recipient entity primary key index ID", description = "Recipient entity primary key index ID",
example = "87584674384", accessMode = READ_ONLY)
private Long id;
-@Column(length = 300)
private String hookUrl;Apply this pattern to remove all @Column annotations throughout the class.
Also applies to: 55-56, 97-98
🤖 Prompt for AI Agents
In
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/entity/dto/vo/NoticeReceiverVO.java
at lines 22-25, 55-56, and 97-98, remove all JPA entity annotations such as @Id,
@GeneratedValue, and @Column from the VO class. These annotations are
inappropriate for a Value Object and should be eliminated to keep the class
focused on data transfer without persistence mapping.
| import org.springframework.data.annotation.CreatedBy; | ||
| import org.springframework.data.annotation.CreatedDate; | ||
| import org.springframework.data.annotation.LastModifiedBy; | ||
| import org.springframework.data.annotation.LastModifiedDate; |
There was a problem hiding this comment.
Remove Spring Data auditing annotations from VO class.
Spring Data auditing annotations (@CreatedBy, @LastModifiedBy, @CreatedDate, @LastModifiedDate) are meant for entity classes, not VOs. These annotations won't function in a VO and create confusion about the class's purpose.
-import org.springframework.data.annotation.CreatedBy;
-import org.springframework.data.annotation.CreatedDate;
-import org.springframework.data.annotation.LastModifiedBy;
-import org.springframework.data.annotation.LastModifiedDate;
@Schema(title = "The creator of this record", example = "tom",
accessMode = READ_ONLY)
-@CreatedBy
private String creator;
@Schema(title = "This record was last modified by", example = "tom", accessMode = READ_ONLY)
-@LastModifiedBy
private String modifier;
@Schema(title = "Record creation time (millisecond timestamp)",
example = "1612198922000", accessMode = READ_ONLY)
-@CreatedDate
private LocalDateTime gmtCreate;
@Schema(title = "Record the latest modification time (timestamp in milliseconds)",
example = "1612198444000", accessMode = READ_ONLY)
-@LastModifiedDate
private LocalDateTime gmtUpdate;Also applies to: 226-241
🤖 Prompt for AI Agents
In
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/entity/dto/vo/NoticeReceiverVO.java
at lines 36-39 and 226-241, remove the Spring Data auditing annotations
@CreatedBy, @LastModifiedBy, @CreatedDate, and @LastModifiedDate from the VO
class. These annotations are intended for entity classes and do not function
properly in VO classes, so deleting them will clarify the class's purpose and
avoid confusion.
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| @Builder | ||
| public class NoticeReceiverVO { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider the class design and naming convention.
The class appears to be a direct copy of an entity class with serializers added. Consider whether this is truly a VO/DTO or if you should:
- Create a proper mapping between entity and VO
- Keep only the fields needed for the specific view/transfer purpose
- Consider renaming to
NoticeReceiverDTOto better reflect its purpose
🤖 Prompt for AI Agents
In
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/entity/dto/vo/NoticeReceiverVO.java
at line 53, the class is currently named NoticeReceiverVO but appears to be a
direct copy of an entity class with serializers added. To fix this, create a
proper mapping between the entity and this class, retain only the fields
necessary for the view or transfer purpose, and consider renaming the class to
NoticeReceiverDTO to better reflect its role as a data transfer object.
| int index = StrUtil.indexOf(email, '@'); | ||
| emailDesensitization = index <= 1 ? email : | ||
| StrUtil.replace(email, 1, index, '*'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve email desensitization logic and use Hutool's built-in method.
The current email desensitization logic has several issues:
- No validation that
@symbol exists in the email - Doesn't handle edge cases where
@is at position 0 or 1 - Hutool already provides
DesensitizedUtil.email()method
-int index = StrUtil.indexOf(email, '@');
-emailDesensitization = index <= 1 ? email :
- StrUtil.replace(email, 1, index, '*');
+emailDesensitization = DesensitizedUtil.email(email);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| int index = StrUtil.indexOf(email, '@'); | |
| emailDesensitization = index <= 1 ? email : | |
| StrUtil.replace(email, 1, index, '*'); | |
| - int index = StrUtil.indexOf(email, '@'); | |
| - emailDesensitization = index <= 1 ? email : | |
| - StrUtil.replace(email, 1, index, '*'); | |
| + emailDesensitization = DesensitizedUtil.email(email); |
🤖 Prompt for AI Agents
In
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/serialize/EmailDesensitizationSerializer.java
around lines 41 to 43, the current email desensitization logic lacks validation
for the presence of '@' and does not handle edge cases where '@' is at position
0 or 1. Replace this custom logic with Hutool's built-in
DesensitizedUtil.email() method, which properly validates and desensitizes email
addresses, ensuring robust handling of all edge cases.
| NoticeReceiverVO currentValue = (NoticeReceiverVO)jsonGenerator.getOutputContext().getCurrentValue(); | ||
| desensitizationMapCache.put(currentValue.getId()+"_"+phoneDesensitization, phone); |
There was a problem hiding this comment.
Add null safety checks and improve reusability.
The code has several issues:
- No null check for
currentValue.getId()before string concatenation - Hard-coded cast to
NoticeReceiverVOmakes the serializer non-reusable - Missing error handling for cache operations
-NoticeReceiverVO currentValue = (NoticeReceiverVO)jsonGenerator.getOutputContext().getCurrentValue();
-desensitizationMapCache.put(currentValue.getId()+"_"+phoneDesensitization, phone);
+Object currentObj = jsonGenerator.getOutputContext().getCurrentValue();
+if (currentObj instanceof NoticeReceiverVO) {
+ NoticeReceiverVO currentValue = (NoticeReceiverVO) currentObj;
+ if (currentValue.getId() != null) {
+ try {
+ desensitizationMapCache.put(currentValue.getId() + "_" + phoneDesensitization, phone);
+ } catch (Exception e) {
+ // Log error but don't fail serialization
+ // Consider using a logger here
+ }
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| NoticeReceiverVO currentValue = (NoticeReceiverVO)jsonGenerator.getOutputContext().getCurrentValue(); | |
| desensitizationMapCache.put(currentValue.getId()+"_"+phoneDesensitization, phone); | |
| Object currentObj = jsonGenerator.getOutputContext().getCurrentValue(); | |
| if (currentObj instanceof NoticeReceiverVO) { | |
| NoticeReceiverVO currentValue = (NoticeReceiverVO) currentObj; | |
| if (currentValue.getId() != null) { | |
| try { | |
| desensitizationMapCache.put( | |
| currentValue.getId() + "_" + phoneDesensitization, | |
| phone | |
| ); | |
| } catch (Exception e) { | |
| // Log error but don't fail serialization | |
| // Consider using a logger here | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/serialize/PhoneDesensitizationSerializer.java
around lines 43 to 44, add a null check for currentValue and
currentValue.getId() before concatenating to avoid NullPointerException.
Refactor the code to avoid hard-coded casting to NoticeReceiverVO by making the
serializer generic or accepting a more flexible type. Also, add error handling
around the desensitizationMapCache.put operation to catch and log any exceptions
during cache updates.
| String jsonString = objectMapper.writeValueAsString(noticeReceiver); | ||
| NoticeReceiverVO noticeReceiverVO = objectMapper.readValue(jsonString, NoticeReceiverVO.class); | ||
|
|
||
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | ||
|
|
||
| assertEquals(noticeReceiver.getEmail(),desensitizationMapCache.get(noticeReceiverVO.getId()+"_"+noticeReceiverVO.getEmail())); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test to verify actual desensitization.
The test only verifies caching but doesn't confirm that the email was actually desensitized in the JSON output. Consider adding assertions to verify the desensitization behavior.
String jsonString = objectMapper.writeValueAsString(noticeReceiver);
+ // Verify that the JSON contains desensitized email, not the original
+ assertFalse(jsonString.contains("[email protected]"), "Original email should be desensitized in JSON");
+
NoticeReceiverVO noticeReceiverVO = objectMapper.readValue(jsonString, NoticeReceiverVO.class);
+
+ // Verify that deserialized object has desensitized email
+ assertNotEquals("[email protected]", noticeReceiverVO.getEmail(), "Email should be desensitized after deserialization");
CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache();
+ // Verify original email is cached for restoration
assertEquals(noticeReceiver.getEmail(),desensitizationMapCache.get(noticeReceiverVO.getId()+"_"+noticeReceiverVO.getEmail()));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String jsonString = objectMapper.writeValueAsString(noticeReceiver); | |
| NoticeReceiverVO noticeReceiverVO = objectMapper.readValue(jsonString, NoticeReceiverVO.class); | |
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | |
| assertEquals(noticeReceiver.getEmail(),desensitizationMapCache.get(noticeReceiverVO.getId()+"_"+noticeReceiverVO.getEmail())); | |
| } | |
| String jsonString = objectMapper.writeValueAsString(noticeReceiver); | |
| // Verify that the JSON contains desensitized email, not the original | |
| assertFalse(jsonString.contains("[email protected]"), "Original email should be desensitized in JSON"); | |
| NoticeReceiverVO noticeReceiverVO = objectMapper.readValue(jsonString, NoticeReceiverVO.class); | |
| // Verify that deserialized object has desensitized email | |
| assertNotEquals("[email protected]", noticeReceiverVO.getEmail(), "Email should be desensitized after deserialization"); | |
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | |
| // Verify original email is cached for restoration | |
| assertEquals(noticeReceiver.getEmail(),desensitizationMapCache.get(noticeReceiverVO.getId()+"_"+noticeReceiverVO.getEmail())); | |
| } |
🤖 Prompt for AI Agents
In
hertzbeat-common/src/test/java/org/apache/hertzbeat/common/serialize/EmailDesensitizationSerializerTest.java
around lines 35 to 41, the test currently only checks if the email is cached but
does not verify if the email was actually desensitized in the JSON output. To
fix this, add assertions that inspect the serialized JSON string to confirm the
email field is masked or altered according to the desensitization rules,
ensuring the desensitization logic is properly tested.
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | ||
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){ | ||
| noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail())); | ||
| } | ||
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){ | ||
| noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone())); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicate cache restoration logic into a helper method.
The same cache restoration logic is duplicated in editReceiver and sendTestMsg methods. This violates the DRY principle and makes maintenance harder.
Extract the logic into a private helper method:
+private void restoreOriginalContactInfo(NoticeReceiver noticeReceiver) {
+ if (noticeReceiver.getId() == null) {
+ return;
+ }
+
+ CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache();
+
+ // Restore original email
+ if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail())) {
+ String cacheKey = noticeReceiver.getId() + "_" + noticeReceiver.getEmail();
+ Object originalEmail = desensitizationMapCache.get(cacheKey);
+ if (originalEmail instanceof String) {
+ noticeReceiver.setEmail((String) originalEmail);
+ }
+ }
+
+ // Restore original phone
+ if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone())) {
+ String cacheKey = noticeReceiver.getId() + "_" + noticeReceiver.getPhone();
+ Object originalPhone = desensitizationMapCache.get(cacheKey);
+ if (originalPhone instanceof String) {
+ noticeReceiver.setPhone((String) originalPhone);
+ }
+ }
+}
@Override
public void editReceiver(NoticeReceiver noticeReceiver) {
- CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache();
- if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){
- noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()));
- }
- if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){
- noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()));
- }
+ restoreOriginalContactInfo(noticeReceiver);
noticeReceiverDao.save(noticeReceiver);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){ | |
| noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail())); | |
| } | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){ | |
| noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone())); | |
| } | |
| // --- new helper at class level --- | |
| private void restoreOriginalContactInfo(NoticeReceiver noticeReceiver) { | |
| if (noticeReceiver.getId() == null) { | |
| return; | |
| } | |
| CommonCacheService<String, Object> desensitizationMapCache = | |
| CacheFactory.getDesensitizationMapCache(); | |
| // Restore original email | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail())) { | |
| String cacheKey = noticeReceiver.getId() + "_" + noticeReceiver.getEmail(); | |
| Object originalEmail = desensitizationMapCache.get(cacheKey); | |
| if (originalEmail instanceof String) { | |
| noticeReceiver.setEmail((String) originalEmail); | |
| } | |
| } | |
| // Restore original phone | |
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone())) { | |
| String cacheKey = noticeReceiver.getId() + "_" + noticeReceiver.getPhone(); | |
| Object originalPhone = desensitizationMapCache.get(cacheKey); | |
| if (originalPhone instanceof String) { | |
| noticeReceiver.setPhone((String) originalPhone); | |
| } | |
| } | |
| } | |
| @Override | |
| public void editReceiver(NoticeReceiver noticeReceiver) { | |
| - CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | |
| - if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){ | |
| - noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail())); | |
| - } | |
| - if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){ | |
| - noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone())); | |
| - } | |
| + restoreOriginalContactInfo(noticeReceiver); | |
| noticeReceiverDao.save(noticeReceiver); | |
| } |
🤖 Prompt for AI Agents
In
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/NoticeConfigServiceImpl.java
around lines 136 to 142, the cache restoration logic for email and phone in
noticeReceiver is duplicated in multiple methods. Extract this repeated logic
into a private helper method that takes a noticeReceiver object, checks and
restores email and phone from the cache, and then call this helper method from
the original locations to eliminate duplication and improve maintainability.
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){ | ||
| noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail())); |
There was a problem hiding this comment.
Add type safety and error handling for cache operations.
The current implementation has several safety issues:
- No type checking before casting cached values to String
- No error handling for cache operations
- Multiple cache lookups for the same key
The suggested helper method above addresses these issues by:
- Adding
instanceof Stringchecks before casting - Eliminating duplicate cache lookups
- Consolidating error-prone logic in one place
🤖 Prompt for AI Agents
In
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/NoticeConfigServiceImpl.java
around lines 137-138, the code performs multiple cache lookups for the same key
without type checking or error handling, and casts cached values directly to
String. Refactor by creating a helper method that retrieves the cached value
once, checks if it is an instance of String before casting, and handles any
potential exceptions during cache access. Replace the existing code with a call
to this helper method to improve type safety and reduce redundant cache lookups.
| if (null != noticeReceiver.getId()){ | ||
| CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache(); | ||
| if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){ | ||
| noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail())); | ||
| } | ||
| if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){ | ||
| noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone())); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same refactoring to sendTestMsg method.
Use the same helper method here to eliminate code duplication:
@Override
public boolean sendTestMsg(NoticeReceiver noticeReceiver) {
- if (null != noticeReceiver.getId()){
- CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache();
- if (ObjectUtil.isNotEmpty(noticeReceiver.getEmail()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()))){
- noticeReceiver.setEmail((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getEmail()));
- }
- if (ObjectUtil.isNotEmpty(noticeReceiver.getPhone()) && ObjectUtil.isNotEmpty(desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()))){
- noticeReceiver.setPhone((String) desensitizationMapCache.get(noticeReceiver.getId()+"_"+noticeReceiver.getPhone()));
- }
- }
+ restoreOriginalContactInfo(noticeReceiver);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/NoticeConfigServiceImpl.java
around lines 279 to 287, the code for desensitizing noticeReceiver's email and
phone is duplicated. Refactor by extracting this logic into a helper method and
then call this helper method from the sendTestMsg method as well to eliminate
duplication and improve maintainability.
| @@ -163,6 +163,7 @@ | |||
| <huawei.sdk.version>3.1.37</huawei.sdk.version> | |||
| <huawei.obs.version>3.23.5</huawei.obs.version> | |||
| <commons-net>3.8.0</commons-net> | |||
| <hutool-all.version>5.8.20</hutool-all.version> | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Hutool version is current and secure.
The property definition follows correct naming conventions. However, please verify that version 5.8.20 is the latest stable version and free from known vulnerabilities.
🌐 Web query:
What is the latest stable version of Hutool (cn.hutool:hutool-all) and are there any known security vulnerabilities in version 5.8.20?
💡 Result:
The latest stable version of Hutool (cn.hutool:hutool-all) as of July 2025 is 5.8.38[4][1].
Version 5.8.20 is known to have security vulnerabilities. Specifically, it is marked with 4 vulnerabilities related to core components[1][6]. These vulnerabilities are:
- CVE-2023-51075: Infinite loop in
StrSplitter.splitByRegex, allowing attackers to cause a denial of service. - CVE-2023-42278: Buffer overflow in the
JSONUtil.parse()function. - CVE-2023-42277 & CVE-2023-42276: Other unspecified vulnerabilities related to
hutool-core[6][5].
These vulnerabilities are considered high severity, as they enable potential denial-of-service (DoS) attacks or, in the case of buffer overflows, may lead to memory corruption or system crashes[3].
It is recommended to upgrade to the latest stable version (at least 5.8.38 as of June 2025) to ensure all known security issues are patched[4][1].
Citations:
- 1: https://mvnrepository.com/artifact/cn.hutool/hutool-core
- 2: https://vulert.com/vuln-db/CVE-2022-45688
- 3: https://stack.watch/product/hutool/
- 4: https://mvnrepository.com/artifact/cn.hutool/hutool-all
- 5: https://security.snyk.io/package/maven/cn.hutool:hutool-core/5.8.20
- 6: https://mvnrepository.com/artifact/cn.hutool/hutool-core/5.8.20
- 7: https://github.com/chinabugotech/hutool
- 8: https://www.acunetix.com/vulnerabilities/sca/cve-2023-24162-vulnerability-in-maven-package-cn-hutool-hutool-all/
- 9: https://nvd.nist.gov/vuln/search/results?adv_search=true&isCpeNameSearch=true&query=cpe%3A2.3%3Aa%3Ahutool%3Ahutool%3A4.6.3%3A%2A%3A%2A%3A%2A%3A%2A%3A%2A%3A%2A%3A%2A
- 10: https://doc.hutool.cn/pages/index/
Upgrade Hutool to a secure, up-to-date version
Version 5.8.20 is known to contain multiple high-severity CVEs (e.g., CVE-2023-51075, CVE-2023-42278) and is not the latest release. Please bump the property in your root pom.xml to at least 5.8.38 to ensure all known vulnerabilities are patched:
• pom.xml (root), around line 166
- <hutool-all.version>5.8.20</hutool-all.version>
+ <hutool-all.version>5.8.38</hutool-all.version>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <hutool-all.version>5.8.20</hutool-all.version> | |
| <hutool-all.version>5.8.38</hutool-all.version> |
🤖 Prompt for AI Agents
In pom.xml at line 166, the hutool-all.version property is set to 5.8.20, which
has known high-severity vulnerabilities. Update this version to at least 5.8.38
to ensure all security issues are addressed by changing the version number in
the property to 5.8.38 or a later secure release.
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Solid Implementation - Let's Address Data Desensitization Security & Reliability!
|
![refacto-test[bot]](https://avatars.githubusercontent.com/u/196302493?s=60&v=4){kind=small}
| @@ -57,4 +60,10 @@ public static CommonCacheService<String, Object> getAlertSilenceCache() { | |||
| public static CommonCacheService<String, Object> getAlertConvergeCache() { | |||
There was a problem hiding this comment.
Sensitive Data Exposure through Unprotected Cache
The implementation stores original sensitive data (emails and phone numbers) in a cache that lacks access controls. While the data is desensitized in API responses, there's no mechanism to ensure that only authorized code can retrieve the original values from the cache. The cache has a long expiration time (1 day) and doesn't implement any authentication or authorization checks for data access.
| public static CommonCacheService<String, Object> getAlertConvergeCache() { | |
| private static final CommonCacheService<String, Object> DESENSITIZATION_MAP_CACHE = | |
| new CaffeineCacheServiceImpl<>(10, 1000, Duration.ofMinutes(30), false); | |
| /** | |
| * Get desensitizationMap cache with appropriate access control checks | |
| * @param accessContext The context of the access request for authorization | |
| * @return desensitizationMap cache if authorized | |
| */ |
Rationale
- Reducing the cache expiration time from 1 day to 30 minutes minimizes the window of exposure if the cache is compromised
- Adding documentation that reminds developers to implement proper access control when retrieving sensitive data
- In a complete implementation, the method should verify the caller has appropriate permissions before returning the original sensitive data
References
- CWE: CWE-922
- OWASP: A02:2021-Cryptographic Failures
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
What's changed?
Checklist
Add or update API
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores