mirror of
https://github.com/twentyhq/twenty.git
synced 2026-09-16 16:06:17 -04:00
martmull
85e02f6a4c
Follow-up to #25439 and #25914 (both merged), now based on `main`. Two review findings on the inheritance gate: a parent only asked for a non-null key or a share row, never for the reader's permission on the parent object nor their row-level restrictions, so a member blocked from reading a person by their role still read the person's notes and attachments through it; and the event path (subscriptions, webhooks, workflows, logic functions) kept treating every INHERITED record as open, a second interpretation of the policy next to the SQL one. Inert until a parent object is PRIVATE and `IS_RECORD_SHARING_ENABLED` is on. ## What changes - One row access policy builder, `buildRowAccessPolicy`, composes for an alias the role's permission on the object, its row-level predicate and the record share gate (OPEN, PRIVATE share rows, INHERITED parents, SYSTEM and APPLICATION denials). Direct queries go through it for their root and joined aliases, and so do the column parents and child rows of an INHERITED object: a gated parent is correlated through the parent row under that policy, `(fk IS NOT NULL AND EXISTS (SELECT 1 FROM parent p WHERE p.id = fk AND <policy>))`, a denied parent grants nothing. A parent the reader may not query grants nothing on a child. - The builder takes a subject, the pieces of an identity the policy depends on: object permissions, share principals, owning application, row-level filter. The repository derives it from its auth context; the object permission predicate moved to a util the query-level validation shares, so the API keeps reporting a denied root object with the error it documents. - `RecordAccessPolicyService` evaluates that policy for a subject holding no auth context, in SQL on a system-context repository. For an INHERITED record's events it decides which of the records the event snapshot points at, and which live child rows point back at it, the subject may read; the snapshot stands in for the record so a destroyed one is still decided on. The record's own share rows stay evaluated in memory as before. - A deleted record's links are captured with its deletion: a note's targets are soft-deleted with it and cascade away when it is destroyed, so the repository reads the child rows an INHERITED record inherits through before a soft delete or a destroy, and the `deleted` and `destroyed` events carry them as `inheritedReadabilityChildRecords`, a server-side transit property stripped before any client payload. The service evaluates those captured rows as snapshots under the subject's policy (object permission, row-level filter, the child's own share rows or parents), so the readers who could see the record through its links still receive its deletion. Subscriptions and logic functions strip the capture before handing the event over; webhooks and workflows only ever took `before` and `after`. - The query gate applies the same rule to a trashed record: a child link counts when it is live, or when it was trashed at or after the record itself. A note soft-deleted with its targets stays visible in the trash, and restorable, for the readers who saw it live; a target detached before the deletion still grants nothing. - A soft delete through the ORM now touches live rows only, unless the builder opted into `withDeleted`, matching the rows its own snapshot select already returned. The target cascades restamped targets detached earlier with the current time, which the rule above would have read as a link trashed with the note. - The note and task target hooks, which trash and restore the targets with their note or task, now run with permission checks bypassed. They cascade a mutation the actor was already permitted on the parent, and the repository they built without a role config carried no object permission at all, which the inherited gate reads as denied on every parent: with the flag on, they trashed nothing. - Subjects per consumer: a subscriber's object permissions, principals and row-level filter; the standard application's default role for workflows; the application's default role for logic functions; everyone alone for webhooks, which carry no identity. `resolveRecordShareGateKind` returns `inherited` for an INHERITED object and the gate carries the record ids readable through parents next to the share rows; `isRecordSharedWithPrincipals` became `isRecordAdmittedByRecordShareGate`. - The subscription publisher checks that a stream has a query on the event's object before building its gate, so a batch no longer costs a parent lookup per stream subscribed to other objects. ## Things to know - A gated parent now costs one correlated EXISTS on the parent table per candidate row instead of a share-row lookup keyed by the foreign key, since the parent's predicate needs the parent row. - An INHERITED object's events cost, per subject and per batch, one gated query per parent object and per child object of that batch, and a soft delete or destroy of such a record costs one read of its child rows per child object at write time. - The timeline entries exposing a private linked record's title (#25454) remain a prerequisite before activation, as does a backfill of share rows for the notes and tasks that exist unattached before the flag turns on (they hold no parent and no share row, so the gate would hide them from everyone). ## Tests - Unit: `buildRowAccessPolicy` composition (open, denied by object permission, role predicate alone, share rows, parents under their own predicate, a parent the subject may not read dropped, owning application), the condition builder on policy-shaped parents including the trashed-with-the-record rule, the gate builder on INHERITED, the gate kind; the mutation builder keeping a soft delete off trashed rows unless `withDeleted`; the event formatter carrying the captured child rows on the deleted and destroyed events of their record only; the publisher skipping the parent lookup for a stream on another object and stripping the capture from what it broadcasts; the logic function payload without the capture. - Integration, in the children spec: a member whose role cannot read people no longer sees the private person's notes, targets and attachments through it even with a share row; the event gate resolves, for the member's subject, exactly the notes the member's query returns; a note on the private person soft-deleted by the admin (who reaches it through its company target) has its targets trashed with it, is unreadable through its live links and readable through the captured ones, and the admin restores it through the target trashed with it; a note deleted after one of its targets was detached stays out of the member's trash. In the note target hooks spec: a target detached before its note keeps its own deletion time when the note is deleted.