fix(people): consistent profile reads, capability-aware navigation, unambiguous email matching, stricter dates, guarded workbook parsing

Author: waleedlatif1

apps/sim/app/o/[organizationId]/layout.tsx

@@ -12,7 +12,7 @@ import { isDashboardsEnabled } from '@/lib/dashboards/feature-flag'
 import { isMothershipModelSelectorEnabled } from '@/lib/mothership/feature-flags'
 import { organizationRoutes, WORKSPACE_SETTINGS_PATH } from '@/lib/navigation/paths'
 import { getOrganizationSurfaceContext } from '@/lib/organizations/surface'
-import { isPeopleEnabled } from '@/lib/people/feature-flag'
+import { canBrowsePeople } from '@/lib/people/feature-flag'
 import { isProjectsEnabled } from '@/lib/projects/rollout.server'
 import { isSimLinksEnabled } from '@/lib/sim-links/feature-flag'
 import { isWorkflowTestsEnabled } from '@/lib/workflow-tests/feature-flag'
@@ -65,7 +65,8 @@ export default async function OrganizationLayout({
   if (!context) {
     return <OrganizationAccessDenied />
   }
-  if (!context.mothershipAvailable && !context.searchAccess.memberScoped)
+  const peopleEnabled = await canBrowsePeople(organizationId, context.viewer.role)
+  if (!context.mothershipAvailable && !context.searchAccess.memberScoped && !peopleEnabled)
     redirect(WORKSPACE_SETTINGS_PATH)
 
   const [
@@ -78,7 +79,6 @@ export default async function OrganizationLayout({
     changelogEnabled,
     projectsEnabled,
     simLinksEnabled,
-    peopleEnabled,
   ] = await Promise.all([
     prefetchOrganizationSidebar(
       queryClient,
@@ -97,7 +97,6 @@ export default async function OrganizationLayout({
     isChangelogEnabled(organizationId),
     isProjectsEnabled(),
     isSimLinksEnabled(organizationId),
-    isPeopleEnabled(organizationId),
   ])
   const initialSidebarCollapsed = cookieStore.get('sidebar_collapsed')?.value === '1'
 

apps/sim/app/o/[organizationId]/page.tsx

@@ -2,6 +2,7 @@ import { notFound, redirect } from 'next/navigation'
 import { getSession } from '@/lib/auth'
 import { organizationRoutes, WORKSPACE_SETTINGS_PATH } from '@/lib/navigation/paths'
 import { getOrganizationSurfaceContext } from '@/lib/organizations/surface'
+import { canBrowsePeople } from '@/lib/people/feature-flag'
 import { buildAuthCrossLink } from '@/app/(auth)/auth-redirect'
 
 export default async function OrganizationPage({
@@ -17,11 +18,11 @@ export default async function OrganizationPage({
   }
   const context = await getOrganizationSurfaceContext(organizationId, session.user.id)
   if (!context) notFound()
+  if (context.mothershipAvailable && context.canBuild) redirect(routes.home)
+  if (context.searchAccess.memberScoped) redirect(routes.search)
   redirect(
-    context.mothershipAvailable && context.canBuild
-      ? routes.home
-      : context.searchAccess.memberScoped
-        ? routes.search
-        : WORKSPACE_SETTINGS_PATH
+    (await canBrowsePeople(organizationId, context.viewer.role))
+      ? routes.people
+      : WORKSPACE_SETTINGS_PATH
   )
 }

apps/sim/app/o/[organizationId]/people/people-import-modal.tsx

@@ -15,6 +15,7 @@ import {
 } from '@sim/emcn'
 import { getErrorMessage } from '@sim/utils/errors'
 import { PEOPLE_IMPORT_MAX_ROWS, type PeopleImportResult } from '@/lib/api/contracts/people'
+import { assertOoxmlPreviewWithinLimits } from '@/lib/file-parsers/ooxml-preview-guard'
 import { saveBlob } from '@/lib/uploads/client/download'
 import { useImportPeople } from '@/hooks/queries/people'
 
@@ -38,8 +39,10 @@ interface PeopleSheet {
  */
 async function readPeopleSheet(file: File): Promise<PeopleSheet> {
   if (file.size > MAX_FILE_BYTES) throw new Error('Choose a file under 5 MB')
+  const data = new Uint8Array(await file.arrayBuffer())
+  if (/\.xlsx$/i.test(file.name)) await assertOoxmlPreviewWithinLimits(data)
   const XLSX = await import('xlsx')
-  const workbook = XLSX.read(await file.arrayBuffer(), {
+  const workbook = XLSX.read(data, {
     type: 'array',
     raw: /\.(csv|tsv)$/i.test(file.name),
     dense: true,
@@ -91,7 +94,11 @@ function plural(count: number, noun: string): string {
 }
 
 /** The preview's issues in file order, with a count of any past the ones listed. */
-function ImportIssueList({ result }: { result: PeopleImportResult }) {
+interface ImportIssueListProps {
+  result: PeopleImportResult
+}
+
+function ImportIssueList({ result }: ImportIssueListProps) {
   const listRef = useRef<HTMLUListElement>(null)
   const edges = useScrollEdges(listRef)
   const unlisted = result.skipped + result.warnings - result.issues.length
@@ -108,9 +115,9 @@ function ImportIssueList({ result }: { result: PeopleImportResult }) {
         <li key={`${issue.row}-${issue.message}`} className='py-0.5'>
           <span className='text-[var(--text-muted)]'>Row {issue.row}</span>{' '}
           <span
-            className={
+            className={cn(
               issue.severity === 'skipped' ? 'text-[var(--text-error)]' : 'text-[var(--text-body)]'
-            }
+            )}
           >
             {issue.email ? `${issue.email}: ` : ''}
             {issue.message}

apps/sim/lib/people/application/use-cases.ts

@@ -56,16 +56,23 @@ export const listDepartments = defineAuthorizedOrganizationUseCase({
     repository.listDepartments(input.organizationId),
 })
 
-/** One person with the managers above them and the people who report to them. */
+/**
+ * One person with the managers above them and the people who report to them, read from one
+ * snapshot so a reporting-line edit made meanwhile can't leave the three disagreeing.
+ */
 export const getPerson = defineAuthorizedOrganizationUseCase({
   operation: peopleOperations.get,
   authorizeResource: authorizeDirectory,
   async execute({ input }: OrganizationUseCaseContext<PersonInput>) {
-    const [person, chain, reports] = await Promise.all([
-      repository.getPerson(db, input.organizationId, input.memberId),
-      repository.getReportingChain(input.organizationId, input.memberId),
-      repository.listDirectReports(input.organizationId, input.memberId),
-    ])
+    const { organizationId, memberId } = input
+    const { person, chain, reports } = await db.transaction(
+      async (tx) => ({
+        person: await repository.getPerson(tx, organizationId, memberId),
+        chain: await repository.getReportingChain(tx, organizationId, memberId),
+        reports: await repository.listDirectReports(tx, organizationId, memberId),
+      }),
+      { isolationLevel: 'repeatable read', accessMode: 'read only' }
+    )
     if (!person) throw new OrchestrationError('not_found', 'Person not found')
     return { person, chain, reports }
   },

apps/sim/lib/people/feature-flag.ts

@@ -1,5 +1,7 @@
+import { isOrgAdminRole } from '@sim/platform-authz/workspace'
 import { isFeatureEnabled } from '@/lib/core/config/feature-flags'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
+import { isOrganizationCapabilityWithheld } from '@/lib/permission-groups/capability-assertions'
 
 export function isPeopleEnabled(organizationId: string): Promise<boolean> {
   return isFeatureEnabled('people', { orgId: organizationId })
@@ -10,3 +12,16 @@ export async function requirePeopleEnabled(organizationId: string): Promise<void
     throw new OrchestrationError('forbidden', 'People is not enabled for this organization')
   }
 }
+
+/**
+ * Whether the People page is on for `organizationId` and its directory open to a member with
+ * `role`: administrators always see it; other members unless their permission group withholds
+ * `organization.member_directory`, the capability its reads enforce.
+ */
+export async function canBrowsePeople(organizationId: string, role: string): Promise<boolean> {
+  if (!(await isPeopleEnabled(organizationId))) return false
+  return (
+    isOrgAdminRole(role) ||
+    !(await isOrganizationCapabilityWithheld(organizationId, 'organization.member_directory'))
+  )
+}

apps/sim/lib/people/import.test.ts

@@ -215,6 +215,7 @@ describe('planPeopleImport', () => {
     ['2021/3/4', '2021-03-04'],
     ['3/4/2021', '2021-03-04'],
     ['2021-03-04T09:00:00Z', '2021-03-04'],
+    ['2021-03-04 09:00', '2021-03-04'],
   ])('reads the start date %s', (value, startDate) => {
     const result = plan(
       ['email', 'start date'],
@@ -223,7 +224,7 @@ describe('planPeopleImport', () => {
     expect(result.changes).toEqual([{ memberId: 'm-ada', profile: profile({ startDate }) }])
   })
 
-  it.each(['2021-02-30', '13/45/2021', '04.03.2021', 'March 4', '1/2/21'])(
+  it.each(['2021-02-30', '13/45/2021', '04.03.2021', 'March 4', '1/2/21', '2021-03-04Tsoon'])(
     'skips the row with the unreadable start date %s instead of guessing',
     (value) => {
       const result = plan(
@@ -338,6 +339,23 @@ describe('planPeopleImport', () => {
     ])
   })
 
+  it('refuses an address two members share once case and spaces are ignored', () => {
+    const twin = { memberId: 'm-twin', email: ' ADA@example.com', profile: null, active: true }
+    const result = plan(
+      ['email', 'title', 'manager email'],
+      [
+        { email: 'ada@example.com', title: 'A', 'manager email': '' },
+        { email: 'bo@example.com', title: 'B', 'manager email': 'Ada@Example.com' },
+      ],
+      [ada, twin, bo]
+    )
+    expect(result.changes).toEqual([{ memberId: 'm-bo', profile: profile({ title: 'B' }) }])
+    expect(result.issues).toEqual([
+      expect.objectContaining({ row: 2, severity: 'skipped' }),
+      expect.objectContaining({ row: 3, severity: 'warning' }),
+    ])
+  })
+
   it('combines location parts, leaving out the blank ones', () => {
     const result = plan(
       ['email', 'city', 'state', 'country'],

apps/sim/lib/people/import.ts

@@ -1,3 +1,4 @@
+import { normalizeEmail } from '@sim/utils/string'
 import { z } from 'zod'
 import {
   type MemberProfileFields,
@@ -179,9 +180,17 @@ export function planPeopleImport({
   rows: readonly Record<string, string>[]
   members: readonly PeopleImportMember[]
 }): PeopleImportPlan {
-  const byEmail = new Map(
-    members.filter((member) => member.active).map((member) => [member.email.toLowerCase(), member])
-  )
+  const byEmail = new Map<string, PeopleImportMember | 'ambiguous'>()
+  for (const member of members) {
+    if (!member.active) continue
+    const key = normalizeEmail(member.email)
+    byEmail.set(key, byEmail.has(key) ? 'ambiguous' : member)
+  }
+  /** The one active member an address names; `undefined` when none or several do. */
+  const memberWith = (email: string) => {
+    const found = byEmail.get(email)
+    return found === 'ambiguous' ? undefined : found
+  }
   const managers = new Map(
     members.map((member) => [member.memberId, member.profile?.managerMemberId ?? null])
   )
@@ -193,14 +202,17 @@ export function planPeopleImport({
   rows.forEach((row, index) => {
     const rowNumber = index + 2
     if (Object.values(row).every((value) => !value?.trim())) return
-    const email = cell(row, columns.email).toLowerCase()
+    const email = normalizeEmail(cell(row, columns.email))
     const skip = (message: string) =>
       issues.push({ row: rowNumber, email: email || null, severity: 'skipped', message })
 
     if (!emailSchema.safeParse(email).success) return skip('Enter a valid email address')
     const earlierRow = acceptedRowByEmail.get(email)
     if (earlierRow !== undefined) return skip(`Already listed in row ${earlierRow}`)
-    const member = byEmail.get(email)
+    if (byEmail.get(email) === 'ambiguous') {
+      return skip('More than one member has this email, so the row was not applied')
+    }
+    const member = memberWith(email)
     if (!member) return skip('Not a member of this organization')
 
     const fields: Partial<MemberProfileFields> = {}
@@ -228,8 +240,8 @@ export function planPeopleImport({
 
     let warning: string | null = null
     if (columns.managerEmail !== undefined) {
-      const managerEmail = cell(row, columns.managerEmail).toLowerCase()
-      const manager = managerEmail ? byEmail.get(managerEmail) : null
+      const managerEmail = normalizeEmail(cell(row, columns.managerEmail))
+      const manager = managerEmail ? memberWith(managerEmail) : null
       if (manager?.memberId === member.memberId) return skip("A person can't be their own manager")
       if (!managerEmail) {
         fields.managerMemberId = null

apps/sim/lib/people/repository.ts

@@ -176,10 +176,11 @@ export async function getPerson(
  * ends at a manager who is suspended or outside the organization, and never revisits anyone.
  */
 export async function getReportingChain(
+  executor: DbOrTx,
   organizationId: string,
   memberId: string
 ): Promise<PersonSummary[]> {
-  const rows = await db.execute<PersonSummary>(sql`
+  const rows = await executor.execute<PersonSummary>(sql`
     WITH RECURSIVE chain(member_id, depth, path) AS (
       SELECT m.id, 1, ARRAY[p.member_id, m.id]
       FROM member_profile p
@@ -206,10 +207,11 @@ export async function getReportingChain(
 
 /** The active people who report to `memberId`, by name. */
 export async function listDirectReports(
+  executor: DbOrTx,
   organizationId: string,
   memberId: string
 ): Promise<PersonSummary[]> {
-  return db
+  return executor
     .select(summaryColumns)
     .from(member)
     .innerJoin(user, eq(user.id, member.userId))

apps/sim/lib/people/validation.ts

@@ -2,7 +2,9 @@ import { z } from 'zod'
 
 export const PROFILE_TEXT_MAX_LENGTH = 200
 
-const ISO_DATE = /^(\d{4})[-/](\d{1,2})[-/](\d{1,2})(?:T.*)?$/
+/** A date, optionally with a time of day and offset, which are dropped. */
+const ISO_DATE =
+  /^(\d{4})[-/](\d{1,2})[-/](\d{1,2})(?:[T ]\d{2}:\d{2}(?::\d{2}(?:\.\d+)?)?(?:Z|[+-]\d{2}:?\d{2})?)?$/
 const US_DATE = /^(\d{1,2})\/(\d{1,2})\/(\d{4})$/
 
 function calendarDate(year: number, month: number, day: number): string | null {

apps/sim/scripts/test-people-e2e.ts

@@ -320,7 +320,7 @@ async function run() {
         from audit_log where resource_id = ${orgId} and action in ${sql(Object.keys(expected))}
         group by action`
       counts = Object.fromEntries(rows.map((row) => [row.action, row.count]))
-      if (counts['org_people.imported'] === expected['org_people.imported']) break
+      if (Object.entries(expected).every(([action, count]) => counts[action] === count)) break
       await sleep(250)
     }
     assert.deepEqual(counts, expected, 'Previews are not audited; each apply and edit is')