fix(lint): the controlledByParent owner arm prescribes ownerMayEdit only for an owner_id-keyed policy

.changeset/26591-cbp-owner-lint-owner-id-only.md

@@ -0,0 +1,16 @@
+---
+'@objectstack/lint': patch
+'@objectstack/spec': patch
+---
+
+fix(lint): `security-controlled-by-parent-owner-policy-without-opt-in` prescribes `ownerMayEdit` only for a policy keyed on `owner_id` (#26591)
+
+Clause-②: no (lint advice is not a contract face; no accepted metadata shape moves)
+Metadata-protocol: none
+
+`sharing.controlledByParent.ownerMayEdit: true` admits one caller past a `controlled_by_parent` detail's master gate: the user in the stored row's `owner_id`, on a detail that carries that column. The lint arm used to prescribe the opt-in for any `update` / `all` policy of the form `FIELD == current_user.id`. So an app whose accountable person is its own user field (`owner == current_user.id`) declared the opt-in, lint went quiet, and that user was still refused with `403 requires edit access to its master record`.
+
+- The opt-in is still prescribed, as before, for a policy keyed on `owner_id` on a detail that carries it and has not opted in, and the arm stays silent once it has.
+- A policy keyed only on another user field is now flagged whether or not the detail opted in. The verdict says the opt-in admits only the stored `owner_id`. The hint never names the opt-in: it says to key the accountable person on `owner_id`, or drop the policy.
+- On a detail with no `owner_id` (`ownership: 'business_unit' | 'org' | 'none'` and no `owner_id` field of its own), the opt-in names nobody. A policy keyed on `owner_id` there gets the same treatment, and the hint says to give the detail an `owner_id` column.
+- The runtime gate is unchanged. The rule's `os explain` entry covers both cases. The `ownerMayEdit` evidence line in `@objectstack/spec`'s liveness ledger now describes the narrowed arm.

packages/lint/src/rule-explanations.ts

@@ -713,14 +713,20 @@ const SECURITY_CBP_OWNER_POLICY_WITHOUT_OPT_IN_EXPLANATION: RuleExplanation = {
       'platform\'s injected per-record owner — updates that row by id with READ access to the master, ' +
       'and the detail\'s own policies then decide as authored. Everyone else, and every insert, delete, ' +
       'multi-row update and on-behalf-of write, still needs edit access to the master. The opt-in ' +
-      'reads `owner_id` and nothing else: a policy keyed on another field stays a narrowing even ' +
-      'after it, so key the policy on `owner_id` or declare what the other field means in prose.',
+      'reads the stored `owner_id` and nothing else, and only on a detail that carries that column ' +
+      '(`ownership: \'user\'`, the default; never `\'business_unit\'`, `\'org\'` or `\'none\'`). So the ' +
+      'rule prescribes the opt-in only for a policy that keys the caller on `owner_id` there. A policy ' +
+      'keyed only on another user field — `owner == current_user.id`, where `owner` is the detail\'s own ' +
+      'accountable-person field — stays a narrowing with or without the opt-in, and the rule says so in ' +
+      'both states instead of prescribing an opt-in that would not admit that user.',
     'This is a warning, not an error: a narrowing can be meant (only the owner among the master\'s ' +
-      'editors may update), and no runtime door refuses the shape. To fix it, declare the opt-in on ' +
-      'the detail when the owner is meant to write with read on the master, or drop the policy when ' +
-      'the master gate alone is the intended rule. The opt-in is refused by the parse on an object ' +
-      'whose `sharingModel` is not `controlled_by_parent`, so it cannot be declared where nothing ' +
-      'reads it.',
+      'editors may update), and no runtime door refuses the shape. To fix a policy keyed on `owner_id`, ' +
+      'declare the opt-in on the detail when the owner is meant to write with read on the master, or ' +
+      'drop the policy when the master gate alone is the intended rule. To fix a policy keyed on another ' +
+      'user field, keep the accountable person in `owner_id` (the column the master gate compares the ' +
+      'caller against) and key the policy on it, or drop the policy. The opt-in is refused by the parse ' +
+      'on an object whose `sharingModel` is not `controlled_by_parent`, so it cannot be declared where ' +
+      'nothing reads it.',
   ],
 };
 

packages/lint/src/validate-security-posture-owner-may-edit.test.ts

@@ -167,6 +167,89 @@ describe('security-controlled-by-parent-owner-policy-without-opt-in (#25760)', (
     });
   });
 
+  // [#26591] The opt-in admits ONE column — the stored row's `owner_id`, on a
+  // detail that carries it — so the arm prescribes it only there. hotclm's
+  // shape: the accountable person is the detail's own user field `owner`, and
+  // the requester's update policy keys on it.
+  describe('the opt-in is prescribed only where it admits (#26591)', () => {
+    const OWNER_FIELD = { owner: { name: 'owner', type: 'user', reference: 'sys_user', label: 'Owner' } };
+    /** hotclm's `clm_obligation`: a `controlled_by_parent` detail with its own `owner` user field. */
+    const hotclmDetail = (extra: Record<string, unknown> = {}) =>
+      detail({ fields: { ...DETAIL_FIELDS, ...OWNER_FIELD }, ...extra });
+    const ownerPolicy = policy({ name: 'obligation_requester_own', using: 'owner == current_user.id' });
+
+    it('an owner_id-keyed policy without the opt-in: flagged, and the opt-in is the remedy', () => {
+      const findings = ownerArm(stack([MASTER, hotclmDetail()], policy()));
+      expect(findings).toHaveLength(1);
+      expect(findings[0].hint).toContain('Declare sharing: { controlledByParent: { ownerMayEdit: true } }');
+    });
+
+    it('an owner_id-keyed policy with the opt-in: silent', () => {
+      expect(ownerArm(stack([MASTER, hotclmDetail(OPT_IN)], policy()))).toEqual([]);
+    });
+
+    it('an other-field policy without the opt-in: flagged with the owner_id remedy, never the opt-in', () => {
+      const findings = ownerArm(stack([MASTER, hotclmDetail()], ownerPolicy));
+      expect(findings).toHaveLength(1);
+      expect(findings[0]).toMatchObject({
+        severity: 'warning',
+        rule: SECURITY_CBP_OWNER_POLICY_WITHOUT_OPT_IN,
+        path: 'permissions[0].rowLevelSecurity[0].using',
+      });
+      expect(findings[0].message).toContain('(owner == current_user.id)');
+      expect(findings[0].message).toContain("ownerMayEdit admits only the row's stored owner_id");
+      expect(findings[0].hint).toContain('Key the accountable person on owner_id');
+      expect(findings[0].hint).not.toContain('ownerMayEdit');
+    });
+
+    it('an other-field policy with the opt-in: still flagged, the opt-in does not admit that field', () => {
+      const findings = ownerArm(stack([MASTER, hotclmDetail(OPT_IN)], ownerPolicy));
+      expect(findings).toHaveLength(1);
+      expect(findings[0].message).toContain('(owner == current_user.id)');
+      expect(findings[0].message).toContain("the opt-in it declares (sharing.controlledByParent.ownerMayEdit) admits only the row's stored owner_id");
+      expect(findings[0].hint).toContain('Key the accountable person on owner_id');
+      expect(findings[0].hint).not.toContain('ownerMayEdit');
+    });
+
+    it('a predicate keying owner_id beside another field is one the opt-in serves: prescribed without it, silent with it', () => {
+      const mixed = policy({ using: 'owner == current_user.id || owner_id == current_user.id' });
+      const findings = ownerArm(stack([MASTER, hotclmDetail()], mixed));
+      expect(findings).toHaveLength(1);
+      expect(findings[0].hint).toContain('ownerMayEdit: true');
+      expect(ownerArm(stack([MASTER, hotclmDetail(OPT_IN)], mixed))).toEqual([]);
+    });
+
+    describe('a detail that carries no owner_id: the opt-in names nobody there', () => {
+      it.each([
+        ['ownership: org', { ownership: 'org' }],
+        ['ownership: none', { ownership: 'none' }],
+        ['ownership: business_unit', { ownership: 'business_unit' }],
+      ])('%s — an owner_id-keyed policy is flagged with the owner-column remedy, with or without the opt-in', (_label, ownership) => {
+        for (const extra of [ownership, { ...ownership, ...OPT_IN }]) {
+          const findings = ownerArm(stack([MASTER, detail(extra)], policy()));
+          expect(findings, JSON.stringify(extra)).toHaveLength(1);
+          expect(findings[0].message).toContain('a column "omd_obligation" does not carry');
+          expect(findings[0].hint).toContain('Give "omd_obligation" an owner_id column');
+          expect(findings[0].hint).not.toContain('ownerMayEdit');
+        }
+      });
+
+      it("an author's own owner_id field is the column the runtime reads: the opt-in is prescribed, then silent", () => {
+        const ownOwner = { ownership: 'none', fields: { ...DETAIL_FIELDS, owner_id: { name: 'owner_id', type: 'user', reference: 'sys_user', label: 'Owner' } } };
+        expect(ownerArm(stack([MASTER, detail(ownOwner)], policy()))[0]?.hint).toContain('ownerMayEdit: true');
+        expect(ownerArm(stack([MASTER, detail({ ...ownOwner, ...OPT_IN })], policy()))).toEqual([]);
+      });
+    });
+
+    it('the hotclm fixture is key-legal: the stack schema refuses no key it carries', () => {
+      const result = ObjectStackSchema.safeParse({
+        manifest: { id: 'com.example.omd', name: 'omd', version: '1.0.0', type: 'app' },
+        ...stack([MASTER, hotclmDetail(OPT_IN)], ownerPolicy),
+      });
+      expect(result.success ? [] : result.error.issues.filter((i) => i.code === 'unrecognized_keys')).toEqual([]);
+    });
+  });
+
   describe('the parsed path reads what the raw path reads', () => {
     it('the PARSED detail and set: flagged without the opt-in, silent with it', () => {
       const parsedSet = PermissionSetSchema.parse({ name: 'omd_requester', objects: {}, rowLevelSecurity: [policy()] });

packages/lint/src/validate-security-posture.test.ts

@@ -1579,6 +1579,14 @@ const NOT_SCHEMA_RECEIVERS: Record<string, string> = {
   // hint text. The scan reads code with its string literals intact, so the
   // spelling presents as a receiver `current_user` reading a key `id`.
   current_user: "the RLS predicate grammar's `current_user.id`, a string literal in the owner arm's message text — not a receiver.",
+  // [#26591] The owner arm keeps, per controlled_by_parent detail, whether it
+  // opted in and whether it carries `owner_id` — this file's own fold; the
+  // authored reads behind it are `controlledByParentOwnerMayEdit(obj)` and
+  // `obj.fields`. And `sharing.controlledByParent.ownerMayEdit` is quoted as
+  // a string literal in the off-column verdict text, so it presents as a
+  // receiver `sharing` reading a key `controlledByParent`.
+  detail: "the owner arm's own `{ optedIn, ownerColumn }` fold of one detail, not an authored surface.",
+  sharing: "the opt-in's spelling `sharing.controlledByParent.ownerMayEdit`, a string literal in the owner arm's message text — not a receiver.",
 };
 
 const READ_SURFACES: Array<{ receiver: string; expected: string[]; declaredBy: string; keys: () => string[] }> = [
@@ -1781,7 +1789,7 @@ describe('validateSecurityPosture — reads only keys the spec declares (meta-te
       'bodies', 'entries', 'targets', 'owners', 'extensions', 'byCarrier', 'writers', 'tiedWriters',
       'merged', 'baseFields', 'blamed',
       // #25760: the controlled-by-parent owner arm's own collections and the predicate string.
-      'cbpDetailsWithoutOptIn', // the detail names without the opt-in — JS Set methods.
+      'cbpDetails', // #26591: the resolving details by name — JS Map methods.
       'policies', // one set's `rowLevelSecurity` records — `.length`, a JS property.
       'predicate', // a policy's `using` / `check` text, a string — `.trim`.
     ]);
@@ -2010,6 +2018,29 @@ const REACHABILITY_CORPUS: Array<{ label: string; stack: Record<string, unknown>
       }],
     },
   },
+  {
+    // [#26591] The same rule's second push site: an opted-in detail whose
+    // `update` policy keys the caller on its own user field, which the opt-in
+    // (the stored `owner_id` only) does not admit.
+    label: 'cbp-caller-policy-off-owner-column',
+    stack: {
+      objects: [
+        objectFixture({ name: 'invoice', sharingModel: 'private' }),
+        objectFixture({
+          name: 'line_item', sharingModel: 'controlled_by_parent',
+          sharing: { controlledByParent: { ownerMayEdit: true } },
+          fields: {
+            invoice: { type: 'master_detail', label: 'Invoice', reference: 'invoice', required: true },
+            assignee: { type: 'user', label: 'Assignee', reference: 'sys_user' },
+          },
+        }),
+      ],
+      permissions: [{
+        name: 'ps', label: 'PS', objects: {},
+        rowLevelSecurity: [{ name: 'assigned_lines', object: 'line_item', operation: 'update', using: 'assignee == current_user.id', enabled: true }],
+      }],
+    },
+  },
   {
     label: 'grant-expired-at-authoring',
     stack: { data: [{ object: 'sys_user_position', records: [{ user_id: 'u1', position: 'p', valid_until: '2020-01-01T00:00:00Z' }] }] },
@@ -2033,7 +2064,8 @@ describe('validateSecurityPosture — every branch is reachable without an undec
     // 16 since #23268: the external dial's alias and wider-than-internal sites left with the key.
     // 17 since #25575: allowDelete: false beside modifyAllRecords: true.
     // 18 since #25760: an owner-keyed update policy on a controlled_by_parent detail that has not opted in.
-    expect(pushedRuleIds()).toHaveLength(18);
+    // 19 since #26591: the same rule id, for a policy keyed only on a column the opt-in does not admit.
+    expect(pushedRuleIds()).toHaveLength(19);
   });
 
   it('reaches every `findings.push` site from that corpus', () => {