-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
doc: add security section to debugging guides #1613
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job. but was this too long for a "getting started" guide of "debugging"? what about creating a new doc for security and link here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
### Browsers, WebSockets and same-origin policy | ||
|
||
Websites open in a web-browser can make WebSocket and HTTP requests under the | ||
browser security model. A initial HTTP connection is necessary to obtain a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: A initial
-> An initial
.
$ node --inspect server.js | ||
``` | ||
|
||
Now, on your local machine from where you want initiate a debug client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: want initiate
-> want to initiate
.
threat. We suggest you ensure appropriate firewalls and access controls in place | ||
to prevent a security exposure. | ||
|
||
See the section on 'Enabling remote debugging scenarios' on some advice on how |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: maybe it is worth to add a link to #enabling-remote-debugging-scenarios
?
I think a discussion of the security risks needs to be up-front in the debugging guide. It might be possible have a summary section and link to a detailed document elsewhere. Let me take a look at refactoring. |
@marswong I took a deeper look at refactoring this PR into two document, and I don't think there is a refactor into two documents that would be satisfactory:
In summary, I think we should keep this a single doc. Even though this doc is called the 'Debugging Getting Started' guide, in reality it is actually the 'Debugging Guide'. I think security implications should stay in this doc. I would like to land this as-is. WDYT? |
@ofrobots Can you please fix the little nits @vsemozhetbyt mentioned, and I think we are good to go. If we feel the need to restructure the document, we can do so at a later point. |
Thank you! |
@ofrobots good job :) |
This talks about the security implications a bit more, in light of https://nodejs.org/en/blog/vulnerability/march-2018-security-releases/#node-js-inspector-dns-rebinding-vulnerability-cve-2018-7160