Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ESM support (WIP) #402

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add esm support to multiple sinks
  • Loading branch information
timokoessler committed Oct 4, 2024
commit 50863a761a42606625535ca829c5b88989166965
2 changes: 2 additions & 0 deletions end2end/tests/hono-sqlite3-esm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ t.test("it blocks in blocking mode", (t) => {

let stdout = "";
server.stdout.on("data", (data) => {
console.log(data.toString());
stdout += data.toString();
});

let stderr = "";
server.stderr.on("data", (data) => {
console.log(data.toString());
stderr += data.toString();
});

Expand Down
2 changes: 1 addition & 1 deletion library/agent/hooks/wrapImport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@

patchPackage(exports, name, baseDir);
} catch (error) {
if (error instanceof Error) {
getInstance()?.onFailedToWrapModule(name, error);
}
}

Check warning on line 74 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L71-L74

Added lines #L71 - L74 were not covered by tests
}

function patchBuiltinModule(exports: any, name: string) {
Expand All @@ -96,8 +96,8 @@

function getRelativeImportedFilePath(packageName: string, importName: string) {
if (!importName.startsWith(packageName)) {
return importName;
}

Check warning on line 100 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L99-L100

Added lines #L99 - L100 were not covered by tests
return importName.substring(packageName.length + 1);
}

Expand All @@ -108,13 +108,13 @@
) {
// Ignore .json files
if (importName.endsWith(".json")) {
return;
}

Check warning on line 112 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L111-L112

Added lines #L111 - L112 were not covered by tests

// Required for getting package version
if (!baseDir) {
throw new Error("Can not patch package without baseDir");
}

Check warning on line 117 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L116-L117

Added lines #L116 - L117 were not covered by tests

// Base dir and importName include the package name
// The importName also includes the path to the file inside the imported package
Expand All @@ -122,9 +122,9 @@
// Do not use pathInfo.base because its wrong, use baseDir instead!
const pathInfoForName = getModuleInfoFromPath(`node_modules/${importName}`);
if (!pathInfoForName) {
// Can happen if the package is not inside a node_modules folder, like the dev build of our library itself
return;
}

Check warning on line 127 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L125-L127

Added lines #L125 - L127 were not covered by tests
const packageName = pathInfoForName.name;

const pathInfo: ModulePathInfo = {
Expand All @@ -141,8 +141,8 @@

// We don't want to patch this package because we do not have any hooks for it
if (!versionedPackages.length) {
return;
}

Check warning on line 145 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L144-L145

Added lines #L144 - L145 were not covered by tests

// Get the original require function (if it was wrapped)
const requireFunc = getOrignalRequire() || require;
Expand All @@ -153,10 +153,10 @@
// Get the version of the installed package
const installedPkgVersion = packageJson.version;
if (!installedPkgVersion) {
throw new Error(
`Could not get installed package version for ${packageName} on import`
);
}

Check warning on line 159 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L156-L159

Added lines #L156 - L159 were not covered by tests

// Check if the installed package version is supported (get all matching versioned packages)
const matchingVersionedPackages = versionedPackages.filter((pkg) =>
Expand All @@ -165,17 +165,17 @@

const agent = getInstance();
if (agent) {
// Report to the agent that the package was wrapped or not if it's version is not supported
agent.onPackageWrapped(packageName, {
version: installedPkgVersion,
supported: !!matchingVersionedPackages.length,
});
}

Check warning on line 173 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L168-L173

Added lines #L168 - L173 were not covered by tests

if (!matchingVersionedPackages.length) {
// We don't want to patch this package version
return;
}

Check warning on line 178 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L176-L178

Added lines #L176 - L178 were not covered by tests

const fullFilePath = join(pathInfo.base, pathInfo.path);

Expand All @@ -195,11 +195,11 @@
.map((pkg) => pkg.getRequireInterceptors())
.flat();
} else {
// If its not the main file, we want to check if the want to patch the required file
// If its not the main file, we check if we want to patch the required file
interceptors = matchingVersionedPackages
.map((pkg) => pkg.getRequireFileInterceptor(pathInfo.path) || [])
.flat();
}

Check warning on line 202 in library/agent/hooks/wrapImport.ts

View check run for this annotation

Codecov / codecov/patch

library/agent/hooks/wrapImport.ts#L198-L202

Added lines #L198 - L202 were not covered by tests

executeInterceptors(interceptors, exports, undefined, undefined, {
name: pathInfo.name,
Expand Down
2 changes: 1 addition & 1 deletion library/agent/hooks/wrapRequire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ function patchPackage(this: mod, id: string, originalExports: unknown) {
.map((pkg) => pkg.getRequireInterceptors())
.flat();
} else {
// If its not the main file, we want to check if the want to patch the required file
// If its not the main file, we check if we want to patch the required file
interceptors = matchingVersionedPackages
.map((pkg) => pkg.getRequireFileInterceptor(pathInfo.path) || [])
.flat();
Expand Down
6 changes: 4 additions & 2 deletions library/sinks/AwsSDKVersion2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ReportingAPIForTesting } from "../agent/api/ReportingAPIForTesting";
import { Context, runWithContext } from "../agent/Context";
import { LoggerForTesting } from "../agent/logger/LoggerForTesting";
import { AwsSDKVersion2 } from "./AwsSDKVersion2";
import { isCJS } from "../helpers/isCJS";

// Suppress upgrade to SDK v3 notice
require("aws-sdk/lib/maintenance_mode_message").suppress = true;
Expand Down Expand Up @@ -32,12 +33,13 @@ t.test("it works", async (t) => {
logger,
new ReportingAPIForTesting(),
undefined,
undefined
undefined,
!isCJS()
);

agent.start([new AwsSDKVersion2()]);

const AWS = require("aws-sdk");
const AWS = isCJS() ? require("aws-sdk") : (await import("aws-sdk")).default;

const s3 = new AWS.S3({
region: "us-east-1",
Expand Down
4 changes: 3 additions & 1 deletion library/sinks/AwsSDKVersion2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ export class AwsSDKVersion2 implements Wrapper {
.addPackage("aws-sdk")
.withVersion("^2.0.0")
.onRequire((exports, pkgInfo) => {
wrapNewInstance(exports, "S3", pkgInfo, (instance) => {
const base = pkgInfo.isESMImport ? exports.default : exports;

wrapNewInstance(base, "S3", pkgInfo, (instance) => {
for (const operation of operationsWithKey) {
wrapExport(instance, operation, pkgInfo, {
inspectArgs: (args) => this.inspectS3Operation(args, operation),
Expand Down
8 changes: 6 additions & 2 deletions library/sinks/BetterSQLite3.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ReportingAPIForTesting } from "../agent/api/ReportingAPIForTesting";
import { runWithContext, type Context } from "../agent/Context";
import { LoggerNoop } from "../agent/logger/LoggerNoop";
import { BetterSQLite3 } from "./BetterSQLite3";
import { isCJS } from "../helpers/isCJS";

const dangerousContext: Context = {
remoteAddress: "::1",
Expand Down Expand Up @@ -54,11 +55,14 @@ t.test("it detects SQL injections", async (t) => {
new LoggerNoop(),
new ReportingAPIForTesting(),
undefined,
"lambda"
"lambda",
!isCJS()
);
agent.start([new BetterSQLite3()]);

const betterSqlite3 = require("better-sqlite3");
const betterSqlite3 = isCJS()
? require("better-sqlite3")
: (await import("better-sqlite3")).default;
const db = new betterSqlite3(":memory:");

try {
Expand Down
6 changes: 4 additions & 2 deletions library/sinks/BetterSQLite3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,17 @@ export class BetterSQLite3 implements Wrapper {
.addPackage("better-sqlite3")
.withVersion("^11.0.0 || ^10.0.0 || ^9.0.0 || ^8.0.0")
.onRequire((exports, pkgInfo) => {
const base = pkgInfo.isESMImport ? exports.default : exports;

for (const func of sqlFunctions) {
wrapExport(exports.prototype, func, pkgInfo, {
wrapExport(base.prototype, func, pkgInfo, {
inspectArgs: (args) => {
return this.inspectQuery(`better-sqlite3.${func}`, args);
},
});
}
for (const func of fsPathFunctions) {
wrapExport(exports.prototype, func, pkgInfo, {
wrapExport(base.prototype, func, pkgInfo, {
inspectArgs: (args) => {
return this.inspectPath(`better-sqlite3.${func}`, args);
},
Expand Down
10 changes: 7 additions & 3 deletions library/sinks/ChildProcess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { ReportingAPIForTesting } from "../agent/api/ReportingAPIForTesting";
import { Context, runWithContext } from "../agent/Context";
import { LoggerNoop } from "../agent/logger/LoggerNoop";
import { ChildProcess } from "./ChildProcess";
import { execFile, execFileSync, fork } from "child_process";
import { execFile, execFileSync } from "child_process";
import { isCJS } from "../helpers/isCJS";

const unsafeContext: Context = {
remoteAddress: "::1",
Expand Down Expand Up @@ -36,12 +37,15 @@ t.test("it works", async (t) => {
new LoggerNoop(),
new ReportingAPIForTesting(),
undefined,
"lambda"
"lambda",
!isCJS()
);

agent.start([new ChildProcess()]);

const { exec, execSync, spawn, spawnSync, fork } = require("child_process");
const { exec, execSync, spawn, spawnSync, fork } = isCJS()
? require("child_process")
: await import("child_process");

const runCommandsWithInvalidArgs = () => {
throws(
Expand Down
76 changes: 41 additions & 35 deletions library/sinks/ChildProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,41 +19,47 @@ const PATH_PREFIXES = [
export class ChildProcess implements Wrapper {
wrap(hooks: Hooks) {
hooks.addBuiltinModule("child_process").onRequire((exports, pkgInfo) => {
wrapExport(exports, "exec", pkgInfo, {
inspectArgs: (args) => {
return this.inspectExec(args, "exec");
},
});
wrapExport(exports, "execSync", pkgInfo, {
inspectArgs: (args) => {
return this.inspectExec(args, "execSync");
},
});
wrapExport(exports, "spawn", pkgInfo, {
inspectArgs: (args) => {
return this.inspectSpawn(args, "spawn");
},
});
wrapExport(exports, "spawnSync", pkgInfo, {
inspectArgs: (args) => {
return this.inspectSpawn(args, "spawnSync");
},
});
wrapExport(exports, "execFile", pkgInfo, {
inspectArgs: (args) => {
return this.inspectExecFile(args, "execFile");
},
});
wrapExport(exports, "execFileSync", pkgInfo, {
inspectArgs: (args) => {
return this.inspectExecFile(args, "execFileSync");
},
});
wrapExport(exports, "fork", pkgInfo, {
inspectArgs: (args) => {
return this.inspectFork(args, "fork");
},
});
const toWrap = pkgInfo.isESMImport
? [exports, exports.default]
: [exports];

for (const exportObj of toWrap) {
wrapExport(exportObj, "exec", pkgInfo, {
inspectArgs: (args) => {
return this.inspectExec(args, "exec");
},
});
wrapExport(exportObj, "execSync", pkgInfo, {
inspectArgs: (args) => {
return this.inspectExec(args, "execSync");
},
});
wrapExport(exportObj, "spawn", pkgInfo, {
inspectArgs: (args) => {
return this.inspectSpawn(args, "spawn");
},
});
wrapExport(exportObj, "spawnSync", pkgInfo, {
inspectArgs: (args) => {
return this.inspectSpawn(args, "spawnSync");
},
});
wrapExport(exportObj, "execFile", pkgInfo, {
inspectArgs: (args) => {
return this.inspectExecFile(args, "execFile");
},
});
wrapExport(exportObj, "execFileSync", pkgInfo, {
inspectArgs: (args) => {
return this.inspectExecFile(args, "execFileSync");
},
});
wrapExport(exportObj, "fork", pkgInfo, {
inspectArgs: (args) => {
return this.inspectFork(args, "fork");
},
});
}
});
}

Expand Down
124 changes: 66 additions & 58 deletions library/sinks/Fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,39 @@ import { LoggerNoop } from "../agent/logger/LoggerNoop";
import { wrap } from "../helpers/wrap";
import { Fetch } from "./Fetch";
import * as dns from "dns";
import { isCJS } from "../helpers/isCJS";

const calls: Record<string, number> = {};
wrap(dns, "lookup", function lookup(original) {
return function lookup() {
const hostname = arguments[0];

if (!calls[hostname]) {
calls[hostname] = 0;
}
if (isCJS()) {
wrap(dns, "lookup", function lookup(original) {
return function lookup() {
const hostname = arguments[0];

calls[hostname]++;
if (!calls[hostname]) {
calls[hostname] = 0;
}

if (hostname === "thisdomainpointstointernalip.com") {
return original.apply(this, [
"localhost",
...Array.from(arguments).slice(1),
]);
}
calls[hostname]++;

if (hostname === "example,prefix.thisdomainpointstointernalip.com") {
return original.apply(this, [
"localhost",
...Array.from(arguments).slice(1),
]);
}
if (hostname === "thisdomainpointstointernalip.com") {
return original.apply(this, [
"localhost",
...Array.from(arguments).slice(1),
]);
}

original.apply(this, arguments);
};
});
if (hostname === "example,prefix.thisdomainpointstointernalip.com") {
return original.apply(this, [
"localhost",
...Array.from(arguments).slice(1),
]);
}

original.apply(this, arguments);
};
});
}

const context: Context = {
remoteAddress: "::1",
Expand Down Expand Up @@ -76,7 +80,8 @@ t.test(
new LoggerNoop(),
api,
new Token("123"),
undefined
undefined,
!isCJS()
);
agent.start([new Fetch()]);

Expand Down Expand Up @@ -154,47 +159,50 @@ t.test(
}
});

await runWithContext(
{
...context,
...{
body: {
image2: [
"http://example",
"prefix.thisdomainpointstointernalip.com",
],
image: "http://thisdomainpointstointernalip.com/path",
// Because we need to wrap dns here in the test
if (isCJS()) {
await runWithContext(
{
...context,
...{
body: {
image2: [
"http://example",
"prefix.thisdomainpointstointernalip.com",
],
image: "http://thisdomainpointstointernalip.com/path",
},
},
},
},
async () => {
const error = await t.rejects(() =>
fetch("http://thisdomainpointstointernalip.com")
);
if (error instanceof Error) {
t.same(
// @ts-expect-error Type is not defined
error.cause.message,
"Zen has blocked a server-side request forgery: fetch(...) originating from body.image"
async () => {
const error = await t.rejects(() =>
fetch("http://thisdomainpointstointernalip.com")
);
}
if (error instanceof Error) {
t.same(
// @ts-expect-error Type is not defined
error.cause.message,
"Zen has blocked a server-side request forgery: fetch(...) originating from body.image"
);
}

const error2 = await t.rejects(() =>
fetch(["http://example", "prefix.thisdomainpointstointernalip.com"])
);
if (error2 instanceof Error) {
t.same(
// @ts-expect-error Type is not defined
error2.cause.message,
"Zen has blocked a server-side request forgery: fetch(...) originating from body.image2"
const error2 = await t.rejects(() =>
fetch(["http://example", "prefix.thisdomainpointstointernalip.com"])
);
}
if (error2 instanceof Error) {
t.same(
// @ts-expect-error Type is not defined
error2.cause.message,
"Zen has blocked a server-side request forgery: fetch(...) originating from body.image2"
);
}

// Ensure the lookup is only called once per hostname
// Otherwise, it could be vulnerable to TOCTOU
t.same(calls["thisdomainpointstointernalip.com"], 1);
}
);
// Ensure the lookup is only called once per hostname
// Otherwise, it could be vulnerable to TOCTOU
t.same(calls["thisdomainpointstointernalip.com"], 1);
}
);
}

await runWithContext(
{
Expand Down
Loading