hackerspace.sh/docs/AUDIT-2026-08.md

Audit & Architecture Plan — hackerspace-management (2026-08)

Branch: audit/cleanup-architecture. Method: 14-agent adversarial audit (each security/correctness finding independently verified against RLS policies, code authz gates, and DB constraints before it counts). All 16 risk findings survived verification; 0 were refuted — the finders were disciplined, not noise. Every finding below has file:line evidence and a concrete failure scenario.

Baseline health (verified today, main @ 9ac0c24)

Check	State
tsc --noEmit	586 errors, all hidden by next.config.mjs typescript.ignoreBuildErrors:true
eslint .	fails — no flat config present (ESLint 9 needs eslint.config.js)
vitest run (unit)	639 tests / 32 files, all green (~11s)
integration / e2e	present but self-skip without a DB / are largely vacuous (see Tests)
build	passes because type errors are ignored
Package manager	pnpm (not npm); there is no typecheck script

Headline: this is a well-built codebase. The server-action pattern (parseInput(Zod) → requireMemberWithRole → .eq('space_id', self.space_id) → logActivity) is uniform and correct, RLS is enabled on all 50 tables, and the layering the brief asks for is ~70% already present. The real work is (a) a handful of concrete bugs, (b) turning type-checking back on, (c) making tests actually test, and (d) subdividing giant files along seams that already exist — evolve, don't rewrite.

---

1. Correctness & security findings (ranked, all verified)

HIGH

H1 — Invite max_uses is a check-then-increment TOCTOU · lib/auth-actions.ts:210 Two users redeem a max_uses=1 invite simultaneously: both read uses_count=0, both pass the JS cap check, both insert a membership, and the non-atomic uses_count = invite.uses_count + 1 write leaves the counter at 1 instead of 2. Single-use invites become multi-use; counter corrupts. Fix: one atomic guarded statement — UPDATE space_invites SET uses_count = uses_count + 1 WHERE id = :id AND (max_uses IS NULL OR uses_count < max_uses) RETURNING id; only proceed if a row came back.

MEDIUM

M1 — Board can re-arm an existing admin-granting invite · lib/actions/invites.ts:89 (privilege escalation) updateInvite runs the canAssignInviteRole guard only when role is in the payload. A board member (who cannot mint an admin invite) can call updateInvite(id, { is_enabled:true, max_uses:5 }) on an exhausted/disabled admin invite — no role field — and re-arm it. RLS invites_update only checks admin/board, so nothing stops it. Board thereby distributes admin access it can't create directly. Fix: load the existing invite's role and require canAssignInviteRole(member.role, existingRole) whenever any privilege-affecting field (is_enabled→true, max_uses, expires_at) changes.

M2 — appealIncident role gate is broader than the RLS write; silent partial failure · lib/actions/incidents.ts:247 Gated by requireMember (any member — the docstring intends reporters to appeal), but the incident UPDATE is restricted to admin/board by incidents_update RLS. A reporter's appeal inserts a draft proposal, silently updates 0 incident rows, and returns success — orphaned proposal, incident never linked. Fix: do the incident update via admin client after re-authorizing caller as reporter-or-admin, or add a reporter clause to incidents_update RLS; and assert rows-affected before creating the proposal.

M3 — SSRF filter misses hex IPv4-mapped IPv6 · lib/door-logic.ts:72 isBlockedDoorIp blocks the dotted form ::ffff:169.254.169.254 but not the hex form ::ffff:a9fe:a9fe (= 169.254.169.254 IMDS) or ::ffff:7f00:1 (= 127.0.0.1). Defense-in-depth gap on the door egress / metadata-endpoint boundary. Node resolvers currently emit the dotted form, so not exploitable today, but it's a latent SSRF bypass. Fix: decode ::ffff:HHHH:HHHH to its 32-bit IPv4 and run the same checks.

M4 — logCashPayment stamps last_paid_at from a caller-supplied (backdatable) date · lib/actions/payments.ts:56 M5 — linkPaymentToMember overwrites last_paid_at with the linked payment's date · lib/actions/payments.ts:104 Both unconditionally set last_paid_at and force payment_status='current' with no GREATEST guard. Linking/logging a backdated (e.g. CSV-imported) payment regresses a member's dues state — a currently-paid member can be pushed backward or a lapsed member wrongly shown current. flexibleDateTime has no lower bound, so historical dates are real inputs. Fix: advance-only (GREATEST(last_paid_at, txn)), derive payment_status from the resulting effective date.

M6 — Null deref crashes the members list on search · app/(app)/members/members-client.tsx:110 m.display_name.toLowerCase() / m.email.toLowerCase() — both columns are nullable (schema + types; the WHERE email IS NOT NULL partial index proves null-email rows are by design, e.g. imported members). Typing any character into the search box throws TypeError and blanks the whole members page. Fix: optional-chain like the adjacent handle already does.

LOW (confirmed)

• L1 reserveEquipment doesn't verify the manager-supplied memberId is in the caller's space (its sibling signUpForClass does) — mis-attributed reservation within the space; no cross-tenant leak. equipment.ts:249
• L2 Unverified members (pending approval in require_approval spaces) can create/open proposals and cast counted votes before an admin vets them. Arguable-by-design; RLS also permits it. proposals.ts:150
• L3 markOnboardingStepDone read-modify-writes the onboarding_progress JSON in JS → concurrent step completions can be lost (last-write-wins). onboarding.ts:142
• L4 Email dates render in server-local TZ (no timeZone option) → off-by-one-day in renewal/booking emails depending on the droplet's TZ. notifications-logic.ts:114
• L5 parseHeatSyncLog: (a) a stale H:M:E time-of-day bleeds into later events (fabricated audit timestamp); (b) one high-half token can be reused for two adjacent grants (wrong card numbers). door-log-logic.ts:78,91
• L6 Null deref in the payment link-picker initials (m.display_name.split(' ')). payments-client.tsx:377
• L7 Unparseable CSV transaction_date throws mid-import → nothing imports, no toast, button stuck on "Importing…" forever (no try/finally). import-client.tsx:215

Public / webhook / cron surface — CLEAN

Stripe (per-space signing secret, replay-protected via unique insert), door inbound (per-connection vault bearer + timingSafeEqual behind a length pre-check, uniform 401/404), both crons (CRON_SECRET via timingSafeEqual, fail-closed 503), public forms (getPublicForm excludes members visibility, writes scoped to the looked-up form's space), /track (192-bit reporter token, redacted projection). Only nits: two auth-gated DoS-hardening gaps on door inbound (trusts Content-Length; DB lookup before rate-limit).

RLS cross-check — CLEAN

All 50 tables have RLS. A programmatic check of all 147 anon-client writes against 287 policies found zero writes to a table lacking that operation's policy. The two policy-less tables (notification_preferences, stripe_webhook_events) are admin-only by design and correctly scoped. The sole WITH CHECK(true) is the intentional space-creation onboarding path.

---

2. Type safety — the linchpin (path to remove ignoreBuildErrors)

Root cause of ~450 of the 586 errors: version drift. package.json pins supabase-js ^2.49.4 but the install resolved to 2.99.0, whose SupabaseClient grew from 3 to 5 generics. @supabase/[email protected]'s createServerClient still passes the old 3-generic shape, feeding the Schema object into the SchemaName string slot. The constraint violation collapses the client's Schema to never, so every .from().select() row is never (→ 396 × TS2339 "property does not exist on never"), every insert value is never, every rpc arg is undefined. This is exactly why lib/actions/* (admin client, supabase-js direct) mostly type-check while every app/**/page.tsx (ssr client) is a wall of never.

Proven interim fix (verified compiling): annotate the two factory returns as SupabaseClient<Database> in lib/supabase/server.ts + lib/supabase/proxy.ts — re-asserting Database lets the 5-generic client recompute Schema and restores full row typing, clearing ~450 errors. Proper fix: align versions (@supabase/ssr ≥0.7 for supabase-js 2.9x, or pin supabase-js to the 2.5x line) so the lockfile stops drifting.

Second cause: types/database.ts is partially hand-maintained and out of sync with the 54 SQL migrations — missing RPCs (class_signup_tx, class_cancel_tx are called but absent), missing the __InternalSupabase marker. Fix: regenerate wholesale via supabase gen types typescript, then delete the as string casts it forced throughout lib/actions/.

Ordered path to drop ignoreBuildErrors: (1) align supabase versions / annotate the two factories → clears ~450; (2) regenerate types/database.ts → clears the RPC/enum union noise; (3) fix ~30 enum-widening setState casts + the 2 null derefs (M6, L6); (4) fix test-file types + vitest.setup.ts beforeAll/afterAll imports; (5) remove the flag and add tsc --noEmit to CI.

Note on the brief's "split types/database.ts (1958 lines)": don't physically split it. It's a generated build artifact — the next supabase gen types overwrites it wholesale, reverting any hand-split. The generated/hand-written seam the brief wants already runs between types/database.ts and lib/types.ts. The right move is to keep database.ts pristine (add a "GENERATED — do not edit" banner) and split the hand-written side (lib/types.ts → types/domain/*). Delivered in the refactor plan below.

---

3. Tests — green but under-testing the risk

• CRITICAL No test exercises admin-client actions for cross-tenant (IDOR) scoping — the #1 stated risk is untested. Nothing would catch a dropped .eq('space_id', …).
• HIGH CI runs no tests at all; the integration suite self-skips without a DB, so "green" is empty.
• HIGH The authz primitives (requireMember/requireMemberWithRole/getAuthMember) have zero tests.
• HIGH __tests__/actions.test.ts asserts on string literals that check themselves (e.g. const query = `.eq('space_id', …)`; expect(query).toContain('space_id')) — never imports an action. The describe blocks ("Row Level Security", "Role-Based Access") give false confidence.
• MEDIUM e2e specs are unauthenticated/vacuous; components.test.tsx tests stub components defined inside the test file; concurrency/money only covered at the DB layer, not the action layer.
• vitest.setup.ts calls beforeAll/afterAll without importing them (works only via globals:true, untyped).

Plan: add a real action-layer IDOR test (seed spaces A/B, call each id-taking action as an A-admin with a B id, assert rejection + B untouched); unit-test the authz primitives; replace the tautological actions.test.ts with tests that drive real actions through a mock client and inspect the .eq calls; make CI fail on 0 executed integration tests.

---

4. Dead code & duplication

• lib/security.ts: 8 of 12 exports unused in production (7 test-only, 1 fully dead); hasSqlInjectionPatterns is security theater over parameterized queries. Prune to what's used.
• lib/validations.ts: 6 exported schemas never referenced (signInSchema, signUpSchema, createSpaceSchema, joinSpaceSchema, proposalStatuses, updateAreaLeadRoleSchema) + 1 test-only.
• Orphaned files: components/task-claim-button.tsx, components/theme-provider.tsx; dead roleDisplayName in lib/role-labels.ts; duplicate escapeHtml (dead one in security.ts).
• No shared date/number formatter — 68 ad-hoc toLocale* call sites with copy-pasted option objects across ~15 files. classes.ts repeats a notification fan-out 3×; door.ts rebuilds an adapter query 5×.

5. Maintainability offenders

• ops-client.tsx (956) — God component: 3 modals + 2 row renderers + 4 tab bodies + filtering, and it writes secrets/area-leads directly from the browser (bypassing the existing lib/actions/secrets.ts / area-leads.ts — skips Zod + activity log). settings-client.tsx / members-client.tsx (663 each) fuse 4 tabs / table+toolbar+2 modals.
• HIGH footgun: MemberFormFields is declared inside members-client.tsx's render → remounts and loses input focus on every keystroke. members-client.tsx:197
• forms.ts submitForm is a ~200-line monolith with 4 overlapping email-linking functions; import-client.tsx embeds a CSV parser + auto-mapper that belong in lib/.

---

6. Architecture & refactor plan (evolve, not rewrite)

The five layers already exist: validation (lib/validations.ts) · guard/auth (lib/auth-helpers.ts

• lib/*-guard.ts) · pure logic (lib/*-logic.ts) · server actions (lib/actions/*.ts) · data access (lib/supabase/*). Feature-folders already exist (lib/door/, lib/stripe/, lib/secrets/) and UI decomposition already has an exemplar in app/(app)/customize/ (thin orchestrator + panels/ + shared types.ts). The plan promotes these existing patterns — every giant file splits along seams that are already marked by section comments or sub-components.

Target conventions: types/database.ts = only generated file (pristine) · types/domain/* = hand-written domain types (from lib/types.ts) · lib/validations/<feature>.ts + primitives.ts + index.ts barrel (keeps @/lib/validations working for all 44 importers) · lib/<feature>/{logic,guard}.ts · lib/actions/<feature>/*.ts behind lib/actions/index.ts barrel · app/(app)/<route>/{page, <route>-client,panels/,components/,types}.tsx. A new feature = one file per layer, wired only through barrels (full add-a-feature guide goes in ARCHITECTURE.md).

Reviewable slice sequence (each gated on tsc + test + build):

0. Enabler — GENERATED banner on types/database.ts; document regen command. (no behavior change)
1. lib/validations.ts → lib/validations/<feature>.ts + primitives.ts + barrel. Proves the barrel-preserving technique on the most-imported module (44 importers, zero call-site churn).
2. lib/types.ts → types/domain/{index,governance,reexports}.ts; convert governance rows to Tables<'proposals'> aliases; leave lib/types.ts as a re-export shim (11 importers). This is the brief's "split the types file," done correctly.
3. lib/actions/classes.ts → classes/{definitions,sessions,signups,instructor}.ts + _guard/_forms.
4. lib/actions/door.ts → door/{connections,operate,self-entry}.ts + _guards/_audit.
5. lib/actions/forms.ts → forms/{manage,submit,member}.ts + _guard.
6. ops-client.tsx → panels/ + components/ and replace the direct browser writes with the existing server actions (behavior change — ships alone).
7. settings-client.tsx → panels/.
8. members-client.tsx → components/ + fix the MemberFormFields-remount focus bug.
9. Cleanup — delete the lib/types.ts shim + dead validations/security exports once importers move.

---

Proposed execution order (fixes first, then refactor)

• Phase A — tooling & type safety (no behavior change): ESLint flat config, typecheck script, align supabase versions / annotate the two clients, regenerate types/database.ts, work toward removing ignoreBuildErrors. Gate each on tsc/test/build.
• Phase B — confirmed fixes (call out the behavior-changing ones): H1, M1–M6, then the LOW batch, each with a regression test. This is where the IDOR test + authz-primitive tests land.
• Phase C — refactor slices 0–9 above, one reviewable commit each.
• Phase D — ARCHITECTURE.md documenting the resulting structure + add-a-feature guide.

Nothing is committed yet beyond this document. No pushes to main without your explicit approval.