Skip to content

Commit e212d9d

Browse files
CopilotCopilot
authored
refactor: relocate misplaced utility functions to semantically correct files (#25460)
1 parent aa358a3 commit e212d9d

6 files changed

Lines changed: 280 additions & 275 deletions

File tree

‎pkg/workflow/compiler.go‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,22 +52,6 @@ const (
5252
//go:embed schemas/github-workflow.json
5353
var githubWorkflowSchema string
5454

55-
// formatCompilerMessage creates a formatted compiler message string (for warnings printed to stderr)
56-
// filePath: the file path to include in the message (typically markdownPath or lockFile)
57-
// msgType: the message type ("error" or "warning")
58-
// message: the message text
59-
func formatCompilerMessage(filePath string, msgType string, message string) string {
60-
return console.FormatError(console.CompilerError{
61-
Position: console.ErrorPosition{
62-
File: filePath,
63-
Line: 0,
64-
Column: 0,
65-
},
66-
Type: msgType,
67-
Message: message,
68-
})
69-
}
70-
7155
// CompileWorkflow compiles a workflow markdown file into a GitHub Actions YAML file.
7256
// It reads the file from disk, parses frontmatter and markdown sections, and generates
7357
// the corresponding workflow YAML. Returns the compiled workflow data or an error.

‎pkg/workflow/compiler_error_formatter.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,19 @@ func formatCompilerErrorWithPosition(filePath string, line int, column int, errT
7171
// cause may be nil for validation errors that have no underlying cause.
7272
return &wrappedCompilerError{formatted: formattedErr, cause: cause}
7373
}
74+
75+
// formatCompilerMessage creates a formatted compiler message string (for warnings printed to stderr)
76+
// filePath: the file path to include in the message (typically markdownPath or lockFile)
77+
// msgType: the message type ("error" or "warning")
78+
// message: the message text
79+
func formatCompilerMessage(filePath string, msgType string, message string) string {
80+
return console.FormatError(console.CompilerError{
81+
Position: console.ErrorPosition{
82+
File: filePath,
83+
Line: 0,
84+
Column: 0,
85+
},
86+
Type: msgType,
87+
Message: message,
88+
})
89+
}

‎pkg/workflow/engine_helpers.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,54 @@ func BuildNpmEngineInstallStepsWithAWF(npmSteps []GitHubActionStep, workflowData
228228
return steps
229229
}
230230

231+
// GenerateMultiSecretValidationStep creates a GitHub Actions step that validates at least one
232+
// of multiple secrets is available.
233+
// secretNames: slice of secret names to validate (e.g., []string{"CODEX_API_KEY", "OPENAI_API_KEY"})
234+
// engineName: the display name of the engine (e.g., "Codex")
235+
// docsURL: URL to the documentation page for setting up the secret
236+
// envOverrides: optional map of env var key to expression override (from engine.env); when set,
237+
// the overridden expression is used instead of the default "${{ secrets.KEY }}" so the
238+
// validation step checks the user-provided secret reference rather than the default one.
239+
func GenerateMultiSecretValidationStep(secretNames []string, engineName, docsURL string, envOverrides map[string]string) GitHubActionStep {
240+
if len(secretNames) == 0 {
241+
// This is a programming error - engine configurations should always provide secrets
242+
// Log the error and return empty step to avoid breaking compilation
243+
engineHelpersLog.Printf("ERROR: GenerateMultiSecretValidationStep called with empty secretNames for engine %s", engineName)
244+
return GitHubActionStep{}
245+
}
246+
247+
// Build the step name
248+
stepName := fmt.Sprintf(" - name: Validate %s secret", strings.Join(secretNames, " or "))
249+
250+
// Build the command to call the validation script
251+
// The script expects: SECRET_NAME1 [SECRET_NAME2 ...] ENGINE_NAME DOCS_URL
252+
// Use shellJoinArgs to properly escape multi-word engine names and special characters
253+
scriptArgs := append(secretNames, engineName, docsURL)
254+
scriptArgsStr := shellJoinArgs(scriptArgs)
255+
256+
stepLines := []string{
257+
stepName,
258+
" id: validate-secret",
259+
" run: bash \"${RUNNER_TEMP}/gh-aw/actions/validate_multi_secret.sh\" " + scriptArgsStr,
260+
" env:",
261+
}
262+
263+
// Add env section with all secrets. When engine.env provides an override for a key,
264+
// use that expression (e.g. "${{ secrets.MY_ORG_TOKEN }}") so the validation step
265+
// validates the user-supplied secret instead of the default one.
266+
for _, secretName := range secretNames {
267+
expr := fmt.Sprintf("${{ secrets.%s }}", secretName)
268+
if envOverrides != nil {
269+
if override, ok := envOverrides[secretName]; ok {
270+
expr = override
271+
}
272+
}
273+
stepLines = append(stepLines, fmt.Sprintf(" %s: %s", secretName, expr))
274+
}
275+
276+
return GitHubActionStep(stepLines)
277+
}
278+
231279
// BuildDefaultSecretValidationStep returns a secret validation step for the given engine
232280
// configuration, or an empty step when a custom command is specified. This consolidates
233281
// the common guard+delegate pattern shared across all engine GetSecretValidationStep

‎pkg/workflow/network_firewall_validation.go‎

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
//
33
// This file contains domain-specific validation functions for network firewall configuration:
44
// - validateNetworkFirewallConfig() - Validates firewall configuration dependencies
5+
// - validateNetworkAllowedDomains() - Validates the allowed domains in network configuration
6+
// - validateDomainPattern() - Validates a single domain pattern
7+
// - isEcosystemIdentifier() - Checks if a string is an ecosystem identifier
58
//
69
// These validation functions are organized in a dedicated file following the validation
710
// architecture pattern where domain-specific validation belongs in domain validation files.
@@ -10,6 +13,10 @@
1013
package workflow
1114

1215
import (
16+
"fmt"
17+
"regexp"
18+
"strings"
19+
1320
"github.com/github/gh-aw/pkg/constants"
1421
)
1522

@@ -46,3 +53,212 @@ func validateNetworkFirewallConfig(networkPermissions *NetworkPermissions) error
4653

4754
return nil
4855
}
56+
57+
// validateNetworkAllowedDomains validates the allowed domains in network configuration
58+
func (c *Compiler) validateNetworkAllowedDomains(network *NetworkPermissions) error {
59+
if network == nil || len(network.Allowed) == 0 {
60+
return nil
61+
}
62+
63+
networkFirewallValidationLog.Printf("Validating %d network allowed domains", len(network.Allowed))
64+
65+
collector := NewErrorCollector(c.failFast)
66+
67+
for i, domain := range network.Allowed {
68+
// "*" means allow all traffic - skip validation
69+
if domain == "*" {
70+
networkFirewallValidationLog.Print("Skipping allow-all wildcard '*'")
71+
continue
72+
}
73+
74+
// Skip ecosystem identifiers - they don't need domain pattern validation
75+
if isEcosystemIdentifier(domain) {
76+
networkFirewallValidationLog.Printf("Skipping ecosystem identifier: %s", domain)
77+
continue
78+
}
79+
80+
if err := validateDomainPattern(domain); err != nil {
81+
wrappedErr := fmt.Errorf("network.allowed[%d]: %w", i, err)
82+
if returnErr := collector.Add(wrappedErr); returnErr != nil {
83+
return returnErr // Fail-fast mode
84+
}
85+
}
86+
}
87+
88+
if err := collector.Error(); err != nil {
89+
networkFirewallValidationLog.Printf("Network allowed domains validation failed: %v", err)
90+
return err
91+
}
92+
93+
networkFirewallValidationLog.Print("Network allowed domains validation passed")
94+
return nil
95+
}
96+
97+
// isEcosystemIdentifierPattern matches valid ecosystem identifiers like "defaults", "node", "dev-tools"
98+
var isEcosystemIdentifierPattern = regexp.MustCompile(`^[a-z][a-z0-9-]*$`)
99+
100+
// isEcosystemIdentifier checks if a domain string is actually an ecosystem identifier
101+
func isEcosystemIdentifier(domain string) bool {
102+
// Ecosystem identifiers are simple lowercase alphanumeric identifiers with optional hyphens
103+
// like "defaults", "node", "python", "dev-tools", "default-safe-outputs".
104+
// They don't contain dots, protocol prefixes, spaces, wildcards, or other special characters.
105+
return isEcosystemIdentifierPattern.MatchString(domain)
106+
}
107+
108+
// domainPattern validates domain patterns including wildcards
109+
// Valid patterns:
110+
// - Plain domains: github.com, api.github.com
111+
// - Wildcard domains: *.github.com
112+
// Invalid patterns:
113+
// - Multiple wildcards: *.*.github.com
114+
// - Wildcard not at start: github.*.com
115+
// - Empty or malformed domains
116+
var domainPattern = regexp.MustCompile(`^(\*\.)?[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$`)
117+
118+
// validateDomainPattern validates a single domain pattern
119+
func validateDomainPattern(domain string) error {
120+
// Check for empty domain
121+
if domain == "" {
122+
return NewValidationError(
123+
"domain",
124+
"",
125+
"domain cannot be empty",
126+
"Provide a valid domain name. Examples:\n - Plain domain: 'github.com'\n - Wildcard: '*.github.com'\n - With protocol: 'https://api.github.com'",
127+
)
128+
}
129+
130+
// Check for invalid protocol prefixes
131+
// Only http:// and https:// are allowed
132+
if strings.Contains(domain, "://") {
133+
if !strings.HasPrefix(domain, "https://") && !strings.HasPrefix(domain, "http://") {
134+
return NewValidationError(
135+
"domain",
136+
domain,
137+
"domain pattern has invalid protocol, only 'http://' and 'https://' are allowed",
138+
"Remove the invalid protocol or use 'http://' or 'https://'. Examples:\n - 'https://api.github.com'\n - 'http://example.com'\n - 'github.com' (no protocol)",
139+
)
140+
}
141+
}
142+
143+
// Strip protocol prefix if present (http:// or https://)
144+
// This allows protocol-specific domain filtering
145+
domainWithoutProtocol := domain
146+
if after, ok := strings.CutPrefix(domain, "https://"); ok {
147+
domainWithoutProtocol = after
148+
} else if after, ok := strings.CutPrefix(domain, "http://"); ok {
149+
domainWithoutProtocol = after
150+
}
151+
152+
// Check for wildcard-only pattern
153+
if domainWithoutProtocol == "*" {
154+
return NewValidationError(
155+
"domain",
156+
domain,
157+
"wildcard-only domain '*' is not allowed",
158+
"Use a specific wildcard pattern with a base domain. Examples:\n - '*.example.com'\n - '*.github.com'\n - 'https://*.api.example.com'",
159+
)
160+
}
161+
162+
// Check for wildcard without base domain (must be done before regex)
163+
if domainWithoutProtocol == "*." {
164+
return NewValidationError(
165+
"domain",
166+
domain,
167+
"wildcard pattern must have a domain after '*.'",
168+
"Add a base domain after the wildcard. Examples:\n - '*.example.com'\n - '*.github.com'\n - 'https://*.api.example.com'",
169+
)
170+
}
171+
172+
// Check for multiple wildcards
173+
if strings.Count(domainWithoutProtocol, "*") > 1 {
174+
return NewValidationError(
175+
"domain",
176+
domain,
177+
"domain pattern contains multiple wildcards, only one wildcard at the start is allowed",
178+
"Use a single wildcard at the start of the domain. Examples:\n - '*.example.com' ✓\n - '*.*.example.com' ✗ (multiple wildcards)\n - 'https://*.github.com' ✓",
179+
)
180+
}
181+
182+
// Check for wildcard not at the start (in the domain part)
183+
if strings.Contains(domainWithoutProtocol, "*") && !strings.HasPrefix(domainWithoutProtocol, "*.") {
184+
return NewValidationError(
185+
"domain",
186+
domain,
187+
"wildcard must be at the start followed by a dot",
188+
"Move the wildcard to the beginning of the domain. Examples:\n - '*.example.com' ✓\n - 'example.*.com' ✗ (wildcard in middle)\n - 'https://*.github.com' ✓",
189+
)
190+
}
191+
192+
// Additional validation for wildcard patterns
193+
if strings.HasPrefix(domainWithoutProtocol, "*.") {
194+
baseDomain := domainWithoutProtocol[2:] // Remove "*."
195+
if baseDomain == "" {
196+
return NewValidationError(
197+
"domain",
198+
domain,
199+
"wildcard pattern must have a domain after '*.'",
200+
"Add a base domain after the wildcard. Examples:\n - '*.example.com'\n - '*.github.com'\n - 'https://*.api.example.com'",
201+
)
202+
}
203+
// Ensure the base domain doesn't start with a dot
204+
if strings.HasPrefix(baseDomain, ".") {
205+
return NewValidationError(
206+
"domain",
207+
domain,
208+
"wildcard pattern has invalid format (extra dot after wildcard)",
209+
"Use correct wildcard format. Examples:\n - '*.example.com' ✓\n - '*.*.example.com' ✗ (extra dot)\n - 'https://*.github.com' ✓",
210+
)
211+
}
212+
}
213+
214+
// Validate domain pattern format (without protocol)
215+
if !domainPattern.MatchString(domainWithoutProtocol) {
216+
// Provide specific error messages for common issues
217+
if strings.HasSuffix(domainWithoutProtocol, ".") {
218+
return NewValidationError(
219+
"domain",
220+
domain,
221+
"domain pattern cannot end with a dot",
222+
"Remove the trailing dot from the domain. Examples:\n - 'example.com' ✓\n - 'example.com.' ✗\n - '*.github.com' ✓",
223+
)
224+
}
225+
if strings.Contains(domainWithoutProtocol, "..") {
226+
return NewValidationError(
227+
"domain",
228+
domain,
229+
"domain pattern cannot contain consecutive dots",
230+
"Remove extra dots from the domain. Examples:\n - 'api.example.com' ✓\n - 'api..example.com' ✗\n - 'sub.api.example.com' ✓",
231+
)
232+
}
233+
if strings.HasPrefix(domainWithoutProtocol, ".") && !strings.HasPrefix(domainWithoutProtocol, "*.") {
234+
return NewValidationError(
235+
"domain",
236+
domain,
237+
"domain pattern cannot start with a dot (except for wildcard patterns)",
238+
"Remove the leading dot or use a wildcard. Examples:\n - 'example.com' ✓\n - '.example.com' ✗\n - '*.example.com' ✓",
239+
)
240+
}
241+
// Check for invalid characters (in the domain part, not protocol)
242+
for _, char := range domainWithoutProtocol {
243+
if (char < 'a' || char > 'z') &&
244+
(char < 'A' || char > 'Z') &&
245+
(char < '0' || char > '9') &&
246+
char != '-' && char != '.' && char != '*' {
247+
return NewValidationError(
248+
"domain",
249+
domain,
250+
fmt.Sprintf("domain pattern contains invalid character '%c'", char),
251+
"Use only alphanumeric characters, hyphens, dots, and wildcards. Examples:\n - 'api-v2.example.com' ✓\n - 'api_v2.example.com' ✗ (underscore not allowed)\n - '*.github.com' ✓",
252+
)
253+
}
254+
}
255+
return NewValidationError(
256+
"domain",
257+
domain,
258+
"domain pattern is not a valid domain format",
259+
"Use a valid domain format. Examples:\n - Plain: 'github.com', 'api.example.com'\n - Wildcard: '*.github.com', '*.example.com'\n - With protocol: 'https://api.github.com'",
260+
)
261+
}
262+
263+
return nil
264+
}

‎pkg/workflow/runtime_step_generator.go‎

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"fmt"
55
"maps"
66
"sort"
7-
"strings"
87

98
"github.com/github/gh-aw/pkg/logger"
109
)
@@ -143,51 +142,3 @@ func generateSetupStep(req *RuntimeRequirement) GitHubActionStep {
143142

144143
return step
145144
}
146-
147-
// GenerateMultiSecretValidationStep creates a GitHub Actions step that validates at least one
148-
// of multiple secrets is available.
149-
// secretNames: slice of secret names to validate (e.g., []string{"CODEX_API_KEY", "OPENAI_API_KEY"})
150-
// engineName: the display name of the engine (e.g., "Codex")
151-
// docsURL: URL to the documentation page for setting up the secret
152-
// envOverrides: optional map of env var key to expression override (from engine.env); when set,
153-
// the overridden expression is used instead of the default "${{ secrets.KEY }}" so the
154-
// validation step checks the user-provided secret reference rather than the default one.
155-
func GenerateMultiSecretValidationStep(secretNames []string, engineName, docsURL string, envOverrides map[string]string) GitHubActionStep {
156-
if len(secretNames) == 0 {
157-
// This is a programming error - engine configurations should always provide secrets
158-
// Log the error and return empty step to avoid breaking compilation
159-
runtimeStepGeneratorLog.Printf("ERROR: GenerateMultiSecretValidationStep called with empty secretNames for engine %s", engineName)
160-
return GitHubActionStep{}
161-
}
162-
163-
// Build the step name
164-
stepName := fmt.Sprintf(" - name: Validate %s secret", strings.Join(secretNames, " or "))
165-
166-
// Build the command to call the validation script
167-
// The script expects: SECRET_NAME1 [SECRET_NAME2 ...] ENGINE_NAME DOCS_URL
168-
// Use shellJoinArgs to properly escape multi-word engine names and special characters
169-
scriptArgs := append(secretNames, engineName, docsURL)
170-
scriptArgsStr := shellJoinArgs(scriptArgs)
171-
172-
stepLines := []string{
173-
stepName,
174-
" id: validate-secret",
175-
" run: bash \"${RUNNER_TEMP}/gh-aw/actions/validate_multi_secret.sh\" " + scriptArgsStr,
176-
" env:",
177-
}
178-
179-
// Add env section with all secrets. When engine.env provides an override for a key,
180-
// use that expression (e.g. "${{ secrets.MY_ORG_TOKEN }}") so the validation step
181-
// validates the user-supplied secret instead of the default one.
182-
for _, secretName := range secretNames {
183-
expr := fmt.Sprintf("${{ secrets.%s }}", secretName)
184-
if envOverrides != nil {
185-
if override, ok := envOverrides[secretName]; ok {
186-
expr = override
187-
}
188-
}
189-
stepLines = append(stepLines, fmt.Sprintf(" %s: %s", secretName, expr))
190-
}
191-
192-
return GitHubActionStep(stepLines)
193-
}

0 commit comments

Comments
 (0)