Skip to content

Commit faec125

Browse files
fix(admin): report a completed workspace move's true source and its credentials (#7290)
* fix(admin): report a completed workspace move's true source and its credentials The durable move payload records `sourceOrganizationId` as a tri-state: absent means the operation predates the field, an explicit `null` means the workspace came from a personal source. The payload parser collapsed both to `null`, so every reload or retry of a personal-source move claimed its source organization had failed to persist. The applied and reloaded responses also blanked the credential summary. Unlike the source impact, those rows are workspace-scoped and travel with the move untouched, so an admin who had just confirmed a move was told the workspace carried no secrets, environment variables, or BYOK keys. * test(admin): cover the deleted source organization and truncated credential counts `getSourceOrganization` was mocked inline in the module factory, so the reload branch that reports a recorded-but-deleted source organization could not be exercised at all. Hoist it like the other mocks and pin that third tri-state. Every credential fixture also reported zero dropped rows, so the applied and reloaded truncation records would not have caught a regression to the hardcoded zeros they replaced.
1 parent 553849a commit faec125

2 files changed

Lines changed: 412 additions & 67 deletions

File tree

apps/sim/lib/workspaces/admin-move.test.ts

Lines changed: 270 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ const {
4646
sendInvitationEmail,
4747
countPendingSeatInvitations,
4848
resolveSeatCapacity,
49+
collectWorkspaceCredentialSummary,
50+
getSourceOrganization,
4951
} = vi.hoisted(() => ({
5052
resolveMoveEntitlements: vi.fn(() =>
5153
Promise.resolve({
@@ -73,8 +75,49 @@ const {
7375
sendInvitationEmail: vi.fn(),
7476
countPendingSeatInvitations: vi.fn(() => Promise.resolve(0)),
7577
resolveSeatCapacity: vi.fn(() => Promise.resolve(10)),
78+
collectWorkspaceCredentialSummary: vi.fn(),
79+
getSourceOrganization: vi.fn(),
7680
}))
7781

82+
const SOURCE_ORGANIZATION = {
83+
id: 'org-source',
84+
name: 'Source',
85+
ownerId: 'source-owner',
86+
ownerName: 'Source Owner',
87+
ownerEmail: 'source-owner@example.com',
88+
}
89+
90+
const EMPTY_CREDENTIALS = {
91+
items: [] as Array<{
92+
id: string
93+
displayName: string
94+
type: string
95+
backedBySourceOrgMember: boolean
96+
}>,
97+
credentialGroupCount: 0,
98+
environmentVariableKeys: [] as string[],
99+
byokKeyCount: 0,
100+
truncatedCredentials: 0,
101+
truncatedEnvironmentVariableKeys: 0,
102+
}
103+
104+
const POPULATED_CREDENTIALS = {
105+
...EMPTY_CREDENTIALS,
106+
items: [
107+
{ id: 'credential-1', displayName: 'Slack', type: 'oauth', backedBySourceOrgMember: true },
108+
],
109+
credentialGroupCount: 1,
110+
environmentVariableKeys: ['OPENAI_API_KEY'],
111+
byokKeyCount: 2,
112+
}
113+
114+
/** A workspace whose secrets exceed the response bounds, so rows were dropped. */
115+
const TRUNCATED_CREDENTIALS = {
116+
...POPULATED_CREDENTIALS,
117+
truncatedCredentials: 3,
118+
truncatedEnvironmentVariableKeys: 7,
119+
}
120+
78121
vi.mock('@sim/audit', () => ({
79122
AuditAction: {
80123
WORKSPACE_UPDATED: 'workspace.updated',
@@ -121,16 +164,7 @@ vi.mock('@/lib/table/billing', () => ({ invalidateWorkspaceTableLimitsCache }))
121164
vi.mock('@/lib/workflows/custom-blocks/operations', () => ({ deleteCustomBlock }))
122165
vi.mock('@/lib/workspaces/admin-move-source-impact', () => ({
123166
cleanupSourceOrganizationArtifactsTx,
124-
collectWorkspaceCredentialSummary: vi.fn(() =>
125-
Promise.resolve({
126-
items: [],
127-
credentialGroupCount: 0,
128-
environmentVariableKeys: [],
129-
byokKeyCount: 0,
130-
truncatedCredentials: 0,
131-
truncatedEnvironmentVariableKeys: 0,
132-
})
133-
),
167+
collectWorkspaceCredentialSummary,
134168
countRetentionRulesForWorkspace: vi.fn(() => ({
135169
piiRedactionRules: 0,
136170
retentionOverrides: 0,
@@ -140,15 +174,7 @@ vi.mock('@/lib/workspaces/admin-move-source-impact', () => ({
140174
findRetainedCollaboratorCaps: vi.fn(() => Promise.resolve([])),
141175
findUnpublishableCustomBlocks,
142176
findSourceOrgCustomBlocksForWorkspace,
143-
getSourceOrganization: vi.fn(() =>
144-
Promise.resolve({
145-
id: 'org-source',
146-
name: 'Source',
147-
ownerId: 'source-owner',
148-
ownerName: 'Source Owner',
149-
ownerEmail: 'source-owner@example.com',
150-
})
151-
),
177+
getSourceOrganization,
152178
resolveMoveEntitlements,
153179
willBrandingChange: vi.fn(() => Promise.resolve(false)),
154180
}))
@@ -210,6 +236,30 @@ function queueMoveSelects(workspaceRow: Record<string, unknown>) {
210236
queueTableRows(organization, [destination])
211237
}
212238

239+
/**
240+
* The reload path reads the completed operation, then the workspace twice — the
241+
* applied-state check and the summary reload — and the destination once.
242+
*/
243+
function queueMoveOperationSelects(audit: Record<string, unknown>) {
244+
queueTableRows(outboxEvent, [
245+
{
246+
eventType: 'admin.workspace-move-operation',
247+
status: 'completed',
248+
payload: {
249+
request: {
250+
workspaceId: movedWorkspace.id,
251+
destinationOrganizationId: destination.id,
252+
expectedOwnerId: movedWorkspace.ownerId,
253+
},
254+
audit,
255+
},
256+
},
257+
])
258+
queueTableRows(workspace, [movedWorkspace])
259+
queueTableRows(workspace, [movedWorkspace])
260+
queueTableRows(organization, [destination])
261+
}
262+
213263
afterAll(resetDbChainMock)
214264

215265
beforeEach(() => {
@@ -227,6 +277,8 @@ beforeEach(() => {
227277
destinationIsEnterprise: false,
228278
capabilitiesLost: [],
229279
})
280+
collectWorkspaceCredentialSummary.mockResolvedValue(EMPTY_CREDENTIALS)
281+
getSourceOrganization.mockResolvedValue(SOURCE_ORGANIZATION)
230282
changeWorkspaceStoragePayerInTx.mockResolvedValue({
231283
billableBytes: 128,
232284
newPayer: { type: 'organization', id: destination.id },
@@ -788,6 +840,205 @@ describe('moveWorkspaceToOrganization retries', () => {
788840
)
789841
})
790842

843+
/**
844+
* A completed move records `sourceOrganizationId` even when it is `null`, so
845+
* a reload can tell "this workspace came from a personal source" apart from
846+
* "this operation predates the field". Collapsing the two made every reload
847+
* of a personal-source move claim its origin had failed to persist.
848+
*/
849+
it('does not warn about an unpersisted source for a move recorded as personal', async () => {
850+
queueMoveOperationSelects({
851+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
852+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
853+
newBillingOwnerId: destination.ownerId,
854+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
855+
sourceOrganizationId: null,
856+
})
857+
858+
const view = await getWorkspaceMoveOperation(
859+
movedWorkspace.id,
860+
destination.id,
861+
movedWorkspace.ownerId,
862+
'operation-1'
863+
)
864+
865+
expect(view.notices).toEqual([])
866+
expect(view.sourceOrganization).toBeNull()
867+
})
868+
869+
it('still warns when the payload never recorded a source organization', async () => {
870+
queueMoveOperationSelects({
871+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
872+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
873+
newBillingOwnerId: destination.ownerId,
874+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
875+
})
876+
877+
const view = await getWorkspaceMoveOperation(
878+
movedWorkspace.id,
879+
destination.id,
880+
movedWorkspace.ownerId,
881+
'operation-1'
882+
)
883+
884+
expect(view.notices).toEqual([
885+
'This move was recorded before the source organization was persisted, so it cannot be reported.',
886+
])
887+
})
888+
889+
it('reports the workspace credentials when a completed operation is reloaded', async () => {
890+
collectWorkspaceCredentialSummary.mockResolvedValueOnce(POPULATED_CREDENTIALS)
891+
queueMoveOperationSelects({
892+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
893+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
894+
newBillingOwnerId: destination.ownerId,
895+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
896+
sourceOrganizationId: 'org-source',
897+
})
898+
899+
const view = await getWorkspaceMoveOperation(
900+
movedWorkspace.id,
901+
destination.id,
902+
movedWorkspace.ownerId,
903+
'operation-1'
904+
)
905+
906+
/** Resolved against the recorded source, so `backedBySourceOrgMember` means something. */
907+
expect(collectWorkspaceCredentialSummary).toHaveBeenCalledWith(movedWorkspace.id, 'org-source')
908+
expect(view.credentials).toEqual(POPULATED_CREDENTIALS)
909+
})
910+
911+
/**
912+
* A recorded id whose organization has since been deleted is the third state:
913+
* the payload answered, but the answer can no longer be resolved to a name.
914+
*/
915+
it('distinguishes a deleted source organization from an unrecorded one', async () => {
916+
getSourceOrganization.mockResolvedValueOnce(null)
917+
queueMoveOperationSelects({
918+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
919+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
920+
newBillingOwnerId: destination.ownerId,
921+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
922+
sourceOrganizationId: 'org-source',
923+
})
924+
925+
const view = await getWorkspaceMoveOperation(
926+
movedWorkspace.id,
927+
destination.id,
928+
movedWorkspace.ownerId,
929+
'operation-1'
930+
)
931+
932+
expect(view.sourceOrganization).toBeNull()
933+
expect(view.notices).toEqual([
934+
'The organization this workspace came from has since been deleted, so it can no longer be named.',
935+
])
936+
})
937+
938+
it('reports the workspace credentials in the applied summary', async () => {
939+
queueMoveSelects(organizationWorkspace)
940+
collectWorkspaceCredentialSummary.mockResolvedValueOnce(POPULATED_CREDENTIALS)
941+
942+
const summary = await moveWorkspaceToOrganization({
943+
workspaceId: organizationWorkspace.id,
944+
destinationOrganizationId: destination.id,
945+
adminEmail: 'admin@sim.ai',
946+
durableOperationId: 'operation-1',
947+
})
948+
949+
/** The PRE-move organization: that is what `backedBySourceOrgMember` compares against. */
950+
expect(collectWorkspaceCredentialSummary).toHaveBeenCalledWith(
951+
organizationWorkspace.id,
952+
'org-source',
953+
expect.anything()
954+
)
955+
expect(summary.credentials).toEqual(POPULATED_CREDENTIALS)
956+
/** Nothing was dropped, so the review is complete and says nothing about truncation. */
957+
expect(summary.sourceOrganizationImpact.truncated).toBeNull()
958+
})
959+
960+
/**
961+
* The applied path used to hardcode these two counters to zero, which would
962+
* present a truncated credential list as a complete one.
963+
*/
964+
it('carries dropped credential counts into the applied truncation record', async () => {
965+
queueMoveSelects(organizationWorkspace)
966+
collectWorkspaceCredentialSummary.mockResolvedValueOnce(TRUNCATED_CREDENTIALS)
967+
968+
const summary = await moveWorkspaceToOrganization({
969+
workspaceId: organizationWorkspace.id,
970+
destinationOrganizationId: destination.id,
971+
adminEmail: 'admin@sim.ai',
972+
durableOperationId: 'operation-1',
973+
})
974+
975+
expect(summary.sourceOrganizationImpact.truncated).toMatchObject({
976+
credentials: 3,
977+
environmentVariableKeys: 7,
978+
})
979+
})
980+
981+
it('carries dropped credential counts into a reloaded truncation record', async () => {
982+
collectWorkspaceCredentialSummary.mockResolvedValueOnce(TRUNCATED_CREDENTIALS)
983+
queueMoveOperationSelects({
984+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
985+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
986+
newBillingOwnerId: destination.ownerId,
987+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
988+
sourceOrganizationId: 'org-source',
989+
})
990+
991+
const view = await getWorkspaceMoveOperation(
992+
movedWorkspace.id,
993+
destination.id,
994+
movedWorkspace.ownerId,
995+
'operation-1'
996+
)
997+
998+
expect(view.sourceOrganizationImpact.truncated).toMatchObject({
999+
credentials: 3,
1000+
environmentVariableKeys: 7,
1001+
})
1002+
})
1003+
1004+
it('reports the workspace credentials on a retry of a completed move', async () => {
1005+
queueMoveSelects(movedWorkspace)
1006+
queueTableRows(outboxEvent, [
1007+
{
1008+
eventType: 'admin.workspace-move-operation',
1009+
status: 'completed',
1010+
payload: {
1011+
request: {
1012+
workspaceId: movedWorkspace.id,
1013+
destinationOrganizationId: destination.id,
1014+
expectedOwnerId: movedWorkspace.ownerId,
1015+
},
1016+
audit: {
1017+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
1018+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
1019+
newBillingOwnerId: destination.ownerId,
1020+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
1021+
sourceOrganizationId: null,
1022+
},
1023+
},
1024+
},
1025+
])
1026+
collectWorkspaceCredentialSummary.mockResolvedValueOnce(POPULATED_CREDENTIALS)
1027+
1028+
const summary = await moveWorkspaceToOrganization({
1029+
workspaceId: movedWorkspace.id,
1030+
destinationOrganizationId: destination.id,
1031+
adminEmail: 'admin@sim.ai',
1032+
expectedOwnerId: movedWorkspace.ownerId,
1033+
auditOperationId: 'operation-1',
1034+
operationCorrelationId: 'operation-1',
1035+
durableOperationId: 'operation-1',
1036+
})
1037+
1038+
expect(summary.credentials).toEqual(POPULATED_CREDENTIALS)
1039+
expect(summary.notices).toEqual([])
1040+
})
1041+
7911042
it('takes shared advisory locks before the workspace row lock and payer mutation', async () => {
7921043
queueMoveSelects(personalWorkspace)
7931044

0 commit comments

Comments
 (0)