Skip to content

Commit c6d1919

Browse files
committed
refactor: ensure fs patch tests are run on real fs library
1 parent 0ff53eb commit c6d1919

15 files changed

+346
-261
lines changed

js/private/node-patches/fs.cjs

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,38 @@ const util = require("util");
4545
// using require here on purpose so we can override methods with any
4646
// also even though imports are mutable in typescript the cognitive dissonance is too high because
4747
// es modules
48-
const _fs = require('node:fs');
48+
const fs = require('node:fs');
4949
const url = require('node:url');
5050
const HOP_NON_LINK = Symbol.for('HOP NON LINK');
5151
const HOP_NOT_FOUND = Symbol.for('HOP NOT FOUND');
52-
function patcher(fs = _fs, roots) {
53-
fs = fs || _fs;
52+
const PATCHED_FS_METHODS = [
53+
'lstat',
54+
'lstatSync',
55+
'realpath',
56+
'realpathSync',
57+
'readlink',
58+
'readlinkSync',
59+
'readdir',
60+
'readdirSync',
61+
'opendir',
62+
];
63+
/**
64+
* Function that patches the `fs` module to not escape the given roots.
65+
* @returns a function to undo the patches.
66+
*/
67+
function patcher(roots) {
68+
if (fs._unpatched) {
69+
throw new Error('FS is already patched.');
70+
}
5471
// Make the original version of the library available for when access to the
5572
// unguarded file system is necessary, such as the esbuild plugin that
5673
// protects against sandbox escaping that occurs through module resolution
5774
// in the Go binary. See
5875
// https://github.com/aspect-build/rules_esbuild/issues/58.
59-
fs._unpatched = Object.assign({}, fs);
76+
fs._unpatched = PATCHED_FS_METHODS.reduce((obj, method) => {
77+
obj[method] = fs[method];
78+
return obj;
79+
}, {});
6080
roots = roots || [];
6181
roots = roots.filter((root) => fs.existsSync(root));
6282
if (!roots.length) {
@@ -245,15 +265,8 @@ function patcher(fs = _fs, roots) {
245265
!isEscape(resolved, next, [escapedRoot])) {
246266
return cb(null, next);
247267
}
248-
// The escape from the root is not mappable back into the root; we must make
249-
// this look like a real file so we call readlink on the realpath which we
250-
// expect to return an error
251-
return origRealpath(resolved, readlinkRealpathCb);
252-
function readlinkRealpathCb(err, str) {
253-
if (err)
254-
return cb(err);
255-
return origReadlink(str, cb);
256-
}
268+
// The escape from the root is not mappable back into the root; throw EINVAL
269+
return cb(einval('readlink', args[0]));
257270
}
258271
}
259272
else {
@@ -366,6 +379,7 @@ function patcher(fs = _fs, roots) {
366379
* this api is available as experimental without a flag so users can access it at any time.
367380
*/
368381
const promisePropertyDescriptor = Object.getOwnPropertyDescriptor(fs, 'promises');
382+
let unpatchPromises;
369383
if (promisePropertyDescriptor) {
370384
const promises = {};
371385
promises.lstat = util.promisify(fs.lstat);
@@ -380,16 +394,28 @@ function patcher(fs = _fs, roots) {
380394
if (promisePropertyDescriptor.get) {
381395
const oldGetter = promisePropertyDescriptor.get.bind(fs);
382396
const cachedPromises = {};
383-
promisePropertyDescriptor.get = () => {
397+
function promisePropertyGetter() {
384398
const _promises = oldGetter();
385399
Object.assign(cachedPromises, _promises, promises);
386400
return cachedPromises;
401+
}
402+
Object.defineProperty(fs, 'promises', Object.assign({
403+
get: promisePropertyGetter,
404+
}, Object.create(promisePropertyDescriptor)));
405+
unpatchPromises = function unpatchFsPromises() {
406+
Object.defineProperty(fs, 'promises', promisePropertyDescriptor);
387407
};
388-
Object.defineProperty(fs, 'promises', promisePropertyDescriptor);
389408
}
390409
else {
410+
const unpatched_promises = Object.keys(promises).reduce((obj, method) => {
411+
obj[method] = fs.promises[method];
412+
return obj;
413+
}, Object.create(fs.promises));
391414
// api can be patched directly
392415
Object.assign(fs.promises, promises);
416+
unpatchPromises = function unpatchFsPromises() {
417+
Object.assign(fs.promises, unpatched_promises);
418+
};
393419
}
394420
}
395421
// =========================================================================
@@ -721,6 +747,13 @@ function patcher(fs = _fs, roots) {
721747
}
722748
}
723749
}
750+
return function revertPatch() {
751+
Object.assign(fs, fs._unpatched);
752+
delete fs._unpatched;
753+
if (unpatchPromises) {
754+
unpatchPromises();
755+
}
756+
};
724757
}
725758
// =========================================================================
726759
// generic helper functions

js/private/node-patches/register.cjs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,14 @@ if (
3535
JS_BINARY__PATCH_NODE_FS != '0' &&
3636
JS_BINARY__FS_PATCH_ROOTS
3737
) {
38-
const fs = require('node:fs')
3938
const module = require('node:module')
4039
const roots = JS_BINARY__FS_PATCH_ROOTS.split(':')
4140
if (JS_BINARY__LOG_DEBUG) {
4241
console.error(
4342
`DEBUG: ${JS_BINARY__LOG_PREFIX}: node fs patches will be applied with roots: ${roots}`
4443
)
4544
}
46-
patchfs(fs, roots)
45+
patchfs(roots)
4746

4847
// Sync the esm modules to use the now patched fs cjs module.
4948
// See: https://nodejs.org/api/esm.html#builtin-modules

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

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,22 +28,45 @@ type Dirent = any
2828
// using require here on purpose so we can override methods with any
2929
// also even though imports are mutable in typescript the cognitive dissonance is too high because
3030
// es modules
31-
const _fs = require('node:fs') as typeof FsType
31+
const fs = require('node:fs') as any
3232
const url = require('node:url') as typeof UrlType
3333

3434
const HOP_NON_LINK = Symbol.for('HOP NON LINK')
3535
const HOP_NOT_FOUND = Symbol.for('HOP NOT FOUND')
3636

3737
type HopResults = string | typeof HOP_NON_LINK | typeof HOP_NOT_FOUND
3838

39-
export function patcher(fs: any = _fs, roots: string[]) {
40-
fs = fs || _fs
39+
const PATCHED_FS_METHODS: ReadonlyArray<keyof typeof FsType> = [
40+
'lstat',
41+
'lstatSync',
42+
'realpath',
43+
'realpathSync',
44+
'readlink',
45+
'readlinkSync',
46+
'readdir',
47+
'readdirSync',
48+
'opendir',
49+
]
50+
51+
/**
52+
* Function that patches the `fs` module to not escape the given roots.
53+
* @returns a function to undo the patches.
54+
*/
55+
export function patcher(roots: string[]): () => void {
56+
if (fs._unpatched) {
57+
throw new Error('FS is already patched.')
58+
}
59+
4160
// Make the original version of the library available for when access to the
4261
// unguarded file system is necessary, such as the esbuild plugin that
4362
// protects against sandbox escaping that occurs through module resolution
4463
// in the Go binary. See
4564
// https://github.com/aspect-build/rules_esbuild/issues/58.
46-
fs._unpatched = { ...fs }
65+
fs._unpatched = PATCHED_FS_METHODS.reduce((obj, method) => {
66+
obj[method] = fs[method]
67+
return obj
68+
}, {})
69+
4770
roots = roots || []
4871
roots = roots.filter((root) => fs.existsSync(root))
4972
if (!roots.length) {
@@ -283,18 +306,8 @@ export function patcher(fs: any = _fs, roots: string[]) {
283306
) {
284307
return cb(null, next)
285308
}
286-
// The escape from the root is not mappable back into the root; we must make
287-
// this look like a real file so we call readlink on the realpath which we
288-
// expect to return an error
289-
return origRealpath(resolved, readlinkRealpathCb)
290-
291-
function readlinkRealpathCb(
292-
err: NodeJS.ErrnoException,
293-
str: string
294-
) {
295-
if (err) return cb(err)
296-
return origReadlink(str, cb)
297-
}
309+
// The escape from the root is not mappable back into the root; throw EINVAL
310+
return cb(einval('readlink', args[0]))
298311
}
299312
} else {
300313
return cb(null, str)
@@ -434,6 +447,9 @@ export function patcher(fs: any = _fs, roots: string[]) {
434447
fs,
435448
'promises'
436449
)
450+
451+
let unpatchPromises: Function
452+
437453
if (promisePropertyDescriptor) {
438454
const promises: any = {}
439455
promises.lstat = util.promisify(fs.lstat)
@@ -448,15 +464,39 @@ export function patcher(fs: any = _fs, roots: string[]) {
448464
const oldGetter = promisePropertyDescriptor.get.bind(fs)
449465
const cachedPromises = {}
450466

451-
promisePropertyDescriptor.get = () => {
467+
function promisePropertyGetter() {
452468
const _promises = oldGetter()
453469
Object.assign(cachedPromises, _promises, promises)
454470
return cachedPromises
455471
}
456-
Object.defineProperty(fs, 'promises', promisePropertyDescriptor)
472+
Object.defineProperty(
473+
fs,
474+
'promises',
475+
Object.assign(
476+
{
477+
get: promisePropertyGetter,
478+
},
479+
Object.create(promisePropertyDescriptor)
480+
)
481+
)
482+
483+
unpatchPromises = function unpatchFsPromises() {
484+
Object.defineProperty(fs, 'promises', promisePropertyDescriptor)
485+
}
457486
} else {
487+
const unpatched_promises = Object.keys(promises).reduce(
488+
(obj, method) => {
489+
obj[method] = fs.promises[method]
490+
return obj
491+
},
492+
Object.create(fs.promises)
493+
)
494+
458495
// api can be patched directly
459496
Object.assign(fs.promises, promises)
497+
unpatchPromises = function unpatchFsPromises() {
498+
Object.assign(fs.promises, unpatched_promises)
499+
}
460500
}
461501
}
462502

@@ -819,6 +859,15 @@ export function patcher(fs: any = _fs, roots: string[]) {
819859
}
820860
}
821861
}
862+
863+
return function revertPatch() {
864+
Object.assign(fs, fs._unpatched)
865+
delete fs._unpatched
866+
867+
if (unpatchPromises) {
868+
unpatchPromises()
869+
}
870+
}
822871
}
823872

824873
// =========================================================================
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
87a6d20e2cabd494a5c1cd816d5d17a5d13cf5b69ab1dd596f22753634ac4ed7 js/private/test/image/cksum_node
1+
0affcc8a198222286524acce43885844cffe37f3bb7660493d79cc8afe2e927e js/private/test/image/cksum_node
22
52836a988c8ac815b4a3b70fa3a3acec67b851699fa989694cef4cc1fa53de96 js/private/test/image/cksum_package_store_3p
33
642b308a0561fb51dfd96d08d74a4ec419c9d2ca501cfa1002a49c8e25fbe4c2 js/private/test/image/cksum_package_store_1p
44
5d45f32dacf0b83e26c33d4e1017c694e79eaff29b8ecc336f9ea8fdee870d45 js/private/test/image/cksum_node_modules

js/private/test/image/custom_layers_nomatch_test_node.listing

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.
88
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/
99
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/
1010
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/
11-
-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
12-
-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
11+
-r-xr-xr-x 0 0 0 34163 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
12+
-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
1313
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
1414
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
1515
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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runf
77
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/
88
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/
99
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/
10-
-r-xr-xr-x 0 100 0 33280 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
11-
-r-xr-xr-x 0 100 0 1702 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
10+
-r-xr-xr-x 0 100 0 34163 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
11+
-r-xr-xr-x 0 100 0 1664 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
1212
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
1313
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
1414
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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runf
77
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/
88
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/
99
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/
10-
-r-xr-xr-x 0 0 0 33280 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
11-
-r-xr-xr-x 0 0 0 1702 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
10+
-r-xr-xr-x 0 0 0 34163 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
11+
-r-xr-xr-x 0 0 0 1664 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
1212
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
1313
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
1414
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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_
99
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/
1010
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/
1111
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/
12-
-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
12+
-r-xr-xr-x 0 0 0 34163 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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_
99
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/
1010
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/
1111
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/
12-
-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
12+
-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
1313
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/
1414
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/
1515
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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.
88
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/
99
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/
1010
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/
11-
-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
12-
-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
11+
-r-xr-xr-x 0 0 0 34163 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
12+
-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
1313
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
1414
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
1515
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/

0 commit comments

Comments
 (0)