delve dap: supporting "console" property and debugging with root privileges

[hyangah]

This is an implementation proposal for feature requests:
#124 debug: support "console" property in launch.json
#558 debug: abiltiy to debug with root privileges
and many others

Implementation proposal

The console attribute provides users an option to specify how to launch and interact with the debugged program (debugee).

• internalConsole (default): the debugee output is shown in the default debug console. (e.g. VS Code Debug Console). The program does not have access to stdin.
• integratedTerminal: The debugee will run in a terminal emulator integrated in IDE (e.g. VS Code Integrated Terminal) and have access to the usual terminal control codes and stdin/stdout/stderr.
• externalTerminal: The debugee runs in a separate console window outside of IDE.

Support for integrated and external terminals are available via DAP’s RunInTerminal request and can address many issues that stemmed from inability to access terminal from debugged programs. See @polinasok’s summary for the known issues.

Overview

Currently, vscode-go extension integrates dlv dap by implementing an inlined debug adapter that spawns dlv dap, connects to the dlv dap server through a TCP port, converts dlv dap's stdout/stderr into OutputEvent, and proxies all the DAP communication between VS Code and dlv dap.

By default dlv runs the program to debug as a foreground process, so the debugged program can access to the terminal tty if dlv dap runs from a regular terminal. However, vscode-go's thin adapter runs without tty so dlv and the debugged program do not have tty in the current setup.

We propose to extend the inlined debug adapter to support the console property by delegating the job of starting dlv to VS Code. VS Code can access the integrated terminals and the external terminals. In order to debug a program with root privileges, the debugger (dlv) has to run with root privileges. Access to the terminal allows us to run sudo dlv.

When a debug session starts,

• VS Code starts a debug adapter (vscode-go thin adapter).
  note: at this point, the debug adapter already knows the value of console property for the session.
• VS Code sends an initialize request with the editor's capabilities (e.g. supportsRunInTerminal).
• The thin adapter starts a single use TCP server for rendezvous, and sends a runInTerminal command (dlv dap --client-addr=single_use_server_addr). If asRoot property is specified, the adapter will request to run sudo dlv dap command.
• VS Code runs dlv dap from the specified terminal.
• The dlv dap command connects to the single use TCP server and the connection between dlv dap and the thin adapter is estabilished.
• Thin adapter starts to proxying between VS Code and dlv dap.

DAP RunInTerminal or VS Code's terminal APIs

There are two different approaches for letting VS Code run dlv dap from the integrated terminals

• Utilizing VS Code's support for DAP's RunInTerminal.
• Utilizing VS Code's terminal APIs.

VS Code's terminal API provides great flexibility compared to the VS Code's RunInTerminal feature which is accessible only through DAP. However, we choose to utilize VS Code's RunInTerminal implementation because:

• Correctly implementing our own debug session management around the terminal API and duplicating VS Code's RunInTerminal implementation is a work.
• VS Code's RunInTerminal supports external terminal mode, too. We don't have to worry about platform-specific implementation to support external terminal mode.
• VS Code's RunInTerminal implementation is maintained by VS Code team, and proven to work for many other debug extensions already.

Violation of DAP spec

Strictly speaking, sending a RunInTerminal request before initialize response is incorrect. According to DAP spec:

Until the debug adapter has responded to with an ‘initialize’ response, the client must not send any additional requests or events to the debug adapter.

In addition the debug adapter is not allowed to send any requests or events to the client until it has responded with an ‘initialize’ response.

One option to correctly using RunInTerminal will look like:

1. first respond with an initialize response specifying minimal capabilities, stash the original initialize request, and complete the initialize message handshake.
2. send runInTerminal.
3. dlv dap starts and connection is established.
4. forward the stashed initialize request to dlv dap.
5. upon receiving the initialize response from dlv dap, compute the difference in capabilities and send an capabilities event to VS Code.

We see VS Code accept RunInTerminal arriving before initialize response empirically. We understand relying on this unintended behavior is not ideal, but this simplifies the implementation significantly.

Another option we are considering is to propose to expose the RunInTerminal implementation as part of VS Code's debug API. That will even allow us to set up the connection with dlv dap before VS Code sends the initialize request. See microsoft/vscode#136523

Implementation

• go-delve/delve#2568: add --client-addr flag to run dap with a predefined client

• vscode-go console_mode

  • cl/358515: add rendezvous TCP server
  • cl/358516: use --client-addr when console=integrated or external
  • cl/358536: start dlv-dap after receiving initialize request (previously, we started before receiving initialize request)
  • cl/358618: support console mode by issuing a runInTerminal request

• vscode-go supports asRoot property

• cosmetic changes

  • cl/361101: adjust env computation to avoid flooding the terminal with many env variables

• TODO

  • Let dlv dap handle env property so environment variables are just passed through launch configuration instead of runInTerminal.
  • Let dlv dap send log messages as OutputEvent to avoid polluting the terminal.

Caveats

• This is an editor-specific solution. Other editors cannot benefit from this work.
• On Mac OS, delve relies on debugserver for launching and tracing the debug target. When users issue Ctrl+C from the terminal, the debugserver receives it and exits without properly detaching or terminating the debug target. This is a known issue and we don't know how to handle this properly yet.
• The terminal is a fully functioning terminal and users can use it after debug session ends. And, the terminal is reused for the identically named debug session. That may be sometimes surprising for some users.

Prior Art and Alternatives considered

According to DAP Spec, RunInTerminal “is typically used to launch the debuggee in a terminal provided by the client.”

How to launch the debugee is an implementation detail.

Many debug adapters maintain a clear separation between the adapter and the debugger. Even when they share a single frontend, often they have a special “launcher” command or script that starts the target program with a debugger attacher, or starts the target program with the runtime debugging functionality enabled.

A typical launch request handling, that does not involve an integrated/external terminal, starts the target program with the launcher. Then the adapter communicates with the debugger or the runtime. When an integrated/external terminal is needed, those debug adapters just need to ask the editor to run the same launch command.

Here are some examples:
(DISCLAIMER: I gathered the following info by skimming through their source, so it’s possible that some details here may not be quite right. Let me know if need correction)

Debugpy

debugpy is a debugger that implements DAP. Their implementation is the primary inspiration of this proposal, so it shares much commonality. When debugpy server receives a launch request, it launches the debugee using the launcher programwhich connects back to the debug adapter at start (similar to the rendezvous port in this proposal) and adds DAP handler. And the debugpy server works as a proxy and delegates request handling to the handler started by the launcher.

When an integrated/external terminal is needed, the debug adapter sends the launch command to the editor instead of running it directly. A typical launch command sent with RunInTerminal looks like this:

$ /usr/bin/env /usr/local/bin/python3 \
   /python-extension/pythonFiles/lib/python/debugpy/launcher 57491 \
   -- /Users/gopher/project/helloworld.py 

To enable debugging with root privileges, the launcher will start with sudo.

Reference: Debug Adapter code

The old python debug adapter ptvsd is also implemented in the same way. See the implementation https://github.com/microsoft/ptvsd/tree/master/src/ptvsd.

MIEngine (c++ debug adapter, open-source)

MIEngine is an open-sourced version of C++ debug adapter VS Code is using. The debug adapter and the debugger are separate, and the debug adapter launches the debugger and debugee (platform-dependent). Unlike python debug adapters that communicate mostly over a TCP socket with the launcher, it sets up pipes (or named pipes for Windows) for communication with the debugger when launching the debugger using RunInTerminal.

For debugging with root privileges, it launches the debugger with the sudo command.
Reference: RunInTerminalTransport.cs code

LLDB Debug Adapter (codelldb)

This is a debug adapter for LLDB, so the roles of debug adapter and the debugger are cleanly separated. It’s possible to run a LLDB command using RunInTerminal, but they took a different path. The debug adapter (codelldb) requests the editor to run the codelldb “in a terminal agent mode”. The terminal agent mode codelldb provides access to the tty. The adapter waits on a TCP socket waiting for the terminal agent mode codelldb dials in, provides the tty info, and connects the tty with LLDB/debugee’s stdio streams. This allows the debug adapter to manage the debugee.

Running LLDB as root is not directly supported. Users either have to disable the security feature, or use the remote debugging (start lldb-server in platform mode and connect to it).

Alternatives considered

Launching only the debugee in the terminal and attaching

We considered launching the target program and attach to the program using the PID information RunInTerminal response may include. microsoft/vscode#61640 (comment)

(+): Only the exact target program command appears in the terminal, so it may look cleaner.
(-): I don’t know an easy way to start a process in suspended mode. It may be possible to write a separate special launcher to achieve the job (maybe reuse some of Delve’s pkg/proc or service/debugger code), that needs work too.
(-): The pid field of RunInTerminal response may not be available. See microsoft/vscode#61640 (comment)
(-): Delve dap still needs changes to handle a new mode in its teardown logic - it looks like attach mode internally, but it is a launch and we need cleanup. This may not be too difficult though.
(-): Support for debugging with root privileges will be tricky. To attach to a program that runs as a root, either we need to run delve as a root or ask users to remove all security features.

Launching a tty agent like codelldb

We considered writing our own terminal agent and connecting the Terminal’s tty with debugged program’s stdio streams.

I am not sure how much work is necessary to implement cross-platform solutions.

(+): Looks cleaner from the terminal. Log or error messages from Delve will not pollute the terminal.
(+): Favored by a delve dev and has a higher chance of getting the RunInTerminal implementation added from the dlv dap.
(-): Delve's tty flag will take less used code path, and we need a terminal agent that works in all target platforms.
(-): Support for debugging with root privileges can't be implemented with this (codelldb doesn’t support directly).

Using VS Code terminal API

In VS Code, we can create our own terminal and start dlv dap from the terminal by sending the command using VS Code Terminal API before starting a debug session using DebugAdapterDescriptorFactory.createDebugAdapterDescriptor API.

(+): Change in Delve is unnecessary.
(-): This is an editor specific solution, and other DAP clients have to implement their own hacks.
(-): We cannot take advantage of VS Code team’s engineering effort.
(-): With the Terminal API, we can send the command text and there is no feedback. We need a separate mechanism to detect when dlv dap server is actually up and running. Options:

• Create a pipe (platform-specific solution), pass it to –log-dest, and watch the stream.
• Keep polling to see the process appear as the child of the shell process and cross fingers.
• Change Delve to dial back or signal us explicitly.

---

Prior proposals

Proposal 1 (2021/07): Support RunInTerminal request and console mode from dlv dap

Initially we proposed to implement the RunInTerminal support and the proxying logic inside dlv dap. This would allow dlv dap to work as a fully functioning debug adapter, and other editors can benefit from it.

However, this adds complexity to Delve side. Given that RunInTerminal is not part of traditional debugger's functionality, and supporting features beyond what's necessary for debugging (i.e. `RunInTerminal) is still questionable, this proposal is held off.

The dlv dap command tightly integrates the debug-adapter-as-server functionality and the traditional debugger functionality. For the sake of simplicity, this document will use "Delve (server)” to refer to the debug-adapter-as-server functionality, and “Delve (debugger)” to refer to the debugger functionality.

When receiving a launch request with the console property, Delve (server) will ask the editor to start the Delve (debugger) using the RunInTerminal request. Then the Editor will start the Delve (debugger) in the specified console. To know exactly when the Delve (debugger) is ready, Delve (adapter) will open a TCP port for rendezvous. Once the Delve (debugger) is ready, it will dial into the rendezvous port. Once they are connected, Delve (server) will forward the cached initialize request and launch request and work as a simple proxy between the Editor and the Delve (debugger).

• RunInTerminal response includes a valid process ID (note: sometimes it's empty :-(). If RunInTerminal response succeeds but the Delve (debugger) fails to connect within a certain time limit, Delve (server) will send SIGINT to the process and terminates the debug session.
• Rendezvous port will be closed soon after Delve (debugger) arrives.
• Implementation-wise, Delve (debugger) shares the same DAP handler logic, but does not run a TCP server that accepts from DAP clients.
• Delve (debugger) only works for this debug session and exits when the debug session ends. Multiple client support doesn't apply.
• Delve (server) forwards the client’s capabilities information because they affect the DAP handler logic.
• Delve (server) will not forward RunInTerminal requests from Delve (debugger). That allows arbitrary code execution in the editor side, and we want to close this attack vector.
• So far, we assumed Delve (server) started with the right environment variable and skipped processing of the env attribute. In this proposal, Delve (debugger) runs in a separate terminal, we need to tell Delve (debugger) to use the same environment variables. RunInTerminal accepts ‘env’ parameter, but prepends ‘env’ in the command line. Sending the full list of environment variables as RunInTerminal ‘env’ parameter floods the terminal. Either we need to shrink the environment variable list or make Delve (debugger) recognize the env attribute.

[hyangah]

@polinasok @suzmue @derekparker @aarzilli @weinand

[aarzilli]

LLDB's solution seems like the cleanest one to me, having a generic DAP proxy inside delve is pretty depressing (I've been saying this for years, the RunInTerminal thing was designed backwards it shouldn't be the debugger's job to decide whether it actually wants to be in a terminal after it has already been started). For unix-like systems the --tty option should work (modulo fixing any bugs on rarely used codepaths), I have no idea how to do it on windows, I see that the target adapter just sends back its own PID but what do they do with it?

[hyangah]

If my proposed --client-addr flag option is available, i am also considering a vscode specific solution instead of pursuing runinterminal approach further.

[vadimcn]

Could you please clarify what questions you are seeking my input on?

[hyangah]

👋 @vadimcn we want to understand how you implemented the 'console' property support (runinterminal), the role of terminal agent mode, if there is anything special with windows, windows/wsl, and maybe how lldb dap implemented supports for debugging as a root.

[vadimcn]

Well, as you've noticed, on Linux and OSX, I have the terminal agent process return the tty name, to which I then redirect debuggee's stdio handles.
On Windows, consoles cannot be named. lnstead, you attach to a console by using a PID of some process already attached to that console. So I have the terminal agent send me its PID, attach to it, then launch debuggee, which inherits the console.

I don't support launching as root from VSCode. It's a pretty niche scenario, and is easy to handle via remote debugging.

[hyangah]

Thanks @vadimcn - that confirms what @aarzilli later researched. I am not familiar with windows so it is still magical to me - I think this is how the terminal agent's pid is used - https://github.com/vadimcn/vscode-lldb/blob/9f05570a5bae4d1e8101593a771e0d2bd26cda87/adapter/src/terminal.rs#L102 which calls https://docs.microsoft.com/en-us/windows/console/attachconsole . Am I guessing right?

Unlike lldb-server that can take the target program/args as a command line args, dlv dap accepts arbitrary launch requests any time and can be a more convenient target to exploit.

[aarzilli]

For LLDB integration details (...)

Looks like they are calling AttachConsole and then presumably something else calls GetStdHandle. It looks reasonable, the --tty parameter already exists and it would be nice to make it work for windows as well. It doesn't look like it would be much code at all.

[hyangah]

Interesting. If it's possible in all platforms, are you recommending to make dlv work as a terminal agent too? We need something to run in the terminal (to get tty/pid info reliably).

[aarzilli]

Interesting. If it's possible in all platforms, are you recommending to make dlv work as a terminal agent too?

Yes, that would be an appropriate use of a hidden cli command.

[hyangah]

This is a real problem vscode users face frequently. My proposal avoids running dlv dap for arbitrary binary execution as a root.

[aarzilli]

You're still going to have a delve process, running as root, connected to a user process. If you are already able to run programs on a user's machine I think all you have to do to compromise vscode is drop a bunch of javascript files in a folder, hardly a problem.

If you want to have a reverse connect option on dlv dap, so that it connects directly to the client instead of listening, we can do that.

[hyangah]

Thanks for the input.

I think we can implement this console & root support without using RunInTerminal option if the reverse connect option is available. Instead of using the default dlv dap, vscode-go will start dlv dap with the reverse connect option from the right console and with the requested privilege. Actually, I like this option better for VS Code Go plugin than my original RunInTerminal proposal because we already have a thin adapter and this will also help simplifying the thin adapter's dlv start sequence.

I will update my proposal to favor VSCode specific approach.

Other editors and plugins can do the same - launch dlv dap with a reverse connect option from the right terminal. Or, implementing a proxy DA on top of the github.com/go-delve/delve/blob/master/service/dap package and use it instead of dlv dap shouldn't be too hard. That way we can keep dlv's DAP less cluttered and focus on only debugger-related aspect of DAP.

[weinand]

Yes, not using RunInTerminal in a DA is perfectly fine. RunInTerminal is just a convenience mechanism for those DAs that need a simple way to launch something in an integrated or external terminal.
But since debug extensions typically have regular VS Code extension code anyway, they can implement their own sophisticated strategy for dealing with integrated terminals easily.

One correction about a statement from above:

...the RunInTerminal thing was designed backwards it shouldn't be the debugger's job to decide whether it actually wants to be in a terminal after it has already been started

It is not the debugger's job to call RunInTerminal. It can only be the DA's job to call RunInTerminal. And the DA can use RunInTerminal when it makes sense, but it doesn't have to.
And in cases where the DA and the debugger are one entity, it certainly doesn't make sense to use RunInTerminal.

[puremourning]

But since debug extensions typically have regular VS Code extension code anyway, they can implement their own sophisticated strategy for dealing with integrated terminals easily.

That extension code only works in vscode though and every DAP client author has to manually write It per adapter. Doesn't scale.

[puremourning]

The original proposal is great, and exactly how users and debug adapter client implementers expect things to work.

I will update my proposal to favor VSCode specific approach.

It's pretty disappointing if that original, standard proposal, is scrapped in favour of "Vscode-specific approach", also known as "requiring every debug adapter client author to write adapter-specific-code (or worse, as suggested above and entire actual adapter!) in order for it to be usable". I thought this kind of decision making was limited to LSP servers and has mostly been avoided thus far in DAP-land (as you have nicely summarised above). Has a final decision been made on this? If so, is there any scope for reviewing this decision?

It wouldn't be so bad actually if the default behaviour of delve was to send the stdout of the program over the DAP channel. It currently doesn't do this so there's no way to even see the stdout of the debugged program currently with most otherwise-protocol-conforming clients. I just added support for delve dap to vimspector and the first user who tried it said "can't see the stdout", which I can understand being a blocker. Having to re-engineer the complex DAP startup process, or "implementing a proxy DA on top of the github.com/go-delve/delve/blob/master/service/dap package and use it instead of dlv dap" for this one adapter seems overkill, whether it's "not too hard" or otherwise :/

To provide some background, my goal is to have a DAP client that is completely server agnostic, and you can plug any DAP-compliant server in by just launching the adapter and communicating with it over stdio or a socket*. All facilities of such debugger UI are provided by DAP requests that are documented in the protocol. I'm not the only DAP client author with little free time and no desire to write complex server-specific untestable code. So far, this has been successful (after a fashion) but there is an endless battle to ensure that adapters don't assume they are being run in VScode or rely on vscode specifics. Alas, I don't expect to win this battle, but I'll keep fighting so long as I have the energy!

* FWIW we tried to do the same for LSP, but that's a forlorn hope.

[puremourning]

Consider any of the arguments of RunInTerminal, where do you think env or cwd comes from?

Yes, from the user's debug configuration (or defaulted). But hat's the case for every debug adapter. I mean users are already accustomed to performing this odious task (telling the debugger how to launch the program to be debugged). Why should they do that differently for delve ?

To be clear, the client does not parse the 'env'/'args'/'program'/'cwd' launch arguments. The launch arguments for an adapter are opaque in DAP.

[aarzilli]

I mean users are already accustomed to performing this odious task (telling the debugger how to launch the program to be debugged). Why should they do that differently for delve ?

I'm not suggesting they should. I'm saying that things would be a lot simpler for us, and not any more complicated for you, if part of the launch configuration was interpreted by the client.

[aarzilli]

I'm saying that things would be a lot simpler for us, and not any more complicated for you, if part of the launch configuration was interpreted by the client.

PS. and this should have been part of the spec, instead of RunInTerminal.

[hyangah]

@puremourning Is it too server-specific for vimspector to launch the dlv dap process from an integrated terminal by default? dlv headless server (so dlv dap) places the debuggee foreground and gives access to tty. I think it would be much simpler if vscode-go took the route.

We didn't implement in that way just because 1) we want to know exactly when dlv dap server is ready to listen on the port - currently done by watching the log stream, 2) we hoped to utilize the same UX vscode provides through RunInTerminal, and 3) we hoped to utilize it for debugging-as-a-root but we are reluctant to start dlv dap as a root. :-P I guess 1 and 3 aren't too critical, and vimspector is already in position of handling 2.

[puremourning]

@hyangah it's server specific in the sense that only delve needs to be started this way. I have a PR open for vimspector to essentially do exactly that, but it's messy and requires a bunch of work for similar reasons as you have mentioned (in particular the "re-use what you would have done if the server had sent a runInTerminal request" is not so straightforward). But yes, you're right, I can implement stuff in the client to make it "look like" delve is doing what all the other adapters do. The reason I commented here was that being unique means it's likely to be a new and excruciating source of bugs and barriers to future development. I'm one guy in extremely limited free time, so any such barriers are difficult for me to absorb.

@aarzilli I'm not arguing that the spec is perfect, but it is the spec we all work to. All APIs are imperfect, but by forking them and creating more specs by de facto, the problem becomes untenable for one-person-in-their-limited-free-time oss projects.