docs(wiki): vision-service hardening backlog + boot-migration data-seed note
Two unrelated leftover wiki edits from earlier sessions:
- NEW concepts/vision-service-hardening.md: the prioritised to-do list from the
2026-07-02 code + security reviews of apps/vision/ (DoS gaps, unauthenticated/
operator-writable model weights, 0.0.0.0 default bind). Cross-linked from
opencv-anpr-service.md ("consult before touching this service").
- container-deployment.md: note that a boot-time migration can be a DATA SEED
(e.g. an RBAC permission granted to the operator role via INSERT OR IGNORE),
and that a built-in-role grant does not auto-apply to a custom role.
Claude-Session: https://claude.ai/code/session_01Xcm6ikLgGoCxxHrxtjkk5V
This commit is contained in:
@@ -0,0 +1,169 @@
|
||||
---
|
||||
type: concept
|
||||
tags: [parking, vision, anpr, security, hardening, tech-debt]
|
||||
sources: []
|
||||
updated: 2026-07-02
|
||||
status: open
|
||||
---
|
||||
|
||||
# Vision Service — Hardening & Fix Backlog
|
||||
|
||||
Tracked findings against the [[opencv-anpr-service]] (`apps/vision/`, the Python/FastAPI ANPR
|
||||
microservice — see [[vision-service-packaging]]). Raised by two code reviews on 2026-07-02: a
|
||||
general pass (bottlenecks / bugs / best-practice) and a security-focused pass. **Nothing here is
|
||||
fixed yet** — this page is the to-do list; tick items off (and note the commit) as they land.
|
||||
|
||||
Framing that shapes the priorities below, both from [[threat-model]]:
|
||||
|
||||
- **A forged image cannot open a barrier by itself.** The Node server pushes snapshot bytes to
|
||||
`/analyze`, then **re-gates** the result server-side (`VISION_ENTRY_MIN_CONFIDENCE` = 0.85,
|
||||
stricter than the service's own advisory 0.5 floor), with debounce and a mid-poll
|
||||
"already-transacted" abort (`apps/server/src/anpr-entry.ts`). So the real exposure is
|
||||
**availability (DoS)** and **network/weight placement**, not decision forgery. See
|
||||
[[lane-presence-and-anpr-entry]], [[fail-state-safety]] (recognition is advisory; the host falls
|
||||
back to the ticket path when vision is down/unsure).
|
||||
- **The primary adversary is the local booth operator**, with filesystem/USB access to the
|
||||
appliance — which is what makes the model-weight and config-placement items real, not theoretical.
|
||||
|
||||
## Priority 1 — DoS (remotely triggerable, zero appliance access)
|
||||
|
||||
1. **Request body buffered *before* the size cap** — `apps/vision/vision_service/app.py:69`.
|
||||
`image = await request.body()` concatenates the whole stream into memory before
|
||||
`len(image) > MAX_IMAGE_BYTES` (12 MB) runs. A chunked POST with no `Content-Length` streams
|
||||
unbounded bytes → RAM exhaustion → OOM-kill **before** the 413 is ever returned. Killing the
|
||||
recognizer forces permanent ticket-fallback (which may be the operator's goal — suppress plate
|
||||
evidence). **Fix:** reject on `Content-Length` up front and cap while draining
|
||||
`request.stream()`; pass `--limit-max-request-body-size` to uvicorn.
|
||||
|
||||
2. **Pixel-bomb: `cv2.imdecode` has no decoded-dimension bound** —
|
||||
`apps/vision/vision_service/recognizer.py:143`. A crafted JPEG *under* the 12 MB byte cap can
|
||||
declare enormous dimensions (e.g. 30000×30000) and decode to a multi-GB BGR ndarray. Combined
|
||||
with item 3 (inference on the event loop), one request both spikes memory and stalls the whole
|
||||
service. **Fix:** check `frame.shape` against a max pixel budget immediately after decode; reject
|
||||
oversize frames with a 422.
|
||||
|
||||
## Priority 2 — Bottleneck (also a DoS amplifier)
|
||||
|
||||
3. **CPU-bound inference runs synchronously on the asyncio event loop** —
|
||||
`apps/vision/vision_service/app.py:84` (calls `rec.analyze`, which does `cv2.imdecode` + ONNX
|
||||
YOLO+OCR). uvicorn runs a single event loop; while inference runs (hundreds of ms on the i5-8500,
|
||||
CPU-only), **nothing else is served — including `GET /health`**. Back-to-back entry/exit
|
||||
snapshots serialize, and health probes time out, making a live service look down. When the Node
|
||||
client's 1500 ms `AbortController` gives up, Python keeps burning CPU on the abandoned request.
|
||||
Flagged independently by ~half the review angles — the one real bottleneck. **Fix:**
|
||||
`await run_in_threadpool(rec.analyze, image)` (or make the `Recognizer` protocol async), plus an
|
||||
`asyncio.Semaphore(1–2)` to bound concurrent inferences. This is a property the `Recognizer`
|
||||
mechanism should own, not a per-call patch.
|
||||
|
||||
## Priority 2 — Security: network placement & weight integrity
|
||||
|
||||
4. **Model weights are unauthenticated *and* operator-writable → persistent recognition-poisoning.**
|
||||
`fast-alpr`'s `ALPR()` downloads ONNX weights + config over plain `urllib` with **no
|
||||
checksum/signature** (verified in `.venv`: `open_image_models/detection/core/hub.py`,
|
||||
`fast_plate_ocr/inference/hub.py`), and a cache-hit is treated as trust (present → skip, never
|
||||
re-verify). The `vision` user (uid 999) owns **both** the `~/.cache` weights and the runtime
|
||||
process, so any same-user write primitive overwrites weights in place — no privesc — surviving
|
||||
container **restarts** (cache is in the writable layer, not re-verified at startup). Under
|
||||
[[threat-model]], a local operator overwriting a cached `.onnx` is a persistent, targeted fraud
|
||||
primitive: a detector tuned to never see a specific plate, or an OCR model that reproducibly
|
||||
substitutes a character. **Fix:** pin + verify a weights hash, mount the weights **read-only**,
|
||||
and assert a known hash at startup. Relates to [[disk-os-hardening]] (physical-tamper chain),
|
||||
[[reconciliation]] (the real backstop).
|
||||
|
||||
5. **Service binds `0.0.0.0` by default at every layer** — `settings.py:18`, `Dockerfile:51`
|
||||
(`ENV VISION_HOST=0.0.0.0`), `Dockerfile:57` (uvicorn `--host 0.0.0.0`). The `.env.example`
|
||||
documents `127.0.0.1`-only intent, but loopback-only exposure then depends entirely on the
|
||||
compose `ports:` prefix being right every time. The app provides **zero** defense-in-depth:
|
||||
`/analyze` and `/health` have no auth, so reachability is the only control. **Fix:** make
|
||||
`127.0.0.1` the default at all three layers; require explicit opt-in to widen. Relates to
|
||||
[[network-isolation]], [[trust-boundary]].
|
||||
|
||||
6. **Dev compose publishes vision on all interfaces** — `docker-compose.dev.yml:29` maps
|
||||
`"8089:8089"` (binds 0.0.0.0), unlike `docker-compose.prod.yml:91`'s `"127.0.0.1:8089:8089"`.
|
||||
Anyone on the same LAN as a dev/staging box — or a field deploy that reuses the dev file — can
|
||||
hit the unauthenticated endpoint: fingerprint the model via `/health`, run the DoS above, or
|
||||
probe confidence behaviour. **Fix:** bind `127.0.0.1:8089:8089` to match prod. See
|
||||
[[container-deployment]].
|
||||
|
||||
7. **No container resource limits or hardening.** The `vision` service has no
|
||||
`mem_limit`/`cpus`/`pids_limit` and no `security_opt: [no-new-privileges:true]` /
|
||||
`cap_drop: [ALL]` / `read_only` in any compose file. Since prod runs the server on the **host
|
||||
network** doing safety-critical device I/O, an unbounded vision container (via items 1–2) can
|
||||
starve the host of RAM/CPU. **Fix:** add `mem_limit`, `pids_limit`, `cap_drop: [ALL]`,
|
||||
`no-new-privileges`. (Already good: runs non-root as uid 999; `.env` is **not** baked into any
|
||||
image layer — `COPY`s are explicit, root `.dockerignore`/`.gitignore` cover `.env`.) See
|
||||
[[disk-os-hardening]], [[container-deployment]].
|
||||
|
||||
## Priority 3 — Correctness, robustness, hygiene
|
||||
|
||||
8. **`env_file=".env"` resolves against the process CWD** — `settings.py:16`. Launched from anywhere
|
||||
but `apps/vision/` (a systemd unit, or a run from the monorepo root), the `.env` isn't found and
|
||||
pydantic-settings raises **no error** — the service silently boots `recognizer="stub"`, `/health`
|
||||
reports `ready: true`, and every scan returns `plate: null`. Production ANPR silently does
|
||||
nothing while looking healthy. **Fix:** anchor the path, e.g.
|
||||
`env_file=Path(__file__).parent.parent / ".env"`.
|
||||
|
||||
9. **`/health` returns HTTP 200 even when `ready` is false**, and the Docker HEALTHCHECK
|
||||
(`Dockerfile:55`) only checks `status==200` — `app.py:54`. If the fast-alpr weights fail to load
|
||||
(e.g. the best-effort build pre-warm was skipped and the appliance is air-gapped), the container
|
||||
stays "healthy" to Docker forever: no restart, no infra alert; only the in-app device monitor
|
||||
notices. **Fix:** return 503 from `/health` when not ready, or make the HEALTHCHECK parse the
|
||||
`ready` field.
|
||||
|
||||
10. **Build pre-warm swallows *all* failures** — `Dockerfile:46`
|
||||
(`... || echo "skipped (no network)"`). It can't distinguish "no network at build, expected"
|
||||
from "download corrupted/tampered/interrupted". A skipped pre-warm silently converts the
|
||||
air-gapped appliance into one that fetches weights from github.com on the **first real
|
||||
`/analyze`** — an unreviewed runtime network dependency contradicting [[offline-first]] (and a
|
||||
first-scan DoS if egress is truly blocked). **Fix:** fail the *prod* build on pre-warm failure
|
||||
(or assert weights present at startup) rather than degrade to a lazy fetch.
|
||||
|
||||
11. **Raw exception strings reflected into HTTP responses** — `app.py:88` (`analysis failed: {exc}`)
|
||||
and the 503 loader path at `app.py:81`. Echoes native cv2/onnxruntime error text (absolute
|
||||
paths, library versions) to any caller. **Fix:** log detail server-side; return a generic
|
||||
message to the client. (Modest severity — the primary adversary already has local access.)
|
||||
|
||||
12. **`min_confidence` has no bounds validation** — `settings.py:33`, unlike `PlateResult.confidence`
|
||||
(`ge=0, le=1`). `VISION_MIN_CONFIDENCE=50` (someone thinking in percent) silently flags every
|
||||
read as `low_confidence`, with no startup error or health symptom. **Fix:**
|
||||
`float = Field(0.5, ge=0.0, le=1.0)`.
|
||||
|
||||
13. **Node client discards the structured error detail** — `apps/server/src/vision-client.ts:187`.
|
||||
`#post` throws `vision /analyze → HTTP 422` without reading the body, so
|
||||
`"could not decode image bytes"` (corrupt camera frame — actionable) is indistinguishable in the
|
||||
logs from a deploy problem. **Fix:** read `detail` from the JSON body before throwing.
|
||||
(Server-side file, just outside `apps/vision/`, but it's the consumer of this contract.)
|
||||
|
||||
14. **`build_recognizer` docstring is wrong** — `recognizer.py:169`. It says "falls back to the stub
|
||||
if the real one can't load," but the function returns the **not-ready** `FastAlprRecognizer`
|
||||
(better behaviour — `/analyze` 503s instead of silently returning no-plate). **Fix:** correct
|
||||
the docstring so nobody "fixes" the code to match it.
|
||||
|
||||
15. **Model-name env vars are undefended in-repo (latent)** — `settings.py:28–29`.
|
||||
`detector_model` / `ocr_model` are plain `str`; the allowlist that makes them safe (rejects
|
||||
unknown names) lives **entirely** in the third-party libs, not here. **Not a bug today** (no
|
||||
SSRF/traversal), but a future recognizer swap (the pluggable design invites one) could turn an
|
||||
operator-settable env var into a URL/path primitive. **Fix:** constrain to a `Literal`/enum or
|
||||
validate explicitly.
|
||||
|
||||
## Lower priority / noted (not scheduled)
|
||||
|
||||
- **Base image tag-, not digest-, pinned** — `Dockerfile:9`
|
||||
(`ghcr.io/astral-sh/uv:python3.12-bookworm-slim`); `apt-get` without package pinning
|
||||
(`Dockerfile:16–18`). `uv sync --frozen` locks the Python deps, so this is OS-layer drift only —
|
||||
low priority hardening.
|
||||
- **CLI reads image files unbounded** — `cli.py:55` (`read_bytes()`, no size cap). Dev-only tool,
|
||||
local invocation; mirror the HTTP size discipline for consistency.
|
||||
- **Architecture note (server, not vision):** `anpr-entry.ts` auto-opens the barrier on a
|
||||
high-confidence plate match with **no second factor**, so a printed duplicate of a known
|
||||
subscriber's plate is a physical-spoofing bypass (mitigated only by the signed event log, once a
|
||||
hardware signer lands — see [[hardware-signer-options]], [[append-only-event-chain]]). A conscious
|
||||
design sign-off, not a vision-service bug — flagged so it isn't an accident. See
|
||||
[[lane-presence-and-anpr-entry]], [[plate-reconciliation]].
|
||||
|
||||
## Clean (checked, no action)
|
||||
|
||||
Content-type is **not** trusted for parsing (cv2 sniffs bytes, ignores the header). The service
|
||||
already runs non-root (uid 999), loads models once at startup (not per request), and `.env` is not
|
||||
committed or baked into an image layer. The `getattr`-based fast-alpr result mapping, the typed
|
||||
`app.state` accessors, and the lifespan factory all have stated rationales and are fine as-is.
|
||||
Reference in New Issue
Block a user