From f37fc329baed2cf01ca6ab465f02eda1b5264ac7 Mon Sep 17 00:00:00 2001 From: IliesAllali Date: Fri, 16 Oct 2026 18:12:21 +0200 Subject: [PATCH] =?UTF-8?q?Immich=20:=20lectures=20born=C3=A9es,=20erreurs?= =?UTF-8?q?=20de=20corps=20en=20503,=20lien=20retir=C3=A9=20avec=20le=20po?= =?UTF-8?q?rtrait?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Vignette lue en flux : au-delà de 5 Mo (ou sans taille annoncée), la lecture s'arrête sans charger le reste. - Vignette servie seulement si les octets sont une image, avec X-Content-Type-Options: nosniff (plus de repli sur image/jpeg). - Liste des personnes : délai global de 30 s sur toutes les pages. - Corps JSON illisible ou flux interrompu : 503 immich_unreachable, au lieu d'une erreur serveur. - Retirer le portrait à la main (DELETE avatar) ou supprimer une personne efface aussi son identifiant Immich, sinon l'association reste invisible. Co-Authored-By: Claude Opus 5.5 --- backend/src/lib/entity-deletion.ts | 2 +- backend/src/lib/immich.ts | 51 +++++++++++++++++-- backend/src/routes/immich.ts | 16 +++--- backend/src/routes/media.ts | 2 + backend/test/immich.test.ts | 79 ++++++++++++++++++++++++++++++ 5 files changed, 138 insertions(+), 12 deletions(-) diff --git a/backend/src/lib/entity-deletion.ts b/backend/src/lib/entity-deletion.ts index 89d347b..eb41427 100644 --- a/backend/src/lib/entity-deletion.ts +++ b/backend/src/lib/entity-deletion.ts @@ -32,7 +32,7 @@ export async function softDeletePersonCascade( await db.person.updateMany({ where: { id: personId, treeId, deletedAt: null }, - data: { deletedAt: now, updatedBy: actorUserId, avatarPath: null, avatarMimeType: null }, + data: { deletedAt: now, updatedBy: actorUserId, avatarPath: null, avatarMimeType: null, immichPersonId: null }, }) const unions = await db.union.findMany({ diff --git a/backend/src/lib/immich.ts b/backend/src/lib/immich.ts index 71ebfaf..8645bde 100644 --- a/backend/src/lib/immich.ts +++ b/backend/src/lib/immich.ts @@ -39,6 +39,8 @@ export class ImmichNotFoundError extends Error { const IMMICH_PEOPLE_PAGE_SIZE = 1000 const IMMICH_TIMEOUT_MS = 10_000 +// Délai global de la liste complète : chaque page a son propre délai, la liste entière aussi. +const IMMICH_LIST_DEADLINE_MS = 30_000 // Identifiants de personnes Immich : UUID v4 (spécification OpenAPI). const IMMICH_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i @@ -69,6 +71,16 @@ export class ImmichClient { // Le chemin de l'instance (ex. /immich) reste devant les chemins de l'API : on concatène, // on ne résout pas une URL relative (qui effacerait ce chemin). + // Corps JSON lu hors de toute gestion d'erreur : un corps illisible est une instance + // défaillante (503), pas une erreur serveur. + private async readJson(response: Response): Promise { + try { + return await response.json() + } catch { + throw new ImmichUnavailableError('Réponse Immich illisible') + } + } + private buildUrl(path: string): string { return `${this.apiUrl}${path}` } @@ -119,10 +131,14 @@ export class ImmichClient { const people: ImmichPersonSummary[] = [] let page = 1 + const deadline = Date.now() + IMMICH_LIST_DEADLINE_MS // Garde-fou : une instance qui répondrait toujours hasNextPage ne doit pas nous faire boucler. for (let guard = 0; guard < 1000; guard += 1, page += 1) { + if (Date.now() > deadline) { + throw new ImmichUnavailableError('Liste des personnes Immich trop longue') + } const response = await this.fetch(`/api/people?page=${page}&size=${IMMICH_PEOPLE_PAGE_SIZE}`) - const payload = (await response.json()) as ImmichPeoplePage + const payload = (await this.readJson(response)) as ImmichPeoplePage if (!Array.isArray(payload.people)) { throw new ImmichUnavailableError('Réponse Immich inattendue sur /api/people') @@ -148,7 +164,7 @@ export class ImmichClient { // se vérifie donc ici. async getPerson(personId: string): Promise { const response = await this.fetch(`/api/people/${encodeURIComponent(personId)}`) - const person = (await response.json()) as { id?: unknown; name?: unknown; thumbnailPath?: unknown } + const person = (await this.readJson(response)) as { id?: unknown; name?: unknown; thumbnailPath?: unknown } if (typeof person?.id !== 'string' || typeof person?.name !== 'string') { throw new ImmichUnavailableError('Réponse Immich inattendue sur /api/people/{id}') } @@ -160,9 +176,36 @@ export class ImmichClient { } // Vignette d'une personne, en octets bruts (JPEG côté Immich). Absente -> ImmichNotFoundError. - async getPersonThumbnail(personId: string): Promise { + // Lue en flux : au-delà de maxBytes, la lecture s'arrête et renvoie null sans charger le reste. + async getPersonThumbnail(personId: string, maxBytes: number): Promise { const response = await this.fetch(`/api/people/${encodeURIComponent(personId)}/thumbnail`) - return Buffer.from(await response.arrayBuffer()) + const declared = Number(response.headers.get('content-length')) + if (Number.isFinite(declared) && declared > maxBytes) { + await response.body?.cancel().catch(() => {}) + return null + } + if (!response.body) { + return Buffer.alloc(0) + } + + const reader = response.body.getReader() + const chunks: Buffer[] = [] + let total = 0 + try { + for (;;) { + const { done, value } = await reader.read() + if (done) break + total += value.byteLength + if (total > maxBytes) { + await reader.cancel().catch(() => {}) + return null + } + chunks.push(Buffer.from(value)) + } + } catch { + throw new ImmichUnavailableError('Lecture de la vignette Immich interrompue') + } + return Buffer.concat(chunks, total) } } diff --git a/backend/src/routes/immich.ts b/backend/src/routes/immich.ts index 6d713a1..8e97219 100644 --- a/backend/src/routes/immich.ts +++ b/backend/src/routes/immich.ts @@ -303,21 +303,23 @@ export const immichRoutes: FastifyPluginAsync = async (app) => { return reply.code(503).send({ error: 'immich_not_configured' }) } - let thumbnail: Buffer + let thumbnail: Buffer | null try { - thumbnail = await client.getPersonThumbnail(params.data.immichPersonId) + thumbnail = await client.getPersonThumbnail(params.data.immichPersonId, MAX_THUMBNAIL_BYTES) } catch (error) { return replyImmichError(request, reply, error) } - if (!thumbnail.length || thumbnail.length > MAX_THUMBNAIL_BYTES) { - request.log.warn({ size: thumbnail.length }, 'Vignette Immich vide ou trop grande') + // Le type se lit dans les octets, pas dans ce qu'Immich déclare : la vignette finit dans une + // balise . Vide, trop grande ou pas une image : rien à montrer. + const mimeType = thumbnail?.length ? sniffMimeType(thumbnail, 'photo') : null + if (!thumbnail || !mimeType?.startsWith('image/')) { + request.log.warn('Vignette Immich vide, trop grande ou qui n’est pas une image') return reply.code(404).send({ error: 'thumbnail_not_found' }) } - // Le type se lit dans les octets, pas dans ce qu'Immich déclare : la vignette finit dans une balise . - const mimeType = sniffMimeType(thumbnail, 'photo') ?? 'image/jpeg' reply.header('Content-Type', mimeType) + reply.header('X-Content-Type-Options', 'nosniff') reply.header('Cache-Control', 'private, max-age=3600') return reply.send(thumbnail) }) @@ -368,7 +370,7 @@ export const immichRoutes: FastifyPluginAsync = async (app) => { let thumbnail: Buffer | null = null if (immichPerson.hasThumbnail) { try { - thumbnail = await client.getPersonThumbnail(immichPersonId) + thumbnail = await client.getPersonThumbnail(immichPersonId, MAX_THUMBNAIL_BYTES) } catch (error) { if (!(error instanceof ImmichNotFoundError)) { return replyImmichError(request, reply, error) diff --git a/backend/src/routes/media.ts b/backend/src/routes/media.ts index 0e07c57..335d7e6 100644 --- a/backend/src/routes/media.ts +++ b/backend/src/routes/media.ts @@ -609,6 +609,8 @@ export const mediaRoutes: FastifyPluginAsync = async (app) => { data: { avatarPath: null, avatarMimeType: null, + // Retirer le portrait retire aussi le lien Immich : l'association ne survit pas au portrait. + immichPersonId: null, updatedBy: request.actor.userId, }, }) diff --git a/backend/test/immich.test.ts b/backend/test/immich.test.ts index 39368a8..b4f5011 100644 --- a/backend/test/immich.test.ts +++ b/backend/test/immich.test.ts @@ -918,3 +918,82 @@ describe('Immich integration: adresse de l’instance', () => { expect(normalizeImmichApiUrl('ftp://immich.test')).toBeNull() }) }) + +describe('Immich integration: durcissement des vignettes et des réponses', () => { + it('serves only images, with nosniff, and refuses bytes that are not an image', async () => { + const app = createApp() + const token = await createUserToken(app) + + fetchMock.mockResolvedValueOnce(new Response(new Uint8Array(jpegBytes), { status: 200 })) + const image = await app.inject({ + method: 'GET', + url: `/trees/tree-1/immich/thumbnail/${immichPersonId}`, + headers: { authorization: `Bearer ${token}` }, + }) + expect(image.statusCode).toBe(200) + expect(image.headers['content-type']).toBe('image/jpeg') + expect(image.headers['x-content-type-options']).toBe('nosniff') + + // Un corps HTML déclaré image n'est jamais servi comme tel. + fetchMock.mockResolvedValueOnce(new Response('', { status: 200, headers: { 'content-type': 'image/png' } })) + const notImage = await app.inject({ + method: 'GET', + url: `/trees/tree-1/immich/thumbnail/${immichPersonId}`, + headers: { authorization: `Bearer ${token}` }, + }) + expect(notImage.statusCode).toBe(404) + expect(notImage.json()).toEqual({ error: 'thumbnail_not_found' }) + await app.close() + }) + + it('stops reading a thumbnail above 5 MB without loading the rest of it', async () => { + const app = createApp() + const token = await createUserToken(app) + // Un flux sans taille annoncée, plus grand que la limite : la lecture doit s'arrêter en route. + const big = new Uint8Array(5 * 1024 * 1024 + 1024) + big.set(jpegBytes.subarray(0, Math.min(jpegBytes.length, big.length))) + fetchMock.mockResolvedValueOnce(new Response(big, { status: 200 })) + + const response = await app.inject({ + method: 'GET', + url: `/trees/tree-1/immich/thumbnail/${immichPersonId}`, + headers: { authorization: `Bearer ${token}` }, + }) + + expect(response.statusCode).toBe(404) + expect(response.json()).toEqual({ error: 'thumbnail_not_found' }) + await app.close() + }) + + it('answers 503 when an Immich response body cannot be read as JSON', async () => { + const app = createApp() + const token = await createUserToken(app) + fetchMock.mockResolvedValue(new Response('passerelle en panne', { status: 200 })) + + const response = await app.inject({ + method: 'GET', + url: '/trees/tree-1/immich/people', + headers: { authorization: `Bearer ${token}` }, + }) + + expect(response.statusCode).toBe(503) + expect(response.json()).toEqual({ error: 'immich_unreachable' }) + await app.close() + }) + + it('clears the Immich link when the portrait is deleted by hand', async () => { + const app = createApp() + const token = await createUserToken(app) + prismaMock.person.findFirst.mockResolvedValue({ id: 'person-1', avatarPath: 'tree-1/person-1/avatar-a.jpg', immichPersonId }) + + const response = await app.inject({ + method: 'DELETE', + url: '/trees/tree-1/persons/person-1/avatar', + headers: { authorization: `Bearer ${token}` }, + }) + + expect(response.statusCode).toBe(200) + expect(prismaMock.person.updateMany.mock.calls[0][0].data).toMatchObject({ avatarPath: null, immichPersonId: null }) + await app.close() + }) +})