From 5ce69af4ac4780307bb2caf37ae1c2502bc902c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 01:04:46 +0000 Subject: [PATCH 1/4] fix(plugin-security): make the seeders' five remaining refusals reach the author The two declared-metadata seeders refuse to write in five more places, and every one of those refusals was spelled `logger?.warn?.(...)` -- optionally chained twice, so a caller that injected no logger got no output at all. Each site now words its own consequence, stamps its own grep token and builds its own record, over ONE shared delivery rule (`reportThroughSink`). The skips themselves are unchanged (ADR-0086 D3/D4). Claude-Session: https://claude.ai/code/session_01WmBwEiWPff9JZPd5BSGNeH Co-authored-by: Claude --- .../src/bootstrap-declared-capabilities.ts | 70 ++-- .../src/bootstrap-declared-permissions.ts | 32 +- packages/plugins/plugin-security/src/index.ts | 29 ++ .../src/seed-refusal-diagnostics.ts | 393 ++++++++++++++++++ .../plugin-security/src/seed-refusal-sink.ts | 83 ++++ 5 files changed, 574 insertions(+), 33 deletions(-) create mode 100644 packages/plugins/plugin-security/src/seed-refusal-diagnostics.ts create mode 100644 packages/plugins/plugin-security/src/seed-refusal-sink.ts diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts b/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts index 3cb4ec4a3ce..512ea4ef420 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts @@ -99,6 +99,17 @@ import { reportCapabilityNameCollisions, type CapabilityNameCollisionDiagnostic, } from './capability-name-collision.js'; +// [#18091] The three remaining refusals of this seeder, each with its OWN +// wording, token and record. They share exactly one thing — `reportThroughSink`, +// the delivery rule — because the alternative measured here was five more +// hand-written copies of the two lines #17516 and #18023 each wrote out. +import { + capabilityDeclarationUnownedDiagnostic, + capabilityPlatformNameRefusedDiagnostic, + reportCapabilityDeclarationUnowned, + reportCapabilityPlatformNameRefused, + reportCapabilityRowsUnreadable, +} from './seed-refusal-diagnostics.js'; import { PLATFORM_CAPABILITY_NAMES } from '@objectstack/spec/security'; /** The only shape this seeder reads off a permission set: who grants what. */ @@ -244,25 +255,10 @@ function indexGrantors(sets: readonly GrantingPermissionSet[] = []): Map 0 - ? `granted by ${grantors.join(', ')}` - : 'granted by no bootstrap permission set'; - const consequence = hasRow - ? 'an existing sys_capability row already resolves it and is left as-is — the declaration adds no package provenance' - : grantors.length > 0 - ? 'falls back to the back-compat derived placeholder — the grant resolves, but with no package provenance (ADR-0086 D3: uninstall undefined)' - : 'nothing derives it either — the capability is materialized nowhere'; - return `[security] declared capability "${name}" has no owning package (${granted}): ${consequence}`; -} +// [#4967 Part 3 / #18091] The unowned-declaration wording — the grantor names +// and the three-way consequence — moved WHOLE into +// `capabilityDeclarationUnownedDiagnostic`, so the record and the sentence are +// one derivation instead of a message here and a record somewhere else. /** * Upsert ONE declared capability into `sys_capability` under the owning @@ -313,7 +309,18 @@ async function upsertPackageCapability( // claim one (that would let it silently redefine `manage_users`, `setup.access`, …). if (PLATFORM_CAPABILITY_NAMES.has(cap.name)) { out.skippedPlatform += 1; - logger?.warn?.('[security] capability name is a curated platform capability — not materialized as package', { name: cap.name }); + // [#18091] ⛔ The refusal is unchanged; what changed is that it is no longer + // invisible. The old line was optionally chained TWICE, so a caller that + // injected no logger got no output at all — measured on the pre-fix tree at + // this exact site: `skippedPlatform = 1`, author-visible console lines = 0 + // across all five channels. Its own wording, because this consequence is + // not the foreign-owner one: the curated row answers for the name, so + // nothing is denied and the remedy is rename-only. + reportCapabilityPlatformNameRefused(logger, capabilityPlatformNameRefusedDiagnostic({ + name: String(cap.name), + declaredBy: packageId ?? null, + grantedBy: grantors, + })); return true; } @@ -340,10 +347,15 @@ async function upsertPackageCapability( // never a swallowed failure — `unknown` returned above. if (!packageId) { out.skippedUnowned += 1; - logger?.warn?.(unownedRefusalMessage(cap.name, grantors, Boolean(existing?.id)), { - name: cap.name, - grantedBy: [...grantors], - }); + // [#18091] Same refusal, now delivered: measured mute at this site on the + // pre-fix tree (`skippedUnowned = 1`, author-visible lines = 0 with no + // logger). The three-way consequence is preserved verbatim — `hasRow` is + // why the existence read happens above this branch. + reportCapabilityDeclarationUnowned(logger, capabilityDeclarationUnownedDiagnostic({ + name: String(cap.name), + grantedBy: grantors, + hasRow: Boolean(existing?.id), + })); return Boolean(existing?.id); } @@ -521,10 +533,12 @@ export async function bootstrapDeclaredCapabilities( // not state one — these names were left ENTIRELY alone, so a genuinely new // declaration among them has not been created and a drifted one has not // been healed; the next boot with a readable database does both. - options.logger?.warn?.( - '[security] declared capabilities left untouched — their sys_capability rows could not be read', - { unreadable: out.unreadable, total: caps.length }, - ); + // [#18091] …and said even when no logger was injected. An unreadable + // database that reports nothing reads exactly like a healthy boot. + reportCapabilityRowsUnreadable(options.logger, { + unreadable: out.unreadable, + total: caps.length, + }); } options.logger?.info?.('[security] declared capabilities seeded into sys_capability (ADR-0066 D1)', { ...out, total: caps.length, diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index c11b5d82fb1..7af67e2abc6 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -74,6 +74,13 @@ import { reportPermissionSetNameCollisions, type PermissionSetNameCollisionDiagnostic, } from './permission-set-name-collision.js'; +// [#18091] This seeder's two remaining refusals, each with its own wording, +// token and record, over the one shared delivery rule. +import { + permissionSetDeclarationUnownedDiagnostic, + reportPermissionSetDeclarationUnowned, + reportPermissionSetRowsUnreadable, +} from './seed-refusal-diagnostics.js'; export type { PermissionSeedOutcome } from './permission-set-projection.js'; @@ -228,7 +235,19 @@ export async function upsertPackagePermissionSet( // undefined again — the exact ambiguity ADR-0086 D3 exists to remove — so a // set with no resolvable owner is skipped rather than materialized unowned. if (!packageId) { - logger?.warn?.('[security] permission set has no owning package — not materialized', { name: ps.name }); + // [#18091] ⛔ The refusal is unchanged — an unowned row would re-create the + // ADR-0086 D3 ambiguity. What changed is that it arrives. Measured on the + // pre-fix tree with no logger, through BOTH doors that reach this branch + // (the boot loop and the ADR-0086 P2 publish materializer, which passes no + // collector): author-visible console lines = 0, and this branch moves no + // counter either, so the outcome said nothing about it. + // + // Its own wording, because the consequence is not the capability axis': + // the declared set stays runtime-enforced and only the RECORD is missing. + reportPermissionSetDeclarationUnowned(logger, permissionSetDeclarationUnownedDiagnostic({ + name: String(ps.name), + ...(opts?.organizationId ? { organizationId: opts.organizationId } : {}), + })); return out; } @@ -422,10 +441,13 @@ export async function bootstrapDeclaredPermissions( // Said once, with the count: these sets were neither seeded nor reconciled // because the record could not be READ. Silence here would read exactly // like "everything was already in order". - options.logger?.warn?.( - '[security] declared permission sets left untouched — their records could not be read', - { unreadable: out.unreadable, total: sets.length, ...(organizationId ? { organization: organizationId } : {}) }, - ); + // [#18091] …and said even when no logger was injected: silence here reads + // exactly like "everything was already in order", which is the whole defect. + reportPermissionSetRowsUnreadable(options.logger, { + unreadable: out.unreadable, + total: sets.length, + ...(organizationId ? { organizationId } : {}), + }); } options.logger?.info?.('[security] declared permission sets seeded into sys_permission_set (ADR-0086 D5)', { diff --git a/packages/plugins/plugin-security/src/index.ts b/packages/plugins/plugin-security/src/index.ts index 6c6b3c169fd..c3f3df8c4c0 100644 --- a/packages/plugins/plugin-security/src/index.ts +++ b/packages/plugins/plugin-security/src/index.ts @@ -85,6 +85,35 @@ export { reportCapabilityNameCollisions, } from './capability-name-collision.js'; export type { CapabilityNameCollisionDiagnostic } from './capability-name-collision.js'; +// [#18091] The seeders' remaining refusals. EXPORTED on the criterion the two +// blocks above state and no wider: a refusal an AUTHOR can cause has a second +// door by construction — the author-time one (`@objectstack/lint`, `os build` / +// `os validate`) — and a door that re-spells the token or re-derives the +// wording is the drift these modules exist to prevent. A package declaring a +// CURATED platform capability name, and a declaration carrying no owning +// package, are both decidable from the declaration alone with no database. +// ⛔ The two unreadable-rows summaries are deliberately NOT here: an unreadable +// database is a runtime condition no compile-time door can raise, so they stay +// package-private for the reason `position_name_fold_grant` does. +export { + CAPABILITY_DECLARATION_UNOWNED, + CAPABILITY_PLATFORM_NAME_REFUSED, + PERMISSION_SET_DECLARATION_UNOWNED, + capabilityDeclarationUnownedDiagnostic, + capabilityPlatformNameRefusedDiagnostic, + permissionSetDeclarationUnownedDiagnostic, + reportCapabilityDeclarationUnowned, + reportCapabilityPlatformNameRefused, + reportPermissionSetDeclarationUnowned, +} from './seed-refusal-diagnostics.js'; +export type { + CapabilityDeclarationUnownedDiagnostic, + CapabilityPlatformNameRefusedDiagnostic, + PermissionSetDeclarationUnownedDiagnostic, +} from './seed-refusal-diagnostics.js'; +// [#18091] The delivery rule itself — one derivation, so a sixth refusal site +// never re-spells `logger?.warn?.(…)` and re-earns this card. +export { reportThroughSink } from './seed-refusal-sink.js'; // [ADR-0094] sys_permission_set pure-projection machinery. export { permissionSetRowFields, diff --git a/packages/plugins/plugin-security/src/seed-refusal-diagnostics.ts b/packages/plugins/plugin-security/src/seed-refusal-diagnostics.ts new file mode 100644 index 00000000000..ca6a3fe2a59 --- /dev/null +++ b/packages/plugins/plugin-security/src/seed-refusal-diagnostics.ts @@ -0,0 +1,393 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#18091] The five remaining refusal diagnostics of the two declared-metadata + * seeders — one wording, one token and one record PER SITE. + * + * ## What was wrong + * + * `bootstrapDeclaredCapabilities` and `bootstrapDeclaredPermissions` refuse to + * write in five more places, and every one of those refusals was spelled + * `logger?.warn?.(…)` — optionally chained TWICE, so a caller that injected no + * logger got NO OUTPUT AT ALL. Measured on the pre-fix tree, each site driven + * with no logger while all five console channels were spied: + * + * ```text + * curated platform capability refused skippedPlatform = 1 lines = 0 + * capability declaration unowned skippedUnowned = 1 lines = 0 + * capability rows unreadable unreadable = 1 lines = 0 + * permission set declaration unowned (no counter) lines = 0 + * permission set rows unreadable unreadable = 1 lines = 0 + * LIT CONTROL capability_name_collision skippedForeign = 1 lines = 1 + * LIT CONTROL permission_set_name_… skippedForeign = 1 lines = 1 + * ``` + * + * The two lit controls are the already-repaired axes in the SAME harness, so + * the zeros are a reading rather than an artefact of the probe. + * + * ⛔ Not one of the skips changes here. They are correct under ADR-0086 D4 (a + * package never writes into a foreign record) and ADR-0086 D3 (a row that + * cannot prove its owner makes uninstall undefined). The defect is only that + * the refusal never reached the author who caused it. + * + * ## Why five diagnostics and not one + * + * #18023's delivery established the rule and it is applied here rather than + * re-argued: the token, the record, the wording and the CONSEQUENCE are + * site-specific — four of the six parts a refusal report is made of. The five + * consequences below are genuinely different facts about the deployment: + * + * - a curated-platform-name hijack still RESOLVES (the curated pass owns the + * row) and loses only the declaring package's authored metadata; + * - an unowned CAPABILITY declaration may resolve, may fall back to the + * back-compat derived placeholder, or may exist nowhere at all — three + * outcomes, decided by what already stands in `sys_capability`; + * - an unowned PERMISSION SET keeps every grant working (the evaluator + * resolves declared sets through the metadata registry) and loses only the + * RECORD — the Setup surface, the provenance axis and uninstall; + * - an unreadable read wrote nothing and compared nothing, so nothing is lost + * and nothing arrived either — and what "nothing arrived" costs differs + * again between the two tables. + * + * Wording them into one generic "declaration skipped" sentence would send every + * author of the first kind hunting for a broken grant that is not broken. + * + * ## What IS shared + * + * Exactly one thing: WHERE the line goes. {@link reportThroughSink} carries the + * delivery rule for all five, so this card adds one derivation rather than five + * more copies of the two lines #17516 and #18023 each wrote out by hand. + * + * ## Why `event` and not `code` + * + * Every token below is a snake_case DATA VALUE, not an ADR-0112 error code: it + * is never routed to `error.code`, never reaches a wire refusal, and travels as + * the structured half of a warning about an artifact — the same discrimination + * `CAPABILITY_NAME_COLLISION` and `PERMISSION_SET_NAME_COLLISION` already make. + */ + +import { reportThroughSink, type CollisionReportSink } from './seed-refusal-sink.js'; + +export type { CollisionReportSink }; + +// ─────────────────────────────────────────────────────────────────────────── +// Tokens — one grep string per refusal kind. +// ─────────────────────────────────────────────────────────────────────────── + +/** A package declared a capability whose name is a CURATED platform capability. */ +export const CAPABILITY_PLATFORM_NAME_REFUSED = 'capability_platform_name_refused'; + +/** A declared capability carries no resolvable owning package. */ +export const CAPABILITY_DECLARATION_UNOWNED = 'capability_declaration_unowned'; + +/** Declared capabilities left untouched because `sys_capability` could not be read. */ +export const CAPABILITY_ROWS_UNREADABLE = 'capability_rows_unreadable'; + +/** A declared permission set carries no resolvable owning package. */ +export const PERMISSION_SET_DECLARATION_UNOWNED = 'permission_set_declaration_unowned'; + +/** Declared sets left untouched because `sys_permission_set` could not be read. */ +export const PERMISSION_SET_ROWS_UNREADABLE = 'permission_set_rows_unreadable'; + +/** + * Severity is `warning` on all five for the #4632 reason: this is FUNCTIONAL + * degradation, never a durability failure. Nothing claimed to be persisted + * silently failed to land. + */ +type RefusalSeverity = 'warning'; + +/** The shape every diagnostic in this module shares — ⛔ the wording is not part of it. */ +interface SeedRefusalDiagnosticBase { + readonly severity: RefusalSeverity; + readonly message: string; + readonly fix: string; +} + +// ─────────────────────────────────────────────────────────────────────────── +// SITE 1 — a package declares a CURATED platform capability name. +// ─────────────────────────────────────────────────────────────────────────── + +/** One declared capability refused because its name is platform-curated. */ +export interface CapabilityPlatformNameRefusedDiagnostic extends SeedRefusalDiagnosticBase { + readonly event: typeof CAPABILITY_PLATFORM_NAME_REFUSED; + /** The declared capability's name — a member of `PLATFORM_CAPABILITY_NAMES`. */ + readonly name: string; + /** + * The package whose declaration was dropped. `null` when the declaration + * carries no resolvable owner either: this refusal is decided BEFORE the + * owner check, so both faults can be true of one declaration and printing + * `undefined` at an author is not an option. + */ + readonly declaredBy: string | null; + /** The bootstrap permission set(s) granting it — the blast radius, named. */ + readonly grantedBy: readonly string[]; +} + +/** + * Word the curated-name refusal. + * + * ⚠️ The consequence differs from a foreign-owner collision even though both + * end in "the declaration was not applied": the curated pass of + * `bootstrapSystemCapabilities` seeds every curated name UNCONDITIONALLY, so + * the row is there whatever this seeder decides. Nothing is denied. What the + * author loses is the authored label/description/scope and the ADR-0086 D3 + * provenance claim — and the remedy is not "co-own the namespace" (the + * ADR-0130 D1 escape the collision diagnostic offers) but "rename", because a + * curated platform name is never available to a package on any terms. + */ +export function capabilityPlatformNameRefusedDiagnostic(input: { + name: string; + declaredBy?: string | null; + grantedBy?: readonly string[]; +}): CapabilityPlatformNameRefusedDiagnostic { + const declaredBy = input.declaredBy ?? null; + const declarer = declaredBy ?? '(a declaration with no owning package)'; + const grantedBy = [...(input.grantedBy ?? [])]; + const blastRadius = grantedBy.length > 0 + ? `Permission set(s) granting it: ${grantedBy.join(', ')} — those grants keep resolving, ` + + `against the PLATFORM's capability rather than this package's declaration.` + : `No bootstrap permission set grants it, so nothing is denied today — but the declaration ` + + `is still inert.`; + return { + event: CAPABILITY_PLATFORM_NAME_REFUSED, + severity: 'warning', + name: input.name, + declaredBy, + grantedBy, + message: + `[security] [${CAPABILITY_PLATFORM_NAME_REFUSED}] Package ${declarer} declares capability ` + + `"${input.name}", but that name is a CURATED PLATFORM capability. The declaration was NOT ` + + `applied — a package must never redefine a platform-owned capability — so the curated pass ` + + `keeps the sys_capability row, the declaring package's authored label, description and ` + + `scope are not in effect, and the capability carries no package provenance (ADR-0086 D3). ` + + `The name still resolves against the curated row, which is why nothing else reports this. ` + + `${blastRadius}`, + fix: + `Rename the capability in package ${declarer} to a name it owns (prefix it with the ` + + `package's own module prefix). ⛔ A curated platform name is not co-ownable: unlike a ` + + `package-to-package collision (ADR-0130 D1) there is no arrangement under which a package ` + + `may declare it.`, + }; +} + +/** Report one curated-name refusal so it reaches the author with no sink injected. */ +export function reportCapabilityPlatformNameRefused( + logger: CollisionReportSink | undefined, + d: CapabilityPlatformNameRefusedDiagnostic, +): void { + reportThroughSink(logger, `${d.message} Fix: ${d.fix}`, { + event: d.event, + name: d.name, + declaredBy: d.declaredBy, + grantedBy: [...d.grantedBy], + fix: d.fix, + }); +} + +// ─────────────────────────────────────────────────────────────────────────── +// SITE 2 — a declared CAPABILITY with no resolvable owning package. +// ─────────────────────────────────────────────────────────────────────────── + +/** One declared capability refused for want of an owning package. */ +export interface CapabilityDeclarationUnownedDiagnostic extends SeedRefusalDiagnosticBase { + readonly event: typeof CAPABILITY_DECLARATION_UNOWNED; + readonly name: string; + /** The bootstrap permission set(s) granting it. */ + readonly grantedBy: readonly string[]; + /** Whether a `sys_capability` row already resolves the name — it decides the consequence. */ + readonly hasRow: boolean; +} + +/** + * Word the unowned-capability refusal. + * + * ⚠️ [#4967 Part 3] The three-way consequence below is LOAD-BEARING and is + * preserved verbatim from the sentence this card found at the call site: a + * seeder-side warn that names the capability but not the permission set(s) that + * grant it does not tell the reader what happened. `hasRow` is the reason the + * existence read is taken BEFORE the refusal rather than after it. + */ +export function capabilityDeclarationUnownedDiagnostic(input: { + name: string; + grantedBy?: readonly string[]; + hasRow: boolean; +}): CapabilityDeclarationUnownedDiagnostic { + const grantedBy = [...(input.grantedBy ?? [])]; + const granted = grantedBy.length > 0 + ? `granted by ${grantedBy.join(', ')}` + : 'granted by no bootstrap permission set'; + const consequence = input.hasRow + ? 'an existing sys_capability row already resolves it and is left as-is — the declaration adds no package provenance' + : grantedBy.length > 0 + ? 'falls back to the back-compat derived placeholder — the grant resolves, but with no package provenance (ADR-0086 D3: uninstall undefined)' + : 'nothing derives it either — the capability is materialized nowhere'; + return { + event: CAPABILITY_DECLARATION_UNOWNED, + severity: 'warning', + name: input.name, + grantedBy, + hasRow: input.hasRow, + message: + `[security] [${CAPABILITY_DECLARATION_UNOWNED}] declared capability "${input.name}" has no ` + + `owning package (${granted}): ${consequence}`, + fix: + `Stamp the declaring package on the capability — the SchemaRegistry's _packageId ` + + `(ADR-0010), or the spec-level packageId (ADR-0086 D3) as the author-declared fallback. ` + + `A package-managed row with no package_id is the ambiguity ADR-0086 D3 removes, which is ` + + `why this seeder writes none.`, + }; +} + +/** Report one unowned-capability refusal so it reaches the author with no sink injected. */ +export function reportCapabilityDeclarationUnowned( + logger: CollisionReportSink | undefined, + d: CapabilityDeclarationUnownedDiagnostic, +): void { + reportThroughSink(logger, `${d.message}. Fix: ${d.fix}`, { + event: d.event, + name: d.name, + grantedBy: [...d.grantedBy], + hasRow: d.hasRow, + fix: d.fix, + }); +} + +// ─────────────────────────────────────────────────────────────────────────── +// SITE 3 — declared capabilities whose rows could not be READ. +// ─────────────────────────────────────────────────────────────────────────── + +/** + * Report the unreadable-rows summary for the capability pass. + * + * ONE line per pass with the count, never a line per name: a database that is + * down refuses every name at once and a warn each is a flood that buries its + * own meaning. + * + * ⚠️ The consequence has to be SAID. "Unreadable" alone states none, and the + * two halves differ: a genuinely new declaration among these names has no row + * at all, while one whose stored metadata drifted from the shipped declaration + * keeps the stale value. Nothing was written, so nothing is lost — the next + * boot with a readable database does both — but until then the registry does + * not reflect what these packages declare. + * + * ⛔ This one is deliberately NOT exported from the package entry: unlike the + * two refusals an author can cause, an unreadable database is a runtime + * condition no compile-time door can raise, so there is no second consumer by + * construction (the discrimination `position_name_fold_grant` already makes). + */ +export function reportCapabilityRowsUnreadable( + logger: CollisionReportSink | undefined, + input: { unreadable: number; total: number }, +): void { + reportThroughSink( + logger, + `[security] [${CAPABILITY_ROWS_UNREADABLE}] declared capabilities left untouched — their ` + + `sys_capability rows could not be read. ${input.unreadable} of ${input.total} declaration(s) ` + + `were neither created nor reconciled: a genuinely new one among them has no sys_capability ` + + `row, and one whose stored label, description or scope has drifted from the shipped ` + + `declaration keeps the stale value. Nothing was written and nothing is lost — the next boot ` + + `with a readable database does both — but until then the registry does not reflect what ` + + `these packages declare.`, + { event: CAPABILITY_ROWS_UNREADABLE, unreadable: input.unreadable, total: input.total }, + ); +} + +// ─────────────────────────────────────────────────────────────────────────── +// SITE 4 — a declared PERMISSION SET with no resolvable owning package. +// ─────────────────────────────────────────────────────────────────────────── + +/** One declared permission set refused for want of an owning package. */ +export interface PermissionSetDeclarationUnownedDiagnostic extends SeedRefusalDiagnosticBase { + readonly event: typeof PERMISSION_SET_DECLARATION_UNOWNED; + readonly name: string; + /** The organization whose pass refused it, when the walled per-organization door is in force. */ + readonly organizationId?: string; +} + +/** + * Word the unowned-permission-set refusal. + * + * ⚠️ The consequence is NOT the capability axis'. `stack.permissions` has + * always been runtime-ENFORCED — the evaluator resolves declared sets through + * the metadata registry — so every grant in the refused set keeps working. What + * is lost is the RECORD (ADR-0086 D5, the ADR-0078 inert-metadata smell this + * seeder exists to close): the Setup admin surface reads `sys_permission_set` + * and cannot see the set, there is no provenance axis for it, and uninstall + * (`cleanupPackagePermissions`) has nothing to reap. An author told "not + * materialized" and left to guess will go looking for a denied user who does + * not exist. + */ +export function permissionSetDeclarationUnownedDiagnostic(input: { + name: string; + organizationId?: string; +}): PermissionSetDeclarationUnownedDiagnostic { + return { + event: PERMISSION_SET_DECLARATION_UNOWNED, + severity: 'warning', + name: input.name, + ...(input.organizationId ? { organizationId: input.organizationId } : {}), + message: + `[security] [${PERMISSION_SET_DECLARATION_UNOWNED}] declared permission set ` + + `"${input.name}" has no owning package — not materialized. A managed_by:'package' row ` + + `without a package_id is the ambiguity ADR-0086 D3 removes, so no row is written. The ` + + `evaluator still resolves the DECLARED set through the metadata registry, so its object, ` + + `field, tab and system permissions all keep working — what is missing is the RECORD: the ` + + `Setup admin surface reads sys_permission_set and cannot see this set, it carries no ` + + `provenance axis, and uninstall has nothing to reap (ADR-0086 D5).`, + fix: + `Stamp the declaring package on the set — the SchemaRegistry's _packageId (ADR-0010), or ` + + `the spec-level packageId (ADR-0086 D3) as the author-declared fallback.`, + }; +} + +/** Report one unowned-set refusal so it reaches the author with no sink injected. */ +export function reportPermissionSetDeclarationUnowned( + logger: CollisionReportSink | undefined, + d: PermissionSetDeclarationUnownedDiagnostic, +): void { + reportThroughSink(logger, `${d.message} Fix: ${d.fix}`, { + event: d.event, + name: d.name, + ...(d.organizationId ? { organization: d.organizationId } : {}), + fix: d.fix, + }); +} + +// ─────────────────────────────────────────────────────────────────────────── +// SITE 5 — declared permission sets whose rows could not be READ. +// ─────────────────────────────────────────────────────────────────────────── + +/** + * Report the unreadable-rows summary for the permission-set pass. + * + * ⚠️ Said once with the count, and the consequence spelled out for the SAME + * reason as the capability summary and with a DIFFERENT content: silence here + * reads exactly like "everything was already in order". The declared sets stay + * runtime-enforced through the metadata registry, so nothing is denied — what + * did not happen is the materialization, and a drifted row keeps its stale + * grants until a boot that can read the table. + * + * ⛔ Package-private for the same reason as its capability sibling: an + * unreadable database is a runtime condition, not an authoring fault. + */ +export function reportPermissionSetRowsUnreadable( + logger: CollisionReportSink | undefined, + input: { unreadable: number; total: number; organizationId?: string }, +): void { + reportThroughSink( + logger, + `[security] [${PERMISSION_SET_ROWS_UNREADABLE}] declared permission sets left untouched — ` + + `their records could not be read. ${input.unreadable} of ${input.total} declared set(s) ` + + `were neither seeded nor reconciled: a genuinely new set has no sys_permission_set row, so ` + + `the Setup admin surface cannot see it and uninstall has nothing to reap (ADR-0086 D5), and ` + + `a set whose stored grants drifted from the shipped declaration keeps the stale row. The ` + + `evaluator still resolves the DECLARED sets through the metadata registry, so no grant is ` + + `denied today; the next boot with a readable database materializes them.`, + { + event: PERMISSION_SET_ROWS_UNREADABLE, + unreadable: input.unreadable, + total: input.total, + ...(input.organizationId ? { organization: input.organizationId } : {}), + }, + ); +} diff --git a/packages/plugins/plugin-security/src/seed-refusal-sink.ts b/packages/plugins/plugin-security/src/seed-refusal-sink.ts new file mode 100644 index 00000000000..b92e7554007 --- /dev/null +++ b/packages/plugins/plugin-security/src/seed-refusal-sink.ts @@ -0,0 +1,83 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#18091] `reportThroughSink` — the ONE derivation of "a seeder refusal + * reaches the author even when no logger was injected". + * + * ## Why this is a module and not a third hand-written copy + * + * The doubly-optional call `logger?.warn?.(…)` evaluates to NOTHING when the + * caller passed no logger: the declaration is dropped, one internal counter + * moves, and no human is told. That shape has now been repaired + * instance-by-instance TWICE — once on the permission-set axis (#17516) and + * once on the capability axis (#18023) — and each repair restated the same two + * lines at its own call site. A shape repaired one instance at a time is a + * CLASS that has not been fixed, which is why the five remaining refusal sites + * in the two declared-metadata seeders share ONE delivery derivation instead of + * gaining five more copies of it. + * + * What is shared here is exactly the part that is axis-INDEPENDENT: WHERE the + * line goes. ⛔ What is deliberately NOT shared is the wording, the token, the + * record and the consequence — those are axis-specific (four of the six parts a + * refusal report is made of), and folding them into one generic sentence is the + * failure mode `seed-refusal-diagnostics.ts` is written to avoid. + * + * ## The three spellings this replaces, and why each is wrong + * + * - ⛔ `logger?.warn?.(…)` — silent with no sink. THE defect. + * - ⛔ `(logger?.warn ?? console.warn)(…)` — evaluates to a bare function and + * calls it with `this === undefined`; a class-based host sink + * (`@objectstack/core`'s `ObjectLogger`) reaches for `this` and throws. The + * property-access call form below keeps the receiver — the same measured + * conclusion `logSeedDurabilityFailure`, `reportPermissionSetNameCollisions` + * and `reportCapabilityNameCollisions` all record. + * - ⛔ `if (logger) logger.warn(…)` — correct for a sink whose `warn` the TYPE + * guarantees, but it THROWS for a host the type cannot reach (a plain-JS + * embedder, or a cast). `permission-set-projection.ts` records that measured + * hazard on `ProjectionLogger.warn`: a sink that lies about its shape + * throws `logger?.warn is not a function` inside a per-row durability catch + * and aborts the very batch that function promises never to stop. + * + * So the guard below is a `typeof` check rather than a truthiness test, and it + * is strictly better than BOTH halves of the old trade-off: a lying host no + * longer buys its silence with a throw — it gets the console, and the author + * still hears the refusal. + * + * ⚠️ This is the REFUSAL channel only. The end-of-pass `logger?.info?.(…)` + * summary in each seeder keeps its outer `?.` deliberately: a pass that did its + * work and refused nothing MUST stay silent on every console channel with no + * sink injected, which is the discriminating control #18023 landed. Routing a + * healthy boot's info line to `console.info` would turn every one of those + * controls into noise and buy no author anything. + */ + +import type { CollisionReportSink } from './permission-set-name-collision.js'; + +/** + * Re-exported for the reason #18088 gave when it did the same: a consumer of + * this delivery rule never declares a second structural copy of the sink. + */ +export type { CollisionReportSink }; + +/** + * Print one refusal so it reaches the author. + * + * `warn`, not `error`: every caller here reports FUNCTIONAL degradation (#4632) + * — the deployment is visibly less than it was authored to be. Nothing claimed + * to be persisted silently failed to land; the write was deliberately never + * attempted, or the read that would have decided it never answered. + * + * @param logger the host sink, or `undefined` when none was injected + * @param message the fully worded, axis-specific line — ⛔ never assembled here + * @param meta the structured half of the same finding + */ +export function reportThroughSink( + logger: CollisionReportSink | undefined, + message: string, + meta?: Record, +): void { + // ⛔ `typeof`, not truthiness — see the module header's third bullet. The + // property-access call form keeps the receiver for a class-based sink. + if (logger && typeof logger.warn === 'function') logger.warn(message, meta); + else console.warn(message, meta); +} From ab9ba9e4589031d0d536cc1c64f599cb813014fb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 01:08:40 +0000 Subject: [PATCH 2/4] test(plugin-security): pin each seeder refusal's own wording, with no logger injected Per-site pins: the token, the site-specific consequence, the unchanged skip, the injected-logger path, a lying host sink, and a class pin that reds when the doubly-optional spelling returns to either seeder as a call site. Claude-Session: https://claude.ai/code/session_01WmBwEiWPff9JZPd5BSGNeH Co-authored-by: Claude --- .../bootstrap-declared-capabilities.test.ts | 210 ++++++++++++++++++ .../bootstrap-declared-permissions.test.ts | 202 ++++++++++++++++- 2 files changed, 411 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.test.ts b/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.test.ts index 8f496efa098..c6a4cd8ba68 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.test.ts @@ -1,9 +1,21 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + import { describe, it, expect, vi } from 'vitest'; import { bootstrapDeclaredCapabilities } from './bootstrap-declared-capabilities.js'; import { bootstrapSystemCapabilities } from './bootstrap-system-capabilities.js'; import { CAPABILITY_NAME_COLLISION } from './capability-name-collision.js'; +import { + CAPABILITY_DECLARATION_UNOWNED, + CAPABILITY_PLATFORM_NAME_REFUSED, + CAPABILITY_ROWS_UNREADABLE, +} from './seed-refusal-diagnostics.js'; + +/** [#18091] Seeded from this file, for the class pin at the bottom. */ +const HERE = dirname(fileURLToPath(import.meta.url)); /** Minimal in-memory ql for sys_capability seeding with a registry stub. */ function makeQl(declared: any[] = []) { @@ -705,3 +717,201 @@ describe('[#18023] a capability-name collision reaches the author', () => { expect(out.collisions).toBeUndefined(); }); }); + +// ─────────────────────────────────────────────────────────────────────────── +// [#18091] The three refusals #18023 left behind in this seeder. Same defect, +// same reading: driven with NO logger the author-visible line count was 0 at +// every one of them while the already-repaired collision path in the harness +// above read 1. Each case below asserts THAT SITE'S OWN sentence, so a future +// regression to one generic "declaration skipped" line is caught by name. +// ─────────────────────────────────────────────────────────────────────────── + +/** `makeQl` with a read that cannot answer — the `unreadable` branch's only entry. */ +function unreadableQl(declared: any[]) { + const ql = makeQl(declared); + (ql as any).find = async () => { throw new Error('sys_capability is unreachable'); }; + return ql; +} + +describe('[#18091] the three remaining capability refusals reach the author', () => { + it('CURATED PLATFORM NAME: prints with NO LOGGER INJECTED, and names what is lost', async () => { + const ql = makeQl([{ name: 'manage_users', label: 'Evil', description: 'Hijack.', _packageId: 'com.acme.evil' }]); + + const cap = captureAllConsole(); + let out: Awaited>; + try { + out = await bootstrapDeclaredCapabilities(ql, null, { + permissionSets: [{ name: 'acme_ops', systemPermissions: ['manage_users'] }], + }); + } finally { + cap.restore(); + } + + // ── The reading this card moves: 0 → 1 ────────────────────────────────── + expect(out.skippedPlatform).toBe(1); + expect(cap.seen).toHaveLength(1); + expect(cap.seen[0]!.startsWith('warn: ')).toBe(true); + expect(cap.seen[0]).toContain(CAPABILITY_PLATFORM_NAME_REFUSED); + expect(cap.seen[0]).toContain('manage_users'); + expect(cap.seen[0]).toContain('com.acme.evil'); + // ── THIS site's consequence, not the foreign-owner one. The curated row + // answers for the name, so nothing is denied — an author sent hunting + // for a broken grant is the failure this wording prevents. + expect(cap.seen[0]).toContain('CURATED PLATFORM capability'); + expect(cap.seen[0]).toContain('The name still resolves'); + expect(cap.seen[0]).toContain('acme_ops'); + // ⛔ And the remedy is rename-only: a curated name is not co-ownable, so + // the ADR-0130 D1 escape the collision diagnostic offers must NOT appear. + expect(cap.seen[0]).toContain('not co-ownable'); + + // ── ⛔ The refusal itself is UNCHANGED ─────────────────────────────────── + expect(ql.rows.find((r) => r.name === 'manage_users')).toBeUndefined(); + expect(out.seeded).toBe(0); + // …and the name is still reported materialized: the curated pass owns it. + expect(out.materializedNames).toEqual(['manage_users']); + }); + + it('UNOWNED DECLARATION: prints with NO LOGGER INJECTED, keeping its three-way consequence', async () => { + const ql = makeQl([{ name: 'showcase.export_data', label: 'Export Data' }]); + + const cap = captureAllConsole(); + let out: Awaited>; + try { + out = await bootstrapDeclaredCapabilities(ql, null, { + permissionSets: [{ name: 'showcase_ops', systemPermissions: ['showcase.export_data'] }], + }); + } finally { + cap.restore(); + } + + expect(out.skippedUnowned).toBe(1); + expect(cap.seen).toHaveLength(1); + expect(cap.seen[0]!.startsWith('warn: ')).toBe(true); + expect(cap.seen[0]).toContain(CAPABILITY_DECLARATION_UNOWNED); + expect(cap.seen[0]).toContain('has no owning package'); + // [#4967 Part 3] The grantor and the ACTUAL consequence, both preserved — + // this arm is "a row will be derived", which is not the other two arms. + expect(cap.seen[0]).toContain('showcase_ops'); + expect(cap.seen[0]).toContain('derived placeholder'); + expect(cap.seen[0]).not.toContain('materialized nowhere'); + // ⛔ Unchanged: no row is written for an unowned declaration. + expect(ql.rows.find((r) => r.name === 'showcase.export_data')).toBeUndefined(); + }); + + it('UNREADABLE ROWS: prints with NO LOGGER INJECTED, with the count and the consequence', async () => { + const ql = unreadableQl([ + { name: 'a.one', _packageId: 'com.a' }, + { name: 'a.two', _packageId: 'com.a' }, + ]); + + const cap = captureAllConsole(); + let out: Awaited>; + try { + out = await bootstrapDeclaredCapabilities(ql, null); + } finally { + cap.restore(); + } + + expect(out.unreadable).toBe(2); + expect(cap.seen).toHaveLength(1); + expect(cap.seen[0]!.startsWith('warn: ')).toBe(true); + expect(cap.seen[0]).toContain(CAPABILITY_ROWS_UNREADABLE); + // The count, and the consequence "unreadable" alone does not state. + expect(cap.seen[0]).toContain('2 of 2'); + expect(cap.seen[0]).toContain('keeps the stale value'); + expect(cap.seen[0]).toContain('nothing is lost'); + // ⛔ Unchanged: a name whose row could not be read is left ENTIRELY alone, + // and stays out of `materializedNames` so the derivation gets its attempt. + expect(ql.rows).toHaveLength(0); + expect(out.materializedNames).toEqual([]); + }); + + it('⭐ each site keeps its OWN sentence — ⛔ never one generic refusal line', async () => { + // The discriminating case for R3: two different refusals in ONE pass. + const ql = makeQl([ + { name: 'manage_users', _packageId: 'com.acme.evil' }, + { name: 'orphan_cap' }, + ]); + + const cap = captureAllConsole(); + try { + await bootstrapDeclaredCapabilities(ql, null); + } finally { + cap.restore(); + } + + expect(cap.seen).toHaveLength(2); + const curated = cap.seen.find((l) => l.includes(CAPABILITY_PLATFORM_NAME_REFUSED)); + const unowned = cap.seen.find((l) => l.includes(CAPABILITY_DECLARATION_UNOWNED)); + expect(curated).toBeDefined(); + expect(unowned).toBeDefined(); + // ⛔ Two tokens, two consequences. A generic sentence would make these two + // assertions pass against ONE wording, so each names a phrase only its own + // site can produce. + expect(curated).toContain('The name still resolves'); + expect(curated).not.toContain('derived placeholder'); + expect(unowned).toContain('materialized nowhere'); + expect(unowned).not.toContain('CURATED PLATFORM capability'); + }); + + it('an INJECTED logger takes all three, and the console stays clean', async () => { + const warn = vi.fn(); + const cap = captureAllConsole(); + try { + await bootstrapDeclaredCapabilities(makeQl([{ name: 'manage_users', _packageId: 'com.a' }]), null, { logger: { warn } }); + await bootstrapDeclaredCapabilities(makeQl([{ name: 'orphan_cap' }]), null, { logger: { warn } }); + await bootstrapDeclaredCapabilities(unreadableQl([{ name: 'a.one', _packageId: 'com.a' }]), null, { logger: { warn } }); + } finally { + cap.restore(); + } + + // ⚠️ FOUR, not three: the third pass ALSO trips the batched existence + // oracle's own read-failure line, which is a different diagnostic in a + // different module (`seed-name-lookup.ts`) and outside this card. Pinning + // three here would have made that line's removal invisible; filtering by + // this card's own tokens keeps the assertion about this card. + expect(warn).toHaveBeenCalledTimes(4); + const events = warn.mock.calls.map((c) => (c[1] as any)?.event).filter(Boolean); + expect(events).toEqual([ + CAPABILITY_PLATFORM_NAME_REFUSED, + CAPABILITY_DECLARATION_UNOWNED, + CAPABILITY_ROWS_UNREADABLE, + ]); + expect(cap.seen).toEqual([]); + }); + + it('a HOST SINK THAT LIES about its shape is reported to the console, never thrown at', async () => { + // ⚠️ `ProjectionLogger.warn` is non-optional, but the type cannot reach a + // plain-JS embedder or a cast. The old `logger?.warn?.()` bought safety here + // with silence; `if (logger) logger.warn()` would buy noise with a throw + // inside a seeding pass. The `typeof` guard buys neither. + const liar = { info: () => {} } as any; + const cap = captureAllConsole(); + let out: Awaited>; + try { + out = await bootstrapDeclaredCapabilities(makeQl([{ name: 'orphan_cap' }]), null, { logger: liar }); + } finally { + cap.restore(); + } + expect(out.skippedUnowned).toBe(1); + expect(cap.seen).toHaveLength(1); + expect(cap.seen[0]).toContain(CAPABILITY_DECLARATION_UNOWNED); + }); + + it('⛔ CLASS PIN: the doubly-optional warn survives in this seeder only as PROSE', async () => { + // The reason this card exists: the shape was repaired one instance at a + // time twice before. A grep that reds when a sixth call site appears is the + // difference between a third instance repair and a class that is closed. + const source = readFileSync(resolve(HERE, 'bootstrap-declared-capabilities.ts'), 'utf8'); + // Positive control — the pin is reading the file it thinks it is. + expect(source).toContain('export async function bootstrapDeclaredCapabilities'); + const hits = source.split('\n').filter((line) => line.includes('logger?.warn?.(')); + for (const line of hits) { + expect(line.trimStart().startsWith('//') || line.trimStart().startsWith('*')).toBe(true); + } + // ⚠️ The INFO channel keeps its outer `?.` deliberately and is NOT part of + // this class: a pass that refused nothing must stay silent on every console + // channel with no sink, which is the control above. + expect(source).toContain("options.logger?.info?.("); + }); +}); diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts index 97bd99c6f99..2981de884cf 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts @@ -1,10 +1,21 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. -import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { describe, it, expect, vi } from 'vitest'; import { bootstrapDeclaredPermissions, upsertPackagePermissionSet, } from './bootstrap-declared-permissions.js'; +import { + PERMISSION_SET_DECLARATION_UNOWNED, + PERMISSION_SET_ROWS_UNREADABLE, +} from './seed-refusal-diagnostics.js'; + +/** [#18091] Seeded from this file, for the class pin at the bottom. */ +const HERE = dirname(fileURLToPath(import.meta.url)); /** Minimal in-memory ql + registry for sys_permission_set seeding. */ function makeQl(declared: any[] = []) { @@ -195,3 +206,192 @@ describe('upsertPackagePermissionSet (ADR-0086 P2 — publish materialization)', // The environment door (env-scope saves, the data-door write-through, boot // reconciliation) moved to permission-set-projection.test.ts (ADR-0094). + +// ─────────────────────────────────────────────────────────────────────────── +// [#18091] The two refusals #17516 left behind in this seeder. Measured mute on +// the pre-fix tree with no logger injected — author-visible lines = 0 at both, +// while the already-repaired collision path in the same harness read 1. +// ─────────────────────────────────────────────────────────────────────────── + +/** + * Capture EVERY console channel — "author-visible output" is not + * channel-specific, and a pin watching only `warn` could be satisfied by a + * change that merely MOVED the silence. + */ +function captureAllConsole() { + const seen: string[] = []; + const spies = (['log', 'info', 'warn', 'error', 'debug'] as const).map((m) => + vi.spyOn(console, m).mockImplementation((...args: unknown[]) => { + seen.push(`${m}: ${args.map((a) => (typeof a === 'string' ? a : JSON.stringify(a))).join(' ')}`); + }), + ); + return { seen, restore: () => spies.forEach((s) => s.mockRestore()) }; +} + +/** `makeQl` with a read that cannot answer — the `unreadable` branch's only entry. */ +function unreadableQl(declared: any[]) { + const ql = makeQl(declared); + (ql as any).find = async () => { throw new Error('sys_permission_set is unreachable'); }; + return ql; +} + +describe('[#18091] the two remaining permission-set refusals reach the author', () => { + it('UNOWNED DECLARATION: prints with NO LOGGER INJECTED, and names the RECORD as what is lost', async () => { + const ql = makeQl([declaredSet({ _packageId: undefined })]); + + const cap = captureAllConsole(); + let r: Awaited>; + try { + r = await bootstrapDeclaredPermissions(ql, undefined); + } finally { + cap.restore(); + } + + // ── The reading this card moves: 0 → 1 ────────────────────────────────── + expect(cap.seen).toHaveLength(1); + expect(cap.seen[0]!.startsWith('warn: ')).toBe(true); + expect(cap.seen[0]).toContain(PERMISSION_SET_DECLARATION_UNOWNED); + expect(cap.seen[0]).toContain('crm_sales_rep'); + expect(cap.seen[0]).toContain('has no owning package'); + // ── THIS site's consequence. ⛔ NOT the capability axis': the evaluator + // resolves declared sets through the metadata registry, so every grant + // keeps working and only the RECORD is missing. An author told merely + // "not materialized" goes looking for a denied user who does not exist. + expect(cap.seen[0]).toContain('keep working'); + expect(cap.seen[0]).toContain('Setup admin surface'); + expect(cap.seen[0]).toContain('ADR-0086 D3'); + + // ── ⛔ The refusal itself is UNCHANGED ─────────────────────────────────── + expect(r.seeded).toBe(0); + expect(ql.rows).toHaveLength(0); + }); + + it('UNOWNED DECLARATION reaches the author through the ADR-0086 P2 PUBLISH door too', async () => { + // ⚠️ The second door onto the same branch: the publish materializer upserts + // ONE set and passes no collector, so a fix that only lit the boot loop + // would leave this caller exactly as mute as before. + const ql = makeQl(); + + const cap = captureAllConsole(); + let r: Awaited>; + try { + r = await upsertPackagePermissionSet(ql, declaredSet({ _packageId: undefined }), null); + } finally { + cap.restore(); + } + + expect(cap.seen).toHaveLength(1); + expect(cap.seen[0]).toContain(PERMISSION_SET_DECLARATION_UNOWNED); + expect(r.seeded).toBe(0); + expect(ql.rows).toHaveLength(0); + }); + + it('UNREADABLE ROWS: prints with NO LOGGER INJECTED, with the count and the consequence', async () => { + const ql = unreadableQl([declaredSet(), declaredSet({ name: 'crm_manager' })]); + + const cap = captureAllConsole(); + let r: Awaited>; + try { + r = await bootstrapDeclaredPermissions(ql, undefined); + } finally { + cap.restore(); + } + + expect(r.unreadable).toBe(2); + const line = cap.seen.find((l) => l.includes(PERMISSION_SET_ROWS_UNREADABLE)); + expect(line).toBeDefined(); + expect(line!.startsWith('warn: ')).toBe(true); + expect(line).toContain('2 of 2'); + // Silence here reads exactly like "everything was already in order", so the + // line has to say what did NOT happen — and that no grant is denied by it. + expect(line).toContain('neither seeded nor reconciled'); + expect(line).toContain('no grant is'); + // ⛔ Unchanged: an unreadable read writes nothing. + expect(ql.rows).toHaveLength(0); + }); + + it('⭐ each site keeps its OWN sentence — ⛔ never one generic refusal line', async () => { + const cap = captureAllConsole(); + try { + await bootstrapDeclaredPermissions(makeQl([declaredSet({ _packageId: undefined })]), undefined); + await bootstrapDeclaredPermissions(unreadableQl([declaredSet()]), undefined); + } finally { + cap.restore(); + } + + const unowned = cap.seen.find((l) => l.includes(PERMISSION_SET_DECLARATION_UNOWNED)); + const unreadable = cap.seen.find((l) => l.includes(PERMISSION_SET_ROWS_UNREADABLE)); + expect(unowned).toBeDefined(); + expect(unreadable).toBeDefined(); + // Each names a phrase only its own site can produce. + expect(unowned).toContain('Setup admin surface'); + expect(unowned).not.toContain('could not be read'); + expect(unreadable).toContain('could not be read'); + expect(unreadable).not.toContain('ADR-0086 D3'); + }); + + it('an INJECTED logger takes both, and the console stays clean', async () => { + const warn = vi.fn(); + const cap = captureAllConsole(); + try { + await bootstrapDeclaredPermissions(makeQl([declaredSet({ _packageId: undefined })]), undefined, { logger: { warn } }); + await bootstrapDeclaredPermissions(unreadableQl([declaredSet()]), undefined, { logger: { warn } }); + } finally { + cap.restore(); + } + + // ⚠️ The second pass also trips the batched existence oracle's own + // read-failure line — a different diagnostic in `seed-name-lookup.ts`, + // outside this card — so filter by this card's tokens rather than counting. + const events = warn.mock.calls.map((c) => (c[1] as any)?.event).filter(Boolean); + expect(events).toEqual([PERMISSION_SET_DECLARATION_UNOWNED, PERMISSION_SET_ROWS_UNREADABLE]); + expect(cap.seen).toEqual([]); + }); + + it('CONTROL: a pass that really seeds and refuses nothing is SILENT on all five channels', async () => { + // ⛔ The discriminating half. Without it, a seeder that warned on every + // declaration would satisfy every assertion above. + const ql = makeQl([declaredSet()]); + + const cap = captureAllConsole(); + let r: Awaited>; + try { + r = await bootstrapDeclaredPermissions(ql, undefined); + } finally { + cap.restore(); + } + + expect(r.seeded).toBe(1); + expect(cap.seen).toEqual([]); + }); + + it('a HOST SINK THAT LIES about its shape is reported to the console, never thrown at', async () => { + // The old `logger?.warn?.()` bought safety against a plain-JS embedder with + // silence; a bare `logger.warn()` would buy noise with a throw inside the + // seeding pass. The `typeof` guard in `reportThroughSink` buys neither. + const liar = { info: () => {} } as any; + const cap = captureAllConsole(); + let r: Awaited>; + try { + r = await bootstrapDeclaredPermissions(makeQl([declaredSet({ _packageId: undefined })]), undefined, { logger: liar }); + } finally { + cap.restore(); + } + expect(r.seeded).toBe(0); + expect(cap.seen).toHaveLength(1); + expect(cap.seen[0]).toContain(PERMISSION_SET_DECLARATION_UNOWNED); + }); + + it('⛔ CLASS PIN: the doubly-optional warn survives in this seeder only as PROSE', async () => { + const source = readFileSync(resolve(HERE, 'bootstrap-declared-permissions.ts'), 'utf8'); + // Positive control — the pin is reading the file it thinks it is. + expect(source).toContain('export async function bootstrapDeclaredPermissions'); + const hits = source.split('\n').filter((line) => line.includes('logger?.warn?.(')); + for (const line of hits) { + expect(line.trimStart().startsWith('//') || line.trimStart().startsWith('*')).toBe(true); + } + // ⚠️ The INFO channel keeps its outer `?.` deliberately — a healthy pass + // must stay silent on every console channel, per the CONTROL above. + expect(source).toContain("options.logger?.info?.("); + }); +}); From cec185ad398774cb35868044944129cac02846a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 01:10:50 +0000 Subject: [PATCH 3/4] chore(changeset): 18091 seeder refusal diagnostics Claude-Session: https://claude.ai/code/session_01WmBwEiWPff9JZPd5BSGNeH Co-authored-by: Claude --- .../18091-seeder-refusal-diagnostics.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 .changeset/18091-seeder-refusal-diagnostics.md diff --git a/.changeset/18091-seeder-refusal-diagnostics.md b/.changeset/18091-seeder-refusal-diagnostics.md new file mode 100644 index 00000000000..878913f2abb --- /dev/null +++ b/.changeset/18091-seeder-refusal-diagnostics.md @@ -0,0 +1,37 @@ +--- +"@objectstack/plugin-security": minor +--- + +**The five remaining seeder refusals now reach the author.** The two declared-metadata seeders refuse to write in five more places, and every one of them reported through `logger?.warn?.(…)` — optionally chained **twice**, so a caller that injected no logger got no output at all (#18091). + +Measured on the pre-change tree, each site driven with **no logger passed** while all five console channels were spied, beside the two already-repaired axes as lit controls in the same harness: + +``` + counter author-visible lines +curated platform capability refused skippedPlatform = 1 0 +capability declaration unowned skippedUnowned = 1 0 +capability rows unreadable unreadable = 1 0 +permission set declaration unowned (no counter at all) 0 +permission set rows unreadable unreadable = 1 0 +LIT CONTROL capability_name_collision skippedForeign = 1 1 +LIT CONTROL permission_set_name_… skippedForeign = 1 1 +``` + +Every one of those zeros is now a 1, with the counters unchanged. + +⛔ **No skip changed.** They are correct under ADR-0086 D4 (a package never writes into a foreign record) and ADR-0086 D3 (a package-managed row with no `package_id` makes uninstall undefined). The defect was only that the refusal never reached the author who caused it. + +**Each site words its own consequence** — the reason a mechanical copy was rejected. A curated-platform-name hijack still *resolves* against the curated row, so nothing is denied and only the authored metadata and the provenance claim are lost; an unowned **capability** has three different outcomes depending on what already stands in `sys_capability`; an unowned **permission set** keeps every grant working (the evaluator resolves declared sets through the metadata registry) and loses only the *record* — the Setup surface, the provenance axis and uninstall; and an unreadable read compared nothing, so nothing is lost and nothing arrived either. One generic "declaration skipped" line would send the first author hunting for a broken grant that is not broken. + +**What is shared is exactly one thing: where the line goes.** This shape had already been repaired one instance at a time twice, each repair restating the same two lines at its own call site. `reportThroughSink()` is now the single derivation, so a sixth refusal site cannot re-earn this card. It also improves on both spellings it replaces: a host sink that lies about its shape used to buy safety with silence (`logger?.warn?.(…)`) or noise with a throw (`logger.warn(…)`) — the `typeof` guard buys neither, and keeps the receiver so a class-based host logger does not throw. + +New published surface on `@objectstack/plugin-security`, on the criterion the two existing collision diagnostics state and no wider — a refusal an **author** can cause has a second door by construction (`@objectstack/lint`, `os build` / `os validate`), and both of these are decidable from the declaration alone with no database: + +- `CAPABILITY_PLATFORM_NAME_REFUSED` / `capabilityPlatformNameRefusedDiagnostic()` / `reportCapabilityPlatformNameRefused()` and the `CapabilityPlatformNameRefusedDiagnostic` record. +- `CAPABILITY_DECLARATION_UNOWNED` / `capabilityDeclarationUnownedDiagnostic()` / `reportCapabilityDeclarationUnowned()` and the `CapabilityDeclarationUnownedDiagnostic` record. +- `PERMISSION_SET_DECLARATION_UNOWNED` / `permissionSetDeclarationUnownedDiagnostic()` / `reportPermissionSetDeclarationUnowned()` and the `PermissionSetDeclarationUnownedDiagnostic` record. +- `reportThroughSink()` — the delivery rule itself. + +⛔ The two unreadable-rows summaries are deliberately **not** published: an unreadable database is a runtime condition no compile-time door can raise, so they stay package-private for the reason `position_name_fold_grant` does. + +⚠️ The end-of-pass `logger?.info?.(…)` summary in each seeder keeps its outer `?.` **deliberately**. A pass that did its work and refused nothing must stay silent on every console channel with no sink injected; routing a healthy boot's info line to the console would turn that control into noise and buy no author anything. The refusal channel is the one where silence was the defect. From 3646db601f135e1538af4ac0db6232e7e09ff974 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 10:56:23 +0000 Subject: [PATCH 4/4] fix(plugin-security): unpublish reportThroughSink from the package entry Contract review item 11: the delivery primitive is not "a refusal an author can cause" and has no second-door consumer -- the cli door has no channel for a sink printer. The package's own logSeedDurabilityFailure records the same class as "deliberately absent from the package's index.ts: an intra-package helper, not public API". The shape is ~390 raw hits across 29 package directories, so a shared delivery rule's home is a core package, not this plugin's entry. Not a breaking change: the export never shipped -- it exists only on this unmerged branch. The function does not move either; seed-refusal-diagnostics.ts imports it relatively and all five report sites keep working. Changeset stays minor (9 value + 3 type exports remain). Claude-Session: https://claude.ai/code/session_01WmBwEiWPff9JZPd5BSGNeH Co-authored-by: Claude --- .changeset/18091-seeder-refusal-diagnostics.md | 1 - packages/plugins/plugin-security/src/index.ts | 3 --- 2 files changed, 4 deletions(-) diff --git a/.changeset/18091-seeder-refusal-diagnostics.md b/.changeset/18091-seeder-refusal-diagnostics.md index 878913f2abb..b31044db520 100644 --- a/.changeset/18091-seeder-refusal-diagnostics.md +++ b/.changeset/18091-seeder-refusal-diagnostics.md @@ -30,7 +30,6 @@ New published surface on `@objectstack/plugin-security`, on the criterion the tw - `CAPABILITY_PLATFORM_NAME_REFUSED` / `capabilityPlatformNameRefusedDiagnostic()` / `reportCapabilityPlatformNameRefused()` and the `CapabilityPlatformNameRefusedDiagnostic` record. - `CAPABILITY_DECLARATION_UNOWNED` / `capabilityDeclarationUnownedDiagnostic()` / `reportCapabilityDeclarationUnowned()` and the `CapabilityDeclarationUnownedDiagnostic` record. - `PERMISSION_SET_DECLARATION_UNOWNED` / `permissionSetDeclarationUnownedDiagnostic()` / `reportPermissionSetDeclarationUnowned()` and the `PermissionSetDeclarationUnownedDiagnostic` record. -- `reportThroughSink()` — the delivery rule itself. ⛔ The two unreadable-rows summaries are deliberately **not** published: an unreadable database is a runtime condition no compile-time door can raise, so they stay package-private for the reason `position_name_fold_grant` does. diff --git a/packages/plugins/plugin-security/src/index.ts b/packages/plugins/plugin-security/src/index.ts index c3f3df8c4c0..9f16157d910 100644 --- a/packages/plugins/plugin-security/src/index.ts +++ b/packages/plugins/plugin-security/src/index.ts @@ -111,9 +111,6 @@ export type { CapabilityPlatformNameRefusedDiagnostic, PermissionSetDeclarationUnownedDiagnostic, } from './seed-refusal-diagnostics.js'; -// [#18091] The delivery rule itself — one derivation, so a sixth refusal site -// never re-spells `logger?.warn?.(…)` and re-earns this card. -export { reportThroughSink } from './seed-refusal-sink.js'; // [ADR-0094] sys_permission_set pure-projection machinery. export { permissionSetRowFields,