add preserve-env variable to doc and also add an integration test

[olamilekan000]

change adds LIMA_SHELLENV_ALLOW and LIMA_SHELLENV_BLOCK env
to the documentation and also include an integration test for the --preserve-env flag.

fixes issue

[jandubois]

Not really a review, just 2 observations that need discussion IMO.

[jandubois]

I'm not convinced that this is the right thing to do. Why would you not want to import dev paths into the VM by default?

Where do we draw the line? Do we block RUBY_*? How about JAVA_OPTS? If you want to block most of the variables, you should just set LIMA_SHELLENV_ALLOW and have everything else blocked by default.

[olamilekan000]

I kind of feel it makes sense blocking PATH (and similar variables) by default inorder to avoid unexpected behaviour inside the VM. If the host injects its paths, the VM might end up pointing to binaries or libraries that don’t exist there, which could cause confusing errors.

[jandubois]

I don't know, --preserve-env variables may still be overwritten in ~/.profile inside the guest:

❯ lima sh -c 'tail -1 ~/.profile'
export GOPATH=/home/jan.linux/go

❯ echo $GOPATH
/Users/jan/go

❯ limactl shell --preserve-env default bash -c "printenv GOPATH"
/home/jan.linux/go

❯ limactl shell default bash -c "GOPATH=$GOPATH printenv GOPATH"
/Users/jan/go

So this is maybe not how people expect --preserve-env to work, but by passing the variables to the shell process, which will then run ~/.profile, the actual value may still be different from the host. You have to overwrite them inside the shell script to be certain they have the host value.

Assuming we stick with the current functionality, I think there is no reason for aggressive filtering of *PATH beyond just PATH.

If we want to change how --preserve-env is implemented, and the host settings would take precedence over the ~/.profile ones, then additional filtering would be warranted. But I feel in that case you probably want to stick with setting LIMA_SHELLENV_ALLOW instead.

I think the current PR should move ahead with the current implementation and *PATH removed, so at least @AkihiroSuda's comments on the closed PR are addressed, and then we can further update the feature based on the result of the discussion thread in a separate PR.

[jandubois]

So this is maybe not how people expect --preserve-env to work, but by passing the variables to the shell process, which will then run ~/.profile, the actual value may still be different from the host. You have to overwrite them inside the shell script to be certain they have the host value.

After subconscious reflection (sleep) I think the current implementation is correct given that limactl shell is running commands in a login shell.

It matches the behaviour of sudo when running a login shell:

$ RBENV_ROOT=foo sudo --preserve-env bash --login -c 'printenv RBENV_ROOT'
/opt/homebrew/var/rbenv

Therefore I think adding the *PATH pattern to the default blocklist is not helpful.

---

We have an ancient (from 2021) request about limactl shell not actually evaluating arguments as a shell inside the VM (so you can pass through wildcards and variable names without them being evaluated), and I suggested to use limactl exec for that functionality.

If we ever implement this alternative, then host environment variables should not be shadowed by ~/.profile settings inside the VM, but we can think about that if we ever implement it. Nobody seems to have complained about it missing in the last few years though...

[jandubois]

We have an ancient (from 2021) request about limactl shell not actually evaluating arguments as a shell inside the VM (so you can pass through wildcards and variable names without them being evaluated)

I have just tested this, and it seems like limactl shell has changed quoting and is passing things through correctly now, so I will close the ancient issue.

$ limactl shell default echo '$HOME'
$HOME

Nobody seems to have complained about it missing in the last few years though...

I guess now I know why...

[olamilekan000]

When I tried the sample command you shared I got something a little different. The env value was actually preserved.

RBENV_ROOT=foo sudo --preserve-env bash --login -c 'printenv RBENV_ROOT'

foo

I also did try it with the limactl shell command and it was consistent with the result I got locally. Correct me if i'm wrong, what I understand from your earlier point is that since the shell command makes use of the login shell, the guest's value would actually take precedence but that's not the case in this scenario regarding PATH as well.

I have some samples below:
This is the value of the PATH env on my local system.

/Users/username/.opam/default/bin:/Users/username/.ops/bin:/Users/username/.gvm/bin:/Users/username/Library/Application:Support/cloud-code/installer/google-cloud-sdk/bin:/Library/Frameworks/Python.framework/Versions/3.11/bin:/usr/lib/google-cloud-sdk/bin:/opt/homebrew/opt/make/libexec/gnubin:/usr/local/sbin:/opt/homebrew/bin:/opt/homebrew/sbin:/Library/Frameworks/Python.framework/Versions/3.10/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Applications/VMware:Fusion:Tech:Preview.app/Contents/Public:/usr/local/go/bin:/Users/username/.ops/bin:/Users/username/.opam/default/bin:/Users/username/.cargo/bin:/Applications/Visual:Studio:Code.app/Contents/Resources/app/bin:/Applications/Visual Studio Code.app/Contents/Resources/app/bin

This is what the value looks like in the VM

limactl shell default bash 
username@lima-default:/Users/username/works/opensource/lima$ echo $PATH
/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/usr/sbin:/sbin:/usr/sbin:/sbin

and then when i use the --preserve-env in the shell command, I get this

LIMA_SHELLENV_ALLOW=PATH limactl shell `--preserve-env` default bash -c 'echo "login shell PATH: $PATH"'

login shell PATH: /Users/username/.opam/default/bin:/Users/username/.ops/bin:/Users/username/.gvm/bin:/Users/username/Library/Application:Support/cloud-code/installer/google-cloud-sdk/bin:/Library/Frameworks/Python.framework/Versions/3.11/bin:/usr/lib/google-cloud-sdk/bin:/opt/homebrew/opt/make/libexec/gnubin:/usr/local/sbin:/opt/homebrew/bin:/opt/homebrew/sbin:/Library/Frameworks/Python.framework/Versions/3.10/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Applications/VMware:Fusion:Tech:Preview.app/Contents/Public:/usr/local/go/bin:/Users/username/.ops/bin:/Users/username/.opam/default/bin:/Users/username/.cargo/bin:/Applications/Visual:Studio:Code.app/Contents/Resources/app/bin:/Applications/Visual Studio Code.app/Contents/Resources/app/bin

which is same as the PATH in the host machine. Based on the login shell expectation, I don't think the PATH value should be the same as the value on the host machine, hence it shouldn't get overwritten even when it is added to the LIMA_SHELLENV_ALLOW list. Is this right?

[jandubois]

When I tried the sample command you shared I got something a little different. The env value was actually preserved.

RBENV_ROOT=foo sudo --preserve-env bash --login -c 'printenv RBENV_ROOT'

foo

This happens because you don't set RBENV_ROOT in your ~/.profile that will overwrite your value:

❯ grep RBENV ~/.bash_profile
export RBENV_ROOT=$BREW_PREFIX/var/rbenv

I also did try it with the limactl shell command and it was consistent with the result I got locally. Correct me if i'm wrong, what I understand from your earlier point is that since the shell command makes use of the login shell, the guest's value would actually take precedence but that's not the case in this scenario regarding PATH as well.

[...]

which is same as the PATH in the host machine. Based on the login shell expectation, I don't think the PATH value should be the same as the value on the host machine, hence it shouldn't get overwritten even when it is added to the LIMA_SHELLENV_ALLOW list. Is this right?

What shell are you running, and why do you have backticks around `--preserve-env`? That tries to run --preserve-env as a command, so I'm curious how this worked for you.

I'm also surprised that you get exactly the same PATH as on the host. Are you using the template://default template for your instance?

When I run your command, I get back the PATH from the host, with a :/snap/bin:/usr/sbin:/sbin suffix.

This happens because PATH is not overwritten in the shell profile scripts in the VM, but appended to:

jan@lima-default:~$ cat /etc/profile.d/apps-bin-path.sh
# shellcheck shell=sh

# Expand $PATH to include the directory where snappy applications go.
snap_bin_path="/snap/bin"
if [ -n "${PATH##*${snap_bin_path}}" ] && [ -n "${PATH##*${snap_bin_path}:*}" ]; then
    export PATH="$PATH:${snap_bin_path}"
fi
...

jan@lima-default:~$ tail -5 ~/.profile
# Lima BEGIN
# Make sure iptables and mount.fuse3 are available
PATH="$PATH:/usr/sbin:/sbin"
export PATH
# Lima END

Which is why PATH needs to be on the default block list.

I have no idea why you don't get the additional directories appended; the only explanation I have is that you use a different template.

[olamilekan000]

What shell are you running, and why do you have backticks around --preserve-env? That tries to run --preserve-env as a command, so I'm curious how this worked for you.

I use zsh and the backtick was probably during formating of the command when trying to type in the comment section. This is what it looks like in my shell

LIMA_SHELLENV_ALLOW=PATH limactl shell --preserve-env default bash -c 'echo "login shell PATH: $PATH"' 

I'm also surprised that you get exactly the same PATH as on the host. Are you using the template://default template for your instance?

Yes, I am using the default template

 limactl ls                                                                                            
NAME       STATUS     SSH                VMTYPE    ARCH       CPUS    MEMORY    DISK      DIR
default    Running    127.0.0.1:60022    qemu      aarch64    4       4GiB      100GiB    ~/.lima/default

[olamilekan000]

This is what the PATH looks like in the guest when it is not included in the "Allowed List"

➜  limactl shell --preserve-env default bash -c 'echo "login shell PATH: $PATH"' 

login shell PATH: /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/usr/sbin:/sbin

[jandubois]

Sorry, in a rush right now, so didn't review the integration test yet, only the other files.

[jandubois]

We have an ancient (from 2021) request about limactl shell not actually evaluating arguments as a shell inside the VM (so you can pass through wildcards and variable names without them being evaluated)

I have just tested this, and it seems like limactl shell has changed quoting and is passing things through correctly now, so I will close the ancient issue.

$ limactl shell default echo '$HOME'
$HOME

Nobody seems to have complained about it missing in the last few years though...

I guess now I know why...

[jandubois]

Let's not touch this in this PR, but I'm wondering if TERM and TERMINFO really should be blocked. The VM shell will run in the same terminal after all.

[jandubois]

2 more comments on the integration test.

[jandubois]

Thanks, LGTM