Skip to content

src,permission: add --allow-inspector ability #59711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
nodejs-github-bot merged 2 commits into from
Sep 11, 2025
Merged

src,permission: add --allow-inspector ability #59711

nodejs-github-bot merged 2 commits into from
Sep 11, 2025
+112 −5

Conversation

@RafaelGSS
Copy link
Member

@RafaelGSS RafaelGSS commented Sep 1, 2025 •
edited
Loading

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

nodejs-github-bot commented Sep 1, 2025

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 •
edited
Loading

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 commented Sep 2, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/69009/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 7, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/69122/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 8, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/69133/

@RafaelGSS
Copy link
Member Author

RafaelGSS commented Sep 8, 2025 •
edited
Loading

Ready to review @nodejs/security-wg

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 8, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/69140/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 9, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/69161/

@RafaelGSS RafaelGSS added the review wanted PRs that need reviews. label Sep 9, 2025
santigimeno
santigimeno approved these changes Sep 11, 2025
View reviewed changes
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.

lib/internal/process/permission.js
Comment on lines 42 to +43
'--allow-net',
'--allow-inspector',
Copy link
Member

@santigimeno santigimeno Sep 11, 2025

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

@RafaelGSS RafaelGSS Sep 11, 2025

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

nodejs-github-bot commented Sep 11, 2025

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.

This was referenced Oct 9, 2025
Merged
Closed
RafaelGSS added a commit to RafaelGSS/node that referenced this pull request Oct 13, 2025
@RafaelGSS
src,permission: add --allow-inspector ability
c8316fe 
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 mentioned this pull request Oct 13, 2025
Closed
@RafaelGSS RafaelGSS removed the backport-requested-v24.x PRs awaiting manual backport to the v24.x-staging branch. label Oct 13, 2025
@RafaelGSS RafaelGSS added the backport-open-v24.x Indicate that the PR has an open backport label Oct 13, 2025
targos pushed a commit that referenced this pull request Dec 5, 2025
@RafaelGSS @targos
src,permission: add --allow-inspector ability
92ea669 
Refs: #48534
PR-URL: #59711
Backport-PR-URL: #60248
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
@github-actions github-actions bot mentioned this pull request Dec 9, 2025
Merged
nodejs-github-bot added a commit that referenced this pull request Dec 9, 2025
@nodejs-github-bot
2025-12-10, Version 24.12.0 'Krypton' (LTS)
0239873 
Notable changes:

http:
  * (SEMVER-MINOR) add optimizeEmptyRequests server option (Rafael Gonzaga) #59778
lib:
  * (SEMVER-MINOR) add options to util.deprecate (Rafael Gonzaga) #59982
module:
  * (SEMVER-MINOR) mark type stripping as stable (Marco Ippolito) #60600
node-api:
  * (SEMVER-MINOR) add napi_create_object_with_properties (Miguel Marcondes Filho) #59953
sqlite:
  * (SEMVER-MINOR) allow setting defensive flag (Bart Louwers) #60217
src:
  * (SEMVER-MINOR) add watch config namespace (Marco Ippolito) #60178
  * (SEMVER-MINOR) add an option to make compile cache portable (Aditi) #58797
src,permission:
  * (SEMVER-MINOR) add --allow-inspector ability (Rafael Gonzaga) #59711
v8:
  * (SEMVER-MINOR) add cpu profile (theanarkh) #59807

PR-URL: #61001
targos pushed a commit that referenced this pull request Dec 9, 2025
@nodejs-github-bot @targos
2025-12-10, Version 24.12.0 'Krypton' (LTS)
70f6b58 
Notable changes:

http:
  * (SEMVER-MINOR) add optimizeEmptyRequests server option (Rafael Gonzaga) #59778
lib:
  * (SEMVER-MINOR) add options to util.deprecate (Rafael Gonzaga) #59982
module:
  * (SEMVER-MINOR) mark type stripping as stable (Marco Ippolito) #60600
node-api:
  * (SEMVER-MINOR) add napi_create_object_with_properties (Miguel Marcondes Filho) #59953
sqlite:
  * (SEMVER-MINOR) allow setting defensive flag (Bart Louwers) #60217
src:
  * (SEMVER-MINOR) add watch config namespace (Marco Ippolito) #60178
  * (SEMVER-MINOR) add an option to make compile cache portable (Aditi) #58797
src,permission:
  * (SEMVER-MINOR) add --allow-inspector ability (Rafael Gonzaga) #59711
v8:
  * (SEMVER-MINOR) add cpu profile (theanarkh) #59807

PR-URL: #61001
targos pushed a commit that referenced this pull request Dec 10, 2025
@nodejs-github-bot @targos
2025-12-10, Version 24.12.0 'Krypton' (LTS)
47d8b88 
Notable changes:

http:
  * (SEMVER-MINOR) add optimizeEmptyRequests server option (Rafael Gonzaga) #59778
lib:
  * (SEMVER-MINOR) add options to util.deprecate (Rafael Gonzaga) #59982
module:
  * (SEMVER-MINOR) mark type stripping as stable (Marco Ippolito) #60600
node-api:
  * (SEMVER-MINOR) add napi_create_object_with_properties (Miguel Marcondes Filho) #59953
sqlite:
  * (SEMVER-MINOR) allow setting defensive flag (Bart Louwers) #60217
src:
  * (SEMVER-MINOR) add watch config namespace (Marco Ippolito) #60178
  * (SEMVER-MINOR) add an option to make compile cache portable (Aditi) #58797
src,permission:
  * (SEMVER-MINOR) add --allow-inspector ability (Rafael Gonzaga) #59711
v8:
  * (SEMVER-MINOR) add cpu profile (theanarkh) #59807

PR-URL: #61001
@a2425rdl
Copy link

a2425rdl commented Dec 18, 2025

Great

1 similar comment
@a2425rdl
Copy link

a2425rdl commented Dec 18, 2025

Great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@santigimeno santigimeno santigimeno approved these changes

@juanarbol juanarbol juanarbol approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-open-v24.x Indicate that the PR has an open backport 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. permission Issues and PRs related to the Permission Model review wanted PRs that need reviews. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@RafaelGSS @nodejs-github-bot @targos @a2425rdl @santigimeno @juanarbol