Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
69 changes: 69 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -1473,10 +1473,62 @@ func Execute() error {
return err
}

// optionalValueFlags lists double-dash Atmos flags that use NoOptDefVal (optional value).
//
// When pflag encounters these flags without "=", it uses NoOptDefVal instead of consuming
// the next argument as the value. This breaks the standard CLI convention of "--flag value"
// (space-separated). For example, "--identity admin" would set identity to "__SELECT__" and
// leave "admin" as an orphaned positional arg that gets passed through to terraform.
//
// This map is used by normalizeOptionalValueFlags to convert "--flag value" to "--flag=value"
// before Cobra parses, ensuring both "--flag=value" and "--flag value" work correctly.
var optionalValueFlags = map[string]bool{
"--identity": true,
}

// normalizeOptionalValueFlags converts "--flag value" to "--flag=value" for flags
// registered with NoOptDefVal (optional value).
//
// pflag's NoOptDefVal feature treats flags like booleans: "--flag" without "=" uses
// the NoOptDefVal instead of consuming the next argument. This breaks the standard
// CLI convention where "--flag value" passes "value" as the flag's value.
//
// This function runs BEFORE Cobra parsing to ensure both forms work correctly:
// - "--identity admin" → "--identity=admin" (normalized).
// - "--identity=admin" → "--identity=admin" (unchanged).
// - "--identity" → "--identity" (unchanged, triggers interactive selection).
// - "--identity --help" → "--identity --help" (unchanged, next arg is a flag).
func normalizeOptionalValueFlags(args []string) []string {
result := make([]string, 0, len(args))
for i := 0; i < len(args); i++ {
arg := args[i]

// Stop normalizing after "--" (end of options marker per POSIX convention).
if arg == "--" {
result = append(result, args[i:]...)
break
}

// Check if this is an optional-value flag without "=".
if optionalValueFlags[arg] && i+1 < len(args) && !strings.HasPrefix(args[i+1], "-") {
// Join flag and value: "--identity admin" → "--identity=admin".
result = append(result, arg+"="+args[i+1])
i++ // Skip the value arg.
continue
}

result = append(result, arg)
}
return result
}

// preprocessCompatibilityFlags separates Atmos flags from pass-through flags.
// 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.
//
// The separated args are stored globally via compat.SetSeparated() and can be
// retrieved in RunE via compat.GetSeparated().
func preprocessCompatibilityFlags() {
Expand All @@ -1485,9 +1537,18 @@ func preprocessCompatibilityFlags() {
return
}

// Normalize flags with NoOptDefVal before any further processing.
// This converts "--identity value" to "--identity=value" so pflag correctly
// captures the value instead of using NoOptDefVal and orphaning the value arg.
osArgs = normalizeOptionalValueFlags(osArgs)

// Find target command without parsing flags.
targetCmd, _, _ := RootCmd.Find(osArgs)
if targetCmd == nil {
// Even if we can't find the command, apply normalization.
if len(osArgs) != len(os.Args[1:]) {
RootCmd.SetArgs(osArgs)
}
return
}

Expand All @@ -1498,12 +1559,20 @@ func preprocessCompatibilityFlags() {
cmdName = c.Name()
}
if cmdName == "" {
// Apply normalization even if we can't determine the command.
if len(osArgs) != len(os.Args[1:]) {
RootCmd.SetArgs(osArgs)
}
return
}

// 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 len(osArgs) != len(os.Args[1:]) {
RootCmd.SetArgs(osArgs)
}
return
}

Expand Down
73 changes: 73 additions & 0 deletions cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,79 @@ func TestParseChdirFromArgs(t *testing.T) {
}
}

// TestNormalizeOptionalValueFlags tests the normalizeOptionalValueFlags function
// that converts "--flag value" to "--flag=value" for flags with NoOptDefVal.
func TestNormalizeOptionalValueFlags(t *testing.T) {
tests := []struct {
name string
args []string
expected []string
}{
{
name: "identity with space-separated value",
args: []string{"terraform", "plan", "vpc", "-s", "dev", "--identity", "admin"},
expected: []string{"terraform", "plan", "vpc", "-s", "dev", "--identity=admin"},
},
{
name: "identity with equals value is unchanged",
args: []string{"terraform", "plan", "vpc", "--identity=admin", "-s", "dev"},
expected: []string{"terraform", "plan", "vpc", "--identity=admin", "-s", "dev"},
},
{
name: "identity without value (interactive selection)",
args: []string{"terraform", "plan", "vpc", "-s", "dev", "--identity"},
expected: []string{"terraform", "plan", "vpc", "-s", "dev", "--identity"},
},
{
name: "identity followed by another flag",
args: []string{"terraform", "plan", "vpc", "--identity", "--stack", "dev"},
expected: []string{"terraform", "plan", "vpc", "--identity", "--stack", "dev"},
},
{
name: "identity followed by short flag",
args: []string{"terraform", "plan", "vpc", "--identity", "-s", "dev"},
expected: []string{"terraform", "plan", "vpc", "--identity", "-s", "dev"},
},
{
name: "identity after end-of-options marker is not normalized",
args: []string{"terraform", "plan", "vpc", "--", "--identity", "admin"},
expected: []string{"terraform", "plan", "vpc", "--", "--identity", "admin"},
},
{
name: "no optional value flags are unchanged",
args: []string{"terraform", "plan", "vpc", "-s", "dev", "--dry-run"},
expected: []string{"terraform", "plan", "vpc", "-s", "dev", "--dry-run"},
},
{
name: "empty args",
args: []string{},
expected: []string{},
},
{
name: "identity at beginning of args",
args: []string{"--identity", "admin", "terraform", "plan", "vpc"},
expected: []string{"--identity=admin", "terraform", "plan", "vpc"},
},
{
name: "multiple identity flags (edge case)",
args: []string{"terraform", "plan", "--identity", "first", "--identity", "second"},
expected: []string{"terraform", "plan", "--identity=first", "--identity=second"},
},
{
name: "identity with value that looks like a component name",
args: []string{"terraform", "plan", "vpc", "-s", "core-usw2-root", "--identity", "vbpt-core-gbl-root-admin"},
expected: []string{"terraform", "plan", "vpc", "-s", "core-usw2-root", "--identity=vbpt-core-gbl-root-admin"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := normalizeOptionalValueFlags(tt.args)
assert.Equal(t, tt.expected, result)
})
}
}

func TestSetupColorProfile(t *testing.T) {
tests := []struct {
name string
Expand Down
121 changes: 121 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,121 @@
# Fix AWS Default Profile Interference with SSO Authentication

**Date:** 2026-02-17

**Related Issue:** Misleading error when `AWS_PROFILE` or `[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 misleading error message when a user has a default AWS
profile configured, making the root cause difficult to diagnose.

## Background

When running `atmos auth login`, the SSO provider loads an AWS config to initialize the OIDC client for
device authorization. The `LoadIsolatedAWSConfig` function was intended to completely isolate this config
loading from the user's existing AWS environment. However, the implementation had a gap:

- `WithIsolatedAWSEnv` correctly unsets `AWS_PROFILE`, `AWS_CONFIG_FILE`, `AWS_SHARED_CREDENTIALS_FILE`,
and credential env vars during config loading.
- `config.WithSharedConfigProfile("")` was used with the intent to "disable shared config loading," but
in the AWS SDK v2, an empty string means "use the default profile."
- The AWS SDK still loads `~/.aws/config` and `~/.aws/credentials` from their default filesystem paths
even when the corresponding env vars are unset.

This means if the user has a `[default]` profile in `~/.aws/config` that references SSO configuration,
credential processes, or other non-trivial settings, the SDK attempts to resolve those during
`LoadDefaultConfig` and may fail with a confusing error.

## Symptoms

```
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 hints suggest region/network issues, but the actual cause is the `[default]` profile in
`~/.aws/config` (or `AWS_PROFILE` env var) interfering with config loading.

## Root Cause

Two issues:

### 1. Incomplete isolation in `LoadIsolatedAWSConfig`

`config.WithSharedConfigProfile("")` does **not** disable shared config file loading. In the AWS SDK v2,
an empty profile name resolves to the default profile (`[default]`). The SDK still reads `~/.aws/config`
and `~/.aws/credentials` from their default paths (`$HOME/.aws/config` and `$HOME/.aws/credentials`).

The correct approach is to use `config.WithSharedConfigFiles([]string{})` and
`config.WithSharedCredentialsFiles([]string{})` to provide empty file lists, which prevents the SDK
from loading any shared config files.

### 2. Misleading error message

The error at `sso.go:153-161` only hints at region/network issues. It does not mention:
- The `AWS_PROFILE` environment variable as a potential cause.
- The `~/.aws/config` default profile as a potential cause.
- That Atmos auth isolates from external AWS configuration (so the user knows this was attempted).

## Fix

### Approach

1. Replace `config.WithSharedConfigProfile("")` with `config.WithSharedConfigFiles([]string{})` and
`config.WithSharedCredentialsFiles([]string{})` for complete filesystem isolation.
2. Add a warning log when `AWS_PROFILE` is set or `~/.aws/config` exists with a default profile,
informing the user that these will be ignored during SSO auth.
3. Improve the error message in the SSO provider to include hints about `AWS_PROFILE` and default
profiles in `~/.aws/config`.

### Implementation

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

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

This completely prevents the AWS SDK from reading any shared config files during isolated operations.

#### 2. Add `WarnIfAWSProfileSet` helper (`pkg/auth/cloud/aws/env.go`)

New function that logs a warning when `AWS_PROFILE` is set, informing the user that it will be
ignored during Atmos auth.

#### 3. Improve SSO error message (`pkg/auth/providers/aws/sso.go`)

Add hints about:
- Checking if `AWS_PROFILE` environment variable is set.
- Checking for a `[default]` profile in `~/.aws/config`.
- The fact that Atmos auth operates in an isolated AWS environment.

#### 4. Call warning from SSO `Authenticate` method

Before loading the isolated config, call `WarnIfAWSProfileSet` to emit a debug-level warning.

### Files changed

| File | Change |
|----------------------------------------------|----------------------------------------------------------------------|
| `pkg/auth/cloud/aws/env.go` | Fix `LoadIsolatedAWSConfig` isolation; add `WarnIfAWSProfileSet` |
| `pkg/auth/providers/aws/sso.go` | Improve error hints; call `WarnIfAWSProfileSet` before config load |
| `pkg/auth/cloud/aws/env_test.go` | Tests for isolation fix and warning detection |
| `pkg/auth/providers/aws/sso_test.go` | Tests for improved error messages |

### 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 |
| `TestWarnIfAWSProfileSet_LogsWarning` | Warning is logged when AWS_PROFILE is set |
| `TestWarnIfAWSProfileSet_NoWarningWhenUnset` | No warning when AWS_PROFILE is not set |
| `TestSSOProvider_Authenticate_ErrorIncludesProfileHint` | Error message includes hint about AWS_PROFILE |
Loading
Loading