paul/picpeak
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(admin/events): delete cascade orphaned photo folders because it read a non-existent column (#608)

Browse Source 
jodrmx reported on v3.44.0 (Pi Lite, Docker compose): admin-UI event
delete removes the DB row but leaves `storage/events/active/<event>/`
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/<old-slug>` manually for those; not worth a
migration script for a one-time deploy ritual.
This commit is contained in:
Paul Nothaft
2026-06-08 23:01:32 +02:00
parent fe0d369836
commit 457c956386
1 changed files with 19 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/routes/adminEvents.js
+18 -2
View File
@@ -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) {
//
// #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');
const eventFolderPath = path.join(storagePath, 'events', 'active', event.folder_path);
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) {