101 Commits

Include renames

Author	SHA1	Message	Date
Paul NothaftandPaul Nothaft	b69dd134d0	feat(faces): People in this gallery — face recognition via an optional ML sidecar (#1074) (#1075) ... * feat(ml): optional face-detection sidecar, opt-in and inert by default (#1074) First of four PRs for "People in this gallery". This one ships only the sidecar, its wiring and its CI — no schema, no backend code, no UI. Nothing in PicPeak calls it yet. picpeak-ml is a single FastAPI + onnxruntime container: three endpoints (/health, /info, /faces), no database, no volumes, no egress, no model download at runtime. Clustering, person identity and every privacy decision stay in the backend where the data already lives. Models are YuNet (detection) + FaceNet-512 (embedding), both MIT, both pinned by URL and SHA-256 and verified at build time. The licence analysis is in ml/LICENSES.md: the more accurate InsightFace weights are non-commercial-only and PicPeak's users are working photographers, so they are never baked into an image we publish. Two things worth review attention: - Alignment uses a least-squares similarity transform (Umeyama), NOT cv2.estimateAffinePartial2D. RANSAC and LMEDS exist to reject outliers among many correspondences; given five landmarks and no outliers they fit a three-point subset exactly and let the rest drift. Measured on a real off-frontal portrait: eyes and nose pinned to 0.11px, mouth corners 11.8px out on a 160px crop. Umeyama distributes it (max 6.5px, rms 5.1 vs 7.4). The failure mode is silent — a bad warp still yields 512 confident floats — so tests/test_pipeline.py pins it numerically. - FACENET_ONNX_URL has no default and the build fails loudly without it. deepface distributes FaceNet-512 as Keras .h5 only, so the ONNX is produced once by tools/convert_facenet.py and published as a release asset. Converting inside the build would drag TensorFlow through both architecture legs of every build to produce a byte-identical file. The CI jobs are gated on the FACENET_ONNX_URL repository variable and skip cleanly until it is set. Off by default, twice over: the sidecar is behind the `faces` compose profile, and the backend will gate on a `faces` feature flag that defaults to false. FACE_ML_URL defaults to http://picpeak-ml:8000 so the standard deployment needs no configuration — nothing dials that host while the flag is off, which is why a non-resolving default is harmless. Verified: 27 pytest tests green; YuNet loads and detects against a real portrait with its landmark order matching the alignment template index-for-index; both compose files validate and the faces profile is correctly excluded from a default `up`; workflow YAML parses and the job graph resolves. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * fix(ml): pin the converter toolchain, verify parity, drop a false reproducibility claim (#1074) Ran the FaceNet-512 conversion for real and corrected what the previous commit assumed about it. The conversion works: 23,497,424 parameters, 89.6 MB ONNX, and the converted graph matches the Keras original to 2.086e-06 absolute / cosine 1.0000000000. That check is now part of the script rather than something I did once by hand — a subtly wrong graph still returns 512 plausible floats, so it refuses to leave the file on disk if parity fails. Also ran the full pipeline against both real models end to end. The embedding is L2-normalized to 1.000000, and the same face survives being re-rendered: half scale 0.973, double scale 0.984, JPEG q40 0.987, rotated 8 degrees 0.984, brightness +40 0.988. Scale invariance in particular is evidence the alignment warp is doing its job. Corrected claim: the conversion is NOT byte-reproducible. Two runs with the same pinned versions on the same machine gave different SHA-256s. The graphs are functionally identical — same 336 nodes, same 271 initializers, every weight matching to 0.000e+00 — but a few initializer names differ because tf2onnx's traced-op naming is not deterministic (Keras layer naming is deterministic; I checked). The previous commit message and README both claimed byte-identical output. They were wrong, and it matters: anyone re-running the conversion gets a different hash, and without this note that reads like tampering. The build-time SHA-256 pins one published artifact so its URL cannot start serving different bytes; validating a fresh conversion is the parity check's job. requirements-convert.txt now pins the exact set that produced the artifact, including transitive keras/protobuf/numpy, and documents that the converter needs Python 3.11 while the image runs 3.12. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * feat(faces): schema, queue, clustering and API for People in this gallery (#1074) Backend half of the feature. Migration 177, a face-detection queue, the clustering engine, the gallery and admin APIs, and the privacy wiring. No UI yet; nothing is reachable until the `faces` feature flag is on, which defaults to false. The flag is the gate, not FACE_ML_URL. That variable now has a working default (the compose service name), so its presence proves nothing about intent — if it were the gate, every install would poll a hostname that does not resolve. faceQueue re-checks the flag every tick, so turning it off stops the workers without a restart. Visibility scoping is the part worth reviewing closely. Face rows have no concept of photo visibility, but guests are restricted to photos.visibility='visible'. A raw count leaks how many hidden photos someone appears in, and an unscoped cover face renders a crop of a photo the guest may not open — with the best-scoring face being the likeliest pick, so it would happen often rather than rarely. facePeopleService recomputes both per request against the caller's own scope, and event_people.face_count_total is named to be conspicuous in a guest path. Six tests cover it, including the case where a person's photos are ALL hidden and they must vanish entirely. Face data is excluded from backups and .picpeak exports, per the decision in the thread: it is derived, so a restore re-scans rather than carrying biometrics between operators. Three separate mechanisms, because the engines cannot be filtered alike — EXCLUDED_TABLES for export, --exclude-table-data (not --exclude-table; the CREATE TABLE must survive or restore breaks on the first query) for Postgres, and DELETE + VACUUM on the temp copy for SQLite, which has no way to exclude a table from a whole-file .backup. The VACUUM is not cosmetic: without it the pages stay in the file and the claim is false on disk. Archiving now purges face data explicitly. photo_faces cascades off photos, but archive deletes neither the photo rows nor the event, so without this an archived gallery kept its biometrics indefinitely. Other decisions: clustering keeps names across a re-cluster by majority inheritance (without it, one button click silently discards every name the photographer typed); consolidation refuses to merge two people who were named differently; assignment never compares across model_version, since embeddings from two pipelines are not comparable; low-quality faces are stored but left unassigned so they show in "this photo contains" without spawning junk people. Migration is 177, not 174 — 174/175/176 landed on main while this branch was open. 29 tests green: 7 migration (idempotency, down(), cascade, and that installing it enqueues NOTHING), 11 clustering, 11 privacy/visibility. Lint clean; the pre-existing error counts in databaseBackup.js and server.js are unchanged. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * feat(faces): People strip, face filter and admin controls (#1074) Frontend half. Renders nothing anywhere unless the `faces` feature flag is on AND the photographer enabled detection for the gallery — the whole guest surface hangs off one boolean, event.people_enabled, which the server computes from the flag, the per-event toggle and the show-to-guests toggle together. Guest side: a People strip between the filter bar and the grid, circular crops from each person's cover face, an active-filter chip row, and a "Show all" bottom sheet. The face filter composes with category, search, media type and the liked/saved/rated filters in the same useMemo rather than replacing them, so "photos of Anna that I liked" works. Two people selected means AND by default — that is what picking a second face almost always asks for — with a toggle to OR that appears only once a second person is picked. Unnamed people show a photo count and never "Person 7". A number is honest about what the system knows; an invented name is not. There is a test asserting we don't do it. The strip renders nothing below two people, collapses to one line when dismissed (persisted per slug, so dismissing one gallery says nothing about the next), and appears mid-backfill with a progress line rather than blocking the gallery behind a spinner. Avatar crops are computed in ratios of the source dimensions so they survive whatever rendition the browser gets; without width/height they fall back to an uncropped thumbnail, since a wrongly-offset crop is worse than no crop. No new download endpoint: "download these N" rides the existing photoIds path, which already enforces access level and per-category permissions server-side. Adding a person_id selector would have been a second thing to authorize for no gain. Guest-facing copy never says "biometric" or "recognition" — those words describe our implementation, not the guest's experience. The sheet's footnote answers the first question every guest has (where does this go?) inline. The admin card, by contrast, is explicit: it states the controller obligation next to the toggle, and warns that scanning materializes the preview tier on galleries that never generated one, which is real CPU and disk an admin should know about before a 2,000-photo backfill. EN + DE translations. 140 frontend tests green (8 new), tsc and eslint clean. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * fix(faces): measured match threshold, working build defaults, 89MB smaller image (#1074) Ran the Phase 0 spike that had been outstanding, published the model, and fixed what both turned up. THRESHOLD IS NOW MEASURED, NOT GUESSED. LFW's standard 1000-pair protocol run through this exact pipeline (YuNet -> Umeyama alignment -> FaceNet-512 ONNX), 100% detection on 2000 images: same person cosine 0.6958 +/- 0.1415 diff person cosine 0.0849 +/- 0.1674 separation 0.6109 peak accuracy 96.60% @ 0.405 So the pipeline separates people well — the thing I could not previously claim, since every earlier number was the same face re-rendered. Default moves 0.62 -> 0.50. The old value was a placeholder and a bad one: it gave 0% false merges but 22.4% false splits, i.e. roughly one in four same-person pairs failing to join, which fragments a gallery badly. 0.50 gives 1.0% false merge / 8.2% false split. Peak accuracy (0.405) is deliberately NOT chosen: for clustering the two errors do not cost the same. A false split is a duplicate row the photographer can merge away; a false merge puts a stranger into someone's "download my photos" — and until the Phase 2 merge/split UI ships, there is no way to undo one. So this sits on the conservative side of the optimum. The spike is committed as ml/tools/benchmark_threshold.py rather than thrown away, so "why 0.50?" has an answer in six months and a re-tune is one command. BUILD DEFAULTS. FACENET_ONNX_URL/_SHA256 now default to the published ml-models-v1 release asset, so `docker build ml/` and `docker compose --profile faces up` work with no arguments. Blanking either still fails loudly — a URL without a checksum is never acceptable, since the checksum is what makes the URL safe to trust. Found by running compose for real: it failed exactly as designed, which was correct behaviour and a bad out-of-box experience now that a canonical artifact exists. IMAGE SIZE. 389MB -> 300MB single-arch. `chown -R` after COPY rewrote every copied file into a fresh layer, duplicating the 90MB model for nothing; the user is now created before the copies and ownership set via COPY --chown. Also drops pip/setuptools from the runtime image. Measured RSS is 186MiB idle, and the container answers /faces end-to-end in well under the 80-150ms/photo the issue budgeted. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * fix(faces): threshold 0.50 -> 0.60 from real clustering, theme-aware People strip (#1074) Both fixes come from running the feature on an actual gallery — 61 photos, 5 real identities — rather than reasoning about it. THRESHOLD. The LFW pairwise sweep in the previous commit said 0.50, and it was wrong. On a real gallery at 0.50, three of six visible clusters were contaminated: two different people merged into one strip entry, which is the exact failure that puts a stranger into someone's "download my photos". Pairwise error rates do not predict cluster purity. Greedy assignment compounds — one wrong face drags the centroid toward the midpoint between two identities, making the next wrong face likelier. A 1% pairwise false-merge rate is not a 1% chance of a clean gallery, and no amount of staring at an ROC curve would have shown that. Sweep against ground truth (5 identities): 0.50 -> 6 clusters, 3 contaminated 0.56 -> 6 clusters, 0 contaminated 0.60 -> 5 clusters, 0 contaminated <- exactly right 0.64 -> 5 clusters, 0 contaminated, fewer faces assigned 0.60 recovers the right number of people with no contamination; higher only loses coverage. Migration 177 carries the full reasoning so the next person to touch this knows why the obvious pairwise answer is the wrong one. THEME. The People strip hardcoded `text-neutral-800` for named people. On a dark gallery — which the screenshot immediately showed — that renders a named person's label almost invisibly, while UNNAMED people stayed legible. Exactly backwards. Labels, headings, the collapsed summary, the scan line and the filter chip row now read the gallery's own theme tokens (--color-text / --color-muted-text / --color-accent / --color-surface-border) like the rest of the gallery surface. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * fix(faces): keep the mobile filter row inside the viewport (#1074) At 390px the photo count and Clear link were pushed against the right edge by ml-auto and clipped. Only apply it from the sm breakpoint up, where there is room; below that they flow after the chips. Found by screenshotting the real thing on an iPhone-sized viewport. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * feat(faces): complete Phase 1, add People management and auto-categories (#1074) Closes the two Phase 1 gaps, then builds Phase 2 and Phase 3. PHASE 1 GAPS. "Download these N" was specified, described as done in an earlier summary, and never actually built — I had verified the backend needed no new endpoint and let that stand as if the button existed. It now hands the filtered photo ids to the same path as a manual selection, so the server re-applies access level and per-category permissions on the way through. Photos in a downloads-disabled category are excluded client-side too, so the number on the button is the number the guest receives. Hidden entirely when downloads are off for the gallery. Lightbox person chips ("In this photo: Anna") are the second way into the face filter — a guest looking at a photo of themselves can act on it without scrolling back to the strip. Tapping one closes the lightbox and filters the grid behind it. PHASE 2. A People management modal over the endpoints that already existed and were already tested: rename inline, merge (multi-select, first pick is the target so the name a photographer typed survives), split via a face picker, hide, ignore. This matters more than it sounds — clustering deliberately errs toward splitting because a wrong merge puts a stranger into someone's download, and that trade only works if merging is easy. PHASE 3. Rule engine over face_count plus face-area ratio: 0 -> Details, 1 large -> Portraits, 2-5 -> Small groups, >5 -> Groups. The area ratio is what separates "a portrait of someone" from "someone is in this landscape". Three guarantees, all tested: it only ever fills an EMPTY category (enforced in the query AND re-checked in the UPDATE, so a photographer setting one mid-run still wins), everything it touches is marked auto_categorized so undo is exact, and it is a no-op unless separately enabled. Migration 178 adds the column — separate from 177, which has already run wherever this branch is deployed. Verified on the real gallery: 61 photos -> 48 portraits + 13 small groups, undo cleared exactly 61 and left the manual ones alone. Merge moved faces and removed the source. Both confirmed against the database, not just the UI. TWO BUGS THE BROWSER CAUGHT, both invisible to tsc: - The lightbox destructure never landed — my patch targeted a line that has a default value, matched nothing, and failed silently. `people` resolved to something else entirely and the chips would never have rendered. eslint's "outer scope value" warning is what surfaced it. - Admin face thumbnails 403'd because <AuthenticatedImage> attaches whatever gallery token is in session storage; an admin who has also opened one of their own galleries sends a type:"gallery" bearer to an admin route. Admin routes authenticate from the httpOnly cookie, which a plain same-origin <img> sends by itself. Worth noting AdminPhotoGrid has the same latent shape; not touched here. Also: the admin card now reports "N people (M shown to guests)" when those differ, so the settings page and the gallery stop disagreeing without explanation. 45 backend tests (8 new) and 140 frontend tests green; tsc and eslint clean. EN + DE for every new string. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * perf(faces): batch migration DDL and drop the face stack from server.js import (#1074) CI's backend job timed out at 10 minutes on the first run of this branch. Nothing failed — 132 of 182 suites passed and the wall clock ran out. Main does the same 182 in 124s, and where main has 12 suites slow enough for jest to print a duration, this branch had 77. Two changes, both worth making regardless of how much of the gap they close: - Migration 177 added its columns one ALTER TABLE at a time (four on photos, three on events, plus a separate index statement) and seeded settings with a SELECT and an INSERT per key. It now uses one alterTable per table and one SELECT plus one bulk INSERT. 178 folds its index into the same statement as its column. That chain replays in ~90 suites, so statement count there is multiplied by 90. - server.js required faceQueue at module scope, which pulls in axios and — through imageProcessor — sharp. Every supertest suite that imports server.js was paying for a module graph it never uses. Now required inside the startup block, next to the call that needs it. Honest about the evidence: locally the migration delta measures at zero (1.15s vs 1.13s for the same suite, three runs each), so batching alone does not explain an eight-minute regression. A fast local disk and many cores mask per-statement and per-import costs that a two-core runner with a shared disk does not. These are the two real costs this branch added to a path that runs in almost every suite; whether they are sufficient is a question for CI, not for another round of local speculation. 37 face tests still green after the change. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * i18n(faces): complete EN and DE coverage for the face feature (#1074) The admin card and the Features toggle were rendering entirely from inline English `defaultValue` fallbacks — 22 keys existed in no locale file at all, so a German admin saw an English consent notice, English toggles and English buttons. The gallery side was already translated; the admin side was not, and nothing in the toolchain flags this because a `defaultValue` always renders something. Adds the missing `admin.faces.*` (19), `settings.features.faces.*` (2) and shared `common.clear/saved/saveFailed` in both languages. Existing keys are left alone (setdefault, not overwrite), so the shared `common` strings other features rely on are untouched. Committed the audit as frontend/scripts/i18n-faces-audit.py rather than throwing it away: it extracts every t() key the face components actually use and diffs it against each locale, and it also reports German values that are byte-identical to English, which is the usual shape of an untranslated copy-paste. Currently: 69 keys in use, EN complete, DE complete, no identical pairs. Verified in the browser, not just in the JSON — the German card reads "61 / 61 Fotos durchsucht · 16 Personen (5 für Gäste sichtbar)" end to end. Also checked the components for hardcoded user-facing text (JSX nodes, title/aria-label/placeholder attributes) outside t(); there is none. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * fix(faces): 13 defects from external review — coordinates, counts, erasure, races (#1074) Codex reviewed the branch against main. Thirteen findings, nine P1. I checked every one against the code and could not dismiss a single one as a false positive, so all thirteen are fixed here. THE WORST ONE: bounding boxes were stored in the wrong coordinate system. The sidecar reports coordinates in the space of the image it was HANDED — which is the ≤1920px preview, not the original — while every consumer compares them against photos.width/height, the original dimensions. A 6000px photo therefore produced boxes ~3x too small and areas ~9x too small: avatar crops landed in the wrong place and the Portraits rule could never fire. It is invisible on any photo already under 1920px, which is exactly why the demo gallery and every screenshot looked correct. Now scaled once in faceProcessor so everything downstream can assume original-image coordinates. ERASURE. The FK cascade on photo_faces is decorative on SQLite: PicPeak never enables `PRAGMA foreign_keys`, so deleting a photo left its embeddings behind. I first enabled the pragma globally and reverted it — six unrelated suites immediately failed on pre-existing dangling references, and switching it on would start rejecting inserts on every existing install. That is a real change worth making, but it is its own PR, not a rider on this one. Instead deletion purges explicitly: purgePhotoFaces in the photo paths (single, bulk, service) and photo_faces/event_people in deleteEventCascade. Tests assert this with the pragma explicitly OFF, so they can only pass if the code does the work. COUNTS. A re-scan deleted the old face rows without undoing their contribution to event_people, so counts inflated on every re-scan and ghost people survived. Now the affected people are recomputed before the replacements are assigned. My own "must not double its faces" test only checked photo_faces rows, which is why it passed throughout. RACES. A worker that finished after an admin purged the event committed its rows anyway — erasure reported success and the data reappeared. The commit is now conditional on the row still being 'processing'. And assignFaces is read-modify-write over an event's people, so two workers lost each other's updates; it is now serialised per event with an in-process mutex plus a Postgres advisory lock for the multi-pod case the queue advertises. METADATA LOSS. Merging discarded the source's name and suppression flags, so a merge could erase a typed name or un-hide someone. Reclustering remembered only people with a label, so an unnamed-but-hidden bystander came back guest-visible after one "Re-group people" — and suppression now propagates to every descendant cluster, not just the majority one. Also: export reset face_status so a restored gallery re-scans instead of claiming to be scanned forever; manual category edits clear auto_categorized so "undo automatic" cannot delete a photographer's own choice; external photos are skipped rather than failed (resolvePhotoStorageKey returns null for them by design); the gallery refetches photo memberships as a scan progresses so filtering is not stale; a failed VACUUM now fails the backup rather than publishing one that may retain biometric pages; and the ML Dockerfile's `|| true` is scoped to the uninstall — as written it was `(install && uninstall) || true`, so a failed dependency install produced a green layer and an image with no onnxruntime. Four new regression tests. Full backend suite failure set verified identical to origin/main; frontend 140 green; tsc and eslint clean. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * fix(faces): 12 more defects from review round 2 — cross-event purge, leaks, lifecycle (#1074) Second Codex round on the same diff, now including round 1's fixes. Twelve findings, seven P1. Again none were false positives. SECURITY, AND MINE FROM ROUND 1: the bulk-delete face purge iterated the raw `photoIds` from the request instead of the event-scoped `photos` rows the handler had already validated. purgePhotoFaces has no event scope of its own, so an editor could pass another gallery's photo id and delete its face data — even though the photo deletion right below it was correctly scoped. Fixing one thing and introducing another is exactly why the second round was worth running. ANOTHER VISIBILITY LEAK, same class as the one round 1 fixed: /people returns scan progress, and getScanStatus counted every photo with a face_status — including hidden ones. Guests could read the hidden-photo count off the progress bar while the people list and covers beside it were properly scoped. Now scoped by the same predicate, with the caller passing its audience. RECLUSTER, ROUND 1'S FIX WAS INCOMPLETE. I made suppression follow every descendant but still copied the flags from the majority ANCESTOR. When reclustering merges a visible named person with a hidden one, the majority ancestor is often the visible one — republishing the hidden person's photos. Suppression is now OR-ed across every ancestor contributing faces. The name also now goes to the genuine largest descendant; the previous code took whichever cluster came first in map order, which the comment already claimed it did not. LIFECYCLE. Face data is excluded from backups and exports, but photos. face_status came across intact, so a restored install claimed every photo was scanned while holding no faces — and the worker only claims 'pending', so it stayed that way forever. Now: the SQLite backup requeues in the dump, restore requeues after the pool reinit (the Postgres path cannot rewrite rows inside pg_dump), the portable importer purges LOCAL face tables (they were excluded from the replace list, so another instance's embeddings survived an import with FK checks suspended) and requeues, and archiving disables detection so a restored archive is honestly off rather than enabled-and-empty. WRITE PATHS. Only processPhoto enqueued. The synchronous upload path (chunked-upload completion, watch-folder) left photos unscanned, and replacePhoto kept the OLD image's faces on a row now pointing at a different picture — stale identities shown on the new photo. FRONTEND. PeopleSheet and the admin manager rendered centred thumbnails and ignored the bbox, so on group photos the avatar showed whoever stood in the middle and two people from one photo were indistinguishable — in the manager whose entire job is telling faces apart. The crop maths is now one shared helper (faceCrop.ts) so the three surfaces cannot drift again. Full-page layouts (gallery-premium, gallery-story) render their own lightbox and never received the people props. Backend failure set verified identical to origin/main; frontend 140 green; tsc and eslint clean. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * fix(faces): round 3 — five of round 2's fixes were wrong or no-ops (#1074) Third and final Codex round. Eight findings, four P1 — and the important part is that FIVE of them are defects in round 2's fixes, not in the original code. - The sync-upload enqueue I added was a silent no-op. It queried through `trx` after the transaction had already been committed, which throws "Transaction query already complete" straight into the catch I had wrapped it in. Chunked uploads and watch-folder imports were still never scanned, and the code read as though they were. Uses `db` now. - The post-restore requeue ran BEFORE the files were restored, in both the portable importer and the native restore. The face worker is live during a restore, so it could claim those rows and scan the previous instance's files, or fail them for originals not yet on disk — with nothing to requeue them afterwards. Both now run after file restoration; the native one is extracted into requeueFaceScans() and called from the full and database-only paths. - The admin face crop mixed coordinate spaces: an original-pixel bbox scaled against the THUMBNAIL's natural size. The API now returns the source dimensions alongside the box, so there is one space to reason about. - Forwarding people props through layoutProps did not make them work — the full-page layouts never destructured them. GalleryStoryLayout now threads them to its own lightbox. Genuinely new findings, all in the same class as ones already fixed: - releaseToPending updated unconditionally, so a photo purged while its sidecar request was in flight came back as 'pending' and was rescanned — biometric rows reappearing after the purge reported success. Round 2 fixed exactly this on the COMMIT path and I did not carry it to the retry path. Now guarded on 'processing'. - purgePhotoFaces left face_status alone, so a worker mid-scan still satisfied its commit guard and could write fresh faces into a photo being deleted — orphans, since the FK cascade is inert on SQLite. It now clears the claim as part of the purge. - Phase 3 was unreachable: the migration seeds face_auto_categorize_enabled false and nothing could ever write it, so the rule engine and its undo endpoint returned "disabled" in every real flow. Added GET/PUT and a toggle on the admin card, EN + DE. NOT fixed, deliberately: GalleryPremiumLayout uses yet-another-react-lightbox rather than the shared PhotoLightbox, so person chips there are a real port rather than a prop forward. Recorded as open rather than bodged. Backend failure set identical to origin/main; 41 face tests and 140 frontend tests green; i18n audit reports EN and DE complete at 71 keys. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * feat(faces): block face recognition on the all-in-one image (#1074, #1042) The single-container image cannot run this feature, so it is refused there rather than left to degrade. WHY, since the reason is not obvious from the code: the AIO image runs the backend, the frontend, SQLite and every background worker inside one container aimed at "one photographer plus guests browsing". It has no Redis, SQLite gives it a single writer, and it contains no ML sidecar to talk to. Face detection would add a second image-processing pipeline competing with Sharp for the same CPU and memory. That failure is not loud — the install just becomes slow and looks broken, which is the worst possible shape for a deployment whose whole promise is one container and no decisions. Gated on an explicit PICPEAK_SINGLE_CONTAINER marker, NOT inferred from SERVE_FRONTEND or a SQLite path: plenty of legitimate multi-container setups serve the frontend from the backend or run SQLite, and none of them should lose the feature by accident. Three layers, because the first is the only one that enforces: - faceSettings.isFeatureEnabled() returns false before consulting the flag, so a database restored from a full deployment with `faces` enabled still cannot switch it on here. - The feature-flag API forces `faces: false` in both directions, so the admin UI reflects reality instead of offering a switch that refuses to stay on. - The Features tab renders the card disabled with a plain-language reason, read from a new `single_container` field on /admin/system/version (an endpoint the admin UI already calls). Documented in ml/README.md and .env.example. Three tests pin the behaviour, including that the marker only accepts explicit truthy values. NOTE FOR PR #1068: this expects `Dockerfile.aio` to set `ENV PICPEAK_SINGLE_CONTAINER=true`. That one line lives on that branch and is not in this commit — until it lands, an AIO build would still offer the feature. Worth adding alongside the `Limits` section of docs/single-container.md. 44 face tests green; EN + DE complete at 72 keys. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * test(faces): pin the bbox coordinate space with a real scale factor (#1074) The coordinate-space bug — boxes stored in preview space while every consumer reads them as original-image pixels — had no test, and could not have been caught by the ones that existed: every photo in the demo gallery is 750px, so the scale factor was always exactly 1.0 and the correction never executed. Verified by hand first, on a real 4000x3000 upload with the face placed off-centre so a wrong crop would be unmistakable. Before the fix the stored box was 1493,204 (preview space, face actually at x≈2850-3618); after, 3110,426 — a factor of 2.083, exactly 4000/1920, landing inside the face. The admin crop then resolved to left=-395px/top=-46px on a 64px window, which is the face centred. That verification is now a test rather than a memory. Three cases: a 4000px photo must scale by 4000/1920, a 1920px photo must NOT change (the case that hid the bug), and a row with no width must fall back to unscaled rather than storing NaN. Note for anyone extending these: jest hoists mock factories above the file, so anything they close over has to be `mock`-prefixed. Getting that wrong fails at transform time with a message that does not name the variable. 47 face tests green. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc --------- Co-authored-by: Paul Nothaft <[email protected]>	2026-08-18 22:37:28 +02:00
LucaandPaul Nothaft	8809564aad	feat(backup): open sqlite → pg .picpeak restore as the supported upgrade direction (#1041) (#1043) ... 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]>	2026-08-16 19:01:36 +02:00
Paul Nothaft	8e3573788b	feat(downloads): per-gallery download resolutions (#858) (#1022) ... 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.	2026-08-11 09:46:46 +02:00
Paul Nothaft	75bfad2b6a	fix(slideshow): stop "no crop" fit letterboxing a pre-cropped frame (#1015) (#1018) ... 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.	2026-08-10 13:32:14 +02:00
Paul Nothaft	a607cea110	fix(auth): issuer-tag the oversize SSO logout marker (#798) (#1010) ... Phase 3 validated a stored ID token hint against the currently configured issuer, but the oversize path never got that check: an ID token above the 3.9KB cookie limit was stored as the bare string 'sso', which collapsed to an undefined hint at logout and skipped validation entirely. Changing the issuer while such a session was live bounced the user to the new IdP on logout. Stores sso.<base64url(issuer)> instead and moves all marker interpretation into buildEndSessionUrl: raw ID token -> iss/aud-validated hint, issuer-tagged marker -> round-trip without a hint, anything else -> no round-trip. Every branch fails closed. Refs #798.	2026-08-10 09:14:56 +02:00
Luca	f00661511c	feat(gallery): admin preview skips the password on protected galleries (#981) ... 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.	2026-08-04 16:48:07 +02:00
Luca	165cebdb5c	feat(accounting): re-bill proof attachment, CRM panel & hours↔re-bills cross-add (#979) ... 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.	2026-08-03 22:03:31 +02:00
Paul NothaftandPaul Nothaft	03087c798c	fix(security): close GHSA-g94x (cross-gallery photo read) + GHSA-pv6w (admin DB export) (#924) ... * 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]>	2026-07-30 14:24:28 +02:00
Paul NothaftandPaul Nothaft	d9ad982373	fix(tests): raise migration-boot hook timeout pins to the 120s default (#900) ... The 3.97.0-beta.0 release PR (#899) failed its backend Tests job on slideshowPublic.test.js: bootCrmDb's full migration chain crossed the suite's explicit 30s beforeAll timeout argument on a slow runner. #860 raised the config default and the jest.setTimeout pins to 120s, but hook-ARGUMENT pins override the config default and were left behind — same time-bomb, different syntax. Every beforeAll that boots the migration chain and pinned 30s/60s is raised to 120000 (16 suites). Untouched on purpose: the three suites whose pinned hooks don't run migrations (webhookDelivery, imageProcessor.storage, storageBackend) and publicQuotes' 30s pin on the rate-limit lockout test — neither grows with the migration chain. No test logic changed. Co-authored-by: Paul Nothaft <[email protected]>	2026-07-29 12:38:26 +02:00
Paul Nothaft	08be2b84f1	fix(security): close 5 Trivy alerts — postcss/tar bumps + drop npm from the runtime image (#878) ... * fix(security): close the 5 open Trivy alerts — dep bumps + drop npm from the runtime image Backend deps: - postcss 8.5.10 -> 8.5.18 (CVE-2026-45623, GHSA-r28c-9q8g-f849; the pin exists to force sanitize-html's transitive copy onto a fixed version) - tar pin/override >=7.5.16 -> >=7.5.21, resolves 7.5.22 (GHSA-r292-9mhp-454m) Runtime image: - Remove the npm CLI from the final stage instead of upgrading it: npm's bundled node_modules ship tar 7.5.19 and brace-expansion 5.0.7 (no npm release bundles the fixed versions — checked 11.18.0 and 12.0.1), and npm never runs in production. wait-for-db.sh now invokes the migration runners via node directly. This ends the recurring npm-bundled-CVE alert class; the previous 'npm install -g npm@11' line was itself a patch for the last batch. * fix(restore): run post-restore migrations via node — the image ships no npm restoreService still shelled out to 'npm run migrate:safe' after a restore; with npm removed from the runtime image that would ENOENT into the non-fatal catch, silently leaving a restored older backup on a schema behind the running code until the next container restart. Invoke migrations/run-migrations-safe.js through node directly, matching wait-for-db.sh. The PR #596 source-contract test now pins the new invocation.	2026-07-27 09:54:33 +02:00
Paul Nothaft	a2e723413e	fix(backup): make backup settings actually apply (#871) (#874) ... * fix(backup): make backup settings actually apply (#871) - Wire the What-to-Backup toggles into the walker: honor backup_include_thumbnails / backup_include_photos (opt-out, default ON) and accept the UI's backup_include_archives spelling for the archived gate (the engine expected _archived, so the Archives checkbox silently never worked). - Fix the 167.6 TB dashboard size: file_size_bytes is a bigint that node-postgres returns as a string, and the S3 path concatenated it onto the byte counter; coerce to Number at the source. - Compute the real next scheduled run (cron-parser) and return it as nextBackup; the UI read a field the API never sent and rendered a hardcoded 'Not scheduled'. A named schedule label now beats the stray default cron the UI always sent, which silently turned weekly schedules into daily 03:00 runs. - Never back up filesystem noise (.nfs* silly-renames, .DS_Store, Thumbs.db) and honor backup_exclude_patterns in the walker (previously rsync-only). - Remove the compression/encryption toggles from the configuration UI: no backend implementation exists, and collecting an encryption passphrase while uploading plaintext is a false promise. Closes #871 * fix(backup): close the review gaps in the settings wiring - The UI's backup_include_archives now beats the migration-seeded backup_include_archived: every install has the singular key seeded true, so the alias-only-when-absent lookup made unchecking Archives a no-op. - rsync destinations now receive the de-selected What-to-Backup paths and the noise filters as anchored --exclude args; previously rsync synced the whole storage root and the walker's selection only shaped the manifest, which then misreported what was actually transferred. - Escape regex metacharacters in the walker's glob matcher: '.nfs*' compiled to /^.nfs.*$/ whose leading dot matched any character, so files like anfs-photo.jpg were silently dropped from backups. - The Backup Coverage report now uses the same gate as the walker (new 'skipped-by-setting' status) instead of re-implementing it without the opt-out toggles and the archives alias. * fix(backup): make the coverage diagnostics agree with the walker - The coverage table shows the alias-aware flag value the gate actually used, instead of the seeded backup_include_archived shadowed by the UI's plural key (true next to a 'Gated off' badge). - skipped-by-setting paths are now counted in the coverage summary (backend, TS contract, summary card, EN/DE locales) so the totals reconcile again when Photos or Thumbnails is unchecked. - The form's thumbnail default now matches the backend's never-saved fallback (include): the checkbox no longer shows 'off' while thumbnails are being backed up, and saving an unrelated setting no longer flips the backup scope. * fix(backup): keep custom crons, exclude disabled rows from rsync, normalize flag display - Saving a named schedule no longer wipes the stored custom cron: the backend already prefers the label, so the cron field stays inert for named schedules and is preserved for switching back to Custom. A custom schedule now validates the 5-field expression before saving (the backend silently fell back to daily 02:00 on a blank value). - resolveExcludedBackupPaths now also returns rows disabled via include_in_default, so rsync excludes them; the enabled-only loader hid them and rsync transferred their contents anyway. - The coverage table normalizes flag values like the walker does — Boolean('false') displayed true beside a gated-off badge.	2026-07-27 09:06:38 +02:00
Paul NothaftandPaul Nothaft	219d07b04a	feat(auth): OIDC logout-to-IdP — phase 3 (#798) (#865) ... * feat(auth): OIDC logout-to-IdP — phase 3 (#798) RP-initiated logout behind a new oidc_logout_from_idp setting: logging out of PicPeak also ends the IdP session. The SSO callback stores the raw ID token in an HttpOnly cookie (also the marker that the session came in via SSO — local-password sessions never bounce to the IdP); /logout builds the end_session URL from discovery metadata with id_token_hint + post_logout_redirect_uri + client_id and returns it as ssoLogoutUrl for the frontend to navigate to. Any failure (no end_session_endpoint, IdP unreachable, feature off) degrades to the plain local logout. Settings surface exposes the toggle plus the computed post-logout redirect URI to register at the IdP. Session timeouts deliberately stay local-only. 6 integration tests over the mock IdP; live-verified against Keycloak 26 (logout ends the Keycloak session, no confirmation prompt). * fix(auth): harden the SSO logout marker cookie (#798 phase 3) Codex review round 1: - Derive the oidc_id_token cookie options from the shared cookie policy (COOKIE_SAMESITE / COOKIE_DOMAIN / secure resolution) — hardcoded Lax meant split-origin deployments running on SameSite=None never sent the marker to the cross-site /logout XHR, silently disabling logout-to-IdP. - Oversized ID tokens (>3.9KB) now store a bare 'sso' marker instead of no cookie, so the claimed client_id-only end-session fallback actually happens; /logout only passes the value as id_token_hint when it is a real JWT. - establishAdminSession clears any stale marker on every fresh login — sessions can die without /logout (deactivation, expiry, restore), and a surviving marker would bounce a later local-password session to the IdP. The SSO callback re-sets the marker for its own session. Tests: oversized-token marker + hint-less end-session URL, stale-marker cleared on local login; helper updated for the clear+set cookie pair. * fix(auth): validate the logout hint against the current OIDC config (#798 phase 3) Codex review round 2: an ID token stored at login can outlive an issuer/client config change; sending it to the newly configured IdP as id_token_hint strands the user on the IdP's error page (providers validate iss/aud on the hint). buildEndSessionUrl now decodes the hint (no verification — routing only): different issuer → skip the round-trip entirely (the session belongs to another IdP); same issuer but changed client → keep the round-trip, drop the unusable hint. Two tests pin both paths. --------- Co-authored-by: Paul Nothaft <[email protected]>	2026-07-24 11:42:09 +02:00
Paul NothaftandPaul Nothaft	323dcae917	fix(gallery): block password form in Instagram in-app browser and unmask login errors (#863) ... * fix(gallery): block password form in Instagram in-app browser (#654) Field reports show gallery password login still failing inside Instagram's IAB after the #656 input-attribute/trim defenses. Three changes: - Replace the advisory amber banner with a red blocking state: the password form is hidden in the Instagram IAB and replaced with platform-specific "open in external browser" instructions plus a copy-link button (clipboard API with execCommand fallback). A "try anyway" link restores the form as an escape hatch. - Stop masking non-password failures as "incorrect password": a request that never got a response (offline, webview killed it) now reports a connection error, and a reCAPTCHA 400 reports a verification failure — both previously fell through to the wrong-password message and sent guests chasing the wrong cause. - Strip invisible Unicode (zero-width chars, word joiner, BOM, soft hyphen) from the submitted password in addition to trimming — these ride along when the password is copy-pasted out of a chat app and fail byte-exact bcrypt compare server-side. * fix(gallery): retry login with typed password + honor execCommand result (#654) Codex review round 1: - Stored passwords can legitimately contain the invisible code points the sanitizer strips (e.g. ZWJ emoji sequences) — creation paths don't normalize. On a 401 where the sanitized form differs from the typed (trimmed) input, retry once with the typed value. Skipped when a reCAPTCHA token is in play (single-use). - document.execCommand('copy') signals failure via its return value, not by throwing — only show "Link copied" when it returns true. * fix(gallery): move invisible-char password fallback server-side (#654) Codex review round 2: the client-side retry either burned the single-use reCAPTCHA token (making exotic-but-valid passwords impossible to enter with reCAPTCHA on) or burned failed-attempt lockout quota on every rescued login. Doing the fallback as a second bcrypt compare inside the same gallery/verify request eliminates both: exact bytes are compared first (stored passwords containing e.g. ZWJ emoji keep working), the sanitized form only on mismatch, and trackFailedAttempt only fires when both fail. Frontend goes back to plain trim-on-submit; the client-side sanitizer util and retry are removed. 7 integration tests pin the contract. --------- Co-authored-by: Paul Nothaft <[email protected]>	2026-07-23 21:45:07 +02:00
Paul NothaftandPaul Nothaft	40eb03f0d8	fix(tests): raise jest timeouts to survive the growing migration chain (#860) ... The 3.94.0-beta.0 release PR (#859) failed its backend job on workflowEngine.test.js: bootCrmDb() runs every core migration in beforeAll, and with migrations 163-165 merged the setup crossed the suite's jest.setTimeout(30000) on CI runners — the log shows migration 099 still seeding after the hook timed out. Same pass is green locally and passed on #857's rebase minutes earlier: borderline-slow, not deterministic. - jest.config.js: testTimeout 120000 as the default, so bootCrmDb suites without an explicit pin stop being time bombs as the chain grows - every suite-level jest.setTimeout below 120s raised to 120s — local pins OVERRIDE the config default, so the 30s/60s ones would keep flaking regardless of the global bump No test logic changed anywhere. Co-authored-by: Paul Nothaft <[email protected]>	2026-07-22 21:27:12 +02:00
Paul NothaftandPaul Nothaft	c6ec93eef9	fix(dates): normalize SQLite epoch timestamps at remaining API surfaces (#485 follow-up) (#857) ... The audit #485 called for: on SQLite (native installs), timestamp columns written with a raw `new Date()` through knex store epoch-ms numbers; Postgres returns ISO strings. Frontend code written against Postgres calls parseISO() on them — parseISO(number) throws and crashes the page. #485 fixed admin Users and listed api tokens / photos / activity as out-of-scope follow-ups. Verified crash on main: Timeline gallery layout parseISO(uploaded_at) against photos written by the archive-RESTORE path (raw Date). Other raw-write surfaces (api_tokens last_used_at/revoked_at, email_queue) degrade rather than crash but violate the ISO contract. - extract toIso() from adminUsers.js into utils/dateNormalize.js (contract unchanged — the 10 existing #485 tests still pin it) - write-side: archive-restore uploaded_at, api-token last_used_at / revoked_at, email_queue created_at/sent_at now write ISO strings - read-side (heals existing corrupted rows): gallery /photos normalizes uploaded_at/captured_at; api-tokens list normalizes all four timestamp fields - frontend defence-in-depth: Timeline layout parses uploaded_at tolerantly (typeof guard) for stale caches / old backends - 2 regression tests seed literal epoch numbers and assert the API serves ISO strings activity_logs turned out safe (created_at comes from the DB default, not a raw Date) — left untouched. Co-authored-by: Paul Nothaft <[email protected]>	2026-07-22 21:07:32 +02:00
Paul NothaftandPaul Nothaft	2f05fcc39d	feat(gallery): reveal mode — hide gallery from guests until reveal (#838) (#856) ... * feat(gallery): reveal mode — hide gallery from guests until reveal (#838) Guests can upload during the event but see no photos until the host reveals the gallery, manually ("Reveal now") or at a scheduled time. - migration 165: events.reveal_mode / reveal_at / revealed_at. Effective visibility is computed at REQUEST time (reveal_at <= now opens the gate exactly on schedule); the minutely scheduler only stamps revealed_at durably and emits a gallery.revealed workflow trigger - server-side enforcement in gallery.js: /photos returns the event shell with photos: [] + hidden_until_reveal for plain guests; image/download/stats endpoints 403 with GALLERY_HIDDEN (photo IDs are sequential — listing-only gating would be probeable); feedback-summary gated too. Slideshow tokens (surprise beamer), client access and the admin preview bypass; the guest upload route stays open - admin: reveal toggle + optional scheduled datetime next to the guest upload settings, status line and "Reveal now" button on the overview; re-enabling the toggle clears revealed_at so a gallery can re-hide - guest UI: upload-only view (hero, friendly message, scheduled time, upload button) for every layout; i18n for all 8 locales - timestamps written as ISO strings — the SQLite driver stringifies raw Date objects into garbage; ISO round-trips on both engines - 14 integration tests over minted gallery/slideshow/client/admin tokens * fix(gallery): reveal/re-arm semantics + upload button i18n key (#838) - "Reveal now" also clears a pending reveal_at: the schedule is consumed, so the full-form admin save can't accidentally re-hide a revealed gallery with a stale future date - setting a FUTURE reveal_at on a revealed gallery re-arms hiding — the one intentional way to re-hide without double-toggling the mode - guest upload button uses the existing upload.uploadPhotos key (gallery.uploadPhotos never existed; the button showed EN everywhere) * fix(gallery): close reveal bypasses from review round 1 (#838) - the hero-derivative route and the secure-images token-mint + secure-download routes are now reveal-gated: hero serves a 1920px derivative of ANY sequential photo id and secure tokens fetch originals — both were open bypasses while hidden. blockHiddenGallery moved to utils/revealMode.js and shared - customer-portal tokens (via:'customer', no accessLevel) now bypass reveal mode — they are the host/customer, not a guest, and were getting the upload-only view - an open hidden guest view refetches exactly at reveal_at plus a 60s fallback poll, so the gallery appears without a manual reload - gallery.revealed added to the workflow editor's trigger picker so the advertised notification hook is reachable in the UI - migration 165 guards each column independently (partial-state safe) * fix(gallery): reveal round 2 — remaining bypass surfaces + lifecycle edges (#838) - legacy /api/images router reveal-gated (view, secure-token + signed-url minting), and the signed-URL SERVE path re-checks hidden state via a backward-compatible bypass flag in the token payload - secure-image tokens record revealBypass at mint and are re-validated at serve time — a re-hide kills in-flight guest tokens within the request, while slideshow/client tokens keep working - OG metadata and the unauthenticated /og cover fall back to the brand logo / 404 while hidden — no hero-photo spoiler for social crawlers - photo-feedback GET/POST reveal-gated (sequential ids were enumerable); /my-feedback returns the empty back-compat shape (rows leak filename + storage path) - the reveal scheduler skips drafts — no premature stamp/notification for unpublished galleries - emitWorkflowEvent gains an additive dedupSuffix; both reveal emitters pass the reveal timestamp so a re-hidden gallery's second reveal fires workflows again instead of deduping into silence * fix(gallery): reveal round 3 — schedule consumption + two-way client sync (#838) - the scheduler now consumes reveal_at when stamping (matching "Reveal now"), and re-arming via a partial API update clears a stale PAST schedule — previously {reveal_mode:true} without reveal_at could instantly re-open the gate through the leftover date - /photos exposes reveal_armed so an open VISIBLE gallery keeps a 60s poll while the mode is on — a re-hide now propagates to open clients in both directions, not just hidden→visible Codex round-3 claim about timestamp-without-timezone drift on non-UTC Postgres was verified FALSE: knex's table.timestamp() creates timestamptz on PG (confirmed via information_schema on a live install), which stores absolute instants regardless of server TZ. --------- Co-authored-by: Paul Nothaft <[email protected]>	2026-07-22 20:59:59 +02:00
Paul NothaftandPaul Nothaft	f8a95d29d2	feat(auth): OIDC role mapping + login policy — phase 2 (#798) (#854) ... * feat(auth): OIDC role mapping + login policy — phase 2 (#798) Role mapping: configurable dot-path roles claim (Keycloak realm_access.roles, Authentik/Pocket ID groups, Entra roles), IdP-value → PicPeak-role mapping table validated against the roles table, re-evaluated on every SSO login with highest-priority-wins on multiple matches. The last active super_admin is never demoted. Optional require-mapped-role policy refuses logins whose token maps to no role (sso_error=no_role). Login policy: oidc_disable_local_login makes the API refuse password logins (403 LOCAL_LOGIN_DISABLED) and the login page render SSO-only; only effective while SSO is enabled+configured, and OIDC_BREAK_GLASS=true always re-opens local login. Public settings expose the EFFECTIVE flag only. Settings UI: Role-mapping card (claim path, mapping rows editor, strict toggle) and Login-policy card with break-glass hint, EN+DE. 14 new integration tests over the mock IdP. * fix(auth): harden phase-2 review findings (#798) - memoize the scrypt-derived OIDC key and serve /public/settings from a 10s-TTL flag cache — the unauthenticated endpoint no longer pays a 13-key config read + blocking scryptSync per request (login route still checks uncached) - make the last-super-admin demotion guard atomic (FOR UPDATE on the active super rows) — concurrent mapped callbacks could previously both count 2 and demote both supers - own-property lookup in role mapping: IdP values like `constructor` now count as unmapped instead of corrupting the roles query - SsoTab clears oidc_disable_local_login in the same save that turns SSO off — the full-form payload otherwise hit the server-side 400 * fix(auth): guarantee break-glass reachability for SSO-only mode (#798) - wire OIDC_BREAK_GLASS and OIDC_ENCRYPTION_KEY through the quick-start docker-compose.yml env allowlist (production compose already passes .env via env_file) and document both in .env.example - refuse enabling oidc_disable_local_login unless an active local-password super_admin exists: OIDC_BREAK_GLASS only re-opens the password route, which OIDC-owned accounts can never use, and settings.edit is super_admin-only — an all-OIDC instance would be unrecoverable during an IdP outage * fix(auth): close SSO-only lockout gaps from review round 3 (#798) - role sync never demotes the last active LOCAL-password super_admin (an OIDC-owned super does not count as break-glass), and isLocalLoginDisabled() disarms itself when no such account remains — self-healing against manual demotion/deactivation/deletion paths - the local-super save-time check now validates the MERGED state, so re-enabling SSO with a stored disable flag is checked too - ALL oidc_* keys are reserved from the generic settings upserts/reads (prefix match) — policy and mapping invariants can only go through the validated PUT /sso - /admin/login/mfa re-checks the policy so an mfa_pending token minted before the flip cannot complete into a local session --------- Co-authored-by: Paul Nothaft <[email protected]>	2026-07-22 20:59:19 +02:00
Paul Nothaft	997a85cdbc	test(crm): mint-path integration tests — quote send, invoice storno, contract countersign (#850) ... * test(crm): mint-path integration tests — quote send, invoice storno, contract countersign (#587) End-to-end through the real HTTP → route → service → DB → email-queue → file pipeline on full-migration SQLite (helpers/crmDb), real pdfkit/ pdf-lib rendering, no mock-fs, no network. 7 tests. Deviations from the issue spec — the tests pin the code's real behavior: - Storno route is POST /:id/cancel (not /:id/storno), responds 200 with { cancelled, stornoId } (not 201). - Quote re-send rejects with 409 (not 400). - Contract statuses are signed_by_customer → fully_signed; the hash columns are pdf_sha256 / signed_pdf_sha256 (no integrity_hash) — the test verifies the stored sha256 against the file on disk. - Business-doc PDFs persist under process.cwd()/storage/business-docs, not STORAGE_PATH — isolated via chdir into the temp dir. Two documented, test-scoped harness workarounds: shrunk pool acquire timeout (guards against the pre-existing logActivity-inside-transaction deadlock in createContract/createStorno on single-connection SQLite — worth its own fix) and Date→ISO binding normalization (node-sqlite3's cross-realm Date detection under jest's vm sandbox). Assisted-by: task agent (worktree) * test(crm): pin sendStorno side effects + real customer-sign flow (codex review of #850) - Storno test now asserts the delivery leg cancelInvoice deliberately swallows on failure: storno status 'sent', PDF on disk, storno_issued email queued to the customer — a broken render/persist/queue no longer stays green. - Contract seed goes through sendContract's token + a real recordCustomerSignature instead of a direct status UPDATE, so countersign exercises the signature-layering path; the test now also pins that the customer's signature asset survives countersigning. * test(crm): prove both signature stamps are embedded in the countersigned PDF (codex review of #850, round 2) Path/hash assertions alone stay green if countersign stamps the admin onto the unsigned base PDF. New pdf-lib helper counts embedded image XObjects per page of the final document and asserts the signature page carries at least two — customer stamp AND admin stamp.	2026-07-19 22:36:38 +02:00
Paul Nothaft	cbde7636aa	Merge remote-tracking branch 'origin/main' into feat/oidc-sso-phase1 ... # Conflicts: # backend/src/middleware/maintenance.js	2026-07-16 13:53:28 +02:00
Paul Nothaft	a77c2c2c57	Merge pull request #813 from PicPeak/feat/harden-picpeak-restore-robustness ... feat(security): harden .picpeak restore robustness — sessions, roles, sequences	2026-07-16 13:39:17 +02:00
Paul Nothaft	340d91bdd5	feat(security): harden .picpeak restore robustness — sessions, roles, sequences ... Implements the three restore-hardening items deferred from the #811 Codex review (all validated against a real Postgres, see __tests__/integration/ picpeakRestorePg.test.js). Backend-only; targets main (feature, not a backport). 1. Global session cutoff (utils/sessionCutoff.js). A restore reassigns admin/ customer/event ids, so ANY pre-restore JWT can rebind to a different restored principal. Revoking just the importing token wasn't enough. importFromPicpeak now stamps a unix-second cutoff in app_settings after the restore commits, and adminAuth / galleryAuth / verifyGalleryAccess / customerAuth reject any token whose iat predates it (cached 30s → one in-memory compare on the hot path). The operator's forced re-login mints a token past the cutoff, so it passes. 2. Role preservation across an RBAC replace (captureOperatorRole / preserveOperatorRole). The operator's role + granted permission NAMES are captured before the wipe; after roles/role_permissions are replaced the role is resolved by NAME against the restored data, and re-created with its grants if the backup omits it — so a crafted or cross-instance backup can't silently downgrade or lock out the operator. reinjectCurrentAdmin now returns the operator's id so the row can be re-pointed at the resolved role. 3. Postgres identity-sequence resync (resyncSequences). batchInsert writes explicit ids without advancing the sequences, so the next natural insert into any restored table collided on the PK. Runs AFTER commit (setval isn't transactional) and guards every table with a column-existence check — pg_get_serial_sequence RAISES on id-less tables like role_permissions. No-op on SQLite. Tests: SQLite unit tests for the cutoff and role preservation; a gated Postgres integration suite (npm run test:pg with PICPEAK_PG_TEST_URL) covering sequence resync, the id-less-table guard, explicit-id reinject, role re-creation, and a full cross-instance replaceAllTables run asserting operator preservation, role re-establishment, FK integrity, and collision-free post-restore inserts. Stacks on #811 (shares the reinject hardening); merge after it.	2026-07-16 12:56:34 +02:00
Paul Nothaft	07f2c90055	fix(security): mask backup credentials on read + unblock MFA login during maintenance ... Two pre-existing bugs surfaced while reviewing #806 (kept separate per scope policy — no OIDC code here): - backup_s3_secret_key and backup_rsync_ssh_key (an SSH PRIVATE KEY) were returned in PLAINTEXT by GET /admin/backup/config and by the generic settings reads (GET /admin/settings and /admin/settings/:type — which mask the recaptcha/umami/rybbit keys but not these). All three now mask with the established bullet sentinel, and PUT /admin/backup/config skips the sentinel on write so the edit form round-trips without clobbering stored credentials (same pattern as the email/WhatsApp config endpoints) - /api/auth/admin/login/mfa was missing from the maintenance-mode allowlist: the first login step passed, the second factor got a 503 — any MFA-enrolled admin was locked out exactly while maintenance mode was on Regression tests: masking on all three read paths, sentinel round-trip preserves stored values, real rotation still writes.	2026-07-16 10:13:34 +02:00
Paul Nothaft	e91c7deaa4	fix(oidc): local-credential lockout, session hydration, split-origin gaps (codex round 3) ... - OIDC-owned accounts can never authenticate locally: the password login rejects auth_provider='oidc' rows outright (generic 401), and the super-admin password reset refuses them with a clear message — previously a reset would have minted a local password bypassing the IdP's MFA/access policies - /auth/session now returns a full adminUser payload (role join) and AdminAuthContext hydrates user state from it: an SSO redirect establishes the session without any login JSON, which left the header identity blank and current-admin form defaults empty - the /sso/login error path redirects absolute to the frontend base (same split-origin reasoning as the callback) - docker-compose.yml passes API_URL through to the backend (production compose uses env_file and needs nothing; dev compose is gitignored) - authSession.symmetry test mock taught the joined admin lookup (leftJoin, prefixed columns, aliases) — the route change made the old mock throw, which read as "table missing, trust token" Tests: new case pins that a known-good password on an OIDC-owned row still gets 401. 14/14 OIDC, 13/13 symmetry.	2026-07-16 10:07:59 +02:00
Paul Nothaft	7f7d38a57f	fix(oidc): security + robustness hardening from codex review rounds 1-2 ... Round 1: - bind SSO identities to (external_issuer, external_subject): OIDC only guarantees sub uniqueness within an issuer, so a sub-only lookup let a newly configured IdP's user inherit an old IdP's admin account on subject collision; migration 162 gains external_issuer + composite unique index (unmerged migration, edited in place) - fetch UserInfo (with sub cross-check) when the ID token carries no email — spec-compliant providers may serve email/profile claims only there; ID-token claims win on merge - allowlist /admin/sso/login + /callback in maintenance mode, or SSO-only (JIT) admins are locked out exactly when they need in - strip reserved keys (oidc_client_secret, setup_token) from BOTH generic settings reads (GET / and GET /:type) Round 2: - redirect_uri prefers API_URL (the API's public origin — where the state cookie lives); final redirects absolute to the frontend base; login button builds its URL via buildResourceUrl — split-origin deployments (absolute VITE_API_URL) work end to end - PUT /sso validates the MERGED resulting state (partial update cannot blank issuer/client while enabled=true survives; enabling requires a derivable redirect URI) - openid scope forced into oidc_scopes on save - discovery-cache key includes a secret fingerprint (multi-worker secret rotation) - email→admin linking claims the row atomically (conditional update on external_subject IS NULL) — concurrent first-time callbacks with the same verified email but different subjects can't both authenticate Tests: mock IdP gains userinfo endpoint + email-via-userinfo-only mode; new cases pin the userinfo merge and issuer-collision non-inheritance; redirect assertions updated for absolute URLs. 13/13.	2026-07-16 09:41:48 +02:00
Paul Nothaft	ac1838fbd7	fix(oidc): fail clearly when no public base URL is configured ... CI exposed that getFrontendBaseUrl() returns '' without FRONTEND_URL or the general_site_url setting (local runs were masked by backend/.env): the flow then sent a RELATIVE redirect_uri to the IdP, which surfaced as an opaque IdP-side error. getRedirectUri now throws OIDC_BAD_CONFIG with an actionable message (login route maps it to sso_error=config); the settings GET degrades to an empty redirect_uri instead of 500ing. The test pins FRONTEND_URL explicitly so it runs identically with and without a local .env.	2026-07-16 08:59:48 +02:00
Paul Nothaft	ed5fc5ad5c	feat(auth): OIDC SSO for admin users — phase 1 (#798) ... Authorization-code + PKCE against a single configurable IdP via openid-client v5, with JIT provisioning. Verified end-to-end against a real Keycloak 26 (realm + confidential client + verified-email user): settings → discovery test → login button → Keycloak → dashboard. Backend: - migration 162: admin_users.auth_provider ('local' default) + external_subject, composite unique index - oidcService: settings-driven config (client secret AES-256-GCM at rest, mfaService pattern, OIDC_ENCRYPTION_KEY fallback JWT_SECRET), cached discovery, sub-based identity binding — email linking of existing admins only with email_verified=true; JIT behind oidc_autoprovision with configurable default role and an unusable random password hash - GET /api/auth/admin/sso/login + /callback: state/nonce/PKCE verifier cross the redirect in a 10-min signed httpOnly SameSite=Lax cookie; the callback reuses the local login's session establishment (completeAdminLogin split into establishAdminSession + JSON wrapper) so SSO sessions are identical downstream; every failure lands on /admin/login?sso_error=<key> as a translated toast - dedicated /admin/settings/sso GET/PUT/test endpoints (secret write-only, redacted to a set-flag; registered ABOVE the generic /:type matcher which would shadow them); oidc_client_secret added to the reserved keys stripped from generic settings upserts - public settings expose only oidc_enabled + oidc_button_label for the login page Frontend: - Settings → Single Sign-On (OIDC) tab: issuer/client/secret, scopes, autoprovision + default role, button label, enable toggle, redirect URI copy box, server-side discovery test - login page: SSO button (custom label) when enabled; sso_error query param surfaced as translated toasts; EN+DE i18n Tests: 11 integration cases against an in-process mock IdP (real discovery/JWKS/PKCE/ID-token validation) — JIT on/off, sub-vs-email binding, unverified-email rejection, deactivated admin, missing/forged state cookie, nonce tamper, secret encryption round-trip, disabled 404. MFA is delegated to the IdP on the SSO path; local login stays available as break-glass. Role-claim mapping and logout-to-IdP follow in phase 2/3.	2026-07-16 08:54:26 +02:00
Paul Nothaft	d64eef8abf	Merge pull request #803 from PicPeak/fix/event-type-hardcoded-deps ... fix(event-types): un-hardcode event type dependencies in v1 API and CRM	2026-07-15 23:02:35 +02:00
Paul Nothaft	5da1c3a12f	fix(event-types): un-hardcode event type dependencies in v1 API and CRM (#800) ... Split out of #801 so the public-API behavior change gets its own review: - v1 POST /events validates event_type against the live event_types catalog instead of the hardcoded whitelist — custom types created in Settings → Event Types were rejected with 400. BREAKING for the never-seeded 'family' slug, which the old whitelist silently accepted and wrote as a dangling reference; create a matching event type to keep using it - new GET /api/v1/event-types (read scope) so API-token clients can discover valid slugs; OpenAPI enum replaced accordingly - standalone contract→event conversion no longer hardcodes event_type: 'wedding' — it resolves via crm_default_event_type, then the catalog catch-all, same chain as quote→event conversion - resolveDefaultEventType moved from quoteService to eventTypeService for shared use (no behavior change)	2026-07-15 22:31:28 +02:00
Paul Nothaft	93301002ba	refactor: move v1 API + CRM event-type un-hardcoding to a follow-up PR ... Keeps #801 scoped to the setup-wizard event-types feature and its load-bearing guards. The v1 validator/discovery endpoint and the contract-conversion default fix ship separately so the public-API behavior change gets its own review weight.	2026-07-15 22:30:00 +02:00
Paul Nothaft	f8ba669716	fix(event-types): harden setup window + catalog validation (codex review) ... Three review rounds on PR #801; fixes in response: - isValidEventType: live catalog is authoritative when it has rows — a deleted or deactivated slug no longer validates via the legacy fallback (fallback now only serves an empty-catalog install) - deleteEventType: refuse deleting the last (and last ACTIVE) type; updateEventType: refuse deactivating the last active type (unknown slugs are rejected since the validator change, so an empty active catalog would brick event creation) - setup window fails closed: only an explicit stored `false` opens it (a portable-backup restore can leave the key absent) and a normal admin login durably closes it (abandoned-wizard case) - reserved bootstrap keys (setup_wizard_completed, setup_token) are stripped from ALL generic settings upserts (/general, /security, /analytics, /seo) so the marker is genuinely one-way - wizard step: deletes ordered so the catalog can never end up empty, and a genuinely failed system-type deletion reloads the list and stays on the step instead of advancing past the only window in which it can be retried - CreateEventPage: snap the hardcoded initial 'wedding' selection to the first active type when the catalog no longer contains it - v1 API: new GET /event-types (read scope) so token clients can discover valid slugs; OpenAPI enum replaced with the live-catalog description	2026-07-15 22:21:20 +02:00
Paul Nothaft	7eb6357b4a	feat(setup): event-types step in first-run wizard + un-hardcode event type deps (#800) ... Fresh installs can now shape the event-type catalog during the setup wizard — rename, delete or replace the seeded defaults while nothing references them. On existing installs system types stay protected. - New wizard step between features and config: edit name/URL prefix, remove, or add types; defaults shown as recommendations - setup_wizard_completed app setting (migration 161): seeded true when an admin already exists, false on fresh installs; POST /api/setup/ complete (adminAuth) flips it when the wizard finishes - deleteEventType: system types deletable only while the flag is unset; in-use check extended to quotes; per-type reminder template (event_reminder_<slug>) is deleted with the type - reminder-template self-heal no longer resurrects templates for slugs removed from the catalog - v1 API event creation validates event_type against the live catalog instead of a hardcoded whitelist (custom types were rejected; the never-seeded 'family' slug is no longer silently accepted) - contract→event conversion resolves the event type via crm_default_event_type / resolveDefaultEventType instead of hardcoding 'wedding' (resolveDefaultEventType moved from quoteService to eventTypeService for reuse)	2026-07-15 21:30:23 +02:00
Luca	a4b4485d32	fix(categories): address PR #790 review — event ownership, migration renumber, nits ... - 🔴 Event ownership: GET /event/:eventId and DELETE /reorder/:eventId now use requireEventOwnership; POST /reorder (event_id in body) gets the equivalent inline check (super_admin bypasses; others limited to owned/ownerless events). New test covers a settings.edit-holding non-super_admin blocked (403) on all three per-event routes. - 🔴 Migration renumber: 158→159, 159→160 (upstream #788 already took 158); headers + the test's require path updated. - 🟢 Nits: stale inline "Drag the arrows" fallback → "Use the arrows" (matches en.json; control is click-only); invalid bg-accent-dark/150 → bg-accent-dark.	2026-07-10 20:10:40 +02:00
Luca	8d0a946478	test(categories): integration tests for layered category ordering (#782) ... Real-DB coverage: migration 158 backfill; global default reorder + a non-customised event following it; per-event override + isolation from other events; override accepts globals / rejects a foreign event's category; reset clears the override; create appends.	2026-07-10 16:24:17 +02:00
Luca	2c7b351458	fix(workflows): backfill existing invoices + anchor grace to due date when dunning is enabled (#750) ... Enabling the invoice-dunning built-in suppressed the legacy reminder ladder but only created runs for invoices sent AFTER enabling — already-sent unpaid invoices got dunned by neither. Now: - Turning dunning ON enrolls every open sent/overdue unpaid invoice via emitWorkflowEvent('invoice.sent') (engine.backfillDunningRuns(), wired into the workflow enable toggle). Idempotent via the per-(flow,entity) dedup. - The grace wait is anchored to the invoice's due date: computeWakeAt now treats { untilVar, delayDays } as "var + offset" (was var-only OR now+offset), and the built-in's waitGrace becomes { untilVar: 'dueDate', delayDays: firstDays } (seed v6 -> v7). An already-overdue invoice duns on its real timeline instead of restarting a fresh grace clock. Note: the due-date-anchored graph applies to freshly seeded built-ins; an already-admin-enabled dunning workflow still enrolls via backfill but keeps its current grace timing until re-seeded.	2026-07-06 21:57:23 +02:00
Paul Nothaft	f564b38c5a	Merge remote-tracking branch 'origin/main' into refactor/codebase-cleanup ... # Conflicts: # backend/src/routes/adminEvents.js # backend/src/routes/protectedImages.js # frontend/src/pages/admin/EventDetailsPage.tsx	2026-07-03 11:54:01 +02:00
Paul Nothaft	cdbfb514bd	test(auth): MFA unit + route + CLI coverage (39 tests) ... mfaService unit (encrypt/decrypt, TOTP, single-use recovery, isEnrolled), adminMfa HTTP (enroll/challenge/verify/recovery/disable; super_admin enrollment guards #735), and reset-admin-mfa.js CLI.	2026-07-03 11:37:51 +02:00
Paul Nothaft	d44ead41c5	test: add smoke tests for invoiceService, adminEvents routes, backupService ... 26 tests as a safety net ahead of decomposition — invoice create/list/ status transitions, adminEvents CRUD via Supertest+SQLite, backup config parsing and manifest validation.	2026-07-03 07:49:45 +02:00
Luca	fa7665c5b1	fix(backup): address .picpeak review — table filter, superuser guard, tests ... From the-luap's review: - Import no longer trusts manifest.tables blindly. It now intersects the manifest's table list with the real data tables of THIS database (listDataTables(), which already excludes knex_migrations/_lock) and drops anything else. A crafted/corrupted .picpeak listing knex_migrations or a non-existent table can no longer wipe it; skipped tables are logged. - The Postgres session_replication_role='replica' SET (needs superuser) is now wrapped: on a managed-PG non-superuser it fails BEFORE any rows are deleted (transaction rolls back) and surfaces a clear, actionable 400 instead of a cryptic permission error. - Export: on an archiver error, the temp out dir (a partial plaintext-secret archive) is now removed instead of orphaned. Tests (+4, now 26): engine-mismatch rejection, forward-only newer-refused, non-picpeak rejection, and files/ restored + filesRestored asserted.	2026-07-03 01:39:13 +02:00
Luca	2920d82186	feat(backup): .picpeak import/restore (full override, keeps current account) ... Receiving half of the roundtrip. picpeakImportService.importFromPicpeak(): - Validates the manifest: rejects non-picpeak files, a newer format, an engine mismatch (pg↔pg / sqlite↔sqlite only), and a backup from a NEWER schema than this instance (forward-only). knex_migrations absence is tolerated (test harnesses). - Snapshots the current logged-in admin, then wipes + reloads every table from the backup NDJSON in one transaction with FK enforcement suspended (pg: session_replication_role=replica reset before commit; sqlite: defer_foreign_keys). knex_migrations is never touched, so the target's schema/migration state is preserved. - Re-injects the current account so the operator is never locked out; a backup admin colliding on email is overwritten with the current creds. - Restores files/ into storage and detects external-media references so the caller can prompt to reconfigure the mount. Roundtrip integration test proves: backup data restored, current account survives a full override (different email → added), and the email-collision case keeps the operator's password.	2026-07-02 19:35:48 +02:00
Luca	38b3aef63d	feat(backup): .picpeak portable export (engine-neutral logical snapshot) ... First half of the GUI-only backup roundtrip. Adds a self-describing ".picpeak" archive that can be downloaded from one instance and (later) re-uploaded to another via the web UI only. - picpeakExportService.createPicpeak(): dumps every table as NDJSON (tables introspected at runtime — no hardcoded list, won't rot), plus a manifest (format version, app version, DB engine, latest migration, per-table row counts + checksums, includePhotos, contains_secrets), plus files/ (business-docs + uploads always; original gallery photos only when includePhotos). NDJSON is engine-neutral so the target rebuilds schema via migrations then loads rows — enabling pg↔pg / sqlite↔sqlite and forward-only auto-migrate. - GET /admin/backup/picpeak/export?includePhotos= streams the file and sets X-Picpeak-Contains-Secrets (the file holds plaintext SMTP pass, admin hashes, API keys — the UI must warn). - Purely additive: no existing backup/restore path is touched. Integration test proves the archive shape, knex-table exclusion, and row-count/NDJSON consistency (85 tables on the seed schema).	2026-07-02 19:13:58 +02:00
Luca	b0912c7427	feat(setup): validate setup token at step 1 before advancing ... Previously "Continue" on the token step only checked the field was non-empty; a wrong token wasn't caught until the final submit, after the user had filled in email + password. Add a non-burning verify: - backend: POST /setup/verify-token constant-time compares the token without consuming it (createInitialAdmin still claims it atomically on submit), gated on no-admin-exists and rate-limited like /setup/admin. - frontend: step-1 "Continue" calls verifyToken and only advances on a valid token; a wrong token shows the invalidToken error on the field, 429 -> too-many-attempts, 409 -> redirect to login. Adds integration tests for accept-without-burn / reject / closed-once-set.	2026-07-02 15:18:22 +02:00
Luca	286975dc52	fix(setup): address PR #714 review — password UX, script token, race, nits ... Blockers: - SetupPage now mirrors the server password rule (>=8 with upper/lower/digit) so a green client isn't bounced by the server; server errors carry a `field` (routes/setup.js) that the client maps to a translated key instead of rendering raw English. New i18n: setup.invalidToken, setup.passwordRequirements. - picpeak-setup.sh: the ADMIN_CREDENTIALS.txt block no longer dead-ends on the wizard path — when no legacy admin was seeded it prints the one-time setup token (from data/SETUP_TOKEN / docker compose logs) and points at /setup. Concern: - createInitialAdmin creates the admin + burns the token in ONE transaction, atomically claiming the token (null-if-present, expect 1 row) so a double-submit can't create two super_admins. Cross-DB (whereNotNull, trx-only writes). Added a concurrency test. Nits: - SetupPage redirects to /login when /setup/status errors (no form flash on a configured instance). - Dropped the unused DATABASE_URL from docker-compose.yml. - Documented why secrets are chmod 644 (three different reader users).	2026-07-02 13:21:55 +02:00
Luca	415bffa04c	feat: zero-config first run — in-browser admin bootstrap + auto-generated secrets ... Fresh installs need nothing in .env. See PR description for the full feature.	2026-07-01 14:49:18 +02:00
Paul Nothaft	56c2386c90	feat(gallery): branded URL shortener — /s/<slug> with OG injection (#699) ... Issue 3 from #699 (@alexvaltchev's report): expose a custom-named short URL per event that bots scrape for OG previews and browsers redirect to the underlying gallery. WhatsApp / iMessage / Facebook cache the OG metadata by the URL they crawl, so the SHORT URL becomes the cache key — admins can rotate or split-test underlying gallery URLs without re-pushing a fresh link to clients. Additive feature; no existing route, table, or column is modified. ## Backend - `gallery_short_urls` table (migration 150): id, short_slug UNIQUE, event_id FK CASCADE, target_path TEXT, created_by/at, hit_count, last_hit_at, deleted_at/by. hasTable-guarded so the migration is idempotent on re-run. - `src/services/galleryShortUrlService.js` — validator + CRUD + resolver. Slug rules: `/^[a-z0-9](?:[a-z0-9-]{0,62}[a-z0-9])?$/`, reserved blocklist (admin, api, auth, gallery, og, s, login, ...). target_path snapshots at create-time from the event + global short-URL toggle, so a later flip of the toggle does NOT silently change where existing short URLs resolve. - `src/routes/adminShortUrls.js` — `GET/POST /api/admin/events/:eventId/short-urls`, `DELETE /api/admin/short-urls/:id`. Structured errors: 400 INVALID_SLUG, 409 SLUG_TAKEN (with `suggested`), 404 EVENT_NOT_FOUND. Gated by events.view / events.edit + requireEventOwnership. - `server.js` /s/:shortSlug public route. Bot UA → server-render the same OG metadata the existing /og/gallery/<slug> handler produces, then override og:url to point at /s/<shortSlug> itself (cache-key invariant — social platforms key by the URL they scrape). Browser UA → 302 to target_path. Soft-deleted slug → 410 Gone (intentional-delete signal, distinct from 404 unknown slug). Hit accounting is fire-and-forget. ## Frontend - `services/shortUrls.service.ts` — list/create/remove. - `components/admin/ShortUrlsCard.tsx` — per-event card on the EventDetailsPage. Form for custom or auto-generated slug, list with copy-to-clipboard + soft-delete. SLUG_TAKEN error surfaces the service's `suggested` slug with a "use suggested" button. - i18n: events.shortUrls.* added to EN + DE. ## Tests 78 new tests, all passing: - `__tests__/utils/galleryShortUrlValidation.test.js` (48) — pure- function tests for validateSlug: accepts/rejects, reserved-slug blocklist, path-traversal + URL-injection vectors. - `__tests__/integration/galleryShortUrls.test.js` (19) — service layer against a real SQLite DB. Covers custom + auto-generated slugs, collision + SLUG_TAKEN + suggested, target_path snapshotting (backward-compat invariant), soft-delete + slug rotation, hit counting. - `__tests__/integration/galleryShortUrlRoute.test.js` (11) — HTTP-level: 302 redirect for browser UA, 200 + OG HTML for bot UA, og:url canonical points at /s/<slug>, 410 for soft-deleted + orphaned events, 404 unknown + malformed. Regression sweep: 47 existing migration-chain integration tests still pass; migration 150 is additive only. ## Backward compatibility - Existing `/gallery/<slug>`, `/gallery/<32-hex-share-token>`, `/gallery/<slug>/show/<token>`, `/og/gallery/<slug>`, `/og/gallery/<slug>/cover` routes are untouched. - The `/s/` namespace is new; no existing route lives there. - Migration 150 only ADDs the new table — no ALTERs on existing schema, no destructive changes. - target_path is snapshotted at create-time so flipping the global "Use short gallery URLs" setting after a short URL exists does NOT change where that short URL resolves.	2026-06-30 16:30:13 +02:00
Luca	539a83711d	fix(workflows): defer quote.accepted/declined emit until the 15-min response window locks ... The customer's accept/decline can be toggled for crm_quotes_accept_window_minutes (default 15) before it locks, and the public page promises exactly that. But the booking workflow fired on the FIRST accept click and immediately converted the quote (status -> 'converted'), so a decline within the window was rejected ('Quote cannot be responded to in status converted') — the grace period was dead on arrival. recordResponse / adminAcceptQuote now DEFER the workflow emit while the toggle window is open; the new scheduler sweep finalizeQuoteResponses fires the FINAL status once response_locked_at passes (idempotent via the new quotes.workflow_response_emitted_at column, migration 149). A response recorded with the window already closed (0-min window, or admin decline which locks immediately) still emits inline. So toggling accept->decline->accept inside the window converts at most once, for the final state, after the customer's grace period — and a plain decline never converts. Trade-off: with the hourly CRM scheduler, the booking flow now starts up to ~1h after the window locks instead of instantly. Acceptable — the flow gates on admin review anyway, and the alternative (graph-level wait) wouldn't reach already- enabled built-ins (admin_toggled_at blocks re-seed). Adds a finalize sweep test (deferred while open, fires + stamps once locked, idempotent).	2026-06-28 14:30:26 +02:00
Luca	882cfc0661	fix(workflows): held booking invoices are 'scheduled', not 'pending_delivery' — so send_document can issue them ... A quote with no explicit payment timing falls back to a single after_delivery installment. spawnInstallmentInvoices marked those 'pending_delivery' even in hold mode, so the booking flow's send_document -> sendInvoice threw 'Cannot send invoice with status pending_delivery', the run failed, and no invoice email went out (the symptom: approve the quote->invoice flow, receive nothing). In hold mode the flow's review gate + explicit send_document IS the delivery release, so a held invoice is always 'scheduled' (editable + sendable) regardless of trigger; scheduled_send_at stays null so the scheduler never auto-sends it. Non-hold after_delivery invoices keep 'pending_delivery' as before. Adds a regression test (default after_delivery term -> draft -> scheduled+null).	2026-06-27 12:36:35 +02:00
Luca	9414b42b7f	feat(workflows): implement remaining stub actions (prepare_quote, prepare_gallery, reserve_date) ... These were the last guard-stubbed actions — offered in the builder palette but refused on enable. Now all three are real, backed by existing converters: - prepare_gallery: alias of prepare_event (a gallery IS an event in picpeak). - reserve_date: convertToEvent({ skipInvoices: true }) — a pure draft date hold with no money documents (new skipInvoices option on convertToEvent). - prepare_quote: createQuote (customer entity) or duplicateQuote (quote entity), producing a status='draft' quote; idempotent via ctx.vars.preparedQuoteId. With no stubs left, the enable-guard switches from a hardcoded DOCUMENT_ACTIONS list to a registry lookup: an action node whose config.action has no registered handler is unimplementable. This can't drift from what the engine can run and also catches typo'd/future actions. (Fixes the enable-route node mapping to carry node.type so the action-node filter matches.) Extends the single-connection SQLite in-trx deadlock fixes to the quote-create path (prepare_quote runs unattended): nextQuoteNumber reads getAppSetting through trx, createQuote logs via trx and hoists its hasColumnCached schema-drift checks before the transaction. Adds tests for reserve_date (no invoices), prepare_quote (draft, no deadlock), and registry coverage; retargets the enable-guard refusal test at a genuinely unregistered action. Full backend suite: 985 passed, 1 skipped.	2026-06-27 00:49:35 +02:00
Luca	4faf5a344a	feat(workflows): implement prepare_event so booking_full/booking_simple are enableable ... The booking_full / booking_simple flows go prepare_event -> prepare_invoice, but prepare_event was still a guard-stub, so enabling either flow returned 409 'uses actions that aren't implemented: prepare_event'. prepare_event now calls convertToEvent({ hold: true }): convertToEvent already creates the event as is_draft=true AND schedules its invoices, so this creates those invoices on HOLD (scheduled_send_at NULL) and stashes their ids in ctx.vars.preparedInvoiceIds. The downstream prepare_invoice already short- circuits on a populated preparedInvoiceIds, so it ADOPTS the event's held invoices instead of calling convertToInvoiceOnly again (which would both double-create and throw ALREADY_CONVERTED_TO_EVENT). The review gate, the wait-until-event-date, and send_document then issue those same invoices. send_document(event)=publish is intentionally left a graceful skip — the gallery is published manually after photos are uploaded, not auto-published on an empty draft. convertToEvent gains the same single-connection SQLite deadlock fixes as convertToInvoiceOnly (getAppSetting reads through trx; logActivity moved after commit) since prepare_event runs unattended, returns invoiceIds (incl. the idempotent already-converted re-entry, which recovers them by event_id), and removes prepare_event from the enable-guard list. Adds a convertToEvent hold-mode test (draft event + held invoices + quote linkage) and updates the enable-guard test to a still-stub action (prepare_gallery). Full backend suite: 982 passed, 1 skipped.	2026-06-27 00:08:05 +02:00
Luca	cf424efb4a	feat(workflows): wire booking document actions (prepare_invoice/contract + send_document) ... 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.	2026-06-26 23:48:12 +02:00
Luca	3ccaed06a2	feat(crm): pre-event reminder falls back to the assigned customer account ... When an event has no inline customer_email/host_email but has customer account(s) assigned (event_customer_assignments), the pre-event reminder now sends to those registered customers instead of skipping with no_recipient. Recipients sent to an assigned account are queued WITHOUT eventId so the language resolver uses the customer's preferred_language (vs the event's language for inline-email sends). Applies to both the flow path (sendReminderForEvent) and the legacy pass. The gallery-ready mail deliberately does NOT fall back to accounts — only the reminder does. Test covers the no-inline-email + assigned-customer case.	2026-06-25 20:01:40 +02:00