Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/blog-auditor.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/cloclo.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/daily-multi-device-docs-tester.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .github/workflows/docs-noob-tester.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/smoke-claude.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/smoke-codex.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .github/workflows/smoke-copilot-no-firewall.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .github/workflows/smoke-copilot-playwright.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/unbloat-docs.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 16 additions & 6 deletions pkg/workflow/custom_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 11 additions & 3 deletions pkg/workflow/mcp-config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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, " ")
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions pkg/workflow/mcp_config_refactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down Expand Up @@ -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
},
},
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/mcp_config_shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 4 additions & 1 deletion pkg/workflow/mcp_renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 18 additions & 26 deletions pkg/workflow/playwright_mcp_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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-*")
Expand All @@ -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
}{
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Loading