Skip to content

feat(monitoring): extend activity logging for enterprise compliance - #112

Merged
2witstudios merged 6 commits into
masterfrom
claude/audit-activity-monitoring-bQuWo
Dec 22, 2025
Merged

2witstudios merged 6 commits into
masterfrom
claude/audit-activity-monitoring-bQuWo

Conversation

@2witstudios

@2witstudios 2witstudios commented Dec 22, 2025 •

Copy link
Copy Markdown
Owner

Adds comprehensive audit trail logging for previously untracked operations:

Security-Critical Operations:

  • Account deletion (logged before GDPR anonymization)
  • Password changes (security events)
  • Token/device revocation (MCP and device tokens)
  • Member role changes (privilege escalation tracking)

Drive & Workspace Operations:

  • Drive creation, update, trash, and restore
  • Drive membership (add members, role changes)
  • Drive role management (create, update, delete)

File & Page Operations:

  • File uploads with metadata
  • Page reordering/moving

Schema Changes:

  • Extended activity_operation enum with 15 new operation types
  • Extended activity_resource enum with 6 new resource types
  • Added migration 0023_activity_log_enums_expansion.sql

New Activity Logger Wrappers:

  • logMemberActivity() - drive membership changes
  • logRoleActivity() - RBAC role changes
  • logUserActivity() - account security events
  • logTokenActivity() - token lifecycle
  • logFileActivity() - file operations

This closes critical gaps for SOX compliance (access control auditing)
and GDPR compliance (deletion audit trail).

Summary by CodeRabbit

  • New Features
    • Enhanced audit logging and activity tracking across account settings, authentication (tokens, password), drive and member management, page/file operations, and uploads — improves security visibility and compliance.
  • Chores
    • Added schema and package updates to support expanded activity types and resources.

✏️ Tip: You can customize this high-level summary in your review settings.

Adds comprehensive audit trail logging for previously untracked operations:

Security-Critical Operations:
- Account deletion (logged before GDPR anonymization)
- Password changes (security events)
- Token/device revocation (MCP and device tokens)
- Member role changes (privilege escalation tracking)

Drive & Workspace Operations:
- Drive creation, update, trash, and restore
- Drive membership (add members, role changes)
- Drive role management (create, update, delete)

File & Page Operations:
- File uploads with metadata
- Page reordering/moving

Schema Changes:
- Extended activity_operation enum with 15 new operation types
- Extended activity_resource enum with 6 new resource types
- Added migration 0023_activity_log_enums_expansion.sql

New Activity Logger Wrappers:
- logMemberActivity() - drive membership changes
- logRoleActivity() - RBAC role changes
- logUserActivity() - account security events
- logTokenActivity() - token lifecycle
- logFileActivity() - file operations

This closes critical gaps for SOX compliance (access control auditing)
and GDPR compliance (deletion audit trail).
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Dec 22, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Adds audit logging: new enum values and DB migration, exposes a monitoring module, adds wrapper logging functions, and integrates actor-aware audit calls into many account, auth, drive, page, and upload API routes.

Changes

Cohort / File(s) Summary
Monitoring schema & migration
packages/db/src/schema/monitoring.ts, packages/db/drizzle/0023_big_vulcan.sql, packages/db/drizzle/meta/_journal.json
Added new activity_operation and activity_resource enum values and appended a migration and journal entry to add them in the database.
Activity logger library & exports
packages/lib/src/monitoring/activity-logger.ts, packages/lib/package.json
Extended ActivityOperation and ActivityResourceType unions, relaxed driveId to `string
Account & auth routes
apps/web/src/app/api/account/route.ts, apps/web/src/app/api/account/password/route.ts, apps/web/src/app/api/account/devices/[deviceId]/route.ts, apps/web/src/app/api/auth/mcp-tokens/route.ts, apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
Imported getActorInfo and per-resource log wrappers; added audit calls (e.g., token_revoke, token_create, password_change, profile_update, account_delete) after successful operations and adjusted a token revocation handler to check existence before logging.
Drive lifecycle routes
apps/web/src/app/api/drives/route.ts, apps/web/src/app/api/drives/[driveId]/route.ts, apps/web/src/app/api/drives/[driveId]/restore/route.ts
Added actor-aware logging for drive create, update (including updatedFields), delete/trash, and restore via logDriveActivity calls after successful operations.
Drive member & invite routes
apps/web/src/app/api/drives/[driveId]/members/route.ts, apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts, apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
Added logMemberActivity calls for member add/remove/role-change flows; invite route queries users to resolve email for logging and logs member_add events.
Drive role routes
apps/web/src/app/api/drives/[driveId]/roles/route.ts, apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
Added logRoleActivity calls on role create/update/delete and introduced summarizePermissions to produce boolean permission maps used in logs.
Page & upload routes
apps/web/src/app/api/pages/reorder/route.ts, apps/web/src/app/api/upload/route.ts
After successful page reorder and file upload, added actor-info retrieval and calls to logPageActivity/logFileActivity with metadata (positions, file details, drive/page ids).
Tests
apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts
Added mocks for @pagespace/lib/monitoring/activity-logger (getActorInfo, logTokenActivity) and switched tests to use db.query.mcpTokens.findFirst behavior for token existence scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to synchronization between DB enum additions and TypeScript unions (migration vs. runtime types).
  • Verify each route supplies the expected metadata shape (e.g., tokenName, deviceInfo, permissions summaries).
  • Confirm wrapper functions remain fire-and-forget and do not change route error behavior.

Possibly related PRs

Poem

🐰 Hopped in with a twitch and a grin,

I logged each move from tail to chin,
Tokens, drives, and members too —
A rabbit's trail of audit glue. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.69% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly describes the main purpose: extending activity logging for enterprise compliance. It directly aligns with the core changeset of adding comprehensive audit trail logging across multiple operations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch claude/audit-activity-monitoring-bQuWo

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (6)
apps/web/src/app/api/drives/[driveId]/roles/route.ts (1)

94-101: Consider error handling for getActorInfo to prevent blocking the response.

The getActorInfo call is awaited before the response is sent. If it fails or is slow, it will block the success response even though logging is a non-critical side-effect. Consider wrapping the entire logging block in a try-catch to prevent audit logging failures from affecting the primary operation.

🔎 Proposed defensive error handling
-    // Log activity for audit trail
-    const actorInfo = await getActorInfo(userId);
-    logRoleActivity(userId, 'create', {
-      roleId: newRole.id,
-      roleName: newRole.name,
-      driveId,
-      permissions: permissions as Record<string, boolean>,
-    }, actorInfo);
+    // Log activity for audit trail (best-effort, don't block response)
+    try {
+      const actorInfo = await getActorInfo(userId);
+      logRoleActivity(userId, 'create', {
+        roleId: newRole.id,
+        roleName: newRole.name,
+        driveId,
+        permissions: permissions as Record<string, boolean>,
+      }, actorInfo);
+    } catch (error) {
+      console.error('[RoleCreate] Failed to log activity:', error);
+    }
apps/web/src/app/api/pages/reorder/route.ts (1)

58-71: Consider error handling for getActorInfo to prevent blocking the response.

Similar to other logging implementations in this PR, the getActorInfo call is awaited before sending the response. Since logging is a non-critical side-effect for audit purposes, failures in actor information retrieval should not prevent a successful reorder operation from returning.

🔎 Proposed defensive error handling
-    // Log activity for audit trail (page moves affect tree structure)
-    const actorInfo = await getActorInfo(auth.userId);
-    logPageActivity(auth.userId, 'reorder', {
-      id: pageId,
-      title: result.pageTitle,
-      driveId: result.driveId,
-    }, {
-      ...actorInfo,
-      metadata: {
-        newParentId,
-        newPosition,
-        previousParentId: result.previousParentId,
-      },
-    });
+    // Log activity for audit trail (best-effort, don't block response)
+    try {
+      const actorInfo = await getActorInfo(auth.userId);
+      logPageActivity(auth.userId, 'reorder', {
+        id: pageId,
+        title: result.pageTitle,
+        driveId: result.driveId,
+      }, {
+        ...actorInfo,
+        metadata: {
+          newParentId,
+          newPosition,
+          previousParentId: result.previousParentId,
+        },
+      });
+    } catch (error) {
+      console.error('[PageReorder] Failed to log activity:', error);
+    }
apps/web/src/app/api/account/password/route.ts (1)

83-88: Consider error handling for getActorInfo to prevent blocking the response.

While password changes are critical security events that warrant audit logging, the current implementation blocks the success response if getActorInfo fails. For consistency with audit logging best practices (fire-and-forget), consider wrapping the logging in a try-catch block.

Note: If blocking is intentional for security-critical operations to ensure logs are written, this should be documented in a comment.

🔎 Proposed defensive error handling
-    // Log activity for audit trail (password changes are critical security events)
-    const actorInfo = await getActorInfo(userId);
-    logUserActivity(userId, 'password_change', {
-      targetUserId: userId,
-      targetUserEmail: undefined, // Don't expose email in logs for password changes
-    }, actorInfo);
+    // Log activity for audit trail (best-effort, don't block response)
+    try {
+      const actorInfo = await getActorInfo(userId);
+      logUserActivity(userId, 'password_change', {
+        targetUserId: userId,
+        targetUserEmail: undefined, // Don't expose email in logs for password changes
+      }, actorInfo);
+    } catch (error) {
+      console.error('[PasswordChange] Failed to log activity:', error);
+    }
apps/web/src/app/api/drives/[driveId]/members/route.ts (1)

91-99: Consider error handling for getActorInfo to prevent blocking the response.

The getActorInfo call blocks the success response if it fails. Since audit logging is a non-critical side-effect, consider wrapping the logging block in a try-catch to prevent logging failures from affecting the member addition operation.

🔎 Proposed defensive error handling
-    // Log activity for audit trail
-    const actorInfo = await getActorInfo(userId);
-    logMemberActivity(userId, 'member_add', {
-      driveId,
-      driveName: access.drive.name,
-      targetUserId: invitedUserId,
-      targetUserEmail: newMember.email,
-      role: role as string,
-    }, actorInfo);
+    // Log activity for audit trail (best-effort, don't block response)
+    try {
+      const actorInfo = await getActorInfo(userId);
+      logMemberActivity(userId, 'member_add', {
+        driveId,
+        driveName: access.drive.name,
+        targetUserId: invitedUserId,
+        targetUserEmail: newMember.email,
+        role: role as string,
+      }, actorInfo);
+    } catch (error) {
+      console.error('[MemberAdd] Failed to log activity:', error);
+    }
apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts (1)

134-143: Consider error handling for getActorInfo to prevent blocking the response.

The getActorInfo call blocks the success response if it fails. Since audit logging is a non-critical side-effect and occurs after notifications have already been sent, consider wrapping the logging block in a try-catch to prevent logging failures from affecting the member role update operation.

🔎 Proposed defensive error handling
-
-      // Log activity for audit trail (role change is a critical security event)
-      const actorInfo = await getActorInfo(currentUserId);
-      logMemberActivity(currentUserId, 'member_role_change', {
-        driveId,
-        driveName: access.drive.name,
-        targetUserId: userId,
-        targetUserEmail: memberData.email,
-        role: role as string,
-        previousRole: oldRole as string,
-      }, actorInfo);
+
+      // Log activity for audit trail (best-effort, don't block response)
+      try {
+        const actorInfo = await getActorInfo(currentUserId);
+        logMemberActivity(currentUserId, 'member_role_change', {
+          driveId,
+          driveName: access.drive.name,
+          targetUserId: userId,
+          targetUserEmail: memberData.email,
+          role: role as string,
+          previousRole: oldRole as string,
+        }, actorInfo);
+      } catch (error) {
+        console.error('[MemberRoleChange] Failed to log activity:', error);
+      }
apps/web/src/app/api/account/devices/[deviceId]/route.ts (1)

55-62: Consider error handling for getActorInfo to prevent blocking the response.

The getActorInfo call blocks the success response if it fails. Since audit logging is a non-critical side-effect and occurs after device revocation and token deletion are complete, consider wrapping the logging block in a try-catch to prevent logging failures from affecting the device revocation operation.

🔎 Proposed defensive error handling
-    // Log activity for audit trail (device revocation is a security event)
-    const actorInfo = await getActorInfo(userId);
-    logTokenActivity(userId, 'token_revoke', {
-      tokenId: deviceId,
-      tokenType: 'device',
-      tokenName: device.deviceName ?? undefined,
-      deviceInfo: `${device.platform ?? 'Unknown'} - ${device.deviceName ?? 'Unknown'}`,
-    }, actorInfo);
+    // Log activity for audit trail (best-effort, don't block response)
+    try {
+      const actorInfo = await getActorInfo(userId);
+      logTokenActivity(userId, 'token_revoke', {
+        tokenId: deviceId,
+        tokenType: 'device',
+        tokenName: device.deviceName ?? undefined,
+        deviceInfo: `${device.platform ?? 'Unknown'} - ${device.deviceName ?? 'Unknown'}`,
+      }, actorInfo);
+    } catch (error) {
+      console.error('[DeviceRevoke] Failed to log activity:', error);
+    }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02cc52c and 43a3546.

📒 Files selected for processing (19)
  • apps/web/src/app/api/account/devices/[deviceId]/route.ts
  • apps/web/src/app/api/account/password/route.ts
  • apps/web/src/app/api/account/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/restore/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/route.ts
  • apps/web/src/app/api/drives/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
  • apps/web/src/app/api/upload/route.ts
  • packages/db/drizzle/0023_activity_log_enums_expansion.sql
  • packages/db/drizzle/meta/_journal.json
  • packages/db/src/schema/monitoring.ts
  • packages/lib/src/monitoring/activity-logger.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/src/app/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/src/app/**/*.ts: In Next.js 15 route handlers with dynamic routes, you MUST await context.params before destructuring because params are Promise objects
Get request body using const body = await request.json();
Get search params using const { searchParams } = new URL(request.url);
Return JSON responses using return Response.json(data) or return NextResponse.json(data)

Files:

  • apps/web/src/app/api/account/password/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/upload/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
  • apps/web/src/app/api/account/route.ts
  • apps/web/src/app/api/drives/route.ts
  • apps/web/src/app/api/account/devices/[deviceId]/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/route.ts
  • apps/web/src/app/api/drives/[driveId]/restore/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never use any types in TypeScript code - always use proper TypeScript types

**/*.{ts,tsx}: No any types - always use proper TypeScript types
Use kebab-case for filenames (e.g., image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: import getUserAccessLevel and canUserEditPage from @pagespace/lib/permissions
Use Drizzle client from @pagespace/db for all database access
Always structure message content using the message parts structure: { parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode

Files:

  • apps/web/src/app/api/account/password/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • packages/db/src/schema/monitoring.ts
  • apps/web/src/app/api/drives/[driveId]/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/upload/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
  • apps/web/src/app/api/account/route.ts
  • apps/web/src/app/api/drives/route.ts
  • apps/web/src/app/api/account/devices/[deviceId]/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
  • packages/lib/src/monitoring/activity-logger.ts
  • apps/web/src/app/api/auth/mcp-tokens/route.ts
  • apps/web/src/app/api/drives/[driveId]/restore/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
apps/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/**/*.{ts,tsx}: Use centralized permission functions from @pagespace/lib/permissions for access control, such as getUserAccessLevel() and canUserEditPage()
Always use Drizzle client from @pagespace/db for database access instead of direct database connections
Always use message parts structure with parts array containing objects with type and text fields when constructing messages for AI

Files:

  • apps/web/src/app/api/account/password/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/upload/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
  • apps/web/src/app/api/account/route.ts
  • apps/web/src/app/api/drives/route.ts
  • apps/web/src/app/api/account/devices/[deviceId]/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/route.ts
  • apps/web/src/app/api/drives/[driveId]/restore/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
apps/web/src/app/**/route.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 dynamic routes, params are Promise objects and must be awaited before destructuring
Get request body using const body = await request.json();
Return JSON responses using Response.json(data) or NextResponse.json(data) in route handlers

Files:

  • apps/web/src/app/api/account/password/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/upload/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
  • apps/web/src/app/api/account/route.ts
  • apps/web/src/app/api/drives/route.ts
  • apps/web/src/app/api/account/devices/[deviceId]/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/route.ts
  • apps/web/src/app/api/drives/[driveId]/restore/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Format code with Prettier and lint with ESLint using the configuration at apps/web/eslint.config.mjs

Files:

  • apps/web/src/app/api/account/password/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • packages/db/src/schema/monitoring.ts
  • apps/web/src/app/api/drives/[driveId]/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/upload/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
  • apps/web/src/app/api/account/route.ts
  • apps/web/src/app/api/drives/route.ts
  • apps/web/src/app/api/account/devices/[deviceId]/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
  • packages/lib/src/monitoring/activity-logger.ts
  • apps/web/src/app/api/auth/mcp-tokens/route.ts
  • apps/web/src/app/api/drives/[driveId]/restore/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
apps/web/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching

Files:

  • apps/web/src/app/api/account/password/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/upload/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
  • apps/web/src/app/api/account/route.ts
  • apps/web/src/app/api/drives/route.ts
  • apps/web/src/app/api/account/devices/[deviceId]/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/route.ts
  • apps/web/src/app/api/drives/[driveId]/restore/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
🧠 Learnings (12)
📚 Learning: 2025-12-14T14:54:47.122Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.122Z
Learning: Applies to **/*.{ts,tsx} : Use centralized permission logic: import `getUserAccessLevel` and `canUserEditPage` from `pagespace/lib/permissions`

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • apps/web/src/app/api/upload/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/**/*.{ts,tsx} : Use centralized permission functions from `pagespace/lib/permissions` for access control, such as `getUserAccessLevel()` and `canUserEditPage()`

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/account/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
📚 Learning: 2025-12-14T14:54:47.122Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.122Z
Learning: Applies to **/*.{ts,tsx} : Use Drizzle client from `pagespace/db` for all database access

Applied to files:

  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to **/*.{ts,tsx} : Always use the Drizzle client and database exports from `pagespace/db` (e.g., `import { db, pages } from 'pagespace/db'`) for all database access

Applied to files:

  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/**/*.{ts,tsx} : Always use Drizzle client from `pagespace/db` for database access instead of direct database connections

Applied to files:

  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use Drizzle client and queries from `pagespace/db` for database access instead of direct queries

Applied to files:

  • apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: The tech stack consists of Next.js 15 with App Router, TypeScript, Tailwind, shadcn/ui, PostgreSQL with Drizzle ORM, Ollama/Vercel AI SDK, custom JWT auth, and Socket.IO for real-time features

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to app/api/**/*.{ts,tsx} : Use `const body = await request.json();` to extract request bodies, `const { searchParams } = new URL(request.url);` for query parameters, and `return Response.json(data)` or `return NextResponse.json(data)` for responses

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: This is a monorepo using pnpm workspaces with structure: `apps/web` (Next.js frontend/backend), `apps/realtime` (Socket.IO service), `apps/processor` (Express file/OCR pipeline), `packages/db` (Drizzle ORM), `packages/lib` (shared utilities)

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: The project uses a pnpm monorepo workspace with structure: `apps/web` (Next.js), `apps/realtime` (Socket.IO), `apps/processor` (Express), `packages/db` (Drizzle ORM), `packages/lib` (shared utilities)

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui (frontend), PostgreSQL + Drizzle ORM (database), Ollama + Vercel AI SDK + OpenRouter + Google AI SDK (AI), custom JWT auth, local filesystem storage, Socket.IO for real-time, Docker deployment

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/members/invite/route.ts
🧬 Code graph analysis (14)
apps/web/src/app/api/account/password/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logUserActivity (415-460)
apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logRoleActivity (378-409)
apps/web/src/app/api/drives/[driveId]/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logDriveActivity (250-284)
apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logTokenActivity (466-500)
apps/web/src/app/api/upload/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logFileActivity (506-539)
apps/web/src/app/api/drives/[driveId]/members/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logMemberActivity (337-372)
apps/web/src/app/api/drives/[driveId]/roles/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logRoleActivity (378-409)
apps/web/src/app/api/drives/[driveId]/members/invite/route.ts (3)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logMemberActivity (337-372)
packages/db/src/index.ts (3)
  • db (20-20)
  • eq (8-8)
  • users (27-27)
packages/db/src/schema/auth.ts (1)
  • users (10-33)
apps/web/src/app/api/account/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logUserActivity (415-460)
apps/web/src/app/api/drives/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logDriveActivity (250-284)
apps/web/src/app/api/pages/reorder/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logPageActivity (157-201)
apps/web/src/app/api/auth/mcp-tokens/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logTokenActivity (466-500)
apps/web/src/app/api/drives/[driveId]/restore/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logDriveActivity (250-284)
apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logMemberActivity (337-372)
⏰ 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: Unit Tests
🔇 Additional comments (22)
apps/web/src/app/api/drives/route.ts (2)

8-8: LGTM: Audit logging utilities imported correctly.

The import of getActorInfo and logDriveActivity is appropriate for the audit trail functionality being added.


73-78: Audit logging implementation is correct and well-structured.

The operation string 'create' is a valid ActivityOperation value. The implementation properly awaits getActorInfo and passes the result to logDriveActivity, which accepts the actor information as optional parameters. The placement before the response ensures the audit trail is captured reliably.

packages/db/drizzle/meta/_journal.json (1)

166-172: LGTM!

The migration journal entry is properly formatted and consistent with existing entries. It correctly references the activity log enums expansion migration.

packages/db/src/schema/monitoring.ts (1)

353-398: LGTM!

The enum expansions comprehensively cover the new audit logging requirements for enterprise compliance. The additions include:

  • Security-critical operations (login, logout, password_change, token operations)
  • Membership operations (member_add, member_remove, member_role_change)
  • Account operations (account_delete, profile_update, avatar_update)
  • File operations (upload, convert)
  • Corresponding resource types (user, member, role, file, token, device)

These align well with SOX and GDPR compliance requirements stated in the PR objectives.

apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts (2)

105-113: LGTM! Audit logging implementation is correct.

The activity logging for role updates is properly implemented. The code correctly captures actor information and logs the operation after a successful update, including both previous and new permission values for compliance auditing.


157-164: LGTM! Deletion audit logging is correct.

The activity logging for role deletion properly captures the role details and previous permissions before deletion, which is essential for maintaining a complete audit trail.

apps/web/src/app/api/drives/[driveId]/route.ts (2)

109-124: LGTM! Drive update audit logging is comprehensive.

The audit logging implementation correctly:

  • Captures actor information
  • Identifies which fields were updated
  • Preserves previous and new names in metadata
  • Logs after successful operation

This provides a complete audit trail for drive updates.


181-186: LGTM! Trash operation is properly logged.

The audit logging for drive deletion (soft delete/trash) is correctly implemented and captures the necessary context for compliance auditing.

packages/db/drizzle/0023_activity_log_enums_expansion.sql (1)

1-24: LGTM! Enum expansion follows PostgreSQL best practices.

The migration correctly uses ADD VALUE IF NOT EXISTS for safe idempotency. Note that PostgreSQL enum additions are append-only and cannot be rolled back without dropping and recreating the enum type, which would require downtime. This is acceptable for adding new enum values that won't be used until the application code is deployed.

apps/web/src/app/api/drives/[driveId]/restore/route.ts (1)

49-54: LGTM! Drive restoration audit logging is correct.

The activity logging is properly implemented, capturing the restore operation after successful completion with full actor context.

apps/web/src/app/api/account/route.ts (2)

94-103: LGTM! Profile update audit logging is well-implemented.

The implementation correctly:

  • Logs after successful profile update
  • Dynamically identifies which fields were updated
  • Includes all necessary context for security auditing

249-255: Excellent! Critical GDPR compliance pattern correctly implemented.

The code correctly logs the account deletion before anonymization (line 259), which is essential for GDPR compliance. This ensures the audit trail captures who deleted their account before PII is removed. The comment clearly documents this critical requirement.

apps/web/src/app/api/auth/mcp-tokens/route.ts (1)

43-49: LGTM! Token creation audit logging is correct.

The activity logging properly captures the token creation event after successful insertion, including all relevant security context (tokenId, tokenType, tokenName) for compliance auditing.

apps/web/src/app/api/drives/[driveId]/members/invite/route.ts (1)

185-197: LGTM! Member addition audit logging is comprehensive.

The implementation goes the extra mile by:

  • Fetching the invited user's email (lines 187-190) to enrich the audit trail
  • Logging after successful member addition
  • Capturing all relevant context (drive, target user, role)

This provides a complete audit record for member management operations.

apps/web/src/app/api/auth/mcp-tokens/[tokenId]/route.ts (2)

21-29: Excellent pattern for capturing audit context before deletion.

The code correctly fetches the token details before revocation (lines 22-25) to capture the token name for the audit log. The explicit existence check (lines 27-29) ensures we only proceed with valid tokens and prevents logging spurious revocations.


42-48: LGTM! Token revocation audit logging is correct.

The activity logging properly uses the captured token name from the earlier query and logs after successful revocation, maintaining a complete security audit trail.

apps/web/src/app/api/upload/route.ts (1)

17-17: LGTM! Audit logging correctly implemented.

The file upload audit trail is properly integrated:

  • Actor information is retrieved before logging
  • Logging occurs only after successful transaction and storage update
  • All relevant file metadata (fileId, fileName, fileType, fileSize, driveId, pageId) is captured
  • Fire-and-forget pattern maintained for non-blocking operation
  • Failed uploads are intentionally excluded from the audit trail (appropriate for success-only logging)

Also applies to: 348-357

packages/lib/src/monitoring/activity-logger.ts (5)

58-77: LGTM! Comprehensive operation type expansion.

The new ActivityOperation entries provide extensive coverage for enterprise audit requirements:

  • Logical grouping with inline comments enhances maintainability
  • Covers membership, authentication/security, file, and account operations
  • Aligns with SOX and GDPR compliance objectives stated in the PR

79-89: LGTM! Resource type expansion supports new audit requirements.

The expanded ActivityResourceType properly supports the new wrapper functions and covers all resource categories needed for comprehensive audit logging.


337-372: LGTM! Member activity wrapper properly implemented.

The logMemberActivity function correctly:

  • Constrains operations to member-specific types ('member_add', 'member_remove', 'member_role_change')
  • Captures role transitions in previousValues/newValues for audit trail
  • Stores relevant metadata (targetUserId, targetUserEmail, driveName)
  • Maintains fire-and-forget pattern with silent error handling

378-409: LGTM! Role activity wrapper correctly tracks permission changes.

The logRoleActivity function properly:

  • Constrains operations to role-specific types ('create', 'update', 'delete')
  • Captures permission transitions in previousValues/newValues
  • Maintains consistent error handling and fire-and-forget pattern

506-539: LGTM! File activity wrapper properly captures file metadata.

The logFileActivity function correctly:

  • Constrains operations to file-specific types ('upload', 'convert', 'delete')
  • Captures essential file metadata (fileType, fileSize)
  • Uses the actual driveId (not 'system' marker) since files belong to specific drives
  • Maintains consistent error handling and fire-and-forget pattern

Comment thread packages/lib/src/monitoring/activity-logger.ts
2witstudios and others added 2 commits December 21, 2025 21:45
Address PR review comment: 'system' string violates foreign key
constraint since driveId references drives.id table.

- Update ActivityLogInput interface to allow driveId: string | null
- Change logUserActivity to use driveId: null
- Change logTokenActivity to use driveId: null

This is safe because the schema already uses onDelete: 'set null'
for the driveId foreign key constraint.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add ./monitoring/activity-logger export to @pagespace/lib package.json
- Fix type errors in activity logging calls:
  - Use user?.email for nested user object in member routes
  - Cast permissions to unknown first for complex role types
  - Handle null->undefined conversion for optional fields
  - Remove non-existent previousParentId from reorder metadata

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5c610c and 584bbd2.

📒 Files selected for processing (6)
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/pages/reorder/route.ts
  • packages/lib/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/web/src/app/api/pages/reorder/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/[roleId]/route.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/src/app/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/src/app/**/*.ts: In Next.js 15 route handlers with dynamic routes, you MUST await context.params before destructuring because params are Promise objects
Get request body using const body = await request.json();
Get search params using const { searchParams } = new URL(request.url);
Return JSON responses using return Response.json(data) or return NextResponse.json(data)

Files:

  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never use any types in TypeScript code - always use proper TypeScript types

**/*.{ts,tsx}: No any types - always use proper TypeScript types
Use kebab-case for filenames (e.g., image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: import getUserAccessLevel and canUserEditPage from @pagespace/lib/permissions
Use Drizzle client from @pagespace/db for all database access
Always structure message content using the message parts structure: { parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode

Files:

  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
apps/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/**/*.{ts,tsx}: Use centralized permission functions from @pagespace/lib/permissions for access control, such as getUserAccessLevel() and canUserEditPage()
Always use Drizzle client from @pagespace/db for database access instead of direct database connections
Always use message parts structure with parts array containing objects with type and text fields when constructing messages for AI

Files:

  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
apps/web/src/app/**/route.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 dynamic routes, params are Promise objects and must be awaited before destructuring
Get request body using const body = await request.json();
Return JSON responses using Response.json(data) or NextResponse.json(data) in route handlers

Files:

  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Format code with Prettier and lint with ESLint using the configuration at apps/web/eslint.config.mjs

Files:

  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
apps/web/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching

Files:

  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
  • apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts
🧠 Learnings (3)
📚 Learning: 2025-12-14T14:54:47.122Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.122Z
Learning: Applies to **/*.{ts,tsx} : Use centralized permission logic: import `getUserAccessLevel` and `canUserEditPage` from `pagespace/lib/permissions`

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/members/route.ts
  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/**/*.{ts,tsx} : Use centralized permission functions from `pagespace/lib/permissions` for access control, such as `getUserAccessLevel()` and `canUserEditPage()`

Applied to files:

  • apps/web/src/app/api/drives/[driveId]/roles/route.ts
🧬 Code graph analysis (3)
apps/web/src/app/api/drives/[driveId]/members/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logMemberActivity (337-372)
apps/web/src/app/api/drives/[driveId]/roles/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logRoleActivity (378-409)
apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts (1)
packages/lib/src/monitoring/activity-logger.ts (2)
  • getActorInfo (24-44)
  • logMemberActivity (337-372)
⏰ 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: Unit Tests
🔇 Additional comments (6)
apps/web/src/app/api/drives/[driveId]/roles/route.ts (1)

9-9: LGTM - Import correctly added.

The activity logger imports are appropriate for the audit trail functionality being added.

packages/lib/package.json (1)

65-69: LGTM - Module export correctly configured.

The activity-logger module export is properly structured with types, import, and require paths, consistent with other monitoring module exports in the package. The typesVersions mapping is also correctly added.

Also applies to: 200-202

apps/web/src/app/api/drives/[driveId]/members/route.ts (2)

10-10: LGTM - Import correctly added.

The activity logger imports are appropriate for the member audit trail functionality.


91-98: LGTM - Member addition audit logging correctly implemented.

The audit log is properly placed after the successful member addition and includes all relevant context (driveId, driveName, targetUserId, role). The pattern is consistent with other audit logging additions across the codebase.

apps/web/src/app/api/drives/[driveId]/members/[userId]/route.ts (2)

13-13: LGTM - Import correctly added.

The activity logger imports are appropriate for the member role change audit functionality.


134-143: LGTM - Role change audit logging correctly implemented.

The audit log is properly placed within the role change conditional block and includes all relevant context including both previous and new roles. This correctly captures role changes as critical security events for compliance. The placement after notifications but within the role-change condition ensures the log is only created when an actual role change occurs.

Comment thread apps/web/src/app/api/drives/[driveId]/roles/route.ts
2witstudios and others added 3 commits December 21, 2025 22:17
Replace unsafe double-cast (as unknown as Record<string, boolean>)
with a proper summarizePermissions helper function that transforms
the complex RolePermissions structure into Record<string, boolean>.

Each page key maps to true if any permission (canView, canEdit,
canShare) is granted for that page.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The manually created migration 0023_activity_log_enums_expansion.sql
failed in CI because PostgreSQL cannot run ALTER TYPE ... ADD VALUE
inside transactions. Drizzle's --statement-breakpoint markers handle
this correctly by running each statement outside of transactions.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add missing mocks for:
- db.query.mcpTokens.findFirst (used to get token name for audit log)
- @pagespace/lib/monitoring/activity-logger (getActorInfo, logTokenActivity)

Update "token not found" tests to mock findFirst returning undefined
instead of mocking db.update with empty results.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ced3fce and 5577f20.

📒 Files selected for processing (1)
  • apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
apps/web/src/app/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/src/app/**/*.ts: In Next.js 15 route handlers with dynamic routes, you MUST await context.params before destructuring because params are Promise objects
Get request body using const body = await request.json();
Get search params using const { searchParams } = new URL(request.url);
Return JSON responses using return Response.json(data) or return NextResponse.json(data)

Files:

  • apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never use any types in TypeScript code - always use proper TypeScript types

**/*.{ts,tsx}: No any types - always use proper TypeScript types
Use kebab-case for filenames (e.g., image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: import getUserAccessLevel and canUserEditPage from @pagespace/lib/permissions
Use Drizzle client from @pagespace/db for all database access
Always structure message content using the message parts structure: { parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode

Files:

  • apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts
apps/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/**/*.{ts,tsx}: Use centralized permission functions from @pagespace/lib/permissions for access control, such as getUserAccessLevel() and canUserEditPage()
Always use Drizzle client from @pagespace/db for database access instead of direct database connections
Always use message parts structure with parts array containing objects with type and text fields when constructing messages for AI

Files:

  • apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Format code with Prettier and lint with ESLint using the configuration at apps/web/eslint.config.mjs

Files:

  • apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts
apps/web/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching

Files:

  • apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts
🧬 Code graph analysis (1)
apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts (1)
packages/db/src/index.ts (1)
  • db (20-20)
⏰ 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: Unit Tests
🔇 Additional comments (3)
apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts (3)

24-24: LGTM! Default mock value improves test setup.

The default findFirst mock returning a token object is a good practice that simplifies test setup. Tests requiring different behavior correctly override this with mockResolvedValueOnce(undefined).


499-500: LGTM! Correct mock override for not-found scenario.

Using mockResolvedValueOnce(undefined) properly simulates the token-not-found case and aligns with the updated route implementation that checks token existence via findFirst.


522-523: LGTM! Correct mock override for authorization check.

Using mockResolvedValueOnce(undefined) correctly simulates the scenario where a token doesn't match the authenticated user's ID, resulting in an expected 404 response.

Comment on lines +60 to +63
vi.mock('@pagespace/lib/monitoring/activity-logger', () => ({
getActorInfo: vi.fn().mockResolvedValue({ email: 'test@example.com' }),
logTokenActivity: vi.fn(),
}));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add test assertions to verify audit logging is called.

The activity logger is mocked but never verified in any test. Given this PR's focus on compliance audit trails, tests should assert that logTokenActivity is called with correct parameters for token creation (POST) and revocation (DELETE) operations.

🔎 Example test assertions to add

For the POST token creation test (around line 114), add:

// After the successful POST assertion
expect(logTokenActivity).toHaveBeenCalledWith({
  operation: expect.stringMatching(/token.*create/i),
  resourceId: 'new-mcp-token-id',
  userId: 'test-user-id',
  actorEmail: 'test@example.com',
  // ... other expected fields
});

For the DELETE token revocation test (around line 453), add:

// After the successful DELETE assertion
expect(logTokenActivity).toHaveBeenCalledWith({
  operation: expect.stringMatching(/token.*revoke/i),
  resourceId: 'token-123',
  userId: 'test-user-id',
  actorEmail: 'test@example.com',
  // ... other expected fields
});
🤖 Prompt for AI Agents
In apps/web/src/app/api/auth/__tests__/mcp-tokens.test.ts around lines 60–63 and
the POST test near line 114 and DELETE test near line 453, add assertions that
the mocked activity logger's logTokenActivity was invoked with the correct
parameters for token creation and revocation. After the POST success assertion,
assert logTokenActivity was called with an object containing an operation
matching /token.*create/i, the created token's resourceId ('new-mcp-token-id' or
the value returned by the test), userId 'test-user-id', actorEmail
'test@example.com', and any other expected metadata; after the DELETE success
assertion, assert logTokenActivity was called with an operation matching
/token.*revoke/i, resourceId 'token-123' (or the deleted id used in the test),
userId 'test-user-id', actorEmail 'test@example.com', etc. Ensure you reference
the mocked function exported from the activity-logger mock (e.g.,
logTokenActivity) and reset/clear its calls between tests if needed.

@2witstudios
2witstudios merged commit a3586f4 into master Dec 22, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants