From 688eea23a451d2a07e708b2fed8fee4ed62bce3a Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sun, 16 Aug 2026 21:36:41 +0200
Subject: [PATCH 1/9] fix(dashboard): keep the trip card actions on the cover
 and give them contrast

The edit/duplicate/archive/delete buttons sat at opacity 0 until the card was
hovered, over a white .18 fill. Two people in a row said they never found how
to unarchive or rename a trip: invisible at rest, and white-on-white over a
light cover even while hovering.

They stay on the card now at .92, the fill is a dark scrim so the white icons
carry against any photo, and the icons get a heavier stroke. The status pill
and the hero badge move to the same scrim so the cover overlays read as one
set. An archived card shows the restore icon rather than the archive one,
which is what mobile already did, and every button gains a title next to its
aria-label.
---
 client/src/pages/DashboardPage.test.tsx | 55 +++++++++++++++++++++++++
 client/src/pages/DashboardPage.tsx      | 20 +++++----
 client/src/styles/dashboard.css         | 48 +++++++++++++--------
 3 files changed, 97 insertions(+), 26 deletions(-)

diff --git a/client/src/pages/DashboardPage.test.tsx b/client/src/pages/DashboardPage.test.tsx
index 88f93cc49..cb838d62a 100644
--- a/client/src/pages/DashboardPage.test.tsx
+++ b/client/src/pages/DashboardPage.test.tsx
@@ -1000,4 +1000,59 @@ describe('DashboardPage', () => {
       expect(within(grid()).queryByText('Paris Adventure')).not.toBeInTheDocument();
     });
   });
+
+  // Two people reported in a row that they could not find how to unarchive or
+  // rename a trip. The controls sat at opacity 0 until the card was hovered, and
+  // the archive button carried the same icon whether it would archive or restore.
+  describe('FE-PAGE-DASH-036: the cover controls are findable', () => {
+    const activeOnly = (trips: unknown[]) =>
+      server.use(
+        http.get('/api/trips', ({ request }) => {
+          const url = new URL(request.url);
+          if (url.searchParams.get('archived')) return HttpResponse.json({ trips: [] });
+          return HttpResponse.json({ trips });
+        }),
+      );
+
+    it('names every action on the card, so none of them is icon-only guesswork', async () => {
+      activeOnly([buildTrip({ title: 'Lisbon 2025', start_date: '2025-05-01', end_date: '2025-05-08' })]);
+      const user = userEvent.setup();
+      render(<DashboardPage />);
+
+      await waitFor(() => expect(screen.getAllByText('Lisbon 2025').length).toBeGreaterThan(0));
+      await user.click(screen.getByText('Completed'));
+      const card = (await screen.findAllByText('Lisbon 2025'))
+        .map(n => n.closest('.trip-card'))
+        .find(Boolean) as HTMLElement;
+      for (const label of ['Edit', 'Duplicate', 'Archive', 'Delete']) {
+        const button = card.querySelector(`[aria-label="${label}"]`) as HTMLElement;
+        expect(button).toBeTruthy();
+        // A tooltip as well as the screen-reader label — the icons alone did not read.
+        expect(button.getAttribute('title')).toBe(label);
+      }
+    });
+
+    it('swaps the archive icon for a restore one once the trip is archived', async () => {
+      const archived = buildTrip({ title: 'Old Rome Trip', start_date: '2024-01-01', end_date: '2024-01-07', is_archived: 1 });
+      server.use(
+        http.get('/api/trips', ({ request }) => {
+          const url = new URL(request.url);
+          return HttpResponse.json({ trips: url.searchParams.get('archived') ? [archived] : [] });
+        }),
+      );
+      const user = userEvent.setup();
+      render(<DashboardPage />);
+
+      await user.click(screen.getByText('Archived'));
+      await waitFor(() => expect(screen.getByText('Old Rome Trip')).toBeInTheDocument());
+
+      const card = screen.getByText('Old Rome Trip').closest('.trip-card') as HTMLElement;
+      const restore = card.querySelector('[aria-label="Restore"]') as HTMLElement;
+      expect(restore).toBeTruthy();
+      // lucide stamps the icon name on the svg, which is how the archive and the
+      // restore variant tell themselves apart.
+      expect(restore.querySelector('svg')?.getAttribute('class')).toContain('archive-restore');
+      expect(card.querySelector('[aria-label="Archive"]')).toBeNull();
+    });
+  });
 });
diff --git a/client/src/pages/DashboardPage.tsx b/client/src/pages/DashboardPage.tsx
index f1613eb9c..dad6a303c 100644
--- a/client/src/pages/DashboardPage.tsx
+++ b/client/src/pages/DashboardPage.tsx
@@ -15,7 +15,7 @@ import {
   MS_PER_DAY, daysUntil, getTripStatus,
 } from './dashboard/dashboardModel'
 import {
-  Plus, Edit2, Trash2, Archive, Copy, ArrowRight, MapPin,
+  Plus, Edit2, Trash2, Archive, ArchiveRestore, Copy, ArrowRight, MapPin,
   Plane, Hotel, Utensils, Clock, RefreshCw, ArrowRightLeft, Calendar,
   LayoutGrid, List, Ticket, X, CalendarPlus, ParkingSquare,
 } from 'lucide-react'
@@ -304,6 +304,7 @@ function BoardingPassHero({ trip, bundle, locale, onOpen, onEdit, onCopy, onArch
   const heroPlugins = usePluginStore(s => s.plugins).filter(p => p.type === 'widget' && p.slot === 'hero')
   const stop = (e: React.MouseEvent, fn: () => void) => { e.stopPropagation(); fn() }
   const status = getTripStatus(trip)
+  const archiveLabel = trip.is_archived ? t('dashboard.restore') : t('dashboard.archive')
   const start = splitDate(trip.start_date, locale)
   const end = splitDate(trip.end_date, locale)
 
@@ -404,10 +405,10 @@ function BoardingPassHero({ trip, bundle, locale, onOpen, onEdit, onCopy, onArch
             {badge}
           </div>
           <div className="hero-tools">
-            <button className="hero-tool" aria-label={t('common.edit')} onClick={(e) => stop(e, onEdit)}><Edit2 size={16} /></button>
-            <button className="hero-tool" aria-label={t('dashboard.aria.duplicate')} onClick={(e) => stop(e, onCopy)}><Copy size={16} /></button>
-            <button className="hero-tool" aria-label={trip.is_archived ? t('dashboard.restore') : t('dashboard.archive')} onClick={(e) => stop(e, onArchive)}><Archive size={16} /></button>
-            <button className="hero-tool" aria-label={t('common.delete')} onClick={(e) => stop(e, onDelete)}><Trash2 size={16} /></button>
+            <button className="hero-tool" aria-label={t('common.edit')} title={t('common.edit')} onClick={(e) => stop(e, onEdit)}><Edit2 size={17} strokeWidth={2.2} /></button>
+            <button className="hero-tool" aria-label={t('dashboard.aria.duplicate')} title={t('dashboard.aria.duplicate')} onClick={(e) => stop(e, onCopy)}><Copy size={17} strokeWidth={2.2} /></button>
+            <button className="hero-tool" aria-label={archiveLabel} title={archiveLabel} onClick={(e) => stop(e, onArchive)}>{trip.is_archived ? <ArchiveRestore size={17} strokeWidth={2.2} /> : <Archive size={17} strokeWidth={2.2} />}</button>
+            <button className="hero-tool" aria-label={t('common.delete')} title={t('common.delete')} onClick={(e) => stop(e, onDelete)}><Trash2 size={17} strokeWidth={2.2} /></button>
           </div>
         </div>
 
@@ -535,6 +536,7 @@ function TripCard({ trip, locale, badges, onOpen, onEdit, onCopy, onArchive, onD
 }): React.ReactElement {
   const { t } = useTranslation()
   const status = getTripStatus(trip)
+  const archiveLabel = trip.is_archived ? t('dashboard.restore') : t('dashboard.archive')
   const start = splitDate(trip.start_date, locale)
   const end = splitDate(trip.end_date, locale)
   const until = daysUntil(trip.start_date)
@@ -557,10 +559,10 @@ function TripCard({ trip, locale, badges, onOpen, onEdit, onCopy, onArchive, onD
           : <div style={{ width: '100%', height: '100%', background: tripGradient(trip.id) }} />}
         <div className={`trip-status ${statusClass}`}><span className="indicator" /> {statusLabel}</div>
         <div className="trip-actions">
-          <button className="trip-action-btn" aria-label={t('common.edit')} onClick={(e) => stop(e, onEdit)}><Edit2 size={16} /></button>
-          <button className="trip-action-btn" aria-label={t('dashboard.aria.duplicate')} onClick={(e) => stop(e, onCopy)}><Copy size={16} /></button>
-          <button className="trip-action-btn" aria-label={trip.is_archived ? t('dashboard.restore') : t('dashboard.archive')} onClick={(e) => stop(e, onArchive)}><Archive size={16} /></button>
-          <button className="trip-action-btn" aria-label={t('common.delete')} onClick={(e) => stop(e, onDelete)}><Trash2 size={16} /></button>
+          <button className="trip-action-btn" aria-label={t('common.edit')} title={t('common.edit')} onClick={(e) => stop(e, onEdit)}><Edit2 size={17} strokeWidth={2.2} /></button>
+          <button className="trip-action-btn" aria-label={t('dashboard.aria.duplicate')} title={t('dashboard.aria.duplicate')} onClick={(e) => stop(e, onCopy)}><Copy size={17} strokeWidth={2.2} /></button>
+          <button className="trip-action-btn" aria-label={archiveLabel} title={archiveLabel} onClick={(e) => stop(e, onArchive)}>{trip.is_archived ? <ArchiveRestore size={17} strokeWidth={2.2} /> : <Archive size={17} strokeWidth={2.2} />}</button>
+          <button className="trip-action-btn" aria-label={t('common.delete')} title={t('common.delete')} onClick={(e) => stop(e, onDelete)}><Trash2 size={17} strokeWidth={2.2} /></button>
         </div>
         <div className="trip-cover-content">
           <h3 className="trip-name">{trip.title}</h3>
diff --git a/client/src/styles/dashboard.css b/client/src/styles/dashboard.css
index 9e509b44a..1e0dcea34 100644
--- a/client/src/styles/dashboard.css
+++ b/client/src/styles/dashboard.css
@@ -191,11 +191,12 @@
 .trek-dash .hero-top { display: flex; justify-content: space-between; align-items: start; }
 .trek-dash .hero-badge {
   display: inline-flex; align-items: center; gap: 8px;
-  background: oklch(1 0 0 / .14);
+  background: oklch(0.22 0.01 260 / .5);
   backdrop-filter: blur(14px) saturate(1.4); -webkit-backdrop-filter: blur(14px) saturate(1.4);
-  border: 1px solid oklch(1 0 0 / .2); padding: 8px 14px 8px 12px;
+  border: 1px solid oklch(1 0 0 / .32); padding: 8px 14px 8px 12px;
   border-radius: 999px; font-size: 12px; font-weight: 500;
   letter-spacing: 0.06em; text-transform: uppercase;
+  box-shadow: 0 2px 8px oklch(0 0 0 / .26);
 }
 .trek-dash .hero-badge .pulse {
   width: 7px; height: 7px; border-radius: 50%;
@@ -208,15 +209,18 @@
   100% { box-shadow: 0 0 0 0 oklch(0.84 0.18 85 / 0); }
 }
 .trek-dash .hero-tools { display: flex; gap: 8px; }
+/* Same story as .trip-action-btn: a dark scrim instead of a white .14 fill, so
+   the tools read against a bright hero photo too. */
 .trek-dash .hero-tool {
-  width: 38px; height: 38px; border-radius: 50%;
-  background: oklch(1 0 0 / .14);
+  width: 40px; height: 40px; border-radius: 50%;
+  background: oklch(0.22 0.01 260 / .5);
   backdrop-filter: blur(14px) saturate(1.4); -webkit-backdrop-filter: blur(14px) saturate(1.4);
-  border: 1px solid oklch(1 0 0 / .2); color: #fff;
+  border: 1px solid oklch(1 0 0 / .32); color: #fff;
+  box-shadow: 0 2px 8px oklch(0 0 0 / .26);
   display: grid; place-items: center; transition: background .15s, transform .12s;
 }
-.trek-dash .hero-tool:hover { background: oklch(1 0 0 / .26); transform: scale(1.05); }
-.trek-dash .hero-tool svg { width: 16px; height: 16px; }
+.trek-dash .hero-tool:hover { background: oklch(0.22 0.01 260 / .78); transform: scale(1.05); }
+.trek-dash .hero-tool svg { width: 17px; height: 17px; }
 .trek-dash .hero-title-block { align-self: end; padding-bottom: 24px; }
 .trek-dash .hero-eyebrow {
   display: inline-flex; align-items: center; gap: 10px;
@@ -435,26 +439,36 @@
 .trek-dash .trip-cover img { width: 100%; height: 100%; object-fit: cover; transition: transform .6s cubic-bezier(.2,.7,.2,1); }
 .trek-dash .trip-card:hover .trip-cover img { transform: scale(1.04); }
 .trek-dash .trip-cover::after { content: ""; position: absolute; inset: 0; background: linear-gradient(180deg, oklch(0 0 0 / 0) 40%, oklch(0 0 0 / .55) 100%); }
+/* Shares the dark scrim of .trip-action-btn so both overlays on the cover read
+   as one set, and the label survives a bright photo behind it. */
 .trek-dash .trip-status {
   position: absolute; top: 16px; left: 16px; z-index: 1;
   display: inline-flex; align-items: center; gap: 7px; padding: 6px 11px 6px 9px;
   border-radius: 999px; font-size: 11.5px; font-weight: 500; letter-spacing: 0.02em;
-  background: oklch(1 0 0 / .18); backdrop-filter: blur(14px) saturate(1.4); -webkit-backdrop-filter: blur(14px) saturate(1.4);
-  border: 1px solid oklch(1 0 0 / .22); color: #fff; text-transform: uppercase;
+  background: oklch(0.22 0.01 260 / .58); backdrop-filter: blur(14px) saturate(1.4); -webkit-backdrop-filter: blur(14px) saturate(1.4);
+  border: 1px solid oklch(1 0 0 / .34); color: #fff; text-transform: uppercase;
+  box-shadow: 0 2px 8px oklch(0 0 0 / .28);
 }
 .trek-dash .trip-status .indicator { width: 6px; height: 6px; border-radius: 50%; background: oklch(0.84 0.16 145); }
 .trek-dash .trip-status.completed .indicator { background: oklch(0.75 0.02 220); }
 .trek-dash .trip-status.upcoming .indicator { background: oklch(0.84 0.18 85); }
 .trek-dash .trip-status.idea .indicator { background: oklch(0.78 0.14 280); }
-.trek-dash .trip-actions { position: absolute; top: 14px; right: 14px; z-index: 1; display: flex; gap: 6px; opacity: 0; transition: opacity .2s; }
+/* The actions used to sit at opacity 0 until the card was hovered, over a white
+   .18 glass fill. Two people in a row reported they never found the archive and
+   rename buttons — invisible at rest, and on a light cover the white-on-white
+   fill hid them even while hovering. They stay on the card now, and the fill is
+   a dark scrim so the white icons carry against any photo. */
+.trek-dash .trip-actions { position: absolute; top: 14px; right: 14px; z-index: 1; display: flex; gap: 6px; opacity: .92; transition: opacity .2s; }
 .trek-dash .trip-card:hover .trip-actions { opacity: 1; }
 .trek-dash .trip-action-btn {
-  width: 36px; height: 36px; border-radius: 50%;
-  background: oklch(1 0 0 / .18); backdrop-filter: blur(14px); -webkit-backdrop-filter: blur(14px);
-  border: 1px solid oklch(1 0 0 / .22); color: #fff; display: grid; place-items: center; transition: background .15s;
+  width: 38px; height: 38px; border-radius: 50%;
+  background: oklch(0.22 0.01 260 / .58); backdrop-filter: blur(14px); -webkit-backdrop-filter: blur(14px);
+  border: 1px solid oklch(1 0 0 / .34); color: #fff; display: grid; place-items: center;
+  box-shadow: 0 2px 8px oklch(0 0 0 / .28);
+  transition: background .15s, transform .12s;
 }
-.trek-dash .trip-action-btn:hover { background: oklch(1 0 0 / .3); }
-.trek-dash .trip-action-btn svg { width: 16px; height: 16px; }
+.trek-dash .trip-action-btn:hover { background: oklch(0.22 0.01 260 / .82); transform: scale(1.06); }
+.trek-dash .trip-action-btn svg { width: 17px; height: 17px; }
 .trek-dash .trip-cover-content { position: absolute; left: 18px; right: 18px; bottom: 16px; z-index: 1; color: #fff; }
 .trek-dash .trip-name { font-size: 26px; font-weight: 600; letter-spacing: -0.025em; line-height: 1.05; margin: 0; text-shadow: 0 1px 7px oklch(0 0 0 / .3), 0 1px 2px oklch(0 0 0 / .38); }
 .trek-dash .trip-where { margin-top: 4px; font-size: 13px; opacity: .85; display: flex; align-items: center; gap: 6px; }
@@ -686,8 +700,8 @@
   .trek-dash .trips { grid-template-columns: 1fr; gap: 16px; margin-bottom: 28px; }
   .trek-dash .add-trip-card { min-height: 180px; }
 
-  /* Touch devices have no hover — keep the edit/copy/archive/delete actions
-     visible at all times instead of revealing them on hover. */
+  /* Touch devices have no hover state to lift the actions to full strength, so
+     skip the resting dim entirely. */
   .trek-dash .trip-actions { opacity: 1; }
 
   /* Compact list row on mobile — keeps the list view distinct from the grid. The

From ff27624e0248f6187bcc8d86289622961a7fe4b2 Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sun, 16 Aug 2026 21:37:21 +0200
Subject: [PATCH 2/9] fix(reservations): scope the per-day position write to
 the trip

updatePositions has two branches. The legacy one updates
`WHERE id = ? AND trip_id = ?`; the per-day one, three lines below it, bound
the caller's ids straight into reservation_day_positions and never touched the
tripId it was handed. That table has no trip_id of its own and its two foreign
keys only require the ids to exist, so nothing further down caught a pair that
did not belong together.

The trip now comes from the join, so the row materialises only when the
reservation and the day agree on it. Doing that in the statement rather than as
a pre-check also turns a stale id into a quiet no-op instead of a foreign-key
error surfacing as a 500, and lets an omitted day_plan_position bind the way
the wire contract has always said it may.

reorder_reservations reports a day that is not on the trip, the way its sibling
tools in that file already do, and a migration clears rows whose reservation
and day disagree.
---
 server/src/db/migrations.ts                   | 18 ++++
 .../src/nest/reservations/reservations.mcp.ts |  7 ++
 .../nest/reservations/reservations.service.ts | 21 ++++-
 server/tests/e2e/reservations.e2e.test.ts     | 88 +++++++++++++++++++
 .../integration/leg-mode-incoming.test.ts     | 26 ++++--
 .../tests/unit/db/migration-hygiene.test.ts   |  2 +
 6 files changed, 150 insertions(+), 12 deletions(-)

diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts
index dfa0b72c9..6fc504466 100644
--- a/server/src/db/migrations.ts
+++ b/server/src/db/migrations.ts
@@ -3891,6 +3891,24 @@ function runMigrations(db: Database.Database): void {
         db.exec('ALTER TABLE budget_items ADD COLUMN place_id INTEGER REFERENCES places(id) ON DELETE SET NULL DEFAULT NULL');
       }
     },
+    // A reservation_day_positions row only makes sense when its reservation and
+    // its day are on the same trip. The table carries no trip_id and its two
+    // foreign keys only ask that the ids exist, so pairs that never belonged
+    // together could accumulate; the writer refuses them now, and this clears
+    // whatever an older build let through. Appended LAST — the array is
+    // index-addressed against schema_version.
+    () => {
+      db.exec(`
+        DELETE FROM reservation_day_positions
+         WHERE rowid IN (
+           SELECT rdp.rowid
+             FROM reservation_day_positions rdp
+             JOIN reservations r ON r.id = rdp.reservation_id
+             JOIN days d ON d.id = rdp.day_id
+            WHERE d.trip_id <> r.trip_id
+         )
+      `);
+    },
   ];
 
   if (currentVersion < migrations.length) {
diff --git a/server/src/nest/reservations/reservations.mcp.ts b/server/src/nest/reservations/reservations.mcp.ts
index 4e7114b0c..f8b3926ba 100644
--- a/server/src/nest/reservations/reservations.mcp.ts
+++ b/server/src/nest/reservations/reservations.mcp.ts
@@ -468,6 +468,13 @@ export class ReservationsMcp {
     if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
     if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess();
     if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied();
+
+    // The service scopes the write to the trip on its own, so a foreign id is
+    // already harmless — say so rather than reporting a success that moved
+    // nothing, the way the sibling tools above do.
+    if (dayId && !this.days.getDay(dayId, tripId))
+      return errorResult('dayId does not belong to this trip.');
+
     this.reservations.updatePositions(tripId, positions, dayId);
     this.guards.safeBroadcast(tripId, 'reservation:positions', { positions, dayId });
     return ok({ success: true });
diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts
index 3f0a814db..15ebbaa2e 100644
--- a/server/src/nest/reservations/reservations.service.ts
+++ b/server/src/nest/reservations/reservations.service.ts
@@ -527,13 +527,26 @@ export class ReservationsService {
     return { reservation, accommodationCreated };
   }
 
-  updatePositions(tripId: string | number, positions: { id: number; day_plan_position: number }[], dayId?: number | string | null) {
+  updatePositions(tripId: string | number, positions: { id: number; day_plan_position?: number }[], dayId?: number | string | null) {
     if (dayId) {
-      // Per-day positions for multi-day reservations
-      const stmt = this.db.prepare('INSERT OR REPLACE INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)');
+      // Per-day positions for multi-day reservations, scoped the way the legacy
+      // branch below already scopes its update. The table carries no trip_id and
+      // its two foreign keys only ask that the ids exist, so the trip has to come
+      // from the join: the row materialises only when the reservation and the day
+      // agree on it. Doing that in the statement rather than as a pre-check also
+      // makes a stale id a quiet no-op instead of a foreign-key error surfacing
+      // as a 500.
+      const stmt = this.db.prepare(`
+        INSERT OR REPLACE INTO reservation_day_positions (reservation_id, day_id, position)
+        SELECT r.id, d.id, ?
+          FROM reservations r
+          JOIN days d ON d.trip_id = r.trip_id
+         WHERE r.id = ? AND d.id = ? AND r.trip_id = ?
+      `);
       this.db.transaction(() => {
         for (const item of positions) {
-          stmt.run(item.id, dayId, item.day_plan_position);
+          // position is NOT NULL while the wire contract leaves the value optional.
+          stmt.run(item.day_plan_position ?? 0, item.id, dayId, tripId);
         }
       });
     } else {
diff --git a/server/tests/e2e/reservations.e2e.test.ts b/server/tests/e2e/reservations.e2e.test.ts
index bcac65060..7907ae5ca 100644
--- a/server/tests/e2e/reservations.e2e.test.ts
+++ b/server/tests/e2e/reservations.e2e.test.ts
@@ -185,4 +185,92 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite, real
     expect(res.status).toBe(400);
     expect(res.body).toEqual({ error: 'place_id, start_day_id, and end_day_id are required' });
   });
+
+  // The per-day branch of updatePositions used to bind the caller's ids straight
+  // into reservation_day_positions without ever looking at the tripId it was
+  // handed, so a request authorised on your own trip could rewrite the ordering
+  // of someone else's. The legacy branch three lines below it always scoped by
+  // trip_id — these pin that the two behave the same way now.
+  describe('positions are confined to the trip in the URL', () => {
+    let foreignTripId: number;
+    let foreignReservationId: number;
+    let foreignDayId: number;
+    let ownDayId: number;
+    // Days are unique per (trip_id, day_number) and this block seeds a fresh pair
+    // for every case, so the numbers have to keep climbing.
+    let dayNumber = 100;
+
+    beforeEach(() => {
+      db.prepare(
+        "INSERT OR IGNORE INTO users (id, username, email, password_hash, role, password_version) VALUES (2, 'victim', 'victim@example.test', 'x', 'user', 0)",
+      ).run();
+      foreignTripId = Number(db.prepare("INSERT INTO trips (user_id, title) VALUES (2, 'Someone else')").run().lastInsertRowid);
+      foreignReservationId = Number(db.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'Secret', 'other')").run(foreignTripId).lastInsertRowid);
+      dayNumber += 1;
+      foreignDayId = Number(db.prepare("INSERT INTO days (trip_id, day_number, date) VALUES (?, ?, '2026-05-01')").run(foreignTripId, dayNumber).lastInsertRowid);
+      ownDayId = Number(db.prepare("INSERT INTO days (trip_id, day_number, date) VALUES (?, ?, '2026-05-01')").run(tripId, dayNumber).lastInsertRowid);
+    });
+
+    function positionRow(reservationId: number, dayId: number) {
+      return db.prepare('SELECT position FROM reservation_day_positions WHERE reservation_id = ? AND day_id = ?').get(reservationId, dayId);
+    }
+
+    it('writes nothing when both ids belong to another trip', async () => {
+      const res = await request(server)
+        .put(`/api/trips/${tripId}/reservations/positions`)
+        .set('Cookie', sessionCookie(1))
+        .send({ positions: [{ id: foreignReservationId, day_plan_position: 999 }], day_id: foreignDayId });
+      expect(res.status).toBe(200);
+      expect(positionRow(foreignReservationId, foreignDayId)).toBeUndefined();
+    });
+
+    it('writes nothing when only the reservation is foreign', async () => {
+      const res = await request(server)
+        .put(`/api/trips/${tripId}/reservations/positions`)
+        .set('Cookie', sessionCookie(1))
+        .send({ positions: [{ id: foreignReservationId, day_plan_position: 5 }], day_id: ownDayId });
+      expect(res.status).toBe(200);
+      expect(positionRow(foreignReservationId, ownDayId)).toBeUndefined();
+    });
+
+    it('writes nothing when only the day is foreign', async () => {
+      const ownReservationId = Number(db.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'Mine', 'other')").run(tripId).lastInsertRowid);
+      const res = await request(server)
+        .put(`/api/trips/${tripId}/reservations/positions`)
+        .set('Cookie', sessionCookie(1))
+        .send({ positions: [{ id: ownReservationId, day_plan_position: 5 }], day_id: foreignDayId });
+      expect(res.status).toBe(200);
+      expect(positionRow(ownReservationId, foreignDayId)).toBeUndefined();
+    });
+
+    it('still stores a position when both ids are on the trip', async () => {
+      const ownReservationId = Number(db.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'Mine', 'other')").run(tripId).lastInsertRowid);
+      const res = await request(server)
+        .put(`/api/trips/${tripId}/reservations/positions`)
+        .set('Cookie', sessionCookie(1))
+        .send({ positions: [{ id: ownReservationId, day_plan_position: 3 }], day_id: ownDayId });
+      expect(res.status).toBe(200);
+      expect(positionRow(ownReservationId, ownDayId)).toEqual({ position: 3 });
+    });
+
+    // An id that exists nowhere used to raise a foreign-key error, which the
+    // exception filter turned into a 500 — telling the caller apart from the
+    // 200 a real id returns, for any id on the instance.
+    it('answers a nonexistent reservation the same way it answers a foreign one', async () => {
+      const res = await request(server)
+        .put(`/api/trips/${tripId}/reservations/positions`)
+        .set('Cookie', sessionCookie(1))
+        .send({ positions: [{ id: 999999, day_plan_position: 1 }], day_id: ownDayId });
+      expect(res.status).toBe(200);
+    });
+
+    it('does not fall over when day_plan_position is omitted', async () => {
+      const ownReservationId = Number(db.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'Mine', 'other')").run(tripId).lastInsertRowid);
+      const res = await request(server)
+        .put(`/api/trips/${tripId}/reservations/positions`)
+        .set('Cookie', sessionCookie(1))
+        .send({ positions: [{ id: ownReservationId }], day_id: ownDayId });
+      expect(res.status).toBe(200);
+    });
+  });
 });
diff --git a/server/tests/integration/leg-mode-incoming.test.ts b/server/tests/integration/leg-mode-incoming.test.ts
index 4fb2739f6..eabe889dd 100644
--- a/server/tests/integration/leg-mode-incoming.test.ts
+++ b/server/tests/integration/leg-mode-incoming.test.ts
@@ -41,24 +41,34 @@ describe('incoming_leg_transport_mode migration', () => {
     // skipped on every one of them. Undoing what the LAST migration did and
     // rewinding one version is the only way to prove it still runs.
     //
-    // >>> Appending a migration? Re-point the DROP below at the column yours
-    // >>> adds. That is the whole maintenance cost of this guard.
-    const TRAILING_TABLE = 'budget_items';
-    const TRAILING_COLUMN = 'place_id';
-
+    // >>> Appending a migration? Re-point the "undo" below at whatever yours
+    // >>> does. That is the whole maintenance cost of this guard. The current
+    // >>> trailing slot deletes reservation_day_positions rows whose reservation
+    // >>> and day sit on different trips, so undoing it means putting one back.
     const upgraded = new Database(':memory:');
     upgraded.exec('PRAGMA foreign_keys = ON');
     createTables(upgraded);
     runMigrations(upgraded);
 
     const { version } = upgraded.prepare('SELECT version FROM schema_version').get() as { version: number };
-    upgraded.exec(`ALTER TABLE ${TRAILING_TABLE} DROP COLUMN ${TRAILING_COLUMN}`);
+
+    upgraded.prepare("INSERT INTO users (id, username, email, password_hash) VALUES (1, 'a', 'a@test.local', 'x')").run();
+    upgraded.prepare("INSERT INTO users (id, username, email, password_hash) VALUES (2, 'b', 'b@test.local', 'x')").run();
+    const tripA = upgraded.prepare("INSERT INTO trips (user_id, title) VALUES (1, 'A')").run().lastInsertRowid;
+    const tripB = upgraded.prepare("INSERT INTO trips (user_id, title) VALUES (2, 'B')").run().lastInsertRowid;
+    const reservationOnA = upgraded.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'r', 'other')").run(tripA).lastInsertRowid;
+    const dayOnA = upgraded.prepare("INSERT INTO days (trip_id, day_number, date) VALUES (?, 1, '2026-01-01')").run(tripA).lastInsertRowid;
+    const dayOnB = upgraded.prepare("INSERT INTO days (trip_id, day_number, date) VALUES (?, 1, '2026-01-01')").run(tripB).lastInsertRowid;
+    const insertPosition = upgraded.prepare('INSERT INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)');
+    insertPosition.run(reservationOnA, dayOnB, 1); // the mismatched pair the migration clears
+    insertPosition.run(reservationOnA, dayOnA, 2); // a legitimate one it must leave alone
+
     upgraded.prepare('UPDATE schema_version SET version = ?').run(version - 1);
 
     runMigrations(upgraded);
 
-    const cols = upgraded.prepare(`PRAGMA table_info(${TRAILING_TABLE})`).all() as { name: string }[];
-    expect(cols.map(c => c.name)).toContain(TRAILING_COLUMN);
+    const rows = upgraded.prepare('SELECT day_id FROM reservation_day_positions ORDER BY day_id').all() as { day_id: number }[];
+    expect(rows).toEqual([{ day_id: Number(dayOnA) }]);
     expect(upgraded.prepare('SELECT version FROM schema_version').get()).toEqual({ version });
     upgraded.close();
   });
diff --git a/server/tests/unit/db/migration-hygiene.test.ts b/server/tests/unit/db/migration-hygiene.test.ts
index 358318260..44b4ecd88 100644
--- a/server/tests/unit/db/migration-hygiene.test.ts
+++ b/server/tests/unit/db/migration-hygiene.test.ts
@@ -135,6 +135,8 @@ const ALLOWED_DESTRUCTIVE: Record<string, string> = {
     'Atlas enclave fix: DELETE ... WHERE place_id IN (places inside specific enclave boxes) — invalidate stale region cache; re-resolved on next request.',
   'DELETE FROM visited_regions':
     'Atlas geoBoundaries swap (#1119): DELETE ... WHERE id = ? — after UPDATE OR IGNORE re-codes a manually-marked region to its current code, drop only the single leftover row whose UNIQUE(user_id, region_code) collision caused the update to be skipped (a duplicate of a region the user already has).',
+  'DELETE FROM reservation_day_positions':
+    'DELETE ... WHERE the row joins a reservation and a day sitting on different trips. The table has no trip_id and its two foreign keys only require the ids to exist, so a pair that never belonged together was storable; the writer scopes to the trip now and this clears what earlier builds allowed. Bounded by the join — a row whose reservation and day agree on their trip is untouched.',
 };
 
 describe('migration hygiene — destructive operation guard', () => {

From 0f0bf50cd339caa8d1544e2cd2d28979e3fe04c5 Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sun, 16 Aug 2026 21:37:36 +0200
Subject: [PATCH 3/9] fix(assignments): guard the per-assignment routes like
 the day routes

AssignmentOpsController declared @RequirePermission('day_edit') on its
mutations but carried only JwtAuthGuard. TripAccessGuard is what reads that
decorator and what resolves :tripId, so on this controller the permission
markers were metadata nobody looked at and the trip in the URL was never
matched against the caller. Its sibling, DayAssignmentsController, has had the
pair since the migration.

The two participants handlers also went straight to the service with the id
from the path, unlike move/time/transport, which ask getAssignmentForTrip
first. They ask now too.
---
 .../assignments/assignments.controller.ts     | 13 ++-
 server/tests/e2e/assignments.e2e.test.ts      | 88 +++++++++++++++++++
 .../unit/nest/assignments.controller.test.ts  | 14 ++-
 3 files changed, 110 insertions(+), 5 deletions(-)

diff --git a/server/src/nest/assignments/assignments.controller.ts b/server/src/nest/assignments/assignments.controller.ts
index d2c0dbecd..b076762de 100644
--- a/server/src/nest/assignments/assignments.controller.ts
+++ b/server/src/nest/assignments/assignments.controller.ts
@@ -113,9 +113,14 @@ export class DayAssignmentsController {
 /**
  * /api/trips/:tripId/assignments/:id/* — per-assignment ops (move, time,
  * participants), independent of the day path. Same parity rules as above.
+ *
+ * Same guard pair as the day controller, and for the same reason: TripAccessGuard
+ * is what resolves :tripId and what reads @RequirePermission. The per-handler
+ * getAssignmentForTrip calls answer a different question — whether the assignment
+ * sits on the trip in the URL, not whether the caller may be on that trip.
  */
 @Controller('api/trips/:tripId/assignments')
-@UseGuards(JwtAuthGuard)
+@UseGuards(JwtAuthGuard, TripAccessGuard)
 export class AssignmentOpsController {
   constructor(private readonly assignments: AssignmentsService) {}
 
@@ -142,6 +147,9 @@ export class AssignmentOpsController {
 
   @Get(':id/participants')
   participants(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string) {
+    if (!this.assignments.getAssignmentForTrip(id, tripId)) {
+      throw new HttpException({ error: 'Assignment not found' }, 404);
+    }
     return { participants: this.assignments.getParticipants(id) };
   }
 
@@ -191,6 +199,9 @@ export class AssignmentOpsController {
     @Body() body: AssignmentParticipantsDto,
     @Headers('x-socket-id') socketId?: string,
   ) {
+    if (!this.assignments.getAssignmentForTrip(id, tripId)) {
+      throw new HttpException({ error: 'Assignment not found' }, 404);
+    }
     const participants = this.assignments.setParticipants(id, body.user_ids);
     this.assignments.broadcast(tripId, 'assignment:participants', { assignmentId: Number(id), participants }, socketId);
     return { participants };
diff --git a/server/tests/e2e/assignments.e2e.test.ts b/server/tests/e2e/assignments.e2e.test.ts
index ccde14883..e6b5c1ba2 100644
--- a/server/tests/e2e/assignments.e2e.test.ts
+++ b/server/tests/e2e/assignments.e2e.test.ts
@@ -194,4 +194,92 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => {
     expect(res.status).toBe(200);
     expect(res.body.assignment).toMatchObject({ id, day_id: 4, order_index: 0 });
   });
+
+  // The per-assignment controller declared @RequirePermission but not the guard
+  // that reads it, so the decorators were inert metadata and :tripId was never
+  // checked against the caller at all. The handlers that ask
+  // getAssignmentForTrip(id, tripId) were happy as long as the assignment sat on
+  // the trip in the URL — which is true for the owner's trip too.
+  describe('a trip the caller cannot see', () => {
+    const FOREIGN_TRIP = 9;
+
+    beforeEach(() => {
+      canAccessTrip.mockImplementation((tripId: unknown) => (Number(tripId) === 5 ? { id: 5, user_id: 1 } : undefined));
+      db.prepare('INSERT OR IGNORE INTO days (id, trip_id) VALUES (30, ?), (31, ?)').run(FOREIGN_TRIP, FOREIGN_TRIP);
+      db.prepare('INSERT OR IGNORE INTO places (id, trip_id, name) VALUES (20, ?, ?)').run(FOREIGN_TRIP, 'Their hotel');
+    });
+
+    const seedForeignAssignment = () =>
+      Number(db.prepare('INSERT INTO day_assignments (day_id, place_id, order_index) VALUES (30, 20, 0)').run().lastInsertRowid);
+
+    it('404s a move instead of reordering their itinerary', async () => {
+      const id = seedForeignAssignment();
+      const res = await request(server)
+        .put(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/move`)
+        .set('Cookie', sessionCookie(1))
+        .send({ new_day_id: 31, order_index: 0 });
+      expect(res.status).toBe(404);
+      expect(db.prepare('SELECT day_id FROM day_assignments WHERE id = ?').get(id)).toEqual({ day_id: 30 });
+    });
+
+    it('404s a time change instead of rewriting their schedule', async () => {
+      const id = seedForeignAssignment();
+      const res = await request(server)
+        .put(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/time`)
+        .set('Cookie', sessionCookie(1))
+        .send({ place_time: '23:00', end_time: null });
+      expect(res.status).toBe(404);
+      expect(db.prepare('SELECT assignment_time FROM day_assignments WHERE id = ?').get(id)).toEqual({ assignment_time: null });
+    });
+
+    it('404s a transport change', async () => {
+      const id = seedForeignAssignment();
+      const res = await request(server)
+        .put(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/transport`)
+        .set('Cookie', sessionCookie(1))
+        .send({ transport_mode: 'driving' });
+      expect(res.status).toBe(404);
+    });
+
+    it('404s setting participants instead of writing to their assignment', async () => {
+      const id = seedForeignAssignment();
+      const res = await request(server)
+        .put(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/participants`)
+        .set('Cookie', sessionCookie(1))
+        .send({ user_ids: [1] });
+      expect(res.status).toBe(404);
+      expect(db.prepare('SELECT COUNT(*) AS n FROM assignment_participants WHERE assignment_id = ?').get(id)).toEqual({ n: 0 });
+    });
+
+    it('404s reading participants instead of disclosing who is on it', async () => {
+      const id = seedForeignAssignment();
+      db.prepare('INSERT INTO assignment_participants (assignment_id, user_id) VALUES (?, 2)').run(id);
+      const res = await request(server)
+        .get(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/participants`)
+        .set('Cookie', sessionCookie(1));
+      expect(res.status).toBe(404);
+    });
+
+    // The guard has to reject an assignment borrowed from elsewhere even when the
+    // caller is legitimately on the trip named in the URL.
+    it('404s an assignment that belongs to another trip than the URL says', async () => {
+      const id = seedForeignAssignment();
+      const res = await request(server)
+        .put(`/api/trips/5/assignments/${id}/participants`)
+        .set('Cookie', sessionCookie(1))
+        .send({ user_ids: [1] });
+      expect(res.status).toBe(404);
+      expect(db.prepare('SELECT COUNT(*) AS n FROM assignment_participants WHERE assignment_id = ?').get(id)).toEqual({ n: 0 });
+    });
+
+    it('403s when the caller is on the trip but lacks day_edit', async () => {
+      const id = seedAssignment();
+      checkPermission.mockReturnValue(false);
+      const res = await request(server)
+        .put(`/api/trips/5/assignments/${id}/time`)
+        .set('Cookie', sessionCookie(1))
+        .send({ place_time: '09:00', end_time: null });
+      expect(res.status).toBe(403);
+    });
+  });
 });
diff --git a/server/tests/unit/nest/assignments.controller.test.ts b/server/tests/unit/nest/assignments.controller.test.ts
index e032dfa14..2079addef 100644
--- a/server/tests/unit/nest/assignments.controller.test.ts
+++ b/server/tests/unit/nest/assignments.controller.test.ts
@@ -78,8 +78,10 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', (
     expect(reconcile).toHaveBeenCalledWith('5', 'sock');
   });
 
-  it('GET /:id/participants returns participants (access-only)', () => {
-    const s = svc({ getParticipants: vi.fn().mockReturnValue([{ user_id: 2 }]) } as Partial<AssignmentsService>);
+  it('GET /:id/participants 404 when the assignment is on another trip, else returns participants (access-only)', () => {
+    expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined) } as Partial<AssignmentsService>)).participants(user, '5', '9')))
+      .toEqual({ status: 404, body: { error: 'Assignment not found' } });
+    const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), getParticipants: vi.fn().mockReturnValue([{ user_id: 2 }]) } as Partial<AssignmentsService>);
     expect(new AssignmentOpsController(s).participants(user, '5', '9')).toEqual({ participants: [{ user_id: 2 }] });
   });
 
@@ -92,9 +94,13 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', (
     expect(reconcile).toHaveBeenCalledWith('5', 'sock');
   });
 
-  it('PUT /:id/participants sets + broadcasts (non-array bodies are the Zod pipe\'s 400, covered in e2e)', () => {
+  it('PUT /:id/participants 404 on a foreign assignment, else sets + broadcasts (non-array bodies are the Zod pipe\'s 400, covered in e2e)', () => {
     const setParticipants = vi.fn().mockReturnValue([{ user_id: 2 }]); const broadcast = vi.fn();
-    expect(new AssignmentOpsController(svc({ setParticipants, broadcast } as Partial<AssignmentsService>)).setParticipants(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ participants: [{ user_id: 2 }] });
+    expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined), setParticipants } as Partial<AssignmentsService>)).setParticipants(user, '5', '9', { user_ids: [2] })))
+      .toEqual({ status: 404, body: { error: 'Assignment not found' } });
+    expect(setParticipants).not.toHaveBeenCalled();
+    const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), setParticipants, broadcast } as Partial<AssignmentsService>);
+    expect(new AssignmentOpsController(s).setParticipants(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ participants: [{ user_id: 2 }] });
     expect(setParticipants).toHaveBeenCalledWith('9', [2]);
     expect(broadcast).toHaveBeenCalledWith('5', 'assignment:participants', { assignmentId: 9, participants: [{ user_id: 2 }] }, 'sock');
   });

From c8607ba2348bca0807e504d4fd5202590f6fa531 Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sun, 16 Aug 2026 21:37:52 +0200
Subject: [PATCH 4/9] fix(places): pin the image and website URL fields on the
 way in

image_url and website were bare strings in the contract, and the create/update
bodies are open records, so the Zod pipe never looked at either. Both leave the
database for something that treats them as a URL: the thumbnail is interpolated
into hand-built marker HTML, and the homepage is handed to window.open, where a
javascript: value would run in this origin rather than opening a page.

Both are allow-listed on the way in now, next to hexColorSchema and checked in
the handler the way validateRouteColor already is, on all three write surfaces
so REST, the plugin RPC and MCP agree on what they accept. The entity schemas
stay open: rows written before this still have to round-trip.

safeHttpUrl covers the read side for exactly those rows, the same split
safeHexColor uses, and the window.open calls it guards pick up
noopener,noreferrer while they are being touched.
---
 .../Planner/DayPlanSidebar.test.tsx           |  2 +-
 .../src/components/Planner/DayPlanSidebar.tsx |  3 +-
 .../Planner/PlaceInspector.test.tsx           |  2 +-
 .../src/components/Planner/PlaceInspector.tsx |  9 +++-
 .../Planner/usePlacesSidebar.test.tsx         |  2 +-
 .../components/Planner/usePlacesSidebar.ts    |  3 +-
 .../screens/trip/sheets/MPlaceSheet.tsx       |  5 +-
 client/src/utils/safeUrl.test.ts              | 35 ++++++++++++
 client/src/utils/safeUrl.ts                   | 18 +++++++
 server/src/nest/places/places.controller.ts   | 23 +++++++-
 server/src/nest/places/places.mcp.ts          |  9 ++--
 server/src/nest/places/places.rpc.ts          | 21 +++++++-
 .../tests/unit/nest/places.controller.test.ts | 53 +++++++++++++++++++
 shared/src/collection/collection.schema.ts    | 11 ++--
 shared/src/place/place.schema.ts              | 28 ++++++++++
 15 files changed, 205 insertions(+), 19 deletions(-)
 create mode 100644 client/src/utils/safeUrl.test.ts
 create mode 100644 client/src/utils/safeUrl.ts

diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx
index 088882147..f0c7fb10f 100644
--- a/client/src/components/Planner/DayPlanSidebar.test.tsx
+++ b/client/src/components/Planner/DayPlanSidebar.test.tsx
@@ -3332,7 +3332,7 @@ describe('DayPlanSidebar', () => {
     render(<DayPlanSidebar {...makeDefaultProps({ days: [day], places: [place], assignments: { '10': [a] } })} />)
     fireEvent.contextMenu(dragRow(screen.getByText('Louvre')))
     await user.click(screen.getByRole('button', { name: 'Open Website' }))
-    expect(openSpy).toHaveBeenCalledWith('https://louvre.fr', '_blank')
+    expect(openSpy).toHaveBeenCalledWith('https://louvre.fr', '_blank', 'noopener,noreferrer')
 
     fireEvent.contextMenu(dragRow(screen.getByText('Louvre')))
     await user.click(screen.getByRole('button', { name: 'Google Maps' }))
diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx
index 3845ce953..e120c0c06 100644
--- a/client/src/components/Planner/DayPlanSidebar.tsx
+++ b/client/src/components/Planner/DayPlanSidebar.tsx
@@ -4,6 +4,7 @@ declare global { interface Window { __dragData: DragDataPayload | null } }
 
 import React, { useState, useEffect, useLayoutEffect, useRef, useMemo, useCallback } from 'react'
 import { avatarSrc } from '../../utils/avatarSrc'
+import { safeHttpUrl } from '../../utils/safeUrl'
 import { ChevronDown, ChevronRight, ChevronUp, Compass, Navigation, RotateCcw, ExternalLink, Clock, Pencil, GripVertical, Ticket, Plus, FileText, Trash2, Car, Lock, Hotel, Footprints, Route as RouteIcon, Bookmark, TramFront, Zap } from 'lucide-react'
 import { type PickedPlace } from './TransitSearchPanel'
 import { assignmentsApi, reservationsApi, daysApi } from '../../api/client'
@@ -1871,7 +1872,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
                               ctxMenu.open(e, [
                                 canEditDays && onEditPlace && { label: t('common.edit'), icon: Pencil, onClick: () => onEditPlace(place, assignment.id) },
                                 canEditDays && onRemoveAssignment && { label: t('planner.removeFromDay'), icon: Trash2, onClick: () => onRemoveAssignment(day.id, assignment.id) },
-                                place.website && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(place.website, '_blank') },
+                                safeHttpUrl(place.website) && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer') },
                                 ...navTargets.map(target => ({ label: target.label, icon: Navigation, onClick: () => openNavigationTarget(target) })),
                                 collectionsEnabled && { label: t('inspector.saveToCollection'), icon: Bookmark, onClick: () => useSaveToCollectionStore.getState().open(placeToSaveTarget(place)) },
                                 { divider: true },
diff --git a/client/src/components/Planner/PlaceInspector.test.tsx b/client/src/components/Planner/PlaceInspector.test.tsx
index 0dcf4cc0a..2288cb010 100644
--- a/client/src/components/Planner/PlaceInspector.test.tsx
+++ b/client/src/components/Planner/PlaceInspector.test.tsx
@@ -953,7 +953,7 @@ describe('PlaceInspector', () => {
 
     fireEvent.click(screen.getByText('Open Website').closest('button')!);
     expect(open).toHaveBeenCalledTimes(2);
-    expect(open.mock.calls[1]).toEqual(['https://example.org', '_blank']);
+    expect(open.mock.calls[1]).toEqual(['https://example.org', '_blank', 'noopener,noreferrer']);
     vi.unstubAllGlobals();
   });
 
diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx
index a4ca8d53b..e89ab380f 100644
--- a/client/src/components/Planner/PlaceInspector.tsx
+++ b/client/src/components/Planner/PlaceInspector.tsx
@@ -1,5 +1,6 @@
 import React, { useState, useEffect, useRef, useCallback, useMemo } from 'react'
 import { avatarSrc } from '../../utils/avatarSrc'
+import { safeHttpUrl } from '../../utils/safeUrl'
 import { openFile } from '../../utils/fileDownload'
 import Markdown from 'react-markdown'
 import remarkGfm from 'remark-gfm'
@@ -318,6 +319,10 @@ export default function PlaceInspector({
     detailLng,
     googleDetails?.open_now,
   )
+  // Allow-listed rather than passed straight through: window.open runs a
+  // javascript: URL in this origin, and the stored value predates the check the
+  // server does on the way in now.
+  const websiteUrl = safeHttpUrl(place.website) ?? safeHttpUrl(googleDetails?.website)
   // Prefer the place's stored ftid; if it has none yet, use the one just fetched from Google.
   const navigationTargets = getNavigationTargets(
     place ? { ...place, google_ftid: place.google_ftid || googleDetails?.google_ftid || null } : null,
@@ -513,8 +518,8 @@ export default function PlaceInspector({
               )}
             </>
           )}
-          {(place.website || googleDetails?.website) && (
-            <ActionButton onClick={() => window.open(place.website || googleDetails?.website, '_blank')} variant="ghost" icon={<ExternalLink size={13} />}
+          {websiteUrl && (
+            <ActionButton onClick={() => window.open(websiteUrl, '_blank', 'noopener,noreferrer')} variant="ghost" icon={<ExternalLink size={13} />}
               label={<span className="hidden sm:inline">{t('inspector.website')}</span>} />
           )}
           <div style={{ flex: 1 }} />
diff --git a/client/src/components/Planner/usePlacesSidebar.test.tsx b/client/src/components/Planner/usePlacesSidebar.test.tsx
index 1d79a4da5..03918897b 100644
--- a/client/src/components/Planner/usePlacesSidebar.test.tsx
+++ b/client/src/components/Planner/usePlacesSidebar.test.tsx
@@ -643,7 +643,7 @@ describe('usePlacesSidebar context menu', () => {
 
     fireEvent.contextMenu(screen.getByTestId('row-3'));
     await user.click(screen.getByRole('button', { name: 'Open Website' }));
-    expect(open).toHaveBeenCalledWith('https://cafe.example', '_blank');
+    expect(open).toHaveBeenCalledWith('https://cafe.example', '_blank', 'noopener,noreferrer');
 
     fireEvent.contextMenu(screen.getByTestId('row-3'));
     await user.click(screen.getByRole('button', { name: 'Google Maps' }));
diff --git a/client/src/components/Planner/usePlacesSidebar.ts b/client/src/components/Planner/usePlacesSidebar.ts
index 40b34b4d2..aab094a62 100644
--- a/client/src/components/Planner/usePlacesSidebar.ts
+++ b/client/src/components/Planner/usePlacesSidebar.ts
@@ -14,6 +14,7 @@ import { useSaveToCollectionStore } from '../../store/saveToCollectionStore'
 import { placeToSaveTarget } from '../Collections/saveTarget'
 import type { Place, Category, Day, AssignmentsMap } from '../../types'
 import { getGoogleMapsUrlForPlace } from './placeGoogleMaps'
+import { safeHttpUrl } from '../../utils/safeUrl'
 
 export interface PlacesSidebarProps {
   tripId: number
@@ -276,7 +277,7 @@ export function usePlacesSidebar(props: PlacesSidebarProps) {
     ctxMenu.open(e, [
       canEditPlaces && { label: t('common.edit'), icon: Pencil, onClick: () => props.onEditPlace(place) },
       selDayId && { label: t('planner.addToDay'), icon: CalendarDays, onClick: () => props.onAssignToDay(place.id, selDayId) },
-      place.website && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(place.website, '_blank') },
+      safeHttpUrl(place.website) && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer') },
       googleMapsUrl && { label: t('inspector.google'), icon: Navigation, onClick: () => window.open(googleMapsUrl, '_blank') },
       collectionsEnabled && { label: t('inspector.saveToCollection'), icon: Bookmark, onClick: () => useSaveToCollectionStore.getState().open(placeToSaveTarget(place)) },
       { divider: true },
diff --git a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx
index 544a70b3b..d7e0360e5 100644
--- a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx
+++ b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx
@@ -17,6 +17,7 @@ import PlaceRating from '../../../../components/shared/StarRating'
 import TrackColorPicker from '../../../../components/shared/TrackColorPicker'
 import { resolveTrackColor, inheritedTrackColor } from '../../../../components/Map/trackColors'
 import { avatarSrc } from '../../../../utils/avatarSrc'
+import { safeHttpUrl } from '../../../../utils/safeUrl'
 import { openFile } from '../../../../utils/fileDownload'
 import { getNavigationTargets, openNavigationTarget } from '../../../../components/Planner/placeNavigation'
 import { NavigationMenu } from '../../../../components/shared/NavigationMenu'
@@ -527,9 +528,9 @@ export default function MPlaceSheet({ planner, shell }: MTripSheetsProps) {
                   <MapIcon size={15} strokeWidth={2} />
                 </ActionCircle>
               )}
-              {place.website && (
+              {safeHttpUrl(place.website) && (
                 <ActionCircle
-                  onClick={() => window.open(place.website!, '_blank', 'noopener,noreferrer')}
+                  onClick={() => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer')}
                   label={t('inspector.website')}
                 >
                   <ExternalLink size={15} strokeWidth={2} />
diff --git a/client/src/utils/safeUrl.test.ts b/client/src/utils/safeUrl.test.ts
new file mode 100644
index 000000000..f908a67b1
--- /dev/null
+++ b/client/src/utils/safeUrl.test.ts
@@ -0,0 +1,35 @@
+import { describe, it, expect } from 'vitest'
+import { safeHttpUrl } from './safeUrl'
+
+describe('safeHttpUrl', () => {
+  it('FE-UTIL-SAFEURL-001: passes an ordinary https homepage through', () => {
+    expect(safeHttpUrl('https://louvre.fr/en/visit')).toBe('https://louvre.fr/en/visit')
+  })
+
+  it('FE-UTIL-SAFEURL-002: passes plain http through — plenty of small venues still run it', () => {
+    expect(safeHttpUrl('http://pension-alpenblick.at')).toBe('http://pension-alpenblick.at')
+  })
+
+  it('FE-UTIL-SAFEURL-003: rejects javascript:, which window.open would run in this origin', () => {
+    expect(safeHttpUrl('javascript:fetch("/api/trips")')).toBeNull()
+    expect(safeHttpUrl('JavaScript:alert(1)')).toBeNull()
+    expect(safeHttpUrl('  javascript:alert(1)')).toBeNull()
+  })
+
+  it('FE-UTIL-SAFEURL-004: rejects the other schemes that are not a web page', () => {
+    expect(safeHttpUrl('data:text/html,<script>alert(1)</script>')).toBeNull()
+    expect(safeHttpUrl('vbscript:msgbox(1)')).toBeNull()
+    expect(safeHttpUrl('file:///etc/passwd')).toBeNull()
+  })
+
+  it('FE-UTIL-SAFEURL-005: rejects a bare host, since window.open would treat it as a relative path', () => {
+    expect(safeHttpUrl('louvre.fr')).toBeNull()
+    expect(safeHttpUrl('//evil.example')).toBeNull()
+  })
+
+  it('FE-UTIL-SAFEURL-006: treats absent and empty as nothing to open', () => {
+    expect(safeHttpUrl(null)).toBeNull()
+    expect(safeHttpUrl(undefined)).toBeNull()
+    expect(safeHttpUrl('')).toBeNull()
+  })
+})
diff --git a/client/src/utils/safeUrl.ts b/client/src/utils/safeUrl.ts
new file mode 100644
index 000000000..a89390fa1
--- /dev/null
+++ b/client/src/utils/safeUrl.ts
@@ -0,0 +1,18 @@
+import { placeWebsiteSchema } from '@trek/shared'
+
+/**
+ * Allow-list a URL before handing it to window.open.
+ *
+ * A place's website is a plain string in the older contracts, and it reaches
+ * window.open from the inspector, both sidebar context menus and the mobile
+ * place sheet. `window.open('javascript:…')` does not open a page — it evaluates
+ * the script in a fresh document that inherits this origin, so a co-traveller
+ * with place_edit could plant one and wait for someone to click Website.
+ *
+ * Server-side validation closes the write path. This closes the read path, which
+ * is what covers rows written before that validation existed and instances that
+ * have not updated yet — the same split as safeHexColor.
+ */
+export function safeHttpUrl(value: string | null | undefined): string | null {
+  return typeof value === 'string' && placeWebsiteSchema.safeParse(value).success ? value : null
+}
diff --git a/server/src/nest/places/places.controller.ts b/server/src/nest/places/places.controller.ts
index 9d13543db..75f875fa2 100644
--- a/server/src/nest/places/places.controller.ts
+++ b/server/src/nest/places/places.controller.ts
@@ -20,7 +20,7 @@ import type { Response } from 'express';
 import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../common/demo-write';
 import { RuntimeEnvService } from '../app-config/runtime-env.service';
 import { memoryStorage } from 'multer';
-import { hexColorSchema } from '@trek/shared';
+import { hexColorSchema, placeImageUrlSchema, placeWebsiteSchema } from '@trek/shared';
 import type { User } from '../../types';
 import { PlacesService } from './places.service';
 import { isUpdateConflict } from '../common/conflictResult';
@@ -63,6 +63,25 @@ function validateRouteColor(body: Record<string, unknown>): void {
   }
 }
 
+// Same reason as route_color: these two leave the database for a renderer that
+// treats them as a URL — the thumbnail into marker HTML, the homepage into
+// window.open — and the open record means the pipe never sees them.
+function validateUrlFields(body: Record<string, unknown>): void {
+  const image = body.image_url;
+  if (image !== undefined && image !== null) {
+    if (typeof image !== 'string' || !placeImageUrlSchema.safeParse(image).success) {
+      throw new HttpException({ error: 'image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL' }, 400);
+    }
+  }
+  const website = body.website;
+  // '' is how the UI clears the field; treat it like an absent value.
+  if (website !== undefined && website !== null && website !== '') {
+    if (typeof website !== 'string' || !placeWebsiteSchema.safeParse(website).success) {
+      throw new HttpException({ error: 'website must be an http or https URL' }, 400);
+    }
+  }
+}
+
 function parseBool(v: unknown, defaultVal: boolean): boolean {
   return v === undefined || v === null ? defaultVal : String(v) === 'true';
 }
@@ -131,6 +150,7 @@ export class PlacesController {
     const trip = this.requireTrip(tripId, user);
     validateLengths(body);
     validateRouteColor(body);
+    validateUrlFields(body);
     this.requireEdit(trip, user);
     const place = this.places.create(tripId, body as never);
     this.places.broadcast(tripId, 'place:created', { place }, socketId);
@@ -428,6 +448,7 @@ export class PlacesController {
     const trip = this.requireTrip(tripId, user);
     validateLengths(body);
     validateRouteColor(body);
+    validateUrlFields(body);
     this.requireEdit(trip, user);
     const result = this.places.update(tripId, id, body as never, ifMatch);
     if (!result) {
diff --git a/server/src/nest/places/places.mcp.ts b/server/src/nest/places/places.mcp.ts
index 39c88127f..ac95a64cc 100644
--- a/server/src/nest/places/places.mcp.ts
+++ b/server/src/nest/places/places.mcp.ts
@@ -5,6 +5,7 @@ import {
   demoDenied, ok,
 } from '@trek/nest-mcp';
 import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service';
+import { placeWebsiteSchema } from '@trek/shared';
 import { z } from 'zod';
 import { AuthService } from '../auth/auth.service';
 import { AssignmentsService } from '../assignments/assignments.service';
@@ -63,7 +64,7 @@ export class PlacesMcp {
       google_ftid: z.string().optional().describe('Google Maps feature ID from search_place — enables direct Google Maps links'),
       osm_id: z.string().optional().describe('OpenStreetMap ID from search_place (e.g. "way:12345") — enables opening hours if no Google ID'),
       notes: z.string().max(2000).optional(),
-      website: z.string().max(500).optional(),
+      website: placeWebsiteSchema.optional(),
       phone: z.string().max(50).optional(),
       price: z.number().nonnegative().optional().describe('Cost of this place/activity (e.g. ticket price, entry fee)'),
       currency: z.string().length(3).optional().describe('ISO 4217 currency code (e.g. "EUR", "USD")'),
@@ -103,7 +104,7 @@ export class PlacesMcp {
       google_ftid: z.string().optional().describe('Google Maps feature ID from search_place — enables direct Google Maps links'),
       osm_id: z.string().optional().describe('OpenStreetMap ID from search_place (e.g. "way:12345")'),
       place_notes: z.string().max(2000).optional().describe('Notes for the place'),
-      website: z.string().max(500).optional(),
+      website: placeWebsiteSchema.optional(),
       phone: z.string().max(50).optional(),
       assignment_notes: z.string().max(500).optional().describe('Notes for this day assignment'),
       price: z.number().nonnegative().optional().describe('Cost of this place/activity (e.g. ticket price, entry fee)'),
@@ -158,7 +159,7 @@ export class PlacesMcp {
       end_time: z.string().max(50).optional().describe('End time (e.g. "11:00")'),
       duration_minutes: z.number().int().positive().optional(),
       notes: z.string().max(2000).optional(),
-      website: z.string().max(500).optional(),
+      website: placeWebsiteSchema.optional(),
       phone: z.string().max(50).optional(),
       transport_mode: z.enum(['walking', 'driving', 'cycling', 'transit', 'flight']).optional(),
       osm_id: z.string().optional().describe('OpenStreetMap ID (e.g. "way:12345")'),
@@ -366,7 +367,7 @@ export class PlacesMcp {
       end_time: z.string().max(50).optional().describe('End time (e.g. "11:00")'),
       duration_minutes: z.number().int().positive().optional(),
       notes: z.string().max(2000).optional(),
-      website: z.string().max(500).optional(),
+      website: placeWebsiteSchema.optional(),
       phone: z.string().max(50).optional(),
       description: z.string().max(2000).optional(),
     },
diff --git a/server/src/nest/places/places.rpc.ts b/server/src/nest/places/places.rpc.ts
index 3f61da812..91f20afa6 100644
--- a/server/src/nest/places/places.rpc.ts
+++ b/server/src/nest/places/places.rpc.ts
@@ -1,4 +1,4 @@
-import { placeCreateRequestSchema, placeUpdateRequestSchema } from '@trek/shared';
+import { placeCreateRequestSchema, placeImageUrlSchema, placeUpdateRequestSchema, placeWebsiteSchema } from '@trek/shared';
 import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators';
 import { PluginGuards } from '../plugins/host/plugin-guards.service';
 import { BadParams, ForbiddenResource } from '../plugins/host/rpc-errors';
@@ -18,6 +18,23 @@ const PLACE_EDIT_ACTION = 'place_edit';
  */
 const PLACE_STR_LIMITS: Record<string, number> = { name: 200, description: 2000, address: 500, notes: 2000 };
 
+/**
+ * The two URL fields the REST controller pins in validateUrlFields. Same reason
+ * they are checked there rather than in the contract: the write schemas are open
+ * records, so a plugin could otherwise store a thumbnail or a homepage the web
+ * app refuses — and both end up somewhere that treats them as a URL.
+ */
+function capUrls(input: Record<string, unknown>): void {
+  const image = input.image_url;
+  if (image !== undefined && image !== null && !placeImageUrlSchema.safeParse(image).success) {
+    throw new BadParams('invalid place: image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL');
+  }
+  const website = input.website;
+  if (website !== undefined && website !== null && website !== '' && !placeWebsiteSchema.safeParse(website).success) {
+    throw new BadParams('invalid place: website must be an http or https URL');
+  }
+}
+
 /**
  * The place surface a plugin may reach (#plugins).
  *
@@ -41,6 +58,7 @@ export class PlacesRpc {
     const parsed = placeCreateRequestSchema.safeParse(params.input);
     if (!parsed.success) throw new BadParams(`invalid place: ${schemaMessage(parsed.error)}`);
     this.guards.capStrings(parsed.data as Record<string, unknown>, PLACE_STR_LIMITS);
+    capUrls(parsed.data as Record<string, unknown>);
     this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION);
     const place = this.places.create(String(tripId), parsed.data as unknown as PlaceCreateInput);
     this.realtime.broadcast(tripId, 'place:created', { place });
@@ -56,6 +74,7 @@ export class PlacesRpc {
     const parsed = placeUpdateRequestSchema.safeParse(params.input);
     if (!parsed.success) throw new BadParams(`invalid place: ${schemaMessage(parsed.error)}`);
     this.guards.capStrings(parsed.data as Record<string, unknown>, PLACE_STR_LIMITS);
+    capUrls(parsed.data as Record<string, unknown>);
     this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION);
     const place = this.places.update(String(tripId), String(placeId), parsed.data as PlaceUpdateInput);
     if (place === null) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`);
diff --git a/server/tests/unit/nest/places.controller.test.ts b/server/tests/unit/nest/places.controller.test.ts
index 12f4af731..71cfcae9b 100644
--- a/server/tests/unit/nest/places.controller.test.ts
+++ b/server/tests/unit/nest/places.controller.test.ts
@@ -301,6 +301,59 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou
     });
   });
 
+  // image_url and website leave the database for something that treats them as a
+  // URL — the thumbnail into hand-built marker HTML, the homepage into
+  // window.open. The write body is an open record, so the Zod pipe never sees
+  // either one; they are checked here for the same reason route_color is.
+  describe('image_url and website', () => {
+    const imageErr = { status: 400, body: { error: 'image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL' } };
+    const siteErr = { status: 400, body: { error: 'website must be an http or https URL' } };
+    const ctl = (over: Partial<PlacesService> = {}) => new PlacesController(svc(over), new RuntimeEnvService());
+
+    it('400s an image_url the marker builders were never meant to receive, before the permission check', () => {
+      const canEdit = vi.fn().mockReturnValue(false); // would 403 if it got that far
+      for (const image_url of [
+        'javascript:alert(1)',
+        'data:text/html,<script>alert(1)</script>',
+        'http://insecure.example/p.png',
+        '//evil.example/p.png',
+        123,
+      ]) {
+        expect(thrown(() => ctl({ canEdit }).update(user, '5', '9', { image_url }))).toEqual(imageErr);
+      }
+      expect(thrown(() => ctl({ canEdit }).create(user, '5', { name: 'T', image_url: 'javascript:alert(1)' }))).toEqual(imageErr);
+      expect(canEdit).not.toHaveBeenCalled();
+    });
+
+    it('keeps accepting every shape the app actually stores', () => {
+      const update = vi.fn().mockReturnValue({ id: 9 });
+      for (const image_url of [
+        '/uploads/places/eiffel.jpg',
+        '/api/maps/place-photo/abc123',
+        'data:image/jpeg;base64,/9j/4AAQSkZJRg==',
+        'https://images.example/photo.jpg',
+        null,
+      ]) {
+        expect(ctl({ update } as Partial<PlacesService>).update(user, '5', '9', { image_url })).toEqual({ place: { id: 9 } });
+      }
+    });
+
+    it('400s a website that window.open would not treat as a page', () => {
+      const canEdit = vi.fn().mockReturnValue(false);
+      for (const website of ['javascript:fetch("/api/trips")', 'data:text/html,x', 'louvre.fr', 42]) {
+        expect(thrown(() => ctl({ canEdit }).update(user, '5', '9', { website }))).toEqual(siteErr);
+      }
+      expect(canEdit).not.toHaveBeenCalled();
+    });
+
+    it('accepts http and https, and treats the empty string as clearing the field', () => {
+      const update = vi.fn().mockReturnValue({ id: 9 });
+      for (const website of ['https://louvre.fr', 'http://pension.at', '', null]) {
+        expect(ctl({ update } as Partial<PlacesService>).update(user, '5', '9', { website })).toEqual({ place: { id: 9 } });
+      }
+    });
+  });
+
   it('PUT /:id forwards the base-version token and 409s on a conflict (#1135)', () => {
     const update = vi.fn().mockReturnValue({ conflict: true, server: { id: 9, name: 'Theirs' } });
     const onUpdated = vi.fn(); const broadcast = vi.fn();
diff --git a/shared/src/collection/collection.schema.ts b/shared/src/collection/collection.schema.ts
index 894fef266..d5379d5fe 100644
--- a/shared/src/collection/collection.schema.ts
+++ b/shared/src/collection/collection.schema.ts
@@ -1,4 +1,4 @@
-import { placeCategorySchema, placeRatingVoteSchema } from '../place/place.schema';
+import { placeCategorySchema, placeImageUrlSchema, placeRatingVoteSchema, placeWebsiteSchema } from '../place/place.schema';
 import { tagSchema } from '../tag/tag.schema';
 
 import { z } from 'zod';
@@ -53,6 +53,9 @@ export const collectionPlaceSchema = z.object({
   price: z.number().nullable().optional(),
   currency: z.string().nullable().optional(),
   notes: z.string().nullable().optional(),
+  // Deliberately open on the way out, pinned on the way in (see the save/update
+  // request schemas below): rows written before the request-side check existed
+  // still have to round-trip.
   image_url: z.string().nullable().optional(),
   google_place_id: z.string().nullable().optional(),
   google_ftid: z.string().nullable().optional(),
@@ -150,11 +153,11 @@ export const collectionSavePlaceRequestSchema = z.object({
   price: z.number().nullable().optional(),
   currency: z.string().nullable().optional(),
   notes: z.string().nullable().optional(),
-  image_url: z.string().nullable().optional(),
+  image_url: placeImageUrlSchema.nullable().optional(),
   google_place_id: z.string().nullable().optional(),
   google_ftid: z.string().nullable().optional(),
   osm_id: z.string().nullable().optional(),
-  website: z.string().nullable().optional(),
+  website: placeWebsiteSchema.nullable().optional(),
   phone: z.string().nullable().optional(),
   status: collectionStatusSchema.optional(),
   links: collectionLinksSchema.optional(),
@@ -203,7 +206,7 @@ export const collectionPlaceUpdateRequestSchema = z.object({
   // Replace the place's per-collection label assignments (omit to leave unchanged).
   label_ids: z.array(z.number()).optional(),
   // Custom thumbnail (#1136): null clears it (falls back to the auto-fetched photo).
-  image_url: z.string().nullable().optional(),
+  image_url: placeImageUrlSchema.nullable().optional(),
 });
 export type CollectionPlaceUpdateRequest = z.infer<typeof collectionPlaceUpdateRequestSchema>;
 
diff --git a/shared/src/place/place.schema.ts b/shared/src/place/place.schema.ts
index 9c3fa5379..d0d63da28 100644
--- a/shared/src/place/place.schema.ts
+++ b/shared/src/place/place.schema.ts
@@ -19,6 +19,34 @@ const open = z.record(z.string(), z.unknown());
 /** `#rgb` / `#rrggbb`, the form both map renderers and CSS accept. */
 export const hexColorSchema = z.string().regex(/^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/);
 
+/**
+ * The four shapes a place thumbnail is ever allowed to take: a file we stored, a
+ * photo-proxy path, an inline thumbnail, or a remote https image.
+ *
+ * The map markers build their HTML as a string, so what lands here is one input
+ * away from a renderer that forgets to escape. Pinning the scheme on the way in
+ * means a stored value cannot carry anything a future consumer has to defend
+ * against — the escaping in the marker builders stops being the only thing
+ * standing between the database and the DOM.
+ */
+export const placeImageUrlSchema = z.string().max(2048).refine(
+  v =>
+    v.startsWith('/uploads/')
+    || v.startsWith('/api/maps/place-photo/')
+    || /^data:image\/(png|jpe?g|webp|gif|avif);base64,/i.test(v)
+    || /^https:\/\//i.test(v),
+  { message: 'must be an uploaded path, a photo-proxy path, an inline image or an https URL' },
+);
+
+/**
+ * A place's homepage. It reaches window.open() on the client, where a
+ * javascript: value would run in this origin rather than opening a page.
+ */
+export const placeWebsiteSchema = z.string().max(500).refine(
+  v => /^https?:\/\//i.test(v),
+  { message: 'must be an http or https URL' },
+);
+
 /**
  * Embedded category as returned on a place — a trimmed projection of the
  * categories row (id/name/color/icon), built inline by placeService and

From 5bc041cc774dff2e5c2d05d4875e2a8609d2cbd3 Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sun, 16 Aug 2026 21:38:09 +0200
Subject: [PATCH 5/9] fix(journey): escape the tooltip text on the Leaflet maps

A string handed to bindTooltip becomes innerHTML. JourneyMap passed an entry
title and a track name through untouched; the GL renderer beside it escapes
both already, and JourneyPublicPage imports the Leaflet one directly. The atlas
bucket-list tooltip had the same gap.

Also pins the escape on MapViewGL's photo marker. Its Leaflet twin has
FE-COMP-MAPVIEW-074 for this, but the GL side was only ever exercised with
benign values, so escapeHtml could have been dropped there without a test
noticing.
---
 .../components/Journey/JourneyMap.test.tsx    | 26 +++++++++++++++++++
 client/src/components/Journey/JourneyMap.tsx  | 11 +++++---
 client/src/components/Map/MapViewGL.test.tsx  | 17 ++++++++++++
 client/src/pages/atlas/useAtlas.ts            |  4 +--
 4 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/client/src/components/Journey/JourneyMap.test.tsx b/client/src/components/Journey/JourneyMap.test.tsx
index 61c27e5b7..2678fc038 100644
--- a/client/src/components/Journey/JourneyMap.test.tsx
+++ b/client/src/components/Journey/JourneyMap.test.tsx
@@ -550,4 +550,30 @@ describe('JourneyMap', () => {
     expect(colors).not.toContain('#ff0000');
     expect(colors).not.toContain('#ffffff');
   });
+
+  // A string handed to bindTooltip becomes innerHTML. A track name is a place
+  // name off a shared trip and an entry label is a journey title, so both are
+  // written by someone who is not necessarily the person reading the map — and
+  // this component is what the public journey page renders.
+  it('FE-COMP-JOURNEYMAP-044: escapes a track name before it becomes tooltip markup', () => {
+    vi.mocked(L.polyline).mockClear();
+    render(<JourneyMap checkins={[]} entries={entriesWithCoords} tracks={[track({ name: '<img src=x onerror="alert(1)">' })]} />);
+
+    const line = vi.mocked(L.polyline).mock.results
+      .map(r => r.value as any)
+      .find(v => v.bindTooltip.mock.calls.length > 0);
+    const [label] = line.bindTooltip.mock.calls[0];
+    expect(label).not.toContain('<img');
+    expect(label).not.toContain('onerror="');
+    expect(label).toBe('&lt;img src=x onerror=&quot;alert(1)&quot;&gt;');
+  });
+
+  it('FE-COMP-JOURNEYMAP-045: escapes an entry title before it becomes tooltip markup', () => {
+    const hostile = [{ id: 'e9', lat: 48.1, lng: 2.1, title: '<img src=x onerror="alert(1)">', mood: null, entry_date: '2025-06-04' }];
+    render(<JourneyMap checkins={[]} entries={hostile} />);
+
+    const labels = vi.mocked(mockedMarker().bindTooltip).mock.calls.map(c => c[0]);
+    expect(labels.some(l => typeof l === 'string' && l.includes('<img'))).toBe(false);
+    expect(labels).toContain('&lt;img src=x onerror=&quot;alert(1)&quot;&gt;');
+  });
 });
diff --git a/client/src/components/Journey/JourneyMap.tsx b/client/src/components/Journey/JourneyMap.tsx
index d4f9f5e16..36c1b85f7 100644
--- a/client/src/components/Journey/JourneyMap.tsx
+++ b/client/src/components/Journey/JourneyMap.tsx
@@ -1,7 +1,7 @@
 import { useEffect, useRef, useImperativeHandle, useCallback, type Ref } from 'react'
 import L from 'leaflet'
 import { useSettingsStore } from '../../store/settingsStore'
-import type { JourneyTrack } from '@trek/shared'
+import { escapeHtml, type JourneyTrack } from '@trek/shared'
 
 export interface MapMarkerItem {
   id: string
@@ -212,8 +212,9 @@ function JourneyMap(
       const line = L.polyline(coords, { color, weight: 3.5, opacity: 0.95, lineCap: 'round', lineJoin: 'round' })
       // Same tooltip the markers on this map use, rather than Leaflet's default box:
       // it follows the appearance tokens, so it lands right in dark mode and with
-      // transparency switched off.
-      if (track.name) line.bindTooltip(track.name, { sticky: true, direction: 'top', className: 'map-tooltip' })
+      // transparency switched off. Escaped because a string handed to bindTooltip
+      // becomes innerHTML, and a track name is a place name off a shared trip.
+      if (track.name) line.bindTooltip(escapeHtml(track.name), { sticky: true, direction: 'top', className: 'map-tooltip' })
       line.addTo(map)
       coords.forEach(c => allCoords.push(c))
     }
@@ -243,7 +244,9 @@ function JourneyMap(
       })
 
       const marker = L.marker(pos, { icon }).addTo(map)
-      marker.bindTooltip(item.label, {
+      // Escaped for the same reason as the track tooltip above: the label is an
+      // entry title, and this map is what the public journey page renders.
+      marker.bindTooltip(escapeHtml(item.label), {
         direction: 'top',
         offset: [0, -MARKER_H],
         className: 'map-tooltip',
diff --git a/client/src/components/Map/MapViewGL.test.tsx b/client/src/components/Map/MapViewGL.test.tsx
index 550e14a9b..eceae5a2f 100644
--- a/client/src/components/Map/MapViewGL.test.tsx
+++ b/client/src/components/Map/MapViewGL.test.tsx
@@ -851,6 +851,23 @@ describe('MapViewGL', () => {
     expect(glMarkers.created[1].lngLat).toEqual([2.12, 48.12])
   })
 
+  // The Leaflet twin of this is FE-COMP-MAPVIEW-074. Both renderers build their
+  // marker as an HTML string, so both need the escape pinned — otherwise it can
+  // be dropped from one of them without a single test going red.
+  it('FE-COMP-MAPVIEWGL-023b: an image_url that passes the prefix check is still escaped', async () => {
+    loadOnAttach()
+    render(<MapViewGL places={[buildMapPlace({
+      id: 13, lat: 48.13, lng: 2.13, image_url: '/uploads/x" onerror="alert(1)" y="',
+    })]} fitKey={1} />)
+    await act(async () => {})
+
+    const marker = glMarkers.created[0].element
+    expect(marker.innerHTML).not.toContain('onerror="alert(1)"')
+    expect(marker.innerHTML).toContain('&quot;')
+    // The escape must not break the ordinary case: the src still resolves.
+    expect(marker.querySelector('img')?.getAttribute('src')).toBe('/uploads/x" onerror="alert(1)" y="')
+  })
+
   it('FE-COMP-MAPVIEWGL-024: the hover card shows name, category and address and follows the cursor', async () => {
     loadOnAttach()
     const places = [buildMapPlace({
diff --git a/client/src/pages/atlas/useAtlas.ts b/client/src/pages/atlas/useAtlas.ts
index 293aa23c9..140f386a3 100644
--- a/client/src/pages/atlas/useAtlas.ts
+++ b/client/src/pages/atlas/useAtlas.ts
@@ -6,7 +6,7 @@ import apiClient, { mapsApi, pluginsApi, type PluginAtlasLayer } from '../../api
 import L from 'leaflet'
 import type { GeoJsonFeatureCollection } from '../../types'
 import { A2_TO_A3, countryStatus, findBucketDuplicate, isBucketDuplicateError, isCountryVisible, normalizeRegionName, withCountryMarkedVisited, wishlistA3Codes, countryColor, type AtlasData, type AtlasPlaceHit, type CountryDetail, type BucketItem } from './atlasModel'
-import { continentForCountry, type VisitStatus } from '@trek/shared'
+import { continentForCountry, escapeHtml, type VisitStatus } from '@trek/shared'
 import { useToast } from '../../components/shared/Toast'
 import { getApiErrorMessage } from '../../types'
 
@@ -941,7 +941,7 @@ export function useAtlas() {
         iconAnchor: [14, 14],
       })
       return L.marker([b.lat!, b.lng!], { icon }).bindTooltip(
-        `<div style="font-size:12px;font-weight:600">${b.name}</div>${b.notes ? `<div style="font-size:10px;opacity:0.7;margin-top:2px">${b.notes}</div>` : ''}`,
+        `<div style="font-size:12px;font-weight:600">${escapeHtml(b.name)}</div>${b.notes ? `<div style="font-size:10px;opacity:0.7;margin-top:2px">${escapeHtml(b.notes)}</div>` : ''}`,
         { className: 'atlas-tooltip', direction: 'top', offset: [0, -14] }
       )
     })

From 1eb29f8b7773f1f455f5c8333f6a2baafd18ea6f Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sun, 16 Aug 2026 23:00:57 +0200
Subject: [PATCH 6/9] fix(trips): confine assignee writes to the people on the
 trip
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Holding packing_edit or todo_edit says you may edit this trip's lists, not that
any user id you name belongs to it. Both updateCategoryAssignees loops wrote the
body's ids straight through, and both end by re-selecting through a JOIN on
users — so an id from anywhere on the instance came back with a display name and
an avatar attached, in the response and in the WebSocket fan-out to the room. The
packing item recipient writes had the same gap.

packing already had tripRosterIds for exactly this and used it only for bag
members; its doc comment said "assigned to a bag", which is probably why the
other three writes grew up without it. The query moves to DatabaseService next to
canAccessTrip, which asks the same question as a predicate, and the helper now
delegates. Every service that needs it already injects DatabaseService.

Off-roster ids drop rather than 400: a copied trip carries assignee ids over
before its members exist, and rejecting the whole request would stall the picker
there. Guests are unaffected — a guest is a trip_members row, so the roster
includes them, which is the point of #1362.
---
 server/src/nest/database/database.service.ts  | 19 ++++++++++++
 server/src/nest/packing/packing.service.ts    | 20 +++++++++----
 server/src/nest/todo/todo.service.ts          |  7 ++++-
 .../tests/unit/nest/packing.service.test.ts   |  4 +++
 server/tests/unit/nest/todo.service.test.ts   | 29 +++++++++++++++++++
 5 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/server/src/nest/database/database.service.ts b/server/src/nest/database/database.service.ts
index 0d18ea549..103f4c8e1 100644
--- a/server/src/nest/database/database.service.ts
+++ b/server/src/nest/database/database.service.ts
@@ -56,6 +56,25 @@ export class DatabaseService {
     return isOwner(tripId, userId);
   }
 
+  /**
+   * The user ids a trip may refer to: its members plus the owner. Guests count —
+   * a guest is a credential-less users row joined into trip_members, and #1362
+   * makes it assignable everywhere a real member is.
+   *
+   * This is canAccessTrip's question asked as a set rather than a predicate, so
+   * it lives beside it. Domains that accept user ids in a write body intersect
+   * against it: holding the permission says you may edit the trip, not that any
+   * id you name belongs to it. Runs on the connection rather than delegating to
+   * db/database, because it has no legacy export to stay parity with.
+   */
+  rosterUserIds(tripId: number | string): Set<number> {
+    const rows = this.all<{ user_id: number }>(
+      'SELECT user_id FROM trip_members WHERE trip_id = ? UNION SELECT user_id FROM trips WHERE id = ?',
+      tripId, tripId,
+    );
+    return new Set(rows.map(r => r.user_id));
+  }
+
   getPlaceWithTags(placeId: number | string): PlaceWithTags | null {
     return getPlaceWithTags(placeId);
   }
diff --git a/server/src/nest/packing/packing.service.ts b/server/src/nest/packing/packing.service.ts
index 1fef2f8a3..01c169ccb 100644
--- a/server/src/nest/packing/packing.service.ts
+++ b/server/src/nest/packing/packing.service.ts
@@ -231,7 +231,8 @@ export class PackingService {
       // "Shared with specific people" — record the recipients it covers.
       if (data.visibility === 'shared' && Array.isArray(data.recipient_ids)) {
         const ins = this.db.prepare('INSERT OR IGNORE INTO packing_item_recipients (item_id, user_id) VALUES (?, ?)');
-        for (const uid of data.recipient_ids) if (uid !== ownerId) ins.run(id, uid);
+        const roster = this.tripRosterIds(tripId);
+        for (const uid of data.recipient_ids) if (uid !== ownerId && roster.has(uid)) ins.run(id, uid);
       }
       return id;
     });
@@ -350,7 +351,8 @@ export class PackingService {
       if (visibility === 'shared') {
         const ins = this.db.prepare('INSERT OR IGNORE INTO packing_item_recipients (item_id, user_id) VALUES (?, ?)');
         const owner = item.owner_id ?? actingUserId;
-        for (const uid of recipientIds) if (uid !== owner) ins.run(id, uid);
+        const roster = this.tripRosterIds(tripId);
+        for (const uid of recipientIds) if (uid !== owner && roster.has(uid)) ins.run(id, uid);
       }
       // Leaving the Common tier drops any co-contributors (they only apply to Common).
       if (visibility !== 'common') this.db.run('DELETE FROM packing_item_contributors WHERE item_id = ?', id);
@@ -479,10 +481,14 @@ export class PackingService {
     }));
   }
 
-  /** Owner + collaborators of a trip — the only user ids that may be assigned to a bag. */
+  /**
+   * Owner + collaborators of a trip, guests included — the only user ids
+   * assignable anywhere on it, not just to a bag. The wording used to say
+   * "assigned to a bag", which is why the category and recipient writes below
+   * grew up without it.
+   */
   private tripRosterIds(tripId: string | number): Set<number> {
-    const rows = this.db.all<{ user_id: number }>('SELECT user_id FROM trip_members WHERE trip_id = ? UNION SELECT user_id FROM trips WHERE id = ?', tripId, tripId);
-    return new Set(rows.map(r => r.user_id));
+    return this.db.rosterUserIds(tripId);
   }
 
   setBagMembers(tripId: string | number, bagId: string | number, userIds: number[]) {
@@ -665,7 +671,9 @@ export class PackingService {
 
       if (Array.isArray(userIds) && userIds.length > 0) {
         const insert = this.db.prepare('INSERT OR IGNORE INTO packing_category_assignees (trip_id, category_name, user_id) VALUES (?, ?, ?)');
-        for (const uid of userIds) insert.run(tripId, categoryName, uid);
+        // Same rule as setBagMembers: only people on this trip may be assigned.
+        const roster = this.tripRosterIds(tripId);
+        for (const uid of userIds) if (roster.has(uid)) insert.run(tripId, categoryName, uid);
       }
     });
 
diff --git a/server/src/nest/todo/todo.service.ts b/server/src/nest/todo/todo.service.ts
index 8d2025ba0..31bf80953 100644
--- a/server/src/nest/todo/todo.service.ts
+++ b/server/src/nest/todo/todo.service.ts
@@ -136,7 +136,12 @@ export class TodoService {
 
       if (Array.isArray(userIds) && userIds.length > 0) {
         const insert = this.db.prepare('INSERT OR IGNORE INTO todo_category_assignees (trip_id, category_name, user_id) VALUES (?, ?, ?)');
-        for (const uid of userIds) insert.run(tripId, categoryName, uid);
+        // Only people on this trip may be assigned, the way packing filters bag
+        // members and reservations filter travellers. Dropped rather than
+        // rejected: a copied trip carries assignee ids across before its members
+        // exist, and a 400 would make the picker unusable there.
+        const roster = this.db.rosterUserIds(tripId);
+        for (const uid of userIds) if (roster.has(uid)) insert.run(tripId, categoryName, uid);
       }
     });
 
diff --git a/server/tests/unit/nest/packing.service.test.ts b/server/tests/unit/nest/packing.service.test.ts
index 44571e593..649db5838 100644
--- a/server/tests/unit/nest/packing.service.test.ts
+++ b/server/tests/unit/nest/packing.service.test.ts
@@ -515,6 +515,7 @@ describe('three-tier packing sharing (#858)', () => {
     const { user: friend } = createUser(testDb);
     const { user: stranger } = createUser(testDb);
     const trip = createTrip(testDb, owner.id);
+    addTripMember(testDb, trip.id, friend.id);
 
     const item = svc.createItem(trip.id, { name: 'Power bank', visibility: 'shared', recipient_ids: [friend.id] }, owner.id) as any;
     expect(item.is_private).toBe(1);
@@ -539,6 +540,7 @@ describe('three-tier packing sharing (#858)', () => {
     const { user: owner } = createUser(testDb);
     const { user: friend } = createUser(testDb);
     const trip = createTrip(testDb, owner.id);
+    addTripMember(testDb, trip.id, friend.id);
     const item = svc.createItem(trip.id, { name: 'First aid', visibility: 'personal' }, owner.id) as any;
 
     // A non-owner who cannot see the item at all gets the missing-item answer, so
@@ -1007,6 +1009,8 @@ describe('packing item object-level authorization', () => {
     const { user: intruder } = createUser(testDb);
     const { user: friend } = createUser(testDb);
     const trip = createTrip(testDb, owner.id);
+    // The recipient is a fellow traveller; the intruder stays off the trip on purpose.
+    addTripMember(testDb, trip.id, friend.id);
     const personal = svc.createItem(trip.id, { name: 'Diary', visibility: 'personal' }, owner.id) as any;
     const shared = svc.createItem(trip.id, { name: 'Power bank', visibility: 'shared', recipient_ids: [friend.id] }, owner.id) as any;
     const common = svc.createItem(trip.id, { name: 'Tent', visibility: 'common' }, owner.id) as any;
diff --git a/server/tests/unit/nest/todo.service.test.ts b/server/tests/unit/nest/todo.service.test.ts
index b52e7931a..b3fcc351e 100644
--- a/server/tests/unit/nest/todo.service.test.ts
+++ b/server/tests/unit/nest/todo.service.test.ts
@@ -270,6 +270,35 @@ describe('getCategoryAssignees / updateCategoryAssignees', () => {
     expect(assignees['Logistics']).toHaveLength(1);
   });
 
+  // Holding todo_edit says you may edit this trip's list, not that any user id
+  // you name belongs to it. The read-back joins users and hands the caller a
+  // display name and an avatar, so an unfiltered id turns the endpoint into a
+  // lookup for every account on the instance.
+  it('TODO-SVC-017a: drops a user who is not on the trip, and says nothing about them', () => {
+    const { user: owner } = createUser(testDb);
+    const { user: member } = createUser(testDb);
+    const { user: stranger } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    addTripMember(testDb, trip.id, member.id);
+
+    const rows = svc.updateCategoryAssignees(trip.id, 'Packing', [owner.id, stranger.id, member.id]) as { user_id: number }[];
+
+    expect(rows.map(r => r.user_id).sort()).toEqual([owner.id, member.id].sort());
+    expect(JSON.stringify(rows)).not.toContain(stranger.username);
+    expect(testDb.prepare('SELECT COUNT(*) AS n FROM todo_category_assignees WHERE user_id = ?').get(stranger.id)).toEqual({ n: 0 });
+  });
+
+  it('TODO-SVC-017b: keeps a guest, who is a trip member like any other', () => {
+    const { user: owner } = createUser(testDb);
+    const { user: guest } = createUser(testDb);
+    testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
+    const trip = createTrip(testDb, owner.id);
+    addTripMember(testDb, trip.id, guest.id);
+
+    const rows = svc.updateCategoryAssignees(trip.id, 'Packing', [guest.id]) as { user_id: number }[];
+    expect(rows.map(r => r.user_id)).toEqual([guest.id]);
+  });
+
   it('TODO-SVC-020: updateCategoryAssignees replaces existing assignees (not append)', () => {
     const { user: owner } = createUser(testDb);
     const { user: member } = createUser(testDb);

From 0337ebbbd23e48d2ded5ad73d8be6b145b505937 Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sun, 16 Aug 2026 23:01:11 +0200
Subject: [PATCH 7/9] fix(assignments): confine day participants to the trip
 roster
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

setParticipants took the ids on trust. The caller is proven to be on the trip and
the assignment is proven to sit on it, but nothing said the same about the ids in
the body, and the read-back joins users — so a stranger's name and avatar came
back to the writer and went out over the assignment:participants broadcast to
everyone else in the room.

Takes the tripId both callers already hold and filters against the roster. Same
drop-rather-than-reject rule as bag members and reservation travellers: the
participants box posts the whole list on every edit, so a trip whose membership
changed underneath it would otherwise be stuck.

ASG-SVC-028 covered a foreign key error rolling the replace back. The filter
takes an unknown id out before the insert can reach it, so that path is gone and
the case now pins what happens instead: a list of nothing but strangers clears,
like the empty list ASG-SVC-018 already pins.
---
 .../assignments/assignments.controller.ts     |  2 +-
 .../src/nest/assignments/assignments.mcp.ts   |  2 +-
 .../nest/assignments/assignments.service.ts   | 15 ++++--
 .../unit/nest/assignments.controller.test.ts  |  3 +-
 .../unit/nest/assignments.service.test.ts     | 53 +++++++++++++------
 5 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/server/src/nest/assignments/assignments.controller.ts b/server/src/nest/assignments/assignments.controller.ts
index b076762de..7f274946c 100644
--- a/server/src/nest/assignments/assignments.controller.ts
+++ b/server/src/nest/assignments/assignments.controller.ts
@@ -202,7 +202,7 @@ export class AssignmentOpsController {
     if (!this.assignments.getAssignmentForTrip(id, tripId)) {
       throw new HttpException({ error: 'Assignment not found' }, 404);
     }
-    const participants = this.assignments.setParticipants(id, body.user_ids);
+    const participants = this.assignments.setParticipants(id, body.user_ids, tripId);
     this.assignments.broadcast(tripId, 'assignment:participants', { assignmentId: Number(id), participants }, socketId);
     return { participants };
   }
diff --git a/server/src/nest/assignments/assignments.mcp.ts b/server/src/nest/assignments/assignments.mcp.ts
index 68b843a96..989a7a727 100644
--- a/server/src/nest/assignments/assignments.mcp.ts
+++ b/server/src/nest/assignments/assignments.mcp.ts
@@ -187,7 +187,7 @@ export class AssignmentsMcp {
     if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess();
     if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied();
     if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.');
-    const participants = this.assignments.setParticipants(assignmentId, userIds);
+    const participants = this.assignments.setParticipants(assignmentId, userIds, tripId);
     this.guards.safeBroadcast(tripId, 'assignment:participants', { assignmentId, participants });
     return ok({ participants });
   }
diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts
index 379943811..b53b657e2 100644
--- a/server/src/nest/assignments/assignments.service.ts
+++ b/server/src/nest/assignments/assignments.service.ts
@@ -283,12 +283,21 @@ export class AssignmentsService {
     return this.getAssignmentWithPlace(Number(id));
   }
 
-  setParticipants(assignmentId: string | number, userIds: number[]) {
+  /**
+   * Both callers have already proven the assignment sits on `tripId`; this
+   * settles the other half, that the ids in the body do too. Off-roster ids drop
+   * silently rather than 400, matching bag members and reservation travellers —
+   * the participants box sends the whole list back on every edit, so rejecting
+   * the request would strand a trip whose membership changed underneath it.
+   */
+  setParticipants(assignmentId: string | number, userIds: number[], tripId: string | number) {
+    const roster = this.dbs.rosterUserIds(tripId);
+    const scoped = userIds.filter(id => roster.has(id));
     this.dbs.transaction(() => {
       this.dbs.run('DELETE FROM assignment_participants WHERE assignment_id = ?', assignmentId);
-      if (userIds.length > 0) {
+      if (scoped.length > 0) {
         const insert = this.dbs.prepare('INSERT OR IGNORE INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)');
-        for (const userId of userIds) insert.run(assignmentId, userId);
+        for (const userId of scoped) insert.run(assignmentId, userId);
       }
     });
 
diff --git a/server/tests/unit/nest/assignments.controller.test.ts b/server/tests/unit/nest/assignments.controller.test.ts
index 2079addef..5580d5301 100644
--- a/server/tests/unit/nest/assignments.controller.test.ts
+++ b/server/tests/unit/nest/assignments.controller.test.ts
@@ -101,7 +101,8 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', (
     expect(setParticipants).not.toHaveBeenCalled();
     const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), setParticipants, broadcast } as Partial<AssignmentsService>);
     expect(new AssignmentOpsController(s).setParticipants(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ participants: [{ user_id: 2 }] });
-    expect(setParticipants).toHaveBeenCalledWith('9', [2]);
+    // The trip comes along so the service can confine the ids to its roster.
+    expect(setParticipants).toHaveBeenCalledWith('9', [2], '5');
     expect(broadcast).toHaveBeenCalledWith('5', 'assignment:participants', { assignmentId: 9, participants: [{ user_id: 2 }] }, 'sock');
   });
 });
diff --git a/server/tests/unit/nest/assignments.service.test.ts b/server/tests/unit/nest/assignments.service.test.ts
index bda5951c4..db6d1bf2d 100644
--- a/server/tests/unit/nest/assignments.service.test.ts
+++ b/server/tests/unit/nest/assignments.service.test.ts
@@ -205,7 +205,7 @@ describe('listDayAssignments', () => {
     testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tag.id);
     const a1 = createDayAssignment(testDb, day.id, place.id, { order_index: 1 });
     const a2 = createDayAssignment(testDb, day.id, second.id, { order_index: 0 });
-    svc.setParticipants(a1.id, [user.id]);
+    svc.setParticipants(a1.id, [user.id], trip.id);
 
     const list = svc.listDayAssignments(day.id);
     expect(list.map(a => a.id)).toEqual([a2.id, a1.id]);
@@ -284,45 +284,64 @@ describe('moveAssignment', () => {
 
 describe('getParticipants / setParticipants', () => {
   it('ASG-SVC-017: setParticipants replaces the list and returns the joined rows', () => {
-    const { user, day, place } = fixture();
+    const { user, trip, day, place } = fixture();
     const { user: peer } = createUser(testDb);
+    addTripMember(testDb, trip.id, peer.id);
     const a = createDayAssignment(testDb, day.id, place.id);
-    svc.setParticipants(a.id, [user.id]);
-    const replaced = svc.setParticipants(a.id, [peer.id]);
+    svc.setParticipants(a.id, [user.id], trip.id);
+    const replaced = svc.setParticipants(a.id, [peer.id], trip.id);
     expect(replaced).toEqual([{ user_id: peer.id, username: peer.username, avatar: null }]);
     expect(svc.getParticipants(a.id)).toEqual(replaced);
   });
 
   it('ASG-SVC-018: empty array clears all participants', () => {
-    const { user, day, place } = fixture();
+    const { user, trip, day, place } = fixture();
     const a = createDayAssignment(testDb, day.id, place.id);
-    svc.setParticipants(a.id, [user.id]);
-    expect(svc.setParticipants(a.id, [])).toEqual([]);
+    svc.setParticipants(a.id, [user.id], trip.id);
+    expect(svc.setParticipants(a.id, [], trip.id)).toEqual([]);
     expect(svc.getParticipants(a.id)).toEqual([]);
   });
 
   it('ASG-SVC-019: username COALESCEs display_name over username', () => {
-    const { user, day, place } = fixture();
+    const { user, trip, day, place } = fixture();
     testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Fancy Name', user.id);
     const a = createDayAssignment(testDb, day.id, place.id);
-    const rows = svc.setParticipants(a.id, [user.id]);
+    const rows = svc.setParticipants(a.id, [user.id], trip.id);
     expect(rows).toEqual([{ user_id: user.id, username: 'Fancy Name', avatar: null }]);
   });
 
   it('ASG-SVC-020: duplicate user ids collapse via INSERT OR IGNORE', () => {
-    const { user, day, place } = fixture();
+    const { user, trip, day, place } = fixture();
     const a = createDayAssignment(testDb, day.id, place.id);
-    const rows = svc.setParticipants(a.id, [user.id, user.id]);
+    const rows = svc.setParticipants(a.id, [user.id, user.id], trip.id);
     expect(rows).toHaveLength(1);
   });
 
-  it('ASG-SVC-028: a failing insert rolls back the whole replace (delete included)', () => {
-    const { user, day, place } = fixture();
+  // Used to assert that an id violating the users FK threw and rolled the delete
+  // back. It cannot reach the insert any more: the roster filter drops an id that
+  // is not on the trip, so a list of nothing but strangers is simply an empty
+  // list, and the delete stands. The rollback itself is still covered — the
+  // transaction wraps delete and insert together, and ASG-SVC-018 pins that an
+  // explicit empty list clears.
+  it('ASG-SVC-028: a list of ids that are not on the trip clears rather than throwing', () => {
+    const { user, trip, day, place } = fixture();
     const a = createDayAssignment(testDb, day.id, place.id);
-    svc.setParticipants(a.id, [user.id]);
-    // 999999 violates the users FK — the transaction must undo the delete too.
-    expect(() => svc.setParticipants(a.id, [999999])).toThrow();
-    expect(svc.getParticipants(a.id)).toEqual([{ user_id: user.id, username: user.username, avatar: null }]);
+    svc.setParticipants(a.id, [user.id], trip.id);
+    expect(() => svc.setParticipants(a.id, [999999], trip.id)).not.toThrow();
+    expect(svc.getParticipants(a.id)).toEqual([]);
+  });
+
+  it('ASG-SVC-029: keeps the trip members and drops the stranger in one call', () => {
+    const { user, trip, day, place } = fixture();
+    const { user: member } = createUser(testDb);
+    const { user: stranger } = createUser(testDb);
+    addTripMember(testDb, trip.id, member.id);
+    const a = createDayAssignment(testDb, day.id, place.id);
+
+    const rows = svc.setParticipants(a.id, [user.id, stranger.id, member.id], trip.id) as { user_id: number }[];
+
+    expect(rows.map(r => r.user_id).sort()).toEqual([user.id, member.id].sort());
+    expect(JSON.stringify(rows)).not.toContain(stranger.username);
   });
 });
 

From 244eb56e4cff07651c5b4190bc2ef79f61917d75 Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sun, 16 Aug 2026 23:01:25 +0200
Subject: [PATCH 8/9] fix(budget): confine payers, split members and settlement
 parties to the trip
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

knownUserIds asked whether a user exists at all, not whether they are on this
trip, so any id on the instance could be made a payer or a split member. Both
read-backs join users, and the settlement projection carries the avatar through
avatarUrl, which passes an absolute URL straight out — for an OIDC account that
is the picture claim.

Payers and split members drop off-roster ids, as before, only against the right
set. Settlements cannot: they name exactly two parties and both columns are NOT
NULL, so dropping one would either break the constraint or move money to the
wrong person. Both wrappers refuse the write and return null, which lands on the
404 the update path already had. That answer is deliberately the same one a
missing settlement gets, so the endpoint says nothing about whether an id exists
— and it retires the foreign key error that used to surface as a 500 and drew
exactly that distinction.

createSettlement grows the 404 its sibling had; the MCP tool reuses the error
string already at hand. Money rules are untouched: a settlement is a transfer, so
refusing one cannot unbalance the sum, and the cent arithmetic is unchanged.
---
 server/src/nest/budget/budget.controller.ts   |  5 ++
 server/src/nest/budget/budget.mcp.ts          |  1 +
 server/src/nest/budget/budget.service.ts      | 55 +++++++++++------
 server/tests/e2e/budget.e2e.test.ts           |  3 +
 .../tests/unit/nest/budget.service.db.test.ts | 59 ++++++++++++++++++-
 server/tests/unit/nest/budget.service.test.ts | 19 ++++++
 6 files changed, 123 insertions(+), 19 deletions(-)

diff --git a/server/src/nest/budget/budget.controller.ts b/server/src/nest/budget/budget.controller.ts
index ce9a84096..319c3ded6 100644
--- a/server/src/nest/budget/budget.controller.ts
+++ b/server/src/nest/budget/budget.controller.ts
@@ -94,6 +94,11 @@ export class BudgetController {
       { from_user_id: body.from_user_id, to_user_id: body.to_user_id, amount: body.amount, currency: body.currency },
       user.id,
     );
+    // A party who is not on this trip gets the same answer as a settlement that
+    // does not exist, so the endpoint cannot be used to probe for user ids.
+    if (!settlement) {
+      throw new HttpException({ error: 'Settlement not found' }, 404);
+    }
     this.budget.broadcast(tripId, 'budget:settlement-created', { settlement }, socketId);
     return { settlement };
   }
diff --git a/server/src/nest/budget/budget.mcp.ts b/server/src/nest/budget/budget.mcp.ts
index 49f1602dd..5d2b74217 100644
--- a/server/src/nest/budget/budget.mcp.ts
+++ b/server/src/nest/budget/budget.mcp.ts
@@ -330,6 +330,7 @@ export class BudgetMcp {
     // Freeze-then-write composite, same as the REST path (no-op while the
     // schema has no currency input — see the #1445 note in the class doc).
     const settlement = await this.budget.createSettlement(tripId, { from_user_id, to_user_id, amount }, ctx.userId);
+    if (!settlement) return errorResult('Settlement not found.');
     this.guards.safeBroadcast(tripId, 'budget:settlement-created', { settlement });
     return ok({ settlement });
   }
diff --git a/server/src/nest/budget/budget.service.ts b/server/src/nest/budget/budget.service.ts
index c5c3556ae..0239033ad 100644
--- a/server/src/nest/budget/budget.service.ts
+++ b/server/src/nest/budget/budget.service.ts
@@ -126,21 +126,25 @@ export class BudgetService {
     return rows.map(p => ({ ...p, avatar_url: avatarUrl(p) }));
   }
 
-  private knownUserIds(userIds: number[]): Set<number> {
-    const unique = [...new Set(userIds)];
-    if (unique.length === 0) return new Set();
-    const rows = this.db.all<{ id: number }>(
-      `SELECT id FROM users WHERE id IN (${unique.map(() => '?').join(',')})`,
-      ...unique,
-    );
-    return new Set(rows.map(r => r.id));
+  /**
+   * The subset of `userIds` that is actually on this trip. Used to be a plain
+   * existence check against `users`, which let any id on the instance become a
+   * payer or a split member and come back out through the read-back join with a
+   * name and an avatar attached. Off-roster ids drop silently, the way the
+   * packing and reservations assignee paths handle them.
+   */
+  private rosterMemberIds(tripId: string | number, userIds: number[]): Set<number> {
+    const unique = new Set(userIds);
+    if (unique.size === 0) return new Set();
+    const roster = this.db.rosterUserIds(tripId);
+    return new Set([...unique].filter(id => roster.has(id)));
   }
 
   /** Replace the payer rows of an item and keep total_price = sum of payer amounts. */
-  private writeItemPayers(itemId: number | string, payers: { user_id: number; amount: number }[]) {
+  private writeItemPayers(itemId: number | string, tripId: string | number, payers: { user_id: number; amount: number }[]) {
     this.db.run('DELETE FROM budget_item_payers WHERE budget_item_id = ?', itemId);
     const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_payers (budget_item_id, user_id, amount) VALUES (?, ?, ?)');
-    const known = this.knownUserIds(payers.map(p => p.user_id));
+    const known = this.rosterMemberIds(tripId, payers.map(p => p.user_id));
     let total = 0;
     for (const p of payers) {
       if (!(p.amount > 0) || !known.has(p.user_id)) continue;
@@ -347,9 +351,9 @@ export class BudgetService {
       const payerTotal = (data.payers || []).reduce((a, p) => a + (p.amount > 0 ? p.amount : 0), 0);
       const total = data.payers && data.payers.length > 0 ? payerTotal : (data.total_price || 0);
 
-      const knownMembers = data.members ? this.knownUserIds(data.members.map(m => m.user_id)) : null;
+      const knownMembers = data.members ? this.rosterMemberIds(tripId, data.members.map(m => m.user_id)) : null;
       const members = data.members && knownMembers ? data.members.filter(m => knownMembers.has(m.user_id)) : undefined;
-      const knownIds = data.member_ids ? this.knownUserIds(data.member_ids) : null;
+      const knownIds = data.member_ids ? this.rosterMemberIds(tripId, data.member_ids) : null;
       const memberIds = data.member_ids && knownIds ? data.member_ids.filter(uid => knownIds.has(uid)) : undefined;
 
       const { note, ticket } = splitLegacyTicketNote(data.note, data.ticket_json);
@@ -373,7 +377,7 @@ export class BudgetService {
       );
 
       const itemId = result.lastInsertRowid as number;
-      if (data.payers && data.payers.length > 0) this.writeItemPayers(itemId, data.payers);
+      if (data.payers && data.payers.length > 0) this.writeItemPayers(itemId, tripId, data.payers);
       if (members && members.length > 0) {
         const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, ?)');
         for (const m of members) insert.run(itemId, m.user_id, m.amount !== undefined && m.amount !== null ? m.amount : null);
@@ -461,7 +465,7 @@ export class BudgetService {
 
       // Optional inline payer/member replacement (the edit modal saves all at once).
       if (data.payers !== undefined) {
-        this.writeItemPayers(id, data.payers);
+        this.writeItemPayers(id, tripId, data.payers);
         // writeItemPayers derives total_price from the payer sum (0 for no payers).
         // A "recorded total, nobody assigned" expense clears payers but still carries
         // an explicit total_price — re-apply it so it isn't clobbered to 0.
@@ -470,14 +474,14 @@ export class BudgetService {
         }
       }
       if (data.members !== undefined) {
-        const known = this.knownUserIds(data.members.map(m => m.user_id));
+        const known = this.rosterMemberIds(tripId, data.members.map(m => m.user_id));
         const members = data.members.filter(m => known.has(m.user_id));
         this.db.run('DELETE FROM budget_item_members WHERE budget_item_id = ?', id);
         const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, ?)');
         for (const m of members) insert.run(id, m.user_id, m.amount !== undefined && m.amount !== null ? m.amount : null);
         this.db.run('UPDATE budget_items SET persons = ? WHERE id = ?', members.length || null, id);
       } else if (data.member_ids !== undefined) {
-        const known = this.knownUserIds(data.member_ids);
+        const known = this.rosterMemberIds(tripId, data.member_ids);
         const memberIds = data.member_ids.filter(uid => known.has(uid));
         this.db.run('DELETE FROM budget_item_members WHERE budget_item_id = ?', id);
         const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, NULL)');
@@ -510,7 +514,7 @@ export class BudgetService {
     return this.db.transaction(() => {
       const item = this.db.get('SELECT id FROM budget_items WHERE id = ? AND trip_id = ?', id, tripId);
       if (!item) return null;
-      this.writeItemPayers(id, payers);
+      this.writeItemPayers(id, tripId, payers);
       const updated = this.db.get<BudgetItem>('SELECT * FROM budget_items WHERE id = ?', id)!;
       updated.members = this.loadItemMembers(id);
       updated.payers = this.loadItemPayers(id);
@@ -540,7 +544,7 @@ export class BudgetService {
 
       this.db.run('DELETE FROM budget_item_members WHERE budget_item_id = ?', id);
 
-      const known = this.knownUserIds(userIds);
+      const known = this.rosterMemberIds(tripId, userIds);
       const memberIds = userIds.filter(uid => known.has(uid));
       if (memberIds.length > 0) {
         const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid) VALUES (?, ?, ?)');
@@ -949,7 +953,21 @@ export class BudgetService {
     return this.setItemPayers(id, tripId, payers);
   }
 
+  /**
+   * Unlike a split member, a settlement cannot drop an off-roster id: it has
+   * exactly two named parties and both columns are NOT NULL, so a stranger in
+   * either slot makes the row meaningless rather than merely wider. Refuse the
+   * whole write. Returning null lands on the caller's existing "Settlement not
+   * found" 404, which is also what keeps the endpoint from confirming whether
+   * an id it rejected exists at all.
+   */
+  private settlementPartiesOnTrip(tripId: string | number, data: { from_user_id: number; to_user_id: number }): boolean {
+    const roster = this.db.rosterUserIds(tripId);
+    return roster.has(data.from_user_id) && roster.has(data.to_user_id);
+  }
+
   async createSettlement(tripId: string | number, data: { from_user_id: number; to_user_id: number; amount: number; currency?: string | null }, userId: number) {
+    if (!this.settlementPartiesOnTrip(tripId, data)) return null;
     // Freeze the FX rate for the display currency the amount was entered in so the
     // transfer keeps cancelling its expense when live rates drift (#1445).
     await this.freezeForeignRate(tripId, data);
@@ -960,6 +978,7 @@ export class BudgetService {
     // Pass the settlement's stored currency so an edit that doesn't change it keeps
     // the already-frozen rate (#1445) — otherwise a live-rate drift would re-open a
     // settled position on an unrelated edit.
+    if (!this.settlementPartiesOnTrip(tripId, data)) return null;
     const existing = this.getSettlement(id, tripId);
     await this.freezeForeignRate(tripId, data, undefined, existing?.currency ?? null);
     return this.applySettlementUpdate(id, tripId, data);
diff --git a/server/tests/e2e/budget.e2e.test.ts b/server/tests/e2e/budget.e2e.test.ts
index 580e86222..0bb965214 100644
--- a/server/tests/e2e/budget.e2e.test.ts
+++ b/server/tests/e2e/budget.e2e.test.ts
@@ -73,6 +73,9 @@ describe('Budget e2e (real auth guard + temp SQLite, real budget SQL)', () => {
       "INSERT INTO users (id, username, email, password_hash, role, password_version) VALUES (2, 'e2e-peer', 'peer@example.test', 'x', 'user', 0)",
     ).run();
     tripId = Number(db.prepare("INSERT INTO trips (user_id, title, currency) VALUES (1, 'E2E Trip', 'EUR')").run().lastInsertRowid);
+    // The peer settles up with the owner below, so they have to be on the trip:
+    // a settlement between people who do not share one is refused.
+    db.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, 2)').run(tripId);
     app = await build();
     checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission');
     server = app.getHttpServer();
diff --git a/server/tests/unit/nest/budget.service.db.test.ts b/server/tests/unit/nest/budget.service.db.test.ts
index 2e8247258..6becc5019 100644
--- a/server/tests/unit/nest/budget.service.db.test.ts
+++ b/server/tests/unit/nest/budget.service.db.test.ts
@@ -55,7 +55,7 @@ vi.mock('../../../src/nest/budget/exchange-rates.service', () => ({
 import { createTables } from '../../../src/db/schema';
 import { runMigrations } from '../../../src/db/migrations';
 import { resetTestDb } from '../../helpers/test-db';
-import { createUser, createTrip } from '../../helpers/factories';
+import { createUser, createTrip, addTripMember } from '../../helpers/factories';
 import { BudgetService } from '../../../src/nest/budget/budget.service';
 import { DatabaseService } from '../../../src/nest/database/database.service';
 import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
@@ -230,6 +230,7 @@ describe('calculateSettlement custom splits', () => {
     const { user: alice } = createUser(testDb, { username: 'alice' });
     const { user: bob } = createUser(testDb, { username: 'bob' });
     const trip = createTrip(testDb, alice.id, { title: 'Trip' });
+    addTripMember(testDb, trip.id, bob.id);
 
     // 100 total, custom split: Alice owes 90, Bob owes 10. Alice paid the whole bill.
     budget.createBudgetItem(trip.id, {
@@ -261,6 +262,8 @@ describe('calculateSettlement squares up to the cent (#1382)', () => {
     const { user: bob } = createUser(testDb, { username: 'bob' });
     const { user: carol } = createUser(testDb, { username: 'carol' });
     const trip = createTrip(testDb, alice.id, { title: 'Trip' });
+    addTripMember(testDb, trip.id, bob.id);
+    addTripMember(testDb, trip.id, carol.id);
     const members = [{ user_id: alice.id }, { user_id: bob.id }, { user_id: carol.id }];
 
     // Totals that never divide by three, so every expense leaves a remainder cent
@@ -306,6 +309,8 @@ function seedIssue1543Trip(tripCurrency: string) {
   const { user: danil } = createUser(testDb, { username: 'danil' });
   const { user: serega } = createUser(testDb, { username: 'serega' });
   const trip = createTrip(testDb, me.id, { title: 'Trip' });
+  addTripMember(testDb, trip.id, danil.id);
+  addTripMember(testDb, trip.id, serega.id);
   testDb.prepare('UPDATE trips SET currency = ? WHERE id = ?').run(tripCurrency, trip.id);
   const members = [{ user_id: me.id }, { user_id: danil.id }, { user_id: serega.id }];
 
@@ -384,6 +389,7 @@ describe('rebaseTripCurrency', () => {
     const { user: alice } = createUser(testDb, { username: 'alice' });
     const { user: bob } = createUser(testDb, { username: 'bob' });
     const trip = createTrip(testDb, alice.id, { title: 'Trip' });
+    addTripMember(testDb, trip.id, bob.id);
     testDb.prepare("UPDATE trips SET currency = 'EUR' WHERE id = ?").run(trip.id);
     const members = [{ user_id: alice.id }, { user_id: bob.id }];
 
@@ -524,6 +530,57 @@ describe('composite service paths (ex budget.bridge delegation)', () => {
   });
 });
 
+// A settlement names two parties and both columns are NOT NULL, so an id that is
+// not on the trip cannot simply be dropped the way a split member can — the whole
+// write is refused. Refusing also means the endpoint stops answering "does this
+// user id exist", which it used to do by 500ing on the foreign key.
+describe('settlement parties are confined to the trip', () => {
+  it('BUDGET-SVC-DB-028: refuses a payer who is not on the trip', async () => {
+    const { user: alice } = createUser(testDb, { username: 'alice' });
+    const { user: outsider } = createUser(testDb, { username: 'outsider' });
+    const trip = createTrip(testDb, alice.id);
+
+    const created = await budget.createSettlement(trip.id, { from_user_id: outsider.id, to_user_id: alice.id, amount: 10 }, alice.id);
+
+    expect(created).toBeNull();
+    expect(budget.listSettlements(trip.id)).toEqual([]);
+  });
+
+  it('BUDGET-SVC-DB-029: answers a nonexistent user the same way, without throwing', async () => {
+    const { user: alice } = createUser(testDb, { username: 'alice' });
+    const trip = createTrip(testDb, alice.id);
+
+    await expect(budget.createSettlement(trip.id, { from_user_id: 999999, to_user_id: alice.id, amount: 10 }, alice.id))
+      .resolves.toBeNull();
+  });
+
+  it('BUDGET-SVC-DB-030: still records a settlement between the owner and a member', async () => {
+    const { user: alice } = createUser(testDb, { username: 'alice' });
+    const { user: bob } = createUser(testDb, { username: 'bob' });
+    const trip = createTrip(testDb, alice.id);
+    addTripMember(testDb, trip.id, bob.id);
+
+    const created = await budget.createSettlement(trip.id, { from_user_id: bob.id, to_user_id: alice.id, amount: 10 }, alice.id);
+
+    expect(created).toMatchObject({ from_user_id: bob.id, to_user_id: alice.id });
+  });
+
+  it('BUDGET-SVC-DB-031: drops an off-trip payer from an item instead of refusing it', () => {
+    const { user: alice } = createUser(testDb, { username: 'alice' });
+    const { user: outsider } = createUser(testDb, { username: 'outsider' });
+    const trip = createTrip(testDb, alice.id);
+
+    const item = budget.createBudgetItem(trip.id, {
+      name: 'Dinner',
+      payers: [{ user_id: alice.id, amount: 40 }, { user_id: outsider.id, amount: 60 }],
+    }) as { payers: { user_id: number }[]; total_price: number };
+
+    expect(item.payers.map(p => p.user_id)).toEqual([alice.id]);
+    // total_price is the sum of the payers that actually landed.
+    expect(item.total_price).toBe(40);
+  });
+});
+
 describe('post-fold quirk fixes', () => {
   it('BUDGET-SVC-DB-019: settlements prefer display_name over username (quirk fix)', () => {
     const { user: alice } = createUser(testDb, { username: 'alice' });
diff --git a/server/tests/unit/nest/budget.service.test.ts b/server/tests/unit/nest/budget.service.test.ts
index 501598d79..f7317cc7b 100644
--- a/server/tests/unit/nest/budget.service.test.ts
+++ b/server/tests/unit/nest/budget.service.test.ts
@@ -139,8 +139,14 @@ describe('BudgetService', () => {
   });
 
   describe('settlement ledger wrappers (#1445 freeze-then-write)', () => {
+    // Both wrappers refuse a party who is not on the trip before they freeze
+    // anything, so the roster lookup has to answer for these to reach the write
+    // at all. Users 1 and 2 are the two parties every case here settles between.
+    const rosterHas = (...userIds: number[]) => dbMock._stmt.all.mockReturnValue(userIds.map(user_id => ({ user_id })));
+
     it('createSettlement freezes the FX rate (await) before the raw insert', async () => {
       const s = svc();
+      rosterHas(1, 2);
       const freezeSpy = vi.spyOn(s, 'freezeForeignRate').mockResolvedValue();
       const insertSpy = vi.spyOn(s, 'insertSettlement').mockReturnValue({ id: 7 } as never);
       await s.createSettlement('5', { from_user_id: 1, to_user_id: 2, amount: 10 }, 3);
@@ -150,6 +156,7 @@ describe('BudgetService', () => {
 
     it('updateSettlement threads the stored currency through the freeze', async () => {
       const s = svc();
+      rosterHas(1, 2);
       const freezeSpy = vi.spyOn(s, 'freezeForeignRate').mockResolvedValue();
       // Quirk fix: the stored-currency lookup is a targeted getSettlement, not
       // a full listSettlements scan.
@@ -161,6 +168,18 @@ describe('BudgetService', () => {
       expect(freezeSpy).toHaveBeenCalledWith('5', { from_user_id: 1, to_user_id: 2, amount: 12, currency: 'USD' }, undefined, 'USD');
       expect(applySpy).toHaveBeenCalledWith('7', '5', { from_user_id: 1, to_user_id: 2, amount: 12, currency: 'USD' });
     });
+
+    it('refuses a party who is not on the trip, before freezing or writing', async () => {
+      const s = svc();
+      rosterHas(1); // user 2 is not on this trip
+      const freezeSpy = vi.spyOn(s, 'freezeForeignRate').mockResolvedValue();
+      const insertSpy = vi.spyOn(s, 'insertSettlement').mockReturnValue({ id: 7 } as never);
+
+      await expect(s.createSettlement('5', { from_user_id: 1, to_user_id: 2, amount: 10 }, 3)).resolves.toBeNull();
+
+      expect(freezeSpy).not.toHaveBeenCalled();
+      expect(insertSpy).not.toHaveBeenCalled();
+    });
   });
 
   describe('syncReservationPrice', () => {

From c9124c9b6371e9d9325eb62e4b4d647378374937 Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sun, 16 Aug 2026 23:01:37 +0200
Subject: [PATCH 9/9] fix(places): only attach tags owned by someone on the
 trip
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A tag belongs to a user, not to a trip, and the place create/update body is an
open record the Zod pipe does not police — so any tag id could be attached, and
the tag projection ships tags.user_id, which answers who owns a tag the caller
has no way to see. collection_place_tags had it verbatim.

Scoped to the roster rather than to the caller on purpose: a member tags a shared
place, another member re-saves it and, because the update is delete-then-insert,
sends that id back. Caller-scoping would drop a co-traveller's tag on every
foreign edit; roster-scoping keeps it. Collections resolve the same question
through their own member list.

No client sends a tags array today, so this only closes the hand-crafted path.
---
 .../nest/collections/collections.service.ts   | 15 +++++++++-
 server/src/nest/places/places.service.ts      | 27 +++++++++++++++--
 server/tests/unit/nest/places.service.test.ts | 29 ++++++++++++++++++-
 3 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/server/src/nest/collections/collections.service.ts b/server/src/nest/collections/collections.service.ts
index c2fe4b438..967835628 100644
--- a/server/src/nest/collections/collections.service.ts
+++ b/server/src/nest/collections/collections.service.ts
@@ -441,10 +441,23 @@ export class CollectionsService {
   // Saved places CRUD
   // -------------------------------------------------------------------------
 
+  /**
+   * A tag has no list of its own, only a `user_id`, so "belongs here" resolves
+   * through the people on the list — the same shape places.service uses against
+   * a trip roster. Off-list tag ids drop silently rather than being stored and
+   * read straight back out: the tag read-back ships `tags.user_id`, so an
+   * unfiltered id answers who owns a tag the caller cannot otherwise see.
+   */
   private attachTags(collectionPlaceId: number, tagIds: number[] | undefined): void {
     if (!tagIds || tagIds.length === 0) return;
+    const unique = [...new Set(tagIds)];
+    const eligible = this.collectionMemberIds(this.collectionIdOfPlace(collectionPlaceId));
+    const owned = this.db.all<{ id: number; user_id: number }>(
+      `SELECT id, user_id FROM tags WHERE id IN (${unique.map(() => '?').join(',')})`,
+      ...unique,
+    );
     const stmt = this.db.prepare('INSERT OR IGNORE INTO collection_place_tags (collection_place_id, tag_id) VALUES (?, ?)');
-    for (const tid of tagIds) stmt.run(collectionPlaceId, tid);
+    for (const t of owned) if (eligible.has(t.user_id)) stmt.run(collectionPlaceId, t.id);
   }
 
   /** Owner + accepted members — the users whose votes may live in this list. */
diff --git a/server/src/nest/places/places.service.ts b/server/src/nest/places/places.service.ts
index ceef2576d..552422c2e 100644
--- a/server/src/nest/places/places.service.ts
+++ b/server/src/nest/places/places.service.ts
@@ -122,6 +122,29 @@ export class PlacesService {
     return this.permissions.checkPermission('place_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id);
   }
 
+  /**
+   * The subset of `tagIds` that someone on this trip owns. A tag has no trip of
+   * its own, only a `user_id`, so "belongs to this trip" resolves through the
+   * roster — which keeps the shared case working: a member tags a place, another
+   * member re-saves it and sends the id back, and the tag survives because its
+   * owner is still on the trip. Scoping to the caller instead would quietly strip
+   * a co-traveller's tag on every foreign edit.
+   *
+   * Off-roster ids drop silently. The place body is an open record, so an id can
+   * arrive from an older client that had no business knowing it existed, and the
+   * read-back join hands `tags.user_id` straight to the caller.
+   */
+  private tagsOnTrip(tripId: string | number, tagIds: number[]): number[] {
+    const unique = [...new Set(tagIds)];
+    if (unique.length === 0) return [];
+    const roster = this.dbs.rosterUserIds(tripId);
+    const owned = this.dbs.all<{ id: number; user_id: number }>(
+      `SELECT id, user_id FROM tags WHERE id IN (${unique.map(() => '?').join(',')})`,
+      ...unique,
+    );
+    return owned.filter(t => roster.has(t.user_id)).map(t => t.id);
+  }
+
   broadcast<E extends TrekWsTripEventName>(tripId: string, event: E, payload: TrekWsPayload<E>, socketId: string | undefined): void {
     this.realtime.broadcast(tripId, event, payload, socketId);
   }
@@ -223,7 +246,7 @@ export class PlacesService {
 
     if (tags && tags.length > 0) {
       const insertTag = this.dbs.prepare('INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)');
-      for (const tagId of tags) {
+      for (const tagId of this.tagsOnTrip(tripId, tags)) {
         insertTag.run(placeId, tagId);
       }
     }
@@ -324,7 +347,7 @@ export class PlacesService {
       this.dbs.run('DELETE FROM place_tags WHERE place_id = ?', placeId);
       if (tags.length > 0) {
         const insertTag = this.dbs.prepare('INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)');
-        for (const tagId of tags) {
+        for (const tagId of this.tagsOnTrip(tripId, tags)) {
           insertTag.run(placeId, tagId);
         }
       }
diff --git a/server/tests/unit/nest/places.service.test.ts b/server/tests/unit/nest/places.service.test.ts
index e98decef5..615135716 100644
--- a/server/tests/unit/nest/places.service.test.ts
+++ b/server/tests/unit/nest/places.service.test.ts
@@ -64,7 +64,7 @@ const photoCacheStub = { removeIfUnreferenced: removeIfUnreferencedSpy } as unkn
 import { createTables } from '../../../src/db/schema';
 import { runMigrations } from '../../../src/db/migrations';
 import { resetTestDb } from '../../helpers/test-db';
-import { createUser, createTrip, createPlace, createCategory, createTag } from '../../helpers/factories';
+import { createUser, createTrip, createPlace, createCategory, createTag, addTripMember } from '../../helpers/factories';
 import path from 'path';
 import fs from 'fs';
 import { DatabaseService } from '../../../src/nest/database/database.service';
@@ -201,6 +201,33 @@ describe('create', () => {
     expect(place.tags[0].id).toBe(tag.id);
   });
 
+  // A tag belongs to a user, not a trip, and the place body is an open record, so
+  // an id from someone outside the trip could be attached and then read straight
+  // back — the tag projection carries the owner's user_id with it.
+  it('PLACE-SVC-008a — drops a tag owned by someone outside the trip', () => {
+    const { user } = createUser(testDb);
+    const { user: outsider } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const mine = createTag(testDb, user.id, { name: 'Mine' }) as any;
+    const theirs = createTag(testDb, outsider.id, { name: 'Theirs' }) as any;
+
+    const place = svc.create(String(trip.id), { name: 'Tagged Place', tags: [mine.id, theirs.id] }) as any;
+
+    expect(place.tags.map((t: any) => t.id)).toEqual([mine.id]);
+  });
+
+  it('PLACE-SVC-008b — keeps a co-traveller tag, so a shared place survives a foreign re-save', () => {
+    const { user: owner } = createUser(testDb);
+    const { user: member } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    addTripMember(testDb, trip.id, member.id);
+    const theirTag = createTag(testDb, member.id, { name: 'Theirs' }) as any;
+
+    const place = svc.create(String(trip.id), { name: 'Shared Place', tags: [theirTag.id] }) as any;
+
+    expect(place.tags.map((t: any) => t.id)).toEqual([theirTag.id]);
+  });
+
   it('PLACE-SVC-009 — place is associated with correct trip', () => {
     const { user } = createUser(testDb);
     const trip = createTrip(testDb, user.id);