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'