diff --git a/.changeset/18091-seeder-refusal-diagnostics.md b/.changeset/18091-seeder-refusal-diagnostics.md new file mode 100644 index 00000000000..b31044db520 --- /dev/null +++ b/.changeset/18091-seeder-refusal-diagnostics.md @@ -0,0 +1,36 @@ +--- +"@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. + +⛔ 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. 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-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.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?.("); + }); +}); 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..9f16157d910 100644 --- a/packages/plugins/plugin-security/src/index.ts +++ b/packages/plugins/plugin-security/src/index.ts @@ -85,6 +85,32 @@ 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'; // [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); +}