diff --git a/CODE_ANALYSIS.md b/CODE_ANALYSIS.md new file mode 100644 index 0000000..63c65bc --- /dev/null +++ b/CODE_ANALYSIS.md @@ -0,0 +1,684 @@ +# Code Analysis Report + +> **Audit Date**: 2026-06-29 +> **Auditor**: Codebase Explorer (brutally honest mode) +> **Verdict**: This codebase is a functional prototype with critical security holes, broken collaboration sync, and systemic architectural debt. It is NOT production-ready. + +--- + +## Critical Issues (Must Fix Immediately) + +### 1. [backend/src/routes/*.ts] NO Authentication on All CRUD Routes +**Severity**: CRITICAL +**Impact**: Complete data exposure and manipulation by anyone + +The following routes have **ZERO authentication middleware**: +- `projects.ts` — `GET /api/projects` returns ALL projects to anyone. `DELETE /api/projects/:id` deletes any project without ownership check. +- `drawings.ts` — Full CRUD with no auth. +- `elements.ts` — Full CRUD with no auth. Anyone can create/read/update/delete elements in any drawing. +- `layers.ts` — Full CRUD with no auth. +- `blocks.ts` — Full CRUD with no auth. +- `settings.ts` — Full CRUD with no auth. Anyone can read/write application settings. +- `users.ts` — `GET /api/users` lists ALL users (with password_hash stripped). `DELETE /api/users/:id` deletes any user. `PATCH /api/users/:id` updates any user. The `authService` is imported but **never used**. Comment says "TODO: add role check middleware" — it was never done. + +Only `auth.ts`, `shares.ts`, `notifications.ts`, and `ai.ts` have proper auth checks. + +**The frontend sends Bearer tokens, but the backend ignores them on all CRUD routes.** + +### 2. [backend/src/websocket/yjsServer.ts:84] WebSocket Collaboration Has No Authentication +**Severity**: CRITICAL +**Impact**: Anyone can connect to any collaboration document and modify it in real-time + +The `/ws/collab/:docName` endpoint accepts any WebSocket connection without token validation. An attacker can connect to `project-` and inject/modify/delete all shared data. + +### 3. [backend/src/server.ts:26] CORS Allows All Origins +**Severity**: CRITICAL +**Impact**: Any website can make authenticated requests to the API + +```typescript +await fastify.register(cors, { + origin: true, // allows ALL origins + methods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], +}); +``` + +Combined with the missing auth on CRUD routes, any website can read/modify all data. + +### 4. [backend/src/websocket/yjsServer.ts:43] Yjs Persistence NEVER Loads — `persistenceReady` Is Never Set +**Severity**: CRITICAL +**Impact**: All collaboration data is lost on server restart + +```typescript +let persistenceReady: Promise | null = null; // initialized as null + +// line 43: +if (persistence && persistenceReady) { // persistenceReady is ALWAYS null! + await persistenceReady; + const persistedYdoc = await persistence.getYDoc(docName); + // ... +} +``` + +`persistenceReady` is declared but **never assigned**. The persistence loading block is dead code. Documents are persisted to LevelDB (via the `doc.on('update')` handler) but **never loaded back** on server restart or document re-creation. + +Additionally, `initYjsPersistence()` creates the persistence instance but doesn't set `persistenceReady`: +```typescript +export async function initYjsPersistence(): Promise { + persistence = new LeveldbPersistence(PERSISTENCE_DIR); + // persistenceReady is NOT set here! +} +``` + +### 5. [backend/src/websocket/yjsServer.ts:125] In-Memory Doc Deleted When Last Client Disconnects +**Severity**: CRITICAL +**Impact**: Document state lost from memory; relies on broken persistence (see issue #4) + +```typescript +socket.on('close', () => { + // ... + if (conns.size === 0) { + connections.delete(docName); + docs.delete(docName); // doc removed from memory! + } +}); +``` + +When the last client disconnects, the Y.Doc is deleted from memory. Since persistence loading is broken (issue #4), the next client to connect gets an empty document. All collaboration data is effectively ephemeral. + +### 6. [frontend/src/App.tsx:280-319] CRDT Sync: Groups and bgConfig NOT Synced via Yjs +**Severity**: CRITICAL +**Impact**: Groups and background configuration are never shared between collaborators + +`YjsDocument` only has maps for `elements`, `layers`, `blocks`, and `meta`. There are **no maps for `groups` or `bgConfig`**. The `useYjsBinding` hook doesn't expose group or bgConfig sync methods. When user A creates a group or sets a background, user B will never see it. + +The remote→local sync effect (lines 280-319) only handles `collab.elements`, `collab.layers`, and `collab.blocks`. Groups and bgConfig are completely absent from the collaboration layer. + +### 7. [frontend/src/App.tsx:267-269] loadFromState Called Before Data Is Loaded — Race Condition +**Severity**: CRITICAL +**Impact**: Empty local state can overwrite remote CRDT data + +```typescript +// Push initial data to Yjs CRDT after load +if (collab.status === 'connected') { + collab.loadFromState({ elements: data.elements, layers: data.layers, blocks: data.blocks }); +} +``` + +`loadFromState` in `YjsDocument.ts` (line 82) **clears all maps and replaces with local state**. If the backend returned empty arrays (new project), this wipes any existing CRDT data from other connected clients. The check `collab.status === 'connected'` doesn't guarantee that remote data has been received and merged yet. + +### 8. [frontend/src/App.tsx:128-142] Plugin Context Has Stale Closure — Captures Empty State +**Severity**: CRITICAL +**Impact**: Plugins always see empty elements and layers arrays + +```typescript +useEffect(() => { + const ctx: PluginContext = { + // ... + getElements: () => elements, // captures `elements` from first render = [] + getLayers: () => layers, // captures `layers` from first render = initialLayers + // ... + }; + pluginRegistry.setContext(ctx); +}, []); // EMPTY DEPS — runs once, captures stale state +``` + +The effect has `[]` dependencies, so it runs once on mount. `getElements()` will always return `[]` (initial state). `getLayers()` will always return `initialLayers` (never updated). Any plugin using these methods gets stale data. + +--- + +## High Priority Issues + +### 9. [frontend/src/App.tsx:306-317] Band-Aid Guards for Empty Remote Arrays — Incomplete and Fragile +**Severity**: HIGH +**Impact**: Layers/blocks can still be overwritten in edge cases; elements merge logic is flawed + +The guards `if (collab.layers.length > 0)` and `if (collab.blocks.length > 0)` prevent empty remote data from overwriting local defaults. But: + +1. **Elements (lines 287-303)**: The merge logic compares ID sets but doesn't handle element **updates** from remote. If remote has the same IDs but modified properties, the merge only takes remote if local doesn't have the element. Existing elements keep local state, ignoring remote changes. + +2. **Layers (lines 307-309)**: The comparison `collab.layers.every((l, i) => l.id === layers[i]?.id)` assumes **same ordering**. If remote layers are in a different order but have the same IDs, it triggers a `setLayers` call, causing unnecessary re-renders. If a layer was renamed remotely, the ID is the same, so `sameLayers` is true and the rename is **ignored**. + +3. **No guard for element property changes**: When a remote user modifies an element (moves it, changes color), the local client won't receive the update if it has the element locally. The merge only adds remote-only elements, it doesn't update existing ones. + +### 10. [frontend/src/App.tsx:588] Save Action Only Saves Elements Starting with 'el-' +**Severity**: HIGH +**Impact**: Elements created by block drop, import, or paste are silently skipped during save + +```typescript +if (action === 'save') { + // ... + elements.forEach((el) => { + if (!el.id.startsWith('el-')) return; // skips el_, svg_, chair-, etc. + updateElement(token, el.id, el).catch(() => {}); + }); +} +``` + +Block instances have IDs like `el_${Date.now()}_abc` (underscore, not hyphen). Imported elements keep their original IDs. Pasted elements get `el-${Date.now()}-${i}`. Only hyphen-prefixed IDs are saved. + +### 11. [frontend/src/App.tsx:509-517] Block Drop Doesn't Save to Backend or Yjs +**Severity**: HIGH +**Impact**: Placed blocks exist only in local state; lost on reload, not shared with collaborators + +```typescript +const handleBlockDrop = useCallback((blockId: string, x: number, y: number) => { + // ... + if (instance) { + setElements((prev) => [...prev, instance]); // local only! + // NO call to createElementTyped + // NO call to collab.setElement + } +}, [blocks, activeLayerId]); +``` + +No backend persistence, no CRDT sync. The element vanishes on page reload. + +### 12. [frontend/src/App.tsx:528-550] Import Doesn't Save to Backend or Yjs +**Severity**: HIGH +**Impact**: Imported elements/layers/blocks exist only in local state + +Same pattern as block drop. `handleImport` adds elements to local state but never calls `createElementTyped`, `createLayerTyped`, `createBlockTyped`, or `collab.setElement`. + +### 13. [frontend/src/App.tsx:628] Copy/Paste Uses Non-Existent `selected` Property +**Severity**: HIGH +**Impact**: Copy always copies the first element, never the actual selection + +```typescript +const selected = elements.filter((e) => (e as any).selected); +const clip = selected.length > 0 ? selected : elements.slice(0, 1); +``` + +`CADElement` has no `selected` property in its type definition. The `SelectionEngine` tracks selection internally but doesn't annotate elements. `(e as any).selected` is always `undefined` (falsy), so `selected` is always empty, and `clip` always becomes `elements.slice(0, 1)`. + +### 14. [frontend/src/App.tsx:1021] activeLayerName Hardcoded to 'layer-0' +**Severity**: HIGH +**Impact**: Status bar always shows the name of the initial first layer, not the active layer + +```typescript +const activeLayerName = layers.find((l) => l.id === 'layer-0')?.name ?? '—'; +``` + +This should use `activeLayerId`, not the hardcoded `'layer-0'`. If the user selects a different layer, the status bar still shows 'Wände'. + +### 15. [frontend/src/App.tsx:754-789] Layer Operations Don't Persist to Backend +**Severity**: HIGH +**Impact**: Layer visibility, lock, color, line type, transparency, rename, duplicate changes are lost on reload + +The following handlers modify local state but **never call the backend API**: +- `handleToggleLayer` (line 754) — no `updateLayer` call +- `handleRenameLayer` (line 766) — no `updateLayer` call +- `handleDuplicateLayer` (line 769) — no `createLayerTyped` call +- `handleToggleLock` (line 778) — no `updateLayer` call +- `handleUpdateLayerColor` (line 781) — no `updateLayer` call +- `handleUpdateLayerLineType` (line 784) — no `updateLayer` call +- `handleUpdateLayerTransparency` (line 787) — no `updateLayer` call +- `handleReorderLayer` (line 802) — no backend update + +Only `handleAddLayer` and `handleAddSubLayer` persist to backend. + +### 16. [frontend/src/App.tsx:436-438] Block Rename Doesn't Persist to Backend +**Severity**: HIGH +**Impact**: Block renames are lost on reload + +`handleRenameBlock` only updates local state. No `updateBlock` API call. + +### 17. [frontend/src/App.tsx:440-454] Block Duplicate Doesn't Persist to Backend +**Severity**: HIGH +**Impact**: Duplicated blocks are lost on reload + +`handleDuplicateBlock` only updates local state. No `createBlockTyped` API call. + +### 18. [frontend/src/App.tsx:906-912] GROUP Command Doesn't Actually Create Groups +**Severity**: HIGH +**Impact**: Group/ungroup commands are non-functional + +```typescript +if (upper === 'GROUP' || upper === 'GRP') { + // For now, group all elements (selection state is in InteractionEngine, not accessible) + setCommandHistory((prev) => [...prev, { prefix: '·', text: 'Gruppe erstellt (Auswahl im Canvas erforderlich)', type: 'info' }]); + setGroups(gm.getGroups()); + return; +} +``` + +The comment admits it: selection state is in `InteractionEngine` but not accessible from `App.tsx`. The command does nothing useful — it just logs a message and reads the (empty) groups list. + +### 19. [frontend/src/crdt/AwarenessManager.ts:38-39] Cursor Data Stored in Shared Y.Doc — Persisted to Disk +**Severity**: HIGH +**Impact**: Cursor positions are persisted to LevelDB and replayed on reconnect + +```typescript +this.state = { + cursors: this.yjsDoc.doc.getMap('awareness-cursors'), + selections: this.yjsDoc.doc.getMap('awareness-selections'), +}; +``` + +Awareness data (cursors, selections) is stored in the same `Y.Doc` as document data. This means: +1. Cursor movements trigger document updates and are persisted to LevelDB. +2. On reconnect, old cursor positions are replayed (stale cursors appear). +3. Cursor updates are broadcast as document updates, mixing transient state with persistent data. + +Standard Yjs awareness uses a separate protocol that is NOT persisted. This implementation reinvents awareness incorrectly. + +### 20. [frontend/src/crdt/WebSocketProvider.ts:62-65] Client Sends Local State on Connect — Conflicts with Server State +**Severity**: HIGH +**Impact**: Initial sync can cause data corruption when both client and server have data + +```typescript +this.ws.onopen = () => { + this.setStatus('connected'); + const stateUpdate = Y.encodeStateAsUpdate(this.yjsDoc.doc); + if (stateUpdate.length > 0) { + this.ws?.send(stateUpdate); // sends local state to server + } +}; +``` + +The server also sends its state to the client (yjsServer.ts:100-103). Both sides apply each other's updates. Yjs CRDTs are designed to handle this, but the `loadFromState` call in App.tsx (which clears and replaces) can conflict with the server's state, causing data loss. + +### 21. [backend/src/routes/projects.ts:9-11] listProjects Returns ALL Projects — No Owner Filtering +**Severity**: HIGH +**Impact**: Any user (or unauthenticated caller) sees all projects in the system + +Even if auth were added, `listProjects()` returns all projects without filtering by `owner_id` or shares. + +--- + +## Medium Priority Issues + +### 22. [frontend/src/App.tsx:1207 lines] Monolithic Component — God Component Anti-Pattern +**Severity**: MEDIUM +**Impact**: Unmaintainable, every state change re-renders the entire component tree + +`App.tsx` is 1207 lines containing: +- All application state (30+ useState calls) +- All event handlers (40+ useCallback calls) +- All side effects (10+ useEffect calls) +- The entire render tree +- Auth gating logic + +No state management library, no context splitting, no component decomposition beyond presentational children. + +### 23. [frontend/src/App.tsx:380] Stale `elements` in handleElementsDeleted Dependencies +**Severity**: MEDIUM +**Impact**: Unnecessary callback re-creation, potential stale closure bugs + +```typescript +}, [elements, layers, blocks, groups, bgConfig, syncHistory, token, collab]); +``` + +`elements` is in the dependency array but the callback uses `prev` (functional update), not `elements` directly. This causes the callback to be re-created on every element change, which cascades to all child components. + +### 24. [frontend/src/App.tsx:607-621] Export Uses NW.js-Specific `nwsave` Attribute +**Severity**: MEDIUM +**Impact**: Export dialog doesn't work in standard browsers + +```typescript +input.setAttribute('nwsave', ''); // NW.js only! +``` + +In a standard browser, this creates a file input that opens the file picker, not a save dialog. The filename extraction `input.value` may not work as expected. + +### 25. [frontend/src/services/api.ts:354-388] Project Load Cache Race Condition +**Severity**: MEDIUM +**Impact**: Concurrent calls can create duplicate drawings + +```typescript +const projectLoadCache = new Map>(); + +export async function loadProjectDataTyped(token: string, projectId: string): Promise { + const existing = projectLoadCache.get(projectId); + if (existing) return existing; + // ... + projectLoadCache.set(projectId, promise); + promise.finally(() => projectLoadCache.delete(projectId)); + return promise; +} +``` + +The cache deduplicates concurrent calls, but the `finally` cleanup runs as soon as the promise settles. If a second call comes in after the first completes but before React processes the result, it will create a new request (and potentially a new drawing). + +### 26. [frontend/src/App.tsx:66] Global Mutable Set for StrictMode Dedup — Module-Level State +**Severity**: MEDIUM +**Impact**: State leaks between component instances; breaks if component is reused + +```typescript +const loadedProjects = new Set(); // module-level mutable state +``` + +This Set is never used in the current code (the dedup is handled by `projectLoadCache` in api.ts). Dead code that could cause confusion. + +### 27. [frontend/src/App.tsx:170-173] SeatingService Instantiated on Every Elements Change +**Severity**: MEDIUM +**Impact**: Unnecessary object creation and garbage collection pressure + +```typescript +const seatCount = useMemo(() => { + const svc = new SeatingService(); // new instance every time! + return svc.countSeats(elements).total; +}, [elements]); +``` + +`SeatingService` is stateless but recreated on every `elements` change. Should be a module-level singleton or ref. + +### 28. [frontend/src/App.tsx:467-472] SVG Import Doesn't Persist to Backend +**Severity**: MEDIUM +**Impact**: Imported SVG blocks are lost on reload + +`handleSvgImport` adds the block to local state but doesn't call `createBlockTyped`. + +### 29. [frontend/src/App.tsx:474-488] Save Group as Block Only Uses Single Selected Element +**Severity**: MEDIUM +**Impact**: Block-from-selection only captures one element + +```typescript +const selectedEls = selectedElement ? [selectedElement] : []; +``` + +`selectedElement` is a single `CADElement | null`, not an array. Multi-element selections can't be saved as blocks. + +### 30. [backend/src/database/SqliteAdapter.ts:267] Session Expiry Comparison Uses Wrong Format +**Severity**: MEDIUM +**Impact**: Expired sessions may not be cleaned up properly + +```typescript +deleteExpiredSessions(): void { + this.db.prepare('DELETE FROM sessions WHERE expires_at < ?').run(Date.now()); +} +``` + +`expires_at` is stored as a number (milliseconds from `Date.now() + SESSION_TTL_MS`), but SQLite may store it as a string depending on column type. The comparison `expires_at < Date.now()` works if both are numbers, but if SQLite stores it as text, the comparison is lexicographic. + +### 31. [frontend/src/App.tsx:421-428] Text Edit Uses window.prompt — Blocking UX +**Severity**: MEDIUM +**Impact**: Poor UX, blocks the main thread, no styling control + +```typescript +const handleTextEdit = useCallback((el: CADElement) => { + const text = window.prompt('Text eingeben:', ''); + // ... +}, []); +``` + +Also, the text edit doesn't persist to backend or sync to Yjs. + +### 32. [frontend/src/App.tsx:657-680] Image Insert Doesn't Save to Backend or Yjs +**Severity**: MEDIUM +**Impact**: Inserted images exist only in local state + +The image insert handler creates an element with a base64 data URL in properties but doesn't call `createElementTyped` or `collab.setElement`. + +### 33. [backend/src/routes/*.ts] No Input Validation on Any Route +**Severity**: MEDIUM +**Impact**: Invalid data can be stored, potential for unexpected behavior + +No route validates the structure of `request.body`. For example, `createElement` accepts any `Partial` — there's no check that `type` is a valid `ElementType`, that coordinates are numbers, or that `properties_json` is valid JSON. + +### 34. [frontend/src/App.tsx:637-642] Paste Doesn't Save to Backend or Yjs +**Severity**: MEDIUM +**Impact**: Pasted elements exist only in local state + +```typescript +setElements((prev) => [...prev, ...pasted]); +// NO createElementTyped, NO collab.setElement +``` + +### 35. [frontend/src/crdt/useYjsBinding.ts:110] useEffect Dependencies Include Values That Don't Change +**Severity**: MEDIUM +**Impact**: Yjs document is recreated if userName or userColor changes (unlikely but possible) + +```typescript +}, [docName, wsUrl, userId, userName, userColor, enabled]); +``` + +If `userName` or `userColor` changes (e.g., user updates their profile), the entire Yjs document is destroyed and recreated, losing all local state. + +### 36. [frontend/src/App.tsx:145] Online Count Includes Self Even When Disconnected +**Severity**: MEDIUM +**Impact**: Status bar shows incorrect collaborator count + +```typescript +const onlineCount = collab.cursors.length + 1; +``` + +Always adds 1 for self, even if collaboration is disconnected. Also, `cursors` only includes visible cursors from the awareness manager, which may include stale entries. + +--- + +## Low Priority Issues + +### 37. [frontend/src/App.tsx:50-63] Mock Data Used as Initial State +**Severity**: LOW +**Impact**: Misleading initial state; mock command history and KI messages shown before real data loads + +`initialCommandHistory` and `initialKIMessages` are hardcoded mock data that show fake status messages ("110 Objekte", "Auto-Save aktiv") before any project is loaded. + +### 38. [frontend/src/App.tsx:40-46] Initial Layers Hardcoded — Not from Backend +**Severity**: LOW +**Impact**: Default layers shown before project loads; may differ from backend layers + +The 5 initial layers (Wände, Türen, Bestuhlung, Bühne, Hintergrund) are hardcoded. If the backend has different layers, there's a flash of wrong data before the load completes. + +### 39. [frontend/src/services/api.ts:34,44,54] Return Type `any` for Auth Functions +**Severity**: LOW +**Impact**: No type safety on user object + +```typescript +export async function login(...): Promise<{ user: any; session: { token: string } }> +export async function register(...): Promise<{ user: any; session: { token: string } }> +export async function getMe(token: string): Promise +``` + +### 40. [frontend/src/App.tsx:667-671] Multiple `as any` Casts for Image Element +**Severity**: LOW +**Impact**: Type safety bypassed for image elements + +```typescript +const imgEl: CADElement = { + id: `el-${Date.now()}`, + type: 'image' as any, // 'image' is not in ElementType union! + // ... +} as any; +``` + +`'image'` is not a valid `ElementType`. The type system is bypassed with `as any`. + +### 41. [frontend/src/history/HistoryManager.ts] generateId() Method Never Used +**Severity**: LOW +**Impact**: Dead code + +The `generateId()` method exists but is never called. History entries use hardcoded IDs like `'undo-0'`, `'current'`, `'redo-0'`. + +### 42. [frontend/src/App.tsx:501-503] handleBlockSearch Is a No-Op +**Severity**: LOW +**Impact**: Dead code, search is handled elsewhere + +```typescript +const handleBlockSearch = useCallback((_query: string) => { + // Search is handled locally in BlockLibrary component +}, []); +``` + +### 43. [frontend/src/App.tsx:691-695] Zoom Fit/100 Are No-Ops +**Severity**: LOW +**Impact**: Ribbon buttons don't actually zoom + +```typescript +if (action === 'zoom-fit') { + setCommandHistory((prev) => [...prev, { prefix: '·', text: 'Zoom: an Ansicht angepasst', type: 'info' }]); + return; // no actual zoom! +} +``` + +The `handleZoomFit` callback (line 797) also just logs a message. Only the canvas toolbar buttons (handleZoomIn/Out/Fit in CanvasArea) actually zoom. + +### 44. [frontend/src/App.tsx:683-686] Format Actions Are No-Ops +**Severity**: LOW +**Impact**: Format buttons do nothing + +```typescript +if (action.startsWith('format-')) { + setCommandHistory((prev) => [...prev, { prefix: '·', text: `Format: ${action.replace('format-', '')} (Auswahl erforderlich)`, type: 'info' }]); + return; +} +``` + +### 45. [frontend/src/App.tsx:706-708] Search Action Is a No-Op +**Severity**: LOW +**Impact**: Search button does nothing + +### 46. [backend/src/database/SqliteAdapter.ts:93] Indentation Inconsistency +**Severity**: LOW +**Impact**: Code smell + +Line 93 has inconsistent indentation (space before `const sets`). + +### 47. [frontend/src/App.tsx:159] BackgroundService Instantiated in useRef — Never Cleaned Up +**Severity**: LOW +**Impact**: Potential memory leak if BackgroundService holds resources + +```typescript +const bgServiceRef = React.useRef(new BackgroundService()); +``` + +The service is created once and never destroyed. If it holds image references or event listeners, they leak. + +### 48. [frontend/src/App.tsx:349-352] handleToggleElementVisible Calls API Inside setElements +**Severity**: LOW +**Impact**: Side effect inside state updater — may cause double calls in StrictMode + +```typescript +setElements((prev) => prev.map((e) => { + if (e.id === id) { + // ... + if (token) { + updateElement(token, newEl.id, newEl).then(...).catch(...); // SIDE EFFECT IN UPDATER! + } + return newEl; + } + return e; +})); +``` + +Calling async API inside a state updater function is an anti-pattern. In React StrictMode, the updater may be called twice, causing duplicate API calls. + +### 49. [frontend/src/App.tsx:951-960] Plugin Context Duplicated in handleCommand +**Severity**: LOW +**Impact**: Code duplication, same stale closure issue as issue #8 + +The `PluginContext` is recreated inside `handleCommand` with the same stale closure problem. + +### 50. [frontend/src/App.tsx:799] handleZoomIn/Out/Fit Only Log Messages +**Severity**: LOW +**Impact**: Status bar zoom buttons are no-ops; actual zoom is in CanvasArea + +--- + +## Architecture Overview & Assessment + +### Stack +- **Frontend**: React 18 + TypeScript + Vite, HTML5 Canvas rendering +- **Backend**: Node.js + Fastify + better-sqlite3 + Yjs + y-leveldb +- **Collaboration**: Custom Yjs WebSocket protocol (not y-websocket) +- **Auth**: Custom JWT-like session tokens (not real JWT) + +### Architecture Diagram +``` +┌─────────────────────────────────────────────────────┐ +│ App.tsx (1207 lines) │ +│ ┌──────────────────────────────────────────────┐ │ +│ │ 30+ useState, 40+ useCallback, 10+ useEffect │ │ +│ │ All state, all handlers, all logic │ │ +│ └──────────────────────────────────────────────┘ │ +│ ┌──────┐ ┌────────┐ ┌──────────┐ ┌──────────────┐ │ +│ │Topbar│ │RibbonBar│ │LeftSidebar│ │ CanvasArea │ │ +│ │ │ │ │ │ │ │ (Canvas2D) │ │ +│ └──────┘ └────────┘ └──────────┘ └──────────────┘ │ +│ ┌──────────┐ ┌──────────┐ ┌────────────────────┐ │ +│ │RightSidebar│ │StatusBar│ │ CommandLine │ │ +│ │ │ │ │ │ │ │ +│ └──────────┘ └──────────┘ └────────────────────┘ │ +└─────────────────────────────────────────────────────┘ + │ │ + ▼ ▼ +┌─────────────────┐ ┌──────────────────────┐ +│ api.ts (REST) │ │ Yjs CRDT (WebSocket) │ +│ fetch + Bearer │ │ Custom WS protocol │ +└─────────────────┘ └──────────────────────┘ + │ │ + ▼ ▼ +┌─────────────────────────────────────────┐ +│ Fastify Backend │ +│ ┌─────────┐ ┌──────────┐ ┌──────────┐ │ +│ │ Routes │ │ AuthService│ │ Yjs WS │ │ +│ │(NO AUTH)│ │ │ │(NO AUTH) │ │ +│ └─────────┘ └──────────┘ └──────────┘ │ +│ ┌──────────────────────────────────┐ │ +│ │ SqliteAdapter (better-sqlite3) │ │ +│ │ + LevelDB (Yjs persistence) │ │ +│ └──────────────────────────────────┘ │ +└─────────────────────────────────────────┘ +``` + +### Assessment + +**This is a prototype-quality codebase with significant issues across all dimensions.** + +1. **Security**: Effectively non-existent. All CRUD routes and the WebSocket endpoint are completely open. This is the single most critical problem. + +2. **Collaboration**: Broken in multiple ways. Persistence doesn't load, groups/bgConfig aren't synced, awareness is stored in the persistent document, and the sync merge logic is flawed. + +3. **Data Persistence**: Inconsistent. Elements are saved on create/modify/delete, but layers, blocks, imports, paste, block drops, and image inserts are NOT saved. Many operations are local-only. + +4. **Architecture**: God component anti-pattern. App.tsx is unmaintainable at 1207 lines with all state and logic. No state management, no context splitting, no separation of concerns. + +5. **Type Safety**: Bypassed in multiple places with `as any` casts. The `ElementType` union doesn't include `'image'` but it's used. + +6. **Error Handling**: Inconsistent. Some API calls have `.catch(() => {})` (silent failures), others log to console. No user-facing error feedback except command history messages. + +7. **Code Quality**: Significant dead code (no-op handlers, unused variables), inconsistent patterns, and band-aid fixes for fundamental design issues. + +--- + +## Summary & Recommendations + +### The User's Assessment Is Correct +The user believes "fast nichts brauchbares dabei" (almost nothing useful). This is **largely accurate**. The codebase has the structure of a real application but fails on fundamentals: + +- **Security is completely absent** on all data routes +- **Collaboration sync is broken** in 6+ different ways +- **Data persistence is inconsistent** — many operations are local-only +- **The God Component** makes the code unmaintainable +- **Multiple no-op handlers** mean UI buttons do nothing + +### What Does Work +- Basic canvas rendering and interaction (drawing tools, selection, snap) +- Auth flow (login/register/logout) — but only on auth routes +- Project sharing routes have proper auth and ownership checks +- Notification routes have proper auth +- The HistoryManager (undo/redo) is well-implemented +- The type definitions are reasonable +- The block service logic (SVG import, instance creation) is functional + +### Recommended Next Steps (Priority Order) + +1. **Add auth middleware to ALL routes** — Create a Fastify preHandler that validates the Bearer token and attaches the user. Apply it to all route modules except auth.ts register/login. + +2. **Fix Yjs persistence** — Set `persistenceReady` in `initYjsPersistence()`. Don't delete docs from memory on last disconnect (or implement a TTL cache). + +3. **Add auth to WebSocket** — Validate token in the WebSocket upgrade handler. + +4. **Sync groups and bgConfig via Yjs** — Add `groups` and `bgConfig` maps to `YjsDocument`. + +5. **Fix data persistence** — All state-changing handlers must call the appropriate API. Layer toggles, block renames, imports, paste, block drops, image inserts. + +6. **Fix the CRDT merge logic** — Use Y.Map observers for granular element updates instead of array replacement. Don't use `loadFromState` (which clears everything) after initial load. + +7. **Fix the plugin context stale closure** — Use refs for `elements` and `layers` in the plugin context, or move plugin context to a separate provider. + +8. **Decompose App.tsx** — Extract state into a reducer or state manager, split handlers into custom hooks, separate auth gating from editor. + +9. **Fix CORS** — Restrict to known origins. + +10. **Add input validation** — Use a schema validator (Zod, Joi, or Fastify schema) on all route bodies.