Files
paliad/docs/improvement-audit.md
cronus bcdd3d7a59 docs(audit): product audit + improvement roadmap (t-paliad-015)
Post-Phase-A–J full-product audit: UX, code, content, architecture,
ops. 5 Critical findings (JWT signature bypass, dashboard Termine
leak, parteien/termin delete policy gap, @hoganlovells-only email
gate, CALDAV_ENCRYPTION_KEY missing from compose), 8 Important, 10
Polish, 11 Feature ideas, 14 tech-debt items. Each item has a file
reference and a concrete fix.

Top-two exploit-paths (detailed in §1):
  1. internal/auth/auth.go:178 — middleware decodes JWT exp but never
     verifies the signature; sub-claim is trusted downstream by every
     service. Any authenticated cookie → impersonate any user.
  2. internal/services/dashboard_service.go:245 — personal Termine
     leaked cross-user on the /dashboard "Kommende Termine" list
     (missing created_by filter on the akte_id IS NULL branch).
2026-04-18 01:22:23 +02:00

25 KiB
Raw Blame History

Paliad — Product Audit & Improvement Roadmap

Prepared by cronus (inventor) on 2026-04-18 for task t-paliad-015. Scope: complete code, UX, content, architecture, and ops audit after the 17 000-line KanzlAI → Paliad integration (Phases AJ, April 2026).

Audit posture: first-real-user (HLC patent lawyer, Munich) and long-term architect. Items are prioritised and actionable — each has a file reference and a suggested fix. No hour estimates; effort is expressed as S / M / L (small / medium / large).

  • S = <1 day of focused work; single file or localised change
  • M = multi-file change, migration, or non-trivial design
  • L = new subsystem, cross-cutting rework, or external dep

TL;DR — the two things to fix today

  1. The session middleware does not verify JWT signatures and the Go backend extracts auth.uid() from that unverified token to gate every database read and write. A forged cookie with any sub and a future exp gives an attacker access to any user's Akten, including admin. See C-1 below. internal/auth/auth.go:178 + internal/auth/user.go:60.
  2. The dashboard leaks every user's personal Termine to every other logged-in user for the next 7 days (title, location, description, start/end). internal/services/dashboard_service.go:245. See C-2.

Both are one-file fixes and must ship before this audit reaches a wider pilot group.


1 — Critical (fix immediately)

C-1. Session JWTs are not signature-verified

File: internal/auth/auth.go:178, internal/auth/user.go:60 Severity: Critical (authZ bypass) Effort: M

Client.Middleware accepts the session cookie, parses exp via DecodeJWTExpiry, and calls next if the token is unexpired. The signature is never checked. WithUserID then base64-decodes the sub claim straight into the request context. AkteService.GetByID and every other service trusts that UUID as the authenticated user.

Exploit: craft any JWT with exp in the future and sub = <admin-user-uuid>. Set it as the patholo_session cookie. The Go backend uses a service-role Postgres connection, so RLS policies do not run and the app-level visibility check sees the forged UUID as a real admin. Effectively: anyone with a Paliad cookie can impersonate anyone.

Fix:

  • Fetch the Supabase project's JWT signing key (JWKS endpoint: ${SUPABASE_URL}/auth/v1/.well-known/jwks.json) or the shared SUPABASE_JWT_SECRET and verify the token in Middleware before trusting any claim.
  • Cache the JWKS for 1 hour; rotate on kid mismatch.
  • WithUserID should read the verified claims, not re-decode the raw cookie.
  • Consider github.com/golang-jwt/jwt/v5 — already idiomatic for Go.

Related: internal/services/akte_service.go:18-24 explicitly documents that RLS "does not kick in because the backend does not provide a JWT-backed auth.uid()" — so the app-layer predicate is the only gate. The JWT must therefore be trusted.


C-2. Dashboard leaks every user's personal Termine cross-user

File: internal/services/dashboard_service.go:232-256 Severity: Critical (privacy / confidentiality) Effort: S

WHERE t.start_at >= $4
  AND t.start_at <  ($4 + interval '7 days')
  AND (t.akte_id IS NULL              -- ← ANY personal Termin, any user
       OR a.firm_wide_visible = true
       OR a.owning_office = $1
       OR $2::uuid = ANY (a.collaborators)
       OR $3 = 'admin')

Personal Termine (akte_id IS NULL) are creator-only by contract (TerminService.canSee enforces created_by = userID). The dashboard forgets this filter and returns up to 10 such rows for any user — title, location, description, times included.

In practice an associate's "Bewerbungsgespräch bei Kanzlei X" is visible to partners and vice-versa.

Fix: change the personal-Termin branch to (t.akte_id IS NULL AND t.created_by = $2::uuid) — mirrors the already- correct rule in termin_service.go:103.


C-3. Any user with visibility can delete Akte children (Parteien, Termine)

File:

  • internal/services/parteien_service.go:93-111 — Delete has no role gate.
  • internal/services/termin_service.go:357-398 — Akte-linked Termine: only personal Termine check creator; Akte-linked pass through with just visibility.

Severity: Critical (data loss; inconsistent with Fristen policy) Effort: S

An associate in London, viewing a Munich firm-wide Akte, can DELETE /api/parteien/{id} and erase the Klägerin-record, or delete any hearing from the Akte's calendar. FristService.Delete already scopes to partner|admin only — the other child services should follow suit.

Fix: require user.Role in {partner, admin} (or created_by = userID) for delete on ParteienService and TerminService. Apply the same rule to Update on both.


C-4. Email gate still pins to @hoganlovells.com

File: internal/handlers/auth.go:113-116 Severity: Critical (post-merger lockout) Effort: S

HLC email domains (@hlc.com, @hlc.de) cannot register or log in. Login-form placeholders still say name@hoganlovells.com.

Fix:

  • Replace isHoganLovellsEmail with an env-configurable whitelist, default hoganlovells.com,hlc.com,hlc.de (design §2 already requested this).
  • Update placeholders in frontend/src/login.tsx:26,34 + the login.hint i18n key (de + en).
  • Error strings ("Zugang nur für @hoganlovells.com …") should become "… für autorisierte HLC-E-Mail-Adressen."

C-5. CALDAV_ENCRYPTION_KEY not wired into production compose

File: docker-compose.yml:4-12 Severity: Critical in effect (feature silently disabled) Effort: S

The compose file declares SUPABASE_URL, SUPABASE_ANON_KEY, GITEA_TOKEN, DATABASE_URL. It does not pass CALDAV_ENCRYPTION_KEY. On the Dokploy compose (Zx147ycurfYagKRl_Zzyo) this means cmd/server/main.go:66 logs "CALDAV_ENCRYPTION_KEY not set — CalDAV endpoints will return 501" and the entire Termine-sync feature is dead on paliad.de. Users who save their CalDAV settings see a 501 from /api/caldav-config and conclude the product is broken.

Fix: add - CALDAV_ENCRYPTION_KEY=${CALDAV_ENCRYPTION_KEY} (and while we're at it, a placeholder ANTHROPIC_API_KEY commented out so Phase H reactivation is one uncomment). Set the actual key on Dokploy.


2 — Important (fix this week)

I-1. Dokumente tab on Akten detail is a dead placeholder

File: frontend/src/akten-detail.tsx:215-222, frontend/src/client/i18n.ts:468,1241 Severity: Important (visible UI dead-end; leaks internal phase names) Effort: S

The tab is rendered, clickable, and shows the text "Dokumenten-Upload folgt in Phase H." — a strong UX signal that the product is unfinished. Phase H is deferred indefinitely.

Fix (pick one):

  • Hide the Dokumente tab entirely until there is real content — drop it from the VALID_TABS list in akten-detail.ts:62 and the TSX tabs strip.
  • Or keep it visible but replace the copy with a neutral "Dokumenten- Upload in Planung" (no phase leak) and a discreet CTA to vote/express interest (write to paliad_feature_interest).

I'd hide it — dead tabs are worse than missing features.

I-2. Office labels in German not translated to English

File: frontend/src/index.tsx:122-128 Severity: Important (i18n regression visible on landing page) Effort: S

Only index.munich has a data-i18n key. Düsseldorf, Hamburg, Amsterdam, London, Paris, Mailand render raw German in EN mode. "Mailand" → "Milan" is the most obvious miss; Düsseldorf/London/Paris are correct in both languages but the fact that only one is i18n'd is a consistency bug waiting for the next new office.

Fix: add index.office.munich|duesseldorf|hamburg|amsterdam|london| paris|milan keys (de: "München / Düsseldorf / Hamburg / Amsterdam / London / Paris / Mailand"; en: "Munich / Düsseldorf / Hamburg / Amsterdam / London / Paris / Milan").

I-3. Gerichtsverzeichnis UPC URLs redirect

File: internal/handlers/gerichte.go (45 occurrences of unifiedpatentcourt.org, no hyphens) Severity: Important (one redirect per link click; flagged as wrong by link-checkers) Effort: S

Canonical UPC website is https://www.unified-patent-court.org (with hyphens). The no-hyphen form returns HTTP 403 challenge pages from Cloudflare on naive curl but does resolve to the same destination. internal/handlers/links.go uses the canonical form — the two files are inconsistent. Pick one. Canonical (hyphenated) is safer.

Fix: sed-replace unifiedpatentcourt.orgunified-patent-court.org across gerichte.go (and re-verify the deep paths still resolve — some /en/court/court-appeal style URLs may have moved).

I-4. loadRecentActivity omits personal Termine audit rows (not a bug yet, but will be)

File: internal/services/dashboard_service.go:259-287 Severity: Medium-Important (correctness risk) Effort: S

loadRecentActivity joins akten_eventsakten, so only Akte-attached events appear. That is correct today because personal Termine explicitly skip the audit (per Phase F memory). If a future contributor adds a personal-Termin event type without reading the design, this query will silently drop it — add a comment or switch to LEFT JOIN + WHERE a.id IS NULL OR <visibility>.

I-5. /api/akten/{id}/events has no pagination or size limit

File: internal/services/akte_service.go:368-383 Severity: Important (scalability; long-running Akten) Effort: M

ListEvents returns every row ever written to akten_events for an Akte. A three-year litigation with CalDAV sync and daily notizen edits could easily reach 510 k rows. The Verlauf tab will then ship 2 MB of JSON on each tab-click.

Fix: add ?before=<uuid>&limit=50 cursor pagination and render "Load more" in the Verlauf tab. Already noted in the Phase E followup list, never actioned.

File: internal/handlers/glossar.go:33-118 (~86 entries) Severity: Important (content gap for the target audience) Effort: S

HLC's patent practice is a heavy SEP/FRAND shop; the glossary has zero entries for: FRAND, SEP, Standard-essentielles Patent, Patentpool, Anti- Anti-Suit Injunction, Injunction gap, Orange-Book-Verfahren, Huawei/ZTE-Verhandlungsmuster, RAND, ETSI IPR Policy, Patent-Hold-up, Patent-Hold-out. These are table-stakes for the intended user.

Fix: add ~12 entries under a new SEP/FRAND category (or stretch Litigation). Pull definitions from the canonical CJEU/BGH case law.

I-7. README is out of date

File: README.md:30-43,107 Severity: Important (onboarding drag) Effort: S

  • Migration list stops at 013; 014 (checklist_instances) is live.
  • Line 107 says "Phase I (Notizen) pending — service and UI aren't built yet"; it shipped on mai/knuth/phase-i-notizen (commit 5a9f8e5, 2026-04-17) with full service + handlers + UI.

Fix: refresh the migrations block (014_checklist_instances …) and the status paragraph. Phase I Done, Phase J docs-only done, infra retirement pending.

I-8. Legacy "patholo_" names everywhere

File: internal/auth/auth.go:17-18, internal/handlers/links.go:315, frontend/src/client/i18n.ts:6 (STORAGE_KEY = "patholo-lang"), frontend/src/client/*.ts (other localStorage keys), paliad_link_suggestions / paliad_link_feedback table names (compare vs. the older patholo_* references in the code). Severity: Important (brand inconsistency; minor confusion) Effort: M

Cookie name patholo_session, storage key patholo-lang, Supabase tables patholo_link_suggestions / patholo_link_feedback. Memory notes a deliberate decision to keep the cookie name so users aren't logged out — that's fine — but storage keys and table names can migrate without user impact.

Fix: plan a single branch that:

  • Renames STORAGE_KEYpaliad-lang with a one-shot migration from the old key (read old, write new, delete old) in i18n.ts init.
  • Renames tables to paliad_link_suggestions / paliad_link_feedback (migration + update callers in handlers/links.go).
  • Leaves the cookie name alone or migrates it with a dual-read grace period.

3 — Polish (nice to have)

internal/handlers/links.go:206-219 — two entries with URL: "#" render as clickable cards that go nowhere.

Fix: either remove the entries until real URLs land, or add a "Coming soon — suggest a URL" flag + disabled styling.

P-2. Dashboard says "Meine Mandate" but the nav and URL say "Akten"

frontend/src/dashboard.tsx:82, i18n key dashboard.matters.heading. The project's naming convention (CLAUDE.md) mandates Akten throughout — the term "Mandate" is explicitly historical. The dashboard heading should read "Meine Akten" / "My Matters".

Fix: change i18n text to "Meine Akten" (DE) and leave "My Matters" (EN). Rename i18n key if desired.

P-3. Login page placeholder still says name@hoganlovells.com

Covered by C-4. Mentioned again here because the hint "Nur für @hoganlovells.com Adressen." is a polish concern even after the whitelist change — update to "Nur für autorisierte HLC-Adressen."

frontend/src/components/Footer.tsx:7. Brand reads as old on every page. Switch to "HLC Patent Practice" post-merger (or drop the firm name entirely since Paliad is supposed to survive renames).

P-5. No explicit empty-state copy on Fristen-Kalender / Termine-Kalender

Check frontend/src/fristen-kalender.tsx, termine-kalender.tsx. Calendars with no events render an empty grid — add a subtle "Keine Fristen / Termine im ausgewählten Zeitraum." string.

P-6. Office names in AkteService.isValidOffice diverge from UI labels

akte_service.go:403-408 accepts keys munich, duesseldorf, hamburg, amsterdam, london, paris, milan. The models.go user has the same list. But the UI (landing page) writes "München", "Düsseldorf", "Mailand". Currently fine because admins edit office via internal role only, but any future user-facing office selector must map label → key — add a single source of truth (internal/calc/offices.go or similar: {key: "munich", labelDE: "München", labelEN: "Munich"}).

P-7. Glossar CSVs hard-coded in one file (230 lines)

Not a bug, but as the list grows toward 150+ terms it wants to move out of Go source into internal/handlers/glossar_data.go or a JSON blob in internal/data/glossar.json. Easier for non-devs to edit.

P-8. Dockerfile lacks HEALTHCHECK and runs as root

Dockerfile:13-19. No HEALTHCHECK, no USER nonroot. Both are easy wins for Dokploy's health surface and container hardening.

# before the CMD
HEALTHCHECK --interval=30s --timeout=3s CMD wget -qO- http://localhost:8080/ || exit 1
RUN addgroup -S paliad && adduser -S -G paliad paliad
USER paliad

Add a GET /healthz route that returns 200 without auth; current / redirects and wgets a 200 anyway, but an explicit probe is cleaner.

P-9. Landing-page copy still frames Paliad as "Paliad — Patentwissen für Hogan Lovells"

frontend/src/client/i18n.ts:38-48. With Phase 0 shipped, Paliad is more than a knowledge hub — it's the Aktenverwaltung. The hero section should call that out. Proposed: "Paliad — alles für den Patentalltag: Akten, Fristen, Termine und Wissen."

P-10. Kostenrechner PDF and Checklisten print — not verified by audit

The design says PDF export works. I did not test it; please verify after any CSS change that affects .tool-page / .print\:hide. Consider adding a visual-regression snapshot to the build.


4 — Features (prioritised backlog)

F-1. Global search (Ctrl-K / Cmd-K)

Impact: Very High Effort: M

A single keystroke that searches across Akten, Fristen, Termine, Parteien, Glossar, Gerichte, Links. Patent lawyers hop between matters constantly; the current UX requires clicking through 2 sidebar entries to find anything. Index on the backend with tsvector + pg_trgm, or build a client-side Lunr index for the static content and a single /api/search?q=… that fans out for user data. Match by Aktenzeichen, party name, Frist title.

F-2. Verfahrensleitfäden (procedure guides)

Impact: Very High (per the original roadmap, never shipped) Effort: L

Step-by-step UPC / BPatG / EPO workflows that stitch together checklists, deadline rules, and recommended templates. This is the biggest original-roadmap item still missing.

F-3. Expand the Downloads registry

Impact: High (2/10 on the roadmap, partial) Effort: M

Only HL Patents Style.dotm is wired up. mWorkRepo has more templates. Wire Vollmacht.dotm, Unterlassungserklärung.dotm, UPC-Klageschrift-Skeleton.dotm — mirror the existing proxy pattern.

F-4. Benachrichtigungen (notifications)

Impact: Medium-High Effort: L

Email digest of today's overdue/due Fristen + tomorrow's Termine, sent at 07:00 CET per user. Supabase pg_cron + Edge Functions or a Go scheduler in the Paliad binary. Opt-in per user.

F-5. What's New / Changelog in-app

Impact: Medium Effort: S

docs/changelog.md rendered under /whatsnew with a red dot on the sidebar when there's an entry newer than the user's last_seen_changelog timestamp. Cheap way to communicate feature releases.

F-6. Akten-Kurzinfo on hover (pop-over)

Impact: Medium Effort: S

Hovering an Aktenzeichen in any list (Dashboard activity, Fristen, Termine) shows a tooltip with matter title, court, next Frist. Reduces tab-switching.

F-7. Parteien mit Kontaktkarten (Update endpoint)

Impact: Medium Effort: S

Currently ParteienService only supports Create + Delete. No Update. Typos in a party name force a delete + re-add and lose the ID. Add PATCH /api/parteien/{id}.

F-8. Fristenrechner — "Als Frist speichern" mit Wiedervorlage

Impact: Medium Effort: S

When creating a Frist with a Rule, also derive a Wiedervorlage-Frist (warning-date = due - 14 d) and offer to create both in one click.

F-9. Dark mode

Impact: Low (but free with CSS variables) Effort: S

global.css already uses CSS variables heavily. Add a @media (prefers-color-scheme: dark) block + a manual toggle in the sidebar.

F-10. Document upload without AI (revisit Phase H)

Impact: Medium (still open per memory episode) Effort: M

The Supabase Storage upload path already works; only the AI-extraction part was rejected. A plain "upload PDF to Akte" with no auto-extraction is still useful and unblocks the Dokumente tab.

F-11. Akten-Tags / Labels

Impact: Medium Effort: M

Free-form tags on Akten (e.g., SEP, OLG-Düsseldorf, Q4-Priority) with a filter UI. Cheaper than a full custom-fields system and used extensively at comparable firms.


5 — Technical Debt

T-1. RLS policies exist but are never enforced

internal/db/migrations/007_rls_policies.up.sql defines full office- scoped RLS keyed off auth.uid(). The Go backend uses a service-role connection (db.OpenPool with DATABASE_URL), so those policies never evaluate — every query bypasses RLS by design.

This is correct today (service-role means app-level checks must be bulletproof, which they mostly are) but it means the RLS layer is dead weight that increases surface for a migration bug to produce a silently-wrong policy. Decide:

  • Option A: accept the "belt-and-braces" position, keep RLS, document loudly that it's not active in prod.
  • Option B: drop the RLS migration and policies — less code, less false sense of security.
  • Option C: switch to PostgREST-style per-request JWT connections so RLS actually runs. This is the most defensive but a substantial rewrite.

My recommendation: Option A, with an internal/db/RLS_NOTE.md and a SET row_security = off; (or equivalent) in the service-role connection so the policies don't quietly cost perf.

T-2. Go module path still mgit.msbls.de/m/patholo

go.mod:1 + every import statement. Not breaking, but a daily reminder of the old name. Rename on the next big refactor, not now — every file in the repo would churn.

T-3. calDAVClient hand-rolls iCal + WebDAV

Documented in the Phase F memory as a deliberate choice. Fine for now, but re-evaluate when any of these lands:

  • Importing foreign UIDs (currently skipped)
  • RRULE / VTIMEZONE / DTSTART-with-tzid
  • Multi-calendar support per user

At that point switch to github.com/emersion/go-ical + github.com/emersion/go-webdav.

T-4. _dev/mock_supabase_auth.sql lives inside migrations/

internal/db/migrations/_dev/. Embed.FS includes all files under migrations/ — the _dev directory is probably ignored by iofs.New (directories not matching the pattern NNN_*.sql are filtered) but it's one hop from a build bug. Move to internal/db/devtools/ or similar, outside the embed root.

T-5. Client-side duplicated helpers

Memory's Phase E followup notes: urgency-color + date-format JS helpers are duplicated across fristen.ts, fristen-detail.ts, fristen-kalender.ts, akten-detail.ts. Extract to frontend/src/client/fristen-shared.ts. Not urgent, but next time anyone touches the Fristen UI: do this first.

T-6. /api/fristen?status=all returns everything client-side filters

Per Phase E memory: fine at current volumes, but a partner with 500 completed Fristen will see the full 500 on every calendar load. Add server-side date-range filtering before that's a support ticket.

T-7. No error monitoring

The server logs to stdout (which Dokploy captures), but there is no Sentry / log-based alert wiring. Nobody knows when something breaks in prod until a user complains. Options:

  • Wire Supabase log drain (if Dokploy supports it).
  • Self-host a lightweight OpenTelemetry collector on mlake.
  • At minimum: GET /metrics (Prometheus text) with basic counters, and a cron that posts to Gotify when error-rate crosses a threshold.

Pick the cheapest now — we can evolve later.

T-8. No structured logging

Mixed log.Printf + slog.Info across the codebase. Pick one, default to slog with a JSON handler in prod. Pre-populate user-id and request-id in the context for correlation.

T-9. No backup verification for paliad schema

Supabase's automatic backups cover the whole instance, but paliad.paliad_schema_migrations collision history (memory episode "paliad migration bootstrap collision") suggests the shared-Postgres posture is fragile. Add a weekly pg_dump --schema=paliad cron on mlake that writes to an offsite bucket and a monthly restore-smoke-test (into an ephemeral Postgres container).

T-10. Test coverage gaps

Only akte_service_test.go, deadline_calculator_test.go, holidays_test.go, fees_test.go exist. Missing: FristService, TerminService, NotizService, CalDAVService (unit tests for iCal encode / decode, encrypt / decrypt, visibility edge-cases). The CalDAV crypto in particular should have a table-driven test with known vectors.

T-11. internal/models/models.go is one big file

Not inherently bad, but it's the dumping ground for 14 types. Split by domain (user.go, akte.go, frist.go, termin.go, …) next time it's touched.

T-12. Dockerfile build cache hygiene

Every COPY . . invalidates on any source change, forcing a full go mod download. Move go.mod/go.sum copy + go mod download before COPY . . — already done. But also consider FROM golang:1.24-alpine AS backendgolang:1.24 with CGO_ENABLED=0 so libc-less alpine runtime doesn't fight any future pgx upgrade. Minor; today it's fine.

T-13. frontend/build.ts has 24 hand-maintained render-and-write pairs

Any new page requires edits in 4 places (build.ts, handlers.go registration, i18n keys, CSS). Consider a page-manifest — an array [{slug, render, clientEntry}] — that the build script iterates over. Less ceremony; harder to forget one of the four places.

T-14. Gitea-backed file proxy has no ETag / If-Modified-Since

internal/handlers/files.go caches bytes and the commit SHA in memory but never sets an ETag or Last-Modified response header, so every browser re-downloads a 500 KB .dotm on each click. Add: w.Header().Set("ETag", entry.sha) + handle If-None-Match with a 304. Cheap win.


Delivery suggestion

  • Week 1 (critical): C-1, C-2, C-3, C-4, C-5. All small- to medium-effort and gates production readiness.
  • Week 2 (important): I-1, I-2, I-3, I-7, I-8. Polish the post-merger brand story.
  • Week 3 (features, pick 2): F-1 (global search) gives the biggest daily-use win; F-2 (Verfahrensleitfäden) is the biggest content win; F-3 (Downloads) closes a roadmap item cheaply.
  • Always-on (tech debt): pick one T-item per sprint until they're gone.

None of the critical items are theoretical — the JWT signature bypass and the dashboard Termine leak are real exploit-paths I could walk through today with a curl command and a Paliad cookie. Fix those two first.