Auth/access-control audit fixes (all pre-existing on main; none are
regressions). Verified end-to-end where noted.
HIGH
- Thumbnail enumeration: photoAuth granted any gallery token access to any
flat /thumbnails/thumb_* file, so a visitor to one gallery could
enumerate another (password-protected) gallery's entire thumbnail set.
Scope thumbnail access to the token's event via photos.thumbnail_path.
Live-verified: cross-event fetch now 404s, own-event still 200s.
- Bulk ownership bypass: bulk-archive/bulk-delete acted on body-supplied
event ids with no owner filter (single-event routes enforce
requireEventOwnership), letting admin/editor archive or cascade-delete
any event. Add filterOwnedEventIds; also guard rename + import-external;
tighten photo-retry to scope admin (not just editor). Fix misleading
bulk-delete comment.
MED
- verifyGalleryAccess never checked decoded.type — assert 'gallery'
instead of relying on other token types incidentally lacking eventId.
- secure-images generate-token/secure-download missing denySlideshowToken
(#646 bypass): a leaked slideshow token could download originals.
- Frontend: AuthenticatedImage + api.ts attached the gallery bearer token
to absolute/external URLs — only attach to relative same-app paths.
LOW hardening
- Pin algorithms:['HS256'] on all auth-boundary jwt.verify calls.
- crypto.timingSafeEqual for share-token + HMAC compares (utils/timingSafe).
- Remove dead photoAuth import in galleryFeedback.
Tests: new regression suites for thumbnail scoping + filterOwnedEventIds;
fixed verifyGalleryAccess.customerRevoke fixture (real customer tokens
carry type:'gallery'). Full backend suite at the pre-existing baseline
(5 suites/27 tests fail on main too), zero new failures.
Same two pre-existing-on-main bugs, at their post-decomposition
locations: clampIntOrUndefined in adminEvents/crud.js slideshow seed;
!! coercion in EventDetailsHeader, EventInformationCard,
ClientAccessCard. Keeps this branch correct in either merge order with
#734 — when merging main afterwards, resolve the adminEvents.js
modify/delete conflict by keeping the deletion.
The create route seeds show_interval_ms/show_transition_ms from
app_settings through an inline guard that pre-checked Number.isFinite(+v)
but then used parseInt(v). The two disagree for null/''/true — +null is 0
(finite) while parseInt(null) is NaN — so when the slideshow settings rows
are absent (getAppSetting returns its null default), NaN flowed through
Math.min/Math.max into the INSERT. PostgreSQL rejects NaN for integer
columns; SQLite silently stores NULL, which is why every SQLite-based
test passed while POST /api/admin/events 500'd on the PG dev stack and
broke the e2e smoke suite.
Fix: parse first, then check — clampIntOrUndefined in utils/numericHelpers
(unit-tested against every failure-mode input). Verified end-to-end: the
previously-failing minimal create now succeeds against the PG dev stack.
The Features-fallback showed raw changelog text, so a commit subject like
'branded URL shortener — /s/<slug> with OG injection' surfaced two problems
in the admin banner:
- release-please escapes <slug> to <slug>; React renders the literal
entity, so the banner read '/s/<slug>'. Decode the entities
(< > & " '), & last to avoid double-decoding.
- the technical tail leaked into a user-facing highlight. Drop a trailing
'— detail' clause (em dash only, so 'mark-paid' is untouched) so the bullet
reads as the headline 'branded URL shortener'.
Only affects the deterministic fallback; curated <!-- whatsnew --> blocks are
unchanged.
Surfaces release highlights to admins, sourced from the GitHub release notes
(no AI at runtime). Bullets are written once per release in CI via GitHub Models
(see docs/ci/whatsnew-highlights.yml) into a <!-- whatsnew --> block; the app
reads that block and falls back to the changelog's "### Features" for releases
without it — so it works against today's releases immediately.
- backend utils/whatsNew.parseWhatsNew(body): curated block else Features
section, strips scope/PR-links, de-dups, caps at 8 (tested).
- GET /admin/system/updates/whatsnew: highlights for every version moved
through since the per-instance marker (whatsnew_last_seen_version); fresh
installs self-anchor silently. Best-effort, never errors.
- POST /admin/system/updates/whatsnew/seen: advance the marker (per-instance).
- /admin/system/updates also returns latestHighlights for the teaser.
- Frontend: WhatsNewBanner (green bar -> modal with "Full changelog" link) on
the dashboard via adminService; UpdateNotification shows a "New features
include:" teaser. i18n de/en. No migration (uses app_settings).
Per-line totals are each rounded to the cent before the net is summed, so
a long time-based invoice can drift a few Rappen from qty × rate — e.g.
68 h × 32.25 = 2193.00, but the 21 rounded line totals sum to 2193.02. This
is the standard "sum of rounded lines" convention (Stripe/QuickBooks/Xero
do the same) and it foots, but some issuers want the total to match the
customer's arithmetic.
New per-issuer setting `crm_invoice_round_total` (default OFF, no migration —
read via getAppSetting with a false default). When on, the create paths store
the full-precision net rounded ONCE (cleanNetMinor), and the drift is shown to
the reader as an explicit "Rundung" row:
Betrag Netto 2'193.02 (= Σ visible line totals, still foots)
Rundung -0.02
Gesamtbetrag 2'193.00
- New util src/utils/invoiceRounding.js (cleanNetMinor) mirrors the
migration-119 hierarchy (priced sub-items override their parent) but sums
at full precision; rate-agnostic, so mixed hourly rates reconcile to one
clean net. Single document-level VAT rate ⇒ one Rundung row.
- computeTotals (quotes) + createInvoice + payload-preview gain the toggle.
- Render contexts derive the row as storedNet − Σ(line totals); legacy/off
documents have equal values ⇒ adjustment 0 ⇒ byte-identical output.
Suppressed on Storno/Mahnung (negated net + sign-flipped lines).
- Storno/tax-report stay correct: both use the stored net scalar, which is
the clean value (createStorno negates net_amount_minor; it never re-sums).
- pdf-i18n: totals_rounding in all 6 locales (de/en/fr confident; nl/pt/ru
machine-translated — flag for native review).
- Frontend: toggle on Settings → CRM (Invoices), default off.
Tests: backend/__tests__/utils/invoiceRounding.test.js (real 68h invoice,
mixed rates, discounts, sub-item hierarchy, no-op case).
Implements the draft-seam booking cutover so the booking_invoice_only flow
becomes enableable. The booking flows trigger on quote.accepted, so the run
entity is the quote:
- prepare_invoice: convertToInvoiceOnly({draft:true}) creates the invoice(s)
on HOLD (scheduled_send_at NULL, status stays 'scheduled') so the scheduler
never auto-sends before the review gate; crash-recovery recovers drafts by
the quote's deal_uuid. Stores ids in ctx.vars.preparedInvoiceIds.
- prepare_contract: createFromQuote (idempotent via converted_contract_id).
- send_document: dispatches the prepared draft (invoice -> sendInvoice each id,
contract -> sendContract).
- resolveActor: quote creator -> workflow creator -> first admin.
- prepare_contract/prepare_invoice/send_document removed from the enable-guard
list; prepare_event/prepare_quote/prepare_gallery/reserve_date still guarded,
so booking_full/booking_simple stay blocked until the event-path increment.
Fixes a latent single-connection SQLite deadlock these unattended paths would
hit: getAppSetting/logActivity/adminActor read or write the global db, which
deadlocks when issued inside an open knex transaction. Thread the active trx
through getAppSetting, logActivity, nextInvoiceNumber, nextContractNumber, the
spawnInstallmentInvoices audit log, and hoist adminActor before createFromQuote's
transaction. convertToInvoiceOnly now logs after commit and returns invoiceIds.
Adds bookingCutover integration test (hold-mode null send-at, normal scheduled
contrast, contract path no-deadlock) and a route test that the now-implemented
booking invoice actions can be enabled.
Reporter @Duecki1 wants to stop telling guests "pick only 5 photos" by
hand. Per-event cap, enforced server-side, with a clear popup when the
11th click would exceed the limit. Per-guest scope matches the "every
couple picks their top 10" mental model; per-gallery aggregate is
explicitly NOT in scope (creates weird "first 10 visitors use up all
slots" race conditions).
## Schema (migration 141)
Two nullable columns on `event_feedback_settings`:
- `max_favorites_per_guest`
- `max_likes_per_guest`
null / 0 = unlimited (preserves current behaviour for every existing
install — operator must opt in). Both shipped together because the
code path is identical; photographers can cap either, both, or neither.
## Backend
- `feedbackService.submitFeedback` cap check on the INSERT branch only.
Toggle-off (un-favoriting) is always allowed, so a guest at 10/10
can free a slot by un-clicking an existing favorite.
- New `countGuestFeedback(eventId, type, guestId, guestIdentifier)` —
matches the exact same guest-key shape the existing duplicate-check
uses (guest_id when present, fallback to guest_identifier in simple
identity mode).
- Limit reduction grandfathers: admin lowering 20→10 keeps existing
rows in place; new adds blocked until the guest removes some.
- Route layer (`galleryFeedback.js` POST) translates a `limit_reached`
service-return into a structured 403 with `code:
'FAVORITE_LIMIT_REACHED'` / `'LIKE_LIMIT_REACHED'`, `limit`, and
`current_count`. Stable UI contract.
- `feedback-settings` GET exposes the caps so the gallery UI can
optionally render a counter near the heart icon (UI extension TBD;
the modal alone is the contract this PR commits to).
- `feedbackValidation`: range guard `0..10000`, null allowed,
per-field error messages.
## Frontend — the popup
New `FeedbackLimitReachedModal` component renders via a `createPortal`
to `document.body` so it escapes any lightbox / sticky parent stacking
context and reliably sits above everything else.
Mobile-first responsive:
- `items-end sm:items-center` — slides up from the bottom on phones
(native action-sheet feel), centers on desktop (familiar modal).
- `w-full sm:max-w-md` — full-width on phones, clamps to 420px on
desktop.
- `rounded-2xl sm:rounded-xl` — more rounded on phones for the
sheet feel.
- `pb-[env(safe-area-inset-bottom)]` — respects the iOS home indicator
and Android gesture bar.
- `z-[60]` — above the lightbox's z-50.
Title + body + "8 of 10 used" pill + "Got it" button. Backdrop click +
Escape both dismiss. Focus management lands on the OK button so
keyboard / screen-reader users can dismiss immediately.
New `useFeedbackLimitModal()` hook is the shared API: components
on every submit-feedback site wire `onError: (err) => handleError(err)`
and render `{limitModal}` in their JSX. Returns `true` from
`handleError` when the error is a structured cap-reached 403 (so the
caller can skip its generic error toast). PhotoFavorites + PhotoLikes
+ PhotoLightbox all wire through the hook — every favorite/like submit
path is covered, including the lightbox's three different submit
sites (guest mode, simple mode, post-identity-modal-confirm).
## Admin UI
`FeedbackSettings` card gets a new "Per-guest limits" section that
only renders when at least one of `allow_favorites` / `allow_likes` is
on. Two numeric inputs (0 / empty = unlimited) side-by-side on
desktop, stacked on mobile. Hint text covers the limit-reduction
grandfathering semantics so admins aren't surprised.
## i18n
EN + DE for:
- Modal title + body (parameterized with `{{limit}}`)
- Counter pill (parameterized with `{{current}}` / `{{limit}}`)
- OK button label
- Admin field labels + hints + section header + grandfathering note
## Tests
**Backend** (`__tests__/utils/feedbackPerGuestLimit.test.js`, 8 cases):
- null cap → unlimited (back-compat)
- 0 cap → unlimited (UI convenience)
- cap=10: rows 1-10 succeed, 11 returns limit_reached
- toggle-off frees a slot at the cap
- limit reduction grandfathers existing rows
- per-guest scope: guest A's cap doesn't affect guest B
- favorite cap doesn't block likes (per-type)
- like cap returns LIKE_LIMIT_REACHED-shaped payload
**Frontend** (`__tests__/useFeedbackLimitModal.test.ts`, 7 cases):
- Non-axios errors → null
- Non-403 axios errors → null
- 403 with wrong code → null
- FAVORITE_LIMIT_REACHED parsed
- LIKE_LIMIT_REACHED parsed
- Falls back to code-implied type when feedback_type missing
- Missing numeric fields → 0 (not NaN)
All 15 pass. tsc --noEmit clean. eslint clean on changed files.
Closes#655.
Each /state poll fired ~10 getAppSetting reads to resolve the watermark/fit;
a leaked link x N tabs amplified that linearly (review concern 2). Add a
5s-TTL cached bundle (utils/slideshowGlobals) for the global slideshow_* +
branding-logo settings, invalidated on PUT /admin/settings/slideshow so admin
live-edit stays instant. slideshowSettings now does ~2 reads per poll (event
row + photo count) on a cache hit. Also documents the frontend
optimistic-default nit.
Two security advisories landed against the open #641 branch — bundling
both because they touch independent surfaces and PR #641 is the next
beta ship vehicle.
**GHSA-9v4w-jrhx-g5wr (BOLA on /admin/photo-export/:eventId/*)** —
the three /:eventId-scoped routes in `adminPhotoExport.js` (filtered,
filter-summary, export) ran `adminAuth + requirePermission(...)` but
not `requireEventOwnership`, so any non-super-admin admin/editor with
photos.view (or photos.download) could enumerate + export the photos
of events created by other admins — leaking `original_filename`,
which routinely encodes client identity. Sibling `adminPhotos.js`
applies the middleware on every :eventId route; this file was the
single drift. Reporter: Wernerina.
**GHSA-wmjx-pc37-272r (NAT64 SSRF in `isPrivateIPv6`)** — the old
implementation did naive string-prefix checks (`startsWith('fc')`,
`startsWith('fe80')`) and had zero coverage for NAT64
(`64:ff9b::/96` per RFC 6052, `64:ff9b:1::/48` per RFC 8215). On
instances with NAT64/DNS64 egress, a webhook URL like
`http://[64:ff9b:1::a9fe:a9fe]/` translated through the gateway and
reached 169.254.169.254 — exfiltrating cloud metadata (IAM creds)
into `webhook_deliveries.response_body`. Rewrote `isPrivateIPv6` to
expand the address to its canonical 8-group form, block both NAT64
prefixes, decode embedded IPv4 from IPv4-mapped (`::ffff:0:0/96`) and
deprecated IPv4-compatible (`::/96`) forms and re-check via
`isPrivateIPv4`, and fail closed on any parse failure. Reporter:
tonghuaroot.
Added 34 unit tests covering: both NAT64 prefixes in hex + mixed
dotted-quad notation, IPv4-mapped IPv6 hex + mixed, deprecated
::IPv4 form, legacy fc00::/fd00::/fe80::/::1/:: cases stay blocked,
and public IPv6 (Google/Cloudflare/Google IPv6) negative controls
stay allowed.
Refs: GHSA-9v4w-jrhx-g5wr, GHSA-wmjx-pc37-272r
1. requireFeatureFlag now caches each flag for 10s (the accounting area is 10+
gated endpoints); PUT /admin/feature-flags invalidates the cache so toggles
still take effect immediately.
2. Customer routes (/quotes, /invoices, /contracts + their PDFs) now gate via
getEffectiveFeaturesForCustomer — the global MASTER flag AND the per-customer
override — instead of the per-customer column alone, via a shared
customerFeatureAllowed() helper. Admin disabling a feature globally is now
honoured for customers too.
4. Tax-report VAT-payable: when accounting_vat_registered is UNSET, stop guessing
from grandTotalVat>0 (a zero-output-VAT quarter silently flipped to "not
registered" and hid the reclaim). Treat null as "not configured":
vatPayableMinor=null + vatRegistrationConfigured=false; the UI renders "—" and
a "configure VAT registration" warning. Tests updated.
5. Shared upsertAppSetting() in utils/appSettings — the two adminSettings upsert
loops use it, so the app_settings created_at class can't be re-introduced.
6. PDF rasterise per-file bound: getRenderedPagePath refuses pages beyond
MAX_RENDERABLE_PAGES (200); page_count is capped to match at ingest, so a
hostile high-page PDF can't drive an unbounded pager.
7. (no code) original_filename is only rendered via auto-escaped JSX; the two
dangerouslySetInnerHTML sites are admin-authored content — paranoia pass clean.
Concerns 3 (foreign-VAT reclaim-country) and 8 (imap_pass plaintext) are PR-reply
/ doc items, addressed in the PR response, not code.
Blocker 1 — CSV/Banana formula injection. Neither csvEscape (ledgerService) nor
the tax-report CSV escape nor the unquoted tab-separated Banana cell formatter
prefixed risky leading chars, so an admin-/sender-controlled cell beginning with
= + - @ TAB CR executes as a formula when the Treuhänder opens the export. New
shared util neutralizeSpreadsheetFormula() prepends a single quote; wired into
all three sinks (quoted CSV + unquoted Banana). Unit test pins one of each char.
Blocker 2 — IMAP intake double-ingest race. received_emails.message_id was
INDEX, not UNIQUE, and the poller ingested attachments BEFORE writing the audit
row, so a second replica / rolling-deploy overlap double-ingested the same mail.
Migration 128 makes message_id UNIQUE (nulls stay distinct); the intake now
CLAIMS the message row (status='processing') BEFORE ingesting — a concurrent
claim hits the unique constraint and skips cleanly (shared isUniqueViolation
helper). Stale 'processing' rows (worker crashed mid-ingest) are reclaimed after
10 min so no attachment is orphaned.
NOT done (deliberate): the suggested UNIQUE on inbound_documents.file_sha256 —
that column is a SOFT dedup key by design (manual re-uploads are kept as flagged
'duplicate' rows + duplicate_of_id for the Duplikat disposition); a unique index
would break that feature. The file race only yields an extra 'unsorted' row (a
data-quality nit, caught by the existing manual Duplikat backstop), not a
double-count. Rationale to be added to the PR reply.
patchingfailed reported on v3.44 stable: a gallery named `Ägypten` with
photo `Ägypten_individual_0050.jpg` downloads as `gypten_...` —
the leading umlaut is dropped entirely. Their hypothesis was a
Content-Disposition encoding issue, but the actual root cause sits
one layer earlier: at UPLOAD time when `generatePhotoFilename` calls
`sanitizeFilename`.
`sanitizeFilename` did:
String(str).trim()
.replace(/\s+/g, '_')
.replace(/[^a-zA-Z0-9_\-\.]/g, '') // ← drops `Ä` outright
.replace(/[_\-]{2,}/g, '_')
.replace(/^[_\-]+|[_\-]+$/g, ''); // ← would strip a leading _ too
For `Ägypten`: alphanumeric-strip → `gypten` (Ä gone, no underscore
left behind because the regex used '' as the replacement, not '_'). The
result is stored in `photos.filename` and that's what downloads serve.
By that point `buildContentDisposition` is doing the right thing
(emits both `filename="..."` ASCII fallback AND RFC 5987
`filename*=UTF-8''…` — Chrome correctly picks the UTF-8 form), but the
string it's encoding has already lost the umlaut at the DB layer.
Cure: NFD-normalize + strip combining marks BEFORE the alphanumeric
strip. Same pipeline `utils/slug.js` (#525) already uses for URL
slugs:
sanitized = sanitized
.normalize('NFD')
.replace(/[̀-ͯ]/g, '');
Now `Ägypten` → NFD-decomposed `A` + combining diaeresis → strip
combining mark → `Agypten` survives the alphanumeric pass. Filename
and URL slug stay in sync (the URL was already `Agypten`, per
patchingfailed's report — the filename now matches).
Test surface: new `filenameSanitizer.test.js` pins:
- the headline #607 contract for German / Portuguese / French / Spanish
accented inputs (with a counter-example using the pre-fix pipeline so
a future edit can't quietly regress it)
- ASCII-input parity — pre-#607 byte-identical output for every
pre-existing ASCII case
- `generatePhotoFilename` composed round-trip
- `sanitizeForContentDisposition` + `buildContentDisposition` RFC 6266
dual-form output (since the helper sits next to this function and is
the next thing to break if a refactor goes sideways)
- `sanitizeForZipEntry` path-traversal blocking
31 cases total, all pass.
Bundled into PR #609 since it's a small targeted fix and that PR is
already an admin-UI polish branch with low review weight.
The SVG->PNG cache was keyed only by source path + mtime + size, so an
override logo rasterised once WITHOUT fonts (text -> tofu) stayed cached
after the font fix - the source SVG was unchanged, so the stale tofu PNG
kept being served. Add a RASTER_VERSION component to the cache key; bumping
it (v2-fonts) invalidates every prior rasterisation without clearing the
cache dir by hand.
Move the scheduled-email business-hours floor onto the business profile
as Google-style per-weekday opening blocks (multiple blocks/day for lunch
breaks). Migration 114 adds business_profile.business_hours (JSON) +
scheduled_email_floor_enabled; emailProcessor snaps a queued email to the
next open block, read in the profile timezone. Editor lives under
Settings → Business profile.
Add an admin "Send queued emails now" flush (POST /admin/email/flush-queue)
that drains the queue immediately, ignoring the business-hours floor — the
escape hatch before maintenance/updates. processEmailQueue now takes
{ignoreSchedule, limit} and returns send counts; the scheduled interval
run is unchanged.
spawnToFile and spawnFromFile passed an unopened WriteStream/ReadStream
directly as a stdio entry to child_process.spawn. Older Node versions
auto-extracted .fd; Node 22 throws synchronously:
The argument 'stdio' is invalid.
Received WriteStream { fd: null, path: '/backup/database/...sql', ... }
Bug bit Ralf's install once today's `bugfix/crm-backup` image landed —
Node 22 came with that image, and Stage A's inline-dump path is the
first caller of spawnToFile on this install. Latent on the previous
image (Node 20); fatal on this one. restoreService's pre-restore
safety snapshot uses the same helper and would have hit it next time
a restore ran.
Cure: stdio: ['ignore', 'pipe', 'pipe'] (and ['pipe', 'pipe', 'pipe']
for spawnFromFile) + manual pipe of child.stdout/stdin through the
file stream. Works on every Node version. Also wires the WriteStream's
'error' event to the promise via settleReject so a future EACCES /
ENOSPC reaches the caller's try/catch instead of becoming a process-
fatal unhandled error event — closing the same "Stage A guard
bypassed" hole noted in the spawned follow-up task.
Side benefit: outStream.end() now awaits flush before resolving, so
fast pg_dump runs can no longer produce a truncated dump.
Resolves a conflict with the CRM merge (#555) that landed on beta
between when this branch was cut and now.
Two conflict regions in backend/src/routes/adminCustomers.js:
1. **Require block** — both branches added new requires after
customerAccountsService. Kept both: this branch's
emailNormalization import AND beta's customerHoursService +
invoiceService imports (the CRM merge added the hours-billing +
invoice-creation paths to this router).
2. **Edit-customer validators** — both branches changed the same set
of body() validators in the PUT /:id handler. This branch added
the IDENTITY_PRESERVING_NORMALIZE_EMAIL options arg to
normalizeEmail; beta changed every body() to optional({ nullable:
true }) so passive-customer records that store nulls for missing
profile fields don't reject on save. Kept both: the nullable
pattern from beta + the email-normalization options from this
branch. Preserved beta's explanatory comment about the nullable
choice.
Also patched one NEW normalizeEmail site the CRM merge introduced:
- backend/src/routes/adminCustomers.js:231 — POST /admin/customers
now exists (CRM-era customer-create endpoint). Same options arg
applied.
backend/src/routes/adminBusinessProfile.js has an isEmail() WITHOUT
normalizeEmail() on the issuer email — intentional (no normalization
means no risk of the Gmail dot-strip bug for that field), no change
needed.
All 18 normalizeEmail sites now pass IDENTITY_PRESERVING_NORMALIZE_EMAIL.
7/7 regression tests still pass. Lint clean on the merged file.
Closes#574.
Reporter (@blazmaric) identified the root cause cleanly:
express-validator's `.normalizeEmail()` applies provider-specific
canonicalization by default — Gmail dot-stripping, +tag stripping,
googlemail → gmail folding, etc. That's wrong for identity: PicPeak
uses email as a login identifier, so `john.doe@gmail.com` getting
silently stored as `johndoe@gmail.com` means the user can't log in
with the address they were invited with.
The bug existed at 17 call sites across the codebase (auth, admin user
create/update, customer create/update, event create/update on three
different routes, customer login, feedback submission). All of them
are identity-bearing — none had a legitimate reason to strip dots
for deduplication.
Fix: introduce one shared options object in `utils/emailNormalization`
disabling every provider-specific normalization
(gmail_remove_dots, gmail_remove_subaddress,
gmail_convert_googlemaildotcom, outlookdotcom_remove_subaddress,
yahoo_remove_subaddress, icloud_remove_subaddress). The only default
left enabled is `all_lowercase`, which is safe — local-parts are
case-insensitive in practice on every major provider, and lowercasing
keeps login lookup consistent.
Every call site updated to pass the shared options. 7 unit tests pin
the preserved-dots, preserved-subaddress, preserved-googlemail-domain,
and still-lowercase behaviours so a future refactor can't silently
regress.
## Migration note
Existing accounts whose emails were already stripped before this fix
remain with the stripped form in the DB. The fix takes effect for new
invitations going forward. If an admin re-invites an existing user
with the un-stripped address, that would create a duplicate account —
out of scope here; if it becomes a real problem we can add a
backward-compat login fallback (try lookup with dot-stripped form too)
as a separate change.
Folds all three follow-up items tracked in #525 into one commit:
1. Mirror PR #500's category scoping on adminPhotos.js. The admin
upload route at adminPhotos.js:231 still accepted any category_id
without event scoping — quietly less strict than the public v1
API after #500 landed. Same one-liner fix (event_id OR is_global)
with a matching 400 response shape so admin + v1 stay consistent.
2. Extract a shared slugify() in backend/src/utils/slug.js with the
NFD-strip-combining-marks fix from #502, and route 5 callers
through it:
- adminEvents.js (event-name slug)
- events.js (event-create slug)
- v1/events.js (replaces local slugify helper)
- adminArchives.js (archive→category slug)
For pure-ASCII input the output is byte-identical to each old
inline pipeline, so existing slugs in the DB keep round-tripping
cleanly via lookup. Accented inputs now transliterate (Família
→ familia) instead of dropping the diacritic (Família → f-mlia).
adminCategories.js stays with its own pipeline (underscores-as-
word-chars semantics differ from the events-style transform —
changing would silently shift wedding_party → wedding-party on
new inserts). xmpGenerator.sanitizeKeyword stays unchanged for
the same compat-cautious reason.
3. Cover the v1 upload happy path. Existing test only exercised the
400-out-of-scope branch. Add two happy-path cases that stub
sharp / generateThumbnail / storage.putFromFile and pin the
response shape (id, category_id, type, etc.) plus the collage-
slug → type='collage' flip. Temp file recreated in beforeEach
because the handler unlinks it on success.
Tests:
- New slug.test.js: 22 cases pinning ASCII parity with the legacy
pipeline (so the refactor is provably non-breaking for existing
data) and the corrected accent handling across de/es/fr/nl/pt
inputs, plus CJK and edge-case behaviour.
- events.category.test.js: 4 tests total (2 existing + 2 new happy
path).
- galleryOgService.shareImage.test.js: 11 (3 added in #521 + 8 pre-
existing) still pass.
37 tests pass across the three touched files.
Refs: #525, follows up #500 and #502
New Settings → General toggle `Use original filenames on download` (off by
default). When on, single-photo downloads, bulk/selection zips, and per-event
archive zips surface `photos.original_filename` instead of the sanitized
storage filename. Storage paths are unchanged.
- Content-Disposition uses RFC 5987 (`filename=` ASCII + `filename*=UTF-8''…`)
so unicode camera filenames survive while header-injection bytes are stripped.
- Zip entries are deduplicated with a deterministic `_1` / `_2` suffix on
collision (folder structure preserved in archive zips).
- Pre-generated download-all zips and the in-memory setting cache are
invalidated when the toggle flips so the next download rebuilds with the
new names.
- Falls back to the storage filename whenever `original_filename` is null
(legacy uploads predating migration 062).
The customer-portal squash inadvertently reverted the upstream/beta
fix from PR #427: production NODE_ENV was flipping the cookie Secure
flag back to hard `true`, which broke admin login on
HTTPS-frontend → HTTP-backend reverse-proxy stacks (browser drops
the Secure cookie over HTTP, login loops indefinitely).
Restored upstream/beta's tokenUtils.js verbatim and re-layered only
the customer cookie helpers (CUSTOMER_COOKIE_NAME,
setCustomerAuthCookie, clearCustomerAuthCookie,
getCustomerTokenFromRequest) on top.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implements the recurring-customer login surface from
the-luap/picpeak#354 plugged into the maintainer's
new feature-flag infrastructure (PR #443) instead of
a parallel toggle.
* New `customerPortal` feature flag (foundation flag for the
not-yet-built calendar/quotes/bills/messaging customer
surfaces). Defaults FALSE on fresh installs, TRUE on existing
installs (events > 0) via migration 095 so live customer
accounts don't disappear mid-deployment.
* Foundation schema: customer_accounts, customer_invitations,
event_customer_assignments, customer_password_resets, plus
RBAC permissions customers.view / .create / .delete granted
to super_admin + admin system roles.
* Backend: /api/admin/customers (invite, list, search, assign,
deactivate, reset password) + /api/customer/auth/* +
/api/customer/* (login, dashboard, accept-invite, reset).
Customer JWT bypass minted via
/api/customer/events/:slug/access-token so existing gallery
middleware stays untouched.
* Frontend: /customer/* route tree gated by RequireFeature flag
customerPortal, with login / dashboard / accept-invite /
reset pages and a customer-side sidebar layout.
/admin/customers and /admin/customers/:id gated identically.
* Settings → Features grows a "Customers" section with a
Customer portal card. The maintainer's Features tab stays the
single source of truth — no parallel Advanced features tab.
* CustomerAccountPicker on event create/edit forms hides itself
when the flag is off; backend ignores customer_account_ids in
that case instead of erroring the whole event save.
Translations: en + de hand-translated. nl/pt/ru fall through to
en — flagged here as needing native review.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two intertwined bugs reported in #427 by @iSchumi6210:
1. Login silently fails over HTTP. Backend defaulted COOKIE_SECURE to true
when NODE_ENV=production. Over plain HTTP the browser drops the Secure
cookie → next /auth/session request returns 401 → redirect back to
/admin/login → no error shown. picpeak-setup.sh writes
NODE_ENV=production but never writes COOKIE_SECURE, so every first-time
install without a reverse proxy hits this.
2. Admin password is generated but admins can't find it. The 001_init.js
migration writes the generated password to data/ADMIN_CREDENTIALS.txt
inside the backend container, but picpeak-setup.sh only copies it out
when --reset-admin-password is passed. Default-path users never see it
and resort to manual bcrypt updates in psql.
Changes:
- tokenUtils.js: production default goes from `true` to `'auto'`. On real
HTTPS req.secure is true → Secure flag is still emitted (no security
regression for reverse-proxy deployments). On plain HTTP req.secure is
false → Secure flag omitted → login works. Users who explicitly want
the strict HTTPS-only behaviour can still set COOKIE_SECURE=true.
- .env.example: rewrite the COOKIE_SECURE block to make the new default
obvious and explain when to override (set =true for strict, =false to
skip the per-request check, leave unset for the auto behaviour).
- picpeak-setup.sh (both Docker and native paths):
- Write COOKIE_SECURE=auto explicitly to the generated .env (defense in
depth so the right behaviour is preserved even if the backend default
flips again later)
- After migrations, ALWAYS copy ADMIN_CREDENTIALS.txt out of the
backend container/data dir to the host data dir, chmod 600, and print
the email + password to the install output. The credentials file
remains as a backup record that the operator should delete after
noting the password.
Verified locally with all 4 permutations of NODE_ENV × COOKIE_SECURE:
production, unset → HTTPS: secure=true ✓ HTTP: secure=false ✓ (was both true)
production, =true → both: secure=true (strict opt-in preserved)
production, =auto → HTTPS: secure=true HTTP: secure=false (already-correct)
development, unset → both: secure=false (dev unchanged)
Bundle of email-renderer and email-caller fixes triggered by a
reproducer on picpeak.nothaft.cloud (gallery_created mail showing
literal `{{#if welcome_message}}` markers and `Passwort: (set at
creation)`). The audit that followed surfaced six more user-visible
defects in the same surface; all are fixed here so customer-facing
mail renders cleanly.
Renderer (`backend/src/services/emailProcessor.js`)
- `safeTemplateReplace` now resolves `{{#if VAR}}…{{/if}}` blocks
before flat `{{var}}` substitution. The shipped templates have used
Handlebars-style conditionals since migration 026; the renderer
ignored them, so the markers leaked verbatim into every mail with
an empty welcome_message. Lifted to module scope and exported so
the conditional contract is unit-testable. Single-pass, non-nested
(commented).
- Added `passwordSetAtCreationI18n` next to the existing two i18n
password sentinels so `(set at creation)` (sent by the publish-
from-draft flow when only the bcrypt hash remains) is localised
to "Das bei der Erstellung der Galerie gesetzte Passwort" /
equivalent in EN/DE/NL/PT/RU instead of the raw English string.
- Added an opt-in `{ escapeHtml: true }` mode to `safeTemplateReplace`
so admin-supplied free text (`event_name`, `host_name`, …) is
HTML-escaped on substitution into the HTML body. Allowlist of
passthrough keys (`welcome_message` already-HTML, server-generated
URLs `gallery_link` / `client_link`). Subject and text body keep
the legacy unescaped behaviour. `formatWelcomeMessage` now escapes
before nl2br so the welcome_message allowlist is safe.
- New `htmlToText()` strips `<style>` and `<script>` blocks (and
their content) before tag-stripping, decodes common entities, and
collapses whitespace. Used by the textBody fallback in
`sendTemplateEmail` — without this, every template missing a
`body_text` produced a "plain-text" mail starting with the 100+
lines of CSS embedded by `wrapEmailHtml()`.
- The client-access section (#172) now mirrors its HTML block into
`textBody` using the same per-language strings, so plain-text
recipients see the link / PIN / warning. `pinLabel = 'PIN'` moved
into `clientAccessI18n` (RU uses ПИН-код).
- Added `getSupportEmail()` exported helper that reads
`branding_support_email` from `app_settings` (JSON-decoded), with
the SMTP from-address as fallback. Used by the gallery_expired and
archive_complete callers below.
- Removed dead `require('handlebars')` (unused since the regex
renderer landed; pre-existing lint error in this file).
Callers (data the templates already reference)
- `expirationChecker.js queueExpirationWarning`: send `expiry_date`
(templates use this, the old code sent `expiration_date` —
typo'd key, never read), drop the hard-coded `.de`/`en` sniff
(the processor formats with the recipient's resolved language),
add the `{{password_security_message}}` sentinel for
`gallery_password` (plaintext is gone by warning time, so
customers used to see literal `{{gallery_password}}` in the mail).
- `expirationChecker.js handleExpiredEvent`: both queueEmail calls
now supply `host_name`, `event_date`, `expiry_date`,
`support_email` so the EN/DE/NL/PT/RU `gallery_expired` template
doesn't render literal `{{host_name}}, your gallery expired on
{{expiry_date}}`. Skip the duplicate admin send when
admin_email == customer_email.
- `archiveService.js`: `archive_complete` queue now supplies
`host_name`, `photo_count` (from `photoEntries.length`),
`archive_date`, `support_email` — the previous payload had only
`event_name` and `archive_size`, so most of the mail was
unfilled placeholders.
Tests
- `__tests__/services/emailProcessor.safeTemplateReplace.test.js`:
16 cases — flat substitution, conditional truthy/falsy/missing/
multi-line/sibling/numeric-0, plus 5 cases for the new
`escapeHtml` option (default off, escape on, allowlist
passthrough for welcome_message and gallery_link).
- `__tests__/services/emailProcessor.htmlToText.test.js`: 7 cases —
the regression scenario (full wrapped body with embedded `<style>`
block), tag-stripping, entity decoding, paragraph spacing.
- `__tests__/utils/formatters.test.js`: 12 cases for `escapeHtml`,
`nl2br`, and the now-escaping `formatWelcomeMessage`.
35 cases total, all green. Lint clean on every touched file
(also fixes a pre-existing `no-prototype-builtins` warning in the
process). Pre-existing failures in
`__tests__/services/backupService.enhanced.test.js` are unrelated
and pre-date this branch.
Adds a third value for the COOKIE_SECURE environment variable that
decides the cookie Secure flag per-request based on req.secure. This
unblocks a common self-hosted setup where the same PicPeak deployment
is reachable over both HTTPS (via reverse proxy) and plain HTTP (e.g.
LAN access at http://192.168.x.x:3001).
Behavior
unset - legacy default: follows NODE_ENV (production=true, dev=false)
true - always set Secure (unchanged)
false - never set Secure (unchanged)
auto - NEW: use req.secure per request. In practice this means
Secure on HTTPS requests (when X-Forwarded-Proto: https
reaches Express via a trusted proxy) and no Secure flag
on plain HTTP requests.
The existing trust proxy config (`app.set('trust proxy',
'loopback, linklocal, uniquelocal')` in server.js) means
X-Forwarded-Proto is honored when forwarded from local/private-network
proxies, which covers Docker network setups and most self-hosted
deployments behind NPM, Traefik, or Caddy.
auto is strictly opt-in. The default behavior is unchanged, so existing
users see no difference. A follow-up release can consider promoting
auto to the default after real-world feedback.
Also fixed (latent bug, benefits everyone)
Cookie clear operations (clearAdminAuthCookie, clearGalleryAuthCookies)
previously wrote the same `secure` attribute as the set path. When a
cookie was set with Secure=true over HTTPS and the clear request came
over HTTP (or vice versa under auto mode), some browsers would reject
the Set-Cookie delete header, leaving the cookie in place. Browsers
match cookies by (name, domain, path) for deletion and don't care about
Secure, so the new buildClearCookieOptions() helper simply omits the
secure attribute.
Implementation
- secureCookie string is replaced by secureCookieMode which can hold
true, false, or 'auto'.
- New resolveSecureFlag(res) returns the boolean for a specific
response, delegating to res.req.secure when in auto mode.
- buildCookieBaseOptions and buildCookieOptionsWithExpiry now take res
and pass it through.
- New buildClearCookieOptions() deliberately omits `secure`.
- setAdminAuthCookie / setGalleryAuthCookies / clearAdminAuthCookie /
clearGalleryAuthCookies all updated to thread res where needed.
Public signatures unchanged — every caller already has res in scope.
Testing
Verified against a real Express instance inside the backend container
with trust proxy configured, covering:
- (unset) + NODE_ENV=production -> secure: true (legacy)
- (unset) + NODE_ENV=development -> secure: false (legacy)
- COOKIE_SECURE=true + req.secure=false -> secure: true (literal wins)
- COOKIE_SECURE=false + req.secure=true -> secure: false (literal wins)
- COOKIE_SECURE=auto + X-Forwarded-Proto: https -> secure: true
- COOKIE_SECURE=auto + plain HTTP -> secure: false
- clearCookie always omits the secure attribute
Documentation
Added a COOKIE_SECURE block to both .env.example files (root for
docker-compose, backend/.env.example for native install) explaining the
four values, when to use auto, and the two requirements (proxy must
forward X-Forwarded-Proto, proxy IP must be in the trust list). Also
documented COOKIE_SAMESITE and COOKIE_DOMAIN alongside, which were
previously undocumented.
Introduces a new "Per-guest selections" identity mode for event
feedback, letting each visitor register under their own name so their
likes/favorites/comments/ratings are tracked independently. Includes
admin insights (list, per-guest detail, aggregate view, export) and
advanced identity features (forget-me, email recovery, invite tokens,
merge).
New event-level setting
- event_feedback_settings.identity_mode = 'simple' | 'guest' (default
'simple' → zero behavior change for existing events).
- Admin UI radio under Feedback Settings to toggle per event.
Root cause of the previous "all guests share state" bug
- generateGuestIdentifier() was sha256(ip + userAgent), so every visitor
on the same WiFi + similar device collided into one identity.
- Now: when a verified guest JWT is present (x-guest-token header),
req.guest.identifier takes precedence — per-person rate limits and
per-person deduplication.
Phase 1 — identity layer
- Migration 078: new gallery_guests, guest_invites, guest_verification_
codes tables; identity_mode column + check constraint; nullable
guest_id FK on photo_feedback.
- New guest JWT type scoped to (eventId, guestId).
- New middleware guestAuth.resolveGuest (non-blocking) + requireGuest.
- POST /gallery/:slug/guest, GET /guest/me, DELETE /guest/me.
- Gallery feedback route enforces guest identity in guest mode and
reads name/email from the verified token (never from the body).
- Frontend GuestIdentityContext + GuestNamePromptModal; axios
interceptor injects x-guest-token on gallery API calls.
- Feedback-only blocking: gallery opens freely, prompt only on first
interactive feedback action.
- Admin "Guests" tab (conditional on identity_mode='guest') with the
AdminGuestsList component.
Phase 2 — admin insights
- GET /admin/events/:eventId/guests list + aggregated counts.
- GET /admin/events/:eventId/guests/:guestId detail with per-type
groupings; AdminGuestDetail modal with thumbnail grid + tabs.
- GET /admin/events/:eventId/guests/aggregate sorted by distinct guest
pick count; GuestSelectionsAggregate component.
- Per-guest export (txt/csv/json) and bulk export-all ZIP.
Phase 3 — polish
- 3.1 Self-service forget-me link in gallery footer.
- 3.2 Email-based identity recovery: POST /guest/recover sends a
6-digit code via the existing emailProcessor, POST /guest/verify
exchanges it for a token (rate-limited, enumeration-safe).
- 3.3 Admin invite tokens: pre-mint identities, share URLs with
?invite=, single-use redemption stripping the param from history.
- 3.4 Admin merge endpoint reassigns feedback + soft-deletes sources.
Shared helper
- useGalleryFeedbackAction hook wraps the identity-check logic for
inline like buttons across Masonry/Grid/Justified/Mosaic/Carousel/
Timeline/Premium layouts.
Backwards compatibility
- Existing events default to 'simple' after migration; behavior
unchanged.
- Legacy photo_feedback rows keep guest_id NULL; admin shows them in
the generic feedback moderation view as before.
- feedback_count denormalized stat now uses COALESCE(guest_id,
guest_identifier) so per-guest counts are accurate without touching
legacy rows.
Verified end-to-end against local Docker
- Migration clean on existing data.
- Simple mode unchanged (no prompt, legacy flow).
- Guest mode: Alice registers on click, tokens persist in
sessionStorage, feedback rows carry guest_id.
- Carol via invite link auto-redeems, sees Alice's "1 likes" badge.
- Admin Guests tab shows both with correct counts; detail modal
displays thumbnail grid with badges; aggregate view sorts by picker
count (photo 227 = 2, others = 1); CSV/JSON export matches DB.
- Merge Carol into Alice: feedback reassigned, Carol soft-deleted,
Alice count = 4.
Draft Mode:
- Events are created as drafts by default — no email sent until published
- Add "Publish & Notify Client" button with confirmation dialog
- Draft banner with yellow styling on event details page
- Draft filter tab in events list
- Gallery middleware blocks public access to draft events
- Migration 076 adds is_draft column to events table
Admin Draft Preview:
- Admins can preview draft galleries via JWT preview token (?preview=)
- "View Gallery" link on drafts auto-appends preview token
Admin & Login Page Branding:
- Admin header uses configured company logo/name from branding settings
- Login page shows configured logo instead of hardcoded PicPeak
- Respects logo_display_mode (logo_only, text_only, logo_and_text)
OG Tag Branding:
- DynamicFavicon component updates OG meta tags and page title from
branding settings
Editable Client Email:
- Customer email is now editable after event creation in edit mode
Branding Inheritance:
- New events inherit hero logo settings (visibility, size, position)
from global branding configuration
Share Link Full Domain URL:
- New getFrontendBaseUrl() utility with DB fallback to general_site_url
- Used in email processor and share link service
- Add photo_cap column to events table (migration 074) to limit photos per event
- Enforce photo cap in upload route, returning 400 when limit exceeded
- Pass photo_cap through all event CRUD routes and frontend forms
- Add complete Portuguese (pt-BR) translation (2300+ strings)
- Register pt locale in i18n config, language selector, date formatting
- Add photoCap/photoCapHelp translation keys to all locale files (en, de, ru, pt)
- Disable production source maps and hide nginx version
- Reduce JSON body limit from 10gb to 50mb (uploads use multer, not JSON)
- Strip database info and error details from health endpoint
- Mask reCAPTCHA secret key in admin settings API responses
- Whitelist sort/order query parameters in events and photos endpoints
- Stop reflecting arbitrary origins in static file CORS headers
- Align nginx security headers with backend Helmet CSP, remove deprecated X-XSS-Protection
- Strip EXIF metadata from generated thumbnails and hero images
- Bind postgres/redis dev ports to localhost in docker-compose configs
- Add safeExec utility (spawn with shell:false) to prevent command injection
- Convert all exec/execAsync calls in backup, restore, and database backup
services to use safe spawn-based helpers
## Changes
### CSS Template System
- Added CSS class hooks to gallery components for custom template targeting
- Gallery sidebar, header, footer, and photo cards can now be styled via CSS templates
- CSS variables on :root allow themes to override colors, effects, and spacing
### Gallery Component CSS Classes Added
- `.gallery-page` - Main gallery container
- `.gallery-header` - Top header bar
- `.gallery-sidebar` - Filter/download sidebar
- `.gallery-sidebar-header`, `.gallery-sidebar-title`, `.gallery-sidebar-close`
- `.gallery-sidebar-content`, `.gallery-sidebar-section`
- `.gallery-sidebar-search-input`, `.gallery-sidebar-search-icon`
- `.gallery-sidebar-backdrop` - Mobile overlay
- `.gallery-btn`, `.gallery-btn-download` - Sidebar buttons
- `.gallery-footer` - Footer section
- `.photo-card`, `.photo-grid` - Photo display elements
### CSS Templates (Database)
- Elegant Dark (id=1): Dark navy theme with light text and red accents
- Liquid Glass Light (id=2): iOS 26 frosted glass effect with gradient background
### Bug Fixes
- Fixed CSS variables not inheriting (moved from .gallery-page to :root)
- Fixed sidebar position breaking layout (removed position: relative override)
- Fixed Elegant Dark sidebar text visibility (white on white issue)
### Other Changes
- Settings page refactoring and cleanup
- i18n locale updates for new gallery features
- Vite proxy port configuration fix
- Admin auth route improvements
- CSS templates service updates
This commit implements full video upload, storage, streaming, and playback functionality
for the PicPeak photo sharing platform, allowing users to upload and view videos alongside
photos in galleries.
Backend Changes:
- Added video processing dependencies (fluent-ffmpeg, @ffmpeg-installer/ffmpeg)
- Created videoProcessor.js service for video metadata extraction and thumbnail generation
- Updated photoProcessor.js to handle both images and videos
- Modified adminPhotos.js to accept video files with 500MB size limit
- Enhanced gallery.js with HTTP range request support for video streaming
- Expanded fileSecurityUtils.js with video MIME types and magic number validation
- Added database migration for video support columns (media_type, duration, codecs, dimensions)
Frontend Changes:
- Updated TypeScript types to include video metadata fields
- Created VideoPlayer.tsx component with custom controls
- Modified PhotoUpload.tsx to accept video files (.mp4, .webm, .mov, .avi)
- Updated UserPhotoUpload.tsx for guest video uploads
- Enhanced PhotoGrid.tsx with video badges and duration display
- Modified PhotoLightbox.tsx to conditionally render VideoPlayer for videos
Database Schema:
- Added media_type column ('image' | 'video')
- Added mime_type, duration, video_codec, audio_codec columns
- Added width and height columns for media dimensions
- Migrated existing photos to media_type 'image'
Features:
- Video thumbnail generation from video frames
- Streaming support with range requests for efficient playback
- Video duration display on thumbnails
- Play button indicators on video items
- Full-featured video player with playback controls
- Support for MP4, WebM, MOV, and AVI formats
- Fixed 400 Bad Request error when submitting feedback with name/email required
- Updated backend validation to properly handle empty/undefined name/email fields
- Modified frontend components to send undefined instead of empty strings when fields are not provided
- Fixed thumbnail display issue in moderation view by using correct admin API endpoints
- Updated FeedbackModerationPanel and EventFeedbackPage to display thumbnails correctly
The issue was caused by the validation logic treating empty strings differently than undefined values.
Frontend components now properly send undefined when name/email are not provided, and the backend
validation correctly handles both cases.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Fixed backend validation to properly handle empty strings in validateGuestRequirements
- Added Boolean conversion for SQLite boolean values in feedback settings API response
- Created FeedbackIdentityModal component for collecting name/email when required
- Updated PhotoLikes, PhotoRating, and PhotoFavorites components to show modal when requireNameEmail is true
- Fixed issue where require_name_email field was not reaching frontend due to missing boolean conversion
This ensures that when 'Require Name & Email' is enabled, guests are prompted with a modal to provide their information before submitting feedback, preventing 400 Bad Request errors.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
Paul Nothaft
paul