From c83945d19c60b1e1f39aafa6b2b8fe688286229c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 09:47:41 +0000 Subject: [PATCH] fix(security): stop reporting the platform bucket as a "pre-fix" leftover with a remedy that recreates it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fresh walled deployment mints 8 organization-less `sys_permission_set` rows from `bootstrapPlatformAdmin` — the fifth seeder, kept outside the per-organization conversion by the 2026-08-20 ruling on #10103. The per-organization pass then warned, once per organization, that "pre-fix organization-less rows are still present" and offered "re-initialize the deployment, or adopt each row by hand". On a deployment hours old nothing was pre-fix, and re-initializing mints exactly those rows again, so the first remedy branch was a loop. The pass now separates the two classes it was conflating and gives each the remedy that fits, carrying a machine-readable `origin` beside the named rows. The seeding itself is byte-identical: the rows stay, unreaped, because PLATFORM_ADMIN is derived from an unscoped grant pointing at the `admin_full_access` row by row id. Part of #11532 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- .../walled-platform-bucket-diagnostic.md | 57 +++ .../src/bootstrap-builtin-positions.ts | 6 +- .../src/bootstrap-declared-permissions.ts | 40 +- .../src/bootstrap-declared-positions.ts | 6 +- .../src/bootstrap-platform-admin.ts | 17 + .../src/per-organization-catalog.ts | 124 ++++-- .../plugin-security/src/security-plugin.ts | 13 +- .../walled-platform-bucket-diagnostic.test.ts | 387 ++++++++++++++++++ scripts/engine-double-contract.pinned.json | 5 + 9 files changed, 623 insertions(+), 32 deletions(-) create mode 100644 .changeset/walled-platform-bucket-diagnostic.md create mode 100644 packages/plugins/plugin-security/src/walled-platform-bucket-diagnostic.test.ts diff --git a/.changeset/walled-platform-bucket-diagnostic.md b/.changeset/walled-platform-bucket-diagnostic.md new file mode 100644 index 0000000000..bc91c77b05 --- /dev/null +++ b/.changeset/walled-platform-bucket-diagnostic.md @@ -0,0 +1,57 @@ +--- +'@objectstack/plugin-security': patch +--- + +Stop the per-organization catalog pass from reporting the platform's own +permission sets as "pre-fix" leftovers with a remedy that recreates them + +On a fresh walled deployment (`OS_TENANCY_POSTURE=isolated`, three +organizations) the boot log warned, once per organization, that *"pre-fix +organization-less `sys_permission_set` rows are still present"* and offered +*"re-initialize the deployment, or adopt each row by hand"*. Both halves were +wrong there: + +- **Nothing was pre-fix.** The eight rows it named (`admin_full_access`, + `organization_admin`, `organization_admin_no_bypass`, `member_default`, + `viewer_readonly`, `mcp_agent_data_read`, `mcp_agent_data_write`, + `mcp_agent_restricted`) were minted 1.3 s earlier — before the deployment's + first organization existed — by `bootstrapPlatformAdmin`, the fifth seeder, + which the #10103 ruling deliberately left outside the per-organization + conversion. An operator on a deployment hours old was told they were carrying + legacy state they never had. +- **Its first remedy did not terminate.** Re-initializing a fresh walled + deployment mints exactly those eight rows again on the next boot, so only the + hand-adoption branch ends — and that one hands a platform-wide bucket to a + single tenant. + +The pass now separates the two classes it was conflating and reports each with +the remedy that fits, carrying a machine-readable `origin` +(`'platform-bucket'` / `'pre-fix-residue'`) beside the named rows: + +- the **platform bucket** — names an organization-less writer still seeds on + every boot — is reported as what it is, states that this organization's own + copies were created and no action is required, and says plainly that + re-initializing does *not* clear it; +- a **genuine pre-fix leftover** keeps the original wording and the original + remedy, unchanged. + +Membership is decided by name rather than by `managed_by`, because the question +the remedy turns on is "will a re-initialized deployment have this row again?" +— true for these names whatever provenance the current row carries (a +pre-#8692 install stores `'admin'` on the very same names). It falls back to the +shipped `defaultPermissionSets`, so a host that never threads the new +`platformBucketNames` option still classifies correctly; the option exists for +a host that overrode `SecurityPluginOptions.defaultPermissionSets`. + +`bootstrapPlatformAdmin` also declares what it wrote: under a walled posture it +now logs that the platform defaults were seeded *without* an organization and +that each organization's copies come from the catalog pass. The rig's boot line +read `{"seeded":8}` with nothing to indicate the rows carried no organization +at all, so the operator's first sight of them was the warning above. + +**No behaviour change to the seeding itself.** The eight rows are still minted, +still organization-less, still unreaped — that is the ruled outcome of #10103 +(2026-08-20), and `PLATFORM_ADMIN` is derived from an unscoped grant pointing at +the `admin_full_access` row *by row id*, so removing them would silently demote +every platform admin. Whether the platform bucket should be materialized per +organization remains the maintainer's open call, not this change. diff --git a/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts b/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts index 3718da5b23..342ecf0f4d 100644 --- a/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts @@ -39,7 +39,7 @@ import { resolveOwnOrganizationRow, rowMatchesDeclaration, seedCtx, - warnPreFixOrganizationLessRows, + warnOrganizationLessRows, } from './per-organization-catalog.js'; /** @@ -130,7 +130,9 @@ export async function bootstrapBuiltinRoles( } } if (organizationId) { - warnPreFixOrganizationLessRows(options.logger, 'sys_position', residue, organizationId); + // See the sibling in `bootstrap-declared-positions.ts`: no organization-less + // writer survives for `sys_position`, so no platform bucket is declared. + warnOrganizationLessRows(options.logger, 'sys_position', residue, organizationId); } if (seeded + updated > 0) { options.logger?.info?.('[security] built-in identity names + audience anchors seeded into sys_position', { diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index 3a1fc96929..efad5490b1 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -59,10 +59,11 @@ import { type ExistingByNameIndex, type ExistingLookupResult, } from './seed-name-lookup.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { resolveOwnOrganizationRow, seedCtx, - warnPreFixOrganizationLessRows, + warnOrganizationLessRows, } from './per-organization-catalog.js'; export type { PermissionSeedOutcome } from './permission-set-projection.js'; @@ -106,6 +107,26 @@ interface SeedOptions { * one place an organization-less catalog row is the correct shape. */ organizationId?: string; + /** + * [#11532] Names that `bootstrapPlatformAdmin` still seeds organization-less + * on every boot — the PLATFORM BUCKET. Supplied by the caller because the + * caller is the one holding that array (`security-plugin.ts` hands the same + * `bootstrapPermissionSets` to the platform bootstrap and to the manifest), + * so the list is exact rather than inferred from a row's provenance. + * + * An organization-less row for one of these names is NOT a pre-fix leftover + * and the pre-fix remedy is a loop for it: re-initializing the deployment + * mints it again on the next boot. See {@link warnOrganizationLessRows}. + * + * Omitted falls back to the SHIPPED `defaultPermissionSets` — which is what + * `bootstrapPlatformAdmin` seeds unless the host passed + * `SecurityPluginOptions.defaultPermissionSets`. So the classification is + * right for every shipped composition even if this option is never threaded, + * and the option exists for the one case the fallback cannot know about: a + * host that overrode the array. Pass `[]` to state that this caller's + * deployment has no organization-less writer at all. + */ + platformBucketNames?: readonly string[]; } /** @@ -263,6 +284,15 @@ export async function upsertPackagePermissionSet( return out; } +/** + * [#11532] The names the SHIPPED `bootstrapPlatformAdmin` seeds + * organization-less on every boot. Computed once from the same declaration the + * platform bootstrap iterates, so the two cannot drift within a release. + */ +const SHIPPED_PLATFORM_BUCKET_NAMES: readonly string[] = defaultPermissionSets + .map((ps) => ps.name) + .filter((n): n is string => typeof n === 'string' && n !== ''); + export async function bootstrapDeclaredPermissions( ql: any, metadataService: any, @@ -311,7 +341,13 @@ export async function bootstrapDeclaredPermissions( } if (organizationId) { - warnPreFixOrganizationLessRows(options.logger, 'sys_permission_set', residue, organizationId); + warnOrganizationLessRows( + options.logger, + 'sys_permission_set', + residue, + organizationId, + options.platformBucketNames ?? SHIPPED_PLATFORM_BUCKET_NAMES, + ); } if (out.unreadable > 0) { // Said once, with the count: these sets were neither seeded nor reconciled diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts index d79109a31b..a061cc1d37 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts @@ -26,7 +26,7 @@ */ import { buildExistingByName } from './seed-name-lookup.js'; -import { seedCtx, warnPreFixOrganizationLessRows } from './per-organization-catalog.js'; +import { seedCtx, warnOrganizationLessRows } from './per-organization-catalog.js'; function genId(prefix: string): string { const rand = Math.random().toString(36).slice(2, 10); @@ -170,7 +170,9 @@ export async function bootstrapDeclaredPositions( } } if (organizationId) { - warnPreFixOrganizationLessRows(options.logger, 'sys_position', residue, organizationId); + // No `platformBucketNames`: nothing mints an organization-less `sys_position` + // row any more, so every leftover here really is pre-fix residue (#11532). + warnOrganizationLessRows(options.logger, 'sys_position', residue, organizationId); } if (unreadable > 0) { // Said once, with the count — see the sibling warn in diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts index d7502d32b7..a9a24a3c16 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts @@ -243,6 +243,23 @@ export async function bootstrapPlatformAdmin( } 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 + // that runs next reports them once per organization. Saying so HERE is what + // stops the operator's first sight of them being a warning that calls the + // platform's own output legacy state: the fresh walled rig logged + // `seeded: 8` with nothing to indicate the rows carried no organization at + // all. Not a behaviour change — the seeding above is byte-identical. + if (seededCount > 0 && postureEnforcesWall(resolveTenancyPosture())) { + logger?.info?.( + '[security] platform default permission sets seeded WITHOUT an organization (the platform ' + + 'bucket) — ruled 2026-08-20 and unchanged: the platform-admin grant points at the ' + + 'admin_full_access row by id. Under a walled posture they are unreadable through the ' + + 'tenant wall; each organization gets its own copies from the per-organization catalog ' + + 'pass, so no principal is missing a set.', + { seeded: seededCount, names: Object.keys(seeded).sort() }, + ); + } // Attached to every return below so `os meta resync` can report the reconcile // outcome even when admin promotion short-circuits (the common dev case: a DB // that already has an admin returns `already_have_admin`). diff --git a/packages/plugins/plugin-security/src/per-organization-catalog.ts b/packages/plugins/plugin-security/src/per-organization-catalog.ts index b0d3272c48..413bf266e7 100644 --- a/packages/plugins/plugin-security/src/per-organization-catalog.ts +++ b/packages/plugins/plugin-security/src/per-organization-catalog.ts @@ -41,8 +41,20 @@ * #8617 reaped its pre-fix organization-less rows. This catalog does not, and * the difference is deliberate rather than an omission: * - * - a fresh walled deployment never mints an organization-less catalog row once - * these seeders run per organization, so there is nothing to migrate; + * - a fresh walled deployment never mints an organization-less catalog row + * FROM THESE FOUR SEEDERS once they run per organization, so there is nothing + * of theirs to migrate. It does NOT follow that a fresh walled deployment + * holds none, and this file used to claim it did (#11532). + * `bootstrapPlatformAdmin` is a FIFTH seeder, outside the four converted + * here, and it writes one organization-less `sys_permission_set` row per + * `defaultPermissionSets` entry on EVERY boot — before any organization + * exists (measured on a fresh walled rig: 8 rows, 1.3 s ahead of the first + * `sys_organization`). That is the RULED outcome rather than a leak: the + * 2026-08-20 maintainer ruling on #10103 keeps that platform bucket + * "unreaped and loudly warned about under walled posture", and PLATFORM_ADMIN + * is derived from an unscoped grant pointing at its `admin_full_access` row + * BY ROW ID, so a reap would silently demote every platform admin. The guard + * below therefore has to tell the two classes apart; * - a `single`-posture deployment is where organization-less rows are the * CORRECT shape, and the carve-out below leaves it byte-for-byte unchanged; * - the rows a reap would delete are grant TARGETS — `sys_user_position`, @@ -52,7 +64,7 @@ * because it never touched a junction table; here the junctions ARE the * grants. * - * So the pass says so instead. {@link warnPreFixOrganizationLessRows} names the + * So the pass says so instead. {@link warnOrganizationLessRows} names the * rows and names the remedy, and — this is the load-bearing half — the pass * still CREATES the organization's own copy. The failure shape it exists to * prevent is the silent no-op: a tenant-threaded pass sees the pre-fix @@ -180,37 +192,99 @@ export function resolveOwnOrganizationRow( } /** - * The loud guard that stands in place of a reap. + * The machine-readable half of the guard below: WHY this organization-less row + * is visible to a per-organization pass. The two answers take opposite + * remedies, so the classification is a field rather than something a reader has + * to infer from prose (#11532). + */ +export type OrganizationLessRowOrigin = 'platform-bucket' | 'pre-fix-residue'; + +/** + * The loud guard that stands in place of a reap — and the ONE place that tells + * the platform bucket apart from a genuine pre-fix leftover. * * Called once per pass with everything the pass found, so an operator gets ONE - * actionable line naming the affected rows rather than a warning per name. The - * remedy is named because "invalid state" with no next step is not a diagnosis: - * either re-initialize the deployment (correct while it is pre-launch, which is - * the premise this whole repair was ruled on), or adopt each row by hand by - * stamping it with the organization that should own it. - * - * The pass that emits this has ALREADY created the organization's own copies — - * the warning describes leftovers, never a refusal to seed. + * actionable line per class rather than a warning per name. + * + * ## The two classes, and why conflating them was a defect (#11532) + * + * - **`pre-fix-residue`** — a row from before the per-organization conversion. + * Nothing regenerates it, so the ruled remedy holds: re-initialize the + * deployment (correct while it is pre-launch, which is the premise this whole + * repair was ruled on), or adopt each row by hand. + * + * - **`platform-bucket`** — a name `bootstrapPlatformAdmin` still seeds + * organization-less on EVERY boot (`platformBucketNames`). Calling one of + * these "pre-fix" tells an operator they are carrying legacy state they never + * had: on the measured fresh walled rig they were minted 1.3 s before the + * first organization existed, by the very code that then warned about them. + * And the pre-fix remedy does not terminate here — re-initializing recreates + * exactly these rows on the next boot, so the only branch that ends is hand + * adoption, which is also the branch an operator is least likely to pick and + * which hands a platform-wide bucket to one tenant. + * + * Membership is decided by NAME, not by `managed_by`, because the question the + * remedy turns on is "will a re-initialized deployment have this row again?" — + * and for these names it will, whatever provenance the current row carries (a + * pre-#8692 install stores `'admin'` on the very same names). + * + * The pass that emits either warning has ALREADY created the organization's own + * copies — both describe rows beside that catalog, never a refusal to seed. */ -export function warnPreFixOrganizationLessRows( +export function warnOrganizationLessRows( logger: SeedLogger | undefined, object: string, names: string[], organizationId: string, + platformBucketNames?: Iterable, ): void { if (names.length === 0) return; - logger?.warn?.( - `[security] pre-fix organization-less ${object} rows are still present for names this ` + - `organization seeds — under a walled posture a row that belongs to no organization is ` + - `invalid state, not a platform-wide default. This organization's own rows WERE created, so ` + - `its catalog is complete; the leftovers below are readable through the driver's ` + - `compatibility arm and belong to nobody. Remedy: re-initialize the deployment, or adopt each ` + - `row by hand by stamping it with the organization that should own it. They are NOT deleted ` + - `automatically — grants (sys_user_position, sys_position_permission_set, ` + - `sys_user_permission_set) point at these row ids, so reaping them would revoke standing ` + - `access with no signal at the moment of loss.`, - { object, organization: organizationId, names: [...names].sort(), count: names.length }, - ); + const bucketNames = new Set(platformBucketNames ?? []); + const bucket = names.filter((n) => bucketNames.has(n)); + const residue = names.filter((n) => !bucketNames.has(n)); + + if (bucket.length > 0) { + logger?.warn?.( + `[security] organization-less ${object} rows for the PLATFORM BUCKET are visible to this ` + + `organization's pass. They are not leftovers from an older release: bootstrapPlatformAdmin ` + + `seeds these names without an organization on every boot, including the one that just ran, ` + + `and the ruling of 2026-08-20 keeps it that way (unreaped, reported, outside the ` + + `per-organization conversion) because the platform-admin grant points at the ` + + `admin_full_access row by id. This organization's own copies WERE created, so its catalog ` + + `is complete and no action is required. Re-initializing the deployment does NOT clear ` + + `them — the next boot mints them again. Adopting one by hand (stamping it with an ` + + `organization) does remove it from this list, but hands a platform-wide row to a single ` + + `tenant, so do that only if that is what you mean.`, + { + object, + organization: organizationId, + origin: 'platform-bucket' satisfies OrganizationLessRowOrigin, + names: [...bucket].sort(), + count: bucket.length, + }, + ); + } + + if (residue.length > 0) { + logger?.warn?.( + `[security] pre-fix organization-less ${object} rows are still present for names this ` + + `organization seeds — under a walled posture a row that belongs to no organization is ` + + `invalid state, not a platform-wide default. This organization's own rows WERE created, so ` + + `its catalog is complete; the leftovers below are readable through the driver's ` + + `compatibility arm and belong to nobody. Remedy: re-initialize the deployment, or adopt each ` + + `row by hand by stamping it with the organization that should own it. They are NOT deleted ` + + `automatically — grants (sys_user_position, sys_position_permission_set, ` + + `sys_user_permission_set) point at these row ids, so reaping them would revoke standing ` + + `access with no signal at the moment of loss.`, + { + object, + organization: organizationId, + origin: 'pre-fix-residue' satisfies OrganizationLessRowOrigin, + names: [...residue].sort(), + count: residue.length, + }, + ); + } } /** diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index da29792526..5c046522c7 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -2926,9 +2926,20 @@ export class SecurityPlugin implements Plugin { ctx.logger.warn('[security] declared-position seeding failed', { error: (e as Error).message, organization: organizationId }); } }; + // [#11532] The names `bootstrapPlatformAdmin` seeds organization-less on + // every boot. The per-organization pass sees those rows through the driver's + // compatibility arm and used to report them as PRE-FIX leftovers with a + // remedy ("re-initialize the deployment") that mints them again — on a fresh + // walled deployment the whole line was false and its first branch was a + // loop. Handed over from here because this is where the one array lives: + // the same `bootstrapPermissionSets` goes to the platform bootstrap above + // and onto the manifest as `permissions`, so the two can never disagree. + const platformBucketNames = this.bootstrapPermissionSets + .map((p) => p.name) + .filter((n): n is string => typeof n === 'string' && n !== ''); const seedCatalogPermissions = async (organizationId?: string): Promise => { try { - await bootstrapDeclaredPermissions(ql, this.metadata, { logger: ctx.logger, organizationId }); + await bootstrapDeclaredPermissions(ql, this.metadata, { logger: ctx.logger, organizationId, platformBucketNames }); } catch (e) { ctx.logger.warn('[security] declared-permission seeding failed', { error: (e as Error).message, organization: organizationId }); } diff --git a/packages/plugins/plugin-security/src/walled-platform-bucket-diagnostic.test.ts b/packages/plugins/plugin-security/src/walled-platform-bucket-diagnostic.test.ts new file mode 100644 index 0000000000..9721b5bf1a --- /dev/null +++ b/packages/plugins/plugin-security/src/walled-platform-bucket-diagnostic.test.ts @@ -0,0 +1,387 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#11532] A FRESH walled deployment mints the PLATFORM BUCKET + * organization-less — and the per-organization pass must stop calling the + * platform's own output a "pre-fix leftover" with a remedy that recreates it. + * + * ## What was measured on a real deployment + * + * A fresh boot of the walled HotCRM SaaS rig (`OS_TENANCY_POSTURE=isolated`, + * three organizations, one SQLite file) stored 50 `sys_permission_set` rows: + * 14 x 3 organizations, plus **8 organization-less** ones carrying + * `managed_by = 'platform'` / `package_id = NULL`, written 1.3 s BEFORE the + * deployment's first `sys_organization` row existed. `bootstrapPlatformAdmin` + * wrote them — it is the fifth seeder, outside the four #11121 converted, and + * its own log line names the count (`{"seeded":8,...}`). + * + * ## What is NOT in scope here, and why the minting is left alone + * + * ⛔ Routing that seeding through the per-organization pass is **ruled out of + * this landing**. #10103's maintainer ruling of 2026-08-20 (Q1/Q2, live + * decision-inbox session) closes it verbatim: + * + * > Q1's platform-defaults residue (`bootstrapPlatformAdmin`'s three sets, + * > the env-door projection) stays outside this card, unreaped and loudly + * > warned about under walled posture; whether customers ever need those + * > visible is a future small decision card, filed only when an onboarding + * > flow actually wants it. + * + * The rows are also load-bearing: PLATFORM_ADMIN is DERIVED from an unscoped + * `sys_user_permission_set` grant pointing at the organization-less + * `admin_full_access` row BY ROW ID (`resolve-authz-context.ts` section 6b), + * which is the same reason the ruled Option C refused a #8617-breadth reap. + * + * So section 1 pins the minting as CURRENT, RULED behaviour — by row identity, + * not by count, because two offsetting errors hold a count constant while the + * identity inverts. What this card repairs is section 3: the diagnostic. + * + * ## Why the diagnostic could not be measured with the diagnostic + * + * The pass's own verdict is self-falsifying — it reports the platform's output + * of 1.3 s earlier as legacy state the operator never had, and offers + * "re-initialize the deployment", which mints exactly these rows again on the + * next boot. Its output therefore cannot serve as evidence for anything, + * including that this fix worked. Every assertion below reads STORED ROWS off + * knex (past every engine-side projection) or the logger's recorded calls — + * never the pass's summary. + * + * ## Why a real engine and a real driver + * + * The defect lives where a scope the ENGINE threads meets a predicate the + * DRIVER emits, exactly as in `per-organization-catalog.test.ts`. A hand-built + * engine double implements neither, so these cases run a real `SqlDriver` on + * better-sqlite3 `:memory:` behind a real `ObjectQL`, drive the SHIPPED + * seeders, and seed from the SHIPPED `defaultPermissionSets` — the same array + * `security-plugin.ts` hands to `bootstrapPlatformAdmin` and puts on the + * manifest as `permissions`. + */ + +import { describe, it, expect, afterEach, beforeEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; + +import { bootstrapPlatformAdmin } from './bootstrap-platform-admin.js'; +import { bootstrapDeclaredPermissions } from './bootstrap-declared-permissions.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; +import { SysPermissionSet } from './objects/sys-permission-set.object.js'; +import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { SysOrganization, SysUser } from '@objectstack/platform-objects/identity'; + +const ORGS = ['org_jia', 'org_yi', 'org_bing'] as const; +const SECURITY_PACKAGE = 'com.objectstack.plugin-security'; + +/** The platform bucket's names, read off the SHIPPED declaration. */ +const PLATFORM_BUCKET_NAMES = defaultPermissionSets.map((ps) => ps.name).sort(); + +/** + * One extra declared set that the platform bootstrap does NOT seed. It is the + * positive control for the OTHER class: a genuine pre-fix organization-less + * row, for which "re-initialize the deployment" remains the correct remedy and + * must keep being offered. + */ +const ACME_SET = { name: 'acme_readonly', label: 'Acme RO', _packageId: 'com.acme.crm', objects: {} }; + +/** + * What `manifest.register({ permissions: this.bootstrapPermissionSets })` + * produces: the platform bucket's own declarations, provenance-stamped for + * plugin-security, plus one foreign package's set. + */ +const DECLARED_PERMISSIONS = [ + ...defaultPermissionSets.map((ps) => ({ ...(ps as any), _packageId: SECURITY_PACKAGE })), + ACME_SET, +]; + +function recordingLogger() { + const warns: Array<{ message: string; meta: any }> = []; + const infos: Array<{ message: string; meta: any }> = []; + return { + warns, + infos, + logger: { + info: (message: string, meta?: any) => { infos.push({ message, meta }); }, + warn: (message: string, meta?: any) => { warns.push({ message, meta }); }, + error: (_m: string, _meta?: any) => { /* the walled elevation refusal; not this card's subject */ }, + }, + }; +} + +const engines: ObjectQL[] = []; +let posture: string | undefined; + +beforeEach(() => { + posture = process.env.OS_TENANCY_POSTURE; + process.env.OS_TENANCY_POSTURE = 'isolated'; +}); + +afterEach(async () => { + if (posture === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = posture; + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(): Promise { + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), + true, + ); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.walled-platform-bucket-11532', + name: 'Walled platform bucket', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [SysPermissionSet, SysUserPermissionSet, SysOrganization, SysUser], + } as any); + await engine.syncSchemas(); + engines.push(engine); + return engine; +} + +/** + * The delegating seam `per-organization-catalog.test.ts` uses: every verb + * reaches the real ObjectQL, `update` opens with the PRODUCER's own dispatch + * predicate, and `registry` answers `readDeclared`. + */ +function withRegistry(engine: any, declared: any[] = DECLARED_PERMISSIONS): any { + return { + find: (o: string, q?: any, opt?: any) => engine.find(o, q, opt), + insert: (o: string, d: any, opt?: any) => engine.insert(o, d, opt), + update: (o: string, d: any, opt?: any) => { + assertEngineUpdateDispatch(d, opt); + return engine.update(o, d, opt); + }, + registry: { listItems: (type: string) => (type === 'permission' ? declared : []) }, + }; +} + +/** Ground truth: every stored row, straight off knex, past all tenancy. */ +async function stored(engine: ObjectQL, table = 'sys_permission_set'): Promise { + const driver: any = (engine as any).getDriver(table); + return driver.knex(table).select('*'); +} + +const orgOf = (r: any): string | null => (r.organization_id ?? null); +const orgLess = (rows: any[]) => rows.filter((r) => orgOf(r) === null); + +/** The boot order `security-plugin.ts` runs: platform bootstrap, then one pass per organization. */ +async function walledBoot(engine: ObjectQL, logger: any): Promise<{ seeded: number }> { + const ql = withRegistry(engine); + for (const org of ORGS) { + await (engine as any).insert('sys_organization', { id: org, name: org }, { context: { isSystem: true } }); + } + const report = await bootstrapPlatformAdmin(ql, defaultPermissionSets as any[], { logger }); + for (const org of ORGS) { + await bootstrapDeclaredPermissions(ql, null, { logger, organizationId: org }); + } + return { seeded: report.seeded }; +} + +/** The one warning class this card is about, whatever wording it ends up carrying. */ +const bucketWarns = (warns: Array<{ message: string; meta: any }>) => + warns.filter((w) => (w.meta?.names ?? []).some((n: string) => PLATFORM_BUCKET_NAMES.includes(n))); + +describe('[#11532] the walled platform bucket and the diagnostic that describes it', () => { + it('1. PREMISE (ruled, unchanged): a fresh walled boot mints the platform bucket organization-less — pinned by ROW IDENTITY, not count', async () => { + const engine = await boot(); + const { logger, infos } = recordingLogger(); + + const { seeded } = await walledBoot(engine, logger); + const rows = await stored(engine); + const bucket = orgLess(rows); + + // (a) IDENTITY, name by name — a count would hold constant under two + // offsetting errors while the membership inverted. + expect(bucket.map((r) => r.name).sort()).toEqual(PLATFORM_BUCKET_NAMES); + expect(seeded).toBe(PLATFORM_BUCKET_NAMES.length); + // The eight names the rig measured are named literally, so a silent change + // to the shipped declaration cannot quietly redefine what this pins. + expect(PLATFORM_BUCKET_NAMES).toEqual([ + 'admin_full_access', + 'mcp_agent_data_read', + 'mcp_agent_data_write', + 'mcp_agent_restricted', + 'member_default', + 'organization_admin', + 'organization_admin_no_bypass', + 'viewer_readonly', + ]); + + // (b) The provenance the rig measured: platform-owned, no package. + for (const row of bucket) { + expect(row.managed_by).toBe('platform'); + expect(row.package_id ?? null).toBeNull(); + } + + // (c) Every organization still holds its OWN complete copy, package-owned — + // which is why nothing is broken for a reader and why the bucket is not + // reaped. Identity again: the per-organization row is a DIFFERENT row id. + const bucketIds = new Set(bucket.map((r) => r.id)); + for (const org of ORGS) { + const own = rows.filter((r) => orgOf(r) === org); + expect(own.map((r) => r.name).sort()).toEqual([...PLATFORM_BUCKET_NAMES, ACME_SET.name].sort()); + for (const r of own) { + expect(bucketIds.has(r.id)).toBe(false); + expect(r.managed_by).toBe('package'); + } + expect(own.filter((r) => r.name === 'admin_full_access')[0].package_id).toBe(SECURITY_PACKAGE); + } + + // (d) The PRODUCER says what it wrote. The rig's boot log read + // `{"seeded":8}` with nothing to indicate the rows carried no organization, + // so the operator's first sight of them was a warning calling them legacy + // state. Under a walled posture the seeder now names the bucket itself. + const declared = infos.filter((i) => i.message.includes('the platform bucket')); + expect(declared).toHaveLength(1); + expect([...(declared[0].meta?.names ?? [])].sort()).toEqual(PLATFORM_BUCKET_NAMES); + expect(declared[0].meta?.seeded).toBe(PLATFORM_BUCKET_NAMES.length); + }); + + it('2. the per-organization sweep leaves the platform bucket ROWS untouched — same ids, same values', async () => { + const engine = await boot(); + const { logger } = recordingLogger(); + const ql = withRegistry(engine); + for (const org of ORGS) { + await (engine as any).insert('sys_organization', { id: org, name: org }, { context: { isSystem: true } }); + } + + await bootstrapPlatformAdmin(ql, defaultPermissionSets as any[], { logger }); + const before = JSON.stringify(orgLess(await stored(engine)).map((r) => ({ ...r })).sort((a, b) => `${a.id}`.localeCompare(`${b.id}`))); + + for (const org of ORGS) await bootstrapDeclaredPermissions(ql, null, { logger, organizationId: org }); + const after = JSON.stringify(orgLess(await stored(engine)).map((r) => ({ ...r })).sort((a, b) => `${a.id}`.localeCompare(`${b.id}`))); + + // Byte-equal: not reaped, not adopted, not re-stamped. The bucket is exactly + // what the ruling said it would be, and its ids are still what the unscoped + // PLATFORM_ADMIN grant points at. + expect(after).toBe(before); + }); + + it('3. THE FIX: the platform bucket is NOT reported as a pre-fix leftover, and its remedy is not the loop', async () => { + const engine = await boot(); + const { logger, warns } = recordingLogger(); + await walledBoot(engine, logger); + + const guard = bucketWarns(warns); + // Once per organization — the guard is per pass, and silence would be the + // OTHER failure (the ruling requires these rows be warned about loudly). + expect(guard).toHaveLength(ORGS.length); + + for (const org of ORGS) { + const forOrg = guard.filter((w) => w.meta?.organization === org); + expect(forOrg).toHaveLength(1); + const [w] = forOrg; + // (a) Every platform-bucket name is named — loudly, by name, as ruled. + expect([...(w.meta.names ?? [])].sort()).toEqual(PLATFORM_BUCKET_NAMES); + expect(w.meta.count).toBe(PLATFORM_BUCKET_NAMES.length); + + // (b) NOT pre-fix. On a fresh deployment there are no pre-fix rows; these + // were minted by this same boot, seconds earlier. Asserted on the + // MACHINE-READABLE classification rather than on prose, so a reworded + // message cannot quietly re-merge the two classes. + expect(w.meta.origin).toBe('platform-bucket'); + expect(w.message).toContain('PLATFORM BUCKET'); + expect(w.message).toContain('on every boot'); + expect(w.message).not.toContain('pre-fix organization-less'); + + // (c) The remedy is not the loop. "Remedy: re-initialize the deployment" + // recreates exactly these rows on the next boot, so it is not offered. + expect(w.message).not.toContain('Remedy: re-initialize the deployment'); + expect(w.message).toContain('Re-initializing the deployment does NOT clear'); + + // (d) …and the reader is told the thing that actually matters: their own + // catalog is complete, so no action is required. + expect(w.message).toContain("own copies WERE created"); + expect(w.message).toContain('no action is required'); + } + }); + + it('4. POSITIVE CONTROL: a genuine pre-fix organization-less row still gets the original wording AND the re-initialize remedy', async () => { + const engine = await boot(); + const { logger, warns } = recordingLogger(); + const ql = withRegistry(engine); + for (const org of ORGS) { + await (engine as any).insert('sys_organization', { id: org, name: org }, { context: { isSystem: true } }); + } + + // A pre-fix deployment: ONE organization-less pass through the SHIPPED + // seeder, the pre-#11121 behaviour, carrying only the foreign package's + // set. `acme_readonly` is not in the platform bucket, so its leftover is + // the real thing — a re-initialized deployment really would not have it. + await bootstrapDeclaredPermissions(withRegistry(engine, [ACME_SET]), null, { logger }); + expect(orgLess(await stored(engine)).map((r) => r.name)).toEqual([ACME_SET.name]); + + warns.length = 0; + await bootstrapPlatformAdmin(ql, defaultPermissionSets as any[], { logger }); + for (const org of ORGS) await bootstrapDeclaredPermissions(ql, null, { logger, organizationId: org }); + + const preFix = warns.filter((w) => (w.meta?.names ?? []).includes(ACME_SET.name)); + expect(preFix).toHaveLength(ORGS.length); + for (const w of preFix) { + expect(w.meta.origin).toBe('pre-fix-residue'); + expect(w.message).toContain('pre-fix organization-less'); + expect(w.message).toContain('re-initialize the deployment'); + expect(w.message).toContain('adopt'); + expect(w.message).toContain('NOT deleted'); + // The two classes are reported SEPARATELY: a pre-fix warning that also + // carried the platform bucket's names would hand the operator a remedy + // that is a loop for most of the list it prints. + expect([...(w.meta.names ?? [])]).toEqual([ACME_SET.name]); + } + }); + + it('5. `single` posture is untouched: organization-less is the correct shape there, and nothing is warned', async () => { + const engine = await boot(); + const { logger, warns, infos } = recordingLogger(); + const ql = withRegistry(engine); + process.env.OS_TENANCY_POSTURE = 'single'; + + await bootstrapPlatformAdmin(ql, defaultPermissionSets as any[], { logger }); + await bootstrapDeclaredPermissions(ql, null, { logger }); + + const rows = await stored(engine); + expect(rows.every((r) => orgOf(r) === null)).toBe(true); + expect(warns.filter((w) => (w.meta?.names ?? []).length > 0)).toEqual([]); + // …and the producer's walled-only declaration stays silent here: an + // organization-less row is the CORRECT shape under `single`. + expect(infos.filter((i) => i.message.includes('the platform bucket'))).toEqual([]); + }); + + it('6. a host that OVERRODE the default sets is classified against ITS array, not the shipped one', async () => { + const engine = await boot(); + const { logger, warns } = recordingLogger(); + const ql = withRegistry(engine); + for (const org of ORGS) { + await (engine as any).insert('sys_organization', { id: org, name: org }, { context: { isSystem: true } }); + } + + // The counterfactual `SecurityPluginOptions.defaultPermissionSets` creates: + // the organization-less writer seeds a DIFFERENT list, so `acme_readonly` + // becomes the re-minted name and the shipped eight become genuine leftovers. + await bootstrapDeclaredPermissions(withRegistry(engine, [ACME_SET]), null, { logger }); + await bootstrapPlatformAdmin(ql, defaultPermissionSets as any[], { logger }); + + warns.length = 0; + for (const org of ORGS) { + await bootstrapDeclaredPermissions(ql, null, { + logger, + organizationId: org, + platformBucketNames: [ACME_SET.name], + }); + } + + const bucket = warns.filter((w) => w.meta?.origin === 'platform-bucket'); + const preFix = warns.filter((w) => w.meta?.origin === 'pre-fix-residue'); + expect(bucket).toHaveLength(ORGS.length); + expect(preFix).toHaveLength(ORGS.length); + for (const w of bucket) expect([...(w.meta.names ?? [])]).toEqual([ACME_SET.name]); + // Positive control on the same run: the shipped eight are NOT silently + // swallowed — they move to the other class rather than disappearing. + for (const w of preFix) expect([...(w.meta.names ?? [])].sort()).toEqual(PLATFORM_BUCKET_NAMES); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 3fdc47edad..6b0ee72942 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1531,6 +1531,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-security/src/walled-platform-bucket-diagnostic.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts", "verb": "delete",