Audit: Log directory management — findings and recommendations after ARC/DinD fixes

[lpcox]

Context

A comprehensive audit of log directory management was performed after 9 permission/ownership bug fixes landed in ~10 days (Jun 28 – Jul 6) following improved ARC/DinD support. The system works correctly today but has accumulated complexity that warrants cleanup and hardening.

Architecture

Log directories follow a 5-phase lifecycle:

1. Path resolution (src/log-paths.ts) — centralizes conditional proxyLogsDir/workDir logic
2. Creation + permissions (src/workdir-setup.ts) — pre-creates dirs with correct UID/GID before container startup
3. Container entrypoint repair (src/services/squid-service.ts) — chown preflight fixes DinD split-filesystem race where Docker daemon auto-creates mount sources as root
4. Pre-shutdown chmod (src/container-stop.ts) — docker exec makes logs readable before container removal
5. Artifact preservation + rootless repair (src/artifact-preservation.ts, src/artifact-permissions.ts) — rename/chmod and last-resort privileged container repair

Findings

1. Triple-Layer Squid Log Defense (Complexity Concern)

Squid log permissions are handled in three separate places:

• workdir-setup.ts: chown(13:13) or chmod(0o777) at creation
• squid-service.ts entrypoint: chown proxy:proxy or chmod 0777 at container start
• container-stop.ts: chmod -R a+rX before shutdown

Each layer compensates for the other's failure mode, which is correct, but there's no documentation explaining the defense-in-depth relationship. Future maintainers may accidentally remove a "redundant" layer.

2. onCreate vs onAfterEnsure Asymmetry

• agent-logs / session-state: Use onAfterEnsure → chown runs whether newly created or pre-existing
• squid-logs: Uses onCreate only → chown only fires on fresh creation

If squid-logs pre-exists from a timeout-killed run with wrong ownership (e.g., root:root from a DinD auto-create that persisted), the host-side chown won't fire. The container entrypoint preflight compensates, but this is an implicit dependency that isn't documented.

3. MCP Logs Never Cleaned Up

/tmp/gh-aw/mcp-logs is:

• Hardcoded outside workDir
• Not included in removeWorkDirectories() cleanup
• Not subject to path-prefix translation (host-only, intentional)
• Accumulates unboundedly across runs on long-lived runners

4. proxyLogsDir Squid Path Inconsistency

When proxyLogsDir is set:

• Squid writes directly to proxyLogsDir/access.log (no subdirectory)
• API proxy writes to proxyLogsDir/api-proxy-logs/
• CLI proxy writes to proxyLogsDir/cli-proxy-logs/

This is legacy behavior that GitHub Actions workflows depend on, but it's confusing for new contributors.

5. Auto-Create Missing rw Mounts Could Mask Errors

Commit 97f10216 auto-creates missing host directories for rw bind mounts. While this solves the ARC/DinD case, it also silently papers over typos in mount paths. Only ro mounts still validate existence.

6. Rootless Repair Uses --pull never

fixArtifactPermissionsForRootless() requires the agent image to already be cached. If the first run fails before pulling the image, the repair silently fails (logged as warning only).

Recommendations

Short-term (hardening)

• Document the triple-layer defense pattern — add a comment block in workdir-setup.ts or a section in LOGGING.md explaining why each layer exists and what failure mode it covers
• Use onAfterEnsure for squid-logs — repair ownership even when the directory pre-exists, matching the agent-logs/session-state pattern (the container entrypoint remains as defense-in-depth)
• Add age-based pruning for /tmp/gh-aw/mcp-logs — clean subdirectories older than N hours during CLI startup to prevent unbounded growth on persistent runners

Medium-term (simplification)

• Consider unifying the proxy log permission model — api-proxy and cli-proxy use 0o777; squid-logs tries chown(13:13) with 0o777 fallback. Evaluate whether all proxy logs should use the same approach
• Add a debug-level log in the auto-create path (volume-parsers.ts) to make it diagnosable when a typo is silently resolved

Longer-term (testing)

• Add integration test for ARC/DinD log flow — simulate the split filesystem scenario (wrapper chowns runner-side, daemon auto-creates its own view) to catch regressions in the layered defense
• Document the proxyLogsDir squid path asymmetry in docs/environment.md or LOGGING.md

Recent Fix Timeline

Date	Commit	Fix
Jul 6	35321a7	Tolerate EROFS on mcp-logs chmod
Jul 6	3b6d0c6	Tolerate EPERM on mcp-logs chmod
Jul 2	660673c	chmod squid logs inside container before compose down
Jul 2	89fa1f1	Ensure chmod runs even when chown fails in rootless repair
Jul 1	97f1021	Auto-create missing host directories for rw volume mounts
Jul 1	dcdbf49	Add write permission in rootless artifact repair
Jun 30	83a4176	Handle EACCES during chroot-home cleanup in rootless Docker
Jun 29	50a0d58	Extract rootless permission repair into artifact-permissions.ts
Jun 28	463dfad	Fix rootless firewall artifact permissions (EACCES on upload)