fix(journey): stop unshared trip photos leaking into journeys, and gate the link

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.
This commit is contained in:
Maurice
2026-08-18 20:02:26 +02:00
committed by Maurice
parent b267771772
commit 630e8efca1
2 changed files with 63 additions and 1 deletions
@@ -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;
@@ -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]);
});
});