implement preserving env from host into vm in shell command #3830

olamilekan000 · 2025-08-13T18:34:16Z

change implements propagating env from host machine into the guest VM.

implements issue

jandubois

Thanks, LGTM and seems to work.

I do have some feedback though...

(I also ran out of time and did not review the tests)

cmd/limactl/shell.go

jandubois · 2025-08-14T05:16:07Z

pkg/envutil/envutil.go

+	"XAUTHORITY",
+	"XDG_*",
+	"_*", // Variables starting with underscore are typically internal
+}


I would sort the builtin list alphabetically, so it is easier to see which variables are filtered.

And I think we should add more variables, but it is of course subject to discussion. Some suggestions:

LD_* DYLD_* SHLVL UID EUID GROUP GID HOSTNAME

Edit: I had LINES and COLUMNS in the list originally, but changed my mind and removed them.

Not sure about excluding shell-specific variables:

BASH* ZSH* FPATH ZDOTDIR

pkg/envutil/envutil.go

pkg/limainfo/limainfo.go

pkg/envutil/envutil.go

Copilot

Pull Request Overview

This PR implements environment variable propagation from the host machine to the guest VM in shell commands, addressing the ability to preserve selected environment variables when executing commands within Lima VMs.

Adds environment filtering utilities with configurable blocklist/allowlist support
Introduces --preserve-env flag to shell commands for controlled environment propagation
Updates nerdctl.lima wrapper script to use the new preserve-env functionality

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/envutil/envutil.go	New utility package for filtering environment variables with builtin blocklist and pattern matching
pkg/envutil/envutil_test.go	Comprehensive test coverage for environment filtering functionality
pkg/limainfo/limainfo.go	Adds shell environment blocklist to Lima info structure
cmd/limactl/shell.go	Implements `--preserve-env` flag and environment variable injection in shell commands
cmd/nerdctl.lima	Updates wrapper script to use `--preserve-env` flag

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

pkg/envutil/envutil_test.go

alexandear

Left a few suggestions.

pkg/envutil/envutil_test.go

pkg/envutil/envutil.go

pkg/envutil/envutil_test.go

jandubois

More feedback.

Meetings coming up, so again didn't review tests yet.

cmd/limactl/shell.go

pkg/envutil/envutil.go

jandubois

Thanks, LGTM

I only have a few nit-picks, that I don't care if you want to address them or not, but I do think the tests should cover that the allow list takes precedence over the block list.

pkg/envutil/envutil_test.go

jandubois · 2025-08-14T19:39:01Z

@lima-vm/maintainers I think this PR is almost ready to be merged.

Please add some feedback about the default block list! Do you agree with all the entries, or should some not be blocked (like the shell-specific ones)? Or are there setting that are missing?

We can still update the list after the PR is merged, but I think once 2.0.0 is released, any further changes would be breaking backwards compatibility.

jandubois

Thanks, LGTM

One thing I stumbled over when reviewing the latest test update was that I expected allowList to come before blockList in the arguments:

filterEnvironmentWithLists(env, blockList, allowList []string)

Both because the allow list takes precedence, and because it comes alphabetically first.

But maybe that is just a personal preference. Feel free to either change or ignore, I'm also fine with merging as-is.

I will leave the PR open for another day or so to give other maintainers a chance to comment on the DefaultBlockList.

Signed-off-by: olalekan odukoya <[email protected]>

jandubois

Thanks, LGTM

alexandear · 2025-08-15T11:38:34Z

pkg/envutil/envutil.go

+// GetDefaultBlockList returns a copy of the default block list.
+func GetDefaultBlockList() []string {
+	return slices.Clone(defaultBlockList)
+}


Could we move this function to the top, right after the var defaultBlockList?

Usually, unexported functions go below exported ones.

I agree that it makes sense to have this function close to the definition because it is a small exported function directly tied to the variable.

In general we don't follow a rules that unexported functions go to the bottom. Most of the code seems to follow the pattern of defining helper functions before the exported function that uses them.

pkg/envutil/envutil_test.go

AkihiroSuda · 2025-08-18T04:19:59Z

pkg/envutil/envutil.go

+	"ZDOTDIR",
+	"ZSH*",
+	"_*", // Variables starting with underscore are typically internal
+}


Was this list taken from somewhere else?
Where is the upstream of this list?

There is no upstream afaik, we just made it up. Which is why I asked for feedback on it. Did check it against my own setup on macOS, and the blocks make sense to me. I'm unsure if we are blocking too much with the BASH* and ZSH* blocks.

AkihiroSuda · 2025-08-18T04:21:05Z

cmd/nerdctl.lima

@@ -1,3 +1,3 @@
 #!/bin/sh
 set -eu
-exec lima nerdctl "$@"
+exec limactl shell --preserve-env default nerdctl "$@"


Needs a comment to explain the reason

AkihiroSuda · 2025-08-18T04:21:13Z

cmd/nerdctl.lima

What about docker.lima, etc. ?

It was an oversight. I only mentioned nerdctl.lima because it always runs in the VM, but didn't think about docker.lima because I always have a local client.

So yes, docker.lima needs it too. I don't know about the others, but maybe we should add it to all of them.

In some ways you could argue that --preserve-env should be the default unless the instance is --plain, but I don't think we want to make this change because it is not backwards compatible (and having it work differently based on instance type is confusing).

--preserve-env should be the default

Scary to leak the credentials by default

AkihiroSuda · 2025-08-18T04:21:52Z

cmd/limactl/shell.go

@@ -54,6 +55,7 @@ func newShellCommand() *cobra.Command {
 	shellCmd.Flags().String("shell", "", "Shell interpreter, e.g. /bin/bash")
 	shellCmd.Flags().String("workdir", "", "Working directory")
 	shellCmd.Flags().Bool("reconnect", false, "Reconnect to the SSH session")
+	shellCmd.Flags().Bool("preserve-env", false, "Propagate environment variables to the shell")


Needs an integration test and the document to clarify the block list

AkihiroSuda · 2025-08-18T04:22:34Z

pkg/envutil/envutil.go

+}
+
+func getBlockList() []string {
+	blockEnv := os.Getenv("LIMA_SHELLENV_BLOCK")


Please update https://lima-vm.io/docs/config/environment-variables/

AkihiroSuda · 2025-08-18T04:23:18Z

Late to the party, but please check the comments

olamilekan000 force-pushed the implement-preserving-env-to-shell branch 2 times, most recently from 810b261 to d9e9da9 Compare August 13, 2025 18:59

jandubois linked an issue Aug 13, 2025 that may be closed by this pull request

Add a --with-env flag to limactl shell #3430

Closed

jandubois reviewed Aug 14, 2025

View reviewed changes

olamilekan000 force-pushed the implement-preserving-env-to-shell branch 3 times, most recently from 98e8fb3 to 523b2bf Compare August 14, 2025 08:21

olamilekan000 requested a review from jandubois August 14, 2025 08:24

alexandear requested a review from Copilot August 14, 2025 09:05

Copilot AI reviewed Aug 14, 2025

View reviewed changes

pkg/envutil/envutil_test.go Show resolved Hide resolved

alexandear reviewed Aug 14, 2025

View reviewed changes

olamilekan000 force-pushed the implement-preserving-env-to-shell branch from 523b2bf to 53e21f5 Compare August 14, 2025 14:16

olamilekan000 requested a review from alexandear August 14, 2025 14:16

olamilekan000 force-pushed the implement-preserving-env-to-shell branch from 53e21f5 to dd731fa Compare August 14, 2025 14:19

jandubois requested changes Aug 14, 2025

View reviewed changes

olamilekan000 force-pushed the implement-preserving-env-to-shell branch from dd731fa to 35c0bb5 Compare August 14, 2025 17:18

olamilekan000 requested a review from jandubois August 14, 2025 17:20

jandubois requested changes Aug 14, 2025

View reviewed changes

pkg/envutil/envutil_test.go Outdated Show resolved Hide resolved

pkg/envutil/envutil_test.go Outdated Show resolved Hide resolved

pkg/envutil/envutil_test.go Outdated Show resolved Hide resolved

pkg/envutil/envutil_test.go Show resolved Hide resolved

olamilekan000 force-pushed the implement-preserving-env-to-shell branch from 35c0bb5 to 71e227b Compare August 14, 2025 21:10

jandubois previously approved these changes Aug 14, 2025

View reviewed changes

implement preserving env from host into vm in shell command

35d82ea

Signed-off-by: olalekan odukoya <[email protected]>

olamilekan000 dismissed jandubois’s stale review via 35d82ea August 14, 2025 23:28

olamilekan000 force-pushed the implement-preserving-env-to-shell branch from 71e227b to 35d82ea Compare August 14, 2025 23:28

olamilekan000 requested a review from jandubois August 14, 2025 23:29

jandubois approved these changes Aug 15, 2025

View reviewed changes

alexandear approved these changes Aug 15, 2025

View reviewed changes

jandubois merged commit c241c8a into lima-vm:master Aug 16, 2025
61 of 63 checks passed

jandubois added this to the v2.0.0 milestone Aug 16, 2025

AkihiroSuda added the enhancement New feature or request label Aug 18, 2025

AkihiroSuda added impact/changelog area/cli limactl CLI user experience labels Aug 18, 2025

AkihiroSuda reviewed Aug 18, 2025

View reviewed changes

AkihiroSuda mentioned this pull request Aug 18, 2025

limactl shell --preserve-env should have tests and docs #3851

Open

implement preserving env from host into vm in shell command #3830

implement preserving env from host into vm in shell command #3830

Conversation

olamilekan000 commented Aug 13, 2025

Uh oh!

jandubois left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jandubois Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

alexandear left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jandubois left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jandubois left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jandubois commented Aug 14, 2025

Uh oh!

jandubois left a comment

Choose a reason for hiding this comment

Uh oh!

jandubois left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jandubois left a comment •

edited

Loading

jandubois Aug 14, 2025 •

edited

Loading

alexandear left a comment •

edited

Loading