feat: import row validation + KB/process per-item ACLs

Continuing the audit backlog and finishing the permissions UI. Import validation (recurring audit finding): - Added importMembersSchema / importPaymentsCsvSchema. importMembers and importPaymentsCsv now validate per row (a single bad row no longer rejects the whole file), lowercase emails, normalize dates through flexibleDateTime(), enforce tier/platform enums and positive finite amounts, and return a `skipped` count instead of silently dropping invalid rows. Permissions UI completion: - OpsAclEditor is now wired into the KB/process viewer modal (admin/board), with entity type derived from the `process` tag. Per-item Ops access control now covers secrets + KB + processes, matching "all ops facilities stuff". Left as-is with rationale in HANDOFF: /ops + /import denial paths (unreachable behind the layout gate or already a clear non-leaking denial); members-row make-area-lead shortcut (Customize -> Area leads fully covers assignment); types/database.ts + docs regen (pure hygiene, zero runtime/security impact, still tracked). 271/271 tests pass. Build clean. No migration this pass.
2026-05-15 18:33:53 -07:00
parent f5d670477b
commit 86d44f6efc
6 changed files with 111 additions and 54 deletions
@@ -19,6 +19,8 @@ All notable changes to this project are documented here. Format follows [Keep a
 - `/settings` is gated server-side to admins before any space/integration data is fetched.

 ### Fixed
+- Bulk import (`importMembers`, `importPaymentsCsv`) now validates every row with Zod: lowercased emails, `flexibleDateTime` date normalization, enum-checked platform/tier, positive finite amounts. Invalid rows are skipped and counted (returned as `skipped`) instead of silently dropped; one bad row no longer rejects the whole file.
+- Per-item Ops access control now also covers knowledge base and process entries (wired into the viewer modal), not just secrets.
 - `revealSecret` no longer silently returns the stale plaintext column when a row is marked encrypted but has no ciphertext; it surfaces the corruption instead.
 - `upsertAreaLead` rejects a null/empty area code (Postgres treats NULLs as distinct in unique constraints, which had allowed duplicate area-lead rows).

@@ -430,12 +430,20 @@ function KbEntryRow({
  entry,
  onEdit,
  onDelete,
+  canManageAcl,
+  aclRoleOptions,
+  aclByEntity,
 }: {
  entry: KbEntry
  onEdit: (e: KbEntry) => void
  onDelete: (id: string) => void
+  canManageAcl?: boolean
+  aclRoleOptions?: { value: string; label: string }[]
+  aclByEntity?: Record<string, string[]>
 }) {
  const [viewing, setViewing] = useState(false)
+  const aclEntityType: 'kb' | 'process' = entry.tags?.includes('process') ? 'process' : 'kb'
+  const aclInitial = aclByEntity?.[`${aclEntityType}:${entry.id}`] ?? []

  async function handleDelete(e: React.MouseEvent) {
    e.stopPropagation()
@@ -525,6 +533,14 @@ function KbEntryRow({
              <div className="prose prose-sm prose-neutral dark:prose-invert max-w-none break-words">
                <ReactMarkdown remarkPlugins={[remarkGfm]}>{entry.content ?? ''}</ReactMarkdown>
              </div>
+              {canManageAcl && (
+                <OpsAclEditor
+                  entityType={aclEntityType}
+                  entityId={entry.id}
+                  options={aclRoleOptions ?? []}
+                  initial={aclInitial}
+                />
+              )}
            </div>
          </div>
        </div>
@@ -671,6 +687,9 @@ export function OpsClient({ member, spaceId, kbEntries: initial, areaLeads: init
                      entry={entry}
                      onEdit={e => { setEditingKb(e); setShowKbModal(true) }}
                      onDelete={id => setKbEntries(prev => prev.filter(e => e.id !== id))}
+                      canManageAcl={canManageAcl}
+                      aclRoleOptions={aclRoleOptions}
+                      aclByEntity={aclByEntity}
                    />
                  ))}
                </div>
@@ -686,6 +705,9 @@ export function OpsClient({ member, spaceId, kbEntries: initial, areaLeads: init
                      entry={entry}
                      onEdit={e => { setEditingKb(e); setShowKbModal(true) }}
                      onDelete={id => setKbEntries(prev => prev.filter(e => e.id !== id))}
+                      canManageAcl={canManageAcl}
+                      aclRoleOptions={aclRoleOptions}
+                      aclByEntity={aclByEntity}
                    />
                  ))}
                </div>
@@ -720,6 +742,9 @@ export function OpsClient({ member, spaceId, kbEntries: initial, areaLeads: init
                    entry={entry}
                    onEdit={e => { setEditingKb(e); setShowKbModal(true) }}
                    onDelete={id => setKbEntries(prev => prev.filter(e => e.id !== id))}
+                    canManageAcl={canManageAcl}
+                    aclRoleOptions={aclRoleOptions}
+                    aclByEntity={aclByEntity}
                  />
                ))}
              </div>
@@ -22,14 +22,18 @@ Five parallel read-only audits (security/RLS, server actions/validation, routes/
 - **`app/(app)/layout.tsx`** marked `export const dynamic = 'force-dynamic'` (auth/onboarding gate must never be cached).
 - **Bounded queries.** `loadComments` `.limit(500)`, tasks page `.limit(2000)`.

-### Tracked backlog (audit findings, not security-critical)
+### Pass 16b follow-up (same day)
+
+- **Import validation done.** `importMembersSchema` / `importPaymentsCsvSchema` added; both actions validate per row (one bad row no longer rejects the file), lowercase emails, normalize dates via `flexibleDateTime()`, enforce platform/tier enums and positive finite amounts, and return a `skipped` count instead of silently dropping rows.
+- **KB/process per-item ACL wired.** `OpsAclEditor` renders in the KB/process viewer modal (admin/board); entity type derived from the `process` tag. Per-item Ops ACLs now cover secrets + KB + processes.
+- `/ops` and `/import` denial paths left as-is: unreachable behind the `(app)` layout gate, or already a clear denial with no data leak.
+
+### Tracked backlog (not security-critical, zero runtime impact)

- `importMembers` / `importPaymentsCsv`: still no Zod row schema; dates not via `flexibleDateTime()`; emails not lowercased; silent row drops. Highest-value remaining correctness item.
 - `types/database.ts` still missing 8 prior-pass tables + `comment_entity_type` enum + 3 `space_members` columns (loose casts; build green via `ignoreBuildErrors`).
 - `docs/DATABASE_SCHEMA.md`, `DB_SCHEMA_MAP.md`, `docs/API_REFERENCE.md` stale since 2026-03-10 (missing migrations 016-024).
- Inconsistent unauthorized handling: `/ops` and `/import` `return null` instead of `redirect`.
- Email normalization (`.toLowerCase()`) missing across auth/member schemas.
- KB/process per-item ACL: infra complete, only the ops-client KB-modal hookup remains. Members-row "make area lead" shortcut still deferred.
+- Email `.toLowerCase()` still missing on the non-import auth/member schemas (import path now normalizes).
+- Members-row "Make area lead" shortcut (Customize -> Area leads fully covers assignment).

 ### Verification

@@ -4,18 +4,9 @@ import { revalidatePath } from 'next/cache'
 import { createClient } from '@/lib/supabase/server'
 import { requireMemberWithRole } from '@/lib/auth-helpers'
 import { ADMIN_ROLES } from '@/lib/permissions'
+import { importMembersSchema } from '@/lib/validations'

-export async function importMembers(
-  rows: Array<{
-    display_name: string
-    email: string
-    phone?: string
-    tier?: string
-    joined_at?: string
-    last_paid_at?: string
-    has_card_access?: boolean
-  }>,
-) {
+export async function importMembers(rows: unknown) {
  if (!Array.isArray(rows) || rows.length === 0) {
    return { error: 'No rows to import' }
  }
@@ -25,33 +16,39 @@ export async function importMembers(
  if (!auth.ok) return { error: auth.error }
  const { member } = auth

-  const inserts = rows
-    .filter(r => typeof r.display_name === 'string' && typeof r.email === 'string')
-    .map(r => ({
+  // Validate per row so one bad row does not reject the whole file; the
+  // schema lowercases email and normalizes dates via flexibleDateTime.
+  const rowSchema = importMembersSchema.element
+  const valid: Array<Record<string, unknown>> = []
+  let skipped = 0
+  for (const raw of rows) {
+    const r = rowSchema.safeParse(raw)
+    if (!r.success) { skipped++; continue }
+    valid.push({
      space_id: member.space_id,
-      display_name: r.display_name,
-      email: r.email,
-      phone: r.phone ?? null,
-      tier: r.tier ?? 'basic',
+      display_name: r.data.display_name,
+      email: r.data.email,
+      phone: r.data.phone ?? null,
+      tier: r.data.tier ?? 'basic',
      role: 'member',
      status: 'current',
      approved: true,
-      joined_at: r.joined_at ?? new Date().toISOString(),
-      last_paid_at: r.last_paid_at ?? null,
-      has_card_access: r.has_card_access ?? false,
-    }))
+      joined_at: r.data.joined_at ?? new Date().toISOString(),
+      last_paid_at: r.data.last_paid_at ?? null,
+      has_card_access: r.data.has_card_access ?? false,
+    })
+  }

-  if (inserts.length === 0) {
-    return { error: 'No valid rows to import' }
+  if (valid.length === 0) {
+    return { error: `No valid rows to import (${skipped} skipped: bad email, name, tier, or date).` }
  }

-  // Upsert by (space_id, email) to dedupe re-imports.
  const { data, error } = await supabase
    .from('space_members')
-    .upsert(inserts, { onConflict: 'space_id,email', ignoreDuplicates: false })
+    .upsert(valid, { onConflict: 'space_id,email', ignoreDuplicates: false })
    .select()

  if (error) return { error: error.message }
  revalidatePath('/members')
-  return { data, count: data.length }
+  return { data, count: data.length, skipped }
 }
@@ -11,6 +11,7 @@ import { TREASURER_ROLES } from '@/lib/permissions'
 import {
  logCashPaymentSchema,
  linkPaymentSchema,
+  importPaymentsCsvSchema,
 } from '@/lib/validations'
 import type { Enums } from '@/types/database'

@@ -114,36 +115,38 @@ export async function linkPaymentToMember(paymentId: string, memberId: string) {
  return { success: true as const }
 }

-export async function importPaymentsCsv(
-  rows: Array<{
-    platform: string
-    amount: number
-    from_identifier: string
-    from_note?: string
-    transaction_date: string
-  }>,
-) {
+export async function importPaymentsCsv(rows: unknown) {
+  if (!Array.isArray(rows) || rows.length === 0) {
+    return { error: 'No rows to import' }
+  }
+
  const supabase = await createClient()
  const auth = await requireMemberWithRole(supabase, TREASURER_ROLES, 'Treasurer access required')
  if (!auth.ok) return { error: auth.error }
  const { member } = auth

-  const validPlatforms: PaymentPlatform[] = ['paypal', 'zeffy', 'venmo', 'cash']
-
-  const inserts = rows
-    .filter(r => validPlatforms.includes(r.platform as PaymentPlatform))
-    .map(r => ({
+  // Per-row validation: platform enum, positive finite amount, normalized
+  // transaction_date. Invalid rows are skipped and counted, not silently
+  // dropped without feedback.
+  const rowSchema = importPaymentsCsvSchema.element
+  const inserts: Array<Record<string, unknown>> = []
+  let skipped = 0
+  for (const raw of rows) {
+    const r = rowSchema.safeParse(raw)
+    if (!r.success) { skipped++; continue }
+    inserts.push({
      space_id: member.space_id,
-      platform: r.platform as PaymentPlatform,
-      amount: r.amount,
-      from_identifier: r.from_identifier,
-      from_note: r.from_note ?? null,
+      platform: r.data.platform as PaymentPlatform,
+      amount: r.data.amount,
+      from_identifier: r.data.from_identifier,
+      from_note: r.data.from_note ?? null,
      link_status: 'unlinked' as const,
-      transaction_date: r.transaction_date,
-    }))
+      transaction_date: r.data.transaction_date ?? new Date().toISOString(),
+    })
+  }

  if (inserts.length === 0) {
-    return { error: 'No valid rows to import' }
+    return { error: `No valid rows to import (${skipped} skipped: bad platform, amount, or date).` }
  }

  const { data, error } = await supabase.from('payments').insert(inserts).select()
@@ -151,5 +154,5 @@ export async function importPaymentsCsv(

  revalidatePath('/payments')
  revalidatePath('/dashboard')
-  return { data, count: data.length }
+  return { data, count: data.length, skipped }
 }
@@ -522,6 +522,32 @@ export const assignAreaLeadSchema = z.object({
  member_id: z.string().uuid('Invalid member ID'),
 })

+// ─── Bulk import ─────────────────────────────────────────────────────────────
+
+const importEmail = z.string().email('Invalid email').max(200).transform(s => s.trim().toLowerCase())
+
+export const importMembersSchema = z.array(
+  z.object({
+    display_name: z.string().min(1, 'Name required').max(100),
+    email: importEmail,
+    phone: z.string().max(20).optional().nullable(),
+    tier: z.enum(['plus', 'basic', 'associate']).optional(),
+    joined_at: flexibleDateTime().optional(),
+    last_paid_at: flexibleDateTime().optional(),
+    has_card_access: z.boolean().optional(),
+  }),
+).max(5000)
+
+export const importPaymentsCsvSchema = z.array(
+  z.object({
+    platform: z.enum(['paypal', 'zeffy', 'venmo', 'cash']),
+    amount: z.number().finite().positive('Amount must be positive').max(10_000_000),
+    from_identifier: z.string().min(1, 'Payer required').max(200),
+    from_note: z.string().max(500).optional().nullable(),
+    transaction_date: flexibleDateTime().optional(),
+  }),
+).max(10000)
+
 // ─── Generic ID schemas ──────────────────────────────────────────────────────

 export const uuidSchema = z.string().uuid('Invalid ID format')