PST-046: The unattended media sweep has no floor: a pass that sees an empty or wrong database deletes every user's files #775

New issue

Closed

opened 2026-10-10 10:56:18 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-10 10:56:18 +00:00

Owner

Found in the security audit of 2026-10-10 (SECURITY-AUDIT-2026-10-10.md), run against main at c30851b from 8 to 10 October 2026, as PST-046. Severity: Low.

The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it.

Severity: Low
Category: data-loss / integrity
Location: src/postulo/documents/scrub.py (97-121)
Found by: file handling

Who can do it, and what they need. No external attacker; an operational hazard for the personal data the application promises to keep. Precondition: the scheduler runs (default POSTULO_MEDIA_SWEEP=true) against a database that names no files while MEDIA_ROOT still holds them: a mis-set database URL after an upgrade (SQLite default path, fresh and migrated), a Postgres pointed at an empty schema, a restore that has replaced the database but not yet written media while the …

How it works.

send_due_reminders calls scrub.sweep() on every scheduler pass.
sweep() iterates media.unreferenced(older_than=now - 24h), which is filestore.listing() minus referenced_names(); referenced_names() is simply every FileField value in every model of the current database.
If that set is empty (or missing a whole model), every file older than the grace period is filestore.delete()d, with only a log line each; there is no check that the account table is non-empty, that the referenced set is non-trivial, that a restore is in progress, or a cap on how many files one pass may remove.
The deletion is irreversible: CVs, cover letters, sent PDFs, kept pages and export archives of every account are gone before anybody reads the log.

Impact. Irreversible loss of every person's uploaded and rendered documents on an instance after one misconfiguration or a restore run without stopping the scheduler. Not a confidentiality issue, but the documents are the personal data the project exists to protect, and the operator's recovery is only a backup.

Evidence.

documents/scrub.py:109-121
    if not settings.POSTULO_MEDIA_SWEEP:
        return []
    removed: list[str] = []
    for stored in strays(grace=grace()):
        try:
            filestore.delete(stored.name)
core/media.py:41-55 (unreferenced): referenced = referenced_names(); for stored in filestore.listing(): if stored.name in referenced: continue ... yield stored
notifications/.../send_due_reminders.py:218
    swept = scrub.sweep()

Verification (verdict: confirmed; severity after verification: Low). Keep as a low-severity data-safety hardening gap, not an attack. Files: src/postulo/documents/scrub.py:103-121 (sweep), src/postulo/core/media.py:41-55 (unreferenced), src/postulo/notifications/management/commands/send_due_reminders.py:218. Drop two stated preconditions: the restore-in-progress scenario (busy_reason refusal; database restored before media; restored files get a fresh mtime and so the 24h grace) and the plugin-removed scenario (no plugin owns a model or FileField; INSTALLED_APPS is static). Real preconditions are a database that names none or few of the files while the media volume is kept (mis-set or dropped POSTULO_DATABASE_URL outside the shipped compose files, a SQLite-to-Postgres switch without restore, a recreated schema), or restore --force of an older archive, where the closing message wrongly implies prune_media is the operator's decision. Suggested remediation: a floor that skips the sweep and logs loudly when the referenced set is empty or the account table is empty while the store is not, and/or a per-pass cap, and/or a default of list-only until opted in.

Suggested remediation. Refuse to sweep when referenced_names() is empty while the store is not, when no user exists, or when a restore lock/marker is present; cap removals per pass (e.g. 100) and skip the pass with a warning above it; move strays to a quarantine directory (trash/<date>/) that the next pass empties after a further grace, rather than unlinking at once; have restore_backup take the same lock backups.take() uses so the scheduler stands down.

Found in the security audit of 2026-10-10 (`SECURITY-AUDIT-2026-10-10.md`), run against `main` at `c30851b` from 8 to 10 October 2026, as **PST-046**. Severity: **Low**. The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it. --- - **Severity:** Low - **Category:** data-loss / integrity - **Location:** `src/postulo/documents/scrub.py` (97-121) - **Found by:** file handling **Who can do it, and what they need.** No external attacker; an operational hazard for the personal data the application promises to keep. Precondition: the scheduler runs (default `POSTULO_MEDIA_SWEEP=true`) against a database that names no files while MEDIA_ROOT still holds them: a mis-set database URL after an upgrade (SQLite default path, fresh and migrated), a Postgres pointed at an empty schema, a restore that has replaced the database but not yet written media while the … **How it works.** 1. `send_due_reminders` calls `scrub.sweep()` on every scheduler pass. 2. `sweep()` iterates `media.unreferenced(older_than=now - 24h)`, which is `filestore.listing()` minus `referenced_names()`; `referenced_names()` is simply every FileField value in every model of the current database. 3. If that set is empty (or missing a whole model), every file older than the grace period is `filestore.delete()`d, with only a log line each; there is no check that the account table is non-empty, that the referenced set is non-trivial, that a restore is in progress, or a cap on how many files one pass may remove. 4. The deletion is irreversible: CVs, cover letters, sent PDFs, kept pages and export archives of every account are gone before anybody reads the log. **Impact.** Irreversible loss of every person's uploaded and rendered documents on an instance after one misconfiguration or a restore run without stopping the scheduler. Not a confidentiality issue, but the documents are the personal data the project exists to protect, and the operator's recovery is only a backup. **Evidence.** ```text documents/scrub.py:109-121 if not settings.POSTULO_MEDIA_SWEEP: return [] removed: list[str] = [] for stored in strays(grace=grace()): try: filestore.delete(stored.name) core/media.py:41-55 (unreferenced): referenced = referenced_names(); for stored in filestore.listing(): if stored.name in referenced: continue ... yield stored notifications/.../send_due_reminders.py:218 swept = scrub.sweep() ``` **Verification** (verdict: confirmed; severity after verification: Low). Keep as a low-severity data-safety hardening gap, not an attack. Files: src/postulo/documents/scrub.py:103-121 (sweep), src/postulo/core/media.py:41-55 (unreferenced), src/postulo/notifications/management/commands/send_due_reminders.py:218. Drop two stated preconditions: the restore-in-progress scenario (busy_reason refusal; database restored before media; restored files get a fresh mtime and so the 24h grace) and the plugin-removed scenario (no plugin owns a model or FileField; INSTALLED_APPS is static). Real preconditions are a database that names none or few of the files while the media volume is kept (mis-set or dropped POSTULO_DATABASE_URL outside the shipped compose files, a SQLite-to-Postgres switch without restore, a recreated schema), or `restore --force` of an older archive, where the closing message wrongly implies prune_media is the operator's decision. Suggested remediation: a floor that skips the sweep and logs loudly when the referenced set is empty or the account table is empty while the store is not, and/or a per-pass cap, and/or a default of list-only until opted in. **Suggested remediation.** Refuse to sweep when `referenced_names()` is empty while the store is not, when no user exists, or when a restore lock/marker is present; cap removals per pass (e.g. 100) and skip the pass with a warning above it; move strays to a quarantine directory (`trash/<date>/`) that the next pass empties after a further grace, rather than unlinking at once; have `restore_backup` take the same lock `backups.take()` uses so the scheduler stands down.

tiagoagueda added the

bug

security

labels

2026-10-10 10:56:19 +00:00

tiagoagueda added this to the 0.6.0 milestone

2026-10-10 10:57:49 +00:00

tiagoagueda referenced this issue

2026-10-10 10:58:40 +00:00

Security audit of 2026-10-10: index of PST-001 to PST-098 (#730–#827) #828

tiagoagueda self-assigned this

2026-10-11 10:54:31 +00:00