From 460e113f799cb5c3b5c9406cc698f4e6fec834ce Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 16:04:41 +0000 Subject: [PATCH] fix: Resolving stale pointer for SOPS vars in env.default during generation `Generate` captured `targetApp` as a range-copy before calling `ResolveForEnvironment`. Resolution writes resolved SOPS values back into `appDef.Apps[i].Env.Production` (the actual slice element), but `targetApp` still pointed to the pre-resolution copy where all SOPS `Value` fields were nil. `cast.ToString(nil)` then produced empty strings, so every variable declared in `env.default` with `"source": "sops"` was written as `KEY=""` in the generated .env file. Fix: store the app index during the lookup, then access `appDef.Apps[targetAppIdx]` after resolution so the merged env reflects the fully resolved values. --- internal/cmd/env/generate.go | 17 +++--- internal/cmd/env/generate_test.go | 92 +++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 8 deletions(-) diff --git a/internal/cmd/env/generate.go b/internal/cmd/env/generate.go index 85e0e445..c54d3156 100644 --- a/internal/cmd/env/generate.go +++ b/internal/cmd/env/generate.go @@ -7,7 +7,6 @@ import ( "github.com/pkg/errors" "github.com/urfave/cli/v3" - "github.com/ainsleydev/webkit/internal/appdef" "github.com/ainsleydev/webkit/internal/cmdtools" "github.com/ainsleydev/webkit/internal/secrets" "github.com/ainsleydev/webkit/pkg/env" @@ -49,15 +48,15 @@ func Generate(ctx context.Context, input cmdtools.CommandInput) error { environment := env.Environment(environmentStr) - var targetApp *appdef.App - for _, app := range appDef.Apps { + targetAppIdx := -1 + for i, app := range appDef.Apps { if app.Name == appName { - targetApp = &app + targetAppIdx = i break } } - if targetApp == nil { + if targetAppIdx == -1 { return fmt.Errorf("app '%s' not found in app.json", appName) } @@ -86,7 +85,9 @@ func Generate(ctx context.Context, input cmdtools.CommandInput) error { return err } - mergedApp := targetApp.MergeEnvironments(appDef.Shared.Env) + // Use the slice element directly after resolution so that SOPS vars resolved + // into appDef.Apps[i].Env are visible, rather than the pre-resolution copy. + mergedApp := appDef.Apps[targetAppIdx].MergeEnvironments(appDef.Shared.Env) vars, err := mergedApp.GetVarsForEnvironment(environment) if err != nil { @@ -101,7 +102,7 @@ func Generate(ctx context.Context, input cmdtools.CommandInput) error { err = writeMapToFile(writeArgs{ Input: input, Vars: vars, - App: *targetApp, + App: appDef.Apps[targetAppIdx], Environment: environment, CustomOutputPath: outputPath, }) @@ -110,7 +111,7 @@ func Generate(ctx context.Context, input cmdtools.CommandInput) error { } if outputPath == "" { - outputPath = fmt.Sprintf("%s/.env%s", targetApp.Path, envSuffix(environment)) + outputPath = fmt.Sprintf("%s/.env%s", appDef.Apps[targetAppIdx].Path, envSuffix(environment)) } printer.Success(fmt.Sprintf("Generated env file for app '%s' (%s) at: %s", appName, environment, outputPath)) diff --git a/internal/cmd/env/generate_test.go b/internal/cmd/env/generate_test.go index 3372833d..38e907a3 100644 --- a/internal/cmd/env/generate_test.go +++ b/internal/cmd/env/generate_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/ainsleydev/webkit/internal/appdef" "github.com/ainsleydev/webkit/pkg/env" @@ -73,6 +74,97 @@ func TestGetEnvironmentVars(t *testing.T) { } } +// TestGenerateDefaultSOPSResolution is a regression test for the stale-pointer bug +// where targetApp was captured as a range copy before ResolveForEnvironment ran, +// causing env.default SOPS vars to appear as empty strings in the generated file. +// It simulates the resolution step and confirms that reading from the updated slice +// element (not the pre-resolution copy) produces non-nil values. +func TestGenerateDefaultSOPSResolution(t *testing.T) { + t.Parallel() + + t.Run("Stale copy has nil SOPS value", func(t *testing.T) { + t.Parallel() + + apps := []appdef.App{ + { + Name: "cms", + Env: appdef.Environment{ + Default: appdef.EnvVar{ + "SECRET_KEY": {Source: appdef.EnvSourceSOPS, Value: nil}, + }, + Production: appdef.EnvVar{ + "NODE_ENV": {Source: appdef.EnvSourceValue, Value: "production"}, + }, + }, + }, + } + + // Simulate what ResolveForEnvironment writes: resolved value into the slice element. + apps[0].Env.Production["SECRET_KEY"] = appdef.EnvValue{ + Source: appdef.EnvSourceSOPS, + Value: "super-secret", + } + + // A range copy (as the old code did) captured before resolution + // would still hold Value: nil for SECRET_KEY. + var staleCopy *appdef.App + origApps := []appdef.App{ + { + Name: "cms", + Env: appdef.Environment{ + Default: appdef.EnvVar{ + "SECRET_KEY": {Source: appdef.EnvSourceSOPS, Value: nil}, + }, + Production: appdef.EnvVar{ + "NODE_ENV": {Source: appdef.EnvSourceValue, Value: "production"}, + }, + }, + }, + } + for _, app := range origApps { + if app.Name == "cms" { + staleCopy = &app //nolint:gosec // intentional copy to reproduce the bug + break + } + } + require.NotNil(t, staleCopy) + + staleVars, err := staleCopy.MergeEnvironments(appdef.Environment{}).GetVarsForEnvironment(env.Production) + require.NoError(t, err) + assert.Nil(t, staleVars["SECRET_KEY"].Value, "stale copy should show the bug: Value is nil") + }) + + t.Run("Slice element has resolved SOPS value", func(t *testing.T) { + t.Parallel() + + apps := []appdef.App{ + { + Name: "cms", + Env: appdef.Environment{ + Default: appdef.EnvVar{ + "SECRET_KEY": {Source: appdef.EnvSourceSOPS, Value: nil}, + }, + Production: appdef.EnvVar{ + "NODE_ENV": {Source: appdef.EnvSourceValue, Value: "production"}, + }, + }, + }, + } + + // Simulate ResolveForEnvironment writing the resolved value into the Production map. + apps[0].Env.Production["SECRET_KEY"] = appdef.EnvValue{ + Source: appdef.EnvSourceSOPS, + Value: "super-secret", + } + + // Reading from the slice element (as the fixed code does) yields the resolved value. + vars, err := apps[0].MergeEnvironments(appdef.Environment{}).GetVarsForEnvironment(env.Production) + require.NoError(t, err) + assert.Equal(t, "super-secret", vars["SECRET_KEY"].Value, "slice element should have resolved value") + assert.Equal(t, "production", vars["NODE_ENV"].Value) + }) +} + func TestEnvSuffix(t *testing.T) { t.Parallel()