code-review-nodejs

Node.js コードレビューのパターン

ソース、ランタイムのエントリーポイント、またはパッケージスクリプトから Node.js の実行が確認できる場合に、これらのチェックを読み込みます。対象には JavaScript、TypeScript、CJS、MJS が含まれます。該当する場合は、TypeScript または CLI のチェックも併せて適用してください。package.json だけでは、サーバーコードとブラウザービルドを区別できません。

これらのチェックを適用する前に、/dh:review-principles を読み込んでください。

リクエストパスでの同期 I/O

• fs.readFileSync、fs.writeFileSync、execSync、spawnSync をアクティブなリクエスト／イベントループのパスまで追跡し、ブロッキングによる影響、またはレイテンシー契約への違反を確認します。
• そのパスをブロックしてはならない処理には、非同期のバリアントまたは fs/promises を使用します。サーバーファイル内にあるという理由だけで、独立した起動時／セットアップ時の I/O を指摘しないでください。

// 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));
});

ストリームのバックプレッシャー

• 高スループットのパスで、バックプレッシャーを処理せずにストリームをパイプしている場合は、ブロッキング対象の指摘となります。
• readable.pipe(writable) はバックプレッシャーを自動的に処理するため、手動の data イベントリスナーよりもこちらを優先します。
• 手動の data イベントリスナーでは、writable.write() の戻り値を確認し、false が返された場合は readable を一時停止する必要があります。

プロセスの終了

• 致命的な起動失敗も含め、シャットダウンを管理するアプリケーション／CLI の境界でプロセスを終了させます。無関係な処理を予期せず終了させたり、必要なクリーンアップを回避したりするライブラリ、ルート、ミドルウェアでの終了処理を報告します。
• エラーをログに記録せずに process.exit() を呼び出す、未処理の process.on("uncaughtException") は、ブロッキング対象の指摘となります。

セキュリティ

• eval() などの動的評価について、入力の信頼性と実行権限を追跡します。信頼できないコードの実行、またはプロジェクトで明示的に禁止されている動作を報告します。
• ユーザーが制御できる code を使う new Function(code) は、ブロッキング対象の指摘となります。
• ユーザー入力がシェル評価やコマンド／オプションの解析に渡る経路を追跡します。シェルへの埋め込みと、個別のプロセス引数を区別し、実際の呼び出しで可能になるインジェクションや不正な操作を報告します。
• シェルの動作が不要な場合は、引数を分けて指定できる execFile を優先します。意図的にシェルを使用する場合は、信頼できる定数コマンドをユーザー入力によるインジェクションとして扱わず、文字列の埋め込み方と入力の信頼性を調べます。
• ユーザーが制御するパスを、その操作で許可されたリソースに制限します。契約上必要な場合は許可ルートの適用を確認し、その境界を越えるパストラバーサルの経路を追跡します。

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

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

依存関係の衛生管理

• バージョン範囲を対象の依存関係およびリリースポリシーと照合します。再現性がないと指摘する前に、ワイルドカード範囲が実際の解決／インストール動作に及ぼす影響を追跡します。
• 選択されたパッケージマネージャーのロックファイルと CI のインストールモードによって、必要な依存関係の解決が維持されていることを確認します。別のサポート対象パッケージマネージャーを使用しているプロジェクトに、npm／yarn のロックファイルを要求しないでください。
• 開発専用の依存関係は dependencies ではなく devDependencies に指定する必要があります。

イベントエミッターのクリーンアップ

• コンポーネント／接続のライフサイクル中に追加した EventEmitter.on() リスナーは、そのライフサイクルの終了時に削除する必要があります。
• リスナーよりもエミッターの寿命が長い場合、removeListener または off() の呼び出しがないことは、ブロッキング対象の指摘となります。
• 手動でのクリーンアップを避けるため、1 回限りのリスナーには EventEmitter.once() を使用します。

環境変数

• 設定を必要とする処理を受け付ける前に、選択された実行モードで必須の設定を検証します。無効になっている任意機能のために、認証情報の設定を要求しないでください。
• process.env.SOME_VAR! を該当する検証および利用箇所まで追跡し、実際の失敗経路とともに、必須値の不足やサポートされていない前提を報告します。
• 必須変数をすべて記載した .env.example ファイルを用意します。ファイルはリポジトリにチェックインし、実際の値を含めないでください。

アンチパターン

// WRONG: missing error handling on EventEmitter
server.on("connection", (socket) => {
  socket.on("data", handleData);
  // missing: socket.on("end", cleanup) and removeListener
});

// WRONG: unvalidated env at use site
const apiKey = process.env.API_KEY;
fetch(url, { headers: { Authorization: apiKey } }); // null if unset

// RIGHT: validate at startup
if (!process.env.API_KEY) {
  console.error("FATAL: API_KEY environment variable is required");
  process.exit(1);
}
const apiKey = process.env.API_KEY;