From 0969c55d4af5af46917add266d461c9e288fd8af Mon Sep 17 00:00:00 2001 From: Josh Poole Date: Wed, 26 Aug 2026 12:03:31 +0100 Subject: [PATCH] 20260826 - Backfill the wizard-completed flag so pre-June nodes can register retina-telemetry is about to gate registration on /data/retina-gui/setup-wizard-completed, so that a node cannot register while its config is still the shipped Greenwich/Crystal Palace default. That flag only arrived in aee29a6 (2026-06-24). Nodes that completed the wizard before then have none, and would be blocked from registering forever: the same failure the gate exists to prevent, and the reason this has to ship and deploy ahead of the telemetry side. The evidence is a location in user.yml, because that is the override layer. The merged config.yml always carries a location, so one there proves nothing, whereas an entry in user.yml means someone chose it. /towers/select has written it since 4afa307 (2026-03-23), three months before the flag, so every node in the gap is covered. Verified on two live nodes, including one configured on 2026-06-30. Not a coordinate check against the default: a node genuinely sited near Greenwich would be refused registration for life, and it would make a config default load-bearing across two repos. Co-Authored-By: Claude Opus 5 --- src/app.py | 13 ++++++++ src/device_state.py | 58 +++++++++++++++++++++++++++++++++++ tests/test_device_state.py | 63 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) diff --git a/src/app.py b/src/app.py index 736a575..f62a43f 100644 --- a/src/app.py +++ b/src/app.py @@ -198,6 +198,19 @@ def _reapply_shell_agreement(): # A calibration run cannot survive a GUI restart — any lock left behind is stale device_state.release_calibration_lock() +# Nodes that finished setup before the completion flag existed have none, and +# retina-telemetry will not register a node without it. Runs on every boot +# rather than once because there is nowhere to record that it has been done, +# and it is a no-op the moment the flag is present. +# +# Guarded because this is the only thing that parses user.yml at import time: +# an unparseable one would otherwise stop the GUI booting at all, which is a +# worse failure than the missing flag this exists to repair. +try: + device_state.backfill_setup_wizard_completed(config_mgr.load_user_config()) +except Exception: + pass + # Enforce radar at the Docker level: stop and remove retina-spectrum if it is running. # retina-spectrum is only allowed while the wizard location step or config toggle is active. if config_mgr.is_retina_node_installed(): diff --git a/src/device_state.py b/src/device_state.py index 88b5da5..52235ae 100644 --- a/src/device_state.py +++ b/src/device_state.py @@ -64,6 +64,10 @@ def __init__(self, data_dir, mender_services, mender_conf_path, self.mender_conf_backup_dir = mender_conf_backup_dir self.mender_conf_backup_path = mender_conf_backup_path self.setup_wizard_file = os.path.join(data_dir, "setup-wizard.json") + # Also read by the retina-telemetry container, which will not register a + # node without it: it is what proves the owner has been through the + # tower step, so the config being reported is theirs rather than the + # shipped default. A cross-repo contract, like the consent file below. self.setup_wizard_completed_flag = os.path.join(data_dir, "setup-wizard-completed") self.calibrate_lock_file = os.path.join(data_dir, "calibrate.lock") self.towers_cache_file = os.path.join(data_dir, "towers-cache.json") @@ -536,6 +540,60 @@ def mark_setup_wizard_completed(self): with open(self.setup_wizard_completed_flag, "w") as f: f.write(datetime.now().isoformat()) + def backfill_setup_wizard_completed(self, user_config: dict | None) -> bool: + """Write the completion flag for a node that finished setup before it existed. + + retina-telemetry gates registration on this flag, so that a node cannot + register while its config is still the shipped Greenwich/Crystal Palace + default. The flag only arrived in aee29a6 (2026-06-24), and nodes that + completed the wizard before then have none. Without this they would be + blocked from registering forever, which is the same failure the gate + exists to prevent. + + `location` in user.yml is the evidence, because that is the *override* + layer: the merged config.yml always carries a location, so the presence + of one there proves nothing, whereas an entry in user.yml means someone + chose it. `/towers/select` has written it since 4afa307 (2026-03-23), + three months before the flag, so every node in the gap is covered. + + Deliberately not a coordinate check against the default. A node genuinely + sited near Greenwich would be refused registration for life, and it would + make a config default load-bearing across two repos. + + Returns: + True if a flag was written. False if one already existed (its + original timestamp is kept: this answers "when was setup finished", + and a backfill has not finished anything), or if there is no + evidence the owner ever chose a location. + """ + if self.has_completed_setup_wizard(): + return False + if not self._has_user_chosen_location(user_config): + return False + try: + self.mark_setup_wizard_completed() + except OSError: + return False # Best effort on startup, as elsewhere in this class. + return True + + @staticmethod + def _has_user_chosen_location(user_config: dict | None) -> bool: + """Whether user.yml records a receiver position the owner picked. + + Both coordinates are required. A partial block is not evidence of a + completed tower step, and `0` is a legitimate latitude, so this tests + for presence rather than truthiness. + """ + if not isinstance(user_config, dict): + return False + location = user_config.get("location") + if not isinstance(location, dict): + return False + rx = location.get("rx") + if not isinstance(rx, dict): + return False + return rx.get("latitude") is not None and rx.get("longitude") is not None + def is_setup_wizard_in_progress(self) -> bool: """Check if setup wizard is active (not completed).""" step = self.get_setup_wizard_step() diff --git a/tests/test_device_state.py b/tests/test_device_state.py index ca8a534..8e27480 100644 --- a/tests/test_device_state.py +++ b/tests/test_device_state.py @@ -432,6 +432,69 @@ def test_malformed_cache_file_treated_as_none(self, ds): assert ds.get_towers_cache() is None +class TestBackfillSetupWizardCompleted: + """Test the flag backfill for nodes that finished setup before it existed. + + retina-telemetry gates registration on this flag, so a false negative here + strands a fully configured node permanently. The evidence is a location in + user.yml, which `/towers/select` has written since 4afa307 (2026-03-23), + three months before the flag arrived in aee29a6 (2026-06-24). + """ + + CONFIGURED = {"location": {"rx": {"latitude": 42.241528, "longitude": -72.648361}}} + + def test_writes_the_flag_for_a_configured_node(self, ds): + """The case this exists for: an old node with a real location.""" + assert ds.backfill_setup_wizard_completed(self.CONFIGURED) is True + assert ds.has_completed_setup_wizard() + + def test_leaves_an_unconfigured_node_blocked(self, ds): + """No evidence the owner chose anything, so no flag. This node should + stay unregistered rather than report the Greenwich default.""" + assert ds.backfill_setup_wizard_completed({"capture": {"fc": 503000000}}) is False + assert not ds.has_completed_setup_wizard() + + def test_empty_user_config_is_not_evidence(self, ds): + for empty in ({}, None): + assert ds.backfill_setup_wizard_completed(empty) is False + assert not ds.has_completed_setup_wizard() + + def test_partial_coordinates_are_not_evidence(self, ds): + """A half-written block does not prove a completed tower step.""" + half = {"location": {"rx": {"latitude": 42.241528}}} + assert ds.backfill_setup_wizard_completed(half) is False + assert not ds.has_completed_setup_wizard() + + def test_zero_is_a_real_coordinate(self, ds): + """Null Island is a legitimate latitude and longitude. Testing these + for truthiness rather than presence would strand a node on the equator + or the prime meridian.""" + origin = {"location": {"rx": {"latitude": 0, "longitude": 0}}} + assert ds.backfill_setup_wizard_completed(origin) is True + + def test_existing_flag_is_not_redated(self, ds): + """The flag answers "when was setup finished", and a backfill has not + finished anything. Rewriting it would also make a re-run look recent.""" + ds.mark_setup_wizard_completed() + with open(ds.setup_wizard_completed_flag) as f: + original = f.read() + + assert ds.backfill_setup_wizard_completed(self.CONFIGURED) is False + with open(ds.setup_wizard_completed_flag) as f: + assert f.read() == original + + def test_malformed_user_config_does_not_raise(self, ds): + """Startup must not die on a config it cannot make sense of.""" + for junk in ("a string", ["a", "list"], {"location": "not a dict"}, + {"location": {"rx": "not a dict"}}): + assert ds.backfill_setup_wizard_completed(junk) is False + + def test_unwritable_data_dir_does_not_raise(self, ds): + """Best effort on startup, as elsewhere in this class.""" + with patch.object(ds, "mark_setup_wizard_completed", side_effect=OSError): + assert ds.backfill_setup_wizard_completed(self.CONFIGURED) is False + + class TestTelemetryConsent: """Test the consent records retina-telemetry refuses to register without.