From cdb57516ae4b575a43c3ff3548ce2551c0061687 Mon Sep 17 00:00:00 2001
From: Hans Ott <hansott@hotmail.be>
Date: Tue, 15 Oct 2024 14:10:41 +0200
Subject: [PATCH] Wrap require("fs").promises if "fs" is required

We only wrapped require("fs/promises") when fs/promises is required

This would leave require("fs").promises unprotected if just fs is
required

We should only wrap promises once

They point to the same functions:

Welcome to Node.js v21.4.0.
Type ".help" for more information.
> require("fs").promises.readFile === require("fs/promises").readFile
true
>
---
 library/sinks/FileSystem.ts | 44 ++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/library/sinks/FileSystem.ts b/library/sinks/FileSystem.ts
index 1141f13c7..650f73341 100644
--- a/library/sinks/FileSystem.ts
+++ b/library/sinks/FileSystem.ts
@@ -2,6 +2,7 @@ import { getContext } from "../agent/Context";
 import { Hooks } from "../agent/hooks/Hooks";
 import { InterceptorResult } from "../agent/hooks/InterceptorResult";
 import { wrapExport } from "../agent/hooks/wrapExport";
+import { WrapPackageInfo } from "../agent/hooks/WrapPackageInfo";
 import { Wrapper } from "../agent/Wrapper";
 import { getSemverNodeVersion } from "../helpers/getNodeVersion";
 import { isVersionGreaterOrEqual } from "../helpers/isVersionGreaterOrEqual";
@@ -14,6 +15,8 @@ type FileSystemFunction = {
 };
 
 export class FileSystem implements Wrapper {
+  private patchedPromises = false;
+
   private inspectPath(
     args: unknown[],
     name: string,
@@ -91,12 +94,33 @@ export class FileSystem implements Wrapper {
     return functions;
   }
 
+  wrapPromises(exports: unknown, pkgInfo: WrapPackageInfo) {
+    if (this.patchedPromises) {
+      // `require("fs").promises.readFile` is the same as `require("fs/promises").readFile`
+      // We only need to wrap the promise version once
+      return;
+    }
+
+    const functions = this.getFunctions();
+    Object.keys(functions).forEach((name) => {
+      const { pathsArgs, promise } = functions[name];
+
+      if (promise) {
+        wrapExport(exports, name, pkgInfo, {
+          inspectArgs: (args) => this.inspectPath(args, name, pathsArgs),
+        });
+      }
+    });
+
+    this.patchedPromises = true;
+  }
+
   wrap(hooks: Hooks) {
     hooks.addBuiltinModule("fs").onRequire((exports, pkgInfo) => {
       const functions = this.getFunctions();
 
       Object.keys(functions).forEach((name) => {
-        const { pathsArgs, sync, promise } = functions[name];
+        const { pathsArgs, sync } = functions[name];
 
         wrapExport(exports, name, pkgInfo, {
           inspectArgs: (args) => {
@@ -119,24 +143,18 @@ export class FileSystem implements Wrapper {
           return this.inspectPath(args, "realpath.native", 1);
         },
       });
+
       wrapExport(exports.realpathSync, "native", pkgInfo, {
         inspectArgs: (args) => {
           return this.inspectPath(args, "realpathSync.native", 1);
         },
       });
-    });
-
-    hooks.addBuiltinModule("fs/promises").onRequire((exports, pkgInfo) => {
-      const functions = this.getFunctions();
-      Object.keys(functions).forEach((name) => {
-        const { pathsArgs, sync, promise } = functions[name];
 
-        if (promise) {
-          wrapExport(exports, name, pkgInfo, {
-            inspectArgs: (args) => this.inspectPath(args, name, pathsArgs),
-          });
-        }
-      });
+      this.wrapPromises(exports.promises, pkgInfo);
     });
+
+    hooks
+      .addBuiltinModule("fs/promises")
+      .onRequire((exports, pkgInfo) => this.wrapPromises(exports, pkgInfo));
   }
 }