Skip to content

Commit 3261cf5

Browse files
authored
fix(release): keep Entra sync running through mid-read drift and tighten release-audit findings (#9649)
* fix(release): harden Entra sync against mid-read drift, keep loaded People views on refresh errors, refuse credentialed citation URLs * improvement(release): reuse shared helpers and name the People sync checkpoint lock * test(people): pin the same-batch manager tie-break and document it
1 parent e1a45a0 commit 3261cf5

14 files changed

Lines changed: 97 additions & 59 deletions

File tree

‎apps/sim/app/o/[organizationId]/people/[memberId]/person.tsx‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,11 @@ export function Person({ organizationId, memberId }: PersonProps) {
100100
const [editing, setEditing] = useState(false)
101101
const routes = organizationRoutes(organizationId)
102102
const originQuery = serializePeopleView({ ...origin, view: origin.view ?? 'directory' })
103-
const data = query.isError ? undefined : query.data
103+
const notFound = isApiClientError(query.error) && query.error.status === 404
104+
const refused = isApiClientError(query.error) && query.error.status < 500
105+
// A transient refresh failure keeps the loaded profile and any edit open on it; a refusal does not.
106+
const loadError = query.data && !refused ? null : query.error
107+
const data = loadError ? undefined : query.data
104108

105109
const chartHref = serializePeopleView(routes.people, {
106110
...origin,
@@ -129,14 +133,14 @@ export function Person({ organizationId, memberId }: PersonProps) {
129133
) : undefined
130134
}
131135
>
132-
{query.error ? (
133-
isApiClientError(query.error) && query.error.status === 404 ? (
136+
{loadError ? (
137+
notFound ? (
134138
<SettingsEmptyState variant='inline'>
135139
This person isn't in your organization.
136140
</SettingsEmptyState>
137141
) : (
138142
<SettingsQueryErrorState
139-
error={query.error}
143+
error={loadError}
140144
fallback='Could not load this person'
141145
isRetrying={query.isFetching}
142146
onRetry={() => void query.refetch()}

‎apps/sim/app/o/[organizationId]/people/directory-sync-form.tsx‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
type PeopleSyncStatus,
1717
peopleSyncConfigSchema,
1818
} from '@/lib/api/contracts/people-sync'
19+
import { flattenFieldErrors } from '@/lib/api/contracts/primitives'
1920
import { organizationRoutes } from '@/lib/navigation/paths'
2021
import { useOrganizationContext } from '@/app/o/[organizationId]/providers/organization-provider'
2122
import { useScimConnection } from '@/ee/scim/hooks/scim'
@@ -45,7 +46,9 @@ export function DirectorySyncForm({
4546
const [tenantId, setTenantId] = useState(current?.tenantId ?? '')
4647
const [clientId, setClientId] = useState(current?.clientId ?? '')
4748
const [secret, setSecret] = useState('')
48-
const [fieldErrors, setFieldErrors] = useState<Record<string, string>>({})
49+
const [fieldErrors, setFieldErrors] = useState<
50+
Partial<Record<'domain' | 'token' | 'tenantId' | 'clientId' | 'clientSecret', string>>
51+
>({})
4952
const { settingsFeatures } = useOrganizationContext()
5053
const provisioning = useScimConnection(organizationId, source === 'scim')
5154
const scimReady = provisioning.isSuccess && provisioning.data.connection?.status === 'active'
@@ -66,11 +69,7 @@ export function DirectorySyncForm({
6669
: { source }
6770
)
6871
if (!config.success) {
69-
setFieldErrors(
70-
Object.fromEntries(
71-
config.error.issues.map((issue) => [String(issue.path[0]), issue.message])
72-
)
73-
)
72+
setFieldErrors(flattenFieldErrors(config.error))
7473
return
7574
}
7675
setFieldErrors({})

‎apps/sim/app/o/[organizationId]/people/people-chart.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@ export function PeopleChart({ organizationId, viewSwitch, onImport, onSync }: Pe
102102
const { focus, page } = origin
103103
const size = useIsMobile() ? 1 : 4
104104
const chart = usePeopleChart(organizationId, focus, Math.max(1, Math.min(page, 1_000_000)), size)
105-
const branch = chart.isError ? undefined : chart.data
105+
const notFound = isApiClientError(chart.error) && chart.error.status === 404
106+
const refused = isApiClientError(chart.error) && chart.error.status < 500
107+
// A transient refresh failure keeps the chart on screen; a refusal replaces it.
108+
const branch = refused ? undefined : chart.data
106109
const profileQuery = serializePeopleView({
107110
...origin,
108111
view: 'chart',
@@ -264,7 +267,7 @@ export function PeopleChart({ organizationId, viewSwitch, onImport, onSync }: Pe
264267
</div>
265268
</>
266269
) : chart.error ? (
267-
isApiClientError(chart.error) && chart.error.status === 404 ? (
270+
notFound ? (
268271
<EmptyState
269272
title='Person unavailable'
270273
description='This person is no longer in your organization’s directory.'

‎apps/sim/app/o/[organizationId]/people/people-directory.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ export function PeopleDirectory({ organizationId }: PeopleProps) {
190190
rows={rows}
191191
onRowClick={(memberId) => router.push(profileHref(memberId))}
192192
onLoadMore={() => void people.fetchNextPage()}
193-
hasMore={people.hasNextPage}
193+
hasMore={people.hasNextPage && !changingFilter}
194194
isLoadingMore={people.isFetchingNextPage}
195195
isFetching={people.isFetching}
196196
loadMoreFailed={people.isFetchNextPageError}

‎apps/sim/app/o/[organizationId]/search/search.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ function OrganizationSearchContent({ userId, chatId }: OrganizationSearchProps)
167167
const { recordQuery } = useSearchHistoryActions()
168168
const router = useRouter()
169169
const pathname = usePathname()
170-
const hasSearchParams = useSearchParams().size > 0
170+
const hasSearchParams = useSearchParams().toString() !== ''
171171
const routes = organizationRoutes(organization.id)
172172
const hasConversation = Boolean(chatId) || pathname.startsWith(`${routes.search}/chat/`)
173173
const queryClient = useQueryClient()

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/resolve-citations.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,13 @@ describe('evidence-linked citations', () => {
160160
true
161161
)
162162
expect(persisted.sources).toEqual(live.sources)
163+
const disguised = {
164+
...webOutput,
165+
results: [{ ...webOutput.results[0], url: 'https://docs.example.test@attacker.test/cli' }],
166+
}
167+
expect(
168+
resolveMessageCitations(webBlocks('success', true, disguised), '', true).sources
169+
).toEqual([])
163170
expect(
164171
resolveMessageCitations(webBlocks('error', false, webOutput), '', true).blocks[1].content
165172
).toBe('Install it ')

‎apps/sim/lib/mcp/utils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,8 @@ export function categorizeMcpServerError(
189189
}
190190

191191
/**
192-
* Enhanced error categorization for more specific HTTP status codes.
193-
* Returns safe, generic messages to prevent leaking internal details.
192+
* HTTP status and safe, generic message for any error: Sim's own MCP error classes
193+
* first, then message matching for SDK and third-party errors.
194194
*/
195195
export function categorizeError(error: unknown): { message: string; status: number } {
196196
if (!(error instanceof Error)) {

‎apps/sim/lib/mothership/chat/citation-evidence.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ type RetrievalCitationSource = {
1414
snippet?: string
1515
}
1616

17-
/** Tools whose successful results supply sources: connected-source retrieval and public web research. */
1817
const WEB_TOOLS = new Set(['web_search', 'web_fetch'])
18+
/** Tools whose successful results supply sources: connected-source retrieval and public web research. */
1919
const CITATION_TOOLS = new Set(['search_workspace', 'read_document', ...WEB_TOOLS])
2020

2121
/** Whether a successful result of this tool may supply source destinations. */
@@ -80,7 +80,9 @@ export function collectRetrievalCitationEvidence(blocks: readonly RetrievalCitat
8080
continue
8181
try {
8282
const url = new URL(result.citationUrl)
83+
// A web page can steer a fetched URL, so `https://trusted.example@other.example` never cites.
8384
if (url.protocol !== 'https:' && url.protocol !== 'http:') continue
85+
if (url.username || url.password) continue
8486
} catch {
8587
continue
8688
}

‎apps/sim/lib/people/sync/entra.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ const reportsResponse = z.object({
4444
'@odata.nextLink': z.string().min(1).max(MAX_NEXT_LINK_LENGTH).optional(),
4545
})
4646

47+
const deletedUserResponse = z.object({
48+
error: z.object({ code: z.literal('Request_ResourceNotFound') }),
49+
})
50+
51+
/** A user deleted after the users phase has no reports left to read; any other 404 still fails. */
52+
function isDeletedUser(status: number, body: unknown) {
53+
return status === 404 && deletedUserResponse.safeParse(body).success
54+
}
55+
4756
/** App-only Graph tokens cannot read /manager; each user's /directReports supports User.Read.All. */
4857
async function readReportPages(
4958
items: readonly DirectoryReportRequest[],
@@ -101,6 +110,8 @@ async function readReportPages(
101110
const timing = { get: (name: string) => headers.get(name) ?? null }
102111
const retryAfterMs = directoryRetryDelay(response.status, timing)
103112
if (retryAfterMs !== null) return { managerId: page.managerId, retryAfterMs }
113+
if (isDeletedUser(response.status, response.body))
114+
return { managerId: page.managerId, reportIds: [], next: null }
104115
requireDirectorySuccess(response.status, timing)
105116
const parsedReports = reportsResponse.safeParse(response.body)
106117
if (!parsedReports.success)

‎apps/sim/lib/people/sync/large-directory.integration.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ it('rotates credentials without bypassing a provider cooldown or publishing the
312312
expect(rows).toEqual([{ id: director, title: 'President' }])
313313
})
314314

315-
it('resumes a large Entra hierarchy without replaying successful batch siblings or creating accounts', async () => {
315+
it('resumes a large Entra hierarchy past a user deleted mid-read without replaying successful batch siblings or creating accounts', async () => {
316316
const total = 10007
317317
const tenantId = generateId()
318318
const graphConfig = {
@@ -378,6 +378,12 @@ it('resumes a large Entra hierarchy without replaying successful batch siblings
378378
body: { error: graphConfig.clientSecret },
379379
}
380380
}
381+
if (managerId === employee(2))
382+
return {
383+
id: request.id,
384+
status: 404,
385+
body: { error: { code: 'Request_ResourceNotFound' } },
386+
}
381387
if (managerId !== lead) return { id: request.id, status: 200, body: { value: [] } }
382388
const offset = Number(target.searchParams.get('$skiptoken') || 0)
383389
const length = Math.min(1000, total - 1 - offset)

0 commit comments

Comments
 (0)