From 86d44f6efcffa9e9fb6334543331a0cbdeb0da8c Mon Sep 17 00:00:00 2001 From: Moheeb Zara Date: Fri, 15 May 2026 18:33:53 -0700 Subject: [PATCH] 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. --- CHANGELOG.md | 2 ++ app/(app)/ops/ops-client.tsx | 25 ++++++++++++++++++ docs/HANDOFF.md | 14 ++++++---- lib/actions/imports.ts | 51 +++++++++++++++++------------------- lib/actions/payments.ts | 47 +++++++++++++++++---------------- lib/validations.ts | 26 ++++++++++++++++++ 6 files changed, 111 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fd82b2..25b35c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/app/(app)/ops/ops-client.tsx b/app/(app)/ops/ops-client.tsx index 5767c9b..e2bdce5 100644 --- a/app/(app)/ops/ops-client.tsx +++ b/app/(app)/ops/ops-client.tsx @@ -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 }) { 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({
{entry.content ?? ''}
+ {canManageAcl && ( + + )} @@ -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} /> ))} @@ -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} /> ))} @@ -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} /> ))} diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index d2938b7..4653aa2 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -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 diff --git a/lib/actions/imports.ts b/lib/actions/imports.ts index ed104ca..761be74 100644 --- a/lib/actions/imports.ts +++ b/lib/actions/imports.ts @@ -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> = [] + 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 } } diff --git a/lib/actions/payments.ts b/lib/actions/payments.ts index a5c1903..660c88c 100644 --- a/lib/actions/payments.ts +++ b/lib/actions/payments.ts @@ -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> = [] + 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 } } diff --git a/lib/validations.ts b/lib/validations.ts index c543d53..6260ba4 100644 --- a/lib/validations.ts +++ b/lib/validations.ts @@ -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')