Skip to content

Commit 2a38bbe

Browse files
CopilotCopilot
authored
Refactor: Consolidate scattered validation functions into domain-specific files (#8185)
1 parent 629b3aa commit 2a38bbe

File tree

7 files changed

+179
-144
lines changed

7 files changed

+179
-144
lines changed

‎pkg/workflow/firewall.go‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package workflow
22

33
import (
4-
"fmt"
54
"strings"
65

76
"github.com/githubnext/gh-aw/pkg/constants"
@@ -188,22 +187,3 @@ func getAWFImageTag(firewallConfig *FirewallConfig) string {
188187
// Strip the 'v' prefix if present (AWF expects version without 'v' prefix)
189188
return strings.TrimPrefix(version, "v")
190189
}
191-
192-
// ValidateLogLevel validates that a firewall log-level value is one of the allowed enum values.
193-
// Valid values are: "debug", "info", "warn", "error".
194-
// Empty string is allowed as it defaults to "info" at runtime.
195-
// Returns an error if the log-level is invalid.
196-
func ValidateLogLevel(level string) error {
197-
// Empty string is allowed (defaults to "info")
198-
if level == "" {
199-
return nil
200-
}
201-
202-
valid := []string{"debug", "info", "warn", "error"}
203-
for _, v := range valid {
204-
if level == v {
205-
return nil
206-
}
207-
}
208-
return fmt.Errorf("invalid log-level '%s', must be one of: %v", level, valid)
209-
}

‎pkg/workflow/firewall_validation.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Package workflow provides firewall validation functions for agentic workflow compilation.
2+
//
3+
// This file contains domain-specific validation functions for firewall configuration:
4+
// - ValidateLogLevel() - Validates firewall log-level values
5+
//
6+
// These validation functions are organized in a dedicated file following the validation
7+
// architecture pattern where domain-specific validation belongs in domain validation files.
8+
// See validation.go for the complete validation architecture documentation.
9+
package workflow
10+
11+
import "fmt"
12+
13+
// ValidateLogLevel validates that a firewall log-level value is one of the allowed enum values.
14+
// Valid values are: "debug", "info", "warn", "error".
15+
// Empty string is allowed as it defaults to "info" at runtime.
16+
// Returns an error if the log-level is invalid.
17+
func ValidateLogLevel(level string) error {
18+
// Empty string is allowed (defaults to "info")
19+
if level == "" {
20+
return nil
21+
}
22+
23+
valid := []string{"debug", "info", "warn", "error"}
24+
for _, v := range valid {
25+
if level == v {
26+
return nil
27+
}
28+
}
29+
return fmt.Errorf("invalid log-level '%s', must be one of: %v", level, valid)
30+
}

‎pkg/workflow/gateway.go‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -69,21 +69,6 @@ func generateMCPGatewaySteps(workflowData *WorkflowData, mcpServersConfig map[st
6969
return steps
7070
}
7171

72-
// validateAndNormalizePort validates the port value and returns the normalized port or an error
73-
func validateAndNormalizePort(port int) (int, error) {
74-
// If port is 0, use the default
75-
if port == 0 {
76-
return DefaultMCPGatewayPort, nil
77-
}
78-
79-
// Validate port is in valid range (1-65535)
80-
if err := validateIntRange(port, 1, 65535, "port"); err != nil {
81-
return 0, err
82-
}
83-
84-
return port, nil
85-
}
86-
8772
// generateMCPGatewayStartStep generates the step that starts the MCP gateway
8873
func generateMCPGatewayStartStep(config *MCPGatewayRuntimeConfig, mcpServersConfig map[string]any) GitHubActionStep {
8974
gatewayLog.Print("Generating MCP gateway start step")

‎pkg/workflow/gateway_validation.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Package workflow provides gateway validation functions for agentic workflow compilation.
2+
//
3+
// This file contains domain-specific validation functions for MCP gateway configuration:
4+
// - validateAndNormalizePort() - Validates and normalizes gateway port values
5+
//
6+
// These validation functions are organized in a dedicated file following the validation
7+
// architecture pattern where domain-specific validation belongs in domain validation files.
8+
// See validation.go for the complete validation architecture documentation.
9+
package workflow
10+
11+
// validateAndNormalizePort validates the port value and returns the normalized port or an error
12+
func validateAndNormalizePort(port int) (int, error) {
13+
// If port is 0, use the default
14+
if port == 0 {
15+
return DefaultMCPGatewayPort, nil
16+
}
17+
18+
// Validate port is in valid range (1-65535)
19+
if err := validateIntRange(port, 1, 65535, "port"); err != nil {
20+
return 0, err
21+
}
22+
23+
return port, nil
24+
}

‎pkg/workflow/sandbox.go‎

Lines changed: 4 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,21 @@
1-
// Package workflow provides sandbox configuration and validation for agentic workflows.
1+
// Package workflow provides sandbox configuration for agentic workflows.
22
//
33
// This file handles:
44
// - Sandbox type definitions (AWF, SRT)
55
// - Sandbox configuration structures and parsing
66
// - Sandbox runtime config generation
7-
// - Domain-specific validation for sandbox configurations
87
//
98
// # Validation Functions
109
//
11-
// This file contains domain-specific validation functions for sandbox configuration:
12-
// - validateMountsSyntax() - Validates container mount syntax
13-
// - validateSandboxConfig() - Validates complete sandbox configuration
14-
//
15-
// These validation functions are co-located with sandbox logic following the principle
16-
// that domain-specific validation belongs in domain files. See validation.go for the
17-
// validation architecture documentation.
10+
// Domain-specific validation functions for sandbox configuration are located in
11+
// sandbox_validation.go following the validation architecture pattern.
12+
// See validation.go for the validation architecture documentation.
1813
package workflow
1914

2015
import (
2116
"encoding/json"
2217
"fmt"
23-
"strings"
2418

25-
"github.com/githubnext/gh-aw/pkg/constants"
2619
"github.com/githubnext/gh-aw/pkg/logger"
2720
)
2821

@@ -245,101 +238,3 @@ func generateSRTConfigJSON(workflowData *WorkflowData) (string, error) {
245238
sandboxLog.Printf("Generated SRT config: %s", string(jsonBytes))
246239
return string(jsonBytes), nil
247240
}
248-
249-
// validateMountsSyntax validates that mount strings follow the correct syntax
250-
// Expected format: "source:destination:mode" where mode is either "ro" or "rw"
251-
func validateMountsSyntax(mounts []string) error {
252-
for i, mount := range mounts {
253-
// Split the mount string by colons
254-
parts := strings.Split(mount, ":")
255-
256-
// Must have exactly 3 parts: source, destination, mode
257-
if len(parts) != 3 {
258-
return fmt.Errorf("invalid mount syntax at index %d: '%s'. Expected format: 'source:destination:mode' (e.g., '/host/path:/container/path:ro')", i, mount)
259-
}
260-
261-
source := parts[0]
262-
dest := parts[1]
263-
mode := parts[2]
264-
265-
// Validate that source and destination are not empty
266-
if source == "" {
267-
return fmt.Errorf("invalid mount at index %d: source path is empty in '%s'", i, mount)
268-
}
269-
if dest == "" {
270-
return fmt.Errorf("invalid mount at index %d: destination path is empty in '%s'", i, mount)
271-
}
272-
273-
// Validate mode is either "ro" or "rw"
274-
if mode != "ro" && mode != "rw" {
275-
return fmt.Errorf("invalid mount at index %d: mode must be 'ro' (read-only) or 'rw' (read-write), got '%s' in '%s'", i, mode, mount)
276-
}
277-
278-
sandboxLog.Printf("Validated mount %d: source=%s, dest=%s, mode=%s", i, source, dest, mode)
279-
}
280-
281-
return nil
282-
}
283-
284-
// validateSandboxConfig validates the sandbox configuration
285-
// Returns an error if the configuration is invalid
286-
func validateSandboxConfig(workflowData *WorkflowData) error {
287-
if workflowData == nil || workflowData.SandboxConfig == nil {
288-
return nil // No sandbox config is valid
289-
}
290-
291-
sandboxConfig := workflowData.SandboxConfig
292-
293-
// Validate mounts syntax if specified
294-
agentConfig := getAgentConfig(workflowData)
295-
if agentConfig != nil && len(agentConfig.Mounts) > 0 {
296-
if err := validateMountsSyntax(agentConfig.Mounts); err != nil {
297-
return err
298-
}
299-
}
300-
301-
// Validate that SRT is only used with Copilot engine
302-
if isSRTEnabled(workflowData) {
303-
// Check if the sandbox-runtime feature flag is enabled
304-
if !isFeatureEnabled(constants.SandboxRuntimeFeatureFlag, workflowData) {
305-
return fmt.Errorf("sandbox-runtime feature is experimental and requires the 'sandbox-runtime' feature flag to be enabled. Set 'features: { sandbox-runtime: true }' in frontmatter or set GH_AW_FEATURES=sandbox-runtime")
306-
}
307-
308-
if workflowData.EngineConfig == nil || workflowData.EngineConfig.ID != "copilot" {
309-
engineID := "none"
310-
if workflowData.EngineConfig != nil {
311-
engineID = workflowData.EngineConfig.ID
312-
}
313-
return fmt.Errorf("sandbox-runtime is only supported with Copilot engine (current engine: %s)", engineID)
314-
}
315-
316-
// Check for mutual exclusivity with AWF
317-
if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil && workflowData.NetworkPermissions.Firewall.Enabled {
318-
return fmt.Errorf("sandbox-runtime and AWF firewall cannot be used together; please use either 'sandbox: sandbox-runtime' or 'network.firewall' but not both")
319-
}
320-
}
321-
322-
// Validate config structure if provided
323-
if sandboxConfig.Config != nil {
324-
if sandboxConfig.Type != SandboxTypeRuntime {
325-
return fmt.Errorf("custom sandbox config can only be provided when type is 'sandbox-runtime'")
326-
}
327-
}
328-
329-
// Validate MCP gateway configuration
330-
if sandboxConfig.MCP != nil {
331-
mcpConfig := sandboxConfig.MCP
332-
333-
// Validate mutual exclusivity of command and container
334-
if mcpConfig.Command != "" && mcpConfig.Container != "" {
335-
return fmt.Errorf("sandbox.mcp: cannot specify both 'command' and 'container', use one or the other")
336-
}
337-
338-
// Validate entrypointArgs is only used with container
339-
if len(mcpConfig.EntrypointArgs) > 0 && mcpConfig.Container == "" {
340-
return fmt.Errorf("sandbox.mcp: 'entrypointArgs' can only be used with 'container'")
341-
}
342-
}
343-
344-
return nil
345-
}

‎pkg/workflow/sandbox_validation.go‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Package workflow provides sandbox validation functions for agentic workflow compilation.
2+
//
3+
// This file contains domain-specific validation functions for sandbox configuration:
4+
// - validateMountsSyntax() - Validates container mount syntax
5+
// - validateSandboxConfig() - Validates complete sandbox configuration
6+
//
7+
// These validation functions are organized in a dedicated file following the validation
8+
// architecture pattern where domain-specific validation belongs in domain validation files.
9+
// See validation.go for the complete validation architecture documentation.
10+
package workflow
11+
12+
import (
13+
"fmt"
14+
"strings"
15+
16+
"github.com/githubnext/gh-aw/pkg/constants"
17+
"github.com/githubnext/gh-aw/pkg/logger"
18+
)
19+
20+
var sandboxValidationLog = logger.New("workflow:sandbox_validation")
21+
22+
// validateMountsSyntax validates that mount strings follow the correct syntax
23+
// Expected format: "source:destination:mode" where mode is either "ro" or "rw"
24+
func validateMountsSyntax(mounts []string) error {
25+
for i, mount := range mounts {
26+
// Split the mount string by colons
27+
parts := strings.Split(mount, ":")
28+
29+
// Must have exactly 3 parts: source, destination, mode
30+
if len(parts) != 3 {
31+
return fmt.Errorf("invalid mount syntax at index %d: '%s'. Expected format: 'source:destination:mode' (e.g., '/host/path:/container/path:ro')", i, mount)
32+
}
33+
34+
source := parts[0]
35+
dest := parts[1]
36+
mode := parts[2]
37+
38+
// Validate that source and destination are not empty
39+
if source == "" {
40+
return fmt.Errorf("invalid mount at index %d: source path is empty in '%s'", i, mount)
41+
}
42+
if dest == "" {
43+
return fmt.Errorf("invalid mount at index %d: destination path is empty in '%s'", i, mount)
44+
}
45+
46+
// Validate mode is either "ro" or "rw"
47+
if mode != "ro" && mode != "rw" {
48+
return fmt.Errorf("invalid mount at index %d: mode must be 'ro' (read-only) or 'rw' (read-write), got '%s' in '%s'", i, mode, mount)
49+
}
50+
51+
sandboxValidationLog.Printf("Validated mount %d: source=%s, dest=%s, mode=%s", i, source, dest, mode)
52+
}
53+
54+
return nil
55+
}
56+
57+
// validateSandboxConfig validates the sandbox configuration
58+
// Returns an error if the configuration is invalid
59+
func validateSandboxConfig(workflowData *WorkflowData) error {
60+
if workflowData == nil || workflowData.SandboxConfig == nil {
61+
return nil // No sandbox config is valid
62+
}
63+
64+
sandboxConfig := workflowData.SandboxConfig
65+
66+
// Validate mounts syntax if specified
67+
agentConfig := getAgentConfig(workflowData)
68+
if agentConfig != nil && len(agentConfig.Mounts) > 0 {
69+
if err := validateMountsSyntax(agentConfig.Mounts); err != nil {
70+
return err
71+
}
72+
}
73+
74+
// Validate that SRT is only used with Copilot engine
75+
if isSRTEnabled(workflowData) {
76+
// Check if the sandbox-runtime feature flag is enabled
77+
if !isFeatureEnabled(constants.SandboxRuntimeFeatureFlag, workflowData) {
78+
return fmt.Errorf("sandbox-runtime feature is experimental and requires the 'sandbox-runtime' feature flag to be enabled. Set 'features: { sandbox-runtime: true }' in frontmatter or set GH_AW_FEATURES=sandbox-runtime")
79+
}
80+
81+
if workflowData.EngineConfig == nil || workflowData.EngineConfig.ID != "copilot" {
82+
engineID := "none"
83+
if workflowData.EngineConfig != nil {
84+
engineID = workflowData.EngineConfig.ID
85+
}
86+
return fmt.Errorf("sandbox-runtime is only supported with Copilot engine (current engine: %s)", engineID)
87+
}
88+
89+
// Check for mutual exclusivity with AWF
90+
if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil && workflowData.NetworkPermissions.Firewall.Enabled {
91+
return fmt.Errorf("sandbox-runtime and AWF firewall cannot be used together; please use either 'sandbox: sandbox-runtime' or 'network.firewall' but not both")
92+
}
93+
}
94+
95+
// Validate config structure if provided
96+
if sandboxConfig.Config != nil {
97+
if sandboxConfig.Type != SandboxTypeRuntime {
98+
return fmt.Errorf("custom sandbox config can only be provided when type is 'sandbox-runtime'")
99+
}
100+
}
101+
102+
// Validate MCP gateway configuration
103+
if sandboxConfig.MCP != nil {
104+
mcpConfig := sandboxConfig.MCP
105+
106+
// Validate mutual exclusivity of command and container
107+
if mcpConfig.Command != "" && mcpConfig.Container != "" {
108+
return fmt.Errorf("sandbox.mcp: cannot specify both 'command' and 'container', use one or the other")
109+
}
110+
111+
// Validate entrypointArgs is only used with container
112+
if len(mcpConfig.EntrypointArgs) > 0 && mcpConfig.Container == "" {
113+
return fmt.Errorf("sandbox.mcp: 'entrypointArgs' can only be used with 'container'")
114+
}
115+
}
116+
117+
return nil
118+
}

‎pkg/workflow/validation.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
// - engine_validation.go: AI engine configuration validation
1919
// - mcp_config_validation.go: MCP server configuration validation
2020
// - template_validation.go: Template structure validation
21+
// - firewall_validation.go: Firewall log-level validation
22+
// - gateway_validation.go: Gateway port validation
23+
// - sandbox_validation.go: Sandbox and mounts validation
2124
//
2225
// # When to Add New Validation
2326
//

0 commit comments

Comments
 (0)