code-review-nodejs

Node.js Code Review Patterns

Stack-specific rules loaded by dh:code-reviewer when package.json and *.js/*.mjs files are detected (without TypeScript).

Synchronous I/O in Request Path

• fs.readFileSync, fs.writeFileSync, execSync, spawnSync in any function called during request handling are blocking findings
• All file system operations in server code must use the async variants or fs/promises

// WRONG: blocks event loop
app.get("/config", (req, res) => {
  const config = fs.readFileSync("./config.json", "utf8");
  res.json(JSON.parse(config));
});

// RIGHT: non-blocking
app.get("/config", async (req, res) => {
  const config = await fs.promises.readFile("./config.json", "utf8");
  res.json(JSON.parse(config));
});

Stream Backpressure

• Piping streams without handling backpressure is a blocking finding for high-throughput paths
• readable.pipe(writable) handles backpressure automatically — prefer it over manual data event listeners
• Manual data event listeners must check writable.write() return value and pause the readable when it returns false

Process Exit

• process.exit() is only acceptable in CLI entrypoints — it is a blocking finding in library code, route handlers, or middleware
• Unhandled process.on("uncaughtException") that calls process.exit() without logging the error is a blocking finding

Security

• eval() is a blocking finding everywhere — no exceptions
• new Function(code) with user-controlled code is a blocking finding
• Shell arguments constructed by string concatenation with user input before passing to exec or spawn are a blocking finding
• execFile is required over exec when calling external programs — exec invokes a shell and is vulnerable to injection
• User-controlled values used as file paths must be validated against an allowed base directory (path traversal)

// WRONG: shell injection vector
exec(`convert ${userInput} output.png`);

// RIGHT: no shell, explicit args
execFile("convert", [userInput, "output.png"]);