* feat(gallery): info banner above the photo grid (#932)
A short informational note rendered at the TOP of a gallery, above the
photos. Distinct from the promotional banner (#440), which stays by the
footer for marketing copy — the reporter's case is an onboarding hint ("use
the menu button to filter"), which is useless below a gallery the guest has
to scroll past first.
Mirrors the promo feature's shape rather than inventing a second one: a
global default in Settings → Branding (branding_info_markdown) plus a
per-event inherit/custom/off override. Markdown via the existing
MarkdownContent sanitiser — no raw HTML, no CSS injection. Empty global
default means nothing renders, so upgrading changes nothing visible.
Deliberately NOT included: an alignment knob (this is short helper copy, not
marketing layout) and guest dismissal — the issue lists dismissal as a
nice-to-have, and it needs per-guest persistence that is its own decision.
Migration 176 is idempotent (hasColumn / existing-key guarded).
Note on the payload plumbing: the per-event fields travel in the /photos
response, not just /info. GalleryAuthContext seeds its cached event from the
gallery LOGIN response — a small identity subset — so anything absent there
is undefined right after a guest signs in. /photos is the payload that
refreshes on every gallery load, which is why the fields were added there
and why GalleryView reads them from `data.event`. Verified in a browser
across all three modes; reading them from the context event instead silently
collapsed every override back to 'inherit'.
* fix(branding): map branding_info_markdown on read so saving can't wipe it (#932)
External review caught this. BrandingSettings declared no info_markdown and
formatBrandingSettings never mapped branding_info_markdown, so BrandingPage's
hydration — setBrandingSettings(prev => ({ ...prev, ...formatted })) — kept
the empty-string initializer instead of the persisted value. The form loaded
blank and the next Save posted '' back, wiping a configured banner. Silently:
the gallery keeps rendering the old copy until that save lands.
This is the same bug the footer/promo fields hit in #441 + #440 / #460, which
the read mapper still carries a comment about. Add the field to the interface
and the mapper, and pin the round-trip for the whole editable branding set so
the next field added is caught by a test rather than by a user losing copy.
Verified: the new test fails 3/4 with the mapper line removed.
* fix(gallery): honour the info-banner override in the reveal-hidden view (#932)
External review, round 2. The hidden-until-reveal branch renders GalleryLayout
with the context `event`, which is seeded from the gallery login response and
carries no banner fields — so while a gallery was hidden, a per-event 'off'
silently resolved to 'inherit' and the global banner appeared on a gallery the
admin had muted.
Resolve the fields there the same way the main render path does. The two
full-page layouts (gallery-premium, gallery-story) are deliberately left alone:
they return before GalleryLayout and render no header, footer or promo banner
either — injecting a wrapper into layouts documented as having 'their own
integrated UI' would be a design change, not a fix.
---------
Co-authored-by: Paul Nothaft <[email protected]>
Reshaped onto main after #1039 landed the coercion engine
(typedColumnsFor / epochToIso / coerceForTargetEngine) — this PR is now
only the policy delta on top of it:
- validateManifest: replace the CLI-only allowEngineSwitch opt-in with a
direction rule — sqlite → pg allowed (upload UI and CLI alike),
pg → sqlite refused with a message naming the supported direction
- importFromPicpeak: derive crossEngine from the manifest's engine
(absent field = target engine, the exact pre-change behavior), log it,
return it; route passes it through
- scripts/migrate-sqlite-to-postgres.js: rely on the shared gate, drop
the flag
- restore card: direction stated in the intro, cross-engine notice after
a converting restore; both strings in en.json + de.json; removed the
orphaned settings.backup.picpeak locale node (unreferenced, stale copy)
- picpeakCrossEngine.test.js: direction policy, epochToIso (ms, seconds,
numeric strings), coerceForTargetEngine units, plus
PICPEAK_PG_TEST_URL-gated real-Postgres stored-value assertions
Co-authored-by: Paul Nothaft <[email protected]>
* feat(permissions): granular permission gating + role editor & presets
Make every admin feature permission-gateable so multi-user studios can
split capability across roles (#747, and phase 1 of #743).
- Split the catch-all settings.edit into dedicated dangerous-config perms
(banking / domains / security / integrations / features): a team member
can no longer change IBAN, domains, SSO, webhooks, API tokens or feature
flags. Reads keep an OR with settings.view so existing roles keep
visibility. The site-URL write inside /general is change-gated on
settings.domains.
- Add dedicated perms for admin surfaces miscategorised under settings.*
(whatsapp, event_types, image_security, notifications, system) plus
roles.manage and vat_codes.view; gate the previously-ungated VAT read.
- Boot self-heal (_permissionsBoot.js): super_admin always holds every
permission (tracks-all) so new perms never need a compensation
migration; all other roles stay frozen (no silent escalation on upgrade).
- Seed two presets: Solo Photographer (full operator) and Team
Photographer (contributor — view events + manage photos + read-only CRM;
no settings/users/billing edits, no events.edit).
- Role editor: adminRoles CRUD (create/edit/clone/delete + permission
matrix; system roles protected, super_admin immutable) and a Roles tab
with a category-grouped matrix and preset cloning.
- Settings page tabs are permission-gated with snap-back; i18n en/de.
Migration 174. Backward-compatible: admin/editor/viewer unchanged.
* feat(permissions): hide in-page action buttons a role can't use
Wrap mutating controls on the surfaces restricted roles actually reach
(Events list, Archives, gallery photo grid, event detail) in
PermissionGate so they are HIDDEN when the user lacks the permission,
rather than shown-then-403:
- Events list: create / bulk archive / bulk delete / row archive /
row delete / download-archive.
- Archives: restore / download / delete.
- Photo grid: single + bulk delete (photos.delete), per-photo download
(photos.download), bulk move/hide/show (photos.edit).
- Event detail: edit / rename / publish (events.edit), duplicate
(events.create), archive (events.archive), create-invoice
(bills.manage); the Actions card is hidden entirely for view-only roles.
- Photos tab: upload / external import (photos.upload), export menu
(photos.download).
Backend already enforces these with 403; this is the matching UX so a
Team Photographer never sees delete/settings controls.
* fix(permissions): close settings-split bypass via generic settings writers
Security review found the settings.edit split was bypassable: the generic
settings writers (/general, /analytics, /seo, /security) upsert arbitrary
setting_keys, so a role holding only settings.edit (or settings.security)
could write keys owned by a narrower permission — repointing the public
site URL (settings.domains), security policy (settings.security) or
VAT/accounting config (settings.banking) via the wrong endpoint.
Add stripUnauthorizedProtectedKeys(): before every generic upsert, drop
any protected key the caller isn't permitted to write (general_site_url →
settings.domains, security_* → settings.security, accounting_* →
settings.banking). Dedicated routes still work because their caller holds
the matching perm. Replaces the narrower in-handler site-URL guard.
Also fix two tests affected by the RBAC changes:
- authzPermissionGaps: API-token management moved to settings.integrations,
so grant that (not settings.edit) to exercise the ownership 404.
- AdminPhotoGrid.viewToggle: stub PermissionGate (its buttons are now gated
and the test renders without a PermissionsProvider).
* fix(permissions): address upstream review (#1045)
- Renumber migration 174 -> 175 (174 now taken by 174_sqlite_nullable_event_dates
from #1035; the collision made picpeakImportService's forward-only restore
guard treat both as order 174 and accept a newer .picpeak onto an older schema).
- Contain the roles.manage blast radius (delegation, not root escalation): a
non-super_admin can no longer edit their own role, nor grant any permission
their own role doesn't already hold (createRole + updateRole).
- Protected-key denial now 403s (naming the keys + required perms) instead of
silently stripping and reporting "saved" (adminSettings generic writers).
- Reserve team_photographer so a custom role can't squat the preset name.
- Boot self-heal: per-step try/catch so a role_permissions insert race on one
replica doesn't skip preset seeding.
- Forward-project the feature .manage perms that also replaced settings.edit
gates (whatsapp/event_types/image_security/notifications/system), matching the
settings.* split projection so the pattern is symmetric for phase-2.
- Guard exports.down's roles/admin_users queries with hasTable.
* fix(permissions): change-detection on protected-key 403 + commit guard tests (#1045)
Round-2 review:
- The protected-key 403 fired on key PRESENCE. The General tab re-posts
general_site_url on every save, so a settings.edit-only role (the office
manager this PR enables) got 403'd on every General save even when the URL
was unchanged. Restore change-detection: compare the incoming value against
the stored one and 403 only on an actual change; unchanged protected keys are
dropped so the rest of the save proceeds. Only /general is affected.
- Commit the self-amplification guard test (was run locally, never staged):
adminRolesGuards.test.js — non-super can't grant perms it lacks, can't edit
its own role, can't escalate another role; super_admin bypasses;
team_photographer name reserved.
- Add adminSettingsProtectedKeys.test.js pinning the change-detection: an
unchanged general_site_url saves, an actual change 403s, super_admin changes it.
Enabling Guest Feedback on an event could silently do nothing.
1. `updateEventFeedbackSettings` spread the request body straight into the
knex UPDATE. The admin event form posts its whole client-side state,
including three keys that were never columns on event_feedback_settings
(`enable_rate_limiting`, `rate_limit_window_minutes`,
`rate_limit_max_requests`), so the write threw and the route answered 500.
Writable columns are now whitelisted; identity columns and timestamps stay
server-managed.
2. EventDetailsPage swallowed that 500 in a bare `catch {}` ("Error already
handled by mutation" — it is a different request), so the admin was left
looking at "Event updated successfully" while the toggle never persisted.
The error is surfaced now and the settings query is invalidated on success.
3. gallery.js declared a duplicate `GET /:slug/feedback-settings`. server.js
mounts galleryRoutes before galleryFeedback, so it shadowed the real
handler and dropped the per-guest caps (#655) from the guest payload — the
gallery could never render the favorite/like limits or their counters.
Timestamps are written as ISO strings so they round-trip on both engines.
Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM
Co-authored-by: Paul Nothaft <[email protected]>
Phase 3 (final) of #1000. The deep content now lives on the docs site (PicPeak/docs#7), making docs.picpeak.app the single source of truth and removing the in-repo copies.
README links flip to docs.picpeak.app; the roadmap table is retired in favour of GitHub Issues. Deletes docs/_to-migrate/ and the five migrated pages. docs/migration-to-org.md stays — it's repo-transitional, not docs-site content.
In-app references to the deleted files are repointed at the docs site, including the CRM disclaimer strings in en.json/de.json and the contract-editor fallback.
Closes#1000.
Clients who need smaller files no longer make the photographer re-export. Two capabilities, both off by default.
STANDARD RESOLUTION — the size a gallery hands out for every ordinary download (single, selected, download-all). Global default in Settings, overridable per gallery with the NULL=inherit tri-state. The pre-built download-all zip is built AT the standard resolution, so changing it invalidates those archives, including a fan-out to inheriting galleries.
RESOLUTION PICKER — opt-in modal letting guests choose a different size. Custom archives are built as a DB-backed job the client polls, never cached. The picker never offers a size above the standard, and Original reappears only when the admin explicitly allows it.
Resize is fit:'inside' + withoutEnlargement — aspect preserved, never upscaled — applied before the watermark, since the mark is sized relative to its input.
Three rounds of external review hardened this: job archives are bound to the requester's visibility scope and re-validated at delivery, the streamed download-all path applies the cap, queue admission is bounded, and rejected resolutions no longer inflate download stats.
Closes#858.
The slideshow resolved its image as preview_url || hero_url || url. preview_url is only emitted when lightbox_preview_enabled is on (default false), so a default install fell through to hero_url — the 1920x1080 fit:'cover' centre crop built for gallery header banners. object-fit: contain then letterboxed an already-cropped 16:9 frame, so portrait photos lost their top and bottom and 'Black Bars (No crop)' looked inert.
Emits slideshow_url (same aspect-preserved preview tier) unconditionally for image photos; the show prefers it and never falls back to hero_url. preview_url stays gated so the lightbox opt-in is unchanged.
Fixes#1015.
Closes#1003.
#999 centralised the attribution so branding_hide_powered_by is honoured
everywhere, but GalleryLayout kept its own inline guard. The gallery footer
therefore still flashed — it kept `!brandingSettings?.hide_powered_by`, where
undefined is falsy, so a white-labelled instance briefly showed the attribution
on first paint, on the surface a white-label customer is most likely to see.
And there were two implementations of one rule, which is the bug class #999
existed to close.
The footer appends the attribution to its copyright line inside an existing
<p>, so a straight swap would nest a <p> in a <p>. Added an inline variant
rendering a <span> that carries the leading ' | ' itself: the separator belongs
to the component, since a caller placing its own would have to repeat the
visibility guard to avoid leaving a dangling separator when the attribution is
hidden.
No extra request — GalleryView already uses usePublicSettings(), the same hook
and react-query key, so the cache is shared. The footer also picks up
common.poweredBy, so it is translated rather than hardcoded English.
Removes the now-unread hide_powered_by from GalleryLayout's prop type and the
mapping feeding it in GalleryView.
Four cases cover the variant — span not paragraph, separator present, separator
hidden with the attribution when white-labelled, hidden while loading. Each was
checked against the pre-fix shape: rendering a <p> or moving the separator out
breaks one.
Closes#997.
Send original files from any event as a token-protected download link, with an
optional client-upload channel. Strictly opt-in behind a new `transfers`
feature flag, default OFF.
Migrations 170-172 (transfers, transfer_files, transfer_extra_files,
transfer_uploads, transfer_recipients, transfer_downloads, default settings and
two email templates) — all hasTable/hasColumn-guarded and idempotent, with
destructive statements confined to down().
Backend: transferService (CRUD, 256-bit download token, 6-char upload token,
cross-event ZIP streaming of originals), admin CRUD routes, and two public
token routes. transferCleanupService runs an hourly retention sweep; source-event
photos are never touched. All three routers fail closed via
requireFeatureFlag('transfers').
Review closed two ownership blockers, both the same root cause — permissions
used where ownership was needed:
- photoIds arrived from the request body and were validated only for existence,
so a scoped admin could bundle any event's originals and hand them out through
the public download token. filterOwnedPhotoIds now resolves ids to their events
and gates them through filterOwnedEventIds, on both the create and add-files
paths.
- The transfer list was unscoped and carried each row's download token, so any
admin with events.view could read another's token and fetch their originals.
The list is now scoped by created_by, the token/url fields are stripped from
the list payload, and a single router.use('/:id', requireTransferOwnership)
covers all twelve /:id routes, 404ing foreign and missing alike.
The admin photo picker filters its event list to the same rule, so the UI stops
offering picks the API would discard.
Fork-PR workflows had not been approved since the fix commits, so the PR's green
checks were stale against the pre-fix head. Verified by dispatching tests.yml
against the actual head: backend and frontend both green.
Follow-up: neither ownership guard has a regression test yet.
Co-authored-by: Luca-Timo <[email protected]>
branding_hide_powered_by only hid the attribution on the main gallery footer. It
stayed visible on the gallery password screen, client access page, Premium
layout, admin and customer login, accept-invite and CMS pages — AdminLoginPage
rendered it unconditionally with no guard at all, so the setting genuinely did
not apply there.
Routes those surfaces through one <PoweredBy /> component in components/common
that reads the public setting itself (the DynamicFavicon pattern) and renders
nothing when white-labeling is on, including while the settings are still
loading so a white-labelled instance never flashes the attribution.
Also collapses three duplicate translation keys (gallery.poweredBy,
adminLogin.poweredBy, customer.login.poweredBy) into a single common.poweredBy,
and translates pages that had 'Powered by' hardcoded in English across all 8
locales.
Fork-PR workflows were never approved so CI did not run. Verified locally
against cf243b44: tsc --noEmit clean, ESLint clean, vitest 124 passed across 24
files, and npm run build succeeds.
GalleryLayout.tsx keeps its own inline guard and is not routed through the new
component; tracked separately.
Co-authored-by: lbossuyt <[email protected]>
Closes#868.
A logged-in admin opening a published, password-protected gallery is let
straight in, mirroring the existing draft-visibility bypass.
Mechanism: an explicit ?admin_preview=1 intent flag AND a verified admin session
read from the httpOnly admin_token cookie (or an admin-typed Bearer) — never a
token from the URL. This retires the old ?preview=<raw-admin-JWT> scheme, which
leaked a 24h admin token into the address bar, referrers and proxy logs.
Per-request bypass only: no gallery JWT is minted, the password endpoint is
never reached so the login_attempts lockout buckets stay clean, and admin
previews are excluded from guest analytics (access_logs, download counts,
per-photo view_count, notification bells).
Review (two rounds) closed three blockers and two concerns:
- Transport: verifyGalleryAccess now resolves admin preview before any gallery
credential, and isAdminPreview reads the admin cookie first and type-checks
every candidate — so an admin Bearer no longer 403s on the type gate, and a
coexisting gallery session can no longer shadow the admin cookie.
- Reveal mode (#838) is a second consumer of isAdminPreview; its bypass is
unchanged, only the transport moves. revealMode.test.js updated off the
retired scheme and now carries a coexisting gallery Bearer.
- Admin previews no longer inflate per-photo view counts, and the internal photo
redirects preserve the flag via withPreview() so they still authorise.
- Happy path: GalleryPage renders GalleryView directly for a preview instead of
attempting the public empty-password auto-login, which 401'd against a
genuinely protected gallery and stranded the page on the skeleton.
The backend job timed out once at the 10-minute CI limit; a re-run completed in
2m02s, in line with main's ~2m10s baseline, so that was a runner flake rather
than a hang.
Relates to #985 — does NOT close it.
Adds registryMigrationRequired to the update-check payload (stable channel below
3.45.0) and an amber block in UpdateNotification explaining that the retired
registry path still responds, so `docker compose pull` appears to succeed while
serving the same frozen build.
Known limitation, established in review and merged deliberately: this cannot
reach the operators #985 describes. PicPeak is self-hosted, so the update-check
code runs inside the operator's own image — a v3.44.0 install runs v3.44.0's
backend forever, and the only external call returns release metadata, not logic.
Every build containing this predicate is >= 3.45.0, where it is false by
definition. The release-notes fallback fails too: the changelog modal shipped
2026-05-29, two days after the freeze.
Correct for any future rename, no runtime cost, but #985 stays open — the
population it describes still has no in-app channel. Viable routes are external
(retired GHCR package description, repo README, docs).
'0.0.0' is excluded from the predicate: that is getCurrentVersion's fallback for
an unreadable package.json, i.e. a broken install, not a pre-rename one.
Closes#983.
The two cross-add counter queries added in #979 were enabled on customers.edit,
but neither endpoint checks that permission:
HoursSection -> GET /expenses/inbound/by-customer/:id needs accounting.view
CustomerCrmPanels -> GET /customers/:id/hour-entries needs customers.view
An admin holding customers.edit but not the corresponding read permission fired
a guaranteed 403 on every customer-detail render. It degraded safely — the count
stayed at its 0 default so the cross-add was never offered, which is the right
outcome for that role — so this was request noise rather than broken behaviour.
Each guard now requires both: the read permission to fetch the count, and the
write permission because there is no point offering the cross-add to someone who
cannot create the combined invoice.
No seeded role is affected: migration 123 grants accounting.view and
accounting.manage together, and customers.edit projects forward from
customers.create, which migration 090 always grants alongside customers.view.
Closes#866.
Three features, all behind the `incomingInvoices` feature flag:
1. Attach the stored supplier proof PDF to the client-invoice email when a
captured invoice is re-billed/passed through, as a SEPARATE attachment so
invoice immutability holds. Global default (off), per-customer tri-state
override, and per-file selection in a new Send dialog. A missing proof at
issue time stamps inbound_documents.proof_attach_error rather than silently
dropping, and never blocks the send. Proof filename is a configurable
template with {INVOICE} {SUPPLIER} {YEAR} {MONTH} {SEQ}/{SEQ:0Nd} tokens.
2. Re-bills & passthrough panel under CRM → Customer, grouped Open/Sent/Paid
with status derived from the linked invoice lifecycle rather than a
duplicated column.
3. Cross-add dialog rolling open hours and open re-bills into one invoice,
symmetric from both entry points. The two stay distinct, contiguous line
groups — never merged into shared line items.
Migration 169 is additive, hasColumn-guarded and idempotent.
Review (two rounds) closed two concerns:
- Storno stranding: nothing cleared inbound_documents.billed_invoice_id when a
covering invoice was cancelled, so a Storno'd re-bill showed as Open in the
new panel while every billing path filters on that column being NULL — the
supplier cost could never be re-billed. releaseRebillsForCancelledInvoice now
detaches the linkage on both invoice-cancel paths, with a regression test on
the issued-cancel path.
- Permission gating: the new controls rendered on data presence alone while
their endpoints require accounting.view / accounting.manage / customers.edit.
Now gated at both the query and render layers.
Known follow-up: two cross-add counter queries are gated on a permission their
endpoint does not check (HoursSection.tsx:174, CustomerCrmPanels.tsx:270) —
degrades safely, one line each.
Closes#969.
The cockpit's email feed rendered preview/resend/cancel/retry/send-now for every mail, consulting neither the caller's role nor their permissions, producing controls that always failed:
404 - requireOwnedQueuedEmail scopes queued mail through email_queue.event_id AND ownership of that event. CRM document mail carries no event_id; and project ownership does not imply event ownership, so a project the caller owns can hold another admin's event.
403 - preview needs events.view but the four write actions need email.send.
getProjectOverview now stamps each email with an authoritative canAct, mirroring filterOwnedEventIds; created_by is selected only for that check and stripped before the response. The cockpit reads canAct and combines it with email.send. A missing canAct reads as false.
Regression from the GHSA-93x4 fix in #960/#966, which added the ownership middleware.
* fix(security): bound inbound-mail resources, redact secrets from logs (GHSA-2qf9, pgmp, r794)
GHSA-2qf9 — emailIntakeService downloaded, parsed and persisted every message
with no size, attachment-count or attachment-byte limit, reachable
unauthenticated by anyone who can email the operator's mailbox:
- fetch the envelope with `size` (same cheap pass) and refuse an oversized
message BEFORE downloading its source;
- cap attachment count and cumulative attachment bytes;
- limits env-overridable, defaults generous for real supplier invoices.
The teeth were in the dedup key. received_emails.message_id is varchar(512)
UNIQUE, and the failure path wrote `err-<uid>-<Date.now()>`, which can never
match the envelope-derived messageId the dedup pass compares against — so an
oversized (or overlong-Message-ID) mail was re-downloaded every poll forever,
and an OOM-kill/restart just resumed the loop. Size-skips are now recorded
under the REAL message id, and overlong ids collapse to a stable sha256 key
that always fits the column.
GHSA-pgmp / r794 — new sanitizeForLog() util (key-name deny-set, recursive,
cycle-safe) applied to the three request-body log sites in adminEvents/crud.js,
plus sanitizeValidationErrors() because express-validator's errors.array()
embeds the SUBMITTED value per field — a rejected plaintext password was still
logged. Scope is wider than filed: the update path also logged
client_password_hash and a LIVE client_share_token bearer credential.
Also: the one-time setup token was logged at warn AND printed to stdout on
every first boot, putting a live first-admin credential in combined.log,
security.log and `docker logs`. It is now written to the 0600 token file and
only surfaced when that write fails — the last-resort path it existed for.
* fix(security): codex round 3 — repair the first-run token recovery flow (GHSA-r794)
Two regressions from keeping the setup token out of the logs.
1. server.js decided whether to print the token by calling existsSync() on the
candidate path. That answers a different question than "did the write
succeed": a stale, read-only or directory-shaped SETUP_TOKEN reports as
present, so the banner suppressed the live token and pointed the operator at
content that is not it — leaving the current token only in combined.log
under default production logging. setupService now records the path the
write actually produced and exposes it via writtenSetupTokenFile().
2. The setup screen, its EN/DE strings, README, SIMPLE_SETUP and .env.example
all still told first-time users to run
`docker compose logs backend | grep -i "setup token"`. On the normal path
that command now returns a path banner and no credential, so the documented
browser-first onboarding could not be completed. They now point at
`docker compose exec backend cat /app/data/SETUP_TOKEN`, with the log
fallback described as what it is — the failure path.
Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me
---------
Co-authored-by: Paul Nothaft <[email protected]>
* fix(security): redact gallery share tokens from analytics page-view tracking (GHSA-7m6c)
* fix(security): codex round-1 — actually disable raw auto-tracking (GHSA-7m6c)
The previous patch was inert: App.tsx passed autoTrack:true (so Umami's
data-auto-track=false was never set) and the sanitized trackPageView had no
caller (useAnalytics sits outside <Router>), so the raw token URL still hit
the collector.
- Umami: drop autoTrack:true → data-auto-track=false; page views now come
from a sanitized manual tracker.
- Rybbit: its initial-load auto pageview can't be intercepted client-side, so
use native data-mask-patterns=['/gallery/**'] to strip the token on every
auto-tracked view; skip manual tracking for it to avoid double counting.
- Mount <AnalyticsRouteTracker/> INSIDE <Router> so manual tracking runs.
---------
Co-authored-by: Paul Nothaft <[email protected]>
* feat(gallery): multi-select feedback filters + sort direction controls (#889)
* fix(gallery): keep mobile sidebar open while combining feedback filters (#889)
* fix(gallery): generic sort icon when direction is uncontrolled (#889)
---------
Co-authored-by: Paul Nothaft <[email protected]>
* feat(gallery): per-event toggle to hide the logo on the password page (#894)
* fix(admin): harden login_logo_visible coercion for SQLite + string booleans (#894)
---------
Co-authored-by: Paul Nothaft <[email protected]>
* fix(security): close two access-control advisories (GHSA-g94x, GHSA-pv6w)
GHSA-g94x-8vv8-3c9f (HIGH) — the secure-image VIEW route
(/secure-images/:slug/secure/:photoId/:token) validated only the token
signature and took the gallery/photo from the URL, so a token minted on
any PUBLIC gallery read every other gallery's photos with no password
(its download sibling has verifyGalleryAccess; the view route can't —
it serves via <img src> with no header). Bind the token to its scope
instead: the URL photoId must equal the token's minted photoId (photos
belong to exactly one gallery, and minting is gallery-scoped), and the
gallery embedded in the token's sessionId must equal the URL gallery.
GHSA-pv6w-rj34-wj9v (MEDIUM) — GET /admin/backup/picpeak/export dumps
every table unredacted (bcrypt hashes, 2FA, SMTP/SSO/WhatsApp/webhook/S3
secrets) and was gated only by backup.create, which the built-in admin
role holds. Gate it behind super_admin, matching the restore side
(backup.restore, already admin-denied) and the masked config APIs.
Regression tests pin both: cross-gallery token reads 403 (photo and
gallery checks), backup export 403 for admin / passes for super_admin.
* test: stub requireSuperAdmin in the backup masking mock
adminBackup now calls requireSuperAdmin() at load (GHSA-pv6w export
gate), and backupSecretMasking mocks the permissions module — add the
new function to the mock so the module loads.
* fix(security): review follow-ups on the export gate (GHSA-pv6w)
- test: place the mocked export in its own mkdtemp dir. The route
recursively deletes path.dirname(filePath) after download, so a stub
in bare os.tmpdir() made the super_admin test wipe the whole temp
root — other jest workers' DB files included (latent CI flake).
- ui: hide PicpeakExportCard from non-super_admins. The role keeps
settings.view + backup.create, so after the gate its Download button
always 403'd with a generic toast; gate the card on role super_admin
to match the endpoint.
* fix(security): keep the token-mismatch audit values within varchar(20) (GHSA-g94x review)
image_access_logs.access_type is varchar(20) (migration 038), but
'token_gallery_mismatch' is 22 chars — on Postgres the audit write
threw value-too-long and logImageAccess swallowed it, so the security
event went unrecorded (the 403 still fired; log is best-effort). Shorten
to 'photo_mismatch' / 'gallery_mismatch' (14/16).
---------
Co-authored-by: Paul Nothaft <[email protected]>
* fix(admin): own-property lookup in the extension MIME map (#908 review round)
A client-controlled filename ending in .constructor / .__proto__ /
.toString made EXTENSION_TO_MIME[ext] return an inherited Object.prototype
member (truthy), and the downstream extMime.startsWith threw —
a permanent 500 on the admin view for that photo instead of the JPEG /
mp4 fallback. hasOwnProperty-gated now; test pins both a .constructor
image and a .__proto__ video.
* fix(admin): drop already-expired events from the dashboard card (#909 review round)
The expiring-soon card ran Math.max(1, ceil(delta)), so an event that
expired while the dashboard sat open (its query isn't polled) showed
'1 day left' indefinitely from the stale cached row. Expired rows are
now filtered out before render; the delta is therefore always positive
and the clamp is gone.
* fix(admin): honor safe stored image MIME for auto-imported formats (#908 review round 2)
My previous round made the image side map-only to dodge the migration
039 image/jpeg backfill and image/svg+xml — but that regressed the S3
auto-importer (STORAGE_AUTO_IMPORT), which stores correct types for
avif/bmp/tiff/heic whose extensions aren't in EXTENSION_TO_MIME. Those
now served as image/jpeg (JPEG-labelled non-JPEG bytes).
Precedence is now mapped-extension (still corrects the 039 backfill on
PNGs) -> stored MIME IF in a safe raster allowlist (avif/bmp/tiff/heic
+ the mapped ones) -> image/jpeg. Allowlist, not a regex: image/svg+xml
stays excluded (scriptable inline). Tests pin avif preserved and svg
degraded to jpeg.
* fix(admin): refresh expiry status live at the boundary (#909 review round 2)
Two review findings on the admin expiry surfaces:
- The dashboard 'expiring soon' card, list badges, and detail banner are
all computed inline from Date.now() at render, so a page left open
across an event's expiry kept showing 'active'/'1 day left' until an
unrelated render — which for editor/viewer roles (no health poll)
never happens.
- My round-1 client-side filter on the dashboard desynced the visible
list from the cached total/stat ('no events expiring' beside 'view
all N').
Both are fixed by new useExpiryRefresh: it fires once at the soonest
future expiry (setTimeout, overflow-guarded). The dashboard refetches
its expiring + stats queries — the backend already excludes expired
events, so rows/total/stats come back consistent (filter removed). The
list and detail pages bump a tick so the inline badges recompute. Hooks
are placed above the loading early-returns (rules-of-hooks is disabled
in eslint, so this was a latent crash otherwise).
* fix(admin): allow any header-safe raster MIME, deny svg/xml (#908 review round 3)
The round-2 hand-listed Set kept missing formats the S3 auto-importer
stores (apng/ico/jxl beyond avif/bmp/tiff). Replace it with a regex:
honor image/<token> EXCEPT the scriptable svg / *+xml family. Covers
every current and future raster type in one rule while still blocking
inline-scriptable svg and header injection. Tests pin apng + x-icon
preserved, svg still degraded to jpeg.
* fix(admin): expiry-refresh precision + filtered refetch (#909 review round 3)
Three refinements to round-2's live-expiry work:
- useExpiryRefresh now re-arms past setTimeout's ~24.8-day overflow
limit (capped wake-up that re-evaluates) instead of dropping the timer,
so a page mounted for weeks still updates.
- The dashboard requests the expiring list ordered by expires_at asc, so
the five shown rows ARE the soonest to expire — the timer schedules
against the true next boundary even when >5 events are expiring
(getEvents gains optional sortBy/sortOrder; backend already whitelists
expires_at).
- EventsListPage refetches instead of only re-rendering at the boundary:
under the 'expiring' filter the backend drops expired rows, so a plain
tick would leave a stale 'Expired' row + total. refetch keeps rows and
totals correct under every filter.
---------
Co-authored-by: Paul Nothaft <[email protected]>
