Skip to content

Commit

Permalink
Wrap require("fs").promises if "fs" is required
Browse files Browse the repository at this point in the history
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
>
  • Loading branch information
hansott committed Oct 15, 2024
1 parent 9c3a9eb commit cdb5751
Showing 1 changed file with 31 additions and 13 deletions.
44 changes: 31 additions & 13 deletions library/sinks/FileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -14,6 +15,8 @@ type FileSystemFunction = {
};

export class FileSystem implements Wrapper {
private patchedPromises = false;

private inspectPath(
args: unknown[],
name: string,
Expand Down Expand Up @@ -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) => {
Expand All @@ -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));
}
}

0 comments on commit cdb5751

Please sign in to comment.