Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions cmd/internal/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/spf13/pflag"

errUtils "github.com/cloudposse/atmos/errors"
"github.com/cloudposse/atmos/pkg/flags"
"github.com/cloudposse/atmos/pkg/flags/compat"
"github.com/cloudposse/atmos/pkg/perf"
)
Expand Down Expand Up @@ -336,3 +337,37 @@ func GetSubcommandCompatFlags(providerName, subcommand string) map[string]compat
}
return providerFlags[subcommand]
}

// commandFlagRegistries stores FlagRegistry instances per command provider.
// This allows preprocessCompatibilityFlags to use PreprocessNoOptDefValArgs
// from the registered flag registries instead of hardcoded flag maps.
var commandFlagRegistries = struct {
mu sync.RWMutex
registries map[string]*flags.FlagRegistry // provider name -> flag registry
}{
registries: make(map[string]*flags.FlagRegistry),
}

// RegisterCommandFlagRegistry registers a FlagRegistry for a command provider.
// The providerName is the top-level command (e.g., "terraform").
// This enables preprocessCompatibilityFlags to use the registry's
// PreprocessNoOptDefValArgs for normalizing NoOptDefVal flags.
func RegisterCommandFlagRegistry(providerName string, registry *flags.FlagRegistry) {
defer perf.Track(nil, "internal.RegisterCommandFlagRegistry")()

commandFlagRegistries.mu.Lock()
defer commandFlagRegistries.mu.Unlock()

commandFlagRegistries.registries[providerName] = registry
}

// GetCommandFlagRegistry returns the FlagRegistry for a command provider.
// Returns nil if no registry is registered for the provider.
func GetCommandFlagRegistry(providerName string) *flags.FlagRegistry {
defer perf.Track(nil, "internal.GetCommandFlagRegistry")()

commandFlagRegistries.mu.RLock()
defer commandFlagRegistries.mu.RUnlock()

return commandFlagRegistries.registries[providerName]
}
84 changes: 84 additions & 0 deletions cmd/internal/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,90 @@ func TestRegisterCommandCompatFlags_Concurrent(t *testing.T) {
}
}

// resetFlagRegistries clears the commandFlagRegistries for test isolation.
func resetFlagRegistries() {
commandFlagRegistries.mu.Lock()
defer commandFlagRegistries.mu.Unlock()
commandFlagRegistries.registries = make(map[string]*flags.FlagRegistry)
}

func TestRegisterCommandFlagRegistry(t *testing.T) {
resetFlagRegistries()

flagReg := flags.NewFlagRegistry()
flagReg.Register(&flags.StringFlag{
Name: "identity",
NoOptDefVal: "__SELECT__",
})

RegisterCommandFlagRegistry("terraform", flagReg)

got := GetCommandFlagRegistry("terraform")
require.NotNil(t, got, "registered flag registry should be retrievable")
assert.Same(t, flagReg, got, "should return the exact same registry instance")
}

func TestGetCommandFlagRegistry_NotFound(t *testing.T) {
resetFlagRegistries()

got := GetCommandFlagRegistry("nonexistent")
assert.Nil(t, got, "should return nil for unregistered provider")
}

func TestRegisterCommandFlagRegistry_Overwrite(t *testing.T) {
resetFlagRegistries()

first := flags.NewFlagRegistry()
second := flags.NewFlagRegistry()

RegisterCommandFlagRegistry("terraform", first)
RegisterCommandFlagRegistry("terraform", second)

got := GetCommandFlagRegistry("terraform")
assert.Same(t, second, got, "second registration should overwrite the first")
}

func TestRegisterCommandFlagRegistry_MultipleProviders(t *testing.T) {
resetFlagRegistries()

tfRegistry := flags.NewFlagRegistry()
hfRegistry := flags.NewFlagRegistry()

RegisterCommandFlagRegistry("terraform", tfRegistry)
RegisterCommandFlagRegistry("helmfile", hfRegistry)

assert.Same(t, tfRegistry, GetCommandFlagRegistry("terraform"))
assert.Same(t, hfRegistry, GetCommandFlagRegistry("helmfile"))
assert.Nil(t, GetCommandFlagRegistry("packer"))
}

func TestRegisterCommandFlagRegistry_Concurrent(t *testing.T) {
resetFlagRegistries()

done := make(chan bool)

// Concurrently register flag registries for different providers.
for i := 0; i < 10; i++ {
go func(idx int) {
provider := fmt.Sprintf("provider%d", idx)
reg := flags.NewFlagRegistry()
RegisterCommandFlagRegistry(provider, reg)
done <- true
}(i)
}

for i := 0; i < 10; i++ {
<-done
}

// Verify all registrations succeeded.
for i := 0; i < 10; i++ {
provider := fmt.Sprintf("provider%d", i)
got := GetCommandFlagRegistry(provider)
require.NotNil(t, got, "flag registry for %s should exist", provider)
}
}

// mockExperimentalCommandProvider is a test implementation that returns IsExperimental = true.
type mockExperimentalCommandProvider struct {
mockCommandProvider
Expand Down
22 changes: 22 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -1477,6 +1477,10 @@ func Execute() error {
// This is called BEFORE Cobra parses, allowing us to filter out terraform/helmfile
// native flags that would otherwise be dropped by FParseErrWhitelist.
//
// It also normalizes flags with NoOptDefVal (like --identity) to ensure both
// "--flag value" and "--flag=value" syntax work correctly, using the existing
// FlagRegistry.PreprocessNoOptDefValArgs from the command's registered flag registry.
//
// The separated args are stored globally via compat.SetSeparated() and can be
// retrieved in RunE via compat.GetSeparated().
func preprocessCompatibilityFlags() {
Expand All @@ -1501,9 +1505,27 @@ func preprocessCompatibilityFlags() {
return
}

// Normalize flags with NoOptDefVal using the command's registered flag registry.
// This converts "--identity value" to "--identity=value" so pflag correctly
// captures the value instead of using NoOptDefVal and orphaning the value arg.
// Uses the same PreprocessNoOptDefValArgs that AtmosFlagParser.Parse() uses,
// driven by the flag registry rather than a hardcoded flag map.
argsChanged := false
if flagRegistry := internal.GetCommandFlagRegistry(cmdName); flagRegistry != nil {
normalized := flagRegistry.PreprocessNoOptDefValArgs(osArgs)
if len(normalized) != len(osArgs) {
osArgs = normalized
argsChanged = true
}
}

// Get compatibility flags from the command registry.
compatFlags := internal.GetCompatFlagsForCommand(cmdName)
if len(compatFlags) == 0 {
// No compat flags, but still apply normalization if args changed.
if argsChanged {
RootCmd.SetArgs(osArgs)
}
return
}

Expand Down
5 changes: 5 additions & 0 deletions cmd/terraform/terraform.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ func init() {
// This enables the COMPATIBILITY FLAGS section in help output.
internal.RegisterCommandCompatFlags("terraform", "terraform", TerraformGlobalCompatFlags())

// Register the flag registry for NoOptDefVal preprocessing in preprocessCompatibilityFlags.
// This enables the existing FlagRegistry.PreprocessNoOptDefValArgs to normalize
// flags like --identity before Cobra parsing.
internal.RegisterCommandFlagRegistry("terraform", terraformParser.Registry())

// Register this command with the registry.
internal.Register(&TerraformCommandProvider{})
}
Expand Down
181 changes: 181 additions & 0 deletions docs/fixes/2026-02-17-aws-default-profile-sso-interference.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
# Fix AWS Default Profile Interference with SSO Authentication

**Date:** 2026-02-17

**Related Issue:** Misleading error when `[default]` profile in `~/.aws/config` interferes with SSO
device authorization flow.

**Affected Atmos Version:** v1.160.0+ (introduced with Atmos Auth)

**Severity:** Medium — SSO authentication fails with a confusing error when a user has a `[default]`
profile with non-trivial settings in `~/.aws/config`.

## Background

When running `atmos auth login`, the SSO provider calls `LoadIsolatedAWSConfig` to load a clean AWS
config for the OIDC client. This function was intended to completely isolate config loading from the
user's environment. The sanitization had two layers:

1. **Env var isolation** (`WithIsolatedAWSEnv`) — Clears `AWS_PROFILE`, `AWS_CONFIG_FILE`,
`AWS_SHARED_CREDENTIALS_FILE`, and credential env vars during config loading. This worked correctly.
2. **Shared config file isolation** (`config.WithSharedConfigProfile("")`) — Intended to prevent loading
from `~/.aws/config`. This did **not** work as expected.

## Symptoms

```text
Error: failed to load AWS config

## Explanation
Failed to load AWS configuration for SSO authentication in region 'us-west-2'

## Hints
💡 Verify that the AWS region is valid and accessible
💡 Check your network connectivity and AWS service availability
```

The error suggests region/network issues, but the actual cause is the `[default]` profile in
`~/.aws/config` being loaded and failing to resolve (e.g., `credential_process` pointing to a
missing binary, or SSO settings that conflict with the Atmos-managed flow).

## Root Cause

The root cause is a gap in the sanitization process: `WithIsolatedAWSEnv` correctly sanitized
environment variables, but the AWS SDK v2 has **two independent mechanisms** to find `~/.aws/config`:

1. **`AWS_CONFIG_FILE` env var** — Sanitized correctly by `WithIsolatedAWSEnv`.
2. **Hardcoded `$HOME/.aws/config` default** — Was **not** blocked by env var cleanup.

The original code attempted to block mechanism #2 with `config.WithSharedConfigProfile("")`, but this
is ineffective due to how the AWS SDK v2 processes these options internally.

### Detailed SDK trace

The following traces through the AWS SDK v2 source code (github.com/aws/aws-sdk-go-v2/config) to show
exactly how `~/.aws/config` leaked through.

#### Step 1: `WithSharedConfigProfile("")` is a no-op

```go
// load_options.go:426-431
func (o LoadOptions) getSharedConfigProfile(ctx context.Context) (string, bool, error) {
if len(o.SharedConfigProfile) == 0 {
return "", false, nil // empty string → found=false
}
return o.SharedConfigProfile, true, nil
}
```

Setting `SharedConfigProfile` to `""` causes the getter to return `found=false`, which means
"no profile was specified." The SDK then falls back to the default profile:

```go
// shared_config.go:584-586
profile, ok, err = getSharedConfigProfile(ctx, configs)
if !ok {
profile = defaultSharedConfigProfile // falls back to "default"
}
```

So `WithSharedConfigProfile("")` effectively means **"use the `[default]` profile"**, the opposite
of the intent to disable profile loading.

#### Step 2: `SharedConfigFiles` was never set (remains `nil`)

The original code did not call `WithSharedConfigFiles(...)`, so `LoadOptions.SharedConfigFiles`
remained `nil`. The SDK's getter treats `nil` as "not specified":

```go
// load_options.go:448-454
func (o LoadOptions) getSharedConfigFiles(ctx context.Context) ([]string, bool, error) {
if o.SharedConfigFiles == nil {
return nil, false, nil // nil → found=false
}
return o.SharedConfigFiles, true, nil
}
```

When `found=false`, the SDK falls back to hardcoded defaults:

```go
// shared_config.go:656-658
if option.ConfigFiles == nil {
option.ConfigFiles = DefaultSharedConfigFiles // → []string{"~/.aws/config"}
}
```

`DefaultSharedConfigFiles` is `[]string{filepath.Join(home, ".aws", "config")}`. This path is
resolved from `$HOME`, not from `AWS_CONFIG_FILE`. Unsetting `AWS_CONFIG_FILE` has no effect.

#### Step 3: SDK loads `~/.aws/config` with `[default]` profile

With `profile = "default"` and `configFiles = ["~/.aws/config"]`, the SDK calls `loadIniFiles`
and `setFromIniSections`, which parses the `[default]` profile and attempts to resolve any
settings it contains (credential processes, SSO configuration, etc.).

If the `[default]` profile contains settings that fail to resolve (e.g., `credential_process`
pointing to a missing binary), the SDK returns an error that propagates up as
"failed to load AWS config" with no indication that it came from the user's shared config file.

### Summary

The env var sanitization in `WithIsolatedAWSEnv` was correct but insufficient. The AWS SDK v2
resolves shared config files through a hardcoded `$HOME/.aws/config` path that is independent of
the `AWS_CONFIG_FILE` env var. The only way to prevent this is to explicitly provide an empty
file list via `WithSharedConfigFiles([]string{})`.

## Fix

### Approach

Replace `config.WithSharedConfigProfile("")` with `config.WithSharedConfigFiles([]string{})` and
`config.WithSharedCredentialsFiles([]string{})` for complete filesystem isolation.

### Why this works

`WithSharedConfigFiles([]string{})` sets `LoadOptions.SharedConfigFiles` to a non-nil empty slice.
The SDK's getter returns `found=true` with an empty list:

```go
func (o LoadOptions) getSharedConfigFiles(ctx context.Context) ([]string, bool, error) {
if o.SharedConfigFiles == nil { // []string{} is NOT nil
return nil, false, nil
}
return o.SharedConfigFiles, true, nil // returns empty slice, found=true
}
```

Since `found=true`, the SDK uses the provided (empty) list instead of falling back to defaults:

```go
if option.ConfigFiles == nil { // NOT nil (it's []string{})
option.ConfigFiles = DefaultSharedConfigFiles // SKIPPED
}
```

`loadIniFiles([]string{})` loads nothing. Combined with `WithIsolatedAWSEnv` clearing env vars,
this achieves complete isolation from both environment variables and filesystem config files.

### Implementation

#### Fix `LoadIsolatedAWSConfig` (`pkg/auth/cloud/aws/env.go`)

Replace `config.WithSharedConfigProfile("")` with:
- `config.WithSharedConfigFiles([]string{})`
- `config.WithSharedCredentialsFiles([]string{})`

### Files changed

| File | Change |
|----------------------------------------------|----------------------------------------------------------------------|
| `pkg/auth/cloud/aws/env.go` | Fix `LoadIsolatedAWSConfig` shared config file isolation |
| `pkg/auth/cloud/aws/env_test.go` | Tests for isolation fix |
| `pkg/auth/providers/aws/sso.go` | Remove misleading error hints now that isolation is complete |

### Tests

| Test | What it verifies |
|---------------------------------------------------------|-------------------------------------------------------------------|
| `TestWithIsolatedAWSEnv_ClearsAllProblematicVars` | All problematic vars are cleared during execution and restored |
| `TestLoadIsolatedAWSConfig_IgnoresDefaultProfile` | Default profile in ~/.aws/config does not affect isolated config |
| `TestLoadIsolatedAWSConfig_DoesNotLoadSharedFiles` | Shared config file settings do not leak into isolated config |
Loading
Loading