Fix non-deterministic secret replacement in workflows with shared fallback expressions

pkg/workflow/secret_extraction.go

@@ -121,7 +121,13 @@ func ExtractEnvExpressionsFromMap(values map[string]string) map[string]string {
 // The backslash is used to escape the ${} for proper JSON rendering in Copilot configs
 func ReplaceSecretsWithEnvVars(value string, secrets map[string]string) string {
 	result := value
-	for varName, secretExpr := range secrets {
+	// Sort keys for deterministic output. When multiple secret names share the same
+	// expression (e.g. "${{ secrets.DD_APPLICATION_KEY || secrets.DD_APP_KEY }}" maps
+	// to both "DD_APPLICATION_KEY" and "DD_APP_KEY"), the alphabetically first key is
+	// processed first and its replacement wins; subsequent keys find the expression
+	// already replaced and are no-ops. This ensures stable lock-file output across runs.
+	for _, varName := range sortedMapKeys(secrets) {
+		secretExpr := secrets[varName]
 		// Replace ${{ secrets.VAR }} with \${VAR} (backslash-escaped for copilot JSON config)
 		result = strings.ReplaceAll(result, secretExpr, "\\${"+varName+"}")
 	}
@@ -137,7 +143,9 @@ func ReplaceSecretsWithEnvVars(value string, secrets map[string]string) string {
 func ReplaceSecretsWithBashVars(value string) string {
 	result := value
 	secrets := ExtractSecretsFromValue(value)
-	for varName, secretExpr := range secrets {
+	// Sort keys for deterministic output; see ReplaceSecretsWithEnvVars for rationale.
+	for _, varName := range sortedMapKeys(secrets) {
+		secretExpr := secrets[varName]
 		result = strings.ReplaceAll(result, secretExpr, "${"+varName+"}")
 	}
 	return result
@@ -321,15 +329,18 @@ func formatInputNameAsEnvVar(inputName string) string {
 func ReplaceTemplateExpressionsWithEnvVars(value string) string {
 	result := value
 
-	// Extract and replace secrets
+	// Extract and replace secrets — sort keys for deterministic output; see
+	// ReplaceSecretsWithEnvVars for rationale.
 	secrets := ExtractSecretsFromValue(value)
-	for varName, secretExpr := range secrets {
+	for _, varName := range sortedMapKeys(secrets) {
+		secretExpr := secrets[varName]
 		result = strings.ReplaceAll(result, secretExpr, "\\${"+varName+"}")
 	}
 
-	// Extract and replace env vars
+	// Extract and replace env vars — sort keys for deterministic output.
 	envVars := ExtractEnvExpressionsFromValue(value)
-	for varName, envExpr := range envVars {
+	for _, varName := range sortedMapKeys(envVars) {
+		envExpr := envVars[varName]
 		result = strings.ReplaceAll(result, envExpr, "\\${"+varName+"}")
 	}
 

pkg/workflow/secret_extraction_test.go

@@ -333,6 +333,71 @@ func TestSharedReplaceSecretsWithEnvVars(t *testing.T) {
 	}
 }
 
+// TestReplaceSecretsWithEnvVars_FallbackExpressionDeterminism verifies that when a single
+// GitHub Actions expression references two secrets (e.g. a fallback pattern like
+// "${{ secrets.DD_APPLICATION_KEY || secrets.DD_APP_KEY }}"), the replacement is
+// deterministic across repeated calls. Previously, Go map iteration produced either
+// "\${DD_APP_KEY}" or "\${DD_APPLICATION_KEY}" non-deterministically, causing
+// smoke-otel-backends.lock.yml to differ between compiler runs.
+func TestReplaceSecretsWithEnvVars_FallbackExpressionDeterminism(t *testing.T) {
+	// Both secrets map to the same expression — this is what ExtractSecretsFromMap
+	// produces for "${{ secrets.DD_APPLICATION_KEY || secrets.DD_APP_KEY }}".
+	sharedExpr := "${{ secrets.DD_APPLICATION_KEY || secrets.DD_APP_KEY }}"
+	secrets := map[string]string{
+		"DD_APPLICATION_KEY": sharedExpr,
+		"DD_APP_KEY":         sharedExpr,
+	}
+
+	// Run many times to surface any non-determinism from Go map iteration.
+	const runs = 50
+	var first string
+	for i := range runs {
+		got := ReplaceSecretsWithEnvVars(sharedExpr, secrets)
+		if i == 0 {
+			first = got
+			continue
+		}
+		if got != first {
+			t.Errorf("non-deterministic output: run 0 produced %q, run %d produced %q", first, i, got)
+		}
+	}
+
+	// The alphabetically first key ("DD_APPLICATION_KEY") should win because it is processed
+	// first in sorted order and performs the replacement; the second key finds nothing
+	// to replace. Note: '_' (ASCII 95) > 'L' (ASCII 76), so "DD_APPLICATION_KEY" sorts
+	// before "DD_APP_KEY" (comparison diverges at position 6: 'L' < '_').
+	want := "\\${DD_APPLICATION_KEY}"
+	if first != want {
+		t.Errorf("ReplaceSecretsWithEnvVars(%q, secrets) = %q, want %q", sharedExpr, first, want)
+	}
+}
+
+// TestReplaceSecretsWithBashVars_FallbackExpressionDeterminism verifies that
+// ReplaceSecretsWithBashVars produces deterministic output for fallback expressions
+// with two secret references. Mirrors TestReplaceSecretsWithEnvVars_FallbackExpressionDeterminism.
+func TestReplaceSecretsWithBashVars_FallbackExpressionDeterminism(t *testing.T) {
+	value := "${{ secrets.DD_APPLICATION_KEY || secrets.DD_APP_KEY }}"
+
+	const runs = 50
+	var first string
+	for i := range runs {
+		got := ReplaceSecretsWithBashVars(value)
+		if i == 0 {
+			first = got
+			continue
+		}
+		if got != first {
+			t.Errorf("non-deterministic output: run 0 produced %q, run %d produced %q", first, i, got)
+		}
+	}
+
+	// "DD_APPLICATION_KEY" sorts before "DD_APP_KEY" ('L' < '_'), so it wins.
+	want := "${DD_APPLICATION_KEY}"
+	if first != want {
+		t.Errorf("ReplaceSecretsWithBashVars(%q) = %q, want %q", value, first, want)
+	}
+}
+
 // TestSharedExtractSecretsFromValueEdgeCases tests edge cases for the shared ExtractSecretsFromValue utility function
 func TestSharedExtractSecretsFromValueEdgeCases(t *testing.T) {
 	tests := []struct {