diff --git a/plugin-sdk/src/generated/host-facts.ts b/plugin-sdk/src/generated/host-facts.ts index 48db7e5eb..b9be32dec 100644 --- a/plugin-sdk/src/generated/host-facts.ts +++ b/plugin-sdk/src/generated/host-facts.ts @@ -155,6 +155,7 @@ export const METHOD_PERMISSION: Readonly> = { 'vacay.toggleEntry': 'db:write:vacay', 'vacay.toggleCompanyHoliday': 'db:write:vacay', 'journal.createEntry': 'db:write:journal', + 'journal.addEntryPhoto': 'db:write:journal', 'journal.updateEntry': 'db:write:journal', 'journal.deleteEntry': 'db:write:journal', 'journal.createJourney': 'db:write:journal', @@ -271,6 +272,7 @@ export const KNOWN_METHODS: string[] = [ 'journal.updateEntry', 'journal.deleteEntry', 'journal.createJourney', + 'journal.addEntryPhoto', 'journal.deleteJourney', 'weather.get', 'categories.list', diff --git a/plugin-sdk/src/index.ts b/plugin-sdk/src/index.ts index a2d79997f..34af7f6e9 100644 --- a/plugin-sdk/src/index.ts +++ b/plugin-sdk/src/index.ts @@ -237,6 +237,15 @@ export interface PluginContext { deleteJourney(journeyId: number): Promise<{ deleted: boolean }>; /** Update an entry (owner/contributor-gated). Needs `db:write:journal`. */ updateEntry(entryId: number, input: Record): Promise; + /** + * Attach a photo to an entry, bytes included. Needs `db:write:journal`. + * + * For an importer that holds an export archive: it has bytes, not a gallery + * photo to point at and not a provider asset. `name` supplies the extension + * only, the stored filename is the host's. Images only, no SVG, 10MB decoded, + * and the operator's allowed-file-types setting applies. + */ + addEntryPhoto(entryId: number, input: { name: string; content_base64: string; caption?: string }): Promise; /** Delete an entry (owner/contributor-gated). Needs `db:write:journal`. */ deleteEntry(entryId: number): Promise<{ deleted: boolean }>; }; diff --git a/plugin-sdk/src/mock-host.ts b/plugin-sdk/src/mock-host.ts index 2f743bb18..b6b0b5395 100644 --- a/plugin-sdk/src/mock-host.ts +++ b/plugin-sdk/src/mock-host.ts @@ -373,6 +373,10 @@ export function createMockHost(opts: MockHostOptions = {}): MockHost { const bucketItems: unknown[] = [...(opts.atlasBucketList ?? [])]; const journals: unknown[] = [...(opts.journals ?? [])]; const journalEntries: unknown[] = [...(opts.journalEntries ?? [])]; + // Photos a plugin attached in this session; the mock keeps them so a test can + // assert on what it wrote without a real gallery behind it. + const journalPhotos: unknown[] = []; + let journalPhotoSeq = 1; const savedPlaces: unknown[] = []; const vacayEntries = new Set(); const vacayHolidays = new Set(); @@ -1010,6 +1014,24 @@ export function createMockHost(opts: MockHostOptions = {}): MockHost { journalEntries.push(entry); return entry; }, + async addEntryPhoto(entryId, input) { + need('db:write:journal', 'journal.addEntryPhoto'); + requireActingUser(); + requireAddon(opts.journeyAddonEnabled, 'journey'); + const entry = rows(journalEntries).find((x) => x.id === entryId); + if (!entry) throw new Error(`RESOURCE_FORBIDDEN: no editable journal entry ${entryId} for this user`); + if (typeof input?.name !== 'string' || input.name.trim() === '') throw new Error('invalid photo input: name is required'); + if (typeof input?.content_base64 !== 'string' || input.content_base64 === '') throw new Error('invalid photo input: content_base64 is required'); + // Same refusals the host applies, so a plugin fails here rather than in production. + const ext = (input.name.slice(input.name.lastIndexOf('.')) || '').toLowerCase(); + if (!['.jpg', '.jpeg', '.png', '.gif', '.webp', '.avif', '.heic', '.heif'].includes(ext)) { + throw new Error(`photo extension '${ext || '(none)'}' is not an allowed image type`); + } + if (input.content_base64.length > 14 * 1024 * 1024) throw new Error('photo exceeds the 10MB plugin upload cap'); + const photo = { id: journalPhotoSeq++, photo_id: journalPhotoSeq, entry_id: entryId, caption: input.caption }; + journalPhotos.push(photo); + return photo; + }, async createJourney(input) { need('db:write:journal', 'journal.createJourney'); requireActingUser(); diff --git a/server/src/nest/journey/journal-rpc.module.ts b/server/src/nest/journey/journal-rpc.module.ts new file mode 100644 index 000000000..d11e363d8 --- /dev/null +++ b/server/src/nest/journey/journal-rpc.module.ts @@ -0,0 +1,28 @@ +import { Module } from '@nestjs/common'; +import { AllowedFileTypesModule } from '../files/allowed-file-types.module'; +import { MemoriesModule } from '../memories/memories.module'; +import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; +import { StorageModule } from '../storage/storage.module'; +import { JourneyDomainModule } from './journey-domain.module'; +import { JournalRpc } from './journal.rpc'; + +/** + * The journal plugin surface, in its own container. + * + * JournalRpc used to sit in JourneyDomainModule, and it cannot stay there now + * that it writes photo bytes: that needs StorageService and, for the EXIF + * backfill, PhotoCaptureBackfillService out of MemoriesModule. JourneyDomainModule + * exists precisely so places, assignments and the plugin host can reach a few + * journey functions WITHOUT dragging MemoriesModule and both photo providers into + * their graphs (see the comment there), so the dependency goes here instead and + * only the plugin host pays for it. + * + * JourneyDomainModule is re-exported so importing this one is a superset of + * importing that one, and nothing that already depended on it has to change. + */ +@Module({ + imports: [JourneyDomainModule, StorageModule, AllowedFileTypesModule, MemoriesModule, PluginGuardsModule], + providers: [JournalRpc], + exports: [JourneyDomainModule], +}) +export class JournalRpcModule {} diff --git a/server/src/nest/journey/journal.rpc.ts b/server/src/nest/journey/journal.rpc.ts index 8a86bbba6..f84fb3fa3 100644 --- a/server/src/nest/journey/journal.rpc.ts +++ b/server/src/nest/journey/journal.rpc.ts @@ -1,11 +1,35 @@ +import pathMod from 'node:path'; +import { randomUUID } from 'node:crypto'; +import { Readable } from 'node:stream'; +import { journalPluginPhotoInputSchema } from '@trek/shared'; import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators'; import { PluginGuards } from '../plugins/host/plugin-guards.service'; import { BadParams, ForbiddenResource } from '../plugins/host/rpc-errors'; import { asPayload, num } from '../plugins/host/rpc-params'; import type { PluginRpcContext } from '../plugins/host/rpc-kit/types'; import { ADDON_IDS } from '../../addons'; +import { readEnv } from '../../app-config'; +import { isDemoEmail } from '../common/demo'; +import { AllowedFileTypesService } from '../files/allowed-file-types.service'; +import { PhotoCaptureBackfillService } from '../memories/photo-capture-backfill.service'; +import { DatabaseService } from '../database/database.service'; +import { StorageService } from '../storage/storage.service'; import { JourneyDomainService } from './journey-domain.service'; +/** 10MB decoded, the same cap the file surface applies to plugin uploads. */ +const PHOTO_CONTENT_MAX = 10 * 1024 * 1024; + +/** + * Images only, and no SVG: the REST filter refuses it because an SVG is a + * document that executes, and a gallery renders what it is given. + */ +const IMAGE_EXTENSIONS = ['.jpg', '.jpeg', '.png', '.gif', '.webp', '.avif', '.heic', '.heif']; + +const MIME_BY_EXT: Record = { + '.jpg': 'image/jpeg', '.jpeg': 'image/jpeg', '.png': 'image/png', '.gif': 'image/gif', + '.webp': 'image/webp', '.avif': 'image/avif', '.heic': 'image/heic', '.heif': 'image/heif', +}; + /** * The journal surface a plugin may reach (#plugins). * @@ -19,6 +43,10 @@ export class JournalRpc { constructor( private readonly journey: JourneyDomainService, private readonly guards: PluginGuards, + private readonly storage: StorageService, + private readonly allowedTypes: AllowedFileTypesService, + private readonly captureBackfill: PhotoCaptureBackfillService, + private readonly db: DatabaseService, ) {} @PluginMethod('journal.listMine', { permission: 'db:read:journal' }) @@ -87,6 +115,78 @@ export class JournalRpc { }); } + /** + * Attach a photo to an entry, bytes and all (#1365). + * + * A journal importer can create the entry and write its story over the methods + * above, but there was no way to give it a picture: uploading the bytes as a + * trip file produces a file on a TRIP, and a journey photo lives in the + * journey's own gallery. This is the missing half. + * + * Deliberately byte-based rather than a link to an existing photo: an importer + * reading an export archive holds bytes and nothing else. It has no gallery + * photo to point at, and provider photos (Immich, Synology) need an asset that + * only exists once somebody has already uploaded it there. + * + * The access decision stays where it is: addPhoto answers null unless the + * acting user may edit the journey the entry belongs to. + */ + @PluginMethod('journal.addEntryPhoto', { permission: 'db:write:journal' }) + async addEntryPhoto(params: Record, ctx: PluginRpcContext): Promise { + const userId = this.requireJournalUser(ctx, 'writes'); + const entryId = num(params.entryId, 'entryId'); + // num() accepts "3" and 1.5; a row id is neither. + if (!Number.isInteger(entryId) || entryId <= 0) throw new BadParams('entryId must be a positive integer'); + this.requireJourneyAddon(); + + const parsed = journalPluginPhotoInputSchema.safeParse(params.input); + if (!parsed.success) throw new BadParams(`invalid photo input: ${parsed.error.issues[0]?.message ?? 'unknown'}`); + const input = parsed.data; + + // Mirrors the REST upload guard: a demo user must not write bytes to the + // shared demo instance, not even through a plugin's db:write:journal. + if (readEnv().demo.enabled) { + const uploader = this.db.prepare('SELECT email FROM users WHERE id = ?').get(userId) as { email?: string } | undefined; + if (isDemoEmail(uploader?.email)) throw new ForbiddenResource('Uploads are disabled in demo mode.'); + } + + // basename first: a name is a name, never a path. + const original = pathMod.basename(input.name); + const ext = pathMod.extname(original).toLowerCase(); + if (!IMAGE_EXTENSIONS.includes(ext)) { + throw new BadParams(`photo extension '${ext || '(none)'}' is not an allowed image type`); + } + // The operator's allow-list gates this path too, or the RPC would be the way + // around an admin setting that the REST upload obeys (journeyImageFileFilter). + const allowed = this.allowedTypes.get().split(',').map((e) => e.trim().toLowerCase()); + if (!allowed.includes('*') && !allowed.includes(ext.slice(1))) { + throw new BadParams(`file type ${ext} is not allowed`); + } + + const buf = Buffer.from(input.content_base64, 'base64'); + if (buf.length === 0) throw new BadParams('photo content is empty'); + if (buf.length > PHOTO_CONTENT_MAX) throw new BadParams('photo exceeds the 10MB plugin upload cap'); + + // The stored name is ours, never the plugin's: the gallery dedupes on + // file_path, so a chosen path could collide with somebody else's row. + const filename = `${randomUUID()}${ext}`; + // Same order as the REST upload: the object is committed before any row + // points at it, so a failure here orphans a blob rather than leaving a + // gallery entry aimed at bytes that never arrived. + await this.storage.put('journey', filename, Readable.from(buf), { contentType: MIME_BY_EXT[ext] ?? 'image/jpeg' }); + + const photo = this.journey.addPhoto(entryId, userId, `journey/${filename}`, undefined, input.caption); + if (!photo) { + // Nothing references the object now, and nothing ever would. + await this.storage.delete('journey', filename).catch(() => {}); + throw new ForbiddenResource(`no editable journal entry ${entryId} for this user`); + } + // Best-effort, exactly as the REST route does it: reads EXIF so the photo + // carries its capture date. + this.captureBackfill.schedule([photo.photo_id].filter((id): id is number => typeof id === 'number'), userId); + return photo; + } + @PluginMethod('journal.deleteJourney', { permission: 'db:write:journal' }) deleteJourney(params: Record, ctx: PluginRpcContext): unknown { const userId = this.requireJournalUser(ctx, 'writes'); diff --git a/server/src/nest/journey/journey-domain.module.ts b/server/src/nest/journey/journey-domain.module.ts index 0a5f5b8a4..1240ac45e 100644 --- a/server/src/nest/journey/journey-domain.module.ts +++ b/server/src/nest/journey/journey-domain.module.ts @@ -2,7 +2,6 @@ import { Module } from '@nestjs/common'; import { RealtimeModule } from '../realtime/realtime.module'; import { TrekPhotosModule } from '../photos/trek-photos.module'; import { JourneyDomainService } from './journey-domain.service'; -import { JournalRpc } from './journal.rpc'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; import { JourneyShareService } from './journey-share.service'; import { SettingsModule } from '../settings/settings.module'; @@ -15,6 +14,10 @@ import { SettingsModule } from '../settings/settings.module'; * (and with it both photo providers) into their graphs. This carries the two * services and nothing else - same shape as TrekPhotosModule. * + * JournalRpc moved out to JournalRpcModule when it started writing photo bytes: + * that needs StorageService and MemoriesModule, and pulling those in here would + * undo the whole point of this module. + * * RealtimeModule is imported explicitly even though it is @Global, so a * single-domain e2e TestingModule can still resolve the broadcast. * @@ -25,7 +28,7 @@ import { SettingsModule } from '../settings/settings.module'; */ @Module({ imports: [RealtimeModule, TrekPhotosModule, PluginGuardsModule, SettingsModule], - providers: [JourneyDomainService, JourneyShareService, JournalRpc], + providers: [JourneyDomainService, JourneyShareService], exports: [JourneyDomainService, JourneyShareService], }) export class JourneyDomainModule {} diff --git a/server/src/nest/plugins/plugins-runtime.module.ts b/server/src/nest/plugins/plugins-runtime.module.ts index ea3706c9f..3572a3ac7 100644 --- a/server/src/nest/plugins/plugins-runtime.module.ts +++ b/server/src/nest/plugins/plugins-runtime.module.ts @@ -36,7 +36,7 @@ import { CollectionsModule } from '../collections/collections.module'; import { AtlasModule } from '../atlas/atlas.module'; import { NotificationsModule } from '../notifications/notifications.module'; import { TripMembershipModule } from '../trip-membership/trip-membership.module'; -import { JourneyDomainModule } from '../journey/journey-domain.module'; +import { JournalRpcModule } from '../journey/journal-rpc.module'; /** * The plugin execution half (#plugins, M1/M2): the process supervisor, the @@ -66,7 +66,7 @@ import { JourneyDomainModule } from '../journey/journey-domain.module'; TodoModule, PackingModule, DaysModule, DayNotesModule, AccommodationsModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule, TripsModule, PlacesModule, PermissionsModule, AuditModule, AddonsModule, CollectionsModule, AtlasModule, - NotificationsModule, TripMembershipModule, JourneyDomainModule, + NotificationsModule, TripMembershipModule, JournalRpcModule, ], providers: [ PluginsService, diff --git a/server/src/nest/plugins/protocol/envelope.ts b/server/src/nest/plugins/protocol/envelope.ts index 54a084541..65aecb9d1 100644 --- a/server/src/nest/plugins/protocol/envelope.ts +++ b/server/src/nest/plugins/protocol/envelope.ts @@ -126,6 +126,7 @@ export const KNOWN_METHODS = [ 'journal.updateEntry', 'journal.deleteEntry', 'journal.createJourney', + 'journal.addEntryPhoto', 'journal.deleteJourney', 'weather.get', 'categories.list', @@ -261,6 +262,7 @@ export const METHOD_PERMISSION = { 'vacay.toggleEntry': 'db:write:vacay', 'vacay.toggleCompanyHoliday': 'db:write:vacay', 'journal.createEntry': 'db:write:journal', + 'journal.addEntryPhoto': 'db:write:journal', 'journal.updateEntry': 'db:write:journal', 'journal.deleteEntry': 'db:write:journal', 'journal.createJourney': 'db:write:journal', diff --git a/server/src/nest/plugins/runtime/plugin-sdk.ts b/server/src/nest/plugins/runtime/plugin-sdk.ts index bc8ce59ba..2df10cb03 100644 --- a/server/src/nest/plugins/runtime/plugin-sdk.ts +++ b/server/src/nest/plugins/runtime/plugin-sdk.ts @@ -203,6 +203,15 @@ export interface PluginContext { getEntries(journeyId: number): Promise; /** Create an entry on a journey the acting user can edit. Needs 'db:write:journal'. */ createEntry(journeyId: number, input: { entry_date: string; [k: string]: unknown }): Promise; + /** + * Attach a photo to an entry, bytes included. Needs 'db:write:journal'. + * + * For an importer that holds an export archive: it has bytes, not a gallery + * photo to point at and not a provider asset. 'name' supplies the extension + * only, the stored filename is the host's. Images only, no SVG, 10MB decoded, + * and the operator's allowed-file-types setting applies. + */ + addEntryPhoto(entryId: number, input: { name: string; content_base64: string; caption?: string }): Promise; /** Update an entry (owner/contributor-gated). Needs 'db:write:journal'. */ updateEntry(entryId: number, input: Record): Promise; /** Delete an entry (owner/contributor-gated). Needs 'db:write:journal'. */ @@ -836,6 +845,7 @@ export function createPluginContext( listMine: () => t.rpc('journal.listMine', { _inv: invocationId }) as Promise, getEntries: (journeyId) => t.rpc('journal.getEntries', { journeyId, _inv: invocationId }) as Promise, createEntry: (journeyId, input) => t.rpc('journal.createEntry', { journeyId, input, _inv: invocationId }), + addEntryPhoto: (entryId, input) => t.rpc('journal.addEntryPhoto', { entryId, input, _inv: invocationId }), updateEntry: (entryId, input) => t.rpc('journal.updateEntry', { entryId, input, _inv: invocationId }), deleteEntry: (entryId) => t.rpc('journal.deleteEntry', { entryId, _inv: invocationId }) as Promise<{ deleted: boolean }>, createJourney: (input) => t.rpc('journal.createJourney', { input, _inv: invocationId }), diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 0d046501c..a9341fd56 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -134,7 +134,9 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF new CollabRpc(collab, realtime, guards), new AtlasRpc(atlas, guards), new VacayRpc(vacay, guards), - new JournalRpc(journey, guards), + // The photo half needs storage plus the allowed-types setting and the EXIF + // backfill; none of the tests on this harness write bytes, so they are stubs. + new JournalRpc(journey, guards, generalStorage, { get: () => '*' } as never, { schedule: () => {} } as never, dbs), new CollectionsRpc(collections, guards), new DbRpc(new PluginUserSettingsService(dbs)), new MetaRpc(dbs, guards), diff --git a/server/tests/helpers/rpc-host-deps.ts b/server/tests/helpers/rpc-host-deps.ts index f62dbd24d..5f63ba54e 100644 --- a/server/tests/helpers/rpc-host-deps.ts +++ b/server/tests/helpers/rpc-host-deps.ts @@ -78,7 +78,7 @@ export function allRpcControllers(): object[] { new CollabRpc(anyService(), anyService(), anyService()), new AtlasRpc(anyService(), anyService()), new VacayRpc(anyService(), anyService()), - new JournalRpc(anyService(), anyService()), + new JournalRpc(anyService(), anyService(), anyService(), anyService(), anyService(), anyService()), new CollectionsRpc(anyService(), anyService()), new DbRpc(anyService()), new MetaRpc(anyService(), anyService()), diff --git a/server/tests/unit/plugins/addon-domains-errors.rpc.test.ts b/server/tests/unit/plugins/addon-domains-errors.rpc.test.ts index 9b11a22ba..1290b5ba2 100644 --- a/server/tests/unit/plugins/addon-domains-errors.rpc.test.ts +++ b/server/tests/unit/plugins/addon-domains-errors.rpc.test.ts @@ -51,7 +51,7 @@ function build(overrides: { new CollabRpc({ createNote: vi.fn(() => ({})), createPoll: vi.fn(() => ({})), votePoll: vi.fn(() => ({ error: null, poll: {} })), createMessage: vi.fn(() => ({ error: null, message: {} })), listNotes: vi.fn(() => []), listPolls: vi.fn(() => []), listMessages: vi.fn(() => []), ...overrides.collab } as never, realtime, guards), new AtlasRpc({ listVisitedCountries: vi.fn(() => []), listManuallyVisitedRegions: vi.fn(() => []), bucketList: vi.fn(() => []), markCountry: vi.fn(), unmarkCountry: vi.fn(), markRegion: vi.fn(), unmarkRegion: vi.fn(), createBucketItem: vi.fn(() => ({})), deleteBucketItem: vi.fn(() => true), ...overrides.atlas } as never, guards), new VacayRpc({ getPlanData: vi.fn(() => ({})), getActivePlanId: vi.fn(() => 1), toggleEntry: vi.fn(() => ({})), toggleCompanyHoliday: vi.fn(() => ({})), ...overrides.vacay } as never, guards), - new JournalRpc({ listJourneys: vi.fn(() => []), listEntries: vi.fn(() => []), createEntry: vi.fn(() => ({})), updateEntry: vi.fn(() => ({})), deleteEntry: vi.fn(() => true), createJourney: vi.fn(() => ({})), deleteJourney: vi.fn(() => true), ...overrides.journey } as never, guards), + new JournalRpc({ listJourneys: vi.fn(() => []), listEntries: vi.fn(() => []), createEntry: vi.fn(() => ({})), updateEntry: vi.fn(() => ({})), deleteEntry: vi.fn(() => true), createJourney: vi.fn(() => ({})), deleteJourney: vi.fn(() => true), ...overrides.journey } as never, guards, {} as never, {} as never, {} as never, {} as never), // deletePlace is async in production, so its double returns a promise: an // un-awaited call in the RPC handler would let a rejection sail past // mapCollectionError as an unhandled rejection while the handler still replied @@ -74,6 +74,7 @@ const USERLESS: Array<[string, Record, string]> = [ ['journal.updateEntry', { entryId: 1, input: {} }, 'journal writes require an authenticated user context'], ['journal.deleteEntry', { entryId: 1 }, 'journal writes require an authenticated user context'], ['journal.createJourney', { input: { title: 'J' } }, 'journal writes require an authenticated user context'], + ['journal.addEntryPhoto', { entryId: 1, input: { name: 'a.jpg', content_base64: 'eA==' } }, 'journal writes require an authenticated user context'], ['journal.deleteJourney', { journeyId: 7 }, 'journal writes require an authenticated user context'], ['atlas.visited', {}, 'atlas reads require an authenticated user context'], ['atlas.bucketList', {}, 'atlas reads require an authenticated user context'], diff --git a/server/tests/unit/plugins/addon-gates.rpc.test.ts b/server/tests/unit/plugins/addon-gates.rpc.test.ts index 403405758..b031aba62 100644 --- a/server/tests/unit/plugins/addon-gates.rpc.test.ts +++ b/server/tests/unit/plugins/addon-gates.rpc.test.ts @@ -68,7 +68,13 @@ function build(addonOn: boolean) { new CollabRpc(spyService(calls, 'collab'), { broadcast: vi.fn() } as never, guards), new AtlasRpc(spyService(calls, 'atlas'), guards), new VacayRpc(spyService(calls, 'vacay'), guards), - new JournalRpc(spyService(calls, 'journey'), guards), + // The photo method writes bytes, so its four extra deps need real shapes or the + // addon-on half of the table would fail for a reason that is not the gate. + new JournalRpc(spyService(calls, 'journey'), guards, + { put: async () => undefined, delete: async () => undefined } as never, + { get: () => '*' } as never, + { schedule: () => undefined } as never, + { prepare: () => ({ get: () => ({ email: 'u@example.test' }) }) } as never), new CollectionsRpc(spyService(calls, 'collections'), guards), new CostsRpc(spyService(calls, 'budget'), db, { broadcast: vi.fn() } as never, guards, spyService(calls, 'membership')), ]); @@ -95,6 +101,7 @@ const GATED: Array<[string, Record, string]> = [ ['journal.updateEntry', { entryId: 1, input: {} }, 'journey'], ['journal.deleteEntry', { entryId: 1 }, 'journey'], ['journal.createJourney', { input: { title: 'J' } }, 'journey'], + ['journal.addEntryPhoto', { entryId: 1, input: { name: 'a.jpg', content_base64: 'eA==' } }, 'journey'], ['journal.deleteJourney', { journeyId: 7 }, 'journey'], ['atlas.visited', {}, 'atlas'], ['atlas.bucketList', {}, 'atlas'], @@ -137,7 +144,7 @@ describe('every addon-gated plugin method refuses when its addon is off', () => it('ADDONGATE-covers-every-gated-method', () => { // Guards the table itself: if a later PR adds an addon-gated method without a row // here, this number is the reminder. - expect(GATED).toHaveLength(37); + expect(GATED).toHaveLength(38); }); it('ADDONGATE-the-same-calls-succeed-with-the-addon-on', async () => { diff --git a/server/tests/unit/plugins/journal-photo.rpc.test.ts b/server/tests/unit/plugins/journal-photo.rpc.test.ts new file mode 100644 index 000000000..675b5fd46 --- /dev/null +++ b/server/tests/unit/plugins/journal-photo.rpc.test.ts @@ -0,0 +1,143 @@ +/** + * journal.addEntryPhoto — the byte path a journal importer needs (#1365). + * + * A plugin holding an export archive has bytes and nothing else: no gallery photo + * to link and no provider asset. These cases pin what the handler refuses before + * it writes anything, and that a refused write leaves no object behind. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// vi.mock is hoisted above the imports, so the flag it reads has to be too. +// Only readEnv().demo is overridden: the module is shared, and replacing it +// wholesale takes defaultLanguage and the rest of the env with it. +const { demo } = vi.hoisted(() => ({ demo: { enabled: false } })); +vi.mock('../../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, readEnv: () => ({ ...actual.readEnv(), demo }) }; +}); + +import { JournalRpc } from '../../../src/nest/journey/journal.rpc'; +import { DEMO_EMAIL_PRIMARY } from '../../../src/nest/common/demo'; + +const ACTOR = { actingUserId: 7 } as never; +const PNG = Buffer.from('89504e470d0a1a0a', 'hex').toString('base64'); + +function build(overrides: { + addPhoto?: unknown; + allowed?: string; + demoEnabled?: boolean; + email?: string; +} = {}) { + // Typed parameters so the assertions below can index mock.calls. + const put = vi.fn(async (_category: string, _filename: string, _body?: unknown, _opts?: unknown) => undefined); + const del = vi.fn(async (_category: string, _filename: string) => undefined); + const schedule = vi.fn(); + const addPhoto = vi.fn(overrides.addPhoto === undefined + ? () => ({ id: 5, photo_id: 42 }) + : () => overrides.addPhoto); + + demo.enabled = overrides.demoEnabled ?? false; + + const rpc = new JournalRpc( + { addPhoto } as never, + { requireAddon: vi.fn() } as never, + { put, delete: del } as never, + { get: () => overrides.allowed ?? '*' } as never, + { schedule } as never, + { prepare: () => ({ get: () => ({ email: overrides.email ?? 'user@example.test' }) }) } as never, + ); + return { rpc, put, del, schedule, addPhoto }; +} + +const input = (over: Record = {}) => ({ name: 'photo.jpg', content_base64: PNG, ...over }); + +beforeEach(() => { + demo.enabled = false; +}); + +describe('journal.addEntryPhoto', () => { + it('JPHOTO-001: stores the bytes under a name of its own, then links the photo', async () => { + const { rpc, put, schedule, addPhoto } = build(); + + const photo = await rpc.addEntryPhoto({ entryId: 3, input: input({ caption: 'Tokyo' }) }, ACTOR); + + expect(photo).toEqual({ id: 5, photo_id: 42 }); + // Category and a generated filename: never the name the plugin sent, which + // could otherwise collide with an existing gallery row on file_path. + expect(put).toHaveBeenCalledTimes(1); + const [category, filename] = put.mock.calls[0]; + expect(category).toBe('journey'); + expect(filename).toMatch(/^[0-9a-f-]{36}\.jpg$/); + expect(addPhoto).toHaveBeenCalledWith(3, 7, `journey/${filename}`, undefined, 'Tokyo'); + expect(schedule).toHaveBeenCalledWith([42], 7); + }); + + it('JPHOTO-002: refuses an entry the acting user cannot edit, and removes the object it just wrote', async () => { + // addPhoto answering null IS the access check: the entry is missing, or it + // belongs to a journey this user may not edit. + const { rpc, put, del } = build({ addPhoto: null }); + + await expect(rpc.addEntryPhoto({ entryId: 3, input: input() }, ACTOR)).rejects.toThrow(/no editable journal entry 3/); + expect(put).toHaveBeenCalledTimes(1); + expect(del).toHaveBeenCalledWith('journey', put.mock.calls[0][1]); + }); + + it('JPHOTO-003: refuses a name that is a path, and an extension that is not an image', async () => { + const { rpc, put } = build(); + + // basename() reduces this to 'evil.sh', which is not an image extension. + await expect(rpc.addEntryPhoto({ entryId: 3, input: input({ name: '../../evil.sh' }) }, ACTOR)) + .rejects.toThrow(/not an allowed image type/); + await expect(rpc.addEntryPhoto({ entryId: 3, input: input({ name: 'map.svg' }) }, ACTOR)) + .rejects.toThrow(/not an allowed image type/); + await expect(rpc.addEntryPhoto({ entryId: 3, input: input({ name: 'noext' }) }, ACTOR)) + .rejects.toThrow(/not an allowed image type/); + expect(put).not.toHaveBeenCalled(); + }); + + it('JPHOTO-004: obeys the operator allowed-file-types setting, so the RPC is no way around it', async () => { + const { rpc, put } = build({ allowed: 'png,pdf' }); + + await expect(rpc.addEntryPhoto({ entryId: 3, input: input({ name: 'photo.jpg' }) }, ACTOR)) + .rejects.toThrow(/file type \.jpg is not allowed/); + expect(put).not.toHaveBeenCalled(); + + await expect(rpc.addEntryPhoto({ entryId: 3, input: input({ name: 'photo.png' }) }, ACTOR)).resolves.toBeTruthy(); + }); + + it('JPHOTO-005: rejects an oversized payload before decoding it, and an empty one after', async () => { + const { rpc, put } = build(); + + // Encoded cap first: a 15MB string never becomes a 11MB buffer in memory. + await expect(rpc.addEntryPhoto({ entryId: 3, input: input({ content_base64: 'A'.repeat(15 * 1024 * 1024) }) }, ACTOR)) + .rejects.toThrow(/invalid photo input/); + await expect(rpc.addEntryPhoto({ entryId: 3, input: input({ content_base64: '@@@@' }) }, ACTOR)) + .rejects.toThrow(/photo content is empty/); + expect(put).not.toHaveBeenCalled(); + }); + + it('JPHOTO-006: rejects an entryId that is not a positive integer, and unknown input keys', async () => { + const { rpc } = build(); + + await expect(rpc.addEntryPhoto({ entryId: 1.5, input: input() }, ACTOR)).rejects.toThrow(/positive integer/); + await expect(rpc.addEntryPhoto({ entryId: 0, input: input() }, ACTOR)).rejects.toThrow(/positive integer/); + // strictObject: an unknown key is a mistake worth reporting, not ignoring. + await expect(rpc.addEntryPhoto({ entryId: 3, input: input({ provider: 'immich' }) }, ACTOR)) + .rejects.toThrow(/invalid photo input/); + }); + + it('JPHOTO-007: refuses to write bytes for a demo user, exactly as the REST upload does', async () => { + const { rpc, put } = build({ demoEnabled: true, email: DEMO_EMAIL_PRIMARY }); + + await expect(rpc.addEntryPhoto({ entryId: 3, input: input() }, ACTOR)).rejects.toThrow(/disabled in demo mode/); + expect(put).not.toHaveBeenCalled(); + }); + + it('JPHOTO-008: a userless call never reaches storage', async () => { + const { rpc, put } = build(); + + await expect(rpc.addEntryPhoto({ entryId: 3, input: input() }, {} as never)) + .rejects.toThrow(/journal writes require an authenticated user context/); + expect(put).not.toHaveBeenCalled(); + }); +}); diff --git a/server/tests/unit/plugins/rpc-coverage.test.ts b/server/tests/unit/plugins/rpc-coverage.test.ts index f7a5cf5c9..9d5516d14 100644 --- a/server/tests/unit/plugins/rpc-coverage.test.ts +++ b/server/tests/unit/plugins/rpc-coverage.test.ts @@ -49,13 +49,13 @@ describe('plugin RPC coverage ledger', () => { expect(fromRegistry.size).toBe(KNOWN_METHODS.length + UNCONDITIONAL_METHODS.length); }); - it('RPCLEDGER-005 the surface is the 113 methods the protocol declares', () => { + it('RPCLEDGER-005 the surface is the 114 methods the protocol declares', () => { // Pinned rather than derived: a method DELETED from KNOWN_METHODS together with // its handler would keep every test above green, and this is the line that makes // that show up in the diff. - expect(KNOWN_METHODS.length).toBe(110); + expect(KNOWN_METHODS.length).toBe(111); expect(UNCONDITIONAL_METHODS.length).toBe(3); - expect(fromRegistry.size).toBe(113); + expect(fromRegistry.size).toBe(114); }); it('RPCLEDGER-006 boot-time total coverage is armed in production', () => { diff --git a/shared/src/i18n/de/admin.ts b/shared/src/i18n/de/admin.ts index b5687b842..c384dba7f 100644 --- a/shared/src/i18n/de/admin.ts +++ b/shared/src/i18n/de/admin.ts @@ -271,7 +271,7 @@ const admin: TranslationStrings = { 'admin.plugins.perm.db:write:vacay': 'Urlaubstage und Betriebsferien im aktiven Plan des handelnden Nutzers umschalten (benötigt das Vacay-Addon)', 'admin.plugins.perm.db:write:journal': - 'Tagebucheinträge in Reisetagebüchern anlegen, bearbeiten und löschen, die der handelnde Nutzer bearbeiten kann (benötigt das Journey-Addon)', + 'Tagebucheinträge in Reisetagebüchern anlegen, bearbeiten, löschen und mit Fotos versehen, die der handelnde Nutzer bearbeiten kann (benötigt das Journey-Addon)', 'admin.plugins.perm.db:write:collections': 'Sammlungen anlegen und bearbeiten und Orte darin speichern, mit der Sammlungsrolle des handelnden Nutzers (benötigt das Collections-Addon)', 'admin.plugins.perm.db:write:files': diff --git a/shared/src/i18n/en/admin.ts b/shared/src/i18n/en/admin.ts index c172c62ac..4af119bbb 100644 --- a/shared/src/i18n/en/admin.ts +++ b/shared/src/i18n/en/admin.ts @@ -322,7 +322,7 @@ const admin: TranslationStrings = { 'admin.plugins.perm.db:write:vacay': "Toggle vacation days and company holidays on the acting user's active plan (needs the Vacay addon)", 'admin.plugins.perm.db:write:journal': - 'Create, edit and delete journal entries on journeys the acting user can edit (needs the Journey addon)', + 'Create, edit and delete journal entries, and attach photos to them, on journeys the acting user can edit (needs the Journey addon)', 'admin.plugins.perm.db:write:collections': "Create and edit collections and save places to them, with the acting user's collection role (needs the Collections addon)", 'admin.plugins.perm.db:write:files': diff --git a/shared/src/journey/journey.schema.ts b/shared/src/journey/journey.schema.ts index 057060a29..1387e0046 100644 --- a/shared/src/journey/journey.schema.ts +++ b/shared/src/journey/journey.schema.ts @@ -91,6 +91,25 @@ export type JourneyLinkPhotoRequest = z.infer; +/** + * One photo handed to a journal entry over the plugin RPC (#1365). + * + * Strict, unlike the REST schemas above: those forward a free-form body to a + * handler that picks what it needs, while this one is the whole contract with + * third-party code, and an unknown key there is a mistake worth reporting rather + * than ignoring. + * + * The encoded cap is the same 14MB the file surface uses. Base64 runs about a + * third larger than the bytes, so it bounds the payload at roughly the 10MB the + * handler enforces after decoding, and it does so before anything is decoded. + */ +export const journalPluginPhotoInputSchema = z.strictObject({ + name: z.string().trim().min(1).max(255), + content_base64: z.string().min(1).max(14 * 1024 * 1024), + caption: z.string().max(2000).optional(), +}); +export type JournalPluginPhotoInput = z.infer; + /** Free-form plus the entry_date the handler demands itself. */ export const journeyEntryCreateRequestSchema = z.looseObject({ entry_date: z.unknown().optional(), diff --git a/wiki/Plugin-Permissions.md b/wiki/Plugin-Permissions.md index 0c45c9713..0c14db54a 100644 --- a/wiki/Plugin-Permissions.md +++ b/wiki/Plugin-Permissions.md @@ -41,7 +41,7 @@ ungranted capability is physically unreachable**, not just disallowed. See | `db:write:tags` | Create/edit/delete the acting user's own tags via `ctx.tags` | User-scoped; ownership re-checked before each write. | | `db:write:atlas` | Mark/unmark visited countries + regions and manage the bucket list via `ctx.atlas` | All rows are the **acting user's own** — no trip scoping, no cross-tenant surface. Needs the Atlas addon; unblocks e.g. AirTrail-style two-way sync. | | `db:write:vacay` | Toggle PTO days + company holidays via `ctx.vacay` | The plan is resolved **host-side from the acting user's active plan** — a plugin can never name another plan; `toggleEntry` only toggles the acting user's own day. Needs the Vacay addon. | -| `db:write:journal` | Create/edit/delete journal entries **and create/delete whole journals** via `ctx.journal` (`createEntry`/`updateEntry`/`deleteEntry` plus `createJourney`/`deleteJourney`) | Entry writes go through the journey domain service's `canEdit`: the journal's owner, or a contributor whose role is `editor`/`owner` — a `viewer` contributor is refused. `createJourney` has no journey-level gate (there is no journey yet); it always creates a **new journal owned by the acting user**, which is how importers bootstrap the journal they then fill. `deleteJourney` is **owner-only** (`isOwner`), not `canEdit` — a contributor, even an editor, cannot delete the journal. Every call needs a bound acting user (a no-user job is refused) and the Journey addon. | +| `db:write:journal` | Create/edit/delete journal entries, **attach photos to them**, **and create/delete whole journals** via `ctx.journal` (`createEntry`/`updateEntry`/`deleteEntry`/`addEntryPhoto` plus `createJourney`/`deleteJourney`) | Entry writes go through the journey domain service's `canEdit`: the journal's owner, or a contributor whose role is `editor`/`owner` — a `viewer` contributor is refused. `createJourney` has no journey-level gate (there is no journey yet); it always creates a **new journal owned by the acting user**, which is how importers bootstrap the journal they then fill. `deleteJourney` is **owner-only** (`isOwner`), not `canEdit` — a contributor, even an editor, cannot delete the journal. `addEntryPhoto` takes the bytes themselves, for an importer that holds an export archive and has no gallery photo to point at: images only, no SVG, 10 MB decoded, the stored filename is the host's, and the operator's allowed-file-types setting applies exactly as it does to the upload form. Every call needs a bound acting user (a no-user job is refused) and the Journey addon. | | `db:write:collections` | Create/edit collections, save places, copy to a trip via `ctx.collections` | The service enforces the acting user's **per-collection role** (owner/admin/editor) itself. Needs the Collections addon. | | `db:write:files` | Attach files + manage links via `ctx.files` (`create`, `createLink`, `update`, `softDelete`) | Trip access **+** the app's `file_upload`/`file_edit`/`file_delete` rights respectively. Content arrives as base64 (10MB cap); the extension is validated against the central blocklist before touching disk; link targets must live on the same trip. | | `db:write:collab` | Post notes, polls and chat messages via `ctx.collab` | Trip access **+** `collab_edit`; needs the Collab addon. Broadcasts the same `collab:*` events as the app. |