Uploaded documents: no orphans, no corrupted files, and one boundary for where the bytes live #663

New issue

Closed

opened 2026-10-03 01:58:42 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-03 01:58:42 +00:00

Owner

What exists today

Ownership and the folder. UploadedDocument (src/postulo/documents/models.py:564) keeps file under documents/<owner pk>/<YYYY>/<MM>/ (upload_to_documents, :45-50). Download goes through OwnedObjectMixin and serve_private_file (documents/views.py:653-667, core/files.py:65-117); a foreign file is a 404. Nothing re-checks, at serve time, that the stored name lies under the owner's own prefix; resolve_media_path (core/files.py:37-48) checks only that it lies under MEDIA_ROOT.
Orphans on delete are handled. #217 added post_delete receivers that remove the file after the transaction commits and only when no other row uses the name (documents/signals.py:38-69), and prune_media. Account deletion names uploads and renders in accounts/deletion.py:61-81.
Orphans that are not handled.
- A write that is rolled back. FileField writes the bytes when the row is saved, inside the request's transaction (ATOMIC_REQUESTS, config/settings/base.py:149). The delete side has an on_commit hook and a test (tests/test_document_record.py:174); the write side has nothing, so a request that fails after the file is written leaves the file with no row.
- The archive importer. importer.load is one @transaction.atomic (core/importer.py:405) and writes each file as it goes (:1000-1004, :1034-1038). A failure half way rolls the rows back and leaves every file already written.
- Files from before #217, and a restore from an older backup, which prune_media names as its reason (documents/management/commands/prune_media.py:1-9).
- prune_media is only run by hand. It is named in the wiki (Files-and-what-you-sent.md:154-155) and in restore's closing message (core/management/commands/restore.py:83), and not in the scheduler's pass (notifications/management/commands/send_due_reminders.py:168-200), which reaps export archives and expired kept pages. Its own hand-kept list is #355's (open): it omits ExportArchive.file, so it would delete live exports.
A checksum exists and is almost never read. UploadedDocument.checksum is written once, the first time the bytes are here (documents/models.py:614, :645-669, #217). Nothing compares it with the bytes afterwards. RenderedDocument has the one check, file_is_as_rendered (:869-885), and it is asked only when deciding whether an earlier render can be handed back (documents/rendering.py:633).
The archive carries no integrity information for uploads. UPLOAD_FIELDS has no checksum (core/export.py:306-318), so on import the checksum is computed from whatever bytes arrived (UploadedDocument.save) and a damaged file gets a valid-looking one. For sent documents SENT_FIELDS carries it (core/export.py:324) and the importer takes it as written without hashing the file (core/importer.py:1017-1041). The zip's own CRC-32 is the only check; _extract catches only KeyError (core/importer.py:1255-1261).
A copy leaves unchecked. documents/archiving.py:135 opens the file and hands it to a store plugin with metadata_for(document), which includes the stored checksum (documents/stores.py:95), without checking that the bytes match it.
Content is not checked. The only validation is the extension list pdf, doc, docx, odt, rtf, txt, png, jpg, jpeg (documents/models.py:581-590) and the 20 MB cap in the form (documents/forms.py:29, :508-515). A file called cv.pdf that is not a PDF is stored, hashed and delivered; the type in the response is guessed from the name (core/files.py:90). The importer runs neither the extension validator nor any cap on uploads (size is #466's). Two precedents for a content check already exist: jobs/pages.py:85-101 (is_what_it_says: first bytes of a PNG, JPEG, WebP or PDF, "not a decoder, and deliberately not one") and core/pictures.py:240-244 (SVG recognised by a pattern). jobs/pages.py:740-775 says of an archive "nothing in it is taken on trust: not the sizes, not the checksums, not what it says a file is" and recomputes every figure; uploads are not restored that way.
The storage boundary. Two contracts exist. StorePlugin (postulo.plugins.api) is for copies and plugins/localstore/__init__.py is "a plugin in shape only", whose put is document.file.save and which is called once, for renders (documents/rendering.py:546); uploads are written by the form's FileField, not through it. Underneath, FileField uses Django's storage API, with FileSystemStorage as the default (config/settings/base.py:619-622). Around that, five places use the directory directly: core/files.py:37-117 (path.is_file(), path.open, X-Accel-Redirect and X-Sendfile), prune_media (rglob over MEDIA_ROOT), accounts/deletion.py:84-151 (Path and rmdir), core/backup.py:328-398 and :510 (tars the tree), and core/server_views.py:136 (sums the tree for the overview). #13 decided "local stays the source of truth. External stores receive copies" and left an external-only mode "deliberately not proposed".

Checked against main at f09b6010b.

Proposal

Five commit-sized parts, in this order.

Check what a file is, once, in one place. A function documents.integrity.looks_like(extension, head_and_tail) (or a module beside core/pictures.py) that answers whether the bytes are plausibly what the extension says: %PDF- at the start and %%EOF near the end for a PDF; a PNG or JPEG that Pillow can open (core/pictures.decode's guard on pixels applies; the bytes are not re-encoded, because an upload is kept as it was); a ZIP with the mimetype entry first for ODT and a [Content_Types].xml entry for DOCX, listed from the zip directory and never unpacked; {\rtf for RTF; the OLE2 header for DOC; text with no NUL bytes for TXT. The container questions go to puremagic if the recommendation under Decisions is taken, and are answered by hand from the zip directory if it is not. Used by the upload form (clean_file), the importer and the scrub below. The refusal says what was expected, not how it failed. Nothing opens the file with a document library.
Verify, in the places that were taking it on trust. The archive carries the checksum and the size for uploads (a new key, so a format bump); the importer hashes what it unpacked and refuses or reports a mismatch as jobs/pages.py does for kept pages; the sent-document checksum is checked the same way. archiving.send compares size and checksum before it hands bytes to a store, and does not send a copy that does not match. serve_private_file for an upload checks that the stored name starts with documents/<owner pk>/.
A scrub that looks for the two kinds of damage. manage.py verify_media (and a slice of it in the scheduler's pass, a fixed number of rows each time, oldest-checked first) reads each upload and render in chunks and reports: a row whose file is missing, a row whose size or SHA-256 differs from what was recorded, and a file under the person's folder that no row names. It never deletes a corrupted file. The Files page shows a short "This file is missing" or "This file no longer matches what was uploaded" on the row, and the person can upload a replacement as a new version (replaces). The operator sees the totals under Server settings → Overview.
No orphan survives, by construction and by sweep. (a) Write side: where a function writes a file and then its row, it removes the file if the row fails to save (as pages._write does, jobs/pages.py:301-322), and the importer removes every file it wrote when load fails. A request that fails after those have returned rolls the row back out of this code's reach (Django has on_commit and no rollback hook), and that case is the sweep's: the grace period below exists for it. (b) Sweep side: the scheduled pass runs the orphan listing over the same derived list of file fields that #355 proposes (apps.get_models()), and removes only a file whose modification time is older than a grace period, never one a request may still be about to attach. The command's manual --remove stays for the rest.
State the boundary, with Django's storage API, and move the five places behind it. The boundary is a small internal protocol in documents/: open for reading, save, delete, exists, size, list names under a prefix, and how to hand a file to a browser (a stream, an X-Accel path, or an expiring address). The default is today's behaviour. serve_private_file, prune_media, deletion, backup and the overview call it and not Path. Nothing is published on postulo.plugins.api until a plugin needs it; the wiki's Files and what you sent gets a short section on what a storage backend has to provide.

Decisions

What "not tolerated" does with a corrupted file. Options: delete it; quarantine it (move aside, keep the row); flag it and keep both. Recommended: flag it and keep it. The file may be the only copy of somebody's diploma; an unattended job deleting it is the failure prune_media was written not to have ("the failure mode of the opposite default is somebody's CV"). The person is told and replaces it.
Verify on every read or on a schedule. Options: hash on every download; compare only size and existence on download and hash on the scrub and before copying out. Recommended: the second. A 20 MB file read twice per download costs a worker, and with X-Accel-Redirect the bytes never pass through Django. Size is a new column beside the checksum (it is not stored today; DocumentMetadata.size reads it from disk).
Which types are allowed. The note names PDF, ODT, DOCX, PNG, JPG; the validator also allows doc, rtf and txt (documents/models.py:585-589). Recommended: keep all nine, because narrowing refuses what people already store, and say so in the wiki. Question for the maintainer: are doc, rtf and txt meant to stay?
Orphan sweep: scheduled or manual. Options: keep manual; schedule with a grace period; schedule with --remove on by default. Recommended: scheduled, with a 24-hour grace period, logged line by line, and a setting to switch it off. It depends on #355 landing first, so that a live export archive counts as referenced. Conflict: prune_media lists by default on purpose (its docstring, #217); this keeps that for the manual command and changes only the unattended one, which can only reach files that are older than a grace period and have no row.
Detecting types: a library or the hand-written check. See Prior art. Recommended: puremagic for the container questions and Pillow (already a dependency) for images, with the hand-written agreement table between extension and detected type, which no library can supply.
Local source of truth. #13 kept local media as the one source of truth and rejected an external-only store. The note says "a step closer to storage-oriented plugins". Recommended: keep #13's rule for now and make the boundary real internally (part 5); the first plugin that needs to be primary reopens #13 with something concrete to answer. Related: #325 (backup destinations), #241 (a WebDAV store), postulo/postulo-paperless (copies).
Pictures. The avatar and the logo are not documents; #662 decides where they live.

Prior art

Content type:

puremagic (https://github.com/cdgriffith/puremagic): identifies files by magic numbers in pure Python, with scanners for ZIP contents; MIT; 2.2.0 released 2026-04-08; requires Python 3.12 or later (Postulo: >=3.12.4); no runtime dependencies; 0 advisories on OSV; not a dependency today. Its detection of DOCX and ODT from ZIP contents is as its README states and was not run here: not verified.
filetype (https://github.com/h2non/filetype.py): MIT; 1.2.0, released 2022-11-02, so no release for nearly four years; no dependencies; reads only the first 261 bytes; 0 advisories. Maintenance is the reason not to pick it.
python-magic (MIT, 0.4.27, 2022-06-07, 0 advisories on OSV): a binding to libmagic, which the image does not install (docker/Dockerfile:246-250); adds a system package and a C parser of untrusted files. Not adopted.
magika (Google; Apache-2.0; 1.0.3, 2026-05-04): a model-based detector that requires onnxruntime. A heavy dependency for nine extensions. Not adopted.
pypdf (BSD-3-Clause, 6.19.0, 2026-09-16): has had many denial-of-service advisories on OSV (101 listed across versions, 0 affecting 6.19.0), all from parsing untrusted PDFs in-process. Not adopted: the check proposed here does not parse a PDF at all.
Hand-written precedent in this repository: jobs/pages.py:85-101 and core/pictures.py:240-244. It applies for four types and does not for ODT, DOCX and DOC, whose evidence is inside a ZIP or an OLE2 container.

Storage:

Django's Storage API is built in and already what FileField uses; STORAGES["default"] can be any backend. It is the boundary to adopt rather than a new one.
django-storages (https://github.com/jschneier/django-storages): BSD-3-Clause; 1.14.6, released 2025-04-02; 0 advisories on OSV; requires Django 3.2 or later (Postulo is on 6.1); backends for S3, Azure, Google Cloud, SFTP, Dropbox and others, each behind an extra. Not a dependency today. It would be what a storage plugin installs, not something core needs, so it is not adopted now; the point of part 5 is that it could work, which it cannot while core/files.py and the other four places read the directory.

Builds on

#217 (closed), #13 (closed), #355, #466, #241, #325.

Needed by

#669 (diplomas and certificates are uploads whose damage matters most), #666 (a received reference letter is an upload), #670 (a credential file is verified as it is read).

Done when

tests/test_documents.py: a file named cv.pdf that is not a PDF is refused with the reason; so is an ODT with no mimetype first entry and a DOCX with no [Content_Types].xml; a real file of each of the nine types is accepted (new tests; the first three fail today).
tests/test_document_record.py: a request that fails after the file is written leaves no file behind (beside test_a_rolled_back_delete_keeps_the_file); the sweep removes an old file with no row, leaves a recent one, leaves one a row names, and leaves a live ExportArchive (shared with #355's test); verify_media reports a missing file, a changed byte and a stray file, and deletes nothing.
tests/test_export.py and tests/security/test_files.py: an archive whose upload is truncated or changed is reported and the row restored without the file; one whose sent-document checksum does not match its bytes is reported; a failure half way through importer.load leaves no file under MEDIA_ROOT.
tests/test_stores.py: a copy whose bytes do not match the recorded checksum is not sent and the copy row says why.
serve_private_file refuses a stored name outside documents/<owner pk>/ (tests/security/test_files.py).
A test lists the five modules above and fails if any of them imports pathlib against MEDIA_ROOT once part 5 is done.
The archive format goes up by one (core/export.py FORMAT_VERSION, 29 on f09b6010b; write one more than whatever it is when the commit lands) for the size and checksum of uploads; older archives still import. Candidate format (4) unchanged. API: the document serializer gains the integrity state if it lists uploads. Search unchanged. Processing record (core/gdpr.py) unchanged: the same files, kept the same way. tests/test_page_coverage.py and tests/security/test_isolation_sweep.py if the Files page gains a view; strings in the working catalogues (en-GB, fr-FR, pt-PT as drafts) and Weblate (#349).

From the list of improvements of 2026-10-02.

The maintainer's rule, from the list of 2026-10-02: "An uploaded document (PDF, ODT, DOCX, PNG, JPG), belongs only to the user that uploads it, and is stored on an isolated folder. Orphan files on that folder are not tolerated. Corrupted files either. Data integrity is key. A step closer to storage-oriented plugins." Ownership and the per-person folder are in place. Orphans are removed on delete but not on every other path, nothing looks for corruption after a file arrives, and the code that touches the bytes assumes a local directory in five places. ## What exists today - **Ownership and the folder.** `UploadedDocument` (`src/postulo/documents/models.py:564`) keeps `file` under `documents/<owner pk>/<YYYY>/<MM>/` (`upload_to_documents`, `:45-50`). Download goes through `OwnedObjectMixin` and `serve_private_file` (`documents/views.py:653-667`, `core/files.py:65-117`); a foreign file is a 404. Nothing re-checks, at serve time, that the stored name lies under the owner's own prefix; `resolve_media_path` (`core/files.py:37-48`) checks only that it lies under `MEDIA_ROOT`. - **Orphans on delete are handled.** #217 added `post_delete` receivers that remove the file after the transaction commits and only when no other row uses the name (`documents/signals.py:38-69`), and `prune_media`. Account deletion names uploads and renders in `accounts/deletion.py:61-81`. - **Orphans that are not handled.** - *A write that is rolled back.* `FileField` writes the bytes when the row is saved, inside the request's transaction (`ATOMIC_REQUESTS`, `config/settings/base.py:149`). The delete side has an `on_commit` hook and a test (`tests/test_document_record.py:174`); the write side has nothing, so a request that fails after the file is written leaves the file with no row. - *The archive importer.* `importer.load` is one `@transaction.atomic` (`core/importer.py:405`) and writes each file as it goes (`:1000-1004`, `:1034-1038`). A failure half way rolls the rows back and leaves every file already written. - *Files from before #217, and a restore from an older backup*, which `prune_media` names as its reason (`documents/management/commands/prune_media.py:1-9`). - *`prune_media` is only run by hand.* It is named in the wiki (`Files-and-what-you-sent.md:154-155`) and in `restore`'s closing message (`core/management/commands/restore.py:83`), and not in the scheduler's pass (`notifications/management/commands/send_due_reminders.py:168-200`), which reaps export archives and expired kept pages. Its own hand-kept list is #355's (open): it omits `ExportArchive.file`, so it would delete live exports. - **A checksum exists and is almost never read.** `UploadedDocument.checksum` is written once, the first time the bytes are here (`documents/models.py:614`, `:645-669`, #217). Nothing compares it with the bytes afterwards. `RenderedDocument` has the one check, `file_is_as_rendered` (`:869-885`), and it is asked only when deciding whether an earlier render can be handed back (`documents/rendering.py:633`). - **The archive carries no integrity information for uploads.** `UPLOAD_FIELDS` has no checksum (`core/export.py:306-318`), so on import the checksum is computed from whatever bytes arrived (`UploadedDocument.save`) and a damaged file gets a valid-looking one. For sent documents `SENT_FIELDS` carries it (`core/export.py:324`) and the importer takes it as written without hashing the file (`core/importer.py:1017-1041`). The zip's own CRC-32 is the only check; `_extract` catches only `KeyError` (`core/importer.py:1255-1261`). - **A copy leaves unchecked.** `documents/archiving.py:135` opens the file and hands it to a store plugin with `metadata_for(document)`, which includes the stored checksum (`documents/stores.py:95`), without checking that the bytes match it. - **Content is not checked.** The only validation is the extension list `pdf, doc, docx, odt, rtf, txt, png, jpg, jpeg` (`documents/models.py:581-590`) and the 20 MB cap in the form (`documents/forms.py:29`, `:508-515`). A file called `cv.pdf` that is not a PDF is stored, hashed and delivered; the type in the response is guessed from the name (`core/files.py:90`). The importer runs neither the extension validator nor any cap on uploads (size is #466's). Two precedents for a content check already exist: `jobs/pages.py:85-101` (`is_what_it_says`: first bytes of a PNG, JPEG, WebP or PDF, "not a decoder, and deliberately not one") and `core/pictures.py:240-244` (SVG recognised by a pattern). `jobs/pages.py:740-775` says of an archive "nothing in it is taken on trust: not the sizes, not the checksums, not what it says a file is" and recomputes every figure; uploads are not restored that way. - **The storage boundary.** Two contracts exist. `StorePlugin` (`postulo.plugins.api`) is for *copies* and `plugins/localstore/__init__.py` is "a plugin in shape only", whose `put` is `document.file.save` and which is called once, for renders (`documents/rendering.py:546`); uploads are written by the form's `FileField`, not through it. Underneath, `FileField` uses Django's storage API, with `FileSystemStorage` as the default (`config/settings/base.py:619-622`). Around that, five places use the directory directly: `core/files.py:37-117` (`path.is_file()`, `path.open`, `X-Accel-Redirect` and `X-Sendfile`), `prune_media` (`rglob` over `MEDIA_ROOT`), `accounts/deletion.py:84-151` (`Path` and `rmdir`), `core/backup.py:328-398` and `:510` (tars the tree), and `core/server_views.py:136` (sums the tree for the overview). #13 decided "local stays the source of truth. External stores receive *copies*" and left an external-only mode "deliberately not proposed". Checked against `main` at `f09b6010b`. ## Proposal Five commit-sized parts, in this order. 1. **Check what a file is, once, in one place.** A function `documents.integrity.looks_like(extension, head_and_tail)` (or a module beside `core/pictures.py`) that answers whether the bytes are plausibly what the extension says: `%PDF-` at the start and `%%EOF` near the end for a PDF; a PNG or JPEG that Pillow can open (`core/pictures.decode`'s guard on pixels applies; the bytes are **not** re-encoded, because an upload is kept as it was); a ZIP with the `mimetype` entry first for ODT and a `[Content_Types].xml` entry for DOCX, **listed from the zip directory and never unpacked**; `{\rtf` for RTF; the OLE2 header for DOC; text with no NUL bytes for TXT. The container questions go to `puremagic` if the recommendation under *Decisions* is taken, and are answered by hand from the zip directory if it is not. Used by the upload form (`clean_file`), the importer and the scrub below. The refusal says what was expected, not how it failed. Nothing opens the file with a document library. 2. **Verify, in the places that were taking it on trust.** The archive carries the checksum and the size for uploads (a new key, so a format bump); the importer hashes what it unpacked and refuses or reports a mismatch as `jobs/pages.py` does for kept pages; the sent-document checksum is checked the same way. `archiving.send` compares size and checksum before it hands bytes to a store, and does not send a copy that does not match. `serve_private_file` for an upload checks that the stored name starts with `documents/<owner pk>/`. 3. **A scrub that looks for the two kinds of damage.** `manage.py verify_media` (and a slice of it in the scheduler's pass, a fixed number of rows each time, oldest-checked first) reads each upload and render in chunks and reports: a row whose file is missing, a row whose size or SHA-256 differs from what was recorded, and a file under the person's folder that no row names. It never deletes a corrupted file. The Files page shows a short "This file is missing" or "This file no longer matches what was uploaded" on the row, and the person can upload a replacement as a new version (`replaces`). The operator sees the totals under Server settings → Overview. 4. **No orphan survives, by construction and by sweep.** (a) Write side: where a function writes a file and then its row, it removes the file if the row fails to save (as `pages._write` does, `jobs/pages.py:301-322`), and the importer removes every file it wrote when `load` fails. A request that fails *after* those have returned rolls the row back out of this code's reach (Django has `on_commit` and no rollback hook), and that case is the sweep's: the grace period below exists for it. (b) Sweep side: the scheduled pass runs the orphan listing over the same derived list of file fields that #355 proposes (`apps.get_models()`), and removes only a file whose modification time is older than a grace period, never one a request may still be about to attach. The command's manual `--remove` stays for the rest. 5. **State the boundary, with Django's storage API, and move the five places behind it.** The boundary is a small internal protocol in `documents/`: open for reading, save, delete, exists, size, list names under a prefix, and how to hand a file to a browser (a stream, an `X-Accel` path, or an expiring address). The default is today's behaviour. `serve_private_file`, `prune_media`, `deletion`, `backup` and the overview call it and not `Path`. Nothing is published on `postulo.plugins.api` until a plugin needs it; the wiki's *Files and what you sent* gets a short section on what a storage backend has to provide. ## Decisions - **What "not tolerated" does with a corrupted file.** Options: delete it; quarantine it (move aside, keep the row); flag it and keep both. **Recommended:** flag it and keep it. The file may be the only copy of somebody's diploma; an unattended job deleting it is the failure `prune_media` was written not to have ("the failure mode of the opposite default is somebody's CV"). The person is told and replaces it. - **Verify on every read or on a schedule.** Options: hash on every download; compare only size and existence on download and hash on the scrub and before copying out. **Recommended:** the second. A 20 MB file read twice per download costs a worker, and with `X-Accel-Redirect` the bytes never pass through Django. Size is a new column beside the checksum (it is not stored today; `DocumentMetadata.size` reads it from disk). - **Which types are allowed.** The note names PDF, ODT, DOCX, PNG, JPG; the validator also allows `doc`, `rtf` and `txt` (`documents/models.py:585-589`). **Recommended:** keep all nine, because narrowing refuses what people already store, and say so in the wiki. Question for the maintainer: are `doc`, `rtf` and `txt` meant to stay? - **Orphan sweep: scheduled or manual.** Options: keep manual; schedule with a grace period; schedule with `--remove` on by default. **Recommended:** scheduled, with a 24-hour grace period, logged line by line, and a setting to switch it off. It depends on #355 landing first, so that a live export archive counts as referenced. Conflict: `prune_media` lists by default on purpose (its docstring, #217); this keeps that for the manual command and changes only the unattended one, which can only reach files that are older than a grace period and have no row. - **Detecting types: a library or the hand-written check.** See *Prior art*. **Recommended:** `puremagic` for the container questions and Pillow (already a dependency) for images, with the hand-written agreement table between extension and detected type, which no library can supply. - **Local source of truth.** #13 kept local media as the one source of truth and rejected an external-only store. The note says "a step closer to storage-oriented plugins". **Recommended:** keep #13's rule for now and make the boundary real internally (part 5); the first plugin that needs to be primary reopens #13 with something concrete to answer. Related: #325 (backup destinations), #241 (a WebDAV store), postulo/postulo-paperless (copies). - **Pictures.** The avatar and the logo are not documents; #662 decides where they live. ## Prior art Content type: - **`puremagic`** (<https://github.com/cdgriffith/puremagic>): identifies files by magic numbers in pure Python, with scanners for ZIP contents; MIT; 2.2.0 released 2026-04-08; requires Python 3.12 or later (Postulo: `>=3.12.4`); no runtime dependencies; 0 advisories on OSV; not a dependency today. Its detection of DOCX and ODT from ZIP contents is as its README states and was not run here: not verified. - **`filetype`** (<https://github.com/h2non/filetype.py>): MIT; 1.2.0, released 2022-11-02, so no release for nearly four years; no dependencies; reads only the first 261 bytes; 0 advisories. Maintenance is the reason not to pick it. - **`python-magic`** (MIT, 0.4.27, 2022-06-07, 0 advisories on OSV): a binding to `libmagic`, which the image does not install (`docker/Dockerfile:246-250`); adds a system package and a C parser of untrusted files. Not adopted. - **`magika`** (Google; Apache-2.0; 1.0.3, 2026-05-04): a model-based detector that requires `onnxruntime`. A heavy dependency for nine extensions. Not adopted. - **`pypdf`** (BSD-3-Clause, 6.19.0, 2026-09-16): has had many denial-of-service advisories on OSV (101 listed across versions, 0 affecting 6.19.0), all from parsing untrusted PDFs in-process. Not adopted: the check proposed here does not parse a PDF at all. - **Hand-written precedent in this repository:** `jobs/pages.py:85-101` and `core/pictures.py:240-244`. It applies for four types and does not for ODT, DOCX and DOC, whose evidence is inside a ZIP or an OLE2 container. Storage: - **Django's `Storage` API** is built in and already what `FileField` uses; `STORAGES["default"]` can be any backend. It is the boundary to adopt rather than a new one. - **`django-storages`** (<https://github.com/jschneier/django-storages>): BSD-3-Clause; 1.14.6, released 2025-04-02; 0 advisories on OSV; requires Django 3.2 or later (Postulo is on 6.1); backends for S3, Azure, Google Cloud, SFTP, Dropbox and others, each behind an extra. Not a dependency today. It would be what a storage plugin installs, not something core needs, so it is **not adopted now**; the point of part 5 is that it could work, which it cannot while `core/files.py` and the other four places read the directory. ## Builds on #217 (closed), #13 (closed), #355, #466, #241, #325. ## Needed by #669 (diplomas and certificates are uploads whose damage matters most), #666 (a received reference letter is an upload), #670 (a credential file is verified as it is read). ## Done when - `tests/test_documents.py`: a file named `cv.pdf` that is not a PDF is refused with the reason; so is an ODT with no `mimetype` first entry and a DOCX with no `[Content_Types].xml`; a real file of each of the nine types is accepted (new tests; the first three fail today). - `tests/test_document_record.py`: a request that fails after the file is written leaves no file behind (beside `test_a_rolled_back_delete_keeps_the_file`); the sweep removes an old file with no row, leaves a recent one, leaves one a row names, and leaves a live `ExportArchive` (shared with #355's test); `verify_media` reports a missing file, a changed byte and a stray file, and deletes nothing. - `tests/test_export.py` and `tests/security/test_files.py`: an archive whose upload is truncated or changed is reported and the row restored without the file; one whose sent-document checksum does not match its bytes is reported; a failure half way through `importer.load` leaves no file under `MEDIA_ROOT`. - `tests/test_stores.py`: a copy whose bytes do not match the recorded checksum is not sent and the copy row says why. - `serve_private_file` refuses a stored name outside `documents/<owner pk>/` (`tests/security/test_files.py`). - A test lists the five modules above and fails if any of them imports `pathlib` against `MEDIA_ROOT` once part 5 is done. - The archive format goes up by one (`core/export.py` `FORMAT_VERSION`, 29 on `f09b6010b`; write one more than whatever it is when the commit lands) for the size and checksum of uploads; older archives still import. Candidate format (4) unchanged. API: the document serializer gains the integrity state if it lists uploads. Search unchanged. Processing record (`core/gdpr.py`) unchanged: the same files, kept the same way. `tests/test_page_coverage.py` and `tests/security/test_isolation_sweep.py` if the Files page gains a view; strings in the working catalogues (en-GB, fr-FR, pt-PT as drafts) and Weblate (#349). From the list of improvements of 2026-10-02.

tiagoagueda added this to the 0.6.0 milestone

2026-10-03 01:58:42 +00:00

tiagoagueda added the

security

enhancement

labels

2026-10-03 01:58:42 +00:00

tiagoagueda referenced this issue

2026-10-03 01:58:44 +00:00

Reference letters: who wrote one, when, and which applications it went with #666

tiagoagueda referenced this issue

2026-10-03 01:58:49 +00:00