Skip to content

Commit 1469ba1

Browse files
committed
fix(security): scope execution log writes to owning workflow; add env-var workspace membership guard
Closes two cross-tenant vulnerabilities: 1. Workflow log cross-tenant write (route.ts + logging-session.ts): - Route: SELECT before creating LoggingSession to verify executionId belongs to the claimed workflowId; reject with 404 if owned by a different workflow. - LoggingSession: add workflow_id to all UPDATE/SELECT WHERE clauses (raw SQL marker queries, flushAccumulatedCost, loadExistingCost) so writes are a no-op if executionId was somehow injected. 2. Env-var workspace membership guard (environment/utils.ts): - getPersonalAndWorkspaceEnv now calls checkWorkspaceAccess when workspaceId is provided; throws if the userId is not a member, preventing any future caller from reading another workspace's decrypted secrets without explicit membership verification at the call site.
1 parent 118a8f0 commit 1469ba1

3 files changed

Lines changed: 45 additions & 3 deletions

File tree

apps/sim/app/api/workflows/[id]/log/route.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
import { db } from '@sim/db'
2+
import { workflowExecutionLogs } from '@sim/db/schema'
13
import { createLogger } from '@sim/logger'
4+
import { eq } from 'drizzle-orm'
25
import type { NextRequest } from 'next/server'
36
import { workflowLogContract } from '@/lib/api/contracts/workflows'
47
import { parseRequest } from '@/lib/api/server'
@@ -40,6 +43,21 @@ export const POST = withRouteHandler(
4043
return createErrorResponse('executionId is required when logging results', 400)
4144
}
4245

46+
// Verify that if executionId already exists in the DB it belongs to this workflow,
47+
// preventing a cross-tenant log write by a caller who owns a different workflow.
48+
const [existingLog] = await db
49+
.select({ workflowId: workflowExecutionLogs.workflowId })
50+
.from(workflowExecutionLogs)
51+
.where(eq(workflowExecutionLogs.executionId, executionId))
52+
.limit(1)
53+
54+
if (existingLog && existingLog.workflowId !== id) {
55+
logger.warn(
56+
`[${requestId}] executionId ${executionId} belongs to workflow ${existingLog.workflowId}, not ${id}`
57+
)
58+
return createErrorResponse('Execution not found', 404)
59+
}
60+
4361
logger.info(`[${requestId}] Persisting execution result for workflow: ${id}`, {
4462
executionId,
4563
success: result.success,

apps/sim/lib/environment/utils.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
getAccessibleEnvCredentials,
1010
syncPersonalEnvCredentialsForUser,
1111
} from '@/lib/credentials/environment'
12+
import { checkWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
1213

1314
const logger = createLogger('EnvironmentUtils')
1415
const EFFECTIVE_ENV_CACHE_TTL_MS = 15_000
@@ -72,6 +73,13 @@ export async function getPersonalAndWorkspaceEnv(
7273
conflicts: string[]
7374
decryptionFailures: string[]
7475
}> {
76+
if (workspaceId) {
77+
const access = await checkWorkspaceAccess(workspaceId, userId)
78+
if (!access.hasAccess) {
79+
throw new Error(`Access denied to workspace ${workspaceId}`)
80+
}
81+
}
82+
7583
const [personalRows, workspaceRows, accessibleEnvCredentials] = await Promise.all([
7684
db.select().from(environment).where(eq(environment.userId, userId)).limit(1),
7785
workspaceId

apps/sim/lib/logs/execution/logging-session.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { db } from '@sim/db'
22
import { workflowExecutionLogs } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { toError } from '@sim/utils/errors'
5-
import { eq, sql } from 'drizzle-orm'
5+
import { and, eq, sql } from 'drizzle-orm'
66
import { BASE_EXECUTION_CHARGE } from '@/lib/billing/constants'
77
import { executionLogger } from '@/lib/logs/execution/logger'
88
import {
@@ -29,6 +29,7 @@ type TriggerData = Record<string, unknown> & {
2929

3030
function buildStartedMarkerPersistenceQuery(params: {
3131
executionId: string
32+
workflowId: string
3233
marker: ExecutionLastStartedBlock
3334
}) {
3435
const markerJson = JSON.stringify(params.marker)
@@ -41,6 +42,7 @@ function buildStartedMarkerPersistenceQuery(params: {
4142
true
4243
)
4344
WHERE execution_id = ${params.executionId}
45+
AND workflow_id = ${params.workflowId}
4446
AND COALESCE(
4547
jsonb_extract_path_text(COALESCE(execution_data, '{}'::jsonb), 'lastStartedBlock', 'startedAt'),
4648
''
@@ -49,6 +51,7 @@ function buildStartedMarkerPersistenceQuery(params: {
4951

5052
function buildCompletedMarkerPersistenceQuery(params: {
5153
executionId: string
54+
workflowId: string
5255
marker: ExecutionLastCompletedBlock
5356
}) {
5457
const markerJson = JSON.stringify(params.marker)
@@ -61,6 +64,7 @@ function buildCompletedMarkerPersistenceQuery(params: {
6164
true
6265
)
6366
WHERE execution_id = ${params.executionId}
67+
AND workflow_id = ${params.workflowId}
6468
AND COALESCE(
6569
jsonb_extract_path_text(COALESCE(execution_data, '{}'::jsonb), 'lastCompletedBlock', 'endedAt'),
6670
''
@@ -190,6 +194,7 @@ export class LoggingSession {
190194
await db.execute(
191195
buildStartedMarkerPersistenceQuery({
192196
executionId: this.executionId,
197+
workflowId: this.workflowId,
193198
marker,
194199
})
195200
)
@@ -205,6 +210,7 @@ export class LoggingSession {
205210
await db.execute(
206211
buildCompletedMarkerPersistenceQuery({
207212
executionId: this.executionId,
213+
workflowId: this.workflowId,
208214
marker,
209215
})
210216
)
@@ -357,7 +363,12 @@ export class LoggingSession {
357363
models: this.accumulatedCost.models,
358364
},
359365
})
360-
.where(eq(workflowExecutionLogs.executionId, this.executionId))
366+
.where(
367+
and(
368+
eq(workflowExecutionLogs.workflowId, this.workflowId),
369+
eq(workflowExecutionLogs.executionId, this.executionId)
370+
)
371+
)
361372

362373
this.costFlushed = true
363374
} catch (error) {
@@ -372,7 +383,12 @@ export class LoggingSession {
372383
const [existing] = await db
373384
.select({ cost: workflowExecutionLogs.cost })
374385
.from(workflowExecutionLogs)
375-
.where(eq(workflowExecutionLogs.executionId, this.executionId))
386+
.where(
387+
and(
388+
eq(workflowExecutionLogs.workflowId, this.workflowId),
389+
eq(workflowExecutionLogs.executionId, this.executionId)
390+
)
391+
)
376392
.limit(1)
377393

378394
if (existing?.cost) {

0 commit comments

Comments
 (0)