fix: skip sudo/chown-root enforcement on macOS Docker Desktop (#64)

[dolphinsboy]

Summary

Fixes #64 — setup.sh --up fails on macOS Docker Desktop due to unnecessary sudo/chown root enforcement.

Root Cause

setup.sh had three EUID guards and two chown root:wheel + chmod 600 blocks that assumed a Linux multi-user server environment. On macOS Docker Desktop the current user already owns the Docker socket, so sudo is neither needed nor appropriate.

Changes (setup.sh, 5 locations)

1. Line ~866 — Early --up EUID guard: skip on Darwin
2. Line ~892 — --smoke-test EUID guard: skip on Darwin
3. Line ~1667 — Inner --up EUID guard: skip on Darwin
4. Line ~1744 — First chown+chmod block: Darwin → chmod 600 only; Linux → unchanged
5. Line ~2269 — Second chown+chmod block: Darwin → chmod 600 only; Linux → unchanged

Linux production path is completely unchanged. Bash syntax validated ✅

Testing

• macOS: ./setup.sh --non-interactive --ip 127.0.0.1 && ./setup.sh --up runs without sudo ✅
• Linux path: no changes to EUID guards or chown logic ✅

[lml2468]

[APPROVE] Clean, well-isolated fix. Linux path untouched; macOS path correctly relaxes ownership enforcement.

Verified

EUID guards (3 locations):
All three now have && [[ "$(uname)" != "Darwin" ]]. On Darwin the guard is skipped; on Linux the original fatal "sudo required" behavior is fully preserved. ✅

chown root blocks (2 locations):
Grep on HEAD confirms both chown "root:${ROOT_GROUP}" calls (lines 1755, 2280) are inside the else branch of if [[ "$(uname)" == "Darwin" ]]. Darwin takes the chmod 600-only path; Linux takes the unchanged chown root + chmod 600 path. ✅

Security posture on macOS:
chmod 600 on a single-user workstation makes .env readable only by the file owner — adequate for macOS Docker Desktop where there is no multi-user server threat model. ✅

No orphaned chown root calls on Darwin path. ✅

Non-blocking nit

🔵 $(uname) is evaluated 5+ times across the script (3 EUID guards + 2 chown blocks). Caching as readonly UNAME="$(uname)" near the top of the script would be marginally more efficient and easier to mock in tests. Low priority for a setup script.

Highlights

• Linux production path is byte-for-byte identical to main ✅
• Error messages on Darwin are concise and accurate
• Fixes a real first-run friction point for macOS contributors

[Jerry-Xin]

The PR is in scope and the macOS-specific sudo/chown relaxation is a relevant fix for this deployment script.

🔴 Blocking: None.

💬 Non-blocking

🟡 Warning — setup.sh still prints Linux-oriented sudo guidance on macOS after the new no-sudo path, e.g. setup.sh, setup.sh, setup.sh, and setup.sh. This does not break execution, but it will keep steering macOS Docker Desktop users toward the old workflow.

🟡 Warning — The Darwin sudo recovery path silently ignores failed ownership restoration: setup.sh and setup.sh. If logname is unavailable or chown fails, the script may leave docker/.env root-owned and later non-root --smoke-test will still fail. Consider using ${SUDO_USER:-$(logname ...)} and warning if the handoff cannot be completed.

🔵 Suggestion — The Darwin chmod/chown block is duplicated at setup.sh and setup.sh. A small helper would reduce future drift, but this is not required for correctness.

✅ Highlights

The Linux behavior remains on the original root-owned 600 path, and the EUID guard changes are correctly scoped to Darwin. bash -n setup.sh passes.

[lml2468]

Review at a6ada2f

Scope

Fixes #64: macOS Docker Desktop doesn't require sudo — the Docker socket is user-accessible. This PR:

1. Adds uname != Darwin guards to 3 sudo/EUID checks
2. Replaces 2 chown root:root + chmod 600 blocks with macOS-aware branching

Analysis

Security model is correct:

• Linux: .env locked to root:${ROOT_GROUP} mode 600 — multi-user server protection (unchanged)
• macOS: user-owned mode 600 — single-user workstation, Docker Desktop runs in user-space VM, root ownership is unnecessary and breaks non-sudo workflows

macOS sudo edge case handled well:
If someone runs sudo ./setup.sh --up on macOS, the script hands .env back to the real user via SUDO_USER / logname, so a subsequent non-sudo --smoke-test can read it. The || true on that chown is acceptable — it's best-effort ownership restoration, not a security-critical path.

Platform detection is sound:
uname == Darwin covers Docker Desktop, Colima, OrbStack — all macOS Docker runtimes run without root.

Verification

Check	Result
CI green	✅
Mergeable	✅
stat -f %Su is BSD (macOS) syntax, guarded by uname == Darwin	✅
Both .env lockdown paths (L1747, L2280) have identical macOS handling	✅
sudo/EUID guards at L869, L895, L1670 all consistently updated	✅

Findings

P2 (non-blocking)

1. 🔵 Code duplication — the 17-line macOS .env handling block is copy-pasted at two sites (L1747 and L2280). Extract to a function like lock_env_file() for DRY. But the existing Linux block was already duplicated, so this is consistent with the codebase style.

2. 🔵 Linux sudo guidance still printed on macOS — Allen flagged this too. Lines like "Re-run as 'sudo ./setup.sh ...'" appear in other code paths that aren't gated by the new uname checks. Follow-up to sweep remaining Linux-only messaging.

Recommendation

Clean, focused fix from an external contributor. Security model is sound — macOS single-user workstation doesn't need root lockdown, and the sudo edge case is properly handled. Ship it.

[Jerry-Xin]

Project relevance gate: PASS. This PR modifies setup.sh, which is directly part of the repository’s deployment workflow.

No blocking issues found; the macOS-specific skip is scoped to Darwin and preserves the Linux root-owned .env path.

💬 Non-blocking

🟡 Warning — setup.sh and setup.sh: the macOS sudo recovery path ignores chown failure with || true. If SUDO_USER is missing or chown fails, docker/.env may remain root-owned with mode 600, and a later non-root --smoke-test will still fail. Consider warning or verifying ownership after the attempted hand-back.

🟡 Warning — setup.sh, setup.sh, setup.sh: several comments and operator-facing messages still describe sudo/root ownership as universal. The runtime behavior is now platform-specific, so these should eventually be made Darwin-aware to avoid confusing macOS users.

🔵 Suggestion — setup.sh, setup.sh, setup.sh: consider defining a single IS_DARWIN variable near the existing ROOT_GROUP detection. That would avoid repeated uname calls and keep platform logic consistent.

🔵 Suggestion — add a small shell test or scripted check that mocks uname=Darwin and verifies non-root --up / --smoke-test no longer hit the EUID guard while Linux still does.

✅ Highlights

The Linux production lockdown path remains intact at setup.sh and setup.sh.

The change is narrowly targeted to macOS Docker Desktop behavior and does not relax Linux root:${ROOT_GROUP} enforcement.

bash -n setup.sh passes.

[yujiawei]

Code Review — PR #73 (octo-deployment)

Summary

Fixes #64 — setup.sh --up failed on macOS Docker Desktop because three EUID guards and two chown root:${ROOT_GROUP} + chmod 600 blocks unconditionally enforced a Linux multi-user server posture. This PR adds [[ "$(uname)" != "Darwin" ]] to the three EUID guards and branches the two .env lockdown blocks on uname == Darwin. Linux production behavior is byte-for-byte unchanged. bash -n passes.

Verification

Check	Result
All 3 EUID guards (L869, L895, L1670) now skip on Darwin via && [[ "$(uname)" != "Darwin" ]]	✅
Linux non-root --up / --smoke-test still fatals with the original message	✅
Both .env lockdown blocks (L1746–1767, L2279–2300) put chown "root:${ROOT_GROUP}" strictly inside the else (Linux) branch	✅
Darwin branch uses chmod 600 only and, when re-entered under sudo, hands the file back to ${SUDO_USER:-$(logname …)} so a subsequent non-sudo --smoke-test can read it	✅
stat -f %Su (BSD-only) is reachable only inside the uname == Darwin branch	✅
ROOT_GROUP resolution (L66) already accounts for Darwin's wheel, so the Linux branch stays correct on either platform	✅
bash -n setup.sh	✅

Findings

P0 / P1 — None

No correctness, security, build, or functional-blocker issues. Linux path is unmodified; the macOS additions are correctly scoped.

P2 / nits (non-blocking)

1. chown failure is swallowed on macOS sudo path (L1756, L2289)
   chown "${REAL_USER}" "${ENV_OUT}" || true deliberately ignores a failed ownership handoff. If SUDO_USER and logname both yield empty, the chown is skipped silently; if chown itself fails, the file stays root:wheel 600. A subsequent non-sudo --smoke-test on macOS would then trip the EACCES preflight in env_get (L585) with a clear remediation, so the failure mode is recoverable — but a one-line warn when the handoff is skipped or fails would make the dead-end easier to diagnose. Acceptable as-is.

2. Duplicated 17-line Darwin block at L1746–1767 and L2279–2300
   The pre-existing Linux block was already duplicated across these two --up branches, so the duplication is consistent with current style. Extracting lock_env_file() would be a small DRY win for the next reviewer that has to keep them in sync.

3. $(uname) evaluated 5+ times (3 EUID guards + 2 chown blocks)
   Caching a readonly IS_DARWIN near the existing ROOT_GROUP detection at L66 would centralize the platform predicate and make future Darwin-specific branches easier to spot. Pure ergonomics.

4. Linux-oriented sudo guidance still printed on macOS
   Several follow-up messages and comments (e.g. the "Next step: sudo ./setup.sh --smoke-test" hint at L2313, and surrounding comments at L1705 / L2242) still describe the sudo+root-ownership model as universal. On macOS the smoke-test path no longer needs sudo, so these strings would benefit from a Darwin-aware variant in a follow-up sweep. Not blocking — they don't break execution, only nudge macOS users toward the old workflow.

Highlights

• Scope discipline is excellent: every Darwin change is an additive guard or a new branch; no existing Linux line is modified semantically.
• The macOS sudo ./setup.sh --up edge case is handled — file ownership is handed back to the invoking user so the documented --smoke-test follow-up still works without sudo on macOS.
• Resolves a real first-run friction point that the README explicitly promised was supported.

Recommendation

Approve. The four P2 nits above are worth a follow-up but do not block merge.