From 5c8876428b569da508fff7e979e31a3873ee4426 Mon Sep 17 00:00:00 2001 From: librelad Date: Thu, 27 Aug 2026 12:22:18 +0100 Subject: [PATCH] test: pin the CLI argv path, and document the third silent truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/dev/lp-cli-argv-test builds stubs from the real invocation line in init.sh and the real LP_CLI_ARGS line in start.sh, then pushes thirteen app names through them — so editing either file is what makes it fail. Verified against both regressions: dropping "$@" from the wrapper, and reading "$@" instead of "${@:10}" in start.sh. Co-Authored-By: Claude Opus 5 --- docs/roadmap/first-run-restore.md | 34 ++++++++++++ scripts/dev/lp-cli-argv-test | 86 +++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100755 scripts/dev/lp-cli-argv-test diff --git a/docs/roadmap/first-run-restore.md b/docs/roadmap/first-run-restore.md index 7251e85..10489c0 100644 --- a/docs/roadmap/first-run-restore.md +++ b/docs/roadmap/first-run-restore.md @@ -110,6 +110,40 @@ cases: an app too big for the disk must be refused, an app this version no longer ships must be refused, an app whose location is gone must be marked as moved. Reintroducing either historical bug fails it. +### 3.2 — And a third: four apps of thirteen, reported as success + +With the preflight finally reading manifests, the run got as far as restoring — +and restored 4 of 13, printing *"First-run restore complete — 4 apps restored"* +with exit 0. + +The app list crosses two chokepoints, and fixing the first had hidden the +second: + +1. the CLI dispatcher calls handlers with **no** arguments, so `"$@"` and + `shift` inside one operate on an empty list. `LP_CLI_ARGS` was added for this. +2. but `LP_CLI_ARGS` was built from `start.sh`'s `"$@"` — and the root wrapper + invokes `start.sh` with exactly **nine hardcoded positional slots**. The + array could never hold more than nine entries, so `${LP_CLI_ARGS[@]:5}` gave + at most four app names. + +Same shape as §3.1 once more: the truncation had no failure path. Four apps +restored perfectly, and the success line counted the list it was handed. + +Three changes, because one would not have been enough: + +- the wrapper forwards the real argv after the nine slots (which stay, since + every dispatcher reads them and unset ones must keep arriving as `"empty"`); + `start.sh` reads it back as `"${@:10}"`. `footprint_version` 7 → 8. +- `restoreFirstRunBulk` with no list is a **whole-host restore**: it discovers + the host's apps and re-applies the preflight itself, because the installer's + report ran in a different process and its decision was otherwise lost — a + skipped app would have been restored anyway. The installer now passes no + list, so there is nothing to truncate. +- it counts what actually landed and returns non-zero naming the failures. + +`scripts/dev/lp-cli-argv-test` builds stubs from the real lines in `init.sh` and +`start.sh` and pushes thirteen app names through them. + ## 4. The password problem, stated plainly **An encrypted repository cannot be opened with anything inside itself.** `CFG_BACKUP_LOC__PASSWORD` lives in the system config — which is *inside the backup*. So on a fresh machine the user must supply the repository password by hand. There is no way around this and it is not a bug; it is what encryption means. diff --git a/scripts/dev/lp-cli-argv-test b/scripts/dev/lp-cli-argv-test new file mode 100755 index 0000000..03c3584 --- /dev/null +++ b/scripts/dev/lp-cli-argv-test @@ -0,0 +1,86 @@ +#!/bin/bash +# Does an open-ended list of app names survive the trip from the command line +# into a CLI handler? Runs in $TMPDIR against stubs built from the REAL lines in +# init.sh and start.sh, so editing either is what makes this fail. +# +# scripts/dev/lp-cli-argv-test +# +# The path has two independent chokepoints, and fixing one hid the other: +# +# 1. the root wrapper (baked from init.sh) invokes start.sh with exactly nine +# hardcoded positional slots, unset ones filled with the literal "empty" +# 2. the CLI dispatcher calls its handlers with NO arguments, so "$@" and +# `shift` inside a handler operate on an empty list +# +# LP_CLI_ARGS was added for (2) but built from start.sh's "$@" — which (1) had +# already capped at nine. `restore first-run bulk` reads "${LP_CLI_ARGS[@]:5}", +# so a 13-app restore arrived as four, restored four, and reported success. +# +# Thirteen is the fixture size for a reason: it is what a real host had. + +REPO="$(cd "$(dirname "$0")/../.." && pwd)" +BASE="$(mktemp -d "${TMPDIR:-/tmp}/lp-cli-argv-test-XXXXXX")" +trap 'rm -rf "$BASE"' EXIT + +fail=0 +chk(){ if [[ "$2" == "$3" ]]; then echo " ok $1"; else echo " FAIL $1: got '$2' want '$3'"; fail=1; fi; } + +# --- the two real lines under test ------------------------------------------- +invoke=$(grep -E '^\s*\./start\.sh "\$command1"' "$REPO/init.sh" | head -1) +capture=$(grep -E '^declare -a LP_CLI_ARGS=' "$REPO/start.sh" | head -1) +[[ -n "$invoke" ]] || { echo " FAIL could not find the start.sh invocation in init.sh"; exit 1; } +[[ -n "$capture" ]] || { echo " FAIL could not find LP_CLI_ARGS in start.sh"; exit 1; } + +# --- stub start.sh: the real capture line, then report what a handler sees ---- +{ + echo '#!/bin/bash' + echo "$capture" + # Offsets as the real handlers use them: :5 for `restore first-run bulk` + # (restore first-run bulk app…), :4 for `restore preflight`. + echo 'bulk=(); for a in "${LP_CLI_ARGS[@]:5}"; do [[ -z "$a" || "$a" == "empty" ]] && continue; bulk+=("$a"); done' + echo 'pref=(); for a in "${LP_CLI_ARGS[@]:4}"; do [[ -z "$a" || "$a" == "empty" ]] && continue; pref+=("$a"); done' + echo 'echo "BULK=${#bulk[@]}:${bulk[*]}"' + echo 'echo "PREF=${#pref[@]}"' + echo 'echo "SLOT1=$1 SLOT5=$5 SLOT9=$9"' +} > "$BASE/start.sh" +chmod +x "$BASE/start.sh" + +# --- stub wrapper: the nine slots exactly as the real one builds them --------- +{ + echo '#!/bin/bash' + for i in $(seq 1 9); do echo "command$i=\"\${$i:-empty}\""; done + echo 'script_dir="$(dirname "$0")"' + echo 'cd "$script_dir"' + echo "$invoke" +} > "$BASE/wrapper" +chmod +x "$BASE/wrapper" + +APPS=(bookstack grafana matrix mattermost navidrome nextcloud prometheus + rocketchat speedtest stalwart stoat trivy vikunja) + +echo "--- 13 app names through 'restore first-run bulk' ---" +out=$("$BASE/wrapper" restore first-run bulk 2 QuantumOtter "${APPS[@]}") +bulk=$(sed -n 's/^BULK=\([0-9]*\):.*/\1/p' <<< "$out") +names=$(sed -n 's/^BULK=[0-9]*://p' <<< "$out") +chk "count" "$bulk" "13" +chk "first app" "${names%% *}" "bookstack" +chk "last app" "${names##* }" "vikunja" + +echo "--- the nine legacy slots still behave as before ---" +# argv here is: restore(1) first-run(2) bulk(3) 2(4) QuantumOtter(5) +# bookstack(6) grafana(7) matrix(8) mattermost(9) — so slot 9 is the fourth app, +# which is exactly the cap that made a 13-app restore into a 4-app one. +slots=$(sed -n 's/^SLOT1=//p' <<< "$out") +chk "slots" "$slots" "restore SLOT5=QuantumOtter SLOT9=mattermost" + +echo "--- unset slots still arrive as the literal \"empty\", and are dropped ---" +out=$("$BASE/wrapper" restore preflight 2 QuantumOtter) +chk "no phantom apps" "$(sed -n 's/^PREF=//p' <<< "$out")" "0" + +echo "--- an explicit six, which used to be capped at five ---" +out=$("$BASE/wrapper" restore preflight 2 QuantumOtter "${APPS[@]:0:6}") +chk "count" "$(sed -n 's/^PREF=//p' <<< "$out")" "6" + +echo "" +if (( fail )); then echo "FAILED"; exit 1; fi +echo "All argv checks passed."