fix(#427): grant docker group on any daemon run; refuse sudo-wrapped installs - #511
Merged
Merged
Conversation
…installs Two compounding identity bugs on Linux: 1. The docker-group grant ran ONLY inside the fresh-Docker-install branch. On a box where Docker was already present and the user wasn't in the group, the else-branch printed "Docker" without granting, and the recovery path dead-ended at "Could not connect to Docker. Try logging out and back in…" — which couldn't help, because membership was never granted. Re-runs looped on the same message. Fix: after the install/else, ensure the invoking user is in the docker group whenever the daemon path is chosen, regardless of a fresh install. Skip if already a member (no redundant usermod); prepare-host stays exempt (only TB_PREPARE_USER is granted, later — least-privilege, #381). The existing sg-docker re-exec then activates the new membership in-session, so no dead-end loop. 2. Nothing was SUDO_USER-aware: `sudo bash install.sh` ran the WHOLE provision as root — usermod granted root (not the user), and ~/.tracebloc, ~/.kube/config, and the chmod-600 credential landed root-owned under /root, with no chown anywhere to undo it. The installer's model is to run as the daily user and elevate per-step (RFC-0002), so rather than a fragile ownership remap, refuse the sudo-wrapped full run early (before any file is created): refuse_sudo_wrapped_install errors when EUID 0 AND $SUDO_USER is a real (non-root) user. Exemptions: a genuine root login (no SUDO_USER); prepare-host (the admin path, already dispatched+exited in main()). Because we never run as root-with-SUDO_USER, every $HOME/$USER path in the tree stays correct with no remap. Also: honor TB_PREPARE_USER as the grant target in the main install (new _real_install_user helper; the #418 Windows peer). Tests: install_docker_engine grants on the pre-installed path + when only TB_PREPARE_USER differs, skips when already a member, never grants the admin in prepare-host, still grants on a fresh install; refuse_sudo_wrapped_install refuses sudo+SUDO_USER but allows root-login / sudo -i / non-root; _real_install_user. Closes #427
…PARE_USER in the refuse hint Bugbot: - The main-install docker-group grant targeted _real_install_user (TB_PREPARE_USER when set), but socket access and the sg-docker re-exec key off $USER. When they differ (e.g. a leftover `export TB_PREPARE_USER=` from prepare-host), the invoking user never got membership and hit the same dead-end this PR fixes. Grant $USER directly — the sudo-wrapped run is already refused, so $USER is the real daily user — and keep TB_PREPARE_USER on the prepare-host path only (where it's granted). Removed the now-unused _real_install_user helper + its tests. - The sudo-refuse hint pointed admins at a BARE `prepare-host`, but run_prepare_host only grants when TB_PREPARE_USER is set — so that remedy prepares the daemon and grants nobody. Name it: `export TB_PREPARE_USER=<user> && … prepare-host`. Tests: the grant targets $USER even with a leftover TB_PREPARE_USER; the refuse hint includes TB_PREPARE_USER=<user>. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d952cc3. Configure here.
…(Bugbot r2)
The sudo-refuse prepare-host remedy filled TB_PREPARE_USER=${SUDO_USER}, but SUDO_USER
is the ADMIN who ran sudo — the "setting up for someone else" case targets a DIFFERENT
researcher. Following it would grant the admin docker-group access and leave the
intended user locked out (the #377 least-privilege footgun). Use a
<researcher-username> placeholder and say "not yourself". Test updated to require the
placeholder and reject $SUDO_USER.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
previously approved these changes
Jul 31, 2026
saadqbal
left a comment
Contributor
There was a problem hiding this comment.
Careful, well-documented PR 👍 Both bugs are real, the fix sits at the right depth (grant on any daemon run + refuse early before any file lands), and the bats coverage is thorough. One small non-blocking nit inline; manifest + shellcheck check out locally.
… the grant (reviewer)
The grant resolved its target with a `${USER:-$(id -un)}` fallback, but the in-session
sg-docker re-exec guard still keyed off bare `$USER`. In the exact USER-unset case the
fallback exists for, the grant landed on `$(id -un)` while that guard saw an empty
`$USER`, skipped the re-exec, and re-introduced the "log out and back in" dead-end.
Hoist _grant_user to the top of install_docker_engine and use it in both places so
they can't disagree. Guard test added.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
approved these changes
Jul 31, 2026
Contributor
|
/fr-pass |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

#427 — grant docker group on any daemon run; refuse sudo-wrapped installs
Two compounding identity bugs on Linux:
1. docker-group grant was gated on a fresh Docker install
The grant ran only inside the
if ! has dockerbranch. On a box where Docker was already present and the user wasn't in thedockergroup, the else-branch printedDockerwithout granting, and the recovery path dead-ended at "Could not connect to Docker. Try logging out and back in…" — which can't help, because membership was never granted. Every re-run looped on the same message.Fix: after the install/else, ensure the invoking user is in the
dockergroup whenever the daemon path is chosen — regardless of a fresh install. Skips if already a member (no redundantusermod);prepare-hoststays exempt (onlyTB_PREPARE_USERis granted, later — least-privilege, #381). The existingsg dockerre-exec then activates the new membership in-session, so no dead-end loop.2. Nothing was
SUDO_USER-awaresudo bash install.shran the whole provision as root:usermod -aG dockergranted root (not the user), and~/.tracebloc,~/.kube/config, and the chmod-600 credential landed root-owned under /root — locking the daily user out, with nochownanywhere to undo it.The installer's model is to run as the daily user and elevate per-step (RFC-CLIENT-0002). Rather than a fragile ownership remap, refuse the sudo-wrapped full run early (before any file is created):
refuse_sudo_wrapped_installerrors whenEUID 0and$SUDO_USERis a real (non-root) user, pointing at running without sudo or usingprepare-host. Exemptions: a genuine root login (noSUDO_USER);sudo -i(SUDO_USER=root); andprepare-host(the admin path, already dispatched-and-exited inmain()). Because we never run as root-with-SUDO_USER, every$HOME/$USERpath in the tree stays correct with no remap — that's why this is minimal.Also honors
TB_PREPARE_USERas the grant target in the main install (new_real_install_user; the #418 Windows peer).Acceptance
dockerto root (it refuses before creating anything).Tests
install_docker_engine: grants on the pre-installed path, grants when onlyTB_PREPARE_USERdiffers, skips when already a member, never grants the admin underprepare-host, still grants on a fresh install (regression).refuse_sudo_wrapped_install: refusessudo+SUDO_USER, allows root-login /sudo -i/ non-root._real_install_user:$USERdefault +TB_PREPARE_USERoverride.shellcheck --severity=error+ check-style + check-drift clean;scripts/manifest.sha256regenerated (R8).Closes #427
Note
Medium Risk
Touches privileged installer identity (docker group, early abort on sudo) on the Linux bootstrap path; mistakes could block installs or mis-grant socket access, but scope is install scripts and tests only.
Overview
Fixes two Linux installer identity bugs: docker-group membership and sudo-wrapped full runs.
Docker group:
install_docker_enginenow adds the invoking user to thedockergroup whenever the daemon path runs—not only on a fresh Docker install. If Docker was already installed but the user wasn’t in the group, re-runs used to hit “log out and back in” without ever callingusermod, causing a loop. Membership is skipped when already present;prepare-hoststill does not grant the admin (researcher grant stays inrun_prepare_host). Grant andsg dockerre-exec both use a shared_grant_user(USERorid -un) so they stay aligned whenUSERis unset.Sudo refusal:
refuse_sudo_wrapped_installruns inmainbefore config validation or log setup. It aborts when the effective UID is root andSUDO_USERis a real non-root user (typicalsudo bash install.sh), with guidance to re-run without sudo or useprepare-hostwithTB_PREPARE_USER. Genuine root logins andSUDO_USER=rootare allowed.Bats coverage for these paths;
scripts/manifest.sha256updated.Reviewed by Cursor Bugbot for commit ff9d71b. Bugbot is set up for automated code reviews on this repo. Configure here.