diff --git a/internal/container/start.go b/internal/container/start.go index 9c044176..e81a5a88 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -863,15 +863,56 @@ func awaitStartup(ctx context.Context, rt runtime.Runtime, sink output.Sink, con } } +// entrypointReservedEnvKeys are variable names that must not be overwritten +// inside the emulator container. LocalStack's docker-entrypoint.sh re-exports +// every LOCALSTACK_ variable as (stripping the prefix) via a +// line-oriented `source <(env | ... | sed ...)`, and does not set PATH itself — +// it relies on the image's own PATH. So forwarding e.g. LOCALSTACK_PATH turns +// into `export PATH=...` inside the container, blanking/overwriting PATH; the +// next entrypoint command (`mkdir -p ...`) then fails with +// "mkdir: command not found" and the emulator exits on startup (DEVX-984). +var entrypointReservedEnvKeys = map[string]bool{ + "PATH": true, + "HOME": true, + "SHELL": true, + "IFS": true, + "ENV": true, + "BASH_ENV": true, + "LD_PRELOAD": true, + "LD_LIBRARY_PATH": true, + "PYTHONPATH": true, + "PYTHONHOME": true, +} + // 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). +// but drops entries that would corrupt the container environment: +// - LOCALSTACK_AUTH_TOKEN, so the host value cannot overwrite the token +// resolved by lstk (which may come from the keyring); +// - any value containing a newline or carriage return, since LocalStack's +// entrypoint re-exports variables through a line-oriented pipeline where a +// multi-line value can inject spurious `export` lines; and +// - LOCALSTACK_* variables whose entrypoint-stripped name is a reserved shell +// variable (see entrypointReservedEnvKeys), e.g. LOCALSTACK_PATH -> PATH. func filterHostEnv(envList []string) []string { var out []string for _, e := range envList { - if strings.HasPrefix(e, "CI=") || - (strings.HasPrefix(e, "LOCALSTACK_") && !strings.HasPrefix(e, "LOCALSTACK_AUTH_TOKEN=")) { + key, value, ok := strings.Cut(e, "=") + if !ok { + continue + } + // A multi-line value would be split across lines by the entrypoint's + // `env | ... | sed` pipeline, potentially injecting rogue exports. + if strings.ContainsAny(value, "\n\r") { + continue + } + switch { + case key == "CI": + out = append(out, e) + case strings.HasPrefix(key, "LOCALSTACK_") && key != "LOCALSTACK_AUTH_TOKEN": + if entrypointReservedEnvKeys[strings.TrimPrefix(key, "LOCALSTACK_")] { + continue + } out = append(out, e) } } diff --git a/internal/container/start_test.go b/internal/container/start_test.go index c862014d..9e3593c9 100644 --- a/internal/container/start_test.go +++ b/internal/container/start_test.go @@ -393,6 +393,9 @@ func TestFilterHostEnv(t *testing.T) { "PATH=/usr/bin", "HOME=/home/user", "CI_PIPELINE=foo", + "LOCALSTACK_PATH=/opt/custom", + "LOCALSTACK_LD_PRELOAD=/tmp/evil.so", + "LOCALSTACK_MULTILINE=a\nLOCALSTACK_PATH=", } got := filterHostEnv(input) @@ -406,6 +409,12 @@ 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=/opt/custom", + "LOCALSTACK_PATH must not be forwarded: the entrypoint strips it to PATH, blanking the container PATH (DEVX-984)") + assert.NotContains(t, got, "LOCALSTACK_LD_PRELOAD=/tmp/evil.so", + "LOCALSTACK_* vars stripping to reserved shell variables must not be forwarded") + assert.NotContains(t, got, "LOCALSTACK_MULTILINE=a\nLOCALSTACK_PATH=", + "values with embedded newlines must not be forwarded: they can inject rogue exports") } func TestAgentEnv(t *testing.T) { diff --git a/test/integration/env/env.go b/test/integration/env/env.go index 4c4467b5..03e78d40 100644 --- a/test/integration/env/env.go +++ b/test/integration/env/env.go @@ -11,6 +11,7 @@ type Key string const ( AuthToken Key = "LOCALSTACK_AUTH_TOKEN" LocalStackHost Key = "LOCALSTACK_HOST" + LocalStackPath Key = "LOCALSTACK_PATH" APIEndpoint Key = "LSTK_API_ENDPOINT" Keyring Key = "LSTK_KEYRING" CI Key = "CI" diff --git a/test/integration/start_test.go b/test/integration/start_test.go index 4dbaf2c7..2c48ca83 100644 --- a/test/integration/start_test.go +++ b/test/integration/start_test.go @@ -52,6 +52,39 @@ func TestStartCommandSucceedsWithValidToken(t *testing.T) { "persistence bullet must be omitted when --persist is not set") } +// DEVX-984: a host LOCALSTACK_PATH must not be forwarded into the emulator +// container. LocalStack's entrypoint re-exports every LOCALSTACK_ as +// and relies on the image's own PATH, so forwarding LOCALSTACK_PATH +// blanks the container PATH and the entrypoint crashes on its first external +// command with "mkdir: command not found". With the fix, lstk drops such +// reserved-name vars and the emulator starts normally. +func TestStartCommandIgnoresPathClobberingHostEnv(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + environ := env.With(env.APIEndpoint, mockServer.URL). + With(env.LocalStackPath, "/opt/lstk-should-be-ignored") + + ctx := testContext(t) + stdout, stderr, err := runLstk(t, ctx, "", environ, "start") + require.NoError(t, err, "lstk start failed: %s\n%s", stdout, stderr) + requireExitCode(t, 0, err) + + assert.NotContains(t, stdout, "mkdir: command not found", + "forwarding LOCALSTACK_PATH must not blank the container PATH (DEVX-984)") + assert.NotContains(t, stdout, "exited unexpectedly") + + inspect, err := dockerClient.ContainerInspect(ctx, containerName, client.ContainerInspectOptions{}) + require.NoError(t, err, "failed to inspect container") + assert.True(t, inspect.Container.State.Running, "container should be running") +} + // PRO-323: a pinned image already present locally must be reused, not re-pulled. // Tags the lightweight test image under a pinned localstack-pro tag so the image // is present locally; lstk should skip the pull and emit "Using local image".