[Fix] 검증 에러 응답의 민감 정보 마스킹#93
Merged
Merged
Conversation
비밀번호 등 민감 필드가 validation 실패 응답과 로그에 원문 그대로 포함되지 않도록 수정했습니다. ExceptionAdvice에서 rejected value 포맷팅 시 민감 필드 여부를 검사하도록 변경 password, token, secret, authorization, credential 키워드가 포함된 필드는 ****로 마스킹 일반 필드의 입력값 노출 방식은 기존 동작 유지 민감 필드 마스킹 여부를 검증하는 단위 테스트 추가
📝 WalkthroughWalkthroughExceptionAdvice adds MASKED_VALUE and a SENSITIVE_FIELD_KEYWORDS set plus isSensitiveField/formatRejectedValue helpers, and applies masking to rejected values in MethodArgumentNotValidException and ConstraintViolationException formatting; tests verify masking, case-insensitivity, non-sensitive passthrough, and null handling. ChangesSensitive Field Masking in Exception Handling
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/jobdri/jobdri_api/global/apiPayload/exception/handler/ExceptionAdviceTest.java (1)
10-19: ⚡ Quick winConsider expanding test coverage for all sensitive keywords.
The test validates
passwordandtokenkeywords, but doesn't coversecret,authorization, andcredential. Additionally, consider testing edge cases like null field names, null values, and case variations (e.g.,"PASSWORD").🧪 Additional test cases to consider
`@Test` `@DisplayName`("모든 민감 키워드를 테스트") void maskAllSensitiveKeywords() { assertThat(ExceptionAdvice.formatRejectedValue("secret", "raw-secret")) .isEqualTo("****"); assertThat(ExceptionAdvice.formatRejectedValue("authorization", "raw-auth")) .isEqualTo("****"); assertThat(ExceptionAdvice.formatRejectedValue("credential", "raw-cred")) .isEqualTo("****"); } `@Test` `@DisplayName`("대소문자 구분 없이 민감 필드를 감지") void maskCaseInsensitiveFields() { assertThat(ExceptionAdvice.formatRejectedValue("PASSWORD", "raw")) .isEqualTo("****"); assertThat(ExceptionAdvice.formatRejectedValue("RefreshToken", "raw")) .isEqualTo("****"); } `@Test` `@DisplayName`("null 필드명과 null 값 처리") void handleNullInputs() { assertThat(ExceptionAdvice.formatRejectedValue(null, "value")) .isEqualTo("value"); assertThat(ExceptionAdvice.formatRejectedValue("email", null)) .isEqualTo("null"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/jobdri/jobdri_api/global/apiPayload/exception/handler/ExceptionAdviceTest.java` around lines 10 - 19, The test covers only "password" and "token" masking; extend ExceptionAdviceTest by adding cases that call ExceptionAdvice.formatRejectedValue to assert masking for additional sensitive keywords ("secret", "authorization", "credential"), verify case-insensitive detection (e.g., "PASSWORD", "RefreshToken"), and add null-handling edge cases: passing null fieldName should return the original value, and passing a null value should return "null" (or whatever ExceptionAdvice.formatRejectedValue currently returns) to ensure consistent behavior; update or add test methods (e.g., maskAllSensitiveKeywords, maskCaseInsensitiveFields, handleNullInputs) to include these assertions against ExceptionAdvice.formatRejectedValue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/test/java/com/jobdri/jobdri_api/global/apiPayload/exception/handler/ExceptionAdviceTest.java`:
- Around line 10-19: The test covers only "password" and "token" masking; extend
ExceptionAdviceTest by adding cases that call
ExceptionAdvice.formatRejectedValue to assert masking for additional sensitive
keywords ("secret", "authorization", "credential"), verify case-insensitive
detection (e.g., "PASSWORD", "RefreshToken"), and add null-handling edge cases:
passing null fieldName should return the original value, and passing a null
value should return "null" (or whatever ExceptionAdvice.formatRejectedValue
currently returns) to ensure consistent behavior; update or add test methods
(e.g., maskAllSensitiveKeywords, maskCaseInsensitiveFields, handleNullInputs) to
include these assertions against ExceptionAdvice.formatRejectedValue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 323df260-632c-40a9-8403-363bf87fc0b5
📒 Files selected for processing (2)
src/main/java/com/jobdri/jobdri_api/global/apiPayload/exception/handler/ExceptionAdvice.javasrc/test/java/com/jobdri/jobdri_api/global/apiPayload/exception/handler/ExceptionAdviceTest.java
검증 실패 응답에서 민감 필드 입력값이 노출되지 않도록 하는 마스킹 로직의 테스트 범위를 확장했습니다. secret, authorization, credential 키워드 마스킹 검증 추가 PASSWORD, RefreshToken 등 대소문자 혼합 필드명 검증 추가 필드명이 null인 경우 기존 입력값을 유지하는지 검증 입력값이 null인 경우 현재 포맷팅 결과를 명시적으로 검증 민감 필드 입력값이 null이어도 마스킹되는지 검증
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/jobdri/jobdri_api/global/apiPayload/exception/handler/ExceptionAdviceTest.java (1)
10-25: ⚡ Quick winConsider parameterized tests to reduce duplication.
The six assertions follow the same pattern. A parameterized test could improve maintainability:
♻️ Refactor to parameterized test
+import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; + +import java.util.stream.Stream; + + static class SensitiveFieldProvider implements ArgumentsProvider { + `@Override` + public Stream<? extends Arguments> provideArguments(ExtensionContext context) { + return Stream.of( + Arguments.of("password"), + Arguments.of("loginRequest.password"), + Arguments.of("refreshToken"), + Arguments.of("clientSecret"), + Arguments.of("authorizationHeader"), + Arguments.of("credentialKey") + ); + } + } + - `@Test` - `@DisplayName`("민감한 검증 실패 입력값은 키워드별로 마스킹한다") - void maskAllSensitiveKeywords() { - assertThat(ExceptionAdvice.formatRejectedValue("password", "raw-password")) - .isEqualTo("****"); - assertThat(ExceptionAdvice.formatRejectedValue("loginRequest.password", "raw-password")) - .isEqualTo("****"); - assertThat(ExceptionAdvice.formatRejectedValue("refreshToken", "raw-token")) - .isEqualTo("****"); - assertThat(ExceptionAdvice.formatRejectedValue("clientSecret", "raw-secret")) - .isEqualTo("****"); - assertThat(ExceptionAdvice.formatRejectedValue("authorizationHeader", "raw-authorization")) - .isEqualTo("****"); - assertThat(ExceptionAdvice.formatRejectedValue("credentialKey", "raw-credential")) - .isEqualTo("****"); - } + `@ParameterizedTest` + `@ArgumentsSource`(SensitiveFieldProvider.class) + `@DisplayName`("민감한 검증 실패 입력값은 키워드별로 마스킹한다") + void maskAllSensitiveKeywords(String fieldName) { + assertThat(ExceptionAdvice.formatRejectedValue(fieldName, "raw-value")) + .isEqualTo("****"); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/jobdri/jobdri_api/global/apiPayload/exception/handler/ExceptionAdviceTest.java` around lines 10 - 25, Replace the duplicated assertions in maskAllSensitiveKeywords() with a parameterized test: change the test method to a `@ParameterizedTest` (e.g., `@ParameterizedTest` with `@CsvSource` or `@MethodSource`) that supplies pairs of field names and expected masked values and call ExceptionAdvice.formatRejectedValue for each pair; reference the existing test name maskAllSensitiveKeywords and the static method ExceptionAdvice.formatRejectedValue when updating the test so each input/expected pair (e.g., "password","****", "loginRequest.password","****", etc.) is asserted in a single parameterized test method to remove duplication and improve maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/test/java/com/jobdri/jobdri_api/global/apiPayload/exception/handler/ExceptionAdviceTest.java`:
- Around line 10-25: Replace the duplicated assertions in
maskAllSensitiveKeywords() with a parameterized test: change the test method to
a `@ParameterizedTest` (e.g., `@ParameterizedTest` with `@CsvSource` or `@MethodSource`)
that supplies pairs of field names and expected masked values and call
ExceptionAdvice.formatRejectedValue for each pair; reference the existing test
name maskAllSensitiveKeywords and the static method
ExceptionAdvice.formatRejectedValue when updating the test so each
input/expected pair (e.g., "password","****", "loginRequest.password","****",
etc.) is asserted in a single parameterized test method to remove duplication
and improve maintainability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7300ca68-d6a2-4817-8dc7-1af5b16ce31d
📒 Files selected for processing (1)
src/test/java/com/jobdri/jobdri_api/global/apiPayload/exception/handler/ExceptionAdviceTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
비밀번호 등 민감 필드가 validation 실패 응답과 로그에 원문 그대로 포함되지 않도록 수정했습니다.
ExceptionAdvice에서 rejected value 포맷팅 시 민감 필드 여부를 검사하도록 변경 password, token, secret, authorization, credential 키워드가 포함된 필드는 ****로 마스킹 일반 필드의 입력값 노출 방식은 기존 동작 유지
민감 필드 마스킹 여부를 검증하는 단위 테스트 추가
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [#7 ]
Summary by CodeRabbit
Bug Fixes
Tests