OpenHands icon indicating copy to clipboard operation
OpenHands copied to clipboard

feat(sandbox): Support sshd-based stateful docker session

Open xingyaoww opened this issue 1 year ago • 3 comments

I tried to implement the ssh-based stateful docker sandbox solution @frankxu2004 proposed. Turns out it is not hard to get working!

I haven't implemented root login yet, but I think opendevin user login works quite well.

See screenshot: (for some reason, the default logger seems to repeating the same output twice - can try to fix in a separate PR) image

xingyaoww avatar Apr 07 '24 06:04 xingyaoww

Fix the logging issue in https://github.com/OpenDevin/OpenDevin/pull/850#issue-2229602109.

Here's the fixed version:

image

xingyaoww avatar Apr 07 '24 07:04 xingyaoww

Thanks for the review @neubig ! That is a very good point! I've added a random user password and set -o 'ListenAddress=127.0.0.1' to prevent users outside localhost from connecting to the sandbox.

xingyaoww avatar Apr 07 '24 08:04 xingyaoww

This is great @xingyaoww so fast for implementing this! Thank you!

I think the pexpect module can also handle things like prompting to input password, as well as those Y/N prompts in the middle of a process.

Now it's using ssh.prompt() to look for shell prompts like # or $ to determine if the previous command is finished as for when to read the output and send the input. For processes asking password like "Password:" or apt-get "Proceed? y/n" do we need to add a bit more regex to the default prompt https://pexpect.readthedocs.io/en/stable/api/pexpect.html#pexpect.spawn.expect in order to handle those "wait-for-input", or does the current implementation works out of the box for "y/n" and "password:". I am thinking if ssh.expect("Password:") or ssh.expect("y/n") is needed?

Not a merge-blocking comment

frankxu2004 avatar Apr 07 '24 17:04 frankxu2004

Great point @frankxu2004 ! Let's start an issue tracking this and we can start a separate PR to get this implemented!

xingyaoww avatar Apr 08 '24 04:04 xingyaoww