Node 18 undici global

[Marsup]

Support plan

• is this issue currently blocking your project? (yes/no): No
• is this issue affecting a production system? (yes/no): No

Context

• node version: 18
• module version with issue: 25.0.1
• last module version without issue: 🤷🏻
• environment (e.g. node, browser, native): node
• used with (e.g. hapi application, another framework, standalone, ...): standalone
• any other relevant information:

What are you trying to achieve or the steps to reproduce?

It looks like node's global detection is triggered on node@18 under certain conditions: https://github.com/Marsup/hapi-mongodb/actions/runs/3227815184/jobs/5283098977
undici is not explicitly installed except for its presence in node's fetch implementation.

What was the result you got?

A global leak

What result did you expect?

Node's global to be excluded

[kanongil]

I had a look into this, since I had already encountered the rogue global.

The global was added here to fix nodejs/undici#1331 and nodejs/node#42814. Interestingly, it only appears once some code has tried to use the experimental built-in implementation of fetch().

Essentially it allows the user-space undici package to expose a setGlobalDispatcher() method, that can customise how the new fetch() method handles requests.

I guess lab just needs to whitelist Symbol.for('undici.globalDispatcher.1') to fix this.

[devinivy]

It's noted in undici that the 1 is technically a version number, and will probably change in the future: https://github.com/nodejs/node/blob/5fad0b93667ffc6e4def52996b9529ac99b26319/deps/undici/src/lib/global.js#L3-L5. Would it be too weird for our automatic safe list to attempt to use fetch() in a way that eagerly fails? E.g. this usage await fetch() will fail with ERR_INVALID_URL but still set the global. This occurs in a worker thread, so I believe it shouldn't affect the globals on the main worker thread:

lab/lib/modules/leaks.js

Lines 112 to 126 in 0542146

	internals.getNodeGlobals = async () => {
	const nodeGlobalsWorker = new WorkerThreads.Worker(__filename);
	const [nodeGlobals] = await Events.once(nodeGlobalsWorker, 'message');
	return nodeGlobals;
	};
	if (!WorkerThreads.isMainThread) {
	// When this module is used as a worker, it posts back global property names and symbols
	WorkerThreads.parentPort.postMessage({
	allowed: Object.getOwnPropertyNames(globalThis),
	symbols: Object.getOwnPropertySymbols(globalThis).map(String)
	});
	}

[kanongil]

It's curious that node wants to pollute the global js scope, when it could have been solved by exposing a virtual node:undici module.

[devinivy]

I read that they wont consider adding it until fetch() is no longer experimental.

[kanongil]

@devinivy I think that is overkill.

The number is very unlikely to change during a major, and for majors we usually have to update the globals whitelist anyway.

[dominykas]

Took a stab at implementation - there's a lazy require of undici when accessing FormData (and some other globals), which avoids having to call fetch() just to get it to throw.