Skip to content

Commit c47777f

Browse files
waleedlatif1waleedlatif1
authored
fix(security): close cross-tenant IDOR gaps in OAuth credential and execution auth (#4549)
* fix(security): close IDOR gaps in OAuth credential and execution authorization Routes that called resolveOAuthAccountId followed by a conditional workspace permission check (only run when workspaceId was set) silently skipped all ownership validation on the legacy account-ID fallback path. Any authenticated user could supply a raw account.id to access another tenant's OAuth credentials. - Replace resolveOAuthAccountId + conditional perm check with authorizeCredentialUse in: auth/oauth/wealthbox/item, tools/gmail/label, tools/onedrive/files, tools/onedrive/folder, tools/outlook/folders, tools/wealthbox/item (routes 1, 3-7) - Add authorizeCredentialUse ownership gate before resolveVertexCredential in providers/route.ts (route 2) - Add verifyFileAccess check on the user-supplied file key before downloadFileFromStorage in tools/wordpress/upload (route 8) - Add workflowId param to PauseResumeManager methods (enqueueOrStartResume, beginPausedCancellation, completePausedCancellation, blockQueuedResumesForCancellation, clearPausedCancellationIntent, getPausedCancellationStatus, processQueuedResumes) and filter all pausedExecutions lookups by workflowId so callers cannot act on another tenant's paused execution by supplying a foreign executionId (route 9) - Update all call sites (cancel, resume, poll routes) to pass workflowId * fix(security): close verifyFileAccess bypass, thread workflowId to processQueuedResumes, fix log level - Fail closed in WordPress upload when userFile.key is present but authResult.userId is absent, preventing silent bypass of ownership check via JWT fallback path - Thread workflowId into processQueuedResumes in the async resume error-recovery path and in pause-persistence.ts to close residual cross-tenant gap - Change logger.error to logger.warn for credential access denial in OneDrive folder route to match all other routes in this PR * fix(security): thread workflowId through all processQueuedResumes call sites Closes residual cross-tenant IDOR gap where processQueuedResumes was called without a workflowId scope in persistPauseResult, startResumeExecution (success and error paths), and clearPausedCancellationIntent. workflowId was already in scope at each site — this wires it through to the existing optional parameter. * fix(security): remove any types, drop extraneous comments, normalize caught errors - catch (error: any) → catch (error) + toError(error).message in resume and cancel routes - Remove what-not-why inline comments from wordpress upload and onedrive/files routes - Remove redundant debug-only item breakdown log and the file-IDs log in onedrive/files - Trim extraneous DAG-edge comments from updateSnapshotAfterResume in HITL manager * fix: use logger.warn for credential access denial in outlook folders route * fix(security): make workflowId required in all HITL pause/resume methods All 7 method signatures (processQueuedResumes, enqueueOrStartResume, beginPausedCancellation, completePausedCancellation, blockQueuedResumesForCancellation, clearPausedCancellationIntent, getPausedCancellationStatus) previously accepted workflowId as optional. Every call site already supplies it — making it required closes the vulnerability at the type level so future callers cannot accidentally omit tenant scoping and silently fall back to an unscoped DB query. * fix(security): thread workflowId through internal HITL cancellation calls and remove dead branches in credential-access * fix(security): harden workflowId scoping and file key guard Replace falsy workflowId checks in PauseResumeManager (all methods now unconditionally apply the workflowId WHERE clause, preventing empty-string bypass). Flip WordPress upload file guard from truthy key check to explicit non-empty validation so key:"" fails closed with a 404 instead of silently skipping access control.
1 parent badfeae commit c47777f

15 files changed

Lines changed: 380 additions & 522 deletions

File tree

‎apps/sim/app/api/auth/oauth/wealthbox/item/route.ts‎

Lines changed: 11 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
1-
import { db } from '@sim/db'
2-
import { account } from '@sim/db/schema'
31
import { createLogger } from '@sim/logger'
4-
import { eq } from 'drizzle-orm'
52
import { type NextRequest, NextResponse } from 'next/server'
63
import { wealthboxOAuthItemContract } from '@/lib/api/contracts/selectors/wealthbox'
74
import { parseRequest } from '@/lib/api/server'
8-
import { getSession } from '@/lib/auth'
5+
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
96
import { validateEnum, validatePathSegment } from '@/lib/core/security/input-validation'
107
import { generateRequestId } from '@/lib/core/utils/request'
118
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
12-
import { refreshAccessTokenIfNeeded, resolveOAuthAccountId } from '@/app/api/auth/oauth/utils'
9+
import { refreshAccessTokenIfNeeded } from '@/app/api/auth/oauth/utils'
1310

1411
export const dynamic = 'force-dynamic'
1512

@@ -31,13 +28,6 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
3128
const requestId = generateRequestId()
3229

3330
try {
34-
const session = await getSession()
35-
36-
if (!session?.user?.id) {
37-
logger.warn(`[${requestId}] Unauthenticated request rejected`)
38-
return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
39-
}
40-
4131
const parsed = await parseRequest(wealthboxOAuthItemContract, request, {})
4232
if (!parsed.success) return parsed.response
4333
const { credentialId, itemId, type } = parsed.data.query
@@ -60,39 +50,18 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
6050
return NextResponse.json({ error: itemIdValidation.error }, { status: 400 })
6151
}
6252

63-
const resolved = await resolveOAuthAccountId(credentialId)
64-
if (!resolved) {
65-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
66-
}
67-
68-
if (resolved.workspaceId) {
69-
const { getUserEntityPermissions } = await import('@/lib/workspaces/permissions/utils')
70-
const perm = await getUserEntityPermissions(
71-
session.user.id,
72-
'workspace',
73-
resolved.workspaceId
74-
)
75-
if (perm === null) {
76-
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
77-
}
78-
}
79-
80-
const credentials = await db
81-
.select()
82-
.from(account)
83-
.where(eq(account.id, resolved.accountId))
84-
.limit(1)
85-
86-
if (!credentials.length) {
87-
logger.warn(`[${requestId}] Credential not found`, { credentialId })
88-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
53+
const credAccess = await authorizeCredentialUse(request, {
54+
credentialId,
55+
requireWorkflowIdForInternal: false,
56+
})
57+
if (!credAccess.ok || !credAccess.credentialOwnerUserId) {
58+
logger.warn(`[${requestId}] Credential access denied`, { error: credAccess.error })
59+
return NextResponse.json({ error: credAccess.error || 'Unauthorized' }, { status: 401 })
8960
}
9061

91-
const accountRow = credentials[0]
92-
9362
const accessToken = await refreshAccessTokenIfNeeded(
94-
resolved.accountId,
95-
accountRow.userId,
63+
credentialId,
64+
credAccess.credentialOwnerUserId,
9665
requestId
9766
)
9867

‎apps/sim/app/api/providers/route.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { eq } from 'drizzle-orm'
66
import { type NextRequest, NextResponse } from 'next/server'
77
import { executeProviderContract } from '@/lib/api/contracts/providers'
88
import { parseRequest } from '@/lib/api/server'
9+
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
910
import { checkInternalAuth } from '@/lib/auth/hybrid'
1011
import { generateRequestId } from '@/lib/core/utils/request'
1112
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -141,6 +142,21 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
141142
let finalApiKey: string | undefined = apiKey
142143
try {
143144
if (provider === 'vertex' && vertexCredential) {
145+
const vertexCredAccess = await authorizeCredentialUse(request, {
146+
credentialId: vertexCredential,
147+
workflowId: workflowId || undefined,
148+
requireWorkflowIdForInternal: false,
149+
})
150+
if (!vertexCredAccess.ok) {
151+
logger.warn(`[${requestId}] Vertex credential access denied`, {
152+
error: vertexCredAccess.error,
153+
credentialId: vertexCredential,
154+
})
155+
return NextResponse.json(
156+
{ error: vertexCredAccess.error || 'Unauthorized' },
157+
{ status: 401 }
158+
)
159+
}
144160
finalApiKey = await resolveVertexCredential(requestId, vertexCredential)
145161
}
146162
} catch (error) {

‎apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ export const POST = withRouteHandler(
141141
try {
142142
const enqueueResult = await PauseResumeManager.enqueueOrStartResume({
143143
executionId,
144+
workflowId,
144145
contextId,
145146
resumeInput,
146147
userId,
@@ -249,7 +250,7 @@ export const POST = withRouteHandler(
249250
contextId: enqueueResult.contextId,
250251
failureReason: 'Failed to queue async resume execution',
251252
})
252-
await PauseResumeManager.processQueuedResumes(executionId)
253+
await PauseResumeManager.processQueuedResumes(executionId, workflowId)
253254
return NextResponse.json(
254255
{ error: 'Failed to queue resume execution. Please try again.' },
255256
{ status: 503 }
@@ -283,15 +284,15 @@ export const POST = withRouteHandler(
283284
executionId: enqueueResult.resumeExecutionId,
284285
message: 'Resume execution started.',
285286
})
286-
} catch (error: any) {
287+
} catch (error) {
287288
logger.error('Resume request failed', {
288289
workflowId,
289290
executionId,
290291
contextId,
291292
error,
292293
})
293294
return NextResponse.json(
294-
{ error: error.message || 'Failed to queue resume request' },
295+
{ error: toError(error).message || 'Failed to queue resume request' },
295296
{ status: 400 }
296297
)
297298
}

‎apps/sim/app/api/resume/poll/route.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ async function dispatchRow(row: DueRow, now: Date): Promise<RowResult> {
128128
try {
129129
const enqueueResult = await PauseResumeManager.enqueueOrStartResume({
130130
executionId: row.executionId,
131+
workflowId: row.workflowId,
131132
contextId: point.contextId,
132133
resumeInput: {},
133134
userId,

‎apps/sim/app/api/tools/gmail/label/route.ts‎

Lines changed: 16 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,13 @@
1-
import { db } from '@sim/db'
2-
import { account } from '@sim/db/schema'
31
import { createLogger } from '@sim/logger'
4-
import { eq } from 'drizzle-orm'
52
import { type NextRequest, NextResponse } from 'next/server'
63
import { gmailLabelSelectorContract } from '@/lib/api/contracts/selectors/google'
74
import { parseRequest } from '@/lib/api/server'
8-
import { getSession } from '@/lib/auth'
5+
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
96
import { validateAlphanumericId } from '@/lib/core/security/input-validation'
107
import { generateRequestId } from '@/lib/core/utils/request'
118
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
129
import { getScopesForService } from '@/lib/oauth/utils'
13-
import {
14-
getServiceAccountToken,
15-
refreshAccessTokenIfNeeded,
16-
resolveOAuthAccountId,
17-
ServiceAccountTokenError,
18-
} from '@/app/api/auth/oauth/utils'
10+
import { refreshAccessTokenIfNeeded, ServiceAccountTokenError } from '@/app/api/auth/oauth/utils'
1911

2012
export const dynamic = 'force-dynamic'
2113

@@ -25,13 +17,6 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
2517
const requestId = generateRequestId()
2618

2719
try {
28-
const session = await getSession()
29-
30-
if (!session?.user?.id) {
31-
logger.warn(`[${requestId}] Unauthenticated label request rejected`)
32-
return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
33-
}
34-
3520
const parsed = await parseRequest(gmailLabelSelectorContract, request, {})
3621
if (!parsed.success) return parsed.response
3722
const { credentialId, labelId } = parsed.data.query
@@ -43,56 +28,22 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
4328
return NextResponse.json({ error: labelIdValidation.error }, { status: 400 })
4429
}
4530

46-
const resolved = await resolveOAuthAccountId(credentialId)
47-
if (!resolved) {
48-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
49-
}
50-
51-
if (resolved.workspaceId) {
52-
const { getUserEntityPermissions } = await import('@/lib/workspaces/permissions/utils')
53-
const perm = await getUserEntityPermissions(
54-
session.user.id,
55-
'workspace',
56-
resolved.workspaceId
57-
)
58-
if (perm === null) {
59-
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
60-
}
31+
const credAccess = await authorizeCredentialUse(request, {
32+
credentialId,
33+
requireWorkflowIdForInternal: false,
34+
})
35+
if (!credAccess.ok || !credAccess.credentialOwnerUserId) {
36+
logger.warn(`[${requestId}] Credential access denied`, { error: credAccess.error })
37+
return NextResponse.json({ error: credAccess.error || 'Unauthorized' }, { status: 401 })
6138
}
6239

63-
let accessToken: string | null = null
64-
65-
if (resolved.credentialType === 'service_account' && resolved.credentialId) {
66-
accessToken = await getServiceAccountToken(
67-
resolved.credentialId,
68-
getScopesForService('gmail'),
69-
impersonateEmail
70-
)
71-
} else {
72-
const credentials = await db
73-
.select()
74-
.from(account)
75-
.where(eq(account.id, resolved.accountId))
76-
.limit(1)
77-
78-
if (!credentials.length) {
79-
logger.warn(`[${requestId}] Credential not found`)
80-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
81-
}
82-
83-
const accountRow = credentials[0]
84-
85-
logger.info(
86-
`[${requestId}] Using credential: ${accountRow.id}, provider: ${accountRow.providerId}`
87-
)
88-
89-
accessToken = await refreshAccessTokenIfNeeded(
90-
resolved.accountId,
91-
accountRow.userId,
92-
requestId,
93-
getScopesForService('gmail')
94-
)
95-
}
40+
const accessToken = await refreshAccessTokenIfNeeded(
41+
credentialId,
42+
credAccess.credentialOwnerUserId,
43+
requestId,
44+
getScopesForService('gmail'),
45+
impersonateEmail
46+
)
9647

9748
if (!accessToken) {
9849
return NextResponse.json({ error: 'Failed to obtain valid access token' }, { status: 401 })

0 commit comments

Comments
 (0)