fix(shared,tests): bound the book contract and close the gaps that hid all this

Book page setup and element geometry were unbounded numbers, so a shared
book could be saved with zero, negative or astronomical dimensions and every
consumer downstream - the canvas, the PDF export - had to cope. Day-note caps
had drifted from the limits their own doc comment states, and icon had none.

On the test side: the path filter meant a PR touching only the root lockfile
ran no jobs at all, which is exactly the change class most likely to break a
native binding; the canAccessTrip mock is hand-copied into 72 files and had
already drifted from the real query; the path-traversal test asserted only
status !== 200, so a 500 from a broken guard passed it; and one unit test
shelled out to a real kitinerary binary if the host happened to have one.

.github/workflows/test.yml

@@ -11,6 +11,13 @@ on:
       - 'client/**'
       - 'shared/**'
       - 'plugin-sdk/**'
+      # npm workspaces hoist every dependency into the single root lockfile, so
+      # a dependency bump touches none of the four directories above — and a
+      # native binding like better-sqlite3 is exactly what needs the suite.
+      - 'package.json'
+      - 'package-lock.json'
+      - 'Dockerfile'
+      - 'docker-compose.yml'
       - '.github/workflows/test.yml'
 
 jobs:

server/src/nest/day-notes/day-notes.mcp.ts

@@ -40,7 +40,7 @@ export class DayNotesMcp {
       dayId: z.number().int().positive(),
       text: z.string().min(1).max(500),
       time: z.string().max(250).optional().describe('Time label (e.g. "09:00" or "Morning")'),
-      icon: z.string().optional().describe('Emoji icon for the note'),
+      icon: z.string().max(64).optional().describe('Emoji icon for the note'),
     },
     annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT,
     access: { group: 'trips', mode: 'write' },
@@ -67,7 +67,7 @@ export class DayNotesMcp {
       noteId: z.number().int().positive(),
       text: z.string().min(1).max(500).optional(),
       time: z.string().max(250).nullable().optional().describe('Time label (e.g. "09:00" or "Morning"), or null to clear'),
-      icon: z.string().optional().describe('Emoji icon for the note'),
+      icon: z.string().max(64).optional().describe('Emoji icon for the note'),
     },
     annotations: TOOL_ANNOTATIONS_WRITE,
     access: { group: 'trips', mode: 'write' },

server/tests/helpers/mcp-test-controllers.ts

@@ -119,8 +119,9 @@ export function createMcpTestRegistry(): McpRegistry {
   const journeyDomain = new JourneyDomainService(dbService, realtimeService, new TrekPhotosRepository(dbService));
   // The last three were previously omitted, which left them `undefined` at
   // runtime — silently fine while nothing called them, a TypeError the moment
-  // the journey skeleton hooks landed on the place write paths. tsconfig only
-  // includes `src`, so nothing here is typechecked; pass them for real.
+  // the journey skeleton hooks landed on the place write paths. tsconfig.tests.json
+  // covers `tests` now and CI runs it (npm run typecheck:tests), so a missed
+  // dependency fails the build — pass them for real regardless of the gate.
   const placesService = new PlacesService(
     dbService, permissionsService, realtimeService, mapsService, queryHelpersService,
     new UnsplashService(dbService, new RuntimeEnvService(), generalStorage),

server/tests/helpers/test-db.ts

@@ -217,6 +217,20 @@ export function resetTestDb(db: Database.Database): void {
   seedDefaults(db);
 }
 
+/**
+ * Byte-for-byte the statement in src/db/database.ts.
+ *
+ * Exported because the same query is copied into ~90 test files, and every one
+ * of those copies had dropped `t.currency` — so the budget domain, which reads
+ * exactly that column off the access row, was only ever exercising its 'EUR'
+ * fallback. Import this instead of retyping it.
+ */
+export const CAN_ACCESS_TRIP_SQL = `
+        SELECT t.id, t.user_id, t.currency FROM trips t
+        LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ?
+        WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)
+      `;
+
 /**
  * Returns the mock factory for vi.mock('../../src/db/database', ...).
  * The returned object mirrors the shape of database.ts exports.
@@ -266,11 +280,7 @@ export function buildDbMock(testDb: Database.Database) {
       };
     },
     canAccessTrip: (tripId: number | string, userId: number) => {
-      return testDb.prepare(`
-        SELECT t.id, t.user_id FROM trips t
-        LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ?
-        WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)
-      `).get(userId, tripId, userId);
+      return testDb.prepare(CAN_ACCESS_TRIP_SQL).get(userId, tripId, userId);
     },
     isOwner: (tripId: number | string, userId: number) => {
       return !!testDb.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId);

server/tests/integration/security.test.ts

@@ -221,7 +221,10 @@ describe('File download path traversal', () => {
     const res = await request(app)
       .get(`/api/trips/${trip.id}/files/${fileId}/download`)
       .set(authHeader(user.id));
-    // path.basename() strips traversal in the download controller; normalized file does not exist in uploads
-    expect(res.status).not.toBe(200);
+    // path.basename() strips traversal in the download controller; the normalized
+    // name does not exist in uploads, so the answer is the same 404 a missing file
+    // gets. Pinned exactly: a 500 from a thrown guard would also be "not 200".
+    expect(res.status).toBe(404);
+    expect(res.body).toEqual({ error: 'File not found' });
   });
 });

server/tests/unit/nest/booking-import/kitinerary-extractor.service.test.ts

@@ -8,10 +8,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
  * at all, and every branch that resolves it is a filesystem lookup that behaves
  * differently on the three platforms TREK ships to.
  */
-const { existsSync, readdirSync, readEnv } = vi.hoisted(() => ({
+const { existsSync, readdirSync, readEnv, execSync, execFile } = vi.hoisted(() => ({
   existsSync: vi.fn(),
   readdirSync: vi.fn(),
   readEnv: vi.fn(),
+  execSync: vi.fn(),
+  // A plain function, because the service promisifies it at module load.
+  execFile: vi.fn(),
 }));
 
 vi.mock('node:fs', () => ({
@@ -20,6 +23,10 @@ vi.mock('node:fs', () => ({
   writeFileSync: vi.fn(),
   unlinkSync: vi.fn(),
 }));
+// The last branch of the probe shells out. Unmocked, the suite spawns a real
+// process on every machine and comes back green-or-red depending on whether the
+// developer happens to have KItinerary installed.
+vi.mock('node:child_process', () => ({ execSync, execFile }));
 vi.mock('../../../../src/app-config', () => ({ readEnv }));
 
 import { KitineraryExtractorService } from '../../../../src/nest/booking-import/kitinerary-extractor.service';
@@ -35,6 +42,7 @@ beforeEach(() => {
   vi.clearAllMocks();
   existsSync.mockReturnValue(false);
   readdirSync.mockReturnValue([]);
+  execSync.mockImplementation(() => { throw new Error('command not found'); });
 });
 
 describe('KitineraryExtractorService binary probe', () => {
@@ -70,4 +78,10 @@ describe('KitineraryExtractorService binary probe', () => {
   it('KIT-EXT-006: extracting without a binary fails loudly', async () => {
     await expect(boot().extract(Buffer.from(''), 'x.pdf')).rejects.toThrow('not available');
   });
+
+  it('KIT-EXT-007: falls back to the binary on PATH when nothing is on disk', () => {
+    execSync.mockReturnValue(Buffer.from(''));
+    expect(boot().isAvailable()).toBe(true);
+    expect(execSync).toHaveBeenCalledWith('kitinerary-extractor --version', expect.objectContaining({ timeout: 3000 }));
+  });
 });

shared/src/book/book.schema.spec.ts

@@ -3,6 +3,7 @@ import {
   bookBadgeElementSchema,
   bookIconElementSchema,
   bookSpreadSchema,
+  bookStatsElementSchema,
   bookTextElementSchema,
   normalizeBookDocument,
 } from './book.schema';
@@ -136,6 +137,22 @@ describe('a badge code', () => {
   });
 });
 
+describe('the figures a stats element carries', () => {
+  const stats = (over: Record<string, unknown> = {}) => ({ id: 's1', kind: 'stats', frame, ...over });
+
+  it('keeps the metrics it is drawn from', () => {
+    const parsed = bookStatsElementSchema.parse(stats({ values: { distance: 1420000, days: 12 } }));
+    expect(parsed.values).toEqual({ distance: 1420000, days: 12 });
+  });
+
+  it('drops a key no reader could ever index by, without failing the element', () => {
+    // Dropped rather than rejected: a rejected stats element is stripped as
+    // unreadable, which loses the page the figures were on.
+    const parsed = bookStatsElementSchema.parse(stats({ values: { distance: 12, sneaky: 1, ['x'.repeat(5000)]: 2 } }));
+    expect(parsed.values).toEqual({ distance: 12 });
+  });
+});
+
 describe('normalizeBookDocument', () => {
   it('keeps the nine elements it can read when the tenth is from another version', () => {
     const nine = Array.from({ length: 9 }, (_, i) => text({ id: `t${i + 1}`, text: `line ${i + 1}` }));
@@ -172,6 +189,34 @@ describe('normalizeBookDocument', () => {
     expect(out.page.pageWidth).toBe(210);
   });
 
+  it('falls back to the preset for a page dimension nothing could be printed at', () => {
+    // A page block that fails takes the whole document with it, and the empty
+    // book that comes back is what the server then writes down. So each of
+    // these degrades to its default and the spread survives.
+    for (const bad of [0, -5, 1e12]) {
+      const out = normalizeBookDocument({
+        ...doc([text({ id: 'a' })]),
+        page: { preset: 'square-210', pageWidth: bad, pageHeight: 210, bleed: 3, safe: 5 },
+      });
+      expect(out.page.pageWidth).toBe(210);
+      expect(out.spreads[0]!.elements.map((el) => el.id)).toEqual(['a']);
+    }
+
+    const negativeBleed = normalizeBookDocument({
+      ...doc([text({ id: 'a' })]),
+      page: { preset: 'square-210', pageWidth: 210, pageHeight: 210, bleed: -3, safe: 5 },
+    });
+    expect(negativeBleed.page.bleed).toBe(3);
+    expect(negativeBleed.spreads[0]!.elements).toHaveLength(1);
+  });
+
+  it('drops an element parked a kilometre off the spread rather than the book', () => {
+    const out = normalizeBookDocument(doc([text({ id: 'a' }), text({ id: 'far', frame: { x: 1e9, y: 0, w: 60, h: 40 } })]));
+
+    expect(out.spreads[0]!.elements.map((el) => el.id)).toEqual(['a']);
+    expect(out.title).toBe('Iceland, end to end');
+  });
+
   it('hands back an empty book for something that is not a document at all', () => {
     for (const junk of [undefined, null, 'a book, honestly', 42, [], { spreads: 'nope' }]) {
       const out = normalizeBookDocument(junk);

shared/src/book/book.schema.ts

@@ -25,8 +25,16 @@ import { z } from 'zod';
  * edits the text in Studio, which sets `overridden` and stops the sync.
  */
 
-/** Two decimals of a millimetre. Anything finer is below print resolution. */
-const mm = z.number().finite().transform(v => Math.round(v * 100) / 100);
+/**
+ * Two decimals of a millimetre. Anything finer is below print resolution.
+ *
+ * Bounded at ten metres in either direction, which is four orders of magnitude
+ * past the largest book anyone binds and still leaves room for an element
+ * parked well off the spread. Without a bound a stored `1e9` is a legal
+ * millimetre, and the editor and the PDF renderer both try to draw a sheet a
+ * thousand kilometres wide.
+ */
+const mm = z.number().finite().min(-10000).max(10000).transform(v => Math.round(v * 100) / 100);
 
 const hex = z.string().regex(/^#[0-9a-fA-F]{6}$/, 'expected #rrggbb');
 
@@ -491,8 +499,21 @@ export const bookStatsElementSchema = z.object({
   layout: z.enum(['grid', 'row', 'column']).default('grid'),
   showIcons: z.boolean().default(true),
   units: bookUnitsSchema.default('metric'),
-  /** Metric to value. Distance is metres, everything else a plain count. */
-  values: z.record(z.string(), z.number().finite()).default({}),
+  /**
+   * Metric to value. Distance is metres, everything else a plain count.
+   *
+   * Anything that is not a metric is dropped rather than rejected: both readers
+   * only ever index by a `BOOK_METRICS` member, so a stray key is payload that
+   * gets persisted and re-served to every viewer without ever being drawn.
+   * Filtered instead of typed as `z.record(z.enum(BOOK_METRICS), …)`, because on
+   * zod 4 an enum key makes the record exhaustive — every metric would become
+   * required and every stats element written so far would fail, then be thrown
+   * away as unreadable.
+   */
+  values: z.record(z.string(), z.number().finite()).default({})
+    .transform(v => Object.fromEntries(
+      Object.entries(v).filter(([k]) => (BOOK_METRICS as readonly string[]).includes(k)),
+    )),
 });
 
 export const bookCountriesElementSchema = z.object({
@@ -729,11 +750,19 @@ export type BookPageNumbers = z.infer<typeof bookPageNumbersSchema>;
 
 export const bookPageSetupSchema = z.object({
   preset: z.enum(['square-210', 'square-300', 'a4-landscape', 'a4-portrait', 'a5-landscape', 'custom']).default('square-210'),
-  /** A single page. A double spread is drawn twice this wide. */
-  pageWidth: mm.default(210),
-  pageHeight: mm.default(210),
-  bleed: mm.default(3),
-  safe: mm.default(5),
+  /**
+   * A single page. A double spread is drawn twice this wide.
+   *
+   * These four fall back rather than fail, like `version` below. The salvage
+   * pass only strips unreadable *elements*, so a page block the contract
+   * cannot read takes the whole book down to an empty one — and that empty
+   * book is what the server writes back. A nonsense dimension degrading to the
+   * preset is the smaller loss by a long way.
+   */
+  pageWidth: mm.refine(v => v > 0, 'page width must be positive').catch(210).default(210),
+  pageHeight: mm.refine(v => v > 0, 'page height must be positive').catch(210).default(210),
+  bleed: mm.refine(v => v >= 0, 'bleed cannot be negative').catch(3).default(3),
+  safe: mm.refine(v => v >= 0, 'safe margin cannot be negative').catch(5).default(5),
   pageNumbers: bookPageNumbersSchema.default(() => bookPageNumbersSchema.parse({})),
 });
 export type BookPageSetup = z.infer<typeof bookPageSetupSchema>;

shared/src/day/day.schema.spec.ts

@@ -35,6 +35,15 @@ describe('dayNoteCreateRequestSchema', () => {
   it('accepts null time/icon (moveDayNote re-sends the nullable entity fields)', () => {
     expect(dayNoteCreateRequestSchema.safeParse({ text: 'ok', time: null, icon: null }).success).toBe(true);
   });
+
+  it('takes an icon as a label and not as a payload', () => {
+    // Both shapes the field is written in: a lucide export name from NOTE_ICONS
+    // and a plain emoji, plus the longest ZWJ sequence anyone types.
+    expect(dayNoteCreateRequestSchema.safeParse({ text: 'ok', icon: 'ParkingSquare' }).success).toBe(true);
+    expect(dayNoteCreateRequestSchema.safeParse({ text: 'ok', icon: '👩‍👩‍👧‍👦' }).success).toBe(true);
+    expect(dayNoteCreateRequestSchema.safeParse({ text: 'ok', icon: 'x'.repeat(64) }).success).toBe(true);
+    expect(dayNoteCreateRequestSchema.safeParse({ text: 'ok', icon: 'x'.repeat(65) }).success).toBe(false);
+  });
 });
 
 describe('dayNoteUpdateRequestSchema', () => {
@@ -42,6 +51,7 @@ describe('dayNoteUpdateRequestSchema', () => {
     expect(dayNoteUpdateRequestSchema.safeParse({}).success).toBe(true);
     expect(dayNoteUpdateRequestSchema.safeParse({ icon: '🍽️' }).success).toBe(true);
     expect(dayNoteUpdateRequestSchema.safeParse({ text: 'x'.repeat(501) }).success).toBe(false);
+    expect(dayNoteUpdateRequestSchema.safeParse({ icon: 'x'.repeat(65) }).success).toBe(false);
   });
 
   it('accepts an explicit null time (clears the label)', () => {

shared/src/day/day.schema.ts

@@ -9,8 +9,10 @@ import { z } from 'zod';
  * Trip-scoped, both gated by the 'day_edit' permission. Served by the Nest
  * DaysController / DayNotesController (server/src/nest/days/). Day rows (with
  * their assignments) are wide and DB-derived, so list responses stay open. Day
- * notes cap text at 500 and time at 150 chars (the legacy
- * validateStringLengths middleware) — reproduced in the controller.
+ * note writes are capped field by field: text 500, time 2000, icon 64, color 9.
+ * Time is wider than the legacy validateStringLengths middleware's 150 because
+ * the raw-body route it replaced already accepted more, and narrowing it now
+ * would reject payloads today's client is allowed to send.
  */
 
 /**
@@ -86,7 +88,9 @@ export type DayTransportRequest = z.infer<typeof dayTransportRequestSchema>;
 export const dayNoteCreateRequestSchema = z.object({
   text: z.string().min(1).max(500),
   time: z.string().max(2000).nullable().optional(),
-  icon: z.string().nullable().optional(),
+  // A lucide name from NOTE_ICONS or a single emoji; 64 leaves room for a ZWJ
+  // sequence with skin-tone modifiers and still keeps the column a label.
+  icon: z.string().max(64).nullable().optional(),
   /** One of NOTE_COLORS, or null for the neutral card (#1629). */
   color: z.string().max(9).nullable().optional(),
   sort_order: z.number().optional(),
@@ -96,7 +100,7 @@ export type DayNoteCreateRequest = z.infer<typeof dayNoteCreateRequestSchema>;
 export const dayNoteUpdateRequestSchema = z.object({
   text: z.string().max(500).optional(),
   time: z.string().max(2000).nullable().optional(),
-  icon: z.string().nullable().optional(),
+  icon: z.string().max(64).nullable().optional(),
   color: z.string().max(9).nullable().optional(),
   sort_order: z.number().optional(),
 });

shared/src/realtime/events.schema.ts

@@ -352,7 +352,7 @@ export const TREK_WS_EVENTS = {
   },
 } as const satisfies Record<string, TrekWsEventContract>;
 
-/** Every registered event name (65 trip-scoped + 29 user-scoped). */
+/** Every registered event name. The registry above is the count; this derives from it. */
 export type TrekWsEventName = keyof typeof TREK_WS_EVENTS;
 
 /** Names broadcast to a trip room (`{ type, tripId, ...payload }` envelope). */