fix: reclaim root-owned sandbox firewall dirs to prevent EACCES on reused runners#44276
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Add `sudo chown` in the post-run "Print firewall logs" step so AWF leaves the sandbox firewall dir owned by the runner user (uid=1001). This ensures the next run's `rm -rf /tmp/gh-aw` succeeds without needing sudo at all, breaking the EACCES cycle on reused runners. - Add pre-flight cleanup + pre-creation of `/tmp/gh-aw/sandbox/firewall` dirs in `create_gh_aw_tmp_dir.sh` as defense-in-depth: if the post-run chown didn't execute (e.g. previous run was killed), sudo rm reclaims the stale root-owned directory before AWF starts; then mkdir creates the firewall dirs as the runner user so AWF never needs to create them. - Update tests and WASM golden files for new chown step output. - Fix stale test assertion for smoke-call-workflow aw_context forwarding (smoke-workflow-call.md no longer declares aw_context as a workflow_call input; recompile corrected the lock file; test updated to match). - Recompile all 258 workflow lock files to pick up the chown step. Closes #44242 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
🤖 PR Triage
Score breakdown: Impact 22 + Urgency 12 + Quality 11 Assessment: Fixes EACCES on reused runners caused by root-owned sandbox firewall dirs. Valid fix via Next step: Author to undraft; defer until higher-priority bugs (#44254, #44282, #44283) are cleared.
|
There was a problem hiding this comment.
Pull request overview
This PR addresses a runner-reuse failure mode where prior AWF runs leave /tmp/gh-aw/sandbox/firewall root-owned, causing EACCES on subsequent runs when AWF tries to write configs. The changes add both post-run “ownership reclaim” and pre-flight cleanup/pre-creation to ensure the runner user can delete and recreate the firewall sandbox directory reliably.
Changes:
- Add
chown(best-effort) beforechmodin the generated firewall log parsing step so reused runners don’t hitEACCESon the next run. - Add targeted pre-flight cleanup for a stale, non-writable
/tmp/gh-aw/sandbox/firewallplus pre-createfirewall/{logs,audit}as the runner user before AWF starts. - Regenerate compiled workflow lock files and update WASM golden + lockfile-related tests.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/engine_firewall_support.go | Generate firewall-log step with best-effort chown+chmod to avoid next-run cleanup failures on reused runners. |
| actions/setup/sh/create_gh_aw_tmp_dir.sh | Pre-flight cleanup of stale root-owned firewall dir and pre-create firewall subdirs before AWF runs. |
| pkg/workflow/engine_firewall_support_test.go | Update tests to assert the new chown behavior is present in generated steps. |
| pkg/workflow/compiled_lock_files_test.go | Update expectations for forwarded workflow_call inputs in the compiled lock file. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden | Golden update reflecting added chown in generated firewall-log step. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden | Golden update reflecting added chown in generated firewall-log step. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden | Golden update reflecting added chown in generated firewall-log step. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden | Golden update reflecting added chown in generated firewall-log step. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/pi.golden | Golden update reflecting added chown in generated firewall-log step. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/gemini.golden | Golden update reflecting added chown in generated firewall-log step. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden | Golden update reflecting added chown in generated firewall-log step. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/codex.golden | Golden update reflecting added chown in generated firewall-log step. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/claude.golden | Golden update reflecting added chown in generated firewall-log step. |
| .github/workflows/*.lock.yml | Regenerated workflow lock files to include the updated firewall-log step (and other regenerated output). |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 271/271 changed files
- Comments generated: 10
- Review effort level: Low
| #!/usr/bin/env bash | ||
| set +o histexpand | ||
|
|
| @@ -148,15 +148,25 @@ func generateFirewallLogParsingStep(workflowName string, workflowData *WorkflowD | |||
| // 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. | |||
| t.Error("Expected firewall log parsing step to contain best-effort chmod for artifact upload even in network-isolation mode") | ||
| } | ||
|
|
||
| // Should contain best-effort non-sudo chown to reclaim ownership for next run |
| // the caller job forwards both from the safe_outputs payload. | ||
| assert.Contains(t, lockContent, "payload: ${{ needs.safe_outputs.outputs.call_workflow_payload }}", "call-workflow job should forward payload from safe_outputs") | ||
| assert.Contains(t, lockContent, "${{ fromJSON(needs.safe_outputs.outputs.call_workflow_payload)['task-description'] }}", "call-workflow job should forward task-description from the handler payload") | ||
| }) |
| - name: Install GitHub Copilot CLI | ||
| run: bash "${RUNNER_TEMP}/gh-aw/actions/install_copilot_cli.sh" 1.0.68 | ||
| env: | ||
| GH_HOST: github.com | ||
| - name: Install AWF binary | ||
| run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.27.27 --rootless | ||
| - name: Determine automatic lockdown mode for GitHub MCP Server | ||
| id: determine-automatic-lockdown | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 (source v9) | ||
| env: | ||
| GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }} | ||
| GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }} | ||
| GH_AW_GITHUB_MIN_INTEGRITY: 'approved' | ||
| with: | ||
| script: | | ||
| const determineAutomaticLockdown = require('${{ runner.temp }}/gh-aw/actions/determine_automatic_lockdown.cjs'); | ||
| await determineAutomaticLockdown(github, context, core); | ||
| - name: Parse integrity filter lists |
| - name: Install AWF binary | ||
| run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.27.27 --rootless | ||
| - name: Determine automatic lockdown mode for GitHub MCP Server | ||
| id: determine-automatic-lockdown | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 (source v9) | ||
| env: | ||
| GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }} | ||
| GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }} | ||
| GH_AW_GITHUB_MIN_INTEGRITY: 'approved' | ||
| with: | ||
| script: | | ||
| const determineAutomaticLockdown = require('${{ runner.temp }}/gh-aw/actions/determine_automatic_lockdown.cjs'); | ||
| await determineAutomaticLockdown(github, context, core); | ||
| - name: Parse integrity filter lists | ||
| id: parse-guard-vars | ||
| env: |
| - name: Install AWF binary | ||
| run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.27.27 --rootless | ||
| - name: Determine automatic lockdown mode for GitHub MCP Server | ||
| id: determine-automatic-lockdown | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 (source v9) | ||
| env: | ||
| GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }} | ||
| GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }} | ||
| GH_AW_GITHUB_MIN_INTEGRITY: 'approved' | ||
| with: | ||
| script: | | ||
| const determineAutomaticLockdown = require('${{ runner.temp }}/gh-aw/actions/determine_automatic_lockdown.cjs'); | ||
| await determineAutomaticLockdown(github, context, core); | ||
| - name: Parse integrity filter lists | ||
| id: parse-guard-vars | ||
| env: |
| env: | ||
| GH_HOST: github.com | ||
| - name: Install AWF binary | ||
| run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.27.27 --rootless | ||
| - name: Determine automatic lockdown mode for GitHub MCP Server | ||
| id: determine-automatic-lockdown | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 (source v9) | ||
| env: | ||
| GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }} | ||
| GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }} | ||
| GH_AW_GITHUB_MIN_INTEGRITY: 'approved' | ||
| with: | ||
| script: | | ||
| const determineAutomaticLockdown = require('${{ runner.temp }}/gh-aw/actions/determine_automatic_lockdown.cjs'); | ||
| await determineAutomaticLockdown(github, context, core); | ||
| - name: Parse integrity filter lists |
| env: | ||
| GH_HOST: github.com | ||
| - name: Install AWF binary | ||
| run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.27.27 --rootless | ||
| - name: Determine automatic lockdown mode for GitHub MCP Server | ||
| id: determine-automatic-lockdown | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 (source v9) | ||
| env: | ||
| GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }} | ||
| GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }} | ||
| GH_AW_GITHUB_MIN_INTEGRITY: 'none' | ||
| with: | ||
| script: | | ||
| const determineAutomaticLockdown = require('${{ runner.temp }}/gh-aw/actions/determine_automatic_lockdown.cjs'); | ||
| await determineAutomaticLockdown(github, context, core); | ||
| - name: Parse integrity filter lists |
| - name: Install AWF binary | ||
| run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.27.27 --rootless | ||
| - name: Determine automatic lockdown mode for GitHub MCP Server | ||
| id: determine-automatic-lockdown | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 (source v9) | ||
| env: | ||
| GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }} | ||
| GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }} | ||
| GH_AW_GITHUB_MIN_INTEGRITY: 'none' | ||
| GH_AW_GITHUB_REPOS: 'all' | ||
| with: | ||
| script: | | ||
| const determineAutomaticLockdown = require('${{ runner.temp }}/gh-aw/actions/determine_automatic_lockdown.cjs'); | ||
| await determineAutomaticLockdown(github, context, core); | ||
| - name: Parse integrity filter lists | ||
| id: parse-guard-vars | ||
| env: |
|
@copilot run pr-finisher skill |
…pt hardening Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed all actionable review feedback in commit
The |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Smoke Copilot: two representative lockfile comments added while validating review tooling.
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
📰 BREAKING: Report filed by Smoke Copilot · 126.5 AIC · ⌖ 6.06 AIC · ⊞ 19.8K
Comment /smoke-copilot to run again
Add label smoke to run again
|
Smoke test discussion interaction: passed. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
|
Smoke test summary:
Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
|
Smoke test results for run 28966796489:
Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
🤖 PR Triage (Updated)
Score breakdown: Impact 30 + Urgency 20 + Quality 8 Rationale: DRAFT. Root-owned sandbox firewall EACCES fix on reused runners. Upgraded from Run §28986426320
|
|
/matt |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs, /tdd, and /codebase-design — requesting changes on test coverage gaps and a correctness risk in the pre-flight guard.
📋 Key Themes & Highlights
Key Themes
- Missing test file —
create_gh_aw_tmp_dir.shhas the most impactful new logic in this PR (defense-in-depth pre-flight cleanup) but is the only modified script in this directory without a companion_test.sh. The three branches (writable, non-writable+sudo ok, non-writable+sudo fail) need coverage. - Test coverage gaps in
print_firewall_logs_test.sh— the chown/chmod ownership-reclaim branches (the actual bug fix) have no test coverage. The existing tests only exercise argument-parsing and theawf not installedpath. - Correctness risk in writability guard —
[ ! -w /tmp/gh-aw/sandbox/firewall ]won't fire if the directory is writable but subdirectories (e.g.logs/,audit/) are root-owned. This is precisely the scenario the PR intends to handle. GITHUB_STEP_SUMMARYundocumented and unguarded — the variable drives the critical tee output in the extracted script but is absent from the# Environment:header; if unset,tee -a ""silently drops step summary output.- Implicit path contract —
print_firewall_logs.shderivesFIREWALL_DIRasdirname(AWF_LOGS_DIR); the Go side controls what that env var is set to. A comment near the constant would make this coupling explicit for future maintainers.
Positive Highlights
- ✅ Excellent extraction of 258 inline bash blocks into a single reusable
print_firewall_logs.sh— major maintainability win - ✅ The
--rootlessflag split is clean and the Go-side logic is well-tested - ✅ Defense-in-depth layering (post-run chown + pre-run cleanup) is the right architectural approach
- ✅ Good test names and structure in both the Go and bash test files
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 140.2 AIC · ⌖ 6.48 AIC · ⊞ 6.6K
Comment /matt to run again
|
@copilot run pr-finisher skill |
…leanup-ownership Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Branch merged with main (
Hand-off: A maintainer needs to approve the pending CI runs before merge. |
|
@copilot address #44276 (review) |
…MMARY guard, path comment, and tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in commit
|
|
@copilot using sudo will break some arc-dind deployments. make sure that sudo is not used when arc-dind is enabled. |
On reused runners, a prior AWF run leaves
/tmp/gh-aw/sandbox/firewallowned by root (Squid/Docker containers run as root). The existing post-runchmod -R a+rXadds read/execute only — uid=1001 still can't delete those dirs, so the next run'swriteConfigsfails withEACCES.Changes
Post-run ownership reclaim (
pkg/workflow/engine_firewall_support.go)Added
sudo chown -R "$(id -u):$(id -g)"before the existingchmodin the generated "Print firewall logs" step. This transfers ownership back to the runner user after every run, so the next run'srm -rf /tmp/gh-awnever needs sudo at all — breaking the root cause.Pre-flight cleanup + pre-creation (
actions/setup/sh/create_gh_aw_tmp_dir.sh)Defense-in-depth for runs where the post-run chown didn't execute (killed/aborted run):
/tmp/gh-aw/sandbox/firewallis not writable,sudo -n rm -rfreclaims it before AWF startsfirewall/logsandfirewall/auditas uid=1001 so AWF never needs to create themRecompile + test updates
compiled_lock_files_test.go:smoke-workflow-call.mdno longer declaresaw_contextas aworkflow_callinput, but the lock file and test had not been updated to reflect this