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. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/23400?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+1
@@ -16,4 +16,5 @@ export type ToolProviderContext = {
|
||||
threadId?: string;
|
||||
locale?: keyof typeof APP_LOCALES;
|
||||
onCodeExecutionUpdate?: CodeExecutionStreamEmitter;
|
||||
requireExplicitObjectGrants?: boolean;
|
||||
};
|
||||
|
||||
+95
-3
@@ -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<FlatObjectMetadata>;
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+52
-5
@@ -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(
|
||||
|
||||
+1
@@ -186,6 +186,7 @@ export class AgentAsyncExecutorService {
|
||||
workspaceId: agent.workspaceId,
|
||||
roleId: agentRoleId,
|
||||
rolePermissionConfig: agentRolePermissionConfig,
|
||||
requireExplicitObjectGrants: true,
|
||||
authContext,
|
||||
actorContext,
|
||||
userId:
|
||||
|
||||
+45
-16
@@ -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 () => {
|
||||
|
||||
-10
@@ -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 & {
|
||||
|
||||
+3
-10
@@ -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');
|
||||
|
||||
+15
@@ -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 [];
|
||||
};
|
||||
Reference in New Issue
Block a user