Feat(#17): 문자 분석 통합 엔드포인트 및 3중 가중치 스코어링 시스템 구현 - #18
Conversation
…sly in ScanService
📝 WalkthroughWalkthroughThe ChangesSmishing analysis
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AnalyzeRouter
participant ScanService
participant GeminiTextAnalyzer
participant HybridUrlEngine
participant ScoringEngine
Client->>AnalyzeRouter: POST /api/analyze
AnalyzeRouter->>ScanService: analyze_pipeline(text)
ScanService->>GeminiTextAnalyzer: analyze text
ScanService->>HybridUrlEngine: scan traced URL
ScanService->>ScoringEngine: calculate score
ScoringEngine-->>ScanService: score, grade, breakdown
ScanService-->>AnalyzeRouter: SmishingAnalysisResponse
AnalyzeRouter-->>Client: analysis response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/router/analyze.py (1)
26-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRouter error path is largely unreachable.
analyze_pipelinealready wraps its body intry/except Exceptionand returns aSmishingAnalysisResponse(status="ERROR", ...)(seeapp/service/scan_service.pyLines 82-92), so thisexcept/HTTPException(500)rarely fires and pipeline failures surface as HTTP 200 withstatus="ERROR"rather than a 5xx. Confirm this is the intended contract for the frontend/Spring Boot client; if a 5xx is expected on failure, the pipeline should re-raise instead of swallowing. Minor: addfrom eto the raise to satisfy B904.🤖 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 `@app/router/analyze.py` around lines 26 - 33, Align the failure contract between the router and analyze_pipeline: update analyze_pipeline so pipeline exceptions are re-raised when failures must produce HTTP 5xx responses, allowing the analyze router’s HTTPException path to execute. Preserve the existing error response only if HTTP 200 with status="ERROR" is the intended client contract; otherwise remove that swallowing behavior and chain the router’s HTTPException with “from e”.
🤖 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 `@app/main.py`:
- Line 11: Update the CORS middleware configuration below the Spring
Boot/frontend integration comment to replace allow_origins=["*"] with an
explicit allowlist containing the frontend and Spring Boot hosts, and replace
wildcard allow_methods and allow_headers with the required enumerated values
while retaining credentials only for trusted origins.
In `@app/utils/scoring_engine.py`:
- Line 26: Clamp the value assigned to llm_contrib in the scoring calculation to
the declared 0–50 range, matching the existing bound enforced by
ContributionBreakdown.llm. Preserve the current scaling for valid llm_score
values while preventing out-of-range scores from producing a contribution above
50.
---
Nitpick comments:
In `@app/router/analyze.py`:
- Around line 26-33: Align the failure contract between the router and
analyze_pipeline: update analyze_pipeline so pipeline exceptions are re-raised
when failures must produce HTTP 5xx responses, allowing the analyze router’s
HTTPException path to execute. Preserve the existing error response only if HTTP
200 with status="ERROR" is the intended client contract; otherwise remove that
swallowing behavior and chain the router’s HTTPException with “from e”.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fa8760b3-4415-4e2c-b12d-70c421919ccc
📒 Files selected for processing (6)
app/dto/schemas.pyapp/main.pyapp/router/analyze.pyapp/service/scan_service.pyapp/service/security/hybrid_url_engine.pyapp/utils/scoring_engine.py
| version=settings.VERSION | ||
| ) | ||
|
|
||
| # Spring Boot 및 프론트엔드 연동을 위한 CORS 미들웨어 설정 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Insecure CORS: wildcard origin combined with credentials.
The CORS middleware configured below (per the static-analysis hint) pairs allow_origins=["*"] with allow_credentials=True and wildcard methods/headers. With credentials enabled, Starlette/FastAPI reflects the request Origin, effectively allowing any site to make authenticated cross-origin requests (CWE-942 / OWASP A05:2021). Replace the wildcard with an explicit origin allowlist (your frontend and Spring Boot hosts) and enumerate allowed methods/headers when credentials are enabled.
🔒️ Suggested direction
- allow_origins=["*"],
+ allow_origins=["https://your-frontend.example", "https://your-springboot.example"],
allow_credentials=True,
- allow_methods=["*"],
- allow_headers=["*"],
+ allow_methods=["GET", "POST", "OPTIONS"],
+ allow_headers=["Content-Type", "Authorization"],🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 11-17: CORSMiddleware allows credentials together with a wildcard origin, methods, or headers, which lets any site issue authenticated cross-origin requests. Use an explicit origin allowlist and enumerate the allowed methods/headers when allow_credentials=True.
Context: app.add_middleware(
CORSMiddleware,
allow_origins=[""],
allow_credentials=True,
allow_methods=[""],
allow_headers=["*"],
)
Note: [CWE-942] Permissive Cross-domain Policy with Untrusted Domains. OWASP A05:2021 Security Misconfiguration. Starlette/FastAPI CORSMiddleware reflects the request Origin when credentials are enabled, so pairing allow_credentials=True with a wildcard origin/methods/headers exposes authenticated endpoints to any site.
(starlette-cors-credentials-wildcard-python)
🤖 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 `@app/main.py` at line 11, Update the CORS middleware configuration below the
Spring Boot/frontend integration comment to replace allow_origins=["*"] with an
explicit allowlist containing the frontend and Spring Boot hosts, and replace
wildcard allow_methods and allow_headers with the required enumerated values
while retaining credentials only for trusted origins.
Source: Linters/SAST tools
| # 텍스트 문맥 점수와 하이브리드 URL 엔진의 결과값을 결합하여 최종 위험도를 산출 | ||
|
|
||
| # 1. LLM 문맥 분석 기여 점수 (최대 50점) | ||
| llm_contrib = round(llm_score * 0.5) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clamp llm_contrib to its declared bound (0–50).
Unlike url_contrib (clamped to 30), llm_contrib is not bounded. If llm_score ever exceeds 100 (e.g. an out-of-range Gemini risk_score), llm_contrib > 50 will fail the ContributionBreakdown.llm le=50 constraint at Line 54, raising a ValidationError that collapses the whole pipeline into the ERROR fallback and discards all analysis.
🛡️ Proposed fix
- llm_contrib = round(llm_score * 0.5)
+ llm_contrib = min(max(round(llm_score * 0.5), 0), 50)📝 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.
| llm_contrib = round(llm_score * 0.5) | |
| llm_contrib = min(max(round(llm_score * 0.5), 0), 50) |
🤖 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 `@app/utils/scoring_engine.py` at line 26, Clamp the value assigned to
llm_contrib in the scoring calculation to the declared 0–50 range, matching the
existing bound enforced by ContributionBreakdown.llm. Preserve the current
scaling for valid llm_score values while preventing out-of-range scores from
producing a contribution above 50.
📝 개요
텍스트 컨텍스트 분석(Gemini)과 URL 하이브리드 검사 파이프라인(VT+GSB)의 결과물을 단일 진입점으로 결합하는 통합 문자 분석 엔드포인트를 구축합니다.
또한, 탐지 결과의 신뢰도를 극대화하고 프론트엔드 원형 게이지 UI에 대응하기 위해 [LLM 50% / VirusTotal 30% / 규칙 기반 20%] 가중치를 합성하는 3중 스코어링 시스템 엔진을 설계하고 백엔드 응답 스키마를 확장합니다.
🔗 관련 이슈
🎯 주요 변경 사항
app/main.py,app/router/analyze.py)/api/analyze**로 최종 엔드포인트 경로를 확정했습니다.app/dto/schemas.py)URLScanRequest및SmishingAnalysisResponse구조로 DTO 명칭과 필드 구조를 완벽하게 동기화했습니다.text로 단일화하여 데이터 변환 오버헤드를 제거했습니다.app/service/scan_service.py)asyncio.gather를 통한 텍스트 트랙(Gemini)과 URL 트랙(Hybrid Engine)의 비동기 병렬 처리 공정을 정교화했습니다.app/utils/scoring_engine.py)ScoringEngine.calculate_score메서드 호출 시 런타임 에러를 유발하던 파라미터 오타(is_url_maliciout->is_url_malicious)를 수정했습니다.app/service/security/gemini_text_analyzer.py)MOCK_SECURITY_API(Sandbox Mode) 설정을 해제하여 실제 Gemini AI가 문자 메시지 문맥을 실시간으로 독해하고 위험도 점수 및 한글 분석 사유를 생성하도록 연동 완료했습니다.📸 사진
✅ PR 체크리스트
uvicorn구동 또는 테스트 코드)를 통과했습니다.Summary by CodeRabbit
New Features
Improvements
/api.