Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion backend/controllers/notesSummaryController.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const axios = require("axios");
const crypto = require("crypto");
const fs = require("fs");
const { generateWithFallback } = require("../utils/geminiHelper");
const {
inspectPdfBuffer,
Expand Down Expand Up @@ -103,14 +104,16 @@
* @example
*/
const summarizeNotes = async (req, res) => {
let uploadedFilePath = null;
try {
let buffer;
let fileName;
let sourceType;
let sourceUrl = null;

if (req.file) {
buffer = req.file.buffer;
uploadedFilePath = req.file.path;
buffer = await fs.promises.readFile(uploadedFilePath);
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
coderabbitai[bot] marked this conversation as resolved.
fileName = req.file.originalname;
sourceType = "upload";
} else {
Expand Down Expand Up @@ -238,6 +241,10 @@
}

res.status(500).json({ success: false, message: "Failed to summarize notes." });
} finally {
if (uploadedFilePath) {
fs.promises.unlink(uploadedFilePath).catch(err => console.error("Failed to delete temp notes PDF:", err));
}
Comment on lines +247 to +252

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | πŸ—οΈ Heavy lift

Make temporary-file cleanup cover the complete post-upload lifecycle.

The controller finally blocks do not run when earlier route middleware returns a validation error. Those files remain on disk and can create a disk-exhaustion DoS.

  • backend/controllers/notesSummaryController.js#L244-L247: clean the file when validateSummarizeNotes short-circuits before summarizeNotes.
  • backend/controllers/resumeController.js#L169-L172: clean the file when validateResumeMagicBytes or validateAnalyzeResume short-circuits before analyzeResume.
πŸ“ Affects 2 files
  • backend/controllers/notesSummaryController.js#L244-L247 (this comment)
  • backend/controllers/resumeController.js#L169-L172
πŸ€– 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/controllers/notesSummaryController.js` around lines 244 - 247, Extend
temporary-file cleanup beyond the controller finally blocks so files are removed
when validation middleware short-circuits: update
backend/controllers/notesSummaryController.js lines 244-247 around
summarizeNotes/validateSummarizeNotes, and
backend/controllers/resumeController.js lines 169-172 around
analyzeResume/validateResumeMagicBytes/validateAnalyzeResume. Ensure each
uploaded temporary file is deleted exactly once across both validation-error and
normal post-upload paths, preserving the existing error handling for unlink
failures.

}
};

Expand All @@ -257,7 +264,7 @@
message: validation.error.issues.map((issue) => issue.message),
});
}

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
const {
fileName,
sourceType,
Expand Down
11 changes: 10 additions & 1 deletion backend/controllers/resumeController.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const axios = require('axios');
const FormData = require('form-data');
const fs = require('fs');
const { generateWithFallback } = require('../utils/geminiHelper');

/**
Expand Down Expand Up @@ -100,11 +101,15 @@
* }
*/
const analyzeResume = async (req, res) => {
let uploadedFilePath = null;
try {
if (!req.file) {
return res.status(400).json({ message: "No resume file uploaded" });
}

uploadedFilePath = req.file.path;
const fileBuffer = await fs.promises.readFile(uploadedFilePath);
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed

const targetRole = req.body.targetRole || "General Professional";

// 2. Prompt Engineering
Expand Down Expand Up @@ -133,7 +138,7 @@
prompt,
{
inlineData: {
data: req.file.buffer.toString("base64"),
data: fileBuffer.toString("base64"),
mimeType: "application/pdf"
}
}
Expand Down Expand Up @@ -161,6 +166,10 @@
} catch (error) {
console.error("Resume Analysis Error:", error);
res.status(500).json({ message: "Failed to analyze resume" });
} finally {
if (uploadedFilePath) {
fs.promises.unlink(uploadedFilePath).catch(err => console.error("Failed to delete temp resume PDF:", err));
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
}
}
}

Expand Down
21 changes: 16 additions & 5 deletions backend/middlewares/uploadMiddleware.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const multer = require("multer");
const fs = require("fs");
const path = require("path");
const { fileTypeFromBuffer } = require("file-type");
const { fileTypeFromBuffer, fileTypeFromFile } = require("file-type");

const MAX_FILE_SIZE = 5 * 1024 * 1024;

Expand Down Expand Up @@ -120,7 +120,12 @@ const validateResumeMagicBytes = async (req, res, next) => {
}

try {
const fileType = await fileTypeFromBuffer(req.file.buffer);
let fileType;
if (req.file.buffer) {
fileType = await fileTypeFromBuffer(req.file.buffer);
} else if (req.file.path) {
fileType = await fileTypeFromFile(req.file.path);
}

const allowedMimeTypes = new Set([
'application/pdf',
Expand Down Expand Up @@ -153,9 +158,12 @@ const upload = multer({
limits: { fileSize: MAX_FILE_SIZE },
});

// Upload instance for resumes (memory storage)
// Upload instance for resumes (disk storage)
const uploadResume = multer({
storage: multer.memoryStorage(),
storage: multer.diskStorage({
destination: (req, file, cb) => cb(null, "uploads/"),
filename: (req, file, cb) => cb(null, `${Date.now()}-${sanitizeBase(file.originalname)}.pdf`)
}),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
fileFilter: resumeFileFilter,
limits: { fileSize: MAX_FILE_SIZE },
});
Expand All @@ -164,7 +172,10 @@ const uploadResume = multer({
const NOTES_MAX_FILE_SIZE = 15 * 1024 * 1024;

const uploadNotes = multer({
storage: multer.memoryStorage(),
storage: multer.diskStorage({
destination: (req, file, cb) => cb(null, "uploads/"),
filename: (req, file, cb) => cb(null, `${Date.now()}-${sanitizeBase(file.originalname)}.pdf`)
}),
fileFilter: resumeFileFilter, // PDF-only, same filter as resumes
limits: { fileSize: NOTES_MAX_FILE_SIZE },
});
Expand Down
Loading