fix: close the four known installer bugs (DU-H1, DU-H2, BUG-1, BUG-2)
- DU-H1: flags are parsed wherever they sit, so `install --unattended` and `--unattended install` are the same run; any unknown flag, word, or subcommand exits 2 to stderr before a package manager is touched. - DU-H2: every download lands in one private mktemp -d (mode 700) workdir per run, is checked non-empty before sudo tar sees it, and an EXIT/INT/TERM trap cleans up. No fixed /tmp paths remain. - BUG-1: ^t is now toggle-shown — it ticks only the rows the active filter is showing, and @needs expansion stops at the first invasive row, so an invasive package can never be ticked off-screen. - BUG-2: ^t journals what it added, so a second ^t over the same shown set unticks exactly that set; the bind no longer clears the query. - lab: the type verb polls fzf's reported query to a deadline instead of a fixed sleep; marks_settled retries within its deadline. Suite 256/0 host, 214/0 docker (ubuntu:24.04), mutations 24/24 killed (six new mutants re-introduce each bug and all die), lab 6/6 green.
This commit is contained in:
bcherb2
+175
-3
@@ -162,6 +162,57 @@ is "bws-secrets stays a non-package" "-" \
|
||||
is "no manifest row installs bws" "0" \
|
||||
"$(awk -F'\t' '!/^[#@]/ && NF>=3 && $2=="bws" {c++} END{print c+0}' "$M")"
|
||||
|
||||
printf '\n\033[1m^t — the bulk toggle over the shown rows\033[0m\n'
|
||||
# ^t is `toggle-shown`, not `toggle`, and these are the two bugs that made it a
|
||||
# separate command. Pure state-file logic, so it is checked here rather than in
|
||||
# the lab: the picker only ever hands it the keys of the matched rows.
|
||||
#
|
||||
# BUG-1: the walk widened past the filter. Typing `nvidia` shows exactly three
|
||||
# gpu rows; gpu/container-toolkit @needs docker, and all three docker rows are
|
||||
# invasive — "the docker group is root-equivalent". One keystroke ticked them
|
||||
# off screen.
|
||||
reset; cp state/selected state/before
|
||||
sh $D toggle-shown p:gpu/nvidia-driver p:gpu/cuda-toolkit p:gpu/container-toolkit >/dev/null
|
||||
is "^t over 'nvidia' ticks exactly the three rows on screen" \
|
||||
"gpu/container-toolkit
|
||||
gpu/cuda-toolkit
|
||||
gpu/nvidia-driver" \
|
||||
"$(grep -vxF -f state/before state/selected | sort || true)"
|
||||
is "…and the invasive group it @needs stays off" "0/3" "$(grp docker)"
|
||||
# BUG-2: and the same keystroke twice is a no-op. The reverse @needs closure
|
||||
# cannot do this — nothing needs the gpu rows — so the journal is what undoes it.
|
||||
sh $D toggle-shown p:gpu/nvidia-driver p:gpu/cuda-toolkit p:gpu/container-toolkit >/dev/null
|
||||
sort -u state/before > state/before.s; sort -u state/selected > state/after.s
|
||||
is "^t ^t returns the selection exactly as it was" "" \
|
||||
"$(diff state/before.s state/after.s 2>&1 || :)"
|
||||
# The guard is on `invasive`, not on "anything off screen": a safe dependency is
|
||||
# still pulled in, and the undo still gives it back.
|
||||
reset; sh $D preset none >/dev/null; cp state/selected state/before
|
||||
sh $D toggle-shown p:agents/codex >/dev/null
|
||||
is "^t still pulls in a SAFE @needs" "on" "$(on core/node)"
|
||||
sh $D toggle-shown p:agents/codex >/dev/null
|
||||
sort -u state/before > state/before.s; sort -u state/selected > state/after.s
|
||||
is "…and ^t ^t hands that back too, node included" "" \
|
||||
"$(diff state/before.s state/after.s 2>&1 || :)"
|
||||
# One row, both commands, so the difference is not hidden behind a bulk case.
|
||||
reset; sh $D preset none >/dev/null; sh $D toggle-shown p:gpu/container-toolkit >/dev/null
|
||||
is "^t never auto-ticks an invasive dependency" "0/3" "$(grp docker)"
|
||||
is "…but does tick the visible row it was handed" "on" "$(on gpu/container-toolkit)"
|
||||
reset; sh $D preset none >/dev/null; sh $D toggle p:gpu/container-toolkit >/dev/null
|
||||
is "space on that row is unchanged — you asked for that one" "3/3" "$(grp docker)"
|
||||
# With no journal to undo, ^t is cmd_toggle's plain all-on/all-off rule.
|
||||
reset; sh $D toggle-shown g:media >/dev/null
|
||||
is "^t on an already-full group turns it off" "0/3" "$(grp media)"
|
||||
reset; sh $D toggle-shown p:core/node >/dev/null
|
||||
is "…and that all-off still drops what needed it" "off" "$(on agents/codex)"
|
||||
is "the picker binds ^t to toggle-shown, never the plain toggle" "1" \
|
||||
"$(grep -c 'ctrl-t:select-all+execute-silent($SELF toggle-shown ' $D)"
|
||||
# clear-query used to hang off that bind. It has to be gone, not merely
|
||||
# unused: it drops the filter, so the second ^t is over the whole manifest and
|
||||
# the undo becomes "tick everything".
|
||||
is "…and does not drop the query out from under the undo" "0" \
|
||||
"$(grep -c -- '+clear-query' $D || true)"
|
||||
|
||||
printf '\n\033[1mcredentials never reach a log\033[0m\n'
|
||||
# `run` echoes its whole argv to stderr. The private init's argv ends in
|
||||
# https://user:TOKEN@host, so it must not go through `run`.
|
||||
@@ -453,9 +504,14 @@ hasnt "…including a listening ssh port" "openssh-server" "$out"
|
||||
has "…but still installs the safe defaults" "ripgrep" "$out"
|
||||
# Determinism: the same command twice, on the same machine, means the same
|
||||
# thing. A state file left by an interactive run must not change it.
|
||||
a=$(box --unattended --print 2>/dev/null || true)
|
||||
# Normalised on the one thing that is MEANT to differ between two runs: every
|
||||
# run gets its own mktemp'd download directory now (DU-H2), and the printed
|
||||
# commands name it. What must not differ is the selection, which is what this
|
||||
# assertion has always been about.
|
||||
strip_wd() { sed 's|/dotup\.[^/]*/|/dotup.XXXXXX/|g'; }
|
||||
a=$(box --unattended --print 2>/dev/null | strip_wd || true)
|
||||
box preset none >/dev/null
|
||||
b=$(box --unattended --print 2>/dev/null || true)
|
||||
b=$(box --unattended --print 2>/dev/null | strip_wd || true)
|
||||
is "unattended is computed, not inherited" "same" \
|
||||
"$([ "$a" = "$b" ] && echo same || echo different)"
|
||||
# `set -e` would kill the subshell at the failing command, so the status is
|
||||
@@ -479,7 +535,9 @@ MIN=${TMPDIR:-/tmp}/dotup-minbin.$$
|
||||
rm -rf "$MIN"; mkdir -p "$MIN"
|
||||
# Resolved by looking, not by `command -v`: an interactive shell can report an
|
||||
# alias or a bare name for these, and a symlink to a bare name is a loop.
|
||||
for t in awk grep sed sort cut tr id uname mkdir cp mv rm cat head sh dirname basename; do
|
||||
# mktemp and chmod earn their place the same way the rest do: dotup gives every
|
||||
# run its own private download directory (DU-H2), and it makes it here.
|
||||
for t in awk grep sed sort cut tr id uname mkdir cp mv rm cat head sh dirname basename mktemp chmod; do
|
||||
for bd in /usr/bin /bin /usr/local/bin; do
|
||||
[ -x "$bd/$t" ] && { ln -sf "$bd/$t" "$MIN/$t"; break; }
|
||||
done
|
||||
@@ -577,6 +635,56 @@ newbox; pick core/ripgrep docker/docker-ce
|
||||
box install >/dev/null 2>&1 || true
|
||||
has "with a human present the same tick does install" "docker-ce" "$(cat "$LOG")"
|
||||
|
||||
printf '\n\033[1mflags are read wherever they sit — DU-H1\033[0m\n'
|
||||
# The production incident. The old parser stopped at the first bare word and
|
||||
# left everything behind it in "$@", where nothing ever looked at it again --
|
||||
# so `dotup install --unattended`, which is how half the people who type this
|
||||
# type it, ran a FULL ATTENDED install and put every ticked invasive package on
|
||||
# the machine. Silence is the worst possible answer to a flag: the operator has
|
||||
# no way to tell the run they asked for from the run they got.
|
||||
#
|
||||
# These are the same assertions the flags-first section above makes, with the
|
||||
# words the other way round. Both orders are one command line.
|
||||
newbox; BOXPATH=$SEALSUDO
|
||||
pick core/ripgrep docker/docker-ce networking/openssh-server
|
||||
out=$(box install --unattended 2>&1 | plain || true)
|
||||
log=$(cat "$LOG")
|
||||
has "install --unattended refuses invasive too" "refusing invasive packages" "$out"
|
||||
hasnt "…so the daemon never reaches a package manager" "docker-ce" "$log"
|
||||
hasnt "…nor does the listening ssh port" "openssh-server" "$log"
|
||||
has "…and the safe package still installs" "ripgrep" "$log"
|
||||
# --print is the flag whose absence is most expensive to discover afterwards.
|
||||
newbox; BOXPATH=$SEALSUDO; pick core/ripgrep
|
||||
out=$(box install --print 2>&1 | plain || true)
|
||||
is "install --print installs nothing" "" \
|
||||
"$(grep -E ' (install|remove|upgrade) ' "$LOG" || true)"
|
||||
has "…while still printing what it would" "apt-get install -y" "$out"
|
||||
# A flag on each side is one command line, not two halves of one.
|
||||
newbox; BOXPATH=$SEALSUDO; pick core/ripgrep docker/docker-ce
|
||||
out=$(box --print install --unattended 2>&1 | plain || true)
|
||||
has "flags on both sides are all parsed" "refusing invasive packages" "$out"
|
||||
is "…and the run still changed nothing" "" \
|
||||
"$(grep -E ' (install|remove|upgrade) ' "$LOG" || true)"
|
||||
# The other half of DU-H1, and the half that keeps the fix honest: a flag or a
|
||||
# word this script does not recognise must STOP the run. A typo that installs
|
||||
# the wrong set of packages is worse than one that installs nothing.
|
||||
newbox; BOXPATH=$SEALSUDO; pick core/ripgrep
|
||||
is "an unknown flag after the subcommand is refused" "2" "$(boxrc install --nonsense)"
|
||||
is "…an unknown word after it too" "2" "$(boxrc install extra)"
|
||||
is "…and an unknown subcommand" "2" "$(boxrc instal)"
|
||||
is "…and a short flag nobody defined" "2" "$(boxrc install -Q)"
|
||||
is "a refused command line installs nothing at all" "" "$(cat "$LOG")"
|
||||
has "…and names the argument it did not understand" "unknown flag: --nonsense" \
|
||||
"$(box install --nonsense 2>&1 | plain || true)"
|
||||
has "…or the command, when that is what was wrong" "unknown command: instal" \
|
||||
"$(box instal 2>&1 | plain || true)"
|
||||
# Operands must survive the same parser: these are how the fzf bindings call
|
||||
# back into dotup, and a picker whose every tick errored would be worse still.
|
||||
is "a subcommand's own operands still reach it" "not selected" \
|
||||
"$(box explain p:media/sox 2>&1 | plain | sed -n 's/^state *//p')"
|
||||
is "…several of them, in order" "0" \
|
||||
"$(boxrc toggle p:media/sox p:media/ffmpeg)"
|
||||
|
||||
printf '\n\033[1ma package that fails is reported, and the run says so\033[0m\n'
|
||||
# Reachable only because the fakes can be made to fail. Every assertion here
|
||||
# survived deleting the code it is about, for want of a way to make brew lose.
|
||||
@@ -679,6 +787,70 @@ is "the tarball is extracted where the symlink points" "$xt" "$xl"
|
||||
is "…into the directory that was cleared and recreated" "$xt $xt" "$xr"
|
||||
is "…and that directory is /opt/nvim" "/opt/nvim" "$xt"
|
||||
|
||||
printf '\n\033[1mdownloads land in a private directory, never a fixed /tmp path — DU-H2\033[0m\n'
|
||||
# The tarball handlers wrote /tmp/nvim.tgz, /tmp/node.tgz and /tmp/go.tgz and
|
||||
# then unpacked them with `sudo tar`. Any account on the box could pre-create
|
||||
# those names as symlinks, or swap the file in the window between the download
|
||||
# and the extract, and have tar write their content anywhere, as root.
|
||||
#
|
||||
# Static half first, because it covers handlers no test drives -- go and node
|
||||
# both write outside $HOME and have no business running here at all.
|
||||
is "no download is written to a fixed /tmp path" "" \
|
||||
"$(grep -nE -- "-o +'?/tmp/|-o +'?.\{TMPDIR:-/tmp\}/" $D || true)"
|
||||
is "…and nothing is extracted from one" "" \
|
||||
"$(grep -nE -- "tar [^|]*-x[a-z]*f +'?/tmp/" $D || true)"
|
||||
is "the working directory comes from mktemp -d" "1" \
|
||||
"$(grep -c 'mktemp -d "\${TMPDIR:-/tmp}/dotup\.' $D)"
|
||||
is "…and its mode is stated rather than assumed" "1" \
|
||||
"$(grep -c 'chmod 700 "\$WORKDIR"' $D)"
|
||||
is "…and a trap removes it however the run ends" "1" \
|
||||
"$(grep -c '^trap dotup_cleanup EXIT INT TERM' $D)"
|
||||
# cmd_private REPLACES that trap with one of its own. If it does not do the
|
||||
# cleanup too, every run that reaches the private tier leaks its directory.
|
||||
is "…including the trap the private tier installs" "1" \
|
||||
"$(grep -c "dotup_cleanup. EXIT INT TERM" $D)"
|
||||
|
||||
# Behavioural half. The directory is gone by the time the run exits, so the
|
||||
# fake curl records it and its mode from the inside (see fakebin/_curl).
|
||||
newbox; BOXPATH=$SEALSUDO; pick apps/chrome
|
||||
box install >/dev/null 2>&1 || true
|
||||
wdline=$(grep '^curl-outdir ' "$LOG" | head -1 || true)
|
||||
is "control: the run recorded where it downloaded to" "yes" \
|
||||
"$([ -n "$wdline" ] && echo yes || echo no)"
|
||||
is "the deb lands in a directory made for this run" "yes" \
|
||||
"$(case ${wdline:-} in *" $SB/tmp/dotup."*) echo yes ;; *) echo "no [$wdline]" ;; esac)"
|
||||
is "…which nobody else can read into" "700" \
|
||||
"$(printf '%s\n' "$wdline" | awk '{print $3}')"
|
||||
is "…and which does not outlive the run" "" \
|
||||
"$(ls "$SB/tmp" 2>/dev/null || true)"
|
||||
# Predictability was the whole bug, so two runs must not agree on the name.
|
||||
# Same box, so the only thing that can differ is what mktemp chose.
|
||||
one=$(printf '%s\n' "$wdline" | awk '{print $2}')
|
||||
: > "$LOG"
|
||||
box install >/dev/null 2>&1 || true
|
||||
two=$(grep '^curl-outdir ' "$LOG" | head -1 | awk '{print $2}' || true)
|
||||
is "control: the second run named one as well" "yes" \
|
||||
"$([ -n "$two" ] && echo yes || echo no)"
|
||||
is "no two runs choose the same directory" "different" \
|
||||
"$([ "$one" = "$two" ] && echo same || echo different)"
|
||||
|
||||
# The neovim handler, read back out of dotup's own dry run: what it downloads
|
||||
# and what it hands to `sudo tar` must be the same file, and that file must not
|
||||
# be under a name anyone could have written down in advance.
|
||||
newbox; pick core/neovim
|
||||
dry=$(box --print install 2>&1 | plain || true)
|
||||
dl=$(printf '%s\n' "$dry" | sed -n "s/.*-o '\([^']*nvim\.tgz\)'.*/\1/p" | head -1)
|
||||
ex=$(printf '%s\n' "$dry" | sed -n "s/.*tar -xzf '\([^']*\)' -C .*/\1/p" | head -1)
|
||||
is "control: the dry run names a download path" "yes" \
|
||||
"$([ -n "$dl" ] && echo yes || echo no)"
|
||||
is "the tarball is extracted from the file just downloaded" "$dl" "$ex"
|
||||
is "…out of this run's own directory, not /tmp/nvim.tgz" "yes" \
|
||||
"$(case $dl in "$SB/tmp/dotup."*/nvim.tgz) echo yes ;; *) echo "no [$dl]" ;; esac)"
|
||||
# Nothing may reach `sudo tar` that was not checked to be there and non-empty:
|
||||
# a truncated proxy error page is a zero-byte file, and tar's complaint about
|
||||
# one is not a sentence anyone can act on.
|
||||
has "…and only if it arrived non-empty" "[ -s '$dl' ]" "$dry"
|
||||
|
||||
printf '\n\033[1mthe picker fetches its own fzf, from the real release URL\033[0m\n'
|
||||
# The stand-in for GitHub answers ONE path -- the release download URL -- and
|
||||
# builds the tarball from the version named in it. So a preflight that reports
|
||||
|
||||
Reference in New Issue
Block a user