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
This commit is contained in:
55
AUDIT.md
55
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.
|
**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)
|
## 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 ./`.
|
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
|
## 3. Architecture Assessment
|
||||||
@@ -393,6 +442,8 @@ Despite the feature gap, KanzlAI has some advantages:
|
|||||||
- [ ] Encrypt CalDAV credentials at rest (1.3)
|
- [ ] Encrypt CalDAV credentials at rest (1.3)
|
||||||
- [ ] Add CORS middleware + security headers (1.4)
|
- [ ] Add CORS middleware + security headers (1.4)
|
||||||
- [ ] Stop leaking internal errors to clients (1.5)
|
- [ ] 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)
|
- [ ] Fix Dockerfile go.sum copy (2.9)
|
||||||
|
|
||||||
### P1 — Before Demo/Beta
|
### P1 — Before Demo/Beta
|
||||||
@@ -401,6 +452,10 @@ Despite the feature gap, KanzlAI has some advantages:
|
|||||||
- [ ] Fix `search_path` connection pool issue (2.8)
|
- [ ] Fix `search_path` connection pool issue (2.8)
|
||||||
- [ ] Add graceful shutdown with signal handling (2.7)
|
- [ ] Add graceful shutdown with signal handling (2.7)
|
||||||
- [ ] Sanitize Content-Disposition filename (2.6)
|
- [ ] 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 multi-tenant security tests
|
||||||
- [ ] Add database migrations system
|
- [ ] Add database migrations system
|
||||||
- [ ] Add `.env.example` file
|
- [ ] Add `.env.example` file
|
||||||
|
|||||||
Reference in New Issue
Block a user