fix(objectql): the create-side static-readonly strip judges the user-writable managedBy buckets (#15719)

.changeset/readonly-create-side-bucket-exclusion-narrows.md

@@ -0,0 +1,94 @@
+---
+"@objectstack/objectql": minor
+"@objectstack/lint": minor
+---
+
+fix(objectql)!: the create-side static-`readonly` strip judges the user-writable `managedBy` buckets, as update already did (#15719)
+
+<!-- adr-0087: not-required (no-migration-prescription) an over-wide runtime exclusion is narrowed; no authored key changes spelling, meaning or legality, and `managedBy`'s own enum is untouched — there is nothing for `objectstack migrate meta` to prescribe -->
+
+**BREAKING** for a non-system caller that CREATES a static `readonly` column on an
+object declaring `managedBy: 'platform'`, `'config'` or `'system-data'` under a name
+outside the reserved `sys_` namespace: the forged value used to be persisted and is
+now stripped, with the field's own `defaultValue` re-derived (#3043) and the drop
+reported on the usual channels (`readonlyStripWarning` at `warn`, `onFieldsDropped`
+under reason `readonly`, `strictReadonlyWrites` refusing before any driver dispatch).
+That is exactly what the same caller's UPDATE of the same column already did. Shipped
+as `minor` under the repo's launch-window convention.
+
+## The census, both halves — neither one is the whole reading
+
+**(b) is greater than zero, so the affected objects are named.** 20 shipped objects sit
+in the three now-judged buckets and carry a static `readonly` column between them — 64
+columns in all:
+
+- `platform` (6 objects, 14 columns): `sys_attachment`, `sys_business_unit`,
+  `sys_business_unit_member`, `sys_comment`, `sys_report_schedule`, `sys_saved_report`
+- `config` (6 objects, 29 columns): `sys_capability`, `sys_email_template`,
+  `sys_permission_set`, `sys_position`, `sys_sharing_rule`, `sys_webhook`
+- `system-data` (8 objects, 21 columns): `sys_approval_delegation`,
+  `sys_notification_preference`, `sys_notification_subscription`,
+  `sys_notification_template`, `sys_position_permission_set`,
+  `sys_user_permission_set`, `sys_user_position`, `sys_user_preference`
+
+**And the shipped behaviour delta is ZERO.** Of the 81 object declarations in this tree
+carrying `managedBy`, **none** is named outside `sys_` — every one of the 20 above
+included — so the namespace test, which this change does not touch, keeps all of them
+exempt exactly as before. `sys_metadata_history.recorded_by`, seeded by a direct
+non-system `engine.insert` from the metadata repository, is doubly exempt
+(`engine-owned` bucket **and** `sys_`) and is pinned as such.
+
+⚠️ **Read both halves together.** "Behaviour-free" on its own overstates it — the
+population the narrowing reaches is real and named above, and an app that declares one
+of those buckets on its own object gets the strip. The population on its own
+understates it — not one shipped object changes behaviour on this release. What moves
+is the contract for **app-authored** objects, which is the population the ruling is
+about.
+
+## What was wrong
+
+`staticReadonlyInsertSubject` returned `null` for `managedBy` set to **anything**,
+carried over byte-for-byte from the deleted DataProtocol ingress copy on ADR-0086 /
+#3004 grounds: those columns have their own 403 guards, and a silent strip must not
+swallow the payload the guard exists to reject. The argument is sound and the bucket
+list was not. `managedBy: 'system-data'` means "platform-defined schema,
+**admin/user-writable data**" by its own definition, and `object.zod.ts` says in the
+same breath that it "carries no such guard; its writes are adjudicated by the
+delegated-admin gate / RLS / permission sets". So the create side skipped the strip on
+objects whose data is the user's, while the update side stripped them — and #14147's
+"one semantics, one enforcement point" was not literally true on that population.
+
+## What it does now
+
+The exclusion follows its reason. `null` is returned for the `sys_` namespace, and for
+the three buckets whose columns really do carry a fail-closed refusal:
+
+| bucket | its own refusal | the create-side strip |
+|:--|:--|:--|
+| `engine-owned` | ADR-0103 engine-owned write guard | steps around it |
+| `append-only` | ADR-0103, same guard (locked default) | steps around it |
+| `better-auth` | ADR-0092 identity write guard | steps around it |
+| `platform` | none — full user CRUD by default | judges it |
+| `config` | none — admin-authored, writable by default | judges it |
+| `system-data` | none — "admin/user-writable DATA" | judges it |
+
+An **unrecognised** bucket value is deliberately not read as platform-internal: the one
+legacy value that can still arrive is `'system'`, retired in protocol 17 (#3355) and
+converted to `'system-data'` — a judging bucket — so exempting unknowns would exempt
+precisely the rows that conversion targets. The partition is pinned against
+`@objectstack/spec`'s own enum, so a seventh bucket fails a test instead of landing
+silently on one side.
+
+The ruling's fallback ("leave it, if those buckets' readonly columns already carry
+their own 403") does not apply: of the 64 columns above, 14 are the ADR-0086
+package-provenance family (`package_id`, `managed_by`, `customized`, `drift_status`,
+`drift_detail`, `is_system`, all on `config` objects) and the other 50 are `id` /
+`created_at` / `updated_at` stamps, which that guard does not reach.
+
+`@objectstack/lint` mirrors this predicate to decide which objects its create-verb
+`flow-update-readonly-field` / `hook-api-update-readonly-field` findings may describe,
+and is narrowed in the same stroke — a lint that kept the wider exemption would go on
+suppressing findings for a strip that now really happens.
+
+⛔ The UPDATE path is untouched, and so is `beforeInsert`'s post-hook strip position.
+The asymmetry is closed by moving CREATE toward UPDATE.

packages/lint/src/validate-readonly-flow-writes.test.ts

@@ -465,14 +465,16 @@ describe('validateReadonlyFlowWrites', () => {
       expect(findings[0].where).toBe('flow "fan_out" › loop "Each" › body › node "C"');
     });
 
-    // The engine's create-side strip does not judge a PLATFORM object at all
-    // (`staticReadonlyInsertSubject`: `managedBy` set, or a `sys_` name — its
-    // own 403 write guard governs it), so a create finding there would
-    // describe a strip that never runs. The UPDATE path applies no such
-    // exclusion, which the update controls pin.
+    // The engine's create-side strip does not judge a PLATFORM-INTERNAL object
+    // at all (`staticReadonlyInsertSubject`: a `sys_` name, or one of the three
+    // `managedBy` buckets whose own 403 write guard governs it), so a create
+    // finding there would describe a strip that never runs. The UPDATE path
+    // applies no such exclusion, which the update controls pin.
     it.each([
       ['a sys_ object', { name: 'sys_audit_entry', fields: { verdict: { type: 'text', readonly: true } } }],
-      ['a managedBy object', { name: 'audit_entry', managedBy: 'engine-owned', fields: { verdict: { type: 'text', readonly: true } } }],
+      ['an engine-owned object', { name: 'audit_entry', managedBy: 'engine-owned', fields: { verdict: { type: 'text', readonly: true } } }],
+      ['an append-only object', { name: 'audit_trail', managedBy: 'append-only', fields: { verdict: { type: 'text', readonly: true } } }],
+      ['a better-auth object', { name: 'identity_row', managedBy: 'better-auth', fields: { verdict: { type: 'text', readonly: true } } }],
     ])('does NOT flag a create_record into %s — outside the create-side strip; the same update_record is still flagged', (_label, platformObject) => {
       const create = {
         name: 'seed_audit',
@@ -490,6 +492,27 @@ describe('validateReadonlyFlowWrites', () => {
       expect(control[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
     });
 
+    // [#15719] …and the other half of the same narrowing: an app-authored
+    // object in a USER-WRITABLE bucket IS judged on create now, so suppressing
+    // the finding would hide a strip that really happens.
+    it.each([
+      ['platform', 'deal_platform'],
+      ['config', 'deal_config'],
+      ['system-data', 'deal_system_data'],
+    ])('DOES flag a create_record into a `managedBy: %s` object — the strip reaches it', (bucket, objectName) => {
+      const userWritable = { name: objectName, managedBy: bucket, fields: { verdict: { type: 'text', readonly: true } } };
+      const create = {
+        name: 'seed_deal',
+        runAs: 'user',
+        nodes: [{ id: 'c', type: 'create_record', label: 'C', config: { objectName, fields: { verdict: 'ok' } } }],
+        edges: [],
+      };
+      const findings = validateReadonlyFlowWrites({ objects: [userWritable], flows: [create] });
+      expect(findings).toHaveLength(1);
+      expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
+      expect(findings[0].path).toBe('flows[0].nodes[0].config.fields.verdict');
+    });
+
     it('skips a templated objectName and a non-literal fields map on create, as on update', () => {
       expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [createFlow({ approval_status: 'x' }, { runAs: 'user' }, { objectName: '{target}' })] })).toEqual([]);
       expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [createFlow('{payload}' as unknown as Record<string, unknown>, { runAs: 'user' })] })).toEqual([]);

packages/lint/src/validate-readonly-flow-writes.ts

@@ -149,26 +149,56 @@ export function buildReadonlyIndex(objects: AnyRec[]): Map<string, Map<string, F
   return idx;
 }
 
+/**
+ * The `managedBy` buckets the engine's create-side strip steps around, mirroring
+ * `PLATFORM_INTERNAL_MANAGED_BUCKETS` in
+ * `packages/objectql/src/validation/rule-validator.ts` (#15719). ⛔ Not derived
+ * from it: this package is dependency-free by design and objectql does not
+ * export the set, so the two are held equal by
+ * `validate-readonly-flow-writes.test.ts` naming the same three buckets and the
+ * engine's own tests pinning them against the `@objectstack/spec` enum.
+ *
+ * The split is the enum's own (`object.zod.ts`, "Enforcement happens in three
+ * places", item 3): these three fail closed on a user-context generic write —
+ * `better-auth` via ADR-0092, `engine-owned` / `append-only` via ADR-0103 —
+ * while `platform` / `config` / `system-data` are writable by default and carry
+ * no such guard.
+ */
+const PLATFORM_INTERNAL_MANAGED_BUCKETS: ReadonlySet<string> = new Set([
+  'engine-owned',
+  'append-only',
+  'better-auth',
+]);
+
 /**
  * Objects the engine's CREATE-side static strip does not judge at all — the
  * two object-level exclusions of `staticReadonlyInsertSubject`
- * (packages/objectql/src/validation/rule-validator.ts): a platform object
- * (`managedBy` set, or the reserved `sys_` namespace) carries its own
- * field-write governance (ADR-0086, a 403 guard) that a silent strip must not
- * pre-empt, so `engine.insert` runs no readonly strip on it. A create finding
- * on such an object would describe a strip that never happens.
+ * (packages/objectql/src/validation/rule-validator.ts): a platform-internal
+ * object (a {@link PLATFORM_INTERNAL_MANAGED_BUCKETS} bucket, or the reserved
+ * `sys_` namespace) carries its own field-write governance (ADR-0086, a 403
+ * guard) that a silent strip must not pre-empt, so `engine.insert` runs no
+ * readonly strip on it. A create finding on such an object would describe a
+ * strip that never happens.
+ *
+ * ⚠️ A USER-WRITABLE bucket (`platform`, `config`, `system-data`) is NOT exempt
+ * — #15719 narrowed this from "`managedBy` set to anything" so the strip reaches
+ * the buckets whose data is the user's. Leaving that narrowing out here would
+ * keep suppressing create findings for a strip that now really happens, which is
+ * exactly the silent disagreement this shared helper exists to prevent.
  *
  * ⚠️ Create-verb ONLY. The UPDATE path applies neither exclusion (stated at
- * that function: "This asymmetry is the create side's"), so the update branch
- * of both rules keeps judging these objects. Shared with
+ * that function: "That remaining asymmetry is the create side's"), so the update
+ * branch of both rules keeps judging these objects. Shared with
  * `validate-readonly-hook-writes.ts` for the same reason the index above is.
  */
 export function buildInsertStripExemptObjects(objects: AnyRec[]): Set<string> {
   const exempt = new Set<string>();
   for (const obj of objects) {
     const name = typeof obj.name === 'string' ? obj.name : undefined;
     if (!name) continue;
-    if (obj.managedBy || name.startsWith('sys_')) exempt.add(name);
+    const internalBucket = typeof obj.managedBy === 'string'
+      && PLATFORM_INTERNAL_MANAGED_BUCKETS.has(obj.managedBy);
+    if (internalBucket || name.startsWith('sys_')) exempt.add(name);
   }
   return exempt;
 }

packages/lint/src/validate-readonly-hook-writes.test.ts

@@ -343,13 +343,15 @@ describe('validateReadonlyHookWrites - GREEN: what a create is NOT judged on', (
   // be false in exactly the way the sudo() hint used to be (#14010). Since
   // #16249 the shape does not arrive at all: the ledger no longer advertises it
   // and the build refuses `.create(` at lowering.
-  // The engine's create-side strip does not judge a PLATFORM object at all
-  // (`staticReadonlyInsertSubject`: `managedBy` set, or a `sys_` name — its
-  // own 403 write guard governs it); the UPDATE path applies no such
-  // exclusion. Pinned in both directions per object shape.
+  // The engine's create-side strip does not judge a PLATFORM-INTERNAL object at
+  // all (`staticReadonlyInsertSubject`: a `sys_` name, or one of the three
+  // `managedBy` buckets whose own 403 write guard governs it); the UPDATE path
+  // applies no such exclusion. Pinned in both directions per object shape.
   it.each([
     ['a sys_ object', { name: 'sys_activity', fields: { verdict: { type: 'text', readonly: true } } }],
-    ['a managedBy object', { name: 'activity', managedBy: 'append-only', fields: { verdict: { type: 'text', readonly: true } } }],
+    ['an append-only object', { name: 'activity', managedBy: 'append-only', fields: { verdict: { type: 'text', readonly: true } } }],
+    ['an engine-owned object', { name: 'run_row', managedBy: 'engine-owned', fields: { verdict: { type: 'text', readonly: true } } }],
+    ['a better-auth object', { name: 'identity_row', managedBy: 'better-auth', fields: { verdict: { type: 'text', readonly: true } } }],
   ])('never flags insert() into %s — outside the create-side strip; the same update() is still flagged', (_label, platformObject) => {
     const hook = (source: string) => ({
       objects: [platformObject],
@@ -361,6 +363,24 @@ describe('validateReadonlyHookWrites - GREEN: what a create is NOT judged on', (
     expect(control[0].rule).toBe(HOOK_API_UPDATE_READONLY_FIELD);
   });
 
+  // [#15719] The other half: a USER-WRITABLE bucket on an app-authored name is
+  // judged on create now, so the insert() write is a real finding.
+  it.each([['platform'], ['config'], ['system-data']])(
+    'DOES flag insert() into a `managedBy: %s` object — the strip reaches it',
+    (bucket) => {
+      const userWritable = { name: 'deal_row', managedBy: bucket, fields: { verdict: { type: 'text', readonly: true } } };
+      const findings = validateReadonlyHookWrites({
+        objects: [userWritable],
+        hooks: [{
+          name: 'log', object: 'crm_case', events: ['afterInsert'],
+          body: { language: 'js', source: "await ctx.api.object('deal_row').insert({ verdict: 'ok' });" },
+        }],
+      });
+      expect(findings).toHaveLength(1);
+      expect(findings[0].rule).toBe(HOOK_API_UPDATE_READONLY_FIELD);
+    },
+  );
+
   it('never flags create() — the shape cannot reach this rule at all since #16249', () => {
     expect(
       validateReadonlyHookWrites(