From 8662b290f93ae8545ec22dae200190edfdc89ea4 Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Tue, 30 Jun 2026 11:45:47 +0300 Subject: [PATCH] fix: exit non-zero when `setup aws` fails to write the profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing the LocalStack AWS profile is the whole purpose of `lstk setup aws` (and the deprecated `lstk config profile`), but awsconfig.Setup emitted a warning and returned nil when the write itself failed — so a read-only ~/.aws, a permission error, or a full disk left the profile unwritten while the process still exited 0, masking the failure from users, CI, and agents. This is the same "prints an error yet exits 0" class as DEVX-941, in a real command. Thread an `explicit` flag through Setup: the user-invoked commands now emit a structured ErrorEvent and return a SilentError on a write failure (exit 1), while the best-effort post-start convenience path (EnsureProfile during `lstk start`) keeps warning and continuing so a profile-write hiccup never aborts an already-running emulator. Add an integration test that makes ~/.aws read-only, confirms the prompt, and asserts a non-zero exit (fails before this change, passes after). --- internal/awsconfig/awsconfig.go | 36 +++++++++++++++----- internal/ui/run_awsconfig.go | 2 +- test/integration/awsconfig_test.go | 53 ++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 9 deletions(-) diff --git a/internal/awsconfig/awsconfig.go b/internal/awsconfig/awsconfig.go index f4679dbe..af1a7446 100644 --- a/internal/awsconfig/awsconfig.go +++ b/internal/awsconfig/awsconfig.go @@ -236,7 +236,7 @@ func EnsureProfile(ctx context.Context, sink output.Sink, interactive bool, reso return nil } if interactive && !configOK && !credsOK { - return Setup(ctx, sink, resolvedHost, status) + return Setup(ctx, sink, resolvedHost, status, false) } emitMissingProfileNote(sink) @@ -246,7 +246,13 @@ func EnsureProfile(ctx context.Context, sink output.Sink, interactive bool, reso // Setup checks for the localstack AWS profile and prompts to create or update it if needed. // resolvedHost must be a host:port string (e.g. "localhost.localstack.cloud:4566"). // status is passed in from EnsureProfile to avoid re-checking the profile status. -func Setup(ctx context.Context, sink output.Sink, resolvedHost string, status profileStatus) error { +// +// explicit is true for the user-invoked `lstk setup aws` / `lstk config profile` +// commands, where writing the profile is the command's whole purpose, so a write +// failure must surface a non-zero exit. It is false for the best-effort post-start +// convenience flow (EnsureProfile during `lstk start`), where a write failure must +// only warn and must not abort an already-running emulator. +func Setup(ctx context.Context, sink output.Sink, resolvedHost string, status profileStatus, explicit bool) error { if !status.anyNeeded() { sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "LocalStack AWS profile is already configured."}) return nil @@ -254,8 +260,7 @@ func Setup(ctx context.Context, sink output.Sink, resolvedHost string, status pr configPath, credsPath, err := awsPaths() if err != nil { - sink.Emit(output.MessageEvent{Severity: output.SeverityWarning, Text: fmt.Sprintf("could not determine AWS config paths: %v", err)}) - return nil + return reportSetupErr(sink, "could not determine AWS config paths", err, explicit) } responseCh := make(chan output.InputResponse, 1) @@ -276,14 +281,12 @@ func Setup(ctx context.Context, sink output.Sink, resolvedHost string, status pr } if status.configNeeded { if err := writeConfigProfile(configPath, resolvedHost); err != nil { - sink.Emit(output.MessageEvent{Severity: output.SeverityWarning, Text: fmt.Sprintf("could not update ~/.aws/config: %v", err)}) - return nil + return reportSetupErr(sink, "could not update ~/.aws/config", err, explicit) } } if status.credsNeeded { if err := writeCredsProfile(credsPath); err != nil { - sink.Emit(output.MessageEvent{Severity: output.SeverityWarning, Text: fmt.Sprintf("could not update ~/.aws/credentials: %v", err)}) - return nil + return reportSetupErr(sink, "could not update ~/.aws/credentials", err, explicit) } } if status.configNeeded && status.credsNeeded { @@ -299,3 +302,20 @@ func Setup(ctx context.Context, sink output.Sink, resolvedHost string, status pr return nil } + +// reportSetupErr surfaces a profile-setup failure according to how setup was invoked. +// For the explicitly-invoked command it emits a structured error and returns a +// SilentError so the process exits non-zero; for the best-effort post-start flow it +// warns and returns nil so an already-running emulator's start is not aborted. +func reportSetupErr(sink output.Sink, msg string, err error, explicit bool) error { + if explicit { + sink.Emit(output.ErrorEvent{ + Title: "Could not set up the LocalStack AWS profile", + Summary: fmt.Sprintf("%s: %v", msg, err), + Actions: []output.ErrorAction{{Label: "Check the permissions of ~/.aws, then re-run:", Value: "lstk setup aws"}}, + }) + return output.NewSilentError(err) + } + sink.Emit(output.MessageEvent{Severity: output.SeverityWarning, Text: fmt.Sprintf("%s: %v", msg, err)}) + return nil +} diff --git a/internal/ui/run_awsconfig.go b/internal/ui/run_awsconfig.go index f66686b2..f3af400d 100644 --- a/internal/ui/run_awsconfig.go +++ b/internal/ui/run_awsconfig.go @@ -34,6 +34,6 @@ func RunConfigProfile(parentCtx context.Context, containers []config.ContainerCo if err != nil { return err } - return awsconfig.Setup(ctx, sink, resolvedHost, status) + return awsconfig.Setup(ctx, sink, resolvedHost, status, true) }) } diff --git a/test/integration/awsconfig_test.go b/test/integration/awsconfig_test.go index dcbc1001..4ae0653b 100644 --- a/test/integration/awsconfig_test.go +++ b/test/integration/awsconfig_test.go @@ -308,6 +308,59 @@ func TestSetupAWSCreatesAWSProfileWhenConfirmed(t *testing.T) { assert.NotContains(t, out.String(), "Skipped adding LocalStack AWS profile.") } +// TestSetupAWSExitsNonZeroWhenProfileWriteFails guards DEVX-941. Writing the +// profile is the whole purpose of `lstk setup aws`, but the command used to emit a +// warning and return nil when the write failed — exiting 0 and masking the failure +// from users, CI, and agents. We make ~/.aws read-only so CheckProfileStatus still +// sees the profile files as absent (the prompt appears) but the actual write fails, +// then confirm the prompt and assert a non-zero exit. +func TestSetupAWSExitsNonZeroWhenProfileWriteFails(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("PTY not supported on Windows") + } + if os.Geteuid() == 0 { + t.Skip("root bypasses directory permissions, so the profile write would not fail") + } + baseEnv, tmpHome := awsConfigEnv(t) + + // A read-only ~/.aws keeps the profile files absent (so the prompt still appears) + // while making their creation fail inside upsertSection's SaveTo. + awsDir := filepath.Join(tmpHome, ".aws") + require.NoError(t, os.MkdirAll(awsDir, 0500)) + // Restore write permission before t.TempDir cleanup so the dir can be removed. + t.Cleanup(func() { _ = os.Chmod(awsDir, 0700) }) + + ctx := testContext(t) + cmd := exec.CommandContext(ctx, binaryPath(), "setup", "aws") + cmd.Env = baseEnv + + ptmx, err := pty.Start(cmd) + require.NoError(t, err, "failed to start command in PTY") + defer func() { _ = ptmx.Close() }() + + out := &syncBuffer{} + outputCh := make(chan struct{}) + go func() { + _, _ = io.Copy(out, ptmx) + close(outputCh) + }() + + require.Eventually(t, func() bool { + return bytes.Contains(out.Bytes(), []byte(awsSetupPrompt)) + }, 2*time.Minute, 200*time.Millisecond, "AWS profile prompt should appear") + + _, err = ptmx.Write([]byte("y")) + require.NoError(t, err) + + err = cmd.Wait() + <-outputCh + requireExitCode(t, 1, err) + + assert.Contains(t, out.String(), "Could not set up the LocalStack AWS profile") + assert.NotContains(t, out.String(), "Created LocalStack profile") +} + func TestConfigProfileDoesNotCreateAWSProfileWhenDeclined(t *testing.T) { t.Parallel() if runtime.GOOS == "windows" {