Skip to content

Commit cc34db9

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-21849-retire-link-social
2 parents c62ac63 + 07c842d commit cc34db9

4 files changed

Lines changed: 371 additions & 11 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
A data-door edit of a permission set saved into a writable runtime package updates that set's stored definition instead of forking it
6+
7+
Clause-②: no
8+
9+
Setup saves a permission set through the data door (`PATCH /api/v1/data/sys_permission_set/:id`), which redirects the edit into the metadata store. A stored definition row is keyed by its package as well as its name, and the redirected save named no package. For a set saved into a writable runtime package (`PUT /api/v1/meta/permission/:name?package=<id>`), whose only stored row is bound to that package, the save therefore created a second, package-less row carrying the edit and left the package's row untouched: two active definitions for one name, with the package's copy no longer receiving the organization's edits. The save now goes into the package the edited row is bound to, read from that row through the metadata door's own item read, so the edit lands on the set's one row and the row stays in its package.
10+
11+
Unchanged: a set with no package binding still saves with none, and a set a code package ships is still refused with `403 NOT_OVERRIDABLE` before anything is read or written. The edit answers `200` as before; no error code, route or field moves.

‎packages/plugins/plugin-security/src/permission-set-projection.test.ts‎

Lines changed: 140 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -110,21 +110,30 @@ function makeQl() {
110110
* Mock metadata protocol over the ql's sys_metadata table: env-scope active
111111
* overlays, layered read (overlay-wins over `declared`), and the ADR-0094
112112
* awaited mutation-projector seam.
113+
*
114+
* [#21861] The package dimension of a row is modelled, because the defect it
115+
* hid lives there. A save targets the ONE row keyed by its `packageId` (absent
116+
* = the package-less row), as `SysMetadataRepository.put` does, so a save that
117+
* names no package beside a package-bound row mints a second row instead of
118+
* quietly overwriting the only one. The reads serve the first env-wide row of
119+
* the name in any package (`findServedOverlayRow` with no package), and the
120+
* single-item read states that row's `package_id` on the item as `_packageId`,
121+
* as `getMetaItem` does.
113122
*/
114123
function makeProtocol(ql: any, declared: Record<string, any> = {}) {
115124
let projector: ((evt: any) => Promise<void>) | null = null;
116-
const overlayFor = (name: string) =>
117-
ql.metaRows.find(
118-
(r: any) =>
119-
r.type === 'permission' && r.name === name && r.state === 'active' && (r.organization_id ?? null) === null,
120-
);
125+
const isActiveEnvRow = (r: any, name: string) =>
126+
r.type === 'permission' && r.name === name && r.state === 'active' && (r.organization_id ?? null) === null;
127+
const overlayFor = (name: string) => ql.metaRows.find((r: any) => isActiveEnvRow(r, name));
128+
const rowAt = (name: string, packageId: string | null) =>
129+
ql.metaRows.find((r: any) => isActiveEnvRow(r, name) && (r.package_id ?? null) === packageId);
121130
const protocol = {
122131
saves: [] as any[],
123132
deletes: [] as any[],
124133
registerMutationProjector(_type: string, fn: (evt: any) => Promise<void>) {
125134
projector = fn;
126135
},
127-
async saveMetaItem(req: { type: string; name: string; item: any; actor?: string }) {
136+
async saveMetaItem(req: { type: string; name: string; item: any; actor?: string; packageId?: string | null }) {
128137
// [#6858 / ADR-0094 D5-R] ADR-0005's tier gate, first — ahead of schema
129138
// validation, exactly as `protocol.ts` orders it. `declared` IS this
130139
// stub's artifact registry, so `declared[name] !== undefined` is the
@@ -165,12 +174,15 @@ function makeProtocol(ql: any, declared: Record<string, any> = {}) {
165174
err.status = 422;
166175
throw err;
167176
}
168-
const existing = overlayFor(req.name);
177+
const packageId = req.packageId ?? null;
178+
const existing = rowAt(req.name, packageId);
169179
if (existing) existing.metadata = JSON.stringify(req.item);
170180
else {
171181
ql.metaRows.push({
172-
id: `meta_${req.name}`, type: 'permission', name: req.name, state: 'active',
173-
organization_id: null, metadata: JSON.stringify(req.item),
182+
id: packageId === null ? `meta_${req.name}` : `meta_${req.name}@${packageId}`,
183+
type: 'permission', name: req.name, state: 'active',
184+
organization_id: null, ...(packageId === null ? {} : { package_id: packageId }),
185+
metadata: JSON.stringify(req.item),
174186
});
175187
}
176188
protocol.saves.push({ ...req });
@@ -195,6 +207,20 @@ function makeProtocol(ql: any, declared: Record<string, any> = {}) {
195207
overlayScope: overlay ? 'env' : null, effective: overlay ?? code,
196208
};
197209
},
210+
getMetaItemCalls: 0,
211+
async getMetaItem(req: { type: string; name: string }) {
212+
protocol.getMetaItemCalls += 1;
213+
const o = overlayFor(req.name);
214+
const item = o
215+
? { ...JSON.parse(o.metadata), ...(o.package_id ? { _packageId: o.package_id } : {}) }
216+
: declared[req.name];
217+
if (item === undefined) {
218+
const err: any = new Error(`permission/${req.name} not found`);
219+
err.status = 404;
220+
throw err;
221+
}
222+
return { type: 'permission', name: req.name, item };
223+
},
198224
};
199225
return protocol;
200226
}
@@ -1071,6 +1097,111 @@ describe('createPermissionSetWriteThrough (data door → metadata store)', () =>
10711097
});
10721098
});
10731099

1100+
// ── [#21861] The update leg writes back into the row it edits ──────────────
1101+
1102+
describe('[#21861] a data-door UPDATE writes back into the stored row it edits — no package-less fork', () => {
1103+
const userCtx = { userId: 'usr_admin' };
1104+
const PKG = 'com.acme.runtime';
1105+
1106+
/** Every active stored row of `name`, as its scope, its binding and the description its body carries. */
1107+
const activeRows = (ql: any, name: string) =>
1108+
ql.metaRows
1109+
.filter((r: any) => r.name === name && r.state === 'active')
1110+
.map((r: any) => ({
1111+
organization_id: r.organization_id ?? null,
1112+
package_id: r.package_id ?? null,
1113+
description: JSON.parse(r.metadata).description,
1114+
}));
1115+
1116+
/** A stored set (one row, optionally package-bound) with its projected record. */
1117+
async function storedSet(packageId?: string) {
1118+
const ql = makeQl();
1119+
const protocol = makeProtocol(ql);
1120+
registerPermissionSetProjection(protocol, { ql });
1121+
await protocol.saveMetaItem({
1122+
type: 'permission', name: 'support_agent', item: envBody({ name: 'support_agent' }),
1123+
...(packageId ? { packageId } : {}),
1124+
});
1125+
protocol.saves.length = 0;
1126+
return { ql, protocol, rowId: ql.permRows[0].id as string };
1127+
}
1128+
1129+
it('a set stored in a writable runtime package: the edit lands on that row, which stays bound', async () => {
1130+
const { ql, protocol, rowId } = await storedSet(PKG);
1131+
expect(activeRows(ql, 'support_agent'), 'before').toEqual([{ organization_id: null, package_id: PKG, description: undefined }]);
1132+
1133+
const opCtx: any = {
1134+
object: 'sys_permission_set', operation: 'update', context: userCtx,
1135+
data: { id: rowId, description: 'edited at the data door' },
1136+
};
1137+
expect(await run(makeMiddleware(ql, protocol), opCtx)).toBe(false);
1138+
1139+
expect(activeRows(ql, 'support_agent'), 'after: still one row, same binding, carrying the edit')
1140+
.toEqual([{ organization_id: null, package_id: PKG, description: 'edited at the data door' }]);
1141+
expect(protocol.saves.map((s: any) => s.packageId), 'the save names the row\'s own package').toEqual([PKG]);
1142+
expect(opCtx.result?.description, 'the projected record follows').toBe('edited at the data door');
1143+
});
1144+
1145+
it('a package-less set: the save still names no package, and its one row carries the edit', async () => {
1146+
const { ql, protocol, rowId } = await storedSet();
1147+
expect(activeRows(ql, 'support_agent'), 'before').toEqual([{ organization_id: null, package_id: null, description: undefined }]);
1148+
1149+
await run(makeMiddleware(ql, protocol), {
1150+
object: 'sys_permission_set', operation: 'update', context: userCtx,
1151+
data: { id: rowId, description: 'edited' },
1152+
});
1153+
1154+
expect(protocol.saves).toHaveLength(1);
1155+
expect(protocol.saves[0], 'no binding is invented for a set that has none').not.toHaveProperty('packageId');
1156+
expect(activeRows(ql, 'support_agent'), 'after')
1157+
.toEqual([{ organization_id: null, package_id: null, description: 'edited' }]);
1158+
});
1159+
1160+
it('a filtered edit spanning a package-bound and a package-less set writes each back into its own row', async () => {
1161+
const ql = makeQl();
1162+
const protocol = makeProtocol(ql);
1163+
registerPermissionSetProjection(protocol, { ql });
1164+
await protocol.saveMetaItem({ type: 'permission', name: 'pkg_set', item: envBody({ name: 'pkg_set' }), packageId: PKG });
1165+
await protocol.saveMetaItem({ type: 'permission', name: 'org_set', item: envBody({ name: 'org_set' }) });
1166+
protocol.saves.length = 0;
1167+
const before = { pkg: activeRows(ql, 'pkg_set'), org: activeRows(ql, 'org_set') };
1168+
expect(before).toEqual({
1169+
pkg: [{ organization_id: null, package_id: PKG, description: undefined }],
1170+
org: [{ organization_id: null, package_id: null, description: undefined }],
1171+
});
1172+
1173+
await run(makeMiddleware(ql, protocol), {
1174+
object: 'sys_permission_set', operation: 'update', context: userCtx,
1175+
data: { description: 'bulk' }, options: { where: { name: { $in: ['pkg_set', 'org_set'] } } },
1176+
});
1177+
1178+
expect({ pkg: activeRows(ql, 'pkg_set'), org: activeRows(ql, 'org_set') }, 'after').toEqual({
1179+
pkg: [{ organization_id: null, package_id: PKG, description: 'bulk' }],
1180+
org: [{ organization_id: null, package_id: null, description: 'bulk' }],
1181+
});
1182+
expect(protocol.saves.map((s: any) => [s.name, s.packageId ?? null]))
1183+
.toEqual([['pkg_set', PKG], ['org_set', null]]);
1184+
});
1185+
1186+
it('a set a code package ships is still refused by the lock (403 NOT_OVERRIDABLE) before any binding read or save', async () => {
1187+
const ql = makeQl();
1188+
const declaredBody = { ...envBody({ name: 'crm_rep' }), _packageId: 'com.example.crm' };
1189+
(ql as any).registry = { listItems: (t: string) => (t === 'permission' ? [declaredBody] : []) };
1190+
const protocol = makeProtocol(ql, { crm_rep: declaredBody });
1191+
ql.permRows.push({ id: 'ps_pkg', name: 'crm_rep', managed_by: 'package', package_id: 'com.example.crm' });
1192+
const before = activeRows(ql, 'crm_rep');
1193+
1194+
await expect(run(makeMiddleware(ql, protocol), {
1195+
object: 'sys_permission_set', operation: 'update', context: userCtx,
1196+
data: { id: 'ps_pkg', description: 'customized' },
1197+
})).rejects.toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 });
1198+
1199+
expect(activeRows(ql, 'crm_rep'), 'the refused edit stored nothing').toEqual(before);
1200+
expect(protocol.saves).toHaveLength(0);
1201+
expect(protocol.getMetaItemCalls, 'the binding is never read for a refused edit').toBe(0);
1202+
});
1203+
});
1204+
10741205
// ── Boot reconciliation + one-time backfill (ADR-0094 D4) ───────────────────
10751206

10761207
describe('reconcilePermissionSetProjection', () => {

‎packages/plugins/plugin-security/src/permission-set-projection.ts‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,47 @@ export function createPermissionSetWriteThrough(
10911091
}
10921092
};
10931093

1094+
/**
1095+
* [#21861] The `saveMetaItem` argument that writes an update back into the
1096+
* stored row it edits: `{ packageId }` when that row is bound to a package,
1097+
* nothing when it is not.
1098+
*
1099+
* A `sys_metadata` row is keyed `(org, type, name, package_id)`, and a save
1100+
* that names no package targets the package-less row. So an update leg that
1101+
* named none, for a set whose only row is bound to a writable runtime
1102+
* package (`PUT /meta/permission/:name?package=`), minted a second,
1103+
* package-less row carrying the edit beside the untouched package-bound one:
1104+
* two active rows for one name.
1105+
*
1106+
* The row is the one the update merges its patch into — the `overlay` layer
1107+
* of `envelope`, which {@link effectiveBodyForRow} takes as the base — and its
1108+
* binding is read from the metadata door's own single-item read, which serves
1109+
* that row by the same served-row resolution the layered read uses and states
1110+
* the row's `package_id` on it as `_packageId`. ⛔ Never from the patch, the
1111+
* projected record (the projector writes no binding onto an admin row), or a
1112+
* registry item: those are copies, and the row is the fact.
1113+
*
1114+
* No `overlay` layer means no stored row, so there is nothing to fork and the
1115+
* save stays package-less, exactly as before. Every target reaching this read
1116+
* has already passed the lock (verdict `org`), so a code-shipped set never
1117+
* gets here. A read that fails is not caught: guessing the binding would
1118+
* choose which row the save lands in.
1119+
*/
1120+
const storedRowPackageArg = async (
1121+
protocol: any,
1122+
name: string,
1123+
envelope: unknown,
1124+
): Promise<{ packageId?: string }> => {
1125+
const overlay = (envelope as { overlay?: unknown } | null | undefined)?.overlay;
1126+
if (overlay === null || overlay === undefined) return {};
1127+
// A protocol with no single-item read (minimal embeddings, unit-test stubs)
1128+
// keys no row by package either.
1129+
if (typeof protocol.getMetaItem !== 'function') return {};
1130+
const served = await protocol.getMetaItem({ type: 'permission', name });
1131+
const packageId = served?.item?._packageId;
1132+
return typeof packageId === 'string' && packageId !== '' ? { packageId } : {};
1133+
};
1134+
10941135
const projectAndFetch = async (protocol: any, name: string): Promise<any> => {
10951136
// The awaited projector inside saveMetaItem/deleteMetaItem normally did
10961137
// this already — re-running is an idempotent upsert, and covers the
@@ -1368,10 +1409,13 @@ export function createPermissionSetWriteThrough(
13681409
const rowState = pickRowStateColumns(patch);
13691410
const results: any[] = [];
13701411
for (const row of targets) {
1371-
const base = await effectiveBodyForRow(protocol, ql, row, layeredByName.get(String(row.name)));
1412+
const envelope = layeredByName.get(String(row.name));
1413+
const base = await effectiveBodyForRow(protocol, ql, row, envelope);
13721414
const body = mergeRowPatchIntoBody(base, patch);
13731415
body.name = row.name;
1374-
await protocol.saveMetaItem({ type: 'permission', name: row.name, item: body, ...actorArg });
1416+
// [#21861] Into the row the base came from — see `storedRowPackageArg`.
1417+
const packageArg = await storedRowPackageArg(protocol, String(row.name), envelope);
1418+
await protocol.saveMetaItem({ type: 'permission', name: row.name, item: body, ...packageArg, ...actorArg });
13751419
// Row state rides along on the same patch but lands on the record, not
13761420
// in the definition (the projector above never touches these columns).
13771421
if (rowState) await tryUpdate(ql, 'sys_permission_set', { id: row.id, ...rowState });

0 commit comments

Comments
 (0)