From 37bf86a8057cd4c148ded02fbbd1c89e0f4976ca Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Fri, 11 Sep 2026 08:26:16 +1000 Subject: [PATCH] BE: conditional device import support, notification supression support during scan #1721 --- server/scan/presence.py | 16 +++++++-- test/db/test_schema_drift_guard.py | 54 +++++++++++++++++++++--------- test/scan/test_presence_helper.py | 30 +++++++++++++++++ 3 files changed, 83 insertions(+), 17 deletions(-) diff --git a/server/scan/presence.py b/server/scan/presence.py index bd89fe82..1d45743a 100644 --- a/server/scan/presence.py +++ b/server/scan/presence.py @@ -13,6 +13,13 @@ def current_scan_presence_condition(mac_column: str) -> str: 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 @@ -24,7 +31,12 @@ def current_scan_presence_condition(mac_column: str) -> str: """ 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 - WHERE scanMac = {mac_column} AND scanPresence = 1 + SELECT 1 FROM CurrentScan AS presence_scan + WHERE presence_scan.scanMac = {mac_column} AND presence_scan.scanPresence = 1 )""" diff --git a/test/db/test_schema_drift_guard.py b/test/db/test_schema_drift_guard.py index 11bd79ed..ab30127c 100644 --- a/test/db/test_schema_drift_guard.py +++ b/test/db/test_schema_drift_guard.py @@ -36,23 +36,37 @@ _APP_SQL_PATH = os.path.join( def _columns_from_ddl(ddl_sql, table): - """Execute a CREATE TABLE (or full multi-statement schema) into a fresh - in-memory connection and return the resulting column name set - uses - SQLite's own DDL parser rather than a hand-rolled regex, so it can't be - fooled by formatting differences that a text-based diff would trip on.""" + """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] for row in conn.execute(f'PRAGMA table_info("{table}")').fetchall()} + 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 names present in one source but not the other. Empty = no drift.""" - expected = set(TABLE_COLUMNS[table].keys()) + """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) - return expected.symmetric_difference(actual) + + 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: @@ -72,17 +86,24 @@ class TestGuardActuallyDetectsDrift: passes regardless of what it's given.""" def test_missing_columns_detected(self): - broken_sql = "CREATE TABLE Events (eveMac TEXT, eveIp TEXT);" + broken_sql = "CREATE TABLE Events (eveMac STRING (50), eveIp STRING (50));" drift = _drift(broken_sql, "Events") assert drift == { - "eveDateTime", "eveEventType", "eveAdditionalInfo", - "evePendingAlertEmail", "evePairEventRowid", + "missing:eveDateTime", "missing:eveEventType", "missing:eveAdditionalInfo", + "missing:evePendingAlertEmail", "missing:evePairEventRowid", } def test_extra_column_detected(self): - broken_sql = "CREATE TABLE Sessions (sesMac TEXT, sesUnexpectedNewColumn TEXT);" + broken_sql = ( + "CREATE TABLE Sessions (sesMac STRING (50), sesUnexpectedNewColumn TEXT);" + ) drift = _drift(broken_sql, "Sessions") - assert "sesUnexpectedNewColumn" in drift + 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: @@ -136,8 +157,11 @@ class TestEnsureTableColumnsBackfill: ok = ensure_table_columns(cur, table) assert ok, f"ensure_table_columns({table}) reported failure" - cols = {row[1] for row in conn.execute(f'PRAGMA table_info("{table}")').fetchall()} - assert cols == set(columns.keys()), f"{table}: backfill did not restore {first_col}" + 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): diff --git a/test/scan/test_presence_helper.py b/test/scan/test_presence_helper.py index c2b2ddc7..cf906b52 100644 --- a/test/scan/test_presence_helper.py +++ b/test/scan/test_presence_helper.py @@ -23,6 +23,7 @@ aggregation - see scan-pipeline-hardening.md Design ยง1's correction. import ast import inspect import os +import sqlite3 import sys import textwrap @@ -60,6 +61,35 @@ class TestHelperCorrectness: 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