✅ Coverage Check Passed

Overall Coverage

Coverage comparison generated by scripts/ci/compare-coverage.ts

Security Review

The fix correctly addresses the root cause (root-owned directories inaccessible after privilege drop), and the if [ -d ... ] guards and placement before the privilege drop are both correct.

However, chmod -R a+rwX is unnecessarily broad and introduces a security concern:

Issue: World-writable permissions on security-sensitive directories

File: containers/agent/entrypoint.sh (new lines ~737–745)

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

a+rwX grants read + write to all users (owner, group, and others). Both paths live on the host filesystem (bind-mounted as /host), so this affects any process running on the host machine, not just the agent container.

Concrete risks:

1. /host/tmp/gh-aw (MCP config) — world-writable MCP config allows any unprivileged host process to modify MCP server definitions before or during the agent run. On self-hosted runners that may run multiple jobs concurrently, a compromised or untrusted job could tamper with another job's MCP configuration.

2. /host/opt/gh-aw/safeoutputs (outputs.jsonl) — world-writable safe-outputs allows any host process to inject content into outputs.jsonl, which could produce fraudulent GitHub issue comments or PR actions attributed to the agent.

Recommended fix

HOST_UID and HOST_GID are already set earlier in the same script (lines ~8–9). Use chown instead of chmod to limit access to the specific agent user only:

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)"
fi

This gives the agent user full access (as owner) without granting write permission to arbitrary other users on the host.

Generated by Security Guard for issue #1711 · ● 99.7K · ◷

Security Review

[CONCERN] chmod -R a+rwX /host/tmp/gh-aw is overly broad — covers security-sensitive subdirectories

• File: containers/agent/entrypoint.sh (new lines in the diff)

• Code: chmod -R a+rwX /host/tmp/gh-aw 2>/dev/null || true

• Explanation: /tmp/gh-aw is a shared tree that contains multiple subdirectories with different security profiles:

  • /tmp/gh-aw/mcp-config/ — MCP server configuration (the stated target of this fix)
  • /tmp/gh-aw/aw-prompts/ — system prompt files
  • /tmp/gh-aw/sandbox/ — sandbox logs and agent output
  • /tmp/gh-aw/mcp-logs/ — MCP logs (already protected by tmpfs overlay, so this chmod is harmless for this subpath)
  • /tmp/gh-aw/safeoutputs/ — safe-output staging area read by post-agent steps

  Making the entire tree world-writable means the agent can modify the MCP config, prompts, and sandbox log directories. While the agent is already inside the container and many of these files are consumed before the agent runs, the recursive write permission on /tmp/gh-aw/safeoutputs/ is particularly concerning: the agent could tamper with safe-output artifacts that downstream workflow steps trust.

• Suggested action: Scope the chmod to only the specific subdirectories the agent actually needs. For example:

  if [ -d /host/tmp/gh-aw/mcp-config ]; then
    chmod -R a+rwX /host/tmp/gh-aw/mcp-config 2>/dev/null || true
  fi

  If other subdirectories are also needed, enumerate them explicitly rather than opening the whole tree. This follows least-privilege principles.

[CONCERN] chmod -R a+rwX /host/opt/gh-aw/safeoutputs allows agent to tamper with safe-output config and validation

• File: containers/agent/entrypoint.sh (new lines in the diff)

• Code: chmod -R a+rwX /host/opt/gh-aw/safeoutputs 2>/dev/null || true

• Explanation: The safe-outputs directory contains three categories of files:

  1. outputs.jsonl — the actual output file the agent writes to (write access needed, this is correct)
  2. config.json — safe-output configuration controlling output behavior
  3. tools.json / validation.json — tool definitions and validation rules that constrain what the agent can output

  Making the entire directory world-writable means the agent can modify config.json, tools.json, and validation.json — effectively rewriting the rules that govern its own output. A compromised agent could relax validation constraints and then write arbitrary outputs that downstream steps trust.

• Suggested action: Consider one of:

  • (a) Only chmod the outputs.jsonl file (or create it with the right ownership): chmod a+rw /host/opt/gh-aw/safeoutputs/outputs.jsonl 2>/dev/null || true
  • (b) Make the directory itself writable but keep config/tools/validation files read-only: chmod a+rwX /host/opt/gh-aw/safeoutputs && chmod a+rw /host/opt/gh-aw/safeoutputs/outputs.jsonl
  • (c) Use chown to the target UID instead of world-writable permissions: chown -R ${HOST_UID}:${HOST_GID} /host/opt/gh-aw/safeoutputs — this is more precise and doesn't weaken permissions for other users/processes.

[INFO] a+rwX vs targeted ownership

• Explanation: Both chmod blocks use a+rwX (all users: read, write, and execute-if-directory). Since the entrypoint already has HOST_UID and HOST_GID variables from the UID/GID mapping logic earlier in the script, using chown -R ${HOST_UID}:${HOST_GID} would be strictly more precise — it grants access only to the specific user the agent will run as, rather than world-writable. This matters in multi-user or shared-runner scenarios.
• Suggested action: Replace chmod -R a+rwX with chown -R ${HOST_UID}:${HOST_GID} for both paths. This achieves the same goal (the chroot user can access the files) without making them world-writable.

[INFO] No iptables or network security impact

• The changes do not affect setup-iptables.sh, host-iptables.ts, squid-config.ts, or any network filtering logic.
• No command injection risk — the paths are hardcoded string literals, not derived from user input.
• No path traversal risk — the -d guards check for directory existence of fixed paths.
• The tmpfs overlay on /host/tmp/gh-aw/mcp-logs takes precedence over the chmod, so MCP log hiding is unaffected.

Summary

The fix addresses a real problem (root-owned directories inaccessible to the unprivileged agent on self-hosted runners), but the permissions are broader than necessary. The two main concerns are:

1. /host/tmp/gh-aw chmod is recursive over the entire tree rather than scoped to mcp-config (the stated need).
2. /host/opt/gh-aw/safeoutputs chmod makes config and validation files writable, allowing the agent to tamper with its own output constraints.

Recommended fix: use chown -R ${HOST_UID}:${HOST_GID} instead of chmod -R a+rwX, and scope /tmp/gh-aw to only the subdirectories the agent needs.

Addressed all review feedback in 4d51fdc:

Security (Copilot + Security Guard): Replaced chmod -R a+rwX with chown -R "${HOST_UID}:${HOST_GID}" for both /host/tmp/gh-aw and /host/opt/gh-aw/safeoutputs. This transfers ownership to the specific agent user instead of making directories world-writable, eliminating the cross-user tampering risk on multi-user self-hosted runners.

Logging: Made success/failure messages conditional on the actual chown exit code. Success now logs the UID:GID used; failure logs a [WARN] message to aid debugging.

Integration test: Deferred to a follow-up — requires Docker/sudo setup and is better suited as a chroot integration test.

Addressed: Updated PR title and description to reflect the chown-based implementation instead of chmod. Thanks for catching the mismatch.

Review Feedback

Good change overall — chown is the right approach (not chmod a+rwX), conditional logging is helpful, and the placement before capsh privilege drop is correct.

One significant issue: the safe-outputs path is stale.

The second chown block targets /host/opt/gh-aw/safeoutputs, but this path no longer exists in practice. Since gh-aw v0.67.0, safe-outputs moved to ${RUNNER_TEMP}/gh-aw/safeoutputs/:

# Every lock file in this repo now uses:
GH_AW_SAFE_OUTPUTS: ${{ runner.temp }}/gh-aw/safeoutputs/outputs.jsonl

Zero lock files reference /opt/gh-aw/safeoutputs anymore:

$ grep -r "/opt/gh-aw/safeoutputs" .github/workflows/
# (no results)

Suggested fix: derive the safe-outputs directory from $GH_AW_SAFE_OUTPUTS (which is passed through to the container env) instead of hardcoding the old path. Something like:

# Handle safe-outputs directory (path varies by gh-aw version)
if [ -n "${GH_AW_SAFE_OUTPUTS:-}" ]; then
  # GH_AW_SAFE_OUTPUTS is e.g. /tmp/gh-aw/safeoutputs/outputs.jsonl — chown parent dir
  _so_dir="/host$(dirname "$GH_AW_SAFE_OUTPUTS")"
  if [ -d "$_so_dir" ]; then
    if chown -R "${HOST_UID}:${HOST_GID}" "$_so_dir" 2>/dev/null; then
      echo "[entrypoint] Transferred $_so_dir ownership to chroot user (${HOST_UID}:${HOST_GID})"
    else
      echo "[entrypoint][WARN] Failed to transfer $_so_dir ownership to chroot user"
    fi
  fi
fi

This is forward-compatible regardless of where gh-aw puts safe-outputs.

Minor items:

• The automated reviewer's unresolved comment about updating the PR title/description is valid — the current title says "chmod" but the implementation is chown.
• The /host/tmp/gh-aw block is correct and covers MCP config ($GH_AW_MCP_CONFIG = /tmp/gh-aw/mcp-config/config.toml). 👍
• Deferring the integration test is reasonable given this needs Docker/sudo.

Addressed: replaced hardcoded /opt/gh-aw/safeoutputs with dynamic path derived from GH_AW_SAFE_OUTPUTS env var. Now forward-compatible regardless of where gh-aw puts safe-outputs.

Smoke Test Results — run 24054424503

✅ GitHub MCP: #1722 feat: propagate host.docker.internal to child containers, #1721 fix: enforce shared net namespace for child containers
✅ Playwright: github.com title contains "GitHub"
✅ File write: /tmp/gh-aw/agent/smoke-test-claude-24054424503.txt created
✅ Bash verify: file content confirmed

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude

🤖 Smoke Test Results

PR: fix: chown gh-aw config dirs to agent user before privilege drop in entrypoint (#1463)
Author: @Mossaka

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

Chroot Version Comparison Results

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environment.

Tested by Smoke Chroot

Smoke Test Status

• feat: support npm install -g in agent container
• fix: enable color output when --tty flag is set
• 1✅ 2❌ 3❌ 4❌ 5✅ 6✅ 7❌ 8✅
• Overall: FAIL
• Run: §24054424518

🔮 The oracle has spoken through Smoke Codex

Smoke Test: GitHub Actions Services Connectivity

Summary: All 3 checks failed. host.docker.internal resolves to 172.17.0.1 but ports 6379 and 5432 are not reachable. Services may not be running, or network rules are blocking access from this sandbox.

🔌 Service connectivity validated by Smoke Services

Security Review

The overall direction of this PR is correct — using chown "${HOST_UID}:${HOST_GID}" (targeted ownership transfer to a specific non-root UID/GID) is more secure than the previous chmod a+rwX approach, which made directories world-accessible on multi-user runners. The HOST_UID/HOST_GID values are validated as numeric and non-zero at the top of the entrypoint, which is good.

One minor hardening concern worth addressing:

GH_AW_SAFE_OUTPUTS path is used without canonicalization

File: containers/agent/entrypoint.sh (~line 750)

if [ -n "${GH_AW_SAFE_OUTPUTS:-}" ]; then
  _so_dir="/host$(dirname "$GH_AW_SAFE_OUTPUTS")"
  if [ -d "$_so_dir" ]; then
    if chown -R "${HOST_UID}:${HOST_GID}" "$_so_dir" 2>/dev/null; then

GH_AW_SAFE_OUTPUTS is used directly in shell path construction with no validation. If the value contains path traversal sequences (e.g., /tmp/../../ or similar), dirname resolves those before the /host prefix is applied, potentially causing chown -R to target a broader directory than intended. For instance, if GH_AW_SAFE_OUTPUTS=/tmp/gh-aw/safeoutputs/../../outputs.jsonl, then dirname → /tmp/gh-aw/safeoutputs/../.. → resolves to /tmp, making _so_dir=/host/tmp — chown -R would then change ownership of all files in the host's /tmp, not just the safeoutputs directory.

Risk level: Low. In practice, GH_AW_SAFE_OUTPUTS is set by workflow YAML (e.g., /opt/gh-aw/safeoutputs/outputs.jsonl) and is not user- or agent-controlled. However, as a defense-in-depth measure, consider validating the resolved path stays within expected bounds before executing chown:

if [ -n "${GH_AW_SAFE_OUTPUTS:-}" ]; then
  _so_dir="/host$(dirname "$GH_AW_SAFE_OUTPUTS")"
  case "$_so_dir" in
    /host/tmp/gh-aw/*|/host/opt/gh-aw/*)
      if [ -d "$_so_dir" ]; then
        chown -R "${HOST_UID}:${HOST_GID}" "$_so_dir" 2>/dev/null || \
          echo "[entrypoint][WARN] Failed to transfer $_so_dir ownership to chroot user"
      fi
      ;;
    *)
      echo "[entrypoint][WARN] GH_AW_SAFE_OUTPUTS resolves to unexpected path: $_so_dir — skipping chown"
      ;;
  esac
fi

This is a suggestion, not a blocker. The PR is a net security improvement.

🤖 Automated security review

Generated by Security Guard for issue #1711 · ● 164.8K · ◷

🏗️ Build Test Suite Results

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #1711 · ● 775.7K · ◷