Skip to content

lib,permission: add permission.drop#62672

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

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

Conversation

@RafaelGSS
Copy link
Copy Markdown
Member

Refs: #62223

@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 68.32298% with 51 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.64%. Comparing base (31da818) to head (db19643).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/permission/fs_permission.cc 71.62% 17 Missing and 4 partials ⚠️
lib/internal/process/permission.js 16.66% 10 Missing ⚠️
src/permission/permission.cc 84.61% 0 Missing and 8 partials ⚠️
src/permission/addon_permission.cc 0.00% 3 Missing ⚠️
src/permission/ffi_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   89.64%   89.64%   -0.01%     
==========================================
  Files         708      708              
  Lines      220399   220562     +163     
  Branches    42267    42303      +36     
==========================================
+ Hits       197577   197718     +141     
- Misses      14667    14700      +33     
+ Partials     8155     8144      -11     
Files with missing lines Coverage Δ
lib/internal/process/pre_execution.js 98.38% <100.00%> (+<0.01%) ⬆️
src/permission/child_process_permission.cc 100.00% <100.00%> (ø)
src/permission/ffi_permission.h 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%> (-37.50%) ⬇️
src/permission/ffi_permission.cc 50.00% <0.00%> (-30.00%) ⬇️
... and 5 more

... and 33 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.

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@RafaelGSS RafaelGSS force-pushed the add-permission-drop branch from f36bd66 to 995de8e Compare April 29, 2026 16:58
@nodejs-github-bot
Copy link
Copy Markdown
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++. 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