Skip to content

feat(enterprise): permission groups, access control - #2736

Merged
icecrasher321 merged 15 commits into
stagingfrom
feat/permission-groups
Jan 10, 2026
Merged

feat(enterprise): permission groups, access control #2736
icecrasher321 merged 15 commits into
stagingfrom
feat/permission-groups

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

Summary

Define permission groups to control what features and integrations team members can use.

Allowed Model Providers - Restrict which AI providers users can access (OpenAI, Anthropic, Google, etc.)
Allowed Blocks - Control which workflow blocks are available
Platform Settings - Hide Knowledge Base, disable MCP tools, or disable custom tools

Type of Change

  • New feature

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Jan 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentReviewUpdated (UTC)
docsSkippedSkippedJan 10, 2026 4:14am

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR introduces Permission Groups (Access Control) as an Enterprise-tier feature, allowing organization admins to restrict team members' access to specific AI model providers, workflow blocks/integrations, and platform features (Knowledge Base, MCP tools, Custom tools, etc.).

Architecture Overview

The implementation spans 49 files across multiple layers:

Backend (API Routes):

  • /api/permission-groups/* - CRUD operations for permission groups and membership management
  • /api/v1/admin/access-control - Admin API for managing groups across organizations
  • Permission checks integrated into workflow execution pipeline

Database Schema:

  • permission_group table with JSONB config column storing restrictions
  • permission_group_member table with unique constraint (one group per user per org)
  • Proper cascade deletes and indexes

Runtime Enforcement:

  • executor/utils/permission-check.ts - Core validation functions called during workflow execution
  • Validates model providers, block types, MCP tools, and custom tools
  • Integration points in block-executor, agent-handler, evaluator-handler, router-handler

UI Layer:

  • React hooks (usePermissionConfig) for client-side filtering
  • Toolbar and combobox components filter available blocks/providers
  • Comprehensive access control UI (1087 lines) for managing groups

Feature Gating:

  • Gated behind Enterprise plan with hasAccessControlAccess() check
  • Self-hosted deployments can enable via ACCESS_CONTROL_ENABLED env var
  • Admin/owner role required for management operations

Critical Security Issues Found

1. Permission Bypass in Tool Management (CRITICAL)

The checkCustomToolsPermission() and checkMcpToolsPermission() functions in copilot tool clients fail-open on errors. If the permission API call fails, returns an error, or the user has no organization, permission checks are silently bypassed. This violates fail-secure principles.

Impact: Users can bypass custom/MCP tool restrictions by:

  • Using personal (non-org) workspaces
  • Triggering API errors through network issues
  • Exploiting race conditions during org queries

2. Race Condition in Membership Assignment (HIGH)

POST /api/permission-groups/{id}/members performs check-then-act without transaction isolation. Two admins adding the same user simultaneously can cause constraint violations or inconsistent state.

3. Performance Issue: No Permission Config Caching (MEDIUM)

getUserPermissionConfig() queries the database on every validation call. A workflow with 10 blocks performing 3 provider checks each = 30+ redundant DB queries per execution.

Additional Issues

  • Authorization checks don't explicitly validate null/undefined roles
  • Environment-based bypasses (!isProd) mask issues in staging
  • Inconsistent null handling in API responses vs. type defaults
  • SQL array operations with single elements may not work across all dialects

Positive Aspects

✅ Comprehensive feature implementation across all layers
✅ Proper database schema with indexes and foreign keys
✅ Multi-layered enforcement (UI + runtime + API)
✅ Clean separation of concerns
✅ Proper admin API for enterprise operations
✅ Good integration with existing billing/subscription system

Confidence Score: 2/5

  • This PR has critical security vulnerabilities that allow permission bypasses. The fail-open design in tool management creates exploitable security holes.
  • Score reflects two critical security issues: (1) tool management permission checks that silently fail-open on errors, allowing complete bypass of restrictions, and (2) race condition in membership assignment that can cause data inconsistency. While the overall architecture is sound and most of the implementation is well-done, these security vulnerabilities must be fixed before merging to production.
  • Pay special attention to: manage-custom-tool.ts and manage-mcp-tool.ts (permission bypass vulnerabilities), permission-groups/[id]/members/route.ts (race condition requiring transaction), and executor/utils/permission-check.ts (needs caching for performance)

Important Files Changed

File Analysis

FilenameScoreOverview
apps/sim/app/api/permission-groups/[id]/members/route.ts2/5Adds/removes members from permission groups. Critical race condition in membership assignment (lines 113-138) could cause data inconsistency
apps/sim/lib/copilot/tools/client/workflow/manage-custom-tool.ts1/5Manages custom tools with permission checks. Permission bypass vulnerability: silently allows operations when org check fails (lines 35-46)
apps/sim/lib/copilot/tools/client/workflow/manage-mcp-tool.ts1/5Manages MCP tools with permission checks. Same fail-open security issue as custom tools - errors bypass restrictions (lines 29-40)
apps/sim/executor/utils/permission-check.ts3/5Core permission validation logic. No caching causes redundant DB queries per execution. Logic is sound but performance needs optimization
apps/sim/app/api/permission-groups/route.ts3/5CRUD operations for permission groups. Generally well-implemented with proper auth checks and validation
apps/sim/lib/billing/core/subscription.ts3/5Adds hasAccessControlAccess function. Environment-based bypass (!isProd) could mask issues in staging environments
apps/sim/executor/execution/block-executor.ts4/5Integrates validateBlockType at execution time (line 80). Proper enforcement at runtime prevents bypass attempts
apps/sim/app/api/v1/admin/access-control/route.ts4/5Admin API for managing permission groups. Properly secured with admin auth, includes cleanup for churned enterprise orgs

Sequence Diagram

sequenceDiagram
participant Admin as Enterprise Admin
participant API as Permission Groups API
participant DB as Database
participant User as Team Member
participant Executor as Workflow Executor
participant Copilot as Copilot Service
Note over Admin,DB: Permission Group Creation
Admin->>API: POST /api/permission-groups
API->>DB: Check enterprise plan status
DB-->>API: Plan confirmed
API->>DB: Create permission_group record
API->>DB: Insert group config (allowed providers, blocks)
DB-->>API: Group created
API-->>Admin: Return permission group
Note over Admin,DB: Add Members to Group
Admin->>API: POST /permission-groups/{id}/members/bulk
API->>DB: Query org members
API->>DB: Check existing memberships
API->>DB: Delete old memberships (move users)
API->>DB: Insert permission_group_member records
DB-->>API: Members assigned
API-->>Admin: Return success
Note over User,Executor: Runtime Permission Enforcement
User->>Executor: Execute workflow with agent block
Executor->>DB: getUserPermissionConfig(userId)
DB-->>Executor: Return group config (allowedModelProviders, etc)
Executor->>Executor: validateModelProvider(userId, model)
alt Model not allowed
Executor-->>User: ProviderNotAllowedError
else Model allowed
Executor->>Executor: validateBlockType(userId, blockType)
alt Block not allowed
Executor-->>User: IntegrationNotAllowedError
else Block allowed
Executor->>Executor: Execute block with tools
Executor->>Executor: validateMcpToolsAllowed(userId)
Executor->>Executor: validateCustomToolsAllowed(userId)
Executor-->>User: Execution result
end
end
Note over User,Copilot: UI Filtering
User->>Copilot: Open workflow editor
Copilot->>API: GET /api/permission-groups/user?organizationId=X
API->>DB: Query user's permission group
DB-->>API: Return config
API-->>Copilot: Permission config
Copilot->>Copilot: Filter toolbar blocks by allowedIntegrations
Copilot->>Copilot: Filter model providers by allowedModelProviders
Copilot-->>User: Display filtered UI
Note over User,Copilot: Tool Management (Vulnerability)
User->>Copilot: Create custom tool
Copilot->>API: GET /api/permission-groups/user
alt API fails or no org
API-->>Copilot: Error/null response
Copilot->>Copilot: checkCustomToolsPermission returns (no-op)
Note over Copilot: SECURITY ISSUE: Permission bypassed!
Copilot->>API: POST /api/tools/custom
API-->>Copilot: Tool created
else Has permission group
API-->>Copilot: config.disableCustomTools = true
Copilot->>Copilot: Throw error
Copilot-->>User: Permission denied
end
Loading

Comment threadapps/sim/app/api/permission-groups/[id]/members/bulk/route.ts Outdated
Comment threadapps/sim/app/api/permission-groups/[id]/members/route.ts Outdated
Comment threadapps/sim/executor/utils/permission-check.ts
Comment threadapps/sim/app/api/permission-groups/route.ts
Comment threadapps/sim/app/api/permission-groups/user/route.ts
Comment threadapps/sim/lib/billing/core/subscription.ts
@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR implements a comprehensive enterprise permission groups feature that allows organizations to control which features and integrations team members can access.

Overview

The implementation adds:

  • Permission Groups: Define groups with configurable access restrictions
  • Model Provider Restrictions: Control which AI providers (OpenAI, Anthropic, Google, etc.) users can access
  • Block/Integration Restrictions: Control which workflow blocks are available
  • Platform Settings: Hide features like Knowledge Base, disable MCP/custom tools, hide templates
  • Enforcement: Both UI-level filtering and runtime validation during workflow execution

Architecture

The feature is properly layered:

  1. Database Layer: Two new tables (permission_group, permission_group_member) with proper foreign keys and cascade deletes
  2. API Layer: Full CRUD operations with admin/owner authorization checks
  3. Validation Layer: Runtime permission checks in executor with context-level caching
  4. UI Layer: React hooks for filtering and comprehensive admin interface
  5. Integration: Copilot integration filters blocks/tools based on permissions

Key Implementation Patterns

  • Feature Gating: Enterprise-only with env variable override for self-hosted deployments
  • Permission Caching: Loads once per execution context and caches to avoid repeated DB queries
  • Validation Points: Block execution, model provider selection, tool usage (MCP/custom)
  • Graceful Degradation: Non-enterprise orgs return null config, allowing full access

Code Quality

  • Proper TypeScript typing throughout
  • Comprehensive error classes for different permission violations
  • Consistent logging for audit trails
  • Transaction handling for multi-step operations
  • Query client cache invalidation in React hooks

Confidence Score: 4/5

  • Safe to merge with minor caching and concurrency issues to address in follow-up
  • The implementation is comprehensive and well-structured with proper database schema, API authorization, runtime validation, and UI integration. Two non-critical issues identified: (1) permission config caching in ExecutionContext could use stale data in long-running workflows, and (2) race condition in concurrent permission group membership changes. Neither issue will cause data corruption or security vulnerabilities in typical usage, but should be addressed for robustness.
  • apps/sim/executor/utils/permission-check.ts (caching strategy), apps/sim/app/api/permission-groups/[id]/members/route.ts (race condition)

Important Files Changed

File Analysis

FilenameScoreOverview
packages/db/schema.ts5/5Adds permission_group and permission_group_member tables with proper foreign keys, indexes, and cascade delete behavior. Schema design is solid.
packages/db/migrations/0137_absurd_sumo.sql5/5Migration creates permission group tables with appropriate indexes and foreign key constraints. Schema changes are safe and reversible.
apps/sim/executor/utils/permission-check.ts3/5Implements permission validation functions with context-level caching. Has permission config caching issue that could allow stale permissions.
apps/sim/app/api/permission-groups/route.ts5/5Implements GET and POST for permission groups with proper auth checks and validation. Clean implementation.
apps/sim/app/api/permission-groups/[id]/members/route.ts3/5Manages permission group membership with single-user operations. Has race condition in concurrent membership changes.
apps/sim/app/api/permission-groups/user/route.ts5/5Returns user's permission config with enterprise plan check. Clean short-circuit logic for non-enterprise orgs.
apps/sim/hooks/queries/permission-groups.ts5/5React Query hooks for permission group CRUD operations. Proper cache invalidation and error handling.
apps/sim/hooks/use-permission-config.ts5/5Hook for filtering blocks and providers based on permission config. Clean utility functions with proper memoization.
apps/sim/lib/billing/core/subscription.ts5/5Adds hasAccessControlAccess and isOrganizationOnEnterprisePlan functions for feature gating. Proper env override logic.
apps/sim/executor/execution/block-executor.ts5/5Adds validateBlockType call before block execution. Properly skips sentinel blocks.
apps/sim/executor/handlers/agent/agent-handler.ts5/5Validates model providers and tools against permission config. Filters unavailable MCP tools and validates permissions before execution.
apps/sim/lib/copilot/tools/server/workflow/edit-workflow.ts4/5Filters blocks and tools during workflow editing based on permission config. Extensive changes to enforce permissions in copilot workflow editing.

Sequence Diagram

sequenceDiagram
participant User
participant API as API Routes
participant PermCheck as Permission Check
participant DB as Database
participant Executor
participant BlockHandler as Block Handler
Note over User,BlockHandler: Setup: Admin creates permission group
User->>API: POST /api/permission-groups
API->>DB: Check user is org admin
API->>DB: Insert permission_group
API->>DB: Insert permission_group_member(s)
API-->>User: Permission group created
Note over User,BlockHandler: Execution: User runs workflow
User->>Executor: Execute workflow
Executor->>PermCheck: getUserPermissionConfig(userId)
PermCheck->>DB: Query member table
PermCheck->>DB: Query permission_group_member
PermCheck->>DB: Query permission_group config
PermCheck-->>Executor: Return config (cached in ctx)
Executor->>BlockHandler: Execute block
BlockHandler->>PermCheck: validateBlockType(blockType, ctx)
PermCheck->>PermCheck: Check ctx.permissionConfig
alt Block not allowed
PermCheck-->>BlockHandler: throw IntegrationNotAllowedError
BlockHandler-->>Executor: Error
Executor-->>User: Block execution blocked
else Block allowed
PermCheck-->>BlockHandler: OK
BlockHandler->>PermCheck: validateModelProvider(model, ctx)
PermCheck->>PermCheck: Check allowedModelProviders
alt Provider not allowed
PermCheck-->>BlockHandler: throw ProviderNotAllowedError
else Provider allowed
PermCheck-->>BlockHandler: OK
BlockHandler->>BlockHandler: Execute block logic
BlockHandler-->>Executor: Block output
end
end
Note over User,BlockHandler: Copilot: Build workflow
User->>API: Copilot edit workflow request
API->>PermCheck: getUserPermissionConfig(userId)
PermCheck-->>API: Permission config
API->>API: Filter blocks by allowedIntegrations
API->>API: Filter tools by disableMcpTools/disableCustomTools
API->>DB: Save workflow
API-->>User: Workflow updated (restricted blocks removed)
Loading

Comment threadapps/sim/executor/utils/permission-check.ts
Comment threadapps/sim/app/api/permission-groups/[id]/members/route.ts
@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

@icecrasher321
icecrasher321 merged commit 47eb060 into stagingJan 10, 2026
5 of 6 checks passed
@icecrasher321
icecrasher321 deleted the feat/permission-groups branch January 10, 2026 04:16
@greptile-apps

Copy link
Copy Markdown
Contributor

Greptile Overview

Greptile Summary

This PR implements a comprehensive permission groups (access control) system for Enterprise organizations. The implementation allows admins/owners to define granular permission groups that control:

  1. Allowed Model Providers: Restrict which AI providers users can access (OpenAI, Anthropic, Google, etc.)
  2. Allowed Blocks/Integrations: Control which workflow blocks are available to users
  3. Platform Settings: Hide UI elements (Knowledge Base tab, API Keys, Files, Templates, etc.) and disable features (MCP tools, custom tools, Copilot)

Architecture

Database Layer:

  • Two new tables: permission_group (with org-unique names) and permission_group_member (one group per user via unique constraint)
  • Proper foreign keys with cascade deletes and indexes for performance
  • Config stored as JSONB for flexibility

API Layer:

  • Full CRUD operations for permission groups (/api/permission-groups)
  • Member management with bulk operations support
  • User config endpoint to fetch active permissions
  • Admin API for enterprise plan churn cleanup
  • All operations gated by hasAccessControlAccess() (Enterprise plan check)

Execution Layer:

  • Permission validation integrated into block executor at execution time
  • Model provider validation in Agent, Router, and Evaluator handlers
  • Block type validation before execution
  • Tool permission validation (MCP, Custom tools)
  • Permission config cached in ExecutionContext to minimize DB queries

UI Layer:

  • React hooks for permission filtering (usePermissionConfig)
  • Client-side permission checks in Copilot tools
  • UI components hidden based on permission flags

Key Implementation Details

  1. One Group Per User: The unique index on permission_group_member.user_id ensures each user can only be in one permission group. Adding a user to a new group automatically removes them from their previous group (handled via transaction).

  2. Enterprise-Only Feature: Access controlled via hasAccessControlAccess() which checks:

    • Organization is on Enterprise plan, OR
    • ACCESS_CONTROL_ENABLED env var is set (self-hosted override)
  3. Null-Safe Filtering: allowedIntegrations: null and allowedModelProviders: null mean "no restrictions" (all allowed). Only non-null arrays enforce restrictions.

  4. Performance: Permission config is loaded once per execution and cached in ctx.permissionConfig to avoid repeated DB queries.

Issues Found

Critical Logic Error:

  • The DELETE operation in apps/sim/app/api/permission-groups/[id]/route.ts (lines 202-203) manually deletes permissionGroupMember records before deleting the group, but this is redundant since the schema already has onDelete: 'cascade'. This creates unnecessary code and a potential race condition. The cascade delete will handle member cleanup automatically.

Overall Assessment

The implementation is well-architected with proper separation of concerns and comprehensive enforcement at both the API and execution layers. The database schema is sound with appropriate constraints and indexes. Permission checks are consistently applied across the codebase.

The one logic issue identified (redundant manual deletion) should be fixed to rely on the cascade delete mechanism, which is safer and more maintainable.

Confidence Score: 4/5

  • This PR is safe to merge after fixing the redundant deletion logic in the DELETE endpoint
  • Score of 4/5 reflects a well-implemented feature with comprehensive permission enforcement across all layers (API, execution, UI). The architecture is sound with proper database constraints, caching mechanisms, and enterprise plan gating. One critical logic issue was found: redundant manual deletion before cascade delete in the permission group DELETE endpoint (lines 202-203). This creates unnecessary code and a potential race condition. After fixing this issue, the PR would be a 5/5. All other aspects are well-designed: proper transactions in bulk operations, consistent permission validation, null-safe filtering, and appropriate use of indexes and foreign key constraints.
  • apps/sim/app/api/permission-groups/[id]/route.ts - Fix redundant deletion before cascade delete

Important Files Changed

File Analysis

FilenameScoreOverview
packages/db/schema.ts5/5Schema definitions correctly match migration with cascade deletes and unique constraints
apps/sim/app/api/permission-groups/[id]/route.ts3/5DELETE operation has redundant manual deletion and missing transaction, could cause race conditions
apps/sim/app/api/permission-groups/[id]/members/route.ts5/5Properly handles member addition with transaction to prevent duplicates and move between groups
apps/sim/executor/utils/permission-check.ts5/5Comprehensive permission validation functions with proper caching in ExecutionContext
apps/sim/executor/handlers/agent/agent-handler.ts5/5Agent handler properly validates model providers, MCP tools, and custom tools before execution
apps/sim/lib/billing/core/subscription.ts5/5hasAccessControlAccess properly checks enterprise plan and ACCESS_CONTROL_ENABLED flag

Sequence Diagram

sequenceDiagram
participant Admin as Admin/Owner
participant API as Permission Groups API
participant DB as Database
participant Executor as Workflow Executor
participant User as Restricted User
Note over Admin,DB: Permission Group Setup
Admin->>API: POST /api/permission-groups
API->>API: Check hasAccessControlAccess()
API->>API: Verify admin/owner role
API->>DB: Create permission_group
DB-->>API: Group created
API-->>Admin: Return group details
Admin->>API: POST /api/permission-groups/{id}/members
API->>DB: Check existing membership
alt User in another group
API->>DB: Delete old membership (transaction)
end
API->>DB: Insert new membership
DB-->>API: Member added
API-->>Admin: Success
Note over User,Executor: Permission Enforcement During Execution
User->>Executor: Execute workflow with Agent block
Executor->>Executor: validateModelProvider(userId, model)
Executor->>DB: Load permission config (cached in ctx)
DB-->>Executor: Return allowedModelProviders
alt Provider not allowed
Executor-->>User: ProviderNotAllowedError
else Provider allowed
Executor->>Executor: validateBlockType(userId, blockType)
Executor->>DB: Check allowedIntegrations
DB-->>Executor: Return config
alt Block not allowed
Executor-->>User: IntegrationNotAllowedError
else Block allowed
Executor->>Executor: validateToolPermissions(MCP/Custom)
alt Tools disabled
Executor-->>User: McpToolsNotAllowedError
else Tools allowed
Executor->>Executor: Execute block
Executor-->>User: Return results
end
end
end
Note over Admin,DB: Permission Group Deletion
Admin->>API: DELETE /api/permission-groups/{id}
API->>API: Check hasAccessControlAccess()
API->>API: Verify admin/owner role
API->>DB: Delete permission_group
Note over DB: CASCADE deletes members automatically
DB-->>API: Deletion complete
API-->>Admin: Success
Loading

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +202 to +203
await db.delete(permissionGroupMember).where(eq(permissionGroupMember.permissionGroupId, id))
await db.delete(permissionGroup).where(eq(permissionGroup.id, id))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manual deletion of permissionGroupMember records on line 202 is redundant because the schema already has onDelete: 'cascade' configured (see schema.ts line 1913). This redundant deletion creates a potential race condition and is unnecessary.

The database will automatically delete all related permissionGroupMember records when the permissionGroup is deleted due to the cascade constraint. The current implementation could fail if another request deletes members between lines 202 and 203.

Issue: Lines 202-203 should either:

  1. Remove line 202 entirely (recommended - rely on cascade delete)
  2. OR wrap both deletes in a transaction if you want explicit control
Suggested change
awaitdb.delete(permissionGroupMember).where(eq(permissionGroupMember.permissionGroupId,id))
awaitdb.delete(permissionGroup).where(eq(permissionGroup.id,id))
awaitdb.delete(permissionGroup).where(eq(permissionGroup.id,id))

The cascade delete in the foreign key constraint will handle member cleanup automatically and atomically.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/permission-groups/[id]/route.ts
Line: 202:203
Comment:
The manual deletion of `permissionGroupMember` records on line 202 is redundant because the schema already has `onDelete: 'cascade'` configured (see schema.ts line 1913). This redundant deletion creates a potential race condition and is unnecessary.
The database will automatically delete all related `permissionGroupMember` records when the `permissionGroup` is deleted due to the cascade constraint. The current implementation could fail if another request deletes members between lines 202 and 203.
**Issue**: Lines 202-203 should either:
1. Remove line 202 entirely (recommended - rely on cascade delete)
2. OR wrap both deletes in a transaction if you want explicit control
```suggestion await db.delete(permissionGroup).where(eq(permissionGroup.id, id))```
The cascade delete in the foreign key constraint will handle member cleanup automatically and atomically.
How can I resolve this? If you propose a fix, please make it concise.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@icecrasher321