diff --git a/.changeset/15204-walled-catalog-package-refusal.md b/.changeset/15204-walled-catalog-package-refusal.md new file mode 100644 index 00000000000..43a7d02a38e --- /dev/null +++ b/.changeset/15204-walled-catalog-package-refusal.md @@ -0,0 +1,31 @@ +--- +'@objectstack/cloud-connection': minor +'@objectstack/runtime': minor +'@objectstack/objectql': minor +'@objectstack/spec': minor +--- + +Under a walled posture a package that carries security catalog items is the platform operator's: tenants are refused installing, upgrading, uninstalling or deleting it + +Clause-②: yes (narrowing) + + + +**BREAKING** accept-set narrowing, shipped as `minor` under the repo's launch-window convention for breaking changes (ADR-0131 D3: "Shared-DB multi-tenant (`group` / `isolated`): tenant admins cannot create or edit catalog items; the operator defines the catalog for every tenant (managed packages, Studio)."). + +**Why.** A package registers its positions, permission sets and capabilities environment-wide, and every organization resolves them by name. Measured on a booted `isolated` and `group` stack, with an organization owner granted `manage_metadata` inside their own organization (never the platform operator): +- the owner re-installed an operator-installed package at a new version through `install-local`, adding `manage_users` to a permission set another organization's member holds, and that member's next resolution carried `manage_users`; +- the owner's `install-local` uninstall of that package took the set away from that member; +- the owner's `DELETE /api/v1/packages/:id` of a runtime package the operator had saved a permission set into did the same. + +The existing name-conflict refusal stops only a package that shadows a name another holder has, never a write by the package that holds it. + +**FROM → TO**, on a `group` or `isolated` deployment, for a caller below the platform operator (the `PLATFORM_ADMIN` rung: the deployment's declared `OS_PLATFORM_OWNER_EMAIL` owner): + +- `POST /api/v1/marketplace/install-local` (`@objectstack/cloud-connection`) of a package whose incoming version declares a position, permission set or capability, or, on an upgrade, whose installed version does: **FROM** admitted to any `manage_metadata` holder **TO** `403 FORBIDDEN`, before the package is registered or the ledger written. The whole package is refused, since a package installs whole. The message names the posture, and `error.details.catalogItems` lists the items. +- `DELETE /api/v1/marketplace/install-local/:id` of a package whose installed version declares one: **FROM** admitted to any `manage_metadata` holder **TO** `403 FORBIDDEN`, before the ledger is touched. +- `DELETE /api/v1/packages/:id` on the runtime dispatcher, of a writable package holding stored positions or permission sets: **FROM** admitted to any `manage_metadata` holder **TO** `403 PERMISSION_DENIED`, before anything is deleted. + +**Unchanged.** A package with no catalog item installs, upgrades and uninstalls as before, for the same callers. The platform operator does all of the above as before. Under `single` (and with no `tenancy` service) nothing changes. Disabling a package is unchanged: it was measured not to change what any organization resolves. + +**The remedy.** The platform operator installs, upgrades and removes catalog-carrying packages. `@objectstack/spec` registers the change as the ADR-0087 semantic entry `walled-catalog-package-writes-retired`. Both doors ask the one predicate the catalog's other write doors ask, `walledCatalogWriteVerdict` (`@objectstack/metadata-core`), once per catalog type the package declares. `@objectstack/objectql` now exports `declaredSecurityCatalogNames`, the engine's own reading of what a manifest declares, so a package door judges the declarations the engine registers. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 382ed879727..8bba0be0dc6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1066,6 +1066,15 @@ jobs: # second red on wall time against `timeout-minutes`. Both constants, their # measured basis and a self-test battery each live in the script. # + # THE RED IS A `schedule` RED ALONE (C2, both timing steps, ruling + # https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511): + # the dataset's weights are measured on the hourly `schedule` runs, so + # only those exit 1 on DRIFT. On `merge_group`, `pull_request`, `push` + # and `workflow_dispatch` the step prints the same verdict, raises the + # DRIFT as a `::warning::` annotation and exits 0. The event reaches the + # script as `--event "$GITHUB_EVENT_NAME"`, which it requires; the rule + # and its self-test live there (TIMING_GRADED_EVENT, timingGateMode). + # # EXECUTED WINDOWS ONLY. Measurements come from samplesFromSummary() in # measure-test-shard-timings.mjs, which drops cache HITs and failed # suites, and the prediction is summed over the same executed packages. @@ -1073,10 +1082,11 @@ jobs: # predicted and vouch for a rotted dataset, so a shard whose packages all # replayed is NOT MEASURED, never "fast". # - # NO `if:` and NO `continue-on-error` (the red is the point), and ABOVE - # the attestation pair for the #6082 reason documented on the upload - # above: a failed unguarded step skips that pair, so a drift red also - # withholds this shard's attestation, the fail-closed direction. A shard + # NO `if:` and NO `continue-on-error` (the red is the point, and the + # `schedule`-only red is the script's `--event`, so neither is needed), + # and ABOVE the attestation pair for the #6082 reason documented on the + # upload above: a failed unguarded step skips that pair, so a drift red + # also withholds this shard's attestation, the fail-closed direction. A shard # with no packages runs no turbo and writes no summary; that is NOT # MEASURED here rather than a usage error from the script. # @@ -1109,7 +1119,7 @@ jobs: exit 0 fi node scripts/partition-test-shards.mjs --check-drift "${SUMMARIES[@]}" \ - --label "Test Core (${{ matrix.shard }}/6)" + --label "Test Core (${{ matrix.shard }}/6)" --event "$GITHUB_EVENT_NAME" # Runs even when the suite failed — that is when it earns its keep. It # answers TWO questions about a red suite, and needs both to be able to @@ -1486,12 +1496,12 @@ jobs: # ── The per-package suite-duration ratchet (#16468) ────────────────── # # A GATE, unlike the two reporting steps above: NO `continue-on-error`, - # so a package whose suite ran past its ceiling reds `Test Core`, naming - # the package and its slowest test files. The ceiling is the package's - # slowest executed run in the refresh window x 1.25, generated into - # scripts/test-shard-timings.json (`ceilings`) by the refresh lane and - # held by every later refresh; it rises only by a ruled entry in the - # script's RULED_CEILING_RAISES. A `provisional` package prints "no + # so on a `schedule` run a package whose suite ran past its ceiling reds + # `Test Core`, naming the package and its slowest test files. The + # ceiling is the package's slowest executed run in the refresh window + # x 1.25, generated into scripts/test-shard-timings.json (`ceilings`) by + # the refresh lane and held by every later refresh; it rises only by a + # ruled entry in the script's RULED_CEILING_RAISES. A `provisional` package prints "no # ceiling: provisional" and is not red; a package absent from the dataset # prints "no ceiling: new package" and is not red until the first refresh # after it lands gives it a ceiling, while a package the dataset knows @@ -1505,6 +1515,20 @@ jobs: # samplesFromSummary, and every vitest file line); a capture that did not # arrive leaves its packages ungraded and says so, never red. # + # EXIT-GRADED ON `schedule` ALONE (C2, both timing steps, ruling + # https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511): + # the ceilings come from the hourly `schedule` runs, so only those red + # this job on a package over its ceiling. On `merge_group`, + # `pull_request`, `push` and `workflow_dispatch` the step prints the same + # table, raises each package over its ceiling as a `::warning::` + # annotation and exits 0; a real slowdown merged to `main` reds the next + # hourly run. A package whose ceiling is under the absolute floor (20s, + # CEILING_FLOOR_SECONDS) prints "below floor" and is never red. The + # event reaches the script as `--event "$GITHUB_EVENT_NAME"`, which it + # requires; a refusal (a malformed table or raise record) is red on every + # event. The rule, the floor's reading and their self-tests live in the + # script. + # # NOT MEASURED, exit 0, while the dataset carries no `ceilings`: they are # generated only where the run-summary artifacts are downloadable (the # refresh lane), so the first refresh after this lands writes them, and @@ -1515,7 +1539,7 @@ jobs: # captured, so a red shard's packages are graded too. - name: Check the per-package suite-duration ceilings if: always() - run: node scripts/check-test-suite-ceilings.mjs --captures "$RUNNER_TEMP/test-timings" --label 'Test Core' + run: node scripts/check-test-suite-ceilings.mjs --captures "$RUNNER_TEMP/test-timings" --label 'Test Core' --event "$GITHUB_EVENT_NAME" # ── Temporal conformance against live, non-UTC servers (ADR-0053 D-A3) ───── diff --git a/content/docs/api/metadata-api.mdx b/content/docs/api/metadata-api.mdx index efcd29fbb03..9e47201922e 100644 --- a/content/docs/api/metadata-api.mdx +++ b/content/docs/api/metadata-api.mdx @@ -123,7 +123,9 @@ Get package details. ### `DELETE /packages/:id` -Uninstall a package. +Uninstall a package. Under a `group` or `isolated` tenancy posture, a package +holding positions or permission sets is the platform operator's to delete: any +other caller is refused `403 PERMISSION_DENIED` (ADR-0131 D3). ### `PATCH /packages/:id/enable` diff --git a/content/docs/permissions/capabilities.mdx b/content/docs/permissions/capabilities.mdx index 70c318f8aab..c0140e99d57 100644 --- a/content/docs/permissions/capabilities.mdx +++ b/content/docs/permissions/capabilities.mdx @@ -242,6 +242,13 @@ has no counterpart in it — nothing in code would ever *require* the name, so t result is an unreferenced grant target sitting in the live authorization namespace. `job` and `agent` carry the same pair for the same reason. +Under a `group` or `isolated` tenancy posture, a caller below the platform +operator reads a different refusal, with the same `403` status: the security +catalog refusal (ADR-0131 D3), whose message names the posture, with the code +`FORBIDDEN` on the REST door and `PERMISSION_DENIED` on the runtime dispatcher. +It runs before the code-only refusal, so the block above is the answer the +platform operator reads, and every caller under `single`. + Two things this does **not** close: - **The package-declaration channel is untouched.** `AppPlugin` registers stack diff --git a/content/docs/permissions/permission-sets.mdx b/content/docs/permissions/permission-sets.mdx index 173313b77d6..1a53465c142 100644 --- a/content/docs/permissions/permission-sets.mdx +++ b/content/docs/permissions/permission-sets.mdx @@ -348,7 +348,11 @@ position/user bindings, and its pending audience-binding suggestions in the same request (no ghost grants); the uninstall response reports the revocation under `data.persisted.cleanups` (the dispatcher's `/packages` domain answers the route, and it nests the protocol's uninstall report under `persisted`). -Environment-authored sets and other packages' rows survive. +Environment-authored sets and other packages' rows survive. Under a `group` or +`isolated` tenancy posture, removing a package that carries positions or +permission sets is the platform operator's (ADR-0131 D3): that delete, and an +`install-local` install, upgrade or uninstall of such a package, refuse any +other caller `403` before anything changes. ## One authoritative store — the record is a projection (ADR-0094) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 9c23669c479..592ff0edfff 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -183,7 +183,7 @@ One of the largest consumers of the flag; | 53 | Package REST route capability gate bypassed | rest | Get: a marketplace publish over REST (`POST /packages/publish`, the one route the REST registrar mounts since #14503) without `manage_metadata`; the package read cohort (`studio.access` / `setup.access`) is enforced by the dispatcher `/packages` domain's own read gate, where the reads are served | `packages/rest/src/package-routes.ts#refusePackageRequest` | | 54 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `packages/runtime/src/domains/packages.ts#requireManageMetadata`, `#requireReadCapability` | | 55 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `packages/runtime/src/domains/activation-gate.ts#refuseUngrantedActivationWrite`, `#refuseUngrantedActivationAuthoring` | -| 55b | Walled catalog refusal does not fire | metadata-core, rest, plugin-security | Get: under `isolated` / `group`, a create, change or delete of a position, a permission set or a capability, at the `/meta` write doors (save, reset, publish, rollback), at the permission-set clone door, and at the data door on `sys_position`, `sys_permission_set` and `sys_capability`. Every other caller below the platform-operator rung is refused `403` there (ADR-0131 D3), because one organization's write would change what every organization resolves by name. The seeders, the package door and migrations write the catalog this way. Under `single` the refusal is inert for every caller | `packages/metadata-core/src/meta-write-capability.ts#walledCatalogWriteVerdict`, `packages/rest/src/rest-server.ts#walledCatalogRefusal`, `packages/plugins/plugin-security/src/walled-catalog-data-door.ts#createWalledCatalogDataDoorRefusal` | +| 55b | Walled catalog refusal does not fire | metadata-core, rest, plugin-security, runtime | Get: under `isolated` / `group`, a create, change or delete of a position, a permission set or a capability, at the `/meta` write doors (save, reset, publish, rollback), at the permission-set clone door, at the data door on `sys_position`, `sys_permission_set` and `sys_capability`, and at the `/packages` delete of a package holding stored positions or permission sets. Every other caller below the platform-operator rung is refused `403` there (ADR-0131 D3), because one organization's write would change what every organization resolves by name. The seeders, the package door and migrations write the catalog this way. Under `single` the refusal is inert for every caller | `packages/metadata-core/src/meta-write-capability.ts#walledCatalogWriteVerdict`, `packages/rest/src/rest-server.ts#walledCatalogRefusal`, `packages/plugins/plugin-security/src/walled-catalog-data-door.ts#createWalledCatalogDataDoorRefusal`, `packages/runtime/src/domains/packages.ts#refuseWalledCatalogPackageDelete` | | 56 | Automation run-state read, flow-authoring write, the paused-run caller gate (screen read and resume), the two operator run-lifecycle writes and the elevated self-triggered start at every door that starts a flow by name all pass | runtime | Get: run state, flow writes, a paused run's screen and its resume with no grant and without being the run's starter — and cancelling or restoring a suspension without the platform-operator rung. The screen read and the resume ask one predicate, so this is one bypass for both. The lifecycle bypass is the in-process owner's door: plugin-approvals' revise-window recall (ADR-0044) cancels on behalf of a decision it already authorized and recorded. And starting a flow declared `runAs: 'system'` whose type is self-triggered (`autolaunched`, `record_change`, `schedule`) through any door that starts a flow by name — the trigger route, an action of `type: 'flow'` (REST `/actions` and the MCP `run_action` bridge) and a declared endpoint of `type: 'flow'` — which every other caller, the guest principal included, is refused `403 PERMISSION_DENIED` (the maintainer's rulings, letters B and A). The three doors ask one predicate, so this is one bypass for all of them, and it is the in-process caller's, such as a job: inbound HTTP never carries the flag | `packages/runtime/src/domains/automation.ts#mayReadRunState`, `#refuseUngrantedFlowWrite`, `#isRunStarterOrRunStateReader`, `#refuseUngrantedRunLifecycleWrite`, `packages/runtime/src/flow-start-admission.ts#refusesElevatedSelfTriggeredStart` | | 57 | Audience-binding suggestion authority gate passes | plugin-security | Get: a system caller lists, accepts and dismisses suggestions without `manage_metadata` and, under a wall, without the platform-operator rung | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts#assertDeploymentAuthority` | | 58 | Email-template organization door passes; webhook provenance stamp skipped | plugin-email, plugin-webhooks | Get (`sys_email_template`): a create or update of a template row. The door refuses every other write that names a caller, `403 PERMISSION_DENIED` (ADR-0131 D6); the plugin itself no longer writes the table (templates resolve from the registry), so the v18 migration ceremony's promotion is the one remaining writer. Lose (`sys_webhook`): the row is not marked as an admin customization | `packages/plugins/plugin-email/src/email-template-door.ts#isOrganizationWrite`, `packages/plugins/plugin-webhooks/src/webhook-provenance.ts#bindWebhookProvenanceStamp` | diff --git a/packages/cloud-connection/package.json b/packages/cloud-connection/package.json index f2757402ae2..bfd482cc1f5 100644 --- a/packages/cloud-connection/package.json +++ b/packages/cloud-connection/package.json @@ -27,6 +27,7 @@ "@objectstack/core": "workspace:*", "@objectstack/metadata-core": "workspace:*", "@objectstack/metadata-protocol": "workspace:*", + "@objectstack/objectql": "workspace:*", "@objectstack/runtime": "workspace:*", "@objectstack/spec": "workspace:*", "@objectstack/types": "workspace:*" diff --git a/packages/cloud-connection/src/marketplace-install-local-plugin.ts b/packages/cloud-connection/src/marketplace-install-local-plugin.ts index 10b51ce6aae..c87f409f713 100644 --- a/packages/cloud-connection/src/marketplace-install-local-plugin.ts +++ b/packages/cloud-connection/src/marketplace-install-local-plugin.ts @@ -121,7 +121,19 @@ import { protocolIncompatibleAnswer, ProtocolIncompatibleError, type ProtocolIncompatibleDiagnostic, + // [#15204 W2] ADR-0131 D3's one predicate, asked here per declared catalog + // type: the same sentence, the same rung, as every catalog write door. + walledCatalogWriteVerdict, + SECURITY_CATALOG_TYPES, } from '@objectstack/metadata-core'; +// [#15204 W2] The engine's own reading of what a manifest declares, and of +// what the running registry holds under a package, so this door judges the +// declarations the engine registers and not a second reading of them. +import { + declaredSecurityCatalogNames, + findPackageHeldSecurityCatalogNames, + type DeclaredSecurityCatalogName, +} from '@objectstack/objectql'; import { resolveCloudUrl } from './cloud-url.js'; import { resolveMarketplacePublicBaseUrl } from './marketplace-public-url.js'; import { join } from 'node:path'; @@ -400,6 +412,20 @@ function organizationWallActive(ctx: PluginContext): boolean { const NO_ACTIVE_ORGANIZATION_CODE = 'PERMISSION_DENIED'; const NO_ACTIVE_ORGANIZATION_STATUS = 403; +/** + * [#15204 W2] What an admitted mutating request carries past + * `requireInstallCapability`: the acting user, plus the two inputs ADR-0131 + * D3's refusal needs, both off the admission's single authorization read. + */ +interface InstallAdmission { + ok: true; + userId: string; + /** The caller's ADR-0095 posture rung. */ + callerPosture: unknown; + /** The EFFECTIVE tenancy posture; `undefined` when no `tenancy` service is registered. */ + tenancyPosture: TenancyPosture | undefined; +} + /** The sample-data doors that seed or purge under the caller's active organization. */ type SampleDataDoor = 'install' | 'reseed' | 'purge'; @@ -1115,6 +1141,19 @@ export class MarketplaceInstallLocalPlugin implements Plugin { const manifestId = declaredId.data; if (inlineManifest) packageId = manifestId; + // [#15204 W2] ADR-0131 D3: under a wall, a package whose incoming or + // installed version carries a catalog item is the operator's to + // install, upgrade or replace. Asked before any later step of this + // install reads or writes anything. + { + const installed = this.ledger.read(manifestId).entry?.manifest; + const walled = this.refuseWalledCatalogPackage( + c, ctx, 'Installing', manifestId, admission, + installed === undefined ? [manifest] : [manifest, installed], + ); + if (walled) return walled; + } + // 1c. [#21762] ⭐ ADR-0087 D1'S PROTOCOL HANDSHAKE, before anything is // registered, written or synced: the same `assertProtocolCompat` // the other package-install door (`POST /api/v1/packages`) and the @@ -1637,6 +1676,16 @@ export class MarketplaceInstallLocalPlugin implements Plugin { if (!this.ledger.has(manifestId)) { return c.json({ success: false, error: { code: 'RESOURCE_NOT_FOUND', message: `No marketplace install for ${manifestId}.` } }, 404); } + // [#15204 W2] ADR-0131 D3: under a wall, uninstalling a package that + // carries a catalog item is the operator's: it removes those items + // from every organization's catalog. Asked before the ledger is touched. + { + const walled = this.refuseWalledCatalogPackage( + c, ctx, 'Uninstalling', manifestId, admission, + [this.ledger.read(manifestId).entry?.manifest], + ); + if (walled) return walled; + } try { this.ledger.remove(manifestId); } catch (err: any) { @@ -2800,7 +2849,14 @@ export class MarketplaceInstallLocalPlugin implements Plugin { private resolveInstallPrincipal = async ( c: any, ctx: PluginContext, - ): Promise<{ userId: string; systemPermissions: string[] } | null> => { + ): Promise<{ + userId: string; + systemPermissions: string[]; + /** [#15204 W2] The ADR-0095 posture rung the resolver derived. */ + posture?: unknown; + /** [#15204 W2] The EFFECTIVE tenancy posture this admission resolved. */ + tenancyPosture?: TenancyPosture; + } | null> => { const headers = c?.req?.raw?.headers; if (!headers) return null; try { @@ -2841,6 +2897,8 @@ export class MarketplaceInstallLocalPlugin implements Plugin { return { userId: String(authz.userId), systemPermissions: Array.isArray(authz.systemPermissions) ? authz.systemPermissions : [], + posture: authz.posture, + tenancyPosture, }; } catch (err) { // [commit 6a180e42d] `null` here means "nobody is authenticated", which is @@ -2889,7 +2947,7 @@ export class MarketplaceInstallLocalPlugin implements Plugin { c: any, ctx: PluginContext, action: string, - ): Promise<{ ok: true; userId: string } | { ok: false; response: Response }> => { + ): Promise => { const principal = await this.resolveInstallPrincipal(c, ctx); if (!principal) return { ok: false, response: this.refuseUnauthenticated(c) }; if (!principal.systemPermissions.includes(INSTALL_LOCAL_CAPABILITY)) { @@ -2908,7 +2966,101 @@ export class MarketplaceInstallLocalPlugin implements Plugin { }, 403), }; } - return { ok: true, userId: principal.userId }; + return { + ok: true, + userId: principal.userId, + callerPosture: principal.posture, + tenancyPosture: principal.tenancyPosture, + }; + }; + + /** + * [#15204 W2] ADR-0131 D3 at the package door: under a wall the security + * catalog is the platform operator's, and a package that declares a + * position, a permission set or a capability is the operator's instrument. + * + * Measured on `main` before this refusal: an organization's + * `manage_metadata` holder (rung `TENANT_ADMIN`) upgraded an + * operator-installed package at a new version, adding `manage_users` to a + * permission set another organization's member held, and that member + * resolved `manage_users` next; the same holder's uninstall took the set + * away from them. `refuseSecurityCatalogNameConflicts` refuses only a + * SHADOW of a name another holder has, never a write by the package that + * holds it. + * + * So the WHOLE package is refused, before any write, when any of + * `manifests` (the incoming version, the installed one, or both) declares + * a catalog item, or the running registry holds one under `manifestId`. + * A partial install is not well-formed: a package is registered whole. + * The decision is {@link walledCatalogWriteVerdict}'s, never restated. It + * is asked FIRST, for the first catalog type: its answer does not vary + * across catalog types, so when it admits this caller (under `single`, + * with no `tenancy` service, or for the `PLATFORM_ADMIN` rung) nothing is + * read and the door behaves exactly as before. Only a refused caller's + * package is read, and the refusal names the first type it declares. A + * package with no catalog item is admitted. The answer is the sibling + * refusals' envelope, `403 FORBIDDEN`. + */ + private refuseWalledCatalogPackage = ( + c: any, + ctx: PluginContext, + action: string, + manifestId: string, + admission: InstallAdmission, + manifests: readonly unknown[], + ): Response | null => { + const verdictFor = (type: string) => walledCatalogWriteVerdict({ + type, + tenancyPosture: admission.tenancyPosture, + callerPosture: admission.callerPosture, + }); + if (verdictFor(SECURITY_CATALOG_TYPES[0]!).allowed) return null; + const declared = this.catalogItemsOf(ctx, manifestId, manifests); + if (declared.length === 0) return null; + for (const type of SECURITY_CATALOG_TYPES) { + if (!declared.some((item) => item.type === type)) continue; + const verdict = verdictFor(type); + if (verdict.allowed) continue; + ctx.logger?.warn?.( + `[MarketplaceInstallLocal] refused ${action} ${manifestId} for ${admission.userId}: ` + + `the package carries security catalog items under the '${admission.tenancyPosture}' posture (ADR-0131 D3)`, + ); + return c.json({ + success: false, + error: { + code: 'FORBIDDEN', + message: `${action} "${manifestId}" is refused: the package carries security catalog items. ${verdict.message}`, + details: { manifestId, catalogItems: declared }, + }, + }, 403); + } + return null; + }; + + /** + * [#15204 W2] Every catalog item `manifests` declare, plus every position + * and permission-set name the running registry holds under `manifestId` + * (a package the registry holds outside this door's ledger, such as a + * writable runtime package an operator authored items into). Read-only. + */ + private catalogItemsOf = ( + ctx: PluginContext, + manifestId: string, + manifests: readonly unknown[], + ): DeclaredSecurityCatalogName[] => { + const out = new Map(); + const add = (item: DeclaredSecurityCatalogName) => out.set(`${item.type}:${item.name}`, item); + for (const manifest of manifests) for (const item of declaredSecurityCatalogNames(manifest)) add(item); + let registry: Parameters[0] | undefined; + try { + registry = (ctx.getService('objectql') as { registry?: typeof registry } | undefined)?.registry; + } catch { /* no data engine: the manifests are the whole reading */ } + if (registry) { + for (const held of findPackageHeldSecurityCatalogNames(registry)) { + if (held.packageIds.includes(manifestId)) add({ type: held.catalogType, name: held.name }); + } + } + return [...out.values()]; }; /** diff --git a/packages/cloud-connection/vitest.config.ts b/packages/cloud-connection/vitest.config.ts index 0d1d4fe56bc..9206035c576 100644 --- a/packages/cloud-connection/vitest.config.ts +++ b/packages/cloud-connection/vitest.config.ts @@ -50,6 +50,15 @@ export default defineConfig({ find: /^@objectstack\/metadata-core$/, replacement: path.resolve(__dirname, '../metadata-core/src/index.ts'), }, + // [#15204 W2] The install-local door reads a manifest's catalog + // declarations through `declaredSecurityCatalogNames`, so every suite + // that imports the plugin judges THIS checkout's engine reading, not an + // `objectql/dist` that may be behind it. Anchored: `@objectstack/objectql` + // exports a `./core` subpath. + { + find: /^@objectstack\/objectql$/, + replacement: path.resolve(__dirname, '../objectql/src/index.ts'), + }, ], }, // Discovery is untouched — but this file is NOT "the alias and nothing else". diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index d3a216a6cd3..658c8ad2810 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -94,6 +94,16 @@ export { // stay private to the engine. export { findPackageHeldSecurityCatalogNames } from './registry.js'; +// [#15204 W2] Every `(type, name)` a manifest declares, read from the sources +// the engine's registration seams read. Exported for the install-local door +// (`@objectstack/cloud-connection`), which refuses a catalog-carrying package +// under a wall (ADR-0131 D3) and must judge the declarations the engine +// registers, not a second reading of the manifest. +export { + declaredSecurityCatalogNames, + type DeclaredSecurityCatalogName, +} from './security-catalog-namespace.js'; + // [#14553] The navigation-contribution group diagnostic (ADR-0029 D7, // ADR-0112 D6c). Exported because `os build` is the SECOND door that has to diff --git a/packages/qa/dogfood/test/wall-package-refusal.dogfood.test.ts b/packages/qa/dogfood/test/wall-package-refusal.dogfood.test.ts new file mode 100644 index 00000000000..dabfca26122 --- /dev/null +++ b/packages/qa/dogfood/test/wall-package-refusal.dogfood.test.ts @@ -0,0 +1,295 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [ADR-0131 D3 · #15204 W2] The walled catalog refusal at the PACKAGE doors, on +// a REAL booted stack. +// +// ADR-0131 D3: under `group` / `isolated` "tenant admins cannot create or edit +// catalog items; the operator defines the catalog for every tenant (managed +// packages, Studio)". A package that declares a position, a permission set or +// a capability is the operator's instrument. +// +// What this harness measured before the refusal, under both walled postures: +// organization A's owner, granted `manage_metadata` inside organization A only +// (rung `TENANT_ADMIN`, never the platform operator): +// - re-installed an operator-installed package at a new version through +// `POST /api/v1/marketplace/install-local`, adding `manage_users` to a +// permission set organization B's member holds: `200`, and B's member +// resolved `manage_users` next; +// - uninstalled it through `DELETE /api/v1/marketplace/install-local/:id`: +// `200`, the set left the catalog and B's member lost it; +// - deleted, through `DELETE /api/v1/packages/:id`, a writable runtime +// package the operator had saved a permission set into +// (`PUT /meta/permission/:name?package=`): `200`, the same loss for B. +// `install-local` also registered A's own catalog-carrying package +// environment-wide. +// +// Pinned here, under each walled posture: +// - an install whose incoming version declares a catalog item, an upgrade +// where either version declares one, and an uninstall of a package whose +// installed version declares one answer `403 FORBIDDEN`, and B's +// resolution does not move; +// - the `/packages` delete of a package holding stored catalog items answers +// `403 PERMISSION_DENIED`, and B's resolution does not move; +// - CONTROLS: A installs, upgrades and uninstalls a catalog-free package and +// deletes a catalog-free runtime package as before; the platform operator +// upgrades, uninstalls and deletes the catalog-carrying ones. +// And under `single` (CONTROL): the same `manage_metadata` holder, who is not +// the platform operator, does all of it as before. +// +// The boot is `multiTenant: 'posture-only'`: a real, non-degraded walled +// posture. The refusal keys on the posture and the caller's rung, never on row +// isolation. The harness declares the seeded admin as `OS_PLATFORM_OWNER_EMAIL`, +// so that admin is the platform operator. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import crmStack from '@objectstack/example-crm'; +import { resolveUserAuthzGrants } from '@objectstack/core'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { MarketplaceInstallLocalPlugin } from '@objectstack/cloud-connection'; +import { mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { buildShapedArtifact } from './build-shaped-artifact.js'; + +const SYS = { isSystem: true, positions: [], permissions: [] }; +const INSTALL_LOCAL = '/marketplace/install-local'; + +interface Engine { + find(object: string, options?: unknown): Promise>>; + insert(object: string, data: Record, options?: unknown): Promise; +} + +/** The ADR-0112 envelope a refusal answered: `error.code`, the message and the HTTP status. */ +async function envelopeOf(res: Response): Promise<{ status: number; code: unknown; message: string }> { + const body = (await res.clone().json().catch(() => ({}))) as { error?: { code?: unknown; message?: unknown } }; + return { status: res.status, code: body?.error?.code, message: String(body?.error?.message ?? '') }; +} + +/** An install-local body for a tiny app stack, with the catalog items it declares. */ +function packageBody(id: string, version: string, catalog: { permissions?: unknown[]; positions?: unknown[] } = {}) { + const stackDef = { + manifest: { id, name: id, version, type: 'app' }, + permissions: catalog.permissions ?? [], + positions: catalog.positions ?? [], + }; + const { artifact } = buildShapedArtifact(stackDef as unknown as Record); + return { manifest: { ...artifact, id, version } }; +} + +const set = (name: string, systemPermissions: string[]) => ({ name, label: name, objects: {}, systemPermissions }); + +const META_AUTHOR = 'w2_meta_author'; +const OP_PKG = 'com.w2.operator'; +const OP_SET = 'w2_operator_set'; +const RT_PKG = 'com.w2.runtime'; +const RT_SET = 'w2_runtime_set'; + +interface Harness { + stack: VerifyStack; + ql: Engine; + operator: string; + tokA: string; + userA: string; + userB: string; + orgA?: string; + orgB?: string; +} + +/** + * Boot, sign in the operator, A and B; under a wall each gets an organization. + * The operator installs `OP_PKG` (carrying `OP_SET`), creates the writable + * runtime package `RT_PKG` and saves `RT_SET` into it; B holds both sets; A + * holds `manage_metadata` (inside org A under a wall). + */ +async function boot(posture: 'isolated' | 'group' | 'single'): Promise { + const walled = posture !== 'single'; + if (walled) process.env.OS_TENANCY_POSTURE = posture; + else delete process.env.OS_TENANCY_POSTURE; + const storageDir = mkdtempSync(join(tmpdir(), `w2-wall-package-${posture}-`)); + const stack = await bootStack(crmStack as never, { + ...(walled ? { multiTenant: 'posture-only' as const } : {}), + extraPlugins: [new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir })], + } as never); + const ql = (await stack.kernel.getServiceAsync('objectql')) as unknown as Engine; + const operator = await stack.signIn(); + const tokA = await stack.signUp(`w2-a-${posture}@wall.test`); + const tokB = await stack.signUp(`w2-b-${posture}@wall.test`); + const h: Harness = { stack, ql, operator, tokA, userA: '', userB: '' }; + if (walled) { + for (const [tok, slug] of [[tokA, `w2-a-${posture}`], [tokB, `w2-b-${posture}`]] as const) { + const created = await stack.apiAs(tok, 'POST', '/auth/organization/create', { name: slug, slug }); + expect(created.status, await created.clone().text()).toBe(200); + const id = ((await created.json()) as { id: string }).id; + if (tok === tokA) h.orgA = id; else h.orgB = id; + const active = await stack.apiAs(tok, 'POST', '/auth/organization/set-active', { organizationSlug: slug }); + expect(active.status, await active.clone().text()).toBe(200); + } + } + const idOf = async (email: string) => + String((await ql.find('sys_user', { where: { email }, limit: 1, context: SYS }))[0]?.id); + h.userA = await idOf(`w2-a-${posture}@wall.test`); + h.userB = await idOf(`w2-b-${posture}@wall.test`); + + const saved = await stack.apiAs(operator, 'PUT', `/meta/permission/${META_AUTHOR}`, set(META_AUTHOR, ['manage_metadata'])); + expect(saved.status, await saved.clone().text()).toBe(200); + const installed = await stack.apiAs(operator, 'POST', INSTALL_LOCAL, packageBody(OP_PKG, '1.0.0', { permissions: [set(OP_SET, [])] })); + expect(installed.status, await installed.clone().text()).toBe(200); + const created = await stack.apiAs(operator, 'POST', '/packages', { id: RT_PKG, name: 'w2 runtime', version: '1.0.0', type: 'app' }); + expect(created.status, await created.clone().text()).toBe(201); + const authored = await stack.apiAs(operator, 'PUT', `/meta/permission/${RT_SET}?package=${RT_PKG}`, set(RT_SET, [])); + expect(authored.status, await authored.clone().text()).toBe(200); + + const grant = (user: string, permissionSet: string, org: string | undefined) => + ql.insert('sys_user_permission_set', { user_id: user, permission_set: permissionSet, ...(org ? { organization_id: org } : {}) }, + { context: { ...SYS, ...(org ? { tenantId: org } : {}) } }); + await grant(h.userA, META_AUTHOR, h.orgA); + await grant(h.userB, OP_SET, h.orgB); + await grant(h.userB, RT_SET, h.orgB); + return h; +} + +const grantsOf = (h: Harness, user: string, org: string | undefined) => + resolveUserAuthzGrants(h.ql as never, user, org ? { tenantId: org } : {}); + +for (const posture of ['isolated', 'group'] as const) { + describe(`[ADR-0131 D3 · #15204 W2] ${posture}: a catalog-carrying package is the platform operator's`, () => { + let h: Harness; + const previous = process.env.OS_TENANCY_POSTURE; + const bGrants = () => grantsOf(h, h.userB, h.orgB); + + beforeAll(async () => { h = await boot(posture); }, 180_000); + afterAll(async () => { + await h?.stack?.stop?.(); + if (previous === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = previous; + }); + + it('guard the guard: a real walled posture; A holds `manage_metadata` in org A and is not the platform operator; B holds both sets', async () => { + const tenancy = await h.stack.kernel.getServiceAsync<{ posture: string; degraded: boolean }>('tenancy'); + expect(tenancy.posture).toBe(posture); + expect(tenancy.degraded).toBe(false); + expect(h.orgA).toBeTruthy(); + expect(h.orgA).not.toBe(h.orgB); + const a = await grantsOf(h, h.userA, h.orgA); + expect(a.systemPermissions).toContain('manage_metadata'); + expect(a.posture).not.toBe('PLATFORM_ADMIN'); + const b = await bGrants(); + expect(b.permissions).toEqual(expect.arrayContaining([OP_SET, RT_SET])); + expect(b.systemPermissions).not.toContain('manage_users'); + }); + + it('install: A installing a package whose incoming version declares a permission set is refused 403 FORBIDDEN, naming the posture', async () => { + const res = await h.stack.apiAs(h.tokA, 'POST', INSTALL_LOCAL, + packageBody('com.w2.tenant', '1.0.0', { permissions: [set('w2_tenant_set', ['manage_users'])] })); + const env = await envelopeOf(res); + expect({ status: env.status, code: env.code }).toEqual({ status: 403, code: 'FORBIDDEN' }); + expect(env.message).toContain(`'${posture}'`); + expect(env.message).toContain('ADR-0131 D3'); + const listed = await h.stack.apiAs(h.operator, 'GET', '/meta/permission/w2_tenant_set'); + expect(listed.status).toBe(404); + }); + + it('install: a position alone in the incoming version is refused the same way', async () => { + const res = await h.stack.apiAs(h.tokA, 'POST', INSTALL_LOCAL, + packageBody('com.w2.tenantpos', '1.0.0', { positions: [{ name: 'w2_tenant_pos', label: 'tenant pos' }] })); + expect(await envelopeOf(res)).toMatchObject({ status: 403, code: 'FORBIDDEN' }); + }); + + it('upgrade (the measured edge): A re-installing the operator\'s package with `manage_users` added is refused, and B does not gain it', async () => { + const res = await h.stack.apiAs(h.tokA, 'POST', INSTALL_LOCAL, + packageBody(OP_PKG, '1.1.0', { permissions: [set(OP_SET, ['manage_users'])] })); + expect(await envelopeOf(res)).toMatchObject({ status: 403, code: 'FORBIDDEN' }); + expect((await bGrants()).systemPermissions).not.toContain('manage_users'); + }); + + it('upgrade: a catalog-free incoming version over an installed catalog-carrying one is refused too, and B keeps the set', async () => { + const res = await h.stack.apiAs(h.tokA, 'POST', INSTALL_LOCAL, packageBody(OP_PKG, '1.2.0')); + expect(await envelopeOf(res)).toMatchObject({ status: 403, code: 'FORBIDDEN' }); + expect((await bGrants()).permissions).toContain(OP_SET); + }); + + it('uninstall (the measured edge): A uninstalling the operator\'s package is refused, and B keeps the set', async () => { + const res = await h.stack.apiAs(h.tokA, 'DELETE', `${INSTALL_LOCAL}/${OP_PKG}`); + expect(await envelopeOf(res)).toMatchObject({ status: 403, code: 'FORBIDDEN' }); + expect((await bGrants()).permissions).toContain(OP_SET); + }); + + it('/packages delete (the measured edge): A deleting a runtime package holding the operator\'s set is refused 403 PERMISSION_DENIED, and B keeps the set', async () => { + const res = await h.stack.apiAs(h.tokA, 'DELETE', `/packages/${RT_PKG}`); + const env = await envelopeOf(res); + expect({ status: env.status, code: env.code }).toEqual({ status: 403, code: 'PERMISSION_DENIED' }); + expect(env.message).toContain(`'${posture}'`); + expect((await bGrants()).permissions).toContain(RT_SET); + const still = await h.stack.apiAs(h.operator, 'GET', `/packages/${RT_PKG}`); + expect(still.status).toBe(200); + }); + + it('CONTROL: A installs, upgrades and uninstalls a catalog-free package, and deletes a catalog-free runtime package, as before', async () => { + for (const version of ['1.0.0', '1.1.0']) { + const res = await h.stack.apiAs(h.tokA, 'POST', INSTALL_LOCAL, packageBody('com.w2.free', version)); + expect(res.status, await res.clone().text()).toBe(200); + } + const removed = await h.stack.apiAs(h.tokA, 'DELETE', `${INSTALL_LOCAL}/com.w2.free`); + expect(removed.status, await removed.clone().text()).toBe(200); + const created = await h.stack.apiAs(h.operator, 'POST', '/packages', { id: 'com.w2.freert', name: 'free', version: '1.0.0', type: 'app' }); + expect(created.status, await created.clone().text()).toBe(201); + const deleted = await h.stack.apiAs(h.tokA, 'DELETE', '/packages/com.w2.freert'); + expect(deleted.status, await deleted.clone().text()).toBe(200); + }); + + it('CONTROL: the platform operator installs, upgrades and uninstalls a catalog-carrying package, and deletes one at /packages', async () => { + // Its own packages, so this control reads the same whatever the refusal + // pins above did to OP_PKG and RT_PKG. + const pkg = 'com.w2.operator2'; + const opSet = 'w2_operator2_set'; + const installed = await h.stack.apiAs(h.operator, 'POST', INSTALL_LOCAL, packageBody(pkg, '1.0.0', { permissions: [set(opSet, [])] })); + expect(installed.status, await installed.clone().text()).toBe(200); + await h.ql.insert('sys_user_permission_set', { user_id: h.userB, permission_set: opSet, organization_id: h.orgB }, + { context: { ...SYS, tenantId: h.orgB } }); + const upgraded = await h.stack.apiAs(h.operator, 'POST', INSTALL_LOCAL, + packageBody(pkg, '1.1.0', { permissions: [set(opSet, ['manage_users'])] })); + expect(upgraded.status, await upgraded.clone().text()).toBe(200); + expect((await bGrants()).systemPermissions).toContain('manage_users'); + const removed = await h.stack.apiAs(h.operator, 'DELETE', `${INSTALL_LOCAL}/${pkg}`); + expect(removed.status, await removed.clone().text()).toBe(200); + expect((await bGrants()).permissions).not.toContain(opSet); + + const rt = 'com.w2.runtime2'; + const created = await h.stack.apiAs(h.operator, 'POST', '/packages', { id: rt, name: 'w2 runtime 2', version: '1.0.0', type: 'app' }); + expect(created.status, await created.clone().text()).toBe(201); + const authored = await h.stack.apiAs(h.operator, 'PUT', `/meta/permission/w2_runtime2_set?package=${rt}`, set('w2_runtime2_set', [])); + expect(authored.status, await authored.clone().text()).toBe(200); + const deleted = await h.stack.apiAs(h.operator, 'DELETE', `/packages/${rt}`); + expect(deleted.status, await deleted.clone().text()).toBe(200); + }); + }); +} + +describe('[ADR-0131 D3 · #15204 W2] single (CONTROL): the refusal is inert', () => { + let h: Harness; + const previous = process.env.OS_TENANCY_POSTURE; + + beforeAll(async () => { h = await boot('single'); }, 180_000); + afterAll(async () => { + await h?.stack?.stop?.(); + if (previous === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = previous; + }); + + it('a `manage_metadata` holder below the operator installs, upgrades and uninstalls catalog-carrying packages, and deletes one at /packages', async () => { + const a = await grantsOf(h, h.userA, undefined); + expect(a.systemPermissions).toContain('manage_metadata'); + expect(a.posture).not.toBe('PLATFORM_ADMIN'); + const installed = await h.stack.apiAs(h.tokA, 'POST', INSTALL_LOCAL, + packageBody('com.w2.tenant', '1.0.0', { permissions: [set('w2_tenant_set', [])] })); + expect(installed.status, await installed.clone().text()).toBe(200); + const upgraded = await h.stack.apiAs(h.tokA, 'POST', INSTALL_LOCAL, + packageBody(OP_PKG, '1.1.0', { permissions: [set(OP_SET, ['manage_users'])] })); + expect(upgraded.status, await upgraded.clone().text()).toBe(200); + expect((await grantsOf(h, h.userB, undefined)).systemPermissions).toContain('manage_users'); + const removed = await h.stack.apiAs(h.tokA, 'DELETE', `${INSTALL_LOCAL}/${OP_PKG}`); + expect(removed.status, await removed.clone().text()).toBe(200); + const deleted = await h.stack.apiAs(h.tokA, 'DELETE', `/packages/${RT_PKG}`); + expect(deleted.status, await deleted.clone().text()).toBe(200); + }); +}); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 918f0cb305f..4467b92274e 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -32,6 +32,11 @@ import { // `OBJECT_SCHEMA_READ_ONLY_EXEMPT_CAPABILITIES` — same value it read before, // no re-ruling of the package cohort as a side effect of #7020. import { OBJECT_SCHEMA_READ_ONLY_EXEMPT_CAPABILITIES } from '@objectstack/metadata-core'; +// [#15204 W2] ADR-0131 D3's one predicate, asked by the package delete below +// per stored catalog type, never restated here. +import { SECURITY_CATALOG_TYPES, isSecurityCatalogType, walledCatalogWriteVerdict } from '@objectstack/metadata-core'; +import { readEffectiveTenancyPosture } from './activation-gate.js'; +import type { IObjectQLEngine } from '@objectstack/spec/contracts'; // [#21727] ADR-0087 D1's protocol handshake and the brand predicate for its // refusal. The install branch asks the predicate ahead of its generic catch, // and its no-protocol fallback runs the same handshake. @@ -404,6 +409,73 @@ function requireReadCapability(deps: DomainHandlerDeps, context: HttpProtocolCon * id still falls through to the route's own 404 rather than being re-labelled * 422 (and so this gate never becomes an existence oracle of its own). */ +/** + * [#15204 W2] ADR-0131 D3 at `DELETE /packages/:id`: under a wall the security + * catalog is the platform operator's, so deleting a package that carries a + * position or a permission set is too. + * + * Measured before this refusal, under `isolated` and `group`: the operator + * created a writable runtime package and saved a permission set into it + * (`PUT /meta/permission/:name?package=`); another organization's member held + * that set; an organization's `manage_metadata` holder (rung `TENANT_ADMIN`) + * deleted the package, `200`, and the set left the catalog and the member's + * grants. The capability gate above checks no rung. + * + * The items are read where the delete reads them: `deletePackage` removes the + * `sys_metadata` rows bound to the package, and those rows are not held under + * the package in the running registry, so the stored rows are what is asked. + * The decision is {@link walledCatalogWriteVerdict}'s, never restated. It is + * asked FIRST, for the first catalog type: its answer does not vary across + * catalog types, so when it admits this caller (under `single`, with no + * `tenancy` service, for `isSystem` or for the `PLATFORM_ADMIN` rung) nothing + * is read and the delete behaves exactly as before. Only a refused caller's + * package is read, and a package without catalog rows is admitted. The answer + * is this domain's `403` (`PERMISSION_DENIED`). + */ +async function refuseWalledCatalogPackageDelete( + deps: DomainHandlerDeps, + context: HttpProtocolContext, + qlService: Pick | null | undefined, + packageId: string, +): Promise { + const ec: any = context?.executionContext; + const tenancyPosture = await readEffectiveTenancyPosture(deps, context); + const verdictFor = (type: string) => walledCatalogWriteVerdict({ + type, + tenancyPosture, + isSystem: ec?.isSystem === true, + callerPosture: ec?.posture, + }); + if (verdictFor(SECURITY_CATALOG_TYPES[0]!).allowed) return null; + if (typeof qlService?.find !== 'function') return null; + let rows: unknown; + try { + rows = await qlService.find('sys_metadata', { where: { package_id: packageId }, context: { isSystem: true } }); + } catch (e) { + return { handled: true, response: deps.errorFromThrown(e, 500) }; + } + const storedTypes = new Set(); + for (const row of Array.isArray(rows) ? rows : []) { + const type = (row as { type?: unknown } | null)?.type; + if (typeof type === 'string' && isSecurityCatalogType(type)) storedTypes.add(type); + } + if (storedTypes.size === 0) return null; + for (const type of SECURITY_CATALOG_TYPES) { + if (!storedTypes.has(type)) continue; + const verdict = verdictFor(type); + if (!verdict.allowed) { + return { + handled: true, + response: deps.error( + `Deleting package "${packageId}" is refused: the package carries security catalog items. ${verdict.message}`, + 403, + ), + }; + } + } + return null; +} + function requireWritablePackage( deps: DomainHandlerDeps, engine: unknown, @@ -2033,7 +2105,13 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // the next restart. const readOnly = requireWritablePackage(deps, qlService, id, 'delete'); if (readOnly) return readOnly; // [ADR-0131 D6/D12] No organization: an uninstall is environment-wide - // by construction, and who may uninstall is the operator gate above. + // by construction. The gate above checks the `manage_metadata` + // capability (or `isSystem`) and no posture rung, so under a wall an + // organization's holder passes it. [#15204 W2] What makes the delete + // the operator's under a wall is the catalog refusal below (ADR-0131 + // D3), for a package that carries a position or a permission set. + const walled = await refuseWalledCatalogPackageDelete(deps, _context, qlService, id); + if (walled) return walled; const protocol = await resolveProtocol(deps, _context); const persists = Boolean(protocol && typeof protocol.deletePackage === 'function'); diff --git a/packages/spec/src/migrations/entries/semantic/18.walled-catalog-package-writes-retired.ts b/packages/spec/src/migrations/entries/semantic/18.walled-catalog-package-writes-retired.ts new file mode 100644 index 00000000000..6933ebc27d8 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.walled-catalog-package-writes-retired.ts @@ -0,0 +1,44 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// ADR-0131 D3 (stage W2 of the C3 card) — a runtime-door narrowing with no +// authorable key: under a walled tenancy posture a package that carries +// security catalog items is the platform operator's to install, upgrade, +// uninstall or delete. Registered because a caller that did so before is +// refused now, and an operator must be told who keeps that authority. +export const entry: SemanticMigration = { + id: 'walled-catalog-package-writes-retired', + // No backticks in `surface`: build-upgrade-guide.ts renders it inside a + // code span AND a table cell. + surface: + 'on a group or isolated deployment, by a caller below the platform operator: ' + + 'POST /api/v1/marketplace/install-local of a package whose incoming version, or whose installed ' + + 'version on an upgrade, declares a position, permission set or capability; ' + + 'DELETE /api/v1/marketplace/install-local/:id of a package whose installed version declares one ' + + '(@objectstack/cloud-connection); and DELETE /api/v1/packages/:id of a package holding stored ' + + 'positions or permission sets on the runtime dispatcher', + replacement: + 'the platform operator (the `PLATFORM_ADMIN` rung, the deployment\'s declared ' + + '`OS_PLATFORM_OWNER_EMAIL` owner) installs, upgrades, uninstalls and deletes those packages; ' + + 'a package with no catalog item installs, upgrades and uninstalls exactly as before. A custom ' + + 'package door asks `walledCatalogWriteVerdict` (`@objectstack/metadata-core`) once per catalog ' + + 'type the package declares, after its capability gate', + reason: + 'ADR-0131 D3: on a shared-database multi-tenant deployment tenant admins cannot create or edit ' + + 'catalog items, and the operator defines the catalog for every tenant, including through ' + + 'managed packages. A package registers its catalog items environment-wide, so before this ' + + 'change a manage_metadata holder scoped to one organization could upgrade, uninstall or delete ' + + 'a package holding a permission set another organization\'s members hold, and change or remove ' + + 'what those members resolve.', + acceptanceCriteria: + 'On an isolated or group deployment, an organization owner holding manage_metadata inside their ' + + 'own organization re-installs, at a new version, an operator-installed package whose permission ' + + 'set another organization\'s member holds: the answer is 403 FORBIDDEN, the message names the ' + + 'posture, and the member resolves exactly what it resolved before. The same owner\'s uninstall ' + + 'of that package answers 403 FORBIDDEN, and DELETE /api/v1/packages/ of a runtime package ' + + 'holding a permission set answers 403 PERMISSION_DENIED; the member keeps the set. The same owner ' + + 'installs, upgrades and uninstalls a package with no catalog item with 200. The platform operator ' + + 'does all of it with 200, and on a single deployment a manage_metadata holder does all of it ' + + 'exactly as before.', +}; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0717ad84049..3d207cecd40 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -766,6 +766,9 @@ importers: '@objectstack/metadata-protocol': specifier: workspace:* version: link:../metadata-protocol + '@objectstack/objectql': + specifier: workspace:* + version: link:../objectql '@objectstack/runtime': specifier: workspace:* version: link:../runtime diff --git a/scripts/check-test-suite-ceilings.mjs b/scripts/check-test-suite-ceilings.mjs index 570a0ceaa04..cc855740c0d 100644 --- a/scripts/check-test-suite-ceilings.mjs +++ b/scripts/check-test-suite-ceilings.mjs @@ -2,11 +2,12 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Per-package suite-duration ratchet (#16468): a run in which a package's test - * suite took longer than its measured ceiling is RED, naming the package and - * its slowest test files, and a ceiling rises only by a ruling. + * Per-package suite-duration ratchet (#16468): a `schedule` run in which a + * package's test suite took longer than its measured ceiling is RED, naming the + * package and its slowest test files; every other run reports the same reading + * as a warning; and a ceiling rises only by a ruling. * - * node scripts/check-test-suite-ceilings.mjs --captures [--label ] + * node scripts/check-test-suite-ceilings.mjs --captures --event [--label ] * node scripts/check-test-suite-ceilings.mjs --self-test * * ## Why it exists @@ -39,6 +40,52 @@ * - A ceiling rises only by a ruling: RULED_CEILING_RAISES, each entry naming * the ruling's comment URL. Nothing else in this file moves a ceiling. * + * ## Exit-graded on `schedule` alone; every other event reports (C2) + * + * Ruling (letter C2, for both timing steps, plus an absolute floor): + * https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511 + * "a timing gate exit-grades only on the run profile its baseline was measured + * on; everywhere else it reports." The ceilings come from the refresh window, + * which reads ci.yml's hourly `schedule` runs alone; those replay part of the + * workspace from the turbo cache, while a merge-queue or PR build executes far + * more of it on the same runners. Graded against that window, 9 of the 19 queue + * builds on the morning the table went live redded on packages no PR in them + * touched, and PR heads did too, while the hourly run read green. + * + * So `--event` is required, and the mode is TIMING_GRADED_EVENT's, shared with + * the drift step (timingGateMode in partition-test-shards.mjs): + * + * schedule exit-graded, as before: a package over its ceiling exits 1 + * with an `::error::` annotation naming it + * every other REPORT ONLY (merge_group, pull_request, push, + * event workflow_dispatch): the same table prints, each package over + * its ceiling is a `::warning::` annotation, and it exits 0 + * + * A real slowdown merged to `main` reds the next hourly run, which names the + * package and its slowest files. A refusal -- a malformed raise record or + * ceiling table, or a missing `--event` -- is not a timing reading, does not + * vary with runner load, and exits 1 on every event. + * + * ⚠ The hourly run grades only the packages it EXECUTES: a turbo cache replay + * carries no window, so it is never graded. A package the hourly run replays on + * every run (ci.yml's `timeout-minutes` note: "scheduled runs mostly replay the + * cache their commit's push run saved") is graded on any hourly run that does + * execute it and reported everywhere else; nothing here forces one to. + * + * ## The absolute floor: CEILING_FLOOR_SECONDS, 20 s + * + * Same ruling: "a package whose ceiling is below the floor is never red; it + * prints 'below floor'". The floor is read off the table's distribution by the + * `domain:devx` seat and is not below 20 s. The reading: on `1f312fdfa` the 72 + * ceilings run 3.40, 3.69, 4.24, 5.18, 6.39, 9.10 and 16.23 s, then 22.71 s + * and up, and 20 s sits in the widest gap of the low band (16.23 -> 22.71). The + * seven packages under it then were knowledge-ragflow, service-cache, + * observability, sdui-parser, knowledge-memory, embedder-openai and + * client-react. A 3.4 s ceiling crossed by a 4.2 s run is noise, not a + * regression. The floor is compared with the EFFECTIVE ceiling, so a ruled + * raise to the floor or above grades the package again; the formula, its + * x 1.25 and RULED_CEILING_RAISES are unchanged. + * * ## No table yet is NOT MEASURED, never red * * The ceilings can only be generated where the run-summary artifacts can be @@ -94,10 +141,11 @@ * ## Exit codes * * 0 OK, or NOT MEASURED (no table yet; nothing executed; no captures); a - * new package alone never makes it 1 - * 1 a package over its ceiling, or a refusal (a malformed ceiling table -- - * a package the dataset knows that lost its row among them -- or raise - * record) + * new package or one below the floor never makes it 1; and on any event + * but `schedule`, a package over its ceiling (reported as a warning) + * 1 a package over its ceiling on a `schedule` run, or a refusal on any + * event (a malformed ceiling table -- a package the dataset knows that + * lost its row among them -- or raise record, or no `--event`) */ import { readFileSync, readdirSync, statSync } from 'node:fs'; @@ -106,7 +154,12 @@ import { fileURLToPath } from 'node:url'; import process from 'node:process'; import { isEntrypoint } from './invoked-as.mjs'; -import { escapeWorkflowCommandMessage } from './partition-test-shards.mjs'; +import { + TIMING_GRADED_EVENT, + escapeWorkflowCommandMessage, + timingGateMode, + timingModeNote, +} from './partition-test-shards.mjs'; const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const DATASET_PATH = path.join(REPO_ROOT, 'scripts', 'test-shard-timings.json'); @@ -115,6 +168,13 @@ const DATASET_REL = 'scripts/test-shard-timings.json'; /** How many of a red package's slowest test files the verdict names. */ export const SLOWEST_FILES_NAMED = 5; +/** + * The absolute floor (C2 ruling): a package whose effective ceiling is below it + * is never red and prints "below floor". Its reading and ruling are in the + * header, "The absolute floor". + */ +export const CEILING_FLOOR_SECONDS = 20; + // ── The ruled raises ────────────────────────────────────────────────────── // // A ceiling rises ONLY here, and only on a ruling. One entry per package: @@ -241,8 +301,9 @@ export function readCeilingTable(dataset) { * Returns `{ table, rows, problems }`; each row is one executed package with a * `status`: * - * over graded, over its effective ceiling RED + * over graded, over its effective ceiling RED on `schedule`, a warning elsewhere * ok graded, within its effective ceiling + * below-floor effective ceiling under CEILING_FLOOR_SECONDS never red * new not in the dataset, and no ruled raise no ceiling, not red * provisional in the dataset's `provisional` list no ceiling, not red * uncapped `uncapped` with a reason (`carried`) no ceiling, not red @@ -297,6 +358,11 @@ export function gradeCeilings({ merged, dataset, raises = RULED_CEILING_RAISES } continue; } row.ceiling = Math.max(generated ?? 0, raise?.seconds ?? 0); + // The absolute floor: strict `<`, so a ceiling AT the floor is graded. + if (row.ceiling < CEILING_FLOOR_SECONDS) { + row.status = 'below-floor'; + continue; + } // Strict `>`: a run AT its ceiling is within it, like the drift bound. row.status = seconds > row.ceiling ? 'over' : 'ok'; } @@ -335,8 +401,13 @@ function ceilingText(row) { * The verdict a runner receives, rendered without printing it so the * self-test can read it: `out` is stdout (workflow-command annotations), * `err` the human log. + * + * `event` is the run's (`--event`, required). Off TIMING_GRADED_EVENT a + * package over its ceiling prints the same table, is a `::warning::` + * annotation, and exits 0 (C2); a refusal exits 1 on every event. */ -export function renderCeilingVerdict(report, label) { +export function renderCeilingVerdict(report, label, event) { + const mode = timingGateMode(event); const head = `suite-ceiling`; if (report.problems.length > 0) { return { @@ -378,6 +449,12 @@ export function renderCeilingVerdict(report, label) { return ` -- ${r.name}: ${r.seconds.toFixed(2)}s -- no ceiling: provisional`; case 'uncapped': return ` -- ${r.name}: ${r.seconds.toFixed(2)}s -- no ceiling: ${r.reason}`; + case 'below-floor': + return ( + ` -- ${r.name}: ${r.seconds.toFixed(2)}s against ${r.ceiling.toFixed(2)}s ` + + `(${(r.seconds / r.ceiling).toFixed(2)}x)${sliceNote(r)} -- below floor: a ceiling under ` + + `${CEILING_FLOOR_SECONDS}s is never red` + ); case 'partial': return ` -- ${r.name}: NOT MEASURED -- a part of its suite is missing from this run${sliceNote(r)}; a part is never graded as the whole`; case 'new': @@ -395,18 +472,22 @@ export function renderCeilingVerdict(report, label) { const basis = `ceilings from ${DATASET_REL}${report.table.measuredAt ? ` (refreshed ${report.table.measuredAt})` : ''}: ` + `each package's slowest executed run x ${report.table.headroom ?? '?'} when first set, held since`; + const belowFloor = report.rows.filter((r) => r.status === 'below-floor').length; + const floorNote = belowFloor ? `; ${belowFloor} below the ${CEILING_FLOOR_SECONDS}s floor, never red` : ''; + const note = `\n ${timingModeNote(event)}.`; if (red.length === 0) { const verdict = graded.length === 0 ? 'NOT MEASURED' : 'OK'; const summaryLine = graded.length === 0 - ? `${head}: NOT MEASURED -- ${label}: no executed package carried a ceiling to grade against (${report.rows.length} executed).` - : `${head}: OK -- ${label}: ${graded.length} package(s) graded, every one within its ceiling.`; + ? `${head}: NOT MEASURED -- ${label}: no executed package carried a ceiling at or above the ` + + `${CEILING_FLOOR_SECONDS}s floor to grade against (${report.rows.length} executed${floorNote}).` + : `${head}: OK -- ${label}: ${graded.length} package(s) graded, every one within its ceiling${floorNote}.`; return { verdict, exitCode: 0, out: [], - err: [`${summaryLine}\n ${basis}.${lines.length ? `\n${lines.join('\n')}` : ''}${stale}`], + err: [`${summaryLine}\n ${basis}.${lines.length ? `\n${lines.join('\n')}` : ''}${stale}${note}`], }; } @@ -417,25 +498,37 @@ export function renderCeilingVerdict(report, label) { `(${(r.seconds / r.ceiling).toFixed(2)}x; ${ceilingText(r)})${sliceNote(r)}. Its slowest test files:\n${filesText(r)}` ) .join('\n'); + const reading = (r) => + `${r.name}: ${r.seconds.toFixed(2)}s, over its ${r.ceiling.toFixed(2)}s ceiling. Slowest file: ` + + `${r.files[0] ? `${r.files[0].file} ${(r.files[0].ms / 1000).toFixed(2)}s` : '(none captured)'}.`; + const text = + `${head}: OVER -- ${label}: ${red.length} package(s) over their suite-duration ceiling.\n` + + `${detail}\n` + + ` ${basis}.\n` + + ' Make the suite faster, or take the raise to a ruling: a ceiling rises only by an entry in\n' + + ' RULED_CEILING_RAISES (scripts/check-test-suite-ceilings.mjs) naming the ruling comment. ⛔ Never\n' + + ` hand-edit \`ceilings\` in ${DATASET_REL} -- it is generated, and the next refresh holds what it finds.\n` + + ` Every executed package this run:\n${lines.join('\n')}${stale}${note}`; + if (mode === 'report') { + return { + verdict: 'OVER', + exitCode: 0, + out: red.map( + (r) => + `::warning title=Suite-duration ceiling::${escapeWorkflowCommandMessage( + `${reading(r)} Report only on a \`${event}\` run: this gate exit-grades on \`${TIMING_GRADED_EVENT}\` ` + + 'runs alone, the profile its ceilings were measured on, so a slowdown that is real reds the next ' + + 'hourly run on main.' + )}` + ), + err: [text], + }; + } return { verdict: 'OVER', exitCode: 1, - out: red.map( - (r) => - `::error title=Suite-duration ceiling::${escapeWorkflowCommandMessage( - `${r.name}: ${r.seconds.toFixed(2)}s, over its ${r.ceiling.toFixed(2)}s ceiling. Slowest file: ` + - `${r.files[0] ? `${r.files[0].file} ${(r.files[0].ms / 1000).toFixed(2)}s` : '(none captured)'}.` - )}` - ), - err: [ - `${head}: OVER -- ${label}: ${red.length} package(s) over their suite-duration ceiling.\n` + - `${detail}\n` + - ` ${basis}.\n` + - ' Make the suite faster, or take the raise to a ruling: a ceiling rises only by an entry in\n' + - ' RULED_CEILING_RAISES (scripts/check-test-suite-ceilings.mjs) naming the ruling comment. ⛔ Never\n' + - ` hand-edit \`ceilings\` in ${DATASET_REL} -- it is generated, and the next refresh holds what it finds.\n` + - ` Every executed package this run:\n${lines.join('\n')}${stale}`, - ], + out: red.map((r) => `::error title=Suite-duration ceiling::${escapeWorkflowCommandMessage(reading(r))}`), + err: [text], }; } @@ -541,11 +634,13 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'file-sliced packages are summed across shards': 6, 'a ruled raise lifts a ceiling, and only a well-formed one does': 8, 'a malformed ceiling table is refused, never read as a pass': 4, + 'the absolute floor: a ceiling under CEILING_FLOOR_SECONDS is never red, and prints "below floor"': 8, + 'off `schedule` the gate reports, never reds (C2)': 8, }); // Deleting an entry silences that battery's floor as effectively as zeroing // it, so the roster's own size is pinned too. -const SELF_TEST_BATTERY_FLOOR = 7; +const SELF_TEST_BATTERY_FLOOR = 9; const UNATTRIBUTED_BATTERY = '(no battery open)'; @@ -594,9 +689,11 @@ function selfTest() { provenance: { measuredAt: '2026-10-12', ceilingHeadroom: 1.25 }, ...extra, }); - const run = (captures, ds = dataset(), raises = {}) => { + // `event` defaults to the graded one, so every battery above C2's reads the + // exit-graded verdict it was written against. + const run = (captures, ds = dataset(), raises = {}, event = TIMING_GRADED_EVENT) => { const report = gradeCeilings({ merged: foldCaptures(captures), dataset: ds, raises }); - return { report, ...renderCeilingVerdict(report, 'Test Core') }; + return { report, ...renderCeilingVerdict(report, 'Test Core', event) }; }; const status = (r, name) => r.report.rows.find((row) => row.name === name)?.status; const raiseUrl = 'https://github.com/objectstack-ai/objectstack/issues/1#issuecomment-2'; @@ -849,6 +946,132 @@ function selfTest() { if (!threw(() => readCeilingTable({ ceilings: {} }))) throw new Error('table: a dataset with no `packages` was accepted'); }); + battery('the absolute floor: a ceiling under CEILING_FLOOR_SECONDS is never red, and prints "below floor"'); + // The ruling's two readings: a 3.40 s ceiling (knowledge-ragflow's) crossed + // at 4.2 s, and the lowest ceiling above the floor, 22.71 s, crossed at 25 s. + const floorSet = dataset({ + packages: { ...dataset().packages, tiny: 2.72, small: 18.17, edge: 16 }, + ceilings: { ...dataset().ceilings, tiny: 3.4, small: 22.71, edge: CEILING_FLOOR_SECONDS }, + }); + check(() => { + if (!(CEILING_FLOOR_SECONDS >= 20)) throw new Error(`floor: ${CEILING_FLOOR_SECONDS}s is below the ruled minimum of 20s`); + }); + check(() => { + const r = run([capture('1/6', [['tiny', 4.2], ['a', 10]])], floorSet); + if (r.exitCode !== 0 || r.verdict !== 'OK' || status(r, 'tiny') !== 'below-floor' || r.out.length !== 0) { + throw new Error(`floor: a 3.40s ceiling run at 4.20s read ${status(r, 'tiny')} / ${r.verdict}, exit ${r.exitCode}`); + } + }); + check(() => { + const r = run([capture('1/6', [['tiny', 4.2], ['a', 10]])], floorSet); + if (!r.err[0].includes('tiny: 4.20s against 3.40s (1.24x) -- below floor') || !r.err[0].includes('1 below the 20s floor')) { + throw new Error(`floor: the line does not say "below floor":\n${r.err[0]}`); + } + }); + check(() => { + // Off schedule too: below the floor is never a warning either. + const r = run([capture('1/6', [['tiny', 4.2], ['a', 10]])], floorSet, {}, 'merge_group'); + if (r.exitCode !== 0 || status(r, 'tiny') !== 'below-floor' || r.out.length !== 0) { + throw new Error(`floor: a below-floor package annotated off schedule: ${JSON.stringify(r.out)}`); + } + }); + check(() => { + const r = run([capture('1/6', [['small', 25]])], floorSet); + if (r.exitCode !== 1 || r.verdict !== 'OVER' || !r.out.some((l) => l.startsWith('::error title=Suite-duration ceiling::small: 25.00s, over its 22.71s ceiling'))) { + throw new Error(`floor: a 22.71s ceiling run at 25s on schedule was not red: ${r.verdict} ${JSON.stringify(r.out)}`); + } + }); + check(() => { + const r = run([capture('1/6', [['small', 25]])], floorSet, {}, 'merge_group'); + const errors = r.out.filter((l) => l.startsWith('::error')); + if (r.exitCode !== 0 || errors.length || r.out.length !== 1 || !r.out[0].startsWith('::warning title=Suite-duration ceiling::small: 25.00s')) { + throw new Error(`floor: a 22.71s ceiling run at 25s off schedule was not a warning alone: exit ${r.exitCode} ${JSON.stringify(r.out)}`); + } + }); + check(() => { + // AT the floor is graded (strict `<`), and so is a below-floor ceiling a ruled raise lifts to it. + const at = run([capture('1/6', [['edge', CEILING_FLOOR_SECONDS + 1]])], floorSet); + const raised = run([capture('1/6', [['tiny', 26]])], floorSet, { tiny: { seconds: 25, ruling: raiseUrl } }); + if (status(at, 'edge') !== 'over' || at.exitCode !== 1 || status(raised, 'tiny') !== 'over' || raised.exitCode !== 1) { + throw new Error(`floor: AT the floor read ${status(at, 'edge')}, a raised tiny read ${status(raised, 'tiny')}`); + } + }); + check(() => { + // Alone, a below-floor package grades nothing: NOT MEASURED, naming the floor. + const r = run([capture('1/6', [['tiny', 400]])], floorSet); + if (r.exitCode !== 0 || r.verdict !== 'NOT MEASURED' || !r.err[0].includes('at or above the 20s floor')) { + throw new Error(`floor: a below-floor package alone read ${r.verdict}:\n${r.err[0]}`); + } + }); + + battery('off `schedule` the gate reports, never reds (C2)'); + const offSchedule = ['merge_group', 'pull_request', 'push', 'workflow_dispatch']; + const twoOver = [capture('1/6', [['a', 151], ['b', 90]], [file('a', 'slow.test.ts', 90_000)])]; + check(() => { + for (const event of offSchedule) { + const r = run(twoOver, dataset(), {}, event); + const warnings = r.out.filter((l) => l.startsWith('::warning title=Suite-duration ceiling::')); + if (r.exitCode !== 0 || r.verdict !== 'OVER' || warnings.length !== 2 || r.out.length !== 2) { + throw new Error(`C2: two packages over on ${event} read ${r.verdict}, exit ${r.exitCode}, ${JSON.stringify(r.out)}`); + } + } + }); + check(() => { + const r = run(twoOver, dataset(), {}, 'merge_group'); + if (r.out.some((l) => l.startsWith('::error')) || r.out.some((l) => l.includes('\n')) || !r.out.every((l) => l.includes('merge_group'))) { + throw new Error(`C2: an off-schedule annotation is an error, spans lines, or does not name the event: ${JSON.stringify(r.out)}`); + } + }); + check(() => { + const r = run(twoOver); + if (r.exitCode !== 1 || r.out.length !== 2 || !r.out.every((l) => l.startsWith('::error title=Suite-duration ceiling::'))) { + throw new Error(`C2: two packages over on schedule were not red: exit ${r.exitCode} ${JSON.stringify(r.out)}`); + } + }); + check(() => { + // The SAME table on every event: only the mode line differs. + const body = (r) => r.err[0].split('\n').slice(0, -1).join('\n'); + const graded = run(twoOver); + const reported = run(twoOver, dataset(), {}, 'pull_request'); + if (body(graded) !== body(reported) || !body(graded).includes('Every executed package this run')) { + throw new Error(`C2: the report-only table is not the graded one:\n${graded.err[0]}\n---\n${reported.err[0]}`); + } + if (!graded.err[0].endsWith('the profile the baseline was measured on.') || !reported.err[0].endsWith('the step exits 0.')) { + throw new Error(`C2: the mode line is missing:\n${graded.err[0]}\n---\n${reported.err[0]}`); + } + }); + check(() => { + // Within every ceiling, off schedule: OK, no annotation, the mode said. + const r = run([capture('1/6', [['a', 10]])], dataset(), {}, 'push'); + if (r.exitCode !== 0 || r.verdict !== 'OK' || r.out.length !== 0 || !r.err[0].includes('report only: a `push` run')) { + throw new Error(`C2: an OK push run read ${r.verdict}:\n${r.err[0]}`); + } + }); + check(() => { + // A refusal is not a timing reading: red on every event. + const r = run([capture('1/6', [['a', 180]])], dataset(), { a: { seconds: 200, ruling: 'no' } }, 'merge_group'); + if (r.exitCode !== 1 || r.verdict !== 'REFUSED') throw new Error(`C2: a malformed raise record off schedule read ${r.verdict}`); + }); + check(() => { + // No table yet is unchanged off schedule: NOT MEASURED with its warning. + const { ceilings: _x, uncapped: _y, ...noTable } = dataset(); + const r = run([capture('1/6', [['a', 9999]])], noTable, {}, 'merge_group'); + if (r.exitCode !== 0 || r.verdict !== 'NOT MEASURED' || !r.out.some((l) => l.startsWith('::warning title=Suite-duration ceilings::NOT MEASURED'))) { + throw new Error(`C2: no table off schedule read ${r.verdict}`); + } + }); + check(() => { + // A missing event is refused, in the renderer and at the command line, never defaulted. + const report = gradeCeilings({ merged: foldCaptures([capture('1/6', [['a', 151]])]), dataset: dataset(), raises: {} }); + if (!threw(() => renderCeilingVerdict(report, 'Test Core'))) throw new Error('C2: a render with no event was graded'); + if (!threw(() => parseCeilingArgs(['--captures', 'd']))) throw new Error('C2: --captures with no --event was accepted'); + if (!threw(() => parseCeilingArgs(['--captures', 'd', '--event', '']))) throw new Error('C2: an empty --event was accepted'); + const parsed = parseCeilingArgs(['--captures', 'd', '--label', 'L', '--event', 'merge_group']); + if (parsed.event !== 'merge_group' || parsed.capturesDir !== 'd' || parsed.label !== 'L') { + throw new Error(`C2: the arguments parsed as ${JSON.stringify(parsed)}`); + } + }); + // The floor: every declared battery ran, and ran its cases. const floorFailures = []; const declared = Object.keys(SELF_TEST_BATTERIES); @@ -883,6 +1106,25 @@ function selfTest() { // ── Entry point ─────────────────────────────────────────────────────────── +/** + * The grading run's arguments. `--event` is required (C2): it decides whether + * a package over its ceiling exits 1 (`schedule`) or is a warning (any other + * event), and a missing one is refused by timingGateMode, never defaulted. + */ +export function parseCeilingArgs(argv) { + let capturesDir = null; + let label = 'Test Core'; + let event = null; + for (let i = 0; i < argv.length; i++) { + if (argv[i] === '--captures') capturesDir = argv[++i]; + else if (argv[i] === '--label') label = argv[++i]; + else if (argv[i] === '--event') event = argv[++i]; + else throw new Error(`unrecognized argument: ${argv[i]}`); + } + if (capturesDir) timingGateMode(event); + return { capturesDir, label, event }; +} + function main() { const argv = process.argv.slice(2); if (argv.includes('--self-test')) { @@ -898,15 +1140,9 @@ function main() { return; } - let capturesDir = null; - let label = 'Test Core'; - for (let i = 0; i < argv.length; i++) { - if (argv[i] === '--captures') capturesDir = argv[++i]; - else if (argv[i] === '--label') label = argv[++i]; - else throw new Error(`unrecognized argument: ${argv[i]}`); - } + const { capturesDir, label, event } = parseCeilingArgs(argv); if (!capturesDir) { - console.error('usage: check-test-suite-ceilings.mjs --captures [--label ] | --self-test'); + console.error('usage: check-test-suite-ceilings.mjs --captures --event [--label ] | --self-test'); process.exit(1); } @@ -914,7 +1150,7 @@ function main() { const merged = foldCaptures(captures); const dataset = JSON.parse(readFileSync(DATASET_PATH, 'utf8')); const report = gradeCeilings({ merged, dataset }); - const rendered = renderCeilingVerdict(report, label); + const rendered = renderCeilingVerdict(report, label, event); for (const line of rendered.out) console.log(line); for (const line of rendered.err) console.error(line); // Not a verdict of their own: a shard capture that could not be read leaves diff --git a/scripts/partition-test-shards.mjs b/scripts/partition-test-shards.mjs index 52581cb6eaf..2c1bcf62605 100644 --- a/scripts/partition-test-shards.mjs +++ b/scripts/partition-test-shards.mjs @@ -75,7 +75,7 @@ // node scripts/partition-test-shards.mjs --shard N/M \ // [--exclude ]... // node scripts/partition-test-shards.mjs --check-drift ... \ -// [--label ] +// --event [--label ] // node scripts/partition-test-shards.mjs --self-test // // `--check-drift` is the half that keeps the dataset honest AFTER it is @@ -83,7 +83,8 @@ // run it just finished has written the measured truth to `.turbo/runs/`; this // mode reads that back, compares it to what this script PREDICTED for the same // packages, raises a `::warning::` past WARN_MEASURED_OVER_PREDICTED, and reds -// past MAX_MEASURED_OVER_PREDICTED. Without it the dataset +// past MAX_MEASURED_OVER_PREDICTED -- on a `schedule` run; on every other event +// that red is a `::warning::` and exits 0 (TIMING_GRADED_EVENT, C2). Without it the dataset // rots silently in one direction and the only instrument that notices is a // shard killed by the job timeout -- which is a shard that produced NO reading // while the rollup read green. @@ -200,6 +201,54 @@ export const MAX_MEASURED_OVER_PREDICTED = 1.5; // the warning cannot be made unreachable by moving either end. export const WARN_MEASURED_OVER_PREDICTED = 1.3; +// ── WHERE THE TWO TIMING GATES EXIT-GRADE: `schedule` ALONE (C2) ────────── +// +// Ruling (letter C2, for both timing steps): +// https://github.com/objectstack-ai/objectstack/issues/22949#issuecomment-6119186511 +// "a timing gate exit-grades only on the run profile its baseline was measured +// on; everywhere else it reports." +// +// Both timing gates read one baseline: the weights `--check-drift` predicts +// from, and the ceilings scripts/check-test-suite-ceilings.mjs grades against. +// The refresh lane measures it on ci.yml's hourly `schedule` runs and no others +// (DEFAULT_RUN_EVENT in scripts/ci/select-shard-timings-run.mjs). A +// `merge_group` or `pull_request` build executes far more of the workspace on +// the same runners. On the morning the refreshed dataset went live this step +// redded 3 of 19 queue builds and the heads of three PRs (#22953, #22954, +// #22959, filed as #22966) at 1.56-1.62x, on packages none of them touched, +// while every hourly run read green. So both gates take the run's event +// (`--event "$GITHUB_EVENT_NAME"` in ci.yml) and: +// +// schedule exit-graded, exactly as before: a red exits 1 +// every other REPORT ONLY (merge_group, pull_request, push, +// event workflow_dispatch): the same table prints, each red becomes a +// `::warning::` annotation, and the step exits 0 +// +// A real slowdown merged to `main` reds the next hourly run. A missing or empty +// `--event` is refused, never defaulted: a step that lost its event would +// otherwise grade everywhere or nowhere, and say nothing either way. +export const TIMING_GRADED_EVENT = 'schedule'; + +/** 'grade' on TIMING_GRADED_EVENT, 'report' on any other event; a missing event is refused. */ +export function timingGateMode(event) { + if (typeof event !== 'string' || event.trim() === '') { + throw new Error( + `the run's event is ${JSON.stringify(event ?? null)}. The timing gates exit-grade on ` + + `\`${TIMING_GRADED_EVENT}\` alone and report on every other event, so they need it: pass ` + + '`--event "$GITHUB_EVENT_NAME"`. A missing event is refused rather than guessed.' + ); + } + return event === TIMING_GRADED_EVENT ? 'grade' : 'report'; +} + +/** The one line both timing gates print to say which of the two modes this run is in. */ +export function timingModeNote(event) { + return timingGateMode(event) === 'grade' + ? `exit-graded: a \`${event}\` run is the profile the baseline was measured on` + : `report only: a \`${event}\` run is not the profile the baseline was measured on ` + + `(\`${TIMING_GRADED_EVENT}\`), so a red reading is a warning here and the step exits 0`; +} + // ── SHARDING ONE PACKAGE BELOW PACKAGE GRANULARITY (#16173) ──────────────── // // The floor argument at the top of this file is not a caveat, it is a wall: a @@ -1082,7 +1131,12 @@ export function escapeWorkflowCommandMessage(text) { return String(text).replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A'); } -export function renderDriftVerdict(report, label) { +// +// `event` is the run's (ci.yml passes `$GITHUB_EVENT_NAME`): off +// TIMING_GRADED_EVENT a DRIFT prints the same text, becomes a `::warning::` +// annotation, and exits 0 (C2, see timingGateMode). +export function renderDriftVerdict(report, label, event) { + const mode = timingGateMode(event); const skipped = report.unpredicted.length === 0 ? '' @@ -1115,25 +1169,37 @@ export function renderDriftVerdict(report, label) { `(${ratioOf(r)}${r.overshoot >= 0 ? '+' : ''}${r.overshoot.toFixed(1)}s)` ) .join('\n'); + const note = `\n ${timingModeNote(event)}.`; if (report.drifted) { - return { - verdict: 'DRIFT', - exitCode: 1, - out: [], - err: [ - `shard-timing-drift: DRIFT -- ${label}, ${head}.${skipped}\n` + - ' Heaviest overshoots:\n' + - `${worst}\n` + - ' scripts/test-shard-timings.json no longer describes this workspace, so the shard split\n' + - ' is balancing a quantity that is not the runtime. Refresh it -- see\n' + - ' scripts/measure-test-shard-timings.mjs for the two refresh paths -- and ⛔ do NOT\n' + - ' hand-edit the dataset or raise this bound to absorb the gap. Expect the refresh to red\n' + - " this script's own balance pins if a single suite has outgrown the acceptance bound:\n" + - ' that is those pins working, and the remedy they name is splitting that suite below\n' + - ' package granularity, never a different shard count.', - ], - }; + const text = + `shard-timing-drift: DRIFT -- ${label}, ${head}.${skipped}\n` + + ' Heaviest overshoots:\n' + + `${worst}\n` + + ' scripts/test-shard-timings.json no longer describes this workspace, so the shard split\n' + + ' is balancing a quantity that is not the runtime. Refresh it -- see\n' + + ' scripts/measure-test-shard-timings.mjs for the two refresh paths -- and ⛔ do NOT\n' + + ' hand-edit the dataset or raise this bound to absorb the gap. Expect the refresh to red\n' + + " this script's own balance pins if a single suite has outgrown the acceptance bound:\n" + + ' that is those pins working, and the remedy they name is splitting that suite below\n' + + ` package granularity, never a different shard count.${note}`; + if (mode === 'report') { + const top = report.rows[0]; + return { + verdict: 'DRIFT', + exitCode: 0, + out: [ + `::warning title=Test Core shard timing drift::${escapeWorkflowCommandMessage( + `${label}: ${head}, past the red bound. Heaviest overshoot: ${top.name}, predicted ` + + `${top.predicted.toFixed(1)}s, measured ${top.measured.toFixed(1)}s. Report only on a \`${event}\` run: ` + + `this step exit-grades on \`${TIMING_GRADED_EVENT}\` runs alone, the profile ` + + 'scripts/test-shard-timings.json was measured on, so a drift that is real reds the next hourly run on main.' + )}`, + ], + err: [text], + }; + } + return { verdict: 'DRIFT', exitCode: 1, out: [], err: [text] }; } if (report.warned) { @@ -1149,7 +1215,7 @@ export function renderDriftVerdict(report, label) { 'Not a failure: this step stays green until the red bound.' )}`, ], - err: [`shard-timing-drift: WARN -- ${label}, ${head}.${skipped}\n Heaviest overshoots:\n${worst}`], + err: [`shard-timing-drift: WARN -- ${label}, ${head}.${skipped}\n Heaviest overshoots:\n${worst}${note}`], }; } @@ -1157,7 +1223,7 @@ export function renderDriftVerdict(report, label) { verdict: 'OK', exitCode: 0, out: [], - err: [`shard-timing-drift: OK -- ${label}, ${head}.${skipped}`], + err: [`shard-timing-drift: OK -- ${label}, ${head}.${skipped}${note}`], }; } @@ -1249,11 +1315,12 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'file-level slice items (#16173)': 20, 'file-level slices reach vitest through OS_TEST_SHARD (#19278)': 11, 'density cap and the per-run slice decision (#22075)': 14, + 'the drift step exit-grades on `schedule` alone, and reports everywhere else (C2)': 8, }); // DELETING an entry silences that battery's floor exactly as effectively as // zeroing it, so the roster's own size is pinned too. -const SELF_TEST_BATTERY_FLOOR = 12; +const SELF_TEST_BATTERY_FLOOR = 13; // The key an assertion is filed under when no battery is open. It is not a // declared battery, so it reds by the same set difference rather than silently @@ -1898,7 +1965,7 @@ function selfTest() { // with a workflow command would mint a real annotation on every lint run). const label = 'Test Core (9/9)'; check(() => { - const v = renderDriftVerdict(tierOf(131), label); + const v = renderDriftVerdict(tierOf(131), label, TIMING_GRADED_EVENT); const commands = v.out.filter((line) => line.startsWith('::')); if (v.verdict !== 'WARN' || v.exitCode !== 0 || commands.length !== 1) { throw new Error( @@ -1910,19 +1977,19 @@ function selfTest() { } }); check(() => { - const v = renderDriftVerdict(tierOf(151), label); + const v = renderDriftVerdict(tierOf(151), label, TIMING_GRADED_EVENT); if (v.verdict !== 'DRIFT' || v.exitCode !== 1 || v.out.some((line) => line.startsWith('::'))) { throw new Error(`drift warning: a red rendered ${v.verdict} exit ${v.exitCode}, ${v.out.length} stdout line(s)`); } }); check(() => { for (const [seconds, expected] of [[118, 'OK'], [WARN_MEASURED_OVER_PREDICTED * 100, 'OK']]) { - const v = renderDriftVerdict(tierOf(seconds), label); + const v = renderDriftVerdict(tierOf(seconds), label, TIMING_GRADED_EVENT); if (v.verdict !== expected || v.exitCode !== 0 || v.out.length !== 0) { throw new Error(`drift warning: ${seconds / 100}x rendered ${v.verdict} with ${v.out.length} stdout line(s)`); } } - const nm = renderDriftVerdict(driftReport(replayed.samples, driftTimings), label); + const nm = renderDriftVerdict(driftReport(replayed.samples, driftTimings), label, TIMING_GRADED_EVENT); if (nm.verdict !== 'NOT MEASURED' || nm.exitCode !== 0 || nm.out.length !== 0) { throw new Error(`drift warning: a replay-only shard rendered ${nm.verdict}`); } @@ -1936,6 +2003,85 @@ function selfTest() { } }); + // -- EXIT-GRADED ON `schedule` ALONE (C2) -------------------------------- + // + // The baseline is measured on hourly `schedule` runs, so only those exit 1 + // on a DRIFT; every other event prints the same text, annotates a warning + // and exits 0. Read as rendered text, like the tier above: printing a + // `::warning::` from here would mint a real annotation on every lint run. + battery('the drift step exit-grades on `schedule` alone, and reports everywhere else (C2)'); + const offSchedule = ['merge_group', 'pull_request', 'push', 'workflow_dispatch']; + + check(() => { + if (TIMING_GRADED_EVENT !== 'schedule' || timingGateMode('schedule') !== 'grade') { + throw new Error(`C2: the graded event is ${TIMING_GRADED_EVENT}, and schedule reads ${timingGateMode('schedule')}`); + } + }); + check(() => { + const graded = offSchedule.filter((e) => timingGateMode(e) !== 'report'); + if (graded.length) throw new Error(`C2: ${graded.join(', ')} exit-grade, but only schedule may`); + }); + // A step that lost its event is refused, never defaulted to either mode. + check(() => { + const accepted = [undefined, null, '', ' '].filter((e) => !threw(() => timingGateMode(e))); + if (accepted.length) throw new Error(`C2: a missing event was read as a mode: ${JSON.stringify(accepted)}`); + if (!threw(() => renderDriftVerdict(tierOf(151), label))) throw new Error('C2: a render with no event was graded'); + }); + check(() => { + const v = renderDriftVerdict(tierOf(151), label, 'schedule'); + if (v.verdict !== 'DRIFT' || v.exitCode !== 1 || v.out.length !== 0 || !v.err[0].includes('exit-graded')) { + throw new Error(`C2: a schedule DRIFT rendered ${v.verdict} exit ${v.exitCode}, ${v.out.length} stdout line(s)`); + } + }); + check(() => { + for (const event of offSchedule) { + const v = renderDriftVerdict(tierOf(151), label, event); + const commands = v.out.filter((line) => line.startsWith('::')); + if (v.verdict !== 'DRIFT' || v.exitCode !== 0 || commands.length !== 1) { + throw new Error(`C2: a ${event} DRIFT rendered ${v.verdict} exit ${v.exitCode} with ${commands.length} command(s)`); + } + if (!commands[0].startsWith('::warning ') || !commands[0].includes(label) || !commands[0].includes(event) || commands[0].includes('\n')) { + throw new Error(`C2: a ${event} DRIFT is not one ::warning line naming the shard and the event: ${commands[0]}`); + } + } + }); + // The SAME table on every event: only the mode line differs. + check(() => { + const body = (v) => v.err[0].split('\n').slice(0, -1).join('\n'); + const graded = renderDriftVerdict(tierOf(151), label, 'schedule'); + const reported = renderDriftVerdict(tierOf(151), label, 'merge_group'); + if (body(graded) !== body(reported) || !body(graded).includes('Heaviest overshoots:')) { + throw new Error(`C2: the report-only text is not the graded text:\n${graded.err[0]}\n---\n${reported.err[0]}`); + } + if (!reported.err[0].endsWith('so a red reading is a warning here and the step exits 0.')) { + throw new Error(`C2: the report-only text does not say so:\n${reported.err[0]}`); + } + }); + check(() => { + // The tiers under the red are unchanged off schedule: WARN annotates, OK and NOT MEASURED say nothing on stdout. + const warn = renderDriftVerdict(tierOf(131), label, 'pull_request'); + const ok = renderDriftVerdict(tierOf(118), label, 'push'); + const nm = renderDriftVerdict(driftReport(replayed.samples, driftTimings), label, 'merge_group'); + if (warn.verdict !== 'WARN' || warn.exitCode !== 0 || warn.out.length !== 1 || !warn.out[0].startsWith('::warning ')) { + throw new Error(`C2: a pull_request WARN rendered ${warn.verdict} exit ${warn.exitCode}`); + } + if (ok.verdict !== 'OK' || ok.exitCode !== 0 || ok.out.length !== 0 || !ok.err[0].includes('report only')) { + throw new Error(`C2: a push OK rendered ${ok.verdict} exit ${ok.exitCode}:\n${ok.err[0]}`); + } + if (nm.verdict !== 'NOT MEASURED' || nm.exitCode !== 0 || nm.out.length !== 0) { + throw new Error(`C2: a merge_group replay-only shard rendered ${nm.verdict}`); + } + }); + // The CLI: `--event` is read, and its absence is refused before any summary is. + check(() => { + const parsed = parseDriftArgs(['--check-drift', 'a.json', '--label', 'L', '--event', 'merge_group']); + if (parsed.event !== 'merge_group' || parsed.inputs.join() !== 'a.json' || parsed.label !== 'L') { + throw new Error(`C2: --check-drift parsed as ${JSON.stringify(parsed)}`); + } + if (!threw(() => parseDriftArgs(['--check-drift', 'a.json']))) throw new Error('C2: --check-drift with no --event was accepted'); + if (!threw(() => parseDriftArgs(['--check-drift', 'a.json', '--event', '']))) throw new Error('C2: an empty --event was accepted'); + }); + // -- FILE-LEVEL SLICE ITEMS (#16173) ------------------------------------ // // The grammar, its refusals, and the two joins that read it. The balancing @@ -2474,19 +2620,31 @@ function selfTest() { // test tasks were all cache replays has said nothing about the dataset, and // reporting that as OK is the #4690 shape -- a check that read nothing // reporting as a check that found nothing wrong. -function checkDrift(argv) { +// +// `--event` is required (C2): the run's event decides whether a DRIFT exits 1 +// (`schedule`) or is reported as a warning (every other event), and a missing +// one is refused by timingGateMode rather than defaulted either way. +export function parseDriftArgs(argv) { const inputs = []; let label = 'this shard'; + let event = null; for (let i = 0; i < argv.length; i++) { const arg = argv[i]; if (arg === '--check-drift') continue; else if (arg === '--label') label = argv[++i]; + else if (arg === '--event') event = argv[++i]; else if (arg.startsWith('--')) throw new Error(`unrecognized argument: ${arg}`); else inputs.push(arg); } + timingGateMode(event); + return { inputs, label, event }; +} + +function checkDrift(argv) { + const { inputs, label, event } = parseDriftArgs(argv); if (inputs.length === 0) { console.error( - 'usage: partition-test-shards.mjs --check-drift ... [--label ]' + 'usage: partition-test-shards.mjs --check-drift ... --event [--label ]' ); process.exit(1); } @@ -2515,7 +2673,7 @@ function checkDrift(argv) { } const timings = loadTimings(); const report = driftReport(merged, timings, MAX_MEASURED_OVER_PREDICTED, observedSlices); - const rendered = renderDriftVerdict(report, label); + const rendered = renderDriftVerdict(report, label, event); for (const line of rendered.out) console.log(line); for (const line of rendered.err) console.error(line); if (rendered.exitCode !== 0) process.exit(rendered.exitCode);