Code review · Node.js
Review an Express User Search Endpoint
Easy Code review Most asked
An admin dashboard needs to search users by name. This Express route was added in a pull request. It uses the pg driver.
Review it: list the problems, why each matters in production, and how to fix it.
The code to review
There are 5 issues to find.
const express = require("express");
const { Pool } = require("pg");
const pool = new Pool();
const router = express.Router();
router.get("/admin/users/search", async (req, res) => {
const name = req.query.name;
const sql = "SELECT * FROM users WHERE name LIKE '%" + name + "%'";
try {
const result = await pool.query(sql);
res.json(result.rows);
} catch (err) {
res.status(500).json({ error: err.stack });
}
});
module.exports = router;
Write your review
Loading the editor… (JavaScript is required to run code in your browser.)
The issues and how to fix them
Show the issues
1. SQL injection
Critical, line 9-11
The search term is concatenated into the SQL string. name=' OR '1'='1 returns every user, and '; DROP TABLE users; -- style payloads can do worse. This is OWASP's injection category and still one of the most exploited bugs.
Fix: Use a parameterised query: pool.query("SELECT id, name, email FROM users WHERE name ILIKE $1", [%${name}%]). The driver sends the value separately from the SQL text.
2. No authentication or authorization
Critical, line 7
The route is under /admin but nothing checks who is calling it. Anyone who finds the URL can enumerate users.
Fix: Mount it behind authentication middleware and check an admin role, for example router.get(..., requireAuth, requireRole("admin"), handler).
3. SELECT * returns sensitive columns
Major, line 9, 12
SELECT * sends every column to the browser, including password hashes, reset tokens and any column added later.
Fix: Select only the fields the screen needs (id, name, email) or map rows through a serializer.
4. Error responses leak internals
Major, line 14
Returning err.stack exposes file paths, library versions and SQL fragments to the client, which helps an attacker.
Fix: Log the error server-side with a request ID and return a generic message: res.status(500).json({ error: "Internal error", requestId }).
5. Unbounded result and missing input validation
Minor, line 8-9
A missing or empty name matches every row, and there is no limit, so one request can return the whole table. A leading % also prevents a normal B-tree index from being used.
Fix: Require a minimum length, add LIMIT 50 with pagination, and use a trigram index (pg_trgm) or full-text search for contains-matching.
Lead with injection and the missing authorization: both are exploitable today. Data exposure and error leakage come next, then performance. Interviewers listen for parameterised queries by name, not "sanitise the input".
Fixed version
const express = require("express");
const { Pool } = require("pg");
const { requireAuth, requireRole } = require("../auth");
const pool = new Pool();
const router = express.Router();
router.get("/admin/users/search", requireAuth, requireRole("admin"), async (req, res, next) => {
const name = String(req.query.name ?? "").trim();
if (name.length < 2) return res.status(400).json({ error: "name must be at least 2 characters" });
try {
const result = await pool.query(
"SELECT id, name, email FROM users WHERE name ILIKE $1 ORDER BY name LIMIT 50",
[`%${name}%`],
);
res.json(result.rows);
} catch (err) {
next(err); // central error handler logs it and returns a generic 500
}
});
module.exports = router;
Follow-up questions
- Why is escaping quotes by hand not a fix?
- How would you make a contains-search fast on 10 million users?
More Code Clinic: Code Review problems
- Review a Laravel Orders Controller · Medium
- Review a Payment Webhook Handler · Medium
- Review a Sign-Up Function · Medium
- Review a CSV Export Endpoint · Medium
- Review a React Search Component · Medium
- Review a Vue Comment List · Medium