Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d204dbf8-0c30-4183-91ab-662ce23a4ebc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d204dbf8-0c30-4183-91ab-662ce23a4ebc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d204dbf8-0c30-4183-91ab-662ce23a4ebc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d204dbf8-0c30-4183-91ab-662ce23a4ebc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
pelikhan
April 21, 2026 16:48
View session
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b86247d5-111e-4b6e-9574-3730e58aa0fe Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b86247d5-111e-4b6e-9574-3730e58aa0fe Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done in 7f58a9c (plus follow-up refinement in 627f931): removed deprecated 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:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
feat(workflow): support sandbox.agent.version for awf resolution
feat(workflow): support sandbox.agent.version and migrate deprecated network.firewall to sandbox.agent
Apr 21, 2026
Collaborator
|
@copilot remove network.firewall.version from JSON schema, add codemod to sandbox.agent.version |
Contributor
|
Hey Contribution check: 🟢 Aligned
The PR looks ready for maintainer review. The draft status appears intentional for the parallel validation pass — once CI and CodeQL checks clear, this should be good to merge. 🚀
|
Contributor
Author
Done in 7f58a9c and refined in 627f931: |
…firewall migration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Commit pushed:
|
Contributor
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (345 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Draft ADR Summary (generated from diff)Decision: Remove Key alternatives considered:
Notable trade-offs:
📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: §24736713102
|
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 88/100✅ Excellent test quality
Test Classification DetailsView all 11 classified tests
Minor Findings — Non-Blocking
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds sandbox.agent.version as the AWF version override, removes deprecated network.firewall from schema/parsing, and expands the CLI codemod/tests to migrate network.firewall usage to sandbox.agent.
Changes:
- Add
sandbox.agent.versionto schema + frontmatter extraction and wire it into firewall/AWF version resolution (install + aw-info reporting). - Remove/ignore deprecated
network.firewallin workflow parsing and update strict-mode messaging/tests to reject it. - Expand codemod + tests to migrate
network.firewallvalues (includingnull,false/disable, and objectversion) tosandbox.agent.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/strict_mode_permissions_validation.go | Updates strict-mode firewall error message to remove network.firewall reference. |
| pkg/workflow/sandbox_custom_agent_test.go | Removes deprecated network.firewall: true from test workflows. |
| pkg/workflow/sandbox_agent_false_test.go | Changes integration test to ensure network.firewall frontmatter is rejected. |
| pkg/workflow/sandbox.go | Adds AgentSandboxConfig.Version field for AWF version override. |
| pkg/workflow/network_merge_import_test.go | Removes deprecated network.firewall usage from merge/import test fixture. |
| pkg/workflow/network_merge_edge_cases_test.go | Removes deprecated network.firewall usage from edge-case merge test fixture. |
| pkg/workflow/frontmatter_extraction_security_test.go | Adds coverage for extracting sandbox.agent.version. |
| pkg/workflow/frontmatter_extraction_security.go | Stops extracting network.firewall; adds extraction for sandbox.agent.version. |
| pkg/workflow/firewall_version_pinning_test.go | Verifies AWF install uses sandbox.agent.version when present. |
| pkg/workflow/firewall_log_level_test.go | Updates tests to assert deprecated network.firewall is ignored during extraction. |
| pkg/workflow/firewall_default_enablement_test.go | Updates tests to use sandbox.agent: false instead of network.firewall: false. |
| pkg/workflow/firewall.go | Adds sandbox.agent.version override logic to getFirewallConfig. |
| pkg/workflow/compiler_yaml.go | Uses getFirewallConfig for aw-info firewall/version reporting. |
| pkg/workflow/compiler_permissions_test.go | Removes deprecated network.firewall: true from permissions compilation test. |
| pkg/workflow/aw_info_versions_test.go | Adds test case asserting sandbox.agent.version overrides aw-info AWF version. |
| pkg/parser/schemas/main_workflow_schema.json | Removes network.firewall from schema; adds sandbox.agent.version. |
| pkg/cli/fix_command_test.go | Updates fix command expectations for codemod migration behavior. |
| pkg/cli/codemod_network_firewall_test.go | Adds/updates codemod tests for null, false, and version migrations. |
| pkg/cli/codemod_network_firewall.go | Implements migration to sandbox.agent (+ version normalization/quoting). |
| docs/src/content/docs/reference/frontmatter-full.md | Documents sandbox.agent.version. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 21/21 changed files
- Comments generated: 3
Comment on lines
23
to
32
| PostTransform: func(lines []string, frontmatter map[string]any, fieldValue any) []string { | ||
| // Note: We no longer set sandbox.agent: false since the firewall is mandatory | ||
| // The firewall is always enabled via the default sandbox.agent: awf | ||
|
|
||
| _, hasSandbox := frontmatter["sandbox"] | ||
|
|
||
| // Add sandbox.agent if not already present AND if firewall was explicitly true | ||
| // (no need to add sandbox.agent: awf if firewall was false, since awf is now the default) | ||
| if !hasSandbox && fieldValue == true { | ||
| // Only add sandbox.agent: awf if firewall was explicitly set to true | ||
| sandboxLines := []string{ | ||
| "sandbox:", | ||
| " agent: awf # Firewall enabled (migrated from network.firewall)", | ||
| } | ||
|
|
||
| // Try to place it after network block | ||
| insertIndex := -1 | ||
| inNet := false | ||
| for i, line := range lines { | ||
| trimmed := strings.TrimSpace(line) | ||
| if strings.HasPrefix(trimmed, "network:") { | ||
| inNet = true | ||
| } else if inNet && len(trimmed) > 0 { | ||
| // Check if this is a top-level key (no leading whitespace) | ||
| if isTopLevelKey(line) { | ||
| // Found next top-level key | ||
| insertIndex = i | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if insertIndex >= 0 { | ||
| // Insert after network block | ||
| newLines := make([]string, 0, len(lines)+len(sandboxLines)) | ||
| newLines = append(newLines, lines[:insertIndex]...) | ||
| newLines = append(newLines, sandboxLines...) | ||
| newLines = append(newLines, lines[insertIndex:]...) | ||
| lines = newLines | ||
| } else { | ||
| // Append at the end | ||
| lines = append(lines, sandboxLines...) | ||
| if !hasSandbox { | ||
| sandboxLines := sandboxAgentLinesFromFirewall(fieldValue) | ||
| if len(sandboxLines) > 0 { | ||
| lines = insertSandboxAfterNetworkBlock(lines, sandboxLines) | ||
| networkFirewallCodemodLog.Print("Converted deprecated network.firewall to sandbox.agent") | ||
| } | ||
|
|
||
| networkFirewallCodemodLog.Print("Added sandbox.agent: awf (firewall was explicitly enabled)") | ||
| } |
There was a problem hiding this comment.
The codemod only migrates network.firewall into sandbox.agent when there is no existing sandbox block (if !hasSandbox). If a workflow already has sandbox: for other reasons (e.g., sandbox.mcp or legacy sandbox.type) and network.firewall was false/disable or contained a version, this codemod will remove network.firewall but skip adding the equivalent sandbox.agent setting, changing behavior (e.g., losing an explicit disable or version pin). Consider also migrating into an existing sandbox: block when present (at least for the disable/version cases), or declining to remove network.firewall unless the equivalent sandbox.agent configuration is ensured.
| case uint64: | ||
| return strconv.FormatUint(value, 10), true | ||
| case float32: | ||
| return strconv.FormatFloat(float64(value), 'f', -1, 64), true |
There was a problem hiding this comment.
normalizeFirewallVersion formats float32 values using strconv.FormatFloat(..., bitSize=64). That can produce unexpectedly long decimal expansions because the float32 value is first converted to float64 and then round-tripped as a 64-bit float. Use bitSize=32 for the float32 case (or avoid the float32 case entirely if your YAML unmarshaling never produces it) to keep version strings stable and readable.
Suggested change
| return strconv.FormatFloat(float64(value), 'f', -1, 64), true | |
| return strconv.FormatFloat(float64(value), 'f', -1, 32), true |
Comment on lines
74
to
76
| // Check network.firewall configuration | ||
| if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil { | ||
| config := workflowData.NetworkPermissions.Firewall |
There was a problem hiding this comment.
This comment still refers to “network.firewall configuration”, but network.firewall frontmatter support is being removed/rejected in this PR. Since workflowData.NetworkPermissions.Firewall can now be populated from other sources (e.g., defaults/auto-enablement or sandbox settings), consider renaming this comment to avoid implying it comes from network.firewall.
Collaborator
|
@copilot review all comments |
Contributor
|
✅ Smoke CI completed successfully! |
Contributor
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3917e2dd-b6b3-4753-a749-56d561d560fa Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Addressed in 6e2022f. I reviewed and applied the actionable review feedback: migrated 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:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot stopped work on behalf of
pelikhan due to an error
April 21, 2026 17:54
github-actions Bot
added a commit
that referenced
this pull request
Apr 22, 2026
Add new glossary entries for terms introduced in recent changes: - GitHub Access Mode (tools.github.mode / gh-proxy) from PR #27707 - Comment Memory (tools.comment-memory) from PR #27479 - Supersede Older Reviews (supersede-older-reviews) from PR #27662 - Update AWF entry to mention sandbox.agent.version from PR #27626 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds support for
sandbox.agent.versionas the AWF version override and follows up on review feedback by removing deprecatednetwork.firewallfrontmatter usage and migrating it tosandbox.agent.What changed
sandbox.agent.versionin schema and frontmatter extraction.sandbox.agent.versioninto effective AWF version resolution used by:network.firewallentries from frontmatter schema/parsing paths.network.firewalltosandbox.agent, including:true→sandbox.agent: awffalse/disable→sandbox.agent: falsenull→sandbox.agent: awfversion→sandbox.agent.versionValidation
network.firewallfrontmattermake fmt,make lint,make test-unit, andmake build