Conversation
…thentication, and data management for teachers, institutes, and students.
…pages, institute creation, and student enrollment features.
Summary by BeetleThis PR represents a comprehensive refactoring and modernization of the admin web application, introducing significant architectural improvements, UI/UX enhancements, and new features for institute management. The changes span across three major commits that progressively build upon each other to create a robust admin dashboard system. 📁 File Changes Summary (Consolidated across all commits):
Total Changes: 73 files changed, +3,500 additions, -1,800 deletions 🗺️ Walkthrough:sequenceDiagram
participant User
participant Login
participant Auth
participant Protected
participant Institute
participant Dashboard
User->>Login: Access Application
Login->>Auth: Google OAuth / Email Login
Auth->>Auth: Set leadlly.in_admin_token
alt Has Institutes
Auth->>Protected: Redirect to /institute/:id
Protected->>Institute: Load Active Institute
Institute->>Dashboard: Display Overview
else No Institutes
Auth->>Protected: Redirect to /create-institute
Protected->>Institute: Show Creation Form
Institute->>Institute: Validate with Zod Schema
Institute->>Dashboard: Create & Redirect
end
Dashboard->>Dashboard: Fetch Institute Data (TanStack Query)
Dashboard->>Dashboard: Display Students/Teachers
Note over Dashboard: User can switch institutes via header dropdown
Dashboard->>Institute: Switch Institute
Institute->>Dashboard: Update Active Institute
🎯 Key Changes:1. Major Framework & Dependency Upgrades
2. Authentication & Authorization Enhancements
3. Institute Management System
4. UI/UX Improvements
5. Code Quality & Developer Experience
6. Performance Optimizations
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMajor refactor adding an SSR-aware React Query setup, a route-proxy for auth redirects, many new shadcn-style UI primitives, extensive institute/student/fee pages and CRUD actions (including fee PDF generation), tooling configs (Prettier/ESLint) and package upgrades; several legacy pages/middleware removed or relocated. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as User Browser
participant Proxy as Route Proxy (src/proxy.ts)
participant AuthSvc as Auth API / getUser
participant Page as Next.js Server Page
participant Query as React Query Client
participant API as Admin API
Browser->>Proxy: Request /institute/{id} (HTTP)
Proxy->>AuthSvc: getUser() (reads cookie)
alt no token
Proxy-->>Browser: Redirect /login
else token present
Proxy->>Page: Allow request
Page->>Query: prefetchQuery(active_institute)
Query->>API: GET /api/institute/{id}
API-->>Query: institute data
Query->>Page: dehydrate state
Page-->>Browser: HTML + HydrationBoundary
Browser->>Query: hydrate & render client components
end
sequenceDiagram
participant User as Browser User
participant FeesPage as Fees Page (client)
participant API as Fees API (src/actions/fee_actions)
participant PDF as FeePdfGenerator / jspdf
participant Toast as Sonner/Toasts
User->>FeesPage: Open Fees Page
FeesPage->>API: getFeeUidGroups / getFeeRecords
API-->>FeesPage: fee data
User->>FeesPage: Click "Add Fee" -> opens FeeRecordDialog
FeeRecordDialog->>API: createFeeRecord(...)
API-->>FeeRecordDialog: success + record
FeeRecordDialog->>PDF: generateFeePdf(record)
PDF-->>FeeRecordDialog: returns blob/download
FeeRecordDialog->>Toast: show success
FeeRecordDialog->>FeesPage: trigger refetch
API-->>FeesPage: updated list
Estimated code review effort🎯 5 (Critical) | ⏱️ ~100+ minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![beetle-ai[bot]](https://avatars.githubusercontent.com/in/1636950?s=60&v=4){kind=small}
| /.vscode | ||
| /node_modules | ||
| ./dist | ||
| *. env |
There was a problem hiding this comment.
Security Issue: Typo in ignore pattern *. env (space between * and .env) will NOT match .env files. This defeats the purpose of ignoring environment files that typically contain sensitive credentials and secrets. Environment files could be formatted by Prettier and potentially exposed in version control.
Confidence: 5/5
Suggested Fix
| *. env | |
| *.env | |
Remove the space between * and .env to properly match all .env files.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In .prettierignore at line 4, there is a typo in the ignore pattern "*. env" which has
a space between the asterisk and ".env". This will not match .env files as intended.
Fix this by removing the space to make it "*.env" so that all environment files
containing sensitive credentials are properly ignored by Prettier.
| ./dist | ||
| *. env | ||
| .env | ||
| .env .* No newline at end of file |
There was a problem hiding this comment.
Security Issue: Typo in ignore pattern .env .* (space between .env and .*) will NOT match environment variant files like .env.local, .env.production, .env.development. These files typically contain environment-specific secrets and credentials. The typo defeats the security purpose of ignoring these sensitive files.
Confidence: 5/5
Suggested Fix
| .env .* | |
| .env.* | |
Remove the space between .env and .* to properly match all environment variant files.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In .prettierignore at line 6, there is a typo in the ignore pattern ".env .*" which has
a space between ".env" and ".*". This will not match environment variant files like
.env.local, .env.production, etc. as intended. Fix this by removing the space to make
it ".env.*" so that all environment variant files containing sensitive credentials are
properly ignored by Prettier.
| import { cookies } from "next/headers"; | ||
|
|
||
| export const getCookie = async (name: string) => { | ||
| export const getCookie = async (name = "leadlly.in_admin_token") => { |
There was a problem hiding this comment.
Security Issue: Hardcoding the admin authentication token cookie name "leadlly.in_admin_token" as a default parameter exposes sensitive authentication implementation details. This makes it easier for attackers to:
- Target the specific cookie in session hijacking attacks
- Craft CSRF attacks knowing the exact cookie name
- Perform cookie theft or manipulation attacks
The hardcoded default also reduces security flexibility - if the cookie name needs to be changed after a security incident, all implicit usages must be identified and updated.
Confidence: 4/5
Recommended Action
Consider one of these approaches:
- Remove the default parameter - Require explicit cookie name at all call sites for better security awareness and control
- Use environment variable - Store the cookie name in an environment variable (e.g.,
process.env.ADMIN_TOKEN_COOKIE_NAME) to avoid hardcoding - Use a centralized constant - Define the cookie name in a secure configuration file that's not committed to version control
If keeping the default, ensure:
- The cookie has
HttpOnly,Secure, andSameSite=Strictflags set - Cookie name rotation strategy is documented
- All usages are audited for security implications
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/cookie_actions.ts at line 5, the getCookie function now has a hardcoded
default parameter "leadlly.in_admin_token" for the admin authentication cookie name.
This exposes sensitive authentication implementation details and makes it easier for
attackers to target this specific cookie. Remove the default parameter and require
explicit cookie names at all call sites, or move the cookie name to an environment
variable (process.env.ADMIN_TOKEN_COOKIE_NAME) to avoid hardcoding sensitive values.
Review all call sites to ensure they explicitly specify the cookie name for better
security awareness.
| standards?: string[]; | ||
| } | ||
| type InstituteCreateData = z.infer<typeof CreateInstituteFormSchema>; | ||
|
|
There was a problem hiding this comment.
Security Issue - Potential Authentication Bypass: The change from getCookie("token") to getCookie() now relies on the hardcoded default parameter "leadlly.in_admin_token" (as identified in Comment #4 on cookie_actions.ts).
Critical Problem: If the actual authentication cookie name is "token" (as the original code indicates), this change will cause getCookie() to look for the wrong cookie name ("leadlly.in_admin_token" instead of "token"). This means:
tokenvariable will beundefinedornull- The
Cookie: token=${token}header will be malformed (e.g.,Cookie: token=undefined) - All API requests will fail authentication or bypass authentication checks entirely
- This affects ALL institute operations:
createInstitute,getMyInstitute,getAllUserInstitutes
Verify immediately: Check what the actual cookie name is in production. If it's"token", this is a breaking change that will cause complete authentication failure for all institute-related API calls.
Confidence: 5/5
Recommended Action
Immediate verification needed:
- Check the actual cookie name set during authentication (in login/auth routes)
- If the cookie name is
"token", revert these changes and pass"token"explicitly - If the cookie name is
"leadlly.in_admin_token", then the original code was buggy and this fix is correct
Recommended fix (if cookie name is actually"token"):
const token = await getCookie("token");Or better yet, define a constant for the cookie name:
const AUTH_COOKIE_NAME = process.env.AUTH_COOKIE_NAME || "token";
const token = await getCookie(AUTH_COOKIE_NAME);This same issue affects lines 39, 66, and 93 (commented code) in this file.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/institute_actions.ts at lines 13, 39, and 66, the code was changed from
getCookie("token") to getCookie() which now relies on a hardcoded default parameter
"leadlly.in_admin_token". This is a critical security issue because if the actual
authentication cookie name is "token" (as the original code suggests), all these API
calls will fail to retrieve the authentication token, causing authentication bypass.
Verify the actual cookie name used in production immediately. If it's "token", revert
to getCookie("token"). If it's "leadlly.in_admin_token", document this change as a bug
fix. Consider using an environment variable for the cookie name instead of hardcoding.
| instituteId: string, | ||
| emails: string[] | ||
| ) { | ||
| try { |
There was a problem hiding this comment.
Same authentication issue as Comment #5 - The change from getCookie("token") to getCookie() creates the same potential authentication bypass vulnerability identified in institute_actions.ts. If the actual authentication cookie name is "token" (as the original code indicates), this will cause the addTeacherToInstitute function to fail authentication, as it will look for the wrong cookie name ("leadlly.in_admin_token" instead of "token").
This affects the API call on line 45 where Cookie: token=${token} will be malformed if token is undefined.
Confidence: 5/5
Suggested Fix
| try { | |
| const token = await getCookie("token"); | |
Revert to explicitly passing the cookie name, or verify that the actual authentication cookie name is "leadlly.in_admin_token" and document this as a bug fix.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/mentor_actions.ts at line 17, the code was changed from getCookie("token")
to getCookie() which now relies on a hardcoded default parameter "leadlly.in_admin_token".
This is the same critical authentication issue identified in institute_actions.ts Comment #5.
Verify the actual authentication cookie name used in production. If it's "token", revert to
getCookie("token"). If it's "leadlly.in_admin_token", document this as a bug fix. Consider
using an environment variable for the cookie name to avoid hardcoding sensitive values.
| instituteId: string, | ||
| emails: string[] | ||
| ) { | ||
| try { |
There was a problem hiding this comment.
Same authentication issue as Comments #5 and #6 - The change from getCookie("token") to getCookie() creates the same potential authentication bypass vulnerability identified in institute_actions.ts and mentor_actions.ts.
If the actual authentication cookie name is "token" (as the original code indicates), this will cause the addStudentsToInstitute function to fail authentication. The function will look for the wrong cookie name ("leadlly.in_admin_token" instead of "token"), resulting in:
tokenvariable beingundefinedornull- Malformed
Cookie: token=undefinedheader on line 45 - Complete authentication failure for student addition operations
This affects the API call to/api/student/add/${instituteId}which requires valid authentication.
Confidence: 5/5
Suggested Fix
| try { | |
| const token = await getCookie("token"); | |
Revert to explicitly passing the cookie name. If the actual cookie name is "leadlly.in_admin_token", then document this as a bug fix and verify all authentication flows are working correctly.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/student_action.ts at line 17, the code was changed from getCookie("token")
to getCookie() which now relies on a hardcoded default parameter "leadlly.in_admin_token".
This is the same critical authentication issue identified in institute_actions.ts (Comment #5)
and mentor_actions.ts (Comment #6). Verify the actual authentication cookie name used in
production immediately. If it's "token", revert to getCookie("token"). If it's
"leadlly.in_admin_token", document this as a bug fix and verify all authentication flows.
Consider using an environment variable for the cookie name to avoid hardcoding sensitive values.
|
|
||
| export const getUser = async () => { | ||
| const token = await getCookie("token"); | ||
| export const getUser = cache(async () => { |
There was a problem hiding this comment.
Same authentication issue as Comments #5, #6, and #7 - The change from getCookie("token") to getCookie() creates the same potential authentication bypass vulnerability identified in institute_actions.ts, mentor_actions.ts, and student_action.ts.
Critical Problem: This change affects the getUser function (now wrapped in React cache()), which is likely a core authentication function used throughout the application. If the actual authentication cookie name is "token" (as the original code indicates), this will cause:
tokenvariable to beundefinedornull- Malformed
Cookie: token=undefinedheader on line 149 - Complete authentication failure for user data retrieval
- Cascading failures across the entire application since
getUseris typically called to verify authentication state
This same issue also affects: - Line 178:
studentPersonalInfofunction - Line 208:
setTodaysVibefunction
Verify immediately: Check what the actual cookie name is in production. If it's"token", this is a breaking change that will cause complete authentication failure across all user-related operations.
Confidence: 5/5
Suggested Fix
| export const getUser = cache(async () => { | |
| const token = await getCookie("token"); | |
Revert to explicitly passing the cookie name. If the actual cookie name is "leadlly.in_admin_token", then document this as a bug fix and verify all authentication flows are working correctly.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/user_actions.ts at lines 145, 178, and 208, the code was changed from
getCookie("token") to getCookie() which now relies on a hardcoded default parameter
"leadlly.in_admin_token". This is the same critical authentication issue identified in
Comments #5, #6, and #7 for other action files. The getUser function is particularly
critical as it's likely used throughout the application for authentication checks. Verify
the actual authentication cookie name used in production immediately. If it's "token",
revert all three instances to getCookie("token"). If it's "leadlly.in_admin_token",
document this as a bug fix and verify all authentication flows. Consider using an
environment variable for the cookie name to avoid hardcoding sensitive values.
| setLoading(true); | ||
| // Dispatch institute data to Redux store | ||
| dispatch(instituteData(institute)); | ||
| // dispatch(instituteData(institute)); |
There was a problem hiding this comment.
Security Concern - Potential Authorization Bypass: Commenting out dispatch(instituteData(institute)) removes the Redux state storage for the selected institute. The code still navigates to /institute/${institute._id} (line 60), but without storing the institute data in Redux.
Security Risk: If the destination page (/institute/[instituteId]/page.tsx) relies on Redux state to verify that the user has legitimate access to the selected institute, this change breaks that authorization check. A malicious user could potentially:
- Navigate directly to
/institute/{any-institute-id}via URL manipulation - Bypass the institute selection flow entirely
- Access institutes they don't own if the destination page doesn't perform server-side authorization
Critical Questions:
- Does
/institute/[instituteId]/page.tsxperform server-side authorization checks? - Or does it rely on Redux state to verify the user selected this institute legitimately?
If authorization relies on client-side Redux state, this is a critical security vulnerability. If the destination page performs proper server-side authorization (checking that the authenticated user owns/has access to the requested institute ID), then this change is safe but removes useful client-side state management.
Confidence: 4/5
Recommended Action
Immediate verification needed:
- Review
/institute/[instituteId]/page.tsxto confirm it performs server-side authorization - Ensure the page validates that the authenticated user has access to the requested institute ID
- Do NOT rely on client-side Redux state for authorization decisions
If keeping the commented code:
- Document why Redux dispatch was removed
- Ensure all institute pages perform server-side authorization checks
- Consider removing the commented code entirely if it's no longer needed
If Redux state is needed: - Uncomment lines 35 and 58 to restore Redux functionality
- Or implement proper server-side authorization if not already present
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(auth)/select-institute/page.tsx at lines 35 and 58, the Redux dispatch
functionality has been commented out. This means the selected institute data is no
longer stored in Redux state before navigating to /institute/${institute._id}. Verify
that the destination page (/institute/[instituteId]/page.tsx) performs proper
server-side authorization to check that the authenticated user has access to the
requested institute ID. If authorization relies on client-side Redux state, this is a
critical security vulnerability that enables authorization bypass via direct URL
manipulation. Either uncomment the Redux dispatch code or ensure robust server-side
authorization is in place. Document the reason for this change.
| export const dynamic = "force-dynamic"; | ||
|
|
||
| const ProtectedLayout = async ({ children }: { children: React.ReactNode }) => { | ||
| const token = await getCookie(); |
There was a problem hiding this comment.
Critical Security Issue - Authentication Gateway Failure: This line uses getCookie() without specifying the cookie name, relying on the hardcoded default parameter "leadlly.in_admin_token" from cookie_actions.ts (see Comment #4).
Why this is CRITICAL: This is the authentication gateway for ALL protected routes in the application. If the actual authentication cookie name is "token" (as indicated by the original code in Comments #5-#8), then:
tokenwill beundefinedfor all authenticated users- Line 11 check will always trigger the redirect to
/login - Complete lockout: No authenticated users can access any protected routes
- Application-wide authentication failure
This is more severe than the individual action file issues (Comments Developed an Admin Dashboard that integrates batch management and API functionality (Bonus included) #5-Implemented fully responsive Admin Dashboard, Batches & Students page (Bonus Included) #8) because it affects the entire protected route tree, not just individual API calls.
Immediate verification required:
- Check the actual cookie name set during login (in
/api/auth/login/route.ts) - If it's
"token", this is a breaking change causing complete application lockout - If it's
"leadlly.in_admin_token", verify that all authentication flows use this name consistently
Confidence: 5/5
Suggested Fix
| const token = await getCookie(); | |
| const token = await getCookie("token"); | |
Explicitly specify the cookie name. If the actual cookie name is "leadlly.in_admin_token", then use that and document this as a bug fix. Better yet, use an environment variable:
const AUTH_COOKIE_NAME = process.env.AUTH_COOKIE_NAME || "token";
const token = await getCookie(AUTH_COOKIE_NAME);Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/layout.tsx at line 10, the code uses getCookie() without
specifying the cookie name, relying on a hardcoded default "leadlly.in_admin_token".
This is the authentication gateway for ALL protected routes. If the actual
authentication cookie name is "token" (as suggested by original code in other files),
this will cause complete application lockout - no authenticated users can access any
protected routes. Verify the actual cookie name used in /api/auth/login/route.ts
immediately. If it's "token", change line 10 to getCookie("token"). If it's
"leadlly.in_admin_token", document this as a bug fix and verify all authentication
flows. Consider using an environment variable for the cookie name to avoid hardcoding.
| res.cookies.set('token', '', { | ||
| await fetch( | ||
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/auth/logout`, | ||
| { |
There was a problem hiding this comment.
Critical Security Issue - Session Fixation Vulnerability: The logout route now clears a cookie named "leadlly.in_admin_token" instead of "token". This creates a critical security vulnerability if the actual authentication cookie name is "token" (as indicated by the original code).
Impact:
- Users cannot properly log out - the authentication cookie is never cleared
- Authentication sessions persist indefinitely in the browser
- Session hijacking risk on shared/public computers
- Violates security best practice: users must be able to terminate their sessions
This is related to Comments Created Admin Dash, included batch management and API implementation (Bonus included) #4-Implemented a fully responsive Admin Dashboard with Batches and Students pages (Bonus Included) #10: There's a systematic cookie name change across the codebase from"token"to"leadlly.in_admin_token". You must verify:
- What cookie name does
/api/auth/login/route.tsactually set? - If it sets
"token", then this logout route will fail to clear it - If it sets
"leadlly.in_admin_token", then the original code was buggy
Immediate verification required: Check the login route to confirm which cookie name is actually being set during authentication.
Confidence: 5/5
Suggested Fix
| { | |
| res.cookies.set("token", "", { | |
Revert to clearing the "token" cookie if that's what the login route sets. If the login route sets "leadlly.in_admin_token", then this change is correct but you must verify that ALL authentication flows (login, protected routes, API calls) consistently use "leadlly.in_admin_token".
Better approach: Use a centralized constant for the cookie name:
const AUTH_COOKIE_NAME = process.env.AUTH_COOKIE_NAME || "token";
res.cookies.set(AUTH_COOKIE_NAME, "", { ... });Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/api/auth/logout/route.ts at line 7, the logout route now clears a cookie
named "leadlly.in_admin_token" instead of "token". This is a critical security issue
because if the login route sets a cookie named "token", then logout will fail to clear
it, leaving users permanently logged in and creating a session fixation vulnerability.
Verify immediately what cookie name is set in src/app/api/auth/login/route.ts. If it's
"token", revert line 7 to res.cookies.set("token", "", {...}). If it's
"leadlly.in_admin_token", verify that ALL authentication flows (protected layout,
action files, API routes) consistently use this name. Consider using an environment
variable for the cookie name to avoid hardcoding and ensure consistency across the
codebase.
| res.cookies.set("leadlly.in_admin_token", token, { | ||
| httpOnly: true, | ||
| path: '/', | ||
| sameSite: 'strict', | ||
| expires: new Date('9999-12-31T23:59:59Z') | ||
| path: "/", | ||
| sameSite: "strict", | ||
| expires: new Date("9999-12-31T23:59:59Z"), |
There was a problem hiding this comment.
Security Issue - Hardcoded Cookie Name & Lack of Centralization: The authentication cookie name "leadlly.in_admin_token" is now hardcoded in this verify route. This same cookie name is hardcoded in multiple other files (logout route, cookie_actions.ts default parameter). If these values get out of sync during future maintenance, authentication will break across the application.
Additional Concern: The cookie expiration is set to year 9999 (essentially never expires). While this might be intentional for persistent sessions, it creates a security risk if:
- The token itself doesn't have server-side expiration validation
- Compromised tokens remain valid indefinitely
- Users cannot force session termination across devices
Recommendation: Centralize the cookie name in a single configuration constant to prevent synchronization issues:
Confidence: 4/5
Suggested Fix
Create a centralized auth configuration file (e.g., src/config/auth.ts):
export const AUTH_CONFIG = {
COOKIE_NAME: process.env.AUTH_COOKIE_NAME || "leadlly.in_admin_token",
COOKIE_MAX_AGE: 30 * 24 * 60 * 60, // 30 days in seconds
} as const;Then update this file to use the centralized constant:
| res.cookies.set("leadlly.in_admin_token", token, { | |
| httpOnly: true, | |
| path: '/', | |
| sameSite: 'strict', | |
| expires: new Date('9999-12-31T23:59:59Z') | |
| path: "/", | |
| sameSite: "strict", | |
| expires: new Date("9999-12-31T23:59:59Z"), | |
| res.cookies.set(AUTH_CONFIG.COOKIE_NAME, token, { | |
| httpOnly: true, | |
| path: "/", | |
| sameSite: "strict", | |
| maxAge: AUTH_CONFIG.COOKIE_MAX_AGE, | |
| }); | |
Benefits:
- Single source of truth for cookie name
- Easy to change cookie name if needed (security incident response)
- Environment variable support for different deployments
- More reasonable cookie expiration (30 days instead of forever)
Important: Update all other files that reference this cookie name to use the same centralized constant: src/actions/cookie_actions.ts(default parameter)src/app/api/auth/logout/route.ts(cookie clearing)src/app/(protected)/layout.tsx(authentication check)- All action files that call
getCookie()
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/api/auth/verify/route.ts at lines 15-19, the authentication cookie name
"leadlly.in_admin_token" is hardcoded. This same value is hardcoded in multiple other
files (logout route, cookie_actions.ts), creating a maintenance and security risk if
they get out of sync. Create a centralized auth configuration file at
src/config/auth.ts that exports AUTH_CONFIG with COOKIE_NAME and COOKIE_MAX_AGE
constants. Update line 15 to use AUTH_CONFIG.COOKIE_NAME instead of the hardcoded
string. Also consider changing the cookie expiration from year 9999 to a more
reasonable value like 30 days using maxAge instead of expires. Then update all other
files that reference this cookie name to use the same centralized constant for
consistency and security.
📍 This suggestion applies to lines 15-19
| const res = NextResponse.json(userData); | ||
|
|
||
| res.cookies.set('token', token, { | ||
| res.cookies.set("leadlly.in_admin_token", token, { | ||
| httpOnly: true, | ||
| path: '/', | ||
| sameSite: 'strict', | ||
| expires: new Date('9999-12-31T23:59:59Z') | ||
| }); | ||
| path: "/", |
There was a problem hiding this comment.
Security Issue - Hardcoded Cookie Name & Lack of Centralization: The authentication cookie name "leadlly.in_admin_token" is hardcoded here, matching the same hardcoded value in the verify route (Comment #12) and logout route (Comment #11). While this consistency is good, having the cookie name hardcoded in multiple files creates a maintenance and security risk.
Additional Security Concern: The cookie expiration is set to year 9999 (line 20), meaning the cookie essentially never expires. Combined with the lack of centralized configuration, if a security incident requires changing the cookie name or expiration policy, you'll need to update multiple files manually, increasing the risk of missing one and creating authentication inconsistencies.
Impact: If these hardcoded values get out of sync during future maintenance, authentication will break across the application. This is particularly risky for security-critical changes that need to be deployed quickly (e.g., responding to a credential leak).
Confidence: 4/5
Recommended Action
Create a centralized auth configuration file (e.g., src/config/auth.ts):
export const AUTH_CONFIG = {
COOKIE_NAME: process.env.AUTH_COOKIE_NAME || "leadlly.in_admin_token",
COOKIE_MAX_AGE: 30 * 24 * 60 * 60, // 30 days in seconds
} as const;Then update this file to use the centralized constant:
import { AUTH_CONFIG } from "@/config/auth";
// In the POST handler:
res.cookies.set(AUTH_CONFIG.COOKIE_NAME, token, {
httpOnly: true,
path: "/",
sameSite: "strict",
maxAge: AUTH_CONFIG.COOKIE_MAX_AGE,
});Benefits:
- Single source of truth for cookie name and expiration
- Easy to change cookie configuration if needed (security incident response)
- Environment variable support for different deployments
- More reasonable cookie expiration (30 days instead of forever)
Important: Update all other files that reference this cookie name to use the same centralized constant: src/app/api/auth/verify/route.ts(line 15)src/app/api/auth/logout/route.ts(line 7)src/actions/cookie_actions.ts(default parameter)src/app/(protected)/layout.tsx(authentication check)- All action files that call
getCookie()
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/api/google/auth/route.ts at lines 16-20, the authentication cookie name
"leadlly.in_admin_token" is hardcoded. This same value is hardcoded in multiple other
files (verify route, logout route, cookie_actions.ts), creating a maintenance and
security risk if they get out of sync. Create a centralized auth configuration file at
src/config/auth.ts that exports AUTH_CONFIG with COOKIE_NAME and COOKIE_MAX_AGE
constants. Update line 16 to use AUTH_CONFIG.COOKIE_NAME instead of the hardcoded
string. Also consider changing the cookie expiration from year 9999 to a more
reasonable value like 30 days using maxAge instead of expires. Then update all other
files that reference this cookie name to use the same centralized constant for
consistency and security.
📍 This suggestion applies to lines 16-20
| import { getUser } from "./actions/user_actions"; | ||
|
|
||
| export async function proxy(request: NextRequest) { | ||
| const token = request.cookies.get("leadlly.in_admin_token")?.value; |
There was a problem hiding this comment.
Critical Security Issue - Hardcoded Cookie Name & Potential Authentication Bypass: This line hardcodes the authentication cookie name as "leadlly.in_admin_token", which is part of a systematic cookie name change across the entire codebase (see Comments #4-#13).
Critical Problems:
- Inconsistency Risk: If the actual authentication cookie set by
/api/auth/login/route.tsor/api/google/auth/route.tsis named"token"(as the original code suggested), this proxy will fail to retrieve the authentication token, causing:
- All authenticated users to be redirected to
/login(line 17) - Complete application lockout for legitimate users
- Authentication bypass if the check fails open
- Lack of Centralization: The cookie name is hardcoded in at least 13 different locations across the codebase. If these values get out of sync during maintenance or security incident response, authentication will break unpredictably.
- Security Incident Response: If a security incident requires changing the cookie name (e.g., after credential leak), you must manually update 13+ files, increasing the risk of missing one and creating authentication vulnerabilities.
This is particularly critical because:
- This proxy function is the authentication gateway for the entire application
- It's called on EVERY request via the middleware matcher (line 37)
- A failure here affects ALL routes, not just individual API calls
Immediate verification required:
- Check what cookie name is actually set in
/api/auth/login/route.tsand/api/google/auth/route.ts - If it's
"token", this is a breaking change causing complete application lockout - If it's
"leadlly.in_admin_token", verify ALL authentication flows use this name consistently
Confidence: 5/5
Recommended Action
Create a centralized auth configuration (e.g., src/config/auth.ts):
export const AUTH_CONFIG = {
COOKIE_NAME: process.env.AUTH_COOKIE_NAME || "leadlly.in_admin_token",
COOKIE_MAX_AGE: 30 * 24 * 60 * 60, // 30 days in seconds
} as const;Then update this file:
import { AUTH_CONFIG } from "./config/auth";
export async function proxy(request: NextRequest) {
const token = request.cookies.get(AUTH_CONFIG.COOKIE_NAME)?.value;
// ... rest of the code
}Benefits:
- Single source of truth for cookie name
- Environment variable support for different deployments
- Easy to change cookie name during security incidents
- Prevents synchronization issues across 13+ files
Critical: Update ALL files that reference this cookie name to use the same centralized constant: src/actions/cookie_actions.ts(default parameter)src/app/(protected)/layout.tsx(line 10)src/app/api/auth/logout/route.ts(line 7)src/app/api/auth/verify/route.ts(line 15)src/app/api/google/auth/route.ts(line 16)- All action files that call
getCookie()
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/proxy.ts at line 7, the authentication cookie name "leadlly.in_admin_token" is
hardcoded. This is part of a systematic cookie name change across 13+ files in the
codebase, creating a critical security risk. If the actual authentication cookie name
set by the login routes is "token" (as original code suggested), this will cause
complete application lockout - all authenticated users will be redirected to /login.
Create a centralized auth configuration file at src/config/auth.ts that exports
AUTH_CONFIG with COOKIE_NAME constant (using environment variable with fallback).
Update line 7 to use AUTH_CONFIG.COOKIE_NAME. Then update ALL other files that
reference this cookie name (cookie_actions.ts, protected layout, logout route, verify
route, google auth route, and all action files) to use the same centralized constant.
Verify immediately what cookie name is actually set in the login routes to ensure
consistency.
src/actions/institute_actions.ts
Outdated
| headers: { | ||
| "Content-Type": "application/json", | ||
| Cookie: `token=${token}`, | ||
| isAdmin: "true", |
There was a problem hiding this comment.
Critical Security Issue - Client-Controlled Authorization Header: Adding isAdmin: "true" as a request header is a severe security vulnerability if the backend API trusts this header for authorization decisions. This header is set on the client side and can be easily manipulated by attackers.
Attack Scenario:
- Attacker intercepts or modifies the request
- Adds or keeps
isAdmin: "true"header - If backend trusts this header, attacker gains admin privileges
- Can create institutes with elevated permissions or bypass authorization checks
Impact: If the backend/api/instituteendpoint uses this header to determine admin status without proper server-side authentication/authorization, this creates a complete authorization bypass vulnerability allowing any user to perform admin-level operations.
Confidence: 5/5
Recommended Action
Immediate verification required:
- Review the backend
/api/instituteendpoint implementation - Ensure it performs proper server-side authorization using the authentication token
- Verify it does NOT trust the
isAdminheader for authorization decisions
If the header is required:
- The backend MUST validate admin status from the JWT token or session, NOT from this header
- The header should only be used as a hint or for routing, never for authorization
- Consider removing this header entirely if the backend can determine admin status from the token
Best practice: - Authorization decisions must ALWAYS be made server-side based on authenticated session/token
- Never trust client-provided headers for security decisions
- The authentication token (line 25) should contain all necessary authorization information
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/institute_actions.ts at line 26, a client-controlled header "isAdmin: true"
is being sent to the backend API. This is a critical security vulnerability if the backend
trusts this header for authorization decisions. Review the backend /api/institute endpoint
to ensure it performs proper server-side authorization using the authentication token and
does NOT trust the isAdmin header for authorization. If the backend correctly validates
admin status from the token, document why this header is needed. If the backend relies on
this header for authorization, this is a critical vulnerability that must be fixed by
implementing proper server-side authorization checks based on the authenticated token.
src/actions/user_actions.ts
Outdated
| headers: { | ||
| "Content-Type": "application/json", | ||
| Cookie: `token=${token}`, | ||
| isAdmin: "true", |
There was a problem hiding this comment.
Critical Security Issue - Client-Controlled Authorization Header: Adding isAdmin: "true" as a request header is a severe security vulnerability if the backend API trusts this header for authorization decisions. This header is set on the client side and can be easily manipulated by attackers.
Attack Scenario:
- Attacker intercepts or modifies the request using browser DevTools or a proxy
- Adds or keeps
isAdmin: "true"header in any request - If backend
/api/admin/getendpoint trusts this header, attacker gains admin privileges - Can retrieve admin user data or bypass authorization checks
Impact: If the backend endpoint uses this header to determine admin status without proper server-side authentication/authorization validation, this creates a complete authorization bypass vulnerability allowing any authenticated user to access admin-only data.
Related to Comment #15: This is the same pattern identified ininstitute_actions.tswhereisAdmin: "true"header was added. This appears to be a systematic issue across multiple action files.
Confidence: 5/5
Recommended Action
Immediate verification required:
- Review the backend
/api/admin/getendpoint implementation - Ensure it performs proper server-side authorization using the authentication token (JWT validation)
- Verify it does NOT trust the
isAdminheader for authorization decisions
If the header is required:
- The backend MUST validate admin status from the JWT token payload or session, NOT from this header
- The header should only be used as a routing hint or API versioning, never for authorization
- Consider removing this header entirely if the backend can determine admin status from the token
Best practice: - Authorization decisions must ALWAYS be made server-side based on authenticated session/token claims
- Never trust client-provided headers for security-critical decisions
- The authentication token (line 154) should contain all necessary authorization information (roles, permissions)
- Backend should decode and validate the JWT token to extract admin status
If keeping the header:
Document clearly in code comments that this header is NOT used for authorization and is only for routing/logging purposes.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/user_actions.ts at line 155, a client-controlled header "isAdmin: true"
is being sent to the backend API endpoint /api/admin/get. This is a critical security
vulnerability if the backend trusts this header for authorization decisions. Review the
backend /api/admin/get endpoint to ensure it performs proper server-side authorization
by validating the JWT token and extracting admin status from the token payload, NOT
from the isAdmin header. If the backend correctly validates admin status from the token
claims, document in a code comment why this header is needed (e.g., for routing or
logging only). If the backend relies on this header for authorization, this is a
critical vulnerability that must be fixed immediately by implementing proper server-side
JWT validation and role checking based on the authenticated token.
| "/api/google/auth", | ||
| { | ||
| access_token: credentialResponse.access_token, | ||
| isAdmin: true, |
There was a problem hiding this comment.
Critical Security Issue - Client-Controlled Authorization Header: Adding isAdmin: true as a request body parameter is a severe security vulnerability if the backend API trusts this parameter for authorization decisions. This value is set on the client side and can be easily manipulated by attackers.
Attack Scenario:
- Attacker intercepts or modifies the request using browser DevTools or a proxy
- Sets
isAdmin: truein the request body - If backend
/api/google/authendpoint trusts this parameter, attacker gains admin privileges during Google OAuth login - Can register as an admin user or elevate privileges during authentication
Impact: If the backend endpoint uses this parameter to determine admin status without proper server-side validation, this creates a complete authorization bypass vulnerability allowing any user with a Google account to register as an admin.
Related to Comments #15 and #16: This is the same pattern identified ininstitute_actions.tsanduser_actions.tswhereisAdmin: "true"header was added. This appears to be a systematic issue across multiple files in this PR.
Confidence: 5/5
Recommended Action
Immediate verification required:
- Review the backend
/api/google/auth/route.tsendpoint implementation (modified in this PR - see Comment feat(api): add teacher and student management endpoints with filtering, pagination and type safety #13) - Ensure it performs proper server-side authorization and does NOT trust the
isAdminparameter for authorization decisions - Verify that admin status is determined by server-side logic (e.g., checking email domain, pre-approved admin list, or existing database records)
If the parameter is required:
- The backend MUST validate admin status through server-side logic, NOT from this client-provided parameter
- The parameter should only be used as a hint for UI/routing purposes, never for authorization
- Consider removing this parameter entirely if the backend can determine admin status independently
Best practice: - Authorization decisions must ALWAYS be made server-side based on trusted data sources
- Never trust client-provided parameters for security-critical decisions
- Admin status should be determined by server-side validation (e.g., email whitelist, database lookup, or organizational domain verification)
If keeping the parameter:
Document clearly in code comments that this parameter is NOT used for authorization and is only for client-side routing/UI purposes. The backend must independently verify admin status.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(auth)/_components/GoogleLoginButton.tsx at line 29, a client-controlled
parameter "isAdmin: true" is being sent to the backend API endpoint /api/google/auth.
This is a critical security vulnerability if the backend trusts this parameter for
authorization decisions during Google OAuth authentication. Review the backend
/api/google/auth/route.ts endpoint to ensure it performs proper server-side
authorization and determines admin status through trusted server-side logic (such as
email domain validation, pre-approved admin list, or database lookup), NOT from the
isAdmin parameter sent by the client. If the backend correctly validates admin status
independently, document in a code comment why this parameter is needed (e.g., for
client-side routing only). If the backend relies on this parameter for authorization,
this is a critical vulnerability that must be fixed immediately by implementing proper
server-side admin validation logic.
|
|
||
| res.cookies.set('token', '', { | ||
| await fetch( | ||
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/auth/logout`, |
There was a problem hiding this comment.
Security Issue - Backend API URL Exposure: Using NEXT_PUBLIC_ADMIN_API_BASE_URL exposes the internal admin API base URL to all client-side code. Environment variables prefixed with NEXT_PUBLIC_* are embedded in the client-side JavaScript bundle and are visible to anyone inspecting the browser DevTools, page source, or network requests.
Security Risks:
- Attackers can discover the backend admin API endpoint structure
- If the backend API is meant to be internal/private, this exposure defeats that security boundary
- Attackers can attempt direct attacks against the exposed backend API endpoints
- The full API base URL becomes public knowledge, making targeted attacks easier
Impact: This is particularly concerning for an admin API, as it exposes administrative endpoints that should typically remain hidden from public view.
Confidence: 5/5
Recommended Action
Option 1: Use server-side environment variable (Recommended)
Since this is a Next.js API route (server-side), use a server-only environment variable:
await fetch(
`${process.env.ADMIN_API_BASE_URL}/api/auth/logout`,
{
method: "GET",
headers: {
"Content-Type": "application/json",
},
credentials: "include",
}
);Then in your .env file:
ADMIN_API_BASE_URL=https://internal-admin-api.example.com
Option 2: Use relative URL if backend is same domain
If the backend API is hosted on the same domain, use a relative URL:
await fetch("/api/auth/logout", {
method: "GET",
headers: {
"Content-Type": "application/json",
},
credentials: "include",
});Additional Recommendations:
- Add error handling for the fetch call to log backend logout failures
- Consider if this backend call is necessary - the cookie clearing on line 15 might be sufficient
- If keeping the backend call, document why it's needed (e.g., server-side session invalidation)
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/api/auth/logout/route.ts at line 6, the code uses NEXT_PUBLIC_ADMIN_API_BASE_URL
which exposes the internal admin API base URL to all client-side code. Since this is a
Next.js API route (server-side), change it to use a server-only environment variable
instead. Replace process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL with
process.env.ADMIN_API_BASE_URL (without the NEXT_PUBLIC_ prefix). Update your .env file
to define ADMIN_API_BASE_URL with the backend API URL. This prevents exposing internal
API endpoints to the client-side JavaScript bundle. Also consider adding error handling
for the fetch call to log any backend logout failures.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/actions/batch_actions.ts (2)
59-67:⚠️ Potential issue | 🟡 MinorSame copy-paste error in
getInstituteBatch.Error messages should reference batch retrieval, not "daily quiz answers".
🐛 Proposed fix
} catch (error) { if (error instanceof Error) { - throw new Error(`Error in saving daily quiz answers: ${error.message}`); + throw new Error(`Error in fetching institute batches: ${error.message}`); } else { throw new Error( - "An unknown error occurred while saving daily quiz answers!", + "An unknown error occurred while fetching institute batches!", ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/batch_actions.ts` around lines 59 - 67, The catch block inside getInstituteBatch currently throws errors mentioning "saving daily quiz answers" due to copy-paste; update the thrown messages in the catch (the branches checking error instanceof Error and the else branch) to reference batch retrieval (e.g., "Error retrieving institute batch" or "An unknown error occurred while retrieving institute batch") and include the original error.message in the instanceof Error branch so getInstituteBatch's logs correctly describe the failure (locate the catch block in getInstituteBatch where it does throw new Error(...)).
31-39:⚠️ Potential issue | 🟡 MinorError messages reference wrong functionality.
The error messages mention "daily quiz answers" but this function handles batch creation. This appears to be a copy-paste error that could cause confusion during debugging.
🐛 Proposed fix
} catch (error) { if (error instanceof Error) { - throw new Error(`Error in saving daily quiz answers: ${error.message}`); + throw new Error(`Error in creating batch: ${error.message}`); } else { throw new Error( - "An unknown error occurred while saving daily quiz answers!", + "An unknown error occurred while creating batch!", ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/batch_actions.ts` around lines 31 - 39, The catch block in src/actions/batch_actions.ts currently throws errors mentioning "daily quiz answers" (the string "Error in saving daily quiz answers") which is a copy-paste mistake; update both thrown messages to correctly reference batch creation (e.g., "Error creating batch(s)" for Error instances and "An unknown error occurred while creating batches" for the unknown branch) so logs match the function's purpose—locate the catch that throws new Error with "Error in saving daily quiz answers" and replace those messages accordingly.src/actions/mentor_actions.ts (2)
82-98:⚠️ Potential issue | 🟠 MajorMissing authentication token in API request.
getInstituteStudentsdoesn't include the authentication token in the request headers, unlikeaddTeacherToInstitute. If this endpoint requires authentication, requests will fail.🔒 Proposed fix to add authentication
export async function getInstituteStudents(instituteId: string) { try { + const token = await getCookie(); + // Validate input if (!instituteId) { throw new Error("Institute ID is required"); } // Make API call to backend const response = await fetch( `${process.env.NEXT_PUBLIC_API_URL}/api/institutes/${instituteId}/students`, { method: "GET", headers: { "Content-Type": "application/json", + Cookie: `token=${token}`, }, } );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/mentor_actions.ts` around lines 82 - 98, getInstituteStudents is missing the authentication token in its fetch headers; mirror the approach used in addTeacherToInstitute by retrieving the same auth token (use the existing auth helper or the same storage mechanism used by addTeacherToInstitute), add an Authorization header like "Authorization: Bearer <token>" to the request options in getInstituteStudents, and validate the token exists (throw or handle error if absent) while preserving the existing Content-Type header.
7-16:⚠️ Potential issue | 🟡 MinorJSDoc comment doesn't match function name.
The JSDoc describes "Add students to an institute" but the function is named
addTeacherToInstitute. Either update the documentation to reflect "teachers" or rename the function if it's actually for students.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/mentor_actions.ts` around lines 7 - 16, The JSDoc for addTeacherToInstitute is incorrect (it says "Add students to an institute"); either update the comment to describe adding teachers or rename the function to reflect student behavior—inspect the implementation of addTeacherToInstitute to determine its purpose, then (a) if it manages teachers, change the JSDoc summary and param descriptions to mention teachers and teacher emails, or (b) if it actually adds students, rename the function to addStudentToInstitute (and update any references) and adjust the JSDoc accordingly; ensure the function name, JSDoc summary, and parameter descriptions (emails, instituteId) are consistent.src/app/(protected)/(root)/institute/[instituteId]/batches/[batchId]/students/page.tsx (1)
206-248:⚠️ Potential issue | 🟠 MajorInvalid Button variant
"outline-solid"— use existing variant instead.The
buttonVariantsonly define:default,outline,secondary,ghost,destructive,link. The"outline-solid"variant does not exist and will cause type/styling issues.Fix — replace with valid variant
<Button - variant={filter === "All" ? "default" : "outline-solid"} + variant={filter === "All" ? "default" : "outline"}Apply to all four filter buttons (lines 206, 217, 228, 239).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/batches/[batchId]/students/page.tsx around lines 206 - 248, The Button components that render the filters use a non-existent variant "outline-solid"; update each of the filter Buttons (the ones comparing filter === "All"/"Excellent"/"Optimal"/"Insufficient") to use a valid variant from buttonVariants (e.g., replace "outline-solid" with "outline") so the variant prop on those Buttons matches the defined variants and styling works; keep the conditional logic (variant={filter === "X" ? "default" : "outline"}) and the onClick handlers that call handleFilter("X") unchanged.src/app/(protected)/(root)/(dashboard)/page.tsx (1)
60-82:⚠️ Potential issue | 🟠 MajorDon’t ship the dashboard with its body commented out.
Lines 60-82 remove the institute, students, and teachers overview cards from the rendered page, so this route now lands as a title plus an empty content area. If the replacement UI is not ready, keep the existing section or gate the new behavior behind a flag.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/(dashboard)/page.tsx around lines 60 - 82, The dashboard page currently has its main content commented out, removing InstituteOverview, StudentsOverview, and TeachersOverview and leaving only the title; restore those components (InstituteOverview, StudentsOverview, TeachersOverview) so they consume dashboardData (dashboardData.institute, dashboardData.students, dashboardData.teachers) and render normally, or if the new UI isn't ready wrap the new layout behind a feature flag and render the existing components when the flag is disabled; ensure props match the existing prop names (instituteCode, address, contact, email, totalStudents, activeCourses, averageAttendance, performanceIndex, totalTeachers, departments, activeClasses, satisfactionRate) and remove the comment block so the components appear on the page.src/app/(auth)/_components/GoogleLoginButton.tsx (1)
21-43:⚠️ Potential issue | 🟠 MajorRemove auth payload logging from the client flow.
Line 23 logs the Google OAuth response, which includes the access token, and Line 43 logs the backend auth response. Both are sensitive for a production login path and should not be written to the browser console.
🔒 Suggested fix
onSuccess: async (credentialResponse) => { setIsLoading(true); - console.log("Google login successful:", credentialResponse); try { const res = await axios.post( "/api/google/auth", @@ toast.success("Login success", { description: res.data.message, }); - - console.log(res.data); if (res.status === 201) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(auth)/_components/GoogleLoginButton.tsx around lines 21 - 43, The client flow currently logs sensitive auth data: remove the console.log("Google login successful:", credentialResponse) inside the onSuccess handler and the console.log(res.data) after the axios.post call; instead, if you need a debug hint use a non-sensitive message (e.g., "Google login succeeded" or rely on toast.success) and perform any detailed logging server-side. Locate these logs in the onSuccess callback (references: onSuccess, credentialResponse, axios.post, res.data, toast.success) and delete or replace the console statements so access_token and backend auth payloads are never written to the browser console.src/app/StoreProvider.tsx (1)
17-22:⚠️ Potential issue | 🟠 MajorKeep the Redux user state synced when the
userprop changes.This only dispatches
userData(user)on the first render. If the server passes a different admin after hydration or a later navigation, the store keeps the stale value.🧭 Suggested fix
-import { useRef } from "react"; +import { useEffect, useRef } from "react"; @@ const storeRef = useRef<AppStore | null>(null); if (storeRef.current === null) { - // Create the store instance the first time this renders storeRef.current = makeStore(); - storeRef.current.dispatch(userData(user)); } + + useEffect(() => { + storeRef.current?.dispatch(userData(user)); + }, [user]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/StoreProvider.tsx` around lines 17 - 22, The store is only seeded once so changes to the incoming user prop aren't propagated; after creating storeRef.current via makeStore() and dispatching userData(user) on initial render, also add an effect that watches the user prop and dispatches storeRef.current.dispatch(userData(user)) whenever user changes (guarding that storeRef.current is non-null). This keeps the Redux user state in sync with the user prop while preserving the initial creation logic that uses makeStore() and userData.
🟠 Major comments (20)
src/components/ui/dropdown-menu.tsx-50-50 (1)
50-50:⚠️ Potential issue | 🟠 MajorSwitch these Radix CSS-variable classes to Tailwind v4 syntax.
origin-[--radix-dropdown-menu-content-transform-origin]andmax-h-[var(--radix-dropdown-menu-content-available-height)]are v3-style arbitrary values. In Tailwind v4, these utilities need the parenthesized CSS-variable form, otherwise the max-height/origin styles won't be generated.♻️ Proposed fix
- "z-50 min-w-[8rem] overflow-hidden rounded-md border bg-popover p-1 text-popover-foreground shadow-lg data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-dropdown-menu-content-transform-origin]", + "z-50 min-w-[8rem] overflow-hidden rounded-md border bg-popover p-1 text-popover-foreground shadow-lg data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-dropdown-menu-content-transform-origin)",- "z-50 max-h-[var(--radix-dropdown-menu-content-available-height)] min-w-[8rem] overflow-y-auto overflow-x-hidden rounded-md border bg-popover p-1 text-popover-foreground shadow-md", - "data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-dropdown-menu-content-transform-origin]", + "z-50 max-h-(--radix-dropdown-menu-content-available-height) min-w-[8rem] overflow-y-auto overflow-x-hidden rounded-md border bg-popover p-1 text-popover-foreground shadow-md", + "data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-dropdown-menu-content-transform-origin)",Also applies to: 68-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/dropdown-menu.tsx` at line 50, In the DropdownMenu content class string (in src/components/ui/dropdown-menu.tsx, e.g., the DropdownMenuContent/DropdownMenu component), replace v3-style arbitrary utilities using raw CSS variables with Tailwind v4 parenthesized form: change origin-[--radix-dropdown-menu-content-transform-origin] to origin-[var(--radix-dropdown-menu-content-transform-origin)] and any max-h-[var(--radix-dropdown-menu-content-available-height)] usages to max-h-[var(--radix-dropdown-menu-content-available-height)] (update all occurrences, including the other instances referenced in the comment).src/app/api/google/auth/route.ts-18-23 (1)
18-23:⚠️ Potential issue | 🟠 MajorAdd
secureflag to cookie for production.The cookie is missing the
secureflag, which means it could be transmitted over insecure HTTP connections. For authentication tokens, this should betruein production environments.🔒 Proposed fix
res.cookies.set("leadlly.in_admin_token", token, { httpOnly: true, + secure: process.env.NODE_ENV === "production", path: "/", sameSite: "strict", expires: new Date("9999-12-31T23:59:59Z"), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/google/auth/route.ts` around lines 18 - 23, The cookie set in res.cookies.set("leadlly.in_admin_token", ...) lacks the secure flag; update the res.cookies.set call in src/app/api/google/auth/route.ts to include secure: true when running in production (e.g., process.env.NODE_ENV === "production" or an isProduction env var) so the token is only sent over HTTPS in prod, keeping the existing httpOnly, path, sameSite, and expires settings intact.src/app/api/google/auth/route.ts-10-10 (1)
10-10:⚠️ Potential issue | 🟠 MajorRemove debug logging of request body.
Logging the full request body may expose sensitive authentication data (tokens, user credentials) in production logs. This is a compliance/privacy risk.
🐛 Proposed fix
- console.log(body, "here is the body");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/google/auth/route.ts` at line 10, Remove the debug console.log that prints the request body to avoid leaking sensitive auth data: locate the console.log(body, "here is the body") call in src/app/api/google/auth/route.ts (inside the route handler that processes the auth request), delete that statement, and if you need observability replace it with a non-sensitive log (e.g., log only safe metadata like request id or status) or use a conditional/debug-only logger that never runs in production.src/proxy.ts-26-32 (1)
26-32:⚠️ Potential issue | 🟠 MajorMissing null check before accessing
user.admin.institutes[0].If
getUser()fails or returns unexpected data, accessinguser.admin.institutes[0]will throw. Add defensive checks.🛡️ Proposed fix
if (token && isPublicPath) { - const user = await getUser(); - - return NextResponse.redirect( - new URL(`/institute/${user.admin.institutes[0].toString()}`, request.url) - ); + try { + const user = await getUser(); + if (user?.admin?.institutes?.length > 0) { + return NextResponse.redirect( + new URL(`/institute/${user.admin.institutes[0].toString()}`, request.url) + ); + } + return NextResponse.redirect(new URL("/create-institute", request.url)); + } catch { + // If user fetch fails, allow access to public path + return NextResponse.next(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/proxy.ts` around lines 26 - 32, The code redirects to `/institute/${user.admin.institutes[0]}` without verifying getUser() returned a valid object and that admin/institutes exist and contain an entry; update the block handling token && isPublicPath to validate the returned value from getUser() (check user is non-null, user.admin exists, user.admin.institutes is an array and has length > 0 and that institutes[0] is defined) before calling NextResponse.redirect, and if any check fails either skip the redirect or choose a safe fallback (e.g., redirect to a default page or return NextResponse.next()); reference the token/isPublicPath condition, getUser(), and the NextResponse.redirect call when making the change.src/actions/mentor_actions.ts-40-50 (1)
40-50:⚠️ Potential issue | 🟠 MajorConfirm API base URL inconsistency and check for missing admin header.
The inconsistency is confirmed:
addTeacherToInstituteusesNEXT_PUBLIC_ADMIN_API_BASE_URLwhilegetInstituteStudentsusesNEXT_PUBLIC_API_URL. The same pattern exists instudent_action.ts, suggesting intentional separation of admin vs. public endpoints.However, compare the add operations across both files:
student_action.tsincludesisAdmin: "true"header in its add request, butmentor_actions.tsdoes not. Verify whether this header is required and ifmentor_actions.tsis missing it unintentionally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/mentor_actions.ts` around lines 40 - 50, The POST in mentor_actions.ts uses NEXT_PUBLIC_ADMIN_API_BASE_URL for /api/student/add but omits the isAdmin header that student_action.ts includes; verify whether this endpoint expects the admin flag and either (a) add headers: { "Content-Type": "application/json", "Cookie": `token=${token}`, "isAdmin": "true" } to the fetch call shown, or (b) if the endpoint should be the public API, change to NEXT_PUBLIC_API_URL to match getInstituteStudents — update the fetch invocation that constructs the URL and headers (using process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL, the POST to /api/student/add/${instituteId}, and the Cookie token) accordingly so the admin header is consistent with student_action.ts.src/app/(protected)/create-institute/page.tsx-15-16 (1)
15-16:⚠️ Potential issue | 🟠 MajorDon’t vertically center a form page.
Line 15 uses
items-centeron amin-h-screenwrapper. OnceCreateInstituteFormgrows beyond the viewport—mobile, validation errors, browser zoom—the top of the card can become unreachable.💡 Proposed fix
- <div className="container mx-auto min-h-screen px-4 py-8 max-w-2xl flex items-center justify-center w-full"> + <div className="container mx-auto min-h-screen px-4 py-8 max-w-2xl flex items-start justify-center w-full">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/create-institute/page.tsx around lines 15 - 16, The wrapper div currently uses min-h-screen with items-center which vertically centers the Card and can make the top unreachable when the form grows; update the container's layout by removing or replacing items-center (e.g., use items-start or omit vertical alignment) and keep sufficient top/bottom padding (px-4 py-8) so the Card and the CreateInstituteForm can scroll normally; locate the outer div with className="container mx-auto min-h-screen px-4 py-8 max-w-2xl flex items-center justify-center w-full" and the Card component wrapping CreateInstituteForm and change the flex alignment accordingly.src/app/provider.tsx-12-14 (1)
12-14:⚠️ Potential issue | 🟠 MajorHandle
getUser()failures before rendering the provider.Line 12 is unguarded. If the lookup throws during SSR, the whole tree fails instead of degrading to
user={null}.💡 Proposed fix
- const userData = await getUser(); + const userData = await getUser().catch(() => null); - return <StoreProvider user={userData?.admin}>{children}</StoreProvider>; + return <StoreProvider user={userData?.admin ?? null}>{children}</StoreProvider>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/provider.tsx` around lines 12 - 14, The getUser() call can throw during SSR causing the whole render to fail; wrap the call to getUser() in a try/catch inside the async provider component (the place that assigns userData) and on error (or if it returns undefined) set userData to null before rendering; then pass user={userData?.admin ?? null} into StoreProvider (the JSX that returns <StoreProvider user={...}>{children}</StoreProvider>) so failures degrade to no user instead of crashing.src/app/api/auth/verify/route.ts-15-20 (1)
15-20:⚠️ Potential issue | 🟠 MajorAdd
secureflag to the admin auth cookie to prevent transmission over HTTP.Lines 15-20 set the admin token cookie without the
secureflag, which allows browsers to transmit it over unencrypted HTTP connections outside of development environments. This weakens the security boundary for a sensitive authentication token.💡 Proposed fix
res.cookies.set("leadlly.in_admin_token", token, { httpOnly: true, + secure: process.env.NODE_ENV === "production", path: "/", sameSite: "strict", expires: new Date("9999-12-31T23:59:59Z"), });Note: This same issue exists in
src/app/api/auth/login/route.ts,src/app/api/auth/logout/route.ts, andsrc/app/api/google/auth/route.ts—apply the same fix to all admin token cookie configurations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/auth/verify/route.ts` around lines 15 - 20, The admin auth cookie set via res.cookies.set("leadlly.in_admin_token", token, { ... }) is missing the secure flag; update the cookie options in the res.cookies.set calls (the ones that set "leadlly.in_admin_token" in verify/login/logout/google auth routes) to include secure: true in production (e.g., secure: process.env.NODE_ENV !== 'development' or secure: true behind your TLS-aware environment) so the cookie is sent only over HTTPS, keeping all other existing options (httpOnly, path, sameSite, expires) intact.src/app/(protected)/(root)/institute/[instituteId]/_components/StudentsOverview.tsx-10-10 (1)
10-10:⚠️ Potential issue | 🟠 MajorDo not make
instituteIdoptional if the link depends on it.Line 111 will render
/institute/undefined/batcheswhenever this prop is missing, so the CTA silently becomes a broken navigation target. MakeinstituteIdrequired or hide/disable the link until it exists.🧭 Suggested fix
interface StudentsOverviewProps { totalStudents: number; activeCourses: number; averageAttendance: number; performanceIndex: number; - instituteId?: string; + instituteId: string; }Also applies to: 111-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/_components/StudentsOverview.tsx at line 10, The StudentsOverview component currently declares instituteId?: string which allows rendering a link to `/institute/undefined/batches`; change the prop to required (in the component props/type: instituteId: string) or, alternatively, guard the link rendering in StudentsOverview (where the CTA is created—the anchor/Link that builds `/institute/${instituteId}/batches`) to only render or enable the link when instituteId is a non-empty string, e.g., hide/disable the CTA and provide fallback UI until instituteId exists.src/app/api/auth/login/route.ts-16-21 (1)
16-21:⚠️ Potential issue | 🟠 MajorHarden the auth cookie before shipping.
Lines 16-20 create a bearer-token cookie that is effectively permanent and not marked
secure. That makes stale sessions linger indefinitely and weakens transport guarantees outside localhost. Prefer a session cookie or derive expiry from the token TTL, and setsecurein production.🔐 Suggested fix
res.cookies.set("leadlly.in_admin_token", token, { httpOnly: true, + secure: process.env.NODE_ENV === "production", path: "/", sameSite: "strict", - expires: new Date("9999-12-31T23:59:59Z"), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/auth/login/route.ts` around lines 16 - 21, The auth cookie set by res.cookies.set("leadlly.in_admin_token", token, {...}) is too persistent and not marked secure; update the login route to (1) avoid a permanent expires — either make it a session cookie by omitting expires or derive expires/maxAge from the token's TTL/JWT exp (use the token variable or decode its exp) and (2) set secure: process.env.NODE_ENV === "production" (or equivalent env flag) so the cookie is only sent over TLS in prod; keep httpOnly, path, sameSite as-is and prefer using maxAge when deriving TTL.src/app/(protected)/(root)/layout.tsx-15-20 (1)
15-20:⚠️ Potential issue | 🟠 MajorAdd
staleTimeto prevent immediate refetch on hydration.Lines 17-20 prefetch without setting
staleTime. React Query defaults tostaleTime: 0, so this data becomes stale immediately. WhenHydrationBoundaryrestores it on the client, theuseSuspenseQuerycall in main-header.tsx will see stale data and refetch, creating the double-fetch you're trying to avoid. AddstaleTime: 60 * 1000(or match your QueryProvider defaults) to the prefetch options.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/layout.tsx around lines 15 - 20, The prefetchQuery call using QueryClient (queryKey ["all_institutes"] and queryFn getAllUserInstitutes) doesn't set staleTime so the data is immediately considered stale and triggers a client refetch after hydration; update the prefetchQuery options to include staleTime: 60 * 1000 (or your QueryProvider default) so the prefetched data remains fresh across hydration and prevents the double-fetch in useSuspenseQuery in main-header.tsx.src/helpers/types/index.ts-588-623 (1)
588-623:⚠️ Potential issue | 🟠 MajorDo not include credential fields on a client-shared admin type.
IAdminnow containspasswordandsalt, and this type is already flowing into client-side provider/store code in this PR. Keeping secret fields on the shared shape makes accidental serialization to the browser much more likely.🔐 Suggested fix
export interface IAdmin { _id: string; firstname: string; lastname: string; email: string; role: "admin" | "super_admin"; phone: string; - password: string; - salt: string; avatar: { public_id: string; url: string; };If server-only code needs those fields, split the model into a public client DTO and a separate server-only type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/helpers/types/index.ts` around lines 588 - 623, The IAdmin interface exposes sensitive credential fields (password, salt) that are leaking into client-side code; remove password and salt from the shared IAdmin type and create a separate server-only interface (e.g., IAdminPrivate or IAdminServer) that extends the public shape and includes password and salt for server use only, then update client-side consumers (providers/stores) to depend on the sanitized public DTO (e.g., IAdminPublic or the updated IAdmin) while server code (auth, models) imports the server-only interface to access credentials.src/app/(protected)/(root)/institute/[instituteId]/_components/add-students-dialog.tsx-28-30 (1)
28-30:⚠️ Potential issue | 🟠 MajorValidate each comma-separated email in the schema.
This currently accepts any non-empty string, so values like
foo, barpass client-side and fail later. Validate the split list up front so the form error is immediate and specific.✅ Suggested fix
+const parseEmails = (value: string) => + value + .split(",") + .map((email) => email.trim()) + .filter(Boolean); + const AddStudentsFormSchema = z.object({ - emails: z.string().min(1, { error: "Enter valid email addresses" }), + emails: z + .string() + .trim() + .min(1, { error: "Enter at least one email address" }) + .refine( + (value) => + parseEmails(value).every( + (email) => z.email().safeParse(email).success + ), + { error: "Enter valid comma-separated email addresses" } + ), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/_components/add-students-dialog.tsx around lines 28 - 30, The emails field currently only checks non-empty string; update AddStudentsFormSchema to validate each comma-separated address by transforming the input into an array (split on commas, trim each entry, filter out empty strings) and then validate that the resulting array has at least one item and every entry is a valid email (use z's email validator or a regex) with a clear error message; update any form handlers that expect a string to accept the transformed array (or derive the array after validation) so client-side validation rejects inputs like "foo, bar" immediately.src/app/(protected)/create-institute/_components/create-institute-form.tsx-169-379 (1)
169-379:⚠️ Potential issue | 🟠 MajorFix the label/control associations across this form.
Most
FieldLabelelements still point tohtmlFor="name"while the actual controls don't expose matchingids. That breaks label click/focus behavior and screen-reader association for nearly every field in this section.♿ Suggested fix pattern
<Controller control={form.control} name="email" render={({ field, fieldState }) => ( <Field data-invalid={fieldState.invalid} className="gap-1.5"> - <FieldLabel htmlFor="name">Email</FieldLabel> + <FieldLabel htmlFor="email">Email</FieldLabel> <Input + id="email" {...field} placeholder="Enter institute email" required aria-invalid={fieldState.invalid} className="shadow-none"Apply the same pattern to each field so every
htmlFormatches a unique controlid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/create-institute/_components/create-institute-form.tsx around lines 169 - 379, Many FieldLabel elements use htmlFor="name" but the corresponding inputs/selects lack matching id attributes, breaking label-control association; update each Controller render (e.g., ones with name="name", "email", "contactNumber", "website", "address1", "address2", "city", "state", "pincode", "country") so the rendered control (Input or Select) has an id that matches the FieldLabel htmlFor (use a unique id per field, e.g., id={`${field.name}`}), and ensure Select uses the same id on its SelectTrigger/SelectValue as appropriate; also keep FieldLabel htmlFor values matched to those ids to restore click/focus and screen-reader associations.src/app/(protected)/(root)/institute/[instituteId]/page.tsx-41-54 (1)
41-54:⚠️ Potential issue | 🟠 MajorReplace the mock metrics before shipping this page.
These values are rendered for every institute, so the dashboard always shows the same student and teacher counts regardless of the selected institute. Wire the cards to real stats or hide them until the API exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/page.tsx around lines 41 - 54, The page currently uses a hard-coded dashboardData object (students/teachers) so every institute shows identical metrics; replace that static dashboardData with a real data flow by calling an async data function (e.g., getInstituteDashboardStats(instituteId) or fetch(`/api/institutes/${instituteId}/stats`)) inside the page component (or a server-side loader) and map the returned students and teachers fields into the existing cards, and if the API is not ready, hide the cards or render a loading/skeleton state when dashboardData is null/undefined; update references to dashboardData, students, and teachers to use the fetched stats (or a feature flag) so metrics are institute-specific.src/actions/institute_actions.ts-32-36 (1)
32-36:⚠️ Potential issue | 🟠 Major
updateTag("userData")does not invalidate the institute queries.None of the fetchers in this file use the
userDatatag. ThegetActiveInstituteandgetAllUserInstitutesfunctions have no cache tags defined and will not be invalidated by this call. If React Query is used for these queries, the invalidation layers are separate and won't communicate.Expected: Define cache tags on the fetchers that read institute data, or use a tag name that actually matches the queries being invalidated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/institute_actions.ts` around lines 32 - 36, The call to updateTag("userData") does not invalidate institute queries because getActiveInstitute and getAllUserInstitutes don't use that tag; either change the tag passed to updateTag to the actual tag those fetchers use (e.g., "institute" or whatever cache key those fetchers register) or add the appropriate cache tag definitions to the institute fetchers (getActiveInstitute and getAllUserInstitutes) so they include the same tag name; update the call site (updateTag) or the fetchers’ tag registration so the tag names match and invalidation will trigger correctly.src/app/(protected)/(root)/_components/main-header.tsx-71-78 (1)
71-78:⚠️ Potential issue | 🟠 MajorUse
onSelectinstead ofonClickfor institute navigation.
DropdownMenuItemselection is Radix's cross-input activation signal—it fires for both pointer and keyboard interactions (Enter/Space on focused items). UsingonClickbypasses keyboard activation, leaving users unable to switch institutes via keyboard navigation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/_components/main-header.tsx around lines 71 - 78, The DropdownMenuItem currently uses an onClick handler for navigation which misses Radix's cross-input activation (keyboard) and prevents keyboard users from selecting institutes; replace the onClick prop on DropdownMenuItem with an onSelect handler that calls router.replace(`/institute/${institute._id}`) (preserving the same key, className and activeInstitute check) so selection works for both pointer and keyboard interactions.src/components/ui/combobox.tsx-74-82 (1)
74-82:⚠️ Potential issue | 🟠 MajorRender the
InputGroupButtonthrough the trigger'srenderprop instead of nesting.The
ComboboxTriggerrenders a<button>by default, so the current code creates a nested button structure (<button><button>), which is invalid HTML and breaks click/focus behavior. Use therenderprop to compose the custom button element.Suggested fix
{showTrigger && ( - <InputGroupButton - size="icon-xs" - variant="ghost" - data-slot="input-group-button" - className="group-has-data-[slot=combobox-clear]/input-group:hidden data-pressed:bg-transparent" - disabled={disabled} - > - <ComboboxTrigger /> - </InputGroupButton> + <ComboboxTrigger + disabled={disabled} + render={ + <InputGroupButton + size="icon-xs" + variant="ghost" + data-slot="input-group-button" + className="group-has-data-[slot=combobox-clear]/input-group:hidden data-pressed:bg-transparent" + disabled={disabled} + /> + } + /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/combobox.tsx` around lines 74 - 82, The InputGroupButton is being nested with ComboboxTrigger which causes a <button><button> invalid structure; replace the nested ComboboxTrigger by using the trigger component's render prop so InputGroupButton becomes the rendered trigger element. Locate the InputGroupButton/ComboboxTrigger block and remove the inner <ComboboxTrigger />; instead call the trigger's render prop (e.g., ComboboxTrigger render or similar API) to pass trigger props/handlers into InputGroupButton so it receives the trigger's aria/onclick/focus props and retains size/variant/disabled/className.src/app/(protected)/(root)/institute/[instituteId]/page.tsx-26-31 (1)
26-31:⚠️ Potential issue | 🟠 MajorWrap the prefetched query in a
HydrationBoundaryto avoid duplicate fetches.
prefetchQuery()in a server component alone doesn't make the cached data available to client-sideuseQuery()hooks—they have their ownQueryClientinstance. The prefetched data is discarded, andInstituteOverviewfetches again on the client, doubling your network requests. You must serialize the server cache withdehydrate(queryClient)and pass it through<HydrationBoundary>for the client to reuse it.Suggested fix
import { HydrationBoundary, QueryClient, dehydrate } from "@tanstack/react-query"; const queryClient = new QueryClient(); await queryClient.prefetchQuery({ queryKey: ["active_institute", instituteId], queryFn: () => getActiveInstitute({ instituteId }), }); return ( <HydrationBoundary state={dehydrate(queryClient)}> {/* existing page JSX */} </HydrationBoundary> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/page.tsx around lines 26 - 31, The server-side QueryClient created as QueryClient() and used with queryClient.prefetchQuery(...) isn't being serialized to the client, so wrap your returned JSX in a HydrationBoundary and pass state={dehydrate(queryClient)} (import HydrationBoundary and dehydrate from `@tanstack/react-query`) so the client-side useQuery() in InstituteOverview can reuse the prefetched cache instead of refetching; ensure you add the imports for HydrationBoundary and dehydrate and wrap the existing page JSX with <HydrationBoundary state={dehydrate(queryClient)}>...</HydrationBoundary>.src/actions/institute_actions.ts-80-95 (1)
80-95:⚠️ Potential issue | 🟠 MajorCheck both
res.okandresponseData.successbefore returning.
fetchdoes not throw on HTTP 4xx/5xx status codes—it only rejects on transport failures. Currently, if the API returns 401/404/500 orsuccess: false, the functions return the error response as if successful. Since these functions are used as TanStack QueryqueryFns (in main-header.tsx, layout.tsx, InstituteOverview.tsx), the query enters the success state instead of the error state when no exception is thrown. Additionally, callers like select-institute/page.tsx dereferencedata.institutesdirectly without checkingdata.success, which would cause runtime errors on API failures.Suggested fix
const responseData: { success: boolean; institute: IInstitute } = await res.json(); + if (!res.ok || !responseData.success) { + throw new Error("Failed to fetch institute"); + } return responseData;const responseData: { success: boolean; institutes: IInstitute[]; count: number; } = await res.json(); + if (!res.ok || !responseData.success) { + throw new Error("Failed to fetch institutes"); + } return responseData;Also applies to: 109-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/institute_actions.ts` around lines 80 - 95, The fetch block that builds `res` and parses `responseData` must validate both the HTTP status and the API payload before returning: after `const res = await fetch(...)` check `if (!res.ok)` and throw an Error (including `res.status` and statusText or parsed body) so TanStack Query treats it as a failure; after `const responseData = await res.json()` also check `if (!responseData.success)` and throw an Error (include `responseData` or a clear message). Apply this change for the institute fetch shown (use the existing variables `res`, `responseData`, `instituteId`, `token`) and replicate the same pattern for the other similar fetch in the file (the block referenced at lines 109-127) so callers never receive a falsy payload as success.
🟡 Minor comments (12)
src/app/api/auth/logout/route.ts-17-22 (1)
17-22:⚠️ Potential issue | 🟡 MinorAdd
secureflag for production cookie security.The cookie should include
secure: truein production to ensure it's only transmitted over HTTPS, preventing potential session hijacking over insecure connections.🔒 Proposed fix
res.cookies.set("leadlly.in_admin_token", "", { httpOnly: true, path: "/", sameSite: "strict", + secure: process.env.NODE_ENV === "production", expires: new Date(0), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/auth/logout/route.ts` around lines 17 - 22, The logout cookie reset call using res.cookies.set("leadlly.in_admin_token", ...) must include the secure flag in production; update the options object passed to res.cookies.set (in route.ts) to add secure: process.env.NODE_ENV === "production" (or an equivalent runtime env check) so the cookie is only sent over HTTPS in production while remaining usable in dev.src/components/ui/skeleton.tsx-1-6 (1)
1-6:⚠️ Potential issue | 🟡 MinorAdd missing React import for type reference.
The component uses
React.HTMLAttributes<HTMLDivElement>but doesn't import React. All other UI components in this project follow the pattern of explicitly importing React for type references.Proposed fix
+import * as React from "react" + import { cn } from "@/lib/utils" function Skeleton({Alternatively, import only the type:
+import type { HTMLAttributes } from "react" + import { cn } from "@/lib/utils" function Skeleton({ className, ...props -}: React.HTMLAttributes<HTMLDivElement>) { +}: HTMLAttributes<HTMLDivElement>) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/skeleton.tsx` around lines 1 - 6, The Skeleton component uses the type React.HTMLAttributes<HTMLDivElement> but never imports React; add an explicit import for React at the top of the file (e.g., import React from "react" or import type React from "react" per project conventions) so the type reference in the Skeleton function signature resolves correctly.src/app/api/google/auth/route.ts-36-36 (1)
36-36:⚠️ Potential issue | 🟡 MinorHandle potentially undefined status.
If
error.responseis undefined (e.g., network error),error.response?.statuswill beundefined, and NextResponse will default to 200. Provide an explicit fallback.🐛 Proposed fix
- { status: error instanceof AxiosError ? error.response?.status : 500 } + { status: error instanceof AxiosError ? (error.response?.status ?? 500) : 500 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/google/auth/route.ts` at line 36, The status value passed to NextResponse can be undefined when AxiosError.response is undefined (e.g., network errors); update the construction that currently uses error instanceof AxiosError ? error.response?.status : 500 to provide an explicit fallback (for example use the nullish coalescing fallback) so the status is always a valid HTTP status (e.g., default to 500); locate the NextResponse creation in src/app/api/google/auth/route.ts where error handling uses AxiosError and replace the conditional to guarantee a numeric status.src/components/ui/tooltip.tsx-22-25 (1)
22-25:⚠️ Potential issue | 🟡 MinorUpdate CSS variable syntax for Tailwind v4.
The
origin-[--radix-tooltip-content-transform-origin]uses bracket syntax for CSS variables, which changed in Tailwind v4. CSS variables now require parentheses instead of brackets.✏️ Proposed fix
className={cn( - "z-50 overflow-hidden rounded-md bg-primary px-3 py-1.5 text-xs text-primary-foreground animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-tooltip-content-transform-origin]", + "z-50 overflow-hidden rounded-md bg-primary px-3 py-1.5 text-xs text-primary-foreground animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-tooltip-content-transform-origin)", className )}As per the library documentation for Tailwind v4: "CSS Variables - Brackets → Parentheses: OLD: bg-[--brand-color] NEW: bg-(--brand-color). All arbitrary values using CSS variables must use parentheses."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/tooltip.tsx` around lines 22 - 25, The Tailwind v4 syntax for CSS variables is wrong in the tooltip's className: replace the arbitrary-bracket usage origin-[--radix-tooltip-content-transform-origin] with the v4 parenthesis form origin-(--radix-tooltip-content-transform-origin) inside the cn(...) call in the Tooltip component so the CSS variable is applied correctly; update the string passed to className (the one containing "origin-[--radix-tooltip-content-transform-origin]") to use parentheses..prettierignore-4-6 (1)
4-6:⚠️ Potential issue | 🟡 MinorFix typos in glob patterns.
Lines 4 and 6 have erroneous spaces that break the ignore patterns:
*. envshould be*.env.env .*should be.env.*These patterns won't match the intended files (e.g.,
.env.local,.env.production).✏️ Proposed fix
-*. env +*.env .env -.env .* +.env.*🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.prettierignore around lines 4 - 6, The glob patterns in .prettierignore contain typos: replace the incorrect entries "*. env" and ".env .*" with corrected patterns "*.env" and ".env.*" so they properly match files like .env.local and .env.production; update the .prettierignore file to remove the stray spaces in those two lines (the file entries shown in the diff are the locations to fix).src/components/ui/field.tsx-194-215 (1)
194-215:⚠️ Potential issue | 🟡 MinorSkip rendering the alert when there are no actual error messages.
If
errorsis defined but every entry lacksmessage, this branch still returns an empty<ul>, soFieldErrorrenders a blankrole="alert". Filter the messages first and returnnullwhen none remain.💡 Proposed fix
const content = useMemo(() => { if (children) { return children } - if (!errors) { + const messages = errors?.flatMap((error) => + error?.message ? [error.message] : [] + ) ?? [] + + if (messages.length === 0) { return null } - if (errors?.length === 1 && errors[0]?.message) { - return errors[0].message + if (messages.length === 1) { + return messages[0] } return ( <ul className="ml-4 flex list-disc flex-col gap-1"> - {errors.map( - (error, index) => - error?.message && <li key={index}>{error.message}</li> - )} + {messages.map((message, index) => ( + <li key={index}>{message}</li> + ))} </ul> ) }, [children, errors])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/field.tsx` around lines 194 - 215, The current useMemo for content returns an empty <ul> when errors exists but none have a message, causing FieldError to render a blank alert; update the useMemo in the content computation to first derive a filteredMessages = (errors || []).filter(e => e?.message) and if filteredMessages.length === 0 return null, otherwise if filteredMessages.length === 1 return filteredMessages[0].message, else render the <ul> by mapping filteredMessages instead of errors (keep the existing children-shortcircuit intact).src/app/(auth)/login/page.tsx-21-23 (1)
21-23:⚠️ Potential issue | 🟡 MinorAdd explicit alt text to the logo image.
Line 22 renders an image without
alt, which makes the header noisier for screen readers. Usealt=""if this is purely decorative, or a short brand label if it is informative.♿ Suggested fix
<Avatar className="mx-auto mb-6 bg-primary/10 size-12"> - <AvatarImage src="/leadlly.jpeg" className="size-full" /> + <AvatarImage + src="/leadlly.jpeg" + alt="Leadlly logo" + className="size-full" + /> </Avatar>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(auth)/login/page.tsx around lines 21 - 23, The AvatarImage inside the login page (component Avatar / AvatarImage in page.tsx) is missing an alt attribute; update the AvatarImage element to include an explicit alt (use alt="" if the image is purely decorative or a short brand label like "Leadlly logo" if it conveys information) so screen readers do not announce noisy or missing-alt warnings.src/helpers/schema/createInstituteSchema.ts-10-10 (1)
10-10:⚠️ Potential issue | 🟡 Minor
contactNumbercurrently accepts arbitrary 10-character strings.Line 10 only checks length, so obviously invalid values can still pass while the message says “valid phone number”. Tighten this to the phone formats you actually support before persisting it.
📞 Suggested fix
- contactNumber: z.string().min(10, "Please enter a valid phone number"), + contactNumber: z + .string() + .trim() + .regex(/^\+?[0-9\s-]{10,15}$/, "Please enter a valid phone number"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/helpers/schema/createInstituteSchema.ts` at line 10, createInstituteSchema's contactNumber currently only checks length and accepts arbitrary 10-char strings; change the schema for contactNumber in createInstituteSchema to validate against the supported phone formats (e.g., digits-only 10-digit, optional country code/E.164 with leading '+', or your app's accepted patterns) using z.string().regex(...) or z.string().refine(...) with a clear error message; update the validation pattern/error text so invalid formats are rejected before persisting and reference the contactNumber field in the schema for locating the change.src/app/(protected)/create-institute/_components/create-institute-form.tsx-482-493 (1)
482-493:⚠️ Potential issue | 🟡 MinorFix the Tailwind class typo on the add-subject icon.
mr-2size-4is parsed as one invalid class, so you lose both the spacing and the icon sizing here.🪄 Suggested fix
-<Plus className="mr-2size-4" /> +<Plus className="mr-2 size-4" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/create-institute/_components/create-institute-form.tsx around lines 482 - 493, The Plus icon's className has a typo "mr-2size-4" which merges two classes into one invalid token; update the class on the Plus element inside the Button in create-institute-form.tsx (the Add "… " button within the CreateInstituteForm component) to separate the spacing and sizing classes (e.g., use "mr-2" and a size class such as "size-4" or "h-4 w-4") so the margin and icon size are applied correctly.src/app/(protected)/(root)/institute/[instituteId]/page.tsx-90-105 (1)
90-105:⚠️ Potential issue | 🟡 MinorWire up or remove the Edit action.
This renders as a primary CTA, but it has no navigation target or click handler. Right now users hit a dead end.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/page.tsx around lines 90 - 105, The "Edit" Button in page.tsx is rendered without a click handler or navigation target (dead CTA); either wire it to the institute edit route or remove it. Fix by updating the Button usage (the Button element containing the SVG and "Edit" text) to perform navigation: add an onClick that uses Next.js router.push(`/institute/${instituteId}/edit`) or replace Button with a Link component with href to the appropriate edit path, and include an accessible label (aria-label="Edit institute") and any necessary disabled/loading state handling so the CTA is actionable and meaningful.src/components/ui/input-group.tsx-60-77 (1)
60-77:⚠️ Potential issue | 🟡 MinorAddon focus delegation only works for plain inputs.
The click handler hard-codes
querySelector("input"), soInputGroupTextareaand any future control usingdata-slot="input-group-control"never receive focus from the addon. Because{...props}comes afteronClick, a consumer can also overwrite the built-in focus behavior by accident.Suggested fix
function InputGroupAddon({ className, align = "inline-start", + onClick, ...props }: React.ComponentProps<"div"> & VariantProps<typeof inputGroupAddonVariants>) { return ( <div role="group" data-slot="input-group-addon" data-align={align} className={cn(inputGroupAddonVariants({ align }), className)} + {...props} onClick={(e) => { + onClick?.(e) + if (e.defaultPrevented) { + return + } if ((e.target as HTMLElement).closest("button")) { return } - e.currentTarget.parentElement?.querySelector("input")?.focus() + e.currentTarget.parentElement + ?.querySelector<HTMLElement>('[data-slot="input-group-control"]') + ?.focus() }} - {...props} /> ) }src/app/(protected)/(root)/institute/[instituteId]/_components/InstituteOverview.tsx-16-19 (1)
16-19:⚠️ Potential issue | 🟡 MinorUse
useSuspenseQueryor add a loading state to match the Suspense boundary.The
useQueryhook in TanStack Query v5 does not suspend, so the<Suspense>boundary wrapping this component (inpage.tsx, line 110) won't catch its loading state. While the parent pre-fetches the query, if the cache isn't hydrated or the client-side query runs before hydration is complete, the component will render with undefinedactiveInstituteinstead of showing the fallback. Either switch touseSuspenseQueryto integrate with Suspense, or add an explicit loading/error branch within the component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/_components/InstituteOverview.tsx around lines 16 - 19, The component currently calls useQuery({ queryKey: ["active_institute", instituteId], queryFn: () => getActiveInstitute({ instituteId }) }) which does not suspend and can render with activeInstitute undefined under a Suspense boundary; update the hook to use useSuspenseQuery with the same queryKey and queryFn (so the Suspense fallback will be shown) or, if you prefer to keep useQuery, add an explicit loading/error branch that checks the query state (isLoading/isError/isFetched and activeInstitute) before rendering the main UI to avoid rendering with undefined activeInstitute; locate the call to useQuery and either swap it to useSuspenseQuery(...) or add conditional returns using isLoading/isError and activeInstitute.
| try { | ||
|
|
||
| const token = await getCookie("token"); | ||
| const token = await getCookie(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'leadlly\.in_admin_token|getCookie\(|Cookie:' srcRepository: Leadlly/leadlly.admin.web
Length of output: 9443
Fix the forwarded cookie key name to match the authentication middleware.
The code reads the admin cookie correctly via getCookie() which uses the key "leadlly.in_admin_token", but forwards it upstream as Cookie: token=... instead of Cookie: leadlly.in_admin_token=.... The backend proxy and authentication endpoints all expect the full cookie key name "leadlly.in_admin_token", not just "token". This mismatch will cause authentication failures.
The same issue occurs at lines 44-48 and throughout similar action files (batch_actions.ts, institute_actions.ts, mentor_actions.ts, user_actions.ts).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/student_action.ts` at line 18, The forwarded cookie header uses
the wrong key name—getCookie() returns the admin token (from
"leadlly.in_admin_token") but the request header is built as "Cookie: token=..."
causing auth failures; update the code that constructs the Cookie header (where
the local variable token is used) to forward the full cookie key name
"leadlly.in_admin_token" (i.e., change occurrences like `Cookie: token=${token}`
to `Cookie: leadlly.in_admin_token=${token}`) in student_action.ts and the other
action files (batch_actions.ts, institute_actions.ts, mentor_actions.ts,
user_actions.ts) so the backend proxy/auth middleware receives the expected
cookie key; keep using the existing getCookie() call and token variable.
| const handleAddStudents = async ( | ||
| data: z.infer<typeof AddStudentsFormSchema> | ||
| ) => { | ||
| try { | ||
| console.log(data); | ||
|
|
||
| const emails = data.emails | ||
| .split(",") | ||
| .map((email) => email.trim()) | ||
| .filter((email) => email); | ||
|
|
||
| // const result = await addStudentsToInstitute(instituteId, emails); | ||
| // if (result.success) { | ||
| // toast.success(result.message); | ||
| // } else { | ||
| // toast.error(result.message); | ||
| // } | ||
| // form.reset(); | ||
| // setIsAddStudentsOpen(false); |
There was a problem hiding this comment.
Wire the submit handler to the server action before shipping this dialog.
Right now submit only parses the textarea and exits. No students are added, no success state is shown, and the dialog never resets/closes.
🚨 Suggested fix
const handleAddStudents = async (
data: z.infer<typeof AddStudentsFormSchema>
) => {
try {
- console.log(data);
-
const emails = data.emails
.split(",")
.map((email) => email.trim())
.filter((email) => email);
- // const result = await addStudentsToInstitute(instituteId, emails);
- // if (result.success) {
- // toast.success(result.message);
- // } else {
- // toast.error(result.message);
- // }
- // form.reset();
- // setIsAddStudentsOpen(false);
+ const result = await addStudentsToInstitute(instituteId, emails);
+ if (!result.success) {
+ toast.error(result.message);
+ return;
+ }
+
+ toast.success(result.message);
+ form.reset();
+ setIsAddStudentsOpen(false);
} catch (error) {
console.error("Error adding students:", error);
toast.error("Failed to add students. Please try again.");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/_components/add-students-dialog.tsx
around lines 42 - 60, The submit handler handleAddStudents currently only parses
emails and returns; wire it to call the server action
addStudentsToInstitute(instituteId, emails), await the result, and handle
success/error by calling toast.success(result.message) or
toast.error(result.message); on success call form.reset() and
setIsAddStudentsOpen(false). Also wrap the call in the existing try/catch so
errors use toast.error and ensure any loading state (if present) is toggled
appropriately; reference handleAddStudents, addStudentsToInstitute, form.reset,
setIsAddStudentsOpen, toast.success, and toast.error when making the changes.
| await fetch( | ||
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/auth/logout`, | ||
| { | ||
| method: "GET", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| credentials: "include", | ||
| } | ||
| ); |
There was a problem hiding this comment.
Server-side fetch won't forward user cookies; response not validated.
Two issues with this external logout call:
-
Token not forwarded:
credentials: "include"is a browser-context option. In a server-side fetch, cookies from the incoming request are not automatically included. The user's auth token must be manually extracted fromreq.cookiesand forwarded to the external API, otherwise the external service cannot identify which session to invalidate. -
Response not checked: If the external logout fails (non-2xx status), the code still clears the local cookie and returns success, leaving the external session active.
🐛 Proposed fix
export async function GET(req: NextRequest) {
try {
- await fetch(
+ const token = req.cookies.get("leadlly.in_admin_token")?.value;
+
+ const response = await fetch(
`${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/auth/logout`,
{
method: "GET",
headers: {
"Content-Type": "application/json",
+ ...(token && { Cookie: `leadlly.in_admin_token=${token}` }),
},
- credentials: "include",
}
);
+
+ if (!response.ok) {
+ const errorData = await response.json().catch(() => ({}));
+ throw new Error(errorData.message || "Logout failed");
+ }
+
const res = NextResponse.json({ message: "Logged Out" });📝 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.
| await fetch( | |
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/auth/logout`, | |
| { | |
| method: "GET", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| credentials: "include", | |
| } | |
| ); | |
| export async function GET(req: NextRequest) { | |
| try { | |
| const token = req.cookies.get("leadlly.in_admin_token")?.value; | |
| const response = await fetch( | |
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/auth/logout`, | |
| { | |
| method: "GET", | |
| headers: { | |
| "Content-Type": "application/json", | |
| ...(token && { Cookie: `leadlly.in_admin_token=${token}` }), | |
| }, | |
| } | |
| ); | |
| if (!response.ok) { | |
| const errorData = await response.json().catch(() => ({})); | |
| throw new Error(errorData.message || "Logout failed"); | |
| } | |
| const res = NextResponse.json({ message: "Logged Out" }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/auth/logout/route.ts` around lines 5 - 14, The server-side logout
fetch in src/app/api/auth/logout/route.ts is not forwarding the user's cookie
and isn't checking the external response; update the route handler that calls
fetch(...) so it reads the incoming request's cookie (e.g., from the Request
object via req.headers.get("cookie") or req.cookies) and forwards that token by
adding a Cookie or Authorization header to the external fetch options (replace
credentials:"include" with explicit headers), then inspect the fetch response
(use response.ok / response.status) and only clear the local auth cookie and
return success when the external logout succeeds; if the external call fails,
return an error status (and do not clear the local cookie) and include the
external response details in the error handling.
| import { NextResponse } from "next/server"; | ||
| import type { NextRequest } from "next/server"; | ||
|
|
||
| import { getUser } from "./actions/user_actions"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify getUser is a server action
head -5 src/actions/user_actions.tsRepository: Leadlly/leadlly.admin.web
Length of output: 170
🏁 Script executed:
cat -n src/proxy.tsRepository: Leadlly/leadlly.admin.web
Length of output: 2109
Server actions cannot be called from Next.js middleware.
getUser() is a server action (from a file with "use server"), but middleware runs in the Edge runtime before the request reaches the server. This will fail at runtime.
Additionally, line 30 accesses user.admin.institutes[0] without null checks on user — if getUser() returns null, this will crash.
Consider:
- Making a direct fetch call to your auth endpoint from middleware
- Or validating the token using a lightweight JWT verification without calling server actions
Also applies to: 27-27, 35-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/proxy.ts` at line 4, Middleware calls the server action getUser (from
"use server") which will fail in the Edge runtime and also accesses
user.admin.institutes[0] without null checks; replace the getUser() invocation
inside the middleware with a direct fetch to your auth endpoint (or perform a
lightweight JWT verify) to obtain user info, and add defensive checks before
accessing nested properties (e.g., confirm user and user.admin exist and that
user.admin.institutes is an array with length > 0) so you never read
user.admin.institutes[0] when user is null/undefined; update the middleware
function where getUser is referenced to use the fetch/JWT flow and guard all
downstream uses of user (including the code path that reads
user.admin.institutes[0]).
|
|
||
| import { getUser } from "./actions/user_actions"; | ||
|
|
||
| export async function proxy(request: NextRequest) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's a middleware.ts file that imports this proxy
fd -e ts -e js 'middleware' --exec cat {}Repository: Leadlly/leadlly.admin.web
Length of output: 51
🏁 Script executed:
# Check if this is a Next.js project
cat package.json | grep -A 5 '"next"'Repository: Leadlly/leadlly.admin.web
Length of output: 251
🏁 Script executed:
# Read the proxy.ts file to understand its purpose
cat -n src/proxy.tsRepository: Leadlly/leadlly.admin.web
Length of output: 2109
🏁 Script executed:
# Search for imports or usages of the proxy function
rg "proxy" --type ts --type js -B 2 -A 2Repository: Leadlly/leadlly.admin.web
Length of output: 346
🏁 Script executed:
# Check for middleware.ts at project root
ls -la middleware.ts 2>/dev/null || echo "Not found at root"Repository: Leadlly/leadlly.admin.web
Length of output: 86
🏁 Script executed:
# Check the project structure around src/
ls -la src/ | head -20Repository: Leadlly/leadlly.admin.web
Length of output: 631
Rename function to middleware and file to middleware.ts per Next.js convention.
Next.js middleware must export a function named middleware from a file named middleware.ts. The current file (src/proxy.ts) and function name (proxy) won't be recognized by Next.js, preventing the authentication logic from executing.
🔧 Proposed fix
- Rename file from
src/proxy.tstosrc/middleware.ts - Rename function from
proxytomiddleware:
-export async function proxy(request: NextRequest) {
+export async function middleware(request: NextRequest) {📝 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.
| export async function proxy(request: NextRequest) { | |
| export async function middleware(request: NextRequest) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/proxy.ts` at line 6, The exported function named proxy must be renamed to
middleware and the source file renamed to middleware.ts so Next.js will load it
as middleware; update the export signature from export async function
proxy(request: NextRequest) to export async function middleware(request:
NextRequest) and move/rename the file accordingly, ensuring any internal
references or imports that called proxy are updated to middleware.
| phone: string; | ||
| password: string; |
There was a problem hiding this comment.
Security Issue - Sensitive Credentials in Frontend Type Definition: The IAdmin interface includes password and salt fields in a shared types file that's likely imported across both frontend and backend code. This is a critical security vulnerability because:
- Password/Salt Should Never Reach Frontend: These fields should NEVER be included in any data sent to the frontend or stored in client-side state (Redux, React state, etc.)
- Type Definition Encourages Insecure Usage: Having these fields in the type definition suggests they might be used in frontend code, which would expose user credentials
- Related to Previous Issues: This connects to Comments Implemented a fully responsive Admin Dashboard with Batches and Students Page #12-#17 where
isAdminheaders are being sent from the client - if admin objects with passwords are being passed to frontend components, this is a severe security breach
Attack Scenario:
- If any API endpoint returns admin data matching this interface to the frontend
- Or if admin data is stored in Redux state (as suggested by the
instituteSlicechanges in this PR) - Attackers can extract password hashes and salts from browser DevTools, network requests, or Redux state
- These can be used for offline password cracking attacks
Confidence: 5/5
Recommended Action
Create separate type definitions for frontend and backend:
- Backend-only type (for database/API operations):
// In a backend-only file (e.g., src/types/backend.ts)
export interface IAdminDB {
firstname: string;
lastname: string;
email: string;
role: "admin" | "super_admin";
phone: string;
password: string; // Only in backend
salt: string; // Only in backend
avatar: {
public_id: string;
url: string;
};
permissions: {
manageUsers: boolean;
manageBatches: boolean;
manageClasses: boolean;
manageTeachers: boolean;
manageStudents: boolean;
managePayments: boolean;
manageReports: boolean;
manageSettings: boolean;
manageAdmins: boolean;
};
institutes: string[];
lastLogin: string | null;
status: "Active" | "Inactive" | "Suspended";
activityLog: Array<{
action: string;
timestamp: string;
details?: string;
ipAddress?: string;
}>;
createdAt: string;
updatedAt: string;
}- Frontend-safe type (for UI/Redux):
// In this file (src/helpers/types/index.ts)
export interface IAdmin {
firstname: string;
lastname: string;
email: string;
role: "admin" | "super_admin";
phone: string;
// password and salt REMOVED - never send to frontend
avatar: {
public_id: string;
url: string;
};
permissions: {
manageUsers: boolean;
manageBatches: boolean;
manageClasses: boolean;
manageTeachers: boolean;
manageStudents: boolean;
managePayments: boolean;
manageReports: boolean;
manageSettings: boolean;
manageAdmins: boolean;
};
institutes: string[];
lastLogin: string | null;
status: "Active" | "Inactive" | "Suspended";
createdAt: string;
updatedAt: string;
}Critical verification needed:
- Review ALL API endpoints that return admin data to ensure they NEVER include
passwordorsaltfields - Check Redux slices and React components to ensure admin credentials are never stored in frontend state
- Implement backend data sanitization to strip sensitive fields before sending to frontend
- Add TypeScript utility types to enforce this separation (e.g.,
Omit<IAdminDB, 'password' | 'salt'>)
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/helpers/types/index.ts at lines 594-595, the IAdmin interface includes password
and salt fields in a shared types file that's used across the application. This is a
critical security vulnerability because these sensitive credential fields should NEVER
be sent to the frontend or stored in client-side state. Remove the password and salt
fields from the IAdmin interface in this file. Create a separate backend-only type
definition (e.g., IAdminDB in src/types/backend.ts) that includes these fields for
database operations only. Review ALL API endpoints that return admin data to ensure
they never include password or salt fields in responses. Check Redux slices and React
components to ensure admin credentials are never stored in frontend state. Implement
backend data sanitization to strip sensitive fields before sending to frontend.
📍 This suggestion applies to lines 594-595
|
|
||
| try { | ||
| const res = await fetch( | ||
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/institute/${instituteId}`, |
There was a problem hiding this comment.
Security Issue - Backend API URL Exposure in Server Action: Using NEXT_PUBLIC_ADMIN_API_BASE_URL exposes the internal admin API base URL to all client-side code. Environment variables prefixed with NEXT_PUBLIC_* are embedded in the client-side JavaScript bundle and visible to anyone inspecting browser DevTools or network requests.
Why this matters for server actions: While this is a server action (runs on the server), Next.js bundles server actions into the client-side JavaScript to enable client-side invocation. This means the NEXT_PUBLIC_* variable and the full API endpoint structure become visible in the client bundle.
Security Risks:
- Attackers can discover the backend admin API endpoint structure
- If the backend API is meant to be internal/private, this exposure defeats that security boundary
- Attackers can attempt direct attacks against the exposed backend API endpoints
- The full API base URL becomes public knowledge, making targeted attacks easier
This same issue affects: - Line 81 in the new
getActiveInstitutefunction - Line 110 in the existing
getAllUserInstitutesfunction (not shown in this patch but likely present) - Other functions in this file that use the same pattern
Confidence: 5/5
Recommended Action
Use a server-only environment variable instead:
Since this is a Next.js server action, use a server-only environment variable (without the NEXT_PUBLIC_ prefix):
const res = await fetch(
`${process.env.ADMIN_API_BASE_URL}/api/institute/${instituteId}`,
{
method: "GET",
headers: {
"Content-Type": "application/json",
Cookie: `token=${token}`,
isAdmin: "true",
},
credentials: "include",
}
);Then in your .env file:
ADMIN_API_BASE_URL=https://internal-admin-api.example.com
Benefits:
- Backend API URL remains server-side only
- Not exposed in client-side JavaScript bundle
- Reduces attack surface by hiding internal API structure
- Follows security best practice of not exposing internal endpoints
Apply this fix to ALL functions in this file that useNEXT_PUBLIC_ADMIN_API_BASE_URL: createInstitute(line 25)getMyInstitute(line 52)getActiveInstitute(line 81) - this patchgetAllUserInstitutes(line 110) - this patch
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/institute_actions.ts at line 81 (and other fetch calls in this file),
the code uses process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL which exposes the internal
admin API base URL to the client-side JavaScript bundle. Even though these are server
actions, Next.js bundles them for client-side invocation, making NEXT_PUBLIC_*
variables visible in the browser. Replace all instances of
process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL with process.env.ADMIN_API_BASE_URL
(without the NEXT_PUBLIC_ prefix) throughout this file. Update your .env file to
define ADMIN_API_BASE_URL with the backend API URL. This prevents exposing internal
API endpoints to the client-side JavaScript bundle and reduces the attack surface.
src/actions/student_action.ts
Outdated
| headers: { | ||
| "Content-Type": "application/json", | ||
| Cookie: `token=${token}`, | ||
| isAdmin: "true", |
There was a problem hiding this comment.
Same critical security issue as Comments #10, #11, and #12 - Adding isAdmin: "true" as a client-controlled request header creates a severe authorization bypass vulnerability if the backend API trusts this header for authorization decisions.
Impact for this endpoint: The /api/student/add/${instituteId} endpoint receives this client-controlled header. If the backend uses this header to determine admin privileges for adding students to institutes, any authenticated user can manipulate this header to gain admin access.
Attack scenario: Attacker modifies the request header to isAdmin: "true" and can add arbitrary students to any institute, bypassing proper authorization checks.
Confidence: 5/5
Suggested Fix
| isAdmin: "true", | |
| Cookie: `token=${token}`, | |
Remove the isAdmin: "true" header. The backend MUST determine admin status from the JWT token validation, not from client-provided headers.
Critical verification required:
- Review the backend
/api/student/add/${instituteId}endpoint - Ensure it validates admin status from the JWT token, NOT from the
isAdminheader - If the header is needed for routing only, document this clearly and ensure backend ignores it for authorization
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/student_action.ts at line 47, a client-controlled header "isAdmin: true"
is being sent to the backend API endpoint /api/student/add/${instituteId}. This is the
same critical security vulnerability identified in Comments #10, #11, and #12 for other
action files. Remove this header from line 47. Review the backend /api/student/add
endpoint to ensure it performs proper server-side authorization by validating admin
status from the JWT token payload, NOT from the isAdmin header. If the backend relies
on this header for authorization, this is a critical vulnerability that must be fixed
immediately by implementing proper server-side JWT validation and role checking.
| const { pathname } = request.nextUrl; | ||
|
|
||
| const isPublicPath = | ||
| pathname === "/" || |
There was a problem hiding this comment.
Critical Security Issue - Authentication Bypass via Dead Code: Adding pathname === "/" to the isPublicPath check creates an authentication bypass vulnerability. This change makes the root path "/" publicly accessible, but the subsequent authentication check on lines 21-23 becomes unreachable dead code.
Why this is a critical vulnerability:
- Line 11 marks
"/"as a public path (isPublicPath = true) - Line 17 checks
if (!token && !isPublicPath)- this will be FALSE for"/"becauseisPublicPathis true - Lines 21-23 check
if (!token && pathname === "/")- this code is UNREACHABLE because line 17 already handled the!tokencase for public paths - Result: Unauthenticated users can access
"/"without being redirected to login
Attack scenario: An unauthenticated user navigates to"/"and bypasses authentication because the path is marked as public, while the intended authentication check (lines 21-23) never executes.
Confidence: 5/5
Suggested Fix
| pathname === "/" || | |
| const isPublicPath = | |
| pathname.startsWith("/login") || | |
| pathname.startsWith("/api/auth/login") || | |
| pathname.startsWith("/api/google/auth"); | |
Remove pathname === "/" from the public paths list. The root path should NOT be public - it should require authentication. The check on lines 21-23 will then properly execute and redirect unauthenticated users to login.
Alternative approach (if you want "/" to be a landing page):
If the root path should be accessible to both authenticated and unauthenticated users with different behaviors, handle it separately BEFORE the isPublicPath check:
// Handle root path separately
if (pathname === "/") {
if (!token) {
return NextResponse.redirect(new URL("/login", request.url));
}
// Authenticated users continue to lines 27-31
}
const isPublicPath =
pathname.startsWith("/login") ||
pathname.startsWith("/api/auth/login") ||
pathname.startsWith("/api/google/auth");Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/proxy.ts at line 11, adding pathname === "/" to the isPublicPath check creates
a critical authentication bypass vulnerability. This makes the root path publicly
accessible, but the authentication check on lines 21-23 becomes unreachable dead code
because line 17's condition (!token && !isPublicPath) will be false for "/" when
there's no token. Remove pathname === "/" from line 11 so the root path requires
authentication. The check on lines 21-23 will then properly execute and redirect
unauthenticated users to login. If the root path needs special handling for both
authenticated and unauthenticated users, move the pathname === "/" check to a separate
condition BEFORE the isPublicPath logic to ensure proper authentication enforcement.
| const user = await getUser(); | ||
|
|
||
| return NextResponse.redirect( | ||
| new URL(`/institute/${user.admin.institutes[0].toString()}`, request.url) | ||
| ); |
There was a problem hiding this comment.
Security Issue - Unhandled Error in Authentication Redirect: The code calls getUser() and accesses user.admin.institutes[0] without error handling. If getUser() fails, returns null, or the admin has no institutes, this will cause an unhandled exception that crashes the middleware.
Potential failure scenarios:
getUser()returns null/undefined (authentication token invalid or expired)user.adminis undefined (user object structure mismatch)user.admin.institutesis empty array (admin has no institutes assigned)user.admin.institutes[0]is undefined (array access out of bounds)
Impact: Middleware crash causes the entire application to fail for authenticated users trying to access public paths. This creates a denial of service condition where legitimate users cannot access the application.
Confidence: 5/5
Suggested Fix
| const user = await getUser(); | |
| return NextResponse.redirect( | |
| new URL(`/institute/${user.admin.institutes[0].toString()}`, request.url) | |
| ); | |
| if (token && isPublicPath) { | |
| try { | |
| const user = await getUser(); | |
| if (!user?.admin?.institutes?.length) { | |
| // Admin has no institutes, redirect to create-institute or error page | |
| return NextResponse.redirect(new URL("/create-institute", request.url)); | |
| } | |
| return NextResponse.redirect( | |
| new URL(`/institute/${user.admin.institutes[0].toString()}`, request.url) | |
| ); | |
| } catch (error) { | |
| // If getUser fails, clear the invalid token and redirect to login | |
| const response = NextResponse.redirect(new URL("/login", request.url)); | |
| response.cookies.delete("leadlly.in_admin_token"); | |
| return response; | |
| } | |
Benefits:
- Handles
getUser()failures gracefully - Validates that admin has institutes before accessing array
- Provides fallback behavior for edge cases
- Prevents middleware crashes that would affect all users
- Clears invalid tokens to prevent repeated failures
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/proxy.ts at lines 27-31, the code calls getUser() and accesses
user.admin.institutes[0] without error handling. If getUser() fails, returns null, or
the admin has no institutes, this will cause an unhandled exception that crashes the
middleware, creating a denial of service condition. Wrap lines 27-31 in a try-catch
block. In the try block, validate that user?.admin?.institutes?.length exists before
accessing institutes[0]. If the admin has no institutes, redirect to /create-institute.
In the catch block, clear the invalid authentication token and redirect to /login. This
prevents middleware crashes and provides graceful fallback behavior for edge cases.
📍 This suggestion applies to lines 27-31
… and improved error handling
| > | ||
| <Avatar className="h-8 w-8 rounded-lg"> | ||
| <AvatarImage | ||
| src={user.avatar.url} |
There was a problem hiding this comment.
Security Vulnerability: Unvalidated URL in Avatar Image Source
Same issue as Comments #1 and #5 - The user.avatar.url is used directly as the image source without validation. This creates a potential XSS vulnerability if the URL contains malicious content (e.g., javascript: protocol) or could be used for tracking/data exfiltration. This occurs twice in this component (lines 71 and 99).
Confidence: 5/5
Suggested Fix
Create a URL validation helper function and use it for both avatar instances:
const getSafeImageUrl = (url: string | undefined): string | undefined => {
if (!url) return undefined;
try {
const parsed = new URL(url);
if (parsed.protocol === 'http:' || parsed.protocol === 'https:') {
return url;
}
} catch {
return undefined;
}
return undefined;
};Then apply it to both AvatarImage components (lines 70-73 and 98-101). Additionally, ensure your Next.js next.config.js has proper image domain restrictions configured in images.remotePatterns.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/(root)/institute/[instituteId]/_components/nav-user.tsx at lines 71 and 99, the user.avatar.url is used directly as an image source without validation, creating a potential XSS vulnerability; create a helper function getSafeImageUrl at the top of the component that validates URL protocols to only allow http/https and returns undefined for invalid URLs, apply this function to both AvatarImage src props at lines 71 and 99, and ensure your Next.js configuration has proper image domain restrictions in next.config.js using images.remotePatterns to whitelist allowed image domains.
| const res = await fetch("/api/auth/logout", { | ||
| method: "GET", |
There was a problem hiding this comment.
Security Vulnerability: CSRF Risk - Logout Using GET Method
The logout functionality uses a GET request (method: "GET"), which is vulnerable to Cross-Site Request Forgery (CSRF) attacks. An attacker could force a user to logout by embedding a malicious image tag or link like <img src="/api/auth/logout" /> on any website. Logout (and any state-changing operation) should use POST, PUT, or DELETE methods with CSRF protection.
Confidence: 5/5
Suggested Fix
Change the logout request to use POST method:
| const res = await fetch("/api/auth/logout", { | |
| method: "GET", | |
| try { | |
| const res = await fetch("/api/auth/logout", { | |
| method: "POST", | |
| }); | |
| if (!res.ok) { | |
| throw new Error("Failed to logout"); | |
| } | |
You'll also need to update the corresponding API route handler at /api/auth/logout/route.ts to accept POST requests instead of GET. Ensure the API route includes proper CSRF protection (Next.js provides this by default for POST requests).
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/(root)/institute/[instituteId]/_components/nav-user.tsx at line 43-44, the logout functionality uses a GET request which is vulnerable to CSRF attacks; change the fetch method from "GET" to "POST", and update the corresponding API route handler in src/app/api/auth/logout/route.ts to accept POST requests instead of GET (change the export from 'export async function GET' to 'export async function POST'), ensuring proper CSRF protection is maintained for state-changing operations.
📍 This suggestion applies to lines 43-44
| children: React.ReactNode; | ||
| params: Promise<{ instituteId: string }>; | ||
| }) => { | ||
| const { instituteId } = await params; |
There was a problem hiding this comment.
Security Vulnerability: Unvalidated Route Parameter at Entry Point
The instituteId is extracted from route parameters without validation and then passed to child components that use it in server actions and URL construction. This is the entry point where the parameter enters the component tree, making it a critical location for validation.
Since child components (InstituteSidebar, NavMain) use this parameter in server actions and database queries (as identified in Comments #4 and #7), an attacker could inject malicious values like SQL injection payloads, NoSQL operators, or path traversal sequences (e.g., ../../admin, '; DROP TABLE--).
Validating at this layout level provides defense-in-depth protection for all child components.
Confidence: 5/5
Suggested Fix
Add validation immediately after extracting instituteId from params:
| const { instituteId } = await params; | |
| const { instituteId } = await params; | |
| // Validate instituteId format (adjust regex based on your ID format) | |
| if (!instituteId || instituteId === "undefined" || !/^[a-zA-Z0-9_-]+$/.test(instituteId)) { | |
| throw new Error('Invalid institute ID'); | |
| } | |
This validation should:
- Check for null/undefined values
- Reject the string "undefined"
- Ensure the ID matches your expected format (UUID, numeric, alphanumeric, etc.)
- Throw an error to prevent rendering with invalid IDs
Important: This validation complements but does not replace validation in child components and server actions. Implement defense-in-depth by validating at multiple layers.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/(root)/institute/[instituteId]/layout.tsx at line 16, the instituteId route parameter is extracted from params without validation before being passed to child components that use it in server actions and database queries, creating a critical security vulnerability for SQL/NoSQL injection or unauthorized data access; add validation immediately after line 16 to check if instituteId is truthy, not equal to the string "undefined", and matches an expected format using a regex pattern like /^[a-zA-Z0-9_-]+$/ (adjust based on your actual ID format such as UUID or numeric), throw an Error with message 'Invalid institute ID' if validation fails to prevent the component from rendering with malicious input, and ensure this validation is consistent with validation added to child components (institute-sidebar.tsx, nav-main.tsx, institute-switcher.tsx) as part of a defense-in-depth security strategy.
| } | ||
|
|
||
| // This sets the cookie to keep the sidebar state. | ||
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; |
There was a problem hiding this comment.
Security Issue: Cookie Missing Security Attributes
The sidebar state cookie is set without security attributes (Secure, SameSite). This creates potential security risks:
- Missing
Secureflag: Cookie can be transmitted over unencrypted HTTP connections, exposing the sidebar state to network sniffing - Missing
SameSiteattribute: Cookie is vulnerable to CSRF attacks where malicious sites could manipulate the sidebar state - No
HttpOnlyconsideration: While not applicable for client-side cookies, the lack of security attributes is concerning
While sidebar state itself isn't sensitive data, following security best practices for all cookies is important to prevent potential exploitation vectors.
Confidence: 4/5
Suggested Fix
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; | |
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; Secure; SameSite=Strict`; | |
Add Secure and SameSite=Strict attributes to the cookie. The Secure flag ensures the cookie is only sent over HTTPS, and SameSite=Strict prevents the cookie from being sent in cross-site requests, protecting against CSRF attacks.
Note: If your application needs to support HTTP in development, you can conditionally add the Secure flag based on the environment:
const cookieAttributes = `path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; SameSite=Strict${
window.location.protocol === 'https:' ? '; Secure' : ''
}`;
document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; ${cookieAttributes}`;Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/components/ui/sidebar.tsx at line 94, the sidebar state cookie is set without security attributes (Secure and SameSite), creating potential security risks for cookie transmission and CSRF attacks; modify the document.cookie assignment to include 'Secure' and 'SameSite=Strict' attributes in the cookie string, changing it to `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; Secure; SameSite=Strict`, and if you need to support HTTP in development environments, conditionally add the Secure flag based on window.location.protocol to ensure the cookie is only marked Secure when using HTTPS.
| } | ||
| }; | ||
|
|
||
| export default function BatchList({ instituteId }: { instituteId: string }) { |
There was a problem hiding this comment.
Security Vulnerability: Unvalidated Route Parameter in Server Action
The instituteId parameter is used directly in the getInstituteBatch server action (line 62) without validation. This is the same critical vulnerability identified in Comments #4, #7, and #10 for other components in this PR. An attacker could inject malicious values (SQL injection payloads, NoSQL operators, path traversal sequences like ../../admin or '; DROP TABLE--) to potentially access unauthorized data or exploit database vulnerabilities.
This is particularly critical because:
useSuspenseQueryexecutes immediately when the component renders- The server action likely performs database queries with this unvalidated input
- Exploitation could lead to data breaches or unauthorized batch access
- No validation guard prevents malicious input from reaching the server action
Confidence: 5/5
Suggested Fix
Add validation at the start of the component function to ensure instituteId matches expected format before using it in any queries:
export default function BatchList({ instituteId }: { instituteId: string }) {
// Validate instituteId format (adjust regex based on your ID format)
const isValidInstituteId = /^[a-zA-Z0-9_-]+$/.test(instituteId);
if (!isValidInstituteId || !instituteId || instituteId === "undefined") {
throw new Error('Invalid institute ID');
}
const [filters, setFilters] = useState({
standard: "",
});Critical: Ensure your server action getInstituteBatch also performs input validation and uses parameterized queries to prevent injection attacks. Defense in depth is essential - validate at both the component level and the server action level.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/(root)/institute/[instituteId]/batches/_components/batch-list.tsx at line 52, the instituteId parameter is used directly in the getInstituteBatch server action call at line 62 without validation, creating a critical security vulnerability for SQL/NoSQL injection or unauthorized data access; add validation immediately after line 52 (at the start of the BatchList function) to check if instituteId matches an expected format using a regex pattern like /^[a-zA-Z0-9_-]+$/ (adjust based on your actual ID format), throw an Error with message 'Invalid institute ID' if validation fails or if instituteId is undefined or equals the string "undefined", and ensure the server action getInstituteBatch also performs its own input validation and uses parameterized queries to prevent injection attacks as a defense-in-depth measure, maintaining consistency with validation added to other components in this PR (institute-sidebar.tsx, nav-main.tsx, institute-switcher.tsx, layout.tsx).
| export default function CreateBatch({ | ||
| instituteId, | ||
| standard, | ||
| trigger | ||
| }: { | ||
| instituteId: string; | ||
| standard?: string; | ||
| trigger?: React.ReactNode; | ||
| }) { |
There was a problem hiding this comment.
Security Vulnerability: Unvalidated Route Parameter in Server Action
The instituteId parameter is accepted as a prop and used directly in the createBatch server action (line 110) without validation. This is the same critical vulnerability identified in Comments #4, #7, #10, and #12 for other components in this PR.
An attacker could potentially:
- Inject malicious values (SQL injection payloads, NoSQL operators like
'; DROP TABLE--,{"$ne": null}) - Create batches in unauthorized institutes by manipulating the
instituteId - Exploit database vulnerabilities if the server action doesn't properly validate input
This is particularly critical because: - The component accepts
instituteIdas a prop without validation - The server action
createBatchreceives this unvalidated input - Exploitation could lead to unauthorized batch creation or data breaches
- No validation guard prevents malicious input from reaching the server action
Confidence: 5/5
Suggested Fix
Add validation at the start of the component function to ensure instituteId matches expected format before using it:
| export default function CreateBatch({ | |
| instituteId, | |
| standard, | |
| trigger | |
| }: { | |
| instituteId: string; | |
| standard?: string; | |
| trigger?: React.ReactNode; | |
| }) { | |
| export default function CreateBatch({ | |
| instituteId, | |
| standard, | |
| trigger | |
| }: { | |
| instituteId: string; | |
| standard?: string; | |
| trigger?: React.ReactNode; | |
| }) { | |
| // Validate instituteId format (adjust regex based on your ID format) | |
| const isValidInstituteId = /^[a-zA-Z0-9_-]+$/.test(instituteId); | |
| if (!isValidInstituteId || !instituteId || instituteId === "undefined") { | |
| throw new Error('Invalid institute ID'); | |
| } | |
| const [open, setOpen] = useState(false); | |
This validation should:
- Check for null/undefined values
- Reject the string "undefined"
- Ensure the ID matches your expected format (UUID, numeric, alphanumeric, etc.)
- Throw an error to prevent rendering with invalid IDs
Critical: Ensure your server actioncreateBatchalso performs input validation and uses parameterized queries to prevent injection attacks. Defense in depth is essential - validate at both the component level and the server action level.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/(root)/institute/[instituteId]/batches/_components/create-batch.tsx at lines 70-78, the instituteId parameter is accepted as a prop without validation and then used directly in the createBatch server action call at line 110, creating a critical security vulnerability for SQL/NoSQL injection or unauthorized batch creation; add validation immediately after line 78 (at the start of the CreateBatch function body) to check if instituteId matches an expected format using a regex pattern like /^[a-zA-Z0-9_-]+$/ (adjust based on your actual ID format such as UUID or numeric), throw an Error with message 'Invalid institute ID' if validation fails or if instituteId is undefined or equals the string "undefined", and ensure the server action createBatch in @/actions/batch_actions also performs its own input validation and uses parameterized queries to prevent injection attacks as a defense-in-depth measure, maintaining consistency with validation added to other components in this PR (institute-sidebar.tsx, nav-main.tsx, institute-switcher.tsx, layout.tsx, batch-list.tsx).
📍 This suggestion applies to lines 70-78
| const { instituteId } = await params; | ||
|
|
||
| const queryClient = new QueryClient(); | ||
|
|
||
| // Prefetch the batches route. | ||
| await queryClient.prefetchQuery({ | ||
| queryKey: ["institute_batches", instituteId], | ||
| queryFn: () => getInstituteBatch(instituteId), |
There was a problem hiding this comment.
Security Vulnerability: Unvalidated Route Parameter in Server Action
The instituteId is extracted from params at line 19 and used directly in the getInstituteBatch server action at line 26 without validation. This is similar to the issues identified in Comments #4, #10, and #12 for other components in this PR.
An attacker could inject malicious values (SQL injection payloads, NoSQL operators like '; DROP TABLE--, {"$ne": null}, or path traversal sequences like ../../admin) to potentially access unauthorized data or exploit database vulnerabilities.
This is particularly critical because:
- This is a server component that executes on the server
- The server action
getInstituteBatchlikely performs database queries with this unvalidated input - The prefetch query executes before any child component validation
- Exploitation could lead to data breaches or unauthorized batch access
While Comment Implemented a fully responsive Admin Dashboard with Batches and Students pages (Bonus Included) #10 suggests validation at the layout level, this page component should also validate since it directly calls a server action. Defense-in-depth requires validation at multiple layers.
Confidence: 5/5
Suggested Fix
Add validation immediately after extracting instituteId from params:
| const { instituteId } = await params; | |
| const queryClient = new QueryClient(); | |
| // Prefetch the batches route. | |
| await queryClient.prefetchQuery({ | |
| queryKey: ["institute_batches", instituteId], | |
| queryFn: () => getInstituteBatch(instituteId), | |
| const { instituteId } = await params; | |
| // Validate instituteId format (adjust regex based on your ID format) | |
| if (!instituteId || instituteId === "undefined" || !/^[a-zA-Z0-9_-]+$/.test(instituteId)) { | |
| throw new Error('Invalid institute ID'); | |
| } | |
| const queryClient = new QueryClient(); | |
| // Prefetch the batches route. | |
| await queryClient.prefetchQuery({ | |
| queryKey: ["institute_batches", instituteId], | |
| queryFn: () => getInstituteBatch(instituteId), | |
| }); | |
This validation should:
- Check for null/undefined values
- Reject the string "undefined"
- Ensure the ID matches your expected format (UUID, numeric, alphanumeric, etc.)
- Throw an error to prevent execution with invalid IDs
Critical: Ensure your server actiongetInstituteBatchalso performs input validation and uses parameterized queries to prevent injection attacks. Defense in depth is essential - validate at the layout level (Comment Implemented a fully responsive Admin Dashboard with Batches and Students pages (Bonus Included) #10), this page level, the child component level (Comment Implemented a fully responsive Admin Dashboard with Batches and Students Page #12), and within the server action itself.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/(root)/institute/[instituteId]/batches/page.tsx at line 19, the instituteId route parameter is extracted from params without validation and then used directly in the getInstituteBatch server action call at line 26, creating a critical security vulnerability for SQL/NoSQL injection or unauthorized data access; add validation immediately after line 19 to check if instituteId is truthy, not equal to the string "undefined", and matches an expected format using a regex pattern like /^[a-zA-Z0-9_-]+$/ (adjust based on your actual ID format such as UUID or numeric), throw an Error with message 'Invalid institute ID' if validation fails to prevent the prefetch query from executing with malicious input, ensure this validation is consistent with validation added to the layout (Comment #10), child components (Comments #12, #13), and other components in this PR, and ensure the server action getInstituteBatch in @/actions/batch_actions also performs its own input validation and uses parameterized queries as a defense-in-depth measure.
📍 This suggestion applies to lines 19-26
| "p-1", | ||
| position === "popper" && | ||
| "h-[var(--radix-select-trigger-height)] w-full min-w-[var(--radix-select-trigger-width)]" | ||
| "h-(--radix-select-trigger-height) w-full min-w-(--radix-select-trigger-width)" |
There was a problem hiding this comment.
Invalid Tailwind CSS Syntax - Component Styling Will Break
The CSS variable syntax was changed from h-[var(--radix-select-trigger-height)] to h-(--radix-select-trigger-height), which is invalid Tailwind CSS syntax. Tailwind requires square brackets [] for arbitrary values, not parentheses (). This will cause the height and min-width styles to not apply, breaking the select dropdown's positioning and sizing.
Confidence: 5/5
Suggested Fix
| "h-(--radix-select-trigger-height) w-full min-w-(--radix-select-trigger-width)" | |
| "h-[var(--radix-select-trigger-height)] w-full min-w-[var(--radix-select-trigger-width)]" | |
Revert to the original syntax using square brackets [] instead of parentheses () for the CSS variable references. This is the correct Tailwind CSS arbitrary value syntax.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/components/ui/select.tsx at line 92, the CSS variable syntax was incorrectly changed from square brackets to parentheses (h-(--radix-select-trigger-height) and min-w-(--radix-select-trigger-width)), which is invalid Tailwind CSS syntax; revert the line to use square brackets for arbitrary values: "h-[var(--radix-select-trigger-height)] w-full min-w-[var(--radix-select-trigger-width)]" to ensure the select dropdown's height and width styles are properly applied.
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
src/app/api/auth/logout/route.ts (1)
3-14:⚠️ Potential issue | 🔴 CriticalStill not forwarding the user's auth to the upstream logout call.
credentials: "include"here does not make the route handler forward the incoming browser cookie, so the admin API can receive an unauthenticated logout request. Because the response is also ignored, this handler still clears the local cookie and returns success even if the upstream session was never invalidated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/auth/logout/route.ts` around lines 3 - 14, The GET route handler is not forwarding the incoming auth cookie to the upstream logout and ignores the upstream response; update the GET function to read the incoming request's Cookie header (e.g., req.headers.get("cookie") or using NextRequest.cookie) and pass it as the "cookie" header in the fetch to `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/auth/logout`, preserve or remove credentials as needed, then await and inspect the upstream response (status and body) before clearing the local auth cookie and returning success — only clear the local cookie and return 200 when the upstream logout succeeded, otherwise propagate an appropriate error response.
🧹 Nitpick comments (11)
src/app/(protected)/(root)/layout.tsx (1)
3-4: Prefer a fragment/pass-through instead of an unconditional wrapper<div>.Line 4 adds an extra DOM node with no behavior. In layout trees, this can unexpectedly affect CSS selectors and spacing. Consider returning
children(or a fragment) directly.♻️ Proposed simplification
-const MainLayout = async ({ children }: { children: React.ReactNode }) => { - return <div>{children}</div>; -}; +const MainLayout = async ({ children }: { children: React.ReactNode }) => { + return <>{children}</>; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/layout.tsx around lines 3 - 4, MainLayout currently wraps its children in an unconditional <div>, which introduces an extra DOM node and can break layout/CSS; change the implementation of the MainLayout component in layout.tsx to return the children directly (or a React fragment) instead of <div>{children}</div> so the component becomes a pass-through (keep the async signature and props type as-is, just replace the JSX return).src/components/ui/sheet.tsx (1)
23-26: Minor: Extra whitespace in className string.There's a double space between
bg-black/80anddata-[state=open]on line 24. This doesn't affect functionality but is inconsistent with the rest of the file.🧹 Suggested fix
className={cn( - "fixed inset-0 z-50 bg-black/80 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0", + "fixed inset-0 z-50 bg-black/80 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0", className )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/sheet.tsx` around lines 23 - 26, The className string passed into the cn call for the sheet backdrop contains an extra space between "bg-black/80" and "data-[state=open]" which is inconsistent; open the cn(...) invocation in src/components/ui/sheet.tsx (the className prop passed to the element) and remove the duplicate space so the classes are separated by a single space, preserving the rest of the tokens and the existing className variable merge.src/app/(protected)/(root)/institute/[instituteId]/batches/_components/create-batch.tsx (2)
260-274: Inline textarea styling duplicates theTextareacomponent's styles.Lines 233-239 use the
<Textarea>component, but lines 260-274 use a raw<textarea>with manually duplicated styling. This creates maintenance burden and potential style inconsistencies.Consider extending the
Textareacomponent to support thefont-mono whitespace-prevariant, or extract the shared styles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/create-batch.tsx around lines 260 - 274, The raw <textarea> in create-batch.tsx duplicates styling from the project's Textarea component; replace it by using the existing Textarea component (or extend Textarea to accept the additional variant props) so styles are centralized. Update the element at the create-batch component where the raw <textarea> is used to instead render <Textarea ...> passing through className="font-mono whitespace-pre resize-y min-h-[120px]" and the same style prop (whiteSpace/overflow settings) and value handling (value={field.value || ""}) or extend the Textarea component API to accept and apply these className/style/value overrides so you remove duplicated inline styles.
322-331:type="number"input can accept negative values despitemin={0}.While
min={0}provides a UI hint, users can still type or paste negative numbers. ThesuperRefinevalidation handles this for "Paid" batches, but consider addingonKeyDownto prevent minus key entry for better UX.💡 Optional UX improvement
<Input id="batch-price" type="number" min={0} placeholder="0" className="pl-9 shadow-none" aria-invalid={fieldState.invalid} {...field} value={field.value ?? ""} + onKeyDown={(e) => { + if (e.key === '-' || e.key === 'e') { + e.preventDefault(); + } + }} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/create-batch.tsx around lines 322 - 331, The numeric price Input (id="batch-price") still allows negative characters despite min={0}; add an onKeyDown handler on the Input to block minus/negative entry (e.g., preventDefault() when event.key === '-' or event.key === 'Minus') and also add an onPaste handler to sanitize pasted values (reject or strip leading '-' before calling field.onChange) while preserving existing props ({...field}, value handling and aria-invalid from fieldState); this complements the existing superRefine validation for "Paid" batches and improves UX.src/app/(protected)/(root)/institute/[instituteId]/batches/_components/batch-list.tsx (3)
60-65: Type assertion without runtime validation risks runtime errors.The
data?.datais cast toApiBatch[]without any runtime type checking. SincegetInstituteBatchreturnsPromise<unknown>(untypedres.json()), any API response shape mismatch will cause silent failures or runtime errors.Consider adding runtime validation with Zod or at minimum a type guard, especially since you're already using Zod in this codebase.
💡 Example validation approach
// At module level, add a Zod schema matching ApiBatch const apiBatchSchema = z.object({ _id: z.string(), name: z.string(), standard: z.string(), status: z.enum(["Active", "Inactive", "Completed"]), // ... other fields }); // In component const rawBatches = data?.data ?? []; const batches = z.array(apiBatchSchema).safeParse(rawBatches); if (!batches.success) { console.error("Invalid batch data:", batches.error); // Handle gracefully }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/batch-list.tsx around lines 60 - 65, The code unsafely casts data?.data to ApiBatch[]; add runtime validation using Zod (or a type guard) before assigning to batches: define a zod schema (e.g., apiBatchSchema) matching ApiBatch, parse/validate the raw response from useSuspenseQuery (returned by getInstituteBatch) with z.array(apiBatchSchema).safeParse(raw), log or handle validation failures gracefully, and only set batches to the parsed result when safeParse.success is true; keep useSuspenseQuery and getInstituteBatch unchanged but replace the direct cast to ApiBatch[] with this validated assignment.
150-155: Magic number 180 for default capacity should be a named constant.The default
totalCapacityof 180 is unexplained. Extract this to a named constant for clarity and maintainability.♻️ Proposed refactor
+const DEFAULT_BATCH_CAPACITY = 180; + // In the map callback: -const totalCapacity = batch.totalCapacity ?? 180; +const totalCapacity = batch.totalCapacity ?? DEFAULT_BATCH_CAPACITY;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/batch-list.tsx around lines 150 - 155, Replace the magic number 180 used as the fallback for batch.totalCapacity with a named constant (e.g., DEFAULT_BATCH_CAPACITY) and use that constant where totalCapacity is computed; update the code around totalStudents/totalCapacity and the progress calculation (references: batch.totalCapacity, totalCapacity, progress) so the default capacity is declared once as a clearly named constant and reused instead of the literal 180.
37-50:getBatchLogoBgonly handles a few hardcoded batch names.This helper returns specific colors only for "Omega", "Sigma", "Alpha", and "Beta". All other batch names fall through to indigo. Consider a more scalable approach using consistent hashing or allowing custom colors from the API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/batch-list.tsx around lines 37 - 50, getBatchLogoBg currently maps only four hardcoded names to colors and defaults everyone else to indigo; replace this with a deterministic mapping so any batchName yields a consistent palette color (or prefer an API-provided color when available). Implement a small color palette array and compute an index from the batchName (e.g., simple hash by summing char codes mod palette.length or a djb2-style hash) inside getBatchLogoBg(batchName) to pick palette[index]; also check for and return batch.color or batch.logoColor if the batch object provides it. This keeps getBatchLogoBg scalable and stable across names while allowing API overrides.src/app/globals.css (2)
107-132: Avoid duplicating invariant tokens in.dark.Lines 167-190 repeat the same font/radius/shadow token values already set in
:root. Keeping only true dark-mode deltas in.darkwill reduce drift risk.Also applies to: 167-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/globals.css` around lines 107 - 132, The .dark block duplicates invariant design tokens already defined in :root (e.g., --font-sans, --font-serif, --font-mono, --radius, --shadow-*, --tracking-normal, --spacing); remove those redundant declarations from the .dark selector and keep only tokens that actually differ for dark mode (delta overrides), leaving all unchanged tokens to inherit from :root to avoid drift and duplication.
5-13: Remove the overriddenmax-widthrule in the container utility.At Line 9,
max-width: noneis effectively overridden by Line 12 for all widths>= 1400px, so this block is dead and can be removed for clarity.♻️ Proposed simplification
`@utility` container { margin-inline: auto; padding-inline: 2rem; - `@media` (width >= --theme(--breakpoint-2xl)) { - max-width: none; - } `@media` (width >= 1400px) { max-width: 1400px; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/globals.css` around lines 5 - 13, The `@utility` container definition contains a redundant media query that sets "max-width: none" (the block starting with "@media (width >= --theme(--breakpoint-2xl))") which is effectively overridden by the later "@media (width >= 1400px) { max-width: 1400px }"; remove the entire "@media (width >= --theme(--breakpoint-2xl)) { max-width: none; }" block from the container utility so only the needed padding/margin and the 1400px max-width media rule remain.src/app/(protected)/(root)/institute/[instituteId]/_components/nav-main.tsx (1)
57-61: Match active routes on a path-segment boundary.
pathname.startsWith(item.href(instituteId))will also mark any sibling path with the same prefix as active. Comparing against the exact href orhref + "/"keeps nested pages highlighted without accidental matches.🧭 Suggested tweak
{navItems.map((item) => ( + (() => { + const href = item.href(instituteId); + const isActive = item.exact + ? pathname === href + : pathname === href || pathname.startsWith(`${href}/`); + + return ( <SidebarMenuItem key={item.label}> <SidebarMenuButton asChild tooltip={item.label} - isActive={ - item.exact - ? pathname === item.href(instituteId) - : pathname.startsWith(item.href(instituteId)) - } + isActive={isActive} > - <Link href={item.href(instituteId)}> + <Link href={href}> <item.icon className="size-4" /> <span>{item.label}</span> </Link> </SidebarMenuButton> </SidebarMenuItem> + ); + })() ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(protected)/(root)/institute/[instituteId]/_components/nav-main.tsx around lines 57 - 61, The active-route check in isActive incorrectly uses pathname.startsWith(item.href(instituteId)) which can match sibling routes; update the non-exact branch to treat the href as a path segment boundary by computing const href = item.href(instituteId) and using (pathname === href || pathname.startsWith(href + '/')) instead of plain startsWith; keep the exact branch using item.exact and pathname === href so nested pages remain highlighted without accidental prefix matches.src/components/ui/sidebar.tsx (1)
107-120: Ignore the sidebar shortcut while focus is in an editable control.The global
Ctrl/Cmd+Bhandler also runs insideinput,textarea,select, andcontenteditableelements. That can steal expected editing/browser shortcuts from forms rendered in or around the sidebar.⌨️ Suggested guard
React.useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { + const target = event.target as HTMLElement | null; + const isEditable = + !!target && + (target.isContentEditable || + /^(INPUT|TEXTAREA|SELECT)$/.test(target.tagName)); + + if (isEditable) return; + if ( event.key === SIDEBAR_KEYBOARD_SHORTCUT && (event.metaKey || event.ctrlKey) ) { event.preventDefault();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/sidebar.tsx` around lines 107 - 120, The global shortcut handler inside the useEffect (handleKeyDown used by toggleSidebar and SIDEBAR_KEYBOARD_SHORTCUT) needs to ignore key events that originate from editable elements; update handleKeyDown to early-return when the event target (or document.activeElement) is an input, textarea, select, or an element with isContentEditable true (or matches a contenteditable selector), so the shortcut doesn't run while focus is in a form field or contentEditable region; keep the rest of the effect and cleanup the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/`(protected)/(root)/institute/[instituteId]/_components/institute-switcher.tsx:
- Around line 69-70: The AvatarFallback currently calls
activeInstitute?.institute?.name?.charAt(0).toUpperCase() which throws when name
is undefined; update the fallback to safely guard and only call toUpperCase when
a character exists — e.g., extract const firstChar =
activeInstitute?.institute?.name?.charAt(0) and render firstChar ?
firstChar.toUpperCase() : a safe fallback (empty string or placeholder) so
AvatarFallback and the component (referencing activeInstitute, institute, name,
charAt, toUpperCase) do not crash for invalid ids.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/_components/InstituteOverview.tsx:
- Around line 17-20: The component uses useSuspenseQuery to call
getActiveInstitute and assumes a valid payload; if getActiveInstitute resolves
with success: false or institute: null the UI will render an empty header
instead of surfacing an error. After the query resolves, validate the payload
returned in activeInstitute (check activeInstitute.success and
activeInstitute.institute) inside the InstituteOverview component and if invalid
either throw a new Error (so the surrounding ErrorBoundary can catch it) or
return an explicit fallback UI; update the code paths around useSuspenseQuery,
activeInstitute, and any render logic that reads activeInstitute.institute to
handle the invalid case deterministically.
In `@src/app/`(protected)/(root)/institute/[instituteId]/_components/nav-user.tsx:
- Around line 37-38: Guard against partial/missing user data before rendering by
null-checking data.admin and its nested fields and computing reusable safe
values (e.g., fullName, initials, avatarUrl) once to use in both avatar and name
blocks; update the component that reads data.admin (user) to first return a safe
fallback UI if no admin exists, then compute const fullName = user?.firstName ||
user?.lastName ? `${user.firstName ?? ""} ${user.lastName ?? ""}`.trim() : null,
const initials = (user?.firstName?.[0] || user?.lastName?.[0] || "U"), and const
avatarUrl = user?.avatar?.url ?? null, and replace direct dereferences
(user.avatar.url, name fields) with those variables for the avatar img src, alt,
and displayed name so the footer/logout controls never throw when fields are
missing.
- Around line 43-45: Change the logout fetch from GET to a mutating method and
update the server route to match: replace the client call in nav-user.tsx that
does fetch("/api/auth/logout", { method: "GET" }) to use method "POST" (or
"DELETE") and, in the API route that handles /api/auth/logout, rename/implement
the HTTP handler for that verb (export async function POST(...) or DELETE(...))
instead of a GET handler so the route only performs sign-out on an explicit
mutating request; ensure response handling and any middleware/CSRF checks remain
compatible with the new verb.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/batch-list.tsx:
- Around line 133-135: Replace the hardcoded "th" suffix by adding an ordinal
helper (e.g., getOrdinal(number): string) that returns the correct suffix for a
given integer (1 -> "st", 2 -> "nd", 3 -> "rd", 4+ -> "th", handling teens like
11-13 as "th"), then use it where the UI currently renders `{standard}th
standard` and `{batch.standard}th Class` (replace with string interpolation
using the number plus getOrdinal(standard) or getOrdinal(batch.standard));
update all occurrences in batch-list.tsx (including the h2 and the other two
places noted) to call this helper so ordinals display correctly.
- Around line 208-210: The status badge currently hardcodes green classes;
update the rendering in BatchList (the component that uses batch.status) to
compute classes dynamically based on batch.status (e.g., a helper
getStatusClasses(status) or a switch/lookup that returns "bg-green-100
text-green-600" for "Active", "bg-gray-100 text-gray-600" for "Inactive", and
"bg-blue-100 text-blue-600" for "Completed", with a sensible default), then
apply that computed string to the span's className instead of the hardcoded
classes so the badge color reflects the actual status.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/create-batch.tsx:
- Around line 49-52: The zod schema for the amount field currently uses
z.string().min(0, ...) which validates string length, not numeric value; remove
the .min(0) from the amount schema in create-batch.tsx (the amount property
inside the form schema) and either switch to z.coerce.number() if you want the
schema to coerce/validate numeric values, or keep it as z.string() and rely on
the existing superRefine numeric checks (the superRefine block that validates
amount value) to produce the correct "Amount must be greater than or equal to 0"
error.
In `@src/app/`(protected)/(root)/institute/[instituteId]/page.tsx:
- Around line 45-59: The hard-coded dashboardData object (students, teachers) is
shipping fabricated totals; replace it with a real fetch for the institute
metrics using the page component's async server-side data loading (use the
instituteId route param) and populate the students and teachers shapes from the
API response, and if the API is not ready or returns null/error, hide the metric
cards or show a loading/empty state instead of static numbers so different
institutes render correct data; update references to dashboardData, students,
and teachers accordingly and add error/null checks before rendering the cards.
In `@src/app/globals.css`:
- Around line 3-5: Stylelint is flagging valid Tailwind v4 at-rules (e.g.
`@custom-variant`, `@utility`, `@theme`) because the project extends
stylelint-config-standard-scss without allowing Tailwind directives; update the
Stylelint config (the .stylelintrc.json) to ignore these at-rules by adding them
to the at-rule-no-unknown ignoreAtRules list (include "custom-variant",
"utility", "theme", etc.) or switch to a Tailwind-aware syntax/preset so the
rules around `@custom-variant`, `@utility`, and `@theme` in globals.css are accepted
and CI false-positives stop.
In `@src/components/create-institute-form.tsx`:
- Around line 197-205: The FieldLabel components (e.g., FieldLabel
htmlFor="name") are not bound to their controls because the corresponding
Input/SelectTrigger elements lack id props; update each labeled control (Input
and SelectTrigger instances referenced near FieldLabel) to include an id
matching the label's htmlFor (for example set id="name" for the Input associated
with FieldLabel htmlFor="name"), and apply the same pattern to the other labeled
pairs listed (lines referencing the country SelectTrigger and the other
FieldLabel/Input groups) so click-to-focus and assistive-tech label associations
work correctly.
In `@src/components/ui/select.tsx`:
- Line 79: The Tailwind CSS variable usage in the select component's class
string uses old bracket syntax; update any occurrences of
max-h-[--radix-select-content-available-height] and
origin-[--radix-select-content-transform-origin] to the Tailwind v4 parentheses
form (e.g., max-h-(--radix-select-content-available-height) and
origin-(--radix-select-content-transform-origin)), and scan the surrounding
class list in the same Select component (the long "relative z-50 ..." class
string) to make all CSS variable references use parentheses to match the
h-(--radix-select-trigger-height) style.
---
Duplicate comments:
In `@src/app/api/auth/logout/route.ts`:
- Around line 3-14: The GET route handler is not forwarding the incoming auth
cookie to the upstream logout and ignores the upstream response; update the GET
function to read the incoming request's Cookie header (e.g.,
req.headers.get("cookie") or using NextRequest.cookie) and pass it as the
"cookie" header in the fetch to
`${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/auth/logout`, preserve or
remove credentials as needed, then await and inspect the upstream response
(status and body) before clearing the local auth cookie and returning success —
only clear the local cookie and return 200 when the upstream logout succeeded,
otherwise propagate an appropriate error response.
---
Nitpick comments:
In `@src/app/`(protected)/(root)/institute/[instituteId]/_components/nav-main.tsx:
- Around line 57-61: The active-route check in isActive incorrectly uses
pathname.startsWith(item.href(instituteId)) which can match sibling routes;
update the non-exact branch to treat the href as a path segment boundary by
computing const href = item.href(instituteId) and using (pathname === href ||
pathname.startsWith(href + '/')) instead of plain startsWith; keep the exact
branch using item.exact and pathname === href so nested pages remain highlighted
without accidental prefix matches.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/batch-list.tsx:
- Around line 60-65: The code unsafely casts data?.data to ApiBatch[]; add
runtime validation using Zod (or a type guard) before assigning to batches:
define a zod schema (e.g., apiBatchSchema) matching ApiBatch, parse/validate the
raw response from useSuspenseQuery (returned by getInstituteBatch) with
z.array(apiBatchSchema).safeParse(raw), log or handle validation failures
gracefully, and only set batches to the parsed result when safeParse.success is
true; keep useSuspenseQuery and getInstituteBatch unchanged but replace the
direct cast to ApiBatch[] with this validated assignment.
- Around line 150-155: Replace the magic number 180 used as the fallback for
batch.totalCapacity with a named constant (e.g., DEFAULT_BATCH_CAPACITY) and use
that constant where totalCapacity is computed; update the code around
totalStudents/totalCapacity and the progress calculation (references:
batch.totalCapacity, totalCapacity, progress) so the default capacity is
declared once as a clearly named constant and reused instead of the literal 180.
- Around line 37-50: getBatchLogoBg currently maps only four hardcoded names to
colors and defaults everyone else to indigo; replace this with a deterministic
mapping so any batchName yields a consistent palette color (or prefer an
API-provided color when available). Implement a small color palette array and
compute an index from the batchName (e.g., simple hash by summing char codes mod
palette.length or a djb2-style hash) inside getBatchLogoBg(batchName) to pick
palette[index]; also check for and return batch.color or batch.logoColor if the
batch object provides it. This keeps getBatchLogoBg scalable and stable across
names while allowing API overrides.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/create-batch.tsx:
- Around line 260-274: The raw <textarea> in create-batch.tsx duplicates styling
from the project's Textarea component; replace it by using the existing Textarea
component (or extend Textarea to accept the additional variant props) so styles
are centralized. Update the element at the create-batch component where the raw
<textarea> is used to instead render <Textarea ...> passing through
className="font-mono whitespace-pre resize-y min-h-[120px]" and the same style
prop (whiteSpace/overflow settings) and value handling (value={field.value ||
""}) or extend the Textarea component API to accept and apply these
className/style/value overrides so you remove duplicated inline styles.
- Around line 322-331: The numeric price Input (id="batch-price") still allows
negative characters despite min={0}; add an onKeyDown handler on the Input to
block minus/negative entry (e.g., preventDefault() when event.key === '-' or
event.key === 'Minus') and also add an onPaste handler to sanitize pasted values
(reject or strip leading '-' before calling field.onChange) while preserving
existing props ({...field}, value handling and aria-invalid from fieldState);
this complements the existing superRefine validation for "Paid" batches and
improves UX.
In `@src/app/`(protected)/(root)/layout.tsx:
- Around line 3-4: MainLayout currently wraps its children in an unconditional
<div>, which introduces an extra DOM node and can break layout/CSS; change the
implementation of the MainLayout component in layout.tsx to return the children
directly (or a React fragment) instead of <div>{children}</div> so the component
becomes a pass-through (keep the async signature and props type as-is, just
replace the JSX return).
In `@src/app/globals.css`:
- Around line 107-132: The .dark block duplicates invariant design tokens
already defined in :root (e.g., --font-sans, --font-serif, --font-mono,
--radius, --shadow-*, --tracking-normal, --spacing); remove those redundant
declarations from the .dark selector and keep only tokens that actually differ
for dark mode (delta overrides), leaving all unchanged tokens to inherit from
:root to avoid drift and duplication.
- Around line 5-13: The `@utility` container definition contains a redundant media
query that sets "max-width: none" (the block starting with "@media (width >=
--theme(--breakpoint-2xl))") which is effectively overridden by the later
"@media (width >= 1400px) { max-width: 1400px }"; remove the entire "@media
(width >= --theme(--breakpoint-2xl)) { max-width: none; }" block from the
container utility so only the needed padding/margin and the 1400px max-width
media rule remain.
In `@src/components/ui/sheet.tsx`:
- Around line 23-26: The className string passed into the cn call for the sheet
backdrop contains an extra space between "bg-black/80" and "data-[state=open]"
which is inconsistent; open the cn(...) invocation in
src/components/ui/sheet.tsx (the className prop passed to the element) and
remove the duplicate space so the classes are separated by a single space,
preserving the rest of the tokens and the existing className variable merge.
In `@src/components/ui/sidebar.tsx`:
- Around line 107-120: The global shortcut handler inside the useEffect
(handleKeyDown used by toggleSidebar and SIDEBAR_KEYBOARD_SHORTCUT) needs to
ignore key events that originate from editable elements; update handleKeyDown to
early-return when the event target (or document.activeElement) is an input,
textarea, select, or an element with isContentEditable true (or matches a
contenteditable selector), so the shortcut doesn't run while focus is in a form
field or contentEditable region; keep the rest of the effect and cleanup the
same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65537f85-31b9-41de-8fd4-f5eff38d7510
📒 Files selected for processing (21)
src/app/(protected)/(root)/institute/[instituteId]/_components/InstituteOverview.tsxsrc/app/(protected)/(root)/institute/[instituteId]/_components/TeachersOverview.tsxsrc/app/(protected)/(root)/institute/[instituteId]/_components/institute-sidebar.tsxsrc/app/(protected)/(root)/institute/[instituteId]/_components/institute-switcher.tsxsrc/app/(protected)/(root)/institute/[instituteId]/_components/nav-main.tsxsrc/app/(protected)/(root)/institute/[instituteId]/_components/nav-user.tsxsrc/app/(protected)/(root)/institute/[instituteId]/batches/_components/batch-list.tsxsrc/app/(protected)/(root)/institute/[instituteId]/batches/_components/create-batch.tsxsrc/app/(protected)/(root)/institute/[instituteId]/batches/page.tsxsrc/app/(protected)/(root)/institute/[instituteId]/layout.tsxsrc/app/(protected)/(root)/institute/[instituteId]/page.tsxsrc/app/(protected)/(root)/institute/[instituteId]/teachers/page.tsxsrc/app/(protected)/(root)/layout.tsxsrc/app/(protected)/create-institute/page.tsxsrc/app/api/auth/logout/route.tssrc/app/globals.csssrc/components/create-institute-form.tsxsrc/components/ui/select.tsxsrc/components/ui/sheet.tsxsrc/components/ui/sidebar.tsxsrc/hooks/use-mobile.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/(protected)/(root)/institute/[instituteId]/batches/page.tsx
- src/app/(protected)/create-institute/page.tsx
| <AvatarFallback> | ||
| {activeInstitute?.institute?.name?.charAt(0).toUpperCase()} |
There was a problem hiding this comment.
Fix the broken optional chain in the avatar fallback.
src/actions/institute_actions.ts:77-109 already returns { success: false, institute: null } for invalid ids. In that case, name?.charAt(0).toUpperCase() still calls toUpperCase() on undefined, so the switcher crashes instead of falling back cleanly.
🛠️ Proposed fix
- {activeInstitute?.institute?.name?.charAt(0).toUpperCase()}
+ {activeInstitute?.institute?.name?.charAt(0)?.toUpperCase() ?? "?"}📝 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.
| <AvatarFallback> | |
| {activeInstitute?.institute?.name?.charAt(0).toUpperCase()} | |
| <AvatarFallback> | |
| {activeInstitute?.institute?.name?.charAt(0)?.toUpperCase() ?? "?"} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/_components/institute-switcher.tsx
around lines 69 - 70, The AvatarFallback currently calls
activeInstitute?.institute?.name?.charAt(0).toUpperCase() which throws when name
is undefined; update the fallback to safely guard and only call toUpperCase when
a character exists — e.g., extract const firstChar =
activeInstitute?.institute?.name?.charAt(0) and render firstChar ?
firstChar.toUpperCase() : a safe fallback (empty string or placeholder) so
AvatarFallback and the component (referencing activeInstitute, institute, name,
charAt, toUpperCase) do not crash for invalid ids.
| const { data: activeInstitute } = useSuspenseQuery({ | ||
| queryKey: ["active_institute", instituteId], | ||
| queryFn: () => getActiveInstitute({ instituteId }), | ||
| }); |
There was a problem hiding this comment.
Handle unsuccessful query payloads before rendering.
useSuspenseQuery only covers thrown/loading states. If getActiveInstitute resolves with success: false or institute: null, this component won't hit the surrounding ErrorBoundary; it will just render a mostly empty header. Guard the payload immediately after the query and throw or render an explicit fallback.
💡 One way to fix it
const { data: activeInstitute } = useSuspenseQuery({
queryKey: ["active_institute", instituteId],
queryFn: () => getActiveInstitute({ instituteId }),
});
+
+if (!activeInstitute.success || !activeInstitute.institute) {
+ throw new Error("Failed to load institute");
+}📝 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.
| const { data: activeInstitute } = useSuspenseQuery({ | |
| queryKey: ["active_institute", instituteId], | |
| queryFn: () => getActiveInstitute({ instituteId }), | |
| }); | |
| const { data: activeInstitute } = useSuspenseQuery({ | |
| queryKey: ["active_institute", instituteId], | |
| queryFn: () => getActiveInstitute({ instituteId }), | |
| }); | |
| if (!activeInstitute.success || !activeInstitute.institute) { | |
| throw new Error("Failed to load institute"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/_components/InstituteOverview.tsx
around lines 17 - 20, The component uses useSuspenseQuery to call
getActiveInstitute and assumes a valid payload; if getActiveInstitute resolves
with success: false or institute: null the UI will render an empty header
instead of surfacing an error. After the query resolves, validate the payload
returned in activeInstitute (check activeInstitute.success and
activeInstitute.institute) inside the InstituteOverview component and if invalid
either throw a new Error (so the surrounding ErrorBoundary can catch it) or
return an explicit fallback UI; update the code paths around useSuspenseQuery,
activeInstitute, and any render logic that reads activeInstitute.institute to
handle the invalid case deterministically.
| const user = data.admin; | ||
|
|
There was a problem hiding this comment.
Guard partial user records before rendering the footer.
data.admin, user.avatar.url, and the name fields are dereferenced twice with no null-safety. A user without an avatar or a complete name will throw here and lose the footer/logout control.
🛡️ Proposed hardening
const user = data.admin;
+ const fullName = `${user?.firstname ?? ""} ${user?.lastname ?? ""}`.trim();
+ const initials = `${user?.firstname?.charAt(0)?.toUpperCase() ?? ""}${user?.lastname?.charAt(0)?.toUpperCase() ?? ""}`;
+ const avatarUrl = user?.avatar?.url;Then reuse fullName, initials, and avatarUrl in both avatar/name blocks, e.g. src={avatarUrl}, alt={fullName || "User"}, and {initials || "U"}.
Also applies to: 70-83, 98-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(protected)/(root)/institute/[instituteId]/_components/nav-user.tsx
around lines 37 - 38, Guard against partial/missing user data before rendering
by null-checking data.admin and its nested fields and computing reusable safe
values (e.g., fullName, initials, avatarUrl) once to use in both avatar and name
blocks; update the component that reads data.admin (user) to first return a safe
fallback UI if no admin exists, then compute const fullName = user?.firstName ||
user?.lastName ? `${user.firstName ?? ""} ${user.lastName ?? ""}`.trim() : null,
const initials = (user?.firstName?.[0] || user?.lastName?.[0] || "U"), and const
avatarUrl = user?.avatar?.url ?? null, and replace direct dereferences
(user.avatar.url, name fields) with those variables for the avatar img src, alt,
and displayed name so the footer/logout controls never throw when fields are
missing.
| const res = await fetch("/api/auth/logout", { | ||
| method: "GET", | ||
| }); |
There was a problem hiding this comment.
Make logout a non-GET request.
Signing out mutates auth state, so keeping this on GET leaves the endpoint triggerable by prefetchers, crawlers, or cross-site navigation. Please switch both this call and the route handler to POST (or DELETE) so logout is an explicit action.
🔐 Proposed client-side change
- const res = await fetch("/api/auth/logout", {
- method: "GET",
- });
+ const res = await fetch("/api/auth/logout", {
+ method: "POST",
+ });📝 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.
| const res = await fetch("/api/auth/logout", { | |
| method: "GET", | |
| }); | |
| const res = await fetch("/api/auth/logout", { | |
| method: "POST", | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(protected)/(root)/institute/[instituteId]/_components/nav-user.tsx
around lines 43 - 45, Change the logout fetch from GET to a mutating method and
update the server route to match: replace the client call in nav-user.tsx that
does fetch("/api/auth/logout", { method: "GET" }) to use method "POST" (or
"DELETE") and, in the API route that handles /api/auth/logout, rename/implement
the HTTP handler for that verb (export async function POST(...) or DELETE(...))
instead of a GET handler so the route only performs sign-out on an explicit
mutating request; ensure response handling and any middleware/CSRF checks remain
compatible with the new verb.
| <h2 className="text-xl md:text-2xl font-bold"> | ||
| {standard}th standard | ||
| </h2> |
There was a problem hiding this comment.
Hardcoded "th" ordinal suffix is incorrect for standards like 1, 2, 3, 11, 12, 13.
Using {standard}th standard and {batch.standard}th Class produces incorrect English for standards 1 ("1th"), 2 ("2th"), 3 ("3th"), 11 ("11th" is correct), 12 ("12th" is correct), etc.
🛠️ Proposed fix with ordinal helper
+const getOrdinalSuffix = (n: number): string => {
+ const s = ["th", "st", "nd", "rd"];
+ const v = n % 100;
+ return n + (s[(v - 20) % 10] || s[v] || s[0]);
+};
+
// Then use:
-<h2 className="text-xl md:text-2xl font-bold">
- {standard}th standard
-</h2>
+<h2 className="text-xl md:text-2xl font-bold">
+ {getOrdinalSuffix(Number(standard))} standard
+</h2>
-<p className="text-gray-400 text-[11px] font-medium">
- {batch.standard}th Class
-</p>
+<p className="text-gray-400 text-[11px] font-medium">
+ {getOrdinalSuffix(Number(batch.standard))} Class
+</p>Also applies to: 204-206
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/batch-list.tsx
around lines 133 - 135, Replace the hardcoded "th" suffix by adding an ordinal
helper (e.g., getOrdinal(number): string) that returns the correct suffix for a
given integer (1 -> "st", 2 -> "nd", 3 -> "rd", 4+ -> "th", handling teens like
11-13 as "th"), then use it where the UI currently renders `{standard}th
standard` and `{batch.standard}th Class` (replace with string interpolation
using the number plus getOrdinal(standard) or getOrdinal(batch.standard));
update all occurrences in batch-list.tsx (including the h2 and the other two
places noted) to call this helper so ordinals display correctly.
| amount: z | ||
| .string() | ||
| .min(0, "Amount must be greater than or equal to 0") | ||
| .optional(), |
There was a problem hiding this comment.
Critical: .min(0) on a string schema validates string length, not numeric value.
The validation z.string().min(0, "Amount must be greater than or equal to 0") checks that the string has at least 0 characters (always passes), not that the numeric value is ≥ 0. The error message is misleading.
Since the amount is stored as a string in the form but needs numeric validation, consider using z.coerce.number() or validate the numeric value in superRefine.
🐛 Proposed fix
payment: z.object({
subscriptionType: z.enum(["Free", "Paid"]),
- amount: z
- .string()
- .min(0, "Amount must be greater than or equal to 0")
- .optional(),
+ amount: z.string().optional(),
currency: z.literal("INR"),
}),The numeric validation is already handled correctly in superRefine (lines 56-65), so the misleading .min(0) can simply be removed.
📝 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.
| amount: z | |
| .string() | |
| .min(0, "Amount must be greater than or equal to 0") | |
| .optional(), | |
| amount: z.string().optional(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/`(protected)/(root)/institute/[instituteId]/batches/_components/create-batch.tsx
around lines 49 - 52, The zod schema for the amount field currently uses
z.string().min(0, ...) which validates string length, not numeric value; remove
the .min(0) from the amount schema in create-batch.tsx (the amount property
inside the form schema) and either switch to z.coerce.number() if you want the
schema to coerce/validate numeric values, or keep it as z.string() and rely on
the existing superRefine numeric checks (the superRefine block that validates
amount value) to produce the correct "Amount must be greater than or equal to 0"
error.
| @custom-variant dark (&:is(.dark *)); | ||
|
|
||
| --chart-3: 197 37% 24%; | ||
|
|
||
| --chart-4: 43 74% 66%; | ||
|
|
||
| --chart-5: 27 87% 67%; | ||
| @utility container { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Candidate stylelint/package config files =="
fd -HI '(^\.stylelintrc(\..+)?$|^stylelint\.config\.(js|cjs|mjs|ts)$|^package\.json$)' -t f
echo
echo "== Stylelint rules relevant to Tailwind v4 directives =="
rg -n --no-heading -e 'scss/at-rule-no-unknown|at-rule-no-unknown|ignoreAtRules|customSyntax|postcss|tailwind' \
$(fd -HI '(^\.stylelintrc(\..+)?$|^stylelint\.config\.(js|cjs|mjs|ts)$|^package\.json$)' -t f)
echo
echo "== Tailwind v4 at-rules present in CSS files =="
rg -n --type=css '@custom-variant|@utility|@theme'Repository: Leadlly/leadlly.admin.web
Length of output: 793
🏁 Script executed:
cat -n .stylelintrc.jsonRepository: Leadlly/leadlly.admin.web
Length of output: 1285
Stylelint configuration does not support Tailwind v4 at-rules.
The .stylelintrc.json extends stylelint-config-standard-scss without configuring Tailwind v4 directives. The at-rule-no-unknown rule inherited from this config will report errors on valid @custom-variant, @utility, and @theme at-rules, causing CI failures or persistent false positives. Add an ignoreAtRules config or use a Tailwind-aware syntax preset to resolve.
Also applies to: 34-35, 61-63, 192-192
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 3-3: Unexpected unknown at-rule "@custom-variant" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 5-5: Unexpected unknown at-rule "@Utility" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/globals.css` around lines 3 - 5, Stylelint is flagging valid Tailwind
v4 at-rules (e.g. `@custom-variant`, `@utility`, `@theme`) because the project extends
stylelint-config-standard-scss without allowing Tailwind directives; update the
Stylelint config (the .stylelintrc.json) to ignore these at-rules by adding them
to the at-rule-no-unknown ignoreAtRules list (include "custom-variant",
"utility", "theme", etc.) or switch to a Tailwind-aware syntax/preset so the
rules around `@custom-variant`, `@utility`, and `@theme` in globals.css are accepted
and CI false-positives stop.
| <Field data-invalid={fieldState.invalid} className="gap-1.5"> | ||
| <FieldLabel htmlFor="name">Name</FieldLabel> | ||
| <Input | ||
| {...field} | ||
| placeholder="Enter institute name" | ||
| required | ||
| aria-invalid={fieldState.invalid} | ||
| className="shadow-none" | ||
| /> |
There was a problem hiding this comment.
Bind each FieldLabel to a real control ID.
These labels use htmlFor, but the matching Input/SelectTrigger never receives that id. That breaks click-to-focus behavior and removes the explicit label/control association for assistive tech across most of the form.
♿ Example fix
<FieldLabel htmlFor="name">Name</FieldLabel>
<Input
+ id="name"
{...field}
placeholder="Enter institute name"
required
aria-invalid={fieldState.invalid}
className="shadow-none"
/>Apply the same pattern to the other labeled Inputs and to the country SelectTrigger.
Also applies to: 218-227, 242-252, 265-274, 288-297, 309-318, 331-340, 355-364, 377-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/create-institute-form.tsx` around lines 197 - 205, The
FieldLabel components (e.g., FieldLabel htmlFor="name") are not bound to their
controls because the corresponding Input/SelectTrigger elements lack id props;
update each labeled control (Input and SelectTrigger instances referenced near
FieldLabel) to include an id matching the label's htmlFor (for example set
id="name" for the Input associated with FieldLabel htmlFor="name"), and apply
the same pattern to the other labeled pairs listed (lines referencing the
country SelectTrigger and the other FieldLabel/Input groups) so click-to-focus
and assistive-tech label associations work correctly.
| ref={ref} | ||
| className={cn( | ||
| "relative z-50 max-h-[--radix-select-content-available-height] min-w-[8rem] overflow-y-auto overflow-x-hidden rounded-md border bg-popover text-popover-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-select-content-transform-origin]", | ||
| "relative z-50 max-h-[--radix-select-content-available-height] min-w-32 overflow-y-auto overflow-x-hidden rounded-md border bg-popover text-popover-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-select-content-transform-origin]", |
There was a problem hiding this comment.
Inconsistent Tailwind v4 CSS variable syntax.
Line 79 uses the old bracket syntax (max-h-[--radix-select-content-available-height], origin-[--radix-select-content-transform-origin]), while line 92 correctly uses the new Tailwind v4 parentheses syntax (h-(--radix-select-trigger-height)). In Tailwind v4, CSS variable references must use parentheses instead of brackets.
🔧 Proposed fix to use consistent Tailwind v4 syntax
- "relative z-50 max-h-[--radix-select-content-available-height] min-w-32 overflow-y-auto overflow-x-hidden rounded-md border bg-popover text-popover-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-select-content-transform-origin]",
+ "relative z-50 max-h-(--radix-select-content-available-height) min-w-32 overflow-y-auto overflow-x-hidden rounded-md border bg-popover text-popover-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-select-content-transform-origin)",As per coding guidelines from Tailwind v4 documentation: "CSS Variables - Brackets → Parentheses: OLD: bg-[--brand-color], NEW: bg-(--brand-color)".
📝 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.
| "relative z-50 max-h-[--radix-select-content-available-height] min-w-32 overflow-y-auto overflow-x-hidden rounded-md border bg-popover text-popover-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-select-content-transform-origin]", | |
| "relative z-50 max-h-(--radix-select-content-available-height) min-w-32 overflow-y-auto overflow-x-hidden rounded-md border bg-popover text-popover-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-select-content-transform-origin)", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/select.tsx` at line 79, The Tailwind CSS variable usage in
the select component's class string uses old bracket syntax; update any
occurrences of max-h-[--radix-select-content-available-height] and
origin-[--radix-select-content-transform-origin] to the Tailwind v4 parentheses
form (e.g., max-h-(--radix-select-content-available-height) and
origin-(--radix-select-content-transform-origin)), and scan the surrounding
class list in the same Select component (the long "relative z-50 ..." class
string) to make all CSS variable references use parentheses to match the
h-(--radix-select-trigger-height) style.
…teacher listing, enhance fee management with UID grouping, and improve error handling in batch actions
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by BeetleThis PR introduces comprehensive teacher management features, enhances fee management with UID-based grouping and improved receipt generation, and adds batch assignment capabilities. The changes include a new teacher dashboard with analytics, multi-batch assignment dialogs, fee record grouping by unique identification numbers, and improved PDF receipt generation with dual-copy printing support. 📁 File Changes Summary (Consolidated across all commits):
Total Changes: 21 files changed, +3,438 additions, -672 deletions 🗺️ Walkthrough:graph TD
A[Teacher Management] --> B[Teacher Dashboard]
A --> C[Batch Assignment]
B --> B1[Overview Stats]
B --> B2[Monthly Trends Chart]
B --> B3[Subject Distribution]
B --> B4[Attendance Overview]
C --> C1[Multi-Select Batches]
C --> C2[Bulk Assignment]
D[Fee Management] --> E[UID Grouping]
D --> F[Receipt Generation]
E --> E1[Student Cards]
E --> E2[Drill-down to Records]
E --> E3[Aggregated Stats]
F --> F1[Dual-Copy PDF]
F --> F2[Additional Fees]
F --> F3[Discount Support]
F --> F4[Print & Download]
G[Batch Management] --> H[Batch Dashboard]
G --> I[Teacher Assignment]
H --> H1[Progress Tracking]
H --> H2[Student List]
H --> H3[Class Schedule]
I --> I1[Assign Teachers Dialog]
I --> I2[Multi-Teacher Selection]
J[Institute Settings] --> K[Edit Form]
K --> K1[Logo Upload]
K --> K2[Document Logo]
K --> K3[Address & Contact]
style A fill:#a855f7,stroke:#7c3aed,color:#fff
style D fill:#3b82f6,stroke:#2563eb,color:#fff
style G fill:#10b981,stroke:#059669,color:#fff
style J fill:#f59e0b,stroke:#d97706,color:#fff
🎯 Key Changes:
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
| "@react-oauth/google": "^0.12.1", | ||
| "@reduxjs/toolkit": "^2.7.0", | ||
| "@tanstack/react-query": "^5.90.21", | ||
| "apexcharts": "^5.10.4", |
There was a problem hiding this comment.
The apexcharts version ^5.10.4 does not exist in the npm registry. The latest stable version of ApexCharts is in the 3.x range (e.g., 3.45.2). This will cause npm install or npm ci to fail.
Confidence: 5/5
Suggested Fix
| "apexcharts": "^5.10.4", | |
| "apexcharts": "^3.45.2", | |
Change the version to a valid ApexCharts version. The react-apexcharts package (v2.1.0) you're adding on line 44 requires ApexCharts 3.x as a peer dependency. Use the latest stable 3.x version instead.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In package.json at line 31, the apexcharts version "^5.10.4" is invalid and does not exist in the npm registry; change it to "^3.45.2" or the latest stable 3.x version, as this is required by react-apexcharts v2.1.0 and will prevent installation failures.
|
|
||
| try { | ||
| const res = await fetch( | ||
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/batch/${batchId}`, |
There was a problem hiding this comment.
The batchId parameter is directly interpolated into the API URL without validation or sanitization. If an attacker can control the batchId value (e.g., through URL parameters or user input), they could potentially inject path traversal sequences (../) or manipulate the URL to access unauthorized endpoints or resources.
Confidence: 4/5
Suggested Fix
Add input validation to ensure batchId contains only expected characters (alphanumeric, hyphens, underscores). Consider validating at the function entry point:
export const getBatchDetails = async (batchId: string) => {
// Validate batchId format
if (!batchId || !/^[a-zA-Z0-9_-]+$/.test(batchId)) {
throw new Error("Invalid batch ID format");
}
const token = await getCookie();
// ... rest of the functionApply the same validation pattern to getBatchClasses and getBatchStudents functions. Alternatively, use URL encoding or a URL builder library that automatically sanitizes path parameters.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/batch_actions.ts at line 82 (and similarly at lines 107 and 132), the batchId parameter is directly interpolated into API URLs without validation or sanitization, which could allow path traversal or URL manipulation attacks; add input validation at the start of each function (getBatchDetails, getBatchClasses, getBatchStudents) to ensure batchId matches a safe pattern like /^[a-zA-Z0-9_-]+$/ and throw an error for invalid formats, preventing potential security exploits.
|
|
||
| try { | ||
| const res = await fetch( | ||
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/class/batch/${batchId}`, |
There was a problem hiding this comment.
Same issue as line 82 - the batchId parameter is directly interpolated into the API URL without validation or sanitization, creating a potential security vulnerability for path traversal or URL manipulation attacks.
Confidence: 4/5
Suggested Fix
Add the same input validation as suggested for getBatchDetails. Validate batchId at the function entry point to ensure it contains only safe characters.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/batch_actions.ts at line 107, the batchId parameter in getBatchClasses is directly interpolated into the API URL without validation; add input validation at the start of the function to ensure batchId matches a safe pattern like /^[a-zA-Z0-9_-]+$/ and throw an error for invalid formats, consistent with the fix applied to getBatchDetails.
|
|
||
| try { | ||
| const res = await fetch( | ||
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/batch/${batchId}/students`, |
There was a problem hiding this comment.
Same issue as lines 82 and 107 - the batchId parameter is directly interpolated into the API URL without validation or sanitization, creating a potential security vulnerability for path traversal or URL manipulation attacks.
Confidence: 4/5
Suggested Fix
Add the same input validation as suggested for the other two functions. Validate batchId at the function entry point to ensure it contains only safe characters.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/batch_actions.ts at line 132, the batchId parameter in getBatchStudents is directly interpolated into the API URL without validation; add input validation at the start of the function to ensure batchId matches a safe pattern like /^[a-zA-Z0-9_-]+$/ and throw an error for invalid formats, consistent with the fix applied to the other batch functions.
| ...(search ? { search } : {}), | ||
| }); | ||
| const res = await fetch( | ||
| `${getBase()}/api/institute/${instituteId}/fees/uid-groups?${params}`, |
There was a problem hiding this comment.
The instituteId parameter is directly interpolated into the API URL without validation or sanitization. If an attacker can control the instituteId value (e.g., through URL parameters or user input), they could potentially inject path traversal sequences (../) or manipulate the URL to access unauthorized endpoints or resources. This is the same vulnerability pattern identified in batch_actions.ts (see Comments #3, #4, #5).
Confidence: 4/5
Suggested Fix
Add input validation at the function entry point to ensure instituteId contains only expected characters (alphanumeric, hyphens, underscores):
| `${getBase()}/api/institute/${instituteId}/fees/uid-groups?${params}`, | |
| export async function getFeeUidGroups( | |
| instituteId: string, | |
| search = "" | |
| ): Promise<{ | |
| success: boolean; | |
| data?: FeeUidGroup[]; | |
| total?: number; | |
| message?: string; | |
| }> { | |
| // Validate instituteId format | |
| if (!instituteId || !/^[a-zA-Z0-9_-]+$/.test(instituteId)) { | |
| throw new Error("Invalid institute ID format"); | |
| } | |
| const token = await getCookie(); | |
Apply the same validation pattern to other functions in this file that use instituteId in URL construction (getFeeRecords, updateFeeRecord, etc.) to maintain consistency and prevent similar vulnerabilities.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/fee_actions.ts at line 149, the instituteId parameter in getFeeUidGroups is directly interpolated into the API URL without validation or sanitization, which could allow path traversal or URL manipulation attacks; add input validation at the start of the function to ensure instituteId matches a safe pattern like /^[a-zA-Z0-9_-]+$/ and throw an error for invalid formats, and apply the same validation to all other functions in this file that use instituteId in URL construction (getFeeRecords, updateFeeRecord, etc.) for consistency.
|
|
||
| try { | ||
| const res = await fetch( | ||
| `${process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL}/api/institute/${instituteId}`, |
There was a problem hiding this comment.
The instituteId parameter is directly interpolated into the API URL without validation or sanitization. This is the same vulnerability pattern identified in Comments #3, #4, #5, and #6 for other action files. If an attacker can control the instituteId value (e.g., through URL parameters or user input), they could potentially inject path traversal sequences (../) or manipulate the URL to access unauthorized endpoints or resources.
Confidence: 5/5
Suggested Fix
Add input validation at the function entry point (line 55-56) to ensure instituteId contains only expected characters (alphanumeric, hyphens, underscores):
export const updateInstitute = async (instituteId: string, data: InstituteUpdateData) => {
// Validate instituteId format
if (!instituteId || !/^[a-zA-Z0-9_-]+$/.test(instituteId)) {
throw new Error("Invalid institute ID format");
}
const token = await getCookie();This validation should be applied consistently across all action files that use ID parameters in URL construction to prevent path traversal and URL manipulation attacks.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/institute_actions.ts at line 60, the instituteId parameter in updateInstitute is directly interpolated into the API URL without validation or sanitization, which could allow path traversal or URL manipulation attacks; add input validation at the start of the function (after line 55) to ensure instituteId matches a safe pattern like /^[a-zA-Z0-9_-]+$/ and throw an error for invalid formats, consistent with the security fixes needed in batch_actions.ts and fee_actions.ts.
| } | ||
|
|
||
| const res = await fetch( | ||
| `${baseUrl}/api/admin/institute/${instituteId}/teachers`, |
There was a problem hiding this comment.
The instituteId parameter is directly interpolated into the API URL without validation or sanitization. This is the same vulnerability pattern identified in Comments #3-#7 for other action files. If an attacker can control the instituteId value (e.g., through URL parameters or user input), they could potentially inject path traversal sequences (../) or manipulate the URL to access unauthorized endpoints or resources.
Confidence: 5/5
Suggested Fix
Add input validation at the function entry point to ensure instituteId contains only expected characters (alphanumeric, hyphens, underscores):
| `${baseUrl}/api/admin/institute/${instituteId}/teachers`, | |
| export async function getInstituteTeachers( | |
| instituteId: string | |
| ): Promise<{ | |
| success: boolean; | |
| teachers: ApiTeacher[]; | |
| message?: string; | |
| }> { | |
| try { | |
| if (!instituteId) { | |
| return { success: false, teachers: [], message: "Institute ID is required" }; | |
| } | |
| // Validate instituteId format | |
| if (!/^[a-zA-Z0-9_-]+$/.test(instituteId)) { | |
| return { success: false, teachers: [], message: "Invalid institute ID format" }; | |
| } | |
| const token = await getCookie(); | |
| const baseUrl = process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL; | |
Apply the same validation pattern to all functions in this file that use ID parameters in URL construction (assignBatchToTeacher, assignBatchesToTeacher, getBatchTeachers, assignTeachersToBatch, getTeacherDashboardById) to maintain consistency and prevent similar vulnerabilities.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/teacher_actions.ts at line 47, the instituteId parameter in getInstituteTeachers is directly interpolated into the API URL without validation or sanitization, which could allow path traversal or URL manipulation attacks; add input validation after the empty check (after line 30) to ensure instituteId matches a safe pattern like /^[a-zA-Z0-9_-]+$/ and return an error for invalid formats, and apply the same validation to all other functions in this file that use ID parameters in URL construction (instituteId, teacherId, batchId) for consistency with the security fixes needed across all action files.
| const baseUrl = process.env.NEXT_PUBLIC_ADMIN_API_BASE_URL; | ||
| if (!baseUrl) return { success: false, teachers: [], message: "API base URL not configured" }; | ||
|
|
||
| const res = await fetch(`${baseUrl}/api/admin/batch/${batchId}/teachers`, { |
There was a problem hiding this comment.
The batchId parameter is directly interpolated into the API URL without validation or sanitization. Same issue as line 47 and Comments #3-#5 in batch_actions.ts. This creates a potential security vulnerability for path traversal or URL manipulation attacks.
Confidence: 5/5
Suggested Fix
Add input validation at the function entry point (after line 180) to ensure batchId contains only safe characters:
| const res = await fetch(`${baseUrl}/api/admin/batch/${batchId}/teachers`, { | |
| try { | |
| if (!batchId) return { success: false, teachers: [], message: "Batch ID is required" }; | |
| // Validate batchId format | |
| if (!/^[a-zA-Z0-9_-]+$/.test(batchId)) { | |
| return { success: false, teachers: [], message: "Invalid batch ID format" }; | |
| } | |
| const token = await getCookie(); | |
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/teacher_actions.ts at line 186, the batchId parameter in getBatchTeachers is directly interpolated into the API URL without validation; add input validation after the empty check (after line 180) to ensure batchId matches a safe pattern like /^[a-zA-Z0-9_-]+$/ and return an error for invalid formats, consistent with the fix applied to other ID parameters.
| return { success: false, message: "API base URL not configured" }; | ||
| } | ||
|
|
||
| const res = await fetch(`${baseUrl}/api/admin/teacher/${teacherId}/dashboard`, { |
There was a problem hiding this comment.
The teacherId parameter is directly interpolated into the API URL without validation or sanitization. Same vulnerability pattern as lines 47 and 186. This creates a potential security vulnerability for path traversal or URL manipulation attacks.
Confidence: 5/5
Suggested Fix
Add input validation at the function entry point (after line 232) to ensure teacherId contains only safe characters:
| const res = await fetch(`${baseUrl}/api/admin/teacher/${teacherId}/dashboard`, { | |
| try { | |
| if (!teacherId) { | |
| return { success: false, message: "Teacher ID is required" }; | |
| } | |
| // Validate teacherId format | |
| if (!/^[a-zA-Z0-9_-]+$/.test(teacherId)) { | |
| return { success: false, message: "Invalid teacher ID format" }; | |
| } | |
| const token = await getCookie(); | |
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/actions/teacher_actions.ts at line 243, the teacherId parameter in getTeacherDashboardById is directly interpolated into the API URL without validation; add input validation after the empty check (after line 232) to ensure teacherId matches a safe pattern like /^[a-zA-Z0-9_-]+$/ and return an error for invalid formats, consistent with the security fixes needed for all ID parameters across the action files.
Summary by BeetleThis PR addresses build-related issues and improves code quality across the application. The changes focus on fixing TypeScript type safety issues, updating API revalidation calls, correcting UI component variants, and replacing mock data with real API-driven data in the dashboard. The PR ensures better type inference, proper cache invalidation, and accurate data representation in the institute dashboard. 📁 File Changes Summary (Consolidated across all commits):
Total Changes: 8 files changed, +71 additions, -54 deletions 🗺️ Walkthrough:graph TD
A["Build Fix PR"] --> B["Type Safety Improvements"]
A --> C["API Integration"]
A --> D["UI Component Fixes"]
B --> B1["Form Type Inference (z.input vs z.infer)"]
B --> B2["Interface Updates (logo property typing)"]
B --> B3["Null Coalescing (subjects/standards arrays)"]
C --> C1["Replace Mock Data"]
C --> C2["Fetch Real API Data (students, teachers, batches)"]
C --> C3["Dynamic Calculations (courses, levels, departments)"]
C --> C4["Cache Revalidation (add 'max' parameter)"]
D --> D1["Button Variants (outline-solid → outline)"]
D --> D2["PDF Color Format (RGB parameters)"]
D --> D3["Combobox Icon Sizing"]
style A fill:#e1f5ff,stroke:#0066cc,stroke-width:2px
style C fill:#fff4e6,stroke:#ff9800,stroke-width:2px
style B fill:#f3e5f5,stroke:#9c27b0,stroke-width:2px
style D fill:#e8f5e9,stroke:#4caf50,stroke-width:2px
🎯 Key Changes:
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
| async function loadImageAsDataUrl(url: string): Promise<string | null> { | ||
| try { | ||
| const res = await fetch(url); | ||
| const blob = await res.blob(); | ||
| return new Promise((resolve) => { | ||
| const reader = new FileReader(); | ||
| reader.onloadend = () => resolve(reader.result as string); | ||
| reader.onerror = () => resolve(null); | ||
| reader.readAsDataURL(blob); | ||
| }); | ||
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The loadImageAsDataUrl function fetches external URLs without validation or sanitization. If meta.docLogoUrl originates from user input or an untrusted database source, this creates a Server-Side Request Forgery (SSRF) vulnerability. An attacker could:
- Access internal network resources (e.g.,
http://localhost:8080/admin) - Scan internal ports and services
- Exfiltrate data to external servers
- Bypass firewall restrictions
Confidence: 5/5
Suggested Fix
| async function loadImageAsDataUrl(url: string): Promise<string | null> { | |
| try { | |
| const res = await fetch(url); | |
| const blob = await res.blob(); | |
| return new Promise((resolve) => { | |
| const reader = new FileReader(); | |
| reader.onloadend = () => resolve(reader.result as string); | |
| reader.onerror = () => resolve(null); | |
| reader.readAsDataURL(blob); | |
| }); | |
| } catch { | |
| return null; | |
| } | |
| async function loadImageAsDataUrl(url: string): Promise<string | null> { | |
| try { | |
| // Validate URL format and protocol | |
| const parsedUrl = new URL(url); | |
| // Only allow HTTPS protocol for external resources | |
| if (parsedUrl.protocol !== 'https:') { | |
| console.error('Only HTTPS URLs are allowed for logo images'); | |
| return null; | |
| } | |
| // Prevent access to private IP ranges and localhost | |
| const hostname = parsedUrl.hostname.toLowerCase(); | |
| const privateIpPatterns = [ | |
| /^localhost$/i, | |
| /^127\./, | |
| /^10\./, | |
| /^172\.(1[6-9]|2[0-9]|3[0-1])\./, | |
| /^192\.168\./, | |
| /^169\.254\./, | |
| /^\[?::1\]?$/, | |
| /^\[?fe80:/i | |
| ]; | |
| if (privateIpPatterns.some(pattern => pattern.test(hostname))) { | |
| console.error('Access to private IP ranges is not allowed'); | |
| return null; | |
| } | |
| const res = await fetch(url, { | |
| // Add timeout to prevent hanging requests | |
| signal: AbortSignal.timeout(5000) | |
| }); | |
| // Validate response content type | |
| const contentType = res.headers.get('content-type'); | |
| if (!contentType || !contentType.startsWith('image/')) { | |
| console.error('Invalid content type for logo image'); | |
| return null; | |
| } | |
| const blob = await res.blob(); | |
| // Validate blob size (e.g., max 5MB) | |
| if (blob.size > 5 * 1024 * 1024) { | |
| console.error('Logo image size exceeds maximum allowed size'); | |
| return null; | |
| } | |
| return new Promise((resolve) => { | |
| const reader = new FileReader(); | |
| reader.onloadend = () => resolve(reader.result as string); | |
| reader.onerror = () => resolve(null); | |
| reader.readAsDataURL(blob); | |
| }); | |
| } catch { | |
| return null; | |
| } | |
Add comprehensive URL validation including:
- Protocol whitelist (HTTPS only)
- Private IP range blocking to prevent SSRF
- Content-Type validation
- File size limits
- Request timeout to prevent resource exhaustion
Additionally, ensuremeta.docLogoUrlis validated and sanitized at the data source (database/API) before reaching this function.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/(root)/institute/[instituteId]/fees/_components/FeePdfGenerator.ts at lines 43-55, the loadImageAsDataUrl function fetches external URLs without validation, creating a critical SSRF vulnerability; add comprehensive URL validation including: (1) parse the URL and validate it's HTTPS protocol only, (2) block private IP ranges (localhost, 127.x, 10.x, 172.16-31.x, 192.168.x, 169.254.x, ::1, fe80:) to prevent internal network access, (3) validate response Content-Type is an image, (4) enforce maximum file size limit (e.g., 5MB), (5) add request timeout (e.g., 5 seconds) using AbortSignal.timeout, and ensure meta.docLogoUrl is validated at the data source before reaching this function.
📍 This suggestion applies to lines 43-55
| phone: inst.contactNumber, | ||
| website: inst.website, | ||
| email: inst.email, | ||
| docLogoUrl: inst.docLogo?.url || inst.logo?.url || "", |
There was a problem hiding this comment.
This line introduces untrusted URL data (inst.docLogo?.url or inst.logo?.url) that feeds directly into the SSRF vulnerability identified in Comment #11 (FeePdfGenerator.ts). If an attacker can control the institute's logo URLs in the database, they can exploit the unvalidated loadImageAsDataUrl function to:
- Access internal network resources (localhost, private IPs)
- Scan internal ports and services
- Exfiltrate sensitive data
- Bypass firewall restrictions
The empty string fallback doesn't mitigate the risk since the primary values come from potentially untrusted database sources.
Confidence: 5/5
Suggested Fix
| docLogoUrl: inst.docLogo?.url || inst.logo?.url || "", | |
| docLogoUrl: validateLogoUrl(inst.docLogo?.url || inst.logo?.url || ""), | |
Add URL validation at the data source before passing to the PDF generator. Create a helper function:
function validateLogoUrl(url: string): string {
if (!url) return "";
try {
const parsed = new URL(url);
// Only allow HTTPS
if (parsed.protocol !== 'https:') return "";
// Block private IP ranges
const hostname = parsed.hostname.toLowerCase();
const privatePatterns = [
/^localhost$/i, /^127\./, /^10\./,
/^172\.(1[6-9]|2[0-9]|3[0-1])\./,
/^192\.168\./, /^169\.254\./,
/^\[?::1\]?$/, /^\[?fe80:/i
];
if (privatePatterns.some(p => p.test(hostname))) return "";
return url;
} catch {
return "";
}This provides defense-in-depth by validating URLs at the entry point, complementing the validation needed in FeePdfGenerator.ts (Comment #11).
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/(root)/institute/[instituteId]/students/[studentId]/page.tsx at line 87, the docLogoUrl property passes untrusted URLs from inst.docLogo?.url or inst.logo?.url directly to the PDF generator, creating an entry point for the SSRF vulnerability in FeePdfGenerator.ts; add URL validation by creating a validateLogoUrl helper function that: (1) validates HTTPS protocol only, (2) blocks private IP ranges (localhost, 127.x, 10.x, 172.16-31.x, 192.168.x, 169.254.x, ::1, fe80:), (3) returns empty string for invalid URLs, and wrap the URL value with this validation function before assigning to docLogoUrl, providing defense-in-depth alongside the fixes needed in FeePdfGenerator.ts.
| const [studentsRes, teachersRes, batchesRes] = await Promise.all([ | ||
| getInstituteStudents(instituteId), | ||
| getInstituteTeachers(instituteId), | ||
| getInstituteBatch(instituteId).catch(() => ({ success: false, data: [] })), | ||
| ]); |
There was a problem hiding this comment.
The instituteId from route params is used directly in data fetching functions without validation or authorization checks. This could allow unauthorized access to institute data if an attacker manipulates the URL parameter.
Confidence: 5/5
Security Concern Details
The code fetches sensitive institute data (students, teachers, batches) using only the instituteId from the URL route parameter. There's no visible authorization check to verify:
- Whether the current user has permission to access this specific institute's data
- Whether the
instituteIdis valid or belongs to the user's authorized institutes - Whether the user's role permits viewing this dashboard
Attack Vector: An attacker could enumerate institute IDs in the URL (e.g.,/institute/1,/institute/2, etc.) to access data from institutes they shouldn't have access to.
Required Fix: Add authorization checks before data fetching, either:
- In this component to verify user permissions for the institute
- Ensure the data fetching functions (
getInstituteStudents,getInstituteTeachers,getInstituteBatch) perform proper authorization checks server-side - Verify that
getActiveInstitutealready validates user access and handle unauthorized cases
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/app/(protected)/(root)/institute/[instituteId]/page.tsx around line 44, the instituteId from route params is used directly to fetch sensitive data (students, teachers, batches) without visible authorization checks. This could allow unauthorized access if an attacker manipulates the URL parameter. Add authorization validation before the Promise.all data fetching to verify the current user has permission to access this specific institute's data, or ensure the data fetching functions themselves perform proper server-side authorization checks and handle unauthorized access appropriately with error boundaries.
📍 This suggestion applies to lines 44-48
2 participants
Summary by CodeRabbit
New Features
Improvements