mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-23 07:26:30 +00:00
fix(packing): authorize every single-item path against the item, not just the trip
A packing item is Common, Personal or Shared with named recipients. listItems() always applied that rule, but every path that touches one item resolved it by trip id alone, so trip membership plus packing_edit was enough to reach an item the member could not see. Update, delete, clone, the contributor routes and the sharing route all shared the lookup, and the same service methods sit behind the MCP tools and the plugin RPC, so all three transports carried the gap. The update was a read as well: it answered with the enriched row, which hands back the name, category, quantity, weight, the owner's username and the recipient list of an item the caller was never shown. Two flows laundered restricted items into places they do not belong. Saving a trip as a template read every row in the trip, so a template captured the names and categories of everyone's Personal items. Copying a trip did the same and re-inserted without is_private/owner_id, so both fell back to the column defaults and another member's Personal or Shared item reappeared in the copy as a Common item. That path needs only trip_create and trip access, not even packing_edit. There is now one visibility rule in the service and every single-item lookup goes through it. The actor is threaded through the HTTP, MCP and plugin-RPC entry points, and a missing actor denies rather than falling through unfiltered. An item the actor may not see is reported as missing, not as forbidden, so the routes cannot be used to confirm that an id exists. Two deliberate contract changes fall out of that. The sharing route answers 404 instead of 403 when the caller cannot see the item at all; a visible item they do not own still answers 403. And a template or a trip copy now carries the Common list plus the actor's own items, with a restricted item staying restricted and owned by the copier. Reported privately by Chichi (@vickyzer027-hash).
This commit is contained in:
@@ -151,7 +151,7 @@ export class PackingController {
|
||||
@Param('id') id: string,
|
||||
@Headers('x-socket-id') socketId?: string,
|
||||
) {
|
||||
const deleted = this.packing.deleteItem(tripId, id);
|
||||
const deleted = this.packing.deleteItem(tripId, id, user.id);
|
||||
if (!deleted) {
|
||||
throw new HttpException({ error: 'Item not found' }, 404);
|
||||
}
|
||||
|
||||
@@ -81,7 +81,7 @@ export class PackingMcp {
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess();
|
||||
if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied();
|
||||
const item = this.packing.updateItem(tripId, itemId, { checked: checked ? 1 : 0 }, ['checked']);
|
||||
const item = this.packing.updateItem(tripId, itemId, { checked: checked ? 1 : 0 }, ['checked'], undefined, ctx.userId);
|
||||
if (!item) return errorResult('Packing item not found.');
|
||||
this.guards.safeBroadcast(tripId, 'packing:updated', { item });
|
||||
return ok({ item });
|
||||
@@ -102,7 +102,7 @@ export class PackingMcp {
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess();
|
||||
if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied();
|
||||
const deleted = this.packing.deleteItem(tripId, itemId);
|
||||
const deleted = this.packing.deleteItem(tripId, itemId, ctx.userId);
|
||||
if (!deleted) return errorResult('Packing item not found.');
|
||||
this.guards.safeBroadcast(tripId, 'packing:deleted', { itemId });
|
||||
return ok({ success: true });
|
||||
@@ -128,7 +128,7 @@ export class PackingMcp {
|
||||
if (!this.packing.verifyTripAccess(tripId, ctx.userId)) return noAccess();
|
||||
if (!this.guards.hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied();
|
||||
const bodyKeys = ['name', 'category'].filter(k => k === 'name' ? name !== undefined : category !== undefined);
|
||||
const item = this.packing.updateItem(tripId, itemId, { name, category }, bodyKeys);
|
||||
const item = this.packing.updateItem(tripId, itemId, { name, category }, bodyKeys, undefined, ctx.userId);
|
||||
if (!item) return errorResult('Packing item not found.');
|
||||
this.guards.safeBroadcast(tripId, 'packing:updated', { item });
|
||||
return ok({ item });
|
||||
|
||||
@@ -76,7 +76,7 @@ export class PackingRpc {
|
||||
const itemId = num(params.itemId, 'itemId');
|
||||
const actor = this.guards.requireActor(ctx, 'packing item');
|
||||
this.guards.requireTripEdit(tripId, actor, PACKING_EDIT_ACTION);
|
||||
const deleted = this.packing.deleteItem(String(tripId), String(itemId)) as PrivacyItem | null;
|
||||
const deleted = this.packing.deleteItem(String(tripId), String(itemId), actor) as PrivacyItem | null;
|
||||
if (!deleted) throw new ForbiddenResource(`no packing item ${itemId} on trip ${tripId}`);
|
||||
this.packing.emitToViewers(String(tripId), 'packing:deleted', { itemId }, deleted, undefined);
|
||||
return { deleted: true };
|
||||
|
||||
@@ -247,7 +247,9 @@ export class PackingService {
|
||||
ifMatch?: string,
|
||||
actingUserId?: number,
|
||||
): unknown | UpdateConflict | null {
|
||||
const item = this.db.get<{ updated_at?: string | null; owner_id?: number | null }>('SELECT * FROM packing_items WHERE id = ? AND trip_id = ?', id, tripId);
|
||||
// Was a trip-scoped lookup, which let any member with packing_edit write to
|
||||
// another member's restricted item (and read it back off the response).
|
||||
const item = this.getItemInTrip(tripId, id, actingUserId);
|
||||
if (!item) return null;
|
||||
|
||||
// Optimistic concurrency (#1135): reject a stale offline overwrite. Absent
|
||||
@@ -295,10 +297,33 @@ export class PackingService {
|
||||
|
||||
// ── Three-tier sharing (#858): recipients, contributors, clone ───────────────
|
||||
|
||||
/** Loads an item scoped to its trip (the trip-access check happens in the controller). */
|
||||
private getItemInTrip(tripId: string | number, id: string | number) {
|
||||
return this.db.get<{ id: number; owner_id: number | null; is_private: number; name: string; category: string | null; quantity: number; weight_grams: number | null; bag_id: number | null }>(
|
||||
'SELECT * FROM packing_items WHERE id = ? AND trip_id = ?', id, tripId
|
||||
/**
|
||||
* The one visibility rule (#858), as a SQL fragment: Common belongs to the whole
|
||||
* trip, a restricted item belongs to its owner and to the people it was shared
|
||||
* with. Binds the actor id twice.
|
||||
*/
|
||||
private static readonly VISIBLE_TO_ACTOR = `(
|
||||
is_private = 0
|
||||
OR owner_id = ?
|
||||
OR EXISTS (SELECT 1 FROM packing_item_recipients r WHERE r.item_id = packing_items.id AND r.user_id = ?)
|
||||
)`;
|
||||
|
||||
/**
|
||||
* Loads an item scoped to its trip AND to what the actor may see.
|
||||
*
|
||||
* Trip membership used to be the whole check here, so any member holding
|
||||
* packing_edit could reach another member's Personal or Shared item by id
|
||||
* through update, delete, clone or the contributor routes. It resolves through
|
||||
* the visibility rule now, and an item the actor may not see comes back
|
||||
* undefined — deliberately indistinguishable from one that does not exist, so
|
||||
* these routes cannot be used to probe for ids. A missing actor denies too,
|
||||
* rather than falling through unfiltered.
|
||||
*/
|
||||
private getItemInTrip(tripId: string | number, id: string | number, actorId: number | undefined) {
|
||||
if (actorId == null) return undefined;
|
||||
return this.db.get<{ id: number; owner_id: number | null; is_private: number; name: string; category: string | null; quantity: number; weight_grams: number | null; bag_id: number | null; updated_at?: string | null }>(
|
||||
`SELECT * FROM packing_items WHERE id = ? AND trip_id = ? AND ${PackingService.VISIBLE_TO_ACTOR}`,
|
||||
id, tripId, actorId, actorId,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -313,7 +338,7 @@ export class PackingService {
|
||||
visibility: PackingVisibility,
|
||||
recipientIds: number[],
|
||||
) {
|
||||
const item = this.getItemInTrip(tripId, id);
|
||||
const item = this.getItemInTrip(tripId, id, actingUserId);
|
||||
if (!item) return null;
|
||||
// The owner controls sharing; an unowned legacy item is claimed by the actor.
|
||||
if (item.owner_id != null && item.owner_id !== actingUserId) return { forbidden: true as const };
|
||||
@@ -335,7 +360,7 @@ export class PackingService {
|
||||
|
||||
/** "I can bring that too" — adds the user as a co-contributor on a Common item. */
|
||||
addContributor(tripId: string | number, id: string | number, userId: number) {
|
||||
const item = this.getItemInTrip(tripId, id);
|
||||
const item = this.getItemInTrip(tripId, id, userId);
|
||||
if (!item || item.is_private !== 0) return null; // co-contribution is a Common-list concept
|
||||
if (item.owner_id === userId) return null; // the bringer is already covering it
|
||||
this.db.run("INSERT OR IGNORE INTO packing_item_contributors (item_id, user_id, status) VALUES (?, ?, 'accepted')", id, userId);
|
||||
@@ -343,7 +368,7 @@ export class PackingService {
|
||||
}
|
||||
|
||||
removeContributor(tripId: string | number, id: string | number, userId: number) {
|
||||
const item = this.getItemInTrip(tripId, id);
|
||||
const item = this.getItemInTrip(tripId, id, userId);
|
||||
if (!item) return null;
|
||||
this.db.run('DELETE FROM packing_item_contributors WHERE item_id = ? AND user_id = ?', id, userId);
|
||||
return this.enrichItems([this.db.get('SELECT * FROM packing_items WHERE id = ?', id)])[0];
|
||||
@@ -370,7 +395,7 @@ export class PackingService {
|
||||
* every traveller was the whole complaint in #207.
|
||||
*/
|
||||
cloneItem(tripId: string | number, id: string | number, userId: number) {
|
||||
const item = this.getItemInTrip(tripId, id);
|
||||
const item = this.getItemInTrip(tripId, id, userId);
|
||||
if (!item) return null;
|
||||
return this.createItem(tripId, {
|
||||
name: item.name,
|
||||
@@ -382,10 +407,12 @@ export class PackingService {
|
||||
}, userId);
|
||||
}
|
||||
|
||||
deleteItem(tripId: string | number, id: string | number) {
|
||||
deleteItem(tripId: string | number, id: string | number, actingUserId?: number) {
|
||||
// Return the deleted row (not just a boolean) so callers can target the
|
||||
// delete broadcast at the owner when the item was private (#858).
|
||||
const item = this.db.get<{ is_private?: number; owner_id?: number | null }>('SELECT * FROM packing_items WHERE id = ? AND trip_id = ?', id, tripId);
|
||||
// Scoped to what the actor may see: trip membership alone used to be enough
|
||||
// to delete another member's restricted item.
|
||||
const item = this.getItemInTrip(tripId, id, actingUserId);
|
||||
if (!item) return null;
|
||||
|
||||
this.db.run('DELETE FROM packing_items WHERE id = ?', id);
|
||||
@@ -577,8 +604,12 @@ export class PackingService {
|
||||
// ── Save as Template ──────────────────────────────────────────────────────
|
||||
|
||||
saveAsTemplate(tripId: string | number, userId: number, templateName: string) {
|
||||
// A template is a durable, shareable artifact, so it may only capture what is
|
||||
// the actor's to publish: the Common list plus their own items. It used to
|
||||
// take every row in the trip, restricted ones included.
|
||||
const items = this.db.all<{ name: string; category: string }>(
|
||||
'SELECT name, category FROM packing_items WHERE trip_id = ? ORDER BY sort_order ASC', tripId
|
||||
'SELECT name, category FROM packing_items WHERE trip_id = ? AND (is_private = 0 OR owner_id = ?) ORDER BY sort_order ASC',
|
||||
tripId, userId,
|
||||
);
|
||||
|
||||
if (items.length === 0) return null;
|
||||
|
||||
@@ -689,14 +689,24 @@ export class TripsService {
|
||||
bagMap.set(bag.id, r.lastInsertRowid);
|
||||
}
|
||||
|
||||
const oldPacking = this.db.prepare('SELECT * FROM packing_items WHERE trip_id = ?').all(sourceTripId) as any[];
|
||||
// Only what the copier may carry over: the Common list plus their own items.
|
||||
// This used to take every row and re-insert it without is_private/owner_id,
|
||||
// so both fell back to the column defaults and another member's Personal or
|
||||
// Shared item reappeared in the copy as a Common item visible to everyone.
|
||||
// A restricted item stays restricted, and it stays owned by the copier —
|
||||
// recipient rows are not carried over, and the copy has its own roster.
|
||||
const oldPacking = this.db.prepare(
|
||||
'SELECT * FROM packing_items WHERE trip_id = ? AND (is_private = 0 OR owner_id = ?)'
|
||||
).all(sourceTripId, newOwnerId) as any[];
|
||||
const insertPacking = this.db.prepare(`
|
||||
INSERT INTO packing_items (trip_id, name, checked, category, sort_order, weight_grams, bag_id, updated_at)
|
||||
VALUES (?, ?, 0, ?, ?, ?, ?, CURRENT_TIMESTAMP)
|
||||
INSERT INTO packing_items (trip_id, name, checked, category, sort_order, weight_grams, bag_id, is_private, owner_id, updated_at)
|
||||
VALUES (?, ?, 0, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)
|
||||
`);
|
||||
for (const p of oldPacking) {
|
||||
const isPrivate = p.is_private ? 1 : 0;
|
||||
insertPacking.run(newTripId, p.name, p.category, p.sort_order, p.weight_grams,
|
||||
p.bag_id ? (bagMap.get(p.bag_id) ?? null) : null);
|
||||
p.bag_id ? (bagMap.get(p.bag_id) ?? null) : null,
|
||||
isPrivate, isPrivate ? newOwnerId : null);
|
||||
}
|
||||
|
||||
const oldNotes = this.db.prepare('SELECT * FROM day_notes WHERE trip_id = ?').all(sourceTripId) as any[];
|
||||
|
||||
@@ -257,7 +257,14 @@ describe('Three-tier packing sharing (#858)', () => {
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
const created = await request(app).post(`/api/trips/${trip.id}/packing`).set('Cookie', authCookie(owner.id)).send({ name: 'Tent', visibility: 'personal' });
|
||||
|
||||
const denied = await request(app).put(`/api/trips/${trip.id}/packing/${created.body.item.id}/sharing`).set('Cookie', authCookie(member.id)).send({ visibility: 'common' });
|
||||
// A member who cannot see the item at all gets 404, not 403: answering 403
|
||||
// would confirm that the id exists (GHSA-vh2h-288v-ggch).
|
||||
const hidden = await request(app).put(`/api/trips/${trip.id}/packing/${created.body.item.id}/sharing`).set('Cookie', authCookie(member.id)).send({ visibility: 'common' });
|
||||
expect(hidden.status).toBe(404);
|
||||
|
||||
// An item the member CAN see but does not own still answers 403.
|
||||
const shared = await request(app).post(`/api/trips/${trip.id}/packing`).set('Cookie', authCookie(owner.id)).send({ name: 'Stove', visibility: 'common' });
|
||||
const denied = await request(app).put(`/api/trips/${trip.id}/packing/${shared.body.item.id}/sharing`).set('Cookie', authCookie(member.id)).send({ visibility: 'personal' });
|
||||
expect(denied.status).toBe(403);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -474,11 +474,11 @@ describe('private items (#858)', () => {
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const item = svc.createItem(trip.id, { name: 'Private', is_private: true }, user.id) as any;
|
||||
|
||||
const deleted = svc.deleteItem(trip.id, item.id) as any;
|
||||
const deleted = svc.deleteItem(trip.id, item.id, user.id) as any;
|
||||
expect(deleted).not.toBeNull();
|
||||
expect(deleted.is_private).toBe(1);
|
||||
expect(deleted.owner_id).toBe(user.id);
|
||||
expect(svc.deleteItem(trip.id, item.id)).toBeNull();
|
||||
expect(svc.deleteItem(trip.id, item.id, user.id)).toBeNull();
|
||||
});
|
||||
|
||||
it('PACK-SVC-018: bulkImport stamps the owner on every item', () => {
|
||||
@@ -541,8 +541,9 @@ describe('three-tier packing sharing (#858)', () => {
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const item = svc.createItem(trip.id, { name: 'First aid', visibility: 'personal' }, owner.id) as any;
|
||||
|
||||
// A non-owner is rejected.
|
||||
expect((svc.setItemSharing(trip.id, item.id, friend.id, 'shared', [friend.id]) as any).forbidden).toBe(true);
|
||||
// A non-owner who cannot see the item at all gets the missing-item answer, so
|
||||
// the route cannot be used to confirm that the id exists (GHSA-vh2h-288v-ggch).
|
||||
expect(svc.setItemSharing(trip.id, item.id, friend.id, 'shared', [friend.id])).toBeNull();
|
||||
|
||||
const updated = svc.setItemSharing(trip.id, item.id, owner.id, 'shared', [friend.id]) as any;
|
||||
expect(updated.recipients.map((r: any) => r.user_id)).toEqual([friend.id]);
|
||||
@@ -675,10 +676,10 @@ describe('legacy-quirk fixes', () => {
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const item = svc.createItem(trip.id, { name: 'Socks', quantity: 5 }, user.id) as any;
|
||||
|
||||
expect((svc.updateItem(trip.id, item.id, { quantity: 0 }, ['quantity']) as any).quantity).toBe(1);
|
||||
expect((svc.updateItem(trip.id, item.id, { quantity: 9999 }, ['quantity']) as any).quantity).toBe(999);
|
||||
expect((svc.updateItem(trip.id, item.id, { quantity: 0 }, ['quantity'], undefined, user.id) as any).quantity).toBe(1);
|
||||
expect((svc.updateItem(trip.id, item.id, { quantity: 9999 }, ['quantity'], undefined, user.id) as any).quantity).toBe(999);
|
||||
// Omitted key leaves the quantity unchanged.
|
||||
expect((svc.updateItem(trip.id, item.id, { name: 'Wool socks' }, ['name']) as any).quantity).toBe(999);
|
||||
expect((svc.updateItem(trip.id, item.id, { name: 'Wool socks' }, ['name'], undefined, user.id) as any).quantity).toBe(999);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -991,3 +992,84 @@ describe('Template item scoping (post-fold quirk fix)', () => {
|
||||
expect(svc.deleteTemplateItem(String(tplA.template.id), String(item.item.id)) as any).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Object-level authorization on a single packing item (GHSA-vh2h-288v-ggch).
|
||||
*
|
||||
* listItems() always filtered by the three-tier rule, but every single-item path
|
||||
* resolved the row by trip id alone, so any member holding packing_edit could
|
||||
* reach another member's Personal or Shared item by id. The update response even
|
||||
* handed the enriched row back, which made the write a read as well.
|
||||
*/
|
||||
describe('packing item object-level authorization', () => {
|
||||
const restrictedTrip = () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: intruder } = createUser(testDb);
|
||||
const { user: friend } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.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;
|
||||
return { owner, intruder, friend, trip, personal, shared, common };
|
||||
};
|
||||
|
||||
it('PACK-SVC-101: a non-viewer cannot update someone else\'s Personal item', () => {
|
||||
const { trip, personal, intruder } = restrictedTrip();
|
||||
expect(svc.updateItem(trip.id, personal.id, { name: 'pwned' }, ['name'], undefined, intruder.id)).toBeNull();
|
||||
expect(testDb.prepare('SELECT name FROM packing_items WHERE id = ?').get(personal.id)).toEqual({ name: 'Diary' });
|
||||
});
|
||||
|
||||
it('PACK-SVC-102: a non-recipient cannot update a Shared item', () => {
|
||||
const { trip, shared, intruder } = restrictedTrip();
|
||||
expect(svc.updateItem(trip.id, shared.id, { name: 'pwned' }, ['name'], undefined, intruder.id)).toBeNull();
|
||||
});
|
||||
|
||||
it('PACK-SVC-103: a non-viewer cannot delete a restricted item', () => {
|
||||
const { trip, personal, intruder } = restrictedTrip();
|
||||
expect(svc.deleteItem(trip.id, personal.id, intruder.id)).toBeNull();
|
||||
expect(testDb.prepare('SELECT COUNT(*) AS n FROM packing_items WHERE id = ?').get(personal.id)).toEqual({ n: 1 });
|
||||
});
|
||||
|
||||
it('PACK-SVC-104: a non-viewer cannot clone a restricted item into their own list', () => {
|
||||
const { trip, personal, intruder } = restrictedTrip();
|
||||
expect(svc.cloneItem(trip.id, personal.id, intruder.id)).toBeNull();
|
||||
});
|
||||
|
||||
it('PACK-SVC-105b: a visible item the caller does not own still reports forbidden, not missing', () => {
|
||||
const { trip, common, intruder } = restrictedTrip();
|
||||
expect((svc.setItemSharing(trip.id, common.id, intruder.id, 'personal', []) as any).forbidden).toBe(true);
|
||||
});
|
||||
|
||||
it('PACK-SVC-105: a non-owner cannot re-share a restricted item they cannot see', () => {
|
||||
const { trip, personal, intruder } = restrictedTrip();
|
||||
expect(svc.setItemSharing(trip.id, personal.id, intruder.id, 'common', [])).toBeNull();
|
||||
expect(testDb.prepare('SELECT is_private FROM packing_items WHERE id = ?').get(personal.id)).toEqual({ is_private: 1 });
|
||||
});
|
||||
|
||||
// Missing actor must deny rather than fall through unfiltered.
|
||||
it('PACK-SVC-106: an update with no actor at all is refused', () => {
|
||||
const { trip, personal } = restrictedTrip();
|
||||
expect(svc.updateItem(trip.id, personal.id, { name: 'pwned' }, ['name'])).toBeNull();
|
||||
expect(svc.deleteItem(trip.id, personal.id)).toBeNull();
|
||||
});
|
||||
|
||||
it('PACK-SVC-107: the owner and the recipients keep working', () => {
|
||||
const { trip, personal, shared, common, owner, friend, intruder } = restrictedTrip();
|
||||
expect(svc.updateItem(trip.id, personal.id, { name: 'Diary v2' }, ['name'], undefined, owner.id)).toBeTruthy();
|
||||
expect(svc.updateItem(trip.id, shared.id, { checked: 1 }, ['checked'], undefined, friend.id)).toBeTruthy();
|
||||
expect(svc.updateItem(trip.id, common.id, { checked: 1 }, ['checked'], undefined, intruder.id)).toBeTruthy();
|
||||
expect(svc.cloneItem(trip.id, common.id, intruder.id)).toBeTruthy();
|
||||
expect(svc.deleteItem(trip.id, personal.id, owner.id)).toBeTruthy();
|
||||
});
|
||||
|
||||
it('PACK-SVC-108: a template captures only the Common list and the actor\'s own items', () => {
|
||||
const { trip, intruder } = restrictedTrip();
|
||||
const templateId = (svc.saveAsTemplate(trip.id, intruder.id, 'Snapshot') as { id: number }).id;
|
||||
const rows = testDb.prepare(`
|
||||
SELECT i.name FROM packing_template_items i
|
||||
JOIN packing_template_categories c ON c.id = i.category_id
|
||||
WHERE c.template_id = ?
|
||||
`).all(templateId) as { name: string }[];
|
||||
expect(rows.map(r => r.name).sort()).toEqual(['Tent']);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -747,6 +747,31 @@ describe('folded trip CRUD', () => {
|
||||
expect((testDb.prepare('SELECT title FROM trips WHERE id = ?').get(secondCopy) as any).title).toBe('Origin');
|
||||
});
|
||||
|
||||
/**
|
||||
* Copying a trip used to take every packing row and re-insert it without
|
||||
* is_private/owner_id, so both fell back to the column defaults and another
|
||||
* member's Personal or Shared item reappeared in the copy as a Common item
|
||||
* that everyone on the new trip could read (GHSA-vh2h-288v-ggch).
|
||||
*/
|
||||
it("TRIP-SVC-046b: copy leaves other members' restricted packing items behind", () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id, { title: 'Origin', start_date: '2025-06-01', end_date: '2025-06-02' });
|
||||
const ins = testDb.prepare('INSERT INTO packing_items (trip_id, name, checked, is_private, owner_id) VALUES (?, ?, 0, ?, ?)');
|
||||
ins.run(trip.id, 'Shared tent', 0, null); // Common
|
||||
ins.run(trip.id, "Owner's diary", 1, owner.id); // the owner's Personal
|
||||
ins.run(trip.id, "Member's meds", 1, member.id); // the copier's own Personal
|
||||
|
||||
const newTripId = svc.copy(trip.id, member.id, 'Copy');
|
||||
const rows = testDb.prepare('SELECT name, is_private, owner_id FROM packing_items WHERE trip_id = ? ORDER BY name').all(newTripId) as any[];
|
||||
|
||||
// The owner's private row is gone, not relabelled as Common.
|
||||
expect(rows.map(r => r.name)).toEqual(["Member's meds", 'Shared tent']);
|
||||
expect(rows.find(r => r.name === 'Shared tent')).toMatchObject({ is_private: 0, owner_id: null });
|
||||
// The copier's own item stays restricted and belongs to them in the copy.
|
||||
expect(rows.find(r => r.name === "Member's meds")).toMatchObject({ is_private: 1, owner_id: member.id });
|
||||
});
|
||||
|
||||
it('TRIP-SVC-059: copy remaps cross-links and carries splits/participants (smoke-test I-01)', () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: friend } = createUser(testDb);
|
||||
|
||||
@@ -147,12 +147,12 @@ describe('updateItem (packing) — optimistic concurrency', () => {
|
||||
it('returns a conflict when the packing token is stale', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const item = packing.createItem(trip.id, { name: 'Socks' }) as { id: number; updated_at: string };
|
||||
const item = packing.createItem(trip.id, { name: 'Socks' }, user.id) as { id: number; updated_at: string };
|
||||
|
||||
const stale = packing.updateItem(trip.id, item.id, { name: 'Mine' }, ['name'], '1999-01-01 00:00:00');
|
||||
const stale = packing.updateItem(trip.id, item.id, { name: 'Mine' }, ['name'], '1999-01-01 00:00:00', user.id);
|
||||
expect(isUpdateConflict(stale)).toBe(true);
|
||||
|
||||
const fresh = packing.updateItem(trip.id, item.id, { name: 'Edited' }, ['name'], item.updated_at);
|
||||
const fresh = packing.updateItem(trip.id, item.id, { name: 'Edited' }, ['name'], item.updated_at, user.id);
|
||||
expect(isUpdateConflict(fresh)).toBe(false);
|
||||
expect((fresh as { name: string }).name).toBe('Edited');
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user