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..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** (`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 -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/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..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,23 +585,26 @@ 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); - 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' }); });