[HOTFIX] secure, sameSite 값 환경변수 등록 #17
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough개요Express 세션 쿠키의 보안 설정이 환경에 따라 동적으로 결정되도록 수정되었습니다. 프로덕션에서는 변경사항세션 쿠키 보안 설정
보안 검토 포인트
가능 관련 이슈
코드 리뷰 난이도🎯 1 (Trivial) | ⏱️ ~5 minutes 축하 시
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main.ts (2)
60-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCORS
origin: true+credentials: true조합은 심각한 보안 취약점
origins에"*"가 포함되면origin: true가 설정되어 모든 출처에서의 인증 요청(세션 쿠키 포함)을 허용합니다. 이는 XRPL 결제가 포함된 B2B 플랫폼에서 CSRF 및 세션 탈취 위험을 크게 높입니다.credentials: true를 사용할 때는 허용 출처를 명시적인 도메인 목록으로만 제한해야 합니다.🛡️ 허용 출처 명시적 제한 예시
app.enableCors({ - origin: origins.includes("*") ? true : origins, + origin: origins, // 와일드카드(*) 허용 금지; 환경변수에서 명시적 도메인만 설정 credentials: true, });환경변수(
cors.origins)에 절대로"*"가 포함되지 않도록 설정 유효성 검사를 추가하거나, 스타트업/도커 시작 시점에 경고를 출력하는 것을 권장합니다.🤖 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 `@src/main.ts` around lines 60 - 63, The CORS setup using app.enableCors with origin: origins.includes("*") ? true : origins plus credentials: true permits credentialed requests from any origin when cors.origins contains "*"; change this by validating the origins array (the origins variable / cors.origins env) at startup: if credentials is true (or credentials is always true in config) reject or remove wildcard entries and require an explicit domain list, log a clear error/warning and exit/startup-fail (or set credentials to false) so app.enableCors is only called with an explicit origins array (not true) when credentials are enabled; add a startup validation function to enforce cors.origins does not include "*" and reference this check before calling app.enableCors.
60-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
origin: true+credentials: true조합 — XRPL 결제 플랫폼에서 심각한 보안 위험
origins배열에"*"가 포함될 경우origin: true로 설정되어 모든 출처에서의 인증 요청(세션 쿠키 포함)이 허용됩니다. 해외 바이어와 국내 스타트업을 연결하는 B2B 플랫폼이며 XRPL 결제가 포함된 서비스에서 이 조합은 CSRF 및 세션 탈취 공격의 직접적인 경로가 됩니다.credentials: true사용 시 허용 출처를 반드시 명시적인 도메인 목록으로만 제한해야 합니다.🛡️ 와일드카드 출처 방어 예시
+ // credentials: true와 함께 wildcard origins는 허용 금지 + if (origins.includes("*")) { + throw new Error("Wildcard CORS origin is not allowed when credentials are enabled"); + } + app.enableCors({ - origin: origins.includes("*") ? true : origins, + origin: origins, credentials: true, });🤖 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 `@src/main.ts` around lines 60 - 63, The CORS config uses origin: true with credentials: true when origins includes "*", which allows credentials for any origin—change app.enableCors usage to restrict credentials to explicit whitelisted domains: validate the origins array (variable origins) and if it contains "*" do not set credentials: true (either set credentials: false or reject wildcard); instead provide a whitelist-based origin handler (pass a function that checks request origin against origins) so that app.enableCors({ origin, credentials }) only allows credentials for explicitly listed domains. Ensure you update the code paths that reference app.enableCors, origins, origin, and credentials to perform this whitelist check and fail-safe when wildcard is present.
🧹 Nitpick comments (2)
src/main.ts (2)
49-50: 💤 Low value
process.env.NODE_ENV를configService대신 직접 사용파일 내 다른 모든 설정값은
configService를 통해 읽지만, 변경된 두 줄만process.env.NODE_ENV를 직접 참조하고 있습니다. NestJS의ConfigModule을 통해 일관되게 관리하는 것을 권장합니다.♻️ ConfigService를 통한 일관성 있는 접근
+ const isProduction = configService.get<string>("NODE_ENV") === "production"; cookie: { httpOnly: true, - secure: process.env.NODE_ENV === "production", - sameSite: process.env.NODE_ENV === "production" ? "none" : "lax", + secure: isProduction, + sameSite: isProduction ? "none" : "lax", maxAge: sessionTtl * 1000, },🤖 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 `@src/main.ts` around lines 49 - 50, The cookie options currently read process.env.NODE_ENV directly for secure and sameSite; replace those direct env accesses with the ConfigService so configuration is consistent—use the existing configService (or the same ConfigService instance used elsewhere) to derive the environment flag and compute secure and sameSite values for the cookie options (the properties named secure and sameSite) instead of process.env.NODE_ENV.
49-50: ⚡ Quick win
express-session1.19.0의sameSite: 'auto'로 단순화 가능
express-session1.19.0은sameSite: 'auto'옵션을 도입했으며, 이는 HTTPS 연결에서 자동으로SameSite=None을, HTTP 연결에서SameSite=Lax를 설정합니다. 현재 lines 49-50의 수동 조건 분기를 이 옵션으로 대체하면 코드가 단순해지고trust proxy설정과 연동되어 더 안정적으로 동작합니다.♻️ `sameSite: 'auto'` 활용 예시
cookie: { httpOnly: true, - secure: process.env.NODE_ENV === "production", - sameSite: process.env.NODE_ENV === "production" ? "none" : "lax", + secure: "auto", // trust proxy 설정과 연동되어 자동으로 HTTPS 여부 판단 + sameSite: "auto", // HTTPS → none, HTTP → lax 자동 처리 maxAge: sessionTtl * 1000, },
secure: "auto"역시express-session1.17.0+에서 지원되며,trust proxy설정에 따라 자동으로 HTTPS/HTTP를 판단합니다.🤖 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 `@src/main.ts` around lines 49 - 50, Replace the manual environment checks for cookie options with the built-in automatic mode: update the session options used where you set secure and sameSite (the object passed to express-session in main.ts) to use secure: "auto" and sameSite: "auto" instead of the ternary branches, and ensure the Express app has trust proxy configured (app.set('trust proxy', true)) so secure:auto and sameSite:auto can correctly detect HTTPS via proxy.
🤖 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 `@src/main.ts`:
- Around line 49-50: The session cookie options use secure/sameSite based on
NODE_ENV but the app never enables Express trust proxy, so behind a reverse
proxy req.secure will be false and secure cookies won't be sent; add a
conditional trust proxy setting (e.g., call app.set('trust proxy', 1) or
app.enable('trust proxy') when process.env.NODE_ENV === 'production') near the
Express app initialization so express-session and the secure/sameSite options
(the lines setting secure and sameSite) will honor X-Forwarded-Proto from
proxies.
- Around line 49-50: 프로덕션에서 secure 쿠키가 리버스 프록시(AWS ALB/nginx 등) 뒤에서도 정상 동작하도록
Express에 프록시를 신뢰하도록 설정해야 합니다; main.ts에서 app 변수(예: app = express())를 생성한 직후에
app.set('trust proxy', 1) 또는 적절한 값으로 프록시 신뢰를 설정하고 이 설정이 세션/쿠키 미들웨어를 등록하기 전에
적용되도록 하세요 — 현재 diff의 secure: process.env.NODE_ENV === "production" 및 sameSite
설정과 함께 사용되도록 위치를 조정하면 됩니다.
---
Outside diff comments:
In `@src/main.ts`:
- Around line 60-63: The CORS setup using app.enableCors with origin:
origins.includes("*") ? true : origins plus credentials: true permits
credentialed requests from any origin when cors.origins contains "*"; change
this by validating the origins array (the origins variable / cors.origins env)
at startup: if credentials is true (or credentials is always true in config)
reject or remove wildcard entries and require an explicit domain list, log a
clear error/warning and exit/startup-fail (or set credentials to false) so
app.enableCors is only called with an explicit origins array (not true) when
credentials are enabled; add a startup validation function to enforce
cors.origins does not include "*" and reference this check before calling
app.enableCors.
- Around line 60-63: The CORS config uses origin: true with credentials: true
when origins includes "*", which allows credentials for any origin—change
app.enableCors usage to restrict credentials to explicit whitelisted domains:
validate the origins array (variable origins) and if it contains "*" do not set
credentials: true (either set credentials: false or reject wildcard); instead
provide a whitelist-based origin handler (pass a function that checks request
origin against origins) so that app.enableCors({ origin, credentials }) only
allows credentials for explicitly listed domains. Ensure you update the code
paths that reference app.enableCors, origins, origin, and credentials to perform
this whitelist check and fail-safe when wildcard is present.
---
Nitpick comments:
In `@src/main.ts`:
- Around line 49-50: The cookie options currently read process.env.NODE_ENV
directly for secure and sameSite; replace those direct env accesses with the
ConfigService so configuration is consistent—use the existing configService (or
the same ConfigService instance used elsewhere) to derive the environment flag
and compute secure and sameSite values for the cookie options (the properties
named secure and sameSite) instead of process.env.NODE_ENV.
- Around line 49-50: Replace the manual environment checks for cookie options
with the built-in automatic mode: update the session options used where you set
secure and sameSite (the object passed to express-session in main.ts) to use
secure: "auto" and sameSite: "auto" instead of the ternary branches, and ensure
the Express app has trust proxy configured (app.set('trust proxy', true)) so
secure:auto and sameSite:auto can correctly detect HTTPS via proxy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
1 participant
Resolves #16
개요
개발 단계에서 지속적 로그인 풀리는 현상 발생
해결
Summary by CodeRabbit
릴리스 노트