From 28af92aaecb2a67b1f5a0a076aa75cb041f8aaf9 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Wed, 26 Aug 2026 10:07:51 -0400 Subject: [PATCH] An unmeasurable backup must not hide a working one CI caught this: judging a host by its stalest kind treated a never-confirmed revision (lastseenat NULL) as the worst case, so it won. The verdict for a NULL confirmation is tri-state None, which renders as NO BADGE - so a host with a healthy ntlars backup and one backfilled row showed nothing at all. Hiding a working backup is the one thing that badge must never do. A NULL confirmation is not stale, it is unmeasurable. The stalest kind is now chosen among kinds that can actually be judged, which is the rule staleness.py already applies: a chain becomes measurable the first time its PC posts and says nothing until then. If every kind is unmeasurable the host still reports which kind it has, with a None verdict and no badge. The test that caught it pinned the old newest-wins behaviour, which part markers made wrong; it now pins stalest-measurable-wins and keeps its NULL clause, which was right all along and is what found the bug. Also worth recording how it was missed: the change was verified against test_geenforce_reporting.py and test_backups.py, and this assertion lives in test_geenforce_publish_gate.py, which was never run. --- plugins/geenforce/api/routes.py | 26 ++++++++++++++----- .../test_geenforce_publish_gate.py | 22 +++++++++++++--- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index 2dec9c5f..9e4ffa39 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -1111,17 +1111,29 @@ def _attach_backup_state(facts, hostnames): # rows are already newest-confirmed-first, so the first is freshest. freshest[kindkey] = revision + # Among kinds that can be JUDGED, the stalest decides. A kind whose newest + # revision has never been confirmed (lastseenat NULL - a backfilled row, or + # one written before the column existed) is not stale; it is unmeasurable, + # and it must not win. + # + # Treating NULL as "worst" looked right and was wrong: a host with a healthy + # ntlars backup and one never-confirmed row showed NO BADGE, because the + # verdict for a NULL confirmation is tri-state None. A working backup then + # renders as nothing at all, which is the one thing the badge must never do. + # This is the same rule staleness.py already applies: a chain becomes + # measurable the first time its PC posts, and says nothing until then. stalest = {} for (key, _kind), revision in freshest.items(): - current = stalest.get(key) - if current is None: - stalest[key] = revision - continue - # None (never confirmed) is worse than any date. if revision.lastseenat is None: + continue + current = stalest.get(key) + if current is None or revision.lastseenat < current.lastseenat: stalest[key] = revision - elif current.lastseenat is not None and revision.lastseenat < current.lastseenat: - stalest[key] = revision + + # Every kind unmeasurable: fall back to any revision so the host still + # reports which kind it has, with a None verdict and no badge. + for (key, _kind), revision in freshest.items(): + stalest.setdefault(key, revision) for key, revision in stalest.items(): entry = facts.get(key) diff --git a/tests/test_plugins/test_geenforce_publish_gate.py b/tests/test_plugins/test_geenforce_publish_gate.py index 88e98e90..6b8255c5 100644 --- a/tests/test_plugins/test_geenforce_publish_gate.py +++ b/tests/test_plugins/test_geenforce_publish_gate.py @@ -155,9 +155,20 @@ def test_hosts_below_libversion_reports_the_floor(db): # -- backup state on the reports table --------------------------------------- -def test_backup_state_prefers_the_newest_confirmed_revision(db): - """A host with several revisions shows the most recently CONFIRMED one, and - a revision that has never been confirmed (lastseenat NULL) never wins. +def test_backup_state_reports_the_stalest_measurable_kind(db): + """A host is judged by the WORST backup it has, not the best. + + This used to prefer the most recently confirmed revision. That was right + while a PC reported one kind, and became wrong when part markers arrived: a + marker bay reports ntlars AND partmarker, so a daily ntlars post kept the + badge green while the marker backup had been failing for a month. A host is + healthy only when everything it backs up is current, and the badge names the + kind that is not. + + A revision that has never been confirmed (lastseenat NULL) still never wins. + It is not stale, it is unmeasurable - and letting it win hid a working + backup entirely, because the verdict for a NULL confirmation is tri-state + None, which renders as no badge at all. Ordering happens in Python on purpose. Doing it in SQL needs ORDER BY ... NULLS LAST, which SQLite accepts and MySQL rejects outright - @@ -191,7 +202,10 @@ def test_backup_state_prefers_the_newest_confirmed_revision(db): facts = {'wjpc01': {'backupkind': None, 'backuplastseen': None}} _attach_backup_state(facts, {'WJPC01'}) - assert facts['wjpc01']['backupkind'] == 'udc' + # ntlars is 9 days old, udc was confirmed just now: the host is judged by + # ntlars, and the badge names it so somebody knows WHICH backup stopped. + assert facts['wjpc01']['backupkind'] == 'ntlars' + # 'files' has never been confirmed, so it neither wins nor blanks the badge. assert facts['wjpc01']['backuplastseen'] is not None