From 503d75a824bdf2b50ddb0760675fa584ff0f7e94 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 14:15:28 +0000 Subject: [PATCH] fix(metadata,runtime): retire the `policies` dead pointer in both artifact registrars `ARTIFACT_FIELD_TO_TYPE` (the artifact door) and `AppPlugin`'s ADR-0057 `SECURITY_FIELDS` list each carried a `policies` -> `policy` entry that could never match: `ObjectStackDefinitionSchema` is a `strictObject` declaring no top-level `policies` key, so a definition carrying one is refused by the door's strict parse and reaches neither registry. On a permission set `policies` is an alias for `rowLevelSecurity` -- a key on an item, never a collection. Both entries are removed with the in-place note this map already writes for a retirement. That was the third such retirement here, so the removal ships with the check the two predecessors lacked: `check-stack-collection-maps` now pins `SECURITY_FIELDS` as an eighth site (new `tupleFirstItems` extractor for its `[collection, kind]` tuple shape, plus a self-test case), so re-adding a key the schema does not declare fails in CI on either registrar. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd --- ...rtifact-registrar-policies-dead-pointer.md | 41 ++++++ packages/metadata/src/plugin.ts | 11 +- ...plugin-artifact-forward-conversion.test.ts | 24 ++-- packages/runtime/src/app-plugin.ts | 18 ++- scripts/check-stack-collection-maps.mjs | 123 ++++++++++++++++-- 5 files changed, 192 insertions(+), 25 deletions(-) create mode 100644 .changeset/artifact-registrar-policies-dead-pointer.md diff --git a/.changeset/artifact-registrar-policies-dead-pointer.md b/.changeset/artifact-registrar-policies-dead-pointer.md new file mode 100644 index 0000000000..893719b7b8 --- /dev/null +++ b/.changeset/artifact-registrar-policies-dead-pointer.md @@ -0,0 +1,41 @@ +--- +"@objectstack/metadata": patch +"@objectstack/runtime": patch +--- + +fix(metadata,runtime): retire the `policies` dead pointer in both artifact registrars, and pin the map that carried it (#12894) + +Zero behaviour change, by construction. Both readers of an artifact boot carried +a `policies` -> `policy` entry — the artifact door's `ARTIFACT_FIELD_TO_TYPE` +(`packages/metadata/src/plugin.ts`) and `AppPlugin`'s ADR-0057 `SECURITY_FIELDS` +list (`packages/runtime/src/app-plugin.ts`) — and **neither could ever match**. +`ObjectStackDefinitionSchema` is a `strictObject` that declares no top-level +`policies` key, so a definition carrying a `policies` array is refused outright +by the door's strict parse and reaches neither registry. The word is real but +lives one level down: on a permission set `policies` is an alias for +`rowLevelSecurity` (`PERMISSION_SET_KEY_ALIASES`) — a key on an **item**, never a +collection. Both entries are removed, each leaving in place the note the map +already writes for a retirement: what it pointed at, and why it could not match. + +That was the third entry retired from `ARTIFACT_FIELD_TO_TYPE` for exactly this +reason (`themes`, then `roles` -> `positions`, which "matched nothing and +silently dropped compiled positions"). So the deletion ships with the thing the +two predecessors did not have — a check that fails when the pattern recurs: + +- `check:stack-collection-maps` now reconciles **eight** hand-maintained + enumerations against the schema, not seven. `SECURITY_FIELDS` is the new + eighth, and how it was missing is the finding rather than a footnote: it is + the only one of the eight that pairs its keys as `[collection, kind]` tuples, + which neither existing extractor could read, so the site was skipped rather + than reported. Re-adding `policies` — or any other key the schema does not + declare — to **either** registrar now fails the gate with the site named. +- A new `tupleFirstItems` extractor reads that shape, with a self-test case + (13 assertions, up from 12) covering the comment/nesting cases the flat + string-array extractor already pins. + +The mirror-image half of the same measurement is **carried, not shipped**: +`capabilities` is a declared top-level collection that `SECURITY_FIELDS` +registers and the door's map does not, making `AppPlugin` its sole registrar on +an artifact boot. Adding it to the door changes what an artifact boot registers, +so it is measured and handed to the route-ownership decision (#12892) instead of +being smuggled in here. The new waiver row records the asymmetry in place. diff --git a/packages/metadata/src/plugin.ts b/packages/metadata/src/plugin.ts index 469d2ddd39..9698aa111f 100644 --- a/packages/metadata/src/plugin.ts +++ b/packages/metadata/src/plugin.ts @@ -89,7 +89,16 @@ const ARTIFACT_FIELD_TO_TYPE: Record = { positions: 'position', permissions: 'permission', sharingRules: 'sharing_rule', - policies: 'policy', + // `policies: 'policy'` removed at #12894: the stack schema is a + // `strictObject` that declares no top-level `policies` key, so a + // definition carrying one is refused by the strict parse a few lines + // below — the entry could never match, and nothing was ever registered + // under `policy` from this map. The word is real, but it lives ONE LEVEL + // DOWN: on a permission set it is an alias for `rowLevelSecurity` + // (`PERMISSION_SET_KEY_ALIASES`, packages/spec/src/security/permission.zod.ts) + // — a key on an ITEM, never a collection. Third retirement of this exact + // shape in this map (`themes` and `roles` above); the reasons are kept + // in place because the first two are what made this one findable. apis: 'api', webhooks: 'webhook', agents: 'agent', diff --git a/packages/runtime/src/app-plugin-artifact-forward-conversion.test.ts b/packages/runtime/src/app-plugin-artifact-forward-conversion.test.ts index 7a450ae2b3..7a75ff165e 100644 --- a/packages/runtime/src/app-plugin-artifact-forward-conversion.test.ts +++ b/packages/runtime/src/app-plugin-artifact-forward-conversion.test.ts @@ -8,8 +8,9 @@ * ADR-0087 forward conversion (#12772), then strict-parses. Canonical. * 2. `AppPlugin`'s ADR-0057 block (this package) — receives the same JSON * from `loadArtifactBundle` (no validation, no conversion) and registers - * `positions` / `permissions` / `capabilities` / `sharingRules` / - * `policies` through `metadata.registerInMemory`. + * `positions` / `permissions` / `capabilities` / `sharingRules` through + * `metadata.registerInMemory`. (It also carried a `policies` entry until + * #12894 retired it as a dead pointer — the test below is what stays.) * * Before this fix reader 2 registered the RAW bytes, so the two copies of the * same item differed and which one a consumer saw depended on registration @@ -355,12 +356,19 @@ describe('#12844 — the artifact boot\'s two readers register the same bytes', }); it('policies: not an authorable stack collection at all — neither reader can see one', async () => { - // `AppPlugin`'s SECURITY_FIELDS and `ARTIFACT_FIELD_TO_TYPE` both carry - // a `policies` → `policy` entry, but `ObjectStackDefinitionSchema` is a - // strictObject with no `policies` key: on the permission set `policies` - // is an ALIAS for `rowLevelSecurity`. A top-level `policies` collection - // is refused by the door outright, so it can never reach either - // registry — both entries are dead pointers. + // `AppPlugin`'s SECURITY_FIELDS and `ARTIFACT_FIELD_TO_TYPE` each + // carried a `policies` → `policy` entry until #12894 removed both: + // `ObjectStackDefinitionSchema` is a strictObject with no `policies` + // key, so a top-level `policies` collection is refused by the door + // outright and neither entry could ever match. On the permission set + // `policies` is an ALIAS for `rowLevelSecurity` — a key on an ITEM. + // + // This case is unchanged by that removal, and deliberately so: it pins + // the SCHEMA fact the removal rests on, which is what makes the entries + // dead. What stops them being re-added is `check:stack-collection-maps`, + // which now reconciles both maps (`ARTIFACT_FIELD_TO_TYPE` and + // `SECURITY_FIELDS`) against this schema — a green run of THIS test is + // not evidence the pointers are gone. const withPolicies = { ...bytes(), policies: [{ name: 'p1', label: 'P1' }] }; const parsed = ObjectStackDefinitionSchema.safeParse(withPolicies); expect(parsed.success).toBe(false); diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index 2561f5e0b2..7f6b0de188 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -633,10 +633,10 @@ export class AppPlugin implements Plugin { } // [ADR-0057 / #2077] Surface stack-declared SECURITY metadata - // (positions, permission sets, sharing rules, policies) in the + // (positions, permission sets, capabilities, sharing rules) in the // metadata registry so the boot seeders (plugin-security / // plugin-sharing) and runtime resolvers can read them via - // `list('position'|'permission'|'sharing_rule')`. + // `list('position'|'permission'|'capability'|'sharing_rule')`. // Without this, bootStack's metadata service holds only objects (the // artifact loader that registers these runs only in compiled serve.ts), // leaving the declarations decorative. @@ -699,7 +699,19 @@ export class AppPlugin implements Plugin { // sys_capability with package provenance. ['capabilities', 'capability'], ['sharingRules', 'sharing_rule'], - ['policies', 'policy'], + // `['policies', 'policy']` removed at #12894, together with + // its twin in the artifact door's `ARTIFACT_FIELD_TO_TYPE` + // (`packages/metadata/src/plugin.ts`). It could never match: + // `ObjectStackDefinitionSchema` is a `strictObject` declaring + // no top-level `policies` key, so the door refuses such a + // definition outright and this loop reads `undefined`. The + // word belongs one level down — on a permission set it is an + // alias for `rowLevelSecurity` (`PERMISSION_SET_KEY_ALIASES`, + // packages/spec/src/security/permission.zod.ts) — so a + // top-level collection of that name never existed to register. + // `check:stack-collection-maps` now pins this list, so a + // fourth attempt at a key the schema does not declare fails + // in CI instead of sitting here inert. ]; let count = 0; for (const [field, type] of SECURITY_FIELDS) { diff --git a/scripts/check-stack-collection-maps.mjs b/scripts/check-stack-collection-maps.mjs index 772dfe91e4..6ac93cbd94 100644 --- a/scripts/check-stack-collection-maps.mjs +++ b/scripts/check-stack-collection-maps.mjs @@ -11,19 +11,29 @@ // ## The hole this closes (#6242) // // `ObjectStackDefinitionSchema` (`packages/spec/src/stack.zod.ts`) decides which -// collections a stack may declare. SEVEN other places re-enumerate that same set +// collections a stack may declare. EIGHT other places re-enumerate that same set // by hand: the map-format field list, the plural->singular map, the artifact // category enum, the ObjectQL registration seam, the artifact-ingest field map, -// the runtime's app-payload probe and the showcase coverage manifest -- and -// until this gate NOTHING compared any of them to the schema or to each other. -// They drifted independently and invisibly: +// the runtime's app-payload probe, AppPlugin's ADR-0057 security surface and the +// showcase coverage manifest -- and until this gate NOTHING compared any of them +// to the schema or to each other. They drifted independently and invisibly: // -// (Eight, when this gate was written: ObjectQL declared its list TWICE, once per -// registration seam, and the two copies had drifted four collections apart -- -// recorded here as a waiver row until #7049 hoisted the single -// `METADATA_ARRAY_KEYS` both seams now read. A divergence between two copies is -// the one deviation no reading of either copy alone produces, which is the -// argument for this gate in one line.) +// (Counted differently when this gate was written: ObjectQL declared its list +// TWICE, once per registration seam, and the two copies had drifted four +// collections apart -- recorded here as a waiver row until #7049 hoisted the +// single `METADATA_ARRAY_KEYS` both seams now read. A divergence between two +// copies is the one deviation no reading of either copy alone produces, which is +// the argument for this gate in one line.) +// +// `SECURITY_FIELDS` joined as the eighth at #12894, and how it was missing is +// the point rather than a footnote: it is the ONLY one of the eight that pairs +// its keys as `[collection, kind]` TUPLES, so the two extractors this gate +// already had (object keys, flat string arrays) could not read it and the site +// was skipped instead of failing. It carried a `policies` dead pointer -- the +// same retired kind waived on two other sites -- and removing that entry from +// the artifact door while leaving this one unpinned would have left exactly one +// place where the fourth instance of a twice-retired pattern could land back +// unnoticed. // // - `PLURAL_TO_SINGULAR` carries `ragPipelines`, which the schema does not // declare. @@ -254,6 +264,43 @@ export function stringArrayItems(body) { return out; } +/** + * The FIRST string of each depth-1 tuple in an array-of-tuples body -- the + * `Array<[collectionKey, metadataType]>` shape `SECURITY_FIELDS` uses. + * + * A separate extractor rather than a flag on `stringArrayItems`, because the two + * disagree about what "the site enumerates" means: the flat form's strings ARE + * the keys, and here only the tuple's head is, with the tail naming the metadata + * kind. Reading a tuple site with the flat extractor returns an empty list at + * depth 0 -- which reconciles against everything and reports no drift, the + * silent-no-op shape this gate exists to refuse. (The caller turns an empty + * result into a FAILURE for that reason; this extractor makes the non-empty + * answer available instead.) + */ +export function tupleFirstItems(body) { + const mask = maskLiterals(body); + const out = []; + let depth = 0; + let taken = false; + for (let i = 0; i < body.length; i++) { + const ch = mask[i]; + if (ch === '{' || ch === '[' || ch === '(') { + depth++; + if (depth === 1) taken = false; + continue; + } + if (ch === '}' || ch === ']' || ch === ')') { depth--; continue; } + if (depth === 1 && !taken && (ch === "'" || ch === '"')) { + const end = mask.indexOf(ch, i + 1); + if (end === -1) break; + out.push(body.slice(i + 1, end)); + taken = true; + i = end; + } + } + return out; +} + /** * The stack-collection set: top-level `ObjectStackDefinitionSchema` keys whose * value is `z.array(Schema)`. @@ -498,10 +545,13 @@ const SITES = [ waivers: [ { direction: 'extra', - keys: ['workflows', 'policies', 'ragPipelines'], + keys: ['workflows', 'ragPipelines'], reason: 'DRIFT — retired kinds still mapped, inert for the same reason as the ObjectQL loops: a parsed ' - + 'artifact cannot carry the fields (#6242 row 4).', + + 'artifact cannot carry the fields (#6242 row 4). `policies` left this row at #12894, removed ' + + 'from the map rather than re-waived: it was the THIRD entry retired here for one reason (after ' + + '`themes` and `roles`), and the map now carries the reason in place so a fourth reader does not ' + + 'have to rediscover it.', }, { direction: 'missing', @@ -567,6 +617,48 @@ const SITES = [ }, ], }, + { + // The EIGHTH enumeration, added at #12894 with the `policies` dead pointer + // it carried. Unlike `APP_CATEGORY_KEYS` above -- same file, different + // question -- this list is a REGISTRATION list: every pair it names really + // does write into the metadata registry, so a key the schema does not + // declare is not a harmless extra word, it is a registration that silently + // never happens. + // + // Pinned in the `extra` direction above all: this is the second registrar + // of the artifact boot's security collections (`ARTIFACT_FIELD_TO_TYPE` is + // the first), and the two had drifted in MIRROR-IMAGE ways -- `policies` + // dead in both, `capabilities` live here and absent there. Reconciling both + // against one schema is what makes that pair of facts visible at all. + id: 'SECURITY_FIELDS', + file: 'packages/runtime/src/app-plugin.ts', + what: "AppPlugin's ADR-0057 stack-declared security metadata -> registerInMemory kind", + extract: (src) => { + const b = sliceBody(src, 'const SECURITY_FIELDS: Array<[string, string]> = ['); + return b && tupleFirstItems(b.body); + }, + waivers: [ + { + direction: 'missing', + keys: [ + 'datasources', 'datasourceMapping', 'translations', 'objects', 'objectExtensions', 'apps', + 'views', 'pages', 'dashboards', 'reports', 'datasets', 'actions', 'flows', 'jobs', + 'emailTemplates', 'docs', 'books', 'apis', 'webhooks', 'agents', 'tools', 'skills', + 'hooks', 'mappings', 'analyticsCubes', 'connectors', 'data', + ], + reason: + 'DELIBERATE — a four-collection SUBSET, not an enumeration of the collection set. This block ' + + 'exists so the boot seeders (plugin-security / plugin-sharing) and the runtime resolvers can ' + + 'read stack-declared SECURITY metadata through `list(...)` on a boot where the artifact door ' + + 'never runs; every other collection reaches the registry through the door or its own seam. ' + + 'Recorded as one row rather than left implicit so that a NEW security collection has to be ' + + 'considered here once — which is the direction this site was actually wrong in: `capabilities` ' + + 'is registered here and NOT by the door, making this block that collection\'s sole registrar on ' + + 'an artifact boot (#12894 half 2, carried to #12892 for the ownership decision — measured, ' + + 'deliberately not changed here).', + }, + ], + }, { id: 'STACK_COLLECTION_COVERAGE', file: 'examples/app-showcase/src/coverage.ts', @@ -774,6 +866,11 @@ export const ObjectStackDefinitionSchema = lazySchema(() => strictObject({ stringArrayItems(`'a', /* 'x' */ 'b', ['c'], // 'd'\n 'e',`), ['a', 'b', 'e'], ); + eq( + 'tupleFirstItems() takes the head of each tuple, not every string', + tupleFirstItems(`['a', 'x'], /* ['q', 'q'] */ ['b', 'y'], // ['d', 'd']\n ['c', 'z'],`), + ['a', 'b', 'c'], + ); eq( 'objectEntries() reads top-level keys only', objectEntries(`a: 'x', b: { c: 'y' }, 'd': 'z',`).map((e) => e.key), @@ -826,7 +923,7 @@ export const ObjectStackDefinitionSchema = lazySchema(() => strictObject({ for (const f of failures) console.error(` • ${f}\n`); return 1; } - console.log('✓ check-stack-collection-maps --self-test: 12 assertions over synthetic sources'); + console.log('✓ check-stack-collection-maps --self-test: 13 assertions over synthetic sources'); return 0; }