PRs reviewed: #1380, #1464, #1465, #1492, #1495
Skipped (docs, dep bumps, test reclassifications, tooling): #1455, #1456, #1457, #1459, #1460, #1461, #1462, #1463, #1468, #1469, #1470, #1471, #1472, #1475, #1476, #1477, #1478, #1479, #1480, #1481, #1482, #1483, #1484, #1485, #1486, #1487, #1488, #1489, #1490, #1491, #1493, #1494, #1497, #1498, #1499, #1501, #1502, #1503, #1504, #1505, #1506, #1507, #1508
Verdict: All clear. One informational observation on RLS policy gap; positive findings on a security fix and new feature security hygiene.
Non-Negotiable Checklist
| Status |
Rule |
Note |
| ✅ |
CORE-SEC-001 — Protect APIs/Actions |
All three new timeline server actions (add/edit/delete) call auth.getUser() immediately and gate on checkPermission() before any mutation. No new unprotected endpoints. |
| ✅ |
CORE-SEC-002 — Validate all inputs |
Zod schemas cover machineId (UUID), tag (userTagSchema, blocks reserved tags), contentJson (parsed + proseMirrorDocSchema + non-empty plaintext check), and event id (UUID). No raw FormData used. |
| ✅ |
CORE-SEC-003 — Security headers via middleware |
Middleware not modified this week. |
| ✅ |
CORE-SEC-004 — Nonce-based CSP |
No CSP changes. |
| ✅ |
CORE-SEC-005 — No hardcoded hostnames |
No new hardcoded localhost/domain literals introduced. |
| ✅ |
CORE-SEC-006 — Minimal data at server→client boundary |
PR #1464 explicitly excludes reporterEmail from the issue detail Drizzle query (columns: { reporterEmail: false }). The new timeline page passes only id, authorName, authorAvatarUrl, tag, content, eventData, and people to client components — no email fields. |
| ✅ |
CORE-SEC-007 — Email privacy |
PR #1464 fixes a pre-existing RSC payload leak on the issue detail page. resolve-person.ts is explicitly documented: "Never surfaces emails (CORE-SEC-007) — callers join only the generated name columns." getMachineTimeline selects author.name/author.avatarUrl only; no email columns fetched. |
| ✅ |
CORE-SEC-008 — localhost, never 127.0.0.1 |
Not touched. |
| ✅ |
CORE-SSR-001 — SSR wrapper |
All new server-side code uses createClient() from ~/lib/supabase/server. |
| ✅ |
CORE-SSR-002 — auth.getUser() immediately |
All three timeline actions call await supabase.auth.getUser() as the first line after client creation with no interleaved logic. |
| ✅ |
CORE-SSR-003 — Middleware required |
Not removed or bypassed. |
| ✅ |
CORE-SSR-007 — No auth.users queries |
No application code queries the internal auth.users table; DB trigger usage in migrations is the documented exception. |
| ✅ |
CORE-ARCH-008 — Permissions matrix |
Three new permissions defined in matrix.ts and enforced in actions.ts: machines.timeline.comment.add (member+, unconditional), machines.timeline.comment.edit (own semantics — author only), machines.timeline.comment.delete (own_or_owner with admin override). Matrix and enforcement are in sync. |
| ✅ |
CORE-ARCH-009 — Drizzle migrations only |
Migrations 0037–0040 all use db:generate / db:migrate pattern; no drizzle-kit push found. |
| ✅ |
CORE-TEST-006 — No live external endpoints in E2E |
Test reclassification PRs move E2E specs to PGlite integration; no production third-party hostnames introduced in any test. |
Broader Analysis
PR #1380 — Machine Timeline V1 (new attack surface)
The largest new surface this week. Key observations:
Public readability is intentional and consistent. /m/* is listed as isPublic in src/lib/supabase/middleware.ts (line 106), a pre-existing policy. The timeline page correctly handles accessLevel = "unauthenticated" for non-logged-in visitors — compose/edit/delete are blocked, read is allowed. Timeline content (comment text, system event descriptions, actor display names) is visible to anyone with the URL. This matches the existing machine-detail and issues-list behavior and is appropriate for a public-venue machine tracker.
Edit/delete actions are scoped to the event's machine — client can't supply a forged machineId. Both editMachineCommentAction and deleteMachineCommentAction load the event row from the DB first (timelineEvents.findFirst by event id) and pull machineId and authorId from the persisted row. The client never supplies these values directly — only the event id and new content. The addMachineCommentAction does accept a machineId from the client (necessary to target the right machine), but validates it as a UUID and performs a DB lookup before insertion; a non-existent machine returns "Machine not found" before any write.
Reserved tags blocked at input validation boundary. userTagSchema rejects the system-only tags lifecycle and issue; a member cannot forge a lifecycle or issue-class timeline event by crafting a comment request.
ProseMirror content risk is bounded. The contentJson field is JSON-parsed, schema-validated, and empty-content-rejected before storage. The stored document is rendered server-side by Tiptap on read — no raw HTML injection path. XSS via ProseMirror node types would require Tiptap to render untrusted HTML, which is the extension's default-off path.
TOCTOU delete-wins-race handled. Both edit and delete use a boolean return from the DB helper (updateMachineComment / softDeleteMachineComment) and surface a meaningful error if the row was concurrently deleted between the pre-check and the write. This is the PP-h850 pattern applied consistently.
PR #1465 — RLS enabled on timeline tables without policies (informational)
Migration 0037 runs:
ALTER TABLE "timeline_event_people" ENABLE ROW LEVEL SECURITY;
ALTER TABLE "timeline_events" ENABLE ROW LEVEL SECURITY;
No permissive policies are added for these tables in 0037 or in the follow-up 0040 migration (PR #1495). The 0040 migration adds policies for user_profiles, invited_users, and discord_integration_config only.
Impact assessment: The app's primary data access is through Drizzle ORM using a direct Postgres connection (service role or equivalent), which bypasses RLS by default. PostgREST (Supabase REST API) access to these tables would be completely blocked for all roles — the PostgreSQL default when RLS is enabled with no permissive policy is deny-all. This is consistent with the intent (no direct PostgREST access to raw timeline tables) but leaves a gap: if a policy is ever needed to grant read access via PostgREST in the future, the developer would need to add one explicitly.
This is not a vulnerability in the current architecture, but warrants a tracking note. Consider adding a comment in the schema or migration explaining the intentional no-policy stance.
PR #1495 — DB advisor remediation (positive)
Three trigger functions (check_machine_owner_not_guest, check_no_demotion_of_machine_owner, check_no_demotion_of_invited_owner) were re-created with SET search_path = pg_catalog, public (Supabase lint 0011 compliance). The handle_new_user() function already had SET search_path = public and is additionally restricted by REVOKE EXECUTE from anon and authenticated (migration 0035), so it was correctly excluded from the scope of this fix.
RLS policies in migration 0040 properly use (select current_setting(...)) wrapping (initplan optimization) and all carry TO authenticated scope with explicit USING/WITH CHECK predicates. The merge of two permissive UPDATE policies on user_profiles correctly uses logical OR to preserve the union of owner-check and admin-check.
New dependency: eslint-plugin-better-tailwindcss (PR #1460)
Added as a devDependency only. No production bundle surface. The rule blocks raw palette classes and hardcoded hex values — a code-quality control, no security impact.
Recommendations
-
(Low priority — observational) timeline_events / timeline_event_people have RLS enabled but no policies. Add a migration comment (or a block comment in schema.ts near .enableRLS()) noting that direct PostgREST access to these tables is intentionally blocked. This prevents future confusion if a maintainer adds a policy incorrectly expecting the "allow" default.
-
(Positive practice to continue) The resolve-person.ts module's explicit "Never surfaces emails (CORE-SEC-007)" comment is a good pattern. Consider adding similar explicit exclusion guards in other data-boundary files that join user_profiles (verify reporterEmail: false is consistently applied anywhere issues is queried with Drizzle's with: or select: API).
Reviewer: automated Claude security routine, 2026-06-07
PRs reviewed: #1380, #1464, #1465, #1492, #1495
Skipped (docs, dep bumps, test reclassifications, tooling): #1455, #1456, #1457, #1459, #1460, #1461, #1462, #1463, #1468, #1469, #1470, #1471, #1472, #1475, #1476, #1477, #1478, #1479, #1480, #1481, #1482, #1483, #1484, #1485, #1486, #1487, #1488, #1489, #1490, #1491, #1493, #1494, #1497, #1498, #1499, #1501, #1502, #1503, #1504, #1505, #1506, #1507, #1508
Verdict: All clear. One informational observation on RLS policy gap; positive findings on a security fix and new feature security hygiene.
Non-Negotiable Checklist
add/edit/delete) callauth.getUser()immediately and gate oncheckPermission()before any mutation. No new unprotected endpoints.machineId(UUID),tag(userTagSchema, blocks reserved tags),contentJson(parsed +proseMirrorDocSchema+ non-empty plaintext check), and eventid(UUID). No raw FormData used.reporterEmailfrom the issue detail Drizzle query (columns: { reporterEmail: false }). The new timeline page passes only id, authorName, authorAvatarUrl, tag, content, eventData, and people to client components — no email fields.resolve-person.tsis explicitly documented: "Never surfaces emails (CORE-SEC-007) — callers join only the generatednamecolumns."getMachineTimelineselectsauthor.name/author.avatarUrlonly; no email columns fetched.localhost, never127.0.0.1createClient()from~/lib/supabase/server.auth.getUser()immediatelyawait supabase.auth.getUser()as the first line after client creation with no interleaved logic.auth.usersqueriesauth.userstable; DB trigger usage in migrations is the documented exception.matrix.tsand enforced inactions.ts:machines.timeline.comment.add(member+, unconditional),machines.timeline.comment.edit(ownsemantics — author only),machines.timeline.comment.delete(own_or_ownerwith admin override). Matrix and enforcement are in sync.db:generate/db:migratepattern; nodrizzle-kit pushfound.Broader Analysis
PR #1380 — Machine Timeline V1 (new attack surface)
The largest new surface this week. Key observations:
Public readability is intentional and consistent.
/m/*is listed asisPublicinsrc/lib/supabase/middleware.ts(line 106), a pre-existing policy. The timeline page correctly handlesaccessLevel = "unauthenticated"for non-logged-in visitors — compose/edit/delete are blocked, read is allowed. Timeline content (comment text, system event descriptions, actor display names) is visible to anyone with the URL. This matches the existing machine-detail and issues-list behavior and is appropriate for a public-venue machine tracker.Edit/delete actions are scoped to the event's machine — client can't supply a forged machineId. Both
editMachineCommentActionanddeleteMachineCommentActionload the event row from the DB first (timelineEvents.findFirstby eventid) and pullmachineIdandauthorIdfrom the persisted row. The client never supplies these values directly — only the eventidand new content. TheaddMachineCommentActiondoes accept amachineIdfrom the client (necessary to target the right machine), but validates it as a UUID and performs a DB lookup before insertion; a non-existent machine returns "Machine not found" before any write.Reserved tags blocked at input validation boundary.
userTagSchemarejects the system-only tagslifecycleandissue; a member cannot forge a lifecycle or issue-class timeline event by crafting a comment request.ProseMirror content risk is bounded. The
contentJsonfield is JSON-parsed, schema-validated, and empty-content-rejected before storage. The stored document is rendered server-side by Tiptap on read — no raw HTML injection path. XSS via ProseMirror node types would require Tiptap to render untrusted HTML, which is the extension's default-off path.TOCTOU delete-wins-race handled. Both edit and delete use a boolean return from the DB helper (
updateMachineComment/softDeleteMachineComment) and surface a meaningful error if the row was concurrently deleted between the pre-check and the write. This is the PP-h850 pattern applied consistently.PR #1465 — RLS enabled on timeline tables without policies (informational)
Migration 0037 runs:
No permissive policies are added for these tables in 0037 or in the follow-up 0040 migration (PR #1495). The 0040 migration adds policies for
user_profiles,invited_users, anddiscord_integration_configonly.Impact assessment: The app's primary data access is through Drizzle ORM using a direct Postgres connection (service role or equivalent), which bypasses RLS by default. PostgREST (Supabase REST API) access to these tables would be completely blocked for all roles — the PostgreSQL default when RLS is enabled with no permissive policy is deny-all. This is consistent with the intent (no direct PostgREST access to raw timeline tables) but leaves a gap: if a policy is ever needed to grant read access via PostgREST in the future, the developer would need to add one explicitly.
This is not a vulnerability in the current architecture, but warrants a tracking note. Consider adding a comment in the schema or migration explaining the intentional no-policy stance.
PR #1495 — DB advisor remediation (positive)
Three trigger functions (
check_machine_owner_not_guest,check_no_demotion_of_machine_owner,check_no_demotion_of_invited_owner) were re-created withSET search_path = pg_catalog, public(Supabase lint 0011 compliance). Thehandle_new_user()function already hadSET search_path = publicand is additionally restricted byREVOKE EXECUTEfromanonandauthenticated(migration 0035), so it was correctly excluded from the scope of this fix.RLS policies in migration 0040 properly use
(select current_setting(...))wrapping (initplan optimization) and all carryTO authenticatedscope with explicitUSING/WITH CHECKpredicates. The merge of two permissive UPDATE policies onuser_profilescorrectly uses logicalORto preserve the union of owner-check and admin-check.New dependency:
eslint-plugin-better-tailwindcss(PR #1460)Added as a
devDependencyonly. No production bundle surface. The rule blocks raw palette classes and hardcoded hex values — a code-quality control, no security impact.Recommendations
(Low priority — observational)
timeline_events/timeline_event_peoplehave RLS enabled but no policies. Add a migration comment (or a block comment inschema.tsnear.enableRLS()) noting that direct PostgREST access to these tables is intentionally blocked. This prevents future confusion if a maintainer adds a policy incorrectly expecting the "allow" default.(Positive practice to continue) The
resolve-person.tsmodule's explicit "Never surfaces emails (CORE-SEC-007)" comment is a good pattern. Consider adding similar explicit exclusion guards in other data-boundary files that joinuser_profiles(verifyreporterEmail: falseis consistently applied anywhereissuesis queried with Drizzle'swith:orselect:API).Reviewer: automated Claude security routine, 2026-06-07