fix(trips): confine assignee writes to the people on the trip

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

@@ -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);
   }

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);
       }
     });
 

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);
       }
     });
 

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;

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);