From 88c65f2ff9ed03a0f0134f98efb4c5175b5f3d51 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 02:30:57 +0000 Subject: [PATCH 1/6] Initial plan From d7fc6e8a6ecd67b994fd720466072dfd75062443 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 02:41:13 +0000 Subject: [PATCH 2/6] Validate single-word network allows are known ecosystem identifiers Fixes the issue where misspelled ecosystem identifiers (e.g., 'rustxxxx', 'defaults2') were silently accepted in network.allowed. Now any single-word identifier that doesn't match a known ecosystem name produces a clear validation error with a list of valid ecosystem identifiers. - Modified validateNetworkAllowedDomains() to check unknown ecosystem-like identifiers via getEcosystemDomains() and return a ValidationError - Added comprehensive tests covering known valid identifiers, unknown identifiers that should fail, valid domain names, and mixed lists Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5e7d08f0-85c6-42b9-be70-0a8bfb06f5ed Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> --- pkg/workflow/network_firewall_validation.go | 19 +++++- .../network_firewall_validation_test.go | 65 +++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/network_firewall_validation.go b/pkg/workflow/network_firewall_validation.go index d68b683d719..9d98cd2beb3 100644 --- a/pkg/workflow/network_firewall_validation.go +++ b/pkg/workflow/network_firewall_validation.go @@ -71,9 +71,24 @@ func (c *Compiler) validateNetworkAllowedDomains(network *NetworkPermissions) er continue } - // Skip ecosystem identifiers - they don't need domain pattern validation + // Check if this looks like an ecosystem identifier (single lowercase word with optional hyphens) if isEcosystemIdentifier(domain) { - networkFirewallValidationLog.Printf("Skipping ecosystem identifier: %s", domain) + // Validate it's a known ecosystem identifier + if len(getEcosystemDomains(domain)) > 0 { + networkFirewallValidationLog.Printf("Skipping known ecosystem identifier: %s", domain) + continue + } + // Unknown ecosystem identifier - error + networkFirewallValidationLog.Printf("Validation error: unknown ecosystem identifier: %s", domain) + wrappedErr := fmt.Errorf("network.allowed[%d]: %w", i, NewValidationError( + "network.allowed", + domain, + fmt.Sprintf("'%s' is not a valid ecosystem identifier", domain), + "Use a valid ecosystem identifier or a domain name containing a dot (e.g., 'example.com').\n\nValid ecosystem identifiers: defaults, github, python, node, go, rust, java, ruby, dotnet, php, swift, kotlin, scala, clojure, dart, elixir, haskell, perl, zig, containers, chrome, playwright, terraform, dev-tools, linux-distros, local, fonts, github-actions, node-cdns, python-native, deno, default-safe-outputs, lean, latex, lua, julia, ocaml, bazel, powershell, r, threat-detection", + )) + if returnErr := collector.Add(wrappedErr); returnErr != nil { + return returnErr // Fail-fast mode + } continue } diff --git a/pkg/workflow/network_firewall_validation_test.go b/pkg/workflow/network_firewall_validation_test.go index 870dee7ec67..f193842dced 100644 --- a/pkg/workflow/network_firewall_validation_test.go +++ b/pkg/workflow/network_firewall_validation_test.go @@ -151,6 +151,71 @@ func TestValidateNetworkFirewallConfig_AllowURLsRequiresSSLBump(t *testing.T) { }) } +func TestValidateNetworkAllowedDomains_EcosystemIdentifiers(t *testing.T) { + compiler := NewCompiler() + + t.Run("known ecosystem identifiers pass validation", func(t *testing.T) { + validEcosystems := []string{ + "defaults", "github", "python", "node", "go", "rust", "java", "ruby", + "dotnet", "php", "swift", "kotlin", "scala", "clojure", "dart", + "elixir", "haskell", "perl", "zig", "containers", "chrome", + "playwright", "terraform", "dev-tools", "linux-distros", "local", + "fonts", "github-actions", "node-cdns", "python-native", "deno", + "default-safe-outputs", "lean", "latex", "lua", "julia", "ocaml", + "bazel", "powershell", "r", "threat-detection", + } + for _, ecosystem := range validEcosystems { + t.Run(ecosystem, func(t *testing.T) { + network := &NetworkPermissions{Allowed: []string{ecosystem}} + err := compiler.validateNetworkAllowedDomains(network) + assert.NoError(t, err, "Known ecosystem identifier '%s' should pass validation", ecosystem) + }) + } + }) + + t.Run("unknown single-word identifiers fail validation", func(t *testing.T) { + invalidEcosystems := []string{ + "rustxxxx", + "defaults2", + "githubx", + "nodexyz", + "unknown", + "fakeecosystem", + "pypi", + "npm", + } + for _, ecosystem := range invalidEcosystems { + t.Run(ecosystem, func(t *testing.T) { + network := &NetworkPermissions{Allowed: []string{ecosystem}} + err := compiler.validateNetworkAllowedDomains(network) + require.Error(t, err, "Unknown ecosystem identifier '%s' should fail validation", ecosystem) + assert.Contains(t, err.Error(), "not a valid ecosystem identifier", "Error should indicate invalid ecosystem identifier") + }) + } + }) + + t.Run("domain names with dots still pass validation", func(t *testing.T) { + validDomains := []string{"github.com", "api.example.com", "*.example.org"} + for _, domain := range validDomains { + t.Run(domain, func(t *testing.T) { + network := &NetworkPermissions{Allowed: []string{domain}} + err := compiler.validateNetworkAllowedDomains(network) + assert.NoError(t, err, "Valid domain '%s' should pass validation", domain) + }) + } + }) + + t.Run("mixed valid and invalid entries collects all errors", func(t *testing.T) { + network := &NetworkPermissions{ + Allowed: []string{"defaults", "rustxxxx", "github.com", "fakeecosystem"}, + } + err := compiler.validateNetworkAllowedDomains(network) + require.Error(t, err, "Should fail when invalid ecosystem identifiers are present") + assert.Contains(t, err.Error(), "rustxxxx", "Error should mention the invalid identifier") + assert.Contains(t, err.Error(), "fakeecosystem", "Error should mention the other invalid identifier") + }) +} + func TestValidateNetworkFirewallConfig_Integration(t *testing.T) { t.Run("compiler rejects workflow with allow-urls but no ssl-bump", func(t *testing.T) { compiler := NewCompiler() From b589959e6ad492cfee282fcc0f46e0177ce23ffd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 02:43:53 +0000 Subject: [PATCH 3/6] Use dynamic list of ecosystem identifiers in error message Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5e7d08f0-85c6-42b9-be70-0a8bfb06f5ed Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> --- pkg/workflow/network_firewall_validation.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/network_firewall_validation.go b/pkg/workflow/network_firewall_validation.go index 9d98cd2beb3..e2c561304f7 100644 --- a/pkg/workflow/network_firewall_validation.go +++ b/pkg/workflow/network_firewall_validation.go @@ -15,6 +15,7 @@ package workflow import ( "fmt" "regexp" + "sort" "strings" "github.com/github/gh-aw/pkg/constants" @@ -84,7 +85,7 @@ func (c *Compiler) validateNetworkAllowedDomains(network *NetworkPermissions) er "network.allowed", domain, fmt.Sprintf("'%s' is not a valid ecosystem identifier", domain), - "Use a valid ecosystem identifier or a domain name containing a dot (e.g., 'example.com').\n\nValid ecosystem identifiers: defaults, github, python, node, go, rust, java, ruby, dotnet, php, swift, kotlin, scala, clojure, dart, elixir, haskell, perl, zig, containers, chrome, playwright, terraform, dev-tools, linux-distros, local, fonts, github-actions, node-cdns, python-native, deno, default-safe-outputs, lean, latex, lua, julia, ocaml, bazel, powershell, r, threat-detection", + "Use a valid ecosystem identifier or a domain name containing a dot (e.g., 'example.com').\n\nValid ecosystem identifiers: "+strings.Join(getValidEcosystemIdentifiers(), ", "), )) if returnErr := collector.Add(wrappedErr); returnErr != nil { return returnErr // Fail-fast mode @@ -120,6 +121,20 @@ func isEcosystemIdentifier(domain string) bool { return isEcosystemIdentifierPattern.MatchString(domain) } +// getValidEcosystemIdentifiers returns a sorted list of all valid ecosystem identifiers, +// including both the base identifiers from ecosystemDomains and compound identifiers. +func getValidEcosystemIdentifiers() []string { + ids := make([]string, 0, len(ecosystemDomains)+len(compoundEcosystems)) + for id := range ecosystemDomains { + ids = append(ids, id) + } + for id := range compoundEcosystems { + ids = append(ids, id) + } + sort.Strings(ids) + return ids +} + // domainPattern validates domain patterns including wildcards // Valid patterns: // - Plain domains: github.com, api.github.com From 8860a742f75fa912da9e920f44968ccf4fa831aa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 02:45:02 +0000 Subject: [PATCH 4/6] Use getValidEcosystemIdentifiers() in tests to stay in sync with production code Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5e7d08f0-85c6-42b9-be70-0a8bfb06f5ed Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> --- pkg/workflow/network_firewall_validation_test.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/pkg/workflow/network_firewall_validation_test.go b/pkg/workflow/network_firewall_validation_test.go index f193842dced..82cca37ee70 100644 --- a/pkg/workflow/network_firewall_validation_test.go +++ b/pkg/workflow/network_firewall_validation_test.go @@ -155,16 +155,8 @@ func TestValidateNetworkAllowedDomains_EcosystemIdentifiers(t *testing.T) { compiler := NewCompiler() t.Run("known ecosystem identifiers pass validation", func(t *testing.T) { - validEcosystems := []string{ - "defaults", "github", "python", "node", "go", "rust", "java", "ruby", - "dotnet", "php", "swift", "kotlin", "scala", "clojure", "dart", - "elixir", "haskell", "perl", "zig", "containers", "chrome", - "playwright", "terraform", "dev-tools", "linux-distros", "local", - "fonts", "github-actions", "node-cdns", "python-native", "deno", - "default-safe-outputs", "lean", "latex", "lua", "julia", "ocaml", - "bazel", "powershell", "r", "threat-detection", - } - for _, ecosystem := range validEcosystems { + // Use getValidEcosystemIdentifiers() to stay in sync with production code + for _, ecosystem := range getValidEcosystemIdentifiers() { t.Run(ecosystem, func(t *testing.T) { network := &NetworkPermissions{Allowed: []string{ecosystem}} err := compiler.validateNetworkAllowedDomains(network) From 0e032844438f55c20c138238e2b484ded52598a9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 02:46:09 +0000 Subject: [PATCH 5/6] Use obviously-invalid names in ecosystem tests to avoid false positives Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5e7d08f0-85c6-42b9-be70-0a8bfb06f5ed Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> --- pkg/workflow/network_firewall_validation_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/network_firewall_validation_test.go b/pkg/workflow/network_firewall_validation_test.go index 82cca37ee70..0e97e45255d 100644 --- a/pkg/workflow/network_firewall_validation_test.go +++ b/pkg/workflow/network_firewall_validation_test.go @@ -173,8 +173,8 @@ func TestValidateNetworkAllowedDomains_EcosystemIdentifiers(t *testing.T) { "nodexyz", "unknown", "fakeecosystem", - "pypi", - "npm", + "notarealecosystem", + "invalidname123", } for _, ecosystem := range invalidEcosystems { t.Run(ecosystem, func(t *testing.T) { From 092bbde945e69995f0465ce1c671a45cda098c77 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 03:26:49 +0000 Subject: [PATCH 6/6] Perf: use isKnownEcosystemIdentifier helper; update network.md docs Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a8e405d8-e362-4db4-b883-8a88af4bc052 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/aw/network.md | 4 ++-- pkg/workflow/network_firewall_validation.go | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/.github/aw/network.md b/.github/aw/network.md index c20ee3e9ed7..756dbeb78ec 100644 --- a/.github/aw/network.md +++ b/.github/aw/network.md @@ -40,7 +40,7 @@ Each entry in `network.allowed` must be one of: | **Exact domain** | `api.example.com`, `registry.npmjs.org` | Must be a fully-qualified domain name (FQDN) | | **Wildcard subdomain** | `*.example.com` | Matches `sub.example.com`, `deep.nested.example.com`, and `example.com` itself | -> ⚠️ **Bare shorthands like `npm`, `pypi`, or `localhost` are NOT valid** unless they are listed in the ecosystem identifiers table below. Use ecosystem identifiers (`node`, `python`) or explicit FQDNs (`registry.npmjs.org`, `pypi.org`) instead. +> ⚠️ **Bare shorthands like `npm`, `pypi`, or `localhost` are NOT valid** unless they are listed in the ecosystem identifiers table below. Using an unrecognised single-word entry causes a **compile-time error**. Use ecosystem identifiers (`node`, `python`) or explicit FQDNs (`registry.npmjs.org`, `pypi.org`) instead. ## Ecosystem Identifiers @@ -87,7 +87,7 @@ These keywords expand to curated lists of domains maintained by gh-aw: ## Invalid Shorthands -These values look like they might work but are **not valid** ecosystem identifiers and will be passed through as literal domain names (and will almost certainly not match any real host): +These values look like ecosystem identifiers but are **not recognised** — using them in `network.allowed` causes a **compile-time error**: | Invalid value | What you probably meant | Correct value | |---|---|---| diff --git a/pkg/workflow/network_firewall_validation.go b/pkg/workflow/network_firewall_validation.go index e2c561304f7..b9409f7a9dd 100644 --- a/pkg/workflow/network_firewall_validation.go +++ b/pkg/workflow/network_firewall_validation.go @@ -74,8 +74,8 @@ func (c *Compiler) validateNetworkAllowedDomains(network *NetworkPermissions) er // Check if this looks like an ecosystem identifier (single lowercase word with optional hyphens) if isEcosystemIdentifier(domain) { - // Validate it's a known ecosystem identifier - if len(getEcosystemDomains(domain)) > 0 { + // Validate it's a known ecosystem identifier using a direct map lookup to avoid allocations + if isKnownEcosystemIdentifier(domain) { networkFirewallValidationLog.Printf("Skipping known ecosystem identifier: %s", domain) continue } @@ -121,6 +121,17 @@ func isEcosystemIdentifier(domain string) bool { return isEcosystemIdentifierPattern.MatchString(domain) } +// isKnownEcosystemIdentifier reports whether id is a recognised ecosystem identifier. +// It checks the base ecosystemDomains map and the compoundEcosystems map directly, +// avoiding the allocations that getEcosystemDomains incurs. +func isKnownEcosystemIdentifier(id string) bool { + if _, ok := ecosystemDomains[id]; ok { + return true + } + _, ok := compoundEcosystems[id] + return ok +} + // getValidEcosystemIdentifiers returns a sorted list of all valid ecosystem identifiers, // including both the base identifiers from ecosystemDomains and compound identifiers. func getValidEcosystemIdentifiers() []string {