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?