From a5108d512f754a937860bda5e0c2c40c7266e19e Mon Sep 17 00:00:00 2001 From: Weiko Date: Thu, 16 Jul 2026 15:19:46 +0200 Subject: [PATCH] Block filters on restricted fields (#22873) ## Summary - Reject filter conditions that target fields without read access, including relation traversals - Add unit and integration coverage for denied field filter access The issue is an information leak through filtering: a user who cannot read a field can still infer its values from totalCount ### Manual reproduction - Create or use a non-admin role. - Give it read access to People records. - Disable read access to the Person jobTitle field. - Assign a test member to that role. - Authenticate as that member. Run: ```gql query People($filter: PersonFilterInput) { people(filter: $filter, first: 0) { totalCount } } Variables: { "filter": { "jobTitle": { "like": "Par%" } } } ``` Ensure at least one Person has a matching jobTitle. Before the fix, the request succeeds: ```gql { "data": { "people": { "totalCount": 1 } } } ``` The caller can probe restricted values using different filters. After the fix, it returns a permission error: ```gql { "errors": [ { "message": "Permission denied" } ] } ``` The same should happen through a relation filter, for example filtering Companies by a restricted Person field: ```gql query Companies($filter: CompanyFilterInput) { companies(filter: $filter, first: 0) { totalCount } } { "filter": { "people": { "jobTitle": { "like": "Par%" } } } } ``` --- .../common-group-by-query-runner.service.ts | 22 +++++++++---- .../graphql-query-filter-field.parser.ts | 20 ++++++++++++ .../read-permissions.integration-spec.ts | 32 +++++++++++++++++++ 3 files changed, 67 insertions(+), 7 deletions(-) diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-group-by-query-runner.service.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-group-by-query-runner.service.ts index 1b394cf747..d69ff917a6 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-group-by-query-runner.service.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-group-by-query-runner.service.ts @@ -4,6 +4,7 @@ import { CompositeFieldSubFieldName, PartialFieldMetadataItemOption, RecordFilterGroupLogicalOperator, + type RestrictedFieldsPermissions, } from 'twenty-shared/types'; import { assertIsDefinedOrThrow, @@ -183,12 +184,14 @@ export class CommonGroupByQueryRunnerService extends CommonBaseQueryRunnerServic args, flatObjectMetadata, flatFieldMetadataMaps, + restrictedFields, appliedFilters, workspaceId, }: { args: GroupByQueryArgs; flatObjectMetadata: FlatObjectMetadata; flatFieldMetadataMaps: FlatEntityMaps; + restrictedFields: RestrictedFieldsPermissions; appliedFilters: ObjectRecordFilter; workspaceId: string; }): Promise { @@ -244,13 +247,15 @@ export class CommonGroupByQueryRunnerService extends CommonBaseQueryRunnerServic const fields = getFlatFieldsFromFlatObjectMetadata( flatObjectMetadata, flatFieldMetadataMaps, - ).map((field) => ({ - id: field.id, - name: field.name, - type: field.type, - label: field.label, - options: field.options as PartialFieldMetadataItemOption[], - })); + ) + .filter((field) => restrictedFields[field.id]?.canRead !== false) + .map((field) => ({ + id: field.id, + name: field.name, + type: field.type, + label: field.label, + options: field.options as PartialFieldMetadataItemOption[], + })); const filtersFromView = computeRecordGqlOperationFilter({ recordFilters, @@ -311,6 +316,9 @@ export class CommonGroupByQueryRunnerService extends CommonBaseQueryRunnerServic args, flatObjectMetadata, flatFieldMetadataMaps, + restrictedFields: + queryBuilder.objectRecordsPermissions[flatObjectMetadata.id] + ?.restrictedFields ?? {}, appliedFilters, workspaceId, }); diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser.ts index ee0ced75a0..1169b0526f 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser.ts @@ -23,6 +23,11 @@ import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-m import { buildFieldMapsFromFlatObjectMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/build-field-maps-from-flat-object-metadata.util'; import { isMorphOrRelationFlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-morph-or-relation-flat-field-metadata.util'; import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; +import { + PermissionsException, + PermissionsExceptionCode, + PermissionsExceptionMessage, +} from 'src/engine/metadata-modules/permissions/permissions.exception'; import { type WorkspaceSelectQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-select-query-builder'; import { GraphqlQueryFilterConditionParser } from './graphql-query-filter-condition.parser'; @@ -30,6 +35,7 @@ import { GraphqlQueryFilterConditionParser } from './graphql-query-filter-condit const ARRAY_OPERATORS = ['in', 'contains', 'notContains']; export class GraphqlQueryFilterFieldParser { + private flatObjectMetadata: FlatObjectMetadata; private flatFieldMetadataMaps: FlatEntityMaps; private flatObjectMetadataMaps?: FlatEntityMaps; private fieldIdByName: Record; @@ -42,6 +48,7 @@ export class GraphqlQueryFilterFieldParser { flatObjectMetadataMaps?: FlatEntityMaps, depth = 0, ) { + this.flatObjectMetadata = flatObjectMetadata; this.flatFieldMetadataMaps = flatFieldMetadataMaps; this.flatObjectMetadataMaps = flatObjectMetadataMaps; this.depth = depth; @@ -78,6 +85,19 @@ export class GraphqlQueryFilterFieldParser { throw new Error(`Field metadata not found for field: ${key}`); } + const objectPermissions = + outerQueryBuilder.objectRecordsPermissions[this.flatObjectMetadata.id]; + + if ( + objectPermissions?.canReadObjectRecords === false || + objectPermissions?.restrictedFields[fieldMetadata.id]?.canRead === false + ) { + throw new PermissionsException( + PermissionsExceptionMessage.PERMISSION_DENIED, + PermissionsExceptionCode.PERMISSION_DENIED, + ); + } + if ( isFilterKeyARelation && isMorphOrRelationFlatFieldMetadata(fieldMetadata) && diff --git a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/fields-permissions/read-permissions.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/fields-permissions/read-permissions.integration-spec.ts index 95de61ce8a..121bd791c4 100644 --- a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/fields-permissions/read-permissions.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/fields-permissions/read-permissions.integration-spec.ts @@ -449,6 +449,38 @@ describe('Field permissions restrictions', () => { expect(response.body.data.companies.edges[0].node.id).toBeDefined(); }); + it('should reject filters on fields without read permission', async () => { + await upsertFieldPermissions({ + roleId: customRoleId, + fieldPermissions: [ + { + objectMetadataId: personObjectId, + fieldMetadataId: restrictedPersonFieldId, + canReadFieldValue: false, + canUpdateFieldValue: null, + }, + ], + }); + + const graphqlOperation = { + query: gql` + query People($filter: PersonFilterInput) { + people(filter: $filter, first: 0) { + totalCount + } + } + `, + variables: { + filter: { jobTitle: { like: 'Par%' } }, + }, + }; + + const response = + await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); + + expectPermissionDeniedError(response); + }); + describe('Aggregate operations', () => { it('1. should allow aggregate over a restricted field', async () => { await restrictAccessToCompanyEmployee(