From 9ea11c864c2f0ce70ea664544a19f85da56d3f4b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 6 May 2026 04:42:00 +0000 Subject: [PATCH 1/4] Initial plan From d15689f0231131e3e7ad80ca84425236a50b56c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 6 May 2026 05:04:21 +0000 Subject: [PATCH 2/4] fix: preserve secrets.generate vars in all config fields to prevent DesiredHash mismatch Add ExpandEnvInMapPreservingVars to config/env_expand.go that, in addition to preserving entire key submaps (env_vars, env_vars_secret, secret_env_vars), also preserves individual ${VAR} references whose variable name appears in the cfg.Secrets.Generate list. This ensures desiredStateHash produces identical output regardless of whether generated secret variables are present in the environment. Fixes the "plan stale: config hash mismatch" error that occurs when a secrets.generate key (e.g. STAGING_PG_PASSWORD) is referenced in a non-env_vars config field (e.g. Droplet user_data cloud-init script): at plan time the var is absent so os.ExpandEnv returns "", at apply time it holds the generated value, causing the hash to diverge. - config/env_expand.go: add ExpandEnvInMapPreservingVars + helpers - config/env_expand_test.go: tests for new function including hash-consistency check - cmd/wfctl/infra.go: add secretGenKeys helper; use ExpandEnvInMapPreservingVars in parseInfraResourceSpecs and planResourcesForEnv - cmd/wfctl/infra_plan_env_vars_preserve_test.go: regression test for user_data hash mismatch - cmd/wfctl/testdata/infra-with-env-var-refs.yaml: add secrets.generate + droplet with user_data Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/643f6bb1-a20d-4468-94a6-ccebf7c70d3c Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- cmd/wfctl/ci_run_dryrun.go | 20 +-- cmd/wfctl/dryrun_test.go | 1 - cmd/wfctl/infra.go | 32 +++- .../infra_plan_env_vars_preserve_test.go | 65 ++++++++ .../testdata/infra-with-env-var-refs.yaml | 20 +++ config/env_expand.go | 70 +++++++++ config/env_expand_test.go | 146 ++++++++++++++++++ 7 files changed, 340 insertions(+), 14 deletions(-) diff --git a/cmd/wfctl/ci_run_dryrun.go b/cmd/wfctl/ci_run_dryrun.go index 25690c316..ab007f35b 100644 --- a/cmd/wfctl/ci_run_dryrun.go +++ b/cmd/wfctl/ci_run_dryrun.go @@ -12,16 +12,16 @@ import ( // DryRunDeployPlan is the structured dry-run output for ci run --phase deploy. type DryRunDeployPlan struct { - Command string `json:"command"` - Environment string `json:"environment"` - Provider string `json:"provider"` - Strategy string `json:"strategy"` - PreDeploy []string `json:"pre_deploy,omitempty"` - DeployTarget string `json:"deploy_target"` - ImageRef string `json:"image_ref"` - ImageTagSource string `json:"image_tag_source"` - HealthCheck *DryRunHealthCheck `json:"health_check,omitempty"` - Secrets []DryRunSecretRef `json:"secrets,omitempty"` + Command string `json:"command"` + Environment string `json:"environment"` + Provider string `json:"provider"` + Strategy string `json:"strategy"` + PreDeploy []string `json:"pre_deploy,omitempty"` + DeployTarget string `json:"deploy_target"` + ImageRef string `json:"image_ref"` + ImageTagSource string `json:"image_tag_source"` + HealthCheck *DryRunHealthCheck `json:"health_check,omitempty"` + Secrets []DryRunSecretRef `json:"secrets,omitempty"` Services []DryRunServiceEntry `json:"services,omitempty"` } diff --git a/cmd/wfctl/dryrun_test.go b/cmd/wfctl/dryrun_test.go index 02fe112e3..0027e2e9f 100644 --- a/cmd/wfctl/dryrun_test.go +++ b/cmd/wfctl/dryrun_test.go @@ -695,4 +695,3 @@ ci: t.Errorf("follow-up command should include --config , got:\n%s", output) } } - diff --git a/cmd/wfctl/infra.go b/cmd/wfctl/infra.go index 7d940da89..25a9cdf97 100644 --- a/cmd/wfctl/infra.go +++ b/cmd/wfctl/infra.go @@ -364,6 +364,27 @@ func resourceSpecFromResolvedModule(r *config.ResolvedModule) interfaces.Resourc return spec } +// secretGenKeys returns the variable names declared in cfg.Secrets.Generate. +// These keys are preserved as literal ${VAR} references during plan-time +// config expansion so that desiredStateHash produces the same result +// regardless of whether the variable is currently set in the process +// environment. This fixes the "plan stale: config hash mismatch" error that +// occurs when a generated secret (e.g. STAGING_PG_PASSWORD) is referenced +// outside env_vars — for example in a Droplet user_data cloud-init script — +// where the variable is absent at plan time but present at apply time. +func secretGenKeys(cfg *config.WorkflowConfig) []string { + if cfg == nil || cfg.Secrets == nil { + return nil + } + keys := make([]string, 0, len(cfg.Secrets.Generate)) + for _, g := range cfg.Secrets.Generate { + if g.Key != "" { + keys = append(keys, g.Key) + } + } + return keys +} + // parseInfraResourceSpecs reads an infra config (resolving imports:) and // returns ResourceSpecs for all infra.* and platform.* modules. func parseInfraResourceSpecs(cfgFile string) ([]interfaces.ResourceSpec, error) { @@ -371,12 +392,13 @@ func parseInfraResourceSpecs(cfgFile string) ([]interfaces.ResourceSpec, error) if err != nil { return nil, fmt.Errorf("load %s: %w", cfgFile, err) } + secretVars := secretGenKeys(cfg) var specs []interfaces.ResourceSpec for _, m := range cfg.Modules { if !isInfraType(m.Type) { continue } - r := &config.ResolvedModule{Name: m.Name, Type: m.Type, Config: config.ExpandEnvInMapPreservingKeys(m.Config, infraPreserveKeys)} + r := &config.ResolvedModule{Name: m.Name, Type: m.Type, Config: config.ExpandEnvInMapPreservingVars(m.Config, infraPreserveKeys, secretVars)} specs = append(specs, resourceSpecFromResolvedModule(r)) } return specs, nil @@ -415,6 +437,7 @@ func planResourcesForEnv(path, envName string) ([]*config.ResolvedModule, error) if envName != "" && cfg.Environments != nil { topEnv = cfg.Environments[envName] } + secretVars := secretGenKeys(cfg) var out []*config.ResolvedModule for i := range cfg.Modules { m := &cfg.Modules[i] @@ -422,7 +445,7 @@ func planResourcesForEnv(path, envName string) ([]*config.ResolvedModule, error) continue } if envName == "" { - out = append(out, &config.ResolvedModule{Name: m.Name, Type: m.Type, Config: config.ExpandEnvInMapPreservingKeys(m.Config, infraPreserveKeys)}) + out = append(out, &config.ResolvedModule{Name: m.Name, Type: m.Type, Config: config.ExpandEnvInMapPreservingVars(m.Config, infraPreserveKeys, secretVars)}) continue } resolved, ok := m.ResolveForEnv(envName) @@ -470,7 +493,10 @@ func planResourcesForEnv(path, envName string) ([]*config.ResolvedModule, error) // Use the preserving variant so that env_vars submaps retain their // ${VAR} literals through plan serialization (apply-time injection // resolves them when the plugin creates/updates the resource). - resolved.Config = config.ExpandEnvInMapPreservingKeys(resolved.Config, infraPreserveKeys) + // secretVars are also preserved so that fields like user_data that + // reference generated secrets produce the same hash at plan time + // (variable unset) and apply time (variable set). + resolved.Config = config.ExpandEnvInMapPreservingVars(resolved.Config, infraPreserveKeys, secretVars) out = append(out, resolved) } return out, nil diff --git a/cmd/wfctl/infra_plan_env_vars_preserve_test.go b/cmd/wfctl/infra_plan_env_vars_preserve_test.go index ad17b4ee7..0d7997426 100644 --- a/cmd/wfctl/infra_plan_env_vars_preserve_test.go +++ b/cmd/wfctl/infra_plan_env_vars_preserve_test.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "os" "path/filepath" "strings" @@ -12,6 +13,7 @@ func TestParseInfraResourceSpecs_PreservesEnvVarRefs(t *testing.T) { t.Setenv("IMAGE_REF", "registry.example.com/api:abc123") t.Setenv("AUTH_TOKEN", "would-be-resolved-secret") t.Setenv("DATABASE_URL", "postgres://would-be-resolved") + // POSTGRES_PASSWORD is in secrets.generate — leave it unset to simulate plan time. specs, err := parseInfraResourceSpecs("testdata/infra-with-env-var-refs.yaml") if err != nil { @@ -82,6 +84,63 @@ func TestParseInfraResourceSpecs_PreservesEnvVarRefs(t *testing.T) { } } +// TestParseInfraResourceSpecs_PreservesSecretGenVarsInUserData is the regression +// test for the DesiredHash mismatch bug: a secret declared in secrets.generate +// that appears in a non-env_vars field (e.g. Droplet user_data) must be +// preserved as a literal ${VAR} so that desiredStateHash is identical whether +// or not the variable is present in the environment. +func TestParseInfraResourceSpecs_PreservesSecretGenVarsInUserData(t *testing.T) { + t.Setenv("DIGITALOCEAN_TOKEN", "actual-do-token") + t.Setenv("IMAGE_REF", "registry.example.com/api:abc123") + t.Setenv("AUTH_TOKEN", "would-be-resolved-secret") + t.Setenv("DATABASE_URL", "postgres://would-be-resolved") + + // --- Plan-time: POSTGRES_PASSWORD is NOT set (as happens in CI) --- + os.Unsetenv("POSTGRES_PASSWORD") + specsAtPlan, err := parseInfraResourceSpecs("testdata/infra-with-env-var-refs.yaml") + if err != nil { + t.Fatalf("parseInfraResourceSpecs (plan): %v", err) + } + hashAtPlan := desiredStateHash(specsAtPlan) + + // --- Apply-time: POSTGRES_PASSWORD IS set to the generated value --- + t.Setenv("POSTGRES_PASSWORD", "deadbeef1234567890abcdef12345678") + specsAtApply, err := parseInfraResourceSpecs("testdata/infra-with-env-var-refs.yaml") + if err != nil { + t.Fatalf("parseInfraResourceSpecs (apply): %v", err) + } + hashAtApply := desiredStateHash(specsAtApply) + + if hashAtPlan != hashAtApply { + // Serialize both spec lists for diagnostics. + planJSON, _ := json.MarshalIndent(specsAtPlan, "", " ") + applyJSON, _ := json.MarshalIndent(specsAtApply, "", " ") + t.Errorf("desiredStateHash mismatch between plan and apply:\n"+ + " plan hash: %s\n apply hash: %s\n\nplan specs:\n%s\n\napply specs:\n%s", + hashAtPlan, hashAtApply, planJSON, applyJSON) + } + + // Also verify that the user_data field in the droplet spec contains the + // literal ${POSTGRES_PASSWORD} reference (not an empty string or the value). + var dropletCfg map[string]any + for _, s := range specsAtApply { + if s.Name == "example-droplet" { + dropletCfg = s.Config + break + } + } + if dropletCfg == nil { + t.Fatal("example-droplet spec not found in parsed specs") + } + ud, _ := dropletCfg["user_data"].(string) + if !strings.Contains(ud, "${POSTGRES_PASSWORD}") { + t.Errorf("user_data should contain literal ${POSTGRES_PASSWORD}, got:\n%s", ud) + } + if strings.Contains(ud, "deadbeef") { + t.Errorf("user_data should NOT contain the resolved secret value, got:\n%s", ud) + } +} + // TestPlanEnvVarPreserveTestdataExists ensures the fixture file exists and // has the env_vars_secret block required for the preservation test. func TestPlanEnvVarPreserveTestdataExists(t *testing.T) { @@ -96,4 +155,10 @@ func TestPlanEnvVarPreserveTestdataExists(t *testing.T) { if !strings.Contains(string(b), "env_vars_secret") { t.Errorf("fixture missing env_vars_secret block — needed for preservation test") } + if !strings.Contains(string(b), "user_data") { + t.Errorf("fixture missing user_data block — needed for secret-gen preservation test") + } + if !strings.Contains(string(b), "secrets:") { + t.Errorf("fixture missing secrets: section — needed for secret-gen preservation test") + } } diff --git a/cmd/wfctl/testdata/infra-with-env-var-refs.yaml b/cmd/wfctl/testdata/infra-with-env-var-refs.yaml index 13876c3c6..9b569f13c 100644 --- a/cmd/wfctl/testdata/infra-with-env-var-refs.yaml +++ b/cmd/wfctl/testdata/infra-with-env-var-refs.yaml @@ -1,3 +1,10 @@ +secrets: + provider: github + generate: + - key: POSTGRES_PASSWORD + type: random_hex + length: 32 + modules: - name: do-provider type: iac.provider @@ -15,3 +22,16 @@ modules: AUTH_TOKEN: "${AUTH_TOKEN}" env_vars_secret: DB_URL: "${DATABASE_URL}" + - name: example-droplet + type: infra.droplet + config: + region: nyc3 + user_data: | + #cloud-config + write_files: + - path: /opt/app/docker-compose.yml + content: | + services: + postgres: + environment: + POSTGRES_PASSWORD: '${POSTGRES_PASSWORD}' diff --git a/config/env_expand.go b/config/env_expand.go index 0ea651f49..4ea978f62 100644 --- a/config/env_expand.go +++ b/config/env_expand.go @@ -100,6 +100,76 @@ func expandEnvInValueWithPreserve(v any, preserve map[string]struct{}) any { } } +// ExpandEnvInMapPreservingVars is like ExpandEnvInMapPreservingKeys but adds +// a second dimension of preservation: individual ${VAR} / $VAR references +// whose variable name appears in preserveVarNames are emitted as the literal +// "${name}" instead of being substituted from the process environment. +// +// Use case: plan-time serialisation of resource specs where a known set of +// secret variable names (e.g. cfg.Secrets.Generate keys) must produce +// hash-identical output regardless of whether the variable is present in the +// current environment. Without this, fields such as user_data that contain +// ${SECRET_VAR} produce different hashes at plan time (var unset → empty +// substitution) and apply time (var set → actual value), causing a spurious +// "plan stale: config hash mismatch". +// +// Precedence: preserveKeys takes priority — if a map key is in preserveKeys +// the entire subtree is deep-copied as-is (no expansion at all, matching +// ExpandEnvInMapPreservingKeys semantics). preserveVarNames only affects +// string values in portions of the tree that are NOT inside a preserved-key +// subtree. +func ExpandEnvInMapPreservingVars(m map[string]any, preserveKeys []string, preserveVarNames []string) map[string]any { + if m == nil { + return nil + } + preserveK := make(map[string]struct{}, len(preserveKeys)) + for _, k := range preserveKeys { + preserveK[k] = struct{}{} + } + preserveV := make(map[string]struct{}, len(preserveVarNames)) + for _, v := range preserveVarNames { + preserveV[v] = struct{}{} + } + return expandEnvInMapWithPreserveVars(m, preserveK, preserveV) +} + +func expandEnvInMapWithPreserveVars(m map[string]any, preserveK, preserveV map[string]struct{}) map[string]any { + out := make(map[string]any, len(m)) + for k, v := range m { + if _, isPreservedKey := preserveK[k]; isPreservedKey { + out[k] = deepCopyValue(v) + continue + } + out[k] = expandEnvInValueWithPreserveVars(v, preserveK, preserveV) + } + return out +} + +func expandEnvInValueWithPreserveVars(v any, preserveK, preserveV map[string]struct{}) any { + switch val := v.(type) { + case string: + if len(preserveV) == 0 { + return os.ExpandEnv(val) + } + return os.Expand(val, func(name string) string { + if _, ok := preserveV[name]; ok { + return "${" + name + "}" + } + return os.Getenv(name) + }) + case map[string]any: + return expandEnvInMapWithPreserveVars(val, preserveK, preserveV) + case []any: + out := make([]any, len(val)) + for i, item := range val { + out[i] = expandEnvInValueWithPreserveVars(item, preserveK, preserveV) + } + return out + default: + return v + } +} + // deepCopyValue copies a value preserving its structure. Maps and slices // are recursively copied; scalars (string, int, bool, nil) are returned // as-is. Used by ExpandEnvInMapPreservingKeys to insulate preserved diff --git a/config/env_expand_test.go b/config/env_expand_test.go index 0abbdb296..d280df9b1 100644 --- a/config/env_expand_test.go +++ b/config/env_expand_test.go @@ -1,6 +1,7 @@ package config import ( + "strings" "testing" ) @@ -246,3 +247,148 @@ func TestExpandEnvInMapPreservingKeys_EmptyPreserveListEqualsExpandEnvInMap(t *t t.Errorf("with empty preserve list, behavior should equal ExpandEnvInMap; got %q", out["k"]) } } + +// ── ExpandEnvInMapPreservingVars tests ─────────────────────────────────────── + +func TestExpandEnvInMapPreservingVars_NilInputReturnsNil(t *testing.T) { + if got := ExpandEnvInMapPreservingVars(nil, nil, nil); got != nil { + t.Errorf("expected nil, got %v", got) + } +} + +// TestExpandEnvInMapPreservingVars_SecretVarPreservedInUserData is the primary +// regression test for the user_data hash-mismatch bug: a secret var referenced +// in a non-env_vars field must be kept as a literal ${VAR} so that +// desiredStateHash is identical whether the var is set or not. +func TestExpandEnvInMapPreservingVars_SecretVarPreservedInUserData(t *testing.T) { + // Simulate plan-time: STAGING_PG_PASSWORD not in environment. + t.Setenv("STAGING_PG_PASSWORD", "") // ensure test isolation; simulates unset + + in := map[string]any{ + "region": "${DO_REGION}", + "user_data": "#cloud-config\nwrite_files:\n - content: |\n " + + "POSTGRES_PASSWORD: '${STAGING_PG_PASSWORD}'\n", + } + t.Setenv("DO_REGION", "nyc3") + + out := ExpandEnvInMapPreservingVars(in, + []string{"env_vars", "env_vars_secret"}, // preserveKeys + []string{"STAGING_PG_PASSWORD"}, // preserveVarNames + ) + + // Normal var (not in preserveVarNames) is still expanded. + if got := out["region"]; got != "nyc3" { + t.Errorf("region: got %q, want nyc3", got) + } + + // Secret var reference in user_data is preserved as literal. + ud, _ := out["user_data"].(string) + if !strings.Contains(ud, "${STAGING_PG_PASSWORD}") { + t.Errorf("user_data: want ${STAGING_PG_PASSWORD} preserved, got:\n%s", ud) + } +} + +// TestExpandEnvInMapPreservingVars_HashConsistencyWithWithout verifies that +// hashing the output is identical whether the secret var is set or not, +// which is the exact invariant needed for desiredStateHash. +func TestExpandEnvInMapPreservingVars_HashConsistencyWithWithout(t *testing.T) { + in := map[string]any{ + "user_data": "POSTGRES_PASSWORD: '${STAGING_PG_PASSWORD}'", + } + + // Run without the var in env. + t.Setenv("STAGING_PG_PASSWORD", "") + outUnset := ExpandEnvInMapPreservingVars(in, nil, []string{"STAGING_PG_PASSWORD"}) + + // Run with the var set to a real value. + t.Setenv("STAGING_PG_PASSWORD", "deadbeef1234567890abcdef") + outSet := ExpandEnvInMapPreservingVars(in, nil, []string{"STAGING_PG_PASSWORD"}) + + if outUnset["user_data"] != outSet["user_data"] { + t.Errorf("hash inconsistency: unset produced %q, set produced %q", + outUnset["user_data"], outSet["user_data"]) + } + want := "POSTGRES_PASSWORD: '${STAGING_PG_PASSWORD}'" + if outSet["user_data"] != want { + t.Errorf("user_data: got %q, want %q", outSet["user_data"], want) + } +} + +func TestExpandEnvInMapPreservingVars_NonSecretVarStillExpanded(t *testing.T) { + t.Setenv("IMAGE_TAG", "v1.2.3") + t.Setenv("SECRET_KEY", "s3cret") + + in := map[string]any{ + "image": "${IMAGE_TAG}", + "config": "${SECRET_KEY}", + } + out := ExpandEnvInMapPreservingVars(in, nil, []string{"SECRET_KEY"}) + + if got := out["image"]; got != "v1.2.3" { + t.Errorf("image: got %q, want v1.2.3", got) + } + if got := out["config"]; got != "${SECRET_KEY}" { + t.Errorf("config: got %q, want ${SECRET_KEY} (secret var preserved)", got) + } +} + +func TestExpandEnvInMapPreservingVars_PreserveKeysTakePriority(t *testing.T) { + t.Setenv("MY_SECRET", "actual-secret") + t.Setenv("OTHER_VAR", "other-value") + + in := map[string]any{ + // env_vars is in preserveKeys — entire submap is deep-copied, no expansion. + "env_vars": map[string]any{ + "TOKEN": "${MY_SECRET}", + }, + // user_data is NOT in preserveKeys — secret var is preserved, other vars expand. + "user_data": "secret=${MY_SECRET} region=${OTHER_VAR}", + } + out := ExpandEnvInMapPreservingVars(in, + []string{"env_vars"}, + []string{"MY_SECRET"}, + ) + + // env_vars submap: fully copied as-is (preserveKeys wins). + ev := out["env_vars"].(map[string]any) + if ev["TOKEN"] != "${MY_SECRET}" { + t.Errorf("env_vars.TOKEN: got %q, want literal ${MY_SECRET}", ev["TOKEN"]) + } + + // user_data: MY_SECRET preserved, OTHER_VAR expanded. + ud, _ := out["user_data"].(string) + if !strings.Contains(ud, "${MY_SECRET}") { + t.Errorf("user_data: want ${MY_SECRET} preserved, got %q", ud) + } + if !strings.Contains(ud, "other-value") { + t.Errorf("user_data: want OTHER_VAR expanded to other-value, got %q", ud) + } +} + +func TestExpandEnvInMapPreservingVars_EmptyVarListBehavesLikePreservingKeys(t *testing.T) { + t.Setenv("SOME_VAR", "expanded") + in := map[string]any{"k": "${SOME_VAR}"} + outNew := ExpandEnvInMapPreservingVars(in, nil, nil) + outOld := ExpandEnvInMapPreservingKeys(in, nil) + if outNew["k"] != outOld["k"] { + t.Errorf("empty preserveVarNames: ExpandEnvInMapPreservingVars(%q) = %q, ExpandEnvInMapPreservingKeys(%q) = %q", + in["k"], outNew["k"], in["k"], outOld["k"]) + } +} + +func TestExpandEnvInMapPreservingVars_SecretVarInNestedSlice(t *testing.T) { + t.Setenv("PG_PASSWORD", "should-be-preserved") + in := map[string]any{ + "services": []any{ + map[string]any{ + "name": "db", + "user_data": "PASSWORD=${PG_PASSWORD}", + }, + }, + } + out := ExpandEnvInMapPreservingVars(in, nil, []string{"PG_PASSWORD"}) + svc := out["services"].([]any)[0].(map[string]any) + if got := svc["user_data"]; got != "PASSWORD=${PG_PASSWORD}" { + t.Errorf("services[0].user_data: got %q, want literal reference", got) + } +} From f55b82f677d2a34dbd80843c602171a779d4f40f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 6 May 2026 13:53:16 +0000 Subject: [PATCH 3/4] fix: use t.Setenv consistently to avoid env cleanup ordering issue in test Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/2787b760-c8b6-4eed-9e60-c87107dade39 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- cmd/wfctl/infra_bootstrap.go | 2 +- cmd/wfctl/infra_plan_env_vars_preserve_test.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/wfctl/infra_bootstrap.go b/cmd/wfctl/infra_bootstrap.go index 86db8b301..5524b2c6d 100644 --- a/cmd/wfctl/infra_bootstrap.go +++ b/cmd/wfctl/infra_bootstrap.go @@ -429,7 +429,7 @@ func bootstrapSecrets(ctx context.Context, provider secrets.Provider, cfg *Secre if forceRotate[gen.Key] { deleteKey := gen.Key if delErr := provider.Delete(ctx, deleteKey); delErr != nil { - // Log and continue regardless of the error kind — both absent/unsupported + // Log and continue regardless of the error kind — both absent/unsupported // secrets and unexpected errors should not block the rotation flow. fmt.Fprintf(os.Stderr, "warn: rotate-pre-delete %s: %v (continuing)\n", deleteKey, delErr) } diff --git a/cmd/wfctl/infra_plan_env_vars_preserve_test.go b/cmd/wfctl/infra_plan_env_vars_preserve_test.go index 0d7997426..e5b1a930c 100644 --- a/cmd/wfctl/infra_plan_env_vars_preserve_test.go +++ b/cmd/wfctl/infra_plan_env_vars_preserve_test.go @@ -95,8 +95,9 @@ func TestParseInfraResourceSpecs_PreservesSecretGenVarsInUserData(t *testing.T) t.Setenv("AUTH_TOKEN", "would-be-resolved-secret") t.Setenv("DATABASE_URL", "postgres://would-be-resolved") - // --- Plan-time: POSTGRES_PASSWORD is NOT set (as happens in CI) --- - os.Unsetenv("POSTGRES_PASSWORD") + // --- Plan-time: POSTGRES_PASSWORD is NOT set (as happens in CI). + // Use t.Setenv so that the original value is restored by t.Cleanup. + t.Setenv("POSTGRES_PASSWORD", "") specsAtPlan, err := parseInfraResourceSpecs("testdata/infra-with-env-var-refs.yaml") if err != nil { t.Fatalf("parseInfraResourceSpecs (plan): %v", err) From 9c3de5204c2be6c7d430a1aa755e11e74895a085 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 6 May 2026 14:00:39 +0000 Subject: [PATCH 4/4] test: add unbraced $VAR canonicalisation test for ExpandEnvInMapPreservingVars Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/b37c58c4-6c11-4a8b-8a39-cbfa685f1bb3 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- config/env_expand_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/config/env_expand_test.go b/config/env_expand_test.go index d280df9b1..603d407a1 100644 --- a/config/env_expand_test.go +++ b/config/env_expand_test.go @@ -376,6 +376,36 @@ func TestExpandEnvInMapPreservingVars_EmptyVarListBehavesLikePreservingKeys(t *t } } +// TestExpandEnvInMapPreservingVars_UnbracedSecretVarCanonicalised verifies that +// an unbraced $SECRET_VAR reference (no curly braces) in a preserved var is +// canonicalised to ${SECRET_VAR} rather than being expanded. +// Apply-time JIT resolution only supports the braced form, so canonicalisation +// is the correct and expected behaviour for unbraced references. +func TestExpandEnvInMapPreservingVars_UnbracedSecretVarCanonicalised(t *testing.T) { + t.Setenv("DB_PASSWORD", "supersecret") + t.Setenv("REGION", "us-east-1") + + in := map[string]any{ + // unbraced: $DB_PASSWORD — secret var, should be canonicalised to ${DB_PASSWORD} + "user_data": "PASSWORD=$DB_PASSWORD region=${REGION}", + } + out := ExpandEnvInMapPreservingVars(in, nil, []string{"DB_PASSWORD"}) + + ud, _ := out["user_data"].(string) + // Secret var must be preserved — canonicalised to braced form. + if !strings.Contains(ud, "${DB_PASSWORD}") { + t.Errorf("user_data: want ${DB_PASSWORD} (canonicalised), got: %q", ud) + } + // Non-secret var must still be expanded normally. + if !strings.Contains(ud, "us-east-1") { + t.Errorf("user_data: want REGION expanded to us-east-1, got: %q", ud) + } + // The raw unbraced form must not appear in the output. + if strings.Contains(ud, "$DB_PASSWORD") && !strings.Contains(ud, "${DB_PASSWORD}") { + t.Errorf("user_data: unbraced $DB_PASSWORD should have been canonicalised, got: %q", ud) + } +} + func TestExpandEnvInMapPreservingVars_SecretVarInNestedSlice(t *testing.T) { t.Setenv("PG_PASSWORD", "should-be-preserved") in := map[string]any{