Skip to content

Fix bug hunt issues: authorization, validation, and schema mismatches #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pranjalkuhikar wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix bug hunt issues: authorization, validation, and schema mismatches #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7673e3b
fix(api): enforce tenant isolation and fix validation bugs
pranjalkuhikar Apr 18, 2026
7946b2e
fix(api): enforce teacher ownership in announcements PUT/DELETE
pranjalkuhikar Apr 18, 2026
f929f38
fix(api): add authorization to assignment update and delete operations
pranjalkuhikar Apr 18, 2026
a3d9b5d
fix(api): add await to findOneAndUpdate and set default maxMarks
pranjalkuhikar Apr 18, 2026
4435784
fix(grades): add teacher authorization to update and delete endpoints
pranjalkuhikar Apr 18, 2026
04be217
refactor(api): simplify profile GET endpoint and improve validation
pranjalkuhikar Apr 18, 2026
be6b9a4
fix(api): validate student data before database insertion
pranjalkuhikar Apr 18, 2026
89fccaa
fix(api): enforce teacher ownership on student update/delete
pranjalkuhikar Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
473 changes: 473 additions & 0 deletions Bug-Fix-Report.md
View file
Open in desktop

Large diffs are not rendered by default.

97 changes: 59 additions & 38 deletions app/api/announcements/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,79 +1,100 @@
import { auth } from '@clerk/nextjs/server'
import { NextRequest, NextResponse } from 'next/server'
import mongoose from 'mongoose'
import { connectDB } from '@/lib/mongodb'
import { Announcement } from '@/models/Announcement'
import { auth } from "@clerk/nextjs/server";
import { NextRequest, NextResponse } from "next/server";
import mongoose from "mongoose";
import { connectDB } from "@/lib/mongodb";
import { Announcement } from "@/models/Announcement";

const ALLOWED_FIELDS = ['title', 'content', 'body', 'audience', 'category', 'pinned', 'expiresAt']
const ALLOWED_FIELDS = ["title", "content", "audience", "category", "pinned"];

export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string }> }) {
const { userId } = await auth()
if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
export async function PUT(
req: NextRequest,
ctx: { params: Promise<{ id: string }> },
) {
const { userId } = await auth();
if (!userId)
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });

try {
const { id } = await ctx.params
const { id } = await ctx.params;

// Validate ObjectId
if (!mongoose.Types.ObjectId.isValid(id)) {
return NextResponse.json({ error: 'Invalid id' }, { status: 400 })
return NextResponse.json({ error: "Invalid id" }, { status: 400 });
}

await connectDB()
let body
await connectDB();

let body;
try {
body = await req.json()
body = await req.json();
} catch {
return NextResponse.json({ error: 'Invalid JSON request body' }, { status: 400 })
return NextResponse.json(
{ error: "Invalid JSON request body" },
{ status: 400 },
);
}

// Sanitize: only allow whitelisted fields
const sanitizedBody: Record<string, unknown> = {}
const sanitizedBody: Record<string, unknown> = {};
for (const key of ALLOWED_FIELDS) {
if (key in body) {
sanitizedBody[key] = body[key]
sanitizedBody[key] = body[key];
}
}
Comment on lines +27 to 43
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Reject non-object JSON before sanitizing.

req.json() can parse null, and key in body will then throw, producing a 500 for an invalid request body.

🛡️ Proposed fix 
     try {
       body = await req.json();
     } catch {
       return NextResponse.json(
         { error: "Invalid JSON request body" },
         { status: 400 },
       );
     }
+
+    if (!body || typeof body !== "object" || Array.isArray(body)) {
+      return NextResponse.json(
+        { error: "Request body must be an object" },
+        { status: 400 },
+      );
+    }
 
     // Sanitize: only allow whitelisted fields
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let body;
try {
body = await req.json()
body = await req.json();
} catch {
return NextResponse.json({ error: 'Invalid JSON request body' }, { status: 400 })
return NextResponse.json(
{ error: "Invalid JSON request body" },
{ status: 400 },
);
}
// Sanitize: only allow whitelisted fields
const sanitizedBody: Record<string, unknown> = {}
const sanitizedBody: Record<string, unknown> = {};
for (const key of ALLOWED_FIELDS) {
if (key in body) {
sanitizedBody[key] = body[key]
sanitizedBody[key] = body[key];
}
}
let body;
try {
body = await req.json();
} catch {
return NextResponse.json(
{ error: "Invalid JSON request body" },
{ status: 400 },
);
}
if (!body || typeof body !== "object" || Array.isArray(body)) {
return NextResponse.json(
{ error: "Request body must be an object" },
{ status: 400 },
);
}
// Sanitize: only allow whitelisted fields
const sanitizedBody: Record<string, unknown> = {};
for (const key of ALLOWED_FIELDS) {
if (key in body) {
sanitizedBody[key] = body[key];
}
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/announcements/`[id]/route.ts around lines 27 - 43, After parsing JSON
with req.json(), validate that the parsed body is a non-null object before using
"key in body" to sanitize; if the parsed body is null or not an object, return a
400 NextResponse.json error. Update the logic around body, the sanitization loop
that fills sanitizedBody, and any early return paths so they check typeof body
=== "object" && body !== null (or equivalent) prior to iterating ALLOWED_FIELDS.


const announcement = await Announcement.findOneAndUpdate(
{ _id: id },
{ _id: id, teacherId: userId },
{ $set: sanitizedBody },
{ new: true, runValidators: true, context: 'query' }
)
if (!announcement) return NextResponse.json({ error: 'Not found' }, { status: 404 })
return NextResponse.json(announcement)
{ new: true, runValidators: true, context: "query" },
);
if (!announcement)
return NextResponse.json({ error: "Not found" }, { status: 404 });
return NextResponse.json(announcement);
} catch (error) {
if (error instanceof Error) {
console.error('PUT /api/announcements/[id] error:', error.message)
console.error("PUT /api/announcements/[id] error:", error.message);
}
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
return NextResponse.json(
{ error: "Internal server error" },
{ status: 500 },
);
}
}

export async function DELETE(_req: NextRequest, ctx: { params: Promise<{ id: string }> }) {
const { userId } = await auth()
if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
export async function DELETE(
_req: NextRequest,
ctx: { params: Promise<{ id: string }> },
) {
const { userId } = await auth();
if (!userId)
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });

try {
const { id } = await ctx.params
const { id } = await ctx.params;

// Validate ObjectId
if (!mongoose.Types.ObjectId.isValid(id)) {
return NextResponse.json({ error: 'Invalid id' }, { status: 400 })
return NextResponse.json({ error: "Invalid id" }, { status: 400 });
}

await connectDB()
const deleted = await Announcement.findOneAndDelete({ _id: id })

await connectDB();
const deleted = await Announcement.findOneAndDelete({
_id: id,
teacherId: userId,
});

if (!deleted) {
return NextResponse.json({ error: 'Not found' }, { status: 404 })
return NextResponse.json({ error: "Not found" }, { status: 404 });
}
return NextResponse.json({ success: true })

return NextResponse.json({ success: true });
} catch (error) {
if (error instanceof Error) {
console.error('DELETE /api/announcements/[id] error:', error.message)
console.error("DELETE /api/announcements/[id] error:", error.message);
}
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
return NextResponse.json(
{ error: "Internal server error" },
{ status: 500 },
);
}
}
106 changes: 68 additions & 38 deletions app/api/assignments/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,79 +1,109 @@
import { auth } from '@clerk/nextjs/server'
import { NextRequest, NextResponse } from 'next/server'
import mongoose from 'mongoose'
import { connectDB } from '@/lib/mongodb'
import { Assignment } from '@/models/Assignment'
import { auth } from "@clerk/nextjs/server";
import { NextRequest, NextResponse } from "next/server";
import mongoose from "mongoose";
import { connectDB } from "@/lib/mongodb";
import { Assignment } from "@/models/Assignment";

const ALLOWED_UPDATE_FIELDS = ['title', 'description', 'dueDate', 'deadline', 'subject', 'class', 'status', 'kanbanStatus', 'maxMarks']
const ALLOWED_UPDATE_FIELDS = [
"title",
"description",
"deadline",
"subject",
"class",
"status",
"kanbanStatus",
"maxMarks",
];

export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string }> }) {
const { userId } = await auth()
if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
export async function PUT(
req: NextRequest,
ctx: { params: Promise<{ id: string }> },
) {
const { userId } = await auth();
if (!userId)
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });

try {
const { id } = await ctx.params
const { id } = await ctx.params;

// Validate ObjectId
if (!mongoose.Types.ObjectId.isValid(id)) {
return NextResponse.json({ error: 'Invalid id' }, { status: 400 })
return NextResponse.json({ error: "Invalid id" }, { status: 400 });
}

await connectDB()
let body
await connectDB();

let body;
try {
body = await req.json()
body = await req.json();
} catch {
return NextResponse.json({ error: 'Invalid JSON in request body' }, { status: 400 })
return NextResponse.json(
{ error: "Invalid JSON in request body" },
{ status: 400 },
);
}

// Sanitize: only allow whitelisted fields
const sanitizedBody: Record<string, unknown> = {}
const sanitizedBody: Record<string, unknown> = {};
for (const key of ALLOWED_UPDATE_FIELDS) {
if (key in body) {
sanitizedBody[key] = body[key]
sanitizedBody[key] = body[key];
}
}

const assignment = await Assignment.findOneAndUpdate(
{ _id: id },
{ _id: id, teacherId: userId },
sanitizedBody,
{ new: true }
)
if (!assignment) return NextResponse.json({ error: 'Not found' }, { status: 404 })
return NextResponse.json(assignment)
{ new: true },
);
Comment on lines +36 to +58
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does Mongoose findOneAndUpdate run schema validators by default, or must runValidators: true be passed?

💡 Result:

Mongoose findOneAndUpdate does not run schema validators by default. You must pass { runValidators: true } in the options to enable update validators.

Citations:

🏁 Script executed:

#!/bin/bash
node <<'EOF'
// Test 1: Does 'in' operator throw with null?
console.log("Test 1: 'in' operator with null");
try {
  const body = null;
  const key = "test";
  const result = key in body;
  console.log("  Result:", result);
} catch (e) {
  console.log("  ✓ THROWS:", e.message);
}

// Test 2: Does 'in' operator work with object?
console.log("\nTest 2: 'in' operator with object");
try {
  const body = { test: 1 };
  const key = "test";
  const result = key in body;
  console.log("  Result:", result);
} catch (e) {
  console.log("  Error:", e.message);
}

// Test 3: Does 'in' operator throw with array?
console.log("\nTest 3: 'in' operator with array");
try {
  const body = [1, 2, 3];
  const key = "0";
  const result = key in body;
  console.log("  Result:", result);
} catch (e) {
  console.log("  Error:", e.message);
}
EOF

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 280

🌐 Web query:

Mongoose runValidators context: "query" update validation option explained

💡 Result:

In Mongoose, when using update validators with the runValidators: true option in operations like updateOne, findOneAndUpdate, etc., the context: "query" option controls the value of this within custom validator functions. By default, during update validation (without context: "query"), this in validators refers to an undefined or null value (or the global object in non-strict mode), because the full document may not be loaded in memory. Setting context: "query" changes this to the Query object. This allows validators to access other parts of the update using query methods like this.get('name') or this.getUpdate.$set.name, enabling cross-field validation based on the update data. Example from older docs (v5.x) and community usage: toySchema.path('color').validate(function(value) { if (this.getUpdate.$set.name.toLowerCase.indexOf('red') !== -1) { return value === 'red'; } return true; }, 'Must be red if name contains "red"'); Toy.updateOne({}, { color: 'blue', name: 'Red Power Ranger' }, { runValidators: true, context: 'query' }); This fails validation because name contains 'red' but color is not 'red'. In recent Mongoose versions (v6+ to v9+ as of 2026), the official docs state that this refers to the Query by default during update validation: "When running with update validators, this is the Query". The explicit context: "query" option appears deprecated or unnecessary, as the behavior is now default. However, Stack Overflow answers and some plugins still recommend it for compatibility. Usage example: Model.findOneAndUpdate( { _id: id }, { $set: { field: value } }, { runValidators: true, context: 'query' } // For cross-field validation access ); Note caveats: Update validators only run on specific operators ($set, $unset, etc.) and only on updated paths. Sources confirm this behavior across versions 5-9.

Citations:

Add request body validation and enable Mongoose update validators.

Sending JSON null as the request body will cause a TypeError when the code executes key in body, since the in operator cannot be used on null. Additionally, Mongoose's findOneAndUpdate does not run schema validators by default—you must explicitly pass runValidators: true to validate fields like status, kanbanStatus, deadline, or maxMarks.

🛡️ Proposed fix 
     try {
       body = await req.json();
     } catch {
       return NextResponse.json(
         { error: "Invalid JSON in request body" },
         { status: 400 },
       );
     }
+
+    if (!body || typeof body !== "object" || Array.isArray(body)) {
+      return NextResponse.json(
+        { error: "Request body must be an object" },
+        { status: 400 },
+      );
+    }
 
     // Sanitize: only allow whitelisted fields
     const sanitizedBody: Record<string, unknown> = {};
     for (const key of ALLOWED_UPDATE_FIELDS) {
       if (key in body) {
         sanitizedBody[key] = body[key];
       }
     }
     
     const assignment = await Assignment.findOneAndUpdate(
       { _id: id, teacherId: userId },
       sanitizedBody,
-      { new: true },
+      { new: true, runValidators: true },
     );
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/assignments/`[id]/route.ts around lines 36 - 58, Guard against
null/non-object request bodies before using the `in` operator: validate `body`
(from `req.json()`) is a plain object (e.g., typeof body === "object" && body
!== null) and return a 400 error if not; then build `sanitizedBody` from
`ALLOWED_UPDATE_FIELDS` as before. Also enable Mongoose validators on update by
passing `runValidators: true` (and optionally `context: "query"`) to
`Assignment.findOneAndUpdate` so schema validators for `status`, `kanbanStatus`,
`deadline`, `maxMarks`, etc. are enforced.

if (!assignment)
return NextResponse.json({ error: "Not found" }, { status: 404 });
return NextResponse.json(assignment);
} catch (error) {
if (error instanceof Error) {
console.error('PUT /api/assignments/[id] error:', error.message)
console.error("PUT /api/assignments/[id] error:", error.message);
}
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
return NextResponse.json(
{ error: "Internal server error" },
{ status: 500 },
);
}
}

export async function DELETE(_req: NextRequest, ctx: { params: Promise<{ id: string }> }) {
const { userId } = await auth()
if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
export async function DELETE(
_req: NextRequest,
ctx: { params: Promise<{ id: string }> },
) {
const { userId } = await auth();
if (!userId)
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });

try {
const { id } = await ctx.params
const { id } = await ctx.params;

// Validate ObjectId
if (!mongoose.Types.ObjectId.isValid(id)) {
return NextResponse.json({ error: 'Invalid id' }, { status: 400 })
return NextResponse.json({ error: "Invalid id" }, { status: 400 });
}

await connectDB()
const deleted = await Assignment.findOneAndDelete({ _id: id })

await connectDB();
const deleted = await Assignment.findOneAndDelete({
_id: id,
teacherId: userId,
});

if (!deleted) {
return NextResponse.json({ error: 'Not found' }, { status: 404 })
return NextResponse.json({ error: "Not found" }, { status: 404 });
}
return NextResponse.json({ success: true })

return NextResponse.json({ success: true });
} catch (error) {
if (error instanceof Error) {
console.error('DELETE /api/assignments/[id] error:', error.message)
console.error("DELETE /api/assignments/[id] error:", error.message);
}
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
return NextResponse.json(
{ error: "Internal server error" },
{ status: 500 },
);
}
}
Loading
Loading