Repository navigation
feat(alerts): realtime alert notifications over SMTP + in-app (#25) - #144
Conversation
Adds an SMTP email channel and in-app notifications for detections, wired through the existing rule engine, rule canvas, settings, and SDK — no new tables (reuses integration_settings + notifications). - notification_service: EmailService (config from integration_settings with SMTP_* env fallback, à la ChatService) + AlertNotifier (email + per-user in-app notification), best-effort so delivery never blocks alerting - rule_engine._fire_alert dispatches notifications when the alert node action is a notify action, threading the node's optional recipients - settings: register 'smtp' integration, mask the password, add a /test probe - rule canvas: 'fire alert + notify' action + optional recipients field - settings UI: Notifications section with an Email (SMTP) integration card - POST /api/v1/alerts to raise alerts directly (owns them under a system 'SDK Alerts' rule) with optional notify - SDK: openuba.send_alert(...) for models - dedicated test subsuite (28 unit tests, no container needed) Closes #25
The workspace image build failed generating pyspark metadata ('setuptools is
not available in the build environment'). Upgrade build tooling right after the
base image so isolated sdist builds resolve setuptools. Unblocks the workspace
CI triggered by the sdk/ change in this branch.
There was a problem hiding this comment.
Pull request overview
This PR adds realtime alert notifications delivered via SMTP email plus in-app notifications, wiring delivery into the existing alert creation path (rule engine + new API endpoint) and reusing existing persisted settings (integration_settings) and in-app notifications rows.
Changes:
- Adds SMTP integration configuration (UI + settings API whitelist +
/testprobe) and a backend notification service to deliver email + in-app notifications. - Extends rule-engine alert firing to optionally dispatch notifications (best-effort) and threads optional per-node recipients from the flow graph.
- Adds an SDK/API surface (
openuba.send_alert+POST /api/v1/alerts) to raise alerts programmatically, with optional notification dispatch.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/src/openuba/client.py | Adds OpenUBAClient.send_alert() that POSTs alert creation to the API, with optional notify/recipients. |
| sdk/src/openuba/init.py | Exposes top-level send_alert() helper and exports it via __all__. |
| interface/src/components/settings/settings-tabs.tsx | Adds SMTP integration card/fields and a “Notifications” integrations section in Settings UI. |
| interface/src/components/rules/flow-nodes.tsx | Adds “fire alert + notify” action and optional recipients input on the alert node. |
| docker/workspace/Dockerfile | Upgrades pip/setuptools/wheel to support sdist-only dependency installs in workspace image. |
| core/tests/test_services/test_rule_engine_notify.py | Adds unit tests for rule-engine → notifier dispatch gating and best-effort behavior. |
| core/tests/test_services/test_notification_service.py | Adds unit tests for SMTP config loading, recipient normalization, SMTP send paths, and notifier behavior. |
| core/tests/test_api_routers/test_settings_smtp.py | Adds unit tests for SMTP integration whitelist/masking and connectivity testing. |
| core/services/rule_engine.py | Hooks notification dispatch into _fire_alert() for notify actions; threads recipients from flow graph. |
| core/services/notification_service.py | Introduces SMTP email sender + in-app notification creation orchestrator (AlertNotifier). |
| core/api_schemas/rules.py | Adds AlertCreate schema for the new alert creation endpoint. |
| core/api_routers/settings.py | Whitelists smtp, adds _test_smtp(), and masks password in integration configs. |
| core/api_routers/rules.py | Adds POST /alerts for SDK-driven alerts, with optional notification dispatch and SDK-owned rule creation. |
Suppressed comments (3)
core/services/notification_service.py:72
_flag()usesbool(config[cfg_key]), but if the DB stores booleans as strings (e.g. "false"),bool("false")is truthy and will incorrectly enable TLS/SSL. Parsing string values defensively avoids surprising behavior and makes DB/env handling consistent.
def _flag(cfg_key: str, env_key: str, default: bool) -> bool:
if cfg_key in config:
return bool(config[cfg_key])
env = os.environ.get(env_key)
if env is None:
core/services/notification_service.py:80
int(config.get("port") or os.environ.get("SMTP_PORT", 587))can raiseValueErrorfor non-numeric inputs (e.g. mis-set env var), which breaks the contract thatsend()/config load never raises. Parse the port defensively and default when invalid.
return {
"host": host,
"port": int(config.get("port") or os.environ.get("SMTP_PORT", 587)),
"username": config.get("username") or os.environ.get("SMTP_USERNAME", ""),
"password": config.get("password") or os.environ.get("SMTP_PASSWORD", ""),
core/api_routers/settings.py:324
- Masking
password(andapi_key) in_mask_sensitive_fields()is good for display, but the Settings UI initializesformDatafromexisting.configand PUTs it back verbatim. That means saving without re-entering the secret can overwrite the stored secret with the masked value (e.g.abcd...wxyz). This existed forapi_keyalready and now also applies to SMTPpassword.
Consider a coordinated fix: (1) UI: clear password fields on panel open and only include them in the PUT payload when the user entered a new value, and/or (2) API: preserve existing secret on update when the incoming value matches the masking pattern or is empty.
def _mask_sensitive_fields(config: dict) -> dict:
'''mask api keys / passwords in config for safe display'''
masked = dict(config)
for key in ("api_key", "password"):
if key in masked and masked[key]:
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if row and row[1]: # enabled | ||
| config = dict(row[0]) if row[0] else {} |
| port = int(config.get("port") or 587) | ||
| use_ssl = bool(config.get("use_ssl", False)) | ||
| use_tls = bool(config.get("use_tls", True)) | ||
| username = config.get("username", "") | ||
| password = config.get("password", "") |
) raising an alert is a detection-output action, not rule-definition management. change create_alert's guard from require_permission("rules", "write") to require_permission("anomalies","write") so emitting alerts (e.g. from a model via the SDK) doesn't require rule-editing privileges. add the missing POST /alerts coverage (audit found ZERO): admin happy path (201, get-or-create SDK rule), unknown rule_id -> 404, and unauth -> 401. also add the _as_admin autouse override + unauthenticated_client fixtures, which fixes the 3 pre-existing auth-less failures in this file (test_create_rule/list_rules/get_rule were red because they never authenticated) — net no regression. local draft, not pushed.
) adversarial-audit follow-ups: - the previous anomalies:write guard was an access no-op (only analyst has write on rules OR anomalies by default, same set) AND inconsistent with GET /alerts which uses the 'alerts' page. switch create_alert to require_permission("alerts","write") — consistent with list_alerts, and 'alerts' write is admin-only by default (appropriate for direct alert injection; grantable per-role via the permissions UI). - the headline permission was previously UNTESTED (every test authenticates as admin, which short-circuits require_permission). add discriminating tests via a real non-admin role + seeded role_permissions reached through get_db_context: an analyst WITH anomalies:write but WITHOUT alerts:write is denied 403 (proves it's keyed on alerts, not anomalies), and an analyst granted alerts:write gets 201. - update SDK send_alert docstring: rules:write -> alerts:write. test_rules.py: 8 passed. local draft, not pushed.
address the two deferred audit items: - _get_or_create_sdk_rule race: give the synthetic rule a deterministic primary key and INSERT inside a SAVEPOINT. concurrent first-time callers now collide on the (already-unique) PK — one wins, the other catches IntegrityError and re-reads the winner — so no duplicate 'SDK Alerts' rows accumulate. no schema migration / no risky global-unique-on-name needed. - unbounded in-app notification fan-out: cap the per-alert fan-out (OPENUBA_MAX_INAPP_NOTIFICATIONS, default 500) so one alert can't write unboundedly many rows synchronously in the request path; report in_app_truncated and log when capped. tests: +sdk-rule idempotency (deterministic id, exactly one row) and +fan-out cap (truncates at the limit). 33 tests pass. local draft, not pushed.
verification found the race-recovery branch untested. re-read the winner by the fixed id (the conflict is on the PK, not the name) and add a test that genuinely drives the IntegrityError path: pre-occupy the fixed id under a different name so the create flush conflicts inside the SAVEPOINT and the except branch re-reads the occupant by id (no raise, no duplicate). test_rules.py: 10 passed. local draft.
What
Delivers realtime alerts (requested in #25) as email (SMTP) + in-app notifications, wired end-to-end through the pieces we already have — no new tables.
How it reuses existing infrastructure
integration_settingstable (integration_type="smtp"), loaded with an env-var fallback exactly likeChatServicedoes for LLM providers.smtpis added to the settings whitelist, the password is masked on read, and there's a/testconnectivity probe.RuleEngine._fire_alert— the single choke point where every rule-fired alert is created. When the alert node's action is a notify action, it dispatches email + in-app. Best-effort: a mail outage can never block alert creation.notificationstable (typealert), so they surface in the notification center already wired into the frontend +notificationsrouter.flow_graph, read by the engine). No new node type.openuba.send_alert(...)lets Python models raise alerts (and optionally notify). Backed by a newPOST /api/v1/alertsthat owns SDK alerts under a systemSDK Alertsrule (alerts require arule_id).Tests (own subsuite, CI-wired)
core/tests/test_services/test_notification_service.py,test_rule_engine_notify.py, andcore/tests/test_api_routers/test_settings_smtp.py— 28 unit tests, all mocked (no SMTP server or DB container): recipient normalization, config load (DB + env), STARTTLS/SSL send paths, error-swallowing, notify-action gating, rule-engine dispatch wiring, and settings whitelist/masking/_test_smtp.Local: backend suite green,
tsc --noEmitclean,next lintclean,next buildgreen.Closes #25