From be2f20d883f0e94055c8fb8f4b45711df2e79fd3 Mon Sep 17 00:00:00 2001 From: yunshingng Date: Mon, 17 Aug 2026 18:31:45 -0400 Subject: [PATCH 1/2] permission: preserve parent allowlist for Worker with empty execArgv When execArgv is set explicitly (including []), workers no longer inherit the parent's CLI flags. Re-attach Permission Model flags from the parent so empty/modified execArgv does not drop filesystem allowlists. Signed-off-by: yunshingng --- lib/internal/worker.js | 56 ++++++++++++++++++- .../test-permission-worker-empty-execargv.js | 39 +++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-permission-worker-empty-execargv.js diff --git a/lib/internal/worker.js b/lib/internal/worker.js index 60e95273ef5f..9bd41dbf6c35 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -5,6 +5,7 @@ const { ArrayPrototypeForEach, ArrayPrototypeMap, ArrayPrototypePush, + ArrayPrototypeSlice, AtomicsAdd, Float64Array, FunctionPrototypeBind, @@ -20,6 +21,7 @@ const { SafeMap, String, StringPrototypeTrim, + StringPrototypeStartsWith, Symbol, SymbolAsyncDispose, SymbolFor, @@ -111,6 +113,8 @@ let debug = require('internal/util/debuglog').debuglog('worker', (fn) => { const dc = require('diagnostics_channel'); const workerThreadsChannel = dc.channel('worker_threads'); +const permission = require('internal/process/permission'); + let cwdCounter; let normalizeHeapProfileOptions; let normalizeCpuProfileOptions; @@ -204,6 +208,52 @@ class HeapProfileHandle { } } + +function ensurePermissionFlagsInExecArgv(execArgv) { + if (!permission.isEnabled() || execArgv == null) { + return execArgv; + } + const flagsToCopy = [ + ...permission.availableFlags(), + '--permission', + '--permission-audit', + ]; + const out = ArrayPrototypeSlice(execArgv); + const hasToken = (token) => { + for (let i = 0; i < out.length; i++) { + const a = out[i]; + if (a === token) return true; + if (typeof a === 'string' && a.startsWith(`${token}=`)) return true; + } + return false; + }; + for (let i = 0; i < process.execArgv.length; i++) { + const arg = process.execArgv[i]; + for (let j = 0; j < flagsToCopy.length; j++) { + const flag = flagsToCopy[j]; + if (arg === flag) { + if (!hasToken(flag)) { + ArrayPrototypePush(out, arg); + const next = process.execArgv[i + 1]; + if (next && !StringPrototypeStartsWith(next, '-')) { + ArrayPrototypePush(out, next); + } + } + } else if (StringPrototypeStartsWith(arg, `${flag}=`)) { + let present = false; + for (let k = 0; k < out.length; k++) { + if (out[k] === arg || StringPrototypeStartsWith(out[k], `${flag}=`)) { + present = true; + break; + } + } + if (!present) ArrayPrototypePush(out, arg); + } + } + } + return out; +} + class Worker extends EventEmitter { constructor(filename, options = kEmptyObject) { throwIfBuildingSnapshot('Creating workers'); @@ -218,6 +268,10 @@ class Worker extends EventEmitter { if (options.execArgv) validateArray(options.execArgv, 'options.execArgv'); + let workerExecArgv = options.execArgv; + if (workerExecArgv) + workerExecArgv = ensurePermissionFlagsInExecArgv(workerExecArgv); + let argv; if (options.argv) { validateArray(options.argv, 'options.argv'); @@ -288,7 +342,7 @@ class Worker extends EventEmitter { // Set up the C++ handle for the worker, as well as some internal wiring. this[kHandle] = new WorkerImpl(url, env === process.env ? null : env, - options.execArgv, + workerExecArgv, parseResourceLimits(options.resourceLimits), !!(options.trackUnmanagedFds ?? true), isInternal, diff --git a/test/parallel/test-permission-worker-empty-execargv.js b/test/parallel/test-permission-worker-empty-execargv.js new file mode 100644 index 000000000000..cca8144af7ea --- /dev/null +++ b/test/parallel/test-permission-worker-empty-execargv.js @@ -0,0 +1,39 @@ +'use strict'; +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); +const allowed = tmpdir.path; +const denied = path.join(tmpdir.path, '..', 'permission-worker-denied-file'); +fs.writeFileSync(denied, 'secret\n'); +const workerSource = ` +const { parentPort } = require('worker_threads'); +const fs = require('fs'); +const path = ${JSON.stringify(denied)}; +let result; +try { result = { ok: true, data: fs.readFileSync(path, 'utf8') }; } +catch (e) { result = { ok: false, code: e.code, message: e.message }; } +parentPort.postMessage(result); +`; +function runCase(label, useEmpty) { + const code = ` +const { Worker } = require('worker_threads'); +const w = new Worker(${JSON.stringify(workerSource)}, { eval: true, ${useEmpty ? 'execArgv: [],' : ''} }); +w.on('message', (msg) => { console.log(JSON.stringify({ label: ${JSON.stringify(label)}, msg })); process.exit(0); }); +w.on('error', (err) => { console.error(err); process.exit(1); }); +`; + return spawnSync(process.execPath, ['--permission', `--allow-fs-read=${allowed}`, '--allow-worker', '-e', code], { encoding: 'utf8', timeout: 15000 }); +} +const defaultWorker = runCase('default', false); +const emptyExec = runCase('empty-execArgv', true); +assert.strictEqual(defaultWorker.status, 0, defaultWorker.stderr); +assert.strictEqual(emptyExec.status, 0, emptyExec.stderr); +const dMsg = JSON.parse(defaultWorker.stdout.trim().split('\n').pop()); +const eMsg = JSON.parse(emptyExec.stdout.trim().split('\n').pop()); +assert.strictEqual(dMsg.msg.ok, false); +assert.strictEqual(dMsg.msg.code, 'ERR_ACCESS_DENIED'); +assert.strictEqual(eMsg.msg.ok, false, JSON.stringify(eMsg)); +assert.strictEqual(eMsg.msg.code, 'ERR_ACCESS_DENIED'); +console.log('ok - permission inheritance consistent for empty execArgv'); From fc2103a4b0a208049adbcceb3f234f4ab0e4eee1 Mon Sep 17 00:00:00 2001 From: yunshingng Date: Mon, 17 Aug 2026 18:58:43 -0400 Subject: [PATCH 2/2] permission: polish Worker execArgv permission flag inheritance - Use only primordials in ensurePermissionFlagsInExecArgv - Improve flag presence checks for --flag and --flag=value forms - Align regression test with parallel test style (common, isMainThread) Signed-off-by: yunshingng --- lib/internal/worker.js | 35 +++--- .../test-permission-worker-empty-execargv.js | 104 +++++++++++++----- 2 files changed, 99 insertions(+), 40 deletions(-) diff --git a/lib/internal/worker.js b/lib/internal/worker.js index 9bd41dbf6c35..24452763a290 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -213,41 +213,46 @@ function ensurePermissionFlagsInExecArgv(execArgv) { if (!permission.isEnabled() || execArgv == null) { return execArgv; } + const flagsToCopy = [ ...permission.availableFlags(), '--permission', '--permission-audit', ]; const out = ArrayPrototypeSlice(execArgv); - const hasToken = (token) => { + + function indexOfFlag(token) { for (let i = 0; i < out.length; i++) { const a = out[i]; - if (a === token) return true; - if (typeof a === 'string' && a.startsWith(`${token}=`)) return true; + if (a === token) { + return i; + } + if (StringPrototypeStartsWith(a, `${token}=`)) { + return i; + } } - return false; - }; + return -1; + } + for (let i = 0; i < process.execArgv.length; i++) { const arg = process.execArgv[i]; for (let j = 0; j < flagsToCopy.length; j++) { const flag = flagsToCopy[j]; if (arg === flag) { - if (!hasToken(flag)) { + if (indexOfFlag(flag) === -1) { ArrayPrototypePush(out, arg); const next = process.execArgv[i + 1]; - if (next && !StringPrototypeStartsWith(next, '-')) { + if (next !== undefined && !StringPrototypeStartsWith(next, '-')) { ArrayPrototypePush(out, next); } } - } else if (StringPrototypeStartsWith(arg, `${flag}=`)) { - let present = false; - for (let k = 0; k < out.length; k++) { - if (out[k] === arg || StringPrototypeStartsWith(out[k], `${flag}=`)) { - present = true; - break; - } + break; + } + if (StringPrototypeStartsWith(arg, `${flag}=`)) { + if (indexOfFlag(flag) === -1) { + ArrayPrototypePush(out, arg); } - if (!present) ArrayPrototypePush(out, arg); + break; } } } diff --git a/test/parallel/test-permission-worker-empty-execargv.js b/test/parallel/test-permission-worker-empty-execargv.js index cca8144af7ea..2b02a5741667 100644 --- a/test/parallel/test-permission-worker-empty-execargv.js +++ b/test/parallel/test-permission-worker-empty-execargv.js @@ -1,39 +1,93 @@ 'use strict'; + +// Consistency under the Permission Model: +// Worker with explicit execArgv: [] must keep the same filesystem allowlist +// as a default Worker (empty/modified execArgv must not drop parent limits). + +const common = require('../common'); +const { isMainThread } = require('worker_threads'); + +if (!isMainThread) { + common.skip('This test only works on a main thread'); +} + const assert = require('assert'); const fs = require('fs'); const path = require('path'); const { spawnSync } = require('child_process'); const tmpdir = require('../common/tmpdir'); + tmpdir.refresh(); + const allowed = tmpdir.path; -const denied = path.join(tmpdir.path, '..', 'permission-worker-denied-file'); -fs.writeFileSync(denied, 'secret\n'); +const deniedFile = path.join(tmpdir.path, '..', 'permission-worker-denied-file'); +fs.writeFileSync(deniedFile, 'secret\n'); + const workerSource = ` -const { parentPort } = require('worker_threads'); -const fs = require('fs'); -const path = ${JSON.stringify(denied)}; -let result; -try { result = { ok: true, data: fs.readFileSync(path, 'utf8') }; } -catch (e) { result = { ok: false, code: e.code, message: e.message }; } -parentPort.postMessage(result); + const { parentPort } = require('worker_threads'); + const fs = require('fs'); + const denied = ${JSON.stringify(deniedFile)}; + let result; + try { + result = { ok: true, data: fs.readFileSync(denied, 'utf8') }; + } catch (err) { + result = { ok: false, code: err.code, message: err.message }; + } + parentPort.postMessage(result); `; -function runCase(label, useEmpty) { + +function runCase(label, useEmptyExecArgv) { + const execArgvLine = useEmptyExecArgv ? 'execArgv: [],' : ''; const code = ` -const { Worker } = require('worker_threads'); -const w = new Worker(${JSON.stringify(workerSource)}, { eval: true, ${useEmpty ? 'execArgv: [],' : ''} }); -w.on('message', (msg) => { console.log(JSON.stringify({ label: ${JSON.stringify(label)}, msg })); process.exit(0); }); -w.on('error', (err) => { console.error(err); process.exit(1); }); -`; - return spawnSync(process.execPath, ['--permission', `--allow-fs-read=${allowed}`, '--allow-worker', '-e', code], { encoding: 'utf8', timeout: 15000 }); + const { Worker } = require('worker_threads'); + const w = new Worker(${JSON.stringify(workerSource)}, { + eval: true, + ${execArgvLine} + }); + w.on('message', (msg) => { + process.stdout.write(JSON.stringify({ label: ${JSON.stringify(label)}, msg }) + '\\n'); + process.exit(0); + }); + w.on('error', (err) => { + console.error(err); + process.exit(1); + }); + `; + return spawnSync( + process.execPath, + [ + '--permission', + `--allow-fs-read=${allowed}`, + '--allow-worker', + '-e', + code, + ], + { + encoding: 'utf8', + timeout: 15000, + }, + ); } + const defaultWorker = runCase('default', false); -const emptyExec = runCase('empty-execArgv', true); +const emptyExecArgv = runCase('empty-execArgv', true); + assert.strictEqual(defaultWorker.status, 0, defaultWorker.stderr); -assert.strictEqual(emptyExec.status, 0, emptyExec.stderr); -const dMsg = JSON.parse(defaultWorker.stdout.trim().split('\n').pop()); -const eMsg = JSON.parse(emptyExec.stdout.trim().split('\n').pop()); -assert.strictEqual(dMsg.msg.ok, false); -assert.strictEqual(dMsg.msg.code, 'ERR_ACCESS_DENIED'); -assert.strictEqual(eMsg.msg.ok, false, JSON.stringify(eMsg)); -assert.strictEqual(eMsg.msg.code, 'ERR_ACCESS_DENIED'); -console.log('ok - permission inheritance consistent for empty execArgv'); +assert.strictEqual(emptyExecArgv.status, 0, emptyExecArgv.stderr); + +const defaultMsg = JSON.parse(defaultWorker.stdout.trim().split('\n').pop()); +const emptyMsg = JSON.parse(emptyExecArgv.stdout.trim().split('\n').pop()); + +assert.strictEqual( + defaultMsg.msg.ok, + false, + `default Worker should deny: ${JSON.stringify(defaultMsg)}`, +); +assert.strictEqual(defaultMsg.msg.code, 'ERR_ACCESS_DENIED'); + +assert.strictEqual( + emptyMsg.msg.ok, + false, + `Worker with execArgv: [] should deny: ${JSON.stringify(emptyMsg)}`, +); +assert.strictEqual(emptyMsg.msg.code, 'ERR_ACCESS_DENIED');