fix(audit): client IP trust-proxy 정합 + 감사 로그 IP 배선 (ALS 요청 컨텍스트)#124
Conversation
- tryClientIp: raw X-Forwarded-For/X-Real-IP 직접 신뢰 제거 → Express trust proxy 가 계산한 req.ip 단일 소스 사용. TRUST_PROXY_HOPS 설정이 비로소 유효해지고 IP 위조 차단 - RequestContextService(AsyncLocalStorage) + RequestContextMiddleware 신설: 요청 진입 시 client IP/UA 를 요청 컨텍스트에 적재 (REST·GraphQL 공통) - AuditLogRepository 가 단일 write 진입점에서 컨텍스트의 IP/UA 자동 보강 (명시 인자가 있으면 우선). seller 감사 로그 ip_address 가 항상 null 이던 문제 해소 - 도메인 서비스 시그니처 무변경 (transport 메타데이터 누수 없음) - testing-module.builder 가 RequestContextService 기본 제공
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Coverage report
Test suite run success1253 tests passing in 146 suites. Report generated by 🧪jest coverage report action from ab05fad |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8de8900779
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| after_json: args.afterJson === null ? Prisma.JsonNull : args.afterJson, | ||
| ip_address: args.ipAddress ?? null, | ||
| user_agent: args.userAgent ?? null, | ||
| ip_address: args.ipAddress ?? ctx?.clientIp ?? null, |
There was a problem hiding this comment.
When TRUST_PROXY_HOPS is enabled, ctx.clientIp comes from req.ip, which can still reflect the proxy-provided X-Forwarded-For value; if an upstream proxy forwards a malformed or overlong value, this new fallback writes it verbatim into audit_log.ip_address (@db.VarChar(64) in prisma/schema.prisma). That makes any seller mutation that creates an audit log fail with a database value-too-long error instead of completing, whereas those calls previously stored null; please validate/truncate the context IP before using it as the default.
Useful? React with 👍 / 👎.
- ip_address(VarChar(64)) 저장 전 net.isIP 로 검증 — trust proxy 가 넘긴 malformed·overlong 값이 컬럼 길이 초과로 insert 를 깨뜨리는 것을 방지. 유효 IPv4/IPv6 가 아니면 null 로 저장 (쓰레기 IP 적재 방지) - user_agent(VarChar(512)) 도 저장 전 512 자로 절단 (defense-in-depth)
1 participant
Summary
리팩토링 플랜 감사 중 발견한 #119(IP/UA 단일화 + trust proxy) PR의 두 가지 미완결을 바로잡습니다.
tryClientIp가 rawX-Forwarded-For(맨 왼쪽)·X-Real-IP헤더를req.ip보다 먼저 신뢰해,main.ts의TRUST_PROXY_HOPS설정이 사실상 죽은 config 였고 클라이언트가 IP 를 위조할 수 있었습니다. Expresstrust proxy가 계산한req.ip단일 소스로 교정합니다.ipAddress를 전달하지 않아audit_log.ip_address가 항상 null 로 기록되고 있었습니다. 요청 컨텍스트(AsyncLocalStorage)를 도입해 단일 write 진입점에서 자동 보강하도록 했습니다.IP 는 transport 계층 메타데이터라 도메인 서비스 시그니처에 인자로 흘려보내면 SoC 를 해칩니다. 그래서 명시적 threading 대신 요청 컨텍스트(ALS) 로 암묵 전파하여, 도메인 서비스/리졸버 시그니처를 전혀 건드리지 않고
AuditLogRepository가 컨텍스트에서 IP/UA 를 읽도록 했습니다.Scope
신규
RequestContextService(src/global/request-context/request-context.service.ts) —AsyncLocalStorage기반 요청 컨텍스트 래퍼.run()/get()/getClientIp()/getUserAgent()RequestContextMiddleware— 요청 진입 시tryClientIp/tryUserAgent로 컨텍스트를 열고next()를 그 안에서 호출 (REST·GraphQL 공통)RequestContextModule(@Global) + barrelrequest-context.service.spec.ts,request-context.middleware.spec.ts변경
http-meta.tstryClientIp— raw XFF/X-Real-IP 분기 제거,req.ip→ socket fallback 만 사용app.module.ts—RequestContextModuleimport +RequestContextMiddleware를 모든 라우트에 가장 먼저 적용AuditLogRepository.createAuditLog—ip_address/user_agent를args→ 요청 컨텍스트 → null 순으로 보강 (명시 인자 우선)testing-module.builder.ts—RequestContextService기본 제공 (실DB spec 들이 실AuditLogRepository를 resolve 하도록)http-meta.spec.ts(spoofing 가드),audit-log.repository.spec.ts(ALS 보강/우선순위/컨텍스트 밖 null)도메인 서비스/리졸버: 변경 없음.
진행 상황
리팩토링 플랜 P2 잔여 작업 진행 중, 플랜·머지 PR 비판적 감사에서 도출된 항목입니다.
Impact
audit_log.ip_address/user_agent가 운영 trust proxy 환경에서 실제 client IP/UA 로 기록됨 (이전엔 seller 도메인 전부 null). 스키마 변경 없음.TRUST_PROXY_HOPS정확 설정 시 XFF 위조 차단. 미설정(0) 시 socket 주소 사용(안전 기본).global/request-context/*100%.yarn validate전체 통과 (146 suites / 1251 tests).Test plan
tryClientIp/clientIpOf— req.ip 우선, 위조 XFF/X-Real-IP 무시(spoofing 가드), socket fallback, undefinedRequestContextService— run 내부/외부, async 경계 유지, 중첩 run 복원, 반환값 전달RequestContextMiddleware— next 시점 컨텍스트 적재, 위조 XFF 무시, async 핸들러 유지, 추출 실패 시 undefinedAuditLogRepository(실DB) — 컨텍스트에서 ip/ua 보강, 명시 인자 우선, 컨텍스트 밖 nullyarn validate(lint + tsc + dto:check + test:cov) 통과