From 9ea80c51d7fe96b780d1c5c259d44b6f6bb5c97a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 22:19:16 +0000 Subject: [PATCH] fix(plugin-sharing): log the write refusal when a federated object's mapped owner column was not served by the driver Since the columnMap -> owner_id arm took mapped objects out of the phantom-anchor test, a plugin driver that ignores the map left every member write refused with no server log. The write gate now passes the pre-image row to the diagnostic, which warns once per object when a non-phantom federated row has no owner_id key. Verdicts are unchanged. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2eGmV67aXQuY2TJg7s --- .../27662-unserved-owner-write-deny-notice.md | 10 ++ .../federated-phantom-owner-scoping.test.ts | 115 +++++++++++++++++- packages/plugins/plugin-sharing/src/index.ts | 1 + .../plugin-sharing/src/sharing-service.ts | 53 ++++++-- 4 files changed, 171 insertions(+), 8 deletions(-) create mode 100644 .changeset/27662-unserved-owner-write-deny-notice.md diff --git a/.changeset/27662-unserved-owner-write-deny-notice.md b/.changeset/27662-unserved-owner-write-deny-notice.md new file mode 100644 index 00000000000..1bbde84d092 --- /dev/null +++ b/.changeset/27662-unserved-owner-write-deny-notice.md @@ -0,0 +1,10 @@ +--- +'@objectstack/plugin-sharing': patch +--- + +fix(plugin-sharing): a refused write on a federated object whose mapped owner column the driver did not serve is logged again + +Clause-②: no +Metadata-protocol: none + +A federated object that maps a remote column onto `owner_id` through `external.columnMap`, served by a plugin driver that does not apply the map, returns rows with no `owner_id`, so every member edit or delete is refused. The refusal is unchanged. Since the mapped class stopped counting as a phantom owner anchor, the one-line server warning that tells the operator why had stopped firing. It fires again, at most once per object, as the new `UNSERVED_OWNER_WRITE_DENY_NOTICE`, which names the driver returning no value for the mapped or declared owner column. A driver that serves the column, including a row whose owner is NULL, stays silent. 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 cc78a6e1393..1043eef34a9 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 @@ -57,7 +57,11 @@ import { type EngineUpdateDispatchInput, } from '@objectstack/metadata-core'; import { ERROR_CODE_LEDGER } from '@objectstack/spec/api'; -import { SharingService, PHANTOM_ANCHOR_WRITE_DENY_NOTICE } from './sharing-service.js'; +import { + SharingService, + PHANTOM_ANCHOR_WRITE_DENY_NOTICE, + UNSERVED_OWNER_WRITE_DENY_NOTICE, +} from './sharing-service.js'; /** The caller: an ordinary member whose read/write DEPTH is narrower than `org`. */ const MEMBER = 'usr_member_1'; @@ -821,3 +825,112 @@ describe('[#8418] the phantom-anchor write refusal is diagnosed, not changed', ( }); }); }); + +/** + * [#27662] The mapped owner column the driver never served is diagnosed too. + * + * #27425 counted an `external.columnMap` entry onto `owner_id` as the author + * declaring the owner column, which took the object out of + * `hasPhantomOwnerAnchor` — and so out of the #8418 line above. A plugin driver + * that does not apply the map answers with the remote column's own name, the + * pre-image carries no `owner_id`, and every member write is refused in + * silence again. The refusal is right and stays; the silence is what goes. + * + * The discrimination is the KEY, not the value: a driver that applies the map + * and serves a NULL owner is an ordinary owner-less row (silent, like + * `LOCAL_OWNERLESS` above), while a row with no `owner_id` key at all is the + * driver dropping the column. + */ +describe('[#27662] a mapped owner column the driver did not serve is diagnosed, not changed', () => { + const OTHER = 'usr_other'; + const mapped = (name: string) => + federatedSchema({ name, external: { remoteName: 'rows', columnMap: { assignee: 'owner_id' } } }); + + let svc: SharingService; + let warn: ReturnType; + + beforeEach(() => { + warn = vi.fn(); + svc = new SharingService({ + engine: makeEngine( + { + // A plugin driver that ignores the map: rows carry `assignee`, never `owner_id`. + ext_plugin_mapped: mapped('ext_plugin_mapped'), + ext_plugin_mapped_2: mapped('ext_plugin_mapped_2'), + // The SQL driver applies the map, so `owner_id` is served (possibly NULL). + ext_sql_mapped: mapped('ext_sql_mapped'), + ext_with_real_owner: DECLARED_REAL_OWNER, + sys_record_share: { name: 'sys_record_share' }, + }, + { + ext_plugin_mapped: [{ id: 'p1', assignee: MEMBER, name: 'Mine, unmapped' }], + ext_plugin_mapped_2: [{ id: 'q1', assignee: MEMBER, name: 'Also unmapped' }], + ext_sql_mapped: [ + { id: 's_mine', owner_id: MEMBER, name: 'Mine' }, + { id: 's_theirs', owner_id: OTHER, name: 'Theirs' }, + { id: 's_nobody', owner_id: null, name: 'Owner-less' }, + ], + ext_with_real_owner: [ + { id: 'a1', owner_id: MEMBER, name: 'Acme' }, + { id: 'a2', owner_id: OTHER, name: 'Globex' }, + ], + }, + ), + logger: { info: vi.fn(), warn, error: vi.fn() } as never, + }); + }); + + const ctx = (scope = 'own') => ({ userId: MEMBER, __writeScope: scope }) as never; + const notices = () => warn.mock.calls.filter((c) => c[0] === UNSERVED_OWNER_WRITE_DENY_NOTICE); + const phantomNotices = () => warn.mock.calls.filter((c) => c[0] === PHANTOM_ANCHOR_WRITE_DENY_NOTICE); + + it('a map-ignoring plugin driver: still DENIES, and says why once', async () => { + // The caller's id IS in the row — under `assignee`, where the gate cannot + // see it. So the refusal is the driver's doing, not the caller's. + expect(await svc.checkEdit('ext_plugin_mapped', 'p1', ctx())).toBe('deny'); + expect(await svc.checkDelete('ext_plugin_mapped', 'p1', ctx())).toBe('deny'); + expect(await svc.checkEdit('ext_plugin_mapped', 'p1', ctx('org'))).toBe('deny'); + + expect(notices()).toHaveLength(1); + expect(notices()[0][1]).toMatchObject({ object: 'ext_plugin_mapped', ownerField: 'owner_id', userId: MEMBER }); + // …and not the phantom wording, which would send the operator to the metadata. + expect(phantomNotices()).toHaveLength(0); + }); + + it('the wording names the driver and the mapped owner column', async () => { + await svc.checkEdit('ext_plugin_mapped', 'p1', ctx()); + const [message] = notices()[0]; + expect(message).toContain('the driver returned no value for the owner column'); + expect(message).toContain('external.columnMap'); + }); + + it('a SECOND map-ignoring object gets its own line; a different caller does not re-arm one', async () => { + await svc.checkEdit('ext_plugin_mapped', 'p1', ctx()); + await svc.checkEdit('ext_plugin_mapped', 'p1', { userId: OTHER, __writeScope: 'own' } as never); + await svc.checkEdit('ext_plugin_mapped_2', 'q1', ctx()); + expect(notices().map((c) => (c[1] as { object: string }).object)).toEqual([ + 'ext_plugin_mapped', + 'ext_plugin_mapped_2', + ]); + }); + + it('a map-applying driver is silent: own row allows, another owner and a NULL owner deny', async () => { + expect(await svc.checkEdit('ext_sql_mapped', 's_mine', ctx())).toBe('allow'); + expect(await svc.checkEdit('ext_sql_mapped', 's_theirs', ctx())).toBe('deny'); + // The served-NULL row reaches the same `owner == null` arm as the plugin + // driver's row. The key is present, so it is an ordinary owner-less row. + expect(await svc.checkEdit('ext_sql_mapped', 's_nobody', ctx())).toBe('deny'); + expect(warn).not.toHaveBeenCalled(); + }); + + it('a declared-owner control is silent, allow and deny alike', async () => { + expect(await svc.checkEdit('ext_with_real_owner', 'a1', ctx())).toBe('allow'); + expect(await svc.checkEdit('ext_with_real_owner', 'a2', ctx())).toBe('deny'); + expect(warn).not.toHaveBeenCalled(); + }); + + it('a record that does not exist says nothing about the driver', async () => { + expect(await svc.checkEdit('ext_plugin_mapped', 'missing', ctx())).toBe('deny'); + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/index.ts b/packages/plugins/plugin-sharing/src/index.ts index 1016a067f28..dfd546f8caa 100644 --- a/packages/plugins/plugin-sharing/src/index.ts +++ b/packages/plugins/plugin-sharing/src/index.ts @@ -22,6 +22,7 @@ export { SharingService, effectiveSharingModel, PHANTOM_ANCHOR_WRITE_DENY_NOTICE, + UNSERVED_OWNER_WRITE_DENY_NOTICE, type SharingEngine, type SharingServiceOptions, type OrphanShareSweepOptions, diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 72c26f6eac2..558afc3b321 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -36,7 +36,7 @@ import { resolveTenantFieldName } from '@objectstack/objectql'; import { owdForAudience } from '@objectstack/core'; import { MEMBERSHIP_ROLE_EXTERNAL } from '@objectstack/spec/identity'; import { WRITE_ACCESS_LEVELS, normalizeAccessLevel } from './access-level.js'; -import { hasPhantomOwnerAnchor } from './federated-phantom-anchors.js'; +import { hasPhantomOwnerAnchor, isFederatedObject } from './federated-phantom-anchors.js'; import { deleteRowsForDeletedRecords, sweepOrphanedRowsByRecordExistence, @@ -403,6 +403,28 @@ export const PHANTOM_ANCHOR_WRITE_DENY_NOTICE = 'injected anchor, not a remote column, so ownership can never match; ' + 'declare the real remote owner column or change the sharing model'; +/** + * [#27662] The second silent cause of the same refusal: a federated object + * whose owner column is NOT the phantom anchor — the author mapped a remote + * column onto `owner_id` through `external.columnMap`, or declared `owner_id` + * outright — served by a driver that returns rows with no `owner_id` key at + * all. A plugin driver that does not apply `columnMap` is the measured case: it + * answers with the remote column's own name, so the pre-image has no owner, and + * every member write is refused exactly as on a phantom anchor (fail closed, + * correct). Since #27425 took the mapped class out of + * {@link hasPhantomOwnerAnchor}, {@link PHANTOM_ANCHOR_WRITE_DENY_NOTICE} no + * longer fires for it, and its wording ("injected anchor") would be wrong if it + * did — the metadata is right here, the driver is what drops the column. + * + * Keyed on the row, not on which of the two declarations the author used: an + * absent key is the one fact both share, and a present `null` (an ordinary + * owner-less row over a driver that DOES serve the column) stays silent. + */ +export const UNSERVED_OWNER_WRITE_DENY_NOTICE = + '[sharing] write refused: the driver returned no value for the owner column this ' + + 'federated object maps (external.columnMap) or declares, so ownership can never match; ' + + 'use a driver that serves the mapped owner column or change the sharing model'; + /** * [ADR-0111 D2] The narrow slice of `ISecurityService` the management gate * needs — kept structural so unit tests can pass a stub and so a deployment @@ -946,7 +968,8 @@ export class SharingService implements ISharingService { limit: 1, context: SYSTEM_CTX, }); - const owner = Array.isArray(own) && own[0] ? (own[0] as any)[OWNER_FIELD] : undefined; + const row = Array.isArray(own) ? (own[0] as Record | undefined) : undefined; + const owner = row ? row[OWNER_FIELD] : undefined; if (owner == null) { // [#8418] The refusal is unchanged — `return false` below is the same // `false` this line has always returned, and the diagnostic is emitted @@ -955,8 +978,10 @@ export class SharingService implements ISharingService { // actually defeats the fast path, not the mere fact that the object is // federated: an author-declared remote `owner_id` that happens to be NULL // on this row reaches here too, and `hasPhantomOwnerAnchor` is what tells - // the two apart (provenance, never an `external` test). - this.notePhantomAnchorWriteDeny(object, context); + // the two apart (provenance, never an `external` test). [#27662] The row + // goes along so a mapped or declared owner column the driver did not + // serve is told apart from one it served as NULL. + this.notePhantomAnchorWriteDeny(object, context, row); return false; } // Middleware-stamped, not a field of the envelope — see buildReadFilter(). @@ -984,14 +1009,28 @@ export class SharingService implements ISharingService { * that can misreport the thing it exists to explain is worse than silence. * The latch is claimed BEFORE the log so a throwing logger cannot turn one * suppressed line into one throw per row (#6783's ordering, same reason). + * + * [#27662] A federated object whose owner column is real by declaration (a + * `columnMap` entry onto `owner_id`, or a declared field) gets + * {@link UNSERVED_OWNER_WRITE_DENY_NOTICE} instead, under the same latch, + * when the pre-image `row` came back with no `owner_id` key. No row at all + * (the record does not exist) says nothing about the driver and stays silent. */ - private notePhantomAnchorWriteDeny(object: string, context: ExecutionContext): void { + private notePhantomAnchorWriteDeny( + object: string, + context: ExecutionContext, + row: Record | undefined, + ): void { if (this.phantomAnchorNotified.has(object)) return; try { const schema = this.engine.getSchema?.(object); - if (!hasPhantomOwnerAnchor(schema)) return; + let notice: string; + if (hasPhantomOwnerAnchor(schema)) notice = PHANTOM_ANCHOR_WRITE_DENY_NOTICE; + else if (isFederatedObject(schema) && row !== undefined && row[OWNER_FIELD] === undefined) { + notice = UNSERVED_OWNER_WRITE_DENY_NOTICE; + } else return; this.phantomAnchorNotified.add(object); - this.logger?.warn?.(PHANTOM_ANCHOR_WRITE_DENY_NOTICE, { + this.logger?.warn?.(notice, { object, ownerField: OWNER_FIELD, userId: context?.userId ?? 'unknown',