From 02557f0db5f7fa8637753c7dc967b4ba048d7f66 Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Sun, 18 Oct 2026 10:01:18 +1100 Subject: [PATCH 01/10] PLG: INTERNET plugin fix #1845 --- server/plugins/internet_ip/script.py | 16 ++- test/plugins/test_internet_ip_fallback.py | 130 ++++++++++++++++++++++ 2 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 test/plugins/test_internet_ip_fallback.py diff --git a/server/plugins/internet_ip/script.py b/server/plugins/internet_ip/script.py index af9c088e1..a19595264 100755 --- a/server/plugins/internet_ip/script.py +++ b/server/plugins/internet_ip/script.py @@ -144,9 +144,21 @@ def get_internet_IP(DIG_GET_IP_ARG): # ------------------------------------------------------------------------------- def fallback_check_ip(): - """Fallback mechanism using `curl ifconfig.me/ip`.""" + """Fallback mechanism using curl, forced to IPv4 over HTTPS and bounded in time. + + The result is validated against an IPv4-only pattern below, so the request + has to be IPv4 too: on a dual-stack host, curl left to its own choice can + answer with the public IPv6 address, which then fails validation and makes + the plugin report no internet despite a working connection. The time limits + keep a hung fallback from consuming the whole INTRNT_RUN_TIMEOUT budget, + which the plugin runner enforces as a kill-timeout for the entire script. + """ try: - cmd_output = subprocess.check_output(['curl', '-s', 'ifconfig.me/ip'], text=True).strip() + cmd_output = subprocess.check_output( + ['curl', '-4', '-fsS', '--connect-timeout', '3', '--max-time', '8', + 'https://ifconfig.me/ip'], + text=True, timeout=10 + ).strip() if cmd_output and re.match(r"^\d{1,3}(\.\d{1,3}){3}$", cmd_output): mylog('verbose', [f'[{pluginName}] Fallback IP retrieved via curl: {cmd_output}']) return cmd_output, f'Fallback via curl: "{cmd_output}"' diff --git a/test/plugins/test_internet_ip_fallback.py b/test/plugins/test_internet_ip_fallback.py new file mode 100644 index 000000000..94a62870e --- /dev/null +++ b/test/plugins/test_internet_ip_fallback.py @@ -0,0 +1,130 @@ +""" +Tests for internet_ip/script.py's curl fallback (INTRNT plugin). + +The fallback validates its result against an IPv4-only regex, so it must ask +curl for an IPv4 address. On a dual-stack host without the IPv4 flag, curl can +answer with the host's public IPv6 address, which the regex then rejects, +making the plugin record 0.0.0.0 even though the internet is reachable +(netalertx/NetAlertX#1845). The call is also bounded in time so a hung +fallback cannot burn the plugin's whole RUN_TIMEOUT budget. + +Run from inside the NetAlertX container (where the full environment is +available), or locally - in that case the NetAlertX-specific modules are +stubbed out automatically before the script is imported. + + pytest test/plugins/test_internet_ip_fallback.py -v +""" + +import importlib.util +import os +import sys +import tempfile +import types +from unittest.mock import MagicMock, patch + +# --------------------------------------------------------------------------- +# Stub NetAlertX-specific modules so tests can run outside the container. +# These stubs are only placeholders for the duration of the `import script` +# below - they are popped from sys.modules again right after, so they don't +# leak into other test files sharing the same pytest session (which would +# otherwise shadow the real modules for every subsequent test). +# --------------------------------------------------------------------------- +_tmp_log = tempfile.mkdtemp() + +_stubbed_module_names = [] + + +def _stub(name: str, **attrs): + if name not in sys.modules: + mod = types.ModuleType(name) + for k, v in attrs.items(): + setattr(mod, k, v) + sys.modules[name] = mod + _stubbed_module_names.append(name) + + +_stub("pytz", timezone=lambda tz: tz) +_stub("conf", tz=None) +_stub("const", logPath=_tmp_log) +_stub("plugin_helper", Plugin_Objects=MagicMock) +_stub("utils") +_stub("utils.datetime_utils", timeNowUTC=lambda: "2026-01-01 00:00:00") +_stub("logger", mylog=lambda *a: None, Logger=MagicMock, append_line_to_file=lambda *a: None) +_stub("helper", check_IP_format=lambda v: v, get_setting_value=lambda k, default="": "UTC") + +# Load by explicit path under a unique module name. A bare `import script` +# would collide in sys.modules with whichever other plugin's script.py was +# imported first in the same pytest session - every plugin's entry point is +# named script.py, so the plain name is ambiguous. +_SCRIPT_PATH = os.path.join( + os.path.dirname(__file__), "..", "..", "server", "plugins", "internet_ip", "script.py" +) +_spec = importlib.util.spec_from_file_location("internet_ip_script", _SCRIPT_PATH) +script = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(script) + +fallback_check_ip = script.fallback_check_ip +no_internet_ip = script.no_internet_ip + +for _name in _stubbed_module_names: + sys.modules.pop(_name, None) + + +def _run_fallback(curl_stdout): + """Call fallback_check_ip() with curl's stdout faked, returning + (ip, message, argv-curl-was-called-with).""" + with patch.object(script.subprocess, "check_output", return_value=curl_stdout) as mocked: + ip, message = fallback_check_ip() + return ip, message, mocked.call_args[0][0] + + +class TestForcesIPv4: + """The regex that validates the result accepts only IPv4, so the request + itself has to be IPv4, otherwise a dual-stack host gets a valid IPv6 + answer that is then thrown away (#1845).""" + + def test_curl_is_asked_for_ipv4(self): + _, _, argv = _run_fallback("203.0.113.7\n") + assert "-4" in argv + + def test_ipv4_response_is_accepted(self): + ip, _, _ = _run_fallback("203.0.113.7\n") + assert ip == "203.0.113.7" + + def test_ipv6_response_is_still_rejected(self): + """Forcing IPv4 does not make the validation laxer: if a response is + somehow IPv6 anyway, it must not be stored as the public IP.""" + ip, _, _ = _run_fallback("2001:db8::1\n") + assert ip == no_internet_ip + + +class TestRequestIsBounded: + """INTRNT_RUN_TIMEOUT is the whole subprocess's kill-timeout, enforced by + the plugin runner - an unbounded fallback can burn it and get the process + killed before it writes its result file.""" + + def test_connect_and_total_time_are_capped(self): + _, _, argv = _run_fallback("203.0.113.7\n") + assert "--connect-timeout" in argv + assert "--max-time" in argv + + def test_subprocess_call_passes_an_explicit_timeout(self): + """CLAUDE.md: every subprocess call needs an explicit timeout, not + only the called program's own time limit.""" + with patch.object(script.subprocess, "check_output", return_value="203.0.113.7\n") as mocked: + fallback_check_ip() + assert mocked.call_args.kwargs.get("timeout") is not None + + def test_timeout_is_reported_not_raised(self): + """A timed-out fallback returns the no-internet sentinel rather than + propagating and killing the run.""" + with patch.object(script.subprocess, "check_output", side_effect=Exception("timed out")): + ip, message = fallback_check_ip() + assert ip == no_internet_ip + assert "timed out" in message + + +class TestUsesHttps: + def test_endpoint_is_https(self): + _, _, argv = _run_fallback("203.0.113.7\n") + assert argv[-1].startswith("https://") From e8c214dda9077baaa214a791b6595c91f8466375 Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Sun, 18 Oct 2026 10:25:23 +1100 Subject: [PATCH 02/10] PLG: INTERNET plugin fix #1845 --- server/plugins/internet_ip/script.py | 13 ++++++------- test/plugins/test_internet_ip_fallback.py | 2 ++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/server/plugins/internet_ip/script.py b/server/plugins/internet_ip/script.py index a19595264..39343bf6c 100755 --- a/server/plugins/internet_ip/script.py +++ b/server/plugins/internet_ip/script.py @@ -144,16 +144,15 @@ def get_internet_IP(DIG_GET_IP_ARG): # ------------------------------------------------------------------------------- def fallback_check_ip(): - """Fallback mechanism using curl, forced to IPv4 over HTTPS and bounded in time. + """Retrieve the public IP via curl when the dig lookup found none. - The result is validated against an IPv4-only pattern below, so the request - has to be IPv4 too: on a dual-stack host, curl left to its own choice can - answer with the public IPv6 address, which then fails validation and makes - the plugin report no internet despite a working connection. The time limits - keep a hung fallback from consuming the whole INTRNT_RUN_TIMEOUT budget, - which the plugin runner enforces as a kill-timeout for the entire script. + Returns (IP, cmd_output), or no_internet_ip if the result is not a valid IPv4. """ try: + # -4 is load-bearing: the check below accepts only IPv4, so an + # unforced request on a dual-stack host can return the public IPv6 + # address and be discarded as invalid. Time limits keep a hung call + # from consuming the plugin runner's whole INTRNT_RUN_TIMEOUT budget. cmd_output = subprocess.check_output( ['curl', '-4', '-fsS', '--connect-timeout', '3', '--max-time', '8', 'https://ifconfig.me/ip'], diff --git a/test/plugins/test_internet_ip_fallback.py b/test/plugins/test_internet_ip_fallback.py index 94a62870e..2a2e28b78 100644 --- a/test/plugins/test_internet_ip_fallback.py +++ b/test/plugins/test_internet_ip_fallback.py @@ -35,6 +35,8 @@ _stubbed_module_names = [] def _stub(name: str, **attrs): + """Register a fake module under `name` with `attrs` set on it, unless the + real one is already importable, and record it for later removal.""" if name not in sys.modules: mod = types.ModuleType(name) for k, v in attrs.items(): From 7bb7ce9c8d5055d31d025a27cfd9b3227360caea Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Mon, 19 Oct 2026 08:05:33 +1100 Subject: [PATCH 03/10] DOCS: permissions on Synology --- docs/DOCKER_INSTALLATION.md | 8 +++----- docs/SYNOLOGY_GUIDE.md | 13 +++++++++++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/DOCKER_INSTALLATION.md b/docs/DOCKER_INSTALLATION.md index b3f8d6424..8eba36961 100644 --- a/docs/DOCKER_INSTALLATION.md +++ b/docs/DOCKER_INSTALLATION.md @@ -51,6 +51,9 @@ See alternative [docker-compose examples](https://docs.netalertx.com/DOCKER_COMP ### Docker environment variables +> [!NOTE] +> If you see only 2 devices, you might need to run the container as root by setting a custom `PUID` and `PGID` as some systems don't support `--cap-add` properly, such as some Synology setups. See [running as root](https://docs.netalertx.com/FILE_PERMISSIONS/#running-as-root). + | Variable | Description | Example/Default Value | | :------------- |:------------------------| -----:| | `PUID` |Runtime UID override, set to `0` to run as root. | `20211` | @@ -127,8 +130,3 @@ You can read or watch several [community configuration guides](https://docs.neta | [![GitHub](https://i.imgur.com/emsRCPh.png)](https://github.com/sponsors/jokob-sk) | [![Buy Me A Coffee](https://i.imgur.com/pIM6YXL.png)](https://www.buymeacoffee.com/jokobsk) | | --- | --- | - -- Bitcoin: `1N8tupjeCK12qRVU2XrV17WvKK7LCawyZM` -- Ethereum: `0x6e2749Cb42F4411bc98501406BdcD82244e3f9C7` - -> 📧 Email me at [support@netalertx.com](mailto:support@netalertx.com?subject=NetAlertX Donations) if you want to get in touch or if I should add other sponsorship platforms. diff --git a/docs/SYNOLOGY_GUIDE.md b/docs/SYNOLOGY_GUIDE.md index 66c8e9fdc..87d430703 100755 --- a/docs/SYNOLOGY_GUIDE.md +++ b/docs/SYNOLOGY_GUIDE.md @@ -82,9 +82,18 @@ volumes: 10. Navigate to `:20211` (or your custom port). 11. Read the [Subnets](./SUBNETS.md) and [Plugins](./PLUGINS_OVERVIEW.md) docs to complete your setup. -## Solving permission issues +## Solving permission issues (2 devices visible) - See also the [Permission overview guide](./FILE_PERMISSIONS.md). +Some Synology setups don't support `--cap-add` properly. This usually results in seeing only 2 devices and the following error shows when running `ARPSCAN`: + +```bash +You don't have permission to perform this capture on that device +(socket: Operation not permitted) +``` + +To fix this you might need to set the container as ptiviledged `privileged: true` and run as root by setting a custom `PUID` and `PGID`. + +See [running as root](./FILE_PERMISSIONS.md) for more details. ### Configuring the permissions via SSH From fbb42fbc61f92b2756e447115d3f64325d8cd2b8 Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Mon, 19 Oct 2026 19:10:30 +1100 Subject: [PATCH 04/10] BE: down notifications offset issue #1851 --- server/models/notification_instance.py | 12 +- .../test_notification_down_clear_timezone.py | 170 ++++++++++++++++++ test/integration/integration_test.py | 3 +- 3 files changed, 178 insertions(+), 7 deletions(-) create mode 100644 test/backend/test_notification_down_clear_timezone.py diff --git a/server/models/notification_instance.py b/server/models/notification_instance.py index 0c9fbf5e8..e28caa310 100755 --- a/server/models/notification_instance.py +++ b/server/models/notification_instance.py @@ -19,7 +19,7 @@ from helper import ( getBuildTimeStampAndVersion, ) from messaging.notification_sections import SECTION_ORDER -from utils.datetime_utils import timeNowUTC, timeNowTZ, get_timezone_offset +from utils.datetime_utils import timeNowUTC, timeNowTZ # ----------------------------------------------------------------------------- @@ -249,18 +249,20 @@ class NotificationInstance: WHERE evePendingAlertEmail = 1 AND eveEventType !='Device Down' """) - # Clear down events flag after the reporting window passed + # Clear down events flag after the reporting window passed. eveDateTime + # is always UTC (timeNowUTC()), matching the down_devices selection + # query's own UTC-only cutoff in notification_sections.py - applying a + # timezone offset here would desync the two (#1851). minutes = int(get_setting_value("NTFPRCS_alert_down_time") or 0) - tz_offset = get_timezone_offset() self.db.sql.execute( """ UPDATE Events SET evePendingAlertEmail = 0 WHERE evePendingAlertEmail = 1 AND eveEventType = 'Device Down' - AND eveDateTime < datetime('now', ?, ?) + AND eveDateTime < datetime('now', ?) """, - (f"-{minutes} minutes", tz_offset), + (f"-{minutes} minutes",), ) mylog("minimal", ["[Notification] Notifications changes: ", self.db.sql.rowcount]) diff --git a/test/backend/test_notification_down_clear_timezone.py b/test/backend/test_notification_down_clear_timezone.py new file mode 100644 index 000000000..ae2d657a4 --- /dev/null +++ b/test/backend/test_notification_down_clear_timezone.py @@ -0,0 +1,170 @@ +""" +Tests for NotificationInstance.clearPendingEmailFlag()'s UTC/localtime +correctness (netalertx/NetAlertX#1851). + +eveDateTime is always stored in UTC (utils.datetime_utils.timeNowUTC() is the +single source of truth for DB timestamps). The down_devices selection query +in notification_sections.py compares it against a plain +`datetime('now', '-N minutes')` cutoff, i.e. UTC vs UTC. clearPendingEmailFlag() +must use the same UTC-only cutoff to clear the flag it set - applying an +additional timezone-offset modifier shifts the clear cutoff by the server's +UTC offset, so west-of-UTC zones leave old 'Device Down' events flagged for +hours (repeatedly re-sent, never cleared) and east-of-UTC zones would clear +them too early. + +License: GNU GPLv3 +""" + +import sys +import os +import types +import unittest +from unittest.mock import patch +from zoneinfo import ZoneInfo + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +from db_test_helpers import ( # noqa: E402 + make_db, + insert_device, + minutes_ago, + DummyDB, + CREATE_PLUGINS_EVENTS, +) + +INSTALL_PATH = os.getenv("NETALERTX_APP", "/app") +sys.path.extend([f"{INSTALL_PATH}/server"]) + +import conf # noqa: E402 + +# json2table isn't needed by this test (only finalize_html() uses it) and +# isn't always installed outside the container; stub it for the duration of +# the import below, then drop it so it doesn't leak into other test files. +_json2table_was_stubbed = "json2table" not in sys.modules +if _json2table_was_stubbed: + _stub = types.ModuleType("json2table") + _stub.convert = lambda *a, **k: "" + sys.modules["json2table"] = _stub + +from models.notification_instance import NotificationInstance # noqa: E402 + +if _json2table_was_stubbed: + sys.modules.pop("json2table", None) + + +def _make_db(): + """make_db() plus Plugins_Events, which clearPendingEmailFlag() always + clears at the end via clearPluginEvents().""" + conn = make_db() + conn.execute(CREATE_PLUGINS_EVENTS) + conn.commit() + return conn + + +def _insert_down_event(conn, mac, minutes_old): + """Insert a pending 'Device Down' Events row *minutes_old* minutes ago (UTC).""" + conn.execute( + """ + INSERT INTO Events (eveMac, eveDateTime, eveEventType, evePendingAlertEmail) + VALUES (?, ?, 'Device Down', 1) + """, + (mac, minutes_ago(minutes_old)), + ) + + +def _get_flag(conn, mac): + row = conn.execute( + "SELECT evePendingAlertEmail FROM Events WHERE eveMac=?", (mac,) + ).fetchone() + return row[0] if row else None + + +def _clear_with_tz(conn, alert_down_minutes, tz_name): + """Instantiate NotificationInstance without running its real __init__ + (which needs a live settings DB/Logger) and call clearPendingEmailFlag() + with conf.tz set to tz_name, restoring the previous conf.tz afterward.""" + original_tz = getattr(conf, "tz", None) + conf.tz = ZoneInfo(tz_name) if tz_name else None + try: + ni = NotificationInstance.__new__(NotificationInstance) + ni.db = DummyDB(conn) + with patch( + "models.notification_instance.get_setting_value", + return_value=str(alert_down_minutes), + ): + ni.clearPendingEmailFlag() + finally: + conf.tz = original_tz + + +class TestClearPendingEmailFlagTimezone(unittest.TestCase): + + def test_old_down_event_clears_west_of_utc(self): + """ + Regression test for the UTC/tz-offset bug (#1851). + + 10 minutes old, 5-minute cutoff: the down_devices selection query + would already be resending this event every loop (UTC-only check). + clearPendingEmailFlag() must clear it too. Under the old + tz-offset-applied query, America/Los_Angeles (UTC-7/-8) pushes the + clear cutoff ~7-8 hours further into the past, so a 10-minute-old + event incorrectly stays flagged and keeps getting resent. + """ + conn = _make_db() + mac = "aa:bb:cc:dd:ee:01" + insert_device(conn, mac, alert_down=1) + _insert_down_event(conn, mac, minutes_old=10) + conn.commit() + + _clear_with_tz(conn, alert_down_minutes=5, tz_name="America/Los_Angeles") + + self.assertEqual( + _get_flag(conn, mac), + 0, + "10-minute-old Device Down event with a 5-minute cutoff must be " + "cleared regardless of server timezone.", + ) + + def test_recent_down_event_stays_pending(self): + """A down event younger than the cutoff must stay pending (sanity + check that the fix isn't simply disabling the time filter).""" + conn = _make_db() + mac = "aa:bb:cc:dd:ee:02" + insert_device(conn, mac, alert_down=1) + _insert_down_event(conn, mac, minutes_old=2) + conn.commit() + + _clear_with_tz(conn, alert_down_minutes=5, tz_name="America/Los_Angeles") + + self.assertEqual( + _get_flag(conn, mac), + 1, + "2-minute-old Device Down event with a 5-minute cutoff must stay pending.", + ) + + def test_recent_down_event_does_not_clear_early_east_of_utc(self): + """ + Mirror check east of UTC (#1851 notes the opposite symptom there): + a positive tz offset shifts the clear cutoff *forward* in time + (e.g. Asia/Kolkata's +05:30 pushes 'now - 5 minutes' to roughly + 'now + 5h25m'), which would clear almost any already-pending event + immediately, regardless of whether NTFPRCS_alert_down_time has + actually elapsed. + """ + conn = _make_db() + mac = "aa:bb:cc:dd:ee:03" + insert_device(conn, mac, alert_down=1) + _insert_down_event(conn, mac, minutes_old=1) + conn.commit() + + _clear_with_tz(conn, alert_down_minutes=5, tz_name="Asia/Kolkata") + + self.assertEqual( + _get_flag(conn, mac), + 1, + "1-minute-old Device Down event with a 5-minute cutoff must stay " + "pending regardless of server timezone.", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/integration/integration_test.py b/test/integration/integration_test.py index 818add9d1..d894d2640 100755 --- a/test/integration/integration_test.py +++ b/test/integration/integration_test.py @@ -129,8 +129,7 @@ def test_existing_db_compatibility(): 'NTFPRCS_alert_down_time': '60' }.get(k, '') - with patch('messaging.reporting.get_timezone_offset', return_value='+00:00'): - result = get_notifications(mock_db) + result = get_notifications(mock_db) assert 'new_devices' in result assert 'events' in result From e9747066f1ac293dffcb49fcb07bb7c9288b1fac Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Mon, 19 Oct 2026 19:10:46 +1100 Subject: [PATCH 05/10] BE: down notifications offset issue #1851 --- .../test_notification_down_clear_timezone.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/test/backend/test_notification_down_clear_timezone.py b/test/backend/test_notification_down_clear_timezone.py index ae2d657a4..167bc90f9 100644 --- a/test/backend/test_notification_down_clear_timezone.py +++ b/test/backend/test_notification_down_clear_timezone.py @@ -36,11 +36,18 @@ sys.path.extend([f"{INSTALL_PATH}/server"]) import conf # noqa: E402 -# json2table isn't needed by this test (only finalize_html() uses it) and -# isn't always installed outside the container; stub it for the duration of -# the import below, then drop it so it doesn't leak into other test files. -_json2table_was_stubbed = "json2table" not in sys.modules -if _json2table_was_stubbed: +# json2table isn't needed by this test (only finalize_html() uses it), but +# notification_instance.py imports it at module level; it's a real +# requirements.txt dependency, normally installed. Only stub it, for the +# duration of the import below, when genuinely unavailable (e.g. a sandbox +# without the full requirements installed) - stubbing it unconditionally +# would shadow the real package for every other test file sharing this +# pytest session once models.notification_instance is cached in sys.modules. +try: + import json2table # noqa: F401 + _json2table_was_stubbed = False +except ImportError: + _json2table_was_stubbed = True _stub = types.ModuleType("json2table") _stub.convert = lambda *a, **k: "" sys.modules["json2table"] = _stub From 10eea5603e402994fd65af7cf4dcc69952484f71 Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Mon, 19 Oct 2026 19:11:30 +1100 Subject: [PATCH 06/10] BE: down notifications offset issue #1851 --- server/models/notification_instance.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/models/notification_instance.py b/server/models/notification_instance.py index e28caa310..284cb7c7a 100755 --- a/server/models/notification_instance.py +++ b/server/models/notification_instance.py @@ -232,8 +232,10 @@ class NotificationInstance: self.save() - # Clear the Pending Email flag from all events and devices def clearPendingEmailFlag(self): + """Clear evePendingAlertEmail on reported Events and stamp + devLastNotification on their devices; 'Device Down' events are only + cleared once NTFPRCS_alert_down_time minutes have passed.""" # Clean Pending Alert Events self.db.sql.execute(""" From ddc7cc67beb7760d0c77befad643021370bf5435 Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Fri, 23 Oct 2026 12:10:42 +1100 Subject: [PATCH 07/10] PR cleanup alpine and python bump --- .devcontainer/scripts/setup.sh | 2 +- install/production-filesystem/README.md | 2 +- test/docker_tests/conftest.py | 2 +- test/docker_tests/test_container_environment.py | 8 ++++---- test/docker_tests/test_puid_pgid.py | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.devcontainer/scripts/setup.sh b/.devcontainer/scripts/setup.sh index d06bcf7c0..72b10f30b 100755 --- a/.devcontainer/scripts/setup.sh +++ b/.devcontainer/scripts/setup.sh @@ -15,7 +15,7 @@ SOURCE_DIR=${SOURCE_DIR:-/workspaces/NetAlertX} -PY_SITE_PACKAGES="${VIRTUAL_ENV:-/opt/venv}/lib/python3.12/site-packages" +PY_SITE_PACKAGES="${VIRTUAL_ENV:-/opt/venv}/lib/python3.14/site-packages" LOG_FILES=( LOG_APP diff --git a/install/production-filesystem/README.md b/install/production-filesystem/README.md index 51c6be041..c81a02e86 100755 --- a/install/production-filesystem/README.md +++ b/install/production-filesystem/README.md @@ -29,7 +29,7 @@ Temporary directory used during Docker image building: - Deleted after build to reduce image size - Only exists during container creation -### `/opt/venv/lib/python3.12/site-packages/aiofreebox` - Certificate Storage +### `/opt/venv/lib/python3.14/site-packages/aiofreebox` - Certificate Storage Contains SSL certificates required for secure communication with Freebox OS devices. The aiofreebox Python package uses these certificates to authenticate and establish encrypted connections when integrating with Freebox routers for network device discovery. Contains certificates for the aiofreebox package, which communicates with Freebox OS devices. diff --git a/test/docker_tests/conftest.py b/test/docker_tests/conftest.py index 2643016b6..4a868e726 100644 --- a/test/docker_tests/conftest.py +++ b/test/docker_tests/conftest.py @@ -29,7 +29,7 @@ def _clean_test_mounts(project_root: pathlib.Path) -> None: cmd = [ "docker", "run", "--rm", "-v", f"{project_root}:/work", - "alpine:3.22", + "alpine:3.24", "rm", "-rf", "/work/test_mounts" ] subprocess.run( diff --git a/test/docker_tests/test_container_environment.py b/test/docker_tests/test_container_environment.py index 1ca1eb019..69cb30bfc 100644 --- a/test/docker_tests/test_container_environment.py +++ b/test/docker_tests/test_container_environment.py @@ -388,7 +388,7 @@ def _seed_volume_text_file( [ "-v", f"{volume_name}:/data", - "alpine:3.22", + "alpine:3.24", "sh", "-c", f"set -eu; mkdir -p \"$(dirname '{container_path}')\"; cat > '{container_path}'; chmod {chmod_mode} '{container_path}'", @@ -417,7 +417,7 @@ def _volume_has_file(volume_name: str, container_path: str) -> bool: "host", "-v", f"{volume_name}:/data", - "alpine:3.22", + "alpine:3.24", "sh", "-c", f"test -f '{container_path}'", @@ -943,7 +943,7 @@ def test_missing_app_conf_triggers_seed(tmp_path: pathlib.Path) -> None: check_conf = subprocess.run( [ "docker", "run", "--rm", "-v", f"{vol}:/data", - "alpine:3.22", "cat", "/data/config/app.conf" + "alpine:3.24", "cat", "/data/config/app.conf" ], capture_output=True, text=True, timeout=SUBPROCESS_TIMEOUT_SECONDS ) @@ -984,7 +984,7 @@ def test_first_run_dynamic_subnet(tmp_path: pathlib.Path) -> None: cmd = [ "docker", "run", "--rm", "-v", f"{paths['app_config']}:/mnt", - "alpine:3.22", + "alpine:3.24", "cat", "/mnt/app.conf" ] read_result = subprocess.run(cmd, capture_output=True, text=True, timeout=SUBPROCESS_TIMEOUT_SECONDS) diff --git a/test/docker_tests/test_puid_pgid.py b/test/docker_tests/test_puid_pgid.py index a7df8a2bc..f3d658d77 100644 --- a/test/docker_tests/test_puid_pgid.py +++ b/test/docker_tests/test_puid_pgid.py @@ -180,7 +180,7 @@ def test_synology_like_fresh_volume_is_primed() -> None: f"{volume}:/data", "--entrypoint", "/bin/sh", - "alpine:3.22", + "alpine:3.24", "-c", seed_cmd, ], @@ -214,7 +214,7 @@ def test_synology_like_fresh_volume_is_primed() -> None: f"{volume}:/data", "--entrypoint", "/bin/sh", - "alpine:3.22", + "alpine:3.24", "-c", stat_cmd, ], @@ -357,7 +357,7 @@ def test_missing_cap_chown_fails_priming() -> None: f"{volume}:/data", "--entrypoint", "/bin/sh", - "alpine:3.22", + "alpine:3.24", "-c", seed_cmd, ], From 32eeb327c2dc32ef88c641743f5715da7c0a4dda Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Fri, 23 Oct 2026 12:48:16 +1100 Subject: [PATCH 08/10] NMAPDEV defalts fix #1852 --- server/plugins/nmap_dev_scan/config.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/plugins/nmap_dev_scan/config.json b/server/plugins/nmap_dev_scan/config.json index fed19d32f..6c73411d7 100755 --- a/server/plugins/nmap_dev_scan/config.json +++ b/server/plugins/nmap_dev_scan/config.json @@ -399,7 +399,7 @@ } ] }, - "default_value": "sudo nmap -sn -PR -oX - ", + "default_value": "sudo nmap --privileged -sn -PR -oX - ", "options": [], "localized": [ "name", @@ -414,7 +414,7 @@ "description": [ { "language_code": "en_us", - "string": "Arguments to run nmap-scan with. Recommended and tested only with the setting:
sudo nmap -sn -PR -oX - .

Note: The plugin attaches the interface and network mask, for example -e eth1 192.168.1.0/24 and performs a separate scan for each interface specified in the SCAN_SUBNETS setting." + "string": "Arguments to run nmap-scan with. Recommended and tested only with the setting:
sudo nmap --privileged -sn -PR -oX - .

Note: The plugin attaches the interface and network mask, for example -e eth1 192.168.1.0/24 and performs a separate scan for each interface specified in the SCAN_SUBNETS setting." } ] }, From 03c8cace8ab841a9244fff39469bd6f358be9965 Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Tue, 27 Oct 2026 14:19:55 +1100 Subject: [PATCH 09/10] BE+FE: hardening --- CLAUDE.md | 3 ++ front/php/server/query_json.php | 34 +-------------- front/php/templates/auth.php | 1 + front/php/templates/security.php | 1 + server/workflows/manager.py | 9 +++- test/backend/test_query_json_write_surface.py | 41 +++++++++++++++++++ test/backend/test_workflows.py | 14 +++++++ 7 files changed, 70 insertions(+), 33 deletions(-) create mode 100644 test/backend/test_query_json_write_surface.py diff --git a/CLAUDE.md b/CLAUDE.md index dc2f8f776..ea3793c4c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,3 +98,6 @@ Procedural/how-to knowledge (running tests, resetting the DB, devcontainer manag - **No new PHP backend logic.** `front/` is being migrated away from PHP, so never add a new PHP endpoint or new server-side computation inside an existing PHP file. If a feature needs backend state or computation, add it to the Python server and expose it to the frontend through an existing read path (`app_state.json` via `query_json.php`, `table_settings.json`, a REST/GraphQL endpoint) - never re-derive logic in PHP that the Python side already knows or could easily expose. Editing existing PHP page logic (templating, bug fixes) is fine; this is about not growing the PHP surface area. - **A stateful UI feature (survives a reload, coordinates across tabs, reacts to a backend push) is a cross-cutting mechanism, not "just a UI feature."** Treat it like one before writing code: trace every existing consumer of the data it needs (e.g. everything that already reads `app_state.json`), not just the one file being edited. The visible surface looking small (a badge, an icon) says nothing about whether the state-propagation mechanism underneath already exists elsewhere. - **No JS/PHP test harness exists in `front/` — simulate before asking for a live test, every time, not after a live test fails.** Write a disposable Node/PHP script that runs the actual function(s) against realistic inputs (including inputs crossing from a different file than the one being edited) before calling a change ready to test. A diff review misses things a five-second script run catches - a renamed parameter still referenced by its old name, an ordering assumption that's wrong once two async steps are both in play. +- **Bug and security fixes are test-first: write the failing test, run it and see it fail on the unfixed code, then fix.** Don't edit source and add tests afterward. +- **Each user-editable config file has exactly one writer, and it must be CSRF-protected.** Read-only endpoints stay GET-only; never add a second write path. `requireCsrfToken()` reads `$_POST['csrf_token']`, so a raw-JSON-body endpoint cannot use it as-is. Loaders of user-editable JSON must tolerate a wrong top-level type and non-object entries rather than crash. +- **Every PHP `session_start()` must be preceded by `session_set_cookie_params(['samesite' => 'Lax', 'httponly' => true])`.** A static test checks the known session-starting files, so add any new one to its list. diff --git a/front/php/server/query_json.php b/front/php/server/query_json.php index 97be56c06..dcd722f3d 100755 --- a/front/php/server/query_json.php +++ b/front/php/server/query_json.php @@ -42,38 +42,8 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { http_response_code(400); echo json_encode(["error" => "Missing 'file' parameter"]); } -} elseif ($_SERVER['REQUEST_METHOD'] === 'POST') { - // Read the input JSON data - $inputData = file_get_contents("php://input"); - $decodedData = json_decode($inputData, true); - - if (json_last_error() !== JSON_ERROR_NONE) { - http_response_code(400); - echo json_encode(["error" => "Invalid JSON data"]); - exit; - } - - // Check if file parameter is provided and is workflows.json - if (!isset($_GET['file']) || $_GET['file'] !== "workflows.json") { - http_response_code(400); - echo json_encode(["error" => "Invalid or missing file parameter"]); - exit; - } - - $file = $_GET['file']; - $configRoot = getenv('NETALERTX_CONFIG') ?: '/data/config'; - $filePath = rtrim($configRoot, '/') . "/" . basename($file); - - // Save new workflows.json (replace existing content) - if (file_put_contents($filePath, json_encode($decodedData, JSON_PRETTY_PRINT))) { - http_response_code(200); - echo json_encode(["success" => "Workflows replaced successfully"]); - } else { - http_response_code(500); - echo json_encode(["error" => "Failed to update workflows.json"]); - } -} -else { +} else { + header('Allow: GET'); http_response_code(405); echo json_encode(["error" => "Method Not Allowed"]); } diff --git a/front/php/templates/auth.php b/front/php/templates/auth.php index a31b6412f..7ea317704 100755 --- a/front/php/templates/auth.php +++ b/front/php/templates/auth.php @@ -1,5 +1,6 @@ 'Lax', 'httponly' => true]); session_start(); } diff --git a/front/php/templates/security.php b/front/php/templates/security.php index af30651dc..9b926fa52 100755 --- a/front/php/templates/security.php +++ b/front/php/templates/security.php @@ -2,6 +2,7 @@ // Start session if not already started if (session_status() == PHP_SESSION_NONE) { + session_set_cookie_params(['samesite' => 'Lax', 'httponly' => true]); session_start(); } diff --git a/server/workflows/manager.py b/server/workflows/manager.py index 3c28994e3..c4f1b03a6 100755 --- a/server/workflows/manager.py +++ b/server/workflows/manager.py @@ -52,7 +52,7 @@ class WorkflowManager: # ------------------------------------------------------------------------- # Loading def load_workflows(self): - """Load workflows from workflows.json, rejecting any with invalid tokens.""" + """Load workflows from workflows.json, skipping non-object entries and any with invalid tokens.""" try: workflows_json_path = fullConfFolder + "/workflows.json" with open(workflows_json_path, "r") as f: @@ -61,8 +61,15 @@ class WorkflowManager: mylog("none", ["[WF] Failed to load workflows.json"]) return [] + if not isinstance(raw, list): + mylog("none", ["[WF] workflows.json must contain a list of workflows - ignoring file"]) + return [] + valid = [] for wf in raw: + if not isinstance(wf, dict): + mylog("none", ["[WF] Skipping workflows.json entry that is not an object"]) + continue if self._validate_workflow_tokens(wf): valid.append(wf) else: diff --git a/test/backend/test_query_json_write_surface.py b/test/backend/test_query_json_write_surface.py new file mode 100644 index 000000000..90828a16b --- /dev/null +++ b/test/backend/test_query_json_write_surface.py @@ -0,0 +1,41 @@ +"""Regression guards: workflows.json must only be writable through the CSRF-protected endpoint.""" + +import os +import re + +REPO_ROOT = os.path.join(os.path.dirname(__file__), "..", "..") +QUERY_JSON = os.path.join(REPO_ROOT, "front", "php", "server", "query_json.php") +SESSION_STARTERS = [ + os.path.join(REPO_ROOT, "front", "php", "templates", "security.php"), + os.path.join(REPO_ROOT, "front", "php", "templates", "auth.php"), +] + + +def _read(path): + """Return the text content of a file.""" + with open(path, "r") as f: + return f.read() + + +def test_query_json_has_no_write_path(): + """query_json.php is read-only: no POST handler and no file writes.""" + src = _read(QUERY_JSON) + assert not re.search(r"REQUEST_METHOD'\]\s*===\s*'POST'", src), "POST branch must not exist" + assert "file_put_contents" not in src, "query_json.php must not write files" + assert "php://input" not in src, "query_json.php must not read a request body" + + +def test_query_json_rejects_non_get_methods(): + """Any non-GET method falls through to a 405 response.""" + src = _read(QUERY_JSON) + assert "http_response_code(405)" in src + + +def test_session_cookie_sets_samesite(): + """Every file that starts the PHP session must set a SameSite attribute first.""" + for path in SESSION_STARTERS: + src = _read(path) + start = src.index("session_start()") + params = src.find("session_set_cookie_params") + assert params != -1 and params < start, f"{path}: SameSite must be set before session_start()" + assert re.search(r"'samesite'\s*=>\s*'(Lax|Strict)'", src), f"{path}: missing SameSite Lax/Strict" diff --git a/test/backend/test_workflows.py b/test/backend/test_workflows.py index fba81deb4..69a66a2b3 100644 --- a/test/backend/test_workflows.py +++ b/test/backend/test_workflows.py @@ -171,6 +171,20 @@ class TestLoadWorkflowsRejectsInvalidTokens(unittest.TestCase): mgr = self._make_manager_loading([wf]) self.assertEqual(len(mgr.workflows), 0) + def test_non_list_content_ignored(self): + for raw in (123, "text", {"name": "x"}, None): + mgr = self._make_manager_loading(raw) + self.assertEqual(mgr.workflows, []) + + def test_non_object_entries_skipped(self): + wf = { + "name": "Valid WF", "enabled": "Yes", + "trigger": {"object_type": "Devices", "event_type": "update"}, + "conditions": [], "actions": [] + } + mgr = self._make_manager_loading([1, "x", None, wf]) + self.assertEqual(len(mgr.workflows), 1) + # --------------------------------------------------------------------------- # DeviceInstance.queryByConditions From 96f63ecc547c73cc27c55bd3a087a871379a4302 Mon Sep 17 00:00:00 2001 From: jokob-sk Date: Tue, 27 Oct 2026 15:06:30 +1100 Subject: [PATCH 10/10] BE: review fixes --- server/conf.py | 1 + server/messaging/notification_sections.py | 7 +- server/messaging/reporting.py | 15 ++- server/models/notification_instance.py | 14 +-- .../test_notification_down_clear_timezone.py | 108 +++++++++++++++++- .../test_notification_orphaned_events.py | 2 +- 6 files changed, 130 insertions(+), 17 deletions(-) diff --git a/server/conf.py b/server/conf.py index 4eda3e847..eebd17cc1 100755 --- a/server/conf.py +++ b/server/conf.py @@ -22,6 +22,7 @@ startTime = "" last_scan_run = "" last_version_check = "" arpscan_devices = [] +down_alert_cutoff = "" # ACTUAL CONFIGRATION ITEMS set to defaults diff --git a/server/messaging/notification_sections.py b/server/messaging/notification_sections.py index 14d826811..058700634 100644 --- a/server/messaging/notification_sections.py +++ b/server/messaging/notification_sections.py @@ -41,7 +41,10 @@ DATETIME_FIELDS = { # # Placeholders: # {condition} — optional WHERE clause appended by condition builder -# {alert_down_minutes} — runtime value, only used by down_devices +# {alert_down_cutoff} — UTC 'YYYY-MM-DD HH:MM:SS' cutoff computed once per +# get_notifications() call (also stored in +# conf.down_alert_cutoff for the clear step), only +# used by down_devices # # Every section here reads through Events_Devices, which LEFT JOINs Devices # onto Events - a device's pending event survives deleting the device (Events @@ -85,7 +88,7 @@ SQL_TEMPLATES = { FROM Events_Devices AS down_events WHERE evePendingAlertEmail = 1 AND down_events.eveEventType = 'Device Down' - AND eveDateTime < datetime('now', '-{alert_down_minutes} minutes') + AND eveDateTime < '{alert_down_cutoff}' AND down_events.devMac IS NOT NULL AND NOT EXISTS ( SELECT 1 diff --git a/server/messaging/reporting.py b/server/messaging/reporting.py index cd001b2a7..72eafb04d 100755 --- a/server/messaging/reporting.py +++ b/server/messaging/reporting.py @@ -13,6 +13,7 @@ import os import json import sys +from datetime import timedelta from zoneinfo import ZoneInfo # Register NetAlertX directories @@ -24,7 +25,7 @@ from helper import ( # noqa: E402 [flake8 lint suppression] ) from logger import mylog # noqa: E402 [flake8 lint suppression] from db.sql_safe_builder import create_safe_condition_builder # noqa: E402 [flake8 lint suppression] -from utils.datetime_utils import format_date_iso # noqa: E402 [flake8 lint suppression] +from utils.datetime_utils import format_date_iso, timeNowUTC, DATETIME_PATTERN # noqa: E402 [flake8 lint suppression] from messaging.notification_sections import ( # noqa: E402 [flake8 lint suppression] SECTION_ORDER, SECTION_TITLES, @@ -126,7 +127,13 @@ def get_notifications(db): AND eveMac IN (SELECT devMac FROM Devices WHERE devAlertDown = 0) """) + # Computed once and shared with NotificationInstance.clearPendingEmailFlag() + # via conf, so the clear step acts on exactly the events selected here. alert_down_minutes = int(get_setting_value("NTFPRCS_alert_down_time") or 0) + alert_down_cutoff = ( + timeNowUTC(as_string=False) - timedelta(minutes=alert_down_minutes) + ).strftime(DATETIME_PATTERN) + conf.down_alert_cutoff = alert_down_cutoff sections = get_setting_value("NTFPRCS_INCLUDED_SECTIONS") or [] mylog("verbose", ["[Notification] Included sections: ", sections]) @@ -158,7 +165,7 @@ def get_notifications(db): return "" # SQL templates with placeholders for runtime values - # {condition} and {alert_down_minutes} are formatted at query time + # {condition} and {alert_down_cutoff} are formatted at query time # Initialize final structure final_json = {} @@ -197,7 +204,7 @@ def get_notifications(db): # Format template with runtime placeholders format_vars = {"condition": safe_condition} if section == "down_devices": - format_vars["alert_down_minutes"] = alert_down_minutes + format_vars["alert_down_cutoff"] = alert_down_cutoff if section == "events": # 'Down Reconnected' has its own dedicated section; exclude it # from events when that section is also active to prevent the @@ -212,7 +219,7 @@ def get_notifications(db): mylog("verbose", [f"[Notification] Error building condition for {section}: ", e]) fallback_vars = {"condition": ""} if section == "down_devices": - fallback_vars["alert_down_minutes"] = alert_down_minutes + fallback_vars["alert_down_cutoff"] = alert_down_cutoff if section == "events": if "down_reconnected" in sections: fallback_vars["event_types"] = "'Connected', 'Disconnected','IP Changed'" diff --git a/server/models/notification_instance.py b/server/models/notification_instance.py index 284cb7c7a..32a7fb2ef 100755 --- a/server/models/notification_instance.py +++ b/server/models/notification_instance.py @@ -251,20 +251,20 @@ class NotificationInstance: WHERE evePendingAlertEmail = 1 AND eveEventType !='Device Down' """) - # Clear down events flag after the reporting window passed. eveDateTime - # is always UTC (timeNowUTC()), matching the down_devices selection - # query's own UTC-only cutoff in notification_sections.py - applying a - # timezone offset here would desync the two (#1851). - minutes = int(get_setting_value("NTFPRCS_alert_down_time") or 0) + # Clear down events flag using the exact UTC cutoff the down_devices + # selection query used (conf.down_alert_cutoff, set by + # get_notifications()), so an event that crossed the threshold after + # selection isn't cleared without ever being reported. eveDateTime is + # always UTC - applying a timezone offset here would desync the two (#1851). self.db.sql.execute( """ UPDATE Events SET evePendingAlertEmail = 0 WHERE evePendingAlertEmail = 1 AND eveEventType = 'Device Down' - AND eveDateTime < datetime('now', ?) + AND eveDateTime < ? """, - (f"-{minutes} minutes",), + (conf.down_alert_cutoff,), ) mylog("minimal", ["[Notification] Notifications changes: ", self.db.sql.rowcount]) diff --git a/test/backend/test_notification_down_clear_timezone.py b/test/backend/test_notification_down_clear_timezone.py index 167bc90f9..bb2f8e929 100644 --- a/test/backend/test_notification_down_clear_timezone.py +++ b/test/backend/test_notification_down_clear_timezone.py @@ -19,7 +19,8 @@ import sys import os import types import unittest -from unittest.mock import patch +from datetime import datetime, timedelta, timezone +from unittest.mock import Mock, patch from zoneinfo import ZoneInfo sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) @@ -57,6 +58,9 @@ from models.notification_instance import NotificationInstance # noqa: E402 if _json2table_was_stubbed: sys.modules.pop("json2table", None) +from messaging.reporting import get_notifications # noqa: E402 +from utils.datetime_utils import timeNowUTC, DATETIME_PATTERN # noqa: E402 + def _make_db(): """make_db() plus Plugins_Events, which clearPendingEmailFlag() always @@ -85,12 +89,20 @@ def _get_flag(conn, mac): return row[0] if row else None -def _clear_with_tz(conn, alert_down_minutes, tz_name): +def _clear_with_tz(conn, alert_down_minutes, tz_name, selection_cutoff=None): """Instantiate NotificationInstance without running its real __init__ (which needs a live settings DB/Logger) and call clearPendingEmailFlag() - with conf.tz set to tz_name, restoring the previous conf.tz afterward.""" + with conf.tz set to tz_name and conf.down_alert_cutoff set to + selection_cutoff (the cutoff get_notifications() would have recorded), + restoring both afterward. When selection_cutoff is None the cutoff is + derived from alert_down_minutes, as get_notifications() would have.""" original_tz = getattr(conf, "tz", None) + had_cutoff = hasattr(conf, "down_alert_cutoff") + original_cutoff = getattr(conf, "down_alert_cutoff", None) conf.tz = ZoneInfo(tz_name) if tz_name else None + conf.down_alert_cutoff = ( + selection_cutoff if selection_cutoff is not None else minutes_ago(alert_down_minutes) + ) try: ni = NotificationInstance.__new__(NotificationInstance) ni.db = DummyDB(conn) @@ -101,6 +113,10 @@ def _clear_with_tz(conn, alert_down_minutes, tz_name): ni.clearPendingEmailFlag() finally: conf.tz = original_tz + if had_cutoff: + conf.down_alert_cutoff = original_cutoff + else: + del conf.down_alert_cutoff class TestClearPendingEmailFlagTimezone(unittest.TestCase): @@ -173,5 +189,91 @@ class TestClearPendingEmailFlagTimezone(unittest.TestCase): ) +class TestClearPendingEmailFlagSelectionCutoff(unittest.TestCase): + """The clear step must reuse the cutoff the down_devices selection query + used, not recompute eligibility against the current time: plugins run + between selection and clear, so an event can cross the threshold in that + gap and would otherwise be cleared without ever having been notified.""" + + def test_event_crossing_cutoff_after_selection_stays_pending(self): + """Selection ran with a 5-minute cutoff, when this 4-minute-old event + was not yet eligible. Recomputing from 'now' with a 3-minute window + stands in for the time that elapsed since selection: the event looks + eligible now, but was never selected so must stay pending.""" + conn = _make_db() + mac = "aa:bb:cc:dd:ee:04" + insert_device(conn, mac, alert_down=1) + _insert_down_event(conn, mac, minutes_old=4) + conn.commit() + + _clear_with_tz( + conn, + alert_down_minutes=3, + tz_name=None, + selection_cutoff=minutes_ago(5), + ) + + self.assertEqual( + _get_flag(conn, mac), + 1, + "Event not eligible at selection time must not be cleared.", + ) + + def test_event_selected_before_cutoff_is_cleared(self): + """An event older than the selection-time cutoff was selected for the + alert, so it must be cleared.""" + conn = _make_db() + mac = "aa:bb:cc:dd:ee:05" + insert_device(conn, mac, alert_down=1) + _insert_down_event(conn, mac, minutes_old=10) + conn.commit() + + _clear_with_tz( + conn, + alert_down_minutes=60, + tz_name=None, + selection_cutoff=minutes_ago(5), + ) + + self.assertEqual(_get_flag(conn, mac), 0) + + def test_get_notifications_records_cutoff_used_in_query(self): + """get_notifications() computes the cutoff once, records it on conf for + the clear step and embeds the same value in the down_devices query.""" + mock_db = Mock() + mock_json_obj = Mock() + mock_json_obj.columnNames = [] + mock_json_obj.json = {"data": []} + mock_db.get_table_as_json.return_value = mock_json_obj + + settings = { + "NTFPRCS_INCLUDED_SECTIONS": ["down_devices"], + "NTFPRCS_alert_down_time": "5", + } + had_cutoff = hasattr(conf, "down_alert_cutoff") + original_cutoff = getattr(conf, "down_alert_cutoff", None) + try: + before = timeNowUTC(as_string=False) + with patch( + "messaging.reporting.get_setting_value", + side_effect=lambda key: settings.get(key, ""), + ): + get_notifications(mock_db) + after = timeNowUTC(as_string=False) + + cutoff = conf.down_alert_cutoff + query = mock_db.get_table_as_json.call_args[0][0] + finally: + if had_cutoff: + conf.down_alert_cutoff = original_cutoff + elif hasattr(conf, "down_alert_cutoff"): + del conf.down_alert_cutoff + + self.assertIn(f"'{cutoff}'", query) + cutoff_dt = datetime.strptime(cutoff, DATETIME_PATTERN).replace(tzinfo=timezone.utc) + self.assertGreaterEqual(cutoff_dt, before - timedelta(minutes=5)) + self.assertLessEqual(cutoff_dt, after - timedelta(minutes=5)) + + if __name__ == "__main__": unittest.main() diff --git a/test/backend/test_notification_orphaned_events.py b/test/backend/test_notification_orphaned_events.py index 9cd7916a1..c1aa73f7d 100644 --- a/test/backend/test_notification_orphaned_events.py +++ b/test/backend/test_notification_orphaned_events.py @@ -81,7 +81,7 @@ class TestDownDevicesExcludesOrphanedEvent: _insert_event(conn, EXISTING_MAC, "Device Down", minutes_old=10) _insert_event(conn, DELETED_MAC, "Device Down", minutes_old=10) - rows = _run_template(conn, "down_devices", alert_down_minutes=0) + rows = _run_template(conn, "down_devices", alert_down_cutoff=now_utc()) macs = {r["eveMac"] for r in rows} assert EXISTING_MAC in macs