From cffd09f946db5930d770cea34134558aa7fe2b94 Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Thu, 9 Jul 2026 16:55:49 +0300 Subject: [PATCH 1/2] fix: stop forwarding LOCALSTACK_* host vars that clobber critical emulator env (DEVX-984) The emulator image's docker-entrypoint.sh strips the LOCALSTACK_ prefix from environment variables and re-exports the remainder, so a host shell var like LOCALSTACK_PATH forwarded by filterHostEnv became PATH inside the emulator and killed startup with 'mkdir: command not found'. Drop LOCALSTACK_* vars whose stripped name collides with a critical runtime variable (PATH, HOME, LD_PRELOAD, LD_LIBRARY_PATH, PYTHONPATH, PYTHONHOME) and emit a warning so the drop is visible to the user. --- internal/container/start.go | 50 +++++++++++++++++++++++++++----- internal/container/start_test.go | 18 +++++++++++- test/integration/start_test.go | 12 ++++++-- 3 files changed, 69 insertions(+), 11 deletions(-) diff --git a/internal/container/start.go b/internal/container/start.go index 9c044176..2e311a5b 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -81,7 +81,13 @@ 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 _, name := range droppedEnv { + sink.Emit(output.MessageEvent{ + Severity: output.SeverityWarning, + Text: fmt.Sprintf("Not forwarding %s to the emulator: it would override %s inside the emulator", name, strings.TrimPrefix(name, "LOCALSTACK_")), + }) + } agentEnvVars := agentEnv(caller.New().Classify()) containers := make([]runtime.ContainerConfig, len(opts.Containers)) @@ -866,16 +872,44 @@ func awaitStartup(ctx context.Context, rt runtime.Runtime, sink output.Sink, con // 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 +// the token resolved by lstk (which may come from the keyring), and drops +// variables whose prefix-stripped name would clobber a critical variable +// inside the emulator (see stripsToCriticalVar). The names of the latter are +// returned in dropped so callers can warn the user; the LOCALSTACK_AUTH_TOKEN +// drop is silent because lstk forwards its own resolved token instead. +func filterHostEnv(envList []string) (out, dropped []string) { for _, e := range envList { - if strings.HasPrefix(e, "CI=") || - (strings.HasPrefix(e, "LOCALSTACK_") && !strings.HasPrefix(e, "LOCALSTACK_AUTH_TOKEN=")) { - out = append(out, e) + if !strings.HasPrefix(e, "CI=") && !strings.HasPrefix(e, "LOCALSTACK_") { + continue + } + if strings.HasPrefix(e, "LOCALSTACK_AUTH_TOKEN=") { + continue + } + if stripsToCriticalVar(e) { + name, _, _ := strings.Cut(e, "=") + dropped = append(dropped, name) + continue } + out = append(out, e) } - return out + return out, dropped +} + +// stripsToCriticalVar reports whether forwarding the LOCALSTACK_* entry would +// break the emulator: the image's docker-entrypoint.sh strips the LOCALSTACK_ +// prefix and re-exports the remainder (only LOCALSTACK_HOST/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). +func stripsToCriticalVar(entry string) bool { + name, _, found := strings.Cut(strings.TrimPrefix(entry, "LOCALSTACK_"), "=") + if !found { + return false + } + switch name { + case "PATH", "HOME", "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..d24dccda 100644 --- a/internal/container/start_test.go +++ b/internal/container/start_test.go @@ -390,12 +390,18 @@ 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_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 +412,16 @@ 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.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, []string{"LOCALSTACK_PATH", "LOCALSTACK_HOME", "LOCALSTACK_PYTHONPATH", "LOCALSTACK_LD_PRELOAD"}, dropped, + "dropped variable names are reported 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) { From 695098d6036d5d2ae76bd113c035bc734ad64eda Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Thu, 9 Jul 2026 18:01:57 +0300 Subject: [PATCH 2/2] fix: drop multiline LOCALSTACK_* values and widen the critical env blocklist (DEVX-984) Folds in the two improvements from #379: a multi-line value is split by the entrypoint's line-oriented env pipeline and can inject rogue exports (e.g. an embedded 'LOCALSTACK_PATH=' line blanks PATH), and IFS/SHELL/ ENV/BASH_ENV join the reserved set (the entrypoint sources the stripped exports mid-script, so IFS corrupts its word splitting). Unlike #379, both drops surface a warning instead of happening silently. Co-Authored-By: Claude --- internal/container/start.go | 74 ++++++++++++++++++++------------ internal/container/start_test.go | 17 +++++++- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/internal/container/start.go b/internal/container/start.go index 2e311a5b..71237f91 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -82,11 +82,12 @@ func Start(ctx context.Context, rt runtime.Runtime, sink output.Sink, opts Start tel := opts.Telemetry hostEnv, droppedEnv := filterHostEnv(os.Environ()) - for _, name := range droppedEnv { - sink.Emit(output.MessageEvent{ - Severity: output.SeverityWarning, - Text: fmt.Sprintf("Not forwarding %s to the emulator: it would override %s inside the emulator", name, strings.TrimPrefix(name, "LOCALSTACK_")), - }) + 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()) @@ -869,25 +870,43 @@ 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), and drops -// variables whose prefix-stripped name would clobber a critical variable -// inside the emulator (see stripsToCriticalVar). The names of the latter are -// returned in dropped so callers can warn the user; the LOCALSTACK_AUTH_TOKEN -// drop is silent because lstk forwards its own resolved token instead. -func filterHostEnv(envList []string) (out, dropped []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_") { + key, value, ok := strings.Cut(e, "=") + if !ok || (key != "CI" && !strings.HasPrefix(key, "LOCALSTACK_")) { continue } - if strings.HasPrefix(e, "LOCALSTACK_AUTH_TOKEN=") { + if key == "LOCALSTACK_AUTH_TOKEN" { continue } - if stripsToCriticalVar(e) { - name, _, _ := strings.Cut(e, "=") - dropped = append(dropped, name) + 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) @@ -895,18 +914,17 @@ func filterHostEnv(envList []string) (out, dropped []string) { return out, dropped } -// stripsToCriticalVar reports whether forwarding the LOCALSTACK_* entry would -// break the emulator: the image's docker-entrypoint.sh strips the LOCALSTACK_ -// prefix and re-exports the remainder (only LOCALSTACK_HOST/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). -func stripsToCriticalVar(entry string) bool { - name, _, found := strings.Cut(strings.TrimPrefix(entry, "LOCALSTACK_"), "=") - if !found { - return false - } +// 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", "LD_PRELOAD", "LD_LIBRARY_PATH", "PYTHONPATH", "PYTHONHOME": + case "PATH", "HOME", "SHELL", "IFS", "ENV", "BASH_ENV", + "LD_PRELOAD", "LD_LIBRARY_PATH", "PYTHONPATH", "PYTHONHOME": return true } return false diff --git a/internal/container/start_test.go b/internal/container/start_test.go index d24dccda..315768c3 100644 --- a/internal/container/start_test.go +++ b/internal/container/start_test.go @@ -394,6 +394,8 @@ func TestFilterHostEnv(t *testing.T) { "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", @@ -417,11 +419,22 @@ func TestFilterHostEnv(t *testing.T) { 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, []string{"LOCALSTACK_PATH", "LOCALSTACK_HOME", "LOCALSTACK_PYTHONPATH", "LOCALSTACK_LD_PRELOAD"}, dropped, - "dropped variable names are reported so start can warn the user; the intentional LOCALSTACK_AUTH_TOKEN drop is not warned about") + 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) {