diff --git a/.claude/skills/scan-pipeline/SKILL.md b/.claude/skills/scan-pipeline/SKILL.md index 4d84cfa6..9cbd4a7e 100644 --- a/.claude/skills/scan-pipeline/SKILL.md +++ b/.claude/skills/scan-pipeline/SKILL.md @@ -34,7 +34,7 @@ Covers what happens after a plugin's rows land in `CurrentScan`: presence comput 6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from `CurrentScan` for this cycle (step 2 reads this as "previous" on the *next* cycle). 7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6 for parent devices. 8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override; runs last, wins over everything above. -9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` +9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` — `update_ipv4_ipv6()` does **not** go through `LatestDeviceScan`/`FIELD_SPECS`; it reads `CurrentScan` directly with its own `PARTITION BY scanMac, address_family` ranking, so a device reporting both an IPv4 and an IPv6 row in the same cycle gets both `devPrimaryIPv4`/`devPrimaryIPv6` set from that cycle, not just whichever family happened to win `devLastIP`'s single-value reduction. See `.gemini/internal-docs/PRDs/dual-stack-primary-ip-support.md`. 10. `pair_sessions_events(db)` — pairs `Events` rows as described above. 11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. `Sessions` reflects step 10's pairing from here. 12. `insertOnlineHistory(db)` — dashboard graph rollup. diff --git a/.gemini/skills/scan-pipeline/SKILL.md b/.gemini/skills/scan-pipeline/SKILL.md index 415750fb..0a5be06d 100644 --- a/.gemini/skills/scan-pipeline/SKILL.md +++ b/.gemini/skills/scan-pipeline/SKILL.md @@ -34,7 +34,7 @@ Covers what happens after a plugin's rows land in `CurrentScan`: presence comput 6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from `CurrentScan` for this cycle (step 2 reads this as "previous" on the *next* cycle). 7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6 for parent devices. 8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override; runs last, wins over everything above. -9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` +9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` — `update_ipv4_ipv6()` does **not** go through `LatestDeviceScan`/`FIELD_SPECS`; it reads `CurrentScan` directly with its own `PARTITION BY scanMac, address_family` ranking, so a device reporting both an IPv4 and an IPv6 row in the same cycle gets both `devPrimaryIPv4`/`devPrimaryIPv6` set from that cycle, not just whichever family happened to win `devLastIP`'s single-value reduction. See `.gemini/internal-docs/PRDs/dual-stack-primary-ip-support.md`. 10. `pair_sessions_events(db)` — pairs `Events` rows as described above. 11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. `Sessions` reflects step 10's pairing from here. 12. `insertOnlineHistory(db)` — dashboard graph rollup. diff --git a/.github/skills/scan-pipeline/SKILL.md b/.github/skills/scan-pipeline/SKILL.md index 5c7234b6..89267fc6 100644 --- a/.github/skills/scan-pipeline/SKILL.md +++ b/.github/skills/scan-pipeline/SKILL.md @@ -34,7 +34,7 @@ Covers what happens after a plugin's rows land in `CurrentScan`: presence comput 6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from `CurrentScan` for this cycle (step 2 reads this as "previous" on the *next* cycle). 7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6 for parent devices. 8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override; runs last, wins over everything above. -9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` +9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` — `update_ipv4_ipv6()` does **not** go through `LatestDeviceScan`/`FIELD_SPECS`; it reads `CurrentScan` directly with its own `PARTITION BY scanMac, address_family` ranking, so a device reporting both an IPv4 and an IPv6 row in the same cycle gets both `devPrimaryIPv4`/`devPrimaryIPv6` set from that cycle, not just whichever family happened to win `devLastIP`'s single-value reduction. See `.gemini/internal-docs/PRDs/dual-stack-primary-ip-support.md`. 10. `pair_sessions_events(db)` — pairs `Events` rows as described above. 11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. `Sessions` reflects step 10's pairing from here. 12. `insertOnlineHistory(db)` — dashboard graph rollup. diff --git a/docs/PLUGINS_DEV.md b/docs/PLUGINS_DEV.md index fc635892..eb538686 100755 --- a/docs/PLUGINS_DEV.md +++ b/docs/PLUGINS_DEV.md @@ -243,6 +243,7 @@ Check your plugin against these repo-wide conventions before opening a PR (verif - **Keep `description` strings short.** They render directly in the Settings UI. Put implementation rationale and design trade-offs in the plugin's README or code comments, not the UI-facing description. - **For "one or more instances of the same thing," use the nested array + popup-form settings pattern**, not a fixed hardcoded count (e.g. "primary"/"secondary"). See `rest_import` (`RSTIMPRT`)'s `imports` setting for a working example — it also gives each instance its own sub-settings (URL, credentials, per-instance flags) for free. - **Persist plugin state under `dbFolderPath`, config artifacts under `configPath`** — see [Persisting Plugin Data](#persisting-plugin-data-state--config-files) below. +- **A plugin mapped to `mapped_to_table: "CurrentScan"` must also map `scanSourcePlugin`** (a static value via `mapped_to_column_data`, see [Static Value Mapping](#static-value-mapping) below) — not mechanically enforced by `test_plugin_conventions.py`, so review it by eye. Omitting it leaves `scanSourcePlugin` `NULL` on every row this plugin inserts, which silently breaks two things in `server/scan/device_handling.py`: `create_new_devices()`'s `plugin_prefix = str(scanSourcePlugin).strip() if scanSourcePlugin else "NEWDEV"` mislabels devices this plugin creates as source `NEWDEV`; and `update_devices_data_from_scan()`'s `SELECT DISTINCT scanSourcePlugin FROM CurrentScan` + `[row[0] for row in plugin_rows if row[0]] or [None]` drops the `NULL` rows entirely (the `or [None]` fallback never triggers once any other plugin contributes a non-null prefix), so this plugin's `CurrentScan` rows never get picked up by the per-plugin device-update loop at all — the plugin can *insert* into `CurrentScan` but never actually confirm/update a device's presence. - **A setting's `dataType` and `default_value` must actually agree.** `dataType: "array"` (or `"object"`) means `default_value` must be a real JSON literal for that shape — `'["default"]'`, not the bare string `"default"`. `setting_value_to_python_type()` (`server/helper.py`) `json.loads()`s the default at runtime; a bare string fails that parse, silently logs a decode error, and returns `[]` instead of your intended default — this shipped for real in `devParentRelType`/`UI_theme`/`UI_TOPOLOGY_ORDER` before being caught. If `elementOptions` already sets `multiple`/`orderable: "false"`, that's a strong signal the setting is actually scalar and `dataType` should be `"string"`, not `"array"`, regardless of what UI widget (`select`, etc.) renders it. --- @@ -310,6 +311,8 @@ To always map a static value (not read from plugin output): } ``` +Every `mapped_to_table: "CurrentScan"` plugin needs this `scanSourcePlugin` mapping — see the Conventions Checklist above for what breaks downstream if it's left out. + ### Import Behavior Columns (`scanCreatesDevice`, `scanNotificationMode`, `scanPresence`) Three optional columns on `CurrentScan` control what happens once a row reaches it — see [Plugin Import Behavior](PLUGINS_IMPORT_BEHAVIOR.md) 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. diff --git a/front/js/common.js b/front/js/common.js index b555fa3a..135b0b36 100755 --- a/front/js/common.js +++ b/front/js/common.js @@ -795,36 +795,6 @@ function isValidIPv4(ip) { return ipv4Regex.test(ip); } - -function formatIPlong(ipAddress) { - if (ipAddress.includes(':') && isValidIPv6(ipAddress)) { - const parts = ipAddress.split(':'); - - return parts.reduce((acc, part, index) => { - if (part === '') { - const remainingGroups = 8 - parts.length + 1; - return acc << (16 * remainingGroups); - } - - const hexValue = parseInt(part, 16); - return acc | (hexValue << (112 - index * 16)); - }, 0); - } else { - // Handle IPv4 address - const parts = ipAddress.split('.'); - - if (parts.length !== 4) { - console.log("⚠ Invalid IPv4 address: " + ipAddress); - return -1; // or any other default value indicating an error - } - - return (parseInt(parts[0]) << 24) | - (parseInt(parts[1]) << 16) | - (parseInt(parts[2]) << 8) | - parseInt(parts[3]); - } -} - // ----------------------------------------------------------------------------- // Check if MAC is a random one function isRandomMAC(mac) diff --git a/install/production-filesystem/entrypoint.sh b/install/production-filesystem/entrypoint.sh index 927d4dc8..c7af2617 100755 --- a/install/production-filesystem/entrypoint.sh +++ b/install/production-filesystem/entrypoint.sh @@ -77,6 +77,17 @@ echo ' Network intruder and presence detector. ' set -u + +# Set APP_CONF_OVERRIDE based on GRAPHQL_PORT if not already set. +# Must run before the entrypoint.d loop below - 35-apply-conf-override.sh +# (which writes APP_CONF_OVERRIDE to app_conf_override.json for the Python +# app to read) lives in that loop, so deriving APP_CONF_OVERRIDE afterwards +# means it never gets picked up on a fresh volume. +if [ -n "${GRAPHQL_PORT:-}" ] && [ -z "${APP_CONF_OVERRIDE:-}" ]; then + export APP_CONF_OVERRIDE='{"GRAPHQL_PORT":"'"${GRAPHQL_PORT}"'"}' + >&2 echo "APP_CONF_OVERRIDE detected (set from GRAPHQL_PORT)" +fi + FAILED_STATUS="" echo "Startup pre-checks" for script in "${ENTRYPOINT_CHECKS}"/*; do @@ -127,13 +138,6 @@ if [ -n "${FAILED_STATUS}" ]; then fi fi -# Set APP_CONF_OVERRIDE based on GRAPHQL_PORT if not already set -if [ -n "${GRAPHQL_PORT:-}" ] && [ -z "${APP_CONF_OVERRIDE:-}" ]; then - export APP_CONF_OVERRIDE='{"GRAPHQL_PORT":"'"${GRAPHQL_PORT}"'"}' - >&2 echo "APP_CONF_OVERRIDE detected (set from GRAPHQL_PORT)" -fi - - # Exit after checks if in check-only mode (for testing) if [ "${NETALERTX_CHECK_ONLY:-0}" -eq 1 ]; then exit 0 diff --git a/server/api_server/graphql_endpoint.py b/server/api_server/graphql_endpoint.py index 59663ff8..61e65135 100755 --- a/server/api_server/graphql_endpoint.py +++ b/server/api_server/graphql_endpoint.py @@ -39,6 +39,26 @@ from models.device_history_instance import DevicesHistoryInstance # noqa: E402 folder = apiPath +# Device fields holding a dotted/colon IP address string — sorted numerically (via +# format_ip_long), not lexicographically, so e.g. 192.168.1.15 sorts before 192.168.1.105. +# devIpLong is excluded: it already holds the pre-computed long value as a decimal +# string, which mixed_type_sort_key sorts numerically on its own. +_IP_SORT_FIELDS = {"devLastIP", "devPrimaryIPv4", "devPrimaryIPv6"} + + +def _ip_sort_key(value): + """Sort key for an IP-address field: (0, long_value) for a real address, + (1, 0) for empty/unparseable - mirrors mixed_type_sort_key's bucketing + (valid values sort before the empty bucket in ascending order, and Python's + sorted(reverse=True) flips both together) instead of relying on + format_ip_long's -1 sentinel, which put blanks *before* real addresses in + ascending order - the opposite of every other column's convention.""" + long_value = format_ip_long(value) if value else -1 + if long_value < 0: + return (1, 0) + return (0, long_value) + + # In-memory cache for lang strings _langstrings_cache = {} # caches lists per file (core JSON or plugin) _langstrings_cache_mtime = {} # tracks last modified times @@ -392,15 +412,24 @@ class Query(ObjectType): # sorting if options.sort: for sort_option in options.sort: - devices_data = sorted( - devices_data, - key=lambda x: mixed_type_sort_key( - x.get(sort_option.field).lower() - if isinstance(x.get(sort_option.field), str) - else x.get(sort_option.field) - ), - reverse=(sort_option.order.lower() == "desc"), - ) + field = sort_option.field + if field in _IP_SORT_FIELDS: + # Numeric sort so e.g. 192.168.1.15 sorts before 192.168.1.105 + devices_data = sorted( + devices_data, + key=lambda x: _ip_sort_key(x.get(field)), + reverse=(sort_option.order.lower() == "desc"), + ) + else: + devices_data = sorted( + devices_data, + key=lambda x: mixed_type_sort_key( + x.get(field).lower() + if isinstance(x.get(field), str) + else x.get(field) + ), + reverse=(sort_option.order.lower() == "desc"), + ) # capture total count after all the filtering and searching, BEFORE pagination total_count = len(devices_data) diff --git a/server/plugins/freebox/config.json b/server/plugins/freebox/config.json index 53d8c139..f605defa 100755 --- a/server/plugins/freebox/config.json +++ b/server/plugins/freebox/config.json @@ -484,8 +484,9 @@ }, { "column": "watchedValue4", + "mapped_to_column": "scanLastConnection", "css_classes": "col-sm-2", - "show": false, + "show": true, "type": "label", "default_value": "", "options": [], @@ -495,7 +496,7 @@ "name": [ { "language_code": "en_us", - "string": "N/A" + "string": "Last Connection" } ] }, diff --git a/server/plugins/freebox/freebox.py b/server/plugins/freebox/freebox.py index 7fb10c7e..ab60a60a 100755 --- a/server/plugins/freebox/freebox.py +++ b/server/plugins/freebox/freebox.py @@ -4,7 +4,7 @@ import os import sys from pytz import timezone import asyncio -from datetime import datetime +from datetime import datetime, timezone as dt_timezone from pathlib import Path from typing import cast import socket @@ -170,7 +170,11 @@ def main(): watched1=host.get("primary_name", "(unknown)"), watched2=host.get("vendor_name", "(unknown)"), watched3=map_device_type(host.get("host_type", "")), - watched4=datetime.fromtimestamp(ip.get("last_time_reachable", 0)).strftime(DATETIME_PATTERN), + # .get(..., 0) alone isn't enough: the Freebox API can return this + # key present but explicitly null, and dict.get()'s default only + # applies when the key is absent, not when its value is None - + # `or 0` catches both, avoiding a TypeError from fromtimestamp(None). + watched4=datetime.fromtimestamp(ip.get("last_time_reachable") or 0, tz=dt_timezone.utc).strftime(DATETIME_PATTERN), extra="", foreignKey=mac, ) diff --git a/server/scan/device_handling.py b/server/scan/device_handling.py index cbaaf97a..4e6bec71 100755 --- a/server/scan/device_handling.py +++ b/server/scan/device_handling.py @@ -348,39 +348,59 @@ def update_devices_data_from_scan(db): def update_ipv4_ipv6(db): """ - Fill devPrimaryIPv4 and devPrimaryIPv6 based on devLastIP. - Skips empty devLastIP and preserves existing values for the other version. + Fill devPrimaryIPv4 and devPrimaryIPv6 from CurrentScan directly, ranking each + scanMac's rows independently per address family (not per plugin, not via the + single already-reduced devLastIP) so a device reporting both families in the + same scan cycle - dual-stack, the normal case on any modern network, not an + edge case - gets both fields populated from that one cycle instead of only + whichever family happened to win devLastIP's single-value reduction. + Skips empty/presence-suppressed rows and preserves existing values for a + family not refreshed this cycle. See .gemini/internal-docs/PRDs/dual-stack-primary-ip-support.md. """ sql = db.sql - mylog("debug", "[Update Devices] Updating devPrimaryIPv4 / devPrimaryIPv6 from devLastIP") + mylog("debug", "[Update Devices] Updating devPrimaryIPv4 / devPrimaryIPv6 from CurrentScan") - devices = sql.execute("SELECT devMac, devLastIP FROM Devices").fetchall() - records_to_update = [] + rows = sql.execute(f""" + WITH ranked AS ( + SELECT + scanMac, + scanLastIP, + CASE WHEN instr(scanLastIP, ':') > 0 THEN 'v6' ELSE 'v4' END AS family, + ROW_NUMBER() OVER ( + PARTITION BY scanMac, CASE WHEN instr(scanLastIP, ':') > 0 THEN 'v6' ELSE 'v4' END + ORDER BY scanLastConnection DESC + ) AS family_rn + FROM CurrentScan + WHERE scanLastIP NOT IN ({NULL_EQUIVALENTS_SQL}) + AND scanMac NOT IN ({NULL_EQUIVALENTS_SQL}) + AND scanPresence = 1 + ) + SELECT scanMac, family, scanLastIP FROM ranked ORDER BY scanMac, family, family_rn + """).fetchall() - for device in devices: - last_ip = device["devLastIP"] - # Keeping your specific skip logic - if not last_ip or last_ip.lower() in ("", "null", "(unknown)", "(Unknown)"): + per_mac = {} + for row in rows: + mac, family, ip = row["scanMac"], row["family"], row["scanLastIP"] + if family in per_mac.get(mac, {}): + # Already have a valid candidate for this (mac, family) from a more- + # recent row (rows arrive ordered by family_rn) - a malformed *newer* + # scanLastIP (passes the SQL-side ':' family heuristic but fails real + # IP parsing below) must not block an older, valid one for the same + # mac/family from being used instead. continue - - ipv4, ipv6 = None, None try: - ip_obj = ipaddress.ip_address(last_ip) - if ip_obj.version == 4: - ipv4 = last_ip - else: - ipv6 = last_ip + ipaddress.ip_address(ip) # defensive re-validation of the SQL-side family classification except ValueError: continue + per_mac.setdefault(mac, {})[family] = ip - records_to_update.append((ipv4, ipv6, device["devMac"])) + records_to_update = [ + (v.get("v4"), v.get("v6"), mac) for mac, v in per_mac.items() + ] if records_to_update: # We use COALESCE(?, Column) so that if the first arg is NULL, # it keeps the current value of the column. - - # mylog("none", f"[Update Devices] Updated records_to_update: {records_to_update}") - sql.executemany( """ UPDATE Devices diff --git a/test/api_endpoints/test_graphq_endpoints.py b/test/api_endpoints/test_graphq_endpoints.py index 2f2f5519..b6a2d1da 100644 --- a/test/api_endpoints/test_graphq_endpoints.py +++ b/test/api_endpoints/test_graphq_endpoints.py @@ -5,7 +5,7 @@ import pytest INSTALL_PATH = "/app" sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"]) -from helper import get_setting_value, count_children_by_parent_mac # noqa: E402 [flake8 lint suppression] +from helper import get_setting_value, count_children_by_parent_mac, format_ip_long # noqa: E402 [flake8 lint suppression] from api_server.api_server_start import app # noqa: E402 [flake8 lint suppression] @@ -120,6 +120,50 @@ def test_graphql_devices_parent_children_count_matches_recount(client, api_token ) +@pytest.mark.parametrize("field", ["devLastIP", "devPrimaryIPv4", "devPrimaryIPv6"]) +@pytest.mark.parametrize("order", ["ASC", "DESC"]) +def test_graphql_devices_sort_by_ip_is_numeric(client, api_token, field, order): + """Sorting devices by an IP field must order numerically (e.g. 192.168.1.15 + before 192.168.1.105), not lexicographically, in both directions - and a + device with no value for that field must consistently land after every + real address in ASC and before every real address in DESC (matching + mixed_type_sort_key's convention for every other column), not wherever + format_ip_long's -1-for-empty sentinel happens to fall in raw numeric + order. Regression guard for #1797. + + Not seeded: reuses whatever devices already exist (see the docstring on + test_graphql_devices_parent_children_count_matches_recount for why this + file avoids create-then-query against table_devices.json). + """ + query = { + "query": f""" + {{ + devices(options: {{sort: [{{field: "{field}", order: "{order}"}}]}}) {{ + devices {{ + {field} + }} + }} + }} + """ + } + resp = client.post("/graphql", json=query, headers=auth_headers(api_token)) + assert resp.status_code == 200 + + values = [d[field] for d in resp.get_json()["data"]["devices"]["devices"]] + + # Blank-value placement, independent of the non-blank values' numeric order: + # False (non-blank) sorts before True (blank), so a properly-grouped ASC + # result is already in sorted(is_blank) order; DESC is the reverse of that. + is_blank = [not v for v in values] + expected_blank_order = sorted(is_blank, reverse=(order == "DESC")) + assert is_blank == expected_blank_order, f"blank {field} values not grouped correctly for {order}: {values}" + + # Non-blank values must be in real numeric IP order for the requested direction. + non_blank_longs = [format_ip_long(v) for v in values if v] + expected = sorted(non_blank_longs, reverse=(order == "DESC")) + assert non_blank_longs == expected + + # --- SETTINGS TESTS --- def test_graphql_post_settings(client, api_token): """POST /graphql should return settings data""" diff --git a/test/docker_tests/test_entrypoint.py b/test/docker_tests/test_entrypoint.py index 94a29a8a..8092f8f2 100644 --- a/test/docker_tests/test_entrypoint.py +++ b/test/docker_tests/test_entrypoint.py @@ -5,6 +5,8 @@ These tests verify the behavior of the entrypoint script under various condition such as environment variable settings and check skipping. ''' +import json +import os import subprocess import uuid import pytest @@ -59,6 +61,55 @@ def test_app_conf_override_from_graphql_port(): assert result.returncode == 0 +@pytest.mark.docker +@pytest.mark.feature_complete +def test_app_conf_override_file_written_from_graphql_port(tmp_path): + # Regression test: a bare GRAPHQL_PORT (no APP_CONF_OVERRIDE set) must result in + # app_conf_override.json actually being written by entrypoint.d/35-apply-conf-override.sh, + # which server/initialise.py reads to override the persisted GRAPHQL_PORT setting. + # This requires the GRAPHQL_PORT->APP_CONF_OVERRIDE derivation in entrypoint.sh to run + # BEFORE the entrypoint.d loop - it previously ran after, so on a fresh volume the + # derived value never reached the file and the setting silently stayed at its default. + config_dir = tmp_path / "config" + config_dir.mkdir() + os.chmod(config_dir, 0o777) + + name = f"netalertx-test-entrypoint-{uuid.uuid4().hex[:8]}".lower() + cmd = [ + "docker", "run", "--rm", "--name", name, + "--network", "host", "--userns", "host", + "--tmpfs", "/tmp:mode=777", + "--cap-add", "CHOWN", + "--cap-add", "NET_RAW", "--cap-add", "NET_ADMIN", "--cap-add", "NET_BIND_SERVICE", + "-v", f"{config_dir}:/data/config", + "-e", "GRAPHQL_PORT=20322", + "-e", "NETALERTX_DEBUG=1", + "-e", "NETALERTX_CHECK_ONLY=1", + "--entrypoint", "/bin/sh", IMAGE, "-c", + "sh /root-entrypoint.sh" + ] + try: + result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) + + override_file = config_dir / "app_conf_override.json" + assert override_file.exists(), ( + "app_conf_override.json was never written by entrypoint.d/35-apply-conf-override.sh - " + "the GRAPHQL_PORT->APP_CONF_OVERRIDE derivation in entrypoint.sh must run before the " + f"entrypoint.d loop.\nstdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + assert json.loads(override_file.read_text()) == {"GRAPHQL_PORT": "20322"} + finally: + # The container writes app_conf_override.json as its own runtime UID, which the + # host test process doesn't own - reopen permissions via a throwaway container so + # pytest's tmp_path fixture teardown can actually delete config_dir afterwards. + subprocess.run( + ["docker", "run", "--rm", "--entrypoint", "chmod", + "-v", f"{config_dir}:/data/config", IMAGE, + "-R", "a+rwX", "/data/config"], + capture_output=True, text=True, timeout=30, + ) + + @pytest.mark.docker @pytest.mark.feature_complete def test_app_conf_override_not_overridden(): diff --git a/test/scan/test_ip_format_and_locking.py b/test/scan/test_ip_format_and_locking.py index d9989a31..d136e8e2 100644 --- a/test/scan/test_ip_format_and_locking.py +++ b/test/scan/test_ip_format_and_locking.py @@ -169,3 +169,152 @@ def test_ipv6_address_format_variations(scan_db, mock_ip_handlers): row = cur.execute("SELECT devPrimaryIPv6 FROM Devices WHERE devLastIP = ?", (ipv6,)).fetchone() assert row is not None + +# --- Dual-stack same-cycle tests (regression for GitHub #1804) --- +# +# The tests above all cover a single address family per scan cycle - either +# one row per mac, or two *separate* cycles (CurrentScan cleared between +# them). None of them reproduce #1804: a device reporting both an IPv4 and an +# IPv6 row for the same mac in the *same* cycle. See +# .gemini/internal-docs/PRDs/dual-stack-primary-ip-support.md. + + +def test_dual_stack_same_cycle_sets_both_primary_ips(scan_db, mock_ip_handlers): + """A single scan cycle reporting both IPv4 and IPv6 for one MAC, from the + same plugin, must set both devPrimaryIPv4 and devPrimaryIPv6 from that one + cycle - regression test for #1804.""" + cur = scan_db.cursor() + cur.execute("INSERT INTO Devices (devMac) VALUES (?)", ("cc:cc:cc:cc:cc:01",)) + cur.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP, scanSourcePlugin, scanLastConnection) VALUES (?, ?, ?, ?)", + ("cc:cc:cc:cc:cc:01", "192.168.1.50", "FREEBOX", "2025-01-01 01:00:00") + ) + cur.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP, scanSourcePlugin, scanLastConnection) VALUES (?, ?, ?, ?)", + ("cc:cc:cc:cc:cc:01", "fe80::abcd", "FREEBOX", "2025-01-01 01:00:01") + ) + scan_db.commit() + + db = Mock(sql_connection=scan_db, sql=cur) + device_handling.update_devices_data_from_scan(db) + device_handling.update_ipv4_ipv6(db) + + row = cur.execute( + "SELECT devPrimaryIPv4, devPrimaryIPv6 FROM Devices WHERE devMac = ?", + ("cc:cc:cc:cc:cc:01",), + ).fetchone() + assert row["devPrimaryIPv4"] == "192.168.1.50" + assert row["devPrimaryIPv6"] == "fe80::abcd" + + +def test_dual_stack_two_plugins_same_cycle_sets_both(scan_db, mock_ip_handlers): + """Same as above, but the IPv4 row and the IPv6 row come from two + different plugins - confirms the per-family ranking is mac-wide, not + scoped to one plugin's own rows.""" + cur = scan_db.cursor() + cur.execute("INSERT INTO Devices (devMac) VALUES (?)", ("cc:cc:cc:cc:cc:02",)) + cur.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP, scanSourcePlugin, scanLastConnection) VALUES (?, ?, ?, ?)", + ("cc:cc:cc:cc:cc:02", "192.168.1.60", "ARPSCAN", "2025-01-01 01:00:00") + ) + cur.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP, scanSourcePlugin, scanLastConnection) VALUES (?, ?, ?, ?)", + ("cc:cc:cc:cc:cc:02", "fe80::beef", "FREEBOX", "2025-01-01 01:00:00") + ) + scan_db.commit() + + db = Mock(sql_connection=scan_db, sql=cur) + device_handling.update_devices_data_from_scan(db) + device_handling.update_ipv4_ipv6(db) + + row = cur.execute( + "SELECT devPrimaryIPv4, devPrimaryIPv6 FROM Devices WHERE devMac = ?", + ("cc:cc:cc:cc:cc:02",), + ).fetchone() + assert row["devPrimaryIPv4"] == "192.168.1.60" + assert row["devPrimaryIPv6"] == "fe80::beef" + + +def test_dual_stack_presence_suppressed_row_excluded(scan_db, mock_ip_handlers): + """A scanPresence=0 row must not win a device's primary address for that + family, same as it doesn't count as a live sighting elsewhere in the scan + pipeline.""" + cur = scan_db.cursor() + cur.execute("INSERT INTO Devices (devMac) VALUES (?)", ("cc:cc:cc:cc:cc:03",)) + cur.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP, scanSourcePlugin, scanLastConnection, scanPresence) VALUES (?, ?, ?, ?, ?)", + ("cc:cc:cc:cc:cc:03", "192.168.1.70", "ARPSCAN", "2025-01-01 01:00:00", 1) + ) + cur.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP, scanSourcePlugin, scanLastConnection, scanPresence) VALUES (?, ?, ?, ?, ?)", + ("cc:cc:cc:cc:cc:03", "fe80::dead", "SOMEPLG", "2025-01-01 01:00:00", 0) + ) + scan_db.commit() + + db = Mock(sql_connection=scan_db, sql=cur) + device_handling.update_devices_data_from_scan(db) + device_handling.update_ipv4_ipv6(db) + + row = cur.execute( + "SELECT devPrimaryIPv4, devPrimaryIPv6 FROM Devices WHERE devMac = ?", + ("cc:cc:cc:cc:cc:03",), + ).fetchone() + assert row["devPrimaryIPv4"] == "192.168.1.70" + assert row["devPrimaryIPv6"] in (None, "") + + +def test_dual_stack_blank_scan_mac_row_is_inert(scan_db, mock_ip_handlers): + """A CurrentScan row with a blank scanMac must never update any Devices + row, even if it has an otherwise-valid scanLastIP - matches the same + blank-scanMac guard create_new_devices() already applies (see the + scan-pipeline skill's Gotcha 5).""" + cur = scan_db.cursor() + cur.execute("INSERT INTO Devices (devMac, devPrimaryIPv4) VALUES (?, ?)", ("cc:cc:cc:cc:cc:07", "203.0.113.1")) + cur.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP, scanSourcePlugin, scanLastConnection) VALUES (?, ?, ?, ?)", + ("", "192.168.1.99", "DOCKERDISC", "2025-01-01 03:00:00") + ) + scan_db.commit() + + db = Mock(sql_connection=scan_db, sql=cur) + device_handling.update_devices_data_from_scan(db) + device_handling.update_ipv4_ipv6(db) + + # The blank-scanMac row must not have created/updated any Devices row - + # in particular, it must not have overwritten the unrelated real device. + row = cur.execute( + "SELECT devPrimaryIPv4 FROM Devices WHERE devMac = ?", + ("cc:cc:cc:cc:cc:07",), + ).fetchone() + assert row["devPrimaryIPv4"] == "203.0.113.1" + + blank_mac_row = cur.execute("SELECT devMac FROM Devices WHERE devMac = ''").fetchone() + assert blank_mac_row is None + + +def test_dual_stack_malformed_newer_row_falls_back_to_valid_older_row(scan_db, mock_ip_handlers): + """When the most-recent CurrentScan row for a (mac, family) has a + malformed scanLastIP (passes the SQL-side ':' family heuristic but fails + real IP validation), an older but valid row for the same mac/family must + still be used instead of the family being dropped entirely this cycle.""" + cur = scan_db.cursor() + cur.execute("INSERT INTO Devices (devMac) VALUES (?)", ("cc:cc:cc:cc:cc:08",)) + cur.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP, scanSourcePlugin, scanLastConnection) VALUES (?, ?, ?, ?)", + ("cc:cc:cc:cc:cc:08", "999.999.999.999", "BUGGYPLG", "2025-01-01 05:00:00") + ) + cur.execute( + "INSERT INTO CurrentScan (scanMac, scanLastIP, scanSourcePlugin, scanLastConnection) VALUES (?, ?, ?, ?)", + ("cc:cc:cc:cc:cc:08", "172.16.0.5", "ARPSCAN", "2025-01-01 04:00:00") + ) + scan_db.commit() + + db = Mock(sql_connection=scan_db, sql=cur) + device_handling.update_devices_data_from_scan(db) + device_handling.update_ipv4_ipv6(db) + + row = cur.execute( + "SELECT devPrimaryIPv4 FROM Devices WHERE devMac = ?", + ("cc:cc:cc:cc:cc:08",), + ).fetchone() + assert row["devPrimaryIPv4"] == "172.16.0.5"