diff --git a/.changeset/24468-lint-rule-hop-undeclared-field.md b/.changeset/24468-lint-rule-hop-undeclared-field.md new file mode 100644 index 00000000000..aaa5c9c0cfc --- /dev/null +++ b/.changeset/24468-lint-rule-hop-undeclared-field.md @@ -0,0 +1,20 @@ +--- +'@objectstack/lint': minor +--- + +fix(lint)!: a validation rule that reads, through a reference, a field the reference's target does not declare is refused where it is written + +Clause-②: no (narrowing) + + + +**BREAKING** (an authoring narrowing), shipped as `minor` under the launch-window convention for breaking changes. + +The validation-rule pass judges the field a rule's `condition` reads one hop through a reference against the target object's fields: the fields it declares, the system columns the platform adds to it, and `id`. It is the same check the field-formula pass makes for a formula. `os validate`, `os build` and the object save door (`PUT /api/v1/meta/object/:name`) run the same pass, under the existing `EXPRESSION_INVALID` rule. It covers a `script` or `cross_field` rule's `condition`, at the top level and inside a `conditional` rule's `then` / `otherwise`. + +- FROM: `record.account.region == 'north'`, where `account` is a lookup to an object that declares no `region`, saved with a 200, and every write the rule judged was refused. TO: refused (`422 INVALID_METADATA` at the save door). The message names the field and the target: "unknown field `region` on ``". +- `record.account.status == 'closed'`, where the target declares `status`, passes as before. +- A target the stack does not carry is not judged. At the save door the stack carries the live registry's objects in the written package's closure, so a target from a package it depends on is judged. +- A reference read more than one hop deep (`record.account.owner.email`) keeps its own refusal and is not judged a second time. + +What to do. Read a field the target declares, or declare the field on the target object. Do not declare it on the object that carries the rule. At `os build` and `os validate` this pass does not see a target field that an `objectExtensions` entry contributes, while the save door judges the merged object. diff --git a/.changeset/24468-objectql-rule-hop-sibling-isolation.md b/.changeset/24468-objectql-rule-hop-sibling-isolation.md new file mode 100644 index 00000000000..c320b5190fb --- /dev/null +++ b/.changeset/24468-objectql-rule-hop-sibling-isolation.md @@ -0,0 +1,16 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): a validation rule reading a field its reference's target does not declare no longer refuses every write through the other rules reading the same reference + +Clause-②: no + +A write loads, once per page, every column the object's validation rules read through each reference. When one rule named a field the target does not declare, the whole reference was answered as undeclared. Every rule reading through it then had no verdict and refused the write, each naming the other rule's field. + +- The undeclared fields are left out of that shared read and judged for each rule on its own. +- A rule that names one has no verdict and refuses the write, naming only its own undeclared fields. +- A rule that reads only declared fields through the same reference gives the verdict it gives alone: it admits what it admits alone, and refuses with its own message. +- The related read keeps its authority and its bound; an undeclared field never enters it. + +What to do. Nothing. To find the rule at fault, re-save the object: the save door refuses the misspelt read and names the field. diff --git a/packages/lint/src/runtime-gate.object-validation-writes.test.ts b/packages/lint/src/runtime-gate.object-validation-writes.test.ts index 8b7e66fe6fe..f6c67ca4da3 100644 --- a/packages/lint/src/runtime-gate.object-validation-writes.test.ts +++ b/packages/lint/src/runtime-gate.object-validation-writes.test.ts @@ -331,3 +331,54 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver expect(expressionFindings(result.errors), dump(result)).toEqual([]); }); }); + +describe('#24468 — the object door refuses a rule hop to a field the target does not declare', () => { + /** The lookup's target, delivered by ANOTHER package the written one depends on. */ + const account = { + name: 'fx_account', label: 'Account', sharingModel: 'private', _packageId: 'com.example.accounts', + fields: { status: { type: 'text', label: 'Status' } }, + }; + const order = (validations: unknown[]) => ({ + name: 'fx_order', + label: 'Order', + sharingModel: 'private', + fields: { account: { type: 'lookup', label: 'Account', reference: 'fx_account' } }, + validations, + }); + const rule = (name: string, condition: string) => ({ name, type: 'script', condition, message: 'x' }); + const gateOrder = (validations: unknown[]) => runRuntimeAuthoringRules({ + type: 'object', + item: order(validations), + context: { objects: [account] }, + packageScope: { packageId: 'com.example.orders', dependencies: ['com.example.accounts'] }, + }); + + it('⭐ LIT — the rule is REFUSED under EXPRESSION_INVALID, naming the field and the target from the other package; its valid sibling is not', () => { + const result = gateOrder([rule('open_account', "record.account.status == 'closed'"), rule('north_only', "record.account.region == 'north'")]); + + const errs = expressionFindings(result.errors); + expect(errs, dump(result)).toHaveLength(1); + const where = "object 'fx_order' · validation 'north_only'"; + expect(errs[0]).toMatchObject({ severity: 'error', where, path: where }); + expect(errs[0]!.message).toContain('unknown field `region` on `fx_account`'); + expect(errs[0]!.message).toContain('the rule has no verdict'); + }); + + it('⭐ LIT — a nested `condition` inside a `conditional` is judged too', () => { + const result = gateOrder([{ + type: 'conditional', name: 'outer', when: 'record.account != null', message: 'x', + then: rule('inner', "record.account.region == 'north'"), + }]); + + const errs = expressionFindings(result.errors); + expect(errs.map((e) => e.where), dump(result)).toEqual(["object 'fx_order' · validation rule 'outer' then → 'inner'"]); + expect(errs[0]!.message).toContain('unknown field `region` on `fx_account`'); + }); + + it('⭐ CONTROL — a rule reading a field the target declares publishes clean', () => { + const result = gateOrder([rule('open_account', "record.account.status == 'closed'")]); + + expect(expressionFindings(result.errors), dump(result)).toEqual([]); + expect(expressionFindings(result.advisories), dump(result)).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-expressions.rule-one-hop.test.ts b/packages/lint/src/validate-expressions.rule-one-hop.test.ts new file mode 100644 index 00000000000..3d7ae893783 --- /dev/null +++ b/packages/lint/src/validate-expressions.rule-one-hop.test.ts @@ -0,0 +1,73 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24468] A validation rule's `condition` reads ONE hop through a reference + * field, and the validation-rule pass judges the hop's COLUMN against the + * target's field index, as the field-formula pass does for a formula (#24410). + * + * ObjectQL's predicate read loads only the columns the target declares; a rule + * naming one it does not declare has no verdict and refuses every write it + * judges. So the build, `os validate` and the object save door refuse it, + * naming the field and the target, under the existing `expression-invalid` + * rule. The runtime half is `packages/objectql/src/engine-predicate-relationship.test.ts`. + * Every fixture goes through `specValid`. + */ +import { describe, expect, it } from 'vitest'; +import { ObjectStackSchema } from '@objectstack/spec'; + +import { validateStackExpressions } from './validate-expressions.js'; + +const MANIFEST = { id: 'com.example.rule-hop', name: 'rule_hop', version: '1.0.0', type: 'app' } as const; + +/** A fixture is only a fixture if the spec accepts it. */ +function specValid(stack: Record): Record { + const result = ObjectStackSchema.safeParse({ manifest: MANIFEST, ...stack }); + if (!result.success) { + throw new Error( + 'fixture is not spec-valid: ' + + result.error.issues.map((i) => `${i.path.join('.') || '(root)'}: ${i.message}`).join('; '), + ); + } + return result.data as unknown as Record; +} + +const account = { name: 'hop_account', label: 'Account', fields: { status: { type: 'text', label: 'Status' } } }; +const order = (condition: string, reference = 'hop_account') => ({ + name: 'hop_order', + label: 'Order', + fields: { + amount: { type: 'number', label: 'Amount' }, + account: { type: 'lookup', label: 'Account', reference }, + }, + validations: [{ name: 'r', type: 'script', severity: 'error', message: 'x', condition }], +}); + +const WHERE = "object 'hop_order' · validation 'r'"; + +const errorsOf = (condition: string, reference?: string) => + validateStackExpressions(specValid({ objects: [account, order(condition, reference)] })) + .filter((i) => i.where === WHERE && (i.severity ?? 'error') === 'error'); + +describe('[#24468] a validation rule reads one hop through a reference', () => { + it('admits a read of a field the target declares', () => { + expect(errorsOf("record.account.status == 'closed'")).toEqual([]); + }); + + it('refuses a read of a field the target does not declare, naming the field and the target', () => { + const errors = errorsOf("record.account.region == 'north' && record.account.status == 'open'"); + expect(errors).toHaveLength(1); + expect(errors[0].message).toMatch(/^unknown field `region` on `hop_account`/); + expect(errors[0].message).toContain('through the reference `record.account`'); + expect(errors[0].message).toContain("declare 'region' on 'hop_account' — not on 'hop_order', the object carrying this rule"); + expect(errors[0].source).toBe("record.account.region == 'north' && record.account.status == 'open'"); + }); + + it('admits the primary key and a column the platform injects on the target', () => { + expect(errorsOf("record.account.id != ''")).toEqual([]); + expect(errorsOf('record.account.created_at != null')).toEqual([]); + }); + + it('does not judge a target the stack does not carry', () => { + expect(errorsOf("record.account.region == 'north'", 'hop_elsewhere')).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 8ea04ffdd56..cf3f006a361 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -3549,6 +3549,10 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t // metadata keys; the metadata keys the index is built from (`type`, // `reference`) are read off the tabled `def` receiver. 'storedMetadataBodyReferenceIndex', 'familyRefs', 'traversals', + // [#24468] The undeclared-hop refusal's whole traversal analysis, read for + // its `traversals` and `multiHopFields` — the formula package's own + // result keys, never metadata keys. + 'analysis', ]); expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]); }); diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 82a510dfb52..3c4940e7459 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -1774,12 +1774,25 @@ function storedMetadataBodyHopMessage( * words, so the two first-segment and second-segment verdicts read as one * family, and names the read, the target and where the field would have to be * declared — never on the object carrying the formula. + * + * [#24468] The same refusal for a validation rule's hydrated `condition`; only + * the consequence differs: the rule has no verdict, so every write it judges + * is refused. */ -function undeclaredHopFieldMessage(objectName: string, field: string, target: string, column: string): string { - return `unknown field \`${column}\` on \`${target}\`: this formula reads it through the reference ` - + `${celMemberPath('record', field)}, and '${target}' declares no field '${column}', so the formula reads ` - + `null on every row. Read a field '${target}' declares, or declare '${column}' on '${target}' — not on ` - + `'${objectName}', the object carrying this formula.`; +function undeclaredHopFieldMessage( + site: 'formula' | 'rule', + objectName: string, + field: string, + target: string, + column: string, +): string { + const consequence = site === 'formula' + ? 'so the formula reads null on every row' + : 'so the rule has no verdict and every write it judges is refused'; + return `unknown field \`${column}\` on \`${target}\`: this ${site} reads it through the reference ` + + `${celMemberPath('record', field)}, and '${target}' declares no field '${column}', ${consequence}. ` + + `Read a field '${target}' declares, or declare '${column}' on '${target}' — not on ` + + `'${objectName}', the object carrying this ${site}.`; } /** @@ -1969,6 +1982,11 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression * written the formula reads null on every row; this says so where it is * written, at the build and at the object save door alike. * + * [#24468] The two sites that serve a hop share it: a formula field's + * `expression` ({@link judgeFieldFormula}) and a validation rule's hydrated + * `condition` ({@link check} with `traversalHydration`), where the rule has + * no verdict and refuses every write it judges. + * * Judged against the target's field index — its declared fields and the * system columns the platform injects on it, the set the registry carries — * and the primary key, which no field map carries and the loader reads. A @@ -1977,22 +1995,30 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression * not judged; a hop into the stored-metadata-body family is refused by * {@link refuseStoredMetadataBodyHop} and not judged twice. */ - const refuseUndeclaredHopField = (where: string, raw: unknown, objectName: string): void => { + const refuseUndeclaredHopField = ( + where: string, + raw: unknown, + objectName: string, + site: 'formula' | 'rule', + ): void => { const source = celSourceOf(raw); if (!source) return; - const traversals = analyzeRelationshipTraversals(source, 'record')?.traversals; + const analysis = analyzeRelationshipTraversals(source, 'record'); + const traversals = analysis?.traversals; if (!traversals || traversals.size === 0) return; for (const obj of objects) { if (obj.name !== objectName) continue; for (const [field, def] of fieldEntries(obj)) { - const columns = traversals.get(field); + // [#24468] A reference read more than one hop deep is refused whole by + // the traversal checks; its first column is not judged a second time. + const columns = analysis?.multiHopFields.has(field) ? undefined : traversals.get(field); const target = columns ? referenceTargetOf(def)?.trim() : undefined; if (!columns || !target || isStoredMetadataBodyObject(target) || !fieldIndex.has(target)) continue; for (const column of [...columns].sort()) { if (column === 'id' || fieldIndex.get(target)?.includes(column)) continue; issues.push({ where, - message: undeclaredHopFieldMessage(objectName, field, target, column), + message: undeclaredHopFieldMessage(site, objectName, field, target, column), source, severity: 'error', }); @@ -2180,6 +2206,7 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression } for (const w of res.warnings) issues.push({ where, message: w.message, source: w.source, severity: 'warning' }); if (objectName && traversalHydration) refuseStoredMetadataBodyHop(where, raw, objectName, 'rule'); + if (objectName && traversalHydration) refuseUndeclaredHopField(where, raw, objectName, 'rule'); // [#8116] Provenance rides every object-bound predicate this helper // validates, whichever scope: the `record`/`previous` roots are explicit // in the source, so flattened flow conditions are covered without ever @@ -2902,7 +2929,7 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression for (const w of res.warnings) issues.push({ where: fieldWhere, message: w.message, source: w.source, severity: 'warning' }); if (objectName) refuseStoredMetadataBodyHop(fieldWhere, f.expression, objectName, 'formula'); // [#24410] And a hop to a field the target does not declare. - if (objectName) refuseUndeclaredHopField(fieldWhere, f.expression, objectName); + if (objectName) refuseUndeclaredHopField(fieldWhere, f.expression, objectName, 'formula'); // [#8116] Formulas read the same record binding the predicates do, so // a `record.` read inside one degrades identically. warnUnprovisionedAnchors(fieldWhere, f.expression, objectName); diff --git a/packages/objectql/src/engine-predicate-relationship.test.ts b/packages/objectql/src/engine-predicate-relationship.test.ts index 6f1ec79a460..fa3637d4a8e 100644 --- a/packages/objectql/src/engine-predicate-relationship.test.ts +++ b/packages/objectql/src/engine-predicate-relationship.test.ts @@ -231,6 +231,59 @@ describe('#18682 — engine-produced relationship bindings', () => { } }); + // [#24468] One rule's undeclared column is judged against THAT rule: a valid + // sibling reading through the same reference keeps the verdict it gives + // alone, and the bad rule's refusal names only its own field. + describe('[#24468] a sibling of a rule reading an undeclared related field', () => { + const refusal = async (data: Record): Promise => { + try { + await engine.insert('crm_opportunity', data, { context: ACTING } as any); + } catch (e) { + return JSON.stringify((e as { errors?: unknown }).errors ?? (e as Error).message); + } + return ''; + }; + + beforeEach(() => { + engine.registry.registerObject({ + name: 'crm_opportunity', + fields: { + name: { type: 'text' }, amount: { type: 'number' }, + account: { type: 'lookup', reference: 'crm_account' }, + }, + validations: [ + { + name: 'partner_cap', type: 'script', severity: 'error', + message: 'Partner accounts are capped at 10000.', + condition: "record.account.type == 'partner' && record.amount > 10000", + }, + { name: 'sneaky', type: 'script', severity: 'error', message: 'x', + condition: "record.account.not_a_column == 'x'" }, + ], + } as any, 'test-package'); + d.calls.length = 0; + }); + + it('the valid rule admits what it admits alone; only the bad rule refuses, naming its own field', async () => { + const text = await refusal({ name: 'A', amount: 50000, account: 'acc_d' }); + expect(text).toContain("Validation rule 'sneaky' could not be evaluated"); + expect(text).toContain("declares no 'not_a_column'"); + expect(text).not.toContain('partner_cap'); + }); + + it('the valid rule refuses with its own message, as it does alone', async () => { + const text = await refusal({ name: 'A', amount: 50000, account: 'acc_p' }); + expect(text).toContain('Partner accounts are capped'); + expect(text).not.toContain("Validation rule 'partner_cap' could not be evaluated"); + }); + + it('the shared system read names the declared columns only, once', async () => { + await refusal({ name: 'A', amount: 10, account: 'acc_d' }); + expect(relatedReads()).toHaveLength(1); + expect([...(relatedReads()[0].ast?.fields ?? [])].sort()).toEqual(['id', 'type']); + }); + }); + // The refusal is about the WRITE, never about the value. A caller can infer a // value by observing refusals — an accepted, deliberately narrow channel — so // nothing may make the refusal more informative than "this rule refused". diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 53961e58c5a..c74d2a1ba00 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -10242,7 +10242,43 @@ export class ObjectQL implements IObjectQLEngine { } const wanted = collectPredicateRelationships(schema); if (wanted.size === 0) return unbound; - return this.resolveOneHopRelated(schema, wanted, rows, context, 'predicate'); + // [#24468] The read set per reference is the UNION of every rule's + // columns, and the loader answers a whole reference `undeclared-field` when + // any column of it is undeclared — so one rule's misspelt column left every + // sibling rule through the same reference without a verdict, each refusing + // the write with the other rule's field. The undeclared columns are taken + // out of the shared read here and judged per rule (`resolveTraversalScope` + // reads `undeclaredFields` against the rule's own columns); the loader + // reads the declared rest, under the same authority and bound as before. + // ⛔ Nothing undeclared enters the system-authority read either way. + const fields = (schema?.fields ?? {}) as Record; + const undeclared = new Map(); + for (const [fk, named] of wanted) { + const target = referenceTargetOf(fields[fk]); + // The loader's own verdicts stand for these: no target, or a hop into + // the stored-metadata-body family, which is never read. + if (!target || isStoredMetadataBodyObject(target)) continue; + const declared = (this._registry.getObject(target) as { fields?: Record } | undefined)?.fields; + const missing = undeclaredRelatedFields(declared, named); + if (missing.length === 0) continue; + undeclared.set(fk, { object: target, fields: missing }); + for (const name of missing) named.delete(name); + if (named.size === 0) wanted.delete(fk); + } + const lookup = wanted.size > 0 + ? await this.resolveOneHopRelated(schema, wanted, rows, context, 'predicate') + : unbound; + if (undeclared.size === 0) return lookup; + return (row) => { + if (!row) return undefined; + const binding: Record = { ...(lookup(row) ?? {}) }; + for (const [fk, { object, fields: missing }] of undeclared) { + // A reference no rule reads a declared column through has no row to + // serve; the binding says why, and every rule through it names its own. + binding[fk] = { ...(binding[fk] ?? { object, unavailable: 'undeclared-field' }), undeclaredFields: missing }; + } + return binding; + }; } /** diff --git a/packages/objectql/src/federated-injected-column-readers.test.ts b/packages/objectql/src/federated-injected-column-readers.test.ts index 273666e74a7..83dc53d56fb 100644 --- a/packages/objectql/src/federated-injected-column-readers.test.ts +++ b/packages/objectql/src/federated-injected-column-readers.test.ts @@ -191,6 +191,12 @@ const READERS: Record = { disposition: 'caller-predicate', why: "judges a formula's own hop against the target's declared field map; no row is read here", }, + // [#24468] The predicate read names each reference's target to judge, per + // validation rule, the related fields that target does not declare. + 'engine.ts#resolvePredicateRelated :: referenceTargetOf()': { + disposition: 'caller-predicate', + why: "judges a validation rule's own hop against the target's declared field map; no row is read here", + }, 'engine.ts#resolveSystemInsertOrganization :: isFederatedObject()': { disposition: 'exempt', why: 'a system insert into a federated object is never stamped with an organization', diff --git a/packages/objectql/src/validation/rule-relationship-traversal.test.ts b/packages/objectql/src/validation/rule-relationship-traversal.test.ts index 06efc89a9f0..f301e1a5013 100644 --- a/packages/objectql/src/validation/rule-relationship-traversal.test.ts +++ b/packages/objectql/src/validation/rule-relationship-traversal.test.ts @@ -436,3 +436,52 @@ describe('#20006 — the reference-cleanup binding changes a text, never a verdi expect(message).toMatch(/^Validation rule 'r' could not be evaluated \(runtime: No such key: kind\)/); }); }); + +describe('[#24468] undeclared related fields are judged per RULE, never per reference', () => { + // The engine leaves every rule's undeclared columns out of the shared read + // and lists them on the binding beside the row the declared rest read. + const withUndeclared = (r: Record) => ({ ...row(r), undeclaredFields: ['region'] }); + const both = { + fields: opportunity.fields, + validations: [ + opportunity.validations[0], + { name: 'north_only', type: 'script', severity: 'error', message: 'north', condition: "record.account.region == 'north'" }, + ], + }; + const errorsOf = (data: Record, related: Record): string => { + try { + evaluateValidationRules(both as any, data, 'insert', { related: related as never }); + } catch (e) { + return JSON.stringify((e as unknown as { errors?: unknown }).errors ?? (e as Error).message); + } + return ''; + }; + + it('a valid sibling keeps its own verdict: it admits, and only the bad rule refuses, naming its own field', () => { + const text = errorsOf({ name: 'A', amount: 50000, account: 'acc_1' }, + { account: withUndeclared({ id: 'acc_1', type: 'direct' }) }); + expect(text).toContain("Validation rule 'north_only' could not be evaluated"); + expect(text).toContain("declares no 'region'"); + expect(text).not.toContain('partner_cap'); + }); + + it('a valid sibling keeps its own verdict: it refuses with its own message', () => { + const text = errorsOf({ name: 'A', amount: 50000, account: 'acc_1' }, + { account: withUndeclared({ id: 'acc_1', type: 'partner' }) }); + expect(text).toContain('Partner accounts are capped at 10000.'); + expect(text).not.toContain("Validation rule 'partner_cap' could not be evaluated"); + }); + + it('the bad rule names only its own undeclared field, not a sibling\'s', () => { + const text = errorsOf({ name: 'A', amount: 1, account: 'acc_1' }, + { account: { ...row({ id: 'acc_1', type: 'direct' }), undeclaredFields: ['region', 'tier'] } }); + expect(text).toContain("declares no 'region'"); + expect(text).not.toContain("'tier'"); + }); + + it('with no declared column left, every rule through the reference still names its own', () => { + const only = { fields: opportunity.fields, validations: [both.validations[1]] }; + expect(() => evaluateValidationRules(only as any, { name: 'A', account: 'acc_1' }, 'insert', + { related: { account: unavailable('undeclared-field', ['region']) } as never })).toThrow(/declares no 'region'/); + }); +}); diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 197820edaa5..65c8a0d29e3 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -720,7 +720,13 @@ export interface RelatedFieldBinding { readonly row?: Record; /** Why `row` is absent. Present iff `row` is absent. */ readonly unavailable?: RelatedUnavailableReason; - /** For `undeclared-field`: the named fields the related object does not declare. */ + /** + * The named fields the related object does not declare. [#24468] Judged per + * RULE, never per reference: a rule naming one of them has no verdict and + * refuses, naming only its own; a rule naming none is judged on `row` (or + * `unavailable`) as if no other rule read through this reference. With no + * declared column left to read, `unavailable` is `undeclared-field`. + */ readonly undeclaredFields?: readonly string[]; } @@ -4184,6 +4190,24 @@ function resolveTraversalScope( }; } + // [#24468] This rule's own undeclared reads first, before any row: they are a + // fault of the expression, whatever the record holds. The engine leaves the + // undeclared columns of every rule out of the shared read and lists them on + // the binding (`undeclaredFields`); a rule naming none of them is served + // from the read exactly as it is alone, and one naming any refuses, naming + // only its own. + for (const [field, named] of analysis.traversals) { + if (!isReference(field)) continue; + const binding = related?.[field]; + const own = binding?.undeclaredFields?.filter((n) => named.has(n)) ?? []; + if (binding && own.length > 0) { + return { + ok: false, + ...traversalRefusal(field, { object: binding.object, unavailable: 'undeclared-field', undeclaredFields: own }, named), + }; + } + } + // (2)/(3) Hydrate what is available; refuse on the first field that is not. let copy: Record | undefined; for (const field of analysis.traversals.keys()) { diff --git a/packages/qa/dogfood/test/rule-hop-undeclared-related-field.dogfood.test.ts b/packages/qa/dogfood/test/rule-hop-undeclared-related-field.dogfood.test.ts new file mode 100644 index 00000000000..24de878f445 --- /dev/null +++ b/packages/qa/dogfood/test/rule-hop-undeclared-related-field.dogfood.test.ts @@ -0,0 +1,143 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#24468] A validation rule that reads, through a lookup, a field the +// lookup's target does not declare — on a real boot: the real `ObjectQL`, SQL +// driver, REST, auth and metadata layers. +// +// ## The two halves +// +// - The save door. `PUT /api/v1/meta/object/:name` refuses a rule condition +// reading `record.account.region` when `rrh_account` declares no `region`, +// under the existing `EXPRESSION_INVALID` rule, naming the field and the +// target object. The same object with only the rule reading a declared field +// saves. +// - The write. A rule already stored with such a read (here: declared in code, +// which this door never judged) refuses every write, naming its own field; +// a VALID sibling rule reading through the SAME lookup keeps the verdict it +// gives alone — it admits an order on an open account and refuses one on a +// closed account with its own message. +// +// Fixtures are synthetic. ⚠️ No test title states a value. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; + +const SYS = { context: { isSystem: true } } as const; +/** The valid rule: an order needs an account that is not closed. */ +const VALID = "record.account.status == 'closed'"; +const VALID_MESSAGE = 'RRHVALID81: the account is closed'; +/** Reads a field `rrh_account` does not declare. */ +const UNDECLARED = "record.account.region == 'north'"; + +const object = (name: string, fields: Record, validations: unknown[]) => + ObjectSchema.create({ name, label: name, pluralLabel: name, sharingModel: 'public_read_write', fields, validations } as never); + +const orderFields = () => ({ + label: Field.text({ label: 'Label' }), + account: Field.lookup('rrh_account', { label: 'Account' }), +}); +const validRule = { name: 'open_account', type: 'script', severity: 'error', message: VALID_MESSAGE, condition: VALID }; +const badRule = { name: 'north_only', type: 'script', severity: 'error', message: 'RRHBAD82', condition: UNDECLARED }; + +const fixtureStack = defineStack({ + manifest: { + id: 'com.dogfood.rule-hop-undeclared', namespace: 'rrh', version: '0.0.0', type: 'app', + name: 'Rule Hop Undeclared Field Fixture', + description: 'Orders whose validation rules read through a lookup to an account.', + }, + objects: [ + object('rrh_account', { status: Field.text({ label: 'Status' }) }, []), + // The stored bad rule: declared in code, beside the valid sibling. + object('rrh_order', orderFields(), [validRule, badRule]), + // Control: the valid rule alone. + object('rrh_order_alone', orderFields(), [validRule]), + ], +}); + +/** The object body a `PUT /meta/object/:name` writes: plain JSON, as Studio or an AI author sends it. */ +const saveBody = (name: string, validations: unknown[]) => ({ + name, + label: name, + pluralLabel: name, + sharingModel: 'public_read_write', + fields: { + label: { type: 'text', label: 'Label' }, + account: { type: 'lookup', label: 'Account', reference: 'rrh_account' }, + }, + validations, +}); + +type Row = Record; + +describe('[#24468] a validation rule reading a field its lookup target does not declare', () => { + let stack: VerifyStack; + let token = ''; + let openId = ''; + let closedId = ''; + + const call = async (method: string, path: string, body?: unknown) => { + const res = await stack.apiAs(token, method, path, body); + const json: any = await res.json().catch(() => ({})); + return { status: res.status, json }; + }; + /** Every refusal text the write's response carries, joined. */ + const refusalText = (json: any): string => JSON.stringify(json ?? {}); + + beforeAll(async () => { + stack = await bootStack(fixtureStack as unknown as Parameters[0]); + const ql = (await stack.kernel.getServiceAsync('objectql')) as any; + token = await stack.signIn(); + openId = String((await ql.insert('rrh_account', { status: 'open' }, SYS)).id); + closedId = String((await ql.insert('rrh_account', { status: 'closed' }, SYS)).id); + }, 180_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('the save door refuses the rule, naming the field and the target object', async () => { + const saved = await call('PUT', '/meta/object/rrh_order_saved', saveBody('rrh_order_saved', [validRule, badRule])); + expect(saved.status, JSON.stringify(saved.json)).toBe(422); + expect(saved.json?.code, JSON.stringify(saved.json)).toBe('INVALID_METADATA'); + const issues = (saved.json?.issues ?? []) as Row[]; + // The existing `EXPRESSION_INVALID` rule, located at the rule the author edits. + expect(issues.map((i) => [i.rule, i.where]), JSON.stringify(saved.json)).toEqual([ + ['expression-invalid', "object 'rrh_order_saved' · validation 'north_only'"], + ]); + expect(String(issues[0].message)).toMatch(/^unknown field `region` on `rrh_account`/); + }); + + it('control: the same object with only the rule reading a declared field saves', async () => { + const saved = await call('PUT', '/meta/object/rrh_order_ok', saveBody('rrh_order_ok', [validRule])); + expect(saved.status, JSON.stringify(saved.json)).toBe(200); + }); + + it('control: the valid rule alone admits an open account and refuses a closed one', async () => { + const open = await call('POST', '/data/rrh_order_alone', { label: 'open', account: openId }); + expect(open.status, JSON.stringify(open.json)).toBe(201); + const closed = await call('POST', '/data/rrh_order_alone', { label: 'closed', account: closedId }); + expect(closed.status, JSON.stringify(closed.json)).toBe(400); + expect(refusalText(closed.json)).toContain(VALID_MESSAGE); + }); + + it('beside a stored bad rule, the valid rule keeps its own verdict and the bad one names only its own field', async () => { + // An open account: the valid rule admits it; only the bad rule refuses, + // and the refusal names `region`, never the valid rule. + const open = await call('POST', '/data/rrh_order', { label: 'open', account: openId }); + expect(open.status, JSON.stringify(open.json)).toBe(400); + const openText = refusalText(open.json); + expect(openText).toContain("Validation rule 'north_only' could not be evaluated"); + expect(openText).toContain("declares no 'region'"); + expect(openText).not.toContain("Validation rule 'open_account'"); + expect(openText).not.toContain(VALID_MESSAGE); + + // A closed account: the valid rule refuses with its OWN message, as alone. + const closed = await call('POST', '/data/rrh_order', { label: 'closed', account: closedId }); + expect(closed.status, JSON.stringify(closed.json)).toBe(400); + const closedText = refusalText(closed.json); + expect(closedText).toContain(VALID_MESSAGE); + expect(closedText).not.toContain("Validation rule 'open_account' could not be evaluated"); + }); +});