fix(plugin-security): a public_read_write OWD opens row-level writes, not just the creator's (#8023)

.changeset/owd-public-read-write-opens-row-writes.md

@@ -0,0 +1,50 @@
+---
+"@objectstack/plugin-security": patch
+---
+
+fix(plugin-security): a `public_read_write` object is writable by everyone the access matrix grants `edit`, not only by each row's creator (#8023)
+
+An object declaring `sharingModel: 'public_read_write'` promised "everyone can
+see and edit" and delivered "everyone can see, only the creator can edit". A
+persona the access matrix grants `edit: true` could `GET` a row **200** and
+`PATCH` the same row **403 PERMISSION_DENIED**, with the record-level refusal
+("You do not have access to this record…"). Three declarations agreed the write
+was allowed — the access matrix's `edit: true`, the object's OWD, and the
+absence of any authored RLS — and the runtime refused it anyway.
+
+The cause is the platform's own row-level write ownership floor.
+`member_default` ships `owner_only_writes` (object `'*'`, operation `update`,
+`created_by == current_user.id`, positions `['org_member']`). The by-id write
+pre-image gate lets `ISharingService`'s tri-state verdict **replace** that floor,
+but only on a positive `allow` — and on a public object the service **abstains**,
+because record sharing genuinely does not enforce there. An abstain keeps the
+floor, so the floor became the object's only row-level write gate and quietly
+overrode its OWD.
+
+An object whose author declared `public_read_write` now never inherits the
+wildcard `update` floor in the first place. Three boundaries are deliberate:
+
+- **`delete` is unchanged.** `public_read_write` is "see and edit"; the legacy
+ `full` alias that also covered transfer/delete was refused a mechanical
+ conversion for being *wider* than it (ADR-0090 D4). `owner_only_deletes` still
+ refuses a non-creator delete.
+- **Only the OWD that says so.** The declared model is read, never
+ `plugin-sharing`'s effective bucket — which folds `controlled_by_parent` and an
+ unset model on a system object into the same `'public'` value. A detail object
+ derives access from its master, and an unset model on a `sys_*` table is a
+ legacy default, so neither opens writes. An unresolvable schema fails closed.
+- **Only the platform's floor.** Provenance decides, so an app-authored policy
+ spelling the identical predicate still reaches the compiler and still refuses
+ (ADR-0049).
+
+Because the floor is removed at collection time, the write class is then empty
+and the derive-from-select scope supplies the write filter — so "you cannot
+mutate what you cannot see" continues to hold on these objects: a caller
+narrowed by select-only RLS still cannot write a row outside its readable set.
+
+Objects with any other OWD are untouched: on `private` and `public_read`, a
+non-owner write is still refused. The object-level gate is untouched too — a
+persona with `edit: false` still gets the object-level refusal, with its own
+distinct sentence. `POST /api/v1/security/explain` follows the same composition,
+so it stops reporting the `rls` layer as `narrows` for `update` on an object
+with zero authored RLS, while continuing to report a narrowing for `delete`.

packages/plugins/plugin-security/src/platform-ownership-policies.test.ts

@@ -12,7 +12,10 @@ import { describe, it, expect } from 'vitest';
 import {
  isPlatformOwnershipFloorPolicy,
  platformOwnershipFloorPolicyCount,
+ owdDeclaresOpenRowWrites,
+ owdOpenWritesCoversOperation,
  OWNERSHIP_FLOOR_PREDICATE,
+ OWD_OPENING_ROW_WRITES,
 } from './platform-ownership-policies.js';
 import { defaultPermissionSets } from './objects/default-permission-sets.js';
 
@@ -95,3 +98,51 @@ describe('[#5492] platform ownership-floor provenance', () => {
  expect(writeClass.length).toBe(platformOwnershipFloorPolicyCount());
  });
 });
+
+// [#8023] The OWD condition on the floor. The headline case is proven
+// end-to-end over HTTP by
+// `packages/qa/dogfood/test/owd-public-read-write-write-floor.dogfood.test.ts`;
+// what belongs HERE is the vocabulary boundary — which spellings open writes
+// and which look like they might but must not.
+describe('[#8023] owdDeclaresOpenRowWrites — the DECLARED model, never the effective bucket', () => {
+ it('the one canonical spelling opens writes, in both the flat and the nested spot', () => {
+ expect(owdDeclaresOpenRowWrites({ sharingModel: 'public_read_write' })).toBe(true);
+ expect(owdDeclaresOpenRowWrites({ security: { sharingModel: 'public_read_write' } })).toBe(true);
+ expect(OWD_OPENING_ROW_WRITES).toBe('public_read_write');
+ });
+
+ it('every OWD that does NOT declare open writes keeps the floor', () => {
+ for (const model of ['private', 'public_read', 'controlled_by_parent']) {
+ expect(owdDeclaresOpenRowWrites({ sharingModel: model }), `${model} must keep the floor`).toBe(false);
+ }
+ });
+
+ it('⚠️ `controlled_by_parent` and an UNSET model on a system object must NOT open writes', () => {
+ // Both collapse into plugin-sharing's `'public'` bucket
+ // (`effectiveSharingModel`), which is why reading THAT here would have been
+ // the bug: `controlled_by_parent` derives access from its master (which has
+ // its own OWD and its own gate) and declares nothing about its own writers,
+ // while an unset model on a `sys_*` table is a legacy default rather than
+ // an author's statement — opening writes there would hand every member
+ // cross-creator writes on the platform's identity tables.
+ expect(owdDeclaresOpenRowWrites({ name: 'owdw_detail', sharingModel: 'controlled_by_parent' })).toBe(false);
+ expect(owdDeclaresOpenRowWrites({ name: 'sys_user_position', isSystem: true })).toBe(false);
+ expect(owdDeclaresOpenRowWrites({ name: 'sys_user_position' })).toBe(false);
+ });
+
+ it('an unresolvable or malformed schema fails CLOSED (the floor stays)', () => {
+ for (const schema of [null, undefined, {}, { sharingModel: null }, { sharingModel: 'read_write' }]) {
+ expect(owdDeclaresOpenRowWrites(schema)).toBe(false);
+ }
+ // `read_write` above is the ADR-0090 D4 LEGACY alias. It no longer parses at
+ // authoring, and a stored value the platform does not recognise must never
+ // be read as the wider posture.
+ });
+
+ it('the opened write class is `update` alone — `owner_only_deletes` survives', () => {
+ expect(owdOpenWritesCoversOperation('update')).toBe(true);
+ for (const operation of ['delete', 'select', 'insert', 'all']) {
+ expect(owdOpenWritesCoversOperation(operation), `${operation} must not be opened`).toBe(false);
+ }
+ });
+});

packages/plugins/plugin-security/src/platform-ownership-policies.ts

@@ -101,3 +101,76 @@ export function isPlatformOwnershipFloorPolicy(
 export function platformOwnershipFloorPolicyCount(): number {
  return PLATFORM_OWNERSHIP_FLOOR_KEYS.size;
 }
+
+/**
+ * [#8023] The ONE Org-Wide Default that DECLARES row-level writes open, and the
+ * reason the floor above must not apply to an object carrying it.
+ *
+ * The floor is the platform's answer for objects whose OWD says nothing about
+ * writes. `public_read_write` is not such an object: the author has declared
+ * "everyone can see and edit" (`spec/security/sharing.zod.ts`), the access
+ * matrix grants `edit: true`, and the runtime was nevertheless refusing every
+ * write by a non-creator — the OWD contract stated and unenforced, which is the
+ * one thing a declaration must never be.
+ *
+ * ## Why this is read here and not deferred to `ISharingService`
+ *
+ * The composition already asks the sharing service for a tri-state write
+ * verdict, and on this object it answers `abstain` — correctly: record sharing
+ * genuinely does not enforce on a public object. But `abstain` is ONE answer
+ * covering three different facts (public OWD / no owner field / bypass-listed
+ * internal), and only the first says anything about writes being open. #5492's
+ * E2 experiment measured what collapsing all three into "permitted" costs: an
+ * ordinary member's cross-creator UPDATE on an `owner_id`-less object turned
+ * 403 into 200. So the abstain's meaning is left exactly as it is, and the OWD
+ * — which is a DECLARATION about this object rather than a verdict about this
+ * row — is read directly, on the security side, where the floor is composed.
+ *
+ * ## The DECLARED model, never the effective one
+ *
+ * `plugin-sharing`'s `effectiveSharingModel` folds `public_read_write`,
+ * `controlled_by_parent` and an unset model on a system object into one
+ * `'public'` bucket. That bucket is right for "does record sharing enforce
+ * here"; it is wrong for this question, and reading it here would open writes
+ * on two classes that never declared them:
+ *
+ * - `controlled_by_parent` derives its access from the MASTER record, which
+ * has its own OWD and its own gate (`assertControlledByParentWrite`) — the
+ * detail declares nothing about who may write it;
+ * - an UNSET model on a `sys_*` / `isSystem` object is a legacy default, not
+ * an author's statement. Opening writes there would hand every member
+ * cross-creator writes on the platform's own identity tables — the shape of
+ * the objectui#2348 incident that made an unset model fail closed in the
+ * first place.
+ *
+ * So the test is EXPLICIT equality with the one canonical spelling. An
+ * unresolvable schema yields `false` and the floor stays (fail closed).
+ */
+export const OWD_OPENING_ROW_WRITES = 'public_read_write';
+
+/**
+ * True iff this object's author DECLARED the org-wide-open write baseline.
+ * Reads both the flat and the nested spot, exactly as every other OWD reader
+ * in the platform does.
+ */
+export function owdDeclaresOpenRowWrites(schema: unknown): boolean {
+ const s = schema as { sharingModel?: unknown; security?: { sharingModel?: unknown } } | null;
+ const model = s?.sharingModel ?? s?.security?.sharingModel;
+ return model === OWD_OPENING_ROW_WRITES;
+}
+
+/**
+ * The write class {@link owdDeclaresOpenRowWrites} opens — `update` and nothing
+ * else, so `owner_only_deletes` survives on a `public_read_write` object.
+ *
+ * This is the OWD vocabulary's own boundary, not a conservative guess:
+ * `public_read_write` is defined as "everyone can see and edit", and the legacy
+ * `full` alias that ALSO covered transfer/delete was refused a mechanical
+ * conversion precisely because it is "wider than `public_read_write`"
+ * (`spec/conversions/registry.ts`, ADR-0090 D4). An object that wants
+ * cross-creator deletes declares it with an app-authored policy or a share,
+ * both of which reach the compiler untouched.
+ */
+export function owdOpenWritesCoversOperation(operation: string): boolean {
+ return operation === 'update';
+}

packages/plugins/plugin-security/src/security-plugin.ts

@@ -49,7 +49,11 @@ import { bootstrapDeclaredCapabilities } from './bootstrap-declared-capabilities
 import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js';
 import { computeTenantLayer0Filter, andComposeLayers } from './tenant-layer.js';
 import { isPlatformTenantPolicy, isAuthoredTenantPolicy } from './platform-tenant-policies.js';
-import { isPlatformOwnershipFloorPolicy } from './platform-ownership-policies.js';
+import {
+ isPlatformOwnershipFloorPolicy,
+ owdDeclaresOpenRowWrites,
+ owdOpenWritesCoversOperation,
+} from './platform-ownership-policies.js';
 import { hasPhantomTenantAnchor } from './federated-phantom-anchors.js';
 import {
  normalizeTenancyPosture,
@@ -177,6 +181,18 @@ interface ObjectSecurityMeta {
  * real remote `organization_id` — both keep the tenant wall exactly as it is.
  */
  tenantAnchorIsPhantom: boolean;
+ /**
+ * [#8023] The object DECLARES `sharingModel: 'public_read_write'` — the one
+ * OWD that states row-level writes are open org-wide. Read from the author's
+ * declaration, never from `plugin-sharing`'s effective bucket; see
+ * `owdDeclaresOpenRowWrites` in `platform-ownership-policies.ts` for why the
+ * two must not be confused here.
+ *
+ * `false` when the schema does not resolve, so the floor stays (fail closed)
+ * — and `unresolved` meta is never cached, so a transient boot miss is
+ * retried rather than frozen into an over-strict answer.
+ */
+ owdOpensRowWrites: boolean;
  requiredPermissions: NormalizedRequiredPermissions;
  fieldRequiredPermissions: Record<string, string[]>;
  /**
@@ -3995,6 +4011,41 @@ export class SecurityPlugin implements Plugin {
  let layer1: Record<string, unknown> | null = null;
  if (!(posturePermits && superuserBypass)) {
  let collected = this.collectRLSPolicies(permissionSets, object, operation, (context?.positions ?? []) as string[]);
+ // [#8023] An object whose author DECLARED `sharingModel:
+ // 'public_read_write'` does not inherit the platform's wildcard `update`
+ // ownership floor AT ALL. QA #7637 measured the contradiction it caused
+ // on the stock showcase: `showcase_project` declares that OWD, the access
+ // matrix grants `showcase_contributor` `edit: true`, no RLS is authored —
+ // and a PATCH of a row the persona had not created answered 403 with the
+ // record-level sentence. The floor is `member_default`'s
+ // `owner_only_writes` (object `'*'`, `created_by == current_user.id`,
+ // positions `['org_member']`), and it survived because the by-id gate
+ // only lets `ISharingService` REPLACE it on a positive `allow`, while a
+ // public object makes the service `abstain`.
+ //
+ // ⚠️ THE PLACEMENT IS THE FIX, not just where it reads best. Removing the
+ // floor HERE — before the #7665 derive-from-select branch below — leaves
+ // the update class EMPTY, so that branch then derives the write scope
+ // from the caller's SELECT narrowing and "you cannot mutate what you
+ // cannot see" still holds on this very object (#7792's by-id
+ // write-visibility property, which the card names as a non-regression
+ // criterion). Removing it at the `dropPlatformOwnershipFloor` filter
+ // further down would instead compile Layer 1 to null and hand back an
+ // UNGATED by-id write — the same hole #7665 closed, reopened by a fix for
+ // a different bug.
+ //
+ // Scope, stated twice because each half is load-bearing:
+ // - only the PLATFORM's floor is dropped (provenance, ADR-0105 D3): an
+ // app-authored policy spelling the identical predicate still reaches
+ // the compiler and still refuses (ADR-0049);
+ // - only `update` (`owdOpenWritesCoversOperation`): `owner_only_deletes`
+ // stays, because `public_read_write` is "see and edit" and the alias
+ // that also meant delete was removed for being wider than it.
+ // Layer 0 (the tenant wall) is untouched — a `public_read_write` object is
+ // org-wide open, never cross-tenant open.
+ if (meta.owdOpensRowWrites && owdOpenWritesCoversOperation(operation)) {
+ collected = collected.filter((p) => !isPlatformOwnershipFloorPolicy(p));
+ }
  // [#7665] The write-visibility floor: a write target must be inside the
  // caller's READABLE set. When NO policy of the write class applies to
  // this (principal, object, operation) — nothing authored for the class,
@@ -4580,6 +4631,8 @@ export class SecurityPlugin implements Plugin {
  // [#7835] Federated object carrying the registry's INJECTED
  // `organization_id` — a column the platform provisions no storage for.
  tenantAnchorIsPhantom: hasPhantomTenantAnchor(obj),
+ // [#8023] The author's DECLARED OWD, not the effective sharing bucket.
+ owdOpensRowWrites: owdDeclaresOpenRowWrites(obj),
  requiredPermissions: normalizeRequiredPermissions((obj as any)?.requiredPermissions),
  fieldRequiredPermissions,
  unresolved: !obj,