Skip to content

docs: Add CSRF security audit report - #108

Merged
2witstudios merged 9 commits into
masterfrom
claude/csrf-security-audit-GejqO
Dec 21, 2025
Merged

2witstudios merged 9 commits into
masterfrom
claude/csrf-security-audit-GejqO

Conversation

@2witstudios

@2witstudios 2witstudios commented Dec 20, 2025 •

Copy link
Copy Markdown
Owner

Comprehensive security audit of PageSpace's CSRF protection mechanisms.

Key findings:

  • Strong core implementation with HMAC-SHA256 tokens and SameSite=strict
  • Routes using authenticateHybridRequest need CSRF enforcement
  • Login CSRF protection recommended
  • Origin header validation suggested for defense-in-depth

Overall security rating: STRONG with minor gaps mitigated by SameSite policy.

Summary by CodeRabbit

  • New Features

    • Added a login CSRF token endpoint and double-submit CSRF protection for login and signup; client now fetches and sends CSRF tokens for web flows.
  • Refactor

    • Authentication now applies distinct security options for read vs. write operations (different protections for GET vs. modifying requests).
  • Tests

    • Updated and added tests to cover CSRF flows and auth option behavior.
  • Documentation

    • Added a CSRF security audit report and improved security event logging for CSRF issues.

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

Comprehensive security audit of PageSpace's CSRF protection mechanisms.

Key findings:
- Strong core implementation with HMAC-SHA256 tokens and SameSite=strict
- Routes using authenticateHybridRequest need CSRF enforcement
- Login CSRF protection recommended
- Origin header validation suggested for defense-in-depth

Overall security rating: STRONG with minor gaps mitigated by SameSite policy.
P1: Add CSRF protection to authenticateHybridRequest routes
- Updated conversation API routes to use authenticateRequestWithOptions
- Added requireCSRF: true for POST, PATCH, DELETE operations
- Updated tests to use new authentication function

P2: Add Login CSRF protection to prevent login CSRF attacks
- Created /api/auth/login-csrf endpoint for pre-login CSRF tokens
- Added CSRF validation to login and signup endpoints
- Uses double-submit cookie pattern with HMAC-SHA256 signed tokens
- 5-minute token expiry for short-lived protection

Files changed:
- apps/web/src/app/api/auth/login-csrf/route.ts (new)
- apps/web/src/app/api/auth/login/route.ts
- apps/web/src/app/api/auth/signup/route.ts
- apps/web/src/app/api/ai/page-agents/.../route.ts (multiple)
- Updated corresponding test files
- docs/security/csrf-audit-report.md (status update)
Add login CSRF token integration to the frontend:
- useAuth hook: Fetch CSRF token from /api/auth/login-csrf before login
- Signup page: Fetch CSRF token on mount and include in signup request
- Both include X-Login-CSRF-Token header for double-submit validation

Update tests for CSRF validation:
- Add mock for validateLoginCSRFToken in all login/signup tests
- Add CSRF headers to test requests using helper functions
- All 172 auth tests passing

This completes P2 (Login CSRF protection) from the CSRF security audit.
Add input validation to generateCSRFToken to reject empty, whitespace-only,
null, undefined, or non-string sessionId values immediately instead of
producing unusable tokens that will never validate.

This prevents a subtle bug where generateCSRFToken would accept empty
strings and produce tokens that validateCSRFToken always rejects.
@coderabbitai

coderabbitai Bot commented Dec 20, 2025 •

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@2witstudios has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 48 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 8020196 and 44d624b.

📒 Files selected for processing (4)
  • apps/web/src/app/api/auth/__tests__/login.test.ts (23 hunks)
  • apps/web/src/app/api/auth/login/route.ts (2 hunks)
  • packages/lib/src/__tests__/csrf-integration.test.ts (1 hunks)
  • packages/lib/src/__tests__/csrf-utils.test.ts (1 hunks)

Walkthrough

Renamed an auth helper to authenticateRequestWithOptions with per-operation AUTH_OPTIONS, added login CSRF generation/validation, wired CSRF checks into login/signup routes and client flows, updated tests and docs, and added input validation for CSRF utilities.

Changes

Cohort / File(s) Summary
Auth helper rename & route auth options
apps/web/src/app/api/ai/chat/messages/route.ts, apps/web/src/app/api/ai/page-agents/.../conversations/.../messages/route.ts, apps/web/src/app/api/ai/page-agents/.../conversations/.../route.ts, apps/web/src/app/api/ai/page-agents/.../conversations/route.ts
Replaced authenticateHybridRequest with authenticateRequestWithOptions(request, AUTH_OPTIONS_*); added AUTH_OPTIONS_READ and AUTH_OPTIONS_WRITE constants and applied them per HTTP method.
Test updates for auth rename
apps/web/src/app/api/ai/chat/messages/__tests__/route.test.ts, apps/web/src/app/api/ai/page-agents/.../conversations/[conversationId]/__tests__/route.test.ts, apps/web/src/app/api/ai/page-agents/.../conversations/__tests__/route.test.ts
Updated imports/mocks/usages to reference authenticateRequestWithOptions; mock behavior adapted to new symbol only.
Login CSRF endpoint
apps/web/src/app/api/auth/login-csrf/route.ts
New GET handler that generates a login CSRF token, sets httpOnly SameSite=Strict cookie (path /api/auth), and returns the token in JSON with appropriate cache and cookie attributes.
CSRF integration in login & signup routes
apps/web/src/app/api/auth/login/route.ts, apps/web/src/app/api/auth/signup/route.ts
Added double-submit CSRF checks (header X-Login-CSRF-Token vs login_csrf cookie), cookie parsing, early client IP extraction, security-event logging, and 403 responses for missing/mismatched/invalid tokens before continuing auth logic.
Client-side auth changes
apps/web/src/app/auth/signup/page.tsx, apps/web/src/hooks/use-auth.ts
Fetch login CSRF token from /api/auth/login-csrf on mount (web), store it, and include X-Login-CSRF-Token header in login/signup requests when available.
CSRF utility additions & validation
apps/web/src/lib/auth/login-csrf-utils.ts, packages/lib/src/auth/csrf-utils.ts, packages/lib/src/__tests__/csrf-utils.test.ts
Added new login-csrf-utils module (generate/validate tokens, constants), added input validation in generateCSRFToken() to reject empty/invalid sessionId, and added tests for invalid sessionId cases.
Auth test helpers & CSRF test mocks
apps/web/src/app/api/auth/__tests__/login.test.ts, apps/web/src/app/api/auth/__tests__/login-redirect.test.ts, apps/web/src/app/api/auth/__tests__/signup.test.ts, apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
Added cookie.parse and validateLoginCSRFToken mocks, introduced createLoginRequest/createSignupRequest helpers that include CSRF header and cookie, updated tests to use helpers and validate CSRF flows.
Test setup & logger types
apps/web/src/test/setup.ts, packages/lib/src/logging/logger-config.ts
Added test env vars (CSRF_SECRET, REALTIME_BROADCAST_SECRET, ENCRYPTION_KEY); expanded logSecurityEvent allowed event literals to include CSRF-related events.
Misc test change
apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx, apps/web/src/hooks/__tests__/use-auth.test.ts
Adjusted Streamdown mock to use React.createElement and updated use-auth test to expect CSRF fetch followed by login fetch (two sequential fetches).
Documentation
docs/security/csrf-audit-report.md
New CSRF security audit report documenting findings, remediation priorities, token design notes, and suggested code/testing changes.

Sequence Diagram

sequenceDiagram
    participant Client
    participant CSRF as /api/auth/login-csrf
    participant Auth as /api/auth/login
    Note over Client,CSRF: CSRF token acquisition
    Client->>CSRF: GET /api/auth/login-csrf
    CSRF->>CSRF: generateLoginCSRFToken()
    CSRF-->>Client: 200 {csrfToken} + Set-Cookie: login_csrf (httpOnly, SameSite=Strict)
    Note over Client,Auth: Login with double-submit CSRF
    Client->>Auth: POST /api/auth/login (body, header X-Login-CSRF-Token, cookie login_csrf)
    Auth->>Auth: parse cookies & read header
    alt missing header or cookie
        Auth->>Auth: logSecurityEvent(login_csrf_missing)
        Auth-->>Client: 403 LOGIN_CSRF_MISSING
    else header != cookie
        Auth->>Auth: logSecurityEvent(login_csrf_mismatch)
        Auth-->>Client: 403 LOGIN_CSRF_MISMATCH
    else validate token signature/timestamp
        Auth->>Auth: validateLoginCSRFToken()
        alt invalid/expired
            Auth->>Auth: logSecurityEvent(login_csrf_invalid)
            Auth-->>Client: 403 LOGIN_CSRF_INVALID
        else valid
            Auth->>Auth: proceed with authentication flow
            Auth-->>Client: 200 auth success
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Attention areas:
    • CSRF token generation/validation code (HMAC, secret selection, timing-safe compare).
    • Early CSRF checks in login/signup handlers and their error/logging paths.
    • Consistency of test helpers/mocks for CSRF across many tests.
    • Correct application of AUTH_OPTIONS_READ/WRITE to all modified routes.

Possibly related PRs

Poem

🐰 I fetched a token in morning light,
Hid it in cookies, kept it tight,
HMAC sings and timestamps rhyme,
Double-submit protects each sign.
Hop, secure, the login’s right—PageSpace sleeps tonight.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% 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 'docs: Add CSRF security audit report' directly and accurately describes the main change—adding comprehensive security audit documentation. The title is concise, specific, and clearly identifies the primary modification.

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: 0

🧹 Nitpick comments (6)
apps/web/src/app/api/auth/login-csrf/route.ts (1)

13-19: Consider using a dedicated CSRF secret without fallback.

The fallback to JWT_SECRET works but using the same secret for both JWT signing and CSRF tokens reduces defense-in-depth. If either secret is compromised, both systems are affected.

🔎 Suggested change
 function getLoginCSRFSecret(): string {
-  const secret = process.env.CSRF_SECRET || process.env.JWT_SECRET;
-  if (!secret || secret.length < 32) {
-    throw new Error('CSRF_SECRET or JWT_SECRET must be at least 32 characters');
+  const secret = process.env.CSRF_SECRET;
+  if (!secret || secret.length < 32) {
+    throw new Error('CSRF_SECRET must be at least 32 characters');
   }
   return secret;
 }
apps/web/src/hooks/use-auth.ts (1)

79-92: Consider optimizing CSRF token fetch for desktop platform.

The login CSRF token is fetched unconditionally on every login attempt, even for desktop platforms that use Bearer token authentication (which is CSRF-exempt). While this doesn't cause functional issues, it creates unnecessary network requests for desktop users.

🔎 Proposed optimization
  const login = useCallback(async (email: string, password: string) => {
    setLoading(true);
    try {
+     const isDesktop = typeof window !== 'undefined' && window.electron?.isDesktop;
+
      // Fetch login CSRF token first (prevents Login CSRF attacks)
+     // Only needed for web platform (desktop uses Bearer tokens which are CSRF-exempt)
      let loginCsrfToken: string | null = null;
-     try {
+     if (!isDesktop) {
+       try {
-       const csrfResponse = await fetch('/api/auth/login-csrf', {
-         credentials: 'include',
-       });
-       if (csrfResponse.ok) {
-         const csrfData = await csrfResponse.json();
-         loginCsrfToken = csrfData.csrfToken;
+         const csrfResponse = await fetch('/api/auth/login-csrf', {
+           credentials: 'include',
+         });
+         if (csrfResponse.ok) {
+           const csrfData = await csrfResponse.json();
+           loginCsrfToken = csrfData.csrfToken;
+         }
+       } catch (csrfError) {
+         console.error('Failed to fetch login CSRF token:', csrfError);
+         // Continue without CSRF token - server will reject if required
        }
-     } catch (csrfError) {
-       console.error('Failed to fetch login CSRF token:', csrfError);
-       // Continue without CSRF token - server will reject if required
      }

-     const isDesktop = typeof window !== 'undefined' && window.electron?.isDesktop;
-
      if (isDesktop && window.electron) {
apps/web/src/app/api/auth/__tests__/signup.test.ts (1)

138-153: Good helper pattern, but consider adding negative CSRF test cases.

The createSignupRequest helper is well-designed and reduces duplication. However, this test suite now only tests the happy path where CSRF validation passes.

Consider adding test cases for CSRF validation failures:

  • Missing CSRF header or cookie
  • Mismatched header/cookie values
  • Invalid/expired token

These would ensure the production CSRF validation code paths are exercised.

🔎 Example test cases to add
describe('CSRF validation', () => {
  it('returns 403 when CSRF header is missing', async () => {
    const request = new Request('http://localhost/api/auth/signup', {
      method: 'POST',
      headers: {
        'Content-Type': 'application/json',
        'Cookie': 'login_csrf=valid-csrf-token',
        // No X-Login-CSRF-Token header
      },
      body: JSON.stringify(validSignupPayload),
    });

    const response = await POST(request);
    expect(response.status).toBe(403);
  });

  it('returns 403 when CSRF tokens mismatch', async () => {
    const request = new Request('http://localhost/api/auth/signup', {
      method: 'POST',
      headers: {
        'Content-Type': 'application/json',
        'X-Login-CSRF-Token': 'token-a',
        'Cookie': 'login_csrf=token-b',
      },
      body: JSON.stringify(validSignupPayload),
    });

    const response = await POST(request);
    expect(response.status).toBe(403);
  });
});
docs/security/csrf-audit-report.md (1)

91-108: Document references outdated function name.

The remediation example still references authenticateHybridRequest, but based on the PR changes, this has been renamed to authenticateRequestWithOptions. Consider updating for consistency with the actual implementation.

🔎 Suggested update
 **Evidence:**
 ```typescript
-// apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts:113
-const auth = await authenticateHybridRequest(request);
-// No CSRF validation - authenticateHybridRequest defaults to requireCSRF: false
+// apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts:113
+const auth = await authenticateRequestWithOptions(request, { allow: ['jwt', 'mcp'] });
+// CSRF not required by default - set requireCSRF: true for mutations
</details>

</blockquote></details>
<details>
<summary>apps/web/src/app/api/auth/signup/route.ts (1)</summary><blockquote>

`48-96`: **Consider extracting shared CSRF validation logic.**

The CSRF validation logic in lines 48-96 is duplicated almost identically in `login/route.ts`. Consider extracting this into a shared utility function to improve maintainability and ensure consistent behavior across auth endpoints.


<details>
<summary>🔎 Suggested refactor</summary>

Create a shared utility:

```typescript
// apps/web/src/lib/auth/login-csrf-validation.ts
import { parse } from 'cookie';
import { logSecurityEvent } from '@pagespace/lib/server';
import { validateLoginCSRFToken } from '@/app/api/auth/login-csrf/route';

export interface CSRFValidationResult {
  valid: boolean;
  error?: Response;
}

export function validateLoginCSRF(req: Request, clientIP: string): CSRFValidationResult {
  const csrfTokenHeader = req.headers.get('x-login-csrf-token');
  const cookieHeader = req.headers.get('cookie');
  const cookies = parse(cookieHeader || '');
  const csrfTokenCookie = cookies.login_csrf;

  if (!csrfTokenHeader || !csrfTokenCookie) {
    logSecurityEvent('login_csrf_missing', {
      ip: clientIP,
      hasHeader: !!csrfTokenHeader,
      hasCookie: !!csrfTokenCookie,
    });
    return {
      valid: false,
      error: Response.json(
        { error: 'Login CSRF token required', code: 'LOGIN_CSRF_MISSING', details: 'Please refresh the page and try again' },
        { status: 403 }
      ),
    };
  }

  // ... remaining validation logic
}

Then use in both routes:

const csrfResult = validateLoginCSRF(req, clientIP);
if (!csrfResult.valid) return csrfResult.error;
apps/web/src/app/api/auth/login/route.ts (1)

35-88: Duplicated CSRF logic confirms need for shared utility.

As noted in the signup route review, this CSRF validation block (lines 40-88) is nearly identical to the signup implementation. Extracting to a shared utility would:

  • Reduce code duplication (~50 lines)
  • Ensure consistent error messages and logging
  • Simplify future maintenance

This is the same refactor recommendation as the signup route.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc9ef93 and 2862ddc.

📒 Files selected for processing (19)
  • apps/web/src/app/api/ai/chat/messages/__tests__/route.test.ts (4 hunks)
  • apps/web/src/app/api/ai/chat/messages/route.ts (1 hunks)
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/__tests__/route.test.ts (6 hunks)
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts (2 hunks)
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/route.ts (3 hunks)
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/__tests__/route.test.ts (6 hunks)
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts (4 hunks)
  • apps/web/src/app/api/auth/__tests__/login-redirect.test.ts (4 hunks)
  • apps/web/src/app/api/auth/__tests__/login.test.ts (26 hunks)
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts (4 hunks)
  • apps/web/src/app/api/auth/__tests__/signup.test.ts (30 hunks)
  • apps/web/src/app/api/auth/login-csrf/route.ts (1 hunks)
  • apps/web/src/app/api/auth/login/route.ts (2 hunks)
  • apps/web/src/app/api/auth/signup/route.ts (2 hunks)
  • apps/web/src/app/auth/signup/page.tsx (3 hunks)
  • apps/web/src/hooks/use-auth.ts (2 hunks)
  • docs/security/csrf-audit-report.md (1 hunks)
  • packages/lib/src/__tests__/csrf-utils.test.ts (1 hunks)
  • packages/lib/src/auth/csrf-utils.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/auth/signup/page.tsx
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/__tests__/route.test.ts
  • packages/lib/src/__tests__/csrf-utils.test.ts
  • apps/web/src/app/api/auth/__tests__/login-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts
  • apps/web/src/hooks/use-auth.ts
  • apps/web/src/app/api/auth/__tests__/login.test.ts
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/login-csrf/route.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/__tests__/route.test.ts
  • packages/lib/src/auth/csrf-utils.ts
  • apps/web/src/app/api/ai/chat/messages/__tests__/route.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/route.ts
  • apps/web/src/app/api/ai/chat/messages/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/auth/signup/page.tsx
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/__tests__/route.test.ts
  • apps/web/src/app/api/auth/__tests__/login-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts
  • apps/web/src/hooks/use-auth.ts
  • apps/web/src/app/api/auth/__tests__/login.test.ts
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/login-csrf/route.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/__tests__/route.test.ts
  • apps/web/src/app/api/ai/chat/messages/__tests__/route.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/route.ts
  • apps/web/src/app/api/ai/chat/messages/route.ts
apps/web/src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/src/**/*.tsx: For document editing, register editing state using useEditingStore.getState().startEditing() and endEditing() to prevent unwanted UI refreshes
For AI streaming operations, register streaming state using useEditingStore.getState().startStreaming() and endStreaming() to prevent unwanted UI refreshes
When using SWR, check useEditingStore state with isAnyActive() and set isPaused to prevent data refreshes during editing or streaming

Files:

  • apps/web/src/app/auth/signup/page.tsx
**/*.{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/auth/signup/page.tsx
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/__tests__/route.test.ts
  • packages/lib/src/__tests__/csrf-utils.test.ts
  • apps/web/src/app/api/auth/__tests__/login-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts
  • apps/web/src/hooks/use-auth.ts
  • apps/web/src/app/api/auth/__tests__/login.test.ts
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/login-csrf/route.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/__tests__/route.test.ts
  • packages/lib/src/auth/csrf-utils.ts
  • apps/web/src/app/api/ai/chat/messages/__tests__/route.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/route.ts
  • apps/web/src/app/api/ai/chat/messages/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/auth/signup/page.tsx
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/__tests__/route.test.ts
  • apps/web/src/app/api/auth/__tests__/login-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts
  • apps/web/src/hooks/use-auth.ts
  • apps/web/src/app/api/auth/__tests__/login.test.ts
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/login-csrf/route.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/__tests__/route.test.ts
  • apps/web/src/app/api/ai/chat/messages/__tests__/route.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/route.ts
  • apps/web/src/app/api/ai/chat/messages/route.ts
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__/signup-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/__tests__/route.test.ts
  • apps/web/src/app/api/auth/__tests__/login-redirect.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts
  • apps/web/src/app/api/auth/__tests__/login.test.ts
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/login-csrf/route.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/__tests__/route.test.ts
  • apps/web/src/app/api/ai/chat/messages/__tests__/route.test.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/route.ts
  • apps/web/src/app/api/ai/chat/messages/route.ts
packages/lib/**/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

Write unit tests for packages/lib and apps/processor with *.test.ts files next to source or in __tests__/ directories

Files:

  • packages/lib/src/__tests__/csrf-utils.test.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/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts
  • apps/web/src/app/api/auth/login-csrf/route.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/route.ts
  • apps/web/src/app/api/ai/chat/messages/route.ts
🧠 Learnings (17)
📚 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 apps/web/src/**/*.{ts,tsx} : Use SWR for server state and caching

Applied to files:

  • apps/web/src/app/auth/signup/page.tsx
📚 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: Use Zustand for client state management and SWR for server state and caching

Applied to files:

  • apps/web/src/app/auth/signup/page.tsx
📚 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 apps/web/src/**/*.{ts,tsx} : Use Zustand for client-side state management

Applied to files:

  • apps/web/src/app/auth/signup/page.tsx
📚 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: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui

Applied to files:

  • apps/web/src/app/auth/signup/page.tsx
📚 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/web/src/app/**/*.ts : Get request body using `const body = await request.json();`

Applied to files:

  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/auth/__tests__/login-redirect.test.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 packages/lib/**/*.test.ts : Write unit tests for shared utilities in `packages/lib` with test files named `*.test.ts` alongside source or in `__tests__/` directory

Applied to files:

  • packages/lib/src/__tests__/csrf-utils.test.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 packages/lib/**/*.test.ts : Write unit tests for `packages/lib` and `apps/processor` with `*.test.ts` files next to source or in `__tests__/` directories

Applied to files:

  • packages/lib/src/__tests__/csrf-utils.test.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 apps/processor/**/*.test.ts : Write unit tests for the processor service with test files named `*.test.ts` alongside source or in `__tests__/` directory

Applied to files:

  • packages/lib/src/__tests__/csrf-utils.test.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 **/__tests__/**/*.test.ts : Unit tests should be placed next to source files or in `__tests__/` directories with `*.test.ts` extension. Add a `test` script to the package and run with `pnpm --filter <pkg> test`

Applied to files:

  • packages/lib/src/__tests__/csrf-utils.test.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/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts
  • apps/web/src/app/api/ai/chat/messages/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/auth/signup/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/auth/signup/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 centralized permission logic: import `getUserAccessLevel` and `canUserEditPage` from `pagespace/lib/permissions`

Applied to files:

  • apps/web/src/app/api/auth/signup/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/auth/signup/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/auth/signup/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/auth/signup/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 apps/web/src/app/**/route.{ts,tsx} : Get request body using `const body = await request.json();`

Applied to files:

  • apps/web/src/app/api/ai/chat/messages/route.ts
🧬 Code graph analysis (10)
apps/web/src/app/auth/signup/page.tsx (2)
apps/web/src/lib/auth/auth-fetch.ts (1)
  • fetch (44-211)
apps/processor/src/logger.ts (1)
  • error (57-63)
apps/web/src/app/api/ai/page-agents/[agentId]/conversations/__tests__/route.test.ts (1)
apps/web/src/lib/auth/index.ts (1)
  • authenticateRequestWithOptions (216-271)
packages/lib/src/__tests__/csrf-utils.test.ts (1)
packages/lib/src/auth/csrf-utils.ts (1)
  • generateCSRFToken (18-34)
apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts (1)
apps/web/src/lib/auth/index.ts (1)
  • authenticateRequestWithOptions (216-271)
apps/web/src/hooks/use-auth.ts (1)
apps/web/src/lib/auth/auth-fetch.ts (1)
  • fetch (44-211)
apps/web/src/app/api/auth/signup/route.ts (1)
apps/web/src/app/api/auth/login-csrf/route.ts (1)
  • validateLoginCSRFToken (42-75)
apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/__tests__/route.test.ts (1)
apps/web/src/lib/auth/index.ts (1)
  • authenticateRequestWithOptions (216-271)
apps/web/src/app/api/ai/chat/messages/__tests__/route.test.ts (1)
apps/web/src/lib/auth/index.ts (1)
  • authenticateRequestWithOptions (216-271)
apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts (1)
apps/web/src/lib/auth/index.ts (1)
  • authenticateRequestWithOptions (216-271)
apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/route.ts (1)
apps/web/src/lib/auth/index.ts (1)
  • authenticateRequestWithOptions (216-271)
🪛 markdownlint-cli2 (0.18.1)
docs/security/csrf-audit-report.md

18-18: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ 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 (29)
packages/lib/src/auth/csrf-utils.ts (1)

19-22: LGTM! Good defensive input validation.

The upfront validation for sessionId properly guards against empty strings, non-string types, and whitespace-only inputs. This prevents generating tokens that would be bound to invalid session identifiers.

packages/lib/src/__tests__/csrf-utils.test.ts (1)

73-90: LGTM! Comprehensive test coverage for input validation.

The new tests thoroughly cover edge cases for invalid sessionId inputs including empty strings, whitespace-only, null/undefined, and non-string types. The as any casts are appropriate for testing runtime behavior with TypeScript type violations.

apps/web/src/app/api/auth/login-csrf/route.ts (1)

89-112: LGTM! Secure CSRF token endpoint implementation.

The GET handler correctly implements the double-submit cookie pattern with appropriate security measures: httpOnly cookie, SameSite=strict, secure flag in production, cache prevention headers, and scoped cookie path.

apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/messages/route.ts (2)

8-9: LGTM! Correct CSRF policy for read operations.

Read-only GET endpoints don't require CSRF protection since browsers with SameSite=strict cookies don't attach credentials to cross-origin requests. The requireCSRF: false setting is appropriate here.


50-51: LGTM! Auth pattern correctly applied.

The migration to authenticateRequestWithOptions with explicit options provides clear documentation of the endpoint's security requirements.

apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/route.ts (2)

7-8: LGTM! Correct CSRF enforcement for write operations.

PATCH and DELETE are state-changing operations that correctly require CSRF validation. The requireCSRF: true setting ensures protection against cross-site request forgery attacks on cookie-based authentication.


21-22: LGTM! Consistent auth pattern across write handlers.

Both PATCH and DELETE handlers correctly use AUTH_OPTIONS_WRITE ensuring uniform CSRF protection for all state-changing operations in this route.

Also applies to: 87-88

apps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.ts (2)

12-14: LGTM! Proper separation of read/write authentication options.

The auth options constants correctly distinguish between read-only operations (GET, no CSRF required) and write operations (POST, CSRF required). This aligns with CSRF best practices where state-changing operations need additional protection.


27-27: Correct CSRF enforcement per operation type.

GET requests use AUTH_OPTIONS_READ (CSRF-exempt for read-only) and POST requests use AUTH_OPTIONS_WRITE (CSRF-required for state changes). This properly protects write operations while avoiding unnecessary overhead on reads.

Also applies to: 117-117

apps/web/src/app/api/ai/chat/messages/route.ts (1)

8-9: LGTM! Read-only endpoint correctly configured.

The GET handler appropriately uses AUTH_OPTIONS_READ with CSRF disabled, as this endpoint only retrieves data without making state changes.

Also applies to: 17-17

apps/web/src/hooks/use-auth.ts (1)

171-174: LGTM! Proper conditional CSRF header injection.

The login request correctly includes the CSRF token header only when available, and the conditional spread syntax cleanly handles the optional header.

apps/web/src/app/auth/signup/page.tsx (2)

29-45: LGTM! Proper Login CSRF protection for signup flow.

Fetching the login CSRF token from /api/auth/login-csrf for the signup form is correct. This protects against Login CSRF attacks where an attacker tricks a victim into signing up or logging into the attacker's account. The error handling appropriately continues without the token, allowing the server to enforce validation.


82-82: Clean conditional header injection.

The spread operator syntax cleanly handles the optional CSRF header, maintaining good code readability.

apps/web/src/app/api/auth/__tests__/login-redirect.test.ts (2)

52-58: LGTM! Proper CSRF validation mocks.

The test setup correctly mocks both cookie parsing and CSRF validation to support the new login CSRF flow.


120-124: Test requests correctly include CSRF authentication.

All test requests now include both the CSRF header (X-Login-CSRF-Token) and cookie (login_csrf), matching the expected authentication flow.

apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts (1)

66-72: LGTM! Consistent CSRF test setup.

The signup tests follow the same CSRF mocking pattern as the login tests, ensuring consistent test coverage across authentication flows.

Also applies to: 156-157

apps/web/src/app/api/ai/page-agents/[agentId]/conversations/__tests__/route.test.ts (1)

36-38: LGTM! Test mocks updated for renamed authentication function.

The test correctly updates imports and mocks to reference authenticateRequestWithOptions instead of the legacy authenticateHybridRequest, maintaining test coverage for the refactored authentication approach.

Also applies to: 57-57

apps/web/src/app/api/ai/chat/messages/__tests__/route.test.ts (1)

23-25: LGTM! Consistent test updates for authentication refactor.

Tests properly updated to use the renamed authenticateRequestWithOptions function, maintaining alignment with the production code changes.

Also applies to: 48-48

apps/web/src/app/api/auth/__tests__/signup.test.ts (1)

91-99: LGTM! Clean CSRF mocking setup.

The mock setup correctly configures cookie parsing and CSRF validation to return valid tokens, enabling the existing tests to pass through the new CSRF checks.

docs/security/csrf-audit-report.md (2)

1-6: Well-structured security audit document.

The report provides a comprehensive analysis of CSRF protection mechanisms with clear findings, remediation priorities, and code examples. The overall structure follows security audit best practices.


306-317: The documentation section is still valid and accurate. The function authenticateHybridRequest continues to exist in apps/web/src/lib/auth/index.ts (line 200) and is not a renamed version—authenticateRequestWithOptions is a separate function that both exist alongside each other. Section 5.2's proposal to update authenticateHybridRequest with requireCSRF: true remains applicable and doesn't require revision or removal.

Likely an incorrect or invalid review comment.

apps/web/src/app/api/auth/__tests__/login.test.ts (2)

64-72: Consistent CSRF mock setup with signup tests.

The mock configuration correctly mirrors the signup test setup, ensuring consistency across auth test suites.


102-117: LGTM! Request helper reduces test boilerplate.

The createLoginRequest helper follows the same pattern as createSignupRequest, providing a clean way to inject CSRF tokens and additional headers. This improves test maintainability.

Same recommendation as signup tests: consider adding negative CSRF validation test cases for comprehensive coverage.

apps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/__tests__/route.test.ts (2)

25-28: LGTM! Auth mock correctly renamed.

The mock declaration properly reflects the renamed authentication function from authenticateHybridRequest to authenticateRequestWithOptions.


41-43: Import updated consistently with mock.

The import statement correctly references the renamed authenticateRequestWithOptions function, maintaining alignment with the mock declaration.

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

6-15: LGTM! Necessary imports for CSRF protection.

The new imports correctly bring in the required utilities for CSRF validation: logSecurityEvent for audit logging, parse for cookie parsing, and validateLoginCSRFToken for token validation.


48-96: Solid CSRF validation implementation with double-submit pattern.

The implementation correctly:

  1. Validates presence of both header and cookie tokens
  2. Verifies tokens match (double-submit pattern)
  3. Validates token signature and expiry via validateLoginCSRFToken
  4. Logs security events for all failure cases with appropriate context
  5. Returns consistent 403 responses with descriptive error codes

The validation runs before request body parsing, which is the correct order for fail-fast behavior.

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

15-20: LGTM! Required imports for CSRF validation.

The imports mirror the signup route, bringing in parse, logSecurityEvent, and validateLoginCSRFToken for the CSRF protection implementation.


35-88: CSRF validation correctly implemented for login route.

The implementation:

  1. Extracts clientIP early for logging (lines 36-38)
  2. Validates CSRF tokens using the double-submit pattern
  3. Logs security events for all failure scenarios
  4. Returns consistent error responses with actionable details

The validation runs before rate limiting and body parsing, ensuring attacks are rejected early without consuming resources.

- Move validateLoginCSRFToken and generateLoginCSRFToken to
  apps/web/src/lib/auth/login-csrf-utils.ts to fix Next.js 15
  route export restrictions (routes can only export handlers)
- Update login/signup routes to import from new utility location
- Update test mocks to use new import path
- Add login CSRF event types to logSecurityEvent type union
- Add missing environment variables to test setup (CSRF_SECRET,
  REALTIME_BROADCAST_SECRET, ENCRYPTION_KEY)
- Fix React import in StreamingMarkdown test mock
- Update use-auth test to account for CSRF token prefetch call
  (login now makes 2 fetch calls: CSRF token + login)

All 2068 tests now pass.

@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: 0

🧹 Nitpick comments (3)
apps/web/src/app/api/auth/login/route.ts (1)

35-88: CSRF protection correctly implemented.

The CSRF validation follows security best practices:

  • Double-submit token pattern (header + cookie match)
  • Timing-safe signature validation
  • Proper security event logging
  • Early validation before expensive operations

However, note significant code duplication with the signup route (lines 48-96 in signup/route.ts). Consider extracting this into a shared middleware or utility function.

🔎 Suggested refactor to reduce duplication

Create a shared CSRF validation function in login-csrf-utils.ts:

+/**
+ * Validates CSRF token from request headers and cookies
+ * Returns error response if validation fails, null if valid
+ */
+export function validateCSRFFromRequest(
+  req: Request,
+  clientIP: string,
+  eventPrefix: 'login' | 'signup'
+): Response | null {
+  const csrfTokenHeader = req.headers.get('x-login-csrf-token');
+  const cookieHeader = req.headers.get('cookie');
+  const cookies = parse(cookieHeader || '');
+  const csrfTokenCookie = cookies.login_csrf;
+
+  if (!csrfTokenHeader || !csrfTokenCookie) {
+    logSecurityEvent(`${eventPrefix}_csrf_missing` as any, {
+      ip: clientIP,
+      hasHeader: !!csrfTokenHeader,
+      hasCookie: !!csrfTokenCookie,
+    });
+    return Response.json(
+      { error: 'Login CSRF token required', code: 'LOGIN_CSRF_MISSING', details: 'Please refresh the page and try again' },
+      { status: 403 }
+    );
+  }
+
+  if (csrfTokenHeader !== csrfTokenCookie) {
+    logSecurityEvent(`${eventPrefix}_csrf_mismatch` as any, { ip: clientIP });
+    return Response.json(
+      { error: 'Invalid login CSRF token', code: 'LOGIN_CSRF_MISMATCH', details: 'Please refresh the page and try again' },
+      { status: 403 }
+    );
+  }
+
+  if (!validateLoginCSRFToken(csrfTokenHeader)) {
+    logSecurityEvent(`${eventPrefix}_csrf_invalid` as any, { ip: clientIP });
+    return Response.json(
+      { error: 'Invalid or expired login CSRF token', code: 'LOGIN_CSRF_INVALID', details: 'Please refresh the page and try again' },
+      { status: 403 }
+    );
+  }
+
+  return null;
+}

Then use it in both routes:

-    // Validate Login CSRF token...
-    const csrfTokenHeader = req.headers.get('x-login-csrf-token');
-    // ... (entire validation block)
+    const csrfError = validateCSRFFromRequest(req, clientIP, 'login');
+    if (csrfError) return csrfError;
apps/web/src/app/api/auth/signup/route.ts (1)

48-96: CSRF protection correctly implemented.

The validation logic is sound and follows the same secure pattern as the login route. Security logging is consistent.

However, this code is duplicated from login/route.ts (lines 40-88). This violates the DRY principle and creates maintenance burden. See the refactoring suggestion in the login route review to extract this into a shared function.

apps/web/src/lib/auth/login-csrf-utils.ts (1)

11-17: Secret fallback requires documentation.

The fallback from CSRF_SECRET to JWT_SECRET should be documented to explain when this is appropriate. While both secrets should be high-entropy, explicitly documenting the rationale helps maintainers understand the security model.

📝 Suggested documentation enhancement
 /**
  * Gets the CSRF secret, with fallback to JWT_SECRET for convenience
+ * 
+ * CSRF_SECRET is preferred for defense-in-depth. JWT_SECRET is used as
+ * fallback to simplify deployment where both secrets have equivalent entropy.
+ * Both must be at least 32 characters for HMAC-SHA256 security.
  */
 function getLoginCSRFSecret(): string {
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2862ddc and 8020196.

📒 Files selected for processing (12)
  • apps/web/src/app/api/auth/__tests__/login-redirect.test.ts (4 hunks)
  • apps/web/src/app/api/auth/__tests__/login.test.ts (26 hunks)
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts (4 hunks)
  • apps/web/src/app/api/auth/__tests__/signup.test.ts (30 hunks)
  • apps/web/src/app/api/auth/login-csrf/route.ts (1 hunks)
  • apps/web/src/app/api/auth/login/route.ts (2 hunks)
  • apps/web/src/app/api/auth/signup/route.ts (2 hunks)
  • apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx (1 hunks)
  • apps/web/src/hooks/__tests__/use-auth.test.ts (2 hunks)
  • apps/web/src/lib/auth/login-csrf-utils.ts (1 hunks)
  • apps/web/src/test/setup.ts (1 hunks)
  • packages/lib/src/logging/logger-config.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/web/src/app/api/auth/tests/login-redirect.test.ts
  • apps/web/src/app/api/auth/login-csrf/route.ts
  • apps/web/src/app/api/auth/tests/login.test.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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:

  • packages/lib/src/logging/logger-config.ts
  • apps/web/src/test/setup.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/lib/auth/login-csrf-utils.ts
  • apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/hooks/__tests__/use-auth.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:

  • packages/lib/src/logging/logger-config.ts
  • apps/web/src/test/setup.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/lib/auth/login-csrf-utils.ts
  • apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/hooks/__tests__/use-auth.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/test/setup.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/lib/auth/login-csrf-utils.ts
  • apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/hooks/__tests__/use-auth.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/test/setup.ts
  • apps/web/src/app/api/auth/signup/route.ts
  • apps/web/src/lib/auth/login-csrf-utils.ts
  • apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/auth/login/route.ts
  • apps/web/src/hooks/__tests__/use-auth.test.ts
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/signup/route.ts
  • apps/web/src/app/api/auth/__tests__/signup.test.ts
  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts
  • apps/web/src/app/api/auth/login/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/auth/signup/route.ts
  • apps/web/src/app/api/auth/login/route.ts
apps/web/src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/src/**/*.tsx: For document editing, register editing state using useEditingStore.getState().startEditing() and endEditing() to prevent unwanted UI refreshes
For AI streaming operations, register streaming state using useEditingStore.getState().startStreaming() and endStreaming() to prevent unwanted UI refreshes
When using SWR, check useEditingStore state with isAnyActive() and set isPaused to prevent data refreshes during editing or streaming

Files:

  • apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx
**/{components,src/**/components}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase for React component names and filenames

Files:

  • apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx
🧠 Learnings (14)
📚 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 .env* : Always include critical environment variables in `.env.example`: `DATABASE_URL`, encryption keys, `WEB_APP_URL`, `NEXT_PUBLIC_*` variables, and service ports; never commit actual secrets

Applied to files:

  • apps/web/src/test/setup.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: Never commit secrets; base configuration should be in `.env.example` with runtime values in `.env`

Applied to files:

  • apps/web/src/test/setup.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: Never commit secrets. Base configuration in `.env.example`, runtime configuration in `.env`

Applied to files:

  • apps/web/src/test/setup.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: Never commit secrets to version control. Base configuration in `.env.example`; runtime configuration in `.env`

Applied to files:

  • apps/web/src/test/setup.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/auth/signup/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/auth/signup/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 centralized permission logic: import `getUserAccessLevel` and `canUserEditPage` from `pagespace/lib/permissions`

Applied to files:

  • apps/web/src/app/api/auth/signup/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/auth/signup/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/auth/signup/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/auth/signup/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/web/src/**/*.tsx : For AI streaming operations, register streaming state using `useEditingStore.getState().startStreaming()` and `endStreaming()` to prevent unwanted UI refreshes

Applied to files:

  • apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx
📚 Learning: 2025-12-16T19:06:20.385Z
Learnt from: 2witstudios
Repo: 2witstudios/PageSpace PR: 91
File: apps/web/src/components/ai/ui/Image.tsx:2-2
Timestamp: 2025-12-16T19:06:20.385Z
Learning: In apps/web/src/components/ai/ui/Image.tsx (TypeScript/React), the intentional use of `Experimental_GeneratedImage` from the Vercel AI SDK is accepted. This type is the correct and intended way to handle AI-generated images with base64/mediaType properties, and will be updated when the AI SDK stabilizes this API.

Applied to files:

  • apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx
📚 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/web/src/app/**/*.ts : Get request body using `const body = await request.json();`

Applied to files:

  • apps/web/src/app/api/auth/__tests__/signup-redirect.test.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/auth/login/route.ts
🧬 Code graph analysis (2)
apps/web/src/app/api/auth/signup/route.ts (2)
packages/lib/src/logging/logger-config.ts (1)
  • logSecurityEvent (179-186)
apps/web/src/lib/auth/login-csrf-utils.ts (1)
  • validateLoginCSRFToken (40-73)
apps/web/src/app/api/auth/login/route.ts (2)
packages/lib/src/logging/logger-config.ts (1)
  • logSecurityEvent (179-186)
apps/web/src/lib/auth/login-csrf-utils.ts (1)
  • validateLoginCSRFToken (40-73)
⏰ 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 (11)
apps/web/src/test/setup.ts (1)

28-31: The test secrets are appropriately using dummy values for the test environment. These critical environment variables (DATABASE_URL, CSRF_SECRET, ENCRYPTION_KEY, REALTIME_BROADCAST_SECRET) are already documented in .env.example, so no action is needed. Note: REALTIME_BROADCAST_SECRET is present in the root .env.example but missing from apps/web/.env.example—consider adding it for consistency.

Likely an incorrect or invalid review comment.

apps/web/src/components/ai/shared/__tests__/StreamingMarkdown.test.tsx (1)

1-9: LGTM! Test mock adapted for compatibility.

The change from JSX to React.createElement in the mock ensures compatibility without altering test behavior. The React import is correctly added to support this.

apps/web/src/hooks/__tests__/use-auth.test.ts (1)

300-327: LGTM! Test correctly validates CSRF fetch flow.

The test properly mocks both the CSRF token fetch (first call) and the login request (second call), and verifies the request body from the correct call index.

packages/lib/src/logging/logger-config.ts (1)

179-186: LGTM! Security event taxonomy properly extended.

The new CSRF-related events follow a consistent naming pattern and cover all validation failure scenarios for both login and signup flows.

apps/web/src/app/api/auth/__tests__/signup-redirect.test.ts (2)

64-72: LGTM! CSRF validation properly mocked.

The test setup correctly mocks both cookie parsing and CSRF validation to simulate successful CSRF checks.


150-166: LGTM! Request construction includes CSRF headers.

All test requests now correctly include the CSRF token in both header and cookie, matching the production validation requirements.

apps/web/src/app/api/auth/__tests__/signup.test.ts (2)

92-99: LGTM! CSRF mocking properly configured.

The mock setup correctly simulates cookie parsing and CSRF validation for test scenarios.


138-153: Excellent refactoring! Helper function improves test maintainability.

The createSignupRequest helper reduces duplication across tests and ensures consistent CSRF header inclusion. The support for additionalHeaders provides flexibility for test-specific needs like IP headers.

apps/web/src/lib/auth/login-csrf-utils.ts (3)

23-34: LGTM! Token generation follows security best practices.

The implementation correctly uses:

  • Cryptographically secure random bytes
  • Unix timestamp for expiry validation
  • HMAC-SHA256 signature with proper key derivation

40-73: LGTM! Validation is robust and secure.

The validation correctly implements:

  • Structure validation (3-part token)
  • Age-based expiry checking
  • Timing-safe signature comparison to prevent timing attacks
  • Defensive error handling

1-73: Consider additional defense-in-depth measures.

The PR objectives mention that "Origin header validation is suggested for defense-in-depth." Consider enhancing CSRF protection by validating the Origin or Referer headers in addition to the token validation.

Based on the security audit findings, would you like me to generate an implementation that adds Origin header validation to complement the double-submit token pattern? This would provide additional protection against cross-origin attacks.

2witstudios and others added 3 commits December 21, 2025 01:25
- csrf-utils.ts: Keep thorough sessionId validation with type check
- login/route.ts: Merge CSRF protection imports with authRepository
- login.test.ts: Use CSRF-aware createLoginRequest helper throughout

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@2witstudios
2witstudios merged commit 39959f2 into master Dec 21, 2025
3 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Dec 22, 2025
5 tasks
@2witstudios
2witstudios deleted the claude/csrf-security-audit-GejqO branch January 29, 2026 02:24
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