Skip to content

The back-compat capability derivation refreshes label/description on ANY existing sys_capability row, including admin-authored ones — the comment above it says the opposite #5876

Description

@baozhoutao

Found while implementing #4967 Part 1 (PR #5875). Out of scope there, filed for triage.

What

bootstrapSystemCapabilities' seed loop updates label/description on whatever row it
finds for a derived name, without looking at managed_by:

packages/plugins/plugin-security/src/bootstrap-system-capabilities.ts (~100):

for (const def of byName.values()) {
  const existing = await tryFind(ql, 'sys_capability', { name: def.name }, 1);
  if (existing[0]?.id) {
    // Keep label/description fresh, but do NOT clobber admin edits — only
    // platform-owned display fields are reconciled. `scope` is an
    // admin-editable classification face …, so it is seed-once …
    if (await tryUpdate(ql, 'sys_capability', { id: existing[0].id, label: def.label, description: def.description })) {

The comment claims admin edits are not clobbered. What #2909 T3 actually made seed-once is
scope, and only scope. For a derived name (as opposed to a curated platform one)
def.label is humanize(name) and def.description is Capability name. — so an
existing row's authored label/description are overwritten with a humanized placeholder
on every boot, whoever authored them.

Measured

PR #5875 added a pin that exercises exactly this shape (it is there to justify a design
choice in that PR, not to cover this defect):

packages/plugins/plugin-security/src/bootstrap-declared-capabilities.test.ts,
REVERSE: dropping that name from the list lets the derivation overwrite the admin row
a row { name: 'showcase.export_data', label: 'Admin Made', managed_by: 'admin' } comes
out of the derivation pass labelled Showcase Export Data.

Reachability

Narrow but real, and it needs the admin row to pre-exist the grant:

  1. an admin creates capability X in Setup (managed_by:'admin' — the only provenance the
    ADR write-guard leaves admin-writable);
  2. an app whose bootstrap permission set grants X via systemPermissions[] is installed;
  3. every boot from then on rewrites that row's label/description to humanized text.

The reverse order is NOT reachable: once the derivation has created the managed_by:'platform'
placeholder, the package/platform write-guard blocks the admin from editing it at all, so
the more obvious "admin renames a derived capability" path fails earlier and differently.

Worth deciding as part of the same question: whether the derived pass should reconcile
display fields on a row it did not create at all, or only on rows it owns
(managed_by:'platform' created by itself) — the curated half of the loop has a real reason
to refresh (the platform's own definitions ship new copy), the derived half does not.

Not touched by #5875, which only stops refused declarations from suppressing the
derivation; the clobber above is on a different path and predates it.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions