fix(handoff,network): port the litclock-dev#660 and litclock-dev#667 fixes - #56
Merged
Merged
Conversation
…fixes Ports the two pre-RC blockers fixed upstream in litclock-dev#668 and #669. Both were found by review of the port PRs that brought #620 and #645 here, so this closes the loop on that train. litclock-dev#660 — the PWA "Factory reset" (litclock-reset.service runs `--wipe-wifi --strict-env-wipe --poweroff --yes`) did not rotate the setup-WiFi key that #620 had just made permanent. Because that path wipes WiFi, the next power-on raises the setup hotspot broadcasting LitClock-Setup with the PREVIOUS owner's key, permanently, surviving every reset the new owner later performs — while the card promises "Erases everything". v0.223.0 had no such leak, so this was a regression #620 introduced. The discriminator is `--wipe-wifi` AND the power-off, not either alone: a hand-run `--poweroff` never raises a hotspot, and `--wipe-wifi --reboot` is #620's same-owner WiFi reset, which must preserve. litclock-dev#667 — #645's link-local blanking removed the address from the same-IP short-circuit's coverage, so every dhcp4-change on a clock that cannot get a lease fell through to a full e-ink render (3 renders measured for 3 identical events). Review found the same hole for a genuinely empty address, which NM's defaults make at least as likely. Coalescing now keys on the observed address in its own tmpfs marker, while the blanked value stays the only thing diagnostics sees. Port notes: - `scripts/reset-setup.sh` HAND-MERGED. This repo has the litclock-dev#528/#636 SSH-off gate, which does not exist upstream at all — it was authored here in #52/#53 — so upstream's `--poweroff` arm has nothing to order against. The rotation call is placed BEFORE `disable_ssh_for_handoff`, matching the gift arm and for the same reason: the rotation fails closed and can exit 1, leaving the device with its current owner, who then still needs SSH to diagnose a read-only card. Verified after merging: the gate body is byte-identical to master and all four call sites survive. - `CLAUDE.md` deliberately NOT ported. The upstream half rewrites a #620 QA block this repo does not have (`grep -c 620 CLAUDE.md` → 0), so applying it would invent that block here. Same phantom-conflict trap as the #641/#648 half of the previous train. - `tests/test_reset_setup_sh.py` hand-merged per hunk rather than wholesale: the conflict replaced the old `_extract_block` helper with the new harness, but a whole-file resolution would have discarded this repo's SSH-gate tests. All twelve survive. - 4 bare dev#NNN references requalified. - Verified nothing was dropped: of every line the two upstream commits added, none is missing here. `test_gift_mode_rotates_hotspot_password_before_disabling_ssh` — added when the two features first met in #54 — is now parametrised over BOTH handoff arms, since #660 gave the `--poweroff` arm the same pairing and nothing covered it. Its anchor is also fixed: it matched the function name in the COMMENT above the call, so inverting the order in the new arm kept it green. Mutation-verified in both arms after the fix. Testing: - ruff + shellcheck clean - pytest: 3392 passed, 62 skipped - npm run test:js: 191 passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the two pre-RC blockers fixed upstream in litclock-dev#668 and litclock-dev#669. Both were found reviewing the PRs that brought #620 and #645 here (#54, #55), so this closes the loop on that train. Last code change before the v0.224.0 RC.
litclock-dev#660 — the PWA Factory reset kept the setup-WiFi key
litclock-reset.servicerunsreset-setup.sh --wipe-wifi --strict-env-wipe --poweroff --yes. Because that wipes WiFi, the next power-on raises the setup hotspot broadcastingLitClock-Setupwith the previous owner's key — permanently, surviving every reset the new owner later performs — while the card promises "Erases everything — all settings and WiFi." v0.223.0 had no such leak because the key regenerated every cycle, so this was a regression #620 introduced.The discriminator is
--wipe-wifiAND the power-off, not either alone. A hand-run--poweroffnever raises a hotspot, and--wipe-wifi --rebootis #620's same-owner WiFi reset, which must preserve the key. Both are pinned by tests.prepare-for-cloning.shalso powers the Pi off now: deleting the key wasn't sufficient, because the script re-enables first-boot, so booting the prepared master even once re-minted a fresh permanent key that then rode every clone.litclock-dev#667 — a clock with no IP repainted on every retry
#645's link-local blanking removed the address from the same-IP short-circuit's coverage, so every
dhcp4-changeon a clock that can't get a lease fell through to a full e-ink render — three renders measured for three identical events. That's the #309 A4 unbounded-render failure: each render holds the SPI bus and spends panel cycles a device with no UPS can't get back. Review found the same hole for a genuinely empty address, which NM's defaults make at least as likely.Coalescing now keys on the observed address in its own tmpfs marker, while the blanked value stays the only thing diagnostics sees.
Port notes — three judgement calls, not cherry-picks
scripts/reset-setup.shhand-merged. This repo has the litclock-dev#528/#636 SSH-off gate, which does not exist upstream at all — it was authored here in #52/#53 — so upstream's--poweroffarm has nothing to order against. The rotation call is placed beforedisable_ssh_for_handoffin both arms, for the same reason it is in the gift arm: the rotation fails closed and canexit 1, leaving the device with its current owner, who then still needs SSH to diagnose a read-only card. Verified after merging: gate body byte-identical to master, all four call sites survive,WIPE_WIFIguard intact.CLAUDE.mddeliberately not ported. The upstream half rewrites a #620 QA block this repo doesn't have (grep -c 620 CLAUDE.md→ 0), so applying it would invent that block here — the same phantom-conflict trap as the #641/#648 half of the previous train.tests/test_reset_setup_sh.pymerged per hunk, not wholesale. The conflict swapped an old helper for the new harness; a--theirswould have discarded this repo's twelve SSH-gate tests. All survive.Also: 4 refs requalified, and every line the two upstream commits added is verified present here.
Porter-authored changes, and why
test_gift_mode_rotates_hotspot_password_before_disabling_ssh(added in feat(wifi): persist the setup hotspot password per device; let the clone warning print #54) is parametrised over both handoff arms — #660 gave the--poweroffarm the same pairing and nothing covered it. Its anchor is also fixed: it matched the function name in the comment above the call, so inverting the order in the new arm kept it green.disable_ssh_for_handoff. Without it every terminal-branch test emittedcommand not foundon stderr — swallowed, since the harness omitsset -e— which polluted assertion messages and elided this repo's security gate from every behavioural test. The ordering is now asserted behaviourally as well as structurally, including that a failed rotation never reaches the SSH gate.Mutation-verified: inverting the order in either arm, deleting either call, or dropping the
WIPE_WIFIguard all turn tests red.Testing
pytest: 3392 passed, 62 skippednpm run test:js: 191 passedFollow-up filed, not bundled
Clone-prep is now structurally the same kind of terminal handoff as the gift arm, but has no SSH posture — whatever the master card holds is frozen into every image, with no boot in between for anyone to notice. Pre-existing rather than introduced here, so it gets its own issue.