From 902bc6db635f9c637601b5d496e039f45f6b101f Mon Sep 17 00:00:00 2001 From: Etienne <45695613+etiennejouan@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:24:33 +0200 Subject: [PATCH] fix(ai-node) - scope AI agent node database tools to explicitly granted objects (#23400) ## Context An AI agent node scoped to a single object was still loading CRUD tools for the whole workspace, inflating every run's prompt to ~200k tokens (~110k on a standard seed workspace: 146 tools across 19 objects, 18 of them system objects). Two mechanisms caused this: the roles permissions cache force-grants every system object to every role (`isSystem ? true`), and blanket role flags (`canReadAllObjectRecords`, ...) grant all remaining objects. The per-object rows written by the agent Permissions tab were additive on top of that, so scoping an agent had almost no effect on its tool payload. ## What **Backend: explicit grants only for the agent node** - New opt-in flag `requireExplicitObjectGrants` on `ToolProviderContext`, set only by the workflow agent executor. - With the flag, `DatabaseToolProvider` generates CRUD tools exclusively from the role's explicit `objectPermission` rows: no row means no tools, and each verb gate reads the row directly (`canReadObjectRecords` for find tools, `canUpdateObjectRecords` for create/update/upsert, `canSoftDeleteObjectRecords` for delete). A verb left null is not granted; composed defaults and the system force-grant can no longer leak through. Composed permissions are still used for `restrictedFields`. - Explicit rows are read from the `flatObjectPermissionMaps` workspace cache key, fetched in the same `getOrRecompute` call as `rolesPermissions`: no extra query. - Without the flag (chat, MCP, tool index, workspace stats), behavior is unchanged: composed permissions, verified live (`getToolIndex` for an Admin returns the same 245 CRUD tools as before). - Removed the `CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT` guard on `upsertObjectPermissions` so system objects can be granted explicitly. **Frontend: grant system objects from the agent Permissions tab** - The objects picker in the workflow agent side panel ends with a new "System objects" submenu listing all active system objects; picking one opens the same CRUD grant flow as regular objects. - Permissions granted on system objects now resolve their labels in the existing permission list and can be deleted (both previously looked up non-system objects only, which would have hidden such grants). Result: an agent granted one object ships ~10 tools instead of 146, cutting the prompt from ~110k tokens to a few thousand and the per-run cost accordingly. ## Notes - Removing the system-object guard affects the whole upsert path: user roles can also receive explicit system object rows via the API. A `canRead: false` row on a system object now takes effect at the query layer for that role. - The agent role is resolved as the first role of the permission config, matching `getObjectsPermissionsFromRolePermissionConfig` (multi-role is not supported yet). ## Tests - `database-tool.provider.spec.ts`: three new cases for the flag (object without a row emits nothing, partial row emits only granted verbs, absent flag keeps composed behavior even with zero rows, which guards the chat regression). - `object-permission.service.spec.ts`: the system-object case now asserts a successful upsert. - Integration: dropped the failing "system object" upsert case and its snapshot, added a successful system object upsert case. Both suites pass against a live server. Review in cubic --- .../WorkflowAiAgentPermissionList.tsx | 2 +- .../WorkflowAiAgentPermissionsObjectsList.tsx | 17 +++- .../WorkflowAiAgentPermissionsTab.tsx | 74 ++++++++++---- ...AiAgentPermissionsStateOnSidePanelClose.ts | 5 + .../useWorkflowAiAgentPermissionActions.ts | 8 +- ...PermissionsIsSystemObjectsListOpenState.ts | 7 ++ .../interfaces/tool-provider-context.type.ts | 1 + .../__tests__/database-tool.provider.spec.ts | 98 ++++++++++++++++++- .../providers/database-tool.provider.ts | 57 ++++++++++- .../services/agent-async-executor.service.ts | 1 + .../object-permission.service.spec.ts | 61 +++++++++--- .../object-permission.service.ts | 10 -- .../permissions/permissions.service.ts | 13 +-- ...le-ids-from-role-permission-config.util.ts | 15 +++ ...permission-upsert.integration-spec.ts.snap | 12 --- ...ject-permission-upsert.integration-spec.ts | 27 ----- ...ject-permission-upsert.integration-spec.ts | 53 ++++++++++ 17 files changed, 357 insertions(+), 104 deletions(-) create mode 100644 packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsSystemObjectsListOpenState.ts create mode 100644 packages/twenty-server/src/engine/twenty-orm/utils/get-role-ids-from-role-permission-config.util.ts diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionList.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionList.tsx index f4f3d8901f..e68a3a095a 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionList.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionList.tsx @@ -33,7 +33,7 @@ export const WorkflowAiAgentPermissionList = ({ onDeletePermission, searchQuery, }: WorkflowAiAgentPermissionListProps) => { - const { alphaSortedActiveNonSystemObjectMetadataItems: objectMetadataItems } = + const { activeObjectMetadataItems: objectMetadataItems } = useFilteredObjectMetadataItems(); const sortedExistingPermissions = objectPermissions diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionsObjectsList.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionsObjectsList.tsx index 38c1546041..b4b1f3103c 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionsObjectsList.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionsObjectsList.tsx @@ -1,9 +1,13 @@ import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; import { SidePanelGroup } from '@/side-panel/components/SidePanelGroup'; import { t } from '@lingui/core/macro'; +import { isDefined } from 'twenty-shared/utils'; +import { IconSettings } from 'twenty-ui/icon'; +import { MenuItem } from 'twenty-ui/navigation'; import { WorkflowAiAgentPermissionsObjectRow } from './WorkflowAiAgentPermissionsObjectRow'; type WorkflowAiAgentPermissionsObjectsListProps = { + heading?: string; objects: Array< Pick< EnrichedObjectMetadataItem, @@ -11,16 +15,19 @@ type WorkflowAiAgentPermissionsObjectsListProps = { > >; onObjectClick: (objectId: string) => void; + onSystemObjectsClick?: () => void; readonly: boolean; }; export const WorkflowAiAgentPermissionsObjectsList = ({ + heading, objects, onObjectClick, + onSystemObjectsClick, readonly, }: WorkflowAiAgentPermissionsObjectsListProps) => { return ( - + {objects.map((objectMetadata) => ( ))} + {isDefined(onSystemObjectsClick) && ( + + )} ); }; diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionsTab.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionsTab.tsx index 5aba5c6a89..d8ca2c61a2 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionsTab.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/components/WorkflowAiAgentPermissionsTab.tsx @@ -8,6 +8,7 @@ import { type WorkflowAiAgentAction } from '@/workflow/types/Workflow'; import { useWorkflowAiAgentPermissionActions } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useWorkflowAiAgentPermissionActions'; import { workflowAiAgentActionAgentState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentActionAgentState'; import { workflowAiAgentPermissionsIsAddingPermissionState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsAddingPermissionState'; +import { workflowAiAgentPermissionsIsSystemObjectsListOpenState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsSystemObjectsListOpenState'; import { workflowAiAgentPermissionsSelectedObjectIdState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsSelectedObjectIdState'; import { useQuery } from '@apollo/client/react'; import { styled } from '@linaria/react'; @@ -83,9 +84,21 @@ export const WorkflowAiAgentPermissionsTab = ({ workflowAiAgentPermissionsIsAddingPermission, setWorkflowAiAgentPermissionsIsAddingPermission, ] = useAtomState(workflowAiAgentPermissionsIsAddingPermissionState); + const [ + workflowAiAgentPermissionsIsSystemObjectsListOpen, + setWorkflowAiAgentPermissionsIsSystemObjectsListOpen, + ] = useAtomState(workflowAiAgentPermissionsIsSystemObjectsListOpenState); - const { alphaSortedActiveNonSystemObjectMetadataItems: objectMetadataItems } = - useFilteredObjectMetadataItems(); + const { + alphaSortedActiveNonSystemObjectMetadataItems: objectMetadataItems, + objectMetadataItems: allObjectMetadataItems, + } = useFilteredObjectMetadataItems(); + + const systemObjectMetadataItems = allObjectMetadataItems + .filter((item) => item.isActive && item.isSystem) + .sort((itemA, itemB) => + itemA.nameSingular.localeCompare(itemB.nameSingular), + ); const { data: rolesData, @@ -114,6 +127,12 @@ export const WorkflowAiAgentPermissionsTab = ({ getSearchableValues: (item) => [item.labelSingular, item.labelPlural], }); + const filteredSystemObjects = filterBySearchQuery({ + items: systemObjectMetadataItems, + searchQuery, + getSearchableValues: (item) => [item.labelSingular, item.labelPlural], + }); + const { filteredPermissions: filteredActionPermissions, filteredEnabledPermissions: filteredEnabledActionPermissions, @@ -152,7 +171,7 @@ export const WorkflowAiAgentPermissionsTab = ({ } const selectedObject = isDefined(workflowAiAgentPermissionsSelectedObjectId) - ? objectMetadataItems.find( + ? allObjectMetadataItems.find( (item) => item.id === workflowAiAgentPermissionsSelectedObjectId, ) : undefined; @@ -164,10 +183,17 @@ export const WorkflowAiAgentPermissionsTab = ({ : undefined; const handleBack = () => { - isDefined(workflowAiAgentPermissionsSelectedObjectId) && + if (isDefined(workflowAiAgentPermissionsSelectedObjectId)) { setWorkflowAiAgentPermissionsSelectedObjectId(undefined); - !isDefined(workflowAiAgentPermissionsSelectedObjectId) && - setWorkflowAiAgentPermissionsIsAddingPermission(false); + return; + } + + if (workflowAiAgentPermissionsIsSystemObjectsListOpen) { + setWorkflowAiAgentPermissionsIsSystemObjectsListOpen(false); + return; + } + + setWorkflowAiAgentPermissionsIsAddingPermission(false); }; const handleObjectClick = (objectId: string) => { @@ -176,6 +202,7 @@ export const WorkflowAiAgentPermissionsTab = ({ const shouldShowBackButton = isDefined(workflowAiAgentPermissionsSelectedObjectId) || + workflowAiAgentPermissionsIsSystemObjectsListOpen || workflowAiAgentPermissionsIsAddingPermission; const shouldShowCrudList = isDefined(selectedObject); const shouldShowSelectionLists = @@ -227,22 +254,33 @@ export const WorkflowAiAgentPermissionsTab = ({ /> )} - {shouldShowSelectionLists && ( - <> + {shouldShowSelectionLists && + (workflowAiAgentPermissionsIsSystemObjectsListOpen ? ( - - - )} + ) : ( + <> + + setWorkflowAiAgentPermissionsIsSystemObjectsListOpen(true) + } + readonly={readonly} + /> + + + ))} {shouldShowExistingPermissions && ( <> diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useResetWorkflowAiAgentPermissionsStateOnSidePanelClose.ts b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useResetWorkflowAiAgentPermissionsStateOnSidePanelClose.ts index e066796143..56643c9351 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useResetWorkflowAiAgentPermissionsStateOnSidePanelClose.ts +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useResetWorkflowAiAgentPermissionsStateOnSidePanelClose.ts @@ -2,6 +2,7 @@ import { useListenToSidePanelClosing } from '@/ui/layout/side-panel/hooks/useLis import { useSetAtomState } from '@/ui/utilities/state/jotai/hooks/useSetAtomState'; import { workflowAiAgentActionAgentState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentActionAgentState'; import { workflowAiAgentPermissionsIsAddingPermissionState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsAddingPermissionState'; +import { workflowAiAgentPermissionsIsSystemObjectsListOpenState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsSystemObjectsListOpenState'; import { workflowAiAgentPermissionsSelectedObjectIdState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsSelectedObjectIdState'; export const useResetWorkflowAiAgentPermissionsStateOnSidePanelClose = () => { @@ -11,6 +12,9 @@ export const useResetWorkflowAiAgentPermissionsStateOnSidePanelClose = () => { const setWorkflowAiAgentPermissionsIsAddingPermission = useSetAtomState( workflowAiAgentPermissionsIsAddingPermissionState, ); + const setWorkflowAiAgentPermissionsIsSystemObjectsListOpen = useSetAtomState( + workflowAiAgentPermissionsIsSystemObjectsListOpenState, + ); const setWorkflowAiAgentActionAgent = useSetAtomState( workflowAiAgentActionAgentState, ); @@ -18,6 +22,7 @@ export const useResetWorkflowAiAgentPermissionsStateOnSidePanelClose = () => { const resetPermissionState = () => { setWorkflowAiAgentPermissionsSelectedObjectId(undefined); setWorkflowAiAgentPermissionsIsAddingPermission(false); + setWorkflowAiAgentPermissionsIsSystemObjectsListOpen(false); setWorkflowAiAgentActionAgent(undefined); }; diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useWorkflowAiAgentPermissionActions.ts b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useWorkflowAiAgentPermissionActions.ts index 748496b288..fa5343bac9 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useWorkflowAiAgentPermissionActions.ts +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useWorkflowAiAgentPermissionActions.ts @@ -7,6 +7,7 @@ import { CRUD_PERMISSIONS } from '@/workflow/workflow-steps/workflow-actions/ai- import { useAtomState } from '@/ui/utilities/state/jotai/hooks/useAtomState'; import { workflowAiAgentActionAgentState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentActionAgentState'; import { workflowAiAgentPermissionsIsAddingPermissionState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsAddingPermissionState'; +import { workflowAiAgentPermissionsIsSystemObjectsListOpenState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsSystemObjectsListOpenState'; import { workflowAiAgentPermissionsSelectedObjectIdState } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsSelectedObjectIdState'; import { t } from '@lingui/core/macro'; import { useMemo } from 'react'; @@ -38,7 +39,7 @@ export const useWorkflowAiAgentPermissionActions = ({ const { enqueueSuccessSnackBar } = useSnackBar(); const [workflowAiAgentActionAgent, setWorkflowAiAgentActionAgent] = useAtomState(workflowAiAgentActionAgentState); - const { alphaSortedActiveNonSystemObjectMetadataItems: objectMetadataItems } = + const { activeObjectMetadataItems: objectMetadataItems } = useFilteredObjectMetadataItems(); const settingsPermissionsConfig = useSettingsRolePermissionFlagConfig({ assignmentCapabilities: { canBeAssignedToAgents: true }, @@ -53,6 +54,9 @@ export const useWorkflowAiAgentPermissionActions = ({ const [, setWorkflowAiAgentPermissionsIsAddingPermission] = useAtomState( workflowAiAgentPermissionsIsAddingPermissionState, ); + const [, setWorkflowAiAgentPermissionsIsSystemObjectsListOpen] = useAtomState( + workflowAiAgentPermissionsIsSystemObjectsListOpenState, + ); const [createRole] = useMutation(CreateOneRoleDocument); const [assignRoleToAgent] = useMutation(AssignRoleToAgentDocument); @@ -201,6 +205,7 @@ export const useWorkflowAiAgentPermissionActions = ({ await refetchAgentAndRoles(); setWorkflowAiAgentPermissionsIsAddingPermission(false); setWorkflowAiAgentPermissionsSelectedObjectId(undefined); + setWorkflowAiAgentPermissionsIsSystemObjectsListOpen(false); }; const handleDeletePermission = async ( @@ -318,6 +323,7 @@ export const useWorkflowAiAgentPermissionActions = ({ await refetchAgentAndRoles(); setWorkflowAiAgentPermissionsIsAddingPermission(false); setWorkflowAiAgentPermissionsSelectedObjectId(undefined); + setWorkflowAiAgentPermissionsIsSystemObjectsListOpen(false); }; const handleDeletePermissionFlag = async (permissionFlagKey: string) => { diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsSystemObjectsListOpenState.ts b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsSystemObjectsListOpenState.ts new file mode 100644 index 0000000000..e34499b877 --- /dev/null +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/ai-agent-action/states/workflowAiAgentPermissionsIsSystemObjectsListOpenState.ts @@ -0,0 +1,7 @@ +import { createAtomState } from '@/ui/utilities/state/jotai/utils/createAtomState'; + +export const workflowAiAgentPermissionsIsSystemObjectsListOpenState = + createAtomState({ + key: 'workflowAiAgentPermissionsIsSystemObjectsListOpenState', + defaultValue: false, + }); diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/interfaces/tool-provider-context.type.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/interfaces/tool-provider-context.type.ts index 720faa20b3..f351e3ecff 100644 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/interfaces/tool-provider-context.type.ts +++ b/packages/twenty-server/src/engine/core-modules/tool-provider/interfaces/tool-provider-context.type.ts @@ -16,4 +16,5 @@ export type ToolProviderContext = { threadId?: string; locale?: keyof typeof APP_LOCALES; onCodeExecutionUpdate?: CodeExecutionStreamEmitter; + requireExplicitObjectGrants?: boolean; }; diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/providers/__tests__/database-tool.provider.spec.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/providers/__tests__/database-tool.provider.spec.ts index 7695c23a00..57b3269a8b 100644 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/providers/__tests__/database-tool.provider.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/tool-provider/providers/__tests__/database-tool.provider.spec.ts @@ -35,8 +35,23 @@ const createFlatObject = ( ...overrides, }); +type ExplicitPermissionRow = { + objectMetadataId: string; + canReadObjectRecords?: boolean; + canUpdateObjectRecords?: boolean; + canSoftDeleteObjectRecords?: boolean; +}; + +type GenerateDescriptorsTestOptions = { + requireExplicitObjectGrants?: boolean; + explicitPermissionRows?: ExplicitPermissionRow[]; +}; + describe('DatabaseToolProvider', () => { - const generateDescriptors = async (objects: FlatObjectMetadata[]) => { + const generateDescriptors = async ( + objects: FlatObjectMetadata[], + options?: GenerateDescriptorsTestOptions, + ) => { const flatObjectMetadataMaps = createEmptyFlatEntityMaps() as FlatEntityMaps; @@ -47,6 +62,15 @@ describe('DatabaseToolProvider', () => { object.universalIdentifier; } + const explicitPermissionRows = + options?.explicitPermissionRows ?? + objects.map((object) => ({ + objectMetadataId: object.id, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: true, + })); + const workspaceCacheService = { getOrRecompute: jest.fn().mockResolvedValue({ rolesPermissions: { @@ -54,6 +78,14 @@ describe('DatabaseToolProvider', () => { objects.map((object) => [object.id, allObjectPermissions]), ), }, + flatObjectPermissionMaps: { + byUniversalIdentifier: Object.fromEntries( + explicitPermissionRows.map((row, index) => [ + `object-permission-${index}`, + { roleId, ...row }, + ]), + ), + }, }), } as unknown as WorkspaceCacheService; @@ -90,13 +122,17 @@ describe('DatabaseToolProvider', () => { workspaceId, roleId, rolePermissionConfig: { unionOf: [roleId] }, + requireExplicitObjectGrants: options?.requireExplicitObjectGrants, }, { includeSchemas: false }, )) as (ToolIndexEntry | ToolDescriptor)[]; }; - const generateDescriptorNames = async (objects: FlatObjectMetadata[]) => { - const descriptors = await generateDescriptors(objects); + const generateDescriptorNames = async ( + objects: FlatObjectMetadata[], + options?: GenerateDescriptorsTestOptions, + ) => { + const descriptors = await generateDescriptors(objects, options); return descriptors.map((descriptor) => descriptor.name); }; @@ -251,4 +287,60 @@ describe('DatabaseToolProvider', () => { expect(descriptor.label.length).toBeGreaterThan(0); } }); + + describe('requireExplicitObjectGrants', () => { + const personObject = createFlatObject({ + nameSingular: 'person', + namePlural: 'people', + }); + const companyObject = createFlatObject({ + nameSingular: 'company', + namePlural: 'companies', + }); + + it('emits no tools for objects without an explicit permission row', async () => { + const descriptorNames = await generateDescriptorNames( + [personObject, companyObject], + { + requireExplicitObjectGrants: true, + explicitPermissionRows: [ + { + objectMetadataId: personObject.id, + canReadObjectRecords: true, + }, + ], + }, + ); + + expect(descriptorNames).toContain('find_many_people'); + expect(descriptorNames).not.toContain('find_many_companies'); + }); + + it('emits only the verbs granted by the explicit row', async () => { + const descriptorNames = await generateDescriptorNames([personObject], { + requireExplicitObjectGrants: true, + explicitPermissionRows: [ + { + objectMetadataId: personObject.id, + canReadObjectRecords: true, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + }, + ], + }); + + expect(descriptorNames).toContain('find_many_people'); + expect(descriptorNames).not.toContain('create_one_person'); + expect(descriptorNames).not.toContain('delete_one_person'); + }); + + it('keeps composed permissions when the flag is not set even without explicit rows', async () => { + const descriptorNames = await generateDescriptorNames([personObject], { + explicitPermissionRows: [], + }); + + expect(descriptorNames).toContain('find_many_people'); + expect(descriptorNames).toContain('create_one_person'); + }); + }); }); diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/providers/database-tool.provider.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/providers/database-tool.provider.ts index 1c85dc8937..b6459fe229 100644 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/providers/database-tool.provider.ts +++ b/packages/twenty-server/src/engine/core-modules/tool-provider/providers/database-tool.provider.ts @@ -27,8 +27,10 @@ import { type ToolDescriptor } from 'src/engine/core-modules/tool-provider/types import { type ToolIndexEntry } from 'src/engine/core-modules/tool-provider/types/tool-index-entry.type'; import { type ToolOutput } from 'src/engine/core-modules/tool/types/tool-output.type'; import { getDatabaseCrudToolFlatObjects } from 'src/engine/metadata-modules/ai/ai-agent/utils/get-database-crud-tool-flat-objects.util'; +import { type FlatObjectPermission } from 'src/engine/metadata-modules/flat-object-permission/types/flat-object-permission.type'; import { WorkspaceManyOrAllFlatEntityMapsCacheService } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.service'; import { getObjectsPermissionsFromRolePermissionConfig } from 'src/engine/twenty-orm/utils/get-objects-permissions-from-role-permission-config.util'; +import { getRoleIdsFromRolePermissionConfig } from 'src/engine/twenty-orm/utils/get-role-ids-from-role-permission-config.util'; import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service'; import { ToolCategory } from 'twenty-shared/ai'; @@ -68,9 +70,10 @@ export class DatabaseToolProvider implements ToolProvider { const toolNames = options?.toolNames; const descriptors: (ToolIndexEntry | ToolDescriptor)[] = []; - const { rolesPermissions } = + const { rolesPermissions, flatObjectPermissionMaps } = await this.workspaceCacheService.getOrRecompute(context.workspaceId, [ 'rolesPermissions', + 'flatObjectPermissionMaps', ]); const objectPermissions = getObjectsPermissionsFromRolePermissionConfig({ @@ -82,6 +85,34 @@ export class DatabaseToolProvider implements ToolProvider { return descriptors; } + const requireExplicitObjectGrants = + context.requireExplicitObjectGrants === true; + + const roleId = getRoleIdsFromRolePermissionConfig( + context.rolePermissionConfig, + )[0]; + + const explicitPermissionByObjectId = new Map< + string, + FlatObjectPermission + >(); + + if (requireExplicitObjectGrants) { + for (const flatObjectPermission of Object.values( + flatObjectPermissionMaps.byUniversalIdentifier, + )) { + if ( + isDefined(flatObjectPermission) && + flatObjectPermission.roleId === roleId + ) { + explicitPermissionByObjectId.set( + flatObjectPermission.objectMetadataId, + flatObjectPermission, + ); + } + } + } + const { flatObjectMetadataMaps, flatFieldMetadataMaps } = await this.flatEntityMapsCacheService.getOrRecomputeManyOrAllFlatEntityMaps( { @@ -96,11 +127,27 @@ export class DatabaseToolProvider implements ToolProvider { for (const flatObject of allFlatObjects) { const permission = objectPermissions[flatObject.id]; + const explicitPermission = explicitPermissionByObjectId.get( + flatObject.id, + ); - if (!permission) { + if ( + !permission || + (requireExplicitObjectGrants && !isDefined(explicitPermission)) + ) { continue; } + const canReadRecords = requireExplicitObjectGrants + ? explicitPermission?.canReadObjectRecords === true + : permission.canReadObjectRecords; + const canUpdateRecords = requireExplicitObjectGrants + ? explicitPermission?.canUpdateObjectRecords === true + : permission.canUpdateObjectRecords; + const canSoftDeleteRecords = requireExplicitObjectGrants + ? explicitPermission?.canSoftDeleteObjectRecords === true + : permission.canSoftDeleteObjectRecords; + const snakePlural = camelToSnakeCase(flatObject.namePlural); const snakeSingular = camelToSnakeCase(flatObject.nameSingular); @@ -125,7 +172,7 @@ export class DatabaseToolProvider implements ToolProvider { const shouldIncludeSchema = (name: string) => includeSchemas && (!toolNames || toolNames.has(name)); - if (permission.canReadObjectRecords) { + if (canReadRecords) { descriptors.push({ name: `find_many_${snakePlural}`, ...getCrudToolLabels( @@ -212,7 +259,7 @@ export class DatabaseToolProvider implements ToolProvider { } } - if (permission.canUpdateObjectRecords && canBeManagedByAutomation) { + if (canUpdateRecords && canBeManagedByAutomation) { descriptors.push({ name: `create_one_${snakeSingular}`, ...getCrudToolLabels( @@ -348,7 +395,7 @@ export class DatabaseToolProvider implements ToolProvider { }); } - if (permission.canSoftDeleteObjectRecords) { + if (canSoftDeleteRecords) { descriptors.push({ name: `delete_one_${snakeSingular}`, ...getCrudToolLabels( diff --git a/packages/twenty-server/src/engine/metadata-modules/ai/ai-agent-execution/services/agent-async-executor.service.ts b/packages/twenty-server/src/engine/metadata-modules/ai/ai-agent-execution/services/agent-async-executor.service.ts index 951b0b1a37..8cd7a84844 100644 --- a/packages/twenty-server/src/engine/metadata-modules/ai/ai-agent-execution/services/agent-async-executor.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/ai/ai-agent-execution/services/agent-async-executor.service.ts @@ -186,6 +186,7 @@ export class AgentAsyncExecutorService { workspaceId: agent.workspaceId, roleId: agentRoleId, rolePermissionConfig: agentRolePermissionConfig, + requireExplicitObjectGrants: true, authContext, actorContext, userId: diff --git a/packages/twenty-server/src/engine/metadata-modules/object-permission/__tests__/object-permission.service.spec.ts b/packages/twenty-server/src/engine/metadata-modules/object-permission/__tests__/object-permission.service.spec.ts index 4317d7d475..847bfe5af3 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-permission/__tests__/object-permission.service.spec.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-permission/__tests__/object-permission.service.spec.ts @@ -94,7 +94,7 @@ describe('ObjectPermissionService', () => { const systemObjectMetadataId = 'system-object-id'; const customObjectMetadataId = 'custom-object-id'; - it('should throw PermissionsException when trying to add object permission on system object', async () => { + it('should successfully create object permission for system object', async () => { const input: UpsertObjectPermissionsInput = { roleId, objectPermissions: [ @@ -108,8 +108,22 @@ describe('ObjectPermissionService', () => { ], }; - workspaceManyOrAllFlatEntityMapsCacheService.getOrRecomputeManyOrAllFlatEntityMaps.mockResolvedValue( - { + const permissionUniversalId = 'system-permission-universal-id'; + const freshFlatObjectPermission = { + id: 'system-permission-id', + universalIdentifier: permissionUniversalId, + roleId, + roleUniversalIdentifier: roleId, + objectMetadataId: systemObjectMetadataId, + objectMetadataUniversalIdentifier: systemObjectMetadataId, + canReadObjectRecords: true, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }; + + workspaceManyOrAllFlatEntityMapsCacheService.getOrRecomputeManyOrAllFlatEntityMaps + .mockResolvedValueOnce({ flatObjectPermissionMaps: { byUniversalIdentifier: {}, universalIdentifierById: {}, @@ -120,24 +134,39 @@ describe('ObjectPermissionService', () => { systemObjectMetadataId, true, ), - } as any, + } as any) + .mockResolvedValueOnce({ + flatObjectPermissionMaps: { + byUniversalIdentifier: { + [permissionUniversalId]: freshFlatObjectPermission, + }, + universalIdentifierById: { + [freshFlatObjectPermission.id]: permissionUniversalId, + }, + byId: { + [freshFlatObjectPermission.id]: freshFlatObjectPermission, + }, + }, + } as any); + + workspaceMigrationValidateBuildAndRunService.validateBuildAndRunWorkspaceMigration.mockResolvedValue( + { status: 'success' } as any, ); - await expect( - service.upsertObjectPermissions({ - workspaceId, - input, - }), - ).rejects.toThrow( - new PermissionsException( - PermissionsExceptionMessage.CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT, - PermissionsExceptionCode.CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT, - ), - ); + const result = await service.upsertObjectPermissions({ + workspaceId, + input, + }); + expect(result).toHaveLength(1); + expect(result[0]).toMatchObject({ + objectMetadataId: systemObjectMetadataId, + canReadObjectRecords: true, + canUpdateObjectRecords: false, + }); expect( workspaceMigrationValidateBuildAndRunService.validateBuildAndRunWorkspaceMigration, - ).not.toHaveBeenCalled(); + ).toHaveBeenCalled(); }); it('should successfully create object permission for custom (non-system) object', async () => { diff --git a/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.ts b/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.ts index 0380a7e27f..bf27a5af31 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.ts @@ -100,16 +100,6 @@ export class ObjectPermissionService { }, ); } - - if (objectMetadata.isSystem === true) { - throw new PermissionsException( - PermissionsExceptionMessage.CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT, - PermissionsExceptionCode.CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT, - { - userFriendlyMessage: msg`You cannot set permissions on system objects as they are managed by the platform.`, - }, - ); - } } const flatEntityToCreate: (UniversalFlatObjectPermission & { diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts index 8134a82aa8..f6892ebc73 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts @@ -28,6 +28,7 @@ import { type UserWorkspacePermissions } from 'src/engine/metadata-modules/permi import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service'; import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config'; +import { getRoleIdsFromRolePermissionConfig } from 'src/engine/twenty-orm/utils/get-role-ids-from-role-permission-config.util'; import { InjectWorkspaceScopedRepository } from 'src/engine/twenty-orm/workspace-scoped-repository/inject-workspace-scoped-repository.decorator'; import { WorkspaceScopedRepository } from 'src/engine/twenty-orm/workspace-scoped-repository/workspace-scoped-repository'; import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service'; @@ -285,16 +286,8 @@ export class PermissionsService { return null; } - let roleIds: string[] = []; - let useIntersection = false; - - if ('intersectionOf' in rolePermissionConfig) { - roleIds = rolePermissionConfig.intersectionOf; - useIntersection = true; - } else if ('unionOf' in rolePermissionConfig) { - roleIds = rolePermissionConfig.unionOf; - useIntersection = false; - } + const roleIds = getRoleIdsFromRolePermissionConfig(rolePermissionConfig); + const useIntersection = 'intersectionOf' in rolePermissionConfig; if (roleIds.length === 0) { throw new Error('No role IDs provided'); diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/get-role-ids-from-role-permission-config.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/get-role-ids-from-role-permission-config.util.ts new file mode 100644 index 0000000000..f574b8feb4 --- /dev/null +++ b/packages/twenty-server/src/engine/twenty-orm/utils/get-role-ids-from-role-permission-config.util.ts @@ -0,0 +1,15 @@ +import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config'; + +export const getRoleIdsFromRolePermissionConfig = ( + rolePermissionConfig: RolePermissionConfig, +): string[] => { + if ('intersectionOf' in rolePermissionConfig) { + return rolePermissionConfig.intersectionOf; + } + + if ('unionOf' in rolePermissionConfig) { + return rolePermissionConfig.unionOf; + } + + return []; +}; diff --git a/packages/twenty-server/test/integration/metadata/suites/object-permission/__snapshots__/failing-object-permission-upsert.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/object-permission/__snapshots__/failing-object-permission-upsert.integration-spec.ts.snap index f922e54671..9c47a01725 100644 --- a/packages/twenty-server/test/integration/metadata/suites/object-permission/__snapshots__/failing-object-permission-upsert.integration-spec.ts.snap +++ b/packages/twenty-server/test/integration/metadata/suites/object-permission/__snapshots__/failing-object-permission-upsert.integration-spec.ts.snap @@ -1,17 +1,5 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing -exports[`Object permission upsert should fail when object is system object 1`] = ` -{ - "extensions": { - "code": "FORBIDDEN", - "subCode": "CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT", - "userFriendlyMessage": "You cannot set permissions on system objects as they are managed by the platform.", - }, - "message": "Cannot add object permission on system object", - "name": "ForbiddenError", -} -`; - exports[`Object permission upsert should fail when objectMetadataId does not exist 1`] = ` { "extensions": { diff --git a/packages/twenty-server/test/integration/metadata/suites/object-permission/failing-object-permission-upsert.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/object-permission/failing-object-permission-upsert.integration-spec.ts index 44b0f2ef14..e64b4be9a6 100644 --- a/packages/twenty-server/test/integration/metadata/suites/object-permission/failing-object-permission-upsert.integration-spec.ts +++ b/packages/twenty-server/test/integration/metadata/suites/object-permission/failing-object-permission-upsert.integration-spec.ts @@ -18,7 +18,6 @@ import { type UpsertObjectPermissionsInput } from 'src/engine/metadata-modules/o type GlobalTestContext = { editableRoleId: string; nonEditableRoleId: string; - systemObjectMetadataId: string; nonSystemObjectMetadataId: string; editableRoleWithNoReadId: string; }; @@ -97,23 +96,6 @@ const failingObjectPermissionUpsertTestCases: EachTestingContext[] }), }, }, - { - title: 'when object is system object', - context: { - input: (globalContext) => ({ - roleId: globalContext.editableRoleId, - objectPermissions: [ - { - objectMetadataId: globalContext.systemObjectMetadataId, - canReadObjectRecords: true, - canUpdateObjectRecords: false, - canSoftDeleteObjectRecords: false, - canDestroyObjectRecords: false, - }, - ], - }), - }, - }, { title: 'when read=false but canUpdateObjectRecords=true (read/write consistency)', @@ -137,7 +119,6 @@ const failingObjectPermissionUpsertTestCases: EachTestingContext[] describe('Object permission upsert should fail', () => { let editableRoleId: string; let nonEditableRoleId: string; - let systemObjectMetadataId: string; let nonSystemObjectMetadataId: string; let editableRoleWithNoReadId: string; @@ -221,13 +202,6 @@ describe('Object permission upsert should fail', () => { getObjectMetadataOperation, ); const edges = objectMetadataResponse.body.data?.objects?.edges ?? []; - const systemObjectNode = edges.find( - (edge: { node: { isSystem: boolean | string } }) => - edge.node.isSystem === true || String(edge.node.isSystem) === 'true', - )?.node; - jestExpectToBeDefined(systemObjectNode); - systemObjectMetadataId = systemObjectNode.id; - const nonSystemObjectNode = edges.find( (edge: { node: { isSystem: boolean | string } }) => edge.node.isSystem === false || String(edge.node.isSystem) === 'false', @@ -257,7 +231,6 @@ describe('Object permission upsert should fail', () => { const globalContext: GlobalTestContext = { editableRoleId: editableRoleId ?? '', nonEditableRoleId: nonEditableRoleId ?? '', - systemObjectMetadataId: systemObjectMetadataId ?? '', nonSystemObjectMetadataId: nonSystemObjectMetadataId ?? '', editableRoleWithNoReadId: editableRoleWithNoReadId ?? '', }; diff --git a/packages/twenty-server/test/integration/metadata/suites/object-permission/successful-object-permission-upsert.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/object-permission/successful-object-permission-upsert.integration-spec.ts index a02793584e..4bc256ee10 100644 --- a/packages/twenty-server/test/integration/metadata/suites/object-permission/successful-object-permission-upsert.integration-spec.ts +++ b/packages/twenty-server/test/integration/metadata/suites/object-permission/successful-object-permission-upsert.integration-spec.ts @@ -1,14 +1,17 @@ +import gql from 'graphql-tag'; import { createOneObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/create-one-object-metadata.util'; import { deleteOneObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/delete-one-object-metadata.util'; import { upsertObjectPermissions } from 'test/integration/metadata/suites/object-permission/utils/upsert-object-permissions.util'; import { createOneRole } from 'test/integration/metadata/suites/role/utils/create-one-role.util'; import { deleteOneRole } from 'test/integration/metadata/suites/role/utils/delete-one-role.util'; +import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util'; import { jestExpectToBeDefined } from 'test/utils/jest-expect-to-be-defined.util.test'; import { isDefined } from 'twenty-shared/utils'; describe('Object permission upsert should succeed', () => { let createdRoleId: string; let customObjectMetadataId: string; + let systemObjectMetadataId: string; beforeAll(async () => { const { data: roleData } = await createOneRole({ @@ -46,6 +49,29 @@ describe('Object permission upsert should succeed', () => { customObjectMetadataId = createOneObject.id; jestExpectToBeDefined(customObjectMetadataId); + + const objectMetadataResponse = await makeMetadataAPIRequest({ + query: gql` + query { + objects(paging: { first: 1000 }) { + edges { + node { + id + isSystem + } + } + } + } + `, + }); + const edges = objectMetadataResponse.body.data?.objects?.edges ?? []; + const systemObjectNode = edges.find( + (edge: { node: { isSystem: boolean | string } }) => + edge.node.isSystem === true || String(edge.node.isSystem) === 'true', + )?.node; + + jestExpectToBeDefined(systemObjectNode); + systemObjectMetadataId = systemObjectNode.id; }); afterAll(async () => { @@ -117,6 +143,33 @@ describe('Object permission upsert should succeed', () => { }); }); + it('should upsert object permission on a system object', async () => { + const { data } = await upsertObjectPermissions({ + expectToFail: false, + input: { + roleId: createdRoleId, + objectPermissions: [ + { + objectMetadataId: systemObjectMetadataId, + canReadObjectRecords: true, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + ], + }, + }); + + expect(data?.upsertObjectPermissions).toHaveLength(1); + expect(data?.upsertObjectPermissions?.[0]).toMatchObject({ + objectMetadataId: systemObjectMetadataId, + canReadObjectRecords: true, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }); + }); + it('should upsert with read and all write permissions true', async () => { const { data } = await upsertObjectPermissions({ expectToFail: false,