Improving automated remote connection via Inspector Protcol #348

mmarchini · 2020-01-15T20:23:26Z

The Inspector Protocol allow us to use tools like V8 CPU Profiler and Heap Snapshots without relying on C++ binding. It also allows us to call those tools remotely, which is useful when running those tools in production via an automated workflow. We can connect to the Inspector Protocol remotely using the following methods:

node inspect host:port will open a REPL for users to run inspector commands
Chrome DevTools, ndb, etc. will provide a nice UI
Connecting via pure WebSockets (for example, via websocat or ws)

1 is not useful for automation, because it doesn't allow users to run scripts (it only works interactively). 2 has similar problems (since those are GUI), plus they also enable the Debugger domain by default, which is not desirable in production. 3 is the best option for automation, but it requires extra environment setup (installing websocat, ws. etc) which is not always possible during an outage, for example.

I would like to propose an alternative: in core, provide a simple way to interact with the inspector protocol on a remote process with automation scripts. Some implementation examples would be:

Allow users to run scripts on node inspect instead of opening a REPL
Allow users to connect to a remote host via the Inspector API (either via a connectToRemoteHost method or with a new RemoteSession class)
Via shortcut commands for the tools, which could be run against a remote process. For example: node diagnostics --cpu-profile --pid $(pgrep node)

Personally, I think the 2 option would be the most flexible and useful for our users, but I want to hear what other members of the WG thing.

Overall, I'm trying to ensure we can trigger those tools given the following constraints (which are common in enterprise environment):

No need to make changes to the application code (as changes can take days, weeks or months to propagate, depending on the company)
No need to install extra dependencies on the fly (companies can have strict rules against mutating the server, or installing dependencies can be forbidden by security until those dependencies are cleared as safe)
Trigger the tools on an already running process, without the need to stop it
Trigger the tool non-interactively / without intervention from a human (users should be able to trigger it from a bash script, for example).

The text was updated successfully, but these errors were encountered:

mhdawson · 2020-01-17T17:28:31Z

+1 on those requirements. Based on past discussion I also think we need to add some requirements around security as that has been a blocker for using the inspector protocol in the past. I think the concerns expressed were about making a security connection but also around limiting what you can do when connected versus what is available today.

mmarchini · 2020-01-17T18:57:03Z

Agreed, security should be taken into account, but that should be solved on the server/main-process side (and it's probably worth its own issue), otherwise we would still be able to bypass security with a raw websocket client.

mhdawson · 2020-01-21T15:04:57Z

@mmarchini I don't think that can address "limiting what you can do when connected versus what is available today.". That is about reducing available functionality when connecting to the inspector.

mmarchini · 2020-01-21T22:08:04Z

I don't think I agree. Implementing the limitations on the client side means only users using that particular client will be barred from using privileged features on the inspector protocol. Any other clients will be able to bypass it, since the inspector protocol is just an exchange of Websocket messages.

If we want to make the inspector protocol more secure, we need to allow the process being debugged to determine which inspector features are allowed and which ones are denied.

(maybe we are saying the same thing. Yes, we should be able to connect to a process with limited functionality. But who determines what is limited should not be client, otherwise an attacker would be able to always connect to a process with unlimited access)

mhdawson · 2020-01-22T14:08:53Z

the process being debugged to determine which inspector features are allowed and which ones are denied.

Yes we are saying the same thing. It should be the process that controls what can be used through the inspector protocol.

gireeshpunathil · 2020-01-23T06:32:46Z

+1 to everything what was said.
in terms of capability, telling inspector to inhibit the client from modifying program state will do?

mhdawson · 2020-01-30T20:58:11Z

@gireeshpunathil I think we'd probably have to have a table of what you can do through the inspector and then indicate what should/should not be possible

naugtur · 2020-01-31T13:02:18Z

It should be configurable in a similar fashion to the restrictions to child process spawning and network access. From what I saw in the Security WG last time I checked there may not be a final decision as to what it's going to be, but I'm bringing it up here because I'd like to suggest it should not be modifiable in runtime by code from node_modules.

IMHO, If a restriction to what can be done via inspector was set, it should remain unchanged for the lifetime of the process if it's supposed to be helpful for security.

Also, is it ok to put the functionality in node itself? Would calling node diagnostics --cpu-profile... start another process identified as node? In an environment where this kind of tool would run it could be confusing. I know it was just an example. But is shipping a separate binary a concern? I don't have much context around that.

github-actions · 2020-07-16T00:36:40Z