Skip to content

permission: add unique warning codes - #64414

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
davidje13:codes-for-permissions-warnings
Aug 2, 2026
Merged

permission: add unique warning codes#64414
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
davidje13:codes-for-permissions-warnings

Conversation

@davidje13

Copy link
Copy Markdown
Contributor

Adds unique warning codes of the form PERM0000 for all permissions-related SecurityWarnings, so that they can be individually (and minimally) silenced if required, e.g. --disable-warning=PERM0002. See #59818 (comment) for an example use-case.

There isn't much of an existing pattern for warning codes, except DEP0000 for deprecations, so this follows that convention. There are currently 2 other places which generate SecurityWarnings which should perhaps be given unique codes as well (something for a separate PR I think)

Fixes: #59818

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Jul 10, 2026
@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.13%. Comparing base (db3a8d8) to head (bdad8d7).
⚠️ Report is 141 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64414      +/-   ##
==========================================
- Coverage   90.14%   90.13%   -0.01%     
==========================================
  Files         741      741              
  Lines      242133   242131       -2     
  Branches    45568    45563       -5     
==========================================
- Hits       218265   218248      -17     
+ Misses      15371    15370       -1     
- Partials     8497     8513      +16     
Files with missing lines Coverage Δ
lib/internal/process/pre_execution.js 97.93% <100.00%> (-0.01%) ⬇️

... and 30 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 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, add a test for --disable-warning for this too.

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>
@davidje13
davidje13 force-pushed the codes-for-permissions-warnings branch from df10aff to bdad8d7 Compare July 21, 2026 23:08
@davidje13

Copy link
Copy Markdown
Contributor Author

Added. Also rebased on latest main

@RafaelGSS RafaelGSS added request-ci Add this label to start a Jenkins CI on a PR. permission Issues and PRs related to the Permission Model labels Jul 22, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 22, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@davidje13

Copy link
Copy Markdown
Contributor Author

Looks like this has been approved and is passing all the tests - is anything stopping it being merged?

@RafaelGSS RafaelGSS added commit-queue Add this label to land a pull request using GitHub Actions. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jul 29, 2026
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 29, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/64414
✔  Done loading data for nodejs/node/pull/64414
----------------------------------- PR info ------------------------------------
Title      permission: add unique warning codes (#64414)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     davidje13:codes-for-permissions-warnings -> nodejs:main
Labels     process, author ready, needs-ci, permission
Commits    2
 - permission: add unique warning codes
 - Add explicit test of disable-warning behaviour
Committers 1
 - David Evans <davidje13@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/64414
Fixes: https://github.com/nodejs/node/issues/59818
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/64414
Fixes: https://github.com/nodejs/node/issues/59818
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 10 Jul 2026 23:04:36 GMT
   ✔  Approvals: 3
   ✔  - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs/node/pull/64414#pullrequestreview-4754274143
   ✔  - Ulises Gascón (@UlisesGascon): https://github.com/nodejs/node/pull/64414#pullrequestreview-4760099362
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/64414#pullrequestreview-4764529814
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-07-23T13:24:06Z: https://ci.nodejs.org/job/node-test-pull-request/75053/
- Querying data for job/node-test-pull-request/75053/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 64414
From https://github.com/nodejs/node
 * branch                  refs/pull/64414/merge -> FETCH_HEAD
✔  Fetched commits as 7168c6e49c4b..bdad8d740529
--------------------------------------------------------------------------------
Auto-merging lib/internal/process/pre_execution.js
[main ed2567fd6d] permission: add unique warning codes
 Author: David Evans <davidje13@users.noreply.github.com>
 Date: Fri Jul 10 23:53:14 2026 +0100
 2 files changed, 10 insertions(+), 12 deletions(-)
[main eccf09174e] Add explicit test of disable-warning behaviour
 Author: David Evans <davidje13@users.noreply.github.com>
 Date: Tue Jul 21 23:41:19 2026 +0100
 1 file changed, 14 insertions(+)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
(node:359) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.
(Use `node --trace-deprecation ...` to show where the warning was created)
Rebasing (2/4)
Executing: git node land --amend --yes
   ⚠  Found Fixes: https://github.com/nodejs/node/issues/59818, skipping..
--------------------------------- New Message ----------------------------------
permission: add unique warning codes

Adds unique warning codes of the form PERM0000 for all permissions
related SecurityWarnings, so that they can be individually silenced
if required.

Fixes: #59818
Signed-off-by: David Evans <davidje13@users.noreply.github.com>
PR-URL: #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>

[detached HEAD 9a65a770a6] permission: add unique warning codes
Author: David Evans <davidje13@users.noreply.github.com>
Date: Fri Jul 10 23:53:14 2026 +0100
2 files changed, 10 insertions(+), 12 deletions(-)
Rebasing (3/4)
Rebasing (4/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
Add explicit test of disable-warning behaviour

PR-URL: #64414
Fixes: #59818
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

[detached HEAD 4fe03a2800] Add explicit test of disable-warning behaviour
Author: David Evans <davidje13@users.noreply.github.com>
Date: Tue Jul 21 23:41:19 2026 +0100
1 file changed, 14 insertions(+)
Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/30471676057

@jasnell jasnell added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 2, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 2, 2026
@nodejs-github-bot
nodejs-github-bot merged commit d1f3d0a into nodejs:main Aug 2, 2026
90 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in d1f3d0a

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. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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.

Specific code for "The flag X must be used with extreme caution"

5 participants