tessera-ctl/.planning/phases/08-dashboard-widgets-vollimplementierung/08-REVIEW.md

---
phase: 08-dashboard-widgets-vollimplementierung
reviewed: 2026-07-01T00:00:00Z
depth: standard
files_reviewed: 24
files_reviewed_list:
  - apps/api/prisma/schema.prisma
  - apps/api/src/app.module.ts
  - apps/api/src/dashboard/dto/create-widget.dto.ts
  - apps/api/src/favorites/dto/create-favorite.dto.ts
  - apps/api/src/favorites/dto/update-favorite.dto.ts
  - apps/api/src/favorites/favorites.controller.ts
  - apps/api/src/favorites/favorites.module.ts
  - apps/api/src/favorites/favorites.service.ts
  - apps/api/src/favorites/icon-discovery.service.ts
  - apps/web/src/app/(portal)/page.tsx
  - apps/web/src/components/dashboard/widget-catalog-modal.tsx
  - apps/web/src/components/dashboard/widget-registry.test.tsx
  - apps/web/src/components/dashboard/widget-registry.tsx
  - apps/web/src/components/dashboard/widgets/calculator-widget.test.tsx
  - apps/web/src/components/dashboard/widgets/calculator-widget.tsx
  - apps/web/src/components/dashboard/widgets/favorites-widget.test.tsx
  - apps/web/src/components/dashboard/widgets/favorites-widget.tsx
  - apps/web/src/components/dashboard/widgets/link-widget.test.tsx
  - apps/web/src/components/dashboard/widgets/link-widget.tsx
  - apps/web/src/components/dashboard/widgets/stopwatch-widget.test.tsx
  - apps/web/src/components/dashboard/widgets/stopwatch-widget.tsx
  - apps/web/src/lib/favorites-api.ts
  - apps/web/src/messages/de.json
  - apps/web/src/messages/en.json
findings:
  critical: 2
  warning: 5
  info: 3
  total: 10
status: issues_found
---

# Phase 08: Code Review Report

**Reviewed:** 2026-07-01
**Depth:** standard
**Files Reviewed:** 24
**Status:** issues_found

## Summary

Phase 8 adds four new dashboard widgets (Calculator, Stopwatch, Favorites, Link) plus the backing NestJS Favorites API and server-side icon/favicon discovery. The overall architecture is solid: per-user ownership checks, SSRF-aware icon fetching, clean widget-registry wiring pattern, and good test coverage. Two blockers require fixing before shipping: an SSRF bypass via IPv4-mapped IPv6 addresses in the icon discovery service, and a data-exposure flaw in the Favorites list endpoint that returns all of a user's favorites when the required `widgetId` parameter is omitted. Five warnings round out the findings, mostly in the frontend error-message handling and a schema data-integrity gap.

## Narrative Findings (AI reviewer)

## Critical Issues

### CR-01: SSRF bypass via IPv4-mapped IPv6 addresses in icon-discovery.service.ts

**File:** `apps/api/src/favorites/icon-discovery.service.ts:55-68`

**Issue:** `isPrivateIpv6` only blocks `::ffff:127.x`, `::ffff:10.x`, and `::ffff:192.168.x`. The entire RFC 1918 172.16–31.x.x range and the 169.254.x.x link-local range are absent. Because `net.isIP('::ffff:172.31.0.1')` returns `6`, the code takes the IPv6 branch and finds no matching prefix, concluding the address is public — allowing the server to make outbound requests to internal infrastructure. An authenticated user who enters `http://[::ffff:172.16.0.1]/` as a favorite URL triggers this path.

```typescript
// Current — incomplete
function isPrivateIpv6(address: string): boolean {
  const lower = address.toLowerCase();
  return (
    lower === '::' ||
    lower === '::1' ||
    lower.startsWith('fc') ||
    lower.startsWith('fd') ||
    lower.startsWith('fe80:') ||
    lower.startsWith('::ffff:127.') ||
    lower.startsWith('::ffff:10.') ||
    lower.startsWith('::ffff:192.168.')
  );
}
```

**Fix:** Add the missing IPv4-mapped IPv6 ranges. The simplest safe approach is to detect the `::ffff:` prefix, parse the embedded IPv4 quad, and delegate to `isPrivateIpv4`:

```typescript
function isPrivateIpv6(address: string): boolean {
  const lower = address.toLowerCase();

  if (
    lower === '::' ||
    lower === '::1' ||
    lower.startsWith('fc') ||
    lower.startsWith('fd') ||
    lower.startsWith('fe80:') ||
    lower.startsWith('ff')
  ) {
    return true;
  }

  // IPv4-mapped IPv6  ::ffff:<ipv4> — delegate to isPrivateIpv4
  const v4MappedMatch = lower.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/);
  if (v4MappedMatch) {
    return isPrivateIpv4(v4MappedMatch[1]);
  }

  return false;
}
```

---

### CR-02: GET /favorites returns all user favorites when widgetId is omitted

**File:** `apps/api/src/favorites/favorites.controller.ts:48-55`

**Issue:** The `widgetId` query parameter is not validated. If a client omits it (`GET /favorites` without a `?widgetId=` query), NestJS assigns `undefined` to the `string`-typed parameter at runtime. The service passes `undefined` to Prisma's `where` clause:

```typescript
// favorites.service.ts — list()
return this.prisma.favoriteLink.findMany({
  where: { userId, widgetId },  // widgetId is undefined → Prisma drops the field
  orderBy: [{ position: 'asc' }, { title: 'asc' }],
});
```

Prisma v7 silently drops `undefined` fields from `where` clauses. The query becomes `WHERE userId = ?` with no widget scope, returning **all** of the user's favorites across every widget instance. This breaks the per-widget isolation that T-08-06 / Pitfall 3 are designed to enforce, and exposes more data than the caller is entitled to.

**Fix:** Add explicit validation in the controller with a `ParseUUIDPipe` or a `@IsUUID()` guard, and add a null-guard in the service:

```typescript
// Controller — require widgetId as a valid UUID
import { ParseUUIDPipe } from '@nestjs/common';

@Get()
async list(
  @Query('widgetId', ParseUUIDPipe) widgetId: string,
  @Req() req: Request,
) {
  const { userId } = this.extractContext(req);
  return this.favoritesService.list(userId, widgetId);
}
```

```typescript
// Service — defensive guard
async list(userId: string, widgetId: string) {
  if (!widgetId) throw new BadRequestException('widgetId is required');
  return this.prisma.favoriteLink.findMany({
    where: { userId, widgetId },
    orderBy: [{ position: 'asc' }, { title: 'asc' }],
  });
}
```

---

## Warnings

### WR-01: link-widget.tsx — all error paths store raw i18n key instead of translated string

**File:** `apps/web/src/components/dashboard/widgets/link-widget.tsx:66,110,147,158`

**Issue:** Every `setError(...)` call in this file stores the literal string `'link.error'` rather than calling `t('link.error')`. The error `<p>` at line 271 renders `{error}` directly, so users see `link.error` in the UI instead of "Fehler beim Laden des Links" / "Error loading link".

```typescript
// All four error sites are identical — representative example:
} catch {
  setError('link.error');  // raw key, not t('link.error')
}
```

**Fix:** Replace each with the translated call:

```typescript
} catch {
  setError(t('link.error'));
}
```

---

### WR-02: favorites-widget.tsx — inconsistent error handling between load and action paths

**File:** `apps/web/src/components/dashboard/widgets/favorites-widget.tsx:74,122,158,168`

**Issue:** The `useEffect` load handler stores the raw key `'favorites.error'` (line 74), while the three action handlers (`handleAdd`, `handleSaveEdit`, `handleDelete`) all store `t('favorites.error')` — the translated string. A load failure will display the raw key to the user; action failures will display the correct translation.

```typescript
// useEffect — raw key (wrong):
if (!cancelled) setError('favorites.error');

// handleAdd — translated (correct):
setError(t('favorites.error'));
```

**Fix:** Change line 74 to use the translation function:

```typescript
if (!cancelled) setError(t('favorites.error'));
```

---

### WR-03: widget-catalog-modal.tsx — `aria-hidden="true"` on the dialog's containing div

**File:** `apps/web/src/components/dashboard/widget-catalog-modal.tsx:59-61`

**Issue:** The outermost wrapper `<div>` at line 59 that acts as the click-to-close backdrop carries `aria-hidden="true"`. This attribute propagates to all descendants, including the inner `<div role="dialog">` at line 66. Screen readers and other assistive technologies will therefore ignore the entire modal — the widget catalog is completely inaccessible when it opens.

```tsx
// Problematic:
<div
  className="fixed inset-0 z-50 flex items-center justify-center"
  onClick={onClose}
  aria-hidden="true"   // ← hides everything inside, including the dialog
>
  <div role="dialog" aria-modal="true" aria-label={t('catalogTitle')}>
    ...
  </div>
</div>
```

**Fix:** Remove `aria-hidden` from the outer wrapper and apply it only to the backdrop overlay; the dialog should remain visible to assistive technologies:

```tsx
<div
  className="fixed inset-0 z-50 flex items-center justify-center"
  onClick={onClose}
>
  {/* Backdrop — visually only, hidden from AT */}
  <div className="fixed inset-0 bg-black/50" aria-hidden="true" />

  {/* Dialog — accessible */}
  <div
    ref={dialogRef}
    role="dialog"
    aria-modal="true"
    aria-label={t('catalogTitle')}
    ...
  >
```

---

### WR-04: calculator-widget.tsx — "M" button is a duplicate of "MR" with identical handler

**File:** `apps/web/src/components/dashboard/widgets/calculator-widget.tsx:339-340`

**Issue:** The memory button row renders two buttons that both call `memoryRecall`:

```tsx
<button type="button" className={memBtn} onClick={memoryRecall} disabled={memory === 0}>MR</button>
<button type="button" className={memBtn} onClick={memoryRecall} disabled={memory === 0}>M</button>
```

Standard calculator conventions define "M" as a memory-panel / memory-view button distinct from "MR" (memory recall). Here both call the same function. This appears to be a copy-paste error when implementing the "M" button. The button provides no unique functionality and may confuse users expecting memory-panel behavior.

**Fix:** If "M" is intended to display the stored memory value (common in modern calculators), implement a distinct handler. If the label is meant to be "MS" (memory store), replace the onClick and label accordingly:

```tsx
{/* If M = MS (memory store) */}
<button type="button" className={memBtn} onClick={memoryStore}>MS</button>
```

---

### WR-05: schema.prisma — FavoriteLink.widgetId has no FK relation to WidgetInstance

**File:** `apps/api/prisma/schema.prisma:237-252`

**Issue:** `FavoriteLink.widgetId` is a plain `String` with an index but no `@relation` referencing `WidgetInstance`. When a `WidgetInstance` record is deleted (e.g., user removes a widget from their dashboard), all associated `FavoriteLink` rows are silently orphaned. The orphaned rows will accumulate in the database without any mechanism to identify or clean them up.

```prisma
model FavoriteLink {
  id        String   @id @default(uuid())
  userId    String
  tenantId  String
  widgetId  String   // ← no @relation, no onDelete cascade
  ...
  @@index([widgetId])
}
```

**Fix:** Add a relation to `WidgetInstance` with cascade delete, or add an application-level cleanup hook in the widget removal service. The Prisma-schema approach:

```prisma
model FavoriteLink {
  id             String         @id @default(uuid())
  userId         String
  tenantId       String
  widgetId       String
  widgetInstance WidgetInstance @relation(fields: [widgetId], references: [id], onDelete: Cascade)
  ...
}

model WidgetInstance {
  ...
  favoriteLinks FavoriteLink[]
}
```

---

## Info

### IN-01: create-widget.dto.ts — stale comment

**File:** `apps/api/src/dashboard/dto/create-widget.dto.ts:4`

**Issue:** The JSDoc comment reads "widgetType must be one of the four supported types" but the `@IsIn([...])` decorator lists eight types. The comment was not updated when Phase 8 types were added.

**Fix:** Update the comment: "widgetType must be one of the eight supported types."

---

### IN-02: calculator-widget.tsx — "Fehler" error string hardcoded in German

**File:** `apps/web/src/components/dashboard/widgets/calculator-widget.tsx:25,109`

**Issue:** The display error string "Fehler" is hardcoded at two points: in `formatNumber` (line 25) and compared against in `displayIsError` (line 109). English locale users see "Fehler" on division-by-zero or arithmetic overflow instead of "Error". The rest of the widget uses `useTranslations('widgets')`.

**Fix:** Add a translation key and use it:

```json
// en.json / de.json under "widgets.calculator":
"error": "Error"   // de: "Fehler"
```

```typescript
// In CalculatorWidget:
const errorLabel = t('calculator.error');
const displayIsError = display === errorLabel;

// In formatNumber (make it accept the label or keep it pure and check outside):
// Alternative: return a sentinel like '!ERR!' and map to translated string at render time
```

---

### IN-03: page.tsx — hardcoded "Loading..." text instead of translation key

**File:** `apps/web/src/app/(portal)/page.tsx:55`

**Issue:** The loading fallback renders `<p>Loading...</p>` as a hardcoded English string. Every other loading state in the codebase uses `t('common.loading')` (which exists in both `de.json` and `en.json`).

**Fix:**

```tsx
// Replace:
<p className="text-sm text-muted-foreground">Loading...</p>

// With:
<p className="text-sm text-muted-foreground">{t('common.loading')}</p>
```

---

_Reviewed: 2026-07-01_
_Reviewer: Claude (gsd-code-reviewer)_
_Depth: standard_