Skip to content

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

Open
RafaelGSS wants to merge 6 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 6 commits into
nodejs:v24.x-stagingfrom
RafaelGSS:backport-permission-updates-to-v24

Conversation

@RafaelGSS

Copy link
Copy Markdown
Member

cc: @aduh95

RafaelGSS and others added 3 commits August 17, 2026 12:36
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
PR-URL: nodejs#61869
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
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>
@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.40586% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.28%. Comparing base (9e39360) to head (3b93be2).

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 ⚠️
src/node_trace_events.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.04%     
=================================================
  Files                711      711              
  Lines             228425   228642     +217     
  Branches           43160    43202      +42     
=================================================
+ Hits              206293   206420     +127     
- Misses             14090    14093       +3     
- Partials            8042     8129      +87     
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.93% <100.00%> (+0.01%) ⬆️
src/node_report.cc 93.13% <ø> (+0.13%) ⬆️
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%> (ø)
... and 7 more

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

edsadr and others added 3 commits August 17, 2026 15:49
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 3b93be2 to 01e422d Compare August 17, 2026 19:57
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. 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.

5 participants