GeezerVison is showing github.com — src,permission: do not throw on denied access in audit mode · nodejs/node@d53d582
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: src/permission/permission.h +20-13Lines changed: 20 additions & 13 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -35,9 +35,11 @@ namespace permission { | 35 | 35 | const auto resource__ = (resource); \ | 36 | 36 | if (!env__->permission()->is_granted(env__, perm__, resource__)) \ | 37 | 37 | [[unlikely]] { \ | 38 | | - node::permission::Permission::ThrowAccessDenied( \ | 39 | | - env__, perm__, resource__); \ | 40 | | - if (!env__->permission()->warning_only()) return __VA_ARGS__; \ | | 38 | + if (!env__->permission()->warning_only()) { \ | | 39 | + node::permission::Permission::ThrowAccessDenied( \ | | 40 | + env__, perm__, resource__); \ | | 41 | + return __VA_ARGS__; \ | | 42 | + } \ | 41 | 43 | } \ | 42 | 44 | } while (0) | 43 | 45 |
| @@ -49,9 +51,11 @@ namespace permission { | 49 | 51 | const auto resource__ = (resource); \ | 50 | 52 | if (!env__->permission()->is_granted(env__, perm__, resource__)) \ | 51 | 53 | [[unlikely]] { \ | 52 | | - node::permission::Permission::AsyncThrowAccessDenied( \ | 53 | | - env__, (wrap), perm__, resource__); \ | 54 | | - if (!env__->permission()->warning_only()) return __VA_ARGS__; \ | | 54 | + if (!env__->permission()->warning_only()) { \ | | 55 | + node::permission::Permission::AsyncThrowAccessDenied( \ | | 56 | + env__, (wrap), perm__, resource__); \ | | 57 | + return __VA_ARGS__; \ | | 58 | + } \ | 55 | 59 | } \ | 56 | 60 | } while (0) | 57 | 61 |
| @@ -63,14 +67,17 @@ namespace permission { | 63 | 67 | const auto resource__ = (resource); \ | 64 | 68 | if (!env__->permission()->is_granted(env__, perm__, resource__)) \ | 65 | 69 | [[unlikely]] { \ | 66 | | - Local<Value> err_access; \ | 67 | | - if (node::permission::CreateAccessDeniedError(env__, perm__, resource__) \ | 68 | | - .ToLocal(&err_access)) { \ | 69 | | - args.GetReturnValue().Set(err_access); \ | 70 | | - } else { \ | 71 | | - args.GetReturnValue().Set(UV_EACCES); \ | | 70 | + if (!env__->permission()->warning_only()) { \ | | 71 | + Local<Value> err_access; \ | | 72 | + if (node::permission::CreateAccessDeniedError( \ | | 73 | + env__, perm__, resource__) \ | | 74 | + .ToLocal(&err_access)) { \ | | 75 | + args.GetReturnValue().Set(err_access); \ | | 76 | + } else { \ | | 77 | + args.GetReturnValue().Set(UV_EACCES); \ | | 78 | + } \ | | 79 | + return __VA_ARGS__; \ | 72 | 80 | } \ | 73 | | - return __VA_ARGS__; \ | 74 | 81 | } \ | 75 | 82 | } while (0) | 76 | 83 |
| |
Collapse file Copy file name to clipboard +59Lines changed: 59 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +'use strict'; | | 2 | + | | 3 | +const common = require('../common'); | | 4 | +const { isMainThread } = require('worker_threads'); | | 5 | + | | 6 | +if (!isMainThread) { | | 7 | + common.skip('This test only works on a main thread'); | | 8 | +} | | 9 | + | | 10 | +const assert = require('assert'); | | 11 | +const { spawnSync } = require('child_process'); | | 12 | +const { test } = require('node:test'); | | 13 | +const fixtures = require('../common/fixtures'); | | 14 | + | | 15 | +const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md'); | | 16 | + | | 17 | +function runAudit(mode) { | | 18 | + const childScript = ` | | 19 | + const dc = require('node:diagnostics_channel'); | | 20 | + const msgs = []; | | 21 | + dc.subscribe('node:permission-model:fs', (m) => msgs.push({ | | 22 | + permission: m.permission, | | 23 | + resource: m.resource, | | 24 | + })); | | 25 | + try { | | 26 | + ${mode === 'eval' ? | | 27 | + `eval('require("node:fs").readFileSync(process.env.BLOCKED_FILE)');` : | | 28 | + `require('node:fs').readFileSync(process.env.BLOCKED_FILE);`} | | 29 | + console.log('RESULT NO_THROW'); | | 30 | + } catch (e) { | | 31 | + console.log('RESULT THREW ' + e.code); | | 32 | + } | | 33 | + console.log('AUDIT ' + JSON.stringify(msgs)); | | 34 | + `; | | 35 | + | | 36 | + const env = { ...process.env, BLOCKED_FILE: blockedFile }; | | 37 | + const { status, stdout, stderr } = spawnSync( | | 38 | + process.execPath, | | 39 | + ['--permission-audit', '-e', childScript], | | 40 | + { encoding: 'utf8', env }, | | 41 | + ); | | 42 | + assert.strictEqual(status, 0, stderr); | | 43 | + const lines = stdout.split('\n'); | | 44 | + assert.ok(lines.includes('RESULT NO_THROW'), stdout); | | 45 | + const auditLine = lines.find((l) => l.startsWith('AUDIT ')); | | 46 | + assert.ok(auditLine, stdout); | | 47 | + const msgs = JSON.parse(auditLine.replace('AUDIT ', '')); | | 48 | + assert.strictEqual(msgs.length, 1); | | 49 | + assert.strictEqual(msgs[0].permission, 'FileSystemRead'); | | 50 | + assert.ok(msgs[0].resource.endsWith('protected-file.md')); | | 51 | +} | | 52 | + | | 53 | +test('permission-audit logs fs denial without throwing', () => { | | 54 | + runAudit('direct'); | | 55 | +}); | | 56 | + | | 57 | +test('permission-audit logs fs denial without throwing (eval)', () => { | | 58 | + runAudit('eval'); | | 59 | +}); |
|