Fix network/firewall schema description and engine support inconsistencies by Copilot · Pull Request #17909 · github/gh-aw

Copilot · 2026-02-23T12:20:03Z

Fix schema network.firewall description removing "Only supported for Copilot engine"
Add cleanup-script to firewall object schema
Fix log_level → log-level (hyphen) in engine.go
Add Blocked list check in hasNetworkRestrictions()
Add gemini case in computeAllowedDomainsForSanitization
Fix computeAllowedDomainsForSanitization to use WithToolsAndRuntimes consistently for all engines
Rename cleanup_script → cleanup-script for consistent dash-style field names

Original prompt

This section details on the original issue you should resolve

<issue_title>[plan] Fix network/firewall schema description and engine support inconsistencies</issue_title>
<issue_description>## Context

From schema consistency discussion #17876 (2026-02-23 run, findings 1–5).

Objective

Fix five related network/firewall bugs where the schema, documentation, and code are inconsistent about which engines support the firewall feature and how firewall config is parsed.

Issues to Fix

1. Schema network.firewall description incorrectly says "Only supported for Copilot engine"

File: pkg/parser/schemas/main_workflow_schema.json:2225

All four engines (copilot, claude, codex, gemini) have supportsFirewall: true — remove the Copilot-only restriction from the description.

2. log_level vs log-level key name mismatch

Files: pkg/workflow/engine.go:220, pkg/workflow/frontmatter_extraction_security.go:123

engine.go uses firewallObj["log_level"] (underscore) while the security extraction path uses firewallObj["log-level"] (hyphen).

Standardize to log-level (hyphen) in engine.go:220 to match frontmatter_extraction_security.go.

3. cleanup_script field missing from schema

Files: pkg/workflow/engine.go:227, pkg/parser/schemas/main_workflow_schema.json

engine.go:227 reads firewallObj["cleanup_script"] but the field is absent from the schema (which has additionalProperties: false).

Add cleanup_script to the appropriate schema object so users can discover and use this field.

4. hasNetworkRestrictions() ignores Blocked domain list

File: pkg/workflow/engine_firewall_support.go:17-35

The function only checks Allowed list; a workflow with only network: blocked: [...] returns false, skipping firewall validation.

Add: if len(networkPermissions.Blocked) > 0 { return true }.

5. computeAllowedDomainsForSanitization missing gemini case

File: pkg/workflow/domains.go:695-706

Gemini falls into default which calls GetAllowedDomains() instead of GetGeminiAllowedDomainsWithToolsAndRuntimes().

Add case "gemini": return GetGeminiAllowedDomainsWithToolsAndRuntimes(...) to the switch.

Files to Modify

pkg/parser/schemas/main_workflow_schema.json

pkg/workflow/engine.go

pkg/workflow/engine_firewall_support.go

pkg/workflow/domains.go

Acceptance Criteria

Schema description for network.firewall no longer says "Only supported for Copilot engine"

cleanup_script is documented in the schema

engine.go uses log-level (hyphen) consistent with security extraction path

hasNetworkRestrictions() returns true when only Blocked list is non-empty

computeAllowedDomainsForSanitization calls the Gemini-specific function for "gemini" engine

Run make agent-finish with no errors before committing

Generated by Plan Command for issue #discussion #17876

expires on Feb 25, 2026, 12:04 PM UTC

Comments on the Issue (you are @copilot in this section)

Fixes [plan] Fix network/firewall schema description and engine support inconsistencies #17906

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…ncies - Fix schema network.firewall description to include all 4 engines - Add cleanup_script field to firewall object schema - Fix log_level → log-level key in engine.go to match schema - hasNetworkRestrictions() now returns true when Blocked list is non-empty - Add gemini case to computeAllowedDomainsForSanitization Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-02-23T13:27:51Z

Hey @Copilot 👋 — excellent work identifying and fixing these five network/firewall inconsistencies! The changes are precise and well-documented. Here are a few things that would strengthen this PR:

Add test coverage — The logic changes in engine_firewall_support.go (checking blocked domains) and domains.go (gemini case) don't have corresponding tests. Consider adding test cases to verify:

hasNetworkRestrictions() with blocked-only config — Test that a workflow with only network.blocked set (no allowed list) correctly returns true
Gemini engine domain sanitization — Test that the gemini case calls GetGeminiAllowedDomainsWithToolsAndRuntimes() instead of falling through to the default
log-level parsing — Test that the hyphenated key is correctly parsed

The project's testing patterns (see scratchpad/testing.md) use table-driven tests with testify/require. Look at existing tests like pkg/workflow/engine_firewall_support_test.go (if it exists) or similar validation test files for patterns.

If you'd like to add test coverage, you can use this prompt:

Add unit tests for the network/firewall bug fixes in PR #17909.

Create or update test files to cover:

1. **pkg/workflow/engine_firewall_support_test.go** — Test `hasNetworkRestrictions()` function:
   - Scenario: workflow with only `network.blocked: ["example.com"]` and empty allowed list → should return `true`
   - Scenario: workflow with only `network.allowed: ["defaults"]` → should return `false`
   - Scenario: workflow with both allowed and blocked → should return `true`

2. **pkg/workflow/domains_test.go** — Test `computeAllowedDomainsForSanitization()` for gemini engine:
   - Verify that engine="gemini" calls `GetGeminiAllowedDomainsWithToolsAndRuntimes()` and includes Gemini-specific domains
   - Compare against codex/claude/copilot cases to ensure consistency

3. **pkg/workflow/engine_test.go** — Test firewall config extraction:
   - Verify that `log-level` (hyphen) is correctly extracted into `firewallConfig.LogLevel`
   - Verify that `cleanup_script` is correctly extracted

Follow these patterns:
- Use table-driven tests with `[]struct` for multiple scenarios
- Use `testify/require` for assertions (not `assert`)
- Use `t.Parallel()` for tests that can run concurrently
- Follow error message format: "[what's wrong]. [what's expected]. [example]"

Run `make test` to verify all tests pass.

Generated by Contribution Check

pelikhan · 2026-02-23T15:23:02Z

pkg/workflow/domains.go

 	case "claude":
 		return GetClaudeAllowedDomains(data.NetworkPermissions)
+	case "gemini":
+		return GetGeminiAllowedDomainsWithToolsAndRuntimes(data.NetworkPermissions, data.Tools, data.Runtimes)


@copilot review why Gemini uses runtime and the other no...

Good catch — Gemini only had the WithToolsAndRuntimes variant available, but the original copilot/codex/claude cases were using the simpler (no tools/runtimes) variants, creating an inconsistency.

The actual engine execution code (copilot_engine_execution.go, codex_engine.go, claude_engine.go, gemini_engine.go) all use WithToolsAndRuntimes when computing the real firewall domains. So computeAllowedDomainsForSanitization should match that — tools (HTTP MCP server domains) and runtimes (ecosystem domains like npm/pypi) must be included to get the correct domain set for sanitization.

Fixed in a981e31 — all four engine cases now call WithToolsAndRuntimes.

…for all engines All engine cases now call the WithToolsAndRuntimes variant for consistency with the actual engine execution code (copilot_engine_execution.go, codex_engine.go, claude_engine.go, gemini_engine.go), ensuring tool MCP domains and runtime ecosystem domains are included in sanitization. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

pelikhan · 2026-02-23T15:36:29Z

@copilot use dashes in field names

… field names Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot · 2026-02-23T15:44:13Z

@copilot use dashes in field names

Done in 34c70b6 — renamed cleanup_script to cleanup-script in both engine.go and the schema.

Copilot

Pull request overview

Aligns network/firewall schema docs and engine behavior, and updates allowed-domain computation so sanitization and runtime firewall inputs stay consistent across engines.

Changes:

Updates network.firewall schema description and adds cleanup-script to the schema.
Standardizes firewall config key parsing in engine.go (log-level, cleanup-script) and expands network restriction detection to include blocked.
Updates allowed-domain computation to include tools/runtimes for all supported engines and adds an explicit gemini branch; refreshes workflow lockfiles with the resulting domain lists.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/workflow/engine_firewall_support.go	Treats `network.blocked` as a network restriction.
pkg/workflow/engine.go	Switches engine firewall parsing keys to dash-style (`log-level`, `cleanup-script`).
pkg/workflow/domains.go	Uses `WithToolsAndRuntimes` consistently and adds a `gemini` case for sanitization domains.
pkg/parser/schemas/main_workflow_schema.json	Fixes firewall description and adds `cleanup-script` to `network.firewall` schema.
.github/workflows/smoke-test-tools.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/smoke-gemini.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` for Gemini to match new domain computation.
.github/workflows/smoke-copilot.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/smoke-copilot-arm.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/smoke-codex.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/smoke-claude.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/slide-deck-maintainer.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/scout.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/schema-consistency-checker.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/research.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/mcp-inspector.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/jsweep.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/github-mcp-tools-report.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/docs-noob-tester.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.
.github/workflows/daily-news.lock.yml	Updates `GH_AW_ALLOWED_DOMAINS` output to match new domain computation.

Comments suppressed due to low confidence (1)

pkg/workflow/engine_firewall_support.go:40

There are unit tests for hasNetworkRestrictions, but they don’t cover the new network.blocked-only behavior. Please add a test case where Blocked is non-empty (and Allowed is empty/unset) to ensure this regression doesn’t come back.

	// If blocked domains are specified, we have restrictions
	if len(networkPermissions.Blocked) > 0 {
		return true
	}

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

Copilot · 2026-02-23T15:53:57Z

pkg/workflow/engine_firewall_support.go

+	// If blocked domains are specified, we have restrictions
+	if len(networkPermissions.Blocked) > 0 {
+		return true
+	}


hasNetworkRestrictions now treats network.blocked as a restriction, but the firewall-disable validation path still only considers len(networkPermissions.Allowed) > 0 as “has restrictions” and its messages/errors mention only network.allowed. This creates a real gap: a workflow with only network.blocked set can disable the firewall without triggering the intended warning/error, undermining the restriction. Please update checkFirewallDisable (and related messages) to treat blocked-only configs as restrictions too.

This issue also appears on line 37 of the same file.

Copilot · 2026-02-23T15:53:58Z

pkg/workflow/domains.go

 	switch engineID {
 	case "copilot":
-		return GetCopilotAllowedDomains(data.NetworkPermissions)
+		return GetCopilotAllowedDomainsWithToolsAndRuntimes(data.NetworkPermissions, data.Tools, data.Runtimes)
 	case "codex":
-		return GetCodexAllowedDomains(data.NetworkPermissions)
+		return GetCodexAllowedDomainsWithToolsAndRuntimes(data.NetworkPermissions, data.Tools, data.Runtimes)
 	case "claude":
-		return GetClaudeAllowedDomains(data.NetworkPermissions)
+		return GetClaudeAllowedDomainsWithToolsAndRuntimes(data.NetworkPermissions, data.Tools, data.Runtimes)
+	case "gemini":
+		return GetGeminiAllowedDomainsWithToolsAndRuntimes(data.NetworkPermissions, data.Tools, data.Runtimes)


computeAllowedDomainsForSanitization now has a dedicated gemini case and uses the WithToolsAndRuntimes variants. There are existing tests for this function, but they don’t cover Gemini (and don’t exercise the tools/runtimes inclusion). Please add test coverage for the gemini branch and at least one case where tools/runtimes contribute domains, so the sanitization allowlist stays aligned with the runtime firewall behavior.

Copilot · 2026-02-23T15:53:58Z

pkg/parser/schemas/main_workflow_schema.json

+                    },
+                    "cleanup-script": {
+                      "type": "string",
+                      "description": "Path to cleanup script run after AWF shuts down (default: ./scripts/ci/cleanup.sh)"


Schema now documents network.firewall.cleanup-script, but the compiler’s extractFirewallConfig path (used for network.firewall) doesn’t parse this field, and CleanupScript is not used anywhere when building AWF commands. As-is, users can set cleanup-script and it will be silently ignored. Please wire this through end-to-end: parse the field from network.firewall (ideally accepting both cleanup-script and the legacy underscore form for compatibility) and ensure it is actually applied during/after AWF shutdown (or remove it from the schema if it’s not supported).

Suggested change

},

"cleanup-script": {

"type": "string",

"description": "Path to cleanup script run after AWF shuts down (default: ./scripts/ci/cleanup.sh)"

Copilot · 2026-02-23T15:53:58Z

pkg/workflow/engine.go

+					// Extract log-level field (default: "debug")
+					if logLevel, hasLogLevel := firewallObj["log-level"]; hasLogLevel {
 						if logLevelStr, ok := logLevel.(string); ok {
 							firewallConfig.LogLevel = logLevelStr
 						}


The comment says log-level defaults to "debug", but runtime behavior defaults to "info" (see BuildAWFArgs using constants.AWFDefaultLogLevel, and schema enum/docs). Please update the comment to avoid misleading users/maintainers about the default.

Initial plan

37dec68

Copilot AI assigned Copilot and pelikhan Feb 23, 2026

Copilot started work on behalf of pelikhan February 23, 2026 12:20 View session

Copilot AI changed the title ~~[WIP] Fix network/firewall schema description and engine support inconsistencies~~ Fix network/firewall schema description and engine support inconsistencies Feb 23, 2026

Copilot finished work on behalf of pelikhan February 23, 2026 12:30

github-actions bot mentioned this pull request Feb 23, 2026

[agentics] No-Op Runs #17408

Open

github-actions bot mentioned this pull request Feb 23, 2026

[Contribution Check Report] Contribution Check — 2026-02-23 #17919

Closed

pelikhan reviewed Feb 23, 2026

View reviewed changes

Copilot started work on behalf of pelikhan February 23, 2026 15:23 View session

Copilot finished work on behalf of pelikhan February 23, 2026 15:32

Copilot started work on behalf of pelikhan February 23, 2026 15:38 View session

Rename cleanup_script to cleanup-script to use dashes consistently in…

34c70b6

… field names Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot finished work on behalf of pelikhan February 23, 2026 15:44

pelikhan approved these changes Feb 23, 2026

View reviewed changes

pelikhan marked this pull request as ready for review February 23, 2026 15:46

Copilot AI review requested due to automatic review settings February 23, 2026 15:46

Copilot started reviewing on behalf of pelikhan February 23, 2026 15:48 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

pelikhan merged commit 62a9499 into main Feb 23, 2026
83 of 86 checks passed

pelikhan deleted the copilot/fix-network-firewall-schema branch February 23, 2026 16:05

github-actions bot mentioned this pull request Feb 23, 2026

[slides] Update AI Engines slide to include Gemini CLI #17940

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Fix network/firewall schema description and engine support inconsistencies#17909

Fix network/firewall schema description and engine support inconsistencies#17909
pelikhan merged 4 commits intomainfrom
copilot/fix-network-firewall-schema

Copilot AI commented Feb 23, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Feb 23, 2026

Uh oh!

pelikhan Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

pelikhan commented Feb 23, 2026

Uh oh!

Copilot AI commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

Copilot AI commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Issues to Fix

1. Schema network.firewall description incorrectly says "Only supported for Copilot engine"

2. log_level vs log-level key name mismatch

3. cleanup_script field missing from schema

4. hasNetworkRestrictions() ignores Blocked domain list

5. computeAllowedDomainsForSanitization missing gemini case

Files to Modify

Acceptance Criteria

Comments on the Issue (you are @copilot in this section)

Uh oh!

github-actions bot commented Feb 23, 2026

Uh oh!

pelikhan Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

pelikhan commented Feb 23, 2026

Uh oh!

Copilot AI commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Feb 23, 2026 •

edited

Loading

1. Schema `network.firewall` description incorrectly says "Only supported for Copilot engine"

2. `log_level` vs `log-level` key name mismatch

3. `cleanup_script` field missing from schema

4. `hasNetworkRestrictions()` ignores `Blocked` domain list

5. `computeAllowedDomainsForSanitization` missing `gemini` case