-
Notifications
You must be signed in to change notification settings - Fork 30
feat: enable firewall on claude and remove hooks" #4792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
✅ Agentic Changeset Generator completed successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request enables firewall support for the Claude engine and removes the legacy network permissions hooks system. The changes replace the hook-based approach (which used Python scripts and Claude settings.json) with the AWF (Agentic Workflows Firewall) binary approach that was previously only available for the Copilot engine. Additionally, the AWF version is updated from v0.3.0 to v0.4.0.
Key changes:
- Claude engine now supports AWF firewall (matching Copilot's capabilities)
- Removed deprecated hooks-based network permissions system
- Updated
enableFirewallByDefaultto use engine interface instead of string-based engine ID - Added
GetClaudeAllowedDomainshelper function for Claude-specific domain handling - Updated all tests to reflect Claude's new firewall support
Reviewed changes
Copilot reviewed 66 out of 66 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/claude_engine.go |
Enabled firewall support flag; removed hooks/settings generation; added AWF wrapper logic in GetExecutionSteps; implemented GetSquidLogsSteps |
pkg/workflow/firewall.go |
Refactored enableFirewallByDefault to accept engine interface and check SupportsFirewall() method |
pkg/workflow/domains.go |
Added GetClaudeAllowedDomains function for Claude-specific domain handling (similar to Copilot's) |
pkg/workflow/compiler.go |
Updated call to enableFirewallByDefault to pass engine instead of engine ID string |
pkg/constants/constants.go |
Updated DefaultFirewallVersion from v0.3.0 to v0.4.0 |
pkg/workflow/engine_firewall_support_test.go |
Updated tests to verify Claude now supports firewall |
pkg/workflow/firewall_default_enablement_test.go |
Updated tests to use engine instances; added Claude firewall tests |
pkg/workflow/firewall_workflow_test.go |
Updated tests to expect AWF installation instead of hooks for Claude |
pkg/workflow/claude_engine_network_test.go |
Updated all tests to expect AWF wrapper instead of settings/hooks |
pkg/workflow/network_test.go |
Removed deprecated test for HasNetworkPermissions |
pkg/workflow/agentic_output_test.go |
Removed tests for Claude hooks cleanup (no longer needed) |
pkg/workflow/claude_settings.go |
Deleted (hooks-based settings no longer used) |
pkg/workflow/claude_settings_test.go |
Deleted (tests for deleted functionality) |
pkg/workflow/claude_settings_tmp_test.go |
Deleted (tests for deleted functionality) |
pkg/workflow/engine_network_hooks.go |
Deleted (hooks system no longer used) |
pkg/workflow/engine_network_test.go |
Deleted (tests for deleted functionality) |
.github/workflows/*.lock.yml |
All compiled workflows updated to use AWF v0.4.0 and remove hooks generation steps |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot Claude agentic engine should be updated to reflect that it supports network firewall. |
.github/aw/actions-lock.json
Outdated
| "actions/github-script@v7": { | ||
| "repo": "actions/github-script", | ||
| "version": "v7", | ||
| "sha": "f28e40c7f34bde8b3046d885e986cb6290c5673b" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot remove github-script@v7 from this branch
.github/aw/actions-lock.json
Outdated
| "actions/github-script@v7": { | ||
| "repo": "actions/github-script", | ||
| "version": "v7", | ||
| "sha": "f28e40c7f34bde8b3046d885e986cb6290c5673b" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot remove github-script@v7 from this pr
5be6c9a to
de259ed
Compare
|
✅ Agentic Changeset Generator completed successfully. |
|
@copilot can you figure out why the integration test is failing and can you fix it? https://github.com/githubnext/gh-aw/actions/runs/19717073693/job/56491771896?pr=4792 |
|
✅ Agentic Changeset Generator completed successfully. |
2f2b0f6 to
e7289bf
Compare
|
❌ Agentic Smoke Copilot failed and wasn't able to produce a result. |
|
Agentic Smoke Codex triggered by this pull request. |
|
Agentic Smoke Copilot No Firewall triggered by this pull request. |
|
❌ Agentic Smoke Claude failed and wasn't able to produce a result. |
|
✅ Agentic Changeset Generator completed successfully. |
|
Smoke test results for the Codex engine:
|
✅ Smoke Test Results: Copilot Engine (No Firewall)All smoke tests for the Copilot engine passed successfully! Test Results1. GitHub MCP Testing ✅Successfully reviewed the last 2 merged pull requests: PR #4892: "docs: clarify AWF does not support wildcard domain syntax"
PR #4877: "Update AWF to v0.5.0 and migrate to new container/log naming conventions"
2. File Writing Testing ✅Successfully created test file 3. Bash Tool Testing ✅Successfully verified file creation using bash commands:
4. Playwright MCP Testing ✅Successfully navigated to https://github.com and verified:
Overall Status🎉 All tests PASSED for Copilot engine (no firewall variant) The smoke test validates:
|
|
@copilot tests failed because it couldn't find claude CLI. you should add claude CLI to the awf mount so that it could be found (or run npx) |
Signed-off-by: Jiaxiao (mossaka) Zhou <duibao55328@gmail.com>
* Initial plan * fix: use npx to run Claude CLI when AWF firewall is enabled Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
d32156c to
505a16d
Compare
|
🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰 |
|
🤖 SYSTEM_INIT: Smoke Copilot No Firewall ACTIVATED. PROCESSING pull request. ALL SUBSYSTEMS ONLINE. |
|
🔮 The ancient spirits stir... Smoke Codex awakens to divine this pull request... |
Copilot Engine Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: PASS ✅
|
|
✅ fix: use npx to run Claude CLI when AWF firewall is enabled
|
Smoke Test: Copilot Engine (No Firewall)
Timestamp: 2025-11-28 00:31 UTC
Status: PASS ✅
All core functionalities validated: GitHub MCP, file writing, bash tools, and Playwright MCP.