From 2f2fe8bd68dca1d72a155ce8dee6f0e201cc2037 Mon Sep 17 00:00:00 2001 From: Ankush Kapoor <50513804+kapoorankush@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:35:13 -0500 Subject: [PATCH 1/3] feat: port the post-v0.228.0 development train MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports litclock-dev afa2d6b4..fde50fc6 — the seven PRs merged 2026-09-19/20 (litclock-dev#872-#878). The boundary was established by CONTENT, not by the previous port commit's stated range: public carries litclock-dev#865's blocked-sha and none of litclock-dev#868/#870/#871, and the file sizes agree (update.sh 2542 vs 2739, state.sh 317 vs 416, quote_corpus.py 249 vs 368). Gated on the bench QA pass of 2026-09-20 (`dev-20260920-512c291`, a fresh flash on a Pi Zero 2 W): 13 checks, no blocking findings, every member of this train exercised on hardware. What owners get (one line in the CHANGELOG, because that is all that is visible from outside): - gift mode and the app's Factory reset now wipe AND LOCK the default shell history files, as clone prep already did (litclock-dev#868). The lock is a root-owned empty directory at the path: `history -c` reaches only the script's own shell, and the console or SSH shell the operator ran it from writes its history back on exit, during the power-off. first-boot.sh removes both directories on the not-yet-set-up path, so the recipient's first login gets an ordinary history file. Shipping silently, by design: - the quote corpus is resolved per active language from the registry (litclock-dev#870). Latent while English is the sole active language: the pre-rendered PNGs stay English, so a second language needs on-device text rendering before it can be activated. - the updater's runtime-render self-test, shipped INERT (litclock-dev#871 Stage A). On a marker-bearing device an applied release now asks whether the device can actually paint a quote from text and records the verdict in /var/lib/litclock/runtime-render-selftest.json; it changes no setting, and a failure is not an update failure. First field measurement: 3.8s on a Pi Zero 2 W during an update, 3.4-3.7s idle — whole-process wall time, not render time. Also: the LKG recorder no longer claims bootcheck/revert is unshipped (litclock-dev#877), the WiFi retry tests stub the IP-geo resolver that was making a live network call (litclock-dev#876), and CLAUDE.md gains the 2026-09-20 bench results plus the rewritten litclock-dev#867 raw-journal check (the old one passed vacuously). Port mechanics: - 48 bare `#NNN` refs and 14 `dev#NNN` shorthands requalified to `litclock-dev#NNN` on ported lines only, validated by this repo's own tests/test_issue_ref_namespace.py rather than by a hand regex. - Hand-merged where public has diverged: README (its own structure and headings), docs/script-reference.md (public's `#resetting` anchor), scripts/lib/state.sh (public's already-qualified env.sh writer-lock header), and the two comment conflicts in update.sh and status.py. - CONTRIBUTING.md edited in place to preserve its CRLF line endings. - Scrubbed on the way over: the `~/archives/litclock-qa` harness path, which does not exist for a reader of this repo. ruff clean, shellcheck clean, pytest 4638 passed / 66 skipped, vitest 209 passed. --- .github/workflows/render-invariants.yml | 4 + CHANGELOG.md | 4 + CLAUDE.md | 82 ++++++- CONTRIBUTING.md | 7 + README.md | 4 +- docs/recovery.md | 4 +- docs/script-reference.md | 2 +- scripts/first-boot.sh | 13 +- scripts/lib/state.sh | 99 ++++++++ scripts/litclock-lkg-record.sh | 10 +- scripts/prepare-for-cloning.sh | 78 ++----- scripts/reset-setup.sh | 102 ++++++++- scripts/update.sh | 231 +++++++++++++++++-- src/control_server/routes/status.py | 15 +- src/literary_clock.py | 51 ++++- src/quote_corpus.py | 139 +++++++++++- src/quote_renderer.py | 7 +- src/strings_catalog.py | 28 ++- tests/state_sh_lifts.py | 38 ++++ tests/test_control_server.py | 18 ++ tests/test_exit_without_finalization.py | 2 + tests/test_first_boot_flow.py | 2 + tests/test_literary_clock_dry_run.py | 122 ++++++++++ tests/test_prepare_for_cloning_sh.py | 70 +++++- tests/test_quote_corpus.py | 288 ++++++++++++++++++++++- tests/test_reset_setup_sh.py | 244 +++++++++++++++++++- tests/test_runtime_render_autostamp.py | 290 ++++++++++++++++++++++++ tests/test_strings_catalog.py | 7 + tests/test_timer_lead.py | 3 + tests/test_update_sh.py | 17 +- tests/test_wifi_retry_flow.py | 165 +++++++++++--- 31 files changed, 1959 insertions(+), 187 deletions(-) create mode 100644 tests/state_sh_lifts.py diff --git a/.github/workflows/render-invariants.yml b/.github/workflows/render-invariants.yml index 4ac5a64..f6fa63d 100644 --- a/.github/workflows/render-invariants.yml +++ b/.github/workflows/render-invariants.yml @@ -38,6 +38,10 @@ on: branches: [master] paths: - 'src/quote_renderer.py' + # quote_corpus owns the corpus walk (litclock-dev#590) and, since litclock-dev#870, + # decides WHICH corpus the renderer reads from languages.json + - 'src/quote_corpus.py' + - 'languages.json' - 'src/gd_measure.py' # render_invariants derives notch geometry from literary_clock's QR # constants — a geometry-only change must re-run this gate diff --git a/CHANGELOG.md b/CHANGELOG.md index 373df8a..af0b818 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to LitClock are documented here. Format loosely follows [Kee ## [Unreleased] +### Fixed + +- Preparing a clock to hand on — gift mode, or a factory reset from the app — now clears the shell history as well, so a previous owner's typed commands do not travel with the device. Preparing an SD card for cloning already did this. + ## [v0.228.0] - 2026-09-18 ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index ebcf79e..f82485c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,9 +51,9 @@ The first-boot flow (`scripts/first-boot.sh`) provisions WiFi via a web UI; ever - **WiFi-only hotspot form**: Verify the setup page shows ONLY the WiFi network picker + password field + Submit button (plus, on a multi-language fleet only, the Language select — dormant while English is the sole active registry language, litclock-dev#532). No Location, Timezone, Temperature, or Mature-content sections — those are PWA-only post-handoff. The hidden-network "Network name" box is NOT visible at load: since litclock-dev#848 it appears only after picking "My network isn't listed" in the dropdown (and is focused then — not at load), and picking a real network afterwards hides it again AND blanks anything typed. Two exceptions arrive with the option pre-selected and the box showing: a retry that echoes a hand-typed name, and an empty scan. Also check **Refresh**: with the manual option selected and a name typed, tap Refresh — the option must still be selected and the name still there when the list lands; and separately, tap Refresh from the placeholder and pick "My network isn't listed" DURING the scan (it runs 2-20s) — the box must NOT vanish when the response arrives. With JavaScript OFF the disclosure is present in every state, as it was before — but on a clean render it is CLOSED, so only its summary line shows and the box needs one tap to reveal; the retry-echo and empty-scan renders arrive open. That is the no-JS fallback, and the server's rule (a picked network wins over typed text) is the only guard there. - **Hotspot creation**: Power on with no known WiFi networks. Verify the Pi creates a hotspot and displays credentials + QR code on the e-ink screen. -- **litclock-dev#620 hotspot-password block — RUN IN ORDER, and only on a device you can re-flash.** Checks 1-4 are sequential and destructive: check 2 needs the phone state check 1 leaves behind, check 3 destroys the password both depend on, and check 4 needs its own fresh flash. Out of order they need a re-flash to redo. Check 5 is order-independent — it only reads a support bundle, so run it any time after provisioning. (These steps describe the litclock-dev#620 feature: if that PR has not merged, none of the paths below exist yet.) +- **litclock-dev#620 hotspot-password block — RUN IN ORDER, and only on a device you can re-flash.** Checks 1-4 are sequential and destructive: check 2 needs the phone state check 1 leaves behind, check 3 destroys the password both depend on, and check 4 needs its own fresh flash. Out of order they need a re-flash to redo. Check 5 needs a shell AND the setup-hotspot path — it reads `/var/lib/litclock/hotspot-password`, which exists only when the setup network was raised — so run it right after check 1's first cycle, before check 3 rotates the key and check 6's SSH-off takes your shell. (These steps describe the litclock-dev#620 feature: if that PR has not merged, none of the paths below exist yet.) 1. **The password is STABLE across cycles** — the core invariant, invisible without hardware. Provision, note the password on the e-ink, then re-enter setup with `sudo systemctl start --no-block litclock-wifi-reset.service`. **Do NOT run `litclock-wifi-reset.sh` foregrounded over SSH**: it deletes every WiFi profile before it clears `.setup-complete`, so your own connection dies mid-script and SIGHUP kills it before it finishes — leaving a device with no WiFi, no hotspot, and the look of a brick. Verify the panel shows **the same password**, then `sudo stat -c '%a %U:%G' /var/lib/litclock/hotspot-password` returns exactly `600 pi:pi`. Ownership matters as much as the mode: `litclock-firstboot.service` is `User=pi`, so a `600 root:root` file (what a maintainer running the CLI under sudo leaves behind) is unreadable to the real writer and silently rotates the password every cycle — the exact bug this feature removes. Both verification commands need a shell, and the reset you just triggered deleted the WiFi profile your SSH session was riding on — **reconnect over the `LitClock-Setup` hotspot and SSH to its gateway IP** (or run this step on an Ethernet-attached rig) before expecting them to work. If cycle 2 differs, get the cause for free with `journalctl -u litclock-firstboot | grep -iE 'hotspot.password'`: the code emits distinguishable lines for unreadable ("minting a REPLACEMENT"), invalid ("regenerating"), unwritable ("this cycle only") and race ("adopting the stored value"). Use that regex, not `'hotspot password'` — the race line is the one message that spells it `hotspot-password` with a hyphen, so a literal-space grep silently hides the single case this list exists to diagnose. - 2. **The phone actually rejoins** — the user-visible payoff. Using a phone that joined during check 1's FIRST cycle (do NOT forget the network), tap `LitClock-Setup`. **The pass condition is that the setup page loads** — captive sheet rises, or the gateway IP serves the WiFi picker — **and neither phone asks you for a password.** Test both platforms. + 2. **The phone actually rejoins** — the user-visible payoff. Using a phone that joined during check 1's FIRST cycle (do NOT forget the network), tap `LitClock-Setup`. **The pass condition is that the setup page LOADS** — captive sheet rises, or the gateway IP serves the WiFi picker. **"It did not ask for a password" is NOT evidence on its own** (litclock-dev#878 review, from the 2026-09-20 row below): on iOS 27.0 a stale-credential FAILURE from Control Centre also asks for nothing — no prompt, no banner, it just reverts to searching. So the absence of a password prompt is consistent with both outcomes there, and only the page actually loading separates them. On Android the absence still carries information, because a stale credential produces an explicit failure string. Test both platforms. - **Judging the result.** Do not use "No Internet Access" to judge a *success*: Android shows that for any AP without an uplink, so it is also what a successful join to a captive setup network looks like. On a genuinely stale credential, current Android is unambiguous — expect an explicit **"Connection failed — Wrong password for `LitClock-Setup`"**. That string is a clean fail signal, so treat its absence, plus the setup page loading, as the pass. - **Both measurements, stamped.** These disagree, and the disagreement is the point — record device + OS + date for anything you add here, because Android's WiFi error surfaces have changed materially across releases and an undated claim cannot be told apart from drift. @@ -62,6 +62,7 @@ The first-boot flow (`scripts/first-boot.sh`) provisions WiFi via a web UI; ever | ~2026-08-10 (litclock-dev#620) | OnePlus 6T | *not recorded* | "No Internet Access"; no password field offered; **a QR scan did not override the saved entry** (the phone did not register an attempt) | | 2026-08-15 (bench, `dev-20260815-b0c0590`) | *not recorded* | *not recorded* | "Connection failed — Wrong password for `LitClock-Setup`"; **offered "Change password"**; **a QR scan connected on the first try**, captive portal followed, provisioning completed normally | | 2026-09-18 (bench, `dev-20260918-787d307`) | iPhone | **iOS 27** | **QR scan joined first try** — camera scan raised a "join LitClock-Setup?" prompt, tapped Join, ~10s, captive portal opened by itself. No password prompt, no failure banner. First iOS measurement; the stale credential was two rotations old (a gift-mode prep and a PWA factory reset had each re-minted the key that same afternoon). **Do not record key values in this file** — it is published; the rotation COUNT is the measurement, the keys are a credential | + | 2026-09-20 (bench, `dev-20260920-512c291`) | iPhone 16 Pro Max | **iOS 27.0** | **a plain tap does NOT join** — and the failure is SILENT from Control Centre: tapping `LitClock-Setup` there shows no error at all, then reverts to searching the moment you tap away. From **Settings** the same tap prompts for the password, which the owner can type to recover. So recovery on iOS exists but is not where a recipient taps first. Stale by ONE rotation (a PWA factory reset minted the key minutes earlier). The QR path was not re-run on this build | The 2026-08-15 run was on a device whose hotspot password had just been rotated by `--gift-mode` — functionally the same condition the original describes. All three of the original's Android claims failed to reproduce. That the device and OS columns are half empty is itself the finding: fill them in next time. @@ -70,9 +71,20 @@ The first-boot flow (`scripts/first-boot.sh`) provisions WiFi via a web UI; ever disagree with each other, so a third undated row would have been unattributable to either a platform difference or OS drift. iOS 27 simply joined — no prompt, no banner, no recovery needed. It was a QR scan, so it - settles the QR-override sub-check below on iOS. The inverse is now the - unrecorded one: whether a plain tap on `LitClock-Setup` (no QR) also joins - past a stale credential on iOS. + settles the QR-override sub-check below on iOS. **The inverse was measured + on 2026-09-20 and the answer is no**: a plain tap does not join. What makes + that row worth reading twice is WHERE it fails. Control Centre reports + nothing — no prompt, no error, it simply goes back to searching — while + Settings offers the password field that recovers it. Any judgement about + whether a recipient can recover has to name which of the two they used. + + **Do not read the OS off the captive-portal user agent.** On that same + iOS 27.0 handset the probe logged `CPU iPhone OS 18_7 like Mac OS X`, + because Apple freezes the UA in the captive WebView. The 2026-09-20 handset was + checked in Settings -> General -> About and reads 27.0, which is what + that row records; the 2026-09-18 row's method was not written down at + the time, so treat its version as consistent-with rather than + independently confirmed. **The owner reports iOS has behaved this way on earlier devices too** (2026-09-18, recollection rather than a dated run — deliberately NOT given @@ -80,11 +92,46 @@ The first-boot flow (`scripts/first-boot.sh`) provisions WiFi via a web UI; ever replace). It does change where re-test effort belongs: the two Android rows contradict each other on whether recovery is discoverable at all, while iOS appears consistent and always has. Treat the uncertainty as - **Android-side** — vary the device and the OS version there — and treat - iOS as settled unless it starts failing. + **Android-side** — vary the device and the OS version there. + + **"iOS is settled" was QR-path evidence only, and the 2026-09-20 row + narrows it** (litclock-dev#878 review). A QR scan joins; a plain tap does not, and + fails SILENTLY from Control Centre. Those are different interaction + paths, not contradictory measurements — but it means the platform is + settled for the recovery we document and unsettled for the one a + recipient reaches for first. Keep plain-tap coverage on iOS. - **Severity, corrected (litclock-dev#648).** The original block argued Android left *"no user-discoverable recovery short of Forget This Network, which the intended recipient will not find."* On the 2026-08-15 device recovery was one tap, and a QR scan bypassed the saved entry entirely. Treat the strong version as unproven rather than as established fact. **This does not weaken litclock-dev#620 itself**, which stays: a stable per-device password means a normal owner never reaches any of these screens, and that is the right outcome however gracefully a given Android build degrades. Only the severity narrative was stale. - **New sub-check: a QR scan overrides a stale saved entry.** Now the interesting case, and previously untested anywhere. Every LitClock broadcasts the same SSID with a per-device password, so a phone that set up clock A meets clock B with the wrong key — the recovery path a real recipient is most likely to stumble into. Scan the panel QR on a phone holding a stale credential for `LitClock-Setup` and confirm it joins. It worked on 2026-08-15; it did not on the original measurement, so this is worth re-running per device rather than assuming. **iOS 27, 2026-09-18: worked.** Camera scan raised a "join LitClock-Setup?" prompt; Join, ~10s, captive portal auto-opened — against a credential two rotations stale. The sub-check now stands at one PASS on iOS, one PASS and one FAIL on Android — the same Android-side split the rest of this block shows. Incidentally the panel says "wait about 20 seconds"; 10 sufficed, so that copy errs the right way. - 3. **Which resets keep the setup password and which rotate it — since litclock-dev#666 the DEFAULT rotates.** The old rule (wipe AND power-off) is gone; erasing both passwords is now what a bare reset does, and `--keep-wifi` is the only way to preserve either. Run the sub-checks in this order, because each destroys state the next would need. First **`--keep-wifi`** (the "same owner, moved house" case): `sudo cat /var/lib/litclock/hotspot-password`, keep the value, `sudo ./scripts/reset-setup.sh --keep-wifi --poweroff`, power back on, `sudo cat` again and confirm it is **UNCHANGED**. Read it from the file, not the panel — with the WiFi kept, the device boots straight onto its saved network and never raises a setup network, so there is no panel password on that path. Then a **bare `sudo ./scripts/reset-setup.sh --yes`** over SSH: expect the session to DROP when the WiFi goes (that is the documented behaviour, not a fault), power-cycle, and confirm the panel password is **DIFFERENT**. Since litclock-dev#833 there is a second thing to check on this path, and it needs a console, not a power-cycle: run the same bare reset from the console (or Ethernet), then `sudo reboot` — the panel must paint "Restarting…" (a `sudo poweroff` would paint "Powered Off") instead of carrying the stale quote across. A power-cycle fires no stop edge, so it cannot test this, and the SSH session a WiFi-wipe drops leaves you no shell to type the reboot from. The plain arm re-arms `litclock-shutdown.service` inside Step 1, right after its own stop consumed the edge, so this holds even when the reset aborts later or the WiFi wipe SIGHUPs it. Then the **PWA Factory reset** (`litclock-reset.service` runs `--wipe-wifi --strict-env-wipe --poweroff --yes`): record the password, trigger it from PWA → System, power on, confirm the panel password is **DIFFERENT** — this is the litclock-dev#660 path and the only one that proves it end to end. Also confirm the PWA's confirm modal and the in-progress screen both say the password will be new and that a phone holding the old one must forget the network. Finally `sudo ./scripts/reset-setup.sh --gift-mode`, power on, confirm the panel password is **DIFFERENT**, and that `--gift-mode --keep-wifi` is **REFUSED** in both orderings. Gift prep must abort loudly ("do NOT ship this device") rather than print "done" if the file cannot be removed; an ordinary failing reset must say "Reset FAILED" instead, not tell you to stop passing the device on. After this sub-check the device has no saved WiFi, so check 4 needs a re-provision or a fresh flash regardless. **Every sub-step here now ends your SSH session** (litclock-dev#657): `--keep-wifi --poweroff` is permitted and still takes the poweroff arm, so it disables SSH too — and that sub-step explicitly rules out the panel and requires a shell, with no hotspot to fall back on because the WiFi was kept. Check 6's advice ("run anything needing SSH before a reset check") cannot rescue this one, because the check IS a before/after comparison across a reset. Either run the whole block from the console, or restore access between sub-steps by putting a blank `ssh` file in the SD card's boot partition. + 3. **Which resets keep the setup password and which rotate it — since litclock-dev#666 the DEFAULT rotates.** The old rule (wipe AND power-off) is gone; erasing both passwords is now what a bare reset does, and `--keep-wifi` is the only way to preserve either. Run the sub-checks in this order, because each destroys state the next would need. First **`--keep-wifi`** (the "same owner, moved house" case): `sudo cat /var/lib/litclock/hotspot-password`, keep the value, `sudo ./scripts/reset-setup.sh --keep-wifi --poweroff`, power back on, `sudo cat` again and confirm it is **UNCHANGED**. Read it from the file, not the panel — with the WiFi kept, the device boots straight onto its saved network and never raises a setup network, so there is no panel password on that path. Then a **bare `sudo ./scripts/reset-setup.sh --yes`** over SSH: expect the session to DROP when the WiFi goes (that is the documented behaviour, not a fault), power-cycle, and confirm the panel password is **DIFFERENT**. Since litclock-dev#833 there is a second thing to check on this path, and it needs a console, not a power-cycle: run the same bare reset from the console (or Ethernet), then `sudo reboot` — the panel must paint "Restarting…" (a `sudo poweroff` would paint "Powered Off") instead of carrying the stale quote across. A power-cycle fires no stop edge, so it cannot test this, and the SSH session a WiFi-wipe drops leaves you no shell to type the reboot from. The plain arm re-arms `litclock-shutdown.service` inside Step 1, right after its own stop consumed the edge, so this holds even when the reset aborts later or the WiFi wipe SIGHUPs it. Then the **PWA Factory reset** (`litclock-reset.service` runs `--wipe-wifi --strict-env-wipe --poweroff --yes`): record the password, trigger it from PWA → System, power on, confirm the panel password is **DIFFERENT** — this is the litclock-dev#660 path and the only one that proves it end to end. Also confirm the PWA's confirm modal and the in-progress screen both say the password will be new and that a phone holding the old one must forget the network. **Both handoff arms now also wipe the shell history (litclock-dev#868):** before triggering the PWA reset, run one throwaway command in an INTERACTIVE shell (console or a normal SSH session). A scripted `ssh host 'cmd'` writes no history, and neither does `ssh -t host 'cmd'` — a pty alone does not make a shell interactive. A bare `ssh -t host`, which drops you at a prompt, DOES save on exit (litclock-dev#878 review corrected the broader claim). Confirm the marker actually landed with `tail -2 ~/.bash_history` before you reset, or you will be testing nothing, then exit that shell so it saves. **Do not try to inspect the lock by mounting the card** — that was the first version of this check and it is impractical from a Windows box and unnecessary. The device removes the lock on its own first boot, so by the time you can look it is already gone, and an absent history file looks identical whether the lock was placed or never placed at all. Read the journal instead, after the reset and the next boot (verified 2026-09-20): + + ```bash + MARKER=dev868-marker # whatever you echoed before the reset + sudo journalctl -b -1 | grep -E 'Clearing shell history before handoff\.\.\. .*(done|FAILED)' + sudo grep -rl "$MARKER" /home/pi /root --exclude-dir={venv,lib,images,.git} 2>/dev/null # nothing + sudo ls -ld /home/pi/.bash_history /root/.bash_history # both absent + ``` + + **The journal line is the load-bearing one, and the only one that can fail.** `clear_and_lock_bash_history` + returns 0 only when BOTH paths were emptied AND both `mkdir` locks were placed; anything else prints `FAILED` + and the red do-not-hand-this-on banner. It reaches the journal because `litclock-reset.service` sets + `StandardOutput=journal`, so it survives the power-off. Three details that each turn this back into a check + that cannot fail if you get them wrong (litclock-dev#878 review): **grep for `done|FAILED`, not for the prefix**, because + the prefix matches both outcomes; **bound it with `-b -1`**, the reset's own boot, because journald here is + `Storage=persistent` and an unbounded grep on a second run happily matches the FIRST run's line; and + **quote a real `$MARKER`**, because a literal `` pasted into a shell is a redirect that produces + empty output, which is exactly what the PASS looks like. + + **The `--gift-mode` repeat needs the PWA, not the CLI.** `sudo ./scripts/reset-setup.sh --gift-mode` writes + that line to your terminal and nowhere else, so there is no journal to read afterwards. Trigger gift prep from + the PWA (`litclock-prepare-for-gift.service`, which also sets `StandardOutput=journal`), or capture the + terminal output yourself before the device powers off. + + **Do NOT use the `rmdir` lines from first-boot as proof the lock existed** (litclock-dev#878 review). `first-boot.sh` + calls `sudo rmdir` unconditionally on both paths with errors suppressed, so the audit lines appear whether or + not a directory was ever there — "the lock was never placed" passes that check exactly as cleanly as + "the lock was placed and cleared". They corroborate the restore step running; they prove nothing about the + lock. The exclusions on the marker search matter too: a bare `grep -r /home` walks the 1.9GB library tree + and looks like a hang. Repeat for `--gift-mode`. A plain `--yes` reset or `--reboot` must leave the history file untouched; those hand control back to an operator, not to a new owner. Finally `sudo ./scripts/reset-setup.sh --gift-mode`, power on, confirm the panel password is **DIFFERENT**, and that `--gift-mode --keep-wifi` is **REFUSED** in both orderings. Gift prep must abort loudly ("do NOT ship this device") rather than print "done" if the file cannot be removed; an ordinary failing reset must say "Reset FAILED" instead, not tell you to stop passing the device on. After this sub-check the device has no saved WiFi, so check 4 needs a re-provision or a fresh flash regardless. **Every sub-step here now ends your SSH session** (litclock-dev#657): `--keep-wifi --poweroff` is permitted and still takes the poweroff arm, so it disables SSH too — and that sub-step explicitly rules out the panel and requires a shell, with no hotspot to fall back on because the WiFi was kept. Check 6's advice ("run anything needing SSH before a reset check") cannot rescue this one, because the check IS a before/after comparison across a reset. Either run the whole block from the console, or restore access between sub-steps by putting a blank `ssh` file in the SD card's boot partition. 4. **The SD-cloning path rotates it too** — the highest-fanout distribution channel, and the one gift mode does NOT cover. `docs/sd-card-cloning.md` is the "SD Cards for Friends & Family" flow: without this step every clone broadcasts `LitClock-Setup` with the SAME key, known to whoever made the cards and never rotated on any recipient. On a provisioned clock run `sudo ./scripts/prepare-for-cloning.sh --no-poweroff` — **the `--no-poweroff` matters**: since litclock-dev#660 the script powers the Pi off when it finishes, so without the flag the device is already down before you can inspect anything, and booting it to look is the exact action that re-mints the key. Confirm `/var/lib/litclock/hotspot-password` is gone along with any `.hotspot-password.*` staging files, and that the script aborts rather than reporting success if it cannot remove them. Separately, run it WITHOUT the flag once and confirm the Pi powers itself off. - **After the `--no-poweroff` run the clock looks bricked. It is not — do not debug it, and do not try to restart it back to life.** The panel freezes on whatever quote was last painted and port 80 refuses connections, indefinitely, with `/` still `rw`, load idle and nothing failed. The script stops `litclock-control.service` (Step 1) and `litclock.timer` (Step 4), but the stops are the transient half: it also clears `/etc/litclock/.setup-complete` and `.handoff-complete`, and `litclock-control.service` and `litclock.service` are `ConditionPathExists`-gated on those, so `systemctl start` on either exits 0 and changes nothing. Each half of the symptom has a familiar fault behind it — a frozen panel is what the litclock-dev#531 lgpio wedge looks like, a refused `:80` is what a bind failure looks like — so the pair reads as two faults at once rather than one intended state. (The pair is in fact a specific signature: `litclock-control.service` has no dependency on `litclock.service`, so neither lookalike produces both. That precision is no help at 1am.) It cost 20 minutes on 2026-08-15 (`dev-20260815-b0c0590`), hours after the fact, when the terminal holding the closing banner was long gone. **Shut the Pi down** (`sudo shutdown -h now`) — the card is a clone master and there is nothing left to test on it. - **The default (no-flag) run reaches the same state only if its power-off fails.** It normally halts, so there is no panel to misread; on the `poweroff || …` recovery path it prints "Power-off FAILED", exits 1, and leaves the identical frozen panel. Both arms of the banner say so. @@ -92,7 +139,17 @@ The first-boot flow (`scripts/first-boot.sh`) provisions WiFi via a web UI; ever - **The bash history is a directory afterwards, and that is the lock, not damage (litclock-dev#834).** `sudo ls -ld /home/pi/.bash_history /root/.bash_history` must show two EMPTY directories after the `--no-poweroff` run. `history -c` reaches only the script's own shell; the console or SSH shell you ran it from writes its history back on exit, during the power-off — on the bench the file was back eight seconds after "Clearing bash history... done", holding the operator's last command. A directory at the path fails that write with EISDIR for root and pi alike (a mode-0 file does not: root bypasses it, and `history -w` renames over it). Exit your shell and confirm both paths are STILL directories, then boot a CLONE (never the master) and confirm `first-boot.sh` removed both (`ls -ld` shows nothing, and the recipient's first `exit` creates a normal file). Run the script as the only open session regardless — but note the rule cannot cover the shell you run it FROM, which is why the lock exists at all. **The lock is now a hard gate**: stage a failure (`sudo mkdir /home/pi/.bash_history && sudo touch /home/pi/.bash_history/x` before a run, which is what an earlier aborted run plus a stray file looks like) and confirm the step prints `FAILED` + `Do NOT clone this card` and exits 1 rather than a yellow note. Remove the stray file and re-run; it must complete. - **A failed env.sh wipe stops BEFORE the WiFi question (litclock-dev#839).** Stage it by holding the sidecar lock from a second shell — `sudo flock /home/pi/litclock/env.sh.lock sleep 120` — then run the script. It must print `Clearing configuration (env.sh)... FAILED` and the `Do NOT clone this card` banner and exit 1 **without** asking `Clear saved WiFi networks?`; afterwards `/var/lib/litclock/hotspot-password` must still exist, `nmcli connection show` must still list your network, and `env.sh` must be byte-identical. Pre-fix the same run printed "Clearing setup-hotspot password... done" BEFORE the red env.sh line, having already wiped everything else. **The device is NOT in the looks-bricked state the `--no-poweroff` bullet below describes**, and that is the point of the fix: the setup-state markers are removed only after the wipe succeeds, so an aborted run leaves a fully provisioned clock that still paints quotes and still boots normally. What IS true until the next boot: Step 1 already stopped `litclock-control.service` (the PWA) and the updater, so port 80 refuses connections in the meantime — reboot, or `sudo systemctl start litclock-control.service`, restores it. Release the lock and re-run; it must complete normally. Also confirm the aborted run did NOT leave `/var/lib/litclock/clone-prep-unfinished` behind: it changed nothing, so the next run must not open with the "previous run did not finish" warning. - **Do not reboot it to "check" it — that contaminates the master.** `first-boot.sh` runs again on that card, and whether the boot also re-mints the setup-WiFi key depends on which branch it takes. `is_wifi_connected()` is literally `ip addr show wlan0 | grep -q 'inet '`, and `litclock-firstboot.service` is ordered `After=NetworkManager.service`, **not** `network-online.target` — so the branch turns on whether `wlan0` happens to hold an address at that instant, not on whether a profile is saved. With **no** address it raises the setup hotspot, and `create_hotspot()` mints a fresh permanent key (litclock-dev#660) that every clone taken afterwards would share. With an address it completes setup inline — no page, no hotspot (litclock-dev#647), **no new key** — and runs straight through to the handoff splash. The script's own closing banner states the key hazard unconditionally, which is the safe direction; do not read the second branch as permission to boot the master. - 5. **The support bundle no longer carries the password.** litclock-dev#620 turns a transient leak into a durable one, so the redaction fix ships with it. After provisioning, note the password, then PWA → Diagnostics → copy the support payload (and the deep logs) and grep for that exact string. It must not appear. Pre-fix, a real `sudo` audit line came back from `redact_text()` with the password intact. + 5. **The password must be absent from the RAW journal — that is the check, not the redacted bundle (rewritten for litclock-dev#867).** The original wording — grep the support payload and the deep logs for the setup password; it must not appear — passed VACUOUSLY on a fresh `dev-20260918-787d307` flash: the password never reached the journal at all — not because of litclock-dev#620's redaction but because litclock-dev#654 moved the PSK off the argv (`_build_hotspot_profile` feeds it to `nmcli connection edit` on stdin), so `redact_text()` had nothing to scrub and the check read identically whether redaction worked or had been deleted outright (the litclock-dev#860 / litclock-dev#864 class). The property worth holding is litclock-dev#654's invariant, and it CAN fail: sudo's command-audit line records the full argv, journald is persistent on the image, and the deep-logs bundle exports the last 50 lines of five units from whatever boots journald retains (no `-b`), so a future change that passes the password on a `sudo` argv puts it in the raw journal for good. If the grep ever fires, the place to look is `src/wifi_provision.py`'s profile writer, not the redactor. (The key IS still on a non-sudo argv during provisioning — `first-boot.sh` passes `--hotspot-password` to `setup_server.py`, readable in `/proc//cmdline` for the life of that process — so "never reaches the journal" is not "never leaves the process".) Preconditions: a provisioning that raised the setup hotspot (the pre-connected litclock-dev#647 path never mints the file, and then this check does not apply), and NO `--hotspot-password` override on that run (a bench override leaves the stored file unchanged, so you would be searching for the wrong key). Over SSH, as separate steps — one pipeline hides three failures behind a `0`: + + ```bash + PW=$(sudo cat /var/lib/litclock/hotspot-password) && [ "${#PW}" -ge 8 ] && echo "key ok (${#PW} chars)" + sudo journalctl --no-pager > /tmp/journal.txt && wc -l < /tmp/journal.txt # ALL boots, default short format (keeps the `sudo[pid]:` identifier); must be non-empty + grep -cF 'sudo[' /tmp/journal.txt # >= 1: the audit-line class that leaked pre-fix is actually in what you searched + grep -cF -e "$PW" /tmp/journal.txt # must print 0 + grep -cF 'LitClock-Setup' /tmp/journal.txt # >= 1: the CONTROL — proves the search works + ``` + + Each line guards the next. An empty `PW` makes `grep -F` match every line (a false FAIL that reads like a catastrophic leak); a failed `journalctl` prints `0` matches (a false PASS); a journal with no `sudo[` lines proves nothing either way. `-e` keeps a key that begins with `-` from being parsed as an option. The last line's `grep` exits non-zero on the PASS — read the printed count, not `$?`. **Never put the value on a `sudo` argv while you are here** (no `sudo grep -r "$PW" /`, no retyping it under `sudo`): sudo would audit-log the password into the persistent journal by your own hand, and check 5 would fail forever after on this device. Only `journalctl` and `cat` run privileged above; the greps are unprivileged on purpose. Redaction itself is NOT a bench check: `tests/test_redaction.py::TestNmcliArgvSecrets` executes `redact_text()` against a real sudo audit line carrying an `nmcli … password …` argv, and `tests/test_diagnostics_no_secrets.py::test_synthetic_journal_secrets_do_not_leak` proves the exporter applies it. Only if the raw count is NON-zero does the bundle become worth reading: then PWA → Diagnostics → copy the deep logs, and the key's absence there tells you whether the redactor caught the shape — with the caveat that the bundle is a 50-line tail per unit, so a leak early in `litclock-firstboot.service`'s output can already have scrolled out of it by export time; the raw journal is the only read that sees the whole provisioning window. Baseline, 2026-09-18: `0` in the raw journal, `0` in all three endpoints, 39 `sudo[` lines of 196 in the bundle. **Re-run 2026-09-20 on `dev-20260920-512c291`, and this time it is demonstrably not vacuous:** 2120 journal lines, **427 `sudo[` audit lines** inside the very file being searched, zero password hits — plus a control string that IS present (`LitClock-Setup`, 15 hits), which is what proves the search works at all. Run that control every time: a `0` from a broken grep is indistinguishable from a `0` from a clean device, and that confusion is the whole reason this check was rewritten. `rm /tmp/journal.txt` when done — it is the unredacted journal. 6. **A factory reset now turns SSH OFF on dev images too, and that is new.** litclock-dev#657 removed the dev-image exception: `disable_ssh_for_handoff` runs on both terminal arms of `reset-setup.sh` — gift mode and the litclock-dev#627 `--poweroff` reset — on every image, and the PWA's Factory reset button takes the second @@ -443,6 +500,13 @@ sudo systemctl start litclock-update.service && journalctl -fu litclock-update ticks verified on hardware 2026-09-18. The pip hash stays deleted across the blocked ticks (the revert removes `HASH_FILE`) so the recovering release re-runs pip once; that is expected, not a second fault. +- **The runtime-render self-test runs after the marker, and its verdict lands in the memo (litclock-dev#871 Stage A — INERT this release).** It runs only on an APPLIED release: a tick with nothing new exits at the no-op early-out before Phase 4.5, so publish one from a local bare remote — and build that release so it touches NONE of the six proof inputs (`fonts/`, the measurement dump, `requirements.txt`, `quote_renderer.py`, `gd_measure.py`, `validate_measurement.py`), or Phase 2b's revoke block removes the marker you are about to tamper with, the KEEP arm re-stamps a fresh one, and the self-test passes; a `runtime-render validation marker removed:` line in the journal means the staging was undone and the run proves nothing. A normal run on a device with a marker must log `Running the runtime-render self-test`, then `[selftest] dry-run: rendered 800x480 image, render_mode=runtime`, then `runtime-render self-test PASSED in N.Ns`, and `/var/lib/litclock/runtime-render-selftest.json` holding `"result":"passed"` with `duration_s` and the release `sha` — the durable pass record Stage B will gate on (the journal is capped at 7 days, so the log line alone would be gone before the next release); read `duration_s`: Stage B will need devices that render inside the 4s lead — and `LITCLOCK_RUNTIME_RENDER` in `env.sh` must be UNCHANGED afterwards, because Stage A changes nothing on the device; that is the point. A failure is silent by design, so stage one to prove the harness can fail — **in the marker, not in the tree**: `sudo sed -i 's/digest=[0-9a-f]*/digest=deadbeef/' /home/pi/litclock/.runtime-render-validated`. The marker is gitignored, so Phase 2's `git reset --hard` leaves it alone; it is still PRESENT, so the KEEP arm does not re-stamp it and clears the memo; and the painter's digest check declines the text tier, so the self-test paints a PNG and exits 3. Expect `[selftest] ... render_mode=image`, then `self-test did not pass: the painter fell back`, then `jq .result,.rc /var/lib/litclock/runtime-render-validation.json` printing `"selftest-failed"` and `3` (the file is compact JSON — do not grep for a spaced literal), `/var/lib/litclock/runtime-render-selftest.json` GONE (a fail retires an earlier pass), and `/api/status` showing the memo as `runtime_render_validation`. **The update must still end in `Update Complete`** and the panel must keep painting — a failed self-test is not an update failure. Two stagings that CANNOT fail, recorded so nobody re-derives them: moving the marker aside (the KEEP arm re-stamps an absent marker before the self-test runs, so that run passes), and `sudo mv fonts{,.bak}` (four tracked files — Phase 2 restores them before anything looks, and even if it did not, the PNG tier needs the same fonts for the masthead, so the smoke gate's own dry-run would revert the release before reaching the self-test). Then restore the marker — `sudo -u pi /home/pi/litclock/venv/bin/python3 tools/validate_measurement.py check --stamp` re-earns it — and apply another release: the present marker clears the memo at the top of the KEEP arm, the self-test passes, and the memo file must be gone. Three more things to confirm while there: with a non-English `LITCLOCK_LANGUAGE` in `env.sh` the `[selftest]` lines render THAT corpus (the catalog probes above are pinned to English; this one deliberately is not); `/run/litclock/current-quote.png` keeps its pre-tick mtime (the self-test renders into a throwaway directory); and no weather request leaves the device during the self-test (`WEATHER_ENABLED` is forced off for it — a capability probe has no business on the network). + + **First field measurement, 2026-09-20 (`dev-20260920-512c291`, Pi Zero 2 W):** the self-test recorded **3.8s** during the update; five hand-run repeats on an idle system gave **3.4 / 3.5 / 3.5 / 3.5 / 3.7s**. The gap is update-time load, not instrument bias, and it errs the safe way. + + **Read what `duration_s` actually contains before you build a threshold on it** (litclock-dev#878 review). It is WHOLE-PROCESS wall time: `t0` is stamped before the subshell, so it includes sourcing `env.sh`, spawning the interpreter, and importing PIL — a material share of 3.5s on a Zero 2 W — and only then the render. It is **not** render time, and it is **not** comparable to a frame-settle figure, which includes a panel refresh the dry-run never performs (it exits before `epd.init()`). + + Worse for a naive gate: the painter pays that same startup BEFORE it computes its target instant, so that share is not covered by the 4.0s lead at all. The lead is not a render budget. What actually makes a frame late — timer fire through `epd.display()` returning, panel included — is measured by neither this number nor the lead, so Stage B needs to decide what it is really gating before picking a value. Two things that ARE settled: render duration cannot change WHICH minute is painted, because the target is computed once and threaded through the quote pick, masthead, status file and clear gate; but a stall BEFORE that computation (a hanging weather fetch, say) can push the target into the next minute and leave one unpainted, so "slow is harmless" is true only on the render side. - **`catalog-count` is stdout-compared, so check its contract directly.** On the device: `sudo -u pi /home/pi/litclock/venv/bin/python3 src/eink_display.py catalog-count` must print an integer and **exit 0**. Break the bundle diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0e6a012..2e38862 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -32,6 +32,13 @@ Translations are welcome — the corpus is built to be localized, and you start, because both are cheap to check once and effectively unauditable across thousands of rows later. +A translated corpus is a CSV of its own: point the language's `corpus.path` in +`languages.json` at it and the clock's text renderer reads that file for the +device's active language (litclock-dev#870) — no per-language image set is +needed. The pre-rendered PNGs under `images/` stay English (the registry's +`fleet_default`), which is why a second language needs on-device text +rendering (litclock-dev#871) before it can be activated. + **Name the edition you took a quote from, in the pull request.** Which edition a row came from is the one fact nobody can recover from a CSV diff. diff --git a/README.md b/README.md index 9ad17cd..95c80eb 100644 --- a/README.md +++ b/README.md @@ -300,7 +300,7 @@ The clock keeps working on whatever SHA it's pinned to; manual updates via the a Most resets don't need a shell — use the control app's **System** tab: - **Reset WiFi** — forget saved networks and return to the LitClock-Setup network (your settings — location, weather, gift mode — are kept) -- **Factory reset** — wipe all settings, your WiFi, and the setup network's password, then power off. The next power-on raises `LitClock-Setup` with a **new** password shown on the clock's screen, so a phone that saved the old one must forget the network first +- **Factory reset** — wipe all settings, your WiFi, the setup network's password, and the default shell history files, then power off. The next power-on raises `LitClock-Setup` with a **new** password shown on the clock's screen, so a phone that saved the old one must forget the network first - **Prepare for Gifting** — wipe WiFi, write a welcome message for the recipient, and power off ready to box up From a shell, the equivalent is: @@ -315,7 +315,7 @@ Flags: - `--reboot` — reboot automatically after reset - `--keep-wifi` — keep your WiFi **and** the setup network's password. For a technical user resetting their own clock: the device stays on its network, never starts a setup network, and an SSH session survives the reset - `--wipe-wifi` — no-op; erasing both passwords is the default (litclock-dev#666) -- `--gift-mode` — prepare for shipping: wipes WiFi, paints a welcome splash on the e-ink, and powers off. Implies `--wipe-wifi --yes`. +- `--gift-mode` — prepare for shipping: wipes WiFi and the default shell history files, paints a welcome splash on the e-ink, and powers off. Implies `--wipe-wifi --yes`. ### Troubleshooting diff --git a/docs/recovery.md b/docs/recovery.md index 7293fb8..ca30f2a 100644 --- a/docs/recovery.md +++ b/docs/recovery.md @@ -69,8 +69,8 @@ clock over SSH: it preserves both, so the device returns to its own network and never raises a hotspot — which also means your SSH session survives. Without it, a reset over SSH drops the connection when the WiFi goes. -Use `--gift-mode` to prepare a device for shipping to someone else (wipes WiFi + -config, writes a welcome splash, powers off). See +Use `--gift-mode` to prepare a device for shipping to someone else (wipes WiFi, config +and the default shell history files, writes a welcome splash, powers off). See [SD Card Cloning](sd-card-cloning.md) for duplicating a configured card. --- diff --git a/docs/script-reference.md b/docs/script-reference.md index 0b9beac..7f39ba3 100644 --- a/docs/script-reference.md +++ b/docs/script-reference.md @@ -9,7 +9,7 @@ Scripts are organized into `scripts/` (shell) and `src/` (Python): | `src/clear.py` | `python3 src/clear.py` | Clear the e-ink display to white | | `src/wifi_provision.py` | `python3 src/wifi_provision.py [flags]` | WiFi provisioning via captive portal hotspot | | `scripts/update.sh` | `./scripts/update.sh` | Pull latest code and apply updates in-place | -| `scripts/reset-setup.sh` | `sudo ./scripts/reset-setup.sh [--yes] [--reboot] [--poweroff] [--keep-wifi] [--gift-mode]` | Reset configuration (see [Resetting](../README.md#resetting)); `--gift-mode` preps the device for shipping with a welcome splash. **`--gift-mode` and `--poweroff` both disable SSH before powering down** (litclock-dev#528) — a handed-on device gets a fresh-flash posture; re-enable per [recovery](recovery.md) | +| `scripts/reset-setup.sh` | `sudo ./scripts/reset-setup.sh [--yes] [--reboot] [--poweroff] [--keep-wifi] [--gift-mode]` | Reset configuration (see [Resetting](../README.md#resetting)); `--gift-mode` preps the device for shipping with a welcome splash. **`--gift-mode` and `--poweroff` both wipe and lock the default shell history files (litclock-dev#868) and disable SSH before powering down** (litclock-dev#528) — a handed-on device gets a fresh-flash posture; re-enable per [recovery](recovery.md) | | `scripts/prepare-for-cloning.sh` | `sudo ./scripts/prepare-for-cloning.sh` | Wipe config and credentials for SD card cloning (see [Creating SD Cards](sd-card-cloning.md)) | | `scripts/cut-release.sh` | `./scripts/cut-release.sh vX.Y.Z [--expect-sha SHA] [-m MSG] [--yes]` | Promote the CHANGELOG heading, commit and tag a release — does not push (see [Building the Image](building-image.md)) | | `scripts/install.sh` | _(retired)_ | Superseded by the flashed image; see the "Running your own code on the clock" section in [README.md](../README.md) | diff --git a/scripts/first-boot.sh b/scripts/first-boot.sh index d6afcb8..f9f6abc 100755 --- a/scripts/first-boot.sh +++ b/scripts/first-boot.sh @@ -507,6 +507,10 @@ disable_first_boot() { # explicit `history -w` and the HISTFILESIZE truncate all fail with EISDIR, # root included). That directory rides every clone; this puts the paths back # to "absent" so bash creates a normal file on the recipient's first login. +# Since litclock-dev#868 the same lock (clear_and_lock_bash_history in +# lib/state.sh) is left by reset-setup.sh's gift-mode and --poweroff arms too, +# and a reset lands on this same not-yet-set-up path, so one restore covers +# every handoff. The name keeps its clone-prep origin. # # `rmdir`, unconditionally, through sudo: it removes an EMPTY DIRECTORY and # nothing else, so a real history file (ENOTDIR) or an absent path (ENOENT) is @@ -533,10 +537,10 @@ restore_bash_history_after_clone_prep() { case "$_verdict" in CLEAR) ;; LOCKED) - log "WARN clone-prep history lock at $_p could not be removed; shell history will not be saved there (litclock-dev#834)" + log "WARN handoff history lock at $_p could not be removed; shell history will not be saved there (litclock-dev#834, litclock-dev#868)" ;; *) - log "WARN could not check the clone-prep history lock at $_p (the privileged probe did not run); if shell history is not saved there, remove it with: sudo rmdir $_p (litclock-dev#834)" + log "WARN could not check the handoff history lock at $_p (the privileged probe did not run); if shell history is not saved there, remove it with: sudo rmdir $_p (litclock-dev#834, litclock-dev#868)" ;; esac done @@ -557,9 +561,8 @@ main() { exit 0 fi - # litclock-dev#834 — undo prepare-for-cloning.sh's history lock before - # anything else on the not-yet-set-up path, which is the only path a - # cloned card takes, so the recipient's first login gets a normal history. + # litclock-dev#834 / litclock-dev#868 — undo the handoff history lock before anything + # else on the not-yet-set-up path (see the function header). restore_bash_history_after_clone_prep /home/pi/.bash_history /root/.bash_history # Stop the clock timer — if re-running first-boot (e.g. after removing diff --git a/scripts/lib/state.sh b/scripts/lib/state.sh index 63a6bdb..a83197e 100644 --- a/scripts/lib/state.sh +++ b/scripts/lib/state.sh @@ -81,6 +81,105 @@ atomic_remove_file() { rm -f "$target" 2>/dev/null || sudo rm -f "$target" 2>/dev/null || true } +# ─── shell-history wipe + write-back lock (litclock-dev#834, litclock-dev#868) ── +# +# clear_and_lock_bash_history ... — empty each history path, then replace +# it with an empty DIRECTORY so that shells still open when the device powers +# off cannot write their history back. Shared by prepare-for-cloning.sh (Step +# 6) and reset-setup.sh's two handoff arms (gift mode and the --poweroff +# factory reset), because all three exist to pass the device to someone else +# and the previous owner's typed commands — an `nmcli … password …` line +# included — must not go with it. Before litclock-dev#868 only clone prep did this. +# +# `history -c` reaches only the calling script's own non-interactive shell. +# The console or SSH shell the operator ran the script FROM writes its history +# back on exit, during the power-off — bash's save_history() APPENDS the +# session's lines, then truncates to HISTFILESIZE — so on the bench the file +# `rm` had removed was back eight seconds later holding the operator's last +# command. A directory at the path fails open(O_WRONLY|O_APPEND), rename() over +# it (`history -w` writes a sibling temp file and renames) and the truncate's +# read with EISDIR, which no capability bypasses (root's DAC override beats a +# mode-0 file; a /dev/null symlink loses to the rename), and one `rmdir` +# removes it. scripts/first-boot.sh removes both directories on the +# not-yet-set-up path (restore_bash_history_after_clone_prep), which every +# clone AND every reset lands on, so the next owner's first login gets an +# ordinary history file. +# +# Reports, does not decide: the caller owns fatality. On return, +# _HIST_DIRTY paths whose CONTENTS survived (could not be removed — an +# immutable file, a read-only parent, a NON-EMPTY directory +# left by an earlier aborted run — or were refused because +# removing the NAME would not remove the contents: a symlink, +# or a file with a second hard link); +# _HIST_UNLOCKED paths that were emptied but could not be locked; +# _HIST_LOCKED paths now holding the lock — a caller that aborts can tell +# the operator which locks stay until the next boot. +# Returns 0 when DIRTY and UNLOCKED are both empty, 1 otherwise. The `rmdir` +# before `rm -f` takes the lock a previous run left (and refuses a non-empty +# one); an absent path satisfies both harmlessly. +# +# What the lock is, and is not (litclock-dev#873 review): a barrier against bash's own +# exit-time write-back and `history -w`, which is the measured leak. It is a +# root-owned directory in a pi-writable home, so the pi user can `rmdir` it — +# that user is the device's current owner, and a hostile owner racing their +# own reset is outside the threat model (the previous owner's history reaching +# the NEXT owner). Only the two default paths are covered; a shell with its own +# HISTFILE saves wherever that points. +clear_and_lock_bash_history() { + history -c 2>/dev/null || true + _HIST_DIRTY=() + _HIST_UNLOCKED=() + _HIST_LOCKED=() + local _h _links + for _h in "$@"; do + # Refuse to follow (litclock-dev#873 review, Codex): `rm -f` on a SYMLINK unlinks + # the link and leaves its target — contents included — on the device; + # a file with a second hard link survives the same way. Both are the + # owner's own arrangement, and the honest verdict is "could not clear", + # never a green line over a file that is still there. Checked BEFORE + # anything is removed, so the evidence of what survived is intact. + # The one symlink that is fine is the standard "disable history" idiom, + # `ln -sf /dev/null ~/.bash_history` (litclock-dev#873 red team): a character + # device holds nothing, so the link is simply removed and locked over. + # `-c` follows the link; `-L` does not. + if [[ -L "$_h" && ! -c "$_h" ]]; then + _HIST_DIRTY+=("$_h") + continue + fi + if [[ -f "$_h" ]]; then + _links=$(stat -c %h "$_h" 2>/dev/null || echo 0) + if [[ "$_links" != 1 ]]; then + _HIST_DIRTY+=("$_h") + continue + fi + fi + # `chattr +a` / `+i` on .bash_history is a common hardening idiom, and + # on the --poweroff arm a DIRTY verdict lands after env.sh is wiped and + # the key rotated, with no PWA left to retry from (litclock-dev#873 review, Claude + # adversarial). Every caller runs as root, so clear the attributes + # first; on a non-ext4 path or a missing chattr this is a no-op and the + # verdict below still tells the truth. Only reached for a regular file + # or a directory — the symlink case was refused above. + chattr -ia "$_h" 2>/dev/null || true + rmdir "$_h" 2>/dev/null || rm -f "$_h" 2>/dev/null || true + if [[ -e "$_h" || -L "$_h" ]]; then + _HIST_DIRTY+=("$_h") + continue + fi + # mkdir IS the lock, and its status is the verdict: it either created + # OUR empty directory or something landed at the path between the rm + # and here. A failure is reported as UNLOCKED, not swallowed and then + # re-tested with `-d`, which follows a symlink and would have called a + # pi-planted link "locked" (litclock-dev#873 review, Codex + security specialist). + if mkdir "$_h" 2>/dev/null && [[ -d "$_h" && ! -L "$_h" ]]; then + _HIST_LOCKED+=("$_h") + else + _HIST_UNLOCKED+=("$_h") + fi + done + (( ${#_HIST_DIRTY[@]} == 0 && ${#_HIST_UNLOCKED[@]} == 0 )) +} + # ─── env.sh writer-lock helpers (issue litclock-dev#274) ───────────────────────── # # Three shell writers mutate env.sh (update.sh Phase 3, reset-setup.sh, diff --git a/scripts/litclock-lkg-record.sh b/scripts/litclock-lkg-record.sh index cb41323..94b501d 100755 --- a/scripts/litclock-lkg-record.sh +++ b/scripts/litclock-lkg-record.sh @@ -21,9 +21,13 @@ # ▼ # write lkg-sha (atomic: .tmp + mv) # -# Bootcheck/revert is a separate follow-up (issue litclock-dev#241 → bootcheck-revert) -# and is intentionally NOT shipped here. This script is observability + -# substrate; consumption lands in its own PR after we have field data. +# This script is the substrate: it records the LKG sha and nothing else. +# Consumption SHIPPED — `litclock-bootcheck.sh` reads this sha, counts failed +# boots, pins `rollback-target`/`blocked-sha` and triggers the updater in +# rollback mode. The sentence that stood here said bootcheck/revert was "a +# separate follow-up ... intentionally NOT shipped", which stopped being true +# when it landed; corrected 2026-09-19 alongside litclock-dev#847's own +# pending-work wording. set -uo pipefail diff --git a/scripts/prepare-for-cloning.sh b/scripts/prepare-for-cloning.sh index dcdd087..f2f1b56 100755 --- a/scripts/prepare-for-cloning.sh +++ b/scripts/prepare-for-cloning.sh @@ -123,7 +123,7 @@ _THIS_SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # "say DO NOT CLONE loudly". Check explicitly, before any step has run, so the # operator gets a reason instead of a bash error. Same check as # reset-setup.sh's, which needs it for correctness rather than for the message. -for _fn in atomic_write_env_sh env_sh_defaults; do +for _fn in atomic_write_env_sh env_sh_defaults clear_and_lock_bash_history; do if ! declare -F "$_fn" >/dev/null 2>&1; then echo -e "${RED}ERROR: $_fn is not defined after sourcing lib/state.sh.${NC}" >&2 echo " $_THIS_SCRIPT_DIR/lib/state.sh is missing or too old for this script." >&2 @@ -528,6 +528,8 @@ rm -f "$STATE_DIR/reset-failed" 2>/dev/null || true # the expected measurement (or that a tick never had the budget to try); every # clone would inherit the master's verdict about hardware it has never run on. rm -f "$STATE_DIR/runtime-render-validation.json" 2>/dev/null || true +# litclock-dev#871 Stage A: the self-test pass record is per-device too. +rm -f "$STATE_DIR/runtime-render-selftest.json" 2>/dev/null || true for _m in .setup-complete .handoff-complete; do # `-L` alongside `-e` because `-e` follows symlinks and is false for a @@ -666,31 +668,11 @@ echo -e "${GREEN}done${NC}" # Step 6: Clear bash history — and lock it against write-back (litclock-dev#834). # -# `history -c` clears only THIS script's non-interactive shell. Any interactive -# shell still open when the Pi powers off (the console login the operator ran -# this from, an SSH session, a `sudo -i` root shell) writes its in-memory -# history back on exit — bash's save_history() APPENDS the session's lines, -# then truncates to HISTFILESIZE — so on the bench the file `rm` had removed was -# back eight seconds later holding the operator's last commands, and the card -# was imaged with them. On a master for strangers that can include -# `nmcli ... password ...` lines. -# -# The lock is an empty DIRECTORY at each history path. The shape is deliberate, -# measured against bash 5.2 / readline 8.2: -# - an empty mode-0 root-owned FILE blocks the exit-time append for `pi` -# (EACCES) but not for a root shell (DAC override), and not `history -w`, -# which readline writes to a sibling temp file and rename()s over the -# target — a rename needs only the parent directory; -# - a `/dev/null` symlink loses to the same rename; -# - `chattr +i` blocks everything but needs ext4 and root to undo, and a -# failed restore leaves the recipient an unremovable file; -# - a directory fails open(O_WRONLY|O_APPEND), rename() over it and the -# truncate's read with EISDIR, which no capability bypasses, and one -# `rmdir` removes it. -# scripts/first-boot.sh removes both directories on the clone's first boot -# (restore_bash_history_after_clone_prep), so the recipient gets an ordinary -# history. A HISTFILE drop-in under /etc/profile.d was rejected: it reaches -# only shells started AFTER it, never the open one doing the write-back. +# The mechanism — why `history -c` is not enough, why the lock is an empty +# DIRECTORY and not a mode-0 file or a symlink, and what first-boot.sh does +# about it on the clone — is documented once, on clear_and_lock_bash_history +# in lib/state.sh (shared with reset-setup.sh's handoff arms since +# litclock-dev#868). What is specific to THIS script is the verdict: # # FATAL if either path cannot be emptied or locked (litclock-dev#855 review). # A YELLOW note was the first cut and is not enough here: this script's whole @@ -701,46 +683,14 @@ echo -e "${GREEN}done${NC}" # "locked" and printed green over the contents, which first-boot's rmdir then # leaves in place on every clone. echo -n "Clearing bash history... " -history -c 2>/dev/null || true -_HIST_DIRTY=() -_HIST_UNLOCKED=() -for _h in "$_PI_BASH_HISTORY" "$_ROOT_BASH_HISTORY"; do - # Empty the path whatever shape it has: rmdir takes the lock a previous run - # left (and refuses a non-empty one), rm -f takes a real history file. An - # absent path satisfies both harmlessly. - # - # The rmdir is NOT redundant with the mkdir below (litclock-dev#855 review - # G): it was, while a surviving entry only warned, but now a surviving - # entry ABORTS — so without it a second run over the lock this script - # itself left would meet a directory `rm -f` cannot remove and refuse the - # card. Pinned by test_a_second_run_over_the_lock_is_a_noop. - rmdir "$_h" 2>/dev/null || rm -f "$_h" 2>/dev/null || true - if [[ -e "$_h" || -L "$_h" ]]; then - # Something we could not remove survives — and on this path that means - # its CONTENTS survive too. `-L` as well as `-e`, which follows - # symlinks and is false for a dangling one. - _HIST_DIRTY+=("$_h") - continue - fi - mkdir "$_h" 2>/dev/null || true - # Just "is it a directory". Emptiness and non-symlink-ness need no test - # HERE and deliberately have none (litclock-dev#855 review E3, and a mutant - # that proved the point): nothing reaches this line unless the path was - # verified gone two lines up, so what `mkdir` leaves is a real, empty - # directory or nothing at all. A non-empty directory or a symlink — the - # shapes that matter — survive removal and are caught by the DIRTY arm - # above, which is where `-L` earns its place, because `-e` follows symlinks - # and is blind to a dangling one. An arm no case can reach is an arm no - # mutant can kill, so it is better not written. - if [[ ! -d "$_h" ]]; then - _HIST_UNLOCKED+=("$_h") - fi -done +# The helper reports; this script decides (see above). +clear_and_lock_bash_history "$_PI_BASH_HISTORY" "$_ROOT_BASH_HISTORY" || true if (( ${#_HIST_DIRTY[@]} )); then _abort_do_not_clone "Could not clear the shell history at ${_HIST_DIRTY[*]}." \ "every copy would carry the commands typed on this device, WiFi passwords included." \ - "A file that will not unlink (chattr +i, a read-only card) or a non-empty directory" \ - "left by an earlier aborted run. This card is already part-way prepared: markers" \ + "A file that will not unlink (chattr +i, a read-only card), a symlinked or hard-linked" \ + "history file, or a non-empty directory left by an earlier aborted run. This card is" \ + "already part-way prepared: markers" \ "cleared, env.sh scrubbed, setup-WiFi key NOT yet removed. Fix the cause, then run" \ "this script again from the start." fi @@ -752,7 +702,7 @@ if (( ${#_HIST_UNLOCKED[@]} )); then "exists. This card is already part-way prepared: markers cleared, env.sh scrubbed," \ "setup-WiFi key NOT yet removed. Fix the cause, then run this script again." fi -unset _HIST_DIRTY _HIST_UNLOCKED _h +unset _HIST_DIRTY _HIST_UNLOCKED _HIST_LOCKED echo -e "${GREEN}done${NC}" # Step 7: Clear legacy SSL certificates (nothing regenerates these since litclock-dev#715) diff --git a/scripts/reset-setup.sh b/scripts/reset-setup.sh index 9b797ec..f992de1 100755 --- a/scripts/reset-setup.sh +++ b/scripts/reset-setup.sh @@ -29,6 +29,11 @@ CONFIG_DIR="/etc/litclock" # Same override convention as the other scripts (wifi-watchdog, bootcheck, # lkg-record, update) and as src/wifi_provision.py's STATE_DIR. STATE_DIR="${LITCLOCK_STATE_DIR:-/var/lib/litclock}" +# litclock-dev#868 — the two shell-history paths the handoff arms wipe and +# lock. Fixed, not env-overridable (same convention as prepare-for-cloning.sh's +# pair); the executed tests inject their own by redefining these two lines. +_PI_BASH_HISTORY="/home/pi/.bash_history" +_ROOT_BASH_HISTORY="/root/.bash_history" # Bound on each `systemctl start litclock-shutdown.service` re-arm (litclock-dev#727, # litclock-dev#833). Must stay well inside litclock-reset.service's TimeoutStartSec=60: # on the PWA arm this script runs INSIDE that unit. @@ -58,11 +63,12 @@ _THIS_SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # knob and the gift language would be gone with nothing saying so, on a device # that is usually about to be shipped to someone. Fail loudly instead: nothing # destructive has run at this point, so exiting here is free. -for _fn in atomic_write_env_sh env_sh_defaults; do +for _fn in atomic_write_env_sh env_sh_defaults clear_and_lock_bash_history; do if ! declare -F "$_fn" >/dev/null 2>&1; then echo -e "${RED}ERROR: $_fn is not defined after sourcing lib/state.sh.${NC}" >&2 echo " $_THIS_SCRIPT_DIR/lib/state.sh is missing or too old for this script." >&2 - echo " Refusing to reset: a partial reset would leave env.sh empty." >&2 + echo " Refusing to reset: a partial reset would leave env.sh empty, or hand the" >&2 + echo " device on with its shell history still on it (litclock-dev#868)." >&2 echo " Re-run the updater, or reinstall from a matching release." >&2 exit 1 fi @@ -77,6 +83,66 @@ done # future call-site move cannot recreate the class; a structural test pins # def-before-first-call for every function in this file. +# litclock-dev#868 — wipe the previous owner's shell history before the device +# leaves their hands, on BOTH handoff arms (gift mode and the --poweroff factory +# reset the PWA runs). prepare-for-cloning.sh had done this since litclock-dev#834; +# these two paths rotate the setup key and wipe env.sh for exactly the same +# "this device is changing hands" reason and left the history behind — measured +# after a PWA Factory reset on 2026-09-18: /home/pi/.bash_history, 7 lines, +# survived. A normal owner never opens a shell, so this usually finds nothing; +# it bites on the devices most likely to be handed on deliberately — one a +# maintainer SSHed into, or one where WiFi was ever configured by hand (an +# `nmcli … password …` line lands in history with the passphrase verbatim). +# +# The plain CLI reset and --reboot are deliberately NOT covered: they hand +# control back to an operator at a console, not to a new owner — the same +# distinction litclock-dev#718 draws for the "Powered Off" splash. +# +# FATAL, on both arms, by decision (litclock-dev#868 left it open). It matches how these +# same arms already treat a failed setup-key rotation: the device stays ON with +# its CURRENT owner and a red banner, rather than powering off as "ready to hand +# on" with the old owner's commands still on it. Called ONCE, for both arms, +# from the guard just above Step 7: after the non-gift rotation belts, BEFORE +# the WiFi wipe (a SIGHUP'd bash saves its history, so the lock must exist +# before the one step that can SIGHUP a CLI-over-SSH run — litclock-dev#873 red team), +# before the completion banner (so a failure never prints one — and that +# banner's literal text is a test anchor, hence "completion banner" here), before +# the gift arm's own rotation, and before disable_ssh_for_handoff, so a +# failure here leaves the owner their shell to fix it with. It does NOT run +# when a gift prep is already going to abort on its env.sh wipe. The lock (a +# directory at each path) is what stops the shell you are typing in from +# writing its history back as the device powers off; first-boot.sh removes it +# on the next owner's first boot. `--keep-wifi --poweroff` (same owner, moved +# house) takes the poweroff arm and gets the wipe too, exactly as it gets +# SSH-off — a technical user's path, and `sudo rmdir` undoes the lock. +clear_shell_history_for_handoff() { + echo -n "Clearing shell history before handoff... " + if clear_and_lock_bash_history "$_PI_BASH_HISTORY" "$_ROOT_BASH_HISTORY"; then + echo -e "${GREEN}done${NC}" + return 0 + fi + echo -e "${RED}FAILED${NC}" + echo -e "${RED}========================================${NC}" + echo -e "${RED} Shell history SURVIVED — do NOT hand this device on${NC}" + echo -e "${RED}========================================${NC}" + if (( ${#_HIST_DIRTY[@]} )); then + echo -e "${RED}Could not clear the shell history at ${_HIST_DIRTY[*]}.${NC}" + echo -e "${RED}The commands typed on this device, WiFi passwords included, are still on it.${NC}" + fi + if (( ${#_HIST_UNLOCKED[@]} )); then + echo -e "${RED}Could not lock ${_HIST_UNLOCKED[*]} against write-back.${NC}" + echo -e "${RED}The shell you are typing in would write its history back as the device powers off.${NC}" + fi + if (( ${#_HIST_LOCKED[@]} )); then + echo -e "${YELLOW}Locks already placed at ${_HIST_LOCKED[*]} stay until the next boot; to keep${NC}" + echo -e "${YELLOW}using shell history on this device meanwhile, 'sudo rmdir' them.${NC}" + fi + echo -e "${RED}NOT powering off. Fix the cause (a symlinked or hard-linked history file, an immutable${NC}" + echo -e "${RED}file, a read-only filesystem, or a non-empty directory left by an earlier run), then${NC}" + echo -e "${RED}run the reset again (litclock-dev#868).${NC}" + exit 1 +} + # litclock-dev#528 + litclock-dev#636: force SSH off before the device leaves the owner's # hands, shared by BOTH handoff paths — gift mode (ships to a recipient) and # the non-gift factory-reset poweroff (the PWA copy invites "move or pass the @@ -353,10 +419,11 @@ while [[ $# -gt 0 ]]; do echo " /etc/litclock/.gift-language and seeded into env.sh" echo " (only meaningful with --gift-mode; litclock-dev#532)" echo "" - echo " --poweroff and --gift-mode DISABLE SSH before powering down, so a device" - echo " being handed on gets the same posture as a fresh flash (litclock-dev#528)." + echo " --poweroff and --gift-mode wipe and lock the shell history (litclock-dev#868)" + echo " and DISABLE SSH before powering down, so a device being handed on gets the" + echo " same posture as a fresh flash (litclock-dev#528). --keep-wifi skips neither." echo " To get back in: put a blank 'ssh' file in the SD card boot partition, or" - echo " use the console. See docs/recovery.md. --keep-wifi does NOT skip this." + echo " use the console. See docs/recovery.md." exit 1 ;; esac @@ -680,6 +747,8 @@ rm -f "$STATE_DIR/reset-failed" 2>/dev/null || true # above: a surviving memo is cosmetic, and aborting a reset over it would be # the wrong trade. rm -f "$STATE_DIR/runtime-render-validation.json" 2>/dev/null || true +# litclock-dev#871 Stage A: the self-test pass record is per-device too. +rm -f "$STATE_DIR/runtime-render-selftest.json" 2>/dev/null || true # Verify, don't assume (litclock-dev#673's lesson). A directory, a symlink, or a # read-only remount leaves the marker in place and `rm -f` still returns 0. This # WARNS rather than aborting: a stale warning marker is the fail-safe direction @@ -934,6 +1003,20 @@ if [[ "$GIFT_MODE" != "true" && "$WIPE_WIFI" == "true" ]]; then fi fi +# litclock-dev#868 — the previous owner's shell history goes with the key, on +# both handoff arms. One call, HERE, above Step 7, for the reason the rotation +# above sits here too — and a stronger one: a SIGHUP'd interactive bash SAVES +# its history on the way out, so a CLI run over SSH-on-WiFi that Step 7 kills +# would write the operator's session to the file at the exact moment the lock +# should already be there (litclock-dev#873 red team). Skipped when the gift arm is +# already going to abort on a failed env.sh wipe: that device stays with its +# owner, and a lock placed now would only mute their shell until the next +# boot. (--poweroff implies --strict-env-wipe, which aborted long before here.) +# Ordering rule and fatality rationale on clear_shell_history_for_handoff. +if [[ "$ENV_WIPE_FAILED" != "true" && ( "$GIFT_MODE" == "true" || "$DO_POWEROFF" == "true" ) ]]; then + clear_shell_history_for_handoff +fi + # Step 7: Optionally wipe saved WiFi networks for fresh-flash simulation. # Only deletes WiFi-type NetworkManager connection profiles — wired # ethernet, VPN (OpenVPN/WireGuard), bluetooth PAN, etc. live in the same @@ -1037,10 +1120,11 @@ if [[ "$GIFT_MODE" == "true" ]]; then # litclock-dev#528 — shared handoff gate; see disable_ssh_for_handoff above. # Deliberately AFTER the env-wipe-failed gate: on a failed prep the device # stays on and the owner may still need SSH to fix it. Also AFTER the - # rotation, for the same reason — that block fails CLOSED and can exit 1, - # which leaves the device with its CURRENT owner, and disabling SSH first - # would strip that owner's remote access on the exact path where they still - # need it to recover. SSH-off is the last thing before poweroff. + # rotation and the history wipe, for the same reason — those blocks fail + # CLOSED and can exit 1, which leaves the device with its CURRENT owner, and + # disabling SSH first would strip that owner's remote access on the exact + # path where they still need it to recover. SSH-off is the last thing + # before poweroff. disable_ssh_for_handoff # Marker was written earlier (pre-stop) so shutdown-splash has already diff --git a/scripts/update.sh b/scripts/update.sh index b247d02..c867d06 100755 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -19,6 +19,8 @@ # Phase 4 venv hash-gate → pip install if hash changed # │ # Phase 4.5 smoke: $PYTHON src/literary_clock.py --dry-run (60s hard timeout) +# (pass) then: runtime-render marker (re-)stamp, and the +# litclock-dev#871 self-test -> memo (both inert to the update's verdict) # │ ╲ # (pass) (fail) # ▼ ▼ @@ -106,8 +108,30 @@ if [[ "${LITCLOCK_UPDATE_LOCK_HELD:-0}" != "1" ]] && command -v flock >/dev/null # The inherited descriptor is the pre-existing behaviour and the # cleanup race is the worse of the two, so it stays. The leak side is # bounded elsewhere: the one gate that could leak a helper is under - # coreutils `timeout` now, and the lock design itself is the follow-up - # on litclock-dev#847 (item 2; it began on litclock-dev#835). + # coreutils `timeout` now. + # + # SETTLED, owner decision 2026-09-19 (litclock-dev#847 item 2, closed + # as accepted — do not reopen this as pending work). Two facts made + # the redesign not worth its risk on the highest-blast-radius script + # in the tree. EVERY production trigger activates the same unit — + # litclock-update.timer, the PWA's "update now" + # (routes/updates.py: `systemctl start --no-block`), and the + # bootcheck/LKG rollback (litclock-bootcheck.sh's UPDATE_TRIGGER_CMD, + # the same argv) — and systemd does not spawn a second process for a + # start request against an invocation that is already running, so all + # three are serialised before this lock is reached. (`Type=oneshot` + # with no RemainAfterExit: the unit is `activating` for the whole run, + # not `active`, and a start lands on that job rather than beside it.) + # What remains is the direct-script path — and it is NOT maintainer- + # only (litclock-dev#877 review): README's "Manual update (optional)" tells an + # owner to run `/home/pi/litclock/scripts/update.sh`, so this flock is + # load-bearing exactly there, which is the argument for leaving it + # alone rather than for redesigning it. The residual is a descendant + # that outlives such a run and holds the descriptor, cleared with a + # kill. A + # root-owned lock helper would close it properly and is the shape to + # reach for IF this ever bites in the field — new privileged surface + # here has to earn its place. flock -n -E 75 "$LITCLOCK_UPDATE_LOCK_FILE" "$0" "$@" _rc=$? if [[ "$_rc" == "75" ]]; then @@ -204,7 +228,9 @@ LEGACY_UPDATE_CHECK_CACHE_FILE="$STATE_DIR/update-check.json" # no new LKG was recorded yet. LAST_UPDATE_FILE="$STATE_DIR/last-update.json" # litclock-dev#847 item 1 — the NEGATIVE-RESULT MEMO for the litclock-dev#531 -# runtime-render validation. Phase 4.5's KEEP arm writes it when the check is +# runtime-render validation — and, since litclock-dev#871 Stage A, the +# runtime-render SELF-TEST's verdict (`selftest-failed` with the painter's rc, +# or `selftest-deferred`). Phase 4.5's KEEP arm writes it when the check is # DEFERRED (not enough of this run's systemd budget left), TIMES OUT or FAILS, # and removes it when the check passes or the marker is already present. # JSON, one object: {result, rc, reason, sha, at_unix}. Persistent (SD-backed, @@ -216,6 +242,14 @@ LAST_UPDATE_FILE="$STATE_DIR/last-update.json" # error from a half-built wheel, and skipping on the second would strand a # device on the PNG tier until the next release. RUNTIME_VALIDATION_MEMO_FILE="$STATE_DIR/runtime-render-validation.json" +# litclock-dev#871 Stage A — the POSITIVE record the memo above cannot carry: +# {result: passed, duration_s, sha, at_unix}, written by the self-test on a +# pass and removed on a fail. The journal is capped at 7 days (the weekly tick +# cadence) and a tick with nothing new exits before Phase 4.5, so a pass logged +# only there is gone before Stage B can read it; and "no memo" is not "passed" +# (a reset or a rollback tick removes the memo without re-asking). Stage B +# gates the flag flip on THIS file, sha-matched (litclock-dev#875 red team). +RUNTIME_SELFTEST_RECORD_FILE="$STATE_DIR/runtime-render-selftest.json" # litclock-dev#845 — where the revert arms re-install the clock units from # (_reinstall_clock_units_from_tree). Overridable only so the executed tests # can point the arms at a fake directory; Phase 5's loop names @@ -585,9 +619,10 @@ _LS_REMOTE_TIMEOUT_S="${LITCLOCK_LS_REMOTE_TIMEOUT_S:-30}" # child is gone and never reaches the -k KILL) — that helper is not # timeout's to kill. git waits for its helpers and git-remote-https honours # TERM, so both shapes are synthetic here; the consequence that would -# matter (a leaked descendant holding the update lock) is the lock-design -# follow-up on litclock-dev#847 (item 2; this residual is its item 3), see -# the flock comment at the top. +# matter (a leaked descendant holding the update lock) was accepted with the +# lock design itself (litclock-dev#847 items 2 and 3, closed 2026-09-19 — +# both accepted, neither pending). See the flock comment at the top for why +# the redesign was judged not worth its risk. _remote_reachable() { timeout -k 5 "$_LS_REMOTE_TIMEOUT_S" git ls-remote --exit-code origin &>/dev/null } @@ -656,7 +691,30 @@ update_status_init "$OLD_SHA" # than a stopped timer, and the status file already says manual # recovery is needed. It is NOT covered by the bootcheck/LKG chain — # that chain asks "did this BOOT paint once", and a mid-uptime update -# has always painted before it ran (litclock-dev#847 item 6). +# has always painted before it ran. +# +# RECOVERY IS BOOT-SCOPED BY DESIGN. Owner decision 2026-09-19 +# (litclock-dev#847 item 6, closed as accepted — not pending work). +# The timer already re-polls every 5 minutes, so the schedule was never +# the missing piece; the predicate is ("heartbeat since boot", not +# "heartbeat fresh"). But swapping the predicate alone would NOT be the +# one-line change that sounds like (litclock-dev#877 review): bootcheck's +# sub-threshold action is `systemctl reboot`, so freshness would first +# produce a REBOOT loop, and `boot-fail-count` counts failed BOOTS — at +# THRESHOLD=3 it would silently come to mean "three consecutive 5-minute +# polls", i.e. revert after ~15 minutes of no paint. Three deliberate +# pieces, not one. On a fleet that is mostly +# unreachable gifts, a clock that reverts itself on a fault that is not +# the code's is worse than one that holds its last frame on a bistable +# panel while the status file asks for a human. That retention is the +# COMMON shape, not a guarantee (litclock-dev#877 review): a painter that dies at +# import or mid-render never reaches the hardware, so the frame stands — +# but `epd.Clear()` runs before `epd.display()` on the nightly +# DISPLAY_CLEAR_HOUR pass, so a failure in that one-minute-a-day window, +# after the clear and before the display, leaves the panel BLANK until +# someone intervenes. Narrow, and it does not change the decision; +# it is the honest bound on it. Revisit with evidence of a real +# mid-uptime painter failure in the field. _LITCLOCK_UPDATE_FINALIZED=0 _LITCLOCK_UPDATE_CLEANED=0 # The one cleanup, IDEMPOTENT, reachable from both the EXIT trap and the @@ -1253,7 +1311,10 @@ unset _PHASE3_ADDED_FILE # re-validates what pi passes — not a broader sudoers line. (The timer-start # grants 020 DOES carry name a fixed unit with no path argument, so they # hand pi nothing it can redirect; that is the scoped default for anything -# that does not need the blanket grant.) Tracked on litclock-dev#847. +# that does not need the blanket grant.) Recorded on litclock-dev#847, which +# closed 2026-09-19: this is a description of a settled arrangement, not a +# scheduled task — the `010` drop it was once contingent on was itself +# reversed (litclock-dev#387). # # Returns non-zero when a unit still differs after the attempt, so the arms # can say so loudly. Never fatal: the arm's own `exit 1` and status stamp are @@ -1656,6 +1717,11 @@ VALIDATOR_TIMEOUT_S=300 # 2W (litclock-dev#835); the reserve is generous because an update that runs # out of budget here loses the stamp, not just the validation. VALIDATOR_BUDGET_RESERVE_S=120 +# litclock-dev#871 Stage A — bound on the runtime-render SELF-TEST (a dry-run +# with the text renderer forced on). The smoke gate's own dry-run is bounded +# at 60s and this is the same render plus ~1.3s of freetype prep, so the same +# bound; it must fit the budget behind the validator's reserve. +SELFTEST_TIMEOUT_S=60 # The budget systemd LOADED for this unit, in whole seconds, on stdout # (0 = no limit); non-zero return when unknown. Read as the raw D-Bus @@ -1768,8 +1834,11 @@ _block_reverted_release() { } # litclock-dev#847 item 1 — write the negative-result memo (see -# RUNTIME_VALIDATION_MEMO_FILE). $1 result: deferred|timeout|failed; -# $2 the validator's rc, or empty when it never ran; $3 the reason, prose. +# RUNTIME_VALIDATION_MEMO_FILE). $1 result: deferred|timeout|failed, or +# selftest-failed|selftest-deferred (litclock-dev#871 Stage A); $2 the +# validator's or the self-test painter's rc, or empty when it never ran; $3 +# the reason, prose. src/control_server/routes/status.py's +# RUNTIME_VALIDATION_RESULTS must list every token written here. # jq builds the object so the reason cannot break the JSON, and the file # lands via the same atomic writer as every other state marker. Best-effort: # a failed memo is a warning, never an update failure. @@ -1809,9 +1878,14 @@ _runtime_validation_memo_write() { # One deferral: the log line the operator reads, then the memo. The # reason is the log text minus the fixed prefix, so the journal and the # memo cannot disagree. Returns 1, the guard's own "does not fit" answer. +# $1 reason; $2 memo token (default `deferred`, the validator's); $3 what +# is being deferred (default the validator) — litclock-dev#871's self-test +# passes its own token and label so the PWA can tell the two apart by +# `result` alone. _defer_runtime_validation() { - log_info "deferring runtime-render validation to the next update: $1 (litclock-dev#835)" - _runtime_validation_memo_write deferred "" "$1" + local token="${2:-deferred}" label="${3:-runtime-render validation}" + log_info "deferring $label to the next update: $1 (litclock-dev#835)" + _runtime_validation_memo_write "$token" "" "$1" return 1 } @@ -1821,17 +1895,26 @@ _defer_runtime_validation() { # (litclock-dev#847 item 1): a device that NEVER has budget never validates # and stays on the PNG tier — the safe direction — and the memo is how that # stops being invisible. +# $1 seconds the work needs (default: the validator's bound); $2 memo token +# and $3 label are passed through to the deferral. litclock-dev#871's self-test +# calls this with its own bound and token. The validator's default deliberately +# does NOT reserve for the self-test that follows it (litclock-dev#875 red team): on the +# transition tick (a 600s unit) that would defer the one step that changes +# device behaviour — earning the marker — to protect an INERT probe that has +# its own guard and loses nothing by waiting. Stamp now, defer the self-test. _validation_fits_remaining_budget() { + local needed="${1:-$VALIDATOR_TIMEOUT_S}" token="${2:-deferred}" + local label="${3:-runtime-render validation}" local elapsed budget rc elapsed=$(_update_elapsed_seconds); rc=$? if [[ $rc -eq 2 ]]; then return 0 elif [[ $rc -ne 0 ]]; then - _defer_runtime_validation "cannot determine how much of this run's systemd budget is left" + _defer_runtime_validation "cannot determine how much of this run's systemd budget is left" "$token" "$label" return 1 fi if ! budget=$(_update_budget_seconds); then - _defer_runtime_validation "cannot read the unit's effective TimeoutStartSec" + _defer_runtime_validation "cannot read the unit's effective TimeoutStartSec" "$token" "$label" return 1 fi # 0 here means UNLIMITED (the helper maps infinity to 0 and rounds a @@ -1839,14 +1922,119 @@ _validation_fits_remaining_budget() { # hold this run at all, and reaching Phase 4.5 under one is impossible, # so treating the two alike is harmless in that direction only). [[ "$budget" -eq 0 ]] && return 0 - if (( elapsed + VALIDATOR_TIMEOUT_S + VALIDATOR_BUDGET_RESERVE_S > budget )); then - _defer_runtime_validation "${elapsed}s of this run's ${budget}s budget are gone and the check needs up to ${VALIDATOR_TIMEOUT_S}s plus a ${VALIDATOR_BUDGET_RESERVE_S}s reserve for the phases after it, with litclock.timer stopped" + if (( elapsed + needed + VALIDATOR_BUDGET_RESERVE_S > budget )); then + _defer_runtime_validation "${elapsed}s of this run's ${budget}s budget are gone and the check needs up to ${needed}s plus a ${VALIDATOR_BUDGET_RESERVE_S}s reserve for the phases after it, with litclock.timer stopped" "$token" "$label" return 1 fi return 0 } # --- litclock-dev#835 budget helpers END --- +# litclock-dev#871 Stage A — the runtime-render SELF-TEST, shipped INERT. +# +# The marker says this device's freetype reproduces the GD measurement dump; +# it does not say this device can paint a quote from text. Between the two sit +# the corpus resolution (litclock-dev#870), the fonts on disk, the renderer's +# own guards, and every reason `_runtime_render_enabled()` has to decline — +# and today every one of them degrades SILENTLY to the PNG tier, so a fleet +# device could carry a valid marker and still never render a line of text. +# Stage B (release N+1) will flip LITCLOCK_RUNTIME_RENDER on devices whose +# self-test passes; this release only asks the question and records the answer. +# +# The same dry-run the smoke gate runs, with the renderer FORCED on (env.sh's +# flag says what the owner chose, not what the device can do) and +# --require-runtime-render, which makes the painter exit 3 when the frame came +# from anywhere but the text tier. env.sh is sourced in the subshell so the +# render uses the DEVICE's language and marker path — the smoke gate pins +# LITCLOCK_LANGUAGE=en for its catalog probes, but the point here is whether +# THIS device's corpus renders (litclock-dev#870 review). Two things env.sh must NOT +# bring along: weather is forced OFF (a capability probe has no business on +# the network, and the fetch could spend the bound before a glyph is drawn — +# litclock-dev#875 review), and the frame goes to a throwaway directory, because a dry-run +# with the renderer on persists its frame and /run/litclock/current-quote.png +# is the panel's current one. No scratch directory, no run (litclock-dev#875 review: the +# painter's default IS the panel's directory). +# +# NEVER fails the update. A failed self-test means "stay on PNGs", which is +# where the device already is. The verdict lands in the litclock-dev#847 memo: +# `selftest-failed` with the painter's rc (3 = fell back, 124 = timed out, +# anything else = the painter died), or `selftest-deferred` when the budget or +# the scratch directory is gone. A pass writes the POSITIVE record +# (RUNTIME_SELFTEST_RECORD_FILE) with its DURATION: Stage B needs a durable +# on-device pass, sha-matched, and needs to know the device renders inside +# the 4s lead, not merely that it renders (litclock-dev#875 review + red team). +# One sample, one minute, one random row: a corpus gap at that minute reads as +# a fallback too, and the painter now says so in its own [selftest] line, so +# a `selftest-failed` is "did not render text this time", never proof of +# incapacity — Stage B acts only on a PASS. +# The pass record (RUNTIME_SELFTEST_RECORD_FILE). $1 duration in seconds, one +# decimal. Same jq/atomic/0644 shape as the memo writer; best-effort. +_runtime_selftest_record_write() { + local duration="$1" sha json + if ! command -v jq >/dev/null 2>&1; then + log_warn "jq missing — not recording the runtime-render self-test pass (litclock-dev#871)" + return 0 + fi + sha=$(git rev-parse HEAD 2>/dev/null || echo "") + json=$(jq -nc --arg d "$duration" --arg sha "$sha" --arg at "$(date +%s 2>/dev/null || echo 0)" \ + '{result: "passed", duration_s: ($d | tonumber), sha: ($sha | if . == "" then null else . end), at_unix: ($at | tonumber)}' 2>/dev/null) || json="" + if [[ -z "$json" ]] || ! atomic_write_file "$RUNTIME_SELFTEST_RECORD_FILE" "$json"; then + log_warn "Could not write $RUNTIME_SELFTEST_RECORD_FILE — Stage B will not see this pass (litclock-dev#871)" + return 0 + fi + chmod 0644 "$RUNTIME_SELFTEST_RECORD_FILE" 2>/dev/null || sudo chmod 0644 "$RUNTIME_SELFTEST_RECORD_FILE" 2>/dev/null || true + chown pi:pi "$RUNTIME_SELFTEST_RECORD_FILE" 2>/dev/null || sudo chown pi:pi "$RUNTIME_SELFTEST_RECORD_FILE" 2>/dev/null || true + return 0 +} + +_runtime_render_selftest() { + local rc dir t0 dur_ms duration + _validation_fits_remaining_budget "$SELFTEST_TIMEOUT_S" selftest-deferred "the runtime-render self-test" || return 0 + if ! dir=$(mktemp -d 2>/dev/null) || [[ -z "$dir" ]]; then + log_info "deferring the runtime-render self-test to the next update: could not create a scratch directory (litclock-dev#871)" + _runtime_validation_memo_write selftest-deferred "" "self-test: could not create a scratch directory (mktemp -d failed)" + return 0 + fi + log_info "Running the runtime-render self-test (litclock-dev#871 Stage A; inert — records a verdict, changes nothing)..." + # Microseconds via EPOCHREALTIME (bash 5): the figure is compared against a + # 4s render lead, and whole-second $SECONDS is ±1s on it (litclock-dev#875 red team). + t0=${EPOCHREALTIME/./} + # Subshell: env.sh must not leak into this script (the same isolation the + # RUNTIME_MARKER resolution above uses). PIPESTATUS, not the pipeline's + # exit — `if cmd | sed; then` tests sed (the smoke gate's own lesson). + ( + [[ -f "${INSTALL_DIR:-}/env.sh" ]] && source "${INSTALL_DIR:-}/env.sh" 2>/dev/null + export LITCLOCK_RUNTIME_RENDER=true + export LITCLOCK_RUNTIME_RENDER_DIR="$dir" + export WEATHER_ENABLED=false + timeout "$SELFTEST_TIMEOUT_S" "$PYTHON" src/literary_clock.py --dry-run --require-runtime-render 2>&1 + ) | sed 's/^/[selftest] /' + rc="${PIPESTATUS[0]}" + dur_ms=$(( (${EPOCHREALTIME/./} - t0) / 1000 )) + duration="$((dur_ms / 1000)).$((dur_ms % 1000 / 100))" + # Only ever the fresh mktemp directory: -d, and never a fallback path. + [[ -n "$dir" && -d "$dir" ]] && rm -rf -- "$dir" 2>/dev/null + if [[ "$rc" -eq 0 ]]; then + log_info "runtime-render self-test PASSED in ${duration}s — this device renders text (litclock-dev#871 Stage A; LITCLOCK_RUNTIME_RENDER is unchanged this release)" + _runtime_selftest_record_write "$duration" + return 0 + fi + # A fail retires any earlier pass: Stage B must never flip on a stale one. + atomic_remove_file "$RUNTIME_SELFTEST_RECORD_FILE" + if [[ "$rc" -eq 124 ]]; then + log_info "runtime-render self-test did not finish within ${SELFTEST_TIMEOUT_S}s (${duration}s elapsed) — staying as is (this is not an update failure)" + _runtime_validation_memo_write selftest-failed "$rc" "the self-test did not finish within ${SELFTEST_TIMEOUT_S}s" + elif [[ "$rc" -eq 3 ]]; then + log_info "runtime-render self-test did not pass: the painter fell back to pre-rendered images after ${duration}s — staying as is (this is not an update failure; the [selftest] lines above say why)" + _runtime_validation_memo_write selftest-failed "$rc" "the painter fell back to pre-rendered images with the text renderer forced on" + else + log_info "runtime-render self-test did not pass (rc=$rc) — staying as is (this is not an update failure)" + _runtime_validation_memo_write selftest-failed "$rc" "src/literary_clock.py --dry-run --require-runtime-render exited $rc" + fi + return 0 +} + + if [[ "$smoke_rc" -eq 0 ]]; then log_info "Smoke test passed" atomic_remove_file "$UPDATE_FAILED_FILE" @@ -1889,7 +2077,10 @@ if [[ "$smoke_rc" -eq 0 ]]; then # item 1): the device validated since — by a later tick, or by an # operator running `check --stamp` by hand — and a memo left behind would # have the PWA report a failure the marker contradicts. - if [[ -f "$RUNTIME_MARKER" ]]; then + # Not in ROLLBACK_MODE (litclock-dev#875 red team): the self-test below does not run + # there, so a `selftest-failed` verdict the memo carries would be erased + # without being re-asked, and Stage B would read the absence as a pass. + if [[ -f "$RUNTIME_MARKER" && "${ROLLBACK_MODE:-0}" -ne 1 ]]; then atomic_remove_file "$RUNTIME_VALIDATION_MEMO_FILE" fi if [[ ! -f "$RUNTIME_MARKER" && -x "$PYTHON" && "${ROLLBACK_MODE:-0}" -ne 1 ]] && _validation_fits_remaining_budget; then @@ -1933,6 +2124,12 @@ if [[ "$smoke_rc" -eq 0 ]]; then fi unset _validate_rc fi + # litclock-dev#871 Stage A — see _runtime_render_selftest. Only with a + # marker (surviving, or just stamped above): without one the painter + # declines before it tries. Not in ROLLBACK_MODE, for the reason above. + if [[ -f "$RUNTIME_MARKER" && -x "$PYTHON" && "${ROLLBACK_MODE:-0}" -ne 1 ]]; then + _runtime_render_selftest + fi else log_error "Smoke test failed (exit $smoke_rc) — reverting to $REVERT_SHA" # REVERT_SHA == OLD_SHA normally; in bootcheck rollback mode it is the diff --git a/src/control_server/routes/status.py b/src/control_server/routes/status.py index 21b55d7..1c295f9 100644 --- a/src/control_server/routes/status.py +++ b/src/control_server/routes/status.py @@ -74,15 +74,22 @@ PHASE3_SKIP_FRESH_WINDOW_S = 86400 # litclock-dev#847 item 1 — the negative-result memo update.sh Phase 4.5 -# writes when the litclock-dev#531 runtime-render validation is deferred, times out or -# fails, and removes when it passes (or the marker is already present). JSON: +# writes when the litclock-dev#531 runtime-render validation is deferred, times +# out or fails, and removes when it passes (or the marker is already present) — +# and, since litclock-dev#871 Stage A, the runtime-render self-test's verdict. JSON: # {result, rc, reason, sha, at_unix}. No freshness clamp, unlike the Phase 3 # marker: it describes the device's CURRENT tier, not a one-off skip, and the # writer clears it on the state change that makes it stale. DEFAULT_RUNTIME_VALIDATION_MEMO_FILE = os.environ.get( "LITCLOCK_RUNTIME_VALIDATION_MEMO_FILE", "/var/lib/litclock/runtime-render-validation.json" ) -RUNTIME_VALIDATION_RESULTS = frozenset({"deferred", "timeout", "failed"}) +# `selftest-failed` / `selftest-deferred` since litclock-dev#871 Stage A: the +# same memo file carries the runtime-render SELF-TEST's verdict (a forced-on +# dry-run that fell back to PNGs, timed out, or died — the rc says which), or +# its deferral (budget or scratch directory gone). Distinct tokens, so a +# marker-bearing device whose validation passed is never reported as +# "validation deferred" (litclock-dev#875 review). +RUNTIME_VALIDATION_RESULTS = frozenset({"deferred", "timeout", "failed", "selftest-failed", "selftest-deferred"}) MAX_RUNTIME_VALIDATION_MEMO_BYTES = 8 * 1024 # litclock-dev#274 follow-up — adversarial-review P1: budget for treating a @@ -573,7 +580,7 @@ def collect_status( "update_phase_index": update_phase, # litclock-dev#847 item 1: null when the last runtime-render validation # passed (or never ran and was never deferred); otherwise - # {result: deferred|timeout|failed, at_unix, rc, reason, sha}. The PWA + # {result: , at_unix, rc, reason, sha}. The PWA # does not render it yet — a Diagnostics/Status surface is the # follow-up; the payload is the contract. "runtime_render_validation": runtime_render_validation, diff --git a/src/literary_clock.py b/src/literary_clock.py index 289bbe5..5446933 100644 --- a/src/literary_clock.py +++ b/src/literary_clock.py @@ -729,10 +729,12 @@ def get_current_quote_runtime( logging.warning(f"runtime render enabled but renderer unavailable ({e}); using images") return None try: - # csv_path=None: quote_corpus's env-aware default, so the renderer - # and lookup_by_filename can never read different corpora (litclock-dev#594 - # review — CORPUS_CSV ignored LITCLOCK_CORPUS_CSV and its spelling - # keyed a second cache entry for the same file). + # csv_path=None: quote_corpus.corpus_path() — the active language's + # registry corpus (litclock-dev#870); LITCLOCK_CORPUS_CSV still + # overrides it, and a local constant here was the litclock-dev#594 bug + # (it ignored that override and keyed a second cache entry for the same + # file). The PNG lookup below reads quote_corpus.image_corpus_path() on + # purpose — the corpus the images were baked from. rows = quote_renderer.rows_for_time(None, now.strftime("%H%M")) except Exception as e: logging.error(f"runtime render: corpus read failed: {e}") @@ -740,7 +742,14 @@ def get_current_quote_runtime( if not allow_nsfw: rows = [r for r in rows if not r.is_nsfw] if not rows: - return None # corpus gap — identical outcome to the PNG-glob miss + # Corpus gap — identical outcome to the PNG-glob miss. SAID, since + # litclock-dev#871: the self-test reads a fallback as "did not render + # text", and without this line the journal could not tell a gap at + # this minute from a renderer that cannot render at all. + logging.warning( + f"runtime render: no rows for {now.strftime('%H:%M')} after the NSFW filter; using pre-rendered images" + ) + return None row = rows[randrange(len(rows))] # ONE guard around render+validate+convert+persist: the "returns None # on ANY failure" contract must hold for unexpected shapes too, not @@ -1058,7 +1067,18 @@ def _stamp_update_failed_glyph(image, draw): "the update so the clock never gets bricked by a bad release." ), ) + parser.add_argument( + "--require-runtime-render", + action="store_true", + help=( + "With --dry-run: exit 3 unless the frame came from the on-device text renderer " + "(render_mode == 'runtime'). The litclock-dev#871 Stage A self-test — a dry-run that " + "silently fell back to a pre-rendered PNG must not count as 'this device renders text'." + ), + ) args = parser.parse_args() + if args.require_runtime_render and not args.dry_run: + parser.error("--require-runtime-render only means something with --dry-run") if args.dry_run: # Smoke test path: exercise image composition end-to-end (fonts, corpus, @@ -1081,7 +1101,26 @@ def _stamp_update_failed_glyph(image, draw): if image is None: logging.error("dry-run: main() returned None image") sys.exit(1) - logging.info("dry-run OK: rendered %sx%s image", image.size[0], image.size[1]) + # litclock-dev#871 Stage A: say WHICH tier produced the frame, and let + # the self-test insist on the text renderer. main() degrades to the + # PNG tier on every runtime failure (marker missing, freetype unusable, + # digest stale, corpus gap, render error) and returns a perfectly good + # image, so exit 0 alone cannot tell "this device renders text" from + # "this device fell back" — which is the whole question the self-test + # asks. `time-only` is the no-quote fallback (quote_meta None). + render_mode = result[1].get("render_mode") if result[1] else "time-only" + # stdout, not the log: setup_logging() defaults to WARNING unless + # LOG_LEVEL is set, so an INFO line is normally invisible — and the + # self-test's verdict is the one line of a dry-run anyone reads + # (`[selftest] dry-run: render_mode=runtime` in the update journal). + print(f"dry-run: rendered {image.size[0]}x{image.size[1]} image, render_mode={render_mode}", flush=True) + if args.require_runtime_render and render_mode != "runtime": + logging.error( + "dry-run: --require-runtime-render but the frame came from render_mode=%s — " + "the text renderer did not produce it (see the warning above for why)", + render_mode, + ) + sys.exit(3) sys.exit(0) # Register signal handlers for graceful shutdown diff --git a/src/quote_corpus.py b/src/quote_corpus.py index 494f2f7..1de2ca9 100644 --- a/src/quote_corpus.py +++ b/src/quote_corpus.py @@ -9,6 +9,12 @@ the forward direction too: ``bucket_entries()`` feeds the runtime renderer's per-minute row selection (``quote_renderer.rows_for_time``). +Which CSV (litclock-dev#870): the forward direction reads the ACTIVE +LANGUAGE's corpus from ``languages.json`` (``corpus_path``); the inverse +reads the corpus the PNGs were baked from — the shipped default the PHP +generator opens by name (``image_corpus_path``). ``LITCLOCK_CORPUS_CSV`` +overrides both. + Both directions share one walk, one cache, and ONE text pipeline (``preprocess_quote``): the eng review of litclock-dev#590 found the previous split — ``preprocess_quote`` on the e-ink path vs a local outer-quote @@ -41,18 +47,128 @@ from __future__ import annotations import csv +import logging import os import re from functools import lru_cache from pathlib import Path +import strings_catalog # acyclic: strings_catalog imports nothing from this package + +logger = logging.getLogger(__name__) + _PROJECT_ROOT = Path(__file__).resolve().parents[1] -_CORPUS_PATH = Path( - os.environ.get( - "LITCLOCK_CORPUS_CSV", - str(_PROJECT_ROOT / "image-gen" / "litclock_annotated.csv"), - ) -) +# The corpus the repo ships AND the one the PHP generator reads by name +# (`image-gen/quote_to_image.php` opens `litclock_annotated.csv` directly, and +# `scripts/download_images.sh` verifies the image set against the same file). +# It is therefore the provenance of every PNG under images/ — which is why +# image_corpus_path() returns it — and the LAST rung of corpus_path(), never +# the first (litclock-dev#870). +_DEFAULT_CORPUS_PATH = _PROJECT_ROOT / "image-gen" / "litclock_annotated.csv" +# Highest-precedence override for BOTH resolvers: tooling and tests point it at +# a synthetic CSV (`monkeypatch.setattr(quote_corpus, "_CORPUS_PATH", ...)` is +# the established seam). ``None`` means "resolve through the registry". It +# says "this file is the whole corpus, images included", so it is NOT the way +# to bench-test a translation — that redirects the PNG metadata too (litclock-dev#874 +# review); register the translation in languages.json and set LITCLOCK_LANGUAGE. +_CORPUS_PATH: Path | None = Path(os.environ["LITCLOCK_CORPUS_CSV"]) if os.environ.get("LITCLOCK_CORPUS_CSV") else None + + +def _registry_corpus(code: str) -> Path | None: + """``languages.json[code].corpus.path`` as a usable file inside the + checkout, else ``None`` — with one warning per code per PROCESS for each + way it can be unusable, because each is a packaging error the clock must + paint through. (The painter is a fresh process every minute, so on a + device with a broken registry entry that is one line per paint — the + rate every other painter warning already has; the dedup is for + long-lived importers.) Contained on purpose (litclock-dev#874 review): ``root / rel`` discards the + root for an absolute ``rel`` and follows ``..`` and symlinks, so a + registry typo could name any pi-readable file and serve its lines as + quote text; a resolved path outside the checkout is refused. A zero-byte + file is refused too, so an empty translation cannot shadow a healthy + English corpus. A file that exists but does not parse is NOT caught here + (that costs a full read per minute) — it falls to the PNG tier exactly + as a corrupt English corpus always has. + """ + rel = strings_catalog.corpus_relpath(code) + if not rel: + return None + candidate = _PROJECT_ROOT / rel + try: + # Inside one guard: resolve() and is_file() raise on EACCES rather than + # answering False, resolve() raises RuntimeError on a symlink loop and + # ValueError on a NUL byte in the registry string (litclock-dev#874 review, red + # team) — and the painter must never die here. + root = _PROJECT_ROOT.resolve() + real = candidate.resolve() + if not real.is_relative_to(root): + strings_catalog.warn_once( + f"corpus-escape:{code}", + "languages.json corpus %r for %r resolves outside the checkout; ignored", + rel, + code, + ) + return None + usable = real.is_file() and real.stat().st_size > 0 + except (OSError, RuntimeError, ValueError): + usable = False + if not usable: + strings_catalog.warn_once( + f"corpus-missing:{code}", "languages.json names corpus %r for %r but it is missing or empty", rel, code + ) + return None + # The plain join, not the resolved path: both accessors then spell a path + # the same way (_index realpaths for the cache key regardless). + return candidate + + +def corpus_path() -> Path: + """The corpus the RUNTIME RENDERER reads: the active language's registry + corpus (litclock-dev#870, unblocking litclock-dev#532 Stage 4). + + Precedence: the ``LITCLOCK_CORPUS_CSV`` override -> ``languages.json``'s + ``corpus.path`` for ``strings_catalog.active_language()`` -> the English + registry corpus -> the shipped default. Degrades the way the strings + catalog does: an unknown or inactive code is already English by the time + it reaches here, and a registry entry that is missing, empty or outside + the checkout falls through with a warning (once per process — see + ``_registry_corpus``). A clock must never stop painting over a registry + typo. + + Resolved on EVERY call, on purpose: the index cache is keyed by path, so + a language change reaches any long-lived importer on its next lookup — + the ``lru_cache`` trap the issue named, where a module-level resolution + would pin the first language's corpus for the life of the process. (No + such importer exists today — see the module docstring — and the registry + itself is cached for process lifetime by ``strings_catalog``, so a + registry EDIT, as opposed to a language change, needs a restart there.) + """ + if _CORPUS_PATH is not None: + return _CORPUS_PATH + code = strings_catalog.active_language() + for candidate_code in dict.fromkeys((code, strings_catalog.CANONICAL_LANGUAGE)): + resolved = _registry_corpus(candidate_code) + if resolved is not None: + return resolved + return _DEFAULT_CORPUS_PATH + + +def image_corpus_path() -> Path: + """The corpus the PRE-RENDERED PNGs were baked from: the shipped default, + whatever the device language and whatever the registry says (litclock-dev#874 review). + + ``lookup_by_filename`` inverts the PHP namer, and the namer walked ONE + file, by name — ``image-gen/litclock_annotated.csv`` — so that file is the + provenance of every PNG under ``images/``, not any registry entry: a + registry that repointed English's ``corpus.path`` must not move the PNG + metadata off the images it describes. ``images/`` has no language + dimension, so a device on a second language while still on the PNG tier + paints these PNGs and gets their metadata from here, not from its own + corpus. (That such a device paints the wrong language at all is why + runtime render is the multilingual enabler, litclock-dev#871.) + """ + return _CORPUS_PATH if _CORPUS_PATH is not None else _DEFAULT_CORPUS_PATH + # Image filename forms generated by quote_to_image.php: # quote_{HHMM}_{idx}.png (image) @@ -180,7 +296,7 @@ def _current_stat(path: Path) -> tuple[int, int]: def _index(csv_path: str | os.PathLike | None = None) -> dict[str, list[dict]]: - path = _CORPUS_PATH if csv_path is None else Path(csv_path) + path = corpus_path() if csv_path is None else Path(csv_path) # realpath: symlinked installs and env-var overrides must key ONE # cache entry per underlying file, not one per spelling of its path. path = Path(os.path.realpath(path)) @@ -193,7 +309,7 @@ def bucket_entries(hhmm: str, csv_path: str | os.PathLike | None = None) -> tupl selection pool — ``quote_renderer.rows_for_time`` builds its ``CorpusRow`` objects from this). Entries carry ``quote_raw``; apply ``preprocess_quote`` to the rows actually used. ``csv_path=None`` - means the production corpus. + means ``corpus_path()`` — the active language's corpus (litclock-dev#870). Returns a tuple so callers cannot reorder/extend the cached bucket; the entry dicts themselves are still the SHARED cached objects — @@ -228,7 +344,9 @@ def lookup_by_filename(filename: str) -> dict[str, str] | None: idx = int(m.group("idx")) filename_is_nsfw = m.group("nsfw") is not None entry = None - for e in _index().get(hhmm, []): + # The corpus the PNGs were baked from, never the device language's + # (see image_corpus_path). + for e in _index(image_corpus_path()).get(hhmm, []): if e["idx"] == idx: entry = e if entry is None or entry["is_nsfw"] != filename_is_nsfw: @@ -245,5 +363,6 @@ def lookup_by_filename(filename: str) -> dict[str, str] | None: def reset_cache() -> None: """Test hook — clears the lru_cache so each test sees a fresh index. Without this, tests that monkeypatch the corpus path will see the - stale index from the first call.""" + stale index from the first call. (The warn-once memory lives in + ``strings_catalog``; its ``reset_cache`` clears that.)""" _index_for.cache_clear() diff --git a/src/quote_renderer.py b/src/quote_renderer.py index 0eaa030..283fef9 100644 --- a/src/quote_renderer.py +++ b/src/quote_renderer.py @@ -508,9 +508,10 @@ def iter_corpus( def rows_for_time(csv_path: str | os.PathLike | None, hhmm: str) -> list[CorpusRow]: """All corpus rows for one HHMM bucket, in file order (the runtime selection pool — mirrors the clock's PNG glob semantics). - ``csv_path=None`` means quote_corpus's default corpus, which honors - ``LITCLOCK_CORPUS_CSV`` — production passes None so the renderer and - the PWA lookup can never read different corpora. + ``csv_path=None`` means ``quote_corpus.corpus_path()`` — the active + language's registry corpus (litclock-dev#870), still overridable by + ``LITCLOCK_CORPUS_CSV``; production passes None. The PWA's PNG lookup + reads ``quote_corpus.image_corpus_path()`` instead, on purpose. litclock-dev#590: served from ``quote_corpus``'s mtime-cached index instead of a full ``iter_corpus`` walk — the walk re-parsed and diff --git a/src/strings_catalog.py b/src/strings_catalog.py index 5c3ed1b..d9d65d1 100644 --- a/src/strings_catalog.py +++ b/src/strings_catalog.py @@ -75,7 +75,9 @@ _warned: set[str] = set() -def _warn_once(subject: str, fmt: str, *args: Any) -> None: +def warn_once(subject: str, fmt: str, *args: Any) -> None: + """Log ``fmt`` once per distinct ``subject`` for the process (public since + litclock-dev#870 — ``quote_corpus`` shares it; ``reset_cache`` clears the memory).""" with _lock: if subject in _warned: return @@ -88,7 +90,7 @@ def _load_json(path: Path) -> dict[str, Any] | None: with open(path, encoding="utf-8") as f: data = json.load(f) except (OSError, json.JSONDecodeError, UnicodeDecodeError) as exc: - _warn_once(f"load:{path}", "strings catalog unreadable at %s: %s", path, exc) + warn_once(f"load:{path}", "strings catalog unreadable at %s: %s", path, exc) return None return data if isinstance(data, dict) else None @@ -165,7 +167,7 @@ def active_language() -> str: return code entry = (_registry().get("languages") or {}).get(code) if not isinstance(entry, dict) or entry.get("status") != "active": - _warn_once( + warn_once( f"lang:{code}", "%s=%r is not an active registry language; using English", ENV_KEY, @@ -231,13 +233,27 @@ def get(key: str, /, **slots: Any) -> str: if template is None and code != CANONICAL_LANGUAGE: template = _catalog(CANONICAL_LANGUAGE).get(key) if template is not None: - _warn_once(f"miss:{code}:{key}", "catalog key %r missing for %r; served English", key, code) + warn_once(f"miss:{code}:{key}", "catalog key %r missing for %r; served English", key, code) if template is None: - _warn_once(f"miss:en:{key}", "catalog key %r missing entirely; serving the key", key) + warn_once(f"miss:en:{key}", "catalog key %r missing entirely; serving the key", key) template = key return _fill(template, slots) +def corpus_relpath(code: str) -> str | None: + """The registry's ``corpus.path`` for ``code`` (relative to the repo root), + or ``None`` when the code is unlisted or the entry has no usable path. + Public because ``quote_corpus.corpus_path`` resolves the quote corpus + through it (litclock-dev#870); the precedence and degrade order live there. + """ + entry = (_registry().get("languages") or {}).get(code) + if not isinstance(entry, dict): + return None + corpus = entry.get("corpus") + rel = corpus.get("path") if isinstance(corpus, dict) else None + return rel if isinstance(rel, str) and rel else None + + def active_languages() -> dict[str, dict[str, Any]]: """Registry entries with ``status == "active"`` (litclock-dev#532 pickers).""" langs = _registry().get("languages") or {} @@ -346,7 +362,7 @@ def get_many(keys: list[str] | tuple[str, ...]) -> dict[str, str]: if template is None: template = fallback.get(key) if template is None: - _warn_once(f"miss:en:{key}", "catalog key %r missing entirely; serving the key", key) + warn_once(f"miss:en:{key}", "catalog key %r missing entirely; serving the key", key) template = key out[key] = template return out diff --git a/tests/state_sh_lifts.py b/tests/state_sh_lifts.py new file mode 100644 index 0000000..849da4c --- /dev/null +++ b/tests/state_sh_lifts.py @@ -0,0 +1,38 @@ +"""Lifts of scripts/lib/state.sh functions shared by the shell-script test +files (litclock-dev#868). One required-parts table, so a mutation of the +helper is caught by every file that executes it rather than by whichever +copy happened to list that part (litclock-dev#873 review).""" + +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +STATE_SH = REPO_ROOT / "scripts" / "lib" / "state.sh" + + +def extract_history_helper() -> str: + """lib/state.sh's clear_and_lock_bash_history, verbatim, with its + load-bearing parts asserted (the litclock-dev#662 rule: a lifted span that + asserts nothing about itself can lose the lock and stay extractable).""" + body = STATE_SH.read_text() + assert body.count("clear_and_lock_bash_history() {") == 1, "helper anchor is not unique" + start = body.index("clear_and_lock_bash_history() {") + end = body.index("\n}\n", start) + len("\n}\n") + fn = body[start:end] + for required, why in ( + ("history -c", "the clear of the script's own shell"), + ( + 'if [[ -L "$_h" && ! -c "$_h" ]]; then', + "the refuse-to-follow symlink check BEFORE removal (litclock-dev#873 review)", + ), + ("stat -c %h", "the hard-link count check (litclock-dev#873 review)"), + ('chattr -ia "$_h"', "clearing append-only/immutable before removal (litclock-dev#873 review)"), + ("rmdir", "taking a previous run's lock (and refusing a non-empty one)"), + ("rm -f", "the removal of a real history file"), + ('if mkdir "$_h" 2>/dev/null && [[ -d "$_h" && ! -L "$_h" ]]; then', "mkdir's status as the lock verdict"), + ("_HIST_DIRTY+=", "the survived-contents verdict"), + ("_HIST_UNLOCKED+=", "the could-not-lock verdict"), + ("_HIST_LOCKED+=", "the placed-locks report"), + ('for _h in "$@"; do', "the variadic path loop"), + ): + assert required in fn, f"clear_and_lock_bash_history lost {why} ({required!r})" + return fn diff --git a/tests/test_control_server.py b/tests/test_control_server.py index 966ad6a..9e00b38 100644 --- a/tests/test_control_server.py +++ b/tests/test_control_server.py @@ -1059,6 +1059,24 @@ def test_a_memo_surfaces_with_its_fixed_shape(self, status_file, tmp_path): "reason": "400s of this run's 600s budget are gone", "sha": "a" * 40, } + def test_a_selftest_failure_memo_surfaces(self, status_file, tmp_path): + """litclock-dev#871 Stage A writes `selftest-failed` into the same memo; + the reader must forward it, or the PWA never learns a device fell back.""" + memo = tmp_path / "memo.json" + memo.write_text('{"result": "selftest-failed", "rc": 3, "reason": "fell back", "sha": null, "at_unix": 5}') + got = self._get(status_file, memo)["runtime_render_validation"] + assert got is not None and got["result"] == "selftest-failed" and got["rc"] == 3 + + def test_a_selftest_deferral_is_its_own_token(self, status_file, tmp_path): + """Distinct from the validator's `deferred`: a marker-bearing device + whose validation passed must not read as 'validation deferred'.""" + memo = tmp_path / "memo.json" + memo.write_text( + '{"result": "selftest-deferred", "rc": null, "reason": "self-test: budget", "sha": null, "at_unix": 5}' + ) + got = self._get(status_file, memo)["runtime_render_validation"] + assert got is not None and got["result"] == "selftest-deferred" and got["rc"] is None + def test_a_failure_memo_carries_its_rc(self, status_file, tmp_path): memo = tmp_path / "memo.json" memo.write_text('{"result": "failed", "rc": 1, "reason": "exited 1", "sha": null, "at_unix": 5}') diff --git a/tests/test_exit_without_finalization.py b/tests/test_exit_without_finalization.py index 22deeec..fdd456c 100644 --- a/tests/test_exit_without_finalization.py +++ b/tests/test_exit_without_finalization.py @@ -177,6 +177,8 @@ def _exec_main_block( class _Args: def __init__(self): self.dry_run = dry_run + # litclock-dev#871 Stage A: the dry-run block reads this flag too. + self.require_runtime_render = False class _Parser: def __init__(self, *a, **k): diff --git a/tests/test_first_boot_flow.py b/tests/test_first_boot_flow.py index 16bbea2..c125da5 100644 --- a/tests/test_first_boot_flow.py +++ b/tests/test_first_boot_flow.py @@ -1768,6 +1768,8 @@ def test_first_boot_actually_writes_every_env_sample_key(tmp_path, with_state_li # ── litclock-dev#834: the clone-prep history lock is undone on first boot ───── +# (since litclock-dev#868 reset-setup.sh's gift-mode and --poweroff arms leave the +# same directory, and a reset lands on this same not-yet-set-up path) # # prepare-for-cloning.sh Step 6 replaces /home/pi/.bash_history and # /root/.bash_history with empty DIRECTORIES so that shells still open when diff --git a/tests/test_literary_clock_dry_run.py b/tests/test_literary_clock_dry_run.py index 8d78dfb..9c11947 100644 --- a/tests/test_literary_clock_dry_run.py +++ b/tests/test_literary_clock_dry_run.py @@ -311,3 +311,125 @@ def test_weather_enabled_master_toggle_is_read(self): ) assert provider_idx != -1 assert get_idx < provider_idx, "WEATHER_ENABLED must be checked BEFORE the weather provider is constructed" + + +# ── litclock-dev#871 Stage A: --dry-run --require-runtime-render ────────────── + + +def _runtime_capable_interpreter() -> str | None: + """An interpreter with freetype-py: the current one (CI installs + requirements into it) or the repo venv (the dev box). None -> skip.""" + for candidate in (sys.executable, str(REPO_ROOT / "venv" / "bin" / "python3")): + if not Path(candidate).exists(): + continue + probe = subprocess.run([candidate, "-c", "import freetype, PIL, requests"], capture_output=True, timeout=60) + if probe.returncode == 0: + return candidate + return None + + +@pytest.fixture(scope="module") +def stamped_marker(tmp_path_factory): + """A REAL validation marker for this checkout, written by the real + validator (~21s on x86). The pass path cannot be faked: the painter binds + the marker to a digest of the fonts, the dump and the FreeType version.""" + py = _runtime_capable_interpreter() + if py is None: + pytest.skip("no interpreter with freetype-py; the runtime pass path needs one") + marker = tmp_path_factory.mktemp("marker") / ".runtime-render-validated" + r = subprocess.run( + [py, str(REPO_ROOT / "tools" / "validate_measurement.py"), "check", "--stamp", "--marker", str(marker)], + cwd=REPO_ROOT, + capture_output=True, + text=True, + timeout=600, + ) + assert r.returncode == 0 and marker.is_file(), f"the validator did not stamp\n{r.stdout}\n{r.stderr}" + return py, marker + + +class TestRequireRuntimeRender: + def _run(self, py, extra_env, *flags): + env = _python_env() + env.update(extra_env) + return subprocess.run( + [py, "-m", "src.literary_clock", "--dry-run", *flags], + cwd=REPO_ROOT, + env=env, + capture_output=True, + text=True, + timeout=120, + ) + + @staticmethod + def _tier(stdout: str) -> str: + import re + + m = re.search(r"^dry-run: rendered \d+x\d+ image, render_mode=([\w-]+)$", stdout, re.M) + assert m, f"the dry-run must print its tier on stdout; got {stdout!r}" + return m.group(1) + + # `images/` is untracked and absent on a clean checkout (CI's unit-test job + # never downloads the image release), so the non-text tier is `image` here + # and `time-only` there (litclock-dev#875 review). Either is a fallback; neither is + # `runtime`. Both tests pin the marker to a missing path so a developer's + # local marker cannot flip them onto the text tier. + _NO_TEXT_TIER = {"image", "time-only"} + + def test_the_default_dry_run_reports_its_tier_and_still_exits_zero(self): + """The smoke gate's contract is untouched: a fallback frame is a pass.""" + r = self._run( + sys.executable, + {"LITCLOCK_RUNTIME_RENDER": "true", "LITCLOCK_RUNTIME_VALIDATED_MARKER": "/nonexistent/marker"}, + ) + assert r.returncode == 0, r.stderr + assert self._tier(r.stdout) in self._NO_TEXT_TIER, r.stdout + + def test_a_fallback_frame_exits_3_with_the_flag(self): + """No marker -> the painter declines the text tier. The flag turns that + silent degrade into the exit code the self-test needs.""" + r = self._run( + sys.executable, + {"LITCLOCK_RUNTIME_RENDER": "true", "LITCLOCK_RUNTIME_VALIDATED_MARKER": "/nonexistent/marker"}, + "--require-runtime-render", + ) + assert r.returncode == 3, (r.returncode, r.stderr) + tier = self._tier(r.stdout) + assert tier in self._NO_TEXT_TIER, "the render itself succeeded; only the tier is wrong" + assert f"--require-runtime-render but the frame came from render_mode={tier}" in r.stderr, r.stderr + + def test_the_flag_without_dry_run_is_refused(self): + """Silently ignoring it would let a typo'd invocation look like a pass.""" + r = subprocess.run( + [sys.executable, "-m", "src.literary_clock", "--require-runtime-render"], + cwd=REPO_ROOT, + env=_python_env(), + capture_output=True, + text=True, + timeout=60, + ) + assert r.returncode == 2, (r.returncode, r.stderr) + assert "only means something with --dry-run" in r.stderr + + def test_a_runtime_frame_exits_zero_with_the_flag(self, stamped_marker, tmp_path): + """The pass path, for real: a stamped marker, the renderer on, and the + frame comes from text. This is the line the OTA self-test reads.""" + py, marker = stamped_marker + r = self._run( + py, + { + "LITCLOCK_RUNTIME_RENDER": "true", + "LITCLOCK_RUNTIME_VALIDATED_MARKER": str(marker), + "LITCLOCK_RUNTIME_RENDER_DIR": str(tmp_path), + }, + "--require-runtime-render", + ) + assert r.returncode == 0, (r.returncode, r.stderr) + assert self._tier(r.stdout) == "runtime", (r.stdout, r.stderr) + assert (tmp_path / "current-quote.png").is_file(), "a runtime dry-run persists its frame where it is told to" + + def test_the_flag_is_argparse_and_dry_run_only(self): + src = LITERARY_CLOCK.read_text(encoding="utf-8") + executed = "\n".join(ln for ln in src.splitlines() if not ln.lstrip().startswith("#")) + assert '"--require-runtime-render"' in executed + assert "sys.exit(3)" in executed, "the fallback verdict is exit 3, distinct from a crash (1)" diff --git a/tests/test_prepare_for_cloning_sh.py b/tests/test_prepare_for_cloning_sh.py index 1cb2b2e..bb47104 100644 --- a/tests/test_prepare_for_cloning_sh.py +++ b/tests/test_prepare_for_cloning_sh.py @@ -12,6 +12,8 @@ PREPARE_SH = REPO_ROOT / "scripts" / "prepare-for-cloning.sh" STATE_SH = REPO_ROOT / "scripts" / "lib" / "state.sh" +from tests.state_sh_lifts import extract_history_helper # noqa: E402 + @pytest.fixture(scope="module") def prepare_sh_content(): @@ -62,7 +64,18 @@ def test_clears_bash_history(self, prepare_sh_content): below) and the step both removes and LOCKS them.""" assert '_PI_BASH_HISTORY="/home/pi/.bash_history"' in prepare_sh_content assert '_ROOT_BASH_HISTORY="/root/.bash_history"' in prepare_sh_content - assert 'for _h in "$_PI_BASH_HISTORY" "$_ROOT_BASH_HISTORY"; do' in prepare_sh_content + # litclock-dev#868: the loop lives in lib/state.sh now, shared with + # reset-setup.sh; the script passes its two fixed paths to it. + assert 'clear_and_lock_bash_history "$_PI_BASH_HISTORY" "$_ROOT_BASH_HISTORY"' in prepare_sh_content + assert 'for _h in "$@"; do' in _extract_history_helper() + + def test_state_sh_gate_requires_the_history_helper(self, prepare_sh_content): + """litclock-dev#868: a new script beside an old lib/state.sh must refuse + before Step 1, not fail Step 6 with `command not found` after the WiFi + profiles are already gone.""" + m = re.search(r"^for _fn in (.+); do$", prepare_sh_content, re.M) + assert m, "the lib/state.sh helper gate is missing" + assert "clear_and_lock_bash_history" in m.group(1).split() def test_clears_ssl_certs(self, prepare_sh_content): """SSL cert contains litclock.local — fine to share, but regenerating @@ -2614,7 +2627,7 @@ def test_a_successful_wipe_runs_through_to_the_gate(self, tmp_path): # real interactive bash, holding history, exiting against the locked path. _HIST_START = 'echo -n "Clearing bash history... "' -_HIST_END = "unset _HIST_DIRTY _HIST_UNLOCKED _h" +_HIST_END = "unset _HIST_DIRTY _HIST_UNLOCKED _HIST_LOCKED" def _extract_history_step() -> str: @@ -2626,17 +2639,27 @@ def _extract_history_step() -> str: end = body.index('echo -e "${GREEN}done${NC}"', body.index(_HIST_END, start)) end += len('echo -e "${GREEN}done${NC}"') span = body[start:end] - assert "mkdir" in span and "history -c" in span, "span lost the lock or the clear" + # litclock-dev#868: the wipe and the lock moved into lib/state.sh's + # clear_and_lock_bash_history (shared with reset-setup.sh's handoff arms); + # the step is the CALL plus the two fatal verdicts, and _extract_history_helper + # pins the helper's own load-bearing parts. + assert 'clear_and_lock_bash_history "$_PI_BASH_HISTORY" "$_ROOT_BASH_HISTORY"' in span, ( + "span lost the shared wipe+lock call" + ) assert span.count("_abort_do_not_clone") == 2, "span lost one of the two aborts" return span +_extract_history_helper = extract_history_helper + + def _run_history_step(tmp_path, pi_path: Path, root_path: Path): script = f"""{_script_shell_environment()} {_YELLOW_DECL} _PI_BASH_HISTORY={shlex.quote(str(pi_path))} _ROOT_BASH_HISTORY={shlex.quote(str(root_path))} {_extract_env_abort_fn()} +{_extract_history_helper()} {_ERREXIT_PROBE} {_extract_history_step()} echo REACHED-NEXT-STEP @@ -2692,6 +2715,47 @@ def test_a_second_run_over_the_lock_is_a_noop(self, tmp_path): assert "FAILED" not in r.stdout assert pi.is_dir() and root.is_dir() + def test_a_symlinked_history_is_refused_and_its_target_survives_untouched(self, tmp_path): + """litclock-dev#873 review (Codex): `rm -f` on a symlink unlinks the LINK and leaves + the target — contents included — on the card, and the old step then + locked the path and printed green. A symlink is now refused BEFORE + anything is removed: abort, link intact, target intact, no lock.""" + home, pi, root = self._paths(tmp_path) + target = tmp_path / "saved-history" + target.write_text("nmcli dev wifi connect x password y\n") + pi.unlink() + pi.symlink_to(target) + r = _run_history_step(tmp_path, pi, root) + assert r.returncode == 1, f"a symlinked history was accepted\n{r.stdout}" + assert "Could not clear the shell history at" in r.stdout and str(pi) in r.stdout + assert "REACHED-NEXT-STEP" not in r.stdout + assert pi.is_symlink(), "the helper must not unlink a symlink it refused" + assert target.read_text() == "nmcli dev wifi connect x password y\n", "the target must be untouched" + + def test_the_dev_null_symlink_idiom_is_cleared_and_locked(self, tmp_path): + """`ln -sf /dev/null ~/.bash_history` is the standard way to disable + history; a blanket symlink refusal would have made every factory reset + of such a device fail forever (litclock-dev#873 red team). A char-device target + holds nothing: the link goes, the lock lands.""" + home, pi, root = self._paths(tmp_path) + pi.unlink() + pi.symlink_to("/dev/null") + r = _run_history_step(tmp_path, pi, root) + assert r.returncode == 0, r.stdout + r.stderr + assert "REACHED-NEXT-STEP" in r.stdout + assert pi.is_dir() and not pi.is_symlink() and not any(pi.iterdir()) + + def test_a_hard_linked_history_is_refused(self, tmp_path): + """Same class: unlinking one NAME of a two-link file leaves the inode + and its contents reachable by the other name.""" + home, pi, root = self._paths(tmp_path) + other = tmp_path / "history-copy" + os.link(pi, other) + r = _run_history_step(tmp_path, pi, root) + assert r.returncode == 1, f"a hard-linked history was accepted\n{r.stdout}" + assert str(pi) in r.stdout + assert pi.is_file() and other.is_file() and "password" in other.read_text() + def test_the_harness_shell_really_writes_history(self, tmp_path): """Control for the test below: WITHOUT the lock the same shell writes the line back, so a passing lock test is not a shell that never saved.""" diff --git a/tests/test_quote_corpus.py b/tests/test_quote_corpus.py index e8b2ae5..b246b4f 100644 --- a/tests/test_quote_corpus.py +++ b/tests/test_quote_corpus.py @@ -118,7 +118,7 @@ def test_lookup_handles_embedded_quote_chars(synthetic_corpus) -> None: assert 'embedded "quotes"' in meta["quote"] -def test_lookup_real_corpus_first_row_smoke() -> None: +def test_lookup_real_corpus_first_row_smoke(monkeypatch) -> None: """Smoke test against the real bundled corpus — pins that the CSV-vs-filename contract holds with the actual production data, catching any drift between PHP-side numbering and our Python-side @@ -131,10 +131,11 @@ def test_lookup_real_corpus_first_row_smoke() -> None: real_csv = repo_root / "image-gen" / "litclock_annotated.csv" if not real_csv.exists(): pytest.skip("bundled corpus CSV not present in this checkout") - os.environ.pop("LITCLOCK_CORPUS_CSV", None) - # Direct attribute swap because the module reads the env var only at - # import time. monkeypatch.setenv won't help post-import. - quote_corpus._CORPUS_PATH = real_csv # type: ignore[attr-defined] + # Attribute swap, RESTORED by monkeypatch (litclock-dev#874 red team): since + # litclock-dev#870 `_CORPUS_PATH` carries a mode — None means "resolve + # through the registry" — and a bare assignment here pinned the override + # rung for every later test in the session. + monkeypatch.setattr(quote_corpus, "_CORPUS_PATH", real_csv) quote_corpus.reset_cache() meta = quote_corpus.lookup_by_filename("quote_0000_0_credits.png") assert meta is not None @@ -387,3 +388,280 @@ def test_tame_filename_refuses_nsfw_row(self): def test_nsfw_filename_refuses_tame_row(self): assert quote_corpus.lookup_by_filename("quote_0300_0_nsfw_credits.png") is None + + +# ── litclock-dev#870: the corpus follows the active language via the registry ── + + +def _write_registry(root: Path, languages: dict[str, dict]) -> None: + import json + + (root / "languages.json").write_text( + json.dumps({"schema_version": 1, "fleet_default": "en", "languages": languages}), encoding="utf-8" + ) + + +def _lang_entry(code: str, corpus_rel: str, *, status: str = "active") -> dict: + return { + "code": code, + "native_name": code, + "status": status, + "corpus": {"path": corpus_rel, "rows": 1, "sfw_coverage_pct": 0.1}, + "strings": f"languages/{code}/strings.json", + "plural_forms": ["one", "other"], + } + + +def _one_row(path: Path, author: str, quote: str = "quote") -> None: + _write_corpus(path, [("00:00", "midnight", quote, f"Title {author}", author, "NO")]) + + +@pytest.fixture +def two_language_registry(tmp_path, monkeypatch): + """A repo root with an English and an `xx` corpus, each one row at 00:00, + registered in a tmp languages.json. Both modules are pointed at the tmp + root, the shipped-default constant at the tmp English file (so the image + tier is observable), and LITCLOCK_LANGUAGE is read from the process env.""" + import strings_catalog + + (tmp_path / "corpora").mkdir() + en = tmp_path / "corpora" / "en.csv" + xx = tmp_path / "corpora" / "xx.csv" + _one_row(en, "Author EN", "english quote") + _one_row(xx, "Author XX", "xx quote") + _write_registry(tmp_path, {"en": _lang_entry("en", "corpora/en.csv"), "xx": _lang_entry("xx", "corpora/xx.csv")}) + monkeypatch.setattr(strings_catalog, "REGISTRY_PATH", tmp_path / "languages.json") + monkeypatch.setattr(strings_catalog, "_REPO_ROOT", tmp_path) + monkeypatch.setattr(quote_corpus, "_PROJECT_ROOT", tmp_path) + monkeypatch.setattr(quote_corpus, "_DEFAULT_CORPUS_PATH", en) + monkeypatch.setattr(quote_corpus, "_CORPUS_PATH", None) + monkeypatch.delenv("LITCLOCK_LANGUAGE", raising=False) + monkeypatch.setenv("LITCLOCK_ENV_FILE", str(tmp_path / "no-env.sh")) # no env.sh channel + strings_catalog.reset_cache() + quote_corpus.reset_cache() + yield {"en": en, "xx": xx, "root": tmp_path} + strings_catalog.reset_cache() + quote_corpus.reset_cache() + + +def _rewrite_registry(reg, languages): + import strings_catalog + + _write_registry(reg["root"], languages) + strings_catalog.reset_cache() + + +def _author(hhmm="0000"): + return quote_corpus.bucket_entries(hhmm)[0]["author"] + + +class TestCorpusFollowsTheActiveLanguage: + def test_default_is_the_english_registry_corpus(self, two_language_registry): + assert quote_corpus.corpus_path() == two_language_registry["en"] + assert [e["quote_raw"] for e in quote_corpus.bucket_entries("0000")] == ["english quote"] + + def test_an_active_language_reads_its_own_corpus(self, two_language_registry, monkeypatch): + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert quote_corpus.corpus_path() == two_language_registry["xx"] + assert _author() == "Author XX" + + def test_switching_language_within_one_process_switches_the_corpus(self, two_language_registry, monkeypatch): + """The lru_cache trap named on the issue: a naive resolution pins the + FIRST language's corpus for the life of the process. Three lookups, + no reset_cache() between them, in a single process.""" + assert _author() == "Author EN" + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert _author() == "Author XX" + monkeypatch.delenv("LITCLOCK_LANGUAGE") + assert _author() == "Author EN" + + def test_the_renderer_selection_pool_follows_too(self, two_language_registry, monkeypatch): + """rows_for_time(None, …) is what the clock's runtime path calls.""" + import quote_renderer + + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert [r.quote for r in quote_renderer.rows_for_time(None, "0000")] == ["xx quote"] + + def test_an_unknown_code_degrades_to_english(self, two_language_registry, monkeypatch): + monkeypatch.setenv("LITCLOCK_LANGUAGE", "zz") + assert quote_corpus.corpus_path() == two_language_registry["en"] + + def test_an_incubating_code_degrades_to_english(self, two_language_registry, monkeypatch): + _rewrite_registry( + two_language_registry, + {"en": _lang_entry("en", "corpora/en.csv"), "xx": _lang_entry("xx", "corpora/xx.csv", status="incubating")}, + ) + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert quote_corpus.corpus_path() == two_language_registry["en"] + + def test_a_registry_corpus_missing_on_disk_falls_back_to_english_with_one_warning( + self, two_language_registry, monkeypatch, caplog + ): + import logging + + _rewrite_registry( + two_language_registry, + {"en": _lang_entry("en", "corpora/en.csv"), "xx": _lang_entry("xx", "corpora/gone.csv")}, + ) + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + with caplog.at_level(logging.WARNING, logger="strings_catalog"): + assert quote_corpus.corpus_path() == two_language_registry["en"] + assert _author() == "Author EN" + quote_corpus.corpus_path() + hits = [r for r in caplog.records if "corpora/gone.csv" in r.getMessage()] + assert len(hits) == 1, "the missing-corpus warning must fire once, not per lookup" + + def test_an_empty_corpus_does_not_shadow_english(self, two_language_registry, monkeypatch): + """litclock-dev#874 review: existence alone let a zero-byte translation win over a + healthy English corpus and paint nothing.""" + two_language_registry["xx"].write_text("", encoding="utf-8") + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert quote_corpus.corpus_path() == two_language_registry["en"] + + @pytest.mark.parametrize("rel", ["../outside.csv", "/etc/passwd"]) + def test_a_corpus_path_outside_the_checkout_is_refused(self, two_language_registry, monkeypatch, caplog, rel): + """litclock-dev#874 review (Codex + security): `root / rel` discards the root for an + absolute rel and follows `..`, so a registry typo could serve any + pi-readable file's lines as quote text.""" + import logging + + outside = two_language_registry["root"].parent / "outside.csv" + _one_row(outside, "Author OUTSIDE") + _rewrite_registry( + two_language_registry, {"en": _lang_entry("en", "corpora/en.csv"), "xx": _lang_entry("xx", rel)} + ) + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + with caplog.at_level(logging.WARNING, logger="strings_catalog"): + assert quote_corpus.corpus_path() == two_language_registry["en"] + assert any("outside the checkout" in r.getMessage() for r in caplog.records) + + def test_a_symlink_loop_is_unusable_not_fatal(self, two_language_registry, monkeypatch): + """litclock-dev#874 red team: resolve() raises RuntimeError on a loop, which the + first guard (OSError only) let escape — and the runtime tier's broad + except would then have dropped to PNGs every minute instead of + falling through to English.""" + loop = two_language_registry["root"] / "corpora" / "loop.csv" + loop.symlink_to(loop) + _rewrite_registry( + two_language_registry, + {"en": _lang_entry("en", "corpora/en.csv"), "xx": _lang_entry("xx", "corpora/loop.csv")}, + ) + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert quote_corpus.corpus_path() == two_language_registry["en"] + + def test_a_symlink_out_of_the_checkout_is_refused(self, two_language_registry, monkeypatch): + outside = two_language_registry["root"].parent / "outside2.csv" + _one_row(outside, "Author OUTSIDE") + link = two_language_registry["root"] / "corpora" / "link.csv" + link.symlink_to(outside) + _rewrite_registry( + two_language_registry, + {"en": _lang_entry("en", "corpora/en.csv"), "xx": _lang_entry("xx", "corpora/link.csv")}, + ) + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert quote_corpus.corpus_path() == two_language_registry["en"] + + def test_both_registry_corpora_missing_lands_on_the_shipped_default_with_two_warnings( + self, two_language_registry, monkeypatch, caplog + ): + import logging + + shipped = two_language_registry["root"] / "shipped.csv" + _one_row(shipped, "Author SHIPPED") + monkeypatch.setattr(quote_corpus, "_DEFAULT_CORPUS_PATH", shipped) + _rewrite_registry( + two_language_registry, + {"en": _lang_entry("en", "corpora/gone-en.csv"), "xx": _lang_entry("xx", "corpora/gone-xx.csv")}, + ) + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + with caplog.at_level(logging.WARNING, logger="strings_catalog"): + assert quote_corpus.corpus_path() == shipped + assert _author() == "Author SHIPPED" + quote_corpus.corpus_path() + hits = [r for r in caplog.records if "gone-" in r.getMessage()] + assert len(hits) == 2, "one warning per missing rung, each once" + + def test_the_env_override_wins_over_the_registry_for_both_tiers(self, two_language_registry, monkeypatch): + override = two_language_registry["root"] / "override.csv" + _one_row(override, "Author OV") + monkeypatch.setattr(quote_corpus, "_CORPUS_PATH", override) + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert quote_corpus.corpus_path() == override + assert quote_corpus.image_corpus_path() == override + assert _author() == "Author OV" + assert quote_corpus.lookup_by_filename("quote_0000_0_credits.png")["author"] == "Author OV" + + def test_the_image_lookup_reads_the_shipped_corpus_whatever_the_language_or_registry_says( + self, two_language_registry, monkeypatch + ): + """See quote_corpus.image_corpus_path: the PNGs' provenance is the + file the generator opens by name, not the device language and not + even the registry's English entry.""" + other = two_language_registry["root"] / "corpora" / "other.csv" + _one_row(other, "Author OTHER") + _rewrite_registry( + two_language_registry, + {"en": _lang_entry("en", "corpora/other.csv"), "xx": _lang_entry("xx", "corpora/xx.csv")}, + ) + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert quote_corpus.image_corpus_path() == two_language_registry["en"] + assert quote_corpus.lookup_by_filename("quote_0000_0_credits.png")["author"] == "Author EN" + # ...while the renderer's pool, in the same process, is the xx corpus, + # and an English device's runtime pool follows the registry's English. + assert _author() == "Author XX" + monkeypatch.delenv("LITCLOCK_LANGUAGE") + assert _author() == "Author OTHER" + + +class TestRegistryCorpusAccessors: + def test_english_points_at_the_shipped_corpus(self, monkeypatch): + import strings_catalog + + monkeypatch.setattr(quote_corpus, "_CORPUS_PATH", None) + monkeypatch.delenv("LITCLOCK_LANGUAGE", raising=False) + monkeypatch.setenv("LITCLOCK_ENV_FILE", "/nonexistent/env.sh") + strings_catalog.reset_cache() + assert strings_catalog.corpus_relpath("en") == "image-gen/litclock_annotated.csv" + assert quote_corpus.corpus_path() == quote_corpus._DEFAULT_CORPUS_PATH + assert quote_corpus.image_corpus_path() == quote_corpus._DEFAULT_CORPUS_PATH + + def test_the_image_corpus_is_the_file_the_generator_opens(self): + """litclock-dev#874 review: nothing else pins the PNGs' provenance to the resolver. + The PHP generator, the release verifier and the render tooling all + name this file; image_corpus_path() must name the same one.""" + repo = Path(__file__).resolve().parents[1] + assert quote_corpus._DEFAULT_CORPUS_PATH == repo / "image-gen" / "litclock_annotated.csv" + assert "litclock_annotated.csv" in (repo / "image-gen" / "quote_to_image.php").read_text(encoding="utf-8") + assert 'CORPUS_FILE="$REPO_ROOT/image-gen/litclock_annotated.csv"' in ( + repo / "scripts" / "download_images.sh" + ).read_text(encoding="utf-8") + + @pytest.mark.parametrize( + "entry", + [ + {"status": "active"}, + {"status": "active", "corpus": None}, + {"status": "active", "corpus": {}}, + {"status": "active", "corpus": {"path": ""}}, + {"status": "active", "corpus": {"path": 7}}, + ], + ids=["no-corpus", "corpus-none", "corpus-empty", "path-empty", "path-int"], + ) + def test_a_malformed_entry_has_no_corpus_and_the_clock_still_resolves( + self, two_language_registry, monkeypatch, entry + ): + import strings_catalog + + xx = {**_lang_entry("xx", "x"), **entry} + if "corpus" not in entry: + del xx["corpus"] + _rewrite_registry(two_language_registry, {"en": _lang_entry("en", "corpora/en.csv"), "xx": xx}) + assert strings_catalog.corpus_relpath("xx") is None + monkeypatch.setenv("LITCLOCK_LANGUAGE", "xx") + assert quote_corpus.corpus_path() == two_language_registry["en"] + + def test_unknown_code_has_no_corpus(self): + import strings_catalog + + strings_catalog.reset_cache() + assert strings_catalog.corpus_relpath("zz") is None diff --git a/tests/test_reset_setup_sh.py b/tests/test_reset_setup_sh.py index be16695..890b609 100644 --- a/tests/test_reset_setup_sh.py +++ b/tests/test_reset_setup_sh.py @@ -14,6 +14,8 @@ STATE_SH = REPO_ROOT / "scripts" / "lib" / "state.sh" +from tests.state_sh_lifts import extract_history_helper # noqa: E402 + @pytest.fixture(scope="module") def reset_sh_content(): @@ -721,6 +723,43 @@ def test_poweroff_does_not_write_the_gift_welcome_marker(self, reset_sh_content) _TIMEOUT_STUB = 'timeout() { shift; "$@"; }\n' +_extract_history_helper = extract_history_helper + + +def _extract_handoff_history_fn(content: str) -> str: + """reset-setup.sh's clear_shell_history_for_handoff, verbatim, with its + fail-closed parts asserted (the litclock-dev#662 rule).""" + assert content.count("clear_shell_history_for_handoff() {") == 1 + start = content.index("clear_shell_history_for_handoff() {") + end = content.index("\n}\n", start) + len("\n}\n") + fn = content[start:end] + for required, why in ( + ('clear_and_lock_bash_history "$_PI_BASH_HISTORY" "$_ROOT_BASH_HISTORY"', "the shared wipe+lock call"), + ("exit 1", "the fail-closed abort"), + ("do NOT hand this device on", "the operator warning"), + ("NOT powering off", "the refusal to power off"), + ): + assert required in fn, f"clear_shell_history_for_handoff lost {why} ({required!r})" + return fn + + +def _history_fixture(tmp_path): + """Two history files with an owner's commands in them, at paths the lifted + handoff arms are pointed at by redefining the script's two fixed + variables (same convention as the cloning harness).""" + home = tmp_path / "home" + home.mkdir(exist_ok=True) + pi, root = home / "pi.bash_history", home / "root.bash_history" + # A test that pre-stages a survivor (a non-empty directory at the path) + # keeps it; the fixture only supplies the ordinary owner's-history shape. + if not pi.exists(): + pi.write_text("nmcli dev wifi connect HomeNet password hunter2\n", encoding="utf-8") + if not root.exists(): + root.write_text("sudo -i\n", encoding="utf-8") + decl = f"_PI_BASH_HISTORY={shlex.quote(str(pi))}\n_ROOT_BASH_HISTORY={shlex.quote(str(root))}\n" + return pi, root, decl + + class TestHotspotPasswordResetSemantics: """litclock-dev#620 — the persisted hotspot password survives a plain reset and a WiFi reset ON PURPOSE (the owner's phone has the network saved, and a @@ -798,7 +837,24 @@ def _terminal_branch(content): f"lifted rotation guard has no call in it: {hoist_block!r}" ) assert "NetworkManager" not in hoist_block, "lifted span reaches Step 7's WiFi wipe" - block = hoist_block + "\n" + content[anchor:] + # litclock-dev#868: the history wipe is a THIRD span, hoisted between + # Step 7 and the "Reset Complete!" banner for both handoff arms. Its + # condition is inside the lifted text, so the non-handoff arms exercise + # the guard rather than an absence. + hist = content.find( + 'if [[ "$ENV_WIPE_FAILED" != "true" && ( "$GIFT_MODE" == "true" || "$DO_POWEROFF" == "true" ) ]]; then' + ) + assert hist != -1, "litclock-dev#868: the hoisted handoff history-wipe guard is missing" + assert hoist_end <= hist < anchor, ( + "the history wipe must sit after the rotation guard and before the terminal branch" + ) + hist_end = content.index("\nfi\n", hist) + len("\nfi\n") + hist_block = content[hist:hist_end] + assert "clear_shell_history_for_handoff" in hist_block, ( + f"lifted history guard has no call in it: {hist_block!r}" + ) + assert "NetworkManager" not in hist_block + block = hoist_block + "\n" + hist_block + "\n" + content[anchor:] for required, why in ( ('elif [[ "$DO_POWEROFF" == "true" ]]', "the --poweroff arm"), ('elif [[ "$DO_REBOOT" == "true" ]]', "the --reboot arm"), @@ -869,6 +925,11 @@ def _run(self, content, tmp_path, gift_mode, do_poweroff="false", do_reboot="fal f"LITCLOCK_STATE_DIR={state}\n" f"{self._state_dir_line(content)}\n" 'RED=""\nGREEN=""\nYELLOW=""\nNC=""\n' + # litclock-dev#868: the handoff arms wipe+lock the shell history. + # Real helper, real function, paths redirected into tmp_path/home. + f"{_history_fixture(tmp_path)[2]}" + f"{_extract_history_helper()}\n" + f"{_extract_handoff_history_fn(content)}\n" f"{extra}\n" f"{self._rotation_fn(content)}\n" f"{self._terminal_branch(content)}" @@ -972,6 +1033,7 @@ def test_the_real_cli_default_reaches_rotation_end_to_end(self, reset_sh_content config.mkdir() content = reset_sh_content + hist_pi, _, hist_decl = _history_fixture(tmp_path) default_line = next(ln for ln in content.splitlines() if ln.startswith("WIPE_WIFI=")) parse_start = content.index("while [[ $# -gt 0 ]]; do") parse_end = content.index("done", parse_start) + len("done") @@ -995,12 +1057,20 @@ def test_the_real_cli_default_reaches_rotation_end_to_end(self, reset_sh_content f"LITCLOCK_STATE_DIR={state}\n" f"{self._state_dir_line(content)}\n" 'RED=""\nGREEN=""\nYELLOW=""\nNC=""\n' + f"{hist_decl}" + f"{_extract_history_helper()}\n" + f"{_extract_handoff_history_fn(content)}\n" f"{self._rotation_fn(content)}\n" f"{self._terminal_branch(content)}" ) # No arguments: the plain `sudo reset-setup.sh` a person actually types. result = subprocess.run(["bash", "-c", program, "bash"], capture_output=True, text=True, timeout=30) assert result.returncode == 0, result.stderr + # litclock-dev#868: the plain reset keeps the operator's history — it hands + # control back to them, not to a new owner. + assert hist_pi.is_file() and "hunter2" in hist_pi.read_text(encoding="utf-8"), ( + "the plain no-flag reset must NOT wipe the shell history — it is the same-owner path" + ) assert "WARNING: could not re-arm" not in result.stdout, result.stdout # litclock-dev#833: see _run assert not pw.exists(), ( "a no-argument factory reset did not rotate the setup network's password. " @@ -1418,6 +1488,12 @@ def test_reset_setup_has_no_other_state_dir_deletion(self, reset_sh_content): # deliberately rather than loosening the scan (this guard caught # the addition, which is what it is for). "runtime-render-validation.json", + # litclock-dev#871 Stage A: the self-test pass record, per-device like the memo. + "runtime-render-selftest.json", + # litclock-dev#871 Stage A: the self-test pass record, per-device like the memo. + "runtime-render-selftest.json", + # litclock-dev#871 Stage A: the self-test pass record, per-device like the memo. + "runtime-render-selftest.json", ) fn = self._rotation_fn(reset_sh_content) @@ -3225,3 +3301,169 @@ def test_it_sits_with_the_other_state_removals(self, reset_sh_content): a = executed.index('rm -f "$STATE_DIR/reset-failed"') b = executed.index('rm -f "$STATE_DIR/runtime-render-validation.json"') assert abs(a - b) < 200, executed[min(a, b): max(a, b) + 80] + + +class TestHandoffHistoryWipe: + """litclock-dev#868 — gift mode and the --poweroff factory reset wipe and + LOCK the previous owner's shell history; the plain reset and --reboot do + not. EXECUTED through the same lifted terminal branch as the rotation + tests (real helper from lib/state.sh, real function from this script), + with the two fixed history paths redirected into tmp_path/home. + """ + + _sem = TestHotspotPasswordResetSemantics + + @staticmethod + def _hist_paths(tmp_path): + home = tmp_path / "home" + return home / "pi.bash_history", home / "root.bash_history" + + def _run(self, content, tmp_path, **kw): + return self._sem()._run(content, tmp_path, **kw) + + @pytest.mark.parametrize( + "kw", + [ + pytest.param(dict(gift_mode="true", wipe_wifi="false"), id="gift"), + pytest.param(dict(gift_mode="false", do_poweroff="true", wipe_wifi="true"), id="pwa-factory-reset"), + pytest.param(dict(gift_mode="false", do_poweroff="true", wipe_wifi="false"), id="keep-wifi-poweroff"), + ], + ) + def test_handoff_arms_wipe_and_lock_the_history_before_ssh_off(self, reset_sh_content, tmp_path, kw): + pi, root = self._hist_paths(tmp_path) + _, result, _ = self._run(reset_sh_content, tmp_path, **kw) + assert result.returncode == 0, result.stdout + result.stderr + assert "STUB_POWEROFF" in result.stdout + for p in (pi, root): + assert p.is_dir() and not p.is_symlink(), f"{p} is not the write-back lock directory" + assert not any(p.iterdir()), f"{p} is not empty" + assert "hunter2" not in "".join(q.read_text() for q in (tmp_path / "home").rglob("*") if q.is_file()) + out = result.stdout + assert "Clearing shell history before handoff... done" in out + assert out.index("Clearing shell history") < out.index("STUB_SSH_GATE") < out.index("STUB_POWEROFF"), ( + "the history wipe must run BEFORE SSH-off (a failure has to leave the owner a shell) and before poweroff" + ) + + @pytest.mark.parametrize( + "kw", + [ + pytest.param(dict(gift_mode="false", do_reboot="true", wipe_wifi="true"), id="reboot"), + pytest.param(dict(gift_mode="false", wipe_wifi="true"), id="plain"), + ], + ) + def test_non_handoff_arms_keep_the_history(self, reset_sh_content, tmp_path, kw): + """The litclock-dev#718 distinction: these hand control back to an + operator at a console, not to a new owner.""" + pi, root = self._hist_paths(tmp_path) + _, result, _ = self._run(reset_sh_content, tmp_path, **kw) + assert result.returncode == 0, result.stdout + result.stderr + assert "Clearing shell history" not in result.stdout + assert pi.is_file() and "hunter2" in pi.read_text(encoding="utf-8") + assert root.is_file() and "sudo -i" in root.read_text(encoding="utf-8") + + @pytest.mark.parametrize( + "kw", + [ + pytest.param(dict(gift_mode="true", wipe_wifi="false"), id="gift"), + pytest.param(dict(gift_mode="false", do_poweroff="true", wipe_wifi="true"), id="pwa-factory-reset"), + ], + ) + def test_a_surviving_history_is_fatal_and_keeps_the_owner_a_shell(self, reset_sh_content, tmp_path, kw): + """A NON-EMPTY directory at the path (what an earlier aborted run plus a + stray file looks like) cannot be removed by rmdir or rm -f, so its + contents survive. Decision (litclock-dev#868): fatal, like a failed rotation — + no poweroff, no SSH-off, red banner, exit 1.""" + pi, root = self._hist_paths(tmp_path) + (tmp_path / "home").mkdir(exist_ok=True) + pi.mkdir() + (pi / "stray").write_text("hunter2\n", encoding="utf-8") + _, result, _ = self._run(reset_sh_content, tmp_path, **kw) + assert result.returncode == 1, result.stdout + result.stderr + assert "Shell history SURVIVED" in result.stdout + assert "do NOT hand this device on" in result.stdout + # The per-path diagnostic, pinned (litclock-dev#873 testing specialist: deleting + # both diagnostic blocks left every test green). + assert "Could not clear the shell history at" in result.stdout and str(pi) in result.stdout + assert "STUB_POWEROFF" not in result.stdout, "must refuse to power off with the old owner's history on it" + assert "STUB_SSH_GATE" not in result.stdout, "SSH-off must not run — the owner still needs a shell to fix this" + assert (pi / "stray").is_file(), "the harness's survivor was not the thing that failed" + # The other path was still processed: report, not short-circuit — and + # the banner tells the owner that lock stays until the next boot. + assert root.is_dir() + assert "Locks already placed at" in result.stdout and str(root) in result.stdout + + @pytest.mark.parametrize( + "kw", + [ + pytest.param(dict(gift_mode="true", wipe_wifi="false"), id="gift"), + pytest.param(dict(gift_mode="false", do_poweroff="true", wipe_wifi="true"), id="pwa-factory-reset"), + ], + ) + def test_a_history_that_cannot_be_locked_is_fatal_and_names_the_path(self, reset_sh_content, tmp_path, kw): + """The UNLOCKED shape (the litclock-dev#834 write-back case): the path + is empty but the lock cannot be placed. Staged with a parent that does + not exist — mirrors the cloning file's test.""" + pi, root = self._hist_paths(tmp_path) + unlockable = tmp_path / "no-such-dir" / ".bash_history" + extra = f"_PI_BASH_HISTORY={shlex.quote(str(unlockable))}\n" + _, result, _ = self._run(reset_sh_content, tmp_path, extra=extra, **kw) + assert result.returncode == 1, result.stdout + result.stderr + assert "Could not lock" in result.stdout and str(unlockable) in result.stdout + assert "STUB_SSH_GATE" not in result.stdout and "STUB_POWEROFF" not in result.stdout + assert not unlockable.exists() + assert root.is_dir() and "Locks already placed at" in result.stdout and str(root) in result.stdout + del pi + + def test_state_sh_gate_requires_the_history_helper(self, reset_sh_content): + """A new reset-setup.sh beside an old lib/state.sh must refuse before + Step 1 rather than reach the handoff arm and fail with + `command not found` — which, with no `set -e`, would print FAILED and + exit 1 only because the function is written fail-closed.""" + m = re.search(r"^for _fn in (.+); do$", reset_sh_content, re.M) + assert m, "the lib/state.sh helper gate is missing" + assert "clear_and_lock_bash_history" in m.group(1).split() + + def test_history_wipe_is_one_hoisted_call_above_the_wifi_wipe(self, reset_sh_content): + """Order is the contract (litclock-dev#873 review + red team): non-gift rotation + belts -> history wipe (both handoff arms, fail-closed) -> Step 7 WiFi + wipe -> "Reset Complete!" banner -> terminal arms (gift gates, SSH-off, + poweroff). A failure must never print the banner, the lock must exist + before the step that can SIGHUP the run, and the terminal arms must not + carry a second call.""" + content = reset_sh_content + calls = [ + (n, ln) + for n, ln in enumerate(content.splitlines(), 1) + if not ln.lstrip().startswith("#") + and re.search(r'(?> {log}\n' + f'printf "render=%s lang=%s weather=%s dir=%s\\n" "${{LITCLOCK_RUNTIME_RENDER:-unset}}" ' + f'"${{LITCLOCK_LANGUAGE:-unset}}" "${{WEATHER_ENABLED:-unset}}" ' + f'"${{LITCLOCK_RUNTIME_RENDER_DIR:-unset}}" >> {log}\n' + f'if [ -d "${{LITCLOCK_RUNTIME_RENDER_DIR:-}}" ]; then echo exists=yes >> {log}; ' + f': > "$LITCLOCK_RUNTIME_RENDER_DIR/current-quote.png"; else echo exists=no >> {log}; fi\n' + f"sleep {sleep}\n" + "echo painter-says-hello\n" + f"exit {painter_rc}\n" + ) + fake_py.chmod(0o755) + install = tmp_path / "install" + install.mkdir(exist_ok=True) + (install / "env.sh").write_text( + f"export LITCLOCK_LANGUAGE={language}\nexport LITCLOCK_RUNTIME_RENDER=false\nexport WEATHER_ENABLED=true\n" + ) + memo = tmp_path / "memo.json" + # A test may call this twice in one tmp_path: start each run clean, or + # the second run's assertions read the first run's log and memo. + log.unlink(missing_ok=True) + memo.unlink(missing_ok=True) + h = TestUpdateShStampBlockExecutes() + if elapsed_s is None: + stub = "_update_elapsed_seconds() { return 2; }\n" + elif elapsed_s == "unknown": + stub = "_update_elapsed_seconds() { return 1; }\n" + else: + stub = f"_update_elapsed_seconds() {{ echo {int(elapsed_s)}; }}\n" + if installed_budget_s is None: + stub += "_update_budget_seconds() { return 1; }\n" + else: + stub += f"_update_budget_seconds() {{ echo {int(installed_budget_s)}; }}\n" + program = ( + "set -u\n" + 'log_info() { echo "[INFO] $1"; }\n' + 'log_warn() { echo "[WARN] $1"; }\n' + 'log_error() { echo "[ERROR] $1"; }\n' + 'atomic_remove_file() { [ "$1" = /dev/null ] || rm -f "$1"; }\n' + 'atomic_write_file() { [ "$1" = /dev/null ] || printf "%s" "$2" > "$1"; }\n' + f"PYTHON={fake_py}\nINSTALL_DIR={install}\nRUNTIME_VALIDATION_MEMO_FILE={memo}\n" + f"RUNTIME_SELFTEST_RECORD_FILE={tmp_path / 'selftest.json'}\n" + f"{h._budget_helpers()}" + # AFTER the lifted helpers, which carry the script's own constants: + # a 2s bound keeps the timeout case fast, and the boundary tests + # compute against it. + "SELFTEST_TIMEOUT_S=2\nVALIDATOR_BUDGET_RESERVE_S=120\n" + f"{stub}" + + ("mktemp() { return 1; }\n" if mktemp_fails else "") + + f"{self._record_fn()}" + + f"{self._selftest_fn()}" + "_runtime_render_selftest\n" + 'echo "REACHED_END rc=$?"\n' + ) + record = tmp_path / "selftest.json" + record.unlink(missing_ok=True) + if pass_record_exists: + record.write_text('{"result": "passed", "duration_s": 1.0, "sha": null, "at_unix": 1}') + r = subprocess.run(["bash", "-c", program], cwd=REPO_ROOT, capture_output=True, text=True, timeout=60) + painter = log.read_text() if log.exists() else "" + return r, painter, (json.loads(memo.read_text()) if memo.exists() else None) + + def _record_fn(self): + body = UPDATE_SH.read_text() + assert body.count("_runtime_selftest_record_write() {") == 1 + start = body.index("_runtime_selftest_record_write() {") + end = body.index("\n}\n", start) + len("\n}\n") + fn = body[start:end] + assert 'result: "passed"' in fn and "duration_s" in fn + return fn + + @staticmethod + def _record(tmp_path): + path = tmp_path / "selftest.json" + return json.loads(path.read_text()) if path.exists() else None + + def test_a_pass_forces_the_renderer_on_with_the_devices_language_and_writes_no_memo(self, tmp_path): + r, painter, memo = self._run(tmp_path, painter_rc=0) + assert "REACHED_END rc=0" in r.stdout, r.stdout + r.stderr + assert "self-test PASSED" in r.stdout + assert "[selftest] painter-says-hello" in r.stdout, "the painter's output must reach the journal, prefixed" + assert "argv=src/literary_clock.py --dry-run --require-runtime-render" in painter, painter + assert "render=true" in painter, "the renderer must be FORCED on, whatever env.sh says" + assert "lang=xx" in painter, "env.sh must be sourced so the device's language is the one rendered" + assert "weather=false" in painter, "weather must be forced OFF, whatever env.sh says" + assert "exists=yes" in painter, "the frame directory must EXIST while the painter runs" + assert "self-test PASSED in " in r.stdout, "the duration is the evidence Stage B needs" + assert memo is None + rec = self._record(tmp_path) + assert rec is not None and rec["result"] == "passed", "a pass must leave a DURABLE record for Stage B" + assert isinstance(rec["duration_s"], (int, float)) and 0 <= rec["duration_s"] < 5, rec + assert rec["sha"] and rec["at_unix"] > 0 + + def test_a_fail_retires_an_earlier_pass_record(self, tmp_path): + _, _, memo = self._run(tmp_path, painter_rc=3, pass_record_exists=True) + assert memo is not None and memo["result"] == "selftest-failed" + assert self._record(tmp_path) is None, "Stage B must never flip on a stale pass" + + def test_a_deferral_leaves_an_earlier_pass_record_alone(self, tmp_path): + _, painter, memo = self._run(tmp_path, painter_rc=0, mktemp_fails=True, pass_record_exists=True) + assert painter == "" and memo is not None and memo["result"] == "selftest-deferred" + assert self._record(tmp_path) is not None, "nothing was asked, nothing learned; the record stands" + + def test_a_fallback_is_memoed_as_selftest_failed_with_rc_3(self, tmp_path): + r, _, memo = self._run(tmp_path, painter_rc=3) + assert "REACHED_END rc=0" in r.stdout, "a failed self-test is not an update failure" + assert "fell back to pre-rendered images" in r.stdout + assert memo is not None and memo["result"] == "selftest-failed" and memo["rc"] == 3, memo + assert "fell back" in memo["reason"] + + def test_a_dead_painter_is_memoed_with_its_rc(self, tmp_path): + r, _, memo = self._run(tmp_path, painter_rc=1) + assert "REACHED_END rc=0" in r.stdout + assert memo is not None and memo["result"] == "selftest-failed" and memo["rc"] == 1, memo + assert "exited 1" in memo["reason"] + + def test_a_timeout_is_told_apart(self, tmp_path): + r, _, memo = self._run(tmp_path, painter_rc=0, sleep=5) + assert "REACHED_END rc=0" in r.stdout + assert "did not finish" in r.stdout + assert memo is not None and memo["result"] == "selftest-failed" and memo["rc"] == 124, memo + + def test_a_disposable_frame_directory_is_removed_afterwards(self, tmp_path): + _, painter, _ = self._run(tmp_path, painter_rc=0) + d = next(ln.split("dir=", 1)[1].strip() for ln in painter.splitlines() if "dir=" in ln) + assert d and d != "unset" and d != "/run/litclock", d + assert "exists=yes" in painter, "the painter left a file in it, so the cleanup removed a NON-empty directory" + assert not Path(d).exists(), d + + def test_no_scratch_directory_means_no_run(self, tmp_path): + """litclock-dev#875 review (Codex, security, adversarial): with mktemp failed the + painter's default frame directory IS the panel's (/run/litclock), so + running anyway would overwrite the live frame. Defer instead.""" + r, painter, memo = self._run(tmp_path, painter_rc=0, mktemp_fails=True) + assert "REACHED_END rc=0" in r.stdout + assert painter == "", "the painter must not run without a scratch directory" + assert memo is not None and memo["result"] == "selftest-deferred" and "scratch" in memo["reason"], memo + + def test_an_unreadable_budget_defers_even_when_elapsed_is_known(self, tmp_path): + """litclock-dev#875 testing specialist: this arm was untested, and a mutant that read + an unreadable budget as UNLIMITED — the inversion the litclock-dev#835 helpers + forbid — stayed green.""" + r, painter, memo = self._run(tmp_path, painter_rc=0, elapsed_s=100, installed_budget_s=None) + assert "REACHED_END rc=0" in r.stdout + assert painter == "", "an unreadable budget is 'cannot afford it', never 'no budget'" + assert memo is not None and memo["result"] == "selftest-deferred" and "TimeoutStartSec" in memo["reason"], memo + + def test_no_budget_left_defers_with_a_memo_and_does_not_run_the_painter(self, tmp_path): + r, painter, memo = self._run(tmp_path, painter_rc=0, elapsed_s=1700, installed_budget_s=1800) + assert "REACHED_END rc=0" in r.stdout + assert "deferring the runtime-render self-test" in r.stdout + assert painter == "", "the painter must not run when the budget cannot hold it" + assert memo is not None and memo["result"] == "selftest-deferred", memo + + def test_enough_budget_runs_it(self, tmp_path): + _, painter, memo = self._run(tmp_path, painter_rc=0, elapsed_s=1600, installed_budget_s=1800) + assert "argv=" in painter and memo is None + + def test_the_boundary_is_exact(self, tmp_path): + # elapsed + 2 (timeout) + 120 (reserve) > budget defers; == does not. + _, painter, _ = self._run(tmp_path, painter_rc=0, elapsed_s=1678, installed_budget_s=1800) + assert "argv=" in painter, "1678+2+120 == 1800 fits" + _, painter, _ = self._run(tmp_path, painter_rc=0, elapsed_s=1679, installed_budget_s=1800) + assert painter == "", "1679+2+120 > 1800 defers" + + def test_an_unknown_budget_defers_an_unlimited_one_runs(self, tmp_path): + _, painter, memo = self._run(tmp_path, painter_rc=0, elapsed_s="unknown") + assert painter == "" and memo is not None and memo["result"] == "selftest-deferred" + _, painter, memo = self._run(tmp_path, painter_rc=0, elapsed_s=5000, installed_budget_s=0) + assert "argv=" in painter and memo is None + + def test_outside_systemd_there_is_no_budget_to_respect(self, tmp_path): + _, painter, memo = self._run(tmp_path, painter_rc=0, elapsed_s=None) + assert "argv=" in painter and memo is None diff --git a/tests/test_strings_catalog.py b/tests/test_strings_catalog.py index 7e5dcd3..16ed27d 100644 --- a/tests/test_strings_catalog.py +++ b/tests/test_strings_catalog.py @@ -149,6 +149,13 @@ def test_active_languages_clear_the_sfw_coverage_floor(self): floor = entry.get("min_coverage_pct", 80) corpus = REPO_ROOT / entry["corpus"]["path"] assert corpus.is_file(), f"{code}: corpus missing at {entry['corpus']['path']}" + # litclock-dev#870 (litclock-dev#874 red team): measure the file the DEVICE + # would read, through the same resolver — a path that escapes the + # checkout, or an empty file, is refused on the device and must + # not pass here as a healthy translation. + assert quote_corpus._registry_corpus(code) == corpus, ( + f"{code}: the device's resolver refuses {entry['corpus']['path']} (outside the checkout, or empty)" + ) quote_corpus.reset_cache() index = quote_corpus._index(corpus) sfw_minutes = { diff --git a/tests/test_timer_lead.py b/tests/test_timer_lead.py index f18514a..c23ff84 100644 --- a/tests/test_timer_lead.py +++ b/tests/test_timer_lead.py @@ -362,6 +362,9 @@ def Clear(self): class _Args: dry_run = False + # litclock-dev#871 Stage A: __main__ reads this before the dry-run + # branch, so every parser stub must carry it. + require_runtime_render = False class _Parser: def __init__(self, *a, **k): diff --git a/tests/test_update_sh.py b/tests/test_update_sh.py index 235dbbf..0cb2a40 100644 --- a/tests/test_update_sh.py +++ b/tests/test_update_sh.py @@ -1664,6 +1664,8 @@ def test_git_diff_failure_also_removes_the_marker_with_an_honest_log(self): # undoes mktemp's 0600 so the pi-user control_server can read a memo a # root-run update wrote. "$RUNTIME_VALIDATION_MEMO_FILE": "a /var/lib state file, not a repo path (and 0644, not +x)", + # litclock-dev#871 Stage A: the self-test pass record, same shape and reason. + "$RUNTIME_SELFTEST_RECORD_FILE": "a /var/lib state file, not a repo path (and 0644, not +x)", } # Targets that are a single repo file rather than a glob. @@ -2746,6 +2748,12 @@ def _run_gate(self, root, span): # TestRevertArmsReinstallTheClockUnits below. '_reinstall_clock_units_from_tree() { echo "STUB_REINSTALL_CLOCK_UNITS"; }\n' '_block_reverted_release() { echo "STUB_BLOCK_REVERTED $*"; }\n' + # litclock-dev#871 Stage A — the KEEP arm calls the self-test once the + # marker is present (it is, below). Stubbed so it neither runs the + # real painter through the probe wrapper (a fifth logged probe) nor + # dies as `command not found`; driven for real in + # tests/test_runtime_render_autostamp.py. + '_runtime_render_selftest() { echo "STUB_SELFTEST"; }\n' f"PYTHON={shlex.quote(str(wrapper))}\n" "REVERT_SHA=deadbeef\nUPDATE_FAILED_FILE=/dev/null\nHASH_FILE=/dev/null\n" # litclock-dev#531 — the KEEP arm now re-stamps the runtime-render @@ -2834,6 +2842,10 @@ def test_the_gate_passes_on_a_non_english_device(self, update_sh_content, tmp_pa f"all four probes must run on the happy path; got {probes}. Dropping an entry " "from the `for probe in ...` list is otherwise invisible." ) + # litclock-dev#871 Stage A: with the marker present, the KEEP arm asks the + # self-test after the probes. Positive, so the stub is not a silent + # `command not found` in disguise. + assert "STUB_SELFTEST" in r.stdout, r.stdout def test_the_gate_still_fails_when_the_catalog_is_gone(self, update_sh_content, tmp_path): """The litclock-dev#532 failure the gate was written for: `languages/` @@ -3592,7 +3604,10 @@ def test_the_lock_keeps_its_inherited_descriptor_on_purpose(self, update_sh_cont leaked descendant holding the lock, but under systemd's group-wide TERM the flock parent dies first and the script's signal cleanup then runs unlocked against a second updater (reproduced). The pre-existing - inheritance stays; the lock design is the issue's follow-up.""" + inheritance stays — and since 2026-09-19 that is a SETTLED decision, + not a pending follow-up: litclock-dev#847 item 2 closed as accepted, + because every production trigger activates the same oneshot and is + serialised by systemd before this lock is reached.""" executed = _executed_lines(update_sh_content) assert 'flock -n -E 75 "$LITCLOCK_UPDATE_LOCK_FILE" "$0" "$@"' in executed assert "--close" not in executed diff --git a/tests/test_wifi_retry_flow.py b/tests/test_wifi_retry_flow.py index 4ee497d..2732fa9 100644 --- a/tests/test_wifi_retry_flow.py +++ b/tests/test_wifi_retry_flow.py @@ -16,6 +16,58 @@ # ── Helpers ────────────────────────────────────────────────────────── +def _wait_for_connect_thread(timeout=30.0): + """Block until the background connect thread clears + ``setup_server.WIFI_CONNECT_IN_FLIGHT``; raise if it never does. + + ONE helper for all four test classes (litclock-dev, 2026-09-19). There were + four copy-paste `_wait_for_thread` staticmethods: one failed loudly, three + returned SILENTLY on expiry, and the silent ones are what turned a slow CI + runner into `assert True is False` at the caller's next line — a message + naming no thread, no wait and no timeout, which reads like a production bug + in the flag's lifecycle. Two runs died that way (master 53be0e79 and branch + 5be644d8) while the file passed locally every time. + + Failing loudly is not only about diagnosability, and the loud copy already + said why: the callers' `finally` un-fakes `wifi_provision`, and the connect + thread sleeps 1s before importing it, so a swallowed timeout lets a + still-live thread re-import the REAL module and shell out to + `sudo nmcli device wifi connect` on the dev box or the runner. + + The flag is re-checked AFTER the loop before raising (litclock-dev#876 review, Codex): + the deadline is tested before the sleep, so a thread that clears the flag + during the final sleep would otherwise be reported as a hang — precisely + under the scheduler delays this exists to tolerate. Demonstrated: with a + 0.2s bound and the flag cleared at 0.19s, the unguarded loop raises and + the guarded one returns. + + The bound is a runaway guard, not a schedule. A healthy wait is a little + over a second (the thread sleeps 1s to let the response flush to the phone + before it does anything), so 30s is slack for a loaded runner; only a + genuinely stuck thread pays it. Callers that want a tighter bound pass one. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if not setup_server.WIFI_CONNECT_IN_FLIGHT: + return + time.sleep(0.05) + if not setup_server.WIFI_CONNECT_IN_FLIGHT: + return + # pytest.fail, not a bare assert: this file's own convention (see the + # litclock-dev#781 note further down) — under `PYTHONOPTIMIZE=1 + # --assert=plain` a bare assert is stripped, and a stripped guard here + # would restore exactly the silent-return this helper exists to remove. + pytest.fail( + f"the background WiFi-connect thread did not clear WIFI_CONNECT_IN_FLIGHT within {timeout}s " + f"(WIFI_CONNECT_ERROR={setup_server.WIFI_CONNECT_ERROR!r}). Production clears it in a NESTED " + "`finally` (src/setup_server.py), so this is a thread still running, not one that died on the " + "way out. Look first at _resolve_location_from_ip — unstubbed it is a LIVE ip-api.com call on " + "a 1/3/9s ladder, ~33s worst case, which is what made this fail on CI before; then connect, " + "hotspot teardown/restore, and the terminate scheduler. The caller's `finally` is about to " + "un-fake wifi_provision underneath it." + ) + + class FakeRequest: """Minimal stand-in for a socket-backed HTTP request.""" @@ -65,6 +117,54 @@ def post_setup(handler): return handler.wfile.read().decode() +class TestWaitForConnectThreadHelper: + """The helper itself (litclock-dev litclock-dev#876 review). Both arms are driven with + a FAKE clock, because the window they differ in is a scheduler delay a + healthy box never produces — the mutation check on the real suite passes + with the guard removed, which is precisely why this test exists.""" + + @staticmethod + def _fake_clock(monkeypatch, *, clears_during_last_sleep): + """A clock that jumps the deadline on the first sleep, optionally + clearing the in-flight flag in the same instant — the real race: the + loop tests the deadline BEFORE it sleeps, so a flag cleared during + that sleep is invisible to the next iteration.""" + now = [1000.0] + monkeypatch.setattr(time, "monotonic", lambda: now[0]) + + def fake_sleep(_seconds): + now[0] += 999.0 + if clears_during_last_sleep: + setup_server.WIFI_CONNECT_IN_FLIGHT = False + + monkeypatch.setattr(time, "sleep", fake_sleep) + + def test_a_flag_cleared_during_the_final_sleep_is_not_a_hang(self, monkeypatch): + setup_server.WIFI_CONNECT_IN_FLIGHT = True + self._fake_clock(monkeypatch, clears_during_last_sleep=True) + _wait_for_connect_thread(timeout=5.0) # must return, not fail + + def test_a_flag_that_never_clears_fails_with_the_diagnostic(self, monkeypatch): + setup_server.WIFI_CONNECT_IN_FLIGHT = True + setup_server.WIFI_CONNECT_ERROR = "boom" + self._fake_clock(monkeypatch, clears_during_last_sleep=False) + with pytest.raises(BaseException) as exc: + _wait_for_connect_thread(timeout=5.0) + message = str(exc.value) + assert "did not clear WIFI_CONNECT_IN_FLIGHT within 5.0s" in message + assert "'boom'" in message, "the recorded error must be in the message" + assert "un-fake wifi_provision" in message, "the reason a swallowed timeout is dangerous" + # The autouse reset_state spends its full 2s drain budget on a flag + # this test deliberately left set; clear it rather than charge every + # run for a condition we fabricated. + setup_server.WIFI_CONNECT_IN_FLIGHT = False + + def test_an_already_clear_flag_returns_without_sleeping(self, monkeypatch): + setup_server.WIFI_CONNECT_IN_FLIGHT = False + monkeypatch.setattr(time, "sleep", lambda _s: pytest.fail("the happy path must not sleep")) + _wait_for_connect_thread(timeout=5.0) + + @pytest.fixture(autouse=True) def reset_globals(monkeypatch): """Reset module globals between tests. @@ -364,6 +464,31 @@ class TestConnectAndTeardown: *before* the POST and keep it alive until the thread finishes. """ + @pytest.fixture(autouse=True) + def _stub_ip_geo(self, monkeypatch): + """The REAL cause of the CI flake this class kept producing (litclock-dev#876 + review, Claude adversarial pass), and a live network call in a unit + test either way. + + This was the ONE class in the file that never stubbed the resolver — + the Ordering class and the manual-SSID harness both do. So on the + success path the background thread ran `_resolve_location_from_ip` + for real: four attempts on a 1/3/9s ladder with a 5s socket timeout, + a documented ~33s worst case (`src/location_resolver.py`), against + ip-api.com, whose free tier rate-limits. That is what outran the + wait and surfaced as `assert True is False` on master (`53be0e79`, + 2026-09-19) — the runner's own log carries the tell, a + `set_system_timezone('America/Chicago')` warning, i.e. a SUCCESSFUL + live geo lookup. + + And on any machine where the installer has run — a Pi, or a dev box — + that success path shells out to + `sudo /usr/local/lib/litclock/litclock-set-timezone`, so a plain + `pytest tests/` could change the SYSTEM TIMEZONE. Stubbing it is the + fix; the longer, louder wait beside it is a backstop, not the cure. + """ + monkeypatch.setattr(setup_server, "_resolve_location_from_ip", lambda *a, **kw: None) + def _make_post_body(self, **overrides): defaults = { "wifi_ssid": "TestNetwork", @@ -376,14 +501,7 @@ def _make_post_body(self, **overrides): defaults.update(overrides) return urllib.parse.urlencode(defaults) - @staticmethod - def _wait_for_thread(timeout=3.0): - """Wait for background daemon threads to finish.""" - deadline = time.monotonic() + timeout - while time.monotonic() < deadline: - if not setup_server.WIFI_CONNECT_IN_FLIGHT: - return - time.sleep(0.05) + _wait_for_thread = staticmethod(_wait_for_connect_thread) def test_post_rejects_own_hotspot_ssid(self, monkeypatch, tmp_env_file): """Submitting the clock's own hotspot SSID is rejected up front — no @@ -783,20 +901,7 @@ def _make_post_body(self, **overrides): defaults.update(overrides) return urllib.parse.urlencode(defaults) - @staticmethod - def _wait_for_thread(timeout=3.0): - """Fails loudly on timeout, unlike the sibling helper. - - The `finally` below un-fakes wifi_provision. The connect thread sleeps - 1s before importing it, so a silently-swallowed timeout would let a - still-live thread re-import the REAL module and shell out to - `sudo nmcli device wifi connect` on the dev box or CI runner.""" - deadline = time.monotonic() + timeout - while time.monotonic() < deadline: - if not setup_server.WIFI_CONNECT_IN_FLIGHT: - return - time.sleep(0.05) - pytest.fail("connect thread did not drain before the fake module was removed") + _wait_for_thread = staticmethod(_wait_for_connect_thread) @staticmethod def _install_fake_wp(monkeypatch, *, connect_result=(True, None), teardown_raises=False, calls=None): @@ -1055,13 +1160,7 @@ def _make_post_body(self, **overrides): defaults.update(overrides) return urllib.parse.urlencode(defaults) - @staticmethod - def _wait_for_thread(timeout=3.0): - deadline = time.monotonic() + timeout - while time.monotonic() < deadline: - if not setup_server.WIFI_CONNECT_IN_FLIGHT: - return - time.sleep(0.05) + _wait_for_thread = staticmethod(_wait_for_connect_thread) def test_wifi_failure_does_not_signal_or_sigterm(self, monkeypatch, tmp_env_file, tmp_path): """connect_to_wifi returns (False, error): no signal, no SIGTERM, @@ -1332,13 +1431,7 @@ def _make_post_body(self, **overrides): defaults.update(overrides) return urllib.parse.urlencode(defaults) - @staticmethod - def _wait_for_thread(timeout=3.0): - deadline = time.monotonic() + timeout - while time.monotonic() < deadline: - if not setup_server.WIFI_CONNECT_IN_FLIGHT: - return - time.sleep(0.05) + _wait_for_thread = staticmethod(_wait_for_connect_thread) def _fake_wp(self, connect_calls, result=(True, None)): import types From 7e1d5c2693bbe6e0899c4bf2eb7433bdf4bba3fe Mon Sep 17 00:00:00 2001 From: Ankush Kapoor <50513804+kapoorankush@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:14:35 -0500 Subject: [PATCH 2/3] fix(test,update): port the litclock-dev#879/#880 review fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports litclock-dev 7c2063ea..9c8ddc81. Both defects were carried over by the train in the parent commit, not introduced by it; both were found by the gstack /review of this branch. - tests/test_wifi_retry_flow.py: the IP-geo stub litclock-dev#876 added covered ONE class of three. Measured with a getaddrinfo probe on this checkout: 24 DNS lookups of ip-api.com across 8 tests. The resolver's success path calls set_system_timezone(), which shells out to sudo, and ENV_FILE is monkeypatched by these tests while that call is not sandboxed by anything — so on a machine where the installer has run, `pytest tests/` could change the system timezone. The fixture is module-scoped and autouse now, so a class inherits it rather than appearing on a list of the classes somebody remembered. Probe on this branch after the port: 0 attempts, 70 passed. - scripts/update.sh: ${EPOCHREALTIME/[.,]/}, not a literal dot. The separator follows LC_NUMERIC, so under a comma locale the strip matched nothing and the comma parsed as bash arithmetic's comma operator. Three measured outcomes, all wrong; the dangerous one is silent (rc 0, nothing on stderr) and IS recorded, beside "result":"passed", in the file litclock-dev#871 Stage B is meant to gate on. Production always got C from systemd; the exposure is a maintainer running the script by hand from a non-English desktop, which README's manual-update section tells owners they may do. The seven tests lift the shipped t0=/dur_ms=/duration= assignments out of _runtime_render_selftest and execute them verbatim with EPOCHREALTIME injected, rather than a copy — a copy proves nothing about the script, and neither did counting occurrences near it (an indirect-expansion mutant satisfied the count and reintroduced the bug). Five mutants verified red against THIS checkout's update.sh, including the full revert. No CHANGELOG entry: nothing here is visible to an owner. ruff clean, shellcheck clean, pytest 4645 passed / 66 skipped, vitest 209. --- scripts/update.sh | 35 ++++++++- tests/test_update_sh.py | 139 ++++++++++++++++++++++++++++++++++ tests/test_wifi_retry_flow.py | 65 ++++++++++------ 3 files changed, 212 insertions(+), 27 deletions(-) diff --git a/scripts/update.sh b/scripts/update.sh index c867d06..183170c 100755 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -1998,7 +1998,38 @@ _runtime_render_selftest() { log_info "Running the runtime-render self-test (litclock-dev#871 Stage A; inert — records a verdict, changes nothing)..." # Microseconds via EPOCHREALTIME (bash 5): the figure is compared against a # 4s render lead, and whole-second $SECONDS is ±1s on it (litclock-dev#875 red team). - t0=${EPOCHREALTIME/./} + # + # `[.,]`, not a literal dot (litclock-dev#879). EPOCHREALTIME's decimal separator + # follows LC_NUMERIC, so under a comma locale `${x/./}` matches NOTHING + # and the comma then parses as bash arithmetic's COMMA OPERATOR, which + # discards the left operand. THREE measured outcomes, all wrong, which one + # you get depending on the digits (litclock-dev#879 follow-up — the first two versions + # of this comment each described a shape that does not occur, so the + # examples below are transcripts, not reasoning): + # + # leading-zero end fraction -> bash: value too great for base + # (invalid octal), duration EMPTY + # "0.-3" / "0.-7" and friends -> `jq … tonumber` REFUSES it, rc 5, so + # the record is not written and the + # "Could not write" warning fires + # a plain wrong number, e.g. -> silent: rc 0, nothing on stderr, + # "0.0" / "0.3" and it IS recorded + # + # Only the third is the real hazard, and it is the whole reason for this + # fix: a wrong duration_s written beside "result":"passed" in the record + # litclock-dev#871 Stage B is meant to gate on, with a falsely SMALL value sailing + # through any "renders inside the 4s lead" threshold where an absurd one + # would at least look wrong to a human reading the file. The other two + # fail loudly. The PROPORTIONS were not characterised — they turn on the + # relationship between the two fractions, and guessing at them is how the + # earlier versions of this comment went wrong. + # + # A character class rather than a pinned LC_ALL: it does not depend on bash + # calling setlocale() for a `local` assignment. litclock-update.service + # runs with no locale so production always got C; the exposure is a + # maintainer running this script by hand from a non-English desktop, which + # README's manual-update section tells owners they may do. + t0=${EPOCHREALTIME/[.,]/} # Subshell: env.sh must not leak into this script (the same isolation the # RUNTIME_MARKER resolution above uses). PIPESTATUS, not the pipeline's # exit — `if cmd | sed; then` tests sed (the smoke gate's own lesson). @@ -2010,7 +2041,7 @@ _runtime_render_selftest() { timeout "$SELFTEST_TIMEOUT_S" "$PYTHON" src/literary_clock.py --dry-run --require-runtime-render 2>&1 ) | sed 's/^/[selftest] /' rc="${PIPESTATUS[0]}" - dur_ms=$(( (${EPOCHREALTIME/./} - t0) / 1000 )) + dur_ms=$(( (${EPOCHREALTIME/[.,]/} - t0) / 1000 )) duration="$((dur_ms / 1000)).$((dur_ms % 1000 / 100))" # Only ever the fresh mktemp directory: -d, and never a fallback path. [[ -n "$dir" && -d "$dir" ]] && rm -rf -- "$dir" 2>/dev/null diff --git a/tests/test_update_sh.py b/tests/test_update_sh.py index 0cb2a40..ad9a9a0 100644 --- a/tests/test_update_sh.py +++ b/tests/test_update_sh.py @@ -4010,3 +4010,142 @@ def test_the_legacy_no_lib_path_still_falls_through(self, update_sh_content, tmp r, data = self._run(update_sh_content, tmp_path, lib_sourced=False) assert "REACHED_END" in r.stdout, (r.stdout, r.stderr) assert "legacy path" in r.stdout + + +class TestSelfTestDurationIsLocaleProof: + """litclock-dev#879 — the self-test's `duration_s` under a comma `LC_NUMERIC`. + + `EPOCHREALTIME`'s decimal separator follows the locale, and the original + code stripped a LITERAL dot. Under a comma locale that matched nothing and + the comma then parsed as bash arithmetic's COMMA OPERATOR, which discards + the left operand. Three measured outcomes, all wrong; only one of them is + silent AND recorded, and that one is the hazard, because it lands beside + `"result":"passed"` in the record litclock-dev#871 Stage B gates on. + + These tests EXECUTE THE SHIPPED LINES, lifted out of `_runtime_render_selftest` + by their assignment names and run verbatim with `EPOCHREALTIME` injected + (`unset` first, which drops its dynamic nature and leaves an ordinary + variable). Two earlier versions tested a hand-written copy of those lines + instead, and a copy proves nothing about the script: a source-text + assertion that merely COUNTED `${EPOCHREALTIME/[.,]/}` occurrences passed + against a mutant that kept two correct-looking probes and did the real + arithmetic through an indirect expansion with a literal-dot strip + (litclock-dev#879 follow-up, Codex cross-model pass). Lifting the real + assignments closes that: whatever the right-hand side becomes, it is what + runs here. + """ + + _T0_SEC, _T0_FRAC = "1789955228", "932373" + _T1_SEC = "1789955232" + _DEFAULT_FRAC = "539821" + + @classmethod + def _shipped_lines(cls, update_sh_content): + """The three duration assignments, verbatim, from the shipped function.""" + body = update_sh_content[update_sh_content.index("_runtime_render_selftest() {"):] + out = {} + for name in ("t0", "dur_ms", "duration"): + hits = [ + ln.strip() for ln in body.splitlines() + if re.match(rf"\s*{name}=", ln) and not ln.lstrip().startswith("#") + ] + assert len(hits) == 1, ( + f"expected exactly one executed `{name}=` assignment in " + f"_runtime_render_selftest, found {len(hits)}: {hits}" + ) + out[name] = hits[0] + return out + + @classmethod + def _expected(cls, frac): + """What the SHIPPED arithmetic must print for this end fraction.""" + t0_us = int(cls._T0_SEC + cls._T0_FRAC) + dur_ms = (int(cls._T1_SEC + frac) - t0_us) // 1000 + return f"{dur_ms // 1000}.{dur_ms % 1000 // 100}" + + @classmethod + def _run(cls, lines, sep, frac=None): + """Run the three lifted lines with EPOCHREALTIME injected. + + `sep` is the decimal separator the locale would produce. `frac` is the + END timestamp's microseconds; it matters because the comma failure has + several shapes and which one you get turns on the digits. + """ + frac = frac or cls._DEFAULT_FRAC + script = "\n".join([ + "unset EPOCHREALTIME", + f'EPOCHREALTIME="{cls._T0_SEC}{sep}{cls._T0_FRAC}"', + lines["t0"], + f'EPOCHREALTIME="{cls._T1_SEC}{sep}{frac}"', + lines["dur_ms"], + lines["duration"], + 'echo "$duration"', + ]) + r = subprocess.run(["bash", "-c", script], capture_output=True, text=True) + return r.returncode, r.stdout.strip(), r.stderr.strip() + + @staticmethod + def _as_literal_dot(lines): + """The pre-fix code: the same lines with the character class reverted.""" + reverted = {k: v.replace("${EPOCHREALTIME/[.,]/}", "${EPOCHREALTIME/./}") for k, v in lines.items()} + assert reverted != lines, "nothing to revert — the shipped lines no longer use [.,]" + return reverted + + def test_shipped_lines_are_direct_epochrealtime_reads(self, update_sh_content): + """Both duration reads come STRAIGHT off EPOCHREALTIME. + + Not a count over the file: a count is satisfied by a decoy in a + comment, or by correct-looking probes beside an indirect read that + does the real work. This pins the right-hand sides that actually feed + the arithmetic. + """ + lines = self._shipped_lines(update_sh_content) + assert lines["t0"] == "t0=${EPOCHREALTIME/[.,]/}", lines["t0"] + assert lines["dur_ms"] == "dur_ms=$(( (${EPOCHREALTIME/[.,]/} - t0) / 1000 ))", lines["dur_ms"] + + def test_shipped_lines_are_correct_with_a_dot(self, update_sh_content): + lines = self._shipped_lines(update_sh_content) + assert self._run(lines, ".") == (0, self._expected(self._DEFAULT_FRAC), "") + + def test_shipped_lines_are_correct_with_a_comma(self, update_sh_content): + """The fix: a comma locale must produce the same elapsed time.""" + lines = self._shipped_lines(update_sh_content) + assert self._run(lines, ",") == (0, self._expected(self._DEFAULT_FRAC), "") + + def test_shipped_lines_are_correct_with_a_leading_zero_fraction(self, update_sh_content): + """The fraction that is an invalid OCTAL literal to bash's arithmetic + must be ordinary microseconds here.""" + lines = self._shipped_lines(update_sh_content) + assert self._run(lines, ",", frac="039821") == (0, self._expected("039821"), "") + + def test_the_control_literal_dot_is_correct_with_a_dot(self, update_sh_content): + """The CONTROL. The old pattern was never wrong in an English locale, + which is exactly why it shipped green.""" + old = self._as_literal_dot(self._shipped_lines(update_sh_content)) + assert self._run(old, ".") == (0, self._expected(self._DEFAULT_FRAC), "") + + def test_the_mutant_literal_dot_is_silently_wrong_with_a_comma(self, update_sh_content): + """The hazard: status 0, NOTHING on stderr, and a duration that is not + the elapsed time, so no caller can tell. + + The wrong value is asserted as a PROPERTY, not a literal. The comma + operator's survivor depends on the digits — `0.0`, `0.3`, `0.-3` and + empty have all been measured — so pinning one would record an + arithmetic accident rather than the defect. + """ + old = self._as_literal_dot(self._shipped_lines(update_sh_content)) + rc, out, err = self._run(old, ",") + assert rc == 0, f"expected the old pattern to fail SILENTLY, got rc={rc}" + assert err == "", f"expected no diagnostic, got: {err!r}" + assert out != self._expected(self._DEFAULT_FRAC), ( + "the old pattern parsed a comma correctly — premise gone" + ) + + def test_the_mutant_literal_dot_is_noisy_on_a_leading_zero_fraction(self, update_sh_content): + """The other direction, kept because the first version of these tests + used THIS shape while describing the silent one above, and never + captured stderr to notice. Wrong out loud rather than quietly.""" + old = self._as_literal_dot(self._shipped_lines(update_sh_content)) + rc, out, err = self._run(old, ",", frac="039821") + assert "value too great for base" in err, f"expected bash's octal diagnostic, got: {err!r}" + assert out != self._expected("039821"), "the old pattern parsed a comma correctly — premise gone" diff --git a/tests/test_wifi_retry_flow.py b/tests/test_wifi_retry_flow.py index 2732fa9..25cf566 100644 --- a/tests/test_wifi_retry_flow.py +++ b/tests/test_wifi_retry_flow.py @@ -13,6 +13,46 @@ import setup_server + +@pytest.fixture(autouse=True) +def _stub_ip_geo(monkeypatch): + """Stub the IP-geo resolver for EVERY test in this file. + + A live network call in a unit test either way, and on any machine where + the installer has run the success path shells out to + `sudo /usr/local/lib/litclock/litclock-set-timezone`, so a plain + `pytest tests/` could change the SYSTEM TIMEZONE. `ENV_FILE` is + monkeypatched by these tests; that sudo call is not sandboxed by anything. + + It is also the cause of the CI flake this file kept producing: four + attempts on a 1/3/9s ladder with a 5s socket timeout, a documented ~33s + worst case (`src/location_resolver.py`), against ip-api.com, whose free + tier rate-limits. That outran the wait and surfaced as `assert True is + False` on master (`53be0e79`, 2026-09-19) — the runner's own log carries + the tell, a `set_system_timezone('America/Chicago')` warning, i.e. a + SUCCESSFUL live geo lookup. + + MODULE-WIDE, not per class, and that is the whole point of this revision + (litclock-dev#879). The first version of this fixture lived on + `TestConnectAndTeardown` and its docstring asserted that "the Ordering + class and the manual-SSID harness both do" stub the resolver. The harness + does; the Ordering class does NOT — only 1 of its 5 tests stubs it + inline, and the shared `_install_fake_wp` helper fakes `connect_to_wifi`, + `teardown_hotspot` and `create_hotspot` and nothing else. Measured with a + socket probe over the whole file: 24 DNS lookups of ip-api.com attributed + to 8 tests across three classes, and 3 of those tests FAIL outright when + the network is blocked. A per-class fixture is a list of the classes + somebody remembered; the resolver has no business being reachable from + any test here, so the stub belongs at module scope where a new class + inherits it by default. + + A test that wants to OBSERVE the call still overrides it locally — a + later `monkeypatch.setattr` wins — which is what + `test_success_path_call_ordering` does. + """ + monkeypatch.setattr(setup_server, "_resolve_location_from_ip", lambda *a, **kw: None) + + # ── Helpers ────────────────────────────────────────────────────────── @@ -464,31 +504,6 @@ class TestConnectAndTeardown: *before* the POST and keep it alive until the thread finishes. """ - @pytest.fixture(autouse=True) - def _stub_ip_geo(self, monkeypatch): - """The REAL cause of the CI flake this class kept producing (litclock-dev#876 - review, Claude adversarial pass), and a live network call in a unit - test either way. - - This was the ONE class in the file that never stubbed the resolver — - the Ordering class and the manual-SSID harness both do. So on the - success path the background thread ran `_resolve_location_from_ip` - for real: four attempts on a 1/3/9s ladder with a 5s socket timeout, - a documented ~33s worst case (`src/location_resolver.py`), against - ip-api.com, whose free tier rate-limits. That is what outran the - wait and surfaced as `assert True is False` on master (`53be0e79`, - 2026-09-19) — the runner's own log carries the tell, a - `set_system_timezone('America/Chicago')` warning, i.e. a SUCCESSFUL - live geo lookup. - - And on any machine where the installer has run — a Pi, or a dev box — - that success path shells out to - `sudo /usr/local/lib/litclock/litclock-set-timezone`, so a plain - `pytest tests/` could change the SYSTEM TIMEZONE. Stubbing it is the - fix; the longer, louder wait beside it is a backstop, not the cure. - """ - monkeypatch.setattr(setup_server, "_resolve_location_from_ip", lambda *a, **kw: None) - def _make_post_body(self, **overrides): defaults = { "wifi_ssid": "TestNetwork", From c0779f5a71fc22673a538de79fc7aee13049a8db Mon Sep 17 00:00:00 2001 From: Ankush Kapoor <50513804+kapoorankush@users.noreply.github.com> Date: Mon, 21 Sep 2026 06:41:47 -0500 Subject: [PATCH 3/3] fix(test,update): port the litclock-dev#882 review round, and two findings of this repo's own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports litclock-dev 9c8ddc81..7438f611, plus two fixes that exist only here. From dev: - `${EPOCHREALTIME//[^0-9]/}` — strip every non-digit, not just `[.,]`. glibc gives fa_IR and ps_AF U+066B (٫), and measured, `[.,]` leaves it in place for rc 0, duration=0.0, `invalid arithmetic operator` on stderr, and a wrong value jq accepts and records. Same hazard class, rarer locale family. - The test that holds it stopped asserting source text. Four review rounds each broke the previous round's rule with a shape it did not model (a decoy in a comment, an indirect expansion, a `local`-prefixed assignment, and finally `printf -v duration '0.0'`, which is not an assignment at all). So a second class EXECUTES the shipped `_runtime_render_selftest` with its helpers stubbed and a painter that sleeps a known time, and asserts on the value `_runtime_selftest_record_write` is handed, with a second painter as the control. Verified against THIS checkout: printf -v and eval red, the full revert red, and narrowing back to `[.,]` red. - The hazard test now uses a fraction whose wrong output `jq tonumber` ACCEPTS, so it exercises the silent-and-recorded shape the fix exists for; the earlier default produced `0.-3`, which jq refuses, i.e. the loud band. Found in this repo, by the review of this branch at its final HEAD: - `tests/test_wifi_retry_flow.py:161` carried `litclock-dev litclock-dev#876` — the requalifier prefixed a `#876` on a line that already said `litclock-dev` with a space. The only occurrence in the tree; this is the dev#767 doubled- qualifier class, so it is worth catching rather than leaving. - `PUBLIC_NUMBER_CEILING` was still 67, two ports behind: #68 and #69 landed the v0.227.0 and v0.228.0 trains and this lands as #70. Audited 68..70 across all tracked files — ZERO bare `#N` in that range, so nothing in the tree changes meaning — and raised to 70 with that audit recorded, which is the deliberate act the module docstring asks for. Also removed a stray empty `0755/` directory in the working tree, a `mkdir -m` typo from 2026-09-13. ruff clean, shellcheck clean, pytest 4648 passed / 66 skipped, vitest 209. --- scripts/update.sh | 28 +++-- tests/test_issue_ref_namespace.py | 8 +- tests/test_update_sh.py | 191 ++++++++++++++++++++++++++++-- tests/test_wifi_retry_flow.py | 2 +- 4 files changed, 207 insertions(+), 22 deletions(-) diff --git a/scripts/update.sh b/scripts/update.sh index 183170c..586701c 100755 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -1999,13 +1999,23 @@ _runtime_render_selftest() { # Microseconds via EPOCHREALTIME (bash 5): the figure is compared against a # 4s render lead, and whole-second $SECONDS is ±1s on it (litclock-dev#875 red team). # - # `[.,]`, not a literal dot (litclock-dev#879). EPOCHREALTIME's decimal separator - # follows LC_NUMERIC, so under a comma locale `${x/./}` matches NOTHING - # and the comma then parses as bash arithmetic's COMMA OPERATOR, which - # discards the left operand. THREE measured outcomes, all wrong, which one - # you get depending on the digits (litclock-dev#879 follow-up — the first two versions - # of this comment each described a shape that does not occur, so the - # examples below are transcripts, not reasoning): + # Strip EVERY non-digit (litclock-dev#879, widened in litclock-dev#881). EPOCHREALTIME's decimal + # separator follows LC_NUMERIC, so under a comma locale the original + # `${x/./}` matched NOTHING and the comma then parsed as bash arithmetic's + # COMMA OPERATOR, which discards the left operand. + # + # `[^0-9]` rather than the `[.,]` this first shipped as: the separator is + # not limited to those two. glibc gives fa_IR and ps_AF U+066B (٫), and + # measured, `${x/[.,]/}` leaves it in place — rc 0, `duration=0.0`, + # `invalid arithmetic operator` on stderr, and a wrong value that jq + # accepts and records. Same hazard class, rarer locale family, and an + # exhaustive class costs nothing (litclock-dev#881 follow-up review). `//`, because + # stripping every non-digit needs the replace-all form. + # + # THREE measured outcomes under the old pattern, all wrong, which one you + # get depending on the digits (the first two versions of this comment each + # described a shape that does not occur, so the examples below are + # transcripts, not reasoning): # # leading-zero end fraction -> bash: value too great for base # (invalid octal), duration EMPTY @@ -2029,7 +2039,7 @@ _runtime_render_selftest() { # runs with no locale so production always got C; the exposure is a # maintainer running this script by hand from a non-English desktop, which # README's manual-update section tells owners they may do. - t0=${EPOCHREALTIME/[.,]/} + t0=${EPOCHREALTIME//[^0-9]/} # Subshell: env.sh must not leak into this script (the same isolation the # RUNTIME_MARKER resolution above uses). PIPESTATUS, not the pipeline's # exit — `if cmd | sed; then` tests sed (the smoke gate's own lesson). @@ -2041,7 +2051,7 @@ _runtime_render_selftest() { timeout "$SELFTEST_TIMEOUT_S" "$PYTHON" src/literary_clock.py --dry-run --require-runtime-render 2>&1 ) | sed 's/^/[selftest] /' rc="${PIPESTATUS[0]}" - dur_ms=$(( (${EPOCHREALTIME/[.,]/} - t0) / 1000 )) + dur_ms=$(( (${EPOCHREALTIME//[^0-9]/} - t0) / 1000 )) duration="$((dur_ms / 1000)).$((dur_ms % 1000 / 100))" # Only ever the fresh mktemp directory: -d, and never a fallback path. [[ -n "$dir" && -d "$dir" ]] && rm -rf -- "$dir" 2>/dev/null diff --git a/tests/test_issue_ref_namespace.py b/tests/test_issue_ref_namespace.py index 3158cf9..de63711 100644 --- a/tests/test_issue_ref_namespace.py +++ b/tests/test_issue_ref_namespace.py @@ -34,7 +34,13 @@ # across all tracked files: exactly ONE, in .github/DEPENDABOT-NOTE.md, and # it genuinely refers to this repo's own PRs #63-#66. Nothing in that range # is a development-repo reference. Ceiling raised to 67. -PUBLIC_NUMBER_CEILING = 67 +# 2026-09-21 (v0.229.0 port): highest issue still 57; PRs #68 and #69 landed +# the v0.227.0 and v0.228.0 port trains, and this port lands as #70. Audited +# 68..70 across all tracked files: ZERO bare `#N` in that range, so nothing +# in the tree changes meaning. Raised so a future note about one of those +# three PRs can say `#68` and mean it. Caught by the port review — the +# ceiling had been left behind by two ports. +PUBLIC_NUMBER_CEILING = 70 # EVERY tracked text file, not an extension allowlist. The first version of # this listed ten extensions, inherited from the audit's own scan command, and diff --git a/tests/test_update_sh.py b/tests/test_update_sh.py index ad9a9a0..e5fab31 100644 --- a/tests/test_update_sh.py +++ b/tests/test_update_sh.py @@ -20,6 +20,7 @@ import shutil import subprocess import sys +import textwrap import time from pathlib import Path @@ -4037,23 +4038,66 @@ class TestSelfTestDurationIsLocaleProof: _T0_SEC, _T0_FRAC = "1789955228", "932373" _T1_SEC = "1789955232" - _DEFAULT_FRAC = "539821" + # The end fraction that makes the OLD pattern silent AND recorded — the + # hazard the fix exists for. The first version defaulted to "539821", + # whose `0.-3` output `jq tonumber` REFUSES (rc 5), so the record is never + # written and the failure is loud downstream: the class docstring claimed + # the silent-and-recorded shape and no test covered it (litclock-dev#881 follow-up + # review). `999821` yields `0.0`, which jq accepts. + _DEFAULT_FRAC = "999821" + _JQ_REFUSED_FRAC = "539821" + + # Assignment prefixes bash accepts. `local t0=…` is an assignment to t0 and + # the extractor below MUST see it: a mutant that leaves the bare line as a + # decoy and does the real work one line later behind `local` reintroduces + # the bug with every test still green (litclock-dev#881 follow-up, Codex + # cross-model pass — reproduced, 7 passed against buggy production). + _DECL = r"(?:local|declare|typeset|readonly|export)\s+" + + @classmethod + def _function_body(cls, update_sh_content): + """Just `_runtime_render_selftest`, not the rest of the file. + + The first version scanned from the function's opening brace to EOF, so + a same-named assignment anywhere below it was in scope. + """ + marker = "_runtime_render_selftest() {" + start = update_sh_content.index(marker) + len(marker) + end = update_sh_content.index("\n}\n", start) + return update_sh_content[start:end] @classmethod def _shipped_lines(cls, update_sh_content): - """The three duration assignments, verbatim, from the shipped function.""" - body = update_sh_content[update_sh_content.index("_runtime_render_selftest() {"):] + """The three duration assignments, verbatim, from the shipped function. + + Every finding here is loud. A silent skip is what let the decoy mutant + through, so an unexpected shape fails the test rather than being + filtered out of the match set. + """ + body = cls._function_body(update_sh_content) + executable = [ln for ln in body.splitlines() if not ln.lstrip().startswith("#")] out = {} for name in ("t0", "dur_ms", "duration"): - hits = [ - ln.strip() for ln in body.splitlines() - if re.match(rf"\s*{name}=", ln) and not ln.lstrip().startswith("#") - ] + # Deliberately matches DECLARED assignments too, so they cannot hide. + hits = [ln.strip() for ln in executable if re.match(rf"\s*(?:{cls._DECL})?{name}=", ln)] assert len(hits) == 1, ( f"expected exactly one executed `{name}=` assignment in " f"_runtime_render_selftest, found {len(hits)}: {hits}" ) + assert not re.match(rf"\s*{cls._DECL}", hits[0]), ( + f"the `{name}` assignment carries a declaration prefix, so lifting it " + f"in isolation would not reproduce what the function runs: {hits[0]!r}" + ) out[name] = hits[0] + + # Nothing else in the function may read the clock. A correct-looking + # probe beside an indirect read is the other half of the same trick. + reads = [ln.strip() for ln in executable if "EPOCHREALTIME" in ln] + unexpected = [ln for ln in reads if ln not in (out["t0"], out["dur_ms"])] + assert not unexpected, ( + "unexpected EPOCHREALTIME read(s) in _runtime_render_selftest — the two " + f"duration assignments are the only ones that may touch it: {unexpected}" + ) return out @classmethod @@ -4087,8 +4131,8 @@ def _run(cls, lines, sep, frac=None): @staticmethod def _as_literal_dot(lines): """The pre-fix code: the same lines with the character class reverted.""" - reverted = {k: v.replace("${EPOCHREALTIME/[.,]/}", "${EPOCHREALTIME/./}") for k, v in lines.items()} - assert reverted != lines, "nothing to revert — the shipped lines no longer use [.,]" + reverted = {k: v.replace("${EPOCHREALTIME//[^0-9]/}", "${EPOCHREALTIME/./}") for k, v in lines.items()} + assert reverted != lines, "nothing to revert — the shipped lines no longer use the non-digit strip" return reverted def test_shipped_lines_are_direct_epochrealtime_reads(self, update_sh_content): @@ -4100,8 +4144,8 @@ def test_shipped_lines_are_direct_epochrealtime_reads(self, update_sh_content): the arithmetic. """ lines = self._shipped_lines(update_sh_content) - assert lines["t0"] == "t0=${EPOCHREALTIME/[.,]/}", lines["t0"] - assert lines["dur_ms"] == "dur_ms=$(( (${EPOCHREALTIME/[.,]/} - t0) / 1000 ))", lines["dur_ms"] + assert lines["t0"] == "t0=${EPOCHREALTIME//[^0-9]/}", lines["t0"] + assert lines["dur_ms"] == "dur_ms=$(( (${EPOCHREALTIME//[^0-9]/} - t0) / 1000 ))", lines["dur_ms"] def test_shipped_lines_are_correct_with_a_dot(self, update_sh_content): lines = self._shipped_lines(update_sh_content) @@ -4140,6 +4184,39 @@ def test_the_mutant_literal_dot_is_silently_wrong_with_a_comma(self, update_sh_c assert out != self._expected(self._DEFAULT_FRAC), ( "the old pattern parsed a comma correctly — premise gone" ) + # …and it is RECORDED. Silence at the bash layer is only half the + # hazard: `_runtime_selftest_record_write` builds the JSON through + # `jq … tonumber`, so a value jq refuses never reaches the file and + # fails loudly instead. This asserts the wrong value gets through. + if shutil.which("jq") is None: + pytest.skip("the record is written through jq") + r = subprocess.run( + ["jq", "-nc", "--arg", "d", out, '{duration_s: ($d | tonumber)}'], + capture_output=True, text=True, + ) + assert r.returncode == 0, ( + f"jq refused {out!r}, so this fraction exercises the LOUD band, not the " + f"silent-and-recorded hazard this test claims: {r.stderr.strip()}" + ) + assert '"duration_s"' in r.stdout, r.stdout + + def test_the_mutant_literal_dot_can_also_land_in_the_loud_band(self, update_sh_content): + """The other silent-at-bash shape, kept so the split is on the record. + + `0.-3` clears bash with rc 0 and no stderr exactly as above, but `jq + tonumber` REFUSES it, so `_runtime_selftest_record_write` writes + nothing and logs "Could not write" instead. Same bug, caught + downstream by luck rather than design — which is why the default + fraction is the one jq accepts. + """ + if shutil.which("jq") is None: + pytest.skip("the record is written through jq") + old = self._as_literal_dot(self._shipped_lines(update_sh_content)) + rc, out, err = self._run(old, ",", frac=self._JQ_REFUSED_FRAC) + assert (rc, err) == (0, ""), (rc, err) + r = subprocess.run(["jq", "-nc", "--arg", "d", out, '($d | tonumber)'], + capture_output=True, text=True) + assert r.returncode != 0, f"expected jq to refuse {out!r}, it accepted it" def test_the_mutant_literal_dot_is_noisy_on_a_leading_zero_fraction(self, update_sh_content): """The other direction, kept because the first version of these tests @@ -4149,3 +4226,95 @@ def test_the_mutant_literal_dot_is_noisy_on_a_leading_zero_fraction(self, update rc, out, err = self._run(old, ",", frac="039821") assert "value too great for base" in err, f"expected bash's octal diagnostic, got: {err!r}" assert out != self._expected("039821"), "the old pattern parsed a comma correctly — premise gone" + + +class TestSelfTestDurationSurvivesToTheRecord: + """litclock-dev#881 — what `_runtime_render_selftest` actually HANDS to the + record writer, observed by running it. + + `TestSelfTestDurationIsLocaleProof` above lifts the three duration + assignments and runs them under injected separators, which is what proves + the strip is locale-complete. But every assertion it makes is ultimately + about SOURCE TEXT, and four review rounds each broke the previous round's + rule with a shape it did not model: a decoy in a comment, an indirect + expansion, a `local`-prefixed assignment doing the real work beside a bare + decoy, and finally `printf -v duration '0.0'` — a write form that is not an + assignment at all, so no amount of assignment-matching sees it. + + Adding a fifth rule loses to a sixth trick. This class stops playing: it + executes the REAL function with its helpers stubbed and a painter that + sleeps a known time, then asserts on the value `_runtime_selftest_record_write` + receives. Anything that corrupts the duration between the clock read and the + record — another write form, a recompute, laundering through a second + variable, a subshell — changes that value and fails this, whatever it looks + like in the source. + + The two layers are complementary and both are needed: this one uses the real + clock, so it cannot vary the decimal separator; the lifted-line tests can, + and cannot see past the lines they lift. + """ + + SLEEP_S = 1.2 + TOLERANCE_S = 1.0 # generous: a loaded box may add most of a second + + @staticmethod + def _function_text(): + sh = (REPO_ROOT / "scripts" / "update.sh").read_text() + start = sh.index("_runtime_render_selftest() {") + return sh[start:sh.index("\n}\n", start) + 3] + + @classmethod + def _run_real_function(cls, tmp_path, sleep_s=None): + """Run the shipped function; return what the record writer was handed.""" + painter = tmp_path / "painter" + painter.write_text(f"#!/bin/bash\nsleep {sleep_s or cls.SLEEP_S}\n") + painter.chmod(0o755) + harness = textwrap.dedent(f""" + set -o pipefail + SELFTEST_TIMEOUT_S=60 + INSTALL_DIR={tmp_path}/nonexistent + RUNTIME_SELFTEST_RECORD_FILE=/dev/null + PYTHON={painter} + _validation_fits_remaining_budget() {{ return 0; }} + log_info() {{ :; }} + log_warn() {{ :; }} + atomic_remove_file() {{ :; }} + _runtime_validation_memo_write() {{ echo "MEMO rc=$2"; }} + _runtime_selftest_record_write() {{ echo "RECORDED=$1"; }} + """) + cls._function_text() + "\n_runtime_render_selftest\n" + r = subprocess.run(["bash", "-c", harness], capture_output=True, text=True, cwd=REPO_ROOT) + return r + + def test_the_recorded_duration_is_the_real_elapsed_time(self, tmp_path): + """The painter sleeps a known time; the record must receive it. + + This is the assertion four rounds of source-text rules were reaching + for. It is immune to how the value is produced. + """ + r = self._run_real_function(tmp_path) + assert r.returncode == 0, (r.returncode, r.stdout, r.stderr) + assert "MEMO" not in r.stdout, f"the self-test did not take the PASS arm:\n{r.stdout}" + m = re.search(r"^RECORDED=(\S+)$", r.stdout, re.M) + assert m, f"_runtime_selftest_record_write was never called:\n{r.stdout}\n{r.stderr}" + recorded = float(m.group(1)) + assert abs(recorded - self.SLEEP_S) <= self.TOLERANCE_S, ( + f"recorded duration {recorded}s is not the {self.SLEEP_S}s the painter took — " + f"something between the clock read and the record corrupted it" + ) + + def test_a_longer_paint_records_a_longer_duration(self, tmp_path): + """The CONTROL for the test above. + + A hardcoded constant, a zero, or an epoch would satisfy a single + measurement just as well; this one only passes if the recorded value + actually TRACKS how long the painter ran. + """ + short = self._run_real_function(tmp_path, sleep_s=0.3) + long = self._run_real_function(tmp_path, sleep_s=2.5) + s = float(re.search(r"^RECORDED=(\S+)$", short.stdout, re.M).group(1)) + ln = float(re.search(r"^RECORDED=(\S+)$", long.stdout, re.M).group(1)) + assert ln - s >= 1.5, ( + f"a painter that ran 2.2s longer moved the recorded duration by only " + f"{ln - s}s ({s} -> {ln}) — the value is not tracking the clock" + ) + diff --git a/tests/test_wifi_retry_flow.py b/tests/test_wifi_retry_flow.py index 25cf566..383fa3c 100644 --- a/tests/test_wifi_retry_flow.py +++ b/tests/test_wifi_retry_flow.py @@ -158,7 +158,7 @@ def post_setup(handler): class TestWaitForConnectThreadHelper: - """The helper itself (litclock-dev litclock-dev#876 review). Both arms are driven with + """The helper itself (litclock-dev#876 review). Both arms are driven with a FAKE clock, because the window they differ in is a scheduler delay a healthy box never produces — the mutation check on the real suite passes with the guard removed, which is precisely why this test exists."""