Weekly Security Review: May 23 – May 30, 2026

[timothyfroehlich]

PRs reviewed: #1380, #1402, #1414, #1415, #1417, #1424, #1425, #1437, #1438, #1442, #1445, #1446, #1449, #1451

PRs skipped (docs, agent tooling, test-only without security relevance, or CI config): #1418, #1419, #1420, #1421, #1422, #1423, #1426, #1427, #1428, #1429, #1430, #1433, #1434, #1436, #1440, #1441, #1444, #1447, #1448, #1450, #1452, #1453, #1454

Verdict: 2 findings need attention — one pre-existing PII leak surfaced by a test change, one pagination correctness gap in an admin action.

---

Non-Negotiable Checklist

Status	Rule	Notes
✅	CORE-SEC-001 Auth checks	All new Server Actions in #1380 and #1417 gate on auth.getUser() immediately; verifyAdmin() still present in #1424
✅	CORE-SEC-002 Zod validation	All new inputs validated (#1380 timeline actions use addSchema/editSchema/deleteSchema; #1417 pref action iterates only known PREF_FIELDS)
✅	CORE-SEC-003/004 CSP / nonces	No changes to middleware.ts or next.config.ts this week
⬜	CORE-SEC-005 No hardcoded hostnames	Not touched
❓	CORE-SEC-006 Minimal data at Server→Client boundary	See Finding 1 — reporterEmail is in RSC payload to IssueTimeline
✅	CORE-SEC-007 Email privacy (display)	#1380 explicitly never stores/returns reporterEmail; resolve-person.ts docstring says "Never surfaces emails (CORE-SEC-007)". #1438 adds a stronger regression test
⬜	CORE-SEC-008 localhost vs 127.0.0.1	Not touched
✅	CORE-SSR-001 SSR wrapper	All new actions import createClient from ~/lib/supabase/server
✅	CORE-SSR-002 getUser() immediately	Verified in all new actions and pages
✅	CORE-SSR-007 No direct auth.users query	#1424 fixes the pre-existing violation; test stub in the same PR queries auth.users directly but falls within the allowed exception (test bootstrapping)
✅	CORE-ARCH-008 Permissions via matrix	#1380 adds three new matrix entries (machines.timeline.comment.add/edit/delete) and routes all enforcement through checkPermission(). MachineRecentActivity hard-codes canEdit/Delete=false (intentional read-only section, commented)
✅	CORE-TEST-006 No live third-party endpoints	#1446 adds a default-throw stub to the Discord mock suite (fetch was called with unmocked URL: …), preventing silent real HTTP calls in tests

---

Broader Analysis

Finding 1 — reporterEmail serialized into RSC payload (CORE-SEC-006 / CORE-SEC-007 boundary) ❓

Severity: Medium | Pre-existing; surfaced by #1438

The issue detail page query (src/app/(app)/m/[initials]/i/[issueNumber]/page.tsx:77–146) does not restrict columns at the root issues table level. Because Issue = InferSelectModel<typeof issues>, the query fetches reporterEmail. The fetched object is then cast to IssueWithAllRelations and passed directly to two components:

• IssueTimeline at line 327 — "use client" component — receives issue={issueWithRelations}
• IssueMetadata at line 314 — Server Component, but passes parts downstream

IssueTimeline receiving the full object means reporterEmail is included in the RSC serialized payload sent to the browser. The field is never rendered (CORE-SEC-007 display rule is not violated), but it is present in the client bundle, violating CORE-SEC-006 (minimal data at server→client boundary).

PR #1438 made this explicit: the test was changed from a column-restricted query to an unrestricted one to "mirror how the page invokes it", confirming the production query fetches reporterEmail.

Recommendation: Either (a) add columns: { reporterEmail: false } to the root issue query, or (b) strip reporterEmail from issueWithRelations before passing to IssueTimeline / IssueMetadata, or (c) narrow IssueWithAllRelations to exclude reporterEmail at the type level. Option (a) is cheapest and most direct.

---

Finding 2 — listUsers() pagination cap in admin invite (#1424) ❓

Severity: Low | Introduced by #1424

inviteUser in src/app/(app)/admin/users/actions.ts now calls adminClient.auth.admin.listUsers() with no pagination arguments to check whether an email is already registered. The Supabase Admin API defaults to perPage: 50. For organizations with more than 50 registered auth.users rows, the .find() on the returned array could return undefined even if the email exists on page 2+, allowing a duplicate invite to proceed.

Austin Pinball Collective currently has far fewer than 50 users, so this is not an active risk today, but it will become one as the platform grows.

Recommendation: Paginate the lookup: call listUsers({ page: 1, perPage: 1000 }) (or the actual user cap), or better, use getUserByEmail() if the Supabase Admin API exposes it — a targeted lookup is O(1) instead of O(n) and avoids the pagination problem entirely. Check supabase.auth.admin.getUserByEmail(email) availability.

File: src/app/(app)/admin/users/actions.ts:205–218

---

Notable positive security practices this week

• feat(machines): machine timeline V1 (PP-0x98) #1380 Machine Timeline — exemplary data hygiene. Three new Server Actions, all auth-gated, all Zod-validated, all matrix-routed. The issue-timeline-helpers.ts explicitly stores guestReporterName and never guestReporterEmail, with a code comment citing CORE-SEC-007. The resolve-person.ts docstring says "Never surfaces emails (CORE-SEC-007)". This is the right pattern.

• fix(notifications): preserve unsubmitted toggles in prefs action (PP-tk45) #1417 + refactor(settings): delete dead hidden inputs and extract repeated rows (PP-dxgo) #1445 — hidden input elimination is a security improvement. The old pattern preserved invisible channel preferences via hidden <input type="hidden"> elements that could be client-side tampered (e.g., a user editing the DOM to flip Discord prefs they shouldn't be able to access). The new server-side partial-update approach (ignore absent fields) is strictly safer — the server only acts on what was submitted, and values for non-visible channels are preserved in the DB without passing through the client.

• refactor(admin/users): replace direct auth.users query in admin invite with Supabase Admin API (PP-zjlf) #1424 — CORE-SSR-007 fix. Replacing a direct Drizzle query against auth.users (which violates the non-negotiable) with the Supabase Admin API is the correct fix. The server-only import in ~/lib/supabase/admin.ts ensures the service-role key cannot accidentally reach a Client Component.

• test(admin-discord-mock): fix correctness issues in mockFetch suite (PP-e6a2) #1446 — default-throw fetch stub. Adding throw new Error("fetch was called with unmocked URL: ...") as the default mock in the Discord test suite closes a class-J gap: previously an unmocked URL would silently pass through to the real network (or a test double that didn't assert), which could hit real Discord endpoints from CI.

• chore(deps)(deps): bump the minor-patch-updates group with 6 updates #1415 Sentry PII gate change. @sentry/nextjs 10.52.0 changed IP/browser inference to only fire when sendDefaultPii: true. PinPoint already has sendDefaultPii: false in all three config files (sentry.server.config.ts:16, sentry.edge.config.ts:16, SentryInitializer.tsx:16), so behavior is unchanged. Worth confirming this remains false as Sentry is upgraded further.

---

Recommendations

1. (Medium) Fix reporterEmail RSC leak: add reporterEmail: false to the issue detail page's root column select, or filter it before passing issueWithRelations to IssueTimeline. src/app/(app)/m/[initials]/i/[issueNumber]/page.tsx:77–146.

2. (Low) Fix listUsers() pagination in admin invite: check if getUserByEmail() is available in the Supabase Admin API; if not, request all pages or set perPage to a safe ceiling. src/app/(app)/admin/users/actions.ts:205–218.

3. (Informational) Re-examine test(privacy): broaden email-privacy regression test to mirror page findFirst (PP-kj0s) #1438's test change with fresh eyes: the comment "Verify that reporterEmail is actually retrieved by the query" is accurate but it describes a current behavior that should probably be fixed (see feat: Setup CI with GitHub Actions #1 above), not a deliberate contract.

---

Reviewed by automated security scan — Claude (scheduled routine). Week covered: 2026-05-23 through 2026-05-30.