Consolidate duplicate secret extraction logic into shared utilities

pkg/cli/secrets.go

@@ -5,20 +5,15 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"regexp"
 	"strings"
 
 	"github.com/githubnext/gh-aw/pkg/logger"
 	"github.com/githubnext/gh-aw/pkg/parser"
+	"github.com/githubnext/gh-aw/pkg/workflow"
 )
 
 var secretsLog = logger.New("cli:secrets")
 
-// Pre-compiled regexes for secret extraction (performance optimization)
-var (
-	secretPattern = regexp.MustCompile(`\$\{\{\s*secrets\.([A-Z_][A-Z0-9_]*)\s*(?:\|\|.*?)?\s*\}\}`)
-)
-
 // SecretInfo contains information about a required secret
 type SecretInfo struct {
 	Name      string // Secret name (e.g., "DD_API_KEY")
@@ -64,19 +59,6 @@ func checkSecretExists(secretName string) (bool, error) {
 	return false, nil
 }
 
-// extractSecretName extracts the secret name from a GitHub Actions expression
-// Examples: "${{ secrets.DD_API_KEY }}" -> "DD_API_KEY"
-//
-//	"${{ secrets.DD_SITE || 'datadoghq.com' }}" -> "DD_SITE"
-func extractSecretName(value string) string {
-	// Match pattern: ${{ secrets.SECRET_NAME }} or ${{ secrets.SECRET_NAME || 'default' }}
-	matches := secretPattern.FindStringSubmatch(value)
-	if len(matches) >= 2 {
-		return matches[1]
-	}
-	return ""
-}
-
 // extractSecretsFromConfig extracts all required secrets from an MCP server config
 func extractSecretsFromConfig(config parser.MCPServerConfig) []SecretInfo {
 	secretsLog.Printf("Extracting secrets from MCP config: command=%s", config.Command)
@@ -85,7 +67,7 @@ func extractSecretsFromConfig(config parser.MCPServerConfig) []SecretInfo {
 
 	// Extract from HTTP headers
 	for key, value := range config.Headers {
-		secretName := extractSecretName(value)
+		secretName := workflow.ExtractSecretName(value)
 		if secretName != "" && !seen[secretName] {
 			secrets = append(secrets, SecretInfo{
 				Name:   secretName,
@@ -97,7 +79,7 @@ func extractSecretsFromConfig(config parser.MCPServerConfig) []SecretInfo {
 
 	// Extract from environment variables
 	for key, value := range config.Env {
-		secretName := extractSecretName(value)
+		secretName := workflow.ExtractSecretName(value)
 		if secretName != "" && !seen[secretName] {
 			secrets = append(secrets, SecretInfo{
 				Name:   secretName,

pkg/cli/secrets_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/githubnext/gh-aw/pkg/parser"
+	"github.com/githubnext/gh-aw/pkg/workflow"
 )
 
 func TestExtractSecretName(t *testing.T) {
@@ -47,7 +48,7 @@ func TestExtractSecretName(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := extractSecretName(tt.value)
+			result := workflow.ExtractSecretName(tt.value)
 			if result != tt.expected {
 				t.Errorf("Expected %q, got %q", tt.expected, result)
 			}

pkg/workflow/mcp-config.go

@@ -300,7 +300,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma
 	// Extract secrets from headers for HTTP MCP tools (copilot engine only)
 	var headerSecrets map[string]string
 	if mcpConfig.Type == "http" && renderer.RequiresCopilotFields {
-		headerSecrets = extractSecretsFromHeaders(mcpConfig.Headers)
+		headerSecrets = ExtractSecretsFromMap(mcpConfig.Headers)
 	}
 
 	// Determine properties based on type
@@ -545,7 +545,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma
 				// Replace secret expressions with env var references for copilot
 				headerValue := mcpConfig.Headers[headerKey]
 				if renderer.RequiresCopilotFields && len(headerSecrets) > 0 {
-					headerValue = replaceSecretsWithEnvVars(headerValue, headerSecrets)
+					headerValue = ReplaceSecretsWithEnvVars(headerValue, headerSecrets)
 				}
 
 				fmt.Fprintf(yaml, "%s  \"%s\": \"%s\"%s\n", renderer.IndentLevel, headerKey, headerValue, headerComma)
@@ -650,82 +650,6 @@ func (m MapToolConfig) GetAny(key string) (any, bool) {
 	return value, exists
 }
 
-// extractSecretsFromValue extracts GitHub Actions secret expressions from a string value
-// Returns a map of environment variable names to their secret expressions
-// Example: "${{ secrets.DD_API_KEY }}" -> {"DD_API_KEY": "${{ secrets.DD_API_KEY }}"}
-// Example: "${{ secrets.DD_SITE || 'datadoghq.com' }}" -> {"DD_SITE": "${{ secrets.DD_SITE || 'datadoghq.com' }}"}
-func extractSecretsFromValue(value string) map[string]string {
-	secrets := make(map[string]string)
-
-	// Pattern to match ${{ secrets.VARIABLE_NAME }} or ${{ secrets.VARIABLE_NAME || 'default' }}
-	// We need to extract the variable name and the full expression
-	start := 0
-	for {
-		// Find the start of an expression
-		startIdx := strings.Index(value[start:], "${{ secrets.")
-		if startIdx == -1 {
-			break
-		}
-		startIdx += start
-
-		// Find the end of the expression
-		endIdx := strings.Index(value[startIdx:], "}}")
-		if endIdx == -1 {
-			break
-		}
-		endIdx += startIdx + 2 // Include the closing }}
-
-		// Extract the full expression
-		fullExpr := value[startIdx:endIdx]
-
-		// Extract the variable name from "secrets.VARIABLE_NAME" or "secrets.VARIABLE_NAME ||"
-		secretsPart := strings.TrimPrefix(fullExpr, "${{ secrets.")
-		secretsPart = strings.TrimSuffix(secretsPart, "}}")
-		secretsPart = strings.TrimSpace(secretsPart)
-
-		// Find the variable name (everything before space, ||, or end)
-		varName := secretsPart
-		if spaceIdx := strings.IndexAny(varName, " |"); spaceIdx != -1 {
-			varName = varName[:spaceIdx]
-		}
-
-		// Store the variable name and full expression
-		if varName != "" {
-			secrets[varName] = fullExpr
-		}
-
-		start = endIdx
-	}
-
-	return secrets
-}
-
-// extractSecretsFromHeaders extracts all secrets from HTTP MCP headers
-// Returns a map of environment variable names to their secret expressions
-func extractSecretsFromHeaders(headers map[string]string) map[string]string {
-	allSecrets := make(map[string]string)
-
-	for _, headerValue := range headers {
-		secrets := extractSecretsFromValue(headerValue)
-		for varName, expr := range secrets {
-			allSecrets[varName] = expr
-		}
-	}
-
-	return allSecrets
-}
-
-// replaceSecretsWithEnvVars replaces secret expressions in a value with environment variable references
-// Example: "${{ secrets.DD_API_KEY }}" -> "\${DD_API_KEY}"
-func replaceSecretsWithEnvVars(value string, secrets map[string]string) string {
-	result := value
-	for varName, secretExpr := range secrets {
-		// Replace ${{ secrets.VAR }} with \${VAR} (backslash-escaped for copilot JSON config)
-		result = strings.ReplaceAll(result, secretExpr, "\\${"+varName+"}")
-	}
-	return result
-}
-
 // collectHTTPMCPHeaderSecrets collects all secrets from HTTP MCP tool headers
 // Returns a map of environment variable names to their secret expressions
 func collectHTTPMCPHeaderSecrets(tools map[string]any) map[string]string {
@@ -737,7 +661,7 @@ func collectHTTPMCPHeaderSecrets(tools map[string]any) map[string]string {
 			if hasMcp, mcpType := hasMCPConfig(toolConfig); hasMcp && mcpType == "http" {
 				// Extract MCP config to get headers
 				if mcpConfig, err := getMCPConfig(toolConfig, toolName); err == nil {
-					secrets := extractSecretsFromHeaders(mcpConfig.Headers)
+					secrets := ExtractSecretsFromMap(mcpConfig.Headers)
 					for varName, expr := range secrets {
 						allSecrets[varName] = expr
 					}

pkg/workflow/mcp_http_headers_test.go

@@ -47,7 +47,7 @@ func TestExtractSecretsFromValue(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := extractSecretsFromValue(tt.value)
+			result := ExtractSecretsFromValue(tt.value)
 
 			if len(result) != len(tt.expected) {
 				t.Errorf("Expected %d secrets, got %d", len(tt.expected), len(result))
@@ -72,7 +72,7 @@ func TestExtractSecretsFromHeaders(t *testing.T) {
 		"Static":             "no-secrets-here",
 	}
 
-	result := extractSecretsFromHeaders(headers)
+	result := ExtractSecretsFromMap(headers)
 
 	expected := map[string]string{
 		"DD_API_KEY":         "${{ secrets.DD_API_KEY }}",
@@ -134,7 +134,7 @@ func TestReplaceSecretsWithEnvVars(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := replaceSecretsWithEnvVars(tt.value, tt.secrets)
+			result := ReplaceSecretsWithEnvVars(tt.value, tt.secrets)
 			if result != tt.expected {
 				t.Errorf("Expected %q, got %q", tt.expected, result)
 			}

pkg/workflow/secret_extraction.go

@@ -0,0 +1,112 @@
+package workflow
+
+import (
+	"regexp"
+	"strings"
+)
+
+// Pre-compiled regex for secret extraction (performance optimization)
+// Matches: ${{ secrets.SECRET_NAME }} or ${{ secrets.SECRET_NAME || 'default' }}
+var secretExprPattern = regexp.MustCompile(`\$\{\{\s*secrets\.([A-Z_][A-Z0-9_]*)\s*(?:\|\|.*?)?\s*\}\}`)
+
+// SecretExpression represents a parsed secret expression
+type SecretExpression struct {
+	VarName  string // The secret variable name (e.g., "DD_API_KEY")
+	FullExpr string // The full expression (e.g., "${{ secrets.DD_API_KEY }}")
+}
+
+// ExtractSecretName extracts just the secret name from a GitHub Actions expression
+// Examples:
+//   - "${{ secrets.DD_API_KEY }}" -> "DD_API_KEY"
+//   - "${{ secrets.DD_SITE || 'datadoghq.com' }}" -> "DD_SITE"
+//   - "plain value" -> ""
+func ExtractSecretName(value string) string {
+	matches := secretExprPattern.FindStringSubmatch(value)
+	if len(matches) >= 2 {
+		return matches[1]
+	}
+	return ""
+}
+
+// ExtractSecretsFromValue extracts all GitHub Actions secret expressions from a string value
+// Returns a map of environment variable names to their full secret expressions
+// Examples:
+//   - "${{ secrets.DD_API_KEY }}" -> {"DD_API_KEY": "${{ secrets.DD_API_KEY }}"}
+//   - "${{ secrets.DD_SITE || 'datadoghq.com' }}" -> {"DD_SITE": "${{ secrets.DD_SITE || 'datadoghq.com' }}"}
+//   - "Bearer ${{ secrets.TOKEN }}" -> {"TOKEN": "${{ secrets.TOKEN }}"}
+func ExtractSecretsFromValue(value string) map[string]string {
+	secrets := make(map[string]string)
+
+	// Pattern to match ${{ secrets.VARIABLE_NAME }} or ${{ secrets.VARIABLE_NAME || 'default' }}
+	// We need to extract the variable name and the full expression
+	start := 0
+	for {
+		// Find the start of an expression
+		startIdx := strings.Index(value[start:], "${{ secrets.")
+		if startIdx == -1 {
+			break
+		}
+		startIdx += start
+
+		// Find the end of the expression
+		endIdx := strings.Index(value[startIdx:], "}}")
+		if endIdx == -1 {
+			break
+		}
+		endIdx += startIdx + 2 // Include the closing }}
+
+		// Extract the full expression
+		fullExpr := value[startIdx:endIdx]
+
+		// Extract the variable name from "secrets.VARIABLE_NAME" or "secrets.VARIABLE_NAME ||"
+		secretsPart := strings.TrimPrefix(fullExpr, "${{ secrets.")
+		secretsPart = strings.TrimSuffix(secretsPart, "}}")
+		secretsPart = strings.TrimSpace(secretsPart)
+
+		// Find the variable name (everything before space, ||, or end)
+		varName := secretsPart
+		if spaceIdx := strings.IndexAny(varName, " |"); spaceIdx != -1 {
+			varName = varName[:spaceIdx]
+		}
+
+		// Store the variable name and full expression
+		if varName != "" {
+			secrets[varName] = fullExpr
+		}
+
+		start = endIdx
+	}
+
+	return secrets
+}
+
+// ExtractSecretsFromMap extracts all secrets from a map of string values
+// Returns a map of environment variable names to their full secret expressions
+// Example:
+//
+//	Input: {"DD_API_KEY": "${{ secrets.DD_API_KEY }}", "DD_SITE": "${{ secrets.DD_SITE || 'default' }}"}
+//	Output: {"DD_API_KEY": "${{ secrets.DD_API_KEY }}", "DD_SITE": "${{ secrets.DD_SITE || 'default' }}"}
+func ExtractSecretsFromMap(values map[string]string) map[string]string {
+	allSecrets := make(map[string]string)
+
+	for _, value := range values {
+		secrets := ExtractSecretsFromValue(value)
+		for varName, expr := range secrets {
+			allSecrets[varName] = expr
+		}
+	}
+
+	return allSecrets
+}
+
+// ReplaceSecretsWithEnvVars replaces secret expressions in a value with environment variable references
+// Example: "${{ secrets.DD_API_KEY }}" -> "\${DD_API_KEY}"
+// 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 {
+		// Replace ${{ secrets.VAR }} with \${VAR} (backslash-escaped for copilot JSON config)
+		result = strings.ReplaceAll(result, secretExpr, "\\${"+varName+"}")
+	}
+	return result
+}