diff --git a/internal/container/start.go b/internal/container/start.go index 9c044176..71237f91 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -81,7 +81,14 @@ func Start(ctx context.Context, rt runtime.Runtime, sink output.Sink, opts Start tel := opts.Telemetry - hostEnv := filterHostEnv(os.Environ()) + hostEnv, droppedEnv := filterHostEnv(os.Environ()) + for _, d := range droppedEnv { + text := fmt.Sprintf("Not forwarding %s to the emulator: it would override %s inside the emulator", d.name, d.overrides) + if d.overrides == "" { + text = fmt.Sprintf("Not forwarding %s to the emulator: its value contains line breaks", d.name) + } + sink.Emit(output.MessageEvent{Severity: output.SeverityWarning, Text: text}) + } agentEnvVars := agentEnv(caller.New().Classify()) containers := make([]runtime.ContainerConfig, len(opts.Containers)) @@ -863,19 +870,64 @@ func awaitStartup(ctx context.Context, rt runtime.Runtime, sink output.Sink, con } } +// droppedHostEnv is a host variable filterHostEnv refused to forward, so the +// caller can warn the user. overrides names the critical variable the +// entrypoint-stripped name would clobber inside the emulator; it is empty when +// the entry was dropped for carrying a multi-line value instead. +type droppedHostEnv struct { + name string + overrides string +} + // filterHostEnv returns the subset of host environment entries that should be // forwarded to the emulator container. It keeps CI and LOCALSTACK_* variables -// but explicitly drops LOCALSTACK_AUTH_TOKEN so the host value cannot overwrite -// the token resolved by lstk (which may come from the keyring). -func filterHostEnv(envList []string) []string { - var out []string +// but drops entries that would corrupt the emulator environment: +// - LOCALSTACK_AUTH_TOKEN, so the host value cannot overwrite the token +// resolved by lstk (which may come from the keyring); this drop is silent +// because lstk forwards its own resolved token instead; +// - values containing a newline or carriage return: the entrypoint re-exports +// variables through a line-oriented `env | sed` pipeline, so an embedded +// line like "LOCALSTACK_PATH=" would inject a rogue export that blanks PATH; +// - variables whose prefix-stripped name would clobber a critical variable +// inside the emulator (see criticalContainerVar). +// +// The latter two are returned in dropped so callers can warn the user. +func filterHostEnv(envList []string) (out []string, dropped []droppedHostEnv) { for _, e := range envList { - if strings.HasPrefix(e, "CI=") || - (strings.HasPrefix(e, "LOCALSTACK_") && !strings.HasPrefix(e, "LOCALSTACK_AUTH_TOKEN=")) { - out = append(out, e) + key, value, ok := strings.Cut(e, "=") + if !ok || (key != "CI" && !strings.HasPrefix(key, "LOCALSTACK_")) { + continue + } + if key == "LOCALSTACK_AUTH_TOKEN" { + continue } + if strings.ContainsAny(value, "\n\r") { + dropped = append(dropped, droppedHostEnv{name: key}) + continue + } + if name := strings.TrimPrefix(key, "LOCALSTACK_"); name != key && criticalContainerVar(name) { + dropped = append(dropped, droppedHostEnv{name: key, overrides: name}) + continue + } + out = append(out, e) } - return out + return out, dropped +} + +// criticalContainerVar reports whether a LOCALSTACK_* variable stripped to name +// would break the emulator: the image's docker-entrypoint.sh strips the +// LOCALSTACK_ prefix and re-exports the remainder (only LOCALSTACK_HOST and +// LOCALSTACK_HOSTNAME are excluded), so e.g. a host LOCALSTACK_PATH becomes +// PATH inside the emulator and startup dies with "mkdir: command not found" +// (DEVX-984). IFS is included because the entrypoint sources the stripped +// exports mid-script, corrupting its own word splitting. +func criticalContainerVar(name string) bool { + switch name { + case "PATH", "HOME", "SHELL", "IFS", "ENV", "BASH_ENV", + "LD_PRELOAD", "LD_LIBRARY_PATH", "PYTHONPATH", "PYTHONHOME": + return true + } + return false } func envHasKey(env []string, key string) bool { diff --git a/internal/container/start_test.go b/internal/container/start_test.go index c862014d..315768c3 100644 --- a/internal/container/start_test.go +++ b/internal/container/start_test.go @@ -390,12 +390,20 @@ func TestFilterHostEnv(t *testing.T) { "LOCALSTACK_API_ENDPOINT=https://example.test", "LOCALSTACK_AUTH_TOKEN=host-token", "LOCALSTACK_PERSISTENCE=1", + "LOCALSTACK_PATH=/home/user/repos/localstack", + "LOCALSTACK_HOME=/root", + "LOCALSTACK_PYTHONPATH=/opt/code", + "LOCALSTACK_LD_PRELOAD=/lib/evil.so", + "LOCALSTACK_IFS=:", + "LOCALSTACK_MULTILINE=a\nLOCALSTACK_PATH=", + "LOCALSTACK_PATHFINDER=1", + "LOCALSTACK_HOSTNAME=custom.host", "PATH=/usr/bin", "HOME=/home/user", "CI_PIPELINE=foo", } - got := filterHostEnv(input) + got, dropped := filterHostEnv(input) assert.Contains(t, got, "CI=true") assert.Contains(t, got, "LOCALSTACK_DISABLE_EVENTS=1") @@ -406,6 +414,27 @@ func TestFilterHostEnv(t *testing.T) { assert.NotContains(t, got, "PATH=/usr/bin") assert.NotContains(t, got, "HOME=/home/user") assert.NotContains(t, got, "CI_PIPELINE=foo", "only exact CI= must be forwarded, not CI_*") + assert.NotContains(t, got, "LOCALSTACK_PATH=/home/user/repos/localstack", + "the emulator entrypoint strips the LOCALSTACK_ prefix, so forwarding this would override PATH inside the emulator and break startup (DEVX-984)") + assert.NotContains(t, got, "LOCALSTACK_HOME=/root") + assert.NotContains(t, got, "LOCALSTACK_PYTHONPATH=/opt/code") + assert.NotContains(t, got, "LOCALSTACK_LD_PRELOAD=/lib/evil.so") + assert.NotContains(t, got, "LOCALSTACK_IFS=:", + "the entrypoint sources the stripped exports mid-script, so IFS would corrupt its word splitting") + assert.NotContains(t, got, "LOCALSTACK_MULTILINE=a\nLOCALSTACK_PATH=", + "a multi-line value is split by the entrypoint's line-oriented env pipeline and can inject rogue exports like a blank PATH") + assert.Contains(t, got, "LOCALSTACK_PATHFINDER=1", "only exact critical names are blocked after prefix stripping, not name prefixes") + assert.Contains(t, got, "LOCALSTACK_HOSTNAME=custom.host", + "the entrypoint excludes LOCALSTACK_HOSTNAME from prefix stripping, so it stays forwardable") + assert.Equal(t, []droppedHostEnv{ + {name: "LOCALSTACK_PATH", overrides: "PATH"}, + {name: "LOCALSTACK_HOME", overrides: "HOME"}, + {name: "LOCALSTACK_PYTHONPATH", overrides: "PYTHONPATH"}, + {name: "LOCALSTACK_LD_PRELOAD", overrides: "LD_PRELOAD"}, + {name: "LOCALSTACK_IFS", overrides: "IFS"}, + {name: "LOCALSTACK_MULTILINE"}, + }, dropped, + "dropped variables are reported with the reason so start can warn the user; the intentional LOCALSTACK_AUTH_TOKEN drop is not warned about") } func TestAgentEnv(t *testing.T) { diff --git a/test/integration/start_test.go b/test/integration/start_test.go index 4dbaf2c7..d282b180 100644 --- a/test/integration/start_test.go +++ b/test/integration/start_test.go @@ -661,12 +661,19 @@ func TestStartCommandPassesCIAndLocalStackEnvVars(t *testing.T) { defer mockServer.Close() ctx := testContext(t) - _, stderr, err := runLstk(t, ctx, "", env.With(env.APIEndpoint, mockServer.URL). + // LOCALSTACK_PATH must not reach the emulator: its entrypoint strips the + // LOCALSTACK_ prefix and re-exports the rest, so a forwarded LOCALSTACK_PATH + // overrides PATH inside the emulator and startup dies with + // "mkdir: command not found" (DEVX-984). + stdout, stderr, err := runLstk(t, ctx, "", env.With(env.APIEndpoint, mockServer.URL). With(env.CI, "true"). - With(env.DisableEvents, "1"), + With(env.DisableEvents, "1"). + With("LOCALSTACK_PATH", "/home/user/repos/localstack"), "start") require.NoError(t, err, "lstk start failed: %s", stderr) requireExitCode(t, 0, err) + assert.Contains(t, stdout+stderr, "Not forwarding LOCALSTACK_PATH", + "dropping a critical-collision variable must be surfaced to the user") inspect, err := dockerClient.ContainerInspect(ctx, containerName, client.ContainerInspectOptions{}) require.NoError(t, err, "failed to inspect container") @@ -676,6 +683,7 @@ func TestStartCommandPassesCIAndLocalStackEnvVars(t *testing.T) { assert.Equal(t, "true", envVars["CI"]) assert.Equal(t, "1", envVars["LOCALSTACK_DISABLE_EVENTS"]) assert.NotEmpty(t, envVars["LOCALSTACK_AUTH_TOKEN"]) + assert.NotContains(t, envVars, "LOCALSTACK_PATH") } func TestStartCommandPersistFlagSetsPersistenceEnv(t *testing.T) {