1971 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Paul Nothaft	f6da5ad25e	fix(video): avoid a SQLite connection deadlock in the placeholder fallback ... generateVideoPlaceholder() unconditionally called getThumbnailSettings(), which queries the database directly (not through any active transaction). videoProcessor.js's new placeholder fallback can run from inside processUploadedPhotos' open per-file SQLite transaction (chunked video upload) -- knex's default SQLite pool has exactly one connection, so that second, un-transacted query deadlocks against the transaction holding it, timing out after acquireConnectionTimeout (60s). Reproduced directly against an isolated SQLite db. generateVideoPlaceholder now skips the settings lookup entirely when the caller supplies explicit width/height, and the video fallback passes the same DEFAULT_THUMBNAIL_WIDTH/HEIGHT the settings lookup would have fallen back to anyway (now exported for reuse). Found by codex review.	2026-09-10 13:31:10 +02:00
Paul Nothaft	408ae1311f	fix(video): fall back to the SVG placeholder when thumbnail generation fails ... processUploadedVideo could return success with thumbnailKey: null when only thumbnail generation failed. The gallery grid (GridGalleryLayout/JustifiedGalleryLayout) falls back to `photo.thumbnail_url || photo.url` when there's no thumbnail, so AuthenticatedImage downloaded the full original video and tried to render it as an <img> -- a broken tile and a potentially huge fetch just from opening the gallery. Falls back to the same ffmpeg-free SVG placeholder the callers already generate for a total processing failure, so a bare thumbnail-generation failure degrades to that placeholder too, never to "no thumbnail at all". Found by codex review.	2026-09-10 13:17:08 +02:00
Paul Nothaft	80bc2b6e55	fix(video): try metadata extraction and thumbnail generation independently ... processUploadedVideo() gated everything behind isValidVideo(), which rejects the whole video if ffprobe can't read even one of duration/width/height -- common on some iPhone/Lightroom-exported MP4s (issue 1370). Callers (photoProcessor.js's processPhoto and processUploadedPhotos) already catch that throw and fall back to a static placeholder thumbnail plus a metadata-only retry (codex review of #845), but that fallback never got a REAL thumbnail even when generateVideoThumbnail() would have succeeded on its own -- thumbnailing doesn't need valid duration/width/height, it just seeks and grabs a frame. processUploadedVideo now tries metadata extraction and thumbnail generation independently, keeping whichever succeeds instead of discarding both on a single failed field. The callers' existing throw handling stays as a backstop. Also: extractVideoMetadata stored duration as 0 (not null) whenever ffprobe had no duration field, masking "unknown" as a fake real zero-second clip and defeating downstream `duration != null` checks meant to skip an untrustworthy value. Relates to issue 1370	2026-09-10 13:04:32 +02:00
Paul Nothaft	34207456e6	fix(backup): honor the configured database-backup destination path (#1367) ... * fix(backup): stop ignoring the configured database-backup destination path databaseBackupService.getBackupConfig() returns the raw database_backup_*-prefixed setting keys, but backup() and startScheduledBackups() destructured unprefixed names off that object (destinationPath, compress, enabled, schedule, retentionDays, emailOnSuccess/Failure). None of those keys ever existed on the config object, so every read silently fell through to its hardcoded default. The visible symptom (reported in issue 1365): the inline database dump that runs before every file backup (default ON) always tried to create /backup/database, regardless of what an admin configured, and died with EACCES on the read-only default path — before the file backup's own (correctly wired) backup_destination_path was ever reached. The standalone scheduled database-backup runner had the same bug: config.enabled was always undefined, so it silently never started regardless of database_backup_enabled. Also fixes saveManifestToLocal's manifest-directory fallback, which hardcoded /backup instead of matching the sane getStoragePath()/backups default used everywhere else for a missing backup_destination_path. Relates to issue 1365 * fix(backup): reject a database-backup destination inside a public static mount Making database_backup_destination_path actually take effect reopens a GHSA-jw8m-43r2-jqrm-class exfiltration path: that setting is writable via PUT /api/admin/database-backup/config under backup.create alone (the built-in admin role has it without settings.edit or backup.restore), with no path validation. Before this fix the setting was silently ignored (the destructuring bug), so pointing it at the public uploads/logos or fonts mount was harmless; now that it is honored, it needed the same defense GHSA-jw8m already applies to the per-request override. Rejects the setting at both the config write (immediate 400) and, defensively, at backup() time before mkdir. Found by codex review. * fix(backup): close two gaps codex round 2 found in the destination guard - The public-roots list missed the bundled fallback fonts dir (backend/assets/fonts, also mounted at /fonts, and nodejs-owned per the Dockerfile's COPY --chown so it's writable at runtime). - The comparison was case-sensitive; on a case-insensitive-but- preserving filesystem (APFS, NTFS, Docker Desktop bind mounts of either) STORAGE_PATH/UPLOADS/Logos names the same directory as uploads/logos on disk. Now compares lowercased. - database_backup_retention_days reached cleanupOldBackups unvalidated. A value <= 0 pushes the cutoff to today or the future, deleting every completed backup on the next scheduled run -- a backup.create holder achieving what backup.delete gates on the manual /cleanup route. Rejected at config-write time (400) and defensively inside cleanupOldBackups itself. - The scheduled-backup cron callback closed over retention_days from schedule-start time; a retention-only /config update (which doesn't restart the schedule) ran stale until restart. Re-reads it on every tick instead. Found by codex review, round 2. * fix(backup): resolve symlinks and add the all-in-one frontend dir to the destination guard Codex round 3 found two more bypasses of the public-root guard, both specific to the all-in-one image (Dockerfile.aio): - /app/frontend/dist (FRONTEND_DIR) ships nodejs-owned and is served unauthenticated as the built SPA -- missing from the protected-roots list. - /app/storage is a symlink to /data/storage (the actual STORAGE_PATH). A destination given as /app/storage/uploads/logos passed the guard's lexical path.resolve() comparison while resolving, on disk, to the exact same directory as the protected STORAGE_PATH/uploads/logos. isUnderPubliclyServableRoot now resolves symlinks in whatever prefix of each path already exists (resolveRealish) before comparing, rather than relying on path.resolve() alone. Also restores three fs.mkdir spies in the test file that were never un-spied, which silently leaked a rejected mock into any later test doing a real fs.mkdir -- exactly what the new symlink test needed to set up its fixture. Found by codex review, round 3. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-09-09 22:27:26 +02:00
Paul Nothaft	6d906349bf	chore(stable): release 3.46.11 (#1356) v3.46.11	2026-09-08 22:39:40 +02:00
Paul Nothaft	143c4035ec	docs: align stable security and backport policy (#1352) ... Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-09-08 08:33:00 +02:00
Paul Nothaft	99df3e204f	chore(stable): release 3.46.10 (#1332) v3.46.10	2026-09-07 23:03:37 +02:00
Paul Nothaft	95e3af0800	Merge pull request #1327 from PicPeak/fix/sanitize-html-2.17.7-stable ... fix(security): bump sanitize-html to 2.17.7 (stable)	2026-09-07 09:18:32 +02:00
Paul Nothaft	8421b7b668	fix(setup): require Node 22.12 for sanitize-html	2026-09-07 09:00:02 +02:00
Paul Nothaft	0f426ef699	fix(security): bump sanitize-html to 2.17.7 ... Trivy flags the backend image on two sanitize-html advisories, both fixed upstream: - CVE-2026-63670 (fixed 2.17.6): a literal solidus after a raw-text end tag (`</textarea/>`) is treated as text by htmlparser2 and re-emitted unescaped, so disallowed markup passes when textarea or xmp is in allowedTags. - CVE-2026-84371 (fixed 2.17.7): an SVG SMIL animation whose attributeName selects href lets the sibling values/from/to/by attributes carry URLs past the scheme policy. 2.17.5 -> 2.17.7, exact pin as before. The new version brings its own htmlparser2 12 / domhandler 6 / domutils 4 / dom-serializer 3 / entities 8 tree under node_modules/sanitize-html; nothing else in the lock moves. That tree is ESM-only, so the backend now needs unflagged require(esm): Node 20.19+ or 22.12+. The image is node:22-alpine and CI runs 22, but engines.node still admitted 22.0-22.11, where require('sanitize-html') throws ERR_REQUIRE_ESM at startup (publicSiteService loads it during initialisation). engines is now ^20.19.0 || >=22.12.0 and the native setup script's Node check enforces the same range instead of accepting any 22.x. On the supported versions the sanitiser behaves identically to 2.17.5 on the tracker and newsletter fixtures. Jest 29's CommonJS registry cannot evaluate ESM either, so every suite importing a route or service that uses the sanitiser would fail at import. jest.config.js now maps `sanitize-html` to jest.sanitizeHtml.js, which hands that one module to Node's real loader via process.getBuiltinModule('module') — a plain require('module') inside Jest is Jest's wrapper and returns an empty object for this package. Verified against a real 2.17.7 install: the sanitiser suites and a settings route suite pass; without the mapper they fail with "Cannot use import statement outside a module".	2026-09-06 23:06:45 +02:00
Paul Nothaft	be243aafe8	chore(stable): release 3.46.9 (#1283) v3.46.9	2026-09-03 22:26:45 +02:00
Paul Nothaft	3f90221f40	Merge pull request #1281 from PicPeak/fix/security-scan-batch-1-stable ... fix(security): batch 1 (stable) — zxcvbn DoS, revocation forgery, unlink traversals, stored Content-Type, edge middleware	2026-09-03 12:36:42 +02:00
Paul Nothaft	ed0a8e7656	chore(deps): apply non-breaking npm audit fixes ... Stable twin of the main commit: backend qs/body-parser, frontend axios, dompurify, linkify-it and the transitive set npm audit fix resolves without a major bump. sanitize-html 2.17.7 (ESM-only parser tree, Jest 29 cannot load it; the advisory needs svg tags no sanitizer config allows) and the tiptap / react-router majors are left out, as on main.	2026-09-03 12:15:53 +02:00
Paul Nothaft	c89ce8e172	docs: say the upload allow-list covers every path, video extensions must be added ... Settings help text for Allowed File Types (EN, DE). The reference page lives in the docs repository (PicPeak/docs#18). (cherry picked from commit f3b062a3, locale files only)	2026-09-03 12:15:39 +02:00
Paul Nothaft	d81cade7cc	fix(security): harden four smaller gallery and contract paths, drop the unmounted photo auth middleware ... - the customer contract PDF stream applies assertContractPdfPath like the admin and public contract routes - OG previews fall back to the site card for draft, archived and deactivated galleries instead of leaking name, date and welcome message - video Range requests are validated before the 206 is written; a NaN, inverted or out-of-file range now answers 416 - share-token comparisons in gallery resolve/info use the constant-time helper share-login already used - middleware/photoAuth.js and the galleryAuth/photoAuth/verifyGalleryAccess exports of middleware/auth.js were unreferenced since the static mounts went; the auth.js copy had neither slug binding nor issuer pin, so it is removed before anyone mounts it (cherry picked from commit 835312e8e6)	2026-09-03 12:15:21 +02:00
Paul Nothaft	406c638451	fix(security): stop reflecting submitted values in validation errors everywhere, cap credential lengths, close the login timing oracle ... safeValidationErrors moves to utils/routeHelpers and replaces every res.status(400).json({ errors: errors.array() }) in the routes, so no 400 body carries the submitted value any more (setup, customer auth and customer change-password were still echoing rejected passwords). Admin login, gallery verify, customer login/register/reset, customer change-password and setup now cap username/slug at 255 and passwords at MAX_PASSWORD_LENGTH at the validator, so an oversized value never reaches the lockout lookup, bcrypt or the failed-attempt log. Admin and customer login run one bcrypt compare on every path; the unknown account branch used to return in microseconds against ~100ms for a wrong password, which enumerated usernames despite the generic message. (cherry picked from commit 40a8a9882a)	2026-09-03 12:14:42 +02:00
Paul Nothaft	b1369068ae	fix(security): close three middleware gaps around the API edge ... Stable port of the main commit; the admin-preview and maintenance-gate items do not exist on this branch. - the general rate limiter skipped anyone holding any verified JWT; a gallery token is minted for free on password-less galleries and slideshow links, so that was an unlimited budget for every /api route. Only admin sessions skip now - the 50mb JSON limit is scoped to /api/admin and /api/v1; everything else gets 2mb, so an unauthenticated body can no longer stall JSON.parse - the CSRF Content-Type gate accepted multipart from any origin; cross-site form posts are now rejected via Sec-Fetch-Site / Origin, with a Host match fallback for same-origin installs that leave FRONTEND_URL unset (cherry picked from commit 839bf4e4, adapted)	2026-09-03 12:14:02 +02:00
Paul Nothaft	a8d57f0d69	fix(security): never serve a photo under its stored MIME, and stop trusting the chunked-upload type ... chunked-upload/init stored the client-declared mimeType on the photo row and the gallery, secure-image and protected-image routes echoed it as Content-Type, so a JPEG/HTML polyglot declared as text/html rendered inline on the app origin for every guest. The admin photo route already resolved the type safely (#908 review); that logic now lives in utils/photoContentType and every serving route uses it. The chunked path derives the MIME from the filename extension and requires that extension to be on the admin allow-list, matching what the multipart path enforces through its multer fileFilter. (cherry picked from commit 063977d97d)	2026-09-03 12:12:56 +02:00
Paul Nothaft	882101b586	fix(security): contain logo, favicon and PDF-logo unlinks to their upload directories ... Settings > Branding persisted logo_url / favicon_url verbatim and on clear unlinked path.join(storage, url) behind a startsWith('/uploads/logos/') check, which '..' segments pass. The business-profile PDF logo did the same behind a /pdf-logo-\d+\./ marker test, and used absolute values as given. Either let a settings.edit or settings.banking holder delete any file the process can reach. Both now resolve through helpers in utils/safePath that only ever name a flat leaf inside the fixed directory. The /favicon.ico streamer is narrowed the same way: it contained to the whole uploads/ root, which also holds signed contracts and transfer files. (cherry picked from commit 3e46530072)	2026-09-03 12:12:12 +02:00
Paul Nothaft	c6d401685f	fix(security): verify the signature before writing a token to the revocation list ... revokeToken() base64-decoded the payload without checking the signature and inserted a row keyed on id-iat-type, the same key isTokenRevoked() matches for real sessions. The logout endpoints are unauthenticated, so anyone could forge a payload naming another user's id, type and login second and log them out remotely; a far-future exp also left rows that cleanup never swept. Expiry is still ignored so logging out an expired session stays idempotent. (cherry picked from commit 0ca0e4a922)	2026-09-03 12:12:12 +02:00
Paul Nothaft	6481708def	fix(security): stop reflecting submitted passwords in validation errors ... Codex review round 2. The 400 I added in the previous commit returned errors.array() verbatim, and express-validator puts the submitted `value` in each error -- so rejecting an oversized password echoed that password back, and re-allocated up to the 50mb body limit on an unauthenticated endpoint, partly undoing the denial-of-service fix this branch exists for. The same call appeared at seven sites in this file, five of which validate a password field: /admin/login, /gallery/verify, /gallery/:slug/client-login, /admin/change-password and /password-strength. Every failed login was returning the attempted password in its response body, where it reaches proxy logs, error monitoring and browser tooling. Fixed at all seven rather than only the one the review pointed at. Only `value` is dropped. `msg`, `path` and the rest are kept, because the two shapes express-validator produces are both consumed in the frontend -- AcceptInvite reads {field, message} from routeHelpers.validateRequest, EventDetails reads {msg, path} from raw errors.array() -- and switching auth.js to the helper's shape would have broken the latter for a reason unrelated to security. 1 more test. Backend suite: 2744 passed. (cherry picked from commit 903e471753)	2026-09-03 12:12:12 +02:00
Paul Nothaft	706d402c1e	fix(security): enforce the strength-endpoint validators, and stop the generator spinning ... Codex review round 1 on the batch-1 security fixes. One finding is a regression this branch introduced. generateSecurePassword retried by recursing on any candidate validatePassword rejected. The new 128-character cap makes EVERY candidate invalid once a caller asks for more than that, so `generateSecurePassword({ length: 129 })` went from returning a password to unbounded recursion and a stack overflow. It now refuses an impossible length up front, and the retry is a bounded loop rather than recursion -- every candidate failing is possible for reasons other than bad luck (a charset that cannot satisfy the configured policy), and that case deserves an error someone can act on rather than a blown stack. No caller in the repo passes a length at all; the hazard was in the exported surface. The route validators were decorative. POST /api/auth/password-strength never called validationResult(), so the length bound I added only recorded an error that nothing read: the oversized body still reached zxcvbn and the endpoint still answered 200. The cap inside validatePassword() was doing all the work. Errors are now returned as a 400 before the validator runs, which is what the previous commit claimed. Also awaited validatePasswordInContext, which is async. Unawaited, `validation` was a Promise and every field in the response -- valid, score, errors, feedback -- came back undefined. Pre-existing, in the lines this change already touches, and it made the endpoint useless for the real-time validation it exists for. 1 more test. Backend suite: 2742 passed. The 23 eslint errors in server.js are pre-existing and identical on main. (cherry picked from commit 054cd6f82f)	2026-09-03 12:12:12 +02:00
Paul Nothaft	ed08ff84ff	fix(security): bound password input before zxcvbn, and drop the legacy media mounts ... Two findings from the GHSA-pwx6-5pqc-c5xq scan bundle, both verified against the code and reproduced before fixing. **Unauthenticated denial of service via password strength (csf_495d53fa).** POST /api/auth/password-strength takes `body('password').notEmpty()` with no upper bound, sits behind express.json({ limit: '50mb' }), and hands the string to zxcvbn, whose matching is superlinear and runs synchronously on the event loop. Measured on this codebase, in ms of blocked loop: 128 -> 41, 512 -> 1367, 1000 -> 5097, 5000 -> did not return in two minutes. One unauthenticated request of about a kilobyte stops the whole process for five seconds; a few kilobytes stops it indefinitely. The /api/auth rate limit does not help when a single request is already enough. The cap lives in validatePassword() so it covers every caller, present and future; the route validator is defence in depth. 128 keeps the worst case at the cost of an ordinary request while staying far above any real password -- bcrypt consumes only the first 72 bytes, so length past that adds no entropy to the stored hash anyway. This is the only unauthenticated reach into zxcvbn: setup is token-gated and self-closing, and acceptInvite/adminAuth use the regex-only validator in passwordGenerator. **The /photos and /thumbnails static mounts (csf_9aa6afe6, csf_559cd5cc, csf_b14d462e, csf_547d26fa, and the gallery half of csf_34e420af).** They served the raw originals and thumbnail trees behind photoAuth, which authorises on a slug match. A static file server cannot apply per-photo rules, so everything the gallery API decides was absent: allow_downloads, per-category allow_downloads, watermarking, the resolution cap, reveal-mode windows, visibility='hidden', download logging, and the customer-assignment re-check that makes revocation immediate. photoAuth also bcrypt-compares an x-gallery-password header per request with no limiter -- both rate-limit gates return early for non-/api paths -- so the mount was an unmetered password oracle. The filenames needed to drive all of this are handed to every guest in the photos listing. Nothing builds these URLs: no reference in frontend/src, none in the email templates, and the only backend mentions are the /api/admin/photos/... API routes and a maintenance-mode prefix list. The equivalent authorised routes are /api/gallery/:slug/photo/:id and /thumbnail/:id. nginx still proxies the two locations; they now 404, which is the intent. **The /uploads mount (csf_1fc92f57).** It exposed the whole uploads/ root with no auth middleware at all, and that root also holds signed contract PDFs (uploads/contracts/signed) and client transfer files (uploads/transfers/<id>), reachable by anyone who learned or guessed a filename. Narrowed to the two public asset trees it exists for; contracts and transfers keep their own authorised routes. Removing the mounts leaves src/middleware/photoAuth.js unreferenced by application code. Left in place deliberately -- deleting it and its tests is a separate cleanup, and a smaller diff backports more safely. Backend suite: 2742 passed. (cherry picked from commit 14cd5eacb3)	2026-09-03 12:12:06 +02:00
Paul Nothaft	7fe80220f1	chore(stable): release 3.46.8 (#1250) v3.46.8	2026-09-01 22:29:02 +02:00
Paul Nothaft	ccdcdd6116	chore: keep issue screenshots out of the source tree (#1260) ... Preventative twin of the main-side cleanup. This branch has no stray images to remove — it just gets the same guard, so the two branches agree and a backport cannot carry one across. Two PR screenshots landed at main's repo root in #1241 and shipped as part of the source tree. Screenshots belong on a `screenshots/*` branch, which is how every other UI change here has attached its evidence. Anchored with a leading slash so docs/ keeps its own images and test-assets/ keeps the fixtures the e2e specs load. Verified no tracked file on this branch matches the new patterns. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-09-01 09:34:55 +02:00
Paul Nothaft	ccc725f36e	fix(upload): let Android guests reach the camera without breaking video (#1248) ... * fix(upload): let Android guests reach the camera without breaking video Stable twin of #1244 (which replaces #1117). The reporter is on 3.46.1, so this branch is where the bug is actually being hit. Recent Android versions route an <input> whose accept list is entirely image/video types to the system photo picker, which has no camera entry — so a guest standing at the event can only pick an existing photo, not take one. Including a type that picker can't handle forces the general chooser, which does offer the camera. Gated on the Android UA: iOS and desktop pickers behave correctly and would only gain a selectable PDF that addFiles then rejects. No image-only guard — #1117 added one that broke video uploads outright on any install configured for them, and it was redundant anyway, since extensionsToMimeTypes only emits types it has a mapping for and the existing allowlist check already rejects a picked PDF. The premise — that this actually surfaces the camera option on Android — is taken at the reporter's description level and still needs confirmation on a device. Co-authored-by: Zszywany <Zszywany@users.noreply.github.com> * fix(upload): use android/allowCamera instead of .pdf for the chooser fallback Same mechanism, better token. Chrome on Android 14/15 sends an input whose accept list is all media types to the photo picker, which has no camera tile; adding a value that picker cannot satisfy makes it fall back to the general chooser, which does offer the camera. `.pdf` achieves that but advertises PDFs as selectable — pick one and the existing allowlist check answers "Invalid file type", which is a dead end we put in front of the guest ourselves. `android/allowCamera` is the token the workaround converged on: not a real MIME type, matches no file, so it flips the picker without offering anything. Neither token ever widened what is accepted — addFiles validates against extensionsToMimeTypes, which only emits types it has a mapping for — but not showing the guest a choice that cannot work is worth the one-line change. Verified in a browser rather than asserted: the real component rendered under an Android UA emits image/jpeg,image/png,image/webp,android/allowCamera and under a desktop UA image/jpeg,image/png,image/webp with the visible modal identical in both, and the format hint still reading "JPG, JPEG, PNG, WEBP" — the token does not leak into anything a guest sees. * fix(upload): keep the camera token off Firefox for Android External review round. The gate was a bare /Android/i, which Firefox for Android matches — so it received a token invented to reroute Chromium's photo picker, a picker it does not use. The doc comment two lines up already said Firefox behaves correctly; the code did not agree with it. Inert at best, and at worst it perturbs a chooser that was working. Narrowed to Android minus Firefox, which is the Chromium-family set the behaviour was actually observed on (Chrome and Edge, Android 14/15), with a UA test to pin it. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local> Co-authored-by: Zszywany <Zszywany@users.noreply.github.com>	2026-09-01 09:24:38 +02:00
Paul Nothaft	fed99ac03d	fix(archives): write a real timestamp on restored photos (#1257) ... Brings this branch in line with main, which fixed it in passing. The archive restore inserted photos with a bare Date for uploaded_at. Inside jest the sqlite3 binding's type dispatch misses sandbox-created Dates and stores the literal string "[object Object]", so every restored photo got a garbage timestamp. Verified on this branch rather than assumed: bare Date -> "[object Object]" toISOString -> "2026-09-01T06:48:41.915Z" Production writes Dates as ms-numbers and is unaffected, which is exactly why it survives unnoticed — it only corrupts what tests read back, so a future test asserting on a restored photo's date would have believed it. The regression test fails against the previous line. Not touched: the category insert a few lines up has the same shape, but it is identical on main, so fixing it here alone would re-open the divergence this commit closes. Worth one small PR against both branches. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-09-01 08:52:35 +02:00
Paul Nothaft	a01731d986	chore: remove .pyc files committed to stable by mistake (#1256) ... My doing, in #1247: I built that stable twin in a working tree that still held untracked bytecode from main's ML sidecar, and a `git add -A` swept 16 .pyc files in alongside the two real ones. Stable never carried the ignore rule because the sidecar itself is main-only — which is precisely why nothing stopped it here. Added, so a shared working tree cannot repeat it. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-09-01 08:41:37 +02:00
Paul Nothaft	1d9f0b6c64	fix(events): apply the gallery password policy to publish (#1255) ... Stable twin of #1253. This branch has only the publish door — send-gallery-email is #1235, main-only — so the same gap exists here in one place rather than two. /publish re-hashes password_hash from a plaintext the admin re-types in the publish dialog, validated with nothing but express-validator's isLength({min:6}). So the configured complexity — moderate by default, meaning 8 characters plus upper, lower and a digit — governed event creation and password reset while this door accepted 'aaaaaa' and made it the live gallery password. Not an escalation: it needs admin auth plus events.edit. It is a policy gap, the admin UI advertising a complexity level this write path did not enforce. BEHAVIOUR CHANGE: an API-only consumer publishing with a sub-policy password now gets 400, with the same body shape event creation returns (error, details, score, feedback). 3 tests, including that the rejection happens BEFORE the write — the gallery keeps its old hash and stays a draft — and that a publish carrying no password at all is untouched. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-09-01 08:36:41 +02:00
Paul Nothaft	261e243070	fix(archives): take the restored category from the manifest (#1240) (stable) (#1243) ... * fix(archives): take the restored category from the manifest (#1240) (stable) Stable twin of #1240. The reporter hit this on 3.46.7 — stable — with 596 photos restored and 0 categories, so this is the branch the bug was actually found on. Stable carries the manifest on both sides already: archiveService selects `photo_categories.name as category_name` and serialises it, and the restore route builds manifestByFilename. It just never read the category out of it, deriving one from the ZIP's first path segment instead. Archives store photos as they sit on disk, so an event whose photos live in the gallery root produces a flat zip, no category resolves, and every photo comes back with category_id null — silently, behind a 200. Carries the whole of #1240, not a subset: the manifest-first resolution and the shared resolveCategoryId from Marian's commit, plus the follow-up that makes the manifest authoritative when it says "no category" — an entry with a null category_name is a photo that was genuinely uncategorized, and falling through to the directory contradicted the record being restored from. That matters because the directory is not a category: entry names are the storage key minus events/active/{slug}, so a real archive yields `individual/` and `collages/`, and reading the first segment invents categories with those names. Re-verified on stable rather than assumed: all four tests pass here, and three of them fail against stable's current route, with the legacy no-manifest fallback passing either way. The two changed files are byte-identical to main. Co-authored-by: Marian <lippitz.marian@yahoo.de> * fix(archives): keep the stable twin to stable's schema, and close two category holes External review on #1243 caught that this twin was ported wrong and that the category resolver has two holes the main PR shares. PORTED WRONG. I took main's whole adminArchives.js rather than applying the category change to stable's, which dragged in main-only face cleanup: photo_faces and event_people have migrations on main and none on stable, so every permanent archive deletion would have thrown a missing-table error — after the ZIP was already unlinked, leaving the event archived with its archive gone and a 500 back. Rebuilt from stable's file with only the category change; the diff against stable is now the fix and nothing else. GLOBAL CATEGORIES WERE CLONED. Seeded categories (Ceremony, Reception) have event_id NULL, so an event-only lookup missed them and created a second row — and is_global defaults to TRUE, so that duplicate then appeared in every other event's category list. The lookup now uses the same visibility rule the photo routes use (own rows OR global), and anything it does create is explicitly is_global false. ORIGINAL-FILENAME ARCHIVES MATCHED NOTHING. With general_use_original_filenames_for_downloads on at archive time, archiveService names each ZIP entry after the original filename while the manifest stays keyed by photos.filename — so the lookup missed every entry and those archives lost categories exactly as before the fix. The manifest is now indexed by original_filename as well, without letting it shadow a real filename key. 7 tests, three of them new; each new one fails against the un-fixed route and the legacy no-manifest fallback passes throughout. * fix(archives): sanitized original names and deterministic category scope Round 2 of external review on #1243. The original_filename index used the raw column, but archiveService runs the name through sanitizeForZipEntry() before writing the entry — so an original containing a slash or control byte was emitted under a different name than the manifest records, and the lookup missed it. Both spellings are indexed now, using the same helper the writer uses. Not total, and the comment says so: uniquifyZipNames() appends `_1` when two photos in one event share an original name, and that suffix cannot be reconstructed from the manifest. Those fall through to the directory exactly as they did before this fix — no worse, just not better. Closing it needs the emitted name recorded at archive time, which is a writer change and a new archive format. The category lookup used one OR-query with .first(). An event-scoped category and a global one may share a name — the category API permits it — so the engine picked whichever, and a photo could be silently reassigned to the global row, losing event-local settings like allow_downloads. Two queries now, event-scoped first: the event's own row is the more specific answer. 9 tests, two new; both fail against the un-fixed route. * fix(archives): don't adopt another event's legacy row, don't guess an alias Round 3 of external review on #1243. The global fallback matched on is_global alone. The very bug fixed here left rows behind on upgraded instances — event-owned AND is_global true, because the column defaults true — so restoring event B could adopt event A's leftover, tying B's photos to a category that disappears when A is deleted. The fallback now requires event_id IS NULL: genuinely global, not merely flagged. The original-filename alias map collapsed rows that share a basename. archiveService treats `individual/IMG.jpg` and `collages/IMG.jpg` as distinct paths and suffixes neither, so both manifest rows claimed one alias and whichever won handed the other photo someone else's category. An alias claimed by more than one row is now dropped and logged, so those photos fall back to the directory: an unresolved category is recoverable, a confidently wrong one is not. 11 tests, two new; both fail against the un-fixed route. * fix(archives): make the manifest lookup order-independent and collision-safe Two bugs found by an external review round, both in the manifest index. The canonical map silently kept the last row for a duplicated photos.filename. That column is not unique within an event — s3AutoImporter takes path.basename(entry.key) and dedupes by path, so two imported files in different subfolders both land as IMG_1234.jpg with different paths. At restore both ZIP entries reduce to the same basename, so one photo got the other's category. Contested names are dropped now, like ambiguous aliases already were. The alias pass could also evict a canonical key: when one row's original_filename equalled another row's filename, the collision was marked ambiguous and the sweep deleted the canonical entry. The comment two lines above says a real filename key is authoritative and must never be overwritten — the code did the opposite, and which way it went depended on manifest iteration order, since the archive query has no ORDER BY. Split into two passes so canonical names are claimed first and aliases only fill names no canonical row wanted. * fix(archives): treat a canonical/alias name clash as ambiguous, resolve categories lazily Round-2 findings, one of which corrects my own round-1 fix. Round 1 made a canonical filename outrank any alias. That is the wrong tiebreak: when photo A's filename equals photo B's original_filename, which file the ZIP actually emitted under that name depends on whether original-filename archiving was on at archive time — with it ON the entry is B's, with it OFF it is A's — and the manifest does not record the mode. Preferring either silently mislabels the other half of the time, so the name is dropped and both fall through to the directory. What the two-pass split still buys is determinism: the archive query has no ORDER BY, so this used to be a coin flip between dropping the name and overwriting it. Categories are resolved inside the !existingPhoto branch. resolveCategoryId find-or-CREATES, and archiveEvent retains photo rows, so restoring an archive whose rows still exist created a category from the stale manifest name that nothing then used — renaming a category while its event was archived left the old name behind as an empty duplicate. Not fixed: two event-scoped categories may share a display name with distinct slugs, and the .first() lookup then picks either row, so manifest entries from both collapse onto one id and can inherit the wrong allow_downloads. Detecting it is easy; resolving it correctly needs a stable category identifier in the manifest, which is a writer change and an archive-format bump. * fix(archives): make a duplicate category name deterministic, and log it Round-2 finding. Two event-scoped categories may share a display name when their slugs differ, and the .first() lookup then picked one arbitrarily — manifest entries for both collapsed onto a single id and half the photos inherited the wrong per-category settings, allow_downloads above all. Fixing it properly needs a stable category identifier in the manifest: a writer change, an archive-format bump, and no help at all for archives already written. Not worth building before knowing it happens. So the collision is surfaced instead — a warning naming the category and the row count — and the tiebreak is made deterministic (lowest id) so at least a re-run lands the same way twice. If this never fires in real logs, the format change was not worth making. If it does, this is the evidence for it. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local> Co-authored-by: Marian <lippitz.marian@yahoo.de>	2026-09-01 08:17:48 +02:00
Paul Nothaft	5470fbe406	fix(gallery): route single-photo downloads through the storage backend (#1246) ... * fix(gallery): route single-photo downloads through the storage backend Stable twin of #1048. The route resolved a local filesystem path unconditionally and handed it to res.sendFile. On an S3/R2 deployment managed photos are never on local disk, so every per-photo download failed — while download-all and secure-images worked, because they already went through getStorage(). That asymmetry is why it went unnoticed: the gallery looks healthy until a guest clicks the download button on one photo. Because sendFile is called WITH a callback, Express does not send a response when the file is missing and the callback only logs — the request does not 404, it hangs until the client gives up. The new tests pin this: all five backend-path cases time out against the current implementation. - watermark branch: materialize a tmp local copy via withLocalCopy in S3 mode and hand applyWatermark the copy's PATH, so its path-keyed cache still applies. Same pattern the zip builders in this file already use. - pass-through branch: local disk keeps res.sendFile, which emits Content-Length, Accept-Ranges, ETag and Last-Modified and answers Range with a 206. Sharing one bare stream.pipe(res) with S3 would silently drop all of it, and a resumed download would append a second full body onto the partial file. On S3 the parts that matter are reproduced via stat() and getRange(). - external/reference photos keep the local-path fallback unchanged — resolvePhotoStorageKey returns null for them. Ranges are parsed defensively: an unchecked parse yields NaN bounds and a 206 with a nonsense Content-Range, which corrupts a resumed download rather than failing it. Malformed or unsatisfiable ranges fall back to a 200. Written against stable's shape rather than cherry-picked — main's version delegates to renderPhotoForDownload (#858), which does not exist here. Co-authored-by: Peifu Mo <peipeimo@users.noreply.github.com> * fix(gallery): open the stream before staging download headers, honour If-Range Both from an external review round on #1048. stat() succeeding does not mean get() will — a concurrent delete or replace, or a transient backend error, lands between them. The fetch was awaited AFTER the headers went out, so the range branch had already called writeHead(206) and the outer catch could only throw ERR_HTTP_HEADERS_SENT (in practice the request hangs), while the full branch would have sent its 500 JSON underneath the staged image/jpeg attachment headers. Opening the stream first also lets a vanished object answer 404 and a transient failure answer 500. If-Range: emitting Last-Modified without honouring the validator built from it is the dangerous half. A client resuming after the object was replaced would get 206 from the NEW bytes and splice two versions into one corrupt file. A non-matching validator now falls back to a full 200. * fix(gallery): HEAD without egress, classify render failures, stage 206 headers Round-2 findings from the external reviewer on #1048, ported. Express routes HEAD through this GET handler and Node discards the body, but the pipe still drains the whole object out of S3 first — a metadata probe cost a full transfer in egress and latency. Everything a HEAD needs is already in stat(). The watermark branch reported every failure as 404. It can equally fail because getToFile timed out, tmp filled up, or sharp died; calling that "photo not found" misleads the guest and hides the incident. The 206 path uses status()+set() instead of writeHead(), which commits immediately and left a stream erroring at byte zero with no outcome but a destroyed connection. Staged headers flush on first write, so that case now returns a clean retryable status. pipeStreamToResponse also cleared Content-Type, Content-Length, ETag and Content-Disposition but not the range headers, so the 500 went out still advertising Content-Range — telling a resuming client the error body IS the partial content. * fix(gallery): answer HEAD before the counters Round-3 finding on #1048, ported. The HEAD short-circuit was inside the storage branch, below both the download_count increment / access_logs insert and the watermark path — so a download manager's metadata probe counted as a real download, and on a watermarked gallery it also pulled the original from S3 and ran sharp over it to build a body Node then discards. HEAD now leaves right after the access checks. Content-Length is included only when the photo ships untransformed and the size is readable from stat(); a watermark changes the length and the only way to learn it is to do the work this branch exists to avoid. Uses stable's inline watermark resolution — resolveWatermarkSettings comes from downloadRendition (#858), which does not exist on this branch. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local> Co-authored-by: Peifu Mo <peipeimo@users.noreply.github.com>	2026-09-01 08:17:19 +02:00
Paul Nothaft	7102687ee8	fix(events): delete stored objects when cascading an event delete (#1245) ... * fix(events): delete stored objects when cascading an event delete Stable twin of #1051. Deleting an event removed its database rows but left every stored file behind: deleteEventCascade() cleaned up with fs.rm over {STORAGE_PATH}/events/{active,archived}/{slug}, and on an S3-compatible backend those paths don't exist locally — the call succeeds against nothing and the real objects stay in the bucket, unreferenced by any row, invisible in the UI, and billed every month. Measured on a v3.45.16 install against Cloudflare R2, deleting one 403-photo event: bucket object count 5,400 before and 5,400 after, while referenced rows dropped from 3,425 to 2,746. Keys are collected BEFORE the transaction removes the photo rows — once they are gone nothing records which objects belonged to the event, and only a full-bucket audit against the whole database could find them again — and deleted AFTER the commit, so a rolled-back delete can never destroy files for an event that still exists. Includes photo.watermark_path and event.archive_path, both storage-backed and both previously fs.unlink-only. event.hero_logo_path is deliberately excluded: multer writes logos to local disk with diskStorage regardless of backend, so they are never bucket objects. Reference/external photos are left alone — resolvePhotoStorageKey returns null for them and PicPeak does not own those bytes. This branch carries the higher priority of the pair: unlike main, stable's deleteEventCascade never calls getStorage() at all, and the leak costs real money for every month it goes unfixed. Co-authored-by: Peifu Mo <peipeimo@users.noreply.github.com> * fix(events): sweep the Download All cache, and delete objects concurrently Both from an external review round on #1051. The pre-built "Download All" zip (events.download_zip_path) lives under events/active/{slug}/.download-cache/. On local disk the recursive fs.rm already covered it, which is exactly why it was easy to miss — on S3 that prefix is not a directory, nothing covered it, and it is gallery-sized. downloadZipService exposes a cleanup() documented as "used on event deletion" that the cascade never called. download_jobs (main's #173) does not exist on this branch, so the per-job archives main also sweeps have no counterpart here. Deletes now run through a bounded pool instead of one await per key: a 400-photo gallery owns well over a thousand objects, and that many sequential DeleteObject round trips runs to minutes — long enough for a proxy to time the request out AFTER the commit, leaving the event deleted and the sweep half-finished. * fix(events): never delete a derivative another gallery still uses Round-2 findings from the external reviewer on #1051, ported. Canonical thumbnail/hero/preview keys are not event-scoped: the basename is the photo's filename, and filenames are not unique across events. A legacy gallery can share a canonical derivative with a photo in another event, and deleting it here blanked a surviving gallery's tile. Derived keys are now checked against photos outside this event and anything still referenced is left alone; if the check itself fails, every derivative is kept — an orphan costs storage, a deleted derivative costs someone else's gallery. Originals need no check, their keys embed the slug. Also cancel any in-flight or debounced Download All build before snapshotting paths, via downloadZipService.cleanup() — the service's own entry point for event deletion. A builder that started before the delete would otherwise upload a gallery-sized zip after the sweep and write its path onto a row that no longer exists. * revert(events): drop the Download All build cancellation It broke CI on this branch: the backend job went from ~2 minutes to exceeding its 10-minute budget, twice, reproducibly. downloadZipService.cleanup() reaches getStorage() through _cleanup(), and in a suite where the S3 backend is configured but unreachable every cascade delete then pays the adapter's retry backoff. The full suite passes locally against SQLite, which is why this only showed up in CI. The race it addressed is real but narrow — a builder that started before the delete uploads its zip after the sweep and writes the path onto a row that no longer exists, orphaning one object. That is a cheaper problem than an unrunnable test suite, so it goes back to being a documented follow-up rather than shipping behind a timeout. The shared-derivative guard from the same review round stays: that one prevented deleting a surviving gallery's thumbnail. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local> Co-authored-by: Peifu Mo <peipeimo@users.noreply.github.com>	2026-09-01 08:16:51 +02:00
Paul Nothaft	5b69e3ec4c	fix(auth): treat zxcvbn suggestions as advice, not blocking errors (#1247) ... Stable twin of #1050. passwordValidation.js is byte-identical between the branches, so this is the same change verbatim. validatePassword() appended zxcvbn's feedback.suggestions to the errors array unconditionally, and validity is errors.length === 0 — so any password that merely earned a suggestion was rejected even when it satisfied every configured rule. The effective policy was stricter than the configured complexity level and invisible to the admin. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local> Co-authored-by: Peifu Mo <peipeimo@users.noreply.github.com>	2026-09-01 08:16:33 +02:00
Paul Nothaft	eebca9900b	fix(auth): treat zxcvbn suggestions as advice, not blocking errors (#1247) ... Stable twin of #1050. passwordValidation.js is byte-identical between the branches, so this is the same change verbatim. validatePassword() appended zxcvbn's feedback.suggestions to the errors array unconditionally, and validity is errors.length === 0 — so any password that merely earned a suggestion was rejected even when it satisfied every configured rule. The effective policy was stricter than the configured complexity level and invisible to the admin. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local> Co-authored-by: Peifu Mo <peipeimo@users.noreply.github.com>	2026-09-01 08:05:57 +02:00
Paul Nothaft	c05faa50d9	chore(stable): release 3.46.7 (#1221) v3.46.7	2026-08-29 03:25:38 +02:00
Paul Nothaft	15c844db06	fix(admin): the "Uncategorized" photo filter returns every photo (#1211) (#1215) ... Stable twin of the same fix on main. The dropdown rendered as `value="0"` and adminPhotos.js:1001 skips '0', so no category condition was applied and the whole event came back. The branch that does the work sits four lines below, keyed on the literal 'uncategorized' that nothing was sending. Silent by nature — a full list reads as 'nothing to narrow' rather than 'the filter did not run' — which is why it survived this long. The reporter in #1209 is on 3.46.4, so this is the branch that reaches them. Tests both ends of the contract, since the bug was the pairing rather than either half. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-28 08:05:21 +02:00
Paul Nothaft	c685a3e931	chore(stable): release 3.46.6 (#1207) v3.46.6	2026-08-28 04:00:46 +02:00
Paul Nothaft	74ff236b51	fix(images): fence the capture-date backfill on the file it read (#1201) (#1205) ... Stable twin of #1204. The capture-date backfill committed its result keyed on the row id alone. It snapshots every candidate up front, then walks them one at a time reading originals off S3 or a NAS mount — a pass that can run for many minutes. replacePhoto, reachable from the replace_by_name upload path, swaps a NEW file under an existing row and rewrites path/filename. A replacement landing inside that window carries no date of its own, so captured_at was still NULL, the whereNull guard passed, and the previous file's EXIF date was written onto the new photo. Silent: nothing errored, the run reported it as a success, and the gallery just sorted that photo to the wrong place. Fenced on path and filename as well as the id, so a replaced row matches zero rows and is skipped. The candidate query already selects both columns, so no query change. Knex renders a null value in the object form as `is null` on both the pg and sqlite3 clients, so a row with a NULL path still matches itself. Those skipped candidates are now counted rather than dropped. replacePhoto is not the only writer of path/filename — eventRenameService rewrites both on an event rename, which is not a content change — and another writer filling captured_at first lands in the same place. Without a counter they fell out of the run's arithmetic entirely: success + noExif + failed no longer added up to the count the operator was shown when they started the job. The card shows the count only when it is non-zero, and states what is known — changed by something else, not updated — rather than promising a retry: for the already-dated case there is nothing to retry, and the Missing Capture Date figure above is what says whether work is left. Locale coverage: en, de, fr, sl, with the defaultValue carrying the rest. Regression test: a replacement landing mid-run leaves captured_at NULL and is not counted as updated. Verified to fail against the unfenced code on this branch.	2026-08-27 08:44:32 +02:00
Paul Nothaft	292dd4fa09	chore(stable): release 3.46.5 (#1193) v3.46.5	2026-08-26 21:44:17 +02:00
Paul Nothaft	5559cd333d	fix(images): respect EXIF orientation in thumbnails, heroes, previews and watermarks (#1185) (#1202) ... Stable twin of #1194. generateThumbnail, generateHeroImage and generatePreviewImage went straight from sharp(imagePath) to .resize(), so a photo whose Orientation tag is not 1 — routine for portrait shots on bodies that tag rather than rotate the sensor data — was resized from the raw frame and came out sideways. The same pipelines then call .withMetadata(false), stripping the tag from the output, so nothing downstream could correct it either. The download path already had this right, which is why the same photo looked correct on download and rotated in the gallery. watermarkService had it too, and it is the one a guest actually sees: gallery.js serves photos.watermark_path ahead of the original when branding watermarking is on. Two details there — metadata() is read from a separate unrotated handle, because .rotate() does not change what it reports and every use of those numbers is positioning; and the composite offsets are floored, because getPositionCoordinates returns fractional pixels, sharp rejects them, and applyWatermark catches its own error and silently returns the image unwatermarked. The rotate is unconditional in the thumbnail and hero generators — neither passes `animated: true`, so both already flatten a multi-frame source and guarding there would protect an animation that was being discarded anyway. generatePreviewImage keeps the guard, since it genuinely does preserve animation. photos.width/height were stored from sharp's metadata, which reports pixels as STORED, not displayed. For orientation 5-8 those are swapped, so a portrait photo landed in the database as landscape and masonry sized its tile with the wrong aspect ratio on top of the image being unrotated. A shared orientedDimensions() helper now does the conversion at all eight image ingest sites. The video path is deliberately untouched: its dimensions come from ffprobe, where EXIF orientation does not apply. Existing rows keep their pre-rotation dimensions until reprocessed; the backfill for those is #1199 on main and is not ported here yet. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 21:13:08 +02:00
Paul Nothaft	ac7ef266dc	fix(admin): make "Storage used" report storage used (#1164) (#1177) ... * fix(admin): make "Storage used" report storage used (#1164) Stable twin of #1170. The tile summed photos.size_bytes — the catalogued size of the ORIGINALS, which in reference mode live on external storage and have no relationship to the disk PicPeak runs on. The reporter's tile read ~80 GB against 21 GB of real usage. Worse than the label: the same number drove the soft-limit warning bar and, via /storage/info, the recommended soft limit — so a reference-mode install got a disk-capacity recommendation computed from bytes that are not on the disk. - new localStorageUsage service walks the storage root and reports the total plus a breakdown. Walking rather than summing DB columns is the point: thumbnail/preview/hero rows record a key and never a byte count, and orphans from a deleted event or an interrupted import are real bytes. - the external media root is excluded when it sits inside the storage root. Its compose default is <storage>/external-media, where the NAS is bind-mounted — a plain directory, not a symlink — so walking it would put every referenced original back into a figure whose purpose is to leave them out. Symlinks are not followed either. - .download-cache gets its own line: it lives inside the event directory, so the naive rule files a multi-GB zip as photography. - concurrent cold-cache callers share one walk; the dashboard, /storage/info and the sidebar are routinely requested together. - S3 installs keep the catalogued figure and the walk is skipped before it runs, since the objects are in the bucket and STORAGE_PATH holds only incidental local files. - an absent measurement reads as "unavailable" and a partial one is marked `+` across the dashboard, analytics, sidebar and status tab — a floor silently compared against a soft limit reads as "safely under". Verified on this branch: 11 new service tests, dashboardScope updated for the changed contract, full suite leaves the same 5 pre-existing failures as origin/stable. Frontend 20 files / 104 tests, tsc clean. * fix(admin): tell "no disk to measure" apart from "the measurement failed" (#1164) External review found both of these on this branch. Both were reported as `storage_measurement: 'catalog'`, so a failed local walk made the dashboard claim the objects live in S3. They are different things — one is a fact about the install, the other is a fault — and there is now an `unavailable` state for the second. The analytics percentage could reach the billions. `safeSoftLimit` fell back to `storageUsed || 1`, and on S3 that is null → 1, while the figure beside it came from `catalogedBytes`. An editor or viewer holds `analytics.view` but not `settings.view`, so `/storage/info` 403s for them and `storageInfo` is undefined — which is exactly when that fallback fires. It now falls back to the measured figure, and suppresses the percentage entirely when there is no real limit rather than dividing usage by itself and always reading 100%. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 09:14:48 +02:00
Paul Nothaft	9ffbe2f98f	fix(previews): preserve alpha and animation in the preview tier (#1176) ... * fix(gallery): stop the lightbox loading originals to display a photo (#1166) Stable twin of #1169. The lightbox read preview_url, which the server only emits once an admin has flipped lightbox_preview_enabled — off by default. So a stock install fell straight through to url, the untouched original: a reporter measured 16.5 MB for a photo whose preview is 345 KB. The lightbox renders its neighbours too, so opening one photo pulled three originals. slideshow_url is the same /preview/:id URL, watermark query included, and has been emitted unconditionally for images since #1015. Preferring it fixes every existing install with no migration and no admin action. Two other surfaces bypass PhotoLightbox entirely and had the same bug: - premium galleries build their own slides with `src: photo.url`. Fixing that also required carrying the photo id on the slide, because the download handler recovered the photo by matching slide.src against photo.url — a derivative src would have made Download a silent no-op. - the Story layout rendered the full original as its GRID TILE, at object-cover in a small card, and its hero rendered one as a full-bleed background when hero_url exists for exactly that. Cards now use the preview tier (not the thumbnail: thumbnail_fit is seeded to 'cover', so a thumbnail would be cropped a second time and reframe every photo) and only load once within 200px of the viewport, since every card mounts at page load. GIF, APNG and PNG keep the original: generatePreviewImage encodes JPEG, which has neither a second frame nor an alpha channel. The backend fix that removes this list is the next commit in this stack. Divergence from the main twin: no responsive `?w=` tiers. #1095 is main-only, so `lightboxImageUrl` here selects a URL and nothing more. It lives in `imageTiers.ts` under the same path main uses, so that backporting #1095 later merges into this file rather than landing beside it. Verified on this branch: 8 new tests; frontend suite 21 files / 113 tests, tsc clean. * fix(gallery): make the Story hero fix actually work on external galleries (#1166) External review. Same two fixes as the main twin. hero_url was inert for external media. ensureHeroImage only ever called resolvePhotoStorageKey, which returns null for external/reference photos by design — and that null was handed straight to withLocalCopy, which throws, so the hero route caught it and redirected to the full ORIGINAL. #1078 fixed exactly this shape for ensurePreviewImage and nobody carried it across. It stayed invisible until this PR pointed the Story hero at hero_url: on a managed gallery that is a real saving, on a reference-mode gallery it quietly changed nothing. Needed one extra piece here that main already had: generateHeroImage on this branch ignores outputBasename and always derives the key from the source basename, so two events referencing the same NAS filename would clobber each other's hero. It now honours the option, matching generateThumbnail and generatePreviewImage. The format bypass trusted mime_type, which is not trustworthy: migration 039 backfilled every pre-existing photo to image/jpeg regardless of what it was, and adminExternalMedia inserts rows with no mime_type at all — so a mislabelled PNG sailed past the guard and came back flattened. It now checks the filename extension as well. * fix(previews): preserve alpha and animation in the preview tier Stable twin of #1171. Stacked on the #1166 twin, whose format bypass this removes. generatePreviewImage encoded JPEG unconditionally. JPEG has no alpha channel and no second frame, so a transparent PNG came back flattened onto a solid background and an animated GIF came back as its first frame — for every consumer of this tier, not just the lightbox. It was only invisible by default because the lightbox served originals. Sources with alpha, or more than one page, are now encoded as WebP, which carries both and is still far smaller than the original. Ordinary photos stay JPEG. - the output extension matches what was written. A PNG source previously produced `preview_foo.png` holding JPEG bytes; harmless while the route hard-coded image/jpeg, wrong once the encoding varies. Existing keys keep working — they are still JPEG and still served as such. - the preview route derives Content-Type from the key. With nosniff set, mislabelling would show a broken image rather than being silently corrected. The watermark branch re-encodes to JPEG and now says so. The frontend guess-by-MIME goes away entirely, including the case it could never get right: a still and an animated WebP declare the same type. Divergence from the main twin: no width-tier case. The responsive `?w=` renditions (#1095) are main-only, so this branch has a single canonical preview per photo. Verified on this branch: 5 new backend tests against real Sharp output; frontend 21 files / 114 tests; full backend suite leaves the same 5 pre-existing failures as origin/stable. * fix(previews): retire the legacy preview keys, and stop mislabelling watermarked ones External review. Same two defects as the main twin. Legacy keys collide with the new naming. The old generator kept the SOURCE basename verbatim while always writing JPEG, so a `.webp` upload produced `previews/preview_shot.webp` holding a JPEG. The claim that pre-existing keys have no .webp suffix was simply wrong. The route now derives Content-Type from the key and the response carries nosniff, so every photo uploaded as WebP would have rendered as a broken image in the lightbox. Legacy `.png` keys are wrong the other way: flattened JPEGs of what may have been transparent sources, which isPreviewValid would have let stand forever. Migration 178 clears photos.preview_path outright — all of it, not just the suspicious extensions, because a `.jpg` key can equally be a flattened rendition and nothing in the key says so. Previews regenerate lazily on next view under the new encoder. The watermark branch mislabelled its output. applyWatermark PRESERVES the source format on this branch too (watermarkService.js: png stays png, webp stays webp), and its input is the preview — so the output already matches the key the header was derived from. Forcing image/jpeg mislabelled every watermarked WebP preview, and nosniff means the browser would not correct it. Numbered 178, not 176: this stack does not carry the external-media migrations, but that stack takes 176 and 177 on this same branch, and two files sharing a numeric prefix would be confusing even though both would run. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 09:14:42 +02:00
Paul Nothaft	75facb4d67	fix(gallery): stop the lightbox loading originals to display a photo (#1166) (#1175) ... * fix(gallery): stop the lightbox loading originals to display a photo (#1166) Stable twin of #1169. The lightbox read preview_url, which the server only emits once an admin has flipped lightbox_preview_enabled — off by default. So a stock install fell straight through to url, the untouched original: a reporter measured 16.5 MB for a photo whose preview is 345 KB. The lightbox renders its neighbours too, so opening one photo pulled three originals. slideshow_url is the same /preview/:id URL, watermark query included, and has been emitted unconditionally for images since #1015. Preferring it fixes every existing install with no migration and no admin action. Two other surfaces bypass PhotoLightbox entirely and had the same bug: - premium galleries build their own slides with `src: photo.url`. Fixing that also required carrying the photo id on the slide, because the download handler recovered the photo by matching slide.src against photo.url — a derivative src would have made Download a silent no-op. - the Story layout rendered the full original as its GRID TILE, at object-cover in a small card, and its hero rendered one as a full-bleed background when hero_url exists for exactly that. Cards now use the preview tier (not the thumbnail: thumbnail_fit is seeded to 'cover', so a thumbnail would be cropped a second time and reframe every photo) and only load once within 200px of the viewport, since every card mounts at page load. GIF, APNG and PNG keep the original: generatePreviewImage encodes JPEG, which has neither a second frame nor an alpha channel. The backend fix that removes this list is the next commit in this stack. Divergence from the main twin: no responsive `?w=` tiers. #1095 is main-only, so `lightboxImageUrl` here selects a URL and nothing more. It lives in `imageTiers.ts` under the same path main uses, so that backporting #1095 later merges into this file rather than landing beside it. Verified on this branch: 8 new tests; frontend suite 21 files / 113 tests, tsc clean. * fix(gallery): make the Story hero fix actually work on external galleries (#1166) External review. Same two fixes as the main twin. hero_url was inert for external media. ensureHeroImage only ever called resolvePhotoStorageKey, which returns null for external/reference photos by design — and that null was handed straight to withLocalCopy, which throws, so the hero route caught it and redirected to the full ORIGINAL. #1078 fixed exactly this shape for ensurePreviewImage and nobody carried it across. It stayed invisible until this PR pointed the Story hero at hero_url: on a managed gallery that is a real saving, on a reference-mode gallery it quietly changed nothing. Needed one extra piece here that main already had: generateHeroImage on this branch ignores outputBasename and always derives the key from the source basename, so two events referencing the same NAS filename would clobber each other's hero. It now honours the option, matching generateThumbnail and generatePreviewImage. The format bypass trusted mime_type, which is not trustworthy: migration 039 backfilled every pre-existing photo to image/jpeg regardless of what it was, and adminExternalMedia inserts rows with no mime_type at all — so a mislabelled PNG sailed past the guard and came back flattened. It now checks the filename extension as well. * test(gallery): the hero fixture follows the root-relative relpath contract (#1166) Same fix as the main twin: external_relpath has been resolved from EXTERNAL_MEDIA_ROOT rather than from event.external_path since #1163 landed, and this fixture still carried the base-relative form, so the two tests stopped resolving the moment that stack merged. Production was never affected. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 09:08:50 +02:00
Paul Nothaft	58ccecc304	fix(admin): move the maintenance sweeps' run state into the database (#1181) (#1188) ... Stable twin of #1184. Both photo sweeps tracked whether they were running in a module-level variable, which is invisible to every other replica: a status poll routed to an idle replica reports isRunning false while another is mid-run, and the next POST starts a second pass over the whole library. Migration 179 adds one row per job, claimed with a conditional UPDATE whose affected-row count is the answer. The lease is fenced on a per-claim token so a runner superseded by a stale takeover cannot renew a claim it has lost or release one it no longer owns; renewal runs on a timer spanning the claim through release, since one hung NAS read can outlast the stale window inside a single iteration. maintenance_jobs is excluded from .picpeak archives. Gated on settings.edit / settings.view rather than main's system.manage, which does not exist on this branch — they are what settings.edit was later split into, so both branches let the same people through. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 09:08:14 +02:00
Paul Nothaft	7f0ed23ea4	fix(external-media): record captured_at on import and add a backfill (stable) (#1183) ... * fix(external-media): record captured_at on import and add a backfill (#1172) Stable twin of #1179. External media never went through photoProcessor, so captured_at stayed NULL for every externally imported photo. The gallery's "Date Taken" sort then degraded into import order through its own COALESCE fallback — a library imported in two batches showed the first days of a trip after the last ones. Ported whole: - adminExternalMedia.js reads the capture date at import, off the file it has already opened for the dimensions. Best-effort, like the dimensions. - A backfill endpoint for photos imported before this, so existing installs can fix historical rows rather than only new imports. Managed originals go through resolvePhotoStorageKey + withLocalCopy so S3 installs work; archived events are excluded because archiving deletes their originals; the run flag is claimed before the candidate query so two POSTs cannot both start. - gallery.js carries photos.id as a tiebreaker on all three sorts. A bulk import writes hundreds of rows inside the same second, so uploaded_at ties are the normal case and the grid reshuffled between page loads. One deliberate difference from main: the backfill is gated on settings.edit / settings.view rather than system.manage / system.view, which do not exist on this branch. They are what settings.edit was later split into, and main's migration 175 projects every settings.edit holder forward onto system.manage, so both branches let exactly the same people through. * fix(external-media): gate the status card on the permission the button needs (#1172) The built-in admin role holds settings.view but not settings.edit (056_add_role_permissions_table.js:63), and StatusTab renders its card and enabled button purely on a successful status payload. Gating the status endpoint on settings.view therefore showed every admin a Backfill button whose every click 403s with no error surfaced. * fix(gallery): make the Date Taken sort correct on SQLite (#1172) Same defect as the main twin: photos.captured_at holds three storage classes on SQLite — an epoch-millisecond integer from managed uploads (photoProcessor.js:441 hands knex a Date), ISO text from external imports and the backfill, and null falling through to uploaded_at's 'YYYY-MM-DD HH:MM:SS' text. SQLite sorts INTEGER before TEXT unconditionally, so a 2027 capture came back before a 2020 one, and within the text values the 'T' separator outranked the space. Normalised in the ORDER BY; Postgres keeps the plain COALESCE, its column being a real timestamp. Regression tests drive the real gallery route on real SQLite. * fix(gallery): normalise epoch-integer uploaded_at too, and stop polling a 403 (#1172) Both follow-ups from the main twin's review, ported. uploaded_at is not always text on SQLite: a .picpeak restore can carry epoch milliseconds in from an install that stored them that way, and the fallback branch read it with substr(), comparing '1830297600000' against '2020-01-01 00:00:00' as text. Both columns now get the integer/real branch. The status card also polled every ten seconds regardless of permission. On this branch that hits every built-in admin — they hold settings.view but not settings.edit — so each would have had a 403 and a logged denial every ten seconds for a panel they were never shown. * style: quote convention in the capture-sort test (#1172) * fix(capture-dates): skip watcher-imported videos, and make the status counts consistent (#1172) All three follow-ups from the main twin, ported: the three-marker video filter (fileWatcher sets type/mime but not media_type, so those rows sat in the backlog forever), the single-aggregate status counts (two queries could report a negative backlog mid-import), and the card's render gated on settings.edit as well as the cached payload. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 09:04:18 +02:00
Paul Nothaft	2b1c3588ae	fix(external-media): store external paths from the media root (#1163) (#1174) ... * fix(external-media): store external paths from the media root (#1163) Stable twin of #1168. Stacked on the #1162 twin, which supplies deleteDuplicatePhotos. Importing a second folder into an event silently invalidated every photo already in it. external_relpath was stored relative to events.external_path, and every import overwrites that column, so the older rows were rebased onto the new folder. Nothing errored and the grid still rendered — thumbnails are written to local storage during the import while the base path is still correct — so only the things that need the original broke. The reporter had 7547 of 8004 rows pointing into the void. - external_relpath is now relative to EXTERNAL_MEDIA_ROOT, so a row is self-describing and nothing an admin does to the event can move it. - migration 177 folds each event's base into its rows. Where the current resolution is missing it walks up for an ancestor holding a file of the same name AND the size the import recorded — existence alone would let a deleted file adopt an unrelated namesake and serve the wrong original. Rows it cannot place keep resolving where they resolve today, and the probe is skipped entirely when the mount is unreachable. - probing is read-only and runs first; the rewrites and the marker commit together, so an interrupted fold cannot be folded twice. - rewrites are staged through a per-row parking value, because a final path can equal another row's current one; and migration 177 re-throws without the driver's error code, which run-migrations-safe would otherwise read as "schema already exists". - the fold also runs after a .picpeak restore, since knex_migrations is excluded from the archive, and a failure there is reported rather than presented as a clean restore. - drops the duplicate-leaf-segment guess in photoResolver, which papered over this same double-prefixing. Divergence from the main twin: no face-scan requeue reordering. Face recognition is main-only, so the hazard of queueing rows against unconverted paths does not exist on this branch — in picpeakImportService or in restoreService. Verified on this branch: 23 new tests pass, and the four suites carrying base-relative fixtures were updated. Full suite leaves the same 5 pre-existing failures as origin/stable, unchanged. * fix(external-media): the fold's staging value must be storable on Postgres (#1163) External review found this on this branch first; it was on both. The two-pass rewrite parks each row on a temporary value, and that value was written with a leading NUL. SQLite stores NUL in TEXT without complaint; Postgres rejects it outright — "invalid byte sequence for encoding UTF8: 0x00" — so migration 177 rolled back on exactly the installs that need the two-pass repair, and only on the engine most of them run. Restores hit the same wall and reported the conversion as failed. The prefix is ordinary text now. Adds a gated Postgres test alongside the existing picpeakRestorePg one, because a SQLite-only suite structurally cannot catch this class: restoring the NUL makes exactly the two-pass repair case fail with that error, and nothing else. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 09:00:05 +02:00
Paul Nothaft	e9fcf4960e	fix(external-media): one row per external file per event (#1162) (#1173) ... * fix(external-media): one row per external file per event (#1162) Stable twin of #1167. Two overlapping import-external runs against the same event inserted every file twice. The route checked for an existing external_relpath and then inserted, with an fs.stat and a sharp().metadata() read sitting in between — a window wide enough for both runs to see "not there". A reporter's event held 8004 rows for 6012 distinct paths. Nothing at the storage layer stopped it: migration 041 created only a NON-unique (event_id, source_origin) index. - migration 176 removes the existing duplicates and adds a partial unique index on (event_id, external_relpath), verified against the catalog afterwards — a failed CREATE INDEX raises 23505 on Postgres, which run-migrations-safe treats as "schema already exists" and would record as applied on an install that never got the index. - dependent rows are removed explicitly rather than by cascade: PicPeak never sets `PRAGMA foreign_keys = ON`, so on SQLite the declared CASCADE is inert and a bare delete strands feedback and access-log rows. Guest feedback moves to the survivor instead of being discarded, keyed on guest identity the way feedbackService defines it, and the survivor's denormalized counters are recomputed. - the route treats a unique violation as a skip, so a writer this process cannot see converges instead of duplicating, and a second import while one is running gets a 409. - a .picpeak taken before migration 176 carries exactly these duplicates, and suspending FK enforcement does not suspend a unique index — so the restore drops the index for the load and rebuilds it after running the same dedupe. Divergences from the main twin, both because the feature is absent here: faces (no faceProcessor, so no purgePhotoFaces reconciliation — the rows are still deleted so nothing dangles), admin marks, transfer membership, and photos.view_count/download_count. The service guards each on hasTable / hasColumn, so those branches simply do not fire. Verified on this branch: 36 new tests pass; full suite leaves the same 5 pre-existing failures as origin/stable, unchanged. * fix(external-media): invalidate the download zip when duplicates are removed (#1162) External review. Same fix as the main twin. The pre-built "download everything" archive still contained the duplicate rows the dedupe had just deleted, so guests kept receiving them. Every ordinary photo-deletion path calls downloadZipService.invalidate for exactly this reason. The columns are cleared rather than the service being called: that service carries debounce timers and a regeneration queue, which a migration should not start. getZipInfo already treats a cleared record as a cache miss and rebuilds on the next request. The stale object is left in storage, as elsewhere. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:55:17 +02:00
Paul Nothaft	f83d144f28	chore(stable): release 3.46.4 (#1159) v3.46.4	2026-08-24 20:34:25 +02:00
Paul Nothaft	b62cd2c290	fix(gallery): a guest's own hidden feedback is hidden from them too (#1150) (#1157) ... Stable twin of #1153. Everything in the system treats a hidden row as absent, but the per-viewer is_liked heart read the row without looking at is_hidden — so a like the photographer had hidden still showed as liked on a photo whose like_count was zero. Making that agree exposes the second half: the duplicate check behind like/favorite toggling did not skip hidden rows either, so the now-empty heart, when clicked, found the hidden row and toggled it OFF. Skipping hidden rows there makes the click create a fresh, visible row. Also carried from review: the per-guest caps, /my-feedback and getEventFeedbackSummary no longer count hidden rows, and unhiding collapses the guest's replacement — skipped when there is no stable identity, since that fallback was 'guest_identifier IS NULL', i.e. other visitors' rows. Not carried: the my_color_label badge (colour labels are #1044) and the clearScope / singleValueScope visibility fix, neither of which exists on this branch. Merged with admin privileges: the author cannot self-approve.	2026-08-23 22:09:41 +02:00
Paul Nothaft	eaa8b41ba3	fix(gallery): guest filters respect show_feedback_to_guests (#1044) (#1156) ... Stable twin of #1147, filter half only. Every filter token on /photos is an OR of two halves: what THIS viewer marked, and what ANYONE marked. The fields built from the second half are gated on show_feedback_to_guests; the filter was not, so with the setting off ?filter=liked still returned exactly the photos other people liked — the membership instead of the count, one token at a time. The half it left standing was also the wrong half: it read guest_identifier from the guest_id query parameter, which never matched anything, and accepting a caller-supplied identifier was a way back through the gate. Resolved from the request now, hidden rows excluded to match what the viewer can see. Not carried: the color: token and the photo_admin_marks concurrent-write fix — colour labels and admin marks are not on this branch. Merged with admin privileges: the author cannot self-approve.	2026-08-23 22:08:23 +02:00