fix(host): correct schema name in new data modules, sync init.sql, update tests

db/init.sql

@@ -85,5 +85,11 @@ CREATE TABLE IF NOT EXISTS repacss_environment.unknown_devices (
     mac macaddr PRIMARY KEY,
     first_seen timestamptz NOT NULL,
     last_seen timestamptz NOT NULL,
-    message_count_24h int NOT NULL
+    -- Running total since first_seen, not a rolling window: nothing trims it.
+    -- The 24 hour summary the README describes is a query over this plus
+    -- last_seen, not a maintained counter.
+    hit_count int NOT NULL,
+    -- Last payload seen, truncated by the writer. Helps identify what a stray
+    -- device actually is before deciding whether to register or block it.
+    last_payload text
 );

host/app/current_state.py

@@ -27,7 +27,7 @@ def get_recent(conn) -> dict:
                 cur.execute(
                     """
                     SELECT upper(replace(mac::text, ':', '')), running_firmware_version
-                    FROM repacss_enviroment.current_status
+                    FROM repacss_environment.current_status
                     WHERE last_seen > now() - interval '1 minutes'
                     """
                 )

host/app/unknown_devices.py

@@ -9,7 +9,7 @@ def record(conn, mac: str, payload_txt) -> None:
             with conn.cursor() as cur:
                 cur.execute(
                     """
-                    INSERT INTO repacss_enviroment.unknown_devices
+                    INSERT INTO repacss_environment.unknown_devices
                         (mac, first_seen, last_seen, hit_count, last_payload)
                     VALUES (%s, now(), now(), 1, %s)
                     ON CONFLICT (mac) DO UPDATE 

host/tests/test_current_state.py

@@ -0,0 +1,58 @@
+"""Tests for app.current_state (the current_status table).
+
+upsert(conn, mac, fw)
+- Runs the current_status upsert with (mac, fw) parameters.
+- Swallows DB errors (logs, does not raise) so a hello is never lost to a
+  write blip.
+
+get_recent(conn)
+- Maps rows to {mac: {firmware_version}}, with the mac normalised to the
+  compact uppercase form the devices report.
+- Returns {} on a DB error.
+"""
+
+import app.current_state as cs
+
+
+# --- upsert -------------------------------------------------------------------
+
+def test_upsert_runs_upsert(cursor_conn):
+    conn, cur = cursor_conn
+    cs.upsert(conn, "MAC", "v1.0")
+    cur.execute.assert_called_once()
+    sql, params = cur.execute.call_args.args
+    assert "INSERT INTO repacss_environment.current_status" in sql
+    assert "ON CONFLICT (mac) DO UPDATE" in sql
+    assert params == ("MAC", "v1.0")
+
+
+def test_upsert_swallows_error(cursor_conn):
+    conn, cur = cursor_conn
+    cur.execute.side_effect = Exception("boom")
+    cs.upsert(conn, "MAC", "v1.0")  # must not raise
+
+
+# --- get_recent ---------------------------------------------------------------
+
+def test_get_recent_maps_rows(cursor_conn):
+    conn, cur = cursor_conn
+    cur.fetchall.return_value = [("MAC1", "v0.0.2"), ("MAC2", "v0.0.3")]
+    assert cs.get_recent(conn) == {
+        "MAC1": {"firmware_version": "v0.0.2"},
+        "MAC2": {"firmware_version": "v0.0.3"},
+    }
+
+
+def test_get_recent_queries_the_right_schema(cursor_conn):
+    # Guards the repacss_enviroment typo that made this silently return {}.
+    conn, cur = cursor_conn
+    cur.fetchall.return_value = []
+    cs.get_recent(conn)
+    sql = cur.execute.call_args.args[0]
+    assert "repacss_environment.current_status" in sql
+
+
+def test_get_recent_returns_empty_on_error(cursor_conn):
+    conn, cur = cursor_conn
+    cur.execute.side_effect = Exception("db down")
+    assert cs.get_recent(conn) == {}

host/tests/test_provisioning_service.py

@@ -2,30 +2,24 @@
 
 Functions covered and their cases:
 
-is_enabled(mac, role)
-- Primary -> True, Standby -> True, any other role -> False.
-
 build_config(mac, device)
-- Returns the full config dict with configured=True and enabled from is_enabled.
+- Returns the full config dict, with enabled taken straight from the registry
+  row (the old is_enabled role filter is gone: device_map.enabled is the truth).
 - Missing a required device key raises KeyError (surfaces bad registry data).
 
-get_device_state()
-- Maps DB rows to {mac: {firmware_version}}.
-- Returns {} on a DB error.
-
-upsert_device_state(mac, fw)
-- Runs the current_status upsert with (mac, fw) parameters.
-- Swallows DB errors (logs, does not raise) so a hello is never lost to a write blip.
-
-(The device_map cache moved to app.device_registry; see test_device_registry.py.)
-
 on_connect(...)
 - Subscribes to the hello topic.
 
 on_message(...)
-- Known device -> upserts state and publishes a config.
-- Unknown device -> publishes configured:false with reason "unknown mac".
+- Known device -> upserts current_status and publishes a config.
+- Unknown device -> records it in unknown_devices and publishes NOTHING.
+  The silence is intended: the device stays unconfigured and keeps helloing,
+  and the host keeps a record instead of replying.
 - Invalid JSON -> does nothing (no upsert, no publish).
+- Oversized payload -> dropped before parsing.
+
+The per-table SQL lives in app.current_state / app.unknown_devices /
+app.device_registry and is tested in their own files.
 """
 
 import json
@@ -36,24 +30,11 @@
 import app.provisioning_service as prov
 
 
-# --- is_enabled ---------------------------------------------------------------
-
-@pytest.mark.parametrize("role,expected", [
-    ("Primary", True),
-    ("Standby", True),
-    ("Unknown", False),
-    ("controllerA", False),
-    ("", False),
-])
-def test_is_enabled(role, expected):
-    assert prov.is_enabled("AABBCCDDEEFF", role) is expected
-
-
 # --- build_config -------------------------------------------------------------
 
-def test_build_config_primary_full_shape():
+def test_build_config_full_shape():
     # rack_id is free-form text (rack codenames like "rpg93"), not a number.
-    cfg = prov.build_config("MAC", {"rack_id": "rpg93", "role": "Primary"})
+    cfg = prov.build_config("MAC", {"rack_id": "rpg93", "role": "Primary", "enabled": True})
     assert cfg == {
         "message_type": "config",
         "mac": "MAC",
@@ -64,48 +45,16 @@ def test_build_config_primary_full_shape():
     }
 
 
-def test_build_config_missing_key_raises_keyerror():
-    with pytest.raises(KeyError):
-        prov.build_config("MAC", {"role": "Primary"})  # no rack_id
-
-
-# --- get_device_state ---------------------------------------------------------
-
-def test_get_device_state_maps_rows(monkeypatch, cursor_conn):
-    conn, cur = cursor_conn
-    cur.fetchall.return_value = [("MAC1", "v0.0.2"), ("MAC2", "v0.0.3")]
-    monkeypatch.setattr(prov, "conn", conn)
-    assert prov.get_device_state() == {
-        "MAC1": {"firmware_version": "v0.0.2"},
-        "MAC2": {"firmware_version": "v0.0.3"},
-    }
-
-
-def test_get_device_state_returns_empty_on_error(monkeypatch, cursor_conn):
-    conn, cur = cursor_conn
-    cur.execute.side_effect = Exception("db down")
-    monkeypatch.setattr(prov, "conn", conn)
-    assert prov.get_device_state() == {}
+def test_build_config_passes_enabled_false_through():
+    # enabled:false is a valid applied config (remote disable), not a rejection.
+    cfg = prov.build_config("MAC", {"rack_id": "rpg93", "role": "Standby", "enabled": False})
+    assert cfg["enabled"] is False
+    assert cfg["configured"] is True
 
 
-# --- upsert_device_state ------------------------------------------------------
-
-def test_upsert_device_state_runs_upsert(monkeypatch, cursor_conn):
-    conn, cur = cursor_conn
-    monkeypatch.setattr(prov, "conn", conn)
-    prov.upsert_device_state("MAC", "v1.0")
-    cur.execute.assert_called_once()
-    sql, params = cur.execute.call_args.args
-    assert "INSERT INTO repacss_environment.current_status" in sql
-    assert "ON CONFLICT (mac) DO UPDATE" in sql
-    assert params == ("MAC", "v1.0")
-
-
-def test_upsert_device_state_swallows_error(monkeypatch, cursor_conn):
-    conn, cur = cursor_conn
-    cur.execute.side_effect = Exception("boom")
-    monkeypatch.setattr(prov, "conn", conn)
-    prov.upsert_device_state("MAC", "v1.0")  # must not raise
+def test_build_config_missing_key_raises_keyerror():
+    with pytest.raises(KeyError):
+        prov.build_config("MAC", {"role": "Primary", "enabled": True})  # no rack_id
 
 
 # --- on_connect ---------------------------------------------------------------
@@ -119,10 +68,11 @@ def test_on_connect_subscribes_to_hello():
 # --- on_message ---------------------------------------------------------------
 
 def test_on_message_known_device_publishes_config(monkeypatch, make_msg):
-    monkeypatch.setattr(prov, "upsert_device_state", MagicMock())
+    upsert = MagicMock()
+    monkeypatch.setattr(prov.current_state, "upsert", upsert)
     monkeypatch.setattr(
         prov.device_registry, "lookup",
-        MagicMock(return_value={"rack_id": "rpg93", "role": "Primary"}),
+        MagicMock(return_value={"rack_id": "rpg93", "role": "Primary", "enabled": True}),
     )
     client = MagicMock()
     msg = make_msg(
@@ -132,37 +82,54 @@ def test_on_message_known_device_publishes_config(monkeypatch, make_msg):
 
     prov.on_message(client, None, msg)
 
-    prov.upsert_device_state.assert_called_once_with("ECE3347C07D0", "v0.0.2")
+    assert upsert.call_args.args[1:] == ("ECE3347C07D0", "v0.0.2")
     client.publish.assert_called_once()
     topic, payload = client.publish.call_args.args[0], client.publish.call_args.args[1]
     assert topic == "repacss/devices/ECE3347C07D0/config"
     body = json.loads(payload)
     assert body["configured"] is True
     assert body["role"] == "Primary"
+    assert body["rack_id"] == "rpg93"
 
 
-def test_on_message_unknown_device_publishes_not_configured(monkeypatch, make_msg):
-    monkeypatch.setattr(prov, "upsert_device_state", MagicMock())
+def test_on_message_unknown_device_records_and_stays_silent(monkeypatch, make_msg):
+    monkeypatch.setattr(prov.current_state, "upsert", MagicMock())
     monkeypatch.setattr(prov.device_registry, "lookup", MagicMock(return_value=None))
+    record = MagicMock()
+    monkeypatch.setattr(prov.unknown_devices, "record", record)
     client = MagicMock()
-    msg = make_msg(
-        "repacss/devices/AABBCCDDEEFF/hello",
-        {"message_type": "hello", "mac": "AABBCCDDEEFF"},
-    )
+    payload = {"message_type": "hello", "mac": "AABBCCDDEEFF"}
+    msg = make_msg("repacss/devices/AABBCCDDEEFF/hello", payload)
 
     prov.on_message(client, None, msg)
 
-    body = json.loads(client.publish.call_args.args[1])
-    assert body["configured"] is False
-    assert body["reason"] == "unknown mac"
+    # Recorded with the raw payload text, and deliberately NOT answered:
+    # the device keeps helloing rather than being told it is unknown.
+    record.assert_called_once()
+    assert record.call_args.args[1] == "AABBCCDDEEFF"
+    assert json.loads(record.call_args.args[2]) == payload
+    client.publish.assert_not_called()
 
 
 def test_on_message_invalid_json_does_nothing(monkeypatch, make_msg):
-    monkeypatch.setattr(prov, "upsert_device_state", MagicMock())
+    upsert = MagicMock()
+    monkeypatch.setattr(prov.current_state, "upsert", upsert)
     client = MagicMock()
     msg = make_msg("repacss/devices/ECE3347C07D0/hello", "not-json{")
 
     prov.on_message(client, None, msg)
 
     client.publish.assert_not_called()
-    prov.upsert_device_state.assert_not_called()
+    upsert.assert_not_called()
+
+
+def test_on_message_oversized_payload_dropped(monkeypatch, make_msg):
+    upsert = MagicMock()
+    monkeypatch.setattr(prov.current_state, "upsert", upsert)
+    client = MagicMock()
+    msg = make_msg("repacss/devices/ECE3347C07D0/hello", "x" * (prov.MAX_HELLO_BYTES + 1))
+
+    prov.on_message(client, None, msg)
+
+    upsert.assert_not_called()
+    client.publish.assert_not_called()

host/tests/test_unknown_devices.py

@@ -0,0 +1,53 @@
+"""Tests for app.unknown_devices (the unknown_devices table).
+
+record(conn, mac, payload_txt)
+- Upserts one row per mac: inserts on first sight, bumps hit_count and
+  last_seen on repeat.
+- Truncates the stored payload to _MAX_PAYLOAD_CHARS so a chatty or hostile
+  device cannot write unbounded text.
+- Passes mac and payload as query parameters, never interpolated (the mac
+  comes off the MQTT topic and is untrusted).
+- Swallows DB errors (logs, does not raise).
+"""
+
+import app.unknown_devices as ud
+
+
+def test_record_upserts_row(cursor_conn):
+    conn, cur = cursor_conn
+    ud.record(conn, "AABBCCDDEEFF", '{"message_type":"hello"}')
+    cur.execute.assert_called_once()
+    sql, params = cur.execute.call_args.args
+    assert "INSERT INTO repacss_environment.unknown_devices" in sql
+    assert "ON CONFLICT (mac) DO UPDATE" in sql
+    assert params == ("AABBCCDDEEFF", '{"message_type":"hello"}')
+
+
+def test_record_bumps_hit_count_on_conflict(cursor_conn):
+    conn, cur = cursor_conn
+    ud.record(conn, "AABBCCDDEEFF", "{}")
+    sql = cur.execute.call_args.args[0]
+    assert "hit_count = unknown_devices.hit_count + 1" in sql
+
+
+def test_record_truncates_payload(cursor_conn):
+    conn, cur = cursor_conn
+    ud.record(conn, "AABBCCDDEEFF", "x" * (ud._MAX_PAYLOAD_CHARS + 500))
+    _, params = cur.execute.call_args.args
+    assert len(params[1]) == ud._MAX_PAYLOAD_CHARS
+
+
+def test_record_uses_parameters_not_interpolation(cursor_conn):
+    # The mac comes off the MQTT topic and is untrusted, so it must never be
+    # baked into the SQL string.
+    conn, cur = cursor_conn
+    ud.record(conn, "'; DROP TABLE unknown_devices; --", "{}")
+    sql, params = cur.execute.call_args.args
+    assert "DROP TABLE" not in sql
+    assert params[0] == "'; DROP TABLE unknown_devices; --"
+
+
+def test_record_swallows_error(cursor_conn):
+    conn, cur = cursor_conn
+    cur.execute.side_effect = Exception("boom")
+    ud.record(conn, "AABBCCDDEEFF", "{}")  # must not raise