Repository navigation
feat(realtime): add real-time permission revocation with zero-trust security - #256
Conversation
…ecurity Implements enterprise-grade real-time access revocation to close the "check once, trust forever" security gap. When permissions are revoked, connected sockets are immediately removed from rooms. Key changes: - Add SocketRegistry for tracking user->socket->room mappings - Add /api/kick endpoint for server-side socket ejection - Add per-event authorization module for sensitive write operations - Integrate kick calls into permission revocation API routes - Add useAccessRevocation hook for client-side notification/redirect - Add comprehensive test coverage for all new modules This ensures sub-5-second revocation latency for enterprise security requirements (SOC 2 compliance, employee offboarding, etc). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a real-time permission-revocation system: a SocketRegistry, Kick API/handler with signature verification, per-event reauthorization, server integration to track sockets/rooms and expose /api/kick, client hook to handle access_revoked events, web helpers to invoke kicks, route integrations, tests, and design documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin/API
participant WebAPI as Web API
participant Realtime as Realtime Server
participant Registry as Socket Registry
participant Client as Client App
Admin->>WebAPI: Remove member / Revoke permission
WebAPI->>WebAPI: Update DB, broadcast member_removed
WebAPI->>Realtime: POST /api/kick (HMAC-signed)
Realtime->>Realtime: verifySignature, parse & validate payload
Realtime->>Registry: getSocketsForUser / getSocketsForUserInRoom
Registry-->>Realtime: matching socketIds
Realtime->>Realtime: executeKick (leave rooms, emit access_revoked)
Realtime->>Client: emit access_revoked to affected sockets
Client->>Client: show toast / redirect or silent handling
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1e8032f9a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await Promise.all([ | ||
| kickUserFromDrive(driveId, targetUserId, 'member_removed', access.drive.name), | ||
| kickUserFromDriveActivity(driveId, targetUserId, 'member_removed'), |
There was a problem hiding this comment.
When a drive member is removed, this code only calls kickUserFromDrive and kickUserFromDriveActivity, which ejects sockets from drive:{id} and activity:drive:{id} rooms, but page rooms are joined by raw pageId (see join_channel in apps/realtime/src/index.ts) and are not matched by the drive room pattern. That means a removed user who already joined one or more page rooms will continue receiving real‑time page updates for those pages until they disconnect, despite their permissions being deleted. Consider kicking from each page room in the drive (or using a room naming scheme that lets you match all pages in a drive) to fully revoke access.
Useful? React with 👍 / 👎.
| useEffect(() => { | ||
| const socket = getSocket(); | ||
| if (!socket) return; | ||
|
|
||
| socket.on('access_revoked', handleAccessRevoked); |
There was a problem hiding this comment.
The effect subscribes once using getSocket() and never reruns when the socket instance changes, so if the store replaces the socket (e.g., on forced reconnect after auth refresh), the new socket will not have the access_revoked handler attached and revocation events will be missed. Subscribing to useSocketStore(state => state.socket) (or similar) and using the socket object as a dependency would ensure the listener is re-registered on reconnect.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use vi.hoisted() to ensure mock variables are available before mock factories run - Add missing websocket mock to permissions route test Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/realtime/src/__tests__/per-event-auth.test.ts`:
- Around line 68-83: The test suite is missing coverage for three
SensitiveEventType values and for the async authorization logic in
reauthorizePageAccess; update the SensitiveEventType test (calling
isSensitiveEvent) to include 'task_create', 'task_update', and 'task_delete',
and add unit tests for reauthorizePageAccess that mock getUserAccessLevel to
assert behavior for allowed access, denied access (throws or returns false as
appropriate), and error propagation; ensure tests exercise the path where
reauthorizePageAccess resolves when getUserAccessLevel grants permission,
rejects/throws when it denies, and properly handles/propagates unexpected errors
from getUserAccessLevel.
In `@apps/realtime/src/kick-handler.ts`:
- Around line 47-54: The JSON parse in parseKickRequest currently uses a blind
type assertion to KickPayload; instead parse to unknown (or any) and then call
the existing validateKickPayload(payload) to perform runtime checks (including
adding/ensuring validation for reason being a string and metadata being an
object if not already checked). Update parseKickRequest to catch JSON parse
errors, call validateKickPayload on the parsed value, and return { success:
false, error: ... } when validation fails; reference the types KickPayload and
ParseResult and the validator function validateKickPayload to locate and
implement the change.
In `@docs/plans/2026-01-26-real-time-revocation-design.md`:
- Around line 22-52: The fenced ASCII diagram in the docs/plans file is missing
a language specifier; update the code fence that opens the diagram (the triple
backticks before the box starting with "┌────────────────...") to include a
language tag such as text (i.e., change ``` to ```text) so the block renders
correctly and satisfies markdown linting; locate the fenced block by the ASCII
diagram header "Permission Change Flow" and modify only the opening fence.
🧹 Nitpick comments (12)
apps/web/src/lib/websocket/socket-utils.ts (1)
85-101: Consider importing types from a shared location to avoid duplication.
KickPayloadandKickResultare defined identically in bothapps/web/src/lib/websocket/socket-utils.tsandapps/realtime/src/kick-handler.ts. This duplication can lead to drift if one is updated without the other.Consider extracting these interfaces to a shared package (e.g.,
@pagespace/lib/types/realtime) and importing from there in both locations to ensure type consistency.apps/realtime/src/__tests__/kick-api.test.ts (1)
121-139: Room pattern matching tests don't exercise the actual implementation.These tests use inline string comparison (
room === exactPattern) and a local regex (/^drive:/) rather than testing the actualroomMatchesPatternfunction fromkick-handler.ts. This means the production room-matching logic isn't being validated.♻️ Suggested improvement
Import and test the actual function:
import { parseKickRequest, validateKickPayload, KickPayload, KickResult, + roomMatchesPattern, } from '../kick-handler'; describe('room pattern matching', () => { it('given exact room pattern, should match only that room', () => { - const exactPattern = 'drive:drive-123'; - const room = 'drive:drive-123'; - expect(room === exactPattern).toBe(true); + expect(roomMatchesPattern('drive:drive-123', 'drive:drive-123')).toBe(true); + expect(roomMatchesPattern('drive:drive-456', 'drive:drive-123')).toBe(false); });apps/realtime/src/per-event-auth.ts (2)
62-86: UnusedresourceIdparameter inshouldReauthorize.The
resourceIdproperty inReauthorizeParamsis never used within the function body. Either remove it from the interface or document its intended future use.♻️ Proposed fix
If not needed:
interface ReauthorizeParams { eventType: string; roomType: 'page' | 'drive' | 'activity' | 'dm' | 'notification'; - resourceId: string; }Or add a TODO comment explaining future use.
42-53: Type and Set definitions can drift.
SensitiveEventType(lines 19-29) andSENSITIVE_EVENTS(lines 42-53) duplicate the same values. If one is updated without the other, they'll become inconsistent.♻️ Suggested pattern to keep them in sync
const SENSITIVE_EVENTS = [ 'document_update', 'page_content_change', 'page_delete', 'page_move', 'file_upload', 'comment_create', 'comment_delete', 'task_create', 'task_update', 'task_delete', ] as const; export type SensitiveEventType = typeof SENSITIVE_EVENTS[number]; const SENSITIVE_EVENTS_SET = new Set<string>(SENSITIVE_EVENTS); export function isSensitiveEvent(eventType: string): boolean { return SENSITIVE_EVENTS_SET.has(eventType); }apps/realtime/src/__tests__/per-event-auth.test.ts (1)
9-10: Unused imports:beforeEachandvi.These are imported but not used in the test file.
♻️ Proposed fix
-import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect } from 'vitest';apps/web/src/app/api/pages/[pageId]/permissions/route.ts (1)
191-195: Consider handling kick failures for observability.The kick operations are critical for zero-trust security but failures are silently swallowed. While it's acceptable to not fail the HTTP response (permission was already revoked in DB), logging failures would aid debugging and security auditing.
🔧 Proposed improvement for error visibility
// CRITICAL: Kick user from real-time rooms immediately (zero-trust revocation) - await Promise.all([ - kickUserFromPage(pageId, userId, 'permission_revoked'), - kickUserFromPageActivity(pageId, userId, 'permission_revoked'), - ]); + try { + await Promise.all([ + kickUserFromPage(pageId, userId, 'permission_revoked'), + kickUserFromPageActivity(pageId, userId, 'permission_revoked'), + ]); + } catch (kickError) { + // Log but don't fail - permission is already revoked in DB + loggers.api.warn('Failed to kick user from real-time rooms', { + pageId, + userId, + error: kickError instanceof Error ? kickError.message : String(kickError), + }); + }apps/web/src/app/api/pages/[pageId]/permissions/__tests__/route.test.ts (1)
72-76: Consider adding test coverage for kick function calls.The mock is correctly set up, but no test explicitly verifies that
kickUserFromPageandkickUserFromPageActivityare called during permission deletion. This would strengthen the test contract for the real-time revocation feature.🧪 Add test to verify kick calls
Add to the DELETE describe block:
import { kickUserFromPage, kickUserFromPageActivity } from '@/lib/websocket'; // In describe('side effects (notifications)'): it('kicks user from real-time rooms on successful deletion', async () => { await DELETE( createRequest({ userId: 'user_456' }), { params: mockParams } ); expect(kickUserFromPage).toHaveBeenCalledWith( mockPageId, 'user_456', 'permission_revoked' ); expect(kickUserFromPageActivity).toHaveBeenCalledWith( mockPageId, 'user_456', 'permission_revoked' ); }); it('does NOT kick user when authorization fails', async () => { (permissionManagementService.canUserManagePermissions as Mock).mockResolvedValue(false); await DELETE( createRequest({ userId: 'user_456' }), { params: mockParams } ); expect(kickUserFromPage).not.toHaveBeenCalled(); expect(kickUserFromPageActivity).not.toHaveBeenCalled(); });apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts (1)
316-321: Consider handling kick failures for observability (consistent with permissions route).Same concern as the permissions route - kick failures are silently swallowed. For consistency and auditability, consider logging failures without failing the request.
🔧 Proposed improvement for error visibility
// CRITICAL: Kick user from real-time rooms immediately (zero-trust revocation) // This ensures the user stops receiving updates even if their socket is still connected - await Promise.all([ - kickUserFromDrive(driveId, targetUserId, 'member_removed', access.drive.name), - kickUserFromDriveActivity(driveId, targetUserId, 'member_removed'), - ]); + try { + await Promise.all([ + kickUserFromDrive(driveId, targetUserId, 'member_removed', access.drive.name), + kickUserFromDriveActivity(driveId, targetUserId, 'member_removed'), + ]); + } catch (kickError) { + // Log but don't fail - member is already removed from DB + loggers.api.warn('Failed to kick user from real-time rooms', { + driveId, + targetUserId, + error: kickError instanceof Error ? kickError.message : String(kickError), + }); + }apps/web/src/hooks/useAccessRevocation.ts (2)
30-56: Potential false positive in pathname matching.The
pathname.includes(metadata.driveId)andpathname.includes(metadata.pageId)checks (lines 40-42, 50-52) could match unintended routes if the ID happens to appear elsewhere in the URL (e.g., as part of another ID or query param). The primary checks using/drives/and/pages/prefixes are more precise.🔧 Consider removing the loose includes checks
function shouldRedirect(pathname: string, payload: AccessRevokedPayload): boolean { const { room, metadata } = payload; // Check if user is in the affected drive if (room.startsWith('drive:') && metadata?.driveId) { // User is viewing something in this drive if (pathname.includes(`/drives/${metadata.driveId}`)) { return true; } - // Also check if pathname includes the drive ID in any format - if (pathname.includes(metadata.driveId)) { - return true; - } } // Check if user is viewing the affected page if (metadata?.pageId) { if (pathname.includes(`/pages/${metadata.pageId}`)) { return true; } - if (pathname.includes(metadata.pageId)) { - return true; - } } return false; }
115-116: Consider removing or conditionalizing debug logging.The
console.logstatement will appear in production browser consoles. For security events, this may be acceptable for debugging, but consider using a debug flag or removing it for cleaner production output.apps/web/src/hooks/__tests__/useAccessRevocation.test.ts (1)
76-148: Consider adding test coverage for edge cases.The test suite covers the main scenarios well but could be strengthened with:
- Deduplication test: Verify that duplicate events for the same room within 5 seconds don't trigger multiple toasts
- Session revocation redirect: Verify
session_revokedreason redirects to/auth/logininstead of/dashboard🧪 Additional test cases
it('given duplicate revocation events for same room, should show toast only once', () => { renderHook(() => useAccessRevocation()); const handlerCall = mockSocket.on.mock.calls.find( (call) => call[0] === 'access_revoked' ); const handler = handlerCall?.[1]; const payload = { room: 'drive:test-drive-id', reason: 'member_removed' as const, metadata: { driveId: 'test-drive-id', driveName: 'Test Drive' }, }; // Trigger same event twice act(() => { handler(payload); handler(payload); }); // Should only show one toast expect(mockToast.error).toHaveBeenCalledTimes(1); }); it('given session_revoked reason, should redirect to /auth/login', () => { renderHook(() => useAccessRevocation()); const handlerCall = mockSocket.on.mock.calls.find( (call) => call[0] === 'access_revoked' ); const handler = handlerCall?.[1]; act(() => { handler({ room: 'drive:test-drive-id', reason: 'session_revoked', metadata: { driveId: 'test-drive-id' }, }); }); expect(mockPush).toHaveBeenCalledWith('/auth/login'); });apps/realtime/src/socket-registry.ts (1)
149-153: Consider minor optimization for high-traffic scenarios.The current implementation creates intermediate arrays. For very high-traffic scenarios with many sockets per user/room, this could be optimized to iterate directly over Sets. However, for typical usage patterns, this implementation is acceptable.
♻️ Optional: Avoid intermediate array allocation
getSocketsForUserInRoom(userId: string, room: string): string[] { - const userSockets = this.getSocketsForUser(userId); - const roomSockets = new Set(this.getSocketsInRoom(room)); - return userSockets.filter(socketId => roomSockets.has(socketId)); + const userSockets = this.userToSockets.get(userId); + const roomSockets = this.roomToSockets.get(room); + if (!userSockets || !roomSockets) return []; + const result: string[] = []; + for (const socketId of userSockets) { + if (roomSockets.has(socketId)) { + result.push(socketId); + } + } + return result; }
| describe('SensitiveEventType', () => { | ||
| it('should include all write operations', () => { | ||
| const sensitiveEvents: SensitiveEventType[] = [ | ||
| 'document_update', | ||
| 'page_content_change', | ||
| 'page_delete', | ||
| 'page_move', | ||
| 'file_upload', | ||
| 'comment_create', | ||
| 'comment_delete', | ||
| ]; | ||
|
|
||
| sensitiveEvents.forEach(event => { | ||
| expect(isSensitiveEvent(event)).toBe(true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Test coverage is incomplete.
-
Missing
reauthorizePageAccesstests: The main async authorization function isn't tested at all. This is a significant gap since it contains the actual permission checking logic. -
Incomplete
SensitiveEventTypearray: The test checks only 7 of the 10 sensitive events. Missing:task_create,task_update,task_delete.
💚 Proposed fix for complete type coverage
it('should include all write operations', () => {
const sensitiveEvents: SensitiveEventType[] = [
'document_update',
'page_content_change',
'page_delete',
'page_move',
'file_upload',
'comment_create',
'comment_delete',
+ 'task_create',
+ 'task_update',
+ 'task_delete',
];Consider adding tests for reauthorizePageAccess that mock getUserAccessLevel and verify authorization logic for granted/denied/error cases.
🤖 Prompt for AI Agents
In `@apps/realtime/src/__tests__/per-event-auth.test.ts` around lines 68 - 83, The
test suite is missing coverage for three SensitiveEventType values and for the
async authorization logic in reauthorizePageAccess; update the
SensitiveEventType test (calling isSensitiveEvent) to include 'task_create',
'task_update', and 'task_delete', and add unit tests for reauthorizePageAccess
that mock getUserAccessLevel to assert behavior for allowed access, denied
access (throws or returns false as appropriate), and error propagation; ensure
tests exercise the path where reauthorizePageAccess resolves when
getUserAccessLevel grants permission, rejects/throws when it denies, and
properly handles/propagates unexpected errors from getUserAccessLevel.
| export function parseKickRequest(body: string): ParseResult { | ||
| try { | ||
| const payload = JSON.parse(body) as KickPayload; | ||
| return { success: true, payload }; | ||
| } catch { | ||
| return { success: false, error: 'Invalid JSON' }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Type assertion without runtime validation could allow malformed payloads.
The as KickPayload assertion trusts the parsed JSON structure. While validateKickPayload checks userId and roomPattern, malformed reason or metadata fields would pass through unchecked.
🔧 Suggested enhancement to validate reason field
+const VALID_REASONS = ['member_removed', 'role_changed', 'permission_revoked', 'session_revoked'] as const;
+
export function validateKickPayload(payload: KickPayload): ValidationResult {
if (!payload.userId || typeof payload.userId !== 'string' || payload.userId.trim() === '') {
return { valid: false, error: 'Missing or invalid userId' };
}
if (!payload.roomPattern || typeof payload.roomPattern !== 'string' || payload.roomPattern.trim() === '') {
return { valid: false, error: 'Missing or invalid roomPattern' };
}
+ if (!payload.reason || !VALID_REASONS.includes(payload.reason)) {
+ return { valid: false, error: 'Missing or invalid reason' };
+ }
+
return { valid: true };
}🤖 Prompt for AI Agents
In `@apps/realtime/src/kick-handler.ts` around lines 47 - 54, The JSON parse in
parseKickRequest currently uses a blind type assertion to KickPayload; instead
parse to unknown (or any) and then call the existing
validateKickPayload(payload) to perform runtime checks (including
adding/ensuring validation for reason being a string and metadata being an
object if not already checked). Update parseKickRequest to catch JSON parse
errors, call validateKickPayload on the parsed value, and return { success:
false, error: ... } when validation fails; reference the types KickPayload and
ParseResult and the validator function validateKickPayload to locate and
implement the change.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- P1: Kick users from page rooms when removed from drive (not just drive room) - P2: Reattach access_revoked listener when socket reconnects - Add missing task events (task_create, task_update, task_delete) to tests - Validate reason field in kick payload - Add language specifier to markdown code block Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts (1)
318-341: Prevent kick failures from skipping cache invalidation or returning false errors.If any kick call throws, the handler returns 500 even though the member was already removed, and the permission cache invalidation is skipped. That can leave stale access in cache and creates a confusing client retry path. Make kicks best-effort and ensure invalidation runs in a
finallyblock.✅ Suggested fix (best-effort kicks + always-invalidate)
- // CRITICAL: Kick user from real-time rooms immediately (zero-trust revocation) - // This ensures the user stops receiving updates even if their socket is still connected - - // First, kick from drive-level rooms - await Promise.all([ - kickUserFromDrive(driveId, targetUserId, 'member_removed', access.drive.name), - kickUserFromDriveActivity(driveId, targetUserId, 'member_removed'), - ]); - - // Also kick from all page rooms in this drive (page rooms use pageId, not drive pattern) - const drivePages = await db.select({ id: pages.id }).from(pages).where(eq(pages.driveId, driveId)); - if (drivePages.length > 0) { - const pageKickPromises = drivePages.flatMap((page) => [ - kickUserFromPage(page.id, targetUserId, 'member_removed'), - kickUserFromPageActivity(page.id, targetUserId, 'member_removed'), - ]); - await Promise.all(pageKickPromises); - } - - // Invalidate permission caches so removed user loses access immediately - await Promise.all([ - invalidateUserPermissions(targetUserId), - invalidateDrivePermissions(driveId), - ]); + try { + // CRITICAL: Kick user from real-time rooms immediately (zero-trust revocation) + // This ensures the user stops receiving updates even if their socket is still connected + + // First, kick from drive-level rooms + await Promise.allSettled([ + kickUserFromDrive(driveId, targetUserId, 'member_removed', access.drive.name), + kickUserFromDriveActivity(driveId, targetUserId, 'member_removed'), + ]); + + // Also kick from all page rooms in this drive (page rooms use pageId, not drive pattern) + const drivePages = await db.select({ id: pages.id }).from(pages).where(eq(pages.driveId, driveId)); + if (drivePages.length > 0) { + const pageKickPromises = drivePages.flatMap((page) => [ + kickUserFromPage(page.id, targetUserId, 'member_removed'), + kickUserFromPageActivity(page.id, targetUserId, 'member_removed'), + ]); + await Promise.allSettled(pageKickPromises); + } + } catch (error) { + loggers.api.error('Kick failed during member removal:', error as Error); + } finally { + // Invalidate permission caches so removed user loses access immediately + await Promise.all([ + invalidateUserPermissions(targetUserId), + invalidateDrivePermissions(driveId), + ]); + }
🧹 Nitpick comments (2)
apps/realtime/src/__tests__/per-event-auth.test.ts (2)
9-9: Remove unused imports.
beforeEachandviare imported but never used in this test file.🧹 Proposed fix
-import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect } from 'vitest';
35-65: Consider adding test coverage for notification room type.The
shouldReauthorizefunction explicitly handles notification rooms (returningfalse), but there's no test covering this branch. Adding a test would improve branch coverage.💚 Suggested test case
it('given notification room event, should NOT require re-auth (user-specific room)', () => { const result = shouldReauthorize({ eventType: 'document_update', roomType: 'notification', resourceId: 'user-123', }); expect(result).toBe(false); });
- Remove unused beforeEach and vi imports from per-event-auth.test.ts - Add test coverage for notification room type in shouldReauthorize Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
/api/kickendpoint with HMAC signature verification for server-side socket removalContext
This closes the "check once, trust forever" security gap where users who joined a Socket.IO room would continue receiving real-time updates even after their access was revoked. This is critical for enterprise security requirements including SOC 2 compliance and employee offboarding scenarios.
Architecture
Server-side enforcement (primary security layer):
Client-side UX (graceful handling):
access_revokedeventsTest plan
cd apps/realtime && pnpm vitest runto verify socket-registry, kick-api, and per-event-auth tests passcd apps/web && pnpm vitest run src/hooks/__tests__/useAccessRevocation.test.tsto verify client hook tests🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.