Skip to content

Feat(#12): URL 검사 엔드포인트 구현 및 하이브리드 검사 파이프라인 조립 - #16

Merged
pearseona merged 7 commits into
developfrom
feat/12-url-scan-pipeline
Jul 20, 2026
Merged

Feat(#12): URL 검사 엔드포인트 구현 및 하이브리드 검사 파이프라인 조립#16
pearseona merged 7 commits into
developfrom
feat/12-url-scan-pipeline

Conversation

@pearseona

@pearseona pearseona commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

📝 개요

Spring Boot 메인 서버와 연동하기 위한 독립된 URL 검사 API 엔드포인트를 구축했습니다.
문자 본문 내 URL 추출부터 단축 URL의 실제 주소 추적, 그리고 VirusTotalGoogle Safe Browsing API의 비동기 병렬 호출 및 동기화까지의 전체 분석 파이프라인을 조립하여 실시간 프로던셕(Real-API) 모드를 완성했습니다.

🔗 관련 이슈

🎯 주요 변경 사항

Spring Boot 통신용 URL 검사 요청/응답 DTO 및 엔드포인트 구축

  • Spring Boot 서버와 안정적으로 데이터를 주고받을 수 있는 규격화된 URLScanResponse Pydantic 응답 스키마 정의

URL 추출 및 단축 URL 실제 주소 추적 모듈 구현

  • 정규식(Regex)을 활용한 문자 본문 내 실시간 URL 추출 모듈 구현
  • bit.ly 등 단축 URL 뒤에 숨겨진 실제 악성 행선지 주소를 끝까지 추적하는 비동기 리다이렉트 헬퍼 함수(trace_url) 매핑

하이브리드 URL 검사 비동기 파이프라인 및 예외 처리 가드 구축

  • VirusTotalGoogle Safe Browsing 인프라를 6.0초 타임아웃 제한 내에서 비동기 호출하도록 처리
  • 외부 API 통신 실패, 네트워크 타임아웃, 할당량 초과 등의 예외 상황에서도 시스템이 디폴트 응답을 반환하도록 설계

📸 사진

✅ PR 체크리스트

  • 관련 이슈를 연결했습니다.
  • 구현 범위와 변경 이유를 설명했습니다.
  • 로컬 테스트(uvicorn 구동 또는 테스트 코드)를 통과했습니다.
  • API 변경 사항이 있다면 Swagger / API 명세에 반영했습니다.
  • 민감 정보(API Key, 시크릿 키 등)가 코드·로그·테스트 데이터에 포함되지 않았습니다.
  • 프론트엔드 또는 메인 백엔드(Spring)에 영향을 주는 응답 스키마 또는 Enum 변경이 있다면 팀에 공유했습니다.
  • 병합(Merge) 전 작업 브랜치를 삭제하지 않았습니다.

Summary by CodeRabbit

  • New Features

    • Added URL detection, redirect tracing, and malicious-link risk analysis to message scanning.
    • Combined message-content analysis with URL security results for more comprehensive smishing detection.
    • Added standardized scan results including risk scores, detection status, traced URLs, and error details.
    • Added support for VirusTotal and Google Safe Browsing checks.
    • Enabled cross-origin access for API clients.
  • Bug Fixes

    • Improved handling of security-service timeouts, errors, and unavailable results.
    • Removed placeholder voice-analysis responses that returned dummy results.

@pearseona pearseona self-assigned this Jul 19, 2026
@pearseona pearseona added the feat New feature or functional additions to the application label Jul 19, 2026
@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

The PR adds URL scanning contracts and utilities, refactors security integrations into engine classes, introduces hybrid URL analysis, and integrates URL maliciousness into the /analyze smishing response. It also adds environment-backed API keys, permissive CORS, and workspace settings.

URL and smishing analysis

Layer / File(s) Summary
Contracts, configuration, and URL tracking
app/core/config.py, app/dto/schemas.py, app/service/security/base.py, app/utils/url_tracker.py, .vscode/settings.json
Security keys, URL scan DTOs, an abstract engine contract, URL extraction, redirect tracing, and workspace Python settings are added.
Security engine implementations
app/service/security/gemini_text_analyzer.py, app/service/security/google_safe_browsing.py, app/service/security/mock_provider.py, app/service/security/virustotal.py
Gemini mock and prompt behavior are updated, and VirusTotal, Google Safe Browsing, and mock URL scanning return normalized engine results.
Hybrid message scanning
app/service/scan_service.py
ScanService extracts and traces URLs, selects mock or production scanning, combines engine results, computes risk, applies infrastructure rules, and returns URLScanResponse.
Analyze endpoint and application wiring
app/router/analyze.py, app/main.py
The /analyze endpoint injects ScanService, combines Gemini and URL results, handles exceptions, and enables permissive CORS.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AnalyzeEndpoint
  participant ScanService
  participant URLTracker
  participant VirusTotalEngine
  participant GoogleSafeBrowsingEngine
  AnalyzeEndpoint->>ScanService: scan_message_text(message)
  ScanService->>URLTracker: extract_urls(message)
  ScanService->>URLTracker: trace_url(original_url)
  ScanService->>VirusTotalEngine: scan_url(traced_url)
  ScanService->>GoogleSafeBrowsingEngine: scan_url(traced_url)
  VirusTotalEngine-->>ScanService: normalized result
  GoogleSafeBrowsingEngine-->>ScanService: normalized result
  ScanService-->>AnalyzeEndpoint: URLScanResponse
Loading

Possibly related issues

Possibly related PRs

  • SafeFam/SafeFam_AI#9 — Earlier URL extraction and redirect-tracing implementation replaced by app/utils/url_tracker.py.
  • SafeFam/SafeFam_AI#11 — Related VirusTotal and Google Safe Browsing hybrid scanning logic refactored into engine classes here.
  • SafeFam/SafeFam_AI#14 — Related /analyze and Gemini text-analysis integration changes.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.33% 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
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: a URL scan endpoint and a hybrid security inspection pipeline.
✨ 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 feat/12-url-scan-pipeline

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

@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: 8

🧹 Nitpick comments (5)
app/dto/schemas.py (1)

19-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prefer model_config = ConfigDict(json_schema_extra=...) over the deprecated class Config.

Pydantic v2 still honors the inner Config class but marks it deprecated; migrating avoids future breakage and silences the RUF012 false positive on this attribute.

🤖 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/dto/schemas.py` around lines 19 - 31, The schema’s inner Config class is
deprecated under Pydantic v2. Replace it with a model_config assignment using
ConfigDict and preserve the existing json_schema_extra example unchanged,
including its field values.

Source: Linters/SAST tools

app/service/scan_service.py (2)

80-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace print debug dumps with logging.

These print statements dump raw engine responses to stdout on every request—noisy and inconsistent with the module logger. Use logger.debug(...) (or remove) before release.

🤖 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/service/scan_service.py` around lines 80 - 83, Replace the four raw
response print statements in the scan response output block with the module’s
logger.debug calls, preserving the VirusTotal and Safe Browsing response details
while avoiding unconditional stdout output. Remove the separator prints or
include equivalent context in a single debug log.

87-97: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

"error" in str(vt_result) is a fragile way to detect engine failures.

Stringifying the whole result and substring-checking for error can both miss and misfire (e.g. a URL/status text containing "error"). Since the engines return exceptions (via return_exceptions=True) or a normalized dict, prefer explicit checks: isinstance(x, Exception) or x.get("error")/"error" in x on the dict directly.

🤖 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/service/scan_service.py` around lines 87 - 97, Update the VirusTotal and
Google Safe Browsing failure checks in the scan flow to avoid substring matching
against str(vt_result) or str(gsb_result). Detect failures only when the result
is an Exception or a dictionary containing an "error" key, while preserving the
existing error logging, fallback results, and error_logs updates.
.vscode/settings.json (1)

2-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Windows-only interpreter paths in committed editor config.

.venv/Scripts/python.exe and .venv/Lib/site-packages are Windows layouts; on macOS/Linux these are .venv/bin/python and .venv/lib/pythonX.Y/site-packages. Committing machine-specific .vscode/settings.json will misconfigure other contributors—consider gitignoring it.

🤖 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 @.vscode/settings.json around lines 2 - 5, Remove the machine-specific Python
interpreter and analysis paths from the committed .vscode/settings.json, or move
them into a user-local ignored configuration so contributors on macOS, Linux,
and Windows are not affected. Preserve only shared editor settings in version
control and ensure the workspace no longer forces the Windows-specific .venv
paths.
app/core/config.py (1)

9-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Drop the deprecated env= kwargs

pydantic-settings v2 ignores Field(..., env=...); these values are already picked up from the matching field names. Remove the extra kwargs, or switch to validation_alias only if the env var names need to differ.

🤖 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/core/config.py` around lines 9 - 10, Update the VIRUSTOTAL_API_KEY and
GOOGLE_SAFE_BROWSING_API_KEY field declarations to remove the deprecated env=
arguments, relying on their matching field names for environment-variable
loading; use validation_alias only if either environment name differs from its
field name.
🤖 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`:
- Around line 11-17: Update the CORSMiddleware configuration to use explicit
known gateway origin(s) instead of allow_origins=["*"] while retaining
allow_credentials=True, or remove allow_credentials when credentialed requests
are not required. Keep the existing methods and headers configuration unchanged.

In `@app/router/analyze.py`:
- Around line 48-49: Update the exception handler in the analyze route to log
the caught exception server-side instead of interpolating str(e) into the client
response. Return a generic Korean error message without internal details, and
configure ApiResponse.error or the route response handling to signal failure
with an appropriate non-200 status code while preserving the existing success
behavior.

In `@app/service/scan_service.py`:
- Around line 117-120: Replace the broad ".ru" substring check in the malicious
URL guard with a parsed-host suffix check using traced_url, so only hostnames
ending in ".ru" trigger the rule; preserve the existing testsafebrowsing
condition and malicious-state updates.
- Around line 69-78: Update the exception handler around the asyncio.wait_for
call in the hybrid scan flow to catch both asyncio.TimeoutError and the built-in
TimeoutError. Preserve the existing timeout log message and {"error": "Timeout"}
fallback assignments for vt_result and gsb_result.

In `@app/service/security/google_safe_browsing.py`:
- Line 11: Update the HTTP failure logging in the Google Safe Browsing request
flow to avoid stringifying HTTPStatusError, since its message contains the
API-key-bearing request URL. Log only safe details such as the response status
code and request method, or omit the exception text entirely; keep the existing
error handling behavior unchanged.

In `@app/service/security/mock_provider.py`:
- Around line 30-58: Align the mock security API used by parser.py with
mock_provider.py: expose the imported is_mock_enabled and get_mock_security_data
helpers from this module, or update parser.py to call the existing
MockSecurityEngine API. Ensure the parser import path resolves successfully and
preserves the current mock URL-scanning behavior.

In `@app/utils/url_tracker.py`:
- Around line 42-46: Bound the total execution time of trace_url, rather than
relying only on the per-request timeout in the HEAD/GET redirect loop. Add an
overall deadline or timeout around the tracing flow, including redirect handling
and GET fallback, so it cannot exceed the pipeline SLA before
scan_service.scan_message_text proceeds.
- Around line 29-46: Update trace_url to validate the initial URL and every
redirect target before issuing HEAD or GET requests, rejecting loopback,
private, link-local, and metadata addresses. Enforce the intended overall
timeout within trace_url itself so redirect processing cannot consume
substantially longer than the external asyncio.wait_for guard.

---

Nitpick comments:
In @.vscode/settings.json:
- Around line 2-5: Remove the machine-specific Python interpreter and analysis
paths from the committed .vscode/settings.json, or move them into a user-local
ignored configuration so contributors on macOS, Linux, and Windows are not
affected. Preserve only shared editor settings in version control and ensure the
workspace no longer forces the Windows-specific .venv paths.

In `@app/core/config.py`:
- Around line 9-10: Update the VIRUSTOTAL_API_KEY and
GOOGLE_SAFE_BROWSING_API_KEY field declarations to remove the deprecated env=
arguments, relying on their matching field names for environment-variable
loading; use validation_alias only if either environment name differs from its
field name.

In `@app/dto/schemas.py`:
- Around line 19-31: The schema’s inner Config class is deprecated under
Pydantic v2. Replace it with a model_config assignment using ConfigDict and
preserve the existing json_schema_extra example unchanged, including its field
values.

In `@app/service/scan_service.py`:
- Around line 80-83: Replace the four raw response print statements in the scan
response output block with the module’s logger.debug calls, preserving the
VirusTotal and Safe Browsing response details while avoiding unconditional
stdout output. Remove the separator prints or include equivalent context in a
single debug log.
- Around line 87-97: Update the VirusTotal and Google Safe Browsing failure
checks in the scan flow to avoid substring matching against str(vt_result) or
str(gsb_result). Detect failures only when the result is an Exception or a
dictionary containing an "error" key, while preserving the existing error
logging, fallback results, and error_logs updates.
🪄 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: d6d4d5fe-1839-4635-b4aa-367203f83bb2

📥 Commits

Reviewing files that changed from the base of the PR and between e47013b and 3e717f9.

📒 Files selected for processing (15)
  • .vscode/settings.json
  • app/core/config.py
  • app/dto/schemas.py
  • app/main.py
  • app/router/analyze.py
  • app/router/analyze_voice.py
  • app/service/scan_service.py
  • app/service/security/base.py
  • app/service/security/gemini_text_analyzer.py
  • app/service/security/google_safe_browsing.py
  • app/service/security/mock_provider.py
  • app/service/security/virustotal.py
  • app/service/url/extractor.py
  • app/service/url/tracer.py
  • app/utils/url_tracker.py
💤 Files with no reviewable changes (3)
  • app/service/url/extractor.py
  • app/service/url/tracer.py
  • app/router/analyze_voice.py

Comment thread app/main.py
Comment on lines +11 to +17
app.add_middleware(
CORSMiddleware,
allow_origins=["*"],
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*"],
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== app/main.py ==\n'
cat -n app/main.py | sed -n '1,120p'

printf '\n== CORS-related references ==\n'
rg -n "CORSMiddleware|allow_origins|allow_credentials|allow_methods|allow_headers" -S .

printf '\n== Dependency files ==\n'
git ls-files | rg '(^|/)(requirements|pyproject|poetry\.lock|Pipfile|Pipfile\.lock|setup\.py|setup\.cfg|package\.json|uv\.lock)$' || true

Repository: SafeFam/SafeFam_AI

Length of output: 1221


🌐 Web query:

Starlette CORSMiddleware allow_origins "*" allow_credentials behavior docs

💡 Result:

As of July 2026, the configuration of Starlette's CORSMiddleware with both allow_origins=[""] and allow_credentials=True is treated as a security misconfiguration and is strictly prohibited [1]. In recent versions of Starlette, initializing CORSMiddleware with allow_origins=[""] and allow_credentials=True simultaneously will raise a ValueError at application startup [1]. This change aligns with W3C CORS specifications, which prohibit the use of credentials with a wildcard origin [1]. Prior to this enforcement, earlier versions of Starlette attempted to handle this combination by dynamically echoing the requesting Origin in the Access-Control-Allow-Origin response header instead of returning the literal * wildcard, whenever credentials (such as cookies or Authorization headers) were present [2][3]. However, this behavior was considered unreliable and potentially dangerous, leading to the current requirement for explicit origin specification when credentials are enabled [1][2]. To support credentialed requests, you must explicitly list the allowed origins (e.g., allow_origins=["https://example.com"]) rather than using the wildcard [1][4].

Citations:


Use explicit origins when credentials are enabled. allow_origins=["*"] with allow_credentials=True is invalid for credentialed CORS and will be rejected by current Starlette/FastAPI releases at startup. Restrict allow_origins to the known gateway origin(s), or drop allow_credentials if cookies/auth headers aren’t needed.

🤖 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` around lines 11 - 17, Update the CORSMiddleware configuration to
use explicit known gateway origin(s) instead of allow_origins=["*"] while
retaining allow_credentials=True, or remove allow_credentials when credentialed
requests are not required. Keep the existing methods and headers configuration
unchanged.

Comment thread app/router/analyze.py
Comment on lines +48 to +49
except Exception as e:
return ApiResponse.error(message=f"통합 스미싱 탐지 중 서버 에러가 발생했습니다: {str(e)}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Avoid returning raw exception text to clients.

f"...{str(e)}" can expose internal details (stack context, paths, keys embedded in messages) to callers, and the response still carries HTTP 200 from the route decorator. Log the exception server-side and return a generic message; consider signalling failure via status code as well.

🧰 Tools
🪛 Ruff (0.15.21)

[warning] 48-48: Do not catch blind exception: Exception

(BLE001)


[warning] 49-49: Use explicit conversion flag

Replace with conversion flag

(RUF010)

🤖 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 48 - 49, Update the exception handler in
the analyze route to log the caught exception server-side instead of
interpolating str(e) into the client response. Return a generic Korean error
message without internal details, and configure ApiResponse.error or the route
response handling to signal failure with an appropriate non-200 status code
while preserving the existing success behavior.

Comment on lines +69 to +78
try:
results = await asyncio.wait_for(
asyncio.gather(vt_task, gsb_task, return_exceptions=True),
timeout=6.0
)
vt_result, gsb_result = results
except asyncio.TimeoutException:
logger.error("[Pipeline Timeout] 외부 보안 API 응답 시간 초과로 하이브리드 스캔이 강제 타임아웃 처리되었습니다.")
vt_result = {"error": "Timeout"}
gsb_result = {"error": "Timeout"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

python asyncio wait_for raises TimeoutError or TimeoutException

💡 Result:

When asyncio.wait_for reaches its specified timeout, it raises a TimeoutError exception [1][2]. It is important to note that the specific class of exception raised has changed over Python versions: 1. In Python 3.11 and later: asyncio.wait_for raises the built-in TimeoutError [3][2]. The name asyncio.TimeoutError has been retained as an alias for the built-in TimeoutError [3][4]. 2. In Python 3.10 and earlier: asyncio.wait_for raises asyncio.TimeoutError, which was a distinct exception class unrelated to the built-in TimeoutError [1][5][6]. If you are writing code that needs to support older versions of Python, you should catch asyncio.TimeoutError [7]. If you are working exclusively in Python 3.11+, you can catch either the built-in TimeoutError or the alias asyncio.TimeoutError [4]. There is no exception named TimeoutException in the Python standard library's asyncio module; TimeoutError is the correct exception to handle [1][3][2]. Additionally, if the task being awaited raises its own exception during the cancellation process triggered by a timeout, asyncio.wait_for will propagate that exception [1][8][2]. This behavior ensures that errors occurring during task cleanup are not silently suppressed [8][9].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '55,100p' app/service/scan_service.py | cat -n

Repository: SafeFam/SafeFam_AI

Length of output: 2748


🏁 Script executed:

python3 - <<'PY'
import asyncio
print("has TimeoutException:", hasattr(asyncio, "TimeoutException"))
print("TimeoutError:", asyncio.TimeoutError)
try:
    print("MRO:", asyncio.TimeoutError.mro())
except Exception as e:
    print(type(e), e)

# Simulate except clause evaluation when attribute missing
class Dummy:
    pass

try:
    raise TimeoutError("boom")
except getattr(Dummy, "Missing", None):
    print("caught")
except Exception as e:
    print("handler exception:", type(e).__name__, e)

# Check whether built-in TimeoutError catches asyncio.TimeoutError
try:
    raise asyncio.TimeoutError("async boom")
except TimeoutError as e:
    print("builtin caught asyncio.TimeoutError:", type(e).__name__, e)
PY

Repository: SafeFam/SafeFam_AI

Length of output: 653


🏁 Script executed:

python3 - <<'PY'
import asyncio

def probe():
    try:
        raise TimeoutError("boom")
    except asyncio.TimeoutException:
        return "caught"
    except Exception as e:
        return f"inner caught {type(e).__name__}: {e}"

try:
    print(probe())
except Exception as e:
    print(f"outer caught {type(e).__name__}: {e}")
PY

Repository: SafeFam/SafeFam_AI

Length of output: 238


Catch the real timeout exceptionasyncio.TimeoutException doesn’t exist, so a timeout raises AttributeError while matching this handler and skips the {"error": "Timeout"} fallback. Use except (asyncio.TimeoutError, TimeoutError): here.

🤖 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/service/scan_service.py` around lines 69 - 78, Update the exception
handler around the asyncio.wait_for call in the hybrid scan flow to catch both
asyncio.TimeoutError and the built-in TimeoutError. Preserve the existing
timeout log message and {"error": "Timeout"} fallback assignments for vt_result
and gsb_result.

Comment on lines +117 to +120
if not is_malicious and (".ru" in traced_url or "testsafebrowsing" in traced_url):
logger.warning("[Infrastructure Guard] 외부 API 응답 공백 감지 - 로컬 정밀 위협 룰셋에 의해 악성 URL로 강제 전환합니다.")
is_malicious = True
risk_score = 0.75

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

".ru" in traced_url is an over-broad substring match causing false positives.

Substring matching flags any URL whose text contains .ru, including legitimate hosts like docs.ruby-lang.org (.ru inside .ruby) or paths/query strings. This forces is_malicious=True incorrectly. Match on the parsed host suffix instead (e.g. urlparse(traced_url).hostname.endswith(".ru")).

🤖 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/service/scan_service.py` around lines 117 - 120, Replace the broad ".ru"
substring check in the malicious URL guard with a parsed-host suffix check using
traced_url, so only hostnames ending in ".ru" trigger the rule; preserve the
existing testsafebrowsing condition and malicious-state updates.

class GoogleSafeBrowsingEngine(BaseSecurityEngine):
def __init__(self):
self.api_key = settings.GOOGLE_SAFE_BROWSING_API_KEY
self.api_url = f"https://safebrowsing.googleapis.com/v4/threatMatches:find?key={self.api_key}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== file outline ==\n'
ast-grep outline app/service/security/google_safe_browsing.py --view expanded || true

printf '\n== relevant lines ==\n'
sed -n '1,140p' app/service/security/google_safe_browsing.py

Repository: SafeFam/SafeFam_AI

Length of output: 3158


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import inspect, httpx
print(httpx.__version__)
print(inspect.getsource(httpx.HTTPStatusError.__str__))
PY

Repository: SafeFam/SafeFam_AI

Length of output: 1042


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import httpx

req = httpx.Request("POST", "https://example.com/path?key=SECRET123")
resp = httpx.Response(403, request=req)
try:
    resp.raise_for_status()
except httpx.HTTPStatusError as e:
    print(str(e))
    print("URL:", e.request.url)
PY

Repository: SafeFam/SafeFam_AI

Length of output: 367


Don't stringify HTTPStatusError here
httpx.HTTPStatusError.__str__() includes the request URL, and self.api_url embeds ?key=..., so logger.error(... {str(e)}) will leak GOOGLE_SAFE_BROWSING_API_KEY into logs on HTTP failures. Log only the status code/method, or omit the exception string.

🤖 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/service/security/google_safe_browsing.py` at line 11, Update the HTTP
failure logging in the Google Safe Browsing request flow to avoid stringifying
HTTPStatusError, since its message contains the API-key-bearing request URL. Log
only safe details such as the response status code and request method, or omit
the exception text entirely; keep the existing error handling behavior
unchanged.

Comment on lines +30 to +58
async def scan_url(self, url: str) -> dict:
"""
[상용 인터페이스 구현]
가상 DB를 매칭하여 상용 엔진과 동일한 규격의 딕셔너리를 반환합니다.
"""
logger.info(f"🚨 [MOCK SECURITY ENGINE] Sandbox API 우회 매칭 -> {url}")

# URL이 DB에 없으면 안전한 상태인 default_safe 적용
data = self.mock_db.get(url, self.default_safe)

gsb_malicious = data.get("google_safe_browsing", False)
vt_malicious_count = data.get("malicious_count", 0)

# 악성 여부 및 위험도 점수 가중치 산정 (상용 규격과 통일)
is_malicious = gsb_malicious or (vt_malicious_count >= 3)

if gsb_malicious:
raw_score = 0.95
elif vt_malicious_count > 0:
raw_score = min(0.2 + (vt_malicious_count * 0.08), 1.0)
else:
raw_score = 0.0

def is_mock_enabled() -> bool:
return MOCK_ENABLED
return {
"is_malicious": is_malicious,
"raw_score": round(raw_score, 2),
"detected_count": vt_malicious_count,
"status": "completed"
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP '\bMockSecurityEngine\b' --type=py

Repository: SafeFam/SafeFam_AI

Length of output: 156


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== mock_provider.py ==\n'
if [ -f app/service/security/mock_provider.py ]; then
  sed -n '1,220p' app/service/security/mock_provider.py
else
  echo 'missing app/service/security/mock_provider.py'
fi

printf '\n== scan_service.py ==\n'
if [ -f app/service/security/scan_service.py ]; then
  sed -n '1,260p' app/service/security/scan_service.py
else
  echo 'missing app/service/security/scan_service.py'
fi

printf '\n== references ==\n'
rg -n "MockSecurityEngine|MOCK_ENABLED|vt_malicious_count|google_safe_browsing" app/service/security -g '*.py'

Repository: SafeFam/SafeFam_AI

Length of output: 5026


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== security tree ==\n'
find app/service/security -maxdepth 2 -type f | sort

printf '\n== parser.py ==\n'
sed -n '1,220p' app/service/security/parser.py

printf '\n== mock provider usages ==\n'
rg -n "mock_provider|MockSecurityEngine|get_mock_text_analysis_data|MOCK_SECURITY_API|MOCK_ENABLED" . -g '*.py'

Repository: SafeFam/SafeFam_AI

Length of output: 4552


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== mock_provider.py (numbered) ==\n'
cat -n app/service/security/mock_provider.py

printf '\n== scan_service.py (numbered) ==\n'
cat -n app/service/scan_service.py

printf '\n== symbol search ==\n'
rg -n "def (is_mock_enabled|get_mock_security_data)|class MockSecurityEngine|from app\.service\.security\.mock_provider import" app/service -g '*.py'

Repository: SafeFam/SafeFam_AI

Length of output: 11281


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "from app\.service\.security\.parser import|import app\.service\.security\.parser|security\.parser|analyze_threat_pipeline|determine_final_threat_grade" . -g '*.py'

Repository: SafeFam/SafeFam_AI

Length of output: 2042


mock_provider.py is missing the helpers parser.py imports.

app/service/security/parser.py imports is_mock_enabled and get_mock_security_data, but this module only defines MockSecurityEngine and get_mock_text_analysis_data. That breaks the parser import path; either export the expected helpers here or update parser.py to use the current API.

🤖 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/service/security/mock_provider.py` around lines 30 - 58, Align the mock
security API used by parser.py with mock_provider.py: expose the imported
is_mock_enabled and get_mock_security_data helpers from this module, or update
parser.py to call the existing MockSecurityEngine API. Ensure the parser import
path resolves successfully and preserves the current mock URL-scanning behavior.

Comment thread app/utils/url_tracker.py
Comment on lines +29 to +46
async def trace_url(url: str, max_redirects: int = 5, timeout: float = 3.0) -> str:

current_url = url

limits = httpx.Limits(max_keepalive_connections=5, max_connections=10)

async with httpx.AsyncClient(limits=limits, follow_redirects=False) as client:
for attempt in range(max_redirects):
try:
headers = {
"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36"
}

response = await client.head(current_url, headers=headers, timeout=timeout)

# HEAD를 차단하거나 거부하는 서버(400, 404, 405)에 대응하기 위한 GET 폴백
if response.status_code in [400, 404, 405]:
response = await client.get(current_url, headers=headers, timeout=timeout)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Block internal targets before following redirects. trace_url sends HEAD/GET requests to user-controlled URLs and follows Location headers, so a crafted link or redirect can reach loopback, RFC1918, link-local, or metadata endpoints. It also runs before the later asyncio.wait_for, so this path can still consume ~30s before the 6s guard applies.

🤖 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/url_tracker.py` around lines 29 - 46, Update trace_url to validate
the initial URL and every redirect target before issuing HEAD or GET requests,
rejecting loopback, private, link-local, and metadata addresses. Enforce the
intended overall timeout within trace_url itself so redirect processing cannot
consume substantially longer than the external asyncio.wait_for guard.

Comment thread app/utils/url_tracker.py
Comment on lines +42 to +46
response = await client.head(current_url, headers=headers, timeout=timeout)

# HEAD를 차단하거나 거부하는 서버(400, 404, 405)에 대응하기 위한 GET 폴백
if response.status_code in [400, 404, 405]:
response = await client.get(current_url, headers=headers, timeout=timeout)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Total tracing time is unbounded relative to the pipeline SLA.

Worst case is max_redirects × (HEAD + GET) × timeout (~30s), and trace_url runs before the 6s asyncio.wait_for guard in scan_service.scan_message_text, so the endpoint's overall latency isn't bounded. Consider an overall deadline for tracing or wrapping it in a timeout as well.

🤖 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/url_tracker.py` around lines 42 - 46, Bound the total execution
time of trace_url, rather than relying only on the per-request timeout in the
HEAD/GET redirect loop. Add an overall deadline or timeout around the tracing
flow, including redirect handling and GET fallback, so it cannot exceed the
pipeline SLA before scan_service.scan_message_text proceeds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat New feature or functional additions to the application

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant