refactor: 전역 API SOLID 원칙 준수 리팩토링 및 서비스 격리 (#62) - #64
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 56 minutes and 13 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
WalkthroughNext.js API 라우트 40+개를 공통 ChangesAPI 인프라 및 서비스 계층 리팩터링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50분 근거:
Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/supabase/server.ts (1)
84-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick win쿠키가 아직 없어도 mock auth는 유효한
user.id를 반환해야 합니다.Why:
signInWithPassword와updateUser가mockUid를 그대로 사용해서undefined를 내보낼 수 있습니다. 그런데E2E_TEST === "true"분기는 쿠키가 없는 최초 로그인에서도 항상 진입합니다. 이 상태의 응답은 실제 Supabase 계약과 달라서, 호출부가user.id를 키로 쓰는 순간 바로 깨집니다.How:
최소 수정 예시
+ const effectiveUid = mockUid || `e2e_uid_${Date.now()}`; + signInWithPassword: async ({ email }: { email: string }) => { return { data: { user: { - id: mockUid, + id: effectiveUid, email, user_metadata: { nickname: mockNickname }, }, session: { access_token: "mock_jwt_token" }, }, error: null, }; }, @@ updateUser: async ({ data }: { data: any }) => { return { data: { user: { - id: mockUid, + id: effectiveUid, email: mockEmail, user_metadata: { ...data, nickname: mockNickname }, }, }, error: null, }; },Also applies to: 119-130
🤖 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/lib/supabase/server.ts` around lines 84 - 95, The mock auth returns undefined user IDs when no cookie exists; update the signInWithPassword (and similarly updateUser) mock implementations to always return a valid id by falling back to mockUid when req.cookies or any cookie-derived id is missing — i.e., in signInWithPassword and updateUser ensure user.id is set to mockUid (or a deterministic fallback) rather than undefined so the E2E_TEST === "true" branch's response matches Supabase's contract and callers can safely use user.id as a key.
🤖 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 @.env.example:
- Around line 19-20: Add the missing E2E environment variable keys to
.env.example so local/CI mock flows are reproducible: add placeholder entries
for NEXT_PUBLIC_SUPABASE_E2E and E2E_TEST (and any other E2E-related keys
referenced by src/lib/supabase/env.ts and src/lib/supabase/server.ts) with clear
example values and brief comments indicating purpose; ensure names match exactly
the symbols used in the code and keep values as safe placeholders to avoid
leaking real credentials.
In `@prisma/schema.prisma`:
- Around line 3-10: The datasource block ("datasource db") currently hardcodes
provider = "sqlite", which breaks production Postgres usage; change provider
back to "postgresql", restore the url/directUrl usage (use env("DATABASE_URL")
and env("DIRECT_URL") as before) and re-enable the schemas setting (e.g.
["public","auth"]) so Prisma targets the same Postgres DB used by migrations; if
sqlite is needed for local tests, move that configuration into a separate
test-only Prisma schema/config rather than altering the main datasource.
In `@src/app/api/calendar/events/query/route.ts`:
- Around line 52-65: The Google auth-expiry handling in
src/app/api/calendar/events/query/route.ts is swallowing 401->expired errors as
502; update the catch path to detect Google token-expiry and return the
standardized 401 expiry response by reusing a shared helper (e.g.,
isGoogleAuthExpiredError(err) and mapGoogleAuthErrorToResponse(err) or
buildGoogleExpiryResponse()) used by the list route; modify the catch around
googleAdapter.queryEvents to call that helper and return the mapped NextResponse
(401 when expired) otherwise log and return the 502 error as before so both
routes share identical expiry mapping and reconnection flow.
- Around line 78-84: The route is using the client-supplied iCloud calendar URL
(targetIdentifier) directly which can leak server-stored Apple credentials to
attacker-controlled hosts; add a server-side whitelist check by implementing
assertAllowedIcloudCalendar(connection, targetIdentifier) and calling it before
any icloudAdapter query/create calls (e.g., before icloudAdapter.queryEvents in
the shown route). Implement assertAllowedIcloudCalendar to authenticate with the
stored credentials (connection.appleId, connection.password), retrieve the list
of calendars via the iCloud adapter discovery/listing API, and verify that
targetIdentifier exactly matches one of the discovered calendar URLs; if not,
throw/return a 4xx error. Apply the same check in the other affected handlers
that accept a client calendar URL (the create and query iCloud routes) so only
calendars discovered for the current account are allowed.
In `@src/app/api/everytime/timetable/route.ts`:
- Around line 9-12: The handler currently calls parseCandidateDays inside POST
and treats its undefined result the same as a missing query, letting downstream
defaults (Mon-Fri) silently mask invalid input; change parseCandidateDays (or
its call in POST) to differentiate "no days param provided" from "days param
provided but all tokens invalid" and return a 400 Bad Request for the latter.
Specifically, ensure parseCandidateDays exposes whether the param was present
(e.g., returns a discriminated result or throws on invalid tokens) and update
the POST route handler (and the other handler using parseCandidateDays) to
respond 400 when the input was present but invalid, while still allowing
undefined when the param is genuinely missing so downstream defaults remain
applied.
In `@src/app/api/google/callback/route.ts`:
- Around line 33-40: When starting the OAuth flow, persist the exact origin (or
full redirect URI) you pass into buildAuthUrl by setting a secure, HttpOnly
cookie (e.g., "google_oauth_origin") in the auth initiation code, and in the
callback handler read that cookie and pass its value into
exchangeCodeForTokens(code, savedOrigin) instead of recomputing origin from
req.url; if the cookie is missing, fail the callback with a clear error so you
don't risk a redirect_uri mismatch. Ensure cookie name matches between both
handlers and is set with Secure, HttpOnly, sameSite and appropriate expiry.
In `@src/app/api/icloud/events/query/route.ts`:
- Around line 19-24: The current presence-only check for body.calendarUrl,
startDate, and endDate lets invalid strings like "not-a-date" proceed; in the
route handler where you read body and set calendarUrl, startDate, endDate, parse
startDate and endDate with new Date(...) and validate with getTime() (or
Number.isNaN(date.getTime())) to detect Invalid Date, and return a 400 error
when either date is invalid or start >= end; ensure the validation runs before
calling any CalDAV functions so only valid, correctly ordered dates are
accepted.
In `@src/features/schedules/schedule.schema.ts`:
- Around line 3-7: timeSlotSchema is too permissive: it allows non-integers and
ranges where startHour === endHour or startHour > endHour, which later fails
repository validation; update timeSlotSchema to require integers for startHour
and endHour (use .int()) and add a cross-field refinement on the object (refine)
to enforce startHour < endHour, keeping the 0..24 bounds (allow endHour==24 if
desired) so route-level validation matches repository expectations.
In `@src/lib/__tests__/errors.test.ts`:
- Around line 1-12: Expand the tests in src/lib/__tests__/errors.test.ts to
explicitly verify the public contract exported from "../errors": assert that
MoimError, UnauthorizedError, ForbiddenError, ExternalServiceError and the
Everytime* classes (EverytimeError, EverytimeAuthError, EverytimeFetchError,
EverytimeScrapeError) are exported (e.g., typeof checks or instanceof use) and
that each error exposes expected shape fields (statusCode/code and for
Everytime* errors that type === "everytime" and details.type maps correctly to
"auth"/"fetch"/"scrape" as appropriate); add boundary cases for missing/empty
message and invalid details to ensure constructors on these classes normalize
fields consistently.
In `@src/lib/api-handler.ts`:
- Around line 47-48: The wrapper currently calls getSession() unconditionally
(see getSession reference in src/lib/api-handler.ts), which ties all routes to
Supabase availability; change the wrapper to not call getSession by default and
add an opt-in flag (e.g., loadSession: boolean) on the handler options so only
routes that request session loading (or call requireAuth) will invoke
getSession(); update requireAuth to call getSession() when enforcing auth and
mark public routes (like signup) with loadSession: false to avoid
session-dependent failures.
In `@src/lib/auth/naver.ts`:
- Around line 40-43: The redirect URI logic currently lets
process.env.NAVER_REDIRECT_URI override a per-request origin causing
multi-origin OAuth to break; update the implementation so that when an origin
argument is provided it takes highest precedence, otherwise fall back to
process.env.NAVER_REDIRECT_URI and lastly to a localhost default; centralize
this into a single helper (e.g., computeNaverRedirectUri or similar) and replace
duplicated logic at the sites referenced by base/redirectUri and the other
occurrence (around lines 73-76) so both authorize and token exchanges use the
same computed redirect URI.
In `@src/lib/calendar/adapter.ts`:
- Around line 58-71: Replace the use of unknown by parameterizing the adapter
with generics so the adapter boundary enforces provider-specific auth and result
shapes: make the abstract adapter generic (e.g., CalendarAdapter<AuthT,
CreateResultT>), change createEvent(auth: unknown, ...) to createEvent(auth:
AuthT, ...) returning Promise<CreateResultT>, and change listCalendars and
queryEvents signatures to use AuthT (listCalendars(auth: AuthT):
Promise<CommonCalendar[]> and queryEvents(auth: AuthT, targetIdentifier: string,
start: Date, end: Date): Promise<CalendarEvent[]>); update any implementing
classes to supply concrete types for AuthT and CreateResultT to restore
compile-time safety.
In `@src/lib/calendar/adapters/__tests__/manual.test.ts`:
- Around line 47-58: Rename the test and make assertions behavior-focused
instead of tied to implementation details: change the describe/it text to
describe the expected behavior (e.g., "converts manual slots to calendar events
with correct title and timing"), remove the strict id format assertion against
manualAdapter.toCalendarEvents (don't assert exact "manual:0:MON-9-11"), and
instead assert that an id exists/is non-empty if needed and verify public
outputs like title ("가용"), startAt.getHours() and endAt.getHours(), and that the
source/slot mapping is preserved; note that id uniqueness is already covered
elsewhere so omit duplicative id format checks in this test.
In `@src/lib/calendar/adapters/manual.ts`:
- Around line 58-79: The toCalendarEvents flow must normalize and validate
weekStart and slot hour bounds before mapping: ensure weekStart (in
toCalendarEvents) is normalized to Monday 00:00 (use or adapt getThisMonday
logic) and replace raw.weekStart with that normalized Date; validate each slot's
startHour and endHour in toCalendarEvents (or just before calling
mapToCalendarEvent) to be integers within 0..24 and require startHour < endHour
(fail-fast by throwing a clear Error); additionally clamp/normalize non-integer
hours if your domain allows or reject them explicitly; then pass the validated
slot and the normalized weekStart into mapToCalendarEvent so downstream code
never receives bad absolute times.
In `@src/lib/calendar/adapters/photo.ts`:
- Around line 13-87: The photo adapter currently exports a runtime instance
(photoAdapter) that throws on use; remove the exported instance to avoid
accidental runtime 500s and instead only export the PhotoCalendarAdapter class
(keep class name PhotoCalendarAdapter and the internal
constructor/implementation unchanged) and create/ export the photoAdapter
instance later at the provider/registry registration site where the feature
becomes enabled; ensure no other modules rely on the exported photoAdapter and
update registration code to instantiate and export the adapter when registering
the "photo" provider.
In `@src/lib/services/auth-service.ts`:
- Around line 123-160: The E2E path currently still calls createAdminClient()
for nickname lookups which can throw and break isolated tests; move the E2E
check (process.env.E2E_TEST === "true") to run immediately after normalizing
loginId so no Supabase admin calls occur during E2E, and inside that branch
resolve users via Prisma only (use prisma.user.findUnique/findFirst by email or
by nickname using the normalized loginId) rather than calling
createAdminClient(); keep Supabase admin lookup (createAdminClient(),
.from("profiles")...) only in the non-E2E branch.
- Around line 196-210: The code treats a missing profiles row as success; update
the block that calls supabase.from("profiles").select(...).maybeSingle() to
throw an error when profile is null/undefined (i.e. check !profile after the
query) instead of returning success: true with fallbacks; ensure the thrown
error bubbles to the common error handler so /api/auth/me returns a failure when
the profiles row for userId is absent (reference the variables profile,
profileError, userId and the maybeSingle() call).
In `@src/lib/services/everytime-service.ts`:
- Around line 7-10: EverytimeResult currently exposes timetable and freeSlots as
unknown; tighten the contract by introducing concrete types (e.g., define a
Timetable interface/object shape and a FreeSlot type or FreeSlot[] array) and
update EverytimeResult to use those types instead of unknown; then update the
return signatures and implementations of processUrl and processIcs to return
EverytimeResult with the new Timetable and FreeSlot types so callers, routes,
and tests get compile-time type safety (refer to EverytimeResult, timetable,
freeSlots, processUrl, processIcs).
- Around line 21-36: The Supabase auth calls currently assume exceptions but
return {data, error}; update the createClient usage in the function that calls
supabase.auth.getUser() and supabase.auth.updateUser() to check the returned
error fields and throw when present so the existing catch logs failures
(specifically inspect the results of supabase.auth.getUser() and
supabase.auth.updateUser() and throw the returned error if non-null), and apply
the same error-check-and-throw pattern to the processIcs function where Supabase
auth/update is used so metadata write failures surface to the existing catch
logger instead of being silently ignored.
In `@src/lib/services/schedule-service.ts`:
- Around line 98-119: The current flow always calls dbConfirmScheduleByCreator
when session exists, causing logged-in non-creators to take the creator path and
fail; fix by only using the creator-confirmation path when the session user is
actually the creator (e.g., check creator explicitly or call
dbConfirmScheduleByCreator and if it returns/throws a not-found/forbidden result
treat the user as non-creator and continue), otherwise fall through to the
hostToken branch; also ensure bodyHostToken is trimmed before deciding presence
(use trimmedBodyHostToken = typeof bodyHostToken === "string" ?
bodyHostToken.trim() : "" and treat empty string as absent) so whitespace-only
tokens are rejected before calling dbConfirmSchedule(id, hostToken,
confirmedSlot).
- Around line 121-130: In the catch block in
src/lib/services/schedule-service.ts (the code that computes message from error
and derives status), do not default unknown exceptions to 400; instead map only
known application errors ("schedule not found" -> 404, "invalid host token" ->
403) to those client statuses and treat all other/unmapped errors as server
errors (set status = 500 or rethrow) so infrastructure/observability and global
500 handling remain effective; update the logic around the variables
error/message/status in that catch handler (or the enclosing method) to return a
500 for unexpected errors or propagate the error to the common handler.
In `@src/lib/supabase/server.ts`:
- Around line 252-257: The lookup for the Naver mock user is incomplete: in
prisma.user.findFirst (the targetUser lookup) you only check { id: naverId } and
the synthetic email but not the upsert pattern id = `naver_${naverId}` (and you
later use .eq("naver_id", naverId)), which causes duplicate mock users; update
the OR conditions used in prisma.user.findFirst to also include { id:
`naver_${naverId}` } or alternatively check the naver_id column (e.g., include {
naver_id: naverId }) so the lookup matches users created via the upsert path;
apply the same fix to the other occurrence around the second find/update block
(the later prisma lookup at lines 304-310).
- Around line 13-17: The createClient function currently calls getSupabaseConfig
and createServerClient before checking the E2E_TEST mock branch, which causes
immediate exceptions when server env vars are absent; change createClient to
first read the E2E_TEST flag (and cookies() if needed for the mock) and, if
E2E_TEST === "true", return the Proxy/mock client immediately without invoking
getSupabaseConfig or createServerClient; otherwise proceed to call
getSupabaseConfig and createServerClient as before. Ensure references:
createClient, getSupabaseConfig, createServerClient, and the Proxy/mock return
path are updated so the mock branch short-circuits prior to any config/client
creation.
---
Outside diff comments:
In `@src/lib/supabase/server.ts`:
- Around line 84-95: The mock auth returns undefined user IDs when no cookie
exists; update the signInWithPassword (and similarly updateUser) mock
implementations to always return a valid id by falling back to mockUid when
req.cookies or any cookie-derived id is missing — i.e., in signInWithPassword
and updateUser ensure user.id is set to mockUid (or a deterministic fallback)
rather than undefined so the E2E_TEST === "true" branch's response matches
Supabase's contract and callers can safely use user.id as a key.
🪄 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.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0ea01404-52dc-4c0c-b62c-8319bc904438
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.json,!package-lock.jsonpackage.jsonis excluded by!**/*.json
📒 Files selected for processing (49)
.env.examplee2e/host-flow.spec.tsprisma/schema.prismascripts/ensure-sqlite-schema.mjssrc/app/api/auth/login/route.tssrc/app/api/auth/me/route.tssrc/app/api/auth/naver/callback/route.tssrc/app/api/auth/naver/login/route.tssrc/app/api/auth/signup/route.tssrc/app/api/calendar/events/create/route.tssrc/app/api/calendar/events/query/route.tssrc/app/api/calendar/list/route.tssrc/app/api/everytime/timetable/route.tssrc/app/api/google/auth/route.tssrc/app/api/google/calendars/route.tssrc/app/api/google/callback/route.tssrc/app/api/google/disconnect/route.tssrc/app/api/google/events/create/route.tssrc/app/api/google/events/query/route.tssrc/app/api/icloud/calendars/route.tssrc/app/api/icloud/events/create/route.tssrc/app/api/icloud/events/query/route.tssrc/app/api/schedules/[id]/route.tssrc/features/schedules/schedule.schema.tssrc/lib/__tests__/api-handler.test.tssrc/lib/__tests__/errors.test.tssrc/lib/api-handler.tssrc/lib/auth/naver.tssrc/lib/auth/session.tssrc/lib/caldav/client.tssrc/lib/calendar/adapter.tssrc/lib/calendar/adapters/__tests__/manual.test.tssrc/lib/calendar/adapters/google.tssrc/lib/calendar/adapters/icloud.tssrc/lib/calendar/adapters/manual.tssrc/lib/calendar/adapters/photo.tssrc/lib/errors.tssrc/lib/everytime/auth.tssrc/lib/everytime/timetable.tssrc/lib/everytime/url-scraper.tssrc/lib/google/__tests__/auth.test.tssrc/lib/google/auth.tssrc/lib/services/auth-service.tssrc/lib/services/everytime-service.tssrc/lib/services/schedule-service.tssrc/lib/supabase/client.tssrc/lib/supabase/env.tssrc/lib/supabase/server.tssupabase/migrations/20260611000000_profiles_naver_id.sql
Comment on lines
+52
to
+65
| try { | ||
| const events = await googleAdapter.queryEvents( | ||
| { accessToken: tokens.accessToken }, | ||
| targetIdentifier, | ||
| start, | ||
| end, | ||
| ); | ||
| return NextResponse.json({ events }); | ||
| } catch (err) { | ||
| console.error("[calendar.events.query] Google 일정 조회 오류:", err); | ||
| return NextResponse.json( | ||
| { error: "일정 조회 중 오류가 발생했습니다." }, | ||
| { status: 502 }, | ||
| ); |
There was a problem hiding this comment.
공통 provider 라우트에서 Google 만료 오류 매핑이 드리프트했습니다.
근거: src/app/api/calendar/events/query/route.ts와 src/app/api/calendar/list/route.ts는 같은 root cause를 공유합니다. 두 파일 모두 Google 전용 라우트가 이미 유지하는 401 만료 계약을 버리고 502로 뭉개고 있어, 재연결 플로우와 클라이언트 분기 기준이 함께 깨집니다.
수정: 두 라우트에 동일한 만료 매핑 헬퍼를 두고, Google 분기에서 공통으로 재사용하세요.
수정 예시
+function mapGoogleExpiredToken(err: unknown, message: string) {
+ if (err instanceof Error && err.message.includes("만료")) {
+ return NextResponse.json({ error: message }, { status: 401 });
+ }
+ return null;
+}
...
} catch (err) {
+ const expired = mapGoogleExpiredToken(
+ err,
+ "Google 인증이 만료되었습니다. 다시 연결해주세요.",
+ );
+ if (expired) return expired;
return NextResponse.json(
{ error: "캘린더 목록 조회 중 오류가 발생했습니다." },
{ status: 502 },
);
}🤖 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/app/api/calendar/events/query/route.ts` around lines 52 - 65, The Google
auth-expiry handling in src/app/api/calendar/events/query/route.ts is swallowing
401->expired errors as 502; update the catch path to detect Google token-expiry
and return the standardized 401 expiry response by reusing a shared helper
(e.g., isGoogleAuthExpiredError(err) and mapGoogleAuthErrorToResponse(err) or
buildGoogleExpiryResponse()) used by the list route; modify the catch around
googleAdapter.queryEvents to call that helper and return the mapped NextResponse
(401 when expired) otherwise log and return the 502 error as before so both
routes share identical expiry mapping and reconnection flow.
Comment on lines
+78
to
+84
| try { | ||
| const events = await icloudAdapter.queryEvents( | ||
| { username: connection.appleId, password: connection.password }, | ||
| targetIdentifier, | ||
| start, | ||
| end, | ||
| ); |
There was a problem hiding this comment.
클라이언트가 준 iCloud 캘린더 URL을 그대로 CalDAV 호출에 쓰고 있습니다.
근거: src/app/api/calendar/events/query/route.ts, src/app/api/icloud/events/create/route.ts, src/app/api/icloud/events/query/route.ts 모두 같은 root cause를 가집니다. 사용자 입력 URL을 그대로 쓰면 서버가 저장한 Apple 자격증명을 공격자 호스트로 보낼 수 있어, 단순 입력 검증 문제가 아니라 자격증명 유출 취약점입니다.
수정: 세 파일 모두 공통 assertAllowedIcloudCalendar 같은 서버 측 검증 단계를 추가해, 현재 계정으로 발견된 캘린더 URL만 허용하세요.
수정 예시
+async function assertAllowedIcloudCalendar(
+ connection: { appleId: string; password: string },
+ calendarUrl: string,
+) {
+ const calendars = await icloudAdapter.listCalendars({
+ username: connection.appleId,
+ password: connection.password,
+ });
+ return calendars.find((item) => item.id === calendarUrl) ?? null;
+}
...
+const allowedCalendar = await assertAllowedIcloudCalendar(connection, calendarUrl);
+if (!allowedCalendar) {
+ return NextResponse.json(
+ { error: "허용되지 않은 iCloud 캘린더입니다." },
+ { status: 400 },
+ );
+}🤖 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/app/api/calendar/events/query/route.ts` around lines 78 - 84, The route
is using the client-supplied iCloud calendar URL (targetIdentifier) directly
which can leak server-stored Apple credentials to attacker-controlled hosts; add
a server-side whitelist check by implementing
assertAllowedIcloudCalendar(connection, targetIdentifier) and calling it before
any icloudAdapter query/create calls (e.g., before icloudAdapter.queryEvents in
the shown route). Implement assertAllowedIcloudCalendar to authenticate with the
stored credentials (connection.appleId, connection.password), retrieve the list
of calendars via the iCloud adapter discovery/listing API, and verify that
targetIdentifier exactly matches one of the discovered calendar URLs; if not,
throw/return a 4xx error. Apply the same check in the other affected handlers
that accept a client calendar URL (the create and query iCloud routes) so only
calendars discovered for the current account are allowed.
Comment on lines
+9
to
12
| export const POST = createApiHandler({}, async ({ req }) => { | ||
| const contentType = req.headers.get("content-type") ?? ""; | ||
| const candidateDays = parseCandidateDays(req); | ||
|
|
There was a problem hiding this comment.
잘못된 days 쿼리를 기본값으로 덮어쓰면 결과가 틀어집니다.
근거(Why): 지금 구현은 유효하지 않은 토큰을 조용히 버린 뒤 전부 버려지면 undefined를 반환합니다. 그러면 downstream 기본값(월~금)이 적용되어, 잘못된 요청이 400이 아니라 정상 200과 잘못된 freeSlots로 보입니다.
수정(How):
`누락`과 `잘못된 값`을 분리하는 최소 수정
export const POST = createApiHandler({}, async ({ req }) => {
const contentType = req.headers.get("content-type") ?? "";
const candidateDays = parseCandidateDays(req);
+ if (candidateDays === null) {
+ return NextResponse.json(
+ { error: "days 쿼리는 MON~SUN 값만 허용됩니다." },
+ { status: 400 },
+ );
+ }
if (contentType.includes("application/json")) {
return handleUrlRequest(req, candidateDays);
}
@@
-function parseCandidateDays(req: NextRequest): DayCode[] | undefined {
+function parseCandidateDays(req: NextRequest): DayCode[] | undefined | null {
const raw = req.nextUrl.searchParams.get("days");
if (!raw) return undefined;
- const days = Array.from(
- new Set(
- raw
- .split(",")
- .map((d) => d.trim().toUpperCase() as DayCode)
- .filter((d) => VALID_DAYS.has(d)),
- ),
- );
+ const tokens = raw
+ .split(",")
+ .map((d) => d.trim().toUpperCase())
+ .filter(Boolean);
+ if (tokens.some((d) => !VALID_DAYS.has(d as DayCode))) {
+ return null;
+ }
+ const days = Array.from(new Set(tokens as DayCode[]));
return days.length > 0 ? days : undefined;
}Also applies to: 166-177
🤖 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/app/api/everytime/timetable/route.ts` around lines 9 - 12, The handler
currently calls parseCandidateDays inside POST and treats its undefined result
the same as a missing query, letting downstream defaults (Mon-Fri) silently mask
invalid input; change parseCandidateDays (or its call in POST) to differentiate
"no days param provided" from "days param provided but all tokens invalid" and
return a 400 Bad Request for the latter. Specifically, ensure parseCandidateDays
exposes whether the param was present (e.g., returns a discriminated result or
throws on invalid tokens) and update the POST route handler (and the other
handler using parseCandidateDays) to respond 400 when the input was present but
invalid, while still allowing undefined when the param is genuinely missing so
downstream defaults remain applied.
Comment on lines
+21
to
+36
| try { | ||
| const supabase = await createClient(); | ||
| const { | ||
| data: { user }, | ||
| } = await supabase.auth.getUser(); | ||
| if (user) { | ||
| await supabase.auth.updateUser({ | ||
| data: { | ||
| everytime_url: url, | ||
| everytime_slots: freeSlots, | ||
| }, | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| console.error("[everytime] 유저 메타데이터 저장 실패:", err); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@supabase/supabase-js 2.x 공식 문서에서 auth.getUser()와 auth.updateUser()가 실패 시 예외를 throw하는지, 아니면 { data, error } 형태로 반환하는지 확인해 주세요.
💡 Result:
@supabase/supabase-js 2.x 공식 문서와 SDK 설계에 따르면, auth.getUser와 auth.updateUser를 포함한 대부분의 Supabase SDK 메서드는 예외를 throw하는 대신 { data, error } 객체를 반환합니다 [1][2][3]. 이러한 설계는 호출 결과가 성공했는지 실패했는지를 개발자가 직접 확인하도록 유도하며, 에러 발생 시 throw 대신 error 속성에 AuthError 객체를 담아 반환합니다 [4][3][5]. 따라서 코드를 작성할 때는 예외 처리(try-catch)가 아닌, 반환된 객체의 error 속성을 체크하는 방식을 사용하는 것이 공식 권장 사항입니다 [3]. 사용 예시: const { data, error } = await supabase.auth.getUser; if (error) { // 에러 처리 로직 console.error(error.message); } else { // 성공 로직 console.log(data.user); }
Citations:
- 1: https://supabase.com/docs/reference/javascript/auth-getuser
- 2: https://supabase.com/docs/reference/javascript/auth-updateuser
- 3: https://supabase.com/docs/guides/api/handling-errors-in-supabase-js
- 4: https://github.com/supabase/auth-js/blob/dcc2b31f/src/lib/types.ts
- 5: https://github.com/supabase/supabase-js/blob/21e9d4b7/packages/core/auth-js/src/lib/errors.ts
Supabase auth.getUser()/auth.updateUser() 실패를 catch가 아닌 반환 error로 처리하세요(메타데이터 조용한 누락 방지)
Why: Supabase JS(v2) auth 메서드는 실패 시 예외를 던지기보다 { data, error }를 반환합니다. 현재처럼 error를 확인하지 않으면 저장 실패가 예외로 이어지지 않아 catch가 실행되지 않고, 메서드는 성공처럼 마무리되어 메타데이터가 누락됩니다.
How: 반환된 error를 즉시 처리(throw → 기존 catch 로깅)하고, processIcs에도 동일 패턴을 적용하세요.
processUrl: 반환 `error` 즉시 처리
const {
data: { user },
- } = await supabase.auth.getUser();
+ error: getUserError,
+ } = await supabase.auth.getUser();
+ if (getUserError) throw getUserError;
if (user) {
- await supabase.auth.updateUser({
+ const { error: updateUserError } = await supabase.auth.updateUser({
data: {
everytime_url: url,
everytime_slots: freeSlots,
},
});
+ if (updateUserError) throw updateUserError;
}🤖 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/lib/services/everytime-service.ts` around lines 21 - 36, The Supabase
auth calls currently assume exceptions but return {data, error}; update the
createClient usage in the function that calls supabase.auth.getUser() and
supabase.auth.updateUser() to check the returned error fields and throw when
present so the existing catch logs failures (specifically inspect the results of
supabase.auth.getUser() and supabase.auth.updateUser() and throw the returned
error if non-null), and apply the same error-check-and-throw pattern to the
processIcs function where Supabase auth/update is used so metadata write
failures surface to the existing catch logger instead of being silently ignored.
Source: Coding guidelines
Comment on lines
+98
to
+119
| try { | ||
| // 1. 로그인 유저가 생성자(호스트)인지 먼저 확인하여 확정 | ||
| if (session) { | ||
| const schedule = await dbConfirmScheduleByCreator( | ||
| id, | ||
| session.userId, | ||
| confirmedSlot, | ||
| ); | ||
| return { success: true, status: 200, schedule }; | ||
| } | ||
|
|
||
| // 2. hostToken 기반 확정 처리 | ||
| const hostToken = | ||
| typeof bodyHostToken === "string" && bodyHostToken.trim() | ||
| ? bodyHostToken | ||
| : cookieHostToken; | ||
|
|
||
| if (!hostToken) { | ||
| return { success: false, status: 400, error: "hostToken is required" }; | ||
| } | ||
|
|
||
| const schedule = await dbConfirmSchedule(id, hostToken, confirmedSlot); |
There was a problem hiding this comment.
로그인된 비생성자는 유효한 hostToken으로도 확정할 수 없습니다.
Why: getSchedule()은 세션이 있어도 creator가 아니면 hostToken 경로로 계속 진행하는데, 여기서는 Line 100에서 creator 전용 확정으로 바로 고정됩니다. 그래서 로그인된 사용자가 같은 호스트 링크로 조회는 성공해도 PATCH는 404/400으로 깨집니다. 같은 분기에서 bodyHostToken.trim() 결과도 버려서 공백 포함 토큰까지 그대로 내려갑니다.
How:
최소 수정 예시
- // 1. 로그인 유저가 생성자(호스트)인지 먼저 확인하여 확정
- if (session) {
- const schedule = await dbConfirmScheduleByCreator(
- id,
- session.userId,
- confirmedSlot,
- );
- return { success: true, status: 200, schedule };
- }
-
- // 2. hostToken 기반 확정 처리
- const hostToken =
- typeof bodyHostToken === "string" && bodyHostToken.trim()
- ? bodyHostToken
- : cookieHostToken;
+ const normalizedBodyHostToken =
+ typeof bodyHostToken === "string" ? bodyHostToken.trim() : "";
+ const hostToken = normalizedBodyHostToken || cookieHostToken;
+
+ // 1. 로그인 유저가 실제 생성자인 경우에만 creator 경로 사용
+ if (session) {
+ const creatorSchedule = await getScheduleForCreator(id, session.userId);
+ if (creatorSchedule) {
+ const schedule = await dbConfirmScheduleByCreator(
+ id,
+ session.userId,
+ confirmedSlot,
+ );
+ return { success: true, status: 200, schedule };
+ }
+ }
if (!hostToken) {
return { success: false, status: 400, error: "hostToken is required" };
}🤖 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/lib/services/schedule-service.ts` around lines 98 - 119, The current flow
always calls dbConfirmScheduleByCreator when session exists, causing logged-in
non-creators to take the creator path and fail; fix by only using the
creator-confirmation path when the session user is actually the creator (e.g.,
check creator explicitly or call dbConfirmScheduleByCreator and if it
returns/throws a not-found/forbidden result treat the user as non-creator and
continue), otherwise fall through to the hostToken branch; also ensure
bodyHostToken is trimmed before deciding presence (use trimmedBodyHostToken =
typeof bodyHostToken === "string" ? bodyHostToken.trim() : "" and treat empty
string as absent) so whitespace-only tokens are rejected before calling
dbConfirmSchedule(id, hostToken, confirmedSlot).
Comment on lines
+121
to
+130
| } catch (error) { | ||
| const message = | ||
| error instanceof Error ? error.message : "invalid request"; | ||
| const status = | ||
| message === "schedule not found" | ||
| ? 404 | ||
| : message === "invalid host token" | ||
| ? 403 | ||
| : 400; | ||
| return { success: false, status, error: message }; |
There was a problem hiding this comment.
알 수 없는 예외를 전부 400으로 누르면 서버 장애가 클라이언트 오류로 위장됩니다.
Why: 여기서 매핑하지 않은 예외까지 400으로 바꾸면 저장소 실패, DB 장애, 버그가 전부 잘못된 요청처럼 보입니다. 그러면 공통 핸들러의 500 처리와 관측이 모두 무력화됩니다.
How:
최소 수정 예시
- } catch (error) {
- const message =
- error instanceof Error ? error.message : "invalid request";
- const status =
- message === "schedule not found"
- ? 404
- : message === "invalid host token"
- ? 403
- : 400;
- return { success: false, status, error: message };
+ } catch (error) {
+ if (!(error instanceof Error)) {
+ throw error;
+ }
+
+ if (error.message === "schedule not found") {
+ return { success: false, status: 404, error: error.message };
+ }
+ if (error.message === "invalid host token") {
+ return { success: false, status: 403, error: error.message };
+ }
+ if (
+ error.message === "hour range must be an integer range between 0 and 24" ||
+ error.message === "confirmed slot must stay inside the candidate window" ||
+ error.message === "confirmed slot must be inside the current common slots"
+ ) {
+ return { success: false, status: 400, error: error.message };
+ }
+
+ throw error;
}🤖 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/lib/services/schedule-service.ts` around lines 121 - 130, In the catch
block in src/lib/services/schedule-service.ts (the code that computes message
from error and derives status), do not default unknown exceptions to 400;
instead map only known application errors ("schedule not found" -> 404, "invalid
host token" -> 403) to those client statuses and treat all other/unmapped errors
as server errors (set status = 500 or rethrow) so infrastructure/observability
and global 500 handling remain effective; update the logic around the variables
error/message/status in that catch handler (or the enclosing method) to return a
500 for unexpected errors or propagate the error to the common handler.
Siul49
force-pushed
the
feature/refactor-calendar-adapters-and-errors
branch
from
June 12, 2026 11:55
7af1d30 to
62e3723
Compare
PR #64에 대한 CodeRabbit 리뷰 중 현재 코드에 유효한 항목 반영: - iCloud query/create 라우트가 클라이언트 calendarUrl을 검증 없이 CalDAV에 넘겨 저장된 Apple 자격증명이 임의 호스트로 유출될 수 있던 문제 차단 (isIcloudCalendarUrl 화이트리스트 + 단위 테스트) - Google OAuth: 인증 시작 시 origin을 쿠키에 저장하고 콜백에서 재사용해 redirect_uri 드리프트(www↔apex 등)를 방지 (쿠키 부재 시 요청 origin 폴백) - manual 어댑터: weekStart를 월요일 0시로 정규화하고 잘못된 슬롯 검증 - errors: 공개 진입점 재-export 동일성 및 Everytime* details 매핑 테스트 보강 스킵: services/*·calendar/events·adapter 제네릭(현재 코드에 없음, stale), schedule 확정/예외 매핑·Google 만료 매핑(이미 반영됨), naver redirect 우선순위(env 우선이 기존 테스트로 고정된 의도된 동작) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dar-adapters-and-errors # Conflicts: # src/app/api/google/auth/route.ts # src/app/api/google/callback/route.ts # src/app/api/icloud/events/create/route.ts # src/app/api/icloud/events/query/route.ts # src/lib/__tests__/errors.test.ts # src/lib/calendar/adapters/manual.ts
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
2 participants
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.
🚀 작업 내용 (What)
createApiHandler제네릭 헬퍼로 통합하여 일관된 요청 파싱, 인증 체크, 에러 핸들링을 수행하도록 개선했습니다.BaseCalendarAdapter및ArrayCalendarAdapter기반의 클래스 상속 구조로 리팩토링하여 Google, iCloud, Manual, Photo 캘린더 등 다양한 어댑터에 공통 로직을 효율적으로 적용할 수 있게 했습니다.MoimError를 부모로 하는 중앙 집중화된 에러 계층(UnauthorizedError,ForbiddenError,ExternalServiceError,CalDAVError,EverytimeError등)을 설계하고 적용했습니다.📣 핵심 변경 이유 (Why)
📸 스크린샷 (Visuals, 선택)
🔗 관련 이슈 (Issue)
Close #62