refactor: ensure fs patch tests are run on real fs library

js/private/node-patches/fs.cjs

@@ -45,18 +45,38 @@ const util = require("util");
 // using require here on purpose so we can override methods with any
 // also even though imports are mutable in typescript the cognitive dissonance is too high because
 // es modules
-const _fs = require('node:fs');
+const fs = require('node:fs');
 const url = require('node:url');
 const HOP_NON_LINK = Symbol.for('HOP NON LINK');
 const HOP_NOT_FOUND = Symbol.for('HOP NOT FOUND');
-function patcher(fs = _fs, roots) {
-    fs = fs || _fs;
+const PATCHED_FS_METHODS = [
+    'lstat',
+    'lstatSync',
+    'realpath',
+    'realpathSync',
+    'readlink',
+    'readlinkSync',
+    'readdir',
+    'readdirSync',
+    'opendir',
+];
+/**
+ * Function that patches the `fs` module to not escape the given roots.
+ * @returns a function to undo the patches.
+ */
+function patcher(roots) {
+    if (fs._unpatched) {
+        throw new Error('FS is already patched.');
+    }
     // Make the original version of the library available for when access to the
     // unguarded file system is necessary, such as the esbuild plugin that
     // protects against sandbox escaping that occurs through module resolution
     // in the Go binary. See
     // https://github.com/aspect-build/rules_esbuild/issues/58.
-    fs._unpatched = Object.assign({}, fs);
+    fs._unpatched = PATCHED_FS_METHODS.reduce((obj, method) => {
+        obj[method] = fs[method];
+        return obj;
+    }, {});
     roots = roots || [];
     roots = roots.filter((root) => fs.existsSync(root));
     if (!roots.length) {
@@ -245,15 +265,8 @@ function patcher(fs = _fs, roots) {
                         !isEscape(resolved, next, [escapedRoot])) {
                         return cb(null, next);
                     }
-                    // The escape from the root is not mappable back into the root; we must make
-                    // this look like a real file so we call readlink on the realpath which we
-                    // expect to return an error
-                    return origRealpath(resolved, readlinkRealpathCb);
-                    function readlinkRealpathCb(err, str) {
-                        if (err)
-                            return cb(err);
-                        return origReadlink(str, cb);
-                    }
+                    // The escape from the root is not mappable back into the root; throw EINVAL
+                    return cb(einval('readlink', args[0]));
                 }
             }
             else {
@@ -366,6 +379,7 @@ function patcher(fs = _fs, roots) {
      * this api is available as experimental without a flag so users can access it at any time.
      */
     const promisePropertyDescriptor = Object.getOwnPropertyDescriptor(fs, 'promises');
+    let unpatchPromises;
     if (promisePropertyDescriptor) {
         const promises = {};
         promises.lstat = util.promisify(fs.lstat);
@@ -380,16 +394,28 @@ function patcher(fs = _fs, roots) {
         if (promisePropertyDescriptor.get) {
             const oldGetter = promisePropertyDescriptor.get.bind(fs);
             const cachedPromises = {};
-            promisePropertyDescriptor.get = () => {
+            function promisePropertyGetter() {
                 const _promises = oldGetter();
                 Object.assign(cachedPromises, _promises, promises);
                 return cachedPromises;
+            }
+            Object.defineProperty(fs, 'promises', Object.assign(Object.create(promisePropertyDescriptor), {
+                get: promisePropertyGetter,
+            }));
+            unpatchPromises = function unpatchFsPromises() {
+                Object.defineProperty(fs, 'promises', promisePropertyDescriptor);
             };
-            Object.defineProperty(fs, 'promises', promisePropertyDescriptor);
         }
         else {
+            const unpatchedPromises = Object.keys(promises).reduce((obj, method) => {
+                obj[method] = fs.promises[method];
+                return obj;
+            }, Object.create(fs.promises));
             // api can be patched directly
             Object.assign(fs.promises, promises);
+            unpatchPromises = function unpatchFsPromises() {
+                Object.assign(fs.promises, unpatchedPromises);
+            };
         }
     }
     // =========================================================================
@@ -721,6 +747,13 @@ function patcher(fs = _fs, roots) {
             }
         }
     }
+    return function revertPatch() {
+        Object.assign(fs, fs._unpatched);
+        delete fs._unpatched;
+        if (unpatchPromises) {
+            unpatchPromises();
+        }
+    };
 }
 // =========================================================================
 // generic helper functions

js/private/node-patches/register.cjs

@@ -35,15 +35,14 @@ if (
     JS_BINARY__PATCH_NODE_FS != '0' &&
     JS_BINARY__FS_PATCH_ROOTS
 ) {
-    const fs = require('node:fs')
     const module = require('node:module')
     const roots = JS_BINARY__FS_PATCH_ROOTS.split(':')
     if (JS_BINARY__LOG_DEBUG) {
         console.error(
             `DEBUG: ${JS_BINARY__LOG_PREFIX}: node fs patches will be applied with roots: ${roots}`
         )
     }
-    patchfs(fs, roots)
+    patchfs(roots)
 
     // Sync the esm modules to use the now patched fs cjs module.
     // See: https://nodejs.org/api/esm.html#builtin-modules

js/private/node-patches/src/fs.cts

@@ -28,22 +28,45 @@ type Dirent = any
 // using require here on purpose so we can override methods with any
 // also even though imports are mutable in typescript the cognitive dissonance is too high because
 // es modules
-const _fs = require('node:fs') as typeof FsType
+const fs = require('node:fs') as any
 const url = require('node:url') as typeof UrlType
 
 const HOP_NON_LINK = Symbol.for('HOP NON LINK')
 const HOP_NOT_FOUND = Symbol.for('HOP NOT FOUND')
 
 type HopResults = string | typeof HOP_NON_LINK | typeof HOP_NOT_FOUND
 
-export function patcher(fs: any = _fs, roots: string[]) {
-    fs = fs || _fs
+const PATCHED_FS_METHODS: ReadonlyArray<keyof typeof FsType> = [
+    'lstat',
+    'lstatSync',
+    'realpath',
+    'realpathSync',
+    'readlink',
+    'readlinkSync',
+    'readdir',
+    'readdirSync',
+    'opendir',
+]
+
+/**
+ * Function that patches the `fs` module to not escape the given roots.
+ * @returns a function to undo the patches.
+ */
+export function patcher(roots: string[]): () => void {
+    if (fs._unpatched) {
+        throw new Error('FS is already patched.')
+    }
+
     // Make the original version of the library available for when access to the
     // unguarded file system is necessary, such as the esbuild plugin that
     // protects against sandbox escaping that occurs through module resolution
     // in the Go binary. See
     // https://github.com/aspect-build/rules_esbuild/issues/58.
-    fs._unpatched = { ...fs }
+    fs._unpatched = PATCHED_FS_METHODS.reduce((obj, method) => {
+        obj[method] = fs[method]
+        return obj
+    }, {})
+
     roots = roots || []
     roots = roots.filter((root) => fs.existsSync(root))
     if (!roots.length) {
@@ -283,18 +306,8 @@ export function patcher(fs: any = _fs, roots: string[]) {
                     ) {
                         return cb(null, next)
                     }
-                    // The escape from the root is not mappable back into the root; we must make
-                    // this look like a real file so we call readlink on the realpath which we
-                    // expect to return an error
-                    return origRealpath(resolved, readlinkRealpathCb)
-
-                    function readlinkRealpathCb(
-                        err: NodeJS.ErrnoException,
-                        str: string
-                    ) {
-                        if (err) return cb(err)
-                        return origReadlink(str, cb)
-                    }
+                    // The escape from the root is not mappable back into the root; throw EINVAL
+                    return cb(einval('readlink', args[0]))
                 }
             } else {
                 return cb(null, str)
@@ -434,6 +447,9 @@ export function patcher(fs: any = _fs, roots: string[]) {
         fs,
         'promises'
     )
+
+    let unpatchPromises: Function
+
     if (promisePropertyDescriptor) {
         const promises: any = {}
         promises.lstat = util.promisify(fs.lstat)
@@ -448,15 +464,36 @@ export function patcher(fs: any = _fs, roots: string[]) {
             const oldGetter = promisePropertyDescriptor.get.bind(fs)
             const cachedPromises = {}
 
-            promisePropertyDescriptor.get = () => {
+            function promisePropertyGetter() {
                 const _promises = oldGetter()
                 Object.assign(cachedPromises, _promises, promises)
                 return cachedPromises
             }
-            Object.defineProperty(fs, 'promises', promisePropertyDescriptor)
+            Object.defineProperty(
+                fs,
+                'promises',
+                Object.assign(Object.create(promisePropertyDescriptor), {
+                    get: promisePropertyGetter,
+                })
+            )
+
+            unpatchPromises = function unpatchFsPromises() {
+                Object.defineProperty(fs, 'promises', promisePropertyDescriptor)
+            }
         } else {
+            const unpatchedPromises = Object.keys(promises).reduce(
+                (obj, method) => {
+                    obj[method] = fs.promises[method]
+                    return obj
+                },
+                Object.create(fs.promises)
+            )
+
             // api can be patched directly
             Object.assign(fs.promises, promises)
+            unpatchPromises = function unpatchFsPromises() {
+                Object.assign(fs.promises, unpatchedPromises)
+            }
         }
     }
 
@@ -819,6 +856,15 @@ export function patcher(fs: any = _fs, roots: string[]) {
             }
         }
     }
+
+    return function revertPatch() {
+        Object.assign(fs, fs._unpatched)
+        delete fs._unpatched
+
+        if (unpatchPromises) {
+            unpatchPromises()
+        }
+    }
 }
 
 // =========================================================================

js/private/test/image/checksum_test.expected

@@ -1,4 +1,4 @@
-87a6d20e2cabd494a5c1cd816d5d17a5d13cf5b69ab1dd596f22753634ac4ed7  js/private/test/image/cksum_node
+a4232a52470e03fc555dfc1bd5a6e83090a71bccad58eb45fbc55f780f84df1e  js/private/test/image/cksum_node
 52836a988c8ac815b4a3b70fa3a3acec67b851699fa989694cef4cc1fa53de96  js/private/test/image/cksum_package_store_3p
 642b308a0561fb51dfd96d08d74a4ec419c9d2ca501cfa1002a49c8e25fbe4c2  js/private/test/image/cksum_package_store_1p
 5d45f32dacf0b83e26c33d4e1017c694e79eaff29b8ecc336f9ea8fdee870d45  js/private/test/image/cksum_node_modules

js/private/test/image/custom_layers_nomatch_test_node.listing

@@ -8,8 +8,8 @@ drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 0      0       33280 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
--r-xr-xr-x  0 0      0        1702 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
+-r-xr-xr-x  0 0      0       34161 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 0      0        1664 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/nodejs/

js/private/test/image/custom_owner_test_node.listing

@@ -7,8 +7,8 @@ drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runf
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 100    0       33280 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
--r-xr-xr-x  0 100    0        1702 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
+-r-xr-xr-x  0 100    0       34161 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 100    0        1664 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/nodejs/

js/private/test/image/default_test_node.listing

@@ -7,8 +7,8 @@ drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runf
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 0      0       33280 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
--r-xr-xr-x  0 0      0        1702 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
+-r-xr-xr-x  0 0      0       34161 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 0      0        1664 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/nodejs/

js/private/test/image/non_ascii/custom_layer_groups_test_just_the_fs_patch.listing

@@ -9,4 +9,4 @@ drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 0      0       33280 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 0      0       34161 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/fs.cjs

js/private/test/image/non_ascii/custom_layer_groups_test_node.listing

@@ -9,7 +9,7 @@ drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 0      0        1702 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/register.cjs
+-r-xr-xr-x  0 0      0        1664 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/register.cjs
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/nodejs/

js/private/test/image/regex_edge_cases_test_node.listing

@@ -8,8 +8,8 @@ drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 0      0       33280 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
--r-xr-xr-x  0 0      0        1702 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
+-r-xr-xr-x  0 0      0       34161 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 0      0        1664 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/nodejs/