fix(gallery): keep an admin draft preview out of the guest share-login flow
Making verify-token pass for a draft preview opened a path that did not exist
before it: the gallery bootstrap then called shareLinkLogin, which refuses a
draft AND records a failed login attempt against the caller's IP while doing
it. Five preview opens inside the attempt window therefore locked share-link
logins out for that IP — including for real guests, and including after the
gallery was published.
An admin preview does not need a guest session at all. The admin cookie plus
admin_preview=1 already authorizes every gallery call, which is exactly how
preview works on a published gallery, so the preview path loads the gallery
directly and never touches the login endpoint.
Deliberately not fixed by relaxing shareLinkLogin's draft check: that endpoint
mints a guest token, and a draft should not be handing those out.
Relates to issue 1386
fix(gallery): let an admin preview a draft through its short share URL
/info has honoured admin_preview since issue 868, but two sibling routes on the
short-URL path never did:
- GET /resolve/:identifier filtered drafts out through ACTIVE_EVENT_FILTER
(shareLinkService.js), with no escape for a verified admin.
- GET /:slug/verify-token/:token repeated the same filter inline, so clearing
the first would only have moved the 404 one step later.
With "use short gallery URLs" off the View Gallery link carries the slug,
GalleryPage never calls /resolve, and the preview worked. With it on the link
is the token form, GalleryPage resolves it first, and the draft answered
"Gallery Not Found".
resolveShareIdentifier takes an includeDrafts option, and /resolve reaches for
it only after the published lookup misses AND verifyAdminPreview accepts the
caller — so the published path keeps its single query and an unverified caller
never learns the draft exists. The frontend already sends admin_preview=1
(EventDetailsHeader.tsx:203, forwarded by config/api.ts:81); only the backend
had to change.
GHSA-rh8r's rule is unchanged and now pinned by test: a bare slug lookup still
never returns share_token, draft or not.
Relates to issue 1386
Once an event left `standard` protection, both halves of the video path were
routed through /api/secure-images, and neither half can carry a video.
galleryQueryService emitted the secure template as a video's `url`. The
lightbox drops that straight into a <video> element; nothing substitutes the
`{{token}}` placeholder (the helper that could, secureToken.service.ts, has no
importers), so the request answered 403 "Invalid or expired token". Even with a
valid token it would still have failed — the secure-images route pipes every
byte through secureImageService.processProtectedImage, which calls sharp() and
throws on an mp4. routes/gallery/media.js bounced the JWT route to that same
endpoint before reaching its own video branch, so there was no way through.
Videos now keep the JWT route at every protection level, on both sides. That is
not a new exposure: thumbnails of those same videos have always been served
from it, and a valid gallery token is still required to reach it. Still images
are unaffected and keep bouncing to the secure endpoint.
VideoPlayer had no `error` listener, so all of this rendered as a poster frozen
at "0:00 / 0:00" behind a play button that did nothing — indistinguishable from
a codec the browser cannot decode, which is the other common cause (HEVC/H.265
phone footage plays in Safari and nowhere else). It now surfaces the failure
and names the codec case, since the answer there is to download the file.
Relates to issue 1370
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
The restore flow accepted an unvalidated database.backup_file from
the manifest (absolute paths and traversal both worked, and no
containment check enforced the configured backup root), then
interpolated it unescaped into a `sqlite3 .restore '<path>'` command,
letting an attacker-chosen source file replace the live database.
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* fix(backend): validate the S3 endpoint host before the restore download
downloadFileFromS3() built an S3StorageAdapter and called .download()
directly, skipping the isHostAllowed() private-IP/DNS-rebinding guard
that testConnection() applies elsewhere — an admin with backup.restore
could point the configured S3 endpoint at an internal/metadata address
for unauthenticated egress via the server.
* fix(backend): pin the restore S3 download to its validated DNS resolution
isHostAllowed() was check-then-connect: the AWS SDK re-resolves the
endpoint hostname independently when it actually connects, so a DNS
rebinding condition between the preflight check and the real
connection could still reach a private/internal address. Reuses the
same pinnedRequestOptions() primitive webhookDeliveryWorker.js already
uses, wired into the S3Client's requestHandler.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
Admin password reset generated a ~2^21-entropy password from a small
wordlist instead of the already-available generateSecurePassword(16),
and must_change_password was written on reset but never checked by
any route-blocking logic — a reset user could keep using the old
session/password indefinitely.
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* fix(backend): require actor to hold every permission of a role they grant
Any admin with `users.edit` could grant an arbitrary non-super_admin
role — including one carrying far more permissions than they
themselves hold — via PUT /api/admin/users/:id. The role-change path
never called the existing assertActorMayGrant() guard that already
protects role create/edit.
* fix(backend): apply the same role-grant guard to admin invitations
createInvitation() only blocked granting super_admin — the same
users.create-holder-can-invite-into-any-role escalation that
updateAdminUser() was fixed for (GHSA-rv8w-m6mx-7j4q) was still open
via POST /admin/users/invite. Reuses assertActorMayGrant().
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* fix(backend): validate event id before using it in the logo storage filename
The multer filename callback built the stored path directly from
req.params.id with no integer validation, letting a traversal payload
in the route param escape the intended uploads/logos/events/
directory — most directly reachable via a super_admin session, since
requireEventOwnership short-circuits with no DB lookup for that role.
* fix(backend): validate contract id before using it in the signed-PDF storage filename
Same pattern as the event-logo fix (GHSA-9q5j-vqfw-32hr) in a
different file adminContracts.js never touched: multer's filename
callback ran before express-validator's :id check, letting a
traversal payload escape uploads/contracts/signed/.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* fix(backend): validate business-profile logo uploads by content, not filename
The upload route skipped the shared validateFileType() helper every
sibling upload route uses, and derived the stored extension from the
client-supplied filename. A file could declare an image MIME type
while carrying an executable/HTML extension and arbitrary content,
then be served same-origin via the mass-assignable logoPath field.
* fix(backend): content-sniff business-profile logo uploads too
fileFilter paired the claimed MIME type against the extension but
never verified the actual bytes matched, unlike other upload routes
that already call validateFileContent(). Defense-in-depth: the
extension-confusion XSS itself was already closed (stored extension
is derived from the validated MIME, not client input), this closes
the remaining gap where declared-vs-actual content can still diverge.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* fix(backend): reject a replayed TOTP code within its validity window
verifyTotp() was stateless — otplib's window:1 tolerance meant the
same 6-digit code could complete two independent logins inside its
~90s validity window. Track each admin's last-consumed step and
reject a code that doesn't advance past it.
* fix(backend): make the TOTP replay-tracking persist atomic
verifyTotpEncryptedStep() read two_factor_last_used_step, then a plain
UPDATE wrote the new step with no conditional guard — two concurrent
requests carrying the same captured code could both pass the check
before either UPDATE landed. The persist is now a conditional UPDATE
(only advances the step, checked via affected-row count), so a losing
concurrent request is correctly treated as a replay.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
The unauthenticated payment-check magic link (intentional, matches
publicQuotes.js) had a 30-day token lifetime and wrote to the invoice
ledger silently. Shortened the TTL and added a best-effort admin
notification on every write via this route, so the no-login
convenience stays but an admin always sees the action happen.
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
GET and POST for an event's short URLs both required
requireEventOwnership; DELETE only checked events.edit permission,
letting any admin holding that permission delete another tenant's
branded gallery short URL. Resolve the short URL's event first, then
apply the same ownership check the other routes use.
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* fix(backend): bump sharp, nodemailer, multer, js-yaml, joi for security fixes
Resolves 12 open code-scanning alerts (#589-600): sharp libheif RCE,
nodemailer address-parser ReDoS + domain-validation bypasses, multer
upload DoS/race conditions, js-yaml parsing DoS, and joi prototype
pollution. All patch/minor bumps within the currently used major
version.
* fix(backend): set multer's fieldArrayIndexLimit to actually close CVE-2026-82333
The advisory is explicit that the 2.3.0 version bump alone doesn't
remediate the array-index DoS — an app must also set
limits.fieldArrayIndexLimit. Set it on every multer instance, sized to
what each route's form actually needs.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* 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
* 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.
* 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.
* fix(video): throw when neither a real thumbnail nor the placeholder can be produced
processUploadedVideo returned success with thumbnailKey: null when
both the real thumbnail AND the SVG placeholder failed -- a total,
systemic failure (storage backend down, disk full), not a quirk of
one file. On stable, which doesn't have the #845 call-site fallback,
this silently completed the video with no thumbnail at all instead
of the retryable 'failed' status a throw here produces. On main,
the pre-existing #845 fallback already absorbed this exact case
(no behavior change there) -- verified against codex's own
git-blame check of the pre-PR stable code before applying this.
Now throws in that case, restoring the pre-existing "let the caller
mark it failed and retryable" behavior for a genuinely unrecoverable
video, while keeping every partial-failure case (the vast majority)
resolving with whatever succeeded.
Found by codex review.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* 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>
* fix(usage): explain and de-emphasize the pending-packet button lock
An admin whose report delivery is stuck (old schema, network issue,
etc.) saw the v5-upgrade and portal buttons greyed out with no
indication why, or what to do about it — the backend's single-packet-
in-flight guard is correct, but silent. Add an inline note pointing at
"Retry / send if due" when a pending packet is the actual cause.
Also: "Review expanded usage.v5 scope" didn't read as an upgrade
action — renamed to "Upgrade to usage.v5" / "Auf usage.v5 upgraden".
"Open usage portal" is now a primary (green) button in both its
signed-in and pre-participation forms, matching the visual weight of
the other primary actions on this tab instead of blending in as a
secondary outline button.
* fix(usage): scope the pending-packet note to controls it actually gates
The note added in the previous commit rendered whenever pending_action
was truthy, regardless of participation status. Outside `active`
(activation_pending, deletion_pending) the portal renders as a plain
un-gated link and no v5-upgrade section exists at all, so the note
named two controls that either weren't blocked or weren't on screen.
And when active but already on the current schema, it wrongly implied
a v5-upgrade button existed.
Gate on `active` (nothing is actually blocked outside it), and choose
between the existing two-control message and a new portal-only one
based on whether consent_update_available — which the v5-upgrade
section itself is gated on — is true. Four new regression tests cover
each shape: activation_pending, deletion_pending, active+current-schema
(portal-only), and active+outdated-schema (both, the original case).
Found by code review.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
#1359 independently added 211_revocations_without_expiry.js against
the same main baseline. Renumbering this one to 212 keeps the
migrations directory sequentially numbered once both land, regardless
of merge order. No functional change — same up()/down(), same column.
Found testing against WebKit at an iPhone-sized viewport: the fixed
overlay + flex-center wrapper has no height limit, so on a short
screen the title is clipped off the top and the "Maybe later" button
off the bottom, with no way to reach either. Capping the card at
max-h-[90vh] with its own overflow-y-auto keeps the backdrop static
and makes the card scroll internally instead.
An admin who already had PicPeak installed before the opt-in reporting
feature existed never gets asked — the setup wizard only runs once, on
a brand-new instance. Adds a one-time modal, shown on the admin's next
dashboard visit after updating, offering the same choice the wizard
gives a new install.
- New `product_usage_state.prompt_shown` column (migration 211) and
UsageService.markPromptShown(), set on either outcome (enable or
decline) from both this modal and the wizard step, so an
installation is never asked twice regardless of which path it took.
- New POST /admin/usage/prompt-seen endpoint.
- Extracted the wizard's three-point pitch (UsageReportingPitch.tsx)
so the modal and the wizard step share identical copy instead of
drifting apart.
- The modal never shows once participation is already active, and
never shows a second time after either the wizard or the modal has
been through it once.
Depends on #1360 (the setup wizard step this reuses).
Adds a step between the SMTP/site-URL config step and the final
thank-you screen, asking whether to participate in the existing
Product usage & feedback reporting. Kept short — three points on how
it differs from typical telemetry (one-way only, no personal data,
and participants can browse the same shared feature-adoption dataset
other installations report) plus an explicit consent checkbox — and
calls the same enable() endpoint the post-install Settings page uses.
Skipping does nothing; participation can be toggled anytime from
Settings → Product usage.
Harden gallery authentication and authorization, consolidate gallery workflows, and prevent token-bearing URLs from leaking through nginx request error logs.
generatePreviewImage rewrites the output extension to match the encoding
it chose, .jpg or .webp for alpha and multi-frame sources. The tier
lookup in ensurePreviewImageAtWidth and the cleanup list in
previewTierKeys kept the SOURCE extension instead, so for anything but a
lowercase .jpg source the stat never matched: every tier request for a
.png, .JPG, .jpeg, .heic or RAW photo re-ran Sharp, and cleanup never
found the files it left behind, which accumulated for the life of the
install.
Both now derive every key the tier can live under: the .jpg and .webp
candidates, plus the source-extension key last so tiers written before
the rewrite are still found by lookup and by cleanup.
Follow-up to issue 1020, where the mismatch was identified during review.
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* fix(images): single-flight lazy rendition generation and keep the old rendition during replacement
The lazy generators in imageProcessor are check-then-generate, and the
check reads the path off the photo row the route already fetched. N
concurrent cold requests for one photo all held a snapshot with the path
still null, all missed, and all ran the same sharp pipeline. Only the
thumbnail tier path had a guard; the canonical thumbnail it falls back
to, heroes, previews and preview tiers had none.
One process-local map now covers every rendition, keyed by photo id and
rendition (`thumbnail:<id>`, `thumbnail:<id>:w<width>`, `hero:<id>`,
`preview:<id>`, `preview:<id>:w<width>`). Concurrent callers share one
promise; the entry is cleared in a finally on success and failure alike
so a rejection cannot poison the key. The tier stat moved inside the
flight so a request arriving as the previous flight clears finds the
written tier instead of missing on a stale probe.
Heroes and previews also deleted the existing object before generating
its replacement, and again in the catch. Both are gone, mirroring what
the thumbnail generator already does: put is the last statement in the
try and replaces atomically on local storage and by key on S3, so the
delete only ever opened a window with no rendition at all, and a source
that failed to read stripped the old rendition with the row still
pointing at it.
No re-read of the photo row inside the flight: the admin regenerate
endpoints force a rebuild by passing a row with the path nulled, and a
re-read would hand back the persisted rendition untouched.
Fixes the single-flight half of issue 1020. The preview cache-key
extension mismatch and any server-wide work queue remain separate.
* fix(images): keep the snapshot validity check outside the single-flight
With the check inside the flight, an admin regeneration (row passed with
the path nulled) could join a viewer's flight for the same photo that was
merely confirming an already-good rendition, and be handed back the very
file it was asked to replace while the endpoint counted a success. Only a
miss enters the flight now; inside it everything is a regeneration.
* fix(images): forced rebuilds run after an in-flight lazy generation instead of adopting it
The admin regenerate endpoints could still join a lazy flight that was
already generating for the same photo. That flight read the thumbnail
settings when it started, so after a settings change it produces exactly
the rendition the regenerate was invoked to replace; adopting it counted
a success while the old size stayed cached.
ensureThumbnail and ensurePreviewImage take `{ force: true }`: skip the
snapshot check and, if a flight is pending, start after it settles. Lazy
misses arriving meanwhile join the forced flight, and an older flight
settling late no longer evicts the newer entry from the map.
* fix(images): key rendition flights by source as well as photo id
replacePhoto keeps the photo id and changes path and filename. Keyed by
id and width alone, a request carrying the replacement row joined a
flight still rendering the file it replaced and was handed the old
image, which the gallery caches for 30 minutes. The tier map this
replaced was keyed by storage key and so already told the two apart.
* test(thumbnails): wait for the regenerate loop's completion line instead of a fixed 150 ms
The loop runs in setImmediate after the response. Under a loaded machine
(fifteen suites in parallel, each booting a migrated SQLite) it took
longer than 150 ms once and the assertions ran against a half-finished
mock call list. Poll the logger spy for the "regeneration complete" line
with a 10 s deadline; the suite also finishes sooner because the wait
ends as soon as the loop does.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
PUT /admin/events/:id spreads the body into the UPDATE. SQLite resolves
quoted identifiers case-insensitively, so `{ "Event_Name": ... }` lands
on event_name there — while every check in the handler (validators, the
field-level permission guards, the deny-set) keys on the exact lowercase
name. The deny-set already case-folded for its own columns; every other
column was reachable through a spelling variant.
Every events column and every input-only key the handler accepts is
lowercase snake_case, so a key with any uppercase in it is not something
a legitimate client sends. Such keys are now removed before anything
looks at the body. Postgres was unaffected (quoted identifiers are
case-sensitive there; a variant produced a 500 instead).
Surfaced by the Codex review of the folder-watcher change, where a
photos.upload guard on external_watch could be walked around this way.
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* feat(external-media): watch reference folders and import new files automatically
Managed uploads dropped into storage/events/active are picked up by the
chokidar watcher; external media had no equivalent, so a NAS folder that
keeps growing needed an admin to open the event and press Import every
time. Relates to issue 1187.
- The import pass moves out of the route into
services/externalImportService.js. The watcher and the Import button
now run the identical function; the route only validates and maps
errors to status codes.
- Mutual exclusion is the per-event claim from maintenanceJobState
(`external_import:<id>`, seeded on demand by the new ensure()) instead
of the in-process Set. The Set stopped a double-click in one process;
the claim also stops the watcher on a second replica, or an admin
clicking while the watcher is mid-run elsewhere. The run heartbeats so
a claim from a dead process is taken over.
- services/externalMediaWatcher.js: per-event opt-in via the new
events.external_watch column (migration 208), chokidar with
awaitWriteFinish so a copy in flight is not imported half-written,
debounced full pass per change, a timer sweep every 15 minutes as the
fallback for NFS/SMB mounts that deliver no inotify events, optional
stat-polling via EXTERNAL_MEDIA_WATCH_POLLING. The set of watched
events is re-read every minute, so the toggle works from any replica.
A watcher that just started runs one pass immediately.
- Deletions are ignored on purpose: a file vanishing from a NAS is at
least as likely to be a reorganisation or a dropped mount as an
intentional removal, and acting on it would delete a guest-visible
photo. Rows whose file is gone stay, as they do today.
- Not gated on STORAGE_BACKEND: EXTERNAL_MEDIA_ROOT is always local.
- Quiet system passes stay out of the activity log; runs that imported
something are logged with actor external-media-watcher.
- Frontend: "Watch folder for new files" checkbox under the external
folder picker, status line in view mode, EN/DE strings.
* fix(external-media): close the review gaps in the folder watcher
Codex review of the watcher, round 1. All six findings were real:
- Enabling the watcher, or pointing an enabled one at another folder,
now requires photos.upload — the permission the manual Import already
requires. events.edit alone was a way around it. Only the transition
is checked, so a role without photos.upload can still edit an
already-watched event. The checkbox is disabled for such roles.
- Automatic passes defer files that are still changing: anything
modified inside the stability window, or whose size moves across one
wait of that window, is left for the next pass. chokidar's
awaitWriteFinish only settles the file that fired the event, and the
sweep sees no events at all, so a sibling still being copied could be
inserted half-written and then skipped forever.
- Photos an admin deleted are not brought back by the sweep. The delete
routes record the file in external_import_exclusions (migration 209);
automatic passes skip the list, the manual Import ignores it and
clears it for what it imports.
- The six EXTERNAL_MEDIA_WATCH* variables are forwarded in all three
compose files; they were documented but the backend services use
explicit environment lists, so the kill switch did nothing.
- A pass re-checks is_active / is_archived at run time, not only in the
minutely reconcile.
- The lease is renewed on a timer for the whole run, walk included, and
ownership is checked before the event row is touched.
* fix(external-media): make automatic passes follow the row, not rewrite it
Codex review round 2, four findings, all applied:
- The event update route drops non-canonical spellings of external_watch
and external_path before the permission guard. SQLite resolves column
names case-insensitively, so `External_Watch` reached the column while
the guard only looked at the lowercase key.
- Exclusions are checked per file at insert time, not against a
snapshot taken before the settle wait. A photo deleted during the wait
was present in the snapshot and got re-inserted by the loop.
- An automatic pass no longer writes source_mode / external_path. It
re-reads the row after the walk and the settle wait and stops if the
folder changed or the event went managed; the manual Import is the
only writer. The options are now `automatic` + `settleMs`.
- A pass that deferred files re-arms the debounced import, so a file
copied just before the watcher started is not stranded when the sweep
is disabled.
* fix(external-media): keep exclusions for replaced photos, stop a pass whose event stopped qualifying
Codex review round 3, both findings applied:
- recordExclusions keys on external_relpath alone. A replaced external
photo becomes managed but keeps its relpath on purpose, and deleting
that replacement must not republish the NAS original.
- An automatic pass checks the full watcher predicate (reference mode,
same folder, watch on, active, not archived) before it inserts and on
every heartbeat tick during the loop, and stops as soon as the event
no longer qualifies.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
On the CMS page the editor has no bounded height, so a long document
scrolls the whole admin content area and the toolbar scrolled away with
it. Editing a 16-section privacy policy meant scrolling back to the top
for every heading or list.
The top toolbar block (mode/save row and formatting row) is now
`sticky top-0` from the md breakpoint up, pinned to the admin page's
scroller. The rounded wrapper clips with `overflow-clip` instead of
`overflow-hidden`, because hidden turns the wrapper into a scroll
container and the toolbar would pin to that instead of to the page. Not
below md: there the formatting row wraps to several lines and a
permanently stuck block would eat most of a phone's editing area.
Two things follow from pinning. The link-entry row moves inside the
sticky block: rendered below it, the URL field sat at the toolbar's
original document position, under the pinned toolbar. And ProseMirror's
selection scrolling gets a top threshold and margin sized from the
block's rendered height (ResizeObserver, re-applied through
editor.setOptions), because the formatting row wraps to two rows at
common desktop widths and the link row comes and goes; a constant would
leave the caret behind the toolbar half the time. Below md the offsets
are zero again.
Verified in Chromium against the CMS page with an 18-section document:
scrolled to the last sections, the toolbar stays at the top of the
content area; on main it is gone. A source-level test pins the sticky
block, the wrapper's clip, the link row's placement and the measured
offsets, since jsdom does not lay out.
Relates to issue 1289
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* feat(security): opt-in recoverable gallery passwords
Gallery passwords are bcrypt hashes, so an admin who needs to hand a
password to a client a second time has to reset it, which invalidates
what the client already has. This adds a security setting,
security_gallery_password_recoverable, off by default, that keeps an
AES-256-GCM encrypted copy of each gallery password and client PIN next
to the hash. The key is derived from GALLERY_PASSWORD_ENCRYPTION_KEY or
JWT_SECRET.
While the setting is on:
- create, publish, send-later, edit, reset and the v1 API write the copy
alongside the hash; turning a gallery's password requirement off
clears it
- GET /api/admin/events/:id/password returns the copy to admins with
events.edit and ownership, and writes a gallery_password_viewed
activity entry on every real reveal
- resend-email uses the stored password instead of the "set at creation"
sentinel, so the client receives what already works
Switching the setting off purges every stored copy. Login and hash
verification are untouched; the copy is never read on the gallery side.
The Security tab carries the toggle with a warning that stays visible,
and the event page shows "Show password" with copy buttons only while
the setting is on and the gallery has a secret.
Relates to issue 1271
* fix(security): close the write-versus-switch-off race in the password vault
The recoverable setting is read while an event insert is assembled and the
client-PIN hash awaits after that, so a settings request that switched the
feature off and purged in that gap was overtaken by the insert. Every write
site now re-reads the setting right after its statement and clears its own
row when the setting is off; the settings writer flips the value before it
purges, so either the purge or the re-check catches the row.
* fix(security): resend carries the stored client PIN and link; deterministic tamper test
The creation mail includes the client-access link and PIN; a resend only
sent the gallery password even when a stored PIN was available. The
ciphertext tamper assertion replaced the last two characters with a
constant, which was a no-op roughly once in 4096 runs.
* fix(security): drop the revealed password after Send gallery email
The send-later route can replace the password; the share card keys its
revealed copy on the event query's refetch time, so invalidate the event
after the send like the other password-changing mutations do.
* fix(security): purge leftovers before the setting write when turning recovery on
Switching on wrote the setting first and purged after, so a password write
that read the new "on" in between stored a copy the purge then deleted.
Turning on now purges before the write; turning off keeps purging after it,
which together with the write-site re-check leaves the vault holding
exactly what was written while the setting was on.
* chore(security): drop the duplicate rateLimitService import left by the rebase
* chore(usage): register the password recovery routes in the v5 coverage inventory
The inventory moved from v4 to v5 on main; the entry added by this branch
followed it.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
* feat(usage): distinguish real edits and template delivery with v5 consent
* fix(usage): exclude queued test messages and count reorders as edits
- queueEmail carries usageEligible: false into email_data and the queue
processor passes it on, so the dev tools' send-test-email no longer
records email_template_delivery once the worker sends it.
- event-types/reorder and categories/reorder-global compare the persisted
order before and after and record the v5 edit markers only when it
changed, matching the display_order edit already counted on PUT.
- normalized() builds arrays with Array.from so a row array from the sqlite
binding compares equal under Jest's separate realm.
* fix(usage): cover per-gallery category order and workflow test runs
- categories/reorder records category_editing when an event's override
changes; reorder/:eventId records it when an override was actually
removed.
- send_email and the collections handoff pass usageEligible: false for a
workflow test run (engine.testRun sets __test), so a non-dry test send is
not counted as template delivery.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
Paul Nothaft