From 457c9563869156bc4773d873661a75d5115b25db Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Mon, 8 Jun 2026 23:01:32 +0200 Subject: [PATCH] fix(admin/events): delete cascade orphaned photo folders because it read a non-existent column (#608) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jodrmx reported on v3.44.0 (Pi Lite, Docker compose): admin-UI event delete removes the DB row but leaves `storage/events/active//` intact on disk. Root cause: `deleteEventCascade` in adminEvents.js read `event.folder_path` and gated the `fs.rm` on it. That column is NEVER WRITTEN anywhere in the codebase — grep confirms two reads in this one function, zero writes elsewhere. So `event.folder_path` was always undefined, `if (event.folder_path)` always false, and the per-folder cleanup silently no-op'd for every delete. The DB-cascade transaction ran fine, so the symptom was always "row gone, files stay" — exactly what jodrmx hit. The actual on-disk location is `events/active/{slug}` everywhere else in the codebase: - adminPhotos.js:260 — `path.posix.join('events/active', event.slug)` - adminEvents.js:610, events.js:155, adminThumbnails.js:153 — read from `events/active/{slug}` - adminArchives.js:171 — reads from same root - photoResolver.js:14-15 — documents the layout The delete cascade was the only path looking at the non-existent column. Cure: drop the `if (event.folder_path)` guard, read `event.slug` instead, and remove from both `events/active/{slug}` (active gallery folder) and `events/archived/{slug}` (the post-archive copy that survives the archive flow). `event.slug` is NOT NULL and slugify- sanitized (lower-case ASCII + dashes only via utils/slug.js), so the path is well-formed and path-traversal-safe. Best-effort `fs.rm` semantics + try/catch unchanged — failures still log a warning rather than unwinding the DB transaction, since orphan files are recoverable noise compared to a half-deleted DB row. Forward fix only — does not retroactively clean up the orphans that have accumulated on existing installs. Admins can `rm -rf storage/events/active/` manually for those; not worth a migration script for a one-time deploy ritual. --- backend/src/routes/adminEvents.js | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/backend/src/routes/adminEvents.js b/backend/src/routes/adminEvents.js index bd8fe4c7..bfe5a76c 100644 --- a/backend/src/routes/adminEvents.js +++ b/backend/src/routes/adminEvents.js @@ -286,9 +286,25 @@ async function deleteEventCascade(eventId, adminContext) { // Best-effort filesystem cleanup. Failures are logged but don't unwind // the transaction — the canonical state lives in the DB; orphan files // are recoverable noise, a half-deleted DB row is a permanent mess. - if (event.folder_path) { - const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage'); - const eventFolderPath = path.join(storagePath, 'events', 'active', event.folder_path); + // + // #608 — previous code read `event.folder_path`, but that column is + // never written anywhere in the codebase (grep confirms: two reads in + // this function, zero writes). It's always undefined, so the + // `if (event.folder_path)` branch silently no-op'd and every event + // delete since this cascade landed left its photos orphaned on disk. + // jodrmx's Pi report (v3.44.0) was the first surfacing. + // + // Files actually live at: + // {STORAGE_PATH}/events/active/{slug}/... (uploaded photos) + // {STORAGE_PATH}/events/archived/{slug}/... (after the event + // was archived — folder copy survives the archive flow) + // + // `event.slug` is NOT NULL on the events table and is slugify-sanitized + // on every write (lower-case ASCII + dashes only via utils/slug.js), + // so path-traversal isn't a concern. + const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage'); + for (const sub of ['active', 'archived']) { + const eventFolderPath = path.join(storagePath, 'events', sub, event.slug); try { await fs.rm(eventFolderPath, { recursive: true, force: true }); } catch (fsErr) {