Skip to content

Commit e824f38

Browse files
committed
fix: address review — export audit timing, copilot delete signal, group reset
- Table export (Cursor MED): audit fires before streaming begins, not after controller.close(), so a mid-stream failure still records the partial export. - deleteCopilotFile (Greptile P1): throw when storage accounting can't be settled instead of silently returning, so callers can detect the file was not deleted. - workspace-scope-sync (Cursor MED): only resetGroups() once workspace metadata is loaded (activeWorkspace present), so a team workspace doesn't transiently lose its org group while organizationId is still null during load.
1 parent fe36c68 commit e824f38

3 files changed

Lines changed: 42 additions & 32 deletions

File tree

apps/sim/app/api/table/[tableId]/export/route.ts

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,29 @@ export const GET = withRouteHandler(async (request: NextRequest, { params }: Rou
5656
const safeName = sanitizeFilename(table.name)
5757
const filename = `${safeName}.${format}`
5858

59+
// Record the export before streaming begins. Rows leave the server incrementally,
60+
// so a mid-stream failure would still exfiltrate partial data — auditing here
61+
// (after access is granted) guarantees every authorized export is recorded.
62+
recordAudit({
63+
workspaceId: table.workspaceId ?? null,
64+
actorId: userId,
65+
action: AuditAction.TABLE_EXPORTED,
66+
resourceType: AuditResourceType.TABLE,
67+
resourceId: tableId,
68+
resourceName: table.name,
69+
description: `Exported table "${table.name}" as ${format.toUpperCase()}`,
70+
metadata: { format, rowCount: table.rowCount },
71+
request,
72+
})
73+
if (table.workspaceId) {
74+
captureServerEvent(
75+
userId,
76+
'table_exported',
77+
{ table_id: tableId, workspace_id: table.workspaceId },
78+
{ groups: { workspace: table.workspaceId } }
79+
)
80+
}
81+
5982
const stream = new ReadableStream<Uint8Array>({
6083
async start(controller) {
6184
const encoder = new TextEncoder()
@@ -101,26 +124,6 @@ export const GET = withRouteHandler(async (request: NextRequest, { params }: Rou
101124
format,
102125
rowCount: table.rowCount,
103126
})
104-
105-
recordAudit({
106-
workspaceId: table.workspaceId ?? null,
107-
actorId: userId,
108-
action: AuditAction.TABLE_EXPORTED,
109-
resourceType: AuditResourceType.TABLE,
110-
resourceId: tableId,
111-
resourceName: table.name,
112-
description: `Exported table "${table.name}" as ${format.toUpperCase()}`,
113-
metadata: { format, rowCount: table.rowCount },
114-
request,
115-
})
116-
if (table.workspaceId) {
117-
captureServerEvent(
118-
userId,
119-
'table_exported',
120-
{ table_id: tableId, workspace_id: table.workspaceId },
121-
{ groups: { workspace: table.workspaceId } }
122-
)
123-
}
124127
} catch (err) {
125128
logger.error(`[${requestId}] Export failed for table ${tableId}`, err)
126129
controller.error(err)

apps/sim/app/workspace/[workspaceId]/providers/workspace-scope-sync.tsx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,17 @@ export function WorkspaceScopeSync() {
2424
if (!workspaceId) return
2525
if (organizationId) {
2626
posthog?.group('organization', organizationId)
27-
} else {
28-
// A personal workspace has no org — drop any organization group carried
29-
// over from a previously-active team workspace so later events don't keep
30-
// rolling up under it. resetGroups clears all groups, so the workspace
31-
// group is re-applied immediately below.
27+
} else if (activeWorkspace) {
28+
// Metadata is loaded and this workspace genuinely has no org — drop any
29+
// organization group carried over from a previously-active team workspace.
30+
// While metadata is still loading, activeWorkspace is undefined and
31+
// organizationId is transiently null, so resetting here would briefly strip
32+
// a team workspace's org group. resetGroups clears all groups, so the
33+
// workspace group is re-applied immediately below.
3234
posthog?.resetGroups()
3335
}
3436
posthog?.group('workspace', workspaceId, workspaceName ? { name: workspaceName } : undefined)
35-
}, [posthog, workspaceId, workspaceName, organizationId])
37+
}, [posthog, workspaceId, workspaceName, organizationId, activeWorkspace])
3638

3739
useEffect(() => {
3840
if (!workspaceId || hydrationWorkspaceId === workspaceId) {

apps/sim/lib/uploads/contexts/copilot/copilot-file-manager.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,16 @@ export async function deleteCopilotFile(key: string): Promise<void> {
280280
}
281281
}
282282

283-
if (released) {
284-
await deleteFile({
285-
key,
286-
context: 'copilot',
287-
})
288-
logger.info(`Successfully deleted copilot file: ${key}`)
283+
// Accounting couldn't be settled (read or release failed). The file is left
284+
// fully intact; throw so the caller knows the deletion did not happen and can
285+
// retry, rather than silently reporting success.
286+
if (!released) {
287+
throw new Error(`Copilot file deletion aborted; storage accounting failed for key: ${key}`)
289288
}
289+
290+
await deleteFile({
291+
key,
292+
context: 'copilot',
293+
})
294+
logger.info(`Successfully deleted copilot file: ${key}`)
290295
}

0 commit comments

Comments
 (0)