feat: allow --experimental-inspector-network-resource node flag#49689
Conversation
|
💖 Thanks for opening this pull request! 💖 Semantic PR titlesWe use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of commit messages with semantic prefixes:
Commit signingThis repo enforces commit signatures for all incoming PRs. PR tipsThings that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
de0736f to
6b79489
Compare
|
@electron-cation please add semver/minor |
6b79489 to
3657c89
Compare
codebytere
left a comment
There was a problem hiding this comment.
This also needs to be documented - see https://github.com/electron/electron/blob/5ddd8a020035970094df86efe8c266b3cca011ab/docs/api/command-line-switches.md
Done. Please review once if the documentation looks fine. |
|
@parthtaneja0001 can you rebase this PR with the latest from main? That should get CI building. |
5918fa3 to
8b34c21
Compare
Rebased onto the latest main and resolved conflicts.Waiting for CI to run. |
nikwen
left a comment
There was a problem hiding this comment.
I triggered CI.
Requesting changes to make sure this doesn't get merged before @electron/wg-security takes a look. (Please dismiss my review once that has happened.)
codebytere
left a comment
There was a problem hiding this comment.
LGTM from a security perspective. The flag operates within the already-privileged inspector context (where arbitrary code execution is already possible), so the risk is imo low.
One suggestion: docs should note that enabling this flag allows the inspector to make outbound network requests to resolve source maps.
Note: When enabled, the Node.js inspector will make network requests to URLs specified in source maps. Be mindful of this in environments where the process has access to internal networks.|
updated the docs with the security note |
|
@nikwen is there any other review left? |
|
I've dismissed my review. @codebytere is still in "request changes" mode. |
b9909ae to
c0c74a9
Compare
|
rebased to latest main to get the CI building |
|
@codebytere do you still have changes requested? |
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
Release Notes Persisted
|
|
I have automatically backported this PR to "41-x-y", please check out #51377 |
|
I have automatically backported this PR to "42-x-y", please check out #51378 |
Fixes #49500
Description of Change
This PR adds
--experimental-inspector-network-resourceto Electron’sNode.js CLI allowlist so it can be passed through to Node.
This enables remote source map resolution in Chrome DevTools for
main and utility processes when using the Node.js inspector.
Checklist
npm testpassesRelease Notes
Notes: Allowed the
--experimental-inspector-network-resourceNode.js flag to be passed through Electron.