Weekly Security Review: May 16 – May 23, 2026

[timothyfroehlich]

PRs reviewed: #1398, #1400, #1410, #1411, #1412, #1413, #1414, #1416, #1417, #1424
PRs skipped (docs, deps, workflow/CI only): #1384, #1397, #1401, #1402, #1403, #1404, #1407, #1409, #1415, #1418, #1419, #1420, #1421, #1422, #1423, #1425, #1426
Verdict: 2 low-severity findings worth addressing; no critical violations

---

Non-Negotiable Checklist

Status	Rule	Note
✅	CORE-SEC-001 — Auth checks on Server Actions	#1417 calls auth.getUser() first; #1424 calls verifyAdmin() before any user data access
✅	CORE-SEC-002 — Zod input validation	All new/changed actions use Zod schemas. #1417's partial-update maps "on"/"off" through Zod correctly
✅	CORE-SEC-003/004 — CSP + nonce	No middleware or CSP changes this week
✅	CORE-SEC-005 — No hardcoded hostnames	#1413's getBlobStoreHostname() derives the preconnect URL from the BLOB_READ_WRITE_TOKEN env var — not hardcoded
✅	CORE-SEC-006 — Minimal data at server→client boundary	No new full domain objects passed as Client Component props observed
✅	CORE-SEC-007 — Email addresses never displayed outside admin/settings	No new email display paths introduced
✅	CORE-SSR-001/002 — SSR wrapper + getUser() immediately	No SSR client construction changes
✅	CORE-SSR-003 — Middleware present	Untouched this week
❓	CORE-SSR-007 — Never query auth.users in application code	See Finding 1 below
✅	CORE-ARCH-008 — Permissions matrix matches enforcement	No new permission checks added outside checkPermission()
✅	CORE-FORM-002 — Correct autocomplete tokens	#1398 fixes all auth forms: current-password, new-password, autocomplete="off" on confirm fields
✅	CORE-A11Y-004 — No <div role="button">	#1416 removes the offending <div role="button"> from InlineEditableField, replaces with a real <button>
✅	CORE-TEST-006 — No live third-party endpoints in E2E	No new external URLs introduced in test specs

---

Broader Analysis

Finding 1 — auth.users raw SQL in a production Server Action file (❓ CORE-SSR-007 borderline)

PR #1424 (refactor(admin/users): replace direct auth.users query in admin invite with Supabase Admin API)

The production path is correct: createAdminClient().auth.admin.listUsers() replaces the old db.query.authUsers. Good change.

However, the test shim embedded in getAdminClient() does this:

// src/app/(app)/admin/users/actions.ts
if (process.env.NODE_ENV === "test") {
  return {
    auth: { admin: { listUsers: async () => {
      const result = (await db.execute(
        sql`SELECT id, email FROM auth.users`  // ← raw auth.users query
      )) as unknown;
      ...
    }}}
  };
}

CORE-SSR-007 exception covers supabase/seed.sql and pglite.ts — not production Server Action files. Embedding a test shim (including as unknown type escape) directly in actions.ts violates the spirit of both rules. The query can't execute in production, but it:

• Lives in the application bundle alongside production code
• Sets a precedent for mixing test shims into action files
• Uses two TypeScript safety escapes (as unknown → array cast)

Recommendation: Move the test shim into a dedicated test helper or use dependency injection (pass the admin client as an argument with a default). The existing pglite.ts setup is the canonical place for auth.users bootstrap queries.

---

Finding 2 — Unpaginated listUsers() in duplicate-invite guard (low severity)

PR #1424, src/app/(app)/admin/users/actions.ts around line 220.

const { data: authUsersData } = await adminClient.auth.admin.listUsers();
const existingAuthUser = authUsersData.users.find(
  (u) => u.email?.toLowerCase() === validated.email
);

listUsers() is called without perPage/page arguments. The Supabase Admin API defaults to returning 50 users per page. If the instance ever exceeds that count, the duplicate-email check silently misses users beyond page 1, and an admin could send a second invite to an already-active account.

This is admin-only and low-impact today (APC is a small org), but the guard should be explicit. Either paginate to exhaustion or — better — add a direct DB lookup for userProfiles by email (the profile row exists if the trigger ran) before falling back to the Admin API call.

---

Positive Practices Worth Calling Out

• refactor(admin/users): replace direct auth.users query in admin invite with Supabase Admin API (PP-zjlf) #1424 Admin API adoption: Removing db.query.authUsers (Drizzle querying auth.users schema) in favour of the Supabase Admin client is exactly right. Good precedent.
• fix(notifications): preserve unsubmitted toggles in prefs action (PP-tk45) #1417 partial-update regression fix + test: The integration test in notification-preferences-action.test.ts covers the exact bug (absent fields silently flipping to false) with a named regression case. Solid test design.
• feat(ui): add :user-invalid styling + aria-invalid blur sync to Input/Textarea (PP-kqbk.2) #1400 handleBlur guard: if (!("aria-invalid" in props)) correctly protects form-library-controlled state from being overwritten by native checkValidity() — a subtle correctness invariant that's easy to miss and was tested.
• perf(ui): tighten LCP image priority discipline + Vercel Blob preconnect (PP-kqbk.5) #1413 getBlobStoreHostname() token handling: Only the public store reference (already embedded in every Blob URL) reaches client HTML; the secret suffix never leaves the server. Clean.
• fix: auth form correctness and autocomplete attributes (PP-kqbk.1) #1398 confirm-password autocomplete="off": Password managers were previously offered autofill on the confirm field (autocomplete="new-password"). This was a UX footgun and is now correctly set to off.

---

Recommendations

1. (Low) Move the getAdminClient() test shim out of actions.ts. Options: extract a createAdminClientForEnv() factory in ~/lib/supabase/admin.ts that branches internally, or inject the client as a default argument. This keeps actions.ts free of test code. Ref: src/app/(app)/admin/users/actions.ts lines 28–79.

2. (Low) Guard against silent page truncation in listUsers(). A short-term fix is to check userProfiles by email first (cheaper, owned data), and only fall back to the Admin API for the edge case where a profile row is missing. Ref: src/app/(app)/admin/users/actions.ts around line 220.

Neither finding is a security vulnerability in the current deployment context. Both are correctness and maintainability concerns that could become relevant at larger scale or if the test-gating assumption ever fails.