Research and implement community feedback features#9
Conversation
Co-authored-by: raavtube <raavtube@icloud.com>
|
Cursor Agent can help with this pull request. Just |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
|
|
||
| {session && !isOwner && ( | ||
| <> | ||
| {isOwner && <DropdownMenuSeparator />} |
There was a problem hiding this comment.
| {isOwner && <DropdownMenuSeparator />} |
Unreachable code: The dropdown separator will never render due to contradictory conditional logic.
View Details
Analysis
Line 181 contains {isOwner && <DropdownMenuSeparator />} inside a block that's already conditioned on {session && !isOwner && (. Since the outer condition requires !isOwner to be true, the inner condition isOwner can never be true, making the separator completely unreachable.
This appears to be a copy-paste error where the separator condition should either be removed entirely (if no separator is needed) or changed to simply <DropdownMenuSeparator /> if a separator should always appear between owner actions and report actions when both sections are present.
Looking at the structure, it seems like the intention was to show a separator between the "Delete" option (for owners) and the "Report" option (for non-owners), but the logic is inverted. The separator should likely be conditional on whether owner actions are present: {session && isOwner && <DropdownMenuSeparator />} to separate owner actions from non-owner actions.
| .where(sql`${rating.ruleId} = ANY(${ruleIds})`) | ||
| .groupBy(rating.ruleId) | ||
|
|
||
| // Get comment counts for all rules | ||
| const commentStats = await db | ||
| .select({ | ||
| ruleId: comment.ruleId, | ||
| totalComments: count(comment.id), | ||
| }) | ||
| .from(comment) | ||
| .where(sql`${comment.ruleId} = ANY(${ruleIds})`) |
There was a problem hiding this comment.
Critical SQL injection vulnerability: User input is directly interpolated into SQL queries without proper sanitization.
View Details
📝 Patch Details
diff --git a/app/api/feed/stats/route.ts b/app/api/feed/stats/route.ts
index 4ded31a..e849dec 100644
--- a/app/api/feed/stats/route.ts
+++ b/app/api/feed/stats/route.ts
@@ -1,7 +1,7 @@
import { NextRequest, NextResponse } from "next/server"
import { db } from "@/lib/db"
import { rating, comment } from "@/lib/schema"
-import { eq, avg, count, sql } from "drizzle-orm"
+import { eq, avg, count, sql, inArray } from "drizzle-orm"
export async function GET(request: NextRequest) {
try {
@@ -20,7 +20,7 @@ export async function GET(request: NextRequest) {
totalRatings: count(rating.id),
})
.from(rating)
- .where(sql`${rating.ruleId} = ANY(${ruleIds})`)
+ .where(inArray(rating.ruleId, ruleIds))
.groupBy(rating.ruleId)
// Get comment counts for all rules
@@ -30,7 +30,7 @@ export async function GET(request: NextRequest) {
totalComments: count(comment.id),
})
.from(comment)
- .where(sql`${comment.ruleId} = ANY(${ruleIds})`)
+ .where(inArray(comment.ruleId, ruleIds))
.groupBy(comment.ruleId)
// Combine the stats
@@ -75,4 +75,4 @@ export async function GET(request: NextRequest) {
console.error("Error fetching feed stats:", error)
return NextResponse.json({ error: "Internal server error" }, { status: 500 })
}
-}
\ No newline at end of file
+}
Analysis
Lines 23 and 33 use sql${rating.ruleId} = ANY(${ruleIds}) and `sql`${comment.ruleId} = ANY(${ruleIds}) where ruleIds comes directly from URL query parameters that are split by comma. This creates a SQL injection vulnerability because the user-controlled ruleIds array is interpolated directly into the SQL query without proper sanitization or parameterization.
An attacker could manipulate the ruleIds parameter to inject malicious SQL code. For example, a request like /api/feed/stats?ruleIds='; DROP TABLE rating; -- could potentially execute dangerous SQL commands.
The fix is to use Drizzle ORM's inArray() function instead of raw SQL interpolation: .where(inArray(rating.ruleId, ruleIds)) and .where(inArray(comment.ruleId, ruleIds)). This will properly parameterize the query and prevent SQL injection attacks.
2 participants
Add discussion and rating features to rules and feed pages.
This PR implements a comprehensive discussion and rating system to gather community input, improve content quality, and foster engagement, as requested by a customer. It includes:
rating,comment, andreport.