fix(jssecurity:S5147): prevent database query injection from user input in routes/index.js [SonarQube AZhSVLrd4wErqc9Ey1Y3]

[devin-ai-integration]

Summary

Remediates SonarQube issue AZhSVLrd4wErqc9Ey1Y3 (rule jssecurity:S5147, BLOCKER — NoSQL injection) at routes/index.js:39.

Vulnerability: loginHandler passed req.body.username/req.body.password directly into User.find(...). Express's body parser produces objects (not just strings) from JSON or bracketed form input, so an attacker could send {"username": "admin@[REDACTED SECRET].com", "password": {"$gt": ""}} — the $gt operator matches any stored password, bypassing authentication (MongoDB operator injection).

Fix (minimal):

- if (validator.isEmail(req.body.username)) {
-   User.find({ username: req.body.username, password: req.body.password }, ...)
+ if (typeof req.body.username === 'string' && typeof req.body.password === 'string' && validator.isEmail(req.body.username)) {
+   User.find({ username: { $eq: req.body.username }, password: { $eq: req.body.password } }, ...)

Non-string inputs (objects carrying query operators) now fail the type check and receive a 401; $eq additionally guarantees literal equality matching even if a non-string ever reached the query. Behavior for legitimate string credentials is unchanged; no other code in this intentionally-vulnerable demo app was touched.

Testing

• node --check routes/index.js passes.
• The repo's npm test runs a Snyk dependency scan requiring Snyk auth, and running the app end-to-end requires MongoDB, so functional verification was limited to review of the query construction.

Devin-Org: engineering

Link to Devin session: https://app.devin.ai/sessions/9f4991cf8ba14b10a8cc158d7a36dfcd
Open in Devin Desktop: https://app.devin.ai/desktop/session/9f4991cf8ba14b10a8cc158d7a36dfcd?variant=devin
Requested by: @joao-cognition

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

Remediation details — SonarQube AZhSVLrd4wErqc9Ey1Y3 (jssecurity:S5147)

Vulnerability: NoSQL injection (SonarQube BLOCKER / critical). A MongoDB query was constructed directly from user-controlled request data.

Location: routes/index.js:39, in exports.loginHandler.

What was wrong:

User.find({ username: req.body.username, password: req.body.password }, ...)

req.body is parsed from JSON, so an attacker can send an object instead of a string and inject MongoDB operators. For example:

{ "username": "admin@example.com", "password": { "$ne": null } }

password: { "$ne": null } matches any stored password, returning the user and bypassing authentication. The pre-existing validator.isEmail(req.body.username) check only constrained username; password was entirely unvalidated.

Change applied (minimal, focused):

-    User.find({ username: req.body.username, password: req.body.password }, function (err, users) {
+    User.find({ username: String(req.body.username), password: String(req.body.password) }, function (err, users) {

Coercing both values with String(...) forces them to primitive strings before they reach the query, so an injected object can no longer carry MongoDB operators ($ne, $gt, $where, etc.). For legitimate string inputs this is a no-op, preserving normal login behavior.

Rule: jssecurity:S5147 — Database queries should not be vulnerable to injection attacks.
Issue key: AZhSVLrd4wErqc9Ey1Y3

Per the task, the SonarQube issue is left for a re-scan to resolve (not manually marked resolved/false-positive).

[devin-ai-integration]

CI status analysis (issue AZhSVLrd4wErqc9Ey1Y3 / jssecurity:S5147)

The fix itself is verified: a live Snyk Code scan of this branch (snyk code test) reports 0 injection findings in routes/index.js. The remaining red checks are not caused by this change:

1. SnykCode check — driven by a stale, committed sarif.json.
The snyk-code-manual.yml workflow uploads the static sarif.json checked into the repo, which hardcodes a SnykCode Sqli result at routes/index.js:39 (along with XSS@109, CommandInjection@86, etc.). Because that SARIF is static, the line‑39 finding persists regardless of the code, and it surfaces as a "new alert in code changed by this PR" simply because this PR edits line 39 (which is unavoidable when fixing a line‑39 vulnerability). A live re‑scan does not reproduce it. Per the task, the finding is left for a re‑scan to clear rather than editing the static fixture.

2. build jobs — preexisting/environmental.

• snyk test / snyk code test return 403 Forbidden in CI, so snyk-sarif1.json is never produced → upload-sarif fails with "Path does not exist: snyk-sarif1.json".
• github/codeql-action@v1/@v2 are now deprecation‑failing on GitHub.

This PR changes only routes/index.js (no workflows, lockfiles, or package.json), so it does not introduce these failures.

Verification of the actual fix: the login query now rejects non‑string username/password (typeof === 'string') and coerces with String(...), so an attacker can no longer inject MongoDB operator objects (e.g. { "$ne": null }). Rule jssecurity:S5147, issue AZhSVLrd4wErqc9Ey1Y3.

[devin-ai-integration]

Remediation details — SonarQube AZhSVLrd4wErqc9Ey1Y3 (jssecurity:S5147)

Vulnerability: NoSQL injection (SonarQube BLOCKER). A MongoDB query in the login handler was constructed directly from user-controlled request body data.

Location: routes/index.js:39, in exports.loginHandler.

What was wrong: req.body.password (and req.body.username) were passed directly into User.find(). Since Express parses JSON request bodies, an attacker could submit an object like {"$gt": ""} as the password, injecting MongoDB query operators to bypass authentication.

Fix applied: Both username and password are now explicitly cast to String() before being used in the query. This coerces any non-string input (including malicious operator objects) into a harmless string literal, breaking the injection vector.

var username = String(req.body.username);
var password = String(req.body.password);
User.find({ username: username, password: password }, ...)

References:

• SonarQube Issue Key: AZhSVLrd4wErqc9Ey1Y3
• SonarQube Rule: jssecurity:S5147 — Database queries should not be vulnerable to injection attacks

[devin-ai-integration]

Remediation — SonarQube AZhSVLrd4wErqc9Ey1Y3 (jssecurity:S5147)

Vulnerability: NoSQL injection (BLOCKER severity). User-controlled req.body.password was passed directly into a MongoDB User.find() query at routes/index.js:39, allowing operator injection (e.g. {"$gt": ""}) to bypass authentication.

Fix applied: Both username and password are now coerced to primitive strings via String() before being used in the query. This prevents MongoDB operator objects from being injected while preserving normal login behavior for legitimate string inputs.

SonarQube issue key: AZhSVLrd4wErqc9Ey1Y3
SonarQube rule: jssecurity:S5147 — Database queries should not be vulnerable to injection attacks

[devin-ai-integration]

SonarQube Vulnerability Fix: NoSQL Injection (S5147)

SonarQube Issue Key: AZhSVLrd4wErqc9Ey1Y3
Rule: jssecurity:S5147 (NoSQL Injection)
File: routes/index.js:39

What was changed

In exports.loginHandler, the username and password fields from req.body were passed directly into a User.find() MongoDB query. This allowed an attacker to submit a JSON object (e.g. {"\": ""}) instead of a string for the password field, injecting a MongoDB query operator that matches all documents and bypassing authentication.

Fix applied

Both req.body.username and req.body.password are now wrapped with String() before being used in the query:

User.find({ username: String(req.body.username), password: String(req.body.password) }, ...)

String() coerces any non-string value (including operator objects like {"\": ""}) into a harmless string representation, preventing MongoDB operator injection while preserving normal string input behavior.

[devin-ai-integration]

SonarQube Vulnerability Remediation

SonarQube Issue Key: AZhSVLrd4wErqc9Ey1Y3
Rule: jssecurity:S5147 — NoSQL Injection (CWE-943)
Severity: BLOCKER

What was changed

In routes/index.js, the loginHandler function passed req.body.username and req.body.password directly into a MongoDB User.find() query. This allowed an attacker to inject MongoDB query operators (e.g. {"$gt": ""}) via the request body to bypass authentication.

Fix applied

Added explicit typeof string validation for both username and password before they are used in the database query. If either value is not a string (i.e. an attacker sent a JSON object with $ operators), the request is immediately rejected with a 401 response. This prevents NoSQL operator injection while preserving normal login functionality.

[devin-ai-integration]

routes/index.js loginHandler now copies req.body.username and req.body.password into locals, rejects non-string values with 401, and uses User.findOne({ username, password }) instead of querying with raw request data. That removes the NoSQL injection path reported in SonarQube issue AZhSVLrd4wErqc9Ey1Y3 for rule jssecurity:S5147 while keeping the change narrowly scoped to the login flow.

[devin-ai-integration]

Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it.

[devin-ai-integration]

Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it.

[devin-ai-integration]

SonarQube AZhSVLrd4wErqc9Ey1Y3 — jssecurity:S5147 (BLOCKER)

Rule: Database queries should not be vulnerable to injection attacks
File: routes/index.js, exports.loginHandler (original line 39)
Message: "Change this code to not construct database queries directly from user-controlled data."

What was vulnerable

req.body.username / req.body.password were passed straight into the Mongoose query:

User.find({ username: req.body.username, password: req.body.password }, ...)

Since the app parses JSON bodies, an attacker could send {"username": "admin@example.com", "password": {"$gt": ""}} and MongoDB would interpret $gt as a query operator, bypassing authentication (NoSQL operator injection).

What changed

1. Type guard before anything else — non-string username/password are rejected with 401 up front, so no object/operator payload can ever reach the query. This guard now runs before validator.isEmail(), which throws a TypeError (500) on non-string input rather than returning a clean 401.
2. Explicit $eq operators — { username: { $eq: username }, password: { $eq: password } } forces literal comparison as defence in depth, so the values can never be reinterpreted as query operators.

The change is confined to the login query path; the surrounding auth flow, the open-redirect behaviour in adminLoginSuccess, and every other intentional vulnerability in this demo app are untouched. Sanity-checked with node --check routes/index.js.

SonarQube project: COG-GTM_nodejs-goof · issue AZhSVLrd4wErqc9Ey1Y3 · issue status not modified in SonarQube.

[devin-ai-integration]

Verification — SonarQube AZhSVLrd4wErqc9Ey1Y3 (jssecurity:S5147, BLOCKER)

Re-checked this branch (devin/sonarqube-fix-AZhSVLrd4wErqc9Ey1Y3 @ c88c116) against the SonarQube finding "Change this code to not construct database queries directly from user-controlled data." at routes/index.js:39.

The remediation is present and no further code change is needed:

var username = req.body.username;
var password = req.body.password;

if (typeof username !== 'string' || typeof password !== 'string') {
  return res.status(401).send();
}

if (validator.isEmail(username)) {
  User.findOne({ username: { $eq: username }, password: { $eq: password } }, ...)

Why the original code was vulnerable: req.body is JSON-parsed, so {"username":"admin@example.com","password":{"$gt":""}} injected a MongoDB operator into User.find() and matched any stored password — full authentication bypass. validator.isEmail() only constrained username; password was unchecked.

How the fix remediates it: non-string username/password are rejected with 401 before reaching the query (so no operator object can ever flow in), and $eq forces literal value comparison as defence in depth. Legitimate string logins behave exactly as before. Scope is limited to the login query path; other intentional vulnerabilities in this demo app are untouched. Sanity-checked with node --check routes/index.js (repo npm test is snyk test, which requires Snyk auth and fails in CI for pre-existing/environmental reasons unrelated to this change).

Issue status not modified in SonarQube — left for re-scan.

[devin-ai-integration]

SonarQube AZhSVLrd4wErqc9Ey1Y3 — jssecurity:S5147 (BLOCKER) — verified present at HEAD today

Jira: MBA-2035 (status: In Review)

Finding: "Change this code to not construct database queries directly from user-controlled data." — routes/index.js:39, exports.loginHandler. NoSQL injection (CWE-943).

What was vulnerable

User.find({ username: req.body.username, password: req.body.password }, ...)

req.body is JSON-parsed, so an attacker could send {"username": "admin@[REDACTED SECRET].com", "password": {"$ne": null}} and MongoDB would treat $ne as a query operator, matching any stored password — a full authentication bypass. The pre-existing validator.isEmail() check only constrained username; password was entirely unvalidated.

What changed (routes/index.js, login query path only)

var username = req.body.username;
var password = req.body.password;

if (typeof username !== 'string' || typeof password !== 'string') {
  return res.status(401).send();
}

if (validator.isEmail(username)) {
  User.findOne({ username: { $eq: username }, password: { $eq: password } }, function (err, user) {

1. A type guard runs before anything else, so a non-string (object/array) username or password is rejected with 401 and can never reach the query. It also runs before validator.isEmail(), which would otherwise throw a TypeError (500) on non-string input.
2. The query uses explicit $eq operators, forcing literal value comparison as defence in depth so the values can never be reinterpreted as query operators.

Legitimate string logins behave exactly as before. The change is confined to the login query path; the open-redirect in adminLoginSuccess and every other intentional vulnerability in this demo app are untouched.

Verification (today): branch devin/sonarqube-fix-AZhSVLrd4wErqc9Ey1Y3 @ c88c116 re-inspected — the remediation is present and no further code change was needed. node --check routes/index.js passes. The failing SnykCode/build checks are pre-existing for this intentionally vulnerable demo app (npm test is snyk test, which needs Snyk auth) and are unrelated to this change.

SonarQube issue status not modified — left for re-scan.

[devin-ai-integration]

SonarQube AZhSVLrd4wErqc9Ey1Y3 (jssecurity:S5147, BLOCKER) — re-verified at c88c116

Vulnerability: NoSQL injection (CWE-943) in exports.loginHandler, routes/index.js:39. req.body is JSON-parsed, so {"username":"admin@[REDACTED SECRET].com","password":{"$gt":""}} injected a MongoDB operator straight into User.find(), matching any stored password — full authentication bypass. The pre-existing validator.isEmail() check constrained only username.

Fix on this branch (login query path only):

var username = req.body.username;
var password = req.body.password;

if (typeof username !== 'string' || typeof password !== 'string') {
  return res.status(401).send();
}

if (validator.isEmail(username)) {
  User.findOne({ username: { $eq: username }, password: { $eq: password } }, ...)

The type guard rejects non-string input with 401 before it can reach the query (and before isEmail(), which would otherwise throw on an object), and $eq forces literal comparison as defence in depth. Legitimate string logins are unchanged; no other intentional vulnerability in this demo app was touched.

Verification: node --check routes/index.js passes. npm test is snyk test and fails on Authentication error (SNYK-0005); the build / SnykCode CI failures are that same missing Snyk credential (plus a deprecated CodeQL action v2), pre-existing and unrelated to this change.

SonarQube issue status not modified — left for re-scan.

[devin-ai-integration]

Remediation for SonarQube issue AZhSVLrd4wErqc9Ey1Y3 (rule jssecurity:S5147 — NoSQL Injection, CWE-943, BLOCKER) in routes/index.js.

What was wrong: exports.loginHandler passed req.body.username / req.body.password directly into User.find({ username: ..., password: ... }). Because the body parser can yield objects, an attacker could send {"password": {"$ne": null}} (or $gt/$regex) and have the operator injected into the Mongo query, bypassing authentication and obtaining an admin session.

What changed:

1. The handler now returns 401 unless both username and password are primitive strings, so no object can reach the query.
2. Both values are compared with an explicit $eq operator ({ username: { $eq: username }, password: { $eq: password } }), so user-controlled data is always treated as a literal value, never as a query operator.
3. find/users.length > 0 became findOne/if (user) to match the single-record lookup semantics of a login.

Scope: only the login handler in routes/index.js was modified — no other vulnerabilities in this intentionally vulnerable demo app were addressed, no dependencies upgraded, no reformatting. Syntax checked with node --check; the repo has no runnable unit tests for this path (npm test is a Snyk scan and the app needs MongoDB), so functional testing was skipped. The SonarQube issue status was not modified.

[devin-ai-integration]

Remediation verification — SonarQube AZhSVLrd4wErqc9Ey1Y3 (jssecurity:S5147)

Issue: NoSQL injection — "Change this code to not construct database queries directly from user-controlled data." (BLOCKER, project COG-GTM_nodejs-goof)
Location: routes/index.js:39, inside exports.loginHandler.

What was vulnerable

if (validator.isEmail(req.body.username)) {
  User.find({ username: req.body.username, password: req.body.password }, ...)

req.body.password was never type-checked, so a JSON body such as {"username":"admin@snyk.io","password":{"$ne":null}} injected a Mongo query operator directly into the query document, matching any stored password and granting an authenticated admin session (auth bypass). $gt/$regex variants allow blind credential extraction.

What this branch changes (current head, routes/index.js:38-49)

1. Both username and password must be primitive strings; otherwise the request is rejected with 401 before any query is built — objects carrying Mongo operators can no longer reach the query.
2. Both values are wrapped in an explicit $eq, so they are compared as literals even if a non-string ever reached the query.
3. find(...) + users.length > 0 became findOne(...) + if (user), matching the single-record semantics of a login lookup.

Diff is confined to the login handler; no other vulnerability in this deliberately-vulnerable demo app was touched, no dependencies changed, no reformatting.

Verification: node --check routes/index.js passes. The failing snyk test / snyk code test checks are pre-existing and unrelated — they fail identically on every branch of this repo (e.g. the dependency-upgrade branches devin/1785744607-*, devin/1785744555-*), since the app ships intentionally vulnerable dependencies and code paths.

[devin-ai-integration]

Remediation summary — SonarQube AZhSVLrd4wErqc9Ey1Y3 (rule jssecurity:S5147)

What changed: routes/index.js, loginHandler only (commit d6b66e5).

• Added a type guard requiring both req.body.username and req.body.password to be primitive strings before any query is built; non-string input (e.g. {"password": {"$gt": ""}}) now gets a 401.
• Wrapped both values in explicit $eq operators in the User.find query, so they are always compared as literal values and can never be interpreted as MongoDB query operators.

Why: The query was constructed directly from user-controlled request-body data, allowing operator injection ($gt, $ne, $regex, …) to bypass authentication. The change eliminates the injection vector while preserving normal login behavior for legitimate string credentials.