diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index f015c71..de7d6e7 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -13,6 +13,12 @@ jobs: docker: name: Docker integration harness runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # Test BOTH supported Debian bases, not just trixie. BASE_IMAGE + # flows in via --build-arg (env), never inline ${{ }} in a shell. + base: [debian:trixie-slim, debian:bookworm-slim] steps: - name: Checkout uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 @@ -21,7 +27,12 @@ jobs: fetch-depth: 1 - name: Build test image - run: docker build -f tests/docker/Dockerfile -t pi-optimiser-tests:ci . + env: + BASE_IMAGE: ${{ matrix.base }} + run: | + docker build -f tests/docker/Dockerfile \ + --build-arg "BASE_IMAGE=${BASE_IMAGE}" \ + -t pi-optimiser-tests:ci . - name: Run integration tests run: docker run --rm pi-optimiser-tests:ci diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5bb3095..0b17864 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -41,6 +41,29 @@ jobs: *) echo "unexpected ref_name: $REF_NAME" >&2; exit 1 ;; esac + - name: Verify version matches tag + # A release whose SCRIPT_VERSION or man-page footer disagrees with + # the tag ships an installer that reports the wrong version and + # makes --check-update comparisons wrong. Fail rather than publish + # the mismatch. REF_NAME flows in via env (not inline ${{ }}) so a + # crafted ref can't inject shell. + env: + REF_NAME: ${{ github.ref_name }} + run: | + expected="${REF_NAME#v}" + expected="${expected%%-*}" # strip any -rc/-beta pre-release suffix + script_ver=$(sed -n 's/^SCRIPT_VERSION="\([^"]*\)".*/\1/p' pi-optimiser.sh | head -1) + if [[ "$script_ver" != "$expected" ]]; then + echo "SCRIPT_VERSION ($script_ver) does not match tag ($expected)" >&2 + exit 1 + fi + man_ver=$(sed -n 's/^footer: pi-optimiser \(.*\)$/\1/p' share/man/pi-optimiser.8.md | head -1) + if [[ -n "$man_ver" && "$man_ver" != "$expected" ]]; then + echo "man page footer ($man_ver) does not match tag ($expected)" >&2 + exit 1 + fi + echo "Version $expected matches tag, SCRIPT_VERSION, and man footer." + - name: Build bundle env: REF_NAME: ${{ github.ref_name }} diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml index f35f3f2..f638434 100644 --- a/.github/workflows/shellcheck.yml +++ b/.github/workflows/shellcheck.yml @@ -32,6 +32,12 @@ jobs: [[ -f "$f" ]] && bash -n "$f" done bash -n install.sh + # The test harness and the Pi-binary stubs are shell too — a + # broken stub silently weakens every integration run, so syntax- + # check them here as well. + for f in tests/docker/run-tests.sh tests/docker/stubs/*; do + [[ -f "$f" ]] && bash -n "$f" + done - name: ShellCheck run: | @@ -42,7 +48,9 @@ jobs: lib/features/*.sh \ lib/ui/*.sh \ scripts/*.sh \ - install.sh + install.sh \ + tests/docker/run-tests.sh \ + tests/docker/stubs/* - name: --help smoke test run: | diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f91d52..454b232 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,103 @@ # Changelog +## 9.5.0 — 2026-06-13 + +Hardening release from a full multi-agent audit. Two critical fixes, a +batch of correctness/security fixes, several safer defaults, and a round +of de-duplication. Re-run after upgrading; affected tasks bump their +version so corrected values re-apply automatically. + +### Fixed (critical) +- **config.txt `dtparam=` lines no longer clobber each other.** + `ensure_config_key_value` keyed every `dtparam=NAME=value` line on the + bare word `dtparam`, so writing one (e.g. `--disable-leds`, + `--enable-watchdog`, `--pcie-gen3`) overwrote/deleted unrelated stock + `dtparam` lines — silently disabling onboard audio, I2C and SPI. The + `dtparam`/`dtoverlay` families are now keyed by parameter name so + distinct settings coexist. If a prior run damaged your config.txt, + `--undo ` restores the pre-change backup. +- **UFW no longer locks you out on a custom SSH port.** SSH-port + detection now reads the effective config via `sshd -T`, honouring + `/etc/ssh/sshd_config.d/*.conf` drop-ins (the canonical place to set + `Port` on Bookworm/Trixie). Previously the firewall opened 22 while + sshd listened elsewhere, dropping the operator's session on enable; + fail2ban was also jailing the wrong port. + +### Fixed (high) +- **`--force` no longer wipes completion markers** of opt-in tasks that + aren't re-requested; the pre-clear is now gated on the task actually + running. `--diff` and `--dry-run` no longer mutate `state.json` (the + proxy/ufw re-run detection moved to dry-run-safe `value_changed` hooks). +- **`ipv6_disable` is now reversible** — it records the sysctl file it + creates so `--undo` actually removes it (and IPv6 comes back). +- **`boot_config` replaces existing keys** (`gpu_mem`, `disable_overscan`, + …) instead of appending a second conflicting line. +- **`usb_uas_quirks` merges** with any existing `usb-storage.quirks=` + token instead of appending a duplicate the kernel would ignore. +- **`tailscale` installs via `ensure_packages`** so a held dpkg lock no + longer fails the task. +- **`var_log_tmpfs` journal archive** is now mode 0600 under + `/var/backups` (was world-readable in `/var/log`, leaking journal + contents on multi-user hosts). +- **Snapshot/restore consistency** — `config-optimisations.json` is now + excluded from snapshots like `state.json`, so `--restore` can't desync + the two state files. +- **TUI**: a non-default ZRAM algorithm (zstd) survives the storage menu; + value-typed tasks (proxy backend, SSH import) set via the Values menu + now actually run; default-enabled tasks in unvisited categories are no + longer dropped on Apply. + +### Changed (safer defaults — please review) +- **IP forwarding is no longer enabled by default.** The default `sysctl` + task previously turned every Pi into a router (`ip_forward=1`); single- + homed hosts no longer get it. Routers/VPN gateways should set it + explicitly. +- **Raspberry Pi Connect is no longer purged** by the default + `remove_bloat` run — it is a first-party remote-access tool, not demo + bloat. +- **`rsyslog` is no longer disabled** by `disable_services`. Combined + with volatile journald + tmpfs `/var/log` it had left no persistent + log after reboot; the `journald` task now also logs that the journal + is RAM-only and how to make it persistent. + +### Fixed (correctness) +- `dns_cache` backs up an existing `/etc/resolv.conf` (including a + symlink) before repointing it, so `--undo` restores the prior resolver. +- `eeprom_config` records the bootloader-config backup path and the + manual revert command (the file-based `--undo` can't reflash EEPROM). +- `config.yaml` `zram.algo` is validated against the CLI allowlist. +- Compute Module 4/5 (and CM3) are now mapped to their Pi-class + generation, so model-gated tasks (KMS, fan, overclock) stop skipping + on a CM. +- `--diff` no longer launches the interactive TUI on a TTY; the + menu-skip flag set is now derived from the task registry so it can't + drift out of sync with the tasks. +- `pi_rollback_release` resolves both paths before comparing, so it can't + "roll back" to the release it's already on. +- `libliftoff` no longer uncomments a deliberately-disabled + `#dtoverlay=vc4-kms-v3d`, and collapses duplicate KMS overlay lines. +- `install.sh` distinguishes a 404 ref from a network error, and no + longer prints "Verified" wording that could outlast the verified bytes. + +### Refactored +- New `apply_config_entries` / `pi_curl_secure` / `pi_add`-style + `write_systemd_unit` helpers collapse loops duplicated across the + config.txt, downloader, and systemd-unit tasks; run and `--diff` + previews now share a single entry source per task (eliminating the + stale-diff class). `oc_conservative` / `underclock` use one per-model + table. Removed dead `validate_timezone`, `is_pi500`, + `fstab_root_has_option`; gave `pi_daemon_reload_now` real callers; + implemented `pi_supports_eeprom` for an accurate `--self-test` label; + replaced inline `systemctl list-unit-files` checks with `unit_exists`. + +### CI / tests +- Release workflow fails if the tag, `SCRIPT_VERSION`, and man-page + footer disagree. ShellCheck/`bash -n` now also cover the test harness + and Pi-binary stubs. Integration matrix runs against **both** Debian + Trixie and Bookworm. The stub invocation log is now exercised by a + test (it was previously dead). Bundle "shebang parity" gate now + actually checks the shebang. + ## 9.4.5 — 2026-04-24 ### Fixed diff --git a/install.sh b/install.sh index 57b40ae..dc01ce4 100755 --- a/install.sh +++ b/install.sh @@ -114,17 +114,44 @@ trap 'rm -f "$tmp_tar"; rm -rf "$tmp_extract"' EXIT echo "Fetching pi-optimiser @ ${PI_OPTIMISER_REF}" # Try tag first (the common case for versioned refs like v9.0.1), then -# fall back to branches (for master/main). `curl -fsSL -o` emits its -# own message on failure, so we route both attempts' stderr to /dev/null -# and only surface a combined error if both fail. +# fall back to branches (for master/main). A tag 404 is expected for a +# branch ref (and vice versa), so we only fall through on an HTTP error +# (curl exit 22). A genuine network failure (DNS/connect/timeout/TLS) +# returns a different exit code — in that case there's no point retrying +# the branch URL against the same broken network, so abort immediately +# with the real curl diagnostic instead of a misleading "tried tag + +# branch" message that hides whether the ref or the network was at fault. tag_url="https://codeload.github.com/${PI_OPTIMISER_REPO}/tar.gz/refs/tags/${PI_OPTIMISER_REF}" branch_url="https://codeload.github.com/${PI_OPTIMISER_REPO}/tar.gz/refs/heads/${PI_OPTIMISER_REF}" -if ! curl "${CURL_SECURE_OPTS[@]}" "$tag_url" -o "$tmp_tar" 2>/dev/null; then - if ! curl "${CURL_SECURE_OPTS[@]}" "$branch_url" -o "$tmp_tar" 2>/dev/null; then - echo "Failed to download ${PI_OPTIMISER_REF} (tried tag + branch)" >&2 +_curl_err=$(mktemp) +# Capture curl's own exit code via `|| rc=$?` (set -e safe). NOTE: an +# `if ! curl; then rc=$?` would capture the NEGATION's exit (0), not +# curl's — so this form is required to tell HTTP 404 (22) from a network +# error apart at all. +tag_rc=0 +curl "${CURL_SECURE_OPTS[@]}" "$tag_url" -o "$tmp_tar" 2>"$_curl_err" || tag_rc=$? +if [[ $tag_rc -ne 0 ]]; then + if [[ $tag_rc -ne 22 ]]; then + echo "Failed to fetch ${PI_OPTIMISER_REF} (network/transport error, curl exit $tag_rc):" >&2 + cat "$_curl_err" >&2 + rm -f "$_curl_err" + exit 1 + fi + # HTTP error on the tag (likely the ref is a branch) — try the branch. + branch_rc=0 + curl "${CURL_SECURE_OPTS[@]}" "$branch_url" -o "$tmp_tar" 2>"$_curl_err" || branch_rc=$? + if [[ $branch_rc -ne 0 ]]; then + if [[ $branch_rc -eq 22 ]]; then + echo "Ref '${PI_OPTIMISER_REF}' not found as a tag or a branch (HTTP 404)" >&2 + else + echo "Failed to fetch ${PI_OPTIMISER_REF} from branch URL (curl exit $branch_rc):" >&2 + cat "$_curl_err" >&2 + fi + rm -f "$_curl_err" exit 1 fi fi +rm -f "$_curl_err" # If the ref is an exact version tag (vX.Y.Z), attempt to fetch and # verify the published release bundle sha256 alongside the tarball. diff --git a/lib/features/diff.sh b/lib/features/diff.sh index c958a1b..4345a31 100644 --- a/lib/features/diff.sh +++ b/lib/features/diff.sh @@ -11,15 +11,27 @@ # Globals (read): PI_CONFIG_PREVIEW_DIR, CONFIG_TXT_FILE # ====================================================================== -# Replay a list of config.txt entries through ensure_config_key_value, -# which buffers under PI_CONFIG_PREVIEW=1. "Unchanged" (rc=1) and parse -# failures (rc=2) are swallowed so a preview doesn't bail on one bad -# line. Used by pi_preview_ functions. +# Replay a list of config.txt entries through the SAME dispatch + +# section as run_ (_pi_config_apply_one), which buffers under +# PI_CONFIG_PREVIEW=1. "Unchanged" (rc=1) and parse failures (rc=2) are +# swallowed so a preview doesn't bail on one bad line. +# +# Usage: +# pi_preview_apply_entries ... # section 'all' +# pi_preview_apply_entries --section pi5 ... # explicit section +# +# The --section form lets previews of [pi5]-scoped tasks (pi5_fan, +# pcie_gen3, ...) match where run_ actually writes. pi_preview_apply_entries() { local target=${CONFIG_TXT_FILE:-/boot/firmware/config.txt} + local section=all + if [[ ${1:-} == --section ]]; then + section=${2:-all} + shift 2 + fi local entry for entry in "$@"; do - ensure_config_key_value "$entry" "$target" >/dev/null 2>&1 || true + _pi_config_apply_one "$entry" "$target" "$section" >/dev/null 2>&1 || true done } diff --git a/lib/features/install.sh b/lib/features/install.sh index 9298b6f..a5b2cda 100644 --- a/lib/features/install.sh +++ b/lib/features/install.sh @@ -153,11 +153,17 @@ pi_rollback_release() { if [[ -L "$PI_PREFIX/current" ]]; then current_target=$(readlink -f "$PI_PREFIX/current") fi - # Pick the newest release that isn't the current target. + # Pick the newest release that isn't the current target. Resolve BOTH + # sides with readlink -f before comparing: current_target is already + # fully resolved, so comparing it against the unresolved "$releases/ + # $entry" string would falsely treat the current release as a rollback + # candidate whenever any path component (e.g. PI_PREFIX) is itself a + # symlink — flipping `current` to the release it already points at. mapfile -t candidates < <(ls -1t "$releases" 2>/dev/null) - local entry + local entry entry_real for entry in "${candidates[@]}"; do - if [[ "$releases/$entry" != "$current_target" ]]; then + entry_real=$(readlink -f "$releases/$entry" 2>/dev/null || echo "$releases/$entry") + if [[ "$entry_real" != "$current_target" ]]; then previous="$releases/$entry" break fi diff --git a/lib/features/profiles.sh b/lib/features/profiles.sh index 6424e35..cb371f0 100644 --- a/lib/features/profiles.sh +++ b/lib/features/profiles.sh @@ -131,7 +131,7 @@ pi_self_test() { # EEPROM tasks on non-Pi4/5 case "$tid" in eeprom_config|eeprom_refresh) - pi_supports_kms_overlays \ + pi_supports_eeprom \ || { ok="SKIP (no EEPROM support)"; any_fail=1; } ;; esac diff --git a/lib/features/snapshot.sh b/lib/features/snapshot.sh index 3a127ca..9fe394b 100644 --- a/lib/features/snapshot.sh +++ b/lib/features/snapshot.sh @@ -85,6 +85,7 @@ pi_take_snapshot() { --exclude="$MARKER_DIR/state.json" \ --exclude="$MARKER_DIR/state.schema" \ --exclude="$MARKER_DIR/state" \ + --exclude="$MARKER_DIR/config-optimisations.json" \ "${existing[@]}" 2>/dev/null; then chmod 600 "$archive" log_info "Snapshot written: $archive ($(stat -c%s "$archive") bytes)" diff --git a/lib/tasks/boot_config.sh b/lib/tasks/boot_config.sh index ca2a446..b65a288 100644 --- a/lib/tasks/boot_config.sh +++ b/lib/tasks/boot_config.sh @@ -61,33 +61,22 @@ run_boot_config() { backup_file "$CONFIG_TXT_FILE" local -a entries=() mapfile -t entries < <(_boot_config_entries) - local applied=0 - local entry rc safe_key - for entry in "${entries[@]}"; do - if ensure_config_line "$entry"; then - log_info "Applied $entry to config.txt" - safe_key=${entry//=/_} - write_json_field "$CONFIG_OPTIMISER_STATE" "boot_config.${safe_key}" "$entry" - applied=1 - else - rc=$? - if [[ $rc -gt 1 ]]; then - log_warn "Failed to ensure $entry in config.txt" - fi - fi - done - if [[ $applied -eq 1 ]]; then - log_info "Boot config tuned for Raspberry Pi desktop display" - else - log_info "Boot config already matched recommended defaults" - fi + # apply_config_entries upserts key=value entries (gpu_mem, disable_overscan, + # dtparam=audio=on, framebuffer_*) by key so an existing raspi-config value + # is REPLACED rather than leaving a second conflicting line, while the bare + # `dtoverlay=vc4-kms-v3d` keeps additive whole-line semantics. + local rc=0 + apply_config_entries "boot_config" all "${entries[@]}" || rc=$? + case $rc in + 0) log_info "Boot config tuned for Raspberry Pi desktop display" ;; + 1) log_info "Boot config already matched recommended defaults" ;; + *) log_warn "One or more boot config entries failed to apply" ;; + esac } pi_preview_boot_config() { pi_supports_kms_overlays || return 0 - local target=${CONFIG_TXT_FILE:-/boot/firmware/config.txt} - local entry - while IFS= read -r entry; do - ensure_config_line "$entry" "$target" >/dev/null 2>&1 || true - done < <(_boot_config_entries) + local -a entries=() + mapfile -t entries < <(_boot_config_entries) + pi_preview_apply_entries "${entries[@]}" } diff --git a/lib/tasks/cpu_governor.sh b/lib/tasks/cpu_governor.sh index 6e97d08..2f36825 100644 --- a/lib/tasks/cpu_governor.sh +++ b/lib/tasks/cpu_governor.sh @@ -21,10 +21,10 @@ run_cpu_governor() { fi mkdir -p "$(dirname "$CPU_GOVERNOR_SERVICE")" - # record_created falls back to backup_file when the path already - # exists, so --undo still restores a pre-existing operator override. - record_created "$CPU_GOVERNOR_SERVICE" - cat <<'CFG' > "$CPU_GOVERNOR_SERVICE" + # write_systemd_unit: record_created (undo) + atomic write + 0644 + + # flag a daemon-reload. pi_daemon_reload_now below forces the reload + # immediately so the enable picks up the freshly-written unit. + write_systemd_unit "$CPU_GOVERNOR_SERVICE" <<'CFG' [Unit] Description=Pin CPU scaling governor to performance (pi-optimiser) After=multi-user.target @@ -38,9 +38,8 @@ ExecStart=/bin/sh -c 'for g in /sys/devices/system/cpu/cpu*/cpufreq/scaling_gove [Install] WantedBy=multi-user.target CFG - chmod 644 "$CPU_GOVERNOR_SERVICE" - systemctl daemon-reload >/dev/null 2>&1 || true + pi_daemon_reload_now # Use enable (not --now) here: the oneshot would try to write to # sysfs paths that may not be writable until late in boot, and the # manual sysfs loop below already applies the setting for the diff --git a/lib/tasks/disable_leds.sh b/lib/tasks/disable_leds.sh index 08fc4c7..4fee926 100644 --- a/lib/tasks/disable_leds.sh +++ b/lib/tasks/disable_leds.sh @@ -27,6 +27,19 @@ pi_task_register disable_leds \ # Saves ~5-15 mA total and stops the strobe in a server rack. Applies # to Pi 4/400/5/500; older models silently ignore the unknown # dtparams. +# Single source of truth for the LED dtparams, consumed by both +# run_disable_leds and pi_preview_disable_leds so --diff can never +# drift from what is actually written. +_disable_leds_entries() { + printf '%s\n' \ + "dtparam=act_led_trigger=none" \ + "dtparam=act_led_activelow=off" \ + "dtparam=pwr_led_trigger=none" \ + "dtparam=pwr_led_activelow=off" \ + "dtparam=eth_led0=4" \ + "dtparam=eth_led1=4" +} + run_disable_leds() { if [[ ${DISABLE_LEDS:-0} -eq 0 ]]; then log_info "LED disable not requested; skipping" @@ -39,37 +52,23 @@ run_disable_leds() { return 2 fi backup_file "$CONFIG_TXT_FILE" - local -a entries=( - "dtparam=act_led_trigger=none" - "dtparam=act_led_activelow=off" - "dtparam=pwr_led_trigger=none" - "dtparam=pwr_led_activelow=off" - "dtparam=eth_led0=4" - "dtparam=eth_led1=4" - ) - local entry rc applied=0 - for entry in "${entries[@]}"; do - rc=0 - ensure_config_key_value "$entry" "$CONFIG_TXT_FILE" || rc=$? - if [[ $rc -eq 0 ]]; then - log_info "Applied $entry" - applied=1 - elif [[ $rc -gt 1 ]]; then - log_warn "Failed to apply $entry" - fi - done - if [[ $applied -eq 1 ]]; then - log_info "Status LEDs disabled (effective after reboot)" - write_json_field "$CONFIG_OPTIMISER_STATE" "display.leds_disabled" "true" - else - log_info "LED disables already present" - fi + local -a entries=() + mapfile -t entries < <(_disable_leds_entries) + local rc=0 + apply_config_entries "" all "${entries[@]}" || rc=$? + case $rc in + 0) + log_info "Status LEDs disabled (effective after reboot)" + write_json_field "$CONFIG_OPTIMISER_STATE" "display.leds_disabled" "true" + ;; + 1) log_info "LED disables already present" ;; + *) log_warn "One or more LED disables failed to apply" ;; + esac } pi_preview_disable_leds() { [[ ${DISABLE_LEDS:-0} -eq 0 ]] && return 0 - pi_preview_apply_entries \ - "dtparam=act_led_trigger=none" "dtparam=act_led_activelow=off" \ - "dtparam=pwr_led_trigger=none" "dtparam=pwr_led_activelow=off" \ - "dtparam=eth_led0=4" "dtparam=eth_led1=4" + local -a entries=() + mapfile -t entries < <(_disable_leds_entries) + pi_preview_apply_entries "${entries[@]}" } diff --git a/lib/tasks/disable_services.sh b/lib/tasks/disable_services.sh index 867010e..60f5bdd 100644 --- a/lib/tasks/disable_services.sh +++ b/lib/tasks/disable_services.sh @@ -1,6 +1,6 @@ # >>> pi-task # id: disable_services -# version: 1.2.0 +# version: 1.3.0 # description: Stop non-essential background services (ModemManager, Avahi, openipmi) # category: system # default_enabled: 1 @@ -10,17 +10,24 @@ pi_task_register disable_services \ description="Stop non-essential background services (ModemManager, Avahi, openipmi)" \ category=system \ - version=1.2.0 \ + version=1.3.0 \ default_enabled=1 run_disable_services() { + # NOTE: rsyslog is intentionally NOT disabled here. The journald task + # sets Storage=volatile (journal in RAM) and var_log_tmpfs moves + # /var/log to RAM — both default-enabled. With rsyslog also off, a Pi + # would have NO persistent system log at all: after a crash + reboot + # there is nothing to diagnose from. Leaving rsyslog running preserves + # a persistent /var/log/syslog for operators who keep /var/log on disk + # (it is lightweight, and absent on Lite images, where unit_exists + # simply skips it). local -a units=( triggerhappy.service bluetooth.service hciuart.service avahi-daemon.service cups.service - rsyslog.service ) local unit for unit in "${units[@]}"; do diff --git a/lib/tasks/dns_cache.sh b/lib/tasks/dns_cache.sh index 1873812..569cf4b 100644 --- a/lib/tasks/dns_cache.sh +++ b/lib/tasks/dns_cache.sh @@ -45,21 +45,25 @@ CFG # (re)starts. Without daemon-reload the running resolved keeps its # old in-memory config and the cache options only kick in on next # boot or manual reload. - systemctl daemon-reload >/dev/null 2>&1 || true + pi_daemon_reload_now systemctl enable --now systemd-resolved >/dev/null 2>&1 || log_warn "Unable to enable systemd-resolved" - # Point /etc/resolv.conf at the stub so clients use the cache. - if [[ -L /etc/resolv.conf ]]; then - local current - current=$(readlink -f /etc/resolv.conf) - if [[ $current != /run/systemd/resolve/stub-resolv.conf ]]; then - ln -sf /run/systemd/resolve/stub-resolv.conf /etc/resolv.conf - log_info "Pointed /etc/resolv.conf at systemd-resolved stub" - fi - else - backup_file /etc/resolv.conf + # Point /etc/resolv.conf at the stub so clients use the cache. Back up + # the existing resolver FIRST — whether it is a regular file or an + # existing symlink (the common Bookworm/Trixie case, e.g. pointing at a + # NetworkManager-managed file). record_created journals the symlink via + # backup_file (cp -a preserves the link target) when it exists, or + # registers a "created" entry when absent, so `--undo dns_cache` + # restores the operator's previous resolver instead of stranding the + # stub symlink. + local current="" + if [[ -e /etc/resolv.conf || -L /etc/resolv.conf ]]; then + current=$(readlink -f /etc/resolv.conf 2>/dev/null || echo "") + fi + if [[ $current != /run/systemd/resolve/stub-resolv.conf ]]; then + record_created /etc/resolv.conf ln -sf /run/systemd/resolve/stub-resolv.conf /etc/resolv.conf - log_info "Replaced /etc/resolv.conf with systemd-resolved stub symlink" + log_info "Pointed /etc/resolv.conf at systemd-resolved stub" fi systemctl restart systemd-resolved >/dev/null 2>&1 || true write_json_field "$CONFIG_OPTIMISER_STATE" "network.dns_cache" "systemd-resolved" diff --git a/lib/tasks/docker.sh b/lib/tasks/docker.sh index 8c6aa60..5e3ce98 100644 --- a/lib/tasks/docker.sh +++ b/lib/tasks/docker.sh @@ -45,23 +45,13 @@ run_docker() { ensure_packages ca-certificates curl gnupg install -m 0755 -d /etc/apt/keyrings - # Secure curl defaults for third-party downloads: pin to HTTPS with - # bounded redirects, explicit timeouts so hung mirrors don't block the - # installer, and retry-with-backoff on transient failures. - local -a _curl_secure=( - --fail --silent --show-error --location - --proto '=https' --proto-redir '=https' - --max-redirs 5 - --connect-timeout 15 --max-time 120 - --tlsv1.2 - --retry 3 --retry-delay 2 --retry-connrefused - ) + # Download security policy lives in pi_curl_secure (lib/util/apt.sh). if [[ ! -f "$DOCKER_KEY_FILE" ]]; then # Register the keyfile as "created" BEFORE the download so --undo # cleans it up even if a partial run dies between download and # repo-list write. record_created "$DOCKER_KEY_FILE" - if curl "${_curl_secure[@]}" "https://download.docker.com/linux/${repo_id}/gpg" | gpg --dearmor > "$DOCKER_KEY_FILE"; then + if pi_curl_secure "https://download.docker.com/linux/${repo_id}/gpg" | gpg --dearmor > "$DOCKER_KEY_FILE"; then chmod a+r "$DOCKER_KEY_FILE" repo_configured=1 else @@ -74,9 +64,7 @@ run_docker() { if [[ $repo_configured -eq 1 ]]; then local docker_suite=$OS_CODENAME - if ! curl --fail --silent --show-error --location \ - --proto '=https' --proto-redir '=https' --max-redirs 5 \ - --connect-timeout 10 --max-time 30 --tlsv1.2 \ + if ! pi_curl_secure --connect-timeout 10 --max-time 30 \ -I "https://download.docker.com/linux/${repo_id}/dists/${docker_suite}/Release" >/dev/null 2>&1; then if [[ $docker_suite != "bookworm" ]]; then log_warn "Docker repo for $docker_suite unavailable; falling back to bookworm" diff --git a/lib/tasks/eeprom_config.sh b/lib/tasks/eeprom_config.sh index b25377c..838a1fe 100644 --- a/lib/tasks/eeprom_config.sh +++ b/lib/tasks/eeprom_config.sh @@ -96,6 +96,12 @@ PY if rpi-eeprom-config --apply "$new_conf" >/dev/null 2>&1; then log_info "Applied SDRAM_BANKLOW=$bank_value to EEPROM (profile: $profile); active after reboot" + # EEPROM state lives in the bootloader, not a file, so the file-based + # --undo journal cannot revert it. Record where the pre-change config + # was saved and how to roll it back manually, so --report/--status + # surface a recovery path. + write_json_field "$CONFIG_OPTIMISER_STATE" "eeprom.config_backup" "$backup_path" + log_info "To revert: sudo rpi-eeprom-config --apply $backup_path && sudo reboot" write_json_field "$CONFIG_OPTIMISER_STATE" "eeprom.sdram_banklow" "$bank_value" write_json_field "$CONFIG_OPTIMISER_STATE" "eeprom.profile" "$profile" return 0 diff --git a/lib/tasks/fstrim.sh b/lib/tasks/fstrim.sh index fa62f40..9a00782 100644 --- a/lib/tasks/fstrim.sh +++ b/lib/tasks/fstrim.sh @@ -14,7 +14,7 @@ pi_task_register fstrim \ default_enabled=1 run_fstrim() { - if ! systemctl list-unit-files fstrim.timer >/dev/null 2>&1; then + if ! unit_exists fstrim.timer; then log_info "fstrim.timer unit not available; skipping" pi_skip_reason "fstrim.timer missing" return 2 diff --git a/lib/tasks/ipv6_disable.sh b/lib/tasks/ipv6_disable.sh index 3f8595c..c52aa6d 100644 --- a/lib/tasks/ipv6_disable.sh +++ b/lib/tasks/ipv6_disable.sh @@ -30,7 +30,12 @@ run_ipv6_disable() { return 2 fi local conf=/etc/sysctl.d/98-pi-optimiser-ipv6.conf - backup_file "$conf" + # record_created registers a "created" journal entry so `--undo + # ipv6_disable` deletes the file (and IPv6 comes back). backup_file + # alone records nothing for a fresh file, so undo would leave it in + # place and IPv6 would stay disabled. record_created falls back to + # backup_file if the file already exists. + record_created "$conf" # Atomic write — a truncated sysctl.d file is rejected at boot and # the values silently fail to apply. _pi_atomic_write "$conf" <<'CFG' diff --git a/lib/tasks/journald.sh b/lib/tasks/journald.sh index 4d51b2e..591a76e 100644 --- a/lib/tasks/journald.sh +++ b/lib/tasks/journald.sh @@ -1,6 +1,6 @@ # >>> pi-task # id: journald -# version: 1.1.1 +# version: 1.2.0 # description: Keep systemd journal in RAM to reduce disk writes # category: storage # default_enabled: 1 @@ -10,7 +10,7 @@ pi_task_register journald \ description="Keep systemd journal in RAM to reduce disk writes" \ category=storage \ - version=1.1.1 \ + version=1.2.0 \ default_enabled=1 run_journald() { @@ -24,4 +24,8 @@ SystemMaxUse=50M MaxRetentionSec=1week CFG systemctl restart systemd-journald >/dev/null 2>&1 || log_warn "systemd-journald restart failed" + # Storage=volatile keeps the journal in RAM to spare the SD card, but + # that means logs do NOT survive a reboot. Make the tradeoff explicit + # so an operator debugging a crash isn't surprised by an empty journal. + log_info "Journal is volatile (RAM-only): logs are cleared on reboot. To keep persistent logs, skip the journald task or set Storage=persistent in $JOURNALD_CONF_FILE." } diff --git a/lib/tasks/libliftoff.sh b/lib/tasks/libliftoff.sh index 9a810ce..c143e26 100644 --- a/lib/tasks/libliftoff.sh +++ b/lib/tasks/libliftoff.sh @@ -1,6 +1,6 @@ # >>> pi-task # id: libliftoff -# version: 1.1.0 +# version: 1.2.0 # description: Disable KMS 'liftoff' to avoid compositor display glitches # category: display # default_enabled: 1 @@ -11,7 +11,7 @@ pi_task_register libliftoff \ description="Disable KMS 'liftoff' to avoid compositor display glitches" \ category=display \ - version=1.1.0 \ + version=1.2.0 \ default_enabled=1 \ power_sensitive=1 \ reboot_required=1 @@ -40,45 +40,70 @@ from pathlib import Path cfg_path = Path(os.environ['CONFIG_FILE']) lines = cfg_path.read_text().splitlines() changed = False -new_lines = [] -for line in lines: - stripped = line.lstrip() - prefix = line[:len(line) - len(stripped)] - commented = False + +def is_kms_overlay(stripped_lower): + # Only ACTIVE dtoverlay lines for the KMS driver (or any line that + # explicitly mentions liftoff). Commented lines are deliberately + # left alone — a `#dtoverlay=vc4-kms-v3d` is the operator opting OUT, + # and uncommenting it would activate KMS against their wishes. + return stripped_lower.startswith('dtoverlay') and ( + 'vc4-kms-v3d' in stripped_lower or 'liftoff' in stripped_lower) + +# Collect the merged option set across EVERY active KMS overlay line and +# remember the first such position. boot_config (which runs earlier and +# writes a bare `dtoverlay=vc4-kms-v3d`) can leave a duplicate alongside +# the one we previously rewrote to `,no-liftoff`; collapsing them here to +# a single canonical line prevents the duplicate accumulating on re-runs. +first_idx = None +merged = [] +seen = set() +for i, line in enumerate(lines): + stripped = line.strip() if stripped.startswith('#'): - after_hash = stripped[1:].lstrip() - if after_hash.lower().startswith('dtoverlay'): - stripped = after_hash - commented = True - lower = stripped.lower() - if lower.startswith('dtoverlay') and ('liftoff' in lower or 'vc4-kms-v3d' in lower): - try: - key, value = stripped.split('=', 1) - except ValueError: - new_lines.append(prefix + stripped) + continue + if not is_kms_overlay(stripped.lower()) or '=' not in stripped: + continue + if first_idx is None: + first_idx = i + for opt in stripped.split('=', 1)[1].split(','): + opt = opt.strip() + if opt and opt.lower() not in seen: + seen.add(opt.lower()) + merged.append(opt) + +out = [] +if first_idx is not None: + # Drop any liftoff-enable token; guarantee exactly one no-liftoff. + cleaned = [] + has_no_liftoff = False + for opt in merged: + ol = opt.lower() + if ol in {'liftoff', 'liftoff=1', 'liftoff=on'}: continue - opts = [opt for opt in value.split(',') if opt] - cleaned_opts = [] - liftoff_disabled = False - for opt in opts: - opt_lower = opt.strip().lower() - if opt_lower in {'liftoff', 'liftoff=1', 'liftoff=on'}: + if ol in {'no-liftoff', 'liftoff=0', 'liftoff=off', 'disable_liftoff=1'}: + has_no_liftoff = True + cleaned.append(opt) + if not has_no_liftoff: + cleaned.append('no-liftoff') + canonical = 'dtoverlay=' + ','.join(cleaned) + for i, line in enumerate(lines): + stripped = line.strip() + if not stripped.startswith('#') and is_kms_overlay(stripped.lower()) and '=' in stripped: + if i == first_idx: + prefix = line[:len(line) - len(line.lstrip())] + if (prefix + canonical) != line: + changed = True + out.append(prefix + canonical) + else: + # duplicate active KMS overlay line — drop it changed = True - continue - if opt_lower in {'no-liftoff', 'liftoff=0', 'liftoff=off', 'disable_liftoff=1'}: - liftoff_disabled = True - cleaned_opts.append(opt) - if not liftoff_disabled: - cleaned_opts.append('no-liftoff') - liftoff_disabled = True - new_value = ','.join(cleaned_opts) - new_line = f"{key}={new_value}" - if commented or new_line != stripped: - changed = True - stripped = new_line - new_lines.append(prefix + stripped) + continue + out.append(line) +else: + out = lines + if changed: - cfg_path.write_text('\n'.join(new_lines) + '\n') + cfg_path.write_text('\n'.join(out) + '\n') print('changed' if changed else 'unchanged') PY ) diff --git a/lib/tasks/limits.sh b/lib/tasks/limits.sh index 4884619..379e54d 100644 --- a/lib/tasks/limits.sh +++ b/lib/tasks/limits.sh @@ -50,7 +50,7 @@ DefaultLimitNPROC=32768 CFG log_info "Configured $SYSTEMD_USER_LIMITS" - systemctl daemon-reload >/dev/null 2>&1 || true + pi_daemon_reload_now if unit_exists systemd-logind.service; then systemctl restart systemd-logind >/dev/null 2>&1 || log_warn "systemd-logind restart encountered an issue" fi diff --git a/lib/tasks/oc_conservative.sh b/lib/tasks/oc_conservative.sh index 12e315b..5984530 100644 --- a/lib/tasks/oc_conservative.sh +++ b/lib/tasks/oc_conservative.sh @@ -20,6 +20,27 @@ pi_task_register oc_conservative \ gate_var=REQUEST_OC_CONSERVATIVE \ reboot_required=1 +# Single source of truth for the per-model overclock plan, consumed by +# both run_oc_conservative and pi_preview_oc_conservative so --diff can +# never drift from what is written (and to the correct per-model section). +# Prints:
\n\n... (no output = unsupported model) +# Each profile is routed into its own config.txt section so booting the +# SD card on a different Pi model can't apply a value the silicon can't +# sustain. +_oc_conservative_plan() { + if is_pi5; then + printf '%s\n' pi5 pi5_2800mhz over_voltage_delta=30000 arm_freq=2800 gpu_freq=950 + elif is_pi400; then + printf '%s\n' pi400 pi400_conservative arm_freq=2000 gpu_freq=600 + elif is_pi4; then + printf '%s\n' pi4 pi4_conservative arm_freq=1750 gpu_freq=600 + elif is_pi3; then + printf '%s\n' pi3 pi3_conservative arm_freq=1400 gpu_freq=500 + elif is_pizero2; then + printf '%s\n' pi02 pi_zero2_conservative arm_freq=1200 gpu_freq=500 + fi +} + run_oc_conservative() { if [[ $REQUEST_OC_CONSERVATIVE -eq 0 ]]; then log_info "Conservative overclock not requested; skipping" @@ -47,91 +68,35 @@ run_oc_conservative() { return 2 fi - local -a entries=() - local profile="" - # Overclock values are model-specific — route each profile into its - # own section so booting a different SD card on another Pi model - # won't apply a value the silicon can't sustain. - local section="all" - if is_pi5; then - entries=( - "over_voltage_delta=30000" - "arm_freq=2800" - "gpu_freq=950" - ) - profile="pi5_2800mhz" - section="pi5" - elif is_pi400; then - entries=( - "arm_freq=2000" - "gpu_freq=600" - ) - profile="pi400_conservative" - section="pi400" - elif is_pi4; then - entries=( - "arm_freq=1750" - "gpu_freq=600" - ) - profile="pi4_conservative" - section="pi4" - elif is_pi3; then - entries=( - "arm_freq=1400" - "gpu_freq=500" - ) - profile="pi3_conservative" - section="pi3" - elif is_pizero2; then - entries=( - "arm_freq=1200" - "gpu_freq=500" - ) - profile="pi_zero2_conservative" - section="pi02" - else + local -a plan=() + mapfile -t plan < <(_oc_conservative_plan) + if (( ${#plan[@]} == 0 )); then log_info "Conservative overclock not supported on model ${SYSTEM_MODEL:-unknown}" pi_skip_reason "model unsupported" return 2 fi + local section=${plan[0]} profile=${plan[1]} + local -a entries=("${plan[@]:2}") backup_file "$CONFIG_TXT_FILE" - local applied=0 entry safe_key rc - for entry in "${entries[@]}"; do - rc=0 - ensure_config_key_value "$entry" "$CONFIG_TXT_FILE" "$section" || rc=$? - if [[ $rc -eq 0 ]]; then - log_info "Applied $entry to config.txt" - safe_key=${entry//=/_} - write_json_field "$CONFIG_OPTIMISER_STATE" "overclock.${safe_key}" "$entry" - applied=1 - elif [[ $rc -gt 1 ]]; then - log_warn "Failed to apply $entry to config.txt" - fi - done - if [[ $applied -eq 1 ]]; then - write_json_field "$CONFIG_OPTIMISER_STATE" "overclock.profile" "$profile" - log_info "Conservative overclock profile applied: $profile" - else - log_info "Overclock profile already present" - fi + # apply_config_entries records each entry under overclock. + # (prefix "overclock"), matching the prior per-entry state shape. + local rc=0 + apply_config_entries "overclock" "$section" "${entries[@]}" || rc=$? + case $rc in + 0) + write_json_field "$CONFIG_OPTIMISER_STATE" "overclock.profile" "$profile" + log_info "Conservative overclock profile applied: $profile" + ;; + 1) log_info "Overclock profile already present" ;; + *) log_warn "One or more overclock entries failed to apply" ;; + esac } pi_preview_oc_conservative() { [[ ${REQUEST_OC_CONSERVATIVE:-0} -eq 0 ]] && return 0 - local -a entries=() - if is_pi5; then - entries=("over_voltage_delta=30000" "arm_freq=2800" "gpu_freq=950") - elif is_pi400; then - entries=("arm_freq=2000" "gpu_freq=600") - elif is_pi4; then - entries=("arm_freq=1750" "gpu_freq=600") - elif is_pi3; then - entries=("arm_freq=1400" "gpu_freq=500") - elif is_pizero2; then - entries=("arm_freq=1200" "gpu_freq=500") - else - return 0 - fi - pi_preview_apply_entries "${entries[@]}" + local -a plan=() + mapfile -t plan < <(_oc_conservative_plan) + (( ${#plan[@]} == 0 )) && return 0 + pi_preview_apply_entries --section "${plan[0]}" "${plan[@]:2}" } diff --git a/lib/tasks/pcie_gen3.sh b/lib/tasks/pcie_gen3.sh index af6d3f2..ee52f18 100644 --- a/lib/tasks/pcie_gen3.sh +++ b/lib/tasks/pcie_gen3.sh @@ -54,5 +54,6 @@ run_pcie_gen3() { pi_preview_pcie_gen3() { [[ ${INSTALL_PCIE_GEN3:-0} -eq 0 ]] && return 0 is_pi5 || return 0 - pi_preview_apply_entries "dtparam=pciex1_gen=3" + # Preview into [pi5] to match where run_pcie_gen3 writes. + pi_preview_apply_entries --section pi5 "dtparam=pciex1_gen=3" } diff --git a/lib/tasks/pi5_fan.sh b/lib/tasks/pi5_fan.sh index 37a20e2..82b99d6 100644 --- a/lib/tasks/pi5_fan.sh +++ b/lib/tasks/pi5_fan.sh @@ -20,6 +20,25 @@ pi_task_register pi5_fan \ gate_var=INSTALL_PI5_FAN_PROFILE \ reboot_required=1 +# Single source of truth for the PWM fan-curve dtparams, consumed by +# both run_pi5_fan and pi_preview_pi5_fan so --diff matches what is +# written (and to the same [pi5] section). +_pi5_fan_entries() { + printf '%s\n' \ + "dtparam=fan_temp0=50000" \ + "dtparam=fan_temp0_hyst=5000" \ + "dtparam=fan_temp0_speed=75" \ + "dtparam=fan_temp1=60000" \ + "dtparam=fan_temp1_hyst=5000" \ + "dtparam=fan_temp1_speed=125" \ + "dtparam=fan_temp2=67000" \ + "dtparam=fan_temp2_hyst=5000" \ + "dtparam=fan_temp2_speed=200" \ + "dtparam=fan_temp3=75000" \ + "dtparam=fan_temp3_hyst=5000" \ + "dtparam=fan_temp3_speed=255" +} + run_pi5_fan() { if [[ $INSTALL_PI5_FAN_PROFILE -eq 0 ]]; then log_info "Pi 5 fan profile not requested; skipping" @@ -37,46 +56,24 @@ run_pi5_fan() { return 2 fi backup_file "$CONFIG_TXT_FILE" - local -a entries=( - "dtparam=fan_temp0=50000" - "dtparam=fan_temp0_hyst=5000" - "dtparam=fan_temp0_speed=75" - "dtparam=fan_temp1=60000" - "dtparam=fan_temp1_hyst=5000" - "dtparam=fan_temp1_speed=125" - "dtparam=fan_temp2=67000" - "dtparam=fan_temp2_hyst=5000" - "dtparam=fan_temp2_speed=200" - "dtparam=fan_temp3=75000" - "dtparam=fan_temp3_hyst=5000" - "dtparam=fan_temp3_speed=255" - ) - local entry rc applied=0 + local -a entries=() + mapfile -t entries < <(_pi5_fan_entries) # Pi 5 fan dtparams live under [pi5] so a user file ending in # [none] or [pi4] doesn't silently swallow the fan curve. - for entry in "${entries[@]}"; do - rc=0 - ensure_config_key_value "$entry" "$CONFIG_TXT_FILE" pi5 || rc=$? - if [[ $rc -eq 0 ]]; then - applied=1 - elif [[ $rc -gt 1 ]]; then - log_warn "Failed to apply $entry" - fi - done - if [[ $applied -eq 1 ]]; then - log_info "Applied Pi 5 PWM fan curve (50/60/67/75 C)" - else - log_info "Pi 5 fan curve already present" - fi + local rc=0 + apply_config_entries "" pi5 "${entries[@]}" || rc=$? + case $rc in + 0) log_info "Applied Pi 5 PWM fan curve (50/60/67/75 C)" ;; + 1) log_info "Pi 5 fan curve already present" ;; + *) log_warn "One or more fan dtparams failed to apply" ;; + esac write_json_field "$CONFIG_OPTIMISER_STATE" "fan.profile" "pi5_50_60_67_75" } pi_preview_pi5_fan() { [[ ${INSTALL_PI5_FAN_PROFILE:-0} -eq 0 ]] && return 0 is_pi5 || return 0 - pi_preview_apply_entries \ - "dtparam=fan_temp0=50000" "dtparam=fan_temp0_hyst=5000" "dtparam=fan_temp0_speed=75" \ - "dtparam=fan_temp1=60000" "dtparam=fan_temp1_hyst=5000" "dtparam=fan_temp1_speed=125" \ - "dtparam=fan_temp2=67000" "dtparam=fan_temp2_hyst=5000" "dtparam=fan_temp2_speed=200" \ - "dtparam=fan_temp3=75000" "dtparam=fan_temp3_hyst=5000" "dtparam=fan_temp3_speed=255" + local -a entries=() + mapfile -t entries < <(_pi5_fan_entries) + pi_preview_apply_entries --section pi5 "${entries[@]}" } diff --git a/lib/tasks/proxy.sh b/lib/tasks/proxy.sh index b3320fd..991ab9b 100644 --- a/lib/tasks/proxy.sh +++ b/lib/tasks/proxy.sh @@ -41,6 +41,23 @@ _proxy_redact_url() { printf '%s' "$url" } +# Re-run the proxy task when the requested backend differs from what was +# last applied (recorded as proxy.backend in state). apply_once consults +# this hook to bypass the already-completed short-circuit and re-run the +# task WITHOUT mutating state.json, so it is safe under --dry-run/--diff — +# unlike the old hard-coded clear_task_state block in main(). Returns 0 +# (changed) to force a re-run, 1 otherwise. +pi_proxy_value_changed() { + [[ -n "${PROXY_BACKEND:-}" ]] || return 1 + local stored + if stored=$(get_stored_proxy_backend); then + [[ "$stored" != "$PROXY_BACKEND" ]] + else + # A backend is requested but none is on record — treat as changed. + return 0 + fi +} + run_proxy() { if [[ -z "$PROXY_BACKEND" ]]; then log_info "Proxy support not requested; skipping proxy configuration" @@ -58,7 +75,7 @@ run_proxy() { backup_file "$conf" rm -f "$conf" fi - if systemctl list-unit-files nginx.service >/dev/null 2>&1; then + if unit_exists nginx.service; then systemctl stop nginx >/dev/null 2>&1 || true systemctl disable nginx >/dev/null 2>&1 || log_warn "Unable to disable nginx service" fi diff --git a/lib/tasks/remove_bloat.sh b/lib/tasks/remove_bloat.sh index 31b0eb8..0bfe102 100644 --- a/lib/tasks/remove_bloat.sh +++ b/lib/tasks/remove_bloat.sh @@ -1,6 +1,6 @@ # >>> pi-task # id: remove_bloat -# version: 1.2.0 +# version: 1.3.0 # description: Remove preinstalled demo and educational packages # category: packages # default_enabled: 1 @@ -10,9 +10,15 @@ pi_task_register remove_bloat \ description="Remove preinstalled demo and educational packages" \ category=packages \ - version=1.2.0 \ + version=1.3.0 \ default_enabled=1 +# NOTE: Raspberry Pi Connect (raspberrypi-connect / rpi-connect*) is +# deliberately NOT purged here. It is a first-party remote-access tool — +# pi-optimiser even has an opt-in install task for it — and silently +# removing it from a default run can cut off a headless operator's way +# back in. It is not demo/educational bloat. + # Decide whether the current run counts as "non-desktop" — kiosk / # server / headless-iot all qualify, and so does an explicit # --remove-cups or KEEP_SCREEN_BLANKING=1 flag. Plain runs (no @@ -39,9 +45,6 @@ run_remove_bloat() { 'nodered' 'nuscratch' 'python-games' - 'raspberrypi-connect' - 'rpi-connect' - 'rpi-connect-server' 'scratch' 'scratch2' 'scratch3' diff --git a/lib/tasks/secure_ssh.sh b/lib/tasks/secure_ssh.sh index 4e40758..e60ce89 100644 --- a/lib/tasks/secure_ssh.sh +++ b/lib/tasks/secure_ssh.sh @@ -17,14 +17,11 @@ pi_task_register secure_ssh \ flags="--secure-ssh" \ gate_var=SECURE_SSH -# Detect the SSH listen port from sshd_config; fall back to 22. Kept -# identical to ufw_firewall's helper so both tasks agree on the port. +# Detect the EFFECTIVE SSH listen port (honours sshd_config.d drop-ins); +# fall back to 22. Delegates to the shared resolver in lib/util/sshd.sh +# so secure_ssh and ufw_firewall can never disagree on the port. _secure_ssh_port() { - local port="" - if [[ -r /etc/ssh/sshd_config ]]; then - port=$(awk '/^[[:space:]]*Port[[:space:]]+[0-9]+/{print $2; exit}' /etc/ssh/sshd_config 2>/dev/null) - fi - echo "${port:-22}" + pi_sshd_effective_port } run_secure_ssh() { @@ -176,11 +173,11 @@ DROPIN # Reload (not restart) so the live control socket survives and any # active SSH session is preserved. Fall back to restart only if reload # is unsupported on this init. - if systemctl list-unit-files ssh.service >/dev/null 2>&1; then + if unit_exists ssh.service; then systemctl reload ssh >/dev/null 2>&1 \ || systemctl restart ssh >/dev/null 2>&1 \ || log_warn "Unable to reload ssh service" - elif systemctl list-unit-files sshd.service >/dev/null 2>&1; then + elif unit_exists sshd.service; then systemctl reload sshd >/dev/null 2>&1 \ || systemctl restart sshd >/dev/null 2>&1 \ || log_warn "Unable to reload sshd service" diff --git a/lib/tasks/ssh_import.sh b/lib/tasks/ssh_import.sh index 9016a96..1df12bc 100644 --- a/lib/tasks/ssh_import.sh +++ b/lib/tasks/ssh_import.sh @@ -68,19 +68,11 @@ run_ssh_import() { # shellcheck disable=SC2064 trap "rm -f '$tmp_keys'" RETURN local imported=0 - # Secure curl defaults for key imports: HTTPS-only (we already reject - # non-https for --ssh-import-url above), bounded redirects + timeouts - # to avoid hung downloads, TLS 1.2+, retry-with-backoff on transient - # network errors. -f ensures a non-2xx response does not leave a - # partially-written HTML error page in $tmp_keys. - local -a _curl_secure=( - --fail --silent --show-error --location - --proto '=https' --proto-redir '=https' - --max-redirs 5 - --connect-timeout 15 --max-time 60 - --tlsv1.2 - --retry 3 --retry-delay 2 --retry-connrefused - ) + # Key imports use the shared download security policy (pi_curl_secure in + # lib/util/apt.sh): HTTPS-only (we already reject non-https for + # --ssh-import-url above), bounded redirects, TLS 1.2+, retry-with- + # backoff. We pass --max-time 60 per call (tighter than the default) and + # --max-filesize to cap the key payload. # Cap on downloaded key file size (128 KiB). A legitimate GitHub .keys # response is a few KiB at most; anything larger is either a mistake @@ -207,7 +199,7 @@ PY return 1 fi local gh_url="https://github.com/${SSH_IMPORT_GITHUB}.keys" - if curl "${_curl_secure[@]}" --max-filesize "$_max_keys_bytes" "$gh_url" -o "$tmp_keys" \ + if pi_curl_secure --max-time 60 --max-filesize "$_max_keys_bytes" "$gh_url" -o "$tmp_keys" \ && [[ -s "$tmp_keys" ]]; then if _ssh_import_merge_keys "$tmp_keys" "$authorized" "github:$SSH_IMPORT_GITHUB" >/dev/null; then imported=1 @@ -230,7 +222,7 @@ PY rm -f "$tmp_keys" return 1 fi - if curl "${_curl_secure[@]}" --max-filesize "$_max_keys_bytes" "$SSH_IMPORT_URL" -o "$tmp_keys" \ + if pi_curl_secure --max-time 60 --max-filesize "$_max_keys_bytes" "$SSH_IMPORT_URL" -o "$tmp_keys" \ && [[ -s "$tmp_keys" ]]; then if _ssh_import_merge_keys "$tmp_keys" "$authorized" "url:$SSH_IMPORT_URL" >/dev/null; then imported=1 diff --git a/lib/tasks/sysctl.sh b/lib/tasks/sysctl.sh index 22bac9d..86ba26b 100644 --- a/lib/tasks/sysctl.sh +++ b/lib/tasks/sysctl.sh @@ -1,6 +1,6 @@ # >>> pi-task # id: sysctl -# version: 1.2.0 +# version: 1.3.0 # description: Tune kernel memory and network settings for server/desktop use # category: system # default_enabled: 1 @@ -10,7 +10,7 @@ pi_task_register sysctl \ description="Tune kernel memory and network settings for server/desktop use" \ category=system \ - version=1.2.0 \ + version=1.3.0 \ default_enabled=1 run_sysctl() { @@ -27,9 +27,13 @@ fs.file-max = 2097152 net.core.rmem_max = 33554432 net.core.wmem_max = 33554432 net.core.netdev_max_backlog = 4096 -net.ipv4.ip_forward = 1 -net.ipv4.conf.all.forwarding = 1 -net.ipv4.conf.default.forwarding = 1 +# NOTE: IP forwarding is intentionally NOT enabled here. A default +# pi-optimiser run targets ordinary single-homed hosts; turning every +# Pi into a router (net.ipv4.ip_forward=1) widens the network surface +# and is wrong for the overwhelming majority. Hosts that genuinely route +# (VPN gateway, NAT box) should set it deliberately. send_redirects=0 is +# kept as harmless hardening — a non-router has no business emitting +# ICMP redirects. net.ipv4.conf.all.send_redirects = 0 net.ipv4.conf.default.send_redirects = 0 # TCP BBR + fq qdisc — Google's production default since ~2017. Better diff --git a/lib/tasks/tailscale.sh b/lib/tasks/tailscale.sh index 3974204..4ee0e2b 100644 --- a/lib/tasks/tailscale.sh +++ b/lib/tasks/tailscale.sh @@ -50,21 +50,12 @@ run_tailscale() { record_created "$TAILSCALE_KEY_FILE" record_created "$TAILSCALE_LIST_FILE" key_url="https://pkgs.tailscale.com/stable/${repo_id}/${repo_suite}.noarmor.gpg" - # Secure curl defaults for the Tailscale apt signing key: HTTPS-only, - # bounded redirects/timeouts, TLS 1.2+, retry-with-backoff. - local -a _curl_secure=( - --fail --silent --show-error --location - --proto '=https' --proto-redir '=https' - --max-redirs 5 - --connect-timeout 15 --max-time 120 - --tlsv1.2 - --retry 3 --retry-delay 2 --retry-connrefused - ) - if ! curl "${_curl_secure[@]}" "$key_url" | gpg --dearmor > "$TAILSCALE_KEY_FILE"; then + # Download security policy lives in pi_curl_secure (lib/util/apt.sh). + if ! pi_curl_secure "$key_url" | gpg --dearmor > "$TAILSCALE_KEY_FILE"; then if [[ $repo_suite != "bookworm" ]]; then local fallback_url="https://pkgs.tailscale.com/stable/${repo_id}/bookworm.noarmor.gpg" log_warn "Tailscale key for $repo_suite unavailable; falling back to bookworm" - if curl "${_curl_secure[@]}" "$fallback_url" | gpg --dearmor > "$TAILSCALE_KEY_FILE"; then + if pi_curl_secure "$fallback_url" | gpg --dearmor > "$TAILSCALE_KEY_FILE"; then repo_suite=bookworm else log_error "Failed to download Tailscale signing key from $fallback_url" @@ -86,7 +77,12 @@ EOF if ! apt_update_once; then log_warn "apt-get update encountered issues after adding Tailscale repo" fi - if ! DEBIAN_FRONTEND=noninteractive apt-get install -y tailscale; then + # ensure_packages waits out a held dpkg lock (apt-daily / unattended + # upgrades routinely hold it on a fresh boot) and short-circuits when + # tailscale is already installed; a raw apt-get install would fail + # immediately on the lock and record the task as failed. Also honours + # the AGENTS.md rule: never apt-get install outside ensure_packages. + if ! ensure_packages tailscale; then log_error "Failed to install tailscale package" return 1 fi diff --git a/lib/tasks/thermal_thresholds.sh b/lib/tasks/thermal_thresholds.sh index 6e20c84..2d9b0e2 100644 --- a/lib/tasks/thermal_thresholds.sh +++ b/lib/tasks/thermal_thresholds.sh @@ -48,35 +48,31 @@ run_thermal_thresholds() { return 1 fi backup_file "$CONFIG_TXT_FILE" - local rc changed=0 - if [[ -n ${TEMP_LIMIT:-} ]]; then - rc=0 - ensure_config_key_value "temp_limit=$TEMP_LIMIT" "$CONFIG_TXT_FILE" || rc=$? - [[ $rc -eq 0 ]] && { log_info "Set temp_limit=$TEMP_LIMIT"; changed=1; } - fi - if [[ -n ${TEMP_SOFT_LIMIT:-} ]]; then - rc=0 - ensure_config_key_value "temp_soft_limit=$TEMP_SOFT_LIMIT" "$CONFIG_TXT_FILE" || rc=$? - [[ $rc -eq 0 ]] && { log_info "Set temp_soft_limit=$TEMP_SOFT_LIMIT"; changed=1; } - fi - if [[ -n ${INITIAL_TURBO:-} ]]; then - rc=0 - ensure_config_key_value "initial_turbo=$INITIAL_TURBO" "$CONFIG_TXT_FILE" || rc=$? - [[ $rc -eq 0 ]] && { log_info "Set initial_turbo=$INITIAL_TURBO"; changed=1; } - fi - if [[ $changed -eq 0 ]]; then - log_info "Thermal thresholds already match requested values" + local -a entries=() + mapfile -t entries < <(_thermal_thresholds_entries) + if (( ${#entries[@]} > 0 )); then + local rc=0 + apply_config_entries "" all "${entries[@]}" || rc=$? + [[ $rc -eq 1 ]] && log_info "Thermal thresholds already match requested values" fi write_json_field "$CONFIG_OPTIMISER_STATE" "hardware.thermal.temp_limit" "${TEMP_LIMIT:-unset}" write_json_field "$CONFIG_OPTIMISER_STATE" "hardware.thermal.temp_soft_limit" "${TEMP_SOFT_LIMIT:-unset}" write_json_field "$CONFIG_OPTIMISER_STATE" "hardware.thermal.initial_turbo" "${INITIAL_TURBO:-unset}" } +# Single source of truth for the firmware thermal entries, gated on +# which values the operator actually requested. Consumed by both +# run_thermal_thresholds and pi_preview_thermal_thresholds. +_thermal_thresholds_entries() { + [[ -n ${TEMP_LIMIT:-} ]] && printf '%s\n' "temp_limit=$TEMP_LIMIT" + [[ -n ${TEMP_SOFT_LIMIT:-} ]] && printf '%s\n' "temp_soft_limit=$TEMP_SOFT_LIMIT" + [[ -n ${INITIAL_TURBO:-} ]] && printf '%s\n' "initial_turbo=$INITIAL_TURBO" + return 0 +} + pi_preview_thermal_thresholds() { [[ ${THERMAL_THRESHOLDS_SET:-0} -eq 0 ]] && return 0 local -a entries=() - [[ -n ${TEMP_LIMIT:-} ]] && entries+=("temp_limit=$TEMP_LIMIT") - [[ -n ${TEMP_SOFT_LIMIT:-} ]] && entries+=("temp_soft_limit=$TEMP_SOFT_LIMIT") - [[ -n ${INITIAL_TURBO:-} ]] && entries+=("initial_turbo=$INITIAL_TURBO") + mapfile -t entries < <(_thermal_thresholds_entries) (( ${#entries[@]} > 0 )) && pi_preview_apply_entries "${entries[@]}" } diff --git a/lib/tasks/ufw_firewall.sh b/lib/tasks/ufw_firewall.sh index 7acfd3b..32de08f 100644 --- a/lib/tasks/ufw_firewall.sh +++ b/lib/tasks/ufw_firewall.sh @@ -17,13 +17,12 @@ pi_task_register ufw_firewall \ flags="--install-firewall" \ gate_var=INSTALL_FIREWALL -# Detect the SSH listen port from sshd_config; fall back to 22. +# Detect the EFFECTIVE SSH listen port (honours sshd_config.d drop-ins); +# fall back to 22. Delegates to the shared resolver in lib/util/sshd.sh +# so ufw_firewall and secure_ssh can never disagree on the port — and so +# enabling UFW can't drop a session listening on a drop-in custom port. _ufw_ssh_port() { - local port="" - if [[ -r /etc/ssh/sshd_config ]]; then - port=$(awk '/^[[:space:]]*Port[[:space:]]+[0-9]+/{print $2; exit}' /etc/ssh/sshd_config 2>/dev/null) - fi - echo "${port:-22}" + pi_sshd_effective_port } # Build a fingerprint of everything that influences the rule set. When @@ -55,6 +54,19 @@ _ufw_fingerprint() { printf '%s\n' "$fp" } +# Re-reconcile the firewall when the inputs that shape the rule set (SSH +# port, VPN interfaces, proxy) have changed since the last run, comparing +# the live fingerprint against the stored one. apply_once consults this +# hook to re-run a completed task without mutating state.json, so it is +# safe under --dry-run/--diff. Returns 0 (changed) when a re-run is due. +pi_ufw_firewall_value_changed() { + [[ ${INSTALL_FIREWALL:-0} -eq 1 ]] || return 1 + local current stored + current=$(_ufw_fingerprint) + stored=$(read_json_field "$CONFIG_OPTIMISER_STATE" "firewall.fingerprint" 2>/dev/null || echo "") + [[ -n "$stored" && "$stored" != "$current" ]] +} + run_ufw_firewall() { if [[ ${INSTALL_FIREWALL:-0} -eq 0 ]]; then log_info "UFW firewall not requested; skipping" diff --git a/lib/tasks/unattended.sh b/lib/tasks/unattended.sh index 8d6a512..dd38eaf 100644 --- a/lib/tasks/unattended.sh +++ b/lib/tasks/unattended.sh @@ -35,8 +35,6 @@ run_unattended() { # back to backup_file, so --undo restores the prior content instead # of deleting it. record_created "$UNATTENDED_CONF_FILE" - record_created "$UNATTENDED_SERVICE" - record_created "$UNATTENDED_TIMER" cat < "$UNATTENDED_CONF_FILE" Unattended-Upgrade::Origins-Pattern { "origin=${os_origin},codename=${codename}-security"; @@ -59,7 +57,7 @@ CFG # - NoNewPrivileges: intentionally NOT set — dpkg triggers may call # helpers that legitimately need setuid (e.g. /usr/bin/sudo postinst # chmod 4755). Forcing it would break some postinst scripts. - cat < "$UNATTENDED_SERVICE" + write_systemd_unit "$UNATTENDED_SERVICE" < "$UNATTENDED_TIMER" + write_systemd_unit "$UNATTENDED_TIMER" <<'CFG' [Unit] Description=Run unattended-upgrades every 6 hours (pi-optimiser) Documentation=man:unattended-upgrade(8) @@ -93,6 +91,6 @@ Persistent=true [Install] WantedBy=timers.target CFG - systemctl daemon-reload >/dev/null 2>&1 || true + pi_daemon_reload_now systemctl enable --now pi-unattended-upgrades.timer >/dev/null 2>&1 || log_warn "Could not enable pi-unattended-upgrades.timer" } diff --git a/lib/tasks/underclock.sh b/lib/tasks/underclock.sh index eb9fb1d..f3a40a6 100644 --- a/lib/tasks/underclock.sh +++ b/lib/tasks/underclock.sh @@ -19,6 +19,22 @@ pi_task_register underclock \ gate_var=REQUEST_UNDERCLOCK \ reboot_required=1 +# Single source of truth for the per-model underclock plan, consumed by +# both run_underclock and pi_preview_underclock so --diff matches what is +# written (and to the correct per-model section). Prints: +#
\n\n... (no output = unsupported model). +_underclock_plan() { + if is_pi5; then + printf '%s\n' pi5 pi5_underclock arm_freq=1800 gpu_freq=700 + elif is_pi4; then + printf '%s\n' pi4 pi4_underclock arm_freq=1200 gpu_freq=400 + elif is_pi3; then + printf '%s\n' pi3 pi3_underclock arm_freq=1000 gpu_freq=300 + elif is_pizero2; then + printf '%s\n' pi02 pi_zero2_underclock arm_freq=900 gpu_freq=300 + fi +} + run_underclock() { if [[ ${REQUEST_UNDERCLOCK:-0} -eq 0 ]]; then log_info "Underclock not requested; skipping" @@ -36,47 +52,19 @@ run_underclock() { return 2 fi - local -a entries=() - local profile="" - # Model-specific values — route into the matching [piN] section so - # a card moved to a different Pi falls back to its own clocks. - local section="all" - if is_pi5; then - entries=("arm_freq=1800" "gpu_freq=700") - profile="pi5_underclock" - section="pi5" - elif is_pi4; then - entries=("arm_freq=1200" "gpu_freq=400") - profile="pi4_underclock" - section="pi4" - elif is_pi3; then - entries=("arm_freq=1000" "gpu_freq=300") - profile="pi3_underclock" - section="pi3" - elif is_pizero2; then - entries=("arm_freq=900" "gpu_freq=300") - profile="pi_zero2_underclock" - section="pi02" - else + local -a plan=() + mapfile -t plan < <(_underclock_plan) + if (( ${#plan[@]} == 0 )); then log_info "Underclock not supported on model ${SYSTEM_MODEL:-unknown}" pi_skip_reason "model unsupported" return 2 fi + local section=${plan[0]} profile=${plan[1]} + local -a entries=("${plan[@]:2}") backup_file "$CONFIG_TXT_FILE" - local entry rc applied=0 safe_key - for entry in "${entries[@]}"; do - rc=0 - ensure_config_key_value "$entry" "$CONFIG_TXT_FILE" "$section" || rc=$? - if [[ $rc -eq 0 ]]; then - log_info "Applied $entry to config.txt" - safe_key=${entry//=/_} - write_json_field "$CONFIG_OPTIMISER_STATE" "underclock.${safe_key}" "$entry" - applied=1 - elif [[ $rc -gt 1 ]]; then - log_warn "Failed to apply $entry" - fi - done + local rc=0 + apply_config_entries "underclock" "$section" "${entries[@]}" || rc=$? # Switch scaling governor to powersave to pair with the lower clocks. local g @@ -85,28 +73,21 @@ run_underclock() { echo powersave > "$g" 2>/dev/null || true done - if [[ $applied -eq 1 ]]; then - write_json_field "$CONFIG_OPTIMISER_STATE" "underclock.profile" "$profile" - log_info "Underclock profile applied: $profile" - else - log_info "Underclock profile already present" - fi + case $rc in + 0) + write_json_field "$CONFIG_OPTIMISER_STATE" "underclock.profile" "$profile" + log_info "Underclock profile applied: $profile" + ;; + 1) log_info "Underclock profile already present" ;; + *) log_warn "One or more underclock entries failed to apply" ;; + esac } pi_preview_underclock() { [[ ${REQUEST_UNDERCLOCK:-0} -eq 0 ]] && return 0 [[ ${REQUEST_OC_CONSERVATIVE:-0} -eq 1 ]] && return 0 - local -a entries=() - if is_pi5; then - entries=("arm_freq=1800" "gpu_freq=700") - elif is_pi4; then - entries=("arm_freq=1200" "gpu_freq=400") - elif is_pi3; then - entries=("arm_freq=1000" "gpu_freq=300") - elif is_pizero2; then - entries=("arm_freq=900" "gpu_freq=300") - else - return 0 - fi - pi_preview_apply_entries "${entries[@]}" + local -a plan=() + mapfile -t plan < <(_underclock_plan) + (( ${#plan[@]} == 0 )) && return 0 + pi_preview_apply_entries --section "${plan[0]}" "${plan[@]:2}" } diff --git a/lib/tasks/usb_uas_quirks.sh b/lib/tasks/usb_uas_quirks.sh index 4aaea54..9af47c0 100644 --- a/lib/tasks/usb_uas_quirks.sh +++ b/lib/tasks/usb_uas_quirks.sh @@ -1,6 +1,6 @@ # >>> pi-task # id: usb_uas_quirks -# version: 1.0.0 +# version: 1.1.0 # description: Disable UAS on known-broken USB-SATA adapters (auto-detect + list) # category: storage # default_enabled: 0 @@ -13,7 +13,7 @@ pi_task_register usb_uas_quirks \ description="Disable UAS on known-broken USB-SATA adapters (auto-detect + list)" \ category=storage \ - version=1.0.0 \ + version=1.1.0 \ default_enabled=0 \ flags="--usb-uas-quirks" \ gate_var=USB_UAS_QUIRKS \ @@ -90,6 +90,32 @@ _usb_uas_detect() { printf '%s' "$out" } +# Union two comma-separated quirks lists, preserving order (existing +# entries first, then newly-detected). usb-storage.quirks is a single +# kernel module parameter, so an existing token already in cmdline.txt +# must be merged with — not clobbered or duplicated by — what we detect +# this run; otherwise a pre-existing quirk (or one for an adapter that +# happens to be unplugged this boot) is silently dropped. +_usb_uas_merge() { + local existing=$1 detected=$2 + local -A seen=() + local out="" item + local -a items=() + IFS=',' read -ra items <<< "$existing" + local -a more=() + IFS=',' read -ra more <<< "$detected" + items+=("${more[@]}") + for item in "${items[@]}"; do + item=${item//[[:space:]]/} + [[ -z "$item" ]] && continue + [[ -n "${seen[$item]:-}" ]] && continue + seen[$item]=1 + [[ -n "$out" ]] && out+="," + out+="$item" + done + printf '%s' "$out" +} + run_usb_uas_quirks() { if [[ ${USB_UAS_QUIRKS:-0} -eq 0 ]]; then log_info "USB UAS quirks not requested; skipping" @@ -109,15 +135,21 @@ run_usb_uas_quirks() { return 2 fi backup_file "$CMDLINE_FILE" - local token="usb-storage.quirks=$quirks" + # Merge with any existing usb-storage.quirks= token and write exactly + # one (cmdline_set_kv replaces by key). cmdline_ensure_token would have + # appended a second token, and the kernel keeps only the last — silently + # dropping a pre-existing quirk or one for an unplugged adapter. + local existing merged + existing=$(cmdline_get_value "usb-storage.quirks" "$CMDLINE_FILE" 2>/dev/null || true) + merged=$(_usb_uas_merge "$existing" "$quirks") local rc=0 - cmdline_ensure_token "$token" "$CMDLINE_FILE" || rc=$? + cmdline_set_kv "usb-storage.quirks" "$merged" "$CMDLINE_FILE" || rc=$? case $rc in - 0) log_info "Applied $token (reboot required)" ;; - 1) log_info "$token already present in cmdline.txt" ;; + 0) log_info "Applied usb-storage.quirks=$merged (reboot required)" ;; + 1) log_info "usb-storage.quirks=$merged already present in cmdline.txt" ;; *) log_warn "Failed to write USB quirks to cmdline.txt"; return 1 ;; esac - write_json_field "$CONFIG_OPTIMISER_STATE" "storage.uas_quirks" "$quirks" + write_json_field "$CONFIG_OPTIMISER_STATE" "storage.uas_quirks" "$merged" } pi_preview_usb_uas_quirks() { @@ -126,5 +158,8 @@ pi_preview_usb_uas_quirks() { quirks=$(_usb_uas_detect) [[ -z "$quirks" ]] && return 0 local target=${CMDLINE_FILE:-/boot/firmware/cmdline.txt} - cmdline_ensure_token "usb-storage.quirks=$quirks" "$target" >/dev/null 2>&1 || true + local existing merged + existing=$(cmdline_get_value "usb-storage.quirks" "$target" 2>/dev/null || true) + merged=$(_usb_uas_merge "$existing" "$quirks") + cmdline_set_kv "usb-storage.quirks" "$merged" "$target" >/dev/null 2>&1 || true } diff --git a/lib/tasks/var_log_tmpfs.sh b/lib/tasks/var_log_tmpfs.sh index 4e6a9bf..2a58945 100644 --- a/lib/tasks/var_log_tmpfs.sh +++ b/lib/tasks/var_log_tmpfs.sh @@ -27,10 +27,22 @@ run_var_log_tmpfs() { if [[ -d /var/log/journal ]]; then if find /var/log/journal -mindepth 1 -print -quit 2>/dev/null | grep -q .; then - local backup_tar - backup_tar=/var/log.journal-backup.pi-optimiser.$(date +%Y%m%d%H%M%S).tar.gz - if tar -czf "$backup_tar" -C /var/log journal >/dev/null 2>&1; then - log_info "Archived existing journal to $backup_tar" + # systemd journals contain sensitive data (auth failures with + # usernames/IPs, sudo invocations, secrets daemons log in the + # clear). On-disk they are 0640 root:systemd-journal; the archive + # MUST NOT downgrade that to world-readable. Write it under + # /var/backups (a sibling of /var/log so it survives the tmpfs + # mount) with umask 077 so it is created 0600 from the start (no + # 0644 race window), and belt-and-suspenders chmod afterwards. + # The rm must precede the mount: once tmpfs is mounted over + # /var/log the underlying journal is shadowed and unreclaimable, + # and it only runs after a successful archive, so no data is lost. + local backup_tar backup_dir=/var/backups + mkdir -p "$backup_dir" + backup_tar="$backup_dir/pi-optimiser-journal.$(date +%Y%m%d%H%M%S).tar.gz" + if ( umask 077; tar -czf "$backup_tar" -C /var/log journal ) >/dev/null 2>&1; then + chmod 600 "$backup_tar" 2>/dev/null || true + log_info "Archived existing journal to $backup_tar (mode 0600)" rm -rf /var/log/journal/* 2>/dev/null || true else log_warn "Failed to archive /var/log/journal prior to tmpfs mount" diff --git a/lib/tasks/wifi_bt_power.sh b/lib/tasks/wifi_bt_power.sh index 1ec14a8..f2ccba4 100644 --- a/lib/tasks/wifi_bt_power.sh +++ b/lib/tasks/wifi_bt_power.sh @@ -31,8 +31,7 @@ run_wifi_bt_power() { if [[ ${WIFI_POWERSAVE_OFF:-0} -eq 1 ]]; then mkdir -p /etc/systemd/system local wifi_unit=/etc/systemd/system/pi-optimiser-wifi-powersave-off.service - record_created "$wifi_unit" - cat <<'CFG' > "$wifi_unit" + write_systemd_unit "$wifi_unit" <<'CFG' [Unit] Description=Disable Wi-Fi power save (pi-optimiser) After=network.target @@ -45,10 +44,7 @@ ExecStart=/bin/sh -c 'for d in /sys/class/net/wlan*; do iface=$(basename "$d"); [Install] WantedBy=multi-user.target CFG - # Pin the unit to 0644 so a loose operator umask can't leave it - # group/world-writable — the unit runs as root on every boot. - chmod 0644 "$wifi_unit" 2>/dev/null || true - systemctl daemon-reload >/dev/null 2>&1 || true + pi_daemon_reload_now systemctl enable --now pi-optimiser-wifi-powersave-off.service >/dev/null 2>&1 || log_warn "Unable to enable wifi-powersave-off unit" log_info "Wi-Fi power save disabled (service enabled)" did=1 diff --git a/lib/tasks/zram.sh b/lib/tasks/zram.sh index 8d94900..018568f 100644 --- a/lib/tasks/zram.sh +++ b/lib/tasks/zram.sh @@ -75,7 +75,7 @@ run_zram() { rm -f "$ZRAM_CONF_FILE" removed=1 fi - if systemctl list-unit-files systemd-zram-setup@.service >/dev/null 2>&1; then + if unit_exists systemd-zram-setup@.service; then systemctl disable --now systemd-zram-setup@zram0 >/dev/null 2>&1 || log_warn "Unable to disable systemd-zram-setup@zram0" fi swapoff /dev/zram0 >/dev/null 2>&1 || true @@ -135,8 +135,8 @@ swap-priority = 100 CFG log_info "Configured $ZRAM_CONF_FILE for ${size_mb}MB ZRAM swap" - systemctl daemon-reload >/dev/null 2>&1 || true - if systemctl list-unit-files systemd-zram-setup@.service >/dev/null 2>&1; then + pi_daemon_reload_now + if unit_exists systemd-zram-setup@.service; then systemctl enable --now systemd-zram-setup@zram0 >/dev/null 2>&1 || log_warn "Unable to enable systemd-zram-setup@zram0" else log_warn "systemd-zram-setup@.service not found; ensure systemd-zram-generator is installed" diff --git a/lib/ui/tui.sh b/lib/ui/tui.sh index 1143a45..bc3a4d9 100644 --- a/lib/ui/tui.sh +++ b/lib/ui/tui.sh @@ -409,13 +409,21 @@ _pi_tui_apply() { # Value-typed gate_vars (hostnames, timezones, URLs) are set via the # "values" forms menu; we never coerce them to "1". local tid gate - local _preserve_wifi=0 _preserve_bt=0 _preserve_zram_disabled=0 + local _preserve_wifi=0 _preserve_bt=0 _preserve_zram_disabled=0 _preserve_zram_algo="" if [[ -n "${PI_TUI_SELECTED[wifi_bt_power]:-}" ]]; then _preserve_wifi=${WIFI_POWERSAVE_OFF:-0} _preserve_bt=${DISABLE_BLUETOOTH:-0} fi - if [[ -n "${PI_TUI_SELECTED[zram]:-}" && "${ZRAM_ALGO_OVERRIDE:-}" == "disabled" ]]; then - _preserve_zram_disabled=1 + if [[ -n "${PI_TUI_SELECTED[zram]:-}" ]]; then + if [[ "${ZRAM_ALGO_OVERRIDE:-}" == "disabled" ]]; then + _preserve_zram_disabled=1 + elif [[ -n "${ZRAM_ALGO_OVERRIDE:-}" ]]; then + # A non-default algo (e.g. zstd) set via config.yaml / CLI must + # survive the visited-category reset below, otherwise re-selecting + # zram in the storage menu silently downgrades it to the lz4 + # default both at runtime and in the saved config.yaml. + _preserve_zram_algo=$ZRAM_ALGO_OVERRIDE + fi fi # For every category the operator visited, reset that category's # binary gates to 0 first. Ticked items in the loop below then @@ -469,6 +477,8 @@ _pi_tui_apply() { ZRAM_ALGO_OVERRIDE=disabled else INSTALL_ZRAM=1 + # Restore a non-default algo (zstd) captured before the reset. + [[ -n $_preserve_zram_algo ]] && ZRAM_ALGO_OVERRIDE=$_preserve_zram_algo fi continue ;; @@ -483,6 +493,29 @@ _pi_tui_apply() { ;; esac done + # Second pass: tasks in categories the operator never opened keep their + # default behaviour, so the TUI's applied set matches what the SAME + # config would do from the CLI. Without this: + # - a value-typed task whose value was set only via the Values menu + # (proxy backend, SSH import, hostname/timezone/locale) would be + # saved to config.yaml but never actually run on this Apply; and + # - every default-enabled foundational task (full_upgrade, sysctl, + # fstab, ...) in an unvisited category would be silently dropped. + # Visited categories are skipped here because there the checklist ticks + # are authoritative (un-ticking must be able to remove a task). + local _seen + for tid in "${PI_TASK_ORDER[@]}"; do + [[ -n "${PI_TUI_SELECTED[$tid]:-}" ]] && continue + [[ -n "${PI_TUI_VISITED_CATEGORIES[${PI_TASK_CATEGORY[$tid]}]:-}" ]] && continue + if [[ ${PI_TASK_DEFAULT[$tid]:-1} == "1" ]] || _pi_tui_gate_active "$tid"; then + # Guard against double-adding (a task is only reached once per loop, + # but keep ONLY_TASKS free of duplicates for clean --only semantics). + _seen=0 + local _ot + for _ot in "${ONLY_TASKS[@]}"; do [[ "$_ot" == "$tid" ]] && { _seen=1; break; }; done + [[ $_seen -eq 0 ]] && ONLY_TASKS+=("$tid") + fi + done if (( ${#ONLY_TASKS[@]} == 0 )); then _whiptail --msgbox "No tasks selected; nothing to apply." 8 50 return 0 diff --git a/lib/util/apt.sh b/lib/util/apt.sh index ee697c2..237e538 100644 --- a/lib/util/apt.sh +++ b/lib/util/apt.sh @@ -1,11 +1,36 @@ # ====================================================================== # lib/util/apt.sh — apt wrappers # -# Functions: apt_update_once, ensure_packages, apt_wait_for_lock +# Functions: apt_update_once, ensure_packages, apt_wait_for_lock, +# pi_curl_secure # Globals (read): APT_UPDATED, NETWORK_AVAILABLE, APT_LOCK_BUSY # Globals (write): APT_UPDATED, APT_LOCK_BUSY # ====================================================================== +# Run curl with hardened defaults for third-party downloads (apt signing +# keys, SSH key imports, ...). Centralises the download security policy +# that was previously copy-pasted as a `_curl_secure` array in +# docker/tailscale/ssh_import — so tightening it (e.g. raising the TLS +# floor to 1.3, or changing the redirect policy) is a single edit instead +# of four, and no downloader can silently drift weaker than the others. +# - HTTPS only, on the initial request AND across redirects (no +# plaintext, no downgrade-on-redirect); +# - bounded redirects; explicit connect/total timeouts so a hung mirror +# can't block the run; TLS 1.2 floor; retry-with-backoff on transient +# failures. +# Caller args (URL, -o, -I, --max-filesize, and any --max-time/-timeout +# override) are appended; curl honours the last occurrence of a repeated +# option, so callers can tighten a timeout per-call. +pi_curl_secure() { + curl --fail --silent --show-error --location \ + --proto '=https' --proto-redir '=https' \ + --max-redirs 5 \ + --connect-timeout 15 --max-time 120 \ + --tlsv1.2 \ + --retry 3 --retry-delay 2 --retry-connrefused \ + "$@" +} + # Wait up to N seconds (default 30) for any concurrent apt/dpkg frontend # to release the lock. Returns 0 if the lock is free (or never was # taken), 1 if still busy after the timeout. Never blocks indefinitely — diff --git a/lib/util/cmdline.sh b/lib/util/cmdline.sh index 558a8e1..ecfd2e2 100644 --- a/lib/util/cmdline.sh +++ b/lib/util/cmdline.sh @@ -1,7 +1,7 @@ # ====================================================================== # lib/util/cmdline.sh — /boot/firmware/cmdline.txt helpers # -# Functions: cmdline_ensure_token +# Functions: cmdline_ensure_token, cmdline_get_value, cmdline_set_kv # Globals (read): CMDLINE_FILE # ====================================================================== @@ -73,3 +73,104 @@ PY fi return 1 } + +# Print the value of the first `key=value` token on the cmdline.txt line. +# Returns 0 and prints the value (possibly empty) if the key is present; +# returns 1 (prints nothing) if absent. Used to read single-valued kernel +# module params (e.g. usb-storage.quirks) so callers can merge rather +# than blindly append a duplicate token. +cmdline_get_value() { + local key=$1 + local file=${2:-$CMDLINE_FILE} + [[ -f "$file" ]] || return 1 + CMDLINE_PATH="$file" CMDLINE_KEY="$key" run_python <<'PY' +import os +from pathlib import Path +path = Path(os.environ['CMDLINE_PATH']) +key = os.environ['CMDLINE_KEY'] +try: + first = path.read_text().replace('\r\n', '\n').replace('\r', '\n').split('\n', 1)[0] +except FileNotFoundError: + raise SystemExit(1) +for tok in first.split(): + if tok.startswith(key + '='): + print(tok[len(key) + 1:]) + raise SystemExit(0) +raise SystemExit(1) +PY +} + +# Set `key=value` as a SINGLE token in cmdline.txt, replacing any existing +# token for that key in place (and dropping duplicates) rather than +# appending a second one. Kernel module params like usb-storage.quirks +# are single-valued — the parser keeps only the last occurrence — so a +# blind append silently discards an earlier setting. Returns 0 if the +# line changed, 1 if it already matched, 2 on error. +cmdline_set_kv() { + local key=$1 + local value=$2 + local file=${3:-$CMDLINE_FILE} + file=$(_pi_config_preview_target "$file") || return 2 + if [[ ! -f "$file" ]]; then + return 2 + fi + local result="" + local rc=0 + result=$(CMDLINE_PATH="$file" CMDLINE_KEY="$key" CMDLINE_VALUE="$value" run_python <<'PY' +import os +from pathlib import Path +path = Path(os.environ['CMDLINE_PATH']) +key = os.environ['CMDLINE_KEY'] +value = os.environ['CMDLINE_VALUE'].replace('\r', '').replace('\n', '').strip() +new_token = key + '=' + value +raw = path.read_text() +first_line = raw.replace('\r\n', '\n').replace('\r', '\n').split('\n', 1)[0] +parts = first_line.split() +out = [] +replaced = False +changed = False +for tok in parts: + if tok == key or tok.startswith(key + '='): + if not replaced: + out.append(new_token) + replaced = True + if tok != new_token: + changed = True + else: + # second/third token for the same key — drop the duplicate + changed = True + continue + out.append(tok) +if not replaced: + out.append(new_token) + changed = True +if not changed: + print('unchanged') + raise SystemExit(0) +payload = " ".join(out) + "\n" +# Atomic write — cmdline.txt is boot-critical; never leave it truncated. +tmp_path = path.with_suffix(path.suffix + '.pi-optimiser.tmp') +with open(tmp_path, 'w') as fh: + fh.write(payload) + fh.flush() + os.fsync(fh.fileno()) +os.replace(tmp_path, path) +try: + dfd = os.open(str(path.parent), os.O_DIRECTORY) + try: + os.fsync(dfd) + finally: + os.close(dfd) +except OSError: + pass +print('changed') +PY + ) || rc=$? + if [[ $rc -ne 0 ]]; then + return 2 + fi + if [[ "$result" == "changed" ]]; then + return 0 + fi + return 1 +} diff --git a/lib/util/config_txt.sh b/lib/util/config_txt.sh index a047f79..c480862 100644 --- a/lib/util/config_txt.sh +++ b/lib/util/config_txt.sh @@ -2,7 +2,8 @@ # lib/util/config_txt.sh — /boot/firmware/config.txt editors # # Functions: ensure_config_line, ensure_config_key_value, -# ensure_line_in_file, _pi_config_preview_target +# apply_config_entries, _pi_config_apply_one, +# _pi_config_entry_is_bare_overlay, _pi_config_preview_target # Globals (read): CONFIG_TXT_FILE, PI_CONFIG_PREVIEW, PI_CONFIG_PREVIEW_DIR # # Return codes (ensure_config_*): @@ -249,6 +250,68 @@ PY return 1 } +# A bare `dtoverlay=NAME` (no parameters, i.e. exactly one '=') is +# additive — a config.txt may legitimately enable several distinct +# overlays — so it uses whole-line semantics. Everything else +# (key=value, dtparam=NAME=val, dtoverlay=NAME,opt=val) is a keyed +# setting and is upserted by key. +_pi_config_entry_is_bare_overlay() { + [[ $1 == dtoverlay=* && $1 != *=*=* ]] +} + +# Apply ONE config.txt entry with the correct primitive. Mirrors the +# dispatch used by apply_config_entries so --diff previews match what +# run_ writes. Returns ensure_config_*'s rc (0 changed, +# 1 unchanged, 2 error). +_pi_config_apply_one() { + local entry=$1 + local target=${2:-$CONFIG_TXT_FILE} + local section=${3:-all} + if _pi_config_entry_is_bare_overlay "$entry"; then + ensure_config_line "$entry" "$target" "$section" + else + ensure_config_key_value "$entry" "$target" "$section" + fi +} + +# apply_config_entries
... +# Upsert each ENTRY into $CONFIG_TXT_FILE within SECTION using the +# shared dispatch above, with uniform logging and return semantics, so +# the config.txt tasks don't each hand-roll the loop. When STATE_PREFIX +# is non-empty, every applied entry is also recorded under +# . in CONFIG_OPTIMISER_STATE (pass "" to skip +# and let the task write its own summary field). +# Returns: 0 if anything changed, 1 if all entries were already +# present, 2 if any write failed. +apply_config_entries() { + local prefix=$1 + local section=${2:-all} + shift 2 + local entry rc changed=0 failed=0 safe_key + for entry in "$@"; do + rc=0 + _pi_config_apply_one "$entry" "$CONFIG_TXT_FILE" "$section" || rc=$? + case $rc in + 0) + changed=1 + log_info "Applied $entry to config.txt" + if [[ -n $prefix ]]; then + safe_key=${entry//=/_} + write_json_field "$CONFIG_OPTIMISER_STATE" "${prefix}.${safe_key}" "$entry" + fi + ;; + 1) : ;; + *) + failed=1 + log_warn "Failed to apply $entry to config.txt" + ;; + esac + done + [[ $failed -eq 1 ]] && return 2 + [[ $changed -eq 1 ]] && return 0 + return 1 +} + # Ensure config.txt contains exactly one `key=value` line for the given # key, inside the given section. # Args: 1=entry (key=value), 2=target (default $CONFIG_TXT_FILE), @@ -269,12 +332,11 @@ ensure_config_key_value() { if [[ ! -f "$target" ]]; then touch "$target" fi - local key=${entry%%=*} local warn_file warn_file=$(mktemp) || return 2 local result="" local rc=0 - result=$(CONFIG_FILE="$target" CONFIG_ENTRY="$entry" CONFIG_KEY="$key" \ + result=$(CONFIG_FILE="$target" CONFIG_ENTRY="$entry" \ CONFIG_SECTION="$section" CONFIG_WARN_FILE="$warn_file" run_python <<'PY' import os from pathlib import Path @@ -284,9 +346,34 @@ warn_path = os.environ.get('CONFIG_WARN_FILE', '') # Strip CR so a caller passing a CRLF-laced value doesn't seed mixed # line endings into /boot/firmware/config.txt. entry = os.environ['CONFIG_ENTRY'].strip().replace('\r', '') -key = os.environ['CONFIG_KEY'].strip().lower() target_section = os.environ['CONFIG_SECTION'].strip().lower() or 'all' +# The dtparam / dtoverlay families are MULTI-VALUED: a single config.txt +# legitimately holds many of them (dtparam=audio=on, dtparam=i2c_arm=on, +# dtparam=spi=on, ...). Keying them all on the bare word 'dtparam' would +# make every dtparam line collide as one key, so writing dtparam=watchdog=on +# would overwrite/delete the stock audio/i2c/spi lines. For these families +# the effective key must include the parameter/overlay NAME so distinct +# settings never collide; for everything else the key is the text before +# the first '=' (gpu_mem, arm_freq, ...). +_MULTI_VALUE_KEYS = ('dtparam', 'dtoverlay') + +def effective_key(text): + text = text.strip() + if '=' not in text: + return None + head = text.split('=', 1)[0].strip().lower() + if head in _MULTI_VALUE_KEYS: + rest = text.split('=', 1)[1] + # dtparam=NAME=value -> NAME ; dtoverlay=NAME,opt=val -> NAME + name = rest.split('=', 1)[0].split(',', 1)[0].strip().lower() + return head + '=' + name if name else head + return head + +# The match key is derived from the entry itself (single source of truth) +# so the shell and Python sides can never diverge. +key = effective_key(entry) + def _warn(msg): if warn_path: with open(warn_path, 'a') as wh: @@ -309,12 +396,9 @@ def parse_sections(lines): return sections def is_key_line(raw, want_key): - stripped = raw.strip() - candidate = stripped.lstrip('#').strip() - if '=' not in candidate: - return False - cand_key = candidate.split('=', 1)[0].strip().lower() - return cand_key == want_key + candidate = raw.strip().lstrip('#').strip() + cand_key = effective_key(candidate) + return cand_key is not None and cand_key == want_key sections = parse_sections(existing) @@ -391,7 +475,7 @@ for idx, (name, body) in enumerate(sections): # Atomic write: stage .tmp + os.replace so a power loss between # truncate and write can never brick the boot config. Durable via -# file + parent-dir fsync. +# file + parent-dir fsync. (apply helpers follow this function.) payload = '\n'.join(out_lines) + '\n' tmp_path = config_path.with_suffix(config_path.suffix + '.pi-optimiser.tmp') with open(tmp_path, 'w') as fh: diff --git a/lib/util/config_yaml.sh b/lib/util/config_yaml.sh index 83aab4f..1ac7d8a 100644 --- a/lib/util/config_yaml.sh +++ b/lib/util/config_yaml.sh @@ -335,8 +335,12 @@ emit_bool("DOCKER_CGROUPV2", docker, "cgroup_v2") zram = get(i, "zram", default={}) emit_bool("INSTALL_ZRAM", zram, "enabled") if has(zram, "algo"): - algo = get_str(zram, "algo", default="") - if algo and algo != "lz4": + algo = get_str(zram, "algo", default="").strip().lower() + # Mirror the CLI's --zram-algo allowlist so a config.yaml value can't + # bypass validation and flow into the systemd zram-generator config. + if algo and algo not in ("lz4", "zstd", "disabled"): + out.append('echo "pi-optimiser: invalid zram.algo in config (allowed: lz4, zstd, disabled)" >&2') + elif algo and algo != "lz4": out.append(f'ZRAM_ALGO_OVERRIDE={sv(algo)}') elif algo == "lz4": out.append('ZRAM_ALGO_OVERRIDE=""') diff --git a/lib/util/fstab.sh b/lib/util/fstab.sh index 161bd59..7da44d5 100644 --- a/lib/util/fstab.sh +++ b/lib/util/fstab.sh @@ -1,16 +1,9 @@ # ====================================================================== # lib/util/fstab.sh — /etc/fstab helpers # -# Functions: fstab_append_line, fstab_root_has_option +# Functions: fstab_append_line # ====================================================================== -# Return success when /etc/fstab's root entry already declares an option. -# Matches option names as comma-delimited tokens in the 4th mount column. -fstab_root_has_option() { - local opt=$1 - grep -E "^[[:space:]]*[^#[:space:]]+[[:space:]]+/[[:space:]]+[^[:space:]]+[[:space:]]+[^[:space:]]*${opt}([,[:space:]]|$)" /etc/fstab >/dev/null -} - # Append a complete fstab entry if not already present. # Matches on the MNT (2nd column) to avoid duplicate mountpoints. The # entry must be a valid 6-field line (fs_spec, mount_point, type, diff --git a/lib/util/hardware.sh b/lib/util/hardware.sh index 94ffae6..47d9c02 100644 --- a/lib/util/hardware.sh +++ b/lib/util/hardware.sh @@ -98,7 +98,7 @@ gather_system_info() { # /proc/device-tree/model is NUL-terminated; some images have # trailing NULs, spaces, or carriage returns after "Rev 1.0". Strip # NUL, CR, LF, then trim leading/trailing whitespace so substring - # matches (`*Raspberry Pi 5*`) and case-folded helpers (`is_pi500`) + # matches (`*Raspberry Pi 5*`) and case-folded helpers (`is_pi400`) # behave consistently across Bookworm/Trixie and custom images. SYSTEM_MODEL=$(tr -d '\0\r\n' 5, CM4(+CM4S)/ + # BCM2711 -> 4, CM3(+)/BCM2837 -> 3. + elif [[ $SYSTEM_MODEL == *"Compute Module 5"* ]]; then + SYSTEM_PI_GEN="5" + elif [[ $SYSTEM_MODEL == *"Compute Module 4"* ]]; then + SYSTEM_PI_GEN="4" + elif [[ $SYSTEM_MODEL == *"Compute Module 3"* ]]; then + SYSTEM_PI_GEN="3" elif [[ $SYSTEM_MODEL == *"Raspberry Pi"* ]]; then SYSTEM_PI_GEN="other" else diff --git a/lib/util/model.sh b/lib/util/model.sh index fcaf971..0d061d9 100644 --- a/lib/util/model.sh +++ b/lib/util/model.sh @@ -4,7 +4,7 @@ # Consolidates the scattered `*raspberry pi 500*` / `*raspberry pi 400*` # string checks so tasks ask a single question. # -# Functions: pi_is_generation, is_pi5, is_pi500, is_pi4, is_pi400, +# Functions: pi_is_generation, is_pi5, is_pi4, is_pi400, # is_pi3, is_pizero2, pi_supports_kms_overlays, # pi_supports_eeprom # Globals (read): SYSTEM_PI_GEN, SYSTEM_MODEL @@ -16,17 +16,11 @@ pi_is_generation() { [[ ${SYSTEM_PI_GEN:-unknown} == "$target" ]] } -# Pi 5 / Pi 500 (generation 5 covers both). +# Pi 5 / Pi 500 / CM5 (generation 5 covers all). is_pi5() { pi_is_generation 5 } -# Pi 500 specifically (the keyboard-form-factor Pi 5). -is_pi500() { - local lower=${SYSTEM_MODEL,,} - [[ $lower == *"raspberry pi 500"* ]] -} - # Pi 4 or Pi 400 (generation 4 covers both). is_pi4() { pi_is_generation 4 @@ -54,3 +48,14 @@ pi_supports_kms_overlays() { esac } +# Bootloader EEPROM is present on Pi 4/5-class hardware (and their Compute +# Modules); Pi 3 / Zero 2 boot straight from the SD card with no EEPROM. +# Used by --self-test so it can report EEPROM support accurately instead +# of using the KMS-overlay check as a misleading proxy. +pi_supports_eeprom() { + case ${SYSTEM_PI_GEN:-unknown} in + 4|5) return 0 ;; + *) return 1 ;; + esac +} + diff --git a/lib/util/sshd.sh b/lib/util/sshd.sh index a1aef24..c5cf263 100644 --- a/lib/util/sshd.sh +++ b/lib/util/sshd.sh @@ -1,9 +1,45 @@ # ====================================================================== # lib/util/sshd.sh — sshd_config editor # -# Functions: update_sshd_config_option +# Functions: update_sshd_config_option, pi_sshd_effective_port # ====================================================================== +# Resolve the EFFECTIVE sshd listen port. On Bookworm/Trixie the +# canonical way to change the port is a drop-in under +# /etc/ssh/sshd_config.d/*.conf (cloud-init, Ansible, raspi-config and +# manual admins all use this; the shipped sshd_config carries only an +# `Include` line and no Port). A naive `awk` over /etc/ssh/sshd_config +# alone misses that and returns 22 — which would make ufw_firewall open +# the wrong port and drop the operator's live session (remote lockout), +# and point fail2ban's jail at the wrong port. +# +# We ask sshd itself for its merged config view (`sshd -T`), which +# honours every Include'd drop-in, then fall back to scanning the main +# file and any drop-ins directly (for environments without the sshd +# binary, e.g. CI), and finally to 22. Prints a single port number. +pi_sshd_effective_port() { + local port="" + if command -v sshd >/dev/null 2>&1; then + # `sshd -T` prints the fully-merged effective config with lowercased + # keys; the first `port N` line is the primary listen port. It exits + # non-zero on a fatal config error, in which case awk yields nothing + # and we fall through to the file scan. + port=$(sshd -T 2>/dev/null | awk '/^port[[:space:]]+[0-9]+/{print $2; exit}') + fi + if [[ -z $port && -r /etc/ssh/sshd_config ]]; then + port=$(awk '/^[[:space:]]*[Pp]ort[[:space:]]+[0-9]+/{print $2; exit}' /etc/ssh/sshd_config 2>/dev/null) + fi + if [[ -z $port ]]; then + local dropin + for dropin in /etc/ssh/sshd_config.d/*.conf; do + [[ -r $dropin ]] || continue + port=$(awk '/^[[:space:]]*[Pp]ort[[:space:]]+[0-9]+/{print $2; exit}' "$dropin" 2>/dev/null) + [[ -n $port ]] && break + done + fi + printf '%s\n' "${port:-22}" +} + # Ensure sshd_config directive is set to the specified value. update_sshd_config_option() { local key=$1 diff --git a/lib/util/systemd.sh b/lib/util/systemd.sh index 8d26175..f85163f 100644 --- a/lib/util/systemd.sh +++ b/lib/util/systemd.sh @@ -3,10 +3,30 @@ # # Functions: unit_exists, unit_disable_now, unit_mask, remount_path, # pi_mark_daemon_reload_needed, pi_daemon_reload_if_needed, -# pi_daemon_reload_now +# pi_daemon_reload_now, write_systemd_unit # Globals (read/write): PI_DAEMON_RELOAD_PENDING # ====================================================================== +# Write a systemd unit / drop-in (body on stdin) the safe way, replacing +# the `record_created; cat > file; chmod 0644` pattern that was copied — +# with subtle variations — across the unit-writing tasks. Does, in order: +# 1. record_created — so --undo can remove (or restore) the file; +# 2. _pi_atomic_write — stage .tmp + fsync + os.replace, so a power +# loss mid-write can never leave systemd an unparseable unit; +# 3. chmod 0644 — units must be world-readable, not umask-derived; +# 4. pi_mark_daemon_reload_needed — batch the reload to end of run. +# A task that must `systemctl enable --now` the unit within the SAME run +# should call pi_daemon_reload_now afterwards (it reloads immediately and +# clears the pending flag, avoiding a duplicate end-of-run reload). +# Returns non-zero only if the atomic write fails. +write_systemd_unit() { + local path=$1 + record_created "$path" + _pi_atomic_write "$path" || return 1 + chmod 0644 "$path" 2>/dev/null || true + pi_mark_daemon_reload_needed +} + # Mark that a task modified a systemd unit and a daemon-reload is needed. # Tasks should call this instead of `systemctl daemon-reload` directly so # the reload is batched to a single invocation at end of run. A fresh diff --git a/lib/util/validate.sh b/lib/util/validate.sh index 9995c5f..d8655be 100644 --- a/lib/util/validate.sh +++ b/lib/util/validate.sh @@ -1,9 +1,13 @@ # ====================================================================== # lib/util/validate.sh — small validators and sanity checks # -# Functions: require_root, validate_hostname, validate_timezone, +# Functions: require_root, validate_hostname, # validate_https_url, arch_sanity_banner # Globals (read): SYSTEM_ARCH, SYSTEM_PI_GEN +# +# Timezone values are validated by validate_timezone_name (strict regex) +# plus the explicit zoneinfo existence check in run_timezone — do not add +# a weaker file-exists-only validator back here. # ====================================================================== # Abort execution unless running as root. @@ -19,11 +23,6 @@ validate_hostname() { [[ $1 =~ ^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?$ ]] } -# Accept any zone that ships under /usr/share/zoneinfo. -validate_timezone() { - [[ -f "/usr/share/zoneinfo/$1" ]] -} - # https:// URL validation for key-import URLs. Enforces: # - scheme must be exactly `https://` (rejects http/file/javascript:) # - no whitespace, CR/LF, backticks, quotes, or shell metacharacters diff --git a/pi-optimiser.sh b/pi-optimiser.sh index 3ede7e5..ebd3640 100755 --- a/pi-optimiser.sh +++ b/pi-optimiser.sh @@ -3,7 +3,7 @@ # Coded by Adrian Jon Kriel :: admin@extremeshok.com # Project home: https://github.com/extremeshok/pi-optimiser # ====================================================================== -# pi-optimiser.sh :: version 9.4.5 +# pi-optimiser.sh :: version 9.5.0 #====================================================================== # One-shot optimiser for Raspberry Pi OS desktops. Key capabilities: # - Removes bundled bloatware and trims apt caches for a lean install @@ -35,7 +35,7 @@ fi SCRIPT_NAME=$(basename "$0") readonly SCRIPT_NAME -SCRIPT_VERSION="9.4.5" +SCRIPT_VERSION="9.5.0" readonly SCRIPT_VERSION # Globals consumed by sourced lib/util/*.sh modules; shellcheck cannot @@ -633,11 +633,16 @@ apply_once() { if [[ $_value_changed -eq 0 ]] && is_task_done "$task"; then log_info "Re-running $task: requested value differs from current system state" fi - if [[ $FORCE -eq 1 && $DRY_RUN -eq 0 ]]; then - # Under --dry-run, preserving the existing state is essential — - # the whole point is that no side effects happen. We still honour - # --force enough to let the task "run" (below) so dry-run output - # shows what would happen, but we don't mutate state.json. + if [[ $FORCE -eq 1 && $DRY_RUN -eq 0 ]] && pi_task_gate_active "$task"; then + # --force re-runs a task, so drop its completion marker to bypass the + # is_task_done short-circuit. Guards: + # - $DRY_RUN -eq 0: --dry-run must not mutate state.json. + # - pi_task_gate_active: only clear a task that will ACTUALLY run. + # Without this, a blanket `--force` (no opt-in flags) wiped the + # completion marker of every gated task — docker, zram, + # node_exporter, ... — because the runner self-skips (return 2) + # and the rc==2 path never re-records state, leaving --status / + # --report / metrics showing installed tasks as never-run. clear_task_state "$task" fi log_info "Running task $task: $desc" @@ -1328,26 +1333,43 @@ main() { # flag is explicitly passed). # shellcheck disable=SC2034 # read by lib/ui/tui.sh::pi_tui_should_launch PI_ARGV_RAW=" $* " - # Flags that clearly mean "don't show the menu." + # Flags that clearly mean "don't show the menu." Global/mode flags are + # listed here; every TASK flag is appended from the task registry just + # below so this set can never drift out of sync with the tasks again + # (it previously omitted --wifi-powersave-off, --zram-algo, --temp-limit + # and others, so passing them on an interactive TTY wrongly launched the + # whiptail menu instead of running the requested task). # shellcheck disable=SC2034 PI_CLI_ACTION_FLAGS=( - --force --dry-run --status --list-tasks --report --snapshot --restore + --force --dry-run --diff --status --list-tasks --list-profiles --report + --validate-config --self-test --snapshot --restore --undo --update --check-update --enable-update-timer --disable-update-timer --uninstall --migrate --rollback --only --skip - --install-tailscale --install-docker --install-zram --install-wireguard - --install-node-exporter --install-smartmontools --install-cli-modern - --install-net-diag --enable-dns-cache - --overclock-conservative --underclock --pi5-fan-profile --pcie-gen3 - --enable-watchdog --secure-ssh --firmware-update --eeprom-update - --install-firewall --power-off-halt --nvme-tune --quiet-boot - --disable-leds --install-pi-connect --remove-cups - --headless-gpu-mem --install-chrony --disable-ipv6 - --usb-uas-quirks --usb-uas-extra --install-hailo - --ssh-import-github --ssh-import-url --hostname --timezone --locale --proxy-backend --profile --config ) + # Append every flag declared in a task's `flags=` metadata (and the + # value-companion flags that gate the same tasks). PI_TASK_FLAGS is the + # single source of truth populated by pi_task_register. + local _tid _tflag + local -a _task_flag_tokens=() + for _tid in "${PI_TASK_ORDER[@]}"; do + [[ -n "${PI_TASK_FLAGS[$_tid]:-}" ]] || continue + IFS=',' read -ra _task_flag_tokens <<< "${PI_TASK_FLAGS[$_tid]}" + for _tflag in "${_task_flag_tokens[@]}"; do + _tflag=${_tflag//[[:space:]]/} + [[ -n "$_tflag" ]] && PI_CLI_ACTION_FLAGS+=("$_tflag") + done + done + # Value-companion flags that tune a task but aren't its gate flag, so + # they don't appear in PI_TASK_FLAGS. Adding them keeps the TUI gate + # correct when only one of these is passed. + PI_CLI_ACTION_FLAGS+=( + --usb-uas-extra --zram-algo --temp-limit --temp-soft-limit + --initial-turbo --docker-buildx-multiarch --docker-cgroupv2 + --wifi-powersave-off --disable-bluetooth + ) # Pre-scan argv for flags that change config-load decisions. We need # to know these BEFORE parse_args so the saved config loads first @@ -1623,33 +1645,12 @@ main() { SUMMARY_SKIPPED+=("$reason") continue fi - # Proxy special case: if the backend URL changed since last run, - # drop the completion marker so the task re-configures nginx. - if [[ "$task" == "proxy" && -n "$PROXY_BACKEND" ]]; then - local current_backend - if current_backend=$(get_stored_proxy_backend); then - if [[ "$current_backend" != "$PROXY_BACKEND" ]]; then - log_info "Proxy backend changed from '$current_backend' to '$PROXY_BACKEND'; re-running" - clear_task_state "$task" - fi - else - clear_task_state "$task" - fi - fi - # ufw_firewall special case: reconcile when the set of things it - # opens ports for (VPN interfaces, proxy symlink, SSH port) has - # changed since the last run. Without this the firewall can - # silently drift when a VPN is added or removed later. - if [[ "$task" == "ufw_firewall" && ${INSTALL_FIREWALL:-0} -eq 1 ]] \ - && declare -F _ufw_fingerprint >/dev/null 2>&1; then - local current_fp stored_fp - current_fp=$(_ufw_fingerprint) - stored_fp=$(read_json_field "$CONFIG_OPTIMISER_STATE" "firewall.fingerprint" 2>/dev/null || echo "") - if [[ -n "$stored_fp" && "$stored_fp" != "$current_fp" ]]; then - log_info "Firewall inputs changed ($stored_fp -> $current_fp); re-reconciling" - clear_task_state "$task" - fi - fi + # Proxy backend changes and ufw_firewall input changes are detected + # by the tasks' own pi__value_changed hooks (proxy.sh / + # ufw_firewall.sh), which apply_once consults to re-run a completed + # task WITHOUT mutating state.json. That keeps --dry-run and --diff + # side-effect free; the old hard-coded clear_task_state blocks here + # leaked state writes into both preview modes. # apply_once returns non-zero on fatal task failure. Under `set -e` # that kills the loop immediately and skips every remaining task. # Capture the rc, keep going, and surface the failure at the end. diff --git a/scripts/build-bundle.sh b/scripts/build-bundle.sh index ee9b5e5..a2bf384 100755 --- a/scripts/build-bundle.sh +++ b/scripts/build-bundle.sh @@ -315,10 +315,13 @@ fi # 3. Shebang + strict-mode parity with the main script. A reordering # refactor that drops `set -euo pipefail` or `umask 0022` from the -# pre-source block would silently ship a less-safe bundle. Compare -# the first four non-blank lines of each. +# pre-source block would silently ship a less-safe bundle. Compare the +# shebang (line 1) PLUS the first three non-comment, non-blank lines. +# The previous filter (`!/^#/`) skipped the shebang entirely — so the +# "shebang parity" check never actually compared the shebang, the one +# line it is named for. _first_strict() { - awk 'NF && !/^#/ && !/^$/ {print; n++; if (n==3) exit}' "$1" + awk 'NR==1 {print; next} NF && !/^#/ {print; n++; if (n==3) exit}' "$1" } if ! diff -u <(_first_strict "$main") <(_first_strict "$tmp") >/dev/null; then echo "build-bundle.sh: bundle shebang/strict-mode header diverged from main script:" >&2 diff --git a/share/man/pi-optimiser.8.md b/share/man/pi-optimiser.8.md index 35ea507..28e1e78 100644 --- a/share/man/pi-optimiser.8.md +++ b/share/man/pi-optimiser.8.md @@ -2,8 +2,8 @@ title: PI-OPTIMISER section: 8 header: System Manager's Manual -footer: pi-optimiser 9.4.5 -date: 2026-04 +footer: pi-optimiser 9.5.0 +date: 2026-06 --- # NAME diff --git a/tests/docker/Dockerfile b/tests/docker/Dockerfile index 2eb9d87..a874b20 100644 --- a/tests/docker/Dockerfile +++ b/tests/docker/Dockerfile @@ -7,7 +7,17 @@ # # Tests assume --dry-run / --diff / --self-test semantics — no # destructive commands should run inside the container. -FROM debian:trixie-slim +# Base image is parameterised so CI can run the harness against BOTH +# supported targets (Debian 13 "trixie" and Debian 12 "bookworm") via a +# --build-arg matrix, instead of only ever testing trixie. +ARG BASE_IMAGE=debian:trixie-slim +FROM ${BASE_IMAGE} + +# Where the Pi-binary stubs record their invocations. Exported so the +# test harness can assert a task actually reached a stub (rather than +# trivially passing by skipping); without this the stubs' logging was +# dead — nothing ever set or read it. +ENV PI_OPTIMISER_STUB_LOG=/tmp/pi-optimiser-stub.log RUN apt-get update \ && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ diff --git a/tests/docker/run-tests.sh b/tests/docker/run-tests.sh index aa0c56b..c9accc0 100755 --- a/tests/docker/run-tests.sh +++ b/tests/docker/run-tests.sh @@ -1759,4 +1759,22 @@ bundle_policy_scan=$(grep -InE "$generic_pat|$service_pat|$footer_pat" "$TEST_TM || fail "generated bundle contains prohibited references" "$bundle_policy_scan" pass "bundle (9.3 flags present)" +step "stub log: preflight actually reaches the Pi-binary stubs" +# The stubs log every invocation to $PI_OPTIMISER_STUB_LOG. Asserting the +# log is populated proves a code path genuinely exercised vcgencmd (rather +# than trivially passing by skipping). Guarded on the stub being our +# logging stub so the test is a no-op outside the Docker image. +if command -v vcgencmd >/dev/null 2>&1 && grep -q PI_OPTIMISER_STUB_LOG "$(command -v vcgencmd)" 2>/dev/null; then + stub_log="$TEST_TMP/stub.log" + : > "$stub_log" + PI_OPTIMISER_STUB_LOG="$stub_log" "$BIN" --report >/dev/null 2>&1 || true + if [[ -s "$stub_log" ]] && grep -q '^vcgencmd' "$stub_log"; then + pass "stub log captured vcgencmd invocations from preflight" + else + fail "stub log empty after --report — preflight never reached vcgencmd" "$(cat "$stub_log" 2>/dev/null)" + fi +else + pass "stub log (skipped: no logging vcgencmd stub on PATH)" +fi + printf '\nAll integration checks passed.\n'