From 9a12f1f794b17473ae7e3d25fbf33e73f7b14513 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 10:43:38 +0000 Subject: [PATCH 1/8] fix(security): surface refused RBAC catalog writes instead of reporting a seed of zero Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- .../src/bootstrap-builtin-positions.ts | 30 +- .../src/bootstrap-declared-capabilities.ts | 26 +- .../src/bootstrap-declared-permissions.ts | 22 +- .../src/bootstrap-declared-positions.ts | 40 ++- .../src/bootstrap-platform-admin.ts | 38 ++- .../src/per-organization-catalog.ts | 266 ++++++++++++++++++ .../src/permission-set-projection.ts | 32 ++- 7 files changed, 423 insertions(+), 31 deletions(-) diff --git a/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts b/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts index 342ecf0f4d..6d1ef819c7 100644 --- a/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts @@ -36,10 +36,13 @@ import { BUILTIN_IDENTITY_NAMES, BUILTIN_IDENTITY_METADATA, EVERYONE_POSITION, GUEST_POSITION } from '@objectstack/spec'; import { + createSeedWriteRefusals, resolveOwnOrganizationRow, rowMatchesDeclaration, seedCtx, warnOrganizationLessRows, + warnSeedWriteRefusals, + type SeedWriteRefusals, } from './per-organization-catalog.js'; /** @@ -73,11 +76,22 @@ async function tryFind(ql: any, object: string, where: any, limit = 100, organiz return Array.isArray(rows) ? rows : []; } catch { return []; } } -async function tryInsert(ql: any, object: string, data: any, organizationId?: string): Promise { - try { return await ql.insert(object, data, { context: seedCtx(organizationId) }); } catch { return null; } +// ⛔ The `catch` RECORDS before it answers — see the sibling in +// `bootstrap-declared-positions.ts`. Answering `null`/`false` alone is what +// made a refused write indistinguishable from "nothing to do". +async function tryInsert( + ql: any, object: string, data: any, organizationId?: string, refusals?: SeedWriteRefusals, +): Promise { + try { + return await ql.insert(object, data, { context: seedCtx(organizationId) }); + } catch (e) { refusals?.record(object, e); return null; } } -async function tryUpdate(ql: any, object: string, data: any, organizationId?: string): Promise { - try { await ql.update(object, data, { context: seedCtx(organizationId) }); return true; } catch { return false; } +async function tryUpdate( + ql: any, object: string, data: any, organizationId?: string, refusals?: SeedWriteRefusals, +): Promise { + try { + await ql.update(object, data, { context: seedCtx(organizationId) }); return true; + } catch (e) { refusals?.record(object, e); return false; } } interface SeedOptions { @@ -101,6 +115,8 @@ export async function bootstrapBuiltinRoles( let updated = 0; let unchanged = 0; const residue: string[] = []; + // One log per pass, not per refused row (see the sibling seeders). + const refusals = createSeedWriteRefusals(); const rows: Array<[string, { label: string; description: string }]> = [ ...BUILTIN_IDENTITY_NAMES.map((n) => [n, BUILTIN_IDENTITY_METADATA[n]] as [string, { label: string; description: string }]), ...Object.entries(AUDIENCE_ANCHOR_METADATA), @@ -121,11 +137,11 @@ export async function bootstrapBuiltinRoles( if (own?.id) { // O(changed declarations): an unchanged row costs no write at all. if (rowMatchesDeclaration(own, fields)) { unchanged += 1; continue; } - if (await tryUpdate(ql, 'sys_position', { id: own.id, ...fields }, organizationId)) updated += 1; + if (await tryUpdate(ql, 'sys_position', { id: own.id, ...fields }, organizationId, refusals)) updated += 1; } else { const created = await tryInsert(ql, 'sys_position', { id: genId('position'), name, ...fields, active: true, is_default: false, - }, organizationId); + }, organizationId, refusals); if (created) seeded += 1; } } @@ -134,6 +150,8 @@ export async function bootstrapBuiltinRoles( // writer survives for `sys_position`, so no platform bucket is declared. warnOrganizationLessRows(options.logger, 'sys_position', residue, organizationId); } + // Before the counts, so an operator reads WHY the count is zero beside it. + warnSeedWriteRefusals(options.logger, refusals, organizationId); if (seeded + updated > 0) { options.logger?.info?.('[security] built-in identity names + audience anchors seeded into sys_position', { seeded, updated, unchanged, total: rows.length, ...(organizationId ? { organization: organizationId } : {}), diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts b/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts index f970252ebd..ff8006d5c6 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts @@ -75,6 +75,11 @@ import { tryUpdate, type ProjectionLogger, } from './permission-set-projection.js'; +import { + createSeedWriteRefusals, + warnSeedWriteRefusals, + type SeedWriteRefusals, +} from './per-organization-catalog.js'; import { buildExistingByName, type ExistingByNameIndex } from './seed-name-lookup.js'; import { readDeclared } from './bootstrap-declared-permissions.js'; import { PLATFORM_CAPABILITY_NAMES } from '@objectstack/spec/security'; @@ -264,6 +269,11 @@ async function upsertPackageCapability( */ existingByName: ExistingByNameIndex, logger?: ProjectionLogger, + /** + * Collects writes the database REFUSED, so the pass reports them once + * instead of returning a zero that reads as "nothing to do". + */ + refusals?: SeedWriteRefusals, ): Promise { if (!cap?.name) return false; @@ -314,7 +324,7 @@ async function upsertPackageCapability( package_id: packageId, active: true, }; - const created = await tryInsert(ql, 'sys_capability', row); + const created = await tryInsert(ql, 'sys_capability', row, undefined, refusals); if (created) { out.seeded += 1; // [#11096] The oracle is a SNAPSHOT taken before the loop, so it cannot @@ -348,7 +358,7 @@ async function upsertPackageCapability( // `bootstrap-seed-round-trips.test.ts` exist to make impossible. if (!capabilityRecordDiffers(existing, fields)) { out.unchanged += 1; - } else if (await tryUpdate(ql, 'sys_capability', { id: existing.id, ...fields })) { + } else if (await tryUpdate(ql, 'sys_capability', { id: existing.id, ...fields }, undefined, refusals)) { out.updated += 1; } } else { @@ -365,7 +375,7 @@ async function upsertPackageCapability( // Non-curated (curated excluded above) platform row = a derived-from- // systemPermissions placeholder. The explicit declaration CLAIMS it: // upgrade to package provenance with the authored label/description/scope. - if (await tryUpdate(ql, 'sys_capability', { id: existing.id, ...fields, managed_by: 'package', package_id: packageId })) { + if (await tryUpdate(ql, 'sys_capability', { id: existing.id, ...fields, managed_by: 'package', package_id: packageId }, undefined, refusals)) { out.claimed += 1; } return true; @@ -399,6 +409,10 @@ export async function bootstrapDeclaredCapabilities( const grantorsByCapability = indexGrantors(options.permissionSets); + // One log per pass, not per refused row: a legacy platform-wide unique index + // refuses EVERY declared capability, and a line each would bury the remedy. + const refusals = createSeedWriteRefusals(); + // [#11096] ONE existence read for the whole declaration, hoisted out of the // loop below. Each declared capability used to cost its own sequential // `SELECT … WHERE name = ? LIMIT 1` — invisible on a local file database, one @@ -426,13 +440,17 @@ export async function bootstrapDeclaredCapabilities( // spec `packageId` (ADR-0086 D3) as fallback. const packageId: string | undefined = cap._packageId ?? cap.packageId ?? undefined; const grantors = grantorsByCapability.get(cap.name) ?? []; - const materialized = await upsertPackageCapability(ql, cap, packageId, out, grantors, existingByName, options.logger); + const materialized = await upsertPackageCapability(ql, cap, packageId, out, grantors, existingByName, options.logger, refusals); // [#4967 Part 1] Report the name ONLY once this pass knows a row exists for // it. Reporting it before the upsert decided anything is what let a refused // declaration suppress the derivation it needed. if (materialized) out.materializedNames.push(cap.name); } + // Before the counts, so an operator reads WHY the count is zero beside it. + // This seeder is organization-less today (see the lookup note above), so the + // report carries no organization either. + warnSeedWriteRefusals(options.logger, refusals); if (out.unreadable > 0) { // [#11096] Said ONCE with the count, like the sibling seeders: a per-name // warn on a database that is down is a log flood that buries its own diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index efad5490b1..3647e6508b 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -61,9 +61,12 @@ import { } from './seed-name-lookup.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { + createSeedWriteRefusals, resolveOwnOrganizationRow, seedCtx, warnOrganizationLessRows, + warnSeedWriteRefusals, + type SeedWriteRefusals, } from './per-organization-catalog.js'; export type { PermissionSeedOutcome } from './permission-set-projection.js'; @@ -190,6 +193,14 @@ export async function upsertPackagePermissionSet( organizationId?: string; /** Collects names whose pre-fix organization-less row is still standing. */ residue?: string[]; + /** + * Collects writes the database REFUSED, so the pass can report them once + * instead of returning a zero that reads as "nothing to do". Passed by the + * boot catalog loop; the ADR-0086 P2 publish materializer passes nothing + * and keeps its own honest outcome (`success: false` with a reason when it + * materialized nothing). + */ + refusals?: SeedWriteRefusals; }, ): Promise { const out: PermissionSeedOutcome = { seeded: 0, updated: 0, unchanged: 0, unreadable: 0, skippedEnvAuthored: 0, skippedForeign: 0 }; @@ -232,7 +243,7 @@ export async function upsertPackagePermissionSet( package_id: packageId, managed_by: 'package', }; - const created = await tryInsert(ql, 'sys_permission_set', row, organizationId); + const created = await tryInsert(ql, 'sys_permission_set', row, organizationId, opts?.refusals); if (created) { out.seeded += 1; // A batched oracle is a snapshot taken before the loop — tell it about @@ -263,7 +274,7 @@ export async function upsertPackagePermissionSet( // beautiful round-trip curve. if (!recordDiffersFromBody(existing, ps)) { out.unchanged += 1; - } else if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...permissionSetRowFields(ps) }, organizationId)) { + } else if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...permissionSetRowFields(ps) }, organizationId, opts?.refusals)) { out.updated += 1; } } else { @@ -325,13 +336,16 @@ export async function bootstrapDeclaredPermissions( // organization's own row is created regardless — the leftover is reported, // never treated as "already seeded" (#10103). const residue: string[] = []; + // One log per pass, not per refused row: a legacy platform-wide unique index + // refuses EVERY declared permission set, and a line each would bury the remedy. + const refusals = createSeedWriteRefusals(); for (const ps of sets) { if (!ps?.name) continue; // Registry provenance first (ADR-0010 `_packageId`), author-declared // spec `packageId` (ADR-0086 D3) as fallback. const packageId: string | undefined = ps._packageId ?? ps.packageId ?? undefined; - const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger, { existingByName, organizationId, residue }); + const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger, { existingByName, organizationId, residue, refusals }); out.seeded += r.seeded; out.updated += r.updated; out.unchanged += r.unchanged; @@ -349,6 +363,8 @@ export async function bootstrapDeclaredPermissions( options.platformBucketNames ?? SHIPPED_PLATFORM_BUCKET_NAMES, ); } + // Before the counts, so an operator reads WHY the count is zero beside it. + warnSeedWriteRefusals(options.logger, refusals, organizationId); if (out.unreadable > 0) { // Said once, with the count: these sets were neither seeded nor reconciled // because the record could not be READ. Silence here would read exactly diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts index a061cc1d37..26db884c76 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts @@ -26,7 +26,13 @@ */ import { buildExistingByName } from './seed-name-lookup.js'; -import { seedCtx, warnOrganizationLessRows } from './per-organization-catalog.js'; +import { + createSeedWriteRefusals, + seedCtx, + warnOrganizationLessRows, + warnSeedWriteRefusals, + type SeedWriteRefusals, +} from './per-organization-catalog.js'; function genId(prefix: string): string { const rand = Math.random().toString(36).slice(2, 10); @@ -34,11 +40,25 @@ function genId(prefix: string): string { return `${prefix}_${ts}${rand}`; } -async function tryInsert(ql: any, object: string, data: any, organizationId?: string): Promise { - try { return await ql.insert(object, data, { context: seedCtx(organizationId) }); } catch { return null; } +// ⛔ The `catch` RECORDS before it answers. Answering `null`/`false` alone is +// what made a refused INSERT indistinguishable from "nothing to do": `seeded` +// never increments, the pass returns normally, and the boot logs a successful +// seed of zero rows. The refusal log is what carries the signal past this +// frame — see `warnSeedWriteRefusals`. Still no rethrow: the pass reports and +// continues, it does not decide whether the deployment boots. +async function tryInsert( + ql: any, object: string, data: any, organizationId?: string, refusals?: SeedWriteRefusals, +): Promise { + try { + return await ql.insert(object, data, { context: seedCtx(organizationId) }); + } catch (e) { refusals?.record(object, e); return null; } } -async function tryUpdate(ql: any, object: string, data: any, organizationId?: string): Promise { - try { await ql.update(object, data, { context: seedCtx(organizationId) }); return true; } catch { return false; } +async function tryUpdate( + ql: any, object: string, data: any, organizationId?: string, refusals?: SeedWriteRefusals, +): Promise { + try { + await ql.update(object, data, { context: seedCtx(organizationId) }); return true; + } catch (e) { refusals?.record(object, e); return false; } } interface SeedOptions { @@ -124,6 +144,9 @@ export async function bootstrapDeclaredPositions( let updated = 0; let unchanged = 0; let unreadable = 0; + // One log per pass, not per refused row: a legacy platform-wide unique index + // refuses EVERY declared position, and a line each would bury the remedy. + const refusals = createSeedWriteRefusals(); for (const r of positions) { if (!r?.name) continue; const fields = positionRowFields(r); @@ -152,14 +175,14 @@ export async function bootstrapDeclaredPositions( // re-seed (#2909 T2), so they can neither cause nor suppress one. if (!positionRecordDiffers(existing, fields)) { unchanged += 1; - } else if (await tryUpdate(ql, 'sys_position', { id: existing.id, ...fields }, organizationId)) { + } else if (await tryUpdate(ql, 'sys_position', { id: existing.id, ...fields }, organizationId, refusals)) { updated += 1; } } else { const row = { id: genId('position'), name: r.name, ...fields, active: true, is_default: false, }; - const created = await tryInsert(ql, 'sys_position', row, organizationId); + const created = await tryInsert(ql, 'sys_position', row, organizationId, refusals); if (created) { seeded += 1; // The batched oracle is a snapshot taken before the loop; a name @@ -174,6 +197,9 @@ export async function bootstrapDeclaredPositions( // row any more, so every leftover here really is pre-fix residue (#11532). warnOrganizationLessRows(options.logger, 'sys_position', residue, organizationId); } + // Before the counts are reported, so an operator reads WHY the count is zero + // in the same place they read the zero. + warnSeedWriteRefusals(options.logger, refusals, organizationId); if (unreadable > 0) { // Said once, with the count — see the sibling warn in // `bootstrap-declared-permissions.ts`. diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts index d439334f60..43c5a68860 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts @@ -67,6 +67,11 @@ import { resolveTenancyPosture, } from '@objectstack/types'; import { claimSeedOwnership } from './claim-seed-ownership.js'; +import { + createSeedWriteRefusals, + warnSeedWriteRefusals, + type SeedWriteRefusals, +} from './per-organization-catalog.js'; interface BootstrapOptions { /** Logger from PluginContext. */ @@ -113,19 +118,31 @@ async function tryFind(ql: any, object: string, where: any, limit = 100): Promis } } -async function tryInsert(ql: any, object: string, data: any): Promise { +// ⛔ The `catch` RECORDS the refusal before it answers, when the caller passed +// a log to record into. Answering `null`/`false` alone is what made a refused +// write indistinguishable from "nothing to do": `seeded` never grows, the pass +// returns normally, and the boot reports a successful seed of zero rows. Still +// no rethrow — this pass reports, it does not decide whether the deployment +// boots. See `warnSeedWriteRefusals` in `per-organization-catalog.ts`. +async function tryInsert( + ql: any, object: string, data: any, refusals?: SeedWriteRefusals, +): Promise { try { return await ql.insert(object, data, { context: SYSTEM_CTX }); - } catch { + } catch (e) { + refusals?.record(object, e); return null; } } -async function tryUpdate(ql: any, object: string, data: any): Promise { +async function tryUpdate( + ql: any, object: string, data: any, refusals?: SeedWriteRefusals, +): Promise { try { await ql.update(object, data, { context: SYSTEM_CTX }); return true; - } catch { + } catch (e) { + refusals?.record(object, e); return false; } } @@ -250,6 +267,9 @@ export async function bootstrapPlatformAdmin( const seeded: Record = {}; let resynced = 0; let resyncSkipped = 0; + // One log per pass, not per refused row: a legacy platform-wide unique index + // refuses EVERY default permission set, and a line each would bury the remedy. + const refusals = createSeedWriteRefusals(); for (const ps of bootstrapPermissionSets) { if (!ps.name) continue; const existing = await tryFind(ql, 'sys_permission_set', { name: ps.name }, 1); @@ -263,7 +283,7 @@ export async function bootstrapPlatformAdmin( // platform still owns. if (options.resync) { if (!row.managed_by || row.managed_by === 'platform') { - if (await tryUpdate(ql, 'sys_permission_set', { id: row.id, ...platformOwnedFields(ps) })) { + if (await tryUpdate(ql, 'sys_permission_set', { id: row.id, ...platformOwnedFields(ps) }, refusals)) { resynced += 1; } } else { @@ -297,11 +317,17 @@ export async function bootstrapPlatformAdmin( // the flag for. Matches `bootstrap-builtin-positions.ts` and // `bootstrap-system-capabilities.ts`, which already stamp `'platform'`. managed_by: 'platform', - }); + }, refusals); if (created?.id) seeded[ps.name] = created.id; else if (created) seeded[ps.name] = id; } + // Reported HERE rather than at function end: every `return` below this point + // is an early exit of the PROMOTION half, and the catalog seed above is + // finished either way. Placing it at the end would make the diagnosis + // conditional on how promotion happened to resolve. + warnSeedWriteRefusals(logger, refusals); + const seededCount = Object.keys(seeded).length; // [#11532] Under a walled posture these rows are organization-less BY RULING // (#10103, 2026-08-20) and unreadable through the wall, and the catalog pass diff --git a/packages/plugins/plugin-security/src/per-organization-catalog.ts b/packages/plugins/plugin-security/src/per-organization-catalog.ts index 413bf266e7..d9c2bc8785 100644 --- a/packages/plugins/plugin-security/src/per-organization-catalog.ts +++ b/packages/plugins/plugin-security/src/per-organization-catalog.ts @@ -77,6 +77,13 @@ */ import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security'; +// The ONE named answer to "is this driver error a unique-constraint +// violation?", and to "which column conflicted". Both are the shipped, +// cross-dialect, measured predicates in `@objectstack/types` — the same pair +// `packages/objectql/src/engine.ts` imports — never a local `23505` / +// `ER_DUP_ENTRY` regex, which is the four-mutually-different-answers defect +// that module was written to retire. +import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; export type SeedLogger = { info?: (m: string, meta?: Record) => void; @@ -313,3 +320,262 @@ export function rowMatchesDeclaration(row: any, fields: Record) } return true; } + +/* ------------------------------------------------------------------------- * + * Refused catalog writes — the loud half of a seed that landed nothing + * ------------------------------------------------------------------------- */ + +/** + * Why a catalog write was refused, as far as a seeder can honestly tell. + * + * Two classes, kept apart on purpose. A `unique-violation` is a DEPLOYMENT + * SCHEMA defect with a migrate remedy; anything else is not, and that remedy + * does not apply to it. Folding the second into the first would send an + * operator to `os migrate` for a failure no migration can touch — the same + * "a confident wrong answer is worse than no answer" reasoning + * `uniqueViolationColumn` is built on — so `other` is reported as its own line + * and never silently relabelled. + */ +export type SeedWriteRefusalClass = 'unique-violation' | 'other'; + +/** One aggregated line's worth of refusals: one object, one class, one pass. */ +export interface SeedWriteRefusalReport { + object: string; + class: SeedWriteRefusalClass; + /** How many writes this pass had refused for this object and class. */ + count: number; + /** + * The driver's own `code`/`errno` values, de-duplicated and sorted. + * Machine constants only — see {@link createSeedWriteRefusals}. + */ + driverCodes: string[]; + /** + * The conflicting COLUMN, when the dialect determinably named one. Usually + * empty: most dialects name an index instead, and the shipped extractor + * answers `undefined` there rather than reading a column out of an index + * name (maintainer ruling, 2026-08-08). + */ + columns: string[]; +} + +/** + * The refusals ONE seeding pass accumulated, so the pass can report them once. + * + * Aggregate-then-warn, for the same reason {@link warnOrganizationLessRows} + * aggregates: the catalog seeds every declared position, permission set and + * capability for every organization, and the failure this exists to surface + * refuses ALL of them. A line per refused row would print hundreds of entries + * into the boot log and bury the one sentence naming the remedy. One + * actionable line per object per class per pass. + */ +export interface SeedWriteRefusals { + /** + * Record one refused write. + * + * Never throws. A reporter that can fail is a reporter that turns a degraded + * seed into a broken boot, which is precisely the behaviour change this + * repair does not make. + */ + record(object: string, error: unknown): void; + /** How many writes were refused across every object in this pass. */ + readonly total: number; + /** One entry per (object, class) that actually saw a refusal, object-sorted. */ + report(): SeedWriteRefusalReport[]; +} + +/** + * The code channel is read from `code`/`errno` only, bounded down `cause`. + * + * Bounded on purpose. `code`/`errno` carry machine constants — `ER_DUP_ENTRY`, + * `23505`, `SQLITE_CONSTRAINT_UNIQUE` — and never a caller's value, which is + * what makes them safe to print into a server log. The MESSAGE channel is the + * one a SQL driver prefixes with the fully bound statement, and nothing here + * reads it. A "code" longer than this is not a code but something else wearing + * the field's name, and is dropped rather than printed. + */ +const MAX_DRIVER_CODE_LENGTH = 64; +const MAX_REPORTED_DRIVER_CODES = 4; +const MAX_REPORTED_COLUMNS = 8; +const MAX_CODE_CAUSE_DEPTH = 4; + +function collectDriverCodes(error: unknown, into: Set, depth = 0): void { + if (error === null || error === undefined || typeof error !== 'object') return; + if (depth > MAX_CODE_CAUSE_DEPTH) return; + const err = error as { code?: unknown; errno?: unknown; cause?: unknown }; + for (const channel of [err.code, err.errno]) { + if (typeof channel === 'number' && Number.isFinite(channel)) { + into.add(String(channel)); + } else if ( + typeof channel === 'string' && + channel !== '' && + channel.length <= MAX_DRIVER_CODE_LENGTH + ) { + into.add(channel); + } + } + collectDriverCodes(err.cause, into, depth + 1); +} + +interface RefusalBucket { + count: number; + codes: Set; + columns: Set; +} + +/** Start a fresh refusal log for one seeding pass. */ +export function createSeedWriteRefusals(): SeedWriteRefusals { + const byObject = new Map>(); + let total = 0; + return { + record(object: string, error: unknown): void { + // The classification is the SHIPPED predicate's, never a local regex. + // `isUniqueViolationError` reads `code`, `errno` and an allowlist of + // measured violation phrasings across the three dialects we ship, and + // answers `false` for everything it does not recognise — including the + // absence sentences that contain the very words "unique constraint". + const klass: SeedWriteRefusalClass = isUniqueViolationError(error) + ? 'unique-violation' + : 'other'; + let entry = byObject.get(object); + if (!entry) { + entry = new Map(); + byObject.set(object, entry); + } + let bucket = entry.get(klass); + if (!bucket) { + bucket = { count: 0, codes: new Set(), columns: new Set() }; + entry.set(klass, bucket); + } + bucket.count += 1; + total += 1; + collectDriverCodes(error, bucket.codes); + // `undefined` whenever the dialect named an INDEX rather than a column, + // which is the usual answer for this failure. That is the shipped + // contract and it is respected here: an absent column is simply not + // printed, never replaced with a guess derived from an index name. + const column = uniqueViolationColumn(error); + if (typeof column === 'string' && column !== '') bucket.columns.add(column); + }, + get total() { + return total; + }, + report(): SeedWriteRefusalReport[] { + const out: SeedWriteRefusalReport[] = []; + const objects = [...byObject.keys()].sort(); + for (const object of objects) { + for (const klass of ['unique-violation', 'other'] as const) { + const bucket = byObject.get(object)?.get(klass); + if (!bucket || bucket.count === 0) continue; + out.push({ + object, + class: klass, + count: bucket.count, + driverCodes: [...bucket.codes].sort().slice(0, MAX_REPORTED_DRIVER_CODES), + columns: [...bucket.columns].sort().slice(0, MAX_REPORTED_COLUMNS), + }); + } + } + return out; + }, + }; +} + +/** + * Report a pass's refused catalog writes — once per object per class. + * + * ## The failure this closes + * + * The catalog seeders answer a refused write with `null`/`false`, which is + * indistinguishable from "nothing to do": the `seeded` counter simply never + * increments and the pass returns normally. On a deployment still enforcing a + * PLATFORM-WIDE unique index on the name column from before per-organization + * materialization, EVERY per-organization insert is refused that way, and the + * boot log reads as a successful seed of zero rows — which is how a deployed + * plane ran for weeks with an empty catalog and a clean log. + * + * The outer handler on the organization-creation hook does not catch this and + * structurally cannot: the refusal is converted to `null` three call layers + * below it, so its `await` resolves normally and it logs "RBAC catalog seeded" + * at `info` over a seed of nothing. Another outer `try`/`catch` would change + * nothing. The signal has to survive the inner helper — which is what + * {@link SeedWriteRefusals} carries and what this function prints. + * + * ## Why it WARNS and does not throw + * + * A rethrow would convert a silent degradation into a boot failure on every + * deployment carrying the legacy index — a far larger behaviour change than + * the diagnosis this repair delivers, and one that decides whether a + * deployment boots at all. Loud is the ask; fatal is not. The pass still + * returns its counts, still creates every row the database accepts, and is + * still retried on the next boot and on organization creation. + * + * ## Where the colliding index is named — and why not here + * + * The identifier the driver printed (`for key '...'` on MySQL, + * `violates unique constraint "..."` on PostgreSQL, + * `UNIQUE constraint failed: index '...'` on SQLite) lives in the error + * MESSAGE, which a SQL driver builds by prefixing the fully bound statement — + * every value inlined — to the database's diagnostic. Printing that from here + * would re-open the server-log exposure `redactBoundStatement` exists to + * close. It does not need reprinting: the query engine already logs every one + * of these refusals at ERROR with that redaction applied, and the redaction + * deliberately KEEPS the identifier-bearing tail so that an operator debugging + * a duplicate can read the index name. So this line points at those entries + * instead of re-deriving them, and prints only the value-free code channel + * plus a column on the rare dialect that determinably names one. + */ +export function warnSeedWriteRefusals( + logger: SeedLogger | undefined, + refusals: SeedWriteRefusals, + organizationId?: string, +): void { + const entries = refusals.report(); + if (entries.length === 0) return; + const scope = organizationId ? { organization: organizationId } : { posture: 'single' }; + + for (const entry of entries) { + const meta = { + object: entry.object, + ...scope, + refused: entry.count, + class: entry.class, + ...(entry.driverCodes.length > 0 ? { driverCodes: entry.driverCodes } : {}), + ...(entry.columns.length > 0 ? { columns: entry.columns } : {}), + }; + + if (entry.class === 'unique-violation') { + logger?.warn?.( + `[security] ${entry.count} ${entry.object} row(s) were REFUSED BY A UNIQUE CONSTRAINT ` + + `while seeding the RBAC catalog — the catalog is INCOMPLETE and this pass's "seeded" ` + + `count is a count of the rows that LANDED, not of the rows that were declared. This is ` + + `a DEPLOYMENT SCHEMA defect rather than a data one: the catalog upserts by ` + + `(name, organization_id), so a refusal means the database still enforces a ` + + `PLATFORM-WIDE unique index on the name column from before per-organization ` + + `materialization. Under that index the first organization takes every catalog name and ` + + `every organization after it is refused, which presents as an empty Setup — no ` + + `positions, no permission sets, no capabilities — under a clean boot log. The COLLIDING ` + + `INDEX is named in the query engine's "Insert operation failed" / "Update operation ` + + `failed" entries logged just before this one: those keep the driver's own identifier ` + + `(MySQL's "for key", PostgreSQL's "violates unique constraint") with the bound ` + + `statement and its values cut. Remedy: run "os migrate plan", where the legacy index is ` + + `reported as a replace_unique_index operation that swaps it for the per-organization ` + + `composite, then "os migrate apply". Until that is applied every boot re-attempts and ` + + `re-refuses these rows — nothing is lost, and nothing arrives either.`, + meta, + ); + continue; + } + + logger?.warn?.( + `[security] ${entry.count} ${entry.object} row(s) were REFUSED while seeding the RBAC ` + + `catalog for a reason that is NOT a unique-constraint violation — the catalog is ` + + `INCOMPLETE and this pass's "seeded" count under-reports what was declared. Reported as ` + + `its own class on purpose: this is NOT the legacy platform-wide-index defect, and the ` + + `"os migrate" remedy for that one does not apply here. What the database actually said is ` + + `in the query engine's "Insert operation failed" / "Update operation failed" entries ` + + `logged just before this one, with the bound statement and its values cut. Seeding is ` + + `re-attempted on the next boot and on organization creation.`, + meta, + ); + } +} diff --git a/packages/plugins/plugin-security/src/permission-set-projection.ts b/packages/plugins/plugin-security/src/permission-set-projection.ts index 3bd4fb2628..c3c71103e8 100644 --- a/packages/plugins/plugin-security/src/permission-set-projection.ts +++ b/packages/plugins/plugin-security/src/permission-set-projection.ts @@ -85,7 +85,7 @@ */ import { PermissionSetSchema } from '@objectstack/spec/security'; -import { seedCtx } from './per-organization-catalog.js'; +import { seedCtx, type SeedWriteRefusals } from './per-organization-catalog.js'; import { buildExistingByName, type ExistingByNameIndex } from './seed-name-lookup.js'; import { ENV_PROJECTION_MARKER, @@ -117,11 +117,33 @@ export async function tryFind(ql: any, object: string, where: any, limit = 100, return Array.isArray(rows) ? rows : []; } catch { return []; } } -export async function tryInsert(ql: any, object: string, data: any, organizationId?: string): Promise { - try { return await ql.insert(object, data, { context: seedCtx(organizationId) }); } catch { return null; } +/** + * ⛔ The `catch` RECORDS the refusal before it answers, when the caller passed + * a log to record into. + * + * Answering `null`/`false` alone is what made a refused write + * indistinguishable from "nothing to do": the caller's `seeded` counter never + * increments, the pass returns normally, and the boot reports a successful + * seed of zero rows. `refusals` is optional so the callers that already report + * their own outcome honestly — the package/environment door, whose publish + * answers `success: false` with a reason when it materialized nothing — are + * unchanged. It is passed by the boot CATALOG seeders, which had no such + * channel. Never a rethrow either way: these helpers report, they do not + * decide whether the deployment boots. + */ +export async function tryInsert( + ql: any, object: string, data: any, organizationId?: string, refusals?: SeedWriteRefusals, +): Promise { + try { + return await ql.insert(object, data, { context: seedCtx(organizationId) }); + } catch (e) { refusals?.record(object, e); return null; } } -export async function tryUpdate(ql: any, object: string, data: any, organizationId?: string): Promise { - try { await ql.update(object, data, { context: seedCtx(organizationId) }); return true; } catch { return false; } +export async function tryUpdate( + ql: any, object: string, data: any, organizationId?: string, refusals?: SeedWriteRefusals, +): Promise { + try { + await ql.update(object, data, { context: seedCtx(organizationId) }); return true; + } catch (e) { refusals?.record(object, e); return false; } } export interface ProjectionLogger { From ff7e829b17fc1369605b71bea55f2c8c671593cc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 10:45:56 +0000 Subject: [PATCH 2/8] test(security): pin the refused-catalog-write warning (loud, aggregated, classified) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- .../src/seed-write-refusal.test.ts | 427 ++++++++++++++++++ 1 file changed, 427 insertions(+) create mode 100644 packages/plugins/plugin-security/src/seed-write-refusal.test.ts diff --git a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts new file mode 100644 index 0000000000..2136a41177 --- /dev/null +++ b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts @@ -0,0 +1,427 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A REFUSED catalog write must be loud — the seeder must never report a + * successful seed of zero rows. + * + * # The defect these pin + * + * The catalog seeders answered a refused write with `null`/`false`, which is + * byte-for-byte the answer for "nothing to do": the `seeded` counter never + * incremented and the pass returned normally. On a deployment still enforcing a + * PLATFORM-WIDE unique index on the name column — the shape that predates + * per-organization materialization — EVERY per-organization insert is refused + * that way, so a deployed plane ran for weeks with an empty RBAC catalog under + * a clean boot log. + * + * ⭐ The outer handler on the organization-creation hook is not missing; it is + * DISARMED. `security-plugin.ts` already wraps `seedCatalogForOrganization` in + * a `try`/`catch` that warns, and it is unreachable for this failure class: the + * refusal is converted to `null` three call layers below, so the `await` + * resolves normally and the hook logs "RBAC catalog seeded" at `info` over a + * seed of nothing. Another outer `try`/`catch` fixes nothing — the signal has + * to survive the inner helper. That is what these tests pin. + * + * # What is pinned, and why each one + * + * 1. a refused INSERT produces the warning AND the pass still RETURNS — loud + * is the ask, fatal is not: a rethrow would turn a silent degradation into a + * boot failure on every deployment carrying the legacy index; + * 2. the warning is AGGREGATED — N refusals in one pass produce one line, not + * N, the same discipline `warnOrganizationLessRows` is built on (a catalog + * that refuses 400 rows must not print 400 warnings and bury the remedy); + * 3. a NON-unique-violation refusal is not silently reclassified as one — it + * gets its own line, because the migrate remedy does not apply to it and + * sending an operator to `os migrate` for a failure no migration can touch + * is a confident wrong answer; + * 4. the existing green path still reports its counts unchanged. + * + * The classification is the SHIPPED predicate's (`isUniqueViolationError` in + * `@objectstack/types`), never a local `23505` / `ER_DUP_ENTRY` regex — the + * four-mutually-different-answers defect that module was written to retire. The + * error spellings below are taken from that module's own MEASURED fixtures + * (`unique-violation-absence-sentences.test.ts`, raised on live SQLite, + * PostgreSQL 16.13 and MariaDB 10.11.14), never invented here. + */ + +import { describe, it, expect } from 'vitest'; +import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; +import { bootstrapBuiltinRoles } from './bootstrap-builtin-positions.js'; +import { + createSeedWriteRefusals, + warnSeedWriteRefusals, +} from './per-organization-catalog.js'; + +/* ------------------------------------------------------------------------- * + * Measured driver errors — spellings copied from the shipped classifier's + * own live-server fixtures, not transcribed from memory. + * ------------------------------------------------------------------------- */ + +/** MySQL/MariaDB 1062. Names an INDEX, never a column — the usual case here. */ +function mysqlDuplicateEntry(): Error & { code: string; errno: number } { + return Object.assign( + new Error("Duplicate entry 'contributor' for key 'sys_position_name_unique'"), + { code: 'ER_DUP_ENTRY', errno: 1062 }, + ); +} + +/** PostgreSQL 23505. Names a CONSTRAINT — also not a column. */ +function postgresUniqueViolation(): Error & { code: string } { + return Object.assign( + new Error('duplicate key value violates unique constraint "sys_position_name_unique"'), + { code: '23505' }, + ); +} + +/** SQLite, the one dialect that determinably names a COLUMN. */ +function sqliteUniqueViolation(): Error { + return new Error('UNIQUE constraint failed: sys_position.name'); +} + +/** NOT a unique violation. Must never be relabelled as one. */ +function connectionFailure(): Error & { code: string } { + return Object.assign(new Error('connect ECONNREFUSED 127.0.0.1:5432'), { + code: 'ECONNREFUSED', + }); +} + +/* ------------------------------------------------------------------------- * + * Doubles + * ------------------------------------------------------------------------- */ + +interface WarnLine { + message: string; + meta: Record; +} + +function makeLogger() { + const warns: WarnLine[] = []; + const infos: WarnLine[] = []; + return { + warns, + infos, + logger: { + info: (message: string, meta?: Record) => { + infos.push({ message, meta: meta ?? {} }); + }, + warn: (message: string, meta?: Record) => { + warns.push({ message, meta: meta ?? {} }); + }, + }, + }; +} + +/** + * `sys_position` double whose INSERT is vetoed the way a legacy platform-wide + * unique index vetoes it: the row never lands and the driver throws. + * + * Modelled on the double in `bootstrap-declared-positions.test.ts` — `$in` is + * supported because the seeders hoist one batched existence read out of their + * loop, and a double that answered `[]` to `$in` would report "nothing is + * seeded" and make every re-seed look like a first boot. + */ +function makeQl( + declared: any[] = [], + opts: { insertThrows?: () => unknown; updateThrows?: () => unknown } = {}, +) { + const rows: any[] = []; + return { + rows, + registry: { listItems: (type: string) => (type === 'position' ? [...declared] : []) }, + async find(object: string, q: any) { + if (object !== 'sys_position') return []; + const where = q?.where ?? {}; + return rows.filter((r) => + Object.entries(where).every(([k, v]) => { + if (v && typeof v === 'object' && !Array.isArray(v)) { + const inList = (v as any).$in; + if (Array.isArray(inList)) return inList.includes(r[k]); + throw new Error(`fake driver: unsupported operator ${Object.keys(v).join(',')}`); + } + return r[k] === v; + }), + ); + }, + async insert(object: string, data: any) { + if (opts.insertThrows) throw opts.insertThrows(); + if (object !== 'sys_position') return null; + rows.push({ ...data }); + return { id: data.id }; + }, + async update(object: string, data: any) { + if (opts.updateThrows) throw opts.updateThrows(); + if (object !== 'sys_position') return; + const r = rows.find((x) => x.id === data.id); + if (r) Object.assign(r, data); + }, + }; +} + +const THREE_POSITIONS = [ + { name: 'contributor', label: 'Contributor', description: 'Does work' }, + { name: 'reviewer', label: 'Reviewer', description: 'Reviews work' }, + { name: 'approver', label: 'Approver', description: 'Approves work' }, +]; + +/** The refusal warnings this change adds, as opposed to any pre-existing ones. */ +function refusalWarnings(warns: WarnLine[]): WarnLine[] { + return warns.filter((w) => typeof w.meta.refused === 'number'); +} + +/* ------------------------------------------------------------------------- * + * 1 — a refused INSERT is LOUD, and the pass still returns + * ------------------------------------------------------------------------- */ + +describe('a unique-violation refusal during catalog seeding is boot-visible', () => { + it('warns, and does NOT throw, when every declared position is vetoed', async () => { + const { logger, warns } = makeLogger(); + const ql = makeQl(THREE_POSITIONS, { insertThrows: mysqlDuplicateEntry }); + + // ⭐ Resolves rather than rejects. A rethrow would turn a silent + // degradation into a boot failure on every deployment carrying the legacy + // index — a behaviour change this repair deliberately does not make. + const r = await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); + + // The seed really did land nothing — the defect's precondition holds. + expect(r.seeded).toBe(0); + expect(ql.rows).toHaveLength(0); + + const refusals = refusalWarnings(warns); + expect(refusals).toHaveLength(1); + const [line] = refusals; + expect(line.meta).toMatchObject({ + object: 'sys_position', + organization: 'org_1', + refused: 3, + class: 'unique-violation', + }); + // The value-free code channel is what identifies the dialect's refusal. + expect(line.meta.driverCodes).toEqual(['1062', 'ER_DUP_ENTRY']); + // The operator is told it is a deployment-schema defect and given the remedy. + expect(line.message).toContain('REFUSED BY A UNIQUE CONSTRAINT'); + expect(line.message).toContain('os migrate plan'); + expect(line.message).toContain('os migrate apply'); + // ⭐ And it points at where the COLLIDING INDEX is named — the engine's own + // redacted entries — rather than reprinting driver text from here. + expect(line.message).toContain('COLLIDING INDEX'); + expect(line.message).toContain('Insert operation failed'); + }); + + it('never echoes the driver message or the bound statement into the warning', async () => { + const { logger, warns } = makeLogger(); + // A driver message shaped the way knex builds one: the fully bound + // statement, values inlined, then the database's own diagnostic. + const leaky = Object.assign( + new Error( + "insert into `sys_position` (`id`, `name`, `label`) values ('position_1', " + + "'contributor', 'SENSITIVE-CANARY-9f3a2b') - Duplicate entry 'contributor' " + + "for key 'sys_position_name_unique'", + ), + { code: 'ER_DUP_ENTRY', errno: 1062 }, + ); + const ql = makeQl(THREE_POSITIONS, { insertThrows: () => leaky }); + + await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); + + const serialized = JSON.stringify(refusalWarnings(warns)); + // [#8682] This is a server LOG, which is exactly the boundary the bound- + // statement redaction governs. The seeder reads the value-free `code` / + // `errno` channel and never the message channel, so a canary in the + // statement cannot reach the log through this line. + expect(serialized).not.toContain('SENSITIVE-CANARY-9f3a2b'); + expect(serialized).not.toContain('insert into'); + }); + + it('reports a refused UPDATE on the same channel as a refused INSERT', async () => { + const { logger, warns } = makeLogger(); + const ql = makeQl( + [{ name: 'contributor', label: 'Contributor v2', description: 'new text' }], + { updateThrows: postgresUniqueViolation }, + ); + ql.rows.push({ + id: 'position_existing', name: 'contributor', label: 'Contributor', description: 'old', + organization_id: 'org_1', + }); + + const r = await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); + + expect(r.updated).toBe(0); + const refusals = refusalWarnings(warns); + expect(refusals).toHaveLength(1); + expect(refusals[0].meta).toMatchObject({ refused: 1, class: 'unique-violation' }); + expect(refusals[0].meta.driverCodes).toEqual(['23505']); + }); +}); + +/* ------------------------------------------------------------------------- * + * 2 — AGGREGATED: N refusals produce ONE line + * ------------------------------------------------------------------------- */ + +describe('the refusal warning is aggregated, not one line per refused row', () => { + it('prints ONE line for 40 refusals in a single pass', async () => { + const { logger, warns } = makeLogger(); + const many = Array.from({ length: 40 }, (_, i) => ({ + name: `position_${i}`, label: `Position ${i}`, description: 'x', + })); + const ql = makeQl(many, { insertThrows: mysqlDuplicateEntry }); + + const r = await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); + + expect(r.seeded).toBe(0); + const refusals = refusalWarnings(warns); + // ⭐ ONE actionable line per object per class per pass. The alternative + // floods the boot log and buries the one sentence naming the remedy — + // the same reason `warnOrganizationLessRows` aggregates. + expect(refusals).toHaveLength(1); + // …and it still carries the true count, so the line is a report and not + // merely a sample. + expect(refusals[0].meta.refused).toBe(40); + }); + + it('aggregates the built-in position pass the same way', async () => { + const { logger, warns } = makeLogger(); + const ql = makeQl([], { insertThrows: mysqlDuplicateEntry }); + + const r = await bootstrapBuiltinRoles(ql, { logger, organizationId: 'org_1' }); + + expect(r.seeded).toBe(0); + const refusals = refusalWarnings(warns); + expect(refusals).toHaveLength(1); + expect(refusals[0].meta.refused).toBeGreaterThan(1); + expect(refusals[0].meta.class).toBe('unique-violation'); + }); +}); + +/* ------------------------------------------------------------------------- * + * 3 — a NON-unique-violation is not reclassified + * ------------------------------------------------------------------------- */ + +describe('a refusal that is not a unique violation keeps its own class', () => { + it('does not send the operator to `os migrate` for a connection failure', async () => { + const { logger, warns } = makeLogger(); + const ql = makeQl(THREE_POSITIONS, { insertThrows: connectionFailure }); + + await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); + + const refusals = refusalWarnings(warns); + expect(refusals).toHaveLength(1); + expect(refusals[0].meta).toMatchObject({ refused: 3, class: 'other' }); + expect(refusals[0].message).toContain('NOT a unique-constraint violation'); + // ⭐ The migrate remedy belongs to the OTHER class and must not appear + // here: no migration can repair a database that is unreachable, and a + // confident wrong remedy is worse than none. + expect(refusals[0].message).not.toContain('os migrate plan'); + }); + + it('separates the two classes into two lines when a pass sees both', () => { + const { logger, warns } = makeLogger(); + const refusals = createSeedWriteRefusals(); + refusals.record('sys_position', mysqlDuplicateEntry()); + refusals.record('sys_position', mysqlDuplicateEntry()); + refusals.record('sys_position', connectionFailure()); + expect(refusals.total).toBe(3); + + warnSeedWriteRefusals(logger, refusals, 'org_1'); + + expect(warns).toHaveLength(2); + const byClass = Object.fromEntries(warns.map((w) => [w.meta.class, w.meta.refused])); + expect(byClass).toEqual({ 'unique-violation': 2, other: 1 }); + }); + + it('does not read a unique violation out of an ABSENCE sentence', () => { + const { logger, warns } = makeLogger(); + const refusals = createSeedWriteRefusals(); + // The words "unique constraint" are adjacent here and the sentence says + // there is NONE (PostgreSQL 42830). The shipped predicate answers `false`; + // this file must not answer otherwise. + refusals.record( + 'sys_position', + new Error( + 'there is no unique constraint matching given keys for referenced table "sys_position"', + ), + ); + + warnSeedWriteRefusals(logger, refusals, 'org_1'); + + expect(warns).toHaveLength(1); + expect(warns[0].meta.class).toBe('other'); + }); + + it('names the conflicting COLUMN only when the dialect determinably gave one', () => { + const { logger, warns } = makeLogger(); + + const sqlite = createSeedWriteRefusals(); + sqlite.record('sys_position', sqliteUniqueViolation()); + warnSeedWriteRefusals(logger, sqlite, 'org_1'); + expect(warns[0].meta.columns).toEqual(['name']); + + warns.length = 0; + const mysql = createSeedWriteRefusals(); + mysql.record('sys_position', mysqlDuplicateEntry()); + warnSeedWriteRefusals(logger, mysql, 'org_1'); + // ⛔ MySQL's `for key '…'` names an INDEX. The shipped extractor refuses to + // read a column out of it (maintainer ruling, 2026-08-08), and this line + // prints no `columns` key rather than a plausible-looking wrong field. + expect(warns[0].meta.columns).toBeUndefined(); + expect(JSON.stringify(warns[0].meta)).not.toContain('sys_position_name_unique'); + }); + + it('reports each object on its own line', () => { + const { logger, warns } = makeLogger(); + const refusals = createSeedWriteRefusals(); + refusals.record('sys_permission_set', postgresUniqueViolation()); + refusals.record('sys_position', postgresUniqueViolation()); + + warnSeedWriteRefusals(logger, refusals, 'org_1'); + + expect(warns.map((w) => w.meta.object)).toEqual(['sys_permission_set', 'sys_position']); + }); +}); + +/* ------------------------------------------------------------------------- * + * 4 — the green path is unchanged + * ------------------------------------------------------------------------- */ + +describe('a pass that is not refused reports exactly what it did before', () => { + it('says nothing about refusals and reports its counts unchanged', async () => { + const { logger, warns } = makeLogger(); + const ql = makeQl(THREE_POSITIONS); + + const r = await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); + + expect(r).toMatchObject({ seeded: 3, updated: 0, unchanged: 0, unreadable: 0 }); + expect(ql.rows).toHaveLength(3); + // ⭐ Silence on the healthy path is load-bearing: a warning printed on + // every boot of every deployment is the false-alarm class that trains + // operators to skim exactly this channel. + expect(refusalWarnings(warns)).toHaveLength(0); + }); + + it('emits nothing at all when a pass recorded no refusal', () => { + const { logger, warns } = makeLogger(); + const refusals = createSeedWriteRefusals(); + expect(refusals.total).toBe(0); + warnSeedWriteRefusals(logger, refusals, 'org_1'); + expect(warns).toHaveLength(0); + }); + + it('survives a logger with no `warn` sink', () => { + const refusals = createSeedWriteRefusals(); + refusals.record('sys_position', mysqlDuplicateEntry()); + // Hosts do inject reduced sinks. Reporting must not become the thing that + // breaks the boot it exists to describe. + expect(() => warnSeedWriteRefusals({}, refusals, 'org_1')).not.toThrow(); + expect(() => warnSeedWriteRefusals(undefined, refusals, 'org_1')).not.toThrow(); + }); + + it('marks a `single`-posture pass as such instead of inventing an organization', () => { + const { logger, warns } = makeLogger(); + const refusals = createSeedWriteRefusals(); + refusals.record('sys_position', mysqlDuplicateEntry()); + + warnSeedWriteRefusals(logger, refusals); + + expect(warns[0].meta.posture).toBe('single'); + expect(warns[0].meta.organization).toBeUndefined(); + }); +}); From 3a65c1ad7999cb1aad8cd426d2c74a08393b8ef7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 10:48:56 +0000 Subject: [PATCH 3/8] chore(security): changeset for the refused-catalog-write repair Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- .../rbac-catalog-seed-refusal-is-loud.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 .changeset/rbac-catalog-seed-refusal-is-loud.md diff --git a/.changeset/rbac-catalog-seed-refusal-is-loud.md b/.changeset/rbac-catalog-seed-refusal-is-loud.md new file mode 100644 index 0000000000..2af7be5feb --- /dev/null +++ b/.changeset/rbac-catalog-seed-refusal-is-loud.md @@ -0,0 +1,44 @@ +--- +"@objectstack/plugin-security": patch +--- + +fix(security): a refused RBAC catalog write is now boot-visible instead of reporting a seed of zero (#12923) + +The five RBAC catalog seeders answered a refused write with `null`/`false`, +which is byte-for-byte the answer for "nothing to do": the `seeded` counter +never incremented and the pass returned normally. On a deployment still +enforcing a **platform-wide** unique index on the name column — the shape that +predates per-organization materialization — every per-organization INSERT is +refused that way, so the boot log read as a successful seed of zero rows. +Measured on a deployed plane, undetected for weeks: an empty Setup (no +positions, no permission sets, no capabilities) under a clean log. + +The outer handler was not missing, it was **disarmed**. `security-plugin.ts` +already wrapped the organization-creation seed in a `try`/`catch` that warns, +and it was unreachable for this failure class: the refusal was converted to +`null` three call layers below, so the `await` resolved normally and the hook +logged "RBAC catalog seeded" at `info` over a seed of nothing. Another outer +`try`/`catch` would fix nothing — the signal has to survive the inner helper, +which is where the change is. + +Each seeder now accumulates the writes the database refused and reports them +**once per object per class per pass**, beside its counts: + +- a **unique violation** is named as a deployment-schema defect, with the + migrate remedy (`os migrate plan` → `os migrate apply`, where the legacy + index surfaces as a `replace_unique_index` operation) and a pointer to the + query engine's own redacted `Insert operation failed` entries, which keep the + colliding index identifier; +- anything **else** gets its own line and is never relabelled as the above, + because no migration repairs it. + +Classification uses the shipped cross-dialect predicates in +`@objectstack/types` (`isUniqueViolationError` / `uniqueViolationColumn`), not +a local `23505` / `ER_DUP_ENTRY` regex. The warning prints only the value-free +`code`/`errno` channel — never the driver's message, which a SQL driver +prefixes with the fully bound statement. + +Diagnosis only: the seeders still **warn and continue**, never throw. A rethrow +would turn a silent degradation into a boot failure on every deployment +carrying the legacy index. Counts, accept/reject behaviour and the healthy-path +logs are unchanged, and a pass that refuses nothing stays silent. From d745e36af63dab7efbefb967d695ef3b58ce5d24 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 10:59:08 +0000 Subject: [PATCH 4/8] test(security): conform the seed-refusal double to the engine/where/limit contracts Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- .../src/seed-write-refusal.test.ts | 24 +++++++++++++++---- scripts/engine-double-contract.pinned.json | 5 ++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts index 2136a41177..199b40c182 100644 --- a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts +++ b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts @@ -45,6 +45,7 @@ */ import { describe, it, expect } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; import { bootstrapBuiltinRoles } from './bootstrap-builtin-positions.js'; import { @@ -131,8 +132,11 @@ function makeQl( async find(object: string, q: any) { if (object !== 'sys_position') return []; const where = q?.where ?? {}; - return rows.filter((r) => + const matched = rows.filter((r) => Object.entries(where).every(([k, v]) => { + // Refuse what this double does not implement, rather than reading a + // combinator as a field name and silently matching nothing. + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); if (v && typeof v === 'object' && !Array.isArray(v)) { const inList = (v as any).$in; if (Array.isArray(inList)) return inList.includes(r[k]); @@ -141,6 +145,10 @@ function makeQl( return r[k] === v; }), ); + // The caller's bound is applied AFTER the filter, by presence: a double + // that silently drops `limit` answers a different question than the real + // engine and hides a paging defect from every test that uses it. + return typeof q?.limit === 'number' ? matched.slice(0, q.limit) : matched; }, async insert(object: string, data: any) { if (opts.insertThrows) throw opts.insertThrows(); @@ -148,11 +156,17 @@ function makeQl( rows.push({ ...data }); return { id: data.id }; }, - async update(object: string, data: any) { + // Pinned to ObjectQL.update's own dispatch predicate: a fake looser than + // the real engine is how a dead call shape ships with its suite green. + async update(object: string, data: any, options?: any) { if (opts.updateThrows) throw opts.updateThrows(); - if (object !== 'sys_position') return; - const r = rows.find((x) => x.id === data.id); - if (r) Object.assign(r, data); + const dispatch = assertEngineUpdateDispatch(data, options); + if (object !== 'sys_position') return dispatch.kind === 'by-id' ? null : 0; + const targets = dispatch.kind === 'by-id' + ? rows.filter((r) => r.id === dispatch.id) + : rows; + for (const r of targets) Object.assign(r, data); + return dispatch.kind === 'by-id' ? (targets[0] ?? null) : targets.length; }, }; } diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index b251850736..20f755d893 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2466,6 +2466,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-security/src/seed-write-refusal.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-security/src/select-only-write-visibility.test.ts", "verb": "delete", From eddfb237ff0638909a280cb4f78b0d050023bef0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 11:44:26 +0000 Subject: [PATCH 5/8] =?UTF-8?q?fix(security):=20split=20the=20refused-cata?= =?UTF-8?q?log-write=20level=20=E2=80=94=20error=20for=20unique=20violatio?= =?UTF-8?q?ns,=20warn=20for=20the=20rest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AGENTS.md degradation rule decides this: a boot that logs 'RBAC catalog seeded' at info over zero landed rows is the shape whose answer is error. A non-unique refusal is a retrying outage and stays warn, per the same section's 'do not over-apply it'. SeedLogger gains an OPTIONAL error carrying the kernel Logger arity, and every emission routes through logSeedDurabilityFailure so the warn fallback cannot be forgotten — never logger.error?.() (silence against a reduced sink) and never (a ?? b)() (detached receiver). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- .../src/bootstrap-builtin-positions.ts | 18 +- .../src/bootstrap-declared-capabilities.ts | 4 +- .../src/bootstrap-declared-permissions.ts | 4 +- .../src/bootstrap-declared-positions.ts | 20 +- .../src/bootstrap-platform-admin.ts | 6 +- .../src/per-organization-catalog.ts | 99 +++++++- .../src/seed-write-refusal.test.ts | 219 ++++++++++++++---- 7 files changed, 307 insertions(+), 63 deletions(-) diff --git a/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts b/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts index 6d1ef819c7..9577c167dd 100644 --- a/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts @@ -41,7 +41,8 @@ import { rowMatchesDeclaration, seedCtx, warnOrganizationLessRows, - warnSeedWriteRefusals, + reportSeedWriteRefusals, + type SeedLogger, type SeedWriteRefusals, } from './per-organization-catalog.js'; @@ -95,7 +96,18 @@ async function tryUpdate( } interface SeedOptions { - logger?: { info: (m: string, meta?: Record) => void; warn: (m: string, meta?: Record) => void }; + logger?: { + info: (m: string, meta?: Record) => void; + warn: (m: string, meta?: Record) => void; + /** + * Durability channel for a catalog write that was refused — see + * {@link SeedLogger.error} for the signature and why it is optional. + * Declared here so the level this seeder reaches for is visible in its + * own options rather than only inside the reporter; absent, the report + * falls back to `warn` and is never dropped. + */ + error?: SeedLogger['error']; + }; /** * Seed THIS organization's copies. Omitted = the `single`-posture pass, the * one place an organization-less catalog row is the correct shape. @@ -151,7 +163,7 @@ export async function bootstrapBuiltinRoles( warnOrganizationLessRows(options.logger, 'sys_position', residue, organizationId); } // Before the counts, so an operator reads WHY the count is zero beside it. - warnSeedWriteRefusals(options.logger, refusals, organizationId); + reportSeedWriteRefusals(options.logger, refusals, organizationId); if (seeded + updated > 0) { options.logger?.info?.('[security] built-in identity names + audience anchors seeded into sys_position', { seeded, updated, unchanged, total: rows.length, ...(organizationId ? { organization: organizationId } : {}), diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts b/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts index ff8006d5c6..5960cde626 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-capabilities.ts @@ -77,7 +77,7 @@ import { } from './permission-set-projection.js'; import { createSeedWriteRefusals, - warnSeedWriteRefusals, + reportSeedWriteRefusals, type SeedWriteRefusals, } from './per-organization-catalog.js'; import { buildExistingByName, type ExistingByNameIndex } from './seed-name-lookup.js'; @@ -450,7 +450,7 @@ export async function bootstrapDeclaredCapabilities( // Before the counts, so an operator reads WHY the count is zero beside it. // This seeder is organization-less today (see the lookup note above), so the // report carries no organization either. - warnSeedWriteRefusals(options.logger, refusals); + reportSeedWriteRefusals(options.logger, refusals); if (out.unreadable > 0) { // [#11096] Said ONCE with the count, like the sibling seeders: a per-name // warn on a database that is down is a log flood that buries its own diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index 3647e6508b..f113622f11 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -65,7 +65,7 @@ import { resolveOwnOrganizationRow, seedCtx, warnOrganizationLessRows, - warnSeedWriteRefusals, + reportSeedWriteRefusals, type SeedWriteRefusals, } from './per-organization-catalog.js'; @@ -364,7 +364,7 @@ export async function bootstrapDeclaredPermissions( ); } // Before the counts, so an operator reads WHY the count is zero beside it. - warnSeedWriteRefusals(options.logger, refusals, organizationId); + reportSeedWriteRefusals(options.logger, refusals, organizationId); if (out.unreadable > 0) { // Said once, with the count: these sets were neither seeded nor reconciled // because the record could not be READ. Silence here would read exactly diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts index 26db884c76..10e2875383 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts @@ -30,7 +30,8 @@ import { createSeedWriteRefusals, seedCtx, warnOrganizationLessRows, - warnSeedWriteRefusals, + reportSeedWriteRefusals, + type SeedLogger, type SeedWriteRefusals, } from './per-organization-catalog.js'; @@ -44,7 +45,7 @@ function genId(prefix: string): string { // what made a refused INSERT indistinguishable from "nothing to do": `seeded` // never increments, the pass returns normally, and the boot logs a successful // seed of zero rows. The refusal log is what carries the signal past this -// frame — see `warnSeedWriteRefusals`. Still no rethrow: the pass reports and +// frame — see `reportSeedWriteRefusals`. Still no rethrow: the pass reports and // continues, it does not decide whether the deployment boots. async function tryInsert( ql: any, object: string, data: any, organizationId?: string, refusals?: SeedWriteRefusals, @@ -62,7 +63,18 @@ async function tryUpdate( } interface SeedOptions { - logger?: { info: (m: string, meta?: Record) => void; warn: (m: string, meta?: Record) => void }; + logger?: { + info: (m: string, meta?: Record) => void; + warn: (m: string, meta?: Record) => void; + /** + * Durability channel for a catalog write that was refused — see + * {@link SeedLogger.error} for the signature and why it is optional. + * Declared here so the level this seeder reaches for is visible in its + * own options rather than only inside the reporter; absent, the report + * falls back to `warn` and is never dropped. + */ + error?: SeedLogger['error']; + }; /** * Seed THIS organization's copies. Omitted = the `single`-posture pass, the * one place an organization-less catalog row is the correct shape. @@ -199,7 +211,7 @@ export async function bootstrapDeclaredPositions( } // Before the counts are reported, so an operator reads WHY the count is zero // in the same place they read the zero. - warnSeedWriteRefusals(options.logger, refusals, organizationId); + reportSeedWriteRefusals(options.logger, refusals, organizationId); if (unreadable > 0) { // Said once, with the count — see the sibling warn in // `bootstrap-declared-permissions.ts`. diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts index 43c5a68860..9a000dedda 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts @@ -69,7 +69,7 @@ import { import { claimSeedOwnership } from './claim-seed-ownership.js'; import { createSeedWriteRefusals, - warnSeedWriteRefusals, + reportSeedWriteRefusals, type SeedWriteRefusals, } from './per-organization-catalog.js'; @@ -123,7 +123,7 @@ async function tryFind(ql: any, object: string, where: any, limit = 100): Promis // write indistinguishable from "nothing to do": `seeded` never grows, the pass // returns normally, and the boot reports a successful seed of zero rows. Still // no rethrow — this pass reports, it does not decide whether the deployment -// boots. See `warnSeedWriteRefusals` in `per-organization-catalog.ts`. +// boots. See `reportSeedWriteRefusals` in `per-organization-catalog.ts`. async function tryInsert( ql: any, object: string, data: any, refusals?: SeedWriteRefusals, ): Promise { @@ -326,7 +326,7 @@ export async function bootstrapPlatformAdmin( // is an early exit of the PROMOTION half, and the catalog seed above is // finished either way. Placing it at the end would make the diagnosis // conditional on how promotion happened to resolve. - warnSeedWriteRefusals(logger, refusals); + reportSeedWriteRefusals(logger, refusals); const seededCount = Object.keys(seeded).length; // [#11532] Under a walled posture these rows are organization-less BY RULING diff --git a/packages/plugins/plugin-security/src/per-organization-catalog.ts b/packages/plugins/plugin-security/src/per-organization-catalog.ts index d9c2bc8785..59d34a2766 100644 --- a/packages/plugins/plugin-security/src/per-organization-catalog.ts +++ b/packages/plugins/plugin-security/src/per-organization-catalog.ts @@ -88,8 +88,60 @@ import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/type export type SeedLogger = { info?: (m: string, meta?: Record) => void; warn?: (m: string, meta?: Record) => void; + /** + * Durability-degradation channel (AGENTS.md "Degradation log levels"). + * A catalog write that was supposed to land and did not is an `error`, not a + * `warn`: nothing looks broken afterwards, which is exactly why it has to be + * loud. {@link reportSeedWriteRefusals} routes only the unique-violation + * class here — see its doc for why the other class stays functional. + * + * OPTIONAL, deliberately: hosts do inject reduced sinks, and forcing this + * member would foreclose them (the measured-and-rejected option in the + * sibling `ProjectionLogger`). The fallback to `warn` is therefore + * mandatory at every call site, and lives in + * {@link logSeedDurabilityFailure} so no site can forget it. + * + * Signature matches `ProjectionLogger.error` in this package and + * `Logger.error` in `@objectstack/spec/contracts` — the CAUSE is its own + * second argument, meta is third — so the kernel logger satisfies this + * as-is. Getting the arity wrong here would put the meta object in the + * error slot, where a `Logger` neither reads nor serializes it. + */ + error?: (m: string, error?: Error, meta?: Record) => void; }; +/** + * Emit one durability-degradation line, falling back to `warn` when the host + * injected a sink with no `error`. + * + * ⛔ NOT `logger?.error?.(...)`. That spelling prints NOTHING against a + * reduced sink, which would silently drop the loudest line in this module in + * order to look tidy — the failure the whole rule exists to prevent. + * + * ⛔ NOT `(logger.error ?? logger.warn)(...)` either. That evaluates to a bare + * FUNCTION and calls it with `this === undefined`; `@objectstack/core`'s + * `ObjectLogger` is a class whose `error` reaches for `this.writeErrorLike`, + * so a detached call throws. Plain-closure sinks — every double in this + * package — survive it perfectly, which is why no suite would catch it. + * Both prohibitions and this exact `if`/`else` spelling are the measured + * conclusions recorded on `SqlDriver.logDurabilityFailure`; the property-access + * call form below keeps the receiver. + * + * The `?.` on `warn` is the backstop for hosts the TYPE cannot reach (a + * plain-JS embedder, or a cast), not doubt about the declaration. + */ +function logSeedDurabilityFailure( + logger: SeedLogger | undefined, + message: string, + meta?: Record, +): void { + // No single cause: this line summarises N refusals, so the cause slot is + // `undefined` and the detail travels in meta — the same shape the sibling + // reconcile summary in `permission-set-projection.ts` uses. + if (logger?.error) logger.error(message, undefined, meta); + else logger?.warn?.(message, meta); +} + /** * How many organizations one boot-time seeding sweep enumerates. * @@ -500,7 +552,7 @@ export function createSeedWriteRefusals(): SeedWriteRefusals { * nothing. The signal has to survive the inner helper — which is what * {@link SeedWriteRefusals} carries and what this function prints. * - * ## Why it WARNS and does not throw + * ## Why it LOGS and does not throw * * A rethrow would convert a silent degradation into a boot failure on every * deployment carrying the legacy index — a far larger behaviour change than @@ -509,6 +561,36 @@ export function createSeedWriteRefusals(): SeedWriteRefusals { * returns its counts, still creates every row the database accepts, and is * still retried on the next boot and on organization creation. * + * ## The two classes take DIFFERENT levels, and the split is the rule's own + * + * AGENTS.md "Degradation log levels" decides this with one question — *after + * the degradation, does the system still look normal from the outside while + * something it claims is persisted has not actually landed?* + * + * - **`unique-violation` -> `error`.** Yes, exactly. The boot goes on to log + * "RBAC catalog seeded" at `info` over zero landed rows; nothing looks + * broken; the loss surfaces later to somebody who cannot connect it back to + * this boot. That is the #4420 accident on a different table — the durable + * suspended-run store was attached to a table that was never created, every + * write failed into a `warn` nobody read, and each restart silently dropped + * every in-flight approval while the system reported itself healthy the + * whole time. Per the rule, such a line owes both halves in its first + * sentence: the CONSEQUENCE (the catalog did not land, and the deployment + * will keep looking healthy) and the FIX (the migrate remedy). + * - **`other` -> `warn`.** No. A refusal that is not a unique violation is + * typically a plain outage — an unreachable database, a transient fault — + * which retries on the next boot and on organization creation, and which + * the next person to open Setup discovers. Escalating it would be the + * over-application the same section warns about: it is what trains everyone + * to skim `error`, and that skimming is what made #4420's `warn` unreadable + * in the first place. + * + * ⚠️ `check:durability-log-level` does NOT vouch for either choice. That gate + * is deliberately narrow: it judges a `catch` whose `try` calls an operation + * in its declared `DURABILITY_CRITICAL_CALLEES` vocabulary, and `ql.insert` is + * not in it. Its green over this file means the site is OUTSIDE the gate's + * reach — NOT MEASURED — never that the level was approved. + * * ## Where the colliding index is named — and why not here * * The identifier the driver printed (`for key '...'` on MySQL, @@ -524,7 +606,7 @@ export function createSeedWriteRefusals(): SeedWriteRefusals { * instead of re-deriving them, and prints only the value-free code channel * plus a column on the rare dialect that determinably names one. */ -export function warnSeedWriteRefusals( +export function reportSeedWriteRefusals( logger: SeedLogger | undefined, refusals: SeedWriteRefusals, organizationId?: string, @@ -544,11 +626,16 @@ export function warnSeedWriteRefusals( }; if (entry.class === 'unique-violation') { - logger?.warn?.( + // Durability channel, with the mandatory `warn` fallback — see + // `logSeedDurabilityFailure`. + logSeedDurabilityFailure( + logger, `[security] ${entry.count} ${entry.object} row(s) were REFUSED BY A UNIQUE CONSTRAINT ` + - `while seeding the RBAC catalog — the catalog is INCOMPLETE and this pass's "seeded" ` + - `count is a count of the rows that LANDED, not of the rows that were declared. This is ` + - `a DEPLOYMENT SCHEMA defect rather than a data one: the catalog upserts by ` + + `while seeding the RBAC catalog — the catalog is INCOMPLETE, this pass's "seeded" count ` + + `is a count of the rows that LANDED rather than of the rows that were declared, and ` + + `THE DEPLOYMENT WILL GO ON LOOKING HEALTHY: the boot reports a completed seed and ` + + `nothing else fails, so this line is the only notice that the catalog did not land. ` + + `It is a DEPLOYMENT SCHEMA defect rather than a data one: the catalog upserts by ` + `(name, organization_id), so a refusal means the database still enforces a ` + `PLATFORM-WIDE unique index on the name column from before per-organization ` + `materialization. Under that index the first organization takes every catalog name and ` + diff --git a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts index 199b40c182..5eed2ad213 100644 --- a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts +++ b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts @@ -50,7 +50,7 @@ import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; import { bootstrapBuiltinRoles } from './bootstrap-builtin-positions.js'; import { createSeedWriteRefusals, - warnSeedWriteRefusals, + reportSeedWriteRefusals, } from './per-organization-catalog.js'; /* ------------------------------------------------------------------------- * @@ -95,12 +95,19 @@ interface WarnLine { meta: Record; } +/** + * A FULL sink — `info`, `warn` and `error`, with `error` carrying the kernel + * `Logger` arity (`message, cause?, meta?`). The cause slot is captured too, + * so a test can prove the meta object did not land in it. + */ function makeLogger() { const warns: WarnLine[] = []; const infos: WarnLine[] = []; + const errors: Array = []; return { warns, infos, + errors, logger: { info: (message: string, meta?: Record) => { infos.push({ message, meta: meta ?? {} }); @@ -108,6 +115,26 @@ function makeLogger() { warn: (message: string, meta?: Record) => { warns.push({ message, meta: meta ?? {} }); }, + error: (message: string, cause?: Error, meta?: Record) => { + errors.push({ message, cause, meta: meta ?? {} }); + }, + }, + }; +} + +/** + * A REDUCED sink — the shape hosts legitimately inject, with no `error`. + * This is the case a bare `logger?.error?.(...)` would answer with silence. + */ +function makeWarnOnlyLogger() { + const warns: WarnLine[] = []; + return { + warns, + logger: { + info: (_m: string, _meta?: Record) => {}, + warn: (message: string, meta?: Record) => { + warns.push({ message, meta: meta ?? {} }); + }, }, }; } @@ -177,9 +204,14 @@ const THREE_POSITIONS = [ { name: 'approver', label: 'Approver', description: 'Approves work' }, ]; -/** The refusal warnings this change adds, as opposed to any pre-existing ones. */ -function refusalWarnings(warns: WarnLine[]): WarnLine[] { - return warns.filter((w) => typeof w.meta.refused === 'number'); +/** + * The refusal lines this change adds, as opposed to any pre-existing ones — + * gathered across BOTH sinks, because the two classes deliberately land on + * different channels. A test that watched only `warn` would read the + * unique-violation line's promotion to `error` as its disappearance. + */ +function refusalLines(...sinks: WarnLine[][]): WarnLine[] { + return sinks.flat().filter((w) => typeof w.meta.refused === 'number'); } /* ------------------------------------------------------------------------- * @@ -188,7 +220,7 @@ function refusalWarnings(warns: WarnLine[]): WarnLine[] { describe('a unique-violation refusal during catalog seeding is boot-visible', () => { it('warns, and does NOT throw, when every declared position is vetoed', async () => { - const { logger, warns } = makeLogger(); + const { logger, warns, errors } = makeLogger(); const ql = makeQl(THREE_POSITIONS, { insertThrows: mysqlDuplicateEntry }); // ⭐ Resolves rather than rejects. A rethrow would turn a silent @@ -200,9 +232,15 @@ describe('a unique-violation refusal during catalog seeding is boot-visible', () expect(r.seeded).toBe(0); expect(ql.rows).toHaveLength(0); - const refusals = refusalWarnings(warns); - expect(refusals).toHaveLength(1); - const [line] = refusals; + // ⭐ On the DURABILITY channel, not the functional one: the boot goes on to + // look healthy while a catalog it reported as seeded did not land. + expect(errors).toHaveLength(1); + expect(refusalLines(warns)).toHaveLength(0); + const [line] = errors; + // The cause slot stays empty and the detail travels in meta — a summary of + // N refusals has no single cause, and putting meta in the cause slot is + // where a `Logger` neither reads nor serializes it. + expect(line.cause).toBeUndefined(); expect(line.meta).toMatchObject({ object: 'sys_position', organization: 'org_1', @@ -222,7 +260,7 @@ describe('a unique-violation refusal during catalog seeding is boot-visible', () }); it('never echoes the driver message or the bound statement into the warning', async () => { - const { logger, warns } = makeLogger(); + const { logger, warns, errors } = makeLogger(); // A driver message shaped the way knex builds one: the fully bound // statement, values inlined, then the database's own diagnostic. const leaky = Object.assign( @@ -237,7 +275,7 @@ describe('a unique-violation refusal during catalog seeding is boot-visible', () await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); - const serialized = JSON.stringify(refusalWarnings(warns)); + const serialized = JSON.stringify(refusalLines(warns, errors)); // [#8682] This is a server LOG, which is exactly the boundary the bound- // statement redaction governs. The seeder reads the value-free `code` / // `errno` channel and never the message channel, so a canary in the @@ -247,7 +285,7 @@ describe('a unique-violation refusal during catalog seeding is boot-visible', () }); it('reports a refused UPDATE on the same channel as a refused INSERT', async () => { - const { logger, warns } = makeLogger(); + const { logger, warns, errors } = makeLogger(); const ql = makeQl( [{ name: 'contributor', label: 'Contributor v2', description: 'new text' }], { updateThrows: postgresUniqueViolation }, @@ -260,10 +298,11 @@ describe('a unique-violation refusal during catalog seeding is boot-visible', () const r = await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); expect(r.updated).toBe(0); - const refusals = refusalWarnings(warns); + const refusals = refusalLines(warns, errors); expect(refusals).toHaveLength(1); expect(refusals[0].meta).toMatchObject({ refused: 1, class: 'unique-violation' }); expect(refusals[0].meta.driverCodes).toEqual(['23505']); + expect(errors).toHaveLength(1); }); }); @@ -273,7 +312,7 @@ describe('a unique-violation refusal during catalog seeding is boot-visible', () describe('the refusal warning is aggregated, not one line per refused row', () => { it('prints ONE line for 40 refusals in a single pass', async () => { - const { logger, warns } = makeLogger(); + const { logger, warns, errors } = makeLogger(); const many = Array.from({ length: 40 }, (_, i) => ({ name: `position_${i}`, label: `Position ${i}`, description: 'x', })); @@ -282,7 +321,7 @@ describe('the refusal warning is aggregated, not one line per refused row', () = const r = await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); expect(r.seeded).toBe(0); - const refusals = refusalWarnings(warns); + const refusals = refusalLines(warns, errors); // ⭐ ONE actionable line per object per class per pass. The alternative // floods the boot log and buries the one sentence naming the remedy — // the same reason `warnOrganizationLessRows` aggregates. @@ -293,13 +332,13 @@ describe('the refusal warning is aggregated, not one line per refused row', () = }); it('aggregates the built-in position pass the same way', async () => { - const { logger, warns } = makeLogger(); + const { logger, warns, errors } = makeLogger(); const ql = makeQl([], { insertThrows: mysqlDuplicateEntry }); const r = await bootstrapBuiltinRoles(ql, { logger, organizationId: 'org_1' }); expect(r.seeded).toBe(0); - const refusals = refusalWarnings(warns); + const refusals = refusalLines(warns, errors); expect(refusals).toHaveLength(1); expect(refusals[0].meta.refused).toBeGreaterThan(1); expect(refusals[0].meta.class).toBe('unique-violation'); @@ -312,12 +351,16 @@ describe('the refusal warning is aggregated, not one line per refused row', () = describe('a refusal that is not a unique violation keeps its own class', () => { it('does not send the operator to `os migrate` for a connection failure', async () => { - const { logger, warns } = makeLogger(); + const { logger, warns, errors } = makeLogger(); const ql = makeQl(THREE_POSITIONS, { insertThrows: connectionFailure }); await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); - const refusals = refusalWarnings(warns); + // ⭐ FUNCTIONAL channel, deliberately. Escalating a retrying outage to + // `error` is the over-application that trains everyone to skim `error`, + // which is what made the founding incident's `warn` unreadable. + expect(errors).toHaveLength(0); + const refusals = refusalLines(warns); expect(refusals).toHaveLength(1); expect(refusals[0].meta).toMatchObject({ refused: 3, class: 'other' }); expect(refusals[0].message).toContain('NOT a unique-constraint violation'); @@ -328,18 +371,21 @@ describe('a refusal that is not a unique violation keeps its own class', () => { }); it('separates the two classes into two lines when a pass sees both', () => { - const { logger, warns } = makeLogger(); + const { logger, warns, errors } = makeLogger(); const refusals = createSeedWriteRefusals(); refusals.record('sys_position', mysqlDuplicateEntry()); refusals.record('sys_position', mysqlDuplicateEntry()); refusals.record('sys_position', connectionFailure()); expect(refusals.total).toBe(3); - warnSeedWriteRefusals(logger, refusals, 'org_1'); + reportSeedWriteRefusals(logger, refusals, 'org_1'); - expect(warns).toHaveLength(2); - const byClass = Object.fromEntries(warns.map((w) => [w.meta.class, w.meta.refused])); - expect(byClass).toEqual({ 'unique-violation': 2, other: 1 }); + // ⭐ Two lines AND two channels: durability for the class that leaves the + // deployment looking healthy, functional for the one that retries. + expect(errors).toHaveLength(1); + expect(errors[0].meta).toMatchObject({ class: 'unique-violation', refused: 2 }); + expect(warns).toHaveLength(1); + expect(warns[0].meta).toMatchObject({ class: 'other', refused: 1 }); }); it('does not read a unique violation out of an ABSENCE sentence', () => { @@ -355,40 +401,127 @@ describe('a refusal that is not a unique violation keeps its own class', () => { ), ); - warnSeedWriteRefusals(logger, refusals, 'org_1'); + reportSeedWriteRefusals(logger, refusals, 'org_1'); expect(warns).toHaveLength(1); expect(warns[0].meta.class).toBe('other'); }); it('names the conflicting COLUMN only when the dialect determinably gave one', () => { - const { logger, warns } = makeLogger(); + const { logger, errors } = makeLogger(); const sqlite = createSeedWriteRefusals(); sqlite.record('sys_position', sqliteUniqueViolation()); - warnSeedWriteRefusals(logger, sqlite, 'org_1'); - expect(warns[0].meta.columns).toEqual(['name']); + reportSeedWriteRefusals(logger, sqlite, 'org_1'); + expect(errors[0].meta.columns).toEqual(['name']); - warns.length = 0; + errors.length = 0; const mysql = createSeedWriteRefusals(); mysql.record('sys_position', mysqlDuplicateEntry()); - warnSeedWriteRefusals(logger, mysql, 'org_1'); + reportSeedWriteRefusals(logger, mysql, 'org_1'); // ⛔ MySQL's `for key '…'` names an INDEX. The shipped extractor refuses to // read a column out of it (maintainer ruling, 2026-08-08), and this line // prints no `columns` key rather than a plausible-looking wrong field. - expect(warns[0].meta.columns).toBeUndefined(); - expect(JSON.stringify(warns[0].meta)).not.toContain('sys_position_name_unique'); + expect(errors[0].meta.columns).toBeUndefined(); + expect(JSON.stringify(errors[0].meta)).not.toContain('sys_position_name_unique'); }); it('reports each object on its own line', () => { - const { logger, warns } = makeLogger(); + const { logger, errors } = makeLogger(); const refusals = createSeedWriteRefusals(); refusals.record('sys_permission_set', postgresUniqueViolation()); refusals.record('sys_position', postgresUniqueViolation()); - warnSeedWriteRefusals(logger, refusals, 'org_1'); + reportSeedWriteRefusals(logger, refusals, 'org_1'); + + expect(errors.map((w) => w.meta.object)).toEqual(['sys_permission_set', 'sys_position']); + }); +}); + +/* ------------------------------------------------------------------------- * + * 3b — the LEVEL split, and the fallback that keeps it from costing silence + * ------------------------------------------------------------------------- */ + +describe('the two classes take different log levels (AGENTS.md degradation rule)', () => { + /** + * The rule's one question: *after the degradation, does the system still + * look normal from the outside while something it claims is persisted has + * not actually landed?* For a refused catalog seed the answer is yes — the + * boot goes on to log "RBAC catalog seeded" at `info` over zero rows. + * + * This is #4420's shape on a different table: the durable suspended-run + * store was attached to a table that was never created, every write failed + * into a `warn` nobody read, and each restart silently dropped every + * in-flight approval while the system reported itself healthy throughout. + */ + it('routes a unique violation to `error` and never to `warn`', () => { + const { logger, warns, errors } = makeLogger(); + const refusals = createSeedWriteRefusals(); + refusals.record('sys_position', mysqlDuplicateEntry()); + + reportSeedWriteRefusals(logger, refusals, 'org_1'); + + expect(errors).toHaveLength(1); + expect(warns).toHaveLength(0); + expect(errors[0].meta.class).toBe('unique-violation'); + }); + + it('states the consequence AND the fix in the durability line', () => { + const { logger, errors } = makeLogger(); + const refusals = createSeedWriteRefusals(); + refusals.record('sys_position', mysqlDuplicateEntry()); + + reportSeedWriteRefusals(logger, refusals, 'org_1'); + + // ① the consequence, concretely — including that nothing else will look + // wrong, which is the half an operator cannot infer. + expect(errors[0].message).toContain('THE DEPLOYMENT WILL GO ON LOOKING HEALTHY'); + // ② the fix. + expect(errors[0].message).toContain('os migrate plan'); + }); + + it('routes a non-unique-violation refusal to `warn` and never to `error`', () => { + const { logger, warns, errors } = makeLogger(); + const refusals = createSeedWriteRefusals(); + refusals.record('sys_position', connectionFailure()); + + reportSeedWriteRefusals(logger, refusals, 'org_1'); - expect(warns.map((w) => w.meta.object)).toEqual(['sys_permission_set', 'sys_position']); + expect(warns).toHaveLength(1); + expect(errors).toHaveLength(0); + expect(warns[0].meta.class).toBe('other'); + }); + + /** + * ⭐ The case a bare `logger?.error?.(...)` fails. Hosts legitimately inject + * reduced sinks, and against one of those that spelling prints NOTHING — + * silently dropping the loudest line in this change to satisfy a matcher. + */ + it('delivers the unique-violation line through `warn` when the host injected no `error` sink', () => { + const { logger, warns } = makeWarnOnlyLogger(); + const refusals = createSeedWriteRefusals(); + refusals.record('sys_position', mysqlDuplicateEntry()); + + reportSeedWriteRefusals(logger, refusals, 'org_1'); + + // Not silence: the message arrives, whole, on the channel that exists. + expect(warns).toHaveLength(1); + expect(warns[0].meta.class).toBe('unique-violation'); + expect(warns[0].message).toContain('REFUSED BY A UNIQUE CONSTRAINT'); + expect(warns[0].message).toContain('os migrate plan'); + // …and the meta rides along rather than being dropped into an argument + // slot the reduced sink does not have. + expect(warns[0].meta.refused).toBe(1); + }); + + it('reaches the durability channel through the real seeder, not just the helper', async () => { + const { logger, warns, errors } = makeLogger(); + const ql = makeQl(THREE_POSITIONS, { insertThrows: postgresUniqueViolation }); + + await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); + + expect(errors).toHaveLength(1); + expect(refusalLines(warns)).toHaveLength(0); }); }); @@ -398,7 +531,7 @@ describe('a refusal that is not a unique violation keeps its own class', () => { describe('a pass that is not refused reports exactly what it did before', () => { it('says nothing about refusals and reports its counts unchanged', async () => { - const { logger, warns } = makeLogger(); + const { logger, warns, errors } = makeLogger(); const ql = makeQl(THREE_POSITIONS); const r = await bootstrapDeclaredPositions(ql, null, { logger, organizationId: 'org_1' }); @@ -408,14 +541,14 @@ describe('a pass that is not refused reports exactly what it did before', () => // ⭐ Silence on the healthy path is load-bearing: a warning printed on // every boot of every deployment is the false-alarm class that trains // operators to skim exactly this channel. - expect(refusalWarnings(warns)).toHaveLength(0); + expect(refusalLines(warns, errors)).toHaveLength(0); }); it('emits nothing at all when a pass recorded no refusal', () => { const { logger, warns } = makeLogger(); const refusals = createSeedWriteRefusals(); expect(refusals.total).toBe(0); - warnSeedWriteRefusals(logger, refusals, 'org_1'); + reportSeedWriteRefusals(logger, refusals, 'org_1'); expect(warns).toHaveLength(0); }); @@ -424,18 +557,18 @@ describe('a pass that is not refused reports exactly what it did before', () => refusals.record('sys_position', mysqlDuplicateEntry()); // Hosts do inject reduced sinks. Reporting must not become the thing that // breaks the boot it exists to describe. - expect(() => warnSeedWriteRefusals({}, refusals, 'org_1')).not.toThrow(); - expect(() => warnSeedWriteRefusals(undefined, refusals, 'org_1')).not.toThrow(); + expect(() => reportSeedWriteRefusals({}, refusals, 'org_1')).not.toThrow(); + expect(() => reportSeedWriteRefusals(undefined, refusals, 'org_1')).not.toThrow(); }); it('marks a `single`-posture pass as such instead of inventing an organization', () => { - const { logger, warns } = makeLogger(); + const { logger, errors } = makeLogger(); const refusals = createSeedWriteRefusals(); refusals.record('sys_position', mysqlDuplicateEntry()); - warnSeedWriteRefusals(logger, refusals); + reportSeedWriteRefusals(logger, refusals); - expect(warns[0].meta.posture).toBe('single'); - expect(warns[0].meta.organization).toBeUndefined(); + expect(errors[0].meta.posture).toBe('single'); + expect(errors[0].meta.organization).toBeUndefined(); }); }); From c9bfc548c79d7cf21d9c3a876563458c8457aaef Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 12:21:17 +0000 Subject: [PATCH 6/8] fix(security): make SeedLogger.warn non-optional so the durability fallback is guaranteed by the type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With both `warn` and `error` optional, `{}` satisfied SeedLogger and every value of the type was permitted to print nothing — a contract that permits silence, which no call-site spelling can repair. `error` stays optional so reduced sinks remain representable. Pinned by reading the declaration's AST rather than a `@ts-expect-error`: measured, zero test files in this package reach any tsc program, so an expect-error here would evaluate never. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- .../src/per-organization-catalog.ts | 18 +++- .../src/seed-write-refusal.test.ts | 94 ++++++++++++++++++- 2 files changed, 107 insertions(+), 5 deletions(-) diff --git a/packages/plugins/plugin-security/src/per-organization-catalog.ts b/packages/plugins/plugin-security/src/per-organization-catalog.ts index 59d34a2766..16868ccfc3 100644 --- a/packages/plugins/plugin-security/src/per-organization-catalog.ts +++ b/packages/plugins/plugin-security/src/per-organization-catalog.ts @@ -87,7 +87,23 @@ import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/type export type SeedLogger = { info?: (m: string, meta?: Record) => void; - warn?: (m: string, meta?: Record) => void; + /** + * The GUARANTEED channel (#9754), and NON-OPTIONAL for that reason. + * + * `error` below is optional because hosts legitimately inject reduced + * sinks — so `warn` is where a durability report degrades to, and a + * fallback that may itself be absent is not a fallback. With both optional, + * `{}` satisfied this type and every value of it was permitted to print + * NOTHING; the call site cannot repair that, only the type can. Making + * `error` required instead is the measured-and-rejected option, and a + * required `info` would not do either: a lost write reported at `info` is + * the reassuring half-truth the degradation-level rule exists to remove. + * + * ⚠️ Call sites still spell it `logger?.warn?.(…)`. That `?.` is the + * backstop for hosts the TYPE cannot reach (a plain-JS embedder, or a + * cast), not doubt about this declaration. + */ + warn: (m: string, meta?: Record) => void; /** * Durability-degradation channel (AGENTS.md "Degradation log levels"). * A catalog write that was supposed to land and did not is an `error`, not a diff --git a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts index 5eed2ad213..8327b5f018 100644 --- a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts +++ b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts @@ -44,6 +44,10 @@ * PostgreSQL 16.13 and MariaDB 10.11.14), never invented here. */ +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; import { describe, it, expect } from 'vitest'; import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; @@ -552,12 +556,13 @@ describe('a pass that is not refused reports exactly what it did before', () => expect(warns).toHaveLength(0); }); - it('survives a logger with no `warn` sink', () => { + it('survives an absent logger entirely', () => { const refusals = createSeedWriteRefusals(); refusals.record('sys_position', mysqlDuplicateEntry()); - // Hosts do inject reduced sinks. Reporting must not become the thing that - // breaks the boot it exists to describe. - expect(() => reportSeedWriteRefusals({}, refusals, 'org_1')).not.toThrow(); + // Reporting must not become the thing that breaks the boot it exists to + // describe. `{}` is deliberately NOT exercised here: since `warn` became + // non-optional it is no longer a `SeedLogger` at all — see the type pin + // below, which is where that property is asserted. expect(() => reportSeedWriteRefusals(undefined, refusals, 'org_1')).not.toThrow(); }); @@ -572,3 +577,84 @@ describe('a pass that is not refused reports exactly what it did before', () => expect(errors[0].meta.organization).toBeUndefined(); }); }); + +/* ------------------------------------------------------------------------- * + * 5 — the fallback channel is guaranteed by the TYPE, not by convention + * ------------------------------------------------------------------------- */ + +/** + * ## Why this is an AST assertion and not a `@ts-expect-error` + * + * MEASURED, not assumed: this package's `tsconfig.json` excludes + * `**\/*.test.ts`, and `tsc --noEmit --listFiles` reports **zero** + * plugin-security test files in the program (the sibling + * `tsconfig.scripts.json` covers `scripts/` only). A `@ts-expect-error` written + * here would therefore be compiled by nothing — it would evaluate never, and + * deleting it would leave every gate exactly as green. That is the phantom + * check AGENTS.md names; this pin reads the declaration itself instead, so it + * has teeth in the suite that actually runs. + * + * ## What it pins, and why that is the load-bearing property + * + * `error` is optional because hosts legitimately inject reduced sinks. That + * makes `warn` the channel a durability report DEGRADES to — and a fallback + * that may itself be absent is not a fallback. While both were optional, `{}` + * satisfied `SeedLogger` and every value of the type was permitted to print + * nothing, which no call-site spelling can repair. + * + * This survives the removal of `check:optional-error-sink-contract`, which is + * the point: the gate found the hole, but the property belongs to this module. + */ +describe('SeedLogger guarantees the channel a durability report degrades to', () => { + const HERE = dirname(fileURLToPath(import.meta.url)); + const CATALOG_SOURCE = resolve(HERE, 'per-organization-catalog.ts'); + + /** The declared members of the `SeedLogger` type alias, and their optionality. */ + function seedLoggerMembers(): Map { + const text = readFileSync(CATALOG_SOURCE, 'utf8'); + const sourceFile = ts.createSourceFile( + CATALOG_SOURCE, text, ts.ScriptTarget.Latest, /* setParentNodes */ true, + ); + let members: Map | undefined; + sourceFile.forEachChild((node) => { + if ( + ts.isTypeAliasDeclaration(node) && + node.name.text === 'SeedLogger' && + ts.isTypeLiteralNode(node.type) + ) { + members = new Map( + node.type.members + .filter(ts.isPropertySignature) + .map((m) => [(m.name as ts.Identifier).text, m.questionToken !== undefined]), + ); + } + }); + // A pin that silently stops finding its subject is worse than no pin: it + // would go green on a renamed or restructured declaration. + if (!members) throw new Error('SeedLogger type alias not found — this pin lost its subject'); + return members; + } + + it('finds the declaration it is pinning', () => { + const members = seedLoggerMembers(); + expect([...members.keys()].sort()).toEqual(['error', 'info', 'warn']); + }); + + it('declares `warn` NON-optional — so no value of the type can be silent', () => { + // ⭐ The whole property. `{}` must not be a `SeedLogger`. + expect(seedLoggerMembers().get('warn')).toBe(false); + }); + + it('keeps `error` optional — reduced sinks stay representable', () => { + // ⛔ Making `error` required is the measured-and-rejected repair: it would + // foreclose the very hosts the fallback exists for. + expect(seedLoggerMembers().get('error')).toBe(true); + }); + + it('does not let a required `info` stand in for the guarantee', () => { + // A lost write reported at `info` is the reassuring half-truth the + // degradation-level rule exists to remove, so `info` carries no guarantee + // and must stay optional. + expect(seedLoggerMembers().get('info')).toBe(true); + }); +}); From b4ca453bd97f4effa61a70fc83bf9a710c6adedd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 12:38:58 +0000 Subject: [PATCH 7/8] test(security): pin SeedLogger's guaranteed warn channel at the type level The earlier AST-reading pin used import.meta.url, which is TS1470 under this package's CommonJS target and pushed TEST_DEBT 11 -> 12 on a shrink-only ratchet. Replaced with a type-level pin that needs no filesystem: the re-measure program DOES compile test files (it lifts the tsconfig test exclusion), so a @ts-expect-error here is enforced rather than phantom. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- .../src/seed-write-refusal.test.ts | 112 ++++++++---------- 1 file changed, 51 insertions(+), 61 deletions(-) diff --git a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts index 8327b5f018..d6736cbf9b 100644 --- a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts +++ b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts @@ -44,10 +44,6 @@ * PostgreSQL 16.13 and MariaDB 10.11.14), never invented here. */ -import { readFileSync } from 'node:fs'; -import { dirname, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; -import ts from 'typescript'; import { describe, it, expect } from 'vitest'; import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; @@ -55,6 +51,7 @@ import { bootstrapBuiltinRoles } from './bootstrap-builtin-positions.js'; import { createSeedWriteRefusals, reportSeedWriteRefusals, + type SeedLogger, } from './per-organization-catalog.js'; /* ------------------------------------------------------------------------- * @@ -583,78 +580,71 @@ describe('a pass that is not refused reports exactly what it did before', () => * ------------------------------------------------------------------------- */ /** - * ## Why this is an AST assertion and not a `@ts-expect-error` - * - * MEASURED, not assumed: this package's `tsconfig.json` excludes - * `**\/*.test.ts`, and `tsc --noEmit --listFiles` reports **zero** - * plugin-security test files in the program (the sibling - * `tsconfig.scripts.json` covers `scripts/` only). A `@ts-expect-error` written - * here would therefore be compiled by nothing — it would evaluate never, and - * deleting it would leave every gate exactly as green. That is the phantom - * check AGENTS.md names; this pin reads the declaration itself instead, so it - * has teeth in the suite that actually runs. - * - * ## What it pins, and why that is the load-bearing property + * ## The property * * `error` is optional because hosts legitimately inject reduced sinks. That * makes `warn` the channel a durability report DEGRADES to — and a fallback * that may itself be absent is not a fallback. While both were optional, `{}` * satisfied `SeedLogger` and every value of the type was permitted to print - * nothing, which no call-site spelling can repair. + * nothing, which no call-site spelling can repair. Only the type can. + * + * ## Why `@ts-expect-error` really does have teeth HERE + * + * Checked rather than assumed, because a `@ts-expect-error` in a file no tsc + * program compiles is a phantom check that evaluates never: * - * This survives the removal of `check:optional-error-sink-contract`, which is - * the point: the gate found the hole, but the property belongs to this module. + * - this package's OWN `typecheck` does NOT compile it — `tsconfig.json` + * excludes `**\/*.test.ts`, and `tsc --noEmit --listFiles` reports zero + * plugin-security test files in that program; + * - but `check:type-check-coverage --re-measure` DOES. It lifts the tsconfig's + * test exclusion into a temp project and re-counts, ratcheting the result in + * `TEST_DEBT` — shrink-only, and green on this branch at the recorded count. + * + * So both directions are enforced, by that ratchet rather than by the sink + * gate: if `warn` goes back to optional the directive below stops matching an + * error and tsc reports an UNUSED `@ts-expect-error` (+1, ratchet red), and if + * `error` were made required the companion line stops compiling (+1, ratchet + * red). This survives removal of `check:optional-error-sink-contract`, which is + * the point — that gate found the hole, but the property belongs to this module. */ describe('SeedLogger guarantees the channel a durability report degrades to', () => { - const HERE = dirname(fileURLToPath(import.meta.url)); - const CATALOG_SOURCE = resolve(HERE, 'per-organization-catalog.ts'); - - /** The declared members of the `SeedLogger` type alias, and their optionality. */ - function seedLoggerMembers(): Map { - const text = readFileSync(CATALOG_SOURCE, 'utf8'); - const sourceFile = ts.createSourceFile( - CATALOG_SOURCE, text, ts.ScriptTarget.Latest, /* setParentNodes */ true, - ); - let members: Map | undefined; - sourceFile.forEachChild((node) => { - if ( - ts.isTypeAliasDeclaration(node) && - node.name.text === 'SeedLogger' && - ts.isTypeLiteralNode(node.type) - ) { - members = new Map( - node.type.members - .filter(ts.isPropertySignature) - .map((m) => [(m.name as ts.Identifier).text, m.questionToken !== undefined]), - ); - } - }); - // A pin that silently stops finding its subject is worse than no pin: it - // would go green on a renamed or restructured declaration. - if (!members) throw new Error('SeedLogger type alias not found — this pin lost its subject'); - return members; - } - - it('finds the declaration it is pinning', () => { - const members = seedLoggerMembers(); - expect([...members.keys()].sort()).toEqual(['error', 'info', 'warn']); + it('refuses a sink that has no `warn` channel', () => { + // ⭐ The whole property: `{ info }` alone must NOT be a `SeedLogger`. + // If this stops being an error, the directive becomes unused and the + // TEST_DEBT ratchet goes red on the next re-measure. + // @ts-expect-error `warn` is non-optional — a sink that can only print at + // `info`, or print nothing at all, is the contract that permits silence. + const silentSink: SeedLogger = { info: () => {} }; + expect(silentSink).toBeDefined(); }); - it('declares `warn` NON-optional — so no value of the type can be silent', () => { - // ⭐ The whole property. `{}` must not be a `SeedLogger`. - expect(seedLoggerMembers().get('warn')).toBe(false); - }); - - it('keeps `error` optional — reduced sinks stay representable', () => { + it('still admits a reduced sink that has no `error` channel', () => { // ⛔ Making `error` required is the measured-and-rejected repair: it would - // foreclose the very hosts the fallback exists for. - expect(seedLoggerMembers().get('error')).toBe(true); + // foreclose the very hosts the fallback exists for. No `@ts-expect-error` + // here on purpose — this line must COMPILE, and the day it stops is the + // day someone made `error` required. + const reducedSink: SeedLogger = { warn: () => {} }; + expect(reducedSink).toBeDefined(); + + // …and the reduced sink is not merely representable, it is SERVED: the + // durability report degrades onto it rather than going silent. + const warns: WarnLine[] = []; + const refusals = createSeedWriteRefusals(); + refusals.record('sys_position', mysqlDuplicateEntry()); + reportSeedWriteRefusals( + { warn: (message, meta) => warns.push({ message, meta: meta ?? {} }) }, + refusals, + 'org_1', + ); + expect(warns).toHaveLength(1); + expect(warns[0].meta.class).toBe('unique-violation'); }); it('does not let a required `info` stand in for the guarantee', () => { // A lost write reported at `info` is the reassuring half-truth the // degradation-level rule exists to remove, so `info` carries no guarantee - // and must stay optional. - expect(seedLoggerMembers().get('info')).toBe(true); + // and must stay optional — this line compiling is that assertion. + const noInfoSink: SeedLogger = { warn: () => {} }; + expect(noInfoSink.info).toBeUndefined(); }); }); From bbb160f8d6659a9561a6b34abf88f59adbca69be Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 12:56:52 +0000 Subject: [PATCH 8/8] test(security): pin SeedLogger's guaranteed warn channel as a runtime AST assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check:type-check-coverage refuses a @ts-expect-error in a file no tsc program the typecheck script runs compiles, and PHANTOM_PIN_DEBT is closed to new entries — it named this file when the pin was written that way. Replaced with a runtime assertion over the declaration's own AST, seeded from __dirname (import.meta is TS1470 under this package's CommonJS resolution). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- .../src/seed-write-refusal.test.ts | 109 +++++++++++------- 1 file changed, 68 insertions(+), 41 deletions(-) diff --git a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts index d6736cbf9b..c4de3cef93 100644 --- a/packages/plugins/plugin-security/src/seed-write-refusal.test.ts +++ b/packages/plugins/plugin-security/src/seed-write-refusal.test.ts @@ -44,6 +44,9 @@ * PostgreSQL 16.13 and MariaDB 10.11.14), never invented here. */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import ts from 'typescript'; import { describe, it, expect } from 'vitest'; import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; @@ -51,7 +54,6 @@ import { bootstrapBuiltinRoles } from './bootstrap-builtin-positions.js'; import { createSeedWriteRefusals, reportSeedWriteRefusals, - type SeedLogger, } from './per-organization-catalog.js'; /* ------------------------------------------------------------------------- * @@ -588,46 +590,79 @@ describe('a pass that is not refused reports exactly what it did before', () => * satisfied `SeedLogger` and every value of the type was permitted to print * nothing, which no call-site spelling can repair. Only the type can. * - * ## Why `@ts-expect-error` really does have teeth HERE + * ## Why this reads the declaration instead of using `@ts-expect-error` * - * Checked rather than assumed, because a `@ts-expect-error` in a file no tsc - * program compiles is a phantom check that evaluates never: + * Measured, twice, rather than assumed. This package's `tsconfig.json` excludes + * `**\/*.test.ts` and `tsc --noEmit --listFiles` reports ZERO plugin-security + * test files in the program its `typecheck` script runs — so a directive here + * would not be evaluated by that script. `check:type-check-coverage` refuses + * exactly that shape by name ("carries a `@ts-expect-error` directive but no + * tsc program the `typecheck` script runs compiles it … replace the pin with a + * runtime assertion", `PHANTOM_PIN_DEBT` closed to new entries), and it refused + * this file when the pin was first written that way. * - * - this package's OWN `typecheck` does NOT compile it — `tsconfig.json` - * excludes `**\/*.test.ts`, and `tsc --noEmit --listFiles` reports zero - * plugin-security test files in that program; - * - but `check:type-check-coverage --re-measure` DOES. It lifts the tsconfig's - * test exclusion into a temp project and re-counts, ratcheting the result in - * `TEST_DEBT` — shrink-only, and green on this branch at the recorded count. + * So the pin is a runtime assertion over the declaration's own AST. It survives + * removal of `check:optional-error-sink-contract`, which is the point — that + * gate found the hole, but the property belongs to this module. * - * So both directions are enforced, by that ratchet rather than by the sink - * gate: if `warn` goes back to optional the directive below stops matching an - * error and tsc reports an UNUSED `@ts-expect-error` (+1, ratchet red), and if - * `error` were made required the companion line stops compiling (+1, ratchet - * red). This survives removal of `check:optional-error-sink-contract`, which is - * the point — that gate found the hole, but the property belongs to this module. + * ⚠️ Seeded from `__dirname`, not `import.meta.url`: under `module: NodeNext` + * this package resolves as CommonJS, where `import.meta` is TS1470 and pushed + * the shrink-only TEST_DEBT ratchet from 11 to 12. */ describe('SeedLogger guarantees the channel a durability report degrades to', () => { - it('refuses a sink that has no `warn` channel', () => { - // ⭐ The whole property: `{ info }` alone must NOT be a `SeedLogger`. - // If this stops being an error, the directive becomes unused and the - // TEST_DEBT ratchet goes red on the next re-measure. - // @ts-expect-error `warn` is non-optional — a sink that can only print at - // `info`, or print nothing at all, is the contract that permits silence. - const silentSink: SeedLogger = { info: () => {} }; - expect(silentSink).toBeDefined(); + const CATALOG_SOURCE = resolve(__dirname, 'per-organization-catalog.ts'); + + /** Declared members of the `SeedLogger` type alias, mapped to their optionality. */ + function seedLoggerMembers(): Map { + const sourceFile = ts.createSourceFile( + CATALOG_SOURCE, + readFileSync(CATALOG_SOURCE, 'utf8'), + ts.ScriptTarget.Latest, + /* setParentNodes */ true, + ); + let members: Map | undefined; + sourceFile.forEachChild((node) => { + if ( + ts.isTypeAliasDeclaration(node) && + node.name.text === 'SeedLogger' && + ts.isTypeLiteralNode(node.type) + ) { + members = new Map( + node.type.members + .filter(ts.isPropertySignature) + .map((m) => [(m.name as ts.Identifier).text, m.questionToken !== undefined]), + ); + } + }); + // A pin that silently stops finding its subject is worse than no pin: it + // would go green over a renamed or restructured declaration. + if (!members) throw new Error('SeedLogger type alias not found — this pin lost its subject'); + return members; + } + + it('finds the declaration it is pinning', () => { + expect([...seedLoggerMembers().keys()].sort()).toEqual(['error', 'info', 'warn']); + }); + + it('declares `warn` NON-optional — so no value of the type can be silent', () => { + // ⭐ The whole property: `{ info }` alone, or `{}`, must not be a SeedLogger. + expect(seedLoggerMembers().get('warn')).toBe(false); }); - it('still admits a reduced sink that has no `error` channel', () => { + it('keeps `error` optional — reduced sinks stay representable', () => { // ⛔ Making `error` required is the measured-and-rejected repair: it would - // foreclose the very hosts the fallback exists for. No `@ts-expect-error` - // here on purpose — this line must COMPILE, and the day it stops is the - // day someone made `error` required. - const reducedSink: SeedLogger = { warn: () => {} }; - expect(reducedSink).toBeDefined(); - - // …and the reduced sink is not merely representable, it is SERVED: the - // durability report degrades onto it rather than going silent. + // foreclose the very hosts the fallback exists for. + expect(seedLoggerMembers().get('error')).toBe(true); + }); + + it('does not let a required `info` stand in for the guarantee', () => { + // A lost write reported at `info` is the reassuring half-truth the + // degradation-level rule exists to remove, so `info` carries no guarantee. + expect(seedLoggerMembers().get('info')).toBe(true); + }); + + it('serves the reduced sink rather than going silent', () => { + // The type-level guarantee's whole purpose, observed end to end. const warns: WarnLine[] = []; const refusals = createSeedWriteRefusals(); refusals.record('sys_position', mysqlDuplicateEntry()); @@ -639,12 +674,4 @@ describe('SeedLogger guarantees the channel a durability report degrades to', () expect(warns).toHaveLength(1); expect(warns[0].meta.class).toBe('unique-violation'); }); - - it('does not let a required `info` stand in for the guarantee', () => { - // A lost write reported at `info` is the reassuring half-truth the - // degradation-level rule exists to remove, so `info` carries no guarantee - // and must stay optional — this line compiling is that assertion. - const noInfoSink: SeedLogger = { warn: () => {} }; - expect(noInfoSink.info).toBeUndefined(); - }); });