Non-fatal chown fallback for macOS bind mounts (complement to #1307)#1537
Non-fatal chown fallback for macOS bind mounts (complement to #1307)#1537ztech-gthb wants to merge 1 commit intocoleam00:devfrom
Conversation
Complements coleam00#1307. On macOS Docker Desktop with VirtioFS bind mounts, host UIDs (e.g. 501) don't map to appuser (1001) and chown fails — the script then exit 1s and the container crash-loops. Now treats chown failure as a warning, falls back to running as root, and exports IS_SANDBOX=1 so ClaudeProvider skips its UID-0 check (we're still sandboxed by Docker).
📝 WalkthroughWalkthroughThe entrypoint script now handles ChangesRoot-Privilege Fallback Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-entrypoint.sh`:
- Line 17: The chown invocation currently silences stderr with `2>/dev/null`,
losing the real OS error; change the `if chown -Rh appuser:appuser /.archon
2>/dev/null; then` pattern to capture stderr into a variable (e.g.
`output=$(chown -Rh appuser:appuser /.archon 2>&1)`) and use the command's exit
status to decide the branch, then include that captured `output` in the warning
message so the exact chown error (like "Operation not permitted" or "Read-only
file system") is logged.
- Around line 17-23: The script currently unconditionally exports IS_SANDBOX=1
on any chown failure, silently bypassing the root-safety guard in
ClaudeProvider; change this so IS_SANDBOX is only set when the user explicitly
opted in (e.g., check if an opt-in env var is already set like IS_SANDBOX=1 or a
new explicit flag SANDBOX_FORCE=1) instead of auto-exporting it on failure.
Update the chown block around the chown -Rh appuser:appuser /.archon check and
the RUNNER assignment so that on failure you print the warning and leave
IS_SANDBOX alone unless the opt-in env var is present; if you need to support
macOS VirtioFS convenience, document/require users to set the opt-in variable
before starting the container rather than assigning IS_SANDBOX=1 automatically.
Ensure references to RUNNER and IS_SANDBOX in docker-entrypoint.sh remain
consistent with this opt-in behavior so ClaudeProvider's getProcessUid() guard
is not silently bypassed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # as a warning and fall back to running as root so the container still starts | ||
| # rather than crash-looping. IS_SANDBOX=1 lets ClaudeProvider skip its UID-0 | ||
| # safety check (we're still inside Docker — sandboxed in the meaningful sense). | ||
| if chown -Rh appuser:appuser /.archon 2>/dev/null; then |
There was a problem hiding this comment.
2>/dev/null discards the actual chown error, making the warning on Line 20 unactionable
When chown fails, the only information in the warning message is "bind mount with incompatible options?" — the actual OS error (Operation not permitted, Read-only file system, etc.) is thrown away. On unexpected Linux failures this makes diagnosis very difficult.
🔍 Proposed fix — include the chown error in the warning
- if chown -Rh appuser:appuser /.archon 2>/dev/null; then
+ if chown_err=$(chown -Rh appuser:appuser /.archon 2>&1); then
RUNNER="gosu appuser"
else
- echo "WARNING: Could not fix ownership of /.archon (bind mount with incompatible options?) — running as root" >&2
+ echo "WARNING: Could not fix ownership of /.archon — running as root (chown said: ${chown_err})" >&2
export IS_SANDBOX=1
RUNNER=""
fiif cmd=$(command 2>&1); then routes both stdout and stderr into the variable while still using the command's exit status to pick the branch — no change to set -e behaviour.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if chown -Rh appuser:appuser /.archon 2>/dev/null; then | |
| if chown_err=$(chown -Rh appuser:appuser /.archon 2>&1); then | |
| RUNNER="gosu appuser" | |
| else | |
| echo "WARNING: Could not fix ownership of /.archon — running as root (chown said: ${chown_err})" >&2 | |
| export IS_SANDBOX=1 | |
| RUNNER="" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-entrypoint.sh` at line 17, The chown invocation currently silences
stderr with `2>/dev/null`, losing the real OS error; change the `if chown -Rh
appuser:appuser /.archon 2>/dev/null; then` pattern to capture stderr into a
variable (e.g. `output=$(chown -Rh appuser:appuser /.archon 2>&1)`) and use the
command's exit status to decide the branch, then include that captured `output`
in the warning message so the exact chown error (like "Operation not permitted"
or "Read-only file system") is logged.
| if chown -Rh appuser:appuser /.archon 2>/dev/null; then | ||
| RUNNER="gosu appuser" | ||
| else | ||
| echo "WARNING: Could not fix ownership of /.archon (bind mount with incompatible options?) — running as root" >&2 | ||
| export IS_SANDBOX=1 | ||
| RUNNER="" | ||
| fi |
There was a problem hiding this comment.
IS_SANDBOX=1 silently bypasses the UID-0 safety guard for any chown failure, not just macOS VirtioFS
The ClaudeProvider constructor (see packages/providers/src/claude/provider.ts Line 904) throws when getProcessUid() === 0 && IS_SANDBOX !== '1'. That guard exists to prevent running Claude with bypassPermissions as root. This PR unconditionally exports IS_SANDBOX=1 whenever chown fails — but inside a Linux container there is no way to distinguish a macOS VirtioFS bind mount from a Linux host with a misconfigured volume driver, an SELinux/AppArmor policy denial, or a wrong --mount type. In all those cases the safety guard is silently bypassed and the Claude binary runs as root.
A minimal mitigation is an explicit opt-in variable so the behaviour is intentional rather than automatic:
🛡️ Proposed fix — gate root fallback on explicit opt-in
- if chown -Rh appuser:appuser /.archon 2>/dev/null; then
+ if chown -Rh appuser:appuser /.archon 2>/tmp/_archon_chown_err; then
RUNNER="gosu appuser"
else
- echo "WARNING: Could not fix ownership of /.archon (bind mount with incompatible options?) — running as root" >&2
- export IS_SANDBOX=1
- RUNNER=""
+ _chown_msg=$(cat /tmp/_archon_chown_err 2>/dev/null); rm -f /tmp/_archon_chown_err
+ echo "WARNING: chown /.archon failed (${_chown_msg:-unknown reason})." >&2
+ if [ "${ARCHON_ALLOW_ROOT_FALLBACK:-0}" = "1" ]; then
+ echo "WARNING: ARCHON_ALLOW_ROOT_FALLBACK=1 — continuing as root with IS_SANDBOX=1." >&2
+ export IS_SANDBOX=1
+ RUNNER=""
+ else
+ echo "ERROR: Set ARCHON_ALLOW_ROOT_FALLBACK=1 (e.g. macOS VirtioFS) to allow root fallback, or fix volume ownership." >&2
+ exit 1
+ fi
fiThis keeps the macOS fix functional (users add one env var to their docker-compose.yml or docker run command) while preventing the safety guard from being bypassed silently on Linux hosts with inadvertent chown failures.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if chown -Rh appuser:appuser /.archon 2>/dev/null; then | |
| RUNNER="gosu appuser" | |
| else | |
| echo "WARNING: Could not fix ownership of /.archon (bind mount with incompatible options?) — running as root" >&2 | |
| export IS_SANDBOX=1 | |
| RUNNER="" | |
| fi | |
| if chown -Rh appuser:appuser /.archon 2>/tmp/_archon_chown_err; then | |
| RUNNER="gosu appuser" | |
| else | |
| _chown_msg=$(cat /tmp/_archon_chown_err 2>/dev/null); rm -f /tmp/_archon_chown_err | |
| echo "WARNING: chown /.archon failed (${_chown_msg:-unknown reason})." >&2 | |
| if [ "${ARCHON_ALLOW_ROOT_FALLBACK:-0}" = "1" ]; then | |
| echo "WARNING: ARCHON_ALLOW_ROOT_FALLBACK=1 — continuing as root with IS_SANDBOX=1." >&2 | |
| export IS_SANDBOX=1 | |
| RUNNER="" | |
| else | |
| echo "ERROR: Set ARCHON_ALLOW_ROOT_FALLBACK=1 (e.g. macOS VirtioFS) to allow root fallback, or fix volume ownership." >&2 | |
| exit 1 | |
| fi | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-entrypoint.sh` around lines 17 - 23, The script currently
unconditionally exports IS_SANDBOX=1 on any chown failure, silently bypassing
the root-safety guard in ClaudeProvider; change this so IS_SANDBOX is only set
when the user explicitly opted in (e.g., check if an opt-in env var is already
set like IS_SANDBOX=1 or a new explicit flag SANDBOX_FORCE=1) instead of
auto-exporting it on failure. Update the chown block around the chown -Rh
appuser:appuser /.archon check and the RUNNER assignment so that on failure you
print the warning and leave IS_SANDBOX alone unless the opt-in env var is
present; if you need to support macOS VirtioFS convenience, document/require
users to set the opt-in variable before starting the container rather than
assigning IS_SANDBOX=1 automatically. Ensure references to RUNNER and IS_SANDBOX
in docker-entrypoint.sh remain consistent with this opt-in behavior so
ClaudeProvider's getProcessUid() guard is not silently bypassed.
1 participant
Summary
chown -Rh appuser:appuser /.archonfails because the host controls ownership and host UIDs (e.g. 501) don't map to container'sappuser(1001). The script thenexit 1s and the container crash-loops.chownis now treated as a warning. Container falls back to running as root, withIS_SANDBOX=1exported soClaudeProviderskips its UID-0 safety check (we're still inside Docker — sandboxed in the meaningful sense).safe.directory-loop (already fixed by fix(docker): register safe.directory for all repos on bind-mount restart #1307), Linux behaviour (chown succeeds → unchanged path), Kubernetes/non-root behaviour (id != 0 → unchanged path), or the security model on Linux.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
Label Snapshot
risk: lowsize: XScorecore:docker-entrypointChange Metadata
bugcoreLinked Issue
safe.directoryissue but not thischownfailure mode.Validation Evidence (required)
bun run type-check,bun run lint,bun run test— touched file is a shell script, not TS; lint-staged was a no-op for.sh.Security Impact (required)
exit 1vs continuing).chownitself is unchanged; only the failure handling differs.IS_SANDBOX=1mirrors how Claude SDK itself handles known-sandboxed contexts; the container is still isolated by Docker.Compatibility / Migration
chownsucceeds are unchanged. Only the previously-fatal failure path is altered.IS_SANDBOX=1is exported in the fallback path. Users who already setIS_SANDBOXexplicitly are not overridden ifchownsucceeds.Human Verification (required)
gh_auth.status_okandcleanup_completedlog lines.:5174is reachable.--user-flag invocation (theelsebranch is unchanged structurally; behaviour-equivalent to pre-PR).Side Effects / Blast Radius (required)
chownto fail (highly unusual) would now silently get root-execution instead of exit. Mitigated by the explicitWARNING:log line.docker compose logs.IS_SANDBOX=1is observable in process env.Rollback Plan (required)
git revert <sha>). One-file change, no DB/state to migrate.ERROR: Failed to fix ownershipmessage — same symptom as before this PR.Risks and Mitigations
chownfails for an unrelated reason (filesystem error, capability missing) would now log a warning and run as root instead of failing fast.IS_SANDBOX=1skips aClaudeProviderUID safety check that exists for a reason.Summary by CodeRabbit