Skip to content

[v24.x] Backport permission updates to v24 - #65354

Open
RafaelGSS wants to merge 8 commits into
nodejs:v24.x-stagingfrom
RafaelGSS:backport-permission-updates-to-v24
Open

[v24.x] Backport permission updates to v24#65354
RafaelGSS wants to merge 8 commits into
nodejs:v24.x-stagingfrom
RafaelGSS:backport-permission-updates-to-v24

Conversation

@RafaelGSS RafaelGSS added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 17, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/security-wg

@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. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch. labels Aug 17, 2026
@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.73109% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.28%. Comparing base (9e39360) to head (499c38a).

Files with missing lines Patch % Lines
src/permission/fs_permission.cc 71.62% 17 Missing and 4 partials ⚠️
src/permission/permission.cc 83.05% 3 Missing and 17 partials ⚠️
lib/internal/process/permission.js 58.33% 5 Missing ⚠️
src/permission/inspector_permission.cc 0.00% 3 Missing ⚠️
src/permission/wasi_permission.cc 0.00% 3 Missing ⚠️
src/node_binding.cc 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           v24.x-staging   #65354      +/-   ##
=================================================
- Coverage          90.31%   90.28%   -0.03%     
=================================================
  Files                711      711              
  Lines             228425   228644     +219     
  Branches           43160    43200      +40     
=================================================
+ Hits              206293   206425     +132     
- Misses             14090    14100      +10     
- Partials            8042     8119      +77     
Files with missing lines Coverage Δ
lib/internal/process/pre_execution.js 98.18% <100.00%> (+0.52%) ⬆️
src/env.cc 80.72% <100.00%> (-0.12%) ⬇️
src/node_options.cc 76.58% <100.00%> (+0.02%) ⬆️
src/node_options.h 97.42% <100.00%> (-0.51%) ⬇️
src/permission/addon_permission.cc 100.00% <100.00%> (ø)
src/permission/child_process_permission.cc 100.00% <100.00%> (ø)
src/permission/fs_permission.h 91.78% <ø> (ø)
src/permission/permission.h 100.00% <100.00%> (+16.66%) ⬆️
src/permission/worker_permission.cc 100.00% <100.00%> (ø)
src/node_binding.cc 82.42% <0.00%> (-0.33%) ⬇️
... and 5 more

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

@RafaelGSS
RafaelGSS force-pushed the backport-permission-updates-to-v24 branch from 3b93be2 to 01e422d Compare August 17, 2026 19:57
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 17, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

RafaelGSS and others added 8 commits August 17, 2026 19:21
Add --permission-audit flag that enables the permission model in
warning-only mode. Instead of throwing ERR_ACCESS_DENIED, it emits
a message via diagnostics channel and allows the operation to
continue.

Publish permission check results to per-scope diagnostics channels
(e.g., node:permission-model:fs) so users can observe permission
decisions at runtime via diagnostics_channel.

Refs: nodejs#59935
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: nodejs#62672
Refs: nodejs#62223
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Signed-off-by: Martin <martin@asymmetric.re>
PR-URL: nodejs#64007
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
The THROW_IF_INSUFFICIENT_PERMISSIONS and
ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS macros called
ThrowAccessDenied/AsyncThrowAccessDenied unconditionally and only
guarded the `return` with `warning_only()`.
ERR_ACCESS_DENIED_IF_INSUFFICIENT_PERMISSIONS had no `warning_only()`
guard at all — it always set the access-denied error and returned.
As a result, running with `--permission-audit` still produced
ERR_ACCESS_DENIED on any denied operation (fs, net, child_process,
worker, addon, ffi, inspector, wasi), defeating the audit-only purpose
of the flag.

Guard the denied-error path behind `!warning_only()` in all three
macros. In audit mode, the diagnostics-channel message is published
(already done in Permission::is_scope_granted) and execution continues;
in enforce mode (`--permission`), behavior is unchanged — the error is
raised and the call returns.

The tests cover both the direct (top-level) call and an `eval()`-wrapped
call: the direct call exercises the normal script path, and the
`eval()`-wrapped call exercises the V8 script-context boundary (the
diagnostics subscriber is registered in the outer module context while
the denied operation runs inside an eval'd string).

Refs: nodejs@9ddd1a9
Signed-off-by: Adrian Estrada <edsadr@gmail.com>
PR-URL: nodejs#64426
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Expand the documentation for the --permission-audit flag, which was
fixed in 51c09ea to no longer throw ERR_ACCESS_DENIED on denied
operations. The previous docs only had a two-sentence description in
cli.md and no mention in the permissions guide or process.permission
API docs.

- permissions.md: add enforce vs audit mode overview, a new
  "Audit Mode" subsection listing the diagnostics channel names
  (node:permission-model:*) and the { permission, resource } message
  shape, and a usage example. Update the Runtime API section to
  mention both --permission and --permission-audit.
- cli.md: expand the --permission-audit section to clarify that
  --permission is not required, --allow-* flags are not needed,
  errors are not thrown, and --permission takes precedence when both
  are set. Add a cross-reference from --permission to
  --permission-audit.
- process.md: note that process.permission is available under both
  flags, and clarify permission.has() and permission.drop() behavior
  in audit mode.
- node.1: regenerated via `make node.1`.

Refs: nodejs#64426
Signed-off-by: Adrian Estrada <edsadr@gmail.com>
PR-URL: nodejs#64791
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Adds unique warning codes of the form PERM0000 for all permissions
related SecurityWarnings, so that they can be individually silenced
if required.

Fixes: nodejs#59818
Signed-off-by: David Evans <davidje13@users.noreply.github.com>
PR-URL: nodejs#64414
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS
RafaelGSS force-pushed the backport-permission-updates-to-v24 branch from 01e422d to 499c38a Compare August 17, 2026 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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. semver-minor PRs that contain new features and should be released in the next minor version. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants