Skip to content

Commit 2516396

Browse files
committed
address greptile comments
1 parent aba68ba commit 2516396

File tree

9 files changed

+92
-50
lines changed

9 files changed

+92
-50
lines changed

apps/sim/app/api/permission-groups/[id]/members/bulk/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { db } from '@sim/db'
22
import { member, permissionGroup, permissionGroupMember } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4-
import { and, eq, inArray, notInArray } from 'drizzle-orm'
4+
import { and, eq, inArray, ne } from 'drizzle-orm'
55
import { type NextRequest, NextResponse } from 'next/server'
66
import { z } from 'zod'
77
import { getSession } from '@/lib/auth'
@@ -123,7 +123,7 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
123123
and(
124124
eq(permissionGroup.organizationId, result.group.organizationId),
125125
inArray(permissionGroupMember.userId, usersToAdd),
126-
notInArray(permissionGroupMember.permissionGroupId, [id])
126+
ne(permissionGroupMember.permissionGroupId, id)
127127
)
128128
)
129129

apps/sim/app/api/permission-groups/[id]/members/route.ts

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -110,42 +110,42 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
110110
)
111111
}
112112

113-
const existingMembership = await db
114-
.select({
115-
id: permissionGroupMember.id,
116-
permissionGroupId: permissionGroupMember.permissionGroupId,
117-
})
118-
.from(permissionGroupMember)
119-
.innerJoin(permissionGroup, eq(permissionGroupMember.permissionGroupId, permissionGroup.id))
120-
.where(
121-
and(
122-
eq(permissionGroupMember.userId, userId),
123-
eq(permissionGroup.organizationId, result.group.organizationId)
124-
)
125-
)
126-
.limit(1)
127-
128-
if (existingMembership.length > 0) {
129-
if (existingMembership[0].permissionGroupId === id) {
130-
return NextResponse.json(
131-
{ error: 'User is already in this permission group' },
132-
{ status: 409 }
113+
const newMember = await db.transaction(async (tx) => {
114+
const existingMembership = await tx
115+
.select({
116+
id: permissionGroupMember.id,
117+
permissionGroupId: permissionGroupMember.permissionGroupId,
118+
})
119+
.from(permissionGroupMember)
120+
.innerJoin(permissionGroup, eq(permissionGroupMember.permissionGroupId, permissionGroup.id))
121+
.where(
122+
and(
123+
eq(permissionGroupMember.userId, userId),
124+
eq(permissionGroup.organizationId, result.group.organizationId)
125+
)
133126
)
127+
.limit(1)
128+
129+
if (existingMembership.length > 0) {
130+
if (existingMembership[0].permissionGroupId === id) {
131+
throw new Error('ALREADY_IN_GROUP')
132+
}
133+
await tx
134+
.delete(permissionGroupMember)
135+
.where(eq(permissionGroupMember.id, existingMembership[0].id))
134136
}
135-
await db
136-
.delete(permissionGroupMember)
137-
.where(eq(permissionGroupMember.id, existingMembership[0].id))
138-
}
139137

140-
const newMember = {
141-
id: crypto.randomUUID(),
142-
permissionGroupId: id,
143-
userId,
144-
assignedBy: session.user.id,
145-
assignedAt: new Date(),
146-
}
138+
const memberData = {
139+
id: crypto.randomUUID(),
140+
permissionGroupId: id,
141+
userId,
142+
assignedBy: session.user.id,
143+
assignedAt: new Date(),
144+
}
147145

148-
await db.insert(permissionGroupMember).values(newMember)
146+
await tx.insert(permissionGroupMember).values(memberData)
147+
return memberData
148+
})
149149

150150
logger.info('Added member to permission group', {
151151
permissionGroupId: id,
@@ -158,6 +158,12 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
158158
if (error instanceof z.ZodError) {
159159
return NextResponse.json({ error: error.errors[0].message }, { status: 400 })
160160
}
161+
if (error instanceof Error && error.message === 'ALREADY_IN_GROUP') {
162+
return NextResponse.json(
163+
{ error: 'User is already in this permission group' },
164+
{ status: 409 }
165+
)
166+
}
161167
logger.error('Error adding member to permission group', error)
162168
return NextResponse.json({ error: 'Failed to add member' }, { status: 500 })
163169
}

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { useParams } from 'next/navigation'
77
import {
88
Badge,
99
Combobox,
10+
type ComboboxOption,
1011
type ComboboxOptionGroup,
1112
Popover,
1213
PopoverContent,

apps/sim/executor/execution/block-executor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export class BlockExecutor {
7777

7878
try {
7979
if (!isSentinel && blockType) {
80-
await validateBlockType(ctx.userId, blockType)
80+
await validateBlockType(ctx.userId, blockType, ctx)
8181
}
8282

8383
resolvedInputs = this.resolver.resolveInputs(ctx, node.id, block.config.params, block)

apps/sim/executor/handlers/agent/agent-handler.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export class AgentBlockHandler implements BlockHandler {
6262
const responseFormat = this.parseResponseFormat(filteredInputs.responseFormat)
6363
const model = filteredInputs.model || AGENT.DEFAULT_MODEL
6464

65-
await validateModelProvider(ctx.userId, model)
65+
await validateModelProvider(ctx.userId, model, ctx)
6666

6767
const providerId = getProviderFromModel(model)
6868
const formattedTools = await this.formatTools(ctx, filteredInputs.tools || [])
@@ -162,11 +162,11 @@ export class AgentBlockHandler implements BlockHandler {
162162
const hasCustomTools = tools.some((t) => t.type === 'custom-tool')
163163

164164
if (hasMcpTools) {
165-
await validateMcpToolsAllowed(ctx.userId)
165+
await validateMcpToolsAllowed(ctx.userId, ctx)
166166
}
167167

168168
if (hasCustomTools) {
169-
await validateCustomToolsAllowed(ctx.userId)
169+
await validateCustomToolsAllowed(ctx.userId, ctx)
170170
}
171171
}
172172

@@ -240,7 +240,7 @@ export class AgentBlockHandler implements BlockHandler {
240240
otherTools.map(async (tool) => {
241241
try {
242242
if (tool.type && tool.type !== 'custom-tool' && tool.type !== 'mcp') {
243-
await validateBlockType(ctx.userId, tool.type)
243+
await validateBlockType(ctx.userId, tool.type, ctx)
244244
}
245245
if (tool.type === 'custom-tool' && (tool.schema || tool.customToolId)) {
246246
return await this.createCustomTool(ctx, tool)

apps/sim/executor/handlers/evaluator/evaluator-handler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class EvaluatorBlockHandler implements BlockHandler {
3838
bedrockRegion: inputs.bedrockRegion,
3939
}
4040

41-
await validateModelProvider(ctx.userId, evaluatorConfig.model)
41+
await validateModelProvider(ctx.userId, evaluatorConfig.model, ctx)
4242

4343
const providerId = getProviderFromModel(evaluatorConfig.model)
4444

apps/sim/executor/handlers/router/router-handler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export class RouterBlockHandler implements BlockHandler {
7474
bedrockRegion: inputs.bedrockRegion,
7575
}
7676

77-
await validateModelProvider(ctx.userId, routerConfig.model)
77+
await validateModelProvider(ctx.userId, routerConfig.model, ctx)
7878

7979
const providerId = getProviderFromModel(routerConfig.model)
8080

@@ -214,7 +214,7 @@ export class RouterBlockHandler implements BlockHandler {
214214
bedrockRegion: inputs.bedrockRegion,
215215
}
216216

217-
await validateModelProvider(ctx.userId, routerConfig.model)
217+
await validateModelProvider(ctx.userId, routerConfig.model, ctx)
218218

219219
const providerId = getProviderFromModel(routerConfig.model)
220220

apps/sim/executor/types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { TraceSpan } from '@/lib/logs/types'
2+
import type { PermissionGroupConfig } from '@/lib/permission-groups/types'
23
import type { BlockOutput } from '@/blocks/types'
34
import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
45

@@ -152,6 +153,9 @@ export interface ExecutionContext {
152153
userId?: string
153154
isDeployedContext?: boolean
154155

156+
permissionConfig?: PermissionGroupConfig | null
157+
permissionConfigLoaded?: boolean
158+
155159
blockStates: ReadonlyMap<string, BlockState>
156160
executedBlocks: ReadonlySet<string>
157161

apps/sim/executor/utils/permission-check.ts

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type PermissionGroupConfig,
88
parsePermissionGroupConfig,
99
} from '@/lib/permission-groups/types'
10+
import type { ExecutionContext } from '@/executor/types'
1011
import { getProviderFromModel } from '@/providers/utils'
1112

1213
const logger = createLogger('PermissionCheck')
@@ -78,15 +79,38 @@ export async function getUserPermissionConfig(
7879
return parsePermissionGroupConfig(groupMembership.config)
7980
}
8081

82+
export async function getPermissionConfig(
83+
userId: string | undefined,
84+
ctx?: ExecutionContext
85+
): Promise<PermissionGroupConfig | null> {
86+
if (!userId) {
87+
return null
88+
}
89+
90+
if (ctx) {
91+
if (ctx.permissionConfigLoaded) {
92+
return ctx.permissionConfig ?? null
93+
}
94+
95+
const config = await getUserPermissionConfig(userId)
96+
ctx.permissionConfig = config
97+
ctx.permissionConfigLoaded = true
98+
return config
99+
}
100+
101+
return getUserPermissionConfig(userId)
102+
}
103+
81104
export async function validateModelProvider(
82105
userId: string | undefined,
83-
model: string
106+
model: string,
107+
ctx?: ExecutionContext
84108
): Promise<void> {
85109
if (!userId) {
86110
return
87111
}
88112

89-
const config = await getUserPermissionConfig(userId)
113+
const config = await getPermissionConfig(userId, ctx)
90114

91115
if (!config || config.allowedModelProviders === null) {
92116
return
@@ -102,13 +126,14 @@ export async function validateModelProvider(
102126

103127
export async function validateBlockType(
104128
userId: string | undefined,
105-
blockType: string
129+
blockType: string,
130+
ctx?: ExecutionContext
106131
): Promise<void> {
107132
if (!userId) {
108133
return
109134
}
110135

111-
const config = await getUserPermissionConfig(userId)
136+
const config = await getPermissionConfig(userId, ctx)
112137

113138
if (!config || config.allowedIntegrations === null) {
114139
return
@@ -120,12 +145,15 @@ export async function validateBlockType(
120145
}
121146
}
122147

123-
export async function validateMcpToolsAllowed(userId: string | undefined): Promise<void> {
148+
export async function validateMcpToolsAllowed(
149+
userId: string | undefined,
150+
ctx?: ExecutionContext
151+
): Promise<void> {
124152
if (!userId) {
125153
return
126154
}
127155

128-
const config = await getUserPermissionConfig(userId)
156+
const config = await getPermissionConfig(userId, ctx)
129157

130158
if (!config) {
131159
return
@@ -137,12 +165,15 @@ export async function validateMcpToolsAllowed(userId: string | undefined): Promi
137165
}
138166
}
139167

140-
export async function validateCustomToolsAllowed(userId: string | undefined): Promise<void> {
168+
export async function validateCustomToolsAllowed(
169+
userId: string | undefined,
170+
ctx?: ExecutionContext
171+
): Promise<void> {
141172
if (!userId) {
142173
return
143174
}
144175

145-
const config = await getUserPermissionConfig(userId)
176+
const config = await getPermissionConfig(userId, ctx)
146177

147178
if (!config) {
148179
return

0 commit comments

Comments
 (0)