From 630e8efca1eb95fc7f3c07469a3b37dcf824ba20 Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 18 Aug 2026 20:02:26 +0200 Subject: [PATCH] fix(journey): stop unshared trip photos leaking into journeys, and gate the link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things found while reading the domain for discussion #1614. A photo a trip member left unshared is denied on the trip path — memories-access requires tp.shared = 1 — but linking that trip to a journey copied every row of trip_photos into the gallery regardless. From there it was readable by every journey contributor, and with an open gallery by anonymous visitors of the share link. journey_photos.shared cannot gate that after the fact: a direct upload writes 0 into the same column, so filtering on it would empty most galleries. The fix is at the door instead — only shared rows are copied in. addTripToJourney checked that the caller could reach the trip but never that they could reach the journey, so any logged-in user could link a trip of theirs into a stranger's journey and seed skeleton entries and photos there. The MCP tool has always checked this; only the REST route was open. Now the service checks it, which covers both and the internal call from createJourney. Neither path had a test. They do now. --- .../nest/journey/journey-domain.service.ts | 12 ++++- .../unit/nest/journey-domain.service.test.ts | 52 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/server/src/nest/journey/journey-domain.service.ts b/server/src/nest/journey/journey-domain.service.ts index 5658ca2b1..3f4967580 100644 --- a/server/src/nest/journey/journey-domain.service.ts +++ b/server/src/nest/journey/journey-domain.service.ts @@ -342,6 +342,10 @@ export class JourneyDomainService { // (cross-tenant leak). Mirrors the trip-access gate every other trip-scoped // path enforces. if (!this.db.canAccessTrip(tripId, userId)) return false; + // And a journey the caller can actually reach. Without this, any logged-in user + // could link a trip of theirs into a stranger's journey and seed entries and + // photos there — the MCP tool has always checked this, the REST route never did. + if (!this.canAccessJourney(journeyId, userId)) return false; const now = this.ts(); try { this.db.prepare('INSERT OR IGNORE INTO journey_trips (journey_id, trip_id, added_at) VALUES (?, ?, ?)').run( @@ -442,9 +446,15 @@ export class JourneyDomainService { } // import trip_photos into journey gallery when a trip is linked + // + // Only the ones the owner actually shared with the trip. A photo left unshared is + // denied on the trip path (memories-access checks tp.shared = 1), and copying it + // here handed it to every journey contributor and, with an open gallery, to + // anonymous visitors of the share link — journey_photos.shared cannot gate that + // afterwards, because a direct upload writes 0 into the same column. private syncTripPhotos(journeyId: number, tripId: number) { const tripPhotos = this.db - .prepare('SELECT tp.photo_id, tp.shared FROM trip_photos tp WHERE tp.trip_id = ?') + .prepare('SELECT tp.photo_id, tp.shared FROM trip_photos tp WHERE tp.trip_id = ? AND tp.shared = 1') .all(tripId) as { photo_id: number; shared: number }[]; if (!tripPhotos.length) return; diff --git a/server/tests/unit/nest/journey-domain.service.test.ts b/server/tests/unit/nest/journey-domain.service.test.ts index 9fedb93fb..9853a5ebc 100644 --- a/server/tests/unit/nest/journey-domain.service.test.ts +++ b/server/tests/unit/nest/journey-domain.service.test.ts @@ -2079,3 +2079,55 @@ describe('journeyTracks', () => { expect(svc.journeyTracks(journey.id, stranger.id)).toBeNull(); }); }); + +// ── Trip linking: whose journey, and whose photos (#1614 review) ───────────── + +describe('addTripToJourney guards', () => { + it('JOURNEY-SVC-100: refuses to link into a journey the caller cannot reach', () => { + const { user: owner } = createUser(testDb); + const { user: stranger } = createUser(testDb); + const journey = createJourney(testDb, owner.id, { title: "Owner's journey" }); + const trip = createTrip(testDb, stranger.id, { title: 'Stranger trip' }); + + // The stranger owns the trip, so the trip gate passes — only the journey gate stops this. + expect(svc.addTripToJourney(journey.id, trip.id, stranger.id)).toBe(false); + const links = testDb.prepare('SELECT * FROM journey_trips WHERE journey_id = ?').all(journey.id); + expect(links).toHaveLength(0); + }); + + it('JOURNEY-SVC-101: a contributor may still link, the owner obviously too', () => { + const { user: owner } = createUser(testDb); + const { user: helper } = createUser(testDb); + const journey = createJourney(testDb, owner.id, { title: 'Shared journey' }); + testDb.prepare('INSERT INTO journey_contributors (journey_id, user_id, role, added_at) VALUES (?, ?, ?, ?)') + .run(journey.id, helper.id, 'editor', new Date().toISOString()); + const trip = createTrip(testDb, helper.id, { title: 'Helper trip' }); + + expect(svc.addTripToJourney(journey.id, trip.id, helper.id)).toBe(true); + }); + + it('JOURNEY-SVC-102: only trip photos the owner shared are copied into the gallery', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id, { title: 'Photo journey' }); + const trip = createTrip(testDb, user.id, { title: 'Photo trip' }); + + const mk = (assetId: string) => { + const r = testDb.prepare( + "INSERT INTO trek_photos (provider, asset_id, owner_id, media_type) VALUES ('immich', ?, ?, 'image')", + ).run(assetId, user.id); + return Number(r.lastInsertRowid); + }; + const sharedPhoto = mk('shared-asset'); + const privatePhoto = mk('private-asset'); + const ins = testDb.prepare('INSERT INTO trip_photos (trip_id, user_id, photo_id, shared) VALUES (?, ?, ?, ?)'); + ins.run(trip.id, user.id, sharedPhoto, 1); + ins.run(trip.id, user.id, privatePhoto, 0); + + svc.addTripToJourney(journey.id, trip.id, user.id); + + const copied = testDb + .prepare('SELECT photo_id FROM journey_photos WHERE journey_id = ? ORDER BY photo_id') + .all(journey.id) as { photo_id: number }[]; + expect(copied.map(r => r.photo_id)).toEqual([sharedPhoto]); + }); +});