Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughFirebase 클라이언트/서비스 워커 구성 개편: 공개 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant App as Web App (Client)
participant SW as Service Worker (/firebase-messaging-sw.js)
participant CDN as Firebase CDN (compat)
participant FCM as FCM
User->>App: 페이지 로드
App->>App: navigator.serviceWorker.register("/firebase-messaging-sw.js")
App->>SW: Register SW (scope: /)
activate SW
Note over SW: install -> self.skipWaiting()
SW->>CDN: importScripts(firebase-app-compat.js, firebase-messaging-compat.js)
SW->>SW: firebase.initializeApp(firebaseConfig)<br/>messaging = firebase.messaging()
FCM-->>SW: push (background message)
SW->>SW: payload 파싱<br/>title: "PINBACK"<br/>body: data.body || 기본문구<br/>icon: data.icon || "/link_Thumbnail.png"
SW->>User: showNotification()
alt Registration/Error
App->>App: 성공/에러 로깅 (기존 처리 유지)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (4 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
|
✅ Storybook chromatic 배포 확인: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
64-65: 컴포넌트 외부에서 Firebase를 초기화하세요.Firebase 앱과 메시징 인스턴스가 컴포넌트 본문에서 초기화되고 있어 매 렌더링마다 재생성됩니다. 이는 성능 문제와 예기치 않은 동작을 유발할 수 있습니다.
Firebase 초기화를 컴포넌트 외부 또는 별도 모듈로 이동하세요:
// firebase.ts (새 파일) import { initializeApp } from 'firebase/app'; import { getMessaging } from 'firebase/messaging'; import { firebaseConfig } from '../../../firebase-config'; export const app = initializeApp(firebaseConfig); export const messaging = getMessaging(app);그런 다음 MainCard.tsx에서 가져와 사용:
- const app = initializeApp(firebaseConfig); - const messaging = getMessaging(app); + import { messaging } from '../../../firebase';
🧹 Nitpick comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
93-107: FCM 토큰 요청을 컴포넌트 마운트 시 자동 실행하는 것을 재검토하세요.컴포넌트가 마운트되자마자 알림 권한을 요청하고 있습니다. 이는 사용자 경험 측면에서 권장되지 않을 수 있습니다. 사용자가 알림 설정 단계(step 3 또는 4)에 도달했을 때 권한을 요청하는 것이 더 나은 UX일 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/extension/public/icon.pngis excluded by!**/*.png
📒 Files selected for processing (6)
apps/client/public/firebase-config.js(1 hunks)apps/client/public/firebase-messaging-sw.js(2 hunks)apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx(3 hunks)apps/client/src/pages/onBoarding/utils/registerServiceWorker.ts(1 hunks)apps/client/vercel.json(1 hunks)apps/extension/manifest.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/client/public/firebase-config.js (1)
apps/client/public/firebase-messaging-sw.js (1)
firebaseConfig(4-12)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (2)
apps/client/src/constants/alarms.ts (1)
AlarmsType(11-15)apps/client/src/pages/onBoarding/utils/formatRemindTime.ts (1)
normalizeTime(1-26)
apps/client/public/firebase-messaging-sw.js (1)
apps/client/public/firebase-config.js (1)
firebaseConfig(1-9)
🪛 Gitleaks (8.28.0)
apps/client/public/firebase-config.js
[high] 5-5: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (5)
apps/client/vercel.json (1)
7-8: 리라이트 규칙 순서가 올바릅니다.특정 경로(
/firebase_sdk/*)를 catch-all 규칙(/*) 앞에 배치하여 올바른 우선순위를 유지하고 있습니다.apps/extension/manifest.json (2)
11-18: 권한 배열 포맷팅이 개선되었습니다.다중 라인 형식으로 변경하여 가독성이 향상되었습니다.
6-10: 아이콘 해상도 수동 확인 필요
apps/extension/public/icon.png파일이 최소 512×512 해상도인지 직접 확인하세요. 부족한 경우 128×128, 256×256, 512×512 크기에 맞는 아이콘을 별도 추가해야 합니다.apps/client/src/pages/onBoarding/utils/registerServiceWorker.ts (1)
1-11: 서비스 워커 경로를 루트 상대 경로로 변경한 것이 올바릅니다.경로를
'firebase-messaging-sw.js'에서'/firebase-messaging-sw.js'로 변경하여 서비스 워커가 루트 스코프에 등록되도록 보장합니다. 이는 서비스 워커가 전체 애플리케이션에서 작동할 수 있도록 하며,vercel.json의 리라이트 규칙과도 일치합니다.apps/client/public/firebase-config.js (1)
1-9: Firebase 보안 규칙 및 설정 파일 존재 여부 수동 확인 필요
리포에firestore.rules,storage.rules,database.rules.json,firebase.json,.firebaserc파일이 보이지 않습니다. 해당 파일들이 적절한 경로에 커밋되었는지 직접 확인해주세요.
| const firebaseConfig = { | ||
| apiKey: 'AIzaSyD3KM0IQ4Ro3Dd2fyAY8fnhE1bQ_NesrBc', | ||
| authDomain: 'pinback-c55de.firebaseapp.com', | ||
| projectId: 'pinback-c55de', | ||
| storageBucket: 'pinback-c55de.firebasestorage.app', | ||
| messagingSenderId: '370851215931', | ||
| appId: '1:370851215931:web:08382b5e57808d29dcba1e', | ||
| measurementId: 'G-847ZNSCC3J', | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
firebaseConfig 중복 제거 필요
firebase-config.js에 이미 정의된 설정을 서비스 워커에서 다시 정의하고 있어 DRY 원칙을 위반하고 있습니다. 향후 설정 변경 시 두 곳을 모두 수정해야 하는 유지보수 부담이 발생합니다.
다음 diff를 적용하여 중복을 제거하세요:
-const firebaseConfig = {
- apiKey: 'AIzaSyD3KM0IQ4Ro3Dd2fyAY8fnhE1bQ_NesrBc',
- authDomain: 'pinback-c55de.firebaseapp.com',
- projectId: 'pinback-c55de',
- storageBucket: 'pinback-c55de.firebasestorage.app',
- messagingSenderId: '370851215931',
- appId: '1:370851215931:web:08382b5e57808d29dcba1e',
- measurementId: 'G-847ZNSCC3J',
-};
-
+importScripts('/firebase-config.js');
+
importScripts(📝 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 firebaseConfig = { | |
| apiKey: 'AIzaSyD3KM0IQ4Ro3Dd2fyAY8fnhE1bQ_NesrBc', | |
| authDomain: 'pinback-c55de.firebaseapp.com', | |
| projectId: 'pinback-c55de', | |
| storageBucket: 'pinback-c55de.firebasestorage.app', | |
| messagingSenderId: '370851215931', | |
| appId: '1:370851215931:web:08382b5e57808d29dcba1e', | |
| measurementId: 'G-847ZNSCC3J', | |
| }; | |
| // Remove the inlined firebaseConfig and instead load it from firebase-config.js | |
| importScripts('/firebase-config.js'); | |
| importScripts( | |
| 'https://www.gstatic.com/firebasejs/9.22.2/firebase-app-compat.js', | |
| 'https://www.gstatic.com/firebasejs/9.22.2/firebase-messaging-compat.js' | |
| ); |
🤖 Prompt for AI Agents
In apps/client/public/firebase-messaging-sw.js around lines 4 to 12, remove the
hardcoded firebaseConfig object and instead load the shared configuration from
the existing firebase-config.js; do this by importing the shared file (use
importScripts('/firebase-config.js') if this SW is non-module or an ES module
import if your SW is module-type) and then use the exported/global
firebaseConfig value; ensure firebase-config.js is available in the public/build
output at the correct path so the service worker can load it at runtime.
| importScripts( | ||
| 'https://www.gstatic.com/firebasejs/9.22.2/firebase-app-compat.js' | ||
| ); | ||
| importScripts( | ||
| './firebase_sdk/firebase-app-compat.js', | ||
| './firebase_sdk/firebase-messaging-compat.js', | ||
| './firebase-config.js' | ||
| 'https://www.gstatic.com/firebasejs/9.22.2/firebase-messaging-compat.js' | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Firebase SDK 버전 검증 필요
Firebase SDK 버전 9.22.2를 사용하고 있습니다. 최신 버전 및 보안 권고사항을 확인하여 업데이트가 필요한지 검토해주세요.
🌐 Web query:
Firebase JavaScript SDK 9.22.2 security vulnerabilities and latest version
💡 Result:
Short answer
- Yes — Firebase JavaScript SDK v9.22.2 is affected by a known vulnerability (CVE-2024-11023) that affects all Firebase JS SDK versions older than 10.9.0; upgrade is recommended. [1][2]
- Latest Firebase JavaScript SDK (stable) as of Sep 30, 2025: v12.3.0 (release notes). [3]
Details & action
- Vulnerability: CVE-2024-11023 — manipulation of the FIREBASE_DEFAULTS cookie (the _authTokenSyncURL field) can allow an attacker to capture session data. Vendor guidance: upgrade to >= 10.9.0. [1][2][4]
- If you are on 9.22.2, update to the current stable release (at minimum >=10.9.0; preferably the latest v12.3.0) and retest. See Firebase release notes for the exact release change log and install instructions. [3][1]
Sources
[1] CVE-2024-11023 advisory / public vulnerability summaries.
[2] Aggregated vulnerability databases summarizing CVE-2024-11023 (references include Firebase PR #8056).
[3] Firebase JavaScript SDK release notes (shows v12.3.0 as latest).
[4] Firebase release notes entry for v10.9.0 (patch for the issue).
Would you like the exact npm install/upgrade command and a short checklist to verify the fix in your app?
Firebase SDK를 최소 v10.9.0(권장 v12.3.0) 이상으로 긴급 업그레이드하세요
현재 apps/client/public/firebase-messaging-sw.js의 importScripts에서 사용 중인 v9.22.2(compat)는 CVE-2024-11023 취약점에 노출되어 있습니다. 다음과 같이 변경하세요:
- importScripts('https://www.gstatic.com/firebasejs/9.22.2/firebase-app-compat.js');
- importScripts('https://www.gstatic.com/firebasejs/9.22.2/firebase-messaging-compat.js');
+ importScripts('https://www.gstatic.com/firebasejs/12.3.0/firebase-app-compat.js');
+ importScripts('https://www.gstatic.com/firebasejs/12.3.0/firebase-messaging-compat.js');(선택) npm 사용 시:
npm install firebase@^12.3.0검증 체크리스트:
- importScripts URL이 v12.3.0 이상으로 변경되었는지 확인
- 서비스 워커 캐시 삭제 후 Firebase 초기화 및 푸시 알림 기능 정상 동작 검증
- 브라우저 호환성 및 기존 기능 회귀 테스트
| const notificationTitle = payload.notification?.title ?? 'PINBACK'; | ||
| const notificationOptions = { | ||
| body: payload.notification?.body, | ||
| body: payload.data?.body ?? '알림이 도착했어요!', | ||
| icon: payload.data?.icon ?? '/link_Thumbnail.png', |
There was a problem hiding this comment.
알림 페이로드 처리 불일치
제목은 payload.notification?.title에서 읽지만, 본문과 아이콘은 payload.data?.body와 payload.data?.icon에서 읽고 있어 일관성이 없습니다.
FCM 메시지 유형에 따라:
- Notification 메시지: 제목만 표시되고 본문/아이콘은 기본값으로 대체됩니다
- Data 메시지: 본문/아이콘만 표시되고 제목은 'PINBACK'으로 대체됩니다
모든 필드를 동일한 소스에서 읽도록 통일하세요:
옵션 1: data 페이로드 우선 (data 메시지 지원 시)
- const notificationTitle = payload.notification?.title ?? 'PINBACK';
+ const notificationTitle = payload.data?.title ?? payload.notification?.title ?? 'PINBACK';
const notificationOptions = {
body: payload.data?.body ?? '알림이 도착했어요!',
icon: payload.data?.icon ?? '/link_Thumbnail.png',
};옵션 2: notification 페이로드 우선 (notification 메시지 지원 시)
- const notificationTitle = payload.notification?.title ?? 'PINBACK';
+ const notificationTitle = payload.notification?.title ?? payload.data?.title ?? 'PINBACK';
const notificationOptions = {
- body: payload.data?.body ?? '알림이 도착했어요!',
- icon: payload.data?.icon ?? '/link_Thumbnail.png',
+ body: payload.notification?.body ?? payload.data?.body ?? '알림이 도착했어요!',
+ icon: payload.notification?.icon ?? payload.data?.icon ?? '/link_Thumbnail.png',
};📝 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 notificationTitle = payload.notification?.title ?? 'PINBACK'; | |
| const notificationOptions = { | |
| body: payload.notification?.body, | |
| body: payload.data?.body ?? '알림이 도착했어요!', | |
| icon: payload.data?.icon ?? '/link_Thumbnail.png', | |
| const notificationTitle = payload.notification?.title ?? payload.data?.title ?? 'PINBACK'; | |
| const notificationOptions = { | |
| body: payload.notification?.body ?? payload.data?.body ?? '알림이 도착했어요!', | |
| icon: payload.notification?.icon ?? payload.data?.icon ?? '/link_Thumbnail.png', | |
| }; |
🤖 Prompt for AI Agents
In apps/client/public/firebase-messaging-sw.js around lines 36 to 39, the code
reads title from payload.notification but body/icon from payload.data, causing
inconsistent behavior for Notification vs Data messages; unify the source by
choosing a single precedence strategy (e.g., data-first): read title, body, and
icon from payload.data when present, otherwise fall back to
payload.notification, and update notificationTitle and notificationOptions to
use that unified lookup so all fields come from the same payload source.
| if (alarmSelected == 1) { | ||
| setRemindTime('09:00'); | ||
| } else if (alarmSelected==2){ | ||
| } else if (alarmSelected == 2) { | ||
| setRemindTime('20:00'); | ||
| } else{ | ||
| } else { | ||
| const raw = AlarmsType[alarmSelected - 1].time; | ||
| setRemindTime(normalizeTime(raw)) | ||
| setRemindTime(normalizeTime(raw)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
느슨한 동등 연산자 대신 엄격한 동등 연산자를 사용하세요.
== 대신 ===를 사용하는 것이 TypeScript/JavaScript 모범 사례입니다. alarmSelected는 1 | 2 | 3 타입이므로 타입 강제 변환이 발생하지 않지만, 일관성과 명확성을 위해 엄격한 비교를 사용해야 합니다.
다음 diff를 적용하여 엄격한 동등 연산자로 변경하세요:
- if (alarmSelected == 1) {
+ if (alarmSelected === 1) {
setRemindTime('09:00');
- } else if (alarmSelected == 2) {
+ } else if (alarmSelected === 2) {
setRemindTime('20:00');
} else {📝 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.
| if (alarmSelected == 1) { | |
| setRemindTime('09:00'); | |
| } else if (alarmSelected==2){ | |
| } else if (alarmSelected == 2) { | |
| setRemindTime('20:00'); | |
| } else{ | |
| } else { | |
| const raw = AlarmsType[alarmSelected - 1].time; | |
| setRemindTime(normalizeTime(raw)) | |
| setRemindTime(normalizeTime(raw)); | |
| } | |
| if (alarmSelected === 1) { | |
| setRemindTime('09:00'); | |
| } else if (alarmSelected === 2) { | |
| setRemindTime('20:00'); | |
| } else { | |
| const raw = AlarmsType[alarmSelected - 1].time; | |
| setRemindTime(normalizeTime(raw)); | |
| } |
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx around lines
132 to 139, the code uses loose equality (==) to compare alarmSelected to
numeric literals; update both comparisons to strict equality (===) so the checks
read alarmSelected === 1 and alarmSelected === 2 to follow TypeScript/JavaScript
best practices and ensure consistent, type-safe comparisons.
| if ((isMac && step < 5) || (!isMac && step < 4)) { | ||
| setDirection(1); | ||
| setStep((prev) => prev + 1); | ||
| } else if ( (isMac && step === 5) || (!isMac && step==4)) { | ||
| } else if ((isMac && step === 5) || (!isMac && step == 4)) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
느슨한 동등 연산자 대신 엄격한 동등 연산자를 사용하세요.
Line 148에서 step == 4 대신 step === 4를 사용해야 합니다.
다음 diff를 적용하세요:
- } else if ((isMac && step === 5) || (!isMac && step == 4)) {
+ } else if ((isMac && step === 5) || (!isMac && step === 4)) {📝 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.
| if ((isMac && step < 5) || (!isMac && step < 4)) { | |
| setDirection(1); | |
| setStep((prev) => prev + 1); | |
| } else if ( (isMac && step === 5) || (!isMac && step==4)) { | |
| } else if ((isMac && step === 5) || (!isMac && step == 4)) { | |
| if ((isMac && step < 5) || (!isMac && step < 4)) { | |
| setDirection(1); | |
| setStep((prev) => prev + 1); | |
| } else if ((isMac && step === 5) || (!isMac && step === 4)) { |
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx around lines
145 to 148, the code uses a loose equality operator (`==`) in the condition
`step == 4`; change it to the strict equality operator (`===`) so the condition
reads `step === 4`. Ensure the comparison type matches (no implicit coercion
expected) and keep the surrounding logic unchanged.
| "source": "/(.*)", | ||
| "destination": "/index.html" | ||
| } | ||
| { "source": "/firebase_sdk/(.*)", "destination": "/firebase_sdk/$1" }, |
There was a problem hiding this comment.
🧩 Analysis chain
Firebase SDK 경로에 대한 실제 자산을 확인하세요.
/firebase_sdk/* 경로에 대한 새로운 리라이트 규칙이 추가되었습니다. 이 경로가 CDN 기반 Firebase SDK 로딩을 지원하기 위한 것으로 보이지만, 실제로 /firebase_sdk 디렉토리나 해당 자산이 존재하는지 확인이 필요합니다.
다음 스크립트를 실행하여 Firebase SDK 자산 디렉토리를 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: Firebase SDK 자산 디렉토리 및 파일 확인
# public 디렉토리 내 firebase_sdk 폴더 검색
fd -t d "firebase_sdk" apps/client/public
# Firebase SDK 관련 파일 검색
fd -e js "firebase.*sdk|firebase.*app|firebase.*messaging" apps/client/publicLength of output: 43
/firebase_sdk 디렉토리 존재 여부 확인 필요
apps/client/vercel.json에 추가된
{ "source": "/firebase_sdk/(.*)", "destination": "/firebase_sdk/$1" },
rewrite 규칙이 가리키는 apps/client/public/firebase_sdk 디렉토리가 존재하지 않습니다. (fd 검색 결과: apps/client/public/firebase-messaging-sw.js만 확인됨)
해당 디렉토리 및 필요한 SDK 자산을 추가하거나, 불필요한 경우 rewrite 규칙을 제거하세요.
🤖 Prompt for AI Agents
In apps/client/vercel.json around line 7, the rewrite rule routing
"/firebase_sdk/(.*)" to "/firebase_sdk/$1" points to a non-existent directory
(apps/client/public/firebase_sdk); either create that directory and add the
required Firebase SDK assets (copy the SDK files into
apps/client/public/firebase_sdk and ensure they are committed), or remove the
rewrite entry from vercel.json if those assets are not needed; after changing,
run a local file-listing or build to verify the path exists and that the rewrite
no longer references a missing target.
1 participant
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
📷 Screenshot
Summary by CodeRabbit