Skip to content

Commit bddb263

Browse files
committed
fix(metadata-protocol): duplicatePackage re-points a copied permission set's fields keys at the renamed objects
Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude <noreply@anthropic.com>
1 parent b1cfa3d commit bddb263

2 files changed

Lines changed: 357 additions & 0 deletions

File tree

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26299,6 +26299,10 @@ export class ObjectStackProtocolImplementation implements
2629926299
* when its target name is already claimed (the source also holds
2630026300
* `iojn_ticket`) the object is refused in `failed[]`, before anything is
2630126301
* written for it, with the source rename that clears it.
26302+
*
26303+
* A copied permission set's field rules follow too: in `fields`, keyed
26304+
* `<object>.<field>`, a key's object part takes the renamed object's name
26305+
* and its field part is left as stored.
2630226306
*/
2630326307
async duplicatePackage(request: {
2630426308
sourcePackageId: string;
@@ -26542,6 +26546,59 @@ export class ObjectStackProtocolImplementation implements
2654226546
}
2654326547
return v;
2654426548
};
26549+
// ⭐ A COPIED PERMISSION SET'S FIELD RULES FOLLOW THE RENAMED OBJECTS.
26550+
// `PermissionSetSchema.fields` is a record keyed `<entity_name>.<field_name>`
26551+
// (`packages/spec/src/security/permission.zod.ts`), and the security
26552+
// plugin reads a set's rules for an object by the key prefix
26553+
// `<object>.` (`PermissionEvaluator.getFieldPermissions`). Left as
26554+
// stored, the copy's rules govern the SOURCE's object and the
26555+
// duplicate's object gets none: a field the source set hides is
26556+
// readable on the copy for whoever holds the copied set.
26557+
//
26558+
// `deepRewrite` cannot carry it: the derived site index reads
26559+
// `permission.fields` as a site naming `field` items (its property
26560+
// spells that type), never an object, and has no arm for a composite
26561+
// key — and widening the index would also change the "Used by" answer
26562+
// it feeds (`findReferencesToMeta`). So this step is keyed on the
26563+
// declared shape — the `fields` record of a copied `permission` item —
26564+
// never on a property name, which would also catch an object's own
26565+
// `fields` (keyed by field name).
26566+
//
26567+
// The split: object and field names are both `^[a-z_][a-z0-9_]*$`, so
26568+
// a key's object part is everything before its FIRST `.`. It is
26569+
// rewritten only when it equals a renamed object name exactly, and the
26570+
// field part is never read — `dupsrc_customer.dupsrc_customer` becomes
26571+
// `dupdst_customer.dupsrc_customer`. A key naming an object outside the
26572+
// package keeps its name.
26573+
//
26574+
// ⛔ REFUSED, NEVER MERGED: when a renamed key equals a key the set
26575+
// already holds (the source also governs an object, outside the
26576+
// package, named like the copy), keeping either rule would silently
26577+
// drop the other, so the set is reported in `failed[]` with both keys.
26578+
// `Object.fromEntries` defines own properties, so no key can land on
26579+
// the prototype.
26580+
const rewriteFieldRuleKeys = (fields: Record<string, unknown>): { fields: Record<string, unknown> } | { collision: string } => {
26581+
const out = new Map<string, unknown>();
26582+
const from = new Map<string, string>();
26583+
for (const [key, rule] of Object.entries(fields)) {
26584+
const dot = key.indexOf('.');
26585+
const renamed = dot > 0 ? renameMap.get(key.slice(0, dot)) : undefined;
26586+
const next = renamed === undefined ? key : `${renamed}${key.slice(dot)}`;
26587+
const other = from.get(next);
26588+
if (other !== undefined) {
26589+
const moved = next === key ? other : key;
26590+
return {
26591+
collision: `its field rule '${moved}' follows object '${moved.slice(0, moved.indexOf('.'))}' `
26592+
+ `to '${next}', a key the set already holds for object `
26593+
+ `'${next.slice(0, next.indexOf('.'))}', which this copy does not rename. `
26594+
+ `Remove '${moved}' or '${next}' on the source, then duplicate again.`,
26595+
};
26596+
}
26597+
out.set(next, rule);
26598+
from.set(next, key);
26599+
}
26600+
return { fields: Object.fromEntries(out) };
26601+
};
2654526602

2654626603
if (srcPkg?.manifest && typeof registry?.installPackage === 'function') {
2654726604
try {
@@ -26671,6 +26728,19 @@ export class ObjectStackProtocolImplementation implements
2667126728
if (singular === 'permission' && rewritten.packageId === request.sourcePackageId) {
2667226729
rewritten.packageId = request.targetPackageId;
2667326730
}
26731+
if (singular === 'permission' && rewritten.fields
26732+
&& typeof rewritten.fields === 'object' && !Array.isArray(rewritten.fields)) {
26733+
const result = rewriteFieldRuleKeys(rewritten.fields);
26734+
if ('collision' in result) {
26735+
failed.push({
26736+
type: row.type,
26737+
name: row.name,
26738+
error: `Permission set '${row.name}' is not copied: ${result.collision}`,
26739+
});
26740+
continue;
26741+
}
26742+
rewritten.fields = result.fields;
26743+
}
2667426744
}
2667526745
try {
2667626746
await this.saveMetaItem({
Lines changed: 287 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,287 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* ADR-0070 D4 — `duplicatePackage` re-points a copied permission set's
5+
* field-level rules at the copied objects: in `PermissionSetSchema.fields`,
6+
* keyed `<entity_name>.<field_name>`, a key's OBJECT part follows the renamed
7+
* object, and nothing else about the key moves.
8+
*
9+
* ## The defect
10+
*
11+
* The copy's `objects` map followed the renamed objects, but its `fields` map
12+
* kept the source's keys (`dupsrc_customer.full_name`). The security plugin
13+
* reads a set's field rules for an object by the key prefix `<object>.`
14+
* (`PermissionEvaluator.getFieldPermissions`), so the copy's rules applied to
15+
* the SOURCE's object and the duplicate's object got none: a field the source
16+
* set hid was readable on the duplicate for whoever held the copied set.
17+
*
18+
* ## The split rule
19+
*
20+
* Object names and field names are both `^[a-z_][a-z0-9_]*$`, so a key's
21+
* object part is everything before its FIRST `.`. That part is rewritten only
22+
* when it equals a renamed object name exactly; the field part is never read.
23+
* A renamed key that meets a key the set already holds is refused in
24+
* `failed[]`, because keeping either rule would silently drop the other.
25+
*
26+
* ## Why a real kernel
27+
*
28+
* `ObjectQLPlugin` binds the real protocol to the real engine and registry, so
29+
* every pin reads the stored `sys_metadata` rows out of the driver, never a
30+
* payload.
31+
*/
32+
33+
import { randomUUID } from 'node:crypto';
34+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
35+
import { ObjectKernel } from '@objectstack/core';
36+
import { ObjectQL } from './engine.js';
37+
import { ObjectQLPlugin } from './plugin.js';
38+
39+
type Stores = Map<string, Map<string, Record<string, unknown>>>;
40+
41+
/** In-memory driver, copied from `protocol-duplicate-package-unprefixed-object.test.ts`; equality-only WHERE. */
42+
function makeMemoryDriver(stores: Stores) {
43+
const storeFor = (obj: string) => {
44+
let s = stores.get(obj);
45+
if (!s) { s = new Map(); stores.set(obj, s); }
46+
return s;
47+
};
48+
// `$and` / `$or` are conjoined WITH their sibling keys, the way a real
49+
// driver ANDs them.
50+
const matchesWhere = (row: Record<string, unknown>, where: any): boolean => {
51+
if (!where || typeof where !== 'object') return true;
52+
for (const [k, v] of Object.entries(where)) {
53+
if (k === '$and' && Array.isArray(v)) {
54+
if (!v.every((w: any) => matchesWhere(row, w))) return false;
55+
continue;
56+
}
57+
if (k === '$or' && Array.isArray(v)) {
58+
if (!v.some((w: any) => matchesWhere(row, w))) return false;
59+
continue;
60+
}
61+
if (k.startsWith('$')) continue;
62+
const rowVal = row[k];
63+
const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
64+
const a = rowVal === undefined ? null : rowVal;
65+
const b = expected === undefined ? null : expected;
66+
if (a !== b) return false;
67+
}
68+
return true;
69+
};
70+
const driver: any = {
71+
name: 'memory', version: '0.0.0', supports: {} as any,
72+
async connect() {}, async disconnect() {}, async checkHealth() { return true; },
73+
async execute() { return null; },
74+
async find(object: string, ast: any) {
75+
const rows = Array.from(storeFor(object).values()).filter((r) => matchesWhere(r, ast?.where));
76+
// The caller's bound, after the filter, by presence (`check:objectql-double-limit`).
77+
return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows;
78+
},
79+
async findOne(object: string, ast: any) {
80+
for (const r of storeFor(object).values()) if (matchesWhere(r, ast?.where)) return r;
81+
return null;
82+
},
83+
async create(object: string, data: Record<string, unknown>) {
84+
const id = (data.id as string) ?? `r_${randomUUID()}`;
85+
const row = { ...data, id };
86+
storeFor(object).set(id, row);
87+
return row;
88+
},
89+
async update(object: string, id: string, data: Record<string, unknown>) {
90+
const s = storeFor(object);
91+
const cur = s.get(id);
92+
if (!cur) throw new Error(`not found: ${object}/${id}`);
93+
const updated = { ...cur, ...data, id };
94+
s.set(id, updated);
95+
return updated;
96+
},
97+
async upsert(object: string, data: Record<string, unknown>) {
98+
const id = data.id as string | undefined;
99+
if (id && storeFor(object).has(id)) return this.update(object, id, data);
100+
return this.create(object, data);
101+
},
102+
async delete(object: string, id: string) { return storeFor(object).delete(id); },
103+
async count(object: string, ast: any) { return (await this.find(object, ast)).length; },
104+
async bulkCreate(object: string, rows: Record<string, unknown>[]) {
105+
return Promise.all(rows.map((r) => this.create(object, r)));
106+
},
107+
async bulkUpdate() { return []; }, async bulkDelete() {},
108+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
109+
async commit() {}, async rollback() {},
110+
};
111+
return driver;
112+
}
113+
114+
/** Source package; `installPackage` derives its namespace `dupsrc`. */
115+
const P = 'com.qa.dupsrc';
116+
/** Target package; namespace `dupdst`, derived the same way. */
117+
const Q = 'com.qa.dupdst';
118+
/** A third package, whose object the source's set also governs. */
119+
const R = 'com.qa.dupext';
120+
121+
const HIDDEN = { readable: false, editable: false };
122+
const LOCKED = { readable: true, editable: false };
123+
const OPEN = { readable: true, editable: true };
124+
125+
/** A prefixed object with a field that spells the object's own name. */
126+
const customerBody = {
127+
name: 'dupsrc_customer', label: 'Customer', sharingModel: 'private',
128+
fields: {
129+
full_name: { name: 'full_name', type: 'text', label: 'Full name' },
130+
dupsrc_customer: { name: 'dupsrc_customer', type: 'text', label: 'Customer code' },
131+
},
132+
};
133+
/** An object whose name lacks the `dupsrc_` prefix, likewise. */
134+
const ticketBody = {
135+
name: 'ticket', label: 'Ticket', sharingModel: 'private',
136+
fields: {
137+
title: { name: 'title', type: 'text', label: 'Title' },
138+
ticket: { name: 'ticket', type: 'text', label: 'Ticket number' },
139+
},
140+
};
141+
/** An object of another package: not copied, so not renamed. */
142+
const accountBody = {
143+
name: 'dupext_account', label: 'Account', sharingModel: 'private',
144+
fields: { name: { name: 'name', type: 'text', label: 'Name' } },
145+
};
146+
const flsBody = {
147+
name: 'dupsrc_fls', label: 'Field rules',
148+
objects: {
149+
dupsrc_customer: { allowRead: true, allowEdit: true },
150+
ticket: { allowRead: true, allowEdit: true },
151+
dupext_account: { allowRead: true },
152+
},
153+
fields: {
154+
'dupsrc_customer.full_name': HIDDEN,
155+
'dupsrc_customer.dupsrc_customer': LOCKED,
156+
'ticket.title': HIDDEN,
157+
'ticket.ticket': OPEN,
158+
'dupext_account.name': LOCKED,
159+
},
160+
};
161+
162+
describe('ADR-0070 D4 — duplicatePackage re-points a copied permission set\'s `fields` keys', () => {
163+
let kernel: ObjectKernel;
164+
let stores: Stores;
165+
let engine: ObjectQL;
166+
let protocol: any;
167+
168+
beforeEach(async () => {
169+
// `logger.level` is the real config key, and gracefulShutdown:false keeps
170+
// process signal handlers out of vitest's fork workers (#4250).
171+
kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false });
172+
stores = new Map();
173+
engine = new ObjectQL();
174+
engine.registerDriver(makeMemoryDriver(stores), true);
175+
await kernel.use(new ObjectQLPlugin({ ql: engine }));
176+
await kernel.bootstrap();
177+
protocol = kernel.getService('protocol');
178+
});
179+
afterEach(async () => {
180+
if (kernel.getState() === 'running') await kernel.shutdown();
181+
});
182+
183+
/** The stored `sys_metadata` rows, as the driver holds them. */
184+
const storedRows = () => Array.from(stores.get('sys_metadata')?.values() ?? []);
185+
const storedBody = (type: string, name: string, packageId: string): any => {
186+
const row = storedRows().find((r) => r.type === type && r.name === name && r.package_id === packageId);
187+
if (!row) return undefined;
188+
return typeof row.metadata === 'string' ? JSON.parse(row.metadata) : row.metadata;
189+
};
190+
191+
async function save(packageId: string, type: string, item: { name: string }): Promise<void> {
192+
const res = await protocol.saveMetaItem({ type, name: item.name, packageId, item });
193+
expect(res.success).toBe(true);
194+
}
195+
196+
/** Seeds the source and the third package, duplicates, and returns the copy's `fields`. */
197+
async function duplicate(): Promise<Record<string, unknown>> {
198+
await protocol.installPackage({ manifest: { id: R, name: 'Other', type: 'application' } });
199+
await save(R, 'object', accountBody);
200+
await protocol.installPackage({ manifest: { id: P, name: 'Source', type: 'application' } });
201+
expect(engine.registry.getPackage(P)?.manifest?.namespace).toBe('dupsrc');
202+
await save(P, 'object', customerBody);
203+
await save(P, 'object', ticketBody);
204+
await save(P, 'permission', flsBody);
205+
206+
const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q });
207+
208+
expect(res.failed).toEqual([]);
209+
expect(res.copied).toEqual(expect.arrayContaining([
210+
{ type: 'object', name: 'dupdst_customer' },
211+
{ type: 'object', name: 'dupdst_ticket' },
212+
{ type: 'permission', name: 'dupdst_fls' },
213+
]));
214+
const copy = storedBody('permission', 'dupdst_fls', Q);
215+
expect(copy).toBeDefined();
216+
return copy.fields;
217+
}
218+
219+
it('a prefixed object\'s key follows its renamed object, with its rule', async () => {
220+
const fields = await duplicate();
221+
222+
expect(fields['dupdst_customer.full_name']).toEqual(HIDDEN);
223+
// NEGATIVE CONTROL: no key of the copy names the source's object.
224+
expect(Object.keys(fields).filter((key) => key.startsWith('dupsrc_customer.'))).toEqual([]);
225+
});
226+
227+
it('a bare object\'s key follows the target-namespace name the copy took', async () => {
228+
const fields = await duplicate();
229+
230+
expect(fields['dupdst_ticket.title']).toEqual(HIDDEN);
231+
// NEGATIVE CONTROL: no key of the copy names the source's `ticket`.
232+
expect(Object.keys(fields).filter((key) => key.startsWith('ticket.'))).toEqual([]);
233+
});
234+
235+
it('only the object part moves: a field part that spells the object\'s name stays', async () => {
236+
const fields = await duplicate();
237+
238+
expect(fields['dupdst_customer.dupsrc_customer']).toEqual(LOCKED);
239+
expect(fields['dupdst_ticket.ticket']).toEqual(OPEN);
240+
// NEGATIVE CONTROL: the field part was not renamed with the object.
241+
expect(fields).not.toHaveProperty(['dupdst_customer.dupdst_customer']);
242+
expect(fields).not.toHaveProperty(['dupdst_ticket.dupdst_ticket']);
243+
});
244+
245+
it('a key naming an object outside the package keeps its name, and the source set is untouched', async () => {
246+
const fields = await duplicate();
247+
248+
// NEGATIVE CONTROL: an object the package does not hold is not renamed.
249+
expect(fields['dupext_account.name']).toEqual(LOCKED);
250+
expect(Object.keys(fields).sort()).toEqual([
251+
'dupdst_customer.dupsrc_customer',
252+
'dupdst_customer.full_name',
253+
'dupdst_ticket.ticket',
254+
'dupdst_ticket.title',
255+
'dupext_account.name',
256+
]);
257+
// The source set keeps every key it had, with its rule.
258+
expect(storedBody('permission', 'dupsrc_fls', P).fields).toEqual(flsBody.fields);
259+
});
260+
261+
it('a renamed key that meets a key the set already holds is refused, never merged', async () => {
262+
await protocol.installPackage({ manifest: { id: P, name: 'Source', type: 'application' } });
263+
await save(P, 'object', customerBody);
264+
// `dupdst_customer.full_name` names no object of the package, so it
265+
// keeps its name, and `dupsrc_customer.full_name` follows onto it.
266+
await save(P, 'permission', {
267+
name: 'dupsrc_clash', label: 'Clash',
268+
objects: { dupsrc_customer: { allowRead: true } },
269+
fields: { 'dupsrc_customer.full_name': HIDDEN, 'dupdst_customer.full_name': OPEN },
270+
} as { name: string });
271+
272+
const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q });
273+
274+
expect(res.failed).toHaveLength(1);
275+
const [refusal] = res.failed;
276+
expect(refusal).toMatchObject({ type: 'permission', name: 'dupsrc_clash' });
277+
// Located: the set, the key that moved, the key it met, and the remedy.
278+
expect(refusal.error).toContain("Permission set 'dupsrc_clash' is not copied");
279+
expect(refusal.error).toContain("'dupsrc_customer.full_name'");
280+
expect(refusal.error).toContain("'dupdst_customer.full_name'");
281+
expect(refusal.error).toContain('on the source, then duplicate again');
282+
// NEGATIVE CONTROL: nothing of the set was written to the target, and
283+
// the object still copied.
284+
expect(storedBody('permission', 'dupdst_clash', Q)).toBeUndefined();
285+
expect(res.copied).toEqual([{ type: 'object', name: 'dupdst_customer' }]);
286+
});
287+
});

0 commit comments

Comments
 (0)