Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v22.x backport] permission: ignore internalModuleStat on module loading #56058

Open
wants to merge 1 commit into
base: v22.x-staging
Choose a base branch
from

Conversation

RafaelGSS
Copy link
Member

This improves Permission Model usage when allowing read access to specific modules. To achieve that, the permission model check on internalModuleStat has been removed meaning that on module loading, uv_fs_stat is performed on files and folders even when the permission model is enabled. Although a uv_fs_stat is performed, reading/executing the module will still pass by the permission model check.

Without this PR when an app tries to --allow-fs-read=./a.js --allow-fs-read=./b.js where a attempt to load b, it will fails as it reads $pwd and no permission has been given to this path.

PR-URL: #55797
Reviewed-By: Yagiz Nizipli [email protected]
Reviewed-By: Ulises Gascón [email protected]

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. v22.x v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. labels Nov 28, 2024
This improves Permission Model usage when allowing read access to
specifi modules. To achieve that, the permission model check on
internalModuleStat has been removed meaning that on module loading,
uv_fs_stat is performed on files and folders even when the permission
model is enabled. Although a uv_fs_stat is performed, reading/executing
the module will still pass by the permission model check.

Without this PR when an app tries to --allow-fs-read=./a.js
--allow-fs-read=./b.js where `a` attempt to load b, it will fails as
it reads $pwd and no permission has been given to this path.

PR-URL: nodejs#55797
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
@RafaelGSS RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 29, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 29, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. v22.x v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants