Skip to content

Commit 1cfd3d4

Browse files
devversionjbedard
authored andcommitted
WIP
1 parent c6d1919 commit 1cfd3d4

20 files changed

+418
-41
lines changed

MODULE.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ bazel_dep(name = "aspect_tools_telemetry", version = "0.2.8")
1313
bazel_dep(name = "bazel_features", version = "1.9.0")
1414
bazel_dep(name = "bazel_skylib", version = "1.5.0")
1515
bazel_dep(name = "platforms", version = "0.0.5")
16-
bazel_dep(name = "rules_nodejs", version = "6.3.0")
16+
bazel_dep(name = "rules_nodejs", version = "6.4.0")
1717

1818
tel = use_extension("@aspect_tools_telemetry//:extension.bzl", "telemetry")
1919
use_repo(tel, "aspect_tools_telemetry_report")

js/private/node-patches/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ write_source_files(
44
name = "checked_in_compile",
55
files = {
66
"fs.cjs": "//js/private/node-patches/src:fs-generated.cjs",
7+
"fs_stat.cjs": "//js/private/node-patches/src:fs_stat.cjs",
78
},
89
)
910

1011
exports_files([
1112
"fs.cjs",
13+
"fs_stat.cjs",
1214
"register.cjs",
1315
])

js/private/node-patches/fs.cjs

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ exports.isSubPath = isSubPath;
4242
exports.escapeFunction = escapeFunction;
4343
const path = require("path");
4444
const util = require("util");
45+
const fs_stat_cjs_1 = require("./fs_stat.cjs");
4546
// using require here on purpose so we can override methods with any
4647
// also even though imports are mutable in typescript the cognitive dissonance is too high because
4748
// es modules
@@ -85,8 +86,6 @@ function patcher(roots) {
8586
}
8687
return;
8788
}
88-
const origLstat = fs.lstat.bind(fs);
89-
const origLstatSync = fs.lstatSync.bind(fs);
9089
const origReaddir = fs.readdir.bind(fs);
9190
const origReaddirSync = fs.readdirSync.bind(fs);
9291
const origReadlink = fs.readlink.bind(fs);
@@ -161,22 +160,21 @@ function patcher(roots) {
161160
return stats;
162161
}
163162
try {
164-
args[0] = unguardedRealPathSync(args[0]);
165-
// there are no hops so lets report the stats of the real file;
166-
// we can't use origRealPathSync here since that function calls lstat internally
167-
// which can result in an infinite loop
168-
return origLstatSync(...args);
163+
lstatPatcher.originalSyncRequested = true;
164+
return fs.lstatSync.apply(fs, args);
169165
}
170-
catch (err) {
171-
if (err.code === 'ENOENT') {
172-
// broken link so there is nothing more to do
173-
return stats;
174-
}
175-
throw err;
166+
finally {
167+
lstatPatcher.originalSyncRequested = false;
176168
}
177169
};
178170
// =========================================================================
179-
// fs.realpath
171+
// fsInternal.lstat (to patch ESM resolve's `realpathSync`!) and
172+
// fs.lstat implementations.
173+
// fs.realpath implementations.
174+
// =========================================================================
175+
lstatPatcher.patch();
176+
// =========================================================================
177+
// fs.realpath.native
180178
// =========================================================================
181179
fs.realpath = function realpath(...args) {
182180
// preserve error when calling function without required callback
@@ -216,14 +214,6 @@ function patcher(roots) {
216214
};
217215
origRealpathNative(...args);
218216
};
219-
fs.realpathSync = function realpathSync(...args) {
220-
const str = origRealpathSync(...args);
221-
const escapedRoot = isEscape(args[0], str);
222-
if (escapedRoot) {
223-
return guardedRealPathSync(args[0], escapedRoot);
224-
}
225-
return str;
226-
};
227217
fs.realpathSync.native = function native_realpathSync(...args) {
228218
const str = origRealpathSyncNative(...args);
229219
const escapedRoot = isEscape(args[0], str);
@@ -382,7 +372,6 @@ function patcher(roots) {
382372
let unpatchPromises;
383373
if (promisePropertyDescriptor) {
384374
const promises = {};
385-
promises.lstat = util.promisify(fs.lstat);
386375
// NOTE: node core uses the newer realpath function fs.promises.native instead of fs.realPath
387376
promises.realpath = util.promisify(fs.realpath.native);
388377
promises.readlink = util.promisify(fs.readlink);
@@ -773,6 +762,7 @@ function stringifyPathLike(p) {
773762
function resolvePathLike(p) {
774763
return path.resolve(stringifyPathLike(p));
775764
}
765+
exports.resolvePathLike = resolvePathLike;
776766
function normalizePathLike(p) {
777767
const s = stringifyPathLike(p);
778768
// TODO: are URLs always absolute?
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
"use strict";
2+
// Patches Node's internal FS bindings, right before they would call into C++.
3+
// See full context in: https://github.com/aspect-build/rules_js/issues/362.
4+
// This is to ensure ESM imports don't escape accidentally via `realpathSync`.
5+
Object.defineProperty(exports, "__esModule", { value: true });
6+
exports.FsInternalStatPatcher = void 0;
7+
/// <reference path="./fs_stat_types.d.cts" />
8+
const binding_1 = require("internal/test/binding");
9+
const utils_1 = require("internal/fs/utils");
10+
const fs_cjs_1 = require("./fs.cjs");
11+
const internalFs = (0, binding_1.internalBinding)('fs');
12+
class FsInternalStatPatcher {
13+
constructor(escapeFns, guardedReadLink, guardedReadLinkSync, unguardedRealPath, unguardedRealPathSync) {
14+
this.escapeFns = escapeFns;
15+
this.guardedReadLink = guardedReadLink;
16+
this.guardedReadLinkSync = guardedReadLinkSync;
17+
this.unguardedRealPath = unguardedRealPath;
18+
this.unguardedRealPathSync = unguardedRealPathSync;
19+
this._originalFsLstat = internalFs.lstat;
20+
this.originalSyncRequested = false;
21+
}
22+
revert() {
23+
internalFs.lstat = this._originalFsLstat;
24+
}
25+
patch() {
26+
const statPatcher = this;
27+
internalFs.lstat = function (path, bigint, reqCallback, throwIfNoEntry) {
28+
if (this.originalSyncRequested) {
29+
return statPatcher._originalFsLstat.call(internalFs, path, bigint, reqCallback, throwIfNoEntry);
30+
}
31+
if (reqCallback === internalFs.kUsePromises) {
32+
return statPatcher._originalFsLstat.call(internalFs, path, bigint, reqCallback, throwIfNoEntry).then((stats) => {
33+
return new Promise((resolve, reject) => {
34+
statPatcher.eeguardStats(path, bigint, stats, throwIfNoEntry, (err, guardedStats) => {
35+
err || !guardedStats ? reject(err) : resolve(guardedStats);
36+
});
37+
});
38+
});
39+
}
40+
else if (reqCallback === undefined) {
41+
const stats = statPatcher._originalFsLstat.call(internalFs, path, bigint, undefined, throwIfNoEntry);
42+
if (!stats) {
43+
return stats;
44+
}
45+
return statPatcher.eeguardStatsSync(path, bigint, throwIfNoEntry, stats);
46+
}
47+
else {
48+
// Just re-use the promise path from above.
49+
internalFs.lstat(path, bigint, internalFs.kUsePromises, throwIfNoEntry)
50+
.then((stats) => reqCallback.oncomplete(null, stats))
51+
.catch((err) => reqCallback.oncomplete(err));
52+
}
53+
};
54+
}
55+
eeguardStats(path, bigint, stats, throwIfNotFound, cb) {
56+
const statsObj = (0, utils_1.getStatsFromBinding)(stats);
57+
if (!statsObj.isSymbolicLink()) {
58+
// the file is not a symbolic link so there is nothing more to do
59+
return cb(null, stats);
60+
}
61+
path = (0, fs_cjs_1.resolvePathLike)(path);
62+
if (!this.escapeFns.canEscape(path)) {
63+
// the file can not escaped the sandbox so there is nothing more to do
64+
return cb(null, stats);
65+
}
66+
return this.guardedReadLink(path, (str) => {
67+
if (str != path) {
68+
// there are one or more hops within the guards so there is nothing more to do
69+
return cb(null, stats);
70+
}
71+
// there are no hops so lets report the stats of the real file;
72+
// we can't use origRealPath here since that function calls lstat internally
73+
// which can result in an infinite loop
74+
return this.unguardedRealPath(path, (err, str) => {
75+
if (err) {
76+
if (err.code === 'ENOENT') {
77+
// broken link so there is nothing more to do
78+
return cb(null, stats);
79+
}
80+
return cb(err);
81+
}
82+
// Forward request to original callback.
83+
const req2 = new internalFs.FSReqCallback(bigint);
84+
req2.oncomplete = (err, realStats) => cb(err, realStats);
85+
return this._originalFsLstat.call(internalFs, str, bigint, req2, throwIfNotFound);
86+
});
87+
});
88+
}
89+
eeguardStatsSync(path, bigint, throwIfNoEntry, stats) {
90+
// No stats available.
91+
if (!stats) {
92+
return stats;
93+
}
94+
const statsObj = (0, utils_1.getStatsFromBinding)(stats);
95+
if (!statsObj.isSymbolicLink()) {
96+
// the file is not a symbolic link so there is nothing more to do
97+
return stats;
98+
}
99+
path = (0, fs_cjs_1.resolvePathLike)(path);
100+
if (!this.escapeFns.canEscape(path)) {
101+
// the file can not escaped the sandbox so there is nothing more to do
102+
return stats;
103+
}
104+
const guardedReadLink = this.guardedReadLinkSync(path);
105+
if (guardedReadLink != path) {
106+
// there are one or more hops within the guards so there is nothing more to do
107+
return stats;
108+
}
109+
try {
110+
path = this.unguardedRealPathSync(path);
111+
// there are no hops so lets report the stats of the real file;
112+
// we can't use origRealPathSync here since that function calls lstat internally
113+
// which can result in an infinite loop
114+
return this._originalFsLstat.call(internalFs, path, bigint, undefined, throwIfNoEntry);
115+
}
116+
catch (err) {
117+
if (err.code === 'ENOENT') {
118+
// broken link so there is nothing more to do
119+
return stats;
120+
}
121+
throw err;
122+
}
123+
}
124+
}
125+
exports.FsInternalStatPatcher = FsInternalStatPatcher;

js/private/node-patches/src/BUILD.bazel

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,24 @@ typescript_bin.tsc(
44
name = "compile",
55
srcs = [
66
"fs.cts",
7+
"fs_stat.cts",
8+
"fs_stat_types.d.cts",
79
"tsconfig.json",
810
"//:node_modules/@types/node",
911
],
1012
outs = [
1113
"fs.cjs",
14+
"fs_stat.cjs",
1215
],
1316
args = [
1417
"-p",
1518
"tsconfig.json",
1619
],
1720
chdir = package_name(),
18-
visibility = ["//js/private/test/node-patches:__pkg__"],
21+
visibility = [
22+
"//js/private/node-patches:__pkg__",
23+
"//js/private/test/node-patches:__pkg__",
24+
],
1925
)
2026

2127
genrule(

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type * as FsType from 'fs'
2020
import type * as UrlType from 'url'
2121
import * as path from 'path'
2222
import * as util from 'util'
23+
import { FsInternalStatPatcher } from './fs_stat.cjs'
2324

2425
// windows cant find the right types
2526
type Dir = any
@@ -100,6 +101,19 @@ export function patcher(roots: string[]): () => void {
100101

101102
const { canEscape, isEscape } = escapeFunction(roots)
102103

104+
// =========================================================================
105+
// fsInternal.lstat (to patch ESM resolve's `realpathSync`!)
106+
// =========================================================================
107+
const lstatEsmPatcher = new FsInternalStatPatcher(
108+
{ canEscape, isEscape },
109+
guardedReadLink,
110+
guardedReadLinkSync,
111+
unguardedRealPath,
112+
unguardedRealPathSync
113+
)
114+
115+
lstatEsmPatcher.patch()
116+
103117
// =========================================================================
104118
// fs.lstat
105119
// =========================================================================
@@ -452,7 +466,6 @@ export function patcher(roots: string[]): () => void {
452466

453467
if (promisePropertyDescriptor) {
454468
const promises: any = {}
455-
promises.lstat = util.promisify(fs.lstat)
456469
// NOTE: node core uses the newer realpath function fs.promises.native instead of fs.realPath
457470
promises.realpath = util.promisify(fs.realpath.native)
458471
promises.readlink = util.promisify(fs.readlink)
@@ -867,6 +880,8 @@ export function patcher(roots: string[]): () => void {
867880
if (unpatchPromises) {
868881
unpatchPromises()
869882
}
883+
884+
lstatEsmPatcher.revert()
870885
}
871886
}
872887

@@ -889,7 +904,7 @@ function stringifyPathLike(p: PathLike): string {
889904
}
890905
}
891906

892-
function resolvePathLike(p: PathLike): string {
907+
export function resolvePathLike(p: PathLike): string {
893908
return path.resolve(stringifyPathLike(p))
894909
}
895910

0 commit comments

Comments
 (0)