node
node copied to clipboard
nodejs
src,permission: resolve path on fs_permission
This resolves a known limitation of the permission model by using the new PathResolve method from the C++ side. Therefore, we don't need to resolve all paths on lib/fs.js when the pm is enabled, we do it only when checking is_granted
It's unlikely to happen, but this might affect users relying on resource: being always the absolute path. With this change, resource will be equal to the given param to fs.* calls.
cc: @nodejs/security-wg
Failed to start CI
⚠ No approving reviews found ✘ Refusing to run CI on potentially unsafe PRhttps://github.com/nodejs/node/actions/runs/8901141681
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Commit Queue failed
- Loading data for nodejs/node/pull/52761 ✔ Done loading data for nodejs/node/pull/52761 ----------------------------------- PR info ------------------------------------ Title src,permission: resolve path on fs_permission (#52761) Author Rafael Gonzagahttps://github.com/nodejs/node/actions/runs/8926738177(@RafaelGSS) Branch RafaelGSS:convert-paths-on-pm -> nodejs:main Labels c++, lib / src, author ready, needs-ci, permission Commits 3 - src,permission: resolve path on fs_permission - fixup! remove known limitation mention - fixup! src,permission: resolve path on fs_permission Committers 1 - RafaelGSS PR-URL: https://github.com/nodejs/node/pull/52761 Reviewed-By: Marco Ippolito ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/52761 Reviewed-By: Marco Ippolito -------------------------------------------------------------------------------- ℹ This PR was created on Tue, 30 Apr 2024 15:48:13 GMT ✔ Approvals: 1 ✔ - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/52761#pullrequestreview-2036027210 ✘ This PR needs to wait 119 more hours to land (or 0 hours if there is one more approval) ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-05-01T14:44:59Z: https://ci.nodejs.org/job/node-test-pull-request/58839/ - Querying data for job/node-test-pull-request/58839/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
It's unlikely to happen, but this might affect users relying on
resource:being always the absolute path. With this change,resourcewill be equal to the given param to fs.* calls.
The feature is experimental and/or deprecated, so breaking changes are permitted anyway, or am I mistaken?
Commit Queue failed
- Loading data for nodejs/node/pull/52761 ✔ Done loading data for nodejs/node/pull/52761 ----------------------------------- PR info ------------------------------------ Title src,permission: resolve path on fs_permission (#52761) Author Rafael Gonzagahttps://github.com/nodejs/node/actions/runs/8931416871(@RafaelGSS) Branch RafaelGSS:convert-paths-on-pm -> nodejs:main Labels c++, lib / src, author ready, needs-ci, permission Commits 3 - src,permission: resolve path on fs_permission - fixup! remove known limitation mention - fixup! src,permission: resolve path on fs_permission Committers 1 - RafaelGSS PR-URL: https://github.com/nodejs/node/pull/52761 Reviewed-By: Marco Ippolito Reviewed-By: Rich Trott ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/52761 Reviewed-By: Marco Ippolito Reviewed-By: Rich Trott -------------------------------------------------------------------------------- ℹ This PR was created on Tue, 30 Apr 2024 15:48:13 GMT ✔ Approvals: 2 ✔ - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/52761#pullrequestreview-2036027210 ✔ - Rich Trott (@Trott): https://github.com/nodejs/node/pull/52761#pullrequestreview-2037076261 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-05-02T15:52:15Z: https://ci.nodejs.org/job/node-test-pull-request/58839/ - Querying data for job/node-test-pull-request/58839/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 52761 From https://github.com/nodejs/node * branch refs/pull/52761/merge -> FETCH_HEAD ✔ Fetched commits as c5cfdd48497f..8e3a422dfa6b -------------------------------------------------------------------------------- Auto-merging src/node_modules.cc [main 75b0cdb2a9] src,permission: resolve path on fs_permission Author: RafaelGSS Date: Mon Apr 29 16:44:08 2024 -0300 20 files changed, 107 insertions(+), 136 deletions(-) delete mode 100644 test/known_issues/test-permission-model-path-resolution.js [main 90396f1840] fixup! remove known limitation mention Author: RafaelGSS Date: Tue Apr 30 12:49:25 2024 -0300 1 file changed, 2 deletions(-) [main 47716ed756] fixup! src,permission: resolve path on fs_permission Author: RafaelGSS Date: Tue Apr 30 12:52:09 2024 -0300 11 files changed, 31 insertions(+), 63 deletions(-) ✔ Patches applied There are 3 commits in the PR. Attempting autorebase. Rebasing (2/5) Rebasing (3/5) Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- src,permission: resolve path on fs_permission
Signed-off-by: RafaelGSS [email protected] PR-URL: https://github.com/nodejs/node/pull/52761 Reviewed-By: Marco Ippolito [email protected] Reviewed-By: Rich Trott [email protected]
[detached HEAD 93ede0c8c1] src,permission: resolve path on fs_permission Author: RafaelGSS [email protected] Date: Mon Apr 29 16:44:08 2024 -0300 20 files changed, 68 insertions(+), 129 deletions(-) delete mode 100644 test/known_issues/test-permission-model-path-resolution.js Rebasing (4/5) Rebasing (5/5)
Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- fixup! remove known limitation mention
PR-URL: https://github.com/nodejs/node/pull/52761 Reviewed-By: Marco Ippolito [email protected] Reviewed-By: Rich Trott [email protected]
[detached HEAD 2c028a1c18] fixup! remove known limitation mention Author: RafaelGSS [email protected] Date: Tue Apr 30 12:49:25 2024 -0300 1 file changed, 2 deletions(-)
Successfully rebased and updated refs/heads/main.
ℹ Add
commit-queue-squashlabel to land the PR as one commit, orcommit-queue-rebaseto land as separate commits.