From 494b837bae2fea9a50594706b437df715d72c658 Mon Sep 17 00:00:00 2001 From: "benjamin.baker" Date: Wed, 9 Sep 2026 15:40:40 -0400 Subject: [PATCH] Make alertscope actually target fleet alerts alertscope was stored, serialised and editable in the authoring form but never read when alerts were handed to an endpoint, so every fleet alert went to every machine regardless of what was entered. The control implied a capability nothing enforced. get_fleet_alerts now resolves the polling endpoint's facts and filters each alert through its scope. Supported keys: hostname, assettype, computertype, location, businessunit. A key may hold one value or a list (any match), matching is case-insensitive since these are operator-typed names, and multiple keys narrow together. Unrecognised keys are ignored rather than treated as match-nothing. A typo like {"hostnam": "BAY-01"} would otherwise mute the alert fleet-wide and look identical to a delivery failure; sending wider than intended is the safe direction for something whose job is to warn people. A garbled non-dict scope falls back to the whole fleet for the same reason. The real fix for typos belongs at authoring time, not delivery. BusinessUnit is not exported from shopdb.api, so its name is resolved by table rather than importing the model and breaking the plugin import contract. Ten tests cover the narrowing cases and the fail-open behaviour. --- plugins/endpoints/api/routes.py | 95 ++++++++++++ .../test_endpoints_fleet_alerts.py | 141 ++++++++++++++++++ 2 files changed, 236 insertions(+) diff --git a/plugins/endpoints/api/routes.py b/plugins/endpoints/api/routes.py index 4125bb23..503970e0 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -3175,6 +3175,93 @@ def serve_operator_report_image(operatorreportid: int): _DEFAULT_ALERT_TYPE_COLOR = '#17a2b8' +# Scope keys an alert may be narrowed by. +# rather than treated as a match-nothing, so a typo cannot silently mute an +# alert - see _alert_targets_endpoint. +_ALERT_SCOPE_KEYS = ('hostname', 'assettype', 'computertype', 'location', + 'businessunit') + + +def _endpoint_scope_facts(hostname): + """The scope-able attributes of the endpoint asking for its alerts. + + Returns a dict of lowercased strings keyed by _ALERT_SCOPE_KEYS, or None if + the hostname resolves to no computer. Values that are unset on the asset are + absent from the dict, which makes them unmatchable rather than empty-string + matchable. + """ + from plugins.computers.models import Computer, ComputerType + from shopdb.api import Location, AssetType + + row = (db.session.query(Computer.hostname, AssetType.assettype, + ComputerType.computertype, Location.locationname, + Asset.businessunitid) + .join(Asset, Asset.assetid == Computer.assetid) + .outerjoin(ComputerType, + ComputerType.computertypeid == Computer.computertypeid) + .outerjoin(AssetType, AssetType.assettypeid == Asset.assettypeid) + .outerjoin(Location, Location.locationid == Asset.locationid) + .filter(db.func.lower(Computer.hostname) == hostname.lower()) + .first()) + if row is None: + return None + + hostname_value, assettype, computertype, locationname, businessunitid = row + + facts = {'hostname': hostname_value, 'assettype': assettype, + 'computertype': computertype, 'location': locationname} + + # BusinessUnit is not on the plugin contract surface (shopdb.api), so resolve + # its name by table rather than importing the model and breaking the import + # contract the way the mailer import did. + if businessunitid is not None: + name = db.session.execute( + db.text('SELECT businessunit FROM businessunits ' + 'WHERE businessunitid = :bid'), + {'bid': businessunitid}).scalar() + facts['businessunit'] = name + + return {k: str(v).strip().lower() for k, v in facts.items() + if v is not None and str(v).strip()} + + +def _alert_targets_endpoint(alertscope, facts): + """Does an alert's scope include this endpoint? + + An absent, empty or unusable scope means the whole fleet - that is the + default and the historical behaviour. A scope with keys narrows the alert: + every recognised key must match, and a key may hold a single value or a list + of values, any of which matches. Matching is case-insensitive because these + are operator-typed names, not identifiers. + + Unrecognised keys are IGNORED rather than failing the match. A scope is + authored by hand and a typo like {"hostnam": "BAY-01"} would otherwise mute + the alert everywhere and look like a delivery failure. Ignoring the key + sends the alert wider than intended, which is the safe direction for + something whose purpose is to warn people. + """ + if not isinstance(alertscope, dict) or not alertscope: + return True + + for key in _ALERT_SCOPE_KEYS: + if key not in alertscope: + continue + wanted = alertscope[key] + if wanted is None: + continue + if isinstance(wanted, (list, tuple, set)): + candidates = {str(v).strip().lower() for v in wanted if v is not None} + else: + candidates = {str(wanted).strip().lower()} + candidates.discard('') + if not candidates: + continue + if facts.get(key) not in candidates: + return False + + return True + + @endpoints_bp.route('/alerts', methods=['GET']) def get_fleet_alerts(): """Fleet alerts for one endpoint by hostname (collector-token auth). @@ -3228,8 +3315,16 @@ def get_fleet_alerts(): results = query.all() + # Resolved once per poll. An unknown hostname cannot happen here - the + # asset lookup above already returned - but guard anyway so a missing + # computer row means fleet-wide rather than a crash. + facts = _endpoint_scope_facts(hostname) or {} + alerts = [] for notification, ntype in results: + if not _alert_targets_endpoint(notification.alertscope, facts): + continue + title = notification.notification[:100] if notification.notification else '' if '\n' in title: title = title.split('\n')[0][:100] or '(no title)' diff --git a/tests/test_plugins/test_endpoints_fleet_alerts.py b/tests/test_plugins/test_endpoints_fleet_alerts.py index 490bfba2..681ac8ed 100644 --- a/tests/test_plugins/test_endpoints_fleet_alerts.py +++ b/tests/test_plugins/test_endpoints_fleet_alerts.py @@ -309,3 +309,144 @@ def test_non_fleet_notification_is_never_delivered(client, db, app): assert resp.status_code == 200 assert resp.get_json()['data'] == [] + + +# --- targeting ------------------------------------------------------------- +# +# alertscope narrows a fleet alert to part of the floor. It was previously +# stored, serialised and editable in the authoring form but never read here, so +# every alert went to every endpoint no matter what was entered - a control that +# looked like it worked and did not. These pin the behaviour it should have had. + + +def _ids(client, app, hostname): + resp = client.get(f'{ALERTS_URL}?hostname={hostname}', + headers=_collector_headers(app)) + assert resp.status_code == 200, resp.get_data(as_text=True) + return [a['notificationId'] for a in resp.get_json()['data']] + + +def test_no_scope_still_means_the_whole_fleet(client, db, app): + """The default is unchanged: an unscoped alert reaches everyone.""" + _computer(db, 'BAY-01') + _computer(db, 'BAY-02') + alert = _fleet_alert(db) + + assert _ids(client, app, 'BAY-01') == [alert.notificationid] + assert _ids(client, app, 'BAY-02') == [alert.notificationid] + + +def test_an_empty_scope_means_the_whole_fleet(client, db, app): + """An empty dict is not a match-nothing.""" + _computer(db, 'BAY-01') + alert = _fleet_alert(db) + alert.alertscope = {} + db.session.commit() + + assert _ids(client, app, 'BAY-01') == [alert.notificationid] + + +def test_hostname_scope_targets_one_endpoint(client, db, app): + """The narrowest case, and the one the test card drives.""" + _computer(db, 'BAY-01') + _computer(db, 'BAY-02') + alert = _fleet_alert(db) + alert.alertscope = {'hostname': 'BAY-01'} + db.session.commit() + + assert _ids(client, app, 'BAY-01') == [alert.notificationid] + assert _ids(client, app, 'BAY-02') == [] + + +def test_hostname_scope_is_case_insensitive(client, db, app): + """Operators type these by hand; case must not decide delivery.""" + _computer(db, 'BAY-01') + alert = _fleet_alert(db) + alert.alertscope = {'hostname': 'bay-01'} + db.session.commit() + + assert _ids(client, app, 'BAY-01') == [alert.notificationid] + + +def test_a_scope_may_list_several_values(client, db, app): + """Any value in a list matches, so a scope can name a handful of bays.""" + _computer(db, 'BAY-01') + _computer(db, 'BAY-02') + _computer(db, 'BAY-03') + alert = _fleet_alert(db) + alert.alertscope = {'hostname': ['BAY-01', 'BAY-03']} + db.session.commit() + + assert _ids(client, app, 'BAY-01') == [alert.notificationid] + assert _ids(client, app, 'BAY-02') == [] + assert _ids(client, app, 'BAY-03') == [alert.notificationid] + + +def test_computertype_scope_targets_a_class_of_machine(client, db, app): + """Targeting by what the machine is, not which one it is.""" + _computer(db, 'BAY-01') + alert = _fleet_alert(db) + alert.alertscope = {'computertype': 'CMM PC'} + db.session.commit() + + assert _ids(client, app, 'BAY-01') == [alert.notificationid] + + alert.alertscope = {'computertype': 'Kiosk'} + db.session.commit() + assert _ids(client, app, 'BAY-01') == [] + + +def test_several_scope_keys_must_all_match(client, db, app): + """Keys narrow together - it is an AND, not an OR.""" + _computer(db, 'BAY-01') + alert = _fleet_alert(db) + + alert.alertscope = {'hostname': 'BAY-01', 'computertype': 'CMM PC'} + db.session.commit() + assert _ids(client, app, 'BAY-01') == [alert.notificationid] + + # Right host, wrong type: the alert must not land. + alert.alertscope = {'hostname': 'BAY-01', 'computertype': 'Kiosk'} + db.session.commit() + assert _ids(client, app, 'BAY-01') == [] + + +def test_an_unknown_scope_key_is_ignored_not_fatal(client, db, app): + """A typo must not silently mute an alert everywhere. + + Scopes are authored by hand. If {"hostnam": "BAY-01"} matched nothing, the + alert would vanish fleet-wide and look like a delivery failure. Sending it + wider than intended is the safe direction for something whose job is to warn + people, so unrecognised keys are ignored. + """ + _computer(db, 'BAY-01') + alert = _fleet_alert(db) + alert.alertscope = {'hostnam': 'BAY-01'} + db.session.commit() + + assert _ids(client, app, 'BAY-01') == [alert.notificationid] + + +def test_a_non_dict_scope_falls_back_to_the_whole_fleet(client, db, app): + """Garbled scope JSON must not stop an alert reaching the floor.""" + _computer(db, 'BAY-01') + alert = _fleet_alert(db) + alert.alertscope = 'BAY-01' + db.session.commit() + + assert _ids(client, app, 'BAY-01') == [alert.notificationid] + + +def test_scoping_does_not_leak_into_acknowledgement(client, db, app): + """A targeted alert is acknowledged the same way any other one is.""" + asset = _computer(db, 'BAY-01') + alert = _fleet_alert(db) + alert.alertscope = {'hostname': 'BAY-01'} + db.session.commit() + + ack = client.post(ACK_URL.format(alert.notificationid), + json={'hostname': 'BAY-01', 'assetid': asset.assetid}, + headers=_collector_headers(app)) + + assert ack.status_code == 201 + assert _ids(client, app, 'BAY-01') == []