From a81d5e11f3f8988d60ce6cd586281715681ab97a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Thu, 31 Aug 2023 08:58:59 +0200 Subject: [PATCH] doc,test: add known path resolution issue in permission model As a side effect of 205f1e643e25648173239b2de885fec430268492, Node.js now resolves some paths differently when the permission model is enabled. While these are mostly edge cases, they are worth mentioning in the documentation. This commit also adds a known_issues test that demonstrates one such difference. PR-URL: https://github.com/nodejs/node/pull/49155 Reviewed-By: Rafael Gonzaga --- doc/api/permissions.md | 2 + .../test-permission-model-path-resolution.js | 46 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 test/known_issues/test-permission-model-path-resolution.js diff --git a/doc/api/permissions.md b/doc/api/permissions.md index 5c5c13541c51e5..33e24f49dd5a7e 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -560,6 +560,8 @@ Wildcards are supported too: There are constraints you need to know before using this system: +* When the permission model is enabled, Node.js may resolve some paths + differently than when it is disabled. * Native modules are restricted by default when using the Permission Model. * OpenSSL engines currently cannot be requested at runtime when the Permission Model is enabled, affecting the built-in crypto, https, and tls modules. diff --git a/test/known_issues/test-permission-model-path-resolution.js b/test/known_issues/test-permission-model-path-resolution.js new file mode 100644 index 00000000000000..909fc64da1f50a --- /dev/null +++ b/test/known_issues/test-permission-model-path-resolution.js @@ -0,0 +1,46 @@ +'use strict'; + +// The permission model resolves paths to avoid path traversals, but in doing so +// it potentially interprets paths differently than the operating system would. +// This test demonstrates that merely enabling the permission model causes the +// application to potentially access a different file than it would without the +// permission model. + +const common = require('../common'); + +const assert = require('assert'); +const { execFileSync } = require('child_process'); +const { mkdirSync, symlinkSync, writeFileSync } = require('fs'); +const path = require('path'); + +if (common.isWindows) + assert.fail('not applicable to Windows'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const a = path.join(tmpdir.path, 'a'); +const b = path.join(tmpdir.path, 'b'); +const c = path.join(tmpdir.path, 'c'); +const d = path.join(tmpdir.path, 'c/d'); + +writeFileSync(a, 'bad'); +symlinkSync('c/d', b); +mkdirSync(c); +mkdirSync(d); +writeFileSync(path.join(c, 'a'), 'good'); + +function run(...args) { + const interestingPath = `${tmpdir.path}/b/../a`; + args = [...args, '-p', `fs.readFileSync(${JSON.stringify(interestingPath)}, 'utf8')`]; + return execFileSync(process.execPath, args, { encoding: 'utf8' }).trim(); +} + +// Because this is a known_issues test, we cannot assert any assumptions besides +// the known issue itself. Instead, do a sanity check and report success if the +// sanity check fails. +if (run() !== 'good') { + process.exit(0); +} + +assert.strictEqual(run('--experimental-permission', `--allow-fs-read=${tmpdir.path}`), 'good');