julian
f486dcbbfc
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
170 lines
12 KiB
Markdown
170 lines
12 KiB
Markdown
---
|
||
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.
|