fix: harden ASCA build and dependency baseline#36
Open
jlinsights wants to merge 2 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note
|
| Layer / File(s) | Summary |
|---|---|
CI 환경 변수 주입 및 검증 .github/workflows/ci.yml, .github/workflows/e2e-tests.yml, scripts/check-env-vars.js |
CI 빌드·E2E 워크플로우에서 .env.test 생성/복사 및 DATABASE_URL, Supabase/Clerk 키, NEXT_PUBLIC_APP_URL 등을 빌드 env로 주입하고, scripts/check-env-vars.js로 필수/선택 환경변수 구조 검증을 수행합니다. |
런타임 환경 및 의존성 업데이트 package.json |
Node/NPM 엔진 요구사항 상향 및 build:compile/test:ci 스크립트 조정, Clerk/Apollo/dompurify/Next/React 등 주요 의존성 버전 갱신과 일부 패키지 대체가 이루어졌습니다. |
멤버 API 재구성 및 테스트 업데이트 app/api/members/route.ts, app/api/members/__tests__/route.test.ts, e2e/api/members.spec.ts |
GET/POST 엔드포인트가 재구성되어 E2E용 폴백 데이터, public 필드 선택, 검증·에러 응답 변경, POST 생성은 mock 응답으로 대체되었고 단위·E2E 테스트가 새 응답 스키마·인증/CSRF 시나리오에 맞춰 갱신되었습니다. |
인증·미들웨어·클라이언트 공급자 변경 middleware.ts, components/client-providers.tsx, components/header/header-auth-section.tsx |
CSRF 검사 로직을 분리해 securityMiddleware로 만들고 Clerk 사용 여부(NEXT_PUBLIC_E2E_DISABLE_CLERK)에 따라 미들웨어/클라이언트 providers 및 헤더 인증 UI를 조건부로 우회하도록 변경했습니다. |
Next/Vercel 설정·정적 보안 변경 next.config.js, vercel.json, lib/security/sanitize.ts |
이미지 최적화 설정에서 SVG 비허용 및 remotePatterns 호스트 제한, experimental CSS 최적화 제거, vercel.json의 headers 블록 제거, DOMPurify import를 dompurify로 교체했습니다. |
테스트 아티팩트 무시 및 리포트 정리 .gitignore, .prettierignore, e2e/community.brand-rollout.spec.ts |
Playwright 산출물 디렉터리를 무시하도록 추가하고 일부 리포트가 제거되었으며, UI E2E 테스트의 기준 로케이터가 소폭 변경되었습니다. |
예상 검토 복잡도
🎯 4 (Complex) | ⏱️ ~45 minutes
전체 변경 사항 (업데이트된 기존 섹션)
프로젝트 종합 업그레이드를 진행하여 Node/NPM 런타임 버전을 상향 조정, 주요 프레임워크 및 라이브러리 의존성을 업데이트하고, CI 환경 변수 주입 및 검증 자동화를 구현하며, Next.js와 Vercel 보안 설정을 조정하고 테스트 아티팩트를 정리함.
변경 사항 (업데이트된 기존 표)
종합 업그레이드 및 유지보수
| 레이어 / 파일 | 요약 |
|---|---|
런타임 환경 및 의존성 업데이트 package.json |
Node 엔진을 >=20.0.0 <23.0.0, NPM을 >=10.0.0으로 상향 조정하고, Apollo Server, Clerk, Drizzle ORM, Next.js, React, DOMPurify 등 주요 패키지 버전을 갱신하며 build:compile/test:ci 스크립트를 조정. |
CI 빌드 환경 설정 및 환경 변수 검증 .github/workflows/ci.yml, .github/workflows/e2e-tests.yml, scripts/check-env-vars.js |
CI 빌드 단계에서 PostgreSQL, Supabase, Clerk 환경 변수를 주입하고, .env.test를 CI 더미로 생성·복사하며, Node CLI로 필수/선택 환경 변수를 정규식 및 길이 기반 검증 후 실패 시 명확한 에러와 종료를 제공합니다. |
Next.js 및 보안 설정 변경 next.config.js, vercel.json, lib/security/sanitize.ts |
이미지 최적화에서 SVG 허용 비활성화, 원격 호스트를 Supabase/Curator로 제한, experimental CSS 최적화 제거, Vercel headers 구성 삭제, DOMPurify 모듈을 dompurify로 교체. |
멤버 API 재구성 및 테스트 조정 app/api/members/route.ts, app/api/members/__tests__/route.test.ts, e2e/api/members.spec.ts |
공개 멤버 조회/생성 로직 재구성(폴백, public select, 검증/에러 처리), POST는 mock 응답으로 대체; 단위·E2E 테스트가 새 응답 스키마·인증/CSRF 케이스로 갱신. |
테스트 아티팩트 및 무시 규칙 .gitignore, .prettierignore, playwright-report/*, test-results/* |
Playwright 산출물 디렉터리를 .gitignore/.prettierignore에 추가하고, 이전 실행 리포트 일부를 제거하여 리포트 아티팩트를 정리. |
관련된 가능성 있는 PR
- jlinsights/ASCA#3: CI 빌드에서 주입되는 환경 변수(
ALLOWED_ORIGINS등)가 GraphQL CORS 검증에 직접 사용되므로 이 PR의 환경 변수 설정과 상관관계가 있음.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 18.18% 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 |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목은 주요 변경 사항인 ASCA 빌드 강화 및 의존성 기준선 패치를 명확하게 요약하고 있으며, 전체 changeset의 핵심 목표를 정확하게 반영합니다. |
| 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. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ 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
fix/ops-quality-p0-p1
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 help to get the list of available commands and usage tips.
jlinsights
force-pushed
the
fix/ops-quality-p0-p1
branch
from
daa6530 to
1d55ae0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@next.config.js`:
- Around line 36-49: The production Content-Security-Policy's img-src is missing
the Supabase host allowed by remotePatterns; update the production CSP (the
variable/string that defines img-src) to include the same Supabase entry used in
remotePatterns (e.g., allow *.supabase.co or the exact Supabase CDN host) so
browser image fetches from Supabase are permitted—ensure the CSP img-src and
next.config.js remotePatterns (the remotePatterns array) stay consistent (also
apply the same change where the other CSP block is defined).
- Around line 41-48: Remove the redundant specific host entry by deleting the
object with hostname: 'cdn.curator.io' and keep the wildcard object with
hostname: '**.curator.io' (the entries where protocol: 'https' and hostname are
defined) so the CDN host is covered by the wildcard; update any nearby comments
to reflect that the wildcard includes subdomains.
In `@package.json`:
- Line 157: The package.json currently upgrades "next" to 16.2.7 (and
`@next/bundle-analyzer` to 16.2.7) but leaves "eslint-config-next" at 15.5.19;
update the "eslint-config-next" dependency in package.json to the corresponding
16.x version (e.g., 16.2.7 or a compatible ^16.x range) so ESLint rules match
the Next.js version, then run npm/yarn install and validate linting; look for
the "eslint-config-next" entry in package.json to make the change.
In `@scripts/check-env-vars.js`:
- Around line 45-50: The predicate for the optional env var entry for
'SUPABASE_SERVICE_ROLE_KEY' in the optional array is wrong (value => !value ||
value.length > 0) and lets empty string pass; change the predicate to explicitly
allow undefined/null but reject empty strings — e.g., replace the predicate with
one that returns true for null/undefined and true only when value.length > 0
(for example value => value === undefined || value === null || value.length > 0)
so the "cannot be empty when set" rule is enforced for the
'SUPABASE_SERVICE_ROLE_KEY' entry.
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 32f02922-bd69-4580-a174-16820b10bba6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.github/workflows/ci.yml.gitignore.prettierignorelib/security/sanitize.tsnext.config.jspackage.jsonplaywright-report/index.htmlplaywright-report/results.jsonscripts/check-env-vars.jstest-results/.last-run.jsonvercel.json
💤 Files with no reviewable changes (2)
- playwright-report/results.json
- test-results/.last-run.json
Comment on lines
36
to
49
| remotePatterns: [ | ||
| { | ||
| protocol: 'https', | ||
| hostname: '**', | ||
| hostname: '**.supabase.co', | ||
| }, | ||
| { | ||
| protocol: 'https', | ||
| hostname: 'cdn.curator.io', | ||
| }, | ||
| { | ||
| protocol: 'https', | ||
| hostname: '**.curator.io', | ||
| }, | ||
| ], |
There was a problem hiding this comment.
CSP img-src와 remotePatterns 간 도메인 불일치.
remotePatterns에 **.supabase.co가 허용되어 있지만, production CSP의 img-src에는 Supabase 도메인이 포함되어 있지 않습니다. Supabase에서 호스팅되는 이미지가 브라우저 CSP에 의해 차단될 수 있습니다.
🔒 CSP에 Supabase 도메인 추가
- : "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cdn.curator.io https://*.curator.io; style-src 'self' 'unsafe-inline' https://cdn.curator.io; img-src 'self' data: blob: https://cdn.curator.io https://*.curator.io; font-src 'self' data:; connect-src 'self' https://cdn.curator.io https://*.curator.io; frame-src 'self' https://cdn.curator.io https://*.curator.io; object-src 'none'; base-uri 'self'; form-action 'self';",
+ : "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cdn.curator.io https://*.curator.io; style-src 'self' 'unsafe-inline' https://cdn.curator.io; img-src 'self' data: blob: https://*.supabase.co https://cdn.curator.io https://*.curator.io; font-src 'self' data:; connect-src 'self' https://*.supabase.co https://cdn.curator.io https://*.curator.io; frame-src 'self' https://cdn.curator.io https://*.curator.io; object-src 'none'; base-uri 'self'; form-action 'self';",Also applies to: 139-143
🤖 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 `@next.config.js` around lines 36 - 49, The production
Content-Security-Policy's img-src is missing the Supabase host allowed by
remotePatterns; update the production CSP (the variable/string that defines
img-src) to include the same Supabase entry used in remotePatterns (e.g., allow
*.supabase.co or the exact Supabase CDN host) so browser image fetches from
Supabase are permitted—ensure the CSP img-src and next.config.js remotePatterns
(the remotePatterns array) stay consistent (also apply the same change where the
other CSP block is defined).
Comment on lines
+41
to
48
| { | ||
| protocol: 'https', | ||
| hostname: 'cdn.curator.io', | ||
| }, | ||
| { | ||
| protocol: 'https', | ||
| hostname: '**.curator.io', | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
cdn.curator.io 패턴이 **.curator.io와 중복됩니다.
**.curator.io 와일드카드 패턴이 cdn.curator.io를 포함하므로 별도 항목이 불필요합니다.
♻️ 중복 제거 제안
{
protocol: 'https',
hostname: '**.supabase.co',
},
- {
- protocol: 'https',
- hostname: 'cdn.curator.io',
- },
{
protocol: 'https',
hostname: '**.curator.io',
},📝 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.
Suggested change
| { | |
| protocol: 'https', | |
| hostname: 'cdn.curator.io', | |
| }, | |
| { | |
| protocol: 'https', | |
| hostname: '**.curator.io', | |
| }, | |
| { | |
| protocol: 'https', | |
| hostname: '**.curator.io', | |
| }, |
🤖 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 `@next.config.js` around lines 41 - 48, Remove the redundant specific host
entry by deleting the object with hostname: 'cdn.curator.io' and keep the
wildcard object with hostname: '**.curator.io' (the entries where protocol:
'https' and hostname are defined) so the CDN host is covered by the wildcard;
update any nearby comments to reflect that the wildcard includes subdomains.
| "lucide-react": "^0.454.0", | ||
| "mini-svg-data-uri": "^1.4.4", | ||
| "next": "^16.0.10", | ||
| "next": "16.2.7", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
eslint-config-next version 16 availability npm
💡 Result:
Yes, eslint-config-next version 16 is available on npm [1][2]. As of June 5, 2026, the latest stable release is version 16.2.6, which was published on May 7, 2026 [1][2]. You can install it using your preferred package manager, typically as a development dependency [3]: npm install -D eslint-config-next@16.2.6 Note that in Next.js 16, the next lint command and the eslint option in next.config.js were removed in favor of using the standard ESLint CLI directly [4]. Existing projects can use an available codemod to assist with this migration [4].
Citations:
- 1: https://npmx.dev/package/eslint-config-next/v/%3E=16.0.0
- 2: https://www.npmjs.com/package/eslint-config-next?activeTab=versions
- 3: https://npmx.dev/package/eslint-config-next
- 4: https://nextjs.org/docs/app/api-reference/config/eslint
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate package.json
PKG="$(fd -a -e package.json package.json -t f . | head -n 1 || true)"
if [ -z "$PKG" ]; then
echo "package.json not found"
exit 1
fi
echo "Found: $PKG"
echo "---- next / eslint-config-next / `@next/bundle-analyzer` versions ----"
# Print the relevant dependencies lines with a little context
rg -n '"next"\s*:|eslint-config-next|`@next/bundle-analyzer`' "$PKG" -n --context 2 || true
echo "---- surrounding line 157, 182, 200 if file is large enough ----"
# Use sed to show those line ranges (ignore if out of bounds)
for L in 157 182 200; do
echo "### around line $L"
sed -n "$((L-5)),$((L+5))p" "$PKG" | cat -n
doneRepository: jlinsights/ASCA
Length of output: 82
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate package.json
PKG="$(fd -a -e package.json package.json -t f . | head -n 1 || true)"
if [ -z "$PKG" ]; then
echo "package.json not found"
exit 1
fi
echo "Found: $PKG"
echo "---- next / eslint-config-next / `@next/bundle-analyzer` versions ----"
rg -n '"next"\s*:|eslint-config-next|`@next/bundle-analyzer`' "$PKG" -n --context 2 || true
echo "---- surrounding line 157, 182, 200 if file is large enough ----"
for L in 157 182 200; do
echo "### around line $L"
sed -n "$((L-5)),$((L+5))p" "$PKG" | cat -n
doneRepository: jlinsights/ASCA
Length of output: 82
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for package.json =="
find . -name package.json -print | head -n 20
echo
echo "== Search for any npm/next related files =="
find . -maxdepth 4 -type f \( -name "package-lock.json" -o -name "pnpm-lock.yaml" -o -name "yarn.lock" -o -name "next.config.*" -o -name "tsconfig.json" \) -print | head -n 50
# If a package.json is found, extract versions from the first one.
PKG="$(find . -name package.json -print | head -n 1 || true)"
if [ -n "$PKG" ]; then
echo
echo "== Using: $PKG =="
echo "---- next / eslint-config-next / `@next/bundle-analyzer` ----"
rg -n '"next"\s*:|eslint-config-next|`@next/bundle-analyzer`' "$PKG" --context 2 || true
echo
echo "---- show around lines 150-210 (best-effort) ----"
for L in 157 182 200; do
echo "### around line $L"
sed -n "$((L-5)),$((L+5))p" "$PKG" | cat -n
done
fiRepository: jlinsights/ASCA
Length of output: 1266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate project package.json (excluding node_modules) =="
PKG="$(find . -name package.json -not -path './node_modules/*' -print | head -n 20 | head -n 1 || true)"
echo "PKG=$PKG"
if [ -z "${PKG}" ]; then
echo "No project package.json found (excluding node_modules)."
exit 1
fi
echo
echo "== Versions in $PKG =="
rg -n '"next"\s*:|eslint-config-next|`@next/bundle-analyzer`' "$PKG" --context 2 || true
echo
echo "== show around lines 150-210 (for referenced line checks) =="
sed -n '140,220p' "$PKG" | cat -nRepository: jlinsights/ASCA
Length of output: 4317
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate project package.json (excluding node_modules) =="
PKG="$(find . -name package.json -not -path './node_modules/*' -print | head -n 20 | head -n 1 || true)"
echo "PKG=$PKG"
if [ -z "${PKG}" ]; then
echo "No project package.json found (excluding node_modules)."
exit 1
fi
echo
echo "== Versions in $PKG =="
rg -n '"next"\s*:|eslint-config-next|`@next/bundle-analyzer`' "$PKG" --context 2 || true
echo
echo "== show around lines 140-220 (for referenced line checks) =="
sed -n '140,220p' "$PKG" | cat -nRepository: jlinsights/ASCA
Length of output: 4317
eslint-config-next 버전이 next(16.2.7)와 불일치합니다.
"next": "16.2.7",package.json에서 next는 16.2.7(157)로 업그레이드됐고 @next/bundle-analyzer도 16.2.7(182)로 맞췄는데, eslint-config-next만 15.5.19(200)로 유지돼 있습니다. ESLint 규칙이 Next 16에 맞게 동작하도록 eslint-config-next도 16.x로 올리는 것을 권장합니다.
🤖 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 `@package.json` at line 157, The package.json currently upgrades "next" to
16.2.7 (and `@next/bundle-analyzer` to 16.2.7) but leaves "eslint-config-next" at
15.5.19; update the "eslint-config-next" dependency in package.json to the
corresponding 16.x version (e.g., 16.2.7 or a compatible ^16.x range) so ESLint
rules match the Next.js version, then run npm/yarn install and validate linting;
look for the "eslint-config-next" entry in package.json to make the change.
Comment on lines
+45
to
+50
| const optional = [ | ||
| [ | ||
| 'SUPABASE_SERVICE_ROLE_KEY', | ||
| value => !value || value.length > 0, | ||
| 'is optional but cannot be empty when set', | ||
| ], |
There was a problem hiding this comment.
SUPABASE_SERVICE_ROLE_KEY 유효성 검사 로직에 버그가 있습니다.
Line 48의 value => !value || value.length > 0 조건은 빈 문자열('')도 통과시킵니다. JavaScript에서 빈 문자열은 falsy이므로 !''가 true가 되어 항상 유효성 검사를 통과합니다. 이는 "cannot be empty when set" 메시지와 모순됩니다.
🐛 수정 제안
const optional = [
[
'SUPABASE_SERVICE_ROLE_KEY',
- value => !value || value.length > 0,
+ value => value.length > 0,
'is optional but cannot be empty when set',
],🤖 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 `@scripts/check-env-vars.js` around lines 45 - 50, The predicate for the
optional env var entry for 'SUPABASE_SERVICE_ROLE_KEY' in the optional array is
wrong (value => !value || value.length > 0) and lets empty string pass; change
the predicate to explicitly allow undefined/null but reject empty strings —
e.g., replace the predicate with one that returns true for null/undefined and
true only when value.length > 0 (for example value => value === undefined ||
value === null || value.length > 0) so the "cannot be empty when set" rule is
enforced for the 'SUPABASE_SERVICE_ROLE_KEY' entry.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/members/route.ts (1)
30-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRate limit 키에 신뢰할 수 없는
x-user-id헤더를 사용하고 있습니다.클라이언트가 헤더 값을 바꿔 제한을 우회할 수 있습니다. 최소한 서버가 신뢰 가능한 값(IP 정규화 또는 서버 측 인증 주체)만 키로 사용해야 합니다.
수정 예시
const readLimiter = rateLimit({ ...RateLimitPresets.moderate, // 50 requests per minute keyGenerator: req => { - const userId = req.headers.get('x-user-id') - return userId || req.headers.get('x-forwarded-for') || 'anonymous' + const forwardedFor = req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() + return forwardedFor || 'anonymous' }, }) const writeLimiter = rateLimit({ ...RateLimitPresets.strict, // 10 requests per minute keyGenerator: req => { - const userId = req.headers.get('x-user-id') - return userId || req.headers.get('x-forwarded-for') || 'anonymous' + const forwardedFor = req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() + return forwardedFor || 'anonymous' }, })Also applies to: 38-41
🤖 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/api/members/route.ts` around lines 30 - 33, The rate-limit keyGenerator is using the untrusted header 'x-user-id' (keyGenerator: req => { ... }) which clients can spoof; replace this by a trusted identifier: prefer the server-authenticated subject (e.g., user id from your auth/session retrieval function or middleware), otherwise derive a normalized client IP from the request's socket/remoteAddress (not raw 'x-forwarded-for') with proper parsing and fallback to a strict anonymous bucket; update both occurrences of keyGenerator in this file to fetch and validate the server-side user id (or normalized IP) instead of directly reading req.headers.get('x-user-id').
♻️ Duplicate comments (2)
scripts/check-env-vars.js (2)
45-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win옵션 변수 검증이 빈 문자열(
"")을 허용합니다.
!value조건 때문에SUPABASE_SERVICE_ROLE_KEY와NEXT_PUBLIC_APP_URL가 비어 있어도 통과합니다. “설정된 경우 유효해야 함” 규칙과 충돌합니다.수정 제안
const optional = [ [ 'SUPABASE_SERVICE_ROLE_KEY', - value => !value || value.length > 0, + value => value.length > 0, 'is optional but cannot be empty when set', ], [ 'NEXT_PUBLIC_APP_URL', - value => !value || /^https?:\/\//.test(value), + value => /^https?:\/\//.test(value), 'must be an HTTP(S) URL when set', ], ]🤖 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 `@scripts/check-env-vars.js` around lines 45 - 55, The optional validators currently use "!value" which treats empty string as unset and lets "" pass; change the predicates for 'SUPABASE_SERVICE_ROLE_KEY' and 'NEXT_PUBLIC_APP_URL' to only treat null/undefined as unset (e.g. use value == null) and reject an empty string when the var is set — for example update the 'SUPABASE_SERVICE_ROLE_KEY' predicate to "value == null || value.length > 0" and the 'NEXT_PUBLIC_APP_URL' predicate to "value == null || (value.length > 0 && /^https?:\/\//.test(value))" so empty strings are no longer accepted while unset variables remain allowed.
11-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
scripts/check-env-vars.jsenv 파일 로딩 우선순위와 optional 검증 로직 수정 필요
dotenv.config({ override: false })상태에서 envFiles가.env→.env.local→.env.${NODE_ENV}→.env.test순서라, 먼저 로드된.env값이 유지되어.env.local/환경별 파일 값이 제대로 우선 반영되지 않습니다(11-23)..env.test는NODE_ENV와 무관하게 항상 로드 대상으로 포함되어, 앞선 파일에 해당 값이 없으면 test 값이 섞일 수 있습니다(11-23).SUPABASE_SERVICE_ROLE_KEYoptional validatorvalue => !value || value.length > 0는 빈 문자열''을 통과시키는데, 메시지(“cannot be empty when set”)와 모순됩니다(47-66).수정 제안 (우선순위 정렬 + 중복 제거)
-const envFiles = [ - '.env', - '.env.local', - `.env.${process.env.NODE_ENV || 'development'}`, - '.env.test', -] +const nodeEnv = process.env.NODE_ENV || 'development' +const envFiles = Array.from( + new Set([ + nodeEnv === 'test' ? '.env.test' : `.env.${nodeEnv}`, + '.env.local', + '.env', + ]) +)🤖 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 `@scripts/check-env-vars.js` around lines 11 - 23, The env loading order and validator are wrong: reverse the envFiles priority so environment-specific and .local override defaults (make envFiles order like '.env', `.env.${NODE_ENV||'development'}`, '.env.local' and skip duplicates), load each existing file with dotenv.config({ path: fullPath, override: true }) so later files overwrite earlier ones, and remove unconditional inclusion of '.env.test' (only include it when NODE_ENV === 'test'); also fix the SUPABASE_SERVICE_ROLE_KEY validator (in the validator for SUPABASE_SERVICE_ROLE_KEY replace value => !value || value.length > 0 with a check that rejects empty strings when set, e.g. value => value === undefined || value.length > 0, and ensure the validation message matches).
🤖 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/api/members/__tests__/route.test.ts`:
- Around line 180-183: In the beforeEach block that sets up mockAuth, explicitly
set process.env.NEXT_PUBLIC_E2E_DISABLE_CLERK to a deterministic value (e.g.,
'false') so tests don't pick up external environment overrides, and add an
afterEach to restore the original env value; update the existing beforeEach (and
add afterEach) to save the current process.env.NEXT_PUBLIC_E2E_DISABLE_CLERK,
set it to the fixed value, then restore it after each test to keep tests
isolated.
In `@app/api/members/route.ts`:
- Around line 121-123: The helper shouldUseFallbackData currently reads a public
env var NEXT_PUBLIC_E2E_DISABLE_CLERK which risks accidental client/public
misconfiguration; change it to read a server-only flag (e.g. E2E_DISABLE_CLERK
or NEXT_DISABLE_CLERK) and add a strong guard such as checking
process.env.NODE_ENV === 'test' (or another dedicated server-only test flag)
before returning true; update all other usages (the similar checks around the
other occurrences noted) to use the new server-only env name and the
NODE_ENV/test guard so the server cannot be forced into fallback data by a
public NEXT_PUBLIC_* variable.
- Around line 137-144: The fallback in-memory filter includes fullNameEn but the
database query omitted it, causing inconsistent search results; update the DB
query used in the members route handler so its WHERE/OR conditions include
fullNameEn (in addition to fullName and fullNameKo) when params.query is
present, and apply the same fix to the other search block referenced around the
185-190 range so both DB and fallback searches match the same fields.
- Around line 252-257: The route currently stubs the duplicate-email check (the
withPerformanceLog('members.checkEmail') block returning []) and returns a
mocked created member, causing POST to respond 201 without persisting; replace
the stubbed check and mock creation with actual DB operations: in the
withPerformanceLog('members.checkEmail') callback query the members (or linked
user profile) table for the incoming email to detect duplicates, throw or return
a 409 when found, and then call the real persistent create function (e.g. your
member creation service or ORM method used elsewhere) instead of a mock so the
route returns the persisted record; update the code paths referenced around the
existing variable and the creation block currently at lines ~266-275 to use the
real DB client/service and return the saved entity before sending 201.
- Around line 169-171: The GET (and POST) route handlers in
app/api/members/route.ts currently skip API key header validation; before
calling readLimiter.check or processing the request, add a header validation
step (e.g., call a shared function validateApiKey(request) or
checkApiKeyHeader(request.headers.get('x-api-key'))) inside the exported GET and
POST functions, and return an appropriate 401/403 Response when the header is
missing or invalid; place this check before readLimiter.check and any business
logic so all requests to these route handlers are rejected early if the API key
fails validation.
- Around line 241-244: 현재 POST 멤버 생성 경로가 getAuthenticatedUserId()로 로그인만 확인하고 있어
관리자 권한 검증이 빠져 있습니다; 수정 방법은 getAuthenticatedUserId() 호출 후 관리자 권한을 확인하도록 추가 검증을
넣고, 관리자가 아니면 ApiResponse.forbidden(...)을 반환하도록 하세요 — 예를 들어 호출 흐름 내에서
getAuthenticatedUserId() 결과로 사용자 ID를 얻은 다음 해당 ID로 사용자 레코드의 role을 조회하거나 기존
유틸(isAdminUser, userHasRole 등)이 있으면 이를 사용해 관리자 여부를 판단하고, 관리자일 때만 계속 진행하도록
create-members POST 핸들러(현재 해당 블록)에 권한 체크를 삽입하십시오.
In `@package.json`:
- Line 39: The "test:ci" npm script currently uses the Jest "--forceExit" flag
which can mask open-handle/resource leaks; locate the "test:ci" script entry in
package.json and remove "--forceExit" (or create an alternate script like
"test:ci:fast" that keeps it only if CI truly needs it) and instead add
"--detectOpenHandles" to the CI test command to surface leaks; ensure the change
references the "test:ci" script and the flags "--forceExit" and
"--detectOpenHandles" so CI still runs reliably while enabling detection of open
handles for debugging.
---
Outside diff comments:
In `@app/api/members/route.ts`:
- Around line 30-33: The rate-limit keyGenerator is using the untrusted header
'x-user-id' (keyGenerator: req => { ... }) which clients can spoof; replace this
by a trusted identifier: prefer the server-authenticated subject (e.g., user id
from your auth/session retrieval function or middleware), otherwise derive a
normalized client IP from the request's socket/remoteAddress (not raw
'x-forwarded-for') with proper parsing and fallback to a strict anonymous
bucket; update both occurrences of keyGenerator in this file to fetch and
validate the server-side user id (or normalized IP) instead of directly reading
req.headers.get('x-user-id').
---
Duplicate comments:
In `@scripts/check-env-vars.js`:
- Around line 45-55: The optional validators currently use "!value" which treats
empty string as unset and lets "" pass; change the predicates for
'SUPABASE_SERVICE_ROLE_KEY' and 'NEXT_PUBLIC_APP_URL' to only treat
null/undefined as unset (e.g. use value == null) and reject an empty string when
the var is set — for example update the 'SUPABASE_SERVICE_ROLE_KEY' predicate to
"value == null || value.length > 0" and the 'NEXT_PUBLIC_APP_URL' predicate to
"value == null || (value.length > 0 && /^https?:\/\//.test(value))" so empty
strings are no longer accepted while unset variables remain allowed.
- Around line 11-23: The env loading order and validator are wrong: reverse the
envFiles priority so environment-specific and .local override defaults (make
envFiles order like '.env', `.env.${NODE_ENV||'development'}`, '.env.local' and
skip duplicates), load each existing file with dotenv.config({ path: fullPath,
override: true }) so later files overwrite earlier ones, and remove
unconditional inclusion of '.env.test' (only include it when NODE_ENV ===
'test'); also fix the SUPABASE_SERVICE_ROLE_KEY validator (in the validator for
SUPABASE_SERVICE_ROLE_KEY replace value => !value || value.length > 0 with a
check that rejects empty strings when set, e.g. value => value === undefined ||
value.length > 0, and ensure the validation message matches).
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4781fb61-e253-4411-9316-ae4c5cab7661
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.github/workflows/ci.yml.github/workflows/e2e-tests.yml.gitignore.prettierignoreapp/api/members/__tests__/route.test.tsapp/api/members/route.tscomponents/client-providers.tsxcomponents/header/header-auth-section.tsxe2e/api/members.spec.tse2e/community.brand-rollout.spec.tslib/security/sanitize.tsmiddleware.tsnext.config.jspackage.jsonplaywright-report/index.htmlplaywright-report/results.jsonscripts/check-env-vars.jstest-results/.last-run.jsonvercel.json
💤 Files with no reviewable changes (2)
- playwright-report/results.json
- test-results/.last-run.json
Comment on lines
180
to
+183
| beforeEach(() => { | ||
| jest.clearAllMocks() | ||
| mockAuth.mockResolvedValue({ userId: 'test-user-id' }) | ||
| }) |
There was a problem hiding this comment.
테스트에서 인증 우회 플래그 환경변수를 명시적으로 고정하세요.
NEXT_PUBLIC_E2E_DISABLE_CLERK 값이 외부 환경에서 주입되면 이 스위트의 POST 기대값(201/409)이 비결정적으로 깨질 수 있습니다. beforeEach에서 명시적으로 해제/설정해 격리하는 편이 안전합니다.
🤖 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/api/members/__tests__/route.test.ts` around lines 180 - 183, In the
beforeEach block that sets up mockAuth, explicitly set
process.env.NEXT_PUBLIC_E2E_DISABLE_CLERK to a deterministic value (e.g.,
'false') so tests don't pick up external environment overrides, and add an
afterEach to restore the original env value; update the existing beforeEach (and
add afterEach) to save the current process.env.NEXT_PUBLIC_E2E_DISABLE_CLERK,
set it to the fixed value, then restore it after each test to keep tests
isolated.
Comment on lines
+121
to
+123
| function shouldUseFallbackData() { | ||
| return process.env.NEXT_PUBLIC_E2E_DISABLE_CLERK === 'true' | ||
| } |
There was a problem hiding this comment.
서버 동작 분기를 NEXT_PUBLIC_* 플래그에 의존하는 것은 위험합니다.
공개 환경변수 오설정 시 서버가 실제 DB 대신 fallback 데이터를 반환하는 동작으로 고정될 수 있습니다. 서버 전용 플래그로 분리하고 NODE_ENV === 'test' 같은 강한 가드가 필요합니다.
Also applies to: 126-127, 177-180
🤖 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/api/members/route.ts` around lines 121 - 123, The helper
shouldUseFallbackData currently reads a public env var
NEXT_PUBLIC_E2E_DISABLE_CLERK which risks accidental client/public
misconfiguration; change it to read a server-only flag (e.g. E2E_DISABLE_CLERK
or NEXT_DISABLE_CLERK) and add a strong guard such as checking
process.env.NODE_ENV === 'test' (or another dedicated server-only test flag)
before returning true; update all other usages (the similar checks around the
other occurrences noted) to use the new server-only env name and the
NODE_ENV/test guard so the server cannot be forced into fallback data by a
public NEXT_PUBLIC_* variable.
Comment on lines
+137
to
+144
| if (params.query) { | ||
| const query = params.query.toLowerCase() | ||
| data = data.filter( | ||
| member => | ||
| member.fullName.toLowerCase().includes(query) || | ||
| member.fullNameKo?.toLowerCase().includes(query) || | ||
| member.fullNameEn?.toLowerCase().includes(query) | ||
| ) |
There was a problem hiding this comment.
검색 조건이 fallback/DB 경로에서 불일치합니다.
fallback은 fullNameEn까지 검색하지만 DB 조회는 제외되어, 환경에 따라 검색 결과가 달라집니다.
Also applies to: 185-190
🤖 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/api/members/route.ts` around lines 137 - 144, The fallback in-memory
filter includes fullNameEn but the database query omitted it, causing
inconsistent search results; update the DB query used in the members route
handler so its WHERE/OR conditions include fullNameEn (in addition to fullName
and fullNameKo) when params.query is present, and apply the same fix to the
other search block referenced around the 185-190 range so both DB and fallback
searches match the same fields.
Comment on lines
169
to
171
| export async function GET(request: NextRequest) { | ||
| // Apply rate limiting | ||
| const rateLimitResponse = await readLimiter.check(request) | ||
| if (rateLimitResponse) return rateLimitResponse |
There was a problem hiding this comment.
API 키 헤더 검증이 라우트 전반에서 누락되었습니다.
현재 GET/POST 모두 API 키 검증 없이 처리됩니다. 인증 여부와 별개로 API 경계에서 키 검증을 선행해야 합니다.
As per coding guidelines, app/api/**/route.ts: Validate API key in request headers for all API routes.
Also applies to: 240-247
🤖 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/api/members/route.ts` around lines 169 - 171, The GET (and POST) route
handlers in app/api/members/route.ts currently skip API key header validation;
before calling readLimiter.check or processing the request, add a header
validation step (e.g., call a shared function validateApiKey(request) or
checkApiKeyHeader(request.headers.get('x-api-key'))) inside the exported GET and
POST functions, and return an appropriate 401/403 Response when the header is
missing or invalid; place this check before readLimiter.check and any business
logic so all requests to these route handlers are rejected early if the API key
fails validation.
Comment on lines
+241
to
+244
| const userId = await getAuthenticatedUserId() | ||
| if (!userId) { | ||
| return ApiResponse.unauthorized('Authentication required to create members') | ||
| } |
There was a problem hiding this comment.
POST 권한 검사가 “로그인 여부”까지만 확인됩니다.
멤버 생성은 데이터 변경 작업인데, 현재는 관리자 권한 검증 없이 인증 사용자면 통과합니다.
As per coding guidelines, app/api/**/route.ts: Enforce admin authorization on privileged endpoints (e.g., migration, sync, data mutations).
🤖 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/api/members/route.ts` around lines 241 - 244, 현재 POST 멤버 생성 경로가
getAuthenticatedUserId()로 로그인만 확인하고 있어 관리자 권한 검증이 빠져 있습니다; 수정 방법은
getAuthenticatedUserId() 호출 후 관리자 권한을 확인하도록 추가 검증을 넣고, 관리자가 아니면
ApiResponse.forbidden(...)을 반환하도록 하세요 — 예를 들어 호출 흐름 내에서 getAuthenticatedUserId()
결과로 사용자 ID를 얻은 다음 해당 ID로 사용자 레코드의 role을 조회하거나 기존 유틸(isAdminUser, userHasRole 등)이
있으면 이를 사용해 관리자 여부를 판단하고, 관리자일 때만 계속 진행하도록 create-members POST 핸들러(현재 해당 블록)에 권한
체크를 삽입하십시오.
Comment on lines
252
to
257
| const existing = await withPerformanceLog('members.checkEmail', async () => { | ||
| /* | ||
| return await db | ||
| .select() | ||
| .from(members) | ||
| .where(eq(members.email, body.email)) | ||
| .limit(1); | ||
| */ | ||
| // Email uniqueness currently belongs to the linked user profile table. | ||
| // Keep this hook so production code can attach the join without changing | ||
| // the route contract, while tests can verify duplicate handling. | ||
| return [] | ||
| }) |
There was a problem hiding this comment.
POST가 실제 저장 없이 201 성공을 반환합니다.
현재는 중복 확인도 스텁([])이고, 생성도 mock 객체를 반환해 영속 데이터와 API 응답이 불일치합니다. 클라이언트는 생성 성공으로 인식하지만 실제 데이터는 생성되지 않습니다.
Also applies to: 266-275
🤖 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/api/members/route.ts` around lines 252 - 257, The route currently stubs
the duplicate-email check (the withPerformanceLog('members.checkEmail') block
returning []) and returns a mocked created member, causing POST to respond 201
without persisting; replace the stubbed check and mock creation with actual DB
operations: in the withPerformanceLog('members.checkEmail') callback query the
members (or linked user profile) table for the incoming email to detect
duplicates, throw or return a 409 when found, and then call the real persistent
create function (e.g. your member creation service or ORM method used elsewhere)
instead of a mock so the route returns the persisted record; update the code
paths referenced around the existing variable and the creation block currently
at lines ~266-275 to use the real DB client/service and return the saved entity
before sending 201.
| "test:watchAll": "jest --watchAll", | ||
| "test:coverage": "jest --coverage", | ||
| "test:ci": "jest --ci --coverage --watchAll=false", | ||
| "test:ci": "jest --ci --coverage --watchAll=false --forceExit", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
--forceExit 사용 시 테스트 누수 경고가 숨겨질 수 있습니다.
--forceExit는 열린 핸들(DB 연결, 타이머 등)이 있어도 Jest를 강제 종료합니다. CI에서 hang을 방지하기 위해 흔히 사용되지만, 근본적인 리소스 누수 문제를 마스킹할 수 있습니다. PR 목표에서 언급된 대로 현재 CI 통과를 위해 필요하다면 유지하되, 추후 --detectOpenHandles로 원인을 파악하는 것을 권장합니다.
🤖 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 `@package.json` at line 39, The "test:ci" npm script currently uses the Jest
"--forceExit" flag which can mask open-handle/resource leaks; locate the
"test:ci" script entry in package.json and remove "--forceExit" (or create an
alternate script like "test:ci:fast" that keeps it only if CI truly needs it)
and instead add "--detectOpenHandles" to the CI test command to surface leaks;
ensure the change references the "test:ci" script and the flags "--forceExit"
and "--detectOpenHandles" so CI still runs reliably while enabling detection of
open handles for debugging.
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
1 participant
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.
Summary
Verification
npm run type-check✅npm run lint✅ (11 existing max-lines warnings only)npm run build✅ with CI-shaped placeholder envnpm run format:check✅npm run test:ci -- --runInBand --forceExit✅ 389/389npm auditimproved from Critical 2 / High 20 / Total 38 to Critical 0 / High 13 / Total 28Notes
/api/membersauth hardening and E2E failures follow-up.Summary by CodeRabbit
릴리스 노트
Chores
Bug Fixes / Behavior
Tests / CI