[Chore] FastAPI 운영 환경 설정 및 Secret 분리 - #35
Conversation
📝 WalkthroughWalkthroughThe PR centralizes environment configuration, adds production validation for credentials and model artifacts, packages model files in Docker, adds production Compose guidance, and removes sensitive values from logs and error responses. ChangesProduction configuration and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/analysis/text/naive_bayes_analyzer.py`:
- Around line 90-95: Update the exception handling in the NaiveBayes
model-loading flow so `_load_error`, which is returned to clients, always uses
the stable public code `MODEL_LOAD_FAILED` rather than the exception class name.
Log the actual exception type separately through the existing logger without
exposing it via the client-facing error field.
In `@app/core/config.py`:
- Around line 112-116: Update the RabbitMQ validation in the relevant
configuration validation method around RABBITMQ_URL to parse the URL and inspect
its hostname rather than comparing one literal. Reject missing hosts, localhost,
and loopback IP addresses while preserving acceptance of valid non-local
brokers; add regression tests covering localhost with different credentials and
127.0.0.1.
In `@app/main.py`:
- Around line 38-46: Update the required-file validation around the missing
artifact check to open every configured model artifact in binary mode, treating
any read/open failure as validation failure alongside nonexistent or non-file
paths. Preserve the existing RuntimeError reporting of affected paths, and add
coverage for an unreadable artifact path.
In `@README.md`:
- Around line 92-98: Update the recommended production Parameter Store examples
in README.md to include the source path for the required RABBITMQ_URL secret, or
document the established alternate mechanism that injects it into .env.runtime
alongside the other production secrets.
🪄 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: 6aa34d15-c79b-4f31-84cc-8b7064662c27
📒 Files selected for processing (23)
.dockerignore.env.example.env.prod.example.gitignoreDockerfileREADME.mdapp/analysis/router.pyapp/analysis/service.pyapp/analysis/text/gemini_analyzer.pyapp/analysis/text/naive_bayes_analyzer.pyapp/analysis/url/analyzer.pyapp/analysis/url/tracker.pyapp/chat/router.pyapp/chat/service.pyapp/core/config.pyapp/infrastructure/google_safe_browsing/client.pyapp/infrastructure/mock_provider.pyapp/infrastructure/virustotal/client.pyapp/main.pydocker-compose.prod.ymlrequirements.txttests/core/__init__.pytests/core/test_config.py
| except Exception as exception: | ||
| _load_error = type(exception).__name__ | ||
| logger.error( | ||
| "[NaiveBayes] 모델 로드 실패. error_type=%s", | ||
| _load_error, | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Do not expose the exception type through error_message.
_load_error is returned to clients at Line [111]. After this change, clients can receive internal classes such as FileNotFoundError or KeyError. Store a stable public code such as MODEL_LOAD_FAILED, and log the exception type separately.
Proposed fix
except Exception as exception:
- _load_error = type(exception).__name__
+ error_type = type(exception).__name__
+ _load_error = "MODEL_LOAD_FAILED"
logger.error(
"[NaiveBayes] 모델 로드 실패. error_type=%s",
- _load_error,
+ error_type,
)📝 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.
| except Exception as exception: | |
| _load_error = type(exception).__name__ | |
| logger.error( | |
| "[NaiveBayes] 모델 로드 실패. error_type=%s", | |
| _load_error, | |
| ) | |
| except Exception as exception: | |
| error_type = type(exception).__name__ | |
| _load_error = "MODEL_LOAD_FAILED" | |
| logger.error( | |
| "[NaiveBayes] 모델 로드 실패. error_type=%s", | |
| error_type, | |
| ) |
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 90-90: Do not catch blind exception: Exception
(BLE001)
🤖 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/analysis/text/naive_bayes_analyzer.py` around lines 90 - 95, Update the
exception handling in the NaiveBayes model-loading flow so `_load_error`, which
is returned to clients, always uses the stable public code `MODEL_LOAD_FAILED`
rather than the exception class name. Log the actual exception type separately
through the existing logger without exposing it via the client-facing error
field.
| local_rabbitmq_url = ( | ||
| "amqp://safefam:safefam-local@localhost:5672/" | ||
| ) | ||
| if self.RABBITMQ_URL == local_rabbitmq_url: | ||
| missing.append("RABBITMQ_URL") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the RabbitMQ host instead of one URL literal.
Line 115 rejects only the default URL. A production value such as amqp://user:pass@127.0.0.1:5672/ or a localhost URL with different credentials passes validation. The worker can then connect to the wrong broker or fail after startup.
Parse RABBITMQ_URL and reject missing hosts, localhost, and loopback IP addresses. Add regression tests for localhost with changed credentials and 127.0.0.1.
🤖 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 112 - 116, Update the RabbitMQ validation in
the relevant configuration validation method around RABBITMQ_URL to parse the
URL and inspect its hostname rather than comparing one literal. Reject missing
hosts, localhost, and loopback IP addresses while preserving acceptance of valid
non-local brokers; add regression tests covering localhost with different
credentials and 127.0.0.1.
| missing = [ | ||
| str(path) | ||
| for path in required_files | ||
| if not path.is_file() | ||
| ] | ||
| if missing: | ||
| raise RuntimeError( | ||
| "Required AI model files are missing: " | ||
| + ", ".join(missing) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate model artifact readability.
Path.is_file() only checks the path type. It does not verify that safefam can read the file.
If a configured artifact has restrictive permissions, startup succeeds and model loading fails later. Open each artifact in binary mode during validation. Add a test for an unreadable artifact path.
Proposed fix
- missing = [
- str(path)
- for path in required_files
- if not path.is_file()
- ]
- if missing:
+ invalid_files: list[str] = []
+ for path in required_files:
+ try:
+ with path.open("rb"):
+ pass
+ except OSError:
+ invalid_files.append(str(path))
+
+ if invalid_files:
raise RuntimeError(
- "Required AI model files are missing: "
- + ", ".join(missing)
+ "Required AI model files are missing or unreadable: "
+ + ", ".join(invalid_files)
)🤖 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 38 - 46, Update the required-file validation around
the missing artifact check to open every configured model artifact in binary
mode, treating any read/open failure as validation failure alongside nonexistent
or non-file paths. Preserve the existing RuntimeError reporting of affected
paths, and add coverage for an unreadable artifact path.
| 권장 Parameter Store 경로는 다음과 같습니다. | ||
|
|
||
| ```text | ||
| /safefam/prod/ai/GEMINI_API_KEY | ||
| /safefam/prod/ai/VIRUSTOTAL_API_KEY | ||
| /safefam/prod/ai/GOOGLE_SAFE_BROWSING_API_KEY | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the production source for RABBITMQ_URL.
RABBITMQ_URL is a required production secret, but the Parameter Store examples omit it. A deployment that follows these examples cannot create a valid .env.runtime file.
Add a RABBITMQ_URL Parameter Store path, or document the alternate secret-injection mechanism.
Proposed fix
/safefam/prod/ai/GEMINI_API_KEY
/safefam/prod/ai/VIRUSTOTAL_API_KEY
/safefam/prod/ai/GOOGLE_SAFE_BROWSING_API_KEY
+/safefam/prod/ai/RABBITMQ_URL📝 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.
| 권장 Parameter Store 경로는 다음과 같습니다. | |
| ```text | |
| /safefam/prod/ai/GEMINI_API_KEY | |
| /safefam/prod/ai/VIRUSTOTAL_API_KEY | |
| /safefam/prod/ai/GOOGLE_SAFE_BROWSING_API_KEY | |
| ``` | |
| 권장 Parameter Store 경로는 다음과 같습니다. | |
🤖 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 `@README.md` around lines 92 - 98, Update the recommended production Parameter
Store examples in README.md to include the source path for the required
RABBITMQ_URL secret, or document the established alternate mechanism that
injects it into .env.runtime alongside the other production secrets.
📝 개요
FastAPI AI Worker의 개발·운영 환경을 분리하고, 운영 Secret 및 AI 모델 파일을 안전하게 주입할 수 있도록 배포 설정을 구성했습니다.
운영 필수 설정이 누락되거나 Mock 모드가 활성화된 경우 애플리케이션 시작을 차단하며, Docker 이미지에 필요한 모델 파일만 포함하도록 정리했습니다. 또한 문자 원문, 분석 URL 및 외부 API 예외 원문이 로그에 노출되지 않도록 보완했습니다.
🔗 관련 이슈
🎯 주요 변경 사항
pydantic-settings기반으로 환경변수 관리를 통합하고 운영 필수 Secret 검증을 추가했습니다..gitignore,.dockerignore및 운영 배포 문서를 보완했습니다.📸 사진
✅ PR 체크리스트
uvicorn구동 또는 테스트 코드)를 통과했습니다.Summary by CodeRabbit