From 4b86dfa4ad54d10ff7486573a4b1bb0d4a609034 Mon Sep 17 00:00:00 2001 From: m Date: Sat, 28 Mar 2026 02:23:50 +0100 Subject: [PATCH] feat: update AUDIT.md with sub-agent findings Added 7 additional issues from deep-dive agents: - Race condition in HolidayService cache (critical) - Rate limiter X-Forwarded-For bypass (critical) - German umlaut typos throughout frontend - Silent error swallowing in createEvent - Missing React error boundaries - No RLS policies at database level - Updated priority roadmap with new items --- AUDIT.md | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/AUDIT.md b/AUDIT.md index efd3d8e..487b7aa 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -81,6 +81,25 @@ Internal error messages (including SQL errors, connection errors, etc.) are sent **Fix:** Log the full error server-side, return a generic message to the client. +### 1.6 Race Condition in HolidayService Cache +**File:** `backend/internal/services/holidays.go` + +The `HolidayService` uses a `map[int][]Holiday` cache without any mutex protection. Concurrent requests (e.g., multiple deadline calculations) will cause a data race. The Go race detector would flag this. + +**Fix:** Add `sync.RWMutex` to HolidayService. + +### 1.7 Rate Limiter Trivially Bypassable +**File:** `backend/internal/middleware/ratelimit.go:78-79` + +```go +ip := r.Header.Get("X-Forwarded-For") +if ip == "" { ip = r.RemoteAddr } +``` + +Rate limiting keys off `X-Forwarded-For`, which any client can spoof. An attacker can bypass AI endpoint rate limits by rotating this header. + +**Fix:** Only trust `X-Forwarded-For` from configured reverse proxy IPs, or use `r.RemoteAddr` exclusively behind a trusted proxy. + --- ## 2. Important Gaps (Fix Before Showing to Anyone) @@ -174,6 +193,36 @@ RUN go mod download Only `go.mod` is copied, not `go.sum`. This means the build isn't reproducible and doesn't verify checksums. Should be `COPY go.mod go.sum ./`. +### 2.10 German Umlaut Typos Throughout Frontend +**Files:** Multiple frontend components + +German strings use ASCII approximations instead of proper characters: +- `login/page.tsx`: "Zurueck" instead of "Zurück" +- `cases/[id]/layout.tsx`: "Anhaengig" instead of "Anhängig" +- `cases/[id]/fristen/page.tsx`: "Ueberfaellig" instead of "Überfällig" +- `termine/page.tsx`: "Uberblick" instead of "Überblick" + +A German lawyer would notice this immediately. It signals "this was built by a machine, not tested by a human." + +### 2.11 Silent Error Swallowing in Event Creation +**File:** `backend/internal/services/case_service.go:260-266` + +```go +func createEvent(ctx context.Context, db *sqlx.DB, ...) { + db.ExecContext(ctx, /* ... */) // Error completely ignored +} +``` + +Case events (audit trail) silently fail to create. The calling functions don't check the return. This means you could have cases with no events and no way to know why. + +### 2.12 Missing Error Boundaries in Frontend +No React error boundaries are implemented. If any component throws, the entire page crashes with a white screen. For a law firm tool where data integrity matters, this is unacceptable. + +### 2.13 No RLS Policies Defined at Database Level +Multi-tenant isolation relies entirely on `WHERE tenant_id = $X` clauses in Go code. If any query forgets this clause, data leaks across tenants. There are no PostgreSQL RLS policies as a safety net. + +**Fix:** Enable RLS on all tenant-scoped tables and create policies tied to `auth.uid()` via `user_tenants`. + --- ## 3. Architecture Assessment @@ -393,6 +442,8 @@ Despite the feature gap, KanzlAI has some advantages: - [ ] Encrypt CalDAV credentials at rest (1.3) - [ ] Add CORS middleware + security headers (1.4) - [ ] Stop leaking internal errors to clients (1.5) +- [ ] Add mutex to HolidayService cache (1.6) +- [ ] Fix rate limiter X-Forwarded-For bypass (1.7) - [ ] Fix Dockerfile go.sum copy (2.9) ### P1 — Before Demo/Beta @@ -401,6 +452,10 @@ Despite the feature gap, KanzlAI has some advantages: - [ ] Fix `search_path` connection pool issue (2.8) - [ ] Add graceful shutdown with signal handling (2.7) - [ ] Sanitize Content-Disposition filename (2.6) +- [ ] Fix German umlaut typos throughout frontend (2.10) +- [ ] Handle createEvent errors instead of swallowing (2.11) +- [ ] Add React error boundaries (2.12) +- [ ] Implement RLS policies on all tenant-scoped tables (2.13) - [ ] Add multi-tenant security tests - [ ] Add database migrations system - [ ] Add `.env.example` file