From: Jérôme Benoit Date: Wed, 14 Oct 2026 21:37:23 +0000 (+0200) Subject: fix: prevent inherited worker launch options X-Git-Url: https://git.piment-noir.org/?a=commitdiff_plain;h=0ce743a1bcc282f0e8f822b58455543d2867ba74;p=poolifier.git fix: prevent inherited worker launch options - harden inherited worker launch options - close worker launch hardening gaps - preserve Windows worker environment precedence --- diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 34df72e51..c05997a04 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -906,9 +906,12 @@ export abstract class AbstractPool< throw new Error('Cannot start a destroying pool') } this.starting = true - this.startMinimumNumberOfWorkers() - this.startTimestamp = performance.now() - this.starting = false + try { + this.startMinimumNumberOfWorkers() + this.startTimestamp = performance.now() + } finally { + this.starting = false + } this.started = true } @@ -1626,7 +1629,7 @@ export abstract class AbstractPool< this.worker, this.filePath, { - env: this.opts.env, + env: Object.hasOwn(this.opts, 'env') ? this.opts.env : undefined, tasksQueueAgingFactor: this.opts.tasksQueueOptions?.agingFactor, tasksQueueBackPressureSize: this.opts.tasksQueueOptions?.size ?? @@ -1636,7 +1639,9 @@ export abstract class AbstractPool< tasksQueueBucketSize: defaultBucketSize, tasksQueueLoadExponent: this.opts.tasksQueueOptions?.loadExponent, tasksQueuePriority: this.getTasksQueuePriority(), - workerOptions: this.opts.workerOptions, + workerOptions: Object.hasOwn(this.opts, 'workerOptions') + ? this.opts.workerOptions + : undefined, } ) // Flag the worker node as ready at pool startup. @@ -2490,18 +2495,21 @@ export abstract class AbstractPool< return } this.startingMinimumNumberOfWorkers = true - while ( - this.workerNodes.reduce( - (accumulator, workerNode) => - !workerNode.info.dynamic ? accumulator + 1 : accumulator, - 0 - ) < this.minimumNumberOfWorkers - ) { - const workerNodeKey = this.createAndSetupWorkerNode() - initWorkerNodeUsage && - this.initWorkerNodeUsage(this.workerNodes[workerNodeKey]) + try { + while ( + this.workerNodes.reduce( + (accumulator, workerNode) => + !workerNode.info.dynamic ? accumulator + 1 : accumulator, + 0 + ) < this.minimumNumberOfWorkers + ) { + const workerNodeKey = this.createAndSetupWorkerNode() + initWorkerNodeUsage && + this.initWorkerNodeUsage(this.workerNodes[workerNodeKey]) + } + } finally { + this.startingMinimumNumberOfWorkers = false } - this.startingMinimumNumberOfWorkers = false } private readonly stealTask = ( diff --git a/src/pools/cluster/fixed.ts b/src/pools/cluster/fixed.ts index 3f9f51506..3548b6cab 100644 --- a/src/pools/cluster/fixed.ts +++ b/src/pools/cluster/fixed.ts @@ -4,6 +4,7 @@ import type { MessageValue } from '../../utility-types.js' import { AbstractPool } from '../abstract-pool.js' import { type PoolOptions, type PoolType, PoolTypes } from '../pool.js' +import { buildClusterSettings } from '../utils.js' import { type WorkerType, WorkerTypes } from '../worker.js' /** @@ -117,7 +118,13 @@ export class FixedClusterPool< /** @inheritDoc */ protected override setupHook (): void { - cluster.setupPrimary({ ...this.opts.settings, exec: this.filePath }) + cluster.setupPrimary( + buildClusterSettings( + cluster.settings, + Object.hasOwn(this.opts, 'settings') ? this.opts.settings : undefined, + this.filePath + ) + ) } /** @inheritDoc */ diff --git a/src/pools/utils.ts b/src/pools/utils.ts index 4f6b48159..3f9c4ac29 100644 --- a/src/pools/utils.ts +++ b/src/pools/utils.ts @@ -1,4 +1,7 @@ -import cluster, { Worker as ClusterWorker } from 'node:cluster' +import cluster, { + type ClusterSettings, + Worker as ClusterWorker, +} from 'node:cluster' import { existsSync } from 'node:fs' import { SHARE_ENV, @@ -41,6 +44,367 @@ export const DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS: Readonly( + ...sources: (Partial | undefined)[] +): T => { + const target = Object.create(null) as T + Object.assign(target, ...sources) + return target +} + +const defineEnumerableDataProperty = ( + target: object, + key: PropertyKey, + value: unknown +): void => { + const descriptor = Object.create(null) as PropertyDescriptor + descriptor.configurable = true + descriptor.enumerable = true + descriptor.value = value + descriptor.writable = true + Object.defineProperty(target, key, descriptor) +} + +const cloneDenseExecArgv = ( + execArgv: readonly string[], + optionName: string +): string[] => { + const clone = new Array(execArgv.length) + for (let index = 0; index < execArgv.length; index++) { + if (!Object.hasOwn(execArgv, index)) { + throw new TypeError(`Invalid ${optionName}: must not be a sparse array`) + } + defineEnumerableDataProperty(clone, index, execArgv[index]) + } + return clone +} + +const cloneArrayOwnSlots = (values: readonly T[]): T[] => { + const clone = new Array(values.length) + for (let index = 0; index < values.length; index++) { + if (Object.hasOwn(values, index)) { + defineEnumerableDataProperty(clone, index, values[index]) + } + } + Object.setPrototypeOf(clone, null) + return clone +} + +const materializeArray = (values: readonly T[]): (T | undefined)[] => { + const clone = new Array(values.length) + for (let index = 0; index < values.length; index++) { + defineEnumerableDataProperty( + clone, + index, + Object.hasOwn(values, index) ? values[index] : undefined + ) + } + return clone +} + +const isOwnPlainObject = (value: unknown): value is object => { + if (typeof value !== 'object' || value === null) { + return false + } + const prototype = Object.getPrototypeOf(value) as null | object + return prototype === null || prototype === Object.prototype +} + +const normalizeStdioDescriptor = (value: unknown): unknown => { + if (!isOwnPlainObject(value)) { + return value + } + const descriptor = Object.create(null) as Record + for (const key of ['fd', 'handle', '_handle']) { + defineEnumerableDataProperty(descriptor, key, undefined) + } + for (const key of Reflect.ownKeys(value)) { + const propertyDescriptor = Object.getOwnPropertyDescriptor(value, key) + if (propertyDescriptor != null) { + Object.setPrototypeOf(propertyDescriptor, null) + Object.defineProperty(descriptor, key, propertyDescriptor) + } + } + return descriptor +} + +const cloneStdio = (stdio: readonly unknown[]): unknown[] => { + const clone = new Array(stdio.length) + for (let index = 0; index < stdio.length; index++) { + defineEnumerableDataProperty( + clone, + index, + Object.hasOwn(stdio, index) + ? normalizeStdioDescriptor(stdio[index]) + : undefined + ) + } + return clone +} + +const getPropertyOwner = (value: object, key: string): null | object => { + let owner: null | object = value + while (owner !== null && !Object.hasOwn(owner, key)) { + owner = Object.getPrototypeOf(owner) as null | object + } + return owner +} + +const getStdioControlValue = ( + entry: object, + key: string, + owner: null | object +): unknown => { + if (owner !== Object.prototype) { + return owner === null ? undefined : Reflect.get(entry, key) + } + const descriptor = Object.getOwnPropertyDescriptor(Object.prototype, key) + if (descriptor == null) { + return undefined + } + if (!Object.hasOwn(descriptor, 'value')) { + throw new TypeError( + `Invalid cluster.settings.stdio entry: inherited accessor '${key}' from Object.prototype is not allowed` + ) + } + return descriptor.value +} + +/** + * Materializes missing cluster stdio slots and validates object-valued entries + * before Node reads them. + * @param stdio - Effective cluster stdio configuration. + * @internal + */ +export const validateAndMaterializeClusterStdio = (stdio: unknown): void => { + if (!Array.isArray(stdio)) { + return + } + for (let index = 0; index < stdio.length; index++) { + if (!Object.hasOwn(stdio, index)) { + defineEnumerableDataProperty(stdio, index, undefined) + } + } + for (const entry of stdio as unknown[]) { + if (typeof entry !== 'object' || entry === null) { + continue + } + const fdOwner = getPropertyOwner(entry, 'fd') + const fdValue = getStdioControlValue(entry, 'fd', fdOwner) + if (typeof fdValue === 'number') { + if (fdOwner === Object.prototype) { + throw new TypeError( + "Invalid cluster.settings.stdio entry: inherited 'fd' from Object.prototype is not allowed" + ) + } + continue + } + for (const key of ['handle', '_handle']) { + const owner = getPropertyOwner(entry, key) + const value = getStdioControlValue(entry, key, owner) + if ( + owner === Object.prototype && + ((typeof value === 'object' && value !== null) || + typeof value === 'function') + ) { + throw new TypeError( + `Invalid cluster.settings.stdio entry: inherited '${key}' from Object.prototype is not allowed` + ) + } + } + } +} + +const normalizeResourceLimits = (resourceLimits: object): object => { + const normalizedResourceLimits = Object.create(null) as Record< + string, + unknown + > + for (const key of [ + 'maxYoungGenerationSizeMb', + 'maxOldGenerationSizeMb', + 'codeRangeSizeMb', + 'stackSizeMb', + ]) { + if (Object.hasOwn(resourceLimits, key)) { + defineEnumerableDataProperty( + normalizedResourceLimits, + key, + Reflect.get(resourceLimits, key) + ) + } + } + return normalizedResourceLimits +} + +/** + * Builds the environment passed through Node's cluster boundary. + * @param processEnv - The process environment. + * @param workerEnv - Explicit worker environment entries. + * @param caseInsensitive - Whether environment names are case-insensitive. + * @returns A normalized environment without a prototype. + * @internal + */ +export const buildClusterWorkerEnvironment = ( + processEnv: NodeJS.ProcessEnv, + workerEnv: Record | undefined, + caseInsensitive = process.platform === 'win32' +): Record => { + if (!caseInsensitive) { + const environment = withNullPrototype>( + processEnv, + workerEnv + ) + for (const key of Object.keys(Object.prototype)) { + if (!Object.hasOwn(environment, key)) { + environment[key] = undefined + } + } + return environment + } + + const effectiveEntries = new Map() + for (const source of [processEnv, workerEnv]) { + if (source == null) { + continue + } + const sourceEntries = new Map() + for (const key of Object.keys(source)) { + const logicalKey = key.toUpperCase() + const previousKey = sourceEntries.get(logicalKey) + if (previousKey == null || key < previousKey) { + sourceEntries.set(logicalKey, key) + } + } + for (const [logicalKey, sourceKey] of sourceEntries) { + const previousEntry = effectiveEntries.get(logicalKey) + const key = + previousEntry != null && previousEntry.key < sourceKey + ? previousEntry.key + : sourceKey + effectiveEntries.set(logicalKey, { key, value: source[sourceKey] }) + } + } + + const prototypeEntries = new Map() + for (const key of Object.keys(Object.prototype)) { + const logicalKey = key.toUpperCase() + const previousKey = prototypeEntries.get(logicalKey) + if (previousKey == null || key < previousKey) { + prototypeEntries.set(logicalKey, key) + } + } + + const environment = Object.create(null) as Record + for (const [logicalKey, effectiveEntry] of effectiveEntries) { + const prototypeKey = prototypeEntries.get(logicalKey) + const key = + prototypeKey != null && prototypeKey < effectiveEntry.key + ? prototypeKey + : effectiveEntry.key + environment[key] = effectiveEntry.value + } + for (const [logicalKey, prototypeKey] of prototypeEntries) { + if (!effectiveEntries.has(logicalKey)) { + environment[prototypeKey] = undefined + } + } + return environment +} + +/** + * Builds cluster settings safe for Node's ordinary-object reconstruction. + * @param currentSettings - Current Node cluster settings. + * @param settings - Explicit pool cluster settings. + * @param exec - Pool worker executable. + * @returns Normalized cluster settings. + * @internal + */ +export const buildClusterSettings = ( + currentSettings: ClusterSettings | undefined, + settings: ClusterSettings | undefined, + exec: string +): ClusterSettings => { + const normalizedSettings = withNullPrototype( + { + cwd: undefined, + gid: undefined, + inspectPort: undefined, + serialization: undefined, + stdio: undefined, + uid: undefined, + windowsHide: undefined, + }, + currentSettings, + settings, + { exec } + ) + if (Array.isArray(normalizedSettings.args)) { + normalizedSettings.args = materializeArray( + normalizedSettings.args + ) as string[] + } + if (Array.isArray(normalizedSettings.execArgv)) { + normalizedSettings.execArgv = cloneDenseExecArgv( + normalizedSettings.execArgv, + 'settings.execArgv' + ) + } + if (Array.isArray(normalizedSettings.stdio)) { + normalizedSettings.stdio = cloneStdio(normalizedSettings.stdio) + } + return normalizedSettings +} + +/** + * Builds options for a worker thread launch. + * @param workerOptions - Explicit worker options. + * @returns Normalized worker options. + * @internal + */ +export const buildThreadWorkerOptions = ( + workerOptions: undefined | WorkerOptions +): WorkerOptions => { + const normalizedWorkerOptions = withNullPrototype( + { env: SHARE_ENV }, + workerOptions + ) + if (Array.isArray(normalizedWorkerOptions.argv)) { + normalizedWorkerOptions.argv = cloneArrayOwnSlots( + normalizedWorkerOptions.argv + ) + } + if (Array.isArray(normalizedWorkerOptions.execArgv)) { + normalizedWorkerOptions.execArgv = cloneDenseExecArgv( + normalizedWorkerOptions.execArgv, + 'workerOptions.execArgv' + ) + } + if ( + Object.hasOwn(normalizedWorkerOptions, 'resourceLimits') && + typeof normalizedWorkerOptions.resourceLimits === 'object' && + // Node accepts null as the absence of resource limits at runtime. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + normalizedWorkerOptions.resourceLimits !== null + ) { + normalizedWorkerOptions.resourceLimits = normalizeResourceLimits( + normalizedWorkerOptions.resourceLimits + ) + } + if (Array.isArray(normalizedWorkerOptions.transferList)) { + normalizedWorkerOptions.transferList = materializeArray( + normalizedWorkerOptions.transferList + ) as NonNullable + } + return normalizedWorkerOptions +} + export const getDefaultTasksQueueOptions = ( poolMaxSize: number ): Required> => { @@ -470,12 +834,15 @@ export const createWorker = ( ): Worker => { switch (type) { case WorkerTypes.cluster: - return cluster.fork(opts.env) as unknown as Worker + validateAndMaterializeClusterStdio(cluster.settings.stdio) + return cluster.fork( + buildClusterWorkerEnvironment(process.env, opts.env) + ) as unknown as Worker case WorkerTypes.thread: - return new ThreadWorker(filePath, { - env: SHARE_ENV, - ...opts.workerOptions, - }) as unknown as Worker + return new ThreadWorker( + filePath, + buildThreadWorkerOptions(opts.workerOptions) + ) as unknown as Worker default: // eslint-disable-next-line @typescript-eslint/restrict-template-expressions throw new Error(`Unknown worker type '${type}'`) diff --git a/tests/pools/prototype-pollution.test.mjs b/tests/pools/prototype-pollution.test.mjs new file mode 100644 index 000000000..b851caccb --- /dev/null +++ b/tests/pools/prototype-pollution.test.mjs @@ -0,0 +1,410 @@ +import cluster from 'node:cluster' +import { randomUUID } from 'node:crypto' +import { once } from 'node:events' +import { existsSync, rmSync } from 'node:fs' +import { createConnection, createServer } from 'node:net' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' + +import { + DynamicThreadPool, + FixedClusterPool, + FixedThreadPool, +} from '../../lib/index.mjs' + +const clusterWorker = './tests/worker-files/cluster/testWorker.cjs' +const clusterIntrospectionWorker = resolve( + './tests/worker-files/cluster/prototypePollutionWorker.cjs' +) +const environmentPollutionKey = 'POOLIFIER_ARBITRARY_ENV_POLLUTION' +const pollutedProperties = [ + '0', + '4', + 'cwd', + 'env', + environmentPollutionKey, + 'execArgv', + 'fd', + 'handle', + 'maxOldGenerationSizeMb', + 'NODE_OPTIONS', + 'settings', + 'workerOptions', +] +const securityMarkers = new Set() +const threadWorker = './tests/worker-files/thread/testWorker.mjs' + +const clearPrototypePollution = () => { + for (const property of pollutedProperties) { + Reflect.deleteProperty(Object.prototype, property) + } +} + +const polluteObjectPrototype = (property, value) => { + // eslint-disable-next-line no-extend-native + Object.defineProperty(Object.prototype, property, { + configurable: true, + enumerable: true, + value, + writable: true, + }) +} + +const createImportPayload = () => { + const marker = join( + tmpdir(), + `poolifier-prototype-pollution-${process.pid.toString()}-${randomUUID()}` + ) + securityMarkers.add(marker) + const source = `import { writeFileSync } from 'node:fs'; writeFileSync(${JSON.stringify(marker)}, 'executed')` + return { + importOption: `--import=data:text/javascript,${encodeURIComponent(source)}`, + marker, + } +} + +const waitFor = async (predicate, timeout = 5000) => { + const deadline = Date.now() + timeout + while (!predicate()) { + if (Date.now() >= deadline) { + throw new Error('Timed out waiting for condition') + } + await new Promise(resolve => setTimeout(resolve, 20)) + } +} + +afterEach(() => { + clearPrototypePollution() + for (const marker of securityMarkers) { + rmSync(marker, { force: true }) + } + securityMarkers.clear() +}) + +describe('Worker option prototype pollution', () => { + it('ignores inherited thread worker options', async () => { + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('workerOptions', { execArgv: [importOption] }) + const pool = new FixedThreadPool(1, threadWorker) + try { + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + expect(existsSync(marker)).toBe(false) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('ignores inherited properties in explicit thread worker options', async () => { + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('execArgv', [importOption]) + const pool = new FixedThreadPool(1, threadWorker, { workerOptions: {} }) + try { + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + expect(existsSync(marker)).toBe(false) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('ignores inherited thread resource limits', async () => { + polluteObjectPrototype('maxOldGenerationSizeMb', 'invalid') + const pool = new FixedThreadPool(1, threadWorker, { + workerOptions: { resourceLimits: {} }, + }) + try { + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('preserves null thread resource limits', async () => { + const pool = new FixedThreadPool(1, threadWorker, { + workerOptions: { resourceLimits: null }, + }) + try { + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + } finally { + await pool.destroy() + } + }) + + it('ignores thread worker options polluted after dynamic pool construction', async () => { + const pool = new DynamicThreadPool(1, 2, threadWorker) + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('workerOptions', { execArgv: [importOption] }) + try { + await Promise.all(Array.from({ length: 4 }, () => pool.execute())) + expect(existsSync(marker)).toBe(false) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('ignores inherited cluster worker environment options', async () => { + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('env', { NODE_OPTIONS: importOption }) + const pool = new FixedClusterPool(1, clusterWorker) + try { + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + expect(existsSync(marker)).toBe(false) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('ignores inherited properties in explicit cluster worker environment options', async () => { + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('NODE_OPTIONS', importOption) + const pool = new FixedClusterPool(1, clusterWorker, { env: {} }) + try { + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + expect(existsSync(marker)).toBe(false) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('ignores inherited cluster settings', async () => { + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('settings', { execArgv: [importOption] }) + const pool = new FixedClusterPool(1, clusterWorker) + try { + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + expect(existsSync(marker)).toBe(false) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('ignores inherited properties in explicit cluster settings', async () => { + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('execArgv', [importOption]) + const pool = new FixedClusterPool(1, clusterWorker, { settings: {} }) + try { + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + expect(existsSync(marker)).toBe(false) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('ignores cluster worker environment options polluted before replacement', async () => { + const pool = new FixedClusterPool(1, clusterWorker) + await pool.execute() + const workerId = pool.workerNodes[0].info.id + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('env', { NODE_OPTIONS: importOption }) + try { + await pool.destroyWorkerNode(0) + await waitFor( + () => + pool.workerNodes[0]?.info.ready === true && + pool.workerNodes[0]?.info.id !== workerId + ) + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + expect(existsSync(marker)).toBe(false) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('shadows arbitrary inherited cluster worker environment keys', async () => { + polluteObjectPrototype(environmentPollutionKey, 'polluted') + const pool = new FixedClusterPool(1, clusterIntrospectionWorker) + try { + await expect( + pool.execute({ envKey: environmentPollutionKey }) + ).resolves.toMatchObject({ hasEnvKey: false }) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('shadows inherited cwd for initial and replacement cluster workers', async () => { + polluteObjectPrototype('cwd', tmpdir()) + const pool = new FixedClusterPool(1, clusterIntrospectionWorker) + try { + await expect(pool.execute()).resolves.toMatchObject({ + cwd: process.cwd(), + }) + const workerId = pool.workerNodes[0].info.id + await pool.destroyWorkerNode(0) + await waitFor( + () => + pool.workerNodes[0]?.info.ready === true && + pool.workerNodes[0]?.info.id !== workerId + ) + await expect(pool.execute()).resolves.toMatchObject({ + cwd: process.cwd(), + }) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('materializes deleted stdio slots before cluster replacement', async () => { + const stdio = ['ignore', 'ignore', 'ignore', 'ipc', undefined] + const pool = new FixedClusterPool(1, clusterWorker, { + settings: { stdio }, + }) + try { + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + const workerId = pool.workerNodes[0].info.id + Reflect.deleteProperty(cluster.settings.stdio, 4) + polluteObjectPrototype('4', 'ipc') + await pool.destroyWorkerNode(0) + await waitFor( + () => + pool.workerNodes[0]?.info.ready === true && + pool.workerNodes[0]?.info.id !== workerId + ) + expect(Object.hasOwn(cluster.settings.stdio, 4)).toBe(true) + expect(cluster.settings.stdio[4]).toBeUndefined() + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + } finally { + clearPrototypePollution() + await pool.destroy() + } + }) + + it('rejects late Object.prototype stdio controls at cluster launch', async () => { + const server = createServer() + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', resolve) + }) + const connectionPromise = once(server, 'connection') + const clientSocket = createConnection(server.address().port, '127.0.0.1') + await once(clientSocket, 'connect') + const [serverSocket] = await connectionPromise + const clientSocketClosed = once(clientSocket, 'close') + const serverSocketClosed = once(serverSocket, 'close') + const previousStdio = cluster.settings.stdio + const pool = new FixedClusterPool(1, clusterWorker, { + settings: { + stdio: ['ignore', clientSocket, 'ignore', 'ipc'], + }, + startWorkers: false, + }) + polluteObjectPrototype('fd', -1) + try { + expect(() => pool.start()).toThrow( + new TypeError( + "Invalid cluster.settings.stdio entry: inherited 'fd' from Object.prototype is not allowed" + ) + ) + clearPrototypePollution() + polluteObjectPrototype('handle', clientSocket._handle) + expect(() => pool.start()).toThrow( + new TypeError( + "Invalid cluster.settings.stdio entry: inherited 'handle' from Object.prototype is not allowed" + ) + ) + clearPrototypePollution() + polluteObjectPrototype('handle', 'polluted') + pool.start() + clearPrototypePollution() + await expect(pool.execute()).resolves.toStrictEqual({ ok: 1 }) + } finally { + clearPrototypePollution() + if (pool.info.started) { + await pool.destroy() + } + cluster.setupPrimary({ stdio: previousStdio }) + clientSocket.destroy() + serverSocket.destroy() + await Promise.all([clientSocketClosed, serverSocketClosed]) + await new Promise((resolve, reject) => { + server.close(error => { + if (error != null) { + reject(error) + return + } + resolve() + }) + }) + } + }) + + it('rejects sparse thread worker execArgv before reading inherited slots', () => { + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('0', importOption) + expect( + () => + new FixedThreadPool(1, threadWorker, { + workerOptions: { execArgv: new Array(1) }, + }) + ).toThrow( + new TypeError( + 'Invalid workerOptions.execArgv: must not be a sparse array' + ) + ) + expect(existsSync(marker)).toBe(false) + }) + + it('rejects sparse cluster execArgv before reading inherited slots', () => { + const { importOption, marker } = createImportPayload() + polluteObjectPrototype('0', importOption) + expect( + () => + new FixedClusterPool(1, clusterWorker, { + settings: { execArgv: new Array(1) }, + }) + ).toThrow( + new TypeError('Invalid settings.execArgv: must not be a sparse array') + ) + expect(existsSync(marker)).toBe(false) + }) + + it.runIf(process.platform === 'win32')( + 'preserves worker precedence across the Windows cluster environment merge', + async () => { + const { importOption, marker } = createImportPayload() + const nodeOptionsEntries = Object.entries(process.env).filter( + ([key]) => key.toUpperCase() === 'NODE_OPTIONS' + ) + for (const [key] of nodeOptionsEntries) { + Reflect.deleteProperty(process.env, key) + } + process.env.NODE_OPTIONS = importOption + let pool + try { + pool = new FixedClusterPool(1, clusterIntrospectionWorker, { + env: { node_options: '--no-warnings' }, + }) + await expect( + pool.execute({ envKey: 'NODE_OPTIONS' }) + ).resolves.toMatchObject({ + envValue: '--no-warnings', + hasEnvKey: true, + }) + expect(existsSync(marker)).toBe(false) + } finally { + for (const key of Object.keys(process.env)) { + if (key.toUpperCase() === 'NODE_OPTIONS') { + Reflect.deleteProperty(process.env, key) + } + } + for (const [key, value] of nodeOptionsEntries) { + process.env[key] = value + } + if (pool != null) { + await pool.destroy() + } + } + } + ) +}) diff --git a/tests/pools/utils.test.mjs b/tests/pools/utils.test.mjs index cc3e980ea..561b68feb 100644 --- a/tests/pools/utils.test.mjs +++ b/tests/pools/utils.test.mjs @@ -1,16 +1,21 @@ import { Worker as ClusterWorker } from 'node:cluster' -import { Worker as ThreadWorker } from 'node:worker_threads' +import { PassThrough } from 'node:stream' +import { SHARE_ENV, Worker as ThreadWorker } from 'node:worker_threads' import { describe, expect, it } from 'vitest' import { CircularBuffer } from '../../lib/circular-buffer.mjs' import { WorkerTypes } from '../../lib/index.mjs' import { + buildClusterSettings, + buildClusterWorkerEnvironment, + buildThreadWorkerOptions, checkValidWorkerNodeKeys, createWorker, DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS, getDefaultTasksQueueOptions, initWorkerInfo, updateMeasurementStatistics, + validateAndMaterializeClusterStdio, } from '../../lib/pools/utils.mjs' import { MeasurementHistorySize } from '../../lib/pools/worker.mjs' @@ -37,6 +42,493 @@ describe('Pool utils test suite', () => { }) }) + it('builds cluster worker environments from own properties', () => { + const inheritedEnvironment = { INHERITED_SOURCE_KEY: 'inherited' } + const processEnvironment = Object.assign( + Object.create(inheritedEnvironment), + { + BASE_KEY: 'base', + OVERRIDDEN_KEY: 'base', + } + ) + const workerEnvironment = Object.assign( + Object.create(inheritedEnvironment), + { + OVERRIDDEN_KEY: 'worker', + WORKER_KEY: 'worker', + } + ) + const pollutionKey = 'POOLIFIER_ARBITRARY_ENV_POLLUTION' + // eslint-disable-next-line no-extend-native + Object.defineProperty(Object.prototype, pollutionKey, { + configurable: true, + enumerable: true, + value: 'polluted', + writable: true, + }) + try { + const environment = buildClusterWorkerEnvironment( + processEnvironment, + workerEnvironment, + false + ) + expect(Object.getPrototypeOf(environment)).toBeNull() + expect(environment).toMatchObject({ + BASE_KEY: 'base', + OVERRIDDEN_KEY: 'worker', + WORKER_KEY: 'worker', + }) + expect(Object.hasOwn(environment, 'INHERITED_SOURCE_KEY')).toBe(false) + expect(Object.hasOwn(environment, pollutionKey)).toBe(true) + expect(environment[pollutionKey]).toBeUndefined() + } finally { + Reflect.deleteProperty(Object.prototype, pollutionKey) + } + }) + + it('preserves explicit Windows environment precedence by logical key', () => { + const processEnvironment = { + NODE_OPTIONS: '--no-deprecation', + PATH: 'base-path', + } + const environment = buildClusterWorkerEnvironment( + processEnvironment, + { + node_options: '--no-warnings', + path: 'worker-path', + }, + true + ) + expect(Object.getPrototypeOf(environment)).toBeNull() + expect( + Object.keys(environment).filter( + key => key.toUpperCase() === 'NODE_OPTIONS' + ) + ).toStrictEqual(['NODE_OPTIONS']) + expect(environment.NODE_OPTIONS).toBe('--no-warnings') + expect( + Object.keys(environment).filter(key => key.toUpperCase() === 'PATH') + ).toStrictEqual(['PATH']) + expect(environment.PATH).toBe('worker-path') + + const environmentWithRemovedOption = buildClusterWorkerEnvironment( + processEnvironment, + { node_options: undefined }, + true + ) + expect(Object.hasOwn(environmentWithRemovedOption, 'NODE_OPTIONS')).toBe( + true + ) + expect(environmentWithRemovedOption.NODE_OPTIONS).toBeUndefined() + expect(Object.hasOwn(environmentWithRemovedOption, 'node_options')).toBe( + false + ) + }) + + it('uses Object.prototype keys to mask Windows environment variants', () => { + // eslint-disable-next-line no-extend-native + Object.defineProperty(Object.prototype, 'NODE_OPTIONS', { + configurable: true, + enumerable: true, + value: undefined, + writable: true, + }) + try { + const environment = buildClusterWorkerEnvironment( + { node_options: '--no-deprecation' }, + undefined, + true + ) + expect(Object.keys(environment)).toStrictEqual(['NODE_OPTIONS']) + expect(environment.NODE_OPTIONS).toBe('--no-deprecation') + } finally { + Reflect.deleteProperty(Object.prototype, 'NODE_OPTIONS') + } + }) + + it('builds cluster settings with safe optional shadows and precedence', () => { + const execArgv = ['--no-warnings'] + const stdio = ['ignore', 'pipe', 'pipe', 'ipc'] + const settings = buildClusterSettings( + { + cwd: '/current', + exec: './current.cjs', + execArgv: ['--trace-warnings'], + silent: true, + }, + { + cwd: '/explicit', + exec: './ignored.cjs', + execArgv, + stdio, + }, + './worker.cjs' + ) + expect(Object.getPrototypeOf(settings)).toBeNull() + expect(settings).toMatchObject({ + cwd: '/explicit', + exec: './worker.cjs', + execArgv, + silent: true, + }) + expect(settings.execArgv).not.toBe(execArgv) + expect(settings.stdio).not.toBe(stdio) + expect(Array.isArray(settings.stdio)).toBe(true) + expect(Object.getPrototypeOf(settings.stdio)).toBe(Array.prototype) + expect([...settings.stdio]).toStrictEqual(stdio) + expect(settings.stdio.map(value => value)).toStrictEqual(stdio) + expect(settings.stdio.length).toBe(stdio.length) + for (let index = 0; index < stdio.length; index++) { + expect(Object.hasOwn(settings.stdio, index)).toBe(true) + expect(settings.stdio[index]).toBe(stdio[index]) + } + for (const key of [ + 'cwd', + 'serialization', + 'windowsHide', + 'stdio', + 'gid', + 'uid', + 'inspectPort', + ]) { + expect(Object.hasOwn(settings, key)).toBe(true) + } + }) + + it('builds thread worker options without cloning opaque values', () => { + const workerData = { value: true } + const execArgv = ['--no-warnings'] + const workerOptions = buildThreadWorkerOptions({ execArgv, workerData }) + expect(Object.getPrototypeOf(workerOptions)).toBeNull() + expect(workerOptions.env).toBe(SHARE_ENV) + expect(workerOptions.workerData).toBe(workerData) + expect(workerOptions.execArgv).toStrictEqual(execArgv) + expect(workerOptions.execArgv).not.toBe(execArgv) + }) + + it('normalizes worker option arrays and resource limits by own slots', () => { + const argvValue = { value: 'argv' } + const argv = new Array(2) + argv[1] = argvValue + const transferable = new ArrayBuffer(8) + const transferList = new Array(2) + transferList[1] = transferable + const inheritedResourceLimits = { maxYoungGenerationSizeMb: 1 } + const resourceLimits = Object.create(inheritedResourceLimits) + Object.defineProperty(resourceLimits, 'maxOldGenerationSizeMb', { + value: 32, + }) + const workerData = { value: 'workerData' } + + const workerOptions = buildThreadWorkerOptions({ + argv, + resourceLimits, + transferList, + workerData, + }) + + expect(Object.getPrototypeOf(workerOptions.argv)).toBeNull() + expect(workerOptions.argv).not.toBe(argv) + expect(Object.hasOwn(workerOptions.argv, 0)).toBe(false) + expect(workerOptions.argv[1]).toBe(argvValue) + expect(workerOptions.transferList).not.toBe(transferList) + expect(Object.hasOwn(workerOptions.transferList, 0)).toBe(true) + expect(workerOptions.transferList[0]).toBeUndefined() + expect(workerOptions.transferList[1]).toBe(transferable) + expect(Object.getPrototypeOf(workerOptions.resourceLimits)).toBeNull() + expect( + Object.hasOwn(workerOptions.resourceLimits, 'maxYoungGenerationSizeMb') + ).toBe(false) + expect(workerOptions.resourceLimits.maxOldGenerationSizeMb).toBe(32) + expect(workerOptions.workerData).toBe(workerData) + }) + + it('normalizes cluster positional arrays without reading inherited slots', () => { + const args = new Array(2) + args[1] = '--worker-argument' + const accessor = Symbol('accessor') + let accessorCalls = 0 + const descriptor = {} + const accessorGetter = () => { + accessorCalls++ + return 'accessor-value' + } + Object.defineProperties(descriptor, { + [accessor]: { + configurable: true, + enumerable: true, + get: accessorGetter, + }, + custom: { + value: 'descriptor-value', + }, + }) + const opaqueStream = new PassThrough() + const stdio = new Array(4) + stdio[0] = descriptor + stdio[1] = opaqueStream + stdio[3] = 'ipc' + + const settings = buildClusterSettings( + { exec: './current.cjs' }, + { args, stdio }, + './worker.cjs' + ) + const accessorCallsAfterBuild = accessorCalls + + expect(settings.args).not.toBe(args) + expect(Object.hasOwn(settings.args, 0)).toBe(true) + expect(settings.args[0]).toBeUndefined() + expect(settings.args[1]).toBe('--worker-argument') + expect(Object.getPrototypeOf(settings.stdio)).toBe(Array.prototype) + expect(Object.hasOwn(settings.stdio, 2)).toBe(true) + expect(settings.stdio[2]).toBeUndefined() + expect(settings.stdio[0]).not.toBe(descriptor) + expect(settings.stdio[1]).toBe(opaqueStream) + expect(Object.getPrototypeOf(settings.stdio[0])).toBeNull() + const customDescriptor = Object.getOwnPropertyDescriptor( + descriptor, + 'custom' + ) + const clonedCustomDescriptor = Object.getOwnPropertyDescriptor( + settings.stdio[0], + 'custom' + ) + expect(clonedCustomDescriptor.configurable).toBe( + customDescriptor.configurable + ) + expect(clonedCustomDescriptor.enumerable).toBe(customDescriptor.enumerable) + expect(clonedCustomDescriptor.value).toBe(customDescriptor.value) + expect(clonedCustomDescriptor.writable).toBe(customDescriptor.writable) + const accessorDescriptor = Object.getOwnPropertyDescriptor( + settings.stdio[0], + accessor + ) + expect(accessorDescriptor.configurable).toBe(true) + expect(accessorDescriptor.enumerable).toBe(true) + expect(accessorDescriptor.get).toBe(accessorGetter) + expect(Object.hasOwn(accessorDescriptor, 'set')).toBe(true) + expect(accessorDescriptor.set).toBeUndefined() + expect(accessorCallsAfterBuild).toBe(0) + for (const key of ['fd', 'handle', '_handle']) { + expect(Object.hasOwn(settings.stdio[0], key)).toBe(true) + expect(settings.stdio[0][key]).toBeUndefined() + } + opaqueStream.destroy() + }) + + it('shadows inherited controls on plain stdio descriptors', () => { + // eslint-disable-next-line no-extend-native + Object.defineProperties(Object.prototype, { + _handle: { + configurable: true, + enumerable: true, + value: 'polluted', + writable: true, + }, + handle: { + configurable: true, + enumerable: true, + value: 'polluted', + writable: true, + }, + }) + try { + const settings = buildClusterSettings( + { exec: './current.cjs' }, + { stdio: [{ fd: 1 }] }, + './worker.cjs' + ) + expect(settings.stdio[0].fd).toBe(1) + expect(Object.hasOwn(settings.stdio[0], 'handle')).toBe(true) + expect(settings.stdio[0].handle).toBeUndefined() + expect(Object.hasOwn(settings.stdio[0], '_handle')).toBe(true) + expect(settings.stdio[0]._handle).toBeUndefined() + } finally { + Reflect.deleteProperty(Object.prototype, '_handle') + Reflect.deleteProperty(Object.prototype, 'handle') + } + }) + + it('accepts numeric stdio fd before irrelevant inherited controls', () => { + const opaqueStdioEntry = Object.create({}) + Object.defineProperty(opaqueStdioEntry, 'fd', { value: 0 }) + // eslint-disable-next-line no-extend-native + Object.defineProperty(Object.prototype, 'handle', { + configurable: true, + enumerable: true, + value: -1, + writable: true, + }) + try { + expect(() => + validateAndMaterializeClusterStdio([opaqueStdioEntry]) + ).not.toThrow() + } finally { + Reflect.deleteProperty(Object.prototype, 'handle') + } + }) + + it('ignores nonnumeric inherited fd before a legitimate handle', () => { + const handle = {} + const opaqueStdioEntry = Object.create({ handle }) + // eslint-disable-next-line no-extend-native + Object.defineProperty(Object.prototype, 'fd', { + configurable: true, + enumerable: true, + value: 'polluted', + writable: true, + }) + try { + expect(() => + validateAndMaterializeClusterStdio([opaqueStdioEntry]) + ).not.toThrow() + } finally { + Reflect.deleteProperty(Object.prototype, 'fd') + } + }) + + it('rejects inherited stdio accessors without invoking them', () => { + for (const key of ['fd', 'handle']) { + let accessorCalls = 0 + // eslint-disable-next-line no-extend-native + Object.defineProperty(Object.prototype, key, { + configurable: true, + get: () => { + accessorCalls++ + return key === 'fd' ? 1 : {} + }, + }) + try { + expect(() => validateAndMaterializeClusterStdio([{}])).toThrow( + new TypeError( + `Invalid cluster.settings.stdio entry: inherited accessor '${key}' from Object.prototype is not allowed` + ) + ) + expect(accessorCalls).toBe(0) + } finally { + Reflect.deleteProperty(Object.prototype, key) + } + } + }) + + it('builds worker options with polluted descriptor fields', () => { + for (const key of ['get', 'set']) { + let accessorCalls = 0 + const stdioDescriptor = {} + const accessor = Symbol('accessor') + const accessorGetter = () => { + accessorCalls++ + return 'accessor-value' + } + Object.defineProperty(stdioDescriptor, accessor, { + get: accessorGetter, + }) + // eslint-disable-next-line no-extend-native + Object.defineProperty(Object.prototype, key, { + configurable: true, + value: () => undefined, + writable: true, + }) + let buildError + let clonedAccessorGetter + let settings + let workerOptions + try { + settings = buildClusterSettings( + undefined, + { + args: new Array(1), + execArgv: ['--no-warnings'], + stdio: [stdioDescriptor, 'ignore', 'ignore', 'ipc'], + }, + './worker.cjs' + ) + workerOptions = buildThreadWorkerOptions({ + argv: new Array(1), + execArgv: ['--no-warnings'], + resourceLimits: { maxOldGenerationSizeMb: 32 }, + transferList: new Array(1), + }) + clonedAccessorGetter = Object.getOwnPropertyDescriptor( + settings.stdio[0], + accessor + ).get + } catch (error) { + buildError = error + } finally { + Reflect.deleteProperty(Object.prototype, key) + } + expect(buildError).toBeUndefined() + expect(Object.hasOwn(settings.args, 0)).toBe(true) + expect(Object.hasOwn(settings.stdio, 0)).toBe(true) + expect(clonedAccessorGetter).toBe(accessorGetter) + expect(Object.hasOwn(workerOptions.argv, 0)).toBe(false) + expect(Object.hasOwn(workerOptions.execArgv, 0)).toBe(true) + expect(Object.hasOwn(workerOptions.transferList, 0)).toBe(true) + expect(workerOptions.resourceLimits.maxOldGenerationSizeMb).toBe(32) + expect(accessorCalls).toBe(0) + } + }) + + it('leaves invalid non-array execArgv values for Node validation', () => { + const invalidExecArgv = '--no-warnings' + expect( + buildClusterSettings( + { exec: './current.cjs' }, + { execArgv: invalidExecArgv }, + './worker.cjs' + ).execArgv + ).toBe(invalidExecArgv) + expect( + buildThreadWorkerOptions({ execArgv: invalidExecArgv }).execArgv + ).toBe(invalidExecArgv) + }) + + it('leaves invalid non-object resource limits for Node validation', () => { + const resourceLimits = 'invalid' + expect(buildThreadWorkerOptions({ resourceLimits }).resourceLimits).toBe( + resourceLimits + ) + }) + + it('preserves null resource limits for Node runtime defaults', () => { + expect( + buildThreadWorkerOptions({ resourceLimits: null }).resourceLimits + ).toBeNull() + }) + + it('rejects sparse effective execArgv arrays', () => { + const sparseExecArgv = new Array(1) + // eslint-disable-next-line no-extend-native + Object.defineProperty(Object.prototype, '0', { + configurable: true, + enumerable: true, + value: '--no-warnings', + writable: true, + }) + try { + expect(() => + buildClusterSettings( + { exec: './current.cjs' }, + { execArgv: sparseExecArgv }, + './worker.cjs' + ) + ).toThrow( + new TypeError('Invalid settings.execArgv: must not be a sparse array') + ) + expect(() => + buildThreadWorkerOptions({ execArgv: sparseExecArgv }) + ).toThrow( + new TypeError( + 'Invalid workerOptions.execArgv: must not be a sparse array' + ) + ) + } finally { + Reflect.deleteProperty(Object.prototype, '0') + } + }) + it('Verify updateMeasurementStatistics() behavior', () => { const measurementStatistics = { history: new CircularBuffer(MeasurementHistorySize), diff --git a/tests/worker-files/cluster/prototypePollutionWorker.cjs b/tests/worker-files/cluster/prototypePollutionWorker.cjs new file mode 100644 index 000000000..08bef924e --- /dev/null +++ b/tests/worker-files/cluster/prototypePollutionWorker.cjs @@ -0,0 +1,21 @@ +'use strict' +const { ClusterWorker, KillBehaviors } = require('../../../lib/index.cjs') + +/** + * Returns the cluster worker launch context needed by security regressions. + * @param data - Requested environment key. + * @returns The observed launch context. + */ +function inspectLaunchContext (data = {}) { + return { + cwd: process.cwd(), + envValue: process.env[data.envKey], + execArgv: process.execArgv, + hasEnvKey: Object.hasOwn(process.env, data.envKey), + } +} + +module.exports = new ClusterWorker(inspectLaunchContext, { + killBehavior: KillBehaviors.HARD, + maxInactiveTime: 500, +})