arthur/xtablo-source
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions 5
xtablo-source/.planning/codebase/CONCERNS.md
Arthur Belleville 2c23906716
docs: map existing codebase
2026-05-14 16:01:31 +02:00

15 KiB
Raw Permalink Blame History

Codebase Concerns Map

Generated 2026-05-14. Catalogs tech debt, security considerations, fragile zones, and known gotchas across the xtablo-source Turborepo. Pointers use repo-relative paths; line numbers are accurate as of this scan.

TODOs / FIXMEs / HACKs / XXX

Grep of apps/ + packages/ (excluding node_modules, dist) — TODO markers are sparse, suggesting either healthy hygiene or undocumented debt.

  • apps/api/src/index.ts:67 — // TODO: Add health check endpoint. No /healthz for Cloud Run, which complicates liveness/readiness probes.
  • apps/api/src/routers/invite.ts:26 — // TODO: Verify that the owner_id is correct. Authorization gap on invite creation.
  • apps/api/src/routers/invite.ts:128 — // TODO: Verify that the event start and end correspond to a slot. Booking integrity not enforced server-side.
  • apps/main/src/components/WebcalModal.tsx:125 — {/* TODO: Add webcal URL */} — feature placeholder shipping incomplete.
  • apps/main/src/lib/rum.ts:25,31 — Datadog RUM session sampling rates are commented // TODO: Uncomment when we have enough data — observability runs in a partial state.
  • apps/api/src/routers/user.ts:54 — // Deprecated: name field is deprecated, use first_name and last_name instead — dual fields still flowing through APIs.

No FIXME, HACK, or XXX markers found in apps/ or packages/ source — but absence is not assurance; many concerns live under euphemisms (see @deprecated below).

@deprecated markers:

  • apps/main/src/hooks/user.ts:16 — useUser hook deprecated in favor of useSession from SessionContext. Likely still has callers.

Known issues (from docs)

The docs/ directory contains 35+ retrospective *_FIX.md, *_SETUP.md, and migration notes. No single TROUBLESHOOTING.md exists; institutional knowledge is scattered. Notable items:

  • docs/MIDDLEWARE_INITIALIZATION_FIX.md — November 2025 incident: routers called MiddlewareManager.getInstance() at module-load time before initialize() ran. Fixed by passing the manager into router factories, but the singleton's eager-throw API (getInstance() throws if not initialized) remains a footgun for any new router that forgets the pattern.
  • docs/STRIPE_SECURITY_FIX.md — Migration 37: public.active_subscriptions view exposed all users' subscription data without RLS. Replaced with SECURITY DEFINER function get_my_active_subscription(). Future views need similar audit.
  • docs/STRIPE_MIGRATION_36.md, STRIPE_WITH_SYNC_ENGINE.md, STRIPE_FINAL_SETUP.md, STRIPE_IMPLEMENTATION_SUMMARY.md, STRIPE_CLEANUP_* — eleven Stripe documents indicate repeated rework on the billing surface.
  • docs/TEST_FIXES.md, docs/TEST_ROUTER_REFACTOR.md, docs/API_TESTS.md, docs/MIDDLEWARE_TESTS.md — test setup has needed periodic refactoring; mocks are tightly coupled to the singleton.
  • SECURITY_NOTICE.md (repo root) — .env files were previously committed to git history. The checklist of credentials to rotate (Supabase service role, Stripe, Stream, Google OAuth, R2) is in the file; verification that rotation occurred is not tracked here.

Security considerations

JWT handling

  • Validation occurs in apps/api/src/helpers/auth.ts via validateAuthHeader + Supabase auth.getUser(token). Wired into authMiddleware / maybeAuthenticatedMiddleware in apps/api/src/middlewares/middleware.ts.
  • apps/main/src uses jwt-decode (^4.0.0) to inspect tokens client-side — purely decode, no verification (correct), but any code path that trusts decoded claims for authorization would be a bug.
  • Client portal sessions use a separate JWT (CLIENT_AUTH_JWT_SECRET, apps/api/src/helpers/clientSessions.ts) with cookie storage (CLIENT_AUTH_COOKIE_NAME). Two independent token systems = two attack surfaces.
  • Admin sessions live in apps/api/src/helpers/adminTokens.ts signed by ADMIN_TOKEN_SIGNING_SECRET — a third token surface.

Service role key usage (RLS bypass)

The Supabase service role key is created exactly once in apps/api/src/middlewares/middleware.ts:164 and injected into every authenticated request via supabaseMiddleware. This means every API handler operates with full RLS bypass. Authorization logic therefore must live in handler code; any forgotten ownership check is a data-exposure bug. Notable areas relying on handler-side checks:

  • apps/api/src/routers/tablo.ts, tablo_data.ts
  • apps/api/src/routers/admin*.ts (six admin routers)
  • apps/api/src/routers/clientPortal.ts
  • The two TODO: Verify ... owner_id/slot comments in invite.ts are direct evidence of this risk.

Stripe webhook verification

  • apps/api/src/routers/stripe.ts:167-191 — verification is delegated to @supabase/stripe-sync-engine's processWebhook(rawBody, signature). The route does retrieve raw body via c.req.text() (correct — verification needs unmodified bytes). Note: webhook router is wired pre-auth in routers/index.ts, which is required by Stripe — verify any future restructuring preserves this ordering.
  • Webhook secret comes from STRIPE_WEBHOOK_SECRET in config; rotation procedure not documented in repo.

Secrets / env vars

  • Production/staging: loaded from Google Secret Manager via apps/api/src/secrets.ts and assembled in apps/api/src/config.ts.
  • Dev: .env via dotenv. .env* is now gitignored (see SECURITY_NOTICE.md).
  • apps/api/src/config.ts:32 requires a long list of secrets — validateEnvVar throws on missing; good fail-fast, but means a single missing env aborts boot with no partial-feature degradation.
  • Frontend env: apps/main reads VITE_* env at build time per environment (build:staging, build:prod). Anything VITE_* is bundled into the public JS — only public keys belong here.

Performance considerations

  • React Query defaults: packages/shared/src/lib/api.ts:18 sets a global staleTime of 5 minutes. Aggressive caching is appropriate for dashboard data but can hide write-after-read bugs; mutations must explicitly invalidate hierarchical keys (["tablos", id], etc.).
  • Custom stale times: apps/main/src/hooks/stripe.ts:114 (5 min) and :221 (10 min) — billing data caching for 10 minutes risks displaying a stale subscription state after a webhook arrives. UI should also listen to mutation success or refresh on Stripe-portal-return paths.
  • Pagination: ~63 hits for .select("*") / .range( / .limit( across apps/main/src + apps/api/src (excluding tests). Many list endpoints (apps/api/src/routers/tablo.ts, admin*.ts) appear to return full tables; no shared cursor/offset helper exists. AG-Grid in main app loads client-side which exacerbates this for orgs with large datasets.
  • Source-only packages: @xtablo/shared, @xtablo/ui, @xtablo/chat-ui, @xtablo/auth-ui, @xtablo/tablo-views, @xtablo/shared-types export TS directly. Pros: instant HMR. Cons: every app re-typechecks and re-bundles them; tree-shaking depends on each app's bundler being able to drop unused exports (Vite generally handles this, but barrel files in packages/shared/src/index.ts-style modules can defeat it — worth auditing if bundle size matters).
  • Bundle size: apps/main has rollup-plugin-visualizer available for analysis but no tracked size budgets. Heavy deps: @blocknote/* (rich text editor), ag-grid-community + ag-grid-react, jspdf, @datadog/browser-rum* — all in dependencies of apps/main/package.json.
  • Datadog dd-trace (apps/api) is initialized in apps/api/src/index.ts:13 before everything else; misconfigured tracing has measurable cold-start cost on Cloud Run.

Fragile areas

Stripe sync engine (Supabase ↔ Stripe)

  • apps/api/src/middlewares/stripeSync.ts instantiates @supabase/stripe-sync-engine@^0.45.0 with a direct Postgres connection string (SUPABASE_CONNECTION_STRING) and base64-encoded CA cert (SUPABASE_CA_CERT). Bypasses Supabase API entirely.
  • revalidateObjectsViaStripeApi: ["subscription", "customer"] — explicit workaround for stale-data bugs in the sync engine.
  • Schema is "stripe" (separate from public); migrations must be applied carefully; eleven separate Stripe docs in docs/ evidence repeated breakage.
  • Sync is eventually consistent: UI hooks with 5–10 min staleTime (above) can show pre-webhook state.

Auth flow (passwordless + temporary accounts)

  • Passwordless flow creates accounts flagged is_temporary: true. Lifecycle (cleanup of abandoned temp accounts, upgrade path to permanent) is not documented in docs/.
  • Three independent token systems (user JWT, client-portal JWT, admin token) live side by side — see Security section. Test reference: apps/api/src/__tests__/README.md:28 documents a test_temp@example.com fixture.
  • SessionContext (main app) listens to supabase.auth.onAuthStateChange(). Deprecated useUser hook at apps/main/src/hooks/user.ts:16 still exists and likely has stragglers.

Middleware singleton initialization order

  • MiddlewareManager in apps/api/src/middlewares/middleware.ts is a singleton with throw-on-misuse semantics. The November 2025 fix (see docs/MIDDLEWARE_INITIALIZATION_FIX.md) is a pattern convention, not a structural guarantee. Any new router that calls MiddlewareManager.getInstance() at module top-level reintroduces the bug.
  • The index.ts router order (apps/api/src/routers/index.ts) is load-bearing: public routes → stripe webhook (no auth, raw body) → auth-applied routes. Refactors that reorder these break either auth or signature verification.

Build / deploy concerns

  • Cloudflare Workers (apps/main, possibly apps/clients, apps/external, apps/admin): apps/main/wrangler.toml sets compatibility_date = "2025-07-09" and no compatibility_flags. Notably no nodejs_compat — any dep pulling Node built-ins at runtime will fail at the edge. Watch for incidental Node imports in shared packages.
  • Type generation: per CLAUDE.md, run npx supabase gen types typescript > packages/shared-types/src/database.types.ts after schema changes. There's no CI guard that types are up to date; drift between DB and types is silent.
  • Cache invalidation gotchas (from CLAUDE.md "Important Notes"): stale builds resolved only by pnpm clean && rm -rf node_modules/.cache && pnpm install && pnpm build. Indicates Turborepo cache occasionally misses dependency changes — possibly because source-only packages don't declare outputs.
  • API deploy uses Cloud Run with Cloud Build (docs/CLOUD_BUILD_*.md, docs/DOCKER_*.md). Multi-stage pnpm Docker build is documented and has needed multiple optimization passes (DOCKER_BUILD_PERFORMANCE.md, DOCKER_PNPM_OPTIMIZATION.md, DOCKER_FIX_SUMMARY.md).
  • API uses tsc only (apps/api/package.json build: tsc) — no bundling, ships dist/ + node_modules. Large image surface; dependency vulnerabilities are deploy-time concerns.
  • Pre-commit: .pre-commit-config.yaml exists at root; behavior not audited here.

Dependencies of concern

Pinned/notable in apps/main/package.json:

  • @typescript/native-preview: 7.0.0-dev.20251010.1 — a preview/nightly TypeScript native compiler pinned to a dated dev build. High churn risk; may break unexpectedly.
  • @types/react: 19.0.10, @types/react-dom: 19.0.4 — exact-pinned (no caret). react: 19.0.0 itself also exact-pinned. Upgrades will require coordinated change.
  • vitest: ^3.2.4 (in apps/main) vs vitest: ^4.0.8 (in apps/api) — different major versions across the monorepo; shared test utilities will be incompatible.
  • @types/react-router-dom: ^5.3.3 listed alongside react-router-dom: ^7.9.4 — the v5 type stubs are wrong for v7; either unused or actively misleading.
  • eslint: ^9.22.0 + @typescript-eslint/*: ^7.0.2 — typescript-eslint v7 predates flat-config-stable ESLint 9; potential plugin compat issues. Note also Biome is the primary linter (biome.json), so ESLint may be vestigial.
  • jest + jest-environment-jsdom + @types/jest in apps/main devDeps despite using Vitest — dead dependencies inflating install.
  • pnpm.overrides: form-data: ^4.0.4, linkifyjs: ^4.3.2 — root-level overrides usually indicate working around a transitive vulnerability or bug; reason isn't documented in repo.

In apps/api/package.json:

  • stripe: ^20.0.0 — Stripe SDK is currently on v17+ as of cutoff; v20 is a future major. Verify lockfile matches expected runtime version.
  • ts-node: ^10.9.2 listed in dependencies (not devDeps) — likely unused at runtime; should be a devDep.
  • multer: ^2.0.2 — major version 2.x is recent; ensure middleware patterns aren't using legacy 1.x APIs.

pnpm-lock.yaml is ~763 KB indicating substantial dependency graph; pnpm audit not run as part of this scan.

Documentation gaps

  • No TROUBLESHOOTING.md despite 35+ retrospective fix docs — there's no central index.
  • docs/ is fix-log heavy and architecture-light. STRIPE_ARCHITECTURE.md and MIDDLEWARE_TESTS.md exist but most flows lack an "as-built" diagram.
  • Three token systems (user JWT, client session JWT, admin token) — no unified auth doc; you must read apps/api/src/helpers/{auth,clientSessions,adminTokens}.ts separately.
  • Temporary accounts lifecycle (creation, retention, cleanup, upgrade to permanent) — undocumented.
  • Six admin routers (adminActions, adminAuth, adminDatasets, adminOverview, adminTables, admin.ts) — admin surface is large and the only docs are ADMIN_APP_ACCESS_SETUP.md for access setup, not authorization model.
  • Frontend bundle budgets — none defined; rollup-plugin-visualizer available but not enforced.
  • Type-generation workflow — only mentioned in CLAUDE.md; no CI check.
  • apps/chat-worker, apps/clients, apps/admin, apps/external — minimal architectural docs; main app dominates CLAUDE.md.
  • Legacy directories at repo root (backend/ Python, go-backend/ Go, frontend_v2/, xtablo-expo/) are present but unmentioned in CLAUDE.md. Status (active? abandoned?) is unclear — this itself is a debt signal.
  • SECURITY_NOTICE.md lists credentials to rotate after the .env-in-git incident, but completion status of that rotation checklist is not tracked.

Tracked-but-unaddressed observations

  • 43 console.error/console.warn calls in apps/api/src/routers/ — direct logging instead of going through pino (which is a devDep but not wired to the routers).
  • 21 explicit : any annotations in apps/api/src (excluding tests). Two visible examples: apps/api/src/routers/clientInvites.ts:192 ((candidate: any) =>) and apps/api/src/helpers/helpers.ts:374 ((u: any) =>).
  • Only 2 @ts-ignore/@ts-expect-error comments across apps/ + packages/ — TypeScript discipline appears solid where types exist.