From 1e884dc02ab336d93b73edde48c2411c27c20ec9 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Thu, 13 Aug 2026 09:53:56 -0400 Subject: [PATCH] geenforce: judge silence on both clocks, not just the server's The fleet table has two time columns and staleness only looked at one. A PC can go quiet in either direction: silent to the server, or still posting while its own cycle has stopped advancing. Only the first was caught, so a machine whose Last check-in had not moved since the previous morning still showed 'ok'. The two are tested independently rather than by taking the newer of them. receivedat is the server's own record and cannot be argued with; lastcheckin is asserted by the client, so a wrong clock there must not be able to vouch for a PC the server has not actually heard from. Checking them separately means a future-dated lastcheckin cannot mask real silence. The tooltip now names both times, since which one stopped says what kind of failure it is. --- plugins/geenforce/api/routes.py | 17 +++++-- .../frontend/views/EnforcementReports.vue | 8 +-- .../test_plugins/test_geenforce_reporting.py | 51 +++++++++++++++++++ 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index 86b8fb2..aa12163 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -1116,9 +1116,20 @@ def list_reports(): latest_cache[key] = _current_published_version(*key) latest = latest_cache[key] known = facts.get((report.hostname or '').lower(), {}) - isstale = (stalecutoff is not None - and (report.receivedat is None - or report.receivedat < stalecutoff)) + # Two separate ways to have gone quiet, and the table has a column for + # each. receivedat is the SERVER's record of the last report it got; + # lastcheckin is the client's own claim about when it last ran. A PC can + # fail either way round - silent to the server, or still posting while + # its own cycle has stopped advancing - so both count. + # + # They are checked independently rather than by taking the newer of the + # two: lastcheckin is client-asserted, so a wrong or future value there + # must not be able to mask real silence on receivedat. + isstale = stalecutoff is not None and ( + report.receivedat is None + or report.receivedat < stalecutoff + or (report.lastcheckin is not None + and report.lastcheckin < stalecutoff)) data.append({ 'reportid': report.reportid, 'hostname': report.hostname, diff --git a/plugins/geenforce/frontend/views/EnforcementReports.vue b/plugins/geenforce/frontend/views/EnforcementReports.vue index 9b4cc46..0f1eeb7 100644 --- a/plugins/geenforce/frontend/views/EnforcementReports.vue +++ b/plugins/geenforce/frontend/views/EnforcementReports.vue @@ -192,9 +192,11 @@ function statusClass(status) { } function staleTitle(report) { - const last = report.receivedat ? formatDate(report.receivedat) : 'never' - return `No report for over ${report.staleafterminutes} minutes.` - + ` Last heard ${last}, reporting "${report.status}".` + const received = report.receivedat ? formatDate(report.receivedat) : 'never' + const checkin = report.lastcheckin ? formatDate(report.lastcheckin) : 'not reported' + return `Nothing heard for over ${report.staleafterminutes} minutes.` + + ` Last report received ${received}, PC last checked in ${checkin},` + + ` reporting "${report.status}".` } function actionClass(action) { return { installed: 'badge-info', skipped: 'badge-success', failed: 'badge-danger', diff --git a/tests/test_plugins/test_geenforce_reporting.py b/tests/test_plugins/test_geenforce_reporting.py index db0da61..daedef3 100644 --- a/tests/test_plugins/test_geenforce_reporting.py +++ b/tests/test_plugins/test_geenforce_reporting.py @@ -285,3 +285,54 @@ def test_zero_disables_the_stale_check(client, db, app, auth_headers): headers=auth_headers).get_json()['data'][0] assert row['isstale'] is False assert row['staleafterminutes'] == 0 + + +def test_a_pc_still_posting_with_a_stalled_checkin_is_stale(client, db, app, + auth_headers): + """The other way round: the server hears from it, but its own cycle stopped. + + receivedat stays current while the PC's self-reported lastcheckin does not + advance. The fleet table has a column for each, and either going quiet is a + problem worth seeing. + """ + from datetime import timedelta + from plugins.geenforce.service import _utcnow + _seed_and_publish(app) + secret = _token(client, auth_headers, ['geenforce.report']) + client.post('/api/geenforce/report', json={ + 'hostname': 'WJCMM01', 'scopename': 'gea-shopfloor-cmm', + 'appliedversion': 1, 'enforcerversion': '2.6', + 'lastcheckin': (_utcnow() - timedelta(hours=22)).isoformat() + 'Z', + 'counts': {'installed': 1, 'skipped': 0, 'failed': 0, 'filtered': 0}, + 'results': [{'name': 'Alpha', 'action': 'installed'}], + }, headers={'X-API-Key': secret}) + + row = client.get('/api/geenforce/reports', + headers=auth_headers).get_json()['data'][0] + assert row['status'] == 'ok', 'the cycle itself reported clean' + assert row['isstale'] is True, 'but it has not checked in for 22 hours' + + +def test_a_future_checkin_cannot_mask_server_side_silence(client, db, app, + auth_headers): + """lastcheckin is client-asserted, so it must not be able to vouch for a PC. + + Taking the newer of the two timestamps would let a PC with a wrong clock + report itself healthy forever. receivedat is checked on its own. + """ + from datetime import timedelta + from plugins.geenforce.service import _utcnow + _seed_and_publish(app) + secret = _token(client, auth_headers, ['geenforce.report']) + client.post('/api/geenforce/report', json={ + 'hostname': 'WJCMM01', 'scopename': 'gea-shopfloor-cmm', + 'appliedversion': 1, 'enforcerversion': '2.6', + 'lastcheckin': (_utcnow() + timedelta(days=3)).isoformat() + 'Z', + 'counts': {'installed': 1, 'skipped': 0, 'failed': 0, 'filtered': 0}, + 'results': [{'name': 'Alpha', 'action': 'installed'}], + }, headers={'X-API-Key': secret}) + _age_report(db, 'WJCMM01', minutes=60 * 26) + + row = client.get('/api/geenforce/reports', + headers=auth_headers).get_json()['data'][0] + assert row['isstale'] is True