refactor(workflow): resolve 8 semantic function clustering issues in pkg/workflow

pkg/workflow/add_comment.go

@@ -183,7 +183,7 @@ func (c *Compiler) parseCommentsConfig(outputMap map[string]any) *AddCommentsCon
 	}
 
 	// Validate target-repo (wildcard "*" is not allowed)
-	if validateTargetRepoSlug(config.TargetRepoSlug, addCommentLog) {
+	if err := validateTargetRepoSlug(config.TargetRepoSlug, addCommentLog); err != nil {
 		return nil // Invalid configuration, return nil to cause validation error
 	}
 

pkg/workflow/add_reviewer.go

@@ -45,7 +45,7 @@ func (c *Compiler) parseAddReviewerConfig(outputMap map[string]any) *AddReviewer
 	}
 
 	// Validate target-repo (wildcard "*" is not allowed for safe outputs)
-	if validateTargetRepoSlug(config.TargetRepoSlug, addReviewerLog) {
+	if err := validateTargetRepoSlug(config.TargetRepoSlug, addReviewerLog); err != nil {
 		return nil
 	}
 

pkg/workflow/agent_validation.go

@@ -153,8 +153,7 @@ func (c *Compiler) validateWebSearchSupport(tools map[string]any, engine CodingA
 
 	// web-search is specified, check if the engine supports it
 	if !engine.SupportsWebSearch() {
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Engine '%s' does not support the web-search tool. See https://github.github.com/gh-aw/guides/web-search/ for alternatives.", engine.GetID())))
-		c.IncrementWarningCount()
+		c.emitWarning(fmt.Sprintf("Engine '%s' does not support the web-search tool. See https://github.github.com/gh-aw/guides/web-search/ for alternatives.", engine.GetID()))
 	}
 }
 

pkg/workflow/close_entity_helpers.go

@@ -120,7 +120,7 @@ func (c *Compiler) parseCloseEntityConfig(outputMap map[string]any, params Close
 	}
 
 	// Validate target-repo (wildcard "*" is not allowed)
-	if validateTargetRepoSlug(config.TargetRepoSlug, logger) {
+	if err := validateTargetRepoSlug(config.TargetRepoSlug, logger); err != nil {
 		logger.Printf("Invalid target-repo slug for %s", params.ConfigKey)
 		return nil
 	}

pkg/workflow/compiler.go

@@ -54,6 +54,12 @@ func formatCompilerMessage(filePath string, msgType string, message string) stri
 	})
 }
 
+// emitWarning prints a warning message to stderr and increments the warning count.
+func (c *Compiler) emitWarning(msg string) {
+	fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg))
+	c.IncrementWarningCount()
+}
+
 // CompileWorkflow compiles a workflow markdown file into a GitHub Actions YAML file.
 // It reads the file from disk, parses frontmatter and markdown sections, and generates
 // the corresponding workflow YAML. Returns the compiled workflow data or an error.
@@ -202,8 +208,7 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath
 
 	// Emit warning for sandbox.agent: false (disables agent sandbox firewall)
 	if isAgentSandboxDisabled(workflowData) {
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage("⚠️  WARNING: Agent sandbox disabled (sandbox.agent: false). This removes firewall protection. The AI agent will have direct network access without firewall filtering. The MCP gateway remains enabled. Only use this for testing or in controlled environments where you trust the AI agent completely."))
-		c.IncrementWarningCount()
+		c.emitWarning("⚠️  WARNING: Agent sandbox disabled (sandbox.agent: false). This removes firewall protection. The AI agent will have direct network access without firewall filtering. The MCP gateway remains enabled. Only use this for testing or in controlled environments where you trust the AI agent completely.")
 	}
 
 	// Validate: threat detection requires sandbox.agent to be enabled (detection runs inside AWF)
@@ -213,20 +218,17 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath
 
 	// Emit experimental warning for safe-inputs feature
 	if IsSafeInputsEnabled(workflowData.SafeInputs, workflowData) {
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Using experimental feature: safe-inputs"))
-		c.IncrementWarningCount()
+		c.emitWarning("Using experimental feature: safe-inputs")
 	}
 
 	// Emit experimental warning for plugins feature
 	if workflowData.PluginInfo != nil && len(workflowData.PluginInfo.Plugins) > 0 {
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Using experimental feature: plugins"))
-		c.IncrementWarningCount()
+		c.emitWarning("Using experimental feature: plugins")
 	}
 
 	// Emit experimental warning for rate-limit feature
 	if workflowData.RateLimit != nil {
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Using experimental feature: rate-limit"))
-		c.IncrementWarningCount()
+		c.emitWarning("Using experimental feature: rate-limit")
 	}
 
 	// Validate workflow_run triggers have branch restrictions
@@ -419,8 +421,7 @@ func (c *Compiler) generateAndValidateYAML(workflowData *WorkflowData, markdownP
 			return "", formatCompilerError(markdownPath, "error", fmt.Sprintf("repository feature validation failed: %v", err), err)
 		}
 	} else if c.verbose {
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Schema validation available but skipped (use SetSkipValidation(false) to enable)"))
-		c.IncrementWarningCount()
+		c.emitWarning("Schema validation available but skipped (use SetSkipValidation(false) to enable)")
 	}
 
 	return yamlContent, nil

pkg/workflow/compiler_orchestrator_tools.go

@@ -2,11 +2,9 @@ package workflow
 
 import (
 	"fmt"
-	"os"
 	"sort"
 	"strings"
 
-	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/parser"
 	"github.com/goccy/go-yaml"
@@ -217,11 +215,9 @@ func (c *Compiler) processToolsAndMarkdown(result *parser.FrontmatterResult, cle
 
 	if !agenticEngine.SupportsToolsAllowlist() {
 		// For engines that don't support tool allowlists (like custom engine), ignore tools section and provide warnings
-		fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Using experimental %s support (engine: %s)", agenticEngine.GetDisplayName(), agenticEngine.GetID())))
-		c.IncrementWarningCount()
+		c.emitWarning(fmt.Sprintf("Using experimental %s support (engine: %s)", agenticEngine.GetDisplayName(), agenticEngine.GetID()))
 		if _, hasTools := result.Frontmatter["tools"]; hasTools {
-			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("'tools' section ignored when using engine: %s (%s doesn't support MCP tool allow-listing)", agenticEngine.GetID(), agenticEngine.GetDisplayName())))
-			c.IncrementWarningCount()
+			c.emitWarning(fmt.Sprintf("'tools' section ignored when using engine: %s (%s doesn't support MCP tool allow-listing)", agenticEngine.GetID(), agenticEngine.GetDisplayName()))
 		}
 		tools = map[string]any{}
 		// For now, we'll add a basic github tool (always uses docker MCP)

pkg/workflow/create_discussion.go

@@ -86,7 +86,7 @@ func (c *Compiler) parseDiscussionsConfig(outputMap map[string]any) *CreateDiscu
 	}
 
 	// Validate target-repo (wildcard "*" is not allowed)
-	if validateTargetRepoSlug(config.TargetRepoSlug, discussionLog) {
+	if err := validateTargetRepoSlug(config.TargetRepoSlug, discussionLog); err != nil {
 		return nil // Invalid configuration, return nil to cause validation error
 	}
 

pkg/workflow/create_issue.go

@@ -79,7 +79,7 @@ func (c *Compiler) parseIssuesConfig(outputMap map[string]any) *CreateIssuesConf
 	}
 
 	// Validate target-repo (wildcard "*" is not allowed)
-	if validateTargetRepoSlug(config.TargetRepoSlug, createIssueLog) {
+	if err := validateTargetRepoSlug(config.TargetRepoSlug, createIssueLog); err != nil {
 		return nil // Invalid configuration, return nil to cause validation error
 	}
 

pkg/workflow/create_pull_request.go

@@ -307,7 +307,7 @@ func (c *Compiler) parsePullRequestsConfig(outputMap map[string]any) *CreatePull
 	}
 
 	// Validate target-repo (wildcard "*" is not allowed)
-	if validateTargetRepoSlug(config.TargetRepoSlug, createPRLog) {
+	if err := validateTargetRepoSlug(config.TargetRepoSlug, createPRLog); err != nil {
 		return nil // Invalid configuration, return nil to cause validation error
 	}
 

pkg/workflow/dangerous_permissions_validation.go

@@ -62,27 +62,16 @@ func findWritePermissions(permissions *Permissions) []PermissionScope {
 		return nil
 	}
 
-	var writePerms []PermissionScope
-
-	// Check all permission scopes
+	// Collect scopes to check, excluding id-token and metadata
+	var scopes []PermissionScope
 	for _, scope := range GetAllPermissionScopes() {
-		// Skip id-token as it's safe and used for OIDC authentication
-		if scope == PermissionIdToken {
-			continue
-		}
-
-		// Skip metadata as it's a built-in read-only permission
-		if scope == PermissionMetadata {
+		if scope == PermissionIdToken || scope == PermissionMetadata {
 			continue
 		}
-
-		level, exists := permissions.Get(scope)
-		if exists && level == PermissionWrite {
-			writePerms = append(writePerms, scope)
-		}
+		scopes = append(scopes, scope)
 	}
 
-	return writePerms
+	return findWriteScopesForPolicy(permissions, scopes)
 }
 
 // formatDangerousPermissionsError formats an error message for write permissions violations

pkg/workflow/data/action_pins.json

@@ -110,6 +110,11 @@
       "version": "v6",
       "sha": "b7c566a772e6b6bfb58ed0dc250532a479d7789f"
     },
+    "anchore/sbom-action@v0": {
+      "repo": "anchore/sbom-action",
+      "version": "v0",
+      "sha": "17ae1740179002c89186b61233e0f892c3118b11"
+    },
     "anchore/sbom-action@v0.22.2": {
       "repo": "anchore/sbom-action",
       "version": "v0.22.2",

pkg/workflow/engine_firewall_support.go

@@ -3,9 +3,7 @@ package workflow
 import (
 	"errors"
 	"fmt"
-	"os"
 
-	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 )
 
@@ -79,8 +77,7 @@ func (c *Compiler) checkNetworkSupport(engine CodingAgentEngine, networkPermissi
 	}
 
 	// In non-strict mode, emit a warning
-	fmt.Fprintln(os.Stderr, console.FormatWarningMessage(message))
-	c.IncrementWarningCount()
+	c.emitWarning(message)
 
 	return nil
 }
@@ -107,8 +104,7 @@ func (c *Compiler) checkFirewallDisable(engine CodingAgentEngine, networkPermiss
 			}
 
 			// In non-strict mode, emit a warning
-			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(message))
-			c.IncrementWarningCount()
+			c.emitWarning(message)
 		}
 
 		// Also check if engine doesn't support firewall in strict mode when there are no restrictions

pkg/workflow/features_validation.go

@@ -11,7 +11,6 @@
 //
 //   - validateFeatures() - Validates all feature flags in WorkflowData
 //   - validateActionTag() - Validates action-tag is a full SHA
-//   - isValidFullSHA() - Checks if a string is a valid 40-character SHA
 //
 // # When to Add Validation Here
 //
@@ -24,15 +23,12 @@ package workflow
 
 import (
 	"fmt"
-	"regexp"
 
 	"github.com/github/gh-aw/pkg/logger"
 )
 
 var featuresValidationLog = logger.New("workflow:features_validation")
 
-var shaRegex = regexp.MustCompile("^[0-9a-f]{40}$")
-
 // validateFeatures validates all feature flags in the workflow data
 func validateFeatures(data *WorkflowData) error {
 	if data == nil || data.Features == nil {
@@ -91,11 +87,3 @@ func validateActionTag(value any) error {
 
 	return nil
 }
-
-// isValidFullSHA checks if a string is a valid 40-character hexadecimal SHA
-func isValidFullSHA(s string) bool {
-	if len(s) != 40 {
-		return false
-	}
-	return shaRegex.MatchString(s)
-}

pkg/workflow/firewall_validation.go

@@ -3,6 +3,7 @@
 // This file contains domain-specific validation functions for firewall configuration:
 //   - validateFirewallConfig() - Validates the overall firewall configuration
 //   - ValidateLogLevel() - Validates firewall log-level values
+//   - validateNetworkFirewallConfig() - Validates firewall configuration dependencies
 //
 // These validation functions are organized in a dedicated file following the validation
 // architecture pattern where domain-specific validation belongs in domain validation files.
@@ -14,6 +15,7 @@ import (
 	"fmt"
 	"slices"
 
+	"github.com/github/gh-aw/pkg/constants"
 	"github.com/github/gh-aw/pkg/logger"
 )
 
@@ -59,3 +61,35 @@ func ValidateLogLevel(level string) error {
 	firewallValidationLog.Printf("Invalid log-level: %s", level)
 	return fmt.Errorf("invalid log-level '%s', must be one of: %v", level, valid)
 }
+
+// validateNetworkFirewallConfig validates network firewall configuration dependencies
+// Returns an error if the configuration is invalid
+func validateNetworkFirewallConfig(networkPermissions *NetworkPermissions) error {
+	if networkPermissions == nil {
+		return nil
+	}
+
+	firewallConfig := networkPermissions.Firewall
+	if firewallConfig == nil {
+		return nil
+	}
+
+	firewallValidationLog.Print("Validating network firewall configuration")
+
+	// Validate allow-urls requires ssl-bump
+	if len(firewallConfig.AllowURLs) > 0 && !firewallConfig.SSLBump {
+		firewallValidationLog.Printf("Validation error: allow-urls specified without ssl-bump: %d URLs", len(firewallConfig.AllowURLs))
+		return NewValidationError(
+			"network.firewall.allow-urls",
+			"requires ssl-bump: true",
+			"allow-urls requires ssl-bump: true to function. SSL Bump enables HTTPS content inspection, which is necessary for URL path filtering",
+			"Enable SSL Bump in your firewall configuration:\n\nnetwork:\n  firewall:\n    ssl-bump: true\n    allow-urls:\n      - \"https://github.com/githubnext/*\"\n\nSee: "+string(constants.DocsNetworkURL),
+		)
+	}
+
+	if len(firewallConfig.AllowURLs) > 0 {
+		firewallValidationLog.Printf("Validated allow-urls: %d URLs with ssl-bump enabled", len(firewallConfig.AllowURLs))
+	}
+
+	return nil
+}

pkg/workflow/frontmatter_extraction_yaml.go

@@ -2,11 +2,9 @@ package workflow
 
 import (
 	"fmt"
-	"os"
 	"strconv"
 	"strings"
 
-	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/parser"
 	"github.com/goccy/go-yaml"
@@ -638,8 +636,7 @@ func (c *Compiler) extractCommandConfig(frontmatter map[string]any) (commandName
 			if hasCommand {
 				// Show deprecation warning if using old field name
 				if isDeprecated {
-					fmt.Fprintln(os.Stderr, console.FormatWarningMessage("The 'command:' trigger field is deprecated. Please use 'slash_command:' instead."))
-					c.IncrementWarningCount()
+					c.emitWarning("The 'command:' trigger field is deprecated. Please use 'slash_command:' instead.")
 				}
 
 				// Check if command is a string (shorthand format)

pkg/workflow/mcp_config_validation.go

@@ -312,14 +312,8 @@ func validateMCPMountsSyntax(toolName string, mountsRaw any) error {
 		return fmt.Errorf("tool '%s' mcp configuration 'mounts' must be an array of strings.\n\nExample:\ntools:\n  %s:\n    container: \"my-registry/my-tool\"\n    mounts:\n      - \"/host/path:/container/path:ro\"\n\nSee: %s", toolName, toolName, constants.DocsToolsURL)
 	}
 
-	for i, mount := range mounts {
-		source, dest, mode, err := validateMountStringFormat(mount)
-		if err != nil {
-			if source == "" && dest == "" && mode == "" {
-				return fmt.Errorf("tool '%s' mcp configuration mounts[%d] must follow 'source:destination:mode' format, got: %q.\n\nExample:\ntools:\n  %s:\n    container: \"my-registry/my-tool\"\n    mounts:\n      - \"/host/path:/container/path:ro\"\n\nSee: %s", toolName, i, mount, toolName, constants.DocsToolsURL)
-			}
-			return fmt.Errorf("tool '%s' mcp configuration mounts[%d] mode must be 'ro' or 'rw', got: %q.\n\nExample:\ntools:\n  %s:\n    container: \"my-registry/my-tool\"\n    mounts:\n      - \"/host/path:/container/path:ro\"  # read-only\n      - \"/host/path:/container/path:rw\"  # read-write\n\nSee: %s", toolName, i, mode, toolName, constants.DocsToolsURL)
-		}
+	if errs := validateMountStrings(mounts, string(constants.DocsToolsURL)); len(errs) > 0 {
+		return fmt.Errorf("tool '%s' mcp configuration %s\n\nExample:\ntools:\n  %s:\n    container: \"my-registry/my-tool\"\n    mounts:\n      - \"/host/path:/container/path:ro\"\n      - \"/host/path:/container/path:rw\"", toolName, errs[0], toolName)
 	}
 
 	return nil