Fix 0.2.3/wordset admin #12
Conversation
요약워드셋 승인 워크플로우, 상태 필터링, 관리 기능을 추가합니다. 새 문서 파일, Dockerfile 업데이트, 상태 기반 조회/삭제/승인/거부 엔드포인트, 예외 클래스, Beanie 저장소 메서드를 도입합니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Admin as 관리자
participant API as API 라우터
participant Service as 서비스
participant Repo as 저장소
participant DB as MongoDB
Admin->>API: PATCH /wordsets/{id}/approve
API->>Service: approve_wordset(wordset_id)
Service->>Repo: find_by_id(wordset_id)
Repo->>DB: 워드셋 조회
DB-->>Repo: 워드셋 반환
Repo-->>Service: ChatBotWordSet
alt 이미 승인됨
Service-->>API: AlreadyApprovedWordSetException
API-->>Admin: 400 에러
else 승인 진행
Service->>Repo: update_status(wordset_id, APPROVED)
Repo->>DB: 상태 업데이트
Service->>Service: CharacterWordSet 구성
Service->>Repo: add_wordset(character_id, wordset)
Repo->>DB: chatbot의 character_wordset 업데이트
API-->>Admin: 200 OK
end
rect rgb(200, 220, 240)
Note over Service,DB: 상태 필터링 조회
Admin->>API: GET /wordsets?status=PENDING
API->>Service: get_chatbot_wordset_with_status(cursor_id, size, "PENDING")
Service->>Repo: find_by_status("PENDING", size)
Repo->>DB: 상태 필터 조회
DB-->>Repo: PENDING 워드셋 목록
API-->>Admin: CursorResponse
end
예상 코드 리뷰 노력🎯 4 (복잡함) | ⏱️ ~60분 워드셋 생명주기 관리(승인/거부/삭제), 상태 기반 쿼리 메커니즘, 다층 예외 처리, 저장소 메서드 확장으로 인한 상당한 논리 밀도. Dockerfile 시스템 의존성 설정, 여러 도메인에 걸친 서비스 통합으로 인해 복잡도 증가. API 라우터 재구성 및 쿼리 파라미터 추가로 엔드포인트 계약 변경. 관련 가능성 있는 PR
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (17)
core/events/deps.py (1)
8-13: BaseSettings의 환경 변수 처리 방식과 중복
os.getenv를 사용하여 기본값을 설정하고 있지만,BaseSettings는 자체적으로 환경 변수와.env파일을 읽습니다. 현재 구현에서는os.getenv가 모듈 로드 시점에 평가되므로.env파일의 값을 올바르게 읽지 못할 수 있습니다.다음과 같이
os.getenv를 제거하고BaseSettings가 환경 변수를 처리하도록 하는 것을 권장합니다:-import os - class KafkaSettings(BaseSettings): """Kafka 설정""" - kafka_bootstrap_servers: str = os.getenv("KAFKA_BOOTSTRAP_SERVERS", "localhost:9092") - kafka_client_id: str = os.getenv("KAFKA_PRODUCER_ID", "fastapi-event-publisher") + kafka_bootstrap_servers: str = "localhost:9092" + kafka_client_id: str = "fastapi-event-publisher" kafka_compression_type: str = "gzip" kafka_acks: str = "all"
BaseSettings의Config클래스에서 이미.env파일을 지정하고 있으므로, 환경 변수KAFKA_BOOTSTRAP_SERVERS와KAFKA_PRODUCER_ID는 자동으로 읽힙니다..serena/memories/suggested_commands.md (1)
31-33: Markdown 스타일: bare URL 형식 개선 권장정적 분석 도구가 bare URL 사용을 감지했습니다. Markdown 표준에 따라 URL을 angle brackets(
<>)로 감싸거나 링크 형식으로 변경하는 것을 권장합니다.다음과 같이 수정할 수 있습니다:
### 서버 접속 -- 로컬: http://localhost:4449 -- API 문서: http://localhost:4449/docs (Swagger UI) -- ReDoc: http://localhost:4449/redoc +- 로컬: <http://localhost:4449> +- API 문서: <http://localhost:4449/docs> (Swagger UI) +- ReDoc: <http://localhost:4449/redoc>.serena/memories/task_completion_checklist.md (1)
37-37: Markdown 스타일: bare URL 형식 개선 권장정적 분석 도구가 bare URL 사용을 감지했습니다.
다음과 같이 수정할 수 있습니다:
-- [ ] Swagger UI (http://localhost:4449/docs)에서 API가 정상 작동하는가? +- [ ] Swagger UI (<http://localhost:4449/docs>)에서 API가 정상 작동하는가?.serena/memories/project_overview.md (1)
47-82: 코드 블록에 언어 지정 권장정적 분석 도구가 fenced code block에 언어 지정이 없음을 감지했습니다. 프로젝트 구조를 나타내는 코드 블록이므로
text또는plaintext를 지정하는 것을 권장합니다.다음과 같이 수정할 수 있습니다:
-``` +```text FastAPIProject/ ├── main.py # FastAPI 앱 진입점 ....serena/memories/code_style_and_conventions.md (3)
10-13: 타입 힌트 공백 규칙 명확화 필요 (PEP 8 불일치).현재 "콜론 앞에 공백" 예시(
user_id : str)와 "대부분 콜론 뒤 공백" 문구가 상충하며, PEP 8/PEP 484와도 다릅니다. 콜론 앞 공백 없이, 뒤에만 공백을 두는 것으로 고치세요.다음처럼 문구/예시를 교정하는 패치를 제안합니다:
- - **형식**: 콜론 앞에 공백 (예: `user_id : str`, `chatbot_id: int`) - - 일관성은 없지만 대부분 콜론 뒤에만 공백 + - **형식**: 콜론 앞 공백 없음, 콜론 뒤 한 칸 (예: `user_id: str`, `chatbot_id: int`)
106-109: 툴링을 문서에 명시하고 저장소에 설정 파일을 추가하세요."명시적 린터/포맷터 없음"은 팀 일관성을 해칩니다. ruff+black+isort를 표준으로 고정하고 pyproject.toml(.ruff.toml)을 커밋하는 것을 권장합니다. 필요 시 샘플 설정 파일 제공 가능합니다.
95-104: 쿼리 패턴에 맞춘 인덱스 정책을 문서화하세요.WordSet는
character_id,status, 그리고 커서 기반 조회를 자주 사용합니다. Beanie Settings에 복합 인덱스(("character_id", "status"),("_id", -1)) 권장 사항을 명시하세요. 런타임 성능/스캔 비용을 낮춥니다.Based on learnings
app/chatbot_wordset/document/chatbot_wordset.py (2)
1-3: 사용되지 않는 import 정리.
datetime,before_event,Insert,Update가 이 파일에서 사용되지 않습니다. 제거해 모듈 로딩 비용과 린트 경고를 줄이세요.-from datetime import datetime -from beanie import Document, before_event, Insert, Update +from beanie import Document from app.chatbot_wordset.document.wordset_status import WordSetStatus
6-11: 상태 필드 추가는 적절합니다. 생성/갱신 타임스탬프와 인덱스도 함께 고려하세요.
status도입은 워크플로에 유용합니다. 추가로created_at/updated_at필드와 상태/캐릭터 복합 인덱스를 두면 조회가 빨라집니다. 이벤트 훅(before_event)을 활용해updated_at자동 갱신도 가능합니다. 샘플 구현 제공 가능.Based on learnings
app/chatbot/service/chatbot_service.py (2)
147-150: 중복 empty-check 제거 제안.
_crawl_namuwiki에서 이미 빈 결과 시NoContentFoundException을 발생시키므로,generate의if not namuwiki_list:가드(라인 148-149)는 중복일 수 있습니다. 한 곳으로 일원화하여 책임을 분리하는 편이 깔끔합니다.
184-209: print 디버깅/불필요 f-string/TRY300 정리 + 표준 로깅으로 전환.
- f-string 자리표시자 없는 문자열에
f접두가 불필요합니다(F541).return을try블록의else로 이동하세요(TRY300).- 표준
logging으로 전환해 수준 제어/구조화 로깅을 사용하세요.아래 패치를 제안합니다(외부 추가: 상단에
import logging및logger = logging.getLogger(__name__)선언):async def _crawl_namuwiki(character_name : str) -> List[Tuple[str, str, str]]: - print(f"Attempting to crawl Namuwiki for character: '{character_name}'") + logger.info("Attempting to crawl Namuwiki for character: %s", character_name) crawler = Crawler() - try: - namuwiki_list = await crawler.get_namuwiki_list(name=character_name) - print(f"Crawler returned {len(namuwiki_list) if namuwiki_list else 0} items") - - if not namuwiki_list: - print(f"Warning: No namuwiki content found for character '{character_name}'") - print(f"This could be due to:") - print(f" 1. Character name not found in Namuwiki") - print(f" 2. Network connectivity issues") - print(f" 3. Character name format/encoding issues") - raise NoContentFoundException(character_name=character_name) - - return namuwiki_list - - except IndexError as e: - # 크롤러 내부에서 deque가 비어있을 때 발생 - print(f"IndexError caught while crawling Namuwiki for '{character_name}': {e}") - print(f"Possible causes:") - print(f" 1. No search results found in Namuwiki") - print(f" 2. HTML parsing failed (structure changed)") - print(f" 3. Network error or access blocked") - print(f" 4. Character name encoding issue") - raise NoContentFoundException(character_name=character_name) from e + try: + namuwiki_list = await crawler.get_namuwiki_list(name=character_name) + except IndexError as e: + # 크롤러 내부에서 deque가 비어있을 때 발생 + logger.warning("IndexError while crawling Namuwiki for %s: %s", character_name, e) + raise NoContentFoundException(character_name=character_name) from e + else: + count = len(namuwiki_list or []) + logger.info("Crawler returned %d items", count) + if count == 0: + logger.info("No Namuwiki content found for character: %s", character_name) + raise NoContentFoundException(character_name=character_name) + return namuwiki_listapp/chatbot_wordset/exception/already_approved_wordset_exception.py (1)
4-8: 상태 코드 409(CONFLICT) 고려.이미 승인된 리소스에 대한 재승인은 상태 충돌에 가깝습니다. 400보다 409가 의미상 더 적합합니다.
http.HTTPStatus사용도 권장합니다.-from core.exceptions.business_exception import BusinessException +from core.exceptions.business_exception import BusinessException +from http import HTTPStatus @@ - status_code = 400 + status_code = HTTPStatus.CONFLICTapp/chatbot/exception/no_content_found_exception.py (1)
4-8: HTTPStatus 상수 사용으로 일관성/가독성 향상.매직 넘버 대신
HTTPStatus.NOT_FOUND를 사용하세요.-from core.exceptions.business_exception import BusinessException +from core.exceptions.business_exception import BusinessException +from http import HTTPStatus @@ - status_code = 404 + status_code = HTTPStatus.NOT_FOUNDapp/chatbot/repository/chatbot_repo.py (1)
48-60: 요소 매칭을$elemMatch로 명시하면 안전합니다.현재
Pull매칭은 객체 전체 동등 비교입니다. 키 누락/순서 이슈를 피하려면$elemMatch로 질의 의도를 구체화하세요.- await character.update( - oper.Pull({ - ChatBot.character_wordset: { - "question": question, - "answer": answer - } - }) - ) + await character.update( + oper.Pull({ + ChatBot.character_wordset: {"$elemMatch": {"question": question, "answer": answer}} + }) + )app/chatbot_wordset/service/chatbot_wordset_service.py (3)
75-92: 암묵적 Optional 해소 + enum 타입 사용 권장.
status: str = None는 RUF013에 걸립니다.WordSetStatus | None로 명시하고, 레포 호출 시.value를 전달하세요.-async def get_chatbot_wordset_with_status(cursor_id: int, size: int, status: str = None) -> CursorResponse: +async def get_chatbot_wordset_with_status(cursor_id: int, size: int, status: WordSetStatus | None = None) -> CursorResponse: @@ - if status: + if status is not None: chatbot_wordset = ( - await chatbot_wordset_repo.find_by_status(status, size) + await chatbot_wordset_repo.find_by_status(status.value, size) if cursor_id is None - else await chatbot_wordset_repo.find_by_cursor_id_and_status(cursor_id, status, size) + else await chatbot_wordset_repo.find_by_cursor_id_and_status(cursor_id, status.value, size) )
94-114: 암묵적 Optional 해소 + enum 타입 사용(캐릭터별 조회).동일한 수정이 필요합니다.
-async def get_chatbot_wordset_by_character_with_status( - character_id: int, cursor_id: int, size: int, status: str = None -) -> CursorResponse: +async def get_chatbot_wordset_by_character_with_status( + character_id: int, cursor_id: int, size: int, status: WordSetStatus | None = None +) -> CursorResponse: @@ - if status: + if status is not None: chatbot_wordset = ( - await chatbot_wordset_repo.find_by_character_id_and_status(character_id, status, size) + await chatbot_wordset_repo.find_by_character_id_and_status(character_id, status.value, size) if cursor_id is None else await chatbot_wordset_repo.find_by_cursor_id_character_id_and_status( - character_id, cursor_id, status, size + character_id, cursor_id, status.value, size ) )
27-48: 중복 추가 방지 및 트랜잭션 고려(권장).승인 동시 처리 시 동일 WordSet이 챗봇에 중복으로 들어갈 수 있습니다. 레포에서
AddToSet사용과 길이 검증을 적용하면 크게 완화됩니다(별도 코멘트 참고). Mongo 트랜잭션/보상 로직(승인 실패 시 상태 롤백)도 고려해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.serena/memories/code_style_and_conventions.md(1 hunks).serena/memories/project_overview.md(1 hunks).serena/memories/suggested_commands.md(1 hunks).serena/memories/task_completion_checklist.md(1 hunks)Dockerfile(1 hunks)api/routers/chatbot.py(1 hunks)api/routers/chatbot_wordset.py(2 hunks)app/chatbot/exception/no_content_found_exception.py(1 hunks)app/chatbot/repository/chatbot_repo.py(1 hunks)app/chatbot/service/chatbot_service.py(3 hunks)app/chatbot_wordset/document/chatbot_wordset.py(1 hunks)app/chatbot_wordset/document/wordset_status.py(1 hunks)app/chatbot_wordset/exception/already_approved_wordset_exception.py(1 hunks)app/chatbot_wordset/repository/chatbot_wordset_repo.py(1 hunks)app/chatbot_wordset/service/chatbot_wordset_service.py(2 hunks)core/events/deps.py(1 hunks)requirements.txt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
app/chatbot_wordset/document/chatbot_wordset.py (2)
api/routers/chatbot_wordset.py (1)
chatbot_wordset(14-20)app/chatbot_wordset/document/wordset_status.py (1)
WordSetStatus(4-7)
app/chatbot/service/chatbot_service.py (1)
app/chatbot/exception/no_content_found_exception.py (1)
NoContentFoundException(4-8)
app/chatbot/repository/chatbot_repo.py (1)
app/chatbot/document/chatbot.py (2)
CharacterWordSet(7-10)ChatBot(12-28)
app/chatbot_wordset/service/chatbot_wordset_service.py (7)
api/routers/chatbot_wordset.py (4)
chatbot_wordset(14-20)approve_wordset(25-27)reject_wordset(32-34)delete_wordset(40-42)app/chatbot_wordset/document/wordset_status.py (1)
WordSetStatus(4-7)app/chatbot/document/chatbot.py (1)
CharacterWordSet(7-10)app/chatbot_wordset/exception/already_approved_wordset_exception.py (1)
AlreadyApprovedWordSetException(4-8)app/chatbot/repository/chatbot_repo.py (5)
find_by_id(21-25)add_wordset(36-44)remove_wordset(48-60)find(68-75)find_by_cursor_id(77-87)app/chatbot_wordset/repository/chatbot_wordset_repo.py (11)
find_by_id(70-72)update_status(75-80)delete_by_id(84-87)find_by_status(90-98)find_by_cursor_id_and_status(101-112)find(20-27)find_by_cursor_id(29-36)find_by_character_id_and_status(115-126)find_by_cursor_id_character_id_and_status(129-143)find_by_character_id(39-46)find_by_cursor_id_and_character_id(49-56)api/schemas/common/response/cursor_response.py (1)
CursorResponse(6-8)
app/chatbot_wordset/exception/already_approved_wordset_exception.py (1)
core/exceptions/business_exception.py (1)
BusinessException(1-5)
app/chatbot_wordset/repository/chatbot_wordset_repo.py (3)
app/chatbot/repository/chatbot_repo.py (2)
find_by_id(21-25)find(68-75)app/chatbot_wordset/document/chatbot_wordset.py (1)
ChatBotWordSet(6-14)app/chat_history/repository/chat_history_repo.py (1)
find(12-19)
app/chatbot/exception/no_content_found_exception.py (1)
core/exceptions/business_exception.py (1)
BusinessException(1-5)
api/routers/chatbot_wordset.py (5)
api/schemas/request/chatbot_wordset_request.py (1)
ChatBotWordIdsRequest(5-7)api/depends/get_user_id.py (1)
get_user_id(3-4)api/schemas/common/response/base_response.py (1)
BaseResponse(5-7)app/chatbot_wordset/service/chatbot_wordset_service.py (6)
add(10-11)approve_wordset(27-47)reject_wordset(50-54)delete_wordset(58-72)get_chatbot_wordset_by_character(20-23)get_chatbot_wordset_by_character_with_status(94-114)api/schemas/common/request/cursor_query.py (1)
CursorQuery(6-8)
🪛 markdownlint-cli2 (0.18.1)
.serena/memories/project_overview.md
47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.serena/memories/task_completion_checklist.md
37-37: Bare URL used
(MD034, no-bare-urls)
.serena/memories/suggested_commands.md
31-31: Bare URL used
(MD034, no-bare-urls)
32-32: Bare URL used
(MD034, no-bare-urls)
33-33: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.14.1)
app/chatbot/service/chatbot_service.py
193-193: f-string without any placeholders
Remove extraneous f prefix
(F541)
194-194: f-string without any placeholders
Remove extraneous f prefix
(F541)
195-195: f-string without any placeholders
Remove extraneous f prefix
(F541)
196-196: f-string without any placeholders
Remove extraneous f prefix
(F541)
199-199: Consider moving this statement to an else block
(TRY300)
204-204: f-string without any placeholders
Remove extraneous f prefix
(F541)
205-205: f-string without any placeholders
Remove extraneous f prefix
(F541)
206-206: f-string without any placeholders
Remove extraneous f prefix
(F541)
207-207: f-string without any placeholders
Remove extraneous f prefix
(F541)
208-208: f-string without any placeholders
Remove extraneous f prefix
(F541)
app/chatbot_wordset/service/chatbot_wordset_service.py
75-75: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
95-95: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
api/routers/chatbot_wordset.py
48-48: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (5)
api/routers/chatbot.py (1)
52-52: 라우트 경로 변경 확인라우트 경로가
@router.get("/")에서@router.get("")으로 변경되었습니다. 이는/chatbots/에서/chatbots로의 변경을 의미합니다. 변경 자체는 문제없으나, 기존 클라이언트에서 trailing slash를 사용하고 있다면 영향을 받을 수 있습니다.기존 API 클라이언트가 영향을 받지 않는지 확인하세요.
app/chatbot_wordset/document/wordset_status.py (1)
1-7: 깔끔한 enum 구현
WordSetStatusenum이 잘 구현되었습니다.str와Enum을 모두 상속하여 JSON 직렬화가 원활하며, 세 가지 상태(PENDING, APPROVED, REJECTED)가 명확하게 정의되어 있어 wordset 승인 워크플로우를 잘 표현하고 있습니다.Dockerfile (1)
7-26: Chrome/Chromium 설정 개선 사항 확인Chromium 관련 설정이 명시적으로 개선되었습니다:
- 필요한 라이브러리를 명시적으로 설치 (
libnss3,libxss1,libappindicator3-1,libasound2,libgbm1,xdg-utils)wget과unzip추가로 유연성 향상- 환경 변수를 명시적으로 설정하여 디버깅 용이성 개선
--headless=new플래그로 최신 headless 모드 사용--no-install-recommends와 apt 캐시 정리로 이미지 크기 최적화requirements.txt (1)
66-66: nadf 버전 업데이트 확인 완료검증 결과, nadf 패키지 0.1.30은 PyPI에서 유효한 버전이며, GitHub 보안 데이터베이스에 알려진 취약점이 없습니다. 다만 nadf 패키지의 구체적인 변경사항이나 Breaking Changes에 대한 공개 문서를 접근할 수 없어 코드베이스에서 사용하는 nadf의 API나 동작이 변경되었는지 직접 확인이 필요합니다.
- PyPI 확인: 0.1.28 → 0.1.30 버전 경로 확인 완료
- 보안: 등록된 CVE 없음
- 문서: 공개 릴리스 노트/변경로그 미접근
api/routers/chatbot_wordset.py (1)
14-20: 적절한 메시지 변경메시지가 "추가됨"에서 "생성 요청됨"으로 변경되어 워크플로우(신청 → 승인 → 적용)를 더 정확하게 반영합니다.
| @router.patch("/{wordset_id}/approve") | ||
| async def approve_wordset(wordset_id: str) -> BaseResponse: | ||
| await chatbot_wordset_service.approve_wordset(wordset_id) | ||
| return BaseResponse(message="chatbot wordset successfully approved") |
There was a problem hiding this comment.
인증 및 권한 검증 누락 (Critical)
관리자 전용 엔드포인트에 인증 및 권한 검증이 없습니다. 누구나 워드셋을 승인할 수 있어 심각한 보안 문제입니다.
관리자 권한 검증 의존성을 추가하세요:
+from api.depends.get_admin_user import get_admin_user # 관리자 검증 의존성 추가 필요
+
@router.patch("/{wordset_id}/approve")
-async def approve_wordset(wordset_id: str) -> BaseResponse:
+async def approve_wordset(
+ wordset_id: str,
+ admin_user: str = Depends(get_admin_user)
+) -> BaseResponse:Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api/routers/chatbot_wordset.py around lines 24 to 27, the approve_wordset
endpoint lacks authentication and admin authorization checks so anyone can call
it; add a dependency parameter that enforces authentication and admin role
(e.g., add Depends(get_current_user) and either check current_user.is_admin or
use a dedicated Depends(admin_required)) to the function signature, import the
required auth utilities, and if the check fails raise an HTTPException(401/403)
accordingly; keep the service call and response but ensure the endpoint only
executes for authenticated admins.
| @router.patch("/{wordset_id}/reject") | ||
| async def reject_wordset(wordset_id: str) -> BaseResponse: | ||
| await chatbot_wordset_service.reject_wordset(wordset_id) | ||
| return BaseResponse(message="chatbot wordset successfully rejected") |
There was a problem hiding this comment.
인증 및 권한 검증 누락 (Critical)
관리자 전용 엔드포인트에 인증 및 권한 검증이 없습니다. 누구나 워드셋을 거절할 수 있어 심각한 보안 문제입니다.
관리자 권한 검증 의존성을 추가하세요:
@router.patch("/{wordset_id}/reject")
-async def reject_wordset(wordset_id: str) -> BaseResponse:
+async def reject_wordset(
+ wordset_id: str,
+ admin_user: str = Depends(get_admin_user)
+) -> BaseResponse:Committable suggestion skipped: line range outside the PR's diff.
| @router.delete("/{wordset_id}") | ||
| async def delete_wordset(wordset_id: str) -> BaseResponse: | ||
| await chatbot_wordset_service.delete_wordset(wordset_id) | ||
| return BaseResponse(message="chatbot wordset successfully deleted") |
There was a problem hiding this comment.
🧩 Analysis chain
권한 검증 필요 여부 확인
삭제 엔드포인트에 권한 검증이 없습니다. 작성자 본인만 삭제 가능해야 하는지, 관리자만 가능한지, 아니면 둘 다 가능한지 확인이 필요합니다.
다음 스크립트로 서비스 레이어의 삭제 로직을 확인하여 권한 검증 요구사항을 파악하세요:
🏁 Script executed:
#!/bin/bash
# 삭제 엔드포인트의 권한 요구사항 확인
rg -nP -A10 "async def delete_wordset" app/chatbot_wordset/service/Length of output: 1013
추가 검증이 필요합니다. 다른 엔드포인트의 권한 검증 패턴과 이 함수의 설계 의도를 확인하겠습니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# 1. 다른 delete 엔드포인트의 권한 검증 패턴 확인
echo "=== 다른 router의 delete 엔드포인트 확인 ==="
rg -nP "@router\.delete" api/routers/ -A5
echo ""
echo "=== create_wordset 엔드포인트 확인 (권한 검증 여부) ==="
rg -nP "async def create_wordset|async def update_wordset" api/routers/chatbot_wordset.py -A3
echo ""
echo "=== 서비스 계층에서 user_id 사용 패턴 확인 ==="
rg -nP "user_id|owner_id|created_by" app/chatbot_wordset/service/ -B2 -A2
Length of output: 1973
더 깊은 검증을 진행하겠습니다. wordset 모델과 create 엔드포인트를 확인합니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# 1. Wordset 모델에서 user_id/owner_id 필드 확인
echo "=== Wordset 모델 스키마 확인 ==="
rg -nP "class.*Wordset|class.*WordSet" app/chatbot_wordset/ -A15 | head -60
echo ""
echo "=== create_wordset 엔드포인트 전체 확인 ==="
rg -nP "async def (create|add).*wordset" api/routers/chatbot_wordset.py -A5
echo ""
echo "=== add 서비스 메서드의 저장 로직 확인 ==="
rg -nP "async def save" app/chatbot_wordset/repository/ -A10
Length of output: 3349
마지막으로 add 메서드가 router에서 어떻게 호출되는지 확인하겠습니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# add 메서드를 호출하는 router 엔드포인트 확인
echo "=== add 메서드 호출 위치 ==="
rg -nP "chatbot_wordset_service\.add" api/routers/chatbot_wordset.py -B5 -A3
echo ""
echo "=== router의 모든 엔드포인트 확인 ==="
rg -nP "@router\.(post|put|get|delete)" api/routers/chatbot_wordset.py -A3 | head -40
Length of output: 946
권한 검증 누락 확인됨 - 보안 취약점
분석 결과:
- 생성 엔드포인트(POST):
user_id: str = Depends(get_user_id)로 user_id를 받아writer_id로 저장 - 삭제 엔드포인트(DELETE): user_id 파라미터 없음, 권한 검증 없음
- 모델:
ChatBotWordSet에writer_id: str필드로 소유권 추적 가능
필수 수정 사항:
delete_wordset엔드포인트에user_id: str = Depends(get_user_id)추가- 서비스 계층에
writer_id == user_id검증 추가 (또는 관리자 권한 별도 고려)
현재 상태로는 누구든지 다른 사용자의 wordset을 삭제할 수 있습니다.
🤖 Prompt for AI Agents
In api/routers/chatbot_wordset.py around lines 39-42, the DELETE endpoint lacks
authorization: add a user_id parameter wired to Depends(get_user_id) to obtain
the caller's ID, pass that user_id into the
chatbot_wordset_service.delete_wordset call, and import get_user_id; then update
the service layer delete_wordset(wordset_id, user_id) to fetch the wordset's
writer_id and verify writer_id == user_id (or allow if the user has admin role),
raising an appropriate 403 HTTPException on mismatch before performing deletion.
| async def get_chatbot_wordset_by_character( | ||
| character_id: int, | ||
| params: CursorQuery = Depends(), | ||
| status: Optional[str] = Query(None, description="PENDING, APPROVED, REJECTED") | ||
| ) -> BaseResponse: | ||
| chatbot_response = await chatbot_wordset_service.get_chatbot_wordset_by_character_with_status( | ||
| character_id, params.cursor_id, params.size, status | ||
| ) | ||
| return BaseResponse(message="chatbot wordset successfully retrieved by character", data=chatbot_response) |
There was a problem hiding this comment.
status 매개변수 검증 누락
status 쿼리 매개변수가 임의의 문자열을 허용합니다. WordSetStatus 열거형 값으로 제한해야 합니다.
다음 diff를 적용하여 열거형 검증을 추가하세요:
+from app.chatbot_wordset.document.wordset_status import WordSetStatus
+
@router.get("/{character_id}")
async def get_chatbot_wordset_by_character(
character_id: int,
params: CursorQuery = Depends(),
- status: Optional[str] = Query(None, description="PENDING, APPROVED, REJECTED")
+ status: Optional[WordSetStatus] = Query(None, description="워드셋 상태 필터")
) -> BaseResponse:🧰 Tools
🪛 Ruff (0.14.1)
48-48: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
In api/routers/chatbot_wordset.py around lines 46 to 54, the status query
parameter currently accepts any string; change its type to the WordSetStatus
enum to enforce valid values (import WordSetStatus), update the Query annotation
to Optional[WordSetStatus] = Query(None, description="PENDING, APPROVED,
REJECTED"), and when calling the service pass status.value (or None) so the
service receives the enum value string; ensure necessary import for
WordSetStatus is added at the top of the file.
| async def find_by_id(wordset_id: str) -> ChatBotWordSet: | ||
| """특정 ID로 wordset 조회""" | ||
| return await ChatBotWordSet.get(PydanticObjectId(wordset_id)) |
There was a problem hiding this comment.
None 반환 처리 필요
ChatBotWordSet.get()은 문서를 찾지 못하면 None을 반환합니다. 이 경우 적절한 예외를 발생시켜야 합니다.
다음 diff를 적용하여 예외 처리를 추가하세요:
async def find_by_id(wordset_id: str) -> ChatBotWordSet:
"""특정 ID로 wordset 조회"""
- return await ChatBotWordSet.get(PydanticObjectId(wordset_id))
+ wordset = await ChatBotWordSet.get(PydanticObjectId(wordset_id))
+ if wordset is None:
+ raise ValueError(f"WordSet not found: {wordset_id}")
+ return wordset📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def find_by_id(wordset_id: str) -> ChatBotWordSet: | |
| """특정 ID로 wordset 조회""" | |
| return await ChatBotWordSet.get(PydanticObjectId(wordset_id)) | |
| async def find_by_id(wordset_id: str) -> ChatBotWordSet: | |
| """특정 ID로 wordset 조회""" | |
| wordset = await ChatBotWordSet.get(PydanticObjectId(wordset_id)) | |
| if wordset is None: | |
| raise ValueError(f"WordSet not found: {wordset_id}") | |
| return wordset |
🤖 Prompt for AI Agents
In app/chatbot_wordset/repository/chatbot_wordset_repo.py around lines 70-72,
ChatBotWordSet.get(...) can return None when no document is found; update the
function to capture the result into a variable, check if it's None, and if so
raise a clear exception (for example ValueError or a custom NotFoundError)
including the wordset_id; otherwise return the found object.
| async def find_by_cursor_id_and_status(cursor_id: int, status: str, size: int) -> List[ChatBotWordSet]: | ||
| """커서 ID와 상태로 wordset 조회""" | ||
| chatbot_wordsets = ( | ||
| await ChatBotWordSet.find( | ||
| oper.LT(ChatBotWordSet.id, cursor_id), | ||
| oper.Eq(ChatBotWordSet.status, status) | ||
| ) | ||
| .sort(("_id", SortDirection.DESCENDING)) | ||
| .limit(size + 1) | ||
| .to_list() | ||
| ) | ||
| return chatbot_wordsets |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
status 매개변수에 WordSetStatus 열거형 사용 필요
타입 안정성을 위해 str 대신 WordSetStatus 열거형을 사용하세요.
-async def find_by_cursor_id_and_status(cursor_id: int, status: str, size: int) -> List[ChatBotWordSet]:
+async def find_by_cursor_id_and_status(cursor_id: int, status: WordSetStatus, size: int) -> List[ChatBotWordSet]:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def find_by_cursor_id_and_status(cursor_id: int, status: str, size: int) -> List[ChatBotWordSet]: | |
| """커서 ID와 상태로 wordset 조회""" | |
| chatbot_wordsets = ( | |
| await ChatBotWordSet.find( | |
| oper.LT(ChatBotWordSet.id, cursor_id), | |
| oper.Eq(ChatBotWordSet.status, status) | |
| ) | |
| .sort(("_id", SortDirection.DESCENDING)) | |
| .limit(size + 1) | |
| .to_list() | |
| ) | |
| return chatbot_wordsets | |
| async def find_by_cursor_id_and_status(cursor_id: int, status: WordSetStatus, size: int) -> List[ChatBotWordSet]: | |
| """커서 ID와 상태로 wordset 조회""" | |
| chatbot_wordsets = ( | |
| await ChatBotWordSet.find( | |
| oper.LT(ChatBotWordSet.id, cursor_id), | |
| oper.Eq(ChatBotWordSet.status, status) | |
| ) | |
| .sort(("_id", SortDirection.DESCENDING)) | |
| .limit(size + 1) | |
| .to_list() | |
| ) | |
| return chatbot_wordsets |
🤖 Prompt for AI Agents
In app/chatbot_wordset/repository/chatbot_wordset_repo.py around lines 101 to
112, the function parameter `status: str` should be changed to use the
WordSetStatus enum for type safety; update the signature to accept `status:
WordSetStatus`, import WordSetStatus at the top of the file, and when building
the query compare the model field to the enum value (e.g., use `status.value` or
the appropriate enum member representation expected by the DB) so the query
remains correct; ensure any call sites are updated to pass a WordSetStatus
instead of a raw string.
| async def find_by_character_id_and_status(character_id: int, status: str, size: int) -> List[ChatBotWordSet]: | ||
| """캐릭터 ID와 상태로 wordset 조회""" | ||
| chatbot_wordsets = ( | ||
| await ChatBotWordSet.find( | ||
| oper.Eq(ChatBotWordSet.character_id, character_id), | ||
| oper.Eq(ChatBotWordSet.status, status) | ||
| ) | ||
| .sort(("_id", SortDirection.DESCENDING)) | ||
| .limit(size + 1) | ||
| .to_list() | ||
| ) | ||
| return chatbot_wordsets |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
status 매개변수에 WordSetStatus 열거형 사용 필요
타입 안정성을 위해 str 대신 WordSetStatus 열거형을 사용하세요.
-async def find_by_character_id_and_status(character_id: int, status: str, size: int) -> List[ChatBotWordSet]:
+async def find_by_character_id_and_status(character_id: int, status: WordSetStatus, size: int) -> List[ChatBotWordSet]:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def find_by_character_id_and_status(character_id: int, status: str, size: int) -> List[ChatBotWordSet]: | |
| """캐릭터 ID와 상태로 wordset 조회""" | |
| chatbot_wordsets = ( | |
| await ChatBotWordSet.find( | |
| oper.Eq(ChatBotWordSet.character_id, character_id), | |
| oper.Eq(ChatBotWordSet.status, status) | |
| ) | |
| .sort(("_id", SortDirection.DESCENDING)) | |
| .limit(size + 1) | |
| .to_list() | |
| ) | |
| return chatbot_wordsets | |
| async def find_by_character_id_and_status(character_id: int, status: WordSetStatus, size: int) -> List[ChatBotWordSet]: | |
| """캐릭터 ID와 상태로 wordset 조회""" | |
| chatbot_wordsets = ( | |
| await ChatBotWordSet.find( | |
| oper.Eq(ChatBotWordSet.character_id, character_id), | |
| oper.Eq(ChatBotWordSet.status, status) | |
| ) | |
| .sort(("_id", SortDirection.DESCENDING)) | |
| .limit(size + 1) | |
| .to_list() | |
| ) | |
| return chatbot_wordsets |
🤖 Prompt for AI Agents
In app/chatbot_wordset/repository/chatbot_wordset_repo.py around lines 115-126,
change the function signature to accept WordSetStatus instead of str for the
status parameter, add the required import for WordSetStatus at the top of the
file, update the DB filter to compare using status.value (or cast to the stored
representation) when calling oper.Eq(ChatBotWordSet.status, ...), update the
docstring to reflect the enum type, and adjust any callers/tests to pass a
WordSetStatus enum instance rather than a raw string.
| async def find_by_cursor_id_character_id_and_status( | ||
| character_id: int, cursor_id: int, status: str, size: int | ||
| ) -> List[ChatBotWordSet]: | ||
| """커서 ID, 캐릭터 ID, 상태로 wordset 조회""" | ||
| chatbot_wordsets = ( | ||
| await ChatBotWordSet.find( | ||
| oper.LT(ChatBotWordSet.id, cursor_id), | ||
| oper.Eq(ChatBotWordSet.character_id, character_id), | ||
| oper.Eq(ChatBotWordSet.status, status) | ||
| ) | ||
| .sort(("_id", SortDirection.DESCENDING)) | ||
| .limit(size + 1) | ||
| .to_list() | ||
| ) | ||
| return chatbot_wordsets |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
status 매개변수에 WordSetStatus 열거형 사용 필요
타입 안정성을 위해 str 대신 WordSetStatus 열거형을 사용하세요.
async def find_by_cursor_id_character_id_and_status(
- character_id: int, cursor_id: int, status: str, size: int
+ character_id: int, cursor_id: int, status: WordSetStatus, size: int
) -> List[ChatBotWordSet]:Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/chatbot_wordset/repository/chatbot_wordset_repo.py around lines 129 to
143, the function currently types the status parameter as str; change the
signature to accept status: WordSetStatus (import WordSetStatus at top), and
when building the query pass the underlying string value stored in the DB (e.g.,
status.value or status.name depending on how the enum is persisted) instead of
the enum object; update any callers if needed to pass a WordSetStatus instance
and keep the rest of the query logic the same.
| async def get_chatbot_wordset_with_status(cursor_id: int, size: int, status: str = None) -> CursorResponse: | ||
| """상태별 wordset 조회""" | ||
| if status: | ||
| chatbot_wordset = ( | ||
| await chatbot_wordset_repo.find_by_status(status, size) | ||
| if cursor_id is None | ||
| else await chatbot_wordset_repo.find_by_cursor_id_and_status(cursor_id, status, size) | ||
| ) | ||
| else: | ||
| chatbot_wordset = ( | ||
| await chatbot_wordset_repo.find(size) | ||
| if cursor_id is None | ||
| else await chatbot_wordset_repo.find_by_cursor_id(cursor_id, size) | ||
| ) | ||
|
|
||
| has_next = len(chatbot_wordset) > size | ||
| return CursorResponse(hasNext=has_next, values=chatbot_wordset[:size]) | ||
|
|
||
|
|
||
| async def get_chatbot_wordset_by_character_with_status( | ||
| character_id: int, cursor_id: int, size: int, status: str = None | ||
| ) -> CursorResponse: | ||
| """캐릭터별, 상태별 wordset 조회""" | ||
| if status: | ||
| chatbot_wordset = ( | ||
| await chatbot_wordset_repo.find_by_character_id_and_status(character_id, status, size) | ||
| if cursor_id is None | ||
| else await chatbot_wordset_repo.find_by_cursor_id_character_id_and_status( | ||
| character_id, cursor_id, status, size | ||
| ) | ||
| ) | ||
| else: | ||
| chatbot_wordset = ( | ||
| await chatbot_wordset_repo.find_by_character_id(character_id, size) | ||
| if cursor_id is None | ||
| else await chatbot_wordset_repo.find_by_cursor_id_and_character_id(character_id, cursor_id, size) | ||
| ) | ||
|
|
||
| has_next = len(chatbot_wordset) > size | ||
| return CursorResponse(hasNext=has_next, values=chatbot_wordset[:size]) |
There was a problem hiding this comment.
🧩 Analysis chain
cursor_id 타입 정합성 확인 요청.
ChatBotWordSet는 Beanie 기본 _id(ObjectId) 사용으로 보입니다. 서비스 시그니처의 cursor_id: int는 불일치 가능성이 큽니다. str | None 혹은 PydanticObjectId | None로의 변경이 필요할 수 있습니다.
다음 스크립트로 실제 ID 타입 사용 위치를 확인해 주세요:
🏁 Script executed:
#!/bin/bash
# ChatBotWordSet의 id 타입 및 커서 사용 패턴 점검
rg -nC2 'class ChatBotWordSet\('
rg -nC2 '\bChatBotWordSet\.(id|_id)\b'
rg -nC2 'PydanticObjectId|ObjectId' app | sed -n '1,200p'
rg -nC2 'find_by_cursor_id' app/chatbot_wordset/repositoryLength of output: 7714
cursor_id 타입 불일치 확인됨. 매개변수는 int이나 ObjectId 필드와 직접 비교됨.
검증 결과, 타입 정합성 문제가 실제로 존재합니다:
- Repository 메서드들(
find_by_cursor_id,find_by_cursor_id_and_character_id,find_by_cursor_id_and_status등)의 시그니처에서cursor_id: int로 정의됨 - 그러나 실제 쿼리에서
oper.LT(ChatBotWordSet.id, cursor_id)로 ObjectId 필드와 int 값을 직접 비교 - 같은 프로젝트 내
chat_history_repo.py에서는 올바르게ObjectId(cursor_id)변환 사용 (line 25) find_by_id(wordset_id: str)에서도PydanticObjectId(wordset_id)변환 패턴 사용 (line 72)
서비스 시그니처 수정 필요:
cursor_id: int→cursor_id: str | None또는cursor_id: PydanticObjectId | None- 또는 repository 함수들에서 ObjectId 변환 로직 추가 필요
현재 코드 상태로는 MongoDB 커서 페이지네이션이 의도대로 작동하지 않을 수 있습니다.
🧰 Tools
🪛 Ruff (0.14.1)
75-75: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
95-95: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🤖 Prompt for AI Agents
In app/chatbot_wordset/service/chatbot_wordset_service.py around lines 75-114,
the service functions accept cursor_id: int but compare it against MongoDB
ObjectId fields; change the cursor_id parameter type to Optional[str] or
Optional[PydanticObjectId] and convert the incoming string ID to an ObjectId (or
PydanticObjectId) before calling repository methods, or alternatively update the
repo calls to perform the conversion; specifically, update both function
signatures to accept cursor_id: str | None (or PydanticObjectId | None), add a
conversion step when cursor_id is not None (e.g., oid =
PydanticObjectId(cursor_id) or ObjectId(cursor_id)), pass oid into the existing
find_by_cursor_* repo calls, and add the necessary import for
PydanticObjectId/ObjectId and adjust type hints accordingly.
| async def add_wordset(character_id: int, character_wordset: CharacterWordSet, contributor: str): | ||
| """챗봇에 wordset 추가""" | ||
| character = await find_by_id(character_id) | ||
|
|
||
| # wordset 추가 | ||
| await character.update( | ||
| oper.Push({ChatBot.character_wordset: character_wordset}), | ||
| oper.AddToSet({ChatBot.contributors: contributor}) | ||
| ) |
There was a problem hiding this comment.
검증 우회로 인한 무결성 위험 + 직렬화 명시 + 중복 방지.
oper.Push는 모델 밸리데이터(최대 5개 제한)를 우회합니다. 검증 없이 6개 이상을 넣어 DB가 “유효하지 않은” 상태가 되면, 이후 로드 시 검증 오류로 터질 수 있습니다.CharacterWordSet를 그대로 푸시하기보다 dict로 직렬화가 안전합니다.- 중복 방지를 위해
AddToSet사용이 바람직합니다.
다음과 같이 길이 체크와 직렬화, 중복 방지를 적용하세요(경쟁 조건 완전 방지는 별도 원자적 조건 업데이트가 필요):
async def add_wordset(character_id: int, character_wordset: CharacterWordSet, contributor: str):
"""챗봇에 wordset 추가"""
character = await find_by_id(character_id)
-
- # wordset 추가
- await character.update(
- oper.Push({ChatBot.character_wordset: character_wordset}),
- oper.AddToSet({ChatBot.contributors: contributor})
- )
+ # 길이 제한(최대 5개) 검증 — 모델 밸리데이터 우회 방지
+ if len(character.character_wordset) >= 5:
+ # CharacterWordSetLengthExceededException 재사용 권장
+ from app.chatbot.exception.character_wordset_length_exceeded_exception import CharacterWordSetLengthExceededException
+ raise CharacterWordSetLengthExceededException(length=len(character.character_wordset))
+
+ # 직렬화 + 중복 방지
+ await character.update(
+ oper.AddToSet({ChatBot.character_wordset: character_wordset.model_dump()}),
+ oper.AddToSet({ChatBot.contributors: contributor}),
+ )참고: 완전한 경쟁 조건 방지를 원하면 $expr로 size < 5 조건을 포함한 단일 원자적 업데이트로 바꾸는 방안을 검토하세요.
Based on learnings
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/chatbot/repository/chatbot_repo.py around lines 36-44, the current async
add_wordset uses oper.Push with a CharacterWordSet object which bypasses model
validators and risks exceeding the 5-item limit; instead, serialize
CharacterWordSet to a plain dict, check the current character.character_wordset
length (or fetch a fresh size) and only push if length < 5, and keep using
AddToSet for contributors to avoid duplicates; if you need atomic protection
against races, replace this two-step logic with a single atomic update using an
$expr condition (e.g., require size < 5) so the insert and length check happen
server-side.
1 participant
wordset 추가 로직 수정
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
인프라
문서