Skip to content

Remove tools.playwright allowed_domains/allowed_hosts; add codemod to migrate to network.allowed - #17629

Merged
pelikhan merged 5 commits into
mainfrom
copilot/remove-allowed-hosts-domains
Feb 22, 2026
Merged

Remove tools.playwright allowed_domains/allowed_hosts; add codemod to migrate to network.allowed#17629
pelikhan merged 5 commits into
mainfrom
copilot/remove-allowed-hosts-domains

Conversation

CopilotAI commented Feb 22, 2026

Copy link
Copy Markdown
Contributor
  • Remove PlaywrightDomain, PlaywrightAllowedDomains types and AllowedDomains field from PlaywrightToolConfig
  • Remove allowed_domains parsing from tools_parser.go
  • Remove generatePlaywrightAllowedDomains; update extractExpressionsFromPlaywrightArgs signature
  • Remove --allowed-hosts/--allowed-origins from MCP renderers
  • Remove allowed_domains/allowed-domains from MCP config validation
  • Remove allowed_domains from JSON schema playwright section
  • Add codemod playwright-allowed-domains-migration to migrate to network.allowed
  • Update existing .github/workflows/*.md files to remove allowed_domains
  • Recompile all 158 workflow lock files
  • Fix pkg/parser/mcp_test.go: remove allowed_domains input, remove PLAYWRIGHT_ALLOWED_DOMAINS expectations
  • Fix WASM golden fixture smoke-copilot.md: remove allowed_domains; regenerate golden file
  • Fix pkg/cli/workflows/test-claude-playwright-accessibility-contrast.md and test-playwright-args.md: remove allowed_domains
  • Fix all test files with stale allowed_domains test data (8 files)
  • Fix TestImportPlaywrightTool and TestImportAllThreeTools: remove example.com/github.com domain assertions that relied on the removed --allowed-hosts flag mechanism
Original prompt

Remove tools.playwright allowed-hosts and allowed-domains. The egress is controlled by the firewall.

Add codemod to move existing domains to network.firewall


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

… migrate to network.allowed
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI changed the title [WIP] Remove Playwright allowed-hosts and allowed-domainsRemove tools.playwright allowed_domains/allowed_hosts; add codemod to migrate to network.allowedFeb 22, 2026
@pelikhan
pelikhan marked this pull request as ready for review February 22, 2026 03:50
CopilotAI review requested due to automatic review settings February 22, 2026 03:50

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the deprecated tools.playwright.allowed_domains configuration field and migrates egress control to the unified workflow firewall (network.allowed). This simplifies the Playwright tool configuration by eliminating redundant network control mechanisms.

Changes:

  • Removed allowed_domains field and related types (PlaywrightDomain, PlaywrightAllowedDomains) from Playwright tool configuration
  • Removed generation of --allowed-hosts and --allowed-origins flags from both TOML and JSON MCP renderers
  • Added codemod playwright-allowed-domains-migration (v0.9.0) to automatically migrate existing configurations
  • Updated 6 workflow files to remove redundant allowed_domains entries (already covered by network.allowed)

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
pkg/workflow/tools_types.goRemoved PlaywrightDomain, PlaywrightAllowedDomains types and AllowedDomains field from PlaywrightToolConfig
pkg/workflow/tools_parser.goRemoved parsing logic for allowed_domains field in Playwright configuration
pkg/workflow/tools_types_test.goUpdated tests to remove allowed_domains from test data
pkg/workflow/mcp_renderer.goRemoved generation of --allowed-hosts/--allowed-origins flags from TOML renderer
pkg/workflow/mcp_config_playwright_renderer.goRemoved domain handling and flag generation from JSON renderer; updated documentation
pkg/workflow/mcp_playwright_config.goRemoved generatePlaywrightAllowedDomains function and AllowedDomains from PlaywrightDockerArgs
pkg/workflow/mcp_environment.goUpdated to extract expressions only from custom args, not allowed_domains
pkg/workflow/mcp_config_validation.goRemoved allowed_domains and allowed-domains from known Playwright fields
pkg/workflow/codex_engine.goRemoved AllowedDomains field from Playwright config creation
pkg/workflow/sandbox_validation.goUpdated example to show playwright: null instead of allowed_domains configuration
pkg/parser/schemas/main_workflow_schema.jsonRemoved allowed_domains property from Playwright tool schema
pkg/parser/mcp.goRemoved allowed_domains handling and PLAYWRIGHT_ALLOWED_DOMAINS env var from MCP parser
pkg/cli/codemod_playwright_domains.goNew codemod to migrate allowed_domains to network.allowed
pkg/cli/codemod_playwright_domains_test.goComprehensive tests for the migration codemod
pkg/cli/fix_codemods.goRegistered the new codemod
pkg/workflow/playwright_mcp_integration_test.goUpdated tests to verify flags are NOT present; removed TestPlaywrightNPXCommandWorks
pkg/workflow/playwright_allowed_domains_secrets_test.goRemoved tests for allowed_domains secret handling; kept tests for custom args
pkg/cli/mcp_inspect_playwright_*.goRemoved allowed_domains from test configurations
pkg/workflow/mcp_benchmark_test.goRemoved allowed_domains from benchmark test data
.github/workflows/*.mdRemoved redundant allowed_domains from 6 workflow files
.github/workflows/*.lock.ymlUpdated lock files to reflect removal of --allowed-hosts/--allowed-origins flags

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadpkg/parser/mcp.go
Comment on lines 438 to 447
@@ -447,34 +446,8 @@ func processBuiltinMCPTool(toolName string, toolValue any, serverFilter string)
Name: "playwright",
}

CopilotAIFeb 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of allowed_domains handling here is correct, but the corresponding tests in pkg/parser/mcp_test.go (lines 254-387) were not updated. These tests still expect PLAYWRIGHT_ALLOWED_DOMAINS environment variable to be set based on the allowed_domains configuration, which is now incorrect behavior. The tests should be updated to remove the allowed_domains field from test inputs and remove expectations of the PLAYWRIGHT_ALLOWED_DOMAINS environment variable.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 24c341c. Updated pkg/parser/mcp_test.go to remove allowed_domains from all Playwright test inputs, remove the PLAYWRIGHT_ALLOWED_DOMAINS env var from expected Args/Env, and merged the now-redundant "localhost default" test case into the renamed "default configuration" case.

Comment on lines 299 to 301
config.Version = fmt.Sprintf("%g", versionNum)
}

CopilotAIFeb 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple test files contain test data with allowed_domains field for Playwright configurations that should be updated to reflect the removal of this field. While these tests will still pass (the field is silently ignored by the parser), keeping outdated test data is misleading. The affected test files include:

  • pkg/workflow/args_field_test.go (lines 55, 69, 83)
  • pkg/workflow/version_field_test.go (lines 67, 78, 88, 99, 110)
  • pkg/workflow/http_mcp_domains_test.go (lines 65, 248, 309, 329)
  • pkg/workflow/mcp_config_shared_test.go (lines 160, 181, 210)
  • pkg/workflow/importable_tools_test.go (lines 29, 286, 455)
  • pkg/workflow/strict_mode_test.go (line 171)
  • pkg/workflow/mcp_logs_upload_test.go (line 24)
  • pkg/workflow/shared_workflow_test.go (line 27)

These test files should be updated to remove allowed_domains from Playwright test configurations to accurately represent the new behavior.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 24c341c. Removed allowed_domains from test data in all 8 listed test files, updated the WASM golden fixture (smoke-copilot.md) and regenerated its golden output file, and cleaned up the two pkg/cli/workflows/ test workflow files as well.

@pelikhan

Copy link
Copy Markdown
Collaborator

…ata and update expectations
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Feb 22, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot fix build wasm https://github.com/github/gh-aw/actions/runs/22267997492/job/64422594247

Fixed in 24c341c. The WASM golden test was failing because smoke-copilot.md fixture still had allowed_domains. Removed it from the fixture and regenerated the golden output file — the --allowed-hosts/--allowed-origins flags no longer appear in the compiled output.

@pelikhan

Copy link
Copy Markdown
Collaborator

@pelikhan

Copy link
Copy Markdown
Collaborator

…e allowed_domains assertions
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Feb 22, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot fix tests https://github.com/github/gh-aw/actions/runs/22270101969/job/64423130685#step:9:1

Fixed in 2da792b. TestImportPlaywrightTool and TestImportAllThreeTools were asserting that example.com/github.com appeared in the compiled output — those values came from allowed_domains being passed as --allowed-hosts flags. With that mechanism removed, the assertions were stale and have been removed.

(The pkg/cli package-level failure in CI is TestFetchIncludeFromSource_SectionExtraction doing a real git clone to GitHub — unrelated to this PR, as remote_workflow_test.go was not touched.)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/actions/github-script/git/ref/tags/v8
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha 273226490/.github/workflows .cfg 64/pkg/tool/linux_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha 1533603152/.github/workflows .cfg 64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/1/artifacts
    • Triggering command: /usr/bin/gh gh run download 1 --dir test-logs/run-1 (http block)
    • Triggering command: /usr/bin/gh gh run download 1 --dir test-logs/run-1 8465465/b214/vet.cfg ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12345/artifacts
    • Triggering command: /usr/bin/gh gh run download 12345 --dir test-logs/run-12345 (http block)
    • Triggering command: /usr/bin/gh gh run download 12345 --dir test-logs/run-12345 5769931/b248/vet.cfg ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12346/artifacts
    • Triggering command: /usr/bin/gh gh run download 12346 --dir test-logs/run-12346 (http block)
    • Triggering command: /usr/bin/gh gh run download 12346 --dir test-logs/run-12346 8465465/b207/vet.cfg ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/2/artifacts
    • Triggering command: /usr/bin/gh gh run download 2 --dir test-logs/run-2 (http block)
    • Triggering command: /usr/bin/gh gh run download 2 --dir test-logs/run-2 8465465/b208/vet.cfg ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/3/artifacts
    • Triggering command: /usr/bin/gh gh run download 3 --dir test-logs/run-3 fuzz/counters_supported.go x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh run download 3 --dir test-logs/run-3 (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/4/artifacts
    • Triggering command: /usr/bin/gh gh run download 4 --dir test-logs/run-4 (http block)
    • Triggering command: /usr/bin/gh gh run download 4 --dir test-logs/run-4 8465465/b215/vet.cfg ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/5/artifacts
    • Triggering command: /usr/bin/gh gh run download 5 --dir test-logs/run-5 (http block)
  • https://api.github.com/repos/github/gh-aw/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 100 (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 6 (http block)
  • https://api.github.com/repos/nonexistent/repo/actions/runs/12345
    • Triggering command: /usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion (http block)
  • https://api.github.com/repos/owner/repo/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo (http block)
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/owner/repo/contents/file.md
    • Triggering command: /tmp/go-build2125769931/b001/cli.test /tmp/go-build2125769931/b001/cli.test -test.paniconexit0 -test.timeout=10m0s -test.v=true -test.count=1 (http block)
    • Triggering command: /tmp/go-build711484274/b001/cli.test /tmp/go-build711484274/b001/cli.test -test.paniconexit0 -test.v=true -test.count=1 -test.timeout=1m0s (http block)
    • Triggering command: /tmp/go-build2065935528/b001/cli.test /tmp/go-build2065935528/b001/cli.test -test.testlogfile=/tmp/go-build2065935528/b001/testlog.txt -test.paniconexit0 -test.v=true -test.run=TestFetch -test.timeout=30s /tmp/go-build2125769931/b016/vetmain 5769931/b188/vet.cfg (http block)
  • https://api.github.com/repos/test-owner/test-repo/actions/secrets
    • Triggering command: /usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name (http block)
    • Triggering command: /usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name INVALID,NEW -j DROP (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@pelikhan
pelikhan merged commit 792084b into mainFeb 22, 2026
48 checks passed
@pelikhan
pelikhan deleted the copilot/remove-allowed-hosts-domains branch February 22, 2026 04:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@pelikhan