Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough이 변경 사항은 두 개의 핵심 파일을 수정합니다. 첫째, 관리자 역할 검증 로직을 단순한 boolean에서 네 가지 상태(authorized, unauthorized, forbidden, error)를 구분하는 discriminator 기반 시스템으로 전환하고, JWT 페이로드 디코딩을 폴백 메커니즘으로 추가했습니다. 둘째, 미들웨어에서 경로 프로브 공격을 필터링하는 차단 목록을 도입하고, robots.txt에 대한 특수 처리를 추가하며, 토큰 만료 확인을 제거하고 refreshToken 쿠키 존재 여부만으로 로그인 게이팅을 단순화했습니다. Estimated code review effortai-inspector-requests/route.ts🎯 4 (Complex) | ⏱️ ~45 minutes middleware.ts🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
변경 사항 상세 분석1. AI Inspector Requests 라우트 수정
2. 미들웨어 보안 및 효율성 개선
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d962ab250c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Fallback: /my 인증(200)은 통과했는데 응답 role 스키마가 다른 경우를 대비. | ||
| if (hasAdminRole(decodeTokenRole(accessToken))) { | ||
| return "authorized"; |
There was a problem hiding this comment.
Ignore token fallback when /my returns a non-admin role
In verifyAdminRole, the fallback to decodeTokenRole(accessToken) runs even after /my returned a parsed role that is explicitly non-admin. That means if token claims and server role ever diverge (for example, after role downgrade while an old token is still valid), a non-admin user can still be authorized for this admin-only endpoint. The token fallback should only apply when the /my role is missing/unparseable, not when a concrete non-admin role is returned.
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| if (needLogin && !hasValidRefreshToken) { | ||
| if (needLogin && !refreshToken) { |
There was a problem hiding this comment.
Reject expired refresh tokens in middleware guard
The middleware now allows protected pages whenever a refreshToken cookie exists, without checking whether that token is expired or malformed. This regresses the previous route guard behavior and lets stale sessions access /mentor, /my, and /community until downstream API calls fail, producing inconsistent auth flow and delayed rejection. The guard should continue validating token freshness (or an equivalent validity check) before permitting these routes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/api/ai-inspector-requests/route.ts`:
- Around line 116-118: The current handler in
apps/web/src/app/api/ai-inspector-requests/route.ts treats all non-ok upstream
responses as "forbidden"; update the logic that checks response (the response
object in this function) so only response.status === 403 returns "forbidden" and
any other non-OK status returns "error" (so 500/502/503 propagate as "error"
instead of being masked as forbidden). Locate the non-OK branch around the
response.ok check and change the conditional to distinguish 403 from other
statuses.
- Around line 120-131: The current fallback trusts JWT role even when the /my
response explicitly returns a non-admin role; change the logic so that you only
consult decodeTokenRole(accessToken) when getRoleFromMyResponse(data) yields no
usable role (null/undefined/invalid), and do not treat a parsed non-admin
response as eligible for fallback; also stop swallowing JSON parse errors from
response.json()—let parsing failures surface as errors (or return an error
result) instead of silently falling back to the token. Ensure you update the
block around response.json(), getRoleFromMyResponse, hasAdminRole and
decodeTokenRole accordingly so only missing responseRole triggers the token
check.
In `@apps/web/src/middleware.ts`:
- Around line 58-66: The current gate only checks presence of refreshToken
(variable refreshToken) causing expired tokens to pass; replace the simple
existence check with the unified server-side login check by calling the existing
isServerStateLogin() helper from apps/web/src/utils/isServerStateLogin.ts (or
replicate its exact validation logic) when computing needLogin and deciding
redirection, i.e., use isServerStateLogin(request.cookies / refreshToken) to
determine authenticated state and rely on that result in the if (needLogin &&
!...) branch so initial entry and subsequent redirect use the same expiration
logic.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: eea0136d-454c-49c3-9e93-661c5c0c4d01
📒 Files selected for processing (2)
apps/web/src/app/api/ai-inspector-requests/route.tsapps/web/src/middleware.ts
| if (!response.ok) { | ||
| return false; | ||
| return "forbidden"; | ||
| } |
There was a problem hiding this comment.
1. /my 업스트림 장애가 403으로 눌립니다.
여기서는 401만 제외하고 나머지 non-OK를 전부 forbidden으로 보내서, /my가 500/502/503을 돌려도 최종 응답이 권한 부족처럼 보입니다. 403만 forbidden으로 남기고 나머지는 error로 돌려야 아래 503 분기가 살아납니다.
🔧 제안 수정
- if (!response.ok) {
- return "forbidden";
- }
+ if (!response.ok) {
+ return response.status === 403 ? "forbidden" : "error";
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/api/ai-inspector-requests/route.ts` around lines 116 - 118,
The current handler in apps/web/src/app/api/ai-inspector-requests/route.ts
treats all non-ok upstream responses as "forbidden"; update the logic that
checks response (the response object in this function) so only response.status
=== 403 returns "forbidden" and any other non-OK status returns "error" (so
500/502/503 propagate as "error" instead of being masked as forbidden). Locate
the non-OK branch around the response.ok check and change the conditional to
distinguish 403 from other statuses.
| const data = (await response.json().catch(() => null)) as unknown; | ||
| const responseRole = getRoleFromMyResponse(data); | ||
| if (hasAdminRole(responseRole)) { | ||
| return "authorized"; | ||
| } | ||
|
|
||
| // Fallback: /my 인증(200)은 통과했는데 응답 role 스키마가 다른 경우를 대비. | ||
| if (hasAdminRole(decodeTokenRole(accessToken))) { | ||
| return "authorized"; | ||
| } | ||
|
|
||
| const data = (await response.json()) as { role?: string }; | ||
| return data.role === UserRole.ADMIN; | ||
| return "forbidden"; |
There was a problem hiding this comment.
2. 서버가 non-admin을 돌려줘도 토큰 role이 ADMIN이면 다시 승인됩니다.
이 fallback은 /my의 최신 권한 판단보다 JWT 페이로드를 더 신뢰해서, 관리자 권한이 회수된 직후에도 stale token으로 작업을 enqueue할 수 있습니다. fallback은 role 필드를 전혀 읽지 못한 경우에만 제한하고, JSON 파싱 실패는 error로 올리는 편이 안전합니다.
🔒 제안 수정
- const data = (await response.json().catch(() => null)) as unknown;
+ let data: unknown;
+ try {
+ data = await response.json();
+ } catch {
+ return "error";
+ }
const responseRole = getRoleFromMyResponse(data);
- if (hasAdminRole(responseRole)) {
- return "authorized";
- }
-
- // Fallback: /my 인증(200)은 통과했는데 응답 role 스키마가 다른 경우를 대비.
- if (hasAdminRole(decodeTokenRole(accessToken))) {
+ if (responseRole !== "") {
+ return hasAdminRole(responseRole) ? "authorized" : "forbidden";
+ }
+
+ // Fallback: role 필드를 전혀 읽지 못한 경우에만 토큰을 참고합니다.
+ if (hasAdminRole(decodeTokenRole(accessToken))) {
return "authorized";
}📝 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.
| const data = (await response.json().catch(() => null)) as unknown; | |
| const responseRole = getRoleFromMyResponse(data); | |
| if (hasAdminRole(responseRole)) { | |
| return "authorized"; | |
| } | |
| // Fallback: /my 인증(200)은 통과했는데 응답 role 스키마가 다른 경우를 대비. | |
| if (hasAdminRole(decodeTokenRole(accessToken))) { | |
| return "authorized"; | |
| } | |
| const data = (await response.json()) as { role?: string }; | |
| return data.role === UserRole.ADMIN; | |
| return "forbidden"; | |
| let data: unknown; | |
| try { | |
| data = await response.json(); | |
| } catch { | |
| return "error"; | |
| } | |
| const responseRole = getRoleFromMyResponse(data); | |
| if (responseRole !== "") { | |
| return hasAdminRole(responseRole) ? "authorized" : "forbidden"; | |
| } | |
| // Fallback: role 필드를 전혀 읽지 못한 경우에만 토큰을 참고합니다. | |
| if (hasAdminRole(decodeTokenRole(accessToken))) { | |
| return "authorized"; | |
| } | |
| return "forbidden"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/api/ai-inspector-requests/route.ts` around lines 120 - 131,
The current fallback trusts JWT role even when the /my response explicitly
returns a non-admin role; change the logic so that you only consult
decodeTokenRole(accessToken) when getRoleFromMyResponse(data) yields no usable
role (null/undefined/invalid), and do not treat a parsed non-admin response as
eligible for fallback; also stop swallowing JSON parse errors from
response.json()—let parsing failures surface as errors (or return an error
result) instead of silently falling back to the token. Ensure you update the
block around response.json(), getRoleFromMyResponse, hasAdminRole and
decodeTokenRole accordingly so only missing responseRole triggers the token
check.
| // HTTP-only 쿠키의 refreshToken 확인 | ||
| const refreshToken = request.cookies.get("refreshToken")?.value; | ||
| const hasValidRefreshToken = Boolean(refreshToken && !isTokenExpired(refreshToken)); | ||
|
|
||
| // 정확한 경로 매칭 | ||
| const needLogin = loginNeedPages.some((path) => { | ||
| return url.pathname === path || url.pathname.startsWith(`${path}/`); | ||
| return pathname === path || pathname.startsWith(`${path}/`); | ||
| }); | ||
|
|
||
| if (needLogin && !hasValidRefreshToken) { | ||
| if (needLogin && !refreshToken) { |
There was a problem hiding this comment.
2. 로그인 게이트가 만료된 세션까지 통과시킵니다.
지금 분기는 refreshToken의 존재만 보므로, apps/web/src/utils/isServerStateLogin.ts:1-11이 이미 로그아웃으로 보는 만료 토큰도 protected page에 먼저 진입합니다. 같은 판정을 재사용하지 않으면 최초 진입과 이후 리다이렉트 판단이 서로 달라집니다.
🔧 제안 수정
+import { isTokenExpired } from "@/utils/jwtUtils";
+
// HTTP-only 쿠키의 refreshToken 확인
const refreshToken = request.cookies.get("refreshToken")?.value;
@@
- if (needLogin && !refreshToken) {
+ if (needLogin && (!refreshToken || isTokenExpired(refreshToken))) {
url.pathname = "/login";
url.searchParams.delete("reason");
return NextResponse.redirect(url);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/middleware.ts` around lines 58 - 66, The current gate only
checks presence of refreshToken (variable refreshToken) causing expired tokens
to pass; replace the simple existence check with the unified server-side login
check by calling the existing isServerStateLogin() helper from
apps/web/src/utils/isServerStateLogin.ts (or replicate its exact validation
logic) when computing needLogin and deciding redirection, i.e., use
isServerStateLogin(request.cookies / refreshToken) to determine authenticated
state and rely on that result in the if (needLogin && !...) branch so initial
entry and subsequent redirect use the same expiration logic.
요약
/api/ai-inspector-requests관리자 검증을authorized/unauthorized/forbidden/error상태로 분리했습니다./my응답의role,data.role형태를 모두 파싱하고, 응답 스키마가 달라도 토큰 role로 보조 검증하도록 보완했습니다.401: 세션 만료,503: 권한 검증 실패)*.php,wp/phpmyadmin/.env/.git/vendor)를 조기 404 처리하도록 추가했습니다./robots.txt는 미들웨어에서Disallow: /+X-Robots-Tag: noindex로 응답하도록 추가했습니다.AGENTS.md상단에 "PR 제목/본문은 한국어 작성" 필수 규칙을 명시했습니다.검증
pnpm --filter @solid-connect/web typecheck