feat: add user portal application#2
Conversation
Node/Express application for the internal user portal. Covers account management, session handling, and activity logging. Part of the Q2 onboarding tooling initiative.
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (11)
📒 Files selected for processing (64)
📝 WalkthroughWalkthroughThis PR introduces the complete OWASP NodeGoat example application—a security training platform built with Express.js, MongoDB, and server-side templating. The codebase demonstrates secure and vulnerable patterns through an investment management interface with authentication, user profiles, and educational security tutorials. ChangesNodeGoat Full-Stack Implementation
🎯 4 (Complex) | ⏱️ ~60 minutes
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the OWASP NodeGoat project, including various database DAOs, Express routes, and frontend assets. Several critical security vulnerabilities were identified in the added code: a NoSQL and Server-Side JavaScript Injection (SSJS) vulnerability in allocations-dao.js due to unsafe $where query usage; plaintext password storage and direct comparison in user-dao.js; a Remote Code Execution (RCE) risk in contributions.js from using eval() on user input; and a Regular Expression Denial of Service (ReDoS) vulnerability in profile.js caused by nested quantifiers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| return { | ||
| $where: `this.userId == ${parsedUserId} && this.stocks > '${threshold}'` | ||
| }; |
There was a problem hiding this comment.
The use of $where with unvalidated string interpolation of the threshold parameter is highly vulnerable to NoSQL Injection and Server-Side JavaScript Injection (SSJS). An attacker can inject arbitrary JavaScript code to execute on the database server, potentially causing a Denial of Service (DoS) or unauthorized data access. Avoid using $where entirely and use standard MongoDB query operators.
| return { | |
| $where: `this.userId == ${parsedUserId} && this.stocks > '${threshold}'` | |
| }; | |
| const parsedThreshold = parseInt(threshold, 10); | |
| return { | |
| userId: parsedUserId, | |
| stocks: { $gt: isNaN(parsedThreshold) ? 0 : parsedThreshold } | |
| }; |
| password //received from request param | ||
| /* | ||
| // Fix for A2-1 - Broken Auth | ||
| // Stores password in a safer way using one way encryption and salt hashing | ||
| password: bcrypt.hashSync(password, bcrypt.genSaltSync()) | ||
| */ |
There was a problem hiding this comment.
Storing passwords in plain text is a critical security vulnerability. Passwords must be hashed using a strong, one-way cryptographic hashing algorithm such as bcrypt before being stored in the database.
| password //received from request param | |
| /* | |
| // Fix for A2-1 - Broken Auth | |
| // Stores password in a safer way using one way encryption and salt hashing | |
| password: bcrypt.hashSync(password, bcrypt.genSaltSync()) | |
| */ | |
| password: bcrypt.hashSync(password, bcrypt.genSaltSync()) |
| const comparePassword = (fromDB, fromUser) => { | ||
| return fromDB === fromUser; | ||
| /* | ||
| // Fix for A2-Broken Auth | ||
| // compares decrypted password stored in this.addUser() | ||
| return bcrypt.compareSync(fromDB, fromUser); | ||
| */ | ||
| }; |
There was a problem hiding this comment.
Comparing plain text passwords directly is insecure. Use bcrypt.compareSync to securely compare the user-supplied password with the hashed password stored in the database.
const comparePassword = (fromDB, fromUser) => {
return bcrypt.compareSync(fromUser, fromDB);
};| const preTax = eval(req.body.preTax); | ||
| const afterTax = eval(req.body.afterTax); | ||
| const roth = eval(req.body.roth); |
There was a problem hiding this comment.
Using eval() to parse user input is extremely dangerous and leads to Remote Code Execution (RCE) or Server-Side JavaScript Injection (SSJS). Parse the input values safely using parseInt instead.
| const preTax = eval(req.body.preTax); | |
| const afterTax = eval(req.body.afterTax); | |
| const roth = eval(req.body.roth); | |
| const preTax = parseInt(req.body.preTax, 10); | |
| const afterTax = parseInt(req.body.afterTax, 10); | |
| const roth = parseInt(req.body.roth, 10); |
| // -- | ||
| // The Fix: Instead of using greedy quantifiers the same regex will work if we omit the second quantifier + | ||
| // const regexPattern = /([0-9]+)\#/; | ||
| const regexPattern = /([0-9]+)+\#/; |
There was a problem hiding this comment.
The regular expression /([0-9]+)+\#/ contains nested quantifiers ([0-9]+)+ which makes it vulnerable to Regular Expression Denial of Service (ReDoS) via catastrophic backtracking. An attacker can supply a long sequence of digits without the trailing # to hang the single-threaded Node.js event loop. Simplify the regex to avoid nested quantifiers.
| const regexPattern = /([0-9]+)+\#/; | |
| const regexPattern = /([0-9]+)\"/; |
![battleops[bot]](https://avatars.githubusercontent.com/in/3522402?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ BattleTest Security Review
🚨 REQUEST CHANGES — Risk Score: 100/100 · 14 blocking · 5 advisory · 3 informational (🔴 6 · 🟠 8 · 🟡 5 · 🔵 0 · ⚪ 3)
This PR introduces the OWASP NodeGoat vulnerable-by-design application as a new example, adding over 27,000 lines of code including multiple critical and high-severity vulnerabilities such as server-side JavaScript injection, SSRF, NoSQL injection, and known CVEs in dependencies. The overall security posture is extremely poor, as the codebase is intentionally insecure for training purposes but is being merged into a production repository without any compensating controls or warnings. The single most important issue is the presence of a critical code injection vulnerability via eval() in the Contributions Handler, which allows full remote server compromise.
↳ Top 4 findings posted as inline comments on the diff below.
Important
Actions required outside the codebase:
- Fixed versions are available for at least one vulnerable dependency. Bump to the versions shown in the inline comments, then re-trigger this review.
**18 more findings** (full detail in the [report](https://battletest.dev/reports/3a473cce-1617-457b-8f59-2cba069c0faf)):
- 🟠 High dependencies — CVE-2026-6321 in fast-uri (CVE-2026-6321)
- 🟠 High dependencies — CVE-2026-6322 in fast-uri (CVE-2026-6322)
- 🟡 Medium dependencies — CVE-2026-47676 in hono (CVE-2026-47676)
- 🟡 Medium dependencies — CVE-2026-47675 in hono (CVE-2026-47675)
- 🟡 Medium dependencies — CVE-2026-44455 in hono (CVE-2026-44455)
- 🔴 Critical Code Injection — Server-Side JavaScript Injection via eval() in Contributions Handler —
examples/nodegoat/app/routes/contributions.js:27 - 🔴 Critical SSRF — Server-Side Request Forgery (SSRF) in Research Endpoint —
examples/nodegoat/app/routes/research.js:12 - 🔴 Critical NoSQL Injection — NoSQL Injection via $where Operator in AllocationsDAO —
examples/nodegoat/app/data/allocations-dao.js:56 - 🟠 High IDOR — Insecure Direct Object Reference (IDOR) in Allocations Endpoint —
examples/nodegoat/app/routes/allocations.js:16 - 🟠 High Open Redirect — Open Redirect via Unvalidated url Query Parameter —
examples/nodegoat/app/routes/index.js:56 - 🟠 High Stored XSS — Stored Cross-Site Scripting (XSS) via Markdown Rendering in Memos —
examples/nodegoat/app/views/memos.html:26 - 🟠 High Broken Authentication — Plaintext Password Storage and Weak Password Validation —
examples/nodegoat/app/data/user-dao.js:62 - 🔴 Critical Security Misconfiguration — Swig Template Engine Has autoescape Disabled — All Template Variables Rendered Without HTML Escaping —
examples/nodegoat/server.js:100 - ⚪ Info XSS via Unsafe Script Injection — environmentalScripts Array Rendered Directly Into Page Without Escaping —
examples/nodegoat/app/views/layout.html:80 - 🔴 Critical attack-chain — NoSQL Injection + IDOR → Global Data Leak of All User Allocations —
examples/nodegoat/app/data/allocations-dao.js:56 - 🔴 Critical attack-chain — Stored XSS + autoescape=off + Bootstrap DOM XSS → Persistent Session Hijacking —
examples/nodegoat/app/views/memos.html:26 - 🟠 High attack-chain — Open Redirect + Stored XSS + Bootstrap DOM XSS → Phishing with Session Theft —
examples/nodegoat/app/routes/index.js:56 - 🟠 High attack-chain — NoSQL Injection + IDOR + Plaintext Passwords → Full Account Takeover Chain —
examples/nodegoat/app/data/allocations-dao.js:56
Vulnerable dependencies:
- 🟠 CVE-2026-6321 —
fast-uri@3.1.0→ fix:3.1.1· CWE-22 · EPSS 0.1% - 🟠 CVE-2026-6322 —
fast-uri@3.1.0→ fix:3.1.2· CWE-436 · EPSS 0.0% - 🟡 CVE-2026-47676 —
hono@4.12.15→ fix:4.12.21· CWE-444 CWE-693 · EPSS 0.1% - 🟡 CVE-2026-47675 —
hono@4.12.15→ fix:4.12.21· CWE-113 CWE-1287 · EPSS 0.1% - 🟡 CVE-2026-44455 —
hono@4.12.15→ fix:4.12.16· CWE-74 · EPSS 0.0% - 🟡 CVE-2026-44456 —
hono@4.12.15→ fix:4.12.16· CWE-400 · EPSS 0.0% - 🟡 CVE-2026-47673 —
hono@4.12.15→ fix:4.12.21· CWE-285 · EPSS 0.0% - 🟡 CVE-2026-44459 —
hono@4.12.15→ fix:4.12.18· CWE-1284 · EPSS 0.0% - 🟡 CVE-2026-44457 —
hono@4.12.15→ fix:4.12.18· CWE-524 · EPSS 0.0% - 🟡 CVE-2026-44458 —
hono@4.12.15→ fix:4.12.18· CWE-74 CWE-116 · EPSS 0.0% - 🟡 CVE-2026-47674 —
hono@4.12.15→ fix:4.12.21· CWE-185 CWE-1289 · EPSS 0.1%
Recommendations:
- Add a
npm auditoryarn auditstep to the CI pipeline (e.g., in.github/workflows/ci.yml) that fails the build on any high or critical severity vulnerability. This would have caught the fast-uri and hono CVEs before they reached production. - Introduce a centralized input validation middleware (e.g., using
express-validatororjoi) insrc/middleware/validate.jsthat enforces type coercion (e.g.,parseIntfor numeric fields) and allowlist-based validation for all user-supplied parameters. This would prevent the eval injection, NoSQL injection, and SSRF patterns from recurring across different handlers. - Create a shared allowlist configuration file (e.g.,
src/config/allowlists.js) that defines permitted stock symbols, redirect URLs, and other user-controlled values. Reference this file in the research endpoint, open redirect handler, and any future endpoints to centralize validation logic and reduce the risk of bypasses. - Add a lint rule (e.g., in
.eslintrc.json) to disallow the use ofeval()andnew Function()entirely, with a custom ESLint plugin likeeslint-plugin-no-evalor the built-inno-evalrule. This would have prevented the critical code injection finding at the code review stage. - Implement a dependency update policy that requires all direct and transitive dependencies to be updated within 7 days of a CVE publication, enforced by a scheduled GitHub Action (e.g.,
dependabot.ymlwithopen-pull-requests-limit: 10andtarget-branch: main). This would have addressed the fast-uri and hono vulnerabilities proactively.
View full report for the complete breakdown, PoC reproductions, and history.
| @@ -0,0 +1,11 @@ | |||
| /*! | |||
There was a problem hiding this comment.
🟡 Medium (advisory) | DOM XSS via jQuery.load() | CWE-79
Bootstrap Modal Plugin — Unsafe jQuery.load() via remote Option
In the minified Bootstrap 3.0.0 code, the modal constructor does: this.options.remote && this.$element.load(this.options.remote). jQuery's .load() method fetches content from a URL and injects it into the selected element. If the response contains <script> tags, they will be executed. While this is typically triggered via data-remote attribute on the modal trigger element, if an attacker can control the remote option through any means (e.g., DOM manipulation via stored XSS, or if the application passes user data to the modal's remote option), this can lead to arbitrary script execution.
Why it's dangerous: In this codebase, the modal component is not directly used in the templates. However, the vulnerability exists in the library code. If a developer adds a modal with a user-controlled data-remote attribute, or if an attacker achieves stored XSS through another vector (e.g., memos with marked() bypass, or the autoescape:false template rendering), they could use this to load arbitrary remote content.
Attack chain — click to expand
- Attacker achieves stored XSS through another vector (e.g., memo content with marked() sanitizer bypass). 2. The stored XSS payload creates a Bootstrap modal with a malicious data-remote attribute pointing to an attacker-controlled server. 3. When triggered, Bootstrap's modal loads the attacker's HTML/JavaScript via jQuery.load(). 4. Attacker executes arbitrary JavaScript in the victim's session.
| @@ -0,0 +1,11 @@ | |||
| /*! | |||
There was a problem hiding this comment.
🟡 Medium (advisory) | DOM XSS via Unsafe Regex Parsing | CWE-79
Bootstrap Components Use Unsafe Regex to Parse href Attributes for jQuery Selectors
In the minified Bootstrap 3.0.0 code, multiple components parse href attributes using the regex replace(/.*(?=#[^\s]*$)/, "") to extract the hash fragment, then pass the result to jQuery. For example, in the alert plugin: d=c.attr("href"),d=d&&d.replace(/.*(?=#[^\s]*$)/,""),var e=a(d). If the href value contains HTML (e.g., javascript:alert(1)# or <img src=x onerror=alert(1)>), the regex extracts the part after the last space before #, and jQuery interprets it as HTML. With the application's autoescape: false setting, user data rendered into href attributes can trigger this.
Why it's dangerous: In this codebase, the scrollspy plugin is particularly relevant because it parses href attributes from nav links. The layout.html template renders user data (firstName, lastName) in the nav bar. If user data is rendered into an href attribute that Bootstrap's scrollspy processes, an attacker could inject a malicious href value that leads to XSS.
Attack chain — click to expand
- Attacker registers with a username containing a malicious payload designed to exploit the href parsing. 2. When the dashboard page renders, the username appears in a nav link href. 3. Bootstrap's scrollspy or other component parses the href and passes it to jQuery. 4. jQuery creates DOM elements and executes the payload.
| @@ -0,0 +1,11 @@ | |||
| /*! | |||
There was a problem hiding this comment.
⚪ Info (advisory) | Outdated Component with Known Vulnerabilities | CWE-79
Bootstrap 3.0.0 (2013) — Multiple Known DOM XSS Vulnerabilities
Bootstrap 3.0.0 is vulnerable to multiple CVEs including CVE-2019-8331 (data-target XSS in collapse plugin), CVE-2018-14041 (data-container XSS in tooltip/popover), and CVE-2018-14042 (data-template XSS). The core issue is that Bootstrap components take data-target and href attribute values and pass them directly to jQuery selectors (e.g., a(e) in the collapse plugin). jQuery's selector engine interprets strings containing HTML tags as HTML, creating DOM elements and executing inline event handlers. The application has autoescape: false set in the Swig template engine (server.js line ~100), meaning template variables are rendered without HTML escaping. This combination means any user-controlled data that ends up in a Bootstrap data attribute can lead to XSS.
Why it's dangerous: In this codebase, Bootstrap 3.0.0 is loaded on every page via layout.html. The application has autoescape: false, so user data (firstName, lastName, website, memo content, etc.) is rendered raw in templates. If any user data flows into a Bootstrap data-target, data-toggle, href, or data-container attribute, an attacker can achieve DOM-based XSS. The memos feature renders user-submitted Markdown via marked() which, even with sanitize:true, has known bypasses in version 0.3.5. The profile page's website field is encoded for HTML context but used in a URL context (href), creating a context-mismatch XSS vector.
Attack chain — click to expand
- Attacker registers an account with a malicious payload in a profile field (e.g., firstName containing
"><img src=x onerror=alert(document.cookie)>). 2. When an admin or other user views the profile page, the payload is rendered without HTML escaping (autoescape: false). 3. If the payload lands in a Bootstrap data-target attribute, Bootstrap's jQuery selector creates DOM elements and executes the payload. 4. Attacker steals session cookies or performs actions on behalf of the victim.
Note
(Verifier: false positive — Bootstrap 3.0.0 is indeed outdated and has known DOM XSS CVEs (CVE-2019-8331, CVE-2018-14041, CVE-2018-14042). However, these vulnerabilities require user-controlled data to flow into Bootstrap data-target, data-container, data-template, or href attributes that Bootstrap uses as jQuery selectors. In this codebase, ALL Bootstrap data attributes are hardcoded static values: data-target='.navbar-ex1-collapse', data-toggle='collapse', data-toggle='dropdown', data-dismiss='alert', etc. No code path exists where user-controlled data ends up in a Bootstrap data attribute. The finding's attack chain is speculative ('If any user data flows into...') but no such flow exists. The library is outdated but the specific DOM XSS vulnerabilities are not exploitable in the current usage context.)
| @@ -0,0 +1,11 @@ | |||
| /*! | |||
There was a problem hiding this comment.
⚪ Info (advisory) | DOM XSS via jQuery Selector Injection | CWE-79
Bootstrap Collapse Plugin — data-target Passed Directly to jQuery Selector (CVE-2019-8331)
In the minified Bootstrap 3.0.0 code, the collapse plugin's event handler does: var c=a(this),d,e=c.attr("data-target")||...||(d=c.attr("href"))&&d.replace(/.*(?=#[^\\s]+$)/,""),f=a(e). The value of data-target is taken directly from the DOM attribute and passed to a(e) (jQuery). jQuery's selector engine treats strings containing < as HTML and creates DOM elements, executing any inline event handlers. This is CVE-2019-8331, fixed in Bootstrap 3.4.1. The same pattern exists in the modal, dropdown, tab, alert, carousel, and scrollspy plugins. With the application's autoescape: false setting, any user-controlled data rendered into a template that ends up in a Bootstrap data attribute can trigger this vulnerability.
Why it's dangerous: In this codebase, the collapse plugin is used in layout.html with data-target=\".navbar-ex1-collapse\" (static). However, the application renders user data in templates with autoescape disabled. If a developer adds a Bootstrap data attribute with user data, or if user data is rendered into an href attribute that Bootstrap processes (e.g., in the scrollspy plugin which parses href attributes), an attacker can achieve XSS. The scrollspy plugin in particular parses href attributes from nav links and uses them as jQuery selectors.
Attack chain — click to expand
- Attacker identifies a template where user data is rendered into an href or data-target attribute (e.g., the profile page's website field, or the memos feature). 2. Attacker submits a payload like
"><img src=x onerror=alert(document.cookie)>as the website URL. 3. When the profile page is rendered, the payload ends up in an attribute that Bootstrap processes. 4. Bootstrap passes the value to jQuery which creates the img element and executes the onerror handler. 5. Attacker steals session cookies or performs actions on behalf of the victim.
Note
(Verifier: false positive — Same analysis as finding [7]. The Bootstrap collapse plugin's data-target is hardcoded as '.navbar-ex1-collapse' in layout.html. No user-controlled data flows into any Bootstrap data-target, href, or data-container attribute that Bootstrap processes as a jQuery selector. The scrollspy plugin parses href attributes from nav links, but all nav link hrefs are hardcoded static values (/benefits, /dashboard, /profile, etc.). The vulnerability exists in the library but is not exploitable in this codebase.)
1 participant
Summary
Brings the Node/Express user portal into the monorepo under
examples/nodegoat/. The portal handles account registration, profile management, session lifecycle, and activity logging for internal users.Previously this lived in its own repo with no shared lint or dependency management — consolidating here so we can apply consistent security policies across services.
Changes
Test plan
npm install && node server.jsstarts on port 4000Summary by CodeRabbit