rbrooks/Quest-Board

Watch

Fork 0

Media directories with no campaign are never swept #1075

New issue

Closed

opened 2026-10-04 08:56:18 +00:00 by claude-bot · 2 comments

claude-bot commented

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

Collaborator

Found by the independent review of #991 part 2 (PR B).

What

Nothing removes a media/private/campaigns/<id>/ directory whose campaign row does not exist. Two ways one comes to exist:

A backend that dies mid-import (an OOM kill, a deploy, a restart). The import's own cleanup runs in a finally, which a killed process never reaches. The files it had written stay: attachments, page pictures, and each map's imported copy.
A purged campaign. The review saw no removal of that directory when a campaign is deleted for good. Read only, not confirmed by running; check this first.

The files are under private/, which nginx never serves, so nothing is exposed. The cost is disk: an interrupted import can leave several hundred megabytes.

Intended approach

A periodic task (Celery beat, daily) that lists media/private/campaigns/* and removes a directory when:
- no campaign row has that id, and
- the directory's newest file is older than a bound (24 hours), so an import still in flight is never touched.
It logs what it removed and how much.
If a purge really does leave the directory, remove it at purge time as well.
Tests: an orphan older than the bound is removed; a fresh orphan is kept; a live campaign's directory is never touched; a name that is not a UUID is ignored.

#991 (PR B), #1074.

Found by the independent review of #991 part 2 (PR B). ## What Nothing removes a `media/private/campaigns/<id>/` directory whose campaign row does not exist. Two ways one comes to exist: - **A backend that dies mid-import** (an OOM kill, a deploy, a restart). The import's own cleanup runs in a `finally`, which a killed process never reaches. The files it had written stay: attachments, page pictures, and each map's `imported` copy. - **A purged campaign.** The review saw no removal of that directory when a campaign is deleted for good. Read only, not confirmed by running; check this first. The files are under `private/`, which nginx never serves, so nothing is exposed. The cost is disk: an interrupted import can leave several hundred megabytes. ## Intended approach - A periodic task (Celery beat, daily) that lists `media/private/campaigns/*` and removes a directory when: - no campaign row has that id, and - the directory's newest file is older than a bound (24 hours), so an import still in flight is never touched. - It logs what it removed and how much. - If a purge really does leave the directory, remove it at purge time as well. - Tests: an orphan older than the bound is removed; a fresh orphan is kept; a live campaign's directory is never touched; a name that is not a UUID is ignored. ## Related #991 (PR B), #1074.

claude-bot referenced this issue

2026-10-04 09:26:40 +00:00

A campaign's export carries handout files and maps (export schema 18) (#991) #1076

claude-bot referenced this issue

2026-10-04 10:33:01 +00:00

Campaign export leaves out quest details, character sheets, maps and handout files #991

claude-bot referenced this issue

2026-10-04 16:37:57 +00:00

[Security] An uploaded image can exhaust the backend's memory: the validator decodes it with no pixel bound #1074

claude-bot commented

2026-10-05 01:49:09 +00:00

Author

Collaborator

Picking this up, on fix/1075-orphan-media-sweep.

Planned

First, check whether purging a campaign really leaves its media directory (read from the code so far, not confirmed).
A daily task that removes a per-campaign media directory only when:
- its name is a UUID;
- no campaign row has that id (a trashed campaign has a row, so its files are kept);
- nothing inside it is newer than 24 hours;
- it is a real directory inside the media root, never a symlink.
If a purge does leave the directory, remove it at purge time as well, after the delete commits.

**Picking this up**, on `fix/1075-orphan-media-sweep`. **Planned** - First, check whether purging a campaign really leaves its media directory (read from the code so far, not confirmed). - A daily task that removes a per-campaign media directory only when: - its name is a UUID; - no campaign row has that id (a trashed campaign has a row, so its files are kept); - nothing inside it is newer than 24 hours; - it is a real directory inside the media root, never a symlink. - If a purge does leave the directory, remove it at purge time as well, after the delete commits.

rbrooks referenced this issue from a commit

2026-10-05 02:06:03 +00:00

fix(webapp): sweep campaign media no campaign owns, and remove it on purge (#1075)

rbrooks referenced this issue from a commit

2026-10-05 02:06:03 +00:00

fix(webapp): keep walk state while any of an orphan's directories is kept (#1075)

rbrooks referenced this issue from a commit

2026-10-05 02:06:03 +00:00

style: ruff format the sweep tests (#1075)

rbrooks referenced this issue from a commit

2026-10-05 02:08:52 +00:00

The orphan sweep removes nothing when there are no campaign rows at all (#1075)

claude-bot referenced this issue from a pull request that will close it,

2026-10-05 02:09:03 +00:00

Files of a campaign that no longer exists are removed, daily and on a purge (#1075) #1093

claude-bot referenced this issue from a commit

2026-10-05 03:19:10 +00:00

Merge PR #1093: files of a campaign that no longer exists are removed, daily and on a purge (#1075)

claude-bot closed this issue

2026-10-05 03:19:10 +00:00

claude-bot commented

2026-10-05 03:19:16 +00:00

Author

Collaborator

Fixed in PR #1093, merged to main with the owner's go-ahead. CI was green on the PR's head.

Confirmed first: a purge left the campaign's files. Nothing removed them.
On a purge: the files go once the delete has committed.
Daily sweep: a campaign folder is removed only when no campaign row has its id (the trash counts as having one), nothing inside has changed for 24 hours, and it is a real directory inside the media root.
Guard: with no campaign rows at all, nothing is removed and a warning is logged.

To know when restoring: after a database backup older than the media volume is restored, files of campaigns made since that backup are swept a day later. docs/OPERATIONS.md says so.

Not yet seen on a real media volume. Its first run on dev will be the beat's own, a day after the next prerelease is deployed. I will read the worker's log for it then.

Reaches dev with the next prerelease.

**Fixed in PR #1093, merged to main with the owner's go-ahead. CI was green on the PR's head.** - **Confirmed first:** a purge left the campaign's files. Nothing removed them. - **On a purge:** the files go once the delete has committed. - **Daily sweep:** a campaign folder is removed only when no campaign row has its id (the trash counts as having one), nothing inside has changed for 24 hours, and it is a real directory inside the media root. - **Guard:** with no campaign rows at all, nothing is removed and a warning is logged. **To know when restoring:** after a database backup older than the media volume is restored, files of campaigns made since that backup are swept a day later. `docs/OPERATIONS.md` says so. **Not yet seen on a real media volume.** Its first run on dev will be the beat's own, a day after the next prerelease is deployed. I will read the worker's log for it then. Reaches dev with the next prerelease.

claude-bot referenced this issue

2026-10-05 05:45:47 +00:00

[Security] Campaign import reads data.json with no cap on its decompressed size #1054