Found while implementing #12923 (PR #12967), which repairs the same class on the five RBAC catalog seeders. These two sites are in the same package and the same shape, but on different paths, so they were deliberately left out of that PR's scope.
Both call the shared tryUpdate in packages/plugins/plugin-security/src/permission-set-projection.ts, which answers false on refusal. After #12967 that helper takes an optional refusal log, so the channel to fix these already exists — neither of these callers passes one.
Measured on origin/main at the time of writing.
1. permission-set-drift.ts — a refused diagnostic write silences its own report
persistPermissionSetDriftDiagnostics counts only successful writes:
if (await tryUpdate(ql, 'sys_permission_set', { id: d.id, drift_status: status, drift_detail: detail }, opts.organizationId)) {
updated += 1;
}
and runPermissionSetDriftDiagnostics then reports only when updated > 0:
if (updated > 0) {
opts.logger?.warn?.('[security] package-declared permission set(s) enforcing grants that differ from the shipped artifact', { ... });
}
So a boot on which every drift write is refused computes the drift correctly, persists none of it, and prints nothing at all — indistinguishable from a deployment with no drift. This is the #12923 shape one path over: zero reads as "nothing to do". The drifted set keeps enforcing grants that differ from the shipped artifact, and the one line that would have said so is gated behind the counter the refusal suppressed.
2. permission-set-overlay-discard.ts — the audit line can describe a discard that did not happen
On the degraded-kernel branch (no metadata protocol) the write's result is discarded entirely — not assigned, not tested:
await tryUpdate(
ql,
'sys_permission_set',
{ id, ...permissionSetRowFields(declaredItem), customized: false },
organizationId,
);
healedRow = (await tryFind(ql, 'sys_permission_set', { id }, 1, organizationId))[0] ?? row;
The function then re-reads the row and logs, at info:
'[security] package-declared permission set overlay discarded (sanctioned operator action)'
with by, overlaysDiscarded, objectGrantsBefore and objectGrantsAfter. If the update was refused, healedRow is the unchanged row, objectGrantsAfter equals objectGrantsBefore, and the entry asserts a sanctioned operator action that never landed — while the caller receives healedObjectGrantCount describing the un-healed row. This one is worse than a missing warning: it is an audit record that is wrong, and it is the record of a deliberately audited operator action ("supported, audited action" per its own comment).
Suggested repair
Same treatment as #12967, reusing what that PR already added:
- pass a
SeedWriteRefusals log into both tryUpdate calls and report once per pass via warnSeedWriteRefusals; - for (2) additionally read the result and either refuse to emit the success audit line or emit it with the failure stated — an audit entry must not claim a write that was refused. Whether that path should propagate to the caller instead of logging is a design question for whoever picks this up; the overlay discard is an operator-invoked action with a caller who could be told.
Notes on scope
Filed unassigned, for triage.
Generated by Claude Code
Found while implementing #12923 (PR #12967), which repairs the same class on the five RBAC catalog seeders. These two sites are in the same package and the same shape, but on different paths, so they were deliberately left out of that PR's scope.
Both call the shared
tryUpdateinpackages/plugins/plugin-security/src/permission-set-projection.ts, which answersfalseon refusal. After #12967 that helper takes an optional refusal log, so the channel to fix these already exists — neither of these callers passes one.Measured on
origin/mainat the time of writing.1.
permission-set-drift.ts— a refused diagnostic write silences its own reportpersistPermissionSetDriftDiagnosticscounts only successful writes:and
runPermissionSetDriftDiagnosticsthen reports only whenupdated > 0:So a boot on which every drift write is refused computes the drift correctly, persists none of it, and prints nothing at all — indistinguishable from a deployment with no drift. This is the #12923 shape one path over: zero reads as "nothing to do". The drifted set keeps enforcing grants that differ from the shipped artifact, and the one line that would have said so is gated behind the counter the refusal suppressed.
2.
permission-set-overlay-discard.ts— the audit line can describe a discard that did not happenOn the degraded-kernel branch (no metadata protocol) the write's result is discarded entirely — not assigned, not tested:
The function then re-reads the row and logs, at
info:with
by,overlaysDiscarded,objectGrantsBeforeandobjectGrantsAfter. If the update was refused,healedRowis the unchanged row,objectGrantsAfterequalsobjectGrantsBefore, and the entry asserts a sanctioned operator action that never landed — while the caller receiveshealedObjectGrantCountdescribing the un-healed row. This one is worse than a missing warning: it is an audit record that is wrong, and it is the record of a deliberately audited operator action ("supported, audited action" per its own comment).Suggested repair
Same treatment as #12967, reusing what that PR already added:
SeedWriteRefusalslog into bothtryUpdatecalls and report once per pass viawarnSeedWriteRefusals;Notes on scope
bootstrap-system-capabilities.tshas the same localtryInsert/tryUpdatecopies but is not in this finding: its refusal case is already counted and warned (blockedCurated, frombootstrapSystemCapabilitiesreconciles an arbitrary row when a curated capability name exists in more than one organization —find(..., limit: 1)has no ORDER BY, so the platform's own row can be left unseeded #8470), deliberately and by ruling.auto-org-admin-grant.tsalready logs on refusal.permission-set-projection.tspackage/environment-door callers are already honest: their publish answerssuccess: falsewith a reason when it materialized nothing.Filed unassigned, for triage.
Generated by Claude Code