Weekly Security Review: Jul 03 – Jul 11, 2026

[timothyfroehlich]

PRs reviewed: #1595, #1606, #1629, #1630, #1634, #1388, #1585
PRs skipped (docs/CI/tooling/deps/test-type fixes): #1582, #1584, #1587, #1596, #1598–#1601, #1604–#1605, #1609, #1612–#1616, #1617–#1621, #1623–#1628, #1631–#1633, #1636–#1638
Verdict: All clear. Two active security fixes landed. No violations found. Several positive security practices observed.

---

Non-Negotiable Checklist

	Rule	Note
✅	CORE-SEC-001 — Protect APIs/Server Actions	#1388 introduces 6 new Server Actions, all guarded by authorizeManage() → supabase.auth.getUser() + checkPermission() before any DB write
✅	CORE-SEC-002 — Validate all inputs with Zod	#1388 uses saveSchema.safeParse(), idSchema.safeParse(), setPreferredSchema.safeParse() etc. on every action entry point; 200 KB byte ceiling on payload too
⬜	CORE-SEC-003/004 — CSP / security headers	No changes to middleware.ts or next.config.ts this week
⬜	CORE-SEC-005 — No hardcoded hostnames	No new hardcoded hostnames found
✅	CORE-SEC-006 — Minimal data at server→client boundary	#1388 getMachineSettingsSets maps to { id, name, isPreferred, description, sections, updatedBy: name-only, updatedAt } — no roles, emails, or internal IDs passed to the SettingsTab client component
✅	CORE-SEC-007 — Email privacy	No email exposed in new #1388 UI; updatedBy is row.updatedByUser?.name ?? "Unknown", never the email address
⬜	CORE-SEC-008/009 — localhost / env var registry	No changes
✅	CORE-SSR-001/002 — SSR wrapper, getUser() immediately	#1388 page and actions call createClient() → auth.getUser() before any logic
⬜	CORE-SSR-003/004 — Middleware / auth callback	Untouched this week
✅	CORE-SSR-007 — No direct auth.users queries	#1388 uses userProfiles, not auth.users. (Pre-existing local dev stub in admin/users/actions.ts queries auth.users via SQL — predates this week, not a new regression.)
✅	CORE-ARCH-008 — Permissions matrix	#1388 adds machines.settings.manage (member → owner-scoped; technician/admin → any) to src/lib/permissions/matrix.ts at line ~11176; all six new actions reference it
✅	CORE-ARCH-011 — No side effects in DB transactions	emitSettingsSetEvent is a pure DB write (createMachineTimelineEvent → INSERT INTO timeline_events). No HTTP, email, Discord, or blob calls inside any db.transaction() callback this week
✅	CORE-ARCH-009 — Drizzle migrations only	#1585 drops machines.owner_notes via drizzle/0047_left_triton.sql; #1595 enables RLS via 0046_clear_bug.sql; #1630 adjusts indexes via 0050_overjoyed_viper.sql. All proper Drizzle migrations.
⬜	CORE-TEST-006 — No live third-party hostnames in E2E	No new E2E specs touching external hosts

---

Broader Analysis

🔒 Security Fixes (positive — both landed this week)

#1634 — Paginate auth.users lookup in admin invite (src/app/(app)/admin/users/actions.ts)

The old inviteUser action called adminClient.auth.admin.listUsers() with no pagination — Supabase/GoTrue defaults to 50 rows, so a duplicate email on page 2+ would slip through the uniqueness check and generate a second invite. The fix:

• Extracts findAuthUserByEmail() that pages with perPage: 1000 until an empty page (count-based, not nextPage-based — correct, since auth-js mis-parses nextPage for page ≥ 10)
• Adds AUTH_USERS_MAX_PAGES = 1000 safety cap with a loud error on overflow
• Integration test seeds 60 users so the collision sorts onto page 2, verifying the exact bug scenario

#1595 — Enable RLS on pinballmap_catalog (drizzle/0046_clear_bug.sql)

Fixes a Supabase advisor ERROR-level finding (rls_disabled_in_public). The table had no row-level security — any authenticated PostgREST client could read the full catalog. The fix enables RLS (fail-closed: no ALLOW policies needed since the app only accesses the table server-side via Drizzle/service role). Confirmed: pinballmap_catalog is referenced only in src/server/db/schema.ts — no client-side Supabase reads exist.

🛡️ Guard Stack Canary (#1629)

After the 2026-07-05 incident where a settings.json rewrite silently wiped the entire PreToolUse hook stack + permissions block, .claude/hooks/verify-guard-stack.cjs was added. It's a warn-only SessionStart hook (always exits 0) that checks for the 7 expected guard hook basenames and non-empty permissions.deny/permissions.ask. Good defensive posture; 30 unit tests covering healthy, degraded, and subprocess fail-open paths.

📋 IDOR Coverage in #1388

The machine settings actions use a two-step ownership chain:

• saveSettingsSetAction: looks up the machine by machineId input, then calls authorizeManage(machine.ownerId) — prevents writing settings to a machine you don't own
• deleteSettingsSetAction / setPreferredSettingsSetAction: calls loadSetWithMachine(id) which joins the set to its machine, then authorizeManage(loaded.machine.ownerId) — the machine ID is resolved from the data, not from user input, blocking cross-machine IDOR
• saveSettingsSetAction update path explicitly checks existing.machineId !== machineId to prevent re-parenting a set to a different machine

📝 Minor Observation — as unknown as SettingsSection[] (#1388, settings/actions.ts ~line 5024)

Two as casts appear in saveSettingsSetAction: as ProseMirrorDoc | null and as unknown as SettingsSection[]. Both are "validate-then-cast" patterns applied after Zod has proven the shape, bridging the compile-time gap between the Zod-inferred type (which strips the client-only _key field) and the branded SettingsSection[] type. Matches the existing pattern in timeline comment actions. Not a runtime safety escape, but technically a CORE-TS-007 marginal case — worth watching for if the pattern spreads.

📦 Dependency Bumps

• tiptap group (8 packages) → 3.27.x (chore(deps)(deps): bump the tiptap group with 8 updates #1596): Rich text editor bump. The XSS pipeline (renderer → sanitize-html with strict tag/attribute allowlist + nonTextTags covering xmp/noscript/noembed/noframes) is unchanged and correct. The sanitize-html config relies on default allowedSchemes (which blocks javascript: and data: in href) — works correctly but an explicit allowedSchemes declaration in src/lib/tiptap/render.ts would make this easier to audit in the future.
• @sentry/nextjs → 10.60.x, actions/cache → 6.1.0, sethvargo/ratchet: No security-relevant changes identified.

---

Recommendations

1. (Low) Explicit allowedSchemes in src/lib/tiptap/render.ts — The SANITIZE_OPTIONS object doesn't set allowedSchemes, relying on sanitize-html's defaults (['http', 'https', 'ftp', 'mailto', 'tel']). Adding allowedSchemes: ['http', 'https', 'mailto', 'tel'] makes the intent self-documenting and makes future audits faster. ftp: is probably not needed.

2. (Low) CORE-TS-007 marginal: "validate-then-cast" pattern — The as unknown as SettingsSection[] pattern is sound today but could drift toward unsafe use if copied without the Zod-proof step. Consider a typed utility like brandedSettingsSection(parsed.data.sections) to make the validation-before-cast invariant structural rather than documented-in-comment.

3. (Pre-existing, not new this week) auth.users direct SQL in admin client stub — src/app/(app)/admin/users/actions.ts getAdminClient() contains a local dev fallback that queries SELECT id, email FROM auth.users directly. This is a CORE-SSR-007 exception case (the real createAdminClient() path is used in production), but it's worth tracking since the rule's only listed exceptions are supabase/seed.sql and pglite.ts.

---

Review generated by automated weekly security scan — 2026-07-11.