Fix fleet alert server surface inherited from wj

The wj merge brought in a half-built fleet alert feature: server-side UI
behaviour still being stood up over a delivery proof of concept. It left
five red tests. This clears them and fixes a real hole underneath.

The cross-machine guard never ran. acknowledge_fleet_alert read .assetid
off _resolve_asset_for_hostname, which returns the id itself, so the
check raised AttributeError on every call and answered 500. That guard is
what stops one endpoint acknowledging another's alert, and the collector
key is shared across the fleet, so until now any bay could clear an alert
on another machine's behalf. Compare the id directly.

Gate both alert routes the way the rest of the agent channel is gated.
They are collector-key authenticated, not JWT+role, because a shop floor
bay has no user session - the same shape as claim_next_command and
upload_operator_report. Declared in the authz allowlists with the reason,
rather than inventing a permission no agent could hold.

Settle the delivery contract ahead of the durable agent work. The poll
response is authoritative for a host: every alert that should be live on
it right now. An alert absent from the feed is the agent's instruction to
retire the pending toast, which is how a server-side close or cancel
reaches a machine already displaying one. Acknowledgement is terminal per
product decision, so a close after an ack is a no-op and the agent can
retry a queued ack safely. A toast that merely timed out is not an
acknowledgement.

Stop reaching past the plugin contract surface for the mailer. One import
was a plain redirect; the other pulled a private helper to pre-check
recipients, duplicating what send_alert already does. It also returned
early when no email recipients existed, which silently suppressed the
independent webhook leg - misconfigured SMTP was dropping Teams alerts.

Register NotificationAcknowledgement in get_models() and give the
notifications plugin ownership of notificationacknowledgements, then move
the expected head to notifications0006fleetalerts.

Add the coverage whose absence let the .assetid bug ship. Thirteen tests
over gating, authoritative-feed withdrawal, terminal and idempotent
acknowledgement, per-asset scoping, and the cross-machine guard. Five of
them fail against the pre-fix handler.

Remaining suite failures are pre-existing and local: the symlink scan
needs elevation, and the docs check wants files absent from this checkout.
This commit is contained in:
benjamin.baker
2026-09-09 13:20:54 -04:00
parent c635457e7f
commit e0a7a51d46
7 changed files with 372 additions and 24 deletions

View File

@@ -32,7 +32,7 @@ from ..models import (
)
from ..commands import COMMAND_ALLOWLIST, is_allowed_action, ALLOWED_ACTIONS_FOR, row_actions_for, requires_target, action_targetkind, elevated_permission_for, PLACE_POLICY_HOLD, ACTIVE_POLICY_HOLDS, HOLDABLE_ACTIONS_FOR, RELEASE_POLICY_HOLD, ACTIVE_LOCK_FOR, PLACE_ENDPOINT_LOCK, RELEASE_ENDPOINT_LOCK
from ..eventcatalog import CRITICAL_EVENT_IDS, event_severity, event_title
from shopdb.utils.mailer import send_alert
from shopdb.api import send_alert
from markupsafe import escape
endpoints_bp = Blueprint('endpoints', __name__)
@@ -2948,21 +2948,16 @@ def _purge_expired_operator_reports():
def _send_operator_report_alert(report, asset_id, hostname):
"""Send email alert for new operator issue report to default alert recipients."""
from flask import url_for, request
from shopdb.utils.mailer import get_smtp_config, _normalize_recipients
current_app.logger.info('_send_operator_report_alert called for report %d, asset %d, hostname %s',
report.operatorreportid, asset_id, hostname)
# Check if we have recipients before doing expensive work
config = get_smtp_config()
current_app.logger.info('SMTP config: enabled=%s, host=%s, alert_recipients=%s',
config.get('enabled'), config.get('host'), config.get('alert_recipients'))
recipients = _normalize_recipients(config['alert_recipients'])
if not recipients:
current_app.logger.warning('Skipping operator report alert: no alert recipients configured')
return
current_app.logger.info('Will send alert to %d recipient(s): %s', len(recipients), recipients)
# No recipient pre-check here. send_alert already resolves and normalises
# alert_recipients and logs when none are configured, so duplicating that
# reached past the plugin contract surface for a private helper to answer a
# question the call itself answers. It also silently skipped the webhook leg:
# send_alert fans out to the alert webhook independently of SMTP, so bailing
# on "no email recipients" dropped Teams notifications that would have sent.
asset = Asset.query.get(asset_id)
if not asset:
current_app.logger.warning('Cannot send operator report alert: asset %d not found', asset_id)
@@ -3187,6 +3182,16 @@ def get_fleet_alerts():
Called by the endpoint agent to fetch active, unacknowledged fleet alerts
for its hostname. Returns alerts with notification type info (typename and
typecolor from database) formatted for display in the agent tray.
The list is AUTHORITATIVE for this host: it is every alert that should be
live on the endpoint right now, so the agent retires any pending alert it
holds that is absent here. That is how a server-side close or cancel reaches
a machine already showing the alert - an alert deactivated or expired server
side drops out of this list and the agent takes the toast down, rather than
the operator being stuck acknowledging something already withdrawn.
Acknowledgement is terminal, so an acknowledged alert is filtered out here
permanently and a later server-side close is a no-op for that asset.
"""
error = _collector_auth_or_error()
if error is not None:
@@ -3245,9 +3250,19 @@ def get_fleet_alerts():
def acknowledge_fleet_alert(notification_id: int):
"""Agent acknowledges a fleet alert (collector-token auth).
The agent posts the hostname plus assetid when dismissing a fleet alert from
the tray. Records acknowledgement so this alert will not reappear in future polls.
Idempotent - repeated acknowledgements succeed.
The agent posts the hostname plus assetid when the operator dismisses a
fleet alert from the tray. Records acknowledgement so this alert will not
reappear in future polls. Idempotent - repeated acknowledgements succeed.
Acknowledgement is TERMINAL for this asset. Once recorded, the alert is
filtered out of get_fleet_alerts for this machine permanently, and a later
server-side close or cancel of the same notification is a no-op here: there
is nothing left to withdraw. The agent therefore retries a queued
acknowledgement until it lands without risk of a conflicting later state.
Only a real operator action reaches this route. A toast that merely timed
out on screen is NOT an acknowledgement and the agent does not post it - the
alert stays pending so it can be surfaced again.
"""
error = _collector_auth_or_error()
if error is not None:
@@ -3285,9 +3300,10 @@ def acknowledge_fleet_alert(notification_id: int):
f'Asset {assetid} not found',
http_code=404)
# Validate hostname matches assetid to prevent cross-machine acknowledgements
resolved_asset = _resolve_asset_for_hostname(hostname)
if resolved_asset is None or resolved_asset.assetid != assetid:
# Validate hostname matches assetid to prevent cross-machine acknowledgements.
# _resolve_asset_for_hostname returns the asset id itself, not an Asset row.
resolved_assetid = _resolve_asset_for_hostname(hostname)
if resolved_assetid is None or resolved_assetid != assetid:
return error_response(ErrorCodes.VALIDATION_ERROR,
'hostname does not match provided assetid')