From 1d1a18064dca926833b05ab3f6c4b5388e188fe2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 02:59:32 +0000 Subject: [PATCH 1/3] fix(runtime): strip read-time decorations before the route-level seed apply's closed parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /packages/:id/publish-drafts` reads each just-published `seed` body back through `protocol.getMetaItem` and hands it to `SeedLoaderRequestSchema`. That read exits through `decorateMetadataItem`, which stamps `_diagnostics` on every body whose type has a registered schema, and `SeedSchema` is closed — so the door refused the document it had just served, on a 200, as `seedApplied.error`: zero rows loaded and the author told their seed body failed spec validation. The direction is settled by the contract, not by judgement. `METADATA_READ_DECORATIONS` declares `_diagnostics` a key the read path derives and attaches to the response, and its module names "any re-parse of a served document" as a consumer that must strip. `_packageId` is deliberately NOT a member and `SeedSchema` accepts it via `MetadataProtectionFields` — measured on the real producer, the served body carries both and the schema refuses exactly one. So the fix is the declared helper, not a widened schema and not the export path's blanket underscore strip, which would drop provenance this schema allowlists on purpose. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- .../seed-apply-read-back-decorations.md | 13 + ...ckages-seed-apply-read-decorations.test.ts | 430 ++++++++++++++++++ packages/runtime/src/domains/packages.ts | 38 +- 3 files changed, 480 insertions(+), 1 deletion(-) create mode 100644 .changeset/seed-apply-read-back-decorations.md create mode 100644 packages/runtime/src/domains/packages-seed-apply-read-decorations.test.ts diff --git a/.changeset/seed-apply-read-back-decorations.md b/.changeset/seed-apply-read-back-decorations.md new file mode 100644 index 0000000000..07f7fa8b47 --- /dev/null +++ b/.changeset/seed-apply-read-back-decorations.md @@ -0,0 +1,13 @@ +--- +"@objectstack/runtime": patch +--- + +The package-publish door's route-level seed apply can consume the platform's own read-back envelope again. + +`POST /packages/:id/publish-drafts` reads each just-published `seed` body back through `protocol.getMetaItem` before handing it to the seed loader. That read exits through `decorateMetadataItem`, which stamps `_diagnostics` on every body whose metadata type has a registered schema — `seed` has one — and `SeedSchema` has been closed since protocol 17. So the door refused the document it had just been served: `unrecognized_keys: ["_diagnostics"]`, minted as a 422 and delivered on a **200** as `seedApplied.error`. Zero rows loaded, and the author was told their seed body failed spec validation when nothing about it was wrong. + +The read-back is now passed through `stripReadDecorations` at the unwrap — the same helper, for the same reason, that the dataset query, the cold-boot flow bind and `saveMetaItem`'s verbatim persist already call. `METADATA_READ_DECORATIONS` is the declared list of keys the read path derives from a document and attaches to the *response*, so removing them restores the document the author actually wrote. + +Nothing is widened to accept them: `SeedLoaderRequestSchema` stays closed, and the publish response keeps its declared shape. The strip is deliberately **not** a blanket `startsWith('_')` sweep — the ADR-0010 protection envelope (`_packageId`, `_provenance`, …) is not a read decoration, and the metadata schemas allowlist it precisely so a served document keeps its provenance when it is parsed again. + +Only protocols that do not self-apply seeds inside `publishPackageDrafts` reach this path; the shipping protocol self-applies and was never affected. diff --git a/packages/runtime/src/domains/packages-seed-apply-read-decorations.test.ts b/packages/runtime/src/domains/packages-seed-apply-read-decorations.test.ts new file mode 100644 index 0000000000..f58c558707 --- /dev/null +++ b/packages/runtime/src/domains/packages-seed-apply-read-decorations.test.ts @@ -0,0 +1,430 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15591 — the route-level seed apply could not consume the shipping + * protocol's own read-back envelope. + * + * `POST /packages/:id/publish-drafts` reads each just-published `seed` body + * back through `protocol.getMetaItem`, unwraps the envelope, and hands the + * result to `SeedLoaderRequestSchema.safeParse`. `getMetaItem` exits through + * `decorateMetadataItem`, which stamps `_diagnostics` on every body whose type + * has a registered Zod schema — `seed` has one — and `SeedSchema` has been + * CLOSED since #4001. So the door refused the platform's own output, minted + * that refusal as a 422 and delivered it on a **200** as `seedApplied.error`: + * zero rows loaded, and the author told their seed body failed spec validation + * when nothing about it is wrong. + * + * ## Which side was wrong — settled by the contract, not by judgement + * + * The card left the direction open (strip at the consumer, or stop decorating + * at the producer) and warned that the two underscore keys are not one + * population. Measured on the real producer, they are not, and the spec says + * so in both directions: + * + * - `_diagnostics` IS a member of `METADATA_READ_DECORATIONS` + * (`spec/kernel/metadata-read-decorations.ts`), whose module states the rule + * this door was missing: each member "is DERIVED from the document on every + * read, so it belongs to the *response*, never to the document — a served + * body is therefore NOT a valid input to the schema that produced it until + * these are removed", and names "**any** re-parse of a served document" as + * the second class of consumer that must strip. The producer is correct; the + * consumer was missing a declared step. + * - `_packageId` is "deliberately NOT" a member — ADR-0010 envelope state, + * which "the closed metadata schemas allowlist … precisely so a served + * document keeps its provenance on re-parse". `SeedSchema` is one of those: + * it spreads `MetadataProtectionFields` on purpose. §1 measures both. + * + * ⇒ The repair is `stripReadDecorations`, the helper that list ships with — + * the same call, for the same reason, that `rest-server.ts` makes before + * parsing a served `dataset` ("A SERVED document is not a valid input to the + * schema that produced it"), that `service-automation`'s cold-boot flow bind + * makes, and that `saveMetaItem` makes before its verbatim persist. ⛔ NOT a + * widened schema (no `.passthrough()`, no request-contract change), and ⛔ NOT + * the blanket `startsWith('_')` strip `assemblePackageManifest` runs 300 lines + * up: a portable manifest must SHED provenance, a re-parse must KEEP it, and + * §3 is the bound that separates those two rules. + * + * ## How this file is composed, and which half is doubled + * + * The PUBLISH is real: the fixture stages a `state:'draft'` seed row and + * promotes it with the shipping `publishPackageDrafts` on a real + * `ObjectStackProtocolImplementation`. The READ-BACK is real: the same + * protocol instance serves `getMetaItem` over the same engine, so the envelope + * under test is the platform's own output and not a hand-written fixture. The + * loader is the real `SeedLoaderService`. + * + * ONE thing is doubled, and only to reach the code under test at all: the + * route-level apply runs *only* for protocols that do not self-apply seeds + * inside `publishPackageDrafts` ("never run both, or an externalId-less seed + * would double-insert"), and the shipping protocol DOES self-apply — measured + * here, `publishPackageDrafts` answers `seedApplied` present. So the route is + * driven through a facade that reports the published seed without that field: + * the exact population this fallback documents itself as existing for, and a + * DECLARED wire behaviour — `PublishPackageDraftsResponseSchema`'s own note + * says the REST door "back-fills `seedApplied` for custom protocols that do not + * self-apply". That declaration is why the fallback is repaired here rather + * than deleted as dead code. + * + * ## Sections, and which are evidence vs. which are the bound + * + * §0 · positive control — the round trip really runs, and the REAL producer's + * read-back carries the decoration. GREEN before and after: without it, + * §2 going green could mean the decoration simply never arrived. + * §1 · the contract reading that decides the direction, read from `spec` + * rather than restated. GREEN before and after — the instrument. + * §2 · the defect — rows load, and the 200 carries no refusal. RED before. + * §3 · the bound — the strip is the DECLARED list, so `_diagnostics` is gone + * from the body the loader receives and `_packageId` is still on it. RED + * before (nothing reaches the loader at all), and RED under the wrong + * repair (a blanket underscore strip), which is what it exists for. + * + * ⛔ No bare `toThrow()` anywhere here: this door does not throw, it REPORTS on + * a 200, and the whole defect is what the report says. + */ + +import { describe, expect, it, vi } from 'vitest'; +import { + assertEngineDeleteDispatch, + assertEngineFindOnePredicate, + assertEngineUpdateDispatch, +} from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { METADATA_READ_DECORATIONS } from '@objectstack/spec/kernel'; +import { SeedSchema } from '@objectstack/spec/data'; +import { SeedLoaderService } from '../seed-loader.js'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +const PKG = 'com.workspace'; +const SEED = 'project_seed'; + +/** The seed body a publish stores and the read-back must return. */ +const SEED_BODY = { + object: 'project', + externalId: 'name', + mode: 'upsert', + records: [{ name: 'Apollo', status: 'active' }, { name: 'Gemini', status: 'planned' }], +}; + +/** The one key the closed `SeedSchema` refuses on a served body. */ +const DECORATION = '_diagnostics'; + +/** The one key it deliberately ACCEPTS on a served body (ADR-0010). */ +const PROVENANCE = '_packageId'; + +// --------------------------------------------------------------------------- +// Engine double — a plain row store. Every write verb opens with the +// PRODUCER's own dispatch predicate (`check:engine-double-contract`), imported +// from `@objectstack/metadata-core` and never from `@objectstack/objectql` +// (that reverse edge is a cycle turbo refuses), so this double cannot accept a +// call the real ObjectQL engine would refuse. +// --------------------------------------------------------------------------- + +interface Row { id: string; [k: string]: unknown } + +function matches(row: Row, where: Record | undefined): boolean { + if (!where) return true; + for (const [key, cond] of Object.entries(where)) { + if (cond === undefined) continue; + if (key === '$or') { + const branches = cond as Array>; + if (!branches.some((b) => matches(row, b))) return false; + continue; + } + const value = row[key]; + if (cond !== null && typeof cond === 'object') { + const op = cond as Record; + if ('$null' in op) { + if ((value === null || value === undefined) !== (op.$null === true)) return false; + continue; + } + if ('$in' in op) { + if (!(op.$in as unknown[]).includes(value)) return false; + continue; + } + continue; + } + if (cond === null) { + if (value !== null && value !== undefined) return false; + continue; + } + if (value !== cond) return false; + } + return true; +} + +function makeEngine() { + const tables = new Map(); + let nextId = 0; + const tableOf = (name: string): Row[] => { + let t = tables.get(name); + if (!t) { t = []; tables.set(name, t); } + return t; + }; + const engine: any = { + registry: { + listItems: () => [], + getItem: () => undefined, + getObject: () => undefined, + getPackage: () => undefined, + getArtifactItem: () => undefined, + getAllPackages: () => [], + isPackageDisabled: () => false, + applyNavContributions: (app: unknown) => app, + registerItem: () => undefined, + registerObject: () => undefined, + }, + async find(table: string, opts?: { where?: Record, limit?: number }) { + const rows = tableOf(table).filter((r) => matches(r, opts?.where)); + // The caller's bound, applied AFTER the filter and by PRESENCE + // (`check:objectql-double-limit`): a double that silently ignores + // `limit` answers more rows than the real engine would. + return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; + }, + async findOne(table: string, opts?: { where?: Record }) { + assertEngineFindOnePredicate(table, opts); + return tableOf(table).find((r) => matches(r, opts?.where)) ?? null; + }, + async insert(table: string, data: any) { + const one = (d: Record): Row => { + nextId += 1; + const row: Row = { id: (d.id as string) ?? `r_${nextId}`, ...d }; + tableOf(table).push(row); + return row; + }; + return Array.isArray(data) ? data.map(one) : one(data); + }, + async update(table: string, data: Record, opts?: { where?: Record }) { + const dispatch = assertEngineUpdateDispatch(data as any, opts as any); + const rows = tableOf(table); + const target = dispatch.kind === 'by-id' + ? rows.find((r) => r.id === dispatch.id) + : rows.find((r) => matches(r, opts?.where)); + if (target) Object.assign(target, data); + return target ?? null; + }, + async delete(table: string, opts?: { where?: Record }) { + const dispatch = assertEngineDeleteDispatch(opts as any); + const rows = tableOf(table); + const keep = dispatch.kind === 'by-id' + ? rows.filter((r) => r.id !== dispatch.id) + : rows.filter((r) => !matches(r, opts?.where)); + const deleted = rows.length - keep.length; + tables.set(table, keep); + return { deleted }; + }, + async count(table: string, opts?: { where?: Record }) { + return tableOf(table).filter((r) => matches(r, opts?.where)).length; + }, + async aggregate() { return []; }, + async execute() { return undefined; }, + rowsOf: tableOf, + }; + return engine; +} + +/** An authenticated package admin — the route's anonymous-deny + capability floor. */ +const PKG_ADMIN = (): any => ({ + request: { headers: {} }, + environmentId: 'env_1', + executionContext: { + userId: 'u_pkg_admin', + systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], + }, +}); + +/** + * Stage a draft seed, promote it with the REAL `publishPackageDrafts` on a real + * protocol, then drive the route whose fallback reads it back through that same + * protocol. + * + * `SeedLoaderService.prototype.load` is SPIED, not replaced — `callThrough` is + * the default, so the shipping loader still runs and §3 gets an observation + * channel onto the exact body the parse handed it. + */ +async function publishThenApply() { + const engine = makeEngine(); + engine.rowsOf('sys_metadata').push({ + id: 'row_seed_draft', + type: 'seed', + name: SEED, + organization_id: null, + package_id: PKG, + state: 'draft', + metadata: JSON.stringify(SEED_BODY), + }); + + const real = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + + // ── The real publish. Draft → active, through the shipping primitive. ── + const published = await real.publishPackageDrafts({ packageId: PKG }); + + // Rows the publish's OWN self-apply loaded are not what this file measures; + // the route-level apply below must stand on its own. + engine.rowsOf('project').length = 0; + + /** The real read-back, recorded. */ + const getMetaItem = vi.fn(async (request: any) => await real.getMetaItem(request)); + + const loadSpy = vi.spyOn(SeedLoaderService.prototype, 'load'); + + const facade = { + // ⛔ Deliberately reports NO `seedApplied`: the route-level apply runs + // only for protocols that do not self-apply, and that branch is the + // code under test. + publishPackageDrafts: async () => ({ + success: true, + outcome: 'published', + publishedCount: 1, + failedCount: 0, + published: [{ type: 'seed', name: SEED, version: 'h' }], + failed: [], + }), + getMetaItem, + }; + + const services: Record = { + protocol: facade, + objectql: engine, + metadata: { + getObject: async () => ({ + name: 'project', + fields: { name: { type: 'text' }, status: { type: 'select' } }, + }), + }, + auth: { api: { getSession: async () => ({ session: {} }) } }, + }; + const kernel: any = { + getServiceAsync: async (name: string) => services[name] ?? null, + getService: (name: string) => services[name] ?? null, + context: { getService: (name: string) => services[name] ?? null }, + }; + + const result = await new HttpDispatcher(kernel).handlePackages( + `/${PKG}/publish-drafts`, 'POST', {}, {}, PKG_ADMIN(), + ); + expect(result.response?.status).toBe(200); + const body: any = (result.response as any)?.body; + // Read the recorded calls BEFORE restoring: `mockRestore` resets the mock's + // state, so a `loadSpy.mock.calls` read after it answers an empty array — + // which §3 would report as "the loader was never called". + const loadCalls = loadSpy.mock.calls.slice(); + loadSpy.mockRestore(); + return { + engine, + published, + getMetaItem, + body, + seedApplied: body?.data?.seedApplied, + /** The seed bodies the shipping loader actually received. */ + loadedSeeds: () => (loadCalls[0]?.[0] as any)?.seeds, + }; +} + +// ═══════════════════════════════════════════════════════════════════════════ +// §0 — the positive control: the round trip runs, and the REAL producer +// decorates. GREEN before AND after. +// ═══════════════════════════════════════════════════════════════════════════ + +describe('#15591 · 0 · the shipping protocol really serves a decorated body', () => { + it('promotes the draft and reads back an envelope carrying our own annotation', async () => { + const { published, engine, getMetaItem } = await publishThenApply(); + + // The publish is the shipping one, and it really promoted the draft. + expect(published?.publishedCount).toBe(1); + expect(engine.rowsOf('sys_metadata').some( + (r: any) => r.type === 'seed' && r.name === SEED && r.state === 'active', + )).toBe(true); + + // And it SELF-APPLIED — which is exactly why the fallback under test is + // invisible in the shipping composition and why the facade above has to + // withhold the field to reach it at all. + expect(Object.prototype.hasOwnProperty.call(published, 'seedApplied')).toBe(true); + + // The read-back happened, against the real protocol, and the body it + // served carries BOTH underscore keys — inside `.item`, which is the + // branch the door's unwrap takes. Without this control, §2 turning + // green would be indistinguishable from "the decoration never arrived". + expect(getMetaItem).toHaveBeenCalled(); + const served: any = await getMetaItem.mock.results[0]?.value; + expect(served?.item?.object).toBe('project'); + expect(served?.item?.records).toHaveLength(2); + expect(served?.item).toHaveProperty(DECORATION); + expect(served?.item).toHaveProperty(PROVENANCE, PKG); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §1 — the contract reading that decided the direction, read from `spec` +// rather than restated. GREEN before AND after: the instrument. +// ═══════════════════════════════════════════════════════════════════════════ + +describe('#15591 · 1 · the two underscore keys are two populations, and spec says which', () => { + it('`_diagnostics` is a DECLARED read decoration; `_packageId` deliberately is not', () => { + expect(METADATA_READ_DECORATIONS).toContain(DECORATION); + expect(METADATA_READ_DECORATIONS).not.toContain(PROVENANCE); + }); + + it('the closed seed schema refuses the decoration BY NAME and accepts the provenance', () => { + // The refusal, with the key named: this is the `unrecognized_keys` + // issue the door minted as a 422 onto a 200 response. + const decorated: any = (SeedSchema as any).safeParse({ ...SEED_BODY, [DECORATION]: { valid: true } }); + expect(decorated.success).toBe(false); + const issue = decorated.error.issues.find((i: any) => i.code === 'unrecognized_keys'); + expect(issue?.keys).toEqual([DECORATION]); + + // ⇒ and the control that makes that reading mean something: the OTHER + // underscore key parses clean, because `SeedSchema` spreads + // `MetadataProtectionFields` on purpose. A blanket underscore strip + // would be dropping a key this schema declares. + expect((SeedSchema as any).safeParse({ ...SEED_BODY, [PROVENANCE]: PKG }).success).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §2 — the defect. RED before the fix. +// ═══════════════════════════════════════════════════════════════════════════ + +describe('#15591 · 2 · the platform can consume its own read-back envelope', () => { + it('loads the rows instead of refusing the body it just served', async () => { + const { seedApplied, engine } = await publishThenApply(); + + expect(seedApplied?.success).toBe(true); + expect(seedApplied?.inserted).toBe(2); + expect(engine.rowsOf('project')).toHaveLength(2); + }); + + it('the 200 carries no refusal of the author\'s seed body', async () => { + const { seedApplied, body } = await publishThenApply(); + + // The pre-fix payload said the author's input failed spec validation. + // Asserted on the TEXT, not on a vague "it changed": these are the + // exact fragments `seedRequestValidationError` puts on the wire. + expect(seedApplied?.error).toBeUndefined(); + expect(seedApplied?.issues).toBeUndefined(); + const wire = JSON.stringify(body) ?? ''; + expect(wire).not.toContain('unrecognized_keys'); + expect(wire).not.toContain('invalid_metadata'); + expect(wire).not.toContain('failed spec validation'); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §3 — the bound. The strip is the DECLARED list, not a blanket underscore +// strip. RED before the fix, and RED under the wrong repair. +// ═══════════════════════════════════════════════════════════════════════════ + +describe('#15591 · 3 · the strip removes the decoration and keeps the provenance', () => { + it('hands the loader a body with no `_diagnostics` and its `_packageId` intact', async () => { + const { loadedSeeds } = await publishThenApply(); + + const seeds = loadedSeeds(); + expect(seeds).toHaveLength(1); + // Removed: our own read-time annotation, which is what the parse refused. + expect(seeds[0]).not.toHaveProperty(DECORATION); + // Kept: ADR-0010 envelope state the schema allowlists "precisely so a + // served document keeps its provenance on re-parse". Reusing the export + // path's blanket `startsWith('_')` strip would delete this, and that is + // the reading this assertion exists to refuse. + expect(seeds[0]).toHaveProperty(PROVENANCE, PKG); + }); +}); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 6c7bba0e7a..1218b8563c 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -61,6 +61,15 @@ import { clientFacingFailureText, seedRequestValidationError } from '@objectstac // [#8805] Moved to `metadata-core` so the REST `/meta` write doors decide this // the same way rather than through a second copy. Behaviour unchanged. import { organizationIdForMetaWrite } from '@objectstack/metadata-core'; +// [#15591] The DECLARED removal of our OWN read-time annotations, imported +// from the list that defines them (`METADATA_READ_DECORATIONS`) rather than +// re-spelled here. `applyPublishedSeeds` below re-parses a SERVED document, and +// `spec/kernel/metadata-read-decorations.ts` states the rule this door was +// missing: "a served body is NOT a valid input to the schema that produced it +// until these are removed". Same helper, same reason, as the three consumers +// that already call it — the dataset query in `rest-server.ts`, the cold-boot +// flow bind in `service-automation`, and `saveMetaItem`'s verbatim persist. +import { stripReadDecorations } from '@objectstack/spec/kernel'; import { setPackageDisabled } from '../package-state-store.js'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -1525,7 +1534,34 @@ _context: HttpProtocolContext, ? item : (item?.item ?? item?.metadata ?? item?.body); if (seed?.object && Array.isArray(seed?.records)) { - datasets.push(seed); + // [#15591] Strip the READ-TIME decorations before the closed parse + // below. `getMetaItem` exits through `decorateMetadataItem`, which + // stamps `_diagnostics` on every body whose type has a registered + // schema — `seed` has one — so the document this door reads back is + // the platform's own output and `SeedSchema` (closed since #4001) + // refused it by name: `unrecognized_keys: ["_diagnostics"]`, minted + // as a 422 by the `safeParse` below, delivered on a **200** as + // `seedApplied.error`. Zero rows loaded, and the author told their + // seed body failed spec validation when nothing about it is wrong. + // + // ⛔ NOT a blanket `startsWith('_')` strip, and deliberately not the + // one `assemblePackageManifest` runs 300 lines up: the two paths + // have opposite obligations, and the spec states both. + // `METADATA_READ_DECORATIONS` is `['_diagnostics', '_draft']` and + // its module says the ADR-0010 envelope (`_packageId`, + // `_provenance`, …) is "deliberately NOT" a member — "the closed + // metadata schemas allowlist them precisely so a served document + // keeps its provenance on re-parse". `SeedSchema` is one of those: + // it spreads `MetadataProtectionFields` on purpose. Measured on the + // real producer, the served body carries BOTH keys and the schema + // refuses exactly one — `_packageId` alone parses clean. So the + // export path's blanket strip would drop provenance this schema + // accepts, which is why the DECLARED list is the one to use here. + // + // ⛔ And NOT a widened schema: nothing about the request contract + // changes. This removes an annotation the READ path added, which is + // the only reason the round trip was not already closed. + datasets.push(stripReadDecorations(seed)); } else { readErrors.push(`seed "${name}" body unreadable (keys: ${item ? Object.keys(item).join(',') : 'none'})`); } From 73cdcb34a76cd6c3f4d76c03b210d9460287d569 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 03:19:40 +0000 Subject: [PATCH 2/3] chore(gates): re-anchor the system-context census and record the new engine double MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both are mechanical regenerations the gates asked for by name, not hand edits: - `check:check-system-context-census` reported pure LINE ROT — the three `domains/packages.ts` anchors on `content/docs/permissions/system-context.mdx` each moved by exactly the +9 lines this branch's import block added. Repaired with the gate's own `--fix`; it now reports OK over 105 sites and 140 anchors. - `check:engine-double-contract` reported the new test file's engine double as RETAINED-but-unrecorded on all three scanned verbs. Regenerated with `--write` (736 rows, 3 added, 0 lost), so the pin protects the file. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- content/docs/permissions/system-context.mdx | 4 ++-- scripts/engine-double-contract.pinned.json | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index ac5d133bfe..76965c7370 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -160,10 +160,10 @@ The largest single consumer — **17 of the 105 sites**. | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` | | 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5231`, `:6657`, `:6905`, `:7336`, `:7529` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | -| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:535`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | +| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:544`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | | 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | -| 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:241`, `:274` | +| 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:250`, `:283` | | 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:138`, `:189` | | 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `domains/automation.ts:254`, `:545`, `:635` | | 58 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `suggested-audience-bindings.ts:703` | diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 81ef3511f3..051bfdb68e 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -3211,6 +3211,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/runtime/src/domains/packages-seed-apply-read-decorations.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/runtime/src/domains/packages-seed-apply-read-decorations.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/runtime/src/domains/packages-seed-apply-read-decorations.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/runtime/src/domains/share-links-enforcement-context.test.ts", "verb": "delete", From 5f05afb33653300daff4043a55a52dd9898e778d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 04:51:40 +0000 Subject: [PATCH 3/3] chore(gates): re-anchor the system-context census after merging origin/main The merge brought main's re-anchoring of `system-context.mdx` and this branch's own +9-line import block in `domains/packages.ts`. `merge=os-regen` resolved the page with exit 0 and no markers while silently keeping one side, so the page had to be regenerated from the merged tree rather than text-merged. `pnpm gen:system-context-census` (= `check-system-context-census.mjs --fix`) rewrote 3 anchors, all `domains/packages.ts`, all by exactly +9: `:241`->`:250`, `:274`->`:283`, `:543`->`:552`. Delta is line-anchor numbers only: with every digit run normalized the page is byte-identical before and after, at the same 423 lines and the same 65 rows. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- content/docs/permissions/system-context.mdx | 34 ++++++++++----------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 76965c7370..2e58d7f6e6 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1651`, `:1680`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1739`, `:1768`), and neither can an action body (`packages/runtime/src/domains/actions.ts:414`). It is written by internal callers only, as an option on the engine call: @@ -103,23 +103,23 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:250` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1683` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1771` | ### 2. Write pipeline and data integrity | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11675` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11858` | -| 20 | **`readonly` strip bypassed — INSERT** | objectql | Same, on create — one gate over BOTH create-side passes since the 2026-09-03 ruling moved the static-`readonly` strip in beside the runtime-owned one and deleted the DataProtocol ingress copy. `isSystem` is the **only** exemption on this path: `preserveAudit` is deliberately not read on create, so a non-system historical import is still stripped | `objectql/src/engine.ts:10323` | -| 21 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:10456`, `readonly-strict-errors.ts:66` | -| 22 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:6182` | -| 23 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3913`, `:3923`, `:3950` | +| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11733` | +| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11916` | +| 20 | **`readonly` strip bypassed — INSERT** | objectql | Same, on create — one gate over BOTH create-side passes since the 2026-09-03 ruling moved the static-`readonly` strip in beside the runtime-owned one and deleted the DataProtocol ingress copy. `isSystem` is the **only** exemption on this path: `preserveAudit` is deliberately not read on create, so a non-system historical import is still stripped | `objectql/src/engine.ts:10381` | +| 21 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:10514`, `readonly-strict-errors.ts:66` | +| 22 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:6240` | +| 23 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3920`, `:3930`, `:3957` | | 24 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` | | 25 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:99` | -| 26 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6881` | -| 27 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:12477` | -| 28 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:12406` | +| 26 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6939` | +| 27 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:12535` | +| 28 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:12464` | | 29 | **Bulk data event `organizationId` OMITTED** — the batch is published "not asserted" | plugin-security | Get: nothing — the `data.records.*` event still publishes. Lose: the per-organization attribution: this exit is taken before the security middleware composes any tenant wall, so it records no Layer 0 verdict on the operation (`OperationContext.tenantLayer0Verdict`, #15813), and the engine's bulk producer — which reads that recorded verdict and nothing else — omits the key rather than filling it from the caller's `tenantId`; a tenant-scoped consumer then does not deliver the event inside an organization wall (#15225) | `security-plugin.ts:1686` | ### 3. Sharing (`plugin-sharing`) @@ -158,9 +158,9 @@ The largest single consumer — **17 of the 105 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:138` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5231`, `:6657`, `:6905`, `:7336`, `:7529` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5319`, `:6766`, `:7014`, `:7445`, `:7638` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | -| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:544`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | +| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:552`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | | 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | | 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:250`, `:283` | @@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| -| 62 | `objectql/src/engine.ts:3720` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:14923` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 62 | `objectql/src/engine.ts:3727` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | +| 63 | `objectql/src/engine.ts:14981` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | | 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report | | 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | @@ -195,11 +195,11 @@ assuming `isSystem` covers it is a documented source of bugs. |:---|:---|:---| | "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:2032` (rationale at `:1942`–`1944`, #3760), `flow.zod.ts:743` | | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) | -| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10306`–`10323` | +| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10364`–`10381` | | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1590` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1651`, `:1680`; `domains/actions.ts:414` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1739`, `:1768`; `domains/actions.ts:414` | ---