Skip to content

fix(install): root-aware sudo (A2, #1173) - #372

Merged
LukasWodka merged 5 commits into
developfrom
feat/lpi-sudo-a2
Jul 23, 2026
Merged

fix(install): root-aware sudo (A2, #1173)#372
LukasWodka merged 5 commits into
developfrom
feat/lpi-sudo-a2

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

What (A2)

The installer primed and ran ~70 privileged steps as sudo <cmd>. Two real users hit the wall:

  • A root container / VM — sudo is unnecessary, and often absent on minimal images, so sudo <cmd> died with sudo: command not found even though <cmd> would have run fine. (id -u returned 0, yet the install failed.)
  • "sudo isn't installed" was reported as "no sudo access" — a different and misleading fix.

How

Shadow sudo with a function in common.sh:

sudo() {
if [ "$(id -u)"-eq 0 ];then"$@"# root: run directly, no sudo binaryelif _have_sudo_bin;then _real_sudo "$@"# non-root: the real sudoelsereturn 127;fi# mimic sudo's not-found (best-effort friendly)
}
  • Every existing sudo <cmd> call site is fixed with no edit — the diff is confined to common.sh (only preflight_sudo and the sole sudo -v/sudo -n option-forms needed touching, via _real_sudo).
  • The no-sudo branch returns non-zero rather than exiting, so best-effort sites like sudo modprobe … || fallback still take their fallback. preflight_sudo aborts up front with an accurate message on the required path.
  • preflight_sudo now: root → return 0 (no priming); non-root + no sudo → accurate error; non-root + passwordless → return 0; else prime once via _real_sudo -v.
  • type -P finds the real binary past the shadow (a bare command -v sudo would resolve to the function). Helper seams _have_sudo_bin / _real_sudo make every branch unit-testable without a real sudo.

Tests

  • 6 new common.bats cases: root runs directly (no real sudo), non-root defers to real sudo, no-sudo returns 127, and the three preflight_sudo postures (root / no-sudo error / passwordless).
  • Full suite adds no new failures (3 failures pre-exist on clean develop: macOS bash-3.2 blindspot + locally-unstamped DEFAULT_REF; green on Linux CI). No new shellcheck findings; R8 manifest regenerated.

Part of the least-privilege install epic (#1168), Wave 1. Independent of #1171 (self-contained; #1172 routing will consume both).

Closes tracebloc/backend#1173

🤖 Generated with Claude Code


Note

Medium Risk
Changes how every privileged installer step runs (root vs sudo vs missing sudo); scope is shell helpers and messaging but mistakes could affect Docker/system package setup on diverse Linux hosts.

Overview
Root-aware privileged execution (RFC 0001 A2)common.sh now shadows sudo so existing sudo <cmd> call sites run the command directly when uid 0, delegate to the real binary when present, or return 127 when neither applies (preserving sudo … || fallback behavior). Helpers _have_sudo_bin / _real_sudo and export -f keep nested bash -c subshells on the same path.

preflight_sudo short-circuits on root, fails early with an explicit “sudo isn’t installed” message when appropriate, and uses _real_sudo for -v / -n and the credential keepalive.

probe.sh_probe_privilege uses those same helpers so host audit no longer treats the shadow as a real sudo binary (Bugbot #372).

Bats coverage for sudo/preflight branches, subshell inheritance, and set -e safety on old bash; probe privilege tests updated; manifest.sha256 refreshed.

Reviewed by Cursor Bugbot for commit 71f8d91. Bugbot is set up for automated code reviews on this repo. Configure here.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

Comment threadscripts/lib/common.sh
Comment threadscripts/lib/common.sh Outdated
LukasWodka added a commit that referenced this pull request Jul 23, 2026
…(review #372)
The sudo() shadow is a shell function; without `export -f`, `sudo <cmd>` inside
setup-linux.sh's `bash -c '…'` blocks (the apt-lock wait and the RHEL-rebuild
Alma/Rocky/OL Docker install) resolved the REAL sudo binary — so a root box
without sudo installed still hit "sudo: command not found" there, the exact case
A2 eliminates everywhere else. Export sudo + _real_sudo + _have_sudo_bin.
Harmless to non-bash children (they ignore BASH_FUNC_*). New bats asserts a
child bash inherits the shadow.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 43cb51c. Configure here.

divyasinghds
divyasinghds previously approved these changes Jul 23, 2026

@divyasinghdsdivyasinghds left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve. Root-aware sudo shadow. Verified no option-led (sudo -flag) call sites exist, so the root branch's "$@" is safe; -n/-v correctly route through _real_sudo. export -f / env_reset interaction reasoned through correctly. Highest blast radius in the stack but well-tested. Optional follow-up: a CI guard forbidding 'sudo -' outside _real_sudo to prevent regressions.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

LukasWodka added a commit that referenced this pull request Jul 23, 2026
type -P sudo prints "type: sudo: not found" to stderr when sudo is absent, which
leaked out right before preflight_sudo`'s clean "not root and sudo isn't
installed" message — undercutting exactly the accurate messaging A2 adds. Add
2>/dev/null.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadscripts/lib/probe.sh
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

Comment threadscripts/lib/common.sh Outdated
LukasWodka added a commit that referenced this pull request Jul 23, 2026
_have_sudo_bin wrapped `type -P sudo` in `[ -n "$(...)" ]`. On bash <4.4
(Amazon Linux 2's 4.2, and macOS's 3.2) a failing command substitution trips
`set -e` even when the helper is called from an `if`/`elif` — so the non-root,
no-sudo path that preflight_sudo is meant to handle with a clear message could
abort before that message ran (Bugbot #372).
Use `type -P sudo >/dev/null 2>&1` as the whole function body: no substitution,
same result (0 iff a real sudo binary is on PATH, ignoring the sudo() shadow),
still quiet. Add a regression test that runs the helper under `set -e` with no
sudo on PATH and asserts it returns cleanly instead of aborting.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1fe12ac. Configure here.

LukasWodka added a commit that referenced this pull request Jul 23, 2026
…(review #372)
The sudo() shadow is a shell function; without `export -f`, `sudo <cmd>` inside
setup-linux.sh's `bash -c '…'` blocks (the apt-lock wait and the RHEL-rebuild
Alma/Rocky/OL Docker install) resolved the REAL sudo binary — so a root box
without sudo installed still hit "sudo: command not found" there, the exact case
A2 eliminates everywhere else. Export sudo + _real_sudo + _have_sudo_bin.
Harmless to non-bash children (they ignore BASH_FUNC_*). New bats asserts a
child bash inherits the shadow.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Jul 23, 2026
type -P sudo prints "type: sudo: not found" to stderr when sudo is absent, which
leaked out right before preflight_sudo`'s clean "not root and sudo isn't
installed" message — undercutting exactly the accurate messaging A2 adds. Add
2>/dev/null.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Jul 23, 2026
_probe_privilege used `has sudo` + a bare `sudo -n true`. Both resolve to
the `sudo()` shell function common.sh installs (the A2 shadow), so on a host
with no sudo at all the probe saw "sudo present" and reported the posture as
sudo_pw instead of no_sudo — misrouting the install tier (Bugbot #372).
Switch to the same primitives preflight_sudo uses: _have_sudo_bin (type -P,
ignores functions) and _real_sudo (command sudo, bypasses the shadow). Update
the three privilege bats cases to mock those helpers.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit ee745f5. Configure here.

LukasWodkaand others added 5 commits July 23, 2026 11:26
…o missing" vs "no access" (A2, #1173)
The installer primed and ran ~70 privileged steps as `sudo <cmd>`, which broke
two real users: a root container/VM (sudo unnecessary, and often ABSENT on
minimal images, so `sudo <cmd>` died with "command not found") and reported
"sudo not installed" as "no sudo access" (a different, misleading fix).
Shadow `sudo` with a function in common.sh: as root, run <cmd> directly (no sudo
binary needed); else defer to the real sudo; else fail the way real sudo would
(non-zero) so best-effort `sudo … || fallback` sites still fall back. Every
existing call site is fixed with NO edit — the diff is confined to common.sh.
preflight_sudo now short-circuits on root, gives an accurate message when sudo is
genuinely absent, and primes credentials via the real sudo (`_real_sudo -v`).
type -P finds the real binary past the shadow; helper seams (_have_sudo_bin,
_real_sudo) make every branch unit-testable. 6 new bats; full suite adds no new
failures; shellcheck introduces no new findings; manifest regenerated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(review #372)
The sudo() shadow is a shell function; without `export -f`, `sudo <cmd>` inside
setup-linux.sh's `bash -c '…'` blocks (the apt-lock wait and the RHEL-rebuild
Alma/Rocky/OL Docker install) resolved the REAL sudo binary — so a root box
without sudo installed still hit "sudo: command not found" there, the exact case
A2 eliminates everywhere else. Export sudo + _real_sudo + _have_sudo_bin.
Harmless to non-bash children (they ignore BASH_FUNC_*). New bats asserts a
child bash inherits the shadow.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
type -P sudo prints "type: sudo: not found" to stderr when sudo is absent, which
leaked out right before preflight_sudo`'s clean "not root and sudo isn't
installed" message — undercutting exactly the accurate messaging A2 adds. Add
2>/dev/null.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_probe_privilege used `has sudo` + a bare `sudo -n true`. Both resolve to
the `sudo()` shell function common.sh installs (the A2 shadow), so on a host
with no sudo at all the probe saw "sudo present" and reported the posture as
sudo_pw instead of no_sudo — misrouting the install tier (Bugbot #372).
Switch to the same primitives preflight_sudo uses: _have_sudo_bin (type -P,
ignores functions) and _real_sudo (command sudo, bypasses the shadow). Update
the three privilege bats cases to mock those helpers.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_have_sudo_bin wrapped `type -P sudo` in `[ -n "$(...)" ]`. On bash <4.4
(Amazon Linux 2's 4.2, and macOS's 3.2) a failing command substitution trips
`set -e` even when the helper is called from an `if`/`elif` — so the non-root,
no-sudo path that preflight_sudo is meant to handle with a clear message could
abort before that message ran (Bugbot #372).
Use `type -P sudo >/dev/null 2>&1` as the whole function body: no substitution,
same result (0 iff a real sudo binary is on PATH, ignoring the sudo() shadow),
still quiet. Add a regression test that runs the helper under `set -e` with no
sudo on PATH and asserts it returns cleanly instead of aborting.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@divyasinghdsdivyasinghds left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at head 71f8d91 (3 new commits since my prior approval on 43cb51c; #370 has since merged to develop, so probe.sh is now in-tree). The new changes are correct and one fixes a real bug:

  • probe.sh_probe_privilege now detects the real sudo binary (_have_sudo_bin, via type -P) and bypasses the shadow (_real_sudo) instead of has sudo / a bare sudo -n true. This is the exact shadow interaction I raised on first pass: once the A2 sudo() function exists, command -v sudo resolves to it and a bare sudo -n true returns 127 on a sudo-less host — so the posture would be misreported as sudo_pw instead of no_sudo, which would defeat #374's Tier 2 + no_sudo fail-fast and let a genuinely-stuck host proceed and crash mid-install. Correctly fixed.
  • _have_sudo_bin dropped the [ -n "$(type -P sudo)" ] command substitution for a whole-body type -P sudo >/dev/null 2>&1set -e-safe on bash <4.4 (Amazon Linux 2's 4.2 / macOS 3.2), stderr silenced, and still finds the real binary past the shadow. Dedicated regression test added.

Coupling is sound: common.sh is sourced unconditionally before probe.sh, and this PR ships both edits plus the regenerated manifest atomically. CI green (29 pass, 1 skip); probe.bats / common.bats updated to match. Approve.

@LukasWodka
LukasWodka merged commit 5fd6487 into developJul 23, 2026
32 checks passed
LukasWodka added a commit that referenced this pull request Jul 24, 2026
…Bugbot r1)
The A2 sudo shadow runs '-n true' as a command when root, and 'has sudo'
is always true because the shadow function exists — use the #372
primitives like preflight_sudo/_probe_privilege do. The root-without-
sudo-binary strip is dropped entirely: the shadow already handles root by
executing the command directly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Jul 27, 2026
…load, no get-helm-3 (#396)
* feat(installer): self-serve Helm prerequisites — verified direct download, no get-helm-3 (#395)
The installer now takes care of its own requirements instead of erroring
out and telling the user to install tools:
- install_helm fetches the pinned Helm release directly from get.helm.sh
and verifies it against the published .sha256sum (fail-closed), exactly
like the k3d direct download (#382). helm's get-helm-3 script is gone —
it floats on the mutable helm/helm@main, performs its checksum step with
openssl (absent on minimal cloud images, Bugbot #383), and its fetches
are unbounded. openssl is no longer needed anywhere in the flow.
- HELM_VERSION pin in common.sh (v4.2.3; 'latest' resolves at install
time via get.helm.sh/helm-latest-version, same verified path; malformed
tags fail closed before any fetch).
- _ensure_unpack_tools: when tar/gzip are genuinely missing on the Tier 0
fast path (which skips install_system_deps), install them via the
package manager — quietly as root/passwordless sudo, with an honest
one-line reason when a password is needed — rather than aborting with
'go install tar'. install_system_deps drops openssl, adds gzip.
Supersedes the fatal-error approach in #395 (product call: never tell the
user to install tools we can install ourselves). Windows already fetches
Helm directly from get.helm.sh, so the ps1 path is untouched.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(installer): route sudo probes through _real_sudo/_have_sudo_bin (Bugbot r1)
The A2 sudo shadow runs '-n true' as a command when root, and 'has sudo'
is always true because the shadow function exists — use the #372
primitives like preflight_sudo/_probe_privilege do. The root-without-
sudo-binary strip is dropped entirely: the shadow already handles root by
executing the command directly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(installer): Tier 0 unpack install — sudo keepalive + dpkg-lock wait + one combined install (Bugbot r2)
_ensure_unpack_tools ran package installs on the Tier 0 path without the
full flow's guards: apt could sit on the dpkg lock invisibly behind the
spinner, and a long wait could outlast the just-primed sudo timestamp so
the next sudo re-prompts behind the spinner and hangs. Prime, then keep
the ticket warm (preflight_sudo's pattern; killed right after the install
— the zero-privilege tier shouldn't hold a warm admin ticket), wait out
the dpkg lock (bounded + visible), and install everything in ONE package-
manager call so there's a single sudo consumer right after priming.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(installer): keepalive ownership + latest-tag isolation (Bugbot r3)
- _ensure_unpack_tools kills only the keepalive IT started: on the Tier
1/2 recovery path SUDO_KEEPALIVE_PID belongs to preflight_sudo, and
killing it would let later privileged steps re-prompt behind a spinner.
The global is only claimed when empty (install_cleanup coverage) and
only cleared when it is ours.
- HELM_VERSION=latest: isolate the endpoint body with tail -1 — retry's
attempt notices go to stdout and a failed-then-successful fetch would
concatenate them into the captured tag, failing the anchored regex with
a false 'couldn't resolve'. (install_k3d's resolver is immune: its
${var##*/} strip discards anything before the redirect URL.)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(copy-catalog): regenerate 00-install golden for the HELM_VERSION knob
The PR adds a HELM_VERSION env var (common.sh), which surfaces one line in
install-k8s.sh --help. emit_install reads --help live, so the golden drifted
by exactly that line. Regenerated via TB_UPDATE_GOLDEN=1; verified locally
(bats copy-catalog now green).
* fix(installer): drop obsolete _ensure_helm_prereqs — Tier 0 no longer blocks on openssl (Bugbot #396)
get-helm-3 is gone (replaced by a sha256-verified direct download), so Helm
no longer needs openssl. The Tier-0 preflight still demanded openssl+tar and
failed fast, aborting minimal-image installs for a dependency Helm doesn't use
— and pre-empting _ensure_unpack_tools, which installs tar/gzip Tier-0-aware.
Remove the function and its Tier-0 call; drop the tests that encoded the old
openssl preflight (tar/gzip stays covered by _ensure_unpack_tools' own tests).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@LukasWodka
LukasWodka deleted the feat/lpi-sudo-a2 branch August 14, 2026 13:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@LukasWodka@divyasinghds@saadqbal@shujaatTracebloc