feat: Add engine-specific validation for workflow frontmatter and network permissions

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Author: Mossaka

pkg/parser/schema.go

@@ -23,12 +23,24 @@ var mcpConfigSchema string
 
 // ValidateMainWorkflowFrontmatterWithSchema validates main workflow frontmatter using JSON schema
 func ValidateMainWorkflowFrontmatterWithSchema(frontmatter map[string]any) error {
-	return validateWithSchema(frontmatter, mainWorkflowSchema, "main workflow file")
+	// First run the standard schema validation
+	if err := validateWithSchema(frontmatter, mainWorkflowSchema, "main workflow file"); err != nil {
+		return err
+	}
+	
+	// Then run custom validation for engine-specific rules
+	return validateEngineSpecificRules(frontmatter)
 }
 
 // ValidateMainWorkflowFrontmatterWithSchemaAndLocation validates main workflow frontmatter with file location info
 func ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter map[string]any, filePath string) error {
-	return validateWithSchemaAndLocation(frontmatter, mainWorkflowSchema, "main workflow file", filePath)
+	// First run the standard schema validation with location
+	if err := validateWithSchemaAndLocation(frontmatter, mainWorkflowSchema, "main workflow file", filePath); err != nil {
+		return err
+	}
+	
+	// Then run custom validation for engine-specific rules
+	return validateEngineSpecificRules(frontmatter)
 }
 
 // ValidateIncludedFileFrontmatterWithSchema validates included file frontmatter using JSON schema
@@ -211,3 +223,40 @@ func min(a, b int) int {
 	}
 	return b
 }
+
+// validateEngineSpecificRules validates engine-specific rules that are not easily expressed in JSON schema
+func validateEngineSpecificRules(frontmatter map[string]any) error {
+	// Check if engine is configured
+	engine, ok := frontmatter["engine"]
+	if !ok {
+		return nil // No engine specified, nothing to validate
+	}
+	
+	// Handle string format engine
+	if engineStr, ok := engine.(string); ok {
+		// String format doesn't support permissions, so no validation needed
+		_ = engineStr
+		return nil
+	}
+	
+	// Handle object format engine
+	engineMap, ok := engine.(map[string]any)
+	if !ok {
+		return nil // Invalid engine format, but this should be caught by schema validation
+	}
+	
+	// Check engine ID
+	engineID, ok := engineMap["id"].(string)
+	if !ok {
+		return nil // Missing or invalid ID, but this should be caught by schema validation
+	}
+	
+	// Check if codex engine has permissions configured
+	if engineID == "codex" {
+		if _, hasPermissions := engineMap["permissions"]; hasPermissions {
+			return errors.New("Engine permissions are not supported for codex engine. Only Claude engine supports permissions configuration.")
+		}
+	}
+	
+	return nil
+}

pkg/parser/schema_test.go

@@ -474,6 +474,56 @@ func TestValidateMainWorkflowFrontmatterWithSchema(t *testing.T) {
 			wantErr:     true,
 			errContains: "additional properties 'invalid_prop' not allowed",
 		},
+		{
+			name: "valid claude engine with network permissions",
+			frontmatter: map[string]any{
+				"on": "push",
+				"engine": map[string]any{
+					"id": "claude",
+					"permissions": map[string]any{
+						"network": map[string]any{
+							"allowed": []string{"example.com", "*.trusted.com"},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "invalid codex engine with permissions",
+			frontmatter: map[string]any{
+				"on": "push",
+				"engine": map[string]any{
+					"id": "codex",
+					"permissions": map[string]any{
+						"network": map[string]any{
+							"allowed": []string{"example.com"},
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "Engine permissions are not supported for codex engine",
+		},
+		{
+			name: "valid codex engine without permissions",
+			frontmatter: map[string]any{
+				"on": "push",
+				"engine": map[string]any{
+					"id":    "codex",
+					"model": "gpt-4o",
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid codex string engine (no permissions possible)",
+			frontmatter: map[string]any{
+				"on":     "push",
+				"engine": "codex",
+			},
+			wantErr: false,
+		},
 	}
 
 	for _, tt := range tests {

pkg/workflow/engine_network_hooks.go

@@ -115,8 +115,10 @@ chmod +x .claude/hooks/network_permissions.py`, hookScript)
 }
 
 // HasNetworkPermissions checks if the engine config has network permissions configured
+// and if the engine is Claude (network permissions are only supported for Claude engine)
 func HasNetworkPermissions(engineConfig *EngineConfig) bool {
 	return engineConfig != nil &&
+		engineConfig.ID == "claude" &&
 		engineConfig.Permissions != nil &&
 		engineConfig.Permissions.Network != nil &&
 		len(engineConfig.Permissions.Network.Allowed) > 0

pkg/workflow/engine_network_test.go

@@ -150,6 +150,19 @@ func TestNetworkPermissionsHelpers(t *testing.T) {
 		if !HasNetworkPermissions(config) {
 			t.Error("Config with network permissions should have network permissions")
 		}
+
+		// Test non-Claude engine with network permissions (should be false)
+		nonClaudeConfig := &EngineConfig{
+			ID: "codex",
+			Permissions: &EnginePermissions{
+				Network: &NetworkPermissions{
+					Allowed: []string{"example.com"},
+				},
+			},
+		}
+		if HasNetworkPermissions(nonClaudeConfig) {
+			t.Error("Non-Claude engine should not have network permissions even if configured")
+		}
 	})
 
 	t.Run("GetAllowedDomains", func(t *testing.T) {
@@ -185,6 +198,20 @@ func TestNetworkPermissionsHelpers(t *testing.T) {
 		if domains[2] != "api.service.org" {
 			t.Errorf("Expected third domain to be 'api.service.org', got '%s'", domains[2])
 		}
+
+		// Test non-Claude engine with network permissions (should return empty)
+		nonClaudeConfig := &EngineConfig{
+			ID: "codex",
+			Permissions: &EnginePermissions{
+				Network: &NetworkPermissions{
+					Allowed: []string{"example.com", "test.org"},
+				},
+			},
+		}
+		domains = GetAllowedDomains(nonClaudeConfig)
+		if len(domains) != 0 {
+			t.Error("Non-Claude engine should return empty domains even if configured")
+		}
 	})
 }