fix(supabase): stop server clients leaking a 30s refresh ticker per request (#1612)

* fix(supabase): stop server clients leaking a 30s refresh ticker per request `autoRefreshToken` defaults to true in supabase-js, and off-browser @supabase/auth-js starts the refresh ticker unconditionally: // in non-browser environments the refresh token ticker runs always this.startAutoRefresh() That is a setInterval firing every 30 s. It calls unref(), so the process still exits, tests pass, and Vercel never notices because the process is torn down long before the tickers accumulate. But unref() does not make a timer collectable: it stays registered in the event loop and remains a GC root for its callback, which closes over the GoTrueClient, the SupabaseClient, and the whole request scope around it. A long-running self-hosted instance therefore leaks one timer plus one entire request graph (socket, IncomingMessage, ServerResponse, headers, route context: ~100 kB) per client constructed. One died of "JavaScript heap out of memory" after 42 h, the last 24 of them completely idle. The heap snapshot showed 445 retained request graphs and ~1050 Timeouts in the 30 000 ms bucket, retained via `autoRefreshTicker`, and the rate matched the traffic exactly: the Docker healthcheck polls /api/health every 30 s and the webhook dispatch cron runs every minute, so 3 clients/min x 148 min = 444. - new lib/supabase/service-client.ts: createServiceRoleClient() applies SERVER_AUTH_OPTIONS, spread LAST so a caller passing its own auth block cannot re-enable the ticker - 22 call sites migrated; only booking-templates/sync/cron had ever passed the options itself - guard 9 in no-new-antipatterns.mjs fails CI on any new value import of supabase-js's createClient outside the wrapper; type-only imports are fine. Verified to fail on a deliberate regression and pass once fixed - browser clients untouched: a signed-in tab genuinely needs the refresh, and lib/supabase/client.ts is built on createBrowserClient anyway Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(checks): catch namespace imports in the leaky-supabase-client guard The guard only matched named imports, so import * as sb from '@supabase/supabase-js' sb.createClient(url, key) reached createClient through member access without ever naming it, and passed. Verified against the real script before and after: the shape is flagged now, and `import type * as sb` still passes. Namespace value imports are treated as leaky outright rather than tracking member access, which keeps the check a regex over source text with no new dependency. Review also suggested excluding *.test.tsx alongside *.test.ts. Skipped: the repo has no .test.tsx files, and all four sibling checks in this file use `.test.ts`. Diverging in one of them would read as an accident; if such files appear, all four should change together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 14:30:17 +02:00
parent 4921d1da5e
commit 43a71aec3c
27 changed files with 285 additions and 51 deletions
@@ -58,7 +58,14 @@
 *      the four Skatteverket-bound org-number paths disagreed outright about
 *      what "valid" meant, which is the kind of drift a customer only discovers
 *      when a filing fails at the deadline. Tracked as a count.
- *   9. off-ladder-radius: a border-radius class outside the locked ladder
+ *   9. leaky-supabase-client: server code importing supabase-js's `createClient`
+ *      as a value instead of `createServiceRoleClient()`. The default
+ *      `autoRefreshToken: true` starts a 30 s setInterval that is never
+ *      cleared; `unref()` keeps the process exitable but not the timer
+ *      collectable, so each constructed client retains its whole request scope.
+ *      Killed a self-hosted instance after 42 idle hours (2026-08-13). No
+ *      baseline: the count is 0 today.
+ *   10. off-ladder-radius: a border-radius class outside the locked ladder
 *      (pill / rounded-xl overlays / rounded-lg surfaces / rounded-sm leaves;
 *      see .claude/rules/design.md). Before the 2026-08 migration the UI had
 *      seven radii in circulation (4/5/6/8/12/16px + pill) and one toolbar row
@@ -181,6 +188,64 @@ function findDirectJelInserts() {
    .sort()
 }

+// The one module allowed to import supabase-js's createClient as a value: it
+// is the wrapper that applies SERVER_AUTH_OPTIONS.
+const LEAKY_CLIENT_SANCTIONED = new Set(['lib/supabase/service-client.ts'])
+const SUPABASE_JS_IMPORT_RE = /import\s+(type\s+)?\{([^}]*)\}\s*from\s*['"]@supabase\/supabase-js['"]/g
+// A namespace import hands over the whole module, so `sb.createClient(...)` is
+// reachable without ever naming it in the import. Treat any value-namespace
+// import as leaky rather than trying to track member access.
+const SUPABASE_JS_NAMESPACE_RE =
+  /import\s+(type\s+)?\*\s+as\s+\w+\s+from\s*['"]@supabase\/supabase-js['"]/g
+
+/**
+ * Files that import supabase-js's `createClient` as a VALUE instead of going
+ * through createServiceRoleClient().
+ *
+ * `autoRefreshToken` defaults to true, and auth-js starts the 30 s refresh
+ * ticker unconditionally off-browser. The ticker calls unref(), so the process
+ * still exits and nothing fails in tests or on Vercel, but unref does not make
+ * a timer collectable: it stays a GC root for its callback and retains the
+ * client plus the whole request scope around it. A self-hosted instance died of
+ * heap exhaustion after 42 idle hours this way (2026-08-13), holding 445
+ * request graphs and ~1050 Timeouts in the 30 000 ms bucket.
+ *
+ * Both named (`{ createClient }`) and namespace (`* as sb`) value imports count:
+ * the latter reaches createClient through member access without naming it.
+ *
+ * Type-only imports are fine; so is the browser client, which needs the ticker
+ * and is built on @supabase/ssr's createBrowserClient anyway.
+ */
+function findLeakySupabaseClients() {
+  const files = [
+    ...walk(path.join(ROOT, 'lib'), ['.ts', '.tsx']),
+    ...walk(path.join(ROOT, 'app'), ['.ts', '.tsx']),
+    ...walk(path.join(ROOT, 'extensions'), ['.ts', '.tsx']),
+  ]
+  return files
+    .filter((f) => {
+      const r = rel(f)
+      if (LEAKY_CLIENT_SANCTIONED.has(r)) return false
+      if (r.includes('__tests__/') || r.endsWith('.test.ts')) return false
+      const src = fs.readFileSync(f, 'utf8')
+      for (const m of src.matchAll(SUPABASE_JS_IMPORT_RE)) {
+        const [, typeOnly, bindings] = m
+        if (typeOnly) continue
+        const bindsCreateClient = bindings
+          .split(',')
+          .map((b) => b.trim())
+          .some((b) => b === 'createClient' || b.startsWith('createClient as'))
+        if (bindsCreateClient) return true
+      }
+      for (const m of src.matchAll(SUPABASE_JS_NAMESPACE_RE)) {
+        if (!m[1]) return true
+      }
+      return false
+    })
+    .map(rel)
+    .sort()
+}
+
 // Statement generators that legitimately read journal_entry_lines directly:
 // the trial-balance stack itself, and the reports whose whole job is to list
 // vouchers or lines rather than to aggregate a fiscal year's balances.
@@ -678,6 +743,7 @@ const current = {
  handRolledInvariants: countHandRolledInvariants(),
  ledgerScanningReports: findLedgerScanningReports(),
  directJelInsert: findDirectJelInserts(),
+  leakySupabaseClients: findLeakySupabaseClients(),
  pinnedDepViolations: findPinnedDepViolations(),
  rawUserErrors: findRawUserErrors(),
  sekLabelledAmounts: findSekLabelledFxAmounts(ROOT),
@@ -744,6 +810,23 @@ if (current.directJelInsert.length) {
  )
 }

+// 1b2. leaky-supabase-client: server code must construct clients through
+// createServiceRoleClient(). No baseline: the count is 0 today.
+if (current.leakySupabaseClients.length) {
+  failed = true
+  console.error(
+    `\n✗ leaky-supabase-client: ${current.leakySupabaseClients.length} file(s) import supabase-js's ` +
+      `createClient as a value instead of createServiceRoleClient():`,
+  )
+  current.leakySupabaseClients.forEach((f) => console.error(`    ${f}`))
+  console.error(
+    '  → import { createServiceRoleClient } from "@/lib/supabase/service-client". Constructing a\n' +
+      '    client directly leaves autoRefreshToken on, which starts a 30 s setInterval that is never\n' +
+      '    cleared and retains the client plus the whole request scope (heap death after ~42 h).\n' +
+      '    Type-only imports are fine: use `import type { SupabaseClient } from "@supabase/supabase-js"`.',
+  )
+}
+
 // 1c. pinned-dep: a version-pinned dependency must match its pin EXACTLY, in
 // both package.json and the lockfile. No baseline: any drift is a hard failure.
 if (current.pinnedDepViolations.length) {
@@ -916,5 +999,5 @@ if (failed) {
  process.exit(1)
 }
 console.log(
-  `\n✓ Antipattern guard passed (raw-route-auth: ${current.rawRouteAuth.length}, naive-ore-round: ${current.naiveOreRound}, hand-rolled-invariant: ${current.handRolledInvariants}, ledger-scanning-report: ${current.ledgerScanningReports.length}, direct-jel-insert: 0, pinned-dep: 0, raw-user-error: 0, sek-labelled-amount: 0, off-ladder-radius: 0, cross-extension-import: 0, ungated-extension-route: ${current.extensionRoutes.ungated.length}/${UNGATED_EXTENSION_ROUTES.size} allowlisted).`,
+  `\n✓ Antipattern guard passed (raw-route-auth: ${current.rawRouteAuth.length}, naive-ore-round: ${current.naiveOreRound}, hand-rolled-invariant: ${current.handRolledInvariants}, ledger-scanning-report: ${current.ledgerScanningReports.length}, direct-jel-insert: 0, leaky-supabase-client: 0, pinned-dep: 0, raw-user-error: 0, sek-labelled-amount: 0, off-ladder-radius: 0, cross-extension-import: 0, ungated-extension-route: ${current.extensionRoutes.ungated.length}/${UNGATED_EXTENSION_ROUTES.size} allowlisted).`,
 )