policy: ensure workers do not read fs for policy

This prevents a main thread from rewriting the policy file and loading a worker that has a different policy from the main thread. PR-URL: #25710 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

Author: bmeck

lib/internal/bootstrap/node.js

@@ -173,7 +173,7 @@ function startup(){
 
 // TODO(joyeecheung): move this down further to get better snapshotting
 constexperimentalPolicy=getOptionValue('--experimental-policy');
-if(experimentalPolicy){
+if(isMainThread&&experimentalPolicy){
 process.emitWarning('Policies are experimental.',
 'ExperimentalWarning');
 const{ pathToFileURL,URL}=NativeModule.require('url');
@@ -364,8 +364,6 @@ function startup(){
 }
 
 functionstartWorkerThreadExecution(){
-prepareUserCodeExecution();
-
 // If we are in a worker thread, execute the script sent through the
 // message port.
 const{
@@ -382,7 +380,9 @@ function startWorkerThreadExecution(){
 debug(`[${threadId}] is setting up worker child environment`);
 
 constport=getEnvMessagePort();
-port.on('message',createMessageHandler(port));
+port.on('message',createMessageHandler(
+port,
+prepareUserCodeExecution));
 port.start();
 
 // Overwrite fatalException
@@ -476,7 +476,7 @@ function prepareUserCodeExecution(){
 
 // For user code, we preload modules if `-r` is passed
 constpreloadModules=getOptionValue('--require');
-if(preloadModules){
+if(preloadModules.length){
 const{
  _preloadModules
 }=NativeModule.require('internal/modules/cjs/loader');

lib/internal/process/policy.js

@@ -5,10 +5,14 @@ const{
 }=require('internal/errors').codes;
 const{ Manifest }=require('internal/policy/manifest');
 letmanifest;
+letmanifestSrc;
+letmanifestURL;
 
 module.exports=Object.freeze({
 __proto__: null,
 setup(src,url){
+manifestSrc=src;
+manifestURL=url;
 if(src===null){
 manifest=null;
 return;
@@ -31,6 +35,20 @@ module.exports = Object.freeze({
 returnmanifest;
 },
 
+getsrc(){
+if(typeofmanifestSrc==='undefined'){
+thrownewERR_MANIFEST_TDZ();
+}
+returnmanifestSrc;
+},
+
+geturl(){
+if(typeofmanifestURL==='undefined'){
+thrownewERR_MANIFEST_TDZ();
+}
+returnmanifestURL;
+},
+
 assertIntegrity(moduleURL,content){
 this.manifest.matchesIntegrity(moduleURL,content);
 }

lib/internal/process/worker_thread_only.js

@@ -43,12 +43,24 @@ function initializeWorkerStdio(){
 };
 }
 
-functioncreateMessageHandler(port){
+functioncreateMessageHandler(port,prepareUserCodeExecution){
 constpublicWorker=require('worker_threads');
 
 returnfunction(message){
 if(message.type===messageTypes.LOAD_SCRIPT){
-const{ filename, doEval, workerData, publicPort, hasStdin }=message;
+const{
+ filename,
+ doEval,
+ workerData,
+ publicPort,
+ manifestSrc,
+ manifestURL,
+ hasStdin
+}=message;
+if(manifestSrc){
+require('internal/process/policy').setup(manifestSrc,manifestURL);
+}
+prepareUserCodeExecution();
 publicWorker.parentPort=publicPort;
 publicWorker.workerData=workerData;
 

lib/internal/worker.js

@@ -12,6 +12,7 @@ const{
 ERR_INVALID_ARG_TYPE,
 }=require('internal/errors').codes;
 const{ validateString }=require('internal/validators');
+const{ getOptionValue }=require('internal/options');
 
 const{
  drainMessagePort,
@@ -112,6 +113,9 @@ class Worker extends EventEmitter{
 doEval: !!options.eval,
 workerData: options.workerData,
 publicPort: port2,
+manifestSrc: getOptionValue('--experimental-policy') ?
+require('internal/process/policy').src :
+null,
 hasStdin: !!options.stdin
 },[port2]);
 // Actually start the new thread now that everything is in place.

test/parallel/test-policy-integrity.js

@@ -33,6 +33,17 @@ const parentFilepath = path.join(tmpdir.path, 'parent.js');
 constparentURL=pathToFileURL(parentFilepath);
 constparentBody='require(\'./dep.js\')';
 
+constworkerSpawningFilepath=path.join(tmpdir.path,'worker_spawner.js');
+constworkerSpawningURL=pathToFileURL(workerSpawningFilepath);
+constworkerSpawningBody=`
+const{Worker } = require('worker_threads');
+// make sure this is gone to ensure we don't do another fs read of it
+// will error out if we do
+require('fs').unlinkSync(${JSON.stringify(policyFilepath)});
+const w = new Worker(${JSON.stringify(parentFilepath)});
+w.on('exit', process.exit);
+`;
+
 constdepFilepath=path.join(tmpdir.path,'dep.js');
 constdepURL=pathToFileURL(depFilepath);
 constdepBody='';
@@ -49,8 +60,9 @@ if (!tmpdirURL.pathname.endsWith('/')){
 }
 functiontest({
  shouldFail =false,
+ preload =[],
  entry,
- onerror,
+ onerror=undefined,
  resources ={}
 }){
 constmanifest={
@@ -65,7 +77,9 @@ function test({
 }
 fs.writeFileSync(policyFilepath,JSON.stringify(manifest,null,2));
 const{ status }=spawnSync(process.execPath,[
-'--experimental-policy',policyFilepath,entry
+'--experimental-policy',policyFilepath,
+ ...preload.map((m)=>['-r',m]).flat(),
+entry
 ]);
 if(shouldFail){
 assert.notStrictEqual(status,0);
@@ -74,13 +88,25 @@ function test({
 }
 }
 
-const{ status }=spawnSync(process.execPath,[
-'--experimental-policy',policyFilepath,
-'--experimental-policy',policyFilepath
-],{
-stdio: 'pipe'
-});
-assert.notStrictEqual(status,0,'Should not allow multiple policies');
+{
+const{ status }=spawnSync(process.execPath,[
+'--experimental-policy',policyFilepath,
+'--experimental-policy',policyFilepath
+],{
+stdio: 'pipe'
+});
+assert.notStrictEqual(status,0,'Should not allow multiple policies');
+}
+{
+constenoentFilepath=path.join(tmpdir.path,'enoent');
+try{fs.unlinkSync(enoentFilepath);}catch{}
+const{ status }=spawnSync(process.execPath,[
+'--experimental-policy',enoentFilepath,'-e',''
+],{
+stdio: 'pipe'
+});
+assert.notStrictEqual(status,0,'Should not allow missing policies');
+}
 
 test({
 shouldFail: true,
@@ -195,6 +221,21 @@ test({
 }
 }
 });
+test({
+shouldFail: false,
+preload: [depFilepath],
+entry: parentFilepath,
+resources: {
+[parentURL]: {
+body: parentBody,
+match: true,
+},
+[depURL]: {
+body: depBody,
+match: true,
+}
+}
+});
 test({
 shouldFail: true,
 entry: parentFilepath,
@@ -295,3 +336,50 @@ test({
 }
 }
 });
+test({
+shouldFail: true,
+entry: workerSpawningFilepath,
+resources: {
+[workerSpawningURL]: {
+body: workerSpawningBody,
+match: true,
+},
+}
+});
+test({
+shouldFail: false,
+entry: workerSpawningFilepath,
+resources: {
+[workerSpawningURL]: {
+body: workerSpawningBody,
+match: true,
+},
+[parentURL]: {
+body: parentBody,
+match: true,
+},
+[depURL]: {
+body: depBody,
+match: true,
+}
+}
+});
+test({
+shouldFail: false,
+entry: workerSpawningFilepath,
+preload: [parentFilepath],
+resources: {
+[workerSpawningURL]: {
+body: workerSpawningBody,
+match: true,
+},
+[parentURL]: {
+body: parentBody,
+match: true,
+},
+[depURL]: {
+body: depBody,
+match: true,
+}
+}
+});