Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
34 changes: 4 additions & 30 deletions backend/src/routes/resume.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { scrapeLinkedInProfile, profileToResumeText } from '../services/linkedinImporter.js';
import { fetchGitHubProfile, convertGitHubToResumeText } from '../services/githubImporter.js';
import { getDefaultProvider } from '../config/aiProviders.js';
import { analyzeResume } from '../services/resumeService.js';

const router = express.Router();

Expand Down Expand Up @@ -585,38 +586,11 @@ router.post('/score', asyncHandler(async (req, res) => {
});
}

const analysisResult = await analyzeResume(resumeText);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: This line enables a paid AI call in an endpoint that is not protected by authentication or AI rate limiting, unlike other AI routes. That allows anonymous abuse and uncontrolled provider-cost consumption. Add the same auth/rate-limit middleware chain used by other AI endpoints before invoking the analysis. [security]

Severity Level: Critical 🚨
❌ /api/resumes/score allows anonymous AI-powered resume scoring.
❌ AI provider billed for unlimited anonymous resume scoring.
⚠️ No aiRateLimiter protects resume scoring request volume.
Steps of Reproduction ✅
1. The main Express app mounts the resume router at `/api/resumes` in
`backend/src/index.js:282-284` (`app.use('/api/resumes', resumeRoutes);`), exposing all
routes in `backend/src/routes/resume.js` publicly unless protected per-route.

2. In `backend/src/routes/resume.js:579-586`, the `/score` endpoint is defined as
`router.post('/score', asyncHandler(async (req, res) => { ... }))` without `verifyToken`,
`extractAIProvider`, or `aiRateLimiter` middleware in its chain, unlike other AI-heavy
routes.

3. Inside that handler, line `589 const analysisResult = await analyzeResume(resumeText);`
calls `analyzeResume()` from `backend/src/services/resumeService.js:9-92`, which in turn
obtains a provider via `getDefaultProvider()` (line 15) and invokes
`provider.generateContent(prompt)` (line 61), making an external, billable AI request on
every call.

4. Compare this with other AI endpoints: `backend/src/routes/enhance.js:8-15` and `:49-55`
wrap AI calls with `verifyToken`, `extractAIProvider`, and `aiRateLimiter`, and
`backend/src/routes/roast.js:15-21` similarly protects `POST /api/roast`. Project
visualizer chat endpoints in `backend/src/routes/projectVisualizer.route.js:194-199,
210-217, 234-241, 253-259, 271-279, 287-290` all use `verifyToken` and `aiRateLimiter`.
The resume scoring route lacks both authentication and `aiRateLimiter`, so repeated
anonymous `POST /api/resumes/score` requests (e.g. via curl or a script) will hit
`provider.generateContent` with no auth or AI-specific rate limiting, allowing
uncontrolled external AI cost consumption.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** backend/src/routes/resume.js
**Line:** 589:589
**Comment:**
	*Security: This line enables a paid AI call in an endpoint that is not protected by authentication or AI rate limiting, unlike other AI routes. That allows anonymous abuse and uncontrolled provider-cost consumption. Add the same auth/rate-limit middleware chain used by other AI endpoints before invoking the analysis.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


res.json({
success: true,
data: {
overallScore: 82,
sections: {
summary: {
score: 80,
feedback: "Good professional summary"
},
skills: {
score: 85,
feedback: "Skills are relevant"
},
experience: {
score: 78,
feedback: "Add more quantified achievements"
},
education: {
score: 88,
feedback: "Education section is clear"
},
projects: {
score: 79,
feedback: "Projects need more impact metrics"
}
},
topSuggestions: [
"Add measurable achievements",
"Improve project descriptions",
"Use stronger action verbs"
]
}
data: analysisResult
});
}));

Expand Down
109 changes: 109 additions & 0 deletions backend/src/services/resumeService.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { getDefaultProvider } from '../config/aiProviders.js';
import { ApiError } from '../middleware/errorHandler.js';

/**
* Analyzes a resume using AI and returns a structured score with feedback
* @param {string} resumeText - The resume text to analyze
* @returns {Promise<Object>} - The analysis result with scores and feedback
*/
export async function analyzeResume(resumeText) {
if (!resumeText || !resumeText.trim()) {
throw new ApiError(400, 'Resume text is required');
}
Comment on lines +10 to +12

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The input guard assumes the value is a string and calls .trim() directly. If a non-string value is passed, this throws a TypeError before structured validation, causing an unintended 500 response instead of a 400 validation error. Validate type before calling string methods. [type error]

Severity Level: Major ⚠️
⚠️ Non-string payloads crash /api/resumes/score handler.
⚠️ Clients receive 500 instead of proper 400 validation error.
Steps of Reproduction ✅
1. The resume scoring API is mounted at `/api/resumes` in `backend/src/index.js:284` and
defines `POST /api/resumes/score` in `backend/src/routes/resume.js:579-590`. This handler
reads `const { resumeText } = req.body;` (line 580) from arbitrary JSON input, since no
`validate()` schema middleware is applied to this route.

2. Immediately after, the route performs `if (!resumeText || !resumeText.trim()) { ... }`
at lines 582-586 in `backend/src/routes/resume.js`. If a client sends a non-string value,
e.g. `{ "resumeText": 123 }`, `!resumeText` is `false`, but `resumeText.trim` is
`undefined` and calling `resumeText.trim()` throws a `TypeError` before any structured
validation or service call.

3. The route is wrapped with `asyncHandler` (line 579), which forwards the thrown
`TypeError` to the global `errorHandler` defined in
`backend/src/middleware/errorHandler.js:12-92`. Because the error is neither an `ApiError`
nor a known Firebase/validation error, it falls through to the default branch at lines
82-87, returning HTTP 500 with an unexpected error message instead of a 400 validation
error.

4. Even if the route-level guard were fixed, `analyzeResume()` in
`backend/src/services/resumeService.js:9-12` repeats the same unchecked pattern `if
(!resumeText || !resumeText.trim()) { throw new ApiError(400, 'Resume text is required');
}`, so any other direct caller passing a non-string would again trigger a `TypeError`
rather than the intended 400 `ApiError` response, confirming the issue in the service
itself.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** backend/src/services/resumeService.js
**Line:** 10:12
**Comment:**
	*Type Error: The input guard assumes the value is a string and calls `.trim()` directly. If a non-string value is passed, this throws a TypeError before structured validation, causing an unintended 500 response instead of a 400 validation error. Validate type before calling string methods.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


try {
const provider = getDefaultProvider();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: This always forces the global default provider and ignores per-request AI provider/key selection used elsewhere via middleware, so BYOK/request-specific configuration cannot work for this flow. Accept an injected provider (or request context) instead of hardwiring default provider resolution in the service. [api mismatch]

Severity Level: Major ⚠️
⚠️ /api/resumes/score ignores X-AI-* BYOK provider headers.
⚠️ Resume scoring always uses server default AI provider.
Steps of Reproduction ✅
1. The project centralizes AI provider selection in
`backend/src/middleware/aiKey.js:18-69`, where `extractAIProvider` reads `X-AI-Provider`,
`X-AI-Key`, and related headers, attaches a provider instance to `req.aiProvider`, and
records `req.aiProviderSource` (e.g. user-supplied key vs server env).

2. All resume enhancement-related routes in `backend/src/routes/enhance.js:8-15, 49-55,
81-88, 111-118, 139-147, 167-175, 195-203` use the middleware chain `verifyToken,
extractAIProvider, aiRateLimiter` and pass `req.aiProvider` into the langchain helpers
(`enhanceResume`, `generateSummary`, `analyzeATSScore`, etc.), which in turn call
`resolveProvider(aiProvider)` in `backend/src/config/langchain.js:12`, choosing the
per-request provider when present and falling back to `getDefaultProvider()` otherwise.

3. By contrast, the new resume scoring flow uses `analyzeResume(resumeText)` from
`backend/src/services/resumeService.js:9-92`, and the route `POST /api/resumes/score` at
`backend/src/routes/resume.js:579-590` does not include `extractAIProvider` or any
ai-provider parameter; it simply calls the service with the text.

4. Inside `analyzeResume()`, line 15 hardwires `const provider = getDefaultProvider();`,
and line 61 calls `provider.generateContent(prompt)` using that global instance. As a
result, even if a client sends `X-AI-Provider` or `X-AI-Key` headers (which are honored on
`/api/enhance` and `/api/roast`), the `/api/resumes/score` flow cannot use them: it never
consults `req.aiProvider`, and the service ignores any injected provider, breaking the
established per-request AI configuration pattern for this feature.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** backend/src/services/resumeService.js
**Line:** 15:15
**Comment:**
	*Api Mismatch: This always forces the global default provider and ignores per-request AI provider/key selection used elsewhere via middleware, so BYOK/request-specific configuration cannot work for this flow. Accept an injected provider (or request context) instead of hardwiring default provider resolution in the service.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


const prompt = `You are an expert resume analyst. Analyze the following resume and provide a detailed evaluation.

Return ONLY a valid JSON object with this exact structure:
{
"overallScore": number (0-100),
"sections": {
"summary": {
"score": number (0-100),
"feedback": "string (brief feedback)"
},
"skills": {
"score": number (0-100),
"feedback": "string (brief feedback)"
},
"experience": {
"score": number (0-100),
"feedback": "string (brief feedback)"
},
"education": {
"score": number (0-100),
"feedback": "string (brief feedback)"
},
"projects": {
"score": number (0-100),
"feedback": "string (brief feedback)"
}
},
"topSuggestions": [
"string (actionable suggestion)",
"string (actionable suggestion)",
"string (actionable suggestion)"
]
}

Scoring criteria:
- Overall score should reflect the resume's overall quality
- Each section should be scored independently (0-100)
- Feedback should be specific and actionable
- Top suggestions should be the most important improvements needed
- Be honest but constructive in your assessment

Resume text:
${resumeText}`;

const result = await provider.generateContent(prompt);
Comment on lines +15 to +61

Copy link
Copy Markdown
Contributor

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect generateContent implementations for timeout handling
ast-grep run --pattern 'generateContent($$$) { $$$ }' --lang javascript backend/src/config/aiProviders.js
rg -n -A10 'generateContent' backend/src/config/aiProviders.js

Repository: anurag3407/career-pilot

Length of output: 4200


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== backend/src/config/aiProviders.js (relevant sections) =="
nl -ba backend/src/config/aiProviders.js | sed -n '1,230p'

echo
echo "== timeout / abort / race search across backend/src =="
rg -n "Promise\.race|AbortController|timeout|timed out|setTimeout\(" backend/src || true

echo
echo "== resume service call site =="
nl -ba backend/src/services/resumeService.js | sed -n '1,120p'

Repository: anurag3407/career-pilot

Length of output: 261


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== backend/src/config/aiProviders.js =="
sed -n '1,240p' backend/src/config/aiProviders.js

echo
echo "== timeout/abort/race references in backend/src =="
rg -n "Promise\.race|AbortController|setTimeout\(|timeout|timed out" backend/src || true

echo
echo "== backend/src/services/resumeService.js =="
sed -n '1,140p' backend/src/services/resumeService.js

Repository: anurag3407/career-pilot

Length of output: 18960


Add a timeout around provider.generateContent()

backend/src/services/resumeService.js awaits the AI call directly, so a slow or unresponsive upstream can tie up this unauthenticated request and exhaust connections under load. Wrap it with a timeout/abort guard before parsing the response.

🤖 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/services/resumeService.js` around lines 15 - 61, The
`provider.generateContent()` call in `resumeService` is awaited directly, so a
slow upstream can hang the request and consume resources. Wrap the
`generateContent` call in a timeout or abortable promise guard inside the resume
analysis flow, and make sure the timeout path returns a controlled error before
any response parsing continues.


// Strip markdown code blocks if present
let jsonText = result.text.trim();
if (jsonText.startsWith('```')) {
jsonText = jsonText.replace(/^```(?:json)?\n?/, '').replace(/\n?```$/, '').trim();
}

// Parse the JSON response
const analysisData = JSON.parse(jsonText);

// Validate the structure
if (!analysisData.overallScore || typeof analysisData.overallScore !== 'number') {
throw new Error('Invalid response: missing or invalid overallScore');
}
Comment on lines +73 to +75

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Falsy check rejects a legitimate overallScore of 0.

!analysisData.overallScore treats 0 as invalid even though it's within the documented 0-100 range, forcing a valid AI response to fail validation and fall back to a generic 500 error.

🐛 Proposed fix
-    if (!analysisData.overallScore || typeof analysisData.overallScore !== 'number') {
+    if (typeof analysisData.overallScore !== 'number' || Number.isNaN(analysisData.overallScore)) {
       throw new Error('Invalid response: missing or invalid overallScore');
     }
📝 Committable suggestion

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

Suggested change
if (!analysisData.overallScore || typeof analysisData.overallScore !== 'number') {
throw new Error('Invalid response: missing or invalid overallScore');
}
if (typeof analysisData.overallScore !== 'number' || Number.isNaN(analysisData.overallScore)) {
throw new Error('Invalid response: missing or invalid overallScore');
}
🤖 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/services/resumeService.js` around lines 73 - 75, The validation
in resumeService’s analysisData.overallScore check is rejecting a valid score of
0 because it uses a falsy test; update the guard to only reject null/undefined
or non-number values, while still allowing 0 through. Keep the fix scoped to the
overallScore validation in the resumeService logic that throws the “Invalid
response” error, so a legitimate AI result in the 0–100 range no longer falls
back to a generic 500.

Comment on lines +73 to +75

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The numeric validation incorrectly treats zero as missing. Because the condition uses a falsy check before the type check, a valid score of 0 will always be rejected and converted into a 500 error. Check for number type first and then validate numeric range explicitly. [falsy zero check]

Severity Level: Major ⚠️
⚠️ /api/resumes/score fails on extremely weak resumes.
⚠️ Users receive 500 instead of clear scoring response.
Steps of Reproduction ✅
1. The resume scoring endpoint `POST /api/resumes/score` in
`backend/src/routes/resume.js:579-590` reads `resumeText` from the request body and calls
`const analysisResult = await analyzeResume(resumeText);` (line 589).

2. `analyzeResume()` in `backend/src/services/resumeService.js:9-92` constructs a prompt
(lines 17-59) that explicitly instructs the AI to return `"overallScore": number (0-100)`
(lines 20-22), meaning 0 is a valid score for a very weak resume.

3. The AI response is parsed into `analysisData` at line 70: `const analysisData =
JSON.parse(jsonText);`. Immediately after, validation at lines 73-75 uses `if
(!analysisData.overallScore || typeof analysisData.overallScore !== 'number') { throw new
Error('Invalid response: missing or invalid overallScore'); }`.

4. When the AI legitimately returns `overallScore: 0`, `!analysisData.overallScore`
evaluates to `true` (because `!0` is `true`), so the condition is satisfied regardless of
the correct `typeof` check. This throws a generic `Error`, which is caught by the `catch`
block at lines 93-107 and, since it is neither a `SyntaxError` nor an `ApiError`, is
converted into `new ApiError(500, 'Failed to analyze resume')`. The `/api/resumes/score`
endpoint then returns HTTP 500 for valid AI output representing a zero score.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** backend/src/services/resumeService.js
**Line:** 73:75
**Comment:**
	*Falsy Zero Check: The numeric validation incorrectly treats zero as missing. Because the condition uses a falsy check before the type check, a valid score of 0 will always be rejected and converted into a 500 error. Check for number type first and then validate numeric range explicitly.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


if (!analysisData.sections || typeof analysisData.sections !== 'object') {
throw new Error('Invalid response: missing or invalid sections');
}

const requiredSections = ['summary', 'skills', 'experience', 'education', 'projects'];
for (const section of requiredSections) {
if (!analysisData.sections[section] || typeof analysisData.sections[section].score !== 'number') {
throw new Error(`Invalid response: missing or invalid ${section} section`);
}
}

if (!analysisData.topSuggestions || !Array.isArray(analysisData.topSuggestions)) {
throw new Error('Invalid response: missing or invalid topSuggestions');
}

return analysisData;
} catch (error) {
console.error('Error analyzing resume:', error);

// If JSON parsing failed, it's likely the AI didn't return valid JSON
if (error instanceof SyntaxError) {
throw new ApiError(500, 'Failed to analyze resume: AI returned invalid JSON format');
}

// Re-throw ApiError instances
if (error instanceof ApiError) {
throw error;
}

// Generic error
throw new ApiError(500, 'Failed to analyze resume');
}
}