code-review-nodejs

Node.js Code Review Patterns

Load these checks when source, runtime entrypoints, or package scripts establish Node.js execution, including JavaScript, TypeScript, CJS, and MJS. Apply TypeScript or CLI checks alongside them when relevant; a package.json alone does not distinguish server code from a browser build.

Load /dh:review-principles before applying these checks.

Synchronous I/O in Request Path

• Trace fs.readFileSync, fs.writeFileSync, execSync, or spawnSync to the active request/event-loop path and establish the blocking consequence or violated latency contract.
• Use async variants or fs/promises for work that must not block that path. Do not flag isolated startup/setup I/O solely because it is in a server file.

// 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

• Keep process termination at the application/CLI boundary that owns shutdown, including fatal startup failure. Report library, route, or middleware exits that unexpectedly terminate unrelated work or bypass required cleanup.
• Unhandled process.on("uncaughtException") that calls process.exit() without logging the error is a blocking finding

Security

• Trace dynamic evaluation, including eval(), to its input authority and execution privileges; report untrusted-code execution or an explicit project prohibition.
• new Function(code) with user-controlled code is a blocking finding
• Trace user input into shell evaluation and command/option parsing. Distinguish shell interpolation from separate process arguments, and report the injection or unauthorized operation the actual call permits.
• Prefer execFile with separate arguments when no shell behavior is needed. For deliberate shell use, inspect interpolation and input authority rather than treating a trusted constant command as user-input injection.
• Constrain user-controlled paths to the resources authorized for the operation; verify allowed-root enforcement where the contract requires it and trace path traversal across that boundary.