Skip to content

lib,permission: add permission.drop#62672

Open
RafaelGSS wants to merge 2 commits intonodejs:mainfrom
RafaelGSS:add-permission-drop
Open

lib,permission: add permission.drop#62672
RafaelGSS wants to merge 2 commits intonodejs:mainfrom
RafaelGSS:add-permission-drop

Conversation

@RafaelGSS
Copy link
Copy Markdown
Member

Refs: #62223

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
@RafaelGSS RafaelGSS requested a review from mcollina April 10, 2026 14:46
@RafaelGSS RafaelGSS added semver-minor PRs that contain new features and should be released in the next minor version. notable-change PRs with changes that should be highlighted in changelogs. permission Issues and PRs related to the Permission Model labels Apr 10, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg

@github-actions
Copy link
Copy Markdown
Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @RafaelGSS.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 10, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 10, 2026

Codecov Report

❌ Patch coverage is 72.78481% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.78%. Comparing base (586403f) to head (f36bd66).
⚠️ Report is 31 commits behind head on main.

Files with missing lines Patch % Lines
src/permission/fs_permission.cc 71.62% 17 Missing and 4 partials ⚠️
src/permission/permission.cc 84.61% 0 Missing and 8 partials ⚠️
lib/internal/process/permission.js 58.33% 5 Missing ⚠️
src/permission/addon_permission.cc 0.00% 3 Missing ⚠️
src/permission/inspector_permission.cc 0.00% 3 Missing ⚠️
src/permission/wasi_permission.cc 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62672      +/-   ##
==========================================
- Coverage   91.52%   89.78%   -1.74%     
==========================================
  Files         354      699     +345     
  Lines      148921   216392   +67471     
  Branches    23352    41364   +18012     
==========================================
+ Hits       136304   194296   +57992     
- Misses      12354    14214    +1860     
- Partials      263     7882    +7619     
Files with missing lines Coverage Δ
lib/internal/process/pre_execution.js 97.22% <100.00%> (+13.11%) ⬆️
src/permission/child_process_permission.cc 100.00% <100.00%> (ø)
src/permission/fs_permission.h 90.00% <ø> (ø)
src/permission/net_permission.cc 100.00% <100.00%> (ø)
src/permission/net_permission.h 100.00% <ø> (ø)
src/permission/permission.h 100.00% <ø> (ø)
src/permission/worker_permission.cc 100.00% <100.00%> (ø)
src/permission/addon_permission.cc 62.50% <0.00%> (ø)
src/permission/inspector_permission.cc 62.50% <0.00%> (ø)
src/permission/wasi_permission.cc 62.50% <0.00%> (ø)
... and 3 more

... and 465 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@RafaelGSS RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 10, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 10, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@absidue
Copy link
Copy Markdown

absidue commented Apr 10, 2026

As mentioned in my comments on the linked issue, I feel like it should be explicitly documented that this only drops permissions and does not close currently open file handles, sockets etc, that it is expected behaviour and is the app code's responsibility to close/release those if they are no longer required.

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++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. permission Issues and PRs related to the Permission Model semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants