permission: fix env var default behavior and add manpage docs

Signed-off-by: nabeel378 <mohammadnabeeljameel@gmail.com>

Author: nabeel378

doc/node.1

@@ -135,6 +135,25 @@ This behavior also applies to \fBchild_process.spawn()\fR, but in that case, the
 flags are propagated via the \fBNODE_OPTIONS\fR environment variable rather than
 directly through the process arguments.
 .
+.It Fl -allow-env
+When using the Permission Model, the process will be able to access all
+environment variables by default.
+When the \fB--allow-env\fR flag is used, the process will only be able to access
+the specified environment variables.
+.Pp
+The valid arguments for the \fB--allow-env\fR flag are:
+.Bl -bullet
+.It
+\fB*\fR - To allow access to all environment variables.
+.It
+Multiple environment variable names can be allowed using multiple
+\fB--allow-env\fR flags.
+Example \fB--allow-env=HOME --allow-env=PATH\fR
+.El
+.Bd -literal
+$ node --permission --allow-env=HOME --allow-fs-read=* index.js
+.Ed
+.
 .It Fl -allow-ffi
 When using the Permission Model, the process will not be able to use
 \fBnode:ffi\fR by default.
@@ -1146,6 +1165,8 @@ WASI - manageable through \fB--allow-wasi\fR flag
 Addons - manageable through \fB--allow-addons\fR flag
 .It
 FFI - manageable through \fB--allow-ffi\fR flag
+.It
+Environment Variables - manageable through \fB--allow-env\fR flag
 .El
 .
 .It Fl -permission-audit
@@ -1860,6 +1881,8 @@ one is included in the list below.
 .It
 \fB--allow-child-process\fR
 .It
+\fB--allow-env\fR
+.It
 \fB--allow-ffi\fR
 .It
 \fB--allow-fs-read\fR

src/env.cc

@@ -952,8 +952,6 @@ Environment::Environment(IsolateData* isolate_data,
     if (!options_->allow_env.empty()) {
       permission()->Apply(
           this, options_->allow_env, permission::PermissionScope::kEnvVar);
-    } else {
-      permission()->Apply(this, {}, permission::PermissionScope::kEnvVar);
     }
 
     // Implicit allow entrypoint to kFileSystemRead

test/parallel/test-permission-env-deny-all.js

@@ -1,4 +1,4 @@
-// Flags: --permission --allow-fs-read=*
+// Flags: --permission --allow-fs-read=* --allow-child-process
 'use strict';
 
 const common = require('../common');
@@ -10,25 +10,46 @@ if (!isMainThread) {
 
 const assert = require('assert');
 
+// When --permission is used without --allow-env, env vars should be
+// freely accessible (backward compatible behavior).
 {
-  assert.ok(!process.permission.has('env'));
+  assert.ok(process.permission.has('env'));
 }
 
 {
-  assert.strictEqual(process.env.HOME, undefined);
-  assert.strictEqual(process.env.PATH, undefined);
+  // Environment variables should be readable
+  assert.ok(typeof process.env.HOME === 'string' || process.env.HOME === undefined);
 }
 
 {
-  assert.throws(() => {
-    process.env.TEST_VAR = 'value';
-  }, common.expectsError({
-    code: 'ERR_ACCESS_DENIED',
-    permission: 'EnvVar',
-  }));
+  // Setting env vars should work
+  process.env.__TEST_PERMISSION_ENV = 'test';
+  assert.strictEqual(process.env.__TEST_PERMISSION_ENV, 'test');
+  delete process.env.__TEST_PERMISSION_ENV;
 }
 
 {
+  // Object.keys should return env vars
   const keys = Object.keys(process.env);
-  assert.strictEqual(keys.length, 0);
+  assert.ok(keys.length > 0);
+}
+
+// Test that restriction activates when --allow-env is explicitly used
+{
+  const { spawnSync } = require('child_process');
+  const { status, stderr } = spawnSync(process.execPath, [
+    '--permission',
+    '--allow-fs-read=*',
+    '--allow-env=__NONEXISTENT_VAR__',
+    '-e',
+    `
+    const assert = require('assert');
+    assert.ok(!process.permission.has('env'));
+    assert.strictEqual(process.env.HOME, undefined);
+    assert.strictEqual(process.env.PATH, undefined);
+    assert.throws(() => { process.env.X = '1'; }, { code: 'ERR_ACCESS_DENIED' });
+    assert.strictEqual(Object.keys(process.env).length, 0);
+    `,
+  ]);
+  assert.strictEqual(status, 0, `child stderr: ${stderr}`);
 }