🛡️ Sentinel: [HIGH] Fix timingSafeEqual DoS Vulnerability (#423)

* Fix timingSafeEqual DoS Vulnerability in Webhook

Co-authored-by: ralksta <16941050+ralksta@users.noreply.github.com>

* Fix memory exhaustion DoS in token decoding

Co-authored-by: ralksta <16941050+ralksta@users.noreply.github.com>

---------

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>

lib/__tests__/albumSort.test.ts

@@ -0,0 +1,211 @@
+import { describe, it, expect } from 'vitest';
+
+import {
+  ALBUM_SORT_MODES,
+  isAlbumSortMode,
+  sortAlbumAssets,
+  type AlbumSortMode,
+  type SortableAsset,
+} from '@/lib/albumSort';
+
+function asset(id: string, overrides: Partial<SortableAsset> = {}): SortableAsset {
+  return {
+    id,
+    originalFileName: `${id}.jpg`,
+    fileCreatedAt: '2024-01-01T00:00:00.000Z',
+    localDateTime: '2024-01-01T00:00:00.000Z',
+    ...overrides,
+  };
+}
+
+const ids = (assets: SortableAsset[]) => assets.map((a) => a.id);
+
+describe('sortAlbumAssets is pure', () => {
+  it('returns a new array and leaves the input untouched', () => {
+    const input = [
+      asset('b', { localDateTime: '2024-02-01T00:00:00.000Z' }),
+      asset('a', { localDateTime: '2024-01-01T00:00:00.000Z' }),
+    ];
+    const before = ids(input);
+
+    const out = sortAlbumAssets(input, { mode: 'oldest' });
+
+    expect(out).not.toBe(input);
+    expect(ids(input)).toEqual(before);
+  });
+
+  // The cached album is shared, so any mode that dropped or duplicated an asset
+  // would corrupt what every later request sees. This is the guard that any
+  // future rewrite of the manual branch has to survive.
+  it.each(ALBUM_SORT_MODES)('preserves the exact set of assets under %s', (mode) => {
+    const input = [asset('a'), asset('b'), asset('c'), asset('d')];
+
+    const out = sortAlbumAssets(input, {
+      mode: mode as AlbumSortMode,
+      immichOrder: 'asc',
+      manualOrder: ['c', 'zzz'],
+    });
+
+    expect(out).toHaveLength(input.length);
+    expect(ids(out).sort()).toEqual(['a', 'b', 'c', 'd']);
+  });
+});
+
+describe('capture-time modes', () => {
+  const jan = asset('jan', { localDateTime: '2024-01-01T00:00:00.000Z' });
+  const feb = asset('feb', { localDateTime: '2024-02-01T00:00:00.000Z' });
+  const mar = asset('mar', { localDateTime: '2024-03-01T00:00:00.000Z' });
+
+  it('orders newest first', () => {
+    expect(ids(sortAlbumAssets([jan, mar, feb], { mode: 'newest' }))).toEqual([
+      'mar',
+      'feb',
+      'jan',
+    ]);
+  });
+
+  it('orders oldest first, exactly reversing newest', () => {
+    const input = [jan, mar, feb];
+    expect(ids(sortAlbumAssets(input, { mode: 'oldest' }))).toEqual(
+      ids(sortAlbumAssets(input, { mode: 'newest' })).reverse(),
+    );
+  });
+
+  it('falls back to fileCreatedAt when localDateTime ties', () => {
+    const early = asset('early', {
+      localDateTime: '2024-01-01T00:00:00.000Z',
+      fileCreatedAt: '2024-01-01T08:00:00.000Z',
+    });
+    const late = asset('late', {
+      localDateTime: '2024-01-01T00:00:00.000Z',
+      fileCreatedAt: '2024-01-01T20:00:00.000Z',
+    });
+
+    expect(ids(sortAlbumAssets([late, early], { mode: 'oldest' }))).toEqual(['early', 'late']);
+  });
+
+  // Date.parse() on garbage yields NaN, and a comparator returning NaN makes
+  // sort() incoherent — the result depends on the engine's algorithm, not the data.
+  it('treats an unparseable date as epoch rather than producing NaN', () => {
+    const broken = asset('broken', { localDateTime: 'not-a-date', fileCreatedAt: 'also-not' });
+
+    const out = sortAlbumAssets([feb, broken, jan], { mode: 'oldest' });
+
+    expect(ids(out)).toEqual(['broken', 'jan', 'feb']);
+  });
+
+  it('immich follows the album direction', () => {
+    const input = [feb, jan, mar];
+    expect(ids(sortAlbumAssets(input, { mode: 'immich', immichOrder: 'asc' }))).toEqual([
+      'jan',
+      'feb',
+      'mar',
+    ]);
+    expect(ids(sortAlbumAssets(input, { mode: 'immich', immichOrder: 'desc' }))).toEqual([
+      'mar',
+      'feb',
+      'jan',
+    ]);
+  });
+});
+
+describe('filename mode', () => {
+  it('collates numerically, so IMG_2 precedes IMG_10', () => {
+    const input = [
+      asset('c', { originalFileName: 'IMG_10.jpg' }),
+      asset('a', { originalFileName: 'IMG_2.jpg' }),
+      asset('b', { originalFileName: 'IMG_9.jpg' }),
+    ];
+
+    expect(ids(sortAlbumAssets(input, { mode: 'filename' }))).toEqual(['a', 'b', 'c']);
+  });
+
+  it('is case-insensitive', () => {
+    const input = [
+      asset('b', { originalFileName: 'beta.jpg' }),
+      asset('a', { originalFileName: 'Alpha.jpg' }),
+    ];
+
+    expect(ids(sortAlbumAssets(input, { mode: 'filename' }))).toEqual(['a', 'b']);
+  });
+
+  // Duplicate filenames across import batches are common; without the id
+  // tiebreak the order of a tie would follow the input and drift per request.
+  it('breaks ties on id so the order is stable', () => {
+    const one = asset('aaa', { originalFileName: 'DSC.jpg' });
+    const two = asset('bbb', { originalFileName: 'dsc.jpg' });
+
+    expect(ids(sortAlbumAssets([two, one], { mode: 'filename' }))).toEqual(['aaa', 'bbb']);
+    expect(ids(sortAlbumAssets([one, two], { mode: 'filename' }))).toEqual(['aaa', 'bbb']);
+  });
+});
+
+describe('manual mode is a pinned prefix', () => {
+  const a = asset('a', { localDateTime: '2024-01-01T00:00:00.000Z' });
+  const b = asset('b', { localDateTime: '2024-02-01T00:00:00.000Z' });
+  const c = asset('c', { localDateTime: '2024-03-01T00:00:00.000Z' });
+
+  it('puts pinned assets first, in the pinned order', () => {
+    const out = sortAlbumAssets([a, b, c], {
+      mode: 'manual',
+      immichOrder: 'asc',
+      manualOrder: ['c', 'a'],
+    });
+
+    expect(ids(out)).toEqual(['c', 'a', 'b']);
+  });
+
+  it('appends unpinned assets in the album Immich order', () => {
+    const desc = sortAlbumAssets([a, b, c], {
+      mode: 'manual',
+      immichOrder: 'desc',
+      manualOrder: ['a'],
+    });
+    const asc = sortAlbumAssets([a, b, c], {
+      mode: 'manual',
+      immichOrder: 'asc',
+      manualOrder: ['a'],
+    });
+
+    expect(ids(desc)).toEqual(['a', 'c', 'b']);
+    expect(ids(asc)).toEqual(['a', 'b', 'c']);
+  });
+
+  // The album can change in Immich after the order was saved; a stale id must
+  // not shift or drop anything.
+  it('ignores pinned ids that are no longer in the album', () => {
+    const out = sortAlbumAssets([a, b], {
+      mode: 'manual',
+      immichOrder: 'asc',
+      manualOrder: ['gone', 'b'],
+    });
+
+    expect(ids(out)).toEqual(['b', 'a']);
+  });
+
+  it('pins a duplicated id once, at its first position', () => {
+    const out = sortAlbumAssets([a, b, c], {
+      mode: 'manual',
+      immichOrder: 'asc',
+      manualOrder: ['c', 'a', 'c'],
+    });
+
+    expect(ids(out)).toEqual(['c', 'a', 'b']);
+  });
+
+  it('falls back to the album order when nothing is pinned', () => {
+    const input = [c, a, b];
+    expect(ids(sortAlbumAssets(input, { mode: 'manual', immichOrder: 'asc' }))).toEqual(
+      ids(sortAlbumAssets(input, { mode: 'immich', immichOrder: 'asc' })),
+    );
+  });
+});
+
+describe('isAlbumSortMode', () => {
+  it('accepts every declared mode and nothing else', () => {
+    for (const mode of ALBUM_SORT_MODES) expect(isAlbumSortMode(mode)).toBe(true);
+    expect(isAlbumSortMode('bogus')).toBe(false);
+    expect(isAlbumSortMode(undefined)).toBe(false);
+    expect(isAlbumSortMode(null)).toBe(false);
+  });
+});

lib/__tests__/config-hero-uuid.test.ts

@@ -76,6 +76,27 @@ describe('gallery.yaml per-album heroImage', () => {
     expect(getConfig().albumHeroImages[ALBUM_ID]).toBe(HERO_ID);
   });
 
+  // Same class of ID, same requirement: a pinned asset ID from `sort: manual`
+  // is written by the admin panel and can be hand-edited, so it goes through
+  // the same gate. A substituted zero UUID simply never matches an asset in the
+  // album, so the pin is ignored rather than corrupting the order.
+  it('routes manual assetOrder IDs through validateUuid, naming the album', () => {
+    vi.mocked(loadYaml).mockImplementation((filename: string) => {
+      if (filename === 'gallery.yaml') {
+        return { albums: [{ [ALBUM_ID]: { sort: 'manual', assetOrder: [HERO_ID] } }] };
+      }
+      if (filename === 'settings.yaml') return {};
+      return null;
+    });
+    invalidateConfigCache();
+
+    const config = getConfig();
+
+    expect(config.albumManualOrders[ALBUM_ID]).toEqual([HERO_ID]);
+    const call = validateUuidSpy.mock.calls.find((c) => c[0] === HERO_ID);
+    expect(call?.[1]).toContain(ALBUM_ID);
+  });
+
   // The substitution the wiring above depends on: validateUuid never throws, so
   // a bad ID degrades to a 404 image rather than taking the whole page down.
   it('real validateUuid warns and substitutes rather than throwing', async () => {

lib/__tests__/derive-gallery.test.ts

@@ -104,6 +104,77 @@ describe('deriveGallery accepts valid structures', () => {
     expect(result.albumHeroImages[A]).toBe(B);
   });
 
+  it('collects the per-album sort mode', () => {
+    const result = deriveGallery({
+      albums: [{ [A]: { title: 'Series', sort: 'filename' } }, B],
+    } as unknown as GalleryYaml);
+
+    expect(result.albumSortModes[A]).toBe('filename');
+    // Absent means the default; an explicit entry would make `immich` and
+    // "never configured" indistinguishable downstream.
+    expect(result.albumSortModes[B]).toBeUndefined();
+  });
+
+  // A typo would otherwise be invisible: the album would keep rendering in
+  // Immich order and nothing would say why. Throwing is also what gives the
+  // admin PUT its 400, via the deriveGallery dry-run.
+  it('rejects an unknown sort mode, naming the album and the valid values', () => {
+    expect(() =>
+      deriveGallery({ albums: [{ [A]: { sort: 'alphabetical' } }] } as unknown as GalleryYaml),
+    ).toThrow(/alphabetical/);
+
+    expect(() =>
+      deriveGallery({ albums: [{ [A]: { sort: 'alphabetical' } }] } as unknown as GalleryYaml),
+    ).toThrow(new RegExp(A));
+
+    expect(() =>
+      deriveGallery({ albums: [{ [A]: { sort: 'alphabetical' } }] } as unknown as GalleryYaml),
+    ).toThrow(/filename/);
+  });
+
+  it('collects the manual asset order, preserving its order', () => {
+    const result = deriveGallery({
+      albums: [{ [A]: { sort: 'manual', assetOrder: [B, A] } }],
+    } as unknown as GalleryYaml);
+
+    expect(result.albumManualOrders[A]).toEqual([B, A]);
+  });
+
+  // Kept regardless of the mode, so toggling manual → newest → manual in the
+  // admin panel does not silently destroy a hand-curated order.
+  it('keeps the asset order even when the mode is not manual', () => {
+    const result = deriveGallery({
+      albums: [{ [A]: { sort: 'newest', assetOrder: [B] } }],
+    } as unknown as GalleryYaml);
+
+    expect(result.albumSortModes[A]).toBe('newest');
+    expect(result.albumManualOrders[A]).toEqual([B]);
+  });
+
+  it('ignores an empty asset order rather than storing one', () => {
+    const result = deriveGallery({
+      albums: [{ [A]: { sort: 'manual', assetOrder: [] } }],
+    } as unknown as GalleryYaml);
+
+    expect(result.albumManualOrders[A]).toBeUndefined();
+  });
+
+  // processAlbumEntry is reached from four places; only the array-subpage path
+  // was covered before, so a regression in the others would go unnoticed.
+  it('applies the sort mode to section and map-style subpage albums too', () => {
+    const sectioned = deriveGallery({
+      subpages: [
+        { name: 'Work', sections: [{ title: 'Recent', albums: [{ [A]: { sort: 'oldest' } }] }] },
+      ],
+    } as unknown as GalleryYaml);
+    expect(sectioned.albumSortModes[A]).toBe('oldest');
+
+    const mapStyle = deriveGallery({
+      subpages: { Trips: { albums: [{ [B]: { sort: 'filename' } }] } },
+    } as unknown as GalleryYaml);
+    expect(mapStyle.albumSortModes[B]).toBe('filename');
+  });
+
   it('parses enabled property on subpages correctly', () => {
     const result = deriveGallery({
       subpages: [

lib/__tests__/immich.test.ts

@@ -2,6 +2,12 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { immich, ImmichUnavailableError } from '../immich';
 import * as config from '../config';
 import { cache } from '../cache';
+import type { AlbumSortMode } from '../albumSort';
+
+// Per-album sort is read on every getAlbum(), so the tests below need to be
+// able to change it between calls without rebuilding the whole mock.
+const albumSortModes: Record<string, AlbumSortMode> = {};
+const albumManualOrders: Record<string, string[]> = {};
 
 // Mock the config by wrapping it in a factory
 vi.mock('../config', async () => {
@@ -16,6 +22,8 @@ vi.mock('../config', async () => {
       subpages: [],
       albumOverrides: { 'album-1': 'Override Name' },
       albumDescriptions: {},
+      albumSortModes,
+      albumManualOrders,
       // Non-zero: a 0 TTL expires an entry in the same millisecond it is
       // written, which would make any caching assertion flaky. Isolation comes
       // from the cache.clear() in beforeEach, not from an unusable TTL.
@@ -37,6 +45,8 @@ describe('ImmichClient', () => {
     // getAlbum() memoises into the module-level LRU; without this, later tests
     // read a previous test's album and never reach the mocked fetch.
     cache.clear();
+    for (const key of Object.keys(albumSortModes)) delete albumSortModes[key];
+    for (const key of Object.keys(albumManualOrders)) delete albumManualOrders[key];
   });
 
   describe('request()', () => {
@@ -579,6 +589,69 @@ describe('ImmichClient', () => {
       const album = await immich.getAlbum('album-1');
       expect(album?.assets.map((a) => a.id)).toEqual(['frame-1', 'frame-2', 'frame-3']);
     });
+
+    /**
+     * The configured sort is applied on the way out of getAlbum(), so the LRU
+     * keeps one canonical Immich-ordered copy per album. Sorting the cached
+     * array in place would reorder the shared entry, and the next reader with a
+     * different mode would then be sorting an already-sorted array.
+     */
+    describe('per-album sort', () => {
+      const items = [
+        { ...asset('b', '2024-02-01T00:00:00Z'), originalFileName: 'IMG_2.jpg' },
+        { ...asset('c', '2024-03-01T00:00:00Z'), originalFileName: 'IMG_10.jpg' },
+        { ...asset('a', '2024-01-01T00:00:00Z'), originalFileName: 'IMG_1.jpg' },
+      ];
+
+      it('overrides the Immich direction', async () => {
+        albumSortModes['album-1'] = 'oldest';
+        mockAlbumApi({ order: 'desc', pages: [{ items, nextPage: null }] });
+
+        const album = await immich.getAlbum('album-1');
+        expect(album?.assets.map((a) => a.id)).toEqual(['a', 'b', 'c']);
+      });
+
+      it('applies the manual pin order', async () => {
+        albumSortModes['album-1'] = 'manual';
+        albumManualOrders['album-1'] = ['c'];
+        mockAlbumApi({ order: 'asc', pages: [{ items, nextPage: null }] });
+
+        const album = await immich.getAlbum('album-1');
+        expect(album?.assets.map((a) => a.id)).toEqual(['c', 'a', 'b']);
+      });
+
+      it('leaves the cached album in canonical order', async () => {
+        albumSortModes['album-1'] = 'filename';
+        mockAlbumApi({ order: 'asc', pages: [{ items, nextPage: null }] });
+
+        const first = await immich.getAlbum('album-1');
+        expect(first?.assets.map((a) => a.id)).toEqual(['a', 'b', 'c']); // IMG_1, IMG_2, IMG_10
+
+        // Same cache entry, different mode: if the first call had sorted the
+        // stored array, the album would still be in filename order here.
+        albumSortModes['album-1'] = 'newest';
+        const second = await immich.getAlbum('album-1');
+        expect(second?.assets.map((a) => a.id)).toEqual(['c', 'b', 'a']);
+        expect(second?.assets).not.toBe(first?.assets);
+      });
+
+      it('gives concurrent callers independent arrays', async () => {
+        albumSortModes['album-1'] = 'manual';
+        albumManualOrders['album-1'] = ['b'];
+        mockAlbumApi({ order: 'asc', pages: [{ items, nextPage: null }] });
+
+        // Coalesced through the same pending promise, but each caller applies
+        // the sort itself — so neither can be handed the other's array.
+        const [one, two] = await Promise.all([
+          immich.getAlbum('album-1'),
+          immich.getAlbum('album-1'),
+        ]);
+
+        expect(one?.assets.map((a) => a.id)).toEqual(['b', 'a', 'c']);
+        expect(two?.assets.map((a) => a.id)).toEqual(['b', 'a', 'c']);
+        expect(one?.assets).not.toBe(two?.assets);
+      });
+    });
   });
 });