Skip to content

Lan spy#134

Open
JonanOribe wants to merge 31 commits intosmittix:mainfrom
JonanOribe:lan-spy
Open

Lan spy#134
JonanOribe wants to merge 31 commits intosmittix:mainfrom
JonanOribe:lan-spy

Conversation

@JonanOribe
Copy link
Copy Markdown
Contributor

Basic LAN analysis functionalities have been added. Scan a network and view simple information. This serves as a foundation for integrating more complex functionalities in the future.

The Dockerfile and .sh file have been updated to install nmap as the library for scanning.

Introduce a new LAN SPY mode for local network device discovery and risk scoring. Adds backend blueprint (routes/lan_spy.py) with endpoints for scanning, device CRUD, risk scoring, OUI updates and an SSE event stream; initializes LAN SPY state in routes/__init__.py. Adds frontend assets (static/js/lan_spy.js, static/css/modes/lan_spy_dashboard.css) and includes the UI partial in templates/index.html. Adds supporting utils (utils/lan_spy/scanner.py, database.py, risk_scoring.py) and declares pyyaml in requirements.txt for OUI handling. Provides real-time scan events, background scan worker, device flag toggles, and a non-blocking loader UI.
Lan Spy moved to the correct main dashboard section
Copy link
Copy Markdown
Owner

@smittix smittix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution — LAN scanning is a great fit for the platform, and the overall architecture (SSE streaming, blueprint pattern, DB tables) follows existing conventions well. However there are several bugs that would prevent this from working correctly. Please address these before merging.

Bugs (blockers)

1. get_risk_score name collision (routes/lan_spy.py)

The route function def get_risk_score(device) shadows the imported DB function of the same name. Then get_devices() calls get_risk_score(device)[0].json — wrong signature and wrong return type (the DB function returns a dict or None, not a list). This will raise a TypeError at runtime whenever devices are fetched.

Rename the route function to something like risk_score_for_device to avoid the collision.

2. DB connection escapes context manager (utils/database.py, add_risk_score)

with get_db() as conn:
    cursor = conn.execute('SELECT id FROM risk_scores ...')
    existing = cursor.fetchone()

if existing:          # ← outside the with block, connection is closed
    conn.execute(...) # ← dead connection

The if existing branch needs to be inside the with get_db() as conn: block.

3. lan_spy_module.lan_devices doesn't exist (app.py health check)

'lan_devices_count': len(lan_spy_module.lan_devices)

lan_spy.py has no lan_devices attribute — only lan_spy_state. This throws an AttributeError on every /health request, breaking the health check for all users regardless of whether they use LAN spy.

4. lan_spy_process health check is always None

LAN spy uses threading.Thread, not a subprocess, so lan_spy_process is never set and lan_spy_process.poll() will never reflect scan state. Either track scan state via lan_spy_state['scan_running'] or remove it from the health check.

Code quality issues

5. Debug print() statements in production code (utils/database.py)

def get_risk_score(mac: str):
    print(f"Looking for risk score for MAC: {mac}")  # remove
    ...
    except Exception as e:
        print(f"Error en get_risk_score: {e}")  # use logger

Please use logger.error() / logger.debug() consistently with the rest of the codebase.

6. OUI database downloaded on every startup

init_lan_spy_state() calls update_oui_database() unconditionally, which makes an outbound HTTP request during gunicorn worker initialisation. This will:

  • Slow or block startup if the IEEE server is slow/down
  • Use plain HTTP (http://standards-oui.ieee.org) — susceptible to MITM

Suggest: only download if the file is missing or older than N days, and switch to HTTPS.

7. socket.setdefaulttimeout() is not thread-safe (_resolve_hostname)

socket.setdefaulttimeout(1)
hostname = socket.gethostbyaddr(ip)[0]
socket.setdefaulttimeout(None)

This modifies the global default for all threads. Use socket.getaddrinfo with a per-socket timeout, or wrap in a concurrent.futures call with a timeout instead.

8. as_completed(..., timeout=5) cuts /24 scans short (_parallel_ping)

A timeout of 5 seconds on the entire as_completed iterator means a 254-host scan will be cut off after 5 seconds of first result, not per-host. For a typical /24 this will silently drop most results. The per-ping timeout is already enforced in _ping_host — the as_completed timeout should be removed or set much higher.

9. Missing utils/lan_spy/__init__.py

This file doesn't appear in the changed files list. Without it the utils.lan_spy package won't import.

Minor

  • The app.py diff contains a large number of whitespace-only changes (tabs → spaces) that add noise without functional effect. Please rebase and keep formatting changes out of the PR.
  • Please verify pyyaml is added to requirements.txt — it's a new dependency used by risk_scoring.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants