Skip to content

Commit d156ba2

Browse files
CopilotCopilot
authored
Consolidate duplicate secret extraction logic into shared utilities (#3752)
1 parent 606a62d commit d156ba2

6 files changed

Lines changed: 458 additions & 104 deletions

File tree

‎pkg/cli/secrets.go‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,15 @@ import (
55
"fmt"
66
"os"
77
"os/exec"
8-
"regexp"
98
"strings"
109

1110
"github.com/githubnext/gh-aw/pkg/logger"
1211
"github.com/githubnext/gh-aw/pkg/parser"
12+
"github.com/githubnext/gh-aw/pkg/workflow"
1313
)
1414

1515
var secretsLog = logger.New("cli:secrets")
1616

17-
// Pre-compiled regexes for secret extraction (performance optimization)
18-
var (
19-
secretPattern = regexp.MustCompile(`\$\{\{\s*secrets\.([A-Z_][A-Z0-9_]*)\s*(?:\|\|.*?)?\s*\}\}`)
20-
)
21-
2217
// SecretInfo contains information about a required secret
2318
type SecretInfo struct {
2419
Name string // Secret name (e.g., "DD_API_KEY")
@@ -64,19 +59,6 @@ func checkSecretExists(secretName string) (bool, error) {
6459
return false, nil
6560
}
6661

67-
// extractSecretName extracts the secret name from a GitHub Actions expression
68-
// Examples: "${{ secrets.DD_API_KEY }}" -> "DD_API_KEY"
69-
//
70-
// "${{ secrets.DD_SITE || 'datadoghq.com' }}" -> "DD_SITE"
71-
func extractSecretName(value string) string {
72-
// Match pattern: ${{ secrets.SECRET_NAME }} or ${{ secrets.SECRET_NAME || 'default' }}
73-
matches := secretPattern.FindStringSubmatch(value)
74-
if len(matches) >= 2 {
75-
return matches[1]
76-
}
77-
return ""
78-
}
79-
8062
// extractSecretsFromConfig extracts all required secrets from an MCP server config
8163
func extractSecretsFromConfig(config parser.MCPServerConfig) []SecretInfo {
8264
secretsLog.Printf("Extracting secrets from MCP config: command=%s", config.Command)
@@ -85,7 +67,7 @@ func extractSecretsFromConfig(config parser.MCPServerConfig) []SecretInfo {
8567

8668
// Extract from HTTP headers
8769
for key, value := range config.Headers {
88-
secretName := extractSecretName(value)
70+
secretName := workflow.ExtractSecretName(value)
8971
if secretName != "" && !seen[secretName] {
9072
secrets = append(secrets, SecretInfo{
9173
Name: secretName,
@@ -97,7 +79,7 @@ func extractSecretsFromConfig(config parser.MCPServerConfig) []SecretInfo {
9779

9880
// Extract from environment variables
9981
for key, value := range config.Env {
100-
secretName := extractSecretName(value)
82+
secretName := workflow.ExtractSecretName(value)
10183
if secretName != "" && !seen[secretName] {
10284
secrets = append(secrets, SecretInfo{
10385
Name: secretName,

‎pkg/cli/secrets_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/githubnext/gh-aw/pkg/parser"
8+
"github.com/githubnext/gh-aw/pkg/workflow"
89
)
910

1011
func TestExtractSecretName(t *testing.T) {
@@ -47,7 +48,7 @@ func TestExtractSecretName(t *testing.T) {
4748

4849
for _, tt := range tests {
4950
t.Run(tt.name, func(t *testing.T) {
50-
result := extractSecretName(tt.value)
51+
result := workflow.ExtractSecretName(tt.value)
5152
if result != tt.expected {
5253
t.Errorf("Expected %q, got %q", tt.expected, result)
5354
}

‎pkg/workflow/mcp-config.go‎

Lines changed: 3 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma
300300
// Extract secrets from headers for HTTP MCP tools (copilot engine only)
301301
var headerSecrets map[string]string
302302
if mcpConfig.Type == "http" && renderer.RequiresCopilotFields {
303-
headerSecrets = extractSecretsFromHeaders(mcpConfig.Headers)
303+
headerSecrets = ExtractSecretsFromMap(mcpConfig.Headers)
304304
}
305305

306306
// Determine properties based on type
@@ -545,7 +545,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma
545545
// Replace secret expressions with env var references for copilot
546546
headerValue := mcpConfig.Headers[headerKey]
547547
if renderer.RequiresCopilotFields && len(headerSecrets) > 0 {
548-
headerValue = replaceSecretsWithEnvVars(headerValue, headerSecrets)
548+
headerValue = ReplaceSecretsWithEnvVars(headerValue, headerSecrets)
549549
}
550550

551551
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) {
650650
return value, exists
651651
}
652652

653-
// extractSecretsFromValue extracts GitHub Actions secret expressions from a string value
654-
// Returns a map of environment variable names to their secret expressions
655-
// Example: "${{ secrets.DD_API_KEY }}" -> {"DD_API_KEY": "${{ secrets.DD_API_KEY }}"}
656-
// Example: "${{ secrets.DD_SITE || 'datadoghq.com' }}" -> {"DD_SITE": "${{ secrets.DD_SITE || 'datadoghq.com' }}"}
657-
func extractSecretsFromValue(value string) map[string]string {
658-
secrets := make(map[string]string)
659-
660-
// Pattern to match ${{ secrets.VARIABLE_NAME }} or ${{ secrets.VARIABLE_NAME || 'default' }}
661-
// We need to extract the variable name and the full expression
662-
start := 0
663-
for {
664-
// Find the start of an expression
665-
startIdx := strings.Index(value[start:], "${{ secrets.")
666-
if startIdx == -1 {
667-
break
668-
}
669-
startIdx += start
670-
671-
// Find the end of the expression
672-
endIdx := strings.Index(value[startIdx:], "}}")
673-
if endIdx == -1 {
674-
break
675-
}
676-
endIdx += startIdx + 2 // Include the closing }}
677-
678-
// Extract the full expression
679-
fullExpr := value[startIdx:endIdx]
680-
681-
// Extract the variable name from "secrets.VARIABLE_NAME" or "secrets.VARIABLE_NAME ||"
682-
secretsPart := strings.TrimPrefix(fullExpr, "${{ secrets.")
683-
secretsPart = strings.TrimSuffix(secretsPart, "}}")
684-
secretsPart = strings.TrimSpace(secretsPart)
685-
686-
// Find the variable name (everything before space, ||, or end)
687-
varName := secretsPart
688-
if spaceIdx := strings.IndexAny(varName, " |"); spaceIdx != -1 {
689-
varName = varName[:spaceIdx]
690-
}
691-
692-
// Store the variable name and full expression
693-
if varName != "" {
694-
secrets[varName] = fullExpr
695-
}
696-
697-
start = endIdx
698-
}
699-
700-
return secrets
701-
}
702-
703-
// extractSecretsFromHeaders extracts all secrets from HTTP MCP headers
704-
// Returns a map of environment variable names to their secret expressions
705-
func extractSecretsFromHeaders(headers map[string]string) map[string]string {
706-
allSecrets := make(map[string]string)
707-
708-
for _, headerValue := range headers {
709-
secrets := extractSecretsFromValue(headerValue)
710-
for varName, expr := range secrets {
711-
allSecrets[varName] = expr
712-
}
713-
}
714-
715-
return allSecrets
716-
}
717-
718-
// replaceSecretsWithEnvVars replaces secret expressions in a value with environment variable references
719-
// Example: "${{ secrets.DD_API_KEY }}" -> "\${DD_API_KEY}"
720-
func replaceSecretsWithEnvVars(value string, secrets map[string]string) string {
721-
result := value
722-
for varName, secretExpr := range secrets {
723-
// Replace ${{ secrets.VAR }} with \${VAR} (backslash-escaped for copilot JSON config)
724-
result = strings.ReplaceAll(result, secretExpr, "\\${"+varName+"}")
725-
}
726-
return result
727-
}
728-
729653
// collectHTTPMCPHeaderSecrets collects all secrets from HTTP MCP tool headers
730654
// Returns a map of environment variable names to their secret expressions
731655
func collectHTTPMCPHeaderSecrets(tools map[string]any) map[string]string {
@@ -737,7 +661,7 @@ func collectHTTPMCPHeaderSecrets(tools map[string]any) map[string]string {
737661
if hasMcp, mcpType := hasMCPConfig(toolConfig); hasMcp && mcpType == "http" {
738662
// Extract MCP config to get headers
739663
if mcpConfig, err := getMCPConfig(toolConfig, toolName); err == nil {
740-
secrets := extractSecretsFromHeaders(mcpConfig.Headers)
664+
secrets := ExtractSecretsFromMap(mcpConfig.Headers)
741665
for varName, expr := range secrets {
742666
allSecrets[varName] = expr
743667
}

‎pkg/workflow/mcp_http_headers_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func TestExtractSecretsFromValue(t *testing.T) {
4747

4848
for _, tt := range tests {
4949
t.Run(tt.name, func(t *testing.T) {
50-
result := extractSecretsFromValue(tt.value)
50+
result := ExtractSecretsFromValue(tt.value)
5151

5252
if len(result) != len(tt.expected) {
5353
t.Errorf("Expected %d secrets, got %d", len(tt.expected), len(result))
@@ -72,7 +72,7 @@ func TestExtractSecretsFromHeaders(t *testing.T) {
7272
"Static": "no-secrets-here",
7373
}
7474

75-
result := extractSecretsFromHeaders(headers)
75+
result := ExtractSecretsFromMap(headers)
7676

7777
expected := map[string]string{
7878
"DD_API_KEY": "${{ secrets.DD_API_KEY }}",
@@ -134,7 +134,7 @@ func TestReplaceSecretsWithEnvVars(t *testing.T) {
134134

135135
for _, tt := range tests {
136136
t.Run(tt.name, func(t *testing.T) {
137-
result := replaceSecretsWithEnvVars(tt.value, tt.secrets)
137+
result := ReplaceSecretsWithEnvVars(tt.value, tt.secrets)
138138
if result != tt.expected {
139139
t.Errorf("Expected %q, got %q", tt.expected, result)
140140
}

‎pkg/workflow/secret_extraction.go‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package workflow
2+
3+
import (
4+
"regexp"
5+
"strings"
6+
)
7+
8+
// Pre-compiled regex for secret extraction (performance optimization)
9+
// Matches: ${{ secrets.SECRET_NAME }} or ${{ secrets.SECRET_NAME || 'default' }}
10+
var secretExprPattern = regexp.MustCompile(`\$\{\{\s*secrets\.([A-Z_][A-Z0-9_]*)\s*(?:\|\|.*?)?\s*\}\}`)
11+
12+
// SecretExpression represents a parsed secret expression
13+
type SecretExpression struct {
14+
VarName string // The secret variable name (e.g., "DD_API_KEY")
15+
FullExpr string // The full expression (e.g., "${{ secrets.DD_API_KEY }}")
16+
}
17+
18+
// ExtractSecretName extracts just the secret name from a GitHub Actions expression
19+
// Examples:
20+
// - "${{ secrets.DD_API_KEY }}" -> "DD_API_KEY"
21+
// - "${{ secrets.DD_SITE || 'datadoghq.com' }}" -> "DD_SITE"
22+
// - "plain value" -> ""
23+
func ExtractSecretName(value string) string {
24+
matches := secretExprPattern.FindStringSubmatch(value)
25+
if len(matches) >= 2 {
26+
return matches[1]
27+
}
28+
return ""
29+
}
30+
31+
// ExtractSecretsFromValue extracts all GitHub Actions secret expressions from a string value
32+
// Returns a map of environment variable names to their full secret expressions
33+
// Examples:
34+
// - "${{ secrets.DD_API_KEY }}" -> {"DD_API_KEY": "${{ secrets.DD_API_KEY }}"}
35+
// - "${{ secrets.DD_SITE || 'datadoghq.com' }}" -> {"DD_SITE": "${{ secrets.DD_SITE || 'datadoghq.com' }}"}
36+
// - "Bearer ${{ secrets.TOKEN }}" -> {"TOKEN": "${{ secrets.TOKEN }}"}
37+
func ExtractSecretsFromValue(value string) map[string]string {
38+
secrets := make(map[string]string)
39+
40+
// Pattern to match ${{ secrets.VARIABLE_NAME }} or ${{ secrets.VARIABLE_NAME || 'default' }}
41+
// We need to extract the variable name and the full expression
42+
start := 0
43+
for {
44+
// Find the start of an expression
45+
startIdx := strings.Index(value[start:], "${{ secrets.")
46+
if startIdx == -1 {
47+
break
48+
}
49+
startIdx += start
50+
51+
// Find the end of the expression
52+
endIdx := strings.Index(value[startIdx:], "}}")
53+
if endIdx == -1 {
54+
break
55+
}
56+
endIdx += startIdx + 2 // Include the closing }}
57+
58+
// Extract the full expression
59+
fullExpr := value[startIdx:endIdx]
60+
61+
// Extract the variable name from "secrets.VARIABLE_NAME" or "secrets.VARIABLE_NAME ||"
62+
secretsPart := strings.TrimPrefix(fullExpr, "${{ secrets.")
63+
secretsPart = strings.TrimSuffix(secretsPart, "}}")
64+
secretsPart = strings.TrimSpace(secretsPart)
65+
66+
// Find the variable name (everything before space, ||, or end)
67+
varName := secretsPart
68+
if spaceIdx := strings.IndexAny(varName, " |"); spaceIdx != -1 {
69+
varName = varName[:spaceIdx]
70+
}
71+
72+
// Store the variable name and full expression
73+
if varName != "" {
74+
secrets[varName] = fullExpr
75+
}
76+
77+
start = endIdx
78+
}
79+
80+
return secrets
81+
}
82+
83+
// ExtractSecretsFromMap extracts all secrets from a map of string values
84+
// Returns a map of environment variable names to their full secret expressions
85+
// Example:
86+
//
87+
// Input: {"DD_API_KEY": "${{ secrets.DD_API_KEY }}", "DD_SITE": "${{ secrets.DD_SITE || 'default' }}"}
88+
// Output: {"DD_API_KEY": "${{ secrets.DD_API_KEY }}", "DD_SITE": "${{ secrets.DD_SITE || 'default' }}"}
89+
func ExtractSecretsFromMap(values map[string]string) map[string]string {
90+
allSecrets := make(map[string]string)
91+
92+
for _, value := range values {
93+
secrets := ExtractSecretsFromValue(value)
94+
for varName, expr := range secrets {
95+
allSecrets[varName] = expr
96+
}
97+
}
98+
99+
return allSecrets
100+
}
101+
102+
// ReplaceSecretsWithEnvVars replaces secret expressions in a value with environment variable references
103+
// Example: "${{ secrets.DD_API_KEY }}" -> "\${DD_API_KEY}"
104+
// The backslash is used to escape the ${} for proper JSON rendering in Copilot configs
105+
func ReplaceSecretsWithEnvVars(value string, secrets map[string]string) string {
106+
result := value
107+
for varName, secretExpr := range secrets {
108+
// Replace ${{ secrets.VAR }} with \${VAR} (backslash-escaped for copilot JSON config)
109+
result = strings.ReplaceAll(result, secretExpr, "\\${"+varName+"}")
110+
}
111+
return result
112+
}

0 commit comments

Comments
 (0)