ceos/unsloth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

install.sh: do not assume sudo consent when there is no terminal (#7435)

Browse source 
* install.sh: do not assume sudo consent when there is no terminal (#7307 P7)

_smart_apt_install printed an "Accept? [Y/n]" prompt, and when /dev/tty was
unreadable it set REPLY=y and escalated anyway. Every sudo call in that branch
redirects stdin from /dev/null, so on any host where sudo needs a password the
install died on sudo's own error rather than the actionable message the no-sudo
path already prints. Containers, CI and locked-down corporate machines hit this.

Probe with `sudo -n true` first. If there is no terminal to prompt on and sudo
would need a password, exit with the missing packages and the exact command to
run, matching the no-sudo path. Passwordless sudo still escalates unattended,
which is the one case where that is legitimate, and says so in the log.

With a readable /dev/tty the behaviour is unchanged, and the prompt now only
prints when something can actually answer it.

Extend tests/sh/test_apt_distro_prompt.sh to drive the real function across all
four TTY/sudo combinations, rewriting /dev/tty to a fixture path the same way
the existing cases rewrite /etc/os-release. Against the old install.sh five of
these assertions fail. Register the file in studio-backend-ci.yml's shell suite,
which did not run it before.

* install.sh: probe the real tty and the real sudo commands (#7307)

Codex review follow-ups on the no-TTY sudo escalation guard.

`test -r /dev/tty` only reads the device node's permission bits. Inside
containers and systemd units those bits look fine while open() fails with
ENXIO, so the guard still fell through to a prompt nobody could answer.
_can_read_tty() does a real open. The subshell is load-bearing: in dash a
failed redirection on the special builtin `:` exits the script.

`sudo -n true` proves only that `true` is allowed. Under a command-specific
rule like `NOPASSWD: /usr/bin/apt-get` it is the wrong question in both
directions. _sudo_runs_unattended() asks the sudoers policy about the exact
argument vectors we are about to elevate, via `sudo -n -l --`, which checks
without running and fails instead of prompting.

Tests cover both: a NOPASSWD-on-trivia-but-not-apt-get sudoers stub, and a
readable-but-unopenable /dev/tty faked with a unix socket (skipped where the
platform cannot produce that shape).

* install.sh: test sudo by running it with -n, not by asking sudo -l

Codex follow-up. `sudo -n -l -- apt-get ...` answers authorization, not
authentication: on a host where apt-get is permitted but still carries the
PASSWD tag, list mode exits 0 while the actual run needs a password, so the
guard reported unattended and the escalation died exactly as #7307 described.

Inferring the answer from list output means parsing for `!authenticate`, which
is human-readable text that varies by sudo version. Drop the inference. In the
no-terminal branch, run the real commands with `sudo -n`: -n never prompts, so
it cannot block on a closed stdin, and its exit status is the question we were
trying to answer. If it is refused, print the actionable manual command as
before. The terminal branch is unchanged: prompt, then plain sudo, which may
ask for a password because someone is there to type it.

The test stub now models sudo properly (-n refuses and runs nothing when a
password is needed) instead of special-casing the probe's argv.

* install.sh: require a real NOPASSWD rule, and stop blaming the password for apt failures

Two review findings on the headless escalation branch.

A cached authentication timestamp from an earlier, unrelated elevation made
`-n` succeed for a PASSWD-tagged apt-get, so packages installed with nobody
having answered the prompt. Add `-k` so the probe ignores the timestamp and
only a real NOPASSWD rule counts as passwordless. Per sudo(8), `-k` alongside
a command ignores the cached credentials for that invocation and "will not
update the user's cached credentials", so an interactive session elsewhere
does not have to re-authenticate afterwards.

A nonzero status from the elevated apt-get was reported as "likely needs a
password" even when sudo had authenticated fine and apt itself failed on a bad
repository, a dpkg lock or a network outage. sudo returns the command's own
exit status when the command runs, so the two cases are not distinguishable
from the status alone. Report both possibilities and point at the real error.

tests/sh/test_apt_distro_prompt.sh: teach the sudo stub about -k, add a cached
mode, and assert both behaviours. The three new assertions fail against the
previous commit.

* install.sh: an unreadable answer at the consent prompt declines

_can_read_tty proves the device opens, not that anyone is there to answer. A
read that hits EOF still fell back to REPLY=y and escalated, so the branch that
does have a terminal kept the behaviour this change removes from the branch
that does not. A drained or half-closed terminal reached it.

Default to n instead, which is what the post-install autostart prompt at the
bottom of this file already does on the same condition. Enter still means yes:
that is a successful read of an empty line, not a failed read.

tests/sh/test_apt_distro_prompt.sh: add an eof tty fixture, which opens
normally and returns EOF immediately. Both new assertions fail against the
previous commit.

* install.sh: tighten the escalation comments, and correct the exit-status claim

Comment-only. The earlier note said a nonzero status from the elevated apt-get
was not distinguishable from the status alone; sudo(8) is more specific than
that. sudo exits 1 on an authentication or configuration failure and passes the
command's own status through when the command runs, while apt-get(8) returns
100 on error, so the two usually are distinguishable. sudo also exits 1 when
the command cannot be executed, which is why the message still states both
causes rather than naming one.

* install.sh, tests: tighten the comments added by this branch

Comment-only pass over the branch's own comments in both files. Same intent,
fewer lines: drop restatement, keep the parts a reader cannot derive from the
code (why test -r is the wrong probe, why the subshell around the redirection
is load-bearing under dash, what -k buys over -n, and why a nonzero status
does not by itself name the cause).

Verified to touch nothing but comments and blank lines.

---------

Co-authored-by: danielhanchen <unslothai@gmail.com>
This commit is contained in:
Leo Borcherding 2026-07-26 06:27:15 -05:00 • committed by GitHub
commit c3d3680e7c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 216 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

64
install.sh
View file

@ -655,6 +655,15 @@ _apt_distro_description() {
)
}
# ── Helper: can the controlling terminal actually be opened for reading? ──
# `test -r` only checks permission bits, which look fine in containers and
# systemd units where open() then fails with ENXIO. Probe with a real open.
# The subshell is required: in dash a failed redirection on the special
# builtin `:` exits the whole script.
_can_read_tty() {
( : </dev/tty ) >/dev/null 2>&1
}
# ── Helper: install packages via apt, escalating to sudo only if needed ──
# Usage: _smart_apt_install pkg1 pkg2 pkg3 ...
_smart_apt_install() {
@ -695,24 +704,51 @@ _smart_apt_install() {
echo " from your distro's official repositories (not a third-party tarball)."
echo " !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
echo ""
printf " Accept? [Y/n] "
if [ -r /dev/tty ]; then
read -r REPLY </dev/tty || REPLY="y"
if _can_read_tty; then
printf " Accept? [Y/n] "
# The device opened, so a failed read is EOF, not consent: decline,
# as the autostart prompt below does. Enter is still yes (a
# successful read of an empty line).
read -r REPLY </dev/tty || REPLY="n"
case "$REPLY" in
[nN]*)
echo ""
echo " Please install these packages first, then re-run Unsloth Studio setup:"
echo " sudo apt-get update -y && sudo apt-get install -y $_STILL_MISSING"
exit 1
;;
esac
sudo apt-get update -y </dev/null
sudo apt-get install -y $_STILL_MISSING </dev/null
else
REPLY="y"
fi
case "$REPLY" in
[nN]*)
# Nobody can answer a prompt or type a password here. -n makes sudo
# refuse rather than prompt into a closed stdin, which is how #7307
# died. Probe with the real commands: `sudo -l` answers whether they
# are *authorized*, not whether running them needs authentication.
# -k ignores any cached timestamp, so only a real NOPASSWD rule gets
# through, not someone's sudo in another shell minutes ago. Per
# sudo(8), -k alongside a command ignores the cached credentials and
# "will not update" them, so other sessions keep theirs.
echo " No terminal to confirm on; trying passwordless sudo."
if sudo -n -k apt-get update -y </dev/null &&
sudo -n -k apt-get install -y $_STILL_MISSING </dev/null; then
echo " Installed with passwordless sudo."
else
echo ""
echo " Please install these packages first, then re-run Unsloth Studio setup:"
echo " Could not install these packages: $_STILL_MISSING"
echo " Detected ${_ad_desc}."
# Either sudo refused, or apt failed on a bad repo, dpkg lock or
# network outage. sudo exits 1 on an auth/config problem and
# when the command cannot be executed, but otherwise passes the
# command's own status through, so state both causes.
echo " Either sudo needs a password here, or apt-get itself"
echo " failed; see the error above. With no terminal to"
echo " authenticate on, this cannot be done unattended."
echo " Please install them first, then re-run Unsloth Studio setup:"
echo " sudo apt-get update -y && sudo apt-get install -y $_STILL_MISSING"
exit 1
;;
*)
sudo apt-get update -y </dev/null
sudo apt-get install -y $_STILL_MISSING </dev/null
;;
esac
fi
fi
else
echo ""
echo " sudo is not available on this system."

166
tests/sh/test_apt_distro_prompt.sh
View file

@ -89,6 +89,172 @@ assert_contains "mentions apt-get" "$_smart" 'sudo apt-get'
assert_contains "mentions official repos" "$_smart" "official repositories"
assert_contains "rejects tarball worry" "$_smart" "not a third-party tarball"
# ── No-TTY sudo escalation (#7307 Problem 7) ────────────────────────
# The old code assumed consent when /dev/tty was unreadable, then ran sudo with
# stdin closed, so a password-requiring host died on a raw sudo error. Drive the
# real function with /dev/tty rewritten to a fixture, the same trick used for
# /etc/os-release above, so every TTY state is reachable hermetically.
echo "=== _smart_apt_install no-TTY escalation ==="
# Closest portable stand-in for the /dev/tty inside containers and systemd
# units: the mode bits satisfy `test -r`, but open() fails with ENXIO. Callers
# must verify the shape before relying on it.
make_unopenable() {
python3 -c 'import socket,sys; socket.socket(socket.AF_UNIX).bind(sys.argv[1])' \
"$1" 2>/dev/null
}
# $1 tty: "tty" | "notty" | "unopenable"
# $2 sudo: "nopasswd" | "needspasswd" | "aptneedspasswd" | "cached" | "absent"
run_smart() {
_tty_mode="$1"; _sudo_mode="$2"
_d=$(mktemp -d -p "$_TMP_ROOT")
case "$_tty_mode" in
tty) printf 'y\n' > "$_d/tty" ;;
# Opens fine but reads EOF straight away (drained/half-closed
# terminal): openable is not the same as answerable.
eof) : > "$_d/tty" ;;
unopenable) make_unopenable "$_d/tty" ;;
esac
_f=$(mktemp -p "$_TMP_ROOT")
sed -n -e '/^_can_read_tty()/,/^}/p' \
-e '/^_smart_apt_install()/,/^}/p' "$INSTALL_SH" \
| sed -e "s#/dev/tty#$_d/tty#g" > "$_f"
(
TAURI_MODE=false
_apt_distro_description() { echo "TestOS 1.0 (debian-like)"; }
_is_pkg_installed() { return 1; } # nothing ever installs
apt-get() { return 1; } # unprivileged attempt fails
command() {
if [ "$1" = -v ] && [ "$2" = sudo ]; then
[ "$_sudo_mode" != absent ]; return $?
fi
builtin command "$@"
}
# Models real sudo: -n refuses (exit 1, nothing runs) when a password
# would be needed. -k ignores any cached timestamp for this invocation
# (sudo(8)), so only a real NOPASSWD rule counts as passwordless.
sudo() {
_noninteractive=false
_ignore_cache=false
while :; do
case "$1" in
-n) _noninteractive=true; shift ;;
-k) _ignore_cache=true; shift ;;
*) break ;;
esac
done
if [ "$_noninteractive" = true ]; then
case "$_sudo_mode" in
nopasswd) ;;
# A valid timestamp from an earlier, unrelated sudo. Without
# -k this looks passwordless; with -k it must not.
cached) [ "$_ignore_cache" = true ] && return 1 ;;
# Authorized for everything, NOPASSWD only on trivial
# commands: `sudo -l` says yes while execution still needs
# a password. Authorization is not the question to ask.
aptneedspasswd)
case " $* " in
*" apt-get "*) return 1 ;;
esac
;;
*) return 1 ;;
esac
fi
echo "SUDO_RAN: $*"
}
# shellcheck disable=SC1090
. "$_f"
_smart_apt_install cmake 2>&1
echo "EXIT:$?"
) || true
}
_out=$(run_smart notty needspasswd)
assert_contains "no tty + password sudo: says it cannot run unattended" \
"$_out" "cannot be done unattended"
assert_contains "no tty + password sudo: gives the manual command" \
"$_out" "sudo apt-get update -y && sudo apt-get install -y cmake"
assert_contains "no tty + password sudo: names the distro" \
"$_out" "TestOS 1.0 (debian-like)"
case "$_out" in
*SUDO_RAN*) echo " FAIL: no tty + password sudo must not run apt-get as root"; FAIL=$((FAIL + 1)) ;;
*) echo " PASS: no tty + password sudo runs nothing as root"; PASS=$((PASS + 1)) ;;
esac
case "$_out" in
*"Accept? [Y/n]"*) echo " FAIL: must not print an unanswerable prompt"; FAIL=$((FAIL + 1)) ;;
*) echo " PASS: no dangling Accept? prompt without a tty"; PASS=$((PASS + 1)) ;;
esac
# Passwordless sudo is the one case where unattended escalation is legitimate.
_out=$(run_smart notty nopasswd)
assert_contains "no tty + passwordless sudo: still installs" "$_out" "SUDO_RAN: apt-get install -y cmake"
assert_contains "no tty + passwordless sudo: says why it proceeded" \
"$_out" "passwordless sudo"
# A readable tty must behave exactly as before: prompt, then honour the answer.
_out=$(run_smart tty needspasswd)
assert_contains "tty present: still prompts" "$_out" "Accept? [Y/n]"
assert_contains "tty present: accepts and installs" "$_out" "SUDO_RAN: apt-get install -y cmake"
# No sudo at all keeps its own message.
_out=$(run_smart notty absent)
assert_contains "no sudo binary: unchanged message" "$_out" "sudo is not available on this system"
# A /dev/tty that passes `test -r` but cannot be opened counts as no tty.
# Only assert where the platform can actually produce that shape.
_probe=$(mktemp -d -p "$_TMP_ROOT")
if make_unopenable "$_probe/tty" && [ -r "$_probe/tty" ] && ! ( : <"$_probe/tty" ) 2>/dev/null; then
_out=$(run_smart unopenable needspasswd)
assert_contains "unopenable tty: treated as no tty" "$_out" "cannot be done unattended"
case "$_out" in
*"Accept? [Y/n]"*) echo " FAIL: unopenable tty must not print a prompt"; FAIL=$((FAIL + 1)) ;;
*) echo " PASS: unopenable tty prints no prompt"; PASS=$((PASS + 1)) ;;
esac
else
echo " SKIP: this platform cannot fake a readable-but-unopenable /dev/tty"
fi
# A tty that opens but yields EOF must decline: a failed read is nobody
# answering, and calling that "yes" escalates through the branch that does
# have a terminal.
_out=$(run_smart eof needspasswd)
assert_contains "eof tty: declines instead of escalating" \
"$_out" "Please install these packages first"
case "$_out" in
*SUDO_RAN*) echo " FAIL: eof tty must not escalate"; FAIL=$((FAIL + 1)) ;;
*) echo " PASS: eof tty runs nothing as root"; PASS=$((PASS + 1)) ;;
esac
# A cached timestamp from an earlier, unrelated sudo must not count as
# passwordless: nobody answered this run's prompt and the apt-get rule still
# carries PASSWD. Asserts the -k is present and effective.
_out=$(run_smart notty cached)
assert_contains "cached credentials: says it cannot run unattended" \
"$_out" "cannot be done unattended"
case "$_out" in
*SUDO_RAN*) echo " FAIL: a cached timestamp must not authorise unattended install"; FAIL=$((FAIL + 1)) ;;
*) echo " PASS: cached credentials run nothing as root"; PASS=$((PASS + 1)) ;;
esac
# The failure message must not blame a password when apt itself failed: sudo
# passes the command's own exit status through when the command runs.
assert_contains "failure message does not blame a password exclusively" \
"$_out" "or apt-get itself"
# Authorized for apt-get but not NOPASSWD on it. Both `sudo -n true` and
# `sudo -n -l -- apt-get ...` read this as unattended, since list mode answers
# authorization, not authentication. Only running it with -n is truthful.
_out=$(run_smart notty aptneedspasswd)
assert_contains "apt-get needs a password: says it cannot run unattended" \
"$_out" "cannot be done unattended"
case "$_out" in
*SUDO_RAN*) echo " FAIL: apt-get needing a password must not run as root"; FAIL=$((FAIL + 1)) ;;
*) echo " PASS: apt-get needing a password runs nothing as root"; PASS=$((PASS + 1)) ;;
esac
echo ""
echo "Results: $PASS passed, $FAIL failed"
[ "$FAIL" -eq 0 ]