Skip to content

Commit 64599ec

Browse files
fix(plugin-sharing): a refused write on a federated object whose mapped owner column the driver did not serve is logged again (#27760)
<!-- claude-projects-attribution: {"github_login":"marchtian"} --> _Requested by **田騏** · [project thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSiDTkG7yDwxUsQaHUFmCQQgM)_ Fixes #27662 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么:** 外部对象用 `columnMap` 把远端列映到 `owner_id`、但驱动没把这列返回时,成员写入被拒的同时,服务器重新打出一行 warn(每个对象一次),点名「驱动没有返回映射的 owner 列」。 - **为什么改:** #27425 把这种映射算作已证 owner 后,#8418 的诊断不再触发,运维只看到一个 403,没有任何日志。 - **风险与回滚:** 只加日志,判决不变(三个写判决矩阵用例照旧);回滚即 revert 本 PR。 - **先看:** `packages/plugins/plugin-sharing/src/sharing-service.ts` 的 `notePhantomAnchorWriteDeny`(新分支按「行里没有 `owner_id` 键」判定);同文件新常量 `UNSERVED_OWNER_WRITE_DENY_NOTICE`;测试文件末尾的 `[#27662]` 块。 - **两分钟验证:** `pnpm --filter @objectstack/plugin-sharing exec vitest run src/federated-phantom-owner-scoping.test.ts`,预期 57 个用例全绿,其中 `[#27662]` 块 6 个。 - **需要你做的:** 无,席位落地。 ## What changes - `matchesOwnerScope` passes the pre-image row it already read to the #8418 diagnostic. No new query. - `notePhantomAnchorWriteDeny` keeps its phantom-anchor branch byte-for-byte. A second branch fires when the object is federated, is not phantom-anchored, and the row came back with no `owner_id` key at all. It logs the new exported `UNSERVED_OWNER_WRITE_DENY_NOTICE` with the same payload (`object`, `ownerField`, `userId`) under the same once-per-object latch. - 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 and stays silent. A missing record (no row) stays silent too. - `hasPhantomOwnerAnchor` and `federated-phantom-anchors.ts` are not touched: the new branch reuses the already-exported `isFederatedObject` and reads no `columnMap` itself, so it adds no fourth reader to the set #27661 is consolidating. ## Tests New `[#27662]` block in `federated-phantom-owner-scoping.test.ts`: - a map-ignoring plugin driver denies on edit, delete and at `org` depth, and logs exactly one unserved-owner notice (never the phantom one); - the wording names the driver and `external.columnMap`; - a second object gets its own line, a second caller does not re-arm the first; - a map-applying driver stays silent on an own row (allow), another owner (deny) and a served NULL owner (deny); - a declared-owner control stays silent on allow and deny; - a record that does not exist stays silent. ## Acceptance notes - **Wider than "mapped" by design.** The new branch also fires for a federated object that DECLARES `owner_id` when its driver returns rows without that key (for example the SQL driver discarding a projection that names a column the remote table lacks). That is the same operator-facing fault, and the message says "maps (external.columnMap) or declares". The card's declared-owner control (owner present) is silent, as pinned. - The method keeps its `notePhantomAnchorWriteDeny` name to keep the diff small; its doc comment now names both notices. ## Verification Readings for head `41cbab7c` are in the os-dev-report on the card. --- _Generated by [Claude Code](https://claude.ai/code/session_017ihL2eGmV67aXQuY2TJg7s)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent ca64f6f commit 64599ec

4 files changed

Lines changed: 171 additions & 8 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/plugin-sharing': patch
3+
---
4+
5+
fix(plugin-sharing): a refused write on a federated object whose mapped owner column the driver did not serve is logged again
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
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.

‎packages/plugins/plugin-sharing/src/federated-phantom-owner-scoping.test.ts‎

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,11 @@ import {
5757
type EngineUpdateDispatchInput,
5858
} from '@objectstack/metadata-core';
5959
import { ERROR_CODE_LEDGER } from '@objectstack/spec/api';
60-
import { SharingService, PHANTOM_ANCHOR_WRITE_DENY_NOTICE } from './sharing-service.js';
60+
import {
61+
SharingService,
62+
PHANTOM_ANCHOR_WRITE_DENY_NOTICE,
63+
UNSERVED_OWNER_WRITE_DENY_NOTICE,
64+
} from './sharing-service.js';
6165

6266
/** The caller: an ordinary member whose read/write DEPTH is narrower than `org`. */
6367
const MEMBER = 'usr_member_1';
@@ -821,3 +825,112 @@ describe('[#8418] the phantom-anchor write refusal is diagnosed, not changed', (
821825
});
822826
});
823827
});
828+
829+
/**
830+
* [#27662] The mapped owner column the driver never served is diagnosed too.
831+
*
832+
* #27425 counted an `external.columnMap` entry onto `owner_id` as the author
833+
* declaring the owner column, which took the object out of
834+
* `hasPhantomOwnerAnchor` — and so out of the #8418 line above. A plugin driver
835+
* that does not apply the map answers with the remote column's own name, the
836+
* pre-image carries no `owner_id`, and every member write is refused in
837+
* silence again. The refusal is right and stays; the silence is what goes.
838+
*
839+
* The discrimination is the KEY, not the value: a driver that applies the map
840+
* and serves a NULL owner is an ordinary owner-less row (silent, like
841+
* `LOCAL_OWNERLESS` above), while a row with no `owner_id` key at all is the
842+
* driver dropping the column.
843+
*/
844+
describe('[#27662] a mapped owner column the driver did not serve is diagnosed, not changed', () => {
845+
const OTHER = 'usr_other';
846+
const mapped = (name: string) =>
847+
federatedSchema({ name, external: { remoteName: 'rows', columnMap: { assignee: 'owner_id' } } });
848+
849+
let svc: SharingService;
850+
let warn: ReturnType<typeof vi.fn>;
851+
852+
beforeEach(() => {
853+
warn = vi.fn();
854+
svc = new SharingService({
855+
engine: makeEngine(
856+
{
857+
// A plugin driver that ignores the map: rows carry `assignee`, never `owner_id`.
858+
ext_plugin_mapped: mapped('ext_plugin_mapped'),
859+
ext_plugin_mapped_2: mapped('ext_plugin_mapped_2'),
860+
// The SQL driver applies the map, so `owner_id` is served (possibly NULL).
861+
ext_sql_mapped: mapped('ext_sql_mapped'),
862+
ext_with_real_owner: DECLARED_REAL_OWNER,
863+
sys_record_share: { name: 'sys_record_share' },
864+
},
865+
{
866+
ext_plugin_mapped: [{ id: 'p1', assignee: MEMBER, name: 'Mine, unmapped' }],
867+
ext_plugin_mapped_2: [{ id: 'q1', assignee: MEMBER, name: 'Also unmapped' }],
868+
ext_sql_mapped: [
869+
{ id: 's_mine', owner_id: MEMBER, name: 'Mine' },
870+
{ id: 's_theirs', owner_id: OTHER, name: 'Theirs' },
871+
{ id: 's_nobody', owner_id: null, name: 'Owner-less' },
872+
],
873+
ext_with_real_owner: [
874+
{ id: 'a1', owner_id: MEMBER, name: 'Acme' },
875+
{ id: 'a2', owner_id: OTHER, name: 'Globex' },
876+
],
877+
},
878+
),
879+
logger: { info: vi.fn(), warn, error: vi.fn() } as never,
880+
});
881+
});
882+
883+
const ctx = (scope = 'own') => ({ userId: MEMBER, __writeScope: scope }) as never;
884+
const notices = () => warn.mock.calls.filter((c) => c[0] === UNSERVED_OWNER_WRITE_DENY_NOTICE);
885+
const phantomNotices = () => warn.mock.calls.filter((c) => c[0] === PHANTOM_ANCHOR_WRITE_DENY_NOTICE);
886+
887+
it('a map-ignoring plugin driver: still DENIES, and says why once', async () => {
888+
// The caller's id IS in the row — under `assignee`, where the gate cannot
889+
// see it. So the refusal is the driver's doing, not the caller's.
890+
expect(await svc.checkEdit('ext_plugin_mapped', 'p1', ctx())).toBe('deny');
891+
expect(await svc.checkDelete('ext_plugin_mapped', 'p1', ctx())).toBe('deny');
892+
expect(await svc.checkEdit('ext_plugin_mapped', 'p1', ctx('org'))).toBe('deny');
893+
894+
expect(notices()).toHaveLength(1);
895+
expect(notices()[0][1]).toMatchObject({ object: 'ext_plugin_mapped', ownerField: 'owner_id', userId: MEMBER });
896+
// …and not the phantom wording, which would send the operator to the metadata.
897+
expect(phantomNotices()).toHaveLength(0);
898+
});
899+
900+
it('the wording names the driver and the mapped owner column', async () => {
901+
await svc.checkEdit('ext_plugin_mapped', 'p1', ctx());
902+
const [message] = notices()[0];
903+
expect(message).toContain('the driver returned no value for the owner column');
904+
expect(message).toContain('external.columnMap');
905+
});
906+
907+
it('a SECOND map-ignoring object gets its own line; a different caller does not re-arm one', async () => {
908+
await svc.checkEdit('ext_plugin_mapped', 'p1', ctx());
909+
await svc.checkEdit('ext_plugin_mapped', 'p1', { userId: OTHER, __writeScope: 'own' } as never);
910+
await svc.checkEdit('ext_plugin_mapped_2', 'q1', ctx());
911+
expect(notices().map((c) => (c[1] as { object: string }).object)).toEqual([
912+
'ext_plugin_mapped',
913+
'ext_plugin_mapped_2',
914+
]);
915+
});
916+
917+
it('a map-applying driver is silent: own row allows, another owner and a NULL owner deny', async () => {
918+
expect(await svc.checkEdit('ext_sql_mapped', 's_mine', ctx())).toBe('allow');
919+
expect(await svc.checkEdit('ext_sql_mapped', 's_theirs', ctx())).toBe('deny');
920+
// The served-NULL row reaches the same `owner == null` arm as the plugin
921+
// driver's row. The key is present, so it is an ordinary owner-less row.
922+
expect(await svc.checkEdit('ext_sql_mapped', 's_nobody', ctx())).toBe('deny');
923+
expect(warn).not.toHaveBeenCalled();
924+
});
925+
926+
it('a declared-owner control is silent, allow and deny alike', async () => {
927+
expect(await svc.checkEdit('ext_with_real_owner', 'a1', ctx())).toBe('allow');
928+
expect(await svc.checkEdit('ext_with_real_owner', 'a2', ctx())).toBe('deny');
929+
expect(warn).not.toHaveBeenCalled();
930+
});
931+
932+
it('a record that does not exist says nothing about the driver', async () => {
933+
expect(await svc.checkEdit('ext_plugin_mapped', 'missing', ctx())).toBe('deny');
934+
expect(warn).not.toHaveBeenCalled();
935+
});
936+
});

‎packages/plugins/plugin-sharing/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export {
2222
SharingService,
2323
effectiveSharingModel,
2424
PHANTOM_ANCHOR_WRITE_DENY_NOTICE,
25+
UNSERVED_OWNER_WRITE_DENY_NOTICE,
2526
type SharingEngine,
2627
type SharingServiceOptions,
2728
type OrphanShareSweepOptions,

‎packages/plugins/plugin-sharing/src/sharing-service.ts‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import { resolveTenantFieldName } from '@objectstack/objectql';
3636
import { owdForAudience } from '@objectstack/core';
3737
import { MEMBERSHIP_ROLE_EXTERNAL } from '@objectstack/spec/identity';
3838
import { WRITE_ACCESS_LEVELS, normalizeAccessLevel } from './access-level.js';
39-
import { hasPhantomOwnerAnchor } from './federated-phantom-anchors.js';
39+
import { hasPhantomOwnerAnchor, isFederatedObject } from './federated-phantom-anchors.js';
4040
import {
4141
deleteRowsForDeletedRecords,
4242
sweepOrphanedRowsByRecordExistence,
@@ -403,6 +403,28 @@ export const PHANTOM_ANCHOR_WRITE_DENY_NOTICE =
403403
'injected anchor, not a remote column, so ownership can never match; ' +
404404
'declare the real remote owner column or change the sharing model';
405405

406+
/**
407+
* [#27662] The second silent cause of the same refusal: a federated object
408+
* whose owner column is NOT the phantom anchor — the author mapped a remote
409+
* column onto `owner_id` through `external.columnMap`, or declared `owner_id`
410+
* outright — served by a driver that returns rows with no `owner_id` key at
411+
* all. A plugin driver that does not apply `columnMap` is the measured case: it
412+
* answers with the remote column's own name, so the pre-image has no owner, and
413+
* every member write is refused exactly as on a phantom anchor (fail closed,
414+
* correct). Since #27425 took the mapped class out of
415+
* {@link hasPhantomOwnerAnchor}, {@link PHANTOM_ANCHOR_WRITE_DENY_NOTICE} no
416+
* longer fires for it, and its wording ("injected anchor") would be wrong if it
417+
* did — the metadata is right here, the driver is what drops the column.
418+
*
419+
* Keyed on the row, not on which of the two declarations the author used: an
420+
* absent key is the one fact both share, and a present `null` (an ordinary
421+
* owner-less row over a driver that DOES serve the column) stays silent.
422+
*/
423+
export const UNSERVED_OWNER_WRITE_DENY_NOTICE =
424+
'[sharing] write refused: the driver returned no value for the owner column this ' +
425+
'federated object maps (external.columnMap) or declares, so ownership can never match; ' +
426+
'use a driver that serves the mapped owner column or change the sharing model';
427+
406428
/**
407429
* [ADR-0111 D2] The narrow slice of `ISecurityService` the management gate
408430
* needs — kept structural so unit tests can pass a stub and so a deployment
@@ -946,7 +968,8 @@ export class SharingService implements ISharingService {
946968
limit: 1,
947969
context: SYSTEM_CTX,
948970
});
949-
const owner = Array.isArray(own) && own[0] ? (own[0] as any)[OWNER_FIELD] : undefined;
971+
const row = Array.isArray(own) ? (own[0] as Record<string, unknown> | undefined) : undefined;
972+
const owner = row ? row[OWNER_FIELD] : undefined;
950973
if (owner == null) {
951974
// [#8418] The refusal is unchanged — `return false` below is the same
952975
// `false` this line has always returned, and the diagnostic is emitted
@@ -955,8 +978,10 @@ export class SharingService implements ISharingService {
955978
// actually defeats the fast path, not the mere fact that the object is
956979
// federated: an author-declared remote `owner_id` that happens to be NULL
957980
// on this row reaches here too, and `hasPhantomOwnerAnchor` is what tells
958-
// the two apart (provenance, never an `external` test).
959-
this.notePhantomAnchorWriteDeny(object, context);
981+
// the two apart (provenance, never an `external` test). [#27662] The row
982+
// goes along so a mapped or declared owner column the driver did not
983+
// serve is told apart from one it served as NULL.
984+
this.notePhantomAnchorWriteDeny(object, context, row);
960985
return false;
961986
}
962987
// Middleware-stamped, not a field of the envelope — see buildReadFilter().
@@ -984,14 +1009,28 @@ export class SharingService implements ISharingService {
9841009
* that can misreport the thing it exists to explain is worse than silence.
9851010
* The latch is claimed BEFORE the log so a throwing logger cannot turn one
9861011
* suppressed line into one throw per row (#6783's ordering, same reason).
1012+
*
1013+
* [#27662] A federated object whose owner column is real by declaration (a
1014+
* `columnMap` entry onto `owner_id`, or a declared field) gets
1015+
* {@link UNSERVED_OWNER_WRITE_DENY_NOTICE} instead, under the same latch,
1016+
* when the pre-image `row` came back with no `owner_id` key. No row at all
1017+
* (the record does not exist) says nothing about the driver and stays silent.
9871018
*/
988-
private notePhantomAnchorWriteDeny(object: string, context: ExecutionContext): void {
1019+
private notePhantomAnchorWriteDeny(
1020+
object: string,
1021+
context: ExecutionContext,
1022+
row: Record<string, unknown> | undefined,
1023+
): void {
9891024
if (this.phantomAnchorNotified.has(object)) return;
9901025
try {
9911026
const schema = this.engine.getSchema?.(object);
992-
if (!hasPhantomOwnerAnchor(schema)) return;
1027+
let notice: string;
1028+
if (hasPhantomOwnerAnchor(schema)) notice = PHANTOM_ANCHOR_WRITE_DENY_NOTICE;
1029+
else if (isFederatedObject(schema) && row !== undefined && row[OWNER_FIELD] === undefined) {
1030+
notice = UNSERVED_OWNER_WRITE_DENY_NOTICE;
1031+
} else return;
9931032
this.phantomAnchorNotified.add(object);
994-
this.logger?.warn?.(PHANTOM_ANCHOR_WRITE_DENY_NOTICE, {
1033+
this.logger?.warn?.(notice, {
9951034
object,
9961035
ownerField: OWNER_FIELD,
9971036
userId: context?.userId ?? 'unknown',

0 commit comments

Comments
 (0)