fix(budget): confine payers, split members and settlement parties to the trip

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

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

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

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

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

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

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', () => {