From 5d952224690bde50c5f840e5374f84b0a0d8b2f1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Sep 2025 12:53:03 +0000 Subject: [PATCH 1/3] Initial plan From 2c62dd92f0104a71b680b985ed512f8245f58ffd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Sep 2025 13:09:57 +0000 Subject: [PATCH 2/3] Fix: Ensure localhost domains are always included in Playwright allowed_domains - Add ensureLocalhostDomains helper functions in parser and workflow packages - Modify mcp.go to preserve localhost/127.0.0.1 when custom domains are specified - Modify compiler.go to preserve localhost/127.0.0.1 when custom domains are specified - Update existing tests to expect localhost domains alongside custom domains - Add comprehensive tests for the new helper functions - Manual verification shows fix working correctly in compiled workflows Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/mcp.go | 43 +++++++++++++++++++++--- pkg/parser/mcp_test.go | 53 ++++++++++++++++++++++++++++-- pkg/workflow/compiler.go | 36 +++++++++++++++++++- pkg/workflow/custom_engine_test.go | 51 ++++++++++++++++++++++++++-- 4 files changed, 173 insertions(+), 10 deletions(-) diff --git a/pkg/parser/mcp.go b/pkg/parser/mcp.go index af89241db9..91f7910d7b 100644 --- a/pkg/parser/mcp.go +++ b/pkg/parser/mcp.go @@ -8,6 +8,37 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) +// ensureLocalhostDomains ensures that localhost and 127.0.0.1 are always included +// in the allowed domains list for Playwright, even when custom domains are specified +func ensureLocalhostDomains(domains []string) []string { + hasLocalhost := false + hasLoopback := false + + for _, domain := range domains { + if domain == "localhost" { + hasLocalhost = true + } + if domain == "127.0.0.1" { + hasLoopback = true + } + } + + result := make([]string, 0, len(domains)+2) + + // Always add localhost domains first + if !hasLocalhost { + result = append(result, "localhost") + } + if !hasLoopback { + result = append(result, "127.0.0.1") + } + + // Add the rest of the domains + result = append(result, domains...) + + return result +} + // MCPServerConfig represents a parsed MCP server configuration type MCPServerConfig struct { Name string `json:"name"` @@ -172,19 +203,23 @@ func ExtractMCPConfigurations(frontmatter map[string]any, serverFilter string) ( if domainsConfig, exists := toolConfig["allowed_domains"]; exists { // For now, we'll use a simple conversion. In a full implementation, // we'd need to use the same domain bundle resolution as the compiler + var customDomains []string switch domains := domainsConfig.(type) { case []string: - allowedDomains = domains + customDomains = domains case []any: - allowedDomains = make([]string, len(domains)) + customDomains = make([]string, len(domains)) for i, domain := range domains { if domainStr, ok := domain.(string); ok { - allowedDomains[i] = domainStr + customDomains[i] = domainStr } } case string: - allowedDomains = []string{domains} + customDomains = []string{domains} } + + // Ensure localhost domains are always included + allowedDomains = ensureLocalhostDomains(customDomains) } // Check for custom Docker image version diff --git a/pkg/parser/mcp_test.go b/pkg/parser/mcp_test.go index ebca37e26e..ce7689ec11 100644 --- a/pkg/parser/mcp_test.go +++ b/pkg/parser/mcp_test.go @@ -6,6 +6,55 @@ import ( "testing" ) +// TestEnsureLocalhostDomains tests the helper function that ensures localhost domains are always included +func TestEnsureLocalhostDomains(t *testing.T) { + tests := []struct { + name string + input []string + expected []string + }{ + { + name: "Empty input should add both localhost domains", + input: []string{}, + expected: []string{"localhost", "127.0.0.1"}, + }, + { + name: "Custom domains without localhost should add localhost domains", + input: []string{"github.com", "*.github.com"}, + expected: []string{"localhost", "127.0.0.1", "github.com", "*.github.com"}, + }, + { + name: "Input with localhost but no 127.0.0.1 should add 127.0.0.1", + input: []string{"localhost", "example.com"}, + expected: []string{"127.0.0.1", "localhost", "example.com"}, + }, + { + name: "Input with 127.0.0.1 but no localhost should add localhost", + input: []string{"127.0.0.1", "example.com"}, + expected: []string{"localhost", "127.0.0.1", "example.com"}, + }, + { + name: "Input with both localhost domains should remain unchanged", + input: []string{"localhost", "127.0.0.1", "example.com"}, + expected: []string{"localhost", "127.0.0.1", "example.com"}, + }, + { + name: "Input with both in different order should just add what's missing", + input: []string{"example.com", "127.0.0.1", "localhost"}, + expected: []string{"example.com", "127.0.0.1", "localhost"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ensureLocalhostDomains(tt.input) + if !reflect.DeepEqual(result, tt.expected) { + t.Errorf("ensureLocalhostDomains(%v) = %v, want %v", tt.input, result, tt.expected) + } + }) + } +} + func TestExtractMCPConfigurations(t *testing.T) { tests := []struct { name string @@ -91,7 +140,7 @@ func TestExtractMCPConfigurations(t *testing.T) { "-e", "PLAYWRIGHT_ALLOWED_DOMAINS", "mcr.microsoft.com/playwright:latest", }, - Env: map[string]string{"PLAYWRIGHT_ALLOWED_DOMAINS": "github.com,*.github.com"}, + Env: map[string]string{"PLAYWRIGHT_ALLOWED_DOMAINS": "localhost,127.0.0.1,github.com,*.github.com"}, }, }, }, @@ -115,7 +164,7 @@ func TestExtractMCPConfigurations(t *testing.T) { "-e", "PLAYWRIGHT_ALLOWED_DOMAINS", "mcr.microsoft.com/playwright:v1.41.0", }, - Env: map[string]string{"PLAYWRIGHT_ALLOWED_DOMAINS": "example.com"}, + Env: map[string]string{"PLAYWRIGHT_ALLOWED_DOMAINS": "localhost,127.0.0.1,example.com"}, }, }, }, diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 5011a48f6a..62d28b23e8 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -2996,6 +2996,37 @@ func getPlaywrightDockerImageVersion(playwrightTool any) string { return playwrightDockerImageVersion } +// ensureLocalhostDomainsWorkflow ensures that localhost and 127.0.0.1 are always included +// in the allowed domains list for Playwright, even when custom domains are specified +func ensureLocalhostDomainsWorkflow(domains []string) []string { + hasLocalhost := false + hasLoopback := false + + for _, domain := range domains { + if domain == "localhost" { + hasLocalhost = true + } + if domain == "127.0.0.1" { + hasLoopback = true + } + } + + result := make([]string, 0, len(domains)+2) + + // Always add localhost domains first + if !hasLocalhost { + result = append(result, "localhost") + } + if !hasLoopback { + result = append(result, "127.0.0.1") + } + + // Add the rest of the domains + result = append(result, domains...) + + return result +} + // generatePlaywrightAllowedDomains extracts domain list from Playwright tool configuration with bundle resolution // Uses the same domain bundle resolution as top-level network configuration, defaulting to localhost only func generatePlaywrightAllowedDomains(playwrightTool any, networkPermissions *NetworkPermissions) []string { @@ -3026,7 +3057,10 @@ func generatePlaywrightAllowedDomains(playwrightTool any, networkPermissions *Ne } // Use the same domain bundle resolution as the top-level network configuration - allowedDomains = GetAllowedDomains(playwrightNetwork) + resolvedDomains := GetAllowedDomains(playwrightNetwork) + + // Ensure localhost domains are always included + allowedDomains = ensureLocalhostDomainsWorkflow(resolvedDomains) } } diff --git a/pkg/workflow/custom_engine_test.go b/pkg/workflow/custom_engine_test.go index e3b17c10ff..b28fac9a25 100644 --- a/pkg/workflow/custom_engine_test.go +++ b/pkg/workflow/custom_engine_test.go @@ -1,10 +1,55 @@ package workflow import ( + "reflect" "strings" "testing" ) +// TestEnsureLocalhostDomainsWorkflow tests the helper function that ensures localhost domains are always included +func TestEnsureLocalhostDomainsWorkflow(t *testing.T) { + tests := []struct { + name string + input []string + expected []string + }{ + { + name: "Empty input should add both localhost domains", + input: []string{}, + expected: []string{"localhost", "127.0.0.1"}, + }, + { + name: "Custom domains without localhost should add localhost domains", + input: []string{"github.com", "*.github.com"}, + expected: []string{"localhost", "127.0.0.1", "github.com", "*.github.com"}, + }, + { + name: "Input with localhost but no 127.0.0.1 should add 127.0.0.1", + input: []string{"localhost", "example.com"}, + expected: []string{"127.0.0.1", "localhost", "example.com"}, + }, + { + name: "Input with 127.0.0.1 but no localhost should add localhost", + input: []string{"127.0.0.1", "example.com"}, + expected: []string{"localhost", "127.0.0.1", "example.com"}, + }, + { + name: "Input with both localhost domains should remain unchanged", + input: []string{"localhost", "127.0.0.1", "example.com"}, + expected: []string{"localhost", "127.0.0.1", "example.com"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ensureLocalhostDomainsWorkflow(tt.input) + if !reflect.DeepEqual(result, tt.expected) { + t.Errorf("ensureLocalhostDomainsWorkflow(%v) = %v, want %v", tt.input, result, tt.expected) + } + }) + } +} + func TestCustomEngine(t *testing.T) { engine := NewCustomEngine() @@ -244,9 +289,9 @@ func TestCustomEngineRenderPlaywrightMCPConfigWithDomainConfiguration(t *testing t.Errorf("Expected --allowed-origins flag in npx args") } - // Check that it contains the specified domains - if !strings.Contains(output, "example.com,*.github.com") { - t.Errorf("Expected configured domains in --allowed-origins value") + // Check that it contains the specified domains AND localhost domains + if !strings.Contains(output, "localhost,127.0.0.1,example.com,*.github.com") { + t.Errorf("Expected configured domains with localhost in --allowed-origins value") } // Check that it does NOT contain the old format environment variables From ab897c37388a6fa0be947de3a12e86ce1954ac67 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Sep 2025 13:12:14 +0000 Subject: [PATCH 3/3] Fix: Ensure localhost domains are always included in Playwright allowed_domains Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../workflows/test-playwright-accessibility-contrast.lock.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/workflows/test-playwright-accessibility-contrast.lock.yml b/pkg/cli/workflows/test-playwright-accessibility-contrast.lock.yml index 245225fadf..6dfd597df9 100644 --- a/pkg/cli/workflows/test-playwright-accessibility-contrast.lock.yml +++ b/pkg/cli/workflows/test-playwright-accessibility-contrast.lock.yml @@ -614,7 +614,7 @@ jobs: "args": [ "@playwright/mcp@latest", "--allowed-origins", - "github.com,*.github.com" + "localhost,127.0.0.1,github.com,*.github.com" ] }, "safe_outputs": {