diff --git a/.dockerignore b/.dockerignore index c2ac49491c..952c331c09 100644 --- a/.dockerignore +++ b/.dockerignore @@ -38,6 +38,12 @@ schema.graphql cloudflare-og-worker/ tools/ fixtures/ +# locale/ is safe to exclude: no .po/.mo files exist in the tree (only a +# placeholder README.rst) and no entrypoint/start script invokes +# `manage.py compilemessages`. Django translations are intentionally not +# active today; if .po files are added later, drop this entry so the build +# context picks them up (the runtime image keeps `gettext` available for +# `compilemessages`). locale/ model_preloaders/ diff --git a/.github/workflows/docker-build-release.yml b/.github/workflows/docker-build-release.yml index d7021e1dbb..abaede4932 100644 --- a/.github/workflows/docker-build-release.yml +++ b/.github/workflows/docker-build-release.yml @@ -56,19 +56,26 @@ jobs: type=semver,pattern={{major}} type=sha,prefix={{tag}}- - - name: Build and push Docker image - id: build + # Django gets a two-step build: first build+load locally so the image + # is on the daemon for the size-budget gate, then `docker push` each + # tag separately. This is split intentionally — combining `load: true` + # with `push: true` in a single `docker/build-push-action` step works + # only on single-platform builds, is not well-documented across Buildx + # versions, and some runner/daemon configurations reject it. Splitting + # makes the contract explicit and resilient to future runner changes. + - name: Build Django image (load locally for size budget gate) + if: matrix.image == 'django' uses: docker/build-push-action@v7 with: context: ${{ matrix.context }} file: ${{ matrix.dockerfile }} - push: true + push: false + load: true tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} cache-from: type=gha cache-to: type=gha,mode=max platforms: linux/amd64 - load: ${{ matrix.image == 'django' }} build-args: | BUILD_ENVIRONMENT=production @@ -94,3 +101,33 @@ jobs: echo "::error::Django image $human_size exceeds the $human_budget budget. See issue #1494." exit 1 fi + + # Push only after the size gate passes. Each tag in steps.meta.outputs.tags + # already points at the image loaded above (same reference Buildx tagged + # locally), so `docker push ` re-uses the local layers — no rebuild. + - name: Push Django image to registry + if: matrix.image == 'django' + run: | + set -euo pipefail + while IFS= read -r tag; do + [ -z "$tag" ] && continue + echo "Pushing $tag" + docker push "$tag" + done <<< "${{ steps.meta.outputs.tags }}" + + # Non-Django images do not have a size budget today, so the original + # combined build-and-push path stays — no `load` involved. + - name: Build and push non-Django image + if: matrix.image != 'django' + uses: docker/build-push-action@v7 + with: + context: ${{ matrix.context }} + file: ${{ matrix.dockerfile }} + push: true + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + cache-from: type=gha + cache-to: type=gha,mode=max + platforms: linux/amd64 + build-args: | + BUILD_ENVIRONMENT=production diff --git a/CHANGELOG.md b/CHANGELOG.md index d991a1edc8..76622516d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Docker image hardening follow-ups to PR #1500** (issue #1505). Six review observations on the slim-production-image PR are addressed without altering the deployed runtime semantics: + - **Python pinned to `3.11.15-slim-bookworm`** in both `compose/production/django/Dockerfile` and `compose/local/django/Dockerfile` (was the floating `3.11-slim-bookworm` alias). Rebuilds on different dates can no longer silently pick up a new CPython point release that changes runtime behaviour. Bumps are now an intentional edit, not a side effect of the calendar. + - **spaCy model install switched to the pinned-URL `wget` pattern in production** (`compose/production/django/Dockerfile`). The runtime stage previously called `python -m spacy download en_core_web_sm` / `en_core_web_lg`, which resolves the wheel URL at build time and could silently drift to a different patch. The production build stage now downloads the same explicit GitHub release URLs already used in `compose/local/django/Dockerfile` into `/usr/src/app/wheels`, with the same retry semantics; the runtime stage installs them via the existing `pip install --no-index --find-links=/wheels/` path and runs a `spacy.load(...)` smoke test. `wget` is added to the build stage only — it never reaches the runtime image. + - **Release workflow split for the Django image** (`.github/workflows/docker-build-release.yml`). The previous step combined `load: true` (so the size-budget gate could `docker image inspect` the result) with `push: true`. That combination only works on single-platform builds, is not well-documented across Buildx versions, and some runner/daemon configurations reject it. The Django image is now built with `load: true, push: false`, gated on the 1.5 GiB size budget, then pushed via `docker push ` per tag — the daemon already has the layers from the load step, so no rebuild occurs. Non-Django images keep the original combined `push: true, load: false` path. + - **`.dockerignore` documents why `locale/` is safely excluded** — the tree contains no `.po`/`.mo` files (only a placeholder `README.rst`) and no `compose/*/start*` script invokes `manage.py compilemessages`. The runtime image still installs `gettext` so future i18n work has a build hook ready; if `.po` files are added, drop the `locale/` line so the build context picks them up. + - **`brotli>=1.1.0` floor rationale documented** in `requirements/base.txt`. The constraint was not chosen incidentally — 1.1.0 is the current major-version line under active upstream maintenance. The underlying `_brotli` C-extension API that `httpx` looks up has been stable since 1.0.x, so a future transitive constraint can lower the floor safely if needed. + - **`ffmpeg` removal verified** by an exhaustive `grep` across `opencontractserver/`, `compose/`, `config/`, `frontend/`, and the workflow tree: no Python `import`, `subprocess`, or shell invocation of `ffmpeg` exists. The PR #1500 removal of the apt package is safe; this entry records the verification trail rather than re-removing anything. + - **SHA-256 checksums added for spaCy wheel downloads** in both `compose/production/django/Dockerfile` and `compose/local/django/Dockerfile`. Pinning to an explicit release URL is good but without a hash check the build is still vulnerable to a GitHub release being retroactively modified or a CDN MITM. Each `wget` step is now followed by a `sha256sum -c` verification that fails the build immediately if the fetched wheel doesn't match the expected digest. + - **Size-budget gate confirmed intact** after the `id: build` step-ID removal. The gate at `.github/workflows/docker-build-release.yml:93` uses `${{ steps.meta.outputs.tags }}` (not the removed `steps.build.outputs.imageid`) to identify the locally-loaded image for `docker image inspect`, so the split build path works correctly. + - **Graduated remaining `config.graphql.*` modules from the mypy baseline** (issue #1485, sub-issue of #1447). Removed the per-module `ignore_errors` blocks for the final 14 modules (`annotation_mutations`, `annotation_queries`, `base`, `conversation_queries`, `corpus_mutations`, `document_mutations`, `document_types`, `extract_mutations`, `extract_queries`, `label_mutations`, `permissioning.permission_annotator.{middleware,mixins,utils}`, `social_queries`) and pruned 174 corresponding lines from `docs/typing/mypy_baseline.txt`. The entire `config.graphql.*` surface is now type-checked. Notable code changes made in service of typing: - `AnnotatePermissionsForReadMixin` declares `_meta` / `is_public` under `TYPE_CHECKING` so the mixin no longer trips `attr-defined` errors when combined with a `DjangoObjectType` (`config/graphql/permissioning/permission_annotator/mixins.py`). - All `from_global_id(kwargs.get("id", None))[1]` Relay node resolver patterns rewritten to `from_global_id(kwargs["id"])[1]` (the `id` arg is required for relay, so the `.get(..., None)` was dead defensiveness that produced an `Any | None` mypy can't accept). @@ -22,7 +32,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Consolidated corpus-modify permission checks behind `user_can_modify_corpus`** (`opencontractserver/utils/permissioning.py`, refs #1450). Four GraphQL mutations were repeating the same inline `corpus.creator == user or user_has_permission_for_obj(user, corpus, UPDATE, include_group_permissions=True)` check (`config/graphql/badge_mutations.py:91, 211, 328` and `config/graphql/document_mutations.py:416`). The pattern is the cluster-2 anti-pattern flagged in the issue: as the permission model evolves (sharing, group inheritance, MIN(doc, corpus)) inline copies drift apart. Replaced each with `user_can_modify_corpus(user, corpus)`, which encodes the canonical "is_superuser OR creator OR explicit guardian UPDATE" semantics in one place. Behaviorally identical: superuser was already admitted via `user_has_permission_for_obj` (returns True for superusers on guardian-enabled models); the helper just makes the short-circuit explicit. Anonymous and `None` callers correctly return `False`. New tests in `opencontractserver/tests/test_user_can_modify_corpus.py` pin the matrix (superuser, creator, user-level UPDATE, group-level UPDATE, group-perm-disabled, outsider, anonymous, `None`, id-by-int, id-by-str). Out of scope for this consolidation PR (flagged for separate review per the "consolidation PRs should not also be behavior-change PRs" rule): `corpus_mutations.py` `UpdateCorpusDescription`/`ReEmbedCorpus` (intentionally creator-only per docstring), `corpus_mutations.py` `CreateCorpusAction`/`AddTemplateToCorpus` (use `PermissionTypes.CRUD` not UPDATE — switching widens access), `worker_mutations.py:154` (no group-perm fallback today). Also removed two stale `resolve_oc_model_queryset` references from `opencontractserver/shared/Managers.py:139` and the docstring of `opencontractserver/tests/test_visibility_managers.py` — that helper has no live callers and visibility lives in the manager method now. - **Graduated `opencontractserver.pipeline.*` from the mypy baseline** (issue #1483, refs #1447, #1331). All fourteen pipeline modules — the `base` package (`base_component`, `chunked_parser`, `embedder`, `parser`, `post_processor`), the embedders (`multimodal_microservice`, `openai_embedder`, `sent_transformer_microservice`), the parsers (`docling_parser_rest`, `docxodus_parser`, `llamaparse_parser`, `oc_markdown_parser`, `oc_text_parser`), `post_processors.pdf_redactor`, plus `pipeline.registry` and `pipeline.utils` — are now type-checked from scratch (103 baseline entries removed from `docs/typing/mypy_baseline.txt`). Notable changes that surfaced from the graduation: shared component metadata (`title`, `description`, `author`, `dependencies`, `input_schema`) is now declared on `PipelineComponentBase` so `type[PipelineComponentBase]` lookups in `pipeline.utils.get_metadata_for_component` resolve attributes through the base class instead of bare `type`; `BasePostProcessor` now declares `supported_file_types: list[FileTypeEnum]` (previously accessed but never declared on the base); `BaseMultimodalMicroserviceEmbedder.url_setting_name` is now a typed class attribute (referenced in error messages, never declared); `pipeline.utils.run_post_processors` validates that the class loaded by `get_component_by_name` is actually a `BasePostProcessor` subclass before calling `process_export`. Image-token construction in `docling_parser_rest` and `llamaparse_parser` only sets optional `PawlsTokenPythonType` fields when present, eliminating spurious `None` writes against `NotRequired`-but-non-`None` slots. `llamaparse_parser._build_image_token` extracted as a shared helper. No runtime behaviour change; all interactions with broader union types are preserved through targeted `cast()` calls at the boundary. - - **Type-checking: graduated `opencontractserver.utils.*` chunk 1 from mypy baseline** (issue #1480, sub-issue of #1447). Removed `[mypy-…]` ignore blocks for `utils.analysis`, `utils.analyzer`, `utils.cleanup`, `utils.corpus_forking`, `utils.embeddings`, and `utils.etl` and fixed all surfaced errors. Notable production fixes uncovered by typing: - `opencontractserver/utils/cleanup.py:34` — `delete_analysis_and_annotations` was calling `Analysis.annotations.filter(...)` on the class-level reverse descriptor (would fail at runtime with `AttributeError`); now correctly uses `analysis.annotations.all().delete()`. - `opencontractserver/utils/analyzer.py:204` — `run_analysis` now raises `ValueError` early when `analyzer.host_gremlin` is `None` instead of crashing on `gremlin.url` later. diff --git a/compose/local/django/Dockerfile b/compose/local/django/Dockerfile index 3c1f51e40b..e16d96d4bd 100644 --- a/compose/local/django/Dockerfile +++ b/compose/local/django/Dockerfile @@ -1,4 +1,7 @@ -ARG PYTHON_VERSION=3.11-slim-bookworm +# Pin to a specific patch (not the floating 3.11 alias) so rebuilds on +# different dates do not silently pick up a new CPython point release. +# Keep this in sync with compose/production/django/Dockerfile. +ARG PYTHON_VERSION=3.11.15-slim-bookworm # ---------------------------------------------------------------------------- # # Build stage: full toolchain for compiling wheels (psycopg2, opencv, etc.) # @@ -98,16 +101,24 @@ RUN pip install --upgrade pip \ # Smoke test: `import cv2` must succeed (see production Dockerfile for context). RUN python -c "import cv2; print(f'cv2 {cv2.__version__} import OK')" -# Download spacy models with retry logic. Using pip directly is more reliable -# than the `spacy download` command in Docker. -RUN pip install https://github.com/explosion/spacy-models/releases/download/en_core_web_sm-3.8.0/en_core_web_sm-3.8.0-py3-none-any.whl \ +# Download spacy models with retry logic and SHA-256 verification. +# Hashes are for the pinned 3.8.0 release wheels from the explosion/spacy-models +# GitHub releases page. Update hashes when bumping the model version. +RUN wget --retry-connrefused --waitretry=1 --read-timeout=20 --timeout=15 -t 5 \ + -O /tmp/en_core_web_sm-3.8.0-py3-none-any.whl \ + https://github.com/explosion/spacy-models/releases/download/en_core_web_sm-3.8.0/en_core_web_sm-3.8.0-py3-none-any.whl \ + && echo "1932429db727d4bff3deed6b34cfc05df17794f4a52eeb26cf8928f7c1a0fb85 /tmp/en_core_web_sm-3.8.0-py3-none-any.whl" | sha256sum -c - \ + && pip install /tmp/en_core_web_sm-3.8.0-py3-none-any.whl \ + && rm /tmp/en_core_web_sm-3.8.0-py3-none-any.whl \ && python -c "import spacy; spacy.load('en_core_web_sm')" # For the large model, use wget with retry to ensure the download completes. RUN wget --retry-connrefused --waitretry=1 --read-timeout=20 --timeout=15 -t 5 \ + -O /tmp/en_core_web_lg-3.8.0-py3-none-any.whl \ https://github.com/explosion/spacy-models/releases/download/en_core_web_lg-3.8.0/en_core_web_lg-3.8.0-py3-none-any.whl \ - && pip install en_core_web_lg-3.8.0-py3-none-any.whl \ - && rm en_core_web_lg-3.8.0-py3-none-any.whl \ + && echo "293e9547a655b25499198ab15a525b05b9407a75f10255e405e8c3854329ab63 /tmp/en_core_web_lg-3.8.0-py3-none-any.whl" | sha256sum -c - \ + && pip install /tmp/en_core_web_lg-3.8.0-py3-none-any.whl \ + && rm /tmp/en_core_web_lg-3.8.0-py3-none-any.whl \ && python -c "import spacy; spacy.load('en_core_web_lg')" COPY ./compose/production/django/entrypoint /entrypoint diff --git a/compose/production/django/Dockerfile b/compose/production/django/Dockerfile index d5d4b24a3a..5fdda68135 100644 --- a/compose/production/django/Dockerfile +++ b/compose/production/django/Dockerfile @@ -1,4 +1,8 @@ -ARG PYTHON_VERSION=3.11-slim-bookworm +# Pin to a specific patch (not the floating 3.11 alias) so rebuilds on +# different dates do not silently pick up a new CPython point release that +# could change runtime behaviour. Bump deliberately when upstream ships a +# CVE/feature release and CI is green against the new patch. +ARG PYTHON_VERSION=3.11.15-slim-bookworm # ---------------------------------------------------------------------------- # # Build stage: full toolchain for compiling wheels (psycopg2, opencv, etc.) # @@ -18,6 +22,9 @@ ENV PIP_RETRIES=10 \ PIP_NO_CACHE_DIR=1 # Build-only system dependencies. None of these end up in the runtime image. +# `wget` is needed to fetch the large spaCy model wheel with explicit retry +# semantics (pip's retry can fall short on a multi-hundred-MB wheel over a +# transient SSL drop). RUN apt-get update && apt-get install --no-install-recommends -y \ build-essential \ cmake \ @@ -32,6 +39,7 @@ RUN apt-get update && apt-get install --no-install-recommends -y \ libtesseract-dev \ libpq-dev \ git \ + wget \ && rm -rf /var/lib/apt/lists/* # Copy the entire requirements directory for recursive building @@ -46,6 +54,22 @@ RUN (find ./requirements -mindepth 2 -type f -name "*.txt" -print; \ | sed 's/^/-r /' \ | xargs -r pip wheel --wheel-dir /usr/src/app/wheels +# Fetch pinned spaCy model wheels into the same wheel cache so the runtime +# stage installs them via the existing `pip install --no-index` step and +# `wget` never reaches the runtime image. Pinning the exact wheel URL is +# more reproducible than `python -m spacy download`, which resolves the +# wheel URL at build time and could silently drift to a different patch. +# `wget` retry args mirror compose/local/django/Dockerfile so the two paths +# behave the same on flaky networks. +RUN wget --retry-connrefused --waitretry=1 --read-timeout=20 --timeout=15 -t 5 \ + -P /usr/src/app/wheels \ + https://github.com/explosion/spacy-models/releases/download/en_core_web_sm-3.8.0/en_core_web_sm-3.8.0-py3-none-any.whl \ + && echo "1932429db727d4bff3deed6b34cfc05df17794f4a52eeb26cf8928f7c1a0fb85 /usr/src/app/wheels/en_core_web_sm-3.8.0-py3-none-any.whl" | sha256sum -c - \ + && wget --retry-connrefused --waitretry=1 --read-timeout=20 --timeout=15 -t 5 \ + -P /usr/src/app/wheels \ + https://github.com/explosion/spacy-models/releases/download/en_core_web_lg-3.8.0/en_core_web_lg-3.8.0-py3-none-any.whl \ + && echo "293e9547a655b25499198ab15a525b05b9407a75f10255e405e8c3854329ab63 /usr/src/app/wheels/en_core_web_lg-3.8.0-py3-none-any.whl" | sha256sum -c - + # ---------------------------------------------------------------------------- # # Runtime stage: slim base, runtime libs only. # # ---------------------------------------------------------------------------- # @@ -70,7 +94,12 @@ RUN groupadd --system django \ # Runtime system dependencies. Keep this list minimal — every package shows # up in the final image. # libpq5 : psycopg2 (binary protocol to Postgres) -# gettext : Django translations (compilemessages) +# gettext : retained as a build hook for `manage.py +# compilemessages`. No `.po`/`.mo` files ship today +# (`locale/` is in `.dockerignore`), so this is dead +# weight at runtime — kept so future i18n work does +# not need a Dockerfile change. Drop if image size +# ever needs the ~5 MB. # poppler-utils : pdf2image (pdftoppm/pdftocairo) # tesseract-ocr* : pytesseract OCR runtime # libgl1, libsm6, : opencv-python-headless transitive runtime deps @@ -104,9 +133,12 @@ RUN pip install --upgrade pip \ # build time but immediate at first request). RUN python -c "import cv2; print(f'cv2 {cv2.__version__} import OK')" -# Spacy models used by TxtParser sentence chunker. -RUN python -m spacy download en_core_web_sm \ - && python -m spacy download en_core_web_lg +# Smoke test: spaCy models load. The wheels were installed alongside other +# dependencies via the build-stage wheel cache (see the pinned-URL `wget` +# block above). `spacy.load(...)` exercises the same code path used by +# `TxtParser`'s sentence chunker so a missing-or-corrupt model fails the +# build immediately rather than at first request. +RUN python -c "import spacy; spacy.load('en_core_web_sm'); spacy.load('en_core_web_lg'); print('spaCy models load OK')" COPY --chown=django:django ./compose/production/django/entrypoint /entrypoint RUN sed -i 's/\r$//g' /entrypoint && chmod +x /entrypoint diff --git a/requirements/base.txt b/requirements/base.txt index 0a78e0f9c5..50e2223fa4 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -12,7 +12,7 @@ pydantic==2.* typing-extensions==4.* # https://github.com/python/typing_extensions requests>=2.33.1,<3 # https://requests.readthedocs.io/en/latest/ httpx>=0.28.1,<1 # https://github.com/encode/httpx - async HTTP for agent tools -brotli>=1.1.0 # decoder for httpx Content-Encoding: br responses (e.g. OpenAI API, VCR cassettes) +brotli>=1.1.0 # decoder for httpx Content-Encoding: br responses (e.g. OpenAI API, VCR cassettes). Floor is 1.1.0 because that is the current major-version line (released 2023-09) under active upstream maintenance; the underlying _brotli C-extension API httpx looks up has been stable since 1.0.x, so loosening the floor is safe if a transitive constraint demands it. # Django