fix(favorites): Symbol-Adresse auch speichern, wenn nur der Browser sie laden kann

- API: ausdrueckliche iconUrl wird nur auf Form (http/https, <= 2048) geprueft und auch gespeichert, wenn der Server sie nicht abrufen kann; keine 422 mehr - Erkennung: Seite mit Fehlerstatus, aber HTML mit <link rel=icon>, liefert diesen Verweis (docuvita); og:image einer Fehlerseite zaehlt nicht - Kachel: Proxy -> iconUrl direkt im Browser (no-referrer, nur http/https) -> Origin-Favicon -> Buchstabe - Meldung iconUrlUnreachable (de/en) entfernt, Hinweis zum Vorrang angepasst Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-29 15:34:58 +02:00
parent 7188c5b958
commit b15c74632b
12 changed files with 307 additions and 112 deletions
@@ -5,6 +5,7 @@ import {
  IsString,
  IsUrl,
  IsUUID,
+  MaxLength,
 } from 'class-validator';

 /**
@@ -24,6 +25,7 @@ export class CreateFavoriteDto {

  @IsOptional()
  @IsString()
+  @MaxLength(2048)
  iconUrl?: string;

  @IsOptional()
@@ -1,4 +1,4 @@
-import { IsInt, IsOptional, IsString, IsUrl } from 'class-validator';
+import { IsInt, IsOptional, IsString, IsUrl, MaxLength } from 'class-validator';

 /**
 * DTO for updating an existing FavoriteLink.
@@ -19,6 +19,8 @@ export class UpdateFavoriteDto {
   * No strict type validation so null passes through to Prisma.
   */
  @IsOptional()
+  @IsString()
+  @MaxLength(2048)
  iconUrl?: string | null;

  @IsOptional()
@@ -6,7 +6,6 @@ import {
  HttpException,
  NotFoundException,
  PayloadTooLargeException,
-  UnprocessableEntityException,
 } from '@nestjs/common';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { FavoritesService } from './favorites.service';
@@ -920,26 +919,50 @@ describe('FavoritesService — Bindung an forTenant() (260911-gwh)', () => {
    });
  });

-  describe('create/update — Abrufprobe fuer eine explizite iconUrl (260923-lrr)', () => {
-    it('create mit expliziter iconUrl: Probe genau einmal; wirft -> UnprocessableEntityException, favoriteLink.create NICHT aufgerufen', async () => {
-      const prisma = makeFakePrisma([], [{ id: 'widget-a1', userId: 'user-a1', tenantId: 't1' }]);
-      const iconDiscovery = makeIconDiscovery({
-        fetchIconBytes: vi.fn(async () => {
-          throw new Error('blocked');
-        }),
+  describe('create/update — ausdrueckliche iconUrl: nur Formpruefung, kein Abruf (260929-lh3)', () => {
+    const widgets = [{ id: 'widget-a1', userId: 'user-a1', tenantId: 't1' }];
+    const failingFetch = () =>
+      vi.fn(async () => {
+        throw new Error('server bekommt 404/HTML');
      });
+
+    it('create mit einer Adresse, die der SERVER nicht abrufen kann: wird gespeichert, KEIN Abruf, KEINE Erkennung', async () => {
+      const prisma = makeFakePrisma([], widgets);
+      const iconDiscovery = makeIconDiscovery({ fetchIconBytes: failingFetch() });
      const service = new FavoritesService(prisma as any, iconDiscovery as any);

+      const created = await service.create('t1', 'user-a1', {
+        widgetId: 'widget-a1',
+        title: 'Docuvita',
+        url: 'https://docuvita.ctl.local/server/services/web/',
+        iconUrl: 'https://docuvita.ctl.local/webclient/docuvita/resources/brandimage/favicon.ico',
+      } as any);
+
+      expect(created.iconUrl).toBe(
+        'https://docuvita.ctl.local/webclient/docuvita/resources/brandimage/favicon.ico',
+      );
+      expect(iconDiscovery.fetchIconBytes).not.toHaveBeenCalled();
+      expect(iconDiscovery.discoverFavoriteIconUrl).not.toHaveBeenCalled();
+      expect(prisma.__favorites.size).toBe(1);
+    });
+
+    it.each([
+      ['kein http/https', 'ftp://x.invalid/icon.png'],
+      ['javascript-Schema', 'javascript:alert(1)'],
+      ['keine Adresse', 'kein url'],
+      ['laenger als 2048 Zeichen', `https://x.invalid/${'a'.repeat(2050)}`],
+    ])('create mit ungueltiger iconUrl (%s) -> BadRequestException, nichts geschrieben', async (_label, iconUrl) => {
+      const prisma = makeFakePrisma([], widgets);
+      const service = new FavoritesService(prisma as any, makeIconDiscovery() as any);
+
      await expect(
        service.create('t1', 'user-a1', {
          widgetId: 'widget-a1',
          title: 'X',
          url: 'https://x.invalid',
-          iconUrl: 'https://x.invalid/logo.png',
+          iconUrl,
        } as any),
-      ).rejects.toThrow(UnprocessableEntityException);
-      expect(iconDiscovery.fetchIconBytes).toHaveBeenCalledTimes(1);
-      expect(iconDiscovery.fetchIconBytes).toHaveBeenCalledWith('https://x.invalid/logo.png');
+      ).rejects.toThrow(BadRequestException);
      expect(prisma.__favorites.size).toBe(0);
    });

@@ -956,24 +979,31 @@ describe('FavoritesService — Bindung an forTenant() (260911-gwh)', () => {
      iconVersion: 0,
    };

-    it('update mit neuer, abweichender iconUrl: fetchIconBytes genau einmal mit dieser Adresse; wirft -> UnprocessableEntityException, favoriteLink.update NICHT aufgerufen', async () => {
+    it('update mit neuer iconUrl, die der Server nicht abrufen kann: gespeichert, iconVersion +1, KEIN Abruf', async () => {
      const prisma = makeFakePrisma([baseRow]);
-      const iconDiscovery = makeIconDiscovery({
-        fetchIconBytes: vi.fn(async () => {
-          throw new Error('blocked');
-        }),
-      });
+      const iconDiscovery = makeIconDiscovery({ fetchIconBytes: failingFetch() });
      const service = new FavoritesService(prisma as any, iconDiscovery as any);

+      const updated = await service.update('t1', 'f1', 'user-a1', {
+        iconUrl: 'https://neu.invalid/icon.png',
+      } as any);
+
+      expect(updated.iconUrl).toBe('https://neu.invalid/icon.png');
+      expect(updated.iconVersion).toBe(1);
+      expect(iconDiscovery.fetchIconBytes).not.toHaveBeenCalled();
+    });
+
+    it('update mit ungueltiger neuer iconUrl -> BadRequestException, Zeile unveraendert', async () => {
+      const prisma = makeFakePrisma([baseRow]);
+      const service = new FavoritesService(prisma as any, makeIconDiscovery() as any);
+
      await expect(
-        service.update('t1', 'f1', 'user-a1', { iconUrl: 'https://neu.invalid/icon.png' } as any),
-      ).rejects.toThrow(UnprocessableEntityException);
-      expect(iconDiscovery.fetchIconBytes).toHaveBeenCalledTimes(1);
-      expect(iconDiscovery.fetchIconBytes).toHaveBeenCalledWith('https://neu.invalid/icon.png');
+        service.update('t1', 'f1', 'user-a1', { iconUrl: 'file:///etc/passwd' } as any),
+      ).rejects.toThrow(BadRequestException);
      expect(prisma.__favorites.get('f1').iconUrl).toBe(baseRow.iconUrl);
    });

-    it('update mit UNVERAENDERTER iconUrl: keine Probe, keine Erhoehung', async () => {
+    it('update mit UNVERAENDERTER iconUrl: keine Pruefung, keine Erhoehung', async () => {
      const prisma = makeFakePrisma([baseRow]);
      const iconDiscovery = makeIconDiscovery();
      const service = new FavoritesService(prisma as any, iconDiscovery as any);
@@ -992,18 +1022,5 @@ describe('FavoritesService — Bindung an forTenant() (260911-gwh)', () => {

      expect(updated.iconVersion).toBe(0);
    });
-
-    it('update mit neuer, erreichbarer iconUrl: iconVersion +1', async () => {
-      const prisma = makeFakePrisma([baseRow]);
-      const iconDiscovery = makeIconDiscovery();
-      const service = new FavoritesService(prisma as any, iconDiscovery as any);
-
-      const updated = await service.update('t1', 'f1', 'user-a1', {
-        iconUrl: 'https://neu.invalid/icon.png',
-      } as any);
-
-      expect(iconDiscovery.fetchIconBytes).toHaveBeenCalledWith('https://neu.invalid/icon.png');
-      expect(updated.iconVersion).toBe(1);
-    });
  });
 });
@@ -9,7 +9,6 @@ import {
  Logger,
  NotFoundException,
  PayloadTooLargeException,
-  UnprocessableEntityException,
 } from '@nestjs/common';
 import type { UploadedFileLike } from '../auth/types/auth-user';
 import { PrismaService } from '../prisma/prisma.service';
@@ -77,12 +76,16 @@ import { IconDiscoveryService, normalizeUrl } from './icon-discovery.service';
 *   Datei, dann die Zeile; scheitert die Zeile, wird die neue Datei wieder
 *   entfernt. Entfernen/Loeschen aktualisiert zuerst die Zeile, ein
 *   Dateifehler wird protokolliert und geschluckt.
- * - Abrufprobe: `assertIconUrlLoadable()` ruft `fetchIconBytes` einmal ab,
- *   um eine im Formular NICHT abrufbare Logo-Adresse (z. B. hinter einer
- *   Cloudflare-Pruefung) mit `UnprocessableEntityException` (422) statt
- *   stiller Speicherung abzuweisen — keine Umgehung von Bot-Sperren, nur
- *   derselbe Abruf, den `GET /favorites/:id/icon` ohnehin ausloest.
+ * - 260929-lh3 (loest die Abrufprobe von 260923-lrr ab): eine ausdrueckliche
+ *   Logo-Adresse wird nur auf Form (http/https, <= 2048 Zeichen) geprueft und
+ *   auch gespeichert, wenn der Server sie nicht abrufen kann — der Browser der
+ *   Kachel laedt sie dann direkt. Ein hochgeladenes Symbol wird von einer
+ *   neuen, abweichenden Adresse verdraengt (Vorrang der Datei sonst: Adresse
+ *   gespeichert, aber unsichtbar).
 */
+/** Hoechstlaenge einer ausdruecklichen Logo-Adresse (260929-lh3). */
+const ICON_URL_MAX_LENGTH = 2048;
+
@Injectable()
 export class FavoritesService {
  private readonly logger = new Logger(FavoritesService.name);
@@ -107,19 +110,31 @@ export class FavoritesService {
  }

  /**
-   * Prueft, ob sich das Bild unter `iconUrl` serverseitig abrufen laesst
-   * (260923-lrr) — derselbe `fetchIconBytes`-Aufruf, den `getIconBytes`
-   * ohnehin ausloest, hier nur zur Speicherzeit als Probe. Jeder Fehler
-   * (SSRF-Ablehnung, Zeitgrenze, kein `image/*`, Cloudflare-Pruefung o. ae.)
-   * wird zu derselben deutschen 422-Meldung — keine Unterscheidung, aus der
-   * sich etwas ueber die gepruefte Adresse ablesen liesse.
+   * Prueft eine ausdruecklich eingetragene Logo-Adresse NUR auf Form (260929-lh3):
+   * gueltige http/https-Adresse, hoechstens 2048 Zeichen. Bewusst KEIN
+   * serverseitiger Abruf mehr — Server wie docuvita liefern dem Server ein
+   * 404/HTML, dem Browser aber das Bild; die fruehere Abrufprobe (422,
+   * 260923-lrr) machte genau diese Adressen unspeicherbar. Entscheidung: auch
+   * eine Antwort, die der Server sieht und die kein Bild ist, weist NICHT ab —
+   * "Server bekommt kein Bild" heisst nicht "Browser bekommt keins", und der
+   * Server kann beides nicht unterscheiden. Der SSRF-Schutz bleibt unveraendert
+   * dort, wo der Server tatsaechlich abruft (`getIconBytes`/Erkennung); scheitert
+   * der Proxy, laedt die Kachel die Adresse direkt im Browser.
   */
-  private async assertIconUrlLoadable(iconUrl: string): Promise<void> {
+  private assertIconUrlWellFormed(iconUrl: string): void {
+    let parsed: URL | null = null;
    try {
-      await this.iconDiscovery.fetchIconBytes(iconUrl);
+      parsed = new URL(iconUrl);
    } catch {
-      throw new UnprocessableEntityException(
-        'Das Bild unter dieser Adresse konnte nicht geladen werden. Die Seite blockiert vermutlich automatische Abrufe (zum Beispiel durch eine Cloudflare-Prüfung) oder ist nicht erreichbar. Bitte laden Sie das Symbol stattdessen hoch.',
+      parsed = null;
+    }
+    if (
+      parsed === null ||
+      (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') ||
+      iconUrl.length > ICON_URL_MAX_LENGTH
+    ) {
+      throw new BadRequestException(
+        'Die Logo-Adresse muss eine gültige http- oder https-Adresse sein (höchstens 2048 Zeichen).',
      );
    }
  }
@@ -129,8 +144,8 @@ export class FavoritesService {
   * Verifies the target widget belongs to the caller BEFORE any icon
   * discovery network call (T-GWH-05).
   * If iconUrl is not provided, triggers server-side icon discovery with SSRF protection.
-   * If iconUrl IS provided (260923-lrr), it must load successfully or the
-   * create is rejected with 422 — nothing is written on a failed probe.
+   * If iconUrl IS provided it is stored as given after a form check only
+   * (260929-lh3, see assertIconUrlWellFormed) — no server-side fetch.
   */
  async create(tenantId: string, userId: string, dto: CreateFavoriteDto) {
    const tenantPrisma = forTenant(this.prisma, tenantId, userId);
@@ -154,8 +169,8 @@ export class FavoritesService {
    let iconUrl = dto.iconUrl ?? null;

    if (iconUrl) {
-      // 260923-lrr: explizit uebergebene Adresse wird einmal probiert.
-      await this.assertIconUrlLoadable(iconUrl);
+      // 260929-lh3: nur Formpruefung, kein serverseitiger Abruf.
+      this.assertIconUrlWellFormed(iconUrl);
    } else {
      // Server-side icon discovery (D-05) — only when caller did not supply an icon
      iconUrl = await this.iconDiscovery.discoverFavoriteIconUrl(url);
@@ -179,10 +194,11 @@ export class FavoritesService {
   * Verifies userId ownership before applying changes (T-08-06).
   * Accepts null as an explicit value for iconUrl (clears stored icon).
   *
-   * 260923-lrr: eine neue, vom gespeicherten Wert ABWEICHENDE `iconUrl`
-   * durchlaeuft die Abrufprobe (`assertIconUrlLoadable`), bevor irgendetwas
-   * geschrieben wird; misslingt sie, bleibt die Zeile unveraendert. Jede
-   * tatsaechliche Aenderung der Symbolquelle erhoeht `iconVersion`.
+   * 260929-lh3: eine neue, vom gespeicherten Wert ABWEICHENDE `iconUrl`
+   * durchlaeuft nur die Formpruefung (`assertIconUrlWellFormed`), bevor
+   * irgendetwas geschrieben wird; sie wird auch gespeichert, wenn der Server
+   * sie nicht abrufen kann. Jede tatsaechliche Aenderung der Symbolquelle
+   * erhoeht `iconVersion`.
   */
  async update(tenantId: string, id: string, userId: string, dto: UpdateFavoriteDto) {
    const tenantPrisma = forTenant(this.prisma, tenantId, userId);
@@ -205,8 +221,8 @@ export class FavoritesService {
    if ('iconUrl' in dto) {
      if (dto.iconUrl) {
        if (dto.iconUrl !== link.iconUrl) {
-          // 260923-lrr: nur eine NEUE, abweichende Adresse wird probiert.
-          await this.assertIconUrlLoadable(dto.iconUrl);
+          // 260929-lh3: nur eine NEUE, abweichende Adresse wird geprueft (Form).
+          this.assertIconUrlWellFormed(dto.iconUrl);
        }
        // Explicit icon URL supplied — respect it as-is.
        data.iconUrl = dto.iconUrl;
@@ -133,6 +133,54 @@ describe('IconDiscoveryService.discoverFavoriteIconUrl', () => {
  });
 });

+describe('IconDiscoveryService.discoverFavoriteIconUrl — Seite mit Fehlerstatus (260929-lh3)', () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+    vi.unstubAllGlobals();
+  });
+
+  function htmlResponse(status: number, html: string) {
+    return {
+      ok: status >= 200 && status < 300,
+      status,
+      headers: {
+        get: (n: string) =>
+          n.toLowerCase() === 'content-type' ? 'text/html; charset=utf-8' : null,
+      },
+      text: async () => html,
+    };
+  }
+
+  it('Seite antwortet 400, traegt aber <link rel="SHORTCUT ICON"> (docuvita) -> dieser Verweis wird genutzt', async () => {
+    const html =
+      '<html><head><link rel="SHORTCUT ICON" type="image/png" href="/webclient/docuvita/resources/brandimage/favicon.ico" /></head></html>';
+    vi.stubGlobal('fetch', vi.fn().mockResolvedValue(htmlResponse(400, html)));
+
+    const icon = await new IconDiscoveryService().discoverFavoriteIconUrl(
+      'http://8.8.8.8/server/services/web/',
+    );
+
+    expect(icon).toBe('http://8.8.8.8/webclient/docuvita/resources/brandimage/favicon.ico');
+  });
+
+  it('Fehlerseite ohne Symbol-Verweis, nur og:image -> Rueckfall <origin>/favicon.ico (og:image einer Fehlerseite zaehlt nicht)', async () => {
+    const html = '<html><head><meta property="og:image" content="https://cdn.invalid/x.png"></head></html>';
+    vi.stubGlobal('fetch', vi.fn().mockResolvedValue(htmlResponse(404, html)));
+
+    const icon = await new IconDiscoveryService().discoverFavoriteIconUrl('http://8.8.8.8/x');
+
+    expect(icon).toBe('http://8.8.8.8/favicon.ico');
+  });
+
+  it('fetchIconBytes bleibt streng: Fehlerstatus -> wirft (kein allowErrorStatus fuer Bilder)', async () => {
+    vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ...htmlResponse(404, ''), headers: { get: () => 'text/html' } }));
+
+    await expect(
+      new IconDiscoveryService().fetchIconBytes('http://8.8.8.8/favicon.ico'),
+    ).rejects.toThrow();
+  });
+});
+
 describe('IconDiscoveryService.fetchIconBytes', () => {
  afterEach(() => {
    vi.restoreAllMocks();
@@ -49,6 +49,8 @@ const LENIENT_TLS_AGENT = new Agent({ connect: { rejectUnauthorized: false } });
 type FetchHtmlResult = {
  html: string;
  finalUrl: string;
+  /** false = die Seite antwortete mit einem Fehlerstatus (z. B. 400/404), lieferte aber HTML (260929-lh3). */
+  ok: boolean;
 };

 function isPrivateIpv4(address: string): boolean {
@@ -202,7 +204,11 @@ function toAbsoluteUrl(value: string | undefined, base: string): string | null {
  }
 }

-function extractIconFromHtml(html: string, baseUrl: string): string | null {
+function extractIconFromHtml(
+  html: string,
+  baseUrl: string,
+  linkTagsOnly = false,
+): string | null {
  const linkTags = html.match(/<link\b[^>]*>/gi) ?? [];
  const metaTags = html.match(/<meta\b[^>]*>/gi) ?? [];

@@ -238,6 +244,10 @@ function extractIconFromHtml(html: string, baseUrl: string): string | null {

  if (imageSrc) return imageSrc;

+  // 260929-lh3: eine Fehlerseite (Status != 2xx) traegt kein Vorschaubild der
+  // Seite — nur die ausdruecklichen Symbol-Verweise (<link rel=...icon>) zaehlen.
+  if (linkTagsOnly) return null;
+
  const metaImage = metaTags
    .map((tag) => parseAttributes(tag))
    .map((a) => ({
@@ -266,7 +276,13 @@ function extractIconFromHtml(html: string, baseUrl: string): string | null {
 */
 async function fetchWithRedirectGuard(
  pageUrl: URL,
-  options: { accept: string; timeoutMs: number; userAgent?: string },
+  options: {
+    accept: string;
+    timeoutMs: number;
+    userAgent?: string;
+    /** 260929-lh3: auch eine 4xx/5xx-Antwort zurueckgeben (nur fuer die HTML-Suche). */
+    allowErrorStatus?: boolean;
+  },
 ): Promise<{ response: UndiciResponse; finalUrl: URL } | null> {
  let currentUrl = pageUrl;

@@ -298,7 +314,7 @@ async function fetchWithRedirectGuard(
        continue;
      }

-      if (!response.ok) return null;
+      if (!response.ok && !options.allowErrorStatus) return null;

      return { response, finalUrl: currentUrl };
    } catch {
@@ -315,6 +331,9 @@ async function fetchHtml(pageUrl: URL): Promise<FetchHtmlResult | null> {
  const result = await fetchWithRedirectGuard(pageUrl, {
    accept: 'text/html,application/xhtml+xml,*/*',
    timeoutMs: HTML_FETCH_TIMEOUT_MS,
+    // 260929-lh3: Server wie docuvita antworten dem Server mit 400, tragen im
+    // HTML aber trotzdem den <link rel=icon> — den Verweis wollen wir haben.
+    allowErrorStatus: true,
  });

  if (!result) return null;
@@ -328,6 +347,7 @@ async function fetchHtml(pageUrl: URL): Promise<FetchHtmlResult | null> {
  return {
    html: html.slice(0, MAX_HTML_CHARS), // T-08-09: HTML cap
    finalUrl: result.finalUrl.toString(),
+    ok: result.response.ok,
  };
 }

@@ -350,7 +370,10 @@ export class IconDiscoveryService {

      if (!htmlResult) return fallback;

-      return extractIconFromHtml(htmlResult.html, htmlResult.finalUrl) ?? fallback;
+      return (
+        extractIconFromHtml(htmlResult.html, htmlResult.finalUrl, !htmlResult.ok) ??
+        fallback
+      );
    } catch {
      return fallback;
    }