refactor/#132 확장자 검증 로직 추가 및 파일 업로드 관련 수정 #147
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이 풀 리퀘스트는 파일 처리 및 보안 관련 코드의 주요 리팩토링을 포함하고 있습니다. 주요 변경 사항은 파일 경로 암호화 로직 제거, 파일 크기 표현 방식 변경, 파일 확장자 검증 메커니즘 추가 등입니다. 보안 설정과 파일 저장소 서비스에 대한 검증 로직이 개선되었으며, 파일 관련 예외 처리 메커니즘도 강화되었습니다. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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.
Copilot reviewed 5 out of 10 changed files in this pull request and generated 2 comments.
Files not reviewed (5)
- aics-api/src/main/resources/application.yml: Evaluated as low risk
- aics-common/src/main/java/kgu/developers/common/config/SecurityConfig.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/file/domain/FileEntity.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/file/exception/FileDomainExceptionCode.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/file/exception/NotSupportedFileExtensionException.java: Evaluated as low risk
aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileStorageServiceImpl.java
Show resolved
Hide resolved
aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileStorageServiceImpl.java
Show resolved
Hide resolved
Test Coverage Report
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #147 +/- ##
==========================================
Coverage ? 83.95%
Complexity ? 79
==========================================
Files ? 21
Lines ? 293
Branches ? 4
==========================================
Hits ? 246
Misses ? 41
Partials ? 6 Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
aics-api/src/main/resources/application.yml (1)
보안: FILE_SECRET_KEY 환경 변수의 기본값 설정 및 검증 필요
application.yml에서file.secret-key가${FILE_SECRET_KEY}환경 변수를 참조하고 있으나, 환경 변수가 설정되지 않은 경우의 기본값이 정의되어 있지 않습니다. 이는 다음과 같은 보안 위험을 초래할 수 있습니다:
- 환경 변수가 설정되지 않은 경우 애플리케이션이 시작되지 않을 수 있습니다.
- 기본값이 없어 파일 암호화 키가 누락될 경우 보안 취약점이 발생할 수 있습니다.
수정 제안:
application.yml의file.secret-key설정에 기본값을 추가하세요:${FILE_SECRET_KEY:your-default-secure-key}- 프로덕션 환경에서는 반드시 환경 변수를 통해 안전한 키 값을 제공하세요.
- 키의 최소 길이와 복잡성 요구사항을 문서화하고 검증 로직을 추가하세요.
🔗 Analysis chain
Line range hint
31-31: 보안: 파일 암호화 키의 환경 변수 검증 필요
FILE_SECRET_KEY환경 변수가 설정되지 않은 경우의 기본값이 정의되어 있지 않습니다.다음 스크립트로 환경 변수 설정을 확인해주세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the presence and minimum length of FILE_SECRET_KEY in environment files # Check for environment files for env_file in .env .env.* docker-compose*.yml; do if [ -f "$env_file" ]; then echo "Checking $env_file for FILE_SECRET_KEY..." grep -l "FILE_SECRET_KEY" "$env_file" fi doneLength of output: 224
Script:
#!/bin/bash # Check application.yml files for FILE_SECRET_KEY configuration fd "application.yml" --exec grep -l "FILE_SECRET_KEY" {} \; # Show the context around FILE_SECRET_KEY in application.yml files fd "application.yml" --exec grep -A 5 -B 5 "FILE_SECRET_KEY" {} \; # Check for any default value configurations related to FILE_SECRET_KEY ast-grep --pattern 'FILE_SECRET_KEY: ${FILE_SECRET_KEY:$_}'Length of output: 552
🧹 Nitpick comments (3)
aics-domain/src/main/java/kgu/developers/domain/file/domain/FileEntity.java (2)
48-50: [암호화된 값 빌더 설정 점검]
생성된FileEntity에 암호화된 논리 이름과 물리 경로, 그리고 사람에게 읽기 쉬운 파일 크기가 저장됩니다. 이후 조회 시 복호화가 필요한 부분은 어디인지 확인해주세요.
55-60: [파일 크기 변환 로직 검증]
convertToReadableFileSize메서드는 파일 크기에 따라 Bytes, KB, MB, GB로 적절히 변환하고 있습니다. 단위 변환의 정확도(소수점 이슈 등)를 고려해 테스트 케이스를 추가하는 것을 권장드립니다.aics-api/src/main/resources/application.yml (1)
Line range hint
29-30: 보안: 파일 업로드 경로 설정 개선 필요
${user.dir}을 사용한 상대 경로 설정은 실행 환경에 따라 예기치 않은 위치에 파일이 저장될 수 있습니다. 보안과 안정성을 위해 절대 경로 사용을 권장합니다.다음과 같이 환경 변수를 통해 절대 경로를 설정하는 것을 제안합니다:
- upload-path: ${user.dir}/cloud + upload-path: ${FILE_UPLOAD_PATH:/var/aics/uploads}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
aics-api/src/main/java/kgu/developers/api/file/application/FileService.java(1 hunks)aics-api/src/main/java/kgu/developers/api/file/presentation/response/FilePathResponse.java(2 hunks)aics-api/src/main/java/kgu/developers/api/post/presentation/response/PostDetailResponse.java(1 hunks)aics-api/src/main/resources/application.yml(1 hunks)aics-common/src/main/java/kgu/developers/common/config/SecurityConfig.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/file/domain/FileEntity.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/file/exception/FileDomainExceptionCode.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/file/exception/NotSupportedFileExtensionException.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FilePathProperties.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileStorageServiceImpl.java(4 hunks)
🔇 Additional comments (21)
aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileStorageServiceImpl.java (6)
11-11: 임포트 추가 확인
Set 컬렉션을 사용하기 위한 임포트로 보이며, 문제 없어 보입니다.
25-25: 신규 예외 임포트
NotSupportedFileExtensionException 예외를 불러오는 의존성이 추가되었으며, 불허 확장자 검증에 필요한 적절한 조치로 보입니다.
31-31: disallowedExtensions 필드 정의
불허 확장자 목록을 안전하게 관리하기 위해 private final로 선언된 점이 적절합니다.
38-38: 불허 확장자 프로퍼티 주입
filePathProperties.getDisallowedExtensions() 결과가 null이거나 빈 Set일 경우를 대비해, 추가적인 예외 처리나 기본값 설정을 고려해볼 수 있습니다.
50-50: 새로운 검증 로직 호출
validateAttributes 메서드를 통한 경로·확장자 검증은 구조적으로 명확하며 안전성 향상에 기여합니다.
78-81: 검증 로직의 명확한 분리
validateAttributes에서 경로와 확장자를 각각 따로 검사하는 구성이 가독성을 높이고 유지보수에 용이합니다.
aics-domain/src/main/java/kgu/developers/domain/file/exception/NotSupportedFileExtensionException.java (1)
1-11: 신규 예외 클래스 추가
CustomException을 상속하여 불허 확장자의 상황을 명확히 표현하고, 적절한 예외 코드를 할당하고 있어 유지보수에 용이합니다.
aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FilePathProperties.java (2)
3-3: Set 임포트 추가
불허 확장자 목록 관리를 위해 Set 타입을 사용하는 점이 적절해 보입니다.
17-17: 불허 확장자 프로퍼티 필드
외부 설정을 통해 확장자 목록을 쉽게 변경할 수 있어 유연성이 높습니다.
aics-api/src/main/java/kgu/developers/api/file/presentation/response/FilePathResponse.java (2)
6-7: 파일 엔티티 & AES 유틸 임포트
FileEntity와 AesUtil을 가져와서 직접 복호화하여 응답으로 제공하는 로직에 대비가 잘 되어 있습니다.
18-21: AES 복호화 로직 통합
FileEntity에서 물리 경로를 직접 복호화하여 반환함으로써, 코드가 간결하고 유지보수성이 높아졌습니다.
aics-domain/src/main/java/kgu/developers/domain/file/exception/FileDomainExceptionCode.java (1)
17-17: 새로운 예외 코드 추가
“NOT_SUPPORTED_FILE_EXTENSION”을 명시적으로 정의해, 불허 확장자를 다루는 로직에 대한 가독성과 유지보수성을 향상시킵니다.
aics-api/src/main/java/kgu/developers/api/file/application/FileService.java (2)
22-22: [파일 경로 암호화 로직 제거]
FileEntity.create() 호출 시 물리 경로(physicalPath)를 암호화하지 않고 그대로 저장하도록 수정되었습니다. 보안 요구사항에 부합하는지 다시 한번 확인해주세요.
25-25: [수정된 응답 로직 점검]
암호화가 제거된 storedPath를 기반으로 FilePathResponse.of(savedFile)을 반환하고 있습니다. 다른 로직(예: PostDetailResponse)에서 동일한 파일 경로 접근 방식과 일관성이 있는지 검토해주세요.
aics-domain/src/main/java/kgu/developers/domain/file/domain/FileEntity.java (4)
11-11: [암호화 유틸 도입 확인]
AesUtil 클래스를 사용하기 위해 해당 import가 추가되었습니다. 엔티티의 암호화/복호화 로직 암호 키 관리가 적절히 이루어졌는지 다시 한번 살펴보세요.
34-34: [fileSize 타입 변경]
Long 타입에서 String 타입으로 변경하여 사람에게 읽기 쉬운 문자열 형태로 파일 크기를 표시합니다. 도메인 규칙과 호환되는지 확인해주세요.
39-42: [파일 단위 변환 상수 정의]
KB, MB, GB 상수를 도입하여 파일 크기 변환 로직을 단순화했습니다. 단위 변환 오류나 오탈자가 없는지 다시 점검해주세요.
44-46: [생성 시 암호화 적용 주의]
logicalName과 physicalPath 모두 AesUtil.encrypt로 암호화되어 빌더에 전달됩니다. 파일 접근 시 복호화 과정 누락 가능성 등을 유념해주세요.
aics-api/src/main/java/kgu/developers/api/post/presentation/response/PostDetailResponse.java (1)
80-80: [게시글 첨부파일 복호화 로직 제거]
첨부파일 정보 설정 시 FilePathResponse.of(post.getFile())를 직접 호출하여 복호화 과정을 생략하고 있습니다. FileEntity가 이미 암호화된 정보를 보유하므로, 해당 응답에서 복호화가 필요한지 여부를 재검토해주세요.
aics-common/src/main/java/kgu/developers/common/config/SecurityConfig.java (2)
62-63: [정적 리소스 범위 확대]
STATIC_RESOURCES_PATTERNS에 "/js/**"와 "/cloud/**"가 추가되어, 해당 경로에 있는 리소스에 대한 접근이 허용됩니다. 외부 접근 시 보안 리스크가 없는지 확인해주세요.
77-77: [공개 엔드포인트 확대]
PUBLIC_ENDPOINTS에 "/api/v1/abouts"를 추가하여 인증 없이 접근을 허용합니다. 대상 리소스가 민감한 정보를 다루지 않는지 검토해주세요.
aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileStorageServiceImpl.java
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
aics-api/src/main/java/kgu/developers/api/file/presentation/response/FilePathResponse.java
Outdated
Show resolved
Hide resolved
Summary
파일 업로드 시 허용되지 않는 확장자를 검증하는 로직을 추가하였습니다. 또한 파일의 logicalName을 암호화 저장하고 파일 size를 String으로 변환하여 저장하는 등 몇가지 주요 로직을 변경하였습니다.
Tasks
logicalName필드를 암호화하여 데이터베이스에 저장하도록 변경To Reviewer
허용되지 않는 확장자 검증
기존에는
application.yml에서disallowed-extensions와 함께allowed-extensions도 설정되어 있었습니다. 하지만 허용되지 않는 확장자만 검증하면 충분하다고 판단하여allowed-extensions설정을 제거했습니다. 만약 특정 확장자만 허용할 경우, 서버가 예상하지 못한 안전한 파일조차 사용자가 업로드할 수 없게 되는 상황을 방지하기 위해 그렇게 조처를 취하였습니다.logicalName암호화 저장파일의 원본 이름인
logicalName을 암호화하여 저장합니다. 보안성을 강화하는 동시에, 동일한 파일 이름의 파일이 업로드될 경우 기존 파일이 덮어씌워지는 문제를 방지하기 위함입니다.파일 사이즈 변환 도메인 규칙 추가
파일 사이즈를 그대로 저장하는 대신, KB, MB, GB 등의 사람이 읽기 쉬운 형식으로 변환하여 저장합니다. 이렇게 하여 클라이언트에서 별도로 변환 로직을 처리해야 하는 불편함을 해소하고자 합니다.
파일 및 이미지 리사이징
파일 및 이미지 리사이징 로직은 추후 구현할 예정입니다.