Skip to content

Commit 8e22c72

Browse files
reloadfastCopilot
andauthored
feat: MAC-stable device identity + label as primary display name (#115)
* feat: MAC-stable device identity + label as primary display name Backend: - upsert_device() now matches by MAC first; if MAC seen at a new IP, updates ip_address while preserving label/trusted/device_type - Adds ix_devices_mac_address index (model + idempotent migration) - RiskOut gains label: str | None populated from r.device.label - device_appeared event detail now includes 'label' (parity with device_disappeared which already had it) - 3 new tests: create new, MAC-first IP update, no-MAC IP fallback Frontend: - Risk interface gains label: string | null - RisksPage: risk cards + device filter use label ?? hostname ?? ip - DeviceDetailPage: page title uses label ?? hostname ?? ip_address; subtitle shows the next level down (hostname or IP as context) Closes #111 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: noqa N806 + import order in MAC upsert tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fde3a3d commit 8e22c72

8 files changed

Lines changed: 156 additions & 24 deletions

File tree

backend/app/api/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ class RiskOut(BaseModel):
275275
device_id: int
276276
ip_address: str
277277
hostname: str | None
278+
label: str | None
278279
severity: str
279280
display_severity: str # may differ from severity based on active network profile
280281
check_id: str
@@ -441,6 +442,7 @@ def _risk_to_out(r, profile: str = "standard_home") -> RiskOut: # noqa: ANN001
441442
device_id=r.device_id,
442443
ip_address=r.device.ip_address,
443444
hostname=r.device.hostname,
445+
label=r.device.label,
444446
severity=r.severity,
445447
display_severity=display_severity_for_check(r.check_id, r.severity, profile),
446448
check_id=r.check_id,

backend/app/db.py

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@ def _migrate_schema(engine) -> None:
4646
if column not in existing_cols:
4747
conn.execute(text(f"ALTER TABLE {table} ADD COLUMN {column} {col_def}")) # noqa: S608 — table/column/col_def are internal constants, not user input
4848
conn.commit()
49+
# Ensure the MAC address index exists (CREATE INDEX IF NOT EXISTS is idempotent).
50+
conn.execute(
51+
text(
52+
"CREATE INDEX IF NOT EXISTS ix_devices_mac_address ON devices (mac_address)" # noqa: S608 — DDL constant
53+
)
54+
)
55+
conn.commit()
4956

5057

5158
def init_db() -> None:
@@ -93,21 +100,42 @@ def upsert_device(
93100
hostname: str | None = None,
94101
os_guess: str | None = None,
95102
) -> Device:
96-
"""Insert or update a Device row keyed on ip_address.
97-
98-
If a Device with the given ip_address already exists, only non-None
99-
fields are written so that richer data from a previous scan is never
100-
overwritten with None. The caller is responsible for committing.
101-
102-
Returns the Device instance (either existing or newly created).
103+
"""Insert or update a Device row, using MAC address as the primary identity key.
104+
105+
Lookup order:
106+
1. If mac_address is provided, find an existing device with that MAC.
107+
- Found at the same IP → normal update (non-None fields only).
108+
- Found at a different IP → update ip_address to the new one, preserving
109+
user-set fields (label, trusted, device_type).
110+
2. Fall back to ip_address lookup (covers devices that don't broadcast MAC,
111+
e.g. traffic routed through a switch without ARP visibility).
112+
3. If no existing device is found, create a new one.
113+
114+
The caller is responsible for committing. Returns the Device instance.
103115
"""
104116
from sqlalchemy import select
105117

106118
from app.models.device import Device
107119

108-
stmt = select(Device).where(Device.ip_address == ip_address)
109-
device: Device | None = session.execute(stmt).scalar_one_or_none()
120+
device: Device | None = None
121+
122+
# --- MAC-first lookup ---
123+
if mac_address is not None:
124+
device = session.execute(
125+
select(Device).where(Device.mac_address == mac_address)
126+
).scalar_one_or_none()
127+
if device is not None and device.ip_address != ip_address:
128+
# Device moved to a new IP — update the address in place so
129+
# user-assigned label/trusted/device_type are preserved.
130+
device.ip_address = ip_address
110131

132+
# --- IP fallback ---
133+
if device is None:
134+
device = session.execute(
135+
select(Device).where(Device.ip_address == ip_address)
136+
).scalar_one_or_none()
137+
138+
# --- Create ---
111139
if device is None:
112140
device = Device(
113141
ip_address=ip_address,
@@ -117,15 +145,17 @@ def upsert_device(
117145
os_guess=os_guess,
118146
)
119147
session.add(device)
120-
else:
121-
if mac_address is not None:
122-
device.mac_address = mac_address
123-
if vendor is not None:
124-
device.vendor = vendor
125-
if hostname is not None:
126-
device.hostname = hostname
127-
if os_guess is not None:
128-
device.os_guess = os_guess
148+
return device
149+
150+
# --- Update non-None scan fields (never overwrite user-set fields) ---
151+
if mac_address is not None:
152+
device.mac_address = mac_address
153+
if vendor is not None:
154+
device.vendor = vendor
155+
if hostname is not None:
156+
device.hostname = hostname
157+
if os_guess is not None:
158+
device.os_guess = os_guess
129159

130160
return device
131161

backend/app/models/device.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
Column,
66
DateTime,
77
ForeignKey,
8+
Index,
89
Integer,
910
String,
1011
UniqueConstraint,
@@ -22,6 +23,8 @@ class Device(Base):
2223
__table_args__ = (
2324
# One row per IP address — upserts update in place rather than inserting duplicates.
2425
UniqueConstraint("ip_address", name="uq_devices_ip_address"),
26+
# Index for MAC-first lookup in upsert_device().
27+
Index("ix_devices_mac_address", "mac_address"),
2528
)
2629

2730
id = Column(Integer, primary_key=True, index=True)

backend/app/scan_runner.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ def _record_scan_events(
323323
{
324324
"ip": d.ip_address,
325325
"hostname": d.hostname,
326+
"label": d.label,
326327
"mac": d.mac_address,
327328
"vendor": d.vendor,
328329
}

backend/tests/test_api.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -992,3 +992,91 @@ def test_wan_returns_stored_ip(client, db_engine):
992992
db.query(AppSetting).filter(AppSetting.key.in_(["wan_ip", "wan_ip_detected_at"])).delete()
993993
db.commit()
994994
db.close()
995+
996+
997+
# ── upsert_device MAC-first identity ─────────────────────────────────────────
998+
999+
1000+
def test_upsert_device_creates_new(db_engine):
1001+
"""upsert_device creates a new Device when MAC and IP are both unseen."""
1002+
from app.db import upsert_device
1003+
from app.models.device import Device
1004+
from sqlalchemy.orm import sessionmaker
1005+
1006+
Session = sessionmaker(bind=db_engine) # noqa: N806 — sessionmaker convention
1007+
db = Session()
1008+
try:
1009+
device = upsert_device(
1010+
db,
1011+
ip_address="192.168.99.10",
1012+
mac_address="aa:bb:cc:dd:ee:01",
1013+
hostname="newhost",
1014+
)
1015+
db.commit()
1016+
assert device.ip_address == "192.168.99.10"
1017+
assert device.hostname == "newhost"
1018+
finally:
1019+
db.query(Device).filter(Device.mac_address == "aa:bb:cc:dd:ee:01").delete()
1020+
db.commit()
1021+
db.close()
1022+
1023+
1024+
def test_upsert_device_mac_first_updates_ip(db_engine):
1025+
"""upsert_device uses MAC as primary key: if MAC seen at a new IP, updates IP."""
1026+
from app.db import upsert_device
1027+
from app.models.device import Device
1028+
from sqlalchemy.orm import sessionmaker
1029+
1030+
Session = sessionmaker(bind=db_engine) # noqa: N806 — sessionmaker convention
1031+
db = Session()
1032+
try:
1033+
# Create device at original IP
1034+
d = upsert_device(
1035+
db,
1036+
ip_address="192.168.99.20",
1037+
mac_address="aa:bb:cc:dd:ee:02",
1038+
)
1039+
d.label = "my-server"
1040+
d.trusted = True
1041+
db.commit()
1042+
original_id = d.id
1043+
1044+
# Simulate DHCP lease change: same MAC, new IP
1045+
d2 = upsert_device(
1046+
db,
1047+
ip_address="192.168.99.21",
1048+
mac_address="aa:bb:cc:dd:ee:02",
1049+
)
1050+
db.commit()
1051+
1052+
assert d2.id == original_id, "Should be the same DB row"
1053+
assert d2.ip_address == "192.168.99.21", "IP should be updated"
1054+
assert d2.label == "my-server", "User label must be preserved"
1055+
assert d2.trusted is True, "Trusted flag must be preserved"
1056+
finally:
1057+
db.query(Device).filter(Device.mac_address == "aa:bb:cc:dd:ee:02").delete()
1058+
db.commit()
1059+
db.close()
1060+
1061+
1062+
def test_upsert_device_no_mac_falls_back_to_ip(db_engine):
1063+
"""upsert_device falls back to IP lookup when MAC is not provided."""
1064+
from app.db import upsert_device
1065+
from app.models.device import Device
1066+
from sqlalchemy.orm import sessionmaker
1067+
1068+
Session = sessionmaker(bind=db_engine) # noqa: N806 — sessionmaker convention
1069+
db = Session()
1070+
try:
1071+
d1 = upsert_device(db, ip_address="192.168.99.30", hostname="alpha")
1072+
db.commit()
1073+
1074+
d2 = upsert_device(db, ip_address="192.168.99.30", hostname="alpha-updated")
1075+
db.commit()
1076+
1077+
assert d1.id == d2.id
1078+
assert d2.hostname == "alpha-updated"
1079+
finally:
1080+
db.query(Device).filter(Device.ip_address == "192.168.99.30").delete()
1081+
db.commit()
1082+
db.close()

frontend/src/pages/DeviceDetailPage.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,14 @@ export function DeviceDetailPage() {
208208
</span>
209209
</div>
210210
<PageHeader
211-
title={device.ip_address}
212-
subtitle={displayLabel ?? device.hostname ?? undefined}
211+
title={displayLabel ?? device.hostname ?? device.ip_address}
212+
subtitle={
213+
displayLabel != null
214+
? (device.hostname ?? device.ip_address)
215+
: device.hostname != null
216+
? device.ip_address
217+
: undefined
218+
}
213219
action={
214220
<div className="flex items-center gap-3">
215221
{device.trusted && (

frontend/src/pages/RisksPage.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ function RiskModal({
203203
className="text-[var(--color-accent-positive)] hover:underline"
204204
onClick={onClose}
205205
>
206-
{risk.hostname || risk.ip_address}
206+
{risk.label ?? risk.hostname ?? risk.ip_address}
207207
</Link>
208208
</dd>
209209
</div>
@@ -381,8 +381,8 @@ export function RisksPage() {
381381
<option value="">All devices</option>
382382
{devices.map((d) => (
383383
<option key={d.id} value={d.id}>
384-
{d.ip_address}
385-
{d.hostname ? ` (${d.hostname})` : ""}
384+
{d.label ?? d.ip_address}
385+
{!d.label && d.hostname ? ` (${d.hostname})` : ""}
386386
</option>
387387
))}
388388
</select>
@@ -428,7 +428,8 @@ export function RisksPage() {
428428
<Card padding="none">
429429
<div className="divide-y divide-[var(--color-border)]">
430430
{risks.map((risk) => {
431-
const deviceLabel = risk.hostname || risk.ip_address;
431+
const deviceLabel =
432+
risk.label ?? risk.hostname ?? risk.ip_address;
432433
return (
433434
<button
434435
key={risk.id}

frontend/src/types/api.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export interface Risk {
6363
device_id: number;
6464
ip_address: string;
6565
hostname: string | null;
66+
label: string | null;
6667
severity: Severity;
6768
display_severity: Severity;
6869
check_id: string;

0 commit comments

Comments
 (0)