Merge pull request #1783 from netalertx/next_release

BE: SQL refactor presence
This commit is contained in:
Jokob @NetAlertX authored and GitHub committed 2026-09-12 08:03:45 +10:00
commit f3620ab63f
15 files changed
+607 -43

No files matched your search

+4 -3
View File
@@ -47,13 +47,14 @@ This skill covers what happens *after* a plugin's rows land in `CurrentScan` —
This is the scan-pipeline-local half of a bigger attribution system — see the `database-patterns` skill for `FIELD_SOURCE_MAP` / `server/db/authoritative_handler.py`, the full `*Source` attribution model, and how SQLite triggers consume it for audit logging. Read both if touching anything that writes a `*Source` column.
## Two real gotchas (not hypothetical — both surfaced live during a design review)
## Four real gotchas (not hypothetical — all surfaced live during a design review)
1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched.
1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. **Update:** `current_scan_presence_condition()` (`server/scan/presence.py`) now centralizes this for five of those sites — `update_presence_from_CurrentScan()` (both statements), `update_devLastConnection_from_CurrentScan()`, and three of `insert_events()`'s four queries (both `Device Down` variants, `Disconnected`) all call it instead of writing their own `EXISTS (...)`. The remaining two ("New Connections", the raw `Sessions` insert in `create_new_devices()`) still can't use it — they need the actual `scanLastIP`/`scanVendor` *value* off the presence-asserting row via `MIN()`/`GROUP BY`, not just a boolean — so a brand-new presence-adjacent query still has to be checked against both patterns, not assumed to be a bare helper call.
2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward.
3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in `server/db/schema/app.sql`) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle.
4. **`server/plugins/sync/sync.py` bypasses this entire pipeline on purpose, twice — a permanent exception, not a bug.** It fires its own direct `INSERT OR IGNORE INTO Events (... 'New Device' ...)` for newly-seen synced devices (hardcoded `evePendingAlertEmail = 1`, no `scanNotificationMode`/quiet awareness), and in `carbon-copy` mode its own raw `Devices` UPSERT via `ON CONFLICT(devMac) DO UPDATE` — both deliberately skipping `create_new_devices()`/`update_devices_data_from_scan()`/`can_overwrite_field()` (`sync.py`'s own comments document this as intentional: "Node is fully authoritative in this mode"). It *is* a normal `mapped_to_table: CurrentScan` plugin for its presence contribution, so `IMPORT_ON`/`scanPresence` apply to it exactly like any other plugin — but its two direct-write paths would silently ignore `scanNotificationMode = 'quiet'` or `scanCreatesDevice = 0` if `sync` ever adopted either. Keep this in mind whenever touching the generic pipeline and assuming every `Events`/`Devices` write went through it — `sync.py` is the one place that doesn't.
**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan` specifically is safe from drift because `ensure_CurrentScan()` unconditionally drops and recreates it on every startup, superseding whatever `app.sql` bootstrapped — but that safety net is unique to the four tables with an `ensure_X()`-style function (`CurrentScan`, `Parameters`, `Settings`, `Plugins_Language_Strings`). `Events`, `Sessions`, `AppEvents`, and `Notifications` have no such function and no `ensure_column()` backfill calls in `server/database.py` either (unlike `Devices`, which has ~30 of them) — for those tables, whatever `app.sql` says *is* the schema, permanently, for every fresh install. Drift there would be a real, live bug, not documentation lag — see `.gemini/internal-docs/PRDs/scan-pipeline-hardening.md` for the follow-up this motivated. Any *new* query added here should be checked the same way (`EXPLAIN QUERY PLAN` at a realistic row count) rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale.
**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan`, `Parameters`, and `Settings` are safe from drift because each has a dedicated `ensure_X()` function (`server/db/db_upgrade.py`) that unconditionally drops and recreates the table on every startup, superseding whatever `app.sql` bootstrapped. `Plugins_Language_Strings` gets the same unconditional drop/recreate, but inside the shared `ensure_plugins_tables()`, not a dedicated function of its own. `AppEvents` gets an equivalent drop/recreate too, via a different mechanism — `AppEvent_obj.__init__()` (`server/workflows/app_events.py`) drops and recreates it on every startup, independent of `db_upgrade.py`. `Devices` has no drop/recreate, but `server/database.py` has 18 explicit `ensure_column()` calls that backfill any column missing from an older `app.sql` snapshot on every startup. **`Events`, `Sessions`, and `Notifications`** — the three tables that genuinely had neither safety net — **now have the same backfill treatment**, per `scan-pipeline-hardening.md` Design §3 (implemented): `ensure_table_columns()` (`server/db/db_upgrade.py`), driven by one Python column-list constant per table (`server/db/schema_columns.py`) that's also diffed against `app.sql` in CI (`test/db/test_schema_drift_guard.py`), so drift between the two is caught rather than silently shipping. `AppEvents`/`Notifications` each also have a *second* schema-definition surface beyond `app.sql` worth knowing about — their own inline `CREATE TABLE IF NOT EXISTS` in `server/workflows/app_events.py`/`server/models/notification_instance.py` respectively — kept in sync via the same drift-check test. Any *new* query added here should still be checked with `EXPLAIN QUERY PLAN` at a realistic row count rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale.
## When to read this vs. other docs/skills
+4 -3
View File
@@ -47,13 +47,14 @@ This skill covers what happens *after* a plugin's rows land in `CurrentScan` —
This is the scan-pipeline-local half of a bigger attribution system — see the `database-patterns` skill for `FIELD_SOURCE_MAP` / `server/db/authoritative_handler.py`, the full `*Source` attribution model, and how SQLite triggers consume it for audit logging. Read both if touching anything that writes a `*Source` column.
## Two real gotchas (not hypothetical — both surfaced live during a design review)
## Four real gotchas (not hypothetical — all surfaced live during a design review)
1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched.
1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. **Update:** `current_scan_presence_condition()` (`server/scan/presence.py`) now centralizes this for five of those sites — `update_presence_from_CurrentScan()` (both statements), `update_devLastConnection_from_CurrentScan()`, and three of `insert_events()`'s four queries (both `Device Down` variants, `Disconnected`) all call it instead of writing their own `EXISTS (...)`. The remaining two ("New Connections", the raw `Sessions` insert in `create_new_devices()`) still can't use it — they need the actual `scanLastIP`/`scanVendor` *value* off the presence-asserting row via `MIN()`/`GROUP BY`, not just a boolean — so a brand-new presence-adjacent query still has to be checked against both patterns, not assumed to be a bare helper call.
2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward.
3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in `server/db/schema/app.sql`) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle.
4. **`server/plugins/sync/sync.py` bypasses this entire pipeline on purpose, twice — a permanent exception, not a bug.** It fires its own direct `INSERT OR IGNORE INTO Events (... 'New Device' ...)` for newly-seen synced devices (hardcoded `evePendingAlertEmail = 1`, no `scanNotificationMode`/quiet awareness), and in `carbon-copy` mode its own raw `Devices` UPSERT via `ON CONFLICT(devMac) DO UPDATE` — both deliberately skipping `create_new_devices()`/`update_devices_data_from_scan()`/`can_overwrite_field()` (`sync.py`'s own comments document this as intentional: "Node is fully authoritative in this mode"). It *is* a normal `mapped_to_table: CurrentScan` plugin for its presence contribution, so `IMPORT_ON`/`scanPresence` apply to it exactly like any other plugin — but its two direct-write paths would silently ignore `scanNotificationMode = 'quiet'` or `scanCreatesDevice = 0` if `sync` ever adopted either. Keep this in mind whenever touching the generic pipeline and assuming every `Events`/`Devices` write went through it — `sync.py` is the one place that doesn't.
**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan` specifically is safe from drift because `ensure_CurrentScan()` unconditionally drops and recreates it on every startup, superseding whatever `app.sql` bootstrapped — but that safety net is unique to the four tables with an `ensure_X()`-style function (`CurrentScan`, `Parameters`, `Settings`, `Plugins_Language_Strings`). `Events`, `Sessions`, `AppEvents`, and `Notifications` have no such function and no `ensure_column()` backfill calls in `server/database.py` either (unlike `Devices`, which has ~30 of them) — for those tables, whatever `app.sql` says *is* the schema, permanently, for every fresh install. Drift there would be a real, live bug, not documentation lag — see `.gemini/internal-docs/PRDs/scan-pipeline-hardening.md` for the follow-up this motivated. Any *new* query added here should be checked the same way (`EXPLAIN QUERY PLAN` at a realistic row count) rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale.
**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan`, `Parameters`, and `Settings` are safe from drift because each has a dedicated `ensure_X()` function (`server/db/db_upgrade.py`) that unconditionally drops and recreates the table on every startup, superseding whatever `app.sql` bootstrapped. `Plugins_Language_Strings` gets the same unconditional drop/recreate, but inside the shared `ensure_plugins_tables()`, not a dedicated function of its own. `AppEvents` gets an equivalent drop/recreate too, via a different mechanism — `AppEvent_obj.__init__()` (`server/workflows/app_events.py`) drops and recreates it on every startup, independent of `db_upgrade.py`. `Devices` has no drop/recreate, but `server/database.py` has 18 explicit `ensure_column()` calls that backfill any column missing from an older `app.sql` snapshot on every startup. **`Events`, `Sessions`, and `Notifications`** — the three tables that genuinely had neither safety net — **now have the same backfill treatment**, per `scan-pipeline-hardening.md` Design §3 (implemented): `ensure_table_columns()` (`server/db/db_upgrade.py`), driven by one Python column-list constant per table (`server/db/schema_columns.py`) that's also diffed against `app.sql` in CI (`test/db/test_schema_drift_guard.py`), so drift between the two is caught rather than silently shipping. `AppEvents`/`Notifications` each also have a *second* schema-definition surface beyond `app.sql` worth knowing about — their own inline `CREATE TABLE IF NOT EXISTS` in `server/workflows/app_events.py`/`server/models/notification_instance.py` respectively — kept in sync via the same drift-check test. Any *new* query added here should still be checked with `EXPLAIN QUERY PLAN` at a realistic row count rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale.
## When to read this vs. other docs/skills
+4 -3
View File
@@ -47,13 +47,14 @@ This skill covers what happens *after* a plugin's rows land in `CurrentScan` —
This is the scan-pipeline-local half of a bigger attribution system — see the `database-patterns` skill for `FIELD_SOURCE_MAP` / `server/db/authoritative_handler.py`, the full `*Source` attribution model, and how SQLite triggers consume it for audit logging. Read both if touching anything that writes a `*Source` column.
## Two real gotchas (not hypothetical — both surfaced live during a design review)
## Four real gotchas (not hypothetical — all surfaced live during a design review)
1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched.
1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. **Update:** `current_scan_presence_condition()` (`server/scan/presence.py`) now centralizes this for five of those sites — `update_presence_from_CurrentScan()` (both statements), `update_devLastConnection_from_CurrentScan()`, and three of `insert_events()`'s four queries (both `Device Down` variants, `Disconnected`) all call it instead of writing their own `EXISTS (...)`. The remaining two ("New Connections", the raw `Sessions` insert in `create_new_devices()`) still can't use it — they need the actual `scanLastIP`/`scanVendor` *value* off the presence-asserting row via `MIN()`/`GROUP BY`, not just a boolean — so a brand-new presence-adjacent query still has to be checked against both patterns, not assumed to be a bare helper call.
2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward.
3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in `server/db/schema/app.sql`) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle.
4. **`server/plugins/sync/sync.py` bypasses this entire pipeline on purpose, twice — a permanent exception, not a bug.** It fires its own direct `INSERT OR IGNORE INTO Events (... 'New Device' ...)` for newly-seen synced devices (hardcoded `evePendingAlertEmail = 1`, no `scanNotificationMode`/quiet awareness), and in `carbon-copy` mode its own raw `Devices` UPSERT via `ON CONFLICT(devMac) DO UPDATE` — both deliberately skipping `create_new_devices()`/`update_devices_data_from_scan()`/`can_overwrite_field()` (`sync.py`'s own comments document this as intentional: "Node is fully authoritative in this mode"). It *is* a normal `mapped_to_table: CurrentScan` plugin for its presence contribution, so `IMPORT_ON`/`scanPresence` apply to it exactly like any other plugin — but its two direct-write paths would silently ignore `scanNotificationMode = 'quiet'` or `scanCreatesDevice = 0` if `sync` ever adopted either. Keep this in mind whenever touching the generic pipeline and assuming every `Events`/`Devices` write went through it — `sync.py` is the one place that doesn't.
**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan` specifically is safe from drift because `ensure_CurrentScan()` unconditionally drops and recreates it on every startup, superseding whatever `app.sql` bootstrapped — but that safety net is unique to the four tables with an `ensure_X()`-style function (`CurrentScan`, `Parameters`, `Settings`, `Plugins_Language_Strings`). `Events`, `Sessions`, `AppEvents`, and `Notifications` have no such function and no `ensure_column()` backfill calls in `server/database.py` either (unlike `Devices`, which has ~30 of them) — for those tables, whatever `app.sql` says *is* the schema, permanently, for every fresh install. Drift there would be a real, live bug, not documentation lag — see `.gemini/internal-docs/PRDs/scan-pipeline-hardening.md` for the follow-up this motivated. Any *new* query added here should be checked the same way (`EXPLAIN QUERY PLAN` at a realistic row count) rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale.
**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan`, `Parameters`, and `Settings` are safe from drift because each has a dedicated `ensure_X()` function (`server/db/db_upgrade.py`) that unconditionally drops and recreates the table on every startup, superseding whatever `app.sql` bootstrapped. `Plugins_Language_Strings` gets the same unconditional drop/recreate, but inside the shared `ensure_plugins_tables()`, not a dedicated function of its own. `AppEvents` gets an equivalent drop/recreate too, via a different mechanism — `AppEvent_obj.__init__()` (`server/workflows/app_events.py`) drops and recreates it on every startup, independent of `db_upgrade.py`. `Devices` has no drop/recreate, but `server/database.py` has 18 explicit `ensure_column()` calls that backfill any column missing from an older `app.sql` snapshot on every startup. **`Events`, `Sessions`, and `Notifications`** — the three tables that genuinely had neither safety net — **now have the same backfill treatment**, per `scan-pipeline-hardening.md` Design §3 (implemented): `ensure_table_columns()` (`server/db/db_upgrade.py`), driven by one Python column-list constant per table (`server/db/schema_columns.py`) that's also diffed against `app.sql` in CI (`test/db/test_schema_drift_guard.py`), so drift between the two is caught rather than silently shipping. `AppEvents`/`Notifications` each also have a *second* schema-definition surface beyond `app.sql` worth knowing about — their own inline `CREATE TABLE IF NOT EXISTS` in `server/workflows/app_events.py`/`server/models/notification_instance.py` respectively — kept in sync via the same drift-check test. Any *new* query added here should still be checked with `EXPLAIN QUERY PLAN` at a realistic row count rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale.
## When to read this vs. other docs/skills
+4
View File
@@ -40,6 +40,10 @@ On almost all plugins there are 2 core settings, `<plugin>_WATCH` and `<plugin>_
Click the **Read more in the docs.** Link at the top of each plugin to get more details on how the given plugin works.
### Plugin-level per-row overrides
A plugin author can also mark individual rows it reports as `quiet` via the `scanNotificationMode` data column, independent of any user-facing setting above - e.g. a bulk inventory import that shouldn't spam notifications for known-offline devices. This is a plugin-authoring concept, not something configured in the UI - see [Data contract](https://docs.netalertx.com/PLUGINS_DEV_DATA_CONTRACT#import-behavior-columns) for the full behavior (when it applies, and how it combines with the **Alert Events**/**Alert Down** device settings above when multiple plugins report the same device).
## Global settings ⚙
![Global notification settings](./img/NOTIFICATIONS/Global-notification-settings.png)
+37 -1
View File
@@ -164,7 +164,7 @@ Three optional `CurrentScan` columns, all independent of each other, control wha
| Column | Type | Default | Meaning |
|---|---|---|---|
| `scanCreatesDevice` | boolean | `1` | Whether this row can originate a *new* `Devices` entry. `0` lets an enrich-only plugin (e.g. a hostname resolver) update an already-existing device's fields without ever being able to create one. |
| `scanNotificationMode` | text (`normal` \| `quiet`) | `normal` | Whether creating/reconnecting this device should dispatch a notification. `quiet` still writes the `Events` row (audit trail intact) but suppresses the outbound email/push. Creation-time-only: it seeds `devAlertDown`/`devAlertEvents` to `0` on the device when it's first created, rather than being an ongoing, per-cycle re-evaluated policy — reclassifying a plugin's row later does not retroactively change an already-created device's alert settings. |
| `scanNotificationMode` | text (`normal` \| `quiet`) | `normal` | Whether this row's notifications are suppressed. `quiet` always suppresses the outbound email/push; whether the `Events` row itself still gets written depends on the event. **Live** (per-cycle aggregate, reclassifying a row changes future events): `New Device`, `Connected`, `Down Reconnected`, `IP Changed` — audit trail always written. `New Device` isn't gated on `scanPresence = 1` like the other three (see flowcharts below). **Frozen** (`devAlertDown`/`devAlertEvents` seeded at device creation, reclassifying later has no retroactive effect): `Device Down`, `Disconnected` — not symmetric. `Disconnected` always writes its `Events` row (`evePendingAlertEmail = 0` when quiet). `Device Down` writes **no row at all** when `devAlertDown = 0`. |
| `scanPresence` | boolean | `1` | Whether this row asserts the device is *currently online*. `0` means "identity/inventory data, no presence claim" — not "offline". A reservation, a lease record, or a static IPAM entry are typical `0` cases. |
**Missing vs. invalid values — these behave differently, not interchangeably:**
@@ -188,6 +188,42 @@ Three optional `CurrentScan` columns, all independent of each other, control wha
`scanNotificationMode` is orthogonal to both of the above and can be combined with any row in the table (e.g. inventory import + quiet, for a fully silent bulk import of known-offline devices).
**Decision: does this row create a device?**
```mermaid
flowchart TD
A[Row reaches CurrentScan] --> B{scanMac blank or<br/>null-equivalent?}
B -- yes --> Z[Never creates a device]
B -- no --> C{Any row this cycle for this<br/>MAC has scanCreatesDevice = 1?}
C -- no, all say 0 --> Y[No device created<br/>enrich-only]
C -- yes, at least one --> D{Devices row already<br/>exists for this MAC?}
D -- yes --> E[No-op - existing device untouched<br/>by this check]
D -- no --> F[New Devices row created<br/>+ New Device event]
```
**Decision: is this event's notification suppressed?**
```mermaid
flowchart TD
A[Event about to fire] --> B{Fired from a row that exists in<br/>CurrentScan this cycle? New Device /<br/>Connected / Down Reconnected / IP Changed}
B -- yes --> C{Live aggregate: any CurrentScan row<br/>for this MAC says<br/>scanNotificationMode = quiet?}
C -- yes --> S[Suppressed<br/>evePendingAlertEmail = 0]
C -- no --> N[Notified<br/>evePendingAlertEmail = 1]
B -- no, fired from row ABSENCE<br/>Device Down / Disconnected --> D{Frozen device setting:<br/>devAlertDown / devAlertEvents,<br/>seeded at creation time}
D -- off --> S
D -- on --> N
```
**Worked scenarios:**
| Scenario | `scanCreatesDevice` | `scanPresence` | `scanNotificationMode` | `scanMac` | Outcome |
|---|---|---|---|---|---|
| Normal discovery (default plugin behavior) | `1` (default) | `1` (default) | `normal` (default) | real MAC | Device created if new, notified normally, presence tracked live. |
| Enrich-only plugin (e.g. a hostname resolver) | `0` | `1` (default) | `normal` (default) | real MAC | Never originates a device; still updates an existing device's fields via `FIELD_SPECS`. If another plugin reports the same MAC with `scanCreatesDevice = 1`, the device still gets created (most-permissive-wins) — this plugin's `0` doesn't block it. |
| Bulk inventory import of known-offline devices | `1` | `0` | `quiet` | real MAC | Creates devices without claiming they're online, and without a wave of "New Device" notifications for a large batch import. |
| Presence-confirming enrichment (e.g. a DHCP lease scanner) | `0` | `1` | `normal` | real MAC | Confirms an *existing* device is online without ever being the plugin that creates it. |
| Row with no usable device identity (e.g. an object with no routable MAC available) | `0` | irrelevant | irrelevant | blank / null-equivalent | Never creates a device — but not for symmetric reasons. The blank-MAC guard blocks the whole aggregated group by its shared `scanMac` value, regardless of any individual row's `scanCreatesDevice` (even a stray `1` from an unrelated plugin sharing the same blank `scanMac` can't override it). Setting `scanCreatesDevice = 0` here is still correct practice, but on its own is only this row's vote — most-permissive-wins means a sibling row for the same `scanMac` asserting `1` would still win. The blank-MAC guard is what actually guarantees safety regardless of what other contributors do. |
## Examples
### Valid Data (9 columns, minimal)
+18
View File
@@ -10,6 +10,7 @@ from db.db_helper import get_table_json, json_obj
from workflows.app_events import AppEvent_obj
from db.db_upgrade import (
ensure_column,
ensure_table_columns,
ensure_CurrentScan,
ensure_plugins_tables,
ensure_Parameters,
@@ -202,6 +203,23 @@ class DB:
# before ensure_plugins_tables which uses IF NOT EXISTS with new names)
migrate_to_camelcase(self.sql)
# Backfill Events/Sessions/Notifications columns that may be
# missing from an older app.sql snapshot - same drift-repair
# pattern as the Devices columns above, generalized instead of
# duplicated (see scan-pipeline-hardening.md Design §3). Must run
# after the camelCase migration above, or a pre-migration table's
# old-style column names would make every new-style column look
# "missing" and get added alongside the stale ones.
#
# AppEvents is deliberately NOT in this list: AppEvent_obj(self)
# below unconditionally drops and recreates it on every startup
# (its own equivalent of ensure_CurrentScan()'s drop/recreate
# pattern), so backfilling it here would be wasted work on a
# table about to be discarded a few lines later.
for _drift_table in ("Events", "Sessions", "Notifications"):
if not ensure_table_columns(self.sql, _drift_table):
raise RuntimeError(f"ensure_table_columns({_drift_table}) failed")
# Settings table setup
ensure_Settings(self.sql)
+33 -1
View File
@@ -3,6 +3,7 @@ from zoneinfo import ZoneInfo
import datetime as dt
from logger import mylog # noqa: E402 [flake8 lint suppression]
from messaging.in_app import write_notification # noqa: E402 [flake8 lint suppression]
from db.schema_columns import TABLE_COLUMNS # noqa: E402 [flake8 lint suppression]
# Define the expected Devices table columns (hardcoded base schema) [v26.1/2.XX]
@@ -81,7 +82,7 @@ def ensure_column(sql, table: str, column_name: str, column_type: str) -> bool:
return True # Already exists
# Validate that this column is in the expected schema
expected = EXPECTED_DEVICES_COLUMNS if table == "Devices" else []
expected = EXPECTED_DEVICES_COLUMNS if table == "Devices" else TABLE_COLUMNS.get(table, {})
if not expected or column_name not in expected:
msg = (
f"[db_upgrade] ⚠ ERROR: Column '{column_name}' is not in expected schema - "
@@ -102,6 +103,37 @@ def ensure_column(sql, table: str, column_name: str, column_type: str) -> bool:
return False
def ensure_table_columns(sql, table: str) -> bool:
"""
Backfill every column in TABLE_COLUMNS[table] (server/db/schema_columns.py)
that's missing from the live table, via ensure_column() - the same
drift-repair pattern Devices already has, generalized instead of
hand-writing one ensure_column() call per column per table (the mirrored
duplication that pattern would otherwise reintroduce - see
scan-pipeline-hardening.md Design §3).
Skips silently (returns True) if the table doesn't exist yet - these
four tables are normally created by app.sql's first-run bootstrap before
this ever runs, but this must not turn a not-yet-created table into a
hard failure that rolls back the whole initDB() transaction.
"""
columns = TABLE_COLUMNS.get(table)
if not columns:
mylog("none", [f"[db_upgrade] ensure_table_columns: no column list registered for '{table}'"])
return False
sql.execute("SELECT name FROM sqlite_master WHERE type='table' AND name=?", (table,))
if not sql.fetchone():
mylog("debug", [f"[db_upgrade] ensure_table_columns: '{table}' does not exist yet, skipping"])
return True
ok = True
for column_name, column_type in columns.items():
if not ensure_column(sql, table, column_name, column_type):
ok = False
return ok
def ensure_mac_lowercase_triggers(sql):
"""
Ensures the triggers for lowercasing MAC addresses exist on the Devices table.
+94
View File
@@ -0,0 +1,94 @@
"""
Single source of truth for the Events/Sessions/AppEvents/Notifications column
lists - used by both the runtime backfill loop (db_upgrade.ensure_table_columns())
and the CI drift-check test (test/db/test_schema_drift_guard.py) against
server/db/schema/app.sql, per scan-pipeline-hardening.md Design §3.
Unlike Devices (server/database.py's ~18 ensure_column() calls, unrelated to
this file), Events/Sessions/Notifications have no drop/recreate safety net
and previously had zero ensure_column() calls at all - app.sql was the only
definition of their schema, with nothing to catch it drifting from what the
rest of the code expects. This is additive/detection-and-backfill only; it
doesn't change any column's meaning or add new columns beyond what app.sql
already defines today.
AppEvents is included here too (for the drift-check test's benefit, since
app.sql also defines it and workflows/app_events.py's inline CREATE TABLE is
a second definition worth keeping in sync), but is NOT part of the runtime
backfill loop in database.py's initDB() - AppEvent_obj.__init__() already
unconditionally drops and recreates the table on every startup, making its
drift harmless the same way ensure_CurrentScan() does for CurrentScan. Found
this while implementing the backfill loop, not before - the correction is
recorded in scan-pipeline-hardening.md.
Column types are copied verbatim from server/db/schema/app.sql's CREATE
TABLE statements for these four tables - keep in sync if that file changes,
the drift-check test will fail loudly if it doesn't.
"""
EVENTS_COLUMNS = {
"eveMac": "STRING (50)",
"eveIp": "STRING (50)",
"eveDateTime": "DATETIME",
"eveEventType": "STRING (30)",
"eveAdditionalInfo": "STRING (250)",
"evePendingAlertEmail": "BOOLEAN",
"evePairEventRowid": "INTEGER",
}
SESSIONS_COLUMNS = {
"sesMac": "STRING (50)",
"sesIp": "STRING (50)",
"sesEventTypeConnection": "STRING (30)",
"sesDateTimeConnection": "DATETIME",
"sesEventTypeDisconnection": "STRING (30)",
"sesDateTimeDisconnection": "DATETIME",
"sesStillConnected": "BOOLEAN",
"sesAdditionalInfo": "STRING (250)",
}
APPEVENTS_COLUMNS = {
"index": "INTEGER",
"guid": "TEXT",
"appEventProcessed": "BOOLEAN",
"dateTimeCreated": "TEXT",
"objectType": "TEXT",
"objectGuid": "TEXT",
"objectPlugin": "TEXT",
"objectPrimaryId": "TEXT",
"objectSecondaryId": "TEXT",
"objectForeignKey": "TEXT",
"objectIndex": "TEXT",
"objectIsNew": "BOOLEAN",
"objectIsArchived": "BOOLEAN",
"objectStatusColumn": "TEXT",
"objectStatus": "TEXT",
"appEventType": "TEXT",
"helper1": "TEXT",
"helper2": "TEXT",
"helper3": "TEXT",
"extra": "TEXT",
}
NOTIFICATIONS_COLUMNS = {
"index": "INTEGER",
"guid": "TEXT",
"dateTimeCreated": "TEXT",
"dateTimePushed": "TEXT",
"status": "TEXT",
"json": "TEXT",
"text": "TEXT",
"html": "TEXT",
"publishedVia": "TEXT",
"extra": "TEXT",
}
# Table name -> {column_name: sql_type}. Drives both the backfill loop and
# the drift-detection test - the one place these four tables' expected
# column lists are written down in Python.
TABLE_COLUMNS = {
"Events": EVENTS_COLUMNS,
"Sessions": SESSIONS_COLUMNS,
"AppEvents": APPEVENTS_COLUMNS,
"Notifications": NOTIFICATIONS_COLUMNS,
}
+5 -1
View File
@@ -307,6 +307,8 @@ def main():
else:
# Fire "New Device" events for genuinely new MACs before the Devices
# INSERT pre-seeds the table (which would block create_new_devices()).
# This bypasses the standard scan pipeline on purpose - see the
# scan-pipeline skill's sync.py bypass gotcha.
if new_devices:
now = timeNowUTC()
cursor.executemany(
@@ -341,7 +343,9 @@ def main():
# that contract by leaving devPresentLastScan to the normal pipeline.
# NOTE: this raw SQL bypasses can_overwrite_field() — ALL other fields
# including USER/LOCKED-sourced ones are overwritten. Node is fully
# authoritative in this mode.
# authoritative in this mode. Also bypasses the standard scan
# pipeline on purpose - see the scan-pipeline skill's sync.py
# bypass gotcha.
_CARBON_COPY_SKIP = {'devMac', 'devPresentLastScan'}
update_cols = [col for col in insert_cols if col not in _CARBON_COPY_SKIP]
update_clause = ', '.join(f'{col}=excluded.{col}' for col in update_cols)
+14 -20
View File
@@ -9,6 +9,7 @@ from const import vendorsPath, vendorsPathNewest, sql_generateGuid, NULL_EQUIVAL
from models.device_instance import DeviceInstance
from scan.name_resolution import NameResolver
from scan.device_heuristics import guess_icon, guess_type
from scan.presence import current_scan_presence_condition
from db.db_helper import sanitize_SQL_input, list_to_where, safe_int
from db.db_upgrade import PARENT_MAC_SENTINELS
from db.authoritative_handler import (
@@ -206,26 +207,18 @@ def update_presence_from_CurrentScan(db):
# (scanPresence = 1). A row can exist purely as identity/inventory data
# (scanPresence = 0, e.g. a DHCP reservation) without claiming the device is
# online right now - "abstain, not override": any other row for the same MAC
# that does assert presence still wins via this same EXISTS check.
sql.execute("""
# that does assert presence still wins via this same predicate.
sql.execute(f"""
UPDATE Devices
SET devPresentLastScan = 1
WHERE EXISTS (
SELECT 1 FROM CurrentScan
WHERE devMac = scanMac
AND scanPresence = 1
)
WHERE {current_scan_presence_condition("devMac")}
""")
# Mark not present if no CurrentScan row for this MAC asserts presence
sql.execute("""
sql.execute(f"""
UPDATE Devices
SET devPresentLastScan = 0
WHERE NOT EXISTS (
SELECT 1 FROM CurrentScan
WHERE devMac = scanMac
AND scanPresence = 1
)
WHERE NOT {current_scan_presence_condition("devMac")}
""")
@@ -245,11 +238,7 @@ def update_devLastConnection_from_CurrentScan(db):
sql.execute(f"""
UPDATE Devices
SET devLastConnection = '{startTime}'
WHERE EXISTS (
SELECT 1 FROM CurrentScan
WHERE devMac = scanMac
AND scanPresence = 1
)
WHERE {current_scan_presence_condition("devMac")}
""")
@@ -682,6 +671,7 @@ def create_new_devices(db):
GROUP BY scanMac
) agg
WHERE agg.scanCreates = 1
AND agg.scanMac NOT IN ({NULL_EQUIVALENTS_SQL})
AND NOT EXISTS (
SELECT 1 FROM Devices
WHERE devMac = agg.scanMac
@@ -802,8 +792,12 @@ def create_new_devices(db):
# if another row for the same MAC says 0 (enrich-only). Rows for
# already-existing devices pass through harmlessly too - the INSERT OR
# IGNORE below is already a no-op for them regardless of this filter.
query = """SELECT scanMac, scanName, scanVendor, scanSourcePlugin, scanLastIP, scanSyncHubNode, scanParentMAC, scanParentPort, scanSite, scanSSID, scanType
FROM CurrentScan WHERE scanCreatesDevice = 1"""
# scanMac NOT IN NULL_EQUIVALENTS blocks creating a device from a blank/
# null-equivalent MAC - a plugin reporting a row it can't originate a
# device from (no real MAC available) should set scanCreatesDevice = 0
# itself, but this is the backstop for one that doesn't.
query = f"""SELECT scanMac, scanName, scanVendor, scanSourcePlugin, scanLastIP, scanSyncHubNode, scanParentMAC, scanParentPort, scanSite, scanSSID, scanType
FROM CurrentScan WHERE scanCreatesDevice = 1 AND scanMac NOT IN ({NULL_EQUIVALENTS_SQL})"""
mylog("debug", f"[New Devices] Collecting New Devices Query: {query}")
current_scan_data = sql.execute(query).fetchall()
+42
View File
@@ -0,0 +1,42 @@
import re
_SQL_IDENTIFIER_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?$")
def current_scan_presence_condition(mac_column: str) -> str:
"""SQL fragment: TRUE if any CurrentScan row for mac_column asserts presence.
The one and only definition of 'is this MAC present this cycle' - every
caller uses this, nobody writes their own CurrentScan presence predicate.
mac_column MUST be a trusted, hardcoded SQL column/table.column reference
written by NetAlertX code (e.g. "devMac", "CurrentScan.scanMac") - this
function does raw string interpolation, not parameterized SQL. Never pass
plugin data, user input, or any runtime string value here.
The inner CurrentScan is aliased as presence_scan so a qualified
mac_column like "CurrentScan.scanMac" resolves to the outer reference,
not this subquery's own row - without the alias the bare table name
shadows it, turning the comparison into a same-row tautology that's
true for any row with a non-NULL scanMac. mac_column may not reference
presence_scan itself for the same reason.
Not usable everywhere a presence check appears: the "New Connections"
query in session_events.py and the raw Sessions insert in
create_new_devices() (device_handling.py) both need the actual
scanLastIP/scanVendor *values* off the presence-asserting row via a
MIN()/GROUP BY aggregate, not just a boolean - see
scan-pipeline-hardening.md Design §1's correction for why those two
(plus "IP Changed", an eighth non-canonical site) keep their own
hand-written aggregation instead of calling this helper.
"""
if not _SQL_IDENTIFIER_RE.match(mac_column):
raise ValueError(f"mac_column must be a plain identifier, got: {mac_column!r}")
if mac_column == "presence_scan" or mac_column.startswith("presence_scan."):
raise ValueError(
f"mac_column must not reference presence_scan - that's this helper's own "
f"internal subquery alias, got: {mac_column!r}"
)
return f"""EXISTS (
SELECT 1 FROM CurrentScan AS presence_scan
WHERE presence_scan.scanMac = {mac_column} AND presence_scan.scanPresence = 1
)"""
+4 -11
View File
@@ -14,6 +14,7 @@ from scan.device_handling import (
update_presence_from_CurrentScan
)
from helper import get_setting_value
from scan.presence import current_scan_presence_condition
from db.db_helper import print_table_schema
from utils.datetime_utils import timeNowUTC
from logger import mylog, Logger
@@ -190,10 +191,7 @@ def insert_events(db):
AND devCanSleep = 0
AND devPresentLastScan = 1
AND {_SQL_NOT_FORCED_ONLINE}
AND NOT EXISTS (SELECT 1 FROM CurrentScan
WHERE devMac = scanMac
AND scanPresence = 1
) """)
AND NOT {current_scan_presence_condition("devMac")} """)
# Check device down sleeping devices whose sleep window has expired
mylog("debug", "[Events] - 1b - Devices down (sleep expired)")
@@ -207,9 +205,7 @@ def insert_events(db):
AND devIsSleeping = 0
AND devPresentLastScan = 0
AND {_SQL_NOT_FORCED_ONLINE}
AND NOT EXISTS (SELECT 1 FROM CurrentScan
WHERE devMac = scanMac
AND scanPresence = 1)
AND NOT {current_scan_presence_condition("devMac")}
AND NOT EXISTS (SELECT 1 FROM Events
WHERE eveMac = devMac
AND eveEventType = 'Device Down'
@@ -274,10 +270,7 @@ def insert_events(db):
WHERE devAlertDown = 0
AND devPresentLastScan = 1
AND {_SQL_NOT_FORCED_ONLINE}
AND NOT EXISTS (SELECT 1 FROM CurrentScan
WHERE devMac = scanMac
AND scanPresence = 1
) """)
AND NOT {current_scan_presence_condition("devMac")} """)
# Check IP Changed
mylog("debug", "[Events] - 4 - IP Changes")
+172
View File
@@ -0,0 +1,172 @@
"""
Tests for the Events/Sessions/AppEvents/Notifications schema drift guard
(server/db/schema_columns.py, server/db/db_upgrade.py's ensure_table_columns())
- see scan-pipeline-hardening.md Design §3.
Three things are tested:
1. No drift today between TABLE_COLUMNS and the real server/db/schema/app.sql.
2. The drift-detection logic can actually detect a real mismatch (not just a
check that always passes trivially) - per the prd-writing skill's rule
that a guard needs its own test, separate from "no drift found today".
3. AppEvents/Notifications specifically have a *second* schema-definition
surface beyond app.sql (their own inline CREATE TABLE IF NOT EXISTS in
application code) - keep that in sync too, using the real classes rather
than re-parsing their SQL as text.
4. ensure_table_columns() backfill: a table missing one column gets it added
with the right type.
"""
import os
import sqlite3
import sys
from unittest.mock import patch
import pytest
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "server"))
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from db.schema_columns import TABLE_COLUMNS # noqa: E402
from db.db_upgrade import ensure_table_columns # noqa: E402
from db_test_helpers import make_db, DummyDB # noqa: E402
_APP_SQL_PATH = os.path.join(
os.path.dirname(__file__), "..", "..", "server", "db", "schema", "app.sql"
)
def _columns_from_ddl(ddl_sql, table):
"""Execute DDL into a fresh in-memory connection and return the
resulting {column: declared_type} map, via SQLite's own DDL parser
rather than a hand-rolled regex. Types normalized (stripped, uppercased)
so harmless casing differences aren't reported as drift."""
conn = sqlite3.connect(":memory:")
try:
conn.executescript(ddl_sql)
return {
row[1]: row[2].strip().upper()
for row in conn.execute(f'PRAGMA table_info("{table}")').fetchall()
}
finally:
conn.close()
def _drift(ddl_sql, table):
"""Column-level differences between TABLE_COLUMNS and ddl_sql's actual
schema for table - missing columns, extra columns, and type mismatches
on columns present in both. Empty set = no drift."""
expected = {name: t.strip().upper() for name, t in TABLE_COLUMNS[table].items()}
actual = _columns_from_ddl(ddl_sql, table)
drift = set()
for name in expected.keys() - actual.keys():
drift.add(f"missing:{name}")
for name in actual.keys() - expected.keys():
drift.add(f"extra:{name}")
for name in expected.keys() & actual.keys():
if expected[name] != actual[name]:
drift.add(f"type:{name}({expected[name]!r} != {actual[name]!r})")
return drift
class TestNoDriftAgainstRealAppSql:
@pytest.mark.parametrize("table", list(TABLE_COLUMNS.keys()))
def test_table_matches_app_sql(self, table):
app_sql = open(_APP_SQL_PATH).read()
drift = _drift(app_sql, table)
assert not drift, (
f"{table}: TABLE_COLUMNS (server/db/schema_columns.py) and "
f"app.sql disagree on {drift} - update whichever one is stale"
)
class TestGuardActuallyDetectsDrift:
"""Proves the comparison logic can fail, not just a check that always
reports success - without this, it's possible to ship a guard that
passes regardless of what it's given."""
def test_missing_columns_detected(self):
broken_sql = "CREATE TABLE Events (eveMac STRING (50), eveIp STRING (50));"
drift = _drift(broken_sql, "Events")
assert drift == {
"missing:eveDateTime", "missing:eveEventType", "missing:eveAdditionalInfo",
"missing:evePendingAlertEmail", "missing:evePairEventRowid",
}
def test_extra_column_detected(self):
broken_sql = (
"CREATE TABLE Sessions (sesMac STRING (50), sesUnexpectedNewColumn TEXT);"
)
drift = _drift(broken_sql, "Sessions")
assert "extra:sesUnexpectedNewColumn" in drift
def test_type_mismatch_detected(self):
broken_sql = "CREATE TABLE Events (eveMac INTEGER, eveIp TEXT);"
drift = _drift(broken_sql, "Events")
assert any(item.startswith("type:eveMac") for item in drift)
class TestInlineDDLMatchesConstant:
"""AppEvents/Notifications each have a second schema-definition surface
beyond app.sql - the inline CREATE TABLE IF NOT EXISTS in their own
Python classes (found during Design §3 implementation - see
scan-pipeline-hardening.md's correction). Exercised via the real classes,
not by re-parsing their embedded SQL as text."""
def test_app_events_inline_ddl_matches_constant(self):
from workflows.app_events import AppEvent_obj
conn = make_db()
db = DummyDB(conn)
AppEvent_obj(db) # drops + recreates AppEvents with the real inline DDL
cols = {row[1] for row in conn.execute('PRAGMA table_info("AppEvents")').fetchall()}
assert cols == set(TABLE_COLUMNS["AppEvents"].keys())
conn.close()
def test_notifications_inline_ddl_matches_constant(self):
from models.notification_instance import NotificationInstance
conn = make_db()
db = DummyDB(conn)
with patch("models.notification_instance.get_setting_value", return_value=""), \
patch("models.notification_instance.Logger"):
NotificationInstance(db)
cols = {row[1] for row in conn.execute('PRAGMA table_info("Notifications")').fetchall()}
assert cols == set(TABLE_COLUMNS["Notifications"].keys())
conn.close()
class TestEnsureTableColumnsBackfill:
"""ensure_table_columns() must repair a table that's missing a column -
mirroring however Devices' existing 18 ensure_column() calls are (or
aren't currently) tested, generalized to the four registered tables."""
@pytest.mark.parametrize("table", list(TABLE_COLUMNS.keys()))
def test_missing_column_is_backfilled(self, table):
columns = TABLE_COLUMNS[table]
first_col, first_type = next(iter(columns.items()))
remaining = {c: t for c, t in columns.items() if c != first_col}
conn = sqlite3.connect(":memory:")
col_defs = ", ".join(f'"{c}" {t}' for c, t in remaining.items())
conn.execute(f"CREATE TABLE {table} ({col_defs})")
cur = conn.cursor()
ok = ensure_table_columns(cur, table)
assert ok, f"ensure_table_columns({table}) reported failure"
info = {row[1]: row[2] for row in conn.execute(f'PRAGMA table_info("{table}")').fetchall()}
assert set(info.keys()) == set(columns.keys()), f"{table}: backfill did not restore {first_col}"
assert info[first_col].strip().upper() == first_type.strip().upper(), (
f"{table}: {first_col} restored with type {info[first_col]!r}, expected {first_type!r}"
)
conn.close()
def test_missing_table_skips_without_error(self):
conn = sqlite3.connect(":memory:")
cur = conn.cursor()
ok = ensure_table_columns(cur, "Notifications")
assert ok, "a not-yet-created table must not be treated as a failure"
conn.close()
+125
View File
@@ -0,0 +1,125 @@
"""
Tests for server/scan/presence.py's current_scan_presence_condition() -
the shared predicate helper from scan-pipeline-hardening.md Design §1.
Two things are tested:
1. The helper itself: correct SQL fragment, and rejects anything that isn't
a plain SQL identifier (the trust-boundary check - this function does raw
string interpolation, never parameterized SQL).
2. A positive, AST-based guard (not a grep for one hand-written spelling,
which is trivially defeated by an equivalent one - scanPresence <> 0,
bare scanPresence, NOT scanPresence = 0, etc.) that each migrated
consumer function's source actually calls the helper the expected number
of times. This is what stops a future change from quietly reintroducing
a hand-written predicate instead of calling the shared one.
Not covered here on purpose: "New Connections" (session_events.py) and the
raw Sessions insert (device_handling.py's create_new_devices()) - both need
the actual scanLastIP/scanVendor *values* off the presence-asserting row via
MIN()/GROUP BY, not just a boolean, so they keep their own hand-written
aggregation - see scan-pipeline-hardening.md Design §1's correction.
"""
import ast
import inspect
import os
import sqlite3
import sys
import textwrap
import pytest
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "server"))
from scan.presence import current_scan_presence_condition # noqa: E402
from scan import device_handling # noqa: E402
from scan import session_events # noqa: E402
class TestHelperCorrectness:
def test_returns_expected_sql_fragment(self):
result = current_scan_presence_condition("devMac")
assert "EXISTS (" in result
assert "SELECT 1 FROM CurrentScan" in result
assert "scanMac = devMac" in result
assert "scanPresence = 1" in result
def test_accepts_qualified_column_reference(self):
result = current_scan_presence_condition("CurrentScan.scanMac")
assert "scanMac = CurrentScan.scanMac" in result
@pytest.mark.parametrize("bad_value", [
"devMac; DROP TABLE Devices--",
"devMac OR 1=1",
"'; DELETE FROM Devices; --",
"devMac)",
"",
"123devMac",
])
def test_rejects_non_identifier_input(self, bad_value):
with pytest.raises(ValueError):
current_scan_presence_condition(bad_value)
@pytest.mark.parametrize("bad_value", ["presence_scan", "presence_scan.scanMac"])
def test_rejects_presence_scan_qualifier(self, bad_value):
"""presence_scan is this helper's own internal subquery alias."""
with pytest.raises(ValueError):
current_scan_presence_condition(bad_value)
class TestQualifiedColumnExecutesCorrectly:
"""Executes the fragment, not just checks the generated SQL text - proves
a qualified mac_column ("CurrentScan.scanMac") still discriminates
per-row rather than collapsing into "does any row assert presence"."""
def test_only_the_present_mac_matches(self):
conn = sqlite3.connect(":memory:")
conn.execute("CREATE TABLE CurrentScan (scanMac TEXT, scanPresence INTEGER)")
conn.execute("INSERT INTO CurrentScan VALUES ('aa', 1)") # present
conn.execute("INSERT INTO CurrentScan VALUES ('bb', 0)") # row exists, not present
conn.commit()
condition = current_scan_presence_condition("CurrentScan.scanMac")
rows = conn.execute(
f"SELECT scanMac, {condition} AS is_present FROM CurrentScan"
).fetchall()
assert dict(rows) == {"aa": 1, "bb": 0}, (
"each row must be checked against its own scanMac, not collapse "
"into a table-wide 'does anything assert presence' check"
)
def _call_count(func, target_name="current_scan_presence_condition"):
"""Count calls to target_name within func's own source (AST-based, not
a text grep - resilient to reformatting, doesn't care how a bypass
might be spelled, only whether the actual call is present)."""
source = textwrap.dedent(inspect.getsource(func))
tree = ast.parse(source)
count = 0
for node in ast.walk(tree):
if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) and node.func.id == target_name:
count += 1
return count
class TestConsumersCallTheHelper:
"""Guards the five sites that were migrated to the shared predicate."""
def test_update_presence_from_current_scan_calls_helper_twice(self):
assert _call_count(device_handling.update_presence_from_CurrentScan) == 2, (
"update_presence_from_CurrentScan() has two statements (present/not-present) "
"- both must call current_scan_presence_condition()"
)
def test_update_dev_last_connection_calls_helper_once(self):
assert _call_count(device_handling.update_devLastConnection_from_CurrentScan) == 1
def test_insert_events_calls_helper_at_least_three_times(self):
"""insert_events() contains four queries total - Device Down (x2),
Disconnected, and New Connections. Only the first three are plain
boolean-predicate sites; New Connections keeps its own present_agg/
MIN(scanLastIP) aggregation on purpose (see module docstring), so
this asserts >= 3, not == 4."""
assert _call_count(session_events.insert_events) >= 3
+47
View File
@@ -170,3 +170,50 @@ class TestNewDeviceEventNoDuplicatesAcrossPlugins:
"differing scanLastIP/scanVendor across plugin rows for the same "
"new MAC must not produce duplicate New Device events"
)
class TestBlankMacNeverCreatesDevice:
"""A row with a blank/null-equivalent scanMac must never originate a
Devices row, even with scanCreatesDevice = 1 (the default) - this is the
backstop for a plugin that has rows it can't attach a real MAC to but
forgot to (or can't) set scanCreatesDevice = 0 itself. A well-behaved
plugin should still set scanCreatesDevice = 0 for such rows (see
plugin-import-behavior-controls.md) - this guard exists for the case
where it doesn't, so a blank MAC can never create a device regardless."""
def test_blank_scanmac_with_creates_device_one_creates_nothing(self):
conn = make_db()
insert_current_scan_row_from_dict(
conn, make_current_scan_dict("", scanCreatesDevice=1)
)
db = DummyDB(conn)
device_handling.create_new_devices(db)
assert _devices(db) == set()
rows = conn.execute(
"SELECT * FROM Events WHERE eveMac = '' AND eveEventType = 'New Device'"
).fetchall()
assert rows == [], "a blank scanMac must not produce an orphan New Device event either"
def test_multiple_plugins_sharing_blank_scanmac_creates_nothing(self):
"""The scenario this guard was actually written for: several
unrelated rows (e.g. containers with no routable MAC) all reporting
scanMac = '' collapse into one CurrentScan group - that group must
never create a device, regardless of how many rows are in it."""
conn = make_db()
insert_current_scan_row_from_dict(
conn, make_current_scan_dict("", scanSourcePlugin="PLUGINA", scanCreatesDevice=0)
)
insert_current_scan_row_from_dict(
conn, make_current_scan_dict("", scanSourcePlugin="PLUGINB", scanCreatesDevice=1)
)
db = DummyDB(conn)
device_handling.create_new_devices(db)
assert _devices(db) == set(), (
"even a single row asserting scanCreatesDevice = 1 for a blank MAC "
"must not create a device - most-permissive-wins does not override "
"the blank-MAC guard"
)