Fix rootless firewall artifact permissions to prevent EACCES on upload#5546
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses artifact upload failures in rootless (--network-isolation / sudo: false) runs by ensuring firewall sidecar outputs (logs/artifacts) remain readable by the GitHub Actions runner user, and by adding a cleanup-time “backstop” repair path for permissions.
Changes:
- Run
api-proxyandcli-proxyservices under the host UID/GID in generated Docker Compose. - Add a rootless-only cleanup-time permission repair that uses a short-lived Docker helper container with daemon-side path translation support (
dockerHostPathPrefix) and image selection inputs (imageRegistry/imageTag). - Harden sidecar log file creation to use
0644, and plumb additional config through cleanup/preservation calls; update/add tests accordingly.
Show a summary per file
| File | Description |
|---|---|
| src/services/cli-proxy-service.ts | Sets user: <host_uid>:<host_gid> for cli-proxy in compose generation. |
| src/services/cli-proxy-service.test.ts | Asserts cli-proxy service includes the user field. |
| src/services/api-proxy-service-config.ts | Sets user: <host_uid>:<host_gid> for api-proxy in compose generation. |
| src/services/api-proxy-service-config.test.ts | Asserts api-proxy service includes the user field. |
| src/container-cleanup.ts | Extends cleanup plumbing to pass docker path prefix + image selection into artifact preservation. |
| src/commands/main-action.ts | Passes dockerHostPathPrefix, imageRegistry, imageTag into cleanup(). |
| src/commands/main-action.test.ts | Updates cleanup() invocation expectations for new parameters. |
| src/artifact-preservation.ts | Adds rootless-only Docker helper permission repair; promotes some chmod-failure logs from debug → warn. |
| src/artifact-preservation-errors.test.ts | Adds tests for rootless repair path and mount path translation. |
| containers/cli-proxy/server.js | Creates log stream with mode: 0644 for new log files. |
| containers/api-proxy/token-persistence.js | Creates diagnostic and token log streams with mode: 0644. |
| containers/api-proxy/otel-exporters.js | Creates OTel span file stream with mode: 0644. |
| containers/api-proxy/blocked-request-diagnostics.js | Creates blocked-request diagnostics stream with mode: 0644. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 13/13 changed files
- Comments generated: 3
| function resolvePermFixerImageRef(imageRegistry?: string, imageTag?: string): string { | ||
| try { | ||
| const registry = imageRegistry || 'ghcr.io/github/gh-aw-firewall'; | ||
| const parsedImageTag = parseImageTag(imageTag || 'latest'); | ||
| return buildRuntimeImageRef(registry, 'agent', parsedImageTag); | ||
| } catch { | ||
| return 'ghcr.io/github/gh-aw-firewall/agent:latest'; | ||
| } |
| const existingDirs = dirs.filter( | ||
| (dir): dir is string => typeof dir === 'string' && dir.length > 0 && fs.existsSync(dir), | ||
| ); | ||
| if (existingDirs.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const uid = getSafeHostUid(); | ||
| const gid = getSafeHostGid(); | ||
| const imageRef = resolvePermFixerImageRef(imageRegistry, imageTag); | ||
|
|
||
| for (const dir of existingDirs) { | ||
| const mount = applyHostPathPrefixToVolumes([`${dir}:/fix:rw`], dockerHostPathPrefix)[0]; |
|
@copilot address the review feedback |
- resolvePermFixerImageRef: use agent-act image when agentImage is 'act' so --pull never succeeds when the act preset was used at runtime - fixArtifactPermissionsForRootless: resolve dirs to absolute paths via path.resolve() before constructing docker -v mount strings (relative paths would be treated as named volumes by Docker) - cli-proxy service: remove user override (host UID:GID) which breaks the entrypoint's write to /tmp/proxy-tls/combined-ca.crt owned by the cliproxy image user - Plumb agentImage through cleanup() -> preserveCleanupArtifacts() chain - Update affected tests
Addressed all three review items in commit
|
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
❌ Security Guard failed. Please review the logs for details. |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
✅ Smoke Claude passed |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
✅ Build Test Suite completed successfully! |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Contribution Check completed successfully! |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Gemini completed. All facets verified. 💎 Gemini Smoke Test: PASS. Titles found: 1. fix(network-isolation): break topology-attach ordering deadlock... 2. refactor(api-proxy): extract sliding-window... File created: /tmp/gh-aw/agent/smoke-test-gemini-28206620319.txt. Connectivity: 200 via proxy. |
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
Thanks for the clear write-up and for adding tests around the rootless artifact permission repair path. One contribution-guideline item needs attention before this fully matches Could you either update the PR description to remove the
|
Smoke Test: Copilot BYOK Mode — PASS ✅
Running in direct BYOK mode via
|
🔥 Smoke Test ResultsPR: Fix rootless firewall artifact permissions to prevent EACCES on upload
Overall: FAIL — pre-step data was not injected (workflow template variables unresolved).
|
🔥 Smoke Test: Copilot PAT Auth — PASS
Overall: PASS · Auth mode: PAT (COPILOT_GITHUB_TOKEN) CC
|
|
Smoke Test Results:
Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw). Overall: FAIL
|
|
5544: fix(network-isolation): break topology-attach ordering deadlock starving cli-proxy health gate ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
🔬 Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios pass. GenAI semantic conventions (
|
Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
|
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall: PASS
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results — Services Connectivity
Overall: ❌ FAIL
|
|
Overall status: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Two unrelated stale-test failures had landed on main: - main-action.test.ts: cleanup() gained a 9th agentImage parameter (rootless perm-fixer, #5546) but the call assertion still expected 8 args. Add the trailing agentImage arg. - 4 workflow lock tests pinned setup@3c7f3b6f (v0.81.0); the lock files were upgraded to setup@b5cde6c (v0.81.2). Update the pins. All 3211 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
In
--network-isolation(sudo: false) mode, firewall artifacts could be written by container UIDs the runner cannot read, and AWF’s host-sidechmodrepair silently failed. This caused artifact upload failures despite successful agent execution.Rootless artifact repair backstop (cleanup-time)
dockerHostPathPrefixfor ARC/DinD compatibility.imageRegistry/imageTag) and disables pulls (--pull never) to avoid introducing network dependency at cleanup.Eliminate sidecar UID mismatch at source
api-proxyandcli-proxyservices now run as"<host_uid>:<host_gid>"in compose generation.Harden log file readability at write-time
0644for generated log files.containers/cli-proxy/server.jscontainers/api-proxy/{token-persistence.js,blocked-request-diagnostics.js,otel-exporters.js}Restore observability for permission regressions
debugtowarnwhere host-side chmod can fail under rootless execution.Cleanup plumbing
dockerHostPathPrefix,imageRegistry, andimageTagso permission repair behavior matches runtime environment/image config.