medtech project is in progress#10
Conversation
📝 WalkthroughWalkthroughThis change expands the application into a telemedicine platform with authenticated doctor and patient workflows, appointments, prescriptions, payments, notifications, administration, real-time chat and calling, refreshed API handling, and new role-based frontend workspaces. ChangesTelemedicine platform
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Patient
participant DoctorProfileView
participant API
participant Database
Patient->>DoctorProfileView: select doctor, date, and time slot
DoctorProfileView->>API: book appointment
API->>Database: create appointment and update time slot
API-->>DoctorProfileView: return appointment
DoctorProfileView->>API: create payment order
API->>Database: save payment and mark appointment paid
API-->>DoctorProfileView: return payment confirmation
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
arpit168
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
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 (1)
backend/src/controllers/auth.controller.js (1)
42-52: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winNo role validation on registration allows privilege escalation.
roleis taken directly fromreq.bodyand passed toUser.createwithout any validation. The route's express-validator checks only coverpassword, andname. A user can POST{"role": "admin"}to gain admin access.🛡️ Proposed fix — add role validation in the route
router.post('/register', [ body('email').isEmail().normalizeEmail(), body('password').isLength({ min: 6 }), - body('name').notEmpty().trim() + body('name').notEmpty().trim(), + body('role').isIn(['patient', 'doctor']).withMessage('Role must be patient or doctor') ], register);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/auth.controller.js` around lines 42 - 52, Validate or ignore the client-provided role in the registration route before calling the controller’s user-creation logic; new users must receive a fixed non-privileged default such as “user,” and roles must not be accepted directly from req.body. Update the route validation and the registration handler around User.create to enforce this consistently.
🟠 Major comments (34)
backend/src/controllers/digitalPrescription.controller.js-96-112 (1)
96-112: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
getDigitalPrescriptionByIdhas no authorization check — any authenticated user can view any prescription.The endpoint fetches a prescription by ID without verifying the requesting user is the associated doctor or patient. This exposes medical data (diagnoses, medicines) to unauthorized access — a privacy and compliance risk.
🛡️ Proposed fix to add authorization check
const prescription = await DigitalPrescription.findById(id) .populate('doctorId', 'name email phone avatarUrl') .populate('patientId', 'name email phone avatarUrl'); if (!prescription) { return res.status(404).json({ success: false, message: 'Prescription not found' }); } + // Verify the requesting user has access to this prescription + const userId = req.user._id.toString(); + const isDoctor = prescription.doctorId?._id?.toString() === userId; + const isPatient = prescription.patientId?._id?.toString() === userId; + const isAdmin = req.user.role === 'admin'; + if (!isDoctor && !isPatient && !isAdmin) { + return res.status(403).json({ success: false, message: 'Not authorized to view this prescription' }); + } + res.json({ success: true, prescription });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/digitalPrescription.controller.js` around lines 96 - 112, add an authorization check in getDigitalPrescriptionById after loading the prescription and before returning it: compare the authenticated user’s identity and role with the prescription’s doctorId or patientId, and return 403 for users who are neither associated party. Preserve the existing 404 response for missing prescriptions and ensure populated IDs are compared safely.backend/src/middleware/audit.middleware.js-6-21 (1)
6-21: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAudit log stores raw
req.body, risking sensitive data exposure.
body: req.bodyis persisted verbatim into theAuditLog.detailsfield. On auth routes this could include plaintext passwords; on healthcare routes it captures medical PII (diagnoses, prescriptions). This creates a secondary sensitive-data store outside normal access controls and may violate GDPR/HIPAA compliance.Sanitize or redact sensitive fields before persisting, or avoid logging
bodyentirely.🛡️ Proposed fix to redact sensitive fields
await AuditLog.create({ userId: req.user?._id || req.user?.id || null, action: actionDescription || `${req.method} ${req.originalUrl}`, details: { - body: req.body, + body: redactSensitive(req.body), query: req.query, params: req.params, statusCode: res.statusCode }, ipAddress: req.ip || req.headers['x-forwarded-for'] || '', status: 'success' });Add a helper to strip known sensitive keys:
const SENSITIVE_KEYS = ['password', 'confirmPassword', 'token', 'secret', 'creditCard']; function redactSensitive(obj) { if (!obj || typeof obj !== 'object') return obj; const cleaned = { ...obj }; for (const key of Object.keys(cleaned)) { if (SENSITIVE_KEYS.some(k => key.toLowerCase().includes(k))) { cleaned[key] = '[REDACTED]'; } } return cleaned; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/middleware/audit.middleware.js` around lines 6 - 21, The audit middleware persists raw request bodies, exposing sensitive credentials and PII. Add a redaction helper near the middleware (such as redactSensitive with a case-insensitive sensitive-key list), then pass the sanitized result to AuditLog.create in the res.on('finish') handler instead of req.body; consider recursively sanitizing nested objects.backend/src/controllers/digitalPrescription.controller.js-37-39 (1)
37-39: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAppointment marked completed without ownership verification.
Appointment.findByIdAndUpdate(appointmentId, { status: 'completed' })updates any appointment by ID. A doctor could mark another doctor's appointment as completed. Verify the appointment belongs to the requesting doctor before updating.🛡️ Proposed fix to verify appointment ownership
if (appointmentId) { - await Appointment.findByIdAndUpdate(appointmentId, { status: 'completed' }); + const appointment = await Appointment.findById(appointmentId); + if (!appointment) { + return res.status(404).json({ success: false, message: 'Appointment not found' }); + } + if (appointment.doctorId.toString() !== doctorId.toString()) { + return res.status(403).json({ success: false, message: 'Not authorized to update this appointment' }); + } + appointment.status = 'completed'; + await appointment.save(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/digitalPrescription.controller.js` around lines 37 - 39, In the appointment completion logic, replace the unrestricted Appointment.findByIdAndUpdate call with an update scoped by both appointmentId and the authenticated requesting doctor’s identifier; use the controller’s existing request-user/authentication fields, and handle a missing match as an unauthorized or not-found response so another doctor’s appointment cannot be marked completed.backend/src/routes/appointment.routes.js-13-17 (1)
13-17: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAdd authz to appointment mutations
PUT /:id/rescheduleruns afterprotectwith no ownership/role check, so any authenticated user can reschedule an appointment by ID.PUT /:id/statusonly has an ownership/admin check in the controller; addauthorize(...)here if status changes should be doctor-only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/routes/appointment.routes.js` around lines 13 - 17, Appointment mutation routes lack explicit authorization, allowing insufficiently privileged users to modify appointments. Update the PUT routes in the appointment router to apply the existing authorize middleware: require ownership or appropriate admin access for rescheduleAppointment, and require the doctor role for updateAppointmentStatus if status changes are doctor-only, keeping protect in the middleware chain.backend/src/routes/digitalPrescription.routes.js-12-13 (1)
12-13: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winScope
GET /:idby owner/role
getDigitalPrescriptionsalready filters byreq.user.roleand ownership, butgetDigitalPrescriptionByIddoes an unscopedfindById(). Any authenticated user can fetch another prescription by ID unless the controller checkspatientId/doctorIdagainstreq.user._idor adds a role-based guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/routes/digitalPrescription.routes.js` around lines 12 - 13, The getDigitalPrescriptionById handler currently performs an unscoped lookup, allowing authenticated users to access prescriptions they do not own. Update getDigitalPrescriptionById to enforce role-based ownership using req.user._id, matching patients through patientId and doctors through doctorId, and return the appropriate not-found/forbidden response when the prescription is not accessible.backend/src/controllers/patient.controller.js-78-105 (1)
78-105: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAny doctor can access any patient's full medical history without a care relationship check.
Lines 81-83 allow any user with role
doctorto pass an arbitrary?patientId=and retrieve that patient's complete medical history — symptoms, reports, digital prescriptions, and OCR prescriptions. There is no verification that the doctor has an appointment or care relationship with the target patient. In a healthcare context, this is a significant privacy violation (HIPAA-like).Additionally,
req.query.patientIdis not validated as a valid ObjectId — invalid values will cause a Mongoose CastError and return a 500 instead of a 400.🔒 Proposed fix: validate ObjectId and verify care relationship
import PatientProfile from '../models/PatientProfile.model.js'; import User from '../models/User.model.js'; import SymptomTimeline from '../models/SymptomTimeline.model.js'; import Report from '../models/Report.model.js'; import DigitalPrescription from '../models/DigitalPrescription.model.js'; import Prescription from '../models/Prescription.model.js'; +import mongoose from 'mongoose'; +import Appointment from '../models/Appointment.model.js'; // ... existing code ... export const getPatientHistory = async (req, res) => { try { + let targetUserId; + // If doctor/admin is requesting another patient's history, check query/param id - const targetUserId = (req.user.role === 'doctor' || req.user.role === 'admin') && req.query.patientId - ? req.query.patientId - : req.user._id; + if ((req.user.role === 'doctor' || req.user.role === 'admin') && req.query.patientId) { + if (!mongoose.isValidObjectId(req.query.patientId)) { + return res.status(400).json({ success: false, message: 'Invalid patient ID' }); + } + targetUserId = req.query.patientId; + + // Doctors must have a care relationship with the patient + if (req.user.role === 'doctor') { + const hasAppointment = await Appointment.exists({ + doctorId: req.user._id, + patientId: targetUserId + }); + if (!hasAppointment) { + return res.status(403).json({ success: false, message: 'No care relationship with this patient' }); + } + } + } else { + targetUserId = req.user._id; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/patient.controller.js` around lines 78 - 105, Restrict doctor access in getPatientHistory by validating req.query.patientId with a valid ObjectId check and returning 400 for invalid values, then verify the requesting doctor has an appointment or other established care relationship with the target patient before querying history; return 403 when unauthorized. Preserve admin access as appropriate, and ensure all history queries use only the validated, authorized targetUserId.backend/src/models/AuditLog.model.js-15-18 (1)
15-18: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAudit log
detailsstores raw request bodies — PHI and credential exposure risk.The audit middleware (
audit.middleware.js:11-17) persistsreq.body,req.query, andreq.paramsdirectly into this field. For a telemedicine platform, request bodies contain passwords (auth routes), medical symptoms, diagnoses, and PII. Storing these unencrypted in audit logs creates HIPAA/GDPR exposure. Add a sanitization layer to redact sensitive fields (password, email, phone, medical data) before persistence, or switch to a field whitelist.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/AuditLog.model.js` around lines 15 - 18, Sanitize audit payloads before assigning them to the `details` field in the audit middleware, rather than persisting raw `req.body`, `req.query`, and `req.params`. Add a reusable redaction or whitelist helper that recursively removes sensitive credentials, contact information, and medical/PII fields, then use it in the middleware before creating the audit record consumed by `AuditLog`; also avoid the mutable `{}` default in the model by using a default factory if supported.backend/src/models/DoctorProfile.model.js-40-44 (1)
40-44: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDefault
verificationStatusshould be'pending', not'verified'.The comment acknowledges this is for demo purposes, but defaulting to
'verified'means any registered doctor is automatically verified without credential or license review. For a healthcare platform, this is a critical security gap — unverified doctors could prescribe medication and treat patients. Default to'pending'and require explicit admin approval.🔒 Proposed fix
verificationStatus: { type: String, enum: ['pending', 'verified', 'rejected'], - default: 'verified' // Default verified for seamless demo & immediate testing + default: 'pending' },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/DoctorProfile.model.js` around lines 40 - 44, Change the default value of DoctorProfile’s verificationStatus field from 'verified' to 'pending' so newly registered doctors require explicit admin approval before verification.backend/src/models/Appointment.model.js-49-53 (1)
49-53: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDefault
paymentStatusshould be'pending', not'paid'.The model defaults to
'paid', and the controller inappointment.controller.js:29also hardcodespaymentStatus: 'paid'with a comment "Simulated instant checkout for smooth testing." This bypasses the payment flow entirely — appointments are marked paid without any payment processing. In production, this would allow free appointments and lost revenue. Default to'pending'and only transition to'paid'after successful payment confirmation.🔧 Proposed fix
paymentStatus: { type: String, enum: ['pending', 'paid', 'refunded', 'free'], - default: 'paid' + default: 'pending' },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/Appointment.model.js` around lines 49 - 53, Change the `paymentStatus` default in `Appointment` to `'pending'` and remove the controller’s hardcoded `paymentStatus: 'paid'` assignment in `appointment.controller.js`. Ensure appointments transition to `'paid'` only through successful payment confirmation, while preserving `'free'` handling for genuinely free appointments.backend/src/models/Appointment.model.js-58-61 (1)
58-61: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReplace
Math.random()withcrypto.randomUUID()for meeting room IDs.
Math.random()is not cryptographically secure. Predictable room IDs could allow unauthorized users to join telemedicine video sessions. Use Node's built-incrypto.randomUUID()for unpredictable, unique room identifiers.🔒 Proposed fix
+import crypto from 'crypto'; + const appointmentSchema = new mongoose.Schema({ // ... meetingRoomId: { type: String, - default: () => `room_${Math.random().toString(36).substring(2, 10)}` + default: () => `room_${crypto.randomUUID()}` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/Appointment.model.js` around lines 58 - 61, Replace the Math.random()-based default in the meetingRoomId schema field with Node’s crypto.randomUUID(), adding the required crypto import and preserving the room_ prefix if needed.backend/src/models/DoctorProfile.model.js-65-68 (1)
65-68: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the manual
updatedAtfield.timestamps: truealready adds and updatesupdatedAt, so the explicitdefault: Date.nowpath is redundant here.backend/src/models/DoctorProfile.model.js:65-70🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/DoctorProfile.model.js` around lines 65 - 68, Remove the explicit updatedAt field with its Date.now default from the DoctorProfile schema, relying on the schema’s existing timestamps: true configuration to manage it automatically.backend/src/controllers/doctor.controller.js-58-61 (1)
58-61: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winInternal error messages leaked to clients in all catch blocks.
Every error handler returns
error.messagedirectly in the JSON response. This can expose database error details, validation internals, or stack information. Return a generic message to clients and log the full error server-side.🔒 Proposed fix (apply to all 4 catch blocks)
} catch (error) { console.error('Get All Doctors Error:', error); - res.status(500).json({ success: false, message: error.message }); + res.status(500).json({ success: false, message: 'Internal server error' }); }Also applies to: 104-107, 143-146, 170-173
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/doctor.controller.js` around lines 58 - 61, Replace error.message in all four catch blocks of the doctor controller with a generic client-safe message such as “Internal server error,” while retaining console.error logging of the full error server-side. Update the handlers around the existing error responses at each referenced catch block consistently.backend/src/controllers/doctor.controller.js-37-55 (1)
37-55: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPublic routes expose doctor PII and patient data to unauthenticated users.
Per the route configuration in
backend/src/routes/doctor.routes.js,GET /andGET /:idhave noprotectmiddleware. The responses include:
- Doctor PII:
phone,licenseNumber,licenseDocumentUrl- Patient PII:
reviewspopulated with patientnameandavatarUrlFor a healthcare platform, this is a privacy concern. Either require authentication on these routes or strip sensitive fields (email, phone, license details, patient identities) from public responses.
Also applies to: 78-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/doctor.controller.js` around lines 37 - 55, Protect the public doctor listing and detail routes by adding the existing authentication middleware to GET / and GET /:id in the doctor route configuration, or alternatively sanitize their controller responses. In the controller methods producing the mapped doctor data, remove email, phone, licenseNumber, licenseDocumentUrl, and patient-identifying review fields such as name and avatarUrl from unauthenticated responses.backend/src/models/User.model.js-44-45 (1)
44-45: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winHash
resetPasswordTokenbefore storage — plain-text tokens enable account takeover on database leak.Password reset tokens are temporary credentials. Storing them in plain text means a database compromise exposes active tokens, allowing attackers to reset passwords and take over accounts. Hash the token (e.g., SHA-256) before saving, and compare the hash during verification.
🔒 Proposed fix using crypto
+import crypto from 'crypto'; + +// When generating a reset token: +// const token = crypto.randomBytes(32).toString('hex'); +// const hashedToken = crypto.createHash('sha256').update(token).digest('hex'); +// user.resetPasswordToken = hashedToken; +// user.resetPasswordExpire = Date.now() + 10 * 60 * 1000; // 10 min +// Send 'token' (plain) to user via email, store only hashedToken + resetPasswordToken: String, resetPasswordExpire: Date,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/User.model.js` around lines 44 - 45, Hash reset password tokens before persisting them in the User model, using a consistent SHA-256 helper or pre-save transformation for resetPasswordToken. Update the password-reset token verification flow to hash the presented token before comparing it with the stored value, while preserving resetPasswordExpire handling and existing token generation behavior.backend/src/controllers/doctor.controller.js-159-166 (1)
159-166: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate
licenseDocumentUrland return error when no license is provided.Two issues:
req.body.licenseDocumentUrlis assigned directly without URL validation — allowsjavascript:,data:, or arbitrary external URLs (XSS/SSRF risk if rendered or fetched server-side).- If neither
req.filenorreq.body.licenseDocumentUrlis provided, the function still returns"License uploaded successfully"— misleading and saves an empty profile.Additionally, the multer configuration in the route file has no
fileFilterorlimits— any file type or size can be uploaded.🔒 Proposed fix
if (req.file) { docProfile.licenseDocumentUrl = req.file.path; docProfile.verificationStatus = 'pending'; } else if (req.body.licenseDocumentUrl) { + const url = req.body.licenseDocumentUrl; + if (!/^\/uploads\/|^https:\/\/.+/.test(url)) { + return res.status(400).json({ success: false, message: 'Invalid license document URL' }); + } docProfile.licenseDocumentUrl = url; docProfile.verificationStatus = 'pending'; + } else { + return res.status(400).json({ success: false, message: 'No license file or URL provided' }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/doctor.controller.js` around lines 159 - 166, Validate req.body.licenseDocumentUrl before assigning it in the license update handler, allowing only safe HTTPS URLs (or the intended trusted storage-host format) and rejecting javascript:, data:, malformed, or untrusted URLs with a client error. Require either req.file or a valid licenseDocumentUrl; otherwise return an error instead of success and avoid saving an empty profile. Also update the multer configuration in the route definition to add an appropriate fileFilter and file-size limits, rejecting unsupported or oversized uploads.backend/src/models/Payment.model.js-39-44 (1)
39-44: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPayment
statusdefaults to'completed'— should default to'pending'.New payment records are created in a
completedstate by default, bypassing payment verification. For a healthcare platform handling real transactions, payments should start as'pending'and transition to'completed'only after gateway confirmation.🛡️ Proposed fix
status: { type: String, enum: ['pending', 'completed', 'failed', 'refunded'], - default: 'completed', + default: 'pending', index: true },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/Payment.model.js` around lines 39 - 44, Update the `status` field in the `Payment` model schema to use `'pending'` as its default value instead of `'completed'`, ensuring new payments remain pending until gateway confirmation.backend/src/models/TimeSlot.model.js-3-22 (1)
3-22: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd a compound unique index on
[doctorId, date]to prevent duplicate time slot documents.Without this constraint, multiple TimeSlot documents can be created for the same doctor on the same date, leading to ambiguous slot lookups and potential double-booking.
🛡️ Proposed fix
}, { timestamps: true }); +timeSlotSchema.index({ doctorId: 1, date: 1 }, { unique: true }); + export default mongoose.model('TimeSlot', timeSlotSchema);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/TimeSlot.model.js` around lines 3 - 22, Add a compound unique index on doctorId and date in the timeSlotSchema definition, ensuring each doctor can have only one TimeSlot document per date.backend/src/models/Review.model.js-3-33 (1)
3-33: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd a compound unique index on
[patientId, appointmentId]to prevent duplicate reviews.Without this constraint, a patient can submit multiple reviews for the same appointment, skewing doctor ratings and enabling review spam.
🛡️ Proposed fix
}, { timestamps: true }); +reviewSchema.index({ patientId: 1, appointmentId: 1 }, { unique: true }); + export default mongoose.model('Review', reviewSchema);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/Review.model.js` around lines 3 - 33, Add a compound unique index on patientId and appointmentId in the reviewSchema definition, ensuring each patient can create at most one review per appointment while preserving the existing schema fields and options.backend/src/controllers/admin.controller.js-105-109 (1)
105-109: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUnescaped user input in
$regexqueries enables ReDoS.
search(Line 107) andaction(Line 159) are passed directly to MongoDB$regex, allowing catastrophic backtracking patterns (e.g.,(a+)+$) to stall the server. Escape special regex characters or useRegExpwith escaped input.🛡️ Proposed fix for getAllUsers
- if (search) { - filter.$or = [ - { name: { $regex: search, $options: 'i' } }, - { email: { $regex: search, $options: 'i' } } - ]; - } + if (search) { + const escaped = search.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + filter.$or = [ + { name: { $regex: escaped, $options: 'i' } }, + { email: { $regex: escaped, $options: 'i' } } + ]; + }🛡️ Proposed fix for getAuditLogs
- if (action) filter.action = { $regex: action, $options: 'i' }; + if (action) { + const escaped = action.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + filter.action = { $regex: escaped, $options: 'i' }; + }Also applies to: 159-159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/admin.controller.js` around lines 105 - 109, Escape user-supplied regex metacharacters before constructing the `$regex` filters in `getAllUsers` for `search` and `getAuditLogs` for `action`; add or reuse a shared regex-escaping helper, then use the escaped values while preserving case-insensitive matching.backend/src/controllers/appointment.controller.js-126-141 (1)
126-141: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
status.toUpperCase()crashes whenstatusis undefined.If the request provides only
doctorNotesorrejectionReasonwithoutstatus, the notification at Line 136 callsstatus.toUpperCase()onundefined, causing aTypeErrorand a 500 response. The notification block should be guarded.🐛 Proposed fix
await appointment.save(); - // Notify the other party - const targetUserId = req.user.role === 'patient' ? appointment.doctorId._id : appointment.patientId._id; - await Notification.create({ - userId: targetUserId, - title: `Appointment ${status.toUpperCase()}`, - message: `Your appointment on ${new Date(appointment.appointmentDate).toLocaleDateString()} at ${appointment.timeSlot} has been marked as ${status}.`, - type: 'appointment', - link: req.user.role === 'patient' ? '/doctor-dashboard' : '/appointments', - relatedId: appointment._id - }); + if (status) { + const targetUserId = req.user.role === 'patient' ? appointment.doctorId._id : appointment.patientId._id; + await Notification.create({ + userId: targetUserId, + title: `Appointment ${status.toUpperCase()}`, + message: `Your appointment on ${new Date(appointment.appointmentDate).toLocaleDateString()} at ${appointment.timeSlot} has been marked as ${status}.`, + type: 'appointment', + link: req.user.role === 'patient' ? '/doctor-dashboard' : '/appointments', + relatedId: appointment._id + }); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/appointment.controller.js` around lines 126 - 141, Guard the notification creation block in the appointment update handler so it runs only when status is provided, preventing status.toUpperCase() from being called with undefined. Keep saving doctorNotes or rejectionReason independently, and wrap the existing targetUserId calculation and Notification.create call under the status check.backend/src/controllers/payment.controller.js-86-100 (1)
86-100: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNo double-refund prevention — already-refunded payments can be refunded again.
refundPaymentdoesn't checkpayment.statusbefore processing. Calling the endpoint twice on the same payment creates duplicate refund records, sends duplicate notifications, and writes duplicate audit logs.🛡️ Proposed fix
const payment = await Payment.findById(id); if (!payment) { return res.status(404).json({ success: false, message: 'Payment record not found' }); } + if (payment.status === 'refunded') { + return res.status(400).json({ success: false, message: 'Payment has already been refunded' }); + } + if (req.user.role !== 'admin' && req.user._id.toString() !== payment.doctorId.toString()) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/payment.controller.js` around lines 86 - 100, Prevent duplicate refunds in refundPayment by checking payment.status immediately after confirming the payment exists and before authorization or mutation; if it is already 'refunded', return an appropriate conflict response without updating refundDetails, saving, or triggering downstream notifications and audit logs. Preserve the existing authorization and successful refund flow for other statuses.backend/src/controllers/appointment.controller.js-19-42 (1)
19-42: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNo double-booking prevention — same slot can be booked by multiple patients.
The appointment is created before any availability check. The TimeSlot marking at Lines 34-41 only updates an existing slot document but doesn't reject the booking if the slot is already taken. Two concurrent requests (or even sequential ones) can book the same doctor/date/time.
🛡️ Proposed fix — check for existing booking before creating
if (!doctorId || !appointmentDate || !timeSlot || !reasonForVisit) { return res.status(400).json({ success: false, message: 'Please provide doctorId, appointmentDate, timeSlot, and reasonForVisit' }); } + const startOfDay = new Date(appointmentDate); + startOfDay.setHours(0, 0, 0, 0); + const endOfDay = new Date(appointmentDate); + endOfDay.setHours(23, 59, 59, 999); + const existing = await Appointment.findOne({ + doctorId, + appointmentDate: { $gte: startOfDay, $lte: endOfDay }, + timeSlot, + status: { $in: ['booked', 'approved'] } + }); + if (existing) { + return res.status(409).json({ success: false, message: 'This time slot is already booked' }); + } + // Create appointment const appointment = await Appointment.create({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/appointment.controller.js` around lines 19 - 42, Prevent double-booking in the appointment creation flow by atomically validating and reserving the requested slot before creating the Appointment. Update the logic surrounding Appointment.create and the TimeSlot lookup to reject requests when the slot is already marked isBooked, and ensure concurrent requests cannot reserve the same doctor/date/time; only create the appointment after the reservation succeeds, rolling back the slot reservation if creation fails.frontend/src/pages/ChatRoom.jsx-83-121 (1)
83-121: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStale
activeConvin the socket listenerThe
receive-messagehandler is registered once on mount, so it keeps the initialactiveConvvalue. Incoming messages for the open chat won’t be appended in real time; only the conversation list refreshes. Read the current conversation id from a ref or rebind the listener whenactiveConvchanges.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/ChatRoom.jsx` around lines 83 - 121, Fix the stale activeConv captured by the receive-message listener in the ChatRoom useEffect: track the current conversation id with a ref updated whenever activeConv changes, or rebind the socket listener when activeConv changes. Use that current id for the message comparison while preserving cleanup and conversation refresh behavior.backend/src/socket/socket.js-8-13 (1)
8-13: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove
*from the Socket.IO CORS origin list.
credentials: trueis incompatible with a wildcard origin, so browser clients using cookies or auth headers will be blocked. Keep the allow-list explicit or drive it from env config.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/socket/socket.js` around lines 8 - 13, Remove the wildcard "*" entry from the Socket.IO CORS origin array in the socket initialization configuration, keeping only explicit allowed origins or values supplied through environment configuration while preserving credentials support.backend/src/routes/chat.routes.js-15-21 (1)
15-21: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSanitize chat attachment filenames and cap uploads
file.originalnameis written straight into a publicuploads/path here, so a crafted multipart filename can escape the directory or overwrite files. This route also lackslimits.fileSizeandfileFilter, so it can accept arbitrary large files and types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/routes/chat.routes.js` around lines 15 - 21, Sanitize attachment names and constrain uploads in the multer configuration near the storage and upload definitions. Update the `filename` callback to generate a safe unique filename without trusting path components from `file.originalname`, and configure `limits.fileSize` plus a `fileFilter` that allows only the intended attachment MIME types/extensions and rejects others.frontend/src/components/Navbar.jsx-292-346 (1)
292-346: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
NotificationDropdownmounts twice (desktop + mobile), doubling fetch/socket subscriptions.Both the
.nb-desktopcontainer and thelg:hiddencontainer render<NotificationDropdown user={user} />unconditionally whenevertokenis set; only CSS visibility (display:none/ Tailwindlg:hidden) differs, so both instances stay mounted at every viewport width. Each instance independently callsfetchNotifications()on mount and subscribes its ownsocket.on('notification', ...)listener, so every notification event triggers two redundant fetches network-wide.Consider rendering a single
NotificationDropdowninstance and positioning it via CSS/flex order for both layouts, or lifting notification state up so both toggle buttons share one data source.Also applies to: 336-345
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Navbar.jsx` around lines 292 - 346, The NotificationDropdown component is mounted twice in Navbar: once in the desktop navigation and once in the mobile controls, causing duplicate fetches and socket subscriptions. Render a single NotificationDropdown instance within the token-gated fragment and use responsive CSS/layout positioning to display it appropriately for desktop and mobile, or lift its notification state into Navbar and share that state between layout controls.frontend/src/components/Navbar.jsx-196-201 (1)
196-201: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDebug styling left in
.nb-logout— gray background + red border.Line 197 sets
border: none;, then line 199 overrides withborder: 1px solid red;(last rule wins), combined withbackground: gray;. This looks like an accidental debug leftover rather than intended styling, and will visibly alter the logout button (red border, gray fill) across the entire app.🎨 Proposed fix
.nb-logout { - border: none; - background: gray; - border: 1px solid red; + border: 1px solid rgba(255,255,255,.08); + background: rgba(255,255,255,.05); width: 34px; height: 34px;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Navbar.jsx` around lines 196 - 201, Remove the accidental debug styling from the `.nb-logout` rule: delete the `background: gray` declaration and the later `border: 1px solid red` override, preserving the intended `border: none` styling for the logout button.frontend/src/App.jsx-71-143 (1)
71-143: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAdd role checks to these workspaces
ProtectedRouteonly checks for a token, so any authenticated patient can open/admin-dashboard,/doctor-dashboard,/doctor-earnings, or/prescription-generatordirectly. Hiding links inNavbaris not an authorization boundary; add a role-aware guard here and keep backend authorization enforced too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/App.jsx` around lines 71 - 143, Add role-based authorization to the workspace routes in App.jsx: configure ProtectedRoute or a dedicated role-aware guard to allow only doctors on /doctor-dashboard, /doctor-earnings, and /prescription-generator, and only admins on /admin-dashboard. Preserve authentication checks and ensure unauthorized users are redirected or denied, while retaining backend authorization.frontend/src/pages/DigitalPrescriptionGenerator.jsx-80-82 (1)
80-82: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly the first medicine's name is validated.
medicines[0].name.trim()is checked, but subsequent rows added via "Add Medication" can be submitted with a blanknameand no client-side rejection. For a clinical document this should validate every row.🐛 Suggested fix
- if (medicines.length === 0 || !medicines[0].name.trim()) { + if (medicines.length === 0 || medicines.some((m) => !m.name.trim())) { return toast.error('Please add at least one medication'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/DigitalPrescriptionGenerator.jsx` around lines 80 - 82, Update the submission validation around the medicine check to validate every entry in the medicines array, rejecting when the array is empty or any medicine has a missing or whitespace-only name. Keep the existing toast error and ensure the validation covers rows added through “Add Medication.”frontend/src/services/api.js-25-49 (1)
25-49: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDe-duplicate refresh attempts in
frontend/src/services/api.js. Multiple simultaneous 401s can each POST to/auth/refresh; if refresh tokens are rotated or single-use, the later calls can fail and force an unnecessary logout. Share one in-flight refresh promise and retry queued requests with the returned token.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/services/api.js` around lines 25 - 49, Deduplicate concurrent token refreshes in the response interceptor by introducing a shared in-flight refresh promise (or equivalent) outside the interceptor. When handling a 401 in the interceptor, reuse the existing promise instead of posting again, then update the request Authorization header and retry after it resolves; clear the shared promise when complete, and only clear storage and redirect to login if the shared refresh operation fails.frontend/src/pages/DoctorProfileView.jsx-88-96 (1)
88-96: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftHandle payment failure after booking
frontend/src/pages/DoctorProfileView.jsx:98-114— the appointment is created beforepaymentsAPI.createOrder(). If payment setup fails, the catch shows a generic booking error even though the appointment already exists, and retrying can create duplicates. Add a compensating path or keep the appointment pending until payment succeeds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/DoctorProfileView.jsx` around lines 88 - 96, Update the booking flow surrounding the appointment creation call and paymentsAPI.createOrder so payment failure does not leave an apparently completed appointment: either create the appointment in a pending state and finalize it only after successful payment, or invoke a reliable cancellation/rollback path when payment setup fails. Adjust the catch handling to report the payment-specific failure and prevent retries from creating duplicate appointments.frontend/src/pages/DigitalPrescriptionGenerator.jsx-396-406 (1)
396-406: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winLoad doctor credentials from
DoctorProfilebefore rendering the prescription header.
login/registeronly put basic user fields intouser, andUserhas nospecializationorlicenseNumber, so this header will fall back to placeholder credentials unless you fetch the doctor profile for the logged-in doctor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/DigitalPrescriptionGenerator.jsx` around lines 396 - 406, The prescription header currently reads specialization and licenseNumber from the basic user object, causing placeholder values. In DigitalPrescriptionGenerator, fetch the logged-in doctor’s DoctorProfile using the user’s identifier, store the profile credentials in component state, and render the doctor name, specialization, and licenseNumber from that profile with appropriate loading or fallback handling.frontend/src/pages/DoctorEarnings.jsx-11-146 (1)
11-146: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the payment records for this ledger
paymentsAPI.getAll()is already fetched, but the table still derives invoice IDs, status, and amounts from appointments. That makes refunded/failed payments and the realinvoiceNumberinvisible here, so the earnings view can drift from the actual settlement data. Also fix the duplicated word in the export button label.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/DoctorEarnings.jsx` around lines 11 - 146, Update DoctorEarnings to build the ledger from the fetched payments state and the payment record’s real invoiceNumber, status, amount, and associated appointment details, rather than mapping completedApps; ensure refunded and failed records are represented accurately and totals use settled payment data. Also change the export button label from “Export Statement Statement PDF” to “Export Statement PDF”.frontend/src/pages/DoctorProfileView.jsx-85-120 (1)
85-120: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake booking + payment idempotent
appointmentsAPI.bookpersists the appointment beforepaymentsAPI.createOrderruns, so a payment-order failure leaves a booked appointment behind with no payment record while the UI shows a generic booking error. Retrying can create a duplicate appointment for the same slot. Make the flow atomic or idempotent so booking and payment stay consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/DoctorProfileView.jsx` around lines 85 - 120, Update the booking flow around the component’s booking handler to make appointment creation and payment-order creation atomic or safely retryable: prefer a backend endpoint/service that creates the appointment and payment together with a shared idempotency key, and call it instead of separate appointmentsAPI.book and paymentsAPI.createOrder requests. Ensure retries reuse the same key and existing appointment/payment records, prevent duplicate bookings for the same doctor/date/time slot, and only set confirmed booking after both operations succeed; surface partial-failure handling rather than leaving orphaned appointments.
🟡 Minor comments (5)
backend/src/middleware/rateLimit.middleware.js-14-21 (1)
14-21: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winApply
authLimiterto the auth routesbackend/server.jsonly mountsapiLimiter, andbackend/src/routes/auth.routes.jsnever attachesauthLimiter, so login/register still use the looser API limit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/middleware/rateLimit.middleware.js` around lines 14 - 21, Attach the existing authLimiter middleware to the login and registration routes in auth.routes.js, and ensure server.js mounts those auth routes without relying only on apiLimiter; use the authLimiter export from rateLimit.middleware.js so authentication endpoints receive the stricter limit.backend/src/models/DoctorProfile.model.js-49-56 (1)
49-56: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDefault
ratingandtotalReviewsto zero, not fabricated values.New doctors default to
rating: 4.8andtotalReviews: 12, displaying fake reviews to patients. This is misleading and could erode trust. Initialize both to0.🔧 Proposed fix
rating: { type: Number, - default: 4.8 + default: 0 }, totalReviews: { type: Number, - default: 12 + default: 0 },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/models/DoctorProfile.model.js` around lines 49 - 56, Update the `rating` and `totalReviews` fields in the `DoctorProfile` schema to use `default: 0` instead of fabricated values, ensuring new doctors start with no rating or reviews.backend/src/controllers/chat.controller.js-123-126 (1)
123-126: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winEscape user input before using it in
$regex.
queryis passed unescaped into$regex, allowing regex-injection/ReDoS against the messages collection via crafted metacharacters. Escape the string (or use$text).🛡️ Proposed fix
+ const escaped = query.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const messages = await Message.find({ conversationId: { $in: convIds }, - text: { $regex: query, $options: 'i' } + text: { $regex: escaped, $options: 'i' } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/chat.controller.js` around lines 123 - 126, Escape the user-provided query before assigning it to `$regex` in the message lookup within the chat controller. Add or reuse a regex-escaping helper, apply it to `query`, and continue using the escaped value with the case-insensitive option to prevent regex injection and ReDoS.frontend/src/pages/DoctorEarnings.jsx-57-57 (1)
57-57: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTypo: duplicated "Statement" in button label.
✏️ Suggested fix
- <Download size={14} className="text-cyan-400" /> Export Statement Statement PDF + <Download size={14} className="text-cyan-400" /> Export Statement PDF🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/DoctorEarnings.jsx` at line 57, Remove the duplicated “Statement” from the export button label in the JSX containing the Download icon, so it reads “Export Statement PDF”.frontend/src/pages/AdminDashboard.jsx-236-247 (1)
236-247: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winHardcoded backend origin breaks the license link outside local dev. Use a shared backend base URL here instead of
http://localhost:4500, or the certificate link will fail in deployed environments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/AdminDashboard.jsx` around lines 236 - 247, Replace the hardcoded http://localhost:4500 origin in the license certificate anchor within the licenseDocumentUrl rendering block with the shared backend base URL configuration already used by the frontend, preserving the document path and link behavior for deployed environments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c421166b-1208-4fb8-9314-c9c3571843c4
⛔ Files ignored due to path filters (2)
backend/package-lock.jsonis excluded by!**/package-lock.jsonfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (56)
backend/package.jsonbackend/server.jsbackend/src/controllers/admin.controller.jsbackend/src/controllers/appointment.controller.jsbackend/src/controllers/auth.controller.jsbackend/src/controllers/chat.controller.jsbackend/src/controllers/digitalPrescription.controller.jsbackend/src/controllers/doctor.controller.jsbackend/src/controllers/notification.controller.jsbackend/src/controllers/patient.controller.jsbackend/src/controllers/payment.controller.jsbackend/src/controllers/report.controller.jsbackend/src/middleware/audit.middleware.jsbackend/src/middleware/auth.middleware.jsbackend/src/middleware/rateLimit.middleware.jsbackend/src/middleware/role.middleware.jsbackend/src/models/Appointment.model.jsbackend/src/models/AuditLog.model.jsbackend/src/models/Conversation.model.jsbackend/src/models/DigitalPrescription.model.jsbackend/src/models/DoctorProfile.model.jsbackend/src/models/Message.model.jsbackend/src/models/Notification.model.jsbackend/src/models/PatientProfile.model.jsbackend/src/models/Payment.model.jsbackend/src/models/Report.model.jsbackend/src/models/Review.model.jsbackend/src/models/TimeSlot.model.jsbackend/src/models/User.model.jsbackend/src/routes/admin.routes.jsbackend/src/routes/appointment.routes.jsbackend/src/routes/auth.routes.jsbackend/src/routes/chat.routes.jsbackend/src/routes/digitalPrescription.routes.jsbackend/src/routes/doctor.routes.jsbackend/src/routes/notification.routes.jsbackend/src/routes/patient.routes.jsbackend/src/routes/payment.routes.jsbackend/src/routes/report.routes.jsbackend/src/socket/socket.jsfrontend/package.jsonfrontend/src/App.jsxfrontend/src/components/CallModal.jsxfrontend/src/components/Navbar.jsxfrontend/src/components/NotificationDropdown.jsxfrontend/src/pages/AdminDashboard.jsxfrontend/src/pages/ChatRoom.jsxfrontend/src/pages/DigitalPrescriptionGenerator.jsxfrontend/src/pages/DoctorDashboard.jsxfrontend/src/pages/DoctorEarnings.jsxfrontend/src/pages/DoctorProfileView.jsxfrontend/src/pages/DoctorSearch.jsxfrontend/src/pages/MedicalRecordsPortal.jsxfrontend/src/pages/PatientAppointments.jsxfrontend/src/services/api.jsfrontend/src/services/socket.js
| app.use(cors({ | ||
| origin: 'http://localhost:5173', | ||
| origin: ['http://localhost:5173', 'http://127.0.0.1:5173', '*'], | ||
| credentials: true | ||
| })); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Critical CORS misconfiguration: credentialed requests allowed from any origin.
With credentials: true and '*' in the origin array, the cors package reflects any request origin back in Access-Control-Allow-Origin while also sending Access-Control-Allow-Credentials: true. This allows any website to make credentialed cross-origin requests to the API, enabling CSRF-style attacks if cookies or other browser-attached credentials are ever used. The '*' entry also renders the specific localhost entries pointless.
Remove '*' and restrict origins to trusted origins only. For production, drive this via an environment variable.
🔒 Proposed fix for CORS configuration
app.use(cors({
- origin: ['http://localhost:5173', 'http://127.0.0.1:5173', '*'],
+ origin: (process.env.CORS_ORIGINS || 'http://localhost:5173,http://127.0.0.1:5173').split(','),
credentials: true
}));📝 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.
| app.use(cors({ | |
| origin: 'http://localhost:5173', | |
| origin: ['http://localhost:5173', 'http://127.0.0.1:5173', '*'], | |
| credentials: true | |
| })); | |
| app.use(cors({ | |
| origin: (process.env.CORS_ORIGINS || 'http://localhost:5173,http://127.0.0.1:5173').split(','), | |
| credentials: true | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/server.js` around lines 22 - 25, Restrict the CORS allowlist in the
`app.use(cors(...))` configuration by removing `'*'` and retaining only trusted
origins. Make the production origins configurable through an environment
variable while preserving `credentials: true` for approved origins.
| doctorProfile.verificationStatus = status; | ||
| if (status === 'rejected') { | ||
| doctorProfile.rejectionReason = rejectionReason || 'License verification failed'; | ||
| } else { | ||
| doctorProfile.rejectionReason = ''; | ||
| if (doctorProfile.userId) { | ||
| doctorProfile.userId.isVerified = true; | ||
| await doctorProfile.userId.save(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
verifyDoctor incorrectly sets isVerified = true for pending status.
The else branch covers both verified and pending, so setting status to pending still calls doctorProfile.userId.isVerified = true. Only verified should trigger this.
🐛 Proposed fix
doctorProfile.verificationStatus = status;
if (status === 'rejected') {
doctorProfile.rejectionReason = rejectionReason || 'License verification failed';
- } else {
+ } else if (status === 'verified') {
doctorProfile.rejectionReason = '';
if (doctorProfile.userId) {
doctorProfile.userId.isVerified = true;
await doctorProfile.userId.save();
}
}📝 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.
| doctorProfile.verificationStatus = status; | |
| if (status === 'rejected') { | |
| doctorProfile.rejectionReason = rejectionReason || 'License verification failed'; | |
| } else { | |
| doctorProfile.rejectionReason = ''; | |
| if (doctorProfile.userId) { | |
| doctorProfile.userId.isVerified = true; | |
| await doctorProfile.userId.save(); | |
| } | |
| } | |
| doctorProfile.verificationStatus = status; | |
| if (status === 'rejected') { | |
| doctorProfile.rejectionReason = rejectionReason || 'License verification failed'; | |
| } else if (status === 'verified') { | |
| doctorProfile.rejectionReason = ''; | |
| if (doctorProfile.userId) { | |
| doctorProfile.userId.isVerified = true; | |
| await doctorProfile.userId.save(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/admin.controller.js` around lines 63 - 72, Update
verifyDoctor so user verification is enabled only when status === 'verified';
keep rejectionReason cleared for other non-rejected statuses, but move the
doctorProfile.userId.isVerified assignment and save into a dedicated
verified-status branch.
| export const rescheduleAppointment = async (req, res) => { | ||
| try { | ||
| const { id } = req.params; | ||
| const { newDate, newTimeSlot } = req.body; | ||
|
|
||
| const appointment = await Appointment.findById(id); | ||
| if (!appointment) { | ||
| return res.status(404).json({ success: false, message: 'Appointment not found' }); | ||
| } | ||
|
|
||
| appointment.appointmentDate = newDate; | ||
| appointment.timeSlot = newTimeSlot; | ||
| appointment.status = 'approved'; // Re-approve when rescheduled | ||
| await appointment.save(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
rescheduleAppointment has no authorization check — any user can reschedule any appointment.
Unlike updateAppointmentStatus (which verifies ownership at Line 122), this function fetches and modifies an appointment by ID without checking that the caller is the patient, doctor, or admin. Additionally, newDate and newTimeSlot are not validated for presence, allowing undefined values to overwrite the appointment.
🛡️ Proposed fix
export const rescheduleAppointment = async (req, res) => {
try {
const { id } = req.params;
const { newDate, newTimeSlot } = req.body;
+ if (!newDate || !newTimeSlot) {
+ return res.status(400).json({ success: false, message: 'newDate and newTimeSlot are required' });
+ }
+
const appointment = await Appointment.findById(id);
if (!appointment) {
return res.status(404).json({ success: false, message: 'Appointment not found' });
}
+ if (req.user.role !== 'admin' &&
+ req.user._id.toString() !== appointment.doctorId.toString() &&
+ req.user._id.toString() !== appointment.patientId.toString()) {
+ return res.status(403).json({ success: false, message: 'Not authorized to reschedule this appointment' });
+ }
+
appointment.appointmentDate = newDate;📝 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 const rescheduleAppointment = async (req, res) => { | |
| try { | |
| const { id } = req.params; | |
| const { newDate, newTimeSlot } = req.body; | |
| const appointment = await Appointment.findById(id); | |
| if (!appointment) { | |
| return res.status(404).json({ success: false, message: 'Appointment not found' }); | |
| } | |
| appointment.appointmentDate = newDate; | |
| appointment.timeSlot = newTimeSlot; | |
| appointment.status = 'approved'; // Re-approve when rescheduled | |
| await appointment.save(); | |
| export const rescheduleAppointment = async (req, res) => { | |
| try { | |
| const { id } = req.params; | |
| const { newDate, newTimeSlot } = req.body; | |
| if (!newDate || !newTimeSlot) { | |
| return res.status(400).json({ success: false, message: 'newDate and newTimeSlot are required' }); | |
| } | |
| const appointment = await Appointment.findById(id); | |
| if (!appointment) { | |
| return res.status(404).json({ success: false, message: 'Appointment not found' }); | |
| } | |
| if (req.user.role !== 'admin' && | |
| req.user._id.toString() !== appointment.doctorId.toString() && | |
| req.user._id.toString() !== appointment.patientId.toString()) { | |
| return res.status(403).json({ success: false, message: 'Not authorized to reschedule this appointment' }); | |
| } | |
| appointment.appointmentDate = newDate; | |
| appointment.timeSlot = newTimeSlot; | |
| appointment.status = 'approved'; // Re-approve when rescheduled | |
| await appointment.save(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/appointment.controller.js` around lines 157 - 170,
Update rescheduleAppointment to validate that newDate and newTimeSlot are
present before modifying the appointment, returning a 400 response for missing
values. Add authorization matching updateAppointmentStatus so only the
appointment’s patient, doctor, or an admin can reschedule it; return 403 for
unauthorized callers, and perform these checks before assigning fields and
saving.
| // For demo/instant testing: generate a simple reset token and return it or store it | ||
| const resetToken = Math.random().toString(36).substring(2, 10); | ||
| user.resetPasswordToken = resetToken; | ||
| user.resetPasswordExpire = Date.now() + 30 * 60 * 1000; // 30 mins | ||
| await user.save(); | ||
|
|
||
| await AuditLog.create({ | ||
| userId: user._id, | ||
| action: 'Password reset requested', | ||
| status: 'success' | ||
| }); | ||
|
|
||
| res.json({ | ||
| success: true, | ||
| message: 'Password reset token generated successfully.', | ||
| resetToken // Returned directly for immediate UI demonstration | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Password reset token is cryptographically insecure and leaked in the response.
Math.random() is predictable and brute-forceable (CWE-330). Additionally, the reset token is returned directly in the JSON response, meaning anyone who knows an email can retrieve the token. Use crypto.randomBytes and deliver the token out-of-band (e.g., email).
🛡️ Proposed fix
- const resetToken = Math.random().toString(36).substring(2, 10);
+ import crypto from 'crypto';
+ const resetToken = crypto.randomBytes(32).toString('hex');
user.resetPasswordToken = resetToken;
user.resetPasswordExpire = Date.now() + 30 * 60 * 1000;
await user.save(); res.json({
success: true,
- message: 'Password reset token generated successfully.',
- resetToken // Returned directly for immediate UI demonstration
+ message: 'If that email exists, a reset link has been sent.'
});🧰 Tools
🪛 ast-grep (0.44.1)
[error] 209-209: Math.random() is not cryptographically secure and is being used to generate a security-sensitive value (token, id, password, secret, nonce, salt, OTP, or CSRF value). Its output is predictable and can be brute-forced, allowing attackers to guess the value. Use a CSPRNG instead, e.g. crypto.randomBytes(...).toString('hex') or crypto.randomUUID() in Node.js, or crypto.getRandomValues(...) in the browser.
Context: Math.random()
Note: [CWE-330] Use of Insufficiently Random Values.
(insecure-random-security-token-javascript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/auth.controller.js` around lines 209 - 225, Replace
the predictable token generation in the password-reset handler with
cryptographically secure bytes via Node’s crypto.randomBytes, store only a hash
of the token in user.resetPasswordToken, and retain the expiry. Remove
resetToken from the response in the handler, and deliver the plaintext token
through the configured out-of-band email mechanism instead.
Source: Linters/SAST tools
| export const getMessages = async (req, res) => { | ||
| try { | ||
| const { conversationId } = req.params; | ||
|
|
||
| const messages = await Message.find({ conversationId }) | ||
| .populate('sender', 'name avatarUrl role') | ||
| .sort('createdAt'); | ||
|
|
||
| // Mark messages not sent by logged in user as seen | ||
| await Message.updateMany( | ||
| { conversationId, sender: { $ne: req.user._id }, seenBy: { $ne: req.user._id } }, | ||
| { $addToSet: { seenBy: req.user._id } } | ||
| ); | ||
|
|
||
| res.json({ success: true, messages }); | ||
| } catch (error) { | ||
| console.error('Get Messages Error:', error); | ||
| res.status(500).json({ success: false, message: error.message }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the controller with line numbers.
sed -n '1,220p' backend/src/controllers/chat.controller.js | cat -n
printf '\n===== conversation model files =====\n'
git ls-files 'backend/src/**/conversation*' 'backend/src/**/Conversation*' 'backend/src/**/message*' 'backend/src/**/Message*'
printf '\n===== search for Conversation / participants / sender usage =====\n'
rg -n "Conversation|participants|conversationId|seenBy|sendMessage|getMessages|searchMessages" backend/src -g '!**/node_modules/**'Repository: arpit168/MedTech
Length of output: 11718
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map nearby models/controllers if the first search shows relevant files.
fd -a -t f 'conversation.*|message.*|chat.*' backend/srcRepository: arpit168/MedTech
Length of output: 384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read any conversation/message schema definitions if the file list reveals them.
for f in $(git ls-files 'backend/src/**/conversation*' 'backend/src/**/Conversation*' 'backend/src/**/message*' 'backend/src/**/Message*' 'backend/src/**/*.js' 'backend/src/**/*.ts'); do
case "$f" in
*conversation*|*Conversation*|*message*|*Message*)
echo "===== $f ====="
sed -n '1,220p' "$f" | cat -n
;;
esac
doneRepository: arpit168/MedTech
Length of output: 2046
Enforce conversation membership before reading or writing messages.
getMessages and sendMessage both trust the supplied conversationId without checking that req.user is one of the conversation participants. That lets any authenticated user read, mark seen, and post into arbitrary conversations. Load the conversation first and return 403 unless the user is included in participants.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/chat.controller.js` around lines 52 - 71, Enforce
participant authorization in getMessages and sendMessage before any message
read, seen-status update, or creation. Load the conversation by the supplied
conversationId, verify req.user._id is included in its participants, and return
a 403 response when the conversation is missing or the user is not a
participant; only then perform the existing message operations.
| if (search) { | ||
| const searchRegex = new RegExp(search, 'i'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
ReDoS vulnerability: user-supplied search passed directly to RegExp constructor.
new RegExp(search, 'i') allows an attacker to craft a search string with catastrophic backtracking patterns (e.g., (a+)+$), hanging the Node.js event loop. Escape special regex characters or use String.prototype.includes() for simple substring matching.
🔒 Proposed fix using escapeRegExp
if (search) {
- const searchRegex = new RegExp(search, 'i');
+ const escapeRegExp = s => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+ const searchRegex = new RegExp(escapeRegExp(search), 'i');
doctorProfiles = doctorProfiles.filter(doc =>📝 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.
| if (search) { | |
| const searchRegex = new RegExp(search, 'i'); | |
| if (search) { | |
| const escapeRegExp = s => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | |
| const searchRegex = new RegExp(escapeRegExp(search), 'i'); | |
| doctorProfiles = doctorProfiles.filter(doc => |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 26-26: Detects non-literal values in regular expressions
Context: new RegExp(search, 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).
(detect-non-literal-regexp)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/doctor.controller.js` around lines 26 - 27, Prevent
ReDoS in the search handling within the doctor controller by avoiding
compilation of raw user input with new RegExp. Prefer case-insensitive substring
matching via String.prototype.includes(), or escape all regex metacharacters
before constructing the pattern, and update the corresponding filtering logic to
use the safe value.
Source: Linters/SAST tools
| const storage = multer.diskStorage({ | ||
| destination: 'uploads/', | ||
| filename: (req, file, cb) => { | ||
| cb(null, `${Date.now()}-${file.originalname}`); | ||
| } | ||
| }); | ||
| const upload = multer({ storage }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== files ==\n'
git ls-files | rg '^(backend/src/routes/doctor\.routes\.js|package\.json|package-lock\.json|pnpm-lock\.yaml|yarn\.lock)$' || true
printf '\n== doctor.routes.js excerpt ==\n'
sed -n '1,80p' backend/src/routes/doctor.routes.js
printf '\n== dependency version search ==\n'
rg -n '"multer"|multer@|multer:' package.json package-lock.json pnpm-lock.yaml yarn.lock backend/package.json backend/package-lock.json backend/pnpm-lock.yaml backend/yarn.lock 2>/dev/null || true
printf '\n== node path probe ==\n'
node - <<'JS'
const path = require('path');
const examples = [
['uploads/', '123-../../../tmp/evil.txt'],
['uploads', '../tmp/evil.txt'],
['uploads', 'safe.txt'],
];
for (const args of examples) {
console.log(args.join(' + '), '=>', path.join(...args));
}
JSRepository: arpit168/MedTech
Length of output: 1291
🏁 Script executed:
set -euo pipefail
printf '\n== multer source probe ==\n'
node - <<'JS'
try {
const multer = require('multer');
console.log('multer package loaded');
console.log('multer path:', require.resolve('multer'));
} catch (e) {
console.log('multer not resolvable:', e.message);
}
JS
printf '\n== search for upload middleware references ==\n'
rg -n "multer\\(" backend/src backend 2>/dev/null || trueRepository: arpit168/MedTech
Length of output: 887
🌐 Web query:
Multer 2.1.1 diskStorage filename originalname path.join source sanitize
💡 Result:
When using Multer (including version 2.1.1) with diskStorage, you should never use the originalname provided by the client directly as the filename on your server [1][2]. Doing so exposes your application to critical security vulnerabilities, most notably path traversal attacks, where a malicious user could supply a filename like ../../etc/passwd to overwrite files outside your intended storage directory [3][1]. To handle file storage safely, follow these practices: 1. Generate Unique, Secure Filenames: Always generate a new, server-side filename using a cryptographically secure method, such as crypto.randomUUID() or crypto.randomBytes() [4][1][2]. Avoid using Math.random(), as it is not cryptographically secure [4]. 2. Decouple User-Provided Names: If you need to track the original name (e.g., for display purposes), store the original filename in your database, not on the filesystem [2]. 3. Validate Extensions: Extract the file extension from the originalname using path.extname(), but strictly validate it against a whitelist of allowed extensions before appending it to your generated filename [1]. 4. Use path.join Securely: When constructing storage paths, use path.join() to combine your intended storage directory with the generated filename [2]. Ensure your storage directory is outside your application's root/public folder to prevent unauthorized direct access [1][2]. Example of a safe diskStorage configuration: javascript const crypto = require('crypto'); const path = require('path'); const multer = require('multer'); const storage = multer.diskStorage({ destination: function (req, file, cb) { // Ensure this directory exists cb(null, '/path/to/secure/storage/directory/'); }, filename: function (req, file, cb) { // 1. Generate a secure, unique name const uniqueName = crypto.randomUUID(); // 2. Safely extract and validate the extension const ext = path.extname(file.originalname).toLowerCase(); const allowedExtensions = ['.jpg', '.jpeg', '.png', '.pdf']; if (!allowedExtensions.includes(ext)) { return cb(new Error('Invalid file type')); } cb(null, uniqueName + ext); } }); const upload = multer({ storage: storage }); If you must process originalname (e.g., if you are not using a secure generator), note that originalname may require decoding using decodeURIComponent() if it contains URL-encoded characters [5]. Additionally, be aware that originalname might be encoded in latin1 depending on the client; if you encounter encoding issues, you may need to convert the buffer: Buffer.from(file.originalname, 'latin1').toString('utf8') [5].
Citations:
- 1: https://justappsec.com/guides/secure-file-uploads-in-nodejs
- 2: https://pompelmi.app/blog/nodejs-file-upload-security-checklist.html
- 3: https://www.nodejs-security.com/learn/secure-file-handling/weak-multer-file-name-manipulation
- 4: Security: README diskStorage example uses Math.random() for filenames instead of crypto-safe random expressjs/multer#1386
- 5: originalname is not properly decoded expressjs/multer#836
🌐 Web query:
Multer diskStorage filename callback originalname source code file.originalname unsanitized
💡 Result:
Using file.originalname directly in a Multer diskStorage filename callback is a significant security risk because originalname is user-controlled data provided via the Content-Disposition multipart header [1][2]. Multer does not sanitize this value by design, treating it as the developer's responsibility [1]. ### Security Risks 1. Path Traversal: Attackers can set the filename to values like ../../malicious.js. If the storage engine uses this value to build a file path, it may allow the attacker to write files outside the intended directory, potentially overwriting application code, configuration files, or other sensitive system files [3][1]. 2. File Collisions: Using the original name can lead to conflicts if two users upload files with the same name, potentially resulting in data being overwritten or inaccessible [4][2]. ### Recommended Best Practices To securely handle file uploads, developers should treat file.originalname as untrusted input and implement the following measures: 1. Never use file.originalname directly: Always generate a new, unique, and safe filename on the server-side [4][2]. 2. Sanitize and Isolate: If you must use parts of the original name (e.g., the extension), use path.basename() to strip directory components and validate the extension against a strict allow-list [1]. 3. Use Cryptographically Secure Names: Instead of relying on Date.now() or Math.random()—which can be predictable—generate filenames using secure random methods like crypto.randomBytes or UUID libraries [5][4]. ### Secure Implementation Example javascript const crypto = require('crypto'); const path = require('path'); const multer = require('multer'); const storage = multer.diskStorage({ destination: (req, file, cb) => { cb(null, '/uploads/'); }, filename: (req, file, cb) => { // Generate a secure, unique filename crypto.randomBytes(16, (err, raw) => { if (err) return cb(err); // Sanitize extension: only keep the extension part and validate it const ext = path.extname(file.originalname).toLowerCase(); const safeFilename = raw.toString('hex') + ext; cb(null, safeFilename); }); } }); const upload = multer({ storage: storage }); By generating server-side filenames and only storing the original filename as metadata in your database, you decouple the user-provided input from the actual filesystem storage [2].
Citations:
- 1: fix(decryptpdf): prevent path traversal in multer filename callback OpenSignLabs/OpenSign#2181
- 2: https://thelinuxcode.com/multer-npm-in-2026-file-uploads-in-express-that-dont-bite-you-later/
- 3: https://www.nodejs-security.com/learn/secure-file-handling/weak-multer-file-name-manipulation
- 4: https://www.stacklesson.com/mean-stack-tutorial/rest-apis-express/file-uploads-multer-storage-engines/
- 5: Security: README diskStorage example uses Math.random() for filenames instead of crypto-safe random expressjs/multer#1386
🌐 Web query:
site:github.com/expressjs/multer diskStorage file.originalname filename callback source
💡 Result:
In Multer, diskStorage allows you to define a filename function that determines the name of the file saved to disk [1][2]. This function is called with three arguments: (req, file, cb) [1][3]. Arguments: - req: The Express request object [1]. - file: An object containing information about the file being processed [1][3]. This object includes the property file.originalname, which represents the name of the file on the user's computer [2][3]. - cb: A callback function used to return the result [1]. Usage: The filename function must call the callback (cb) with two arguments: (error, filename) [1][2]. - The first argument should be null if there is no error [1]. - The second argument must be the desired filename, including the file extension [1][2]. Important Considerations: - Multer does not automatically append file extensions; you must include them in the returned filename [1][4]. - If no filename function is provided, Multer generates a random filename without an extension [1][3]. - The destination function is always executed before the filename function [5]. - If you need to use the original name, you can access it via file.originalname [6]. Note that in some environments, you may need to manually decode this value using decodeURIComponent if it is not properly encoded [7]. - Because the filename function is called when a file is detected on the wire, req.body may not yet be populated if the form fields were sent after the file [6]. Example: const storage = multer.diskStorage({ destination: function (req, file, cb) { cb(null, '/tmp/my-uploads') }, filename: function (req, file, cb) { cb(null, file.originalname) // Uses the original file name } })
Citations:
- 1: https://github.com/expressjs/multer/
- 2: https://github.com/expressjs/multer/blob/master/README.md
- 3: https://github.com/expressjs/multer/tree/v2.1.1
- 4: https://github.com/expressjs/multer/blob/main/README.md
- 5: How can i get mutated filename value inside destination? expressjs/multer#793
- 6: How do I change filename of the file? expressjs/multer#280
- 7: originalname is not properly decoded expressjs/multer#836
🌐 Web query:
expressjs multer diskStorage originalname sanitize path.join directory traversal
💡 Result:
When using Multer with diskStorage, never trust file.originalname directly for your file system paths [1][2]. Relying on user-provided input can lead to path traversal vulnerabilities where a malicious filename (such as../../../etc/passwd) could allow an attacker to write files outside your intended upload directory [3][1]. To secure your file uploads, follow these practices: 1. Generate Unique Filenames Server-Side: Do not use the user-provided filename [1][2]. Instead, generate a unique, cryptographically secure identifier on the server (e.g., using crypto.randomUUID or crypto.randomBytes) [4][1][2]. 2. Sanitize and Validate Extensions: If you must preserve the original file extension for display purposes, extract it using path.extname and validate it against an allowlist of permitted extensions [1][2]. Ensure you sanitize or ignore any path-related characters in the original name [2]. 3. Isolate Storage: Use path.join to explicitly define your upload directory and ensure the final path is resolved within that expected directory [1][2]. 4. Store Metadata Separately: Store the original filename in your database as metadata only [2]. When serving or processing files later, use your server-generated identifier as the key [2]. Example of secure implementation: const crypto = require('crypto'); const path = require('path'); const multer = require('multer'); const storage = multer.diskStorage({ destination: (req, file, cb) => { cb(null, path.join(dirname, 'uploads')); }, filename: (req, file, cb) => { const uniqueName = crypto.randomUUID; const ext = path.extname(file.originalname).toLowerCase; // Validate ext here if necessary cb(null, ${uniqueName}${ext}); } }); const upload = multer({ storage: storage });
Citations:
- 1: https://www.stacklesson.com/mean-stack-tutorial/rest-apis-express/file-uploads-multer-storage-engines/
- 2: https://thelinuxcode.com/multer-npm-in-2026-file-uploads-in-express-that-dont-bite-you-later/
- 3: https://www.nodejs-security.com/learn/secure-file-handling/weak-multer-file-name-manipulation
- 4: Security: README diskStorage example uses Math.random() for filenames instead of crypto-safe random expressjs/multer#1386
Sanitize upload filenames and cap uploads
file.originalnameis user-controlled; using it directly infilenamelets../segments escapeuploads/and write outside the intended directory.- Add
limits.fileSizeand afileFilterallowlist too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/routes/doctor.routes.js` around lines 8 - 14, Sanitize the
user-controlled filename in the multer `storage.filename` callback by removing
path components and unsafe characters before constructing the destination name,
ensuring uploads remain within `uploads/`. Configure the `multer` instance in
`upload` with an appropriate `limits.fileSize` cap and a `fileFilter` that
allowlists permitted file types.
| e.preventDefault(); | ||
| if (!patientId && !appointmentId && !patientName) { | ||
| return toast.error('Please specify patient details'); | ||
| } | ||
| if (medicines.length === 0 || !medicines[0].name.trim()) { | ||
| return toast.error('Please add at least one medication'); | ||
| } | ||
|
|
||
| setSubmitting(true); | ||
| try { | ||
| const payload = { | ||
| patientId: patientId || user.id || '64101234567890abcdef1234', | ||
| appointmentId: appointmentId || undefined, | ||
| diagnosis, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Broken validation lets prescriptions submit with no real patient, silently falling back to a hardcoded ObjectId.
patientName defaults to 'Patient Consultation' (line 19) and is never empty, so the guard at line 77 (!patientId && !appointmentId && !patientName) can never be true — a doctor can submit the form with patientId and appointmentId both blank. The payload then falls back to a hardcoded fake ObjectId ('64101234567890abcdef1234') at line 87, silently attaching a real, legally relevant prescription to an unrelated/nonexistent patient record instead of rejecting the submission.
🐛 Suggested fix
- if (!patientId && !appointmentId && !patientName) {
- return toast.error('Please specify patient details');
+ if (!patientId && !appointmentId) {
+ return toast.error('Please specify a patient ID or select an appointment');
} const payload = {
- patientId: patientId || user.id || '64101234567890abcdef1234',
+ patientId,
appointmentId: appointmentId || undefined,📝 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.
| e.preventDefault(); | |
| if (!patientId && !appointmentId && !patientName) { | |
| return toast.error('Please specify patient details'); | |
| } | |
| if (medicines.length === 0 || !medicines[0].name.trim()) { | |
| return toast.error('Please add at least one medication'); | |
| } | |
| setSubmitting(true); | |
| try { | |
| const payload = { | |
| patientId: patientId || user.id || '64101234567890abcdef1234', | |
| appointmentId: appointmentId || undefined, | |
| diagnosis, | |
| e.preventDefault(); | |
| if (!patientId && !appointmentId) { | |
| return toast.error('Please specify a patient ID or select an appointment'); | |
| } | |
| if (medicines.length === 0 || !medicines[0].name.trim()) { | |
| return toast.error('Please add at least one medication'); | |
| } | |
| setSubmitting(true); | |
| try { | |
| const payload = { | |
| patientId, | |
| appointmentId: appointmentId || undefined, | |
| diagnosis, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/DigitalPrescriptionGenerator.jsx` around lines 76 - 89,
Fix patient validation in the prescription submit handler: do not rely on the
default patientName value, and require a valid patientId before submission.
Remove the hardcoded fallback ObjectId from the payload construction, using the
validated patientId directly; reject the submission with an error toast when it
is missing or invalid. Update the validation and payload logic around the submit
handler and its patientId handling.
1 participant
medtech project is in project
Summary by CodeRabbit