fix: validate country field with valid Adzuna country codes (Issue #1301) - #1302
fix: validate country field with valid Adzuna country codes (Issue #1301)#1302saidai-bhuvanesh wants to merge 1 commit into
Conversation
…nopus-Labs#1301) - Add VALID_ADZUNA_COUNTRIES list with supported country codes - Add isValidCountry() helper function - Validate country parameter in getJobs endpoint - Return 400 error with valid codes for invalid input - Normalize country code to lowercase for consistency
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| "ca", // Canada | ||
| "de", // Germany | ||
| "es", // Spain | ||
| "fr", // France | ||
| "gb", // United Kingdom | ||
| "in", // India | ||
| "it", // Italy | ||
| "mx", // Mexico |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with 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.
Inline comments:
In `@backend/controllers/jobController.js`:
- Around line 83-90: Move countryParam extraction and the isValidCountry
validation block in the job controller to the start of the request flow, before
the disabled-configuration response and before Session.findOne(). Preserve the
existing HTTP 400 response and valid-country behavior.
- Around line 83-92: Update refreshJobCache to normalize the resolved country
once, then reuse that lowercase value for both the cache key and
fetchFromAdzuna(). Ensure the default ADZUNA_COUNTRY follows the same
normalization as the country used by getJobs, including when it is configured as
uppercase.
- Around line 33-35: The isValidCountry helper currently lowercases non-string
values and can throw for array query parameters. Add a typeof country ===
"string" guard before calling toLowerCase, preserving validation for valid
strings, and add a regression test through getJobs that verifies repeated
country parameters return HTTP 400 rather than 500.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 93d03358-9973-4ead-b6d2-050360d895ef
📒 Files selected for processing (1)
backend/controllers/jobController.js
| const isValidCountry = (country) => { | ||
| return VALID_ADZUNA_COUNTRIES.includes(country.toLowerCase()); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- controller excerpt ---'
cat -n backend/controllers/jobController.js | sed -n '1,120p'
printf '%s\n' '--- Express dependency/config references ---'
rg -n '"express"|express\(|query parser|queryParser|req\.query|isValidCountry|VALID_ADZUNA_COUNTRIES' \
package.json package-lock.json yarn.lock pnpm-lock.yaml backend 2>/dev/null | head -250
printf '%s\n' '--- relevant tracked files ---'
git ls-files | rg '(^|/)(package(-lock)?\.json|yarn\.lock|pnpm-lock\.yaml|jobController\.js|app|server|routes?)'Repository: Canopus-Labs/PrepPilot
Length of output: 8341
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- server configuration ---'
cat -n backend/server.js | sed -n '1,100p'
printf '%s\n' '--- job routes ---'
cat -n backend/routes/jobRoutes.js | sed -n '1,100p'
printf '%s\n' '--- parser configuration and job endpoint tests ---'
rg -n -i 'query\s*parser|set\(["'\'']query|job|country' backend --glob '!package-lock.json' | head -250
printf '%s\n' '--- standalone Node querystring behavior ---'
node - <<'JS'
const querystring = require('node:querystring');
for (const input of [
'country=us',
'country=us&country=ca',
'country[]=us&country[]=ca',
'country=',
]) {
const parsed = querystring.parse(input);
const country = parsed.country;
let result;
try {
result = {
parsed,
typeofCountry: typeof country,
isArray: Array.isArray(country),
lowercased: country.toLowerCase(),
};
} catch (error) {
result = {
parsed,
typeofCountry: typeof country,
isArray: Array.isArray(country),
error: error.message,
};
}
console.log(input, JSON.stringify(result));
}
JSRepository: Canopus-Labs/PrepPilot
Length of output: 10803
🌐 Web query:
Express 5.1 application query parser default simple repeated query parameters arrays official documentation
💡 Result:
In Express 5.1, the default setting for the query parser application setting is 'simple' [1][2]. This parser uses the Node.js built-in querystring module rather than the qs library that was used as the default in previous versions [1][2]. Because of this change, the 'simple' parser does not automatically parse repeated query parameters or bracketed parameters (e.g., items[]) into arrays [3][4]. If you require array parsing for repeated query parameters, you must change the query parser setting [1]. To restore behavior similar to Express 4 (where qs handles array parsing), you can set the parser to 'extended' [4][2]: app.set('query parser', 'extended'); The 'extended' parser uses the qs library, which automatically parses repeated query parameters and bracketed syntax into arrays [4][5]. Note that when using the 'extended' parser, there is a default arrayLimit of 1000, which ensures that repeated keys are correctly returned as arrays rather than objects for up to 1000 items [6][5]. If you need further customization (such as specific qs options), you can also provide a custom function to the query parser setting [7]: const qs = require('qs'); app.set('query parser', (str) => qs.parse(str, { /* custom options */ }));
Citations:
- 1: https://expressjs.com/en/guide/migrating-5/
- 2: https://github.com/expressjs/express/blob/5.x/History.md
- 3:
item[]format is parsed differently in express v5 compared to v4 expressjs/express#5060 - 4: In Express 5, how to make a query param parsed as an array when it is an array of string of one element ? expressjs/express#6207
- 5: expressjs/express@8d09bfe
- 6: fix: keep repeated extended query params as arrays beyond 20 values expressjs/express#7151
- 7: https://expressjs.com/en/5x/api/request/
Reject non-string country values before lowercasing.
With Express 5.1’s default parser, ?country=us&country=ca produces an array. .toLowerCase() then throws, and getJobs returns HTTP 500 instead of HTTP 400. Add a typeof country === "string" guard and a regression test.
🤖 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/jobController.js` around lines 33 - 35, The
isValidCountry helper currently lowercases non-string values and can throw for
array query parameters. Add a typeof country === "string" guard before calling
toLowerCase, preserving validation for valid strings, and add a regression test
through getJobs that verifies repeated country parameters return HTTP 400 rather
than 500.
| const countryParam = req.query.country || ADZUNA_COUNTRY; | ||
|
|
||
| // Validate country parameter (Issue #1301) | ||
| if (!isValidCountry(countryParam)) { | ||
| return res.status(400).json({ | ||
| message: `Invalid country code. Valid codes are: ${VALID_ADZUNA_COUNTRIES.join(", ")}`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate country before configuration and database work.
The validation block runs after the disabled-configuration response and after Session.findOne(). An unsupported country therefore receives HTTP 200 when Adzuna is disabled, and it can receive HTTP 500 if the session lookup fails. This conflicts with the stated contract that invalid codes return HTTP 400.
Move country extraction and validation before both operations.
🤖 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/jobController.js` around lines 83 - 90, Move countryParam
extraction and the isValidCountry validation block in the job controller to the
start of the request flow, before the disabled-configuration response and before
Session.findOne(). Preserve the existing HTTP 400 response and valid-country
behavior.
| const countryParam = req.query.country || ADZUNA_COUNTRY; | ||
|
|
||
| // Validate country parameter (Issue #1301) | ||
| if (!isValidCountry(countryParam)) { | ||
| return res.status(400).json({ | ||
| message: `Invalid country code. Valid codes are: ${VALID_ADZUNA_COUNTRIES.join(", ")}`, | ||
| }); | ||
| } | ||
|
|
||
| const country = countryParam.toLowerCase(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Use the normalized default country in refreshJobCache.
If ADZUNA_COUNTRY=US, getJobs uses a role|us cache key, but refreshJobCache at Lines 123-124 uses role|US. The scheduled refresh then writes a cache entry that getJobs does not read. Normalize the default once and pass that value to both the cache key and fetchFromAdzuna().
🤖 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/jobController.js` around lines 83 - 92, Update
refreshJobCache to normalize the resolved country once, then reuse that
lowercase value for both the cache key and fetchFromAdzuna(). Ensure the default
ADZUNA_COUNTRY follows the same normalization as the country used by getJobs,
including when it is configured as uppercase.
|
@saidai-bhuvanesh CodeQL detected a potential SSRF (Server-Side Request Forgery) vulnerability because the request URL depends on a user-controlled value (country) . please validate the country input using a whitelist of allowed values (e.g- in, us, gb) before constructing the URL. this prevents invalid or malicious input from being used in outgoing requests. Let me know once it's fixed |
|
@saidai-bhuvanesh Don't create separate PR for changes, commit the changes in this PR |
4 participants
Description
Fixes the issue where the country field accepts any input (including numeric values) instead of validating against valid country codes.
Changes
backend/controllers/jobController.js
Summary
countryquery parameter ingetJobs.Ready to merge.