Skip to content

lib: handle --permission-audit when propagating flags - #63047

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
RafaelGSS:fix-permission-audit-child-process-env
May 6, 2026
Merged

lib: handle --permission-audit when propagating flags#63047
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
RafaelGSS:fix-permission-audit-child-process-env

Conversation

@RafaelGSS

Copy link
Copy Markdown
Member

As titled.

I have also included --permission-audit in the check of FFI permissions.

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
@RafaelGSS RafaelGSS added the permission Issues and PRs related to the Permission Model. label Apr 30, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Apr 30, 2026
@codecov

codecov Bot commented Apr 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 43.75000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.65%. Comparing base (a7f487f) to head (b1ec186).
⚠️ Report is 62 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/process/permission.js 44.44% 5 Missing ⚠️
lib/child_process.js 0.00% 2 Missing ⚠️
lib/ffi.js 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63047      +/-   ##
==========================================
+ Coverage   89.63%   89.65%   +0.01%     
==========================================
  Files         708      708              
  Lines      220402   220423      +21     
  Branches    42271    42270       -1     
==========================================
+ Hits       197551   197610      +59     
+ Misses      14706    14662      -44     
- Partials     8145     8151       +6     
Files with missing lines Coverage Δ
lib/child_process.js 95.45% <0.00%> (-0.10%) ⬇️
lib/ffi.js 95.56% <60.00%> (-1.16%) ⬇️
lib/internal/process/permission.js 92.75% <44.44%> (-7.25%) ⬇️

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

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>

@ShogunPanda ShogunPanda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@RafaelGSS RafaelGSS added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs with CI started, the required approvals, and no outstanding review comments. labels May 4, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 4, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@RafaelGSS RafaelGSS added the commit-queue PRs queued for automated landing through the Commit Queue. label May 6, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label May 6, 2026
@nodejs-github-bot
nodejs-github-bot merged commit dc99d18 into nodejs:main May 6, 2026
87 of 88 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in dc99d18

aduh95 pushed a commit that referenced this pull request May 19, 2026
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: #63047
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request May 19, 2026
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: #63047
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
theSnackOverflow added a commit to theSnackOverflow/node that referenced this pull request Aug 30, 2026
--permission-audit is documented to perform permission checks without
denying access, publishing each violation through the diagnostics
channel while execution continues. Three code paths still enforced:

* fs.lstat() and fs.symlink() are checked in the JavaScript layer,
  which threw ERR_ACCESS_DENIED without consulting audit mode. The
  isAuditMode() helper introduced for ffi was never wired into fs.
* Addon loading was disabled upfront in Environment setup, so
  process.dlopen() failed with ERR_DLOPEN_DISABLED before reaching
  the audit-aware permission check, and no audit event was published.

Skip the throw in the JavaScript checks when audit mode is on, keeping
the permission.has() call first so the diagnostics channel event is
still published. Leave native addons enabled in audit mode while still
applying the kAddon scope, so DLOpen() publishes the denial through
the channel and continues to the regular loading path. This also
covers the worker path, which inherits kNoNativeAddons from the same
option.

Each fs API carries its own copy of the check in its sync, callback
and promise flavour, so the regression test exercises all six call
sites, asserting the --permission enforce behaviour as a control.

Fixes: nodejs#65419
Refs: nodejs#63047
Assisted-by: claude:opus-5
Signed-off-by: Ji Hoon Kang <ivory.ma9ic@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. child_process Issues and PRs related to the child_process subsystem. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run. permission Issues and PRs related to the Permission Model. process Issues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants