Skip to content

refactor: 관리자 대학 이미지 업로드 경로 추가#760

Merged
lsy1307 merged 4 commits into
developfrom
refactor/759-admin-university-image-upload
Jun 17, 2026
Merged

refactor: 관리자 대학 이미지 업로드 경로 추가#760
lsy1307 merged 4 commits into
developfrom
refactor/759-admin-university-image-upload

Conversation

@lsy1307

@lsy1307 lsy1307 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

관련 이슈

작업 내용

  • 관리자 대학 로고와 배경 이미지 업로드 API를 추가했습니다.
  • 대학 영문명을 서버에서 업로드 디렉토리명으로 변환해 S3 경로에 사용하도록 구성했습니다.
  • 정규화된 대학 영문명에 원본 영문명 해시를 붙여 S3 저장소 경로 식별자 충돌 위험을 줄였습니다.
  • 파견 대학 영문명 중복 검증과 DB unique 제약을 추가했습니다.
  • 업로드 경로별 이미지 전용 검증 정책을 추가해 관리자 대학 이미지 업로드에서 문서 확장자를 차단했습니다.
  • 대학 영문명 디렉토리명 변환, 정규화 충돌, 동적 업로드 경로 생성, 확장자 검증 테스트를 추가했습니다.

특이 사항

  • 업로드 경로는 admin/logo/{대학_영문명_slug}_{hash}admin/background/{대학_영문명_slug}_{hash} 형식입니다.
  • 대학 영문명은 소문자 변환, 공백 _ 치환, 특수문자 제거 규칙을 적용하고 SHA-256 기반 12자리 hex suffix를 추가합니다.
  • 신규 관리자 업로드 API는 관리자 권한 검증을 수행합니다.
  • host_university.english_name에 unique 제약이 추가되므로 기존 운영 데이터에 중복 영문명이 없어야 합니다.
  • GPA, 어학 성적, 채팅, 멘토 증빙 업로드는 문서 확장자를 계속 허용합니다.

리뷰 요구사항 (선택)

  • 대학 영문명 unique 제약 추가가 현재 운영 데이터와 정책에 맞는지 확인 부탁드립니다.
  • 현재 S3 업로드 비동기 처리의 정합성 보완은 별도 작업으로 논의가 필요합니다.

lsy1307 added 2 commits June 17, 2026 01:42
- 관리자 대학 로고와 배경 이미지 업로드 API를 추가

- 대학 영문명을 서버에서 업로드 디렉토리명으로 변환

- S3 업로드 서비스에 동적 하위 디렉토리 경로 생성을 추가
- 대학 영문명 디렉토리명 변환 테스트를 추가

- 동적 하위 디렉토리 기반 S3 업로드 경로 테스트를 추가
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

이번 PR은 크게 두 가지 기능 변경을 담고 있어요.

  1. HostUniversity 영문명 유일성 제약 추가

    • HostUniversity 엔티티의 english_name 컬럼에 unique = true 제약을 추가했습니다.
    • HostUniversityRepositoryfindByEnglishName 메서드를 선언했습니다.
    • Flyway V52 마이그레이션으로 DB에 uk_host_university_english_name UNIQUE 제약을 적용했습니다.
    • AdminHostUniversityService의 생성/수정 흐름에 영문명 중복 검증 헬퍼(validateEnglishNameNotExists, validateEnglishNameNotDuplicated)를 추가했습니다.
  2. 관리자 대학 이미지 S3 업로드 엔드포인트 신설

    • UploadPath enum에 ADMIN_UNIVERSITY_LOGO, ADMIN_UNIVERSITY_BACKGROUND 상수와 imageOnly 플래그를 추가했습니다.
    • UploadDirectoryName 유틸 클래스를 신설하여 대학 영문명을 정규화하고 SHA-256 해시 접미를 붙인 S3 디렉터리명을 생성합니다.
    • S3ServicesubDirectory를 받는 uploadFile 오버로드와 createFileName 헬퍼를 추가했습니다.
    • S3Controller에 관리자 전용(@RequireRoleAccess(Role.ADMIN)) 로고·배경 이미지 업로드 엔드포인트 2개를 추가했습니다.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • wibaek
  • sukangpunch
  • whqtker
  • Hexeong
  • JAEHEE25
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목이 변경사항의 핵심(관리자 대학 이미지 업로드 경로 추가)을 명확하게 요약하고 있습니다.
Description check ✅ Passed PR 설명이 템플릿의 필수 섹션(관련 이슈, 작업 내용, 특이 사항)을 모두 포함하고 있으며, 상세하고 명확합니다.
Linked Issues check ✅ Passed PR 변경사항이 #759 이슈의 모든 요구사항을 충족합니다: S3 파일 업로드 로직, Admin University Domain 수정(DTO, Controller, Service).
Out of Scope Changes check ✅ Passed 모든 변경사항이 #759 이슈 범위 내에 있습니다. 영문명 unique 제약 추가는 업로드 경로 충돌 방지를 위한 필요한 작업입니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/759-admin-university-image-upload

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Hexeong Hexeong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하십니다! 의견 남겨두었습니다!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/test/java/com/example/solidconnection/s3/domain/UploadDirectoryNameTest.java (1)

17-49: ⚡ Quick win

1) 정규화 운영 적합성 확인을 위해 null·비ASCII 케이스 테스트를 추가해 주세요.

PR 목적(운영 데이터 기준 정규화 충분성 검증) 대비 현재 케이스가 조금 얕습니다. null 입력, 그리고 정규화 후 빈 문자열이 되는 입력(예: 한글/특수문자-only)을 명시적으로 고정해 두면 회귀 방지에 유리합니다.

테스트 추가 예시
+        `@Test`
+        void null_입력이면_예외가_발생한다() {
+            assertThatThrownBy(() -> UploadDirectoryName.fromUniversityEnglishName(null))
+                    .isInstanceOf(CustomException.class);
+        }
+
+        `@Test`
+        void 비ASCII만_있어_정규화_결과가_비면_예외가_발생한다() {
+            String englishName = "東京大学";
+            assertThatThrownBy(() -> UploadDirectoryName.fromUniversityEnglishName(englishName))
+                    .isInstanceOf(CustomException.class);
+        }
🤖 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/example/solidconnection/s3/domain/UploadDirectoryNameTest.java`
around lines 17 - 49, Add additional test cases for the
UploadDirectoryName.fromUniversityEnglishName method to improve test coverage of
edge cases. Create test methods that cover: a null input case (which should
throw a CustomException), non-ASCII characters only input such as Korean or CJK
characters that would result in an empty string after normalization, and inputs
containing only special characters. These tests should verify that the method
handles these edge cases gracefully, either by throwing appropriate exceptions
or handling the normalization in a defined way that prevents regressions in
production.
src/main/java/com/example/solidconnection/s3/controller/S3Controller.java (1)

83-113: ⚡ Quick win

1) 중복 업로드 흐름을 헬퍼로 묶어 두 엔드포인트의 분기 오차를 줄여주세요.

현재 Line 90-96, Line 106-112의 로직이 사실상 동일해서, 다음 변경 때 한쪽만 수정되는 드리프트 위험이 있습니다. UploadPath만 인자로 받는 private helper로 합치면 유지보수성이 좋아집니다.

예시 리팩터링
 `@RequireRoleAccess`(roles = Role.ADMIN)
 `@PostMapping`("/admin/university/logo")
 public ResponseEntity<UploadedFileUrlResponse> uploadAdminUniversityLogo(
         `@AuthorizedUser` long adminId,
         `@RequestParam`("file") MultipartFile imageFile,
         `@RequestParam`("englishName") String englishName
 ) {
-    String directoryName = UploadDirectoryName.fromUniversityEnglishName(englishName);
-    UploadedFileUrlResponse logoImageUrl = s3Service.uploadFile(
-            imageFile,
-            UploadPath.ADMIN_UNIVERSITY_LOGO,
-            directoryName
-    );
-    return ResponseEntity.ok(logoImageUrl);
+    return ResponseEntity.ok(uploadAdminUniversityImage(imageFile, englishName, UploadPath.ADMIN_UNIVERSITY_LOGO));
 }
 
 `@RequireRoleAccess`(roles = Role.ADMIN)
 `@PostMapping`("/admin/university/background")
 public ResponseEntity<UploadedFileUrlResponse> uploadAdminUniversityBackground(
         `@AuthorizedUser` long adminId,
         `@RequestParam`("file") MultipartFile imageFile,
         `@RequestParam`("englishName") String englishName
 ) {
-    String directoryName = UploadDirectoryName.fromUniversityEnglishName(englishName);
-    UploadedFileUrlResponse backgroundImageUrl = s3Service.uploadFile(
-            imageFile,
-            UploadPath.ADMIN_UNIVERSITY_BACKGROUND,
-            directoryName
-    );
-    return ResponseEntity.ok(backgroundImageUrl);
+    return ResponseEntity.ok(uploadAdminUniversityImage(imageFile, englishName, UploadPath.ADMIN_UNIVERSITY_BACKGROUND));
 }
+
+private UploadedFileUrlResponse uploadAdminUniversityImage(
+        MultipartFile imageFile,
+        String englishName,
+        UploadPath uploadPath
+) {
+    String directoryName = UploadDirectoryName.fromUniversityEnglishName(englishName);
+    return s3Service.uploadFile(imageFile, uploadPath, directoryName);
+}
🤖 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/main/java/com/example/solidconnection/s3/controller/S3Controller.java`
around lines 83 - 113, The methods uploadAdminUniversityLogo and
uploadAdminUniversityBackground contain duplicate logic where both retrieve the
directoryName from englishName and invoke s3Service.uploadFile with different
UploadPath enum values. Create a private helper method that accepts the
MultipartFile, UploadPath enum value, and englishName as parameters, then
extract the common upload logic (directoryName retrieval and
s3Service.uploadFile invocation) into this helper. Update both
uploadAdminUniversityLogo and uploadAdminUniversityBackground to call this
helper method with their respective UploadPath.ADMIN_UNIVERSITY_LOGO and
UploadPath.ADMIN_UNIVERSITY_BACKGROUND values to eliminate code duplication and
reduce maintenance drift.
src/test/java/com/example/solidconnection/s3/service/S3ServiceDynamicPathTest.java (1)

38-55: ⚡ Quick win

1) 응답 URL뿐 아니라 실제 업로드 키 인자도 함께 검증해 주세요.

지금은 response.fileUrl()만 확인해서, 내부 업로드 호출 키가 어긋나도 놓칠 수 있습니다. fileUploadService.uploadFile(...)의 path 인자에 동적 디렉터리가 포함됐는지 같이 검증하면 계약 테스트가 더 단단해집니다.

검증 보강 예시
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.verify;
+
+import org.mockito.ArgumentCaptor;
@@
             assertAll(
                     () -> assertThat(response.fileUrl()).startsWith("admin/logo/university_of_tokyo/"),
                     () -> assertThat(response.fileUrl()).endsWith(".png")
             );
+
+            ArgumentCaptor<String> pathCaptor = ArgumentCaptor.forClass(String.class);
+            verify(fileUploadService).uploadFile(eq(null), pathCaptor.capture(), any(), eq("image/png"));
+            assertThat(pathCaptor.getValue()).contains("admin/logo/university_of_tokyo/");
         }
🤖 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/example/solidconnection/s3/service/S3ServiceDynamicPathTest.java`
around lines 38 - 55, The test currently only verifies the response URL but does
not validate that the internal upload call uses the correct path argument with
the dynamic directory. Add mock verification to ensure that the underlying
upload method within s3Service.uploadFile is being called with a path argument
that correctly includes the dynamic subdirectory "university_of_tokyo" alongside
the base UploadPath.ADMIN_UNIVERSITY_LOGO. Use a verification mechanism (such as
Mockito's verify) to assert that the path passed to the internal upload call
contains both the static upload path and the dynamic directory component to
strengthen the contract test.
🤖 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.

Inline comments:
In
`@src/main/java/com/example/solidconnection/s3/domain/UploadDirectoryName.java`:
- Around line 11-27: The fromUniversityEnglishName method generates S3 directory
names from university English names, but since the englishName field has no
unique constraint in the database, different universities can normalize to the
same slug, causing file collisions in S3 paths. To fix this, either modify the
method to include a stable identifier like universityId as part of the directory
name path (making it unique across universities), or add a unique constraint to
the englishName field in the HostUniversity entity to prevent duplicate English
names at the database level. Choose the appropriate solution based on your
architecture and requirements.

---

Nitpick comments:
In `@src/main/java/com/example/solidconnection/s3/controller/S3Controller.java`:
- Around line 83-113: The methods uploadAdminUniversityLogo and
uploadAdminUniversityBackground contain duplicate logic where both retrieve the
directoryName from englishName and invoke s3Service.uploadFile with different
UploadPath enum values. Create a private helper method that accepts the
MultipartFile, UploadPath enum value, and englishName as parameters, then
extract the common upload logic (directoryName retrieval and
s3Service.uploadFile invocation) into this helper. Update both
uploadAdminUniversityLogo and uploadAdminUniversityBackground to call this
helper method with their respective UploadPath.ADMIN_UNIVERSITY_LOGO and
UploadPath.ADMIN_UNIVERSITY_BACKGROUND values to eliminate code duplication and
reduce maintenance drift.

In
`@src/test/java/com/example/solidconnection/s3/domain/UploadDirectoryNameTest.java`:
- Around line 17-49: Add additional test cases for the
UploadDirectoryName.fromUniversityEnglishName method to improve test coverage of
edge cases. Create test methods that cover: a null input case (which should
throw a CustomException), non-ASCII characters only input such as Korean or CJK
characters that would result in an empty string after normalization, and inputs
containing only special characters. These tests should verify that the method
handles these edge cases gracefully, either by throwing appropriate exceptions
or handling the normalization in a defined way that prevents regressions in
production.

In
`@src/test/java/com/example/solidconnection/s3/service/S3ServiceDynamicPathTest.java`:
- Around line 38-55: The test currently only verifies the response URL but does
not validate that the internal upload call uses the correct path argument with
the dynamic directory. Add mock verification to ensure that the underlying
upload method within s3Service.uploadFile is being called with a path argument
that correctly includes the dynamic subdirectory "university_of_tokyo" alongside
the base UploadPath.ADMIN_UNIVERSITY_LOGO. Use a verification mechanism (such as
Mockito's verify) to assert that the path passed to the internal upload call
contains both the static upload path and the dynamic directory component to
strengthen the contract test.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cf63ea4b-50d4-41a4-9fd3-c9dd412f24e6

📥 Commits

Reviewing files that changed from the base of the PR and between bf0b7b2 and 136df16.

📒 Files selected for processing (6)
  • src/main/java/com/example/solidconnection/s3/controller/S3Controller.java
  • src/main/java/com/example/solidconnection/s3/domain/UploadDirectoryName.java
  • src/main/java/com/example/solidconnection/s3/domain/UploadPath.java
  • src/main/java/com/example/solidconnection/s3/service/S3Service.java
  • src/test/java/com/example/solidconnection/s3/domain/UploadDirectoryNameTest.java
  • src/test/java/com/example/solidconnection/s3/service/S3ServiceDynamicPathTest.java

Comment thread src/main/java/com/example/solidconnection/s3/domain/UploadDirectoryName.java Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 136df16b05

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/main/java/com/example/solidconnection/s3/domain/UploadPath.java Outdated
@lsy1307 lsy1307 force-pushed the refactor/759-admin-university-image-upload branch from c8900fb to 136df16 Compare June 17, 2026 06:35
lsy1307 added 2 commits June 17, 2026 15:45
- 대학 영문명 디렉토리명에 원본 영문명 해시를 추가

- 파견 대학 영문명 중복 검증과 유니크 제약을 추가

- 정규화 충돌과 영문명 중복 검증 테스트를 추가
- 업로드 경로별 이미지 전용 검증 정책을 추가

- 관리자 대학 이미지 업로드에서 문서 확장자를 차단

- 증빙 파일 업로드의 문서 확장자 허용 동작을 테스트로 보장

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/test/java/com/example/solidconnection/s3/service/S3ServiceTest.java (1)

114-127: ⚡ Quick win

2) 신규 배경 경로(ADMIN_UNIVERSITY_BACKGROUND)도 동일한 차단 테스트를 추가해 주세요.

지금은 Line 115 기준 ADMIN_UNIVERSITY_LOGO만 문서 확장자 차단을 검증합니다. 동일하게 imageOnly=trueADMIN_UNIVERSITY_BACKGROUND 케이스도 추가하면 enum 상수별 회귀를 더 빨리 잡을 수 있습니다.

✅ 제안 테스트 추가
+        `@Test`
+        void 이미지_전용_배경_업로드_경로도_문서_확장자를_허용하지_않는다() {
+            // given
+            MockMultipartFile pdfFile = createMockFile("background.pdf", 100);
+
+            // when & then
+            assertThatThrownBy(() -> s3Service.uploadFile(
+                    pdfFile,
+                    UploadPath.ADMIN_UNIVERSITY_BACKGROUND,
+                    "university_of_tokyo"
+            ))
+                    .isInstanceOf(CustomException.class)
+                    .hasMessageContaining("허용된 형식");
+        }
🤖 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/example/solidconnection/s3/service/S3ServiceTest.java`
around lines 114 - 127, Add a new test method to the S3ServiceTest class that
mirrors the existing test method (이미지_전용_업로드_경로는_문서_확장자를_허용하지_않는다) but uses
UploadPath.ADMIN_UNIVERSITY_BACKGROUND instead of
UploadPath.ADMIN_UNIVERSITY_LOGO. Create a MockMultipartFile with a PDF
extension (e.g., "logo.pdf"), call s3Service.uploadFile with the background path
and the PDF file, and assert that it throws a CustomException with the message
containing "허용된 형식". This ensures both image-only paths properly reject document
file types.
src/test/java/com/example/solidconnection/admin/service/AdminHostUniversityServiceTest.java (1)

369-391: ⚡ Quick win

1. excludeId 허용 분기 회귀를 막는 성공 케이스도 함께 추가해주세요.

현재는 “다른 대학 영문명 충돌” 실패 경로는 잘 검증되지만, “같은 대학의 기존 영문명으로 수정” 성공 경로가 없어 validateEnglishNameNotDuplicated의 핵심 분기 회귀를 놓칠 수 있습니다. 아래처럼 성공 테스트 1개를 보강하면 안전합니다.

테스트 보강 예시
@@
     `@Test`
     void 같은_대학의_한글명으로_수정하면_성공한다() {
@@
     }
+
+    `@Test`
+    void 같은_대학의_영문명으로_수정하면_성공한다() {
+        // given
+        HostUniversity university = universityFixture.괌_대학();
+
+        AdminHostUniversityUpdateRequest request = new AdminHostUniversityUpdateRequest(
+                "Updated Korean Name",
+                university.getEnglishName(),
+                "수정된 표시명",
+                null, null, null,
+                "https://logo.com/image.png",
+                "https://background.com/image.png",
+                null,
+                university.getCountry().getCode(),
+                university.getRegion().getCode()
+        );
+
+        // when
+        AdminHostUniversityDetailResponse response = adminHostUniversityService.updateHostUniversity(
+                university.getId(), request);
+
+        // then
+        assertThat(response.englishName()).isEqualTo(university.getEnglishName());
+    }
🤖 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/example/solidconnection/admin/service/AdminHostUniversityServiceTest.java`
around lines 369 - 391, The current test validates the failure scenario where
updating with another university's English name raises an exception, but lacks a
success test case to verify that updating a university with its own existing
English name succeeds. Add a new test method that updates university1 with its
own English name to ensure the excludeId filtering logic in the
validateEnglishNameNotDuplicated method correctly allows duplicate names when
they belong to the same university being updated. This success case is critical
to prevent regressions in the excludeId filtering branch.
🤖 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.

Inline comments:
In `@src/main/java/com/example/solidconnection/s3/domain/UploadPath.java`:
- Around line 42-46: The validateExtension method in the UploadPath class
currently only checks file extensions, which can be bypassed by renaming
malicious files. Enhance the validation by implementing magic byte (file
signature) based validation as the primary check and use the existing extension
validation as a secondary verification method. Add a new validation method to
verify the actual file content against expected magic bytes for each allowed
file type, and integrate this content-based check into the validateExtension
method so that both the file content signature and extension must match the
allowed types before the file is accepted.

---

Nitpick comments:
In
`@src/test/java/com/example/solidconnection/admin/service/AdminHostUniversityServiceTest.java`:
- Around line 369-391: The current test validates the failure scenario where
updating with another university's English name raises an exception, but lacks a
success test case to verify that updating a university with its own existing
English name succeeds. Add a new test method that updates university1 with its
own English name to ensure the excludeId filtering logic in the
validateEnglishNameNotDuplicated method correctly allows duplicate names when
they belong to the same university being updated. This success case is critical
to prevent regressions in the excludeId filtering branch.

In `@src/test/java/com/example/solidconnection/s3/service/S3ServiceTest.java`:
- Around line 114-127: Add a new test method to the S3ServiceTest class that
mirrors the existing test method (이미지_전용_업로드_경로는_문서_확장자를_허용하지_않는다) but uses
UploadPath.ADMIN_UNIVERSITY_BACKGROUND instead of
UploadPath.ADMIN_UNIVERSITY_LOGO. Create a MockMultipartFile with a PDF
extension (e.g., "logo.pdf"), call s3Service.uploadFile with the background path
and the PDF file, and assert that it throws a CustomException with the message
containing "허용된 형식". This ensures both image-only paths properly reject document
file types.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c0a4c6c4-a4fa-49d5-b75a-03ebf54a9824

📥 Commits

Reviewing files that changed from the base of the PR and between c8900fb and b3f0ce6.

📒 Files selected for processing (9)
  • src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java
  • src/main/java/com/example/solidconnection/s3/domain/UploadDirectoryName.java
  • src/main/java/com/example/solidconnection/s3/domain/UploadPath.java
  • src/main/java/com/example/solidconnection/university/domain/HostUniversity.java
  • src/main/java/com/example/solidconnection/university/repository/HostUniversityRepository.java
  • src/main/resources/db/migration/V52__add_unique_constraint_to_host_university_english_name.sql
  • src/test/java/com/example/solidconnection/admin/service/AdminHostUniversityServiceTest.java
  • src/test/java/com/example/solidconnection/s3/domain/UploadDirectoryNameTest.java
  • src/test/java/com/example/solidconnection/s3/service/S3ServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/main/resources/db/migration/V52__add_unique_constraint_to_host_university_english_name.sql
  • src/main/java/com/example/solidconnection/university/repository/HostUniversityRepository.java
  • src/main/java/com/example/solidconnection/university/domain/HostUniversity.java
  • src/main/java/com/example/solidconnection/s3/domain/UploadDirectoryName.java
  • src/test/java/com/example/solidconnection/s3/domain/UploadDirectoryNameTest.java
  • src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java

Comment on lines 42 to 46
public void validateExtension(String extension) {
if (extension == null || !FileConstants.ALL_ALLOWED_EXTENSIONS.contains(extension.toLowerCase())) {
if (extension == null || !getAllowedExtensions().contains(extension.toLowerCase())) {
throw new CustomException(ErrorCode.NOT_ALLOWED_FILE_EXTENSIONS,
"허용된 형식: " + getAllowedExtensionsMessage());
}

@coderabbitai coderabbitai Bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

1) 확장자-only 검증은 우회 업로드를 막지 못합니다.

Line 43 기준으로 현재 검증은 파일명 확장자만 신뢰합니다. 이 방식은 내용이 다른 파일(예: 실행파일/스크립트)을 확장자만 바꿔 통과시킬 수 있어요. 최소한 매직바이트(파일 시그니처) 기반 검증을 추가하고, 확장자 검증은 보조 수단으로 두는 편이 안전합니다.

🔧 제안 패치 (개념 예시)
 public void validateExtension(String extension) {
     if (extension == null || !getAllowedExtensions().contains(extension.toLowerCase())) {
         throw new CustomException(ErrorCode.NOT_ALLOWED_FILE_EXTENSIONS,
                                   "허용된 형식: " + getAllowedExtensionsMessage());
     }
 }
+
+// S3Service.validateFile(...)에서 확장자 검증 후 추가:
+// - imageOnly 경로: JPEG/PNG/WEBP 등 허용 포맷 시그니처 검사
+// - 비 imageOnly 경로: MIME + 시그니처 기반 최소 검증
🤖 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/main/java/com/example/solidconnection/s3/domain/UploadPath.java` around
lines 42 - 46, The validateExtension method in the UploadPath class currently
only checks file extensions, which can be bypassed by renaming malicious files.
Enhance the validation by implementing magic byte (file signature) based
validation as the primary check and use the existing extension validation as a
secondary verification method. Add a new validation method to verify the actual
file content against expected magic bytes for each allowed file type, and
integrate this content-based check into the validateExtension method so that
both the file content signature and extension must match the allowed types
before the file is accepted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 나주에 수정할게

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lsy1307 알겠어요! 나중에 작업하실 때 놓치지 않도록, 팔로업 이슈를 생성해 드릴까요?

  1. 이슈 내용 요약
    • UploadPath.validateExtension에 매직바이트(파일 시그니처) 기반 검증 추가
    • 확장자 검증은 보조 수단으로 유지
    • imageOnly 경로와 그 외 경로에 대한 검증 전략 분리

원하시면 "이슈 만들어줘"라고 말씀해 주세요 😊

(๑˃ᵕ˂)و ✨

@Hexeong Hexeong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다~

@lsy1307 lsy1307 merged commit 70d8feb into develop Jun 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: 어드민 학교 상세정보 이미지 업로드를 url에서 파일 업로드로 변경

2 participants