Skip to content

Fix firewall cleanup permissions to include audit logs directory #27742

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.

Sign up for GitHub

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

Merged
pelikhan merged 3 commits into from
Apr 22, 2026
Merged

Fix firewall cleanup permissions to include audit logs directory #27742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f4778df
Initial plan
Copilot Apr 22, 2026
bd49347
fix(workflow): chmod firewall parent dir for audit log upload
Copilot Apr 22, 2026
71cff9c
refactor(workflow): derive firewall chmod path from constants
Copilot Apr 22, 2026
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
9 changes: 6 additions & 3 deletions pkg/workflow/engine_firewall_support.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import (
"errors"
"fmt"
"os"
"path"
"strings"

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/logger"
)

Expand Down Expand Up @@ -118,7 +120,8 @@ func generateSquidLogsUploadStep(workflowName string) GitHubActionStep {
// generateFirewallLogParsingStep creates a GitHub Actions step to parse firewall logs and create step summary.
func generateFirewallLogParsingStep(workflowName string) GitHubActionStep {
// Firewall logs are at a known location in the sandbox folder structure
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

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

Good fix! Using path.Dir(firewallLogsDir) to get the parent firewall directory ensures both logs/ and audit/ subdirectories get the correct permissions for artifact upload. This correctly addresses the EACCES issue.

firewallLogsDir := "/tmp/gh-aw/sandbox/firewall/logs"
firewallLogsDir := constants.AWFProxyLogsDir
firewallDir := path.Dir(firewallLogsDir)

Comment on lines 122 to 125
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

generateFirewallLogParsingStep hard-codes both firewallLogsDir and the new firewallDir. Since these paths already exist as constants (constants.AWFProxyLogsDir, constants.AWFAuditDir) and firewallDir is just the parent of the logs dir, consider deriving firewallDir from the logs constant (e.g., via path.Dir) to avoid future drift if the sandbox layout changes.

Copilot uses AI. Check for mistakes.
stepLines := []string{
" - name: Print firewall logs",
Expand All @@ -127,9 +130,9 @@ func generateFirewallLogParsingStep(workflowName string) GitHubActionStep {
" env:",
" AWF_LOGS_DIR: " + firewallLogsDir,
" run: |",
" # Fix permissions on firewall logs so they can be uploaded as artifacts",
" # Fix permissions on firewall logs/audit dirs so they can be uploaded as artifacts",
" # AWF runs with sudo, creating files owned by root",
fmt.Sprintf(" sudo chmod -R a+r %s 2>/dev/null || true", firewallLogsDir),
fmt.Sprintf(" sudo chmod -R a+r %s 2>/dev/null || true", firewallDir),
" # Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step)",
" if command -v awf &> /dev/null; then",
" awf logs summary | tee -a \"$GITHUB_STEP_SUMMARY\"",
Expand Down
18 changes: 18 additions & 0 deletions pkg/workflow/engine_firewall_support_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
package workflow

import (
"path"
"strings"
"testing"

"github.com/github/gh-aw/pkg/constants"
)

func TestHasNetworkRestrictions(t *testing.T) {
Expand Down Expand Up @@ -267,3 +270,18 @@ func TestCheckFirewallDisable(t *testing.T) {
}
})
}

func TestGenerateFirewallLogParsingStepFixesFirewallPermissions(t *testing.T) {
step := generateFirewallLogParsingStep("test-workflow")
stepContent := strings.Join(step, "\n")
expectedLogsDir := constants.AWFProxyLogsDir
expectedFirewallDir := path.Dir(expectedLogsDir)

if !strings.Contains(stepContent, "AWF_LOGS_DIR: "+expectedLogsDir) {
t.Error("Expected firewall log parsing step to keep AWF_LOGS_DIR set to logs directory")
}

if !strings.Contains(stepContent, "sudo chmod -R a+r "+expectedFirewallDir+" 2>/dev/null || true") {
t.Error("Expected firewall log parsing step to chmod the parent firewall directory for logs and audit upload")
}
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Apr 22, 2026

Choose a reason for hiding this comment

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

Nice addition of a dedicated test for the chmod fix. The test cleanly verifies both that AWF_LOGS_DIR still points to the logs subdirectory and that the chmod targets the parent firewall/ directory.

}
4 changes: 2 additions & 2 deletions pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,9 +505,9 @@ jobs:
env:
AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs
run: |
# Fix permissions on firewall logs so they can be uploaded as artifacts
# Fix permissions on firewall logs/audit dirs so they can be uploaded as artifacts
# AWF runs with sudo, creating files owned by root
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall/logs 2>/dev/null || true
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall 2>/dev/null || true
# Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step)
if command -v awf &> /dev/null; then
awf logs summary | tee -a "$GITHUB_STEP_SUMMARY"
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/testdata/TestWasmGolden_CompileFixtures/claude-with-network.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,9 +507,9 @@ jobs:
env:
AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs
run: |
# Fix permissions on firewall logs so they can be uploaded as artifacts
# Fix permissions on firewall logs/audit dirs so they can be uploaded as artifacts
# AWF runs with sudo, creating files owned by root
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall/logs 2>/dev/null || true
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall 2>/dev/null || true
# Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step)
if command -v awf &> /dev/null; then
awf logs summary | tee -a "$GITHUB_STEP_SUMMARY"
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,9 +673,9 @@ jobs:
env:
AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs
run: |
# Fix permissions on firewall logs so they can be uploaded as artifacts
# Fix permissions on firewall logs/audit dirs so they can be uploaded as artifacts
# AWF runs with sudo, creating files owned by root
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall/logs 2>/dev/null || true
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall 2>/dev/null || true
# Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step)
if command -v awf &> /dev/null; then
awf logs summary | tee -a "$GITHUB_STEP_SUMMARY"
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-test-tools.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,9 +572,9 @@ jobs:
env:
AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs
run: |
# Fix permissions on firewall logs so they can be uploaded as artifacts
# Fix permissions on firewall logs/audit dirs so they can be uploaded as artifacts
# AWF runs with sudo, creating files owned by root
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall/logs 2>/dev/null || true
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall 2>/dev/null || true
# Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step)
if command -v awf &> /dev/null; then
awf logs summary | tee -a "$GITHUB_STEP_SUMMARY"
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,9 +506,9 @@ jobs:
env:
AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs
run: |
# Fix permissions on firewall logs so they can be uploaded as artifacts
# Fix permissions on firewall logs/audit dirs so they can be uploaded as artifacts
# AWF runs with sudo, creating files owned by root
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall/logs 2>/dev/null || true
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall 2>/dev/null || true
# Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step)
if command -v awf &> /dev/null; then
awf logs summary | tee -a "$GITHUB_STEP_SUMMARY"
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/testdata/wasm_golden/WasmBinary/basic-copilot.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,9 +479,9 @@ jobs:
env:
AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs
run: |
# Fix permissions on firewall logs so they can be uploaded as artifacts
# Fix permissions on firewall logs/audit dirs so they can be uploaded as artifacts
# AWF runs with sudo, creating files owned by root
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall/logs 2>/dev/null || true
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall 2>/dev/null || true
# Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step)
if command -v awf &> /dev/null; then
awf logs summary | tee -a "$GITHUB_STEP_SUMMARY"
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/testdata/wasm_golden/WasmBinary/with-imports.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,9 @@ jobs:
env:
AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs
run: |
# Fix permissions on firewall logs so they can be uploaded as artifacts
# Fix permissions on firewall logs/audit dirs so they can be uploaded as artifacts
# AWF runs with sudo, creating files owned by root
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall/logs 2>/dev/null || true
sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall 2>/dev/null || true
# Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step)
if command -v awf &> /dev/null; then
awf logs summary | tee -a "$GITHUB_STEP_SUMMARY"
Expand Down
Loading