8000 src,permission: add --allow-inspector ability by RafaelGSS · Pull Request #59711 · nodejs/node · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@RafaelGSS
Copy link
Member
@RafaelGSS RafaelGSS commented Sep 1, 2025

Refs: #48534

Remaining questions:

  • How should it behave when connecting through a Websocket? Should the net permissions deny this access?
    • Answer: We no longer create the InspectorIO thread when no --allow-inspector

The test case I included makes sure we won't reintroduce 3df13d5

cc: @nodejs/security-wg

@RafaelGSS RafaelGSS added the permission Issues and PRs related to the Permission Model label Sep 1, 2025
@nodejs-github-bot
Copy link
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. labels Sep 1, 2025
@codecov
Copy link
codecov bot commented Sep 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.26%. Comparing base (255dd7b) to head (350d7ef).
⚠️ Report is 303 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59711      +/-   ##
==========================================
- Coverage   89.93%   88.26%   -1.68%     
==========================================
  Files         667      701      +34     
  Lines      196790   206759    +9969     
  Branches    38423    39757    +1334     
==========================================
+ Hits       176982   182494    +5512     
- Misses      12240    16281    +4041     
- Partials     7568     7984     +416     
Files with missing lines Coverage Δ
lib/internal/process/permission.js 77.08% <100.00%> (+0.48%) ⬆️
lib/internal/process/pre_execution.js 98.51% <100.00%> (+2.03%) ⬆️
src/env.cc 80.80% <100.00%> (-0.08%) ⬇️
src/node_options.cc 77.89% <100.00%> (-6.45%) ⬇️
src/node_options.h 97.90% <100.00%> (+0.02%) ⬆️

... and 125 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 added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 2, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@RafaelGSS
Copy link
Member Author
RafaelGSS commented Sep 8, 2025

Ready to review @nodejs/security-wg

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@RafaelGSS RafaelGSS added the review wanted PRs that need reviews. label Sep 9, 2025
Copy link
Member
@santigimeno santigimeno left a comment

Choose a reason for hiding this comment

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

LGTM with a nit.

Comment on lines 42 to +43
'--allow-net',
'--allow-inspector',
Copy link
Member

Choose a reason for hiding this comment

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

Use alphabetical order here?

Suggested change
'--allow-net',
'--allow-inspector',
'--allow-inspector',
'--allow-net',

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do it in a follow-up PR so I don't need to re-run the tests

@RafaelGSS RafaelGSS added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 11, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 11, 2025
@nodejs-github-bot nodejs-github-bot merged commit 29738c7 into nodejs:main Sep 11, 2025
64 of 65 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 29738c7

@targos targos added the backport-requested-v24.x PRs awaiting manual backport to the v24.x-staging branch. label Sep 12, 2025
@targos
Copy link
Member
targos commented Sep 12, 2025

There are many conflicts with v24.x-staging.

RafaelGSS added a commit to RafaelGSS/node that referenced this pull request Oct 13, 2025
Refs: nodejs#48534
PR-URL: nodejs#59711
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
@RafaelGSS RafaelGSS removed the backport-requested-v24.x PRs awaiting manual backport to the v24.x-staging branch. label Oct 13, 2025