diff --git a/.github/workflows/blog-auditor.lock.yml b/.github/workflows/blog-auditor.lock.yml index 6ff1fd5f595..5745e58b84a 100644 --- a/.github/workflows/blog-auditor.lock.yml +++ b/.github/workflows/blog-auditor.lock.yml @@ -2315,6 +2315,8 @@ jobs: "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", + "localhost;localhost:*;127.0.0.1;127.0.0.1:*;githubnext.com;www.githubnext.com", + "--allowed-origins", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;githubnext.com;www.githubnext.com" ] }, diff --git a/.github/workflows/cloclo.lock.yml b/.github/workflows/cloclo.lock.yml index bdb8bf8d30a..d4136a91e88 100644 --- a/.github/workflows/cloclo.lock.yml +++ b/.github/workflows/cloclo.lock.yml @@ -3939,6 +3939,8 @@ jobs: "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", + "localhost;localhost:*;127.0.0.1;127.0.0.1:*", + "--allowed-origins", "localhost;localhost:*;127.0.0.1;127.0.0.1:*" ] }, diff --git a/.github/workflows/daily-multi-device-docs-tester.lock.yml b/.github/workflows/daily-multi-device-docs-tester.lock.yml index 5afa00df225..3ed4a2aad18 100644 --- a/.github/workflows/daily-multi-device-docs-tester.lock.yml +++ b/.github/workflows/daily-multi-device-docs-tester.lock.yml @@ -2132,6 +2132,8 @@ jobs: "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", + "localhost;localhost:*;127.0.0.1;127.0.0.1:*", + "--allowed-origins", "localhost;localhost:*;127.0.0.1;127.0.0.1:*" ] }, diff --git a/.github/workflows/docs-noob-tester.lock.yml b/.github/workflows/docs-noob-tester.lock.yml index 1fe2eeb8461..202b4b6f67f 100644 --- a/.github/workflows/docs-noob-tester.lock.yml +++ b/.github/workflows/docs-noob-tester.lock.yml @@ -2042,7 +2042,7 @@ jobs: "playwright": { "type": "local", "command": "docker", - "args": ["run", "-i", "--rm", "--init", "mcr.microsoft.com/playwright/mcp", "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", "localhost;localhost:*;127.0.0.1;127.0.0.1:*"], + "args": ["run", "-i", "--rm", "--init", "mcr.microsoft.com/playwright/mcp", "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", "localhost;localhost:*;127.0.0.1;127.0.0.1:*", "--allowed-origins", "localhost;localhost:*;127.0.0.1;127.0.0.1:*"], "tools": ["*"] }, "safeoutputs": { diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 6a509aab029..92f7ddfd4cf 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -4013,6 +4013,8 @@ jobs: "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", + "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com", + "--allowed-origins", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com" ] }, diff --git a/.github/workflows/smoke-codex.lock.yml b/.github/workflows/smoke-codex.lock.yml index 8a946c17bc5..1e47177277b 100644 --- a/.github/workflows/smoke-codex.lock.yml +++ b/.github/workflows/smoke-codex.lock.yml @@ -3822,6 +3822,8 @@ jobs: "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", + "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com", + "--allowed-origins", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com" ] diff --git a/.github/workflows/smoke-copilot-no-firewall.lock.yml b/.github/workflows/smoke-copilot-no-firewall.lock.yml index 757ba72ec28..e240923c4f1 100644 --- a/.github/workflows/smoke-copilot-no-firewall.lock.yml +++ b/.github/workflows/smoke-copilot-no-firewall.lock.yml @@ -5167,7 +5167,7 @@ jobs: "playwright": { "type": "local", "command": "docker", - "args": ["run", "-i", "--rm", "--init", "mcr.microsoft.com/playwright/mcp", "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com"], + "args": ["run", "-i", "--rm", "--init", "mcr.microsoft.com/playwright/mcp", "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com", "--allowed-origins", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com"], "tools": ["*"] }, "safeinputs": { diff --git a/.github/workflows/smoke-copilot-playwright.lock.yml b/.github/workflows/smoke-copilot-playwright.lock.yml index f80c0a0f364..f70f78be42c 100644 --- a/.github/workflows/smoke-copilot-playwright.lock.yml +++ b/.github/workflows/smoke-copilot-playwright.lock.yml @@ -5157,7 +5157,7 @@ jobs: "playwright": { "type": "local", "command": "docker", - "args": ["run", "-i", "--rm", "--init", "mcr.microsoft.com/playwright/mcp", "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com", "--save-trace"], + "args": ["run", "-i", "--rm", "--init", "mcr.microsoft.com/playwright/mcp", "--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com", "--allowed-origins", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com", "--save-trace"], "tools": ["*"] }, "safeinputs": { diff --git a/.github/workflows/unbloat-docs.lock.yml b/.github/workflows/unbloat-docs.lock.yml index 5d6b7cadaea..b32843e3734 100644 --- a/.github/workflows/unbloat-docs.lock.yml +++ b/.github/workflows/unbloat-docs.lock.yml @@ -3665,6 +3665,8 @@ jobs: "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", "localhost;localhost:*;127.0.0.1;127.0.0.1:*", + "--allowed-origins", + "localhost;localhost:*;127.0.0.1;127.0.0.1:*", "--viewport-size", "1920x1080" ] diff --git a/pkg/workflow/custom_engine_test.go b/pkg/workflow/custom_engine_test.go index 19cacc7fb6b..16b5f9766d7 100644 --- a/pkg/workflow/custom_engine_test.go +++ b/pkg/workflow/custom_engine_test.go @@ -305,15 +305,20 @@ func TestCustomEngineRenderPlaywrightMCPConfigWithDomainConfiguration(t *testing t.Errorf("Expected official Playwright MCP Docker image in output") } - // Check that it contains --allowed-hosts flag when domains are configured + // Check that it contains --allowed-hosts and --allowed-origins flags when domains are configured if !strings.Contains(output, "--allowed-hosts") { t.Errorf("Expected --allowed-hosts flag in docker args") } + if !strings.Contains(output, "--allowed-origins") { + t.Errorf("Expected --allowed-origins flag in docker args") + } // Check that it contains the specified domains AND localhost domains with port variations // Domains should be sorted alphabetically: *.github.com, example.com - if !strings.Contains(output, "localhost;localhost:*;127.0.0.1;127.0.0.1:*;*.github.com;example.com") { - t.Errorf("Expected configured domains with localhost and port variations in --allowed-hosts value (sorted)") + // Both flags should have the same domain list + expectedDomains := "localhost;localhost:*;127.0.0.1;127.0.0.1:*;*.github.com;example.com" + if !strings.Contains(output, expectedDomains) { + t.Errorf("Expected configured domains with localhost and port variations in --allowed-hosts and --allowed-origins values (sorted)") } // Check that it does NOT contain the old format environment variables @@ -357,14 +362,19 @@ func TestCustomEngineRenderPlaywrightMCPConfigDefaultDomains(t *testing.T) { t.Errorf("Expected official Playwright MCP Docker image in output") } - // Check that it contains --allowed-hosts flag for default domains + // Check that it contains --allowed-hosts and --allowed-origins flags for default domains if !strings.Contains(output, "--allowed-hosts") { t.Errorf("Expected --allowed-hosts flag in docker args") } + if !strings.Contains(output, "--allowed-origins") { + t.Errorf("Expected --allowed-origins flag in docker args") + } // Check that it contains default domains with port variations (localhost, localhost:*, 127.0.0.1, 127.0.0.1:*) - if !strings.Contains(output, "localhost;localhost:*;127.0.0.1;127.0.0.1:*") { - t.Errorf("Expected default domains with port variations in --allowed-hosts value") + // Both flags should have the same domain list + expectedDomains := "localhost;localhost:*;127.0.0.1;127.0.0.1:*" + if !strings.Contains(output, expectedDomains) { + t.Errorf("Expected default domains with port variations in --allowed-hosts and --allowed-origins values") } // Check that it does NOT contain the old format environment variables diff --git a/pkg/workflow/mcp-config.go b/pkg/workflow/mcp-config.go index d9ad88de0cc..1edafcef4c7 100644 --- a/pkg/workflow/mcp-config.go +++ b/pkg/workflow/mcp-config.go @@ -54,7 +54,9 @@ func renderPlaywrightMCPConfigWithOptions(yaml *strings.Builder, playwrightTool // Inline format for Copilot yaml.WriteString(" \"args\": [\"run\", \"-i\", \"--rm\", \"--init\", \"" + playwrightImage + "\", \"--output-dir\", \"/tmp/gh-aw/mcp-logs/playwright\"") if len(allowedDomains) > 0 { - yaml.WriteString(", \"--allowed-hosts\", \"" + strings.Join(allowedDomains, ";") + "\"") + domainsStr := strings.Join(allowedDomains, ";") + yaml.WriteString(", \"--allowed-hosts\", \"" + domainsStr + "\"") + yaml.WriteString(", \"--allowed-origins\", \"" + domainsStr + "\"") } // Append custom args if present writeArgsToYAMLInline(yaml, customArgs) @@ -70,9 +72,12 @@ func renderPlaywrightMCPConfigWithOptions(yaml *strings.Builder, playwrightTool yaml.WriteString(" \"--output-dir\",\n") yaml.WriteString(" \"/tmp/gh-aw/mcp-logs/playwright\"") if len(allowedDomains) > 0 { + domainsStr := strings.Join(allowedDomains, ";") yaml.WriteString(",\n") yaml.WriteString(" \"--allowed-hosts\",\n") - yaml.WriteString(" \"" + strings.Join(allowedDomains, ";") + "\"") + yaml.WriteString(" \"" + domainsStr + "\",\n") + yaml.WriteString(" \"--allowed-origins\",\n") + yaml.WriteString(" \"" + domainsStr + "\"") } // Append custom args if present writeArgsToYAML(yaml, customArgs, " ") @@ -276,9 +281,12 @@ func renderPlaywrightMCPConfigTOML(yaml *strings.Builder, playwrightTool any) { yaml.WriteString(" \"--output-dir\",\n") yaml.WriteString(" \"/tmp/gh-aw/mcp-logs/playwright\"") if len(args.AllowedDomains) > 0 { + domainsStr := strings.Join(args.AllowedDomains, ";") yaml.WriteString(",\n") yaml.WriteString(" \"--allowed-hosts\",\n") - yaml.WriteString(" \"" + strings.Join(args.AllowedDomains, ";") + "\"") + yaml.WriteString(" \"" + domainsStr + "\",\n") + yaml.WriteString(" \"--allowed-origins\",\n") + yaml.WriteString(" \"" + domainsStr + "\"") } // Append custom args if present diff --git a/pkg/workflow/mcp_config_refactor_test.go b/pkg/workflow/mcp_config_refactor_test.go index 52b6c77bade..ffad7a5b427 100644 --- a/pkg/workflow/mcp_config_refactor_test.go +++ b/pkg/workflow/mcp_config_refactor_test.go @@ -77,6 +77,7 @@ func TestRenderPlaywrightMCPConfigWithOptions(t *testing.T) { inlineArgs: true, expectedContent: []string{ `"--allowed-hosts"`, + `"--allowed-origins"`, `"localhost;localhost:*;127.0.0.1;127.0.0.1:*"`, // Default localhost is always added }, unexpectedContent: []string{}, @@ -296,6 +297,7 @@ func TestRenderPlaywrightMCPConfigTOML(t *testing.T) { }, expectedContent: []string{ `"--allowed-hosts"`, + `"--allowed-origins"`, `"localhost;localhost:*;127.0.0.1;127.0.0.1:*"`, // Default localhost is added }, }, diff --git a/pkg/workflow/mcp_config_shared_test.go b/pkg/workflow/mcp_config_shared_test.go index edf2a7cb40c..566bb38f5ad 100644 --- a/pkg/workflow/mcp_config_shared_test.go +++ b/pkg/workflow/mcp_config_shared_test.go @@ -31,6 +31,7 @@ func TestRenderPlaywrightMCPConfigShared(t *testing.T) { `"--output-dir"`, `"/tmp/gh-aw/mcp-logs/playwright"`, `"--allowed-hosts"`, + `"--allowed-origins"`, `example.com;test.com`, // Domains are joined with semicolons }, wantEnding: "},\n", diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index c87f4143de2..dd49128cf0f 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -132,9 +132,12 @@ func (r *MCPConfigRendererUnified) renderPlaywrightTOML(yaml *strings.Builder, p yaml.WriteString(" \"--output-dir\",\n") yaml.WriteString(" \"/tmp/gh-aw/mcp-logs/playwright\"") if len(args.AllowedDomains) > 0 { + domainsStr := strings.Join(args.AllowedDomains, ";") yaml.WriteString(",\n") yaml.WriteString(" \"--allowed-hosts\",\n") - yaml.WriteString(" \"" + strings.Join(args.AllowedDomains, ";") + "\"") + yaml.WriteString(" \"" + domainsStr + "\",\n") + yaml.WriteString(" \"--allowed-origins\",\n") + yaml.WriteString(" \"" + domainsStr + "\"") } // Append custom args if present diff --git a/pkg/workflow/playwright_mcp_integration_test.go b/pkg/workflow/playwright_mcp_integration_test.go index 34b9cde4d6d..b07ee5fbb78 100644 --- a/pkg/workflow/playwright_mcp_integration_test.go +++ b/pkg/workflow/playwright_mcp_integration_test.go @@ -11,7 +11,7 @@ import ( ) // TestPlaywrightMCPIntegration tests that compiled workflows generate correct Docker Playwright commands -// This test verifies that the official Playwright MCP Docker image is used with --allowed-hosts flag +// This test verifies that the official Playwright MCP Docker image is used with both --allowed-hosts and --allowed-origins flags func TestPlaywrightMCPIntegration(t *testing.T) { // Create a temporary directory for test files tmpDir, err := os.MkdirTemp("", "gh-aw-playwright-integration-*") @@ -23,8 +23,7 @@ func TestPlaywrightMCPIntegration(t *testing.T) { tests := []struct { name string workflowContent string - expectedFlag string - unexpectedFlag string + expectedFlags []string expectedDomains []string shouldContainPackage bool }{ @@ -44,8 +43,7 @@ tools: Test playwright with custom domains. `, - expectedFlag: "--allowed-hosts", - unexpectedFlag: "--allowed-origins", + expectedFlags: []string{"--allowed-hosts", "--allowed-origins"}, expectedDomains: []string{"example.com", "test.com", "localhost", "127.0.0.1"}, shouldContainPackage: true, }, @@ -62,8 +60,7 @@ tools: Test playwright with default domains only. `, - expectedFlag: "--allowed-hosts", - unexpectedFlag: "--allowed-origins", + expectedFlags: []string{"--allowed-hosts", "--allowed-origins"}, expectedDomains: []string{"localhost", "127.0.0.1"}, shouldContainPackage: true, }, @@ -82,8 +79,7 @@ tools: Test playwright with copilot engine. `, - expectedFlag: "--allowed-hosts", - unexpectedFlag: "--allowed-origins", + expectedFlags: []string{"--allowed-hosts", "--allowed-origins"}, expectedDomains: []string{"github.com", "localhost", "127.0.0.1"}, shouldContainPackage: true, }, @@ -120,14 +116,11 @@ Test playwright with copilot engine. } } - // Verify the correct flag is used - if !strings.Contains(lockStr, tt.expectedFlag) { - t.Errorf("Expected lock file to contain flag %s\nActual content:\n%s", tt.expectedFlag, lockStr) - } - - // Verify the old flag is NOT used - if strings.Contains(lockStr, tt.unexpectedFlag) { - t.Errorf("Did not expect lock file to contain deprecated flag %s\nActual content:\n%s", tt.unexpectedFlag, lockStr) + // Verify all expected flags are used + for _, flag := range tt.expectedFlags { + if !strings.Contains(lockStr, flag) { + t.Errorf("Expected lock file to contain flag %s\nActual content:\n%s", flag, lockStr) + } } // Verify expected domains are present @@ -157,17 +150,16 @@ func TestPlaywrightNPXCommandWorks(t *testing.T) { outputStr := string(output) - // Verify that --allowed-hosts is in the help output (this is the flag we use for server host restrictions) + // Verify that --allowed-hosts and --allowed-origins are in the help output if !strings.Contains(outputStr, "--allowed-hosts") { t.Errorf("Expected npx playwright help to mention --allowed-hosts flag\nActual output:\n%s", outputStr) } - - // Note: --allowed-origins was added in v0.0.48 as a separate feature for browser request filtering - // It's different from --allowed-hosts which controls which hosts the MCP server serves from - // Both flags can now coexist, so we no longer check for its absence - - // Verify that the help output contains the expected option description - if !strings.Contains(outputStr, "allowed-hosts") { - t.Errorf("Expected help output to contain 'allowed-hosts' option description\nActual output:\n%s", outputStr) + if !strings.Contains(outputStr, "--allowed-origins") { + t.Errorf("Expected npx playwright help to mention --allowed-origins flag\nActual output:\n%s", outputStr) } + + // Note: --allowed-origins was added in v0.0.48 for browser request filtering + // --allowed-hosts controls which hosts the MCP server serves from (CORS) + // --allowed-origins controls which origins the Playwright browser can navigate to + // Both flags are now used together for complete network control }