From 5a2138b8d727ca963959d9de3cc5ea214ef308b2 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Tue, 7 Jul 2026 05:58:12 -0700 Subject: [PATCH 1/3] fix: reclaim stale root-owned /tmp/gh-aw before AWF invocation On persistent runners, a previous AWF run can leave /tmp/gh-aw/sandbox/ owned by root. The next run's setup.sh does `rm -rf /tmp/gh-aw` which silently fails (EACCES), then AWF's mkdirSync fails with: EACCES: permission denied, mkdir '/tmp/gh-aw/sandbox/firewall/logs' ## Changes ### actions/setup/setup.sh - Check if /tmp/gh-aw is writable before attempting removal - Fall back to `sudo rm -rf` when the directory (or its children) are not writable by the current user - Works on GitHub-hosted runners where sudo is passwordless ### pkg/workflow/engine_firewall_support.go - Add best-effort `chmod -R a+rX` in network-isolation mode (non-sudo) - Previously, the post-run chmod was skipped entirely when network-isolation was enabled, assuming rootless == no root-owned files. This is incorrect: Docker containers still write as non-runner UIDs (Squid: UID 13), and if AWF is killed by timeout/OOM, its cleanup never runs. ### pkg/workflow/engine_firewall_support_test.go - Update test assertions to expect non-sudo chmod in network-isolation mode Ref: https://github.com/github/gh-aw/actions/runs/28856007222/job/85583002595 Supersedes: #42400 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- actions/setup/setup.sh | 17 +++++++++++++++-- pkg/workflow/engine_firewall_support.go | 12 ++++++++++-- pkg/workflow/engine_firewall_support_test.go | 7 ++++++- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/actions/setup/setup.sh b/actions/setup/setup.sh index 3b3f10293fb..03fa2afff32 100755 --- a/actions/setup/setup.sh +++ b/actions/setup/setup.sh @@ -91,8 +91,21 @@ debug_log "Safe-output custom tokens support: ${SAFE_OUTPUT_CUSTOM_TOKENS_ENABLE create_dir "${DESTINATION}" debug_log "Created directory: ${DESTINATION}" -# Remove and recreate /tmp/gh-aw directory to ensure a clean state -rm -rf /tmp/gh-aw +# Remove and recreate /tmp/gh-aw directory to ensure a clean state. +# On persistent runners, a previous AWF run may leave this directory (or subdirectories +# like sandbox/firewall/) owned by root. Plain rm -rf fails with EACCES in that case, +# so we fall back to sudo rm -rf which is available passwordless on GitHub-hosted runners. +if [ -d /tmp/gh-aw ] && [ ! -w /tmp/gh-aw ]; then + debug_log "/tmp/gh-aw exists but is not writable (likely root-owned from prior run); using sudo to remove" + sudo rm -rf /tmp/gh-aw +elif [ -d /tmp/gh-aw ]; then + # Directory is writable — but subdirectories may not be (e.g., sandbox/firewall/ owned by root). + # Attempt plain rm first; if it fails, escalate to sudo. + if ! rm -rf /tmp/gh-aw 2>/dev/null; then + debug_log "/tmp/gh-aw has non-writable children (likely root-owned from prior AWF run); using sudo to remove" + sudo rm -rf /tmp/gh-aw + fi +fi mkdir -p /tmp/gh-aw debug_log "Created /tmp/gh-aw directory" diff --git a/pkg/workflow/engine_firewall_support.go b/pkg/workflow/engine_firewall_support.go index a2dfcb50b32..eb66e85ed88 100644 --- a/pkg/workflow/engine_firewall_support.go +++ b/pkg/workflow/engine_firewall_support.go @@ -143,14 +143,22 @@ func generateFirewallLogParsingStep(workflowName string, workflowData *WorkflowD " run: |", } - // When sudo is false (network isolation mode), AWF runs rootless so firewall files - // are not owned by root — skip the sudo chmod permission-fix step. + // When sudo is false (network isolation mode), AWF runs rootless but Docker + // containers still write files as non-runner UIDs (e.g., Squid writes as UID 13). + // AWF's own post-run cleanup (fixArtifactPermissionsForRootless) normally handles + // this, but if AWF is killed by timeout or OOM, the cleanup never runs. + // Use a non-sudo chmod as a best-effort fallback for artifact upload accessibility. if !isAWFNetworkIsolationEnabled(workflowData) { stepLines = append(stepLines, " # 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+rX %s 2>/dev/null || true", firewallDir), ) + } else { + stepLines = append(stepLines, + " # Best-effort permission fix for artifact upload (AWF cleanup may not have run)", + fmt.Sprintf(" chmod -R a+rX %s 2>/dev/null || true", firewallDir), + ) } stepLines = append(stepLines, diff --git a/pkg/workflow/engine_firewall_support_test.go b/pkg/workflow/engine_firewall_support_test.go index 34258d8f157..51a311a46b8 100644 --- a/pkg/workflow/engine_firewall_support_test.go +++ b/pkg/workflow/engine_firewall_support_test.go @@ -300,7 +300,12 @@ func TestGenerateFirewallLogParsingStepNetworkIsolationOmitsSudo(t *testing.T) { stepContent := strings.Join(step, "\n") if strings.Contains(stepContent, "sudo chmod") { - t.Error("Expected firewall log parsing step to omit sudo chmod when sudo is false (network isolation mode)") + t.Error("Expected firewall log parsing step to use non-sudo chmod when network isolation is enabled") + } + + // Should contain best-effort non-sudo chmod for artifact upload + if !strings.Contains(stepContent, "chmod -R a+rX") { + t.Error("Expected firewall log parsing step to contain best-effort chmod for artifact upload even in network-isolation mode") } // Should still contain the awf logs summary logic From a0ef41764ed732cd2dc850dd1f8751b6eeb90172 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Tue, 7 Jul 2026 06:07:19 -0700 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- actions/setup/setup.sh | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/actions/setup/setup.sh b/actions/setup/setup.sh index 03fa2afff32..92968eb518e 100755 --- a/actions/setup/setup.sh +++ b/actions/setup/setup.sh @@ -97,13 +97,23 @@ debug_log "Created directory: ${DESTINATION}" # so we fall back to sudo rm -rf which is available passwordless on GitHub-hosted runners. if [ -d /tmp/gh-aw ] && [ ! -w /tmp/gh-aw ]; then debug_log "/tmp/gh-aw exists but is not writable (likely root-owned from prior run); using sudo to remove" - sudo rm -rf /tmp/gh-aw + if command -v sudo >/dev/null 2>&1; then + sudo -n rm -rf /tmp/gh-aw + else + echo "::error::/tmp/gh-aw exists but is not writable, and sudo is not available to reclaim it." + exit 1 + fi elif [ -d /tmp/gh-aw ]; then # Directory is writable — but subdirectories may not be (e.g., sandbox/firewall/ owned by root). # Attempt plain rm first; if it fails, escalate to sudo. if ! rm -rf /tmp/gh-aw 2>/dev/null; then debug_log "/tmp/gh-aw has non-writable children (likely root-owned from prior AWF run); using sudo to remove" - sudo rm -rf /tmp/gh-aw + if command -v sudo >/dev/null 2>&1; then + sudo -n rm -rf /tmp/gh-aw + else + echo "::error::/tmp/gh-aw contains non-writable children, and sudo is not available to reclaim it." + exit 1 + fi fi fi mkdir -p /tmp/gh-aw From 470a2bdfcaf9b7379b73c53efda18e74aa8dbb2b Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Tue, 7 Jul 2026 06:07:27 -0700 Subject: [PATCH 3/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- pkg/workflow/engine_firewall_support.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/workflow/engine_firewall_support.go b/pkg/workflow/engine_firewall_support.go index eb66e85ed88..e62dbb5dd33 100644 --- a/pkg/workflow/engine_firewall_support.go +++ b/pkg/workflow/engine_firewall_support.go @@ -157,7 +157,7 @@ func generateFirewallLogParsingStep(workflowName string, workflowData *WorkflowD } else { stepLines = append(stepLines, " # Best-effort permission fix for artifact upload (AWF cleanup may not have run)", - fmt.Sprintf(" chmod -R a+rX %s 2>/dev/null || true", firewallDir), + fmt.Sprintf(" sudo -n chmod -R a+rX %[1]s 2>/dev/null || chmod -R a+rX %[1]s 2>/dev/null || true", firewallDir), ) }