Skip to content

Commit 44aa875

Browse files
nabeel378nabeel378
committed
permission: update documentation and tests for --allow-env flag behavior
Signed-off-by: nabeel378 <[email protected]>
1 parent 591de5d commit 44aa875

15 files changed

Lines changed: 50 additions & 56 deletions

‎doc/api/cli.md‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -193,18 +193,16 @@ directly through the process arguments.
193193

194194
### `--allow-env`
195195

196+
<!-- YAML
197+
added: REPLACEME
198+
-->
199+
196200
> Stability: 1.1 - Active development
197201
198202
When using the [Permission Model][], access to environment variables is
199-
unrestricted by default. To restrict access, the `--allow-env` flag must be
200-
explicitly passed when starting Node.js. Once `--allow-env` is specified,
201-
only the listed environment variables are accessible.
202-
203-
* Reading a restricted variable (`process.env.HOME`) silently returns
204-
`undefined`.
205-
* Writing (`process.env.FOO = 'bar'`) or deleting (`delete process.env.FOO`)
206-
a restricted variable throws `ERR_ACCESS_DENIED`.
207-
* Enumerating (`Object.keys(process.env)`) only returns allowed variables.
203+
restricted by default. Reading a restricted variable silently returns
204+
`undefined`, while writing or deleting throws `ERR_ACCESS_DENIED`.
205+
Enumerating (`Object.keys(process.env)`) only returns allowed variables.
208206

209207
The valid arguments for the `--allow-env` flag are:
210208

@@ -216,17 +214,17 @@ Examples:
216214

217215
```console
218216
$ node --permission --allow-fs-read=* -p 'process.env.HOME'
219-
/Users/user
220-
$ node --permission --allow-env=PATH --allow-fs-read=* -p 'process.env.HOME'
221217
undefined
222218
$ node --permission --allow-env=HOME --allow-fs-read=* -p 'process.env.HOME'
223219
/Users/user
220+
$ node --permission --allow-env=* --allow-fs-read=* -p 'process.env.HOME'
221+
/Users/user
224222
```
225223

226224
Attempting to write a restricted variable throws:
227225

228226
```console
229-
$ node --permission --allow-env=HOME --allow-fs-read=* -e "process.env.FOO = 'bar'"
227+
$ node --permission --allow-fs-read=* -e "process.env.FOO = 'bar'"
230228
node:internal/process/per_thread:12
231229
throw new ERR_ACCESS_DENIED('EnvVar', name);
232230
^

‎doc/api/process.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,12 +1560,11 @@ The `process.env` property returns an object containing the user environment.
15601560
See environ(7).
15611561
15621562
When the [Permission Model][] is enabled, access to environment variables is
1563-
unrestricted by default. To restrict access, the `--allow-env` flag must be
1564-
explicitly passed. Once `--allow-env` is specified, reading a restricted
1565-
variable silently returns `undefined`, while writing or deleting throws
1566-
`ERR_ACCESS_DENIED`. Use `--allow-env=*` to grant access to all variables or
1567-
`--allow-env=HOME,PATH` to grant access to specific ones. See the
1568-
[`--allow-env`][] documentation for more details.
1563+
restricted by default. Reading a restricted variable silently returns
1564+
`undefined`, while writing or deleting throws `ERR_ACCESS_DENIED`. Use
1565+
`--allow-env=*` to grant access to all variables or `--allow-env=HOME,PATH`
1566+
to grant access to specific ones. See the [`--allow-env`][] documentation
1567+
for more details.
15691568
15701569
An example of this object looks like:
15711570
@@ -3197,6 +3196,7 @@ The available scopes are:
31973196
* `fs.write` - File System write operations
31983197
* `child` - Child process spawning operations
31993198
* `worker` - Worker thread spawning operation
3199+
* `env` - Environment variable operations
32003200
* `ffi` - Foreign function interface operations
32013201
32023202
```js

‎src/env.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -949,7 +949,9 @@ Environment::Environment(IsolateData* isolate_data,
949949
permission()->Apply(this, {"*"}, permission::PermissionScope::kWASI);
950950
}
951951

952-
if (!options_->allow_env.empty()) {
952+
if (options_->allow_env.empty()) {
953+
permission()->Apply(this, {}, permission::PermissionScope::kEnvVar);
954+
} else {
953955
permission()->Apply(
954956
this, options_->allow_env, permission::PermissionScope::kEnvVar);
955957
}

‎test/parallel/test-permission-child-process-inherit-flags.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --permission --allow-child-process --allow-fs-read=* --allow-worker
1+
// Flags: --permission --allow-child-process --allow-fs-read=* --allow-worker --allow-env=*
22
'use strict';
33

44
const common = require('../common');

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

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --permission --allow-fs-read=* --allow-child-process
1+
// Flags: --permission --allow-fs-read=*
22
'use strict';
33

44
const common = require('../common');
@@ -10,46 +10,31 @@ if (!isMainThread) {
1010

1111
const assert = require('assert');
1212

13-
// When --permission is used without --allow-env, env vars should be
14-
// freely accessible (backward compatible behavior).
13+
// When --permission is used without --allow-env, all env vars should be denied
14+
// (deny-by-default, consistent with other permission flags).
1515
{
16-
assert.ok(process.permission.has('env'));
16+
assert.ok(!process.permission.has('env'));
1717
}
1818

1919
{
20-
// Environment variables should be readable
21-
assert.ok(typeof process.env.HOME === 'string' || process.env.HOME === undefined);
20+
// Reading a denied variable should silently return undefined
21+
assert.strictEqual(process.env.HOME, undefined);
22+
assert.strictEqual(process.env.PATH, undefined);
2223
}
2324

2425
{
25-
// Setting env vars should work
26-
process.env.__TEST_PERMISSION_ENV = 'test';
27-
assert.strictEqual(process.env.__TEST_PERMISSION_ENV, 'test');
28-
delete process.env.__TEST_PERMISSION_ENV;
26+
// Writing a denied variable should throw
27+
assert.throws(() => { process.env.__TEST_PERMISSION_ENV = 'test'; },
28+
{ code: 'ERR_ACCESS_DENIED' });
2929
}
3030

3131
{
32-
// Object.keys should return env vars
33-
const keys = Object.keys(process.env);
34-
assert.ok(keys.length > 0);
32+
// Deleting a denied variable should throw
33+
assert.throws(() => { delete process.env.__TEST_PERMISSION_ENV; },
34+
{ code: 'ERR_ACCESS_DENIED' });
3535
}
3636

37-
// Test that restriction activates when --allow-env is explicitly used
3837
{
39-
const { spawnSync } = require('child_process');
40-
const { status, stderr } = spawnSync(process.execPath, [
41-
'--permission',
42-
'--allow-fs-read=*',
43-
'--allow-env=__NONEXISTENT_VAR__',
44-
'-e',
45-
`
46-
const assert = require('assert');
47-
assert.ok(!process.permission.has('env'));
48-
assert.strictEqual(process.env.HOME, undefined);
49-
assert.strictEqual(process.env.PATH, undefined);
50-
assert.throws(() => { process.env.X = '1'; }, { code: 'ERR_ACCESS_DENIED' });
51-
assert.strictEqual(Object.keys(process.env).length, 0);
52-
`,
53-
]);
54-
assert.strictEqual(status, 0, `child stderr: ${stderr}`);
38+
// Enumerating should return empty
39+
assert.strictEqual(Object.keys(process.env).length, 0);
5540
}

‎test/parallel/test-permission-fs-read.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
1+
// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-child-process --allow-env=*
22
'use strict';
33

44
const common = require('../common');
@@ -36,6 +36,7 @@ const commonPath = path.join(__filename, '../../common');
3636
// Do not uncomment this line
3737
// `--allow-fs-read=${file}`,
3838
`--allow-fs-read=${commonPathWildcard}`,
39+
'--allow-env=*',
3940
file,
4041
],
4142
{

‎test/parallel/test-permission-fs-symlink-target-write.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
1+
// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-child-process --allow-env=*
22
'use strict';
33

44
const common = require('../common');
@@ -44,6 +44,7 @@ fs.writeFileSync(path.join(readWriteFolder, 'file'), 'NO evil file contents');
4444
'--permission',
4545
`--allow-fs-read=${file}`, `--allow-fs-read=${commonPathWildcard}`, `--allow-fs-read=${readOnlyFolder}`, `--allow-fs-read=${readWriteFolder}`,
4646
`--allow-fs-write=${readWriteFolder}`, `--allow-fs-write=${writeOnlyFolder}`,
47+
'--allow-env=*',
4748
file,
4849
],
4950
{

‎test/parallel/test-permission-fs-symlink.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
1+
// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-child-process --allow-env=*
22
'use strict';
33

44
const common = require('../common');
@@ -57,6 +57,7 @@ const traversalSymlink = path.join(allowedFolder, 'deep1', 'deep2', 'deep3', 'go
5757
`--allow-fs-read=${file}`, `--allow-fs-read=${commonPathWildcard}`, `--allow-fs-read=${symlinkFromBlockedFile}`,
5858
`--allow-fs-read=${allowedFolder}`,
5959
`--allow-fs-write=${symlinkFromBlockedFile}`,
60+
'--allow-env=*',
6061
file,
6162
],
6263
{

‎test/parallel/test-permission-fs-traversal-path.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-child-process
1+
// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-child-process --allow-env=*
22
'use strict';
33

44
const common = require('../common');
@@ -40,6 +40,7 @@ const commonPathWildcard = path.join(__filename, '../../common*');
4040
'--permission',
4141
`--allow-fs-read=${file}`, `--allow-fs-read=${commonPathWildcard}`, `--allow-fs-read=${allowedFolder}`,
4242
`--allow-fs-write=${allowedFolder}`,
43+
'--allow-env=*',
4344
file,
4445
],
4546
{

‎test/parallel/test-permission-fs-write.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --permission --allow-fs-read=* --allow-child-process
1+
// Flags: --permission --allow-fs-read=* --allow-child-process --allow-env=*
22
'use strict';
33

44
const common = require('../common');
@@ -33,6 +33,7 @@ const file = fixtures.path('permission', 'fs-write.js');
3333
'--permission',
3434
'--allow-fs-read=*',
3535
`--allow-fs-write=${regularFile}`, `--allow-fs-write=${commonPath}`,
36+
'--allow-env=*',
3637
file,
3738
],
3839
{

0 commit comments

Comments
 (0)