이 문서를 통해 아래 질문들에 답할 수 있게 됩니다.
- 코드 리뷰에서 보안 질문을 어떤 순서로 던져야 변경 diff의 위험을 찾을 수 있는가?
- 리뷰어는 요청값, 권한, 데이터 접근, 오류, 로그, 운영 설정을 어떻게 따라가야 하는가?
- 리뷰 코멘트를 테스트와 체크리스트 업데이트로 남기는 기준은 무엇인가?
개요
보안 코드 리뷰는 취향을 고르는 시간이 아니다. 변경 diff가 새로운 신뢰 경계를 만들었는지, 기존 권한 정책을 우회했는지, 민감값을 노출했는지, 실패를 관측 가능하게 남기는지 확인하는 일이다.
리뷰 질문은 “이 코드가 깔끔한가”보다 “공격자가 어떤 값을 바꿀 수 있는가”에서 시작해야 한다. 요청값의 출처를 Controller에서 보고, 권한 결정을 Service와 policy에서 확인하고, Repository 조건과 로그 이벤트까지 따라간다.
리뷰 결과는 코멘트로 끝나면 다음 PR에서 사라진다. 중요한 발견은 테스트 케이스, 보안 체크리스트 항목, lint rule, API 설계 템플릿으로 재사용되어야 한다.
이 문서의 핵심은 리뷰어가 머릿속으로 공격 요청을 실행하며 diff를 읽는 것이다.
공격 시나리오
- 공격자는 새 request field가 DTO에 추가된 순간을 노린다.
- 리뷰어가 Controller annotation만 보고 Service의 권한 우회를 놓친다.
- Repository query에서 tenant/member 조건이 빠진 것을 단위 테스트가 잡지 못한다.
- error handler 변경으로 내부 exception message가 외부 응답에 섞인다.
- 로그 개선 PR이 token이나 request body 원문을 남긴다.
- gateway route 변경으로 인증이 필요한 endpoint가 public으로 열린다.
취약한 요청/응답 예시
+ public record UserPatchRequest(String nickname, String role, boolean emailVerified) {}
+ @PatchMapping("/api/users/me")
+ public UserResponse update(@RequestBody UserPatchRequest request) {
+ return UserResponse.from(userService.update(request));
+ }review=approved
question=none
security-test=nonerole과 emailVerified가 사용자 self-service API에 추가되었는데 질문이 없다면 리뷰가 공격 요청을 상상하지 못한 것이다.
주제별 핵심 판단
- 요청값의 출처와 신뢰 수준을 먼저 묻는다.
- 인증과 인가를 분리해서 본다.
- 객체 소유권과 기능 권한을 모두 본다.
- DTO와 entity 경계를 확인한다.
- Repository 조건이 권한 정책과 일치하는지 본다.
- 오류와 로그가 민감값을 노출하지 않는지 본다.
- 운영 설정과 배포 경로가 코드 의도를 깨지 않는지 본다.
개선된 요청/응답 예시
review-question:
- Can a member set role or emailVerified through this request?
- Where is ownership checked?
- Which test proves non-owner access fails?
- Which audit event is emitted on rejection?
required-change:
- Remove role/emailVerified from UserPatchRequest.
- Add UserSecurityTest.unexpectedRoleFieldRejected.
- Add audit event user_profile_update_rejected.좋은 리뷰는 “위험해 보입니다”가 아니라 어떤 필드를 제거하고 어떤 테스트와 로그를 추가할지까지 연결한다.
방어 설계
- diff를 endpoint, DTO, service, repository, configuration, logging 순서로 읽는다.
- 새 public endpoint는 API inventory와 보안 체크리스트 링크가 있는지 확인한다.
- 새 request field는 누가 쓸 수 있고 어디서 검증되는지 묻는다.
- 새 DB query는 tenant/member/resource 조건을 확인한다.
- 새 권한 정책은 실패 테스트와 성공 테스트를 함께 요구한다.
- 새 로그는 민감값 마스킹과 event schema를 확인한다.
- 새 외부 호출은 SSRF, timeout, retry, secret, response trust boundary를 확인한다.
- 보안 우려가 반복되면 lint, template, test fixture로 자동화한다.
Spring/HTTP 예시
// Review focus: request DTO must not include privilege fields.
public record UserPatchRequest(
@Size(max = 30) String nickname
) {}// Review focus: repository query includes owner boundary.
Optional<Order> findByIdAndMemberId(long orderId, MemberId memberId);// Review focus: method security delegates to testable policy.
@PreAuthorize("@orderPolicy.canRead(authentication, #orderId)")
@GetMapping("/api/orders/{orderId}")
public OrderResponse get(@PathVariable long orderId) {
return OrderResponse.from(orderService.get(orderId));
}리뷰 질문은 코드 스타일보다 경계 유지 여부에 집중한다. policy는 테스트 가능한 형태로 분리되어야 한다.
운영 로그와 감사 지점
level=INFO event=security_review_completed result=allowed pr=842 reviewer=backend:kim focus=authz,dto,logging evidence=OrderSecurityTest.nonOwnerCannotRead traceId=291ca811level=WARN event=security_review_blocker result=blocked pr=842 reason=missing_repository_owner_condition file=OrderRepository.java owner=team-order traceId=291ca811- 보안 blocker는 PR label이나 required check로 남긴다.
- risk acceptance는 reviewer comment만으로 끝내지 않는다.
- 반복 리뷰 지적은 checklist와 test template에 반영한다.
- 리뷰 완료 기록에는 검토 범위와 증거를 남긴다.
- 리뷰에서 발견된 취약 패턴은 검색 가능한 rule로 바꾼다.
실전 판단 기준
- 새 필드가 추가되면 공격자가 그 필드를 보낼 수 있다고 가정한다.
- Controller annotation만 보고 권한 검토를 끝내면 부족하다.
- Repository 조건을 보지 않는 리뷰는 IDOR을 놓치기 쉽다.
- 로그 개선 PR은 정보 노출 PR이 될 수 있다.
- “내부 API”라는 말은 인증과 권한 검사를 생략할 이유가 아니다.
- 리뷰 코멘트가 테스트로 남지 않으면 회귀가 반복된다.
테스트 포인트
- 새 endpoint가 인증 없음과 권한 없음 테스트를 갖는지 확인한다.
- 새 request field에 예상 밖 값 테스트가 있는지 확인한다.
- 새 resource access에 non-owner 테스트가 있는지 확인한다.
- 새 query에 tenant/member 조건이 있는지 확인한다.
- 새 error handling에 정보 노출 테스트가 있는지 확인한다.
- 새 log에 token/password/secret 원문이 없는지 확인한다.
- 새 설정 변경이 배포 전 체크리스트에 반영되는지 확인한다.
위험 신호!
- 리뷰에서 “보안은 나중에 테스트”라고 넘어간다.
- request DTO에 권한/상태/소유자 필드가 들어간다.
findById만 보고 resource를 반환한다.- 실패 테스트 없이 권한 annotation만 추가한다.
- 로그에 request body 전체가 추가된다.
- risk acceptance에 owner와 만료일이 없다.
확인 질문
확인 질문
- 보안 코드 리뷰의 첫 질문은 무엇인가?
- 이 값은 누가 만들었고 어디서 검증되는가다.
- 리뷰어가 Repository까지 봐야 하는 이유는 무엇인가?
- 객체 소유권은 최종 데이터 조회 조건에서 빠질 때 IDOR로 이어지기 때문이다.
- 좋은 리뷰 코멘트의 후속 조치는 무엇인가?
- 테스트, 체크리스트, lint rule, 설계 템플릿 중 재사용 가능한 증거로 남기는 것이다.