Files

AlacrisDevs 75a2aefadb Resruiin

2026-02-07 23:10:10 +02:00

18 KiB

Raw Blame History

Comprehensive Codebase Audit Report (v5)

Project: root-org — Event Organizing Platform (SvelteKit 2 + Svelte 5 + Supabase + Tailwind v4)
Date: 2026-02-07
Auditor: Cascade

Baseline Status

Metric	Result
`svelte-check`	0 errors, 9 warnings (a11y)
`vite build`	Success
`vitest`	112 tests passed across 9 files
TypeScript	`strict: true` enabled
Migrations	31 SQL files (001–031)
Components	44 UI + 8 module widgets + 15 matrix + 11 message
API modules	13 files in `$lib/api/`
Routes	7 route groups + 5 API endpoints

1. Security

S-1 · 🔴 CRITICAL — Real credentials committed to `.env` in git history

File: .env

The .env file contains real Supabase keys, Google API key, Google service account private key, and Matrix admin token. While .env is in .gitignore, it was committed before the rule was added and remains in git history.

Exposed secrets:

Supabase URL + anon key
Google API key
Google service account full JSON (including private key)
Matrix admin token

Fix (manual — cannot be automated):

git rm --cached .env to untrack
Use git filter-repo to purge .env from all history
Rotate ALL keys immediately — Supabase anon key, Google API key, Google service account, Matrix admin token
Force-push the cleaned history

Severity: If repo is ever made public or shared, all backend services are fully compromised.

S-2 · 🔴 CRITICAL — Google Calendar events endpoint lacks auth

File: src/routes/api/google-calendar/events/+server.ts

The GET handler reads org_id from query params and fetches calendar events. It does check auth (safeGetSession) and membership — this was fixed in a previous audit round. However, verify the current state matches the fix.

Status: Verify — was marked fixed in v4 audit.

S-3 · 🟡 HIGH — Auth callback open redirect

File: src/routes/auth/callback/+server.ts

The next/redirect query parameter is used in redirect(303, next) without validating it's a relative URL. Attacker can craft ?next=https://evil.com.

Fix:

function safeRedirect(target: string): string {
    if (target.startsWith('/') && !target.startsWith('//')) return target;
    return '/';
}

S-4 · 🟡 HIGH — Client-side mutations bypass server authorization

Problem: Settings page, event pages, and team management perform Supabase mutations directly from the browser. Security relies entirely on RLS policies being correct. The admin dashboard (newly added) correctly uses SvelteKit form actions — this pattern should be adopted everywhere.

Affected areas:

[orgSlug]/settings/ — org update, delete, member management
[orgSlug]/events/[eventSlug]/ — event update, delete
[orgSlug]/events/[eventSlug]/team/ — member add/remove, department CRUD

Fix: Migrate destructive operations to SvelteKit form actions with explicit server-side auth checks (like the admin dashboard pattern).

S-6 · 🟡 MEDIUM — Document lock RLS race condition

File: supabase/migrations/016_document_locks.sql

Two competing cleanup paths for expired locks: RLS policy allows any user to delete expired locks, and acquireLock() in document-locks.ts also deletes them client-side.

Fix: Consolidate to one path — either RLS-only or a server-side cron.

S-7 · 🟡 MEDIUM — `@inlang/paraglide-js` in both deps and devDeps

File: package.json

@inlang/paraglide-js appears in both dependencies and devDependencies. Should only be in devDependencies (it's a build-time tool).

2. Type Safety

T-1 · 🟡 HIGH — Supabase types are stale — 21 `as any` casts remain

Files: 12 files still use as any, concentrated in:

src/lib/matrix/sdk-types.ts (6) — Matrix SDK interop, acceptable
src/lib/api/budget.ts, contacts.ts, schedule.ts, sponsors.ts, department-dashboard.ts (5 total) — all use db() cast for tables added in migrations 026-031
src/routes/admin/+page.server.ts (1) — admin uses db() for is_platform_admin
src/routes/[orgSlug]/events/[eventSlug]/dept/[deptId]/+page.server.ts (3)
src/routes/[orgSlug]/events/[eventSlug]/team/+page.svelte (1)
src/routes/[orgSlug]/events/[eventSlug]/dept/[deptId]/+page.svelte (1)
src/lib/utils/permissions.ts (1)
Test files (3) — acceptable in tests

Root cause: Types haven't been regenerated since migrations 022-031 were added. Tables like events, event_members, event_departments, department_dashboards, budget_categories, sponsors, schedule_stages, department_contacts, and column is_platform_admin on profiles are all missing from the generated types.

Fix: Run npm run db:types to regenerate. This single command will eliminate ~15 of the 21 as any casts. The remaining 6 (Matrix SDK) are acceptable interop casts.

T-2 · 🟡 MEDIUM — Manual type aliases drift from generated types

File: src/lib/supabase/types.ts:1700-1877

15 manually-defined interfaces at the bottom of the generated types file (DepartmentDashboard, BudgetCategory, Sponsor, etc.). These will become redundant after type regeneration and could drift from the actual DB schema.

Fix: After regenerating types, derive these from the generated Database type:

export type BudgetCategory = Database['public']['Tables']['budget_categories']['Row'];

T-3 · 🟢 GOOD — TypeScript strict mode enabled

tsconfig.json has "strict": true, "checkJs": true, "forceConsistentCasingInFileNames": true. This is best-practice configuration.

3. Code Quality & Consistency

C-1 · 🟡 HIGH — 41 raw `console.*` calls bypass structured logger

13 files use console.log/warn/error directly instead of the project's createLogger() system. Concentrated in:

src/lib/matrix/client.ts (10)
src/lib/components/matrix/MessageInput.svelte (5)
src/routes/[orgSlug]/chat/+page.svelte (5)
src/routes/api/matrix-provision/+server.ts (5)
Various matrix components (6)
src/lib/stores/theme.ts (1)

Impact: Loses structured context, timestamps, and the ring buffer for error reports. The logger is already imported in most non-matrix files.

Fix: Replace all console.* with createLogger() calls. The Matrix integration was ported from another project and didn't adopt the logger.

C-2 · 🟡 MEDIUM — Inconsistent error catching: `e: any` vs `e` vs `e: unknown`

The codebase uses three different patterns:

catch (e: any) — 12 occurrences (team page, events pages, dept page)
catch (e) — 8 occurrences (tasks page, documents page, chat page)
catch (e: unknown) — 1 occurrence (chat page)

Best practice: Use catch (e: unknown) and narrow with e instanceof Error. The e: any pattern loses type safety.

C-3 · 🟡 MEDIUM — `$lib/stores/ui.ts` uses legacy Svelte stores alongside Svelte 5 runes

File: src/lib/stores/ui.ts

Uses writable() from svelte/store for sidebarOpen, membersPanelOpen, activeModal, modalData, isLoading, loadingMessage. The rest of the app uses Svelte 5 $state() runes.

Fix: Migrate to runes-based stores using .svelte.ts files (like toast.svelte.ts already does).

All in 3 files:

SponsorsWidget.svelte — color picker buttons missing labels (4 warnings)
admin/+page.svelte — form labels not associated with controls (3 warnings)
SponsorsWidget.svelte — buttons without text (2 warnings)

Fix: Add aria-label attributes to color picker buttons; use id/for pairs on admin form labels.

C-5 · 🟢 GOOD — Consistent API module pattern

All 13 API modules in $lib/api/ follow the same pattern: typed function signatures, Supabase client as first param, structured error logging via createLogger(), and thrown errors for callers to handle. This is clean and maintainable.

C-6 · 🟢 GOOD — Centralized error handling

Both server (hooks.server.ts) and client (hooks.client.ts) have handleError functions that generate error IDs, log structured data, and return consistent error shapes. The +error.svelte page shows error ID + "Copy Error Report" with recent logs dump. This is production-quality.

4. Architecture

A-1 · 🟡 HIGH — Client-side mutations should use form actions

The admin dashboard correctly uses SvelteKit form actions for all CRUD. But other pages (settings, events, team management) perform mutations via direct supabase.from() calls from the browser.

Recommendation: Adopt the admin dashboard pattern (form actions + use:enhance) for all destructive operations. This provides:

Server-side auth verification
Progressive enhancement (works without JS)
Automatic invalidateAll() on success

A-2 · 🟡 MEDIUM — Large page components

Several pages are 500+ lines:

[orgSlug]/events/[eventSlug]/dept/[deptId]/+page.svelte — ~1100 lines (department dashboard)
[orgSlug]/events/[eventSlug]/team/+page.svelte — ~500 lines
[orgSlug]/chat/+page.svelte — ~400 lines
admin/+page.svelte — ~660 lines

The department dashboard is the largest but is inherently complex (8 module types). The admin page is well-structured with tab separation.

Recommendation: Extract reusable table/list components from admin page for potential reuse in other admin-like views.

A-3 · 🟢 GOOD — Clean module architecture

The project follows a clear separation:

$lib/api/ — Data access layer (13 modules)
$lib/components/ui/ — Design system (44 components)
$lib/components/modules/ — Feature widgets (8 modules)
$lib/stores/ — Global state (4 stores)
$lib/utils/ — Utilities (7 files)
$lib/matrix/ — Matrix chat integration (isolated)

A-4 · 🟢 GOOD — Server-side auth pattern

hooks.server.ts correctly uses getUser() (server-verified) before getSession() (client-provided). The safeGetSession pattern prevents session spoofing. All page server loads check auth before data fetching.

5. Performance

P-1 · 🟡 MEDIUM — Google Fonts loaded via external CSS import

File: src/routes/layout.css:1-2

@import url('https://fonts.googleapis.com/css2?family=Tilt+Warp&...');
@import url('https://fonts.googleapis.com/css2?family=Material+Symbols+Rounded:...');

Two render-blocking external CSS imports. Material Symbols font is ~200KB.

Fix options:

Self-host fonts (fastest — eliminates external dependency)
Add <link rel="preconnect" href="https://fonts.googleapis.com"> to app.html
Use font-display: swap (already set via &display=swap)

P-2 · 🟡 MEDIUM — `emojiData.ts` is 40KB

File: src/lib/utils/emojiData.ts (40,133 bytes)

This is a static JSON blob imported at module level. It's included in the client bundle even if the user never opens the emoji picker.

Fix: Lazy-load via dynamic import:

const emojiData = await import('$lib/utils/emojiData');

P-3 · 🟢 GOOD — Optimistic updates on kanban

Card moves use synchronous state updates + fire-and-forget DB persist. Realtime handlers use incremental diffs instead of full reloads. optimisticMoveIds Set suppresses realtime echoes. This is well-implemented.

P-4 · 🟢 GOOD — Vite config optimized for Windows

File watcher ignores node_modules, test-results, .svelte-kit, and paraglide cache. Matrix crypto WASM excluded from SSR bundling. This prevents common Windows dev server performance issues.

6. Testing

TS-1 · 🟡 MEDIUM — No Svelte component tests

112 unit tests exist but all test pure TypeScript functions (API modules, utils, markdown parsing). No Svelte component rendering tests despite vitest-browser-svelte being installed and configured.

Priority components to test:

Button, Modal, Input, TabBar — core UI
Avatar — name-to-color hashing logic
StatusBadge — status-to-color mapping

TS-2 · 🟡 MEDIUM — E2E tests don't cover new features

Playwright tests cover: auth, org CRUD, documents, kanban, calendar. But missing:

Events CRUD flow
Department dashboard
Admin dashboard
Chat integration
Team management

TS-3 · 🟢 GOOD — Test infrastructure is solid

Vitest with browser + server projects
requireAssertions: true prevents empty tests
Playwright with auth setup, cleanup, and CI config
112 passing tests with 0 failures

7. Dependencies

DEP-1 · 🟡 MEDIUM — `@inlang/paraglide-js` duplicated in deps

Listed in both dependencies and devDependencies. Should only be in devDependencies.

DEP-2 · 🟡 LOW — `@types/twemoji` may be unnecessary

twemoji v14 may ship its own types. Check if removing @types/twemoji causes any errors.

DEP-3 · 🟢 GOOD — All dependencies actively used

Every package in dependencies has corresponding imports:

@supabase/ssr + @supabase/supabase-js — core data layer
@tanstack/svelte-virtual — virtualized lists in chat
@tiptap/* — rich text editor
google-auth-library — calendar integration
highlight.js + marked — markdown rendering
matrix-js-sdk — chat
twemoji — emoji rendering

8. DevOps & Infrastructure

DO-1 · 🟢 GOOD — Docker setup

Multi-stage Dockerfile with builder + production stages. Health check endpoint at /health. docker-compose.yml with both production and dev services. Environment variables properly externalized.

DO-2 · 🟢 GOOD — Database migrations

31 sequential migrations with clear naming. RLS policies on all tables. Platform admin bypass policies (migration 031). Auto-create triggers for department dashboards.

DO-3 · 🟡 MEDIUM — No database seed file

No supabase/seed.sql for development setup. New developers must manually create test data.

Fix: Create a seed file with sample org, users, events, and departments.

9. Accessibility

Color picker buttons in SponsorsWidget.svelte lack labels
Admin page form labels not associated with controls
All are fixable with aria-label or id/for attributes

Modal.svelte has role="dialog", aria-modal="true", aria-labelledby, keyboard escape handling, and backdrop click. This follows WAI-ARIA dialog pattern.

+error.svelte shows status code, message, error ID, context, and a "Copy Error Report" button with recent logs. Excellent for debugging.

Summary Scorecard (v5)

Area	Score	Key Notes
Security	3/5	S-1 (credential rotation) still critical. S-4 (server-side auth) partially addressed by admin form actions. RLS bypass for platform admins added.
Type Safety	4/5	`strict: true`. 21 `as any` casts remain, 15 fixable by `npm run db:types`. Matrix SDK casts are acceptable.
Code Quality	4/5	Consistent API pattern. 41 raw `console.*` calls in Matrix code. Inconsistent error catch typing.
Architecture	4/5	Clean module separation. Admin dashboard uses form actions (good pattern). Other pages still use client-side mutations.
Performance	4/5	Optimistic updates, incremental realtime, Windows-optimized watcher. External font loading and 40KB emoji data could improve.
Testing	3.5/5	112 unit tests, Playwright E2E, CI pipeline. Missing: component tests, new feature E2E coverage.
Dependencies	4.5/5	All deps used. One duplicate (`paraglide-js`).
DevOps	4/5	Docker, health checks, 31 migrations. Missing: seed file.
Accessibility	4/5	Good modal/error patterns. 9 minor a11y warnings.
Error Handling	4/5	Structured logger, error IDs, ring buffer, toast system. Matrix code bypasses logger.

Overall: 4.0 / 5.0

Priority Action Items

Tier 1 — Critical (do now)

S-1: Purge .env from git history and rotate all secrets
- git rm --cached .env && git commit
- Use git filter-repo to purge from history
- Rotate: Supabase keys, Google API key, Google service account, Matrix admin token

Tier 2 — High (this week)

T-1: Regenerate Supabase types — npm run db:types eliminates 15 as any casts
C-1: Replace console.* with createLogger() in Matrix integration files
S-3: Fix auth callback open redirect — add safeRedirect() validator
A11Y-1: Fix 9 a11y warnings — add aria-label and id/for attributes

Tier 3 — Medium (this month)

S-4/A-1: Migrate mutations to form actions — start with settings page
C-3: Migrate $lib/stores/ui.ts to runes — align with Svelte 5 patterns
P-1: Self-host fonts or add preconnect hints
P-2: Lazy-load emoji data
TS-1: Add Svelte component tests for core UI components
DO-3: Create database seed file
DEP-1: Remove duplicate paraglide-js from dependencies

Tier 4 — Low (backlog)

C-2: Standardize error catching to catch (e: unknown)
TS-2: Expand E2E tests to cover events, departments, admin, chat
T-2: Derive manual type aliases from generated types after regeneration
S-6: Consolidate lock cleanup to single path
DEP-2: Check if @types/twemoji is needed

Feature Backlog

Permission enforcement (hasPermission() utility)
Global search across all entities
Keyboard shortcuts
Notifications system
Mobile responsive layout
Undo/redo with toast-based undo
Onboarding flow for new users

18 KiB Raw Blame History Unescape Escape