Skip to content

fix(security): enforce drive access checks in mentions search - #265

Merged
2witstudios merged 2 commits into
masterfrom
high-mentions-search-must-enforce-drive-access
Jan 27, 2026
Merged

2witstudios merged 2 commits into
masterfrom
high-mentions-search-must-enforce-drive-access

Conversation

@2witstudios

@2witstudios 2witstudios commented Jan 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Security fix: The mentions search endpoint (/api/mentions/search) loaded drives by ID without verifying the requester had access, enabling cross-drive data enumeration
  • Within-drive search: Added getUserDriveAccess gating — unauthorized requests now return 403 instead of leaking data
  • Cross-drive search: Replaced the broken getUserAccessibleDrives (which only returned owned drives) with getDriveIdsForUser, which correctly includes owned, member, and page-permission drives
  • Validation: Added Zod validation for the driveId parameter
  • Tests: Added 15 contract tests covering authentication, authorization, drive enumeration prevention, and shared drive access scenarios

Test plan

  • All 15 new tests pass (apps/web/src/app/api/mentions/search/__tests__/route.test.ts)
  • TypeScript compiles without errors across all packages
  • Manual test: search mentions in an owned drive — results returned
  • Manual test: search mentions in a shared drive (member) — results returned
  • Manual test: search mentions in an unauthorized drive — 403 returned
  • Manual test: cross-drive search only returns results from accessible drives

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Enhanced input validation with JSON error responses and appropriate HTTP status codes for mentions search functionality.
    • Improved authorization checks to ensure proper access verification before processing cross-drive and within-drive searches.
  • Tests

    • Added comprehensive test suite covering authentication, authorization, and security scenarios for search functionality.

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

The mentions search endpoint loaded drives by ID without verifying the
requester had access, allowing cross-drive data enumeration. Replace the
local getUserAccessibleDrives (which only returned owned drives) with
getDriveIdsForUser for cross-drive search and add getUserDriveAccess
gating for within-drive search. Unauthorized requests now receive 403.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@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.

@2witstudios

Copy link
Copy Markdown
Owner Author

Code review

Found 1 issue:

  1. Error responses use inconsistent formats: lines 259 and 324 return plain text (new NextResponse('Drive not found', { status: 404 }) and new NextResponse('Internal Server Error', { status: 500 })), while all other error responses in the same file use JSON format (NextResponse.json({ error: '...' }, { status: ... })). CLAUDE.md section 2.2 says "Return JSON: return Response.json(data) or return NextResponse.json(data)".

if (!drive) {
return new NextResponse('Drive not found', { status: 404 });
}

} catch (error) {
loggers.api.error('[MENTIONS_SEARCH_GET]', error as Error);
return new NextResponse('Internal Server Error', { status: 500 });
}

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Replace plain text error responses with NextResponse.json() to match
the format used by all other error responses in the same file.

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

Copy link
Copy Markdown
Owner Author

Fixed in 54dadbd. Both plain-text error responses (lines 259 and 324) now use NextResponse.json({ error: '...' }, { status }) to match the JSON format used by all other error responses in the file.

  • new NextResponse('Drive not found', { status: 404 }) → NextResponse.json({ error: 'Drive not found' }, { status: 404 })
  • new NextResponse('Internal Server Error', { status: 500 }) → NextResponse.json({ error: 'Internal Server Error' }, { status: 500 })

Tests: 15/15 passing. No new type errors.

@coderabbitai

coderabbitai Bot commented Jan 27, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds comprehensive test coverage and access control enhancements to the mentions search route. Implements input validation via Zod, introduces permission service integrations (getUserDriveAccess, getDriveIdsForUser) for authorization checks, and restructures cross-drive and within-drive search logic with improved error handling patterns.

Changes

Cohort / File(s) Summary
Mentions Search Route Implementation
apps/web/src/app/api/mentions/search/route.ts
Refactored access control logic with Zod schema validation for driveId; integrated getUserDriveAccess and getDriveIdsForUser service functions for robust authorization; separated cross-drive and within-drive search paths; replaced text responses with structured JSON error objects; added pre-query access verification to prevent data leakage.
Mentions Search Route Tests
apps/web/src/app/api/mentions/search/__tests__/route.test.ts
Added 312-line contract test suite covering authentication flows (401 responses), input validation (400 for invalid driveId), authorization enforcement (403 for unauthorized access), service function call patterns, and verification that access checks execute before database queries.
Library Exports
packages/lib/src/server.ts
Re-exported getDriveIdsForUser from permissions module to support route-level access control.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 A hop, a skip, through drives we pick,
With access checks so slick and thick,
No data leaks, no sneaky peeks,
Just validated paths for weeks and weeks! 🔐✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(security): enforce drive access checks in mentions search' directly and accurately summarizes the main security fix: adding access control validation to the mentions search endpoint to prevent unauthorized drive access.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@2witstudios
2witstudios merged commit ade8668 into master Jan 27, 2026
3 checks passed
@2witstudios
2witstudios deleted the high-mentions-search-must-enforce-drive-access branch January 29, 2026 02:22
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.

1 participant