diff --git a/actions/setup/setup.sh b/actions/setup/setup.sh index 3b3f10293fb..92968eb518e 100755 --- a/actions/setup/setup.sh +++ b/actions/setup/setup.sh @@ -91,8 +91,31 @@ 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" + 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" + 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 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..e62dbb5dd33 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(" sudo -n chmod -R a+rX %[1]s 2>/dev/null || chmod -R a+rX %[1]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