Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion cmd/auth_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func executeAuthExecCommandCore(cmd *cobra.Command, args []string) error {

// Prepare shell environment with file-based credentials.
// Start with current OS environment + global env from atmos.yaml and let PrepareShellEnvironment configure auth.
baseEnv := envpkg.MergeGlobalEnv(os.Environ(), atmosConfig.Env)
baseEnv := envpkg.MergeGlobalEnv(os.Environ(), atmosConfig.GetCaseSensitiveEnvVars())
envList, err := authManager.PrepareShellEnvironment(ctx, identityName, baseEnv)
if err != nil {
return fmt.Errorf("failed to prepare command environment: %w", err)
Expand Down
4 changes: 2 additions & 2 deletions cmd/cmd_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ func executeCustomCommand(
// Prepare ENV vars
// ENV var values support Go templates and have access to {{ .ComponentConfig.xxx.yyy.zzz }} Go template variables
// Start with current environment + global env from atmos.yaml to inherit PATH and other variables.
env := envpkg.MergeGlobalEnv(os.Environ(), atmosConfig.Env)
env := envpkg.MergeGlobalEnv(os.Environ(), atmosConfig.GetCaseSensitiveEnvVars())
for _, v := range commandConfig.Env {
key := strings.TrimSpace(v.Key)
value := v.Value
Expand Down Expand Up @@ -629,7 +629,7 @@ func executeCustomCommand(
commandName := fmt.Sprintf("%s-step-%d", commandConfig.Name, i)

// Pass the prepared environment with custom variables to the subprocess
err = e.ExecuteShell(commandToRun, commandName, workDir, env, false)
err = e.ExecuteShell(&atmosConfig, commandToRun, commandName, workDir, env, false)
errUtils.CheckErrorPrintAndExit(err, "", "")
}
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

57 changes: 51 additions & 6 deletions internal/exec/shell_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
log "github.com/cloudposse/atmos/pkg/logger"
"github.com/cloudposse/atmos/pkg/perf"
"github.com/cloudposse/atmos/pkg/schema"
"github.com/cloudposse/atmos/pkg/ui"
"github.com/cloudposse/atmos/pkg/ui/theme"
u "github.com/cloudposse/atmos/pkg/utils"
)
Expand Down Expand Up @@ -48,9 +49,28 @@ func ExecuteShellCommand(
}

cmd := exec.Command(command, args...)
// Build environment: os.Environ() + global env (atmos.yaml) + command-specific env.
// Global env has lowest priority after system env, command-specific env overrides both.
cmdEnv := envpkg.MergeGlobalEnv(os.Environ(), atmosConfig.Env)
// Build environment: os.Environ() + global env (atmos.yaml) + working dir .env + command-specific env.
// Global env has lowest priority after system env, command-specific env overrides all.
cmdEnv := envpkg.MergeGlobalEnv(os.Environ(), atmosConfig.GetCaseSensitiveEnvVars())

// Load working directory .env files if enabled.
if atmosConfig.Env.Files.Enabled && dir != "" {
dirEnv, loadedFiles, err := envpkg.LoadFromDirectory(
dir,
atmosConfig.Env.Files.Paths,
atmosConfig.Env.Files.Parents,
atmosConfig.BasePath,
)
if err != nil {
log.Debug("Failed to load .env files from working directory", "dir", dir, "error", err)
} else {
for _, file := range loadedFiles {
ui.Success(fmt.Sprintf("Loaded %s", filepath.Base(file)))
}
cmdEnv = envpkg.MergeEnvSlices(cmdEnv, envpkg.MapToSlice(dirEnv))
}
}

cmdEnv = append(cmdEnv, env...)
cmdEnv = append(cmdEnv, fmt.Sprintf("ATMOS_SHLVL=%d", newShellLevel))
cmd.Env = cmdEnv
Expand Down Expand Up @@ -111,13 +131,14 @@ func ExecuteShellCommand(

// ExecuteShell runs a shell script.
func ExecuteShell(
atmosConfig *schema.AtmosConfiguration,
command string,
name string,
dir string,
envVars []string,
dryRun bool,
) error {
defer perf.Track(nil, "exec.ExecuteShell")()
defer perf.Track(atmosConfig, "exec.ExecuteShell")()

newShellLevel, err := u.GetNextShellLevel()
if err != nil {
Expand All @@ -130,6 +151,30 @@ func ExecuteShell(
// This matches the behavior before commit 9fd7d156a where the environment
// was merged rather than replaced.
mergedEnv := os.Environ()

// Merge global env from atmos.yaml if atmosConfig is provided.
if atmosConfig != nil {
mergedEnv = envpkg.MergeGlobalEnv(mergedEnv, atmosConfig.GetCaseSensitiveEnvVars())

// Load working directory .env files if enabled.
if atmosConfig.Env.Files.Enabled && dir != "" {
dirEnv, loadedFiles, err := envpkg.LoadFromDirectory(
dir,
atmosConfig.Env.Files.Paths,
atmosConfig.Env.Files.Parents,
atmosConfig.BasePath,
)
if err != nil {
log.Debug("Failed to load .env files from working directory", "dir", dir, "error", err)
} else {
for _, file := range loadedFiles {
ui.Success(fmt.Sprintf("Loaded %s", filepath.Base(file)))
}
mergedEnv = envpkg.MergeEnvSlices(mergedEnv, envpkg.MapToSlice(dirEnv))
}
}
}

for _, envVar := range envVars {
mergedEnv = envpkg.UpdateEnvVar(mergedEnv, parseEnvVarKey(envVar), parseEnvVarValue(envVar))
}
Expand Down Expand Up @@ -249,7 +294,7 @@ func execTerraformShellCommand(

// Merge env vars, ensuring componentEnvList takes precedence.
// Include global env from atmos.yaml (lowest priority after system env).
mergedEnv := envpkg.MergeSystemEnvWithGlobal(componentEnvList, atmosConfig.Env)
mergedEnv := envpkg.MergeSystemEnvWithGlobal(componentEnvList, atmosConfig.GetCaseSensitiveEnvVars())

// Transfer stdin, stdout, and stderr to the new process and also set the target directory for the shell to start in
pa := os.ProcAttr{
Expand Down Expand Up @@ -354,7 +399,7 @@ func ExecAuthShellCommand(

// Merge env vars, ensuring authEnvList takes precedence.
// Include global env from atmos.yaml (lowest priority after system env).
mergedEnv := envpkg.MergeSystemEnvSimpleWithGlobal(authEnvList, atmosConfig.Env)
mergedEnv := envpkg.MergeSystemEnvSimpleWithGlobal(authEnvList, atmosConfig.GetCaseSensitiveEnvVars())

// Determine shell command and args.
shellCommand, shellCommandArgs := determineShell(shellOverride, shellArgs)
Expand Down
7 changes: 7 additions & 0 deletions internal/exec/shell_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ func TestExecuteShell(t *testing.T) {

t.Run("simple echo command", func(t *testing.T) {
err := ExecuteShell(
nil, // no atmosConfig needed for basic tests
"echo 'test'",
"test-shell",
".",
Expand All @@ -609,6 +610,7 @@ func TestExecuteShell(t *testing.T) {

t.Run("dry run mode", func(t *testing.T) {
err := ExecuteShell(
nil, // no atmosConfig needed for basic tests
"echo 'test'",
"test-shell",
".",
Expand All @@ -620,6 +622,7 @@ func TestExecuteShell(t *testing.T) {

t.Run("syntax error in shell command", func(t *testing.T) {
err := ExecuteShell(
nil, // no atmosConfig needed for basic tests
"echo 'unclosed quote",
"test-shell",
".",
Expand All @@ -631,6 +634,7 @@ func TestExecuteShell(t *testing.T) {

t.Run("custom environment variables", func(t *testing.T) {
err := ExecuteShell(
nil, // no atmosConfig needed for basic tests
"echo test",
"test-shell",
".",
Expand All @@ -654,6 +658,7 @@ func TestExecuteShell(t *testing.T) {
// The shell command should see the override value, not the parent value,
// but should still have access to PATH.
err := ExecuteShell(
nil, // no atmosConfig needed for basic tests
"test \"$TEST_OVERRIDE\" = \"custom_value\" && env | grep -q PATH",
"test-shell",
".",
Expand All @@ -677,6 +682,7 @@ func TestExecuteShell(t *testing.T) {

// Call ExecuteShell with empty env slice, just like workflow commands do.
err := ExecuteShell(
nil, // no atmosConfig needed for basic tests
"env",
"test-shell",
".",
Expand Down Expand Up @@ -711,6 +717,7 @@ func TestExecuteShell(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.cmd, func(t *testing.T) {
err := ExecuteShell(
nil, // no atmosConfig needed for basic tests
tc.cmd,
"test-shell",
".",
Expand Down
2 changes: 1 addition & 1 deletion internal/exec/stack_processor_process_stacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func ProcessStackConfig(
}

// Include atmos.yaml global env as lowest priority in the merge chain.
atmosConfigEnv := envpkg.ConvertMapStringToAny(atmosConfig.Env)
atmosConfigEnv := envpkg.ConvertMapStringToAny(atmosConfig.GetCaseSensitiveEnvVars())
globalAndTerraformEnv, err := m.Merge(atmosConfig, []map[string]any{atmosConfigEnv, globalEnvSection, terraformEnv})
if err != nil {
return nil, err
Expand Down
4 changes: 3 additions & 1 deletion internal/exec/workflow_adapters.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ func NewWorkflowCommandRunner(_ *schema.RetryConfig) *WorkflowCommandRunner {
}

// RunShell executes a shell command using ExecuteShell.
// Note: atmosConfig is nil here because workflows call RunAtmos for atmos commands,
// and shell commands don't need atmosConfig for basic execution.
func (r *WorkflowCommandRunner) RunShell(command, name, dir string, env []string, dryRun bool) error {
defer perf.Track(nil, "exec.WorkflowCommandRunner.RunShell")()

return ExecuteShell(command, name, dir, env, dryRun)
return ExecuteShell(nil, command, name, dir, env, dryRun)
}

// RunAtmos executes an atmos command using ExecuteShellCommand.
Expand Down
4 changes: 2 additions & 2 deletions internal/exec/workflow_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,14 +323,14 @@ func ExecuteWorkflow(

// Prepare environment variables: start with system env + global env from atmos.yaml.
// If identity is specified, also authenticate and add credentials.
stepEnv, err := prepareStepEnvironment(stepIdentity, step.Name, authManager, atmosConfig.Env)
stepEnv, err := prepareStepEnvironment(stepIdentity, step.Name, authManager, atmosConfig.GetCaseSensitiveEnvVars())
if err != nil {
return err
}
switch commandType {
case "shell":
commandName := fmt.Sprintf("%s-step-%d", workflow, stepIdx)
err = ExecuteShell(command, commandName, ".", stepEnv, dryRun)
err = ExecuteShell(&atmosConfig, command, commandName, ".", stepEnv, dryRun)
case "atmos":
args := strings.Fields(command)

Expand Down
100 changes: 96 additions & 4 deletions pkg/config/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ import (
errUtils "github.com/cloudposse/atmos/errors"
"github.com/cloudposse/atmos/pkg/auth/provisioning"
"github.com/cloudposse/atmos/pkg/config/casemap"
"github.com/cloudposse/atmos/pkg/env"
"github.com/cloudposse/atmos/pkg/filesystem"
log "github.com/cloudposse/atmos/pkg/logger"
"github.com/cloudposse/atmos/pkg/schema"
"github.com/cloudposse/atmos/pkg/ui"
u "github.com/cloudposse/atmos/pkg/utils"
"github.com/cloudposse/atmos/pkg/version"
"github.com/cloudposse/atmos/pkg/xdg"
Expand Down Expand Up @@ -276,6 +278,11 @@ func LoadConfig(configAndStacksInfo *schema.ConfigAndStacksInfo) (schema.AtmosCo
}
setEnv(v)

// Early .env file loading: MUST happen before profile detection.
// This allows ATMOS_PROFILE and other ATMOS_* vars in .env files to influence Atmos behavior.
// The base path may not be fully resolved yet, so we use what's available from config.
loadEnvFilesEarly(v, v.GetString("base_path"))

// Load profiles if specified via --profile flag or ATMOS_PROFILE env var.
// Profiles are loaded after base config but before final unmarshaling.
// This allows profiles to override base config settings.
Expand Down Expand Up @@ -325,9 +332,11 @@ func LoadConfig(configAndStacksInfo *schema.ConfigAndStacksInfo) (schema.AtmosCo
// Both AtmosConfiguration.Env and Command.Env use "env" but with different types
// (map[string]string vs []CommandEnv), causing mapstructure to silently drop Commands.
// Using mapstructure:"-" on the Env fields and extracting manually here fixes this.
if envMap := v.GetStringMapString("env"); len(envMap) > 0 {
atmosConfig.Env = envMap
}
//
// The env section supports two forms:
// 1. Structured: env.vars (map) + env.files (EnvFilesConfig)
// 2. Flat (legacy): env as direct key-value map
parseEnvConfig(v, &atmosConfig)
if envMap := v.GetStringMapString("templates.settings.env"); len(envMap) > 0 {
atmosConfig.Templates.Settings.Env = envMap
}
Expand Down Expand Up @@ -1231,10 +1240,93 @@ func loadEmbeddedConfig(v *viper.Viper) error {
return nil
}

// parseEnvConfig extracts the env section from Viper, supporting both structured and flat forms.
// Structured form: env.vars (map) + env.files (EnvFilesConfig)
// Flat form (legacy): env as direct key-value map.
func parseEnvConfig(v *viper.Viper, atmosConfig *schema.AtmosConfiguration) {
// Check if env section uses structured or flat form.
envRaw := v.Get("env")
if envRaw == nil {
return
}

envMap, ok := envRaw.(map[string]any)
if !ok {
return
}

// Detect structured form by presence of "vars" or "files" keys.
_, hasVars := envMap["vars"]
_, hasFiles := envMap["files"]

if hasVars || hasFiles {
// Structured form: env.vars + env.files.
if hasVars {
atmosConfig.Env.Vars = v.GetStringMapString("env.vars")
}
// Files config is parsed via mapstructure into atmosConfig.Env.Files.
atmosConfig.Env.Files.Enabled = v.GetBool("env.files.enabled")
atmosConfig.Env.Files.Paths = v.GetStringSlice("env.files.paths")
atmosConfig.Env.Files.Parents = v.GetBool("env.files.parents")
} else {
// Flat form: all keys are vars (backward compatibility).
atmosConfig.Env.Vars = v.GetStringMapString("env")
}
Comment on lines +1251 to +1278
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the parseEnvConfig function and surrounding context in load.go
cd cloudposse/atmos && head -1300 pkg/config/load.go | tail -100

Repository: cloudposse/atmos

Length of output: 126


🏁 Script executed:

# Check how Viper is configured in load.go - look for case sensitivity settings
rg -n "viper\.|SetEnvKeyReplacer|SetEnvPrefix" pkg/config/load.go -A 2 -B 2

Repository: cloudposse/atmos

Length of output: 10707


🏁 Script executed:

# Search for tests of parseEnvConfig
fd -t f "load.*test.go" pkg/config/ && rg -n "parseEnvConfig\|Test.*Env" pkg/config/ | head -30

Repository: cloudposse/atmos

Length of output: 401


🏁 Script executed:

# Check Viper's default behavior - search for any documentation or config about case handling
rg -n "caseSensitive|CaseSensitive" pkg/config/load.go -B 3 -A 3

Repository: cloudposse/atmos

Length of output: 2177


🏁 Script executed:

# Look for legacy env config examples or tests
rg -n "env:\s*\n\s+VARS|env:\s*\n\s+FILES" . -i --type yaml

Repository: cloudposse/atmos

Length of output: 251


🏁 Script executed:

# Look at the parseEnvConfig function in detail
sed -n '1250,1280p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 988


🏁 Script executed:

# Check the preserveCaseSensitiveMaps and casemap extraction to understand case preservation
sed -n '1329,1375p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 1923


🏁 Script executed:

# Search for tests that exercise parseEnvConfig with legacy flat env configs
rg -n "parseEnvConfig\|env:" pkg/config/load_test.go -A 5 -B 5 | head -80

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Check the env package for parent-walking logic mentioned in the scratchpad
fd -t f "env.go" pkg/ internal/ && rg -n "Parent\|parents" pkg/env/ internal/env/ 2>/dev/null | head -30

Repository: cloudposse/atmos

Length of output: 228


🏁 Script executed:

# Check Viper's behavior with case sensitivity and Get() method
web_search
Viper library case sensitivity behavior for map keys Get method

Repository: cloudposse/atmos

Length of output: 154


🏁 Script executed:

# Check env.go for parent-walking logic
sed -n '1,100p' pkg/env/env.go

Repository: cloudposse/atmos

Length of output: 3224


🏁 Script executed:

# Look for tests that cover env config parsing with legacy formats
rg -n "env:" pkg/config/load_test.go -A 10 | head -60

Repository: cloudposse/atmos

Length of output: 1829


🏁 Script executed:

# Check if there are YAML test fixtures with env configs
fd -t f ".*\.yaml" pkg/config | xargs rg "^env:" 2>/dev/null | head -20

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Understand how v.Get() works with Viper - look for usage patterns
rg -n "v\.Get\(" pkg/config/load.go -B 2 -A 5 | head -50

Repository: cloudposse/atmos

Length of output: 634


🏁 Script executed:

# Check how case preservation affects parseEnvConfig - does it restore case before parsing?
rg -n "parseEnvConfig\|CaseMaps\|Case.*Map" pkg/config/load.go | head -30

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Look at the order of operations: when is parseEnvConfig called vs when are case maps applied?
sed -n '200,400p' pkg/config/load.go | grep -n "parseEnvConfig\|preserveCaseSensitiveMaps\|Unmarshal"

Repository: cloudposse/atmos

Length of output: 275


🏁 Script executed:

# Check the schema.AtmosConfiguration to understand Env field structure
rg -n "type.*Env\s|Env.*struct" pkg/schema/ -A 15 | head -50

Repository: cloudposse/atmos

Length of output: 3762


🏁 Script executed:

# Search for any legacy env configs in tests or examples with VARS or FILES as flat keys
rg -n "VARS:|FILES:" . --type yaml -i -B 2 -A 2

Repository: cloudposse/atmos

Length of output: 50373


🏁 Script executed:

# Check if there are any integration tests showing backward compatibility for env
rg -n "backward|legacy|compat" pkg/config/load_test.go -i -B 2 -A 5

Repository: cloudposse/atmos

Length of output: 771


🏁 Script executed:

# Let's understand the exact behavior - check what v.Get returns for structured vs flat env
sed -n '1250,1280p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 988


🏁 Script executed:

# Check loadEnvFilesEarly to see if it ignores parents
sed -n '1285,1330p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 1389


🏁 Script executed:

# Look for how env vars with case are preserved - check the CaseMap usage
rg -n "CaseMap\|CaseMaps" pkg/config/load.go -A 3 -B 3 | head -60

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Check if there are any existing tests that show env parsing with edge cases
rg -n "env.*vars\|env.*files" pkg/config/load_test.go -B 5 -A 5 | head -80

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Check the order of operations - when is parseEnvConfig called vs preserveCaseSensitiveMaps
sed -n '120,160p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 1366


🏁 Script executed:

# Search for any env config in test fixtures with VARS or FILES as actual flat keys
find tests/ -name "*.yaml" -type f -exec grep -l "^env:" {} \; | head -5 | xargs grep -A 10 "^env:"

Repository: cloudposse/atmos

Length of output: 3733


🏁 Script executed:

# Check the schema to understand what Env field structure expects
rg -n "type Env struct|type EnvFilesConfig" pkg/schema/ -A 10

Repository: cloudposse/atmos

Length of output: 904


🏁 Script executed:

# Check the actual order of operations for parseEnvConfig and preserveCaseSensitiveMaps
sed -n '100,160p' pkg/config/load.go | grep -n "parseEnvConfig\|preserveCaseSensitiveMaps\|Unmarshal"

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Look for any actual usage or documentation of env config structure with VARS/FILES
rg -n "env:" tests/ --type yaml -A 15 | grep -i "vars:\|files:" | head -30

Repository: cloudposse/atmos

Length of output: 2024


🏁 Script executed:

# Check if there are integration tests that verify env config parsing behavior
rg -n "TestParseEnvConfig\|env.*config" pkg/config/load_test.go -i

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Look at the schema to understand legacy vs new form
sed -n '54,75p' pkg/schema/schema.go

Repository: cloudposse/atmos

Length of output: 1169


🏁 Script executed:

# Check if case maps are actually applied back to Viper or just tracked
rg -n "CaseMap\|RestoreCase\|applyCase" pkg/config/load.go pkg/casemap/ -i

Repository: cloudposse/atmos

Length of output: 112


🏁 Script executed:

# Check loadEnvFilesEarly - does it use parents option?
sed -n '1285,1328p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 1244


🏁 Script executed:

# Check env.LoadEnvFiles to see if it respects the parents flag
rg -n "LoadEnvFiles\|func.*parents" pkg/env/env.go -A 5 -B 2 | head -40

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Check if case maps are used/restored before parseEnvConfig
sed -n '100,150p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 1710


🏁 Script executed:

# Check if env.LoadEnvFiles signature includes parents parameter
rg -n "func LoadEnvFiles" pkg/env/env.go -A 10

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Let's verify the actual issue - create a test to see what happens with flat env with VARS key
rg -n "TestParseEnv\|parseEnv" pkg/config/load_test.go

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Check exact order: is parseEnvConfig called before or after preserveCaseSensitiveMaps?
sed -n '100,160p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 2140


🏁 Script executed:

# Look at what GetStringMapString does when value is not a map
rg -n "GetStringMapString" pkg/config/load.go -B 3 -A 3

Repository: cloudposse/atmos

Length of output: 919


🏁 Script executed:

# Get the full InitCliConfig function to see order of operations
sed -n '100,160p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 2140


🏁 Script executed:

# Search for preserveCaseSensitiveMaps call timing
rg -n "preserveCaseSensitiveMaps" pkg/config/load.go -B 5 -A 5

Repository: cloudposse/atmos

Length of output: 1346


🏁 Script executed:

# Verify: does GetStringMapString return empty map for non-existent paths?
python3 << 'EOF'
# Test Viper behavior conceptually
# When you call GetStringMapString("env.vars") on a config without env.vars but with env.vars as a flat key,
# does it return the flat key or empty?

# Viper documentation says GetStringMapString converts the value at that path to map[string]string
# If the path doesn't exist, it returns an empty map

print("Viper GetStringMapString behavior:")
print("- If path 'env.vars' doesn't exist as a nested map, returns empty map {}")
print("- If path 'env' exists with flat key 'vars' (string value), that's not a nested 'env.vars' path")
print("- So GetStringMapString('env.vars') returns empty {}")
print("\nThis means the review comment's concern is valid:")
print("1. Config has: env: { VARS: value }")
print("2. Viper lowercases to: env: { vars: value }")
print("3. Code checks hasVars := envMap['vars'] → true (key exists)")
print("4. Code calls GetStringMapString('env.vars') → empty (no nested map at that path)")
print("5. Result: atmosConfig.Env.Vars = {} (LOST)")
EOF

Repository: cloudposse/atmos

Length of output: 635


🏁 Script executed:

# Check the exact sequence in InitCliConfig
sed -n '95,150p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 1814


🏁 Script executed:

# Verify one more detail: does the case preservation actually help if called before parseEnvConfig?
# Check if CaseMaps are used to restore values
rg -n "CaseMap\|ApplyCase\|Restore" pkg/config/ --type go

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Final verification: test scenario with legacy flat env VARS key
python3 << 'EOF'
# Scenario: atmos.yaml contains:
# env:
#   GITHUB_TOKEN: "secret"
#   AWS_KEY: "key123"

# Viper loads and lowercases:
# env:
#   github_token: "secret"
#   aws_key: "key123"

# Current code does:
# envMap = v.Get("env") → {github_token: "secret", aws_key: "key123"}
# _, hasVars = envMap["vars"] → false (no "vars" key)
# _, hasFiles = envMap["files"] → false (no "files" key)
# if hasVars || hasFiles → false
# else → v.GetStringMapString("env") ✓ WORKS

# But with legacy config containing VARS/FILES keys:
# env:
#   VARS:
#     GITHUB_TOKEN: "secret"
#   FILES:
#     enabled: true

# Viper lowercases:
# env:
#   vars:  ← THIS IS A STRING VALUE, not a map!
#     github_token: "secret"
#   files:  ← THIS IS A BOOL or struct, not a map!
#     enabled: true

# Current code does:
# envMap = v.Get("env") → {vars: {...}, files: {...}}
# _, hasVars = envMap["vars"] → true (KEY EXISTS)
# _, hasFiles = envMap["files"] → true (KEY EXISTS)
# if hasVars || hasFiles → true ✓ assumes structured
# v.GetStringMapString("env.vars") → empty {} ✗ LOST DATA

print("CONFIRMED: The review comment correctly identifies a real backward compatibility bug.")
print("\nLegacy config with VARS/FILES as flat keys will lose data:")
print("- hasVars/hasFiles check succeeds (keys exist)")
print("- GetStringMapString fails (values aren't nested maps)")
print("- Result: silent data loss")
EOF

Repository: cloudposse/atmos

Length of output: 334


Check value types before treating env config as structured form.

Legacy env configs with VARS or FILES as flat keys will be misclassified as structured. Because Viper lowercases keys to vars and files, the presence check succeeds even when those keys contain string values, not nested maps. Then GetStringMapString("env.vars") returns empty, silently losing the data.

✅ Proposed fix
-	_, hasVars := envMap["vars"]
-	_, hasFiles := envMap["files"]
-
-	if hasVars || hasFiles {
+	varsRaw, hasVars := envMap["vars"]
+	filesRaw, hasFiles := envMap["files"]
+	_, varsIsMap := varsRaw.(map[string]any)
+	_, filesIsMap := filesRaw.(map[string]any)
+
+	// If VARS/FILES are legacy flat keys (non-map), keep flat parsing.
+	if (hasVars && !varsIsMap) || (hasFiles && !filesIsMap) {
+		atmosConfig.Env.Vars = v.GetStringMapString("env")
+		return
+	}
+
+	if varsIsMap || filesIsMap {
 		// Structured form: env.vars + env.files.
 		if hasVars {
 			atmosConfig.Env.Vars = v.GetStringMapString("env.vars")
 		}
🤖 Prompt for AI Agents
In `@pkg/config/load.go` around lines 1251 - 1278, The code currently treats
presence of "vars" or "files" keys (via viper-lowercased envMap["vars"/"files"])
as indicating structured form even when those keys are flat string values;
update the detection logic to verify the actual types before assuming structured
form: check envMap["vars"] is a map[string]any (or can be decoded by
GetStringMapString) and envMap["files"] is a map[string]any or a slice/struct
expected by atmosConfig.Env.Files, only then set hasVars/hasFiles true and use
GetStringMapString("env.vars")/GetBool/GetStringSlice; otherwise fall back to
the flat form and populate atmosConfig.Env.Vars from
v.GetStringMapString("env"). Ensure you reference and update the existing
symbols envRaw, envMap, envMap["vars"], envMap["files"], hasVars/hasFiles,
GetStringMapString("env.vars"), and atmosConfig.Env.Files handling so legacy
string values are not lost.

}

// loadEnvFilesEarly loads .env files early in the config loading process.
// This must happen before profile detection to allow ATMOS_* variables
// from .env files to influence Atmos behavior.
// Returns the list of loaded files for UI feedback.
func loadEnvFilesEarly(v *viper.Viper, basePath string) []string {
// Check if env.files is enabled (quick bootstrap read).
if !v.GetBool("env.files.enabled") {
return nil
}

paths := v.GetStringSlice("env.files.paths")
if len(paths) == 0 {
paths = []string{".env"} // Default pattern.
}

// Determine base path for loading.
if basePath == "" {
basePath = v.GetString("base_path")
}
if basePath == "" {
basePath = "."
}

// Load .env files from base path.
envVars, loadedFiles, err := env.LoadEnvFiles(basePath, paths)
if err != nil {
log.Debug("Failed to load .env files", "error", err)
Comment on lines +1286 to +1307
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's understand the context around lines 1286-1307 in pkg/config/load.go
cd /tmp && head -1320 /dev/stdin | tail -100 <<'EOF'
File will be read next
EOF

# Actually, let me read the file directly
head -1320 pkg/config/load.go | tail -100

Repository: cloudposse/atmos

Length of output: 247


🏁 Script executed:

# Find the function definition around line 1286
sed -n '1250,1320p' pkg/config/load.go | cat -n

Repository: cloudposse/atmos

Length of output: 2763


🏁 Script executed:

# Look for env.files.parents usage in the codebase
rg "env\.files\.parents" -A 3 -B 3

Repository: cloudposse/atmos

Length of output: 1157


🏁 Script executed:

# Find the env.LoadEnvFiles function signature
rg "func.*LoadEnvFiles" -A 5

Repository: cloudposse/atmos

Length of output: 633


🏁 Script executed:

# Search for the main loader that handles .env files
rg "LoadEnvFiles" -B 5 -A 10

Repository: cloudposse/atmos

Length of output: 8528


🏁 Script executed:

# Check the entire function that contains lines 1286-1307
sed -n '1200,1350p' pkg/config/load.go

Repository: cloudposse/atmos

Length of output: 4722


🏁 Script executed:

# Find LoadFromDirectory implementation
rg "LoadFromDirectory" -A 20

Repository: cloudposse/atmos

Length of output: 10461


🏁 Script executed:

# Search for where the parents flag is actually used in env loading
rg "\.Parents" pkg/env/ -B 3 -A 3

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Check if there's a broader function that respects parents
rg "parents" pkg/env/file.go -B 2 -A 5

Repository: cloudposse/atmos

Length of output: 834


🏁 Script executed:

# Look for where the main loader (after profile detection) loads env files
rg "loadEnvFiles" -i -B 3 -A 3

Repository: cloudposse/atmos

Length of output: 7519


Early .env loader doesn't respect env.files.parents flag.

Currently, loadEnvFilesEarly uses env.LoadEnvFiles(basePath, paths), which only loads from a single directory and has no parents support. The main loader later uses env.LoadFromDirectory(dir, patterns, parents, repoRoot) which correctly walks up to the repository root when env.files.parents: true. Since early loading happens before profile detection, variables like ATMOS_PROFILE in parent directories won't be seen, causing the behavior to diverge from the main loader's handling of the same flag.

Update the early loader to use LoadFromDirectory with the parents flag (and repoRoot from config) so parent-walked .env files are honored during profile detection.

🤖 Prompt for AI Agents
In `@pkg/config/load.go` around lines 1286 - 1307, The early .env loader
(loadEnvFilesEarly) currently calls env.LoadEnvFiles(basePath, paths) which
ignores the env.files.parents flag; change it to call
env.LoadFromDirectory(basePath, paths, v.GetBool("env.files.parents"),
v.GetString("repo_root")) so parent traversal is honored during early loading
and profile detection, ensuring it uses the same repoRoot and parents flag as
the main loader; preserve returned envVars, loadedFiles and existing error
handling (log.Debug and behavior) after the switch.

return nil
}

// Inject into process environment (don't override existing).
// This allows ATMOS_* vars to influence Atmos behavior.
for k, val := range envVars {
if _, exists := os.LookupEnv(k); !exists {
if err := os.Setenv(k, val); err != nil { //nolint:forbidigo // Intentional: setting env vars from .env files
log.Debug("Failed to set env var from .env file", "key", k, "error", err)
}
}
}

// Report loaded files via UI.
for _, file := range loadedFiles {
ui.Success(fmt.Sprintf("Loaded %s", filepath.Base(file)))
}

return loadedFiles
}

// caseSensitivePaths lists the YAML paths that need case preservation.
// Viper lowercases all map keys, but these sections need original case.
var caseSensitivePaths = []string{
"env", // Environment variables (e.g., GITHUB_TOKEN)
"env", // Environment variables (e.g., GITHUB_TOKEN) - flat form
"env.vars", // Environment variables - structured form
"auth.identities", // Auth identity names (e.g., SuperAdmin)
}

Expand Down
Loading
Loading