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

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

43 changes: 39 additions & 4 deletions pkg/parser/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
Expand Down
53 changes: 51 additions & 2 deletions pkg/parser/mcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"},
},
},
},
Expand All @@ -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"},
},
},
},
Expand Down
36 changes: 35 additions & 1 deletion pkg/workflow/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}

Expand Down
51 changes: 48 additions & 3 deletions pkg/workflow/custom_engine_test.go
Original file line number Diff line number Diff line change
@@ -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()

Expand Down Expand Up @@ -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
Expand Down
Loading