Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughFirebase 메시징 서비스 워커에서 install/activate 이벤트를 화살표 함수로 정리하고, activate 시 clients.claim()을 추가했습니다. 백그라운드 메시지 수신 시 고정 제목/본문과 아이콘을 사용하며 payload.data.url을 추출해 notification.data로 전달합니다. 클릭 시 기존 탭 포커스 또는 해당 URL 새 오픈 흐름으로 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor FCM as FCM
participant SW as Service Worker
participant Clients as Browser Clients (Tabs/Windows)
rect rgba(227,242,253,0.6)
note over SW: Lifecycle
SW->>SW: install (arrow callback)
SW->>SW: activate (arrow callback)
SW->>Clients: clients.claim()
end
rect rgba(232,245,233,0.6)
note over FCM,SW: Background message handling
FCM-->>SW: push/FCM background payload
SW->>SW: Extract data.url (fallback to default)
SW->>SW: showNotification(title="pinback", body, icon, data:{url})
end
rect rgba(255,243,224,0.6)
note over SW,Clients: Notification click flow
SW-->>SW: notificationclick
SW->>SW: Read event.notification.data.url (with default)
alt Existing pinback client found
SW->>Clients: Focus matching client
else None found
SW->>Clients: Open new window with derived URL
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/client/public/firebase-messaging-sw.js (1)
56-69: 알림 클릭 시 URL 탐색 불일치기존
pinback.today탭을 찾으면 포커스만 하고 종료하지만(Line 62), 사용자가 클릭한 알림의 실제 목적지 URL(url변수)로 탐색하지 않습니다. 이로 인해 다음과 같은 UX 문제가 발생합니다:
- 사용자가 특정 페이지(예:
/bookmarks/123)로 이동하려는 알림을 클릭- 기존 pinback 탭이 다른 페이지(예:
/settings)에 있음- 탭이 포커스되지만 의도한 페이지로 이동하지 않음
다음 diff를 적용하여 포커스된 클라이언트를 올바른 URL로 탐색하세요:
event.waitUntil( clients .matchAll({ type: 'window', includeUncontrolled: true }) .then((clientList) => { for (const client of clientList) { if (client.url.includes('pinback.today') && 'focus' in client) { - return client.focus(); + return client.focus().then(() => { + if ('navigate' in client) { + return client.navigate(url); + } + }); } } if (clients.openWindow) { return clients.openWindow(url); } }) );
♻️ Duplicate comments (1)
apps/client/public/firebase-messaging-sw.js (1)
54-54: 알림 클릭 시 검증되지 않은 URL 사용이 URL은 Line 37-43에서 설정된
notification.data.url에서 가져옵니다. Line 37의 URL 검증 문제가 해결되지 않으면, 여기서도 동일한 XSS 위험이 있습니다.Line 37-46의 URL 검증 수정 사항을 적용한 후, 이 라인은 자동으로 안전한 URL만 받게 됩니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/client/public/firebase-messaging-sw.js(3 hunks)
🔇 Additional comments (2)
apps/client/public/firebase-messaging-sw.js (2)
21-23: LGTM! 이벤트 리스너 현대화화살표 함수로의 전환과
skipWaiting()호출이 적절합니다. 새 서비스 워커가 즉시 활성화되도록 보장합니다.
25-28: LGTM!clients.claim()추가로 즉시 제어 확보
clients.claim()을 추가하여 활성화 즉시 모든 클라이언트를 제어할 수 있도록 개선했습니다. 이는 서비스 워커 모범 사례입니다.
| const url = payload.data?.url || 'https://www.pinback.today'; | ||
|
|
||
| const notificationTitle = 'pinback'; // 무조건 기본값 | ||
| const notificationOptions = { | ||
| body: payload.data?.body ?? '저장한 북마크를 확인해 보세요!', | ||
| icon: payload.data?.icon ?? '/FCM-IMG.png', | ||
| data: { | ||
| url: 'https://www.pinback.today', | ||
| }, | ||
| body: '저장한 북마크를 확인해 보세요!', | ||
| icon: '/FCM-IMG.png', | ||
| data: { url }, | ||
| requireInteraction: true, | ||
| renotify: true, | ||
| }; |
There was a problem hiding this comment.
URL 검증 누락으로 인한 보안 취약점
payload.data?.url에서 추출된 URL을 검증 없이 사용하고 있습니다. 악의적인 FCM 메시지가 javascript:, data:, 또는 기타 위험한 스킴을 포함한 URL을 전달할 수 있어 클릭 시 XSS 공격에 노출될 수 있습니다.
다음 diff를 적용하여 URL 스킴을 검증하세요:
- const url = payload.data?.url || 'https://www.pinback.today';
+ const rawUrl = payload.data?.url || 'https://www.pinback.today';
+ let url;
+ try {
+ const parsedUrl = new URL(rawUrl);
+ // HTTP/HTTPS만 허용
+ if (parsedUrl.protocol === 'http:' || parsedUrl.protocol === 'https:') {
+ url = rawUrl;
+ } else {
+ console.warn('Invalid URL scheme:', parsedUrl.protocol);
+ url = 'https://www.pinback.today';
+ }
+ } catch (e) {
+ console.warn('Invalid URL:', rawUrl);
+ url = 'https://www.pinback.today';
+ }📝 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 url = payload.data?.url || 'https://www.pinback.today'; | |
| const notificationTitle = 'pinback'; // 무조건 기본값 | |
| const notificationOptions = { | |
| body: payload.data?.body ?? '저장한 북마크를 확인해 보세요!', | |
| icon: payload.data?.icon ?? '/FCM-IMG.png', | |
| data: { | |
| url: 'https://www.pinback.today', | |
| }, | |
| body: '저장한 북마크를 확인해 보세요!', | |
| icon: '/FCM-IMG.png', | |
| data: { url }, | |
| requireInteraction: true, | |
| renotify: true, | |
| }; | |
| const rawUrl = payload.data?.url || 'https://www.pinback.today'; | |
| let url; | |
| try { | |
| const parsedUrl = new URL(rawUrl); | |
| // HTTP/HTTPS만 허용 | |
| if (parsedUrl.protocol === 'http:' || parsedUrl.protocol === 'https:') { | |
| url = rawUrl; | |
| } else { | |
| console.warn('Invalid URL scheme:', parsedUrl.protocol); | |
| url = 'https://www.pinback.today'; | |
| } | |
| } catch (e) { | |
| console.warn('Invalid URL:', rawUrl); | |
| url = 'https://www.pinback.today'; | |
| } | |
| const notificationTitle = 'pinback'; // 무조건 기본값 | |
| const notificationOptions = { | |
| body: '저장한 북마크를 확인해 보세요!', | |
| icon: '/FCM-IMG.png', | |
| data: { url }, | |
| requireInteraction: true, | |
| renotify: true, | |
| }; |
🤖 Prompt for AI Agents
In apps/client/public/firebase-messaging-sw.js around lines 37 to 46, the code
uses payload.data?.url without validation which allows dangerous schemes
(javascript:, data:, etc.); validate and sanitize the URL before using it by
parsing with the URL constructor inside a try/catch, ensure the protocol is
either "http:" or "https:", and if parsing fails or the protocol is not allowed
fall back to the safe default 'https://www.pinback.today'; assign this
validated/sanitized URL to notificationOptions.data.url (do not use the raw
payload value).
1 participant
📌 Related Issues
⭐ PR Point (To Reviewer)
[참고] 서버쪽이랑 FCM 알람 오류 실시간 확인 중이라!! 커밋 중복되게 계속 수정 중입니다 ㅠㅠ
📄 Tasks
📷 Screenshot
Summary by CodeRabbit