Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .changeset/refused-capability-declaration-hole.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-security": patch
---

fix(plugin-security): 被拒收的 capability 声明不再连派生占位一起压掉 (#4967 Part 1/3)

`SecurityPlugin` 分两遍种 `sys_capability`:第一遍落包声明的 capability
(`managed_by:'package'` + `package_id`),第二遍种平台 curated 集合 + 从
permission set 的 `systemPermissions[]` **派生**的 back-compat 占位,并**跳过**
第一遍报上来的名字,以免占位把已写好的声明覆盖掉。

问题在于第一遍报的是「读到的每个名字」,而不是「真正落了行的名字」:
`bootstrapDeclaredCapabilities` 在 upsert 作出任何决定**之前**就把
`cap.name` 推进了返回列表。而 upsert 有三条**拒收**路径,一行都不写。其中
「声明没有归属包」这一条既没写行、又占住了名字,于是派生占位也被跳过——
capability **在任何一行里都不存在**。净效果是:**写下这条声明,比不写还糟**
(不写至少还有派生占位)。这正是 showcase 的
`showcase.export_data` 只留下一条 `warn` 的成因。

修法是把「上报」与「读到」拆开:一个名字进入上报列表(现更名为
`materializedNames`)的条件,是本遍**确认它有行**——本遍写成了
(seeded / updated / claimed),或找到一行不能被覆盖的既有行(admin 自建、他包
所有、curated 平台名)。三条拒收路径按「派生是否会覆盖既有 authored 行」分别
处置,理由写在代码里:

- **curated 平台名**:仍然上报。curated 那一遍无条件种这些名字,行必然存在;
且派生路径本来就够不到 curated 名(它已在 curated 表里)。
- **他包所有 / admin 自建**:仍然上报。行存在且 label/description 是**作者写
的**,派生会把它们刷成 humanize 出来的占位——压掉派生正是这份列表的用途。
- **没有归属包**:仅当已存在一行时才上报。没有行时回落到派生占位,和「从未
写过这条声明」时一样。

同时补上这条路径此前缺失的计数器 `skippedUnowned`,于是每条具名声明恰好落在
一个计数器里,列表与计数器可以对账。

**行为变化(升级须知)**:一条被拒收(无归属包)且被某个 permission set 授权
的 capability,此前在 `sys_capability` 里**没有任何行**,现在会出现一行
`managed_by:'platform'` 的派生占位——即它在 Setup 的能力列表里可见、可解析、
带 humanize 出来的 label。注意这不改变**运行时判定**:权限求值一直是按
`systemPermissions[]` 里的字符串取并集的,从不查 `sys_capability`;恢复的是
注册表一侧的 declared = enforced(能力有定义记录、可见、可管理、有 provenance),
不是把一个原本不生效的授权变成生效。若某个部署依赖「那条能力在能力列表里查不
到」,升级后它会出现。

诊断消息同时按 #4632 改进(级别仍为 `warn` —— 功能性降级,非持久性失败):
拒收时点名**授权它的 permission set**,并写明真实后果,例如
`[security] declared capability "showcase.export_data" has no owning package (granted by showcase_ops): falls back to the back-compat derived placeholder …`。
无人授权、或已有行的情形各有对应措辞。
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('bootstrapDeclaredCapabilities (ADR-0066 D1 package declaration)', () =
]);
const out = await bootstrapDeclaredCapabilities(ql, null);
expect(out.seeded).toBe(1);
expect(out.declaredNames).toEqual(['export_data']);
expect(out.materializedNames).toEqual(['export_data']);
const row = ql.rows.find((r) => r.name === 'export_data');
expect(row).toMatchObject({
name: 'export_data',
Expand Down Expand Up @@ -88,6 +88,7 @@ describe('bootstrapDeclaredCapabilities (ADR-0066 D1 package declaration)', () =
const ql = makeQl([{ name: 'orphan_cap', label: 'Orphan' }]);
const out = await bootstrapDeclaredCapabilities(ql, null);
expect(out.seeded).toBe(0);
expect(out.skippedUnowned).toBe(1);
expect(ql.rows.find((r) => r.name === 'orphan_cap')).toBeUndefined();
});

Expand Down Expand Up @@ -123,11 +124,11 @@ describe('bootstrapDeclaredCapabilities (ADR-0066 D1 package declaration)', () =
});

it('declared name suppresses the implicit derived placeholder (no clobber)', async () => {
// Full boot order: declared first, then system with declaredNames.
// Full boot order: declared first, then system with materializedNames.
const ql = makeQl([{ name: 'export_data', label: 'Export Data', scope: 'org', _packageId: 'com.acme.reports' }]);
const cap = await bootstrapDeclaredCapabilities(ql, null);
await bootstrapSystemCapabilities(ql, [{ systemPermissions: ['export_data'] }], {
declaredCapabilityNames: cap.declaredNames,
materializedCapabilityNames: cap.materializedNames,
});
const row = ql.rows.find((r) => r.name === 'export_data');
// The package row is untouched — no humanized placeholder overwrote it.
Expand All @@ -138,6 +139,222 @@ describe('bootstrapDeclaredCapabilities (ADR-0066 D1 package declaration)', () =
it('returns an empty outcome when nothing is declared', async () => {
const ql = makeQl([]);
const out = await bootstrapDeclaredCapabilities(ql, null);
expect(out).toMatchObject({ seeded: 0, updated: 0, claimed: 0, declaredNames: [] });
expect(out).toMatchObject({ seeded: 0, updated: 0, claimed: 0, skippedUnowned: 0, materializedNames: [] });
});
});

// ───────────────────────────────────────────────────────────────────────────
// [#4967 Part 1] A REFUSED declaration must not suppress the back-compat
// derivation. `materializedNames` reports the names this pass CONFIRMED have a
// row — the three refusal paths land on different sides of that line, for
// different reasons, so each gets its own pin (and, where the direction is not
// obvious, the reverse case that shows what the other answer would cost).
// ───────────────────────────────────────────────────────────────────────────
describe('refused declarations vs. the derived placeholder (#4967 Part 1)', () => {
const OPS_SETS = [{ name: 'showcase_ops', systemPermissions: ['setup.access', 'showcase.export_data'] }];

it('NO OWNING PACKAGE + no row: falls through, so the derivation materializes it', async () => {
// The showcase repro: `showcase.export_data` declared without a resolvable
// owner, granted by `showcase_ops`.
const ql = makeQl([{ name: 'showcase.export_data', label: 'Export Data' }]);
const out = await bootstrapDeclaredCapabilities(ql, null, { permissionSets: OPS_SETS });

// The declaration is refused — no package row, and (the fix) the name is
// NOT reported as materialized.
expect(out.seeded).toBe(0);
expect(out.skippedUnowned).toBe(1);
expect(out.materializedNames).toEqual([]);

// Second pass — the capability now exists, as the back-compat placeholder
// it would have had if the declaration had never been written.
await bootstrapSystemCapabilities(ql, OPS_SETS, { materializedCapabilityNames: out.materializedNames });
expect(ql.rows.find((r) => r.name === 'showcase.export_data')).toMatchObject({
name: 'showcase.export_data', managed_by: 'platform', active: true,
});
// …and it RESOLVES by name, which is what the granting permission set needs
// from the registry (Setup listing, provenance, ADR-0066 ⑨ lint sources).
expect(await ql.find('sys_capability', { where: { name: 'showcase.export_data' } })).toHaveLength(1);
});

it('REVERSE: the pre-#4967 list (every declared name) leaves the capability in no row at all', async () => {
// Same fixture, same second pass — only the skip list is the old one, which
// reported a name the first pass refused to write. Nothing derives it and
// nothing declared it: the hole.
const ql = makeQl([{ name: 'showcase.export_data', label: 'Export Data' }]);
await bootstrapDeclaredCapabilities(ql, null, { permissionSets: OPS_SETS });
await bootstrapSystemCapabilities(ql, OPS_SETS, {
materializedCapabilityNames: ['showcase.export_data'], // ← the old `declaredNames`
});
expect(ql.rows.find((r) => r.name === 'showcase.export_data')).toBeUndefined();
});

it('is stable across boots: the refusal re-derives nothing and duplicates nothing', async () => {
const ql = makeQl([{ name: 'showcase.export_data', label: 'Export Data' }]);
for (let boot = 0; boot < 2; boot += 1) {
const out = await bootstrapDeclaredCapabilities(ql, null, { permissionSets: OPS_SETS });
await bootstrapSystemCapabilities(ql, OPS_SETS, { materializedCapabilityNames: out.materializedNames });
}
expect(ql.rows.filter((r) => r.name === 'showcase.export_data')).toHaveLength(1);
// Boot 2 finds the placeholder, so the refusal now reports the name — the
// row exists and must not be re-derived over.
const out2 = await bootstrapDeclaredCapabilities(ql, null, { permissionSets: OPS_SETS });
expect(out2.skippedUnowned).toBe(1);
expect(out2.materializedNames).toEqual(['showcase.export_data']);
});

it('NO OWNING PACKAGE + an admin row: still suppresses, so the placeholder cannot clobber it', async () => {
const ql = makeQl([{ name: 'showcase.export_data', label: 'Declared Label' }]);
ql.rows.push({ id: 'cap_admin', name: 'showcase.export_data', label: 'Admin Made', description: 'Admin wrote this.', managed_by: 'admin' });
const out = await bootstrapDeclaredCapabilities(ql, null, { permissionSets: OPS_SETS });
expect(out.skippedUnowned).toBe(1);
expect(out.materializedNames).toEqual(['showcase.export_data']);
await bootstrapSystemCapabilities(ql, OPS_SETS, { materializedCapabilityNames: out.materializedNames });
expect(ql.rows.find((r) => r.name === 'showcase.export_data')).toMatchObject({
label: 'Admin Made', description: 'Admin wrote this.', managed_by: 'admin',
});
});

it('REVERSE: dropping that name from the list lets the derivation overwrite the admin row', async () => {
// Why the unowned path checks for an EXISTING row instead of always
// falling through: the derived defaults refresh label/description on any
// row they find.
const ql = makeQl([]);
ql.rows.push({ id: 'cap_admin', name: 'showcase.export_data', label: 'Admin Made', description: 'Admin wrote this.', managed_by: 'admin' });
await bootstrapSystemCapabilities(ql, OPS_SETS, { materializedCapabilityNames: [] });
expect(ql.rows.find((r) => r.name === 'showcase.export_data')?.label).toBe('Showcase Export Data');
});

it('FOREIGN owner: suppresses, because the other package authored that row', async () => {
const sets = [{ name: 'ops', systemPermissions: ['shared_cap'] }];
const ql = makeQl([{ name: 'shared_cap', label: 'Mine', _packageId: 'com.b' }]);
ql.rows.push({ id: 'cap_x', name: 'shared_cap', label: 'Owner Label', description: 'Owner wrote this.', managed_by: 'package', package_id: 'com.a' });
const out = await bootstrapDeclaredCapabilities(ql, null, { permissionSets: sets });
expect(out.skippedForeign).toBe(1);
expect(out.materializedNames).toEqual(['shared_cap']);
await bootstrapSystemCapabilities(ql, sets, { materializedCapabilityNames: out.materializedNames });
expect(ql.rows.find((r) => r.name === 'shared_cap')).toMatchObject({
label: 'Owner Label', description: 'Owner wrote this.', package_id: 'com.a',
});
});

it('CURATED platform name: suppresses, and the curated pass seeds the row regardless', async () => {
// A no-op for the skip list (the derived path never reaches a curated name
// — it is already in the curated map), but a truthful answer: the row
// exists after the second pass either way.
const sets = [{ name: 'ops', systemPermissions: ['manage_users'] }];
const ql = makeQl([{ name: 'manage_users', label: 'Evil', _packageId: 'com.acme.evil' }]);
const out = await bootstrapDeclaredCapabilities(ql, null, { permissionSets: sets });
expect(out.skippedPlatform).toBe(1);
expect(out.materializedNames).toEqual(['manage_users']);
await bootstrapSystemCapabilities(ql, sets, { materializedCapabilityNames: out.materializedNames });
expect(ql.rows.find((r) => r.name === 'manage_users')).toMatchObject({
label: 'Manage Users', managed_by: 'platform', // curated definition, not the package's
});
});

it('materializedNames reconciles with the outcome counters', async () => {
const ql = makeQl([
{ name: 'a.new', _packageId: 'com.a' }, // → seeded
{ name: 'a.own', label: 'Fresh', _packageId: 'com.a' }, // → updated
{ name: 'a.derived', _packageId: 'com.a' }, // → claimed
{ name: 'a.admin', _packageId: 'com.a' }, // → skippedAdmin
{ name: 'a.foreign', _packageId: 'com.a' }, // → skippedForeign
{ name: 'manage_users', _packageId: 'com.a' }, // → skippedPlatform
{ name: 'a.orphan' }, // → skippedUnowned, NO row
]);
ql.rows.push({ id: 'c1', name: 'a.own', managed_by: 'package', package_id: 'com.a' });
ql.rows.push({ id: 'c2', name: 'a.derived', managed_by: 'platform' });
ql.rows.push({ id: 'c3', name: 'a.admin', managed_by: 'admin' });
ql.rows.push({ id: 'c4', name: 'a.foreign', managed_by: 'package', package_id: 'com.z' });

const out = await bootstrapDeclaredCapabilities(ql, null);

expect(out).toMatchObject({
seeded: 1, updated: 1, claimed: 1,
skippedAdmin: 1, skippedForeign: 1, skippedPlatform: 1, skippedUnowned: 1,
});
// Every named declaration lands in exactly one counter…
const counted = out.seeded + out.updated + out.claimed
+ out.skippedAdmin + out.skippedForeign + out.skippedPlatform + out.skippedUnowned;
expect(counted).toBe(7);
// …and `materializedNames` is that set minus the refusal that found no row.
expect(out.materializedNames).toEqual(['a.new', 'a.own', 'a.derived', 'a.admin', 'a.foreign', 'manage_users']);
expect(out.materializedNames).toHaveLength(counted - out.skippedUnowned);
expect(out.materializedNames).not.toContain('a.orphan');
});
});

// ───────────────────────────────────────────────────────────────────────────
// [#4967 Part 3] The refusal diagnostic names the GRANTOR permission set(s)
// and the actual consequence. Level stays `warn` per #4632 (functional
// degradation, not a durability failure).
// ───────────────────────────────────────────────────────────────────────────
describe('unowned-declaration diagnostic (#4967 Part 3)', () => {
function spyLogger() {
const warns: Array<{ msg: string; meta?: Record<string, any> }> = [];
const errors: string[] = [];
return {
warns,
errors,
logger: {
warn: (msg: string, meta?: Record<string, any>) => { warns.push({ msg, meta }); },
error: (msg: string) => { errors.push(msg); },
},
};
}

it('names every permission set that grants the capability, and stays a warn', async () => {
const { warns, errors, logger } = spyLogger();
const ql = makeQl([{ name: 'showcase.export_data', label: 'Export Data' }]);
await bootstrapDeclaredCapabilities(ql, null, {
logger,
permissionSets: [
{ name: 'showcase_ops', systemPermissions: ['setup.access', 'showcase.export_data'] },
{ name: 'showcase_admin', systemPermissions: ['showcase.export_data'] },
{ name: 'unrelated', systemPermissions: ['setup.access'] },
],
});
const w = warns.find((x) => x.msg.includes('showcase.export_data'));
expect(w).toBeDefined();
expect(w!.msg).toContain('has no owning package');
expect(w!.msg).toContain('showcase_ops');
expect(w!.msg).toContain('showcase_admin');
expect(w!.msg).not.toContain('unrelated');
// The consequence, not just the name: it still exists, without provenance.
expect(w!.msg).toContain('derived placeholder');
expect(w!.meta?.grantedBy).toEqual(['showcase_ops', 'showcase_admin']);
// [#4632] functional degradation → warn, never error.
expect(errors).toEqual([]);
});

it('says so plainly when NOTHING grants the capability (it exists nowhere)', async () => {
const { warns, logger } = spyLogger();
const ql = makeQl([{ name: 'never_granted' }]);
await bootstrapDeclaredCapabilities(ql, null, { logger, permissionSets: [{ name: 'ops', systemPermissions: ['setup.access'] }] });
const w = warns.find((x) => x.msg.includes('never_granted'));
expect(w!.msg).toContain('granted by no bootstrap permission set');
expect(w!.msg).toContain('materialized nowhere');
expect(w!.meta?.grantedBy).toEqual([]);
});

it('reports the existing row when one already resolves the name', async () => {
const { warns, logger } = spyLogger();
const ql = makeQl([{ name: 'showcase.export_data' }]);
ql.rows.push({ id: 'cap_p', name: 'showcase.export_data', managed_by: 'platform' });
await bootstrapDeclaredCapabilities(ql, null, {
logger,
permissionSets: [{ name: 'showcase_ops', systemPermissions: ['showcase.export_data'] }],
});
const w = warns.find((x) => x.msg.includes('showcase.export_data'));
expect(w!.msg).toContain('left as-is');
expect(w!.meta?.grantedBy).toEqual(['showcase_ops']);
});

it('falls back to a placeholder label for an unnamed permission set', async () => {
const { warns, logger } = spyLogger();
const ql = makeQl([{ name: 'orphan_cap' }]);
await bootstrapDeclaredCapabilities(ql, null, { logger, permissionSets: [{ systemPermissions: ['orphan_cap'] }] });
const w = warns.find((x) => x.msg.includes('orphan_cap'));
expect(w!.meta?.grantedBy).toEqual(['(unnamed permission set)']);
});
});
Loading
Loading