Skip to content

Fix/critical stability, security, and logic fixes #7

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
Kalebtes2031 wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Fix/critical stability, security, and logic fixes #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7a6787a
fix: validate student input by checking safeParse result before datab…
Kalebtes2031 Apr 18, 2026
093f27d
fix: add missing await on Grade.findOneAndUpdate in POST handler
Kalebtes2031 Apr 18, 2026
d05041e
fix: default maxMarks to 100 when not provided to prevent NaN grade c…
Kalebtes2031 Apr 18, 2026
367f2dd
fix: prevent error stack trace leakage in grades API responses
Kalebtes2031 Apr 18, 2026
05ad368
fix: prevent error stack trace leakage in assignments API response
Kalebtes2031 Apr 18, 2026
684354c
fix: add teacherId filter to students update/delete to prevent unauth…
Kalebtes2031 Apr 18, 2026
fbfa136
fix: add teacherId filter to announcements update/delete to prevent u…
Kalebtes2031 Apr 18, 2026
f4e0311
fix: add teacherId filter to assignments update/delete to prevent una…
Kalebtes2031 Apr 18, 2026
4efd6ce
fix: add teacherId filter and ObjectId validation to grades update/de…
Kalebtes2031 Apr 18, 2026
84dc180
fix: use total count from API instead of capped array length for dash…
Kalebtes2031 Apr 18, 2026
7a181d2
fix: correct grade boundary for A+ to use >= 90 instead of > 90
Kalebtes2031 Apr 18, 2026
b6014c0
fix: Grade findOneAndUpdate hook now validates marks inside operator
Kalebtes2031 Apr 18, 2026
4961a8b
fix: remove userId query param bypass in profile GET to prevent unaut…
Kalebtes2031 Apr 18, 2026
ac3daff
fix: correct grade point for D from 4 to 5 in dashboard CGPA calculation
Kalebtes2031 Apr 18, 2026
f5558d9
fix: Grade updateOne hook now validates marks inside operator
Kalebtes2031 Apr 18, 2026
52182a7
fix: check API response status before processing dashboard data to pr…
Kalebtes2031 Apr 18, 2026
d8a4c3b
fix: handle negative and zero values in timeAgo to prevent displaying…
Kalebtes2031 Apr 18, 2026
484c7af
fix: guard against division by zero in grade percentage calculation
Kalebtes2031 Apr 18, 2026
91c0486
fix: guard against division by zero in API calcGrade function
Kalebtes2031 Apr 18, 2026
ef31c4b
fix: validate studentId format in attendance GET to prevent 500 errors
Kalebtes2031 Apr 18, 2026
c024e02
fix: validate studentId format in grades GET to prevent 500 errors
Kalebtes2031 Apr 18, 2026
801f414
fix: validate studentId format in attendance POST (single and bulk) t…
Kalebtes2031 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
4 changes: 2 additions & 2 deletions app/api/announcements/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string
}

const announcement = await Announcement.findOneAndUpdate(
{ _id: id },
{ _id: id, teacherId: userId },
{ $set: sanitizedBody },
{ new: true, runValidators: true, context: 'query' }
)
Expand All @@ -63,7 +63,7 @@ export async function DELETE(_req: NextRequest, ctx: { params: Promise<{ id: str
}

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

if (!deleted) {
return NextResponse.json({ error: 'Not found' }, { status: 404 })
Expand Down
4 changes: 2 additions & 2 deletions app/api/assignments/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string
}

const assignment = await Assignment.findOneAndUpdate(
{ _id: id },
{ _id: id, teacherId: userId },
sanitizedBody,
{ new: true }
)
Expand All @@ -63,7 +63,7 @@ export async function DELETE(_req: NextRequest, ctx: { params: Promise<{ id: str
}

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

if (!deleted) {
return NextResponse.json({ error: 'Not found' }, { status: 404 })
Expand Down
2 changes: 1 addition & 1 deletion app/api/assignments/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export async function GET(req: NextRequest) {
if (error instanceof Error) {
console.error('GET /api/assignments error:', error.message)
}
return NextResponse.json({ error: error instanceof Error ? error.stack : 'Internal server error' }, { status: 500 })
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
}
}

Expand Down
27 changes: 23 additions & 4 deletions app/api/attendance/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { auth } from '@clerk/nextjs/server'
import { NextRequest, NextResponse } from 'next/server'
import mongoose from 'mongoose'
import { connectDB } from '@/lib/mongodb'
import { Attendance } from '@/models/Attendance'
import { z } from 'zod'
Expand Down Expand Up @@ -76,7 +77,12 @@ export async function GET(req: NextRequest) {
query.date = dateRange;
}
if (cls) query.class = cls;
if (studentId) query.studentId = studentId;
if (studentId) {
if (!mongoose.Types.ObjectId.isValid(studentId)) {
return NextResponse.json({ error: "Invalid studentId" }, { status: 400 });
}
query.studentId = studentId;
}

const records = await Attendance.find(query)
.sort({ date: -1, studentName: 1 })
Expand Down Expand Up @@ -124,7 +130,15 @@ export async function POST(req: NextRequest) {
);

if (isBulk) {
const ops = (parsed.data as z.infer<typeof BulkSchema>).map((record) => ({
const data = parsed.data as z.infer<typeof BulkSchema>;
// Validate all studentIds
for (const record of data) {
if (!mongoose.Types.ObjectId.isValid(record.studentId)) {
return NextResponse.json({ error: `Invalid studentId: ${record.studentId}` }, { status: 400 });
}
}

const ops = data.map((record) => ({
updateOne: {
filter: { teacherId: userId, studentId: record.studentId, date: record.date },
update: { $set: { ...record, teacherId: userId } },
Expand All @@ -134,9 +148,14 @@ export async function POST(req: NextRequest) {
await Attendance.bulkWrite(ops)
return NextResponse.json({ success: true, count: ops.length })
} else {
const data = parsed.data as z.infer<typeof AttendanceSchema>;
if (!mongoose.Types.ObjectId.isValid(data.studentId)) {
return NextResponse.json({ error: "Invalid studentId" }, { status: 400 });
}

const record = await Attendance.findOneAndUpdate(
{ teacherId: userId, studentId: (parsed.data as z.infer<typeof AttendanceSchema>).studentId, date: (parsed.data as z.infer<typeof AttendanceSchema>).date },
{ $set: { ...(parsed.data as z.infer<typeof AttendanceSchema>), teacherId: userId } },
{ teacherId: userId, studentId: data.studentId, date: data.date },
{ $set: { ...data, teacherId: userId } },
{ upsert: true, new: true }
)
return NextResponse.json(record, { status: 201 })
Expand Down
10 changes: 8 additions & 2 deletions app/api/grades/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string

await connectDB()
const grade = await Grade.findOneAndUpdate(
{ _id: id },
{ _id: id, teacherId: userId },
sanitizedBody,
{ new: true }
)
Expand All @@ -55,8 +55,14 @@ export async function DELETE(_req: NextRequest, ctx: { params: Promise<{ id: str

try {
const { id } = await ctx.params

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

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

if (!deleted) {
return NextResponse.json({ error: 'Grade not found' }, { status: 404 })
Expand Down
19 changes: 13 additions & 6 deletions app/api/grades/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { auth } from '@clerk/nextjs/server'
import { NextRequest, NextResponse } from 'next/server'
import mongoose from 'mongoose'
import { connectDB } from '@/lib/mongodb'
import { Grade } from '@/models/Grade'
import { z } from 'zod'
Expand All @@ -20,8 +21,9 @@ const GradeSchema = z.object({
)

function calcGrade(marks: number, max: number): string {
if (max <= 0) return 'F'
const pct = (marks / max) * 100
if (pct > 90) return 'A+'
if (pct >= 90) return 'A+'
if (pct >= 80) return 'A'
if (pct >= 70) return 'B+'
if (pct >= 60) return 'B'
Expand All @@ -41,14 +43,19 @@ export async function GET(req: NextRequest) {
const subject = searchParams.get('subject')

const query: Record<string, unknown> = { teacherId: userId }
if (studentId) query.studentId = studentId
if (studentId) {
if (!mongoose.Types.ObjectId.isValid(studentId)) {
return NextResponse.json({ error: "Invalid studentId" }, { status: 400 });
}
query.studentId = studentId
}
if (subject) query.subject = subject

const grades = await Grade.find(query).sort({ createdAt: -1 }).lean()
return NextResponse.json(grades)
} catch (error) {
console.error('GET /api/grades error:', error instanceof Error ? error.message : error)
return NextResponse.json({ error: error instanceof Error ? error.stack : 'Internal server error' }, { status: 500 })
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
}
}

Expand All @@ -70,10 +77,10 @@ export async function POST(req: NextRequest) {
if (!parsed.success) return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 })

const data = parsed.data
const max = data.maxMarks!
const max = data.maxMarks ?? 100
const term = data.term ?? 'Term 1'

const grade = Grade.findOneAndUpdate(
const grade = await Grade.findOneAndUpdate(
{ teacherId: userId, studentId: data.studentId, subject: data.subject, term },
{ $set: { ...data, term, teacherId: userId, grade: calcGrade(data.marks, max) } },
{ upsert: true, new: true }
Expand All @@ -83,6 +90,6 @@ export async function POST(req: NextRequest) {
if (error instanceof Error) {
console.error('POST /api/grades error:', error.message)
}
return NextResponse.json({ error: error instanceof Error ? error.stack : 'Internal server error' }, { status: 500 })
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
}
}
11 changes: 2 additions & 9 deletions app/api/profile/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,8 @@ import { NextRequest, NextResponse } from 'next/server'
import { connectDB } from '@/lib/mongodb'
import { Teacher } from '@/models/Teacher'

export async function GET(req: NextRequest) {
const { searchParams } = new URL(req.url)
const queryUserId = searchParams.get('userId')

let userId: string | null = queryUserId
if (!userId) {
const session = await auth()
userId = session.userId
}
export async function GET() {
const { userId } = await auth()
if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })

try {
Expand Down
29 changes: 18 additions & 11 deletions app/api/students/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { NextRequest, NextResponse } from 'next/server'
import mongoose from 'mongoose'
import { connectDB } from '@/lib/mongodb'
import { Student } from '@/models/Student'
import { z } from 'zod'

const ALLOWED_UPDATE_FIELDS = ['name', 'email', 'grade', 'rollNo', 'class', 'phone', 'address', 'parentName', 'parentPhone']

Expand All @@ -25,19 +26,25 @@ export async function PUT(req: NextRequest, ctx: { params: Promise<{ id: string
return NextResponse.json({ error: 'Bad Request' }, { 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 StudentUpdateSchema = z.object({
name: z.string().min(1).optional(),
rollNo: z.string().min(1).optional(),
class: z.string().min(1).optional(),
email: z.string().email().optional().or(z.literal('')),
phone: z.string().optional(),
address: z.string().optional(),
parentName: z.string().optional(),
parentPhone: z.string().optional(),
})

const parsed = StudentUpdateSchema.safeParse(body)
if (!parsed.success) return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 })

await connectDB()
const student = await Student.findOneAndUpdate(
{ _id: id },
sanitizedBody,
{ new: true }
{ _id: id, teacherId: userId },
{ $set: parsed.data },
{ new: true, runValidators: true }
)
if (!student) return NextResponse.json({ error: 'Not found' }, { status: 404 })
return NextResponse.json(student)
Expand Down Expand Up @@ -65,7 +72,7 @@ export async function DELETE(_req: NextRequest, ctx: { params: Promise<{ id: str
}

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

if (!deleted) {
return NextResponse.json({ error: 'Student not found' }, { status: 404 })
Expand Down
5 changes: 3 additions & 2 deletions app/api/students/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ export async function POST(req: NextRequest) {
return NextResponse.json({ error: 'Malformed JSON' }, { status: 400 })
}

StudentSchema.safeParse(body)
const parsed = StudentSchema.safeParse(body)
if (!parsed.success) return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 })

const student = await Student.create({ ...(body as Record<string, unknown>), teacherId: userId })
const student = await Student.create({ ...parsed.data, teacherId: userId })
return NextResponse.json(student, { status: 201 })
} catch (error) {
if (error instanceof Error) {
Expand Down
9 changes: 7 additions & 2 deletions app/dashboard/OverviewClient.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ export function OverviewClient() {
fetch("/api/announcements?limit=5"),
]);

// Check that all responses are OK before processing
for (const res of [studentsRes, assignmentsRes, attendanceRes, gradesRes, announcementsRes]) {
if (!res.ok) throw new Error(`API error: ${res.url} returned ${res.status}`);
}

const [students, assignmentsData, attendance, grades, announcements] =
await Promise.all([
studentsRes.json(),
Expand Down Expand Up @@ -254,7 +259,7 @@ export function OverviewClient() {
"B+": 8,
B: 7,
C: 6,
D: 4,
D: 5,
F: 0,
};
const termMap: Record<string, number[]> = {};
Expand Down Expand Up @@ -316,7 +321,7 @@ export function OverviewClient() {
.slice(0, 5);

setStats({
totalStudents: students.students?.length ?? 0,
totalStudents: students.total ?? students.students?.length ?? 0,
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the response shape of /api/assignments to see if a `total` field exists
fd -t f 'route.(ts|js)' app/api/assignments | xargs -I{} sh -c 'echo "=== {} ==="; cat {}'

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 5767

Apply the same totalAssignments fix as used for totalStudents.

The /api/assignments endpoint returns a paginated response shape { assignments, total, page, pages } (confirmed in app/api/assignments/route.ts). Using assignments.total instead of deriving totalAssignments from the (possibly truncated) assignments array will prevent under-reporting, matching the fix applied to totalStudents.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/dashboard/OverviewClient.tsx` at line 324, Update totalAssignments to use
the paginated total field rather than deriving from the possibly truncated
array: replace the current expression that uses assignments.assignments?.length
with an expression that prefers assignments.total (e.g., assignments.total ??
assignments.assignments?.length ?? 0). Modify the totalAssignments assignment in
OverviewClient (where totalStudents was fixed) so it mirrors the totalStudents
pattern and uses the paginated response's total property.

totalAssignments: Array.isArray(assignments)
? assignments.length
: (assignments.length ?? 0),
Expand Down
2 changes: 2 additions & 0 deletions app/dashboard/announcements/AnnouncementsClient.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ const CATEGORY_BADGE: Record<string, 'info' | 'success' | 'warning' | 'purple' |

function timeAgo(date: string) {
const diff = Date.now() - new Date(date).getTime()
if (diff < 0 || isNaN(diff)) return 'Just now'
const mins = Math.floor(diff / 60000)
if (mins < 1) return 'Just now'
if (mins < 60) return `${mins}m ago`
const hrs = Math.floor(mins / 60)
if (hrs < 24) return `${hrs}h ago`
Expand Down
1 change: 1 addition & 0 deletions app/dashboard/grades/GradesClient.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ function sortTerms(terms: string[]) {
}

function pct(marks: number, max: number) {
if (max <= 0) return 0;
return Math.round((marks / max) * 100);
}

Expand Down
12 changes: 8 additions & 4 deletions models/Grade.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ GradeSchema.pre("save", function () {
GradeSchema.pre("findOneAndUpdate", function () {
const update = this.getUpdate() as Record<string, unknown>;
if (update && typeof update === "object") {
const marks = update.marks;
const maxMarks = update.maxMarks;
// Support both direct updates and $set operator
const source = (update.$set && typeof update.$set === "object" ? update.$set : update) as Record<string, unknown>;
const marks = source.marks;
const maxMarks = source.maxMarks;
if (
marks !== undefined && typeof marks === "number" &&
maxMarks !== undefined && typeof maxMarks === "number" &&
Expand All @@ -56,8 +58,10 @@ GradeSchema.pre("findOneAndUpdate", function () {
GradeSchema.pre("updateOne", function () {
const update = this.getUpdate() as Record<string, unknown>;
if (update && typeof update === "object") {
const marks = update.marks;
const maxMarks = update.maxMarks;
// Support both direct updates and $set operator
const source = (update.$set && typeof update.$set === "object" ? update.$set : update) as Record<string, unknown>;
const marks = source.marks;
const maxMarks = source.maxMarks;
if (
marks !== undefined && typeof marks === "number" &&
maxMarks !== undefined && typeof maxMarks === "number" &&
Expand Down
Loading