diff --git a/PLAN.md b/PLAN.md index 4f93bba..f69c6ab 100644 --- a/PLAN.md +++ b/PLAN.md @@ -2365,6 +2365,51 @@ Open items from the drive — fill in as reported: --- +### Phase 30: Pre-road code audit & hardening (round 2) — ✅ DONE (2026-08-15) +**Completed 2026-08-15.** Read-only audit of every Pi module, the server, the deploy +scripts/units/sudoers, SQL migrations, and the Android build config, focused on "is this +ready for the road". 99 tests pass; all Python compiles; every referenced template exists; +sudoers entries were verified against the exact commands the app invokes (`mount -o r[ow] *` +correctly matches `mount -o ro /dev/sda1 /mnt/dashcam` because sudoers `*` spans whitespace). + +**Tracking-path safety**: none of the fixes below touch the core tracking path +(`app_gps._poll_loop` → `app_sql._log_loop`/`insert_tracking`, `app_obd.obd_speed`, +`app_trip.build_trip_summaries`/`refresh_trips`). The only `app_sql` change is in +`get_overview_stats()` (the `/api/stats` + status-page reader, not the logger). Verified +after the edits: compile clean + 99/99 tests, including the trip-building and trip-SQL suites. + +#### Bugs found and fixed + +| File | Severity | Bug | Fix | +|---|---|---|---| +| `modules/app_sql.py` | MEDIUM | `get_overview_stats()` ran a `SUM(DISTANCE_KM)` query on the shared SQLite connection *outside* `_conn_lock` — a race with the logging thread using the same connection | Moved the query inside the existing `with _conn_lock:` block | +| `modules/app_upload.py` | MEDIUM | `_upload_loop()` only caught `requests.RequestException`/`CellularError`; an `OSError` (dashcam file deleted between listing and hashing, disk full) escaped and permanently killed the upload thread | Added `OSError` to the retry/backoff `except` clause | +| `server/app.py` | LOW | `portal_trips_compat()` called `int(request.args.get("limit"))` without try/except — `?limit=abc` → 500 | Wrapped in `try/except (ValueError, TypeError)` → default 200 | +| `server/app.py` | LOW | `portal_select_device()` called `int(device_id)` without try/except — non-numeric id → 500 | Validate + return 400 on non-numeric id | + +#### Audit false positives (verified NOT bugs) +- `modules/app_account.py` `_load_config()` **does** end in `return {}` (the initial read + omitted the final no-newline line; confirmed via `cat -A`/`od -c`). The file is CRLF — + cosmetic only. +- `server/app.py` `portal_dashcam_stream()` Range parsing: a leading `-` (e.g. `bytes=-5`) + always parses to an empty `start_s` and is rejected by `int("")` → 416, so no negative + seek can ever reach `f.seek()`. Already safe. +- OBD unit conversions: verified the installed `obd==0.7.3` + pint handle + `.to("kph")`, `.to("percent")`, and `.to("degC")` correctly. + +#### Known / deferred (documented, not changed this phase) +- `modules/app_settings.py` `update_settings()` requires the full payload and reports omitted + booleans as "Must be true or false." — strict but safe (the web UI always sends every field). +- `modules/app_dash.py`: `dashcam_storage_usage()` clobbers its result on unmount failure, and + `import_dashcam()` releases `_device_lock` while copying so a concurrent delete could remount + read-write mid-copy. Low-likelihood; left for a follow-up. +- `app.py` `/api/local-video/convert` reports `"started"` for a `.mp4` (a no-op). Cosmetic; left + untouched to avoid editing the CRLF `app.py` for a UX-only nit. +- `android/gradle/libs.versions.toml`: `compose-compiler = "1.5.15"` is dead config (the build + uses the `kotlin.compose` plugin). Harmless. + +--- + ### Deployment automation — fresh-OS installer + hardware→software guide (2026-08-15) Added a one-shot fresh-install entrypoint for the deploy folder and fixed three deployment gaps that would have broken a from-scratch install: diff --git a/modules/app_sql.py b/modules/app_sql.py index ad607c8..77e5603 100644 --- a/modules/app_sql.py +++ b/modules/app_sql.py @@ -504,6 +504,9 @@ def get_overview_stats(): dtc_row = conn.execute( "SELECT COUNT(*) FROM dtc WHERE IS_SET = 1" ).fetchone() + cumul = conn.execute( + "SELECT COALESCE(SUM(DISTANCE_KM), 0) FROM trips" + ).fetchone() from modules import app_video as _av try: video_count = sum(1 for _ in _av.list_local_videos().get("files", [])) @@ -511,7 +514,6 @@ def get_overview_stats(): video_count = 0 from modules import app_settings base = app_settings._number_value("odometer_baseline", float, 0) - cumul = conn.execute("SELECT COALESCE(SUM(DISTANCE_KM), 0) FROM trips").fetchone() computed = base + (cumul[0] if cumul else 0) return { "trip_count": row[0], diff --git a/modules/app_upload.py b/modules/app_upload.py index 605c88e..e68499a 100644 --- a/modules/app_upload.py +++ b/modules/app_upload.py @@ -390,7 +390,11 @@ def _upload_loop(): time.sleep(CATCHUP_INTERVAL) else: time.sleep(UPLOAD_INTERVAL) - except (requests.RequestException, app_cellular.CellularError) as e: + except ( + requests.RequestException, + app_cellular.CellularError, + OSError, + ) as e: _last_error = str(e) print(f"Upload failed ({e}), retrying in {backoff}s") time.sleep(backoff) diff --git a/server/app.py b/server/app.py index 07b2b27..f19d786 100644 --- a/server/app.py +++ b/server/app.py @@ -137,7 +137,11 @@ def portal_select_device(): session.pop("selected_device_id", None) session.pop("selected_device_name", None) return jsonify(status="ok", message="Device selection cleared.") - dev = db.get_device_by_id(int(device_id)) + try: + device_id = int(device_id) + except (TypeError, ValueError): + return jsonify(error="Invalid device_id."), 400 + dev = db.get_device_by_id(device_id) if dev is None: return jsonify(error="Device not found"), 404 session["selected_device_id"] = dev["id"] @@ -860,7 +864,10 @@ def portal_trips_compat(): device_id = _portal_device_id() if device_id is None: return jsonify(error="device not configured"), 404 - limit = min(int(request.args.get("limit", 200)), 1000) + try: + limit = min(int(request.args.get("limit", 200)), 1000) + except (ValueError, TypeError): + limit = 200 trips = db.list_trips(device_id, limit=limit) return jsonify(trips=trips)