Ratchet MORNING_END_HOUR, in the direction it actually runs

adjust_shutdown_schedule.sh compared only $1 and $2, so the third value passed
through unchecked in both directions: the morning shutdown window could be
shortened at will while the two evening hours were guarded.

The fix is not the obvious one. day-specific-shutdown-check.sh shuts down while
the clock is BELOW morning_end_minutes, so a LARGER MORNING_END_HOUR means a
longer shutdown window -- stricter. Copying the evening hours' `-gt` comparison
would have ratcheted it backwards and blessed exactly the loosening it was
meant to stop. The guard is `-lt`, and the inversion is now stated in the
header rather than left to be rediscovered.

Two smaller fixes in the same path: validation accepted 24 while its own error
message said 0-23, and the config is now unset before being sourced so an
unreadable file falls back to the documented loosest defaults instead of
inheriting whatever those names last held.

--restore is deliberately unchanged. It looks like a bypass, and it is one, but
it is load-bearing: shutdown_base.json says 21 while the live config is 23,
because the workout reward pushes the schedule later THROUGH --restore.
Bounding it at the base -- as was originally proposed -- would have capped the
reward at 21 and broken a daily flow. Bounding it needs a policy ceiling, which
is a decision, not a refactor.

The script is now sourceable, with the config path overridable, so the ratchet
has tests at all: 20 cases covering both directions, the source-clobber trap,
and the missing/unreadable config fallbacks. A ratchet with no test is how the
$3 gap survived for months.

Verified in a vmbox sandbox as root against a real /etc and a real chattr (the
immutable flag genuinely applied, which a tmpfs cannot do): a stricter write
lands, a looser evening hour is refused, a shortened morning window is refused
with the reason, and --restore still loosens. Not exercised against the host's
own /etc/shutdown-schedule.conf, deliberately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B5jMviyk15MKw1PqW1pzKr

screen_locker/adjust_shutdown_schedule.sh

@@ -1,93 +1,155 @@
 #!/bin/bash
-# Helper script to adjust shutdown schedule
-# This script should be allowed via sudoers for the workout locker
+# Helper script to adjust the shutdown schedule, allowed via sudoers.
 #
-# Usage: sudo adjust_shutdown_schedule.sh [--restore] <mon_wed_hour> <thu_sun_hour> <morning_end_hour>
+# Usage: sudo adjust_shutdown_schedule.sh [--restore] <mon_wed> <thu_sun> <morning_end>
 #
-# --restore: Allow restoring to original (possibly later) times
-#            Without this flag, only stricter (earlier) times are allowed
+# Without --restore only STRICTER schedules are accepted. "Stricter" is not the
+# same direction for all three values, which is the subtlety this script got
+# wrong for months:
+#
+#   MON_WED_HOUR / THU_SUN_HOUR  -> smaller is stricter (shut down earlier)
+#   MORNING_END_HOUR             -> LARGER is stricter
+#
+# day-specific-shutdown-check.sh shuts down while the clock is *below*
+# morning_end_minutes, so raising MORNING_END_HOUR lengthens the morning
+# shutdown window. Comparing it with `-gt`, as the two evening hours do, would
+# ratchet it backwards and quietly allow the window to be shortened.
 #
 # Add to /etc/sudoers.d/workout-locker:
-#   <username> ALL=(root) NOPASSWD: /home/kuhy/testsAndMisc/python_pkg/screen_locker/adjust_shutdown_schedule.sh
+#   <username> ALL=(root) NOPASSWD: /home/kuhy/screen-locker/screen_locker/adjust_shutdown_schedule.sh
 
 set -euo pipefail
 
-CONFIG_FILE="/etc/shutdown-schedule.conf"
-GUARD_NAME="shutdown-schedule"
+# Overridable so the test harness can point at a fixture tree. Production never
+# sets these; a test that forgets to is writing to the real /etc, which is why
+# the harness sets both explicitly rather than relying on a default.
+CONFIG_FILE="${SHUTDOWN_CONFIG_FILE:-/etc/shutdown-schedule.conf}"
+GUARD_NAME="${SHUTDOWN_GUARD_NAME:-shutdown-schedule}"
+readonly CONFIG_FILE GUARD_NAME
 
-CANONICAL_FILE="$(guardctl file-guard canonical-path "$GUARD_NAME" 2>/dev/null || true)"
-if [[ -z "$CANONICAL_FILE" ]]; then
-    echo "Error: guard-lib instance '$GUARD_NAME' is not installed (guardctl file-guard canonical-path returned empty)" >&2
-    exit 1
-fi
+readonly MAX_HOUR=23
 
-# Check for --restore flag
-RESTORE_MODE=false
-if [[ "${1:-}" == "--restore" ]]; then
-    RESTORE_MODE=true
-    shift
-fi
+usage() {
+	echo "Usage: $0 [--restore] <mon_wed_hour> <thu_sun_hour> <morning_end_hour>" >&2
+	exit 1
+}
 
-# Validate arguments
-if [[ $# -ne 3 ]]; then
-    echo "Usage: $0 [--restore] <mon_wed_hour> <thu_sun_hour> <morning_end_hour>" >&2
-    exit 1
-fi
+# Validate that every argument is an hour in [0, 23].
+validate_hours() {
+	local hour
+	for hour in "$@"; do
+		if ! [[ "$hour" =~ ^[0-9]+$ ]] || [[ "$hour" -gt $MAX_HOUR ]]; then
+			echo "Error: Hours must be integers between 0 and ${MAX_HOUR}" >&2
+			return 1
+		fi
+	done
+}
 
-MON_WED_HOUR="$1"
-THU_SUN_HOUR="$2"
-MORNING_END_HOUR="$3"
+# Reject a schedule that is looser than the one currently on disk.
+#
+# Reads the OLD values by sourcing the config, which overwrites any same-named
+# variable in this shell -- so the new values must be passed in as arguments and
+# never read from a global. Getting that wrong compares a value with itself and
+# always passes.
+check_stricter_only() {
+	local new_mon_wed="$1" new_thu_sun="$2" new_morning_end="$3"
+	local old_mon_wed old_thu_sun old_morning_end
 
-# Validate hours are integers between 0-23
-for hour in "$MON_WED_HOUR" "$THU_SUN_HOUR" "$MORNING_END_HOUR"; do
-    if ! [[ "$hour" =~ ^[0-9]+$ ]] || [[ "$hour" -lt 0 ]] || [[ "$hour" -gt 24 ]]; then
-        echo "Error: Hours must be integers between 0 and 23" >&2
-        exit 1
-    fi
-done
+	[[ -f "$CONFIG_FILE" ]] || return 0
 
-# Read current values to check if we're making schedule stricter
-if [[ -f "$CONFIG_FILE" ]] && [[ "$RESTORE_MODE" == false ]]; then
-    # shellcheck source=/dev/null
-    source "$CONFIG_FILE" 2>/dev/null || true
-    OLD_MON_WED="${MON_WED_HOUR:-24}"
-    OLD_THU_SUN="${THU_SUN_HOUR:-24}"
+	# Cleared first so a config that is present but unreadable falls back to the
+	# defaults below instead of silently inheriting whatever these names last
+	# held. In production the script is a fresh process and they are unset
+	# anyway; this makes the function honest when called twice in one shell.
+	unset MON_WED_HOUR THU_SUN_HOUR MORNING_END_HOUR
+	# shellcheck source=/dev/null
+	source "$CONFIG_FILE" 2>/dev/null || true
+	# Defaults are the LOOSEST value in each direction, so an unreadable config
+	# can never reject a legitimate first write.
+	old_mon_wed="${MON_WED_HOUR:-$MAX_HOUR}"
+	old_thu_sun="${THU_SUN_HOUR:-$MAX_HOUR}"
+	old_morning_end="${MORNING_END_HOUR:-0}"
 
-    # Reset variables to new values for comparison
-    # shellcheck disable=SC2034
-    MON_WED_HOUR_OLD="$OLD_MON_WED"
-    # shellcheck disable=SC2034
-    THU_SUN_HOUR_OLD="$OLD_THU_SUN"
+	if [[ "$new_mon_wed" -gt "$old_mon_wed" ]] ||
+		[[ "$new_thu_sun" -gt "$old_thu_sun" ]]; then
+		echo "Error: Can only make schedule stricter (earlier shutdown times)" >&2
+		echo "Use --restore flag to restore original times" >&2
+		return 1
+	fi
 
-    # Only allow making schedule stricter (earlier shutdown) unless in restore mode
-    if [[ "$1" -gt "${MON_WED_HOUR_OLD:-24}" ]] || [[ "$2" -gt "${THU_SUN_HOUR_OLD:-24}" ]]; then
-        echo "Error: Can only make schedule stricter (earlier shutdown times)" >&2
-        echo "Use --restore flag to restore original times" >&2
-        exit 1
-    fi
-fi
+	# Note the inverted comparison: a SMALLER morning end is looser.
+	if [[ "$new_morning_end" -lt "$old_morning_end" ]]; then
+		echo "Error: Can only make schedule stricter (a later morning end)" >&2
+		echo "  ${old_morning_end}:00 -> ${new_morning_end}:00 shortens the" \
+			"morning shutdown window" >&2
+		echo "Use --restore flag to restore original times" >&2
+		return 1
+	fi
+}
 
-NEW_CONFIG="# Shutdown schedule configuration
+# Resolve the guard-lib canonical copy, failing loudly when the guard is absent.
+resolve_canonical() {
+	local canonical
+	canonical="$(guardctl file-guard canonical-path "$GUARD_NAME" 2>/dev/null || true)"
+	if [[ -z "$canonical" ]]; then
+		echo "Error: guard-lib instance '$GUARD_NAME' is not installed" \
+			"(guardctl file-guard canonical-path returned empty)" >&2
+		return 1
+	fi
+	printf '%s' "$canonical"
+}
+
+# Write both copies, canonical first.
+#
+# Order matters: shutdown-schedule-guard.path triggers on CONFIG_FILE and
+# restores it from the canonical copy, so writing the watched file first races
+# the guard and loses.
+write_schedule() {
+	local canonical="$1" body="$2"
+
+	chattr -i "$CONFIG_FILE" 2>/dev/null || true
+	chattr -i "$canonical" 2>/dev/null || true
+
+	printf '%s' "$body" >"$canonical"
+	chmod 644 "$canonical"
+	chattr +i "$canonical" || echo "Warning: Could not set immutable on $canonical" >&2
+
+	printf '%s' "$body" >"$CONFIG_FILE"
+	chmod 644 "$CONFIG_FILE"
+	chattr +i "$CONFIG_FILE" || echo "Warning: Could not set immutable on $CONFIG_FILE" >&2
+}
+
+main() {
+	local restore_mode=false
+	if [[ "${1:-}" == "--restore" ]]; then
+		restore_mode=true
+		shift
+	fi
+
+	[[ $# -eq 3 ]] || usage
+	local mon_wed="$1" thu_sun="$2" morning_end="$3"
+
+	validate_hours "$mon_wed" "$thu_sun" "$morning_end" || exit 1
+
+	if [[ "$restore_mode" == false ]]; then
+		check_stricter_only "$mon_wed" "$thu_sun" "$morning_end" || exit 1
+	fi
+
+	local canonical body
+	canonical="$(resolve_canonical)" || exit 1
+	body="# Shutdown schedule configuration
 # Modified by screen_locker sick day feature at $(date)
-MON_WED_HOUR=$1
-THU_SUN_HOUR=$2
-MORNING_END_HOUR=$3
+MON_WED_HOUR=${mon_wed}
+THU_SUN_HOUR=${thu_sun}
+MORNING_END_HOUR=${morning_end}
 "
+	write_schedule "$canonical" "$body"
 
-# Remove immutable attributes
-chattr -i "$CONFIG_FILE" 2>/dev/null || true
-chattr -i "$CANONICAL_FILE" 2>/dev/null || true
+	echo "Shutdown schedule updated: Mon-Wed=${mon_wed}:00," \
+		"Thu-Sun=${thu_sun}:00, Morning end=${morning_end}:00"
+}
 
-# Write canonical copy FIRST (before the watched config) to avoid
-# a race with shutdown-schedule-guard.path which triggers on CONFIG_FILE
-# changes and restores from CANONICAL_FILE.
-echo "$NEW_CONFIG" > "$CANONICAL_FILE"
-chmod 644 "$CANONICAL_FILE"
-chattr +i "$CANONICAL_FILE" || echo "Warning: Could not set immutable on $CANONICAL_FILE" >&2
-
-# Now write the watched config — guard will see content matches canonical
-echo "$NEW_CONFIG" > "$CONFIG_FILE"
-chmod 644 "$CONFIG_FILE"
-chattr +i "$CONFIG_FILE" || echo "Warning: Could not set immutable on $CONFIG_FILE" >&2
-
-echo "Shutdown schedule updated: Mon-Wed=${1}:00, Thu-Sun=${2}:00, Morning end=${3}:00"
+# Sourced by the test harness; executed in production.
+if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
+	main "$@"
+fi

screen_locker/tests/shell/test_adjust_shutdown_schedule.sh

@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+# Tests for the shutdown-schedule ratchet.
+#
+# This ratchet had no test at all, which is how MORNING_END_HOUR went
+# unchecked for months while the two evening hours were guarded. The direction
+# cases below are the point of the file: "stricter" means SMALLER for the
+# evening hours and LARGER for the morning end.
+#
+# Nothing here touches /etc: SHUTDOWN_CONFIG_FILE points at a tmpdir, and
+# chattr/guardctl are shimmed on PATH because chattr cannot mark a file
+# immutable on tmpfs and would abort the write.
+set -uo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+readonly SCRIPT_DIR
+readonly TARGET="${SCRIPT_DIR}/../../adjust_shutdown_schedule.sh"
+
+PASS=0
+FAIL=0
+
+_t_pass() {
+	PASS=$((PASS + 1))
+	printf '  OK: %s\n' "$1"
+}
+
+_t_fail() {
+	FAIL=$((FAIL + 1))
+	printf '  FAIL: %s\n' "$1"
+}
+
+# The ratchet returns 0 to allow a write and 1 to block it. Which way it fails
+# is the entire point, so assert on the exit status explicitly.
+_t_allows() {
+	local what="$1"
+	shift
+	if "$@" >/dev/null 2>&1; then
+		_t_pass "$what"
+	else
+		_t_fail "$what (blocked when it should have allowed)"
+	fi
+}
+
+_t_blocks() {
+	local what="$1"
+	shift
+	if "$@" >/dev/null 2>&1; then
+		_t_fail "$what (allowed when it should have blocked)"
+	else
+		_t_pass "$what"
+	fi
+}
+
+_t_has() {
+	local haystack="$1" needle="$2" what="$3"
+	if [[ "$haystack" == *"$needle"* ]]; then
+		_t_pass "$what"
+	else
+		_t_fail "$what (want substring '${needle}')"
+	fi
+}
+
+TMP="$(mktemp -d)"
+trap 'rm -rf "$TMP"' EXIT
+
+# Shim the two commands that need root or a real filesystem.
+mkdir -p "$TMP/bin"
+cat >"$TMP/bin/chattr" <<'EOF'
+#!/bin/bash
+exit 0
+EOF
+cat >"$TMP/bin/guardctl" <<'EOF'
+#!/bin/bash
+printf '%s' "${FAKE_CANONICAL:-}"
+EOF
+chmod +x "$TMP/bin/chattr" "$TMP/bin/guardctl"
+export PATH="$TMP/bin:$PATH"
+
+export SHUTDOWN_CONFIG_FILE="$TMP/shutdown-schedule.conf"
+export FAKE_CANONICAL="$TMP/canonical"
+
+write_config() {
+	cat >"$SHUTDOWN_CONFIG_FILE" <<EOF
+MON_WED_HOUR=$1
+THU_SUN_HOUR=$2
+MORNING_END_HOUR=$3
+EOF
+}
+
+# shellcheck source=/dev/null
+source "$TARGET"
+
+echo "validate_hours"
+_t_allows "accepts 0 and 23" validate_hours 0 23 5
+_t_blocks "rejects 24 (the message says 0-23, and now so does the check)" \
+	validate_hours 24 23 5
+_t_blocks "rejects a non-integer" validate_hours 21 abc 5
+_t_blocks "rejects a negative (the minus makes it non-numeric)" \
+	validate_hours -1 21 5
+
+echo "check_stricter_only: evening hours (smaller is stricter)"
+write_config 23 23 5
+_t_allows "allows an earlier Mon-Wed" check_stricter_only 21 23 5
+_t_allows "allows an unchanged schedule" check_stricter_only 23 23 5
+_t_blocks "blocks a later Mon-Wed" check_stricter_only 24 23 5
+_t_blocks "blocks a later Thu-Sun" check_stricter_only 23 24 5
+
+echo "check_stricter_only: morning end (LARGER is stricter -- inverted)"
+write_config 23 23 5
+_t_allows "allows a later morning end, which lengthens the window" \
+	check_stricter_only 23 23 7
+_t_blocks "blocks an earlier morning end, which shortens the window" \
+	check_stricter_only 23 23 3
+out="$(check_stricter_only 23 23 3 2>&1 || true)"
+_t_has "$out" "shortens the" "explains WHY the morning end was rejected"
+
+echo "check_stricter_only: the source-clobber trap"
+# `source` overwrites MON_WED_HOUR et al with the OLD values. If the comparison
+# read those globals instead of its arguments it would compare a value with
+# itself and pass everything.
+write_config 21 21 5
+_t_blocks "still blocks a loosening after source has clobbered the globals" \
+	check_stricter_only 23 23 5
+
+echo "check_stricter_only: missing or unreadable config"
+rm -f "$SHUTDOWN_CONFIG_FILE"
+_t_allows "allows any write when no config exists yet" check_stricter_only 23 23 0
+printf 'garbage not a config\n' >"$SHUTDOWN_CONFIG_FILE"
+_t_allows "falls back to the loosest defaults rather than rejecting" \
+	check_stricter_only 23 23 0
+
+echo "end to end"
+write_config 23 23 5
+if (main 21 21 5 >/dev/null 2>&1); then
+	_t_pass "a stricter write succeeds"
+else
+	_t_fail "a stricter write succeeds"
+fi
+_t_has "$(cat "$SHUTDOWN_CONFIG_FILE")" "MON_WED_HOUR=21" "config was rewritten"
+_t_has "$(cat "$FAKE_CANONICAL")" "MON_WED_HOUR=21" "canonical copy was written too"
+
+write_config 21 21 5
+if (main 23 23 5 >/dev/null 2>&1); then
+	_t_fail "a looser write is refused"
+else
+	_t_pass "a looser write is refused"
+fi
+_t_has "$(cat "$SHUTDOWN_CONFIG_FILE")" "MON_WED_HOUR=21" "config was left untouched"
+
+# --restore still bypasses the ratchet on purpose: the workout reward pushes the
+# schedule later than shutdown_base.json, so bounding it here would break a
+# daily flow. Bounding it is a separate, open decision.
+write_config 21 21 5
+if (main --restore 23 23 5 >/dev/null 2>&1); then
+	_t_pass "--restore still loosens (deliberately unchanged)"
+else
+	_t_fail "--restore still loosens (deliberately unchanged)"
+fi
+
+echo
+printf 'passed: %d, failed: %d\n' "$PASS" "$FAIL"
+[[ $FAIL -eq 0 ]]