diff --git a/.changeset/tolerant-reference-alias-readers.md b/.changeset/tolerant-reference-alias-readers.md new file mode 100644 index 0000000000..183fb39f93 --- /dev/null +++ b/.changeset/tolerant-reference-alias-readers.md @@ -0,0 +1,68 @@ +--- +"@objectstack/plugin-security": patch +"@objectstack/verify": minor +--- + +fix(security,verify): the last two tolerant `reference_to` readers — one made loud, one narrowed with a named reason (#13250) + +`@objectstack/spec` declares `reference` as the only relationship spelling and +`FieldSchema` rejects `reference_to` / `referenceTo` (#11567, "one key, one +answer"). Three live consumers still read the rejected alias as an accepted +fallback. The lint reader was narrowed in #13322; the two remaining ones get +**different** dispositions, because their failure modes are different in kind +(maintainer ruling, 2026-08-30). + +**`@objectstack/plugin-security` — the tolerance STAYS, and is now LOUD.** +`resolveCbpRelation` reads `ql.getSchema()`, i.e. the `SchemaRegistry`, and a +raw `registerObject` skips Zod by design, so the alias genuinely reaches it +(re-measured: a raw round-trip serves the field back as +`["name","type","required","reference_to"]`, canonical absent). A miss there is +not a quiet wrong answer, it is a **denial** — `resolveCbpRelation` returning +null is fail-closed, giving `RLS_DENY_FILTER` (zero rows for every non-admin +caller) on read and throwing `MasterDetailRelationMissingError` on write. So +narrowing it would take a raw-registered, alias-spelled `controlled_by_parent` +object from "access derived from its master" to "everything denied, and writes +throw": an availability outage on a population that provably exists. The alias +therefore still resolves, unchanged, and the plugin now reports it **once per +object** through its own report sink — the same `warn` channel and console +backed default as every other report site there. The message names the object, +the field, the alias key and the rename, and states that access is unaffected +so nobody goes hunting for an outage that did not happen. Nothing is reported +for the canonical spelling, or when a canonical `reference` won over a stale +alias on the same field. + +The report's granularity is the cache's: it sits inside `resolveCbpRelation`'s +resolution body, which runs only on a `cbpRelCache` miss, so 25 reads of one +object produce one report — and it re-arms when `metadata.watch('*')` clears +that cache, which is exactly when a Studio / AI-authoring author is listening. + +**`@objectstack/verify` — narrowed, and the finding says WHY.** `deriveCrudCases` +reads its config from `loadConfig()`, which does not validate ("the gate lives +in the loaded module"), so the alias reaches it through two unparsed doors — +a plain-object config, and the documented `defineStack(cfg, { strict: false })` +(re-measured: the same fixture is refused by the default strict parse with +*"Unrecognized key(s) on this field: `reference_to`"*, and survives both doors +verbatim). Unlike the security reader, verify's failure mode is a **report +line** rather than a refusal, so narrowing costs coverage, not availability — +and it is safe. But a verifier that silently under-verifies is the defect +#5262 was about, so the narrowing ships **with** its reason: an alias-spelled +required relation now reports + +> required lookup field "company_id" spells the rejected alias `reference_to` +> instead of `reference` — `reference` is the only relationship spelling +> @objectstack/spec declares, so this app's target "company" was not derived; +> rename the key + +rather than degrading to the generic "has no `reference` target", and an +optional one is skipped under `relation-rejected-reference-alias:` rather +than the generic `relation-missing-reference`. Both land in the existing +free-form `CrudCase.blocked` / `skippedFields[].reason` strings — no new +exported type, no new status, no widened published surface. + +No shipped metadata spells either alias: the repo-wide sweep finds the spelling +only in tests, the spec's own alias tables and other readers' documentation — +no example app or platform object uses it. + +⛔ Narrowing the security reader for real remains out of scope here, and is +only honest behind a migration that sweeps stored / raw-registered metadata +first. diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-reference-alias.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-reference-alias.test.ts new file mode 100644 index 0000000000..50ef1aece0 --- /dev/null +++ b/packages/plugins/plugin-security/src/controlled-by-parent-reference-alias.test.ts @@ -0,0 +1,249 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#13250] `resolveCbpRelation` KEEPS reading the rejected `reference` aliases, +// and now says so out loud. +// +// ## The disposition this suite pins, and why it is the inverse of its siblings +// +// `@objectstack/spec` declares `reference` as the only relationship spelling; +// `FieldSchema` rejects `reference_to` / `referenceTo` (#11567). Two lint +// readers were narrowed to canonical-only on that ruling. This one was NOT +// (maintainer ruling, 2026-08-30), and the reason is measured rather than +// stylistic: +// +// • The alias REACHES here. This reader consumes `ql.getSchema()`, i.e. the +// `SchemaRegistry`, and a raw `registerObject` skips Zod by design (#3896). +// • A miss here is a DENIAL, not a wrong answer. `resolveCbpRelation` +// returning null is fail-closed: the read leg answers `RLS_DENY_FILTER` +// (zero rows for every non-admin caller) and the write leg throws +// `MasterDetailRelationMissingError`. Narrowing would take a +// raw-registered, alias-spelled `controlled_by_parent` object from "access +// derived from its master" to "everything denied" — an availability outage +// on a population that provably exists. +// +// So the card's defect (a tolerant consumer hiding an authoring mistake) is +// answered by making the tolerance LOUD, not by removing it. +// +// ## What this suite refuses to let a later change do +// +// The load-bearing tests here are the two NEGATIVES, and they point in opposite +// directions, so neither "narrow it after all" nor "drop the noisy log" can +// pass quietly: +// +// 1. THE ALIAS STILL RESOLVES. A future narrowing reddens `resolves the +// relation` below rather than shipping as a silent availability change. +// 2. IT IS REPORTED, ONCE PER OBJECT. Deleting the diagnostic reddens the +// report tests; making it per-read reddens the cache test; making it +// fire-once-per-process reddens the re-arm test. + +import { describe, it, expect, vi } from 'vitest'; +import { SecurityPlugin } from './security-plugin.js'; + +/** The MASTER. Spelling is irrelevant to it — it declares no relation. */ +const ACCOUNT_SCHEMA = { + name: 'crm_account', + sharingModel: 'private', + fields: { id: { name: 'id', type: 'text' } }, +}; + +/** A detail whose master FK is spelled with `field` carrying `value`. */ +function detailSchema(field: string, value: unknown, extra: Record = {}) { + return { + name: 'crm_contact', + sharingModel: 'controlled_by_parent', + fields: { + id: { name: 'id', type: 'text' }, + account: { name: 'account', type: 'master_detail', required: true, [field]: value, ...extra }, + }, + }; +} + +/** + * Boot a real `SecurityPlugin` over a fake registry, through the plugin's OWN + * lifecycle — `start()` is what binds `this.logger` to the host sink and what + * subscribes the cache invalidation, so both facts under test here are only + * true of a started plugin. Returns the metadata `watch` callback so a test can + * drive a real metadata-change invalidation rather than reaching into the cache. + */ +async function boot(schemas: Record) { + const warn = vi.fn(); + let onMetadataChange: (() => void) | undefined; + const ql: any = { + getSchema: (object: string) => schemas[object], + registerMiddleware: () => {}, + }; + const metadata: any = { + watch: (_pattern: string, cb: () => void) => { + onMetadataChange = cb; + return () => {}; + }, + list: async () => [], + }; + const services: Record = { + objectql: ql, + metadata, + manifest: { register: () => {} }, + }; + const ctx: any = { + logger: { info: vi.fn(), warn, error: vi.fn(), debug: vi.fn() }, + registerService: () => {}, + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({}); + await plugin.init(ctx); + await plugin.start(ctx); + return { + plugin, + warn, + resolve: (object: string) => (plugin as any).resolveCbpRelation(object), + /** Fire the real `metadata.watch('*')` subscription `start()` installed. */ + changeMetadata: () => { + if (!onMetadataChange) throw new Error('plugin did not subscribe to metadata changes'); + onMetadataChange(); + }, + }; +} + +/** Every alias-carrying warn this suite cares about, in call order. */ +const aliasWarns = (warn: ReturnType) => + warn.mock.calls.filter((c) => String(c[0]).includes('[security/reference-spelling]')); + +describe('resolveCbpRelation — the rejected `reference` alias still RESOLVES (#13250)', () => { + it('`reference_to` resolves the relation — narrowing it would deny access, so it must not be narrowed', async () => { + const { resolve } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('reference_to', 'crm_account'), + }); + // ⛔ THE invariant the 2026-08-30 ruling protects. If this ever reads + // `null`, a raw-registered object spelled this way answers + // RLS_DENY_FILTER on read and throws on write. + expect(resolve('crm_contact')).toMatchObject({ fk: 'account', master: 'crm_account' }); + }); + + it('`referenceTo` resolves it too — the tolerance covers both rejected spellings', async () => { + const { resolve } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('referenceTo', 'crm_account'), + }); + expect(resolve('crm_contact')).toMatchObject({ fk: 'account', master: 'crm_account' }); + }); + + it('the canonical `reference` resolves it, unchanged', async () => { + const { resolve } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('reference', 'crm_account'), + }); + expect(resolve('crm_contact')).toMatchObject({ fk: 'account', master: 'crm_account' }); + }); + + it('an ABSENT-then-aliased chain falls through exactly as `??` did (the refKey mirror)', async () => { + // `refKey` replaced a literal `reference ?? reference_to ?? referenceTo` + // chain, so the fall-through semantics are pinned rather than assumed: + // `??` skips null and undefined ONLY, which is what `!= null` tests. A + // present-but-null canonical key must therefore keep falling through to + // the alias — and a truthiness-based rewrite would fail this. + const nullReference = null; + const { resolve } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('reference', nullReference, { reference_to: 'crm_account' }), + }); + expect(resolve('crm_contact')).toMatchObject({ fk: 'account', master: 'crm_account' }); + }); +}); + +describe('resolveCbpRelation — the tolerance is LOUD (#13250)', () => { + it('reports the alias, and the report names the object, the field, the key and the fix', async () => { + const { resolve, warn } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('reference_to', 'crm_account'), + }); + resolve('crm_contact'); + + const calls = aliasWarns(warn); + expect(calls).toHaveLength(1); + const [message, detail] = calls[0]; + expect(message).toContain('crm_contact'); + expect(message).toContain('account'); + expect(message).toContain('reference_to'); + expect(message).toContain('`reference`'); + // The operator must not go hunting for an outage that did not happen: the + // text has to say the alias still worked. + expect(message).toMatch(/UNAFFECTED/); + expect(detail).toEqual({ + object: 'crm_contact', + field: 'account', + alias: 'reference_to', + master: 'crm_account', + }); + }); + + it('names `referenceTo` when THAT is what resolved it — the report tracks the key actually read', async () => { + const { resolve, warn } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('referenceTo', 'crm_account'), + }); + resolve('crm_contact'); + expect(aliasWarns(warn)[0]?.[1]).toMatchObject({ alias: 'referenceTo' }); + }); + + it('says NOTHING for the canonical spelling — this is a defect report, not a trace', async () => { + const { resolve, warn } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('reference', 'crm_account'), + }); + resolve('crm_contact'); + expect(aliasWarns(warn)).toHaveLength(0); + }); + + it('says nothing when canonical WINS over a stale alias on the same field', async () => { + const { resolve, warn } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('reference', 'crm_account', { reference_to: 'stale_legacy' }), + }); + expect(resolve('crm_contact')).toMatchObject({ master: 'crm_account' }); + // The alias was present but was not what answered, so it is not news. + expect(aliasWarns(warn)).toHaveLength(0); + }); + + it('ONCE per object, not once per read — this sits under the per-request RLS path', async () => { + const { resolve, warn } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('reference_to', 'crm_account'), + }); + for (let i = 0; i < 25; i++) resolve('crm_contact'); + // The cache miss is the report's gate: 25 resolutions, one report. + expect(aliasWarns(warn)).toHaveLength(1); + }); + + it('once per OBJECT — two alias-spelled objects are two reports, not one', async () => { + const { resolve, warn } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('reference_to', 'crm_account'), + crm_note: { ...detailSchema('reference_to', 'crm_account'), name: 'crm_note' }, + }); + resolve('crm_contact'); + resolve('crm_note'); + expect(aliasWarns(warn).map((c) => c[1].object)).toEqual(['crm_contact', 'crm_note']); + }); + + it('RE-ARMS on a metadata change — the author who just edited the object hears the verdict', async () => { + const { resolve, warn, changeMetadata } = await boot({ + crm_account: ACCOUNT_SCHEMA, + crm_contact: detailSchema('reference_to', 'crm_account'), + }); + resolve('crm_contact'); + resolve('crm_contact'); + expect(aliasWarns(warn)).toHaveLength(1); + + // Studio / AI authoring edited metadata: `start()`'s `metadata.watch('*')` + // subscription clears `cbpRelCache`, and the report rides that same + // lifecycle deliberately — a process-lifetime "already warned" set would + // stay silent here, which is the one moment the author is listening. + changeMetadata(); + resolve('crm_contact'); + expect(aliasWarns(warn)).toHaveLength(2); + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 2b63620fb5..3553d4e319 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -5852,7 +5852,56 @@ export class SecurityPlugin implements Plugin { : fields && typeof fields === 'object' ? (Object.entries(fields) as Array<[string, any]>) : []; - const ref = (f: any) => f?.reference ?? f?.reference_to ?? f?.referenceTo; + // ── The reference spelling: TOLERANT HERE, and deliberately so ─────────── + // + // `@objectstack/spec` declares `reference` as the ONLY relationship + // spelling, and `FieldSchema` REJECTS `reference_to` / `referenceTo` with + // *"Did you mean `reference_to` → `reference`?"* (#11567, "one key, one + // answer"). Two sibling readers were narrowed to canonical-only on that + // ruling — `refOf` in `packages/lint/src/validate-security-posture.ts` + // (which records its reasoning in full) and `refOf` in + // `packages/lint/src/data-model-rules.ts`. This reader is the DELIBERATE + // INVERSE of both, and the asymmetry is the answer rather than an + // oversight (maintainer ruling, 2026-08-30): + // + // • Those two read a stack that WENT THROUGH the parse, so the alias + // cannot legitimately reach them — narrowing there costs nothing and + // the schema names the real defect (the alias key) itself. + // • This one reads `ql.getSchema()`, i.e. the `SchemaRegistry`, and a + // raw `registerObject` SKIPS Zod by design (#3896 — the registry's own + // header names those doors). So here the alias is genuinely REACHABLE. + // • And here a miss is not a quiet wrong answer, it is a DENIAL. + // `resolveCbpRelation` returning null is FAIL-CLOSED: the read leg + // answers `{ ...RLS_DENY_FILTER }` (zero rows for every non-admin + // caller) and the write leg throws `MasterDetailRelationMissingError`. + // Narrowing this reader would take a raw-registered, alias-spelled + // `controlled_by_parent` object from "access derived from its master" + // to "everything denied, and writes throw" — an availability outage on + // a population that provably exists, not a spelling correction. + // + // So the tolerance STAYS, and is made LOUD instead: the alias still + // resolves — no deployment loses access — and the author hears about it, + // once per object, through `reportRejectedReferenceAlias` below. ⛔ Do not + // narrow this in place. Narrowing is only honest behind a migration that + // sweeps stored / raw-registered metadata first, and that is a separate + // card rather than a rider on the one that made this loud. + // + // `refKey` exists so the diagnostic and the resolution cannot disagree + // about WHICH key was read: `ref` is derived from it rather than spelling + // the `??` chain a second time. The mirror is exact for every shape either + // call site can observe — `??` falls through on null/undefined only, which + // is what `!= null` tests. The one difference is unobservable: with EVERY + // spelling present-but-null the old chain evaluated to `null` and this + // returns `undefined`, and both call sites below take `ref` as a + // truthiness test or read it only after one (`pick` requires `ref(f)`, and + // `String(ref(def))` runs only on what `pick` returned). Pinned in + // `controlled-by-parent-reference-alias.test.ts`. + const REFERENCE_SPELLINGS = ['reference', 'reference_to', 'referenceTo'] as const; + const refKey = (f: any) => REFERENCE_SPELLINGS.find((k) => f?.[k] != null); + const ref = (f: any) => { + const k = refKey(f); + return k === undefined ? undefined : f[k]; + }; const pick = (pred: (f: any) => boolean) => entries.find(([, f]) => pred(f) && ref(f)); const found = pick((f) => f?.type === 'master_detail' && f?.required) ?? @@ -5860,6 +5909,13 @@ export class SecurityPlugin implements Plugin { pick((f) => f?.type === 'lookup' && f?.required); if (found) { const def = found[1]; + // Resolved from the SAME field def the relation itself came from, and + // from the same `refKey`, so the report can never name a different field + // or a different key than the one that actually answered. + const spelling = refKey(def); + if (spelling !== undefined && spelling !== 'reference') { + this.reportRejectedReferenceAlias(object, String(found[0]), spelling, String(ref(def))); + } rel = { fk: String(found[0]), master: String(ref(def)), @@ -5884,6 +5940,78 @@ export class SecurityPlugin implements Plugin { return rel; } + /** + * Report that a `controlled_by_parent` master relation resolved ONLY because + * {@link SecurityPlugin.resolveCbpRelation} still accepts a spelling + * `@objectstack/spec` rejects (#11567) — the loud half of the deliberate + * tolerance documented at that reader. + * + * ## Granularity: once per object, per cache generation — and that is the + * ## cache's lifecycle rather than a second one invented here + * + * The only call site is inside `resolveCbpRelation`'s resolution body, which + * runs ONLY on a `cbpRelCache` miss: every later read of the same object is + * served from the cache and reports nothing. Two consequences, both of them + * the reason this is the right granularity: + * + * • ⛔ NOT per read. This sits under the RLS path, which runs per request + * per object — a per-read report would be a noise defect of its own, and + * a noisy channel is a channel operators filter out, which would make + * the tolerance silent again by a longer route. + * • It RE-ARMS on a metadata change, because `start()`'s + * `metadata.watch('*')` subscription clears `cbpRelCache` alongside the + * other metadata-derived caches. That is exactly when an author wants to + * hear it: they just edited the metadata (Studio / AI authoring), so the + * next resolution reports the verdict on what they changed instead of + * staying quiet because a process-lifetime "already warned" set had been + * ticked at boot. A hand-rolled set would have to reproduce this + * invalidation to stay correct, and would silently rot the day another + * cache-clearing door was added. + * + * ## Why this is not a duplicate of the registry's own diagnostic + * + * `SchemaRegistry.registerObject` already prints a functional-completeness + * line for such a field — `field/relationship-without-reference`, "the + * consumer silently skips each one, so it reads 0/null/never-resolves". For + * THIS consumer that is measurably false: the relation resolves, and the + * object is enforced normally. So the registry's line is not the report this + * seam needs — it fires at registration rather than at resolution, and it + * describes a behaviour that is not the one this reader has. Correcting the + * record where the resolution actually happens is the point. + * + * Reported through `this.logger`, the plugin's own {@link SecurityReportSink} + * — the same channel and the same `warn?.(…)` spelling as every other report + * site here, so an un-injected host still hears it on the console-backed + * default (#10556, maintainer ruling 2026-08-24). ⛔ No new channel: a + * diagnostic surface invented for one message is a surface of its own. + * + * The text names the SPELLING as the defect and states that access is + * unaffected, because both halves are load-bearing: an operator who reads + * "rejected alias" and assumes something was denied would go looking for an + * outage that did not happen. + */ + private reportRejectedReferenceAlias( + object: string, + field: string, + alias: string, + master: string, + ): void { + // The tag is a resolvable NAME, not a tracker id: this string reaches + // operators and generated surfaces, and `#NNNN` means nothing to any of + // them (`check:doc-authoring`). The anchor for a reader who CAN resolve it + // is this comment — the ruling is #11567, and git history keeps it. + this.logger.warn?.( + `[security/reference-spelling] object "${object}": its controlled_by_parent master relation resolved ` + + `only from the REJECTED alias \`${alias}\` on field "${field}" — \`reference\` is the one ` + + 'relationship spelling @objectstack/spec declares, and this object reached the registry ' + + 'without being parsed (a raw registerObject skips Zod by design). Access is UNAFFECTED: ' + + 'this reader keeps accepting the alias precisely so no deployment loses access, and the ' + + 'relation still derives from the master. Rename the key to `reference` — every parsed ' + + 'authoring path already refuses this shape, so the two answers disagree until you do.', + { object, field, alias, master }, + ); + } + /** * ADR-0055 — master-detail "controlled by parent" READ derivation. * diff --git a/packages/verify/src/derive.test.ts b/packages/verify/src/derive.test.ts index b4c6c9000a..0016a60d98 100644 --- a/packages/verify/src/derive.test.ts +++ b/packages/verify/src/derive.test.ts @@ -1,7 +1,12 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { deriveCrudCases } from './derive'; +// `.js` extension, deliberately: under `moduleResolution: NodeNext` a relative +// import without it does not RESOLVE, so every symbol it names becomes `any` — +// and the type-layer ratchet reads this file (`TEST_DEBT`, via the package's +// tsconfig with the test exclusion lifted). The bare specifier here was +// costing 1 × TS2835 plus one TS7006 per callback parameter in the file. +import { deriveCrudCases } from './derive.js'; describe('deriveCrudCases — federated (external) objects (ADR-0015)', () => { it('blocks a read-only external object so verify never probe-inserts it', () => { @@ -43,3 +48,119 @@ describe('deriveCrudCases — federated (external) objects (ADR-0015)', () => { expect(c?.blocked).toBeFalsy(); }); }); + +/** + * [#13250] `reference_to` / `referenceTo` are REJECTED aliases of `reference` + * (#11567 — "one key, one answer"), and this deriver used to read them as + * accepted fallbacks. It was narrowed (maintainer ruling, 2026-08-30) because + * its failure mode is a report line rather than a refusal — but the narrowing + * is only safe if the report says WHY. + * + * ## What each half of this suite is for + * + * The NARROWING alone would be indistinguishable from a bug: an alias-spelled + * required relation would become `blocked` with the same generic "has no + * `reference` target" sentence an object with no relationship metadata at all + * gets. The operator would read "this object could not be derived" and never + * learn the cause was a key the platform refuses — one silent seam traded for + * another (#5262's defect). So every test that pins the narrowing has a + * partner pinning the REASON, and the reason assertions are written against + * the sentence's meaning (it names the alias, and names `reference` as what to + * write instead), not against its punctuation. + * + * The alias is genuinely reachable here — `loadConfig()` does not parse, and + * both a plain-object config and `defineStack(cfg, { strict: false })` carry an + * unparsed shape into `deriveCrudCases` — so these are not hypothetical + * fixtures. + */ +describe('deriveCrudCases — rejected `reference` aliases are narrowed AND named (#13250)', () => { + const withRef = (fieldDef: Record) => ({ + objects: [ + { name: 'company', fields: { title: { type: 'text' } } }, + { name: 'contact', fields: { company_id: fieldDef } }, + ], + }); + + it('a REQUIRED alias-spelled relation is blocked — the alias no longer derives a target', () => { + const c = deriveCrudCases( + withRef({ type: 'lookup', required: true, reference_to: 'company' }), + ).find((x) => x.object === 'contact'); + expect(c?.blocked).toBeTruthy(); + // ⛔ NOT a relationalRef: the whole point of the narrowing is that the + // alias stops resolving a target here. + expect(c?.relationalRefs).toBeUndefined(); + }); + + it('…and the block says WHY — it names the alias and names `reference` as the fix', () => { + const c = deriveCrudCases( + withRef({ type: 'lookup', required: true, reference_to: 'company' }), + ).find((x) => x.object === 'contact'); + expect(c?.blocked).toContain('reference_to'); + expect(c?.blocked).toMatch(/rejected alias/i); + expect(c?.blocked).toContain('company_id'); + // The target the app MEANT is echoed, so the reader can see the rename is + // mechanical rather than a metadata investigation. + expect(c?.blocked).toContain('company'); + // ⛔ The load-bearing NEGATIVE: it must not degrade to the generic sentence + // an object with no relationship metadata at all receives. Without this, + // narrowing to the generic message would pass every assertion above. + expect(c?.blocked).not.toMatch(/has no `reference` target/); + }); + + it('`referenceTo` is named too — the alias list is not just the snake_case one', () => { + const c = deriveCrudCases( + withRef({ type: 'master_detail', required: true, referenceTo: 'company' }), + ).find((x) => x.object === 'contact'); + expect(c?.blocked).toContain('referenceTo'); + expect(c?.blocked).toMatch(/rejected alias/i); + }); + + it('an OPTIONAL alias-spelled relation is skipped under its OWN reason, not the generic one', () => { + const c = deriveCrudCases( + withRef({ type: 'lookup', reference_to: 'company' }), + ).find((x) => x.object === 'contact'); + expect(c?.blocked).toBeFalsy(); + const reasons = (c?.skippedFields ?? []).map((s) => s.reason); + expect(reasons).toContain('relation-rejected-reference-alias:reference_to'); + expect(reasons).not.toContain('relation-missing-reference'); + }); + + it('a relation with NO target at all keeps the generic reason — the two findings stay distinct', () => { + const blocked = deriveCrudCases( + withRef({ type: 'lookup', required: true }), + ).find((x) => x.object === 'contact'); + expect(blocked?.blocked).toMatch(/has no `reference` target/); + expect(blocked?.blocked).not.toMatch(/rejected alias/i); + + const skipped = deriveCrudCases( + withRef({ type: 'lookup' }), + ).find((x) => x.object === 'contact'); + expect((skipped?.skippedFields ?? []).map((s) => s.reason)).toContain('relation-missing-reference'); + }); + + it('the canonical `reference` still derives a target — the narrowing did not break the live path', () => { + const c = deriveCrudCases( + withRef({ type: 'lookup', required: true, reference: 'company' }), + ).find((x) => x.object === 'contact'); + expect(c?.blocked).toBeFalsy(); + expect(c?.relationalRefs).toEqual([ + { field: 'company_id', target: 'company', required: true, multiple: false }, + ]); + }); + + it('canonical WINS over a stale alias on the same field, and reports nothing', () => { + const c = deriveCrudCases( + withRef({ type: 'lookup', required: true, reference: 'company', reference_to: 'stale_legacy' }), + ).find((x) => x.object === 'contact'); + expect(c?.blocked).toBeFalsy(); + expect(c?.relationalRefs?.[0]?.target).toBe('company'); + }); + + it('an EMPTY alias value is not a finding — there is no spelling to rename', () => { + const c = deriveCrudCases( + withRef({ type: 'lookup', required: true, reference_to: '' }), + ).find((x) => x.object === 'contact'); + expect(c?.blocked).toMatch(/has no `reference` target/); + expect(c?.blocked).not.toMatch(/rejected alias/i); + }); +}); diff --git a/packages/verify/src/derive.ts b/packages/verify/src/derive.ts index 2fb96e3e80..64064b3251 100644 --- a/packages/verify/src/derive.ts +++ b/packages/verify/src/derive.ts @@ -98,12 +98,60 @@ function synth(type: string, f: any): { value: unknown; kind: AssertKind } | nul } } -/** The target object a relational field references (snake_case object name), or null. */ +/** + * The spellings `@objectstack/spec` REJECTS for a relationship target. + * + * `FieldSchema` declares `reference` and only `reference` (#11567 — "one key, + * one answer"), answering any of these with `unrecognized_keys` and *"Did you + * mean `reference_to` → `reference`?"*. They are listed here so this deriver + * can NAME what an app spelled instead of silently deriving nothing from it. + */ +const REJECTED_REFERENCE_ALIASES = ['reference_to', 'referenceTo'] as const; + +/** + * The target object a relational field references (snake_case object name), or + * null — read from the CANONICAL `reference` key only. + * + * This reader used to accept `reference ?? reference_to ?? referenceTo`, and + * the alias is genuinely reachable here: `loadConfig()` does not parse, and + * both a plain-object config and `defineStack(cfg, { strict: false })` carry an + * unparsed shape straight into `deriveCrudCases`. It was narrowed anyway + * (maintainer ruling, 2026-08-30), because this consumer's failure mode is a + * REPORT LINE rather than a refusal: narrowing costs coverage, not + * availability. (The same alias is deliberately still tolerated by + * `resolveCbpRelation` in `@objectstack/plugin-security`, where a miss is + * fail-closed and narrowing would deny access rather than print something — + * that reader documents the asymmetry at its own site.) + * + * ⛔ The narrowing is only safe WITH {@link rejectedReferenceAlias} beside it. + * A verifier that quietly under-verifies is the defect #5262 was about, and + * degrading an alias-spelled relation to the generic "has no `reference` + * target" would have traded one silent seam for another: the operator would + * read "this object could not be derived" and never learn that the cause was a + * key the platform refuses. Every call site below therefore asks WHY before it + * reports THAT. + */ function relationTarget(f: any): string | null { - const ref = f?.reference ?? f?.reference_to ?? f?.referenceTo; + const ref = f?.reference; return typeof ref === 'string' && ref.length > 0 ? ref : null; } +/** + * The rejected alias this field spelled instead of `reference`, or null. + * + * Only ever consulted once {@link relationTarget} has already answered null, so + * a field carrying BOTH a canonical `reference` and a stale alias is derived + * from the canonical key and reports nothing — the alias is only news when it + * is the reason nothing was derived. + */ +function rejectedReferenceAlias(f: any): string | null { + for (const key of REJECTED_REFERENCE_ALIASES) { + const v = f?.[key]; + if (typeof v === 'string' && v.length > 0) return key; + } + return null; +} + interface Draft { name: string; body: Record; @@ -165,8 +213,24 @@ export function deriveCrudCases(config: any): CrudCase[] { if (RELATIONAL.has(type)) { const target = relationTarget(f); if (!target) { - if (isRequired) { d.blocked = `required ${type} field "${name}" has no \`reference\` target`; break; } - d.skippedFields.push({ name, type, reason: 'relation-missing-reference' }); + // NAME the cause. `alias` non-null means the app spelled a key + // `@objectstack/spec` refuses, which is a different finding from "no + // target was declared at all" and needs a different sentence: one is + // a rename, the other is missing metadata. + const alias = rejectedReferenceAlias(f); + if (isRequired) { + d.blocked = alias + ? `required ${type} field "${name}" spells the rejected alias \`${alias}\` instead of ` + + `\`reference\` — \`reference\` is the only relationship spelling @objectstack/spec ` + + `declares, so this app's target "${f[alias]}" was not derived; rename the key` + : `required ${type} field "${name}" has no \`reference\` target`; + break; + } + d.skippedFields.push({ + name, + type, + reason: alias ? `relation-rejected-reference-alias:${alias}` : 'relation-missing-reference', + }); continue; } if (!byName.has(target)) { diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index 1f52cbaa6d..9952d97063 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -1088,7 +1088,16 @@ const TEST_DEBT = { + 'src/validate-visibility-predicates.test.ts held 10 of the 32 and reports none today.', }, '@objectstack/formula': { errors: 17, note: 'TS2591 x6 (`process`), TS2345 x3, TS2352 x3, TS1470 x2, TS2339 x2. Re-measured 17 at 5ab08428, up from 12; the TS2591 half doubled, which is the missing `types:["node"]` again rather than five new defects.' }, - '@objectstack/verify': { errors: 8, note: 'TS2835 x4, TS7006 x4. Re-measured 8 at 5ab08428, up from 6; both classes are the NodeNext pair from the top-of-ledger note.' }, + '@objectstack/verify': { + errors: 3, + note: 'TS2835 x3 -- `harness.host-resolution`, `harness.posture-only` and `harness.posture` each ' + + 'import `./harness` without the `.js` extension. Re-tallied from the 8 measured at 5ab08428 ' + + '(TS2835 x4, TS7006 x4) when `derive.test.ts` gained its own extension: that ONE unresolved ' + + 'import was carrying 1 x TS2835 plus every TS7006 in the file, because a specifier that does ' + + 'not resolve under NodeNext makes every symbol it names `any` and so every callback parameter ' + + 'implicitly any. The remainder is the same NodeNext pair from the top-of-ledger note, and the ' + + 'same one-line fix graduates this entry.', + }, '@objectstack/connector-mcp': { errors: 5, note: 'TS2339 x5. Re-measured 5 at 5ab08428, exact.' }, '@objectstack/connector-openapi': { errors: 5, note: 'TS2339 x5. Re-measured 5 at 5ab08428, exact.' }, '@objectstack/http-conformance': {