Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -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/

Expand Down
45 changes: 41 additions & 4 deletions .github/workflows/docker-build-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 <tag>` 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
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <tag>` 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).
Expand All @@ -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.
Expand Down
23 changes: 17 additions & 6 deletions compose/local/django/Dockerfile
Original file line number Diff line number Diff line change
@@ -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.) #
Expand Down Expand Up @@ -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
Expand Down
42 changes: 37 additions & 5 deletions compose/production/django/Dockerfile
Original file line number Diff line number Diff line change
@@ -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.) #
Expand All @@ -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 \
Expand All @@ -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
Expand All @@ -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. #
# ---------------------------------------------------------------------------- #
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading