fix: chmod gh-aw config dirs before privilege drop in entrypoint#1711
fix: chmod gh-aw config dirs before privilege drop in entrypoint#1711
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
Updates the agent container entrypoint to ensure gh-aw runtime/config directories created as root on self-hosted runners remain accessible after the chroot privilege drop, preventing silent failures in MCP tool discovery and safe-output writes (closes #1463).
Changes:
- Recursively relaxes permissions on
/host/tmp/gh-awbefore executing the chrooted agent command. - Recursively relaxes permissions on
/host/opt/gh-aw/safeoutputsbefore executing the chrooted agent command.
Show a summary per file
| File | Description |
|---|---|
containers/agent/entrypoint.sh |
Adds a pre-privilege-drop permission fixup for host-mounted gh-aw config and safe-output directories in chroot mode. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 3
containers/agent/entrypoint.sh
Outdated
| chmod -R a+rwX /host/tmp/gh-aw 2>/dev/null || true | ||
| echo "[entrypoint] Made /host/tmp/gh-aw accessible to chroot user" | ||
| fi | ||
| if [ -d /host/opt/gh-aw/safeoutputs ]; then | ||
| chmod -R a+rwX /host/opt/gh-aw/safeoutputs 2>/dev/null || true |
There was a problem hiding this comment.
chmod -R a+rwX makes these host-mounted directories (especially /host/opt/gh-aw/safeoutputs) world-readable and world-writable, and the permission change persists on the host after the job. This is broader than necessary and can allow unintended cross-user tampering / data exposure on multi-user self-hosted runners. Consider changing ownership to the intended chroot user (e.g., chown -R ${HOST_UID}:${HOST_GID}) and using a more restrictive mode (e.g., user/group +rwX only) instead of granting access to a.
| chmod -R a+rwX /host/tmp/gh-aw 2>/dev/null || true | |
| echo "[entrypoint] Made /host/tmp/gh-aw accessible to chroot user" | |
| fi | |
| if [ -d /host/opt/gh-aw/safeoutputs ]; then | |
| chmod -R a+rwX /host/opt/gh-aw/safeoutputs 2>/dev/null || true | |
| chown -R "${HOST_UID}:${HOST_GID}" /host/tmp/gh-aw 2>/dev/null || true | |
| chmod -R ug+rwX,o-rwX /host/tmp/gh-aw 2>/dev/null || true | |
| echo "[entrypoint] Made /host/tmp/gh-aw accessible to chroot user" | |
| fi | |
| if [ -d /host/opt/gh-aw/safeoutputs ]; then | |
| chown -R "${HOST_UID}:${HOST_GID}" /host/opt/gh-aw/safeoutputs 2>/dev/null || true | |
| chmod -R ug+rwX,o-rwX /host/opt/gh-aw/safeoutputs 2>/dev/null || true |
There was a problem hiding this comment.
Fixed in 4d51fdc. Replaced chmod -R a+rwX with chown -R "${HOST_UID}:${HOST_GID}" so that only the specific agent user gets access, not all users on the host. This addresses the world-writable concern on multi-user self-hosted runners.
containers/agent/entrypoint.sh
Outdated
| chmod -R a+rwX /host/tmp/gh-aw 2>/dev/null || true | ||
| echo "[entrypoint] Made /host/tmp/gh-aw accessible to chroot user" | ||
| fi | ||
| if [ -d /host/opt/gh-aw/safeoutputs ]; then | ||
| chmod -R a+rwX /host/opt/gh-aw/safeoutputs 2>/dev/null || true | ||
| echo "[entrypoint] Made /host/opt/gh-aw/safeoutputs accessible to chroot user" |
There was a problem hiding this comment.
The success logs here are unconditional even if chmod fails (errors are redirected and || true forces a 0 exit). This can be misleading when debugging permission issues on self-hosted runners. Consider only printing the success message when the chmod command succeeds, and logging a warning when it fails.
| chmod -R a+rwX /host/tmp/gh-aw 2>/dev/null || true | |
| echo "[entrypoint] Made /host/tmp/gh-aw accessible to chroot user" | |
| fi | |
| if [ -d /host/opt/gh-aw/safeoutputs ]; then | |
| chmod -R a+rwX /host/opt/gh-aw/safeoutputs 2>/dev/null || true | |
| echo "[entrypoint] Made /host/opt/gh-aw/safeoutputs accessible to chroot user" | |
| if chmod -R a+rwX /host/tmp/gh-aw 2>/dev/null; then | |
| echo "[entrypoint] Made /host/tmp/gh-aw accessible to chroot user" | |
| else | |
| echo "[entrypoint][WARN] Failed to make /host/tmp/gh-aw accessible to chroot user" | |
| fi | |
| fi | |
| if [ -d /host/opt/gh-aw/safeoutputs ]; then | |
| if chmod -R a+rwX /host/opt/gh-aw/safeoutputs 2>/dev/null; then | |
| echo "[entrypoint] Made /host/opt/gh-aw/safeoutputs accessible to chroot user" | |
| else | |
| echo "[entrypoint][WARN] Failed to make /host/opt/gh-aw/safeoutputs accessible to chroot user" | |
| fi |
There was a problem hiding this comment.
Fixed in 4d51fdc. The log messages are now conditional — success prints the ownership transfer with the UID:GID, and failure prints a [WARN] message. This makes debugging permission issues on self-hosted runners much easier.
containers/agent/entrypoint.sh
Outdated
| # Make gh-aw config directories readable/writable by the chroot user. | ||
| # On self-hosted runners these directories are created by the host-side | ||
| # gh-aw tooling as root, so the unprivileged chroot user cannot access them. | ||
| if [ -d /host/tmp/gh-aw ]; then | ||
| chmod -R a+rwX /host/tmp/gh-aw 2>/dev/null || true | ||
| echo "[entrypoint] Made /host/tmp/gh-aw accessible to chroot user" | ||
| fi | ||
| if [ -d /host/opt/gh-aw/safeoutputs ]; then | ||
| chmod -R a+rwX /host/opt/gh-aw/safeoutputs 2>/dev/null || true | ||
| echo "[entrypoint] Made /host/opt/gh-aw/safeoutputs accessible to chroot user" | ||
| fi |
There was a problem hiding this comment.
This change fixes an important chroot-mode failure mode on root-runner self-hosted setups, but there isn’t a regression test covering it. Adding an integration test that creates root-owned /tmp/gh-aw and /opt/gh-aw/safeoutputs on the host and then verifies the chrooted agent can read the MCP config and write safe-outputs would help prevent future breakage.
There was a problem hiding this comment.
Good suggestion. This would require an integration test that creates root-owned directories and verifies the chroot agent can still access them. Deferring this to a follow-up since it needs Docker/sudo and would be better suited as a chroot integration test in the existing test suite. The smoke tests passing on this PR provide some coverage in the meantime.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Security ReviewThe fix correctly addresses the root cause (root-owned directories inaccessible after privilege drop), and the However, Issue: World-writable permissions on security-sensitive directoriesFile: chmod -R a+rwX /host/tmp/gh-aw 2>/dev/null || true
# ...
chmod -R a+rwX /host/opt/gh-aw/safeoutputs 2>/dev/null || true
Concrete risks:
Recommended fix
if [ -d /host/tmp/gh-aw ]; then
chown -R "$HOST_UID:$HOST_GID" /host/tmp/gh-aw 2>/dev/null || true
echo "[entrypoint] Transferred /host/tmp/gh-aw ownership to chroot user ($HOST_UID:$HOST_GID)"
fi
if [ -d /host/opt/gh-aw/safeoutputs ]; then
chown -R "$HOST_UID:$HOST_GID" /host/opt/gh-aw/safeoutputs 2>/dev/null || true
echo "[entrypoint] Transferred /host/opt/gh-aw/safeoutputs ownership to chroot user ($HOST_UID:$HOST_GID)"
fiThis gives the agent user full access (as owner) without granting write permission to arbitrary other users on the host.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Security Review[CONCERN]
|
Replace `chmod -R a+rwX` with `chown -R $HOST_UID:$HOST_GID` to avoid making gh-aw config directories world-writable on the host filesystem. Also make success/failure log messages conditional on the actual command result to aid debugging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Addressed all review feedback in 4d51fdc: Security (Copilot + Security Guard): Replaced Logging: Made success/failure messages conditional on the actual Integration test: Deferred to a follow-up — requires Docker/sudo setup and is better suited as a chroot integration test. |
|
Smoke Test Results — Run 24044349061
Overall: PASS
|
|
Smoke test matrix (titles only):
|
🤖 Smoke Test Results
PR: fix: chmod gh-aw config dirs before privilege drop in entrypoint Overall: PASS
|
Smoke Test: GitHub Actions Services Connectivity
Root cause: The AWF sandbox iptables rules block database ports (Redis 6379, PostgreSQL 5432) as "dangerous ports". The
|
Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environment.
|
Summary
/tmp/gh-aw/(MCP config) and/opt/gh-aw/safeoutputs/(safe-output target) as root-owned directories. After AWF drops privileges viacapsh --user=<host-user>, the unprivileged chroot user cannot read or write these paths.chmod -R a+rwXblock inentrypoint.shbefore the privilege drop to make these directories accessible to the chroot user.Closes #1463
Test plan
/tmp/gh-aw/mcp-config/config.tomlis readable/opt/gh-aw/safeoutputs/outputs.jsonlsucceedif -dguards skip gracefully)🤖 Generated with Claude Code