From bdaf6545aff578eedfe479bc2875255f179d2ccd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 07:59:10 +0000 Subject: [PATCH 1/2] fix(plugin-sharing)!: an unproven owner anchor on a federated object narrows the read and bulk-write scopes The sharing service decided whether a federated object's owner_id was real from the declared fields alone, and read the registry's injected anchor as "no owner column": the read filter went unscoped. A driver can still serve owner_id, so a private object answered every row to every member. The owner now counts as proven when fields declare owner_id or an external.columnMap entry maps a remote column onto it. Otherwise the read filter admits no row by ownership below org depth (the pending-approval arm stays), and the bulk write filter admits none either. A member's edit on such a private object now answers 404 (its pre-image read is scoped too). Pins: a real SqlDriver over a remote owner column (served-undeclared refused, mapped own-only, declared own-only) and the plugin-driver shapes over the sharing double. The spike's (e3) pin, the lint warning's refusal wording and two guide sentences follow the 404. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2g7zSyVZAkBv268xuK --- ...27425-phantom-owner-anchor-fails-closed.md | 19 +++ .../data-modeling/external-datasources.mdx | 5 + .../docs/data-modeling/virtual-objects.mdx | 6 +- packages/lint/src/rule-explanations.ts | 8 +- .../validate-external-owner-sharing.test.ts | 11 +- .../src/validate-external-owner-sharing.ts | 11 +- .../external-owner-anchor-sql-read.test.ts | 160 ++++++++++++++++++ .../src/federated-phantom-anchors.ts | 27 ++- .../federated-phantom-owner-scoping.test.ts | 123 +++++++++++--- .../plugin-sharing/src/sharing-service.ts | 43 +++-- ...lugin-driver-external-object.spike.test.ts | 13 +- 11 files changed, 371 insertions(+), 55 deletions(-) create mode 100644 .changeset/27425-phantom-owner-anchor-fails-closed.md create mode 100644 packages/plugins/plugin-sharing/src/external-owner-anchor-sql-read.test.ts diff --git a/.changeset/27425-phantom-owner-anchor-fails-closed.md b/.changeset/27425-phantom-owner-anchor-fails-closed.md new file mode 100644 index 00000000000..4f57d359fc5 --- /dev/null +++ b/.changeset/27425-phantom-owner-anchor-fails-closed.md @@ -0,0 +1,19 @@ +--- +'@objectstack/plugin-sharing': patch +'@objectstack/lint': patch +--- + +fix(plugin-sharing)!: on a federated object whose `owner_id` is the platform's injected anchor, the read and bulk-write scopes narrow below `org` instead of going unscoped (#27425) + +Clause-②: no (narrowing) — `SharingService.buildReadFilter` and `buildWriteFilter` answer a deny-all scope, where they answered `null` (no scope), for a caller below `org` depth on an ADR-0015 federated object that neither declares `owner_id` in `fields` nor maps a remote column onto it; on `private` such an object's refused edit answers 404 where it answered 403. Nothing is widened and no key is added, removed or renamed. +Metadata-protocol: none + + + +**BREAKING** read-scope narrowing in record sharing (`@objectstack/plugin-sharing`). + +- **Before.** On a federated object (`external` binding) with an owner-keyed sharing model, the sharing service decided whether `owner_id` was real from the declared `fields` alone. When the author had not declared it, the registry's injected anchor was taken to mean "no owner column", and the read filter applied no scope. A driver can still serve `owner_id` (the SQL driver through `external.columnMap`, a remote column of that name, a plugin driver on its own), so a `private` object answered its rows to members it was meant to hide them from. Bulk updates and deletes went unscoped the same way, while single-record writes were refused with 403. +- **Now.** The owner column counts as proven when `fields` declares `owner_id` or an `external.columnMap` entry maps a remote column onto it; such an object keeps ordinary owner scoping (a driver that does not apply the map serves no `owner_id`, so the owner predicate admits nothing there). Otherwise an unproven owner narrows the scope: below `org` read depth the read filter admits no row by ownership, no owner predicate and no share rows (a grant on such an object is already refused as inert), and a pending approval step still opens the record it waits on. The bulk write filter admits no row below `org` write depth. Because an edit reads its record through the same scope first, a member's PATCH on a `private` object of this shape now answers 404 `RECORD_NOT_FOUND` where it answered 403 `PERMISSION_DENIED`; on `public_read` it stays 403. `org` depth, the Modify All bypass, system callers and local objects are unchanged. +- **Who sees it.** An app with a federated object on `private` (including no `sharingModel`, which the runtime treats as `private`), or `public_read` for bulk writes, that neither declares nor maps `owner_id`: members below `org` depth now read no row there, and their refused edits answer 404. An external principal on a `public_read` object meets the same closed scope. `os validate`'s `security-external-owner-unmapped` warning (`@objectstack/lint`) now names the 404 on `private`, and its explanation says reads are refused too. + +**The remedy.** Choose on purpose. Declare `owner_id` in `fields` and have the driver return the remote owner under that name (a built-in SQL driver through `external.columnMap`; a plugin driver must return it itself), and the object scopes to the owner. Or choose `sharingModel: 'public_read'` (everyone reads) or `'public_read_write'` (everyone reads and edits, a `beforeUpdate` hook decides who may). diff --git a/content/docs/data-modeling/external-datasources.mdx b/content/docs/data-modeling/external-datasources.mdx index 087bc144708..6ac3c8c601d 100644 --- a/content/docs/data-modeling/external-datasources.mdx +++ b/content/docs/data-modeling/external-datasources.mdx @@ -91,6 +91,11 @@ export const Customer = ObjectSchema.create({ That's it. `GET /api/v1/data/ext_customer` now returns live rows from the remote `customers` table; filters (`?region=EU`) push down to the remote query. +`private` keys reads to the row's owner, and only an `owner_id` declared in `fields`, +or named by an `external.columnMap` entry, counts as one. Without it a member reads +no row, while a holder of View All (`viewAllRecords`) still reads them all. For a +table everyone may read, choose `sharingModel: 'public_read'`. + The object's **own** `datasource` must name a datasource declared `schemaMode: 'external'` or `'validate-only'` (ADR-0015 §4.2). An `external` binding on `'default'` (also the value when `datasource` is omitted) is refused by `ObjectSchema.create()`, diff --git a/content/docs/data-modeling/virtual-objects.mdx b/content/docs/data-modeling/virtual-objects.mdx index bf38ecbad43..18db5f104c6 100644 --- a/content/docs/data-modeling/virtual-objects.mdx +++ b/content/docs/data-modeling/virtual-objects.mdx @@ -372,9 +372,9 @@ What each piece does on a plugin driver: `PATCH /data/berth_slot/:id` above. -On the default sharing model, **every member edit is refused** (`403`), even for a -member whose permission set grants `allowEdit`. Before a write, the platform's ownership -checks ask the remote for `owner_id` and for `created_by` equal to the caller — columns a +On the default sharing model, **every member edit is refused** (`404`: the row is outside +the member's read scope), even for a member whose permission set grants `allowEdit`. +Before a write, the platform's ownership checks ask the remote for `owner_id` and for `created_by` equal to the caller — columns a remote row does not carry — so every row reads as someone else's ([#27332](https://github.com/objectstack-ai/objectstack/issues/27332)). Declare `sharingModel: 'public_read_write'` and decide who may edit in `beforeUpdate`, as above. diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 7b45b3727ae..e08be3cc15c 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -743,7 +743,8 @@ const SECURITY_EXTERNAL_OWNER_UNMAPPED_EXPLANATION: RuleExplanation = { 'the remote database owns the table and the platform creates no column for either. So the remote row ' + 'answers no `owner_id` (record sharing refuses) and no `created_by` equal to the caller (the floor ' + 'refuses). A member whose permission set grants `allowEdit` is refused every edit and every delete ' + - 'with 403 `PERMISSION_DENIED`, unless an app-authored row-level policy admits the row; otherwise only ' + + 'with 403 `PERMISSION_DENIED` (404 on `private`, where reads are scoped by the same owner and no row is ' + + 'in reach), unless an app-authored row-level policy admits the row; otherwise only ' + 'an administrator can write. An object with no `sharingModel` is ' + 'judged as \'private\', the value the runtime falls back to.', '`ownership: \'none\'` (or `\'org\'`, `\'business_unit\'`) is not a way out: without `owner_id` record ' + @@ -756,8 +757,9 @@ const SECURITY_EXTERNAL_OWNER_UNMAPPED_EXPLANATION: RuleExplanation = { 'that name, so the owner edits and deletes the row and other members are refused. A built-in SQL ' + 'driver does that through `external.columnMap` (remote column to `owner_id`); `columnMap` is applied ' + 'by those drivers, not by the engine, so a plugin driver must return the field itself. Declaring ' + - 'the field matters for reads too: the platform\'s own `owner_id` is treated as unstored, and reads ' + - 'are not scoped by owner. On an object without `owner_id`, declare `created_by` the same way instead, ' + + 'the field matters for reads too: the platform\'s own `owner_id`, with no field or `columnMap` entry ' + + 'naming it, proves no owner, so on `private` a member reads no row at all. On an object without ' + + '`owner_id`, declare `created_by` the same way instead, ' + 'so the creator writes the row.', 'A `columnMap` entry for the column without a declared field is reported too, with its own wording: ' + 'it serves the check on a built-in SQL driver and does nothing on a plugin driver, and the ' + diff --git a/packages/lint/src/validate-external-owner-sharing.test.ts b/packages/lint/src/validate-external-owner-sharing.test.ts index 160ad459b14..fd188848dce 100644 --- a/packages/lint/src/validate-external-owner-sharing.test.ts +++ b/packages/lint/src/validate-external-owner-sharing.test.ts @@ -11,7 +11,8 @@ import { /** * The two external objects of `packages/runtime/src/plugin-driver-external-object.spike.test.ts`, * copied verbatim (fields trimmed to what the rule reads). That spike measures the runtime half: - * on `spike_owned` (no `sharingModel`) a member holding `allowEdit` gets 403 on every PATCH, + * on `spike_owned` (no `sharingModel`) a member holding `allowEdit` is refused every PATCH (404 + * since #27425: the read scope admits no row), * step (e3); on `spike_node` (`public_read_write`) the same member's write reaches the driver. */ const SPIKE_STACK = defineStack({ @@ -135,6 +136,14 @@ describe('security-external-owner-unmapped — which objects it judges', () => { expect(findings[0]!.message).toContain('columnMap'); }); + it('[#27425] names the refusal the model produces: 404 on private (reads are scoped too), 403 on public_read', () => { + for (const obj of [ext(), ext({ sharingModel: 'private' }), ext({ sharingModel: 'private' }, { columnMap: { assignee: 'owner_id' } })]) { + expect(findingsFor(obj)[0]!.message).toContain("refused (404: the member's read scope admits no row)"); + } + expect(findingsFor(ext({ sharingModel: 'public_read' }))[0]!.message).toContain('refused (403)'); + expect(findingsFor(ext({ sharingModel: 'private', ownership: 'none' }))[0]!.message).toContain('refused (403)'); + }); + it('a columnMap that maps only created_by does not serve the owner check', () => { const findings = findingsFor(ext({ sharingModel: 'private' }, { columnMap: { author: 'created_by' } })); expect(findings).toHaveLength(1); diff --git a/packages/lint/src/validate-external-owner-sharing.ts b/packages/lint/src/validate-external-owner-sharing.ts index cb286d1fed3..3fb0ab1680c 100644 --- a/packages/lint/src/validate-external-owner-sharing.ts +++ b/packages/lint/src/validate-external-owner-sharing.ts @@ -163,6 +163,13 @@ export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharin : 'with no owner_id, the ownership floor reads created_by == the caller'; const mapped = columnMapTargets(external as AnyRec, column); const role = column === OWNER_COLUMN ? 'owner' : 'creator'; + // [#27425] On `private`, record sharing also scopes READS by owner_id, and + // an owner it cannot read admits no row: the edit's pre-image read finds + // none, so the refusal is a 404 rather than the write check's 403. + const refusal = + column === OWNER_COLUMN && effectiveSharingModelOf(obj) === 'private' + ? "(404: the member's read scope admits no row)" + : '(403)'; const declareWayOut = `declare ${column} in fields and have the driver return the remote ${role} under that name ` + '(a built-in SQL driver does it through external.columnMap; a plugin driver must return it itself)'; @@ -178,10 +185,10 @@ export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharin message: mapped ? `external object on sharing model ${model} maps a remote column to '${column}' through ` + `external.columnMap but declares no '${column}' field; ${check} before every edit and delete, ` + - 'and only a built-in SQL driver applies columnMap, so on a plugin driver every member write is refused (403) ' + + `and only a built-in SQL driver applies columnMap, so on a plugin driver every member write is refused ${refusal} ` + 'unless an app-authored row-level policy admits the row' : `external object on sharing model ${model} declares no '${column}' field a remote row serves; ` + - `${check} before every edit and delete, so every member write is refused (403) unless an app-authored ` + + `${check} before every edit and delete, so every member write is refused ${refusal} unless an app-authored ` + 'row-level policy admits the row', hint: mapped ? `Declare ${column} in fields (a plugin driver must return the remote ${role} under that name itself), ` + diff --git a/packages/plugins/plugin-sharing/src/external-owner-anchor-sql-read.test.ts b/packages/plugins/plugin-sharing/src/external-owner-anchor-sql-read.test.ts new file mode 100644 index 00000000000..e2bcc2c839c --- /dev/null +++ b/packages/plugins/plugin-sharing/src/external-owner-anchor-sql-read.test.ts @@ -0,0 +1,160 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27425] The SQL-driver half of the pin, over `private` federated objects + * whose remote rows carry an owner. + * + * - The remote column is literally `owner_id` and `fields` do not declare it: + * the driver serves it, but the registered field is the platform's injected + * anchor and nothing declared says the column exists. Before the fix that + * read as "ownership contributes nothing" (`null`), so a member read every + * row; now the scope narrows to none. + * - `external.columnMap` maps the remote owner column onto `owner_id`: the map + * is declared metadata, the SQL driver applies it, and the member reads own + * rows only, as with a declared field. + * - `owner_id` declared in `fields`: the control, own rows only. + * + * Real `ObjectQL`, real `SqlDriver` on `better-sqlite3`, the real sharing + * middleware: the registry's injection, the driver's remap and the composed + * filter are each the shipped code, not a fixture of it. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; + +import { SharingService } from './sharing-service.js'; +import { buildSharingMiddleware } from './sharing-plugin.js'; +import { hasPhantomOwnerAnchor } from './federated-phantom-anchors.js'; + +const MEMBER = 'u_member'; +const ADMIN = 'u_admin'; + +/** The remote table: its owner column is called `assignee`. */ +const REMOTE = { + name: 'remote_rows', + fields: { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + name: { type: 'text', name: 'name', label: 'Name' }, + assignee: { type: 'text', name: 'assignee', label: 'Assignee' }, + }, +}; + +const SHARE_FIELDS: Record> = { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + object_name: { type: 'text', name: 'object_name', label: 'Object' }, + record_id: { type: 'text', name: 'record_id', label: 'Record' }, + recipient_type: { type: 'text', name: 'recipient_type', label: 'Recipient type' }, + recipient_id: { type: 'text', name: 'recipient_id', label: 'Recipient' }, + access_level: { type: 'text', name: 'access_level', label: 'Access' }, +}; + +/** A second remote table whose owner column is literally `owner_id`. */ +const REMOTE_OWNED = { + name: 'remote_owned', + fields: { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + name: { type: 'text', name: 'name', label: 'Name' }, + owner_id: { type: 'text', name: 'owner_id', label: 'Owner' }, + }, +}; + +const binding = { remoteName: REMOTE.name, columnMap: { assignee: 'owner_id' } }; + +/** Served but undeclared: no map, no field, the registry injects its anchor. */ +const SERVED_UNDECLARED = { + name: 'ext_owned', + label: 'External owned rows', + sharingModel: 'private', + external: { remoteName: REMOTE_OWNED.name }, + fields: { name: { type: 'text', label: 'Name' } }, +}; + +/** `owner_id` undeclared, but the columnMap names the remote owner column. */ +const MAPPED = { + name: 'ext_rows', + label: 'External rows', + sharingModel: 'private', + external: binding, + fields: { name: { type: 'text', label: 'Name' } }, +}; + +/** The control: `owner_id` declared, the author vouches for the column. */ +const DECLARED = { + name: 'ext_rows_declared', + label: 'External rows (declared owner)', + sharingModel: 'private', + external: binding, + fields: { + name: { type: 'text', label: 'Name' }, + owner_id: { type: 'lookup', reference: 'sys_user', label: 'Owner' }, + }, +}; + +describe('[#27425] a private SQL federated object whose remote rows carry an owner', () => { + let driver: SqlDriver; + let ql: ObjectQL; + + beforeAll(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + ql = new ObjectQL(); + ql.registerDriver(driver as never, true); + await ql.init(); + + await driver.initObjects([ + REMOTE as never, + REMOTE_OWNED as never, + { name: 'sys_record_share', fields: SHARE_FIELDS } as never, + ]); + await driver.create(REMOTE.name, { id: 'r_mine', name: 'Mine', assignee: MEMBER }); + await driver.create(REMOTE.name, { id: 'r_theirs', name: 'Theirs', assignee: ADMIN }); + await driver.create(REMOTE_OWNED.name, { id: 'r_mine', name: 'Mine', owner_id: MEMBER }); + await driver.create(REMOTE_OWNED.name, { id: 'r_theirs', name: 'Theirs', owner_id: ADMIN }); + + ql.registerObject({ name: 'sys_record_share', label: 'Record share', isSystem: true, fields: SHARE_FIELDS } as never); + for (const object of [SERVED_UNDECLARED, MAPPED, DECLARED]) { + ql.registerObject(object as never); + // What the schema sync does for a federated object: bind it, no DDL. + driver.registerExternalObject(ql.getSchema(object.name) as never); + } + ql.registerMiddleware(buildSharingMiddleware(new SharingService({ engine: ql as never })) as never, { object: '*' }); + }); + + afterAll(async () => { + await driver?.disconnect?.(); + }); + + const memberReads = async (object: string): Promise => { + const rows = (await ql.find(object, { context: { userId: MEMBER } } as never)) as Array<{ id: string }>; + return rows.map((r) => r.id).sort(); + }; + + it('PREMISE: the driver serves owner_id on all three, and only the unmapped, undeclared one is the injected anchor', async () => { + for (const object of [SERVED_UNDECLARED.name, MAPPED.name, DECLARED.name]) { + const served = (await driver.find(object, {} as never)) as Array>; + expect(served.map((r) => [r.id, r.owner_id]).sort()).toEqual([ + ['r_mine', MEMBER], + ['r_theirs', ADMIN], + ]); + } + expect(hasPhantomOwnerAnchor(ql.getSchema(SERVED_UNDECLARED.name))).toBe(true); + expect(hasPhantomOwnerAnchor(ql.getSchema(MAPPED.name))).toBe(false); + expect(hasPhantomOwnerAnchor(ql.getSchema(DECLARED.name))).toBe(false); + }); + + it('owner_id served but neither declared nor mapped: the member reads no row (it read 2 of 2 before)', async () => { + expect(await memberReads(SERVED_UNDECLARED.name)).toEqual([]); + }); + + it('columnMap → owner_id with no declared field: own rows only (it read 2 of 2 before)', async () => { + expect(await memberReads(MAPPED.name)).toEqual(['r_mine']); + }); + + it('ANTI-VACUITY: owner_id declared over the same table reads own-only', async () => { + expect(await memberReads(DECLARED.name)).toEqual(['r_mine']); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/federated-phantom-anchors.ts b/packages/plugins/plugin-sharing/src/federated-phantom-anchors.ts index 1f91d94ef8d..211080fcfdb 100644 --- a/packages/plugins/plugin-sharing/src/federated-phantom-anchors.ts +++ b/packages/plugins/plugin-sharing/src/federated-phantom-anchors.ts @@ -140,12 +140,31 @@ export function isFederatedObject(schema: unknown): boolean { * platform's injected anchor on an object whose storage the platform never * provisioned? * - * `true` ⇒ record-level ownership scoping must treat the object as carrying NO - * owner column, because it does not (see the module docs). `false` for every - * local object (the platform DID provision the column there) and for a - * federated object whose author declared a real remote `owner_id`. + * `true` ⇒ no owner predicate can be proven for the object: nothing declared + * says a remote column answers to `owner_id`, even where a driver happens to + * return one of its own accord. [#27425] Record-level scoping then NARROWS — + * the read and bulk write scopes admit no row by ownership below `org` — and + * never widens. `false` for every local object (the platform DID provision the + * column there), for a federated object whose author declared a real remote + * `owner_id`, and for one whose `external.columnMap` maps a remote column onto + * `owner_id` (see {@link mapsRemoteColumnOntoOwner}). */ export function hasPhantomOwnerAnchor(schema: unknown): boolean { if (!isFederatedObject(schema)) return false; + if (mapsRemoteColumnOntoOwner(schema)) return false; return equalsShippedDef(readFieldDef(schema, OWNER_COLUMN), OWNER_FIELD_DEF); } + +/** + * [#27425] Does the object's `external.columnMap` (remote column → local field) + * name `owner_id` as a target? That is the author declaring which remote column + * carries the owner, so the anchor is not phantom. The built-in SQL drivers + * apply the map and serve the column; a driver that does not apply it returns + * no `owner_id`, so the owner predicate scoping composes matches no row — the + * fail direction stays closed either way. + */ +function mapsRemoteColumnOntoOwner(schema: unknown): boolean { + const columnMap = (schema as { external?: { columnMap?: unknown } } | null | undefined)?.external?.columnMap; + if (!columnMap || typeof columnMap !== 'object' || Array.isArray(columnMap)) return false; + return Object.values(columnMap as Record).includes(OWNER_COLUMN); +} diff --git a/packages/plugins/plugin-sharing/src/federated-phantom-owner-scoping.test.ts b/packages/plugins/plugin-sharing/src/federated-phantom-owner-scoping.test.ts index 222ef4d0d0c..cc78a6e1393 100644 --- a/packages/plugins/plugin-sharing/src/federated-phantom-owner-scoping.test.ts +++ b/packages/plugins/plugin-sharing/src/federated-phantom-owner-scoping.test.ts @@ -208,22 +208,23 @@ describe('[#7858] ownership scoping vs federated (external) objects', () => { }); }); - describe('the INJECTED anchor contributes nothing', () => { - // The card's measured values, pre-fix: - // buildReadFilter(measure_ext_nostamp, __readScope='own') = {"owner_id":"usr_member_1"} - // buildReadFilter(measure_ext_nostamp, __readScope='unit') = {"owner_id":"usr_member_1"} + // [#27425] #7858 pinned `null` here ("ownership contributes nothing"), which + // read as every row to every member: a driver may serve `owner_id` without + // the schema declaring it, so "undeclared" never meant "no owner". The + // anchor is unproven, and an unproven anchor narrows the scope. + describe('the INJECTED anchor narrows the scope, never widens it', () => { it.each(['own', 'own_and_reports', 'unit', 'unit_and_below'] as const)( - 'read: no owner predicate at __readScope=%s', + 'read: deny-all at __readScope=%s', async (scope) => { const filter = await svc.buildReadFilter('measure_ext_nostamp', { userId: MEMBER, __readScope: scope, } as never); - expect(filter).toBeNull(); + expect(filter).toEqual({ id: '__deny_all__' }); }, ); - it('read: `org` scope stays null, exactly as it already did', async () => { + it('read: `org` scope stays null — it never reads the owner column', async () => { const filter = await svc.buildReadFilter('measure_ext_nostamp', { userId: MEMBER, __readScope: 'org', @@ -232,23 +233,107 @@ describe('[#7858] ownership scoping vs federated (external) objects', () => { }); it.each(['update', 'delete'] as const)( - 'write: no owner predicate on a bulk %s', + 'write: a bulk %s admits no row below `org`, and `org` stays null', async (verb) => { - const filter = await svc.buildWriteFilter( - 'measure_ext_nostamp', - { userId: MEMBER, __writeScope: 'own' } as never, - verb, - ); - expect(filter).toBeNull(); + expect( + await svc.buildWriteFilter('measure_ext_nostamp', { userId: MEMBER, __writeScope: 'own' } as never, verb), + ).toEqual({ id: '__deny_all__' }); + expect( + await svc.buildWriteFilter('measure_ext_nostamp', { userId: MEMBER, __writeScope: 'org' } as never, verb), + ).toBeNull(); }, ); - it('read: the principal-less degenerate case is not reached either', async () => { - // Ownership contributes nothing BEFORE the deny-all fallback, so a - // federated object does not become unreadable to an anonymous API key - // over a column it has not got. + it('read: the principal-less degenerate case reaches the deny-all fallback', async () => { const filter = await svc.buildReadFilter('measure_ext_nostamp', {} as never); - expect(filter).toBeNull(); + expect(filter).toEqual({ id: '__deny_all__' }); + }); + + it('read: an external principal on a `public_read` phantom object meets the owner_only floor, closed', async () => { + const svcPublicRead = new SharingService({ + engine: makeEngine({ + measure_ext_nostamp: federatedSchema({ sharingModel: 'public_read' }), + sys_record_share: { name: 'sys_record_share' }, + }), + }); + const filter = await svcPublicRead.buildReadFilter('measure_ext_nostamp', { + userId: 'u_ext', tenantId: 'org_a', audience: 'external', posture: 'EXTERNAL', __readScope: 'own', + } as never); + expect(filter).toEqual({ id: '__deny_all__' }); + }); + + it('read: a pending approval step still opens the record it waits on — an id list, not an owner read', async () => { + const withApproval = new SharingService({ + engine: makeEngine({ + measure_ext_nostamp: federatedSchema(), + sys_record_share: { name: 'sys_record_share' }, + }), + approvals: () => ({ + pendingApproverReadGrants: async () => new Map([['c1', ['apr_1']]]), + }), + }); + const filter = await withApproval.buildReadFilter('measure_ext_nostamp', { + userId: MEMBER, + __readScope: 'own', + } as never); + expect(filter).toEqual({ id: { $in: ['c1'] } }); + }); + }); + + /** + * [#27425] A remote row arrives WITH an `owner_id` the schema's `fields` do + * not declare. Each fixture stores the row exactly as the driver serves it, + * and the read filter is applied to those rows by the double's `find`, so + * the assertion is on rows, not on a filter shape. Before the fix the filter + * was `null` and the member read 2 of 2, the other owner's row too. + */ + describe('a driver serves `owner_id` that `fields` does not declare', () => { + const OTHER = 'usr_admin'; + const SERVED_ROWS = [ + { id: 'r_mine', owner_id: MEMBER, name: 'Mine' }, + { id: 'r_theirs', owner_id: OTHER, name: 'Theirs' }, + ]; + + const readAs = async (object: string, schema: unknown, rows: Array>) => { + const engine = makeEngine( + { [object]: schema, sys_record_share: { name: 'sys_record_share' } }, + { [object]: rows.map((r) => ({ ...r })) }, + ); + const filter = await new SharingService({ engine }).buildReadFilter(object, { + userId: MEMBER, + __readScope: 'own', + } as never); + const seen = await engine.find(object, filter == null ? {} : { where: filter as Record }); + return seen.map((r) => r.id); + }; + + it('a plugin driver returning owner_id with nothing declared: the member reads no row, not 2 of 2', async () => { + const schema = federatedSchema({ name: 'ext_plugin_served', external: { remoteName: 'rows' } }); + expect(await readAs('ext_plugin_served', schema, SERVED_ROWS)).toEqual([]); + }); + + it('the SQL driver with columnMap → owner_id (it applies the map): own rows only', async () => { + // The map is the author's declaration of the owner column, so the anchor + // is not phantom and ordinary scoping applies over the served column. + const schema = federatedSchema({ + name: 'ext_sql_mapped', + external: { remoteName: 'rows', columnMap: { assignee: 'owner_id' } }, + }); + expect(await readAs('ext_sql_mapped', schema, SERVED_ROWS)).toEqual(['r_mine']); + }); + + it('a driver that does NOT apply the columnMap serves no owner_id: the member reads no row', async () => { + const schema = federatedSchema({ + name: 'ext_plugin_mapped', + external: { remoteName: 'rows', columnMap: { assignee: 'owner_id' } }, + }); + const unmapped = SERVED_ROWS.map(({ owner_id, ...rest }) => ({ ...rest, assignee: owner_id })); + expect(await readAs('ext_plugin_mapped', schema, unmapped)).toEqual([]); + }); + + it('ANTI-VACUITY: the same rows with `owner_id` DECLARED read own-only', async () => { + const schema = { ...DECLARED_REAL_OWNER, name: 'ext_declared' }; + expect(await readAs('ext_declared', schema, SERVED_ROWS)).toEqual(['r_mine']); }); }); diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 4533a92b934..72c26f6eac2 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -653,8 +653,8 @@ export class SharingService implements ISharingService { // [ADR-0138 D3 item 6, as the maintainer's ruling C amends it — comment // 6148183570 on #23266] The GUEST never takes the OWD branch below: not the // object's `sharingModel` (a `public_read` or `public_read_write` object, a - // `controlled_by_parent` detail), and not its owner-less or phantom-anchor - // fall-throughs, which read as OWD-public. `externalSharingModel` is not + // `controlled_by_parent` detail), and not its owner-less fall-through, + // which reads as OWD-public. `externalSharingModel` is not // read here for anyone (#23268). The guest owns nothing (ADR-0138 D1), so // it reaches rows only through share links — and a link is redeemed at its // own door, which checks the token and `publicSharing.eligibility` and @@ -671,16 +671,6 @@ export class SharingService implements ISharingService { // baseline never widens it. if (owdForPrincipal(schema, context, object, 'read') !== 'private') return null; if (!hasOwnerField(schema)) return null; - // [#7858] …and an `owner_id` the REGISTRY injected into a FEDERATED object - // is not an owner column at all: the platform provisions no storage for one, - // so the column exists in the schema and in no store. Scoping by it composes - // a predicate the remote table cannot resolve — constant-false on SQLite - // (0 rows, HTTP 200), a hard error on Postgres/MySQL — which is why this - // reads as "ownership contributes nothing", exactly like the owner-less - // object one line up, rather than as a narrower filter. A federated object - // that DECLARES a real remote owner column keeps its scoping, and every - // local object is untouched; see `federated-phantom-anchors.ts`. - if (hasPhantomOwnerAnchor(schema)) return null; if (!context.userId) { // Authenticated context with no user id is a degenerate case // (e.g. anonymous API key). Restrict to nothing rather than @@ -702,6 +692,24 @@ export class SharingService implements ISharingService { // seam as authorable, client-supplied vocabulary. const readScope = (context as any).__readScope as ('own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org' | undefined); if (readScope === 'org') return null; + // [#27425] …and an `owner_id` the REGISTRY injected into a FEDERATED object + // proves nothing about the remote row: the platform provisions no storage + // for it, so whether any column answers to `owner_id` is up to the driver + // (a remote column of that name, or a plugin driver returning it on its + // own), and nothing declared says so. #7858 read that as "ownership + // contributes nothing" and returned `null` — every row to every member, on + // the object whose author chose `private`. The scope NARROWS instead: no + // owner predicate can be proven, so the owner arm admits nothing, and share + // rows are not consulted either (a grant on such an object is refused as + // inert, see `inertGrantReason`). The pending-approval arm stays: it is an + // id list the approvals service vouches for, not an owner read. `org` above + // never reads the owner column, so it is unchanged. A federated object that + // DECLARES its owner column keeps ordinary scoping, and every local object + // is untouched; see `federated-phantom-anchors.ts`. + if (hasPhantomOwnerAnchor(schema)) { + const approvalIds = [...(await this.pendingApproverReadGrants(object, context)).keys()]; + return approvalIds.length > 0 ? { id: { $in: approvalIds } } : { id: '__deny_all__' }; + } const ownerIds = await this.resolveOwnerScopeIds(context, readScope); const ownerMatch: Record = ownerIds.length === 1 ? { [OWNER_FIELD]: ownerIds[0] } @@ -803,12 +811,6 @@ export class SharingService implements ISharingService { // an external unless a held grant's RLS speaks for it. if (owdForPrincipal(schema, context, object, 'write') === 'public') return null; if (!hasOwnerField(schema)) return null; - // [#7858] The write half of the same phantom-anchor test the read filter - // applies — the card measured BOTH gates. Fixing only the read half would - // leave a bulk `update`/`delete` AND-composing `owner_id = ` onto a - // federated table that has no such column: the same constant-false / - // hard-error split, here silently touching zero rows instead of refusing. - if (hasPhantomOwnerAnchor(schema)) return null; if (!context.userId) { // Authenticated but principal-less → edit nothing (fail closed), // mirroring buildReadFilter's degenerate-context handling. @@ -818,6 +820,11 @@ export class SharingService implements ISharingService { // Middleware-stamped, not a field of the envelope — see buildReadFilter(). const writeScope = (context as any).__writeScope as ('own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org' | undefined); if (writeScope === 'org') return null; + // [#27425] The write half of the read filter's phantom-anchor narrowing. + // `null` here (#7858) let a bulk `update`/`delete` touch every row while the + // single-record gates refuse each one (`matchesOwnerScope` reads no owner); + // the bulk scope admits nothing either, and no share row is consulted. + if (hasPhantomOwnerAnchor(schema)) return { id: '__deny_all__' }; const ownerIds = await this.resolveOwnerScopeIds(context, writeScope); const ownerMatch: Record = ownerIds.length === 1 ? { [OWNER_FIELD]: ownerIds[0] } diff --git a/packages/runtime/src/plugin-driver-external-object.spike.test.ts b/packages/runtime/src/plugin-driver-external-object.spike.test.ts index d185f786965..72505b51981 100644 --- a/packages/runtime/src/plugin-driver-external-object.spike.test.ts +++ b/packages/runtime/src/plugin-driver-external-object.spike.test.ts @@ -589,19 +589,22 @@ describe('(e) PATCH /data/spike_node/:id runs `beforeUpdate` first, then `driver const mark = calls.length; const res = await as(plannerToken, 'PATCH', `/data/${OWNED}/o_1`, { name: 'Renamed' }); const body = await readJson(res); - expect(res.status, JSON.stringify(body)).toBe(403); - expect(body?.code).toBe('PERMISSION_DENIED'); + // [#27425] The undeclared `owner_id` proves no owner, so the member's read + // scope admits no row and the edit's pre-image read finds none: 404, not + // the 403 it answered while the read went unscoped. + expect(res.status, JSON.stringify(body)).toBe(404); + expect(body?.code).toBe('RECORD_NOT_FOUND'); const seen = calls.slice(mark).filter((c) => c.object === OWNED); expect(seen.map((c) => c.method)).not.toContain('update'); - // What the remote is asked for: `owner_id` (the sharing check) and - // `created_by == current_user.id` (the platform ownership floor). + // What the remote is asked for: `owner_id` by id (the sharing check), and + // the scoped pre-image read, which carries the deny-all sentinel. // [#27277] Under the floor the remote sees each as a capped `find`; the // engine applies the projection and `limit: 1` itself. expect(seen).toContainEqual({ method: 'find', object: OWNED, query: { where: { id: 'o_1' }, limit: EXTERNAL_ROW_CAP + 1, object: OWNED } }); expect(seen).toContainEqual({ method: 'find', object: OWNED, - query: { where: { $and: [{ id: 'o_1' }, { created_by: plannerId }] }, object: OWNED, limit: EXTERNAL_ROW_CAP + 1 }, + query: { where: { $and: [{ id: 'o_1' }, { id: '__deny_all__' }] }, object: OWNED, limit: EXTERNAL_ROW_CAP + 1 }, }); expect(owned[0]).toEqual({ id: 'o_1', name: 'Remote row' }); }); From 9cc5f83a65a200f4be69921b3e0a50c69db3cf28 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 09:18:35 +0000 Subject: [PATCH 2/2] docs(virtual-objects): the callout and the spike's (e3) title name the read scope as the cause of the 404 On the default model the member's read scope admits no row, so lists are empty and an edit answers 404; the write-time owner checks and their 403 are the public_read case. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2g7zSyVZAkBv268xuK --- content/docs/data-modeling/virtual-objects.mdx | 10 ++++++---- .../src/plugin-driver-external-object.spike.test.ts | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/content/docs/data-modeling/virtual-objects.mdx b/content/docs/data-modeling/virtual-objects.mdx index 18db5f104c6..99b4c970ce3 100644 --- a/content/docs/data-modeling/virtual-objects.mdx +++ b/content/docs/data-modeling/virtual-objects.mdx @@ -372,10 +372,12 @@ What each piece does on a plugin driver: `PATCH /data/berth_slot/:id` above. -On the default sharing model, **every member edit is refused** (`404`: the row is outside -the member's read scope), even for a member whose permission set grants `allowEdit`. -Before a write, the platform's ownership checks ask the remote for `owner_id` and for `created_by` equal to the caller — columns a -remote row does not carry — so every row reads as someone else's +On the default sharing model, **every member edit is refused**, even for a member whose +permission set grants `allowEdit`. That model keys reads to the row's `owner_id`, a column +a remote row does not carry, so the member's read scope admits no row: lists come back +empty and an edit answers `404`. On `public_read` the rows are readable, but before a write +the platform's ownership checks ask the remote for `owner_id` and for `created_by` equal to +the caller, find neither, and refuse the edit with `403` ([#27332](https://github.com/objectstack-ai/objectstack/issues/27332)). Declare `sharingModel: 'public_read_write'` and decide who may edit in `beforeUpdate`, as above. Serving an owner column the checks can read is the other way out; the reference test diff --git a/packages/runtime/src/plugin-driver-external-object.spike.test.ts b/packages/runtime/src/plugin-driver-external-object.spike.test.ts index 72505b51981..7a4b751f023 100644 --- a/packages/runtime/src/plugin-driver-external-object.spike.test.ts +++ b/packages/runtime/src/plugin-driver-external-object.spike.test.ts @@ -585,7 +585,7 @@ describe('(e) PATCH /data/spike_node/:id runs `beforeUpdate` first, then `driver expect(body.record).toMatchObject({ id: 'n_cut', name: 'Cut', start_date: '2026-11-03', end_date: '2026-11-07', guard_note: 'checked', locked: false }); }); - it('(e3) on the default sharing model, a member holding allowEdit is refused every edit: the ownership checks read columns no remote row carries', async () => { + it('(e3) on the default sharing model, a member holding allowEdit is refused every edit: no declared owner, so the member\'s read scope admits no row (404)', async () => { const mark = calls.length; const res = await as(plannerToken, 'PATCH', `/data/${OWNED}/o_1`, { name: 'Renamed' }); const body = await readJson(res);