fix: 코드 감사 배치 1 — 백테스트 정확성·XSS·배포 안전성 - #50
Merged
Merged
Conversation
클래스 속성명(sma_short/sma_long)이 공개 파라미터명(short_window/ long_window)과 달라 BacktestEngine._build_strategy의 hasattr 필터에서 전부 탈락, 사용자가 무엇을 입력하든 항상 기본값 10/20으로 실행됐다. 속성명을 공개명으로 통일하고, 검증기를 mock하지 않고 실제 경로를 타는 오버라이드 회귀 테스트를 6개 전략 전체에 추가했다(전수 점검 결과 불일치는 SMA 단독). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- weight만 입력된 포트폴리오에서 분모(total_amount)가 100으로 하드코딩돼, 스키마가 허용하는 비중 합 95~105%에서 수익률이 왜곡됐다 (합 95 → 평평한 시장에서 -5% 보고). 실제 환산 금액 합계로 교체. - 전략 포트폴리오의 종목별 BacktestRequest에 commission을 전달하지 않아 사용자 수수료(예: 3%)가 스키마 기본값 0.2%로 대체되던 문제 수정. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
기본값 "buy_and_hold"는 StrategyType에 존재하지 않는 값이라, strategy를 생략한 요청이 전략 경로로 라우팅된 뒤 종목별 enum 검증에서 전멸해 "모든 종목 실패" 오류로 이어졌다. buy_hold_strategy로 교체하고, StrategyType에 없는 전략명은 422로 조기 거부하는 검증을 추가했다. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
외부(네이버 API) 뉴스 제목/본문을 dangerouslySetInnerHTML로 주입하던 것을 엔티티 디코드 후 텍스트 렌더로 전환. BE의 <.*?> 정규식은 닫는 >가 없는 태그(<img src=x onerror=...)를 통과시키므로 우회 가능했다. 디코더는 HTML 파서 비의존 순수 문자열 치환으로 구현(파서 기반은 happy-dom에서 태그 모양 텍스트를 삼키는 것을 실측 확인), 범위 초과 숫자 엔티티의 String.fromCodePoint RangeError 크래시도 상한 가드로 차단. 같은 취약 패턴을 가진 미사용 NewsModal·UnifiedInfoSection과 그 테스트를 삭제하고 XSS 회귀 테스트 5개를 추가했다. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- getParamLabel은 상태/props에 의존하지 않는 순수 함수라 모듈 스코프로 이동해 useCallback 의존성 경고 2건을 해소. - 임포터가 없던 useAsync 훅과 그 테스트를 삭제해 spread 의존성 경고 1건을 해소 (disable 주석 없이 전부 근본 해결). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
10회 시도가 모두 실패해도 경고 echo 후 exit 0으로 끝나, 서비스가 내려간 배포도 성공으로 표시되던 문제. 시도 소진 시 exit 1로 종료한다. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
location /api/v1/backtest/ 는 슬래시 없는 POST /api/v1/backtest를 301로 리다이렉트하고, 브라우저는 301에서 POST를 GET으로 바꾸므로 FastAPI가 405를 반환한다. URI 없는 proxy_pass로 원본 경로를 그대로 전달하도록 수정. 정의되지 않은 /api/* 경로는 SPA fallback(HTML 200) 대신 404를 반환하고, 장시간 백테스트용 read timeout 180s를 명시했다. nginx.conf(호스트 프록시용)와 nginx.prod.conf 동일 적용, nginx -t 검증. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
미검증 원격 스크립트를 빌드 중 root로 실행하는 공급망 리스크인데, entrypoint·Dockerfile.dev·scripts 어디에서도 uv를 사용하지 않는다. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0.0.0.0:3306 공개는 compose에 커밋된 기본 비밀번호 폴백과 조합돼 LAN에 알려진 자격증명의 DB를 노출한다. BE는 compose 네트워크로 접속하므로 영향 없다. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TODO.md: 두 독립 분석(Claude 5영역 감사 + Codex)을 통합한 우선순위 백로그. 전 항목 파일:라인 근거 포함, 배치 1 완료분은 체크 처리. CLAUDE.md: 테스트 기준선 153/104로 갱신, 재발 방지 제약 2건 추가 (전략 클래스 속성명 = 공개 파라미터명 규칙, API 텍스트 dangerouslySetInnerHTML 금지), 존재하지 않는 Zustand 서술 정정, TODO.md 참조 섹션 추가. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
리포지토리 감사 결과 중 변경량 대비 효과가 큰 항목을 우선 적용해, 백테스트 결과 정확성(전략 파라미터/포트폴리오 계산/커미션), FE 뉴스 렌더링 XSS 표면, 그리고 배포 파이프라인/프록시 설정의 실패 감지를 강화하는 PR입니다.
Changes:
- BE 정확성/검증 강화: SMA 전략 파라미터 오버라이드가 무시되던 문제를 수정하고(속성명 정합), weight 모드 분모 계산 및 commission 전달 누락을 교정했으며, 기본 strategy 값/전략명 검증을 추가하고 회귀 테스트를 보강했습니다.
- FE XSS 차단 및 dead code 정리: 뉴스 렌더링을
dangerouslySetInnerHTML에서 텍스트 렌더 + HTML 엔티티 디코드로 전환하고, 관련 회귀 테스트를 추가했으며 미사용 컴포넌트/훅과 테스트를 삭제했습니다. - 인프라/CI 안전성: Jenkins 헬스체크가 실제로 실패할 수 있도록 수정했고, nginx API 라우팅(POST 301→GET 문제 및 SPA fallback)을 정리했으며 dev MySQL 포트를 루프백으로 제한하고 uv 설치를 제거했습니다.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | 감사 결과 백로그(P1~P3) 및 검증 기준선/후속 작업 정리 추가 |
| Jenkinsfile | Health Check가 타임아웃 시 실패 처리(exit 1)하도록 수정 |
| compose.dev.yaml | dev MySQL 포트 바인딩을 127.0.0.1로 제한 |
| CLAUDE.md | FE 상태관리/제약사항/테스트 기준선 및 TODO.md 안내 갱신 |
| backtest_fe/src/shared/hooks/useAsync.ts | 미사용 훅 삭제 |
| backtest_fe/src/shared/hooks/tests/useAsync.test.ts | useAsync 삭제에 따른 테스트 제거 |
| backtest_fe/src/features/backtest/hooks/useStrategyParams.ts | 파라미터 라벨 함수 모듈 스코프 분리 및 lint 경고 해소 |
| backtest_fe/src/features/backtest/components/volatility/NewsModal.tsx | 미사용(취약 패턴 포함) 컴포넌트 삭제 |
| backtest_fe/src/features/backtest/components/results/UnifiedInfoSection.tsx | 미사용(취약 패턴 포함) 컴포넌트 삭제 |
| backtest_fe/src/features/backtest/components/results/LatestNewsSection.tsx | 뉴스 렌더링을 텍스트 기반 + 엔티티 디코드로 변경 |
| backtest_fe/src/features/backtest/components/results/tests/UnifiedInfoSection.test.tsx | 삭제된 컴포넌트에 대한 테스트 제거 |
| backtest_fe/src/features/backtest/components/results/tests/LatestNewsSection.test.tsx | XSS/엔티티 디코딩 회귀 테스트 추가 |
| backtest_fe/package.json | ESLint max-warnings를 0으로 강화 |
| backtest_fe/nginx.prod.conf | /api 404 처리, backtest location 슬래시/proxy_pass 수정 및 타임아웃 추가 |
| backtest_fe/nginx.conf | dev nginx 설정도 prod와 동일하게 라우팅/타임아웃 수정 |
| backtest_be_fast/tests/unit/test_strategy_param_override.py | 전략 파라미터 오버라이드 회귀 테스트 추가(validator mock 없이) |
| backtest_be_fast/tests/unit/test_sma_strategy.py | SMA 파라미터명을 short_window/long_window로 갱신 |
| backtest_be_fast/tests/unit/test_portfolio_schemas.py | 기본 strategy 유효성/임의 strategy 거부 테스트 추가 |
| backtest_be_fast/tests/unit/test_portfolio_manager_fixes.py | weight 분모/commission 전달 회귀 테스트 추가 |
| backtest_be_fast/Dockerfile | 빌드 중 `curl |
| backtest_be_fast/app/strategies/strategies.py | SMA 전략 클래스 속성명을 공개 파라미터명과 일치하도록 변경 |
| backtest_be_fast/app/services/portfolio_manager_service.py | weight 분모 계산 수정 및 BacktestRequest에 commission 전달 |
| backtest_be_fast/app/schemas/schemas.py | 기본 strategy 수정 + StrategyType 기반 strategy 값 검증 추가 |
Comment on lines
+24
to
+37
| const PARAM_LABEL_MAP: Record<string, string> = { | ||
| 'short_window': '단기 이동평균 기간', | ||
| 'long_window': '장기 이동평균 기간', | ||
| 'rsi_period': 'RSI 기간', | ||
| 'rsi_oversold': 'RSI 과매도 기준', | ||
| 'rsi_overbought': 'RSI 과매수 기준', | ||
| 'bb_period': '볼린저 밴드 기간', | ||
| 'bb_std': '볼린저 밴드 표준편차', | ||
| 'macd_fast': 'MACD 빠른 기간', | ||
| 'macd_slow': 'MACD 느린 기간', | ||
| 'macd_signal': 'MACD 신호선 기간', | ||
| 'fast_window': '단기 EMA 기간', | ||
| 'slow_window': '장기 EMA 기간' | ||
| }; |
Comment on lines
+57
to
+70
| const decodeHtmlEntities = (value: string): string => | ||
| value.replace(/&(#x[0-9a-fA-F]+|#\d+|[a-zA-Z]+);/g, (match, entity: string) => { | ||
| if (entity[0] === '#') { | ||
| const isHex = entity[1] === 'x' || entity[1] === 'X'; | ||
| const codePoint = parseInt(entity.slice(isHex ? 2 : 1), isHex ? 16 : 10); | ||
| // 숫자 엔티티는 외부(네이버 API) 입력이라 공격자가 값을 채울 수 있다. | ||
| // String.fromCodePoint는 0x10FFFF를 넘는 코드포인트에 RangeError를 | ||
| // 던지므로, 이름 없는 엔티티와 동일하게 원본 문자열을 그대로 둔다. | ||
| return Number.isNaN(codePoint) || codePoint > MAX_UNICODE_CODE_POINT | ||
| ? match | ||
| : String.fromCodePoint(codePoint); | ||
| } | ||
| return NAMED_HTML_ENTITIES[entity] ?? match; | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
리포지토리 전체 감사(BE 로직 / FE / 테스트 / 인프라·CI·DB / 문서 5개 영역)에서 확인된 항목 중, 변경량이 적으면서 영향이 큰 것들을 우선 처리했습니다. 전체 백로그는 이 PR에 포함된
TODO.md에 있습니다.사용자에게 틀린 숫자가 나가던 버그 (BE)
SMA 전략 파라미터가 완전히 무시되고 있었습니다. 공개 파라미터명(
short_window)과 전략 클래스 속성명(sma_short)이 달라,BacktestEngine._build_strategy의hasattr필터에서 사용자 입력이 전부 탈락하고 항상 기본값 10/20으로 실행됐습니다. 기존 단위 테스트는 검증기를 identity로 mock하고 클래스 속성명과 일치하는 키를 넘겨서 이를 놓치고 있었습니다. 6개 전략을 전수 점검한 결과 불일치는 SMA 단독이며, 나머지 4개는 새 테스트가 수정 전부터 통과해 무결이 입증됐습니다.100.0으로 하드코딩돼, 스키마가 허용하는 비중 합 95~105% 구간에서 수익률이 최대 ±5%p 어긋났습니다(합 95면 평평한 시장에서 −5% 보고 → 0%로 교정).BacktestRequest에commission을 넘기지 않아, 사용자가 지정한 3%가 스키마 기본값 0.2%로 실행됐습니다."buy_and_hold"는StrategyType에 없는 값이라strategy를 생략한 요청이 전 종목 실패로 이어졌습니다. 이제 임의 문자열도 422로 거부합니다.XSS 차단 (FE)
외부(네이버 API) 뉴스 제목·본문을
dangerouslySetInnerHTML로 주입하고 있었고, 유일한 방어인 BE의re.compile('<.*?>')는 닫는>가 없는 페이로드(<img src=x onerror=...)를 그대로 통과시킵니다. 텍스트 렌더 + 엔티티 디코드로 전환했습니다.디코더는 HTML 파서에 의존하지 않는 순수 문자열 치환으로 구현했습니다 — 파서 기반(textarea/DOMParser)은 happy-dom에서 태그 모양 텍스트를 통째로 삼키는 것을 실측 확인했습니다. 리뷰 중 범위 초과 숫자 엔티티(
�)가String.fromCodePoint에 RangeError를 일으켜 렌더 중 섹션이 죽는 경로를 발견해 상한 가드를 추가했습니다. 같은 취약 패턴을 가진 미사용 컴포넌트 2개(NewsModal,UnifiedInfoSection)도 제거했습니다.배포 안전성 (인프라)
exit 0으로 끝나 서비스가 내려간 배포도 성공으로 표시됐습니다.location /api/v1/backtest/(트레일링 슬래시)는 슬래시 없는 POST를 301로 리다이렉트하고, 브라우저는 301에서 POST를 GET으로 바꾸므로 FastAPI가 405를 반환합니다. URI 없는proxy_pass로 원본 경로를 보존하도록 수정하고, 정의되지 않은/api/*가 SPA fallback으로 HTML 200이 되는 것도 404로 막았습니다. 실서비스가 정상이었다면 저장소 밖 엣지 프록시가 이 설정을 우회 중이었을 가능성이 있어, 배포 후 실제 경로 확인이 필요합니다.curl | sh로 설치하던 uv 제거(이미지 내 사용처 없음), dev MySQL을 루프백 바인드로 제한.검증
모든 수정은 TDD로 진행했습니다 — 실패 테스트를 먼저 작성하고, 예상한 이유로 실패하는 것을 확인한 뒤 고쳤습니다.
--max-warnings 0으로 강화docker build --target testBE·FE 양쪽 재현 통과순수 삭제 728줄 / 추가 188줄입니다. 커밋은 수정 코드와 회귀 테스트를 함께 묶어 논리 단위로 나눴으므로 개별 revert가 가능합니다.
후속
TODO.md에 남은 P1은 모두 구조 변경이 필요한 항목입니다 — DCA 계획/실행 회차 불일치, 포트폴리오 실패가 HTTP 200으로 반환되는 문제, 상장폐지 종목 리밸런싱 시 자산 중복 계상. 별도 PR로 진행합니다.🤖 Generated with Claude Code