feat(enterprise): permission groups, access control #2736
Merged
+13,669
−478
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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_grouptable with JSONB config column storing restrictionspermission_group_membertable 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_ENABLEDenv 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
| Filename | Score | Overview |
|---|---|---|
| apps/sim/app/api/permission-groups/[id]/members/route.ts | 2/5 | Adds/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.ts | 1/5 | Manages 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.ts | 1/5 | Manages 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.ts | 3/5 | Core 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.ts | 3/5 | CRUD operations for permission groups. Generally well-implemented with proper auth checks and validation |
| apps/sim/lib/billing/core/subscription.ts | 3/5 | Adds hasAccessControlAccess function. Environment-based bypass (!isProd) could mask issues in staging environments |
| apps/sim/executor/execution/block-executor.ts | 4/5 | Integrates validateBlockType at execution time (line 80). Proper enforcement at runtime prevents bypass attempts |
| apps/sim/app/api/v1/admin/access-control/route.ts | 4/5 | Admin 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
Collaborator
Author
Contributor
There was a problem hiding this comment.
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:
- Database Layer: Two new tables (
permission_group,permission_group_member) with proper foreign keys and cascade deletes - API Layer: Full CRUD operations with admin/owner authorization checks
- Validation Layer: Runtime permission checks in executor with context-level caching
- UI Layer: React hooks for filtering and comprehensive admin interface
- 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
| Filename | Score | Overview |
|---|---|---|
| packages/db/schema.ts | 5/5 | Adds 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.sql | 5/5 | Migration creates permission group tables with appropriate indexes and foreign key constraints. Schema changes are safe and reversible. |
| apps/sim/executor/utils/permission-check.ts | 3/5 | Implements permission validation functions with context-level caching. Has permission config caching issue that could allow stale permissions. |
| apps/sim/app/api/permission-groups/route.ts | 5/5 | Implements GET and POST for permission groups with proper auth checks and validation. Clean implementation. |
| apps/sim/app/api/permission-groups/[id]/members/route.ts | 3/5 | Manages permission group membership with single-user operations. Has race condition in concurrent membership changes. |
| apps/sim/app/api/permission-groups/user/route.ts | 5/5 | Returns user's permission config with enterprise plan check. Clean short-circuit logic for non-enterprise orgs. |
| apps/sim/hooks/queries/permission-groups.ts | 5/5 | React Query hooks for permission group CRUD operations. Proper cache invalidation and error handling. |
| apps/sim/hooks/use-permission-config.ts | 5/5 | Hook for filtering blocks and providers based on permission config. Clean utility functions with proper memoization. |
| apps/sim/lib/billing/core/subscription.ts | 5/5 | Adds hasAccessControlAccess and isOrganizationOnEnterprisePlan functions for feature gating. Proper env override logic. |
| apps/sim/executor/execution/block-executor.ts | 5/5 | Adds validateBlockType call before block execution. Properly skips sentinel blocks. |
| apps/sim/executor/handlers/agent/agent-handler.ts | 5/5 | Validates 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.ts | 4/5 | Filters 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)
Collaborator
Author
Contributor
Greptile OverviewGreptile SummaryThis PR implements a comprehensive permission groups (access control) system for Enterprise organizations. The implementation allows admins/owners to define granular permission groups that control:
ArchitectureDatabase Layer:
API Layer:
Execution Layer:
UI Layer:
Key Implementation Details
Issues FoundCritical Logic Error:
Overall AssessmentThe 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
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
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
|
Comment on lines
+202
to
+203
| await db.delete(permissionGroupMember).where(eq(permissionGroupMember.permissionGroupId, id)) | ||
| await db.delete(permissionGroup).where(eq(permissionGroup.id, id)) |
Contributor
There was a problem hiding this 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:
- Remove line 202 entirely (recommended - rely on cascade delete)
- OR wrap both deletes in a transaction if you want explicit control
Suggested change
| await db.delete(permissionGroupMember).where(eq(permissionGroupMember.permissionGroupId, id)) | |
| await db.delete(permissionGroup).where(eq(permissionGroup.id, id)) | |
| await db.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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Testing
Tested manually
Checklist