🔀 :: [#721] - 애플리케이션 컨테이너 생성시 유효하지 않은 볼륨 이름이 들어가는 현상 수정 #722
Conversation
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
Contributor
WalkthroughDocker 컨테이너 생성 시 볼륨 마운트 플래그 구성에서 사용 속성을 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/com/dcd/server/core/domain/application/service/impl/CreateContainerServiceImpl.kt (1)
41-42: 치명적: rm -rf에 사용자 입력 직접 삽입 → 명령 주입 및 과도 삭제 위험
rm -rf ${application.name}는 인자 검증/인용 없이 셸로 전달되어 주입과 와일드카드 확장, 옵션 혼동 위험이 큽니다. 실패 시 컨테이너/볼륨 정리를 의도했다면 셸rm대신 Docker 명령 또는 안전한 파일 API를 사용하세요.단기 수정(컨테이너 정리로 대체):
- commandPort.executeShellCommand("rm -rf ${application.name}") + // 안전한 정리: 생성(시도)된 컨테이너 제거 + commandPort.executeShellCommand("docker rm -f -- ${application.containerName} || true")장기 권장:
- 로컬 디렉터리 정리가 필요하다면: 베이스 디렉터리 고정 + 경로 정규화(normalize) +
Files.walkFileTree로 삭제. 셸 호출 금지.- 모든 이름형 입력(
application.name/containerName/networkName/volumeName)은 공용 NAME_SAFE_REGEX로 검증.필요하시면 안전 삭제 유틸과 공용 검증 유틸(Regex 포함) 패치를 만들어 드리겠습니다.
Also applies to: 51-52
🧹 Nitpick comments (4)
src/main/kotlin/com/dcd/server/core/domain/application/service/impl/CreateContainerServiceImpl.kt (4)
21-30: 마운트 플래그 빌드 단순화 및 트레일링 공백 제거현재 방식은 마지막에 불필요한 공백이 남습니다.
joinToString로 간결하게 만들고, 뒤따르는-p와의 공백도 안전하게 처리하세요.- val volumeMountBuilder = StringBuilder() - queryVolumePort.findAllMountByApplication(application) - .forEach { - val volume = it.volume - volumeMountBuilder.append("-v ${volume.volumeName}:${it.mountPath}") - if (it.readOnly) - volumeMountBuilder.append(":ro") - volumeMountBuilder.append(" ") - } - val volumeMountFlags = volumeMountBuilder.toString() + val volumeMountFlags = queryVolumePort.findAllMountByApplication(application) + .joinToString(" ") { m -> + buildString { + append("-v ") + append(m.volume.volumeName) + append(":") + append(m.mountPath) + if (m.readOnly) append(":ro") + } + } + .let { if (it.isNotBlank()) "$it " else "" } // 뒤의 -p와 안전하게 구분
19-21: 포트 유효성 검증 추가 제안(경계값 오류 예방)실행 전 포트 범위를 검사해 조기 실패하도록 하세요.
override suspend fun createContainer(application: Application, externalPort: Int) { withContext(Dispatchers.IO) { + require(externalPort in 1..65535) { "externalPort out of range: $externalPort" } + require(application.port in 1..65535) { "application.port out of range: ${application.port}" }
46-54: 하드코딩된 네트워크명(dcd) — 의도 확인 필요컨테이너를 이미
${application.workspace.networkName}로 생성한 뒤, 추가로dcd네트워크에 연결하고 있습니다. 이중 네트워크가 의도인지, 아니면 설정값이어야 하는지 확인 부탁드립니다. 의도라면 상수 하드코딩 대신 구성값으로 두는 것을 권장합니다.
31-35: :latest 하드코딩 제거 — 이미지 태그를 불변(버전 또는 @sha256:...)으로 지정하거나 태그 주입으로 변경하드코딩된 ':latest'는 배포 재현성과 롤백성을 저하시킵니다.
발견 위치:
- src/main/kotlin/com/dcd/server/core/domain/application/service/impl/CreateContainerServiceImpl.kt:35
- src/main/kotlin/com/dcd/server/core/domain/application/service/impl/BuildDockerImageServiceImpl.kt:31, 37, 52, 55
- src/test/kotlin/com/dcd/server/core/domain/application/service/CreateContainerServiceImplTest.kt:33
- src/test/kotlin/com/dcd/server/core/domain/application/service/BuildDockerImageServiceImplTest.kt:32, 46
- src/test/kotlin/com/dcd/server/core/domain/application/usecase/DeployApplicationUseCaseTest.kt:70
- docker-compose.yml:9 (mariadb:latest), 15 (redis:latest), 23 (nginx:latest)
권장:
- 이미지 태그를 빌드 산출물의 고정된 버전 또는 다이제스트(@sha256:...)로 대체하거나, 이미지:태그를 구성값/인수로 주입하도록 변경.
- 연관 빌드 스크립트와 테스트는 새로운 태그 전략에 맞게 수정(테스트는 태그 주입을 모킹하거나 기대값 갱신).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
-
src/main/kotlin/com/dcd/server/core/domain/application/service/impl/CreateContainerServiceImpl.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: dolong2
PR: dolong2/dcd#710
File: src/main/kotlin/com/dcd/server/core/domain/volume/service/impl/DeleteDockerVolumeServiceImpl.kt:13-18
Timestamp: 2025-09-04T14:59:15.319Z
Learning: dolong2 addressed command injection vulnerability in volume deletion by adding validation in the volume creation service (CreateDockerVolumeServiceImpl) using NAME_SAFE_REGEX pattern, preferring to validate at the source rather than in the deletion service for better security and fail-fast behavior.
Learnt from: dolong2
PR: dolong2/dcd#703
File: src/main/kotlin/com/dcd/server/core/domain/volume/model/Volume.kt:1-12
Timestamp: 2025-09-01T14:52:42.625Z
Learning: dolong2 prefers to implement validation logic (such as path validation for Volume.physicalPath) in the business logic layer rather than in domain model init blocks, keeping domain models as simple data containers.
📚 Learning: 2025-09-04T14:59:15.319Z
Learnt from: dolong2
PR: dolong2/dcd#710
File: src/main/kotlin/com/dcd/server/core/domain/volume/service/impl/DeleteDockerVolumeServiceImpl.kt:13-18
Timestamp: 2025-09-04T14:59:15.319Z
Learning: dolong2 addressed command injection vulnerability in volume deletion by adding validation in the volume creation service (CreateDockerVolumeServiceImpl) using NAME_SAFE_REGEX pattern, preferring to validate at the source rather than in the deletion service for better security and fail-fast behavior.
Applied to files:
src/main/kotlin/com/dcd/server/core/domain/application/service/impl/CreateContainerServiceImpl.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test
🔇 Additional comments (1)
src/main/kotlin/com/dcd/server/core/domain/application/service/impl/CreateContainerServiceImpl.kt (1)
25-28: 긴급: volume.volumeName 검증 누락 — 쉘 명령에 직접 삽입되어 보안 위험(수정 필요)
- volume.name 잔존 참조 없음.
- mountPath는 프레젠테이션 DTO에서 정규식 검증됨 — src/main/kotlin/com/dcd/server/presentation/domain/volume/data/request/MountVolumeRequest.kt.
- volume.volumeName 생성: src/main/kotlin/com/dcd/server/core/domain/volume/model/Volume.kt:12 (val volumeName = "${name.replace(" ", "_")}-$id") — 공백만 치환, 특수문자·콜론·개행·명령삽입 미검증.
- volumeName이 쉘에 직접 사용되는 위치(수정필요):
- src/main/kotlin/com/dcd/server/core/domain/volume/service/impl/CreateDockerVolumeServiceImpl.kt:14 (docker volume create ${volume.volumeName})
- src/main/kotlin/com/dcd/server/core/domain/volume/service/impl/DeleteDockerVolumeServiceImpl.kt:14 (docker volume rm ${volume.volumeName})
- src/main/kotlin/com/dcd/server/core/domain/volume/service/impl/CopyDockerVolumeServiceImpl.kt:19-20 (docker run ... -v ${existingVolume.volumeName}:/from -v ${newVolume.volumeName}:/to)
- src/main/kotlin/com/dcd/server/core/domain/application/service/impl/CreateContainerServiceImpl.kt:25 (-v ${volume.volumeName}:${it.mountPath})
- NAME_SAFE_REGEX 정의/사용 없음(검색 결과 없음).
조치: volume.volumeName에 대해 허용 문자 검증(NAME_SAFE_REGEX 등) 추가 또는 쉘 인자 분리/안전한 이스케이프 적용(직접 문자열 결합 금지). 레거시 참조 재확인.
⛔ Skipped due to learnings
Learnt from: dolong2 PR: dolong2/dcd#710 File: src/main/kotlin/com/dcd/server/core/domain/volume/service/impl/DeleteDockerVolumeServiceImpl.kt:13-18 Timestamp: 2025-09-04T14:59:15.319Z Learning: dolong2 addressed command injection vulnerability in volume deletion by adding validation in the volume creation service (CreateDockerVolumeServiceImpl) using NAME_SAFE_REGEX pattern, preferring to validate at the source rather than in the deletion service for better security and fail-fast behavior.Learnt from: dolong2 PR: dolong2/dcd#517 File: src/main/kotlin/com/dcd/server/presentation/domain/application/data/exetension/ApplicationRequestDataExtension.kt:13-13 Timestamp: 2025-02-15T10:33:37.315Z Learning: In the DCD project, while spaces are allowed in application names (UI/display), they must be replaced with appropriate characters in specific technical contexts: 1. With underscores for Docker container names (in Application.kt) 2. With hyphens for HTTP configuration files (in GenerateHttpConfigServiceImpl.kt)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
개요
작업내용
체크리스트
Summary by CodeRabbit