Skip to content

Commit 56835de

Browse files
committed
fix(observability): audit file/credential egress only on success
Address Cursor Bugbot review: - GET OAuth token: emit CREDENTIAL_ACCESSED/credential_used after refreshTokenIfNeeded succeeds (matches POST), not before. - File export: audit on each actual success exit via a shared helper — including the non-markdown serve redirect (previously unaudited) and after the zip is generated (previously before asset fetch).
1 parent 9c4c342 commit 56835de

2 files changed

Lines changed: 65 additions & 55 deletions

File tree

apps/sim/app/api/auth/oauth/token/route.ts

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -332,32 +332,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
332332
}
333333

334334
const actorId = authz.requesterUserId
335-
if (actorId) {
336-
const workspaceId = authz.workspaceId ?? null
337-
recordAudit({
338-
workspaceId,
339-
actorId,
340-
action: AuditAction.CREDENTIAL_ACCESSED,
341-
resourceType: AuditResourceType.CREDENTIAL,
342-
resourceId: resolvedCredentialId,
343-
description: `Accessed OAuth credential for provider ${credential.providerId}`,
344-
metadata: {
345-
provider: credential.providerId,
346-
credentialType: 'oauth',
347-
},
348-
request,
349-
})
350-
captureServerEvent(
351-
actorId,
352-
'credential_used',
353-
{
354-
credential_type: 'oauth',
355-
provider_id: credential.providerId,
356-
...(workspaceId ? { workspace_id: workspaceId } : {}),
357-
},
358-
workspaceId ? { groups: { workspace: workspaceId } } : undefined
359-
)
360-
}
335+
const workspaceId = authz.workspaceId ?? null
361336

362337
try {
363338
const { accessToken } = await refreshTokenIfNeeded(
@@ -366,6 +341,34 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
366341
resolvedCredentialId
367342
)
368343

344+
// Emitted only after the token is successfully resolved, so a failed
345+
// refresh never records a spurious credential access.
346+
if (actorId) {
347+
recordAudit({
348+
workspaceId,
349+
actorId,
350+
action: AuditAction.CREDENTIAL_ACCESSED,
351+
resourceType: AuditResourceType.CREDENTIAL,
352+
resourceId: resolvedCredentialId,
353+
description: `Accessed OAuth credential for provider ${credential.providerId}`,
354+
metadata: {
355+
provider: credential.providerId,
356+
credentialType: 'oauth',
357+
},
358+
request,
359+
})
360+
captureServerEvent(
361+
actorId,
362+
'credential_used',
363+
{
364+
credential_type: 'oauth',
365+
provider_id: credential.providerId,
366+
...(workspaceId ? { workspace_id: workspaceId } : {}),
367+
},
368+
workspaceId ? { groups: { workspace: workspaceId } } : undefined
369+
)
370+
}
371+
369372
// For Salesforce, extract instanceUrl from the scope field
370373
let instanceUrl: string | undefined
371374
if (credential.providerId === 'salesforce' && credential.scope) {

apps/sim/app/api/files/export/[id]/route.ts

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,43 @@ export const GET = withRouteHandler(
7070
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
7171
}
7272

73+
// Records the egress only once the export response is actually produced, so a
74+
// mid-export failure never logs a download that never happened. Covers every
75+
// exit (redirect to serve, plain markdown, and the bundled zip).
76+
const auditExport = (format: 'file' | 'markdown' | 'zip', assetCount: number) => {
77+
recordAudit({
78+
workspaceId: record.workspaceId ?? null,
79+
actorId: userId,
80+
action: AuditAction.FILE_DOWNLOADED,
81+
resourceType: AuditResourceType.FILE,
82+
resourceId: record.id,
83+
resourceName: record.originalName,
84+
description: `Exported file "${record.originalName}"`,
85+
metadata: {
86+
fileId: record.id,
87+
fileName: record.originalName,
88+
bytes: record.size,
89+
format,
90+
assetCount,
91+
},
92+
request,
93+
})
94+
captureServerEvent(
95+
userId,
96+
'file_downloaded',
97+
{
98+
workspace_id: record.workspaceId ?? '',
99+
is_bulk: assetCount > 0,
100+
file_count: 1 + assetCount,
101+
},
102+
record.workspaceId ? { groups: { workspace: record.workspaceId } } : undefined
103+
)
104+
}
105+
73106
if (!isMarkdown(record.originalName, record.contentType)) {
74107
const storagePrefix = USE_BLOB_STORAGE ? 'blob' : 's3'
75108
const servePath = `/api/files/serve/${storagePrefix}/${encodeURIComponent(record.key)}`
109+
auditExport('file', 0)
76110
return NextResponse.redirect(new URL(servePath, request.url), { status: 302 })
77111
}
78112

@@ -86,38 +120,10 @@ export const GET = withRouteHandler(
86120

87121
logger.info('Exporting markdown', { id, imageCount: imageIds.length })
88122

89-
const exportFormat = imageIds.length === 0 ? 'markdown' : 'zip'
90-
recordAudit({
91-
workspaceId: record.workspaceId ?? null,
92-
actorId: userId,
93-
action: AuditAction.FILE_DOWNLOADED,
94-
resourceType: AuditResourceType.FILE,
95-
resourceId: record.id,
96-
resourceName: record.originalName,
97-
description: `Exported file "${record.originalName}"`,
98-
metadata: {
99-
fileId: record.id,
100-
fileName: record.originalName,
101-
bytes: record.size,
102-
format: exportFormat,
103-
assetCount: imageIds.length,
104-
},
105-
request,
106-
})
107-
captureServerEvent(
108-
userId,
109-
'file_downloaded',
110-
{
111-
workspace_id: record.workspaceId ?? '',
112-
is_bulk: imageIds.length > 0,
113-
file_count: 1 + imageIds.length,
114-
},
115-
record.workspaceId ? { groups: { workspace: record.workspaceId } } : undefined
116-
)
117-
118123
if (imageIds.length === 0) {
119124
const mdName = safeFilename(record.originalName)
120125
const mdBytes = Buffer.from(mdContent, 'utf-8')
126+
auditExport('markdown', 0)
121127
return new NextResponse(new Uint8Array(mdBytes), {
122128
status: 200,
123129
headers: {
@@ -182,6 +188,7 @@ export const GET = withRouteHandler(
182188
const zipBuffer = await zip.generateAsync({ type: 'nodebuffer', compression: 'DEFLATE' })
183189
const zipName = safeFilename(`${record.originalName.replace(/\.[^.]+$/, '')}.zip`)
184190

191+
auditExport('zip', assetMap.size)
185192
return new NextResponse(new Uint8Array(zipBuffer), {
186193
status: 200,
187194
headers: {

0 commit comments

Comments
 (0)