Fix orphaned mirrored field permission blocking relation field updates

[FelixMalfait]

Context

Reported by a user: after creating a new role and explicitly enabling Edit on a relation field (the field backing a member_id join column), updating that field still fails with the generic "User does not have permission" error, even though every permission visible in the role settings is enabled.

Root cause

Restricting a relation field intentionally mirrors the restriction onto the inverse relation field of the target object (addRelatedFieldPermissionsToDesired), writing a second fieldPermission row on another object. Granting the permission back, however, only deleted the row for the field named in the request: the mirror pass mapped null (clear) to undefined (keep as is), and the deletion loop only considered fields present in the input. The mirrored row survived as an invisible orphan.

Since updates always write the join column owned by the many-to-one side, the enforcement layer resolved memberId back to that relation field, found the orphaned canUpdateFieldValue: false row, and denied the update, while role settings showed Edit enabled on the side the admin had toggled.

Reproduced end to end on a seeded workspace: restricting company.people creates rows for both company.people and person.company; granting company.people back deleted only its own row; a member updating person.companyId was then denied despite the role showing the field as editable.

Changes

field-permission.service.ts

• Grant-back now propagates to the mirrored inverse field permission: a cleared source row clears the mirror row too (null propagates, undefined still means keep)
• Rows left with no restriction (both flags cleared) are deleted instead of kept, and empty mirror rows are no longer created
• The delete-entity mapping is extracted into toUniversalFlatFieldPermissionToDelete, used by both deletion sites

isOneToManyRelationFieldReadOnlyDueToTargetUpdatePermission.ts

• The one-to-many editability check now also honors a field-level update restriction on the inverse many-to-one field, so the UI shows the field read-only instead of offering an edit the server will reject

Tests

• New integration spec relation-field-permission-grant-back.integration-spec.ts exercising the real upsert flow: restricting a relation field mirrors the restriction onto the inverse field, granting it back deletes both rows, and repeated grants do not create empty permission rows (the original unit-spec coverage was moved here after test(server): delete service unit specs #24094 removed all mock-based service unit specs on main)
• New frontend unit tests: the one-to-many check returns read-only when the inverse field is update-restricted and stays editable when only unrelated fields are restricted
• Verified live on a seeded instance: restrict still blocks while active, grant-back now removes both rows and the previously failing join-column update succeeds
• Lint, format and typecheck pass for both packages

Notes

This fix stops new orphans from being created. Orphaned mirror rows already present in existing databases remain until an admin re-toggles Edit on either side of the relation (the grant now cascades and clears them). An automated cleanup could be done as a follow-up data migration, kept out of this PR deliberately since it rewrites existing permission data.

[greptile-apps]

Greptile Summary

This PR fixes orphaned mirrored relation-field permissions by propagating grant-back operations to inverse fields and deleting permission rows once both restrictions are cleared. It also makes ordinary one-to-many relation fields read-only in the frontend when the inverse field cannot be updated.

• Propagates relation permission clears to mirrored inverse fields.
• Avoids creating or retaining permission rows with no restrictions.
• Aligns frontend relation editability with inverse field-level update permissions.
• Adds frontend and server regression tests for grant-back and read-only behavior.

Confidence Score: 3/5

This PR is not safe to merge until partial permission updates preserve omitted inverse restrictions instead of deleting them.

The new mirror-clear branch treats an omitted permission flag as an explicit clear, which can remove an inverse field’s update restriction and allow a relation join-column update that should remain denied.

Files Needing Attention: packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts

Security Review

A partial permission update can now clear an omitted inverse update restriction because the mirror-clear condition treats undefined as equivalent to explicit null. This can remove the join-column permission check and allow relation updates that should remain restricted.

Important Files Changed

Filename	Overview
packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts	Adds mirrored grant-back deletion and empty-row suppression, but conflates omitted and explicitly cleared flags during partial updates.
packages/twenty-front/src/modules/object-record/read-only/utils/isOneToManyRelationFieldReadOnlyDueToTargetUpdatePermission.ts	Correctly checks the target object's inverse field metadata ID against its field-level update restriction.
packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/tests/field-permissions.service.spec.ts	Covers complete grant-back and absent-row behavior but does not exercise partial null/undefined permission updates.
packages/twenty-front/src/modules/object-record/read-only/utils/tests/isOneToManyRelationFieldReadOnlyDueToTargetUpdatePermission.test.ts	Adds focused coverage for inverse-field and unrelated-field update restrictions.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A["Relation field permission input"] --> B["Build desired source permission"]
  B --> C["Mirror onto inverse relation field"]
  C --> D{"Both effective flags cleared?"}
  D -- Yes --> E["Delete permission row"]
  D -- No --> F["Create or update permission row"]
  E --> G["Recompute workspace permission cache"]
  F --> G
  G --> H["Frontend read-only check and server update enforcement"]

Loading

_{Reviews (1): Last reviewed commit: "Fix orphaned mirrored field permission b..." | Re-trigger Greptile}

[greptile-apps]

Partial updates clear inverse restrictions

When a partial relation-field permission update clears one flag with null and omits the other with undefined, sourceIsCleared treats both flags as cleared and writes null for the omitted inverse flag, deleting its existing update restriction and allowing relation join-column updates that should remain denied.

How this was verified: The optional input flags were traced through the mirror-clear branch and empty-row deletion to the join-column field-permission check.

Suggested change

	const sourceIsCleared =
	!isDefined(fieldPermission.canReadFieldValue) &&
	!isDefined(fieldPermission.canUpdateFieldValue);
	const sourceIsCleared =
	fieldPermission.canReadFieldValue === null &&
	fieldPermission.canUpdateFieldValue === null;

Knowledge Base Used: Metadata System and Workspace ORM (twenty-orm)

[FelixMalfait]

Not applying this: the premise conflicts with the service's pre-existing contract for the source row.

An input row with {canReadFieldValue: null, canUpdateFieldValue: undefined} is already treated as a full clear by the unchanged bothNull handling earlier in upsertFieldPermissions: both flags count as cleared whether they are null or undefined, the row is excluded from desiredMap, and the deletion loop removes the source permission row entirely, including its update restriction. That input shape means "remove this field's permission row", not "clear read, keep update".

sourceIsCleared deliberately uses the same predicate so the mirrored row follows the source row. With the suggested === null && === null, that same input would delete the source row but leave the inverse row restricted, which is exactly the orphaned-mirror state this PR fixes, reachable through a different input encoding.

A caller who wants to clear read while keeping the update restriction sends {canReadFieldValue: null, canUpdateFieldValue: false} (this is what the role settings UI sends). That keeps sourceIsCleared false and propagates {null, false} to the inverse field, preserving its update restriction.

---

Generated by Claude Code

[greptile-apps]

That's a well-reasoned explanation — thank you for tracing it through bothNull. You're right: since {canReadFieldValue: null, canUpdateFieldValue: undefined} is already treated as a full row deletion by the earlier bothNull branch, sourceIsCleared must use the same predicate (!isDefined) so the mirror row follows the source. My === null && === null suggestion would have reintroduced the orphaned-mirror case for exactly that input shape. The current implementation is correct.

[cubic-dev-ai]

1 issue found across 4 files

_{You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts">

<violation number="1" location="packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts:514">
P1: When a relation permission clears one flag while leaving the other `undefined`, `sourceIsCleared` treats `undefined` as cleared and clears both inverse flags. This can remove the inverse update restriction and re-enable join-column updates; only treat explicit `null` values as cleared.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: When a relation permission clears one flag while leaving the other undefined, sourceIsCleared treats undefined as cleared and clears both inverse flags. This can remove the inverse update restriction and re-enable join-column updates; only treat explicit null values as cleared.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts, line 514:

<comment>When a relation permission clears one flag while leaving the other `undefined`, `sourceIsCleared` treats `undefined` as cleared and clears both inverse flags. This can remove the inverse update restriction and re-enable join-column updates; only treat explicit `null` values as cleared.</comment>

<file context>
@@ -476,11 +505,24 @@ export class FieldPermissionService {
+      // Otherwise the mirrored row survives as an orphan that keeps blocking
+      // updates on the relation's join column after the admin granted the field back.
+      const sourceIsCleared =
+        !isDefined(fieldPermission.canReadFieldValue) &&
+        !isDefined(fieldPermission.canUpdateFieldValue);
+
</file context>

Suggested change

	!isDefined(fieldPermission.canReadFieldValue) &&
	!isDefined(fieldPermission.canUpdateFieldValue);
	fieldPermission.canReadFieldValue === null &&
	fieldPermission.canUpdateFieldValue === null;

[FelixMalfait]

Same finding as the thread above, same answer: {null, undefined} already deletes the entire source permission row under the pre-existing bothNull handling in this method (unchanged here), so treating undefined differently in sourceIsCleared would delete the source row while keeping the inverse restricted, recreating the orphaned-mirror bug this PR fixes. Partial clears that keep the update restriction are expressed as {canReadFieldValue: null, canUpdateFieldValue: false} and are unaffected by this branch.

---

Generated by Claude Code

[twenty-ci-bot-public]

✅ Standard review · no findings

Safe to merge — no outstanding findings

High-level — Focused single-purpose bug fix (mirror relation-field permission clears to the inverse field, delete emptied rows, align FE one-to-many read-only with inverse field-level update restriction), well under size budget, reuses existing seams, and ships FE unit + BE integration regression tests.
Low-level — Prior DRY duplication is now extracted into toUniversalFlatFieldPermissionToDelete, the added comments are justified WHY notes on the grant-back/mirror mechanics, and no new let/as/manual-nullish/signature violations were introduced.

---

_{Reviewed against the pr-review standard — high-level then low-level. Advisory; human review still required. Run details.}

[cubic-dev-ai]

All reported issues were addressed across 2 files (changes from recent commits).

_{You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[twenty-ci-bot-public]

🚀 Preview Environment Ready!

Your preview environment is available at: https://dts-example-routes-enabled.trycloudflare.com

This environment will automatically shut down after 5 hours.

[twenty-ci-bot-public]

🔍 Automated Pre-Review

✅ No issues detected - This PR is ready for human review.

---

View details

Automated pre-review — human approval still required.

[twenty-ci-bot-public]

🔍 Visual Regression Review — twenty-front

✅ 2 visual change(s) reviewed — all explained by this PR.

Changed: 2 · Added: 0 · Removed: 0 · Unchanged: 751

2 item(s) to double-check (uncertain / low confidence)

	Story	Verdict	Confidence	Explained by
🟡	ui-data-field-input-richtextfieldinput--default	uncertain	72%	No changed file in this PR touches RichTextFieldInput or any rich-text component…
🟡	modules-pagelayout-widgets-graphwidgetpiechart--default	uncertain	65%	No changed file in this PR touches GraphWidgetPieChart or any page-layout widget…

Changed stories

Story	Diff %
ui-data-field-input-richtextfieldinput--default	17%
modules-pagelayout-widgets-graphwidgetpiechart--default	7%

---

View run details · advisory mode

[twenty-eng-sync]

Auto-closed: this PR is draft and has had no update in over 24 hours. Please reopen it once you have the bandwidth to take it forward.