From c3bdd92b8dbe85c198aee422216a5048dd29f360 Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Thu, 10 Sep 2026 13:51:09 +1000 Subject: [PATCH] BE: conditional device import support, notification supression support during scan #1721 --- .claude/skills/plugin-development/SKILL.md | 1 + .claude/skills/scan-pipeline/SKILL.md | 2 +- .../skills/plugin-development/plugin-skill.md | 1 + .gemini/skills/scan-pipeline/SKILL.md | 2 +- .../skills/plugin-run-development/SKILL.md | 1 + .github/skills/scan-pipeline/SKILL.md | 2 +- docs/PLUGINS_DEV.md | 26 +++ docs/PLUGINS_DEV_DATA_CONTRACT.md | 37 +++- docs/PLUGINS_DEV_SETTINGS.md | 1 + server/db/db_upgrade.py | 14 +- server/db/schema/app.sql | 5 +- server/plugin.py | 13 +- server/scan/device_handling.py | 89 +++++++- server/scan/session_events.py | 23 +- test/db_test_helpers.py | 96 +++++++-- test/scan/test_import_on.py | 130 +++++++++++ test/scan/test_scan_creates_device.py | 130 +++++++++++ test/scan/test_scan_notification_mode.py | 201 ++++++++++++++++++ test/scan/test_scan_presence.py | 178 ++++++++++++++++ 19 files changed, 915 insertions(+), 37 deletions(-) create mode 100644 test/scan/test_import_on.py create mode 100644 test/scan/test_scan_creates_device.py create mode 100644 test/scan/test_scan_notification_mode.py create mode 100644 test/scan/test_scan_presence.py diff --git a/.claude/skills/plugin-development/SKILL.md b/.claude/skills/plugin-development/SKILL.md index eccc9741..fad051fa 100644 --- a/.claude/skills/plugin-development/SKILL.md +++ b/.claude/skills/plugin-development/SKILL.md @@ -33,6 +33,7 @@ server/plugins// - `_CMD`: script path. - `_RUN_TIMEOUT`: timeout in seconds — **enforced by the core plugin runner as the whole script's kill-timeout** (`server/plugin.py` passes it straight to `subprocess(..., timeout=...)`). Not a safe per-HTTP-call timeout — don't reuse it for individual network calls in a loop, or one slow call can burn the whole budget and get the process killed before it writes its result file. Two correct alternatives: `config.json`'s `"timeoutMultiplier": true` on a `params[]` entry for a config-declared, known-length loop (see `arp_scan`); `plugin_helper.per_item_timeout()` for a runtime-variable-length loop (see the `_publisher_*` plugins). - `_WATCH`: columns to watch for changes. +- `_IMPORT_ON`: optional — gates whether this run's rows get promoted into `CurrentScan` (only relevant if `mapped_to_table: "CurrentScan"`). See `docs/PLUGINS_DEV_DATA_CONTRACT.md` for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns. ## Data Contract diff --git a/.claude/skills/scan-pipeline/SKILL.md b/.claude/skills/scan-pipeline/SKILL.md index ccd35d11..f271a53a 100644 --- a/.claude/skills/scan-pipeline/SKILL.md +++ b/.claude/skills/scan-pipeline/SKILL.md @@ -51,7 +51,7 @@ This is the scan-pipeline-local half of a bigger attribution system — see the 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. 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 the otherwise-unused `server/db/schema/app.sql` reference copy) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. 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. +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 the otherwise-unused `server/db/schema/app.sql` reference copy) 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. 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. ## When to read this vs. other docs/skills diff --git a/.gemini/skills/plugin-development/plugin-skill.md b/.gemini/skills/plugin-development/plugin-skill.md index 39129d51..9f5a8061 100644 --- a/.gemini/skills/plugin-development/plugin-skill.md +++ b/.gemini/skills/plugin-development/plugin-skill.md @@ -43,6 +43,7 @@ server/plugins// - `_CMD`: script path - `_RUN_TIMEOUT`: timeout in seconds — **this is enforced by the core plugin runner as the whole script's kill-timeout** (`server/plugin.py` passes it straight to `subprocess(..., timeout=...)`). It is not a safe per-HTTP-call timeout — don't reuse it for individual network calls in a loop, or one slow call can burn the whole budget and get the process killed before it writes its result file. - `_WATCH`: columns to watch for changes +- `_IMPORT_ON`: optional — gates whether this run's rows get promoted into `CurrentScan` (only relevant if `mapped_to_table: "CurrentScan"`). See `docs/PLUGINS_DEV_DATA_CONTRACT.md` for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns. ## Data Contract diff --git a/.gemini/skills/scan-pipeline/SKILL.md b/.gemini/skills/scan-pipeline/SKILL.md index 64dc00d6..848e156c 100644 --- a/.gemini/skills/scan-pipeline/SKILL.md +++ b/.gemini/skills/scan-pipeline/SKILL.md @@ -51,7 +51,7 @@ This is the scan-pipeline-local half of a bigger attribution system — see the 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. 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 the otherwise-unused `server/db/schema/app.sql` reference copy) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. 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. +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 the otherwise-unused `server/db/schema/app.sql` reference copy) 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. 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. ## When to read this vs. other docs/skills diff --git a/.github/skills/plugin-run-development/SKILL.md b/.github/skills/plugin-run-development/SKILL.md index 5df100d1..95933b40 100644 --- a/.github/skills/plugin-run-development/SKILL.md +++ b/.github/skills/plugin-run-development/SKILL.md @@ -44,6 +44,7 @@ server/plugins// - `_CMD`: script path - `_RUN_TIMEOUT`: timeout in seconds — **this is enforced by the core plugin runner as the whole script's kill-timeout** (`server/plugin.py` passes it straight to `subprocess(..., timeout=...)`). It is not a safe per-HTTP-call timeout — don't reuse it for individual network calls in a loop, or one slow call can burn the whole budget and get the process killed before it writes its result file. - `_WATCH`: columns to watch for changes +- `_IMPORT_ON`: optional — gates whether this run's rows get promoted into `CurrentScan` (only relevant if `mapped_to_table: "CurrentScan"`). See `docs/PLUGINS_DEV_DATA_CONTRACT.md` for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns. ## Data Contract diff --git a/.github/skills/scan-pipeline/SKILL.md b/.github/skills/scan-pipeline/SKILL.md index 670a9acf..1c84d3e2 100644 --- a/.github/skills/scan-pipeline/SKILL.md +++ b/.github/skills/scan-pipeline/SKILL.md @@ -51,7 +51,7 @@ This is the scan-pipeline-local half of a bigger attribution system — see the 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. 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 the otherwise-unused `server/db/schema/app.sql` reference copy) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. 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. +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 the otherwise-unused `server/db/schema/app.sql` reference copy) 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. 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. ## When to read this vs. other docs/skills diff --git a/docs/PLUGINS_DEV.md b/docs/PLUGINS_DEV.md index 6125a0a6..4c7f573f 100755 --- a/docs/PLUGINS_DEV.md +++ b/docs/PLUGINS_DEV.md @@ -226,6 +226,7 @@ These control core plugin behavior: | `WATCH` | Monitor for changes | optional | Column names | | `REPORT_ON` | When to notify | optional | `new`, `watched-changed`, `watched-not-changed`, `missing-in-last-scan` | | `DB_PATH` | External DB path | If using SQLite | `/path/to/db.db` | +| `IMPORT_ON` | Gate whether this run's rows are promoted into `CurrentScan` | optional | Boolean. Only affects plugins with `mapped_to_table: "CurrentScan"` — see [Database Mapping](#database-mapping) below. | See [PLUGINS_DEV_SETTINGS.md](PLUGINS_DEV_SETTINGS.md) for full component types and examples. @@ -308,6 +309,31 @@ To always map a static value (not read from plugin output): } ``` +### Import Behavior Columns (`scanCreatesDevice`, `scanNotificationMode`, `scanPresence`) + +Three optional columns on `CurrentScan` control what happens once a row reaches it — see the [Data contract](PLUGINS_DEV_DATA_CONTRACT.md#import-behavior-columns) for the full contract (allowed values, defaults, downstream effects). All three default to today's behavior if never mapped, so existing plugins need no changes. + +Most plugins map a single static value for the whole import via `mapped_to_column_data` — e.g. an enrichment-only plugin that should never originate a new device: + +```json +{ + "column": "NameDoesntMatter", + "mapped_to_column": "scanCreatesDevice", + "mapped_to_column_data": { + "value": 0 + } +} +``` + +A plugin sophisticated enough to know per-row whether an entry is a live/active sighting (e.g. a DHCP lease with a `state` field) can instead map a per-row value via the normal `mapped_to_column` mechanism, the same way any other data-carrying column is mapped: + +```json +{ + "column": "watchedValue1", + "mapped_to_column": "scanPresence" +} +``` + --- ## Persisting Plugin Data (State & Config Files) diff --git a/docs/PLUGINS_DEV_DATA_CONTRACT.md b/docs/PLUGINS_DEV_DATA_CONTRACT.md index c2ad2f14..e278d1c6 100644 --- a/docs/PLUGINS_DEV_DATA_CONTRACT.md +++ b/docs/PLUGINS_DEV_DATA_CONTRACT.md @@ -149,11 +149,44 @@ CREATE TABLE CurrentScan ( scanParentMAC STRING(250), scanParentPort STRING(250), scanType STRING(250), - UNIQUE(scanMac) + scanCreatesDevice BOOLEAN NOT NULL DEFAULT (1) CHECK (scanCreatesDevice IN (0, 1)), + scanNotificationMode STRING(10) NOT NULL DEFAULT ('normal'), + scanPresence BOOLEAN NOT NULL DEFAULT (1) CHECK (scanPresence IN (0, 1)) ) ``` -As the documentation might become outdated, it's good practice to check the latest definition of the `CurrentScan` table in the `app.sql` script in the code base. +As the documentation might become outdated, it's good practice to check the latest definition of the `CurrentScan` table in `server/db/db_upgrade.py`'s `ensure_CurrentScan()` (the version that actually runs) in the code base — not `app.sql`, which is a reference-only copy the running application never loads. + +### Import Behavior Columns + +Three optional `CurrentScan` columns, all independent of each other, control what happens once a row reaches the table. + +| 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. | +| `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. | + +**Fallback for missing/invalid values** + +| Column | Missing/invalid value → | +|---|---| +| `scanCreatesDevice` | `1` (create) | +| `scanNotificationMode` | `normal` | +| `scanPresence` | `1` (asserts presence) | + +**Multiple plugins reporting the same MAC in the same scan cycle** (the normal case, not an edge case — see the `scan-pipeline` skill) resolve per column, not uniformly: `scanCreatesDevice` and `scanPresence` are most-permissive-wins (any row saying `1` wins), while `scanNotificationMode` is most-*restrictive*-wins (any row saying `quiet` suppresses the notification, even if a sibling row says `normal`) — erring toward under-notifying rather than spamming. + +**Combination matrix** — not every combination is meaningful for every plugin; pick the one that matches what your plugin actually knows: + +| `scanCreatesDevice` | `scanPresence` | Meaning | +|---|---|---| +| 1 | 1 | Normal discovery (the default) | +| 1 | 0 | Inventory/identity import — create the device, but don't claim it's online right now | +| 0 | 1 | Presence-confirming enrichment — never originate a device, but assert presence for one that exists | +| 0 | 0 | Silent enrichment — never originate a device, no presence claim either | + +`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). ## Examples diff --git a/docs/PLUGINS_DEV_SETTINGS.md b/docs/PLUGINS_DEV_SETTINGS.md index 7688ce77..9f791eac 100644 --- a/docs/PLUGINS_DEV_SETTINGS.md +++ b/docs/PLUGINS_DEV_SETTINGS.md @@ -92,6 +92,7 @@ These function names have special meaning and control core plugin behavior: | `WATCH` | **Which columns to monitor for changes** | multi-select | optional | Column names from data contract | | `REPORT_ON` | **When to send notifications** | select | optional | `"new"`, `"watched-changed"`, `"watched-not-changed"`, `"missing-in-last-scan"` | | `DB_PATH` | **External database path** | input | If using SQLite plugin | File path: `"/etc/pihole/pihole-FTL.db"` | +| `IMPORT_ON` | **Gates whether this run's rows get promoted into `CurrentScan`** | checkbox | optional | Boolean. Only meaningful for plugins with `mapped_to_table: "CurrentScan"`. Absent = always import. `False` skips *only* the `CurrentScan` promotion — the plugin still runs, and its own data table (`Plugins_Objects`) still gets written. See [Data contract](PLUGINS_DEV_DATA_CONTRACT.md) for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns, which control finer-grained behavior once a row *does* reach `CurrentScan`. | ### API & Data Settings diff --git a/server/db/db_upgrade.py b/server/db/db_upgrade.py index 3851f8e8..245c1759 100755 --- a/server/db/db_upgrade.py +++ b/server/db/db_upgrade.py @@ -572,16 +572,22 @@ def ensure_CurrentScan(sql) -> bool: scanParentMAC STRING(250), scanParentPort STRING(250), scanFQDN STRING(250), - scanType STRING(250) + scanType STRING(250), + scanCreatesDevice BOOLEAN NOT NULL DEFAULT (1) CHECK (scanCreatesDevice IN (0, 1)), + scanNotificationMode STRING(10) NOT NULL DEFAULT ('normal'), + scanPresence BOOLEAN NOT NULL DEFAULT (1) CHECK (scanPresence IN (0, 1)) ); """) # scanMac has no uniqueness constraint - multiple plugins commonly report # the same MAC in one cycle (e.g. arp_scan + nslookup), so every lookup # keyed on scanMac (update_presence_from_CurrentScan, insert_events, the # LatestDeviceScan/LatestEventsPerMAC views) was a full table scan without - # this. Table is dropped every cycle, so the index is rebuilt with it - - # cheap insurance against O(n^2) scans at NOC-scale device counts (10k+ in - # real deployments). + # this. ensure_CurrentScan() itself only runs once, at app startup + # (DB.initDB() -> __main__.py) - each scan cycle only does + # DELETE FROM CurrentScan (session_events.py), which does not drop the + # table or its index, so this index is built once and then maintained + # incrementally, not rebuilt every cycle. Still cheap insurance against + # O(n^2) scans at NOC-scale device counts (10k+ in real deployments). sql.execute("CREATE INDEX IF NOT EXISTS idx_currentscan_scanmac ON CurrentScan(scanMac);") return True diff --git a/server/db/schema/app.sql b/server/db/schema/app.sql index 66358d01..b78d652a 100644 --- a/server/db/schema/app.sql +++ b/server/db/schema/app.sql @@ -165,7 +165,10 @@ CREATE TABLE CurrentScan ( scanParentMAC STRING(250), scanParentPort STRING(250), scanFQDN STRING(250), - scanType STRING(250) + scanType STRING(250), + scanCreatesDevice BOOLEAN NOT NULL DEFAULT (1) CHECK (scanCreatesDevice IN (0, 1)), + scanNotificationMode STRING(10) NOT NULL DEFAULT ('normal'), + scanPresence BOOLEAN NOT NULL DEFAULT (1) CHECK (scanPresence IN (0, 1)) ); CREATE INDEX idx_currentscan_scanmac ON CurrentScan(scanMac); CREATE TABLE IF NOT EXISTS AppEvents ( diff --git a/server/plugin.py b/server/plugin.py index bd2e6d55..8b81d285 100755 --- a/server/plugin.py +++ b/server/plugin.py @@ -974,7 +974,18 @@ def process_plugin_events(db, plugin, plugEventsArr): raise e # Perform database table mapping if enabled for the plugin - if len(pluginEvents) > 0 and "mapped_to_table" in plugin: + # IMPORT_ON is an optional reserved setting name - only plugins that declare it + # get gated; get_setting_value(default=None) distinguishes "never declared" + # (None, always import) from "declared and turned off" (falsy). Only the + # mapped_to_table promotion is skipped - the plugin's own Plugins_Objects/ + # Plugins_Events/Plugins_History writes above already happened regardless. + import_on_key = pluginPref + "_IMPORT_ON" + import_on = get_setting_value(import_on_key, default=None) + import_disabled = import_on is not None and not import_on + + if import_disabled: + mylog("debug", f"[Plugins] {import_on_key} is disabled - skipping table mapping for {pluginPref} this run") + elif len(pluginEvents) > 0 and "mapped_to_table" in plugin: # Initialize an empty list to store SQL parameters. sqlParams = [] diff --git a/server/scan/device_handling.py b/server/scan/device_handling.py index 59e5149c..2d7efe0d 100755 --- a/server/scan/device_handling.py +++ b/server/scan/device_handling.py @@ -202,23 +202,29 @@ def update_presence_from_CurrentScan(db): sql = db.sql mylog("debug", "[Update Devices] - Updating devPresentLastScan") - # Mark present if exists in CurrentScan + # Mark present only if a CurrentScan row for this MAC actually asserts presence + # (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(""" UPDATE Devices SET devPresentLastScan = 1 WHERE EXISTS ( SELECT 1 FROM CurrentScan WHERE devMac = scanMac + AND scanPresence = 1 ) """) - # Mark not present if not in CurrentScan + # Mark not present if no CurrentScan row for this MAC asserts presence sql.execute(""" UPDATE Devices SET devPresentLastScan = 0 WHERE NOT EXISTS ( SELECT 1 FROM CurrentScan WHERE devMac = scanMac + AND scanPresence = 1 ) """) @@ -647,17 +653,33 @@ def create_new_devices(db): # Insert events for new devices from CurrentScan (not yet in Devices) mylog("debug", '[New Devices] Insert "New Device" Events') + # scanCreates/scanQuiet are a per-MAC aggregate (one GROUP BY pass, not a + # correlated subquery re-evaluated per row - see prd-writing/scan-pipeline + # skills on why that matters at scale) so multiple plugins reporting the + # same never-before-seen MAC get one consistent decision instead of an + # arbitrary one: most-permissive-wins for whether it creates a device at + # all (skip the event entirely if nothing actually creates it below), + # most-restrictive-wins for whether it's quiet (evePendingAlertEmail). query_new_device_events = f""" INSERT OR IGNORE INTO Events ( eveMac, eveIp, eveDateTime, eveEventType, eveAdditionalInfo, evePendingAlertEmail ) - SELECT DISTINCT scanMac, scanLastIP, '{startTime}', 'New Device', scanVendor, 1 - FROM CurrentScan - WHERE NOT EXISTS ( + SELECT DISTINCT c.scanMac, c.scanLastIP, '{startTime}', 'New Device', c.scanVendor, + CASE WHEN agg.scanQuiet = 1 THEN 0 ELSE 1 END + FROM CurrentScan c + JOIN ( + SELECT scanMac, + MAX(scanCreatesDevice) AS scanCreates, + MAX(CASE WHEN scanNotificationMode = 'quiet' THEN 1 ELSE 0 END) AS scanQuiet + FROM CurrentScan + GROUP BY scanMac + ) agg ON agg.scanMac = c.scanMac + WHERE agg.scanCreates = 1 + AND NOT EXISTS ( SELECT 1 FROM Devices - WHERE devMac = scanMac + WHERE devMac = c.scanMac ) """ @@ -674,7 +696,8 @@ def create_new_devices(db): ) SELECT scanMac, scanLastIP, 'Connected', '{startTime}', NULL, NULL, 1, scanVendor FROM CurrentScan - WHERE EXISTS ( + WHERE scanPresence = 1 + AND EXISTS ( SELECT 1 FROM Devices WHERE devMac = scanMac ) @@ -706,7 +729,14 @@ def create_new_devices(db): devReqNicsOnline """ - newDevDefaults = f"""{safe_int("NEWDEV_devAlertEvents")}, + # Two variants of the same defaults, differing only in the alert-related + # leading two fields - devAlertDown/devAlertEvents are seeded to 0 instead + # of the NEWDEV_* globals when this MAC is quiet, so Down/Disconnected + # notifications are suppressed for the device's whole lifecycle "for free" + # through the existing devAlertDown/devAlertEvents gates in insert_events(), + # without needing an ongoing per-cycle re-classification (creation-time-only + # "quiet", not an import-owned ongoing policy - see the PRD's decision on this). + newDevDefaults_normal = f"""{safe_int("NEWDEV_devAlertEvents")}, {safe_int("NEWDEV_devAlertDown")}, {safe_int("NEWDEV_devPresentLastScan")}, {safe_int("NEWDEV_devIsArchived")}, @@ -724,9 +754,44 @@ def create_new_devices(db): {safe_int("NEWDEV_devReqNicsOnline")} """ - # Fetch data from CurrentScan skipping ignored devices by IP and MAC + newDevDefaults_quiet = f"""0, + 0, + {safe_int("NEWDEV_devPresentLastScan")}, + {safe_int("NEWDEV_devIsArchived")}, + {safe_int("NEWDEV_devIsNew")}, + {safe_int("NEWDEV_devSkipRepeated")}, + {safe_int("NEWDEV_devScan")}, + '{sanitize_SQL_input(get_setting_value("NEWDEV_devOwner"))}', + {safe_int("NEWDEV_devFavorite")}, + '{sanitize_SQL_input(get_setting_value("NEWDEV_devGroup"))}', + '{sanitize_SQL_input(get_setting_value("NEWDEV_devComments"))}', + {safe_int("NEWDEV_devLogEvents")}, + '{sanitize_SQL_input(get_setting_value("NEWDEV_devLocation"))}', + '{sanitize_SQL_input(get_setting_value("NEWDEV_devCustomProps"))}', + '{sanitize_SQL_input(get_setting_value("NEWDEV_devParentRelType"))}', + {safe_int("NEWDEV_devReqNicsOnline")} + """ + + # Most-restrictive-wins across every row for a MAC, one GROUP BY pass - + # matches the aggregate used for the "New Device" Events insert above. + quiet_macs = { + str(row[0]).lower() + for row in sql.execute(""" + SELECT scanMac + FROM CurrentScan + GROUP BY scanMac + HAVING SUM(CASE WHEN scanNotificationMode = 'quiet' THEN 1 ELSE 0 END) > 0 + """).fetchall() + } + + # Fetch data from CurrentScan skipping ignored devices by IP and MAC. + # scanCreatesDevice = 1 filter is most-permissive-wins: a MAC with at + # least one contributing row asserting creation still gets created, even + # 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 """ + FROM CurrentScan WHERE scanCreatesDevice = 1""" mylog("debug", f"[New Devices] Collecting New Devices Query: {query}") current_scan_data = sql.execute(query).fetchall() @@ -762,6 +827,10 @@ def create_new_devices(db): scanType, ) = row + newDevDefaults = ( + newDevDefaults_quiet if scanMac.lower() in quiet_macs else newDevDefaults_normal + ) + # Preserve raw values to determine source attribution raw_name = str(scanName).strip() if scanName else "" raw_vendor = str(scanVendor).strip() if scanVendor else "" diff --git a/server/scan/session_events.py b/server/scan/session_events.py index 77f5f0a8..bda637d8 100755 --- a/server/scan/session_events.py +++ b/server/scan/session_events.py @@ -192,6 +192,7 @@ def insert_events(db): AND {_SQL_NOT_FORCED_ONLINE} AND NOT EXISTS (SELECT 1 FROM CurrentScan WHERE devMac = scanMac + AND scanPresence = 1 ) """) # Check device down – sleeping devices whose sleep window has expired @@ -207,7 +208,8 @@ def insert_events(db): AND devPresentLastScan = 0 AND {_SQL_NOT_FORCED_ONLINE} AND NOT EXISTS (SELECT 1 FROM CurrentScan - WHERE devMac = scanMac) + WHERE devMac = scanMac + AND scanPresence = 1) AND NOT EXISTS (SELECT 1 FROM Events WHERE eveMac = devMac AND eveEventType = 'Device Down' @@ -216,6 +218,13 @@ def insert_events(db): # Check new Connections or Down Reconnections mylog("debug", "[Events] - 2 - New Connections") + # scanPresence = 1 filter: a row that doesn't assert presence never counts + # as "just connected", even if another row for the same MAC does (that + # row still passes the filter on its own - abstain, not override). + # evePendingAlertEmail comes from a per-MAC aggregate (one GROUP BY pass, + # not a correlated subquery - see the scan-pipeline skill): most- + # restrictive-wins, so any contributing row saying quiet suppresses the + # notification even if a sibling row for the same MAC says normal. sql.execute(f""" INSERT OR IGNORE INTO Events (eveMac, eveIp, eveDateTime, eveEventType, eveAdditionalInfo, evePendingAlertEmail) @@ -225,10 +234,17 @@ def insert_events(db): ELSE 'Connected' END, '', - 1 + CASE WHEN agg.scanQuiet = 1 THEN 0 ELSE 1 END FROM CurrentScan AS c LEFT JOIN LatestEventsPerMAC AS last_event ON c.scanMac = last_event.eveMac - WHERE last_event.devPresentLastScan = 0 OR last_event.eveMac IS NULL + JOIN ( + SELECT scanMac, + MAX(CASE WHEN scanNotificationMode = 'quiet' THEN 1 ELSE 0 END) AS scanQuiet + FROM CurrentScan + GROUP BY scanMac + ) agg ON agg.scanMac = c.scanMac + WHERE (last_event.devPresentLastScan = 0 OR last_event.eveMac IS NULL) + AND c.scanPresence = 1 """) # Check disconnections @@ -244,6 +260,7 @@ def insert_events(db): AND {_SQL_NOT_FORCED_ONLINE} AND NOT EXISTS (SELECT 1 FROM CurrentScan WHERE devMac = scanMac + AND scanPresence = 1 ) """) # Check IP Changed diff --git a/test/db_test_helpers.py b/test/db_test_helpers.py index e9ca7c44..edf10ffa 100644 --- a/test/db_test_helpers.py +++ b/test/db_test_helpers.py @@ -6,6 +6,7 @@ Import from any test subdirectory with: import sys, os sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) from db_test_helpers import make_db, insert_device, minutes_ago, DummyDB, down_event_macs, make_device_dict, sync_insert_devices + from db_test_helpers import make_current_scan_dict, insert_current_scan_row_from_dict from db_test_helpers import make_plugin_db, make_plugin_dict, make_plugin_event_row, seed_plugin_object, plugin_history_rows, plugin_objects_rows, PluginFakeDB from db_test_helpers import make_history_db """ @@ -94,19 +95,22 @@ CREATE_EVENTS = """ CREATE_CURRENT_SCAN = """ CREATE TABLE IF NOT EXISTS CurrentScan ( - scanMac TEXT, - scanLastIP TEXT, - scanVendor TEXT, - scanSourcePlugin TEXT, - scanName TEXT, - scanLastQuery TEXT, - scanLastConnection TEXT, - scanSyncHubNode TEXT, - scanSite TEXT, - scanSSID TEXT, - scanParentMAC TEXT, - scanParentPort TEXT, - scanType TEXT + scanMac TEXT, + scanLastIP TEXT, + scanVendor TEXT, + scanSourcePlugin TEXT, + scanName TEXT, + scanLastQuery TEXT, + scanLastConnection TEXT, + scanSyncHubNode TEXT, + scanSite TEXT, + scanSSID TEXT, + scanParentMAC TEXT, + scanParentPort TEXT, + scanType TEXT, + scanCreatesDevice INTEGER NOT NULL DEFAULT 1, + scanNotificationMode TEXT NOT NULL DEFAULT 'normal', + scanPresence INTEGER NOT NULL DEFAULT 1 ) """ @@ -117,6 +121,19 @@ CREATE_SETTINGS = """ ) """ +CREATE_SESSIONS = """ + CREATE TABLE IF NOT EXISTS Sessions ( + sesMac TEXT, + sesIp TEXT, + sesEventTypeConnection TEXT, + sesDateTimeConnection TEXT, + sesEventTypeDisconnection TEXT, + sesDateTimeDisconnection TEXT, + sesStillConnected INTEGER, + sesAdditionalInfo TEXT + ) +""" + # --------------------------------------------------------------------------- # DB factory @@ -136,6 +153,7 @@ def make_db(sleep_minutes: int = 30) -> sqlite3.Connection: cur.execute(CREATE_EVENTS) cur.execute(CREATE_CURRENT_SCAN) cur.execute(CREATE_SETTINGS) + cur.execute(CREATE_SESSIONS) cur.execute( "INSERT OR REPLACE INTO Settings (setKey, setValue) VALUES (?, ?)", ("NTFPRCS_sleep_time", str(sleep_minutes)), @@ -436,6 +454,58 @@ def insert_device_from_dict(conn: sqlite3.Connection, device: dict) -> None: conn.commit() +def make_current_scan_dict(mac: str = "aa:bb:cc:dd:ee:ff", **overrides) -> dict: + """ + Return a CurrentScan row dict with safe defaults matching the real + schema's defaults (scanCreatesDevice=1, scanNotificationMode='normal', + scanPresence=1 — i.e. today's unconditional behavior for a plugin that + never heard of these columns). Pass keyword arguments to override. + """ + base = { + "scanMac": mac, + "scanLastIP": "192.168.1.10", + "scanVendor": "Acme", + "scanSourcePlugin": "ARPSCAN", + "scanName": "Test Device", + "scanLastQuery": "2024-01-02 00:00:00", + "scanLastConnection": "2024-01-02 00:00:00", + "scanSyncHubNode": "", + "scanSite": "", + "scanSSID": "", + "scanParentMAC": "", + "scanParentPort": "", + "scanType": "", + "scanCreatesDevice": 1, + "scanNotificationMode": "normal", + "scanPresence": 1, + } + base.update(overrides) + return base + + +def insert_current_scan_row_from_dict(conn: sqlite3.Connection, row: dict) -> None: + """Insert a CurrentScan row dict (as produced by make_current_scan_dict). + + No dedup/uniqueness — CurrentScan legitimately holds multiple rows per + MAC (one per contributing plugin), unlike Devices. Accepts any subset of + CurrentScan columns; only keys present in the table are written. + """ + cur = conn.cursor() + cur.execute("PRAGMA table_info(CurrentScan)") + db_columns = {r[1] for r in cur.fetchall()} + + cols = [k for k in row.keys() if k in db_columns] + placeholders = ", ".join("?" for _ in cols) + col_list = ", ".join(cols) + values = [row[c] for c in cols] + + cur.execute( + f"INSERT INTO CurrentScan ({col_list}) VALUES ({placeholders})", + values, + ) + conn.commit() + + # --------------------------------------------------------------------------- # DummyDB — minimal wrapper used by scan.session_events helpers # --------------------------------------------------------------------------- diff --git a/test/scan/test_import_on.py b/test/scan/test_import_on.py new file mode 100644 index 00000000..61a5e7d5 --- /dev/null +++ b/test/scan/test_import_on.py @@ -0,0 +1,130 @@ +""" +Tests for the IMPORT_ON reserved setting name (server/plugin.py:process_plugin_events()). + +IMPORT_ON is an optional reserved setting: a plugin that never declares it +behaves exactly as today (always promote mapped_to_table rows into +CurrentScan). A plugin that declares it and has it set to a falsy value skips +only the CurrentScan promotion for this run - the plugin's own +Plugins_Objects/Plugins_Events/Plugins_History writes are unaffected. +""" + +import sys +import os + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +from db_test_helpers import ( # noqa: E402 + make_plugin_db, + make_plugin_dict, + make_plugin_event_row, + plugin_objects_rows, + CREATE_CURRENT_SCAN, +) + +from plugin import process_plugin_events # noqa: E402 + +PREFIX = "TESTPLG" + + +@pytest.fixture +def plugin_db(): + """PluginFakeDB backed by an in-memory SQLite DB that also has CurrentScan.""" + db, conn = make_plugin_db() + conn.execute(CREATE_CURRENT_SCAN) + conn.commit() + yield db, conn + conn.close() + + +def _mapped_plugin_dict(prefix: str) -> dict: + """A plugin dict with mapped_to_table: CurrentScan, mapping objectPrimaryId -> scanMac.""" + plugin = make_plugin_dict(prefix) + plugin["mapped_to_table"] = "CurrentScan" + plugin["database_column_definitions"] = [ + {"column": "objectPrimaryId", "mapped_to_column": "scanMac"}, + {"column": "objectSecondaryId", "mapped_to_column": "scanLastIP"}, + ] + return plugin + + +def _current_scan_macs(conn) -> set: + cur = conn.cursor() + cur.execute("SELECT scanMac FROM CurrentScan") + return {r[0] for r in cur.fetchall()} + + +def _settings(import_on_value): + """Monkeypatch target: _IMPORT_ON -> import_on_value, _REPORT_ON -> [], else ''.""" + def _get(key, default=""): + if key.endswith("_IMPORT_ON"): + return import_on_value + if key.endswith("_REPORT_ON"): + return [] + return default + return _get + + +class TestImportOnUndeclared: + """A plugin that never declares IMPORT_ON must behave exactly like today.""" + + def test_currentscan_promotion_happens_by_default(self, plugin_db, monkeypatch): + db, conn = plugin_db + # get_setting_value(default=None) for an undeclared setting must return + # the passed default (None here) - simulate that "never declared" reality. + monkeypatch.setattr("plugin.get_setting_value", _settings(None)) + + plugin = _mapped_plugin_dict(PREFIX) + events = [make_plugin_event_row(PREFIX, "aa:bb:cc:dd:ee:01", secondary_id="1.2.3.4")] + + process_plugin_events(db, plugin, events) + + assert _current_scan_macs(conn) == {"aa:bb:cc:dd:ee:01"} + assert len(plugin_objects_rows(conn, PREFIX)) == 1 + + +class TestImportOnDeclaredTrue: + def test_currentscan_promotion_happens(self, plugin_db, monkeypatch): + db, conn = plugin_db + monkeypatch.setattr("plugin.get_setting_value", _settings(True)) + + plugin = _mapped_plugin_dict(PREFIX) + events = [make_plugin_event_row(PREFIX, "aa:bb:cc:dd:ee:02", secondary_id="1.2.3.4")] + + process_plugin_events(db, plugin, events) + + assert _current_scan_macs(conn) == {"aa:bb:cc:dd:ee:02"} + + +class TestImportOnDeclaredFalse: + """The core behavior this mechanism exists for.""" + + def test_currentscan_promotion_skipped_but_plugin_tables_still_populate( + self, plugin_db, monkeypatch + ): + db, conn = plugin_db + monkeypatch.setattr("plugin.get_setting_value", _settings(False)) + + plugin = _mapped_plugin_dict(PREFIX) + events = [make_plugin_event_row(PREFIX, "aa:bb:cc:dd:ee:03", secondary_id="1.2.3.4")] + + process_plugin_events(db, plugin, events) + + assert _current_scan_macs(conn) == set(), ( + "IMPORT_ON=False must skip the CurrentScan promotion" + ) + assert len(plugin_objects_rows(conn, PREFIX)) == 1, ( + "the plugin's own Plugins_Objects write must be unaffected by IMPORT_ON" + ) + + def test_zero_is_also_falsy(self, plugin_db, monkeypatch): + """Settings are often stored/typed as 0/1, not Python bool - 0 must gate too.""" + db, conn = plugin_db + monkeypatch.setattr("plugin.get_setting_value", _settings(0)) + + plugin = _mapped_plugin_dict(PREFIX) + events = [make_plugin_event_row(PREFIX, "aa:bb:cc:dd:ee:04")] + + process_plugin_events(db, plugin, events) + + assert _current_scan_macs(conn) == set() diff --git a/test/scan/test_scan_creates_device.py b/test/scan/test_scan_creates_device.py new file mode 100644 index 00000000..194de316 --- /dev/null +++ b/test/scan/test_scan_creates_device.py @@ -0,0 +1,130 @@ +""" +Tests for scanCreatesDevice (server/scan/device_handling.py:create_new_devices()). + +scanCreatesDevice = 0 lets a plugin report a row without ever originating a +new Devices entry from it (an enrich-only/metadata plugin), while still being +able to update an already-existing device's fields via the separate +update_devices_data_from_scan() code path. Multiple plugins reporting the +same never-before-seen MAC resolve via most-permissive-wins: any row saying 1 +creates the device, regardless of how many other rows say 0. +""" + +import sys +import os +from unittest.mock import patch + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +from db_test_helpers import ( # noqa: E402 + make_db, + make_current_scan_dict, + insert_current_scan_row_from_dict, + make_device_dict, + insert_device_from_dict, + DummyDB, +) + +from server.scan import device_handling # noqa: E402 + + +def _devices(db: DummyDB) -> set: + rows = db._conn.execute("SELECT devMac FROM Devices").fetchall() + return {r["devMac"].lower() for r in rows} + + +class TestScanCreatesDeviceGate: + def test_scan_creates_device_zero_never_creates(self): + """A brand-new MAC reported only with scanCreatesDevice=0 is never created.""" + conn = make_db() + insert_current_scan_row_from_dict( + conn, make_current_scan_dict("aa:bb:cc:dd:ee:01", scanCreatesDevice=0) + ) + db = DummyDB(conn) + + device_handling.create_new_devices(db) + + assert _devices(db) == set() + + def test_scan_creates_device_one_creates_normally(self): + """Default (1) preserves today's behavior - regression guard.""" + conn = make_db() + insert_current_scan_row_from_dict( + conn, make_current_scan_dict("aa:bb:cc:dd:ee:02", scanCreatesDevice=1) + ) + db = DummyDB(conn) + + device_handling.create_new_devices(db) + + assert _devices(db) == {"aa:bb:cc:dd:ee:02"} + + def test_most_permissive_wins_across_plugins(self): + """Two plugins report the same never-before-seen MAC: one 0, one 1 -> created.""" + conn = make_db() + insert_current_scan_row_from_dict( + conn, + make_current_scan_dict( + "aa:bb:cc:dd:ee:03", scanSourcePlugin="ENRICH", scanCreatesDevice=0 + ), + ) + insert_current_scan_row_from_dict( + conn, + make_current_scan_dict( + "aa:bb:cc:dd:ee:03", scanSourcePlugin="ARPSCAN", scanCreatesDevice=1 + ), + ) + db = DummyDB(conn) + + device_handling.create_new_devices(db) + + assert _devices(db) == {"aa:bb:cc:dd:ee:03"} + + def test_missing_column_defaults_to_creates(self): + """An old plugin's row (column never mapped) must default to scanCreatesDevice=1.""" + conn = make_db() + # Insert without scanCreatesDevice at all - relies on the schema DEFAULT. + conn.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP) VALUES (?, ?)", + ("aa:bb:cc:dd:ee:04", "192.168.1.50"), + ) + conn.commit() + db = DummyDB(conn) + + device_handling.create_new_devices(db) + + assert _devices(db) == {"aa:bb:cc:dd:ee:04"} + + +class TestScanCreatesDeviceFieldAuthorityRegression: + """Locks in that update_devices_data_from_scan() needs zero scanCreatesDevice + awareness - a scanCreatesDevice=0 row updates an existing device's fields + exactly like any other row, through the pre-existing FIELD_SPECS/ + can_overwrite_field() authority mechanism.""" + + def test_enrich_only_row_still_updates_existing_device(self): + conn = make_db() + insert_device_from_dict( + conn, make_device_dict("aa:bb:cc:dd:ee:05", devName="(unknown)", devNameSource="") + ) + insert_current_scan_row_from_dict( + conn, + make_current_scan_dict( + "aa:bb:cc:dd:ee:05", + scanSourcePlugin="NSLOOKUP", + scanName="resolved-hostname", + scanCreatesDevice=0, + ), + ) + db = DummyDB(conn) + + with patch( + "server.scan.device_handling.get_plugin_authoritative_settings", + return_value={}, + ): + device_handling.update_devices_data_from_scan(db) + + row = conn.execute( + "SELECT devName FROM Devices WHERE devMac = 'aa:bb:cc:dd:ee:05'" + ).fetchone() + assert row["devName"] == "resolved-hostname", ( + "scanCreatesDevice=0 must not block ordinary field updates on an " + "existing device - identity/creation and field updates are separate paths" + ) diff --git a/test/scan/test_scan_notification_mode.py b/test/scan/test_scan_notification_mode.py new file mode 100644 index 00000000..d9d6585f --- /dev/null +++ b/test/scan/test_scan_notification_mode.py @@ -0,0 +1,201 @@ +""" +Tests for scanNotificationMode (server/scan/device_handling.py:create_new_devices(), +server/scan/session_events.py:insert_events()). + +quiet suppresses the outbound notification (evePendingAlertEmail=0) for New +Device/Connected/Down Reconnected events while still writing the Events row +(audit trail intact). It is creation-time-only ("decision A" in the PRD): a +device seeded quiet keeps devAlertDown=0/devAlertEvents=0 for its whole +lifecycle via the ordinary per-device settings, which then also suppresses +Device Down/Disconnected for free through the existing devAlertDown/ +devAlertEvents gates - no separate code path needed for those two. Multiple +plugins reporting the same MAC resolve via most-restrictive-wins: any row +saying quiet suppresses, even if a sibling row says normal. +""" + +import sys +import os + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +from db_test_helpers import ( # noqa: E402 + make_db, + make_current_scan_dict, + insert_current_scan_row_from_dict, + insert_device, + minutes_ago, + DummyDB, +) + +from server.scan import device_handling # noqa: E402 +from server.scan.session_events import insert_events # noqa: E402 + +MAC = "aa:bb:cc:dd:ee:01" + + +def _fake_get_setting_value(key, default=""): + """NEWDEV_devAlertEvents/devAlertDown -> 1 (a distinguishable non-zero + 'normal' default), everything else -> a harmless empty-ish value so the + rest of create_new_devices() doesn't choke on missing settings.""" + if key in ("NEWDEV_devAlertEvents", "NEWDEV_devAlertDown"): + return 1 + if key in ("NEWDEV_devPresentLastScan", "NEWDEV_devIsArchived", "NEWDEV_devIsNew", + "NEWDEV_devSkipRepeated", "NEWDEV_devScan", "NEWDEV_devFavorite", + "NEWDEV_devLogEvents", "NEWDEV_devReqNicsOnline"): + return 0 + return default + + +@pytest.fixture(autouse=True) +def _settings(monkeypatch): + # safe_int() re-imports get_setting_value from `helper` on every call + # (see server/db/db_helper.py) - patch the source module, not just + # device_handling's already-bound reference, or the patch is a no-op there. + monkeypatch.setattr("helper.get_setting_value", _fake_get_setting_value) + monkeypatch.setattr("server.scan.device_handling.get_setting_value", _fake_get_setting_value) + + +def _device_row(db: DummyDB, mac: str): + return db._conn.execute( + "SELECT devAlertDown, devAlertEvents FROM Devices WHERE devMac = ?", (mac,) + ).fetchone() + + +def _events(db: DummyDB, mac: str): + return db._conn.execute( + "SELECT eveEventType, evePendingAlertEmail FROM Events WHERE eveMac = ?", (mac,) + ).fetchall() + + +class TestQuietCreationTimeSeeding: + def test_quiet_new_device_gets_suppressed_event_and_seeded_alerts(self): + conn = make_db() + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanNotificationMode="quiet") + ) + db = DummyDB(conn) + + device_handling.create_new_devices(db) + + dev = _device_row(db, MAC) + assert dev["devAlertDown"] == 0 + assert dev["devAlertEvents"] == 0 + + events = _events(db, MAC) + assert len(events) == 1 + assert events[0]["eveEventType"] == "New Device" + assert events[0]["evePendingAlertEmail"] == 0 + + def test_normal_new_device_uses_global_defaults(self): + """Regression guard: default ('normal') behaves like today.""" + conn = make_db() + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanNotificationMode="normal") + ) + db = DummyDB(conn) + + device_handling.create_new_devices(db) + + dev = _device_row(db, MAC) + assert dev["devAlertDown"] == 1 + assert dev["devAlertEvents"] == 1 + + events = _events(db, MAC) + assert events[0]["evePendingAlertEmail"] == 1 + + def test_most_restrictive_wins_across_plugins(self): + """One plugin says normal, another says quiet for the same brand-new MAC.""" + conn = make_db() + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanSourcePlugin="ARPSCAN", scanNotificationMode="normal") + ) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanSourcePlugin="DOCKER", scanNotificationMode="quiet") + ) + db = DummyDB(conn) + + device_handling.create_new_devices(db) + + dev = _device_row(db, MAC) + assert dev["devAlertDown"] == 0 and dev["devAlertEvents"] == 0, ( + "most-restrictive-wins: any row saying quiet must suppress, " + "even though a sibling row for the same MAC says normal" + ) + events = _events(db, MAC) + assert len(events) == 1, "exactly one New Device event, not two conflicting ones" + assert events[0]["evePendingAlertEmail"] == 0 + + +class TestQuietIsCreationTimeOnlyNotOngoing: + """Decision 'A': quiet only affects the creation moment. Reclassifying the + plugin's row later must NOT retroactively change an already-seeded device's + alert settings - there is no ongoing per-cycle re-derivation.""" + + def test_reclassifying_to_normal_later_does_not_unsuppress(self): + conn = make_db() + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanNotificationMode="quiet") + ) + db = DummyDB(conn) + device_handling.create_new_devices(db) + assert _device_row(db, MAC)["devAlertDown"] == 0 + + # Next cycle: same device now reported as 'normal'. create_new_devices() + # is a no-op for it (INSERT OR IGNORE, already exists) - nothing should + # touch devAlertDown/devAlertEvents again. + conn.execute("DELETE FROM CurrentScan") + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanNotificationMode="normal") + ) + device_handling.create_new_devices(db) + + dev = _device_row(db, MAC) + assert dev["devAlertDown"] == 0 and dev["devAlertEvents"] == 0, ( + "quiet must not be an ongoing/import-owned policy - once seeded, " + "it's an ordinary per-device setting nothing re-derives" + ) + + +class TestQuietDeviceDownSuppressedForFree: + """Because devAlertDown=0 was seeded at creation, insert_events()'s + existing 'WHERE devAlertDown != 0' gate suppresses Device Down for this + device with zero new code in insert_events() itself.""" + + def test_quiet_device_going_absent_generates_no_down_event(self): + conn = make_db() + # Simulate a device already created quiet (devAlertDown/devAlertEvents=0), + # previously present, now absent this cycle (CurrentScan left empty). + insert_device( + conn, MAC, alert_down=0, present_last_scan=1, + last_connection=minutes_ago(60), + ) + db = DummyDB(conn) + + insert_events(db) + + rows = conn.execute( + "SELECT * FROM Events WHERE eveMac = ? AND eveEventType = 'Device Down'", (MAC,) + ).fetchall() + assert rows == [], "devAlertDown=0 (seeded via quiet) must suppress Device Down entirely" + + +class TestQuietReconnection: + def test_quiet_reconnect_event_is_suppressed(self): + conn = make_db() + insert_device( + conn, MAC, alert_down=1, present_last_scan=0, + last_connection=minutes_ago(60), + ) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanNotificationMode="quiet") + ) + db = DummyDB(conn) + + insert_events(db) + + rows = conn.execute( + "SELECT eveEventType, evePendingAlertEmail FROM Events WHERE eveMac = ?", (MAC,) + ).fetchall() + assert len(rows) == 1 + assert rows[0]["evePendingAlertEmail"] == 0 diff --git a/test/scan/test_scan_presence.py b/test/scan/test_scan_presence.py new file mode 100644 index 00000000..25e5c5f2 --- /dev/null +++ b/test/scan/test_scan_presence.py @@ -0,0 +1,178 @@ +""" +Tests for scanPresence (server/scan/device_handling.py:update_presence_from_CurrentScan(), +server/scan/session_events.py:insert_events() "New Connections" query, and the +raw Sessions insert inside create_new_devices()). + +scanPresence = 0 means "this row makes no presence claim" (identity/inventory +data), not "this device is offline" - semantics are abstain, not override: a +contradicting row from another plugin for the same MAC in the same cycle +still wins. Multiple call sites independently re-derive "is this MAC +currently present" from CurrentScan and all needed the same treatment - see +the scan-pipeline skill's gotcha on this. +""" + +import sys +import os + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +from db_test_helpers import ( # noqa: E402 + make_db, + make_current_scan_dict, + insert_current_scan_row_from_dict, + insert_device, + minutes_ago, + DummyDB, + down_event_macs, +) + +from server.scan import device_handling # noqa: E402 +from server.scan.session_events import insert_events, pair_sessions_events, create_sessions_snapshot # noqa: E402 + +MAC = "aa:bb:cc:dd:ee:01" + + +def _present(db: DummyDB, mac: str) -> int: + row = db._conn.execute( + "SELECT devPresentLastScan FROM Devices WHERE devMac = ?", (mac,) + ).fetchone() + return row["devPresentLastScan"] + + +class TestPresenceGateOnExistingDevice: + """update_presence_from_CurrentScan() - the badge/devPresentLastScan side.""" + + def test_default_presence_one_preserves_today(self): + conn = make_db() + insert_device(conn, MAC, alert_down=1, present_last_scan=0) + insert_current_scan_row_from_dict(conn, make_current_scan_dict(MAC)) + db = DummyDB(conn) + + device_handling.update_presence_from_CurrentScan(db) + + assert _present(db, MAC) == 1 + + def test_presence_zero_does_not_assert_online(self): + conn = make_db() + insert_device(conn, MAC, alert_down=1, present_last_scan=0) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanPresence=0) + ) + db = DummyDB(conn) + + device_handling.update_presence_from_CurrentScan(db) + + assert _present(db, MAC) == 0, ( + "scanPresence=0 must not claim the device is online, even though " + "a row for it exists in CurrentScan" + ) + + def test_abstain_not_override_contradicting_row_wins(self): + """One plugin abstains (0), another asserts presence (1) for the same MAC.""" + conn = make_db() + insert_device(conn, MAC, alert_down=1, present_last_scan=0) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanSourcePlugin="KEAAPI", scanPresence=0) + ) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanSourcePlugin="ARPSCAN", scanPresence=1) + ) + db = DummyDB(conn) + + device_handling.update_presence_from_CurrentScan(db) + + assert _present(db, MAC) == 1, "a contradicting presence=1 row must still win" + + +class TestNewConnectionsRespectsPresence: + """insert_events()'s New Connections query must not fire Connected for a + scanPresence=0-only row - this is what would otherwise leave a session + open forever with no way to close it (see the scan-pipeline skill).""" + + def test_presence_zero_brand_new_device_no_connected_event(self): + conn = make_db() + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanPresence=0) + ) + db = DummyDB(conn) + + insert_events(db) + + rows = conn.execute( + "SELECT * FROM Events WHERE eveMac = ? AND eveEventType IN ('Connected','Down Reconnected')", + (MAC,), + ).fetchall() + assert rows == [], "scanPresence=0 must not generate a Connected event" + + +class TestOnlineToPresenceZeroTransitionClosesSession: + """The regression this PRD review round specifically caught: a device + going from online to a scanPresence=0-only report must still get a + Device Down/Disconnected Event, or pair_sessions_events() never pairs a + closing event and the session view shows it as open forever.""" + + def test_device_down_fires_and_session_closes(self): + conn = make_db() + insert_device( + conn, MAC, alert_down=1, present_last_scan=1, + last_connection=minutes_ago(120), + ) + # Open session, as if create_new_devices() opened it on first connect. + conn.execute( + """INSERT INTO Sessions + (sesMac, sesIp, sesEventTypeConnection, sesDateTimeConnection, + sesEventTypeDisconnection, sesDateTimeDisconnection, sesStillConnected, sesAdditionalInfo) + VALUES (?, '192.168.1.10', 'Connected', ?, NULL, NULL, 1, '')""", + (MAC, minutes_ago(120)), + ) + conn.execute( + "INSERT INTO Events (eveMac, eveIp, eveDateTime, eveEventType, eveAdditionalInfo, evePendingAlertEmail) " + "VALUES (?, '192.168.1.10', ?, 'New Device', '', 1)", + (MAC, minutes_ago(120)), + ) + conn.commit() + + # This cycle: only a scanPresence=0 report for this MAC (e.g. an + # inventory plugin) - device transitions from online to "no presence". + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanPresence=0) + ) + db = DummyDB(conn) + + insert_events(db) + + assert MAC in down_event_macs(conn.cursor()), ( + "the Device Down query must fire even though a CurrentScan row " + "still exists for this MAC (just with scanPresence=0) - without " + "this the session below never gets a closing event to pair against" + ) + + pair_sessions_events(db) + create_sessions_snapshot(db) + + row = conn.execute( + "SELECT sesStillConnected FROM Sessions WHERE sesMac = ?", (MAC,) + ).fetchone() + assert row["sesStillConnected"] == 0, ( + "session must close (sesStillConnected=0) once the Device Down " + "event exists and gets paired - this is the bug found in PRD review" + ) + + +class TestPresenceZeroSessionsInsertGate: + """create_new_devices()'s raw INSERT INTO Sessions for already-existing + reconnecting devices must respect scanPresence too.""" + + def test_presence_zero_existing_device_no_open_session_inserted(self): + conn = make_db() + insert_device(conn, MAC, alert_down=1, present_last_scan=0) + insert_current_scan_row_from_dict( + conn, make_current_scan_dict(MAC, scanPresence=0) + ) + db = DummyDB(conn) + + device_handling.create_new_devices(db) + + rows = conn.execute( + "SELECT * FROM Sessions WHERE sesMac = ? AND sesStillConnected = 1", (MAC,) + ).fetchall() + assert rows == [], "scanPresence=0 must not open a session for a reconnecting device"