Files
TREK/server/tests/unit/services/conflictUpdate.test.ts
T
Maurice f23de88b74 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).
2026-08-14 22:09:21 +02:00

160 lines
6.6 KiB
TypeScript

/**
* Optimistic-concurrency / 409 conflict tests (#1135) for the place + packing
* update services. A matching If-Match token (or none) updates as before; a
* stale token returns the conflict sentinel carrying the server's current row.
*/
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
const { testDb, dbMock } = vi.hoisted(() => {
const Database = require('better-sqlite3');
const db = new Database(':memory:');
db.exec('PRAGMA journal_mode = WAL');
db.exec('PRAGMA foreign_keys = ON');
db.exec('PRAGMA busy_timeout = 5000');
function getPlaceWithTags(placeId: number | string) {
const p = db.prepare('SELECT * FROM places WHERE id = ?').get(placeId);
if (!p) return null;
return { ...(p as object), category: null, tags: [] };
}
const mock = {
db,
closeDb: () => {},
reinitialize: () => {},
getPlaceWithTags,
canAccessTrip: () => null,
isOwner: () => false,
};
return { testDb: db, dbMock: mock };
});
vi.mock('../../../src/db/database', () => dbMock);
vi.mock('../../../src/config', () => ({
JWT_SECRET: 'test-secret',
ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2',
updateJwtSecret: () => {},
}));
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 { isUpdateConflict } from '../../../src/nest/common/conflictResult';
import { DatabaseService } from '../../../src/nest/database/database.service';
import { PackingService } from '../../../src/nest/packing/packing.service';
import { PlacesService } from '../../../src/nest/places/places.service';
import { MapsService } from '../../../src/nest/maps/maps.service';
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
import { UnsplashService } from '../../../src/nest/unsplash/unsplash.service';
import { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service';
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service';
import { notificationsStub } from '../../helpers/notifications';
const dbs = new DatabaseService(testDb);
const realtime = new RealtimeService();
const runtimeEnv = new RuntimeEnvService();
// One cache instance shared by maps and places, the way the container wires it:
// the service's stampede guard only works if there is exactly one of them.
const photoCache = new PlacePhotoCacheService(dbs, runtimeEnv);
const packing = new PackingService(dbs, new PermissionsService(dbs), realtime, notificationsStub());
const places = new PlacesService(
dbs,
new PermissionsService(dbs),
realtime,
new MapsService(dbs, photoCache),
new QueryHelpersService(dbs),
new UnsplashService(dbs, runtimeEnv),
photoCache,
new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs)),
);
beforeAll(() => {
createTables(testDb);
runMigrations(testDb);
});
beforeEach(() => {
resetTestDb(testDb);
});
afterAll(() => {
testDb.close();
});
function freshPlace(tripId: number) {
const place = places.create(String(tripId), { name: 'Original' }) as unknown as { id: number; updated_at: string };
return place;
}
describe('PlacesService.update — optimistic concurrency', () => {
it('updates normally when no If-Match token is sent (back-compat)', () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const place = freshPlace(trip.id);
const result = places.update(String(trip.id), String(place.id), { name: 'Edited' });
expect(isUpdateConflict(result)).toBe(false);
expect((result as { name: string }).name).toBe('Edited');
});
it('updates when the If-Match token matches the current updated_at', () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const place = freshPlace(trip.id);
const result = places.update(String(trip.id), String(place.id), { name: 'Edited' }, place.updated_at);
expect(isUpdateConflict(result)).toBe(false);
expect((result as { name: string }).name).toBe('Edited');
});
it('returns a conflict (with the server row) when the token is stale', () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const place = freshPlace(trip.id);
const result = places.update(String(trip.id), String(place.id), { name: 'Mine' }, '1999-01-01 00:00:00');
expect(isUpdateConflict(result)).toBe(true);
if (isUpdateConflict(result)) {
expect((result.server as { name: string }).name).toBe('Original');
}
// The row must NOT have been overwritten.
const row = testDb.prepare('SELECT name FROM places WHERE id = ?').get(place.id) as { name: string };
expect(row.name).toBe('Original');
});
it('returns null for a place that does not exist', () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
expect(places.update(String(trip.id), '999999', { name: 'x' }, 'whatever')).toBeNull();
});
});
describe('updateItem (packing) — optimistic concurrency', () => {
it('migration added updated_at and createItem stamps it', () => {
const cols = testDb.prepare("PRAGMA table_info('packing_items')").all() as { name: string }[];
expect(cols.map(c => c.name)).toContain('updated_at');
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const item = packing.createItem(trip.id, { name: 'Socks' }) as { id: number; updated_at: string | null };
expect(item.updated_at).toBeTruthy();
});
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' }, 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', user.id);
expect(isUpdateConflict(stale)).toBe(true);
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');
});
})