From 63a6887fee8e64db8d142f5f610eaedd07ea731e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Feb 2026 02:40:32 +0000 Subject: [PATCH 1/3] Initial plan From 15eca7b99cab7f8591e2cb7bcb2e61cbe20bead3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Feb 2026 02:48:10 +0000 Subject: [PATCH 2/3] Allow custom domains in strict mode network configuration Modified strict mode validation to allow truly custom domains (domains not part of known ecosystems) while still enforcing ecosystem identifiers for domains that belong to known ecosystems. Changes: - Updated validateStrictFirewall to distinguish between ecosystem domains and custom domains - Only reject domains that belong to known ecosystems but are not specified as identifiers - Allow truly custom domains (e.g., api.example.com) in strict mode - Updated error messages to clarify that custom domains are allowed - Updated tests to reflect new behavior - Updated documentation to match implementation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- docs/src/content/docs/reference/network.md | 13 +- pkg/workflow/strict_mode_llm_gateway_test.go | 145 +++++++++++++++---- pkg/workflow/strict_mode_validation.go | 34 +++-- 3 files changed, 138 insertions(+), 54 deletions(-) diff --git a/docs/src/content/docs/reference/network.md b/docs/src/content/docs/reference/network.md index 05c008d53b6..99bb30b0993 100644 --- a/docs/src/content/docs/reference/network.md +++ b/docs/src/content/docs/reference/network.md @@ -238,26 +238,23 @@ network: When strict mode rejects an individual ecosystem domain, the error message suggests the appropriate ecosystem identifier: ````text -error: strict mode: network domains must be from known ecosystems (e.g., 'defaults', -'python', 'node') for all engines in strict mode. Custom domains are not allowed for -security. Did you mean: 'pypi.org' belongs to ecosystem 'python', 'npmjs.org' belongs -to ecosystem 'node'? Set 'strict: false' to use custom domains. +error: strict mode: domains that belong to known ecosystems must be specified using ecosystem identifiers (e.g., 'python', 'node') instead of individual domain names for security and maintainability. Did you mean: 'pypi.org' belongs to ecosystem 'python', 'npmjs.org' belongs to ecosystem 'node'? Truly custom domains (not part of known ecosystems) are allowed. ```` ### Bypassing Strict Mode -To use individual domains for development or testing, disable strict mode: +To use individual ecosystem domains (rather than ecosystem identifiers) for development or testing, disable strict mode: ````yaml wrap strict: false network: allowed: - defaults - - "pypi.org" # Individual domain allowed when strict: false - - "api.example.com" # Custom domain allowed + - "pypi.org" # Individual ecosystem domain allowed when strict: false + - "api.example.com" # Custom domain (also works in strict mode) ```` -Disabling strict mode reduces security validation. For production workflows, use ecosystem identifiers and keep strict mode enabled. +**Note**: Custom domains that are not part of any known ecosystem (e.g., `api.example.com`, `cdn.myservice.io`) are allowed in strict mode. You only need to disable strict mode if you want to use individual ecosystem domains instead of ecosystem identifiers. ## Implementation diff --git a/pkg/workflow/strict_mode_llm_gateway_test.go b/pkg/workflow/strict_mode_llm_gateway_test.go index 7c189e53cce..0731dce8acd 100644 --- a/pkg/workflow/strict_mode_llm_gateway_test.go +++ b/pkg/workflow/strict_mode_llm_gateway_test.go @@ -11,7 +11,7 @@ import ( // TestValidateStrictFirewall_LLMGatewaySupport tests the LLM gateway validation in strict mode func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { - t.Run("codex engine rejects custom domains in strict mode", func(t *testing.T) { + t.Run("codex engine allows truly custom domains in strict mode", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -23,15 +23,12 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { } err := compiler.validateStrictFirewall("codex", networkPerms, nil) - if err == nil { - t.Error("Expected error for codex engine with custom domains in strict mode, got nil") - } - if err != nil && !strings.Contains(err.Error(), "network domains must be from known ecosystems") { - t.Errorf("Expected error about known ecosystems, got: %v", err) + if err != nil { + t.Errorf("Expected no error for codex engine with truly custom domains in strict mode, got: %v", err) } }) - t.Run("copilot engine rejects custom domains in strict mode", func(t *testing.T) { + t.Run("copilot engine allows truly custom domains in strict mode", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -43,11 +40,8 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { } err := compiler.validateStrictFirewall("copilot", networkPerms, nil) - if err == nil { - t.Error("Expected error for copilot engine with custom domains, got nil") - } - if err != nil && !strings.Contains(err.Error(), "network domains must be from known ecosystems") { - t.Errorf("Expected error about known ecosystems, got: %v", err) + if err != nil { + t.Errorf("Expected no error for copilot engine with truly custom domains in strict mode, got: %v", err) } }) @@ -152,7 +146,7 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { } }) - t.Run("copilot engine rejects mixed ecosystems and custom domains in strict mode", func(t *testing.T) { + t.Run("copilot engine allows mixed ecosystems and truly custom domains in strict mode", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -164,15 +158,12 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { } err := compiler.validateStrictFirewall("copilot", networkPerms, nil) - if err == nil { - t.Error("Expected error for copilot engine with mixed ecosystems and custom domains, got nil") - } - if err != nil && !strings.Contains(err.Error(), "network domains must be from known ecosystems") { - t.Errorf("Expected error about known ecosystems, got: %v", err) + if err != nil { + t.Errorf("Expected no error for copilot engine with mixed ecosystems and truly custom domains in strict mode, got: %v", err) } }) - t.Run("claude engine without LLM gateway support rejects custom domains", func(t *testing.T) { + t.Run("claude engine allows truly custom domains in strict mode", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -184,11 +175,8 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { } err := compiler.validateStrictFirewall("claude", networkPerms, nil) - if err == nil { - t.Error("Expected error for claude engine with custom domains, got nil") - } - if err != nil && !strings.Contains(err.Error(), "network domains must be from known ecosystems") { - t.Errorf("Expected error about known ecosystems, got: %v", err) + if err != nil { + t.Errorf("Expected no error for claude engine with truly custom domains in strict mode, got: %v", err) } }) @@ -413,7 +401,7 @@ func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { } }) - t.Run("no suggestion for truly custom domains", func(t *testing.T) { + t.Run("truly custom domains are allowed without errors", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -425,13 +413,8 @@ func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { } err := compiler.validateStrictFirewall("copilot", networkPerms, nil) - if err == nil { - t.Fatal("Expected error for custom domain in strict mode, got nil") - } - - // Should NOT include "Did you mean" since this is a truly custom domain - if strings.Contains(err.Error(), "Did you mean") { - t.Errorf("Error should not include 'Did you mean' suggestion for truly custom domain, got: %v", err) + if err != nil { + t.Errorf("Expected no error for truly custom domain in strict mode, got: %v", err) } }) @@ -489,3 +472,101 @@ func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { } }) } + +// TestValidateStrictFirewall_CustomDomainBehavior tests the new behavior where truly custom domains are allowed +func TestValidateStrictFirewall_CustomDomainBehavior(t *testing.T) { + t.Run("truly custom domain is allowed in strict mode", func(t *testing.T) { + compiler := NewCompiler() + compiler.strictMode = true + + networkPerms := &NetworkPermissions{ + Allowed: []string{"api.example.com"}, + Firewall: &FirewallConfig{ + Enabled: true, + }, + } + + err := compiler.validateStrictFirewall("copilot", networkPerms, nil) + if err != nil { + t.Errorf("Expected no error for truly custom domain, got: %v", err) + } + }) + + t.Run("multiple truly custom domains are allowed in strict mode", func(t *testing.T) { + compiler := NewCompiler() + compiler.strictMode = true + + networkPerms := &NetworkPermissions{ + Allowed: []string{"api.example.com", "cdn.myservice.io", "*.assets.example.org"}, + Firewall: &FirewallConfig{ + Enabled: true, + }, + } + + err := compiler.validateStrictFirewall("copilot", networkPerms, nil) + if err != nil { + t.Errorf("Expected no error for multiple truly custom domains, got: %v", err) + } + }) + + t.Run("ecosystem identifier with custom domains are allowed", func(t *testing.T) { + compiler := NewCompiler() + compiler.strictMode = true + + networkPerms := &NetworkPermissions{ + Allowed: []string{"python", "node", "api.example.com", "cdn.example.com"}, + Firewall: &FirewallConfig{ + Enabled: true, + }, + } + + err := compiler.validateStrictFirewall("copilot", networkPerms, nil) + if err != nil { + t.Errorf("Expected no error for ecosystem identifiers with custom domains, got: %v", err) + } + }) + + t.Run("ecosystem domain without identifier is rejected even with custom domains", func(t *testing.T) { + compiler := NewCompiler() + compiler.strictMode = true + + networkPerms := &NetworkPermissions{ + Allowed: []string{"pypi.org", "api.example.com"}, + Firewall: &FirewallConfig{ + Enabled: true, + }, + } + + err := compiler.validateStrictFirewall("copilot", networkPerms, nil) + if err == nil { + t.Fatal("Expected error for ecosystem domain without identifier, got nil") + } + if !strings.Contains(err.Error(), "pypi.org") { + t.Errorf("Error should mention 'pypi.org', got: %v", err) + } + if !strings.Contains(err.Error(), "python") { + t.Errorf("Error should suggest 'python' ecosystem, got: %v", err) + } + // Should still mention that truly custom domains are allowed + if !strings.Contains(err.Error(), "Truly custom domains") { + t.Errorf("Error should mention that truly custom domains are allowed, got: %v", err) + } + }) + + t.Run("defaults with custom domains are allowed", func(t *testing.T) { + compiler := NewCompiler() + compiler.strictMode = true + + networkPerms := &NetworkPermissions{ + Allowed: []string{"defaults", "api.example.com"}, + Firewall: &FirewallConfig{ + Enabled: true, + }, + } + + err := compiler.validateStrictFirewall("copilot", networkPerms, nil) + if err != nil { + t.Errorf("Expected no error for defaults with custom domains, got: %v", err) + } + }) +} diff --git a/pkg/workflow/strict_mode_validation.go b/pkg/workflow/strict_mode_validation.go index a519137b5a6..b15012ee2de 100644 --- a/pkg/workflow/strict_mode_validation.go +++ b/pkg/workflow/strict_mode_validation.go @@ -344,18 +344,19 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N return fmt.Errorf("strict mode: 'sandbox.agent: false' is not allowed because it disables the agent sandbox firewall. This removes important security protections. Remove 'sandbox.agent: false' or set 'strict: false' to disable strict mode. See: https://github.github.com/gh-aw/reference/sandbox/") } - // In strict mode, ALL engines must use network domains from known ecosystems (not custom domains) + // In strict mode, ALL engines must use ecosystem identifiers for domains that belong to known ecosystems // This applies regardless of LLM gateway support + // Truly custom domains (domains not part of any known ecosystem) are allowed if networkPermissions != nil && len(networkPermissions.Allowed) > 0 { strictModeValidationLog.Printf("Validating network domains in strict mode for all engines") - // Check if allowed domains contain only known ecosystem identifiers - // Track domains that are not ecosystem identifiers (both individual ecosystem domains and truly custom domains) + // Check if allowed domains contain only known ecosystem identifiers or truly custom domains + // Track domains that belong to known ecosystems but are not specified as ecosystem identifiers type domainSuggestion struct { domain string ecosystem string // empty if no ecosystem found, non-empty if domain belongs to known ecosystem } - var invalidDomains []domainSuggestion + var ecosystemDomainsNotAsIdentifiers []domainSuggestion for _, domain := range networkPermissions.Allowed { // Skip wildcards (handled below) @@ -373,30 +374,35 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N // Not an ecosystem identifier - check if it belongs to any ecosystem ecosystem := GetDomainEcosystem(domain) - // Add to invalid domains (with or without ecosystem suggestion) strictModeValidationLog.Printf("Domain '%s' ecosystem: '%s'", domain, ecosystem) - invalidDomains = append(invalidDomains, domainSuggestion{domain: domain, ecosystem: ecosystem}) + + if ecosystem != "" { + // This domain belongs to a known ecosystem but was not specified as an ecosystem identifier + // In strict mode, we require ecosystem identifiers for ecosystem domains + ecosystemDomainsNotAsIdentifiers = append(ecosystemDomainsNotAsIdentifiers, domainSuggestion{domain: domain, ecosystem: ecosystem}) + } else { + // This is a truly custom domain (not part of any known ecosystem) - allowed in strict mode + strictModeValidationLog.Printf("Domain '%s' is a truly custom domain, allowed in strict mode", domain) + } } - if len(invalidDomains) > 0 { - strictModeValidationLog.Printf("Engine '%s' has invalid domains in strict mode, failing validation", engineID) + if len(ecosystemDomainsNotAsIdentifiers) > 0 { + strictModeValidationLog.Printf("Engine '%s' has ecosystem domains not specified as identifiers in strict mode, failing validation", engineID) // Build error message with ecosystem suggestions - errorMsg := "strict mode: network domains must be from known ecosystems (e.g., 'defaults', 'python', 'node') for all engines in strict mode. Custom domains are not allowed for security." + errorMsg := "strict mode: domains that belong to known ecosystems must be specified using ecosystem identifiers (e.g., 'python', 'node') instead of individual domain names for security and maintainability." // Add suggestions for domains that belong to known ecosystems var suggestions []string - for _, ds := range invalidDomains { - if ds.ecosystem != "" { - suggestions = append(suggestions, fmt.Sprintf("'%s' belongs to ecosystem '%s'", ds.domain, ds.ecosystem)) - } + for _, ds := range ecosystemDomainsNotAsIdentifiers { + suggestions = append(suggestions, fmt.Sprintf("'%s' belongs to ecosystem '%s'", ds.domain, ds.ecosystem)) } if len(suggestions) > 0 { errorMsg += " Did you mean: " + strings.Join(suggestions, ", ") + "?" } - errorMsg += " Set 'strict: false' to use custom domains. See: https://github.github.com/gh-aw/reference/network/" + errorMsg += " Truly custom domains (not part of known ecosystems) are allowed. See: https://github.github.com/gh-aw/reference/network/" return fmt.Errorf("%s", errorMsg) } From c85d9886b234d2224d0d4ab3e0a52b15938064e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Feb 2026 03:24:13 +0000 Subject: [PATCH 3/3] Change strict mode to warn instead of reject ecosystem domains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per feedback, changed strict mode behavior: - BEFORE: Reject ecosystem domains (e.g., pypi.org) with error - AFTER: Allow ecosystem domains with warning suggesting ecosystem identifiers Changes: - Modified validateStrictFirewall to emit warnings instead of errors for ecosystem domains - Warnings suggest using ecosystem identifiers (e.g., 'pypi.org' → 'python') - Updated all tests to expect warnings instead of errors - Updated documentation to reflect new warning-based approach Truly custom domains remain allowed without warnings. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- docs/src/content/docs/reference/network.md | 41 ++--- pkg/workflow/strict_mode_llm_gateway_test.go | 173 ++++++++----------- pkg/workflow/strict_mode_validation.go | 27 ++- 3 files changed, 96 insertions(+), 145 deletions(-) diff --git a/docs/src/content/docs/reference/network.md b/docs/src/content/docs/reference/network.md index 99bb30b0993..2eb6bab3ea8 100644 --- a/docs/src/content/docs/reference/network.md +++ b/docs/src/content/docs/reference/network.md @@ -201,30 +201,30 @@ Mix ecosystem identifiers with specific domains for fine-grained control: ## Strict Mode Validation -When [strict mode](/gh-aw/reference/frontmatter/#strict-mode-strict) is enabled (default), network configuration is validated to ensure security best practices. Strict mode enforces the use of ecosystem identifiers instead of individual domains for all engines. +When [strict mode](/gh-aw/reference/frontmatter/#strict-mode-strict) is enabled (default), network configuration is validated to ensure security best practices. Strict mode recommends using ecosystem identifiers instead of individual domains for better maintainability. -### Ecosystem Identifier Requirement +### Ecosystem Identifier Recommendation -Strict mode requires ecosystem identifiers (e.g., `python`, `node`) instead of individual ecosystem member domains (e.g., `pypi.org`, `npmjs.org`). This applies to all engines, including those with LLM gateway support. +Strict mode allows but warns about individual ecosystem member domains (e.g., `pypi.org`, `npmjs.org`), recommending ecosystem identifiers (e.g., `python`, `node`) instead. This applies to all engines, including those with LLM gateway support. ````yaml wrap -# ❌ Rejected in strict mode (all engines) +# ⚠ Allowed with warning in strict mode (all engines) strict: true network: allowed: - defaults - - "pypi.org" # Individual domain rejected - - "npmjs.org" # Individual domain rejected + - "pypi.org" # Allowed but warns: recommend using 'python' + - "npmjs.org" # Allowed but warns: recommend using 'node' -# ✅ Accepted in strict mode +# ✅ Recommended in strict mode (no warnings) strict: true network: allowed: - defaults - - python # Ecosystem identifier - - node # Ecosystem identifier + - python # Ecosystem identifier (recommended) + - node # Ecosystem identifier (recommended) -# ✅ Custom domains still allowed in strict mode +# ✅ Custom domains allowed in strict mode (no warnings) strict: true network: allowed: @@ -233,28 +233,15 @@ network: - "api.example.com" # Custom domain (not part of known ecosystem) ```` -### Helpful Error Messages +### Warning Messages -When strict mode rejects an individual ecosystem domain, the error message suggests the appropriate ecosystem identifier: +When strict mode encounters an individual ecosystem domain, it emits a warning suggesting the appropriate ecosystem identifier: ````text -error: strict mode: domains that belong to known ecosystems must be specified using ecosystem identifiers (e.g., 'python', 'node') instead of individual domain names for security and maintainability. Did you mean: 'pypi.org' belongs to ecosystem 'python', 'npmjs.org' belongs to ecosystem 'node'? Truly custom domains (not part of known ecosystems) are allowed. +warning: strict mode: recommend using ecosystem identifiers instead of individual domain names for better maintainability: 'pypi.org' → 'python', 'npmjs.org' → 'node' ```` -### Bypassing Strict Mode - -To use individual ecosystem domains (rather than ecosystem identifiers) for development or testing, disable strict mode: - -````yaml wrap -strict: false -network: - allowed: - - defaults - - "pypi.org" # Individual ecosystem domain allowed when strict: false - - "api.example.com" # Custom domain (also works in strict mode) -```` - -**Note**: Custom domains that are not part of any known ecosystem (e.g., `api.example.com`, `cdn.myservice.io`) are allowed in strict mode. You only need to disable strict mode if you want to use individual ecosystem domains instead of ecosystem identifiers. +The workflow will compile successfully, but the warning helps maintain best practices. ## Implementation diff --git a/pkg/workflow/strict_mode_llm_gateway_test.go b/pkg/workflow/strict_mode_llm_gateway_test.go index 0731dce8acd..065ff3a24c4 100644 --- a/pkg/workflow/strict_mode_llm_gateway_test.go +++ b/pkg/workflow/strict_mode_llm_gateway_test.go @@ -79,11 +79,11 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { } }) - t.Run("copilot engine rejects domains from known ecosystems but suggests ecosystem identifier in strict mode", func(t *testing.T) { + t.Run("copilot engine allows domains from known ecosystems with warning suggesting ecosystem identifier in strict mode", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true - // These domains are from known ecosystems (python, node) but users should use ecosystem identifiers instead + // These domains are from known ecosystems (python, node) and will emit warnings suggesting ecosystem identifiers networkPerms := &NetworkPermissions{ Allowed: []string{"pypi.org", "registry.npmjs.org"}, Firewall: &FirewallConfig{ @@ -91,16 +91,14 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("copilot", networkPerms, nil) - if err == nil { - t.Fatal("Expected error for individual ecosystem domains in strict mode, got nil") - } - // Should suggest using ecosystem identifiers instead - if !strings.Contains(err.Error(), "python") { - t.Errorf("Error should suggest 'python' ecosystem, got: %v", err) + if err != nil { + t.Errorf("Expected no error for individual ecosystem domains in strict mode, got: %v", err) } - if !strings.Contains(err.Error(), "node") { - t.Errorf("Error should suggest 'node' ecosystem, got: %v", err) + // Should have incremented warning count + if compiler.GetWarningCount() != initialWarnings+1 { + t.Errorf("Expected warning count to increase by 1, got %d warnings", compiler.GetWarningCount()-initialWarnings) } }) @@ -121,11 +119,11 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { } }) - t.Run("codex engine rejects domains from known ecosystems but suggests ecosystem identifier", func(t *testing.T) { + t.Run("codex engine allows domains from known ecosystems with warning suggesting ecosystem identifier", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true - // These domains are from known ecosystems (python, node) but users should use ecosystem identifiers instead + // These domains are from known ecosystems (python, node) and will emit warnings suggesting ecosystem identifiers networkPerms := &NetworkPermissions{ Allowed: []string{"pypi.org", "registry.npmjs.org"}, Firewall: &FirewallConfig{ @@ -133,16 +131,14 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("codex", networkPerms, nil) - if err == nil { - t.Fatal("Expected error for individual ecosystem domains in strict mode, got nil") - } - // Should suggest using ecosystem identifiers instead - if !strings.Contains(err.Error(), "python") { - t.Errorf("Error should suggest 'python' ecosystem, got: %v", err) + if err != nil { + t.Errorf("Expected no error for individual ecosystem domains in strict mode, got: %v", err) } - if !strings.Contains(err.Error(), "node") { - t.Errorf("Error should suggest 'node' ecosystem, got: %v", err) + // Should have incremented warning count + if compiler.GetWarningCount() != initialWarnings+1 { + t.Errorf("Expected warning count to increase by 1, got %d warnings", compiler.GetWarningCount()-initialWarnings) } }) @@ -309,9 +305,9 @@ func TestSupportsLLMGateway(t *testing.T) { } } -// TestValidateStrictFirewall_EcosystemSuggestions tests ecosystem suggestions in error messages +// TestValidateStrictFirewall_EcosystemSuggestions tests ecosystem suggestions in warning messages func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { - t.Run("suggests ecosystem when individual domain from ecosystem is used", func(t *testing.T) { + t.Run("warns with ecosystem suggestion when individual domain from ecosystem is used", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -322,24 +318,18 @@ func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("copilot", networkPerms, nil) - if err == nil { - t.Fatal("Expected error for individual ecosystem domain in strict mode, got nil") - } - - // Should suggest using 'python' ecosystem identifier - if !strings.Contains(err.Error(), "pypi.org") { - t.Errorf("Error should mention domain 'pypi.org', got: %v", err) - } - if !strings.Contains(err.Error(), "python") { - t.Errorf("Error should suggest 'python' ecosystem, got: %v", err) + if err != nil { + t.Errorf("Expected no error for individual ecosystem domain in strict mode, got: %v", err) } - if !strings.Contains(err.Error(), "Did you mean") { - t.Errorf("Error should include 'Did you mean' suggestion, got: %v", err) + // Should have emitted a warning + if compiler.GetWarningCount() != initialWarnings+1 { + t.Errorf("Expected warning count to increase by 1, got %d warnings", compiler.GetWarningCount()-initialWarnings) } }) - t.Run("suggests ecosystem for multiple domains from same ecosystem", func(t *testing.T) { + t.Run("warns with ecosystem suggestion for multiple domains from same ecosystem", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -350,24 +340,18 @@ func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("copilot", networkPerms, nil) - if err == nil { - t.Fatal("Expected error for individual ecosystem domains in strict mode, got nil") - } - - // Should suggest using 'node' ecosystem identifier for both - if !strings.Contains(err.Error(), "npmjs.org") { - t.Errorf("Error should mention domain 'npmjs.org', got: %v", err) - } - if !strings.Contains(err.Error(), "node") { - t.Errorf("Error should suggest 'node' ecosystem, got: %v", err) + if err != nil { + t.Errorf("Expected no error for individual ecosystem domains in strict mode, got: %v", err) } - if !strings.Contains(err.Error(), "Did you mean") { - t.Errorf("Error should include 'Did you mean' suggestion, got: %v", err) + // Should have emitted a warning + if compiler.GetWarningCount() != initialWarnings+1 { + t.Errorf("Expected warning count to increase by 1, got %d warnings", compiler.GetWarningCount()-initialWarnings) } }) - t.Run("suggests ecosystem for domains from different ecosystems", func(t *testing.T) { + t.Run("warns with ecosystem suggestion for domains from different ecosystems", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -378,30 +362,18 @@ func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("copilot", networkPerms, nil) - if err == nil { - t.Fatal("Expected error for individual ecosystem domains in strict mode, got nil") - } - - // Should suggest both 'python' and 'node' ecosystems - if !strings.Contains(err.Error(), "pypi.org") { - t.Errorf("Error should mention domain 'pypi.org', got: %v", err) - } - if !strings.Contains(err.Error(), "python") { - t.Errorf("Error should suggest 'python' ecosystem, got: %v", err) - } - if !strings.Contains(err.Error(), "npmjs.org") { - t.Errorf("Error should mention domain 'npmjs.org', got: %v", err) - } - if !strings.Contains(err.Error(), "node") { - t.Errorf("Error should suggest 'node' ecosystem, got: %v", err) + if err != nil { + t.Errorf("Expected no error for individual ecosystem domains in strict mode, got: %v", err) } - if !strings.Contains(err.Error(), "Did you mean") { - t.Errorf("Error should include 'Did you mean' suggestion, got: %v", err) + // Should have emitted a warning + if compiler.GetWarningCount() != initialWarnings+1 { + t.Errorf("Expected warning count to increase by 1, got %d warnings", compiler.GetWarningCount()-initialWarnings) } }) - t.Run("truly custom domains are allowed without errors", func(t *testing.T) { + t.Run("truly custom domains are allowed without errors or warnings", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -412,13 +384,18 @@ func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("copilot", networkPerms, nil) if err != nil { t.Errorf("Expected no error for truly custom domain in strict mode, got: %v", err) } + // Should NOT have emitted a warning + if compiler.GetWarningCount() != initialWarnings { + t.Errorf("Expected no warnings for truly custom domain, got %d warnings", compiler.GetWarningCount()-initialWarnings) + } }) - t.Run("mixed custom and ecosystem domains shows suggestions only for ecosystem domains", func(t *testing.T) { + t.Run("mixed custom and ecosystem domains shows warnings only for ecosystem domains", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -429,33 +406,18 @@ func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("copilot", networkPerms, nil) - if err == nil { - t.Fatal("Expected error for mixed domains in strict mode, got nil") - } - - // Should suggest 'python' for pypi.org but not mention custom-domain.com in suggestions - if !strings.Contains(err.Error(), "pypi.org") { - t.Errorf("Error should mention domain 'pypi.org', got: %v", err) - } - if !strings.Contains(err.Error(), "python") { - t.Errorf("Error should suggest 'python' ecosystem, got: %v", err) - } - if !strings.Contains(err.Error(), "Did you mean") { - t.Errorf("Error should include 'Did you mean' suggestion, got: %v", err) + if err != nil { + t.Errorf("Expected no error for mixed domains in strict mode, got: %v", err) } - // custom-domain.com should NOT appear in the "Did you mean" part - errMsg := err.Error() - didYouMeanIdx := strings.Index(errMsg, "Did you mean") - if didYouMeanIdx != -1 { - didYouMeanPart := errMsg[didYouMeanIdx:] - if strings.Contains(didYouMeanPart, "custom-domain.com") { - t.Errorf("Error should not suggest ecosystem for custom-domain.com, got: %v", err) - } + // Should have emitted a warning for pypi.org only + if compiler.GetWarningCount() != initialWarnings+1 { + t.Errorf("Expected warning count to increase by 1, got %d warnings", compiler.GetWarningCount()-initialWarnings) } }) - t.Run("allows ecosystem identifiers without suggestions", func(t *testing.T) { + t.Run("allows ecosystem identifiers without warnings", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -466,10 +428,15 @@ func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("copilot", networkPerms, nil) if err != nil { t.Errorf("Expected no error for ecosystem identifiers in strict mode, got: %v", err) } + // Should NOT have emitted any warnings + if compiler.GetWarningCount() != initialWarnings { + t.Errorf("Expected no warnings for ecosystem identifiers, got %d warnings", compiler.GetWarningCount()-initialWarnings) + } }) } @@ -526,7 +493,7 @@ func TestValidateStrictFirewall_CustomDomainBehavior(t *testing.T) { } }) - t.Run("ecosystem domain without identifier is rejected even with custom domains", func(t *testing.T) { + t.Run("ecosystem domain with custom domains emits warning for ecosystem domain only", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -537,23 +504,18 @@ func TestValidateStrictFirewall_CustomDomainBehavior(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("copilot", networkPerms, nil) - if err == nil { - t.Fatal("Expected error for ecosystem domain without identifier, got nil") - } - if !strings.Contains(err.Error(), "pypi.org") { - t.Errorf("Error should mention 'pypi.org', got: %v", err) - } - if !strings.Contains(err.Error(), "python") { - t.Errorf("Error should suggest 'python' ecosystem, got: %v", err) + if err != nil { + t.Errorf("Expected no error for ecosystem domain with custom domain, got: %v", err) } - // Should still mention that truly custom domains are allowed - if !strings.Contains(err.Error(), "Truly custom domains") { - t.Errorf("Error should mention that truly custom domains are allowed, got: %v", err) + // Should have emitted a warning for pypi.org + if compiler.GetWarningCount() != initialWarnings+1 { + t.Errorf("Expected warning count to increase by 1, got %d warnings", compiler.GetWarningCount()-initialWarnings) } }) - t.Run("defaults with custom domains are allowed", func(t *testing.T) { + t.Run("defaults with custom domains are allowed without warnings", func(t *testing.T) { compiler := NewCompiler() compiler.strictMode = true @@ -564,9 +526,14 @@ func TestValidateStrictFirewall_CustomDomainBehavior(t *testing.T) { }, } + initialWarnings := compiler.GetWarningCount() err := compiler.validateStrictFirewall("copilot", networkPerms, nil) if err != nil { t.Errorf("Expected no error for defaults with custom domains, got: %v", err) } + // Should NOT have emitted any warnings + if compiler.GetWarningCount() != initialWarnings { + t.Errorf("Expected no warnings for defaults with custom domains, got %d warnings", compiler.GetWarningCount()-initialWarnings) + } }) } diff --git a/pkg/workflow/strict_mode_validation.go b/pkg/workflow/strict_mode_validation.go index b15012ee2de..22de1dcdf57 100644 --- a/pkg/workflow/strict_mode_validation.go +++ b/pkg/workflow/strict_mode_validation.go @@ -41,8 +41,10 @@ package workflow import ( "fmt" + "os" "strings" + "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" ) @@ -344,9 +346,9 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N return fmt.Errorf("strict mode: 'sandbox.agent: false' is not allowed because it disables the agent sandbox firewall. This removes important security protections. Remove 'sandbox.agent: false' or set 'strict: false' to disable strict mode. See: https://github.github.com/gh-aw/reference/sandbox/") } - // In strict mode, ALL engines must use ecosystem identifiers for domains that belong to known ecosystems + // In strict mode, suggest using ecosystem identifiers for domains that belong to known ecosystems // This applies regardless of LLM gateway support - // Truly custom domains (domains not part of any known ecosystem) are allowed + // Both ecosystem domains and truly custom domains are allowed, but we warn about ecosystem domains if networkPermissions != nil && len(networkPermissions.Allowed) > 0 { strictModeValidationLog.Printf("Validating network domains in strict mode for all engines") @@ -378,7 +380,7 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N if ecosystem != "" { // This domain belongs to a known ecosystem but was not specified as an ecosystem identifier - // In strict mode, we require ecosystem identifiers for ecosystem domains + // In strict mode, we suggest using ecosystem identifiers instead ecosystemDomainsNotAsIdentifiers = append(ecosystemDomainsNotAsIdentifiers, domainSuggestion{domain: domain, ecosystem: ecosystem}) } else { // This is a truly custom domain (not part of any known ecosystem) - allowed in strict mode @@ -387,24 +389,19 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N } if len(ecosystemDomainsNotAsIdentifiers) > 0 { - strictModeValidationLog.Printf("Engine '%s' has ecosystem domains not specified as identifiers in strict mode, failing validation", engineID) + strictModeValidationLog.Printf("Engine '%s' has ecosystem domains not specified as identifiers in strict mode, emitting warning", engineID) - // Build error message with ecosystem suggestions - errorMsg := "strict mode: domains that belong to known ecosystems must be specified using ecosystem identifiers (e.g., 'python', 'node') instead of individual domain names for security and maintainability." - - // Add suggestions for domains that belong to known ecosystems + // Build warning message with ecosystem suggestions var suggestions []string for _, ds := range ecosystemDomainsNotAsIdentifiers { - suggestions = append(suggestions, fmt.Sprintf("'%s' belongs to ecosystem '%s'", ds.domain, ds.ecosystem)) - } - - if len(suggestions) > 0 { - errorMsg += " Did you mean: " + strings.Join(suggestions, ", ") + "?" + suggestions = append(suggestions, fmt.Sprintf("'%s' → '%s'", ds.domain, ds.ecosystem)) } - errorMsg += " Truly custom domains (not part of known ecosystems) are allowed. See: https://github.github.com/gh-aw/reference/network/" + warningMsg := fmt.Sprintf("strict mode: recommend using ecosystem identifiers instead of individual domain names for better maintainability: %s", strings.Join(suggestions, ", ")) - return fmt.Errorf("%s", errorMsg) + // Print warning message and increment warning count + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warningMsg)) + c.IncrementWarningCount() } }