fix: resolve Insecure Direct Object Reference (IDOR) in annotations API#2356
fix: resolve Insecure Direct Object Reference (IDOR) in annotations API#2356Abhishek2005-ard wants to merge 7 commits into
Conversation
|
@Abhishek2005-ard is attempting to deploy a commit to the Nisshchaya's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
More reviews will be available in 9 minutes and 53 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThe ChangesAnnotations authorization and config resilience
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎉 Thanks for your contribution, @Abhishek2005-ard!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
1 similar comment
🎉 Thanks for your contribution, @Abhishek2005-ard!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/api/annotations/route.ts (1)
20-20: 💤 Low valueConsider validating
repositoryIdis a valid integer before parsing.If
repositoryIdis a non-numeric string (e.g.,"abc"),parseIntreturnsNaN, which Prisma will reject or handle unexpectedly. A validation check could provide a clearer 400 response.✨ Optional improvement
if (!repositoryId) { return NextResponse.json({ error: "repositoryId is required" }, { status: 400 }); } + const repoIdNum = parseInt(repositoryId, 10); + if (isNaN(repoIdNum)) { + return NextResponse.json({ error: "repositoryId must be a valid integer" }, { status: 400 }); + } + // Verify user has access to the repository to prevent IDOR const repo = await prisma.repository.findFirst({ where: { - id: parseInt(repositoryId), + id: repoIdNum, userId: user.userId, } });Then reuse
repoIdNumin themapAnnotation.findManyquery as well.Also applies to: 31-31
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/annotations/route.ts` at line 20, The code directly parses repositoryId without validating it is a numeric string first, causing parseInt to return NaN for invalid inputs which Prisma will not handle correctly. Before calling parseInt on repositoryId, add a validation check to ensure it is a valid numeric string and return a 400 Bad Request response if validation fails. Store the parsed integer result in a variable (such as repoIdNum) and use this variable in both the id field on line 20 and in the mapAnnotation.findMany query on line 31, replacing the direct parseInt calls at both locations.next.config.js (1)
14-19: Narrow thecatchto only the missing-module case.At line 18, the unconditional
catchsuppresses all initialization errors and can silently disable the analyzer when there's a real config or runtime problem. Narrow the catch to handle only theMODULE_NOT_FOUNDerror for the missing analyzer package, and rethrow other errors.Suggested patch
let withBundleAnalyzer; try { withBundleAnalyzer = require('`@next/bundle-analyzer`')({ enabled: process.env.ANALYZE === 'true', }); -} catch { - withBundleAnalyzer = (config) => config; +} catch (error) { + const isMissingAnalyzer = + error && + error.code === 'MODULE_NOT_FOUND' && + String(error.message || '').includes('`@next/bundle-analyzer`'); + + if (isMissingAnalyzer) { + withBundleAnalyzer = (config) => config; + } else { + throw error; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@next.config.js` around lines 14 - 19, The catch block in the withBundleAnalyzer initialization is too broad and silently suppresses all errors, masking real configuration or runtime problems. Modify the catch block to check if the error is specifically a MODULE_NOT_FOUND error (indicating the analyzer package is missing). If it is a missing module error, assign the passthrough function to withBundleAnalyzer as currently done. If the error is anything else, rethrow it so that real problems are not silently ignored and can be properly diagnosed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/api/annotations/route.ts`:
- Line 20: The code directly parses repositoryId without validating it is a
numeric string first, causing parseInt to return NaN for invalid inputs which
Prisma will not handle correctly. Before calling parseInt on repositoryId, add a
validation check to ensure it is a valid numeric string and return a 400 Bad
Request response if validation fails. Store the parsed integer result in a
variable (such as repoIdNum) and use this variable in both the id field on line
20 and in the mapAnnotation.findMany query on line 31, replacing the direct
parseInt calls at both locations.
In `@next.config.js`:
- Around line 14-19: The catch block in the withBundleAnalyzer initialization is
too broad and silently suppresses all errors, masking real configuration or
runtime problems. Modify the catch block to check if the error is specifically a
MODULE_NOT_FOUND error (indicating the analyzer package is missing). If it is a
missing module error, assign the passthrough function to withBundleAnalyzer as
currently done. If the error is anything else, rethrow it so that real problems
are not silently ignored and can be properly diagnosed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b28f7c6-56a8-4b59-a4d1-b0f1767b84d8
📒 Files selected for processing (2)
app/api/annotations/route.tsnext.config.js
🎉 Thanks for your contribution, @Abhishek2005-ard!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
🎉 Thanks for your contribution, @Abhishek2005-ard!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
🎉 Thanks for your contribution, @Abhishek2005-ard!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
🎉 Thanks for your contribution, @Abhishek2005-ard!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
🎉 Thanks for your contribution, @Abhishek2005-ard!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
1 participant
Description This PR addresses a critical Insecure Direct Object Reference (IDOR) vulnerability in the GET /api/annotations endpoint. Previously, the endpoint fetched map annotations solely based on the provided repositoryId query parameter, without verifying if the authenticated user actually had read access or ownership of that repository. This allowed unauthorized users to view sensitive annotations on private repositories.
Changes Included
Added an authorization check using prisma.repository.findFirst in the GET endpoint of app/api/annotations/route.ts.
The endpoint now strictly verifies that the requested repositoryId belongs to the authenticated user.userId before returning the mapAnnotation records.
Returns a 403 Forbidden response if the repository does not exist or if the user lacks the necessary permissions.
Security Impact
Prevents unauthorized data disclosure of repository-specific notes, map annotations, and private discussions.
Resolves IDOR exposure on the /api/annotations endpoint.
close #2353
Summary by CodeRabbit
Bug Fixes
Chores