diff --git a/COVERAGE.md b/COVERAGE.md index 2e4f36a..05f161f 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -4,6 +4,19 @@ Measured on 2026-09-24 against upstream master (33c43b8), following the project decision 0003 (90 percent floor per repository, 95 percent for every file our changes touch). +## Branch fix/password-once-and-updates-record: shell 99.58, Python 99 (2026-10-02) + +`firstboot.d/95secupdates` is measured for the first time: 49 of 50 +lines, 98 percent, from 9 tests in `tests/test-secupdates.bats` (preseeded +SKIP and FORCE, Skip and Install on the screen, a failing screen, an +invalid preseed, a record that cannot be written, dpkg in an inconsistent +state with a new kernel arming 99reboot, no conf file). The line not run +is the TurnKey Hub status call, for a machine registered with the Hub. +Shell total 99.58, down from 99.76 only because a file under 100 percent +joined the measured set. Python: `libinithooks/dialog_wrapper.py` +unchanged at 98 percent (the same two lines missed as before), 373 tests; +total 99 percent. + ## Branch feat/first-boot-role: shell 99.76, Python 99 (2026-09-30) `lib/keel-firstboot.sh` 8/8, `firstboot.d/75keel-role` 4/4 and diff --git a/debian/changelog b/debian/changelog index d3fc805..37e53ba 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,25 @@ +inithooks (2.3.6+keel16) trixie; urgency=medium + + * A generated password is shown on one screen only. The screen that shows + it says "It is not shown again", and the question after it, "Did you + save this password?", showed it again, so the first screen's word was + not true. The question now asks "Did you save the password?" without + it; an operator who did not save it answers New and gets another one, + shown once as well. Found in the maintainer's screenshots of the Core + image (002 and 003), 2026-10-02. + * 95secupdates leaves its answer behind: skip or force, one line in + /var/lib/inithooks/sec-updates, preseeded or asked. Nothing else on the + machine records it (the cron-apt install action every image ships looks + the same either way), so keel inspect reported + security.updates_at_first_boot as force after the operator chose Skip + (screenshot 034); keel 0.15.1 reads this line. A line that cannot be + written is logged and the boot goes on. The hook takes + INITHOOKS_DEFAULT, SEC_UPDATES_RECORD and SEC_UPDATES_LOG from the + environment, for tests/test-secupdates.bats, its first tests (98 + percent of its lines under kcov). + + -- Marcos Mendez Fri, 02 Oct 2026 06:00:00 +0000 + inithooks (2.3.6+keel15) trixie; urgency=medium * The first boot names Keel Linux. The backtitle at the top of every diff --git a/firstboot.d/95secupdates b/firstboot.d/95secupdates index 043f4c8..d0e7e95 100755 --- a/firstboot.d/95secupdates +++ b/firstboot.d/95secupdates @@ -2,8 +2,9 @@ # install security updates # SEC_UPDATES: SKIP, FORCE (if none specified, will be interactive) +INITHOOKS_DEFAULT="${INITHOOKS_DEFAULT:-/etc/default/inithooks}" # shellcheck source=default/inithooks -source /etc/default/inithooks +source "$INITHOOKS_DEFAULT" if [[ -e "$INITHOOKS_CONF" ]]; then # shellcheck disable=SC1090 source "$INITHOOKS_CONF" @@ -13,6 +14,19 @@ fi grep -qs boot=live /proc/cmdline && exit 2 SEC_UPDATES="${SEC_UPDATES,,}" +# The answer, skip or force, is the one trace this hook leaves: the +# cron-apt install action every image ships looks the same either way, so +# keel inspect reads this line to report security.updates_at_first_boot. +SEC_UPDATES_RECORD="${SEC_UPDATES_RECORD:-/var/lib/inithooks/sec-updates}" +SEC_UPDATES_LOG="${SEC_UPDATES_LOG:-/var/log/inithooks/secupdates.log}" + +record() { + if ! { mkdir -p "$(dirname "$SEC_UPDATES_RECORD")" \ + && echo "$1" > "$SEC_UPDATES_RECORD"; } 2>/dev/null; then + logger -t inithooks -p warn \ + "[95secupdates] could not record $1 in $SEC_UPDATES_RECORD" + fi +} install_updates() { # if registered with hub, update with status @@ -20,8 +34,8 @@ install_updates() { hubclient-status sec-updates fi - LOGFILE=/var/log/inithooks/secupdates.log - mkdir -p /var/log/inithooks + LOGFILE=$SEC_UPDATES_LOG + mkdir -p "$(dirname "$LOGFILE")" # 'ls' stderr is suppressed as containers don't have the checked paths. If # any other errors occur we've got much bigger problems! # SC2012 is a shellcheck warning re use of 'ls'. 'ls' used to provide @@ -32,11 +46,11 @@ install_updates() { if [[ -n "$(dpkg --audit 2>/dev/null)" ]]; then msg="[95secupdates] dpkg in an inconsistent state (see $LOGFILE)" logger -t inithooks -p warn "$msg" - echo "WARNING: $msg" >> $LOGFILE - dpkg --audit 2>&1 | tee -a $LOGFILE + echo "WARNING: $msg" >> "$LOGFILE" + dpkg --audit 2>&1 | tee -a "$LOGFILE" fi DEBIAN_FRONTEND=noninteractive dpkg --force-confdef --force-confold \ - --configure -a 2>&1 | tee -a $LOGFILE + --configure -a 2>&1 | tee -a "$LOGFILE" apt-get update DEBIAN_FRONTEND=noninteractive apt-get autoclean -y DEBIAN_FRONTEND=noninteractive apt-get dist-upgrade -y \ @@ -44,7 +58,7 @@ install_updates() { -o Dir::Etc::sourceparts=/dev/null \ -o Dir::Etc::sourcelist=/etc/apt/sources.list.d/security.sources.sources \ -o DPkg::Options::=--force-confdef \ - -o DPkg::Options::=--force-confold | tee -a $LOGFILE + -o DPkg::Options::=--force-confold | tee -a "$LOGFILE" # per above note re containers # shellcheck disable=SC2012 @@ -56,9 +70,11 @@ install_updates() { # SEC_UPDATES preseeded - unset SEC_UPDATES will run interactive (below) if [[ "$SEC_UPDATES" == "skip" ]]; then + record skip logger -t inithooks -p warn "[95secupdates] security updates skipped" exit 0 elif [[ "$SEC_UPDATES" == "force" ]]; then + record force logger -t inithooks "[95secupdates] security updates being installed" install_updates exit 0 @@ -72,6 +88,7 @@ exit_code=0 $INITHOOKS_PATH/bin/secupdates-ask.py || exit_code=$? if [[ $exit_code -eq 99 ]]; then # secupdates-ask.py returns 99 if user selects 'skip' + record skip logger -t inithooks -p warn "[95secupdates] security updates skipped" exit 0 elif [[ $exit_code -ne 0 ]]; then @@ -80,6 +97,7 @@ elif [[ $exit_code -ne 0 ]]; then exit "$exit_code" else # exit_code == 0 + record force logger -t inithooks "[95secupdates] security updates being installed" install_updates fi diff --git a/libinithooks/dialog_wrapper.py b/libinithooks/dialog_wrapper.py index 064d160..5e5936e 100644 --- a/libinithooks/dialog_wrapper.py +++ b/libinithooks/dialog_wrapper.py @@ -554,11 +554,12 @@ def _generate_password_flow( colors=True, ) + # The screen above says the password is not shown again, so + # the question does not show it: an operator who did not save + # it answers New and gets another one, shown once as well. confirm = "\n".join( [ - self._centered("Did you save this password?", width), - "", - band, + self._centered("Did you save the password?", width), "", self._centered( "Saved: continue. New: discard it, show another.", diff --git a/tests/test-secupdates.bats b/tests/test-secupdates.bats new file mode 100644 index 0000000..fd709e7 --- /dev/null +++ b/tests/test-secupdates.bats @@ -0,0 +1,137 @@ +#!/usr/bin/env bats +# Tests for firstboot.d/95secupdates: the first boot's security updates, +# preseeded (SEC_UPDATES) or asked (bin/secupdates-ask.py), and the line +# the hook leaves of the answer. Nothing else on the machine records it: +# the cron-apt install action every image ships looks the same after Skip +# and after Install, so keel inspect said force after Skip (the +# maintainer's screenshot 034). +# +# apt-get, dpkg, logger and ls are stubs; secupdates-ask.py is a stub +# under INITHOOKS_PATH that exits with the status a test sets. + +bats_require_minimum_version 1.5.0 + +load helpers + +REPO=$BATS_TEST_DIRNAME/.. + +setup() { + setup_stubs + stub logger + stub apt-get + stub dpkg 'if [[ "$1" == --audit ]]; then echo "${DPKG_AUDIT-}"; fi' + # the module and boot listing before and after the upgrade: the same + # unless LS_CHANGES is set, when the second call differs + stub ls 'n=$(wc -l < "'"$STUBS"'/ls.calls") +if [[ -n "${LS_CHANGES-}" ]]; then echo "listing $n"; else echo listing; fi' + + export INITHOOKS_PATH=$BATS_TEST_TMPDIR/inithooks + mkdir -p "$INITHOOKS_PATH/bin" "$INITHOOKS_PATH/firstboot.d" + touch "$INITHOOKS_PATH/firstboot.d/99reboot" + printf '#!/bin/bash\nexit "${ASK_STATUS:-0}"\n' \ + > "$INITHOOKS_PATH/bin/secupdates-ask.py" + chmod +x "$INITHOOKS_PATH/bin/secupdates-ask.py" + + export INITHOOKS_CONF=$BATS_TEST_TMPDIR/inithooks.conf + export INITHOOKS_DEFAULT=$BATS_TEST_TMPDIR/default-inithooks + { + echo "INITHOOKS_PATH=$INITHOOKS_PATH" + echo "INITHOOKS_CONF=$INITHOOKS_CONF" + } > "$INITHOOKS_DEFAULT" + export SEC_UPDATES_RECORD=$BATS_TEST_TMPDIR/var/lib/inithooks/sec-updates + export SEC_UPDATES_LOG=$BATS_TEST_TMPDIR/secupdates.log + unset SEC_UPDATES DPKG_AUDIT LS_CHANGES ASK_STATUS +} + +@test "a preseeded SKIP installs nothing and records skip" { + echo "export SEC_UPDATES=SKIP" > "$INITHOOKS_CONF" + + run "$REPO/firstboot.d/95secupdates" + + [ "$status" -eq 0 ] + [ "$(cat "$SEC_UPDATES_RECORD")" = "skip" ] + [ -z "$(calls apt-get)" ] +} + +@test "a preseeded FORCE records force and installs the updates" { + echo "export SEC_UPDATES=FORCE" > "$INITHOOKS_CONF" + + run "$REPO/firstboot.d/95secupdates" + + [ "$status" -eq 0 ] + [ "$(cat "$SEC_UPDATES_RECORD")" = "force" ] + [[ "$(calls apt-get)" == *"dist-upgrade -y"* ]] + [ ! -x "$INITHOOKS_PATH/firstboot.d/99reboot" ] +} + +@test "Skip on the screen records skip" { + export ASK_STATUS=99 + + run "$REPO/firstboot.d/95secupdates" + + [ "$status" -eq 0 ] + [ "$(cat "$SEC_UPDATES_RECORD")" = "skip" ] + [ -z "$(calls apt-get)" ] +} + +@test "Install on the screen records force and installs" { + export ASK_STATUS=0 + + run "$REPO/firstboot.d/95secupdates" + + [ "$status" -eq 0 ] + [ "$(cat "$SEC_UPDATES_RECORD")" = "force" ] + [[ "$(calls apt-get)" == *"dist-upgrade -y"* ]] +} + +@test "a screen that fails records nothing and fails the hook" { + export ASK_STATUS=3 + + run "$REPO/firstboot.d/95secupdates" + + [ "$status" -eq 3 ] + [ ! -e "$SEC_UPDATES_RECORD" ] +} + +@test "an invalid preseed records nothing and fails the hook" { + echo "export SEC_UPDATES=maybe" > "$INITHOOKS_CONF" + + run "$REPO/firstboot.d/95secupdates" + + [ "$status" -eq 1 ] + [ ! -e "$SEC_UPDATES_RECORD" ] + [[ "$(calls logger)" == *"invalid preseed value: maybe"* ]] +} + +@test "a record that cannot be written is said and the boot goes on" { + echo "export SEC_UPDATES=SKIP" > "$INITHOOKS_CONF" + export SEC_UPDATES_RECORD=$BATS_TEST_TMPDIR/file/sec-updates + touch "$BATS_TEST_TMPDIR/file" + + run "$REPO/firstboot.d/95secupdates" + + [ "$status" -eq 0 ] + [[ "$(calls logger)" == *"could not record skip in $SEC_UPDATES_RECORD"* ]] +} + +@test "dpkg in an inconsistent state is logged, and a new kernel arms 99reboot" { + echo "export SEC_UPDATES=FORCE" > "$INITHOOKS_CONF" + export DPKG_AUDIT="libfoo is half configured" + export LS_CHANGES=1 + + run "$REPO/firstboot.d/95secupdates" + + [ "$status" -eq 0 ] + [[ "$(cat "$SEC_UPDATES_LOG")" == *"dpkg in an inconsistent state"* ]] + [[ "$(cat "$SEC_UPDATES_LOG")" == *"libfoo is half configured"* ]] + [ -x "$INITHOOKS_PATH/firstboot.d/99reboot" ] +} + +@test "without a conf file the screen is asked" { + export ASK_STATUS=99 + + run "$REPO/firstboot.d/95secupdates" + + [ "$status" -eq 0 ] + [ "$(cat "$SEC_UPDATES_RECORD")" = "skip" ] +} diff --git a/tests/test_dialog_wrapper.py b/tests/test_dialog_wrapper.py index c300ac7..0e31d40 100644 --- a/tests/test_dialog_wrapper.py +++ b/tests/test_dialog_wrapper.py @@ -182,8 +182,11 @@ def test_generated_password_is_shown_confirmed_and_returned(self): _, confirm, _, confirm_kwargs = d.console.calls[2] self.assertEqual(len(password), 20) self.assertEqual(shown_password(shown), password) - self.assertEqual(shown_password(confirm), password) self.assertIn("not shown again", shown) + # the screen said so, and the question after it keeps its word + self.assertNotIn(password, confirm) + self.assertNotIn("\\Zr", confirm) + self.assertIn("Did you save the password?", confirm) self.assertTrue(shown_kwargs["colors"]) self.assertEqual(confirm_kwargs["yes_label"], "Saved") self.assertIn("New", confirm_kwargs["no_label"]) @@ -253,10 +256,23 @@ def test_escape_in_the_generate_flow_never_skips_the_password(self): self.assertEqual( d.console.widgets(), ["menu", "msgbox", "msgbox", "yesno", "yesno"] ) - for call in d.console.calls[1:]: + for call in d.console.calls[1:3]: self.assertEqual(shown_password(call[1]), password) + for call in d.console.calls[3:]: + self.assertNotIn(password, call[1]) self.assertNotIn("quit", d.console.shown()) + def test_the_password_is_on_one_screen_only(self): + # "It is not shown again": a refused confirmation shows a new + # password once, and never the one it discarded + d = dialog(GENERATE, OK, CANCEL, OK, OK) + password = d.get_password("Root Password", "text") + bands = [call[1] for call in d.console.calls if "\\Zr" in call[1]] + self.assertEqual(len(bands), 2) + self.assertEqual(shown_password(bands[-1]), password) + self.assertEqual(sum(password in call[1] for call in d.console.calls), + 1) + def test_generator_refusing_the_length_falls_back_to_manual(self): d = dialog(GENERATE, OK, (OK, "Abcdefg1"), (OK, "Abcdefg1")) self.assertEqual(d.get_password("t", "x", gen_length=8), "Abcdefg1") @@ -463,14 +479,20 @@ def draw_text(): text = shown.removeprefix("\n") # wrapper() adds it self.assertEqual(width, 68, widget) self.assertEqual(height, d._calc_height(text, width), widget) - self.assertLess(height, d._calc_height(text), widget) + if password in text: + # the wide band wraps at the default width, not at 68 + self.assertLess(height, d._calc_height(text), widget) + self.assertEqual([call[0] for call in d.console.calls + if password in call[1]], ["msgbox", "msgbox"]) def test_generated_password_never_reaches_a_captured_stdout(self): def draw_text(): os.write(1, d.console.calls[-1][1].encode()) return OK - d = dialog(GENERATE, draw_text, draw_text) + # the password is on the first screen only; the question after it + # draws nothing here, so it cannot write over what the first drew + d = dialog(GENERATE, draw_text, OK) with mock.patch.object(dw, "TTY", self.tty): password = d.get_password("t", "x") self.assertIn(password.encode(), self.read(self.tty)) diff --git a/tests/test_setpass.py b/tests/test_setpass.py index 98d2812..24ab04a 100644 --- a/tests/test_setpass.py +++ b/tests/test_setpass.py @@ -64,7 +64,9 @@ def test_escape_in_the_generate_flow_still_sets_the_password(self): user, _, password = given.partition(":") self.assertEqual(user, "root") self.assertEqual(len(password), dw.GENERATED_LENGTH) - self.assertIn(password, console.calls[-1][1]) + # shown on the message, never on the question that follows it + self.assertIn(password, console.calls[3][1]) + self.assertNotIn(password, console.calls[-1][1]) self.assertEqual( console.widgets(), ["menu", "menu", "msgbox", "msgbox", "yesno", "yesno"],