diff --git a/pkg/workflow/bundler_safety_validation.go b/pkg/workflow/bundler_safety_validation.go index e0ef98d411..95c1c9bca0 100644 --- a/pkg/workflow/bundler_safety_validation.go +++ b/pkg/workflow/bundler_safety_validation.go @@ -79,8 +79,12 @@ func validateNoLocalRequires(bundledContent string) error { if len(foundRequires) > 0 { bundlerSafetyLog.Printf("Validation failed: found %d un-inlined local require statements", len(foundRequires)) - return fmt.Errorf("bundled JavaScript contains %d local require(s) that were not inlined:\n %s", - len(foundRequires), strings.Join(foundRequires, "\n ")) + return NewValidationError( + "bundled-javascript", + fmt.Sprintf("%d un-inlined requires", len(foundRequires)), + "bundled JavaScript contains local require() statements that were not inlined during bundling", + fmt.Sprintf("Found un-inlined requires:\n\n%s\n\nThis indicates a bundling failure. Check:\n1. All required files are in actions/setup/js/\n2. Bundler configuration includes all dependencies\n3. No circular dependencies exist\n\nRun 'make build' to regenerate bundles", strings.Join(foundRequires, "\n")), + ) } bundlerSafetyLog.Print("Validation successful: no local require statements found") @@ -117,8 +121,12 @@ func validateNoModuleReferences(bundledContent string) error { if len(foundReferences) > 0 { bundlerSafetyLog.Printf("Validation failed: found %d module references", len(foundReferences)) - return fmt.Errorf("bundled JavaScript for GitHub Script mode contains %d module reference(s) that should have been removed:\n %s\n\nGitHub Script mode does not support module.exports or exports; these references must be removed during bundling", - len(foundReferences), strings.Join(foundReferences, "\n ")) + return NewValidationError( + "bundled-javascript", + fmt.Sprintf("%d module references", len(foundReferences)), + "bundled JavaScript for GitHub Script mode contains module.exports or exports references", + fmt.Sprintf("Found module references:\n\n%s\n\nGitHub Script mode does not support CommonJS module system. Check:\n1. Bundle configuration removes module references\n2. Code doesn't use module.exports or exports\n3. Using appropriate runtime mode (consider 'nodejs' mode if module system is needed)\n\nRun 'make build' to regenerate bundles", strings.Join(foundReferences, "\n")), + ) } bundlerSafetyLog.Print("Validation successful: no module references found") @@ -201,8 +209,12 @@ func ValidateEmbeddedResourceRequires(sources map[string]string) error { if len(missingDeps) > 0 { bundlerSafetyLog.Printf("Validation failed: found %d missing dependencies", len(missingDeps)) - return fmt.Errorf("embedded JavaScript files have %d missing local require(s):\n %s\n\nThese files must be added to GetJavaScriptSources() in js.go", - len(missingDeps), strings.Join(missingDeps, "\n ")) + return NewValidationError( + "embedded-javascript", + fmt.Sprintf("%d missing dependencies", len(missingDeps)), + "embedded JavaScript files have missing local require() dependencies", + fmt.Sprintf("Missing dependencies:\n\n%s\n\nTo fix:\n1. Add missing .cjs files to actions/setup/js/\n2. Update GetJavaScriptSources() in pkg/workflow/js.go to include them\n3. Ensure file paths match require() statements\n4. Run 'make build' to regenerate bundles\n\nExample:\n//go:embed actions/setup/js/missing-file.cjs\nvar missingFileSource string", strings.Join(missingDeps, "\n")), + ) } bundlerSafetyLog.Printf("Validation successful: all local requires are available in sources") diff --git a/pkg/workflow/domains_protocol_integration_test.go b/pkg/workflow/domains_protocol_integration_test.go index 0ff8f427e9..581b5ffd57 100644 --- a/pkg/workflow/domains_protocol_integration_test.go +++ b/pkg/workflow/domains_protocol_integration_test.go @@ -74,7 +74,7 @@ Test HTTPS-only wildcard domains. on: push permissions: contents: read - issues: write + issues: read engine: copilot strict: false network: diff --git a/pkg/workflow/expression_validation.go b/pkg/workflow/expression_validation.go index 104427174b..d02752025f 100644 --- a/pkg/workflow/expression_validation.go +++ b/pkg/workflow/expression_validation.go @@ -171,8 +171,12 @@ func validateExpressionSafety(markdownContent string) error { allowedList.WriteString(" - inputs.* (workflow_call)\n") allowedList.WriteString(" - env.*\n") - return fmt.Errorf("unauthorized expressions:%s\nallowed:%s", - unauthorizedList.String(), allowedList.String()) + return NewValidationError( + "expressions", + fmt.Sprintf("%d unauthorized expressions found", len(unauthorizedExpressions)), + fmt.Sprintf("expressions are not in the allowed list:%s", unauthorizedList.String()), + fmt.Sprintf("Use only allowed expressions:%s\nFor more details, see the expression security documentation.", allowedList.String()), + ) } expressionValidationLog.Print("Expression safety validation passed") @@ -432,8 +436,12 @@ func validateRuntimeImportFiles(markdownContent string, workspaceDir string) err if len(validationErrors) > 0 { expressionValidationLog.Printf("Runtime-import validation failed: %d file(s) with errors", len(validationErrors)) - return fmt.Errorf("runtime-import files contain expression errors:\n\n%s", - strings.Join(validationErrors, "\n\n")) + return NewValidationError( + "runtime-import", + fmt.Sprintf("%d files with errors", len(validationErrors)), + fmt.Sprintf("runtime-import files contain expression errors:\n\n%s", strings.Join(validationErrors, "\n\n")), + "Fix the expression errors in the imported files listed above. Each file must only use allowed GitHub Actions expressions. See expression security documentation for details.", + ) } expressionValidationLog.Print("All runtime-import files validated successfully") diff --git a/pkg/workflow/features_validation.go b/pkg/workflow/features_validation.go index 391c383560..3b21d24307 100644 --- a/pkg/workflow/features_validation.go +++ b/pkg/workflow/features_validation.go @@ -65,7 +65,12 @@ func validateActionTag(value any) error { // Convert to string strVal, ok := value.(string) if !ok { - return fmt.Errorf("action-tag must be a string, got %T", value) + return NewValidationError( + "features.action-tag", + fmt.Sprintf("%T", value), + fmt.Sprintf("action-tag must be a string, got %T", value), + "Provide a string value for action-tag. Example:\nfeatures:\n action-tag: \"a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0\"", + ) } // Allow empty string (falls back to version) @@ -75,7 +80,12 @@ func validateActionTag(value any) error { // Validate it's a full SHA (40 hex characters) if !isValidFullSHA(strVal) { - return fmt.Errorf("action-tag must be a full 40-character commit SHA, got %q (length: %d). Short SHAs are not allowed. Use 'git rev-parse ' to get the full SHA", strVal, len(strVal)) + return NewValidationError( + "features.action-tag", + strVal, + fmt.Sprintf("action-tag must be a full 40-character commit SHA (length: %d). Short SHAs are not allowed", len(strVal)), + "Use 'git rev-parse ' to get the full SHA. Example:\n\n$ git rev-parse HEAD\na1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0\n\nThen use in workflow:\nfeatures:\n action-tag: \"a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0\"", + ) } return nil diff --git a/pkg/workflow/mcp_gateway_schema_validation.go b/pkg/workflow/mcp_gateway_schema_validation.go index 0c8d258f38..1700bc1bf3 100644 --- a/pkg/workflow/mcp_gateway_schema_validation.go +++ b/pkg/workflow/mcp_gateway_schema_validation.go @@ -67,7 +67,13 @@ func getCompiledMCPGatewaySchema() (*jsonschema.Schema, error) { // Parse the embedded schema var schemaDoc any if err := json.Unmarshal([]byte(mcpGatewayConfigSchema), &schemaDoc); err != nil { - mcpGatewaySchemaCompileError = fmt.Errorf("failed to parse embedded MCP Gateway configuration schema: %w", err) + mcpGatewaySchemaCompileError = NewOperationError( + "compile", + "MCP Gateway schema", + "embedded schema", + err, + "This is an internal error with the embedded MCP Gateway schema. Please report this issue:\nhttps://github.com/githubnext/gh-aw/issues/new", + ) return } @@ -75,14 +81,26 @@ func getCompiledMCPGatewaySchema() (*jsonschema.Schema, error) { loader := jsonschema.NewCompiler() schemaURL := "https://docs.github.com/gh-aw/schemas/mcp-gateway-config.schema.json" if err := loader.AddResource(schemaURL, schemaDoc); err != nil { - mcpGatewaySchemaCompileError = fmt.Errorf("failed to add MCP Gateway schema resource: %w", err) + mcpGatewaySchemaCompileError = NewOperationError( + "compile", + "MCP Gateway schema", + schemaURL, + err, + "This is an internal error with the MCP Gateway schema resource. Please report this issue:\nhttps://github.com/githubnext/gh-aw/issues/new", + ) return } // Compile the schema once schema, err := loader.Compile(schemaURL) if err != nil { - mcpGatewaySchemaCompileError = fmt.Errorf("failed to compile MCP Gateway configuration schema: %w", err) + mcpGatewaySchemaCompileError = NewOperationError( + "compile", + "MCP Gateway schema", + schemaURL, + err, + "This is an internal error compiling the MCP Gateway schema. Please report this issue:\nhttps://github.com/githubnext/gh-aw/issues/new", + ) return } diff --git a/pkg/workflow/npm_validation.go b/pkg/workflow/npm_validation.go index 86984bb520..fb4dcce920 100644 --- a/pkg/workflow/npm_validation.go +++ b/pkg/workflow/npm_validation.go @@ -56,7 +56,13 @@ func (c *Compiler) validateNpxPackages(workflowData *WorkflowData) error { _, err := exec.LookPath("npm") if err != nil { npmValidationLog.Print("npm command not found, cannot validate npx packages") - return fmt.Errorf("npm command not found - cannot validate npx packages. Install Node.js/npm or disable validation") + return NewOperationError( + "validate", + "npx packages", + "", + err, + "Install Node.js and npm to enable npx package validation:\n\nUsing nvm (recommended):\n$ nvm install --lts\n\nOr download from https://nodejs.org\n\nAlternatively, disable validation by setting GH_AW_SKIP_NPX_VALIDATION=true", + ) } var errors []string @@ -80,7 +86,12 @@ func (c *Compiler) validateNpxPackages(workflowData *WorkflowData) error { if len(errors) > 0 { npmValidationLog.Printf("npx package validation failed with %d errors", len(errors)) - return fmt.Errorf("npx package validation failed:\n - %s", strings.Join(errors, "\n - ")) + return NewValidationError( + "npx.packages", + fmt.Sprintf("%d packages not found", len(errors)), + "npx packages not found on npm registry", + fmt.Sprintf("Fix package names or verify they exist on npm:\n\n%s\n\nCheck package availability:\n$ npm view \n\nSearch for similar packages:\n$ npm search ", strings.Join(errors, "\n")), + ) } npmValidationLog.Print("All npx packages validated successfully") diff --git a/pkg/workflow/pip_validation.go b/pkg/workflow/pip_validation.go index 492f1b876b..7aefb25d6c 100644 --- a/pkg/workflow/pip_validation.go +++ b/pkg/workflow/pip_validation.go @@ -133,7 +133,13 @@ func (c *Compiler) validateUvPackages(workflowData *WorkflowData) error { _, pip3Err := exec.LookPath("pip3") if pip3Err != nil { pipValidationLog.Print("Neither uv nor pip commands found, cannot validate") - return fmt.Errorf("uv and pip commands not found - cannot validate uv packages. Install uv/pip or disable validation") + return NewOperationError( + "validate", + "uv packages", + "", + pip3Err, + "Install uv or pip to enable package validation:\n\nInstall uv (recommended):\n$ curl -LsSf https://astral.sh/uv/install.sh | sh\n\nOr install pip:\n$ python -m ensurepip --upgrade\n\nAlternatively, disable validation by setting GH_AW_SKIP_UV_VALIDATION=true", + ) } pipCmd = "pip3" pipValidationLog.Print("Using pip3 for validation") @@ -166,7 +172,12 @@ func (c *Compiler) validateUvPackages(workflowData *WorkflowData) error { } if len(errors) > 0 { - return fmt.Errorf("uv package validation requires network access:\n - %s", strings.Join(errors, "\n - ")) + return NewValidationError( + "uv.packages", + fmt.Sprintf("%d packages require validation", len(errors)), + "uv package validation requires network access or local cache", + fmt.Sprintf("Ensure network access or cache uv packages locally:\n\n%s\n\nCache packages:\n$ uv pip install --no-cache\n\nOr connect to network for validation", strings.Join(errors, "\n")), + ) } return nil diff --git a/pkg/workflow/runtime_validation.go b/pkg/workflow/runtime_validation.go index 64a31b3752..599088921f 100644 --- a/pkg/workflow/runtime_validation.go +++ b/pkg/workflow/runtime_validation.go @@ -136,7 +136,12 @@ func (c *Compiler) validateContainerImages(workflowData *WorkflowData) error { } if len(errors) > 0 { - return fmt.Errorf("container image validation failed:\n - %s", strings.Join(errors, "\n - ")) + return NewValidationError( + "container.images", + fmt.Sprintf("%d images failed validation", len(errors)), + "container image validation failed", + fmt.Sprintf("Fix the following container image issues:\n\n%s\n\nEnsure:\n1. Container images exist and are accessible\n2. Registry URLs are correct\n3. Image tags are specified\n4. You have pull permissions for private images", strings.Join(errors, "\n")), + ) } return nil @@ -170,7 +175,12 @@ func (c *Compiler) validateRuntimePackages(workflowData *WorkflowData) error { } if len(errors) > 0 { - return fmt.Errorf("runtime package validation failed:\n - %s", strings.Join(errors, "\n - ")) + return NewValidationError( + "runtime.packages", + fmt.Sprintf("%d package validation errors", len(errors)), + "runtime package validation failed", + fmt.Sprintf("Fix the following package issues:\n\n%s\n\nEnsure:\n1. Package names are spelled correctly\n2. Packages exist in their respective registries (npm, PyPI)\n3. Package managers (npm, pip, uv) are installed\n4. Network access is available for registry checks", strings.Join(errors, "\n")), + ) } return nil @@ -261,7 +271,12 @@ func validateNoDuplicateCacheIDs(caches []CacheMemoryEntry) error { seen := make(map[string]bool) for _, cache := range caches { if seen[cache.ID] { - return fmt.Errorf("duplicate cache-memory ID '%s' found. Each cache must have a unique ID", cache.ID) + return NewValidationError( + "sandbox.cache-memory", + cache.ID, + "duplicate cache-memory ID found - each cache must have a unique ID", + "Change the cache ID to a unique value. Example:\n\nsandbox:\n cache-memory:\n - id: cache-1\n size: 100MB\n - id: cache-2 # Use unique IDs\n size: 50MB", + ) } seen[cache.ID] = true } @@ -275,7 +290,12 @@ func validateSecretReferences(secrets []string) error { for _, secret := range secrets { if !secretNamePattern.MatchString(secret) { - return fmt.Errorf("invalid secret name: %s. Secret names must start with an uppercase letter and contain only uppercase letters, numbers, and underscores. Example: MY_SECRET_KEY", secret) + return NewValidationError( + "secrets", + secret, + "invalid secret name format - must follow environment variable naming conventions", + "Secret names must:\n- Start with an uppercase letter\n- Contain only uppercase letters, numbers, and underscores\n\nExamples:\n MY_SECRET_KEY ✓\n API_TOKEN_123 ✓\n mySecretKey ✗ (lowercase)\n 123_SECRET ✗ (starts with number)\n MY-SECRET ✗ (hyphens not allowed)", + ) } } diff --git a/pkg/workflow/safe_outputs_domains_validation.go b/pkg/workflow/safe_outputs_domains_validation.go index f4bacfde13..2638d9a4fe 100644 --- a/pkg/workflow/safe_outputs_domains_validation.go +++ b/pkg/workflow/safe_outputs_domains_validation.go @@ -70,14 +70,24 @@ func validateSafeOutputsAllowedDomains(config *SafeOutputsConfig) error { func validateDomainPattern(domain string) error { // Check for empty domain if domain == "" { - return fmt.Errorf("domain cannot be empty") + return NewValidationError( + "domain", + "", + "domain cannot be empty", + "Provide a valid domain name. Examples:\n - Plain domain: 'github.com'\n - Wildcard: '*.github.com'\n - With protocol: 'https://api.github.com'", + ) } // Check for invalid protocol prefixes // Only http:// and https:// are allowed if strings.Contains(domain, "://") { if !strings.HasPrefix(domain, "https://") && !strings.HasPrefix(domain, "http://") { - return fmt.Errorf("domain pattern '%s' has invalid protocol, only 'http://' and 'https://' are allowed", domain) + return NewValidationError( + "domain", + domain, + "domain pattern has invalid protocol, only 'http://' and 'https://' are allowed", + "Remove the invalid protocol or use 'http://' or 'https://'. Examples:\n - 'https://api.github.com'\n - 'http://example.com'\n - 'github.com' (no protocol)", + ) } } @@ -92,33 +102,63 @@ func validateDomainPattern(domain string) error { // Check for wildcard-only pattern if domainWithoutProtocol == "*" { - return fmt.Errorf("wildcard-only domain '*' is not allowed, use a specific wildcard pattern like '*.example.com' or 'https://*.example.com'") + return NewValidationError( + "domain", + domain, + "wildcard-only domain '*' is not allowed", + "Use a specific wildcard pattern with a base domain. Examples:\n - '*.example.com'\n - '*.github.com'\n - 'https://*.api.example.com'", + ) } // Check for wildcard without base domain (must be done before regex) if domainWithoutProtocol == "*." { - return fmt.Errorf("wildcard pattern '%s' must have a domain after '*.' (e.g., '*.example.com' or 'https://*.example.com')", domain) + return NewValidationError( + "domain", + domain, + "wildcard pattern must have a domain after '*.'", + "Add a base domain after the wildcard. Examples:\n - '*.example.com'\n - '*.github.com'\n - 'https://*.api.example.com'", + ) } // Check for multiple wildcards if strings.Count(domainWithoutProtocol, "*") > 1 { - return fmt.Errorf("domain pattern '%s' contains multiple wildcards, only one wildcard at the start is allowed (e.g., '*.example.com' or 'https://*.example.com')", domain) + return NewValidationError( + "domain", + domain, + "domain pattern contains multiple wildcards, only one wildcard at the start is allowed", + "Use a single wildcard at the start of the domain. Examples:\n - '*.example.com' ✓\n - '*.*.example.com' ✗ (multiple wildcards)\n - 'https://*.github.com' ✓", + ) } // Check for wildcard not at the start (in the domain part) if strings.Contains(domainWithoutProtocol, "*") && !strings.HasPrefix(domainWithoutProtocol, "*.") { - return fmt.Errorf("domain pattern '%s' has wildcard in invalid position, wildcard must be at the start followed by a dot (e.g., '*.example.com' or 'https://*.example.com')", domain) + return NewValidationError( + "domain", + domain, + "wildcard must be at the start followed by a dot", + "Move the wildcard to the beginning of the domain. Examples:\n - '*.example.com' ✓\n - 'example.*.com' ✗ (wildcard in middle)\n - 'https://*.github.com' ✓", + ) } // Additional validation for wildcard patterns if strings.HasPrefix(domainWithoutProtocol, "*.") { baseDomain := domainWithoutProtocol[2:] // Remove "*." if baseDomain == "" { - return fmt.Errorf("wildcard pattern '%s' must have a domain after '*.' (e.g., '*.example.com' or 'https://*.example.com')", domain) + return NewValidationError( + "domain", + domain, + "wildcard pattern must have a domain after '*.'", + "Add a base domain after the wildcard. Examples:\n - '*.example.com'\n - '*.github.com'\n - 'https://*.api.example.com'", + ) } // Ensure the base domain doesn't start with a dot if strings.HasPrefix(baseDomain, ".") { - return fmt.Errorf("wildcard pattern '%s' has invalid format, use '*.example.com' or 'https://*.example.com' instead", domain) + return NewValidationError( + "domain", + domain, + "wildcard pattern has invalid format (extra dot after wildcard)", + "Use correct wildcard format. Examples:\n - '*.example.com' ✓\n - '*.*.example.com' ✗ (extra dot)\n - 'https://*.github.com' ✓", + ) } } @@ -126,13 +166,28 @@ func validateDomainPattern(domain string) error { if !domainPattern.MatchString(domainWithoutProtocol) { // Provide specific error messages for common issues if strings.HasSuffix(domainWithoutProtocol, ".") { - return fmt.Errorf("domain pattern '%s' cannot end with a dot", domain) + return NewValidationError( + "domain", + domain, + "domain pattern cannot end with a dot", + "Remove the trailing dot from the domain. Examples:\n - 'example.com' ✓\n - 'example.com.' ✗\n - '*.github.com' ✓", + ) } if strings.Contains(domainWithoutProtocol, "..") { - return fmt.Errorf("domain pattern '%s' cannot contain consecutive dots", domain) + return NewValidationError( + "domain", + domain, + "domain pattern cannot contain consecutive dots", + "Remove extra dots from the domain. Examples:\n - 'api.example.com' ✓\n - 'api..example.com' ✗\n - 'sub.api.example.com' ✓", + ) } if strings.HasPrefix(domainWithoutProtocol, ".") && !strings.HasPrefix(domainWithoutProtocol, "*.") { - return fmt.Errorf("domain pattern '%s' cannot start with a dot (except for wildcard patterns like '*.example.com')", domain) + return NewValidationError( + "domain", + domain, + "domain pattern cannot start with a dot (except for wildcard patterns)", + "Remove the leading dot or use a wildcard. Examples:\n - 'example.com' ✓\n - '.example.com' ✗\n - '*.example.com' ✓", + ) } // Check for invalid characters (in the domain part, not protocol) for _, char := range domainWithoutProtocol { @@ -140,10 +195,20 @@ func validateDomainPattern(domain string) error { (char < 'A' || char > 'Z') && (char < '0' || char > '9') && char != '-' && char != '.' && char != '*' { - return fmt.Errorf("domain pattern '%s' contains invalid character '%c', only alphanumeric, hyphens, dots, and wildcards are allowed", domain, char) + return NewValidationError( + "domain", + domain, + fmt.Sprintf("domain pattern contains invalid character '%c'", char), + "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' ✓", + ) } } - return fmt.Errorf("domain pattern '%s' is not a valid domain format", domain) + return NewValidationError( + "domain", + domain, + "domain pattern is not a valid domain format", + "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'", + ) } return nil diff --git a/pkg/workflow/sandbox_validation.go b/pkg/workflow/sandbox_validation.go index 3c9e331f5e..fc8a88833d 100644 --- a/pkg/workflow/sandbox_validation.go +++ b/pkg/workflow/sandbox_validation.go @@ -28,7 +28,12 @@ func validateMountsSyntax(mounts []string) error { // Must have exactly 3 parts: source, destination, mode if len(parts) != 3 { - return fmt.Errorf("invalid mount syntax at index %d: '%s'. Expected format: 'source:destination:mode' (e.g., '/host/path:/container/path:ro')", i, mount) + return NewValidationError( + fmt.Sprintf("sandbox.mounts[%d]", i), + mount, + "mount syntax must follow 'source:destination:mode' format with exactly 3 colon-separated parts", + "Use the format 'source:destination:mode'. Example:\nsandbox:\n mounts:\n - \"/host/path:/container/path:ro\"", + ) } source := parts[0] @@ -37,15 +42,30 @@ func validateMountsSyntax(mounts []string) error { // Validate that source and destination are not empty if source == "" { - return fmt.Errorf("invalid mount at index %d: source path is empty in '%s'", i, mount) + return NewValidationError( + fmt.Sprintf("sandbox.mounts[%d].source", i), + mount, + "source path cannot be empty", + "Provide a valid source path. Example:\nsandbox:\n mounts:\n - \"/host/path:/container/path:ro\"", + ) } if dest == "" { - return fmt.Errorf("invalid mount at index %d: destination path is empty in '%s'", i, mount) + return NewValidationError( + fmt.Sprintf("sandbox.mounts[%d].destination", i), + mount, + "destination path cannot be empty", + "Provide a valid destination path. Example:\nsandbox:\n mounts:\n - \"/host/path:/container/path:ro\"", + ) } // Validate mode is either "ro" or "rw" if mode != "ro" && mode != "rw" { - 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) + return NewValidationError( + fmt.Sprintf("sandbox.mounts[%d].mode", i), + mode, + "mount mode must be 'ro' (read-only) or 'rw' (read-write)", + "Change the mount mode to either 'ro' or 'rw'. Example:\nsandbox:\n mounts:\n - \"/host/path:/container/path:ro\" # read-only\n - \"/host/path:/container/path:rw\" # read-write", + ) } sandboxValidationLog.Printf("Validated mount %d: source=%s, dest=%s, mode=%s", i, source, dest, mode) @@ -88,7 +108,12 @@ func validateSandboxConfig(workflowData *WorkflowData) error { if isSRTEnabled(workflowData) { // Check if the sandbox-runtime feature flag is enabled if !isFeatureEnabled(constants.SandboxRuntimeFeatureFlag, workflowData) { - 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") + return NewConfigurationError( + "features.sandbox-runtime", + "not enabled", + "sandbox-runtime feature is experimental and requires the feature flag to be enabled", + "Enable the sandbox-runtime feature flag in your workflow:\nfeatures:\n sandbox-runtime: true\n\nOr set the environment variable: GH_AW_FEATURES=sandbox-runtime", + ) } if workflowData.EngineConfig == nil || workflowData.EngineConfig.ID != "copilot" { @@ -96,19 +121,34 @@ func validateSandboxConfig(workflowData *WorkflowData) error { if workflowData.EngineConfig != nil { engineID = workflowData.EngineConfig.ID } - return fmt.Errorf("sandbox-runtime is only supported with Copilot engine (current engine: %s)", engineID) + return NewConfigurationError( + "engine", + engineID, + "sandbox-runtime is only supported with Copilot engine", + "Change your workflow to use the Copilot engine:\nengine: copilot\nsandbox: sandbox-runtime", + ) } // Check for mutual exclusivity with AWF if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil && workflowData.NetworkPermissions.Firewall.Enabled { - return fmt.Errorf("sandbox-runtime and AWF firewall cannot be used together; please use either 'sandbox: sandbox-runtime' or 'network.firewall' but not both") + return NewConfigurationError( + "sandbox", + "sandbox-runtime with network.firewall", + "sandbox-runtime and AWF firewall cannot be used together", + "Choose one sandbox approach:\n\nOption 1 (sandbox-runtime):\nsandbox: sandbox-runtime\n\nOption 2 (AWF firewall):\nnetwork:\n firewall: true", + ) } } // Validate config structure if provided if sandboxConfig.Config != nil { if sandboxConfig.Type != SandboxTypeRuntime { - return fmt.Errorf("custom sandbox config can only be provided when type is 'sandbox-runtime'") + return NewConfigurationError( + "sandbox.config", + string(sandboxConfig.Type), + "custom sandbox config can only be provided when type is 'sandbox-runtime'", + "Set sandbox type to 'sandbox-runtime' to use custom config:\nsandbox:\n type: sandbox-runtime\n config:\n # your custom config here", + ) } } @@ -124,7 +164,12 @@ func validateSandboxConfig(workflowData *WorkflowData) error { sandboxConfig.Type != "" if hasExplicitSandboxConfig && !HasMCPServers(workflowData) { - return fmt.Errorf("agent sandbox is enabled but MCP gateway is not enabled. The agent sandbox requires MCP servers to be configured. Add tools that use MCP (e.g., 'github', 'playwright') or disable the sandbox with 'sandbox: false'") + return NewConfigurationError( + "sandbox", + "enabled without MCP servers", + "agent sandbox requires MCP servers to be configured", + "Add MCP tools to your workflow:\n\ntools:\n github:\n mode: remote\n playwright:\n allowed_domains: [\"example.com\"]\n\nOr disable the sandbox:\nsandbox: false", + ) } if hasExplicitSandboxConfig { sandboxValidationLog.Print("Sandbox enabled with MCP gateway - validation passed") diff --git a/pkg/workflow/sandbox_validation_test.go b/pkg/workflow/sandbox_validation_test.go index 87f9fb9b21..ff9922a233 100644 --- a/pkg/workflow/sandbox_validation_test.go +++ b/pkg/workflow/sandbox_validation_test.go @@ -368,7 +368,7 @@ func TestSandboxMCPGatewayValidation(t *testing.T) { Tools: map[string]any{}, // No tools configured }, expectErr: true, - errContains: "agent sandbox is enabled but MCP gateway is not enabled", + errContains: "agent sandbox requires MCP servers to be configured", }, { name: "sandbox enabled with MCP servers - should pass", @@ -488,7 +488,7 @@ func TestSandboxMCPGatewayValidation(t *testing.T) { Tools: map[string]any{}, // No tools configured }, expectErr: true, - errContains: "agent sandbox is enabled but MCP gateway is not enabled", + errContains: "agent sandbox requires MCP servers to be configured", }, } diff --git a/pkg/workflow/step_order_validation.go b/pkg/workflow/step_order_validation.go index 6b36a97129..dfbe818a4e 100644 --- a/pkg/workflow/step_order_validation.go +++ b/pkg/workflow/step_order_validation.go @@ -112,7 +112,13 @@ func (t *StepOrderTracker) ValidateStepOrdering() error { // If there are artifact uploads but no secret redaction, that's a bug if !t.secretRedactionAdded { stepOrderLog.Print("Validation failed: artifact uploads found but no secret redaction step") - return fmt.Errorf("compiler bug: artifact uploads found but no secret redaction step was added (this is a critical security issue)") + return NewOperationError( + "compile", + "workflow steps", + "artifact uploads without secret redaction", + fmt.Errorf("artifact uploads found but no secret redaction step was added"), + "This is a critical security issue - a compiler bug. Please report this issue to the gh-aw maintainers with your workflow file:\nhttps://github.com/githubnext/gh-aw/issues/new", + ) } stepOrderLog.Printf("Secret redaction step found at order %d", t.secretRedactionOrder) @@ -126,16 +132,26 @@ func (t *StepOrderTracker) ValidateStepOrdering() error { } if len(uploadsBeforeRedaction) > 0 { - return fmt.Errorf("compiler bug: secret redaction must happen before artifact uploads, but found %d upload(s) before redaction: %s", - len(uploadsBeforeRedaction), strings.Join(uploadsBeforeRedaction, ", ")) + return NewOperationError( + "compile", + "workflow steps", + "incorrect step ordering", + fmt.Errorf("found %d upload(s) before secret redaction: %s", len(uploadsBeforeRedaction), strings.Join(uploadsBeforeRedaction, ", ")), + "This is a compiler bug - secret redaction must happen before artifact uploads. Please report this issue to the gh-aw maintainers with your workflow file:\nhttps://github.com/githubnext/gh-aw/issues/new", + ) } // Check that all uploaded paths are covered by secret redaction // Secret redaction scans all files in /tmp/gh-aw/ with extensions .txt, .json, .log unscannable := t.findUnscannablePaths(artifactUploads) if len(unscannable) > 0 { - return fmt.Errorf("compiler bug: the following artifact upload paths are not covered by secret redaction: %s", - strings.Join(unscannable, ", ")) + return NewOperationError( + "compile", + "workflow steps", + "artifact paths not covered by secret redaction", + fmt.Errorf("paths not covered: %s", strings.Join(unscannable, ", ")), + "This is a compiler bug - all artifact uploads must be covered by secret redaction. Please report this issue to the gh-aw maintainers with your workflow file:\nhttps://github.com/githubnext/gh-aw/issues/new", + ) } return nil