Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 25 additions & 2 deletions actions/setup/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The passwordless sudo -n rm -rf fallback is a solid approach for GitHub-hosted runners. Consider logging when the sudo path is taken to aid future debugging.

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
Comment thread
Copilot marked this conversation as resolved.
mkdir -p /tmp/gh-aw
debug_log "Created /tmp/gh-aw directory"

Expand Down
12 changes: 10 additions & 2 deletions pkg/workflow/engine_firewall_support.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good clarification in this comment — noting that Docker containers write as non-runner UIDs (e.g. Squid UID 13) makes the rationale much clearer.

// 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),
)
}
Comment thread
Copilot marked this conversation as resolved.

stepLines = append(stepLines,
Expand Down
7 changes: 6 additions & 1 deletion pkg/workflow/engine_firewall_support_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading