System objects still bypass role defaults and ORM permission checks

[remihuigen]

This is a follow-up to #23062. These concerns were raised in the follow-up comment(#23062 (comment)), but were not addressed by the fix merged in #23280.

#23280 changes the role-cache precedence so that an explicit object-permission value is retained instead of being overwritten by the system-object default. However, the current code still contains two separate isSystem authorization bypasses.

1. Non-workflow, non-workspace-member system objects still ignore deny-all role defaults

The role cache first initializes CRUD values from the role-wide defaults, then resolves each non-workflow, non-workspace-member object as:

overrideValue ?? (isSystem ? true : defaultValue);

twenty/packages/twenty-server/src/engine/metadata-modules/role/services/workspace-roles-permissions-cache.service.ts

Lines 137 to 201 in a0281f6

	let canRead = role.canReadAllObjectRecords;
	let canUpdate = role.canUpdateAllObjectRecords;
	let canSoftDelete = role.canSoftDeleteAllObjectRecords;
	let canDestroy = role.canDestroyAllObjectRecords;
	const restrictedFields: RestrictedFieldsPermissions = {};
	const isWorkspaceMemberObject =
	universalIdentifier === WORKSPACE_MEMBER_OBJECT_UNIVERSAL_IDENTIFIER;
	const isWorkflowRelatedObject =
	WORKFLOW_STANDARD_OBJECT_UNIVERSAL_IDENTIFIERS.includes(
	universalIdentifier as (typeof WORKFLOW_STANDARD_OBJECT_UNIVERSAL_IDENTIFIERS)[number],
	);
	if (isWorkflowRelatedObject) {
	const hasWorkflowsPermissions =
	this.hasSettingsGatedObjectPermissions(
	role,
	roleRolePermissionFlags,
	PermissionFlagType.WORKFLOWS,
	);
	canRead = hasWorkflowsPermissions;
	canUpdate = hasWorkflowsPermissions;
	canSoftDelete = hasWorkflowsPermissions;
	canDestroy = hasWorkflowsPermissions;
	} else {
	if (isWorkspaceMemberObject) {
	const hasWorkspaceMembersPermissions =
	this.hasSettingsGatedObjectPermissions(
	role,
	roleRolePermissionFlags,
	PermissionFlagType.WORKSPACE_MEMBERS,
	);
	canRead = true;
	canUpdate = hasWorkspaceMembersPermissions;
	canSoftDelete = hasWorkspaceMembersPermissions;
	canDestroy = hasWorkspaceMembersPermissions;
	} else {
	const objectRecordPermissionsOverride = roleObjectPermissions.find(
	(objectPermission) =>
	objectPermission.objectMetadataId === objectMetadataId,
	);
	const getPermissionValue = (
	overrideValue: boolean | undefined,
	defaultValue: boolean,
	) => overrideValue ?? (isSystem ? true : defaultValue);
	canRead = getPermissionValue(
	objectRecordPermissionsOverride?.canReadObjectRecords,
	canRead,
	);
	canUpdate = getPermissionValue(
	objectRecordPermissionsOverride?.canUpdateObjectRecords,
	canUpdate,
	);
	canSoftDelete = getPermissionValue(
	objectRecordPermissionsOverride?.canSoftDeleteObjectRecords,
	canSoftDelete,
	);
	canDestroy = getPermissionValue(
	objectRecordPermissionsOverride?.canDestroyObjectRecords,
	canDestroy,
	);

Therefore, when a system object has no explicit object-permission row, isSystem forces canRead, canUpdate, canSoftDelete, and canDestroy to true, even if all corresponding role-wide defaults are false.

For example, a role with:

{
  canReadAllObjectRecords: false,
  canUpdateAllObjectRecords: false,
  canSoftDeleteAllObjectRecords: false,
  canDestroyAllObjectRecords: false,
  objectPermissions: [],
}

still receives full cached CRUD permissions for a non-workflow, non-workspace-member system object such as message.

The regression test added in #23280 explicitly asserts this behavior: it creates a system message object with no override and expects every CRUD permission to be true.

twenty/packages/twenty-server/src/engine/metadata-modules/role/services/__tests__/workspace-roles-permissions-cache.service.spec.ts

Lines 327 to 343 in a0281f6

	describe('system object (message)', () => {
	it('should default to full access when no object permission override exists', async () => {
	roleRepository.find.mockResolvedValue([
	createBaseRole({
	rolePermissionFlags: [],
	objectPermissions: [],
	}),
	]);
	const result = await service.computeForCache(WORKSPACE_ID);
	const messagePermissions = result[ROLE_ID][MESSAGE_OBJECT_METADATA_ID];
	expect(messagePermissions.canReadObjectRecords).toBe(true);
	expect(messagePermissions.canUpdateObjectRecords).toBe(true);
	expect(messagePermissions.canSoftDeleteObjectRecords).toBe(true);
	expect(messagePermissions.canDestroyObjectRecords).toBe(true);
	});

The adjacent test confirms that #23280 preserves an explicit false value in the cache:

twenty/packages/twenty-server/src/engine/metadata-modules/role/services/__tests__/workspace-roles-permissions-cache.service.spec.ts

Lines 345 to 370 in a0281f6

	it('should honor an explicit deny override instead of forcing system default', async () => {
	objectPermissionRepository.find.mockResolvedValue([
	{
	roleId: ROLE_ID,
	objectMetadataId: MESSAGE_OBJECT_METADATA_ID,
	canReadObjectRecords: false,
	canUpdateObjectRecords: false,
	canSoftDeleteObjectRecords: false,
	canDestroyObjectRecords: false,
	} as ObjectPermissionEntity,
	]);
	roleRepository.find.mockResolvedValue([
	createBaseRole({
	rolePermissionFlags: [],
	objectPermissions: [],
	}),
	]);
	const result = await service.computeForCache(WORKSPACE_ID);
	const messagePermissions = result[ROLE_ID][MESSAGE_OBJECT_METADATA_ID];
	expect(messagePermissions.canReadObjectRecords).toBe(false);
	expect(messagePermissions.canUpdateObjectRecords).toBe(false);
	expect(messagePermissions.canSoftDeleteObjectRecords).toBe(false);
	expect(messagePermissions.canDestroyObjectRecords).toBe(false);

That is a cache-precedence fix, but it does not make deny-all role defaults apply to these system objects.

2. The ORM still bypasses object- and field-permission enforcement for most system objects

validateOperationIsPermittedOrThrow still returns before it reads the object permission map or runs the operation and field checks:

if (objectMetadataIsSystem && !isWorkspaceMemberObject) {
  return;
}

twenty/packages/twenty-server/src/engine/twenty-orm/repository/permissions.utils.ts

Lines 116 to 149 in a0281f6

	const objectMetadataIsSystem = objectMetadata.isSystem === true;
	const isWorkspaceMemberObject =
	objectMetadata.universalIdentifier ===
	WORKSPACE_MEMBER_OBJECT_UNIVERSAL_IDENTIFIER;
	// TODO: this should be improved, we may have more complex permission configuration for is system objects
	if (objectMetadataIsSystem && !isWorkspaceMemberObject) {
	return;
	}
	const columnNameToFieldMetadataIdMap = getColumnNameToFieldMetadataIdMap(
	objectMetadata,
	flatFieldMetadataMaps,
	);
	const permissionsForEntity = objectsPermissions[objectMetadataIdForEntity];
	switch (operationType) {
	case 'select':
	if (!permissionsForEntity?.canReadObjectRecords) {
	throw new PermissionsException(
	PermissionsExceptionMessage.PERMISSION_DENIED,
	PermissionsExceptionCode.PERMISSION_DENIED,
	);
	}
	validateReadFieldPermissionOrThrow({
	restrictedFields: permissionsForEntity.restrictedFields,
	selectedColumns,
	columnNameToFieldMetadataIdMap,
	allFieldsSelected,
	entityName,
	flatFieldMetadataMaps,
	});

The return occurs before the select, insert, update, delete, restore, and soft-delete permission checks. It also occurs before validateReadFieldPermissionOrThrow and validateUpdateFieldPermissionOrThrow are called.

Consequently, the explicit-override cache fix in #23280 does not by itself make normal ORM paths enforce an override for these objects: the ORM validator still returns early based solely on isSystem.

Existing trusted-context bypass is already separate

The code already has a specific bypass for SystemAuthContext:

if (isSystemAuthContext(authContext)) {
  return { shouldBypassPermissionChecks: true };
}

twenty/packages/twenty-server/src/engine/twenty-orm/utils/resolve-role-permission-config.util.ts

Lines 18 to 32 in a0281f6

	if (isSystemAuthContext(authContext)) {
	return { shouldBypassPermissionChecks: true };
	}
	const roleId = resolveRoleIdFromAuthContext({
	authContext,
	userWorkspaceRoleMap,
	apiKeyRoleMap,
	});
	if (!isDefined(roleId)) {
	return null;
	}
	return { intersectionOf: [roleId] };

User and API-key contexts resolve to role IDs instead:

twenty/packages/twenty-server/src/engine/twenty-orm/utils/resolve-role-id-from-auth-context.util.ts

Lines 18 to 30 in a0281f6

	if (isUserAuthContext(authContext)) {
	return userWorkspaceRoleMap[authContext.userWorkspaceId];
	}
	if (isApiKeyAuthContext(authContext)) {
	return apiKeyRoleMap[authContext.apiKey.id];
	}
	if (
	isApplicationAuthContext(authContext) &&
	isDefined(authContext.application.defaultRoleId)
	) {
	return authContext.application.defaultRoleId;

The generic isSystem bypass therefore remains an additional authorization bypass for normal role-based contexts.

Expected behavior

For normal user and API-key contexts, non-workflow system objects should use the role defaults unless there is an explicit object-level override:

role object permission → row-level permission → field permission

Trusted internal operations may continue to use the existing SystemAuthContext / shouldBypassPermissionChecks bypass.

[sonarly]

🔴 Sonarly Analysis — Critical Severity

Authorization bypass for system object permissions

Normal role-based requests can bypass object and field permissions for most system objects. Restrictive roles may still read, create, update, delete, or restore system-object records.

Root Cause

A system-object authorization bypass exists in both permission computation and ORM enforcement.

The permission cache initializes CRUD permissions from role-wide defaults, but the non-workflow, non-workspace-member branch still uses:

(overrideValue ?? (isSystem ? true : defaultValue))

on the #23280 code path. When a system object has no explicit object-permission row, each undefined override falls back to true, ignoring canReadAllObjectRecords, canUpdateAllObjectRecords, canSoftDeleteAllObjectRecords, and canDestroyAllObjectRecords. The #23280 change only moved explicit overrides ahead of that fallback; it preserves the unconditional true result when no override exists. The checked repository HEAD contains the earlier form, (isSystem ? true : (overrideValue ?? defaultValue)), and the #23280 commit is present only in the repository history, not as an ancestor of HEAD [ref:workspace-cache-read] [ref:d6-show] [ref:git-head-history].

Separately, validateOperationIsPermittedOrThrow resolves the target metadata and then returns immediately for objectMetadata.isSystem === true unless the object is the workspace-member object:

if (objectMetadataIsSystem && !isWorkspaceMemberObject) {
  return;
}

That return occurs before reading objectsPermissions, before CRUD checks, and before read-field or update-field validation [ref:permissions-utils-read]. The ORM select, insert, and update query builders invoke validateQueryIsPermittedOrThrow, which passes the role-derived permission map and bypass flag into this path [ref:query-builder-call-sites]. Therefore, even an explicit deny override or restricted field permission is ineffective for these system objects through normal ORM requests.

The bypass is not required for trusted internal operations. resolveRolePermissionConfig returns shouldBypassPermissionChecks: true only for SystemAuthContext; user, API-key, and application contexts resolve to role IDs or role intersections [ref:resolve-role-config-read]. Git history also records the unconditional system-object bypass as a redundant security hole and identifies removal of both checks as the intended security behavior [ref:689-history]. This eliminates the hypothesis that the generic isSystem bypass is an intentional substitute for the trusted system context.

The triggering state reported for this issue is a normal role with all four role-wide CRUD defaults set to false, no explicit object-permission row, and a request against a non-workflow, non-workspace-member system object such as message. The #23280 remediation triggered the follow-up because it corrected only explicit override precedence: its no-override branch still grants true, while the ORM early return remained unchanged. The exact release that exposed the state, request volume, and affected tenants cannot be proven from this checkout because no production observability data was supplied.

Blame commit: bc28e155 by ijreilly — Introduce updateWorkspaceMemberSettings and clarify product (#19441)

Suggested Fix

Updated system-object permission computation to use role-wide CRUD defaults when no object override exists, instead of implicitly granting access. Removed the ORM validator's early return for system objects so object, CRUD, and field permissions are enforced consistently. The trusted SystemAuthContext bypass remains unchanged.

Added regression coverage for restrictive roles accessing system message objects and for ORM enforcement of denied system-object permissions. The recent #23280 commit was reviewed but only fixed explicit override precedence; it did not address the remaining system-default fallback or ORM early return.

---

Analyzed by Sonarly · View full analysis

[thomtrp]

Legit request. But not that simple TBH. System objects are supposed to be used as side effects of other objects. Before we remove their access to user, we need to ensure that it would not impact how the app is working. Will dive into this when I get more time.

[remihuigen]

System objects are supposed to be used as side effects of other objects

Should those side effects run under the users auth context, or SystemAuthContext ? (probably user)

[thomtrp]

If those effects run under user context, it would break the app. Let's assume you remove system objects permission but enable companies edition. You still want timeline logs - which are systems - being created when the company is edited. So we need to ensure side effects like those are not dependent on the user permissions. And this requires an audit of the different flows we have, that's why it's quite complex.