security(ocr): run OCR container as non-root user (CIS Docker §4.1) #459

Open
opened 2026-05-07 17:21:55 +02:00 by marcel · 8 comments
Owner

Context

ocr-service/Dockerfile lacks a USER directive. uvicorn runs as UID 0 inside the container.

# ocr-service/Dockerfile (current)
COPY . .
RUN chmod +x /app/entrypoint.sh
EXPOSE 8000
CMD ["/app/entrypoint.sh"]
# ← no USER directive anywhere

Any RCE in the OCR pipeline (PyTorch, Surya, Kraken, opencv, libvips — all processing user-uploaded PDFs at scale) yields root inside the container. Container-escape exploits are sporadically published, and root-in-container remains a CIS Docker §4.1 violation. The OCR service is also the only service in the stack that ingests fully-untrusted user content directly into native image-processing libraries — exactly the threat model that motivates the non-root rule.

This issue is OCR-only; #134 (backend image) and #135 (frontend image) cover the other two services.

Approach

Add a non-root user, transfer ownership, switch context. Plus harden the runtime in docker-compose.yml.

ocr-service/Dockerfile

# … existing layers up through `COPY . .` …

RUN useradd --no-create-home --shell /usr/sbin/nologin --uid 1000 ocr \
    && chown -R ocr:ocr /app
RUN chmod +x /app/entrypoint.sh
USER ocr
EXPOSE 8000
CMD ["/app/entrypoint.sh"]

Verify Kraken/Surya can write to their model cache. The compose file mounts ocr_models:/app/models and ocr_cache:/root/.cache — the second mount path is wrong for a non-root user. Change to:

volumes:
  - ocr_models:/app/models
  - ocr_cache:/home/ocr/.cache    # was /root/.cache

(Or set HF_HOME=/app/cache in the env and mount there — keeps everything under /app.)

docker-compose.yml — runtime hardening

ocr-service:
  # … existing config …
  read_only: true
  tmpfs:
    - /tmp
  cap_drop: [ALL]
  security_opt:
    - no-new-privileges:true

read_only: true may break Surya/Kraken if they write to a non-volume path; if so, scope the override to specific writable mounts.

Critical files

  • ocr-service/Dockerfile
  • docker-compose.yml — ocr-service service block, volume path
  • ocr-service/entrypoint.sh — verify it works as a non-root user

Verification

  1. docker compose build ocr-service && docker compose up -d ocr-service
  2. docker exec archive-ocr id → returns uid=1000(ocr), not uid=0(root).
  3. docker exec archive-ocr ls -la /app/models → ocr:ocr ownership.
  4. Healthcheck still green: docker compose ps ocr-serviceUp (healthy).
  5. End-to-end OCR run: trigger a real OCR on a small PDF via the backend, confirm streaming response completes without permission errors.
  6. docker run --rm -v /var/run/docker.sock:/var/run/docker.sock -v $(pwd):/root docker/docker-bench-security reports 0 CIS §4.1 violations on archive-ocr.

Acceptance criteria

  • OCR container runs as UID 1000.
  • read_only: true and cap_drop: [ALL] set in compose.
  • Existing OCR end-to-end flow still works (streaming + non-streaming).
  • Training endpoint still works (token-protected per ADR-001).
  • Persisted model cache survives container recreate.

Effort

S — half a day including the cache-path migration and verification.

Risk if not addressed

Container compromise = root in container. Combined with Docker socket access on the host, that's a path to host compromise on a multi-tenant or shared host.

Tracked in audit doc as F-06 (Critical).

## Context `ocr-service/Dockerfile` lacks a `USER` directive. uvicorn runs as UID 0 inside the container. ```dockerfile # ocr-service/Dockerfile (current) COPY . . RUN chmod +x /app/entrypoint.sh EXPOSE 8000 CMD ["/app/entrypoint.sh"] # ← no USER directive anywhere ``` Any RCE in the OCR pipeline (PyTorch, Surya, Kraken, opencv, libvips — all processing user-uploaded PDFs at scale) yields root inside the container. Container-escape exploits are sporadically published, and root-in-container remains a CIS Docker §4.1 violation. The OCR service is also the **only** service in the stack that ingests fully-untrusted user content directly into native image-processing libraries — exactly the threat model that motivates the non-root rule. This issue is OCR-only; #134 (backend image) and #135 (frontend image) cover the other two services. ## Approach Add a non-root user, transfer ownership, switch context. Plus harden the runtime in `docker-compose.yml`. ### `ocr-service/Dockerfile` ```dockerfile # … existing layers up through `COPY . .` … RUN useradd --no-create-home --shell /usr/sbin/nologin --uid 1000 ocr \ && chown -R ocr:ocr /app RUN chmod +x /app/entrypoint.sh USER ocr EXPOSE 8000 CMD ["/app/entrypoint.sh"] ``` Verify Kraken/Surya can write to their model cache. The compose file mounts `ocr_models:/app/models` and `ocr_cache:/root/.cache` — the second mount path is wrong for a non-root user. Change to: ```yaml volumes: - ocr_models:/app/models - ocr_cache:/home/ocr/.cache # was /root/.cache ``` (Or set `HF_HOME=/app/cache` in the env and mount there — keeps everything under `/app`.) ### `docker-compose.yml` — runtime hardening ```yaml ocr-service: # … existing config … read_only: true tmpfs: - /tmp cap_drop: [ALL] security_opt: - no-new-privileges:true ``` `read_only: true` may break Surya/Kraken if they write to a non-volume path; if so, scope the override to specific writable mounts. ## Critical files - `ocr-service/Dockerfile` - `docker-compose.yml` — ocr-service service block, volume path - `ocr-service/entrypoint.sh` — verify it works as a non-root user ## Verification 1. `docker compose build ocr-service && docker compose up -d ocr-service` 2. `docker exec archive-ocr id` → returns `uid=1000(ocr)`, not `uid=0(root)`. 3. `docker exec archive-ocr ls -la /app/models` → ocr:ocr ownership. 4. Healthcheck still green: `docker compose ps ocr-service` → `Up (healthy)`. 5. End-to-end OCR run: trigger a real OCR on a small PDF via the backend, confirm streaming response completes without permission errors. 6. `docker run --rm -v /var/run/docker.sock:/var/run/docker.sock -v $(pwd):/root docker/docker-bench-security` reports 0 CIS §4.1 violations on `archive-ocr`. ## Acceptance criteria - [ ] OCR container runs as UID 1000. - [ ] `read_only: true` and `cap_drop: [ALL]` set in compose. - [ ] Existing OCR end-to-end flow still works (streaming + non-streaming). - [ ] Training endpoint still works (token-protected per ADR-001). - [ ] Persisted model cache survives container recreate. ## Effort S — half a day including the cache-path migration and verification. ## Risk if not addressed Container compromise = root in container. Combined with Docker socket access on the host, that's a path to host compromise on a multi-tenant or shared host. Tracked in audit doc as **F-06** (Critical).
marcel added this to the Production v1 milestone 2026-05-07 17:21:55 +02:00
marcel added the P0-criticaldevopsphase-2: container-imagessecurity labels 2026-05-07 17:23:31 +02:00
Author
Owner

🏗️ Markus Keller — Application Architect

Observations

  • The OCR service is the correct scope for this issue. It is explicitly justified as a separate service due to different resource requirements (8GB+ memory, GPU optional) and deployment cadence — this extraction has an ADR (ADR-001), and the single-node constraint is documented.
  • The ocr_cache:/root/.cache volume mount is the critical problem: once USER ocr is set, /root/.cache becomes inaccessible. The issue correctly identifies this. However, the two fix options (change mount path vs. set HF_HOME=/app/cache) have different operational implications worth spelling out.
  • ensure_blla_model.py uses os.path.expanduser("~/.local/share/htrmopo") as HTRMOPO_DIR. After switching to USER ocr (with --no-create-home), ~ expands to /, not /home/ocr. The blla download path will break silently unless HTRMOPO_DIR or HOME is explicitly set. This is an additional wrinkle the issue does not mention.
  • read_only: true in the compose block will conflict with tempfile.TemporaryDirectory() in the /train and /train-sender endpoints — both write to the system /tmp. The issue anticipates this: "if so, scope the override to specific writable mounts."
  • The docs/architecture/c4/l2-containers.puml describes the OCR service but does not document its security posture. This change does not add a new container, so no diagram update is strictly required per the doc matrix — but a comment noting the non-root runtime would be a worthwhile one-liner addition.
  • Backend and frontend Dockerfiles also lack USER directives — those are tracked separately in #134 and #135. That decomposition is correct; do not scope-creep into this issue.

Recommendations

  • Choose HF_HOME=/app/cache over changing the mount path. It is more explicit (the variable name documents the intent), keeps all writable paths under /app (a single chown covers everything), and is immune to the ~ expansion problem with --no-create-home.
  • Set HOME=/app in the Dockerfile (or at minimum set HTRMOPO_DIR explicitly in ensure_blla_model.py) to prevent the silent ~ resolution failure when kraken get downloads the blla model.
  • Add tmpfs: [/tmp] alongside read_only: true rather than widening the read-only restriction. The training endpoints need /tmp; the tmpfs mount gives them a writable, non-persistent, memory-backed /tmp while keeping the rest of the filesystem read-only.
  • Do not add an ADR for this change. Non-root container operation is an infrastructure hygiene decision consistent with existing patterns, not an architectural decision with lasting structural consequences. The CIS reference in the issue title is sufficient documentation.
## 🏗️ Markus Keller — Application Architect ### Observations - The OCR service is the correct scope for this issue. It is explicitly justified as a separate service due to different resource requirements (8GB+ memory, GPU optional) and deployment cadence — this extraction has an ADR (ADR-001), and the single-node constraint is documented. - The `ocr_cache:/root/.cache` volume mount is the critical problem: once `USER ocr` is set, `/root/.cache` becomes inaccessible. The issue correctly identifies this. However, the two fix options (change mount path vs. set `HF_HOME=/app/cache`) have different operational implications worth spelling out. - `ensure_blla_model.py` uses `os.path.expanduser("~/.local/share/htrmopo")` as `HTRMOPO_DIR`. After switching to `USER ocr` (with `--no-create-home`), `~` expands to `/`, not `/home/ocr`. The blla download path will break silently unless `HTRMOPO_DIR` or `HOME` is explicitly set. This is an additional wrinkle the issue does not mention. - `read_only: true` in the compose block will conflict with `tempfile.TemporaryDirectory()` in the `/train` and `/train-sender` endpoints — both write to the system `/tmp`. The issue anticipates this: "if so, scope the override to specific writable mounts." - The `docs/architecture/c4/l2-containers.puml` describes the OCR service but does not document its security posture. This change does not add a new container, so no diagram update is strictly required per the doc matrix — but a comment noting the non-root runtime would be a worthwhile one-liner addition. - Backend and frontend Dockerfiles also lack `USER` directives — those are tracked separately in #134 and #135. That decomposition is correct; do not scope-creep into this issue. ### Recommendations - **Choose `HF_HOME=/app/cache` over changing the mount path.** It is more explicit (the variable name documents the intent), keeps all writable paths under `/app` (a single chown covers everything), and is immune to the `~` expansion problem with `--no-create-home`. - **Set `HOME=/app` in the Dockerfile** (or at minimum set `HTRMOPO_DIR` explicitly in `ensure_blla_model.py`) to prevent the silent `~` resolution failure when `kraken get` downloads the blla model. - **Add `tmpfs: [/tmp]` alongside `read_only: true`** rather than widening the read-only restriction. The training endpoints need `/tmp`; the tmpfs mount gives them a writable, non-persistent, memory-backed `/tmp` while keeping the rest of the filesystem read-only. - **Do not add an ADR for this change.** Non-root container operation is an infrastructure hygiene decision consistent with existing patterns, not an architectural decision with lasting structural consequences. The CIS reference in the issue title is sufficient documentation.
Author
Owner

👨‍💻 Felix Brandt — Fullstack Developer

Observations

  • The ocr-service/Dockerfile currently has no USER directive. uvicorn runs as UID 0. The fix is mechanical: add useradd, chown, USER — three lines.
  • entrypoint.sh is minimal (set -euo pipefail + python3 /app/ensure_blla_model.py + exec uvicorn). It will work unchanged as a non-root user provided all the paths it touches (/app/ensure_blla_model.py, /app/models/) are owned by the ocr user. The chmod +x /app/entrypoint.sh must happen before the USER directive — it already does in the proposed snippet, which is correct.
  • ensure_blla_model.py has a latent bug under --no-create-home: HTRMOPO_DIR = os.path.expanduser("~/.local/share/htrmopo") — with no home directory, ~ becomes / and the path resolves to /.local/share/htrmopo. The kraken get download will either fail or write to the filesystem root. This must be fixed as part of this issue, not deferred.
  • The training functions use tempfile.TemporaryDirectory(). tempfile defaults to TMPDIR env var, then /tmp. With read_only: true and no tmpfs, training will fail with a ReadOnlyError. The tmpfs: [/tmp] entry in compose is required for training to continue working.
  • main.py is clean Python: type hints on public functions, _ prefix on private helpers, Pydantic models, asyncio.to_thread() for CPU-bound work. No style issues introduced by this change.
  • The test_training_auth.py suite has good coverage of token auth for /train, /train-sender, and /segtrain. No test currently verifies that the process runs as UID 1000 — this is an infrastructure-layer test, not a Python unit test, and is correctly left to the verification steps in the issue rather than added to the test suite.

Recommendations

  • Fix ensure_blla_model.py in the same PR. Add HTRMOPO_DIR = os.path.expanduser(os.environ.get("HTRMOPO_DIR", "~/.local/share/htrmopo")) and set HOME=/home/ocr in the Dockerfile (creating the directory with mkdir -p /home/ocr && chown ocr:ocr /home/ocr) — or use HF_HOME=/app/cache and set HTRMOPO_DIR=/app/models/.htrmopo explicitly. The key point: don't leave it to ~ expansion with a no-home user.
  • Order the Dockerfile layers correctly: useraddchown -R ocr:ocr /appchmod +x /app/entrypoint.shUSER ocr. Running chmod after chown as root is fine; running it after USER ocr would require the file to already be owned by ocr.
  • The chown -R ocr:ocr /app layer will be large because it touches the PyTorch wheels and all installed packages. Accept this: it runs once per build and the security benefit justifies the layer cost. Do not try to merge it into an earlier layer — it must come after COPY . ..
  • Verify the ocr_models volume ownership on first run. Docker named volumes are owned by root by default. The chown in the Dockerfile only affects image layers, not the mounted volume. The service will fail to write models on first startup unless either: (a) the entrypoint script runs a one-time chown as root before dropping privileges (not possible with USER ocr), or (b) the volume is initialized with correct ownership. The cleanest solution is to mkdir -p /app/models && chown ocr:ocr /app/models in the Dockerfile — Docker will copy that ownership to a new named volume on first mount.
## 👨‍💻 Felix Brandt — Fullstack Developer ### Observations - The `ocr-service/Dockerfile` currently has no `USER` directive. `uvicorn` runs as UID 0. The fix is mechanical: add `useradd`, `chown`, `USER` — three lines. - `entrypoint.sh` is minimal (`set -euo pipefail` + `python3 /app/ensure_blla_model.py` + `exec uvicorn`). It will work unchanged as a non-root user provided all the paths it touches (`/app/ensure_blla_model.py`, `/app/models/`) are owned by the `ocr` user. The `chmod +x /app/entrypoint.sh` must happen _before_ the `USER` directive — it already does in the proposed snippet, which is correct. - `ensure_blla_model.py` has a latent bug under `--no-create-home`: `HTRMOPO_DIR = os.path.expanduser("~/.local/share/htrmopo")` — with no home directory, `~` becomes `/` and the path resolves to `/.local/share/htrmopo`. The `kraken get` download will either fail or write to the filesystem root. This must be fixed as part of this issue, not deferred. - The training functions use `tempfile.TemporaryDirectory()`. `tempfile` defaults to `TMPDIR` env var, then `/tmp`. With `read_only: true` and no `tmpfs`, training will fail with a `ReadOnlyError`. The `tmpfs: [/tmp]` entry in compose is required for training to continue working. - `main.py` is clean Python: type hints on public functions, `_` prefix on private helpers, Pydantic models, `asyncio.to_thread()` for CPU-bound work. No style issues introduced by this change. - The `test_training_auth.py` suite has good coverage of token auth for `/train`, `/train-sender`, and `/segtrain`. No test currently verifies that the process runs as UID 1000 — this is an infrastructure-layer test, not a Python unit test, and is correctly left to the verification steps in the issue rather than added to the test suite. ### Recommendations - **Fix `ensure_blla_model.py` in the same PR.** Add `HTRMOPO_DIR = os.path.expanduser(os.environ.get("HTRMOPO_DIR", "~/.local/share/htrmopo"))` and set `HOME=/home/ocr` in the Dockerfile (creating the directory with `mkdir -p /home/ocr && chown ocr:ocr /home/ocr`) — or use `HF_HOME=/app/cache` and set `HTRMOPO_DIR=/app/models/.htrmopo` explicitly. The key point: don't leave it to `~` expansion with a no-home user. - **Order the Dockerfile layers correctly:** `useradd` → `chown -R ocr:ocr /app` → `chmod +x /app/entrypoint.sh` → `USER ocr`. Running `chmod` after `chown` as root is fine; running it after `USER ocr` would require the file to already be owned by `ocr`. - **The `chown -R ocr:ocr /app` layer will be large** because it touches the PyTorch wheels and all installed packages. Accept this: it runs once per build and the security benefit justifies the layer cost. Do not try to merge it into an earlier layer — it must come after `COPY . .`. - **Verify the `ocr_models` volume ownership** on first run. Docker named volumes are owned by root by default. The `chown` in the Dockerfile only affects image layers, not the mounted volume. The service will fail to write models on first startup unless either: (a) the entrypoint script runs a one-time `chown` as root before dropping privileges (not possible with `USER ocr`), or (b) the volume is initialized with correct ownership. The cleanest solution is to `mkdir -p /app/models && chown ocr:ocr /app/models` in the Dockerfile — Docker will copy that ownership to a new named volume on first mount.
Author
Owner

🚀 Tobias Wendt — DevOps & Platform Engineer

Observations

  • The ocr_cache:/root/.cache mount is broken today for any process that drops root — but it also exists currently as root, which means it works now. After adding USER ocr, this mount will be readable but not writable (volume owned by root, no write permission for UID 1000). Every Hugging Face model download attempt will silently fail or raise a PermissionError. This is the highest-risk item in the change.
  • The ocr-service block in docker-compose.yml has no read_only, cap_drop, or security_opt — consistent with the other services (backend, frontend), which also lack these. This is a dev Compose file, so the absence is expected, but the issue is right to add them here since the OCR service is uniquely exposed to untrusted content.
  • minio:latest is still present. This is pre-existing and out of scope for this issue, but worth a note. Tracked separately.
  • The backend and frontend Dockerfiles also run as root (no USER directive). The backend image uses a multi-stage build with eclipse-temurin:21.0.10_7-jre-noble — the JRE base image version is pinned, which is good. The ocr-service base image python:3.11.9-slim is also pinned — good.
  • mem_limit: 12g and memswap_limit: 12g are set on the OCR service. These survive this change — the memory limits apply to the container regardless of the UID running inside.
  • The healthcheck on ocr-service uses curl -f http://localhost:8000/health. After switching to non-root, curl must remain in the image. It is currently installed via the apt-get install curl layer in the Dockerfile. No change required there.
  • docker-bench-security is referenced in the verification steps. This is a reasonable one-time check; it does not need to be added to CI for this project at current scale.

Recommendations

  • Use HF_HOME=/app/cache as the cache redirect strategy and update the compose volume to ocr_cache:/app/cache. This avoids any path involving /root and keeps all service-writable paths under /app. Add HF_HOME to the environment: block in compose and to the ENV directives in the Dockerfile so both container and build-time pip installations agree on the path.
  • Add the compose hardening block exactly as specified in the issueread_only: true, tmpfs: [/tmp], cap_drop: [ALL], security_opt: [no-new-privileges:true]. The tmpfs entry is non-negotiable: /train and /train-sender both use tempfile.TemporaryDirectory() which writes to /tmp.
  • Existing ocr_cache volumes on dev machines will have root ownership after this change. Document this in the PR description with the one-liner workaround: docker volume rm familienarchiv_ocr_cache (or equivalent) before docker compose up. Users will see a permission error on first startup otherwise.
  • Do not add read_only: true to the backend or frontend compose services in this PR. That is out of scope. This PR is OCR-only per the issue title. #134 and #135 cover the other services.

Open Decisions

  • tmpfs size limit. The default tmpfs in Docker Compose has no size limit (uses available memory). Training uploads can be large ZIP files (multi-MB). On a CX32 VPS with 8GB RAM and 12GB already reserved for OCR models, an uncapped tmpfs could cause OOM under concurrent training. Consider tmpfs: - /tmp:size=512m or similar. This needs a human judgment call on maximum expected training ZIP size.
## 🚀 Tobias Wendt — DevOps & Platform Engineer ### Observations - **The `ocr_cache:/root/.cache` mount is broken today** for any process that drops root — but it also exists currently as root, which means it works now. After adding `USER ocr`, this mount will be readable but not writable (volume owned by root, no write permission for UID 1000). Every Hugging Face model download attempt will silently fail or raise a `PermissionError`. This is the highest-risk item in the change. - The `ocr-service` block in `docker-compose.yml` has no `read_only`, `cap_drop`, or `security_opt` — consistent with the other services (backend, frontend), which also lack these. This is a dev Compose file, so the absence is expected, but the issue is right to add them here since the OCR service is uniquely exposed to untrusted content. - `minio:latest` is still present. This is pre-existing and out of scope for this issue, but worth a note. Tracked separately. - The backend and frontend Dockerfiles also run as root (no `USER` directive). The backend image uses a multi-stage build with `eclipse-temurin:21.0.10_7-jre-noble` — the JRE base image version is pinned, which is good. The `ocr-service` base image `python:3.11.9-slim` is also pinned — good. - `mem_limit: 12g` and `memswap_limit: 12g` are set on the OCR service. These survive this change — the memory limits apply to the container regardless of the UID running inside. - The `healthcheck` on `ocr-service` uses `curl -f http://localhost:8000/health`. After switching to non-root, `curl` must remain in the image. It is currently installed via the `apt-get install curl` layer in the Dockerfile. No change required there. - `docker-bench-security` is referenced in the verification steps. This is a reasonable one-time check; it does not need to be added to CI for this project at current scale. ### Recommendations - **Use `HF_HOME=/app/cache` as the cache redirect strategy** and update the compose volume to `ocr_cache:/app/cache`. This avoids any path involving `/root` and keeps all service-writable paths under `/app`. Add `HF_HOME` to the `environment:` block in compose and to the `ENV` directives in the Dockerfile so both container and build-time pip installations agree on the path. - **Add the compose hardening block exactly as specified in the issue** — `read_only: true`, `tmpfs: [/tmp]`, `cap_drop: [ALL]`, `security_opt: [no-new-privileges:true]`. The `tmpfs` entry is non-negotiable: `/train` and `/train-sender` both use `tempfile.TemporaryDirectory()` which writes to `/tmp`. - **Existing `ocr_cache` volumes on dev machines will have root ownership after this change.** Document this in the PR description with the one-liner workaround: `docker volume rm familienarchiv_ocr_cache` (or equivalent) before `docker compose up`. Users will see a permission error on first startup otherwise. - **Do not add `read_only: true` to the backend or frontend compose services in this PR.** That is out of scope. This PR is OCR-only per the issue title. #134 and #135 cover the other services. ### Open Decisions - **`tmpfs` size limit.** The default `tmpfs` in Docker Compose has no size limit (uses available memory). Training uploads can be large ZIP files (multi-MB). On a CX32 VPS with 8GB RAM and 12GB already reserved for OCR models, an uncapped tmpfs could cause OOM under concurrent training. Consider `tmpfs: - /tmp:size=512m` or similar. This needs a human judgment call on maximum expected training ZIP size.
Author
Owner

📋 Elicit — Requirements Engineer

Observations

  • The issue is specification-dense and well-structured. It clearly identifies: the threat model, the affected component, the proposed approach with code snippets, critical files, and a concrete five-step verification procedure. The acceptance criteria are binary and testable. Effort is sized (S). This meets the Definition of Ready.
  • The acceptance criterion "Training endpoint still works (token-protected per ADR-001)" is correct but slightly underspecified. There are three training endpoints: /train, /train-sender, and /segtrain (present in test_training_auth.py). The AC should reference all three, or be written as "all training endpoints still work."
  • The verification step 5 ("trigger a real OCR on a small PDF via the backend") is underspecified for automated acceptance: it does not name the specific PDF, expected output format, or what "streaming response completes without permission errors" means in terms of observable HTTP status. For manual verification this is fine; for a future automated smoke test it would need tightening.
  • The issue correctly cross-references #134 (backend) and #135 (frontend) as related issues. This decomposition is good scope hygiene. However, those issues do not yet appear to exist in the tracker — worth confirming they are created before this one is closed, so the complete hardening picture is tracked.
  • "Persisted model cache survives container recreate" is an acceptance criterion but depends on correct volume ownership — a runtime condition, not a code condition. This is the most likely failure mode and deserves an explicit verification step (e.g., docker compose stop ocr-service && docker compose up -d ocr-service and confirm healthcheck passes).

Recommendations

  • Expand AC "Training endpoint still works" to explicitly name all three endpoints: /train, /train-sender, and /segtrain. This prevents inadvertently shipping a broken training endpoint that was not in the tester's mental model.
  • Add a verification step for model cache persistence: stop and recreate the container, then confirm that docker exec archive-ocr ls -la /app/models/ shows models present and no re-download occurs in the logs.
  • Confirm that issues #134 and #135 exist (or create them) before closing this issue. The audit document references F-06 as Critical — the full remediation picture needs to be tracked for Production v1.
## 📋 Elicit — Requirements Engineer ### Observations - The issue is specification-dense and well-structured. It clearly identifies: the threat model, the affected component, the proposed approach with code snippets, critical files, and a concrete five-step verification procedure. The acceptance criteria are binary and testable. Effort is sized (S). This meets the Definition of Ready. - The acceptance criterion "Training endpoint still works (token-protected per ADR-001)" is correct but slightly underspecified. There are **three** training endpoints: `/train`, `/train-sender`, and `/segtrain` (present in `test_training_auth.py`). The AC should reference all three, or be written as "all training endpoints still work." - The verification step 5 ("trigger a real OCR on a small PDF via the backend") is underspecified for automated acceptance: it does not name the specific PDF, expected output format, or what "streaming response completes without permission errors" means in terms of observable HTTP status. For manual verification this is fine; for a future automated smoke test it would need tightening. - The issue correctly cross-references #134 (backend) and #135 (frontend) as related issues. This decomposition is good scope hygiene. However, those issues do not yet appear to exist in the tracker — worth confirming they are created before this one is closed, so the complete hardening picture is tracked. - "Persisted model cache survives container recreate" is an acceptance criterion but depends on correct volume ownership — a runtime condition, not a code condition. This is the most likely failure mode and deserves an explicit verification step (e.g., `docker compose stop ocr-service && docker compose up -d ocr-service` and confirm healthcheck passes). ### Recommendations - **Expand AC "Training endpoint still works" to explicitly name all three endpoints:** `/train`, `/train-sender`, and `/segtrain`. This prevents inadvertently shipping a broken training endpoint that was not in the tester's mental model. - **Add a verification step for model cache persistence:** stop and recreate the container, then confirm that `docker exec archive-ocr ls -la /app/models/` shows models present and no re-download occurs in the logs. - **Confirm that issues #134 and #135 exist** (or create them) before closing this issue. The audit document references F-06 as Critical — the full remediation picture needs to be tracked for Production v1.
Author
Owner

🔐 Nora "NullX" Steiner — Security Engineer

Observations

  • Root-in-container is the confirmed vulnerability. CWE-250 (Execution with Unnecessary Privileges). The OCR service is the only component in the stack that directly ingests attacker-controlled content (user-uploaded PDFs) into native C libraries (libvips, opencv, PyTorch, pypdfium2). RCE surface in these libraries is real and historically exploited. Running as root maximizes the blast radius.
  • The training endpoints are token-protected (_check_training_token) and the tests in test_training_auth.py verify this correctly — including fail-closed behavior when TRAINING_TOKEN is empty. The non-root change does not affect this control.
  • subprocess.run(["ketos", ...]) uses list form with no shell=True. This is correct and remains safe under non-root.
  • _validate_url() implements SSRF protection via ALLOWED_PDF_HOSTS. This control is unaffected by the UID change.
  • _validate_zip_entry() implements ZIP Slip protection. This control is unaffected.
  • cap_drop: [ALL] is the highest-value hardening in the compose block. Dropping all Linux capabilities prevents privilege escalation attacks even from within the container. A container-escape exploit that requires CAP_SYS_ADMIN (common) is blocked by this. Combined with no-new-privileges:true, even a setuid binary in the image cannot escalate.
  • read_only: true significantly reduces post-exploitation persistence options. An attacker who achieves RCE cannot write a backdoor to the filesystem; any written files disappear on container restart. The tmpfs /tmp is memory-only and also non-persistent.
  • The ocr_cache volume changing from /root/.cache to /app/cache (or similar) should be done atomically: if the volume still mounts at /root/.cache after the UID switch, Hugging Face model downloads silently use a root-owned directory. The process has read access (world-readable cache files) but not write access — this means model downloads that succeed for root will silently fail for the ocr user. This is a security-adjacent reliability issue: a confused process that partially reads a root-owned cache could load stale or corrupted model state.
  • The ensure_blla_model.py path issue (HTRMOPO_DIR = os.path.expanduser("~/.local/share/htrmopo") expanding to /.local/... under --no-create-home) is not a security vulnerability, but it is a startup reliability issue. If the blla model cannot be validated, the container starts in a degraded state that could mask other errors.

Recommendations

  • Add cap_drop: [ALL] first, then confirm OCR and training work. Some capability drops are surprising — CAP_SETUID, CAP_SETGID are already absent for non-root, but CAP_CHOWN may be needed during container init if any setup script calls chown. Confirm by testing, not by assumption.
  • Add a regression test that asserts id output is not uid=0(root) — or more practically, add a startup check to main.py: if os.getuid() == 0: logger.warning("Running as root — CIS Docker §4.1 violation"). This produces an observable signal in logs during verification and acts as a canary if the USER directive is accidentally removed in a future Dockerfile edit.
  • Do not add CAP_NET_RAW back. The OCR service makes outbound HTTP requests (to MinIO) but uses httpx over standard TCP — no raw socket operations needed.
  • The TRAINING_TOKEN empty-string check (_check_training_token) returns 503 when the token is not configured. This is fail-closed and correct. Verify that this behavior is preserved after the UID change (it will be — it is application logic, not filesystem permission logic).
  • Write a verification script for the docker-bench-security step that can be re-run after any Dockerfile change. Even a one-line wrapper around the CIS check makes future audits faster.
## 🔐 Nora "NullX" Steiner — Security Engineer ### Observations - **Root-in-container is the confirmed vulnerability.** CWE-250 (Execution with Unnecessary Privileges). The OCR service is the only component in the stack that directly ingests attacker-controlled content (user-uploaded PDFs) into native C libraries (libvips, opencv, PyTorch, pypdfium2). RCE surface in these libraries is real and historically exploited. Running as root maximizes the blast radius. - The training endpoints are token-protected (`_check_training_token`) and the tests in `test_training_auth.py` verify this correctly — including fail-closed behavior when `TRAINING_TOKEN` is empty. The non-root change does not affect this control. - `subprocess.run(["ketos", ...])` uses list form with no `shell=True`. This is correct and remains safe under non-root. - `_validate_url()` implements SSRF protection via `ALLOWED_PDF_HOSTS`. This control is unaffected by the UID change. - `_validate_zip_entry()` implements ZIP Slip protection. This control is unaffected. - **`cap_drop: [ALL]` is the highest-value hardening in the compose block.** Dropping all Linux capabilities prevents privilege escalation attacks even from within the container. A container-escape exploit that requires `CAP_SYS_ADMIN` (common) is blocked by this. Combined with `no-new-privileges:true`, even a setuid binary in the image cannot escalate. - **`read_only: true` significantly reduces post-exploitation persistence options.** An attacker who achieves RCE cannot write a backdoor to the filesystem; any written files disappear on container restart. The tmpfs `/tmp` is memory-only and also non-persistent. - The `ocr_cache` volume changing from `/root/.cache` to `/app/cache` (or similar) should be done atomically: if the volume still mounts at `/root/.cache` after the UID switch, Hugging Face model downloads silently use a root-owned directory. The process has read access (world-readable cache files) but not write access — this means model downloads that succeed for root will silently fail for the `ocr` user. This is a security-adjacent reliability issue: a confused process that partially reads a root-owned cache could load stale or corrupted model state. - The `ensure_blla_model.py` path issue (`HTRMOPO_DIR = os.path.expanduser("~/.local/share/htrmopo")` expanding to `/.local/...` under `--no-create-home`) is not a security vulnerability, but it is a startup reliability issue. If the blla model cannot be validated, the container starts in a degraded state that could mask other errors. ### Recommendations - **Add `cap_drop: [ALL]` first, then confirm OCR and training work.** Some capability drops are surprising — `CAP_SETUID`, `CAP_SETGID` are already absent for non-root, but `CAP_CHOWN` may be needed during container init if any setup script calls `chown`. Confirm by testing, not by assumption. - **Add a regression test** that asserts `id` output is not `uid=0(root)` — or more practically, add a startup check to `main.py`: `if os.getuid() == 0: logger.warning("Running as root — CIS Docker §4.1 violation")`. This produces an observable signal in logs during verification and acts as a canary if the `USER` directive is accidentally removed in a future Dockerfile edit. - **Do not add `CAP_NET_RAW` back.** The OCR service makes outbound HTTP requests (to MinIO) but uses `httpx` over standard TCP — no raw socket operations needed. - **The `TRAINING_TOKEN` empty-string check** (`_check_training_token`) returns 503 when the token is not configured. This is fail-closed and correct. Verify that this behavior is preserved after the UID change (it will be — it is application logic, not filesystem permission logic). - **Write a verification script for the docker-bench-security step** that can be re-run after any Dockerfile change. Even a one-line wrapper around the CIS check makes future audits faster.
Author
Owner

🧪 Sara Holt — QA Engineer

Observations

  • The existing test suite for the OCR service is well-structured: test_training_auth.py (token auth), test_stream.py (streaming), test_confidence.py, test_engines.py, test_preprocessing.py. All use AsyncClient + ASGITransport for in-process API testing — correct pattern.
  • No test currently verifies that the service runs as a non-root user. This is expected — UID verification is an infrastructure concern, not a unit-test concern. The verification procedure in the issue (docker exec archive-ocr id) is the right approach.
  • The most likely regression from this change is a file-permission error on startup — specifically when ensure_blla_model.py tries to write to a volume directory owned by root. This failure mode is silent in the test suite (tests mock the engine, not the filesystem). The issue's verification step 4 (healthcheck green) catches this, but only if the healthcheck's start_period: 120s is long enough to observe a permission-error crash and retry.
  • test_training_auth.py tests the /train, /train-sender, and /segtrain endpoints for authentication. After this change, these same tests must continue to pass — they will, since they mock filesystem operations. A manual E2E training run (verification step 5) is still required.
  • No test exercises the ensure_blla_model.py path when running as non-root. test_ensure_blla_model.py exists but mocks subprocess.run — it does not validate the ~ expansion or HTRMOPO_DIR resolution under a non-root user. This gap is acceptable to leave as a manual verification step for this issue, but it should be noted.
  • The healthcheck retries: 12 with interval: 10s gives 120s of retry time. Given that model loading takes 30–50s on cold start, this is appropriate. After the UID change, a permission error on the model volume would cause uvicorn to start but _models_ready to remain False, returning 503 on /health. The healthcheck would eventually mark the container unhealthy. This is correct behavior — fail loudly.

Recommendations

  • Add one integration-level test to test_ensure_blla_model.py that asserts HTRMOPO_DIR resolution is not root-dependent: mock os.path.expanduser to return a temp directory and verify that _download_blla resolves the path correctly. This guards against the --no-create-home regression being silently reintroduced.
  • Add a verification checklist item: after container recreate (not just restart), confirm that docker compose logs ocr-service | grep "blla model OK" appears and no PermissionError lines appear. This catches the volume ownership issue that unit tests cannot.
  • The acceptance criterion "Existing OCR end-to-end flow still works" is correct but needs a concrete test artifact. Use a small, known-good PDF (1–2 pages) stored in the repo under ocr-service/testdata/ or similar, and document the exact command to trigger it in the verification steps. This makes the verification reproducible by anyone, not just the implementer.
  • Do not add a UID check to the unit test suite. os.getuid() in a test is environment-specific and will break CI runs that do not honor the USER directive. Keep UID verification in the operational checklist, not the automated suite.
## 🧪 Sara Holt — QA Engineer ### Observations - The existing test suite for the OCR service is well-structured: `test_training_auth.py` (token auth), `test_stream.py` (streaming), `test_confidence.py`, `test_engines.py`, `test_preprocessing.py`. All use `AsyncClient` + `ASGITransport` for in-process API testing — correct pattern. - **No test currently verifies that the service runs as a non-root user.** This is expected — UID verification is an infrastructure concern, not a unit-test concern. The verification procedure in the issue (`docker exec archive-ocr id`) is the right approach. - **The most likely regression from this change is a file-permission error on startup** — specifically when `ensure_blla_model.py` tries to write to a volume directory owned by root. This failure mode is silent in the test suite (tests mock the engine, not the filesystem). The issue's verification step 4 (healthcheck green) catches this, but only if the healthcheck's `start_period: 120s` is long enough to observe a permission-error crash and retry. - `test_training_auth.py` tests the `/train`, `/train-sender`, and `/segtrain` endpoints for authentication. After this change, these same tests must continue to pass — they will, since they mock filesystem operations. A manual E2E training run (verification step 5) is still required. - **No test exercises the `ensure_blla_model.py` path when running as non-root.** `test_ensure_blla_model.py` exists but mocks `subprocess.run` — it does not validate the `~` expansion or `HTRMOPO_DIR` resolution under a non-root user. This gap is acceptable to leave as a manual verification step for this issue, but it should be noted. - The healthcheck `retries: 12` with `interval: 10s` gives 120s of retry time. Given that model loading takes 30–50s on cold start, this is appropriate. After the UID change, a permission error on the model volume would cause uvicorn to start but `_models_ready` to remain `False`, returning 503 on `/health`. The healthcheck would eventually mark the container unhealthy. This is correct behavior — fail loudly. ### Recommendations - **Add one integration-level test to `test_ensure_blla_model.py`** that asserts `HTRMOPO_DIR` resolution is not root-dependent: mock `os.path.expanduser` to return a temp directory and verify that `_download_blla` resolves the path correctly. This guards against the `--no-create-home` regression being silently reintroduced. - **Add a verification checklist item:** after container recreate (not just restart), confirm that `docker compose logs ocr-service | grep "blla model OK"` appears and no `PermissionError` lines appear. This catches the volume ownership issue that unit tests cannot. - **The acceptance criterion "Existing OCR end-to-end flow still works"** is correct but needs a concrete test artifact. Use a small, known-good PDF (1–2 pages) stored in the repo under `ocr-service/testdata/` or similar, and document the exact command to trigger it in the verification steps. This makes the verification reproducible by anyone, not just the implementer. - **Do not add a UID check to the unit test suite.** `os.getuid()` in a test is environment-specific and will break CI runs that do not honor the `USER` directive. Keep UID verification in the operational checklist, not the automated suite.
Author
Owner

🎨 Leonie Voss — UX Design Lead

Observations

This is a pure infrastructure security change with no frontend UI impact. No Svelte components, no routes, no styling, and no user-facing behavior are affected. The OCR service runs as an internal microservice on the Docker network — users interact with it only indirectly through the Java backend's OCR trigger and streaming progress UI.

One indirect UX concern worth flagging: if the non-root change causes a startup permission error, the OCR service will fail its healthcheck and the backend's depends_on: ocr-service: condition: service_started (note: service_started, not service_healthy) means the backend starts regardless. Users who trigger OCR while the OCR service is recovering from a permission error will see a failure state in the OCR progress UI. This is not a new failure mode — it exists today — but the non-root change introduces a new class of startup failure that did not exist before. The UI already handles this (503 from the OCR service surfaces as an error state in the progress indicator); no UI changes are needed.

Recommendations

  • No UI changes required.
  • Confirm that OCR error states (503 from OCR service) are handled gracefully in the existing streaming UI — this is pre-existing behavior and should already be covered, but worth a manual smoke-test after the change.
## 🎨 Leonie Voss — UX Design Lead ### Observations This is a pure infrastructure security change with no frontend UI impact. No Svelte components, no routes, no styling, and no user-facing behavior are affected. The OCR service runs as an internal microservice on the Docker network — users interact with it only indirectly through the Java backend's OCR trigger and streaming progress UI. One indirect UX concern worth flagging: if the non-root change causes a startup permission error, the OCR service will fail its healthcheck and the backend's `depends_on: ocr-service: condition: service_started` (note: `service_started`, not `service_healthy`) means the backend starts regardless. Users who trigger OCR while the OCR service is recovering from a permission error will see a failure state in the OCR progress UI. This is not a new failure mode — it exists today — but the non-root change introduces a new class of startup failure that did not exist before. The UI already handles this (503 from the OCR service surfaces as an error state in the progress indicator); no UI changes are needed. ### Recommendations - No UI changes required. - Confirm that OCR error states (503 from OCR service) are handled gracefully in the existing streaming UI — this is pre-existing behavior and should already be covered, but worth a manual smoke-test after the change.
Author
Owner

📬 Decision Queue

One open decision was raised across the reviews — needs a human call before implementation:


Theme: tmpfs size cap for /tmp

Raised by: Tobias Wendt (@tobiwendt)

The tmpfs: [/tmp] mount required for training is uncapped by default. Docker will allow it to consume all available memory. On the CX32 VPS (8GB RAM, with 12GB already allocated to the OCR container via mem_limit), an uncapped /tmp under concurrent training could cause OOM and crash the container.

The tradeoff:

  • No size cap — simpler config, no risk of training failing due to tmpfs exhaustion on a large ZIP, but exposes the host to runaway memory use if a malformed or unusually large training upload is processed.
  • Capped tmpfs (e.g., size=512m) — limits blast radius, but requires knowing the maximum expected training ZIP size. If the cap is too small, /train will fail mid-run with a no-space error.

What's needed: a human decision on the maximum expected training ZIP file size. Training ZIPs contain .png scan images and .gt.txt ground-truth pairs — a typical batch might be 20–50 images at 1–5MB each, so 100MB–250MB is a plausible upper bound. A 512MB cap with a 10–20% safety margin would cover typical usage.

Suggested resolution: cap at 512m with a comment explaining the rationale, and log a warning in _run_training() if the extracted ZIP contents exceed a soft threshold (e.g., 400MB). Revisit if training datasets grow larger.

## 📬 Decision Queue One open decision was raised across the reviews — needs a human call before implementation: --- ### Theme: tmpfs size cap for `/tmp` **Raised by:** Tobias Wendt (@tobiwendt) The `tmpfs: [/tmp]` mount required for training is uncapped by default. Docker will allow it to consume all available memory. On the CX32 VPS (8GB RAM, with 12GB already allocated to the OCR container via `mem_limit`), an uncapped `/tmp` under concurrent training could cause OOM and crash the container. **The tradeoff:** - **No size cap** — simpler config, no risk of training failing due to tmpfs exhaustion on a large ZIP, but exposes the host to runaway memory use if a malformed or unusually large training upload is processed. - **Capped tmpfs (e.g., `size=512m`)** — limits blast radius, but requires knowing the maximum expected training ZIP size. If the cap is too small, `/train` will fail mid-run with a no-space error. **What's needed:** a human decision on the maximum expected training ZIP file size. Training ZIPs contain `.png` scan images and `.gt.txt` ground-truth pairs — a typical batch might be 20–50 images at 1–5MB each, so 100MB–250MB is a plausible upper bound. A 512MB cap with a 10–20% safety margin would cover typical usage. **Suggested resolution:** cap at `512m` with a comment explaining the rationale, and log a warning in `_run_training()` if the extracted ZIP contents exceed a soft threshold (e.g., 400MB). Revisit if training datasets grow larger.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#459