feat(backend): API 엔드포인트 구현 (AIdol, Companion, Lead) #16
Conversation
- AIdol, Companion, Lead 리포지토리 팩토리의 생성 및 동작 검증
- ClaimToken 필수 여부 및 필드 매핑 검증\n- 응답 시 민감 정보(ClaimToken) 제외 여부 검증
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces API endpoints for "AIdol", "Companion", and "Lead", and migrates the image generation logic from OpenAI to Google Gemini. The separation of business logic into service layers, schema expansion, and extensive test coverage significantly improve code quality and maintainability. However, a security audit revealed critical and high-severity Broken Access Control (IDOR) vulnerabilities in the "AIdol" and "Companion" update/delete endpoints due to a lack of ownership verification. A medium-severity Prompt Injection vulnerability was also found in the image generation service. Additionally, there are concerns regarding database session management and broad exception handling. Addressing these security and code quality concerns is crucial before merging, alongside a few other suggested improvements.
| """Remove Companion from Group (Unassign).""" | ||
| # Get item first | ||
| self._get_item_or_404(repository, item_id) | ||
|
|
||
| # Update aidol_id to None (remove from group) | ||
| update_data = CompanionUpdate(aidol_id=None) | ||
|
|
||
| updated = repository.update(item_id, update_data) |
There was a problem hiding this comment.
This delete_companion endpoint, which unassigns a companion from a group, suffers from a high-severity Broken Access Control (IDOR) vulnerability. It lacks proper authorization checks, allowing any user to unassign any companion from its AIdol group, leading to unauthorized data modification and potential denial of service. Ownership verification is crucial here. Furthermore, the current implementation includes an unnecessary database lookup via _get_item_or_404 before calling repository.update. The repository.update method's return value can be directly checked for item existence, which would improve efficiency by removing redundant database queries.
| # TODO: Verify ClaimToken if strict ownership is required (Sprint 1) | ||
| updated = repository.update(item_id, data) |
There was a problem hiding this comment.
The update_aidol endpoint lacks an authorization check to verify ownership of the AIdol group. As noted in the TODO comment, any user can currently update any AIdol group by providing its ID. This is a Broken Access Control (IDOR) vulnerability. You should verify the ClaimToken provided in the request headers against the claim_token stored in the database for the target AIdol group before allowing the update.
| """Update Companion.""" | ||
| # Exclude system_prompt from request - should not be set via API | ||
| sanitized_data = data.model_dump( | ||
| exclude={"system_prompt"}, exclude_unset=True | ||
| ) | ||
| sanitized_request = CompanionUpdate(**sanitized_data) | ||
|
|
||
| updated = repository.update(item_id, sanitized_request) |
There was a problem hiding this comment.
The update_companion endpoint does not perform any ownership verification. Any user can modify the details of any companion by providing its ID. This is a Broken Access Control (IDOR) vulnerability. Access should be restricted to the owner of the AIdol group that the companion belongs to, likely by verifying a ClaimToken against the associated AIdol group.
backend/aidol/api/lead.py
Outdated
| with self.db_session_factory() as session: | ||
| aidol_repo = self.aidol_repository_factory.create_repository( | ||
| session | ||
| ) |
There was a problem hiding this comment.
create_lead 함수 내에서 with self.db_session_factory() as session:을 사용하여 수동으로 새로운 데이터베이스 세션을 생성하고 있습니다. 이 방식은 FastAPI의 요청-응답 주기 동안의 세션 관리 흐름을 따르지 않아 트랜잭션 원자성 및 커넥션 풀 관리에 예기치 않은 문제를 일으킬 수 있습니다. 주석으로 언급하신 것처럼 두 리포지토리가 동일한 세션을 공유하는 것이 이상적입니다. lead_repository에 이미 주입된 세션을 재사용하여 이 문제를 해결할 수 있습니다.
aidol_repo = self.aidol_repository_factory.create_repository(
lead_repository.db_session
)| response = self.client.models.generate_content( | ||
| model="gemini-3-pro-image-preview", | ||
| contents=[prompt], # type: ignore[arg-type] | ||
| ) |
There was a problem hiding this comment.
User-supplied prompts are passed directly to the Gemini image generation model without sanitization or a constraining system prompt. This makes the application vulnerable to Prompt Injection, where an attacker can manipulate the model's behavior to generate inappropriate content or bypass safety filters. Consider implementing prompt sanitization or using a system instruction to enforce safety and content guidelines.
| """ | ||
| result = self.generate_image(prompt, size, quality) | ||
| if result is None: | ||
| except Exception as e: # pylint: disable=broad-exception-caught |
There was a problem hiding this comment.
generate_and_download_image 함수에서 포괄적인 Exception을 잡고 있습니다. pylint 경고를 비활성화하신 것을 보면 인지하고 계신 것 같지만, 외부 API 호출 시 발생할 수 있는 특정 예외(예: google.api_core.exceptions.GoogleAPICallError, google.auth.exceptions.DefaultCredentialsError 등)를 명시적으로 처리하는 것이 더 안전합니다. 이렇게 하면 KeyboardInterrupt와 같은 의도치 않은 예외까지 숨기는 것을 방지하고, 오류 처리 로직을 더 견고하게 만들 수 있습니다.
There was a problem hiding this comment.
제형님, 이 전반적으로 매우 잘 작성되었습니다!
좋은 점:
- 명확한 레이어 분리 (Repository → Service → API)
- 포괄적인 테스트 커버리지
- TODO로 미해결 이슈 명시
개선이 필요한 점:
- 세션 관리 패턴
- Exception 처리
- 중복 코드 추출
- import 정리
1-2는 품질 개선이라 이번 PR에서 수정 권장, 3-4는 리팩토링이라 별도 PR로 분리해도 됩니다.
Sprint 2:
- 취약점 3곳 - TODO로 남겨두신 대로 Sprint 1 이후 처리
feat(backend): Sprint 1 API 엔드포인트 구현 (AIdol, Companion, Lead)
요약
AIdol 그룹, Companion(연습생), Lead(이메일) 관리를 위한 전체 API 엔드포인트를 구현하고 **명세(Spec)**를 확정했습니다.
목적
Sprint 1의 목표인 '기본적인 아이돌 그룹 생성 및 멤버 관리, 잠재 고객 확보' 기능을 백엔드 API로 제공하기 위함입니다. 또한
ClaimToken기반의 소유권 검증 로직을 도입하여 보안성을 강화했습니다.변경 사항
POST), 조회(GET), 수정(PATCH) 및 엠블럼 이미지 생성(POST /images) 기능을 구현했습니다.POST), 목록 조회(GET- 성별/캐스팅 필터 포함), 수정(PATCH), 삭제(DELETE) 및 프로필 이미지 생성(POST /images) 기능을 구현했습니다.POST /leads) 기능을 추가했습니다. (유효한 토큰 소유 시 그룹 이메일 업데이트, 아니면 단순 Lead 저장)tests/integration/), Service 레이어 테스트, 스키마/팩토리 유닛 테스트(tests/unit/)를 통해 안정성을 검증했습니다.테스트
ClaimToken헤더가 정상 반환되는지 확인gender,isCast필터가 정상 동작하는지 확인python -m unittest discover -s tests)가 모두 통과하는지 확인