From 4bded210e4672bb3809f9848d0dd0d8e60fe3648 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Thu, 13 Aug 2026 13:35:05 -0400 Subject: [PATCH] geenforce: the backup hover names both dates, because they mean different things "Checked 13 Aug 1:20 PM. Verified the backup taken 12 Aug 5:20 PM is still current." Two facts, and one date could not carry both. lastseenat moves on every successful confirmation and proves the check is still running. collectedat moves only when the configuration actually CHANGED and says what is stored. Showing lastseenat alone hid what is in the backup; showing collectedat alone read as "last backup was a month ago" on a machine that is perfectly healthy, because a stable config writes no new revision. The hover now states the check and the capture separately and says outright that the second being older is the normal case. The stale wording gets the same treatment: it names the check that stopped AND the newest copy that exists, which is the thing someone recovering from it actually needs. collectedat is exposed as backupcollectedat. Both stay tooltip-only - the badge is still just the kind and a colour, so nothing here changes what a new backup kind has to do to inherit it. --- docs/BACKUP-KINDS.md | 9 ++++- plugins/geenforce/api/routes.py | 11 ++++++- .../frontend/views/EnforcementReports.vue | 25 ++++++++++---- .../test_plugins/test_geenforce_reporting.py | 33 ++++++++++++++++--- 4 files changed, 66 insertions(+), 12 deletions(-) diff --git a/docs/BACKUP-KINDS.md b/docs/BACKUP-KINDS.md index d93c7bc..ac65053 100644 --- a/docs/BACKUP-KINDS.md +++ b/docs/BACKUP-KINDS.md @@ -75,10 +75,17 @@ they would have to go and look up. | Field | Meaning | |-------|---------| | `backupkind` | Which kind was most recently confirmed. `null` = no backup at all. | -| `backuplastseen` | When it was last CONFIRMED (ISO). Tooltip only. | +| `backuplastseen` | When it was last CONFIRMED still current (ISO). Tooltip only. | +| `backupcollectedat` | When the config was last CAPTURED (ISO). Only moves on a real change. Tooltip only. | | `backupok` | `true` good, `false` stale, `null` nothing to judge. | | `backupstaleafterdays` | The threshold in force, so the UI can explain itself. | +Both dates are reported because they answer different questions and one cannot +stand for both. The hover reads *"ntlars: checked 13 Aug 1:20 PM. Verified the +backup taken 12 Aug 5:20 PM is still current."* - the first date proves the +check is running, the second says what is actually stored. Collapsing them was +what made a healthy machine look neglected. + `backupok` is deliberately **tri-state**. `null` means there is no revision for that host, or the check is disabled - and it renders as no badge, never green. "Never seen" must not read as healthy. diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index d501f43..fab0fb2 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -960,7 +960,7 @@ def _asset_facts(hostnames): 'toolpluginid': None, 'displayrole': None, 'backupkind': None, 'backuplastseen': None, - 'backuplastseenat': None, + 'backuplastseenat': None, 'backupcollectedat': None, } _attach_controlled_assets(facts, assetidtohost) @@ -1083,6 +1083,14 @@ def _attach_backup_state(facts, hostnames): # Raw value kept beside the wire string so the verdict compares # datetimes rather than re-parsing its own output. entry['backuplastseenat'] = revision.lastseenat + # collectedat is when this configuration was CAPTURED, and it only + # moves when the config actually changed. Pairing it with lastseenat + # is what lets the hover say "checked X, and what it verified is + # still the backup taken at Y" instead of one date that has to mean + # both and ends up meaning neither. + entry['backupcollectedat'] = ( + revision.collectedat.isoformat() + 'Z' + if revision.collectedat else None) def _backup_ok(lastseenat, cutoff): @@ -1225,6 +1233,7 @@ def list_reports(): # trust; the date alone reads as "nothing has happened since", which # is the healthy steady state and looks like neglect. 'backupok': _backup_ok(known.get('backuplastseenat'), backupcutoff), + 'backupcollectedat': known.get('backupcollectedat'), 'backupstaleafterdays': backupafterdays, 'scopename': report.scopename, 'phase': report.phase, diff --git a/plugins/geenforce/frontend/views/EnforcementReports.vue b/plugins/geenforce/frontend/views/EnforcementReports.vue index 414fe5b..a9ae87a 100644 --- a/plugins/geenforce/frontend/views/EnforcementReports.vue +++ b/plugins/geenforce/frontend/views/EnforcementReports.vue @@ -213,17 +213,30 @@ function backupClass(report) { return '' } +// Two dates, and they mean different things. checked = when the collector last +// CONFIRMED this config; taken = when the config was last CAPTURED, which only +// moves when something actually changed. Showing one date forced it to stand +// for both, which is what made a healthy machine look neglected. function backupTitle(report) { - const seen = report.backuplastseen ? formatDate(report.backuplastseen) : 'never' + const checked = report.backuplastseen ? formatDate(report.backuplastseen) : 'never' + const taken = report.backupcollectedat ? formatDate(report.backupcollectedat) : null + const kind = report.backupkind + if (report.backupok === true) { - return `${report.backupkind}: checked and verified ${seen}.` - + ' Config unchanged since, which is why the date does not move.' + return taken + ? `${kind}: checked ${checked}. Verified the backup taken ${taken} is still current.` + : `${kind}: checked and verified ${checked}.` } if (report.backupok === false) { - return `${report.backupkind}: last confirmed ${seen}, more than` - + ` ${report.backupstaleafterdays} day(s) ago - the backup has stopped running.` + const age = `more than ${report.backupstaleafterdays} day(s) ago` + return taken + ? `${kind}: last checked ${checked}, ${age} - the backup has stopped running.` + + ` The newest copy is the one taken ${taken}.` + : `${kind}: last checked ${checked}, ${age} - the backup has stopped running.` } - return `${report.backupkind}: last confirmed ${seen}.` + return taken + ? `${kind}: last checked ${checked}; backup taken ${taken}.` + : `${kind}: last checked ${checked}.` } function staleTitle(report) { diff --git a/tests/test_plugins/test_geenforce_reporting.py b/tests/test_plugins/test_geenforce_reporting.py index ae814a3..4f371f7 100644 --- a/tests/test_plugins/test_geenforce_reporting.py +++ b/tests/test_plugins/test_geenforce_reporting.py @@ -412,10 +412,13 @@ def _backup_revision(db, hostname, kind, ageda): db.session.add(asset) db.session.flush() db.session.add(Computer(assetid=asset.assetid, hostname=hostname)) - db.session.add(BackupRevision(assetid=asset.assetid, backupkind=kind, - sourcehostname=hostname, - contenthash='0' * 64, - lastseenat=_utcnow() - timedelta(days=ageda))) + db.session.add(BackupRevision( + assetid=asset.assetid, backupkind=kind, sourcehostname=hostname, + contenthash='0' * 64, + # captured well before the last confirmation on purpose: an unchanged + # config is the normal case, and the two dates must not be conflated + collectedat=_utcnow() - timedelta(days=ageda + 30), + lastseenat=_utcnow() - timedelta(days=ageda))) db.session.commit() @@ -461,3 +464,25 @@ def test_no_backup_at_all_is_unknown_not_good(client, db, app, auth_headers): row = _row_for(client, auth_headers, 'WJBAK03') assert row['backupkind'] is None assert row['backupok'] is None + + +def test_the_capture_date_is_reported_separately_from_the_check(client, db, app, auth_headers): + """Two dates, two meanings: when it was last CHECKED vs last CAPTURED. + + collectedat only moves when the config actually changed, so a healthy + machine has a recent check against an old capture. The hover says both; + one date would have to stand for both and ends up meaning neither. + """ + _seed_and_publish(app) + secret = _token(client, auth_headers, ['geenforce.report']) + _backup_revision(db, 'WJBAK04', 'ntlars', ageda=1) + client.post('/api/geenforce/report', json={ + 'hostname': 'WJBAK04', 'scopename': 'gea-shopfloor-cmm', 'counts': {}}, + headers={'X-API-Key': secret}) + + row = _row_for(client, auth_headers, 'WJBAK04') + assert row['backupok'] is True + assert row['backuplastseen'] is not None + assert row['backupcollectedat'] is not None + assert row['backupcollectedat'] < row['backuplastseen'], \ + 'captured before it was last confirmed'