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

Conversation

@RafaelGSS
Copy link
Member

@RafaelGSSRafaelGSS 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

@RafaelGSSRafaelGSS 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-botnodejs-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

codecovbot 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 linesCoverage Δ
lib/internal/process/permission.js77.08% <100.00%> (+0.48%)⬆️
lib/internal/process/pre_execution.js98.51% <100.00%> (+2.03%)⬆️
src/env.cc80.80% <100.00%> (-0.08%)⬇️
src/node_options.cc77.89% <100.00%> (-6.45%)⬇️
src/node_options.h97.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.

@RafaelGSSRafaelGSS 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
MemberAuthor

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/

@RafaelGSSRafaelGSS 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

@santigimenosantigimeno 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

@santigimenosantigimenoSep 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
MemberAuthor

@RafaelGSSRafaelGSSSep 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

@RafaelGSSRafaelGSS 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-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 11, 2025
@nodejs-github-botnodejs-github-bot merged commit 29738c7 into nodejs:mainSep 11, 2025
64 of 65 checks passed
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 11, 2025

Landed in 29738c7

@targostargos 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 <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Signed-off-by: RafaelGSS <[email protected]>
@RafaelGSSRafaelGSS mentioned this pull request Oct 13, 2025
Closed
@RafaelGSSRafaelGSS removed the backport-requested-v24.x PRs awaiting manual backport to the v24.x-staging branch. label Oct 13, 2025
@RafaelGSSRafaelGSS 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 <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Signed-off-by: RafaelGSS <[email protected]>
@github-actionsgithub-actionsbot 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

This was referenced Jan 3, 2026
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@santigimenosantigimenosantigimeno approved these changes

@juanarboljuanarboljuanarbol approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.backport-open-v24.xIndicate that the PR has an open backportc++Issues and PRs that require attention from people who are familiar with C++.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.permissionIssues and PRs related to the Permission Modelreview wantedPRs that need reviews.semver-minorPRs 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