From a3a6319c2921c0adb4404e3e11d9fe41b17f129f Mon Sep 17 00:00:00 2001 From: Ralksta <16941050+ralksta@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:12:13 +0200 Subject: [PATCH] release: v0.16.0 See CHANGELOG.md for the full v0.16.0 entry. --------- Signed-off-by: dependabot[bot] Co-authored-by: ralksta Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 Co-authored-by: Ralf Co-authored-by: Paul Thies <110185070+ImScheinox@users.noreply.github.com> Co-authored-by: lancetm714 <108419425+lancetm714@users.noreply.github.com> --- CHANGELOG.md | 318 ++++++++ CONTRIBUTING.md | 20 + app/[...path]/PhotoGrid.tsx | 87 ++- app/[...path]/SubpageGridView.tsx | 60 +- app/[...path]/page.tsx | 94 ++- app/about/page.tsx | 2 + app/admin/AdminShell.tsx | 27 +- app/admin/__tests__/AdminShell.test.tsx | 71 ++ app/admin/__tests__/SaveBar.test.tsx | 88 +++ .../album-entries-round-trip.test.ts | 80 ++ app/admin/components/AdminDashboard.tsx | 54 +- app/admin/components/AdminLogin.tsx | 6 +- app/admin/components/HelpView.tsx | 13 +- app/admin/components/JournalStudio.tsx | 55 +- app/admin/components/PageBuilder.tsx | 133 ++-- app/admin/components/SettingsEditor.tsx | 87 ++- .../__tests__/AdminDashboard-reload.test.tsx | 98 +++ .../__tests__/sessionExpiry.test.ts | 38 + .../components/page-builder/albumEntries.ts | 87 +++ app/admin/components/page-builder/types.ts | 13 + app/admin/components/sessionExpiry.ts | 24 + app/admin/page.tsx | 14 +- app/admin/styles/base.css | 12 + app/admin/styles/drawers.css | 9 - app/admin/styles/help.css | 4 - app/api/admin/__tests__/about-backup.test.ts | 68 ++ app/api/admin/__tests__/admin-guards.test.ts | 59 ++ app/api/admin/__tests__/doctor-guards.test.ts | 148 +++- .../admin/__tests__/settings-write.test.ts | 77 ++ app/api/admin/about/route.ts | 52 +- .../admin/albums/[albumId]/assets/route.ts | 94 ++- app/api/admin/albums/route.ts | 13 +- app/api/admin/alt-text/route.ts | 13 +- app/api/admin/analytics/route.ts | 13 +- app/api/admin/assets/route.ts | 13 +- app/api/admin/backups/route.ts | 24 +- app/api/admin/doctor/route.ts | 44 +- app/api/admin/favicon/route.ts | 34 +- app/api/admin/gallery/route.ts | 24 +- app/api/admin/journal/[slug]/route.ts | 35 +- app/api/admin/journal/route.ts | 24 +- app/api/admin/reload/route.ts | 13 +- app/api/admin/settings/route.ts | 38 +- app/api/admin/status/route.ts | 13 +- app/api/admin/thumbnail/[id]/route.ts | 90 ++- .../analytics/__tests__/track-route.test.ts | 155 ++++ app/api/analytics/track/route.ts | 86 ++- app/api/install/route.ts | 8 +- app/globals.css | 81 ++ app/install/InstallWizard.tsx | 11 + app/install/install.css | 11 + app/journal/[slug]/page.tsx | 63 +- app/journal/page.tsx | 15 +- app/layout.tsx | 23 +- components/JournalNav.tsx | 66 ++ components/Lightbox.tsx | 21 +- components/MobileNav.tsx | 98 +++ components/NavLink.tsx | 47 ++ components/SubpageNav.tsx | 16 +- components/__tests__/NavLink.test.tsx | 105 +++ docs/admin-panel.md | 16 +- e2e/photo-permalink.spec.ts | 83 +++ lib/__tests__/admin-auth.test.ts | 23 +- lib/__tests__/atomicWrite.test.ts | 120 +++ lib/__tests__/auth.test.ts | 28 + lib/__tests__/backup-prune.test.ts | 111 +++ lib/__tests__/backup-restore.test.ts | 43 +- lib/__tests__/config-cache.test.ts | 1 + lib/__tests__/config-hero-uuid.test.ts | 1 + lib/__tests__/config-safe.test.ts | 1 + lib/__tests__/config.test.ts | 50 ++ lib/__tests__/derive-gallery.test.ts | 70 ++ lib/__tests__/doctor.test.ts | 87 +++ lib/__tests__/grid-columns-clamp.test.ts | 116 +++ lib/__tests__/image-size.test.ts | 35 +- lib/__tests__/immich.test.ts | 213 +++++- lib/__tests__/immichCache.test.ts | 107 +++ lib/__tests__/immichTransport.test.ts | 208 ++++++ lib/__tests__/install-hero.test.ts | 7 + lib/__tests__/install.test.ts | 127 ++++ lib/__tests__/journal-backup.test.ts | 52 ++ lib/__tests__/journal-roundtrip.test.ts | 92 +++ lib/__tests__/journalNav.test.ts | 51 ++ lib/__tests__/mapService.test.ts | 85 +++ lib/__tests__/navActive.test.ts | 56 ++ lib/__tests__/photoHash.test.ts | 71 +- lib/__tests__/proxy.test.ts | 48 ++ lib/__tests__/rate-limit.test.ts | 74 ++ lib/__tests__/settings-schema.test.ts | 124 ++++ lib/__tests__/theme-scalars.test.ts | 62 ++ lib/__tests__/yaml-write.test.ts | 33 + lib/admin/doctor.ts | 89 +++ lib/admin/journal-service.ts | 97 ++- lib/admin/withAdmin.ts | 32 + lib/admin/yaml-service.ts | 94 ++- lib/atomicWrite.ts | 56 ++ lib/auth.ts | 16 + lib/config/index.ts | 74 +- lib/config/schema.ts | 43 +- lib/config/settingsSchema.ts | 201 +++++ lib/config/theme.ts | 35 +- lib/env.ts | 25 +- lib/i18n/locales/de.ts | 9 + lib/i18n/locales/en.ts | 9 + lib/imageSize.ts | 18 +- lib/immich.ts | 212 ++---- lib/immichCache.ts | 91 +++ lib/immichTransport.ts | 130 ++++ lib/install.ts | 65 +- lib/journal.ts | 104 ++- lib/journalNav.ts | 57 ++ lib/mapService.ts | 5 +- lib/photoHash.ts | 51 +- lib/rate-limit.ts | 77 +- package-lock.json | 695 +++++++++++++++++- package.json | 7 +- proxy.ts | 21 +- scripts/__tests__/doctor-cli.test.ts | 67 +- scripts/doctor.mts | 18 +- vitest.config.ts | 11 +- 120 files changed, 6818 insertions(+), 975 deletions(-) create mode 100644 app/admin/__tests__/AdminShell.test.tsx create mode 100644 app/admin/__tests__/SaveBar.test.tsx create mode 100644 app/admin/__tests__/album-entries-round-trip.test.ts create mode 100644 app/admin/components/__tests__/AdminDashboard-reload.test.tsx create mode 100644 app/admin/components/__tests__/sessionExpiry.test.ts create mode 100644 app/admin/components/page-builder/albumEntries.ts create mode 100644 app/admin/components/sessionExpiry.ts create mode 100644 app/api/admin/__tests__/about-backup.test.ts create mode 100644 app/api/admin/__tests__/settings-write.test.ts create mode 100644 app/api/analytics/__tests__/track-route.test.ts create mode 100644 components/JournalNav.tsx create mode 100644 components/MobileNav.tsx create mode 100644 components/NavLink.tsx create mode 100644 components/__tests__/NavLink.test.tsx create mode 100644 e2e/photo-permalink.spec.ts create mode 100644 lib/__tests__/atomicWrite.test.ts create mode 100644 lib/__tests__/backup-prune.test.ts create mode 100644 lib/__tests__/grid-columns-clamp.test.ts create mode 100644 lib/__tests__/immichCache.test.ts create mode 100644 lib/__tests__/immichTransport.test.ts create mode 100644 lib/__tests__/journal-roundtrip.test.ts create mode 100644 lib/__tests__/journalNav.test.ts create mode 100644 lib/__tests__/mapService.test.ts create mode 100644 lib/__tests__/navActive.test.ts create mode 100644 lib/__tests__/settings-schema.test.ts create mode 100644 lib/__tests__/theme-scalars.test.ts create mode 100644 lib/admin/withAdmin.ts create mode 100644 lib/atomicWrite.ts create mode 100644 lib/config/settingsSchema.ts create mode 100644 lib/immichCache.ts create mode 100644 lib/immichTransport.ts create mode 100644 lib/journalNav.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d47777c..d111a4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,324 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Releases up to and including v0.9.2 are documented in the [GitHub releases](https://github.com/ralksta/immich-folio/releases). +## [0.16.0] — 2026-09-22 + +### Security + +- **The image proxy no longer serves un-downsampled originals** + ([#653](https://github.com/ralksta/immich-folio/pull/653), + [GHSA-36m4-p39x-9wx8](https://github.com/ralksta/immich-folio/security/advisories/GHSA-36m4-p39x-9wx8)). + `resolveImageSize()` could resolve to the original tier from a single + request parameter. `/api/image` treats the opaque asset token as its whole + capability check — holding one means you saw the page it was rendered on, + which is the right bar for a preview but not for the un-downsampled file. + `resolveImageSize()` now caps its result at `preview` regardless of what + `?size=` or `?w=` ask for; a small width still narrows to `thumbnail` as + before. Originals remain available only through `/api/download`, which + verifies the album allowlist, the album's `download: true` opt-in, and + every password gate on the way. **See the upgrade notes: this needs an + `IMAGE_CACHE_VERSION` bump to reach browsers that already cached an + original under the old response.** + +- **Password-protected journal entries and albums no longer leak outside + their own gate** + ([#654](https://github.com/ralksta/immich-folio/pull/654), + [GHSA-fvgv-97g3-wjr7](https://github.com/ralksta/immich-folio/security/advisories/GHSA-fvgv-97g3-wjr7)). + A journal entry's `password:` only ever gated the body of + `/journal/`. Three other surfaces handed out its content regardless, + and the same class of bug reached album and subpage metadata: the public + journal index listed a protected entry's title, subtitle, cover image and + excerpt to everyone; `generateMetadata` for a journal entry, album or + subpage ran ahead of the page's own gate, so a locked page's real title, + photo count and cover image reached `` of a 200 response nobody had + authenticated for — including a shared `?photo=` link's Open Graph image, + which named the exact photo; and the journal detail page spread an entry's + whole frontmatter into the essay renderer's props, so the stored password + (plaintext or a scrypt hash) rode along in the RSC payload of anyone who + had unlocked the entry once. All three now run the same + `isProtected`/`isAuthenticated` checks the page body already used. + +- **Every route to an album now enforces its subpage's and its own + password** + ([#638](https://github.com/ralksta/immich-folio/pull/638), + [GHSA-g85g-xcqc-f59h](https://github.com/ralksta/immich-folio/security/advisories/GHSA-g85g-xcqc-f59h), + [GHSA-jhmx-5j9c-qg42](https://github.com/ralksta/immich-folio/security/advisories/GHSA-jhmx-5j9c-qg42)). + Two related fixes from the v0.16.0 audit. `getAlbumBySlug()` now searches + only the albums reachable from the route the slug arrived on — standalone + albums for a top-level slug, a subpage's own albums otherwise, nothing for + a disabled subpage — so an album listed only inside a subpage no longer + answers at the bare `/`, bypassing that subpage's gate. + Separately, every exclusion in the proxy's route matcher now ends on a + segment boundary, closing a gap where a path merely _starting with_ an + excluded name skipped the site password and CSP entirely. + +- **Hardening: cache-bypass, analytics, rate limiting, file permissions** + ([#655](https://github.com/ralksta/immich-folio/pull/655), + [GHSA-w293-x8pc-j4cv](https://github.com/ralksta/immich-folio/security/advisories/GHSA-w293-x8pc-j4cv)). + Four lower-severity findings from the same audit: + - The unauthenticated `?fresh=1`/`?preview=true` cache-bypass flags are now + gated to `isAdminAuthenticated()`, and a forced refresh no longer evicts + the existing cache entry before the refetch succeeds — a request timed + during an Immich outage could otherwise wipe out the stale fallback that + was keeping the page up for everyone else. + - `POST /api/analytics/track` gained a rate limit (60 rpm per IP), a cap on + distinct page keys tracked per day, and writes through the same atomic + writer as everything else — it previously used a plain `fs.writeFile` + with an unbounded, request-supplied key space. + - The rate limiter was a fixed window, not the sliding window it + documented: a burst timed around the window boundary could clear close + to double the configured limit. It is now a weighted two-bucket window, + shared by every rate-limited route. + - `install.json`, which holds the Immich API key and `authSecret`, was + briefly world-readable between being written and a follow-up `chmod`. + `atomicWrite` now accepts a file mode applied before the rename, so the + file is never created with the wrong permissions. + +- **Journal backup paths hardened against a CodeQL path-injection alert** + ([#586](https://github.com/ralksta/immich-folio/pull/586)). Not + exploitable — the slug reaching `snapshotEntry` had already passed + `isValidSlug` or the anchored backup-name pattern in every caller — but + CodeQL could not follow that validation across modules. The backup path + is now built through the same `containedPath()` guard every other path in + the file already used, so a path that would escape `.backups/` throws + instead of being trusted. + +### Added + +- **Diagnostics is a tab in the admin header, and its duplicate entry + points are gone** + ([#621](https://github.com/ralksta/immich-folio/pull/621), + [#656](https://github.com/ralksta/immich-folio/pull/656)). `/admin` and + `/admin/pages` used to render the same page builder under two addresses; + `/admin` now redirects to the one the Pages tab actually links to. + Diagnostics, a real page since v0.15.0, was reachable only from the + status dropdown and the Help page — it is now a tab of its own, and the + now-redundant Backups/Diagnostics buttons in the status dropdown are + gone (Backups keeps its header button). + +- **Journal entries get previous/next navigation, subpages get a "next + subpage" link, and both — plus `/about` — get a way back** + ([#658](https://github.com/ralksta/immich-folio/pull/658)). At the end of + a journal entry, at the bottom of a subpage's album grid, and on the About + page, there was previously no way onward or back except the browser's own + button. Journal prev/next mirrors the existing album navigation: neighbours + come from the same publication-order list the journal index renders, with + no wrap-around, and a draft or a password-protected entry the visitor + hasn't unlocked never appears here even by name. The "next subpage" link + follows the same order the header navigation and homepage already use, so + it never points somewhere hidden or disabled. + +- **A proper mobile menu for the public header** + ([#659](https://github.com/ralksta/immich-folio/pull/659)). The header + used to be one flat row that scrolled sideways on narrow viewports with + the scrollbar hidden — nothing indicated that more sections existed + off-screen, so the site appeared to have fewer than it does. Below a new + 640px breakpoint, a hamburger button opens the same links (never + duplicated in the markup) as a full-width vertical panel; above it, the + layout is pixel-identical to before. + +- **`settings.yaml` is validated before it is written** + ([#623](https://github.com/ralksta/immich-folio/pull/623)). + `PUT /api/admin/settings` used to cast the request body and write it + straight through — the only admin save path with no shape check at all, + and one that is read by every public page. A zod schema now checks + structure (a scalar where a section belongs, a list where a record + belongs), while value narrowing stays where the resolvers already do it, + since only they also see hand-edited YAML. + +### Fixed + +- **Config values reach the page they render on, no longer silently + discarded** + ([#645](https://github.com/ralksta/immich-folio/pull/645), + [#618](https://github.com/ralksta/immich-folio/pull/618)). The admin + panel rewrites `gallery.yaml`/`settings.yaml` wholesale on every save, so + a field the panel doesn't know about isn't merely ignored — it's deleted. + This hit an album's `location` precision and `download` opt-in (any + unrelated save in the panel silently reset both to their defaults) and, + separately, unclamped `grid.columns`/`theme.radius` values that could + reach CSS as an invalid `repeat()` or a concatenated `8pxpx`. + +- **The Immich client no longer serves a cached "not found" as real data + during an outage** + ([#624](https://github.com/ralksta/immich-folio/pull/624)). Splitting + transport and cache-policy concerns out of the 835-line client surfaced a + real bug in the process: once a definitive-404 cache entry's TTL passed + during an Immich outage, it came back as a truthy sentinel object with no + fields, every `if (!asset)` guard passed on it, and the page answered 500 + for the length of the outage instead of falling back to the last known + album. A stale "missing" now resolves to `null` — the answer every caller + already handled — while every other stale-cache path still throws and + lets the outage propagate correctly. + +- **A never-checked expired admin session no longer fails silently** + ([#650](https://github.com/ralksta/immich-folio/pull/650)). Admin + sessions last 24 hours, but nothing re-checked one after the initial page + load — a tab left open overnight looked logged in while every save quietly + answered 401. A 401 from any admin save or reload now drops back to the + login screen with a notice instead of a confusing generic error or, for + the reload button, no feedback at all. + +- **Saving no longer overwrites a configuration that failed to load** + ([#619](https://github.com/ralksta/immich-folio/pull/619)). Each admin + editor holds the whole file in memory and writes all of it back — so a + load that quietly failed left an empty form indistinguishable from a + freshly-configured site, and saving replaced the real file with that + emptiness. A failed load now replaces the form with a notice and refuses + to save, including against the keyboard shortcut, until a fresh load + succeeds. + +- **Removing an album, hero image or section entry now asks first** + ([#649](https://github.com/ralksta/immich-folio/pull/649)). Removing a + subpage already asked for confirmation; removing an album — which also + discards its grid overrides, cover asset and manual photo order — did + not, with no undo besides reloading the page and losing every other + unsaved edit. + +- **The active nav link actually shows which page you're on** + ([#620](https://github.com/ralksta/immich-folio/pull/620)). The + `.active` styling and its underline existed in the CSS since the header + was written, but nothing ever applied the class, and `aria-current` was + missing outright. Both the public header and the admin panel's own tabs + now derive and announce the active section. + +- **Pre-restore backups no longer crowd out ordinary ones** + ([#617](https://github.com/ralksta/immich-folio/pull/617)). Pre-restore + snapshots were exempt from the ten-backup cap entirely, so after a dozen + restores the backup manager showed almost nothing else. Each kind is now + counted and capped separately, and pruning also runs on restore rather + than waiting for the next save. + +- **A failed backup now aborts the save instead of continuing without + one** + ([#641](https://github.com/ralksta/immich-folio/pull/641)). Restoring a + backup used a plain `fs.copyFile`, which truncates the live file before + writing, and its own pre-restore safety copy swallowed every error as "the + file probably doesn't exist yet." On a full content volume this could + truncate `gallery.yaml` to zero bytes with no snapshot to recover from. + Restores now write through the same atomic (temp-file-plus-rename) + primitive as every other save, and only `ENOENT` is treated as "nothing to + back up" — any other failure aborts before the live file is touched. The + same swallow-everything shape existed in the ordinary save path for + `gallery.yaml`, `settings.yaml`, journal entries and `about.md`, and got + the same fix. + +- **Editing a journal entry that came from the legacy `essays/` folder no + longer resurrects its old version** + ([#639](https://github.com/ralksta/immich-folio/pull/639)). Saving always + wrote to `content/journal/`, but a legacy entry kept its original copy in + `content/essays/` too, and deleting only ever removed the new copy — so a + "deleted" entry reappeared with its pre-edit text and frontmatter on the + next request. Saving a legacy-origin entry now retires its old copy, and + deletion checks both locations directly. + +- **The journal serializer no longer mangles what it just parsed** + ([#640](https://github.com/ralksta/immich-folio/pull/640)). Re-saving an + entry — including a bare frontmatter edit, which re-serializes the whole + document — dropped every link in a paragraph, left literal HTML tags in + quotes and photo captions, doubled HTML-entity-encoded apostrophes in + photo-pair captions, and turned `my_photo_2024.jpg` into + `my*photo*2024.jpg`. The serializer now walks the exact markup the + renderer produces instead of stripping tags with a flat regex, and + emphasis markers now require CommonMark's non-word boundary on both + sides. + +- **The setup wizard no longer overwrites an existing `gallery.yaml`** + ([#642](https://github.com/ralksta/immich-folio/pull/642)). The wizard + already protected `settings.yaml` from being silently replaced, five + lines above the identical unconditional overwrite of `gallery.yaml` — and + unlike the admin panel's own writes, this path kept no backup. A site + could reach the wizard again through a resolvable-but-wrong Immich URL or + a corrupted `install.json`, at which point every subpage, password and + per-album override was gone with nothing recoverable on disk. Both files + now get the same "keep if it already exists" treatment; the wizard tells + the operator when that happened. + +- **`npm run doctor` no longer passes two broken deployments** + ([#643](https://github.com/ralksta/immich-folio/pull/643)). A + `TRUSTED_PROXY_HOPS` value lower than the observed proxy chain (but above + zero) fell through to the "matches the observed chain" success case + instead of warning. Separately, the album-configuration check only ran + when Immich's response was non-empty — so an API key pointed at the wrong + Immich account, answering `200 []`, passed every check while every + configured album page was actually empty. + +- **Two subpages, or two albums on the same page, can no longer share a + slug silently** + ([#644](https://github.com/ralksta/immich-folio/pull/644)). Subpage and + album slugs are derived from their names with no uniqueness check, and + `slugify` folds diacritics and punctuation readily enough that distinct + names collide (`Portfolio 2024` and `Portfolio-2024` both become + `portfolio-2024`). The second entry became unreachable, and its password + and other settings silently resolved from the first. Colliding subpage + names, and colliding album title overrides on the same page, are now + rejected at save time with both names in the error; a collision that only + becomes visible once Immich's own album names are known surfaces as a new + diagnostics finding instead. + +- **A trailing slash on the Immich URL no longer breaks every request + after setup** + ([#646](https://github.com/ralksta/immich-folio/pull/646)). The install + wizard's connection test normalised the URL before pinging it, but stored + the raw input — so `https://immich.example.com/api/` passed the wizard + and then sent every later request to a doubled `/api/api/` path. The + stored value is now normalised the same way at both write and read time, + so an `install.json` written before this fix heals without re-running the + wizard. + +- **Coordinates of exactly 0 no longer vanish from the map, and error + responses no longer leak sockets** + ([#647](https://github.com/ralksta/immich-folio/pull/647)). The map's + coordinate check used plain falsiness, so a photo on the equator or the + prime meridian was treated as having no location at all. Separately, + several Immich-facing error paths never read or cancelled the upstream + response body before returning — under a sustained upstream fault this + slowly exhausts the connection pool. + +- **A shared photo link survives reordering or deleting other photos in + the album** + ([#651](https://github.com/ralksta/immich-folio/pull/651)). The lightbox + addressed a photo by its position in the album (`#photo-N`); reordering or + deleting a photo silently repointed every previously shared link at a + different image. Links now address the photo by its stable asset id via + `?photo=`, which also means a shared link's Open Graph preview can + show the actual photo instead of a generic card. Links shared before this + change keep working — the old `#photo-N` format is still read as a + fallback. + +### Internal + +- **One `withAdmin()` wrapper instead of the same guard copied into 24 + route handlers** + ([#615](https://github.com/ralksta/immich-folio/pull/615)). The + enabled-check-plus-auth-check pattern was hand-copied across 16 admin + route files; a new route protected by nobody used to be a diff that + looked completely ordinary. A structural test now rejects any admin + handler exported outside the wrapper. +- **One atomic-write primitive instead of six independently-drifted + copies** + ([#616](https://github.com/ralksta/immich-folio/pull/616)). The + temp-file-then-rename pattern existed six times across the admin, journal, + install and favicon code, each with its own (and by now different) temp + filename scheme. +- **Component testing is set up** + ([#622](https://github.com/ralksta/immich-folio/pull/622)). `jsdom` and + `@testing-library/react` land as dev dependencies, opt-in per file via a + `// @vitest-environment jsdom` pragma; extracting logic out of a + component is still preferred over rendering it wherever that's possible. + +### Upgrade notes + +**Bump `IMAGE_CACHE_VERSION` when you deploy this release**, the same +variable v0.15.0 introduced. Image URLs are served `immutable` with a +one-year cache; a browser (or an edge cache) that already has an `original` +response cached under a still-valid image URL keeps serving that original +after the upgrade unless the URL itself changes. Increment the value if it +is already set; set it to anything if it is not. + +No configuration schema changed beyond what `settings.yaml` validation +already accepts; there is nothing else to migrate. + ## [0.15.0] — 2026-09-19 ### Security diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 03b6eba..5f0052a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,6 +55,26 @@ npx vitest run # all tests must pass npm run build # build must succeed ``` +## Tests + +Tests live in `__tests__/` next to the code they cover, and run in Node by +default — most of what is worth testing is plain logic. + +A test that needs a DOM opts in per file, with this as its very first line: + +```tsx +// @vitest-environment jsdom +``` + +Then render with `@testing-library/react` and call `cleanup` in `afterEach`. +See `app/admin/__tests__/SaveBar.test.tsx` for a worked example. + +**Prefer extracting the logic over rendering it.** A pure function moved into +`lib/` — or into a sibling module, as `page-builder/albumEntries.ts` was — is +cheaper to test and stays tested when the markup around it changes. Reach for a +component test when the behaviour *is* the rendering: a state that hides a form, +a control that must stay disabled, a keyboard shortcut. + ## Code style - **TypeScript** — strict mode, no `any` without justification diff --git a/app/[...path]/PhotoGrid.tsx b/app/[...path]/PhotoGrid.tsx index 3aa5023..d40127c 100644 --- a/app/[...path]/PhotoGrid.tsx +++ b/app/[...path]/PhotoGrid.tsx @@ -2,8 +2,11 @@ * PhotoGrid — client component wrapping the masonry grid and lightbox. * Handles image click → lightbox open, keyboard nav, and EXIF fetching. * - * Deep-link support: syncs lightbox index with the URL hash (#photo-N). - * Sharing a link with a hash opens the lightbox to that photo directly. + * Deep-link support: syncs the lightbox with `?photo=`, a stable + * per-photo id rather than a position, so reordering or deleting other + * photos in the album does not repoint an already-shared link (#588). A + * legacy `#photo-N` hash is still read on the initial load — those links + * have been shared for months — but the grid never writes that form again. */ 'use client'; @@ -15,7 +18,7 @@ import { FadeIn } from '@/components/FadeIn'; import { ProofingProvider, useProofing } from '@/components/ProofingContext'; import { ProofingModal } from '@/components/ProofingModal'; import { useDictionary } from '@/components/I18nProvider'; -import { parsePhotoHash, buildPhotoHash } from '@/lib/photoHash'; +import { parsePhotoHash, parsePhotoQuery, buildPhotoQuery } from '@/lib/photoHash'; export interface PhotoItem { id: string; @@ -77,49 +80,73 @@ function PhotoGridInner({ return assets; }, [assets, proofing]); - // ── Initial hash check (Client-only to avoid hydration mismatch) ── + // ── Initial deep-link check (client-only to avoid hydration mismatch) ── + // ?photo= wins when present; #photo-N is read only as a fallback, + // for a link shared before this format existed. useEffect(() => { + const token = parsePhotoQuery(window.location.search); + const byToken = token ? displayedAssets.findIndex((a) => a.id === token) : -1; + if (byToken >= 0) { + // eslint-disable-next-line react-hooks/set-state-in-effect + setLightboxIndex(byToken); + return; + } const idx = parsePhotoHash(window.location.hash); if (idx !== null && idx < displayedAssets.length) { - // state from URL hash on mount only - // eslint-disable-next-line react-hooks/set-state-in-effect setLightboxIndex(idx); } - }, [displayedAssets.length]); + }, [displayedAssets]); - // ── Sync URL hash when lightbox state changes ───────────────── + // ── Sync the URL's `photo` param when the lightbox state changes ── useEffect(() => { - if (lightboxIndex !== null && lightboxIndex < displayedAssets.length) { - const hash = buildPhotoHash(lightboxIndex); - if (window.location.hash !== hash) { - window.history.replaceState(null, '', hash); - } + const assetId = + lightboxIndex !== null && lightboxIndex < displayedAssets.length + ? displayedAssets[lightboxIndex].id + : null; + if (assetId === null) return; + const query = buildPhotoQuery(window.location.search, assetId); + // Building the full URL (not just the search string) drops any leftover + // #photo-N hash from a legacy link this page was opened with. + const url = window.location.pathname + query; + if (window.location.pathname + window.location.search !== url || window.location.hash) { + window.history.replaceState(null, '', url); } - }, [lightboxIndex, displayedAssets.length]); + }, [lightboxIndex, displayedAssets]); - // ── Listen for browser back/forward (hash change) ───────────── + // ── Listen for browser back/forward ──────────────────────────── useEffect(() => { - const handleHashChange = () => { - const idx = parsePhotoHash(window.location.hash); - if (idx !== null && idx < displayedAssets.length) { - setLightboxIndex(idx); - } else { - setLightboxIndex(null); - } + const handlePopState = () => { + const token = parsePhotoQuery(window.location.search); + const byToken = token ? displayedAssets.findIndex((a) => a.id === token) : -1; + setLightboxIndex(byToken >= 0 ? byToken : null); }; - window.addEventListener('hashchange', handleHashChange); - return () => window.removeEventListener('hashchange', handleHashChange); - }, [displayedAssets.length]); + window.addEventListener('popstate', handlePopState); + return () => window.removeEventListener('popstate', handlePopState); + }, [displayedAssets]); - const openLightbox = useCallback((index: number) => { - setLightboxIndex(index); - window.history.pushState(null, '', buildPhotoHash(index)); - }, []); + const openLightbox = useCallback( + (index: number) => { + setLightboxIndex(index); + const asset = displayedAssets[index]; + if (asset) { + window.history.pushState( + null, + '', + window.location.pathname + buildPhotoQuery(window.location.search, asset.id), + ); + } + }, + [displayedAssets], + ); const closeLightbox = useCallback(() => { setLightboxIndex(null); - window.history.pushState(null, '', window.location.pathname + window.location.search); + window.history.pushState( + null, + '', + window.location.pathname + buildPhotoQuery(window.location.search, null), + ); }, []); const goNext = useCallback(() => { diff --git a/app/[...path]/SubpageGridView.tsx b/app/[...path]/SubpageGridView.tsx index eea504e..788ca65 100644 --- a/app/[...path]/SubpageGridView.tsx +++ b/app/[...path]/SubpageGridView.tsx @@ -3,6 +3,7 @@ import Image from 'next/image'; import { imageUrl } from '@/lib/urls'; import { SubpageSectionConfig } from '@/lib/config'; import { getServerDictionary } from '@/lib/i18n/server'; +import { BackLink } from '@/components/BackLink'; interface SubpageAlbum { id: string; @@ -30,6 +31,8 @@ interface SubpageGridViewProps { index?: number; /** `--subpage-columns` / `--subpage-gap` from the resolved grid config. */ gridStyle?: React.CSSProperties; + /** The subpage that follows this one in the nav/homepage order, if any (#591). */ + nextSubpage?: { slug: string; name: string }; } const pad2 = (n: number) => String(n).padStart(2, '0'); @@ -120,6 +123,7 @@ export function SubpageGridView({ sections, index, gridStyle, + nextSubpage, }: SubpageGridViewProps) { // Build lookup maps once const albumMap = new Map(albums.map((a) => [a.id, a])); @@ -131,22 +135,23 @@ export function SubpageGridView({ return (
- {(title || subtitle) && ( -
-
- {index !== undefined && ( - - )} - {title &&

{title}

} - {subtitle &&

{subtitle}

} -
+
+
+ + {index !== undefined && ( + + )} + {title &&

{title}

} + {subtitle &&

{subtitle}

} +
+ {(title || subtitle) && ( -
- )} + )} +
{/* Typographic Table of Contents */} {hasSections && ( @@ -194,6 +199,35 @@ export function SubpageGridView({ gridStyle={gridStyle} /> )} + + {/* Onward navigation to the next subpage in nav order (#591) — a subpage + grid otherwise dead-ends into the browser's back button. */} + {nextSubpage && ( + + )}
); } diff --git a/app/[...path]/page.tsx b/app/[...path]/page.tsx index 74a7ced..b3e3293 100644 --- a/app/[...path]/page.tsx +++ b/app/[...path]/page.tsx @@ -25,7 +25,7 @@ import { downloadUrl, assetAspectRatio, } from '@/lib/urls'; -import { encodeAssetId } from '@/lib/tokens'; +import { encodeAssetId, decodeAssetId } from '@/lib/tokens'; import { buildCoverGridVars, getConfig, @@ -34,7 +34,7 @@ import { resolveProofing, type GridConfig, } from '@/lib/config'; -import { isProtected, isAuthenticated } from '@/lib/auth'; +import { isProtected, isAuthenticated, withoutLockedAlbums } from '@/lib/auth'; import { isAdminAuthenticated } from '@/lib/admin/auth'; import PasswordGate from '@/components/PasswordGate'; import { AdminDiagnosticBanner } from '@/components/AdminDiagnosticBanner'; @@ -56,7 +56,21 @@ interface PathPageProps { searchParams?: Promise<{ [key: string]: string | string[] | undefined }>; } -export async function generateMetadata({ params }: PathPageProps): Promise { +/** + * Whether `key` is password-protected and this request has not unlocked it. + * generateMetadata runs unconditionally — unlike the page body, nothing + * downstream of it stops a real title, photo count or cover image from + * reaching an unauthenticated `` unless this is checked first + * (GHSA-fvgv-97g3-wjr7). + */ +async function isLocked(key: string, type: 'subpage' | 'album'): Promise { + if (!isProtected(key, type)) return false; + const cookieStore = await cookies(); + const getCookie = (name: string) => cookieStore.get(name)?.value; + return !isAuthenticated(key, getCookie, type); +} + +export async function generateMetadata({ params, searchParams }: PathPageProps): Promise { // Next hands catch-all segments over percent-encoded, so a non-ASCII slug // ("/家族相册") would never match a stored one. Decode once, here, and every // comparison downstream works on the same form (#522). @@ -64,58 +78,80 @@ export async function generateMetadata({ params }: PathPageProps): Promise a.type === 'IMAGE' || a.type === 'VIDEO').length; subtitle = `${count} photo${count === 1 ? '' : 's'}`; + if (photoAssetId) photoAsset = album.assets.find((a) => a.id === photoAssetId); } - } else { + } else if (!subpageLocked) { title = result.subpage.title || result.subpage.name; } } } else if (path.length === 2) { const album = await immich.getAlbumBySlug(path[1], slug); - if (album) { + if (album && !(await isLocked(slug, 'subpage')) && !(await isLocked(album.id, 'album'))) { title = album.albumName; const count = album.assets.filter((a) => a.type === 'IMAGE' || a.type === 'VIDEO').length; subtitle = `${count} photo${count === 1 ? '' : 's'}`; + if (photoAssetId) photoAsset = album.assets.find((a) => a.id === photoAssetId); } } else { const album = await immich.getAlbumBySlug(slug); - if (album) { + if (album && !(await isLocked(album.id, 'album'))) { title = album.albumName; const count = album.assets.filter((a) => a.type === 'IMAGE' || a.type === 'VIDEO').length; subtitle = `${count} photo${count === 1 ? '' : 's'}`; + if (photoAssetId) photoAsset = album.assets.find((a) => a.id === photoAssetId); } } - const ogUrl = `/api/og?title=${encodeURIComponent(title)}${subtitle ? `&subtitle=${encodeURIComponent(subtitle)}` : ''}`; + // The title stays the album's — it's still the context a reader wants — + // but the description prefers the photo's own caption, and the image is + // the photo itself rather than the generated text card. + const ogDescription = + photoAsset?.exifInfo?.description?.trim() || + description || + (subtitle ? `${title} — ${subtitle}` : undefined); + const ogImage = photoAsset + ? imageUrl(photoAsset.id, 'preview') + : `/api/og?title=${encodeURIComponent(title)}${subtitle ? `&subtitle=${encodeURIComponent(subtitle)}` : ''}`; return { title, - description: description || (subtitle ? `${title} — ${subtitle}` : undefined), + description: ogDescription, openGraph: { title, - description: description || (subtitle ? `${title} — ${subtitle}` : undefined), - images: [ogUrl], + description: ogDescription, + images: [ogImage], }, twitter: { card: 'summary_large_image', title, - description: description || (subtitle ? `${title} — ${subtitle}` : undefined), - images: [ogUrl], + description: ogDescription, + images: [ogImage], }, }; } @@ -229,7 +265,13 @@ export default async function PathPage({ params, searchParams }: PathPageProps) const { path: rawPath } = await params; const path = rawPath?.map(normalizeSlug); const sParams = (await searchParams) || {}; - const forceFresh = sParams.fresh === '1' || sParams.preview === 'true'; + // ?fresh=1 / ?preview=true forces a cache-bypassing Immich refetch — cheap + // to trigger, and every subpage request fans out into an album-list fetch + // plus paged metadata search calls per album. Gated to admins so it cannot + // be used to force-refresh (or, before the immich.ts fix alongside this, + // to empty) the shared cache from an unauthenticated request. + const forceFresh = + (sParams.fresh === '1' || sParams.preview === 'true') && (await isAdminAuthenticated()); const config = getConfig(); @@ -382,9 +424,13 @@ export default async function PathPage({ params, searchParams }: PathPageProps) ? loadEssayFromFile(result.subpage.essayFile) : null; - // Fetch assets from all subpage albums + // Fetch assets from the subpage's albums. An essay has no per-album + // gate to pass through, so an album with its own password stays out + // until it has been unlocked. + const cookieStore = await cookies(); + const openAlbums = withoutLockedAlbums(albums, (name) => cookieStore.get(name)?.value); const allAlbums = await Promise.all( - albums.map((a) => immich.getAlbumBySlug(a.slug, slug, forceFresh)), + openAlbums.map((a) => immich.getAlbumBySlug(a.slug, slug, forceFresh)), ); const allAssets = allAlbums.flatMap((a) => (a ? a.assets : [])); const images = toPhotoItems( @@ -400,7 +446,7 @@ export default async function PathPage({ params, searchParams }: PathPageProps) title: result.subpage.title || result.subpage.name, subtitle: result.subpage.subtitle, }, - blocks: albums.flatMap((a) => [ + blocks: openAlbums.flatMap((a) => [ { type: 'heading' as const, level: 2, text: a.albumName }, ...a.assets.map((asset) => ({ type: 'photo' as const, @@ -508,6 +554,15 @@ export default async function PathPage({ params, searchParams }: PathPageProps) const enabledSubpages = config.subpages.filter((sp) => sp.enabled !== false); const subpageIndex = enabledSubpages.findIndex((sp) => sp.slug === slug); + // The subpage that follows this one in the same list the nav/homepage + // build from — hidden and disabled subpages are already excluded there, + // so "next" never points somewhere the visitor could not otherwise reach + // (#591). + const navSubpages = await immich.getSubpages(forceFresh); + const navIndex = navSubpages.findIndex((sp) => sp.slug === slug); + const nextSubpage = + navIndex >= 0 && navIndex < navSubpages.length - 1 ? navSubpages[navIndex + 1] : undefined; + return ( = 0 ? { index: subpageIndex + 1 } : {})} + {...(nextSubpage + ? { nextSubpage: { slug: nextSubpage.slug, name: nextSubpage.name } } + : {})} /> ); } diff --git a/app/about/page.tsx b/app/about/page.tsx index 1e7e8e1..6a937a7 100644 --- a/app/about/page.tsx +++ b/app/about/page.tsx @@ -13,6 +13,7 @@ import { immich } from '@/lib/immich'; import { getConfig } from '@/lib/config'; import { getServerDictionary } from '@/lib/i18n/server'; import { notFound } from 'next/navigation'; +import { BackLink } from '@/components/BackLink'; import './about.css'; export const dynamic = 'force-dynamic'; @@ -104,6 +105,7 @@ export default async function AboutPage() { {/* ── Text ─────────────────────────────────────── */}
+ diff --git a/app/admin/AdminShell.tsx b/app/admin/AdminShell.tsx index 5a9cfd0..2951a8c 100644 --- a/app/admin/AdminShell.tsx +++ b/app/admin/AdminShell.tsx @@ -3,6 +3,7 @@ import { useState, useEffect, type ReactNode } from 'react'; import AdminLogin from './components/AdminLogin'; import AdminDashboard from './components/AdminDashboard'; +import { SESSION_EXPIRED_EVENT } from './components/sessionExpiry'; import './admin.css'; /** @@ -13,6 +14,13 @@ import './admin.css'; export default function AdminShell({ children }: { children: ReactNode }) { const [authenticated, setAuthenticated] = useState(null); const [enabled, setEnabled] = useState(true); + // Sessions last 24 hours (lib/admin/auth.ts), but nothing re-checked + // authentication after the mount-time check below — leave a tab open + // overnight and every admin request answered 401 while the UI still looked + // signed in (#596). SESSION_EXPIRED_EVENT is raised by the fetch call sites + // that noticed; this is what turns that into dropping back to the login + // screen with an explanation instead of each of them failing quietly. + const [sessionExpired, setSessionExpired] = useState(false); useEffect(() => { async function checkAuth() { @@ -28,6 +36,15 @@ export default function AdminShell({ children }: { children: ReactNode }) { checkAuth(); }, []); + useEffect(() => { + function onSessionExpired() { + setAuthenticated(false); + setSessionExpired(true); + } + window.addEventListener(SESSION_EXPIRED_EVENT, onSessionExpired); + return () => window.removeEventListener(SESSION_EXPIRED_EVENT, onSessionExpired); + }, []); + if (authenticated === null) { return (
@@ -51,7 +68,15 @@ export default function AdminShell({ children }: { children: ReactNode }) { } if (!authenticated) { - return setAuthenticated(true)} />; + return ( + { + setSessionExpired(false); + setAuthenticated(true); + }} + /> + ); } return setAuthenticated(false)}>{children}; diff --git a/app/admin/__tests__/AdminShell.test.tsx b/app/admin/__tests__/AdminShell.test.tsx new file mode 100644 index 0000000..5bf9332 --- /dev/null +++ b/app/admin/__tests__/AdminShell.test.tsx @@ -0,0 +1,71 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor, act } from '@testing-library/react'; +import AdminShell from '../AdminShell'; +import { SESSION_EXPIRED_EVENT } from '../components/sessionExpiry'; + +vi.mock('next/navigation', () => ({ + usePathname: () => '/admin', +})); + +afterEach(cleanup); + +/** + * Sessions last 24 hours, but nothing re-checked authentication after the + * mount-time check below — leave a tab open overnight and every admin + * request answered 401 while the UI still looked signed in (#596). + * SESSION_EXPIRED_EVENT, raised by any admin fetch call site that notices a + * 401, is what this pins: it drops the whole panel back to the login screen + * with an explanation, instead of each call site failing quietly. + */ +describe('AdminShell session expiry', () => { + it('returns to the login screen with a notice when the session expires', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string) => { + if (url === '/api/admin/auth') { + return new Response(JSON.stringify({ authenticated: true, enabled: true }), { + status: 200, + }); + } + // AdminDashboard's own status/doctor fetches — a failure there is + // already handled as "unknown", not the thing under test here. + return new Response(null, { status: 500 }); + }), + ); + + render( + +
Panel content
+ , + ); + + await waitFor(() => expect(screen.getByText('Panel content')).toBeTruthy()); + expect(screen.queryByLabelText('Password')).toBeNull(); + + act(() => { + window.dispatchEvent(new CustomEvent(SESSION_EXPIRED_EVENT)); + }); + + await waitFor(() => + expect(screen.getByText('Your session expired. Sign in again.')).toBeTruthy(), + ); + expect(screen.queryByText('Panel content')).toBeNull(); + }); + + it('shows no notice on an ordinary first visit', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(JSON.stringify({ authenticated: false, enabled: true }))), + ); + + render( + +
Panel content
+ , + ); + + await waitFor(() => expect(screen.getByLabelText('Password')).toBeTruthy()); + expect(screen.queryByText('Your session expired. Sign in again.')).toBeNull(); + }); +}); diff --git a/app/admin/__tests__/SaveBar.test.tsx b/app/admin/__tests__/SaveBar.test.tsx new file mode 100644 index 0000000..24973c2 --- /dev/null +++ b/app/admin/__tests__/SaveBar.test.tsx @@ -0,0 +1,88 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent } from '@testing-library/react'; +import SaveBar from '../components/SaveBar'; + +/** + * The first component test in the project, and deliberately a small one: it is + * here to show the shape (`// @vitest-environment jsdom` on the first line, + * render, assert, cleanup) as much as to cover SaveBar. + * + * SaveBar earns it though. It decides whether an outcome reads as success or + * failure by testing whether the message happens to start with "Error" — #600 + * replaces that with a typed state, and this pins the behaviour so the swap can + * be verified rather than eyeballed. + */ + +afterEach(cleanup); + +const props = { + dirty: false, + saving: false, + saveMessage: '', + onSave: () => {}, + label: 'Save', +}; + +describe('SaveBar', () => { + it('stays out of the way when there is nothing to act on', () => { + const { container } = render(); + + // It is pinned over the page, so rendering it with nothing to say would + // cover content for no reason. + expect(container.firstChild).toBeNull(); + }); + + it('announces unsaved changes', () => { + render(); + + expect(screen.getByText('Unsaved changes')).toBeTruthy(); + }); + + it('only offers saving when there is something to save', () => { + const { rerender } = render(); + expect(screen.getByRole('button', { name: 'Save' })).toHaveProperty('disabled', true); + + rerender(); + expect(screen.getByRole('button', { name: 'Save' })).toHaveProperty('disabled', false); + }); + + it('blocks a second save while one is in flight', () => { + render(); + + const button = screen.getByRole('button', { name: 'Saving...' }); + expect(button).toHaveProperty('disabled', true); + }); + + it('calls onSave when clicked', () => { + const onSave = vi.fn(); + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + + expect(onSave).toHaveBeenCalledOnce(); + }); + + it('tells a failure from a success by the message prefix', () => { + // Pinned as it is, not as it should be: the prefix check is the thing #600 + // replaces. A message that fails without starting with "Error" is styled as + // a success today, which is precisely the argument for a typed state. + const { rerender } = render(); + expect(screen.getByText('Error: Failed to save').className).toContain('error'); + + rerender(); + expect(screen.getByText('Saved!').className).toContain('success'); + + rerender(); + expect(screen.getByText('Could not reach the server').className).toContain('success'); + }); + + it('offers the preview link only where it belongs', () => { + const { rerender } = render(); + expect(screen.queryByRole('link')).toBeNull(); + + rerender(); + // `?fresh=1` is what bypasses the cache, so the preview shows the save. + expect(screen.getByRole('link')).toHaveProperty('href', expect.stringContaining('/?fresh=1')); + }); +}); diff --git a/app/admin/__tests__/album-entries-round-trip.test.ts b/app/admin/__tests__/album-entries-round-trip.test.ts new file mode 100644 index 0000000..0031c25 --- /dev/null +++ b/app/admin/__tests__/album-entries-round-trip.test.ts @@ -0,0 +1,80 @@ +import { describe, it, expect } from 'vitest'; +import { + parseAlbumEntries, + serializeAlbumEntries, + hasAlbumOptions, +} from '../components/page-builder/albumEntries'; +import type { AlbumEntryObject } from '@/lib/config/schema'; + +/** + * The page builder rewrites gallery.yaml wholesale on every save, so a field it + * does not know about is not merely ignored — it is deleted. `location` was + * lost this way (#614): an album deliberately placed at city level, or hidden + * from the map, silently went back to exact coordinates the next time anything + * unrelated was saved. + * + * The round-trip test below is the part that matters. It is driven by a sample + * entry carrying every key of AlbumEntryObject, so the next field added to the + * schema and forgotten here fails this suite instead of quietly disappearing + * from someone's configuration. + */ + +const uuid = '11111111-1111-1111-1111-111111111111'; + +/** + * One value per key of AlbumEntryObject. The type annotation is the enforcement: + * adding a key to the schema without adding it here is a type error, and the + * round-trip test then proves the builder preserves it. + */ +const everyOption: Required> & Pick = { + title: 'Kloster Chorin', + description: 'Brick gothic in the Uckermark', + password: 'hunter2', + heroImage: '22222222-2222-2222-2222-222222222222', + sort: 'oldest', + assetOrder: ['33333333-3333-3333-3333-333333333333'], + coverPosition: '50% 25%', + download: true, + location: 'hidden', + grid: { columns: 4, gap: 8, aspectRatio: '3/2', layout: 'masonry' }, +}; + +describe('album entry round trip', () => { + it('preserves every option a gallery.yaml entry can carry', () => { + const parsed = parseAlbumEntries([{ [uuid]: everyOption }]); + const [serialized] = serializeAlbumEntries(parsed); + + expect(serialized).toEqual({ [uuid]: everyOption }); + }); + + it('keeps map precision across a save', () => { + const parsed = parseAlbumEntries([{ [uuid]: { location: 'city' } }]); + + expect(parsed[0].location).toBe('city'); + expect(serializeAlbumEntries(parsed)).toEqual([{ [uuid]: { location: 'city' } }]); + }); + + it('does not collapse a location-only entry back to a bare id', () => { + // This is the actual mechanism of the bug: an entry that looks "empty" is + // written back as a plain UUID string, taking its options with it. + const entry = { id: uuid, location: 'hidden' }; + + expect(hasAlbumOptions(entry)).toBe(true); + expect(serializeAlbumEntries([entry])).toEqual([{ [uuid]: { location: 'hidden' } }]); + }); + + it('carries a value this version does not recognise', () => { + // Narrowing belongs to deriveGallery(), which rejects a typo loudly. The + // builder's job is to hand back what it was given rather than drop it. + const parsed = parseAlbumEntries([{ [uuid]: { location: 'district' } }]); + + expect(serializeAlbumEntries(parsed)).toEqual([{ [uuid]: { location: 'district' } }]); + }); + + it('still collapses an entry that really is bare', () => { + expect(serializeAlbumEntries([{ id: uuid }])).toEqual([uuid]); + expect(serializeAlbumEntries([{ id: uuid, title: 'Just a title' }])).toEqual([ + { [uuid]: 'Just a title' }, + ]); + }); +}); diff --git a/app/admin/components/AdminDashboard.tsx b/app/admin/components/AdminDashboard.tsx index d2e4400..79401f2 100644 --- a/app/admin/components/AdminDashboard.tsx +++ b/app/admin/components/AdminDashboard.tsx @@ -6,6 +6,7 @@ import { usePathname } from 'next/navigation'; import BackupManagerModal from './BackupManagerModal'; import type { DoctorLevel } from '@/lib/admin/doctor'; import { DOCTOR_LEVEL_EVENT, systemHealth } from './systemHealth'; +import { reportIfSessionExpired } from './sessionExpiry'; import * as Icons from './Icons'; interface Props { @@ -15,10 +16,11 @@ interface Props { } const TABS = [ - { label: 'Pages', href: '/admin/pages', match: /^\/admin(\/pages)?$/ }, + { label: 'Pages', href: '/admin/pages', match: /^\/admin\/pages$/ }, { label: 'Journal', href: '/admin/journal', match: /^\/admin\/journal/ }, { label: 'Settings', href: '/admin/settings', match: /^\/admin\/settings/ }, { label: 'Analytics', href: '/admin/analytics', match: /^\/admin\/analytics/ }, + { label: 'Diagnostics', href: '/admin/diagnostics', match: /^\/admin\/diagnostics/ }, { label: 'Help', href: '/admin/help', match: /^\/admin\/help/ }, ]; @@ -47,7 +49,15 @@ export default function AdminDashboard({ onLogout, children }: Props) { async function handleReload() { setSaving(true); try { - await fetch('/api/admin/reload', { method: 'POST' }); + const res = await fetch('/api/admin/reload', { method: 'POST' }); + // A 401 or a 500 used to be indistinguishable from success here — the + // result was never checked at all (#596). + if (!res.ok) { + if (!reportIfSessionExpired(res)) { + alert(`Reload failed (HTTP ${res.status}).`); + } + return; + } // Refresh status after reload await fetchStatus(); } finally { @@ -141,15 +151,19 @@ export default function AdminDashboard({ onLogout, children }: Props) {

Immich Folio

@@ -247,24 +261,6 @@ export default function AdminDashboard({ onLogout, children }: Props) { )}
-
- setShowStatus(false)} - > - Diagnostics - - -
)} diff --git a/app/admin/components/AdminLogin.tsx b/app/admin/components/AdminLogin.tsx index d70621d..1cd9d65 100644 --- a/app/admin/components/AdminLogin.tsx +++ b/app/admin/components/AdminLogin.tsx @@ -4,9 +4,11 @@ import { useState } from 'react'; interface Props { onSuccess: () => void; + /** Shown above the form, e.g. "Your session expired. Sign in again." (#596) */ + notice?: string; } -export default function AdminLogin({ onSuccess }: Props) { +export default function AdminLogin({ onSuccess, notice }: Props) { const [password, setPassword] = useState(''); const [error, setError] = useState(''); const [loading, setLoading] = useState(false); @@ -44,6 +46,8 @@ export default function AdminLogin({ onSuccess }: Props) {

Admin Panel

+ {notice &&
{notice}
} +
Checks the things that fail silently — a wrong proxy hop count, a deleted album ID, a - plaintext password, an album Immich does not consider shared. It also sits behind the - status badge in the top bar, which is where it belongs when the question is whether - anything needs attention right now. + plaintext password, an album Immich does not consider shared. Open it from the{' '} + Diagnostics tab in the top bar; the status badge next to it still gives + a quick read on whether anything needs attention right now.
- -
- - Open diagnostics - -
); diff --git a/app/admin/components/JournalStudio.tsx b/app/admin/components/JournalStudio.tsx index 51b70e6..39010b4 100644 --- a/app/admin/components/JournalStudio.tsx +++ b/app/admin/components/JournalStudio.tsx @@ -26,6 +26,7 @@ import { import AssetPicker from './AssetPicker'; import { BlockBadge } from './BlockBadge'; import { useUnsavedGuard } from './useUnsavedGuard'; +import { reportIfSessionExpired } from './sessionExpiry'; import { EssayView } from '@/app/[...path]/EssayView'; import type { PhotoItem } from '@/app/[...path]/PhotoGrid'; import './journal-studio.css'; @@ -435,23 +436,33 @@ function JournalEditor({ slug, onBack }: JournalEditorProps) { title: string; } | null>(null); + /** Set when the entry could not be fetched; blocks saving over it. */ + const [loadError, setLoadError] = useState(null); + // Load entry useEffect(() => { async function load() { setLoading(true); + setLoadError(null); try { const res = await fetch(`/api/admin/journal/${slug}`); - if (res.ok) { - const data = await res.json(); - const md = data.entry.rawMarkdown; - setRawMarkdown(md); - setParsed(parseJournalMarkdown(md)); - setDirty(false); - } else { - alert('Failed to load journal entry'); + if (!res.ok) { + throw new Error( + res.status === 401 + ? 'Your session has expired. Sign in again to continue.' + : `The server answered ${res.status}.`, + ); } - } catch { - alert('Error loading journal entry'); + const data = await res.json(); + const md = data.entry.rawMarkdown; + setRawMarkdown(md); + setParsed(parseJournalMarkdown(md)); + setDirty(false); + } catch (err) { + console.error('Failed to load journal entry:', err); + // An empty editor saved over the entry replaces it with nothing, so + // this blocks the save rather than only reporting it. + setLoadError(err instanceof Error ? err.message : 'The entry could not be loaded.'); } finally { setLoading(false); } @@ -504,6 +515,9 @@ function JournalEditor({ slug, onBack }: JournalEditorProps) { // Save const handleSave = async () => { + // The editor is not rendered in this state, but Cmd+S still reaches here. + if (loadError) return; + setSaving(true); try { const res = await fetch(`/api/admin/journal/${slug}`, { @@ -516,7 +530,7 @@ function JournalEditor({ slug, onBack }: JournalEditorProps) { if (res.ok) { setDirty(false); - } else { + } else if (!reportIfSessionExpired(res)) { const data = await res.json(); alert(data.error || 'Failed to save'); } @@ -636,6 +650,25 @@ function JournalEditor({ slug, onBack }: JournalEditorProps) { ); } + /** + * Replaces the editor rather than sitting above it: an empty editor saved + * over the entry would replace the text with nothing. + */ + if (loadError) { + return ( +
+ This entry could not be loaded. {loadError} +

+ Nothing has been changed. Saving stays disabled until it loads, so an empty editor cannot + overwrite the entry. +

+ +
+ ); + } + return (
{/* Top Bar */} diff --git a/app/admin/components/PageBuilder.tsx b/app/admin/components/PageBuilder.tsx index e8fc018..ef62507 100644 --- a/app/admin/components/PageBuilder.tsx +++ b/app/admin/components/PageBuilder.tsx @@ -27,6 +27,7 @@ import AlbumDrawer from './page-builder/AlbumDrawer'; import { SortableAlbumCard } from './page-builder/AlbumCard'; import SubpageDrawer from './page-builder/SubpageDrawer'; import { findAlbumAddress } from './page-builder/findAlbumAddress'; +import { parseAlbumEntries, serializeAlbumEntries } from './page-builder/albumEntries'; import { seedCoverGrid, type ActiveEditAlbumAddress, @@ -40,8 +41,7 @@ import { } from './page-builder/types'; import { useScrollLock } from './useScrollLock'; import { useUnsavedGuard } from './useUnsavedGuard'; -import { DEFAULT_ALBUM_SORT, isAlbumSortMode } from '@/lib/albumSort'; -import { type AlbumEntryObject } from '@/lib/config/schema'; +import { reportIfSessionExpired } from './sessionExpiry'; import { IconCamera, IconFolder, @@ -62,73 +62,6 @@ interface GalleryState { subpages: Subpage[]; } -// ── Helpers ──────────────────────────────────────────────────── - -type RawAlbumEntry = string | Record; - -/** - * Whether an entry carries anything beyond its ID. - * - * Both collapse rules below depend on this, and they are the reason every new - * per-album option has to be listed here: an entry that looks "empty" is - * serialized back to a bare UUID string, so a field missing from this check is - * silently dropped on the next save. - */ -function hasAlbumOptions(entry: AlbumEntry): boolean { - return Boolean( - entry.description || - entry.password || - entry.heroImage || - (entry.sort && entry.sort !== DEFAULT_ALBUM_SORT) || - entry.assetOrder?.length || - entry.grid || - entry.coverPosition, - ); -} - -function parseAlbumEntries(raw: RawAlbumEntry[] | undefined): AlbumEntry[] { - if (!raw) return []; - return raw.map((entry) => { - if (typeof entry === 'string') return { id: entry }; - const [id, value] = Object.entries(entry)[0]; - if (typeof value === 'string') return { id, title: value }; - return { - id, - title: value.title, - description: value.description, - password: value.password, - heroImage: value.heroImage, - sort: isAlbumSortMode(value.sort) ? value.sort : undefined, - assetOrder: value.assetOrder, - grid: value.grid, - coverPosition: value.coverPosition, - }; - }); -} - -function serializeAlbumEntries(entries: AlbumEntry[]): RawAlbumEntry[] { - return entries.map((entry) => { - const extras = hasAlbumOptions(entry); - if (!entry.title && !extras) return entry.id; - if (entry.title && !extras) return { [entry.id]: entry.title }; - - const val: AlbumEntryObject = {}; - // Only when set: a sort-only entry would otherwise be written with an empty - // title, which deriveGallery ignores but which still lands in the YAML. - if (entry.title) val.title = entry.title; - if (entry.description) val.description = entry.description; - if (entry.password) val.password = entry.password; - if (entry.heroImage) val.heroImage = entry.heroImage; - if (entry.sort && entry.sort !== DEFAULT_ALBUM_SORT) val.sort = entry.sort; - // Persisted regardless of the mode, so manual → newest → manual does not - // throw away a hand-curated order. - if (entry.assetOrder?.length) val.assetOrder = entry.assetOrder; - if (entry.grid) val.grid = entry.grid; - if (entry.coverPosition) val.coverPosition = entry.coverPosition; - return { [entry.id]: val }; - }); -} - // ── Sortable Hero Tile ───────────────────────────────────────── function SortableHeroTile({ @@ -269,6 +202,12 @@ export default function PageBuilder() { const [gallery, setGallery] = useState({ hero: [], albums: [], subpages: [] }); const [immichAlbums, setImmichAlbums] = useState([]); const [loading, setLoading] = useState(true); + /** + * Set when gallery.yaml could not be fetched. It blocks saving: the builder + * writes the whole file from its own state, so saving an empty builder + * publishes an empty gallery. + */ + const [loadError, setLoadError] = useState(null); const [saving, setSaving] = useState(false); const [dirty, setDirty] = useState(false); const [saveMessage, setSaveMessage] = useState(''); @@ -356,25 +295,35 @@ export default function PageBuilder() { async function loadData() { setLoading(true); + setLoadError(null); try { const [galleryRes, albumsRes] = await Promise.all([ fetch('/api/admin/gallery'), fetch('/api/admin/albums'), ]); - if (galleryRes.ok) { - const { gallery: raw } = await galleryRes.json(); - const parsed = parseGalleryYaml(raw); - setGallery(parsed); - openAlbumFromLink(parsed); + if (!galleryRes.ok) { + throw new Error( + galleryRes.status === 401 + ? 'Your session has expired. Sign in again to continue.' + : `The server answered ${galleryRes.status}.`, + ); } + const { gallery: raw } = await galleryRes.json(); + const parsed = parseGalleryYaml(raw); + setGallery(parsed); + openAlbumFromLink(parsed); + + // A failed album list is survivable — it only empties the picker, and + // saving with it empty changes nothing in gallery.yaml. if (albumsRes.ok) { const { albums } = await albumsRes.json(); setImmichAlbums(albums); } } catch (err) { console.error('Failed to load admin data:', err); + setLoadError(err instanceof Error ? err.message : 'The page structure could not be loaded.'); } finally { setLoading(false); } @@ -424,6 +373,7 @@ export default function PageBuilder() { password: sp.password as string | undefined, enabled: sp.enabled !== false, hidden: sp.hidden === true, + location: sp.location as string | undefined, essayText: sp.essayText as string | undefined, essayFile: sp.essayFile as string | undefined, albums: parseAlbumEntries(sp.albums as Array> | undefined), @@ -451,6 +401,7 @@ export default function PageBuilder() { password: sp.password as string | undefined, enabled: sp.enabled !== false, hidden: sp.hidden === true, + location: sp.location as string | undefined, essayText: sp.essayText as string | undefined, essayFile: sp.essayFile as string | undefined, albums: parseAlbumEntries( @@ -474,6 +425,9 @@ export default function PageBuilder() { // ── Save ────────────────────────────────────────────────────── async function handleSave() { + // Not reachable from the UI in this state, but Cmd+S still gets here. + if (loadError) return; + setSaving(true); setSaveMessage(''); @@ -497,6 +451,7 @@ export default function PageBuilder() { if (sp.essayFile) entry.essayFile = sp.essayFile; if (sp.grid) entry.grid = sp.grid; if (sp.coverGrid) entry.coverGrid = sp.coverGrid; + if (sp.location) entry.location = sp.location; if (sp.sections && sp.sections.length > 0) { entry.sections = sp.sections.map((sec) => { @@ -530,7 +485,7 @@ export default function PageBuilder() { setDirty(false); setSaveMessage(data.message || 'Saved successfully!'); setTimeout(() => setSaveMessage(''), 5000); - } else { + } else if (!reportIfSessionExpired(res)) { const err = await res.json(); setSaveMessage(`Error: ${err.error}`); } @@ -729,7 +684,12 @@ export default function PageBuilder() { } // ── Album Removal ──────────────────────────────────────────── + // Confirmed like removeSubpage: an album entry carries its grid overrides, + // cover asset and manual assetOrder with it, there is no undo stack in the + // builder, and the only escape from an accidental click used to be + // reloading the page — which discards every other unsaved edit too (#597). function removeStandaloneAlbum(index: number) { + if (!confirm('Remove this album from the gallery?')) return; setGallery((g) => ({ ...g, albums: g.albums.filter((_, i) => i !== index), @@ -738,6 +698,7 @@ export default function PageBuilder() { } function removeSubpageAlbum(subpageIndex: number, albumIndex: number) { + if (!confirm('Remove this album from the subpage?')) return; setGallery((g) => { const subpages = [...g.subpages]; const sp = { ...subpages[subpageIndex] }; @@ -749,6 +710,7 @@ export default function PageBuilder() { } function removeSectionAlbum(subpageIndex: number, sectionIndex: number, albumIndex: number) { + if (!confirm('Remove this album from the section?')) return; setGallery((g) => { const subpages = [...g.subpages]; const sp = { ...subpages[subpageIndex] }; @@ -765,6 +727,7 @@ export default function PageBuilder() { // ── Hero Management ────────────────────────────────────────── function removeHero(index: number) { + if (!confirm('Remove this photo from the home page hero?')) return; setGallery((g) => ({ ...g, hero: g.hero.filter((_, i) => i !== index), @@ -813,6 +776,26 @@ export default function PageBuilder() { ); } + /** + * Replaces the builder rather than sitting above it. An empty builder looks + * exactly like a site with no pages yet, and saving it used to publish an + * empty gallery.yaml over a working one. + */ + if (loadError) { + return ( +
+ The page structure could not be loaded. {loadError} +

+ Nothing has been changed. Saving stays disabled until it loads, so an empty builder cannot + overwrite your gallery. +

+ +
+ ); + } + // Filter standalone albums const filteredAlbums = gallery.albums.filter((album) => { if (!searchQuery) return true; diff --git a/app/admin/components/SettingsEditor.tsx b/app/admin/components/SettingsEditor.tsx index 8d7c583..6854383 100644 --- a/app/admin/components/SettingsEditor.tsx +++ b/app/admin/components/SettingsEditor.tsx @@ -6,6 +6,7 @@ import Link from 'next/link'; import * as Icons from './Icons'; import SaveBar from './SaveBar'; import { useUnsavedGuard } from './useUnsavedGuard'; +import { reportIfSessionExpired } from './sessionExpiry'; import ToggleCard from './fields/ToggleCard'; import OptionGrid, { toOptions } from './fields/OptionGrid'; // Direct import from the theme module, not from '@/lib/config': the config @@ -587,6 +588,12 @@ export default function SettingsEditor() { source: 'env' | 'settings' | 'none'; } | null>(null); const [loading, setLoading] = useState(true); + /** + * Set when the settings could not be fetched. It blocks saving, because an + * empty form saved over a live settings.yaml replaces it with whatever one + * field the user happened to touch. + */ + const [loadError, setLoadError] = useState(null); // Collapsed by default: the four metadata switches are a detail of one // decision, and showing them permanently is what made the section a wall (#510). const [metadataOpen, setMetadataOpen] = useState(false); @@ -671,15 +678,28 @@ export default function SettingsEditor() { async function loadSettings() { setLoading(true); + setLoadError(null); try { const res = await fetch('/api/admin/settings'); - if (res.ok) { - const { settings: data, siteUrl } = await res.json(); - setSettings(data || {}); - setSiteUrlInfo(siteUrl ?? null); + if (!res.ok) { + // 401 is the common one: the session lasts 24h and nothing re-checks + // it, so a tab left open overnight lands here. reportIfSessionExpired + // also drops the whole panel back to the login screen (#596); the + // thrown message below still covers the brief moment before that + // re-render happens, and any other non-ok status. + reportIfSessionExpired(res); + throw new Error( + res.status === 401 + ? 'Your session has expired. Sign in again to continue.' + : `The server answered ${res.status}.`, + ); } + const { settings: data, siteUrl } = await res.json(); + setSettings(data || {}); + setSiteUrlInfo(siteUrl ?? null); } catch (err) { console.error('Failed to load settings:', err); + setLoadError(err instanceof Error ? err.message : 'The settings could not be loaded.'); } finally { setLoading(false); } @@ -726,6 +746,9 @@ export default function SettingsEditor() { } async function handleSave() { + // The form is not rendered in this state, but Cmd+S still reaches here. + if (loadError) return; + setSaving(true); setSaveMessage(''); @@ -750,9 +773,15 @@ export default function SettingsEditor() { setSaveMessage(data.message || 'Saved!'); router.refresh(); setTimeout(() => setSaveMessage(''), 5000); - } else { + } else if (!reportIfSessionExpired(res)) { const err = await res.json(); - setSaveMessage(`Error: ${err.error}`); + // A rejected save names the fields that caused it. Listing them beats + // "could not be saved" over a form with forty inputs; putting the + // message next to each input is the job of #600. + const fields: string[] = Array.isArray(err.fields) + ? err.fields.map((f: { field?: string }) => f.field || 'settings') + : []; + setSaveMessage(`Error: ${err.error}${fields.length ? ` — ${fields.join(', ')}` : ''}`); } } catch { setSaveMessage('Error: Failed to save'); @@ -763,23 +792,36 @@ export default function SettingsEditor() { async function loadAboutContent() { setAboutLoading(true); + setAboutMessage(''); try { const res = await fetch('/api/admin/about'); - if (res.ok) { - const data = await res.json(); - setAboutMeta(data.meta || {}); - setAboutBody(data.body || ''); - setAboutGearText(data.meta?.gear?.join('\n') || ''); - setAboutLoaded(true); + if (!res.ok) { + throw new Error( + res.status === 401 + ? 'Your session has expired. Sign in again to continue.' + : `The server answered ${res.status}.`, + ); } + const data = await res.json(); + setAboutMeta(data.meta || {}); + setAboutBody(data.body || ''); + setAboutGearText(data.meta?.gear?.join('\n') || ''); + setAboutLoaded(true); } catch (err) { console.error('Failed to load about content:', err); + // aboutLoaded stays false, which is what blocks the save below: an empty + // editor written to about.md would replace the page with nothing. + setAboutMessage( + `Error: ${err instanceof Error ? err.message : 'The About page could not be loaded.'}`, + ); } finally { setAboutLoading(false); } } async function saveAboutContent() { + if (!aboutLoaded) return; + setAboutSaving(true); setAboutMessage(''); const cleanedMeta = { ...aboutMeta }; @@ -842,6 +884,26 @@ export default function SettingsEditor() { ); } + /** + * Deliberately replaces the form rather than sitting above it. An empty form + * is indistinguishable from a site with no settings yet, and editing one + * field in it used to publish that field as the entire settings.yaml. + */ + if (loadError) { + return ( +
+ Settings could not be loaded. {loadError} +

+ Nothing has been changed. Saving stays disabled until they load, so an empty form cannot + overwrite your configuration. +

+ +
+ ); + } + // Resolved the same way the site resolves it, so the switches show what a // visitor actually sees — including a config that only ever set the older // `exifOnHover`. @@ -898,6 +960,7 @@ export default function SettingsEditor() { key={sec.id} href={`/admin/settings/${sec.id}`} className={`settings-nav-item ${activeSection === sec.id ? 'active' : ''}`} + aria-current={activeSection === sec.id ? 'page' : undefined} > {sec.label} diff --git a/app/admin/components/__tests__/AdminDashboard-reload.test.tsx b/app/admin/components/__tests__/AdminDashboard-reload.test.tsx new file mode 100644 index 0000000..3d21183 --- /dev/null +++ b/app/admin/components/__tests__/AdminDashboard-reload.test.tsx @@ -0,0 +1,98 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; +import AdminDashboard from '../AdminDashboard'; +import { SESSION_EXPIRED_EVENT } from '../sessionExpiry'; + +vi.mock('next/navigation', () => ({ + usePathname: () => '/admin', +})); + +afterEach(cleanup); + +/** + * `handleReload` used to await POST /api/admin/reload without checking + * `res.ok`, so a 401 or a 500 was indistinguishable from success (#596). + */ +describe('AdminDashboard reload failure reporting', () => { + it('reports a non-ok, non-401 reload failure', async () => { + const alertMock = vi.fn(); + vi.stubGlobal('alert', alertMock); + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string, init?: RequestInit) => { + if (url === '/api/admin/reload' && init?.method === 'POST') { + return new Response(null, { status: 500 }); + } + return new Response(null, { status: 500 }); // status/doctor probes + }), + ); + + render( + {}}> +
Panel
+ , + ); + + fireEvent.click(screen.getByTitle('Reload config & clear cache')); + + await waitFor(() => expect(alertMock).toHaveBeenCalledWith('Reload failed (HTTP 500).')); + }); + + it('raises the session-expired event instead of alerting on a 401', async () => { + const alertMock = vi.fn(); + vi.stubGlobal('alert', alertMock); + const listener = vi.fn(); + window.addEventListener(SESSION_EXPIRED_EVENT, listener); + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string, init?: RequestInit) => { + if (url === '/api/admin/reload' && init?.method === 'POST') { + return new Response(null, { status: 401 }); + } + return new Response(null, { status: 500 }); + }), + ); + + render( + {}}> +
Panel
+ , + ); + + fireEvent.click(screen.getByTitle('Reload config & clear cache')); + + await waitFor(() => expect(listener).toHaveBeenCalledOnce()); + expect(alertMock).not.toHaveBeenCalled(); + window.removeEventListener(SESSION_EXPIRED_EVENT, listener); + }); + + it('does not alert when the reload succeeds', async () => { + const alertMock = vi.fn(); + vi.stubGlobal('alert', alertMock); + const fetchMock = vi.fn(async (url: string, init?: RequestInit) => { + if (url === '/api/admin/reload' && init?.method === 'POST') { + return new Response(null, { status: 200 }); + } + return new Response(null, { status: 500 }); + }); + vi.stubGlobal('fetch', fetchMock); + + render( + {}}> +
Panel
+ , + ); + + fireEvent.click(screen.getByTitle('Reload config & clear cache')); + + await waitFor(() => + expect( + fetchMock.mock.calls.filter( + ([url, init]) => url === '/api/admin/reload' && init?.method === 'POST', + ), + ).toHaveLength(1), + ); + expect(alertMock).not.toHaveBeenCalled(); + }); +}); diff --git a/app/admin/components/__tests__/sessionExpiry.test.ts b/app/admin/components/__tests__/sessionExpiry.test.ts new file mode 100644 index 0000000..4e2ece1 --- /dev/null +++ b/app/admin/components/__tests__/sessionExpiry.test.ts @@ -0,0 +1,38 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { SESSION_EXPIRED_EVENT, reportIfSessionExpired } from '../sessionExpiry'; + +/** + * Sessions last 24 hours, but nothing re-checked authentication after the + * mount-time check in AdminShell — every admin action after that just failed + * quietly with the UI still looking signed in (#596). This is the one place + * every admin fetch call site is meant to route a non-ok response through. + */ +describe('reportIfSessionExpired', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('dispatches the event and returns true for a 401', () => { + const listener = vi.fn(); + window.addEventListener(SESSION_EXPIRED_EVENT, listener); + + const result = reportIfSessionExpired(new Response(null, { status: 401 })); + + expect(result).toBe(true); + expect(listener).toHaveBeenCalledOnce(); + window.removeEventListener(SESSION_EXPIRED_EVENT, listener); + }); + + it('does nothing and returns false for other statuses', () => { + const listener = vi.fn(); + window.addEventListener(SESSION_EXPIRED_EVENT, listener); + + for (const status of [200, 400, 403, 404, 500]) { + expect(reportIfSessionExpired(new Response(null, { status }))).toBe(false); + } + + expect(listener).not.toHaveBeenCalled(); + window.removeEventListener(SESSION_EXPIRED_EVENT, listener); + }); +}); diff --git a/app/admin/components/page-builder/albumEntries.ts b/app/admin/components/page-builder/albumEntries.ts new file mode 100644 index 0000000..0b967f4 --- /dev/null +++ b/app/admin/components/page-builder/albumEntries.ts @@ -0,0 +1,87 @@ +/** + * The gallery.yaml ↔ page-builder translation for album entries. + * + * Extracted from PageBuilder.tsx so it can be tested on its own: this is the + * code that decides what survives a save, and a field it does not know about + * is discarded silently rather than failing (#614). + */ + +import { DEFAULT_ALBUM_SORT, isAlbumSortMode } from '@/lib/albumSort'; +import { type AlbumEntryObject } from '@/lib/config/schema'; +import type { AlbumEntry } from './types'; + +export type RawAlbumEntry = string | Record; + +/** + * Whether an entry carries anything beyond its ID. + * + * Both collapse rules below depend on this, and they are the reason every new + * per-album option has to be listed here: an entry that looks "empty" is + * serialized back to a bare UUID string, so a field missing from this check is + * silently dropped on the next save. + */ +export function hasAlbumOptions(entry: AlbumEntry): boolean { + return Boolean( + entry.description || + entry.password || + entry.heroImage || + (entry.sort && entry.sort !== DEFAULT_ALBUM_SORT) || + entry.assetOrder?.length || + entry.grid || + entry.coverPosition || + entry.location || + entry.download, + ); +} + +export function parseAlbumEntries(raw: RawAlbumEntry[] | undefined): AlbumEntry[] { + if (!raw) return []; + return raw.map((entry) => { + if (typeof entry === 'string') return { id: entry }; + const [id, value] = Object.entries(entry)[0]; + if (typeof value === 'string') return { id, title: value }; + return { + id, + title: value.title, + description: value.description, + password: value.password, + heroImage: value.heroImage, + sort: isAlbumSortMode(value.sort) ? value.sort : undefined, + assetOrder: value.assetOrder, + grid: value.grid, + coverPosition: value.coverPosition, + location: value.location, + download: value.download, + }; + }); +} + +export function serializeAlbumEntries(entries: AlbumEntry[]): RawAlbumEntry[] { + return entries.map((entry) => { + const extras = hasAlbumOptions(entry); + if (!entry.title && !extras) return entry.id; + if (entry.title && !extras) return { [entry.id]: entry.title }; + + const val: AlbumEntryObject = {}; + // Only when set: a sort-only entry would otherwise be written with an empty + // title, which deriveGallery ignores but which still lands in the YAML. + if (entry.title) val.title = entry.title; + if (entry.description) val.description = entry.description; + if (entry.password) val.password = entry.password; + if (entry.heroImage) val.heroImage = entry.heroImage; + if (entry.sort && entry.sort !== DEFAULT_ALBUM_SORT) val.sort = entry.sort; + // Persisted regardless of the mode, so manual → newest → manual does not + // throw away a hand-curated order. + if (entry.assetOrder?.length) val.assetOrder = entry.assetOrder; + if (entry.grid) val.grid = entry.grid; + if (entry.coverPosition) val.coverPosition = entry.coverPosition; + // No UI sets this yet; it is preserved so that saving an unrelated change + // cannot quietly republish exact coordinates for an album deliberately + // placed at city level or hidden from the map (#469). + if (entry.location) val.location = entry.location; + // Same story as location: set by hand in YAML since #475, with no UI, so + // the builder was turning original downloads back off on the next save. + if (entry.download) val.download = entry.download; + return { [entry.id]: val }; + }); +} diff --git a/app/admin/components/page-builder/types.ts b/app/admin/components/page-builder/types.ts index 1297d06..4e7d117 100644 --- a/app/admin/components/page-builder/types.ts +++ b/app/admin/components/page-builder/types.ts @@ -13,6 +13,17 @@ export interface AlbumEntry { grid?: { columns?: number; gap?: number; aspectRatio?: string; layout?: string }; /** EXPERIMENTAL: focal point for the cover crop, e.g. "50% 25%" or "top" */ coverPosition?: string; + /** + * Map precision for this album: `exact`, `city`, `country` or `hidden` (#469). + * + * Carried verbatim rather than narrowed. The builder has no UI for it yet, so + * its only job is to hand back exactly what it was given — including a value + * this version does not recognise, which deriveGallery() is the right place + * to reject. + */ + location?: string; + /** Whether visitors may download the originals from this album (#475). */ + download?: boolean; } export interface Section { @@ -37,6 +48,8 @@ export interface Subpage { grid?: { columns?: number; gap?: number; aspectRatio?: string; layout?: string }; /** The album-cover tiles on this page only (#523). */ coverGrid?: { columns?: number; gap?: number; aspectRatio?: string; layout?: string }; + /** Map precision for every album here that does not set its own (#469). */ + location?: string; } /** diff --git a/app/admin/components/sessionExpiry.ts b/app/admin/components/sessionExpiry.ts new file mode 100644 index 0000000..3b7b5f2 --- /dev/null +++ b/app/admin/components/sessionExpiry.ts @@ -0,0 +1,24 @@ +/** + * Fired on `window` by an admin action whose request came back 401 — the + * session cookie expired (sessions last 24 hours) or was cleared elsewhere. + * AdminShell listens and drops back to the login screen with an explanation, + * instead of each save/reload path failing quietly while the UI still looks + * signed in (#596). + * + * Same pattern as DOCTOR_LEVEL_EVENT in systemHealth.ts: a window event + * rather than a prop or context, because the components that need to raise + * this (PageBuilder, SettingsEditor, JournalStudio, AdminDashboard) sit + * several routes below AdminShell, which owns the auth gate. + */ +export const SESSION_EXPIRED_EVENT = 'folio:admin-session-expired'; + +/** + * Call with the Response from an admin fetch. If it is a 401, dispatches the + * event and returns true, so the caller can skip its own generic error + * handling for that response and let AdminShell take over instead. + */ +export function reportIfSessionExpired(res: Response): boolean { + if (res.status !== 401) return false; + window.dispatchEvent(new CustomEvent(SESSION_EXPIRED_EVENT)); + return true; +} diff --git a/app/admin/page.tsx b/app/admin/page.tsx index 94fad4f..0f9df4c 100644 --- a/app/admin/page.tsx +++ b/app/admin/page.tsx @@ -1,5 +1,15 @@ -import PageBuilder from './components/PageBuilder'; +import { redirect } from 'next/navigation'; +/** + * `/admin` and `/admin/pages` both rendered the page builder, so the same + * screen had two addresses: two routes to keep in step, two things to find + * when something about the builder changes, and no way to tell which one a + * bookmark or a bug report meant. + * + * `/admin/pages` wins because that is what the Pages tab links to. `/admin` + * stays as the entry point people type and bookmark — and it is what the + * public diagnostic banner links to — so it redirects rather than disappearing. + */ export default function AdminHomePage() { - return ; + redirect('/admin/pages'); } diff --git a/app/admin/styles/base.css b/app/admin/styles/base.css index 813418b..a5af86f 100644 --- a/app/admin/styles/base.css +++ b/app/admin/styles/base.css @@ -298,3 +298,15 @@ font-size: 0.8rem; margin-bottom: 0.75rem; } + +/* Neutral, not an error: "your session expired" is expected behaviour after + 24 hours, not a mistake the operator made (#596). */ +.admin-notice { + background: var(--admin-bg-raised); + border: 1px solid var(--admin-border); + color: var(--admin-text-secondary); + padding: 0.5rem 0.75rem; + border-radius: 6px; + font-size: 0.8rem; + margin-bottom: 0.75rem; +} diff --git a/app/admin/styles/drawers.css b/app/admin/styles/drawers.css index d739dc0..58e3694 100644 --- a/app/admin/styles/drawers.css +++ b/app/admin/styles/drawers.css @@ -473,15 +473,6 @@ h3 .svg-icon { border-bottom: 1px solid var(--admin-border); } -/* ── Status Dropdown Footer ── */ -.status-dropdown-footer { - padding: 0.5rem 0.85rem; - background: var(--admin-bg-elevated); - border-top: 1px solid var(--admin-border); - display: flex; - justify-content: flex-end; -} - .admin-btn-sm { padding: 0.35rem 0.65rem !important; font-size: 0.75rem !important; diff --git a/app/admin/styles/help.css b/app/admin/styles/help.css index 2b30320..6b3e43f 100644 --- a/app/admin/styles/help.css +++ b/app/admin/styles/help.css @@ -60,10 +60,6 @@ font-size: 0.78rem; } -.help-action { - padding: 12px 4px 4px; -} - @media (max-width: 600px) { .help-shortcuts-row { grid-template-columns: 1fr; diff --git a/app/api/admin/__tests__/about-backup.test.ts b/app/api/admin/__tests__/about-backup.test.ts new file mode 100644 index 0000000..b278129 --- /dev/null +++ b/app/api/admin/__tests__/about-backup.test.ts @@ -0,0 +1,68 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('@/lib/admin/auth', () => ({ + isAdminEnabled: vi.fn(() => true), + isAdminAuthenticated: vi.fn(async () => true), + COOKIE_NAME: 'folio_admin_session', +})); + +vi.mock('next/cache', () => ({ revalidatePath: vi.fn() })); + +// The route imports from 'fs' directly; atomicWrite (used for the actual +// write) imports from 'fs/promises'. Both need the same mock functions so a +// test can see calls made through either. +const fsMock = vi.hoisted(() => ({ + mkdir: vi.fn(async () => undefined), + access: vi.fn(async () => undefined), + copyFile: vi.fn(async () => undefined), + readdir: vi.fn(async () => []), + unlink: vi.fn(async () => undefined), + writeFile: vi.fn(async () => undefined), + rename: vi.fn(async () => undefined), + readFile: vi.fn(async () => { + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + }), +})); +vi.mock('fs', () => ({ promises: fsMock })); +vi.mock('fs/promises', () => ({ default: fsMock, ...fsMock })); + +const fs = fsMock; +import { PUT } from '../about/route'; + +const put = (body: unknown) => + PUT( + new Request('http://localhost/api/admin/about', { + method: 'PUT', + body: JSON.stringify(body), + }), + ); + +/** + * "No file yet" and "the backup could not be written" used to share one + * catch, so a `.backups/` this save couldn't write to looked exactly like a + * brand-new file and the save went ahead with no snapshot (#630). + */ +describe('PUT /api/admin/about backup handling', () => { + beforeEach(() => vi.clearAllMocks()); + + it('saves a brand-new file with no backup', async () => { + vi.mocked(fs.access).mockRejectedValueOnce( + Object.assign(new Error('ENOENT'), { code: 'ENOENT' }), + ); + + const res = await put({ meta: { name: 'Me' }, body: 'Hello' }); + + expect(res.status).toBe(200); + expect(fs.copyFile).not.toHaveBeenCalled(); + expect(fs.writeFile).toHaveBeenCalledOnce(); + }); + + it('aborts the save when the file exists but the backup copy fails', async () => { + vi.mocked(fs.copyFile).mockRejectedValueOnce( + Object.assign(new Error('EACCES'), { code: 'EACCES' }), + ); + + await expect(put({ meta: { name: 'Me' }, body: 'Hello' })).rejects.toThrow('EACCES'); + expect(fs.writeFile).not.toHaveBeenCalled(); + }); +}); diff --git a/app/api/admin/__tests__/admin-guards.test.ts b/app/api/admin/__tests__/admin-guards.test.ts index f497aad..bb15975 100644 --- a/app/api/admin/__tests__/admin-guards.test.ts +++ b/app/api/admin/__tests__/admin-guards.test.ts @@ -266,3 +266,62 @@ describe('admin route guards', () => { expect(uncovered, `Admin routes with no guard test: ${uncovered.join(', ')}`).toEqual([]); }); }); + +/** + * The table above proves each route refuses an unauthenticated caller. This + * proves they all refuse it the *same way* — by going through withAdmin() + * rather than by carrying their own copy of the check, which is how the two + * refusals drifted apart in the first place. + */ +describe('admin routes are wrapped in withAdmin', () => { + const read = async () => { + const fs = await import('fs'); + const path = await import('path'); + const adminDir = path.join(process.cwd(), 'app/api/admin'); + + const files: { rel: string; src: string }[] = []; + const walk = (dir: string) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === '__tests__') continue; + walk(full); + } else if (entry.name === 'route.ts') { + const rel = path.relative(adminDir, full).replace(/\\/g, '/'); + // The login endpoint must stay reachable without a session. + if (rel.startsWith('auth/')) continue; + files.push({ rel, src: fs.readFileSync(full, 'utf8') }); + } + } + }; + walk(adminDir); + return files; + }; + + it('exports no handler outside the wrapper', async () => { + const offenders: string[] = []; + for (const { rel, src } of await read()) { + const bare = src.match(/export\s+(?:async\s+)?function\s+(GET|PUT|POST|DELETE|PATCH)\b/g); + if (bare) offenders.push(`${rel}: ${bare.join(', ')}`); + } + + expect( + offenders, + `Admin route handlers not wrapped in withAdmin():\n${offenders.join('\n')}`, + ).toEqual([]); + }); + + it('never re-implements the guard by hand', async () => { + const offenders: string[] = []; + for (const { rel, src } of await read()) { + if (src.includes('isAdminEnabled') || src.includes('isAdminAuthenticated')) { + offenders.push(rel); + } + } + + expect( + offenders, + `These call the auth helpers directly instead of using withAdmin():\n${offenders.join('\n')}`, + ).toEqual([]); + }); +}); diff --git a/app/api/admin/__tests__/doctor-guards.test.ts b/app/api/admin/__tests__/doctor-guards.test.ts index a20b6f6..0b2314c 100644 --- a/app/api/admin/__tests__/doctor-guards.test.ts +++ b/app/api/admin/__tests__/doctor-guards.test.ts @@ -14,18 +14,28 @@ vi.mock('@/lib/admin/auth', () => ({ COOKIE_NAME: 'folio_admin_session', })); +const baseConfig = { + needsCredentials: true, + trustedProxyHops: 0, + authSecret: 'x'.repeat(64), + albums: [] as string[], + standaloneAlbums: [] as string[], + albumOverrides: {} as Record, + subpages: [] as Array<{ name: string; albumIds: string[] }>, + albumPasswords: {}, + sitePassword: '', + immich: { apiUrl: '', apiKey: '' }, + immichTimeoutMs: 1000, +}; +const getConfigMock = vi.fn(() => baseConfig); + vi.mock('@/lib/config', () => ({ - getConfig: () => ({ - needsCredentials: true, - trustedProxyHops: 0, - authSecret: 'x'.repeat(64), - albums: [], - subpages: [], - albumPasswords: {}, - sitePassword: '', - immich: { apiUrl: '', apiKey: '' }, - immichTimeoutMs: 1000, - }), + getConfig: () => getConfigMock(), + slugify: (name: string) => + name + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, ''), })); vi.mock('@/lib/env', () => ({ env: { AUTH_SECRET: 'x'.repeat(64) } })); @@ -89,3 +99,119 @@ describe('GET /api/admin/doctor', () => { expect(proxy.level).toBe('warn'); }); }); + +/** + * `if (albums.length)` used to skip the album check entirely when Immich + * answered with an empty list — an API key regenerated under a different + * account, say — so every connection check passed while every album page was + * silently empty (#629). + */ +describe('GET /api/admin/doctor — album checks', () => { + const ALBUM_ID = '11111111-1111-1111-1111-111111111111'; + + function withCredentialsAndAlbums(albums: string[]) { + getConfigMock.mockReturnValueOnce({ + ...baseConfig, + needsCredentials: false, + albums, + immich: { apiUrl: 'https://immich.example', apiKey: 'key' }, + }); + } + + it('reports every configured album as missing when Immich returns an empty list', async () => { + withCredentialsAndAlbums([ALBUM_ID]); + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string) => { + if (url.includes('/albums')) return { ok: true, json: async () => [] } as Response; + return { ok: true, json: async () => ({}) } as Response; + }), + ); + + const body = await (await GET(req())).json(); + const albumFinding = body.findings.find((f: { id: string }) => f.id === 'album-ids'); + + expect(albumFinding?.level).toBe('error'); + expect(albumFinding?.title).toContain('1 of 1'); + + vi.unstubAllGlobals(); + }); + + it('does not reject a non-array album response outright', async () => { + withCredentialsAndAlbums([ALBUM_ID]); + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string) => { + if (url.includes('/albums')) return { ok: true, json: async () => ({}) } as Response; + return { ok: true, json: async () => ({}) } as Response; + }), + ); + + const body = await (await GET(req())).json(); + const albumFinding = body.findings.find((f: { id: string }) => f.id === 'album-ids'); + + // A non-array body is treated the same as an empty list, not left uncast + // and unchecked. + expect(albumFinding?.level).toBe('error'); + + vi.unstubAllGlobals(); + }); +}); + +/** + * Two albums that slugify to the same URL leave the second unreachable and + * its settings resolved from the first (#632). + */ +describe('GET /api/admin/doctor — album slug collisions', () => { + const knownAlbums = [ + { id: 'a', albumName: 'Japan Trip' }, + { id: 'b', albumName: 'Japan-Trip' }, + ]; + + function withCredentialsAndAlbums(config: Partial) { + getConfigMock.mockReturnValueOnce({ + ...baseConfig, + needsCredentials: false, + immich: { apiUrl: 'https://immich.example', apiKey: 'key' }, + ...config, + }); + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string) => { + if (url.includes('/albums')) return { ok: true, json: async () => knownAlbums } as Response; + return { ok: true, json: async () => ({}) } as Response; + }), + ); + } + + it('flags two standalone albums whose Immich names slugify alike', async () => { + withCredentialsAndAlbums({ albums: ['a', 'b'], standaloneAlbums: ['a', 'b'] }); + + const body = await (await GET(req())).json(); + const finding = body.findings.find((f: { id: string }) => f.id === 'album-slugs'); + + expect(finding?.level).toBe('error'); + expect(finding?.detail).toContain('Japan Trip'); + expect(finding?.detail).toContain('Japan-Trip'); + + vi.unstubAllGlobals(); + }); + + it('is quiet when the same names live in different subpages', async () => { + withCredentialsAndAlbums({ + albums: ['a', 'b'], + standaloneAlbums: [], + subpages: [ + { name: 'Trips A', albumIds: ['a'] }, + { name: 'Trips B', albumIds: ['b'] }, + ], + }); + + const body = await (await GET(req())).json(); + const finding = body.findings.find((f: { id: string }) => f.id === 'album-slugs'); + + expect(finding?.level).toBe('ok'); + + vi.unstubAllGlobals(); + }); +}); diff --git a/app/api/admin/__tests__/settings-write.test.ts b/app/api/admin/__tests__/settings-write.test.ts new file mode 100644 index 0000000..e6df529 --- /dev/null +++ b/app/api/admin/__tests__/settings-write.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('@/lib/admin/auth', () => ({ + isAdminEnabled: vi.fn(() => true), + isAdminAuthenticated: vi.fn(async () => true), + COOKIE_NAME: 'folio_admin_session', +})); + +vi.mock('@/lib/admin/yaml-service', () => ({ + readSettingsYaml: vi.fn(async () => ({})), + writeSettingsYaml: vi.fn(async () => undefined), +})); + +vi.mock('@/lib/config', () => ({ + invalidateConfigCache: vi.fn(), + getConfigOrNull: vi.fn(() => null), +})); + +vi.mock('@/lib/immich', () => ({ + immich: { invalidateAll: vi.fn() }, +})); + +vi.mock('next/cache', () => ({ revalidatePath: vi.fn() })); + +import { writeSettingsYaml } from '@/lib/admin/yaml-service'; +import { PUT } from '../settings/route'; + +const put = (settings: unknown) => + PUT( + new Request('http://localhost/api/admin/settings', { + method: 'PUT', + body: JSON.stringify({ settings }), + }), + ); + +/** + * The point of the check is not that a bad payload is reported — it is that + * nothing is written. settings.yaml is read by every public page, so a + * malformed save is discovered by visitors rather than by the person who made + * it (#599). + */ +describe('PUT /api/admin/settings', () => { + beforeEach(() => vi.clearAllMocks()); + + it('writes a valid payload', async () => { + const res = await put({ title: 'Folio', theme: 'minimal' }); + + expect(res.status).toBe(200); + expect(writeSettingsYaml).toHaveBeenCalledOnce(); + }); + + it('leaves the file untouched when the payload is malformed', async () => { + const res = await put({ title: { de: 'Folio' }, grid: 3 }); + + expect(res.status).toBe(400); + expect(writeSettingsYaml).not.toHaveBeenCalled(); + }); + + it('answers with the offending fields, so the panel can point at them', async () => { + const res = await put({ title: 1, theme: { fonts: { heading: 42 } } }); + const body = await res.json(); + + expect(body.fields.map((f: { field: string }) => f.field).sort()).toEqual([ + 'theme.fonts.heading', + 'title', + ]); + }); + + it('still writes settings carrying keys this version does not know', async () => { + // Older and newer configs are both in the wild, and there is no + // schemaVersion to tell them apart. + const res = await put({ title: 'Folio', fromALaterRelease: { nested: true } }); + + expect(res.status).toBe(200); + expect(writeSettingsYaml).toHaveBeenCalledOnce(); + }); +}); diff --git a/app/api/admin/about/route.ts b/app/api/admin/about/route.ts index 103e0fe..460921c 100644 --- a/app/api/admin/about/route.ts +++ b/app/api/admin/about/route.ts @@ -1,9 +1,10 @@ import { NextResponse } from 'next/server'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { revalidatePath } from 'next/cache'; import { promises as fs } from 'fs'; import path from 'path'; import yaml from 'js-yaml'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { atomicWrite } from '@/lib/atomicWrite'; const CONTENT_DIR = path.resolve(process.cwd(), 'content'); const FILENAME = 'about.md'; @@ -21,14 +22,7 @@ interface AboutBody { body?: string; } -export async function GET() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async () => { const filePath = path.join(CONTENT_DIR, FILENAME); let meta: AboutMeta = {}; let body = ''; @@ -49,16 +43,9 @@ export async function GET() { } return NextResponse.json({ meta, body }); -} - -export async function PUT(request: Request) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } +}); +export const PUT = withAdmin(async (request: Request) => { const data = (await request.json().catch(() => null)) as AboutBody | null; if (!data) { return NextResponse.json({ error: 'Invalid request body' }, { status: 400 }); @@ -77,10 +64,22 @@ export async function PUT(request: Request) { const content = `---\n${frontmatter}\n---\n\n${data.body ?? ''}\n`; const filePath = path.join(CONTENT_DIR, FILENAME); + await fs.mkdir(CONTENT_DIR, { recursive: true }); - // ── Backup existing file ──────────────────────────────────── + // "No file yet" and "the backup could not be written" used to share one + // catch, so a `.backups/` this save couldn't write to looked exactly like a + // brand-new file: the save went ahead with no snapshot taken (#630). Only + // ENOENT means there is nothing to back up; anything else aborts the save + // before it overwrites the live file. + let fileExists = true; try { await fs.access(filePath); + } catch (err) { + if ((err as NodeJS.ErrnoException & { code?: string }).code !== 'ENOENT') throw err; + fileExists = false; + } + + if (fileExists) { const backupDir = path.join(CONTENT_DIR, '.backups'); await fs.mkdir(backupDir, { recursive: true }); const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); @@ -92,22 +91,11 @@ export async function PUT(request: Request) { while (aboutBackups.length > MAX_BACKUPS) { await fs.unlink(path.join(backupDir, aboutBackups.shift()!)); } - } catch { - // File doesn't exist yet — no backup needed, but make sure the dir exists - await fs.mkdir(CONTENT_DIR, { recursive: true }); } - // ── Atomic write (temp file + rename) ─────────────────────── - const tmpPath = `${filePath}.${process.pid}.${Date.now()}.tmp`; - try { - await fs.writeFile(tmpPath, content, 'utf-8'); - await fs.rename(tmpPath, filePath); - } catch (err) { - await fs.unlink(tmpPath).catch(() => {}); - throw err; - } + await atomicWrite(filePath, content); revalidatePath('/about', 'layout'); return NextResponse.json({ success: true, message: 'About page saved.' }); -} +}); diff --git a/app/api/admin/albums/[albumId]/assets/route.ts b/app/api/admin/albums/[albumId]/assets/route.ts index 181a21c..85cc193 100644 --- a/app/api/admin/albums/[albumId]/assets/route.ts +++ b/app/api/admin/albums/[albumId]/assets/route.ts @@ -4,64 +4,56 @@ */ import { NextRequest, NextResponse } from 'next/server'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { getConfig } from '@/lib/config'; import { immich } from '@/lib/immich'; import { isUuid } from '@/lib/uuid'; -export async function GET( - request: NextRequest, - { params }: { params: Promise<{ albumId: string }> }, -) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } +export const GET = withAdmin( + async (request: NextRequest, { params }: { params: Promise<{ albumId: string }> }) => { + const config = getConfig(); + // Credentials, not `needsSetup`: this route is how the operator picks the + // albums that gallery.yaml is built from, so refusing to run until that file + // exists is a deadlock (#507). + if (config.needsCredentials) { + return NextResponse.json({ error: 'Immich not configured' }, { status: 503 }); + } - const config = getConfig(); - // Credentials, not `needsSetup`: this route is how the operator picks the - // albums that gallery.yaml is built from, so refusing to run until that file - // exists is a deadlock (#507). - if (config.needsCredentials) { - return NextResponse.json({ error: 'Immich not configured' }, { status: 503 }); - } + const { albumId } = await params; - const { albumId } = await params; + // Without this, `..%2f..%2fusers` resolves inside fetch() to an arbitrary + // Immich endpoint, called with the server's API key. + if (!isUuid(albumId)) { + return NextResponse.json({ error: 'Invalid album ID' }, { status: 400 }); + } - // Without this, `..%2f..%2fusers` resolves inside fetch() to an arbitrary - // Immich endpoint, called with the server's API key. - if (!isUuid(albumId)) { - return NextResponse.json({ error: 'Invalid album ID' }, { status: 400 }); - } + // Videos are opt-in. The hero picker feeds imageUrl()/assetPlaceholder and + // wants images only; the sort editor must show everything the public grid + // renders, or a video could never be pinned and would always fall to the tail. + const includeVideos = request.nextUrl.searchParams.get('types') === 'all'; - // Videos are opt-in. The hero picker feeds imageUrl()/assetPlaceholder and - // wants images only; the sort editor must show everything the public grid - // renders, or a video could never be pinned and would always fall to the tail. - const includeVideos = request.nextUrl.searchParams.get('types') === 'all'; + try { + // Via the client, not a bare fetch of `GET /albums/:id`: Immich 3.x no + // longer embeds assets in that response, so reading `album.assets` returned + // nothing (and threw outright when the key was absent). getAlbumAssetsRaw() + // pages through the metadata search and applies the album's own order, so + // this list matches what the site renders under `sort: immich`. + const all = await immich.getAlbumAssetsRaw(albumId); - try { - // Via the client, not a bare fetch of `GET /albums/:id`: Immich 3.x no - // longer embeds assets in that response, so reading `album.assets` returned - // nothing (and threw outright when the key was absent). getAlbumAssetsRaw() - // pages through the metadata search and applies the album's own order, so - // this list matches what the site renders under `sort: immich`. - const all = await immich.getAlbumAssetsRaw(albumId); + const assets = all + .filter((a) => a.type === 'IMAGE' || (includeVideos && a.type === 'VIDEO')) + .map((a) => ({ + id: a.id, + type: a.type, + originalFileName: a.originalFileName, + fileCreatedAt: a.fileCreatedAt, + isFavorite: a.isFavorite ?? false, + })); - const assets = all - .filter((a) => a.type === 'IMAGE' || (includeVideos && a.type === 'VIDEO')) - .map((a) => ({ - id: a.id, - type: a.type, - originalFileName: a.originalFileName, - fileCreatedAt: a.fileCreatedAt, - isFavorite: a.isFavorite ?? false, - })); - - return NextResponse.json({ assets, nextPage: null }); - } catch (err) { - console.error('Failed to fetch album assets:', err); - return NextResponse.json({ error: 'Failed to fetch album assets' }, { status: 500 }); - } -} + return NextResponse.json({ assets, nextPage: null }); + } catch (err) { + console.error('Failed to fetch album assets:', err); + return NextResponse.json({ error: 'Failed to fetch album assets' }, { status: 500 }); + } + }, +); diff --git a/app/api/admin/albums/route.ts b/app/api/admin/albums/route.ts index 3466ee0..e493e91 100644 --- a/app/api/admin/albums/route.ts +++ b/app/api/admin/albums/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from 'next/server'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { getConfig } from '@/lib/config'; interface ImmichAlbumSummary { @@ -15,14 +15,7 @@ interface ImmichAlbumSummary { } /** GET: List ALL shared Immich albums (not just allowlisted ones). */ -export async function GET() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async () => { const config = getConfig(); // Credentials, not `needsSetup`: this route is how the operator picks the // albums that gallery.yaml is built from, so refusing to run until that file @@ -75,4 +68,4 @@ export async function GET() { console.error('[Admin] Failed to fetch albums from Immich:', err); return NextResponse.json({ error: 'Failed to connect to Immich' }, { status: 502 }); } -} +}); diff --git a/app/api/admin/alt-text/route.ts b/app/api/admin/alt-text/route.ts index 5509385..21e2073 100644 --- a/app/api/admin/alt-text/route.ts +++ b/app/api/admin/alt-text/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from 'next/server'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { getConfig } from '@/lib/config'; import { immich } from '@/lib/immich'; import { @@ -34,14 +34,7 @@ async function immichExternalDomain(apiUrl: string, apiKey: string, timeoutMs: n * public site: a check right after browsing the gallery costs no Immich calls. * An album that fails to load is counted, not fatal — the rest still report. */ -export async function GET() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async () => { const config = getConfig(); if (config.needsCredentials) { return NextResponse.json({ error: 'Not configured' }, { status: 503 }); @@ -80,4 +73,4 @@ export async function GET() { }, { headers: { 'Cache-Control': 'no-store' } }, ); -} +}); diff --git a/app/api/admin/analytics/route.ts b/app/api/admin/analytics/route.ts index ff9a1d0..3d2fd7c 100644 --- a/app/api/admin/analytics/route.ts +++ b/app/api/admin/analytics/route.ts @@ -1,19 +1,12 @@ import { NextResponse } from 'next/server'; +import { withAdmin } from '@/lib/admin/withAdmin'; import fs from 'fs/promises'; import path from 'path'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; import { getConfigOrNull } from '@/lib/config'; const ANALYTICS_FILE = path.join(process.cwd(), 'content', 'analytics.json'); -export async function GET() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async () => { try { const config = getConfigOrNull(); let data; @@ -58,4 +51,4 @@ export async function GET() { { status: 500 }, ); } -} +}); diff --git a/app/api/admin/assets/route.ts b/app/api/admin/assets/route.ts index 9bba1e1..3c33f2e 100644 --- a/app/api/admin/assets/route.ts +++ b/app/api/admin/assets/route.ts @@ -4,7 +4,7 @@ */ import { NextRequest, NextResponse } from 'next/server'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { getConfig } from '@/lib/config'; interface ImmichSearchResult { @@ -22,14 +22,7 @@ interface ImmichSearchResult { } /** GET: Search/browse assets from Immich for hero image selection. */ -export async function GET(request: NextRequest) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async (request: NextRequest) => { const config = getConfig(); // Credentials, not `needsSetup`: this route is how the operator picks the // albums that gallery.yaml is built from, so refusing to run until that file @@ -87,4 +80,4 @@ export async function GET(request: NextRequest) { console.error('[Admin] Failed to fetch assets from Immich:', err); return NextResponse.json({ error: 'Failed to connect to Immich' }, { status: 502 }); } -} +}); diff --git a/app/api/admin/backups/route.ts b/app/api/admin/backups/route.ts index a6b8e1f..cd020f7 100644 --- a/app/api/admin/backups/route.ts +++ b/app/api/admin/backups/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from 'next/server'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { revalidatePath } from 'next/cache'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; import { listBackups, restoreBackup } from '@/lib/admin/yaml-service'; import { listJournalBackups, restoreJournalBackup } from '@/lib/admin/journal-service'; import { invalidateConfigCache } from '@/lib/config'; @@ -44,14 +44,7 @@ function parseBackupInfo(filename: string, target: BackupTarget): BackupItem { } /** GET: List all available backups, grouped by the file they restore. */ -export async function GET() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async () => { try { const gallery = (await listBackups('gallery.yaml')).map((f) => parseBackupInfo(f, 'gallery')); const settings = (await listBackups('settings.yaml')).map((f) => @@ -79,17 +72,10 @@ export async function GET() { console.error('[Admin API] Error listing backups:', err); return NextResponse.json({ error: 'Failed to list backups' }, { status: 500 }); } -} +}); /** POST: Restore a specific backup file. */ -export async function POST(req: Request) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const POST = withAdmin(async (req: Request) => { try { const body = await req.json(); const { backupFilename, target } = body || {}; @@ -132,4 +118,4 @@ export async function POST(req: Request) { console.error('[Admin API] Error restoring backup:', err); return NextResponse.json({ error: 'Failed to restore backup' }, { status: 500 }); } -} +}); diff --git a/app/api/admin/doctor/route.ts b/app/api/admin/doctor/route.ts index dff0196..ed15386 100644 --- a/app/api/admin/doctor/route.ts +++ b/app/api/admin/doctor/route.ts @@ -1,12 +1,13 @@ import { NextRequest, NextResponse } from 'next/server'; +import { withAdmin } from '@/lib/admin/withAdmin'; import fs from 'node:fs/promises'; import path from 'node:path'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; -import { getConfig } from '@/lib/config'; +import { getConfig, slugify } from '@/lib/config'; import { env } from '@/lib/env'; import { listJournalEntries } from '@/lib/admin/journal-service'; import { checkAlbumIds, + checkAlbumSlugCollisions, checkAlbumsShared, checkAuthSecret, checkImmichCalls, @@ -17,6 +18,7 @@ import { PROXY_MARKER_HEADERS, worstLevel, type AlbumRef, + type AlbumSlugGroup, type DoctorFinding, type PasswordRef, } from '@/lib/admin/doctor'; @@ -28,14 +30,7 @@ import { * rather than failing the whole report, because a broken install is exactly * when this route needs to answer. */ -export async function GET(request: NextRequest) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async (request: NextRequest) => { const config = getConfig(); const findings: DoctorFinding[] = []; @@ -87,18 +82,35 @@ export async function GET(request: NextRequest) { if (albumRes) { try { - albums = (await albumRes.json()) as AlbumRef[]; + const parsed: unknown = await albumRes.json(); + if (Array.isArray(parsed)) albums = parsed as AlbumRef[]; } catch { // A malformed body is already reflected by the call above. } } findings.push(checkImmichCalls(calls)); - } - if (albums.length) { - findings.push(checkAlbumIds(config.albums, albums)); - findings.push(checkAlbumsShared(config.albums, albums)); + // Whenever albums are configured, run the check even if Immich answered + // with an empty list — an API key regenerated under a different account, + // say. `if (albums.length)` used to skip this entirely, so a `200 []` + // response passed every connection check and reported nothing about + // albums at all, while every album page was silently empty (#629). + // checkAlbumIds treats every configured ID as missing when none resolve. + if (config.albums.length) { + findings.push(checkAlbumIds(config.albums, albums)); + findings.push(checkAlbumsShared(config.albums, albums)); + + // deriveGallery already rejects a slug collision between two albums + // that both have a title override; this is the other half, using the + // album's resolved name (override, or otherwise whatever Immich calls + // it), which is only known once Immich has answered (#632). + const slugGroups: AlbumSlugGroup[] = [ + { context: 'gallery.yaml albums', albumIds: config.standaloneAlbums }, + ...config.subpages.map((sp) => ({ context: `subpage "${sp.name}"`, albumIds: sp.albumIds })), + ]; + findings.push(checkAlbumSlugCollisions(slugGroups, config.albumOverrides, albums, slugify)); + } } // ── Passwords: every place one can be configured ───────────────────── @@ -144,4 +156,4 @@ export async function GET(request: NextRequest) { { level: worstLevel(findings), findings }, { headers: { 'Cache-Control': 'no-store' } }, ); -} +}); diff --git a/app/api/admin/favicon/route.ts b/app/api/admin/favicon/route.ts index ad32c82..2f74f95 100644 --- a/app/api/admin/favicon/route.ts +++ b/app/api/admin/favicon/route.ts @@ -1,7 +1,8 @@ import { NextResponse } from 'next/server'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { promises as fs } from 'fs'; import path from 'path'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { atomicWrite } from '@/lib/atomicWrite'; const CONTENT_DIR = path.resolve(process.cwd(), 'content'); const DEST = 'favicon.svg'; @@ -16,24 +17,10 @@ const MAX_SIZE = 512 * 1024; // 512 kB async function writeFavicon(content: Buffer | string): Promise { await fs.mkdir(CONTENT_DIR, { recursive: true }); const dest = path.join(CONTENT_DIR, DEST); - const tmpPath = `${dest}.${process.pid}.${Date.now()}.tmp`; - try { - await fs.writeFile(tmpPath, content); - await fs.rename(tmpPath, dest); - } catch (err) { - await fs.unlink(tmpPath).catch(() => {}); - throw err; - } + await atomicWrite(dest, content); } -export async function PUT(request: Request) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const PUT = withAdmin(async (request: Request) => { let body: FormData; try { body = await request.formData(); @@ -88,17 +75,10 @@ export async function PUT(request: Request) { await writeFavicon(svg); return NextResponse.json({ success: true, message: 'Favicon updated.' }); -} +}); /** Clean up stale favicon files left by a previous upload of a different format. */ -export async function DELETE() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const DELETE = withAdmin(async () => { try { await fs.unlink(path.join(CONTENT_DIR, DEST)); return NextResponse.json({ @@ -108,4 +88,4 @@ export async function DELETE() { } catch { return NextResponse.json({ error: 'No custom favicon to remove' }, { status: 404 }); } -} +}); diff --git a/app/api/admin/gallery/route.ts b/app/api/admin/gallery/route.ts index 7512af2..c6b6eae 100644 --- a/app/api/admin/gallery/route.ts +++ b/app/api/admin/gallery/route.ts @@ -1,33 +1,19 @@ import { NextResponse } from 'next/server'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { revalidatePath } from 'next/cache'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; import { readGalleryYaml, writeGalleryYaml } from '@/lib/admin/yaml-service'; import { invalidateConfigCache, deriveGallery } from '@/lib/config'; import { immich } from '@/lib/immich'; import type { GalleryYaml } from '@/lib/config/schema'; /** GET: Read current gallery.yaml config. */ -export async function GET() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async () => { const gallery = await readGalleryYaml(); return NextResponse.json({ gallery: gallery || { hero: [], albums: [], subpages: [] } }); -} +}); /** PUT: Write gallery.yaml config. */ -export async function PUT(request: Request) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const PUT = withAdmin(async (request: Request) => { const body = await request.json().catch(() => null); if (!body?.gallery) { return NextResponse.json({ error: 'Missing gallery data' }, { status: 400 }); @@ -72,4 +58,4 @@ export async function PUT(request: Request) { console.error('[Admin] Failed to write gallery.yaml:', err); return NextResponse.json({ error: 'Failed to save gallery config' }, { status: 500 }); } -} +}); diff --git a/app/api/admin/journal/[slug]/route.ts b/app/api/admin/journal/[slug]/route.ts index ca90ea0..fc8f11d 100644 --- a/app/api/admin/journal/[slug]/route.ts +++ b/app/api/admin/journal/[slug]/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from 'next/server'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { isValidSlug, sanitizeSlug, parseJournalMarkdown } from '@/lib/journal'; import { readJournalEntry, @@ -11,14 +11,7 @@ interface RouteContext { params: Promise<{ slug: string }>; } -export async function GET(request: Request, context: RouteContext) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async (request: Request, context: RouteContext) => { const { slug } = await context.params; if (!isValidSlug(slug)) { return NextResponse.json({ error: 'Invalid slug' }, { status: 400 }); @@ -34,16 +27,9 @@ export async function GET(request: Request, context: RouteContext) { console.error(`[Admin API] Failed to get journal entry "${slug}":`, err); return NextResponse.json({ error: 'Failed to read journal entry' }, { status: 500 }); } -} - -export async function PUT(request: Request, context: RouteContext) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } +}); +export const PUT = withAdmin(async (request: Request, context: RouteContext) => { const { slug } = await context.params; if (!isValidSlug(slug)) { return NextResponse.json({ error: 'Invalid slug' }, { status: 400 }); @@ -85,16 +71,9 @@ export async function PUT(request: Request, context: RouteContext) { console.error(`[Admin API] Failed to update journal entry "${slug}":`, err); return NextResponse.json({ error: 'Failed to update journal entry' }, { status: 500 }); } -} - -export async function DELETE(request: Request, context: RouteContext) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } +}); +export const DELETE = withAdmin(async (request: Request, context: RouteContext) => { const { slug } = await context.params; if (!isValidSlug(slug)) { return NextResponse.json({ error: 'Invalid slug' }, { status: 400 }); @@ -110,4 +89,4 @@ export async function DELETE(request: Request, context: RouteContext) { console.error(`[Admin API] Failed to delete journal entry "${slug}":`, err); return NextResponse.json({ error: 'Failed to delete journal entry' }, { status: 500 }); } -} +}); diff --git a/app/api/admin/journal/route.ts b/app/api/admin/journal/route.ts index e6ca2a0..0633832 100644 --- a/app/api/admin/journal/route.ts +++ b/app/api/admin/journal/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from 'next/server'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { sanitizeSlug, isValidSlug, type JournalFrontmatter } from '@/lib/journal'; import { listJournalEntries, @@ -7,14 +7,7 @@ import { readJournalEntry, } from '@/lib/admin/journal-service'; -export async function GET() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async () => { try { const entries = await listJournalEntries(); return NextResponse.json({ entries }); @@ -22,16 +15,9 @@ export async function GET() { console.error('[Admin API] Failed to list journal entries:', err); return NextResponse.json({ error: 'Failed to list journal entries' }, { status: 500 }); } -} - -export async function POST(request: Request) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } +}); +export const POST = withAdmin(async (request: Request) => { try { const body = await request.json(); const rawTitle = typeof body.title === 'string' ? body.title.trim() : 'Untitled'; @@ -73,4 +59,4 @@ export async function POST(request: Request) { console.error('[Admin API] Failed to create journal entry:', err); return NextResponse.json({ error: 'Failed to create journal entry' }, { status: 500 }); } -} +}); diff --git a/app/api/admin/reload/route.ts b/app/api/admin/reload/route.ts index 157a96c..8ff98e2 100644 --- a/app/api/admin/reload/route.ts +++ b/app/api/admin/reload/route.ts @@ -1,21 +1,14 @@ import { NextResponse } from 'next/server'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { revalidatePath } from 'next/cache'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; import { invalidateConfigCache } from '@/lib/config'; import { immich } from '@/lib/immich'; /** POST: Force reload config and clear caches. */ -export async function POST() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const POST = withAdmin(async () => { invalidateConfigCache(); immich.invalidateAll(); revalidatePath('/', 'layout'); return NextResponse.json({ success: true, message: 'Config and cache reloaded' }); -} +}); diff --git a/app/api/admin/settings/route.ts b/app/api/admin/settings/route.ts index 4fa0b28..6d1f09e 100644 --- a/app/api/admin/settings/route.ts +++ b/app/api/admin/settings/route.ts @@ -1,20 +1,14 @@ import { NextResponse } from 'next/server'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { revalidatePath } from 'next/cache'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; import { readSettingsYaml, writeSettingsYaml } from '@/lib/admin/yaml-service'; import { invalidateConfigCache, getConfigOrNull } from '@/lib/config'; import { immich } from '@/lib/immich'; +import { validateSettings } from '@/lib/config/settingsSchema'; import type { SettingsYaml } from '@/lib/config/schema'; /** GET: Read current settings.yaml config. */ -export async function GET() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async () => { const settings = await readSettingsYaml(); const config = getConfigOrNull(); return NextResponse.json({ @@ -26,22 +20,28 @@ export async function GET() { source: config?.siteUrlSource ?? 'none', }, }); -} +}); /** PUT: Write settings.yaml config. */ -export async function PUT(request: Request) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const PUT = withAdmin(async (request: Request) => { const body = await request.json().catch(() => null); if (!body?.settings) { return NextResponse.json({ error: 'Missing settings data' }, { status: 400 }); } + // Checked before the write, not after: settings.yaml is read by every public + // page, so a malformed save is discovered by visitors rather than here. + const validation = validateSettings(body.settings); + if (!validation.ok) { + return NextResponse.json( + { + error: 'These settings could not be saved.', + fields: validation.errors, + }, + { status: 400 }, + ); + } + const settings = body.settings as SettingsYaml; try { @@ -57,4 +57,4 @@ export async function PUT(request: Request) { console.error('[Admin] Failed to write settings.yaml:', err); return NextResponse.json({ error: 'Failed to save settings' }, { status: 500 }); } -} +}); diff --git a/app/api/admin/status/route.ts b/app/api/admin/status/route.ts index 842f44d..6885ce8 100644 --- a/app/api/admin/status/route.ts +++ b/app/api/admin/status/route.ts @@ -1,19 +1,12 @@ import { NextResponse } from 'next/server'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { immich } from '@/lib/immich'; import { getConfig } from '@/lib/config'; import { cache } from '@/lib/cache'; import { listBackups, readGalleryYaml, readSettingsYaml } from '@/lib/admin/yaml-service'; import { getUpdateStatus } from '@/lib/updateCheck'; -export async function GET() { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - +export const GET = withAdmin(async () => { // 1. Immich Connection Status let immichOk = false; try { @@ -96,4 +89,4 @@ export async function GET() { }, update, }); -} +}); diff --git a/app/api/admin/thumbnail/[id]/route.ts b/app/api/admin/thumbnail/[id]/route.ts index 7de789c..6d05c19 100644 --- a/app/api/admin/thumbnail/[id]/route.ts +++ b/app/api/admin/thumbnail/[id]/route.ts @@ -4,55 +4,53 @@ */ import { NextRequest, NextResponse } from 'next/server'; -import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; +import { withAdmin } from '@/lib/admin/withAdmin'; import { getConfig } from '@/lib/config'; import { isUuid } from '@/lib/uuid'; -export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { - if (!isAdminEnabled()) { - return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); - } - if (!(await isAdminAuthenticated())) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } - - const config = getConfig(); - // Credentials, not `needsSetup`: this route is how the operator picks the - // albums that gallery.yaml is built from, so refusing to run until that file - // exists is a deadlock (#507). - if (config.needsCredentials) { - return NextResponse.json({ error: 'Not configured' }, { status: 503 }); - } - - const { id: assetId } = await params; - - if (!isUuid(assetId)) { - return NextResponse.json({ error: 'Invalid asset ID' }, { status: 400 }); - } - - try { - const res = await fetch(`${config.immich.apiUrl}/assets/${assetId}/thumbnail?size=thumbnail`, { - headers: { - 'x-api-key': config.immich.apiKey, - Accept: 'application/octet-stream', - }, - }); - - if (!res.ok) { - return NextResponse.json({ error: 'Asset not found' }, { status: 404 }); +export const GET = withAdmin( + async (request: NextRequest, { params }: { params: Promise<{ id: string }> }) => { + const config = getConfig(); + // Credentials, not `needsSetup`: this route is how the operator picks the + // albums that gallery.yaml is built from, so refusing to run until that file + // exists is a deadlock (#507). + if (config.needsCredentials) { + return NextResponse.json({ error: 'Not configured' }, { status: 503 }); } - const contentType = res.headers.get('content-type') || 'image/jpeg'; - const body = res.body; + const { id: assetId } = await params; - return new NextResponse(body, { - headers: { - 'Content-Type': contentType, - 'Cache-Control': 'private, max-age=3600', - }, - }); - } catch (err) { - console.error('[Admin Thumbnail] Failed to fetch:', err); - return NextResponse.json({ error: 'Failed to fetch thumbnail' }, { status: 502 }); - } -} + if (!isUuid(assetId)) { + return NextResponse.json({ error: 'Invalid asset ID' }, { status: 400 }); + } + + try { + const res = await fetch( + `${config.immich.apiUrl}/assets/${assetId}/thumbnail?size=thumbnail`, + { + headers: { + 'x-api-key': config.immich.apiKey, + Accept: 'application/octet-stream', + }, + }, + ); + + if (!res.ok) { + return NextResponse.json({ error: 'Asset not found' }, { status: 404 }); + } + + const contentType = res.headers.get('content-type') || 'image/jpeg'; + const body = res.body; + + return new NextResponse(body, { + headers: { + 'Content-Type': contentType, + 'Cache-Control': 'private, max-age=3600', + }, + }); + } catch (err) { + console.error('[Admin Thumbnail] Failed to fetch:', err); + return NextResponse.json({ error: 'Failed to fetch thumbnail' }, { status: 502 }); + } + }, +); diff --git a/app/api/analytics/__tests__/track-route.test.ts b/app/api/analytics/__tests__/track-route.test.ts new file mode 100644 index 0000000..ce14e5c --- /dev/null +++ b/app/api/analytics/__tests__/track-route.test.ts @@ -0,0 +1,155 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +/** + * POST /api/analytics/track had none of the safeguards every comparable route + * has: no rate limit, an unbounded key space (`pagePath` straight from the + * request body becomes a permanent object key), and a plain `fs.writeFile` + * that a process killed mid-write truncates — which the next read then + * silently resets to zero (GHSA-w293-x8pc-j4cv). + */ + +vi.mock('@/lib/config', () => ({ + getConfigOrNull: vi.fn(() => ({ analytics: true })), +})); + +const checkRateLimit = vi.fn( + (_ip: string, _maxRpm: number): { success: boolean; remaining: number; resetAt: number } => ({ + success: true, + remaining: 59, + resetAt: 0, + }), +); +vi.mock('@/lib/rate-limit', () => ({ + checkRateLimit: (ip: string, maxRpm: number) => checkRateLimit(ip, maxRpm), + getClientIp: vi.fn(() => '127.0.0.1'), + retryAfterSeconds: vi.fn(() => 60), +})); + +const atomicWrite = vi.fn(async (_filePath: string, _content: string): Promise => undefined); +vi.mock('@/lib/atomicWrite', () => ({ + atomicWrite: (filePath: string, content: string) => atomicWrite(filePath, content), +})); + +let fixtureAnalytics: unknown = null; +vi.mock('fs/promises', () => ({ + default: { + readFile: vi.fn(async () => { + if (fixtureAnalytics === null) throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + return JSON.stringify(fixtureAnalytics); + }), + mkdir: vi.fn(async () => undefined), + }, +})); + +import { POST } from '../track/route'; +import { getConfigOrNull } from '@/lib/config'; + +const mockConfig = getConfigOrNull as unknown as ReturnType; + +const post = (body: unknown) => + POST( + new Request('http://localhost/api/analytics/track', { + method: 'POST', + body: JSON.stringify(body), + headers: { 'user-agent': 'Mozilla/5.0 (X11; Linux x86_64)' }, + }) as never, + ); + +const savedData = () => { + const call = atomicWrite.mock.calls.at(-1); + return JSON.parse(String(call?.[1])); +}; + +beforeEach(() => { + vi.clearAllMocks(); + checkRateLimit.mockReturnValue({ success: true, remaining: 59, resetAt: 0 }); + mockConfig.mockReturnValue({ analytics: true }); + fixtureAnalytics = null; +}); + +describe('POST /api/analytics/track', () => { + it('records a pageview and writes through atomicWrite, not a raw write', async () => { + const res = await post({ path: '/deutschland/kloster-chorin' }); + expect(res.status).toBe(200); + + expect(atomicWrite).toHaveBeenCalledOnce(); + const data = savedData(); + expect(data.summary.totalViews).toBe(1); + const [dateKey] = Object.keys(data.days); + expect(data.days[dateKey].pages['/deutschland/kloster-chorin']).toBe(1); + }); + + it('is rate-limited per IP', async () => { + checkRateLimit.mockReturnValue({ success: false, remaining: 0, resetAt: Date.now() + 60_000 }); + + const res = await post({ path: '/' }); + + expect(res.status).toBe(429); + expect(res.headers.get('Retry-After')).toBeTruthy(); + expect(atomicWrite).not.toHaveBeenCalled(); + }); + + it('truncates a path far longer than any real route', async () => { + const huge = '/' + 'a'.repeat(1000); + await post({ path: huge }); + + const data = savedData(); + const [dateKey] = Object.keys(data.days); + const keys = Object.keys(data.days[dateKey].pages); + expect(keys).toHaveLength(1); + expect(keys[0].length).toBeLessThanOrEqual(200); + }); + + it('drops the query string from the path', async () => { + await post({ path: '/journal?utm_source=newsletter' }); + + const data = savedData(); + const [dateKey] = Object.keys(data.days); + expect(Object.keys(data.days[dateKey].pages)).toEqual(['/journal']); + }); + + it('caps distinct page keys per day without dropping the pageview itself', async () => { + const dateKey = new Date().toISOString().split('T')[0]; + const pages: Record = {}; + for (let i = 0; i < 500; i++) pages[`/existing-${i}`] = 1; + fixtureAnalytics = { + summary: { totalViews: 500, lastUpdated: '' }, + days: { [dateKey]: { pageviews: 500, pages, devices: {} } }, + }; + + const res = await post({ path: '/one-path-too-many' }); + expect(res.status).toBe(200); + + const data = savedData(); + // The day's total still counts this visit... + expect(data.days[dateKey].pageviews).toBe(501); + // ...but the 501st distinct path did not get its own key. + expect(Object.keys(data.days[dateKey].pages)).toHaveLength(500); + expect(data.days[dateKey].pages['/one-path-too-many']).toBeUndefined(); + }); + + it('still increments an already-tracked path past the cap', async () => { + const dateKey = new Date().toISOString().split('T')[0]; + const pages: Record = { '/journal': 10 }; + for (let i = 0; i < 500; i++) pages[`/existing-${i}`] = 1; + fixtureAnalytics = { + summary: { totalViews: 510, lastUpdated: '' }, + days: { [dateKey]: { pageviews: 510, pages, devices: {} } }, + }; + + await post({ path: '/journal' }); + + const data = savedData(); + expect(data.days[dateKey].pages['/journal']).toBe(11); + }); + + it('does nothing when analytics are disabled', async () => { + mockConfig.mockReturnValue({ analytics: false }); + + const res = await post({ path: '/' }); + const body = await res.json(); + + expect(body.trackingDisabled).toBe(true); + expect(atomicWrite).not.toHaveBeenCalled(); + }); +}); diff --git a/app/api/analytics/track/route.ts b/app/api/analytics/track/route.ts index 02c4690..960883e 100644 --- a/app/api/analytics/track/route.ts +++ b/app/api/analytics/track/route.ts @@ -1,11 +1,28 @@ -import { NextResponse } from 'next/server'; +import { NextRequest, NextResponse } from 'next/server'; import fs from 'fs/promises'; import path from 'path'; import { getConfigOrNull } from '@/lib/config'; +import { atomicWrite } from '@/lib/atomicWrite'; +import { checkRateLimit, getClientIp, retryAfterSeconds } from '@/lib/rate-limit'; const CONTENT_DIR = path.join(process.cwd(), 'content'); const ANALYTICS_FILE = path.join(CONTENT_DIR, 'analytics.json'); +/** Requests per minute per IP. One real pageview fires this once. */ +const ANALYTICS_RPM = 60; + +/** + * Distinct page paths tracked per day. Every route this comparably-sized site + * has, many times over — high enough that no real page is ever dropped, low + * enough that a flood of made-up paths cannot grow the file without bound. + * A path over the cap still counts toward the day's total pageviews; it just + * does not get its own key. + */ +const MAX_PAGES_PER_DAY = 500; + +/** Well past any real route on this app; anything longer is not a path someone navigated to. */ +const MAX_PATH_LENGTH = 200; + interface AnalyticsData { summary: { totalViews: number; @@ -33,41 +50,76 @@ async function getAnalytics(): Promise { async function saveAnalytics(data: AnalyticsData) { try { await fs.mkdir(CONTENT_DIR, { recursive: true }); - await fs.writeFile(ANALYTICS_FILE, JSON.stringify(data, null, 2), 'utf8'); + // Temp-file + rename, not a plain write: a process killed mid-write used + // to truncate the file, and a truncated (unparseable) file made the next + // getAnalytics() silently reset every count to zero. + await atomicWrite(ANALYTICS_FILE, JSON.stringify(data, null, 2)); } catch (err) { console.error('[Analytics] Failed to save analytics data:', err); } } -export async function POST(req: Request) { +/** + * Serializes the read-modify-write below within this process. Two concurrent + * requests each reading the file, incrementing their own in-memory copy and + * writing it back would silently drop whichever one wrote last — the file's + * own atomicity (saveAnalytics above) does not cover that race, only the + * write itself. + */ +let writeQueue: Promise = Promise.resolve(); + +export async function POST(req: NextRequest) { try { const config = getConfigOrNull(); if (config?.analytics === false) { return NextResponse.json({ ok: true, trackingDisabled: true }); } - const body = await req.json(); - const pagePath = (body.path || '/').split('?')[0]; + const ip = getClientIp(req); + const { success, resetAt } = checkRateLimit(`analytics:${ip}`, ANALYTICS_RPM); + if (!success) { + return NextResponse.json( + { error: 'Too many requests' }, + { + status: 429, + headers: { 'Retry-After': String(retryAfterSeconds(resetAt)), 'Cache-Control': 'no-store' }, + }, + ); + } + + const body = await req.json().catch(() => null); + const rawPath = typeof body?.path === 'string' ? body.path : '/'; + const pagePath = (rawPath.split('?')[0].slice(0, MAX_PATH_LENGTH) || '/') as string; const userAgent = req.headers.get('user-agent') || ''; const isMobile = /mobile|iphone|ipad|android/i.test(userAgent); const deviceType = isMobile ? 'mobile' : 'desktop'; - const dateKey = new Date().toISOString().split('T')[0]; - const data = await getAnalytics(); - data.summary.totalViews = (data.summary.totalViews || 0) + 1; - data.summary.lastUpdated = new Date().toISOString(); + const run = writeQueue.then(async () => { + const data = await getAnalytics(); - if (!data.days[dateKey]) { - data.days[dateKey] = { pageviews: 0, pages: {}, devices: {} }; - } + data.summary.totalViews = (data.summary.totalViews || 0) + 1; + data.summary.lastUpdated = new Date().toISOString(); - const dayObj = data.days[dateKey]; - dayObj.pageviews = (dayObj.pageviews || 0) + 1; - dayObj.pages[pagePath] = (dayObj.pages[pagePath] || 0) + 1; - dayObj.devices[deviceType] = (dayObj.devices[deviceType] || 0) + 1; + if (!data.days[dateKey]) { + data.days[dateKey] = { pageviews: 0, pages: {}, devices: {} }; + } + + const dayObj = data.days[dateKey]; + dayObj.pageviews = (dayObj.pageviews || 0) + 1; + dayObj.devices[deviceType] = (dayObj.devices[deviceType] || 0) + 1; + + // An already-tracked path may still increment past the cap; only a + // brand-new key is refused once the day is full. + if (dayObj.pages[pagePath] !== undefined || Object.keys(dayObj.pages).length < MAX_PAGES_PER_DAY) { + dayObj.pages[pagePath] = (dayObj.pages[pagePath] || 0) + 1; + } + + await saveAnalytics(data); + }); + writeQueue = run.catch(() => {}); + await run; - await saveAnalytics(data); return NextResponse.json({ ok: true }); } catch { return NextResponse.json({ error: 'Failed to record tracking' }, { status: 500 }); diff --git a/app/api/install/route.ts b/app/api/install/route.ts index 05f2fe4..c6eae40 100644 --- a/app/api/install/route.ts +++ b/app/api/install/route.ts @@ -98,7 +98,7 @@ export async function POST(request: NextRequest) { const hero = albums.length ? await fetchAlbumCovers(apiBase, apiKey, albums) : []; try { - await completeInstall({ + const { galleryWritten, settingsWritten } = await completeInstall({ hero, apiUrl, apiKey, @@ -115,7 +115,11 @@ export async function POST(request: NextRequest) { immich.invalidateAll(); revalidatePath('/', 'layout'); - return NextResponse.json({ success: true }); + // completeInstall never overwrites a gallery.yaml or settings.yaml that + // was already there (#627) — worth telling the operator when that's what + // just happened, since it means the wizard they walked through did less + // than it looked like. + return NextResponse.json({ success: true, galleryWritten, settingsWritten }); } catch (err) { console.error('[Install] Failed to write configuration files:', err); return NextResponse.json({ error: 'Failed to write configuration files' }, { status: 500 }); diff --git a/app/globals.css b/app/globals.css index 0526030..948ccb5 100644 --- a/app/globals.css +++ b/app/globals.css @@ -103,6 +103,26 @@ border-bottom-color: var(--text-primary); } +/* The links (Home, subpages, standalone albums, About, Map) live inside + MobileNav's wrapper, but `display: contents` erases the wrapper's own box + above the mobile breakpoint — the links become direct flex items of + `.header__nav`, so desktop keeps the exact layout it always had. */ +.header__nav-links { + display: contents; +} + +/* Hidden above the mobile breakpoint — MobileNav renders it unconditionally, + but only the CSS decides when it is reachable, same as `.header__wordmark`. */ +.header__menu-btn { + display: none; +} + +.header__menu-btn svg { + width: 20px; + height: 20px; + display: block; +} + /* ── Main Content ─────────────────────────────────── */ .main { max-width: var(--max-width); @@ -1918,6 +1938,67 @@ padding: 32px 20px 80px; } + /* A proper menu replaces the horizontal scroll below this width — see + `.header__nav-links` / `.header__menu-btn` above and MobileNav (#590). */ + .header__menu-btn { + display: flex; + align-items: center; + justify-content: center; + margin-left: auto; + padding: 8px; + background: none; + border: none; + color: var(--text-secondary); + cursor: pointer; + } + + .header__nav-links { + display: none; + } + + /* `.header` is the nearest positioned ancestor (it's `position: fixed`), + so `top: 100%` sits the panel right under it regardless of the actual + header height, which differs by preset (plain padding vs. studio-modern's + fixed `--header-height`). `left: 50%` + `width: 100vw` + negative margin + breaks the panel out to the full viewport width regardless of `.header`'s + own horizontal padding, which also differs by preset (`--sm-gutter` for + studio-modern vs. a fixed 24px elsewhere). */ + .header__nav-links[data-open='true'] { + display: flex; + flex-direction: column; + position: absolute; + top: 100%; + left: 50%; + width: 100vw; + margin-left: -50vw; + max-height: calc(100vh - 64px); + overflow-y: auto; + background: rgba(26, 26, 26, 0.98); + border-bottom: 1px solid var(--border-subtle); + z-index: 101; + } + + [data-theme='light'] .header__nav-links[data-open='true'] { + background: rgba(245, 245, 240, 0.98); + } + + .header__nav-links[data-open='true'] .header__nav-link, + .header__nav-links[data-open='true'] .header__nav-link--external { + width: 100%; + padding: 16px 24px; + font-size: 0.75rem; + } + + /* Covers the full viewport, including the area behind the header — the + header's own `z-index: 100` keeps it crisp above the dim, so this does + not need to know the header's height either. */ + .header__nav-backdrop { + position: fixed; + inset: 0; + background: rgba(0, 0, 0, 0.4); + z-index: 99; + } + .header__nav-link { padding: 14px 10px; font-size: 0.6rem; diff --git a/app/install/InstallWizard.tsx b/app/install/InstallWizard.tsx index 84a774a..8b079d4 100644 --- a/app/install/InstallWizard.tsx +++ b/app/install/InstallWizard.tsx @@ -63,6 +63,7 @@ export function InstallWizard({ initialApiUrl, setupToken }: Props) { const [installing, setInstalling] = useState(false); const [installError, setInstallError] = useState(''); const [done, setDone] = useState(false); + const [galleryKept, setGalleryKept] = useState(false); const selectedCount = Object.values(selected).filter(Boolean).length; @@ -131,6 +132,10 @@ export function InstallWizard({ initialApiUrl, setupToken }: Props) { setInstallError(data.error || 'Install failed'); return; } + // The wizard never overwrites a gallery.yaml that was already there + // (#627) — worth surfacing, since the album picks and site settings + // above did not end up in the file that was just kept. + setGalleryKept(data.galleryWritten === false); setDone(true); } catch { setInstallError('Could not reach the install service'); @@ -148,6 +153,12 @@ export function InstallWizard({ initialApiUrl, setupToken }: Props) { Immich Folio

Install complete

Your portfolio is ready. Add or rearrange albums any time from the admin panel.

+ {galleryKept && ( +

+ A gallery.yaml already existed in content/, so it was left untouched — the albums + picked above were not written. Use the admin panel to change it. +

+ )}
{/* install-body carries the card's padding — every other step renders inside it, and skipping it here left the buttons diff --git a/app/install/install.css b/app/install/install.css index cd3ffc4..9ad4791 100644 --- a/app/install/install.css +++ b/app/install/install.css @@ -46,6 +46,17 @@ line-height: 1.5; } +/* Existing gallery.yaml kept as-is rather than replaced (#627) — worth more + than the muted paragraph color the completion message above it uses. */ +.install-card__header p.install-notice { + margin-top: 0.75rem; + padding: 0.5rem 0.75rem; + border: 1px solid var(--admin-border); + border-left: 3px solid var(--admin-accent); + border-radius: 6px; + color: var(--admin-text); +} + /* ── Step indicator ──────────────────────────────────────────── */ .install-steps { diff --git a/app/journal/[slug]/page.tsx b/app/journal/[slug]/page.tsx index b17d1d5..c43abc4 100644 --- a/app/journal/[slug]/page.tsx +++ b/app/journal/[slug]/page.tsx @@ -1,9 +1,11 @@ import { notFound } from 'next/navigation'; import { cookies } from 'next/headers'; import type { Metadata } from 'next'; -import { readJournalEntry } from '@/lib/admin/journal-service'; +import { readJournalEntry, listJournalEntries } from '@/lib/admin/journal-service'; import type { ParsedJournal } from '@/lib/journal'; import { isAdminAuthenticated } from '@/lib/admin/auth'; +import { isAuthenticated } from '@/lib/auth'; +import { journalNeighbours } from '@/lib/journalNav'; import { getConfig } from '@/lib/config'; import { immich } from '@/lib/immich'; import { @@ -16,6 +18,7 @@ import { } from '@/lib/urls'; import { encodeAssetId } from '@/lib/tokens'; import { EssayView } from '@/app/[...path]/EssayView'; +import { JournalNav } from '@/components/JournalNav'; import type { PhotoItem } from '@/app/[...path]/PhotoGrid'; import type { ImmichAsset } from '@/lib/immich'; import PasswordGate from '@/components/PasswordGate'; @@ -36,12 +39,30 @@ export async function generateMetadata({ params }: JournalDetailPageProps): Prom if (!entry) return { title: t.journal.notFound }; const { frontmatter } = entry.parsed; - const title = frontmatter.title || slug; - const description = frontmatter.subtitle || t.journal.entryDescription; - const ogImages = frontmatter.coverAssetId - ? [{ url: imageUrl(frontmatter.coverAssetId, 'preview') }] - : []; + // generateMetadata runs unconditionally, ahead of the page body's own + // draft/password gate below — without this check a draft's or a locked + // entry's real title, subtitle and cover image reached the of a + // 200 response no authentication was ever asked for (GHSA-fvgv-97g3-wjr7). + const isAuthedAdmin = await isAdminAuthenticated(); + const blocked = + (frontmatter.draft && !isAuthedAdmin) || + (!!frontmatter.password && + !isJournalAuthenticated( + slug, + frontmatter.password, + (await cookies()).get(`lb_auth_journal_${slug}`)?.value, + )); + + const title = blocked ? t.journal.title : frontmatter.title || slug; + const description = blocked + ? t.journal.description + : frontmatter.subtitle || t.journal.entryDescription; + + const ogImages = + !blocked && frontmatter.coverAssetId + ? [{ url: imageUrl(frontmatter.coverAssetId, 'preview') }] + : []; return { title, @@ -111,6 +132,26 @@ export default async function JournalDetailPage({ params }: JournalDetailPagePro } } + // Prev/next through the other entries a visitor may actually see (#591) — + // mirrors AlbumNav's rule (#483) of drawing neighbours from the surrounding + // list rather than a separate order of its own. A draft or a + // password-protected entry the visitor has not unlocked must not appear + // here even by name, the same rule the index and this entry's own gate + // already enforce (GHSA-fvgv-97g3-wjr7). + const journalCookies = await cookies(); + const allEntries = await listJournalEntries(); + const visibleEntries = isAuthedAdmin + ? allEntries + : allEntries.filter((e) => { + if (e.frontmatter.draft) return false; + if (!e.frontmatter.password) return true; + return isAuthenticated(e.slug, (name) => journalCookies.get(name)?.value, 'journal'); + }); + const nav = journalNeighbours( + visibleEntries.map((e) => ({ slug: e.slug, title: e.frontmatter.title || e.slug })), + slug, + ); + const config = getConfig(); // Fetch all referenced assets from Immich @@ -151,8 +192,15 @@ export default async function JournalDetailPage({ params }: JournalDetailPagePro const toToken = (assetId: string) => tokenByAssetId.get(assetId) ?? ''; const essayForClient: ParsedJournal = { + // Named fields, not a spread of `frontmatter`: EssayView reads only + // title, subtitle, coverAssetId, author and date, but a spread put the + // stored password — plaintext or a scrypt hash — into every visitor's + // RSC payload the moment they unlocked the entry once (GHSA-fvgv-97g3-wjr7). frontmatter: { - ...frontmatter, + title: frontmatter.title, + subtitle: frontmatter.subtitle, + author: frontmatter.author, + date: frontmatter.date, coverAssetId: frontmatter.coverAssetId ? toToken(frontmatter.coverAssetId) : undefined, }, blocks: blocks.map((block) => { @@ -205,6 +253,7 @@ export default async function JournalDetailPage({ params }: JournalDetailPagePro subtitle={frontmatter.subtitle} watermark={config.watermark} /> + ); } diff --git a/app/journal/page.tsx b/app/journal/page.tsx index 6ba6c87..c4db307 100644 --- a/app/journal/page.tsx +++ b/app/journal/page.tsx @@ -1,9 +1,11 @@ import Link from 'next/link'; import Image from 'next/image'; import type { Metadata } from 'next'; +import { cookies } from 'next/headers'; import { type JournalEntrySummary } from '@/lib/journal'; import { listJournalEntries } from '@/lib/admin/journal-service'; import { isAdminAuthenticated } from '@/lib/admin/auth'; +import { isAuthenticated } from '@/lib/auth'; import { immich } from '@/lib/immich'; import { imageUrl, assetPlaceholder } from '@/lib/urls'; import { BackLink } from '@/components/BackLink'; @@ -29,10 +31,19 @@ export default async function JournalIndexPage() { const isAuthedAdmin = await isAdminAuthenticated(); const allEntries = await listJournalEntries(); - // Non-admins only see published entries + // Non-admins only see published entries, and only a password-protected one + // once they have actually unlocked it — the card otherwise names it, shows + // its cover and quotes its excerpt to every visitor regardless of the + // password on its body (GHSA-fvgv-97g3-wjr7). + const cookieStore = isAuthedAdmin ? null : await cookies(); + const getCookie = (name: string) => cookieStore?.get(name)?.value; const visibleEntries = isAuthedAdmin ? allEntries - : allEntries.filter((e) => !e.frontmatter.draft); + : allEntries.filter((e) => { + if (e.frontmatter.draft) return false; + if (!e.frontmatter.password) return true; + return isAuthenticated(e.slug, getCookie, 'journal'); + }); // Fetch cover image placeholders and URLs const enrichedEntries: EnrichedJournalEntry[] = await Promise.all( diff --git a/app/layout.tsx b/app/layout.tsx index 51717b6..bf4e4b5 100644 --- a/app/layout.tsx +++ b/app/layout.tsx @@ -6,9 +6,10 @@ import type { Metadata } from 'next'; import { cookies, headers } from 'next/headers'; -import Link from 'next/link'; import './globals.css'; +import { NavLink } from '@/components/NavLink'; import { SubpageNav } from '@/components/SubpageNav'; +import { MobileNav } from '@/components/MobileNav'; import { ThemeToggle } from '@/components/ThemeToggle'; import { ScrollToTop } from '@/components/ScrollToTop'; import { Footer } from '@/components/Footer'; @@ -175,20 +176,12 @@ export default async function RootLayout({ children }: { children: React.ReactNo - - {t.nav.home} - - - {config.aboutEnabled && ( - - {t.nav.about} - - )} - {config.map && ( - - {t.nav.map} - - )} + + {t.nav.home} + + {config.aboutEnabled && {t.nav.about}} + {config.map && {t.nav.map}} + diff --git a/components/JournalNav.tsx b/components/JournalNav.tsx new file mode 100644 index 0000000..951e949 --- /dev/null +++ b/components/JournalNav.tsx @@ -0,0 +1,66 @@ +/** + * JournalNav — the "← previous entry / next entry →" pair at the foot of a + * journal entry, mirroring AlbumNav (#483) for the journal (#591). + * + * Renders nothing when there is no neighbour in either direction, so a lone + * entry keeps its quiet ending. + */ + +import Link from 'next/link'; +import { getServerDictionary } from '@/lib/i18n/server'; +import type { JournalNavPair } from '@/lib/journalNav'; + +function Chevron({ direction }: { direction: 'prev' | 'next' }) { + return ( + + ); +} + +export function JournalNav({ prev, next }: JournalNavPair) { + const t = getServerDictionary(); + if (!prev && !next) return null; + + return ( + + ); +} diff --git a/components/Lightbox.tsx b/components/Lightbox.tsx index 9acb051..c6eedaf 100644 --- a/components/Lightbox.tsx +++ b/components/Lightbox.tsx @@ -131,19 +131,20 @@ export function Lightbox({ /** * The deep link to the photo on screen. * - * Built from the current index rather than read back from `location.hash`, - * so it does not depend on the grid's hash-sync effect having run yet. The - * `#photo-N` form is the permalink the grid already writes and restores; - * this only supplies the affordance to copy it (#478). + * Built from `current.id` — the asset's own stable token, the same one the + * grid already carries on every `PhotoItem` — rather than read back from + * `location.search`, so it does not depend on the grid's query-sync effect + * having run yet. This only supplies the affordance to copy it (#478); the + * `?photo=` form itself is what the grid writes and restores. * - * The link is positional, as `gallery.yaml.example` documents: reordering an - * album moves where a shared link lands. That is a property of the existing - * permalink, not of the button. + * Addressed by the photo, not a position: reordering the album, or + * deleting a different photo, no longer repoints an already-shared link + * (#588). */ const permalink = useCallback(() => { - if (typeof window === 'undefined') return ''; - return buildPhotoPermalink(window.location, currentIndex); - }, [currentIndex]); + if (typeof window === 'undefined' || !current) return ''; + return buildPhotoPermalink(window.location, current.id); + }, [current]); const handleCopyLink = useCallback(() => { const url = permalink(); diff --git a/components/MobileNav.tsx b/components/MobileNav.tsx new file mode 100644 index 0000000..e01228b --- /dev/null +++ b/components/MobileNav.tsx @@ -0,0 +1,98 @@ +'use client'; + +import { useCallback, useEffect, useState } from 'react'; +import { usePathname } from 'next/navigation'; +import { useDictionary } from './I18nProvider'; + +/** + * MobileNav — hamburger button + dropdown panel for the header links. + * + * `children` — Home, subpages, standalone albums, About, Map — is the exact + * same server-rendered markup the desktop header already used. It is never + * duplicated: `.header__nav-links` is `display: contents` above the mobile + * breakpoint, so on desktop this component is invisible and the links sit + * directly in `.header__nav`'s flex row exactly as before. Below the + * breakpoint the same markup becomes the dropdown panel's content, toggled + * by the `data-open` attribute rather than by mounting/unmounting (#590). + * + * The header used to scroll sideways with the scrollbar hidden on narrow + * viewports, so a visitor with more subpages than fit the width had no sign + * that any were missing. + */ +export function MobileNav({ children }: { children: React.ReactNode }) { + const [open, setOpen] = useState(false); + const pathname = usePathname(); + const t = useDictionary(); + + const close = useCallback(() => setOpen(false), []); + + // The root layout persists across navigations, so without this the panel + // would still be open on the page a visitor just tapped through to. Adjusted + // during render rather than in an effect, per React's guidance for state + // that only needs to change alongside a prop — it resolves in this render + // pass instead of triggering an extra one. + const [prevPathname, setPrevPathname] = useState(pathname); + if (pathname !== prevPathname) { + setPrevPathname(pathname); + setOpen(false); + } + + useEffect(() => { + if (!open) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape') close(); + }; + document.addEventListener('keydown', onKey); + document.body.style.overflow = 'hidden'; + return () => { + document.removeEventListener('keydown', onKey); + document.body.style.overflow = ''; + }; + }, [open, close]); + + return ( + <> + + {open &&