stable
70
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Paul NothaftandPaul Nothaft
|
222b144eed |
fix(gallery): bound the cached-zip builder's reads and cap rebuild concurrency (stable) (#1421)
Backport of the two halves that landed on main, which together are what the issue asked for: a per-build read cap plus a separate build-concurrency cap. Per build: the builder opened one storage read per photo and handed each to archiver, which drains them one at a time — so every read past the one being written parked an S3 socket holding unread bytes. Nothing reclaimed them: archiver's abort() does not touch source streams, and the SDK arms its socket timeout on a 3s delay then clears it as soon as response headers land, so a fast response never gets one. Reads are now capped at two and destroyed on every exit, including the invalidated one, which previously cleaned up nothing at all. Invalidation also cancels an in-flight build directly rather than leaving a note for the loop, which matters once the loop can be parked waiting for a slot a stalled archive will never free. Across builds: invalidateAll() invalidates every event holding a cached zip and each invalidate() arms its own debounce timer in the same tick, so they all fired together and every one started building at once. Background rebuilds now run two at a time. The cap is on that path only — a foreground generateZip, where a guest is waiting on the download, is never queued behind a burst. Two deliberate differences from the main twins. The read cap uses the shared archiveStreamGuard helper this branch already has rather than main's inline copy — same contract, less duplicated code. And main's stop() drain has no counterpart here because this branch has no stop(), so that machinery is left out rather than carried as dead code. Relates to issue 1399 Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
b663f2a994 |
fix(backend): contain and sanitize the SQLite restore source path (#1393)
The restore flow accepted an unvalidated database.backup_file from the manifest (absolute paths and traversal both worked, and no containment check enforced the configured backup root), then interpolated it unescaped into a `sqlite3 .restore '<path>'` command, letting an attacker-chosen source file replace the live database. Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
c008f43a7e |
fix(backend): validate the S3 endpoint host before the restore download (stable) (#1391)
* fix(backend): validate the S3 endpoint host before the restore download
downloadFileFromS3() built an S3StorageAdapter and called .download()
directly, skipping the isHostAllowed() private-IP/DNS-rebinding guard
that testConnection() applies elsewhere — an admin with backup.restore
could point the configured S3 endpoint at an internal/metadata address
for unauthenticated egress via the server.
Backport of
|
||
|
Paul NothaftandPaul Nothaft
|
167755fdec |
fix(backend): use the strong password generator for resets and enforce must_change_password (#1396)
Stable backport of
|
||
|
Paul NothaftandPaul Nothaft
|
050eaf6481 |
fix(backend): require actor to hold every permission of a role they grant (stable) (#1380)
* fix(backend): require actor to hold every permission of a role they grant Any admin with `users.edit` could grant an arbitrary non-super_admin role — including one carrying far more permissions than they themselves hold — via PUT /api/admin/users/:id. The role-change path never called the existing assertActorMayGrant() guard that already protects role create/edit. * fix(backend): require actor to hold every permission of a role they grant Backport of the same fix on main (#1378), adapted to stable's schema — stable has no custom-role-creation service or roles.manage containment helper yet, so assertActorMayGrant() is added locally in userManagementService.js instead of reused from elsewhere. Any admin with `users.edit` could grant an arbitrary non-super_admin role — including one carrying far more permissions than they themselves hold — via PUT /api/admin/users/:id. * fix(backend): apply the same role-grant guard to admin invitations (stable) Stable counterpart of the main-branch fix: createInvitation() only blocked granting super_admin — the same users.create-holder-can-invite- into-any-role escalation that updateAdminUser() was fixed for (GHSA-rv8w-m6mx-7j4q) was still open via POST /admin/users/invite. Reuses stable's local assertActorMayGrant(). --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
33d6904e66 |
fix(backend): reject a replayed TOTP code within its validity window (stable) (#1398)
* fix(backend): reject a replayed TOTP code within its validity window verifyTotp() was stateless — otplib's window:1 tolerance meant the same 6-digit code could complete two independent logins inside its ~90s validity window. Track each admin's last-consumed step and reject a code that doesn't advance past it. * fix(backend): make the TOTP replay-tracking persist atomic Backport of the same fix on main: the persist for two_factor_last_used_step is now a conditional UPDATE (only advances the step, checked via affected-row count) instead of a plain unconditional write, closing a TOCTOU race where two concurrent requests carrying the same captured code could both pass before either UPDATE landed. --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
b2ae7f1c50 |
fix(backend): shorten payment-check token TTL and notify admin on use (#1392)
The unauthenticated payment-check magic link (intentional, matches
publicQuotes.js) had a 30-day token lifetime and wrote to the invoice
ledger silently. Shortened the TTL and added a best-effort admin
notification on every write via this route, so the no-login
convenience stays but an admin always sees the action happen.
(cherry picked from commit
|
||
|
Paul NothaftandPaul Nothaft
|
1316ed05b3 |
fix(video): try metadata extraction and thumbnail generation independently (#1372)
* fix(video): try metadata extraction and thumbnail generation independently processUploadedVideo() gated everything behind isValidVideo(), which rejects the whole video if ffprobe can't read even one of duration/width/height -- common on some iPhone/Lightroom-exported MP4s (issue 1370). Callers (photoProcessor.js's processPhoto and processUploadedPhotos) already catch that throw and fall back to a static placeholder thumbnail plus a metadata-only retry (codex review of #845), but that fallback never got a REAL thumbnail even when generateVideoThumbnail() would have succeeded on its own -- thumbnailing doesn't need valid duration/width/height, it just seeks and grabs a frame. processUploadedVideo now tries metadata extraction and thumbnail generation independently, keeping whichever succeeds instead of discarding both on a single failed field. The callers' existing throw handling stays as a backstop. Also: extractVideoMetadata stored duration as 0 (not null) whenever ffprobe had no duration field, masking "unknown" as a fake real zero-second clip and defeating downstream `duration != null` checks meant to skip an untrustworthy value. Relates to issue 1370 * fix(video): fall back to the SVG placeholder when thumbnail generation fails processUploadedVideo could return success with thumbnailKey: null when only thumbnail generation failed. The gallery grid (GridGalleryLayout/JustifiedGalleryLayout) falls back to `photo.thumbnail_url || photo.url` when there's no thumbnail, so AuthenticatedImage downloaded the full original video and tried to render it as an <img> -- a broken tile and a potentially huge fetch just from opening the gallery. Falls back to the same ffmpeg-free SVG placeholder the callers already generate for a total processing failure, so a bare thumbnail-generation failure degrades to that placeholder too, never to "no thumbnail at all". Found by codex review. * fix(video): avoid a SQLite connection deadlock in the placeholder fallback generateVideoPlaceholder() unconditionally called getThumbnailSettings(), which queries the database directly (not through any active transaction). videoProcessor.js's new placeholder fallback can run from inside processUploadedPhotos' open per-file SQLite transaction (chunked video upload) -- knex's default SQLite pool has exactly one connection, so that second, un-transacted query deadlocks against the transaction holding it, timing out after acquireConnectionTimeout (60s). Reproduced directly against an isolated SQLite db. generateVideoPlaceholder now skips the settings lookup entirely when the caller supplies explicit width/height, and the video fallback passes the same DEFAULT_THUMBNAIL_WIDTH/HEIGHT the settings lookup would have fallen back to anyway (now exported for reuse). Found by codex review. * fix(video): throw when neither a real thumbnail nor the placeholder can be produced processUploadedVideo returned success with thumbnailKey: null when both the real thumbnail AND the SVG placeholder failed -- a total, systemic failure (storage backend down, disk full), not a quirk of one file. On stable, which doesn't have the #845 call-site fallback, this silently completed the video with no thumbnail at all instead of the retryable 'failed' status a throw here produces. On main, the pre-existing #845 fallback already absorbed this exact case (no behavior change there) -- verified against codex's own git-blame check of the pre-PR stable code before applying this. Now throws in that case, restoring the pre-existing "let the caller mark it failed and retryable" behavior for a genuinely unrecoverable video, while keeping every partial-failure case (the vast majority) resolving with whatever succeeded. Found by codex review. --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
5559cd333d |
fix(images): respect EXIF orientation in thumbnails, heroes, previews and watermarks (#1185) (#1202)
Stable twin of #1194. generateThumbnail, generateHeroImage and generatePreviewImage went straight from sharp(imagePath) to .resize(), so a photo whose Orientation tag is not 1 — routine for portrait shots on bodies that tag rather than rotate the sensor data — was resized from the raw frame and came out sideways. The same pipelines then call .withMetadata(false), stripping the tag from the output, so nothing downstream could correct it either. The download path already had this right, which is why the same photo looked correct on download and rotated in the gallery. watermarkService had it too, and it is the one a guest actually sees: gallery.js serves photos.watermark_path ahead of the original when branding watermarking is on. Two details there — metadata() is read from a separate unrotated handle, because .rotate() does not change what it reports and every use of those numbers is positioning; and the composite offsets are floored, because getPositionCoordinates returns fractional pixels, sharp rejects them, and applyWatermark catches its own error and silently returns the image unwatermarked. The rotate is unconditional in the thumbnail and hero generators — neither passes `animated: true`, so both already flatten a multi-frame source and guarding there would protect an animation that was being discarded anyway. generatePreviewImage keeps the guard, since it genuinely does preserve animation. photos.width/height were stored from sharp's metadata, which reports pixels as STORED, not displayed. For orientation 5-8 those are swapped, so a portrait photo landed in the database as landscape and masonry sized its tile with the wrong aspect ratio on top of the image being unrotated. A shared orientedDimensions() helper now does the conversion at all eight image ingest sites. The video path is deliberately untouched: its dimensions come from ffprobe, where EXIF orientation does not apply. Existing rows keep their pre-rotation dimensions until reprocessed; the backfill for those is #1199 on main and is not ported here yet. Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
ac7ef266dc |
fix(admin): make "Storage used" report storage used (#1164) (#1177)
* fix(admin): make "Storage used" report storage used (#1164) Stable twin of #1170. The tile summed photos.size_bytes — the catalogued size of the ORIGINALS, which in reference mode live on external storage and have no relationship to the disk PicPeak runs on. The reporter's tile read ~80 GB against 21 GB of real usage. Worse than the label: the same number drove the soft-limit warning bar and, via /storage/info, the recommended soft limit — so a reference-mode install got a disk-capacity recommendation computed from bytes that are not on the disk. - new localStorageUsage service walks the storage root and reports the total plus a breakdown. Walking rather than summing DB columns is the point: thumbnail/preview/hero rows record a key and never a byte count, and orphans from a deleted event or an interrupted import are real bytes. - the external media root is excluded when it sits inside the storage root. Its compose default is <storage>/external-media, where the NAS is bind-mounted — a plain directory, not a symlink — so walking it would put every referenced original back into a figure whose purpose is to leave them out. Symlinks are not followed either. - .download-cache gets its own line: it lives inside the event directory, so the naive rule files a multi-GB zip as photography. - concurrent cold-cache callers share one walk; the dashboard, /storage/info and the sidebar are routinely requested together. - S3 installs keep the catalogued figure and the walk is skipped before it runs, since the objects are in the bucket and STORAGE_PATH holds only incidental local files. - an absent measurement reads as "unavailable" and a partial one is marked `+` across the dashboard, analytics, sidebar and status tab — a floor silently compared against a soft limit reads as "safely under". Verified on this branch: 11 new service tests, dashboardScope updated for the changed contract, full suite leaves the same 5 pre-existing failures as origin/stable. Frontend 20 files / 104 tests, tsc clean. * fix(admin): tell "no disk to measure" apart from "the measurement failed" (#1164) External review found both of these on this branch. Both were reported as `storage_measurement: 'catalog'`, so a failed local walk made the dashboard claim the objects live in S3. They are different things — one is a fact about the install, the other is a fault — and there is now an `unavailable` state for the second. The analytics percentage could reach the billions. `safeSoftLimit` fell back to `storageUsed || 1`, and on S3 that is null → 1, while the figure beside it came from `catalogedBytes`. An editor or viewer holds `analytics.view` but not `settings.view`, so `/storage/info` 403s for them and `storageInfo` is undefined — which is exactly when that fallback fires. It now falls back to the measured figure, and suppresses the percentage entirely when there is no real limit rather than dividing usage by itself and always reading 100%. --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
9ffbe2f98f |
fix(previews): preserve alpha and animation in the preview tier (#1176)
* fix(gallery): stop the lightbox loading originals to display a photo (#1166) Stable twin of #1169. The lightbox read preview_url, which the server only emits once an admin has flipped lightbox_preview_enabled — off by default. So a stock install fell straight through to url, the untouched original: a reporter measured 16.5 MB for a photo whose preview is 345 KB. The lightbox renders its neighbours too, so opening one photo pulled three originals. slideshow_url is the same /preview/:id URL, watermark query included, and has been emitted unconditionally for images since #1015. Preferring it fixes every existing install with no migration and no admin action. Two other surfaces bypass PhotoLightbox entirely and had the same bug: - premium galleries build their own slides with `src: photo.url`. Fixing that also required carrying the photo id on the slide, because the download handler recovered the photo by matching slide.src against photo.url — a derivative src would have made Download a silent no-op. - the Story layout rendered the full original as its GRID TILE, at object-cover in a small card, and its hero rendered one as a full-bleed background when hero_url exists for exactly that. Cards now use the preview tier (not the thumbnail: thumbnail_fit is seeded to 'cover', so a thumbnail would be cropped a second time and reframe every photo) and only load once within 200px of the viewport, since every card mounts at page load. GIF, APNG and PNG keep the original: generatePreviewImage encodes JPEG, which has neither a second frame nor an alpha channel. The backend fix that removes this list is the next commit in this stack. Divergence from the main twin: no responsive `?w=` tiers. #1095 is main-only, so `lightboxImageUrl` here selects a URL and nothing more. It lives in `imageTiers.ts` under the same path main uses, so that backporting #1095 later merges into this file rather than landing beside it. Verified on this branch: 8 new tests; frontend suite 21 files / 113 tests, tsc clean. * fix(gallery): make the Story hero fix actually work on external galleries (#1166) External review. Same two fixes as the main twin. hero_url was inert for external media. ensureHeroImage only ever called resolvePhotoStorageKey, which returns null for external/reference photos by design — and that null was handed straight to withLocalCopy, which throws, so the hero route caught it and redirected to the full ORIGINAL. #1078 fixed exactly this shape for ensurePreviewImage and nobody carried it across. It stayed invisible until this PR pointed the Story hero at hero_url: on a managed gallery that is a real saving, on a reference-mode gallery it quietly changed nothing. Needed one extra piece here that main already had: generateHeroImage on this branch ignores outputBasename and always derives the key from the source basename, so two events referencing the same NAS filename would clobber each other's hero. It now honours the option, matching generateThumbnail and generatePreviewImage. The format bypass trusted mime_type, which is not trustworthy: migration 039 backfilled every pre-existing photo to image/jpeg regardless of what it was, and adminExternalMedia inserts rows with no mime_type at all — so a mislabelled PNG sailed past the guard and came back flattened. It now checks the filename extension as well. * fix(previews): preserve alpha and animation in the preview tier Stable twin of #1171. Stacked on the #1166 twin, whose format bypass this removes. generatePreviewImage encoded JPEG unconditionally. JPEG has no alpha channel and no second frame, so a transparent PNG came back flattened onto a solid background and an animated GIF came back as its first frame — for every consumer of this tier, not just the lightbox. It was only invisible by default because the lightbox served originals. Sources with alpha, or more than one page, are now encoded as WebP, which carries both and is still far smaller than the original. Ordinary photos stay JPEG. - the output extension matches what was written. A PNG source previously produced `preview_foo.png` holding JPEG bytes; harmless while the route hard-coded image/jpeg, wrong once the encoding varies. Existing keys keep working — they are still JPEG and still served as such. - the preview route derives Content-Type from the key. With nosniff set, mislabelling would show a broken image rather than being silently corrected. The watermark branch re-encodes to JPEG and now says so. The frontend guess-by-MIME goes away entirely, including the case it could never get right: a still and an animated WebP declare the same type. Divergence from the main twin: no width-tier case. The responsive `?w=` renditions (#1095) are main-only, so this branch has a single canonical preview per photo. Verified on this branch: 5 new backend tests against real Sharp output; frontend 21 files / 114 tests; full backend suite leaves the same 5 pre-existing failures as origin/stable. * fix(previews): retire the legacy preview keys, and stop mislabelling watermarked ones External review. Same two defects as the main twin. Legacy keys collide with the new naming. The old generator kept the SOURCE basename verbatim while always writing JPEG, so a `.webp` upload produced `previews/preview_shot.webp` holding a JPEG. The claim that pre-existing keys have no .webp suffix was simply wrong. The route now derives Content-Type from the key and the response carries nosniff, so every photo uploaded as WebP would have rendered as a broken image in the lightbox. Legacy `.png` keys are wrong the other way: flattened JPEGs of what may have been transparent sources, which isPreviewValid would have let stand forever. Migration 178 clears photos.preview_path outright — all of it, not just the suspicious extensions, because a `.jpg` key can equally be a flattened rendition and nothing in the key says so. Previews regenerate lazily on next view under the new encoder. The watermark branch mislabelled its output. applyWatermark PRESERVES the source format on this branch too (watermarkService.js: png stays png, webp stays webp), and its input is the preview — so the output already matches the key the header was derived from. Forcing image/jpeg mislabelled every watermarked WebP preview, and nosniff means the browser would not correct it. Numbered 178, not 176: this stack does not carry the external-media migrations, but that stack takes 176 and 177 on this same branch, and two files sharing a numeric prefix would be confusing even though both would run. --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
75facb4d67 |
fix(gallery): stop the lightbox loading originals to display a photo (#1166) (#1175)
* fix(gallery): stop the lightbox loading originals to display a photo (#1166) Stable twin of #1169. The lightbox read preview_url, which the server only emits once an admin has flipped lightbox_preview_enabled — off by default. So a stock install fell straight through to url, the untouched original: a reporter measured 16.5 MB for a photo whose preview is 345 KB. The lightbox renders its neighbours too, so opening one photo pulled three originals. slideshow_url is the same /preview/:id URL, watermark query included, and has been emitted unconditionally for images since #1015. Preferring it fixes every existing install with no migration and no admin action. Two other surfaces bypass PhotoLightbox entirely and had the same bug: - premium galleries build their own slides with `src: photo.url`. Fixing that also required carrying the photo id on the slide, because the download handler recovered the photo by matching slide.src against photo.url — a derivative src would have made Download a silent no-op. - the Story layout rendered the full original as its GRID TILE, at object-cover in a small card, and its hero rendered one as a full-bleed background when hero_url exists for exactly that. Cards now use the preview tier (not the thumbnail: thumbnail_fit is seeded to 'cover', so a thumbnail would be cropped a second time and reframe every photo) and only load once within 200px of the viewport, since every card mounts at page load. GIF, APNG and PNG keep the original: generatePreviewImage encodes JPEG, which has neither a second frame nor an alpha channel. The backend fix that removes this list is the next commit in this stack. Divergence from the main twin: no responsive `?w=` tiers. #1095 is main-only, so `lightboxImageUrl` here selects a URL and nothing more. It lives in `imageTiers.ts` under the same path main uses, so that backporting #1095 later merges into this file rather than landing beside it. Verified on this branch: 8 new tests; frontend suite 21 files / 113 tests, tsc clean. * fix(gallery): make the Story hero fix actually work on external galleries (#1166) External review. Same two fixes as the main twin. hero_url was inert for external media. ensureHeroImage only ever called resolvePhotoStorageKey, which returns null for external/reference photos by design — and that null was handed straight to withLocalCopy, which throws, so the hero route caught it and redirected to the full ORIGINAL. #1078 fixed exactly this shape for ensurePreviewImage and nobody carried it across. It stayed invisible until this PR pointed the Story hero at hero_url: on a managed gallery that is a real saving, on a reference-mode gallery it quietly changed nothing. Needed one extra piece here that main already had: generateHeroImage on this branch ignores outputBasename and always derives the key from the source basename, so two events referencing the same NAS filename would clobber each other's hero. It now honours the option, matching generateThumbnail and generatePreviewImage. The format bypass trusted mime_type, which is not trustworthy: migration 039 backfilled every pre-existing photo to image/jpeg regardless of what it was, and adminExternalMedia inserts rows with no mime_type at all — so a mislabelled PNG sailed past the guard and came back flattened. It now checks the filename extension as well. * test(gallery): the hero fixture follows the root-relative relpath contract (#1166) Same fix as the main twin: external_relpath has been resolved from EXTERNAL_MEDIA_ROOT rather than from event.external_path since #1163 landed, and this fixture still carried the base-relative form, so the two tests stopped resolving the moment that stack merged. Production was never affected. --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
2b1c3588ae |
fix(external-media): store external paths from the media root (#1163) (#1174)
* fix(external-media): store external paths from the media root (#1163) Stable twin of #1168. Stacked on the #1162 twin, which supplies deleteDuplicatePhotos. Importing a second folder into an event silently invalidated every photo already in it. external_relpath was stored relative to events.external_path, and every import overwrites that column, so the older rows were rebased onto the new folder. Nothing errored and the grid still rendered — thumbnails are written to local storage during the import while the base path is still correct — so only the things that need the original broke. The reporter had 7547 of 8004 rows pointing into the void. - external_relpath is now relative to EXTERNAL_MEDIA_ROOT, so a row is self-describing and nothing an admin does to the event can move it. - migration 177 folds each event's base into its rows. Where the current resolution is missing it walks up for an ancestor holding a file of the same name AND the size the import recorded — existence alone would let a deleted file adopt an unrelated namesake and serve the wrong original. Rows it cannot place keep resolving where they resolve today, and the probe is skipped entirely when the mount is unreachable. - probing is read-only and runs first; the rewrites and the marker commit together, so an interrupted fold cannot be folded twice. - rewrites are staged through a per-row parking value, because a final path can equal another row's current one; and migration 177 re-throws without the driver's error code, which run-migrations-safe would otherwise read as "schema already exists". - the fold also runs after a .picpeak restore, since knex_migrations is excluded from the archive, and a failure there is reported rather than presented as a clean restore. - drops the duplicate-leaf-segment guess in photoResolver, which papered over this same double-prefixing. Divergence from the main twin: no face-scan requeue reordering. Face recognition is main-only, so the hazard of queueing rows against unconverted paths does not exist on this branch — in picpeakImportService or in restoreService. Verified on this branch: 23 new tests pass, and the four suites carrying base-relative fixtures were updated. Full suite leaves the same 5 pre-existing failures as origin/stable, unchanged. * fix(external-media): the fold's staging value must be storable on Postgres (#1163) External review found this on this branch first; it was on both. The two-pass rewrite parks each row on a temporary value, and that value was written with a leading NUL. SQLite stores NUL in TEXT without complaint; Postgres rejects it outright — "invalid byte sequence for encoding UTF8: 0x00" — so migration 177 rolled back on exactly the installs that need the two-pass repair, and only on the engine most of them run. Restores hit the same wall and reported the conversion as failed. The prefix is ordinary text now. Adds a gated Postgres test alongside the existing picpeakRestorePg one, because a SQLite-only suite structurally cannot catch this class: restoring the NUL makes exactly the two-pass repair case fail with that error, and nothing else. --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
e9fcf4960e |
fix(external-media): one row per external file per event (#1162) (#1173)
* fix(external-media): one row per external file per event (#1162) Stable twin of #1167. Two overlapping import-external runs against the same event inserted every file twice. The route checked for an existing external_relpath and then inserted, with an fs.stat and a sharp().metadata() read sitting in between — a window wide enough for both runs to see "not there". A reporter's event held 8004 rows for 6012 distinct paths. Nothing at the storage layer stopped it: migration 041 created only a NON-unique (event_id, source_origin) index. - migration 176 removes the existing duplicates and adds a partial unique index on (event_id, external_relpath), verified against the catalog afterwards — a failed CREATE INDEX raises 23505 on Postgres, which run-migrations-safe treats as "schema already exists" and would record as applied on an install that never got the index. - dependent rows are removed explicitly rather than by cascade: PicPeak never sets `PRAGMA foreign_keys = ON`, so on SQLite the declared CASCADE is inert and a bare delete strands feedback and access-log rows. Guest feedback moves to the survivor instead of being discarded, keyed on guest identity the way feedbackService defines it, and the survivor's denormalized counters are recomputed. - the route treats a unique violation as a skip, so a writer this process cannot see converges instead of duplicating, and a second import while one is running gets a 409. - a .picpeak taken before migration 176 carries exactly these duplicates, and suspending FK enforcement does not suspend a unique index — so the restore drops the index for the load and rebuilds it after running the same dedupe. Divergences from the main twin, both because the feature is absent here: faces (no faceProcessor, so no purgePhotoFaces reconciliation — the rows are still deleted so nothing dangles), admin marks, transfer membership, and photos.view_count/download_count. The service guards each on hasTable / hasColumn, so those branches simply do not fire. Verified on this branch: 36 new tests pass; full suite leaves the same 5 pre-existing failures as origin/stable, unchanged. * fix(external-media): invalidate the download zip when duplicates are removed (#1162) External review. Same fix as the main twin. The pre-built "download everything" archive still contained the duplicate rows the dedupe had just deleted, so guests kept receiving them. Every ordinary photo-deletion path calls downloadZipService.invalidate for exactly this reason. The columns are cleared rather than the service being called: that service carries debounce timers and a regeneration queue, which a migration should not start. getZipInfo already treats a cleared record as a cache miss and rebuilds on the next request. The stale object is left in storage, as elsewhere. --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul Nothaft
|
dc9e3cdc5e |
fix(thumbnails): regenerate external photos, and stop destroying good ones (#1129)
POST /admin/thumbnails/regenerate resolved every source as storage/events/active/<photo.path> and fs.access'd it. External and reference rows are not there — their originals live under events.external_path — so every one failed the check and was counted as an error, while the UI reported success because the response is sent before the background loop starts. It now goes through ensureThumbnail, which resolves both source kinds and writes thumbnail_path back itself. Nulling thumbnail_path stops it short-circuiting on isThumbnailValid, which matters because the old thumbnail is normally still readable at exactly the moment someone presses regenerate. And the half that destroys data: generateThumbnail deleted the target BEFORE sharp had opened the source, and again in its catch. A source that could not be read — a NAS mount that blipped — left the previous rendition gone and the database pointing at it. Across a bulk regenerate that is the whole gallery. Neither delete was needed: put stages to a temp file and renames atomically, and put is the last statement in the try so no partial object can exist. Also: videos filtered out, and the superseded rendition removed only when the storage key actually moved, compared through the same canonicalisation the backends apply. Stable twin of #1134. |
||
|
Paul NothaftandPaul Nothaft
|
6df42ab22c |
fix(preview): generate lightbox previews for external/reference photos (#1078) (#1080)
* fix(preview): generate lightbox previews for external/reference photos (#1078) Stable twin of the main-line fix. ensurePreviewImage() resolved its source only via resolvePhotoStorageKey(), which returns null for external/reference photos by design — those live on a media mount outside the managed storage tree. The null went straight into withLocalCopy(), which throws, so the preview route fell back to redirecting at the full-size original. Galleries whose photos are all external got no benefit from the preview tier (#492): guests paid 5-12 MB on every lightbox open. Add the external branch ensureThumbnail() already has: resolve via resolvePhotoFilePath() and feed the mount path to generatePreviewImage() directly, with an ext<id>_ output basename. generatePreviewImage() on this branch hardcoded path.basename(imagePath) and ignored options.outputBasename, so it needs the same one-line honouring that generateThumbnail() already does — without it two events referencing the same NAS basename collide on one preview key. Also return null rather than throwing for a row with no source_origin in a reference-mode event, whose mode falls back to the event's. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * fix(preview): select the columns the external branch needs on bulk regenerate POST /api/admin/thumbnails/regenerate-previews selected only id, event_id, path, media_type, mime_type and preview_path, so photo.source_origin was undefined by the time ensurePreviewImage branched on it. Every external row in a reference gallery took the managed path, resolvePhotoStorageKey returned null for it, and the endpoint reported success while generating nothing. Add source_origin, external_relpath and filename to the select, plus a source-inspection test pinning the caller contract and a service-level test showing a column-starved row is indistinguishable from a managed one. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
45ffe64b7c |
fix(storage): write business documents under STORAGE_PATH, not the cwd (#1072)
Stable twin of #1070. persistDocPdf, the invoice sending and reminder writers, both contract signature writers and persistSignatureImage built their targets from `path.join(process.cwd(), 'storage', 'business-docs', ...)` and never consulted STORAGE_PATH. Both compose files pin STORAGE_PATH=/app/storage and the image's WORKDIR is /app, so on a stock deployment the two name the same directory and nothing looked wrong. Point STORAGE_PATH anywhere else and quotes, invoices, Mahnungen, contracts and signature images land outside the configured storage root: missed by the backup walker, invisible to storage accounting, and gone when the container is replaced. assertContractPdfPath moves with them. On this branch the writers and the guard are wrong together, so contract downloads currently work — migrating the writers alone would have introduced PATH_OUTSIDE_STORAGE on every newly generated contract. The guard now resolves through getStoragePath() like the writers, and keeps the legacy cwd root so contracts written before this still resolve; their absolute paths are in the database. Also on the shared resolver: the custom PDF font lookup (a font under STORAGE_PATH/fonts was never found, and the document silently fell back to the built-in face) and the two backup diagnostics, which otherwise inspect a different root than the backup walker when STORAGE_PATH is unset. No migration needed — the persisted path is stored absolute. Verified on this branch, not inferred from main: the new test is 6/6, and contract/quote/invoice/pdf/safePath suites are 213/213 both before and after the change. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul Nothaft
|
bf9bd76278 |
fix(security): vet the destination project when linking a deal (stable) (#992)
Backport of #991. stable carried the identical code path and the same missing guards. A scoped admin could point a quote or contract at a project they do not own — the quote/contract create+update paths pass a body-supplied projectId with no ownership check, and linkDealToProject's lineage guard is skipped when the deal has no event yet. On an ownerless project this escalated to a read once the quote converted to an event. Vetted at the service choke point, ahead of both the null-deal early return and the customer check. 404 PROJECT_NOT_FOUND throughout. super_admin unaffected. |
||
|
Paul Nothaft
|
2d0e6ab2dc |
fix(projects): stop the cockpit offering email controls the API rejects (stable) (#977)
Closes #969 on stable. Backport of #976. The cockpit's email feed rendered preview/resend/cancel/retry/send-now for every mail regardless of role or permission, producing 404s (CRM document mail has no event_id; project ownership does not imply event ownership) and 403s (preview needs events.view, the write actions need email.send). getProjectOverview now stamps each email with an authoritative canAct, mirroring filterOwnedEventIds; created_by is selected only for that check and stripped before the response. A missing canAct reads as false. |
||
|
Paul NothaftandPaul Nothaft
|
fecc18cbc8 |
fix(security): enforce project ownership on project + project-email routes (stable) (#966)
* fix(security): enforce project ownership (GHSA-wrg5, GHSA-93x4) Project routes authorized on generic events.view / events.edit with NO ownership check, so an editor-like admin could enumerate, read, update and aggregate projects belonging to other admins' events. The project email endpoints keyed on an email_queue id alone — any admin with events.view / email.send could preview, resend, cancel or retry ANY queued mail by walking ids. The earlier 'needs a migration, deferred' assessment was wrong in one direction and right in another: ownership IS derivable transitively via events.project_id -> events.created_by, but only for projects that already have a linked event. A brand-new EMPTY project has no derivable owner, which is exactly where the create -> attach flow starts. So migration 167 adds projects.created_by (backfilled from the single linked event owner, skipping ambiguous multi-owner projects) and createProject finally persists the adminId it was already being passed. - ownedProjectIds(): union of the stored owner and the transitive path, so pre-167 rows and new empty projects both resolve. Reads created_by defensively so an instance that hasn't run 167 falls back to the transitive rule instead of throwing. - requireProjectOwnership on detail/update/attach-event/attach-quote/ attach-contract/overview; list filtered by an id allowlist (empty array means 'owns nothing' and must return no rows, hence null-vs-[] care). - POST /:id/events also validates the INCOMING eventId — owning the project is not enough, or an editor could pull a foreign event in and read its rolled-up documents via /:id/overview. - Queued-email routes scoped via email_queue.event_id. CRM document mail has event_id NULL and no ownable parent here, so a scoped caller is denied rather than guessed into access. 404 (not 403) so it isn't an id oracle. Note: adminEmail.js:315/332 let any email.view/edit admin archive or delete any email_queue row — the same class, pre-existing and outside these two advisories. Left untouched and reported rather than silently widened. * fix(security): codex round 2 — make the stored project owner authoritative (GHSA-wrg5) The first predicate union'd 'any linked event I can see' with the stored owner, which opened two holes: - A project owned by admin B containing ONE legacy ownerless event became readable by every admin — and /:id/overview aggregates B's other events, invoices and emails, so a single legacy event exposed the whole project. - Migration 167 deliberately leaves multi-owner (ambiguous) projects NULL rather than guessing an owner. A NULL owner was then treated as 'everyone's', so exactly those mixed projects became globally accessible. Now: the stored created_by wins outright, and a project without a usable stored owner only derives access when EVERY linked event is accessible (and at least one exists). A created_by pointing at a hard-deleted admin degrades to 'no usable owner' so the project falls back to its events instead of being locked away — no ON DELETE SET NULL migration needed. A project with neither a usable owner nor linked events stays super_admin-only: failing closed beats failing open, and a super_admin can reassign it. Also returns a knex SUBQUERY rather than a materialised id list, so a large project count can't hit the driver's bind-parameter limit. * fix(security): codex round 3 — enforce deal-lineage ownership on project attach (GHSA-wrg5) requireProjectOwnership vets only the DESTINATION project, while attaching a quote or contract cascades through linkDealToProject — which re-points every event the deal produced into that project. An editor could therefore create an empty project of their own, attach another admin's quote, and pull that admin's events (plus the invoices, emails and gallery that roll up with them) into a project they own and can read via /:id/overview. The single-customer guard did not stand in the way: an unassigned project ADOPTS the deal's customer rather than rejecting it. linkDealToProject now refuses to move lineage events the actor cannot own, and assignDocument cascades BEFORE stamping the document so a refused attach leaves nothing half-applied (the old order committed the foreign document into the caller's project and only then declined the cascade). The quote/contract create+update paths, which reach the same cascade with an arbitrary project_id, thread their adminId through as well; isSuperAdmin() resolves the role for them and fails closed when it cannot. Events are the only ownership signal a deal carries — quotes and contracts have no created_by in this schema — so a lineage that produced no event still cannot be attributed. That is a property of the CRM model, noted in the code. Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me (cherry picked from commit 688e318850db1b5f4ea2a4ae3c0fcf0fc137620d) * docs(security): drop the stale ownership JSDoc left by the rebase (GHSA-wrg5) Rebasing onto stable (which had gained scopeEventsQuery from #963) replayed the round-1 doc block above round-2's replacement, leaving a comment that describes the ORIGINAL union rule — "a project is the caller's when … it has at least one linked event they own" — directly above the code that deliberately no longer does that. That union is the hole round 2 closed; a comment asserting it is worse than none. Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
7f27e6771f |
fix(security): escape brand tokens, block tracker redirects, trim logo diagnostic (stable) (#967)
* fix(security): escape brand tokens, block tracker redirects, trim logo diagnostic (GHSA-j347, mw76, 29vm)
GHSA-j347 — buildCachedPayload sanitizes the operator's HTML and THEN runs
applyBrandTokens over the result with a plain String.replace, so any markup in
a token value reached the public origin unfiltered. The default templates
interpolate tokens into text AND into quoted attributes
(<img src="{{brand_logo_url}}" alt="{{company_name}} logo">,
href="mailto:{{support_email}}"), so a value could close the attribute and
inject. Token values are now HTML-escaped on substitution, mirroring
galleryOgService's escapeHtml. sanitizeBrandUrl's case-sensitive literal
'javascript:' check (which 'JavaScript:' walked straight past) is replaced by
an http/https scheme allowlist; relative logo paths are unaffected.
Writer is settings.edit (super_admin only) and the CSP blocks inline script,
so this is defence-in-depth — but sanitize-then-substitute is a real ordering
bug regardless.
GHSA-mw76 — the SSRF decline STANDS: self-hosted operators legitimately point
analytics at private addresses, so connection-time IP blocking would break real
deployments. Fixed only the narrow leak: undici strips
Authorization/Cookie/Proxy-Authorization/Host across a cross-origin redirect,
but umamiAdapter sends a CUSTOM x-umami-api-key header, which would be replayed
verbatim to the redirect target. Both adapters now use redirect: 'error'.
GHSA-29vm — the logo diagnostic echoed absolute storage roots, process.cwd()
and absolute candidate paths. It now reports candidates relative to
<STORAGE>/<CWD_STORAGE>, which answers the same 'which candidate existed'
question. It also still advertised the raw-absolute candidate that GHSA-c7x5
removed from resolveLogoFile, so it was misreporting what the resolver tries —
aligned with the real candidate list.
publicSiteService.test.js expectation updated: an '&' in a company name is now
emitted as '&'. Renders identically; the raw payload string differs.
* fix(security): codex round 2 — stop the remaining logo-path disclosure, mirror the resolver (GHSA-29vm)
- sources[].value was still echoed verbatim. branding_logo_path is stored
ABSOLUTE by multer, so relativising only resolvedTo and the candidate paths
left the filesystem layout going out anyway. It is now relativised too.
- Round 1 dropped the raw-absolute candidate on the grounds that GHSA-c7x5
removed it from resolveLogoFile — but the c7x5 follow-up RE-ADDED it (kept,
subject to the containment filter, so a legitimate multer path still
resolves). The diagnostic therefore reported every candidate as missing for
a contained absolute logo while resolvedTo named the file. It now mirrors the
resolver, containment filter included.
One deliberate cosmetic divergence, commented in place: for an absolute value
the resolver also tries path.join(root, value-minus-leading-slash), which can
never exist and would re-embed the absolute path this endpoint must stop
echoing. Omitted; every candidate that can actually match is still shown.
* fix(security): codex round 3 — mirror the resolver for root-relative logo paths (GHSA-29vm)
The logo diagnostic skipped the `<STORAGE>/<value>` candidates whenever
path.isAbsolute(value) was true. That test cannot distinguish a multer disk
path from a root-relative URL such as `/custom/logo.png`, and for the URL form
resolveLogoFile.generateCandidates() does try `<STORAGE>/custom/logo.png` and
can resolve it — so the endpoint reported "no source candidate exists" about a
logo that renders fine, and collapsed the configured value to its basename.
The stripped joins are now built unconditionally, exactly as the resolver does.
Disclosure stays closed: every candidate still passes the containment filter and
redact() rewrites survivors to `<STORAGE>/…`, never an absolute host path.
Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me
(cherry picked from commit 093480a753ff3d4b6ed48dd9f1108f975c8e0d47)
* fix(security): gate the logo stripped-joins on containment, not isAbsolute (GHSA-29vm)
The previous commit dropped the isAbsolute() gate entirely and regressed
logoDiagnostic's own disclosure assertion: for a genuine multer disk path,
path.join(root, value-minus-leading-slash) yields
`<STORAGE>/tmp/…/storage/custom/logo.png`, and redact() only rewrites the
LEADING root — so the inner absolute path went straight back into the payload.
The right discriminator is not "is this absolute" (which cannot separate a disk
path from a root-relative URL) but "does the value already resolve inside a
storage root". If it does, it is a real disk path, the raw candidate already
covers it, and the stripped join is the double-prefixed junk that can never
exist. If it does not — the `/custom/logo.png` URL form — the stripped join is
exactly what resolveLogoFile resolves, and is shown.
Covered by a new case asserting both halves: the candidate appears for the URL
form, and the payload still contains neither the storage root nor cwd.
Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me
(cherry picked from commit c6b95d3cd1cb28e5c2828d29d4d63fadad981dcf)
---------
Co-authored-by: Paul Nothaft <[email protected]>
|
||
|
Paul NothaftandPaul Nothaft
|
ccab9024d4 |
fix(security): bound inbound-mail resources, redact secrets from logs (stable) (#965)
* fix(security): bound inbound-mail resources, redact secrets from logs (GHSA-2qf9, pgmp, r794) GHSA-2qf9 — emailIntakeService downloaded, parsed and persisted every message with no size, attachment-count or attachment-byte limit, reachable unauthenticated by anyone who can email the operator's mailbox: - fetch the envelope with `size` (same cheap pass) and refuse an oversized message BEFORE downloading its source; - cap attachment count and cumulative attachment bytes; - limits env-overridable, defaults generous for real supplier invoices. The teeth were in the dedup key. received_emails.message_id is varchar(512) UNIQUE, and the failure path wrote `err-<uid>-<Date.now()>`, which can never match the envelope-derived messageId the dedup pass compares against — so an oversized (or overlong-Message-ID) mail was re-downloaded every poll forever, and an OOM-kill/restart just resumed the loop. Size-skips are now recorded under the REAL message id, and overlong ids collapse to a stable sha256 key that always fits the column. GHSA-pgmp / r794 — new sanitizeForLog() util (key-name deny-set, recursive, cycle-safe) applied to the three request-body log sites in adminEvents/crud.js, plus sanitizeValidationErrors() because express-validator's errors.array() embeds the SUBMITTED value per field — a rejected plaintext password was still logged. Scope is wider than filed: the update path also logged client_password_hash and a LIVE client_share_token bearer credential. Also: the one-time setup token was logged at warn AND printed to stdout on every first boot, putting a live first-admin credential in combined.log, security.log and `docker logs`. It is now written to the 0600 token file and only surfaced when that write fails — the last-resort path it existed for. (stable) * fix(security): codex round 3 — repair the first-run token recovery flow (GHSA-r794) Two regressions from keeping the setup token out of the logs. 1. server.js decided whether to print the token by calling existsSync() on the candidate path. That answers a different question than "did the write succeed": a stale, read-only or directory-shaped SETUP_TOKEN reports as present, so the banner suppressed the live token and pointed the operator at content that is not it — leaving the current token only in combined.log under default production logging. setupService now records the path the write actually produced and exposes it via writtenSetupTokenFile(). 2. The setup screen, its EN/DE strings, README, SIMPLE_SETUP and .env.example all still told first-time users to run `docker compose logs backend | grep -i "setup token"`. On the normal path that command now returns a path banner and no credential, so the documented browser-first onboarding could not be completed. They now point at `docker compose exec backend cat /app/data/SETUP_TOKEN`, with the log fallback described as what it is — the failure path. Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me (cherry picked from commit 9a54b6f0231c3285df4c4865eb846e63e1ed0dda) --------- Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul NothaftandPaul Nothaft
|
3b88036fda |
fix(security): backup/restore hardening — public-dir DB dump, restore path allowlist, gunzip bound, manifest keying (stable) (#962)
* fix(security): stop caller-chosen database backup destination (GHSA-jw8m)
POST /api/admin/database-backup/backup forwarded req.body straight into
databaseBackupService.backup(), which merges options over config:
const { destinationPath = '/backup/database', ... } = { ...config, ...options }
destinationPath is not a persistable setting — the /config allowlist only
accepts database_backup_* keys — so the request body was its only source.
The built-in `admin` role holds backup.create but neither settings.edit nor
backup.restore, so it could aim a full DB dump (admin bcrypt hashes, gallery
password hashes, encrypted SMTP creds) at the PUBLIC /uploads static mount
(server.js mounts it with no auth middleware) and then fetch it
unauthenticated. Filed low; it is a privilege escalation to unauthenticated
disclosure.
Forward only the real knobs, and only when present so absent keys can't
override config defaults via spread.
* fix(security): backup/restore hardening — restore path allowlist, gunzip bound, manifest checksum keying (GHSA-fw4c, h652, hgp8)
- adminRestore /validate + /start: constrain caller-supplied source and
manifestPath to the operator-configured backup roots — the SAME set the
restore wizard discovers from — so disaster recovery from a rescued mount
still works, with RESTORE_ALLOWED_ROOTS as an escape hatch (GHSA-fw4c).
- restoreService.decompressFile: bound the EXPANDED size and abort the
pipeline when exceeded; default 50 GB, RESTORE_MAX_DECOMPRESSED_BYTES
overrides (GHSA-h652).
- backupManifest: BACKUP_MANIFEST_KEY upgrades new manifests to a keyed
HMAC (GHSA-hgp8). Deliberately opt-in and verify-if-present — the key
cannot live in the database because the database is inside the backup, so
a mandatory HMAC would lock operators out of the exact disaster-recovery
case this exists for.
Also fixes a pre-existing bug found while testing hgp8: the checksum passed
Object.keys().sort() as JSON.stringify's second argument, which is an array
REPLACER (a property allowlist applied at every depth), not a key sorter. All
nested keys — path, size, per-file checksum — were dropped before hashing, so
the file list sat outside the integrity check entirely and a manifest path
could be rewritten to ../../etc/passwd without disturbing the digest. Now
hashes a recursively-canonicalized copy, with the legacy serialization
accepted on validation so existing backups stay restorable.
* fix(security): codex round 2 — unbreak the restore wizard, share checksum verification, guard downgrades
- adminRestore: `source` is usually a SOURCE TYPE ('local'|'s3'|'upload'),
not a path — restoreService branches on those literals. The containment
check treated it as a path, so path.resolve('local') fell outside the
backup roots and BOTH /validate and /start returned 400, blocking every
normal restore. Type tokens are now excluded from the path check.
- backupManifest: extracted verifyManifestChecksum() as the single source of
truth for the legacy/keyed fallbacks. restoreService.performPreRestoreValidation
recomputed the digest itself with the default canonical+keyed settings,
which rejected EVERY backup written before this batch. It now delegates.
- backupManifest: guard the algorithm downgrade — with a key configured, an
attacker able to rewrite the backup store could strip checksum_algorithm,
edit the manifest and recompute a plain SHA-256 that verified. Opt-in via
BACKUP_MANIFEST_REQUIRE_KEYED so pre-key backups keep restoring by default.
* fix(security): codex round 3 — close two manifest-verification fail-opens (GHSA-hgp8)
verifyManifestChecksum returned valid for a manifest with no
verification.total_checksum at all, and restoreService only called it when
that field was present. Deleting the field was therefore a complete bypass of
the keying work: no digest check, no downgrade guard, no
BACKUP_MANIFEST_REQUIRE_KEYED. Every manifest this codebase writes stamps the
field, so an absent one now fails validation, and the call site invokes the
verifier unconditionally.
Second fail-open: the strict-mode rejection of an unkeyed manifest was gated on
`&& key`, so with BACKUP_MANIFEST_REQUIRE_KEYED=true and no BACKUP_MANIFEST_KEY
configured a plain SHA-256 manifest sailed through. Strict mode is a statement
about the operator's manifests, not about the host — it is exactly the fresh
disaster-recovery box that lacks the secret. The rejection no longer depends on
a key being present.
Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me
(cherry picked from commit 292a5b784acd7f47099aa234c1c2ea00050fca97)
---------
Co-authored-by: Paul Nothaft <[email protected]>
|
||
|
Paul NothaftandPaul Nothaft
|
962f1d9586 |
fix(tests): raise jest timeouts to the 120s convention (stable) (#902)
Stable backport combining #860 (never reached stable) and #900: - jest.config.js gains testTimeout: 120000 — stable still ran on Jest's 5s default for anything unpinned, while its migration chain (134 core migrations via backports) is nearly as long as beta's. - All 19 suite-level jest.setTimeout(30000/60000) pins raised to 120s; local pins override the config default (#860's rationale). - All 15 hook-ARGUMENT timeout pins on migration-booting beforeAll hooks raised to 120s (#900's rationale — the 3.97.0-beta.0 release PR failed on exactly this class on the beta side). Untouched: the three suites whose pinned hooks don't run migrations (webhookDelivery, imageProcessor.storage, storageBackend) and publicQuotes' 30s pin on the rate-limit lockout test. No test logic changed. Co-authored-by: Paul Nothaft <[email protected]> |
||
|
Paul Nothaft
|
64bcd0ab9f |
fix(update): target docker-compose.production.yml in dashboard update steps
Production installs use docker-compose.production.yml (the README's documented
path, pinned GHCR images, no dev services), but the dashboard's update
instructions emitted bare `docker compose pull` / `up -d`. Bare `docker compose`
operates on docker-compose.yml — a different, build-based stack — so a
production user who followed the steps:
- never pulled/recreated their real containers (stayed on the old version,
e.g. stuck on 3.44.0 after "updating" to 3.45.2), and
- started the dev-only mailhog service that docker-compose.yml defines
(reported restart-looping).
The backend runs inside a container and can't stat the host's compose files, but
docker-compose.production.yml passes PICPEAK_RELEASE_CHANNEL into the backend env
and docker-compose.yml does not. detectEnvironment() now derives
isProductionCompose from it, and the Docker update steps prepend
`-f docker-compose.production.yml` when set. The non-production branch keeps the
bare commands but the warning now tells users to add `-f docker-compose.production.yml`
if they installed with it.
Also gates the mailhog service in docker-compose.yml behind a `dev` compose
profile so a plain `docker compose up -d` never starts it (opt in with
`docker compose --profile dev up -d`). Nothing depends on it (SMTP_HOST comes
from .env), so gating is safe. Verified: `docker compose config` lists mailhog
only with `--profile dev`; production compose is unchanged.
Adds unit tests for the production-vs-default command generation.
|
||
|
Paul Nothaft
|
b00a16159e |
fix(security): harden .picpeak restore operator-preservation (GHSA-qxfx follow-up)
The req.admin.id fix activated reinjectCurrentAdmin(); hardening its preservation logic (found across Codex review rounds of #811): - MFA hijack: reinject wrote back only password_hash/is_active/ must_change_password, leaving a crafted backup's two_factor_* on the operator's row — it could strip or replace their second factor. The email- matched row is now updated with the operator's full AUTH set (login identity, password, and all two_factor_* columns). Relationship/audit FKs (role_id, created_by) are deliberately NOT forced from the snapshot: on a cross-instance restore those pre-restore ids may be absent from the backup and would dangle the FK (SQLite rolls back at commit); the restored row keeps its own valid values. - Cross-instance restore rollback / FK safety: reinject matched only by email, so a backup shipping a different admin with the default `admin` username hit UNIQUE(username) and rolled the whole restore back; email and username could even collide on two different rows. Reconciliation is now non-destructive: the email-matching row is updated in place (id preserved → restored FKs like events.created_by stay valid); any different row holding the operator's username is RENAMED, not deleted (deletion would fire ON DELETE actions / dangle references); only when no row has the operator's email is a fresh row inserted, with created_by nulled and an explicit max(id)+1 id (batchInsert left the Postgres identity sequence unadvanced, so a sequence-based insert could collide). - Stale session after restore: admin_users ids shift on restore, but the operator's live JWT is bound only to decoded.id (IP logged not enforced; the backup controls password_changed_at). The route now revokes the token (result checked and logged) and clears the admin cookie; the client redirects to a fresh login via a sessionInvalidated flag. Cookie clear is the unconditional guarantee. Adds SQLite-backed reinject regression tests (in-place login/MFA restore with id and FK columns preserved, username-only rename, email+username on different rows, clean insert with created_by nulled) and the frontend redirect on sessionInvalidated. Deferred (design decisions / pre-existing, need a Postgres test env — see PR discussion): global "invalidate all pre-restore sessions" cutoff; preserving the operator's ROLE semantics across an RBAC-table replace; and resyncing Postgres identity sequences after any restore (batchInsert leaves them behind max(id) — pre-existing, affects every restored table). |
||
|
Paul Nothaft
|
dcfcb67f9b |
fix(security): sanitize chunked-upload filename (GHSA-pc72-jf53-w28j)
The chunked video upload stored req.body.filename unmodified and later built the merged path as path.join(tempDir, uploadMeta.filename). path.join does not neutralise '../', so a filename like '../../uploads/logos/evil.svg' escaped the temp dir on merge and overwrote arbitrary files. Requires admin with photos.upload. Fix: path.basename() the client filename in initializeUpload() and reject names that collapse to nothing. Adds a regression test. |
||
|
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 |
||
|
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. |
||
|
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. |
||
|
Paul Nothaft
|
9d7b6f0e11 | test: extend documentSequences mock for shared nextDocumentNumber | ||
|
Paul Nothaft
|
0205c7dcce |
chore: migrate Docker registry + GitHub URLs to PicPeak org
Repo transferred from the-luap/picpeak → PicPeak/picpeak. Docker images
publish to ghcr.io/picpeak/picpeak/{backend,frontend} (lowercase, per the
GHCR canonical form computed by docker-build.yml's `${GITHUB_REPOSITORY,,}`).
Sweep covers:
- docker-compose.production.yml + Dockerfiles → new image registry path
- README, CONTRIBUTING, SECURITY, SIMPLE_SETUP, scripts/picpeak-setup.sh
→ new GitHub URLs
- Update-check / release-notes services (updateCheckService,
environmentService, updateNotificationService, adminSystem,
UpdateNotification, githubReleaseUrl) → GitHub API + tag URLs use the
canonical PicPeak/picpeak path
- Issue templates + README-DOCKER + workflow README → updated package URLs
- One commit-context comment in migrations/090 + customerAccountsService
CHANGELOG.md is intentionally untouched (historical release entries are
immutable; GitHub auto-redirects the old URLs indefinitely).
CLAUDE.md keeps the bare `(the-luap)` reference — that's the maintainer's
personal handle, not a repo URL.
22 files, 48/48 line swaps (every change is a 1:1 URL replacement).
|
||
|
Paul Nothaft
|
ab501459a4 |
feat(analytics): pluggable trackers — Umami + Rybbit + Custom (#663 Phase 1)
Implements the hybrid scope agreed on in #663: two native adapters (Umami + Rybbit) for trackers we'd keep maintained, plus a Custom script-paste mode for everyone else (Plausible, Matomo, Pirsch, GA4, GoatCounter, Fathom, Cloudflare Web Analytics). Phase 2 (Plausible native, deeper metrics) explicitly deferred until someone asks. ## Architecture **Backend `services/trackers/`**: - `TrackerAdapter` shape (single method): `fetchDeviceBreakdown` → `{ desktop, mobile, tablet } | null`. Null = route falls back to access_logs heuristic. - `umamiAdapter.js` — extracted from the `services/umamiClient.js` that landed in #662. Same 10 test contract preserved. - `rybbitAdapter.js` — new. Hits `/api/site/{id}/breakdown?dimension= device` with Bearer auth, accepts both bare-array and `{data:[...]}` envelope variants, tolerates `sessions`/`visitors`/`value`/`count` metric keys. - `customScriptSanitiser.js` — sanitize-html with a tracker-tight allowlist (`<script>` / `<noscript>` / `<link rel=preconnect| dns-prefetch>` / `<meta>`). Strips event-handler attributes, `javascript:` and `data:` URLs. - `index.js` factory: `resolveAdapter()` reads `analytics_tracker_provider` setting → dispatches. Back-compat: when provider is unset, infers `umami` from the legacy `analytics_umami_enabled` flag so #662 installs keep working without an admin touching settings. **Backend routes**: - `adminDashboard.js /analytics`: now goes through `resolveAdapter()`. Old `fetchUmamiDeviceBreakdown` direct import removed; both `umamiClient.js` and its test file deleted (replaced by the adapter shape). - `adminSettings.js PUT /analytics`: validates the new `analytics_tracker_provider` enum, sanitises any incoming `analytics_custom_head_html` on save via the sanitiser. Masks the new `analytics_rybbit_api_key` on every GET — same pattern as Umami's API key and recaptcha secret. - `publicSettings.js`: emits `analytics_tracker_provider`, `rybbit_url`/`rybbit_website_id` (only when provider=rybbit), and the pre-sanitised `analytics_custom_head_html` (only when provider=custom). Legacy `umami_*` fields stay for back-compat. **Frontend**: - `analytics.service.ts` reworked into a provider-aware shape. `initialize({provider, ...config})` dispatches to Umami / Rybbit / Custom / None. `track()` calls dispatch to `window.umami.track` / `window.rybbit.event` / no-op based on the loaded provider. - `App.tsx` `AnalyticsBootstrap` reads `analytics_tracker_provider` from public-settings and routes to the right `initialize` call. Legacy `umami_enabled`-based path preserved as fallback when the new field is missing. - `AnalyticsTab.tsx` (Settings → Analytics) reworked with a "Provider" dropdown switching between None / Umami / Rybbit / Custom panels. Each panel renders its own config fields; Custom panel surfaces an explicit CSP-reminder banner. - `useSettingsState.ts` shape extended with `tracker_provider`, `rybbit_url`/`rybbit_website_id`/`rybbit_api_key`, `custom_head_html`. Save mutation keeps `umami_enabled` in sync with `tracker_provider==='umami'` for back-compat with downstream consumers (publicSettings shape, embedded iframe). - `publicSettings.service.ts` type extended. **i18n**: EN + DE for the provider heading + description + dropdown options + Rybbit fields + Custom HTML field + CSP warning. ## Custom mode — script execution caveat When the gallery `<head>` receives the custom HTML, simply assigning innerHTML to a container element wouldn't execute the embedded `<script>` tags (per the HTML spec, dynamically-inserted scripts via innerHTML are non-running). `analytics.service.ts:120-130` re-creates each `<script>` element manually so the browser actually evaluates it. Non-script nodes (link, meta, noscript) move in directly. ## Tests **Backend** (42 cases, all pass locally): - `umamiAdapter.test.js` (10) — pinned from the original `umamiClient.test.js`: missing-config / URL shape / encoding / payload normalisation / `laptop`→`desktop` / unknown buckets / empty / non-2xx / invalid JSON / network error. - `rybbitAdapter.test.js` (9) — same shape adapted for Rybbit: bare-array + envelope payload, `sessions`/`visitors`/`dimension` key tolerance, encoding, failure modes. - `trackerFactory.test.js` (6) — resolves null for `none`/`custom`, correct adapter for `umami`/`rybbit`, back-compat path via legacy `analytics_umami_enabled`, garbage-provider defensive null. - `customScriptSanitiser.test.js` (12) — Plausible-style passthrough, Umami-style passthrough, inline body passthrough, `<noscript>` allowed, `<link rel="preconnect|dns-prefetch">` allowed, `<link rel="stylesheet">` stripped, disallowed tags stripped, `javascript:`/`data:` URLs stripped, `on*` event handlers stripped, defensive on malformed input. - `analyticsDateMerge.test.js` (5) — preserved from #662. **Frontend**: full 84-case vitest suite green; tsc + eslint clean on changed files. Adapter changes are narrow refactors of code covered by backend tests; no new analytics-page unit test added. ## End-to-end smoke (dockerised backend + my changes mounted) ``` test 1 (back-compat: no provider, umami_enabled=true) → factory returns umami adapter, /analytics returns devicesSource:access_logs (umami fetch to fake host fails gracefully). ✓ test 2 (invalid provider value) → 400 "analytics_tracker_provider must be one of: none, umami, rybbit, custom" ✓ test 3 (save custom HTML with XSS payload) → stored sanitised: `<script>alert(1)</script>evil<script async defer data-domain="x.com" src="https://plausible.io/js/script.js"></script>` (<div> stripped; script tags survive but CSP `script-src 'self'` still blocks inline + non-allowlisted external at runtime) ✓ test 4 (public-settings exposes the provider switch) → `analytics_tracker_provider: 'custom'`, `analytics_custom_head_html: '<sanitised>'` ✓ ``` ## Out of scope (next discussions) - **Plausible native** — covered via Custom mode for now; native is Phase 2 if someone explicitly asks. - **CSP "trusted domains" admin input** — Phase 1.5. For now operators add their tracker domain to nginx/proxy CSP manually; the new CSP-reminder banner in the Custom panel makes that clear. - **Refactor `(window as any).umami.track(...)` direct calls** in PhotoLightbox/PhotoGrid to go through `analyticsService.track()` so events fire on the right tracker. Currently a no-op when Umami isn't loaded; functional but not optimal. Closes #663 Phase 1. |
||
|
Paul Nothaft
|
b5279155ea |
Merge pull request #636 from Luca-Timo/feat/accounting-inbound-invoices
feat(accounting): incoming-invoice workflow v2 + VAT/financial settings consolidation |
||
|
Luca
|
db9e41d198 |
fix(accounting): tax-report storno totals + hours-line date on Postgres
Two pre-existing HIGH bugs surfaced by the codebase audit (accounting surface): - taxReportService: income totals excluded only `status='cancelled'`, never `kind='storno'`. A Storno (status='sent', amounts stored negative) netted into the totals on top of the already-excluded cancelled original → double-subtract, so a cancel-and-reissue read as 0 income instead of the reissued amount. Now exclude storno rows from grandTotal*/byRate (kept visible in the row list). Regression test reproduces the real cancel→storno→reissue 3-row flow. - customerHoursService.buildLineItemFromEntry: `String(entry.entry_date).slice(0,10)` on a `date` column → Postgres returns a JS Date, baking "Wed Apr 06" into the invoice line + PDF (SQLite returns the bare string, so SQLite-only tests pass). Normalise via the Date branch like every other date read. |
||
|
Luca
|
707c5d0277 |
fix(accounting): address the-luap PR #636 review
- #1 resolveTaxTreatment: an unconfigured (empty) reclaim-countries list no longer auto-classifies every supplier — incl. the admin's own domestic one — as foreign; defer auto-classification until the setting is set (+ test). - #2 pending re-bills on customer erase: eraseCustomer now returns the customer's not-yet-billed inbound docs to the inbox (null customer + unsorted) so they aren't billable to an anonymized account. (NB: picpeak has no hard customer delete — erase anonymizes in place — so the orphan/404 premise can't occur; this is hardening.) - #4 VatRateSelect: when >1 configured code shares the same rate, fall through to the legacy "(not configured)" option instead of silently picking the first. - #5 unwindBilledLine: delete the (mutable, never-issued) invoice when the unwound re-bill was its only line, instead of leaving a net-zero survivor. - #6 isInvoiceMutable: clarify in a comment that invoices have no 'draft' status (the editable state is 'scheduled' w/o send-at) — no behaviour change. - nit: collapse normalizeCurrency's tautological ternary. - Fix VAT picker i18n: t('vat.legacyRate') → 'ledger.vat.legacyRate' (the key's real home), so the legacy label localizes instead of always showing English. - Remove dead i18n keys left by the settings refactor (businessProfile.field VAT /hourly + profileFields.title/savedToast). |
||
|
Luca
|
267b121d66 |
feat(accounting): supplier-country tax default + configurable default output VAT code
VAT supplier-country reclaim default: - Migration 134 adds inbound_documents.supplier_country. - categorizeInbound auto-derives tax_treatment via resolveTaxTreatment: explicit treatment wins; else country in the reclaim list → domestic, outside it → foreign_vat_non_reclaimable, unknown → domestic. Consumes the previously-stored-but-unused accounting_vat_reclaim_countries. - Triage modal gains a Supplier country dropdown (saved via updateInbound). +5 unit tests for resolveTaxTreatment. Configurable default output VAT code for new invoices: - New accounting_default_output_vat_code setting (PUT wired; getSettings/type). - Settings → Accounting dropdown to pick it. - Invoice + quote editors seed their VAT picker (rate + code) from it on a blank new document — skipping edits/conversions, never clobbering a touched value. New docs no longer silently start at 0%. i18n en + de. |
||
|
Luca
|
36a8e42f90 |
feat(accounting): re-categorize incoming invoices, note field, pending re-bill pool
Address three incoming-invoice issues: 1. Re-categorization: a categorized invoice can now be changed again (e.g. passthrough → company expense). New "Re-categorize" button pre-fills the triage modal from the existing disposition/customer/markup/note. categorizeInbound is re-runnable — it unwinds any prior re-bill line (removes the invoice line + recomputes totals) before applying the new disposition, and refuses (INVOICE_LOCKED) when the re-bill is on an already-issued invoice. 2. Note field: new `note` column (migration 132 — 126 is already on beta) captured in triage and shown in the read-only view. 3. Re-bill like hours: rebill/passthrough now persist customer_account_id. Per-event customers accumulate as PENDING items, surfaced in a new "Pending re-bills" card and bundled into one invoice via "Bill these" (mirrors unbilled-hours billing). Monthly/manual customers keep auto-consolidating onto their running draft. Passthrough (durchlaufend) can now also attach to a customer with optional markup. Adds backend unit tests for buildInboundLineItem + isInvoiceMutable and en/de translations (other locales fall back to English defaults). |
||
|
Paul Nothaft
|
a239fec9d7 |
fix(admin/exports): Lightroom TXT export joins with comma + drops extension (#623)
The PhotoExportMenu's TXT format advertises "Simple text list for Lightroom search" but emitted newline-separated filenames WITH `.jpg`. Lightroom's filename search wants a comma-separated one-liner, and the gallery JPEGs may correspond to RAW files in the catalog — so the search has to match on the stem only. The frontend now passes `separator: 'comma'` + `include_extension: false` for the TXT format specifically. The backend gains an `include_extension` option (defaulting to true so direct API consumers don't break), and the comma case joins without a trailing space (the form Lightroom expects). Unit test pins the Lightroom-mode output AND the backward-compatible default for any direct API caller. CSV / XMP / JSON exports are unchanged. |
||
|
Luca
|
86dff75898 |
test(accounting): cover export scope, unique-violation detector, PDF page cap
Closes the test gaps from the PR #622 work + the export-scope feature: - export scope: scopeLedger/normalizeScope (exported via _internal) unit tests + renderTaxReportCsv income/cost/all output assertions (income drops supplier rows, cost drops invoice rows, filename gets the scope tag). - isUniqueViolation: Postgres 23505 / SQLITE_CONSTRAINT / "UNIQUE constraint failed" message, false for FK + nullish (the IMAP claim-first race detector). - getRenderedPagePath: out-of-range pages reject with PAGE_OUT_OF_RANGE before touching pdftoppm/disk (the per-file resource bound). |
||
|
Luca
|
a93b6dc232 |
fix(accounting): PR #622 concerns — flag-cache, customer master gate, VAT-unconfigured, helpers, page cap
1. requireFeatureFlag now caches each flag for 10s (the accounting area is 10+ gated endpoints); PUT /admin/feature-flags invalidates the cache so toggles still take effect immediately. 2. Customer routes (/quotes, /invoices, /contracts + their PDFs) now gate via getEffectiveFeaturesForCustomer — the global MASTER flag AND the per-customer override — instead of the per-customer column alone, via a shared customerFeatureAllowed() helper. Admin disabling a feature globally is now honoured for customers too. 4. Tax-report VAT-payable: when accounting_vat_registered is UNSET, stop guessing from grandTotalVat>0 (a zero-output-VAT quarter silently flipped to "not registered" and hid the reclaim). Treat null as "not configured": vatPayableMinor=null + vatRegistrationConfigured=false; the UI renders "—" and a "configure VAT registration" warning. Tests updated. 5. Shared upsertAppSetting() in utils/appSettings — the two adminSettings upsert loops use it, so the app_settings created_at class can't be re-introduced. 6. PDF rasterise per-file bound: getRenderedPagePath refuses pages beyond MAX_RENDERABLE_PAGES (200); page_count is capped to match at ingest, so a hostile high-page PDF can't drive an unbounded pager. 7. (no code) original_filename is only rendered via auto-escaped JSX; the two dangerouslySetInnerHTML sites are admin-authored content — paranoia pass clean. Concerns 3 (foreign-VAT reclaim-country) and 8 (imap_pass plaintext) are PR-reply / doc items, addressed in the PR response, not code. |
||
|
Luca
|
d7107aaf0a |
feat(accounting): tax report VAT-payable honours registration + reclaim
The report's vatPayable is now: 0 when not VAT-registered; otherwise output VAT minus the RECLAIMABLE input VAT only (costs with tax_treatment foreign_vat_non_reclaimable are excluded from the deduction). Registration reads accounting_vat_registered; when unset it falls back to a behaviour-preserving heuristic (charged output VAT this period ⇒ registered), so existing reports are unchanged and non-VAT installs correctly show 0. loadCosts now tracks reclaimableVat. Tests updated; 32 pass. |
||
|
Luca
|
53a16f9f6f |
fix(accounting): Banana I&E export uses the 'Category' column (not 'ContraAccount')
Real Banana Income & Expense files name the category column 'Category', not 'ContraAccount' (which the doc listed but is a double-entry concept) — so the income/expense account never landed and Banana warned 'ContraAccount column not found'. Use 'Category'. VatCode stays (it only warns on a non-VAT-enabled file; amounts are gross). Test updated. |
||
|
Luca
|
0c0fb29770 |
fix(accounting): emit ISO dates in exports (Postgres returns Date objects)
The Date column imported empty into Banana because dateOnly() did String(d).slice(0,10) — on Postgres the date columns come back as JS Date objects, so that yields "Thu Jan 15" instead of "2026-01-15", which Banana rejects. (SQLite returns strings, so the tests never caught it — the pg-date-serialisation trap.) - ledgerService.dateOnly + taxReportService CSV now format Date objects to yyyy-mm-dd via local calendar parts (DATE columns are local-midnight). - Regression test added with a real Date object (the existing tests all used string dates). |
||
|
Luca
|
445d6d7b6d |
feat(accounting): add a Banana "Income & Expense" (cash-book) export format
The Banana export assumed a double-entry file; a user importing into an Income & Expense (Einnahmen-Ausgaben) file got "AccountDebit/AccountCredit/Amount/ VatCode column not found", since those columns only exist in double-entry. Add a second Banana format alongside the double-entry one: - ledgerService: new `banana_ie` format → Banana I&E columns Date, Doc, Description, Income, Expenses, ContraAccount (the income/expense account), VatCode (banana.ch doc 9946). Revenue → gross in Income + revenue account; cost → gross in Expenses + expense account. Same tab-separated .txt shape. - Frontend: ExportFormat + dropdown gain `banana_ie`; .txt extension covers both Banana variants. Labels relabelled: "Banana — double-entry" and "Banana — income & expense" (de equivalents). Hint de-"double-entry"-fied. - Test added for the I&E format. Pairs with the prior UTF-8 BOM fix (the "·" mojibake). Tests + build green. |
||
|
Luca
|
a19506749a |
fix(accounting): Banana export is now a tab-separated .txt (actually importable)
Banana's "Text file with column headers" import (Actions → Import into accounting) requires a TAB-separated .txt with unquoted values — picpeak was emitting a comma-separated, quoted .csv, which won't even show in Banana's *.txt file picker, let alone parse into columns. - ledgerService.exportPostings: the `banana` format now serialises TAB-separated with no quoting, .txt extension, text/plain content-type. generic + bexio stay comma-CSV (RFC 4180). Tab/newline chars in a cell are collapsed to spaces. - Frontend ledger.service: download filename uses .txt for banana. - Tests updated for the new banana shape (tab header, .txt, text/plain). The column names already matched Banana's NameXml; only the serialisation was wrong. bexio left as comma-CSV (verify against bexio's import spec separately). |
||
|
Luca
|
b584aaf7a6 |
test(accounting): update tax-report CSV tests for the unified ledger format
The CSV rework (unified, typed ledger) replaced the 'Rechnung' column with 'Referenz' (+ a 'Typ' column) and dropped the separate cancelled 0/1 column in favour of a localised '(Cancelled)' suffix on the Reference cell. Update the two assertions in taxReportPdf.test.js accordingly. All 11 cases pass. |
||
|
Luca
|
03cc250b47 |
feat(accounting): Layer A backend — chart of accounts, VAT codes, Treuhänder export
Prepares picpeak to feed a Treuhänder's double-entry software once a user crosses the CHF ~500k threshold (LI PGR Art. 1045), without becoming an ERP. - migration 129: ledger_accounts (seeded Swiss/LI KMU-Kontenrahmen) + vat_codes (CH/LI MWST 8.1/2.6/3.8/0 + reverse charge), expense_categories gains ledger_account_id, app_settings default-account + VAT-map seeds - ledgerService: full CRUD for accounts + VAT codes + mappings; buildPostings() turns revenue invoices + incoming invoices + expenses into accrual Buchungssätze (Dr/Cr + VAT code); generic/banana/bexio CSV export - routes /api/admin/ledger/* (accounting master gated; export also requires taxReport); 12 unit tests (posting engine + formatters) Accrual basis only — payment/bank postings are Layer B. Output is a guideline (Treuhänder caveat on the UI). |
||
|
Luca
|
545ef334f4 |
feat(accounting): tax window shows all costs (incoming invoices + expenses) alongside revenue
Einnahmen-Ausgaben view for the Milchbüchlein/simple-accounting case: - taxReportService.getTaxReport now returns a cost side (loadCosts: incoming invoices + internal expenses, company- or event-booked, schema-guarded) plus a summary (income / costs / result, VAT payable) - declined/duplicate costs excluded; re-billed costs kept (matching re-bill revenue is counted, so the net is correct) - CSV + PDF exports gain a Costs section and an income/costs/result summary; pdf-i18n keys added for all 6 locales (fr/nl/pt/ru machine — flag for native review) - frontend tax page renders the summary card, a costs table (company vs event), and a 'verify with Treuhänder' disclaimer - tax-report tests cover the cost aggregation + zeroed summary when the accounting tables are absent; adminCrmAuth test enables the accounting master flag the route now requires fr/nl/pt/ru strings are machine-generated and need native review. |
||
|
Luca
|
5e78fb6475 |
feat(accounting): backend rework - incoming invoices vs internal expenses (stage 2)
Implements the split decided in review: Incoming invoices (external) - the inbound_documents row IS the payable: - categorizeInbound now UPDATES the document (disposition + tax_treatment + booking event_id (null=company) + category), no derived expense row, so a supplier invoice appears only in the incoming-invoices surface. - rebillInbound mints the client invoice from the document (base = invoice total + markup) and links it on the doc. - markInboundSupplierPayment records supplier payment ON the incoming invoice (mark-paid lives here now). Expenses (internal) - own costs only: - createExpense: kind = amount|mileage|per_diem; amount = quantity x rate (rate from accounting settings, per-entry override; snapshotted); optional proof file; booked to an event or the company; require-proof enforced from settings. No supplier payment, always own-cost. - listExpenses returns internal rows only (inbound_document_id IS NULL). Routes: per-flag gating (incomingInvoices vs expenses; categories on the accounting master); supplier-payment + re-bill moved under /inbound/:id/*; POST/PATCH expenses accept a multipart proof upload; GET /:id/proof streams it (PDF download-only, image inline). getAccountingSettings reads app_settings. Verified: node -c, require-graph, 12 unit tests (markup + expense amount/build). Frontend rework (service + the two UIs + settings tab + category i18n) follows. |