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.

---

Progress log

Phase A — tooling & type safety ✅ (branch audit/cleanup-architecture)

• tsc --noEmit: 586 → 0. next.config ignoreBuildErrors removed — builds/deploys now type-check.
• Root cause fixed by annotating the Supabase client factories (server/client/proxy) as SupabaseClient<Database>; parseInput rewritten to infer via z.infer; flexibleDateTime typed; tsconfig target → ES2020; missing RPCs added to the generated types. A 40-agent behavior-preserving sweep cleared the long tail (joins bridged with as unknown as + // TODO(types) to regenerate).
• ESLint 9 flat config added (eslint.config.mjs) — pnpm run lint now passes (0 errors). Added typecheck script. next build green.

Phase B — confirmed fixes ✅ (one commit per fix, each with a regression test)

• H1 invite max_uses TOCTOU → atomic compare-and-swap claim (lib/invite-claim.ts).
• M1 board re-arming an admin invite → privilege guard on any re-arming field.
• M2 appealIncident → restricted to admin/board; asserts the link UPDATE, rolls back orphan proposal.
• M3 SSRF filter → blocks hex IPv4-mapped IPv6.
• M4/M5 payment last_paid_at → advance-only (resolveDuesAdvance).
• M6 / L6 members-list & payment-picker null derefs → guarded.
• member-directory visibility gate → repaired (was comparing against non-existent enum values).
• L1 reserveEquipment → verifies target member is in the caller's space.
• L2 unverified members → blocked from proposals/votes (per decision).
• L5 parseHeatSyncLog → stale time-of-day + token-reuse fixed.
• L7 CSV import → guards unparseable dates, never wedges the button.
• Tests: tautological actions.test replaced with real authz/IDOR tests; authz primitives covered. 626 tests green.
• Deferred — L3 (markOnboardingStepDone read-modify-write race): a correct fix needs a DB-side atomic append (an RPC/migration) that can't be applied to the live DB from here. Low severity (duplicate step-completions only). Follow-up: add a jsonb append RPC or a unique constraint.

CI ✅

• Added .github/workflows/ci.yml — a checks-only workflow (typecheck + lint + unit tests) on every PR and on main, fully separate from deploy.yml. Follow-up: a DB-backed integration run that fails on 0 executed tests.

Phase C — refactor ✅ (all 10 slices)

• Slice 0: types/database.ts marked GENERATED.
• Slice 1: lib/validations.ts (143 exports) → lib/validations/* barrel'd folder (symbol-diff verified, 40 importers untouched).
• Slice 2: lib/types.ts → types/domain/*.
• Slices 3–5: lib/actions/{classes,door,forms}.ts (800–908 lines) → per-capability folders with _guard/_helpers (plain modules) + 'use server' sub-files + index.ts barrel. Export parity verified (14/14/15); no helper leaked into the public surface.
• Slices 6–8: ops/settings/members client components decomposed into thin orchestrators (956→230, 663→79, 663→255) + panels/+components/+types.ts. Slice 8 also fixed the MemberFormFields-in-render focus footgun.
• Slice 6b (security): the ops "Add Secret" modal wrote the value plaintext from the browser despite its "encrypted at rest" label — routed through createSecret (AES-GCM + authz + audit).
• Slice 9: deleted orphaned files/exports + the 6 dead validation schemas + cleanupRateLimits; migrated @/lib/types → @/types/domain and dropped the shim.
• Each slice: its own commit; gated on tsc 0 + tests + (for UI) next build.

Phase D — ARCHITECTURE.md ✅

• ARCHITECTURE.md at repo root: layers, the server-action contract + space-scope invariant, the feature-folder conventions, and a step-by-step add-a-feature guide.

Re-audit + separation-of-concerns pass ✅ (post-refactor)

A fresh 8-agent re-audit (SoC + refactor-regression + security/correctness) found zero refactor regressions (every authz gate + space_id scope + handler independently verified across the split action files and decomposed components). It surfaced the remaining SoC gaps + a few correctness misses, all now fixed with tests:

• SoC — direct browser writes eliminated: the ops area-lead roster (create/update/delete) and the comms message send now go through server actions (admin/member gate, validation, space-scoping, audit). The comms send also derives identity server-side (was trusting client user_id/ display_name/handle — impersonation). No client component writes to the DB directly anymore.
• Payments IDOR: logCashPayment/linkPaymentToMember now verify the member_id is in the caller's space (same class as L1).
• Stale revalidate: the invites/onboarding/areas/roles/tiers actions revalidated /settings after their panels moved to /customize — corrected.
• Invite CAS: claimInviteUse now distinguishes contention (retry) from a real cap (reject).

Browser smoke test (this environment)

No local Docker → no Supabase, so authenticated screens couldn't be driven with data. Verified on the real dev server: landing + login render with no console/runtime errors; the decomposed authed routes (/ops, /settings, /members) correctly redirect to /login (middleware + server compile OK). All routes compile via next build.

Remaining follow-ups

• Regenerate types/database.ts (drops the // TODO(types) join casts + hand-added RPCs; lets the governance rows in types/domain/governance.ts become Tables<>/Enums<>).
• Deferred L3 onboarding-race RPC; DB-backed CI integration run.
• Manual UI smoke-test the decomposed ops/settings/members screens against a real backend before merge.