Changing the VM user's shell to fish causes lima to wait forever for the SSH requirement to be satisfied

[BurnerWah]

Description

When setting the lima VM user's shell to fish, lima will get stuck forever waiting for the the SSH requirement to be satisfied.

lima-start.log
The loop is in the following lines:

time="2024-11-06T13:14:17-06:00" level=info msg="[hostagent] Waiting for the essential requirement 1 of 2: \"ssh\""
time="2024-11-06T13:14:17-06:00" level=debug msg="[hostagent] executing script \"ssh\""
time="2024-11-06T13:14:17-06:00" level=debug msg="[hostagent] executing ssh for script \"ssh\": /usr/bin/ssh [ssh -F /dev/null -o IdentityFile=\"/Users/burner/.config/lima/_config/user\" -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o NoHostAuthenticationForLocalhost=yes -o GSSAPIAuthentication=no -o PreferredAuthentications=publickey -o Compression=no -o BatchMode=yes -o IdentitiesOnly=yes -o Ciphers=\"^[email protected],[email protected]\" -o User=burner -o ControlMaster=auto -o ControlPath=\"/Users/burner/.config/lima/alpine/ssh.sock\" -o ControlPersist=yes -p 49540 127.0.0.1 -- /bin/bash -c $'while read -r line; do [ -n \"$line\" ] && export \"$line\"; done<<EOF\\n$(sudo cat /mnt/lima-cidata/param.env)\\nEOF\\n/bin/bash']"
time="2024-11-06T13:14:17-06:00" level=debug msg="[hostagent] stdout=\"\", stderr=\"fish: Expected a variable name after this $.\\n/bin/bash -c $'while read -r line; do [ -n \\\"$line\\\" ] && export \\\"$line\\\"; done<<EOF\\\\n$(sudo cat /mnt/lima-cidata/param.env)\\\\nEOF\\\\n/bin/bash'\\n             ^\\n\", err=failed to execute script \"ssh\": stdout=\"\", stderr=\"fish: Expected a variable name after this $.\\n/bin/bash -c $'while read -r line; do [ -n \\\"$line\\\" ] && export \\\"$line\\\"; done<<EOF\\\\n$(sudo cat /mnt/lima-cidata/param.env)\\\\nEOF\\\\n/bin/bash'\\n             ^\\n\": exit status 127"

Lima is running a script that was presumably meant for bash or another standard shell, but it runs it in whatever the user's configured shell is. If the user's shell is set to fish, this causes syntax a syntax error.

Reproduced on alpine & fedora VMs. I can provide any extra logs or make some dedicated VMs specifically for testing this if needed.

OS: Mac OS 14.7.1
CPU: Apple M1 Pro
Lima version: 1.0.0 (via homebrew)

[AkihiroSuda]

Is this a regression in v1.0?
Was it working in the previous release?

[afbjorklund]

It seems like fish doesn't accept the $'' form of shell quoting:

// ssh.ExecuteScript will strip the `#!` prefix from the first line and invoke the rest
// of the line as the command. The full script is then passed via STDIN. We use the $' '
// form of shell quoting to be able to use \n as newline escapes to fit everything on a
// single line:
//
//      #!/bin/bash -c $'while … done<<EOF\n$(sudo …)\nEOF\n/usr/bin/env ruby'
//      #!/usr/bin/env ruby

$ fish
Welcome to fish, the friendly interactive shell
Type `help` for instructions on how to use fish
anders@ubuntu ~> /bin/bash -c $'while … done<<EOF\n$(sudo …)\nEOF\n/usr/bin/env ruby'
fish: Expected a variable name after this $.
/bin/bash -c $'while … done<<EOF\n$(sudo …)\nEOF\n/usr/bin/env ruby'

commit 2fd8ad7

[afbjorklund]

https://fishshell.com/docs/current/design.html#the-law-of-orthogonality

The many Posix quoting styles are silly, especially $.

One workaround would be to printf to a temporary file, and execute it?

With all the annoyance of having to mktemp before and rm afterwards.

[jandubois]

Is this a regression in v1.0?

Looks like it. According to what @afbjorklund found, this was changed in #2570. There are 20 lines of comments on why the code is this way.

I guess writing to a temp file is an option, but where would we put it, in /tmp?

[afbjorklund]

I just used mktemp, which uses $TMPDIR (that should default to /tmp on most...)

like in 27603c1 for the guest-install

You can probably do it without three separate calls to ssh, though (didn't bother)

[jandubois]

Is mktemp guaranteed to exist and accept these parameters? It doesn't seem to be in POSIX.

Should you fall back to /tmp if mktemp returns an error?

[afbjorklund]

I don't think there are any absolute guarantees. Could it be quoted in some other way?
It does seem that the current implementation has been done after some consideration...

The fish workarounds reminds me of the horrible things you had to do for tcsh
Like trying to get shell completion to work, or stream redirection in general. Harmful.

[afbjorklund]

According to the fish manual, you are supposed to just drop the quotes.

https://fishshell.com/docs/current/fish_for_bash_users.html#quoting

But that would require some kind of lima config, to change the syntax.

EDIT: And of course it didn't work either.

fish: Expected a string, but instead found a redirection
/bin/bash -c while … done<<EOF\n$(sudo …)\nEOF\n/usr/bin/env ruby

So it probably needs to take the long route.

[afbjorklund]

Maybe it (the support for param.env) could be added to sshocker ssh, like support for sudo?

https://github.com/lima-vm/sshocker/blob/master/pkg/ssh/ssh.go#L88

• Need feature for running ExecuteScript as the root user sshocker#87

Declaring fish as unsupported is another option... "it hurts when I do this"

[BurnerWah]

Is mktemp guaranteed to exist and accept these parameters? It doesn't seem to be in POSIX.

Should you fall back to /tmp if mktemp returns an error?

While not part of posix to my knowledge, mktemp is present in coreutils, uutils, and busybox, so I would expect it to be installed on basically every system.
Falling back to placing scripts in /tmp is probably fine too, though.

Having used fish as my default shell for years I don't really see much reason to adapt the scripts specifically so that they work within fish if it's an option to just use a temporary file.
There would be issues beyond just the string quoting to solve, and the extra code seems like it wouldn't be worth the effort.
For instance, fish has a pretty different syntax for while loops which would cause the while ...; do; ...; done loops to fail from syntax errors, as fish uses the end builtin where bash uses the done builtin.
It'll also complain about missing commands, because fish lacks a do builtin entirely.
Fish also doesn't support the << syntax for redirection.
And while it's less likely to matter, fish's read builtin doesn't have a -r flag.

All of those quirks are probably things that can be dealt with, but temporary files seem like a much more reliable solution, and it also improves compatibility with any other non-posix login shell (ex. nushell, xonsh, elvish, or powershell, though I've only ever heard of one person using any of those as a login shell)

[jandubois]

There would be issues beyond just the string quoting to solve, and the extra code seems like it wouldn't be worth the effort.
For instance, fish has a pretty different syntax for while loops which would cause the while ...; do; ...; done loops to fail from syntax errors, as fish uses the end builtin where bash uses the done builtin.

The while loop is executed by /bin/bash. It is just the quoting of the string that is an issue because it needs to be parsed by the default shell.

I wonder if we can replace /bin/bash -c $'…' with /bin/bash -c "$(printf "…")" (after doubling all % characters in …). Then we depend on printf; I wonder if that will do the right thing on all platforms.

[BurnerWah]

There would be issues beyond just the string quoting to solve, and the extra code seems like it wouldn't be worth the effort.
For instance, fish has a pretty different syntax for while loops which would cause the while ...; do; ...; done loops to fail from syntax errors, as fish uses the end builtin where bash uses the done builtin.

The while loop is executed by /bin/bash. It is just the quoting of the string that is an issue because it needs to be parsed by the default shell.

I wonder if we can replace /bin/bash -c $'…' with /bin/bash -c "$(printf "…")" (after doubling all % characters in …). Then we depend on printf; I wonder if that will do the right thing on all platforms.

printf is also in coreutils, uutils, and busybox, so it seems like it'd be a safe thing to depend on. And after trying bash -c "$(printf "...")" with the command where lima got stuck, it does actually seem like it'd work too. It probably requires some testing on other commands though.