From 3a74316624c3f47214c2ec8b45b81afbb5d10499 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sat, 29 Jul 2023 16:22:16 +0300 Subject: [PATCH] test_runner: add `__proto__` null PR-URL: https://github.com/nodejs/node/pull/48663 Reviewed-By: Antoine du Hamel Reviewed-By: Chemi Atlow --- .eslintrc.js | 8 + lib/internal/test_runner/coverage.js | 2 +- lib/internal/test_runner/harness.js | 3 +- lib/internal/test_runner/mock/mock.js | 8 +- lib/internal/test_runner/mock/mock_timers.js | 5 +- lib/internal/test_runner/reporter/spec.js | 4 +- lib/internal/test_runner/reporter/tap.js | 2 +- lib/internal/test_runner/runner.js | 21 +-- lib/internal/test_runner/test.js | 12 +- lib/internal/test_runner/tests_stream.js | 4 +- lib/internal/test_runner/utils.js | 2 +- .../set-proto-to-null-in-object.js | 140 ++++++++++++++++++ 12 files changed, 186 insertions(+), 25 deletions(-) create mode 100644 tools/eslint-rules/set-proto-to-null-in-object.js diff --git a/.eslintrc.js b/.eslintrc.js index 9c31bf3da17dbf..b427c9986f679f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -122,6 +122,14 @@ module.exports = { 'curly': 'error', }, }, + { + files: [ + 'lib/internal/test_runner/**/*.js', + ], + rules: { + 'node-core/set-proto-to-null-in-object': 'error', + }, + }, ], rules: { // ESLint built-in rules diff --git a/lib/internal/test_runner/coverage.js b/lib/internal/test_runner/coverage.js index a20055fa91b601..70f88984c82150 100644 --- a/lib/internal/test_runner/coverage.js +++ b/lib/internal/test_runner/coverage.js @@ -250,7 +250,7 @@ class TestCoverage { let dir; try { - mkdirSync(this.originalCoverageDirectory, { recursive: true }); + mkdirSync(this.originalCoverageDirectory, { __proto__: null, recursive: true }); dir = opendirSync(this.coverageDirectory); for (let entry; (entry = dir.readSync()) !== null;) { diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index f29d5c95cc6084..d0a71735afe729 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -114,6 +114,7 @@ function setup(root) { const globalOptions = parseCommandLine(); const hook = createHook({ + __proto__: null, init(asyncId, type, triggerAsyncId, resource) { if (resource instanceof Test) { testResources.set(asyncId, resource); @@ -214,7 +215,7 @@ function runInParentContext(Factory) { const test = (name, options, fn) => run(name, options, fn); ArrayPrototypeForEach(['skip', 'todo', 'only'], (keyword) => { - test[keyword] = (name, options, fn) => run(name, options, fn, { [keyword]: true }); + test[keyword] = (name, options, fn) => run(name, options, fn, { __proto__: null, [keyword]: true }); }); return test; } diff --git a/lib/internal/test_runner/mock/mock.js b/lib/internal/test_runner/mock/mock.js index 71363154c88328..a704b41996e6d2 100644 --- a/lib/internal/test_runner/mock/mock.js +++ b/lib/internal/test_runner/mock/mock.js @@ -133,7 +133,7 @@ class MockTracker { validateObject(options, 'options'); const { times = Infinity } = options; validateTimes(times, 'options.times'); - const ctx = new MockFunctionContext(implementation, { original }, times); + const ctx = new MockFunctionContext(implementation, { __proto__: null, original }, times); return this.#setupMock(ctx, original); } @@ -189,7 +189,7 @@ class MockTracker { ); } - const restore = { descriptor, object: objectOrFunction, methodName }; + const restore = { __proto__: null, descriptor, object: objectOrFunction, methodName }; const impl = implementation === kDefaultFunction ? original : implementation; const ctx = new MockFunctionContext(impl, restore, times); @@ -238,6 +238,7 @@ class MockTracker { } return this.method(object, methodName, implementation, { + __proto__: null, ...options, getter, }); @@ -265,6 +266,7 @@ class MockTracker { } return this.method(object, methodName, implementation, { + __proto__: null, ...options, setter, }); @@ -297,6 +299,7 @@ class MockTracker { throw err; } finally { FunctionPrototypeCall(trackCall, ctx, { + __proto__: null, arguments: argList, error, result, @@ -321,6 +324,7 @@ class MockTracker { throw err; } finally { FunctionPrototypeCall(trackCall, ctx, { + __proto__: null, arguments: argList, error, result, diff --git a/lib/internal/test_runner/mock/mock_timers.js b/lib/internal/test_runner/mock/mock_timers.js index dc75bcd4afbee5..6e16e172d255c1 100644 --- a/lib/internal/test_runner/mock/mock_timers.js +++ b/lib/internal/test_runner/mock/mock_timers.js @@ -44,7 +44,7 @@ function setPosition(node, pos) { } function abortIt(signal) { - return new AbortError(undefined, { cause: signal.reason }); + return new AbortError(undefined, { __proto__: null, cause: signal.reason }); } const SUPPORTED_TIMERS = ['setTimeout', 'setInterval']; @@ -193,7 +193,9 @@ class MockTimers { #toggleEnableTimers(activate) { const options = { + __proto__: null, toFake: { + __proto__: null, setTimeout: () => { this.#realSetTimeout = globalThis.setTimeout; this.#realClearTimeout = globalThis.clearTimeout; @@ -232,6 +234,7 @@ class MockTimers { }, }, toReal: { + __proto__: null, setTimeout: () => { globalThis.setTimeout = this.#realSetTimeout; globalThis.clearTimeout = this.#realClearTimeout; diff --git a/lib/internal/test_runner/reporter/spec.js b/lib/internal/test_runner/reporter/spec.js index 16cbdf1d5aa901..87d893aecf606f 100644 --- a/lib/internal/test_runner/reporter/spec.js +++ b/lib/internal/test_runner/reporter/spec.js @@ -42,7 +42,7 @@ class SpecReporter extends Transform { #failedTests = []; constructor() { - super({ writableObjectMode: true }); + super({ __proto__: null, writableObjectMode: true }); } #indent(nesting) { @@ -127,7 +127,7 @@ class SpecReporter extends Transform { } } _transform({ type, data }, encoding, callback) { - callback(null, this.#handleEvent({ type, data })); + callback(null, this.#handleEvent({ __proto__: null, type, data })); } _flush(callback) { if (this.#failedTests.length === 0) { diff --git a/lib/internal/test_runner/reporter/tap.js b/lib/internal/test_runner/reporter/tap.js index 4aec4ba072d954..9c737efef35b96 100644 --- a/lib/internal/test_runner/reporter/tap.js +++ b/lib/internal/test_runner/reporter/tap.js @@ -18,7 +18,7 @@ const kDefaultIndent = ' '; // 4 spaces const kFrameStartRegExp = /^ {4}at /; const kLineBreakRegExp = /\n|\r\n/; const kDefaultTAPVersion = 13; -const inspectOptions = { colors: false, breakLength: Infinity }; +const inspectOptions = { __proto__: null, colors: false, breakLength: Infinity }; let testModule; // Lazy loaded due to circular dependency. function lazyLoadTest() { diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index cff115e58102f2..4580fa9917fa24 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -134,7 +134,10 @@ function createTestFileList() { for (let i = 0; i < testPaths.length; i++) { const absolutePath = resolve(testPaths[i]); - processPath(absolutePath, testFiles, { userSupplied: true }); + processPath(absolutePath, testFiles, { + __proto__: null, + userSupplied: true, + }); } } catch (err) { if (err?.code === 'ENOENT') { @@ -347,9 +350,9 @@ class FileTest extends Test { function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) { const watchMode = filesWatcher != null; const subtest = root.createSubtest(FileTest, path, async (t) => { - const args = getRunArgs({ path, inspectPort, testNamePatterns }); + const args = getRunArgs({ __proto__: null, path, inspectPort, testNamePatterns }); const stdio = ['pipe', 'pipe', 'pipe']; - const env = { ...process.env, NODE_TEST_CONTEXT: 'child-v8' }; + const env = { __proto__: null, ...process.env, NODE_TEST_CONTEXT: 'child-v8' }; if (watchMode) { stdio.push('ipc'); env.WATCH_REPORT_DEPENDENCIES = '1'; @@ -358,7 +361,7 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) { env.FORCE_COLOR = '1'; } - const child = spawn(process.execPath, args, { signal: t.signal, encoding: 'utf8', env, stdio }); + const child = spawn(process.execPath, args, { __proto__: null, signal: t.signal, encoding: 'utf8', env, stdio }); if (watchMode) { filesWatcher.runningProcesses.set(path, child); filesWatcher.watcher.watchChildProcessModules(child, path); @@ -375,7 +378,7 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) { subtest.parseMessage(data); }); - const rl = createInterface({ input: child.stderr }); + const rl = createInterface({ __proto__: null, input: child.stderr }); rl.on('line', (line) => { if (isInspectorMessage(line)) { process.stderr.write(line + '\n'); @@ -393,8 +396,8 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) { }); const { 0: { 0: code, 1: signal } } = await SafePromiseAll([ - once(child, 'exit', { signal: t.signal }), - finished(child.stdout, { signal: t.signal }), + once(child, 'exit', { __proto__: null, signal: t.signal }), + finished(child.stdout, { __proto__: null, signal: t.signal }), ]); if (watchMode) { @@ -427,7 +430,7 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) { function watchFiles(testFiles, root, inspectPort, signal, testNamePatterns) { const runningProcesses = new SafeMap(); const runningSubtests = new SafeMap(); - const watcher = new FilesWatcher({ debounce: 200, mode: 'filter', signal }); + const watcher = new FilesWatcher({ __proto__: null, debounce: 200, mode: 'filter', signal }); const filesWatcher = { __proto__: null, watcher, runningProcesses, runningSubtests }; watcher.on('changed', ({ owners }) => { @@ -512,7 +515,7 @@ function run(options) { }); } - const root = createTestTree({ concurrency, timeout, signal }); + const root = createTestTree({ __proto__: null, concurrency, timeout, signal }); let testFiles = files ?? createTestFileList(); if (shard) { diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 9c76a371462057..a53ea8b4fc1d44 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -79,7 +79,7 @@ function stopTest(timeout, signal) { if (timeout === kDefaultTimeout) { return once(signal, 'abort'); } - return PromisePrototypeThen(setTimeout(timeout, null, { ref: false, signal }), () => { + return PromisePrototypeThen(setTimeout(timeout, null, { __proto__: null, ref: false, signal }), () => { throw new ERR_TEST_FAILURE( `test timed out after ${timeout}ms`, kTestTimeoutFailure, @@ -506,7 +506,7 @@ class Test extends AsyncResource { getRunArgs() { const ctx = new TestContext(this); - return { ctx, args: [ctx] }; + return { __proto__: null, ctx, args: [ctx] }; } async runHook(hook, args) { @@ -540,12 +540,12 @@ class Test extends AsyncResource { const { args, ctx } = this.getRunArgs(); const after = async () => { if (this.hooks.after.length > 0) { - await this.runHook('after', { args, ctx }); + await this.runHook('after', { __proto__: null, args, ctx }); } }; const afterEach = runOnce(async () => { if (this.parent?.hooks.afterEach.length > 0) { - await this.parent.runHook('afterEach', { args, ctx }); + await this.parent.runHook('afterEach', { __proto__: null, args, ctx }); } }); @@ -554,7 +554,7 @@ class Test extends AsyncResource { await this.parent.runHook('before', this.parent.getRunArgs()); } if (this.parent?.hooks.beforeEach.length > 0) { - await this.parent.runHook('beforeEach', { args, ctx }); + await this.parent.runHook('beforeEach', { __proto__: null, args, ctx }); } const stopPromise = stopTest(this.timeout, this.signal); const runArgs = ArrayPrototypeSlice(args); @@ -811,7 +811,7 @@ class Suite extends Test { getRunArgs() { const ctx = new SuiteContext(this); - return { ctx, args: [ctx] }; + return { __proto__: null, ctx, args: [ctx] }; } async run() { diff --git a/lib/internal/test_runner/tests_stream.js b/lib/internal/test_runner/tests_stream.js index 20e7d458704b20..901987681f319b 100644 --- a/lib/internal/test_runner/tests_stream.js +++ b/lib/internal/test_runner/tests_stream.js @@ -12,7 +12,7 @@ class TestsStream extends Readable { #canPush; constructor() { - super({ objectMode: true }); + super({ __proto__: null, objectMode: true }); this.#buffer = []; this.#canPush = true; } @@ -83,6 +83,8 @@ class TestsStream extends Readable { [kEmitMessage](type, data) { this.emit(type, data); + // Disabling as this going to the user-land + // eslint-disable-next-line node-core/set-proto-to-null-in-object this.#tryPush({ type, data }); } diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index 203d64c355bc63..dc8e27b21dc31e 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -82,7 +82,7 @@ function createDeferredCallback() { resolve(); }; - return { promise, cb }; + return { __proto__: null, promise, cb }; } function isTestFailureError(err) { diff --git a/tools/eslint-rules/set-proto-to-null-in-object.js b/tools/eslint-rules/set-proto-to-null-in-object.js new file mode 100644 index 00000000000000..e1a6af38c032dc --- /dev/null +++ b/tools/eslint-rules/set-proto-to-null-in-object.js @@ -0,0 +1,140 @@ +'use strict'; + +module.exports = { + meta: { + messages: { + error: 'Add `__proto__: null` to object', + }, + fixable: 'code', + }, + create: function(context) { + return { + ObjectExpression(node) { + // Not adding __proto__ to module.exports as it will break a lot of libraries + if (isModuleExportsObject(node) || isModifiedExports(node)) { + return; + } + + const properties = node.properties; + let hasProto = false; + + for (const property of properties) { + + if (!property.key) { + continue; + } + + if (property.key.type === 'Identifier' && property.key.name === '__proto__') { + hasProto = true; + break; + } + + if (property.key.type === 'Literal' && property.key.value === '__proto__') { + hasProto = true; + break; + } + } + + if (hasProto) { + return; + } + + if (properties.length > 0) { + // If the object has properties but no __proto__ property + context.report({ + node, + message: 'Every object must have __proto__: null', + fix: function(fixer) { + // Generate the fix suggestion to add __proto__: null + const sourceCode = context.getSourceCode(); + const firstProperty = properties[0]; + const firstPropertyToken = sourceCode.getFirstToken(firstProperty); + + + const isMultiLine = properties.length === 1 ? + // If the object has only one property, + // it's multiline if the property is not on the same line as the object parenthesis + properties[0].loc.start.line !== node.loc.start.line : + // If the object has more than one property, + // it's multiline if the first and second properties are not on the same line + properties[0].loc.start.line !== properties[1].loc.start.line; + + const fixText = `__proto__: null,${isMultiLine ? '\n' : ' '}`; + + // Insert the fix suggestion before the first property + return fixer.insertTextBefore(firstPropertyToken, fixText); + }, + }); + } + + if (properties.length === 0) { + // If the object is empty and missing __proto__ + context.report({ + node, + message: 'Every empty object must have __proto__: null', + fix: function(fixer) { + // Generate the fix suggestion to create the object with __proto__: null + const fixText = '{ __proto__: null }'; + + // Replace the empty object with the fix suggestion + return fixer.replaceText(node, fixText); + }, + }); + } + }, + }; + }, +}; + +// Helper function to check if the object is `module.exports` +function isModuleExportsObject(node) { + return ( + node.parent && + node.parent.type === 'AssignmentExpression' && + node.parent.left && + node.parent.left.type === 'MemberExpression' && + node.parent.left.object && + node.parent.left.object.name === 'module' && + node.parent.left.property && + node.parent.left.property.name === 'exports' + ); +} + +function isModifiedExports(node) { + return ( + node.parent && + (isObjectAssignCall(node.parent) || isObjectDefinePropertiesCall(node.parent)) + ); +} + +// Helper function to check if the node represents an ObjectAssign call +function isObjectAssignCall(node) { + return ( + node.type === 'CallExpression' && + node.callee && + node.callee.type === 'Identifier' && + node.callee.name === 'ObjectAssign' && + node.arguments.length > 1 && + node.arguments.some((arg) => + arg.type === 'MemberExpression' && + arg.object.name === 'module' && + arg.property.name === 'exports', + ) + ); +} + +// Helper function to check if the node represents an ObjectDefineProperties call +function isObjectDefinePropertiesCall(node) { + return ( + node.type === 'CallExpression' && + node.callee && + node.callee.type === 'Identifier' && + node.callee.name === 'ObjectDefineProperties' && + node.arguments.length > 1 && + node.arguments.some((arg) => + arg.type === 'MemberExpression' && + arg.object.name === 'module' && + arg.property.name === 'exports', + ) + ); +}